fix(read): don't flag UTF-8 text as binary when byte 1000 cuts a multibyte char - #80261
fix(read): don't flag UTF-8 text as binary when byte 1000 cuts a multibyte char#80261luntion wants to merge 1 commit into
Conversation
…ibyte char The binary-content sampler in read_file/read_file_raw took the first 1000 bytes with head -c 1000 and decoded with errors="replace". When the byte-aligned cut landed inside a multi-byte UTF-8 character, the truncated tail decoded to U+FFFD, and _is_likely_binary — correctly, for genuine mojibake — flagged the whole file as binary and refused to read it. Legitimate UTF-8 text (CJK, Cyrillic, emoji) was blocked roughly half the time depending on exact byte length. Fix both call sites to sample via python and extend the window to the next UTF-8 character boundary before writing the sample, so the probe never contains a truncated character. Genuinely undecodable content still produces U+FFFD mid-sample and is still caught by _is_likely_binary, preserving the read→edit→write corruption guard that errors="ignore" would have defeated. Adds TestReadUtf8BoundaryCut regression tests (3-byte CJK straddling byte 1000, 4-byte emoji spanning the tail, genuine binary still flagged) and updates the command-shape assertion in test_read_file_uses_bash_safe_windows_paths.
|
Triage note: this appears to duplicate three other PRs that fix the same issue — UTF-8 files whose 1000-byte read_file sample lands mid-multibyte-char (trailing U+FFFD) get misclassified as binary. Same files (tools/file_operations.py + tests), same fix. The cluster: #80186, #80188, #80250. Recommend picking one to champion and closing the rest as duplicates. |
…ort-lossy text Fixes the read_file half of #80308 and the class behind #80261, #80250, The binary sniff sampled files via 'head -c 1000' through the terminal transport, which decodes stdout with errors="replace". A multibyte character cut at byte 1000 therefore arrived as U+FFFD, and _is_likely_binary treated any U+FFFD as binary — flagging valid CJK and emoji text as unreadable. At the text layer a stored replacement char and a transport-manufactured one are indistinguishable, which is why per-callsite adjustments kept leaving siblings open. Sample as 'head -c 1000 | base64' so raw bytes survive the transport (fail-open to the legacy heuristic when the transport cannot produce clean base64), then classify bytes: NUL => binary; valid UTF-8 allowing one incomplete multibyte sequence at the sample end => text; mid-stream invalid UTF-8 (latin-1, true binaries) => read-only, preserving the anti-mojibake guarantee the old check existed for. Files legitimately containing U+FFFD become readable.
|
Closing — superseded by #81961, which fixed this bug class (1000-byte sample cutting a multibyte char → binary false positive) at the byte layer across read_file, patch, and search_files, with regression tests for truncated-CJK/BOM/UTF-16/NUL cases. ~24 PRs raced on this one; earliest diagnosis credit to @webtecnica (#76924), merged implementation from @ayushnangia (#80440). Thanks for jumping on it. |
…ort-lossy text Fixes the read_file half of NousResearch#80308 and the class behind NousResearch#80261, NousResearch#80250, The binary sniff sampled files via 'head -c 1000' through the terminal transport, which decodes stdout with errors="replace". A multibyte character cut at byte 1000 therefore arrived as U+FFFD, and _is_likely_binary treated any U+FFFD as binary — flagging valid CJK and emoji text as unreadable. At the text layer a stored replacement char and a transport-manufactured one are indistinguishable, which is why per-callsite adjustments kept leaving siblings open. Sample as 'head -c 1000 | base64' so raw bytes survive the transport (fail-open to the legacy heuristic when the transport cannot produce clean base64), then classify bytes: NUL => binary; valid UTF-8 allowing one incomplete multibyte sequence at the sample end => text; mid-stream invalid UTF-8 (latin-1, true binaries) => read-only, preserving the anti-mojibake guarantee the old check existed for. Files legitimately containing U+FFFD become readable.
…ort-lossy text Fixes the read_file half of NousResearch#80308 and the class behind NousResearch#80261, NousResearch#80250, The binary sniff sampled files via 'head -c 1000' through the terminal transport, which decodes stdout with errors="replace". A multibyte character cut at byte 1000 therefore arrived as U+FFFD, and _is_likely_binary treated any U+FFFD as binary — flagging valid CJK and emoji text as unreadable. At the text layer a stored replacement char and a transport-manufactured one are indistinguishable, which is why per-callsite adjustments kept leaving siblings open. Sample as 'head -c 1000 | base64' so raw bytes survive the transport (fail-open to the legacy heuristic when the transport cannot produce clean base64), then classify bytes: NUL => binary; valid UTF-8 allowing one incomplete multibyte sequence at the sample end => text; mid-stream invalid UTF-8 (latin-1, true binaries) => read-only, preserving the anti-mojibake guarantee the old check existed for. Files legitimately containing U+FFFD become readable.
…ort-lossy text Fixes the read_file half of NousResearch#80308 and the class behind NousResearch#80261, NousResearch#80250, The binary sniff sampled files via 'head -c 1000' through the terminal transport, which decodes stdout with errors="replace". A multibyte character cut at byte 1000 therefore arrived as U+FFFD, and _is_likely_binary treated any U+FFFD as binary — flagging valid CJK and emoji text as unreadable. At the text layer a stored replacement char and a transport-manufactured one are indistinguishable, which is why per-callsite adjustments kept leaving siblings open. Sample as 'head -c 1000 | base64' so raw bytes survive the transport (fail-open to the legacy heuristic when the transport cannot produce clean base64), then classify bytes: NUL => binary; valid UTF-8 allowing one incomplete multibyte sequence at the sample end => text; mid-stream invalid UTF-8 (latin-1, true binaries) => read-only, preserving the anti-mojibake guarantee the old check existed for. Files legitimately containing U+FFFD become readable.
…ort-lossy text Fixes the read_file half of NousResearch#80308 and the class behind NousResearch#80261, NousResearch#80250, The binary sniff sampled files via 'head -c 1000' through the terminal transport, which decodes stdout with errors="replace". A multibyte character cut at byte 1000 therefore arrived as U+FFFD, and _is_likely_binary treated any U+FFFD as binary — flagging valid CJK and emoji text as unreadable. At the text layer a stored replacement char and a transport-manufactured one are indistinguishable, which is why per-callsite adjustments kept leaving siblings open. Sample as 'head -c 1000 | base64' so raw bytes survive the transport (fail-open to the legacy heuristic when the transport cannot produce clean base64), then classify bytes: NUL => binary; valid UTF-8 allowing one incomplete multibyte sequence at the sample end => text; mid-stream invalid UTF-8 (latin-1, true binaries) => read-only, preserving the anti-mojibake guarantee the old check existed for. Files legitimately containing U+FFFD become readable.
Summary
read_file/read_file_rawmisclassify legitimate UTF-8 text files as binarywhen the 1000-byte sampling window lands mid-way through a multi-byte character.
Symptoms
A UTF-8 text file whose byte 1000 falls inside a multi-byte char (CJK, Cyrillic,
emoji — anything 2–4 bytes) gets flagged
is_binary: trueand refuses to read.Concretely on a Chinese-language markdown file:
Same file reads fine on a retry because the byte position of the truncation
depends on the file's exact byte length — it is a ~50% lottery per file, not a
content property.
Root cause
tools/file_operations.pysamples the first 1000 bytes withhead -c 1000, then decodes that sample witherrors="replace". When byte1000 cuts a multi-byte UTF-8 character in half, the truncated tail decodes to
U+FFFD (replacement char).
_is_likely_binarysees U+FFFD and — correctly, forgenuine mojibake — treats the file as binary and blocks the read. The sample
boundary is byte-aligned, not character-aligned, so any UTF-8 text can trip it.
Verified: for the affected files,
head -c 1000ends with a continuation byte(
0x80–0xBF); decoding that sample yields exactly one U+FFFD at the tail;_is_likely_binarythen returns True.Changes
tools/file_operations.py(both call sites —read_fileandread_file_raw):Replace the raw
head -c 1000sample with a small python probe that reads thefirst 1000 bytes and extends the window to the next UTF-8 character
boundary before writing the sample:
while n < len(d) and (d[n] & 0xC0) == 0x80: n += 1— skip continuationbytes so the sample always ends on a complete character.
sys.stdout.buffer.write(raw bytes, no re-encoding),matching how the terminal env already decodes stdout as UTF-8.
sys.executableis shell-escaped via the existing_escape_shell_arg.Why this approach (not
errors="ignore"): decoding the sample witherrors="ignore"would drop the U+FFFD signal entirely, so genuinelyundecodable content (latin-1 blobs, raw binary without a known extension) would
pass
_is_likely_binaryand get returned as lossy text — enabling aread→edit→write round-trip to silently corrupt the original bytes. That
protection is the entire point of the U+FFFD branch (see its docstring). The
boundary-extension fix removes only the artifact of the byte-aligned cut;
real mojibake still produces U+FFFD mid-sample and is still caught.
Testing
TestReadUtf8BoundaryCutclass (4 tests, real subprocess viaLocalEnvironment):read_filereturns text, not binaryread_file_rawtest_read_file_uses_bash_safe_windows_pathscommandassertions to the new python sampler command shape.
tests/tools/test_file_operations.py+test_file_operations_edge_cases.py:63 passed, 8 failed — all 8 failures are pre-existing on clean
mainon thisWindows box (
TestSearchFilesFallbackHiddenPaths×2, atomic-write umaskpermissions ×4, symlink writes ×2 — local-environment limitations, unrelated
to this change; verified by stashing the change and re-running).
Related
errors="ignore"andwas rejected in review for weakening the mojibake guard; this PR fixes the
same bug without that regression).
Risks
python -c …instead ofhead -c 1000. It runsonce per read call, reads at most ~1003 bytes, and exits immediately — cost
is negligible.
sys.executableis escaped like every other shell arg in thefile. Non-UTF-8 text files (e.g. latin-1) keep the existing behavior: sample
decodes with replacement chars → flagged binary → read-only, preventing
silent corruption. No config, schema, or env changes.