diff --git a/frontend/editor/src/core/contexts/file/FileReducer.test.ts b/frontend/editor/src/core/contexts/file/FileReducer.test.ts index bde6a17a02..cdba68b013 100644 --- a/frontend/editor/src/core/contexts/file/FileReducer.test.ts +++ b/frontend/editor/src/core/contexts/file/FileReducer.test.ts @@ -209,6 +209,54 @@ describe("fileContextReducer — silent CONSUME_FILES (background enforcement)", ]); }); + it("carries classificationConfidence forward with the labels", () => { + // The confidence is part of the verdict: without it the escalation decision + // (shouldDispatchToAi) dies at the version boundary and a chained + // classification never runs. + const start = stateWith([ + stub("a", { + classificationLabels: ["Invoice"], + classificationConfidence: "low", + }), + ]); + const next = fileContextReducer(start, { + type: "CONSUME_FILES", + payload: { + inputFileIds: ["a" as FileId], + outputStirlingFileStubs: [stub("a-v2")], + }, + }); + expect(next.files.byId["a-v2" as FileId].classificationConfidence).toBe( + "low", + ); + }); + + it("an output with its own verdict keeps it — no confidence bleed from the input", () => { + // A fresh classify result carries its own labels; stamping the input's + // heuristic confidence onto them would mislabel an AI verdict as unsure. + const start = stateWith([ + stub("a", { + classificationLabels: ["Invoice"], + classificationConfidence: "low", + }), + ]); + const next = fileContextReducer(start, { + type: "CONSUME_FILES", + payload: { + inputFileIds: ["a" as FileId], + outputStirlingFileStubs: [ + stub("b", { classificationLabels: ["Contract"] }), + ], + }, + }); + expect(next.files.byId["b" as FileId].classificationLabels).toEqual([ + "Contract", + ]); + expect( + next.files.byId["b" as FileId].classificationConfidence, + ).toBeUndefined(); + }); + it("non-silent CONSUME_FILES still moves the output to the front (unchanged)", () => { const start = stateWith([stub("a"), stub("b")]); const next = fileContextReducer(start, { diff --git a/frontend/editor/src/core/contexts/file/FileReducer.ts b/frontend/editor/src/core/contexts/file/FileReducer.ts index a9dcae7b38..1ed23b11d6 100644 --- a/frontend/editor/src/core/contexts/file/FileReducer.ts +++ b/frontend/editor/src/core/contexts/file/FileReducer.ts @@ -386,14 +386,19 @@ export function fileContextReducer( ), ); - // Carry the document's classification labels forward across the edit: any + // Carry the document's classification verdict forward across the edit: any // tool that versions/derives a classified file keeps it in its label // groups instead of dropping to "Other" and waiting on a PDF re-read. - // Inherited from the first input that has any; an output that already - // carries its own (e.g. a fresh classify result) keeps them. - const inheritedLabels = inputFileIds - .map((id) => state.files.byId[id]?.classificationLabels) - .find((labels) => labels && labels.length > 0); + // Inherited from the first input that has labels, together with that + // verdict's confidence - the escalation decision (shouldDispatchToAi) is + // about the document, not about which step produced the current bytes, so + // it must survive the version boundary. An output that already carries its + // own verdict (e.g. a fresh classify result) keeps it. + const verdictDonor = inputFileIds + .map((id) => state.files.byId[id]) + .find( + (s) => s?.classificationLabels && s.classificationLabels.length > 0, + ); // Mark every consume output as tool-produced (the single chokepoint for // both versioned edits and independent artifacts like convert/split/merge) @@ -404,7 +409,14 @@ export function fileContextReducer( ...stub, derivedFromTool: true, sourceFileIds, - classificationLabels: stub.classificationLabels ?? inheritedLabels, + ...(stub.classificationLabels == null && verdictDonor + ? { + classificationLabels: verdictDonor.classificationLabels, + classificationConfidence: + stub.classificationConfidence ?? + verdictDonor.classificationConfidence, + } + : {}), })); // Silent (background enforcement): replace inputs in their existing grid diff --git a/frontend/editor/src/proprietary/components/policies/policyRunStore.test.ts b/frontend/editor/src/proprietary/components/policies/policyRunStore.test.ts index 498a688ac2..3ee9057b3e 100644 --- a/frontend/editor/src/proprietary/components/policies/policyRunStore.test.ts +++ b/frontend/editor/src/proprietary/components/policies/policyRunStore.test.ts @@ -83,6 +83,29 @@ describe("policyRunStore", () => { expect(isDispatched("security", "f1")).toBe(true); }); + it("a browser-local run does not claim the (policy, file) dispatch key", () => { + // The local classification heuristic records a run for the same (classification, file) pair + // the server escalation is keyed on. If that claimed the key, the auto-run would read + // "already dispatched" and never ask the AI - which killed escalation entirely. + recordRunStart( + rec({ + runId: "local-classification-f1-1", + categoryId: "classification", + fileId: "f1", + target: "local", + browserLocal: true, + status: "RUNNING", + }), + ); + expect(getRun("local-classification-f1-1")).toBeDefined(); + expect(isDispatched("classification", "f1")).toBe(false); + }); + + it("a real backend run still claims the dispatch key", () => { + recordRunStart(rec({ runId: "srv-1", categoryId: "classification" })); + expect(isDispatched("classification", "f1")).toBe(true); + }); + 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 2fd91d215a..a598b8b9b1 100644 --- a/frontend/editor/src/proprietary/components/policies/policyRunStore.ts +++ b/frontend/editor/src/proprietary/components/policies/policyRunStore.ts @@ -47,6 +47,14 @@ export interface PolicyRunRecord { retrying?: boolean; /** Epoch ms when the run was dispatched. */ startedAt: number; + /** + * Ran in the browser (the local classification heuristic), not on a backend. Such a run has no + * server-side status to poll, and - crucially - must NOT claim the (policy, file) dispatch key: + * it is the first pass, not the policy's run, so claiming it would suppress the server run the + * verdict may still need to escalate to. Distinct from {@link target}, which says which BACKEND + * holds a real run's outputs. + */ + browserLocal?: boolean; } /** Statuses of a run that is still executing (not yet settled). */ @@ -216,11 +224,15 @@ export function recordRunStart(record: PolicyRunRecord) { const waveStartedAt = state.runs.some(isRunInFlight) ? state.waveStartedAt : record.startedAt; + // A browser-local run is the first pass, not the policy's run: claiming the dispatch key here + // would permanently suppress the server run its verdict may still need to escalate to. + const claimsDispatch = !record.browserLocal; state = { runs: capRuns([record, ...state.runs]), - dispatched: state.dispatched.includes(key) - ? state.dispatched - : [...state.dispatched, key], + dispatched: + !claimsDispatch || state.dispatched.includes(key) + ? state.dispatched + : [...state.dispatched, key], waveStartedAt, }; emit(); diff --git a/frontend/editor/src/proprietary/components/policies/useClientSideClassification.test.tsx b/frontend/editor/src/proprietary/components/policies/useClientSideClassification.test.tsx index d399b09553..2149161f59 100644 --- a/frontend/editor/src/proprietary/components/policies/useClientSideClassification.test.tsx +++ b/frontend/editor/src/proprietary/components/policies/useClientSideClassification.test.tsx @@ -77,7 +77,10 @@ vi.mock("@app/services/classificationMeter", () => ({ meterClassificationRun: (payload: unknown) => mocks.meter(payload), })); -import { useClientSideClassification } from "@app/components/policies/useClientSideClassification"; +import { + useClientSideClassification, + LOCAL_METER_CATEGORY, +} from "@app/components/policies/useClientSideClassification"; // Run idle callbacks immediately so batches start without timer waits. vi.stubGlobal("requestIdleCallback", (cb: () => void) => { @@ -181,8 +184,9 @@ describe("useClientSideClassification delivery", () => { }); it("heals a previously-dispatched file whose result was lost, without re-metering", async () => { - // A past session classified + metered this file but the delivery was lost. - markDispatched("classification", "lost"); + // A past session classified + metered this file but the delivery was lost. The marker is the + // local-meter key, NOT the classification dispatch key - that one belongs to the server run. + markDispatched(LOCAL_METER_CATEGORY, "lost"); mocks.workspace = [stub("lost")]; mocks.classify.mockResolvedValue({ labels: ["bank-statement"] }); diff --git a/frontend/editor/src/proprietary/components/policies/useClientSideClassification.ts b/frontend/editor/src/proprietary/components/policies/useClientSideClassification.ts index ede1768b2a..0ae9238b12 100644 --- a/frontend/editor/src/proprietary/components/policies/useClientSideClassification.ts +++ b/frontend/editor/src/proprietary/components/policies/useClientSideClassification.ts @@ -22,6 +22,12 @@ import type { StirlingFile, StirlingFileStub } from "@app/types/fileContext"; import type { HeuristicConfidence } from "@app/services/heuristic/types"; import { CLASSIFICATION_CATEGORY_ID } from "@app/data/classificationPolicy"; +/** + * Dispatch-store key namespace for "this file's local pass has been metered". Deliberately NOT the + * Classification category id: that key is the server escalation's own guard, so metering under it + * would tell the auto-run the policy had already run and kill the escalation entirely. + */ +export const LOCAL_METER_CATEGORY = `${CLASSIFICATION_CATEGORY_ID}:local-meter`; /** Files classified per idle pass, so a large library drains over several ticks. */ const CLASSIFY_BATCH = 3; /** How long to wait for an upload's bytes to land in IndexedDB (20 × 250ms ≈ 5s). @@ -161,9 +167,7 @@ async function classifyStub( // A local run is still a billable policy run, so it belongs in the activity feed; recorded only // once the bytes are in hand, so a file whose bytes never land leaves no phantom row. - // Read before recordRunStart, which takes the dispatch key itself and would otherwise always - // answer "already dispatched", silently stopping metering. - const alreadyMetered = isDispatched(CLASSIFICATION_CATEGORY_ID, fileId); + const alreadyMetered = isDispatched(LOCAL_METER_CATEGORY, fileId); const runId = `local-${CLASSIFICATION_CATEGORY_ID}-${fileId}-${Date.now()}`; recordRunStart({ runId, @@ -172,6 +176,9 @@ async function classifyStub( fileName, fileSize, target: "local", + // Ran here, not on a backend: nothing to poll, and it must not claim the classification + // dispatch key - that key is what the server escalation checks before running. + browserLocal: true, status: "RUNNING", outputs: [], error: null, @@ -201,7 +208,7 @@ async function classifyStub( labels, }); } - markDispatched(CLASSIFICATION_CATEGORY_ID, fileId); + markDispatched(LOCAL_METER_CATEGORY, fileId); // Labels, no output file - the same settle shape the server-run classification uses. updateRun(runId, { status: "COMPLETED", diff --git a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.chain.test.tsx b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.chain.test.tsx index a6f78392be..a03d55f757 100644 --- a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.chain.test.tsx +++ b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.chain.test.tsx @@ -10,7 +10,13 @@ const aiEnabled = vi.hoisted(() => ({ value: true })); vi.mock("@app/hooks/useAiEngineEnabled", () => ({ useAiEngineEnabled: () => aiEnabled.value, })); -const fileStubs: { id: string; name: string; derivedFromTool?: boolean }[] = []; +const fileStubs: { + id: string; + name: string; + derivedFromTool?: boolean; + classificationLabels?: string[]; + classificationConfidence?: "none" | "low" | "medium" | "high"; +}[] = []; vi.mock("@app/contexts/FileContext", () => ({ useAllFiles: () => ({ fileStubs }), useFileManagement: () => ({ addFiles: vi.fn() }), @@ -55,10 +61,11 @@ import { updateRun, resetPolicyRuns, } from "@app/components/policies/policyRunStore"; -import { runStoredPolicy } from "@app/services/policyApi"; +import { runStoredPolicy, getPolicyRun } from "@app/services/policyApi"; import { fileStorage } from "@app/services/fileStorage"; const runStored = vi.mocked(runStoredPolicy); +const getPolicyRunMock = vi.mocked(getPolicyRun); const getFile = vi.mocked(fileStorage.getStirlingFile); /** Reset the shared file list between tests without swapping the array identity. */ @@ -67,6 +74,27 @@ function setFileStubs(next: typeof fileStubs) { fileStubs.push(...next); } +/** A completed security run whose imported output is file-1-v2, ready to chain from. */ +function seedCompletedSecurityRun() { + recordRunStart({ + runId: "run-sec", + categoryId: "security", + fileId: "file-1", + fileName: "doc.pdf", + fileSize: 100, + target: "saas", + status: "PENDING", + outputs: [], + error: null, + startedAt: 0, + }); + updateRun("run-sec", { + status: "COMPLETED", + imported: true, + outputFileIds: ["file-1-v2"], + }); +} + beforeEach(() => { vi.useFakeTimers(); localStorage.clear(); @@ -133,6 +161,154 @@ describe("auto-run ordered chaining", () => { ); }); + it("escalates a chained output that carries no verdict", async () => { + // The output stub is in the workspace shaped as a new_file-mode delivery (or a + // version made before the upload's verdict landed) produces it: tool-derived, + // labels inherited, NO classificationConfidence. No local pass ever runs on a + // derived file, so waiting for a verdict would skip classification forever — + // it must dispatch to the engine instead. + seedCompletedSecurityRun(); + setFileStubs([ + { + id: "file-1-v2", + name: "doc.pdf", + derivedFromTool: true, + classificationLabels: ["invoice"], + }, + ]); + runStored.mockResolvedValue("run-cls"); + + renderHook(() => usePolicyAutoRun()); + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); + + expect(runStored).toHaveBeenCalledWith( + "backend-cls", + [{ size: 100 }], + "file-1-v2", + ); + }); + + it("chains classification onto an output that inherited an unsure verdict", async () => { + // The default (new_version) delivery: createChildStub copies the parent's + // verdict onto the output, so a low confidence rides through and escalates. + seedCompletedSecurityRun(); + setFileStubs([ + { + id: "file-1-v2", + name: "doc.pdf", + derivedFromTool: true, + classificationLabels: ["invoice"], + classificationConfidence: "low", + }, + ]); + runStored.mockResolvedValue("run-cls"); + + renderHook(() => usePolicyAutoRun()); + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); + + expect(runStored).toHaveBeenCalledWith( + "backend-cls", + [{ size: 100 }], + "file-1-v2", + ); + }); + + it("lets an inherited confident verdict stand — no engine call for the chained output", async () => { + seedCompletedSecurityRun(); + setFileStubs([ + { + id: "file-1-v2", + name: "doc.pdf", + derivedFromTool: true, + classificationLabels: ["invoice"], + classificationConfidence: "high", + }, + ]); + runStored.mockResolvedValue("run-cls"); + + renderHook(() => usePolicyAutoRun()); + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); + + expect( + runStored.mock.calls.some(([backendId]) => backendId === "backend-cls"), + ).toBe(false); + }); + + it("still escalates after the local pass has recorded its own run for the file", async () => { + // The regression that made the whole escalation dead in practice: the local heuristic records + // a run for the SAME (classification, file) pair, and recordRunStart claims the dispatch key. + // The auto-run then reads "already dispatched" and skips the server run forever. A + // browser-local run must not claim that key - it is the first pass, not the policy's run. + seedCompletedSecurityRun(); + // The local pass ran on the chained output and recorded its own run for it. + recordRunStart({ + runId: "local-classification-file-1-v2-123", + categoryId: "classification", + fileId: "file-1-v2", + fileName: "doc.pdf", + fileSize: 100, + target: "local", + browserLocal: true, + status: "COMPLETED", + outputs: [], + error: null, + startedAt: 0, + }); + // Its verdict was unsure, so the AI must still be asked. + setFileStubs([ + { + id: "file-1-v2", + name: "doc.pdf", + derivedFromTool: true, + classificationConfidence: "low", + }, + ]); + runStored.mockResolvedValue("run-cls"); + + renderHook(() => usePolicyAutoRun()); + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); + + expect(runStored).toHaveBeenCalledWith( + "backend-cls", + [{ size: 100 }], + "file-1-v2", + ); + }); + + it("does not poll a browser-local run against the server", async () => { + // There is no server-side run to ask about: polling 404s, and MAX_NOT_FOUND consecutive + // misses would mark a local run that actually succeeded as FAILED. + recordRunStart({ + runId: "local-classification-file-9-456", + categoryId: "classification", + fileId: "file-9", + fileName: "doc.pdf", + fileSize: 100, + target: "local", + browserLocal: true, + status: "RUNNING", + outputs: [], + error: null, + startedAt: 0, + }); + setFileStubs([]); + + renderHook(() => usePolicyAutoRun()); + await act(async () => { + await vi.advanceTimersByTimeAsync(3000); + }); + + expect(getPolicyRunMock).not.toHaveBeenCalled(); + }); + it("keeps classification out of the server chain when the AI engine is off", async () => { // AI off: classification runs client-side (useClientSideClassification), so the // server chain must skip it - only the normal (security) policy dispatches. @@ -150,11 +326,9 @@ describe("auto-run ordered chaining", () => { [{ size: 100 }], "file-1", ); - expect(runStored).not.toHaveBeenCalledWith( - "backend-cls", - expect.anything(), - expect.anything(), - ); + expect( + runStored.mock.calls.some(([backendId]) => backendId === "backend-cls"), + ).toBe(false); }); it("never dispatches on a file marked derivedFromTool", async () => { diff --git a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.escalation.test.tsx b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.escalation.test.tsx new file mode 100644 index 0000000000..d3eec22b5a --- /dev/null +++ b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.escalation.test.tsx @@ -0,0 +1,155 @@ +/** + * The default shipped setup: Classification is the ONLY upload policy, so it dispatches directly + * on the upload rather than through the chain. This is the configuration the escalation was built + * for, and the one where it was completely dead: the browser-side first pass records its own run + * for the same (classification, file) pair, and recordRunStart claims the dispatch key, so the + * auto-run read "already dispatched" and never asked the AI - whatever the verdict said. + * + * Driven against the REAL run store; mocking the store is what let the regression through. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; + +vi.mock("@app/hooks/useAiEngineEnabled", () => ({ + useAiEngineEnabled: () => true, +})); +const fileStubs: { + id: string; + name: string; + derivedFromTool?: boolean; + classificationLabels?: string[]; + classificationConfidence?: "none" | "low" | "medium" | "high"; +}[] = []; +vi.mock("@app/contexts/FileContext", () => ({ + useAllFiles: () => ({ fileStubs }), + useFileManagement: () => ({ addFiles: vi.fn() }), + useFileContext: () => ({ consumeFiles: vi.fn() }), +})); +vi.mock("@app/hooks/usePolicies", () => ({ + usePolicies: () => ({ + policies: { + classification: { + configured: true, + status: "active", + backendId: "backend-cls", + runOn: "upload", + order: 0, + }, + }, + }), +})); +vi.mock("@app/services/policyApi", () => ({ + runStoredPolicy: vi.fn(), + getPolicyRun: vi.fn(), + downloadPolicyOutput: vi.fn(), + resolvePolicyRunTarget: () => "saas", +})); +vi.mock("@app/services/fileStorage", () => ({ + fileStorage: { getStirlingFile: vi.fn(), getStirlingFileStub: vi.fn() }, +})); +vi.mock("@app/contexts/IndexedDBContext", () => ({ + useIndexedDB: () => ({ bumpRevision: vi.fn() }), +})); + +import { usePolicyAutoRun } from "@app/components/policies/usePolicyAutoRun"; +import { + recordRunStart, + resetPolicyRuns, +} from "@app/components/policies/policyRunStore"; +import { runStoredPolicy } from "@app/services/policyApi"; +import { fileStorage } from "@app/services/fileStorage"; + +const runStored = vi.mocked(runStoredPolicy); +const getFile = vi.mocked(fileStorage.getStirlingFile); + +function setFileStubs(next: typeof fileStubs) { + fileStubs.length = 0; + fileStubs.push(...next); +} + +/** + * Exactly what useClientSideClassification does when its heuristic pass finishes: a run row for + * the activity feed, categorised as classification, for the file it just read. + */ +function recordLocalPassFor(fileId: string) { + recordRunStart({ + runId: `local-classification-${fileId}-1`, + categoryId: "classification", + fileId, + fileName: "low-confidence-classification-test.pdf", + fileSize: 1460, + target: "local", + browserLocal: true, + status: "COMPLETED", + outputs: [], + error: null, + startedAt: 0, + }); +} + +beforeEach(() => { + vi.useFakeTimers(); + resetPolicyRuns(); + runStored.mockReset(); + runStored.mockResolvedValue("run-cls"); + getFile.mockReset(); + getFile.mockResolvedValue({ size: 1460 } as never); + setFileStubs([]); +}); +afterEach(() => vi.useRealTimers()); + +async function render() { + renderHook(() => usePolicyAutoRun()); + await act(async () => { + await vi.advanceTimersByTimeAsync(1); + }); +} + +describe("classification escalation (single-policy setup)", () => { + it("asks the AI about an unsure verdict even though the local pass already ran", async () => { + // low-confidence-classification-test.pdf: the heuristic emits labels but only at "low". + recordLocalPassFor("file-1"); + setFileStubs([ + { + id: "file-1", + name: "low-confidence-classification-test.pdf", + classificationLabels: ["contract", "invoice"], + classificationConfidence: "low", + }, + ]); + + await render(); + + expect(runStored).toHaveBeenCalledWith( + "backend-cls", + [{ size: 1460 }], + "file-1", + ); + }); + + it("leaves a confident local verdict alone (no engine call, no charge)", async () => { + recordLocalPassFor("file-2"); + setFileStubs([ + { + id: "file-2", + name: "invoice.pdf", + classificationLabels: ["invoice"], + classificationConfidence: "high", + }, + ]); + + await render(); + + expect(runStored).not.toHaveBeenCalled(); + }); + + it("waits for the verdict rather than racing the local pass", async () => { + // No verdict yet on a plain upload: dispatching now would pay for an answer the free + // first pass is about to produce. The effect re-runs when the verdict lands. + setFileStubs([{ id: "file-3", name: "unknown.pdf" }]); + + await render(); + + expect(runStored).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts index 0638b1432c..fa3fcf13d3 100644 --- a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts +++ b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts @@ -308,8 +308,10 @@ export function usePolicyAutoRun(): void { for (const outputId of outputIds) { if (isDispatched(nextCategory, outputId as FileId)) continue; const outputStub = stubsRef.current.find((s) => s.id === outputId); - // Nothing to escalate: either the heuristic already answered confidently, or it has - // not reported yet and this effect re-runs when the verdict lands. + // The output's inherited verdict decides here and now (no local pass ever runs + // on a derived file, so there is nothing to defer to): a confident one stands, + // anything else - including no verdict at all, e.g. a new_file-mode delivery - + // escalates. A stub not yet in the snapshot falls through to dispatch too. if (outputStub && !shouldDispatchToAi(nextCategory, outputStub)) continue; void runPolicyOnFile( @@ -326,6 +328,9 @@ export function usePolicyAutoRun(): void { // Poll each in-flight run to a terminal state. useEffect(() => { for (const run of runs) { + // A browser-local run has no server-side status: polling it 404s (and after MAX_NOT_FOUND + // marks a run that actually succeeded as failed). Its own pass settles it. + if (run.browserLocal) continue; if (isTerminal(run.status) || polling.current.has(run.runId)) continue; polling.current.add(run.runId); void poll(run.runId, onRunFinished).finally(() => diff --git a/frontend/editor/src/proprietary/data/classificationPolicy.test.ts b/frontend/editor/src/proprietary/data/classificationPolicy.test.ts index d3cb55c043..cb5dfec779 100644 --- a/frontend/editor/src/proprietary/data/classificationPolicy.test.ts +++ b/frontend/editor/src/proprietary/data/classificationPolicy.test.ts @@ -14,6 +14,14 @@ const stub = ( ): StirlingFileStub => ({ classificationConfidence: confidence }) as StirlingFileStub; +const derivedStub = ( + confidence?: StirlingFileStub["classificationConfidence"], +): StirlingFileStub => + ({ + derivedFromTool: true, + classificationConfidence: confidence, + }) as StirlingFileStub; + describe("isClassificationCategory", () => { it("recognises the classification category and nothing else", () => { expect(isClassificationCategory("classification")).toBe(true); @@ -88,4 +96,19 @@ describe("shouldDispatchToAi", () => { expect(shouldDispatchToAi("classification", stub("low"))).toBe(true); expect(shouldDispatchToAi("classification", stub("none"))).toBe(true); }); + + 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 + // the chained case for a new_file-mode output, or a version made before the + // upload's verdict landed. + expect(shouldDispatchToAi("classification", derivedStub())).toBe(true); + }); + + it("lets a derived file's inherited verdict decide like an upload's own", () => { + expect(shouldDispatchToAi("classification", derivedStub("high"))).toBe( + false, + ); + expect(shouldDispatchToAi("classification", derivedStub("low"))).toBe(true); + }); }); diff --git a/frontend/editor/src/proprietary/data/classificationPolicy.ts b/frontend/editor/src/proprietary/data/classificationPolicy.ts index 4327b40d39..60ef0de23f 100644 --- a/frontend/editor/src/proprietary/data/classificationPolicy.ts +++ b/frontend/editor/src/proprietary/data/classificationPolicy.ts @@ -56,8 +56,11 @@ export function orderRewritesFirst(categoryIds: string[]): string[] { const TRUSTED_CONFIDENCE: ClassificationConfidence = "high"; /** - * Whether the AI classifier should be asked about this file. Only once the heuristic has reported: - * dispatching before then races the first pass and bills for an answer it was about to produce. + * 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. */ export function shouldDispatchToAi( categoryId: string, @@ -65,5 +68,6 @@ export function shouldDispatchToAi( ): boolean { if (!isClassificationCategory(categoryId)) return true; const confidence = stub.classificationConfidence; - return confidence != null && confidence !== TRUSTED_CONFIDENCE; + if (confidence == null) return Boolean(stub.derivedFromTool); + return confidence !== TRUSTED_CONFIDENCE; } diff --git a/frontend/editor/src/proprietary/services/heuristic/fixtures/low-confidence-classification.pdf b/frontend/editor/src/proprietary/services/heuristic/fixtures/low-confidence-classification.pdf new file mode 100644 index 0000000000..9f1e9852ea --- /dev/null +++ b/frontend/editor/src/proprietary/services/heuristic/fixtures/low-confidence-classification.pdf @@ -0,0 +1,95 @@ +%PDF-1.4 +1 0 obj +<< /Type /Catalog /Pages 2 0 R >> +endobj +2 0 obj +<< /Type /Pages /Kids [3 0 R] /Count 1 >> +endobj +3 0 obj +<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources << /Font << /F1 5 0 R >> >> /Contents 4 0 R >> +endobj +4 0 obj +<< /Length 782 >> +stream +BT +/F1 20 Tf +72 720 Td +(Summary Document) Tj +ET +BT +/F1 12 Tf +72 680 Td +(The parties hereto acknowledge the position set out below.) Tj +ET +BT +/F1 12 Tf +72 658 Td +(Term and termination provisions apply.) Tj +ET +BT +/F1 12 Tf +72 636 Td +(Hereinafter referred to as the Supplier.) Tj +ET +BT +/F1 12 Tf +72 614 Td +(Amount due: 1,250.00) Tj +ET +BT +/F1 12 Tf +72 592 Td +(Payment terms apply.) Tj +ET +BT +/F1 12 Tf +72 570 Td +(Total payable: 1,250.00) Tj +ET +BT +/F1 12 Tf +72 548 Td +(Balance due: 1,250.00) Tj +ET +BT +/F1 12 Tf +72 526 Td +(This document has been prepared for internal review.) Tj +ET +BT +/F1 12 Tf +72 504 Td +(Please retain a copy for your records.) Tj +ET +BT +/F1 12 Tf +72 482 Td +(Reference: SD-2024-0417.) Tj +ET +BT +/F1 12 Tf +72 460 Td +(Prepared by the operations team on 17 April 2024.) Tj +ET +endstream +endobj +5 0 obj +<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >> +endobj +6 0 obj +<< /Title (Summary Document) /Producer (Stirling-PDF classification test fixture) >> +endobj +xref +0 7 +0000000000 65535 f +0000000009 00000 n +0000000058 00000 n +0000000115 00000 n +0000000241 00000 n +0000001074 00000 n +0000001144 00000 n +trailer +<< /Size 7 /Root 1 0 R /Info 6 0 R >> +startxref +1244 +%%EOF diff --git a/frontend/editor/src/proprietary/services/heuristic/fixtures/lowConfidenceFixture.test.ts b/frontend/editor/src/proprietary/services/heuristic/fixtures/lowConfidenceFixture.test.ts new file mode 100644 index 0000000000..69b07979f1 --- /dev/null +++ b/frontend/editor/src/proprietary/services/heuristic/fixtures/lowConfidenceFixture.test.ts @@ -0,0 +1,60 @@ +/** + * Pins the verdict of `low-confidence-classification.pdf`, the manual-repro fixture for the + * classification escalation. The document is only useful as a repro while the heuristic still + * returns something OTHER than "high" for it - a rules change that made it confident would + * silently turn the manual test into a no-op. + * + * The text below is verbatim pdf.js output for that file, so a failure here means the PDF and + * these expectations have drifted apart - re-extract before changing either. + */ +import { beforeAll, describe, expect, it } from "vitest"; +import { + classifyHeuristic, + ensureRulesLoaded, +} from "@app/services/heuristic/heuristicEngine"; +import type { HeuristicDoc } from "@app/services/heuristic/types"; + +const EXTRACTED = + "Summary Document The parties hereto acknowledge the position set out below. " + + "Term and termination provisions apply. Hereinafter referred to as the Supplier. " + + "Amount due: 1,250.00 Payment terms apply. Total payable: 1,250.00 " + + "Balance due: 1,250.00 This document has been prepared for internal review. " + + "Please retain a copy for your records. Reference: SD-2024-0417. " + + "Prepared by the operations team on 17 April 2024."; + +beforeAll(async () => { + await ensureRulesLoaded(); +}); + +describe("low-confidence-classification.pdf fixture", () => { + const doc: HeuristicDoc = { + fileName: "low-confidence-classification.pdf", + pageCount: 1, + meta: { Title: "Summary Document" }, + titleZone: "Summary Document", + firstZone: EXTRACTED, + allZone: EXTRACTED, + }; + + it("is English, so it is not rejected before scoring", () => { + expect(classifyHeuristic(doc).isEnglish).toBe(true); + }); + + it("emits labels but is not trusted, so it must escalate", () => { + const r = classifyHeuristic(doc); + expect(r.labels.length).toBeGreaterThan(0); + expect(r.confidence).not.toBe("high"); + }); + + it("stays unsure because two document types score within the medium margin", () => { + // The margin is what holds this document at "low": "medium" needs >= 8 and "high" >= 15, + // so a near-tie can't be promoted however high the raw scores go. + const r = classifyHeuristic(doc, { explain: true }); + const [first, second] = r.explain?.candidates ?? []; + expect(first).toBeDefined(); + expect(second).toBeDefined(); + expect(first.score - second.score).toBeLessThan(8); + // ...and comfortably clear of the floor, so it doesn't collapse to "no label" either. + expect(first.score).toBeGreaterThanOrEqual(28); + }); +});