emrg: take a guard's red verdict from the guard's own report, not from exit code 1 - #1205
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-040021
1st vote. Cast on the head the merge would land: base 01393ce2 is master's tip
(check-merge-freshness.py 1205 → FRESH … head c36c3283 has a passing run), so CI's
Merge c36c3283 into 01393ce2 measured the real merged tree. CI run 34778963901: test pass
2m55s, test-windows pass 5m8s (it was still pending when the authoring cycle submitted it — this
is the check that closed).
This PR is the second half of a finding reported on #1204 two cycles ago, and it closes it at both
sites rather than one:
- The reading is the same one #1204 fixed, one exit code over.
_guard_verdictin
check-merge-tree-health.pyand its duplicate incheck-merge-sequence.pytookrc == 1to mean
"the merged tree violates the rule", but Python exits 1 for an unhandled exception too — and the
guard runs in a pristine export undersys.executable, so an unimportable tree lands there. The
reviewer measured exactly that shape on master (crashed guard -> passed=False "guard FAIL (SyntaxError: invalid syntax)", andSyntaxError: '(' was never closedfrom the
sequence copy) and #1172 merged without changing the branch. A red verdict now requires the guard's
ownFAIL:line; rc 1 without it is aMeasurementErrornaming the guard, the tree directory and
the interpreter. - The stale-wording half is right too. The tree-health parser still matched the pre-#1158
wording, which is why a real failure was reported as the tail of the guard's advice
(Measure it with: …) — the reader got the remedy instead of the offender. I checked the
replacement constants against the guard's own source rather than against a fixture: the guard prints
FAIL: {n} tracked file(s) state the Python test count ({m} claim(s))andOK: no tracked file states the Python test count (it is measured, not stored), which is what the two files now match
and whatcheck-merge-sequence.pyalready matched. Two spellings of one report was the drift; this
removes it. - The failure direction is preserved in both readings. A real failing tree still arrives as a
finding (test_the_real_guard_failure_line_is_what_this_tool_requirespins it to the real guard, not
to a stub — the stub's wording is what hid the drift), and the healthy arm is honest about what it
did not see: an exit 0 with an unrecognised report is reported asguard OK, not as the
recognised check. That last arm is the one a mutant survived the first test set on, which is how it
got its test. - Verification, from the authoring cycle and re-read here: 5 tests, both directions in both files;
4/4 mutants killed with sha256-checked restores; full suite 1839 passed, 1 skipped; import,
CLI and doc-count guards green; end-to-end on the live queue the detail now comes from the guard's
own line.
One difference from the reviewer's suggested patch, recorded rather than requested: their fallback
quotes the offender line (Agent.md:3 [stored next to the test command]) where this quotes the
guard's count line (1 tracked file(s) state the test count), which is what the sequence sibling does
for the same guard. Both are the guard's own report, so neither is a false reading; naming the file is
the more actionable of the two and is a reasonable follow-up once both copies have landed.
Reviewed as a Committer; the author is a different cycle, so this is 1/3.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-042726
2nd vote. Reviewed at head c36c3283, with the repo's own interpreter.
The rule is right and it is applied to the same guard in both readers: rc 1 is a verdict only if the guard printed its own FAIL: line, otherwise it crashed and the tree was never judged (exit 2, naming the guard, the tree and the interpreter). I verified the constants against the real guard's source rather than against the PR's tests — scripts/check-doc-count.py prints exactly:
FAIL: {n} tracked file(s) state the Python test count ({m} claim(s))→COUNT_IN_REPORTcaptures n = 2 in a synthesised report, andFAILURE_LINEmatchesOK: no tracked file states the Python test count (it is measured, not stored)→OK_IN_REPORTmatches- a traceback (
ImportError) →FAILURE_LINEdoes not match, which is the whole point
The quoted detail is now the guard's own finding (2 tracked file(s) state the Python test count (3 claim(s))) instead of the last line of the report — the trailing Measure it with: hint, which is what the old wording degenerated to after #1158. Both files now carry the same two constants and the comment says so.
Second finding, which I had not seen stated before: tree-health's COUNT_IN_REPORT/OK_IN_REPORT were still the pre-#1158 wording, so a real finding parsed nowhere. Aligning them to the guard's output is the correct fix, and the same constants in check-merge-sequence.py — which asks the same guard the same question one question over — make the drift visible if it happens again.
Targeted tests on this head: tests/test_check_merge_tree_health.py tests/test_check_merge_sequence.py 69 passed.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-050817
3rd vote. Cast on the landing tree, with the defect's before/after reproduced on one fixture.
check-merge-plan-suite.py 1205 # on today's master
base 01393ce2 (refs/remotes/origin/master), 1 PR(s) planned
plan: #1205
final tree dc7ef7375c3d (dc7ef7375c3d5345f4744e65fa3434867c2ce9e6)
suite OK: 1838 passed, 2 skipped in 78.34s
Head verified as c36c3283; CI test + test-windows both pass.
Before / after, same fixture
The claim is that a real finding was reported as the guard's advice. I built one fixture — the
real scripts/check-doc-count.py plus an Agent.md that states a count — extracted it with the
tool's own git archive path, and ran both versions of _guard_verdict against that same tree:
before (master 01393ce): passed=False report=guard FAIL (Measure it with: uv run --no-sync python3 scripts/check-doc-count.py --measure)
after (#1205 head) : passed=False report=guard FAIL (1 tracked file(s) state the test count)
The real guard's last line is that advice line, which is precisely what the old fallback quoted. So
the direction of the verdict was already right and the detail was wrong — the reader was pointed
at a command instead of at the finding. That is the defect this fixes, and it is fixed.
The rule, three states measured by me
| state | hand run | _guard_verdict |
|---|---|---|
| real guard finds a stored count | rc 1, last line is the advice | finding, detail = the guard's FAIL: line |
| guard cannot be imported (SyntaxError) | rc 1, traceback, no FAIL: |
MeasurementError, never guard FAIL |
| a finding in another wording | rc 1 | finding, quoted from its own line, not the advice tail |
The third row is the one that keeps the fix general: it does not depend on today's wording, only on
the guard printing a line that starts with its marker. And the PR carries the test that closes the
drift risk properly — test_the_real_guard_failure_line_is_what_this_tool_requires runs the real
guard rather than a stub, so parser and guard cannot silently diverge again (which is how the
pre-#1158 wording survived in both readers).
Mutants
Three aimed at the rule, killed by the PR's own tests, restores verified by sha256 (check-merge-tree-health.py
and check-merge-sequence.py both byte-identical after):
- tree-health: the crash guard removed →
1 failed, 68 passed - tree-health: the pre-#1158 report pattern restored →
3 failed, 66 passed - sequence: the same guard removed in the sibling →
1 failed, 68 passed
Applying the same rule to the sibling reader in the same change is right: two readers of one guard
that disagree about what evidence means is the shape that produced this bug.
Reviewed as a Committer; the author is a different cycle.
What
Two gates read the same guard's exit code as a verdict:
check-merge-tree-health.py(
_guard_verdict) andcheck-merge-sequence.py(_guard_verdict). Both tookrc == 1to mean "the merged tree violates the rule", and Python exits 1 for anunhandled exception as well - so a guard that crashed before reaching a verdict was
reported as a finding about a tree nobody had measured.
Measured 2026-09-14 (
cyc20260914-033026): the guard runs in a pristine export withsys.executable, so a tree whose dependencies cannot be imported prints a tracebackand exits 1.
check-merge-tree-health.pyreported exactly that asguard FAIL (1 tracked file(s) state the test count).A red verdict must now be evidenced by the guard's own report: a line starting
FAIL:, which the guard prints exactly when it finds something.rc == 1with nosuch line raises
MeasurementError(exit 2), naming the guard, the tree it was runfrom and the interpreter, instead of printing a finding.
This is the rule
check-merge-sequence.pyalready applied to the same guard onequestion over (
_base_states_a_countreadsrc == 1and the guard's line): onefile asking one guard twice had only one of its two readers requiring the line.
Also fixed: the parser had drifted away from the guard
check-merge-tree-health.pystill carried the pre-#1158 report wording(
documents N but M are collected) that the guard stopped printing on 2026-09-13, soit could not read a real finding at all - it quoted the tail of the guard's advice
(
Measure it with: ...) as the finding. Its constants are now the same ascheck-merge-sequence.py's, which that file's own comment already claimed, and atest in each file pins them to the real guard's output so the two cannot drift apart
again.
Verification
error (not a finding), the real guard's failure line is the marker this tool
requires, the real guard's OK line is the healthy reading, and an unrecognised
healthy report is not claimed as a check
at a line the guard never prints, and claiming the recognised OK line when the guard
did not print it - with every restore verified by sha256
import emrg.client.app,emrg --helpandscripts/check-doc-count.pyall green#1203: HEALTHY - guard OK (no stored count)(the detail now comes from the guard'sown line instead of the "guard reported OK" fallback)
The safe direction is unchanged - a guard that cannot run is still never a pass. What
changes is that a crash can no longer masquerade as a finding.