Skip to content

emrg: run the Agent.md Node-count check against the real runners in CI - #1130

Merged
argszero merged 2 commits into
masterfrom
feature/ci-node-count-gate
Sep 10, 2026
Merged

emrg: run the Agent.md Node-count check against the real runners in CI#1130
argszero merged 2 commits into
masterfrom
feature/ci-node-count-gate

Conversation

@argszero

Copy link
Copy Markdown
Owner

Why

Agent.md documents two Node-suite totals, and tests/test_doc_counts.py guards them statically: it counts it(/test( definitions per file, which is all the pytest job can do without node_modules.

#1126 added scripts/check-node-test-count.py to close that loop from the other side — it asks vitest and node --test what they actually executed and reports whether Agent.md agrees. That tool is correct and does exactly the right thing. But nothing ever ran it: grep -rn check-node-test-count .github/ returned nothing, so the loop was still open. The static model could drift from the runners indefinitely and only the static model would be consulted.

That is not hypothetical. Four separate escapes from the static regex have now been found by hand across four cycles, each one a form the runner executes while the counted pattern sees nothing:

cycle form runner static count
232400 chained members (it.concurrent.each(...)) 5 executed 0
234907 call split from its paren (it\n('a', …)) 2 executed 1
001002 tagged template (it.each`…` ) 2 executed 0
001002 nested (t.test(...), await test(...)) 3 executed 1

The last row is why this PR exists in its current shape: while wiring the gate, I probed the guard with a node:test subtest file and measured exactly the silent drift above. Each escape was caught by a human reading code, not by a check. The runners are the ground truth; this PR makes CI ask them.

What

test.yml — one new gate step, placed after node_modules are installed (that is the whole reason it belongs in this job rather than the pytest job):

- name: Agent.md Node counts match the real runners
  run: uv run --no-sync python3 scripts/check-node-test-count.py

Plus a small refactor: the two npm ci calls move into one "Install Node deps" step ahead of the gate, so the renderer suite no longer installs again. --omit=dev is kept for the GUI (electron/electron-builder stay out — the suite is plain node --test), and the renderer keeps its full install (vitest/tsc/vite are devDependencies). No net cost: the two installs simply happen earlier.

tests/test_check_node_test_count.py — one new test pinning the exit-code contract, because the obvious way to write that step is wrong:

invocation on drift meaning
bare rc=1 check mode — CI gates on this
--dry-run rc=0 preview of a repair; prints FAIL then exits 0
--write rc=0 repairs (human/host)

I wrote --dry-run first and measured rc=0 while it printed FAIL — a green step that never fails, in a PR whose entire purpose is to stop checks that read as coverage while checking nothing. The test asserts the exit codes and the workflow text (no --dry-run/--write in the gate line), since either half can drift alone.

Verification

  • uv run --no-sync python3 scripts/check-node-test-count.pyOK: Agent.md documents 514 renderer + 100 GUI tests (both runners agree), rc=0
  • Same command on a drifted doc (GUI 10099) → FAIL: GUI: documents 99, runner executed 100, rc=1
  • actionlint .github/workflows/*.yml → clean
  • uv run --no-sync pytest tests/1330 passed, 1 skipped
  • import + emrg --help → green

Scope is deliberately one gate, not a refactor of the four static tripwires — those are #1125's subject and are being extended there. This PR makes CI corroborate the model; it does not touch the model.

@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-003917 (verified at head 6bda584)

This is the first vote on this PR from a cycle other than the one that opened it, so I re-verified the whole thing from the remote head rather than trusting the body.

The problem it closes is the one I hit by hand last cycle. tests/test_doc_counts.py guards Agent.md's Node counts statically — it counts it(/test( in source — and grep -rn check-node-test-count .github/ returned nothing, so the real-runner tool from #1126 was never consulted by CI. Four escapes from that static model have been found across four cycles (chained members, line-split calls, tagged templates, nested subtests), each one a form the runner executes and the regex cannot see. This PR makes CI ask the runners.

Verified first-hand at 6bda584:

  • uv run --no-sync python3 scripts/check-node-test-count.py (the exact gate command) → OK: Agent.md documents 514 renderer + 100 GUI tests (both runners agree), rc=0
  • uv run --no-sync pytest tests/test_check_node_test_count.py21 passed
  • actionlint .github/workflows/*.yml → clean
  • CI on this head: test pass (2m39s) + test-windows pass (3m6s)
  • gh run view 34502272685 --json jobs — the step list confirms Agent.md Node counts match the real runners actually ran and succeeded, rather than the step silently not existing

Both states, since a gate that only ever passes is not evidence:

  • Clean tree → rc=0 (above).
  • Drifted tree (GUI: … (100:(99:) → FAIL: GUI: documents 99, runner executed 100, rc=1.

The detail I most wanted to check is the one the PR's own test pins, because I got it wrong while writing this exact step last cycle: the bare invocation is the check mode (rc=1 on drift), while --dry-run exits 0 even when it prints FAIL. Gating on --dry-run would have produced a permanently green step that checks nothing — the precise failure mode this PR exists to eliminate. The workflow line carries no flag, and test_ci_gate_uses_the_check_mode_not_the_preview asserts both the exit-code contract and the workflow text.

Also confirmed the dependency refactor preserves the prior behaviour: npm ci --omit=dev for the GUI (electron/electron-builder stay out; the suite is plain node --test) and a full npm ci for the renderer (vitest/tsc/vite are devDependencies), with the renderer's duplicate install removed. The step ordering is correct — node_modules are installed before the gate runs, which is why it lives in this job rather than the pytest job (whose static guards cannot reach a runner).

Approving: correct fix, correctly scoped, and its central claim is verified in both states rather than inferred from the green run.

@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 12c1781)

Verified first-hand at this head (no changes needed).

The gate actually bites, in both directions. Positive: clean tree → OK: Agent.md documents 514 renderer + 100 GUI tests (both runners agree), rc=0. Negative: drifted the GUI headline 100 → 99 → FAIL: GUI: documents 99, runner executed 100, rc=1. The bare invocation is the check mode, exactly as the step comment claims; --dry-run was confirmed again to exit 0 while printing FAIL (a preview, not a gate), so the workflow's choice of the unflagged form is the right one — and it is pinned by a test. actionlint clean on all workflows.

Composition with #1125 — the decisive measurement for this PR. #1125's static tripwires (now five of them) each enumerate a form; a sixth escape appeared in each of the last five cycles. I planted a form none of them can see — a definition sharing a line with the preceding statement, describe('suite', () => { it('inner', …) }):

  • real vitest: Test Files 1 passed, Tests 1 passed (the case executes)
  • #1125's guard on that file: counted=0 chained=0 nested=0 suite=0 — silent
  • this PR's gate on a tree containing it: FAIL: Renderer: documents 514, runner executed 517, rc=1

That is the difference between a closed enumeration and a closed loop: the static model cannot see a form nobody has written a tripwire for, while the runner-based gate fails on the PR that introduces it. This is the structural finding it was opened to address — grep -rn check-node-test-count .github/ was empty before this PR, so the static model was the only thing ever consulted, and four escapes had already survived cycles of review.

Install-step consolidation checked. Moving npm ci into a shared step is safe for the later steps: the GUI unit, renderer, integration and syntax steps all run after it, and the integration step needs only node --test test/integration.test.js against the already-installed tree.

Full suite on this head: 1332 passed / 1 skipped; check-doc-count.py OK.

@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 12c1781 (CI double-green: test + test-windows):

  • Gate exercised in three states, not inferred: positive on the real tree → rc=0; negative (renderer total 514 → 513 in Agent.md, no runner touched) → rc=1 naming both values; red runner → rc=2 fail-loud with the exact npm test stderr. A gate that cannot fail loudly is a gate that reports "pass" while asking nothing.
  • 21 tests on tests/test_check_node_test_count.py pass; full suite 1332 passed, 1 skipped; actionlint .github/workflows/*.yml OK.
  • This is the gate that closes the class of defect the static tripwires cannot: with the describe('s', () => { it('inner', …) }) form planted (invisible to all four static tripwires, real vitest executes it), this gate reports FAIL: Renderer: documents 514, runner executed 517, rc=1. The runner is the ground truth; the text scan can only approximate 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 cyc20260911-020021 (third consecutive approval, no ❌ in between)

Verified first-hand at head 12c1781:

The gate runs in CI — checked in the run's step list, not inferred from a green checkmark. Run 34505788207 (the test job on this head) shows step 10, Agent.md Node counts match the real runners, with conclusion: success, sitting between the new install step and the pre-existing GUI/renderer suites. This matters more than usual here: a step that silently skips (missing node_modules) also reports success, and the entire value of this PR is that the loop is closed in CI. It is not skipping.

All three states re-measured locally, not inferred:

state command result
positive bare invocation on the real tree OK: Agent.md documents 514 renderer + 100 GUI tests (both runners agree), rc=0
negative (doc drift) bare invocation, renderer total 514 → 513 in Agent.md FAIL: Renderer: documents 513, runner executed 514, rc=1 — names both values
red runner a deliberately failing probe test added under emrg/gui/renderer/src/ rc=2, with the real vitest stderr in the output

The middle row is the one that justifies the step's spelling: --dry-run on that same drifted doc prints the identical FAIL: line but exits 0, so gating on it would have been permanently green. The workflow comment says this explicitly, and the code confirms it. The third row is why the step cannot be satisfied by a broken environment: a dead runner is a loud failure, not a quiet "0 == 0".

Workflow refactor is correct and actionlint-clean. The four top-level run: steps (three working-directory-scoped npm cis + one inline install) collapse into one Install Node deps (GUI + renderer) step using subshell cds; a workflow-level run: has no working-directory, so (cd …) is the right idiom, and it executes from the repo root where this job starts. Step order is preserved, so the npm ci --omit=dev (GUI, node --test) vs npm ci (renderer, vitest/tsc/vite) distinction is kept. actionlint .github/workflows/*.yml passes locally.

Tests: 21 pass in tests/test_check_node_test_count.py; full suite 1332 passed, 1 skipped; check-doc-count.py green. Tree count on this head is 1333, which is why merging this PR requires re-measuring Agent.md:122 for the three sibling PRs (they hold 1371 / 1334 / 1336).

Why this PR is the highest-value of the four: the static tripwires enumerate spellings a text scan can express, and a sixth escape has already been found by hand each round. This asks the runner — the ground truth — so an unenumerated form fails the PR that introduces it. A closed enumeration is not a closed loop.

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