From ea6e32753fed8ce98afe4c88080b4f44998d9850 Mon Sep 17 00:00:00 2001 From: EthanHealy01 Date: Thu, 27 Aug 2026 14:40:16 +0100 Subject: [PATCH] Report every failure a run produces, not just a total one Three gaps meant a failure could happen and nothing would ever say so. A batch that lost one input to a bad PDF reached the success path, because processFiles only rethrows when EVERY input fails. The per-input errors were discarded, so the report and the retry stash never ran: one bad file in twenty was silently swallowed. processFiles now returns the failed inputs with their errors, and both paths report through one helper so they cannot drift. A locked document never escalated to the server classifier. The local pass throws on an encrypted file and writes no verdict, and shouldDispatchToAi read an absent verdict as "not yet" rather than "never" - so no server run was dispatched, nothing recorded the failure, and the bell stayed empty. A recurrence could not reopen an incident closed as FILE_REMOVED. A library re-adds a file under the same id, so every later failure folded into the closed row and left the queue for good. RESOLVED and FILE_REMOVED now both reopen; DISMISSED still stands, being a reviewer's decision rather than a claim about the document. Also: appliedCategoriesFor no longer counts the browser-local pass as the policy having run, which would have suppressed the same escalation; a retry opens the failed tool in the viewer, the only view that scopes a tool to one document; and View in processor navigates in place now that the workbench survives the trip. --- .../proprietary/failure/FailureKind.java | 6 +- .../failure/FileRunEventRepository.java | 9 +- .../failure/FileRunEventStatus.java | 5 +- .../proprietary/failure/FileRunEventView.java | 4 +- .../notification/NotificationController.java | 4 +- .../failure/FileRunEventServiceTest.java | 29 +++++ .../InMemoryFileRunEventRepository.java | 4 +- .../shared/useToolApiCalls.failures.test.ts | 99 +++++++++++++++ .../hooks/tools/shared/useToolApiCalls.ts | 28 ++++- .../hooks/tools/shared/useToolOperation.ts | 89 +++++++++----- .../src/core/services/failureReporting.ts | 11 +- .../notificationActions.test.tsx | 29 +---- .../notifications/notificationActions.ts | 14 +-- .../policies/policyRunStore.test.ts | 115 ++++++++++++++++++ .../components/policies/policyRunStore.ts | 34 ++++-- .../components/policies/usePolicyAutoRun.ts | 14 ++- .../data/classificationPolicy.test.ts | 12 ++ .../proprietary/data/classificationPolicy.ts | 11 +- 18 files changed, 404 insertions(+), 113 deletions(-) create mode 100644 frontend/editor/src/core/hooks/tools/shared/useToolApiCalls.failures.test.ts diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java index ec9146b122..dfa83b69a0 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java @@ -25,10 +25,8 @@ import lombok.AccessLevel; import lombok.Getter; /** - * The registry of failure kinds, described as data: a stable id, i18n keys and an English fallback - * like {@code ExceptionUtils.ErrorCode}, plus the facets a review surface needs. - * - *

