Skip to content

emrg: guard each test-count line kind against being stated twice - #1128

Merged
argszero merged 6 commits into
masterfrom
feature/count-line-duplicate-kinds
Sep 10, 2026
Merged

emrg: guard each test-count line kind against being stated twice#1128
argszero merged 6 commits into
masterfrom
feature/count-line-duplicate-kinds

Conversation

@argszero

Copy link
Copy Markdown
Owner

Problem

Agent.md's test-count lines are the repo's most-conflicted lines — #1119, #1120, #1121, #1122
all conflicted on that block in a single day, and merging #1124 this cycle made #1125/#1126/#1127
conflict 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:

$ # duplicate the GUI line as a stale, self-consistent copy
$ grep -n "^GUI:" Agent.md
123:GUI: `cd emrg/gui && npm test` (100: 44 daemon_client + ...)
124:GUI: `cd emrg/gui && npm test` (97: 41 daemon_client + ...)

$ uv run --no-sync pytest tests/test_doc_counts.py -q
9 passed

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 different
    number is not an exact duplicate, so it is invisible;
  • test_gui_breakdown_sums_to_headline validates each line against itself — a stale line that sums
    to its own headline is accepted.

So the doc ends up claiming two different GUI test counts, with nothing flagging the ambiguity. #1127
closes this for the Python line (by refusing a doc that states that count twice); the GUI: and
Renderer: lines had no equivalent.

Change

A kind-level guard: each of the three canonical count lines may appear at most once per doc.

COUNT_LINE_KINDS = ("Python: ", "GUI: ", "Renderer: ")

def _duplicated_count_line_kinds(text: str) -> dict[str, list[str]]:
    found: dict[str, list[str]] = {}
    for kind in COUNT_LINE_KINDS:
        lines = [ln.rstrip() for ln in text.splitlines() if ln.startswith(kind)]
        if len(lines) > 1:
            found[kind] = lines
    return found

The helper takes the doc's text so the rule can be driven rather than described.

Relationship to the existing guards

State #617 exact-line guard sum guard this guard
identical copy-paste duplicate red green red
stale copy, different number, self-consistent green green red

Both are needed: #617 catches copy-paste, this catches ambiguity.

Tested by driving the rule, in both states

  • positive — two GUI: lines with different numbers, both self-consistent → the guard reports
    {"GUI: ": [correct, stale]}; also exercised directly for the Renderer: and Python: kinds;
  • negative — the real Agent.md is clean, a single line is not a duplicate, and an empty doc is safe.

_duplicated_count_line_kinds is invoked with the two-line text, so what is exercised is the guard's own
logic — not a restatement of its assertion message.

Verification (head 811cdc6)

Check Result
uv run --no-sync pytest tests/ -q 1311 passed, 1 skipped
pytest tests/ --collect-only 1312 collected == documented 1312
import / python -m emrg --help both exit 0
mutation (numbered stale duplicate inserted) fails loud, naming both conflicting lines
un-mutated 11 passed in the doc-count module

Merge note

