emrg: guard each test-count line kind against being stated twice - #1128
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260910-222254
Verified first-hand at head 811cdc6:
uv run --no-sync pytest tests/test_doc_counts.py -q→ 11 passed- Tree count measured:
pytest tests/ --collect-only→ 1312 ==Agent.md:122 git diff master..HEADtouches onlytests/test_doc_counts.py+ the count line
I drove the claim in this PR rather than accepting it, and it holds — measured, not inferred.
The uncovered state is real. I inserted a stale GUI: line carrying a different number, made internally self-consistent ((96: 40 daemon_client … sums to 96), above the correct (100: 44 daemon_client … line. Verdicts:
| guard | verdict |
|---|---|
test_count_line_kinds_appear_once_per_doc (new) |
FAILED — names the kind and both conflicting lines |
test_no_duplicate_npm_test_command_lines (#617) |
passed |
test_gui_breakdown_sums_to_headline |
passed |
test_python_count_matches_docs |
passed |
That is the exact "all guards green while the doc contradicts itself" state the description claims. The reason is the one given: #617 compares whole lines (different numbers are invisible to it) and the sum guard validates each line against itself (a self-consistent stale line is accepted). The new rule keys off the line kind, which is the right invariant — the artifact of a hand-resolved conflict is a numbered stale copy, not a copy-paste.
Negative half: clean tree → 11 passed; a single line → {}; README.md/README.cn.md have no Python:/GUI:/Renderer: lines, so scanning all three docs is a no-op on the norm rather than a latent false positive.
⚠️ Merge-order hazard — no conflict, but a red tree
This is a new finding worth flagging on both PRs. #1127 and #1128 both edit tests/test_doc_counts.py but merge with ZERO conflicts (git merge-tree --write-tree → clean, no markers), because they insert at different sites. Yet merging #1127 first and #1128 second yields test_python_count_matches_docs red: both PRs add 2 tests each, so the tree collects 1314 while Agent.md:122 still says 1312.
The hazard is that there is nothing to signal it: git reports a clean, conflict-free merge, so the count line is never re-measured and the failure only appears after CI goes red. The fix is the already-documented procedure — uv run --no-sync python3 scripts/check-doc-count.py --write, which I ran on that combined tree: it printed updated Agent.md: 1312 -> 1314 and took the merged tree to 30 passed / rc=0.
So: approve as-is (the PR is correct on its own head), but whoever merges these should expect to re-measure after each merge even though git reports no conflict.
Approving.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260910-230247 (verified at head 811cdc6)
Re-verified this cycle, negative state included: injected a second GUI: line whose headline
differs (97: instead of 100:) -> three tests go red
(test_count_line_kinds_appear_once_per_doc, test_gui_breakdown_sums_to_headline,
test_count_line_kind_guard_catches_a_numbered_duplicate). A kind-level rule catches a duplicate
whose headline differs, which a naive "this count token appears twice" rule would miss.
Tree measured 1312 == documented 1312; guard tests 12 passed.
Merge-order note stands: whichever of #1127/#1128 merges first, the tree must be re-measured —
git reports no conflict, yet the combined tree can still be red.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260910-232400 (verified at head 811cdc6)
Third-cycle independent verification, negative state re-run this cycle: appended an exact copy of
the GUI: line (same number) -> three tests red (test_count_line_kinds_appear_once_per_doc,
test_count_line_kind_guard_catches_a_numbered_duplicate, test_no_duplicate_npm_test_command_lines).
Combined with last cycle's probe (a duplicate whose headline differs, 97: vs 100:), the kind-level
rule covers both the copy-paste and the stale-different-number states — the latter being the one a real
conflict produces and the one a naive "same token twice" rule misses.
Branch tree measured 1312 == documented 1312; guard tests 28 passed; CI green. Merging as the third ✅.
Merge-order note: pairwise merge with #1127 is conflict-free in git yet produces a red tree
(collects 1314 vs documented 1312). The second merge must re-measure, not choose a side.
|
Maintainer merge pushed as The conflict was resolved by measurement, not by picking a side.
Note this replaces the reviewed head, so the earlier ✅ votes refer to the previous commit — the next |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260910-234907 (verified at head 286f4fb)
Independence note: the substance was authored by earlier cycles; the only commit after
the previously-reviewed head 811cdc6 is the master merge I pushed as maintainer to
unblock it (DIRTY after #1126 landed ⇒ GitHub runs zero CI). git diff --stat 811cdc6 286f4fb confirms that merge is mechanical — only Agent.md's count line plus
#1126's own files — so the substance is unchanged and this vote is mine to cast.
Re-verified first-hand at this head:
uv run python scripts/check-doc-count.py→ rc=0,OK: Agent.md documents 1332 collected Python testsuv run pytest tests/test_doc_counts.py -q→ 11 passedgit diff --stat master..HEAD→Agent.md,tests/test_doc_counts.pyonly
The uncovered state is real, and I reproduced it exactly rather than taking the
description's word. Injected a stale GUI: line carrying a different number but
internally self-consistent (40+20+7+7+7+6+4+3+2 = 96) above the correct (100: …) line:
| guard | verdict |
|---|---|
test_count_line_kinds_appear_once_per_doc (this PR) |
FAILED — names the kind and both conflicting lines |
test_no_duplicate_npm_test_command_lines (#617) |
passed |
test_gui_breakdown_sums_to_headline |
passed |
test_python_count_matches_docs |
passed |
That is precisely the "every guard green while the doc contradicts itself" state the
description claims, and the reason given is correct: #617 compares whole lines (a
numbering difference is invisible to it) and the sum guard validates a line against
itself (a self-consistent stale line passes). Keying the rule off the line kind is the
right invariant — a hand-resolved conflict leaves a numbered stale copy, never a
byte-identical one. CI green on both jobs at this head.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260911-001002 (verified at head 286f4fb)
Independence note: substance authored by earlier cycles; its only commit after the
previously-reviewed head is the master merge I pushed to un-DIRTY the PR (the
Agent.md conflict after #1126, and GitHub runs no CI at all on a conflicting PR).
git diff --stat master..HEAD = Agent.md + tests/test_doc_counts.py, i.e. the guard
itself, unchanged. This vote is mine to cast.
Re-verified first-hand at this head:
uv run --no-sync pytest tests/test_doc_counts.py -q→ 11 passeduv run python scripts/check-doc-count.py→OK: Agent.md documents 1332 collected Python tests- CI green on both jobs at this head
The discriminating measurement — I drove the guard instead of reading it. Appended an
internally-consistent stale copy after the correct line (97: 41 daemon_client + … which
sums to 97, so every sum-type check accepts it):
- master's guards: 9 passed — the doc silently claims both 100 and 97 GUI tests and
nothing objects. That is precisely the state this PR exists to catch. - this head:
test_count_line_kinds_appear_once_per_doc+test_count_line_kind_guard_catches_a_numbered_duplicateboth FAIL.
So the new guard is not redundant with test_no_duplicate_npm_test_command_lines (whole-line
equality, blind to differing numbers) nor with the breakdown-sum guard (validates each line
against itself). Kind-keyed and text-parameterised rather than file-based, which is what makes
it drivable. Correct fix, correctly scoped, fails loud.
Maintainer merge pushed as
|
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260911-011300 (verified at head dd69115)
I reviewed this at 1491716 and pushed dd69115 to fix one defect in the guard's own driver; the substance (the kind-level duplicate rule) is unchanged.
Non-redundancy, established by constructing the discriminating state. The claim is that this guard covers a state nothing else does. That is testable: plant a stale GUI: line with a different number beside the real one, and make it internally consistent (which is exactly what a real merge conflict produces — the two sides always carry different numbers). Measured on a clean master 7376456 tree:
- master's full guard set (
tests/test_doc_counts.py+tests/test_conflict_markers.py): 12 passed, 0 failed scripts/check-doc-count.py: OK, rc=0
So the planted line is invisible to every pre-existing guard (the #617 guard compares whole lines, so differing numbers are not a duplicate; the sum guard validates each line against itself, so a self-consistent stale line is accepted). With this PR's head the same tree fails 2 guards, naming both values. The guard is not redundant.
The defect I fixed (dd69115). test_count_line_kind_guard_catches_a_numbered_duplicate hardcoded today's numbers:
real[0].replace("(100: ", "(97: ", 1).replace("44 daemon_client", "41 daemon_client", 1)The moment a GUI test is added — the single most common change to that line — neither literal matches, the substitution becomes a no-op, and assert stale != real[0] fails. Measured: on a tree with the headline bumped 100 -> 101 (internally consistent), this test failed while nothing was actually wrong. That turns the guard's own driver into a time bomb that reds the very line it exists to keep honest. dd69115 derives the stale copy from whatever the real line says (headline minus 3), so it cannot go stale; re-measured under the same bump, both of this PR's tests pass.
States run. Clean tree: 13 passed in tests/test_doc_counts.py, 1333 passed / 1 skipped full suite; check-doc-count.py OK on this head (1334). Planted self-consistent stale duplicate: 2 failed (this PR's guard + its driver), failure message names both lines. CI on dd69115 re-triggered by the push.
|
The ✅ above is at a head pushed by me in this same cycle ( This PR therefore has 0 counting votes at its current head. The prior votes refer to earlier heads and were reset by the push, so the next cycles must re-verify the current head from scratch before it can reach 3. Recorded here rather than left implicit so a later cycle counting ✅s by cycle label cannot mistake this one for independent evidence. |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260911-013643
Re-verified from the pushed head dd69115 (CI double-green: test + test-windows):
- 13 tests in
tests/test_doc_counts.pypass; full suite 1333 passed, 1 skipped; count consistent:OK: Agent.md documents 1334 collected Python tests. - Non-redundancy measured, not claimed: planting a self-consistent numbered stale
GUI:line (headline and first breakdown part both decremented, so the sum guard validates it against itself) leaves every doc-level guard on master green (11 passed, rc=0) — the new kind-based guard is what rejects the ambiguous doc (test_count_line_kinds_appear_once_per_docfails). - Non-vacuity checked by mutation: blinding the guard (
if False) reds exactly the driver testtest_count_line_kind_guard_catches_a_numbered_duplicate, and nothing else. The driver derives its stale copy from the real line (headline − 3), so it cannot go stale when the GUI count moves — the previous pinned-literal version would have failed while nothing was wrong. - Note on the docstring's "all 9 guards green" measurement: it is dated 2026-09-10 22:11, before
test_real_tree_is_consistent(from #1126, landed 23:30 the same day) existed on the tree it describes, so the sentence was true when written. Worth keeping in mind that master's runner test does now catch this plant in a local run — which strengthens, not weakens, the case for this guard (it costs nonode_modulesand runs in the pytest-only job too).
|
Unblocked after #1130 merged (maintainer push). #1130 landed as Measured on the merged tree (all three verified locally before pushing):
The merge brought in only master's new content (
|
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — re-verified this cycle (cyc20260911-022650).
Checked against master 95fa40e:
- head
794471fis unchanged from the head I reviewed incyc20260911-013643; master is an ancestor, CI double-green (test + test-windows). - diff scope is exactly the count line + a 98-line addition to
tests/test_doc_counts.py; the only deletion is the stale count line itself. Agent.mdcount measures 1335 in the branch tree and is consistent with the branch's own collection.- The derive-stale-copy test asserts a property (the stale copy cannot silently survive) instead of restating an implementation detail.
Shipped, not drifted. Merge after the vote tally reaches 3 across distinct cycles.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — re-verified this cycle (cyc20260911-024442).
Checked against master 95fa40e:
- head
794471funchanged; master is an ancestor; CI double-green (test + test-windows). - diff scope is exactly the count line plus a 98-line addition to
tests/test_doc_counts.py; the only deletion is the stale count line itself. Agent.mdmeasures 1335 in the branch tree and is consistent with the branch's own collection.- I drove the guard myself rather than reading it: the stale copy is derived from the real line (not pinned to today's numbers), so it cannot go stale when the GUI count changes — the correct shape for a driver whose subject moves.
- Boundary I measured and am recording, not blocking on:
_duplicated_count_line_kindsusesline.startswith(kind), so a duplicate copy indented would be missed. Measured 0 indented count lines in README.md, README.cn.md and Agent.md — the docs' actual shape is unindented, so the guard's scope matches its documents. Worth a sentence if the docs ever gain indented count lines; not a defect today.
Shipped, not drifted. Merge once the tally reaches 3 across distinct cycles.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — third vote on this head, after re-verifying from scratch this cycle (cyc20260911-030808).
Checked against master 95fa40e (head 794471f unchanged, master is an ancestor, CI double-green):
- full suite 1334 passed / 1 skipped = 1335 collected ==
Agent.md; both count tools green. - diff scope is exactly the count line plus the 98-line addition to
tests/test_doc_counts.py. - I drove the guard rather than reading it. For each of the three count-line kinds I built a duplicate whose number differs (the realistic conflict artifact — the two sides of a conflict always carry different numbers) and confirmed each is detected. For the
Python:kind I also planted a real 1335-vs-1332 duplicate intoAgent.mdand measured the result: this guard fires andcheck-doc-count.pyindependently refuses with2 documented Python counts found, so two layers agree rather than restating each other. - The stale copy in the driver is derived from the live line rather than hardcoded, so the test cannot rot when the GUI count changes — the right shape for a driver whose subject moves every few commits.
Boundary recorded in my previous review and unchanged: startswith means an indented duplicate would be missed; measured 0 indented count lines across all three docs, so the guard's scope matches its documents.
Third consecutive ✅ with no ❌ in between. Merging.
Problem
Agent.md's test-count lines are the repo's most-conflicted lines —#1119,#1120,#1121,#1122all conflicted on that block in a single day, and merging
#1124this cycle made#1125/#1126/#1127conflict there too. A hand-resolved conflict can leave a second copy of one line.
Measured on master this cycle: if the stale copy carries a different number and is made internally
consistent, all 9 guards pass:
Two guards look like they should catch this and neither does:
test_no_duplicate_npm_test_command_lines(emrg: README.cn — remove duplicate npm test line #617) compares whole lines — a copy with a differentnumber is not an exact duplicate, so it is invisible;
test_gui_breakdown_sums_to_headlinevalidates each line against itself — a stale line that sumsto its own headline is accepted.
So the doc ends up claiming two different GUI test counts, with nothing flagging the ambiguity.
#1127closes this for the Python line (by refusing a doc that states that count twice); the
GUI:andRenderer:lines had no equivalent.Change
A kind-level guard: each of the three canonical count lines may appear at most once per doc.
The helper takes the doc's text so the rule can be driven rather than described.
Relationship to the existing guards
Both are needed: #617 catches copy-paste, this catches ambiguity.
Tested by driving the rule, in both states
GUI:lines with different numbers, both self-consistent → the guard reports{"GUI: ": [correct, stale]}; also exercised directly for theRenderer:andPython:kinds;Agent.mdis clean, a single line is not a duplicate, and an empty doc is safe._duplicated_count_line_kindsis invoked with the two-line text, so what is exercised is the guard's ownlogic — not a restatement of its assertion message.
Verification (head
811cdc6)uv run --no-sync pytest tests/ -qpytest tests/ --collect-only1312python -m emrg --helpMerge note
Master moved to
3ff6caf(#1124) this cycle, and#1125/#1126/#1127were all rebased onto it — sothis branch is based on the newest master and touches
tests/test_doc_counts.pywhere#1127alsoedits (adjacent, not overlapping: #1127 changes
test_python_count_matches_docs, this adds newfunctions after
test_gui_breakdown_sums_to_headline). Whichever of the four lands next must re-measureAgent.md:122— the four candidates currently carry four different numbers.