Repository navigation
Conversation
`rtk read` defaults to `--level none`, and that default is right: filtering
source code hides the code the reader came for. But "do not filter" was also
being read as "do not bound", and those are different promises. The hook
rewrites every `cat` into this command, so a pathological file goes straight
into the model's context in full.
Measured on a 15.8 MB rust-analyzer stats log of 29 lines, one of them 7.8
million characters: `rtk read` returned all 16 571 395 bytes.
Two ceilings, both in LimitsConfig so they can be tuned or turned off:
read_max_line_chars = 2000 a line this long is generated, minified or a
packed log record, never something a reader
follows to the end
read_max_total_chars = 400000 set well past any hand-written file, so
ordinary reading is untouched
Content is never reinterpreted, only bounded, and each cut says what it
dropped and how to get it: the file is still on disk, the total-ceiling hint
names `rtk read <file> --tail-lines N`, and the line-ceiling hint names
`rtk proxy cat`. That follows the contributing guide's rule that capping is
only acceptable with a hint that recovers the hidden data.
Results: the 15.8 MB log returns 11 401 bytes; a 110 KB Rust source and a
36 KB one come back byte for byte identical to raw.
Contributor
|
This pull request has been automatically marked as stale due to 90 days of inactivity. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. Your contribution is very much appreciated — we're sorry we haven't been able to review it yet. |
Contributor
|
Closing this pull request after a prolonged period of inactivity. If this is still relevant, please ask for it to be reopened. Thank you! |
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
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.
What
rtk readreturns a pathological file in full: a 15.8 MB log of 29 lines — the longest 7 842 749 characters — comes back as all 16 571 395 bytes.Why it matters here specifically
rtk readdefaults to--level none, and I think that default is right: filtering source code hides the code the reader came for. But "do not filter" was also serving as "do not bound", and those are different promises. Since the hook rewrites everycatinto this command, one such file goes straight into the model's context in full — in our usage a singlecatof that log cost more than a day of ordinary commands.Change
Two ceilings, both added to
LimitsConfigso they can be tuned or disabled:read_max_line_charsread_max_total_charsContent is never reinterpreted, only bounded, and each cut states what it dropped and how to get it back:
…+N more lines [read on: rtk read <file> --tail-lines N][N line(s) cut at 2000 chars; full text: rtk proxy cat <file>]The file itself is untouched on disk, so nothing is unrecoverable. This follows the contributing guide's rule that "capping output at N items is only acceptable if accompanied by a hint that lets the agent recover the hidden data."
guard::never_worsestill has the final say, as before.For stdin the total-ceiling hint goes through the existing
teemechanism, since there is no file to point at.Results
Tests
New
tests/read_ceilings_test.rs:ordinary_source_file_is_returned_unchanged— the guard that matters most; a 400-function source file must come back byte for byte.absurd_line_is_cut_and_says_how_to_recover_itrunaway_total_is_cut_and_says_how_to_read_onempty_and_tiny_files_are_untouchedFull suite green (2505 passed, 0 failed).
Note
Sent separately from #3208 on purpose — that one is a bug fix in
grep, this one changesreadbehaviour by default and deserves its own discussion. Happy to adjust the two defaults, or to gate the ceilings behind a flag if you would rather they were opt-in.