UVH-IN3: name a verdict/chain disagreement instead of calling it missing data - #4494
Conversation
…ing data
When a case failed but the chain recorded no failure category, the decision
summary said one thing: "no failure category was recorded". That covers two
runs that are not alike, and describes the more interesting one wrongly.
If the chain never validated, or measured nothing, information really is
absent. But if the chain validated, every applicable stage came back passed,
and the verdict still says failed, that is not an absence — it is two things
we hold in conflict, and sending someone to look for a missing measurement
sends them after the wrong thing.
The claim is asserted only when all four halves are structurally established:
the chain validated, at least one stage passed, no stage failed, and the
verdict is failed. Each does real work, and each is pinned by a test that
fails when its guard is removed:
- Something must have PASSED. A policy-blocked run has all six stages
notMeasured/blockedByPolicy, so nothing failed — but nothing was measured
either, and there is nothing for a verdict to disagree with.
- Nothing may have FAILED. failureCategory is read off the stored row, and
the derivation schema pins only firstFailedStage to the failed row, never
the category — so a row can validate carrying a failed stage and no
category, and "the chain found nothing wrong" would be contradicted by the
row itself.
- The verdict must say FAILED. A trial that recorded no verdict is still
diagnosed and its chain can be entirely green; nothing was decided, so
nothing is in conflict.
One cause is knowable rather than guessed: a chain derived before analyzer 7
could not report an errored tool call on a case that authored no tool
expectation, so those rows name that cause and say to re-run. Read from the
row's own stageAnalyzerVersion, never inferred from the failure's shape.
Nothing else guesses at a cause — a guess dressed as a finding is what this
vocabulary exists to prevent.
STAGE_ANALYZER_VERSION_EVIDENCE_TRIGGERED_RESPONSE lives beside the analyzer
history it names rather than beside the copy that reads it, so a future bump
is edited where the bump is already being written down.
Wording only: the contract stays .strict() at schemaVersion 1, no stage row,
state, reason or category moves, and no verdict changes. The CLI gate suite
(1182 tests) confirms the neutrality.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4jTtZJsDeaterEzKwpF2p
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2c0fa59f-b5c5-4b4d-a82d-36fcb0521cc5) |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f726ec13c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const measuredSomething = chain.stages.some((row) => row.state === "passed"); | ||
| const anyStageFailed = chain.stages.some((row) => row.state === "failed"); |
There was a problem hiding this comment.
Require all applicable stages to pass before declaring disagreement
When some stages pass but another applicable stage is notMeasured—for example, connection and discovery passed while selection or user value has noEvidenceCaptured—measuredSomething is true and anyStageFailed is false, so this schema-valid chain is labeled as disagreeing with the failed verdict. In that case the chain is incomplete rather than green, and the new action again sends the operator after the wrong condition; the disagreement predicate should require every applicable stage to be passed (while allowing notApplicable), not merely one passed stage and no failed stages.
Useful? React with 👍 / 👎.
Internal previewPreview URL: https://mcp-inspector-pr-4494.up.railway.app |
…vc-mcp-eval-reporting-gyycwl-in3
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e6273b54-0550-45ee-93d7-e3aed5ada786) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 245d35c408
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return chain.analyzerVersion !== undefined && | ||
| chain.analyzerVersion < STAGE_ANALYZER_VERSION_EVIDENCE_TRIGGERED_RESPONSE | ||
| ? DECISION_SUMMARY_STALE_ANALYZER_DISAGREEMENT_NEXT_ACTION |
There was a problem hiding this comment.
Gate the stale-analyzer message on evidence of its cause
For every verified failed row with a passing stage, no failed stage, and an analyzer version below 7, this branch selects wording specifically about an errored tool call on an unauthored case. The version only proves that the analyzer could not detect that scenario, not that it occurred; the new test even reaches this branch by cloning a fixture with an authored expected tool and zero actual tool calls. Consequently, legacy rows with any other uncategorized disagreement receive misleading remediation that implies rerunning will attribute a tool error. Either confirm the relevant tool-error/unauthored-case evidence before selecting this message or retain the generic disagreement action.
Useful? React with 👍 / 👎.
…vc-mcp-eval-reporting-gyycwl-in3
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8310655a-6473-4d5b-9fb3-39b4fa32f8d2) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49df0b0fa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const DECISION_SUMMARY_VERDICT_CHAIN_DISAGREEMENT_NEXT_ACTION = | ||
| "the recorded verdict disagrees with the measured chain; inspect the case trace"; |
There was a problem hiding this comment.
Re-export the new decision-action constants
The package export map exposes this area only through @mcpjam/sdk/contract, whose barrel selectively re-exports the fallback action and category map but not either newly added disagreement constant. Consequently, published SDK consumers cannot import these canonical values—even though diagnostics can now return them—and importing the source module directly is blocked by package.json exports. Add both disagreement constants to sdk/src/contract/index.ts (and the main compatibility barrel if they are intended alongside the existing action exports).
Useful? React with 👍 / 👎.
…vc-mcp-eval-reporting-gyycwl-in3
Two review findings, both a claim reaching past its evidence — the specific failure this wording exists to fix, so getting it wrong here was worse than usual. AN UNMEASURED STAGE MAKES THE CHAIN INCOMPLETE, NOT GREEN. The predicate asked for "something passed and nothing failed", which a chain with connection and discovery passed and selection notMeasured/noEvidenceCaptured satisfies — while the verdict may be failing on exactly the stage the chain could not read. That is a measurement gap, and calling it a conflict sends someone looking for a contradiction that is not there. Every applicable stage must now have passed. `notApplicable` is the one state that does not block the claim: a stage the case never exercises is out of scope rather than missing evidence, and requiring it to pass would make the claim unreachable for any case that does not use all six stages. THE STALE-ANALYZER WORDING NAMED AN UNPROVEN CAUSE. It said the chain "predates the analyzer that reports an errored tool call on an unauthored case — re-run the case to attribute it". The version proves only what the analyzer was ABLE to see; it is not evidence that such a call occurred. Every legacy row with any other uncategorised disagreement was being sent after one specific finding. It now says only what the row establishes — the chain was derived by an analyzer measuring strictly less than the current one, so re-deriving may attribute what this one could not. "Re-run" stays a real instruction with no cause attached to it. Mutation-checked: reverting to the weak predicate, refusing notApplicable, and restoring the tool-error wording each fail exactly their intended tests. SDK 6899 passed; CLI gate suite 1182 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4jTtZJsDeaterEzKwpF2p
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_cd0f778d-5d08-41c3-b621-bcc77e4b4bac) |
|
Both Codex findings were right, and both are this PR's own failure mode — a claim reaching past its evidence. Fixed in The predicate was too weak. "Something passed and nothing failed" is satisfied by a chain with connection and discovery Every applicable stage must now have The stale-analyzer wording named a cause the version does not establish. Exactly right — the version proves only what the analyzer was able to see, not that an errored tool call occurred. Every legacy row with any other uncategorised disagreement was being sent after one specific finding. It now says only what the row establishes:
"Re-run" stays a real instruction with no cause attached to it, and a test asserts the line does not contain "tool". Mutation-checked: reverting to the weak predicate, refusing Generated by Claude Code |
…vc-mcp-eval-reporting-gyycwl-in3
…vc-mcp-eval-reporting-gyycwl-in3
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e27e3ac2-bd05-4fe3-95d3-1ad5a7bd32cb) |
Stacked on #4493 (UVH-IN4). Review only the top commit; the base carries the rest of the stack.
The problem
When a case failed but the chain recorded no failure category, the decision summary said exactly one thing:
That sentence covers two runs that are not alike, and it describes the more interesting one wrongly.
If the chain never validated, or measured nothing, then information really is absent and there is nothing more to say. But if the chain validated, every applicable stage came back
passed, and the recorded verdict still says failed — that is not an absence. It is two things we hold in conflict, and sending someone to look for a missing measurement sends them after the wrong thing.That run now reads:
The claim is asserted only when it is structurally established
Four conditions, each doing real work, each pinned by a test that fails when its guard is removed:
passednotMeasured / blockedByPolicy— nothing failed, but nothing was measured either, and there is nothing for a verdict to disagree withfailedfailureCategoryis read off the stored row, and the derivation schema pins onlyfirstFailedStageto the failed row, never the category — so a row can validate carrying afailedstage and no category, and "the chain found nothing wrong" would be flatly contradicted by the row itselffailedpassedis filtered out) and its chain can be entirely green — nothing was decided, so nothing is in conflictThe second and third are not hypotheticals I invented for symmetry. The policy-block golden fixture rejected an earlier, looser predicate that omitted the "something passed" half; and the schema's own refinements were read to establish that a failed stage can validate without a category.
One cause is knowable rather than guessed
A chain derived before analyzer 7 could not report an errored tool call on a case that authored no tool expectation — every applicable stage green, the verdict red, and no row in which to say why. That is the exact defect UVH-IN7 (#4491) fixed. For those rows the wording names the cause and says to re-run, because a newer analyzer will attribute it:
Read from the row's own
stageAnalyzerVersion, never inferred from the shape of the failure. Nothing else guesses at a cause. The assembler cannot see one from here, and a guess dressed as a finding is exactly what this vocabulary exists to prevent.STAGE_ANALYZER_VERSION_EVIDENCE_TRIGGERED_RESPONSE = 7lives instage-derivation.tsbeside the analyzer history it names, not beside the copy that reads it, so a future bump is edited where the bump is already being written down.Scope
Wording only. The contract stays
.strict()at schemaVersion 1. No stage row, state, reason or category moves. No verdict changes. No analyzer bump — this PR is the only one in the stack that does not moveSTAGE_ANALYZER_VERSION.Post-IN7 this fires rarely by design: it now serves legacy-analyzer rows and residual unknowns. Worth stating plainly rather than presenting as a common case.
Verification
sdksuite: 6897 passed, 8 skipped, 313 fileseval-run-decision-summary.test.ts: 85 passed (5 new)CLI gate suite: 1182 passed, 0 failed — verdict/gate neutrality, as expected for a copy-only change
tsc --noEmitclean; eslint 0 errors; prettier cleanMutation-tested all five branches. Each mutation was caught by exactly its intended test:
result !== "failed"measuredSomethingpolicy-block-is-not-a-failuregoldenanyStageFailedchain.status !== "verified"unverified-and-version-aheadgolden (both cases)The
anyStageFailedmutation initially survived — no test made it load-bearing. Rather than delete the guard I checked whether the shape it guards is reachable, found that the derivation schema deliberately does not couplefailureCategoryto the failed row, and added the test. That is the third time this discipline has found something real in this stack.The new tests build their input by cloning a real golden fixture and overriding only the iteration's chain, so the surrounding envelope stays what the assembler is fed in production rather than a hand-rolled approximation of it.
Generated by Claude Code
Note
Low Risk
Copy and conditional next-action selection in the decision summary assembler only; no schema, analyzer version, or verdict logic changes, with broad test coverage on the new branches.
Overview
When a failed trial has no failure category, diagnostic
nextActionno longer always says inspect the case trace; no failure category was recorded.uncategorisedNextActionindecision-summary.tspicks among three operator strings instead of always using the fallback.Verdict vs chain disagreement — If the chain is verified, the trial failed, at least one stage passed, and every applicable stage is
passedornotApplicable(anotMeasuredstage is treated as a measurement gap, not a clean chain), the summary says the recorded verdict disagrees with the measured chain and to inspect the trace. Unverified chains, policy blocks, failed stages (with or without category), and trials with no verdict keep the old fallback.Legacy analyzer rows — Chains with
analyzerVersionbelow 7 (STAGE_ANALYZER_VERSION_EVIDENCE_TRIGGERED_RESPONSE) get wording that the chain came from an older analyzer that measures less and to re-run — without asserting a specific cause (e.g. tool error).New copy lives in
decision-labels.ts;@mcpjam/sdkpatch changesets document the behavior. Wording/assembly only — schemas, stage semantics, and verdicts are unchanged.eval-run-decision-summary.test.tsadds UVH-IN3 cases for each guard.Reviewed by Cursor Bugbot for commit 6c36711. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Changes the decision summary so a failed verdict on a fully measured, green chain reads as a disagreement between the verdict and the chain, not as missing data. The summary now says
the recorded verdict disagrees with the measured chain; inspect the case tracewhere it previously saidno failure category was recorded.Disagreement wording
failedverdict, and every applicable stagepassed;notApplicablestages are out of scope, but anotMeasuredstage is a measurement gap and keeps the old wording.failedstage without a category, and a trial with no verdict all keep the old fallback wording.Scope
.strict()at schemaVersion 1, and no stage row, state, reason, category, or verdict changes.@mcpjam/sdkpatch changesets.Written for commit 6c36711. Summary will update on new commits.