fix(read): make head/tail rewrites faithful to the native commands

[iliaal]

Problem

Rewriting head -N FILE to rtk read FILE --max-lines N selects a structural preview instead of the first N lines. Ordinary text can lose half the requested lines, and head -1 can produce no content. Bare head FILE becomes an unbounded read. The existing line-window implementation also drops CRLF endings, and --max-lines 0 underflows.

Change

Add --head-lines N for the exact first N lines, keeping --max-lines as a structural preview. Route bare head, head -N, head -n N, head --lines N, and head --lines=N through the new window.

Head and tail windows slice at newline byte offsets, preserving CRLF, NUL bytes, and unterminated final lines. At --level none with line numbering off, files and stdin use byte reads and writes, so non-UTF-8 input survives without a decoding failure. Filtering and numbering retain their text-processing path.

An operand allowlist keeps globs, options, and uncertain shell operands on the native command. Separate trailing redirects from appended line-count values without changing shared prefix-rewrite behavior. Guard --max-lines 0 against underflow.

Scope

Exact byte output applies to head/tail windows at the default --level none with line numbering off. Other read modes still require UTF-8. Reads still consume input through EOF: bounded streaming and FIFO/device handling remain under #4012, and savings accounting remains under #3339. The head and tail corrections stay together because they share the same window and operand-handling rules.

Verification

• cargo fmt --all --check passed.
• cargo clippy --all-targets --all-features -- -D warnings passed.
• cargo build --release passed. Local head-window timing was 49.7 ms versus 50.6 ms for the existing structural-preview path; neither read path met the repository's 10 ms target on this machine.
• cargo test --all: 3,471 passed, 0 failed, and 8 ignored. The three ignored read tests passed when run explicitly.
• Five new CLI regression tests cover non-UTF-8 files and stdin, byte boundaries, filtering, numbering, and rewrite-to-execution comparisons against native head.
• 144 native head/tail comparisons matched stdout bytes and exit status across files and stdin, including empty input, CRLF, unterminated lines, multibyte UTF-8, non-UTF-8/NUL bytes, and blank lines, with N in {0, 1, 3, 10, 50, 500}.

[KuSh]

Round 1 of 3 — CHANGES REQUESTED

Claim: head/tail rewrites produce byte-identical output to the native commands, and operands that could expand to several words or turn into options stay native.

Scope: accept (round 1, frozen). This supersedes #1018, #3166 and #3668, and closes #3421. #1018 adds the same --head-lines flag but is a strict subset — lines().join() (so CRLF is still lost), a starts_with('-') blocklist rather than an allowlist (so '--', \--help, #c and globs still route), no redirect separation, no --max-lines 0 fix — and it is CONFLICTING against a registry.rs that has since moved from lazy_static! to LazyLock. This branch is mergeable and green. Recommend closing those three against this one.

Ran: both binaries built from worktrees. head/tail vs rtk read md5 over {200-line, CRLF, unterminated, empty, 5-line} × N ∈ {0,1,3,10,50,500} — 30/30 identical for --head-lines, and --tail-lines now matches on CRLF where develop corrupted it (b\r\nc\r\n → b\nc\n). 441-operand differential fuzz of is_single_file_operand against real bash word-splitting: zero misroutes (the 27 flagged are the ;/& operator-split cases your own test documents, and they are correct). 2,292 head/tail commands over real repo paths: zero coverage lost vs develop. Merge into latest upstream/develop is clean, 3358 tests green on the merged tree. Startup 1.7 ms. Mutation-tested the new tests: an off-by-one in head_window fails 7, opening the allowlist fails 3, removing the max_lines == 0 guard fails 1.

Savings do not apply here: a byte-verbatim window is 0% by construction, and that is the point of the change, not a regression against the 20% floor.

CI @ed8480f3: 11 SUCCESS, 1 skipped — green.

Blocks merge (1, frozen at round 1)

1. src/discover/registry.rs:89 — newly routed spellings lose content on non-UTF-8 input. HEAD_N_SPACE and HEAD_LINES_SPACE send head -n N FILE and head --lines N FILE into rtk read, which does fs::read_to_string(...)?. On develop those two spellings had no matcher and ran native, so this is a regression on the input class, not a pre-existing hazard:

   $ printf '\xff\xfe bad\nline2\nline3\n' > bin.log
   $ head -n 2 bin.log                 # develop: native
   <13 bytes of content>               rc=0
   $ rtk read bin.log --head-lines 2   # this branch
   cat: bin.log: stream did not contain valid UTF-8
   <0 bytes on stdout>                 rc=1
   

   Same cause, second symptom: head -n 2 /dev/urandom returns instantly natively and never terminates once routed (killed at 5 s). head -2 and bare head already had both symptoms on develop — this widens them to the -n N spelling.

   Smallest fix, your choice: drop the two new matchers (restores develop's behaviour for those spellings, costs the coverage they add), or make read::run fall back to byte mode when read_to_string returns InvalidData — head_window/tail_window are already byte-sliced, so they work on &[u8] nearly unchanged, and that also fixes develop's existing head -N / bare-head routes. I'll file the class either way; I am not asking you to fix the FIFO/streaming half here.

Optional — will not hold merge

All docs/comment-level; none of it needs your time. I have these fixed locally and will push them as a fixup on top of your next push, so don't spend a commit on them unless you'd rather own them yourself.

• docs/contributing/TECHNICAL.md:169, src/discover/README.md:29 still document head -N → --max-lines N; docs/usage/FEATURES.md:133 has no --head-lines/--tail-lines row, so the new flag is undocumented outside --help.
• src/discover/registry.rs:83 — the #1362 comment still says "banners that rtk read --max-lines cannot reproduce"; this path no longer emits --max-lines.
• src/discover/registry.rs:1592 — starts_with("head -") || starts_with("head "): the first disjunct is dead, every string matching it matches the second.
• .claude/hooks/rtk-suggest.sh:92,96 — still suggests rtk read FILE --max-lines N for head -N, i.e. exactly the mapping this branch proves wrong.

Filed as follow-ups

• #4012 — rtk read loses all content on non-UTF-8 files and never terminates on FIFOs/character devices; every head/tail rewrite routes through it. The class behind the blocking item; stays open for develop's existing head -N and bare-head routes however this PR lands.
• #4013 — head --lines[=]N is GNU-only, so routing it turns a macOS usage error into a silent success. --lines=N already had this on develop. Flagged in the issue that I could not execute the macOS side.
• Savings baseline is the whole file, so a byte-faithful window reports savings that do not exist: rtk read seq5000.txt --head-lines 5 reports Tokens saved: 6.0K (99.9%) for 10 bytes identical to what head -n 5 would have printed. Pre-existing mechanism, now fully fictitious for this route — absorbed by #3339, evidence added there rather than filed separately.

Checked and correct — no need to re-verify

• head_window/tail_window byte slicing, including the terminal-newline discount in tail_window, multibyte UTF-8, blank-only files, n=0, n > line count.
• is_single_file_operand: no accepted token is still shell-active; ~ is correctly reasoned about.
• join_redirect_suffix: 1>&2 is not split, a>b keeps the flag value, an already-spaced suffix gains no second space.
• smart_truncate: kept_lines + 1 >= max_lines is arithmetically identical to the old form for all max_lines >= 1 (md5-equal output for N ∈ {1,2,5,10,50}); the max_lines == 0 guard fixes a real debug panic, confirmed on develop (attempt to subtract with overflow at filter.rs:349).
• Multi-file and glob operands stay native on both head and tail; develop routed head src/core/*.rs and tail -20 src/core/*.rs into a banner-less concatenation.

Hypotheses built and dropped

• Memory blow-up on large files is a regression — no. On a 191 MB file: native 11 MB, this branch 389 MB, develop 445 MB. This branch is the better of the two; the whole-file read is pre-existing and goes in the class issue.
• --max-lines was left untested when its tests moved to --head-lines — no. test_apply_line_window_max_lines_still_works is retained verbatim and test_max_lines_zero_is_empty is added. (Separately: mutating the structural branch to if true — i.e. making --max-lines behave like --head-lines — breaks no test, but that gap is identical on develop, so it is not this PR's.)
• The allowlist misroutes ;/& operands — no, the lexer splits on them first and the rewrite is semantically equivalent.

Your questions

• "The operand-guard and CRLF changes affect tail as well as head; I can split those into a separate commit if you prefer." → Keep them together. The tail side carries two of this branch's real fixes on its own: develop routes tail -20 src/core/*.rs into a banner-less concatenation, and develop's --tail-lines corrupts CRLF (b\r\nc\r\n → b\nc\n). Splitting would leave tail misrouting between two merges for no gain.

Next

Fix the one blocking item — dropping the two new matchers is enough if you'd rather not touch read::run. Everything under "Optional" is mine: I have it as a fixup locally (docs, the stale comment, the dead disjunct, the suggest hook; fmt clean, clippy 0 warnings, 3358 tests green) and I'll push it on top once you've pushed and CI is green, so it never lands on an unbuilt head.

Rounds: 1/3. Threads: 1 open (blocking item), 0 resolved this round.

[KuSh]

Correction to my review — one of the three follow-ups was wrong, and it's withdrawn.

#4013 (head --lines[=]N is GNU-only, so routing it hides a macOS usage error) is closed as invalid. I filed it without running the macOS side — I said so in the issue, but I should have found a way to check rather than file on a remembered interface.

macOS head supports --lines. Apple open-sources it in text_cmds, and it declares {"lines", required_argument, NULL, 'n'} in a getopt_long table. I built that source in a container and ran it:

$ ./head_macos --lines=2 f.txt   →  line1 line2   rc=0
$ ./head_macos --lines 2 f.txt   →  line1 line2   rc=0

So HEAD_LINES_SPACE is fine on macOS and there is nothing to change for it. FreeBSD is the same; only OpenBSD lacks the long option, and that isn't a target platform here.

Unchanged: the blocking item stands — it never depended on this. head -n N FILE is the regression surface (non-UTF-8 content loss, exit-code flip, and the /dev/urandom hang), and #4012 is the class behind it. Both were verified by execution on Linux.

Net effect on what I'm asking for: nothing. Sorry for the detour.

[KuSh]

Round 2 of 3 — APPROVED, follow-ups filed

Claim: head/tail rewrites produce byte-identical output to the native commands, and operands that could expand to several words or turn into options stay native.

Scope: accept (round 1, frozen). Still supersedes #1018, #3166 and #3668, and closes #3421.

Ran: rebuilt ed8480f and 23a1be6 side by side. 140 cases of head/tail vs rtk read across {200-line, CRLF, unterminated, empty, blank-only, multibyte UTF-8, 3 KB random binary, \xff\xfe text, embedded NUL} × N ∈ {0,1,2,3,10,50,500} — 0 mismatches on bytes and exit code. 50 further UTF-8 boundary cases (multibyte straddling the slice point, lone \xff next to valid text): 0 mismatches. 70-case regression check against the round-1 head on UTF-8 input: 0 differences. End-to-end rewrite-then-execute for all five spellings on binary input: all match native. stdin path, multi-file concatenation, missing-file and directory error paths, the stdin dedup warning: identical to the round-1 head. Routing re-fuzzed (968 operand-commands, 0 unsafe routes; 1600 real-path commands, 0 coverage lost). Merged into latest upstream/develop: clean, compiles, 3613 tests, fmt clean, clippy 0. Startup 1.6–1.8 ms.

Savings do not apply: a byte-verbatim window is 0% by construction, which is the point of the change rather than a miss against the 20% floor.

CI @23a1be62: 11 SUCCESS, 1 skipped — green. Fixup pushed on top; approving on 784d3470 once its own run is green.

Blocks merge (0 — the round-1 item is fixed)

The single round-1 blocker is closed by 6f4913b, verified by execution:

$ printf '\xff\xfe bad\nline2\nline3\n' > bin.log
$ head -n 2 bin.log                 → 13 bytes, rc=0
ed8480f3  rtk read … --head-lines 2 → 0 bytes,  rc=1   "stream did not contain valid UTF-8"
23a1be62  rtk read … --head-lines 2 → 13 bytes, rc=0   byte-identical to native

Worth saying plainly: you fixed more than I asked for. head -2 FILE and bare head FILE were broken on develop too, and the byte path repairs both — so this PR now closes the content-loss half of #4012 rather than merely not widening it. The whole-file read remains, so the /dev/urandom hang and the streaming half stay open there, exactly as you said.

The text path also got cheaper, not just correct: on a 144 MB UTF-8 file peak RSS drops from 293 MB to 151 MB, because from_utf8_lossy borrows instead of copying when the input is valid.

Optional — will not hold merge

Covered by the fixup below; nothing here needs your time.

• tests/read_window_bytes_test.rs — rewritten_head_spellings_match_native_on_non_utf8_files isolates CLAUDE_CONFIG_DIR but reads the ambient XDG_CONFIG_HOME. With exclude_commands = ["head", "tail"] in ~/.config/rtk/config.toml the rewrite is suppressed and the exit-code-3 assertion fails (4 passed, 1 failed, reproduced locally). Clean runners have no such config, so it is green in CI and red only for developers who opted out of these very rewrites.
• src/cmds/system/read.rs:29,127 — the fast path returns before the --verbose diagnostics, so rtk -v read f --head-lines 2 loses the Lines: N -> M line and -vv loses Detected language:. I am not putting this in the fixup, because restoring the old line verbatim would restore a wrong number: it was computed before apply_line_window, so on the round-1 head --head-lines 2 on a 5-line file printed Lines: 5 -> 5. Silence is less misleading than that. If you want the diagnostic back, it wants the post-window count — your call, and fine as a follow-up.
• The round-1 docs items, unchanged and still applying cleanly: docs/contributing/TECHNICAL.md:169, src/discover/README.md:29, no --head-lines row in docs/usage/FEATURES.md, the stale --max-lines wording in the #1362 comment, the dead starts_with("head -") disjunct, and .claude/hooks/rtk-suggest.sh still suggesting --max-lines N for head -N.

Filed as follow-ups

• #4012 — now partly fixed by this PR. The non-UTF-8 content loss is gone for all spellings; the whole-file read, the FIFO/character-device hang and the large-file footprint remain. I'll narrow its title and add the evidence once this merges, rather than leaving it describing a symptom you have fixed.
• #3339 — savings accounting. New evidence from this push, added there: the fast path calls timer.track with String::from_utf8_lossy of the whole input, and estimate_tokens is just len()/4, so the lossy string is materialised only to take a length. Every invalid byte expands to a 3-byte U+FFFD, which inflates the input side only. A 406-byte file of \xff plus one short line records input_tokens=302, savings=99.34% where the same-size valid-UTF-8 file correctly records 102, 98.04%. Same root cause as the footprint: 144 MB of binary peaks at 411 MB. Fix is a length-based tracking call rather than a lossy string — that changes a number users see in rtk gain, so it is yours or a follow-up, not something I fold into a fixup.
• No new issue filed this round; both leftovers land on existing issues.

Checked and correct — no need to re-verify

• Byte fast path is equivalent to the old text path for valid UTF-8 across every case above, including --head-lines 0, n > line count, unterminated final lines and CRLF.
• Returning Ok(()) early skips nothing that mattered: never_worse cannot fire on a window that is by construction no larger than its input, and the empty-filter safety net is not reachable at --level none.
• --level and --line-numbers still take the text path, and your own test pins all four combinations.
• Round-1 surface (operand allowlist, join_redirect_suffix, smart_truncate, clap wiring) is untouched by this push and re-probed above with no change.

Hypotheses built and dropped

• error[E0308]: type mismatch in the merged-tree test run — not a compiler error. It is fixture text printed by a test in src/core/stream.rs, spliced mid-line into the progress output. cargo test --all --no-run and the full run both exit 0.
• from_utf8_lossy on the whole input is a new memory regression — no. For valid UTF-8 it borrows, and the text path got lighter (293 MB → 151 MB on 144 MB). Binary input costs 411 MB where develop simply errored out, so it is the price of the new capability, not a regression; it belongs to the whole-file-read half of #4012.
• The new tests write to the developer's tracking DB — true (history.db grows), but 12 of 12 integration test files spawn the binary and 0 set RTK_DB_PATH; this file follows the existing convention and #3758 is the repo-wide fix. The tests never read the DB, so it is a write side-effect, not a dependence — not this PR's to carry.

Your questions

• "I'll keep the head/tail changes together" → Agreed, and that was the right call for the same reason as round 1: the tail side carries its own fixes (glob misroute, CRLF corruption), so splitting would leave tail broken between two merges.
• "leave your prepared optional docs/comment/hook fixups to you" → Taken. They are in the fixup below, rebased onto your head and re-verified.
• "Reads still consume through EOF, so this does not fix the /dev/urandom hang … Savings accounting remains under #3339" → Agreed on both, and neither holds this PR. Confirmed the hang is unchanged on both heads; #4012 and #3339 carry them.
• "No changes are needed for the withdrawn #4013 finding" → Correct, and that was my error, not yours.

Next

Nothing on your side — approving.

Fixup pushed on top of your head, two commits, after CI went green on 23a1be6:

• e224b159 — the round-1 docs items (pipeline diagram, discover walkthrough, the rtk read option table, the stale #1362 comment, the dead head - disjunct, and rtk-suggest.sh).
• 784d3470 — XDG_CONFIG_HOME isolation for the rewrite test, verified to pass both with and without exclude_commands = ["head", "tail"].

cargo fmt --all --check clean, clippy 0 warnings, full suite green, and the merge into latest develop is clean at 3613 tests. Revert either if you disagree — they are additive and touch nothing you wrote.

Rounds: 2/3. Threads: 0 open, 1 resolved this round (the round-1 blocker, fixed by 6f4913b).