fix(stella-tools): cut read_file's payload cap from 76% of the context budget to 12% (#1842) - #1888
Merged
Merged
Conversation
…t budget to 12% `MAX_RENDER_BYTES` was 400 KB — about 114k estimated tokens, or 76% of the whole 150k compaction budget in ONE tool result. That is worse than the number alone suggests, because a single large result does not trigger the compaction that would reclaim it. With the rest of the transcript small, `compact_measured` returns early (under budget) and the retention horizon (`tool_result_horizon_steps: Some(8)`) then keeps the result verbatim for the next eight tool-bearing steps. One read of a lockfile, a schema dump or a bundled JS file cost roughly 900k input tokens. Option (a) of the two the issue offered — the per-result cap, not a retention change — because it is the simpler mechanism and the one that matches how comparable tools bound a read. 64 KB is ~18k tokens, about 12% of the budget: a bound a turn can carry eight times over. The trade-off is real and is stated in the constant's doc comment rather than buried: a full 2000-line read of ordinary source (~45 bytes a line, ~90 KB) now stops around line 1400 and the model pages once. That cost IS the fix. `read_file` already supports `offset`/`limit`, and moving the number is one edit if a maintainer wants a different point on the curve. The second half is what makes paging cheap. The footer said "re-read with offset/limit to continue" and never said FROM WHERE — and the answer is not the line count the model can see, because `start` may be non-zero and clipped lines still count as shown. It now names the line: "continue with offset=N". Witness: `the_total_payload_cap_stops_the_render_and_reports_it` gains the paging assertions — the footer names a resume offset, that line is NOT already in the render (an off-by-one silently skips or repeats a line and the model cannot tell), and re-reading at it begins exactly there. Its existing cap assertion also stops hard-coding "400 KB" and derives the number from the constant, since an assertion that would go on passing for a cap that moved is the shape a size guard can least afford. `cargo test -p stella-tools` — 737 passed, 0 failed. Clippy and fmt clean. Closes #1842
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Reviewer's GuideReduces read_file’s maximum rendered payload size and makes paged reads precise and test-backed so large file reads don’t monopolize the compaction budget and the footer provides an exact resume offset. Sequence diagram for read_file paged reads with precise resume offsetsequenceDiagram
actor Model
participant ReadFile
Model->>ReadFile: execute({path, offset=0}, workdir)
ReadFile-->>Model: ToolOutput::Ok(content with footer, stopped at 64 KB payload cap, continue with offset=start+shown+1)
Model->>ReadFile: execute({path, offset=start+shown+1}, workdir)
ReadFile-->>Model: ToolOutput::Ok(content starting at resume line)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
macanderson
added a commit
that referenced
this pull request
Aug 6, 2026
…put instead of dropping the tail (#1889) (#1900) ## Problem `bash.rs` and `custom.rs` capped tool output at `MAX_OUTPUT_BYTES = 100 KB` — ~28k estimated tokens, **~19% of the 150k compaction budget in one result**. A single large result does not trigger the compaction that would reclaim it (`compact_measured` returns early when the rest of the transcript is small), and the 8-step retention horizon then keeps it verbatim: one `cargo test --workspace` or `npm ci` printing 100 KB cost ~224k input tokens, not 28k (#1889, the deliberate residue of #1842). On top of the budget shape, the crate had grown **three independent elision spellings**: `exec::truncate_middle` (50/50 split, one marker format), `bash.rs`'s inline copy (50/50, a second marker format), and `custom.rs::truncate_middle_out` (40/60, the second marker format again). Three copies is the shape that lets one drift — and they already had, on both the split and the marker. ## Decision **Head + tail elision through one shared helper, and a 64 KB cap.** - **One spelling:** `exec::truncate_middle_capped(s, max_bytes)` is now the crate's single model-facing elision primitive. `exec::truncate_middle`, `bash`, and `custom` all cut through it; `custom.rs`'s private copy (and its boundary helpers) are deleted. It sits in `exec.rs` because that module already owns the output-cap policy for every other runner (`truncate_middle`, `CappedStream`, `truncate_preview`), and `exec.rs` is not a god file. - **Split: 40% head / 60% tail.** Tail-biased because a failing command's densest signal is at the end — the final test summary, the last error, the exit status. This is lesson L-S3, already ratified in `custom.rs`; `bash` moves from 50/50 to match rather than the reverse, and the shared function makes future divergence structurally impossible. Both cuts land on UTF-8 char boundaries (the existing discipline of `truncate_preview` / the old inline code). - **Marker names the elided byte count *and* the cap:** `[… N bytes truncated: output exceeded the 65536-byte cap; the head and tail are kept …]` — the model can account for every byte and knows the bound it is working under, mirroring `CappedStream`'s "say which cap did this" convention. - **Cap: 100 KB → 64 KB (~12% of the compaction budget)** — the exact point #1842 ratified for `read_file` (400 KB → 64 KB, PR #1888), for the same multiplier argument. It stays the same order of magnitude and preserves #616's ratio argument: still 2.2x `exec::MAX_OUTPUT_BYTES` (30k), so the shell remains the agent's wide sensory channel while `read_output` pages stay cheap. `custom.rs` now **aliases** `bash`'s constant (`pub(crate) use`) instead of carrying a copy, so the two cannot drift. - **Retention-aging is deliberately not here.** The issue's option (b) — aging an oversized result out of retention early — belongs to the engine's retention fold, tracked by #1819 (reclaimable-bytes gate) and the #1438 file-budget umbrella. This PR bounds the cost at the source; those bound what survives the horizon. Exemplar for the consolidation shape: the crate's own `shell_quote`, which collapsed five drifting copies into one `exec.rs` primitive with a "a new one must be too" contract; this PR does the same for elision. ## Witness `over_cap_output_keeps_first_and_last_lines_with_a_named_elision`, once per surface (`bash::tests`, `custom::tests`): a command/script emitting a first sentinel line, `MAX_OUTPUT_BYTES` of filler, and a last sentinel line yields a result containing **both sentinel lines**, a marker naming the **elided byte count** and the **cap**, bounded by the cap plus the marker. Every size is derived from the constant — nothing hard-codes a human-readable size, the assertion shape #1842 caught going stale. Verified failing on the old code the artisanal way — both tests spliced onto the parent commit: ``` test bash::tests::over_cap_output_keeps_first_and_last_lines_with_a_named_elision ... FAILED test custom::tests::over_cap_output_keeps_first_and_last_lines_with_a_named_elision ... FAILED panicked: 'the marker names the cap it enforced' ``` …and passing on this branch. `exec::tests` additionally pin the helper itself: the marker's elided count is arithmetically exact (derived, not hard-coded), tail budget ≥ head budget (L-S3), both cuts survive landing mid-multibyte-char, and at-or-below-cap input is byte-identical. ## Verification - `cargo test -p stella-tools`: **723 passed** (lib) + all integration suites green, 0 failed. - `cargo clippy -p stella-tools --all-targets -- -D warnings` clean; `cargo fmt -p stella-tools --check` clean. - Workspace-wide validation left to CI per build economy (no public API changed; every touched item is `pub(crate)`). Closes #1889 Refs #1842 #1819 #1438 ## Summary by Sourcery Unify and tighten stdout/stderr truncation across exec, bash, and custom tools to keep both the head and tail of oversized outputs under a smaller shared cap. Bug Fixes: - Ensure over-cap bash and custom tool output retains both the first and last lines instead of dropping the tail under some sizes. Enhancements: - Introduce a shared truncate_middle_capped helper in exec that performs UTF-8-safe, tail-biased head+tail elision with an explicit marker naming the elided byte count and cap. - Reduce the bash/custom output cap from 100 KB to 64 KB to better bound transcript and compaction costs, while preserving the intended ratio to exec output limits. - Alias custom tool output caps and elision behavior to bash so their output budgets and truncation semantics cannot drift apart. Tests: - Add unit tests for truncate_middle_capped to validate exact elided byte accounting, UTF-8 boundary safety, and no-op behavior at or below the cap. - Add integration tests for bash and custom tools that verify over-cap outputs keep both sentinel lines, include a cap- and byte-count-bearing elision marker, and remain within the bounded size. Co-authored-by: Stella Test <test@stella.local>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
MAX_RENDER_BYTESwas 400 KB — about 114k estimated tokens, or 76% of the whole 150k compaction budget in one tool result.That is worse than the number alone suggests, because a single large result does not trigger the compaction that would reclaim it. With the rest of the transcript small,
compact_measuredreturns early (under budget), and the retention horizon (tool_result_horizon_steps: Some(8)) then keeps the result verbatim for the next eight tool-bearing steps. One read of a lockfile, a schema dump or a bundled JS file cost roughly 900k input tokens.Change
Option (a) of the two the issue offered — the per-result cap, not a retention change — because it is the simpler mechanism and matches how comparable tools bound a read.
MAX_RENDER_BYTES12% is a bound a turn can carry eight times over — which is exactly the horizon that used to multiply it.
The trade-off, stated rather than buried
A full 2000-line read of ordinary source (~45 bytes a line, ~90 KB) now stops around line 1400 and the model pages once. That cost is the fix, not a side effect. It lives in the constant's doc comment so the next reader sees the reasoning and not just the number, and moving it is one edit if you want a different point on the curve.
The half that makes paging cheap
The footer said:
…and never said from where. The answer is not the line count the model can see, because
startmay be non-zero and clipped lines still count as shown. So the note cost a guess, and a wrong guess costs another read. It now names the line:Witness
the_total_payload_cap_stops_the_render_and_reports_itgains the paging assertions the issue asks for:execute.Its existing cap assertion also stops hard-coding
"400 KB"and derives the number from the constant. That assertion would have gone on passing for a cap that moved — the shape a size guard can least afford, and the reason this PR had to touch it at all.cargo clippy -p stella-tools --all-targets -- -D warningsandcargo fmt --checkclean.Not in scope
bash.rsandcustom.rscarry the same shape atMAX_OUTPUT_BYTES = 100 KB(~28k tokens). Smaller magnitude, same argument — left for a follow-up rather than folded in, since each deserves its own trade-off discussion and its own witness.Closes #1842
Summary by Sourcery
Reduce the read_file tool’s maximum rendered payload size to limit per-call context usage and improve paging behavior for large files.
New Features:
Enhancements:
Tests: