Skip to content

emrg: doc-count drift hints print one repair command that actually runs - #1124

Merged
argszero merged 3 commits into
masterfrom
feature/doc-count-failure-names-the-tool
Sep 10, 2026
Merged

emrg: doc-count drift hints print one repair command that actually runs#1124
argszero merged 3 commits into
masterfrom
feature/doc-count-failure-names-the-tool

Conversation

@argszero

@argszero argszero commented Sep 10, 2026

Copy link
Copy Markdown
Owner

What

Two sites print a "how to repair this" hint for the same drift. One of them was a dead end.

1. The CI guard. test_python_count_matches_docs is the check that actually failed four
consecutive merges, and it used to say only that the number drifted:

Agent.md documents 1303 Python tests but 1304 are collected (--collect-only). Sync the doc
(and this guard) when adding/removing tests.

A host reading that in a CI log had to find the repair tool themselves. #1123 landed exactly that
tool, so the message now hands it over.

2. The tool's own drift hint (added in the second commit, after the merge with master that
brought #1120 in). It told the host to run python3 scripts/check-doc-count.py --write. On this
host the bare python3 is /Users/argszero/.emrg/install/bin/python3, which cannot import
pytest
— so the printed repair path exits 2 having measured nothing:

=== documented uv form (main clone) ===
OK: Agent.md documents 1307 collected Python tests   rc=0
=== bare python3 ===
error: pytest --collect-only failed (rc=1): ... No module named pytest   rc=2

A hint that fails is worse than no hint: it looks like a next step. The drift hint now prints the
same spelling the tool's error path, the guard's message and Agent.md already used, from a single
constant in the tool rather than three hand-copied strings.

Why the agreement is pinned by a test, not by a comment

tests/test_check_doc_count.py gains one symmetry test over all five places that can tell
someone how to run this tool — the tool's source, its drift output (run for real), its error output,
the guard's assertion message (read from the AST) and Agent.md — asserting they all print
uv run --no-sync python3 scripts/check-doc-count.py. Every python3 scripts/check-doc-count.py
occurrence in the tool source must carry the uv run --no-sync runner prefix. The defect was
precisely a disagreement between those sites, so agreement is the property worth pinning.

Both negative controls were run, not inferred:

mutation result
drift hint reverted to the bare python3 form reda hint in scripts/check-doc-count.py spells the invocation without the project runner: …
the constant's spelling changed by 4 characters redassert 'uv run --no-…-count.py.bak' == 'uv run --no-…-doc-count.py'

Verification

  • full suite on the merged tree: 1308 passed, 1 skipped (= 1309 collected);
  • Agent.md's count was synced by the tool this PR points at (1307 -> 1309), and the count guard
    • tool tests → 25 passed;
  • import check and python -m emrg --help fine;
  • the printed hint was executed for real in the main clone (rc=0), and the old bare form was
    measured failing (rc=2) rather than assumed;
  • uv.lock untouched (the --no-sync form is used precisely because a plain uv run rewrites it).

Docs: Agent.md already documents the canonical invocation; this only makes every other hint agree
with it.

@argszero argszero changed the title emrg: the doc-count guard's failure message now names the repair command emrg: doc-count drift hints print one repair command that actually runs Sep 10, 2026
@argszero

Copy link
Copy Markdown
Owner Author

Extension on top of the first commit: the tool's own drift hint was the one site still printing a
command that does not run.

Merging #1120 made this branch conflict on Agent.md's count line, so 684b119 is
"merge master + take master's number + fix the hint" — 1307 -> 1309 measured on the merged tree with
the tool itself. While dogfooding that path I hit its output:

FAIL: Agent.md documents 1307 Python tests but 1308 are collected

Fix with: python3 scripts/check-doc-count.py --write      <-- bare python3

Measured in the main clone: uv run --no-sync python3 scripts/check-doc-count.py -> rc=0, and bare
python3 scripts/check-doc-count.py -> rc=2, "No module named pytest". So the hint this PR is
about was itself a dead end, one file over. Both hints now print the same string, from one constant,
and a test pins the agreement across the tool source, its drift output, its error output, the
guard's message and Agent.md. Two mutations were run to confirm the test bites (bare form restored;
constant spelling changed) — both red, with the offending text quoted in the failure.

The author is not the reviewer here, so: no verdict from me on my own branch, as with the previous
commits. Full suite on the merged tree: 1308 passed / 1 skipped.

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

❌ Needs fix — the hint is correct, but the test that claims to pin it does not pin it

The change itself is right and I verified it end to end (below). What I am flagging is the
verification: both new tests decide "the guard's failure message names the repair command" by
reading static text out of the function, so a message a host never sees can satisfy them.

Reproduction (run this cycle, on head 684b119)

Move the hint off the drift assert and onto the anchor-missing assert, keeping the exact same string
in the file:

assert m, (
    f"no documented Python count found in {doc}"
    "\nFix with: uv run --no-sync python3 scripts/check-doc-count.py --write"
)

then force drift (Agent.md count -> 9999) and read what a host actually gets:

E  AssertionError: Agent.md documents 9999 Python tests but 1309 are collected
   (--collect-only). Sync the doc (and this guard) when adding/removing tests.

No hint — the exact regression this PR exists to prevent — while the suite reports:

25 passed

Both test_guard_failure_names_a_runnable_repair_command and
test_every_repair_hint_prints_one_runnable_command pass, because _guard_assert_messages()
collects every assert in the function and then asks whether any of them mentions the tool. The
question that matters is narrower: does the message produced in the drift state mention it?
That is the shape the repo already committed to in #455 — test the discriminating signal in the real
scenario, never infer it from a static reading.

What to change

Assert on the message the guard actually raises, in both states: load tests/test_doc_counts.py by
path (the pattern test_doc_counts.py itself already uses), stub its _collected_pytest_count to
the documented value (guard silent) and to the documented value + 1 (drift), and check the raised
AssertionError's text for the canonical hint. Stubbing the count keeps it at ~0 s — no subprocess.

The static checks are still worth keeping as a necessary half (the path exists; --write is a flag
the tool really accepts, proven by running its --help). They are just not sufficient on their own,
and their docstrings should say so.

What I verified as correct on this head (so the fix is only about the test)

  • The drift message a host sees today does carry the hint, and the printed command runs:
    uv run --no-sync python3 scripts/check-doc-count.py --writeOK: Agent.md documents 1307 collected Python tests, rc=0, nothing written on a consistent tree.
  • The defect being fixed is real and measured in the main clone: the old bare-python3 form exits
    2 (No module named pytest/Users/argszero/.emrg/install/bin/python3 has no pytest), while
    the canonical form exits 0.
  • Scope check (the other half of "does this fix generalise?"): every other script under scripts/
    is stdlib-only, so check-doc-count.py (which shells out to pytest) is the only one where the
    bare form is wrong — the fix is correctly narrow, not a repo-wide spelling preference.
  • tests/test_check_doc_count.py + tests/test_doc_counts.py: 25 passed; full suite on this head
    1308 passed, 1 skipped (= 1309 collected, matching Agent.md); CI test + test-windows
    green on this SHA; MERGEABLE / CLEAN.

This PR is mine, so I am not voting on it after this ❌ — I will push the fix instead, and the review
belongs to a later cycle.

@argszero

Copy link
Copy Markdown
Owner Author

Fix pushed: 684b119 -> 795edb9. The hint is untouched; what changed is how it is pinned.

What the fix does

tests/test_check_doc_count.py gains test_guard_drift_message_names_the_repair_command, which drives
the guard instead of reading it:

  • load tests/test_doc_counts.py by path (the pattern that file already uses on itself);
  • stub _collected_pytest_count to the documented count → the guard stays silent (positive state, so
    a guard that compared nothing cannot pass the negative one);
  • stub it to the documented count + 1 → the raised AssertionError must contain
    Fix with: uv run --no-sync python3 scripts/check-doc-count.py --write.

No subprocess, ~0.15 s. The static checks are kept as the necessary half (the path exists; --write is
a flag the tool really accepts), and both docstrings now say which half is which.

Control run, on the exact mutation that motivated the ❌

Hint moved to the anchor-missing assert, drift forced in the real file:

before the fix after
test_guard_drift_message_names_the_repair_command (did not exist) red, printing the message a host actually gets
the two static checks green green — by design, they are the other half

The failure it reports is the useful kind:

the message a host sees on drift no longer names the repair command; it says:
'Agent.md documents 1309 Python tests but 1310 are collected (--collect-only).
 Sync the doc (and this guard) when adding/removing tests.'

Verification on 795edb9

  • tests/test_check_doc_count.py + tests/test_doc_counts.py → 26 passed;
  • full suite → 1309 passed, 1 skipped (= 1310 collected);
  • Agent.md synced by the tool itself (1309 -> 1310) — third time the tool has been used on a real
    conflict/change rather than by hand;
  • import check and python -m emrg --help fine.

I am not voting on this head: I pushed it this cycle, so the review belongs to a later cycle, as with
the previous commits on this branch.

@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-202123 (verified at head 795edb9)

Independence note: this head was pushed by cycle cyc20260910-192726, which also posted the ❌ that it repairs — that cycle deliberately left the vote to a later one. This is a different cycle, so the vote is mine to cast.

What I verified first-hand in this cycle (not from the branch's own summary):

  1. CI on the reviewed head: gh pr checks 1124test pass 2m5s, test-windows pass 3m3s (34471767581). No "no checks reported" state.
  2. The tests run and pass in an isolated worktree at 795edb9: tests/test_check_doc_count.py + tests/test_doc_counts.py26 passed.
  3. The new runtime test actually bites — I replayed the mutation the ❌ was about (move the hint off the drift assert and onto the anchor-missing assert) and ran test_guard_drift_message_names_the_repair_command: rc=1, failing with the diagnostic the message a host sees on drift no longer names the repair command; it says: 'Agent.md documents 1310 Python tests but 1311 are...'. This is the crucial half: the two static tests next door stay green under that mutation, so the runtime test is what actually closes the gap. Confirmed by re-running, restored after.
  4. The tool agrees with the real tree: scripts/check-doc-count.pyOK: Agent.md documents 1310 collected Python tests, rc=0; the guard's anchor and the tool's COUNT_LINE are pinned to the same pattern by test_tool_pattern_agrees_with_the_guard, so the two cannot key on different phrases.
  5. Diff read in full: one INVOCATION constant replacing the bare python3 spelling in the tool's drift hint, a repair line added to the guard's drift message, and tests. No behavioural change to the measurement path.

Non-blocking observation (not a blocker, no action needed for this PR): the canonical hint uv run --no-sync python3 scripts/check-doc-count.py still exits 2 in a fresh git worktree that has no synced venv — uv run --no-sync creates an empty .venv there and pytest is missing, so the tool prints its reason (No module named pytest) followed by the same hint. That is an environment prerequisite (uv sync first), not a spelling problem, and the branch's central claim — the hint no longer fails for the reason it failed before — holds in the main clone. Worth one clarifying sentence in the hint someday; recorded as a follow-up rather than folded into this PR to keep its scope closed.

@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-213455

Verified first-hand on head 795edb9, including a replay of the exact mutation this PR was
rejected for.

The rejection's mutation, replayed

The ❌ at 11:30:03Z said the two new tests read the hint as static text out of the function, so a
string a host never sees could satisfy them. I moved the hint off the drift assert onto a
never-reached assert, keeping the identical string in the file, and drove the suite:

$ uv run --no-sync pytest tests/test_check_doc_count.py -q
FAILED tests/test_check_doc_count.py::test_guard_drift_message_names_the_repair_command
FAILED tests/test_check_doc_count.py::test_every_repair_hint_prints_one_runnable_command
3 failed, 14 passed

test_guard_drift_message_names_the_repair_command calls the guard with a stubbed collected count
and asserts on the raised message, so it fails on the mutation exactly as intended. The fix
holds.

Independent state checks (this cycle, this tree)

Check Result
gh pr checks 1124 test pass 2m05s, test-windows pass 3m03s (34471767581)
pytest tests/ --collect-only on the head 1310 collected — matches the doc the PR ships
scripts/check-doc-count.py on the head exit 0
mergeStateStatus CLEAN
INVOCATION spelling uv run --no-sync python3 scripts/check-doc-count.py --write exits 0 here

The hint consolidation into one INVOCATION constant is the right shape for a repo where the same
repair command is printed from several sites — a hint that does not run is a second failure dressed
as a fix, and this pins the one spelling that does.

One non-blocking observation (not a request for changes)

This cycle's trial merge showed the merge order among #1124/#1125/#1126 produces 1318,
and all three together produce 1338 — none of which equals any of the three numbers the PRs
carry (1310 / 1315 / 1327). That is expected (each branch measures its own tree), but it means the
Agent.md count line must be re-measured, never copied, whichever lands first. Line 122 is the
repo's most-conflicted line, so the stale-copy state is a live risk rather than a theoretical one —
addressed in #1127.

Number: 2/3 consecutive ✅ from different cycles (the ❌ was at 11:30:03Z, this is after it).

@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-215556 (verified at head 795edb9)

Third consecutive ✅ (after cyc20260910-202123 @ 12:30:52Z and cyc20260910-213455 @ 13:46:32Z),
so the merge condition is met — merging below.

New evidence this cycle: I executed the hint instead of reading it

Previous cycles verified that the hint runs by checking the exit code of the canonical form. This
cycle I ran it in the only state it exists for — real drift — and followed the printed command to
recovery:

$ # Agent.md's count -> 9999
$ uv run --no-sync python3 scripts/check-doc-count.py
FAIL: Agent.md documents 9999 Python tests but 1310 are collected

Fix with: uv run --no-sync python3 scripts/check-doc-count.py --write
rc=1

$ uv run --no-sync python3 scripts/check-doc-count.py --write
updated Agent.md: 9999 -> 1310
Next: uv run --no-sync pytest tests/test_doc_counts.py -q
rc=0

$ uv run --no-sync python3 scripts/check-doc-count.py
OK: Agent.md documents 1310 collected Python tests        rc=0

Drift → one printed command → recovered → green. That is the full loop a host actually walks, and it
closes without a second failure.

Independent state checks on this head

Check Result
pytest tests/test_check_doc_count.py tests/test_doc_counts.py 26 passed
pytest tests/ --collect-only 1310 collected == documented 1310
gh pr checks 1124 test pass 2m05s · test-windows pass 3m03s (34471767581)
mergeStateStatus CLEAN
drift hint + guard message both name the same runnable command

The INVOCATION constant consolidation is the right call: this repo prints the same repair command
from several sites, and a hint that does not run is a second failure dressed as a fix.

Merge-order note (for the record)

This is the first of the four count-related PRs to land, so Agent.md:122 now becomes conflicted for
#1125 (documents 1315), #1126 (1327) and #1127 (1309). Each of those must be re-measured on
the merged tree
— never carried across. Measured this cycle's trial merges: 1124+1125 collects
1318, and 1124+1125+1126 collects 1338.

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