From 19ef468fae57f03977288901322706471cdde3e3 Mon Sep 17 00:00:00 2001 From: "Lyra (Xingyu)" Date: Sat, 8 Aug 2026 22:50:35 +0800 Subject: [PATCH] fix: stop misclassifying UTF-8 files cut at the 1000-byte sample boundary as binary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit read_file samples the first 1000 bytes with head -c 1000 and decodes with errors=replace. When byte 1000 lands mid-character in a multi-byte UTF-8 sequence, the truncated tail decodes to a single trailing U+FFFD, and _is_likely_binary() treated any U+FFFD as binary evidence — so perfectly valid text files were refused with 'Binary file - cannot display as text'. Distinguish the truncation artifact from real undecodable bytes: a true binary sample carries U+FFFD in the middle of the sample and/or more than once, whereas a boundary cut produces at most one replacement char squeezed against the tail (UTF-8 sequences are ≤4 bytes, so a cut loses ≤3 bytes). A lone trailing U+FFFD now falls through to the existing non-printable ratio check, which still catches NUL-heavy binary content. Adds regression tests covering the artifact case, mid-sample U+FFFD, multiple U+FFFDs, and the ratio check still firing with a trailing artifact char. --- .../tools/test_file_operations_edge_cases.py | 31 +++++++++++++++++++ tools/file_operations.py | 23 +++++++++++--- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/tests/tools/test_file_operations_edge_cases.py b/tests/tools/test_file_operations_edge_cases.py index 6a1898cc8254e..f82d3ec0e8de9 100644 --- a/tests/tools/test_file_operations_edge_cases.py +++ b/tests/tools/test_file_operations_edge_cases.py @@ -51,6 +51,37 @@ def test_content_sample_longer_than_1000(self, ops): sample = "\x00" * 200 + "a" * 800 + "\x00" * 1000 assert ops._is_likely_binary("file.xyz", content_sample=sample) is False + def test_truncation_artifact_trailing_replacement_char(self, ops): + """A lone U+FFFD at the sample tail is a head -c 1000 truncation + artifact (multi-byte UTF-8 char sliced at the boundary), not binary + evidence — legitimate text must not be refused as 'Binary file'.""" + # Valid UTF-8 file whose 1000th byte lands mid-character: the sample + # is 999 ASCII + one replacement char squeezed against the tail. + sample = "a" * 999 + "\ufffd" + assert ops._is_likely_binary("file.md", content_sample=sample) is False + # A few printable chars after the boundary-cut char (e.g. second + # sample pass or line-joined output) must also stay text. + sample2 = "a" * 998 + "\ufffd" + "b" + assert ops._is_likely_binary("file.md", content_sample=sample2) is False + + def test_replacement_char_in_middle_is_binary(self, ops): + """U+FFFD away from the tail = real undecodable bytes = binary.""" + sample = "a" * 100 + "\ufffd" + "b" * 899 + assert ops._is_likely_binary("file.xyz", content_sample=sample) is True + + def test_multiple_replacement_chars_is_binary(self, ops): + """Two U+FFFDs anywhere (even near the tail) = binary: valid UTF-8 + cut at one boundary produces at most a single replacement char.""" + sample = "a" * 998 + "\ufffd" + "\ufffd" + assert ops._is_likely_binary("file.xyz", content_sample=sample) is True + + def test_trailing_replacement_char_with_binary_ratio(self, ops): + """A trailing U+FFFD does NOT mask real binary content: the + non-printable ratio check still catches NUL-heavy samples.""" + # 301 NULs + 697 'a' + trailing artifact char: ratio > 30% → binary + sample = "\x00" * 301 + "a" * 697 + "\ufffd" + assert ops._is_likely_binary("file.xyz", content_sample=sample) is True + # ========================================================================= # _check_lint edge cases diff --git a/tools/file_operations.py b/tools/file_operations.py index ab4965ea14d83..2e4fc4dea58f4 100644 --- a/tools/file_operations.py +++ b/tools/file_operations.py @@ -901,13 +901,26 @@ def _is_likely_binary(self, path: str, content_sample: str = None) -> bool: # lossy text would let a read→edit→write round-trip silently # overwrite the original bytes with mojibake. Treat a file whose # sample carries the replacement char as binary (read-only) so the - # agent can't corrupt it. Legitimate UTF-8 text effectively never - # contains U+FFFD. - if "\ufffd" in content_sample[:1000]: + # agent can't corrupt it. + # + # Caveat: `head -c 1000` can slice a multi-byte UTF-8 character + # at the sample boundary, and the truncated tail decodes to a + # single trailing U+FFFD even for perfectly valid text. That + # truncation artifact must NOT be treated as binary evidence — + # otherwise legitimate UTF-8 files whose 1000th byte lands mid- + # character are refused with "Binary file". Distinguish it from + # real undecodable bytes: a true binary sample carries U+FFFD in + # the middle of the sample and/or more than once, not a lone + # replacement char squeezed against the tail. + sample = content_sample[:1000] + fffd_count = sample.count("\ufffd") + tail_start = max(0, len(sample) - 3) # UTF-8 seq ≤4 bytes → cut loses ≤3 + trailing_only = fffd_count == 1 and sample.rfind("\ufffd") >= tail_start + if fffd_count and not trailing_only: return True - non_printable = sum(1 for c in content_sample[:1000] + non_printable = sum(1 for c in sample if ord(c) < 32 and c not in '\n\r\t') - return non_printable / min(len(content_sample), 1000) > 0.30 + return non_printable / min(len(sample), 1000) > 0.30 return False