From d16252aa18cac216d89fcd76c1b040a3fc8cd2af Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Wed, 26 Aug 2026 12:56:26 +0800 Subject: [PATCH 1/7] Fix document cover preview reliability --- src/app/api/documents/[id]/cover/route.ts | 34 +++- .../answer-source-drawer.tsx | 4 +- .../clinical-dashboard/signed-image.tsx | 20 +- .../clinical-dashboard/use-document-cover.ts | 99 ++++++--- tests/answer-source-marks.dom.test.tsx | 7 + tests/answer-source-rail.dom.test.tsx | 59 +++++- tests/document-cover-route.test.ts | 163 +++++++++++++++ tests/signed-image.dom.test.tsx | 96 +++++++++ tests/use-document-cover.dom.test.tsx | 192 ++++++++++++++++++ 9 files changed, 632 insertions(+), 42 deletions(-) create mode 100644 tests/use-document-cover.dom.test.tsx diff --git a/src/app/api/documents/[id]/cover/route.ts b/src/app/api/documents/[id]/cover/route.ts index 3d2f54c265..f722e16d14 100644 --- a/src/app/api/documents/[id]/cover/route.ts +++ b/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); diff --git a/src/components/clinical-dashboard/answer-source-drawer.tsx b/src/components/clinical-dashboard/answer-source-drawer.tsx index f0e25daa2e..5835b07910 100644 --- a/src/components/clinical-dashboard/answer-source-drawer.tsx +++ b/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)} />
Front page diff --git a/src/components/clinical-dashboard/signed-image.tsx b/src/components/clinical-dashboard/signed-image.tsx index 51c9c8bd5b..724fefb13a 100644 --- a/src/components/clinical-dashboard/signed-image.tsx +++ b/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(null); const [retryDisabled, setRetryDisabled] = useState(false); const [automaticRetryCount, setAutomaticRetryCount] = useState(0); + const notifiedSettledFailureRef = useRef(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 (
{ setLoaded(true); setAutomaticRetryCount(0); + notifiedSettledFailureRef.current = null; }} onError={handleImageError} className={cn( diff --git a/src/components/clinical-dashboard/use-document-cover.ts b/src/components/clinical-dashboard/use-document-cover.ts index 5c5ea30342..ffa0dc66b8 100644 --- a/src/components/clinical-dashboard/use-document-cover.ts +++ b/src/components/clinical-dashboard/use-document-cover.ts @@ -1,6 +1,8 @@ "use client"; -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, 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,35 @@ import { useEffect, useState } from "react"; const coverImageIds = new Map(); const inFlight = new Map>(); +function coverCacheKey(documentId: string, authIdentity: string | null) { + return JSON.stringify([authIdentity, documentId]); +} + /** `string`/`null` are answers and get cached; `undefined` is a transient failure. */ -async function loadCoverImageId(documentId: string): Promise { - const cached = coverImageIds.get(documentId); +async function loadCoverImageId( + key: string, + documentId: string, + authorizationHeader: Record, +): Promise { + const cached = coverImageIds.get(key); if (cached !== undefined) return cached; - const pending = inFlight.get(documentId); + const pending = inFlight.get(key); 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 +64,68 @@ async function loadCoverImageId(documentId: string): Promise void; +} { + const { authorizationHeader, session } = useAuthSession(); const id = documentId ?? null; + const authIdentity = session?.user?.id ?? null; + const key = id ? coverCacheKey(id, authIdentity) : null; + const currentKeyRef = useRef(key); + currentKeyRef.current = key; /** * 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(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, authorizationHeader).then((resolved) => { if (active) setFetched(resolved ?? null); }); return () => { active = false; }; - }, [id]); + }, [authorizationHeader, id, key]); + + const markCoverUnavailable = useCallback( + (imageId: string) => { + // A settled image failure can report after the drawer has paged to a new + // document/account. It may only invalidate the entry that is still + // current, and only if that entry still names the failing image. + if (!key || currentKeyRef.current !== key || coverImageIds.get(key) !== imageId) return; + coverImageIds.set(key, null); + setFetched(null); + // `coverImageIds` is external mutable state. After an identity round trip + // `fetched` can already be null, so setting it to null is a React no-op; + // explicitly notify this hook so the new cached null is observed. + 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. */ diff --git a/tests/answer-source-marks.dom.test.tsx b/tests/answer-source-marks.dom.test.tsx index bc3039a191..d35c7e5d15 100644 --- a/tests/answer-source-marks.dom.test.tsx +++ b/tests/answer-source-marks.dom.test.tsx @@ -8,6 +8,13 @@ vi.mock("@/components/clinical-dashboard/signed-image", () => ({ SignedImage: ({ caption }: { caption?: string }) =>

{caption}

, })); +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"; diff --git a/tests/answer-source-rail.dom.test.tsx b/tests/answer-source-rail.dom.test.tsx index 1cf8a5c416..0bf08dafa7 100644 --- a/tests/answer-source-rail.dom.test.tsx +++ b/tests/answer-source-rail.dom.test.tsx @@ -4,8 +4,43 @@ import { render, screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +vi.mock("@/lib/supabase/client", () => ({ + useAuthSession: () => ({ + authorizationHeader: { Authorization: "Bearer cover-test" }, + session: { user: { id: "cover-test-user" } }, + }), +})); + vi.mock("@/components/clinical-dashboard/signed-image", () => ({ - SignedImage: ({ caption, alt }: { caption?: string; alt?: string }) =>

{caption ?? alt}

, + SignedImage: ({ + caption, + alt, + failurePresentation, + onSettledFailure, + }: { + caption?: string; + alt?: string; + failurePresentation?: "message" | "hidden"; + onSettledFailure?: (failure: { + source: "response"; + status: number; + retryable: boolean; + retryAfterMs: null; + }) => void; + }) => ( + <> +

{caption ?? alt}

+ {failurePresentation === "hidden" && onSettledFailure ? ( + + ) : null} + + ), })); import { AnswerSupportSummaryCard } from "@/components/clinical-dashboard/evidence-panels"; @@ -607,4 +642,26 @@ describe("answer source drawer cover", () => { // passage down with it. expect(screen.getByTestId("answer-source-drawer-passage")).toBeInTheDocument(); }); + + it("removes only a failed optional cover while preserving source evidence and actions", async () => { + fetchMock.mockResolvedValue({ ok: true, json: async () => ({ coverImageId: "cover-1" }) }); + const user = userEvent.setup(); + render( + , + ); + await user.click(screen.getAllByTestId("answer-source-rail-row")[0]); + + const cover = await screen.findByTestId("answer-source-drawer-cover"); + await user.click(within(cover).getByTestId("settle-hidden-signed-image")); + + expect(screen.queryByTestId("answer-source-drawer-cover")).not.toBeInTheDocument(); + expect(screen.getByTestId("answer-source-drawer-passage")).toBeInTheDocument(); + expect(screen.getByTestId("answer-source-drawer-pager")).toBeInTheDocument(); + expect(screen.getByTestId("answer-source-drawer-menu-trigger")).toBeInTheDocument(); + expect(screen.getByRole("link", { name: "View original PDF" })).toBeInTheDocument(); + expect(screen.getByText("Ordinary evidence")).toBeInTheDocument(); + }); }); diff --git a/tests/document-cover-route.test.ts b/tests/document-cover-route.test.ts index e82108edb2..46e856fcec 100644 --- a/tests/document-cover-route.test.ts +++ b/tests/document-cover-route.test.ts @@ -3,12 +3,14 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const documentId = "11111111-1111-4111-8111-111111111111"; const selectedCoverId = "22222222-2222-4222-8222-222222222222"; const stagedCoverId = "33333333-3333-4333-8333-333333333333"; +const committedCoverId = "66666666-6666-4666-8666-666666666666"; const committedGeneration = "44444444-4444-4444-8444-444444444444"; const stagedGeneration = "55555555-5555-4555-8555-555555555555"; type QueryCall = { table: string; selected?: string; + limit?: number; filters: Array<{ column: string; value: unknown }>; }; type QueryResult = { data: unknown; error: { message: string } | null }; @@ -35,6 +37,18 @@ class QueryBuilder { return this; } + limit(value: number) { + this.call.limit = value; + return this; + } + + then( + onfulfilled?: ((value: QueryResult) => TResult1 | PromiseLike) | null, + onrejected?: ((reason: unknown) => TResult2 | PromiseLike) | null, + ): Promise { + return Promise.resolve(this.resolve(this.call)).then(onfulfilled, onrejected); + } + maybeSingle() { return Promise.resolve(this.resolve(this.call)); } @@ -103,6 +117,155 @@ beforeEach(() => { }); describe("GET /api/documents/[id]/cover", () => { + it("selects the committed cover from legacy candidates instead of handing off a stale row", async () => { + const calls = setRouteData((call) => { + if (call.table === "documents") { + return { + data: { id: documentId, metadata: { index_generation_id: committedGeneration } }, + error: null, + }; + } + if (call.table === "document_images") { + return { + data: [ + { + id: stagedCoverId, + document_id: documentId, + source_kind: "cover_page", + metadata: { index_generation_id: stagedGeneration }, + }, + { + id: committedCoverId, + document_id: documentId, + source_kind: "cover_page", + metadata: { index_generation_id: committedGeneration }, + }, + { + id: selectedCoverId, + document_id: "99999999-9999-4999-8999-999999999999", + source_kind: "cover_page", + metadata: { index_generation_id: committedGeneration }, + }, + { + id: "77777777-7777-4777-8777-777777777777", + document_id: documentId, + source_kind: "figure", + metadata: { index_generation_id: committedGeneration }, + }, + ], + error: null, + }; + } + return { data: null, error: null }; + }); + + const response = await GET(request(), routeParams()); + + await expect(response.json()).resolves.toEqual({ coverImageId: committedCoverId }); + expect(calls.find((call) => call.table === "document_images")).toMatchObject({ + selected: "id,document_id,source_kind,metadata", + limit: 65, + filters: expect.arrayContaining([ + { column: "document_id", value: documentId }, + { column: "source_kind", value: "cover_page" }, + ]), + }); + expect(mocks.fetchDocumentCoverImageIds).not.toHaveBeenCalled(); + }); + + it("returns null for a legacy document when no committed cover candidate exists", async () => { + setRouteData((call) => { + if (call.table === "documents") { + return { + data: { id: documentId, metadata: { index_generation_id: committedGeneration } }, + error: null, + }; + } + if (call.table === "document_images") { + return { + data: [ + { + id: stagedCoverId, + document_id: documentId, + source_kind: "cover_page", + metadata: { index_generation_id: stagedGeneration }, + }, + ], + error: null, + }; + } + return { data: null, error: null }; + }); + + const response = await GET(request(), routeParams()); + + await expect(response.json()).resolves.toEqual({ coverImageId: null }); + }); + + it("fails closed when the bounded legacy query returns its 65-row overflow sentinel", async () => { + setRouteData((call) => { + if (call.table === "documents") { + return { + data: { id: documentId, metadata: { index_generation_id: committedGeneration } }, + error: null, + }; + } + if (call.table === "document_images") { + return { + data: Array.from({ length: 65 }, (_, index) => ({ + id: `88888888-8888-4888-8888-${index.toString(16).padStart(12, "0")}`, + document_id: documentId, + source_kind: "cover_page", + metadata: { index_generation_id: committedGeneration }, + })), + error: null, + }; + } + return { data: null, error: null }; + }); + + const response = await GET(request(), routeParams()); + + await expect(response.json()).resolves.toEqual({ coverImageId: null }); + expect(mocks.fetchDocumentCoverImageIds).not.toHaveBeenCalled(); + }); + + it("fails closed when two legacy candidates belong to the committed generation", async () => { + setRouteData((call) => { + if (call.table === "documents") { + return { + data: { id: documentId, metadata: { index_generation_id: committedGeneration } }, + error: null, + }; + } + if (call.table === "document_images") { + return { + data: [ + { + id: committedCoverId, + document_id: documentId, + source_kind: "cover_page", + metadata: { index_generation_id: committedGeneration }, + }, + { + id: selectedCoverId, + document_id: documentId, + source_kind: "cover_page", + metadata: { index_generation_id: committedGeneration }, + }, + ], + error: null, + }; + } + return { data: null, error: null }; + }); + + const response = await GET(request(), routeParams()); + + await expect(response.json()).resolves.toEqual({ coverImageId: null }); + expect(mocks.fetchDocumentCoverImageIds).not.toHaveBeenCalled(); + }); + it("returns only the selected cover after validating its document, kind, and committed generation", async () => { const calls = setRouteData((call) => { if (call.table === "documents") { diff --git a/tests/signed-image.dom.test.tsx b/tests/signed-image.dom.test.tsx index 0ddf7b35a8..78281c0fbb 100644 --- a/tests/signed-image.dom.test.tsx +++ b/tests/signed-image.dom.test.tsx @@ -81,6 +81,102 @@ describe("SignedImage failure/retry (jsdom)", () => { expect(fetchMock).toHaveBeenCalledTimes(1); }); + it("keeps the default settled fallback visible and reports it once", async () => { + const onSettledFailure = vi.fn(); + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue({ ok: false, status: 404, json: async () => ({ error: "not found" }) }), + ); + + const { rerender } = render( + , + ); + + expect(await screen.findByText("Image preview failed.")).toBeInTheDocument(); + await waitFor(() => expect(onSettledFailure).toHaveBeenCalledTimes(1)); + expect(onSettledFailure).toHaveBeenCalledWith( + expect.objectContaining({ source: "response", status: 404, retryable: false }), + ); + + const replacementCallback = vi.fn(); + rerender( + , + ); + expect(onSettledFailure).toHaveBeenCalledTimes(1); + expect(replacementCallback).not.toHaveBeenCalled(); + }); + + it("hides the fallback and reports only after automatic retries are exhausted", async () => { + const onSettledFailure = vi.fn(); + const fetchMock = vi.fn().mockResolvedValue({ ok: false, status: 500, json: async () => ({ error: "boom" }) }); + vi.stubGlobal("fetch", fetchMock); + + render( + , + ); + + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + expect(onSettledFailure).not.toHaveBeenCalled(); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(3), { timeout: 3_000 }); + await waitFor(() => expect(onSettledFailure).toHaveBeenCalledTimes(1)); + expect(screen.queryByText("Cover failed.")).not.toBeInTheDocument(); + expect(screen.queryByRole("status")).not.toBeInTheDocument(); + }); + + it("starts a new settled-failure callback cycle after manual retry", async () => { + const user = userEvent.setup(); + const onSettledFailure = vi.fn(); + const fetchMock = vi.fn().mockResolvedValue({ ok: false, status: 404, json: async () => ({ error: "not found" }) }); + vi.stubGlobal("fetch", fetchMock); + + render( + , + ); + + await screen.findByText("Image preview failed."); + await waitFor(() => expect(onSettledFailure).toHaveBeenCalledTimes(1)); + await user.click(screen.getByRole("button", { name: "Retry" })); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(onSettledFailure).toHaveBeenCalledTimes(2)); + }); + + it("starts a new settled-failure callback cycle when the endpoint changes", async () => { + const onSettledFailure = vi.fn(); + const fetchMock = vi.fn().mockResolvedValue({ ok: false, status: 404, json: async () => ({ error: "not found" }) }); + vi.stubGlobal("fetch", fetchMock); + + const { rerender } = render( + , + ); + await waitFor(() => expect(onSettledFailure).toHaveBeenCalledTimes(1)); + + rerender(); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(onSettledFailure).toHaveBeenCalledTimes(2)); + }); + it("honours Retry-After before automatically retrying a rate limit", async () => { const fetchMock = vi .fn() diff --git a/tests/use-document-cover.dom.test.tsx b/tests/use-document-cover.dom.test.tsx new file mode 100644 index 0000000000..6b058daf1a --- /dev/null +++ b/tests/use-document-cover.dom.test.tsx @@ -0,0 +1,192 @@ +import { act, cleanup, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const auth = vi.hoisted(() => ({ + value: { + authorizationHeader: { Authorization: "Bearer token-a" } as Record, + session: { user: { id: "user-a" } } as { user: { id: string } } | null, + }, +})); + +vi.mock("@/lib/supabase/client", () => ({ + useAuthSession: () => auth.value, +})); + +import { + resetDocumentCoverCacheForTests, + useDocumentCoverImageId, +} from "@/components/clinical-dashboard/use-document-cover"; + +const DOCUMENT_ID = "11111111-1111-4111-8111-111111111111"; + +function coverResponse(coverImageId: string | null) { + return { ok: true, status: 200, json: async () => ({ coverImageId }) }; +} + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((accept, decline) => { + resolve = accept; + reject = decline; + }); + return { promise, reject, resolve }; +} + +function setAuth(userId: string | null, token: string) { + auth.value = { + authorizationHeader: { Authorization: `Bearer ${token}` }, + session: userId ? { user: { id: userId } } : null, + }; +} + +beforeEach(() => { + resetDocumentCoverCacheForTests(); + setAuth("user-a", "token-a"); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + resetDocumentCoverCacheForTests(); +}); + +describe("useDocumentCoverImageId", () => { + it("forwards the active authorization header", async () => { + const fetchMock = vi.fn().mockResolvedValue(coverResponse("cover-a")); + vi.stubGlobal("fetch", fetchMock); + + const { result } = renderHook(() => useDocumentCoverImageId(DOCUMENT_ID)); + + await waitFor(() => expect(result.current.coverImageId).toBe("cover-a")); + expect(fetchMock).toHaveBeenCalledWith(`/api/documents/${DOCUMENT_ID}/cover`, { + headers: { Authorization: "Bearer token-a" }, + }); + }); + + it("binds in-flight work and resolved values to the authenticated identity", async () => { + const first = deferred>(); + const second = deferred>(); + const fetchMock = vi.fn().mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise); + vi.stubGlobal("fetch", fetchMock); + + const { result, rerender } = renderHook(() => useDocumentCoverImageId(DOCUMENT_ID)); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + + setAuth("user-b", "token-b"); + rerender(); + expect(result.current.coverImageId).toBeNull(); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + + await act(async () => first.resolve(coverResponse("cover-a"))); + expect(result.current.coverImageId).toBeNull(); + + await act(async () => second.resolve(coverResponse("cover-b"))); + await waitFor(() => expect(result.current.coverImageId).toBe("cover-b")); + + setAuth("user-a", "token-a-refreshed"); + rerender(); + expect(result.current.coverImageId).toBe("cover-a"); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("keeps a healthy cover through same-user token refresh without refetching", async () => { + const fetchMock = vi.fn().mockResolvedValue(coverResponse("cover-a")); + vi.stubGlobal("fetch", fetchMock); + const { result, rerender } = renderHook(() => useDocumentCoverImageId(DOCUMENT_ID)); + await waitFor(() => expect(result.current.coverImageId).toBe("cover-a")); + + setAuth("user-a", "token-a-refreshed"); + rerender(); + + expect(result.current.coverImageId).toBe("cover-a"); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("caches an authoritative null for the current identity and document", async () => { + const payload = deferred<{ coverImageId: null }>(); + const json = vi.fn().mockReturnValue(payload.promise); + const fetchMock = vi.fn().mockResolvedValue({ ok: true, status: 200, json }); + vi.stubGlobal("fetch", fetchMock); + const first = renderHook(() => useDocumentCoverImageId(DOCUMENT_ID)); + await waitFor(() => expect(json).toHaveBeenCalledTimes(1)); + await act(async () => payload.resolve({ coverImageId: null })); + first.unmount(); + + const second = renderHook(() => useDocumentCoverImageId(DOCUMENT_ID)); + + expect(second.result.current.coverImageId).toBeNull(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("does not cache a transient failure and retries on the next open", async () => { + const failedRequest = deferred>(); + const fetchMock = vi + .fn() + .mockReturnValueOnce(failedRequest.promise) + .mockResolvedValueOnce(coverResponse("cover-after-retry")); + vi.stubGlobal("fetch", fetchMock); + const { result, rerender } = renderHook( + ({ documentId }: { documentId: string | null }) => useDocumentCoverImageId(documentId), + { initialProps: { documentId: DOCUMENT_ID as string | null } }, + ); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + await act(async () => failedRequest.reject(new Error("offline"))); + + rerender({ documentId: null }); + rerender({ documentId: DOCUMENT_ID }); + + await waitFor(() => expect(result.current.coverImageId).toBe("cover-after-retry")); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("does not cache a malformed successful payload and retries on the next open", async () => { + const malformedPayload = deferred<{ unexpected: true }>(); + const malformedJson = vi.fn().mockReturnValue(malformedPayload.promise); + const fetchMock = vi + .fn() + .mockResolvedValueOnce({ ok: true, status: 200, json: malformedJson }) + .mockResolvedValueOnce(coverResponse("cover-after-malformed")); + vi.stubGlobal("fetch", fetchMock); + const { result, rerender } = renderHook( + ({ documentId }: { documentId: string | null }) => useDocumentCoverImageId(documentId), + { initialProps: { documentId: DOCUMENT_ID as string | null } }, + ); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(malformedJson).toHaveBeenCalledTimes(1)); + await act(async () => malformedPayload.resolve({ unexpected: true })); + + rerender({ documentId: null }); + rerender({ documentId: DOCUMENT_ID }); + + await waitFor(() => expect(result.current.coverImageId).toBe("cover-after-malformed")); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("ignores stale image callbacks and callbacks from a previous identity", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(coverResponse("cover-a")) + .mockResolvedValueOnce(coverResponse("cover-b")); + vi.stubGlobal("fetch", fetchMock); + const { result, rerender } = renderHook(() => useDocumentCoverImageId(DOCUMENT_ID)); + await waitFor(() => expect(result.current.coverImageId).toBe("cover-a")); + const markForUserA = result.current.markCoverUnavailable; + + act(() => result.current.markCoverUnavailable("another-cover")); + expect(result.current.coverImageId).toBe("cover-a"); + + setAuth("user-b", "token-b"); + rerender(); + await waitFor(() => expect(result.current.coverImageId).toBe("cover-b")); + act(() => markForUserA("cover-a")); + expect(result.current.coverImageId).toBe("cover-b"); + + setAuth("user-a", "token-a-refreshed"); + rerender(); + expect(result.current.coverImageId).toBe("cover-a"); + + act(() => result.current.markCoverUnavailable("cover-a")); + expect(result.current.coverImageId).toBeNull(); + }); +}); From d7d48cfa804abc2c4cf60082c275dfba744a3198 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:09:43 +0800 Subject: [PATCH 2/7] Allow failed cover previews to recover --- .../clinical-dashboard/use-document-cover.ts | 15 ++++++++--- tests/use-document-cover.dom.test.tsx | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/components/clinical-dashboard/use-document-cover.ts b/src/components/clinical-dashboard/use-document-cover.ts index ffa0dc66b8..3937d12ce3 100644 --- a/src/components/clinical-dashboard/use-document-cover.ts +++ b/src/components/clinical-dashboard/use-document-cover.ts @@ -113,11 +113,18 @@ export function useDocumentCoverImageId(documentId: string | null | undefined): // document/account. It may only invalidate the entry that is still // current, and only if that entry still names the failing image. if (!key || currentKeyRef.current !== key || coverImageIds.get(key) !== imageId) return; - coverImageIds.set(key, null); + // 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. After an identity round trip - // `fetched` can already be null, so setting it to null is a React no-op; - // explicitly notify this hook so the new cached null is observed. + // `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], diff --git a/tests/use-document-cover.dom.test.tsx b/tests/use-document-cover.dom.test.tsx index 6b058daf1a..ef0963d2dc 100644 --- a/tests/use-document-cover.dom.test.tsx +++ b/tests/use-document-cover.dom.test.tsx @@ -189,4 +189,29 @@ describe("useDocumentCoverImageId", () => { act(() => result.current.markCoverUnavailable("cover-a")); expect(result.current.coverImageId).toBeNull(); }); + + it("re-resolves a failed cover on the next open instead of pinning a negative cache entry", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(coverResponse("stale-cover")) + .mockResolvedValueOnce(coverResponse("replacement-cover")); + vi.stubGlobal("fetch", fetchMock); + + const { result, rerender } = renderHook( + ({ documentId }: { documentId: string | null }) => useDocumentCoverImageId(documentId), + { initialProps: { documentId: DOCUMENT_ID as string | null } }, + ); + await waitFor(() => expect(result.current.coverImageId).toBe("stale-cover")); + + act(() => result.current.markCoverUnavailable("stale-cover")); + expect(result.current.coverImageId).toBeNull(); + // Invalidating the external cache must not create an immediate request + // loop while the failed optional image is still mounted. + expect(fetchMock).toHaveBeenCalledTimes(1); + + rerender({ documentId: null }); + rerender({ documentId: DOCUMENT_ID }); + await waitFor(() => expect(result.current.coverImageId).toBe("replacement-cover")); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); }); From a1a148e6d29ceb1f4bd5c846c8805485ad2c1aec Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:30:36 +0800 Subject: [PATCH 3/7] Scope late cover failure invalidation --- .../clinical-dashboard/use-document-cover.ts | 11 +++++------ tests/use-document-cover.dom.test.tsx | 8 +++++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/components/clinical-dashboard/use-document-cover.ts b/src/components/clinical-dashboard/use-document-cover.ts index 3937d12ce3..88edeae111 100644 --- a/src/components/clinical-dashboard/use-document-cover.ts +++ b/src/components/clinical-dashboard/use-document-cover.ts @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { useAuthSession } from "@/lib/supabase/client"; @@ -79,8 +79,6 @@ export function useDocumentCoverImageId(documentId: string | null | undefined): const id = documentId ?? null; const authIdentity = session?.user?.id ?? null; const key = id ? coverCacheKey(id, authIdentity) : null; - const currentKeyRef = useRef(key); - currentKeyRef.current = key; /** * Reset happens during render, not in an effect. Clearing the previous * identity/document answer from inside an effect renders one frame with the @@ -110,9 +108,10 @@ export function useDocumentCoverImageId(documentId: string | null | undefined): const markCoverUnavailable = useCallback( (imageId: string) => { // A settled image failure can report after the drawer has paged to a new - // document/account. It may only invalidate the entry that is still - // current, and only if that entry still names the failing image. - if (!key || currentKeyRef.current !== key || coverImageIds.get(key) !== imageId) return; + // 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 diff --git a/tests/use-document-cover.dom.test.tsx b/tests/use-document-cover.dom.test.tsx index ef0963d2dc..9a2d0c5c35 100644 --- a/tests/use-document-cover.dom.test.tsx +++ b/tests/use-document-cover.dom.test.tsx @@ -167,7 +167,8 @@ describe("useDocumentCoverImageId", () => { const fetchMock = vi .fn() .mockResolvedValueOnce(coverResponse("cover-a")) - .mockResolvedValueOnce(coverResponse("cover-b")); + .mockResolvedValueOnce(coverResponse("cover-b")) + .mockResolvedValueOnce(coverResponse("cover-a-recovered")); vi.stubGlobal("fetch", fetchMock); const { result, rerender } = renderHook(() => useDocumentCoverImageId(DOCUMENT_ID)); await waitFor(() => expect(result.current.coverImageId).toBe("cover-a")); @@ -184,9 +185,10 @@ describe("useDocumentCoverImageId", () => { setAuth("user-a", "token-a-refreshed"); rerender(); - expect(result.current.coverImageId).toBe("cover-a"); + expect(result.current.coverImageId).toBeNull(); + await waitFor(() => expect(result.current.coverImageId).toBe("cover-a-recovered")); - act(() => result.current.markCoverUnavailable("cover-a")); + act(() => result.current.markCoverUnavailable("cover-a-recovered")); expect(result.current.coverImageId).toBeNull(); }); From 09ff8ca36489411f26408d6b557888459151bb4a Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:52:48 +0800 Subject: [PATCH 4/7] Keep answer source auth mock stable --- tests/answer-source-rail.dom.test.tsx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/answer-source-rail.dom.test.tsx b/tests/answer-source-rail.dom.test.tsx index 0bf08dafa7..d8c019065a 100644 --- a/tests/answer-source-rail.dom.test.tsx +++ b/tests/answer-source-rail.dom.test.tsx @@ -4,11 +4,16 @@ import { render, screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +const coverAuth = vi.hoisted(() => ({ + authorizationHeader: { Authorization: "Bearer cover-test" }, + session: { user: { id: "cover-test-user" } }, +})); + vi.mock("@/lib/supabase/client", () => ({ - useAuthSession: () => ({ - authorizationHeader: { Authorization: "Bearer cover-test" }, - session: { user: { id: "cover-test-user" } }, - }), + // The real provider memoizes this value until credentials change. Keeping + // the mock stable prevents an ordinary state update from masquerading as a + // token refresh and starting a new cover lookup in the same open drawer. + useAuthSession: () => coverAuth, })); vi.mock("@/components/clinical-dashboard/signed-image", () => ({ From a8b981ea8b8a73e1eba50adbf6db9d6066ea1bd8 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:13:59 +0800 Subject: [PATCH 5/7] Record image preview reliability review --- ...7505308b9e046181f8b978d85a4122db061e5b56a5f3cc0d988.record.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/branch-review-records/04098ccb23c407505308b9e046181f8b978d85a4122db061e5b56a5f3cc0d988.record.md diff --git a/docs/branch-review-records/04098ccb23c407505308b9e046181f8b978d85a4122db061e5b56a5f3cc0d988.record.md b/docs/branch-review-records/04098ccb23c407505308b9e046181f8b978d85a4122db061e5b56a5f3cc0d988.record.md new file mode 100644 index 0000000000..80d6968ae7 --- /dev/null +++ b/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 | From 67feb728c799316ab962e89fdfce13cc6597c98a Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:05:49 +0800 Subject: [PATCH 6/7] Key in-flight cover requests by credential A same-user token rotation while a cover lookup is pending must start a new request instead of inheriting a 401 from the old header. Co-authored-by: Cursor --- data/repo-awareness-snapshot.json | 16 +++++++++---- .../clinical-dashboard/use-document-cover.ts | 24 +++++++++++++++---- tests/use-document-cover.dom.test.tsx | 23 ++++++++++++++++++ 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/data/repo-awareness-snapshot.json b/data/repo-awareness-snapshot.json index 8d7bd3c84f..10e4b4e8b0 100644 --- a/data/repo-awareness-snapshot.json +++ b/data/repo-awareness-snapshot.json @@ -1,8 +1,8 @@ { "version": "repo-awareness-snapshot-v1", "captured_revision": { - "sha": "0abca58a554adeba03299363d436c4fed843618e", - "committed_at": "2026-08-26T15:30:47+08:00" + "sha": "3186205d5ef2bd630157a31b2e777f9c1873b8c5", + "committed_at": "2026-08-26T21:42:06+08:00" }, "routes": { "modes": [ @@ -3806,6 +3806,14 @@ "outcome": "built and verified; verify:cheap exit 0 (876 files / 10547 tests), verify:phone-chrome escalated to full Chromium 521 passed", "checks": "verify:cheap, verify:phone-chrome (full verify:ui), lint, typecheck" }, + { + "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/medication-risk-highlights", @@ -24776,8 +24784,8 @@ } ], "counts": { - "records": 2623, - "refs": 1602 + "records": 2624, + "refs": 1603 } } } diff --git a/src/components/clinical-dashboard/use-document-cover.ts b/src/components/clinical-dashboard/use-document-cover.ts index 88edeae111..9dfa98926e 100644 --- a/src/components/clinical-dashboard/use-document-cover.ts +++ b/src/components/clinical-dashboard/use-document-cover.ts @@ -31,15 +31,29 @@ function coverCacheKey(documentId: string, authIdentity: string | null) { return JSON.stringify([authIdentity, documentId]); } +function coverInFlightKey( + documentId: string, + authIdentity: string | null, + authorizationHeader: Record, +) { + // 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( key: string, documentId: string, + authIdentity: string | null, authorizationHeader: Record, ): Promise { const cached = coverImageIds.get(key); if (cached !== undefined) return cached; - const pending = inFlight.get(key); + const requestKey = coverInFlightKey(documentId, authIdentity, authorizationHeader); + const pending = inFlight.get(requestKey); if (pending) return pending; const request = (async () => { @@ -64,9 +78,9 @@ async function loadCoverImageId( } })(); - inFlight.set(key, request); + inFlight.set(requestKey, request); const resolved = await request; - inFlight.delete(key); + inFlight.delete(requestKey); if (resolved !== undefined) coverImageIds.set(key, resolved); return resolved; } @@ -97,13 +111,13 @@ export function useDocumentCoverImageId(documentId: string | null | undefined): useEffect(() => { if (!id || !key || coverImageIds.get(key) !== undefined) return; let active = true; - void loadCoverImageId(key, id, authorizationHeader).then((resolved) => { + void loadCoverImageId(key, id, authIdentity, authorizationHeader).then((resolved) => { if (active) setFetched(resolved ?? null); }); return () => { active = false; }; - }, [authorizationHeader, id, key]); + }, [authIdentity, authorizationHeader, id, key]); const markCoverUnavailable = useCallback( (imageId: string) => { diff --git a/tests/use-document-cover.dom.test.tsx b/tests/use-document-cover.dom.test.tsx index 9a2d0c5c35..4745b34b13 100644 --- a/tests/use-document-cover.dom.test.tsx +++ b/tests/use-document-cover.dom.test.tsx @@ -103,6 +103,29 @@ describe("useDocumentCoverImageId", () => { expect(fetchMock).toHaveBeenCalledTimes(1); }); + it("does not reuse an in-flight cover request after the same user rotates credentials", async () => { + const first = deferred<{ ok: boolean; status: number; json: () => Promise }>(); + const second = deferred>(); + const fetchMock = vi.fn().mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise); + vi.stubGlobal("fetch", fetchMock); + + const { result, rerender } = renderHook(() => useDocumentCoverImageId(DOCUMENT_ID)); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + + setAuth("user-a", "token-a-refreshed"); + rerender(); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + expect(fetchMock).toHaveBeenLastCalledWith(`/api/documents/${DOCUMENT_ID}/cover`, { + headers: { Authorization: "Bearer token-a-refreshed" }, + }); + + await act(async () => first.resolve({ ok: false, status: 401, json: async () => ({}) })); + expect(result.current.coverImageId).toBeNull(); + + await act(async () => second.resolve(coverResponse("cover-after-rotation"))); + await waitFor(() => expect(result.current.coverImageId).toBe("cover-after-rotation")); + }); + it("caches an authoritative null for the current identity and document", async () => { const payload = deferred<{ coverImageId: null }>(); const json = vi.fn().mockReturnValue(payload.promise); From b8c2bdf50ac06ba771871c149e232c628336b40d Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:44:24 +0800 Subject: [PATCH 7/7] Regenerate repo-awareness snapshot after merging main. The merge brought in new review records from #2390, so the committed snapshot lagged review_state and would fail Static PR. Co-authored-by: Cursor --- data/repo-awareness-snapshot.json | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/data/repo-awareness-snapshot.json b/data/repo-awareness-snapshot.json index d2394503cf..f946d7e65e 100644 --- a/data/repo-awareness-snapshot.json +++ b/data/repo-awareness-snapshot.json @@ -1,8 +1,8 @@ { "version": "repo-awareness-snapshot-v1", "captured_revision": { - "sha": "3186205d5ef2bd630157a31b2e777f9c1873b8c5", - "committed_at": "2026-08-26T21:42:06+08:00" + "sha": "ea58e0fcc734a79db3af625f800953333203f5bd", + "committed_at": "2026-08-26T22:38:53+08:00" }, "routes": { "modes": [ @@ -3827,6 +3827,14 @@ "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", @@ -24797,7 +24805,7 @@ } ], "counts": { - "records": 2625, + "records": 2626, "refs": 1604 } }