Fix document cover preview reliability

data/repo-awareness-snapshot.json

@@ -1,8 +1,8 @@
 {
  "version": "repo-awareness-snapshot-v1",
  "captured_revision": {
- "sha": "38fb26025fe7dfb4d007182c319769dd42ed861d",
- "committed_at": "2026-08-26T08:43:32+00:00"
+ "sha": "ea58e0fcc734a79db3af625f800953333203f5bd",
+ "committed_at": "2026-08-26T22:38:53+08:00"
  },
  "routes": {
  "modes": [
@@ -3819,6 +3819,22 @@
  "outcome": "Whole-branch review: approved with findings, 0 P0, 0 P1, 7 P2 — all seven fixed and mutation-tested. Four earlier automated findings: three fixed, one rejected with reasons.",
  "checks": "npm run test (3 pre-existing failures, identical on clean origin/main); npm run typecheck; npm run lint (eslint cache cleared); prettier --check on all changed files; chromium-mockups ward-management + ward-coordinator + ward-discharges; screenshots at 390/820/1440 on five screens, looked at"
  },
+ {
+ "date": "2026-08-26",
+ "ref": "codex/chat-image-preview-reliability-image-preview-reliability",
+ "head": "09ff8ca36489411f26408d6b557888459151bb4a",
+ "scope": "authenticated document cover selection, retry lifecycle, optional preview failure, and source evidence preservation",
+ "outcome": "All substantiated P2 findings fixed; no remaining P0-P2 findings",
+ "checks": "lint pass; typecheck pass; focused preview 79/79; production build pass; full suite 10489 pass with six unrelated Windows Claude-cloud failures after the one preview failure was fixed; offline fixture contracts pass; snapshot and medication checks pass"
+ },
+ {
+ "date": "2026-08-26",
+ "ref": "codex/chat-image-preview-reliability-image-preview-reliability",
+ "head": "57ab01cbe28977ed3e0db830c45a00749977e351",
+ "scope": "PR #2391",
+ "outcome": "fixes-applied",
+ "checks": "PR policy body canonicalized; cover in-flight keyed by credential; vitest use-document-cover 9/9; snapshot regenerated; threads resolved; CI watching"
+ },
  {
  "date": "2026-08-26",
  "ref": "codex/medication-risk-highlights",
@@ -24789,8 +24805,8 @@
  }
  ],
  "counts": {
- "records": 2624,
- "refs": 1603
+ "records": 2626,
+ "refs": 1604
  }
  }
 }

docs/branch-review-records/04098ccb23c407505308b9e046181f8b978d85a4122db061e5b56a5f3cc0d988.record.md

@@ -0,0 +1 @@
+| 2026-08-26 | codex/chat-image-preview-reliability-image-preview-reliability | 09ff8ca36489411f26408d6b557888459151bb4a | authenticated document cover selection, retry lifecycle, optional preview failure, and source evidence preservation | All substantiated P2 findings fixed; no remaining P0-P2 findings | lint pass; typecheck pass; focused preview 79/79; production build pass; full suite 10489 pass with six unrelated Windows Claude-cloud failures after the one preview failure was fixed; offline fixture contracts pass; snapshot and medication checks pass |

docs/branch-review-records/a3ef569d0937ef0d619ee26c491cf0bac4c4d808d1597a7d1969c1cd5fe49762.record.md

@@ -0,0 +1 @@
+| 2026-08-26 | codex/chat-image-preview-reliability-image-preview-reliability | 57ab01cbe28977ed3e0db830c45a00749977e351 | PR #2391 | fixes-applied | PR policy body canonicalized; cover in-flight keyed by credential; vitest use-document-cover 9/9; snapshot regenerated; threads resolved; CI watching |

src/app/api/documents/[id]/cover/route.ts

@@ -5,7 +5,6 @@ import { rateLimitJsonResponse } from "@/lib/api-rate-limit";
 import { demoImages } from "@/lib/demo-data";
 import { isDemoMode } from "@/lib/env";
 import { jsonError, PublicApiError, publicErrorResponse } from "@/lib/http";
