Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ jobs:
- name: Verify action pins
run: bash scripts/verify-action-pins.sh

- name: Verify sources contain no unapproved non-English text
run: go run scripts/verify-english-only.go

- name: Check formatting
run: |
unformatted="$(gofmt -s -l .)"
Expand Down
6 changes: 4 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ open-code-review (`ocr`) is an AI-powered code review CLI tool written in Go (mo

## Code Style

- After writing code, run `make check` to format and check the code.
- `make check` runs: license check, `go mod tidy`, `gofmt -s -w .`, and `go vet`.
- 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

Expand Down
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
build-all dist sha256sum version-info \
build-linux-amd64 build-linux-arm64 build-darwin-amd64 build-darwin-arm64 \
build-windows-amd64 build-windows-arm64 \
license-check license-add
license-check license-add english-check

BINARY_NAME := opencodereview
GO := go
Expand Down Expand Up @@ -64,7 +64,7 @@ fmt:
vet:
LC_ALL=C $(GO) vet $(PACKAGES)

check: license-check
check: license-check english-check
$(GO) mod tidy
gofmt -s -w .
LC_ALL=C $(GO) vet $(PACKAGES)
Expand All @@ -73,6 +73,9 @@ check: license-check
license-check:
@bash scripts/verify-license.sh

english-check:
@$(GO) run scripts/verify-english-only.go

license-add:
@bash scripts/add-license.sh

Expand Down
2 changes: 1 addition & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ inputs:
language:
description: >-
Review output language, written via `ocr config set language`
(e.g. English, 中文). No env var exists for this.
(e.g. English, Chinese). No env var exists for this.
required: false
default: 'English'
llm_timeout:
Expand Down
2 changes: 1 addition & 1 deletion cmd/opencodereview/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func TestSanitizeTerminal(t *testing.T) {
{"strips carriage return", "fake\rreal", "fakereal"},
{"empty string", "", ""},
{"only control chars", "\x1b\x07\x00\x7f", ""},
{"unicode preserved", "代码审查 レビュー 🔍", "代码审查 レビュー 🔍"},
{"unicode preserved", "代码审查 レビュー 🔍", "代码审查 レビュー 🔍"}, // allow-non-english: fixture asserts non-ASCII output is preserved verbatim
{"mixed safe and unsafe", "path\x1b[0m/file.go", "path[0m/file.go"},
{"strips C1 CSI (U+009B)", "before\u009bafter", "beforeafter"},
{"strips C1 OSC (U+009D)", "before\u009dafter", "beforeafter"},
Expand Down
4 changes: 2 additions & 2 deletions cmd/opencodereview/session_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,11 @@ func TestRunSessionShow_MissingID(t *testing.T) {
}

func TestTruncateUnicode(t *testing.T) {
got := truncate("错误原因:超过限制", 6)
got := truncate("错误原因:超过限制", 6) // allow-non-english: fixture exercises rune-boundary truncation
if !strings.HasSuffix(got, "…") {
t.Fatalf("expected ellipsis suffix, got %q", got)
}
if !strings.Contains(got, "错误") {
if !strings.Contains(got, "错误") { // allow-non-english: fixture exercises rune-boundary truncation
t.Fatalf("expected valid truncated unicode text, got %q", got)
}
}
Expand Down
12 changes: 6 additions & 6 deletions examples/gerrit_ci/post_review_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,10 @@ def test_suggestion_without_existing(self):
self.assertNotIn("**Suggestion:**", msg)

def test_unicode_comment_roundtrip(self):
content = "空指针解引用:y 可能为 nil"
ri = build([comment(path="pkg/服务.go", content=content)])
self.assertIn("pkg/服务.go", ri["comments"])
self.assertIn(content, entry_of(ri, "pkg/服务.go")["message"])
content = "空指针解引用:y 可能为 nil" # allow-non-english: fixture exercises UTF-8 comment bodies
ri = build([comment(path="pkg/服务.go", content=content)]) # allow-non-english: fixture exercises UTF-8 file paths
self.assertIn("pkg/服务.go", ri["comments"]) # allow-non-english: fixture exercises UTF-8 file paths
self.assertIn(content, entry_of(ri, "pkg/服务.go")["message"]) # allow-non-english: fixture exercises UTF-8 file paths
self.assertEqual(json.loads(json.dumps(ri, ensure_ascii=False)), ri)

def test_path_with_spaces(self):
Expand Down Expand Up @@ -539,15 +539,15 @@ def fake_urlopen(req, timeout=None):
def test_preemptive_basic_auth_and_utf8_body(self):
import base64

req, _parsed = self.post({"message": "空指针解引用:y 可能为 nil"})
req, _parsed = self.post({"message": "空指针解引用:y 可能为 nil"}) # allow-non-english: fixture exercises UTF-8 request payloads
auth = req.get_header("Authorization")
self.assertIsNotNone(auth, "Authorization header must be set preemptively")
self.assertTrue(auth.startswith("Basic "))
self.assertEqual(
base64.b64decode(auth[len("Basic "):]).decode("utf-8"),
"review-bot:s3cret-pass",
)
self.assertIn("空指针解引用".encode("utf-8"), req.data)
self.assertIn("空指针解引用".encode("utf-8"), req.data) # allow-non-english: fixture exercises UTF-8 request payloads
self.assertIn("application/json", req.get_header("Content-type"))

def test_xssi_response_parses(self):
Expand Down
2 changes: 1 addition & 1 deletion internal/config/toolsconfig/tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
"main_task": true,
"definition": {
"name": "file_read",
"description": "Use this tool to read file content when you need to get context for git diff. You can specify start_line and end_line to view specific parts of the file.\n\n**Line Range Strategy:**\n- Git diff hunk header provides guidance on how to get more relevant context.\n- Git diff hunk header \"@@-x,y +m,n@@\" indicates that the old file has y lines starting from line x, and the new file has n lines starting from line m.\n- For example, when you need to read 50 lines above and below the current changed code block in the new file, set start_line = m - 50, end_line = m + n + 50.\n\n**Example output:**\nFile:path/to/example.go (Total lines: 50)\nIS_TRUNCATED: false\nLINE_RANGE: 10-12\n// The following is the original content of the file\nfunc main() {\n fmt.Println(\"Hello, World!\")\n}\n\n**Limitations:**\n- If the specified range exceeds 500 lines, only 500 lines will be returned with a truncation notice.\n- This tool can only read file content from the modified version (after changes) in git diff.",
"description": "Use this tool to read file content when you need to get context for git diff. You can specify start_line and end_line to view specific parts of the file.\n\n**Line Range Strategy:**\n- Git diff hunk header provides guidance on how to get more relevant context.\n- Git diff hunk header \"@@-x,y +m,n@@\" indicates that the old file has y lines starting from line x, and the new file has n lines starting from line m.\n- For example, when you need to read 50 lines above and below the current changed code block in the new file, set start_line = m - 50, end_line = m + n + 50.\n\n**Example output:**\nFile: path/to/example.go (Total lines: 50)\nIS_TRUNCATED: false\nLINE_RANGE: 10-12\n// The following is the original content of the file\nfunc main() {\n fmt.Println(\"Hello, World!\")\n}\n\n**Limitations:**\n- If the specified range exceeds 500 lines, only 500 lines will be returned with a truncation notice.\n- This tool can only read file content from the modified version (after changes) in git diff.",
"parameters": {
"type": "object",
"properties": {
Expand Down
4 changes: 2 additions & 2 deletions internal/diff/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func initRepoWithNonASCIIChange(t *testing.T) (string, string) {
runGitTest(t, repo, "config", "commit.gpgsign", "false")
runGitTest(t, repo, "config", "core.quotepath", "true")

relPath := "src/café/(authenticated)/文件.ts"
relPath := "src/café/(authenticated)/文件.ts" // allow-non-english: fixture exercises non-ASCII paths
file := filepath.Join(repo, filepath.FromSlash(relPath))
if err := os.MkdirAll(filepath.Dir(file), 0o755); err != nil {
t.Fatalf("create non-ASCII path: %v", err)
Expand Down Expand Up @@ -153,7 +153,7 @@ func TestWorkspaceDiffPreservesNonASCIIUntrackedPath(t *testing.T) {
repo, trackedPath := initRepoWithNonASCIIChange(t)
runGitTest(t, repo, "checkout", "--", trackedPath)

untrackedPath := "src/café/(authenticated)/新增.ts"
untrackedPath := "src/café/(authenticated)/新增.ts" // allow-non-english: fixture exercises non-ASCII paths
if err := os.WriteFile(filepath.Join(repo, filepath.FromSlash(untrackedPath)), []byte("untracked\n"), 0o644); err != nil {
t.Fatalf("write non-ASCII untracked file: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/session/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ func TestSanitizeReasonTruncatesAndSingleLine(t *testing.T) {
t.Fatal("newlines not collapsed")
}
// Multibyte input must not be cut mid-rune.
multibyte := strings.Repeat("世", maxReasonLen+50)
multibyte := strings.Repeat("世", maxReasonLen+50) // allow-non-english: fixture exercises multibyte truncation
if !utf8.ValidString(sanitizeReason(multibyte)) {
t.Fatal("truncation produced invalid UTF-8")
}
Expand Down
4 changes: 2 additions & 2 deletions internal/viewer/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ func TestTruncateText(t *testing.T) {
{"truncated with ellipsis", 3, "hello", "hel…"},
{"empty string", 5, "", ""},
{"n=0 always truncates non-empty", 0, "hi", "…"},
{"unicode shorter than n bytes", 20, "你好世界", "你好世界"},
{"unicode truncated at byte boundary", 6, "你好世界", "你好…"},
{"unicode shorter than n bytes", 20, "你好世界", "你好世界"}, // allow-non-english: fixture exercises rune-boundary truncation
{"unicode truncated at byte boundary", 6, "你好世界", "你好…"}, // allow-non-english: fixture exercises rune-boundary truncation
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions pages/src/components/Footer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import type { Language } from '../i18n/types';

const LANG_OPTIONS: { value: Language; label: string }[] = [
{ value: 'en', label: 'English' },
{ value: 'zh', label: '中文' },
{ value: 'ja', label: '日本語' },
{ value: 'ru', label: 'Русский' },
{ value: 'zh', label: '中文' }, // allow-non-english: language options are labelled in their own language
{ value: 'ja', label: '日本語' }, // allow-non-english: language options are labelled in their own language
{ value: 'ru', label: 'Русский' }, // allow-non-english: language options are labelled in their own language
];

const Footer: React.FC = () => {
Expand Down
2 changes: 1 addition & 1 deletion pages/src/components/HeroSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ const terminalLines = [
{ num: 9, content: <span>&nbsp;</span> },
{ num: 10, content: <span style={{ color: TC.dim }}>─── internal/auth/login.go:42-45 ───</span> },
{ num: 11, content: <span style={{ color: TC.text }}>Consider using bcrypt cost factor ≥ 12 for password hashing.</span> },
{ num: 12, content: <span className="terminal-cursor" style={{ color: TC.text }}>|</span> },
{ num: 12, content: <span className="terminal-cursor" style={{ color: TC.text }}>|</span> }, // allow-non-english: fullwidth bar renders the terminal cursor
];

const INSTALL_CHANNELS = [
Expand Down
13 changes: 10 additions & 3 deletions pages/src/components/MarkdownRenderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,24 @@ import { describe, expect, it } from 'vitest';
import { LanguageProvider } from '../i18n';
import MarkdownRenderer from './MarkdownRenderer';

// The headings are Russian on purpose: explicit heading IDs exist for text that
// cannot produce a usable ASCII slug on its own. Held in constants rather than
// inline, because a marker comment on the JSX line would render as heading text.
const H2 = 'Что делает навык'; // allow-non-english: fixture heading that cannot produce an ASCII slug
const H4 = 'Публикация'; // allow-non-english: fixture heading that cannot produce an ASCII slug
const CONTENT = `## ${H2} {#what-the-skill-does}\n\n#### ${H4} {#service-account}`;

describe('MarkdownRenderer heading IDs', () => {
it('renders an explicit heading ID without displaying its marker', () => {
render(
<LanguageProvider>
<MarkdownRenderer content={'## Что делает навык {#what-the-skill-does}\n\n#### Публикация {#service-account}'} />
<MarkdownRenderer content={CONTENT} />
</LanguageProvider>,
);

const heading = screen.getByRole('heading', { name: 'Что делает навык' });
const heading = screen.getByRole('heading', { name: H2 });
expect(heading.getAttribute('id')).toBe('what-the-skill-does');
expect(heading.textContent).not.toContain('{#what-the-skill-does}');
expect(screen.getByRole('heading', { name: 'Публикация', level: 4 }).getAttribute('id')).toBe('service-account');
expect(screen.getByRole('heading', { name: H4, level: 4 }).getAttribute('id')).toBe('service-account');
});
});
10 changes: 5 additions & 5 deletions pages/src/components/Navbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ import type { Language } from '../i18n/types';

const LANG_OPTIONS: { value: Language; label: string }[] = [
{ value: 'en', label: 'English' },
{ value: 'zh', label: '中文' },
{ value: 'ja', label: '日本語' },
{ value: 'ru', label: 'Русский' },
{ value: 'zh', label: '中文' }, // allow-non-english: language options are labelled in their own language
{ value: 'ja', label: '日本語' }, // allow-non-english: language options are labelled in their own language
{ value: 'ru', label: 'Русский' }, // allow-non-english: language options are labelled in their own language
];

const LANG_BADGE: Record<Language, string> = {
en: 'En',
zh: '中',
ja: 'あ',
zh: '中', // allow-non-english: single-glyph locale badge
ja: 'あ', // allow-non-english: single-glyph locale badge
ru: 'Ru',
};

Expand Down
12 changes: 8 additions & 4 deletions pages/src/utils/headingId.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@ import { describe, expect, it } from 'vitest';
import { extractHeadings } from './extractHeadings';
import { parseExplicitHeadingId } from './headingId';

// Russian on purpose: explicit heading IDs exist for text that cannot produce a
// usable ASCII slug on its own.
const HEADING = 'Что делает навк'; // allow-non-english: fixture heading that cannot produce an ASCII slug

describe('explicit heading IDs', () => {
it('separates a trailing explicit ID from the visible heading text', () => {
expect(parseExplicitHeadingId('Что делает навк {#what-the-skill-does}')).toEqual({
text: 'Что делает навк',
expect(parseExplicitHeadingId(`${HEADING} {#what-the-skill-does}`)).toEqual({
text: HEADING,
id: 'what-the-skill-does',
});
});
Expand All @@ -18,8 +22,8 @@ describe('explicit heading IDs', () => {
});

it('uses the explicit ID in the table of contents without exposing its marker', () => {
expect(extractHeadings('## Что делает навк {#what-the-skill-does}')).toEqual([
{ id: 'what-the-skill-does', text: 'Что делает навк', level: 2 },
expect(extractHeadings(`## ${HEADING} {#what-the-skill-does}`)).toEqual([
{ id: 'what-the-skill-does', text: HEADING, level: 2 },
]);
});
});
4 changes: 2 additions & 2 deletions scripts/github-actions/check-translation-sync.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ function testIdenticalStructurePasses() {
// Same outline, DIFFERENT heading text (simulating translations). Must pass:
// the check compares structure, not text.
const en = readme(["## What is it?", "### Details", "## Usage"]);
const zh = readme(["## 这是什么?", "### 细节", "## 使用方法"]);
const ja = readme(["## これは何ですか?", "### 詳細", "## 使い方"]);
const zh = readme(["## 这是什么?", "### 细节", "## 使用方法"]); // allow-non-english: fixture mimics translated README headings
const ja = readme(["## これは何ですか?", "### 詳細", "## 使い方"]); // allow-non-english: fixture mimics translated README headings
const { ok, errors } = compareReadmeStructures([
{ name: "README.md", content: en },
{ name: "README.zh-CN.md", content: zh },
Expand Down
Loading
Loading