alibaba / alibaba/open-code-review
Handle non-UTF-8 source files without corrupting review comments
- Dominant language
- Go
- Stars
- 24.4k
- Forks
- 1.8k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 105
Description
### Problem Statement
## Summary
Open Code Review currently assumes that source files and Git diff output are UTF-8. Repositories may contain files encoded in GBK, GB18030, Big5, Shift-JIS, or other legacy encodings, sometimes without any predictable path or extension pattern.
When such content is sent to the LLM, invalid UTF-8 bytes may be replaced or corrupted, causing Chinese or other non-ASCII text to appear as mojibake or replacement characters (`�`) in review comments.
## Current behavior
Several input paths currently convert raw bytes directly to Go strings without validating or decoding the source encoding:
- Full-file scan reads files with `os.ReadFile` and converts them using `string(content)`.
- Diff review consumes raw `git diff` and `git show` output.
- The `file_read` tool returns file content without encoding conversion.
When these strings are serialized into a JSON request, invalid UTF-8 content may be replaced before it reaches the model.
This also affects comment positioning:
1. The model sees corrupted or converted content.
2. The model returns `existing_code` as UTF-8.
3. The line resolver attempts to match it against the original undecoded diff or file content.
4. The match may fail, causing missing or incorrect line locations.
## Steps to reproduce
1. Add a source file containing Chinese text and save it as GBK or GB18030.
2. Commit the file or include it in a full-file scan.
3. Run either:
```bash
ocr scan
```
or:
```bash
ocr review --from --to
```
4. Observe that the source content or generated review comment contains mojibake or `�`.
5. Comments whose `existing_code` contains non-ASCII text may also fail line-number resolution.
## Expected behavior
- OCR should safely review repositories containing files with different text encodings.
- Content sent to the LLM should always be valid UTF-8.
- `existing_code` should be matched against the same decoded UTF-8 content shown to the model.
- Decoding must not modify files in the working tree.
- UTF-8 repositories should retain their current behavior and performance as much as possible.
- If an encoding cannot be detected with sufficient confidence, OCR should warn and skip the file instead of silently replacing invalid bytes.
## Acceptance criteria
- [ ] Valid UTF-8 files are unchanged.
- [ ] A repository can contain UTF-8 and GBK/GB18030 files simultaneously.
- [ ] Chinese content from a GBK/GB18030 file is readable in the LLM request and generated comments.
- [ ] `existing_code` containing decoded Chinese text resolves to the correct line.
- [ ] Diff and full-file scan modes behave consistently.
- [ ] `file_read` returns valid UTF-8 content for supported legacy encodings.
- [ ] Line numbers remain unchanged after decoding.
- [ ] Low-confidence or unsupported encodings produce a clear warning and are skipped.
- [ ] Invalid bytes are not silently replaced with `�`.
- [ ] Unit tests cover mixed-encoding repositories and line-number resolution.
## Additional notes
Automatic character-set detection is inherently heuristic. The implementation should therefore expose the detected encoding and confidence in debug output and prefer an explicit warning or skip over silently delivering corrupted content to the model.
### Proposed Solution
## Proposed approach
Introduce a shared in-memory text decoding layer.
For each file:
1. Preserve valid UTF-8 content without modification.
2. Detect BOM-based encodings where applicable.
3. Detect the encoding of non-UTF-8 content on a per-file basis.
4. Decode the content into UTF-8 before it is used by the prompt, tools, or line resolver.
5. Use the same detected encoding for both:
- the per-file unified diff;
- the complete new-file content.
For unified diffs, only hunk payload lines should be decoded. Git metadata such as `diff --git`, file paths, and hunk headers should remain unchanged.
The decoding layer should be applied consistently to:
- `ocr review` diff content;
- `NewFileContent`;
- `ocr scan`;
- `file_read` and line-based file reads;
- other tool results that may contain raw source lines.
The existing line resolver should then continue matching UTF-8 `existing_code` against UTF-8 diff and file content without requiring encoding-specific logic.
### Alternatives Considered
_No response_
### Affected Area
Built-in Tools (file_read, code_search, etc.)
### Additional Context
_No response_
Contributor guide
Research direction
Start by tracing the ocr scan and ocr review entry points, then inspect file_read, full-file reads, diff handling, and the line resolver. Use the reproduction steps with mixed UTF-8 and GBK/GB18030 files, and add unit coverage for decoding, consistent diff and scan behavior, and existing_code line resolution. Done means supported content reaches the LLM as valid UTF-8, low-confidence cases warn and skip, and files are not modified.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- devtools
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100