Skip to content

Verdict v2 diffChecks are model output but gate absolutely — a hallucinated boolean routes a correct PR to editor, and biases Stage 4 calibration #148

Description

@mmcky

The four diffChecks gate the auto-merge recommendation absolutely — review-verdict.ts says so in its own comment ("the diffChecks booleans, which gate absolutely on their own") and enforces it at L403–405, where any of the four not being exactly true appends a blocking reason. That fail-closed shape is right for missing data. The problem is that these booleans are model output, not deterministic checks: reviewer.ts asks for them as JSON in the diff-quality prompt's response format (~L1246), so a confidently wrong boolean from the reviewer is indistinguishable, to the gate, from a genuine structural failure.

This is no longer hypothetical. It fired on the second organic production sync PR ever reviewed under verdict v2.

What happened

QuantEcon/lecture-python.myst#975 changed perm_income.md to the NumPy Generator API. The sync produced QuantEcon/lecture-python.zh-cn#199, which was reviewed as PASS, overall 8.5, and routed to editor with exactly one reason: diff check failed: positionCorrect. It was accompanied by three minor/structure findings all asserting the same thing — that the source's code changes were never applied to the target. One of them reads: "The reported diff stats (+5/-5) do not match any visible content change between the target's before and after markdown, suggesting the actual code edits were not applied."

That is false. Diffing the source PR against the target PR shows the target carries the identical change: def time_path(T)def time_path(T, rng), np.random.randnrng.standard_normal, the added rng = np.random.default_rng(), and both call sites updated. The sync did its job correctly and the reviewer reported the opposite.

For contrast, the first organic PR (QuantEcon/lecture-python.zh-cn#198) was routed to editor for a genuinely correct reason — the glossary specifies 边缘分布 for "marginal distribution" and the translation used 边际分布 throughout. So on the first two organic production PRs the gate was right once and wrong once.

It is not an obvious input-construction bug

Worth stating, because it was my first hypothesis and it is wrong. I checked the prompt construction before filing:

Checked Finding
Code stripped from the prompt? No — full documents are interpolated
Documents truncated? No truncation of sourceBefore/sourceAfter/targetBefore/targetAfter
targetBefore vs targetAfter accidentally identical? No — targetBefore is fetched at pr.base.sha, the after-content at pr.head.sha

So the reviewer was shown a correct before/after pair containing the change, and still concluded the change was absent. This is model error on the data it was given, not a plumbing defect.

A likely contributing factor

positionCorrect is specified in the prompt as "Do changes appear in the same sections as source? Section order should match." That is a prose-oriented criterion. #199's diff is entirely code — a code-cell edit plus the .translate/state frontmatter. There is no prose movement to assess, so the criterion is being applied outside the shape it was written for. A code-only diff may be a systematic weak spot rather than a one-off, which is testable: run the reviewer over a set of code-only sync diffs and measure how often positionCorrect comes back false.

Why this matters now rather than later

Not because of one bad review — the direction is safe, it fails toward a human, and a spurious editor costs reviewer time rather than correctness.

It matters because of Stage 4 calibration (QuantEcon/project-translation PROJECT-PLAN-HUMAN-REVIEWS.md). The whole point of shadow mode is to choose the per-criterion floors and measure the auto-merge base rate from real data. If a meaningful share of editor routings are reviewer error rather than translation defects, then the base rate reads lower than it truly is, and floors get calibrated against noise. Worse, the two are indistinguishable in the shadow data unless someone diffs each PR by hand — which is exactly what nobody will do at volume. The measurement instrument has an error term that the measurement cannot see.

This also lines up with the plan's own position in WS3, which already argues that the risk classifier must be "deterministic … uninfluenceable by any model output". The same argument applies to anything else that gates absolutely.

Suggested directions

Not prescriptive — the right fix depends on how much of this is recoverable deterministically.

Make the checks deterministic where they can be. scopeCorrect is comparing changed-file sets, which is computable. positionCorrect — "changes appear in the same sections" — is largely computable from the existing section parser plus the heading map. headingMapCorrect is already about a structure the engine writes itself. These need not be model judgements at all.

Separate "the check failed" from "the model says the check failed." A model-asserted diff-check failure could be recorded as a gating finding with its own category, so it still routes to a human, while the boolean itself is reserved for deterministic results. That keeps fail-closed behaviour but stops model error from being silently laundered into the same field that Stage 4 will treat as ground truth.

At minimum, measure it before trusting it. If the checks stay model-driven into shadow mode, the shadow data needs a way to distinguish a diff-check gate from a findings gate, so their precisions can be reported separately rather than blended into one base rate.

Refs

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions