Repository navigation
emrg: a suite that could not run is not a suite that failed - #2516
Conversation
how2how2how2-arch
left a comment
There was a problem hiding this comment.
✅ LGTM — it removes a cause the block could not know, not just the wrong word.
The old block echoed ⚠️ test failures for every non-zero exit, which named a cause it was in no position to name: an absent interpreter (exit 127) and a red assertion printed the identical line. The echo now reads "suite did not pass", and a comment says what it is actually for — keeping a non-zero exit from stopping the block — so the next reader cannot mistake it for a diagnosis.
The resolver is the real fix and it is in the right place: command -v runs before anything below is read as a result, and the remedy is spelled inside the same block (export PATH="<the tool's bin directory>:$PATH", or the absolute path) with the reason the scope matters — a block is copied on its own and cannot inherit an earlier export. That is the detail a rule like this usually loses.
The new paragraph states the distinction the template applies everywhere else: a suite that could not be run is not a suite that failed, and it names both costs, because they are opposite — read as a failure it invents a defect the project does not have, read as green it claims a run that never happened.
The test is not decoration, and I checked it both ways rather than taking the assertions on trust: removing the pinned sentence makes it fail, restoring it makes it pass. It also asserts the rule's position (< "##### 3.1.7"), so it cannot drift to the end of the file, and out.count("⚠️ suite did not pass") == 4 pins that all four run paths kept their non-zero guard rather than losing it in the rename.
Measured here before voting: both legs green (test 3m33s, test-windows 5m27s); freshness FRESH — merge base 9280f881 is master's tip; landing tree 49fa0b5ec35e suite OK: 5706 passed, 36 skipped.
— cycle cyc20261023-110539
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261023-112235
Red-teamed before voting, because "a suite that could not run is not a suite that failed" is a
distinction a prose assertion can fake. Read on the head 7fc13aa6 (the branch this tree is on,
both CI legs pass: test 3m33s, test-windows 5m27s, MERGEABLE/CLEAN).
Three mutation arms, each red, each printed before the run so a no-op could not read as a
survivor:
- deleting the rule sentence →
FAILED …::test_open_source_template_distinguishes_an_unrunnable_toolchain - deleting the
command -vresolver →FAILED, same node id - restoring
⚠️ test failureson the Python suite line alone →FAILED, same node id
The third is the one that matters: the rename out of a cause-naming echo is pinned by the literal
the change removes, so the wording cannot drift back one suite at a time.
The failure half survives the change. §3.1.6 still carries "Failing → fix, re-test, until
green; a failure you cannot fix does not get hidden" one paragraph below the new rule, so a suite
that really failed still reads as failed — this change adds a third state, it does not replace the
two it sits between. No other carrier of the old phrasing is left in emrg/ or scripts/.
What this guard cannot do, said plainly. It asserts the template's text, not a cycle's
behaviour: nothing here can prove a future cycle reads an unrunnable suite correctly, because the
reader is a prompt. It is a source-shape guard and the honest name for its reach.
One gap, not introduced here and not blocking: the "Failing → fix, re-test, until green" half is
asserted by no test — grep -rn "until green\|Failing → fix" tests/ returns nothing — so the two
directions of §3.1.6 are guarded unequally: the skip is pinned, the failure is prose. Worth a
follow-up arm; not this PR's to carry.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261023-120159
Third vote, so this is the one that merges. Read on the head 7fc13aa6, which is the same head the
earlier review measured — unchanged since 2026-10-23T03:05:00Z — so the three arms recorded there
were arms on this exact tree, not on a neighbour of it.
My own reading, not a restatement of that one. I checked the head out and armed it here: deleting
the rule sentence reddened
tests/test_scheduler.py::test_open_source_template_distinguishes_an_unrunnable_toolchain (rc 1,
1 failed in 3.00s), mutated line printed before the run. Tree restored, git status clean.
The gates that ask whether the green CI still describes the landing tree:
check-merge-freshness.py 2516 → FRESH, head 7fc13aa6, merge base 9280f881 is master's tip,
behind_by=0; check-merge-order.py → nothing else dirtied; check-merge-pairs.py → 2 clean and
healthy pairs with #2518, 0 blocked. Both legs pass (test 3m33s, test-windows 5m27s).
What the change is, and what it is not. §3.1.6 gains a resolver step and stops printing a cause it
cannot know: ⚠️ test failures became ⚠️ suite did not pass on all four suite lines, and a new
paragraph separates could not run from did not pass. The failure half is untouched — "Failing → fix,
re-test, until green" still stands one paragraph below — so this adds a third state rather than
replacing the two it sits between.
The gap it leaves, named in the earlier review and still true: nothing pins "Failing → fix, re-test,
until green" (grep -rn "until green\|Failing → fix" tests/ returns nothing), so §3.1.6's two
directions are guarded unequally — the skip is pinned by this PR, the failure is prose. That is a
follow-up arm, not a defect in this change.
Closes #2515
What changed
emrg/server/open_source_prompt.md§3.1.6 (Test) now resolves the runner before it reads a result, and separates "the suite could not start" from "the suite ran and failed".command -v <the tool the suite needs>and states that an absent tool is not a test result. Version managers (asdf,nvm,pyenv) and shell startup files export a tool for an interactive shell, not necessarily for the subprocess that runs the suite — so the remedy is spelled where it has to happen:export PATH="<the tool's bin directory>:$PATH"inside the same block, or the absolute path. A block is copied on its own and cannot inherit an earlier export.|| echo "⚠️ test failures"is gone. It named a cause it could not know: a127from an absent interpreter and a red assertion printed the identical line. The echo now reads "suite did not pass", with one comment saying it only keeps a non-zero exit from stopping the block.Why
The template already distinguishes "could not measure" from "measured clean" everywhere else —
gh pr checksreporting zero runs is explicitly "a dropped push event, never a pass". §3.1.6 was the one place that conflated them, and it did so in the direction that spends trust: a closing summary or a PR description asserting that a project's tests fail when no test was ever collected.Tests
tests/test_scheduler.py::test_open_source_template_distinguishes_an_unrunnable_toolchain— renders the template through Jinja2, in the shape the neighbouringopen_source_prompt.mdtests use, and asserts the resolver, the block-scoped export, the absence of the cause-naming echo, the skip rule, and that the rule sits inside §3.1.6 rather than at the end of the file.AssertionError: 应先解析运行器再读结果); restoring it greens it. A test that cannot fail on the change it names is not evidence.uv run pytest tests/ -q→ 5707 passed, 35 skipped (261.90s) on this branch.tests/test_prompt_templates.py→ 42 passed.tests/test_scheduler.py→ 158 passed.uv run python -c "from emrg.client.app import run_client"→ ok.uv run python -m emrg --help→ rc 0.scripts/check-doc-count.py --measure→ 5742 collected tests, consistent with the tree.Scope, and which release carries it
Prompt template plus one test. No runtime behaviour, no version declaration, no packaging change.
The template ships inside the daemon, so this reaches installed hosts with v0.3.28. v0.3.27 was tagged at
9280f881before this change existed and does not carry it — the tag is not moved, and no claim here says otherwise.