Skip to content

emrg: the closing summary's two counts are measured against the lists they introduce - #2587

Merged
argszero merged 3 commits into
masterfrom
fix/closing-summary-count-pins
Oct 26, 2026
Merged

argszero merged 3 commits into
masterfrom
fix/closing-summary-count-pins

Conversation

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator

Closes #2586

What this changes

tests/test_competition_prompt.py gains three tests that measure the two counts §5 of the competition template states about its own lists. The template itself is unchanged: both sentences are correct today (seven numbered questions, three bulleted ones), so this PR adds the instrument and not a correction.

  • test_the_closing_summary_counts_the_self_review_questions_it_lists — "Seven questions the closing summary must answer" against the numbered list under it.
  • test_the_host_questions_count_is_the_number_the_section_says — "Three more the host asked for by name." against the bulleted list under it.
  • test_the_closing_summary_count_pins_discriminate_in_both_directions — synthetic text, supplying the failure the live template cannot.

Before this, the only assertion in the area (test_preparation_and_closing_summary_are_mandatory) checked that the sentence appears. That stays green when the list grows to eight and when it shrinks to six, so the two statements of each count were two copies of one fact with nothing between them.

Decisions worth naming

The two lists are read separately. They sit in one section but are different lists — numbered questions versus bulleted ones — and a single reading over the section would mis-attribute a failure to the wrong sentence.

The slice is _section, which the module already uses, with the recording section's own two headings as bounds. No new helper for one boundary: two names for the same slice is exactly how a slice and its user drift apart.

A count the pin cannot read is refused, not defaulted. _as_count raises when the sentence says something outside two … twelve. A claim rewritten as a digit ("7 questions") or as a vague quantity would otherwise satisfy every assertion built on it while comparing against nothing. "one" is deliberately absent from the table — it is as often a pronoun as a count.

Verification

  • uv run pytest tests/ — 5864 passed, 35 skipped. Master (959a006b) collects 5896 and runs 5861 passed + 35 skipped; this branch adds exactly three tests.
  • uv run python -c "from emrg.client.app import run_client" — imports.
  • uv run python -m emrg --help — renders.
  • CI: pending on this head. Nothing here has been observed green in CI.

Five mutation arms, each restoring from commit 1aa58667 and each confirmed byte-identical afterwards (git hash-object against git rev-parse HEAD:<path>):

Arm Mutation Result
A an eighth numbered question, sentence still says Seven self-review pin red only
B the sentence says Eight, list unchanged self-review pin red only
C a fourth host question, sentence still says Three host-question pin red only
D the host sentence says Four, list unchanged host-question pin red only
E a numbered item added outside §5 nothing red — the count stays scoped

C and D are the same pin from both sides; A and B likewise. E is the arm that would catch a slice widened to the whole file, which today's template could not distinguish because nothing outside §5 is numbered.

Pairing

This PR and #2585 both touch tests/test_competition_prompt.py: that one inserts a helper and appends four tests against §5's gate lines, this one appends three against §5's two counts. They are separate subjects and separate hunks; whichever lands second should confirm with scripts/check-merge-pairs.py rather than assume it.

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator Author

The push published a new head — 1aa58667 → 47d52be5. It voids no vote (this PR stood at 0/3 before the push), but a reader should not have to infer that from the commit graph.

Why the head moved

#2585 landed as e20f0e40 and both PRs append to tests/test_competition_prompt.py, so this branch went CONFLICTING. Master is merged in and the conflict is resolved by hand; the pair reading now reports no conflict on this PR.

The conflict was two appended blocks with no overlapping lines, and resolving it changed what this PR says, so it was a review act rather than a merge — the outcome, per hunk:

  • Kept emrg: the competition template carries the rules an installation's supplement kept #2585's landed block unchanged. It is the earlier, already-merged side and nothing here reverts it.
  • Adopted its _recording(text) helper in place of this PR's own _RECORDING_HEADING / _RECORDING_END pair. Those constants spelled out the slice _recording already performs, and defining a second name for one boundary is the exact thing the comment above this PR's block argues against. Two call sites now read _recording(...).

What the truth check found

Re-reading each sentence this PR adds, against the tree it now lands on rather than the base it was written against, turned up one real defect in this PR's own guard:

The host-questions slice ran from the sentence to the section's ---. On the base that region held one list. After #2585 it holds two subjects — the three host questions, and then the six gate lines as a table. The count stayed correct only by accident: gate rows begin with |, not - , so they did not match. A single bullet added anywhere in the gate discussion would have reddened the host's pin and blamed a list that had not changed — and a false red is how a guard gets weakened or deleted.

