Skip to content

emrg: a suite that could not run is not a suite that failed - #2516

Merged
argszero merged 1 commit into
masterfrom
fix/toolchain-a-subprocess-may-not-inherit
Oct 23, 2026
Merged

argszero merged 1 commit into
masterfrom
fix/toolchain-a-subprocess-may-not-inherit

Conversation

@argszero

Copy link
Copy Markdown
Owner

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

  • The block begins with 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.
  • The masking || echo "⚠️ test failures" is gone. It named a cause it could not know: a 127 from 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.
  • A new paragraph states the rule: a suite that could not be run is not a suite that failed. Name the command and its exact error, state that the suite did not run, and offer no verdict about that tree — read as a failure it invents a defect the project does not have, and read as green it claims a run that never happened.

Why

The template already distinguishes "could not measure" from "measured clean" everywhere else — gh pr checks reporting 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

  • New: tests/test_scheduler.py::test_open_source_template_distinguishes_an_unrunnable_toolchain — renders the template through Jinja2, in the shape the neighbouring open_source_prompt.md tests 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.
  • Checked in both directions: reverting the template reddens it (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 9280f881 before this change existed and does not carry it — the tag is not moved, and no claim here says otherwise.

@how2how2how2-arch how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

✅ 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 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 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 -v resolver → FAILED, same node id
  • restoring ⚠️ test failures on 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 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 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.

@argszero
argszero merged commit f1ed9d5 into master Oct 23, 2026
2 checks passed
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.

open_source_prompt.md §3.1.6 reads an unrunnable toolchain as a failing test suite

2 participants