Review verdict says PASS with diff 10/10 on a sync PR that cannot build — no cross-file reachability check, and the reviewer never learns the run errored

[mmcky]

On QuantEcon/lecture-python.zh-cn#202 — the first organic production sync on v0.23.0 — the translation review returned PASS with diff quality 10/10, scopeCorrect ✅, structurePreserved ✅ and headingMapCorrect ✅, on a PR whose _toc.yml names a document that is in neither the PR nor the target repo, and whose build fails.

"diffChecks": { "scopeCorrect": true, "positionCorrect": true, "structurePreserved": true, "headingMapCorrect": true },
"diffCheckSources": { "scopeCorrect": "model", "positionCorrect": "model", "structurePreserved": "deterministic", "headingMapCorrect": "deterministic" },
"engineVersion": "0.23.0"

The underlying defect is #156 (a partial run committed a TOC entry for a file it refused to write). This issue is about the second-order failure: nothing downstream noticed.

Two distinct gaps

1. No deterministic reachability check. structurePreserved and headingMapCorrect are deterministic as of #148, but both are within-file invariants. Nothing checks the cheapest cross-file invariant there is: every _toc.yml entry, and every {doc}/{numref}/{cite} reference introduced by the diff, should resolve against the union of (target repo ∪ this PR). That is a pure filesystem-and-regex check with no model call, and it would have caught six dangling references here.

2. The reviewer never learns the sync run errored. Review runs against the committed diff with no channel carrying "this PR is the output of a run that failed on one of ten files". Even a perfect reviewer of the diff-as-committed cannot see that, because the evidence lives in the source repo's run log and its auto-filed issue, not in the diff.

Why this matters beyond one PR

This is the first organic production data point for #147, and it is a PASS on a PR that cannot build. The gating floors in verdict v2 exist to stop exactly this from reaching auto-merge. Had the routing been auto-merge rather than editor, this would have landed a broken build on main — it routed to editor only because of unrelated terminology findings, which is to say the thing that saved it was a coincidence.

For Stage 4 calibration the consequence is sharper than a single miss: any base rate fitted on diff-check scores currently treats "PR builds" as out of scope, so the floors will be fitted against a quality signal that is uncorrelated with the most consequential failure mode we have actually observed in production.

Suggested fix

Add the reachability check as a deterministic diff-check (a fifth entry alongside structurePreserved, sourced deterministic), and thread the run's error set into the review input so a partial sync cannot be scored as if it were complete. The first is worth doing regardless of the second — it is cheap, it has no model dependency, and it generalises past this incident to any PR that introduces a reference faster than the asset it points at (see #117).

[mmcky]

Cross-links for this incident.

Origin: found during #147's verdict-v2 production-validation window — that issue has now closed as completed, and this is one of the three live reviewer defects it produced (with #187 and #135). Worth recording, because "review passed a PR that cannot build" is exactly the kind of finding a validation exercise exists to surface.

Same incident, other layer: #156. The sync committed a _toc.yml entry for a file the parity guard had correctly refused to write; this issue is why nothing caught it afterwards. #156 is the repair (transactional file-set semantics — a failed file withdraws its TOC and state entries, or the run fails); this is the detection. Both should be scheduled together.

Same detector, different defect: #117. The check proposed here — every _toc.yml entry and every {doc}/{cite} introduced by the diff must resolve against target-repo ∪ PR — is also the deterministic detector for #117's dangling .bib keys and missing _admonition/* includes, which today reach production and fail the strict build (-n -W) with no reviewer signal. Building it once covers both classes.

[mmcky]

Third field instance, and it argues the check should run in both directions

QuantEcon/lecture-python.zh-cn#208 (v0.24.0) adds lectures/olg_adaptive_money.md at +1263 lines and never touches lectures/_toc.yml. Merged as-is, the target gains a full lecture that is in no toctree: not built, not linked, invisible to readers, while coverage metrics improve.

That is the exact inverse of the case this issue was opened on. #202 and #207 had a _toc.yml entry with no file; QuantEcon/lecture-python.zh-cn#208 has a file with no _toc.yml entry. Both are unreachability, and both would be caught by one pass over the same data.

The check as specified above validates one direction:

every _toc.yml entry, and every {doc}/{numref}/{cite} reference introduced by the diff, should resolve against the union of (target repo ∪ this PR)

Suggest adding the converse: every markdown file the PR adds under lectures/ should be reachable from _toc.yml (again as the union of target repo ∪ PR). Same filesystem-and-regex pass, no model call, no extra inputs.

The forward direction fails loudly — a dangling _toc.yml entry breaks the build under -W, so the estate finds out eventually even without this check. The reverse direction fails silently: an orphaned lecture builds green, reviews clean, and simply never appears on the site. QuantEcon/lecture-python.zh-cn#208's verdict is PASS with structurePreserved ✅ and headingMapCorrect ✅, both of which are correct, because both are within-file invariants and the defect is cross-file. Nothing else in the pipeline looks at reachability at all.

Worth noting for the calibration argument already in this issue: QuantEcon/lecture-python.zh-cn#208's diff block did score 7.5 with scopeCorrect: ❌, so this PR was not scored as clean — but for the wrong reason. The reviewer argued olg_adaptive_money.md "already existed in the source's Before state, so it should have been an update, not a fresh addition", reasoning from the source when the relevant fact is the target (the file is 404 in lecture-python.zh-cn@main; it is a genuine coverage gap). It never mentioned the missing _toc.yml entry. So the one number that flagged this PR is uncorrelated with the actual defect — which is the same failure mode this issue already describes for QuantEcon/lecture-python.zh-cn#202, just with the sign flipped.

Three field instances now, in both directions. The upstream cause of QuantEcon/lecture-python.zh-cn#208's case is filed separately in #222.