A new kind ships as a registry entry plus copy. Each offer says who it is for and where. + * The registry of failure kinds as data: id, i18n keys, English fallback, plus the facets a review + * surface needs. A new kind ships as an entry plus copy; each offer says who it is for and where. */ @Getter public enum FailureKind { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java index 316dd34925..ae6b7acb0c 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java @@ -64,16 +64,17 @@ public interface FileRunEventRepository extends JpaRepository ({ + default: { post: (...args: unknown[]) => post(...args) }, +})); + +// Mirrors the response body, so a 0-byte answer really does produce an empty output. +vi.mock("@app/utils/toolResponseProcessor", () => ({ + processResponse: (blob: Blob, files: { name: string }[]) => + Promise.resolve([new File([blob], `out-${files[0].name}`)]), +})); + +const { useToolApiCalls } = + await import("@app/hooks/tools/shared/useToolApiCalls"); + +const file = (name: string, id: string): StirlingFile => + ({ name, fileId: id, size: 10 }) as unknown as StirlingFile; + +function run(files: StirlingFile[]) { + const { processFiles } = renderHook(() => useToolApiCalls()).result.current; + return processFiles( + undefined, + files, + { + endpoint: "/api/v1/misc/compress-pdf", + buildFormData: () => new FormData(), + }, + () => {}, + () => {}, + ); +} + +beforeEach(() => { + post.mockReset(); +}); + +describe("processFiles failure reporting", () => { + it("names the failed input and keeps its error while the rest succeed", async () => { + expectConsole.error("[processFiles] Failed"); + const boom = new Error("corrupted"); + let call = 0; + post.mockImplementation(() => { + call += 1; + return call === 2 + ? Promise.reject(boom) + : Promise.resolve({ data: new Blob(["ok"]), status: 200, headers: {} }); + }); + + const result = await run([ + file("a.pdf", "f-a"), + file("bad.pdf", "f-bad"), + file("c.pdf", "f-c"), + ]); + + expect(result.outputFiles).toHaveLength(2); + expect(result.successSourceIds).toEqual(["f-a", "f-c"]); + expect(result.failedInputs).toEqual([ + { fileId: "f-bad", name: "bad.pdf", error: boom }, + ]); + }); + + it("reports nothing when every input succeeded", async () => { + post.mockResolvedValue({ + data: new Blob(["ok"]), + status: 200, + headers: {}, + }); + + const result = await run([file("a.pdf", "f-a")]); + + expect(result.failedInputs).toEqual([]); + }); + + it("treats an empty output as a failure the caller must hear about", async () => { + // A 200 with a 0-byte body is a failure the old code counted only in a status string. + expectConsole.warn("[processFiles] Empty output treated as failure"); + let call = 0; + post.mockImplementation(() => { + call += 1; + return Promise.resolve({ + data: new Blob(call === 1 ? ["ok"] : []), + status: 200, + headers: {}, + }); + }); + + const result = await run([file("a.pdf", "f-a"), file("empty.pdf", "f-e")]); + + expect(result.successSourceIds).toEqual(["f-a"]); + expect(result.failedInputs.map((f) => f.fileId)).toEqual(["f-e"]); + }); +}); diff --git a/frontend/editor/src/core/hooks/tools/shared/useToolApiCalls.ts b/frontend/editor/src/core/hooks/tools/shared/useToolApiCalls.ts index cca1f5e563..c5c9246ef7 100644 --- a/frontend/editor/src/core/hooks/tools/shared/useToolApiCalls.ts +++ b/frontend/editor/src/core/hooks/tools/shared/useToolApiCalls.ts @@ -9,6 +9,13 @@ import { isEmptyOutput } from "@app/services/errorUtils"; import type { ProcessingProgress } from "@app/hooks/tools/shared/useToolState"; import type { StirlingFile, FileId } from "@app/types/fileContext"; +/** An input that did not survive the batch, with the error it failed on. */ +export interface FailedInput { + fileId: FileId; + name: string; + error: unknown; +} + export interface ApiCallsConfig { endpoint: string | null | ((params: TParams) => string | null); buildFormData: (params: TParams, file: File) => FormData; @@ -28,9 +35,16 @@ export const useToolApiCalls = () => { onProgress: (progress: ProcessingProgress) => void, onStatus: (status: string) => void, markFileError?: (fileId: FileId) => void, - ): Promise<{ outputFiles: File[]; successSourceIds: FileId[] }> => { + ): Promise<{ + outputFiles: File[]; + successSourceIds: FileId[]; + failedInputs: FailedInput[]; + }> => { const processedFiles: File[] = []; const successSourceIds: FileId[] = []; + // Kept with their errors: a batch where only some inputs fail still owes the caller a + // report for each one, and it cannot derive the kind without the error. + const failedInputs: FailedInput[] = []; const failedFiles: string[] = []; const total = validFiles.length; @@ -89,6 +103,11 @@ export const useToolApiCalls = () => { name: file.name, }); failedFiles.push(file.name); + failedInputs.push({ + fileId: file.fileId, + name: file.name, + error: new Error(`${endpoint} returned an empty output`), + }); try { markFileError?.(file.fileId); } catch (e) { @@ -109,6 +128,7 @@ export const useToolApiCalls = () => { } console.error("[processFiles] Failed", { name: file.name, error }); failedFiles.push(file.name); + failedInputs.push({ fileId: file.fileId, name: file.name, error }); // mark errored file so UI can highlight try { markFileError?.(file.fileId); @@ -140,7 +160,11 @@ export const useToolApiCalls = () => { outputs: processedFiles.length, failed: failedFiles.length, }); - return { outputFiles: processedFiles, successSourceIds }; + return { + outputFiles: processedFiles, + successSourceIds, + failedInputs, + }; }, [], ); diff --git a/frontend/editor/src/core/hooks/tools/shared/useToolOperation.ts b/frontend/editor/src/core/hooks/tools/shared/useToolOperation.ts index 28f58664f5..295f6c9084 100644 --- a/frontend/editor/src/core/hooks/tools/shared/useToolOperation.ts +++ b/frontend/editor/src/core/hooks/tools/shared/useToolOperation.ts @@ -138,6 +138,49 @@ export const useToolOperation = ( outputFileIds: FileId[]; } | null>(null); + /** + * Record a failure and keep what a retry needs. Shared with the batch's per-input failures: + * one bad file in twenty is still a failure the user has to be told about. + */ + const reportFailure = useCallback( + ( + error: unknown, + fileIds: FileId[], + runtimeEndpoint: string | undefined, + params: TParams, + ) => { + if (fileIds.length === 0 || wasCancelled(error)) return; + + void reportToolFailure({ + operation: config.operationType, + error, + fileIds, + }).then(refreshNotificationsNow); + + // Skipped where nothing could use it: a custom processor's request cannot be replayed + // generically, and a build with no bell has nothing to read the stash. + if ( + !runtimeEndpoint || + config.toolType === ToolType.custom || + !notificationsAvailable + ) { + return; + } + void errorCodeOf(error).then((errorCode) => + stashRetryPayload({ + operation: config.operationType, + endpoint: runtimeEndpoint, + params: params as Record, + fileIds, + multiFile: config.toolType === ToolType.multiFile, + errorCode, + recordedAt: Date.now(), + }), + ); + }, + [config.operationType, config.toolType, notificationsAvailable], + ); + const executeOperation = useCallback( async (params: TParams, selectedFiles: StirlingFile[]): Promise => { // Validation @@ -268,9 +311,20 @@ export const useToolOperation = ( ); processedFiles = result.outputFiles; successSourceIds = result.successSourceIds; + // Reported here, not in the catch: this loop only throws when EVERY input failed, + // so a batch that lost one file to a bad PDF reaches the success path. + for (const failed of result.failedInputs) { + reportFailure( + failed.error, + [failed.fileId], + runtimeEndpoint, + params, + ); + } console.debug("[useToolOperation] Multi-file results", { outputFiles: processedFiles.length, successSources: result.successSourceIds.length, + failedInputs: result.failedInputs.length, }); break; } @@ -633,35 +687,13 @@ export const useToolOperation = ( void _e; } - // Report it so a leader sees the failure too, then carry on with the user's - // own error handling. Fire-and-forget: the reporter swallows its own errors. - // Chained, not fired alongside: the re-read must happen after the row exists. - void reportToolFailure({ - operation: config.operationType, + // The whole run failed, so every input is a casualty. + reportFailure( error, - fileIds: validFiles.map((file) => file.fileId), - }).then(refreshNotificationsNow); - - // Keep what a retry needs: the report carries none of it. Skipped where nothing - // could use it, and gated on the reporter's own cancellation test. - if ( - !wasCancelled(error) && - runtimeEndpoint && - config.toolType !== ToolType.custom && - notificationsAvailable - ) { - void errorCodeOf(error).then((errorCode) => - stashRetryPayload({ - operation: config.operationType, - endpoint: runtimeEndpoint, - params: params as Record, - fileIds: validFiles.map((file) => file.fileId), - multiFile: config.toolType === ToolType.multiFile, - errorCode, - recordedAt: Date.now(), - }), - ); - } + validFiles.map((file) => file.fileId), + runtimeEndpoint, + params, + ); const errorMessage = config.getErrorMessage?.(error) || extractErrorMessage(error); @@ -689,6 +721,7 @@ export const useToolOperation = ( checkCredits, continueResolutions, notificationsAvailable, + reportFailure, ], ); diff --git a/frontend/editor/src/core/services/failureReporting.ts b/frontend/editor/src/core/services/failureReporting.ts index 664513e98f..52f2aa891f 100644 --- a/frontend/editor/src/core/services/failureReporting.ts +++ b/frontend/editor/src/core/services/failureReporting.ts @@ -158,15 +158,8 @@ function messageOf(error: unknown): string { } /** - * A user cancelling is the one failure worth dropping: nothing went wrong and there - * is nothing for a reviewer to do. `useToolApiCalls` rethrows an axios cancellation - * as a plain Error with the original as its cause, so both shapes are checked. - * - *

