Skip to content

fix(read): stop misclassifying UTF-8 text files as binary - #79641

Closed
enedelko wants to merge 1 commit into
NousResearch:mainfrom
enedelko:fix/read-file-utf8-binary-detection
Closed

fix(read): stop misclassifying UTF-8 text files as binary#79641
enedelko wants to merge 1 commit into
NousResearch:mainfrom
enedelko:fix/read-file-utf8-binary-detection

Conversation

@enedelko

@enedelko enedelko commented Aug 5, 2026

Copy link
Copy Markdown

Problem

read_file samples the first 1000 bytes with head -c 1000, which cuts the file at an arbitrary byte boundary. When byte 1000 lands mid-way through a multi-byte UTF-8 character (2 bytes for Cyrillic, 3-4 for other scripts/emoji), the truncated tail decodes to U+FFFD (replacement char).

The binary detector in _is_likely_binary treats U+FFFD as mojibake (correctly, for real corruption) and flags the file as binary, refusing to read it. Result: legitimate UTF-8 text files (e.g. Russian markdown) are randomly blocked roughly half the time.

Fix

Read the sample with python instead of head -c 1000: same first 1000 bytes, decoded with errors="ignore" so a split multi-byte char is dropped instead of becoming U+FFFD. Real binary files are still caught by the non-printable ratio check in _is_likely_binary. Both call sites patched; sys import added.

read_file sampled the first 1000 bytes with `head -c 1000`, which can cut
mid-way through a multi-byte UTF-8 character. The truncated byte sequence
decoded to U+FFFD (replacement char), and the binary detector — correctly
treating U+FFFD as mojibake — flagged the file as binary and refused to
read it. Legitimate Cyrillic/UTF-8 text files were blocked roughly half
the time depending on where byte 1000 landed.

Sample with python instead: read the first 1000 bytes, decode with
errors='ignore' so a split multi-byte char is dropped instead of becoming
U+FFFD. Real binary files are still caught by the >30% non-printable
ratio check in _is_likely_binary.
@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets tool/file File tools (read, write, patch, search) P2 Medium — degraded but workaround exists labels Aug 5, 2026
@spfcraze

spfcraze commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary:
The errors='ignore' sample decode removes the U+FFFD signal _is_likely_binary uses to catch undecodable content, so binary files without a recognized extension pass the content check and are read as lossy text with is_binary=False.

Problems:

  • _is_likely_binary's docstring says the terminal decodes stdout with errors="replace", so any non-UTF-8 byte arrives as U+FFFD, and the "�" in content_sample branch is what makes undecodable files read-only. The python sample decode now drops those bytes instead of replacing them, so the sample reaching the check is already filtered.
  • The non-printable ratio then measures the residue: a high-bit binary file's residue is mostly printable ASCII (control ratio far below the 0.30 threshold at tools/file_operations.py:910), and a fully undecodable sample decodes to an empty string, which the if content_sample: guard at line 896 treats as not binary. So the body's claim that 'Real binary files are still caught by the non-printable ratio check' does not hold for the class that previously relied on the U+FFFD branch — raw blobs, latin-1 text, and PNG-like data without a known extension all returned is_binary=True before this change.
  • read_file then falls through to the sed read (tools/file_operations.py:1205) and returns the lossy content, so a read→edit→write round-trip can silently overwrite the original bytes — the corruption the docstring says the U+FFFD check exists to prevent.

Solution:
Only the boundary cut is an artifact: a split multi-byte char produces exactly one U+FFFD at the end of the 1000-byte window, while genuine undecodable content produces replacement chars anywhere in it. Decoding the sample with 'replace' and ignoring a trailing U+FFFD when the file is longer than 1000 bytes keeps the Cyrillic fix without weakening the guard; extending the window to the next character boundary works the same way.


Checked against 7a5794a — the PR head when this was written — and d1f9e77, main at the same moment.

@luntion

luntion commented Aug 6, 2026

Copy link
Copy Markdown

独立复现 + 修正版实现,见 #80261

复现(中文 UTF-8 markdown,Windows):文件前 1000 字节恰好切在 3 字节中文字符中间时(例如 998 个 ASCII 字节 + 一个中文),head -c 1000 的样本解码后尾部出现 U+FFFD,_is_likely_binary 把整个文件判为二进制,read_file 拒绝读取。同目录其他文件边界干净就正常——这是一个约 50% 概率的字节位置彩票,不是内容问题。

关于 errors='ignore' 方案的顾虑(与 triage 评论一致):ignore 会把 U+FFFD 信号整个丢掉,无扩展名的真实二进制/latin-1 文本会逃过检测,read→edit→write 往返可能静默损坏原始字节——这正是 _is_likely_binary 里 U+FFFD 分支存在的意义。

修正版方案(#80261:采样时把窗口扩展到下一个 UTF-8 字符边界(跳过延续字节),样本永远是完整字符;真实损坏字节仍产生 U+FFFD 并被捕获。另修复了原实现里 sys.executable 未转义的问题(Windows 路径含空格会炸)。已带 4 个回归测试 + 全量验证(63 passed,8 个预存在失败与本改动无关,clean main 上同样失败)。

@teknium1

teknium1 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

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.

@teknium1 teknium1 closed this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants