diff --git a/.changeset/evals-provider-error-attribution.md b/.changeset/evals-provider-error-attribution.md new file mode 100644 index 0000000000..fd455f172d --- /dev/null +++ b/.changeset/evals-provider-error-attribution.md @@ -0,0 +1,24 @@ +--- +"@mcpjam/sdk": patch +"@mcpjam/inspector": patch +--- + +A model-provider failure is attributed to us, not filed against the server + +20 trials in one audited prod window failed on "credit balance too low… Anthropic API". The step errored, the asserts were skipped, and the chain ended `userValue: notMeasured / noEvidenceCaptured` with **no failure category at all** — our provider's outage presented as an unattributed server failure, in a report card whose entire job is to say whose side broke. + +The layer that failed is now tagged at the catch site and carried into the chain. `providerError` marks the stages a model-call failure left blank, and `categoryFor` files the run under `setup` — the existing bucket for our own side breaking, so no new category was needed. + +**Classified structurally, never by reading the message.** `drive-hosted-eval-turn.ts` already knows which layer it is in: `failTurn` is the engine's stream, `mapThrownTurnError` names its own call site, and "pre-turn setup" is the one that never reached the model. A text classifier would be one provider's wording away from mis-attributing a whole class of run. The engine's `code` and `httpStatus` ride along as diagnostics for a reader, and are deliberately _not_ part of the decision. + +Three boundaries are deliberate: + +- **A stage with its own observation keeps its own row.** A provider dying at turn 4 does not un-observe turns 1–3. What is re-labelled is a stage that measured nothing, plus — see "withdraws the failures it made unknowable" below — a `failed` row whose verdict rests on an *absence* the outage could equally well explain. A verdict resting on something actually observed always stands. +- **Never `failed`.** A run that could not be attempted has measured nothing about the server, and inflating a server failure rate with our own outage is the mis-attribution this reason exists to prevent. +- **A broken grader still outranks it.** `evaluator` is never folded into another category. + +`providerError` is broader than its name: it covers a provider outage, an exhausted credit balance, a rate limit, and our own spend guardrails. What they share is that _our_ side of the call broke, which is the only distinction the chain needs to stop blaming the server. That is stated in the reason's own docblock. + +Analyzer 7 → 8. This bump moves `STAGE_REASONS`, and the backend mirror already carries the member — it shipped deliberately ahead of this change — so nothing quarantines during the deploy window. + +**Stated limitation, unchanged:** the legacy verdict still counts these trials failed. Changing verdict population is a customer gate change and remains deferred behind its own product decision and release note. diff --git a/.changeset/evals-provider-error-catch-sites.md b/.changeset/evals-provider-error-catch-sites.md new file mode 100644 index 0000000000..dc0635de46 --- /dev/null +++ b/.changeset/evals-provider-error-catch-sites.md @@ -0,0 +1,22 @@ +--- +"@mcpjam/inspector": patch +--- + +Provider-error attribution reaches the local path, and stops over-claiming on the harness + +Two catch sites turned out to be broader than the comments above them claimed. Both were found in review, and both are the provider-error defect wearing a different coat — attributing to the model a failure that never reached it, which misleads a report card exactly as much as the original did. + +**The harness reports setup failures through the same callback as stream failures.** `runHarnessTurn` wraps its entire turn — preparation included — in one `try`, so a missing `projectId`, a missing auth bearer, or disabled broker credential delivery arrives at `onEngineError` looking precisely like a provider outage. `failTurn` tagged every one of them `model`, which files our own setup bug as the provider's. + +The engine now reports the **phase** it failed in, derived from the trace-started flag the emitter already holds to decide whether a turn happened at all — never from reading the message. `failTurn` reads that phase. An engine that reports none still means `model`: every emitter that omits it today is a real stream failure, and defaulting the other way would un-attribute the outages this work was built for. + +**The local path carried no source at all.** When `orgByokRuntime.kind === "local"`, execution goes to `runLocalIteration` rather than the hosted driver, and neither of its finish calls supplied a `stepError`. So local-BYOK trials that died on the model call still finalized with blank stage reasons and no failure category — the exact misattribution the original change removed, surviving untouched on the path it never covered. + +The local driver now records the layer at both of its error sites, and threads it through both finish paths: + +- **An empty model stream** is unambiguously the model call — no other layer reaches that branch. +- **A non-tool error span** is the model call **only when its category is `llm`**. That branch selects every non-tool error span, and `connection`, `discovery` and `oauth` spans are all reachable there; tagging those `model` would blame the provider for a server we could not reach. + +Anything else leaves the source unset. An absent source attributes nothing, which is the right floor — no attribution is strictly better than a confident wrong one. + +Both decisions are extracted as small pure functions (`failedLayerForEngineError`, `modelLayerForErrorSpan`) so the choice can be read and tested on its own rather than inferred from a call site's position. Mutation-checked: ignoring the phase, tagging every span, and tagging none each fail exactly their intended tests. diff --git a/.changeset/evals-provider-error-chain-consistency.md b/.changeset/evals-provider-error-chain-consistency.md new file mode 100644 index 0000000000..ff18aad909 --- /dev/null +++ b/.changeset/evals-provider-error-chain-consistency.md @@ -0,0 +1,15 @@ +--- +"@mcpjam/sdk": patch +--- + +Withdrawing a provider-blocked failure no longer leaves the chain arguing with itself + +Two follow-ups on the withdrawal added in the previous round. + +**The cascade ran before the withdrawal.** The positional pass reads `failed` rows to decide which later stages "never ran". Withdrawing a provider-blocked failure *after* it had run left `call`, `response` and `userValue` still saying `earlierStageFailed` while no stage failed and no `firstFailedStage` existed — three rows citing a failure the chain no longer records. + +The withdrawal now happens first, so the cascade sees the rows as they will actually be reported: a provider outage marks the later stages `providerError`, which is *why* they were not measured, rather than blaming a stage that is no longer failed. A failure the provider did **not** explain still cascades exactly as before — `unexpectedToolCall` survives the withdrawal, stays the first failed row, and the stages after it still read `notReached`. The cascade is repaired, not disabled. + +**The reason's label spoke for the run.** It read *"the model provider failed the call, so the run never reached the server"* — but `providerError` is applied per row, so a multi-turn iteration whose provider died at turn 4 keeps its earlier measured rows. The run-level claim would sit directly beside a `call: passed` that disproves it, and send a reader after the wrong timeline. It now says *"…so this stage was never measured"*, which is true of the row it labels. + +Mutation-checked: restoring the old ordering reproduces the self-contradicting chain, and restoring the run-level wording fails the label's scope test. diff --git a/.changeset/evals-provider-error-reaches-the-runner.md b/.changeset/evals-provider-error-reaches-the-runner.md new file mode 100644 index 0000000000..cecd0ed9cb --- /dev/null +++ b/.changeset/evals-provider-error-reaches-the-runner.md @@ -0,0 +1,21 @@ +--- +"@mcpjam/inspector": patch +--- + +Provider attribution actually reaches the runner — it was cut in three places + +Three breaks in the wire between a classified provider failure and an attributed chain. Together the first two meant `providerError` **never fired on the hosted path at all** — the path the audited Anthropic-credit trials ran on. + +**The hosted bridge copied only the message.** `buildHostedStepHandlers` converts a `HostedEvalTurnOutcome` into a `StepEngineOutcome`, and both of its sites copied `iterationError` / `iterationErrorDetails` and nothing else. `drive-hosted-eval-turn` classified the failure and `step-executor` was ready to propagate it, but the classification died one hop from where it was made, so `iterationStepError` was never built and no hosted run was ever attributed. + +**The widget follow-up loop reduced a full outcome to a bare string.** A turn that dies on the provider is the same event whether it was the prompt or a `ui/message` follow-up; reporting one and not the other made attribution depend on which turn the model happened to fail on. + +**The judge second pass re-derived without it.** `stepError` is transient runner state — an input to the first derivation, never persisted — so the moment a judge verdict landed, `providerError` and the `setup` category were silently dropped and the run went back to being filed against the server. + +That last one is recovered from the stored chain rather than a new persisted field: `providerError` is written **if and only if** the model layer was classified as the failure, so its presence in `stageResults` is a faithful witness of that input. Nothing is invented. Only `code` and `httpStatus` are lost, and those were explicitly diagnostics rather than part of the classification. + +## Why all three shipped green + +Every existing provider-error test builds a `StageEvidence` with `stepError` already on it and asserts what the analyzer does with it. **None exercised the plumbing that puts it there** — so a unit test of the analyzer could not fail when the wire was cut. + +This adds tests that drive the real `step-handlers` → `step-executor` path and the real judge payload builder. Each was verified to reproduce the exact bug it covers: restoring any of the three breaks fails its test and nothing else. diff --git a/.changeset/evals-provider-error-withdraws-absences.md b/.changeset/evals-provider-error-withdraws-absences.md new file mode 100644 index 0000000000..f5d0bb34a9 --- /dev/null +++ b/.changeset/evals-provider-error-withdraws-absences.md @@ -0,0 +1,19 @@ +--- +"@mcpjam/sdk": patch +"@mcpjam/inspector": patch +--- + +A provider outage withdraws the failures it made unknowable, and is dated from the handover + +Two further review findings, and the first is the one that mattered most: the provider-error fix was **inert on the shape it exists for**. + +**A missing tool call is not a selection defect when the provider never let us make it.** `applyProviderError` re-labelled only rows that measured *nothing*. But a case expecting a tool call whose provider died already has `selection: failed / missingToolCall` written by the matcher before the chain is derived — so `firstFailedStage` stayed `selection`, `categoryFor` returned `selection`, and the outage was filed as a model-selection defect. The exact misattribution this reason was built to remove, on the commonest case in the corpus. + +The fix turns on a distinction worth stating plainly: + +- An **absence** verdict — no call arrived, an assertion over the output did not hold, the judge scored a truncated transcript low — is only sound if the run was allowed to finish. When our own model call died first, "it did not happen" has a second explanation that outranks the accusation, and we cannot tell which is true. Those rows become `notMeasured / providerError`, and their evidence goes with the verdict it supported. +- A **presence** verdict stands: an unexpected call was really made, arguments really mismatched, a tool really errored, a render really failed. `connectFailed` and `toolsListFailed` matter most here — they happen *before* any model call, so a server that would not connect is never excused by a provider error that came later. Letting a provider blip launder a genuine server defect would be the worse bug of the two. + +**The phase flag was read too late.** It used `driver.traceStarted`, and the driver is only built after `agent.stream(...)` *resolves* — so an immediate provider rejection (auth, quota, a rate limit) was reported as our setup failing when the model had in fact been asked. The flag is now set immediately before the call, marking the handover itself rather than a successful one. + +Mutation-checked in both directions: never withdrawing a failed row reproduces the original bug; withdrawing every failed row launders the server defects; and keeping stale evidence on a withdrawn row leaves it arguing for a failure it no longer claims. SDK 6,894 passed; CLI gate suite 1,182 passed, 0 failed; server harness + evals 1,079 passed. diff --git a/mcpjam-inspector/server/services/evals-runner.ts b/mcpjam-inspector/server/services/evals-runner.ts index 0daf9b511e..4bd93eb7d1 100644 --- a/mcpjam-inspector/server/services/evals-runner.ts +++ b/mcpjam-inspector/server/services/evals-runner.ts @@ -3367,6 +3367,7 @@ const runLocalIteration = async ({ activeTraceCtx: null, iterationError: undefined, iterationErrorDetails: undefined, + stepErrorSource: undefined, pinnedSetupFailure: false, }; // PR 4d review fix (CodeRabbit): hoisted so persistence sites in the @@ -3848,6 +3849,13 @@ const runLocalIteration = async ({ // consumer than the stored transcript). const finishParams = buildIterationFinishParams({ iterationId, + // The layer that failed, when this driver could tell — the local twin of + // the hosted path's `stepError`. Without it a local-BYOK trial that died + // on the model call finalizes uncategorised, which is the very failure + // the provider-error work removed on the hosted path. + ...(acc.stepErrorSource + ? { stepError: { source: acc.stepErrorSource } } + : {}), // Keys shadow-mismatch telemetry only; never read for the verdict. ...(runId !== null ? { runId: String(runId) } : {}), // The run's FROZEN position. Threaded so a per-suite `off` is honoured on @@ -4084,6 +4092,12 @@ const runLocalIteration = async ({ // success path. const failParams = buildIterationFinishParams({ iterationId, + // Same as the success path: carry the layer when the driver could tell. + // This branch is the one a model-call failure most often ends on, so + // omitting it here would leave the fix half-applied. + ...(acc.stepErrorSource + ? { stepError: { source: acc.stepErrorSource } } + : {}), ...(runId !== null ? { runId: String(runId) } : {}), ...(gradingMode ? { gradingMode } : {}), scoreMatchOptions: scoreMatchOptionsFor(test), @@ -4679,6 +4693,10 @@ const runHostedIterationWithBrowser = async ( let iterationError: string | undefined = undefined; let iterationErrorDetails: string | undefined = undefined; + /** Which layer raised `iterationError`, when the executor classified it. */ + let iterationStepError: + | { source?: "model" | "setup"; code?: string; httpStatus?: number } + | undefined = undefined; const capturedSpans: EvalTraceSpan[] = []; // PR 4d review fix (Codex P2 / Cursor Medium): see hoist above the // `prepareChatV2` try. @@ -4872,6 +4890,15 @@ const runHostedIterationWithBrowser = async ( if (result.iterationError) { iterationError = result.iterationError; iterationErrorDetails = result.iterationErrorDetails; + if (result.errorSource) { + iterationStepError = { + source: result.errorSource, + ...(result.errorCode ? { code: result.errorCode } : {}), + ...(typeof result.errorHttpStatus === "number" + ? { httpStatus: result.errorHttpStatus } + : {}), + }; + } } // Pinned setup failure (server not connected) — drives `status:"setup_failed"` // below, mirroring the local runner. @@ -4977,6 +5004,9 @@ const runHostedIterationWithBrowser = async ( : {}), spans: capturedSpans, prompts: promptTraceSummaries, + // UVH-IN2: the layer that raised the fatal error, so the chain can say a + // provider outage was ours rather than filing it against the server. + ...(iterationStepError ? { stepError: iterationStepError } : {}), ...(widgetSnapshots ? { widgetSnapshots } : {}), // Browser-rendered MCP App eval (PR 14): hosted-path browser artifacts // (see the non-stream backend runner). diff --git a/mcpjam-inspector/server/services/evals/__tests__/__snapshots__/runner-parity.test.ts.snap b/mcpjam-inspector/server/services/evals/__tests__/__snapshots__/runner-parity.test.ts.snap index 84d97181b0..cc43dd7138 100644 --- a/mcpjam-inspector/server/services/evals/__tests__/__snapshots__/runner-parity.test.ts.snap +++ b/mcpjam-inspector/server/services/evals/__tests__/__snapshots__/runner-parity.test.ts.snap @@ -181,7 +181,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-batch h "missingCount": 0, "multiTurn": true, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -210,7 +210,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-batch h }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -417,7 +417,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-batch h "stepIndex": 1, }, ], - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -446,7 +446,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-batch h }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -747,7 +747,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-batch m "missingCount": 0, "multiTurn": true, "outputTokens": 4, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -776,7 +776,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-batch m }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -977,7 +977,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-batch p "mismatchCount": 0, "missingCount": 0, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -1006,7 +1006,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-batch p }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -1246,7 +1246,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-stream "missingCount": 0, "multiTurn": true, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -1275,7 +1275,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-stream }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -1494,7 +1494,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-stream "mismatchCount": 0, "missingCount": 0, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -1523,7 +1523,7 @@ exports[`runner parity (golden Convex payload + event sequence) > hosted-stream }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -1720,7 +1720,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch ca }, }, ], - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -1749,7 +1749,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch ca }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -1999,7 +1999,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch in }, }, ], - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -2028,7 +2028,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch in }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -2201,7 +2201,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch mo "reason": "no widget render observations recorded", }, ], - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -2230,7 +2230,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch mo }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -2430,7 +2430,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch mu "missingCount": 0, "multiTurn": true, "outputTokens": 4, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -2459,7 +2459,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch mu }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -2616,7 +2616,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch ne "mismatchCount": 0, "missingCount": 0, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -2645,7 +2645,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch ne }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -2801,7 +2801,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch pr "mismatchCount": 0, "missingCount": 0, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -2830,7 +2830,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch pr }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -3040,7 +3040,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch to }, }, ], - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -3069,7 +3069,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch to }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -3242,7 +3242,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch to "mismatchCount": 0, "missingCount": 0, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -3271,7 +3271,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch to }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -3437,7 +3437,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch to "mismatchCount": 0, "missingCount": 0, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -3466,7 +3466,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch to }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -3617,7 +3617,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch un "mismatchCount": 0, "missingCount": 0, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -3646,7 +3646,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch un }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -3816,7 +3816,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch wi "reason": "widget rendered (1/1 observation(s))", }, ], - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -3845,7 +3845,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-batch wi }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -4040,7 +4040,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-stream h "missingCount": 0, "multiTurn": true, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -4069,7 +4069,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-stream h }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -4294,7 +4294,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-stream m "missingCount": 0, "multiTurn": true, "outputTokens": 4, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -4323,7 +4323,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-stream m }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -4503,7 +4503,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-stream p "mismatchCount": 0, "missingCount": 0, "outputTokens": 0, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -4532,7 +4532,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-stream p }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { @@ -4659,7 +4659,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-stream p "mismatchCount": 0, "missingCount": 0, "outputTokens": 2, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "stageMeasurements": { "rows": [ { @@ -4688,7 +4688,7 @@ exports[`runner parity (golden Convex payload + event sequence) > local-stream p }, ], "schemaVersion": 1, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, }, "stageResults": [ { diff --git a/mcpjam-inspector/server/services/evals/__tests__/judge-second-pass.test.ts b/mcpjam-inspector/server/services/evals/__tests__/judge-second-pass.test.ts index 4ffb982632..9dd64efac9 100644 --- a/mcpjam-inspector/server/services/evals/__tests__/judge-second-pass.test.ts +++ b/mcpjam-inspector/server/services/evals/__tests__/judge-second-pass.test.ts @@ -12,7 +12,9 @@ import { type MetadataAttributionStageDerivationBody, } from "../judge-stage-backend.js"; import { + deriveIterationPayload, judgeEvidenceFromVerdict, + stepErrorFromStoredChain, metadataAttributionEvidenceFromVerdict, runJudgeSecondPass, type JudgeSecondPassPorts, @@ -941,3 +943,89 @@ describe("the second pass keeps its contract with the run and the first pass", ( ).toBe(true); }); }); + +describe("stepErrorFromStoredChain", () => { + // `stepError` is an INPUT to the first derivation and is never persisted, so + // the judge pass re-derived without it and dropped `providerError` and the + // `setup` category the moment a verdict landed — a run our own provider + // killed went back to being filed against the server. + + it("recovers the model layer from a chain that recorded providerError", () => { + expect( + stepErrorFromStoredChain([ + { stage: "connection", state: "passed", reason: "observed" }, + { stage: "selection", state: "notMeasured", reason: "providerError" }, + ]), + ).toEqual({ source: "model" }); + }); + + it("recovers nothing from a chain that recorded no provider failure", () => { + // The witness has to be the reason itself. Inferring a provider error from + // any other blank row would re-introduce the guess this reason removed. + expect( + stepErrorFromStoredChain([ + { stage: "selection", state: "notMeasured", reason: "noEvidenceCaptured" }, + { stage: "userValue", state: "failed", reason: "predicateFailed" }, + ]), + ).toBeUndefined(); + }); + + it("recovers nothing when there is no chain at all", () => { + expect(stepErrorFromStoredChain(undefined)).toBeUndefined(); + expect(stepErrorFromStoredChain([])).toBeUndefined(); + expect(stepErrorFromStoredChain("not an array")).toBeUndefined(); + }); +}); + +describe("the recovery is WIRED into the re-derivation", () => { + // Testing `stepErrorFromStoredChain` alone cannot fail when the wire is cut, + // and a cut wire is exactly how this attribution kept getting lost. So this + // drives the real payload builder and asserts the re-derived chain still + // says the model layer failed. + // + // The fixture is the SHAPE a provider failure actually leaves: the server was + // reached (spans exist, so connection and discovery are implied) and then our + // model call died, leaving the stages after it blank. An iteration with no + // evidence at all derives to `setupAborted`, which is a more specific reason + // and correctly outranks `providerError` — so it would prove nothing here. + const reachedThenDied = { + status: "failed", + traceComplete: true, + stageCase: { + mode: "model_driven", + expectsToolCall: false, + expectsWidgetRender: false, + assertionCount: 0, + }, + spans: [{ id: "s1", name: "tools/call", category: "tool", status: "ok" }], + }; + + const derive = (stageResults: unknown) => + deriveIterationPayload({ + iteration: { ...reachedThenDied, metadata: { stageResults } }, + mode: "advisory", + judgeVerdict: { status: "scored", verdict: "fail", score: 0.1 }, + attributionVerdict: undefined, + } as never).stage; + + const reasons = (stage: Record) => + (stage.stageResults as { reason: string }[]).map((r) => r.reason); + + it("keeps providerError and the setup category through a judge pass", () => { + const stage = derive([ + { stage: "selection", state: "notMeasured", reason: "providerError" }, + ]); + expect(reasons(stage)).toContain("providerError"); + expect(stage.failureCategory).toBe("setup"); + }); + + it("does not invent a provider failure on a run that had none", () => { + // The same evidence, a stored chain that never blamed the provider. The + // blank stage stays blank rather than acquiring an attribution the first + // derivation did not make. + const stage = derive([ + { stage: "selection", state: "notMeasured", reason: "noEvidenceCaptured" }, + ]); + expect(reasons(stage)).not.toContain("providerError"); + }); +}); diff --git a/mcpjam-inspector/server/services/evals/__tests__/provider-error-attribution.test.ts b/mcpjam-inspector/server/services/evals/__tests__/provider-error-attribution.test.ts new file mode 100644 index 0000000000..8bfe2690d1 --- /dev/null +++ b/mcpjam-inspector/server/services/evals/__tests__/provider-error-attribution.test.ts @@ -0,0 +1,60 @@ +/** + * WHICH LAYER an eval trial's failure is attributed to. + * + * The provider-error work exists to stop our own model-call failures being + * filed against the server under test. Both decisions here are the places that + * work can go wrong in the OTHER direction — attributing to the model a + * failure that never reached it — which is the same defect wearing a different + * coat, and just as misleading in a report card whose job is to say whose side + * broke. + * + * Both were found in review after the first version shipped, and both are + * about a catch site being broader than the comment above it claimed. + */ +import { describe, expect, it } from "vitest"; +import { failedLayerForEngineError } from "../drive-hosted-eval-turn.js"; +import { modelLayerForErrorSpan } from "../drive-local-eval-turn.js"; + +describe("the hosted path reads the engine's reported phase", () => { + it("attributes a stream failure to the model", () => { + expect(failedLayerForEngineError({ phase: "stream" })).toBe("model"); + }); + + it("attributes a PRE-STREAM harness failure to setup, not the model", () => { + // `runHarnessTurn` wraps its whole turn in one try, so a missing + // projectId, a missing auth bearer and disabled broker credential delivery + // all arrive through the same callback a provider outage does. None of + // them reached a model, and calling them `providerError` would file our + // own setup bug as the provider's. + expect(failedLayerForEngineError({ phase: "setup" })).toBe("setup"); + }); + + it("still says model when the engine reports no phase at all", () => { + // The compatibility floor. Every emitter that omits a phase today is a + // real stream failure, and defaulting the other way would un-attribute + // the outages this work was built for. + expect(failedLayerForEngineError({})).toBe("model"); + expect(failedLayerForEngineError(undefined)).toBe("model"); + }); +}); + +describe("the local path reads the error span's own category", () => { + it("attributes an llm span to the model", () => { + expect(modelLayerForErrorSpan({ category: "llm" })).toBe("model"); + }); + + it.each([["connection"], ["discovery"], ["oauth"], ["step"], ["execution"]])( + "attributes NOTHING to a %s span", + (category) => { + // The branch that finds these selects every non-tool error span. A + // server we could not connect to is not a provider outage, and saying so + // would blame the wrong side — the exact failure this work removes. + expect(modelLayerForErrorSpan({ category })).toBeUndefined(); + }, + ); + + it("attributes nothing to a span with no category", () => { + expect(modelLayerForErrorSpan({})).toBeUndefined(); + expect(modelLayerForErrorSpan(undefined)).toBeUndefined(); + }); +}); diff --git a/mcpjam-inspector/server/services/evals/__tests__/provider-error-plumbing.test.ts b/mcpjam-inspector/server/services/evals/__tests__/provider-error-plumbing.test.ts new file mode 100644 index 0000000000..88ef3c3cfe --- /dev/null +++ b/mcpjam-inspector/server/services/evals/__tests__/provider-error-plumbing.test.ts @@ -0,0 +1,212 @@ +/** + * The WIRE from a classified provider failure to an attributed chain. + * + * Every other provider-error test builds a `StageEvidence` with `stepError` + * already on it and asserts what the analyzer does with it. None of them + * exercises the plumbing that is supposed to PUT it there — and that gap is why + * four independent breaks in that plumbing all shipped green: + * + * - the hosted bridge in `step-handlers` copied only the error message; + * - the widget follow-up loop reduced a full outcome to a bare string; + * - the local driver carried no source at all; + * - the judge second pass re-derived without it. + * + * So these tests run the REAL `executeSteps` over real handler outcomes and + * assert the attribution survives each hop. A unit test of the analyzer cannot + * fail when the wire is cut; these can. + */ +import { describe, it, expect, vi } from "vitest"; +import type { TestStep } from "@/shared/steps"; + +// Mocked so the HOSTED BRIDGE itself is under test: `buildHostedStepHandlers` +// converts a `HostedEvalTurnOutcome` into a `StepEngineOutcome`, and that +// conversion is where the attribution was being dropped. +const { driveHostedEvalTurnMock } = vi.hoisted(() => ({ + driveHostedEvalTurnMock: vi.fn(), +})); +vi.mock("../drive-hosted-eval-turn", async () => { + const actual = await vi.importActual< + typeof import("../drive-hosted-eval-turn") + >("../drive-hosted-eval-turn"); + return { ...actual, driveHostedEvalTurn: driveHostedEvalTurnMock }; +}); + +import { + createStepExecutionState, + executeSteps, + type StepEngineOutcome, + type StepExecutorHandlers, +} from "../step-executor"; +import { buildHostedStepHandlers } from "../step-handlers"; +import type { BrowserSessionContext } from "../../browser-session-context"; + +type BrowserMock = Pick< + BrowserSessionContext, + | "replayInteractStep" + | "evaluateWidgetAssertion" + | "setKeepWidgetsMountedForSteps" + | "setActivePromptIndex" + | "setActiveAuthoredStepId" + | "widgetRenderObservations" + | "drainFollowUps" +>; + +function makeBrowser(overrides: Partial = {}): BrowserMock { + return { + setActivePromptIndex: vi.fn(), + setActiveAuthoredStepId: vi.fn(), + setKeepWidgetsMountedForSteps: vi.fn(), + replayInteractStep: vi.fn(async () => ({ ok: true })), + evaluateWidgetAssertion: vi.fn(async () => ({ ok: true })), + widgetRenderObservations: [], + drainFollowUps: vi.fn(() => []), + ...overrides, + }; +} + +/** What a hosted turn returns when our own model call died. */ +const PROVIDER_DIED: StepEngineOutcome = { + iterationError: "credit balance too low", + iterationErrorDetails: "Anthropic API", + errorSource: "model", + errorCode: "billing_limit_reached", + errorHttpStatus: 429, +}; + +const ONE_PROMPT: TestStep[] = [ + { id: "p", kind: "prompt", prompt: "Find my order" }, +]; + +function run( + steps: TestStep[], + handlers: Partial, + browser = makeBrowser(), +) { + return executeSteps({ + steps, + state: createStepExecutionState(), + browser: browser as unknown as BrowserSessionContext, + handlers: { + onPrompt: vi.fn(async () => ({}) as StepEngineOutcome), + onToolCall: vi.fn(async () => ({}) as StepEngineOutcome), + ...handlers, + } as StepExecutorHandlers, + }); +} + +describe("a classified provider failure reaches the runner", () => { + it("carries source, code and status off a failed prompt turn", async () => { + // The main hosted path, and the one the audited Anthropic-credit trials + // ran on. Before this the executor received only `iterationError`, so + // `iterationStepError` was never built and NO hosted run was attributed. + const result = await run(ONE_PROMPT, { + onPrompt: vi.fn(async () => PROVIDER_DIED), + }); + + expect(result.iterationError).toBe("credit balance too low"); + expect(result.errorSource).toBe("model"); + // Diagnostics ride along; they are never the basis for the classification. + expect(result.errorCode).toBe("billing_limit_reached"); + expect(result.errorHttpStatus).toBe(429); + }); + + it("carries the same attribution off a widget FOLLOW-UP turn", async () => { + // A turn that dies on the provider is the same event whether it was the + // prompt or a `ui/message` follow-up. Reporting one and not the other made + // the attribution depend on which turn the model happened to fail on. + const browser = makeBrowser({ + drainFollowUps: vi + .fn() + .mockReturnValueOnce(["add the red one"]) + .mockReturnValue([]), + }); + + const result = await run( + [ + { id: "p", kind: "prompt", prompt: "Show me a redbull" }, + { + id: "i", + kind: "interact", + toolName: "search-products", + action: { kind: "click", target: { text: "🛒" } }, + }, + ], + { + onPrompt: vi.fn(async () => ({}) as StepEngineOutcome), + onFollowUp: vi.fn(async () => PROVIDER_DIED), + }, + browser, + ); + + expect(result.iterationError).toBe("credit balance too low"); + expect(result.errorSource).toBe("model"); + expect(result.errorHttpStatus).toBe(429); + }); + + it("says nothing when the engine classified nothing", async () => { + // The compatibility floor, end to end. A handler that cannot name the + // layer must leave the source absent rather than have the wire invent one. + const result = await run(ONE_PROMPT, { + onPrompt: vi.fn(async () => ({ + iterationError: "something went wrong", + })), + }); + + expect(result.iterationError).toBe("something went wrong"); + expect(result.errorSource).toBeUndefined(); + expect(result.errorCode).toBeUndefined(); + }); + + it("says nothing about a turn that did not fail", async () => { + const result = await run(ONE_PROMPT, { + onPrompt: vi.fn(async () => ({}) as StepEngineOutcome), + }); + expect(result.iterationError).toBeUndefined(); + expect(result.errorSource).toBeUndefined(); + }); +}); + +describe("the hosted bridge does not drop what the engine classified", () => { + it("converts a classified turn outcome into an attributed step outcome", async () => { + // THE BREAK THIS FILE EXISTS FOR. `driveHostedEvalTurn` classifies the + // failure; `buildHostedStepHandlers` converts its outcome for the executor. + // That conversion copied only the message, so the classification died one + // hop from where it was made and every hosted run went unattributed. + driveHostedEvalTurnMock.mockResolvedValue({ + kind: "failed", + iterationError: "credit balance too low", + iterationErrorDetails: "Anthropic API", + errorSource: "model", + errorCode: "billing_limit_reached", + errorHttpStatus: 429, + }); + + const handlers = buildHostedStepHandlers({ + acc: { + messageHistory: [], + capturedSpans: [], + accumulatedUsage: { inputTokens: 0, outputTokens: 0, totalTokens: 0 }, + toolsCalledByPrompt: [], + assistantMessageByPrompt: [], + toolErrorsByPrompt: [], + pinnedToolErrors: [], + }, + browser: { + setActivePromptIndex: vi.fn(), + setActiveWidgetChecks: vi.fn(), + dismissCarriedWidget: vi.fn(async () => {}), + }, + } as never); + + const outcome = await handlers.onPrompt({ + step: { id: "p", kind: "prompt", prompt: "Find my order" }, + stepIndex: 0, + turnOrdinal: 0, + } as never); + + expect(outcome.iterationError).toBe("credit balance too low"); + expect(outcome.errorSource).toBe("model"); + expect(outcome.errorCode).toBe("billing_limit_reached"); + expect(outcome.errorHttpStatus).toBe(429); + }); +}); diff --git a/mcpjam-inspector/server/services/evals/drive-hosted-eval-turn.ts b/mcpjam-inspector/server/services/evals/drive-hosted-eval-turn.ts index b7fdb45a0e..21dbc6e391 100644 --- a/mcpjam-inspector/server/services/evals/drive-hosted-eval-turn.ts +++ b/mcpjam-inspector/server/services/evals/drive-hosted-eval-turn.ts @@ -64,6 +64,27 @@ export type HostedEvalTurnOutcome = kind: "failed"; iterationError: string; iterationErrorDetails?: string; + /** + * WHICH LAYER failed, decided by the catch site rather than by reading + * the message. + * + * `model` means the model-call layer — the engine's stream, or a throw + * escaping the assistant turn. A failure there is ours or our + * provider's: an outage, an exhausted credit balance, a spend guardrail. + * It says nothing about the MCP server under test, which is exactly why + * the chain must not file it as an unattributed server failure. + * + * `setup` is pre-turn work that never reached the model. + * + * Deliberately NOT derived from the error text. A message classifier + * would be one provider's wording away from silently mis-attributing a + * whole class of run, and the catch site already knows the answer. + */ + errorSource?: "model" | "setup"; + /** The engine's structured code, when the failure carried one. */ + errorCode?: string; + /** HTTP status, when the failure came from a non-OK response. */ + errorHttpStatus?: number; }; /** Stream-runner SSE concerns, layered over the shared skeleton per turn. */ @@ -278,6 +299,29 @@ const truncateError = (message: string): string => * forever. Consumed by the executor (R3); this module only exports the value. */ export const MAX_WIDGET_FOLLOWUP_TURNS = 3; + +/** + * Which layer failed, from what the engine REPORTED rather than from where + * this was called. + * + * The first version of this decision lived inline and assumed every engine + * error was a stream failure. That holds for the chat engine and not for the + * harness: `runHarnessTurn` wraps its whole turn — preparation included — in + * one try, so a missing `projectId`, a missing auth bearer, or disabled broker + * credential delivery arrives looking exactly like a provider outage. Calling + * those `model` files our own setup bug as the provider's, which is the + * mis-attribution this work exists to remove, moved one layer over. + * + * `model` stays the answer for an engine that reports no phase at all: every + * emitter that omits it today is a real stream failure, and defaulting the + * other way would un-attribute the outages this work was built for. + */ +export function failedLayerForEngineError( + event: { phase?: "setup" | "stream" } | undefined, +): "model" | "setup" { + return event?.phase === "setup" ? "setup" : "model"; +} + export async function driveHostedEvalTurn( params: DriveHostedEvalTurnParams ): Promise { @@ -395,7 +439,16 @@ export async function driveHostedEvalTurn( ...(iterationErrorDetails ? { iterationErrorDetails } : {}), }; sinks.onTurnFailure?.(failure); - return { kind: "failed", ...failure }; + // `failedStage` already names the layer; "pre-turn setup" is the one call + // site that never reached the model. + return { + kind: "failed" as const, + ...failure, + errorSource: + failedStage === "pre-turn setup" + ? ("setup" as const) + : ("model" as const), + }; }; // Pre-turn setup that can genuinely throw: the Chromium widget dismissal @@ -660,7 +713,33 @@ export async function driveHostedEvalTurn( : { iterationError: fallbackError }; logger.error(logLine); sinks.onTurnFailure?.(failure); - return { kind: "failed", ...failure }; + // WHICH LAYER, from the engine's own report rather than from this call + // site's position. + // + // The first version of this said "every path through here is the engine's + // stream failing". That is true of the chat engine and false of the + // HARNESS: `runHarnessTurn` wraps its entire turn — preparation included — + // in one try, so a missing projectId, a missing auth bearer or disabled + // broker credential delivery arrives here exactly like a provider outage. + // Calling those `model` would file our own setup bug as the provider's, + // which is the mis-attribution this whole change exists to remove, just + // moved one layer over. + // + // So the engine's `phase` decides when it is reported, and `model` remains + // the default only for emitters that do not report one — every such + // emitter today is a real stream failure. + const failedLayer = failedLayerForEngineError(lastEngineError); + // The structured code and status ride along when the engine captured them + // — they are diagnostics, never the basis for the classification. + return { + kind: "failed" as const, + ...failure, + errorSource: failedLayer, + ...(lastEngineError?.code ? { errorCode: lastEngineError.code } : {}), + ...(typeof lastEngineError?.httpStatus === "number" + ? { errorHttpStatus: lastEngineError.httpStatus } + : {}), + }; }; if (!turnResult.turnTrace) { @@ -669,7 +748,7 @@ export async function driveHostedEvalTurn( `[evals] runAssistantTurn${logSuffix} returned no turnTrace (engine runSucceeded=false); treating as cycle failure (messagesGrew=${ newMessages.length > 0 }, engineError=${ - lastEngineError ? (lastEngineError.code ?? "uncoded") : "none" + lastEngineError ? lastEngineError.code ?? "uncoded" : "none" })` ); } @@ -677,7 +756,7 @@ export async function driveHostedEvalTurn( return failTurn( "Backend step returned no content (stream error or empty response)", `[evals] runAssistantTurn${logSuffix} produced no new messages this turn; treating as cycle failure (engineError=${ - lastEngineError ? (lastEngineError.code ?? "uncoded") : "none" + lastEngineError ? lastEngineError.code ?? "uncoded" : "none" })` ); } @@ -699,7 +778,7 @@ export async function driveHostedEvalTurn( `[evals] runAssistantTurn${logSuffix} turnTrace has non-tool error-status span; treating as cycle failure (span=${ stepErrorSpan.name } category=${stepErrorSpan.category} engineError=${ - lastEngineError ? (lastEngineError.code ?? "uncoded") : "none" + lastEngineError ? lastEngineError.code ?? "uncoded" : "none" })` ); } diff --git a/mcpjam-inspector/server/services/evals/drive-local-eval-turn.ts b/mcpjam-inspector/server/services/evals/drive-local-eval-turn.ts index 1791b85e9b..cd3bc428c1 100644 --- a/mcpjam-inspector/server/services/evals/drive-local-eval-turn.ts +++ b/mcpjam-inspector/server/services/evals/drive-local-eval-turn.ts @@ -45,9 +45,41 @@ export type LocalEvalTurnAcc = { activeTraceCtx: ReturnType | null; iterationError: string | undefined; iterationErrorDetails: string | undefined; + /** + * WHICH LAYER failed, when this driver can tell. + * + * The hosted path carries this from its catch site; the local one had no + * equivalent, so a local-BYOK trial that died on the model call finalized + * with blank stage reasons and no failure category — the exact + * mis-attribution the provider-error work exists to remove, surviving on the + * path the hosted fix never touched. + * + * Left UNSET whenever the layer is not structurally knowable. An absent + * source changes nothing downstream, which is the right floor: no + * attribution beats a wrong one. + */ + stepErrorSource: "model" | undefined; pinnedSetupFailure: boolean; }; +/** + * Whether an error span means the MODEL-CALL layer failed. + * + * The branch that finds these spans selects every non-tool error span, and + * `connection`, `discovery` and `oauth` spans are all reachable there — so + * treating the whole set as the model would blame the provider for a server we + * could not reach, which is the mis-attribution this work removes rather than + * relocates. + * + * `undefined` for everything else, deliberately. An absent source attributes + * nothing, and no attribution is strictly better than a confident wrong one. + */ +export function modelLayerForErrorSpan( + span: { category?: string } | undefined, +): "model" | undefined { + return span?.category === "llm" ? "model" : undefined; +} + export type LocalEvalTurnOutcome = | { kind: "completed" } | { kind: "cancelled" }; @@ -391,6 +423,9 @@ export async function driveLocalEvalTurn( if (promptResponseMessages.length === 0) { acc.iterationError = "Stream returned no content (local-BYOK driver failed)"; + // The model stream itself returned nothing. Unambiguously the model-call + // layer — there is no other layer this branch can be reached from. + acc.stepErrorSource = "model"; logger.error( "[evals] streamText returned no new messages this turn; treating as cycle failure" ); @@ -426,6 +461,7 @@ export async function driveLocalEvalTurn( ); if (stepErrorSpan) { acc.iterationError = `Local-BYOK step failed mid-turn: ${stepErrorSpan.name}`; + acc.stepErrorSource = modelLayerForErrorSpan(stepErrorSpan); logger.error( `[evals] streamText recorded non-tool error span; treating as cycle failure (span=${stepErrorSpan.name} category=${stepErrorSpan.category})` ); diff --git a/mcpjam-inspector/server/services/evals/finalize-iteration.ts b/mcpjam-inspector/server/services/evals/finalize-iteration.ts index 62292841cb..bba515fc95 100644 --- a/mcpjam-inspector/server/services/evals/finalize-iteration.ts +++ b/mcpjam-inspector/server/services/evals/finalize-iteration.ts @@ -125,6 +125,8 @@ function buildStageEvidence(args: { * failure would leave `call`/`response` looking unmeasured. */ toolErrors?: unknown[]; + /** Which layer raised a fatal step error, when the runner classified it. */ + stepError?: StageEvidence["stepError"]; toolSignals?: ToolExposureSignals; setupSignals?: StageSetupSignals; /** Advisory judge evidence. Absent on the first pass; see {@link buildStageMetadata}. */ @@ -159,6 +161,7 @@ function buildStageEvidence(args: { }>, } : {}), + ...(args.stepError ? { stepError: args.stepError } : {}), ...(args.toolSignals ? { toolSignals: args.toolSignals } : {}), ...(args.setupSignals ? { setupSignals: args.setupSignals } : {}), ...(args.judgeEvidence ? { judgeEvidence: args.judgeEvidence } : {}), @@ -198,6 +201,12 @@ export function buildStageMetadata(args: { predicateResults?: unknown[]; widgetRenderObservations?: RunnerWidgetRenderObservation[]; stageToolErrors?: unknown[]; + /** + * Which layer raised a fatal step error, when the runner classified it. + * Reported by the catch site, never parsed out of `error` — see + * `StageStepErrorLike`. + */ + stepError?: StageEvidence["stepError"]; toolSignals?: ToolExposureSignals; setupSignals?: StageSetupSignals; /** @@ -234,6 +243,7 @@ export function buildStageMetadata(args: { predicateResults: args.predicateResults, widgetRenderObservations: args.widgetRenderObservations, toolErrors: args.stageToolErrors, + ...(args.stepError ? { stepError: args.stepError } : {}), toolSignals: args.toolSignals, setupSignals: args.setupSignals, ...(args.judgeEvidence ? { judgeEvidence: args.judgeEvidence } : {}), @@ -441,8 +451,10 @@ function buildScoreMetadata(args: { * first-pass mismatch means the score projection disagrees with `passed`. */ function readUserValueRow( - stageMetadata: Record -): { state: StageResultRow["state"]; reason: StageResultRow["reason"] } | undefined { + stageMetadata: Record, +): + | { state: StageResultRow["state"]; reason: StageResultRow["reason"] } + | undefined { const rows = stageMetadata.stageResults; if (!Array.isArray(rows)) return undefined; for (const row of rows) { @@ -473,7 +485,9 @@ function buildSelectionToolCatalogMetadata(args: { if (!Array.isArray(rows)) return {}; const selectionRow = rows.find( (row): row is Partial => - typeof row === "object" && row !== null && (row as { stage?: unknown }).stage === "selection" + typeof row === "object" && + row !== null && + (row as { stage?: unknown }).stage === "selection", ); if (selectionRow?.state !== "failed") return {}; @@ -594,6 +608,11 @@ export function buildIterationFinishParams(args: { * them unless they are threaded here. */ stageToolErrors?: unknown[]; + /** + * Which layer raised the fatal step error, forwarded from the executor. + * Absent when nothing fatal happened, or when the caller could not say. + */ + stepError?: StageEvidence["stepError"]; /** Execution-layer policy blocks; persisted as metadata, never a failure. */ policyBlocks?: PolicyBlockRecord[]; /** Non-fatal policy configuration warnings, persisted for run consumers. */ @@ -705,6 +724,7 @@ export function buildIterationFinishParams(args: { predicateResults, widgetRenderObservations, stageToolErrors, + ...(args.stepError ? { stepError: args.stepError } : {}), toolSignals, setupSignals, policy: diff --git a/mcpjam-inspector/server/services/evals/judge-second-pass.ts b/mcpjam-inspector/server/services/evals/judge-second-pass.ts index 74492721d8..75dfe883b9 100644 --- a/mcpjam-inspector/server/services/evals/judge-second-pass.ts +++ b/mcpjam-inspector/server/services/evals/judge-second-pass.ts @@ -269,8 +269,44 @@ function isPredicateRow(value: unknown): value is StoredPredicateRow { ); } -/** Everything the derivation needs from one stored iteration. */ -function deriveIterationPayload(args: { +/** + * Recover the FIRST derivation's verdict about the model layer from the chain + * it wrote. + * + * `stepError` is transient runner state — an INPUT to that derivation, never + * persisted — so the judge second pass re-derived without it and silently + * dropped `providerError` and the `setup` category the moment a verdict + * arrived. A run our own provider killed went back to being filed against the + * server, which is exactly what that reason exists to prevent. + * + * Read from the stored chain rather than from a new persisted field, because + * the chain already says it: `providerError` is written if and only if the + * model layer was classified as the failure, so its presence is a faithful + * witness of that input. Nothing is invented here. + * + * `code` and `httpStatus` are not recoverable and are not recovered. They are + * diagnostics for a reader and were explicitly never part of the + * classification, so their absence changes no verdict. + */ +export function stepErrorFromStoredChain( + stageResults: unknown, +): { source: "model" } | undefined { + const rows = asArray(stageResults); + if (!rows) return undefined; + return rows.some((row) => asRecord(row)?.reason === "providerError") + ? { source: "model" } + : undefined; +} + +/** + * Everything the derivation needs from one stored iteration. + * + * Exported for tests: the recovery above is only useful if it is actually + * WIRED into this payload, and a test of the recovery alone cannot fail when + * the wire is cut — which is the precise gap that let four separate breaks in + * this attribution ship green. + */ +export function deriveIterationPayload(args: { iteration: JudgeSecondPassIterationRow; mode: GradingEngineMode; judgeVerdict: JudgeVerdictMetadata | undefined; @@ -331,6 +367,8 @@ function deriveIterationPayload(args: { // failed. const traceUsable = iteration.traceComplete !== false; + const recoveredStepError = stepErrorFromStoredChain(metadata.stageResults); + const stage = traceUsable ? buildStageMetadata({ ...(stageCase ? { stageCase } : {}), @@ -338,6 +376,7 @@ function deriveIterationPayload(args: { ...(iteration.prompts?.length ? { prompts: iteration.prompts } : {}), ...(iteration.messages?.length ? { messages: iteration.messages } : {}), ...(predicateRows.length ? { predicateResults: predicateRows } : {}), + ...(recoveredStepError ? { stepError: recoveredStepError } : {}), ...(iteration.toolSignals ? { toolSignals: iteration.toolSignals } : {}), ...(iteration.setupSignals ? { setupSignals: iteration.setupSignals } diff --git a/mcpjam-inspector/server/services/evals/step-executor.ts b/mcpjam-inspector/server/services/evals/step-executor.ts index 477863bd25..b82d042a49 100644 --- a/mcpjam-inspector/server/services/evals/step-executor.ts +++ b/mcpjam-inspector/server/services/evals/step-executor.ts @@ -161,6 +161,20 @@ export interface StepEngineOutcome { */ iterationError?: string; iterationErrorDetails?: string; + /** + * WHICH LAYER produced `iterationError`, reported by the catch site that + * raised it rather than inferred from its text. + * + * `model` is the model-call layer — our provider, not the MCP server under + * test. The chain uses it to stop filing an outage or an exhausted credit + * balance as an unattributed server failure. Absent when the caller does + * not know, which reads as "unclassified" and changes nothing. + */ + errorSource?: "model" | "setup"; + /** The engine's structured code, when the failure carried one. */ + errorCode?: string; + /** HTTP status, when the failure came from a non-OK response. */ + errorHttpStatus?: number; /** * When true, the iterationError is a SETUP failure (status:"failed"), not an * assertion failure (status:"completed"+error). Mirrors the pinned @@ -214,6 +228,10 @@ export interface StepExecutorResult { /** Set when a `prompt`/`toolCall` step reported a fatal error. */ iterationError?: string; iterationErrorDetails?: string; + /** Which layer raised `iterationError` — see `StepEngineOutcome`. */ + errorSource?: "model" | "setup"; + errorCode?: string; + errorHttpStatus?: number; /** True when `iterationError` is a setup (not assertion) failure. */ setupFailure: boolean; } @@ -340,7 +358,10 @@ async function drainAndDriveFollowUps( browser: Pick, handlers: StepExecutorHandlers, state: StepExecutionState, -): Promise { + // The failing OUTCOME, not just its message. Reducing it to a string here + // discarded the layer attribution, so a provider failure on a widget + // follow-up turn stayed uncategorised even once the hosted bridge carried it. +): Promise { if (!handlers.onFollowUp) return undefined; let remaining = MAX_WIDGET_FOLLOWUP_TURNS; while (remaining > 0) { @@ -362,7 +383,7 @@ async function drainAndDriveFollowUps( remaining -= 1; const outcome = await handlers.onFollowUp!({ text, stepIndex, turnOrdinal: turn }); applyOutcome(state, outcome, turn); - if (outcome.iterationError) return outcome.iterationError; + if (outcome.iterationError) return outcome; } } return undefined; @@ -507,7 +528,7 @@ export async function executeSteps(args: { sIdx: number, turn: number, ): Promise => { - const err = await drainAndDriveFollowUps( + const failed = await drainAndDriveFollowUps( label, sIdx, turn, @@ -515,16 +536,32 @@ export async function executeSteps(args: { handlers, state, ); - if (!err) return undefined; + if (!failed) return undefined; emitStatus(sIdx, "fail"); recordSkippedSteps( state, steps, sIdx + 1, - `widget follow-up turn errored (step ${sIdx}): ${err}`, + `widget follow-up turn errored (step ${sIdx}): ${failed.iterationError}`, ); emitSkipped(sIdx + 1); - return { state, iterationError: err, setupFailure: false }; + // The SAME shape the prompt-step failure path returns. A follow-up turn + // dying on the provider is the same event as a prompt turn dying on it, and + // reporting one and not the other made the attribution depend on which + // turn the model happened to fail. + return { + state, + iterationError: failed.iterationError, + ...(failed.iterationErrorDetails + ? { iterationErrorDetails: failed.iterationErrorDetails } + : {}), + ...(failed.errorSource ? { errorSource: failed.errorSource } : {}), + ...(failed.errorCode ? { errorCode: failed.errorCode } : {}), + ...(typeof failed.errorHttpStatus === "number" + ? { errorHttpStatus: failed.errorHttpStatus } + : {}), + setupFailure: false, + }; }; for (let stepIndex = 0; stepIndex < steps.length; stepIndex++) { @@ -556,6 +593,11 @@ export async function executeSteps(args: { state, iterationError: outcome.iterationError, iterationErrorDetails: outcome.iterationErrorDetails, + ...(outcome.errorSource ? { errorSource: outcome.errorSource } : {}), + ...(outcome.errorCode ? { errorCode: outcome.errorCode } : {}), + ...(typeof outcome.errorHttpStatus === "number" + ? { errorHttpStatus: outcome.errorHttpStatus } + : {}), setupFailure: outcome.setupFailure === true, }; } @@ -593,6 +635,11 @@ export async function executeSteps(args: { state, iterationError: outcome.iterationError, iterationErrorDetails: outcome.iterationErrorDetails, + ...(outcome.errorSource ? { errorSource: outcome.errorSource } : {}), + ...(outcome.errorCode ? { errorCode: outcome.errorCode } : {}), + ...(typeof outcome.errorHttpStatus === "number" + ? { errorHttpStatus: outcome.errorHttpStatus } + : {}), setupFailure: outcome.setupFailure === true, }; } diff --git a/mcpjam-inspector/server/services/evals/step-handlers.ts b/mcpjam-inspector/server/services/evals/step-handlers.ts index cfe88325cd..bed9aded70 100644 --- a/mcpjam-inspector/server/services/evals/step-handlers.ts +++ b/mcpjam-inspector/server/services/evals/step-handlers.ts @@ -260,6 +260,21 @@ export function buildHostedStepHandlers( ...(outcome.iterationErrorDetails ? { iterationErrorDetails: outcome.iterationErrorDetails } : {}), + // WHICH LAYER failed, carried across the bridge. + // + // Dropping these was enough to make provider attribution dead on + // the hosted path entirely: `drive-hosted-eval-turn` classifies the + // failure, `step-executor` propagates whatever it is handed, and + // `evals-runner` builds `stepError` from it — but this conversion + // sat in the middle copying only the message, so `errorSource` was + // always undefined and no hosted run was ever attributed. + ...(outcome.errorSource + ? { errorSource: outcome.errorSource } + : {}), + ...(outcome.errorCode ? { errorCode: outcome.errorCode } : {}), + ...(typeof outcome.errorHttpStatus === "number" + ? { errorHttpStatus: outcome.errorHttpStatus } + : {}), } : {}), }; @@ -310,6 +325,21 @@ export function buildHostedStepHandlers( ...(outcome.iterationErrorDetails ? { iterationErrorDetails: outcome.iterationErrorDetails } : {}), + // WHICH LAYER failed, carried across the bridge. + // + // Dropping these was enough to make provider attribution dead on + // the hosted path entirely: `drive-hosted-eval-turn` classifies the + // failure, `step-executor` propagates whatever it is handed, and + // `evals-runner` builds `stepError` from it — but this conversion + // sat in the middle copying only the message, so `errorSource` was + // always undefined and no hosted run was ever attributed. + ...(outcome.errorSource + ? { errorSource: outcome.errorSource } + : {}), + ...(outcome.errorCode ? { errorCode: outcome.errorCode } : {}), + ...(typeof outcome.errorHttpStatus === "number" + ? { errorHttpStatus: outcome.errorHttpStatus } + : {}), } : {}), }; diff --git a/mcpjam-inspector/server/utils/harness/run-harness-turn.ts b/mcpjam-inspector/server/utils/harness/run-harness-turn.ts index ac9869f64b..f2e185f8a2 100644 --- a/mcpjam-inspector/server/utils/harness/run-harness-turn.ts +++ b/mcpjam-inspector/server/utils/harness/run-harness-turn.ts @@ -725,6 +725,19 @@ export async function runHarnessTurn( // PersistedTurnTrace / abort). Constructed at STREAM start once `traceBaseMs` // is finalized; read by `onFinishEngine` (a sibling closure) for the trace. let driver: StreamTurnDriver | undefined; + /** + * Whether the model request was HANDED OVER — the boundary between our own + * preparation and the provider's turn. + * + * Not `driver.traceStarted`, which was the first version of this and is set + * far too late: the driver is only built after `agent.stream(...)` RESOLVES, + * so an immediate provider rejection (auth, quota, a rate limit) would be + * reported as our setup failing when the model had in fact been asked. + * + * Set immediately before the call instead, so the flag marks the handover + * itself rather than a successful one. + */ + let modelInvoked = false; const executeEngine = async ({ writer }: { writer: ChunkWriter }) => { onStreamWriterReady?.(writer); @@ -1862,6 +1875,9 @@ export async function runHarnessTurn( // WS3: a resume carries no new user prompt — feed the approval decision // into the in-flight turn via continueStream (the adapter collapses // `messages` to the last user message, so stream() would re-prompt). + // Everything above this line is ours; everything at or below it is the + // model's turn. See `modelInvoked`. + modelInvoked = true; const res = resumeFromApproval ? await agent.continueStream({ session, @@ -2767,6 +2783,14 @@ export async function runHarnessTurn( message: errorText, rawText: errorText, promptIndex, + // This catch covers the WHOLE turn, preparation included: the throws + // for a missing projectId, a missing auth bearer and disabled broker + // delivery all land here, and none of them ever reached a model. + // + // The flag is set at the HANDOVER, not once the stream is running, so + // a provider that rejects the request outright still reads as the + // model's failure rather than ours. + phase: modelInvoked ? "stream" : "setup", }); } finally { stopScopeStepUpBridge(); diff --git a/mcpjam-inspector/server/utils/mcpjam-stream-handler.ts b/mcpjam-inspector/server/utils/mcpjam-stream-handler.ts index 1920dd67ad..b8e3ca8a06 100644 --- a/mcpjam-inspector/server/utils/mcpjam-stream-handler.ts +++ b/mcpjam-inspector/server/utils/mcpjam-stream-handler.ts @@ -422,6 +422,22 @@ export interface MCPJamEngineErrorEvent { promptIndex: number; /** Step index when fired inside `processOneStep`; omitted for site (3). */ stepIndex?: number; + /** + * WHICH LAYER was running when this fired — not what the message says. + * + * `"setup"` means the turn died before the model stream began: the harness + * catches its own pre-stream preparation (no `projectId`, no auth bearer, + * broker credential delivery disabled, a sandbox it could not reserve) in + * the same block that catches a stream failure, and reports both here. + * A consumer that assumed every engine error was a provider failure would + * file our own setup bug as the provider's outage. + * + * Derived from a flag the emitter already holds — whether the turn's trace + * ever started — never from reading the message. Omitted by emitters that + * cannot distinguish the two, and a consumer must treat that as unknown + * rather than as either answer. + */ + phase?: "setup" | "stream"; /** * Classified form of this failure, including its `origin` — whose fault the * turn dying was. diff --git a/sdk/src/contract/decision-labels.ts b/sdk/src/contract/decision-labels.ts index e9f8736eab..4684e49f05 100644 --- a/sdk/src/contract/decision-labels.ts +++ b/sdk/src/contract/decision-labels.ts @@ -102,6 +102,13 @@ export const STAGE_REASON_LABELS = Object.freeze({ blockedByPolicy: "a policy blocked the run before it could be measured", evaluatorError: "the evaluator itself failed, so the run says nothing about the server", + // Scoped to THIS STAGE, not to the run. `providerError` is applied per row, + // and a multi-turn iteration whose provider died at turn 4 keeps its earlier + // measured rows — so a run-level "never reached the server" would sit + // directly beside a `call: passed` that disproves it, and send a reader + // after the wrong timeline. + providerError: + "the model provider failed the call, so this stage was never measured", setupAborted: "the environment was never prepared, so the test never began", connectFailed: "the configured server was reached and initialize failed there", diff --git a/sdk/src/contract/index.ts b/sdk/src/contract/index.ts index 90d9ca1acd..cd61b701c2 100644 --- a/sdk/src/contract/index.ts +++ b/sdk/src/contract/index.ts @@ -156,6 +156,7 @@ export type { StageSetupPhaseSignal, StageSetupSignals, StageSpanLike, + StageStepErrorLike, StageToolErrorLike, } from "./stage-derivation.js"; export { diff --git a/sdk/src/contract/stage-derivation.ts b/sdk/src/contract/stage-derivation.ts index 94d337a704..cc847edc0b 100644 --- a/sdk/src/contract/stage-derivation.ts +++ b/sdk/src/contract/stage-derivation.ts @@ -76,8 +76,13 @@ import { * every applicable stage green while its legacy verdict failed on exactly * that tool error — a disagreement the chain had no row to express. Re-uses * `toolError`, so again no mirror re-pin. + * + * 8 (UVH-IN2): a model-call-layer failure is attributed to `providerError` + * instead of leaving the trial uncategorised. This one DOES move + * `STAGE_REASONS`; the backend mirror already carries the member (UVH-BE1 + * shipped it deliberately ahead of this bump), so nothing quarantines. */ -export const STAGE_ANALYZER_VERSION = 7; +export const STAGE_ANALYZER_VERSION = 8; /** * Why a stage landed where it did. @@ -108,6 +113,16 @@ export const STAGE_REASONS = [ "blockedByPolicy", /** The grader itself failed, so the run says nothing about the server. */ "evaluatorError", + /** + * The MODEL-CALL layer failed: a provider outage, an exhausted credit + * balance, a rate limit, or one of our own spend guardrails. + * + * Broader than the name suggests, and deliberately so — what every case has + * in common is that OUR side of the call broke, so the run says nothing + * about the MCP server under test. Never `failed`: blaming the server for + * our provider's bad day is the mis-attribution this reason exists to stop. + */ + "providerError", /** The harness never got to the test (setup abort). */ "setupAborted", /** @@ -304,6 +319,20 @@ export type StageToolErrorLike = { toolName?: string; }; +/** + * The layer a fatal step error came from, reported by the catch site. + * + * `deriveStageResults` reads only `source`; `code` and `httpStatus` ride along + * as diagnostics for a reader, and are deliberately NOT part of the + * classification — a rule keyed on a provider's status codes would be one + * provider away from mis-attributing a whole class of run. + */ +export type StageStepErrorLike = { + source?: "model" | "setup"; + code?: string; + httpStatus?: number; +}; + export type StageRenderObservationLike = { status?: string; }; @@ -414,6 +443,14 @@ export type StageEvidence = { prompts?: readonly StagePromptSummaryLike[]; predicateResults?: readonly StagePredicateResultLike[]; toolErrors?: readonly StageToolErrorLike[]; + /** + * The fatal step error's LAYER, when one was raised and the runner knew it. + * + * Distinct from `toolErrors`, which are the server's answers. This is our + * own side breaking, and it is the difference between "the server gave us + * bad data" and "we never got to ask". + */ + stepError?: StageStepErrorLike; renderObservations?: readonly StageRenderObservationLike[]; /** `tools_total_before` / `tools_exposed` — the one direct discovery signal. */ toolSignals?: { toolsTotalBefore?: number; toolsExposed?: number }; @@ -1100,7 +1137,14 @@ function categoryFor( evidence: StageEvidence ): FailureCategory | undefined { if (!firstFailed) { - return evidence.evaluatorErrored ? "evaluator" : undefined; + if (evidence.evaluatorErrored) return "evaluator"; + // A model-call failure leaves nothing failed — there was nothing to fail + // against. Before UVH-IN2 that produced a run with no category at all, + // which reads as "we cannot say what went wrong" when in fact we can say + // precisely: our provider did. `setup` is the existing bucket for our own + // side breaking, which is why this needs no new category. + if (evidence.stepError?.source === "model") return "setup"; + return undefined; } const failedRow = rows.find((r) => r.stage === firstFailed); switch (firstFailed) { @@ -1278,11 +1322,27 @@ export function deriveStageResults( // those measured rows with "never ran" destroys the evidence an operator // needs and states something the run disproves. `firstFailedStage` already // carries "where the chain broke" — the rows do not have to lie to say it. - const firstFailedIndex = derived.findIndex((r) => r.state === "failed"); + // WITHDRAWN FIRST, then cascaded — the order matters and getting it wrong + // produces a chain that argues with itself. + // + // The cascade reads `failed` rows to decide which later stages "never ran". + // Withdrawing a provider-blocked failure AFTER it has run leaves the + // downstream rows still saying `earlierStageFailed` while no stage failed + // and no `firstFailedStage` exists — three rows citing a failure the chain + // no longer records. Running the withdrawal first means the cascade sees the + // rows as they will actually be reported, so a provider outage marks the + // later stages `providerError` (which is why they were not measured) rather + // than blaming a stage that is no longer failed. + // + // A failure the provider did NOT explain still cascades exactly as before: + // `unexpectedToolCall` at `selection` survives the withdrawal, stays the + // first failed row, and the stages after it still read `notReached`. + const withdrawn = applyProviderError(derived, evidence); + const firstFailedIndex = withdrawn.findIndex((r) => r.state === "failed"); const rows = firstFailedIndex < 0 - ? derived - : derived.map((r, i) => + ? withdrawn + : withdrawn.map((r, i) => i > firstFailedIndex && r.state === "notMeasured" ? row(r.stage, "notReached", "earlierStageFailed") : r @@ -1323,11 +1383,114 @@ function mergeMetadataAttributionEvidence( ); } +/** + * Reasons a stage reported NOTHING, which a model-call failure explains. + * + * Each reads as "we looked and the server told us nothing" — an accusation, + * when the truth is that our own provider never let us ask. + */ +const PROVIDER_BLANKED_REASONS: ReadonlyArray = [ + "noEvidenceCaptured", + "traceAbsent", + "executorEmitsNoSpans", +]; + +/** + * Failure reasons that conclude from something NOT HAPPENING. + * + * This is the distinction that decides whether a `failed` row survives a + * provider outage, and it is the whole of the second half of this function. + * + * An ABSENCE verdict — no tool call arrived, an assertion over the output did + * not hold, the judge scored a transcript low — is only sound if the run was + * allowed to finish. When our own model call died first, "it did not happen" + * has a second explanation that outranks the accusation, and we cannot tell + * which is true. The honest answer is that we did not measure it. + * + * PRESENCE verdicts are deliberately absent from this list and keep their + * rows: an unexpected call was really made, arguments really mismatched, a + * tool really errored, a render really failed. Those observations stand + * whatever killed the turn afterwards. `connectFailed` and `toolsListFailed` + * matter most here — they happen BEFORE any model call, so a server that would + * not connect must never be excused by a provider error that came later. + */ +const PROVIDER_UNKNOWABLE_FAILURES: ReadonlyArray = [ + "missingToolCall", + "predicateFailed", + "judgePartial", + "judgeFailed", +]; + +/** + * Re-label what a MODEL-CALL failure made unknowable. + * + * Applied BEFORE the positional cascade, and again in `finalize` for the + * early-return paths that never reach it — idempotent, so the second pass over + * already-converted rows finds nothing to do. An earlier revision of this + * docblock said "applied last"; that was true until the withdrawal had to move + * ahead of the cascade, which reads `failed` rows to decide which later stages + * never ran. Withdrawing after it ran left three rows citing a failure the + * chain no longer recorded. See the comment at the call site. + * + * Two parts. + * + * BLANK ROWS. A stage that measured nothing is re-labelled, while a stage with + * its own evidence keeps its own row: the provider dying at turn 4 does not + * un-observe what turns 1-3 established. + * + * FAILED ROWS THAT REST ON AN ABSENCE. The first version of this stopped at + * blank rows, and that left the fix inert on the shape it matters most for. A + * case expecting a tool call whose provider died first still had + * `selection: failed / missingToolCall` written by the matcher before this + * ever ran — so `firstFailedStage` stayed `selection`, `categoryFor` returned + * `selection`, and the outage was filed as a model-selection defect. The one + * thing this reason exists to prevent, on the most common case in the corpus. + * + * `notMeasured` throughout, never `failed`. A run that could not be attempted + * has measured nothing about the server, and inflating a server failure rate + * with our own outage is exactly what this reason exists to prevent. + */ +function applyProviderError( + rows: StageResultRow[], + evidence: StageEvidence +): StageResultRow[] { + if (evidence.stepError?.source !== "model") return rows; + return rows.map((r) => { + if ( + r.state === "notMeasured" && + PROVIDER_BLANKED_REASONS.includes(r.reason) + ) { + return { ...r, reason: "providerError" as const }; + } + if ( + r.state === "failed" && + PROVIDER_UNKNOWABLE_FAILURES.includes(r.reason) + ) { + // The EVIDENCE goes with the verdict it supported. Those lines say why + // the absence was judged a failure, and that judgement is exactly what + // is being withdrawn — keeping them would leave a `notMeasured` row + // arguing for a failure it no longer claims. + const { evidence: _dropped, ...rest } = r; + return { + ...rest, + state: "notMeasured" as const, + reason: "providerError" as const, + }; + } + return r; + }); +} + function finalize( rows: StageResultRow[], evidence: StageEvidence, forcedCategory?: FailureCategory ): StageDerivation { + // IDEMPOTENT, and applied here as well as before the positional cascade: the + // early-return paths above never reach that cascade, so this is the only + // place they get it. A second pass over rows the first already converted + // finds nothing left to change — `providerError` is in neither list. + rows = applyProviderError(rows, evidence); const firstFailedStage = rows.find((r) => r.state === "failed")?.stage; const failureCategory = forcedCategory ?? categoryFor(firstFailedStage, rows, evidence); diff --git a/sdk/tests/fixtures/eval-run-decision-summary-fixtures.json b/sdk/tests/fixtures/eval-run-decision-summary-fixtures.json index 110060d901..f29f8600f5 100644 --- a/sdk/tests/fixtures/eval-run-decision-summary-fixtures.json +++ b/sdk/tests/fixtures/eval-run-decision-summary-fixtures.json @@ -2469,7 +2469,7 @@ "analyzerVersion": 99, "analyzerVersionAhead": { "reported": 99, - "known": 7 + "known": 8 } }, "expected": { diff --git a/sdk/tests/fixtures/parity/v1/MANIFEST.json b/sdk/tests/fixtures/parity/v1/MANIFEST.json index 049f146b9f..470fb78f07 100644 --- a/sdk/tests/fixtures/parity/v1/MANIFEST.json +++ b/sdk/tests/fixtures/parity/v1/MANIFEST.json @@ -27,104 +27,104 @@ "maxIterations": 200, "maxBytes": 2097152 }, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "iterations": 19, "bytes": 34215, - "corpusDigest": "878f6d376c5370c13aa5c35e73ee7b9ea96b41bb4467f791892245ca7a819321", + "corpusDigest": "53b9c34d077cbeecce0e1ac29d3f42c285b2c554e88b1c5cb62965dd059fe6a0", "files": [ { "path": "iteration-0001.json", - "sha256": "0dbc450eea310da28f20936a9237ce6680602d1e60c27158f713464dbdb536b7", + "sha256": "79253c9a3716b5ded0c5223088936038a14926a08be78648d813f6b6ce8353dd", "bytes": 1964 }, { "path": "iteration-0002.json", - "sha256": "002a69070e2692549173a0e15203d95d948ee48a4914d583aa507e9fa08b63a4", + "sha256": "3ec5c4e90fa615c46f3984e9e06f3b89421392dc5e242c5f33ec76983bb6161c", "bytes": 2254 }, { "path": "iteration-0003.json", - "sha256": "d5e3dacda886713ae9d7280de56b2b11d712dd3cdaebdf7ddf97be825b6a8b69", + "sha256": "6b0d0b96c651cc51c38b2dbc9a72e90763a0dbfdafed62ae60535fb17d2514ed", "bytes": 1798 }, { "path": "iteration-0004.json", - "sha256": "a0a8f56d21e166dca2bed0b9ab2b70437fa8f775b295c6d24e72a0d0da55d7bc", + "sha256": "6eb831aac09f403a02369e84d615af8c3fd10f79b7b2a561cbcc9765c9aba2e7", "bytes": 1986 }, { "path": "iteration-0005.json", - "sha256": "355cf2c77f69aeabb3a3c204ec69e951a34cdb4dc60dbd7d9f128c5f51d8b297", + "sha256": "ea441a999f66ddb8b043add4299bd325f1bfc2ab630e67e8c5e5ba330dc5da5a", "bytes": 1933 }, { "path": "iteration-0006.json", - "sha256": "59865c00ee14c90d289961d613235c9a6b7b342ba8b82443f13e0259713cda38", + "sha256": "74534c971315c8c2ddea649ad8e458b57f8a12b236aa0ae7baaad0ad1cb98c27", "bytes": 1510 }, { "path": "iteration-0007.json", - "sha256": "85fdd897c33cf41ca5789c46f79c9491d2abe9701b2d7e00631d4ed65cf37f03", + "sha256": "e4f9697ad5d36a0d351d53c8ed24a5264fa9b3c23c4c8743eeb3246728900983", "bytes": 1512 }, { "path": "iteration-0008.json", - "sha256": "936168e81f242bc01f19d5ea6218901821533cf8be7247cbe32eaab6346a0578", + "sha256": "02cb20b659c1b20696a9e9501836aba4070713b21e16ff3cfd7d946357aea81f", "bytes": 1202 }, { "path": "iteration-0009.json", - "sha256": "b34b97aa9c370e656e9bc69b81d54eff38646f0e4741b631d261722cfcad1775", + "sha256": "8c1283b6ffda30bce6dfed9dedb5224c0da672f5c2df85061d84371aed87ac22", "bytes": 1583 }, { "path": "iteration-0010.json", - "sha256": "e77240046df9a0c992cd155a471cff741fb4772faeed1c388d99502d259f1ea6", + "sha256": "1ea1f568ea41f821a05bd96f163b0ab8b2b02fa3926a6f36559b7f81c2a14919", "bytes": 1602 }, { "path": "iteration-0011.json", - "sha256": "ef36ec48540c87d6100429d4641a17c219af841be25c4ce1541c20e6878eac9e", + "sha256": "e29deff5e828e451764531a8d83349a23adb568a31894fb470d4c5cb332590e6", "bytes": 2201 }, { "path": "iteration-0012.json", - "sha256": "2383fbad01107d300decb40fed1211d4a7696b71272d1835b770bff654366c58", + "sha256": "1335e6c6d80ce303e596c3824e4ca71bdb01d8f3696fa9877d997f34bb6f3209", "bytes": 1616 }, { "path": "iteration-0013.json", - "sha256": "2c24f860be94f279b3acd94845898b3419533e2a95511f992bf5aa68a29860fd", + "sha256": "e8191ee2dbe37d0fdae40faa0e62172fb9a596086cee57f0f8acab407b670c72", "bytes": 1878 }, { "path": "iteration-0014.json", - "sha256": "fb0eb96bc62f721bfd35027f12f78fbae3cbb8798a04111c3bc1588ff2fb487c", + "sha256": "a233cfa58443aa1d81f27202193abb36730978c0acfdae3971bf5be67075e3cc", "bytes": 1874 }, { "path": "iteration-0015.json", - "sha256": "12f5c4dc972aabd9bc01c8e095426064ad9c48fdcafd90d6918590ba5f64b6b6", + "sha256": "070ecee8d19ddad4fbefa5e22a9d6fcd06db2e9cfe4a53488efc5a11a1d53892", "bytes": 1846 }, { "path": "iteration-0016.json", - "sha256": "7d9f4f5dc8080c598fe8b2eaf93c83763550ffed7ae62562685a347a2625ae29", + "sha256": "64d3a700ce158e1e58d59d7a01013130fee480b2cdbc9ea29df21fb43b6810ec", "bytes": 1821 }, { "path": "iteration-0017.json", - "sha256": "e30bd72bd1596bc4a21f74aed893a46f9b07db67373514b4fe31f457c7d836f4", + "sha256": "5e67b557a53c68c0ce87e020cc1f1c590010c68045641d66ec83e043fadad607", "bytes": 1971 }, { "path": "iteration-0018.json", - "sha256": "a46e1490142534f2c8484639df05dfea300ca0e2dad2cfd0d8df7a4355f38cf0", + "sha256": "1dc27f05db7a8c496d58630f64f47a48c507ff964501b4b6bd0efa7b75e50609", "bytes": 1833 }, { "path": "iteration-0019.json", - "sha256": "5285863ba6cb805ee0e866ceb26ee811b5052dba7dfb69477ac1b5b78bc5b6a7", + "sha256": "f120eb03496d89b3b787b373e173264713df898cec2e0ab86e3989cc1503f6a8", "bytes": 1831 } ], diff --git a/sdk/tests/fixtures/parity/v1/iteration-0001.json b/sdk/tests/fixtures/parity/v1/iteration-0001.json index 02b72ee663..dbfd26ff64 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0001.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0001.json @@ -97,6 +97,6 @@ "reason": "observed" } ], - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0002.json b/sdk/tests/fixtures/parity/v1/iteration-0002.json index 671e58dd23..2b2d649261 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0002.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0002.json @@ -108,6 +108,6 @@ ], "firstFailedStage": "userValue", "failureCategory": "userValue", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0003.json b/sdk/tests/fixtures/parity/v1/iteration-0003.json index 0bd186321c..12bd18d709 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0003.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0003.json @@ -85,6 +85,6 @@ ], "firstFailedStage": "selection", "failureCategory": "selection", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0004.json b/sdk/tests/fixtures/parity/v1/iteration-0004.json index 643b425a4d..d99fcb15a4 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0004.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0004.json @@ -95,6 +95,6 @@ ], "firstFailedStage": "call", "failureCategory": "serverData", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0005.json b/sdk/tests/fixtures/parity/v1/iteration-0005.json index 4a0b054d2b..151e885fe8 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0005.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0005.json @@ -91,6 +91,6 @@ ], "firstFailedStage": "call", "failureCategory": "setup", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0006.json b/sdk/tests/fixtures/parity/v1/iteration-0006.json index 3b76d6e3e8..7d1557d32c 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0006.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0006.json @@ -65,6 +65,6 @@ ], "firstFailedStage": "connection", "failureCategory": "setup", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0007.json b/sdk/tests/fixtures/parity/v1/iteration-0007.json index 12d81432c4..e5bf06e90f 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0007.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0007.json @@ -67,6 +67,6 @@ ], "firstFailedStage": "discovery", "failureCategory": "setup", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0008.json b/sdk/tests/fixtures/parity/v1/iteration-0008.json index 260069371d..603051bc62 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0008.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0008.json @@ -55,6 +55,6 @@ "reason": "traceAbsent" } ], - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0009.json b/sdk/tests/fixtures/parity/v1/iteration-0009.json index d6bd8710c9..85666d705a 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0009.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0009.json @@ -75,6 +75,6 @@ "reason": "observed" } ], - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0010.json b/sdk/tests/fixtures/parity/v1/iteration-0010.json index 1e8d5b7bc3..094f72b5f7 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0010.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0010.json @@ -77,6 +77,6 @@ "reason": "observed" } ], - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0011.json b/sdk/tests/fixtures/parity/v1/iteration-0011.json index 3b6278a1c8..7df75ff5cb 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0011.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0011.json @@ -106,6 +106,6 @@ ], "firstFailedStage": "selection", "failureCategory": "selection", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0012.json b/sdk/tests/fixtures/parity/v1/iteration-0012.json index 078ef955ea..9266dbc53b 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0012.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0012.json @@ -80,6 +80,6 @@ "reason": "observed" } ], - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0013.json b/sdk/tests/fixtures/parity/v1/iteration-0013.json index 70d227da83..be8d3c0804 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0013.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0013.json @@ -94,6 +94,6 @@ "reason": "observed" } ], - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0014.json b/sdk/tests/fixtures/parity/v1/iteration-0014.json index dc327d7faa..01d1209e6f 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0014.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0014.json @@ -91,6 +91,6 @@ ], "firstFailedStage": "response", "failureCategory": "serverData", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0015.json b/sdk/tests/fixtures/parity/v1/iteration-0015.json index d78b7171e1..63c27ad90d 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0015.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0015.json @@ -90,6 +90,6 @@ } ], "failureCategory": "evaluator", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0016.json b/sdk/tests/fixtures/parity/v1/iteration-0016.json index b34d8605e1..8b5c905204 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0016.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0016.json @@ -88,6 +88,6 @@ "reason": "notAuthored" } ], - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0017.json b/sdk/tests/fixtures/parity/v1/iteration-0017.json index 3d1923830b..7d903ed1e0 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0017.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0017.json @@ -95,6 +95,6 @@ ], "firstFailedStage": "call", "failureCategory": "arguments", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0018.json b/sdk/tests/fixtures/parity/v1/iteration-0018.json index 2c48c2506d..47a041d68e 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0018.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0018.json @@ -85,6 +85,6 @@ ], "firstFailedStage": "selection", "failureCategory": "selection", - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/parity/v1/iteration-0019.json b/sdk/tests/fixtures/parity/v1/iteration-0019.json index 39b266116a..8f41d9ce66 100644 --- a/sdk/tests/fixtures/parity/v1/iteration-0019.json +++ b/sdk/tests/fixtures/parity/v1/iteration-0019.json @@ -88,6 +88,6 @@ "reason": "noEvidenceCaptured" } ], - "stageAnalyzerVersion": 7 + "stageAnalyzerVersion": 8 } } diff --git a/sdk/tests/fixtures/stage-analytics-golden.json b/sdk/tests/fixtures/stage-analytics-golden.json index 5c4466c109..b94ff5df57 100644 --- a/sdk/tests/fixtures/stage-analytics-golden.json +++ b/sdk/tests/fixtures/stage-analytics-golden.json @@ -11,7 +11,7 @@ "runCompletedAt": 1700000100000, "sourceIterationCount": 7, "sourceMaxUpdatedAt": 1700000090000, - "stageAnalyzerVersion": 7, + "stageAnalyzerVersion": 8, "measurementsSchemaVersion": 1, "materializationState": "provisional", "createdAt": 1700000150000, diff --git a/sdk/tests/stage-derivation.test.ts b/sdk/tests/stage-derivation.test.ts index 789cbd293f..c7003f2d69 100644 --- a/sdk/tests/stage-derivation.test.ts +++ b/sdk/tests/stage-derivation.test.ts @@ -12,6 +12,7 @@ */ import { describe, expect, test } from "vitest"; +import { STAGE_REASON_LABELS } from "../src/contract/decision-labels.js"; import { finalizePassedForEval } from "../src/eval-tool-execution"; import { MAX_EVIDENCE_REASONS, @@ -782,6 +783,220 @@ describe("userValue", () => { }); }); +// ── UVH-IN2: a model-call failure is OURS, not the server's ────────────────── +// +// 20 prod trials failed on "credit balance too low… Anthropic API": the step +// errored, asserts were skipped, and the chain ended `noEvidenceCaptured` with +// NO failure category — an outage filed as an unattributed server failure. + +describe("a model-call failure is attributed, not left blank", () => { + const providerDied = { stepError: { source: "model" as const } }; + + test("blank stages read as providerError, and the run is categorised setup", () => { + const { stageResults, failureCategory, firstFailedStage } = derive({ + evidence: { traceAbsent: true, ...providerDied }, + }); + + // Every applicable stage says the same true thing: we never got to ask. + for (const r of stageResults.filter((x) => x.state === "notMeasured")) { + expect(r.reason).toBe("providerError"); + } + // `setup` is the existing bucket for our own side breaking, so no new + // category was needed — but a category there MUST be. + expect(failureCategory).toBe("setup"); + // Never `failed`: our provider's bad day is not the server's defect. + expect(firstFailedStage).toBeUndefined(); + expect(stageResults.some((r) => r.state === "failed")).toBe(false); + }); + + test("stages that DID measure something keep their own rows", () => { + // A provider dying at turn 4 does not un-observe turns 1-3. Only the + // blank rows are re-labelled. + const { stageResults } = derive({ + evidence: { + spans: [toolSpan()], + prompts: [cleanTurn], + ...providerDied, + }, + }); + expect(stateOf(stageResults, "call").state).toBe("passed"); + expect(stateOf(stageResults, "selection").state).toBe("passed"); + }); + + test("a missing call the provider never let us make is not a selection defect", () => { + // THE CASE THIS REASON EXISTS FOR, and the one the first version missed. + // + // A case expecting a tool call whose provider died has + // `selection: failed / missingToolCall` written by the matcher before the + // chain is derived. Re-labelling only BLANK rows left that standing, so + // `firstFailedStage` stayed `selection` and the outage was filed as a + // model-selection defect — the exact misattribution this whole reason was + // built to remove, on the commonest shape in the corpus. + const { stageResults, failureCategory, firstFailedStage } = derive({ + evidence: { + prompts: [{ promptIndex: 0, missing: [{ toolName: "search" }] }], + ...providerDied, + }, + }); + + const selection = stateOf(stageResults, "selection"); + expect(selection.state).toBe("notMeasured"); + expect(selection.reason).toBe("providerError"); + // The evidence went with the verdict it supported: a notMeasured row must + // not still be arguing for a failure it no longer claims. + expect(selection.evidence).toBeUndefined(); + expect(firstFailedStage).toBeUndefined(); + expect(failureCategory).toBe("setup"); + }); + + test("the reason speaks for its own stage, not for the run", () => { + // Review finding on the label. `providerError` is applied PER ROW, so a + // multi-turn iteration whose provider died late keeps its earlier measured + // rows — and a run-level "the run never reached the server" would sit + // directly beside a `call: passed` that disproves it. + const { stageResults } = derive({ + evidence: { + spans: [toolSpan()], + prompts: [cleanTurn], + ...providerDied, + }, + }); + // The precondition that makes the label's scope matter: the server WAS + // reached on this run. + expect(stateOf(stageResults, "call").state).toBe("passed"); + expect(STAGE_REASON_LABELS.providerError).not.toContain( + "never reached the server" + ); + expect(STAGE_REASON_LABELS.providerError).toContain("this stage"); + }); + + test("the chain does not argue with itself after a withdrawal", () => { + // Review finding on the withdrawal itself. The positional cascade reads + // `failed` rows to decide which later stages "never ran", so withdrawing + // the failure AFTER it ran left `call`, `response` and `userValue` saying + // `earlierStageFailed` while no stage failed and no firstFailedStage + // existed — three rows citing a failure the chain no longer records. + const { stageResults, firstFailedStage } = derive({ + evidence: { + prompts: [{ promptIndex: 0, missing: [{ toolName: "search" }] }], + ...providerDied, + }, + }); + + expect(firstFailedStage).toBeUndefined(); + expect(stageResults.some((r) => r.state === "failed")).toBe(false); + // Nothing may still be blaming a stage that is no longer failed. + expect(stageResults.some((r) => r.reason === "earlierStageFailed")).toBe( + false + ); + // And the later stages say the true thing about why they are blank. + for (const stage of ["call", "response", "userValue"] as const) { + const r = stateOf(stageResults, stage); + if (r.state === "notMeasured") expect(r.reason).toBe("providerError"); + } + }); + + test("a failure the provider did NOT explain still cascades", () => { + // The other side. An unexpected call survives the withdrawal, so it stays + // the first failed row and the stages after it still read `notReached` — + // the cascade is repaired, not disabled. + const { stageResults, firstFailedStage } = derive({ + evidence: { + spans: [toolSpan()], + prompts: [ + { + promptIndex: 0, + unexpected: [{ toolName: "delete_all" }], + passed: false, + }, + ], + ...providerDied, + }, + }); + expect(firstFailedStage).toBe("selection"); + const after = stageResults.slice( + stageResults.findIndex((r) => r.stage === "selection") + 1 + ); + expect(after.some((r) => r.state === "notReached")).toBe(true); + }); + + test("a call that really was made wrongly still counts against the server", () => { + // The other side of that line, and the one that keeps this honest. An + // UNEXPECTED call was actually observed — a presence, not an absence — so + // a provider dying afterwards does not un-observe it. Withdrawing this too + // would let any provider blip launder a genuine server defect. + const { stageResults } = derive({ + evidence: { + spans: [toolSpan()], + prompts: [ + { + promptIndex: 0, + unexpected: [{ toolName: "delete_all" }], + passed: false, + }, + ], + ...providerDied, + }, + }); + const selection = stateOf(stageResults, "selection"); + expect(selection.state).toBe("failed"); + expect(selection.reason).toBe("unexpectedToolCall"); + }); + + test("a server that would not connect is never excused by a later outage", () => { + // `connection` fails BEFORE any model call, so a provider error that came + // afterwards cannot explain it. This is the failure mode that would be + // most damaging to launder away. + const { stageResults, firstFailedStage } = derive({ + evidence: { + setupSignals: { + connection: { + outcome: "failed", + attribution: "theirs", + egressVerified: true, + spanIds: ["run-connect-s1"], + }, + }, + ...providerDied, + }, + }); + expect(stateOf(stageResults, "connection").state).toBe("failed"); + expect(firstFailedStage).toBe("connection"); + }); + + test("a SETUP-layer error is not a provider error", () => { + // Pre-turn setup never reached the model, and `setupAborted` already says + // so precisely. Widening `providerError` over it would lose that. + const { stageResults } = derive({ + evidence: { traceAbsent: true, stepError: { source: "setup" } }, + }); + expect(stageResults.every((r) => r.reason !== "providerError")).toBe(true); + }); + + test("an unclassified error changes nothing", () => { + // Callers that cannot say which layer broke leave `stepError` absent, and + // the chain reports exactly what it did before. + const before = derive({ evidence: { traceAbsent: true } }); + expect(before.stageResults.every((r) => r.reason !== "providerError")).toBe( + true + ); + expect(before.failureCategory).toBeUndefined(); + }); + + test("a broken grader still outranks it", () => { + // `evaluator` is never folded into another category — a grader bug is not + // an infrastructure outage, and counting it as one poisons both rates. + const { failureCategory } = derive({ + evidence: { + traceAbsent: true, + evaluatorErrored: true, + ...providerDied, + }, + }); + expect(failureCategory).toBe("evaluator"); + }); +}); + // ── UVH-IN7: an observed tool error makes `response` measurable ────────────── // // The disagreement class this closes: a case authors only transcript