fix(read_file): treat lone trailing U+FFFD as UTF-8 truncation artifact, not binary - #81834
Closed
VIVAAN-DHAWAN wants to merge 1 commit into
Closed
VIVAAN-DHAWAN wants to merge 1 commit into
VIVAAN-DHAWAN wants to merge 1 commit into
Conversation
…ct, not binary read_file samples via `head -c 1000`, which truncates at a byte boundary. When that boundary splits a multi-byte UTF-8 character, the terminal env's errors=replace decode produces exactly one *trailing* U+FFFD — which the binary detector flagged as binary, blocking reading of valid UTF-8 text files like Portuguese .md docs (NousResearch#81480). Genuine binary data yields many scattered replacement chars, so only a sample with >1 U+FFFD, or one that ends without a trailing U+FFFD, is treated as binary. A lone trailing replacement is now read as text. Adds regression tests for the sampling-artifact case and keeps the scattered-replacement binary case guarded.
alt-glitch
added
type/bug
Collaborator
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #81480
read_filesamples file contents viahead -c 1000and decodes witherrors="replace". When a multibyte UTF-8 character straddles the 1000-byte sample boundary, the truncation produces a single trailing U+FFFD replacement character, which_is_likely_binarythen treats as a binary-content signal — so valid UTF-8 text files are flaggedisBinary: true.This fixes the whole class, not just the one site: a lone trailing U+FFFD is a sampling artifact and no longer implies binary; multiple U+FFFDs, or a trailing U+FFFD accompanied by other garbage, are still treated as binary.
Changes
tools/file_operations.py::_is_likely_binary: only treat U+FFFD as a binary signal when it appears more than once or is not the trailing character.tests/tools/test_file_operations_edge_cases.py::TestIsLikelyBinary: added coverage for the three cases (single trailing artifact → not binary; multiple scattered replacements → binary; trailing replacement + other garbage → binary).Verification
tests/tools/test_file_operations_edge_cases.py— 24 passed.