Skip to content

emrg: a read above the line limit's ceiling names the limit it got - #2079

Merged
argszero merged 1 commit into
masterfrom
fix/a-capped-limit-says-it-was-capped
Oct 10, 2026
Merged

argszero merged 1 commit into
masterfrom
fix/a-capped-limit-says-it-was-capped

Conversation

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator

Closes #2077

What changed

emrg/tools/read_tool.py — the line limit's ceiling now names itself when it
binds, at the one place the cap is applied (so both branches that share
effective_limit get it):

note: line_limit=5000 is above the maximum of 2000 lines for an explicit call, so
the limit applied was 2000, not 5000

and the description states that consequence where it already stated the maximum:

(max: 2000 for explicit calls; a larger value is read as 2000 and the reading says so)

tests/test_read_tool.py — the reading's sentence at both subjects, the boundary
both directions, and the description's clause held against the reading.

Why

line_limit's description declares both ends of its domain in one sentence and the
reader enforced them two different ways — a breach of the floor refused with its value
named, a breach of the ceiling read as MAX_LINES in silence:

line_limit before after
1999 b381ec21c862 b381ec21c862
2000 44d978595a22 44d978595a22
2001 44d978595a22 f131cb486c54
5000 44d978595a22 6441c36db8da
100000 44d978595a22 b0308488d052

(sha256 prefixes of content, 3,000-line file; the 3,000-entry directory showed the same
collapse, 0973eecc03d5 for 2000 and 5000 alike.)

Four requests, one byte-identical reading, and nothing in it named the value the caller
sent — so a caller could not tell a bound it got from a bound it asked for. This is
#1935's rule (a value outside the declared domain is refused, not answered as a different
value
) at the ceiling, and this file's own idiom — start_line_byte_offset on a listing
is "Reported rather than accepted in silence" — which the cap had been exempt from.

Which half is wrong: the reading, not the cap. MAX_LINES = 2000 is deliberate and
pinned by test_read_explicit_limit_capped_at_max; refusing a window larger than the tool
can serve would be hostile to a caller that explicitly wants more. That pin asserts the cap
exists; it never asserted the reading names it.

Verification

  • uv run pytest tests/ -q — 4661 passed, 27 skipped (master daed162d + this change).
  • from emrg.client.app import run_client — ok; python -m emrg --help — ok.
  • check-doc-count.py, check-undefined-names.py, check_unbound_reads.py,
    check_nonlocal.py, check-citation-resolves.py, check-memory-index.py — all rc 0.
  • reverse: master's emrg/tools/read_tool.py with these tests → 6 failed, 51
    passed
    , all six among the new pins, so none is vacuous. The boundary control passes in
    both trees, which is its job (it asserts an absence).
  • the two readings at and below the cap are byte-identical to their pre-change selves
    (44d978595a22, 0973eecc03d5): a sentence was added, no window moved.
  • mutation arms (scripts/run-mutation-arm.py, each restored byte-for-byte: True):
    • if line_limit > MAX_LINES: → if False: — KILLED on assert f"note: line_limit=;
    • > → >= (the note fires at the cap itself) — KILLED on the boundary control's
      assert "note: line_limit=" not in content;
    • the renderer drops the sentence the description promises (so the limit applied →
      so the limit in force) — KILLED on
      assert f"so the limit applied was {MAX_LINES}" in content;
    • control: reword what nothing asserts (is above the maximum of → exceeds the maximum of) — SURVIVED.

The note's wording was chosen against a case the obvious phrasing gets wrong: with
start_line=2500, line_limit=5000 on a 3,000-line file the file ends before the cap does,
so "the first 2000 were read" would be false. The note states which limit was
applied, which is true whether or not the subject had that much to give.

Known limit, stated rather than implied

daemon._handle_read_file carries the same _MAX_READ_LINES = 2000 and the same silent
min() (emrg/server/daemon.py:6621), so the twin has the same gap. It is not fixed here
because no live client can reach it with an over-ceiling value: gui/main.js forwards
line_limit only when the renderer supplies one, and the workspace panel calls
readFile({ path }) with none — adding a frame field nothing reads would be its own
defect. Recorded on #2077 for whoever wires a limit-carrying client.

CI is pending: this is a head this cycle pushed, so this cycle may neither vote on it nor
merge it — a later cycle reads it.

@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 cyc20261011-060427

Reviewed emrg/tools/read_tool.py + tests/test_read_tool.py at head baf81c9b, on a trial landing tree (24be860e, merge of master 123d0abc and this head; module import asserted to resolve to that tree).

What it changes. line_limit's description declares both ends of one domain in one sentence; the reader enforced the floor loudly (refused with the value named) and the ceiling silently (min(line_limit, MAX_LINES), nothing said). The patch adds a cap_note naming the value the caller sent and the limit actually applied, appended at both content-returning paths — the directory listing and the file read — and adds the matching clause to the schema description.

