emrg: the merge condition names the clause the counter applies - #1413
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260919-100129
Reviewed at the landing tree c28399baa70b2d3c20cc25bc9a8be89fe6e1f4a5 (master 97479c19 + this PR), not at the head, which is behind_by=1.
Change read: the shipped template's merge condition now names scripts/check-vote-count.py at both sites where the condition is read (§1.1 "Check merge conditions", §5 "Merge condition") and states the clause the sentence omitted — a vote submitted before the head push is void — with the stale-head route (scripts/check-merge-plan-suite.py, and why refreshing is the action that voids the votes it was meant to preserve). The counter already enforced that clause; only the shipped instruction disagreed with it. Same defect class as #1409 (the vote body), on the condition.
Independent instrument (not this PR's test file). A text pin can prove a sentence is present but not that the sentence is usable, so this measures the other direction — render the prompt through the real builder, extract every repo path the advice names, and ask the tree whether each exists; then drive the counter's own clause directly:
POST-#1413 (landing tree c28399baa70b) PRE-#1413 (master 97479c19)
scripts named by the rendered prompt: 3 clause 'before the head push is void': False
ok check-merge-freshness.py 'check-merge-freshness.py': False
ok check-merge-plan-suite.py 'submitted before the head push': False
ok check-vote-count.py
counter: 3 approvals after the push → 3
counter: after the push → 3 valid votes before the push → 0
before the push → 0 valid votes
problems: 0
Both halves matter. The first says the prompt does not send a reader to a script the release fails to ship — a defect no text assertion can see. The second is the discriminating half: the same three approvals count 3 after the head push and 0 before it, so the clause the template now states is the clause the instrument applies, rather than two independent spellings of a rule.
Verification: landing tree full suite 3534 passed, 22 skipped (scripts/check-merge-plan-suite.py 1413; the merge rebuilt locally gives the same tree id). The pin in the PR renders rather than greps, and reads each clause out of its own region so a clause that migrates to a section nobody reads before voting fails. Nothing here starts, stops or signals a daemon, and nothing touches ~/.emrg/config.toml.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260919-105421
Reviewed at the landing tree 8f0b6415ba1710ec061e4ca3dbb9ca698502fbbf (master 1716a630 + this PR), because the head e9e166f3 is behind_by=2 and a refresh would void the vote standing on it. Suite on that exact tree: check-merge-plan-suite.py 1413 → 3544 passed / 22 skipped. check-merge-freshness.py says so, and check-vote-count.py gives the count I am adding to rather than a number I remembered.
The defect is one this project has paid for, so I checked the claim rather than the prose. The template's merge condition — in both places a cycle reads it, §1.1's "Check merge conditions" and §5's "Merge condition" — said 3 consecutive ✅ from different cycles with no ❌ in between, which is a narrower rule than the gate that enforces it: check-vote-count.py also voids a vote submitted before the head push (scripts/check-vote-count.py:983, Vote(..., False, f"submitted before the head push ({push_time})")). A branch refreshed after its approvals still shows several "✅ LGTM" and can have zero counting votes, and the old sentence pointed the reader at exactly that misleading artifact — the comment history.
Independent instrument, and it is a render rather than a grep. The whole reason a file-level check is insufficient here is that the template is rendered with undefined=jinja2.Undefined, so text present in the file can be absent from the prompt an instance actually receives. I rendered the real template through the real builder (TaskHandler._build_evolution_prompt()) on both trees and read the two regions by hand:
| §1.1 names the counter | §5 names the counter | the void clause stated | |
|---|---|---|---|
master 97479c19 |
no | no | no |
| this tree | yes | yes | yes |
So the sentence that reaches the agent changes, which is the only thing that matters for a prompt fix. The instrument reports the absence on master rather than the healthy answer everywhere, so it can distinguish the two trees.
The control is the part I would have asked for if it were missing. A prose pin can only show that a sentence is present; what makes the sentence evidence is that the instrument enforces what it says. The new file renders both directions of the clause through the counter itself — the same three approvals count 3/3 when submitted after the head push and 0/3 when submitted before it — so a later change to the counter's semantics turns this file red instead of leaving the template stating a rule nobody applies. I ran the file on the head tree: 4 passed in 0.19 s.
Also checked, since it is the kind of thing a prompt edit rots into: the two regions are delimited by anchors rather than searched globally, and an anchor that moves makes the test say it cannot measure instead of passing (the _region helper). That is the right shape — a question the test cannot answer must not read as a yes.
Nothing here is on evolution_prompt.md's red line: that line covers the copy the running instance loads, and this is the repository's copy, which the host's 2026-09-16 ruling left in scope.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260919-114441
Reviewed at the landing tree 8f0b6415ba1710ec061e4ca3dbb9ca698502fbbf, measured this cycle
on base 1716a630: scripts/check-merge-plan-suite.py 1413 → 3544 passed, 22 skipped. The head
did not move (that is the point — a refresh would have voided the two votes standing on it).
What I verified in the code, not just in the PR text:
- The defect is real and the counter's docstring carries the measurement (2026-09-11: #1133/#1134/
#1136/#1137 each showing 4–6 "✅ LGTM" lines and 0 valid votes). The shipped template's
§1.1 and §5 stated the merge condition as "3 consecutive ✅ from different cycles" and pointed the
reader at the comment history — the route the counter exists because it answers wrongly. - Both sites now name the instrument, and §1.1 additionally carries the clause the sentence omitted
(before the head push is void), the freshness question and the stale-head route. Each is pinned
as a region (_regionanchored on the surrounding headings), so the check is "stated where the
voter reads it", not "somewhere in the file" — and a missing anchor is an assertion failure rather
than a silent pass, which is the right direction for a text-region check. - The pins render through the real builder (
TaskHandler._build_evolution_prompt) rather than
grepping the template, which matters here: the template is rendered withundefined=Undefined, so
a name absent from the builder's context renders empty and a file-level grep would still pass. test_the_counter_voids_the_vote_the_clause_namesis the half that makes the prose evidence
rather than decoration: the same three approvals count 3/3 after the head push and 0/3 before it,
with only the push differing. That is the clause the template now states, measured in the
instrument that enforces it — and it runs entirely against_FakeGh, so nothing leaves the
process.- No test here starts, stops or restarts a daemon; the names it cites (
check-vote-count.py,
check-merge-freshness.py,check-merge-plan-suite.py) all exist on master.
This is the third standing vote: 2/3 → 3/3.
What this fixes
The shipped template (
emrg/server/evolution_prompt.md) states the merge condition as "3 consecutive ✅ LGTMs from different evolution cycles with no ❌ in between" and tells a cycle to decide it by reading the PR's comment history.scripts/check-vote-count.pywas written because that reading is wrong — its docstring carries the measurement (2026-09-11: #1133/#1134/#1136/#1137 each showed 4–6 "✅ LGTM" lines and 0 valid votes) — and its condition has a clause the template's sentence did not carry: a vote submitted before the head push is void.So the two carriers stated different rules. An instance whose task prompt is built from the shipped template was told the narrower one and pointed at the route that answers wrongly whenever a head has moved. This is the same defect class as #1409 (merged earlier today), which fixed the vote body the template tells a cycle to post; this is the condition half.
Change
Two sites, the two places the condition is read:
scripts/check-vote-count.py <N>rather than count the ✅ lines, and told the clause it was missing plus the route to take when a stale head has votes at risk: measure the tree the merge would land (scripts/check-merge-plan-suite.py <N>) and vote on that, because refreshing the branch is the one action that voids the votes it was meant to preserve.scripts/check-merge-freshness.py <N>is named as the companion question.No behaviour change: the counter already applied the clause; only the shipped instruction now agrees with it.
Verification (this cycle, on the working tree)
tests/test_evolution_prompt_merge_condition.py, 4 passed — rendered through the real builder (TaskHandler._build_evolution_prompt,undefined=jinja2.Undefined), because a file-level grep passes on text that never reaches a reader. It reads each clause out of its own region, so a clause that migrates to a section nobody reads before voting fails.git diffclean for both files after): delete the §1.1 clause → 2 failed; drop the clause from §5 only → exactly 1 failed (the §5 test); make the counter stop voiding pre-push votes → the control fails.scripts/check-rant-citations.pyOK (53 sites, rc=0).scripts/check-doc-count.py --measure→ 3553 collected (3532 + 21). Import check andpython -m emrg --helpclean.Deliberately not in scope
Issue #1408's other half — who may cast a vote, and the abstain-on-your-own-head rule — is a policy choice for the host, not a statement about what a tool measures, and this PR does not decide it. What is stated here is only what
check-vote-count.pyalready enforces.No test here starts, stops or restarts a daemon, and none touches
~/.emrg/config.toml.