Skip to content

fix: stop misclassifying multibyte UTF-8 text files as binary in read_file - #80250

Closed
0809android wants to merge 1 commit into
NousResearch:mainfrom
0809android:fix/read-file-utf8-binary-misdetection
Closed

fix: stop misclassifying multibyte UTF-8 text files as binary in read_file#80250
0809android wants to merge 1 commit into
NousResearch:mainfrom
0809android:fix/read-file-utf8-binary-misdetection

Conversation

@0809android

Copy link
Copy Markdown

Summary

read_file (and read_file_raw) classify valid UTF-8 text files as binary when the 1000-byte sampling boundary (head -c 1000) cuts through a multibyte character. Japanese, Chinese, Korean, and emoji-heavy text files are affected frequently (3-byte UTF-8 chars make a mid-character cut highly likely).

Reproduction

Any UTF-8 text file whose byte 1000 lands inside a multibyte char, e.g. a Japanese markdown file > 1000 bytes. read_file returns:

Binary file - cannot display as text. Use appropriate tools to handle this file type.

Verified against a real 10 KB Japanese file: head -c 1000 cut "自" (\xe5\xbe\xaa) after its 2nd byte, the incremental decoder (errors="replace") produced one trailing U+FFFD at sample position 386, and _is_likely_binary returned True.

Root cause

  1. read_file samples the first 1000 bytes (head -c 1000) for binary detection.
  2. The terminal backend decodes child output with codecs.getincrementaldecoder("utf-8")(errors="replace") (tools/environments/base.py), so the dangling bytes of a cut multibyte char arrive as U+FFFD at the end of the sample.
  3. _is_likely_binary treats any U+FFFD in the sample as evidence of mojibake → binary. That guard exists to prevent read→edit→write corruption of genuinely non-UTF-8 files, but it does not distinguish a truncation artifact (1–3 trailing U+FFFD) from genuine mojibake (U+FFFD throughout).

Fix

In _is_likely_binary, strip a trailing run of ≤3 U+FFFD from the sample before judging. A 4-byte UTF-8 char split across the cut yields at most 3 dangling bytes, so a longer trailing run (≥4) still counts as binary (safe direction). Genuine mojibake (U+FFFD in the middle of the sample) is still flagged binary, preserving the corruption guard.

sample = content_sample[:1000]
stripped = sample.rstrip("\ufffd")
if len(sample) - len(stripped) <= 3:
    sample = stripped
if "\ufffd" in sample:
    return True
non_printable = sum(1 for c in sample if ord(c) < 32 and c not in '\n\r\t')
return non_printable / min(len(sample), 1000) > 0.30

Tests

Added 3 regression tests to tests/tools/test_file_operations.py (TestReadNonUtf8IsBinary):

  • trailing U+FFFD (cut artifact) → text (not binary)
  • mid-sample U+FFFD (genuine mojibake) → binary (guard preserved)
  • trailing run of 5 → binary / trailing run of 3 → text (safe direction)

Full file: 49 passed.

read_file samples the first 1000 bytes (head -c 1000) for binary
detection. When the byte boundary cuts through a multibyte UTF-8
character, the terminal backend's incremental decoder (errors=replace)
turns the dangling bytes into U+FFFD at the END of the sample.
_is_likely_binary then treats any U+FFFD as mojibake evidence and
classifies the file as binary — so valid Japanese/Chinese/Korean text
files frequently cannot be read.

Strip a trailing run of <=3 U+FFFD (a 4-byte char yields at most 3
dangling bytes) before judging. Genuine mojibake (U+FFFD throughout
the sample) is still flagged binary, preserving the read-only guard.
@0809android

Copy link
Copy Markdown
Author

Fixes #80251

@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 needs-decision Awaiting maintainer decision before any implementation sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 6, 2026

@monerostar monerostar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ubuntu 26.04 on linux-5800x (kernel 7.0.0-28-generic).

Live ShellFileOperations.read_file on a 1200-byte Japanese hiragana file ( x 400). Byte 1000 lands mid-character so head -c 1000 yields a trailing U+FFFD in the sample.

fixture main this PR
jp.txt (multibyte cut) is_binary=True is_binary=False, content_len=402
ascii.txt text OK text OK
nul.bin binary binary
bytes(range(256))*4 binary binary

Also: _is_likely_binary on that sample is True on main, False on PR. tests/tools/test_file_operations.py 49 passed here.

Looks good. The trailing ≤3 FFFD carve-out matches the real head -c cut; real binary still blocked.

@kyssta-exe

Copy link
Copy Markdown
Contributor

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, #80261. Recommend picking one to champion and closing the rest as duplicates.

teknium1 pushed a commit that referenced this pull request Aug 8, 2026
…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.
@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
ma1138569845 pushed a commit to ma1138569845/dechnicAuditor-agent that referenced this pull request Aug 10, 2026
…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.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…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.
blut-agent pushed a commit to blut-agent/hermes-agent-fork that referenced this pull request Aug 11, 2026
…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.
33hodl pushed a commit to 33hodl/hermes-agent that referenced this pull request Aug 12, 2026
…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.
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 needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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