fix(file): utf-8 detection across multibyte cut (#76886)

[webtecnica]

Closes #76886

Summary

read_file / read_file_raw classify valid UTF-8 text as binary when the 1000-byte sample boundary (head -c 1000) cuts a multibyte character in half. The terminal env decodes stdout with errors="replace", so the truncated sequence arrives as a synthetic U+FFFD the file never contained, and the U+FFFD check in _is_likely_binary (added in 0.19.1 to guard against mojibake round-trips) rejects the file as binary.

Regression from 0.19.0 → 0.19.1; affects any UTF-8 file with a multibyte char starting near byte 1000 (Turkish notes with ç/ğ/ı/ö/ş/ü, CJK files, emoji, …).

Root cause

• _is_likely_binary treats any U+FFFD in the lossy-decoded sample as real file content.
• But a U+FFFD produced by a byte-truncated sample is a decode artifact, indistinguishable from real content in the already-lossy string.
• Real non-UTF-8 bytes (the mojibake case the guard exists for) and a cut multibyte char produce the same U+FFFD in the sample.

Fix (class-level, in _is_likely_binary)

A synthetic artifact has a distinctive signature: it is always the last character of the sample and is the sample's only U+FFFD (a cut sequence decodes to exactly one). When that pattern matches, re-read the raw bytes without loss (head -c N … | od -An -v -tx1 — plain ASCII hex survives the lossy decode) and strict-decode slightly larger windows:

• valid UTF-8 decodes cleanly once the window crosses onto a character edge → the U+FFFD was fabricated → text (fall through to the non-printable ratio check)
• a file that keeps failing really does contain non-UTF-8 bytes → binary (mojibake round-trip guard preserved)

Both read_file and read_file_raw call _is_likely_binary, so the fix covers both sampling paths. The guard still triggers for genuine latin-1/other-encoding content, including when an invalid byte lands exactly on the sample boundary.

Reproduction (no deps)

printf 'a%.0s' $(seq 999) > fails.md && printf '\303\247\nx\n' >> fails.md   # 'ç' starts at byte 1000
printf 'a%.0s' $(seq 998) > ok.md    && printf '\303\247\nx\n' >> ok.md      # 'ç' fits inside the sample

Before: read_file fails.md → is_binary: true. After: reads normally. Same content, same encoding, one byte of offset.

Tests

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

• issue repro: valid UTF-8, 'ç' at byte 1000 → not binary (was binary)
• genuine latin-1 byte at the sample boundary → still binary (guard intact)
• end-to-end read_file and read_file_raw on the repro file → content read, is_binary: false

Verified: 70/70 tests in test_file_operations.py + test_file_operations_edge_cases.py pass; full tests/tools/ run shows no new failures vs. clean main (pre-existing env-dependent failures unchanged).

[teknium1]

Thanks for preserving the non-UTF-8 read/edit/write safety goal while addressing #76886.

Problems

• tools/file_operations.py:945 retries only six whole-prefix strict decodes. A valid UTF-8 file can place a multibyte character at each selected endpoint (1000, 1004, 1008, 1016, 1032, and 1064); each prefix then raises unexpected end of data, so _sample_ufffd_is_truncation_artifact() returns False even though the complete file is valid UTF-8. This was verified with a valid constructed byte stream against the exact probe lengths.

Suggested changes

• Decode the bounded raw extension with a strict incremental decoder rather than requiring a complete character boundary at one fixed endpoint; only finalize a pending sequence when the raw read reached EOF. Add coverage for repeated boundary-straddling multibyte characters.

This is an automated hermes-sweeper review.

[teknium1]

These fixed endpoints are not character-safe: a valid UTF-8 file can place another multibyte character across each of 1000, 1004, 1008, 1016, 1032, and 1064, making every whole-prefix strict decode fail with unexpected end of data. Use strict incremental decoding of a bounded extension (finalizing only at EOF) so a later boundary cut does not reintroduce the false binary classification.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address #76886's false binary classification when a 1000-byte sample splits a UTF-8 character: #76924 uses repeated raw-prefix decoding, #76925 exempts a trailing U+FFFD, and #76934 introduces a shared bounded sampler with raw-byte validation for both read paths. The current #76934 diff most directly addresses the reported cause while preserving fail-closed handling of malformed input; #76924 retains false positives at fixed retry boundaries, and #76925 can accept a genuinely invalid boundary byte.

Related pull requests

• fix(file): utf-8 detection across multibyte cut (#76886) #76924 best fix — (+132/-2) — partial, superseded by fix(tools): keep invalid UTF-8 boundaries binary #76934: raw-byte probing fixes the original reproduction and preserves rejection of an invalid byte at byte 1000, but every fixed retry endpoint can itself split another valid multibyte character. Despite the keep_open review on fix(file): utf-8 detection across multibyte cut (#76886) #76924, its requested boundary-safe salvage is now represented more completely by fix(tools): keep invalid UTF-8 boundaries binary #76934's shared sampler and raw-byte-validated extended-boundary tests, so fix(file): utf-8 detection across multibyte cut (#76886) #76924 can close as a duplicate of fix(tools): keep invalid UTF-8 boundaries binary #76934.
• fix(tools): don't flag boundary-cut UTF-8 as binary in read_file (#76886) #76925 duplicate — (+72/-1) — partial, superseded by fix(tools): keep invalid UTF-8 boundaries binary #76934: ignoring a lone trailing U+FFFD fixes the reported valid-UTF-8 case but cannot distinguish it from an actual invalid byte at byte 1000, weakening the existing corruption guard. Despite the keep_open review on fix(tools): don't flag boundary-cut UTF-8 as binary in read_file (#76886) #76925, the diff's blocker is resolved by fix(tools): keep invalid UTF-8 boundaries binary #76934's raw-byte proof rather than this unconditional exemption, so fix(tools): don't flag boundary-cut UTF-8 as binary in read_file (#76886) #76925 can close as a duplicate of fix(tools): keep invalid UTF-8 boundaries binary #76934.
• fix(tools): keep invalid UTF-8 boundaries binary #76934 duplicate — (+169/-6) — strongest salvage candidate: it centralizes sampling for read_file and read_file_raw, extends only on the boundary signal, and removes a trailing replacement only when bounded raw bytes prove an incomplete otherwise-valid UTF-8 sequence. The visible keep_open review's blocking byte-1004 case is explicitly addressed in the current diff by retaining the original fail-closed sample when proof fails and by adding read_file/read_file_raw regressions for invalid and valid extended-boundary cases; an independent native-Windows run reports all nine relevant regressions passing, including both read paths and fail-closed invalid-boundary cases, so keep it open pending refreshed contributor review.

Duplicates

#76924, #76925, and #76934 are competing implementations for the same defect in #76886. #76924 and #76925 can close as duplicates of #76934 because #76934 incorporates the safer raw-byte distinction, a shared sampler for both read paths, and the relevant original- and extended-boundary regressions.

Suggested consolidation

Keep #76934 open with a salvage path: request a refreshed contributor review against the current diff, specifically confirming that _sample_ends_with_incomplete_utf8 preserves fail-closed behavior for invalid bytes at the original and extended boundaries while accepting valid repeated boundary splits. The reported 9/9 native-Windows verification independently supports that matrix but does not replace the requested contributor review; after that review, close #76924 and #76925 as duplicates of #76934.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I76886(["issue #76886 (open)"])
    subgraph Dup76924 ["PRs duplicating each other"]
        P76924["PR #76924 (open)"]
        P76925["PR #76925 (open)"]
        P76934["PR #76934 (open)"]
    end
    P76924 -->|best fix| I76886
    class I76886 open
    class P76924 open
    class P76925 open
    class P76934 open
    class P76924 best
    class P76934 best
    class P76924 target
    click I76886 "https://github.com/NousResearch/hermes-agent/issues/76886"
    click P76924 "https://github.com/NousResearch/hermes-agent/pull/76924"
    click P76925 "https://github.com/NousResearch/hermes-agent/pull/76925"
    click P76934 "https://github.com/NousResearch/hermes-agent/pull/76934"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 24 kB of PR diffs, 13 kB of issue/PR text, 10 kB of discussion (9 comments), 6 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[teknium1]

Closing with credit: you were the FIRST to correctly diagnose and fix this bug (Aug 2, 16:35Z) — the earliest of ~24 submissions. The merged fix (#81961, from #80440) was chosen because its byte-layer detection covers both file-ops sites with one detection contract, where the od-hex re-verify here was read_file-only and re-probed per read. Your diagnosis was right on the money — thank you.

[webtecnica]

Thanks @teknium1 for the credit — glad the diagnosis held up as the earliest of the ~24. The byte-layer detection in #81961 covering both file-ops sites is the right contract, and the truncated-CJK/BOM/UTF-16/NUL regressions make it solid. Appreciate the recognition!