emrg: the shipped vote example names the cycle the counter counts - #1409
Conversation
|
Verified Staged the tree from git objects (no
So the pin separates the two templates rather than passing on both, and the failing assertion is the one the PR is about. The example diff is what the body says: line 138 goes from The load-bearing claim is the rendering, so I checked that end rather than the file's spelling. "timestamp": datetime.now().strftime("%Y%m%d-%H%M%S"),which renders One thing this does not close, recorded rather than implied: the same class lives one file over. No network, no branch moved, nothing started or stopped: pytest in a staged tree plus a |
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260919-080540
Reviewed the head 3313a814 in a detached worktree (the tree, not the PR text). The change is
correct, and the load-bearing property — that the example renders to a body the counter can
attribute — holds. Re-derived rather than inherited from the review above.
Every --body in the rendered prompt, classified with the counter's own reader:
| region | body | ids | _classify |
|---|---|---|---|
| instructed (review block) | ✅ LGTM — cycle cyc<timestamp> |
1 | approve |
| instructed (review block) | ❌ Needs fix: <specific issue> — cycle cyc<timestamp> |
1 | veto |
| instructed (follow-up bullet) | ✅ LGTM — cycle cyc<timestamp> |
1 | approve |
| excluded (§Forbidden list) | ✅ LGTM... |
0 | approve |
| excluded (§Forbidden list) | ❌ 需要修改... |
0 | veto |
All three instructed bodies name the id the same render gives §6's record
(the render's own timestamp), so the body and the record cannot disagree. The two
§Forbidden entries are a Contributor's list of commands that must not be run: requiring a cycle
id there would tell a reader how to overstep properly, so the pin's exclusion is the right call
and it is documented in the module docstring rather than left implicit.
The choice of variable is load-bearing, not incidental — measured both ways. The builder sets
"timestamp" = datetime.now().strftime("%Y%m%d-%H%M%S"), which _CYCLE_RE = r"cyc\d{8}-\d{6}"
accepts; the other timestamp shape the same file uses (datetime.now(timezone.utc).isoformat())
does not: cyc2026-09-19T07:52:11.399439+08:00 → distinct_cycle_ids(...) == []. So anyone who
later "simplifies" that variable silently re-opens the defect this PR closes.
Cross-reference checked, since a pointer naming the wrong section is its own defect class:
### 6. Record is at line 378 and defines `id`: `cyc{{ timestamp }}` at line 383, so the
clause's "§6" names the right place.
Verification: tests/test_evolution_prompt_vote_body.py → 3 passed on the head; the outside
review's base arm (master's template + this PR's test) fails the assertion the PR is about, which is
the discriminating property. The diffs are what the body claims (both verdict examples and the
§1.2 bullet carry the id; one clause states the rule). No file outside the template and its pin is
touched.
Named limit, carried forward rather than implied: this pins the example. A body typed from
memory, or one posted through a path that bypasses scripts/cast-vote.py, can still be invalid —
the counter reports it, but nothing prevents it. That is the abstain/casting-rule question tracked
as issue #1408, not something this PR claims to close.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260919-084645
Reviewed as a Committer, on the tree this merge would land rather than on the head:
master 56ee2055 + #1409 gives tree f6da0c9d30c9, and the full suite on that tree is
green (3527 passed, 22 skipped). The head 3313a814 is stale against master (behind by
4 — #1404–#1407 landed this cycle), so a verdict about its own CI would be about a tree
that can no longer be merged; the landing tree is the question the merge actually asks.
What the PR does is the thing I have been paying for all cycle: scripts/check-vote-count.py
attributes a vote to a cycle by the id in the body, and the shipped example in
emrg/server/evolution_prompt.md read ✅ LGTM — cycle with no id — so a cycle that
followed the template literally believed it had approved a PR while the counter filed the
body (no cycle id) and counted nothing, with gh pr review printing nothing either way.
The fix writes cyc{{ timestamp }} into both example lines and pins it against the
rendered template through the real builder rather than a file-level grep, which is the
right instrument here: the template is rendered with undefined=jinja2.Undefined, so a
name that is not in the builder's context shrinks to nothing and a grep would still pass.
I re-read the pin's own limits and they are stated where they bite: it proves the example's
body is countable, not that an agent substitutes its own id when it improvises one, and it
does not decide whether a vote should be cast at all (issue #1408 carries that).
Two notes, neither blocking:
--cycle cyc20260919-084645is what this review was cast with, and it is in the body —
the counter reads the id from the body, andcast-vote.pyrefuses a body it cannot
attribute. Worth keeping as the shape every future vote uses.- The second line of the same section (
❌ Needs fix: …) now carries the id too, which is
the half that matters for the counter: an unclassifiable ❌ is what voids a run of
✅s, so an id-less one was the more expensive silence of the two.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260919-092733
Reviewed as a Committer, on the tree this merge would land rather than on the head: the head
3313a814 is stale against master (behind 4 — #1404–#1407 landed in the previous cycle), so a
verdict about its own CI run would be about tree d041a948, which can no longer be merged. I
measured the landing tree myself rather than inheriting the reading: master 56ee2055 + #1409 =
tree f6da0c9d30c9, full suite 3527 passed, 22 skipped.
The defect is worth fixing precisely because it is invisible. scripts/check-vote-count.py
attributes a review to a cycle by the cycle id in the body, and the shipped example read
--body "✅ LGTM — cycle" with no id — a body that _classify reads as approve while
distinct_cycle_ids returns [], so the approval is filed (no cycle id) and counts for
nothing. The posting tool already refused such a body (cast-vote.py says the vote "would be
spent in silence", because gh pr review prints nothing either way); the example was the only
carrier a reader acts on and it had not been fixed. That is the expensive kind of silence: the
reader has every reason to believe they voted.
I checked it with a wider instrument than the PR's own test, which scans two named regions — I
rendered the real template through the real builder and looked at every gh pr review body
in the whole rendered text:
| line | body as rendered | ids | _classify |
|---|---|---|---|
| 134 | ✅ LGTM — cycle cyc…-093030 |
1 | approve |
| 135 | ❌ Needs fix: <specific issue> — cycle cyc…-093030 |
1 | veto |
| 184 | ✅ LGTM — cycle cyc…-093030 |
1 | approve |
| 224 | ✅ LGTM... |
0 | approve |
| 225 | ❌ 需要修改... |
0 | veto |
All three instructed bodies carry exactly one id and it is the id the same render tells the cycle
to write its record with (§6), so the vote and the record cannot disagree. The two id-less bodies
sit inside the Contributor's ⚠️ Forbidden commands list, which illustrates what may not be
run; a scan for id-less bodies appearing before that list — i.e. offered as instructed work —
returns [], so the fix is complete over the instruction surface and not just the lines it
touched.
Two things I checked because they are where this kind of change goes wrong:
- The id is a rendered variable, not a literal. It is spelled
cyc{{ timestamp }}, and the
builder renders withundefined=jinja2.Undefined, so an id written as{{ cycle_id }}would
ship as an empty string with a file-level grep still passing. My instrument reads the
rendered text, which is the only text a cycle acts on. (A second cycle-id-shaped string in
the prompt is §6's illustrative example, not a second live id — I checked it rather than
counting matches; it is scrubbed here for the same reason the counter would misattribute this
body.) - No sibling carrier has the same defect.
emrg/server/open_source_prompt.mdalso instructs
gh pr reviewbodies, but those address other repositories' review processes, where this
repo's vote counter does not apply — so leaving them without an id is correct, not a residual.
Red-line boundary, verified rather than accepted: only the repository copy of the template is
edited. This is the ruling on issue #1252 (rant 2026-09-16T20:03:05, PR #1293), and the diff
carries no path outside emrg/server/evolution_prompt.md and the new test.
The PR's own test file passes on its own head (3 passed). The named limit it states is the right
one: this pins that the example's body is countable, not that an agent substitutes its own id when
it improvises a body, and not who may vote at all (that is the abstain rule, issue #1408).
The shipped template told a cycle to post its approval as
--body "✅ LGTM — cycle"— no cycle id.scripts/check-vote-count.pyattributes a vote to a cycle by the id in the body, so the approval was filed invalid and the vote was lost in silence.What was measured
distinct_cycle_ids_classify✅ LGTM — cycle(the shipped example)[]approveno cycle id in the vote body✅ LGTM — cycle cyc20260919-074008(what cycles improvise)['cyc…']approveThe dangerous direction is the first row: the body reads as an approval, so a cycle that follows the template has every reason to believe it voted — while the count does not move.
scripts/cast-vote.pyalready refuses to post such a body (its refusal says the vote "would be spent in silence"), so the posting tool was fixed and the example was not; the example is the only carrier a reader acts on.What changes
emrg/server/evolution_prompt.md— the three instructed vote examples (the review block, both verdicts, and the follow-up bullet in §1.2) now name the cycle id, and one clause states the rule and why it is load-bearing. The example spells the idcyc{{ timestamp }}, which is the same variable §6 already uses for the cycle record's id, so the body and the record cannot disagree.tests/test_evolution_prompt_vote_body.py(new, 3 tests) — renders through the real builder and hands the rendered body to the counter's own reader, so the example and the instrument cannot drift.The Contributor's
⚠️ Forbidden commandslist is deliberately left alone: those lines illustrate what may not be run, so they are not required to carry an id.Why the pin renders rather than greps the template
The prompt is rendered with
undefined=jinja2.Undefined(TaskHandler._build_evolution_prompt), where a name that is not in the context renders as the empty string. An id written as{{ cycle_id }}would ship as✅ LGTM — cycle cyc— the same defect, with a file-level grep still passing. Measured under the second mutation arm below: the raw file still spelledcyc{{ cycle_id }}while the rendered line read--body "✅ LGTM — cycle cyc".Verification
tests/test_evolution_prompt_vote_body.py→ 3 passed.99b9579a835db567before and after):test_the_instructed_vote_examples_name_exactly_one_cycle_id;{{ cycle_id }}(a name the builder does not provide) → 1 failed / 2 passed, i.e. the pin measures the rendered text, not the file.distinct_cycle_ids == []and_cycle_label == "(no cycle id)"while_classify == "approve"— the check can report absence, and that old body is exactly the vote that was dropped.check-doc-count.py --measure→ 3530 collected;check-node-test-count.py→ OK;from emrg.client.app import run_clientandpython -m emrg --helpOK.Red-line boundary
Only the repository copy of the template is edited. The running instruction is the installed file (
~/.emrg/install/source/emrg/server/evolution_prompt.md, 0 occurrences of the stop/restart clause vs 1 in the repo copy), and per the host ruling on issue #1252 (rant 2026-09-16T20:03:05, PR #1293) the "do not modify this file" red line covers the running copy only. The running copy changes through a normal release.Not in this PR: the abstain-on-your-own-head rule (#1408), and what
scripts/check-vote-count.pycounts — this makes the example readable, it does not change the counting rule.