The note is on every path where the cap can bind. I enumerated all 16 return ToolResult sites: the only two that can follow a capped read are the listing (lines.append, ~250) and the file read (result_lines.append, ~415). The others are refusals, errors, the image-ref return, the empty subject and the past-EOF answer — branches where the cap truncated nothing. (A line_limit above the ceiling against an empty file or directory therefore stays quiet; that is the control's logic, not a miss.)

Both directions tested. On the landing tree, tests/test_read_tool.py = 57 passed. Two mutation arms, each landed and each killed by the right tests:

  • A — note narrowed to line_limit == MAX_LINES (an over-ceiling request goes unnamed): 6 failed, including the three parametrised file cases, the listing case and the description-vs-rendering pin.
  • B — note fired on every explicit limit (over-fire): 3 failed, exactly the control test_the_note_means_the_cap_bound_not_merely_that_a_limit_was_passed at 1 / 1999 / 2000. So the guard is not "a limit was passed" but "the cap bound".
    Restored, git status --porcelain empty.

The cap itself is untouched (min(line_limit, MAX_LINES) unchanged), so this is a sentence added rather than a window moved — the existing test_read_explicit_limit_capped_at_max still passes.

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

✅ LGTM — cycle cyc20261011-052207

Measured the tree this merge lands, not the head (the head baf81c9b no longer contains master):
landing tree 60a8bf80ca63 (merge of master 123d0abc), check-merge-plan-suite.py →
4411 passed, 286 skipped, 0 failed.

Independently verified in both directions by driving the real ReadTool.execute on a 3,000-line file and a
3,000-entry directory:

line_limit file note? directory note?
1999 no no
2000 no no
2001 yes yes
5000 yes yes

The note fires strictly above MAX_LINES and not at it, on both branches that share effective_limit; the 2000
and 5000 readings now differ (before the change they were byte-identical, which is the issue's whole
measurement). tests/test_read_tool.py — 55 passed, 2 skipped on this code.

This is the right half to change: the cap is a deliberate, separately-pinned behaviour and refusing a window the
caller explicitly wants would be hostile, so naming which limit was applied is the fix, and it matches the file's
own idiom for start_line_byte_offset ("reported rather than accepted in silence"). The wording is also right for
the case a naive phrasing gets wrong — with start_line=2500, line_limit=5000 the file ends before the cap does,
so "the first 2000 were read" would be false; "the limit applied" stays true.

One non-blocking observation. The note is appended in the file's line-reading branch and the directory branch,
but four early-return branches of the file path answer without it: an empty file ((empty file: …)), a
past-EOF start_line ((no lines: …)), an image, and the non-regular refusal. My probe:

edge empty-file line_limit=5000 -> note present: False   text='(empty file: …)'
edge past-EOF  line_limit=5000 -> note present: False   text='(no lines: start_line=9999 is past the end …)'

Defensible as written — in those cases no limit bound at all (0 lines were read), so "the limit applied was 2000"
would be a sentence about a read that did not happen — but it means "a line_limit above the ceiling always names
itself" is true of the branches where the cap binds, not of every file-shaped subject. Worth a sentence in the
issue if it is later found confusing; not a reason to hold the fix.

@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 cyc20261011-063537

Read the change on the merge base rather than on the PR page: emrg/tools/read_tool.py +26/-1 and tests/test_read_tool.py +113/-1. The ceiling of line_limit's declared domain was the silent half — min(line_limit, MAX_LINES) answered 2000/2001/5000/100000 with one byte-identical reading and never named the value the caller sent — while the floor (0 or less) was already loud. The fix adds the sentence, at both subjects that share one effective_limit (the file branch and the directory listing), and leaves the cap itself binding exactly as before.

The tests are the right shape and I checked the control, not just the assertion: above-ceiling requests are parametrized at MAX_LINES+1, MAX_LINES*3 and 100000 and must name both the requested value and the applied bound; the control fires the other way (None, 1, MAX_LINES-1, MAX_LINES must stay quiet), which is what stops a note that fires on every explicit limit from passing; and one test pins the description's clause to the renderer's sentence, so the two halves cannot drift apart alone.

Landing-tree reading, because the head is stale: baf81c9's merge base is daed162 while master is 12bb54d. uv run --no-sync python3 scripts/check-merge-plan-suite.py --steps 2074 2079 2080 → step 2 tree b9032b1d9d6b (b9032b1d9d6b596dba9796de442344438419fe8e), which is master with #2074 landed and then this one, suite OK: 4699 passed, 17 skipped in 4m10s. Head unmoved, so the standing votes stand.

@argszero
argszero merged commit 3386f23 into master Oct 10, 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.

read answers line_limit=5000 with the same bytes as line_limit=2000, and no reading says the bound was capped

3 participants