From 2a85da67f7a024bf31250ad7ee6671330658509e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 13 Aug 2026 12:07:39 +0200 Subject: [PATCH 01/11] Stop claiming a summary is pending when auto-read is off The upload path enqueued a summary job and stamped PENDING without consulting the owner's auto-read opt-in; the job's opt-out branch then deliberately left the state alone, so a document uploaded with the opt-in off said "summary is being generated" forever. Two writers, contradictory assumptions. Both ends of the contract now agree. enqueueDocumentSummary checks the opt-in first and refuses to mint a job (or a PENDING) it knows the job cannot resolve; the job keeps its own check for a pref flipped between enqueue and run and heals a stranded PENDING back to NONE, scoped so READY / WITHHELD / UNAVAILABLE stay untouched. Migration 0333 drains the rows already orphaned. The view needs nothing: NONE already renders the manual summary action. Refs #788 --- .../migration.sql | 12 +++ .../jobs/__tests__/document-summary.test.ts | 75 ++++++++++++++++++- src/lib/jobs/document-summary.ts | 48 +++++++++--- 3 files changed, 123 insertions(+), 12 deletions(-) create mode 100644 prisma/migrations/0333_summary_pending_heal_index_outcome/migration.sql diff --git a/prisma/migrations/0333_summary_pending_heal_index_outcome/migration.sql b/prisma/migrations/0333_summary_pending_heal_index_outcome/migration.sql new file mode 100644 index 000000000..9acfc02ee --- /dev/null +++ b/prisma/migrations/0333_summary_pending_heal_index_outcome/migration.sql @@ -0,0 +1,12 @@ +-- Refs #788 — heal phantom summary PENDING rows. +-- `enqueueDocumentSummary` used to claim PENDING even when the owner's +-- auto-read opt-in was OFF; the job's opt-out branch then left the state +-- alone, so the detail view said "being generated" forever. Both writers now +-- agree (the enqueue end no longer claims what the job cannot resolve, the +-- job end heals PENDING→NONE on opt-out); this one-time data fix drains the +-- rows already stranded. Safe by construction: READY / WITHHELD / UNAVAILABLE +-- are never touched, and a job that is genuinely mid-flight rewrites its +-- terminal state after this UPDATE anyway. +UPDATE "inbound_documents" +SET "summary_state" = 'NONE' +WHERE "summary_state" = 'PENDING'; diff --git a/src/lib/jobs/__tests__/document-summary.test.ts b/src/lib/jobs/__tests__/document-summary.test.ts index 6cf1db45c..a88010654 100644 --- a/src/lib/jobs/__tests__/document-summary.test.ts +++ b/src/lib/jobs/__tests__/document-summary.test.ts @@ -55,8 +55,12 @@ vi.mock("@/lib/ai/ai-budgets", () => ({ AI_BUDGETS: { documentSummary: { temperature: 0.3, maxTokens: 600 } }, })); -import { runDocumentSummaryJob } from "../document-summary"; +import { + enqueueDocumentSummary, + runDocumentSummaryJob, +} from "../document-summary"; import { prisma } from "@/lib/db"; +import { getGlobalBoss } from "@/lib/jobs/boss-instance"; import { documentAutoReadEnabled } from "@/lib/documents/document-settings"; import { resolveDocumentVisionProvider } from "@/lib/documents/provider-order"; import { @@ -124,14 +128,19 @@ beforeEach(() => { }); describe("runDocumentSummaryJob — gating", () => { - it("does NOTHING when the opt-in is OFF (no provider call, no write)", async () => { + it("does no provider work when the opt-in is OFF (only the PENDING heal may write)", async () => { vi.mocked(documentAutoReadEnabled).mockResolvedValue(false); await runDocumentSummaryJob({ userId: "user-1", documentId: "doc-1" }); expect(resolveDocumentVisionProvider).not.toHaveBeenCalled(); expect(runDocumentSummary).not.toHaveBeenCalled(); - expect(prisma.inboundDocument.updateMany).not.toHaveBeenCalled(); + // The single permitted write is the PENDING→NONE heal; nothing may claim + // an attempt (UNAVAILABLE/WITHHELD) or a summary (READY) here. + const calls = vi.mocked(prisma.inboundDocument.updateMany).mock.calls; + for (const [arg] of calls) { + expect(arg.data).toEqual({ summaryState: "NONE" }); + } }); it("generates and PERSISTS the summary encrypted when opt-in is ON + a provider is configured", async () => { @@ -248,4 +257,64 @@ describe("runDocumentSummaryJob — gating", () => { }), ); }); + + it("heals a phantom PENDING back to NONE when the opt-in is OFF", async () => { + vi.mocked(documentAutoReadEnabled).mockResolvedValue(false); + + await runDocumentSummaryJob({ userId: "user-1", documentId: "doc-1" }); + + // The opt-out branch may not leave a claimed PENDING standing — the view + // reads PENDING as "being generated" and this run is the only writer that + // can resolve it. The heal is scoped to PENDING: UNAVAILABLE / WITHHELD + // record real attempts and stay, READY is never touched. + expect(prisma.inboundDocument.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ summaryState: "PENDING" }), + data: { summaryState: "NONE" }, + }), + ); + // Still strictly no provider work. + expect(resolveDocumentVisionProvider).not.toHaveBeenCalled(); + expect(runDocumentSummary).not.toHaveBeenCalled(); + }); +}); + +describe("enqueueDocumentSummary — the pending contract", () => { + it("does not enqueue and does not claim PENDING when the opt-in is OFF", async () => { + vi.mocked(documentAutoReadEnabled).mockResolvedValue(false); + const send = vi.fn().mockResolvedValue("job-1"); + vi.mocked(getGlobalBoss).mockReturnValue({ send } as never); + + const result = await enqueueDocumentSummary("user-1", "doc-1"); + + // With auto-read OFF the job's only move is the opt-out no-op, so a + // PENDING claimed here could never be resolved to a real outcome. No job, + // no state write — the view keeps offering the manual action. + expect(result).toEqual({ enqueued: false }); + expect(send).not.toHaveBeenCalled(); + expect(prisma.inboundDocument.updateMany).not.toHaveBeenCalled(); + }); + + it("enqueues and claims PENDING when the opt-in is ON and a job was created", async () => { + const send = vi.fn().mockResolvedValue("job-1"); + vi.mocked(getGlobalBoss).mockReturnValue({ send } as never); + + const result = await enqueueDocumentSummary("user-1", "doc-1"); + + expect(result).toEqual({ enqueued: true }); + expect(send).toHaveBeenCalledTimes(1); + expect(prisma.inboundDocument.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ data: { summaryState: "PENDING" } }), + ); + }); + + it("claims nothing when the boss dropped the send (no job id)", async () => { + const send = vi.fn().mockResolvedValue(null); + vi.mocked(getGlobalBoss).mockReturnValue({ send } as never); + + const result = await enqueueDocumentSummary("user-1", "doc-1"); + + expect(result).toEqual({ enqueued: false }); + expect(prisma.inboundDocument.updateMany).not.toHaveBeenCalled(); + }); }); diff --git a/src/lib/jobs/document-summary.ts b/src/lib/jobs/document-summary.ts index 83bd6e022..ab3bff03b 100644 --- a/src/lib/jobs/document-summary.ts +++ b/src/lib/jobs/document-summary.ts @@ -109,8 +109,23 @@ export async function runDocumentSummaryJob( // on-demand route stays the only way to summarise). Fail-closed on a missing // row. if (!(await documentAutoReadEnabled(userId))) { - // Opting out is not a failed attempt — leave the state alone so the view - // keeps offering the manual action rather than claiming we tried. + // Opting out is not a failed attempt — but a claimed PENDING may not + // outlive this run either. PENDING promises the reader "a job will resolve + // this", and this job IS that resolution: with the opt-in OFF the honest + // terminal state is NONE (never attempted), which is what the enqueue end + // of the contract (`enqueueDocumentSummary`) also concludes when the pref + // is off. The heal is scoped to PENDING only — NONE needs no write, + // UNAVAILABLE / WITHHELD record real attempts, READY is a stored summary. + // This also drains any phantom PENDING left by a pref flip mid-flight. + await prisma.inboundDocument.updateMany({ + where: { + id: documentId, + userId, + deletedAt: null, + summaryState: "PENDING", + }, + data: { summaryState: "NONE" }, + }); annotate({ action: { name: "documents.summary.autoSkipped" }, meta: { documentId, reason: "opt-out" }, @@ -282,14 +297,29 @@ export async function enqueueDocumentSummary( userId: string, documentId: string, ): Promise<{ enqueued: boolean }> { - const boss = getGlobalBoss(); - if (!boss) return { enqueued: false }; - const payload: DocumentSummaryPayload = { - userId, - documentId, - enqueuedAt: new Date().toISOString(), - }; try { + // Contract, enqueue end: PENDING may only be claimed for work the job can + // terminally resolve. With the auto-read opt-in OFF the job's only branch + // is the opt-out no-op, so enqueueing would mint a job whose sole purpose + // is to undo this function's own PENDING — don't. No job, no state write; + // the document stays NONE and the view offers the manual action. The job + // keeps its own pref check (belt-and-braces for a pref flipped between + // enqueue and run) and heals PENDING→NONE there — the other end of this + // contract lives in `runDocumentSummaryJob`'s opt-out branch. + if (!(await documentAutoReadEnabled(userId))) { + annotate({ + action: { name: "documents.summary.autoSkipped" }, + meta: { documentId, reason: "opt-out" }, + }); + return { enqueued: false }; + } + const boss = getGlobalBoss(); + if (!boss) return { enqueued: false }; + const payload: DocumentSummaryPayload = { + userId, + documentId, + enqueuedAt: new Date().toISOString(), + }; const jobId = await boss.send(DOCUMENT_SUMMARY_QUEUE, payload, { retryLimit: 2, retryDelay: 30, From 4543fa71923a201823e88fd6016a2fc9c5bed3bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 13 Aug 2026 12:08:59 +0200 Subject: [PATCH 02/11] Let the app CSP frame and render its own blob previews Since the record fence, vault previews load through object URLs: the detail sheet frames a decrypted PDF from a blob: URL and thumbnails render from blob: images. The app CSP carried no frame-src, so the PDF iframe fell through to default-src 'self' and was blocked outright, while img-src 'self' data: silently degraded every thumbnail to its fallback icon. Add frame-src 'self' blob: and extend img-src with blob: in both the dev and prod policies. This admits exactly the app's own documents: a blob: URL is mintable only by same-origin script, script execution stays nonce-bound in production, and object-src 'none', frame-ancestors 'none' and default-src 'self' are untouched. The vault serve route keeps its stricter document CSP. A unit test pins the directives so a future CSP edit cannot silently regress the previews. Refs #787 --- src/__tests__/proxy-csp-blob-preview.test.ts | 100 +++++++++++++++++++ src/proxy.ts | 18 +++- 2 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 src/__tests__/proxy-csp-blob-preview.test.ts diff --git a/src/__tests__/proxy-csp-blob-preview.test.ts b/src/__tests__/proxy-csp-blob-preview.test.ts new file mode 100644 index 000000000..06811925f --- /dev/null +++ b/src/__tests__/proxy-csp-blob-preview.test.ts @@ -0,0 +1,100 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; + +/** + * Refs #787 — the record fence made every vault preview load through + * `useFencedObjectUrl`, i.e. `blob:` URLs. The app CSP had no `frame-src`, so + * `default-src 'self'` blocked the PDF preview iframe loudly, and + * `img-src 'self' data:` degraded every blob thumbnail to an icon silently. + * + * These tests pin the fix exactly: + * - `frame-src` exists and admits precisely 'self' and blob: — nothing else. + * - `img-src` admits blob: alongside 'self' and data:. + * - The boundaries that make blob: safe stay untouched: `object-src 'none'`, + * `frame-ancestors 'none'`, `default-src 'self'`, nonce-bound script-src. + * - The vault SERVE route keeps its own stricter document CSP unchanged. + * + * Security rationale (mirrors the comment in proxy.ts): a `blob:` URL can only + * be minted by same-origin script, and script execution stays nonce-bound, so + * admitting blob: admits exactly the app's own decrypted documents. + */ + +vi.mock("@/lib/process-type", () => ({ + shouldRunWeb: () => true, +})); + +import { proxy } from "../proxy"; + +function setNodeEnv(value: "development" | "production") { + vi.stubEnv("NODE_ENV", value); +} + +beforeEach(() => { + setNodeEnv("production"); +}); + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +function makeRequest(pathname: string): NextRequest { + return new NextRequest(`http://localhost${pathname}`, { + headers: { cookie: "healthlog_session=sess-1" }, + }); +} + +/** Parse one directive's source list out of a CSP header string. */ +function directive(csp: string, name: string): string[] | null { + const match = csp + .split(";") + .map((part) => part.trim()) + .find((part) => part.startsWith(`${name} `) || part === name); + if (!match) return null; + return match.split(/\s+/).slice(1); +} + +describe("proxy.ts blob preview CSP (#787)", () => { + it("production frame-src admits exactly 'self' and blob:", () => { + const res = proxy(makeRequest("/documents")); + const csp = res.headers.get("content-security-policy") ?? ""; + expect(directive(csp, "frame-src")).toEqual(["'self'", "blob:"]); + }); + + it("production img-src admits blob: alongside 'self' and data:", () => { + const res = proxy(makeRequest("/documents")); + const csp = res.headers.get("content-security-policy") ?? ""; + const sources = directive(csp, "img-src"); + expect(sources).toContain("blob:"); + expect(sources).toContain("'self'"); + expect(sources).toContain("data:"); + }); + + it("production keeps the boundaries that make blob: safe", () => { + const res = proxy(makeRequest("/documents")); + const csp = res.headers.get("content-security-policy") ?? ""; + expect(directive(csp, "object-src")).toEqual(["'none'"]); + expect(directive(csp, "frame-ancestors")).toEqual(["'none'"]); + expect(directive(csp, "default-src")).toEqual(["'self'"]); + expect(csp).toMatch(/script-src 'self' 'nonce-/); + }); + + it("development frame-src admits exactly 'self' and blob:", () => { + setNodeEnv("development"); + const res = proxy(makeRequest("/documents")); + const csp = res.headers.get("content-security-policy") ?? ""; + expect(directive(csp, "frame-src")).toEqual(["'self'", "blob:"]); + }); + + it("development img-src admits blob:", () => { + setNodeEnv("development"); + const res = proxy(makeRequest("/documents")); + const csp = res.headers.get("content-security-policy") ?? ""; + expect(directive(csp, "img-src")).toContain("blob:"); + }); + + it("leaves the vault serve route's document CSP untouched (no frame-src, no blob:)", () => { + const res = proxy(makeRequest("/api/documents/inbound/doc-1/original")); + const csp = res.headers.get("content-security-policy") ?? ""; + expect(csp).toBe("default-src 'none'; frame-ancestors 'self';"); + }); +}); diff --git a/src/proxy.ts b/src/proxy.ts index 1a2b435f5..dd95fdf28 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -528,9 +528,21 @@ export function proxy(request: NextRequest) { // HTML/SVG denied entirely, and attachment-only posture for every // non-passive format. `default-src 'none'; frame-ancestors 'self';` - : isDev - ? `default-src 'self'; script-src 'self' 'unsafe-eval' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self'; font-src 'self';` - : `default-src 'self'; script-src 'self' 'nonce-${nonce}'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self'${aiConnectSrc}${withingsConnectSrc}${whoopConnectSrc}${stravaConnectSrc}; font-src 'self'; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'; worker-src 'self'; report-uri ${cspReportEndpoint}; report-to csp-endpoint;`; + : // v1.37.15 — `frame-src 'self' blob:` + `blob:` in `img-src` (both dev + // and prod). Since the record fence (v1.37.0) every vault preview loads + // through `useFencedObjectUrl`, i.e. a `blob:` object URL: without a + // `frame-src` the PDF preview iframe fell through to `default-src + // 'self'` and was blocked loudly, and `img-src 'self' data:` degraded + // every blob thumbnail to its fallback icon silently. Admitting `blob:` + // is same-origin-tight: a blob: URL can only be minted by script running + // on this origin (`URL.createObjectURL`), script execution stays + // nonce-bound (prod), and `object-src 'none'` / `frame-ancestors + // 'none'` / `default-src 'self'` are unchanged — no foreign origin + // gains anything. Reverting the previews to direct `/api` src instead + // is barred by the record-fence guard. + isDev + ? `default-src 'self'; script-src 'self' 'unsafe-eval' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob:; frame-src 'self' blob:; connect-src 'self'; font-src 'self';` + : `default-src 'self'; script-src 'self' 'nonce-${nonce}'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob:; frame-src 'self' blob:; connect-src 'self'${aiConnectSrc}${withingsConnectSrc}${whoopConnectSrc}${stravaConnectSrc}; font-src 'self'; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'; worker-src 'self'; report-uri ${cspReportEndpoint}; report-to csp-endpoint;`; response.headers.set("Content-Security-Policy", csp); // Production-only headers. HSTS carries `preload` so the domain stays From 86a3d7418768b4358913778854bc2ec5a291eafa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 13 Aug 2026 12:13:55 +0200 Subject: [PATCH 03/11] Report how many documents the reindex actually queues The index-all trigger returned Boolean(jobId) while the client typed the field as a number, so the toast read "Indexing true document(s)". The route now answers with the count of live documents still lacking a content index, computed from the same live-scoped counts the usage gauge reads, and an honest 0 when no job was created. OpenAPI schema updated to the integer shape; the hook's type carries a contract note so the wire and the toast cannot drift apart again. Route test pins the number, a component test pins that the toast renders the count and never the word "true". Refs #776 --- docs/api/openapi.yaml | 7 +- .../inbound/reindex/__tests__/route.test.ts | 116 ++++++++++++++++ .../api/documents/inbound/reindex/route.ts | 28 +++- .../documents-view-reindex-toast.test.tsx | 131 ++++++++++++++++++ src/components/documents/use-content-index.ts | 11 +- src/lib/openapi/routes/documents.ts | 5 +- 6 files changed, 290 insertions(+), 8 deletions(-) create mode 100644 src/app/api/documents/inbound/reindex/__tests__/route.test.ts create mode 100644 src/components/documents/__tests__/documents-view-reindex-toast.test.tsx diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 8562757f0..8a1a951f6 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -12792,7 +12792,8 @@ paths: immediately; the work runs off-request on the queue. responses: "200": - description: Whether a backfill job was enqueued. + description: How many live documents the enqueued backfill still has to index (0 when everything is indexed or no job + was created). content: application/json: schema: @@ -38680,7 +38681,9 @@ components: type: object properties: enqueued: - type: boolean + type: integer + minimum: 0 + maximum: 9007199254740991 required: - enqueued additionalProperties: false diff --git a/src/app/api/documents/inbound/reindex/__tests__/route.test.ts b/src/app/api/documents/inbound/reindex/__tests__/route.test.ts new file mode 100644 index 000000000..d26443e4b --- /dev/null +++ b/src/app/api/documents/inbound/reindex/__tests__/route.test.ts @@ -0,0 +1,116 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; + +/** + * Refs #776 — the reindex trigger's wire honesty. The client types + * `{ enqueued: number }` and interpolates it into "Indexing {count} + * document(s)"; the route used to return `Boolean(jobId)`, so the toast said + * "Indexing true document(s)". Pins: `enqueued` is a NUMBER — the count of + * live documents still lacking a content index (the same figure the usage + * gauge derives) when a job was created, and an honest 0 when none was. + */ + +vi.mock("@/lib/db", () => ({ + prisma: { + inboundDocument: { count: vi.fn() }, + documentContentIndex: { count: vi.fn() }, + }, +})); +vi.mock("@/lib/jobs/document-content-index-backfill", () => ({ + enqueueContentIndexBackfill: vi.fn(), +})); +vi.mock("@/lib/documents/provider-order", () => ({ + resolveDocumentVisionProvider: vi.fn(), +})); +vi.mock("@/lib/ai/consent-guard", () => ({ + assertDocumentEgressConsent: vi.fn().mockResolvedValue(undefined), + ConsentRequiredError: class ConsentRequiredError extends Error {}, +})); +vi.mock("@/lib/modules/gate", () => ({ + requireModuleEnabled: vi.fn().mockResolvedValue({ enabled: true }), +})); +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn().mockResolvedValue({ allowed: true }), + rateLimitHeaders: () => ({}), +})); +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { POST } from "../route"; +import { prisma } from "@/lib/db"; +import { getSession } from "@/lib/auth/session"; +import { enqueueContentIndexBackfill } from "@/lib/jobs/document-content-index-backfill"; +import { resolveDocumentVisionProvider } from "@/lib/documents/provider-order"; + +const SESSION_OK = { + session: { id: "s1", expiresAt: new Date(Date.now() + 3_600_000) }, + user: { id: "user-1", username: "tester", role: "USER" as const }, +}; + +const PICK = { + chain: [{ providerType: "anthropic", instance: {} }], + pick: { + entry: { providerType: "anthropic", instance: {} }, + providerType: "anthropic", + pdfSupported: true, + }, +}; + +const req = () => + new NextRequest(new URL("http://localhost/api/documents/inbound/reindex"), { + method: "POST", + }); + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(resolveDocumentVisionProvider).mockResolvedValue(PICK as never); + vi.mocked(enqueueContentIndexBackfill).mockResolvedValue({ enqueued: true }); + // 7 live documents, 4 already indexed → 3 remain. + vi.mocked(prisma.inboundDocument.count).mockResolvedValue(7 as never); + vi.mocked(prisma.documentContentIndex.count).mockResolvedValue(4 as never); +}); + +describe("POST /api/documents/inbound/reindex", () => { + it("returns the remaining un-indexed count as a NUMBER, never a boolean", async () => { + const res = await POST(req() as never, {} as never); + expect(res.status).toBe(200); + const body = await res.json(); + expect(typeof body.data.enqueued).toBe("number"); + expect(body.data.enqueued).toBe(3); + }); + + it("returns an honest 0 when no job was created", async () => { + vi.mocked(enqueueContentIndexBackfill).mockResolvedValue({ + enqueued: false, + }); + const res = await POST(req() as never, {} as never); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.data.enqueued).toBe(0); + }); + + it("scopes both counts to the caller's LIVE documents (the gauge's own figures)", async () => { + await POST(req() as never, {} as never); + expect(prisma.inboundDocument.count).toHaveBeenCalledWith({ + where: { userId: "user-1", deletedAt: null }, + }); + expect(prisma.documentContentIndex.count).toHaveBeenCalledWith({ + where: { userId: "user-1", document: { deletedAt: null } }, + }); + }); +}); diff --git a/src/app/api/documents/inbound/reindex/route.ts b/src/app/api/documents/inbound/reindex/route.ts index c8f97cf9b..de2a13ff8 100644 --- a/src/app/api/documents/inbound/reindex/route.ts +++ b/src/app/api/documents/inbound/reindex/route.ts @@ -11,6 +11,7 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiError, apiSuccess, getClientIp } from "@/lib/api-response"; import { assertDocumentEgressConsent } from "@/lib/ai/consent-guard"; import { auditLog } from "@/lib/auth/audit"; +import { prisma } from "@/lib/db"; import { enqueueContentIndexBackfill } from "@/lib/jobs/document-content-index-backfill"; import { resolveDocumentVisionProvider } from "@/lib/documents/provider-order"; import { annotate } from "@/lib/logging/context"; @@ -58,16 +59,37 @@ export const POST = apiHandler(async (request) => { surface: "insights", }); - const { enqueued } = await enqueueContentIndexBackfill(user.id); + const { enqueued: jobCreated } = await enqueueContentIndexBackfill(user.id); + + // Wire honesty (#776): the client types `enqueued` as a NUMBER and + // interpolates it into "Indexing {count} document(s)" — a boolean here once + // rendered as "Indexing true document(s)". Report the count of live + // documents still lacking a content index (the exact figures the usage + // gauge already derives, same live-scoping: a soft-deleted document keeps + // its index row until hard purge, so both counts filter `deletedAt: null`). + // An honest 0 when no job was created — the client then says "everything is + // already indexed" instead of promising work nobody queued. + let enqueued = 0; + if (jobCreated) { + const [indexedCount, totalCount] = await Promise.all([ + prisma.documentContentIndex.count({ + where: { userId: user.id, document: { deletedAt: null } }, + }), + prisma.inboundDocument.count({ + where: { userId: user.id, deletedAt: null }, + }), + ]); + enqueued = Math.max(0, totalCount - indexedCount); + } await auditLog("documents.inbound.reindex", { userId: user.id, ipAddress: getClientIp(request), - details: { enqueued }, + details: { enqueued, jobCreated }, }); annotate({ action: { name: "documents.contentIndex.backfillEnqueue" }, - meta: { enqueued }, + meta: { enqueued, jobCreated }, }); return apiSuccess({ enqueued }); diff --git a/src/components/documents/__tests__/documents-view-reindex-toast.test.tsx b/src/components/documents/__tests__/documents-view-reindex-toast.test.tsx new file mode 100644 index 000000000..8b679a628 --- /dev/null +++ b/src/components/documents/__tests__/documents-view-reindex-toast.test.tsx @@ -0,0 +1,131 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +/** + * Refs #776 — the "index all" toast's count honesty. + * + * The reindex route reports `enqueued` as a NUMBER (documents still to + * index); the toast interpolates it into "Indexing {count} document(s)". + * Before the route fix the wire carried `Boolean(jobId)` and the toast said + * "Indexing true document(s)". This pins the component end of that contract: + * fed the real wire shape, the success toast renders the count and NEVER the + * word "true"; a 0 says "everything is already indexed" instead of promising + * queued work. + * + * House convention: `renderToStaticMarkup` + handler exercised via a mocked + * child reference (the filter bar captures `onIndexAll`), no DOM events. + */ + +let mockSearch = ""; +vi.mock("next/navigation", () => ({ + useRouter: () => ({ push: vi.fn(), replace: vi.fn() }), + usePathname: () => "/documents", + useSearchParams: () => new URLSearchParams(mockSearch), +})); + +vi.mock("@/hooks/use-auth", () => ({ + useAuth: () => ({ + user: { id: "u1", modules: { inboundDocuments: true } }, + isLoading: false, + isAuthenticated: true, + }), +})); + +vi.mock("@/lib/api/api-fetch", async () => { + const actual = await vi.importActual( + "@/lib/api/api-fetch", + ); + return { + ...actual, + apiGet: () => new Promise(() => {}), + apiPatch: vi.fn(), + apiPost: vi.fn(), + apiDelete: vi.fn(), + }; +}); + +const toastSuccess = vi.fn(); +const toastError = vi.fn(); +vi.mock("sonner", () => ({ + toast: { + success: (...args: unknown[]) => toastSuccess(...args), + error: (...args: unknown[]) => toastError(...args), + }, +})); + +// Capture the view's index-all handler through the filter bar's prop — the +// static render cannot click, so the mocked child is the seam. +let capturedOnIndexAll: (() => void) | undefined; +vi.mock("../document-filter-bar", () => ({ + DocumentFilterBar: (props: { onIndexAll?: () => void }) => { + capturedOnIndexAll = props.onIndexAll; + return null; + }, +})); + +// The reindex mutation resolves synchronously with a configurable wire +// payload — exactly what `apiPost` would unwrap from the route envelope. +let wirePayload: { enqueued: number }; +vi.mock("../use-content-index", () => ({ + useIndexDocument: () => ({ mutate: vi.fn(), isPending: false }), + useReindexAll: () => ({ + mutate: ( + _vars: void, + opts?: { onSuccess?: (result: { enqueued: number }) => void }, + ) => { + opts?.onSuccess?.(wirePayload); + }, + isPending: false, + }), +})); + +import { I18nProvider } from "@/lib/i18n/context"; +import { DocumentsView } from "../documents-view"; + +function render() { + mockSearch = ""; + capturedOnIndexAll = undefined; + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + renderToStaticMarkup( + + + + + , + ); +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe(" index-all toast (#776)", () => { + it("renders the remaining count — never the word 'true'", () => { + wirePayload = { enqueued: 3 }; + render(); + expect(capturedOnIndexAll).toBeDefined(); + + capturedOnIndexAll!(); + + expect(toastSuccess).toHaveBeenCalledTimes(1); + const message = String(toastSuccess.mock.calls[0]![0]); + expect(message).toContain("3"); + expect(message).not.toContain("true"); + expect(toastError).not.toHaveBeenCalled(); + }); + + it("says everything is indexed on an honest 0, not 'indexing 0 documents'", () => { + wirePayload = { enqueued: 0 }; + render(); + + capturedOnIndexAll!(); + + expect(toastSuccess).toHaveBeenCalledTimes(1); + const message = String(toastSuccess.mock.calls[0]![0]); + expect(message).toContain("already indexed"); + expect(message).not.toContain("true"); + }); +}); diff --git a/src/components/documents/use-content-index.ts b/src/components/documents/use-content-index.ts index 3d9434cac..28f164ba8 100644 --- a/src/components/documents/use-content-index.ts +++ b/src/components/documents/use-content-index.ts @@ -41,7 +41,16 @@ export function useIndexDocument() { }); } -/** Fire the corpus backfill; resolves with how many docs were enqueued. */ +/** + * Fire the corpus backfill; resolves with how many docs were enqueued. + * + * Wire contract (#776): `enqueued` is the NUMBER of live documents the + * backfill still has to index — the reindex route computes it from the same + * live-scoped counts the usage gauge reads, and returns an honest 0 when no + * job was created. The toast interpolates it as a count, so this type is + * load-bearing: it must match the route's shape exactly (a boolean here once + * rendered as "Indexing true document(s)"). + */ export function useReindexAll() { const queryClient = useQueryClient(); return useMutation<{ enqueued: number }, Error, void>({ diff --git a/src/lib/openapi/routes/documents.ts b/src/lib/openapi/routes/documents.ts index 5c52046f8..2014415a7 100644 --- a/src/lib/openapi/routes/documents.ts +++ b/src/lib/openapi/routes/documents.ts @@ -1028,12 +1028,13 @@ export const inboundDocumentPaths: NonNullable = { "Enqueues a background job that content-indexes the caller's not-yet-indexed documents (one provider transcription each, bounded + resumable). Gated on the module, a configured vision provider (422 `documents.inbound.providerUnsupported` otherwise), and the existing AI consent (403 otherwise). Returns immediately; the work runs off-request on the queue.", responses: { "200": { - description: "Whether a backfill job was enqueued.", + description: + "How many live documents the enqueued backfill still has to index (0 when everything is indexed or no job was created).", content: { "application/json": { schema: dataEnvelope( z - .object({ enqueued: z.boolean() }) + .object({ enqueued: z.number().int().nonnegative() }) .meta({ id: "DocumentReindexResponse" }), "DocumentReindexEnvelope", ), From 45dc93932a0f3e05d1535f7879fe24e8b92ced78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 13 Aug 2026 12:24:46 +0200 Subject: [PATCH 04/11] Settle a timed-out assessment refresh instead of calling it failed The force regenerate aborts its request at 45 s while the server's inline generation keeps running and usually writes the fresh briefing moments later. The old handling fired an error toast at the abort and re-read the old cache once; with a one hour staleTime and focus refetch off, nothing ever converged, so the page claimed failure while holding a finished assessment at arm's length. A timeout now enters a settling state: the hook remembers the cachedAt baseline it was showing and polls the read path on the existing bounded cadence (25 s, up to 10 attempts) until it can prove an outcome. Cache advanced past the baseline: the fresh payload lands and the success toast fires. A failure marker newer than the baseline, or the attempt cap: an honest error. While settling, the strip's button stays busy and says the assessment is still being prepared; no toast fires until the outcome is proven. Both poll reasons live in the nextAdvisorPollInterval seam, the verdict in resolveAdvisorSettle, the toast mapping in regenerateToastKind; all three are pure and pinned by unit tests. The contract is documented at both ends (hook and generate route). New copy in all six locales. Refs #786 --- messages/de.json | 2 + messages/en.json | 2 + messages/es.json | 2 + messages/fr.json | 2 + messages/it.json | 2 + messages/pl.json | 2 + .../inbound/reindex/__tests__/route.test.ts | 6 +- src/app/api/insights/generate/route.ts | 13 ++ .../insights-tab-strip-settling.test.tsx | 89 +++++++++ .../use-insights-advisor-settling.test.ts | 143 +++++++++++++++ .../insights/insights-layout-shell.tsx | 1 + .../insights/insights-tab-strip.tsx | 86 +++++++-- .../insights/use-insights-advisor.ts | 172 +++++++++++++++++- 13 files changed, 498 insertions(+), 24 deletions(-) create mode 100644 src/components/insights/__tests__/insights-tab-strip-settling.test.tsx create mode 100644 src/components/insights/__tests__/use-insights-advisor-settling.test.ts diff --git a/messages/de.json b/messages/de.json index 2d115aed0..779d21e65 100644 --- a/messages/de.json +++ b/messages/de.json @@ -4172,6 +4172,8 @@ "regenerateRateLimited": "Aktualisierungslimit erreicht — bitte später erneut versuchen.", "regenerateUnavailable": "Keine neue Analyse erhalten — bitte später erneut versuchen.", "regenerateError": "Aktualisierung läuft noch — bitte gleich erneut versuchen.", + "regenerateSettling": "Die Einschätzung wird noch erstellt", + "regenerateSettleFailed": "Die Aktualisierung konnte nicht abgeschlossen werden. Bitte später erneut versuchen.", "deviceScore": { "learning": "Deine Basislinie wird noch gelernt – ein paar weitere Nächte vervollständigen den Verlauf.", "average": "30-Tage-Durchschnitt {value}", diff --git a/messages/en.json b/messages/en.json index 15f1df75d..f417c2673 100644 --- a/messages/en.json +++ b/messages/en.json @@ -4172,6 +4172,8 @@ "regenerateRateLimited": "Refresh limit reached — please try again later.", "regenerateUnavailable": "No fresh analysis arrived — please try again later.", "regenerateError": "Still refreshing — please try again in a moment.", + "regenerateSettling": "Your assessment is still being prepared", + "regenerateSettleFailed": "The refresh could not be completed. Please try again later.", "deviceScore": { "learning": "Still learning your baseline — a few more nights will fill in the trend.", "average": "30-day average {value}", diff --git a/messages/es.json b/messages/es.json index 13c66febd..09bea375a 100644 --- a/messages/es.json +++ b/messages/es.json @@ -4172,6 +4172,8 @@ "regenerateRateLimited": "Límite de actualizaciones alcanzado: inténtalo de nuevo más tarde.", "regenerateUnavailable": "No llegó un análisis nuevo: inténtalo de nuevo más tarde.", "regenerateError": "Aún se está actualizando: inténtalo de nuevo en un momento.", + "regenerateSettling": "La valoración todavía se está preparando", + "regenerateSettleFailed": "No se pudo completar la actualización. Inténtalo de nuevo más tarde.", "deviceScore": { "learning": "Aún estamos aprendiendo tu línea base: unas noches más completarán la tendencia.", "average": "Media de 30 días {value}", diff --git a/messages/fr.json b/messages/fr.json index dd414bccc..f55859fba 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -4172,6 +4172,8 @@ "regenerateRateLimited": "Limite d'actualisation atteinte — réessayez plus tard.", "regenerateUnavailable": "Aucune nouvelle analyse reçue — réessayez plus tard.", "regenerateError": "Actualisation en cours — réessayez dans un instant.", + "regenerateSettling": "L’évaluation est encore en cours de préparation", + "regenerateSettleFailed": "L’actualisation n’a pas pu aboutir. Réessayez plus tard.", "deviceScore": { "learning": "Nous apprenons encore votre référence : quelques nuits de plus compléteront la tendance.", "average": "Moyenne sur 30 jours {value}", diff --git a/messages/it.json b/messages/it.json index 7ee83cfc5..844cdb904 100644 --- a/messages/it.json +++ b/messages/it.json @@ -4172,6 +4172,8 @@ "regenerateRateLimited": "Limite di aggiornamento raggiunto — riprova più tardi.", "regenerateUnavailable": "Nessuna nuova analisi ricevuta — riprova più tardi.", "regenerateError": "Aggiornamento ancora in corso — riprova tra un momento.", + "regenerateSettling": "La valutazione è ancora in preparazione", + "regenerateSettleFailed": "Non è stato possibile completare l’aggiornamento. Riprova più tardi.", "deviceScore": { "learning": "Stiamo ancora imparando la tua linea di base: qualche notte in più completerà l'andamento.", "average": "Media a 30 giorni {value}", diff --git a/messages/pl.json b/messages/pl.json index 91caae79a..082ee3bac 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -4172,6 +4172,8 @@ "regenerateRateLimited": "Osiągnięto limit odświeżeń — spróbuj ponownie później.", "regenerateUnavailable": "Nie otrzymano nowej analizy — spróbuj ponownie później.", "regenerateError": "Trwa odświeżanie — spróbuj ponownie za chwilę.", + "regenerateSettling": "Ocena jest wciąż przygotowywana", + "regenerateSettleFailed": "Nie udało się ukończyć odświeżania. Spróbuj ponownie później.", "deviceScore": { "learning": "Wciąż poznajemy Twoją linię bazową – kilka kolejnych nocy uzupełni trend.", "average": "Średnia z 30 dni {value}", diff --git a/src/app/api/documents/inbound/reindex/__tests__/route.test.ts b/src/app/api/documents/inbound/reindex/__tests__/route.test.ts index d26443e4b..ddcf1900b 100644 --- a/src/app/api/documents/inbound/reindex/__tests__/route.test.ts +++ b/src/app/api/documents/inbound/reindex/__tests__/route.test.ts @@ -87,7 +87,7 @@ beforeEach(() => { describe("POST /api/documents/inbound/reindex", () => { it("returns the remaining un-indexed count as a NUMBER, never a boolean", async () => { - const res = await POST(req() as never, {} as never); + const res = await POST(req() as never); expect(res.status).toBe(200); const body = await res.json(); expect(typeof body.data.enqueued).toBe("number"); @@ -98,14 +98,14 @@ describe("POST /api/documents/inbound/reindex", () => { vi.mocked(enqueueContentIndexBackfill).mockResolvedValue({ enqueued: false, }); - const res = await POST(req() as never, {} as never); + const res = await POST(req() as never); expect(res.status).toBe(200); const body = await res.json(); expect(body.data.enqueued).toBe(0); }); it("scopes both counts to the caller's LIVE documents (the gauge's own figures)", async () => { - await POST(req() as never, {} as never); + await POST(req() as never); expect(prisma.inboundDocument.count).toHaveBeenCalledWith({ where: { userId: "user-1", deletedAt: null }, }); diff --git a/src/app/api/insights/generate/route.ts b/src/app/api/insights/generate/route.ts index 1cd21297c..8c13b2c62 100644 --- a/src/app/api/insights/generate/route.ts +++ b/src/app/api/insights/generate/route.ts @@ -1,3 +1,16 @@ +/** + * Refs #786 — async-truth contract, server end (the client end lives in + * `src/components/insights/use-insights-advisor.ts`, `AdvisorSettleState`): + * the force POST generates INLINE and, on success, writes the fresh briefing + * to the user cache (`insightsCachedAt` advances) — even when the client has + * long since aborted its 45 s request. A failed inline generation writes a + * failure marker the GET reports as `generationFailed`. The client relies on + * exactly those two signals to settle a timed-out regenerate: it polls the + * GET until `cachedAt` advances past its baseline (success) or a NEWER + * failure marker appears (honest error). Renaming or short-circuiting either + * write breaks that convergence — the settling seam tests pin the client + * half, this comment is the server half of the contract. + */ import { prisma } from "@/lib/db"; import { auditLog } from "@/lib/auth/audit"; import { diff --git a/src/components/insights/__tests__/insights-tab-strip-settling.test.tsx b/src/components/insights/__tests__/insights-tab-strip-settling.test.tsx new file mode 100644 index 000000000..c296cab43 --- /dev/null +++ b/src/components/insights/__tests__/insights-tab-strip-settling.test.tsx @@ -0,0 +1,89 @@ +import { describe, it, expect, vi } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; + +/** + * Refs #786 — the regenerate button's settling state. + * + * While a timed-out force regenerate is settling (the server is still + * generating; the hook polls for the outcome) the button may NOT offer a + * retry or read as failed: it stays disabled, keeps the spinner, and its + * label says the assessment is still being prepared. Static renders pin the + * three visible facts; the toast edges live in the hook/strip effect and are + * pinned by the outcome mapping (`regenerateToastKind`). + */ + +vi.mock("next/navigation", () => ({ + usePathname: () => "/insights", +})); +vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +import { InsightsTabStrip, regenerateToastKind } from "../insights-tab-strip"; +import { I18nProvider } from "@/lib/i18n/context"; + +function render(node: React.ReactNode) { + return renderToStaticMarkup( + {node}, + ); +} + +describe(" regenerate settling state (#786)", () => { + it("settling: button disabled, spinner on, label says still being prepared", () => { + const html = render( + {}} + regenerating={false} + regenerateSettling + />, + ); + expect(html).toContain('data-slot="insights-tab-strip-regenerate"'); + expect(html).toContain("Your assessment is still being prepared"); + expect(html).toContain("disabled"); + expect(html).toContain("animate-spin"); + }); + + it("idle: button enabled with the regenerate label", () => { + const html = render( + {}} regenerating={false} />, + ); + expect(html).toContain("Re-run assessment"); + expect(html).not.toContain("Your assessment is still being prepared"); + }); +}); + +describe("regenerateToastKind — falling-edge outcome mapping (#786)", () => { + it("maps settle-failed to the honest settle error, never success", () => { + expect(regenerateToastKind("settle-failed")).toEqual({ + kind: "error", + messageKey: "insights.regenerateSettleFailed", + }); + }); + + it("keeps the existing mappings", () => { + expect(regenerateToastKind("fresh")).toEqual({ + kind: "success", + messageKey: "insights.regenerateSuccess", + }); + expect(regenerateToastKind("timeout")).toEqual({ + kind: "error", + messageKey: "insights.regenerateError", + }); + expect(regenerateToastKind("rate-limited")).toEqual({ + kind: "error", + messageKey: "insights.regenerateRateLimited", + }); + expect(regenerateToastKind("empty")).toEqual({ + kind: "error", + messageKey: "insights.regenerateUnavailable", + }); + expect(regenerateToastKind("no-provider")).toBe(null); + // Legacy mounts without an outcome keep the unconditional success. + expect(regenerateToastKind(null)).toEqual({ + kind: "success", + messageKey: "insights.regenerateSuccess", + }); + expect(regenerateToastKind(undefined)).toEqual({ + kind: "success", + messageKey: "insights.regenerateSuccess", + }); + }); +}); diff --git a/src/components/insights/__tests__/use-insights-advisor-settling.test.ts b/src/components/insights/__tests__/use-insights-advisor-settling.test.ts new file mode 100644 index 000000000..855c1c6cf --- /dev/null +++ b/src/components/insights/__tests__/use-insights-advisor-settling.test.ts @@ -0,0 +1,143 @@ +import { describe, it, expect } from "vitest"; + +/** + * Refs #786 — the async-truth contract for the force regenerate. + * + * A regenerate the client gives up on at 45 s is NOT a failure: the server's + * inline generation keeps running and usually writes the cache moments later. + * The old behaviour fired an error toast and then never converged (staleTime + * 1 h + focus refetch off), so the page kept showing the old assessment while + * claiming the refresh failed. The fix enters a "settling" state: remember + * the baseline `cachedAt`, keep polling on the existing bounded cadence, and + * resolve to a REAL outcome — fresh (cache advanced), settle-failed (a newer + * failure marker, or the attempt cap). + * + * Two pure seams pin the state machine: + * - `nextAdvisorPollInterval` gains settling as the second poll reason, + * with attempts counted from the settle start. + * - `resolveAdvisorSettle` decides fresh / settle-failed / keep-waiting. + */ + +import { + ADVISOR_REVALIDATE_POLL_MAX_ATTEMPTS, + ADVISOR_REVALIDATE_POLL_MS, + nextAdvisorPollInterval, + resolveAdvisorSettle, + type AdvisorSettleState, +} from "../use-insights-advisor"; + +function settle( + overrides: Partial = {}, +): AdvisorSettleState { + return { + baselineCachedAt: "2026-08-13T06:00:00.000Z", + baselineFailed: false, + startedAtUpdateCount: 4, + ...overrides, + }; +} + +describe("nextAdvisorPollInterval — settling is the second poll reason (#786)", () => { + it("polls while settling even when nothing reports revalidating", () => { + expect(nextAdvisorPollInterval(false, 5, settle())).toBe( + ADVISOR_REVALIDATE_POLL_MS, + ); + expect(nextAdvisorPollInterval(undefined, 5, settle())).toBe( + ADVISOR_REVALIDATE_POLL_MS, + ); + }); + + it("counts settling attempts from the settle start, not from mount", () => { + // 12 data updates total but only 8 since settling began → keep polling. + expect( + nextAdvisorPollInterval(false, 12, settle({ startedAtUpdateCount: 4 })), + ).toBe(ADVISOR_REVALIDATE_POLL_MS); + }); + + it("stops at the settling attempt ceiling", () => { + expect( + nextAdvisorPollInterval( + false, + 4 + ADVISOR_REVALIDATE_POLL_MAX_ATTEMPTS, + settle({ startedAtUpdateCount: 4 }), + ), + ).toBe(false); + }); + + it("keeps the pre-existing revalidating behaviour when not settling", () => { + expect(nextAdvisorPollInterval(true, 1, null)).toBe( + ADVISOR_REVALIDATE_POLL_MS, + ); + expect(nextAdvisorPollInterval(false, 1, null)).toBe(false); + expect(nextAdvisorPollInterval(true, 1)).toBe(ADVISOR_REVALIDATE_POLL_MS); + }); +}); + +describe("resolveAdvisorSettle — the settle state machine (#786)", () => { + it("resolves fresh once cachedAt advances past the baseline", () => { + expect( + resolveAdvisorSettle( + settle(), + { cachedAt: "2026-08-13T06:02:31.000Z", generationFailed: false }, + 6, + ), + ).toBe("fresh"); + }); + + it("resolves fresh on ANY cachedAt when there was no baseline (first generation)", () => { + expect( + resolveAdvisorSettle( + settle({ baselineCachedAt: null }), + { cachedAt: "2026-08-13T06:02:31.000Z", generationFailed: false }, + 6, + ), + ).toBe("fresh"); + }); + + it("keeps waiting while the cache is unchanged and attempts remain", () => { + expect( + resolveAdvisorSettle( + settle(), + { cachedAt: "2026-08-13T06:00:00.000Z", generationFailed: false }, + 6, + ), + ).toBe(null); + }); + + it("resolves settle-failed on a NEWER failure marker (failed flipped during settling)", () => { + expect( + resolveAdvisorSettle( + settle({ baselineFailed: false }), + { cachedAt: "2026-08-13T06:00:00.000Z", generationFailed: true }, + 6, + ), + ).toBe("settle-failed"); + }); + + it("does NOT treat a failure marker that predates the settle as fresh evidence", () => { + // The baseline already carried the failed flag — an unchanged flag says + // nothing about THIS generation; keep waiting for cachedAt or the cap. + expect( + resolveAdvisorSettle( + settle({ baselineFailed: true }), + { cachedAt: "2026-08-13T06:00:00.000Z", generationFailed: true }, + 6, + ), + ).toBe(null); + }); + + it("resolves settle-failed at the attempt cap — never waits forever", () => { + expect( + resolveAdvisorSettle( + settle({ startedAtUpdateCount: 4 }), + { cachedAt: "2026-08-13T06:00:00.000Z", generationFailed: false }, + 4 + ADVISOR_REVALIDATE_POLL_MAX_ATTEMPTS, + ), + ).toBe("settle-failed"); + }); + + it("a missing payload keeps waiting (no evidence either way)", () => { + expect(resolveAdvisorSettle(settle(), null, 6)).toBe(null); + expect(resolveAdvisorSettle(settle(), undefined, 6)).toBe(null); + }); +}); diff --git a/src/components/insights/insights-layout-shell.tsx b/src/components/insights/insights-layout-shell.tsx index bc861d344..dbf4d0ea6 100644 --- a/src/components/insights/insights-layout-shell.tsx +++ b/src/components/insights/insights-layout-shell.tsx @@ -200,6 +200,7 @@ export function InsightsLayoutShell({ children }: { children: ReactNode }) { onRegenerate={advisorEnabled ? advisor.regenerate : undefined} regenerating={advisor.isRegenerating} regenerateOutcome={advisor.regenerateOutcome} + regenerateSettling={advisor.regenerateSettling} availability={availability} visibleTileIds={visibleTileIds} tileOrder={tileOrder} diff --git a/src/components/insights/insights-tab-strip.tsx b/src/components/insights/insights-tab-strip.tsx index 33e41e837..60f6613d0 100644 --- a/src/components/insights/insights-tab-strip.tsx +++ b/src/components/insights/insights-tab-strip.tsx @@ -88,7 +88,20 @@ export interface InsightsTabStripProps { * mounts / no regenerate) falls back to the prior unconditional success. */ regenerateOutcome?: - "fresh" | "empty" | "rate-limited" | "timeout" | "no-provider" | null; + | "fresh" + | "empty" + | "rate-limited" + | "timeout" + | "no-provider" + | "settle-failed" + | null; + /** + * Refs #786 — true while a timed-out regenerate is settling (the server is + * still generating; the hook polls for the proven outcome). The button + * stays busy with a "still being prepared" label, and the falling-edge + * toast waits for the COMBINED busy state so no error fires mid-settle. + */ + regenerateSettling?: boolean; /** * v1.4.27 F19 — analytics + event-driven availability inputs the * gating helper reads. When omitted the strip falls back to its @@ -637,10 +650,38 @@ function buildTabs( return entries; } +/** + * Refs #786 — the falling-edge toast decision as a pure seam. Maps the last + * settled outcome to the toast that fires when the busy state falls: + * `settle-failed` (a settling poll proved the generation failed, or the + * attempt cap ran out) gets its own honest error copy; `no-provider` stays + * silent (the surface already shows the connect-AI empty state); a missing + * outcome (legacy mounts) keeps the historical unconditional success. + */ +export function regenerateToastKind( + outcome: InsightsTabStripProps["regenerateOutcome"] | undefined, +): { kind: "success" | "error"; messageKey: string } | null { + switch (outcome) { + case "timeout": + return { kind: "error", messageKey: "insights.regenerateError" }; + case "rate-limited": + return { kind: "error", messageKey: "insights.regenerateRateLimited" }; + case "empty": + return { kind: "error", messageKey: "insights.regenerateUnavailable" }; + case "settle-failed": + return { kind: "error", messageKey: "insights.regenerateSettleFailed" }; + case "no-provider": + return null; + default: + return { kind: "success", messageKey: "insights.regenerateSuccess" }; + } +} + function InsightsTabStripImpl({ onRegenerate, regenerating = false, regenerateOutcome, + regenerateSettling = false, availability, visibleTileIds, tileOrder, @@ -686,24 +727,33 @@ function InsightsTabStripImpl({ // regenerate claimed "refreshed" while serving the old text. The latest // outcome is read through a ref-free dep so the effect fires only on the // spinner edge. - const lastRegeneratingRef = useRef(regenerating); + // + // Refs #786 — the edge watches the COMBINED busy state (`regenerating || + // regenerateSettling`): a timed-out regenerate flows straight into + // settling, so the spinner never falls at 45 s and NO error toast fires + // while the server may still deliver. The edge fires once settling + // resolves, with the proven outcome — `"fresh"` (success) or + // `"settle-failed"` (its own honest error copy). The mapping is the pure + // `regenerateToastKind` seam above. + const busy = regenerating || regenerateSettling; + const lastBusyRef = useRef(busy); useEffect(() => { - if (lastRegeneratingRef.current && !regenerating) { - if (regenerateOutcome === "timeout") { - toast.error(t("insights.regenerateError")); - } else if (regenerateOutcome === "rate-limited") { - toast.error(t("insights.regenerateRateLimited")); - } else if (regenerateOutcome === "empty") { - toast.error(t("insights.regenerateUnavailable")); - } else if (regenerateOutcome !== "no-provider") { - // `"fresh"` or legacy `undefined`/null → success. - toast.success(t("insights.regenerateSuccess")); + if (lastBusyRef.current && !busy) { + const decision = regenerateToastKind(regenerateOutcome); + if (decision) { + if (decision.kind === "error") { + toast.error(t(decision.messageKey)); + } else { + toast.success(t(decision.messageKey)); + } } } - lastRegeneratingRef.current = regenerating; - }, [regenerating, regenerateOutcome, t]); + lastBusyRef.current = busy; + }, [busy, regenerateOutcome, t]); - const regenerateLabel = t("insights.regenerateAnalysis"); + const regenerateLabel = regenerateSettling + ? t("insights.regenerateSettling") + : t("insights.regenerateAnalysis"); const customizeLabel = t("insights.customize"); // Overflow affordance for the pill row. The row hides its scrollbar @@ -1000,7 +1050,9 @@ function InsightsTabStripImpl({