Everything else is reported, client-side refusals included: an unsupported input - * format is the same class of problem as the processor rejecting a file type, which - * is already recorded. - * - *

Exported so a run the user cancelled does not get a retry stashed for it. + * A user cancelling is the one failure worth dropping. Both shapes are checked, since + * `useToolApiCalls` rethrows an axios cancellation as an Error with the original as its cause. */ export function wasCancelled(error: unknown): boolean { const candidate = error as { diff --git a/frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx b/frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx index 9cdf82f8eb..9793a280bf 100644 --- a/frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx +++ b/frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx @@ -637,33 +637,8 @@ describe("useNotificationActions", () => { expect(navigate).toHaveBeenCalledWith("/processor/documents#failures"); }); - it("opens a new tab rather than costing the reader a loaded workbench", () => { - // Navigating away would unload their files, costing them every upload again. + it("navigates in place even with a loaded workbench, never opening a tab", () => { openFileIds = ["f-1"]; - const openTab = vi.spyOn(window, "open").mockReturnValue({} as Window); - - registry().VIEW_IN_PROCESSOR?.run(context()); - - expect(openTab).toHaveBeenCalledWith( - "/processor/documents#failures", - "_blank", - "noopener", - ); - expect(navigate).not.toHaveBeenCalled(); - openTab.mockRestore(); - }); - - it("navigates in place when the tab would be refused", () => { - openFileIds = ["f-1"]; - const openTab = vi.spyOn(window, "open").mockReturnValue(null); - - registry().VIEW_IN_PROCESSOR?.run(context()); - - expect(navigate).toHaveBeenCalledWith("/processor/documents#failures"); - openTab.mockRestore(); - }); - - it("navigates in place from an empty workbench, which costs the reader nothing", () => { const openTab = vi.spyOn(window, "open"); registry().VIEW_IN_PROCESSOR?.run(context()); @@ -673,7 +648,7 @@ describe("useNotificationActions", () => { openTab.mockRestore(); }); - it("navigates in place from the processor, which has no workbench to lose", () => { + it("navigates in place from the processor too", () => { const openTab = vi.spyOn(window, "open"); registry(inProcessor).VIEW_IN_PROCESSOR?.run(context()); diff --git a/frontend/editor/src/proprietary/components/notifications/notificationActions.ts b/frontend/editor/src/proprietary/components/notifications/notificationActions.ts index b8307690da..5c3ddcab92 100644 --- a/frontend/editor/src/proprietary/components/notifications/notificationActions.ts +++ b/frontend/editor/src/proprietary/components/notifications/notificationActions.ts @@ -97,11 +97,6 @@ function takeSelection(): Handoff | null { } } -/** False when the browser refused it, so the caller can fall back to navigating in place. */ -function openInNewTab(path: string): boolean { - return window.open(withBasePath(path), "_blank", "noopener") !== null; -} - /** Not the router's `navigate`: the editor reads its tool on mount and on a history pop. */ function goToEditor(path: string): void { window.history.pushState({}, "", withBasePath(path)); @@ -458,14 +453,7 @@ export function useNotificationActions(): ClientActionRegistry { // Dev-only until failures get a review screen; portal/views/Documents holds the other half. available: () => import.meta.env.DEV, closesPanel: true, - run: () => { - // Leaving would cost them a loaded workbench, and every file in it a re-upload. - const holdsFiles = (fileStore?.getState().files.ids.length ?? 0) > 0; - if (canOpenHere && holdsFiles && openInNewTab(FAILURES_DESTINATION)) { - return; - } - navigate(FAILURES_DESTINATION); - }, + run: () => navigate(FAILURES_DESTINATION), }; return { diff --git a/frontend/editor/src/proprietary/components/policies/policyRunStore.test.ts b/frontend/editor/src/proprietary/components/policies/policyRunStore.test.ts index 3ee9057b3e..72c3e33b21 100644 --- a/frontend/editor/src/proprietary/components/policies/policyRunStore.test.ts +++ b/frontend/editor/src/proprietary/components/policies/policyRunStore.test.ts @@ -1,8 +1,10 @@ import { describe, it, expect, beforeEach } from "vitest"; import { + appliedCategoriesFor, dispatchKey, getRun, isDispatched, + localPassFailed, markDispatched, recordRunStart, removeRun, @@ -106,6 +108,119 @@ describe("policyRunStore", () => { expect(isDispatched("classification", "f1")).toBe(true); }); + describe("localPassFailed", () => { + it("reports a browser-local pass that could not produce a verdict", () => { + recordRunStart( + rec({ + runId: "local-c", + categoryId: "classification", + fileId: "f1", + target: "local", + browserLocal: true, + }), + ); + updateRun("local-c", { status: "FAILED", error: "encrypted" }); + + expect(localPassFailed("classification", "f1")).toBe(true); + expect(localPassFailed("classification", "f2")).toBe(false); + }); + + it("is false while the pass is still running, so the gate keeps waiting", () => { + recordRunStart( + rec({ + runId: "local-c", + categoryId: "classification", + fileId: "f1", + target: "local", + browserLocal: true, + status: "RUNNING", + }), + ); + + expect(localPassFailed("classification", "f1")).toBe(false); + }); + + it("ignores a failed SERVER run, which says nothing about the local pass", () => { + recordRunStart( + rec({ runId: "srv-1", categoryId: "classification", fileId: "f1" }), + ); + updateRun("srv-1", { status: "FAILED", error: "boom" }); + + expect(localPassFailed("classification", "f1")).toBe(false); + }); + }); + + describe("appliedCategoriesFor", () => { + it("walks a rewriting chain back to the uploaded document", () => { + recordRunStart( + rec({ runId: "r-w", categoryId: "watermark", fileId: "f1" }), + ); + updateRun("r-w", { status: "COMPLETED", outputFileIds: ["f2"] }); + recordRunStart( + rec({ runId: "r-s", categoryId: "security", fileId: "f2" }), + ); + updateRun("r-s", { status: "COMPLETED", outputFileIds: ["f3"] }); + + expect([...appliedCategoriesFor("f3")].sort()).toEqual([ + "security", + "watermark", + ]); + }); + + it("counts an annotating run, which names its input as its own output", () => { + recordRunStart( + rec({ runId: "r-c", categoryId: "classification", fileId: "f1" }), + ); + updateRun("r-c", { status: "COMPLETED", outputFileIds: ["f1"] }); + + expect([...appliedCategoriesFor("f1")]).toEqual(["classification"]); + }); + + it("does not count a browser-local pass as the policy having run", () => { + // The local heuristic settles COMPLETED with the input as its own output. Counting it + // would make a retry skip classification, killing the escalation #7667 restored. + recordRunStart( + rec({ + runId: "local-c", + categoryId: "classification", + fileId: "f1", + target: "local", + browserLocal: true, + }), + ); + updateRun("local-c", { status: "COMPLETED", outputFileIds: ["f1"] }); + + expect(appliedCategoriesFor("f1").size).toBe(0); + }); + + it("keeps climbing past an annotating run rather than stalling on it", () => { + // The annotating run's output IS its input, so the walk must not treat that as a + // lineage step - otherwise the cursor never moves and earlier policies are missed. + recordRunStart( + rec({ runId: "r-w", categoryId: "watermark", fileId: "f1" }), + ); + updateRun("r-w", { status: "COMPLETED", outputFileIds: ["f2"] }); + recordRunStart( + rec({ runId: "r-c", categoryId: "classification", fileId: "f2" }), + ); + updateRun("r-c", { status: "COMPLETED", outputFileIds: ["f2"] }); + + expect([...appliedCategoriesFor("f2")].sort()).toEqual([ + "classification", + "watermark", + ]); + }); + + it("ignores a run that failed, so it stays eligible to run again", () => { + recordRunStart( + rec({ runId: "r-f", categoryId: "security", fileId: "f1" }), + ); + updateRun("r-f", { status: "FAILED", outputFileIds: ["f2"] }); + + expect(appliedCategoriesFor("f2").size).toBe(0); + }); + }); + it("never evicts in-flight runs, even past the soft cap", () => { // A large upload batch can exceed the cap while still processing. Dropping a // live run would orphan its polling/import and undercount progress, so every diff --git a/frontend/editor/src/proprietary/components/policies/policyRunStore.ts b/frontend/editor/src/proprietary/components/policies/policyRunStore.ts index 0f16cf34bd..bb77f21b21 100644 --- a/frontend/editor/src/proprietary/components/policies/policyRunStore.ts +++ b/frontend/editor/src/proprietary/components/policies/policyRunStore.ts @@ -215,20 +215,34 @@ export function isDispatched(categoryId: string, fileId: string): boolean { return state.dispatched.includes(dispatchKey(categoryId, fileId)); } -/** Walked back through this document's lineage. Only a COMPLETED run counts as applied. */ +/** True when the browser's own pass ran for this file and could not produce a verdict. */ +export function localPassFailed(categoryId: string, fileId: string): boolean { + return state.runs.some( + (run) => + run.browserLocal === true && + run.status === "FAILED" && + run.categoryId === categoryId && + run.fileId === fileId, + ); +} + +/** Walked back through this document's lineage. Only a COMPLETED server run counts as applied. */ export function appliedCategoriesFor(fileId: string): Set { const applied = new Set(); - let cursor = fileId; + let cursor: string | null = fileId; // A lineage cannot outrun the recorded runs, and the bound also breaks a hand-edited cycle. for (let step = 0; step < state.runs.length; step++) { - const child = cursor; - const producer = state.runs.find( - (run) => - run.status === "COMPLETED" && (run.outputFileIds ?? []).includes(child), - ); - if (!producer) break; - applied.add(producer.categoryId); - cursor = producer.fileId; + if (cursor === null) break; + const child: string = cursor; + cursor = null; + for (const run of state.runs) { + // A local first pass is not the policy's run: counting it would skip the escalation. + if (run.status !== "COMPLETED" || run.browserLocal) continue; + if (!(run.outputFileIds ?? []).includes(child)) continue; + applied.add(run.categoryId); + // An annotating run names its input as its own output, so it adds no lineage step. + if (run.fileId !== child) cursor = run.fileId; + } } return applied; } diff --git a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts index ac1b468be5..68a70cfd8c 100644 --- a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts +++ b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts @@ -49,6 +49,7 @@ import { dispatchKey, getRun, isDispatched, + localPassFailed, markDispatched, recordRunStart, removeRun, @@ -252,8 +253,17 @@ export function usePolicyAutoRun(): void { ) { continue; } - // A confident local verdict stands; only an unsure one is escalated to the engine. - if (!shouldDispatchToAi(firstCategory, stub)) continue; + // A confident local verdict stands; only an unsure one is escalated to the engine. A pass + // that threw counts as unsure: an unreadable file will never report one of its own. + if ( + !shouldDispatchToAi( + firstCategory, + stub, + localPassFailed(firstCategory, stub.id), + ) + ) { + continue; + } dispatching.current.add(key); void runPolicyOnFile(firstCategory, backendId, stub.id, stub.name) .catch(() => { diff --git a/frontend/editor/src/proprietary/data/classificationPolicy.test.ts b/frontend/editor/src/proprietary/data/classificationPolicy.test.ts index cb5dfec779..88b842684a 100644 --- a/frontend/editor/src/proprietary/data/classificationPolicy.test.ts +++ b/frontend/editor/src/proprietary/data/classificationPolicy.test.ts @@ -97,6 +97,18 @@ describe("shouldDispatchToAi", () => { expect(shouldDispatchToAi("classification", stub("none"))).toBe(true); }); + it("escalates an upload whose local pass threw, rather than waiting forever", () => { + // An encrypted document never reports a local verdict, so waiting means the server run + // never dispatches, nothing records the failure, and the bell stays empty. + expect(shouldDispatchToAi("classification", stub(), true)).toBe(true); + }); + + it("still lets a confident verdict stand even if an earlier pass had thrown", () => { + expect(shouldDispatchToAi("classification", stub("high"), true)).toBe( + false, + ); + }); + it("escalates a tool-derived file with no verdict at all", () => { // A derived file gets no local pass (useClientSideClassification skips it), so // there is no verdict to wait for: holding back would skip it forever. This is diff --git a/frontend/editor/src/proprietary/data/classificationPolicy.ts b/frontend/editor/src/proprietary/data/classificationPolicy.ts index 60ef0de23f..c51c5abdda 100644 --- a/frontend/editor/src/proprietary/data/classificationPolicy.ts +++ b/frontend/editor/src/proprietary/data/classificationPolicy.ts @@ -56,18 +56,17 @@ export function orderRewritesFirst(categoryIds: string[]): string[] { const TRUSTED_CONFIDENCE: ClassificationConfidence = "high"; /** - * Whether the AI classifier should be asked about this file. For an upload, only once the - * heuristic has reported: dispatching before then races the first pass and bills for an answer it - * was about to produce. A tool-derived file gets no local pass (useClientSideClassification skips - * it) and only ever carries an inherited verdict, so an absent verdict there is permanent - - * escalate rather than wait for a report that will never come. + * Whether to ask the AI classifier. An upload waits for the local pass, since dispatching races it + * and bills for a free answer; a derived file or a failed pass waits forever, so both escalate. */ export function shouldDispatchToAi( categoryId: string, stub: StirlingFileStub, + localPassFailed = false, ): boolean { if (!isClassificationCategory(categoryId)) return true; const confidence = stub.classificationConfidence; - if (confidence == null) return Boolean(stub.derivedFromTool); + if (confidence == null) + return Boolean(stub.derivedFromTool) || localPassFailed; return confidence !== TRUSTED_CONFIDENCE; }