Skip to content

fix(grep): detect encoding from a character-safe peek - #8321

Open
tancheng33 wants to merge 1 commit into
esengine:main-v2from
tancheng33:fix/grep-utf8-peek-boundary
Open

fix(grep): detect encoding from a character-safe peek#8321
tancheng33 wants to merge 1 commit into
esengine:main-v2from
tancheng33:fix/grep-utf8-peek-boundary

Conversation

@tancheng33

Copy link
Copy Markdown

Summary

  • grep classifies a file from its first 8 KiB peek, and in CJK text that window usually stops mid-character: most Chinese characters are three bytes wide, so roughly two of three cut points land inside one. Detect reads "not valid UTF-8" as evidence of a legacy charset, and the GB18030 decoder accepts nearly any byte string, so a plain UTF-8 file was decoded as GB18030. Chinese patterns then never matched, and ASCII patterns matched but printed their lines as mojibake.
  • Adds encoding.DetectPrefix(window, truncated), which trims a truncated window to a character boundary before classifying it, and routes grep's peek and read_file's detection sample through it.
  • read_file already trimmed its sample to the last newline for this reason. DetectPrefix subsumes that and also covers a sample with no newline in it, such as a single-line CJK export or a minified bundle.

This also accounts for the ASCII-ratio correlation in the report: the more ASCII a file contains, the likelier the 8192-byte boundary lands on a character boundary and the file happens to be classified correctly. It is a probability, not a threshold, which is why the reporter's 15.4% file was ambiguous while 18.7% and above looked fine.

Issues

Fixes #8299

Verification

  • New tests, each confirmed to fail with the trim disabled and to pass with it:
    • TestE2EGrepUTF8CJKLargerThanPeek: grep matches a Chinese pattern in a UTF-8 file larger than the peek, and an ASCII match keeps its surrounding Chinese readable.
    • TestE2EGrepGBKLargerThanPeek: a real GB18030 file over the peek still decodes, guarding against over-trimming.
    • TestE2EReadFileSingleLineCJKLargerThanSample: read_file handles a single-line CJK file larger than its 256 KiB detection sample.
    • TestDetectPrefixTrimsTruncatedCJKWindow, TestDetectPrefixKeepsGB18030Detectable, TestTrimPartialRuneDropsOnlyTruncatedTail.
  • The fixtures assert that the detection boundary actually splits a character before running the tool, so they cannot pass by luck.
  • gofmt -l . clean, go vet ./... clean, make lint clean (golangci-lint v2.12.2 reports 0 issues; repolint clean against the baseline, which is unchanged).
  • go test ./internal/tool/... ./internal/boot/ ./internal/fileutil/... ./internal/checkpoint/ ./internal/config/ passes.

Documentation impact

Documentation-impact: none - detection is internal. docs/MIGRATING.md:200 already promises that grep "decodes before matching", which is the behavior this restores for UTF-8 CJK files.

Cache impact

Cache-impact: none - no tool schema, description, registration, or other provider-visible prefix byte changes. The only change is which local byte window is used for encoding detection.
Cache-guard: the provider-visible prefix is untouched, so existing tool-schema and boot guards cover it. Ran go test ./internal/tool/... ./internal/boot/.
System-prompt-review: N/A

grep classifies a file from its first 8 KiB, and that window usually stops
mid-character in CJK text: three-byte characters make it a two-in-three
chance. Detect reads "not valid UTF-8" as a legacy charset and the GB18030
decoder accepts nearly any byte string, so a plain UTF-8 file was read as
GB18030 — Chinese patterns never matched, and ASCII matches printed their
lines as mojibake.

Add encoding.DetectPrefix, which trims a truncated window to a character
boundary before classifying it, and use it for grep's peek and read_file's
detection sample. read_file already trimmed its sample to the last newline;
DetectPrefix subsumes that and also covers a sample with no newline in it,
such as a single-line CJK export.
@github-actions github-actions Bot added v2 Go rewrite (1.x) — main-v2 branch, active development skills Skill system (internal/skill, internal/tool) labels Aug 11, 2026

Copy link
Copy Markdown

I independently arrived at #8322 while validating #8299. I cross-ran this PR's head with the two inverse regression cases from #8322—an ASCII-only header followed by a long GB18030 line, for both grep and read_file—and both passed on this head without code changes. Since the implementations overlap, I do not plan to stack the full PR; if the extra coverage would be useful, I can send those two tests as a small PR against this head branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skills Skill system (internal/skill, internal/tool) v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 内置grep不能正确处理部分中西混合文件编码问题

2 participants