Skip to content

fix(grep): tee log for +N more overflow now holds full untruncated lines#3001

Open
albatrossflyon-coder wants to merge 1 commit into
rtk-ai:developfrom
albatrossflyon-coder:fix/grep-tee-log-full-fidelity
Open

fix(grep): tee log for +N more overflow now holds full untruncated lines#3001
albatrossflyon-coder wants to merge 1 commit into
rtk-ai:developfrom
albatrossflyon-coder:fix/grep-tee-log-full-fidelity

Conversation

@albatrossflyon-coder

Copy link
Copy Markdown

Summary

Fixes #2988 (the tee-log-fidelity half of it — see scope note below).

rtk grep -r caps per-file results and truncates long lines via
clean_line, then points overflow at a tee log via a +N more ... [see remaining: tail -n +N ...] hint. The bug: clean_line's truncated output
is the only copy of each line kept in by_file's entries — the
original, untruncated text is discarded right after truncation. Both the
interactive display and the tee log build from that same already-lossy
copy, so the tee log — whose entire purpose is full-fidelity recovery —
was just as truncated as the display it's supposed to recover from. There
was no way to get a matched line's full text back once it crossed the
length threshold, even by following the tool's own pointed-to recovery
path.

Fix

by_file's entries now carry both the cleaned (display) text and the
original raw text: (line_num, is_match, cleaned, raw). The interactive
body output keeps using cleaned, unchanged. match_block — which
builds both the tee log content and the skipped_files overflow block —
now uses raw instead. Same compact display, but the recovery path
actually recovers.

Scope note

The issue also raised two related but distinct observations I left out of
this PR, since they're either a separate bug or a design call for a
maintainer:

  • Whether the per-file cap / truncation should default to off (matching
    rtk read's behavior) is a design decision, not a pure bug — didn't want
    to bundle a behavior change into a fidelity fix.
  • The "single-file mode ignores --max-len" observation looked like a
    possibly-separate bug in a different code path; didn't want to guess at
    a fix for something I hadn't isolated.

Happy to open a follow-up for either if a maintainer wants them tackled
separately.

Testing

  • Reproduced the exact fixture from the issue (2 files × 30 matching
    lines, alternating short/padded-past-80-chars) against the local build:
    before the fix, the tee log's recovered lines were still ...-truncated
    exactly as reported; after the fix, tail -n +26 <tee log> shows the
    complete untruncated line text.
  • cargo test: 2449 passed, 8 ignored, 0 failed (1 more pass than before —
    the new regression test)
  • New unit test test_match_block_uses_raw_not_cleaned_content on
    match_block directly
  • cargo clippy --all-targets -- -D warnings: clean
  • cargo fmt --check: clean

rtk grep -r's per-file cap and clean_line's max-length truncation both
draw from the same by_file entries, and only the truncated (cleaned)
copy was ever kept. The "+N more ... [see remaining: tail -n +N ...]"
hint points at a tee log that's supposed to restore full fidelity for
the lines that didn't fit in the display, but since only the truncated
copy existed by that point, the tee log was just as lossy — there was
no way to recover a matched line's full text at all once it exceeded
the length threshold.

Carry the original (pre-clean_line) content alongside the cleaned copy
in by_file's entries, and use the raw copy when building match_block
for the tee log, while the interactive display keeps using the cleaned
copy as before. Same token-budget-friendly display, but the recovery
path this now actually recovers.

Fixes rtk-ai#2988

@tapheret2 tapheret2 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: fix(grep): tee log for +N more overflow now holds full untruncated lines

Files: src/cmds/system/search.rs
Size: +31 / -7

Notes

  • Static pass on the patch: no obvious blockers from the diff alone.

Thanks @albatrossflyon-coder — independent review on the patch.

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.

rtk grep -r silently truncates matched lines and caps per-file results, contradicting --help's "prints the whole matching line" behavior

2 participants