Repository navigation
fix(diff): remove misleading overflow indicator from condense_unified_diff - #2652
Conversation
d312ac6 to
34f9a34
Compare
…_diff All change lines are already printed in full (the comment says "Never truncate diff content"), but "... +N more" was appended when total changes exceeded 10. This confused LLM consumers into requesting "remaining" data that was already shown, wasting tokens.
34f9a34 to
35bd650
Compare
…ssert The section header still read "truncation accuracy" and one test still referred to a 10-line display cap. Neither describes the code any more: condense_unified_diff emits every change line, and this PR removes the indicator that claimed otherwise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
KuSh
left a comment
There was a problem hiding this comment.
Approving. Reviewed at depth and found nothing to change in the fix itself.
What I verified:
- The
changesvec is never capped — every+/-line is pushed unconditionally and all are emitted — sototal > 10 → "+{total-10} more"was pure fabrication, always claiming hidden lines that had in fact just been printed. Removing it is the correct fix, not a masking of real truncation. added/removedare still consumed by the[file] {} (+{} -{})header, so no dead variables.condense_unified_diffhas one caller (run_stdin), and no other module, test or doc references the overflow string. The other... +N moreoccurrences in the repo (deps.rs,pip_cmd.rs,summary.rs,prettier_cmd.rs) back real truncation and are untouched.- Output only shrinks, so
never_worse's raw-fallback behaviour is unaffected. cargo fmt --check,cargo clippy --all-targetsandcargo test --all(2583 unit + integration) are clean on the head.
Confirmed live on 0.46.0 before the change, so this is a real user-facing lie, not a theoretical one:
$ rtk diff - < twelve_changes.diff | tail -1
... +14 more # after printing all 24 change lines
I pushed one commit to your branch (7aa6347): the test module's section header still read // --- truncation accuracy --- and test_condense_unified_diff_no_false_overflow still mentioned a "10-line display cap". Neither describes the code once the indicator is gone, and leaving them would have re-planted the same misconception this PR removes. Shout if you'd rather word them differently.
Thanks for this one — it's a small diff, but "the filter tells the model that data is missing when it isn't" costs a re-fetch every time, and that's the expensive kind of wrong.
`" +"` / `" -"` match a one-space indent, but the form this guards against is two spaces, so the assertion passed on the code it rejects and only the equality loop above it killed the mutant. Match on leading whitespace instead. Verified by neutralising that loop and reverting the flush sites: the assertion now fails on its own. The trailer the old comment carved out for is gone with rtk-ai#2652, so the scoping it justified is no longer needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
Closes #2653.
condense_unified_diffprints ALL change lines in full (honoring the "Never truncate diff content" comment), then appends... +N morewhen total changes exceed 10. This misleads LLM consumers into requesting "remaining" data that was already shown, wasting tokens on redundant fetches.Before: 200 changes printed, then
... +190 more(implies 190 lines were hidden)After: 200 changes printed, no misleading indicator
Test plan
test_condense_unified_diff_overflow_count_accuracy→ renamed totest_condense_unified_diff_large_no_false_overflow_indicator, now asserts NO overflow message appears and verifies all lines are presenttest_condense_unified_diff_no_false_overflowcontinues to passcargo fmt --all && cargo clippy --all-targets && cargo test --allclean (2237 tests, 0 failures)