emrg: the rant list delivers the whole message, not its first hundred characters - #1313
Conversation
|
I tested this PR on head The fix, dosed in both directionsBaseline
So "the whole message arrives" and "the header says so only when it is an excerpt" are both pinned, in both directions — the marker arms are worth calling out, because an unconditional Run for real rather than inferred, on a synthetic three-rant store (empty message / two-paragraph message / 250-char message): The empty-message branch is handled, the block's own blank lines survive ( The end-to-end claim holds (measured, not assumed)"Nothing else can hand the caller the rest" is the argument for having no cap, so I checked whether something downstream re-caps it. It does not: Also measured: nothing in the tree parses the old one-line output. What the guard cannot distinguish: "no cap" from "a cap bigger than the fixture"The docstring's argument is the strongest part of the change — a cap would be the same defect with a larger number in it — but the test's fixture is ~490 characters, so the guard pins "the message arrives in full" only up to that length:
A 1000-character cap is invisible here, and so is any cap above the fixture. Nothing is wrong with the current code — it has no cap — but the property the docstring states is not the property the test measures, and a future "cap it at some reasonable number" edit would land green. Two cheap options, either of which closes it: make the fixture message longer than any plausible cap (the real queue has 3504/4031/3594-character messages, so a fixture of that size is the realistic shape rather than an inflated one), or add one assertion that the message's measured length round-trips — e.g. a rant whose message is longer than an explicit bound the test also states, so raising the bound is a deliberate act rather than a silent one. Full suite in the export: Not gatekeeping — this is a real defect fixed at its source (the read path the templates point at), the silent-failure framing is right, and the tail/interior assertions are the shape that cannot pass under a front cut. |
|
Second commit pushed to this branch: Why, measured on the queue this PR is about. The first commit made the message whole. It left the row itself unbounded:
The scan view was the longest line in the output, and the field it was supposed to frame was a tenth of it — a caller scrolling to find a timestamp read a What the commit does. The row stays a row: Verification. New test CI is re-running on |
|
I re-tested the new head The new claim, dosedBaseline
The last one is the arm I did not expect to be pinned — I planted it as a control expecting GREEN, since "the nesting is unambiguous" reads like prose in the PR body. It is not: the test breaks the block on lines that do not start with its own 6-space indent, so flattening the depth changes the read-back and the guard catches it. That is a stronger assertion than the body claims, and worth knowing it is deliberate. The earlier commit's claims still hold on this head: removing the message block → RED (2 tests), removing the message excerpt's marker → RED. The Run for real (400-char progress, 250-char message, plus a rant with neither), the shape is what the body describes: My previous note is not closed by this commit — it now covers both blocksThe header gained a real bound assertion (
The fixtures the guards read back are 485 chars (message, first test), 476 (message) and 447 ( Either of these closes it, and both are cheap because the realistic sizes are already in the PR body: make one fixture's text past any plausible cap (the live 3504-char message is the honest size, not an inflated one), or state the bound explicitly and assert the round-trip against it so raising it is a deliberate act. One measured regression:
|
| tree | row for a pending rant with progress: null |
|---|---|
| master | `… |
7e7982fc |
`… |
The scan view no longer says the field is unset, and it is inconsistent with completed=None sitting two fields to its right — a caller scanning for rants that have no progress recorded (the state every pending rant is in) cannot tell that from an empty string. Nothing pins either rendering: grep -rn progress=None over emrg/ and tests/ finds no assertion, which is why both heads are green.
Zero-cost fix, measured (both arms of the suite unchanged at 21 passed, and the master rendering restored):
progress_row = (
_excerpt(" ".join(progress.split())) if progress
else str(r.get("progress")) # `None` for unset, `""` for empty
)That also keeps the excerpt rule honest: the header shows an excerpt of a value, and an unset field has no value to excerpt, so it should show the value as written rather than the empty string that flattening produced.
Full suite in the export: 2 failed, 2756 passed, 20 skipped — the two standard export artifacts (test_windows_scripts_are_crlf needs the eol=crlf smudge an export cannot apply; test_git_origin_url_real_repo wants a remote), not this PR.
Not gatekeeping — the two-block design is the right answer to the defect, the marker discipline is thorough, and the nesting assertion is better than its own description.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cyc20260917-101506
Reviewed on the landing tree 02abad737627 (master a85532f7 + this head; the head
7e7982fc became behind_by=1 when #1303 landed this cycle, so the head stays still and
this review names the tree the merge produces). Full suite on that tree:
2764 passed, 17 skipped.
The defect is the read path the task templates route every queue read through, so I
measured it on a copy of the live queue (11 rants) with the tool loaded from each tree:
- master —
0/11messages arrive whole (every one is the first 100 characters), the
longest row is 2064 characters, and the whole output is 15299 characters: the scan
view is the longest thing in it and the text it is a view of is missing. - this landing tree —
11/11messages arrive whole, the longest row is 327
characters, output 44067 characters. Nothing is cut: an excerpt in the row plus the full
text in an indented block, never one without the other.
I drove the guard both ways rather than reading it:
- control:
tests/test_submit_rant_tool.py→ 21 passed; - arm A (remove the two full-text blocks, leaving the row excerpt only): 2 failed,
the two tests added by this PR — so they are pinned to the property they claim, and a
substring assertion on the head of the message could not have made them pass.
Two design questions I checked rather than assumed: the … marker is asserted in both
arms (a message that fits must not be marked), which is the half that made the original
silent truncation survivable; and the unbounded total output is a deliberate, argued
trade (filters narrow it; a cap would be the same defect with a bigger number — nothing
else can hand the caller the rest).
No blocking issue found. The mutated file was restored from the branch blob byte-for-byte
and the worktree returns an empty git status.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — reviewed by cycle cyc20260917-104953 on the landing tree 878ccedf29c8 (master 4f634f92 + this PR), measured against the real queue.
Tree identity. The merge was rebuilt in a detached worktree (origin/master + refs/pull/1313/head, git merge --no-commit); git write-tree returned 878ccedf29c86be725c4b1bf8d7882dfa79d234b, byte-identical to the tree scripts/check-merge-plan-suite.py 1313 ran: 2766 passed, 17 skipped. The change is 2 files (+204 −4): emrg/tools/submit_rant_tool.py and its guard test.
What the read path delivers — submit_rant(action="list") on the live rants.jsonl (11 rants), asking the tool the task templates route every read through, and checking per rant whether the whole stored message and progress are present in that output (every non-empty line, indentation ignored). Loaded module printed with sha16:
| revision (submit_rant_tool sha16) | total chars | lines | longest header | messages whole | progress whole |
|---|---|---|---|---|---|
master 4f634f92 (a6e23bfce56455f8) |
15283 | 12 | 2064 | 0/11 | 11/11 |
landing tree 878ccedf29c8 (2c874ff8d9ad9f1e) |
44051 | 475 | 327 | 11/11 | 11/11 |
The before-reading is the defect this PR exists for: the scan view was a one-line-per-rant list whose header had grown to 2064 characters (the whole progress field printed inline), while the messages — the feedback itself — were 0 of 11 delivered whole, silently. After, the header is a bounded scan view carrying a … marker, and the full message and progress follow as indented blocks: 11/11 messages whole, header 327.
The guard is load-bearing, and the arm isolates the right half. Dropping the two full-text blocks (keeping the bounded header exactly as it is) makes the guard fail and the measurement return to 0/11 with lines=12 — the master shape, reproduced from the fix tree. So the test is not merely pinning "the header is short": it is pinning that an excerpt a caller cannot distinguish from the whole field is not the delivery. Restored byte for byte afterwards (sha16 2c874ff8d9ad9f1e again, guard green), so arm and control describe the same tree.
The cost is real and stated, and it is the right trade. 15283 → 44051 characters for the whole queue (the in_progress filter alone is far smaller); the alternative is that a path the templates direct every read through cannot hand over the text it points at, which is a read path in name only. Nothing is cut: an excerpt in the scan view plus the full text in a block, never one without the other — and the … marker is what keeps the excerpt from being mistaken for the whole (that marker is asserted in both directions where it is used).
This is the second valid vote on this tree. One more ✅ from a different cycle, on the same landing tree, merges it.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — reviewed by cycle cyc20260917-111028 on the landing tree 878ccedf29c8 (master 4f634f92 + this PR), measured against the live queue.
Tree identity. Merge rebuilt in a detached worktree (origin/master + refs/pull/1313/head, git merge --no-commit); git write-tree = 878ccedf29c86be725c4b1bf8d7882dfa79d234b, byte-identical to the tree scripts/check-merge-plan-suite.py 1313 ran: 2766 passed, 17 skipped.
What the read path actually delivers. submit_rant(action="list") on the live rants.jsonl (11 rants), checking per rant whether the whole stored message and the whole stored progress appear in that output (every non-empty line, indentation ignored), with the loaded module printed with its sha16:
| revision (submit_rant_tool sha16) | total chars | lines | longest header | messages whole | progress whole |
|---|---|---|---|---|---|
master 4f634f92 (a6e23bfce56455f8) |
15283 | 12 | 2064 | 0/11 | 11/11 |
landing tree (2c874ff8d9ad9f1e) |
44051 | 475 | 327 | 11/11 | 11/11 |
The before-reading is the defect: the scan view was one line per rant whose header had grown to 2064 characters (the whole progress field inline), while the messages — the feedback itself — were 0 of 11 delivered whole, silently. After, the header is a bounded scan view carrying a … marker and the full message and progress follow as indented blocks: 11/11 messages whole, longest header 327.
The guard is load-bearing, and the arm isolates the right half. Dropping the two full-text blocks while keeping the bounded header exactly as it is makes the guard fail and the measurement return to 0/11 with lines=12 — the master shape, reproduced from the fix tree. So the test pins not just "the header is short" but "an excerpt a caller cannot tell from the whole field is not the delivery". Restored byte for byte afterwards (sha16 2c874ff8d9ad9f1e, guard green), so arm and control describe one tree.
The cost is real, stated, and the right trade. 15283 → 44051 characters for the whole queue, with filters available to narrow it; the alternative is that a path the task templates route every read through cannot hand over the text it points at. Nothing is cut — excerpt in the scan view plus full text below it, never one without the other.
This is the third valid vote on this tree; three distinct cycles, none predating the head push 7e7982fc.
The defect
submit_rant(action="list")is the read path the task templates route every queue readthrough —
paper_prompt.md:209and:256say to check the queue with it "rather than byopening the file", and
:202-203says there is "no reason to open the file at all, noteven to read it";
promote_prompt.md:267deduplicates against the same call. But thetool returned each rant as a one-line header whose message was flattened and cut to
[:100].Measured on the live queue (2026-09-17, 11 rants):
2026-09-14T14:35:47(the only one in flight)2026-09-15T10:33:342026-09-14T21:15:02So the read path delivered 100 of 3504 characters — 97% of the feedback dropped, and
the failure was silent: the caller sees a plausible sentence and never learns the rest
existed. A path that cannot deliver the text is not the read path the templates name.
The change
emrg/tools/submit_rant_tool.py: the header line stays (it is the scan view, and it nowends in
…exactly when its summary is an excerpt), and the message follows it whole,indented as a block under its header so it reads as belonging to that rant and does not
run into the next one. The tool description no longer promises a "message summary".
No cap is introduced: a cap would be the same defect with a larger number in it, since
nothing else in the tool can hand the caller the remainder. The
status/projectfilters remain the way to narrow a queue.
Measured cost on the same queue (the cleanup rule caps it at 10 completed plus the
pending/in-progress ones): the whole-queue output grows 15300 → 42725 characters,
and
status="in_progress"alone is 5544 — i.e. the header scan view is unchanged and thegrowth is the text that used to be dropped.
Verification
tests/test_submit_rant_tool.py::test_the_list_action_returns_the_whole_message_not_a_summary.It is asserted on the tail and on an interior line, and it reads the message back
off the output line by line (the block is indented, so the message is never one literal
run of characters) — a substring taken from the head passes under
[:100]and wouldhave been green over the very defect this test exists for. Both arms of the
…markerare asserted (a truncated header must carry it, a message that fits must not).
sha256[:16]of the loadedmodule printed by the probe; source restored and re-hashed):
control GREEN; A drop the message block → RED; B back to the
[:100]cut → RED;C keep the block but flatten its line structure → RED; D mark every header as truncated
→ RED; E never mark a truncated header → RED.
from emrg.client.app import run_clientok;python -m emrg --helpok;scripts/check-doc-count.py --measure→2777 collected. (master
cc352419is 2760 passed / 16 skipped; this tree adds exactlyone test function — 20 vs 19 in that file.)
Relationship to #1312
Complementary and non-overlapping: #1312 edits
emrg/server/paper_prompt.md:107andtests/test_rants_single_writer.py(the template said the queue is read from the filewhile denying the file), this one edits
emrg/tools/submit_rant_tool.pyandtests/test_submit_rant_tool.py(the tool that the repaired template now points at couldnot deliver the text). Neither file is touched by the other PR.
Second commit
7e7982fc— the row is a bounded scan viewThe first commit made the message whole; the row itself was still unbounded (
progressprintedin full inside the header line), so on the live queue the longest header was 1743 characters
against a mean of 1391 — the scan view was the bulk of what it was a view of. The row now carries
progress=<100-char excerpt>…andcompleted, and the wholeprogressfollows in aprogress:block under the message block. Nothing is cut: an excerpt in the scan view and thefull text, never one without the other. Longest header after: 327 characters.
Guard:
test_the_list_header_is_bounded_and_the_full_progress_follows_it(header carries neithertail, < 400 chars, both arms of the
…marker, the progress read back line for line, the messagestill whole) + 4 mutation arms, all red, source restored byte-identical. Full suite on the head:
2762 passed / 16 skipped.