Skip to content

UVH-IN3: name a verdict/chain disagreement instead of calling it missing data - #4494

Open
chelojimenez wants to merge 7 commits into
claude/uvc-mcp-eval-reporting-gyycwl-in4from
claude/uvc-mcp-eval-reporting-gyycwl-in3
Open

UVH-IN3: name a verdict/chain disagreement instead of calling it missing data#4494
chelojimenez wants to merge 7 commits into
claude/uvc-mcp-eval-reporting-gyycwl-in4from
claude/uvc-mcp-eval-reporting-gyycwl-in3

Conversation

@chelojimenez

@chelojimenez chelojimenez commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

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:

inspect the case trace; no failure category was recorded

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 recorded verdict disagrees with the measured chain; inspect the case trace

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:

Condition What it prevents
The chain validated An unverified chain has no stage states to disagree with anything
At least one stage passed A policy-blocked run has all six stages notMeasured / blockedByPolicy — nothing failed, but nothing was measured either, and there is nothing for a verdict to disagree with
No stage 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 flatly contradicted by the row itself
The verdict says failed A trial that recorded no verdict is still diagnosed (only passed is filtered out) and its chain can be entirely green — nothing was decided, so nothing is in conflict

The 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:

…this run's chain predates the analyzer that reports an errored tool call on an unauthored case — re-run the case to 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 = 7 lives in stage-derivation.ts beside 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 move STAGE_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

  • sdk suite: 6897 passed, 8 skipped, 313 files

  • eval-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 --noEmit clean; eslint 0 errors; prettier clean

  • Mutation-tested all five branches. Each mutation was caught by exactly its intended test:

    Mutation Caught by
    drop result !== "failed" "does NOT claim a disagreement when there is no verdict to disagree WITH"
    drop measuredSomething policy-block-is-not-a-failure golden
    drop anyStageFailed "does NOT claim a disagreement over a failed stage with no category"
    drop the stale-analyzer branch "names the KNOWN cause when the chain predates analyzer 7"
    drop chain.status !== "verified" unverified-and-version-ahead golden (both cases)

    The anyStageFailed mutation 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 couple failureCategory to 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 nextAction no longer always says inspect the case trace; no failure category was recorded. uncategorisedNextAction in decision-summary.ts picks 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 passed or notApplicable (a notMeasured stage 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 analyzerVersion below 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/sdk patch changesets document the behavior. Wording/assembly only — schemas, stage semantics, and verdicts are unchanged. eval-run-decision-summary.test.ts adds 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 trace where it previously said no failure category was recorded.

Disagreement wording

  • The disagreement requires a verified chain, a failed verdict, and every applicable stage passed; notApplicable stages are out of scope, but a notMeasured stage is a measurement gap and keeps the old wording.
  • An unverified chain, a policy block, a failed stage without a category, and a trial with no verdict all keep the old fallback wording.
  • Chains derived before analyzer 7 get wording that says the chain was derived by an older analyzer that measures less and to re-run; the wording no longer names a specific cause.

Scope

  • Wording only: the contract stays .strict() at schemaVersion 1, and no stage row, state, reason, category, or verdict changes.
  • No analyzer version bump; ships as @mcpjam/sdk patch changesets.

Written for commit 6c36711. Summary will update on new commits.

Review in cubic

…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
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 30, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-30T21:34:35.955156Z 0b9a540 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cursor

cursor Bot commented Aug 30, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@chelojimenez

chelojimenez commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread sdk/src/contract/decision-summary.ts Outdated
Comment on lines +866 to +867
const measuredSomething = chain.stages.some((row) => row.state === "passed");
const anyStageFailed = chain.stages.some((row) => row.state === "failed");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 noEvidenceCapturedmeasuredSomething 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 👍 / 👎.

@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Internal preview

Preview URL: https://mcp-inspector-pr-4494.up.railway.app
Deployed commit: b67bd33
PR head commit: 6c36711
Backend target: staging fallback.
Health: ✅ Convex reachable
Access is employee-only in non-production environments.

@cursor

cursor Bot commented Aug 30, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +876 to +878
return chain.analyzerVersion !== undefined &&
chain.analyzerVersion < STAGE_ANALYZER_VERSION_EVIDENCE_TRIGGERED_RESPONSE
? DECISION_SUMMARY_STALE_ANALYZER_DISAGREEMENT_NEXT_ACTION

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@cursor

cursor Bot commented Aug 30, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +208 to +209
export const DECISION_SUMMARY_VERDICT_CHAIN_DISAGREEMENT_NEXT_ACTION =
"the recorded verdict disagrees with the measured chain; inspect the case trace";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

claude added 2 commits August 30, 2026 21:24
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
@cursor

cursor Bot commented Aug 30, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

Copy link
Copy Markdown
Contributor Author

Both Codex findings were right, and both are this PR's own failure mode — a claim reaching past its evidence. Fixed in 0b9a540.

The predicate was too weak. "Something passed and nothing failed" is satisfied by a chain with connection and discovery passed and selection notMeasured / noEvidenceCaptured — 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. Both directions are pinned by tests.

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:

the recorded verdict disagrees with the measured chain; this run's chain was derived by an older analyzer that measures less than the current one — re-run the case before investigating further

"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 notApplicable, and restoring the tool-error wording each fail exactly their intended tests. SDK 6,899 passed; CLI gate suite 1,182 passed, 0 failed.


Generated by Claude Code

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@cursor

cursor Bot commented Aug 30, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants