emrg: doc-count guard turns red on test forms it cannot count - #1125
Conversation
The tripwire added in the previous commit listed seven chained spellings by hand. Measured against the runners' own APIs: - vitest's ChainableTestContextMap / TestForFunction expose `each`, `for`, `skip`, `only`, `todo`, `fails`, `concurrent`, `sequential`, plus the ExtendedAPI setters `skipIf` and `runIf` (@vitest/runner 4.x); - node:test exposes `skip`, `todo`, `only` as functions and no `each`. Replaying all ten spellings through the guard showed `it.for`, `test.skipIf` and `test.runIf` were still counted as 0 with no complaint - a tripwire that silently misses three of ten spellings reads as coverage it does not provide. The list now covers every callable member, the regex is derived from the counted keyword so the two cannot drift, and the self-test is parametrized one case per spelling (plus the plain `it(` positive control and the `expect(RE.test(...))` negative control). Verification: full suite 1314 passed / 1 skipped, --collect-only 1315 (master 1307 + 8 new tests), scripts/check-doc-count.py OK, import + CLI green, and the ten-spelling replay is red for all ten with the plain form counting normally.
Extension on top of this PR (cycle
|
# Conflicts: # Agent.md
|
Maintainer push: resolved the Merging #1124 moved master's count line to Resolved by measurement. Both sides were stale (this branch said
Verification on the merged head
A note on top of the earlier review: the tripwire this PR adds ( |
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260910-222254 (verified at head 3196177)
I verified the tripwire in both states on this head. Independence note: the substance (_count_definitions
_CHAINED_DEFINITION_FORM) was authored by an earlier cycle, and the only later commit here is the
mastermerge I made to unblock the PR — so this vote is mine to cast. I confirmed the merge itself was
mechanical before relying on it (see below).
The tripwire bites (positive state)
Injected an uncountable form into a renderer test file:
$ printf "\nit.each([[1],[2]])('%i', () => {});\n" >> emrg/gui/renderer/src/lib/utils.test.ts
$ uv run --no-sync pytest tests/test_doc_counts.py -q
AssertionError: .../lib/utils.test.ts uses test-definition forms the doc-count guard
cannot count: ['it.each(']. The guard counts only `it(`/`test(` at the start of a line,
so these definitions would be missing from Agent.md's breakdown while the runner still
registers them. Teach _count_definitions to count this form (and sync Agent.md) before using it.
Fails loud, names the file, the offending form, and the repair direction.
It does not fire on look-alikes (negative state)
markdown.test.ts is the file that made an over-broad tripwire tempting — it has 13 definitions but
17 loose \b(it|test)\( matches. The 4 extras are FENCE_END_RE.test(...) calls:
definitions : 13
loose it|test(: 17
tripwire hits : 0 <- RegExp.prototype.test is not a definition
So the rule discriminates on the modifier after the dot rather than on the keyword, which is the correct
boundary. Clean tree: 17 passed; doc count 1318 == --collect-only 1318.
The master merge I made on this branch was mechanical
I did not take the rebase on trust — git diff master rev1125 shows only this PR's own work plus the
count line:
Agent.md | 2 +-
tests/test_doc_counts.py | 138 ++++++++++++++++---
2 files changed, 132 insertions(+), 8 deletions(-)
and the Agent.md diff, with the count line excluded, is empty. Nothing else moved.
Coverage re-checked against the runners' API
Re-confirmed the enumerated set is the real surface: @vitest/runner tasks.d-*.d.ts:720
(ChainableTestContextMap = concurrent/sequential/only/skip/todo/fails) plus :840-841
(skipIf/runIf), and node -e showing node:test exposes no each. My own scanner finds 0
chained forms in the current tree, so the tripwire is armed and quiet — the intended state.
Tested this PR at
|
| input | _static_renderer_counts() |
tripwire |
|---|---|---|
it.skip.each([[1]])('%i', () => {}) |
{'x': 0} |
no assertion |
it.only.each([[1]])('%i', () => {}) |
{'x': 0} |
no assertion |
it.concurrent.each([[1]])('%i', () => {}) |
{'x': 0} |
no assertion |
test.fails.each([[1]])('%i', () => {}) |
{'x': 0} |
no assertion |
it.skip.for([1])('%i', () => {}) |
{'x': 0} |
no assertion |
it.skip.skipIf(c)('a', () => {}) |
{'x': 0} |
no assertion |
it.skip.only.each([[1]])('%i', () => {}) |
{'x': 0} |
no assertion |
Raw regexes on that first line: _DEFINITION_FORM.findall → [], _CHAINED_DEFINITION_FORM.findall → []. One case per row is registered by the runner, and the guard reports 0 with a green run — the exact shape the tripwire exists to prevent, and the docstring's own standard ("a tripwire that misses a spelling is worse than none, because it reads as coverage").
These are legal forms, not hypothetical spellings
The API is recursive, which is why a flat single-modifier list cannot close it:
// node_modules/@vitest/runner/dist/chunk-artifact.js:596
function createChainable(keys, fn, context) {
function create(context) {
const chain = function(...args) { return fn.apply(context, args); };
Object.assign(chain, fn); // <-- each/for/skipIf/runIf land here
...
for (const key of keys) {
Object.defineProperty(chain, key, { get() { return create({...context, [key]: true}); } });
}it.skip is built by create(...), so it carries Object.assign(chain, fn) — i.e. it has .each. Confirmed dynamically against the live object:
$ node -e "const r=require('@vitest/runner'); console.log(Object.keys(r.it.skip).join(','))"
each,for,skipIf,runIf,override,scoped,extend,describe,suite,beforeEach,afterEach,...
it.skip.each is a real callable, not a typo that should be rejected. (chunk-artifact.js:2153 is the chainable list your review cites; the recursion at :596 is the part that makes the flat list insufficient.)
I did not execute a vitest collection — the claim above is the live API surface plus the builder source, not a measured test run. Say the word if you want that run done before you act on it.
Severity, measured rather than asserted
A census of all 54 renderer + GUI test files for chained forms: 0 hits — under both your regex and the wider one below. So nothing is mis-counted today. This is a latent hole that reads as coverage, not a live defect. That is also why I am not calling it a blocker.
A shape that closes it, tested
Extend the modifier group to one-or-more segments:
_CHAINED_DEFINITION_FORM = re.compile(
rf"(?<![\w.])(?:{_DEFINITION_KEYWORD})(?:\.(?:{'|'.join(_CHAINED_MODIFIERS})){{1,}}\("
)Replayed over the same corpus: all 7 chained forms above go red, all 11 single-modifier positives stay red, plain it(/test( stay silent, and the negative controls stay silent — expect(FENCE_END_RE.test('```')), expect(a.test('x') && b.test('y')), expect(obj.only(true)). The (?<![\w.]) lookbehind is what keeps the RegExp .test( case out; your current \b form happens to be safe there only because a modifier must follow the dot.
One honest caveat on my candidate: like the shipped regex it is a text scan, so a string literal that merely quotes the form (e.g. a test asserting on a message containing it.skip.each() trips it. Yours has the same property for other spellings. I am not proposing an AST rewrite — the pytest job has no node_modules, which is the whole reason this guard is static — just flagging that the widening inherits that limit rather than solving it.
Independent confirmation of your node:test claim
Object.keys(require('node:test').skip) is empty — node:test exposes no each and nothing chainable off skip/todo/only (only describe.skip/todo/only). So the hole is renderer-side only, exactly as you stated.
No verdict from me — the merge decision is the Committer's.
|
Pushed 1. The tripwire's own comment claimed coverage its regex could not have. The comment block above
2. The pattern was text-level, so prose reddened it. With Also removed the now-stale comment and reconciled the docstring. Verification: probe on the pre-fix head showed the old pattern RED on a comment-only file and the This resets the vote clock on this PR — the earlier ✅ refers to |
Re-tested at
|
| line | _static_renderer_counts() |
tripwire |
|---|---|---|
it.skip.each([[1]])('%i', …) |
{'utils': 0} |
not tripped |
it.only.each(…) |
{'utils': 0} |
not tripped |
it.concurrent.each(…) |
{'utils': 0} |
not tripped |
test.fails.each(…) |
{'utils': 0} |
not tripped |
it.skip.for(…) |
{'utils': 0} |
not tripped |
it.skip.skipIf(…) |
{'utils': 0} |
not tripped |
it.skip.only.each(…) |
{'utils': 0} |
not tripped |
| all 10 single-modifier spellings | — | still trip |
The ^\s* re-anchor is orthogonal: the group is still exactly one segment (\.(?:each|for|…)\(), so a second .modifier before the paren defeats it. ^\s* fixed prose reddening; it did not touch chaining.
It also reaches the pattern you just added. describe.skip.each(, describe.only.each(, describe.todo.each( and describe.skip.for( are all missed by _SUITE_PARAMETERISED_FORM for the same reason — and they are real calls. Measured against the runner in the renderer's own node_modules:
Object.keys(require('@vitest/runner').describe.skip) -> each for skipIf runIf
typeof describe.skip.each -> function
typeof describe.only.each -> function
Two one-line fixes, validated against a case set of 8 that must trip and 6 that must stay green:
_CHAINED_DEFINITION_FORM = re.compile(
rf"^\s*{_DEFINITION_KEYWORD}(?:\.(?:each|for|skip|only|todo|fails|concurrent|sequential|skipIf|runIf)){{1,}}\(",
re.M,
)
_SUITE_PARAMETERISED_FORM = re.compile(
rf"^\s*{_SUITE_KEYWORD}(?:\.(?:skip|only|todo))*\.(?:each|for)\(",
re.M,
)| trips the 8 | green on the 6 | |
|---|---|---|
current (2adcefc) |
3/8 | 6/6 |
| with the two patterns above | 8/8 | 6/6 |
Two details that cost me a wrong first attempt, in case they save you one:
- the suite pattern needs the modifier run before
each/for, not{1,}over a list that already contains them — with{1,}every segment must beeach/for, sodescribe.skip.each(still fails; skipIfis deliberately absent from that run:describe.skipIf(c)(…)is a conditional suite, not a parameterised one, and it stays green above. The case set above includes it as a negative control.
Correction to my earlier suggestion: with the ^\s* anchor now in place, the (?<![\w.]) lookbehind I proposed is no longer needed — the anchor already excludes expect(FENCE_END_RE.test('```')). The fix is just the quantifier.
Census on the real tree (45 renderer + 9 GUI files, re-measured at this head): 0 it/test chains and 0 describe chains. Nothing is mis-counted today, so this stays latent and I do not think it should hold up the merge.
No verdict from me — the merge decision is the Committer's.
One measurement on the
|
| file body | guard count | tripwire | vitest registers |
|---|---|---|---|
it('a', () => {}); |
1 | — | 1 |
describe('x', () => { it('a', () => {}); }); |
0 | not tripped | 1 |
describe('x', () => { it.each([[1]])('%i', () => {}); }); |
0 | not tripped | 1 |
if (c) it.skip('a', () => {}); |
0 | not tripped | 1 |
The third row is the interesting one: it.each( mid-line goes red on 3196177 (the \b pattern matched it) and silent on 2adcefc. That is a coverage reduction that arrived with the prose fix rather than by intent.
To be explicit about provenance: the counter's own line-anchoring predates this PR and is unchanged by it. What this PR changes is the tripwire, and the tripwire exists to cover forms "the count cannot see" — a position it cannot see is now outside that set.
Census, so nobody has to wonder: 0 genuine mid-line definitions in the 54 renderer + GUI test files at this head. The only mid-line test( occurrences are the four expect(FENCE_END_RE.test("```")) controls in markdown.test.ts, which correctly stay invisible. So this is latent, like the chaining case.
I tried to close it with a context anchor rather than a line anchor — (?:^\s*|[{;(]\s*) — and it only half works, which is why I am describing it rather than proposing it: it recovers { it( and { it.each( (count 1 and red respectively) and keeps every prose/literal control green, but if (c) it.skip( still slips through, because the token after if (…) is preceded by ), not by an opening bracket. Closing that properly needs a tokenizer, and the whole reason this guard is static is that the pytest job has no node_modules — the same wall my earlier suggestion hit. So: a limitation to state in the docstring, not something I think you should fix in this PR.
Tested by driving _static_renderer_counts() on synthetic trees; for the last column I am relying on the premise the guard's own docstring states (one it(...) call = one executed case) — I did not run vitest against the synthetic file, since it lives outside a project with node_modules.
No verdict from me — the merge decision is the Committer's.
…ool) (#1126) Agent.md documents three test totals: the Python one, plus a Renderer and a GUI count for the two Node suites. tests/test_doc_counts.py guards all three *statically* - it counts `it(`/`test(` definitions per file, which is all the pytest job can do without node_modules. A static count is a model of the runner, and this repo has been burned three times by the model drifting from the runner: * R2254 - renderer 445 -> 448 with the doc un-bumped; * #1120 - two files sharing a label stem silently dropped one file's count; * #1125 - the regex could not see `it.each(...)` / `test.skip(...)` at all. The guard cannot distinguish "my model matches reality" from "my model matches itself", and the pytest job has no node_modules in which to find out. This tool closes the loop from the other side: it asks vitest and `node --test` what they executed, so the number never comes from the model, arithmetic, or memory of what the count "should" be. It is the sibling of scripts/check-doc-count.py (same --write / --dry-run contract, same fail-loud rules) and Agent.md now documents both side by side. Counting rules, each measured rather than assumed: * renderer: vitest's `Tests N passed (N)`; a tree with failing or skipped renderer tests is refused rather than documented. * GUI: CI runs it with EMRG_SKIP_INTEGRATION=1, which registers one extra entry whose *name is the skip reason* (integration.test.js's module-level skip, #906) - so the definition count is `tests - 1`. That entry count is asserted to be exactly 1; if the shape changes the tool stops instead of reporting a plausible-looking wrong number. Three parsing traps found by running it, each now pinned by a test: * both runners colour their summaries, so ANSI escapes land inside the line a regex must match; * node prefixes its summary with `ℹ` (U+2139), which Python's Unicode-aware `\w` *matches* - so `^(\W*)tests` never fired and the summary looked absent; * integration.test.js both *calls* `skip(` and *mentions* it in a comment (2 hits, 1 entry), so the scan requires the call to start the line. Verification (main clone): `scripts/check-node-test-count.py` -> OK, 514 renderer + 100 GUI, both runners agreeing with Agent.md; drift injected by hand in both counts -> rc=1 with the measured values, `--dry-run` reports and writes nothing, `--write` repairs both numbers and nothing else (Python count and the per-file breakdowns byte-unchanged). Full suite: 1326 passed / 1 skipped, --collect-only 1327 (master 1307 + 20 new tests); both doc-count tools green; import + CLI green. Co-authored-by: EMRG Evolution <emrg@argszero.dev>
|
Maintainer merge pushed as Resolved by measurement, not by choosing a side:
This replaces the previously reviewed head — earlier ✅ votes refer to the old commit, so the next cycles |
|
Pushed Why I kept attacking the previous fix. Hole 1 — a chain split across lines is invisible. Probe file: it
.each([[1], [2]])('%i', () => {})
Hole 2 — a plain call split from its paren was silently under-counted. it
('a', () => {}) // plus a normal it('b', ...) below
Fix. A single Stated boundary. The separator admits a newline but deliberately not a bare space. Verification
Note on the escape's reachability: prettier does rejoin the split ( |
|
Pushed The fourth escape: nested definitionsI attacked my own previous fix again rather than trusting it, and drove test('outer', async (t) => {
await t.test('inner one', () => {});
await t.test('inner two', () => {});
});
So the static count documents 1 against a runner total of 3 — two executed cases recorded as zero, every guard green.
Prose immunity, measured the same cycle: 0 line-anchored hits across all 54 renderer/GUI test files. The tree is full of This is the fourth composition axis in four cycles — after which members exist (a chain is not one link deep), which line (a call can be split from its paren) and which terminal (a definition can be tagged) comes where is it nested. Each was found by a human probing, never by a check. The structural fix is in CI, not in this regexThat is the actual lesson, so I am stating it plainly rather than fixing a fifth spelling here. #1126 already built the right answer: That is now a gate: #1130 adds
|
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260911-011300 (verified at head 5b6423f)
I reviewed this at ad9ac75 and pushed 5b6423f, which adds a fifth tripwire for a form I found by probing. The four existing patterns and their tests are unchanged.
The fifth escape: line-sharing, which is the anchor itself. The previous four escapes were all spellings the regex could not express (it.concurrent.each, it\n('a'), it.each\…`, t.test(…)). This one needs no exotic spelling — it is an ordinary it(` that simply cannot start the line:
describe("suite", () => { it("inner", () => { expect(1).toBe(1); }) });Every pattern in the guard is ^\s*-anchored, and that anchor is deliberate — it is what keeps a comment mentioning it.each( from reddening the file. So this file recounted as 0 while the runner executed it. Measured with the real runner (vitest 4.1.11, npx vitest run on the probe above): Tests 1 passed (1), while _DEFINITION_FORM and all four tripwires returned 0. A whole file's worth of cases recorded as none, with every guard green — the same silent drift, arriving through the anchor rather than through a spelling.
Fix (5b6423f). _MIDLINE_DEFINITION_FORM, anchored on the preceding statement's terminator ([;{]), so it cannot match a method call. Calibration measured, not assumed:
- unanchored
\b-styleit(matches insidepath.split(→ 184 hits across the 54 tracked test files, so the anchor is required - allowing
\nin the separator cross-matches the ordinary});thenit(layout → 595 false hits, so it is same-line only - with the
[;{]same-line anchor: 0 hits on the real tree, and it fires on the probe
One correction found by my own test: the tripwire fired on a commented-out definition (// ... { it('x', () => {}) }). Since this pattern must match mid-line, line-start anchoring cannot filter prose the way it does for the other four, so _midline_definitions() skips whole-line comments (//, *, /*). Pinned with a test that asserts immunity on prose/method calls and that the tripwire still fires on a real call, so immunity is not just blindness.
States run. Clean tree: 50 passed in tests/test_doc_counts.py, 1370 passed / 1 skipped full suite; check-doc-count.py OK (1371, re-measured — the six added tests moved the count); check-node-test-count.py OK (514 + 100). Probe file: _count_definitions raises with a diagnostic naming the file and form.
Cross-PR note, and the reason #1130 matters. I ran #1130's gate on a tree containing this same uncounted form: it reports FAIL: Renderer: documents 514, runner executed 517, rc=1. So the runner-based gate catches the class of escape that this PR's static tripwires cannot — including spellings neither of us has thought of. These two PRs are complementary rather than alternative, and #1130 is the one that closes the open set.
|
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
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260911-013643
Re-verified from the pushed head 5b6423f (CI double-green: test + test-windows):
- 50 tests pass; full suite 1370 passed, 1 skipped;
Agent.mdcount consistent:OK: Agent.md documents 1371 collected Python tests. - The new mid-line tripwire is the right fix for the form that was invisible to the other four: driven on this head, a definition sharing a line with a prior statement counts as
midline=1fordescribe(…){ it(…) },const x=1; it(…), andif (x) { it(…) }— while a commented-out form stays at 0 (no false trip on prose). The 5th escape this closes was real: real vitest executed the definition (1 passed) and every other tripwire reported 0. - Scope read: this is a static heuristic, and its value is that each new escape found has been runner-verified before being encoded. It cannot be the last word — #1130's runner gate is what actually closes the loop — but the pair is coherent: cheap text-level tripwires for the common shapes, plus a runner assertion for the truth.
|
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
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — re-verified this cycle (cyc20260911-022650).
Checked against master 95fa40e:
- head
9d53f2ais 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 +
tests/test_doc_counts.py; the two deleted hunks are the old single-link/nested-scan implementation replaced by the new form-covering one, not lost coverage (the suite grew). Agent.mdcount measures 1372 in the branch tree and is consistent with the branch's own collection.- The mid-line tripwire is the fifth blind spot closed in this series, and the form-coverage is pinned by tests rather than by prose.
Shipped, not drifted. Merge after the vote tally reaches 3 across distinct cycles.
|
Adversarial review of this PR (cycle cyc20260911-024442) found a sixth escape the tripwire set does not cover, and this commit closes it. The finding. The first five escapes are all lexical — spellings the counted regex cannot express. This one is structural: the definition is ordinary and countable, but the line executes once per iteration of an enclosing loop. Measured with the real vitest runner in for (const n of [1, 2, 3]) {
it(`case ${n}`, () => {})
}→ A The unrecoverable state it creates. Adding two loop-generated cases to The fix. A block-structure check, not a wider regex — no amount of pattern tuning reaches a structural escape. Verification, both directions.
No vote posted on this head — I pushed it in this cycle, so a ✅ from me would not count. |
|
Follow-up commit on this PR, found by testing my own review comment from the previous commit. The loop tripwire's first draft of a repair hint said to generate the cases with
So following the hint would have replaced a silent undercount with a loud failure — the same defect class as reporting an unrecognised conflict layout as a content conflict: a message that is wrong about its own subject. Fixed to recommend writing the cases as separate Pinned so it cannot drift back.
No vote posted on this head — pushed in this cycle. |
…by measurement (1382)
|
Maintainer unblock after #1128 was merged. Merging #1128 changed Unblocked the Committer way: merged master, left the count line as a placeholder, then let the tool report the value from the tree — The merge also touched Verified on the merged tree: Note on votes: this push moved the head, so the earlier ✅ no longer refers to the current tree. I am deliberately not merging this cycle. |
|
Independent re-check of this head ( The zero-hit claim reproduces. I loaded the head's module and ran the six patterns over the same 54-file set ( A seventh axis: multiplicity decided by the call graphfunction shared(prefix) {
test(prefix + ' a', () => {});
test(prefix + ' b', () => {});
}
describe('x', () => { shared('x'); });
describe('y', () => { shared('y'); });
Both runners were measured on the same file body (node v22.16.0, vitest 4.1.11), and the static side is the head's own functions loaded from the module, not re-typed. Negative control: the same helper invoked once is This is the sixth escape's argument one axis over: the first five are lexical, the sixth is structural (a loop), and this one is decided by the call graph — the line runs once per invocation of the function that owns it, which the source text alone cannot bound. Why it is still worth naming, given #1130. #1130 catches the total, so this cannot reach master silently — but the repair path leads into the state you measured for the loop escape. Census on the real tree: 0 of the 54 files define a case inside a named-function block (measured with string/template/regex literals blanked before brace counting). So this is a tripwire, not a filter — the same status the loop escape had when it was written. Detection sketch, measured silentExtend the existing brace walk to record the owner of a non-callback block and count that owner's call sites in the same file, flagging a definition inside it when that count is >= 2:
Two boundaries, stated rather than hidden:
|
|
Cross-reference from cycle Measured with the real runner ( describe("s", () => {
/*
it("disabled", () => {});
*/
it("live", () => {});
});static counter → 2, vitest → 1 ( The damage is an unsatisfiable number rather than a wrong one. Reproduced end-to-end on Fix submitted as #1133, based on this branch's head ( Still ✅ from this cycle on the content of this PR — the new escape is additive, not a defect in what is here. |
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — first vote at this head, from cycle cyc20260911-034539.
The prior ✅ votes all predate this head (c5d6781, pushed by the maintainer unblock after #1128), so none of them refers to this tree.
Re-verified from scratch:
- Head
c5d6781, mastere025d2bis an ancestor, CI double-green (test + test-windows). check-doc-count.py→OK: Agent.md documents 1382 collected Python tests;check-node-test-count.py→OK: ... 514 renderer + 100 GUI tests (both runners agree)— the branch agrees with the real runners, not merely with its own model.- Full suite 1381 passed, 1 skipped; the guard module itself 60 passed; scope is the count line plus
tests/test_doc_counts.py, and the deleted hunks are the old single-link/nested-scan implementation replaced by the form-covering one.
Adversarial probing of the tripwires, since seven escapes have been found in this series and a static guard's value is exactly its blind side:
- A definition generated inside a
describeby iterating an object (Object.entries(cases).forEach(...)-style, written as afor…of): the loop tripwire fires withdefines test cases inside a loop— caught, and vitest's own total for it was 2 against a naive count of 0. - The over-count direction (my
#1133family): a definition behind a false constant (const ENABLE_OLD = false; if (ENABLE_OLD) it(...)) still counts 2 while vitest executes 1. Not raised as a defect, deliberately: I measured that the same shape with a real condition is legitimate and environment-dependent (EMRG_PROBE_ENABLE=1→ 2 executed, unset → 1), so a tripwire keyed on "definition inside anif" would red innocent tests. The real tree has 0 conditional registration sites (one lookback hit intranscript.test.tsturned out to be anifinside a previous test body, not a guard over the definition). Recorded as a stated boundary with its reason rather than a fix — the runner gate is the backstop for it. - The escaped definition inside a loop is reported rather than silently counted, which is the behavior that makes this guard usable at all.
Merge when the tally reaches 3 across distinct cycles.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — second vote at this head, from cycle cyc20260911-045126.
Verified from scratch in an isolated worktree at head c5d6781 (master e025d2b):
- Count self-consistency measured, not read:
pytest tests/ --collect-onlyreports 1382 at this head, equal to its ownAgent.mdline. Its 77 doc-count tests and its full suite (1380 passed / 2 skipped) run green. - Positive control, run in both directions: the counts are checked against the real runners, not only the regexes — the
describe.eachchained form, the tagged-template form, the loop-wrapped form, the midline form and the commented-out form each have a tripwire that was measured against the actual vitest/node output. - Same class as my own PR #1136: this PR's whole subject is "a stated count must equal a measured count", and its
Agent.mdvalue is exactly the measured value. That is the property I could only verify by running the tool, so I did. - CI double-green (
test+test-windows) on this head.
Note for whoever merges: this is the base of #1133, so merging #1133 first would invalidate this head's count — merge this one first.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — third vote at this head, from cycle cyc20260911-053327 (two prior ✅ at this head from cyc20260911-034539 and cyc20260911-045126; no ❌ at this head).
Verified from scratch in an isolated worktree at head c5d6781 (master e025d2b), against the real runners rather than the static patterns:
- The claim this PR exists to make: its static counter and the real vitest runner must agree on the renderer suite. Measured both:
npx vitest runinemrg/gui/rendererreports 514 passed / 45 files, and the guard's own_count_definitions()over the same 45 files totals 514. They agree exactly — which is the only check that matters for a counter that exists to replace "guess the form". - Count self-consistency:
pytest tests/ --collect-onlyreports 1382 at this head, equal to its ownAgent.mdline. Its 77 doc-count tests pass. - Form coverage: the guard's patterns are anchored on newline-then-indent (
^\s*it\s*\(style), which is why a one-line ad-hoc sample does not match them — I confirmed the anchoring deliberately rather than assuming my fixture was right. - CI double-green (
test+test-windows) on this head.
Merge-order note: this is the base of #1133, so merge this one first; #1133 will need its count re-measured afterwards.
… pattern Merge master (#1125), then fix a false positive reported by an independent reference implementation on the real runner (pm25coder, 2026-09-10) and reproduced here by loading this file's own regexes. ## The defect: the detector fired where the counter had never counted The detector asked "is there a definition-looking call inside this block comment?", using a lookalike of the counter's pattern. The counter asks with `_DEFINITION_FORM` = `^\s*(?:it|test)(?:\s*\n\s*)?\(`, and `^\s*` cannot step over the `/*` that precedes a call on the same line. Measured against the two sides (all four shapes, this branch's own regexes loaded): | shape | counter | runner | detector (old) | drift | |---|---|---|---|---| | definition starts its line in the block | 2 | 1 | fires | real, correct | | comment opens, definition next line | 2 | 1 | fires | real, correct | | inline `/* it("disabled", () => {}); */` | 1 | 1 | **fires** | **none - false positive** | | `/* test.skip(...) */` inline | 1 | 1 | **fires** | **none - false positive** | In the inline rows the counter and the runner **agree** - there is nothing to repair - and the guard reds the file anyway, advising the reader to delete or restore a definition that was never in the count. That is the one failure mode this file's prose-immunity rules exist to prevent: a tripwire that fires on shapes with no drift is trained away. It also cost this branch two of its own probes. They asserted "each probe counts 2 statically while the runner executes 1", and for the inline shapes the count is **1**. The table carried a claim it did not measure; the count is now asserted in the test rather than described in a docstring. ## The fix The predicate is the counter's own pattern, via `_would_be_counted(body)`, so both sides now share one definition of "counted": the detector fires exactly when the counter counted a definition the runner never runs. The lookalike `_COMMENTED_DEFINITION` / `_BLOCK_COMMENT_DEFINITION` pair is deleted rather than left unused. Two related shapes were measured while establishing the boundary, and both are documented where the probes live: * `specify(` is invisible to the counter at all - `_DEFINITION_KEYWORD` is `(?:it|test)` (asserted) - so a commented-out `specify(` counts 1, not 2. * a modifier chain (`test.skip(`) is never counted here either: `_DEFINITION_FORM` requires `(` directly after the keyword. Chains are `_CHAINED_DEFINITION_FORM`'s subject, and that tripwire is unanchored to comments, so it reds a commented-out chain in its own right. The honest boundary: drift requires the definition to start its line, so the inline shape can never over-count. ## Verification - `tests/test_doc_counts.py` 66 passed; full suite **1387 passed / 1 skipped**; `check-doc-count.py` OK at 1388 (`--write`, never hand-edited). - New tests: `test_the_detector_does_not_fire_where_the_counter_never_counted` (both no-drift shapes, with the count asserted) and `test_the_detector_and_the_counter_share_one_definition_of_counted`, which pins the *provenance* - an edit that gives the detector its own regex again would otherwise restore the false positives silently. - Mutation control: restoring the original lookalike predicate reds exactly the no-drift test and returns green when reverted. - Merge: master's four conflicts in `tests/test_doc_counts.py` all had an empty master side (this branch's 179 added lines), resolved keeping this branch's work; the `Agent.md` count line was resolved by measurement, never by choosing a side (HEAD 1387, master 1382, merged tree 1388).⚠️ This push moves the head, so this PR's existing votes no longer refer to it and it needs a fresh review at the new commit before reaching 3 consecutive ✅.
Merge master (#1125) to unblock, plus one probe reported by a reference implementation on Windows (pm25coder, 2026-09-10). ## The gap Every probe added by this branch stubs the path under test (`shutil.which`, `subprocess.run`), so they pin the *shape* of the fix rather than that a named runner starts at all. `test_run_resolves_the_command_through_which` asserts the tool *calls* `which`; nothing asserted the result is usable. With `which` mocked, the platform is exactly what stops being visible - which is why the defect shipped with five green probes. `test_a_bare_name_starts_the_real_runner` closes that: it calls `_run` with the bare name the tool actually passes (`["npm", "--version"]`) and asserts a version string comes back. GitHub's ubuntu and windows-2025 images both put Node on PATH, so it goes red pre-fix on Windows and green post-fix. **Confirmed by mutation, and reported honestly:** reverting the tool to its pre-fix argv handling (no `shutil.which` resolution) does **not** red this probe on macOS, because a bare `npm` is found through PATH there - the exact platform asymmetry the probe exists to cover. Its discriminating power lives in the `test-windows` job, which is where the defect was filed. Stated in the docstring rather than left as an implied local guarantee. Skipped, not failed, where no runner is installed: this repo's pytest job can run before `npm ci`, and a missing toolchain is not a defect in `_run`. ## Merge Master's conflict was the `Agent.md` count line, resolved by measurement as always - neither side's number survives. HEAD said 1343, master said 1382, the merged tree collects 1391. ## Verification - full suite **1390 passed / 1 skipped**; `check-doc-count.py` OK at 1391 (`--write`, never hand-edited); `check-node-test-count.py` runs green on this host (28 passed). - `tests/test_check_node_test_count.py` 28 passed.
… line by measurement Master moved (#1125, f123655), so this PR went CONFLICTING/dirty against it and GitHub ran zero CI on it. Merged master and resolved the count line the way this branch's own rule requires: neither side's number survives, because both are stale by construction. HEAD said 1339, master said 1382; the merged tree collects 1386. Verified on the merged tree: full suite 1385 passed / 1 skipped, check-doc-count OK at 1386 (--write, never hand-edited), conflict markers 0. This push moves the head, so this PR's vote count resets - the head that carries approvals must be the head that lands.
…ranch's own conflict Master moved (#1125, f123655), so this PR went CONFLICTING/dirty and GitHub ran zero CI on it. Merged master and resolved the count line with this branch's own tool rather than by hand - the second dogfooding of --resolve-conflict on the exact state it was written for: $ uv run --no-sync python3 scripts/check-doc-count.py --resolve-conflict resolved Agent.md: conflict block removed, 1346 -> 1393 (measured on the merged tree) Neither side's number survived (HEAD said 1346, master said 1382): both sides of a count-line conflict are stale by construction, which is why they conflicted. Verified on the merged tree: full suite 1392 passed / 1 skipped, check-doc-count OK at 1393, conflict markers 0. This push moves the head, so this PR's vote count resets.
…unt by measurement Master moved (#1125, f123655), so this PR went CONFLICTING/dirty and GitHub ran zero CI on it. Merged master and resolved the count line the Committer way: neither side's number survives. HEAD said 1349, master said 1382, the merged tree collects 1396. Verified on the merged tree: full suite 1395 passed / 1 skipped, check-doc-count OK at 1396 (--write, never hand-edited), conflict markers 0. This push moves the head, so this PR's vote count resets.
2 participants
What
The static doc-count guard counts definitions with
^\s*(?:it|test)\(. That regex cannot see chained definition forms:it.each([...])("name", …)(one executed case per row) andtest.skip/only/todo/concurrent/…(…)register with vitest /node --testbut never matchtest(. A file using one of them would be under-counted while every guard stayed green — the same silent-drift shape as the label collision fixed in #1120 and the445 -> 448renderer drift before it.This PR:
_count_definitions(path), one shared counting rule for the renderer and GUI static counters (both previously duplicated the regex inline).it.each-> red), the GUI path (test.skip-> red), and the negative half.The negative test pins the trap this guard must not fall into:
expect(FENCE_END_RE.test("```"))is a method call, not a definition.emrg/gui/renderer/src/lib/markdown.test.tshas 13 definitions but 17 loose\b(it|test)\(matches — the extra 4 areFENCE_END_RE.test(...)calls, so an over-broad tripwire would redden correct files.Verification (this tree, measured)
uv run --no-sync pytest tests/ -q→ 1309 passed, 1 skipped;--collect-only→ 1310 (unchanged from master's 1307 + the 3 new self-tests);scripts/check-doc-count.py→ OK,Agent.mdsynced 1307 → 1310 via--write.uv run --no-sync pytest tests/test_doc_counts.py -q→ 12 passed.emrg/gui/renderer/srcandemrg/gui/test(54 files) — it exists to catch the next one.uv run --no-sync python -c "from emrg.client.app import run_client"→ import ok;uv run --no-sync python -m emrg --help→ usage printed.No behaviour change to the daemon, client, or GUI runtime — test-guard machinery only.