-import { fetchDocumentCoverImageIds } from "@/lib/document-enrichment";
 import { committedIndexGeneration, isCommittedGenerationMetadata } from "@/lib/reindex-pipeline";
 import { parseRouteParams } from "@/lib/validation/params";
 import { enforceDocumentReadRateLimit, withOwnerReadScope } from "@/lib/public-api-access";
@@ -16,6 +15,7 @@ export const runtime = "nodejs";
 
 const coverRouteParamsSchema = z.object({ id: z.string().uuid() });
 const coverImageIdSchema = z.string().uuid();
+const maxLegacyCoverCandidates = 64;
 
 /**
  * The document's first-page cover thumbnail id, for surfaces that show what a
@@ -105,12 +105,32 @@ export async function GET(request: Request, { params }: { params: Promise<{ id:
  return NextResponse.json({ coverImageId: cover.id });
  }
 
- // Documents indexed before the pointer existed carry no such key. Fall back
- // to the scan rather than drop their thumbnail: it is the same resolution
- // the document search cards already use, and the signed-url route still
- // re-checks ownership and committed generation before it hands anything out.
- const covers = await fetchDocumentCoverImageIds(supabase, [id], request.signal);
- return NextResponse.json({ coverImageId: covers.get(id) ?? null });
+ // Documents indexed before the selected pointer existed carry no such key.
+ // Resolve their cover locally so the route never hands a staged/stale row
+ // to the signed-url route. The database predicates enforce document + kind;
+ // the defensive checks below preserve that invariant even if query data is
+ // malformed. A bounded ambiguous legacy result fails closed.
+ const committedGeneration = committedIndexGeneration(metadata);
+ const { data: candidates, error: candidatesError } = await supabase
+ .from("document_images")
+ .select("id,document_id,source_kind,metadata")
+ .eq("document_id", id)
+ .eq("source_kind", "cover_page")
+ .abortSignal(request.signal)
+ .limit(maxLegacyCoverCandidates + 1);
+ if (candidatesError) throw new Error(candidatesError.message);
+
+ const boundedCandidates = candidates ?? [];
+ if (boundedCandidates.length > maxLegacyCoverCandidates) {
+ return NextResponse.json({ coverImageId: null });
+ }
+ const committedCandidates = boundedCandidates.filter((candidate) => {
+ if (candidate.document_id !== id || candidate.source_kind !== "cover_page") return false;
+ if (!coverImageIdSchema.safeParse(candidate.id).success) return false;
+ const candidateGeneration = committedIndexGeneration(candidate.metadata);
+ return committedGeneration ? candidateGeneration === committedGeneration : candidateGeneration === null;
+ });
+ return NextResponse.json({ coverImageId: committedCandidates.length === 1 ? committedCandidates[0].id : null });
  } catch (error) {
  if (error instanceof AuthenticationError) return unauthorizedResponse();
  if (error instanceof PublicApiError) return jsonError(error);

src/components/clinical-dashboard/answer-source-drawer.tsx

@@ -142,7 +142,7 @@ export function AnswerSourceDrawer({
  const stale = source ? sourceRowIsStale(source) : false;
  // Hooks cannot be conditional, so this asks for the open source's cover on
  // every render and resolves to null while the drawer is closed.
- const coverImageId = useDocumentCoverImageId(source?.documentId);
+ const { coverImageId, markCoverUnavailable } = useDocumentCoverImageId(source?.documentId);
  const numbered = sources.length <= NUMBERED_PAGER_LIMIT;
 
  return (
@@ -280,6 +280,8 @@ export function AnswerSourceDrawer({
  className="rounded-lg border border-t-[3px] border-[color:var(--border-lux)] border-t-[color:var(--clinical-accent)] bg-[color:var(--surface)] shadow-[var(--shadow-inset)]"
  rootMargin="0px"
  priority
+ failurePresentation="hidden"
+ onSettledFailure={() => markCoverUnavailable(coverImageId)}
  />
  <figcaption className={cn("mt-1 text-3xs leading-4", textMuted)}>
  Front page

src/components/clinical-dashboard/signed-image.tsx

@@ -89,6 +89,8 @@ export const SignedImage = memo(function SignedImage({
  aspectRatio,
  priority = false,
  expandLabel,
+ failurePresentation = "message",
+ onSettledFailure,
 }: {
  /** Signed-URL API route, e.g. `/api/images/{id}/signed-url`. */
  endpoint: string;
