mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-08-24 08:04:47 +00:00
* 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.
302 lines
10 KiB
JavaScript
302 lines
10 KiB
JavaScript
#!/usr/bin/env node
|
|
|
|
// SPDX-License-Identifier: Apache-2.0
|
|
// Copyright 2026 alibaba/open-code-review Contributors
|
|
|
|
"use strict";
|
|
|
|
// Unit tests for scripts/github-actions/check-translation-sync.js.
|
|
//
|
|
// Run via: node scripts/github-actions/check-translation-sync.test.js
|
|
// (also wired as `npm run test:github-actions`).
|
|
//
|
|
// Plain Node + assert, no external deps and no `node --test`, mirroring
|
|
// post-review-comments.test.js so both run on node >= 14.
|
|
|
|
const assert = require("assert");
|
|
const path = require("path");
|
|
const {
|
|
extractHeadings,
|
|
level2Headings,
|
|
structuralSignature,
|
|
compareReadmeStructures,
|
|
counterpartPaths,
|
|
findMissingTranslations,
|
|
getChangedFiles,
|
|
splitList,
|
|
} = require(path.join(__dirname, "check-translation-sync.js"));
|
|
|
|
// A minimal but realistic README skeleton: an intro H1, three `##` sections,
|
|
// one of them with a `###` subheading, plus a fenced bash block whose `#`
|
|
// comment lines must NOT be parsed as headings.
|
|
function readme(headings) {
|
|
// headings: array of "## Text" / "### Text" / "# Text" lines already formed.
|
|
return [
|
|
"# Project",
|
|
"",
|
|
"Intro paragraph.",
|
|
"",
|
|
"```bash",
|
|
"# this is a shell comment, not a heading",
|
|
"## also not a heading",
|
|
"echo hi",
|
|
"```",
|
|
"",
|
|
...headings.flatMap((h) => [h, "", "body text", ""]),
|
|
].join("\n");
|
|
}
|
|
|
|
function testExtractHeadingsSkipsCodeFences() {
|
|
const md = readme(["## Alpha", "### Alpha One", "## Beta"]);
|
|
const headings = extractHeadings(md);
|
|
// Only the real headings: H1 + 2x H2 + 1x H3. The fenced `#`/`##` lines are ignored.
|
|
assert.deepStrictEqual(
|
|
headings.map((h) => `${h.level}:${h.text}`),
|
|
["1:Project", "2:Alpha", "3:Alpha One", "2:Beta"]
|
|
);
|
|
assert.deepStrictEqual(level2Headings(md), ["Alpha", "Beta"]);
|
|
assert.deepStrictEqual(structuralSignature(md), [1, 2, 3, 2]);
|
|
}
|
|
|
|
function testExtractHeadingsTildeFenceAndTrailingHashes() {
|
|
const md = ["# T", "~~~", "## fenced", "~~~", "## Real ##"].join("\n");
|
|
assert.deepStrictEqual(
|
|
extractHeadings(md).map((h) => `${h.level}:${h.text}`),
|
|
["1:T", "2:Real"] // closing `## Real ##` trailing hashes stripped
|
|
);
|
|
}
|
|
|
|
function testExtractHeadingsInnerShorterFenceDoesNotClose() {
|
|
// CommonMark: a closing fence must be at least as long as the opening one.
|
|
// A block opened with ```` (4 backticks) must NOT be closed by an inner ```
|
|
// (3 backticks); everything up to the matching 4-backtick line stays code.
|
|
const md = [
|
|
"# Title",
|
|
"````", // open, length 4
|
|
"## not a heading (inside code)",
|
|
"```", // shorter than opening -> does NOT close
|
|
"## also inside code",
|
|
"````", // length >= 4 -> real close
|
|
"## Real Heading",
|
|
].join("\n");
|
|
assert.deepStrictEqual(
|
|
extractHeadings(md).map((h) => `${h.level}:${h.text}`),
|
|
["1:Title", "2:Real Heading"]
|
|
);
|
|
}
|
|
|
|
// --- README structure comparison -------------------------------------------
|
|
|
|
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(["## 这是什么?", "### 细节", "## 使用方法"]); // 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 },
|
|
{ name: "README.ja-JP.md", content: ja },
|
|
]);
|
|
assert.strictEqual(ok, true, JSON.stringify(errors));
|
|
assert.strictEqual(errors.length, 0);
|
|
}
|
|
|
|
function testAddedHeadingFails() {
|
|
const en = readme(["## Alpha", "## Beta"]);
|
|
const zh = readme(["## Alpha", "## Beta", "## Gamma"]); // extra ##
|
|
const { ok, errors } = compareReadmeStructures([
|
|
{ name: "README.md", content: en },
|
|
{ name: "README.zh-CN.md", content: zh },
|
|
]);
|
|
assert.strictEqual(ok, false);
|
|
assert.strictEqual(errors.length, 1);
|
|
assert.strictEqual(errors[0].file, "README.zh-CN.md");
|
|
assert.match(errors[0].message, /level-2 \(##\) heading/);
|
|
assert.match(errors[0].message, /has 3 level-2 .* has 2/);
|
|
}
|
|
|
|
function testDroppedHeadingFails() {
|
|
const en = readme(["## Alpha", "## Beta", "## Gamma"]);
|
|
const ja = readme(["## Alpha", "## Gamma"]); // dropped Beta
|
|
const { ok, errors } = compareReadmeStructures([
|
|
{ name: "README.md", content: en },
|
|
{ name: "README.ja-JP.md", content: ja },
|
|
]);
|
|
assert.strictEqual(ok, false);
|
|
assert.strictEqual(errors[0].file, "README.ja-JP.md");
|
|
assert.match(errors[0].message, /has 2 level-2 .* has 3/);
|
|
}
|
|
|
|
function testReorderedHeadingFails() {
|
|
// Two `##` sections with different sub-structure; swapping them keeps the
|
|
// level-2 COUNT identical but changes the outline order -> must fail.
|
|
// The sub-structure must differ so the reorder is detectable structurally:
|
|
// same level-2 count, different heading-level outline.
|
|
const en2 = readme(["## Alpha", "### A1", "## Beta"]); // [1,2,3,2]
|
|
const zh2 = readme(["## Beta", "## Alpha", "### A1"]); // [1,2,2,3]
|
|
const { ok, errors } = compareReadmeStructures([
|
|
{ name: "README.md", content: en2 },
|
|
{ name: "README.zh-CN.md", content: zh2 },
|
|
]);
|
|
assert.strictEqual(ok, false);
|
|
assert.strictEqual(errors[0].file, "README.zh-CN.md");
|
|
assert.match(errors[0].message, /reordered or its level changed/);
|
|
assert.match(errors[0].message, /heading #3/); // first divergence position
|
|
}
|
|
|
|
function testMissingReferenceOnlyIsFine() {
|
|
// A single file (only the reference) cannot diverge.
|
|
const { ok } = compareReadmeStructures([
|
|
{ name: "README.md", content: readme(["## A"]) },
|
|
]);
|
|
assert.strictEqual(ok, true);
|
|
}
|
|
|
|
// --- docs en -> zh/ja/ru counterpart sync -------------------------------------
|
|
|
|
function testCounterpartPaths() {
|
|
assert.deepStrictEqual(
|
|
counterpartPaths("pages/src/content/docs/en/integrations/ci.md"),
|
|
[
|
|
{ locale: "zh", path: "pages/src/content/docs/zh/integrations/ci.md" },
|
|
{ locale: "ja", path: "pages/src/content/docs/ja/integrations/ci.md" },
|
|
{ locale: "ru", path: "pages/src/content/docs/ru/integrations/ci.md" },
|
|
]
|
|
);
|
|
}
|
|
|
|
function testEnOnlyChangeWarns() {
|
|
const changed = [
|
|
"pages/src/content/docs/en/faq.md",
|
|
"internal/agent/agent.go",
|
|
];
|
|
const warnings = findMissingTranslations(changed);
|
|
// zh, ja, and ru counterparts missing -> three warnings.
|
|
assert.strictEqual(warnings.length, 3);
|
|
assert.deepStrictEqual(
|
|
warnings.map((w) => w.counterpart).sort(),
|
|
[
|
|
"pages/src/content/docs/ja/faq.md",
|
|
"pages/src/content/docs/ru/faq.md",
|
|
"pages/src/content/docs/zh/faq.md",
|
|
]
|
|
);
|
|
}
|
|
|
|
function testEnWithBothCounterpartsNoWarn() {
|
|
const changed = [
|
|
"pages/src/content/docs/en/faq.md",
|
|
"pages/src/content/docs/zh/faq.md",
|
|
"pages/src/content/docs/ja/faq.md",
|
|
"pages/src/content/docs/ru/faq.md",
|
|
];
|
|
assert.deepStrictEqual(findMissingTranslations(changed), []);
|
|
}
|
|
|
|
function testEnWithOnlyOneCounterpartWarnsForTheOther() {
|
|
const changed = [
|
|
"pages/src/content/docs/en/mcp.md",
|
|
"pages/src/content/docs/zh/mcp.md", // ja and ru still missing
|
|
];
|
|
const warnings = findMissingTranslations(changed);
|
|
assert.strictEqual(warnings.length, 2);
|
|
assert.deepStrictEqual(
|
|
warnings.map((w) => w.locale).sort(),
|
|
["ja", "ru"]
|
|
);
|
|
assert.deepStrictEqual(
|
|
warnings.map((w) => w.counterpart).sort(),
|
|
[
|
|
"pages/src/content/docs/ja/mcp.md",
|
|
"pages/src/content/docs/ru/mcp.md",
|
|
]
|
|
);
|
|
}
|
|
|
|
function testNonDocsAndNonEnChangesIgnored() {
|
|
const changed = [
|
|
"pages/src/content/docs/zh/faq.md", // zh-only change: not our trigger
|
|
"README.md",
|
|
"pages/src/content/docs/en/logo.png", // en but not markdown
|
|
];
|
|
assert.deepStrictEqual(findMissingTranslations(changed), []);
|
|
}
|
|
|
|
// --- changed-files resolution ----------------------------------------------
|
|
|
|
function testGetChangedFilesFromExplicitEnv() {
|
|
const env = { OCR_CHANGED_FILES: "a.md\npages/src/content/docs/en/x.md\n" };
|
|
assert.deepStrictEqual(getChangedFiles(env), [
|
|
"a.md",
|
|
"pages/src/content/docs/en/x.md",
|
|
]);
|
|
}
|
|
|
|
function testGetChangedFilesFromGitDiff() {
|
|
const calls = [];
|
|
const fakeExec = (cmd, args) => {
|
|
calls.push({ cmd, args });
|
|
return "pages/src/content/docs/en/faq.md\ninternal/x.go\n";
|
|
};
|
|
const env = { OCR_BASE_SHA: "base", OCR_HEAD_SHA: "head" };
|
|
const out = getChangedFiles(env, fakeExec);
|
|
assert.deepStrictEqual(out, [
|
|
"pages/src/content/docs/en/faq.md",
|
|
"internal/x.go",
|
|
]);
|
|
assert.strictEqual(calls[0].cmd, "git");
|
|
assert.deepStrictEqual(calls[0].args, ["diff", "--name-only", "base...head"]);
|
|
}
|
|
|
|
function testGetChangedFilesGitFailureIsNonFatal() {
|
|
const env = { OCR_BASE_SHA: "base", OCR_HEAD_SHA: "head" };
|
|
const boom = () => {
|
|
throw new Error("no merge base");
|
|
};
|
|
// Silence the expected ::warning annotation this path emits.
|
|
const orig = console.log;
|
|
console.log = () => {};
|
|
try {
|
|
assert.deepStrictEqual(getChangedFiles(env, boom), []);
|
|
} finally {
|
|
console.log = orig;
|
|
}
|
|
}
|
|
|
|
function testGetChangedFilesEmptyWhenNoInputs() {
|
|
assert.deepStrictEqual(getChangedFiles({}), []);
|
|
}
|
|
|
|
function testSplitList() {
|
|
assert.deepStrictEqual(splitList("a\nb,c\n\n d "), ["a", "b", "c", "d"]);
|
|
}
|
|
|
|
function main() {
|
|
testExtractHeadingsSkipsCodeFences();
|
|
testExtractHeadingsTildeFenceAndTrailingHashes();
|
|
testExtractHeadingsInnerShorterFenceDoesNotClose();
|
|
testIdenticalStructurePasses();
|
|
testAddedHeadingFails();
|
|
testDroppedHeadingFails();
|
|
testReorderedHeadingFails();
|
|
testMissingReferenceOnlyIsFine();
|
|
testCounterpartPaths();
|
|
testEnOnlyChangeWarns();
|
|
testEnWithBothCounterpartsNoWarn();
|
|
testEnWithOnlyOneCounterpartWarnsForTheOther();
|
|
testNonDocsAndNonEnChangesIgnored();
|
|
testGetChangedFilesFromExplicitEnv();
|
|
testGetChangedFilesFromGitDiff();
|
|
testGetChangedFilesGitFailureIsNonFatal();
|
|
testGetChangedFilesEmptyWhenNoInputs();
|
|
testSplitList();
|
|
console.log("All check-translation-sync tests passed.");
|
|
}
|
|
|
|
try {
|
|
main();
|
|
} catch (err) {
|
|
console.error(err);
|
|
process.exit(1);
|
|
}
|