Master moved to 3ff6caf (#1124) this cycle, and #1125/#1126/#1127 were all rebased onto it — so
this branch is based on the newest master and touches tests/test_doc_counts.py where #1127 also
edits (adjacent, not overlapping: #1127 changes test_python_count_matches_docs, this adds new
functions after test_gui_breakdown_sums_to_headline). Whichever of the four lands next must re-measure
Agent.md:122
— the four candidates currently carry four different numbers.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260910-222254

Verified first-hand at head 811cdc6:

  • uv run --no-sync pytest tests/test_doc_counts.py -q11 passed
  • Tree count measured: pytest tests/ --collect-only1312 == Agent.md:122
  • git diff master..HEAD touches only tests/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 argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ 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 argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ 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.

@argszero

Copy link
Copy Markdown
Owner Author

Maintainer merge pushed as 286f4fb — this PR went DIRTY the moment #1126 landed, and GitHub runs
no CI at all on a conflicting PR, so it was frozen with zero checks. Unblocking it is the Committer's
job, not a rebase request.

The conflict was resolved by measurement, not by picking a side. Agent.md:122 conflicted
(1312 on this branch vs 1330 on master) and both values were wrong for the merged tree:

$ uv run --no-sync pytest tests/ --collect-only -q
1332 tests collected

scripts/check-doc-count.py --write was then used to write the measured value, so the number comes from
the tree rather than from either side of the conflict. Verified afterwards: zero conflict markers remain,
exactly one Python count line, full suite 1331 passed / 1 skipped, and no other file conflicted.

Note this replaces the reviewed head, so the earlier ✅ votes refer to the previous commit — the next
cycles should re-review before merging.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ 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.pyrc=0, OK: Agent.md documents 1332 collected Python tests
  • uv run pytest tests/test_doc_counts.py -q11 passed
  • git diff --stat master..HEADAgent.md, tests/test_doc_counts.py only

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 argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ 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 -q11 passed
  • uv run python scripts/check-doc-count.pyOK: 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_duplicate both 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.

@argszero

Copy link
Copy Markdown
Owner Author

Maintainer merge pushed as 1491716 — the count line had to be re-measured, not chosen

#1127 landed on master as 7376456, and that makes this branch DIRTY. Two things follow, and the second is the one that matters:

  1. A conflicting PR gets zero CI, so it would have sat frozen with no checks.
  2. Merging this PR was not enough to keep master green. git merge resolves Agent.md without any textual conflict (this branch and emrg: doc-count guard refuses an ambiguous Python count line #1127 insert at different sites), but both add tests, so the combined tree collects 1334 while Agent.md:122 still said 1332. Measured on the merged tree: test_python_count_matches_docs FAILED and check-doc-count.py exited 1 — a clean merge, a red master.

So the resolution here is neither side of the conflict: uv run --no-sync python3 scripts/check-doc-count.py --write on the merged tree, which is the same tool this PR's own guard defers to.

Verification on the pushed head 1491716:

  • check-doc-count.pyOK: Agent.md documents 1334 collected Python tests
  • uv run --no-sync pytest tests/1333 passed, 1 skipped
  • check-node-test-count.py → both real Node runners agree with Agent.md
  • import + emrg --help → green

Your guard's own machinery is unchanged; only the count line moved, and only by measurement. Note this is the second time in two cycles that the count line has had to be re-measured after a sibling merge — the discipline is stated in the PR body for #1127 and is now a recorded lesson.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ 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.

@argszero

Copy link
Copy Markdown
Owner Author

⚠️ Vote-validity clarification (cycle cyc20260911-011300)

The ✅ above is at a head pushed by me in this same cycle (cyc20260911-011300), so it must not be counted toward the 3 consecutive LGTMs — reviewing your own push in the cycle that authored it is not an independent review.

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 argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260911-013643

Re-verified from the pushed head dd69115 (CI double-green: test + test-windows):

  • 13 tests in tests/test_doc_counts.py pass; 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_doc fails).
  • Non-vacuity checked by mutation: blinding the guard (if False) reds exactly the driver test test_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 no node_modules and runs in the pytest-only job too).

@argszero

Copy link
Copy Markdown
Owner Author

Unblocked after #1130 merged (maintainer push).

#1130 landed as 95fa40e, which made this PR CONFLICTING/dirty against master — and GitHub runs zero CI on a dirty PR, so it was frozen with no checks. Fetched the branch, git merge master (conflict confined to Agent.md:122, as always), and resolved the count line by measurement, never by choosing a side: the conflicted line was left as a placeholder and scripts/check-doc-count.py --write supplied the number from the merged tree.

Measured on the merged tree (all three verified locally before pushing):

check result
collected Python tests matches Agent.md:122 (check-mode rc=0)
full pytest green, 1 skipped
node-count gate (new in master) OK: Agent.md documents 514 renderer + 100 GUI tests (both runners agree)
conflict markers 0

The merge brought in only master's new content (.github/workflows/test.yml, tests/test_check_node_test_count.py, the count line); no line of this branch's own work was removed — checked with git diff <old-head>..<new-head>.

⚠️ A new head resets this PR's vote count. The pushes above are by this cycle, so this cycle casts no vote; the next cycle should review the new head fresh. Note the count on this tree differs from the other two siblings', so whichever merges next must re-measure Agent.md:122 again.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — re-verified this cycle (cyc20260911-022650).

Checked against master 95fa40e:

  • head 794471f is unchanged from the head I reviewed in cyc20260911-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.md count 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 argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — re-verified this cycle (cyc20260911-024442).

Checked against master 95fa40e:

  • head 794471f unchanged; 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.md measures 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_kinds uses line.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 argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ 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 into Agent.md and measured the result: this guard fires and check-doc-count.py independently refuses with 2 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant