From 9a6584096b679d0c6cd5ff69de01dc5aea4c7ff5 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 5 May 2026 22:37:10 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix:=20stuck=20"Preparing=20prev?= =?UTF-8?q?iew=E2=80=A6"=20+=20inline=20the=20chip=20subtitle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related fixes for a stuck-spinner bug a user reported in manual testing of PR #12957. **Stuck spinner (the bug)** The deferred preview render can complete a few seconds AFTER the SSE stream closes (typical case: PPTX render finishes ~3s after the LLM emits FINAL). When that happens, the SSE update is silently dropped (`isStreamWritable` returns false on a closed stream) and polling is the only recovery path. The earlier polling gate was `status === 'pending' && isAnySubmitting`, which mirrored the original design intent ("only query while the LLM is still generating"). But `isAnySubmitting` flips false the moment the model emits FINAL — milliseconds before the deferred render commits. Polling never runs, the chip stays "Preparing preview…" forever even though the DB has `status: 'ready'` with valid HTML. Drop the `isAnySubmitting` part of the gate. `useFilePreview`'s `refetchInterval` is already a function-form that returns `false` on the first terminal response, so polling auto-stops within one tick of resolution. The server-side render ceiling (60s) plus the lazy sweep in the preview endpoint cap the worst case to ~24 polls per pending attachment. Polling itself never blocks UX — the gate's purpose was "don't waste cycles", and capping by terminal status is the correct expression of that. **Inline the chip subtitle (the visual)** The previous design rendered "Preparing preview…" as a loose-feeling spinner+text BELOW the file chip. The chip itself looked done while a floating annotation said it wasn't. `FileContainer` gains an optional `subtitle?: ReactNode` prop that overrides the default file-type label. `Attachment.tsx` passes a `PreviewStatusSubtitle` (spinner + "Preparing preview…" / alert + "Preview unavailable") into that slot when the file's preview is pending or failed. The chip footprint stays identical to its `'ready'` form — just the second row swaps from "PowerPoint Presentation" to the status indicator. No floating element, no layout shift. Tests: regression test pinning down "polling stays enabled after the LLM finishes" so a future revert can't reintroduce the stuck-spinner bug. Existing FileContainer tests pass unchanged (subtitle override is opt-in). 522 frontend tests pass; lint clean. --- .../Chat/Input/Files/FileContainer.tsx | 21 ++++++- .../Messages/Content/Parts/Attachment.tsx | 55 ++++++++++--------- .../useAttachmentPreviewSync.spec.tsx | 16 ++++-- .../hooks/Files/useAttachmentPreviewSync.ts | 26 ++++++--- 4 files changed, 75 insertions(+), 43 deletions(-) diff --git a/client/src/components/Chat/Input/Files/FileContainer.tsx b/client/src/components/Chat/Input/Files/FileContainer.tsx index a4b902fac4..551f477a84 100644 --- a/client/src/components/Chat/Input/Files/FileContainer.tsx +++ b/client/src/components/Chat/Input/Files/FileContainer.tsx @@ -1,3 +1,4 @@ +import type { ReactNode } from 'react'; import type { TFile } from 'librechat-data-provider'; import type { ExtendedFile } from '~/common'; import { getFileType, cn } from '~/utils'; @@ -8,6 +9,7 @@ const FileContainer = ({ file, overrideType, displayName, + subtitle, buttonClassName, containerClassName, onDelete, @@ -21,6 +23,15 @@ const FileContainer = ({ * persisted user files leave this undefined and render the raw filename. */ displayName?: string; + /** + * Optional override for the subtitle line (defaults to the file + * type's localized title — e.g. "PowerPoint Presentation"). Used by + * the deferred-preview flow to surface "Preparing preview…" / + * "Preview unavailable" inline within the chip rather than as a + * loose-feeling annotation below it. Pass a ReactNode so callers + * can include icons (spinner, alert) alongside the text. + */ + subtitle?: ReactNode; buttonClassName?: string; containerClassName?: string; onDelete?: () => void; @@ -49,9 +60,13 @@ const FileContainer = ({
{visibleName}
-
- {fileType.title} -
+ {subtitle != null ? ( + subtitle + ) : ( +
+ {fileType.title} +
+ )} diff --git a/client/src/components/Chat/Messages/Content/Parts/Attachment.tsx b/client/src/components/Chat/Messages/Content/Parts/Attachment.tsx index 37c165d0d1..3d36cd9d96 100644 --- a/client/src/components/Chat/Messages/Content/Parts/Attachment.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/Attachment.tsx @@ -25,43 +25,42 @@ import { cn } from '~/utils'; const COLLAPSED_MAX_HEIGHT = 320; /** - * Small inline indicator rendered next to a code-execution file chip - * while its inline preview is in flight or has failed. Three states: + * Inline subtitle for a code-execution file chip with a pending or + * failed preview render. Renders into the chip's existing subtitle + * slot (replacing the file-type label), so the chip footprint stays + * identical to its `'ready'` form — just the second row swaps from + * "PowerPoint Presentation" (or whatever the type label is) to a + * spinner + "Preparing preview…" or an alert + "Preview unavailable". * - * - `'pending'`: tiny spinner + "preparing preview…". Driven by the - * polling `useAttachmentPreviewSync` hook (gated on `isSubmitting`) - * and by the SSE `attachment` update event when the response stream - * is still open. - * - `'failed'`: alert icon + "preview unavailable" with the - * backend's machine-readable `previewError` as a tooltip - * (`'timeout'`, `'parser-error'`, `'orphaned'`, …). The download - * chip itself remains fully functional. - * - `'ready'` (or undefined): nothing rendered — the chip looks - * exactly as it did before this PR. + * Driven by the polling `useAttachmentPreviewSync` hook for the + * post-stream-close gap and by the `attachment` SSE update event + * while the response stream is still open. For `'ready'` (or + * undefined / legacy) status the chip falls back to its default + * subtitle and this component isn't rendered. */ -const PreviewStatusIndicator = memo( +const PreviewStatusSubtitle = memo( ({ status, previewError }: { status: 'pending' | 'failed'; previewError?: string }) => { const localize = useLocalize(); if (status === 'pending') { return ( -
-
); }); diff --git a/client/src/hooks/Files/__tests__/useAttachmentPreviewSync.spec.tsx b/client/src/hooks/Files/__tests__/useAttachmentPreviewSync.spec.tsx index 4e5b519741..1e5536574e 100644 --- a/client/src/hooks/Files/__tests__/useAttachmentPreviewSync.spec.tsx +++ b/client/src/hooks/Files/__tests__/useAttachmentPreviewSync.spec.tsx @@ -118,7 +118,7 @@ function setup({ } describe('useAttachmentPreviewSync', () => { - it('enables polling only when status=pending AND a conversation is submitting', () => { + it('enables polling whenever status=pending (no longer gated on isSubmitting)', () => { const ctx = setup({ attachment: makeAttachment({ status: 'pending' }), isSubmitting: true, @@ -142,14 +142,20 @@ describe('useAttachmentPreviewSync', () => { expect(ctx.enabled).toBe(false); }); - it('does NOT enable polling when no conversation is submitting (LLM finished)', () => { + it('STILL enables polling for pending records even after the LLM has finished generating', () => { + /* Regression for the stuck-spinner bug: the deferred render can + * complete a few seconds AFTER the SSE stream closes. With the + * earlier `isAnySubmitting` gate, polling stopped the moment the + * model finished and the resolved-but-not-yet-emitted state would + * never reach the UI. Polling now runs on `status === 'pending'` + * alone; `useFilePreview`'s `refetchInterval` auto-stops on the + * first terminal response, and the server-side render ceiling + + * lazy sweep cap the worst case. */ const ctx = setup({ attachment: makeAttachment({ status: 'pending' }), isSubmitting: false, }); - /* User's explicit gate: once the LLM is done, polling stops. The - * frontend can refetch on demand from the next interaction. */ - expect(ctx.enabled).toBe(false); + expect(ctx.enabled).toBe(true); }); it('does NOT enable polling when the attachment has no file_id', () => { diff --git a/client/src/hooks/Files/useAttachmentPreviewSync.ts b/client/src/hooks/Files/useAttachmentPreviewSync.ts index ec9f11e79b..0a1cdd771f 100644 --- a/client/src/hooks/Files/useAttachmentPreviewSync.ts +++ b/client/src/hooks/Files/useAttachmentPreviewSync.ts @@ -1,5 +1,5 @@ import { useEffect } from 'react'; -import { useRecoilValue, useSetRecoilState } from 'recoil'; +import { useSetRecoilState } from 'recoil'; import type { TAttachment, TFile, TFilePreview } from 'librechat-data-provider'; import { useFilePreview } from '~/data-provider'; import store from '~/store'; @@ -39,24 +39,34 @@ interface UseAttachmentPreviewSyncResult { * * Polling is gated on: * - `attachment.file_id` present (no id → nothing to poll for) - * - Effective status is `'pending'` (terminal states need no work) - * - Some conversation in the app is submitting (per the user's - * explicit gate: "only query when the preview is still live and - * `isSubmitting` is still true"). When the LLM finishes, polling - * stops; the user can refresh on demand from the next interaction. + * - Effective status is `'pending'` (terminal states need no work). + * `useFilePreview`'s `refetchInterval` returns `false` the moment + * the server reports `ready`/`failed`, so polling auto-terminates + * within one tick of resolution. Bounded ceiling: the server-side + * render timeout is 60s, so a stuck pending record gets ~24 polls + * max before the lazy sweep in the preview endpoint forces it to + * `'failed'`. + * + * NOTE: an earlier version of this hook also gated on `isAnySubmitting` + * (the LLM still generating). That gate was removed because the + * deferred render can complete *after* the SSE stream closes — when it + * does, the SSE update is silently dropped, and polling is the only + * recovery path. With the gate in place, the chip would stay stuck on + * "Preparing preview…" forever (until manual refresh) for any render + * that landed even seconds after submission ended. The polling itself + * doesn't block UX; the user can keep messaging regardless. */ export default function useAttachmentPreviewSync( attachment: TAttachment | undefined, ): UseAttachmentPreviewSyncResult { const setAttachmentsMap = useSetRecoilState(store.messageAttachmentsMap); - const isAnySubmitting = useRecoilValue(store.anySubmittingSelector); const file = (attachment ?? undefined) as Partial | undefined; const fileId = file?.file_id; const baseStatus: 'pending' | 'ready' | 'failed' = file?.status ?? 'ready'; const messageId = (attachment as Partial | undefined)?.messageId; - const enabled = !!fileId && baseStatus === 'pending' && isAnySubmitting; + const enabled = !!fileId && baseStatus === 'pending'; const previewQuery = useFilePreview(fileId, { enabled });