@@ -128,6 +130,10 @@ export const SignedImage = memo(function SignedImage({
  * unchanged.
  */
  expandLabel?: string;
+ /** Settled failures remain visible by default; optional decoration may hide them. */
+ failurePresentation?: "message" | "hidden";
+ /** Called once when a failure remains after the bounded automatic retry cycle. */
+ onSettledFailure?: (failure: SignedImageFailure) => void;
 }) {
  const [shouldLoad, setShouldLoad] = useState(() => priority || Boolean(getCachedSignedUrl(endpoint)));
  const [loaded, setLoaded] = useState(false);
@@ -136,15 +142,18 @@ export const SignedImage = memo(function SignedImage({
  const triggerRef = useRef<HTMLButtonElement>(null);
  const [retryDisabled, setRetryDisabled] = useState(false);
  const [automaticRetryCount, setAutomaticRetryCount] = useState(0);
+ const notifiedSettledFailureRef = useRef<SignedImageFailure | null>(null);
  const [seenEndpoint, setSeenEndpoint] = useState(endpoint);
  if (endpoint !== seenEndpoint) {
  setSeenEndpoint(endpoint);
  setAutomaticRetryCount(0);
  setLoaded(false);
+ notifiedSettledFailureRef.current = null;
  }
  const { url, failed, failure, retry, markFailed } = useSignedImageUrl(endpoint, shouldLoad);
  const nextAutomaticRetryDelay = automaticRetryDelay(failure, automaticRetryCount);
  const automaticRetryPending = nextAutomaticRetryDelay !== null;
+ const settledFailure = failed && !automaticRetryPending ? failure : null;
 
  // Defer the request until the frame is near the viewport. A cached URL seeds
  // `shouldLoad` synchronously, so already-fetched images skip the observer.
@@ -185,10 +194,17 @@ export const SignedImage = memo(function SignedImage({
  });
  }, [endpoint, nextAutomaticRetryDelay, retry]);
 
+ useEffect(() => {
+ if (!settledFailure || notifiedSettledFailureRef.current === settledFailure) return;
+ notifiedSettledFailureRef.current = settledFailure;
+ onSettledFailure?.(settledFailure);
+ }, [onSettledFailure, settledFailure]);
+
  function retryImage() {
  if (retryDisabled) return;
  setRetryDisabled(true);
  setAutomaticRetryCount(0);
+ notifiedSettledFailureRef.current = null;
  setLoaded(false);
  setShouldLoad(true);
  retry();
@@ -200,7 +216,8 @@ export const SignedImage = memo(function SignedImage({
  markFailed();
  }
 
- if (failed && !automaticRetryPending) {
+ if (settledFailure) {
+ if (failurePresentation === "hidden") return null;
  return (
  <div
  ref={frameRef}
@@ -270,6 +287,7 @@ export const SignedImage = memo(function SignedImage({
  onLoad={() => {
  setLoaded(true);
  setAutomaticRetryCount(0);
+ notifiedSettledFailureRef.current = null;
  }}
  onError={handleImageError}
  className={cn(

src/components/clinical-dashboard/use-document-cover.ts

@@ -1,6 +1,8 @@
 "use client";
 
-import { useEffect, useState } from "react";
+import { useCallback, useEffect, useState } from "react";
+
+import { useAuthSession } from "@/lib/supabase/client";
 
 /**
  * The first-page cover thumbnail id for a document, fetched on demand.
@@ -10,10 +12,10 @@ import { useEffect, useState } from "react";
  * editing retrieval hydration — a protected RAG surface, and far more blast
  * radius than a thumbnail earns. So the drawer asks for it when a source opens.
  *
- * Cached per document for the page's lifetime, including the authoritative
- * misses. A document with no cover is the common case for a text-only upload,
- * and re-asking on every drawer open would spend a document-read rate-limit
- * token each time to learn the same `null`.
+ * Cached per authenticated identity and document for the page's lifetime,
+ * including authoritative misses. A document with no cover is the common case
+ * for a text-only upload, and re-asking on every drawer open would spend a
+ * document-read rate-limit token each time to learn the same `null`.
  *
  * An authoritative miss is not the same as a failed lookup, and the first cut
  * of this cached both as `null`. A 429, a 5xx or an offline blip then pinned
@@ -25,26 +27,49 @@ import { useEffect, useState } from "react";
 const coverImageIds = new Map<string, string | null>();
 const inFlight = new Map<string, Promise<string | null | undefined>>();
 
+function coverCacheKey(documentId: string, authIdentity: string | null) {
+ return JSON.stringify([authIdentity, documentId]);
+}
+
+function coverInFlightKey(
+ documentId: string,
+ authIdentity: string | null,
+ authorizationHeader: Record<string, string>,
+) {
+ // Resolved answers stay keyed by user + document so a healthy cover survives
+ // token refresh. In-flight work must also include the credential: the same
+ // user can rotate a header while a request is pending, and reusing that
+ // promise would let a 401 settle as a miss after the new header is in use.
+ return JSON.stringify([authIdentity, documentId, authorizationHeader.Authorization ?? ""]);
+}
+
 /** `string`/`null` are answers and get cached; `undefined` is a transient failure. */
-async function loadCoverImageId(documentId: string): Promise<string | null | undefined> {
- const cached = coverImageIds.get(documentId);
+async function loadCoverImageId(
+ key: string,
+ documentId: string,
+ authIdentity: string | null,
+ authorizationHeader: Record<string, string>,
+): Promise<string | null | undefined> {
+ const cached = coverImageIds.get(key);
  if (cached !== undefined) return cached;
- const pending = inFlight.get(documentId);
+ const requestKey = coverInFlightKey(documentId, authIdentity, authorizationHeader);
+ const pending = inFlight.get(requestKey);
  if (pending) return pending;
 
  const request = (async () => {
  try {
- const response = await fetch(`/api/documents/${encodeURIComponent(documentId)}/cover`);
+ const response = await fetch(`/api/documents/${encodeURIComponent(documentId)}/cover`, {
+ headers: authorizationHeader,
+ });
  // 404 is an answer: the document is gone or not ours to read. Anything
  // else non-ok (429, 5xx) is the server declining for now, not saying no.
  if (response.status === 404) return null;
  if (!response.ok) return undefined;
  const payload: unknown = await response.json();
- const value =
- payload && typeof payload === "object" && "coverImageId" in payload
- ? (payload as { coverImageId: unknown }).coverImageId
- : null;
- return typeof value === "string" && value.length > 0 ? value : null;
+ if (!payload || typeof payload !== "object" || !("coverImageId" in payload)) return undefined;
+ const value = (payload as { coverImageId: unknown }).coverImageId;
+ if (value === null) return null;
+ return typeof value === "string" && value.length > 0 ? value : undefined;
  } catch {
  // Offline, aborted, or unparseable. A cover is decoration for a citation,
  // never the citation itself, so this renders no thumbnail and changes
@@ -53,44 +78,74 @@ async function loadCoverImageId(documentId: string): Promise<string | null | und
  }
  })();
 
- inFlight.set(documentId, request);
+ inFlight.set(requestKey, request);
  const resolved = await request;
- inFlight.delete(documentId);
- if (resolved !== undefined) coverImageIds.set(documentId, resolved);
+ inFlight.delete(requestKey);
+ if (resolved !== undefined) coverImageIds.set(key, resolved);
  return resolved;
 }
 
-export function useDocumentCoverImageId(documentId: string | null | undefined): string | null {
+export function useDocumentCoverImageId(documentId: string | null | undefined): {
+ coverImageId: string | null;
+ markCoverUnavailable: (imageId: string) => void;
+} {
+ const { authorizationHeader, session } = useAuthSession();
  const id = documentId ?? null;
+ const authIdentity = session?.user?.id ?? null;
+ const key = id ? coverCacheKey(id, authIdentity) : null;
  /**
  * Reset happens during render, not in an effect. Clearing the previous
- * document's answer from inside an effect renders one frame with the wrong
- * cover attached to the new source — a picture of the last document beside
- * this document's passage — and `react-hooks/set-state-in-effect` rejects the
- * synchronous set that would cause it. The effect below only ever sets state
- * from the resolved promise.
+ * identity/document answer from inside an effect renders one frame with the
+ * wrong cover attached to the new source — a picture from another document
+ * or account beside this document's passage. The effect below only ever sets
+ * state from the resolved promise.
  */
- const [renderedId, setRenderedId] = useState(id);
+ const [renderedKey, setRenderedKey] = useState(key);
  const [fetched, setFetched] = useState<string | null>(null);
- if (renderedId !== id) {
- setRenderedId(id);
+ const [, notifyCacheChanged] = useState(0);
+ if (renderedKey !== key) {
+ setRenderedKey(key);
  setFetched(null);
  }
 
  useEffect(() => {
- if (!id || coverImageIds.get(id) !== undefined) return;
+ if (!id || !key || coverImageIds.get(key) !== undefined) return;
  let active = true;
- void loadCoverImageId(id).then((resolved) => {
+ void loadCoverImageId(key, id, authIdentity, authorizationHeader).then((resolved) => {
  if (active) setFetched(resolved ?? null);
  });
  return () => {
  active = false;
  };
- }, [id]);
+ }, [authIdentity, authorizationHeader, id, key]);
+
+ const markCoverUnavailable = useCallback(
+ (imageId: string) => {
+ // A settled image failure can report after the drawer has paged to a new
+ // document/account. The captured identity+document key and exact image
+ // guard ensure it can only invalidate the result that actually failed;
+ // a newer result for that key and every other account remain untouched.
+ if (!key || coverImageIds.get(key) !== imageId) return;
+ // This is not an authoritative "no cover" answer. The signed-url or
+ // object download may have failed transiently, or a reindex may have
+ // selected a replacement since this id was resolved. Hide the optional
+ // thumbnail for this mounted drawer, but discard the stale lookup so the
+ // next open can resolve the current cover instead of pinning a negative
+ // result for the rest of the page lifetime.
+ coverImageIds.delete(key);
+ setFetched(null);
+ // `coverImageIds` is external mutable state. `fetched` can already be
+ // null, so setting it to null is a React no-op; explicitly notify this
+ // hook so the deleted cached id is observed without refetching until a
+ // later mount/open runs the effect again.
+ notifyCacheChanged((revision) => revision + 1);
+ },
+ [key],
+ );
 
- if (!id) return null;
- const cached = coverImageIds.get(id);
- return cached !== undefined ? cached : fetched;
+ if (!id || !key) return { coverImageId: null, markCoverUnavailable };
+ const cached = coverImageIds.get(key);
+ return { coverImageId: cached !== undefined ? cached : fetched, markCoverUnavailable };
 }
 
 /** Test-only reset for the process-local cover cache. */

tests/answer-source-marks.dom.test.tsx

@@ -8,6 +8,13 @@ vi.mock("@/components/clinical-dashboard/signed-image", () => ({
  SignedImage: ({ caption }: { caption?: string }) => <p>{caption}</p>,
 }));
 
+vi.mock("@/lib/supabase/client", () => ({
+ useAuthSession: () => ({
+ authorizationHeader: { Authorization: "Bearer cover-test" },
+ session: { user: { id: "cover-test-user" } },
+ }),
+}));
+
 import { AnswerSourceDrawer } from "@/components/clinical-dashboard/answer-source-drawer";
 import { NaturalLanguageAnswer, primaryAnswerDisplayText } from "@/components/clinical-dashboard/answer-content";
 import { type AnswerSourceRow } from "@/components/clinical-dashboard/answer-source-rows";