fix(ledger): read the line budget per command, not per reply - #752
Conversation
rations_its_output was narrowed to command position by #742, and read that position off the previous token. split_whitespace has already eaten the newline by then, so a two-line command is one command and nothing on the second line can open one. That is the #741 shape back for the way an agent actually writes a command: cd /path/to/repo sed -n '58,95p' src/app.tsx Replaying both versions of the predicate over the 4,193 commands recorded in ~/.omni/omni.db, the narrowing gave up the guard on 168 of them, and 136 of those to a newline. 3.2% of the corpus, folding lines the caller had just named. Split the reply at its separators and ask the question per segment. A newline is one of them, the sed scan can no longer reach a later command's 5p, and a separator glued to the following token works too. sudo, env, time, nohup and VAR=value introduce a command without being one, and a wrapper's argument list is somebody else's command line, so the name counts anywhere inside it. Closes #750 Closes #751
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fn opens_a_command(tokens: &[&str], i: usize) -> bool { | ||
| match i.checked_sub(1).map(|p| tokens[p]) { | ||
| None => true, | ||
| Some(prev) => prev.ends_with([';', '|', '&']), | ||
| } | ||
| tokens[..i].iter().all(|t| introduces_a_command(t)) | ||
| } |
There was a problem hiding this comment.
When a bounded reader uses an option-bearing prefix such as sudo -u root tail -5 app.log, opens_a_command rejects the reader because -u and root are not introducers, causing project-origin folds to replace explicitly requested lines with a retrieval marker.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ledger/mod.rs
Line: 988-990
Comment:
**Prefix options hide bounded commands**
When a bounded reader uses an option-bearing prefix such as `sudo -u root tail -5 app.log`, `opens_a_command` rejects the reader because `-u` and `root` are not introducers, causing project-origin folds to replace explicitly requested lines with a retrieval marker.
**Knowledge Base Used:**
- [Restore command-aware ledger folding](https://app.greptile.com/weekndlabs/-/custom-context/knowledge-base/fajarhide/omni/-/reverts/incident-mitigation_737-20260830-command-budgeted-output-cdc73f6.md)
- [Restore accurate ledger handling for bounded command output](https://app.greptile.com/weekndlabs/-/custom-context/knowledge-base/fajarhide/omni/-/reverts/rollback_742-20260901-ledger-line-budget-detection-f1fc2a4.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
Valid, and fixed in 09570e0. Same root as the other two P1s: the walk asked that every earlier token be an introducer, so an introducer's own option and its argument put the reader out of position. sudo, env, time, nohup and a VAR=value assignment now make the segment opaque, on the same reasoning wraps_another_command already applies to docker exec, so the name counts anywhere inside it and their flag grammars never have to be parsed.
| command | ||
| .split(['\n', ';', '|', '&']) |
There was a problem hiding this comment.
The unconditional split treats separators inside heredoc bodies, comments, and quoted arguments as command boundaries, so literal head, tail, or numeric sed text disables project folding for replies that did not request bounded output, increasing delivered context unnecessarily.
Knowledge Base Used: Restore accurate ledger handling for bounded command output
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ledger/mod.rs
Line: 954-955
Comment:
**Literal separators become command boundaries**
The unconditional split treats separators inside heredoc bodies, comments, and quoted arguments as command boundaries, so literal `head`, `tail`, or numeric `sed` text disables project folding for replies that did not request bounded output, increasing delivered context unnecessarily.
**Knowledge Base Used:** [Restore accurate ledger handling for bounded command output](https://app.greptile.com/weekndlabs/-/custom-context/knowledge-base/fajarhide/omni/-/reverts/rollback_742-20260901-ledger-line-budget-detection-f1fc2a4.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Valid in principle, and measured at zero instances, so it is not fixed here.
Replaying the predicate over the 4,193 commands recorded in ~/.omni/omni.db, 51 of them
are rationed by this version and were not by the anywhere-match that preceded #742. Every
one of the 51 is a real bounded reader that the token walk could not see: a separator
glued to the next token (2>&1|tail -1, --json x|head), or a multi-address sed -n '1,20p;70,110p' that the old version read as one token and rejected. None came from a
heredoc body or a quoted argument.
The direction also matters. A boundary invented inside a quote costs context and cannot
delete a line, which is the cheap half of this predicate's two failure modes. A
quote-aware splitter is the parser this predicate keeps refusing to be, and it gets
written when a payload demands it rather than before.
There was a problem hiding this comment.
Agreed. Given the replay found zero instances of this failure mode, while the current splitter recovers real bounded-reader commands and only risks over-preserving context when it encounters a quoted or literal separator, this is not actionable for this PR. A quote-aware shell splitter can be a separate improvement if future payloads demonstrate the need.
| matches!( | ||
| token.rsplit('/').next().unwrap_or(token), | ||
| "sudo" | "env" | "time" | "nohup" | ||
| ) || token.split_once('=').is_some_and(|(name, _)| { | ||
| !name.is_empty() && name.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'_') | ||
| }) | ||
| } |
There was a problem hiding this comment.
When a bounded reader uses an ordinary option-bearing prefix such as env -i head -20 file or time -p sed -n 5p file, the option token fails introduces_a_command, causing project-origin folds to replace the requested lines with a retrieval marker.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ledger/mod.rs
Line: 994-1000
Comment:
**Introducer options bypass line protection**
When a bounded reader uses an ordinary option-bearing prefix such as `env -i head -20 file` or `time -p sed -n 5p file`, the option token fails `introduces_a_command`, causing project-origin folds to replace the requested lines with a retrieval marker.
**Knowledge Base Used:**
- [Restore command-aware ledger folding](https://app.greptile.com/weekndlabs/-/custom-context/knowledge-base/fajarhide/omni/-/reverts/incident-mitigation_737-20260830-command-budgeted-output-cdc73f6.md)
- [Restore accurate ledger handling for bounded command output](https://app.greptile.com/weekndlabs/-/custom-context/knowledge-base/fajarhide/omni/-/reverts/rollback_742-20260901-ledger-line-budget-detection-f1fc2a4.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Valid, and fixed by the same change. env -i and time -p are the introducer's own flags, and the segment is now opaque rather than walked.
| let opaque = registry::wraps_another_command(segment); | ||
| tokens.iter().enumerate().any(|(i, tok)| { | ||
| if !opens_a_command(&tokens, i) { | ||
| if !opaque && !opens_a_command(&tokens, i) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
When an opaque wrapper is preceded by an introducer, as in sudo docker exec app tail -5 app.log, wrapper detection sees sudo instead of docker and command-position checking rejects the nested reader, causing project folding to replace the wrapper’s requested output with a retrieval marker.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ledger/mod.rs
Line: 968-972
Comment:
**Prefixed wrappers lose opacity**
When an opaque wrapper is preceded by an introducer, as in `sudo docker exec app tail -5 app.log`, wrapper detection sees `sudo` instead of `docker` and command-position checking rejects the nested reader, causing project folding to replace the wrapper’s requested output with a retrieval marker.
**Knowledge Base Used:**
- [Pipeline stages and quality controls](https://app.greptile.com/weekndlabs/-/custom-context/knowledge-base/fajarhide/omni/-/docs/pipeline-stages-and-quality.md)
- [Restore command-aware ledger folding](https://app.greptile.com/weekndlabs/-/custom-context/knowledge-base/fajarhide/omni/-/reverts/incident-mitigation_737-20260830-command-budgeted-output-cdc73f6.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Valid, and fixed by the same change. sudo docker exec app tail -5 x is opaque on the sudo, so the wrapper behind it no longer has to be found.
1 participant
rations_its_outputwas narrowed to command position by #742, and read that positionoff the previous token.
split_whitespacehas already eaten the newline by then, so atwo-line command is one command and nothing on the second line can open one. That put
the #741 shape back for the way an agent actually writes a command:
The reply is now split at its separators and the question is asked per command. A
newline separates, the
sedscan cannot reach a later command's5p, a separator gluedto the following token works,
sudo/env VAR=1/time/nohupstep aside, and awrapper such as
docker exec app tail -5 app.logkeeps the lines it named, since itsargument list is somebody else's command line and no position in it is ours to read.
Measured by replaying the predicate over the 4,193 commands recorded in
~/.omni/omni.db:168 of the 201 recovered are the regression, 136 of them to a newline. The other 33 are
sed -n '255,300p;403,497p' fand friends, where the old version read the wholemulti-address as one token and rejected it. The 18 commands the anywhere-match caught
and this does not are #738's own case: the word inside a heredoc body or a path.
Verification:
make cigreen.rations_its_output_reads_a_line_budget_and_nothing_elsewas driven red five times, once per direction the fix can be wrong (newline not a
separator, wrapper not opaque, no prefix words,
;not a separator, and thesedboundary on its own with the positives that shadow it removed).
Closes #750
Closes #751
Greptile Summary
The PR changes ledger line-budget detection to evaluate newline- and operator-separated command segments, recognizes several command introducers, and reuses wrapper detection from the pipeline registry.
head,tail, and numericsedbudgets.sudo,env,time, andnohupprefixes.Confidence Score: 3/5
The PR should not merge until option-bearing prefixes and prefixed opaque wrappers reliably preserve explicitly requested lines.
The new command-position logic misses bounded readers after common prefix options, and wrapper detection misses wrappers preceded by introducers, allowing project folds to hide the requested output; raw separator splitting also causes a non-blocking regression for literal command text.
Files Needing Attention: src/ledger/mod.rs and src/pipeline/registry.rs
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Raw command] --> B[Split into segments] B --> C{Opaque wrapper?} C -->|Yes| D[Scan bounded reader anywhere] C -->|No| E[Find command after introducers] D --> F{Line budget found?} E --> F F -->|Yes| G[Preserve project-origin lines] F -->|No| H[Allow normal ledger folding]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(ledger): read the line budget per co..." | Re-trigger Greptile
Context used (3)