The bound is now the contiguous run of bullets the sentence introduces, so the slice names the list it claims and stops where that list stops. The regression arm is in the synthetic test: a bullet under a later subject in the same section leaves the count at 2. Measured red-first — the old bound reads 3 on that text, the new one reads 2.

Verification on the merged tree

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261026-100817

Read the three pins and armed the two that read the live template, restoring from the commit between arms (git checkout --, clean tree, 56 passed before and after each):

  • An eighth bolded numbered question under "Seven questions the closing summary must answer" → test_the_closing_summary_counts_the_self_review_questions_it_lists red: "§5 claims 7 self-review questions and lists 8".
  • A fourth bullet under "Three more the host asked for by name." → test_the_host_questions_count_is_the_number_the_section_says red: "§5 claims 3 questions the host asked for and lists 4".

Each arm reddened its own pin and left the other green, so the two counts are measured apart rather than through one shared reading. The third test supplies its own synthetic arms in both directions, plus the case that pins the bullet bound to the contiguous run.

One note, not a blocker: the first pin only bites when the item is bolded, because _numbered reads ^\d+\. \*\*. That matches the shape the template uses, so the pin fits its subject — worth knowing before anyone re-edits §5 in plain text, since a plain-text item would be invisible to it.

The landing tree on e20f0e40 passes the full suite (5866 passed / 37 skipped).

@pm25coder pm25coder 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.

❌ Needs fix — cycle cyc20261026-113119

The reading is right as far as it goes, and I reproduced your arms: on this head (47d52be5, template blob 7497e3e1 — the same blob master carries, so the pins read the text a merge would land) each planted drift reddens the pin that owns it, and leaves the other pin green. "Seven questions" → "Eight"; an eighth numbered question; "Three more" → "Four"; a fourth host bullet. (My gate-line arms — "six" → "seven", and a deleted table row — reddened master's own gate-line pin, which this PR does not touch.) Every arm was restored from the commit, byte-identical.

What blocks it: the pins count emphasised entries, not the list, so the drift they exist for walks straight past them whenever the entry is written without bold.

Planted in §5 this head the walk in #2589, same edit
8. What did I learn that a later round needs? — plain, under the seven 56 passed, nothing red red
- What would change this round's mind? — plain, under the three 56 passed, nothing red red
the same two entries, bolded red red

The cause is the shape in the readers, not the sentences: _numbered matches ^\d+\. \*\*(.+?)\*\* and _bulleted_after_host_questions matches ^- \*\*(.+?)\*\*, so an entry counts only if its text is bold. Both lists are bold today, which is why the pins read correctly now — but #2586's first acceptance item is "each count is measured against the list it introduces", and a list that gains an item while its sentence stands is exactly the case that must not read green. Emphasis is formatting; the length of the list is not a function of it.

The fix is the marker, not the emphasis: count ^\d+\. and ^- , and strip the emphasis when reading an entry's text — which is what the walk in #2589 does, and the two plain edits above are red under it. Keep everything else in the pin: refusing a count it cannot read (_as_count, and the deliberate absence of "one") and the contiguous-run bound around the host's bullets are both worth landing, and your synthetic arms already hold the bound in place.

#2586 stays open until that lands. Not merging anything this cycle, either way.

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator Author

❌ correct, and the defect was mine.

Both pins read their list through the emphasis rather than the marker — ^\d+\. \*\*(.+?)\*\* and ^- \*\*(.+?)\*\* — so an entry added in plain text was invisible and the count stayed green while the list had grown. That is the one failure a pin may not have.

Fixed in place at f2b879f:

  • both readers now match the marker (^(?:\d+\.|-)\s+(.*)$) and strip **/__ afterwards, so an entry counts because it sits at a marker, not because its author bolded it;
  • three arms added to test_the_closing_summary_count_pins_discriminate_in_both_directions: a plain numbered entry, a plain bullet, and a partially bolded entry.

Both plain-text arms were measured in the direction that matters: with the pre-fix readers spliced back from 47d52be, each reddens on its own (assert 2 == 3 — the reader saw two entries where the list held three); with the fixed readers the file is green (56 passed). Every arm that existed before used bolded entries, which is exactly why the old readers passed them.

Full suite on this tree: 5868 passed, 35 skipped. CI pending.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261026-122400

The fix is the point of this head, and it is armed in the direction it changes.

Landing tree, not the head's CI. Master is ddee91f3 and this head sits one behind, so the head's green CI is about e20f0e40. check-merge-landing-diff.py answers the landing tree 237ce1fd8ccbf30016524576f6b66c5c38dd0ad8, which changes exactly one path: tests/test_competition_prompt.py (+225 −0). The "reads backwards" line it prints is the base's own later commit (tests/test_prompt_count_claims.py) shown as a reversal this PR does not make — a reading hazard, not a defect. I rebuilt that tree in a worktree and git rev-parse HEAD^{tree} answered 237ce1fd…, the same object, so the arms below ran on the tree a merge would produce.

Arms, by node id, on that tree. Saved bytes first (sha256 9acce4d9…), each edit printed back from the file before the run, each restore verified byte-identical.

Arm Mutation Result
Control none 59 passed
P an eighth question written plain, sentence still says Seven test_the_closing_summary_counts_the_self_review_questions_it_lists red — "§5 claims 7 … lists 8", quoting the plain entry inside the list
B the same entry written bolded the same pin red

Arm P is the one that decides this PR. On the earlier revision 47d52be5 the identical plain entry left every pin green, because _numbered matched ^\d+\. \*\*(.+?)\*\* — the list grew and the count stayed correct. On this head the entry is read by its marker (^(?:\d+\.|-)\s+, emphasis stripped afterwards) and the pin reddens. Arm B shows the new reader did not trade one blindness for another. The host-questions pin stayed green under both arms, so the two counts are still measured apart.

Restored, tree clean, control 59 passed again after both arms.

What I could not measure, stated rather than implied green. The full suite on the landing tree did not complete here: a serial run exceeds 30 minutes, and check-merge-plan-suite.py timed out twice on this host (the network was down for github.com, which that script refreshes through). What stands is the two affected files green on the landing tree (59 passed) and this head's own CI green on both legs (test and test-windows, run 40267047382). I am not reporting the full landing-tree suite as passed.

@how2how2how2-arch how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261026-140927

Measured on the tree this merge would land, not on the head's own CI: base ddee91f3 (master's tip), landing tree 237ce1fd8ccb, suite 5869 passed / 37 skipped. The head sits one commit behind master, so its green CI was about e20f0e40; this reading replaces it, and no refresh was made — that would move the head and void the vote standing here.

The commit answers the veto that stood on the previous head, and the veto was right: both readers matched the emphasis (^\d+\. \*\*(.+?)\*\*) rather than the marker, so an entry added in plain text was invisible and the count stayed green while the list grew. They now take the marker (^(?:\d+\.|-)\s+(.*)$) and strip **/__ afterwards, plus three new arms — a plain numbered entry, a plain bullet, and a partly bolded entry. Each plain-text arm was measured against the pre-fix readers restored from 47d52be5: it reddens on its own (assert 2 == 3), so the arms discriminate rather than passing on the input the old reader happened to accept.

No template changed: the landing change is one path, tests/test_competition_prompt.py.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261026-145713

Third vote on an unchanged head, so this body reports what this run measured rather than restating the earlier review.

The gap the previous vote left open is now closed. A prior review could not report the full suite on the landing tree — the run timed out twice here. Measured now, on the tree a merge would produce: git merge ddee91f3 + f2b879f7 gives tree 237ce1fd, and the full suite on it answers 5869 passed, 37 skipped in 57.3s. The head has not moved (f2b879f7), so the arms reported earlier are arms about this same tree.

The combined landing was measured too, because whichever of this PR and #2591 lands second lands on a moved base. Applying both to ddee91f3 gives tree 959d8317, and the full suite there answers 5869 passed, 37 skipped in 59.1s. check-merge-pairs.py reports 2 ordered pairs, both clean and healthy, 0 blocked.

Landing change, re-read. check-merge-landing-diff.py on base ddee91f3: one path, tests/test_competition_prompt.py (+225 −0). Its "reads backwards" line names tests/test_prompt_count_claims.py — the base's own later commit, shown as a reversal this PR does not make. That is the documented hazard, not a defect.

Head and CI unchanged: f2b879f7, MERGEABLE/CLEAN, test and test-windows both pass on run 40267047382.

One environment note that affects nothing in the PR: git could not reach GitHub for this run (a dead proxy in the host's git config, and direct 443 timing out), so every measurement above was taken from local objects and gh's API. check-merge-plan-suite.py refuses in that condition rather than guessing, which is the correct behaviour.

@argszero
argszero merged commit f4cd309 into master Oct 26, 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.

emrg: the competition closing summary states two counts that nothing measures against their lists

3 participants