mirror of https://github.com/alibaba/open-code-review.git synced 2026-08-21 22:54:39 +00:00

chore(ci): fail CI when unapproved non-English text appears in source files (#876 )

* fix(prompt): replace the fullwidth colon in the file_read tool description

tools.json advertised the example output as "File：path/to/example.go" with
a fullwidth colon (U+FF1A), while file_read.go actually emits "File: %s".
The description is sent to the model on every review, so the example did
not match the output it was describing.

Also switches action.yml's OCR_LANGUAGE example from 中文 to Chinese, for
the same reason as #861: the value is fed to the LLM and Chinese is what
the rest of the project uses.

* chore(ci): fail CI when CJK characters appear in source files

Comments, identifiers and strings in this repository are meant to be
English, but nothing enforced it — #861 had to clean up leftovers by hand,
and the same drift keeps arriving through generated code and contributions
written internally.

scripts/verify-cjk.go walks the index plus untracked files and reports Han
ideographs, kana, CJK punctuation and fullwidth forms. Written in Go rather
than shell so it does not depend on the container's grep having PCRE, and so
`unicode.Is` decides what counts as CJK instead of a byte range that would
flag the em dashes used throughout the comments. `//go:build ignore` keeps
it out of ./..., so it does not affect go vet, go build or the coverage
threshold.

Untracked files are included (--others --exclude-standard) so a new file is
checked before it lands: while writing this, the script's own comment used
Chinese punctuation as an example and went unreported until it was staged.

Two escape hatches, preferring the narrow one: an `allow-cjk: <reason>`
marker comment on a single line, or a prefix in allowedPrefixes for a whole
tree. 23 existing lines get markers (UTF-8 encoding fixtures, multibyte
truncation fixtures, language-switcher labels, the fullwidth bar used as a
terminal cursor). pages/src/i18n/ is allowlisted as translated UI copy;
extensions/vscode/ is allowlisted TEMPORARILY — its comments, test names
and zh-cn NLS bundle are still Chinese and need a follow-up pass.

Wired into CI next to the license and action-pin checks, plus
`make cjk-check` and `make check` for local runs.

* chore(ci): generalise the CJK check to all non-English text

Addresses the review feedback, and widens the rule that the feedback
exposed.

Review feedback:

- exemptMarker requires its colon, so a bare "allow-cjk" can no longer
  exempt a line without giving a reason.
- The script is named for CJK but missed Hangul.
- git ls-files gains -z, so paths that are not plain ASCII arrive
  unquoted, and its stderr is reported rather than a bare exit status.
- main discarded run()'s error entirely and only called os.Exit(1),
  which is what made the lost stderr invisible in the first place.
- The CI step and AGENTS.md say "unapproved", since escape hatches exist.

The check was skewed by writing system rather than by language. In one
array the 'zh' and 'ja' labels each needed a marker while the adjacent
'ru' label passed untouched, and nine lines of Russian sat in the tree
unflagged: two language-switcher labels and the heading-ID fixtures.
Contributors writing Chinese had to justify every line; contributors
writing Russian had nothing to justify.

The rule is now "a letter outside ASCII", since written English needs no
letter beyond the ASCII 26 -- Cyrillic and Han as much as the diacritics
that spell German or Vietnamese. Scripts are not enumerated, so one
nobody has contributed in yet is covered when it arrives. Common and
Inherited pass, so letterlike symbols (U+2139, U+2113) are not mistaken
for prose, and combining accents are caught, so the decomposed spelling
of an accented letter cannot slip through. Symbols and emoji stay out of
scope by construction: they are not letters.

Renamed to scripts/verify-english-only.go and make english-check, and
the marker to allow-non-english:. Text spelled entirely in ASCII still
takes a dictionary to identify and stays a matter for review.

* docs(agents): restate the English-only rule as rule, homes, hatches

The rule was one dense bullet that led with the detection mechanism and
mentioned the exemptions only in passing, which is the wrong order for
the reader: an agent needs to know where a translation may go before it
needs to know which Unicode scripts are flagged. Split into three.

The homes are now spelled out from what the tree actually holds, rather
than left as "<locale> docs or an i18n table": README and CONTRIBUTING
in zh-CN, ja-JP, ko-KR and ru-RU; the doc pages under
pages/src/content/docs/ in en, zh, ja and ru; the UI copy tables in
pages/src/i18n/. Also why the two are exempt for different reasons --
Markdown by extension, the i18n tables by prefix because they are .ts --
since that decides where a new translation can safely go.

Drops the enumerated list of what "make check" runs. It duplicated the
Makefile, went stale the moment a check was added (this PR had to edit
it), and told an agent nothing it would not read in the output anyway.
What is worth saying is that the target writes to the tree.

* fix(ci): detect U+FE10–FE6F CJK punctuation in english-only check

The vertical forms (U+FE10–FE19), CJK compatibility forms (U+FE30–FE4F)
and small form variants (U+FE50–FE6F) were not caught, even though their
fullwidth counterparts (U+FF00–FFEF) already were. A small question mark
(U+FE56 ﹖) or vertical comma (U+FE10 ︐) left in source reads as correct
English punctuation and is invisible in review — the same class of typo
the fullwidth range already defends against.

Skip U+FE20–FE2F (Combining Half Marks) which are used in Latin text.

2026-08-13 14:43:55 +08:00

3.2 KiB

Raw Permalink Blame History

Agent Guidelines for open-code-review

This file provides instructions for AI coding assistants working on this project.

Project Overview

open-code-review (ocr) is an AI-powered code review CLI tool written in Go (module: github.com/alibaba/open-code-review).

Git Commit Notes

Before committing, conduct a code review by running:

ocr review --audience agent --background "briefly summarize the background requirements"

Commit messages must be written in English.
Verify line endings. Line endings must be LF, not CRLF. Run git add --renormalize . to correct line endings and commit them. New binary files must have their extensions added to .gitattributes.

License Headers

Every source file (.go, .sh, .js, .mjs, .ts, .tsx) must have an SPDX license header.
After creating new files, run make license-add to add the header automatically.

Code Style

After writing code, run make check. It formats and tidies in place, so there is no need to run gofmt or go vet separately.
Source files are written in English — comments, identifiers and strings alike. make english-check enforces this in CI. It flags any letter outside ASCII, whichever the writing system (Han, kana, Hangul, Cyrillic, and equally the diacritics that spell German or Vietnamese), plus combining accents and fullwidth punctuation (：, （), which is easy to leave behind in an otherwise English sentence. Symbols and emoji (─ → ≥ ✅) pass, since they are not letters. Prose spelled entirely in ASCII (Loeschen der Datei, or a romanised transcription) takes a dictionary to spot and stays a matter for review.
Translated prose has its own homes, none of them scanned. README.<locale>.md and CONTRIBUTING.<locale>.md (zh-CN, ja-JP, ko-KR, ru-RU); the doc pages under pages/src/content/docs/<locale>/ (en, zh, ja, ru, Markdown throughout); and the UI copy tables in pages/src/i18n/<locale>.ts. Markdown is out of scope by extension, so translations go there freely. The i18n tables are .ts and would be scanned, so they are exempt by prefix instead — translated UI strings belong in those tables rather than inline in a component.
Two escape hatches for the exceptional case, narrower one preferred. Append an allow-non-english: <reason> marker comment to the offending line — the right choice for a handful of lines, such as an encoding fixture or a language-switcher label, and it leaves the rest of the file protected. Only for a whole tree that is inherently non-English, add a prefix to allowedPrefixes in scripts/verify-english-only.go; it currently holds just pages/src/i18n/ and extensions/vscode/, the latter temporary until the extension's Chinese comments are translated.

Testing

Run unit tests with make test, not go test directly.
make test sets LC_ALL=C to ensure git outputs English messages.
When writing or modifying code, add necessary unit tests to maintain coverage. The project enforces a 90% coverage threshold via make coverage.

README

When modifying README.md, always sync the changes to all localized versions:
- README.zh-CN.md
- README.ja-JP.md
- README.ko-KR.md
- README.ru-RU.md

3.2 KiB Raw Permalink Blame History Unescape Escape