From 0f0f33787b76274faa198b127b61457c2a8af375 Mon Sep 17 00:00:00 2001 From: verify Date: Thu, 23 Jul 2026 13:22:41 +0800 Subject: [PATCH] feat(cli): post the review body bilingually when the PR description is Chinese MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the PR author writes Chinese, the posted /review body was English-only. fetch-pr now records whether the PR description contains Han characters (prDescriptionHasHan, detected from the same gh pr view call and stamped into the plan report), and compose-review renders the body bilingually off that flag: the English body leads, the complete Chinese version rides collapsed in a
中文说明 block, and the model footer stays outside the fold. The signal is the CLI's own — the caller cannot toggle the register of a certified body — and a local plan has no field, so nothing changes for terminal-only reviews. Every deterministic body fragment carries an en/zh pair end to end: compose-review's clause templates and describeChunkGap phrases, the coverage disclosures (reasons, publicLabel role subjects via a new publicLabelZh, the path-free unread-brief reason) and the Step 4/5 gap texts including the combined same-shape sentence. Fragments with no deterministic translation — model-written findings, caller echoes, interpolated errors — ride verbatim in both halves. verificationGaps now returns structural {subject, reason, subjectZh, reasonZh} entries, which also removes compose-review's last recover-the-boundary-from-prose parse. SKILL.md instructs the same format for the model-authored inline comments: English finding first (marker and suggestion block stay in the English half — tooling filters on them), full Chinese translation collapsed beneath, footer last. --- .../commands/review/check-coverage.test.ts | 47 ++-- .../commands/review/compose-review.test.ts | 89 ++++++- .../cli/src/commands/review/compose-review.ts | 245 +++++++++++++----- packages/cli/src/commands/review/fetch-pr.ts | 13 +- .../src/commands/review/lib/agent-briefs.ts | 24 ++ .../cli/src/commands/review/lib/coverage.ts | 185 ++++++++++--- .../core/src/skills/bundled/review/SKILL.md | 4 +- 7 files changed, 484 insertions(+), 123 deletions(-) diff --git a/packages/cli/src/commands/review/check-coverage.test.ts b/packages/cli/src/commands/review/check-coverage.test.ts index 647f6dffac9..6c596967cb3 100644 --- a/packages/cli/src/commands/review/check-coverage.test.ts +++ b/packages/cli/src/commands/review/check-coverage.test.ts @@ -1348,6 +1348,11 @@ describe('an agent that paged its chunk still read it', () => { }); }); +/** The old rendered shape, for the regex assertions: structural gaps, joined. */ +const gapText = (r: { + gaps: Array<{ subject: string; reason: string }>; +}): string => r.gaps.map((g) => `${g.subject} — ${g.reason}`).join(' '); + describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () => { // A Step 4/5 agent as a real run leaves it: the CLI's record of the prompt it // built (`agent-prompt --role `), the brief that prompt points at, and the @@ -1427,7 +1432,7 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () const r = verificationGaps(p, { postsFindings: true }, ENV); expect(r.ok).toBe(false); - expect(r.gaps.join(' ')).toMatch(/verification — /); + expect(gapText(r)).toMatch(/verification — /); // The compliant launch — the full printed prompt — clears it. transcript('v-full', full, { calls: 2, opens: [brief] }); @@ -1468,7 +1473,7 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () const p = plan(); // no reverse-audit fixture: the step was skipped const r = verificationGaps(p, { postsFindings: false }, ENV); expect(r.ok).toBe(false); - const gap = r.gaps.join(' '); + const gap = gapText(r); expect(gap).toMatch( /reverse audit — no auditor was launched with a prompt this skill builds/, ); @@ -1490,7 +1495,7 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () step45(p, 'reverse-audit', { rewritten: true }); const r = verificationGaps(p, { postsFindings: false }, ENV); expect(r.ok).toBe(false); - const gap = r.gaps.join(' '); + const gap = gapText(r); // It says what happened — the auditor ran AND opened its brief (that is how // this shape is even detected, and a text denying it publishes a false // mechanism) … @@ -1527,7 +1532,7 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () step45(p, 'reverse-audit'); step45(p, 'verify', { rewritten: true }); const r = verificationGaps(p, { postsFindings: true }, ENV); - const gap = r.gaps.join(' '); + const gap = gapText(r); expect(gap).toMatch(/a verifier ran and opened its brief/); expect(gap).toMatch(/no agent was launched with the prompt the CLI built/); expect(gap).not.toMatch(/no verifier ran/); @@ -1546,7 +1551,7 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () step45(p, 'reverse-audit', { opensBrief: false }); const r = verificationGaps(p, { postsFindings: false }, ENV); expect(r.ok).toBe(false); - expect(r.gaps.join(' ')).toMatch( + expect(gapText(r)).toMatch( /reverse audit — it was launched with the built prompt but never opened its brief/, ); }); @@ -1556,7 +1561,7 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () step45(p, 'reverse-audit', { launch: false }); const r = verificationGaps(p, { postsFindings: false }, ENV); expect(r.ok).toBe(false); - expect(r.gaps.join(' ')).toMatch( + expect(gapText(r)).toMatch( /reverse audit — its prompt was built, but no agent was launched with it/, ); }); @@ -1565,7 +1570,7 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () const p = plan(); step45(p, 'reverse-audit--chunk-1'); const r = verificationGaps(p, { postsFindings: false }, ENV); - expect(r.gaps.join(' ')).not.toMatch(/reverse audit/); + expect(gapText(r)).not.toMatch(/reverse audit/); }); it('requires a verifier when the review posts findings', () => { @@ -1573,16 +1578,14 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () step45(p, 'reverse-audit'); // isolate the verify gap const r = verificationGaps(p, { postsFindings: true }, ENV); expect(r.ok).toBe(false); - expect(r.gaps.join(' ')).toMatch( - /verification — the review posts findings/, - ); + expect(gapText(r)).toMatch(/verification — the review posts findings/); }); it('does not require a verifier when the review confirmed nothing', () => { const p = plan(); step45(p, 'reverse-audit'); const r = verificationGaps(p, { postsFindings: false }, ENV); - expect(r.gaps.join(' ')).not.toMatch(/verification/); + expect(gapText(r)).not.toMatch(/verification/); }); it('flags a verifier built but whose agent never opened its brief', () => { @@ -1590,7 +1593,7 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () step45(p, 'reverse-audit'); step45(p, 'verify', { opensBrief: false }); const r = verificationGaps(p, { postsFindings: true }, ENV); - expect(r.gaps.join(' ')).toMatch( + expect(gapText(r)).toMatch( /verification — it was launched with the built prompt but never opened its brief/, ); }); @@ -1603,7 +1606,7 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () step45(p, 'reverse-audit'); step45(p, 'verify', { launch: false }); const r = verificationGaps(p, { postsFindings: true }, ENV); - expect(r.gaps.join(' ')).toMatch( + expect(gapText(r)).toMatch( /verification — its prompt was built, but no agent was launched with it/, ); }); @@ -1620,10 +1623,12 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () expect(r.ok).toBe(false); expect(r.gaps).toHaveLength(1); const gap = r.gaps[0]; - expect(gap).toMatch(/^verification and reverse audit — /); - expect(gap).toMatch(/each ran and opened its brief/); - expect(gap).toMatch(/written by hand/); - expect(gap).toMatch(/cannot be counted as verified/); + expect(gap.subject).toBe('verification and reverse audit'); + expect(gap.subjectZh).toBe('验证与反向审计'); + expect(gap.reasonZh).toContain('手写'); + expect(gap.reason).toMatch(/each ran and opened its brief/); + expect(gap.reason).toMatch(/written by hand/); + expect(gap.reason).toMatch(/cannot be counted as verified/); // The remediation stays per-role: the two rebuild commands differ. const fix = r.remediation.join(' '); expect(fix).toContain('--role reverse-audit'); @@ -1639,13 +1644,13 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () step45(p, 'verify', { launch: false }); const r = verificationGaps(p, { postsFindings: true }, ENV); expect(r.gaps).toHaveLength(2); - expect(r.gaps.join(' ')).toMatch( + expect(gapText(r)).toMatch( /reverse audit — an auditor ran and opened its brief/, ); - expect(r.gaps.join(' ')).toMatch( + expect(gapText(r)).toMatch( /verification — its prompt was built, but no agent was launched with it/, ); - expect(r.gaps.join(' ')).not.toMatch(/verification and reverse audit/); + expect(gapText(r)).not.toMatch(/verification and reverse audit/); }); it('does not merge when the review posts no findings — verify was never owed', () => { @@ -1654,6 +1659,6 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () const p = plan(); // neither step on record const r = verificationGaps(p, { postsFindings: false }, ENV); expect(r.gaps).toHaveLength(1); - expect(r.gaps[0]).toMatch(/^reverse audit — /); + expect(r.gaps[0].subject).toBe('reverse audit'); }); }); diff --git a/packages/cli/src/commands/review/compose-review.test.ts b/packages/cli/src/commands/review/compose-review.test.ts index e0569d5d401..0375dc51f9f 100644 --- a/packages/cli/src/commands/review/compose-review.test.ts +++ b/packages/cli/src/commands/review/compose-review.test.ts @@ -59,12 +59,15 @@ const DIFF = '/abs/diff.txt'; * satisfies that one. A plan that requires nothing is not a plan any capture * command writes, and coverage now reads the roster out of it. */ -function plan(opts: { step45?: boolean } = {}): string { +function plan(opts: { step45?: boolean; han?: boolean } = {}): string { const p = join(dir, 'plan.json'); writeFileSync( p, JSON.stringify({ diffPathAbsolute: DIFF, + // What fetch-pr records when the PR description contains Han + // characters — the deterministic bilingual-body switch. + ...(opts.han ? { prDescriptionHasHan: true } : {}), srcDiffLines: 5000, diffLines: 5000, files: [{ path: 'a.ts', kind: 'source', removedLines: 0, heavy: false }], @@ -279,10 +282,11 @@ function blindPrompt(chunk: number): string { */ function coveredPlan( step45Keys: string[] = ['verify', 'reverse-audit'], + planOpts: { han?: boolean } = {}, ): string { transcript('a1', goodPrompt(1), { toolCalls: 3 }); transcript('a2', goodPrompt(2), { toolCalls: 2 }); - const p = plan({ step45: false }); + const p = plan({ step45: false, ...planOpts }); recordBuilt(p, 1); recordBuilt(p, 2); recordMatrix(p); @@ -1983,6 +1987,7 @@ describe('describeChunkGap — chunk ids leave in the author units', () => { it('every planned chunk collapses to the diff itself', () => { expect(describeChunkGap([2, 1, 3], planned)).toEqual({ phrase: 'the entire diff', + phraseZh: '整个 diff', plural: false, }); }); @@ -1990,10 +1995,12 @@ describe('describeChunkGap — chunk ids leave in the author units', () => { it('names the files of a narrow gap — sorted by id, deduped', () => { expect(describeChunkGap([2], planned)).toEqual({ phrase: 'the diff section covering src/b.ts, src/c.ts', + phraseZh: '涉及 src/b.ts、src/c.ts 的 diff 片段', plural: false, }); expect(describeChunkGap([3, 1], planned)).toEqual({ phrase: 'the diff sections covering src/a.ts, src/d.ts', + phraseZh: '涉及 src/a.ts、src/d.ts 的 diff 片段', plural: true, }); // A subject disclosed twice is one gap. @@ -2008,6 +2015,7 @@ describe('describeChunkGap — chunk ids leave in the author units', () => { ]; expect(describeChunkGap([1, 2], wide)).toEqual({ phrase: "2 of the diff's 3 sections", + phraseZh: 'diff 3 个片段中的 2 个', plural: true, }); }); @@ -2020,6 +2028,7 @@ describe('describeChunkGap — chunk ids leave in the author units', () => { ]; expect(describeChunkGap([1, 2], partial)).toEqual({ phrase: "2 of the diff's 3 sections", + phraseZh: 'diff 3 个片段中的 2 个', plural: true, }); }); @@ -2027,11 +2036,87 @@ describe('describeChunkGap — chunk ids leave in the author units', () => { it('still says something with no plan to count against', () => { expect(describeChunkGap([7], [])).toEqual({ phrase: '1 section of the diff', + phraseZh: 'diff 中的 1 个片段', plural: false, }); expect(describeChunkGap([9, 7], [])).toEqual({ phrase: '2 sections of the diff', + phraseZh: 'diff 中的 2 个片段', plural: true, }); }); }); + +describe('bilingual body — the PR author writes Chinese (prDescriptionHasHan)', () => { + it('folds the complete Chinese version under the English body, footer outside the fold', () => { + // Not base(): its planPath default runs coveredPlan() again on the same + // path and would overwrite the han-stamped plan. + const r = composeReview({ + suggestionsInline: 1, + planPath: coveredPlan(undefined, { han: true }), + env: ENV, + modelId: MODEL, + }); + expect(r.event).toBe('COMMENT'); + // English leads, untouched. + expect( + r.body.startsWith('Reviewed — no blockers. Suggestions are inline.'), + ).toBe(true); + // The complete Chinese version rides collapsed. + expect(r.body).toContain('
\n中文说明'); + expect(r.body).toContain('已审查——无阻断问题。 建议见行内评论。'); + // One footer, after the fold — never inside it. + expect(r.body.endsWith(FOOTER)).toBe(true); + expect(r.body.split(FOOTER)).toHaveLength(2); + expect(r.body.indexOf('
')).toBeLessThan(r.body.indexOf(FOOTER)); + }); + + it('stays English-only without the plan flag', () => { + const r = composeReview(base({ suggestionsInline: 1 })); + expect(r.body).not.toContain('
'); + expect(r.body).not.toContain('中文'); + }); + + it('translates the LGTM body', () => { + const r = composeReview({ + planPath: coveredPlan(undefined, { han: true }), + env: ENV, + modelId: MODEL, + }); + expect(r.event).toBe('APPROVE'); + expect(r.body).toContain('No issues found. LGTM! ✅'); + expect(r.body).toContain('未发现问题。LGTM！✅'); + }); + + it('translates the disclosures — role phrase and Not-reviewed frame', () => { + // test-matrix required and never built → one role gap, both languages. + const p = plan({ han: true }); + transcript('a1', goodPrompt(1), { toolCalls: 3 }); + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + recordBuilt(p, 1); + recordBuilt(p, 2); + const r = composeReview({ planPath: p, env: ENV, modelId: MODEL }); + expect(r.body).toContain( + 'Not reviewed: the whole-diff test-coverage check', + ); + expect(r.body).toContain('未审查：全 diff 测试覆盖检查——'); + // The zh sentence carries the translated reason, not the English one. + expect(r.body).toContain('没有记录表明它的 brief 到达过任何 agent'); + }); + + it('quotes untranslatable caller text as-is in both halves', () => { + const r = composeReview({ + suggestionsInline: 1, + cannotTellCriticals: ['old blocker at a.ts:1 — still reachable?'], + planPath: coveredPlan(undefined, { han: true }), + env: ENV, + modelId: MODEL, + }); + expect(r.body).toContain('Unresolved, please confirm:'); + expect(r.body).toContain('未决，请确认：'); + // The caller's text, once per half. + expect( + r.body.match(/old blocker at a\.ts:1 — still reachable\?/g) ?? [], + ).toHaveLength(2); + }); +}); diff --git a/packages/cli/src/commands/review/compose-review.ts b/packages/cli/src/commands/review/compose-review.ts index 0aaa0ae46e0..a920b3b7d76 100644 --- a/packages/cli/src/commands/review/compose-review.ts +++ b/packages/cli/src/commands/review/compose-review.ts @@ -215,6 +215,8 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { reason: string; publicSubject?: string; publicReason?: string; + subjectZh?: string; + reasonZh?: string; }> = []; // The fixes for the gaps above, for stderr — never for the body. The gap says // what the review cannot certify, to the PR author; the remediation names the @@ -282,6 +284,8 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { reason: 'no plan was given, so this run cannot show that any of the diff ' + 'was read', + subjectZh: '覆盖情况', + reasonZh: '未提供 plan，本次运行无法证明 diff 的任何部分被读过', }); criticalsUnverified = criticalsNeedingVerify >= 1; } else { @@ -305,6 +309,7 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { coverageEntries.push({ subject: label, reason: 'the agent made no tool call: it read nothing', + reasonZh: '该 agent 未发起任何工具调用：它什么都没读', }); } if (cov.idleAgents.length > 0) { @@ -327,6 +332,7 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { reason: 'launched with a prompt that never named the diff file, so it ' + 'could not have read it', + reasonZh: '启动 prompt 从未提到 diff 文件，它不可能读过 diff', }); } if (cov.blindAgents.length > 0) { @@ -347,6 +353,8 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { reason: 'pointed at diff lines it never opened: it made tool calls, but ' + 'none of them read the diff', + reasonZh: + '它被指向 diff 的行却从未打开：有工具调用，但没有一次读取 diff', }); } if (cov.unopenedAgents.length > 0) { @@ -411,9 +419,15 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { err instanceof TranscriptsUnavailableError ? `could not read the agents' transcripts (${err.message})` : `the plan could not be used (${(err as Error).message})`; + const whyZh = + err instanceof TranscriptsUnavailableError + ? `无法读取 agent 的运行记录（${err.message}）` + : `plan 无法使用（${(err as Error).message}）`; coverageEntries.push({ subject: 'coverage', reason: `${why}, so this run cannot show that any of the diff was read`, + subjectZh: '覆盖情况', + reasonZh: `${whyZh}，本次运行无法证明 diff 的任何部分被读过`, }); } @@ -436,18 +450,15 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { { postsFindings: findingsToVerify > 0 }, input.env, ); + // Structural, both languages — no boundary is recovered from rendered + // prose (reparsing was the bug the disclosure entries already fixed). for (const gap of verification.gaps) { - // The machine's own two subjects ('verification', 'reverse audit'), - // dash-free by construction — the first separator is the boundary. - const cut = gap.indexOf(' — '); - coverageEntries.push( - cut === -1 - ? { subject: gap, reason: '' } - : { - subject: gap.slice(0, cut), - reason: gap.slice(cut + ' — '.length), - }, - ); + coverageEntries.push({ + subject: gap.subject, + reason: gap.reason, + subjectZh: gap.subjectZh, + reasonZh: gap.reasonZh, + }); } remediation.push(...verification.remediation); criticalsUnverified = @@ -458,6 +469,8 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { reason: `could not check that Step 4 and Step 5 ran ` + `(${(err as Error).message})`, + subjectZh: '验证', + reasonZh: `无法检查步骤 4 与步骤 5 是否运行（${(err as Error).message}）`, }); // Fail closed: a verification that cannot be CHECKED is not a // verification that happened. @@ -572,13 +585,30 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { } const footer = `_— ${modelId} via Qwen Code /review_`; - const finish = (text: string): string => - text === '' ? '' : `${text}\n\n${footer}`; + // Bilingual rendering: when the plan (fetch-pr's report) says the PR + // description contains Han characters, the posted body carries the complete + // Chinese version collapsed under the English one — the shape this repo's + // own PR descriptions use, decided by the plan the CLI wrote, never by the + // caller. Fragments with no deterministic translation (model-written + // findings, caller echoes, error interpolations) ride verbatim in both + // halves. The footer stays outside the fold, once. A `zh === en` body has + // nothing translated, so no empty fold is published. + const bilingual = bilingualFromPlan(input.planPath); + const render = (parts: Bi[], sep: string): string => { + const en = parts.map((p) => p.en).join(sep); + if (en === '') return ''; + const zh = parts.map((p) => p.zh).join(sep); + const text = + bilingual && zh !== en + ? `${en}\n\n
\n中文说明\n\n${zh}\n\n
` + : en; + return `${text}\n\n${footer}`; + }; // Clause 6 — scope nobody reviewed. Legal on COMMENT and (alongside body // Criticals) on REQUEST_CHANGES: the blocker must not squeeze out the // disclosure of what was never read. - const notReviewedParts: string[] = []; + const notReviewedParts: Bi[] = []; if (missingReceipts.length > 0) { // One block for both channels, so an edit cannot touch the disclosure and // miss its repair (or vice versa) — the drift the rest of this file exists @@ -609,9 +639,10 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { if (unexplainedReceipts.length > 0) { const gap = describeChunkGap(unexplainedReceipts, plannedChunks); const pron = gap.plural ? 'them' : 'it'; - notReviewedParts.push( - `Not reviewed: ${gap.phrase} — no agent reported covering ${pron}; nobody read ${pron}.`, - ); + notReviewedParts.push({ + en: `Not reviewed: ${gap.phrase} — no agent reported covering ${pron}; nobody read ${pron}.`, + zh: `未审查：${gap.phraseZh}——没有 agent 报告覆盖过这部分，也没有人读过它。`, + }); } } if (uncoverable.length > 0) { @@ -627,15 +658,14 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { if (m) bareIds.push(Number(m[1])); else callerNamed.push(e); } - const shown = [ - ...(bareIds.length > 0 - ? [describeChunkGap(bareIds, plannedChunks).phrase] - : []), - ...callerNamed, - ]; - notReviewedParts.push( - `Not reviewed: ${shown.join(', ')} — a line there exceeds the read limit.`, - ); + const bareGap = + bareIds.length > 0 ? describeChunkGap(bareIds, plannedChunks) : null; + const shown = [...(bareGap ? [bareGap.phrase] : []), ...callerNamed]; + const shownZh = [...(bareGap ? [bareGap.phraseZh] : []), ...callerNamed]; + notReviewedParts.push({ + en: `Not reviewed: ${shown.join(', ')} — a line there exceeds the read limit.`, + zh: `未审查：${shownZh.join('、')}——其中有一行超出单次读取上限。`, + }); } // One disclosure per subject, one sentence per cause — structurally, not by // reparsing prose. The first cut recovered a subject/reason boundary from @@ -667,12 +697,17 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { const whiffedDimensions = callerLeft.filter((d) => !d.includes(' — ')); const explainedCaller = callerLeft.filter((d) => d.includes(' — ')); if (whiffedDimensions.length > 0) { - notReviewedParts.push( - `Not reviewed: ${whiffedDimensions.join(', ')} — the agent returned no evidence of its walk twice.`, - ); + notReviewedParts.push({ + en: `Not reviewed: ${whiffedDimensions.join(', ')} — the agent returned no evidence of its walk twice.`, + zh: `未审查：${whiffedDimensions.join('、')}——该 agent 连续两次未返回任何检查过程的证据。`, + }); } for (const d of explainedCaller) { - notReviewedParts.push(`Not reviewed: ${d}.`); + // Caller prose, untranslatable by construction — quoted as-is in both. + notReviewedParts.push({ + en: `Not reviewed: ${d}.`, + zh: `未审查：${d}。`, + }); } // Same cause, one sentence: forty-three chunks launched with rewritten // prompts are one failure with forty-three subjects, not forty-three @@ -688,8 +723,9 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { const seenSubjects = new Set(); const byReason = new Map< string, - Array<{ subject: string; publicSubject?: string }> + Array<{ subject: string; publicSubject?: string; subjectZh?: string }> >(); + const reasonZhOf = new Map(); for (const e of covEntries) { if (seenSubjects.has(e.subject)) continue; seenSubjects.add(e.subject); @@ -699,8 +735,18 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // is the per-subject repetition this map exists to kill. const key = e.publicReason ?? e.reason; const group = byReason.get(key) ?? []; - group.push({ subject: e.subject, publicSubject: e.publicSubject }); + group.push({ + subject: e.subject, + publicSubject: e.publicSubject, + subjectZh: e.subjectZh, + }); byReason.set(key, group); + // One printed reason, one translation: entries sharing the printed + // English reason share the Chinese one by construction (both derive from + // the same source string). Entries with none fall back to the English. + if (e.reasonZh !== undefined && !reasonZhOf.has(key)) { + reasonZhOf.set(key, e.reasonZh); + } } for (const [reason, entries] of byReason) { // Chunk subjects leave in the author's units, not the run's. `chunk 28` @@ -714,40 +760,54 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // public phrase can never shadow a chunk id out of the chunk collapse. const chunkIds: number[] = []; const named: string[] = []; + const namedZh: string[] = []; for (const e of entries) { const m = /^chunk (\d+)$/.exec(e.subject); if (m) chunkIds.push(Number(m[1])); - else named.push(e.publicSubject ?? e.subject); + else { + named.push(e.publicSubject ?? e.subject); + namedZh.push(e.subjectZh ?? e.publicSubject ?? e.subject); + } } - const shown = [ - ...(chunkIds.length > 0 - ? [describeChunkGap(chunkIds, plannedChunks).phrase] - : []), - ...named, - ]; - notReviewedParts.push( - reason + const gap = + chunkIds.length > 0 ? describeChunkGap(chunkIds, plannedChunks) : null; + const shown = [...(gap ? [gap.phrase] : []), ...named]; + const shownZh = [...(gap ? [gap.phraseZh] : []), ...namedZh]; + const reasonZh = reasonZhOf.get(reason) ?? reason; + notReviewedParts.push({ + en: reason ? `Not reviewed: ${shown.join(', ')} — ${reason}.` : `Not reviewed: ${shown.join(', ')}.`, - ); + zh: reason + ? `未审查：${shownZh.join('、')}——${reasonZh}。` + : `未审查：${shownZh.join('、')}。`, + }); } // Clause 5 — blockers the review could neither confirm nor clear. They // survive every event shape: erasing one is how a review approves the // very thing it is asking about. - const cannotTellBlock = + const cannotTellBlock: Bi[] = cannotTell.length === 0 ? [] : [ - `Unresolved, please confirm: ${cannotTell - .map((l) => withMarker(l)) - .join(' ')}`, + { + en: `Unresolved, please confirm: ${cannotTell + .map((l) => withMarker(l)) + .join(' ')}`, + zh: `未决，请确认：${cannotTell.map((l) => withMarker(l)).join(' ')}`, + }, ]; - const bodyCriticalBlock = bodyCriticals.map((l) => withMarker(l)); + // Model-written blockers: quoted as-is in both halves. + const bodyCriticalBlock: Bi[] = bodyCriticals + .map((l) => withMarker(l)) + .map((l) => ({ en: l, zh: l })); - const contextUnavailableClause = - 'Reviewed diff-only — the PR’s existing discussion could not be fetched, so this is not an approval and not a no-blockers claim.'; + const contextUnavailableClause: Bi = { + en: 'Reviewed diff-only — the PR’s existing discussion could not be fetched, so this is not an approval and not a no-blockers claim.', + zh: '仅审查了 diff——无法获取 PR 已有的讨论，因此这不构成批准，也不构成"无阻断问题"的结论。', + }; if (event === 'REQUEST_CHANGES') { // Empty body, except the disclosures: every clause whose state holds @@ -762,7 +822,7 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { ]; return { event, - body: finish(parts.join('\n\n')), + body: render(parts, '\n\n'), baseEvent, cappedBy, downgraded, @@ -774,7 +834,10 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { if (event === 'APPROVE') { return { event, - body: finish('No issues found. LGTM! ✅'), + body: render( + [{ en: 'No issues found. LGTM! ✅', zh: '未发现问题。LGTM！✅' }], + ' ', + ), baseEvent, cappedBy, downgraded, @@ -785,14 +848,16 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // COMMENT: ordered clause composition — each clause present iff its // condition holds, nothing else. - const clauses: string[] = []; + const clauses: Bi[] = []; // 1. Downgrade sentence (only when a presubmit flag changed the event). if (downgraded && downgradedFrom) { const reasons = downgradeReasons.join('; '); - clauses.push( - `⚠️ Downgraded from ${downgradedFrom} to Comment${reasons ? `: ${reasons}` : ''}.`, - ); + const fromZh = downgradedFrom === 'Approve' ? '批准' : '请求修改'; + clauses.push({ + en: `⚠️ Downgraded from ${downgradedFrom} to Comment${reasons ? `: ${reasons}` : ''}.`, + zh: `⚠️ 已从${fromZh}降级为评论${reasons ? `：${reasons}` : ''}。`, + }); } // 2. Context-unavailable clause — when present, it opens the body and no @@ -839,10 +904,13 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { coveredChunks.every((id) => disclosedChunkIds.has(id))); clauses.push( nothingCertified - ? '⚠️ This run could not certify that any of this diff was reviewed.' + ? { + en: '⚠️ This run could not certify that any of this diff was reviewed.', + zh: '⚠️ 本次运行无法证明这个 diff 的任何部分经过了审查。', + } : canCertify - ? 'Reviewed — no blockers.' - : 'Reviewed.', + ? { en: 'Reviewed — no blockers.', zh: '已审查——无阻断问题。' } + : { en: 'Reviewed.', zh: '已审查。' }, ); } @@ -851,17 +919,23 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // the discarded sentence says the opposite is the round-6 collision // this module exists to kill. (`s` stays right for the event — see // above.) - if (suggestionsInline > 0) clauses.push('Suggestions are inline.'); + if (suggestionsInline > 0) { + clauses.push({ en: 'Suggestions are inline.', zh: '建议见行内评论。' }); + } if (suggestionsDiscarded > 0) { // Self-contained: this lands in the posted body, and "see the terminal // output" pointed the PR author at a terminal only the operator has — // eight hours of real bot reviews carried that dead reference on five // different pull requests. - clauses.push( - `${suggestionsDiscarded} Suggestion-level finding(s) could not be ` + + clauses.push({ + en: + `${suggestionsDiscarded} Suggestion-level finding(s) could not be ` + `anchored to a changed line and were dropped; nothing further to act ` + `on here.`, - ); + zh: + `${suggestionsDiscarded} 条建议级发现无法锚定到改动行，已丢弃；` + + `此处无需进一步处理。`, + }); } // 5. Unresolved existing Criticals. @@ -880,7 +954,7 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { return { event, - body: finish(clauses.join(' ')), + body: render(clauses, ' '), baseEvent, cappedBy, downgraded, @@ -909,16 +983,17 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { * caller-echo dedup and the certification test all key on `chunk ` — and * in the stderr remediation, where the id is the selector a reader can act * on. `plural` is the phrase's grammatical number, for the one caller whose - * sentence carries a pronoun. + * sentence carries a pronoun; `phraseZh` is the same phrase for the Chinese + * half of a bilingual body. */ export function describeChunkGap( ids: readonly number[], planned: ReadonlyArray<{ id: number; files: string[] }>, -): { phrase: string; plural: boolean } { +): { phrase: string; phraseZh: string; plural: boolean } { const uniq = [...new Set(ids)].sort((a, b) => a - b); const inGap = new Set(uniq); if (planned.length > 0 && planned.every((p) => inGap.has(p.id))) { - return { phrase: 'the entire diff', plural: false }; + return { phrase: 'the entire diff', phraseZh: '整个 diff', plural: false }; } // The union of the gap's files, in plan order. One unknown chunk poisons // the list: naming three files over a gap that also covers a fourth, @@ -936,6 +1011,7 @@ export function describeChunkGap( if (allKnown && files.length <= 4) { return { phrase: `the diff ${uniq.length === 1 ? 'section' : 'sections'} covering ${files.join(', ')}`, + phraseZh: `涉及 ${files.join('、')} 的 diff 片段`, plural: uniq.length > 1, }; } @@ -944,10 +1020,47 @@ export function describeChunkGap( planned.length > 0 ? `${uniq.length} of the diff's ${planned.length} sections` : `${uniq.length} ${uniq.length === 1 ? 'section' : 'sections'} of the diff`, + phraseZh: + planned.length > 0 + ? `diff ${planned.length} 个片段中的 ${uniq.length} 个` + : `diff 中的 ${uniq.length} 个片段`, plural: uniq.length > 1, }; } +/** + * One body fragment, in the two languages a posted body can carry. + * + * `zh` renders only when `bilingualFromPlan` says the PR author writes + * Chinese; a fragment with no deterministic translation — a model-written + * finding, a caller echo, an interpolated error — carries the same text in + * both, and the Chinese section quotes it as it is. + */ +interface Bi { + en: string; + zh: string; +} + +/** + * Whether the posted body carries the collapsed Chinese version: the plan + * (fetch-pr's report) recorded Han characters in the PR description. The + * signal is the CLI's own — never the caller's, who could otherwise toggle + * the register of a certified body. A local plan has no such field, and a + * plan that cannot be read defaults to English-only: the language must never + * take the review down. + */ +function bilingualFromPlan(planPath: string | undefined): boolean { + if (!planPath) return false; + try { + const plan = JSON.parse(readFileSync(planPath, 'utf8')) as { + prDescriptionHasHan?: unknown; + }; + return plan?.prDescriptionHasHan === true; + } catch { + return false; + } +} + interface ComposeReviewCliArgs { input: string | undefined; comments: string; diff --git a/packages/cli/src/commands/review/fetch-pr.ts b/packages/cli/src/commands/review/fetch-pr.ts index 5e4945691ec..4ac8ae81712 100644 --- a/packages/cli/src/commands/review/fetch-pr.ts +++ b/packages/cli/src/commands/review/fetch-pr.ts @@ -61,6 +61,8 @@ interface PrMetadata { deletions: number; changedFiles: number; isCrossRepository: boolean; + /** The PR description, fetched only to detect the author's language. */ + body?: string; } interface FetchPrArgs { @@ -91,6 +93,14 @@ type FetchPrResult = PlanReport & { diffPath: string | null; /** Absolute path — `read_file` rejects relative paths. Agents use this. */ diffPathAbsolute: string | null; + /** + * True when the PR description contains Han characters — the author writes + * Chinese. `compose-review` reads it from this report (its `planPath`) and + * renders the posted body bilingually, English first with the full Chinese + * version collapsed; the skill mirrors the format on inline comments. A + * local review's plan has no such field: nothing is posted there. + */ + prDescriptionHasHan: boolean; }; /** Count lines of `:`, or 0 if it does not exist there. */ @@ -176,7 +186,7 @@ async function runFetchPr(args: FetchPrArgs): Promise { '--repo', ownerRepo, '--json', - 'headRefName,headRefOid,baseRefName,additions,deletions,changedFiles,isCrossRepository', + 'headRefName,headRefOid,baseRefName,additions,deletions,changedFiles,isCrossRepository,body', ); meta = JSON.parse(json) as PrMetadata; } catch (err) { @@ -286,6 +296,7 @@ async function runFetchPr(args: FetchPrArgs): Promise { baseFetchFailed, diffPath, diffPathAbsolute, + prDescriptionHasHan: /\p{Script=Han}/u.test(meta.body ?? ''), ...buildPlanReport(plan, (path) => fileLineCount(fetchedSha, path)), }; diff --git a/packages/cli/src/commands/review/lib/agent-briefs.ts b/packages/cli/src/commands/review/lib/agent-briefs.ts index 79d2e9693de..25fd2ea546b 100644 --- a/packages/cli/src/commands/review/lib/agent-briefs.ts +++ b/packages/cli/src/commands/review/lib/agent-briefs.ts @@ -67,6 +67,12 @@ export interface Brief { * subject in a grouped disclosure. */ publicLabel: string; + /** + * `publicLabel`, for the Chinese half of a bilingual posted body — rendered + * when the PR description is written in Chinese (the plan's + * `prDescriptionHasHan`). Same invariants: author-facing, distinct per role. + */ + publicLabelZh: string; /** * Does a path rule belong in this agent's brief? * @@ -130,6 +136,7 @@ export const BRIEFS: Record = { '0': { label: 'Agent 0: Issue fidelity & root-cause ownership', publicLabel: 'the linked-issue fidelity pass', + publicLabelZh: '关联 issue 一致性检查', readsDiff: true, brief: `You are **Agent 0: Issue Fidelity & Root-Cause Ownership**. Your scope is issue fidelity, not general code review — do not report ordinary code defects; other agents own those. @@ -153,6 +160,7 @@ If \`gh\` fails (auth, rate limit, network), **retry that fetch once**. If it fa reviewsCode: true, label: 'Agent 1a: Line-by-line correctness', publicLabel: 'the line-by-line correctness pass', + publicLabelZh: '逐行正确性检查', readsDiff: true, brief: `You are **Agent 1a: the line-by-line scan**. Your dimension is defined by *how you walk*, not by a topic — a topical "find correctness bugs" brief makes every agent converge on the same visibly-suspicious hunks, which is redundancy, not coverage. @@ -171,6 +179,7 @@ Scope guard: reading the enclosing function is for **context**. A defect entirel reviewsCode: true, label: 'Agent 1b: Removed-behavior audit', publicLabel: 'the removed-behavior audit', + publicLabelZh: '删除行为审计', readsDiff: true, brief: `You are **Agent 1b: the removed-behavior audit**. You own the diff's **deleted side**, and you are the only agent who can see it: the \`-\` lines exist *only* in the diff. The post-change tree carries no trace of what was removed — the line is simply not there, and nothing marks where it was — so no agent reading the new code alone can find this class of defect. @@ -189,6 +198,7 @@ Each failure scenario must name what input or state now slips past the removed b reviewsCode: true, label: 'Agent 1c: Cross-file tracer', publicLabel: 'the cross-file consistency pass', + publicLabelZh: '跨文件一致性检查', readsDiff: true, brief: `You are **Agent 1c: the cross-file tracer**. You own the *whole* cross-file walk, end to end. It used to be a duty shared by six agents, and a duty shared by six agents is a duty nobody finishes while the same symbols get grepped six times. @@ -216,6 +226,7 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th reviewsCode: true, label: 'Agent 2: Security', publicLabel: 'the security pass', + publicLabelZh: '安全检查', readsDiff: true, brief: `You are **Agent 2: Security**. Review the diff for: @@ -233,6 +244,7 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th reviewsCode: true, label: 'Agent 3: Code quality', publicLabel: 'the code-quality pass', + publicLabelZh: '代码质量检查', readsDiff: true, brief: `You are **Agent 3: Code Quality**. Review the diff for: @@ -247,6 +259,7 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th reviewsCode: true, label: 'Agent 4: Performance & efficiency', publicLabel: 'the performance pass', + publicLabelZh: '性能检查', readsDiff: true, brief: `You are **Agent 4: Performance & Efficiency**. Review the diff for: @@ -262,6 +275,7 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th reviewsCode: true, label: 'Agent 5: Test coverage', publicLabel: 'the test-coverage pass', + publicLabelZh: '测试覆盖检查', readsDiff: true, brief: `You are **Agent 5: Test Coverage**. Review the diff for: @@ -279,6 +293,7 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th reviewsCode: true, label: 'Agent 6a: Undirected audit — attacker mindset', publicLabel: 'the open-ended audit (attacker mindset)', + publicLabelZh: '开放式审计（攻击者视角）', readsDiff: true, brief: `You are **Agent 6a: the undirected audit, attacker mindset.** @@ -299,6 +314,7 @@ You are undirected on purpose. Do not restrict yourself to the list.`, reviewsCode: true, label: 'Agent 6b: Undirected audit — 3 AM oncall mindset', publicLabel: 'the open-ended audit (oncall mindset)', + publicLabelZh: '开放式审计（值班排障视角）', readsDiff: true, brief: `You are **Agent 6b: the undirected audit, 3 AM oncall mindset.** @@ -319,6 +335,7 @@ You are undirected on purpose. Do not restrict yourself to the list.`, reviewsCode: true, label: 'Agent 6c: Undirected audit — six-months-later maintainer', publicLabel: 'the open-ended audit (maintainer mindset)', + publicLabelZh: '开放式审计（后续维护者视角）', readsDiff: true, brief: `You are **Agent 6c: the undirected audit, six-months-later maintainer mindset.** @@ -338,6 +355,7 @@ You are undirected on purpose. Do not restrict yourself to the list.`, '7': { label: 'Agent 7: Build & test verification', publicLabel: 'the build-and-test check', + publicLabelZh: '构建与测试验证', readsDiff: false, brief: `You are **Agent 7: Build & Test Verification**. You do not review the diff — you run the project's own deterministic checks and report what they say. Your evidence is **the commands you ran and their output**; a return that names no command has not done this job. @@ -354,6 +372,7 @@ Use \`Source: [build]\` or \`Source: [test]\`, never \`[review]\`.`, 'test-matrix': { label: 'Test coverage matrix (whole-diff)', publicLabel: 'the whole-diff test-coverage check', + publicLabelZh: '全 diff 测试覆盖检查', readsDiff: true, brief: `You are the **test-coverage matrix** agent — Agent 5's cross-chunk counterpart. The territory agents each see either an implementation or a test, rarely both. You see the whole diff, so you own the pairing. @@ -366,6 +385,7 @@ Use \`Source: [build]\` or \`Source: [test]\`, never \`[review]\`.`, reviewsCode: true, label: 'Invariant agent A: state, timers, collections', publicLabel: 'the invariant check (state, timers, collections)', + publicLabelZh: '不变量检查（状态、定时器、集合）', readsDiff: true, brief: `You are **invariant agent A: state, timers, and collections.** @@ -385,6 +405,7 @@ Report a **Critical** for each violation, and give **both** locations that toget label: 'Invariant agent B: counters, return values, error taxonomies', publicLabel: 'the invariant check (counters, return values, error taxonomies)', + publicLabelZh: '不变量检查（计数器、返回值、错误分类）', readsDiff: true, brief: `You are **invariant agent B: counters, return values, and error taxonomies.** @@ -403,6 +424,7 @@ Report a **Critical** for each violation, and give **both** locations that toget reviewsCode: true, label: 'Invariant agent C: config fields, early returns', publicLabel: 'the invariant check (config fields, early returns)', + publicLabelZh: '不变量检查（配置字段、提前返回）', readsDiff: true, brief: `You are **invariant agent C: config fields and early returns.** @@ -422,6 +444,7 @@ Report a **Critical** for each violation, and give **both** locations that toget acceptsFindings: true, label: 'Verification agent', publicLabel: 'verification', + publicLabelZh: '验证', readsDiff: true, brief: `You are a **verification agent**. You do not look for new problems — you rule on the findings you were handed, listed in the message that launched you, each with a file, a line, an issue, and a **failure scenario**. The failure scenario is the finding's testable claim, and your verdict is the **result of tracing it through the real code**, not a plausibility vote on how the finding reads. @@ -452,6 +475,7 @@ Return, for each finding, one verdict: acceptsFindings: true, label: 'Reverse audit agent', publicLabel: 'reverse audit', + publicLabelZh: '反向审计', readsDiff: true, brief: `You are a **reverse audit agent**. Prior agents have already reviewed this diff and their confirmed findings are listed in the message that launched you. Your job is not to re-report them — it is to find the **gaps**: the important issues no prior agent or round caught. diff --git a/packages/cli/src/commands/review/lib/coverage.ts b/packages/cli/src/commands/review/lib/coverage.ts index 306a7d15793..f64bb74c026 100644 --- a/packages/cli/src/commands/review/lib/coverage.ts +++ b/packages/cli/src/commands/review/lib/coverage.ts @@ -167,6 +167,15 @@ export interface CoverageFromTranscripts { * only an operator can use — today, the unread brief's filesystem path. */ publicReason?: string; + /** + * The printed subject and reason, for the Chinese half of a bilingual + * body (the plan's `prDescriptionHasHan`). `subjectZh` is absent for + * chunk subjects — the chunk collapse translates those — and for + * subjects with no Chinese variant the renderer falls back to the + * English text rather than dropping the disclosure. + */ + subjectZh?: string; + reasonZh?: string; }>; /** * Every planned chunk with the source files it covers, in plan order — the @@ -311,6 +320,13 @@ function publicRoleLabel(req: RequiredAgent): string | undefined { return req.file ? `${base} on ${req.file}` : base; } +/** `publicRoleLabel`, for the Chinese half of a bilingual body. */ +function publicRoleLabelZh(req: RequiredAgent): string | undefined { + if (req.role === 'chunk') return undefined; + const base = BRIEFS[req.role].publicLabelZh; + return req.file ? `${base}（${req.file}）` : base; +} + /** Something a reader can act on. `agentName` is `general-purpose` for all of them. */ function label(rec: AgentRecord, chunk: number | null): string { if (chunk !== null) return `chunk ${chunk}`; @@ -354,13 +370,20 @@ export function coverageFromTranscripts( const disclose = ( subject: string, reason: string, - pub?: { subject?: string; reason?: string }, + pub?: { + subject?: string; + reason?: string; + subjectZh?: string; + reasonZh?: string; + }, ): string => { disclosures.push({ subject, reason, publicSubject: pub?.subject, publicReason: pub?.reason, + subjectZh: pub?.subjectZh, + reasonZh: pub?.reasonZh, }); return `${subject} — ${reason}`; }; @@ -475,6 +498,11 @@ export function coverageFromTranscripts( name, 'ran on a prompt the run wrote itself (none was built for this ' + 'chunk), so the brief with its method and rules never reached it', + { + reasonZh: + '运行在这次 run 自行编写的 prompt 上（该 chunk 从未构建过 ' + + 'prompt），承载方法与规则的 brief 从未到达该 agent', + }, ), ); } @@ -485,6 +513,7 @@ export function coverageFromTranscripts( disclose( name, 'launched with a prompt that is not the one the CLI built', + { reasonZh: '启动时使用的 prompt 不是 CLI 构建的那一份' }, ), ); } @@ -575,6 +604,14 @@ export function coverageFromTranscripts( `reviewed, if at all, from prompts the run wrote for itself: no ` + `record shows the severity bar, the finding format or this ` + `project's own rules reaching an agent`, + { + subjectZh: '所有维度', + reasonZh: + `${roster.length} 个必需 agent 中没有任何一个有记录表明是用本 ` + + `skill 构建的 prompt 启动的，这个 diff 即便被审查过，也是基于这次 ` + + `run 自行编写的 prompt：没有记录表明严重级别标准、发现格式或本项目` + + `自己的规则到达过任何 agent`, + }, ), ); } @@ -639,7 +676,13 @@ export function coverageFromTranscripts( roleLabel(req), 'no record shows its brief reaching an agent, so this dimension ' + 'was reviewed, if at all, from a prompt the run wrote for itself', - { subject: publicRoleLabel(req) }, + { + subject: publicRoleLabel(req), + subjectZh: publicRoleLabelZh(req), + reasonZh: + '没有记录表明它的 brief 到达过任何 agent，这个维度即便被审查' + + '过，也是基于这次 run 自行编写的 prompt', + }, ), ); } @@ -661,7 +704,14 @@ export function coverageFromTranscripts( 'transcript cannot certify two dimensions' : 'its prompt was built, but no agent on record was launched ' + 'with it', - { subject: publicRoleLabel(req) }, + { + subject: publicRoleLabel(req), + subjectZh: publicRoleLabelZh(req), + reasonZh: anyMatch + ? '它的 prompt 只到达了一个已被记入其他区块的 agent；一个 agent ' + + '被塞进了多个区块，而一份运行记录无法为两个维度作证' + : '它的 prompt 已构建，但没有任何 agent 有记录用它启动过', + }, ), ); missingRoleSelectors.push(selectorOf(req)); @@ -704,6 +754,8 @@ export function coverageFromTranscripts( reason: 'never opened its brief, so it reviewed without the ' + 'instructions it was launched to follow', + subjectZh: publicRoleLabelZh(req), + reasonZh: '从未打开自己的 brief，审查时缺失了它本应遵循的指令', }, ), ); @@ -784,6 +836,8 @@ type Delivery = interface GapEntry { /** Author-facing: what this review cannot certify, and why. */ gap: string; + /** `gap`, for the Chinese half of a bilingual posted body. */ + gapZh: string; /** Orchestrator-facing: the exact fix, printed to stderr. */ fix: string; } @@ -826,6 +880,9 @@ const REVERSE_AUDIT_GAP: GapText = { 'no auditor was launched with a prompt this skill builds — the pass ' + 'that hunts what the rest of the review missed ran, if at all, without ' + 'the method its brief carries', + gapZh: + '没有审计 agent 是用本 skill 构建的 prompt 启动的——负责搜寻评审其余部分' + + '遗漏问题的这道工序，即便运行过，也缺失了 brief 承载的方法', fix: rebuildFix('reverse-audit', 'round'), }, // Same reach limit as `not-built`: a hand-written auditor that never opened @@ -836,6 +893,9 @@ const REVERSE_AUDIT_GAP: GapText = { 'its prompt was built, but no agent was launched with it — the pass ' + 'that hunts what the rest of the review missed ran, if at all, without ' + 'the method its brief carries, and cannot be certified', + gapZh: + '它的 prompt 已构建，但没有 agent 用它启动——负责搜寻评审其余部分遗漏' + + '问题的这道工序，即便运行过，也缺失了 brief 承载的方法，无法作证', fix: rebuildFix('reverse-audit', 'round'), }, // `rewritten` is reached only after a successful call OPENED the brief — so @@ -847,6 +907,10 @@ const REVERSE_AUDIT_GAP: GapText = { 'an auditor ran and opened its brief, but no agent was launched with the ' + 'prompt the CLI built — the launch was written by hand, and what the ' + 'agent was actually asked is not what this skill certifies', + gapZh: + '有审计 agent 运行并打开了自己的 brief，但没有 agent 是用 CLI 构建的 ' + + 'prompt 启动的——启动 prompt 是手写的，agent 实际被要求做的并不是本 ' + + 'skill 所认证的内容', fix: rebuildFix('reverse-audit', 'round'), }, 'brief-unread': { @@ -854,6 +918,9 @@ const REVERSE_AUDIT_GAP: GapText = { 'it was launched with the built prompt but never opened its brief, so it ' + 'audited without the gaps-only method and the finding format it was ' + 'launched to follow', + gapZh: + '它用构建的 prompt 启动，却从未打开自己的 brief，审计时缺失了只报缺口的' + + '方法和它本应遵循的发现格式', fix: 'relaunch with the same printed prompt — the agent must OPEN the brief ' + 'file the prompt names; that read is the receipt', @@ -868,12 +935,17 @@ const VERIFY_GAP: GapText = { 'the review posts findings, but no verifier was launched with a prompt ' + 'this skill builds — they were ruled on, if at all, without the verdict ' + 'bar its brief carries', + gapZh: + '本次评审发布了发现，但没有验证 agent 是用本 skill 构建的 prompt 启动的' + + '——这些发现即便被裁定过，也缺失了 brief 承载的裁定标准', fix: rebuildFix('verify', 'shard'), }, 'not-launched': { gap: 'its prompt was built, but no agent was launched with it, so the posted ' + 'findings cannot be counted as verified', + gapZh: + '它的 prompt 已构建，但没有 agent 用它启动，发布的发现不能算作已验证', fix: rebuildFix('verify', 'shard'), }, rewritten: { @@ -881,12 +953,18 @@ const VERIFY_GAP: GapText = { 'a verifier ran and opened its brief, but no agent was launched with the ' + 'prompt the CLI built — the launch was written by hand, and the posted ' + 'findings cannot be counted as verified against it', + gapZh: + '有验证 agent 运行并打开了自己的 brief，但没有 agent 是用 CLI 构建的 ' + + 'prompt 启动的——启动 prompt 是手写的，发布的发现不能算作经它验证', fix: rebuildFix('verify', 'shard'), }, 'brief-unread': { gap: 'it was launched with the built prompt but never opened its brief, so it ' + 'ruled on the findings without the verdict bar it was launched to apply', + gapZh: + '它用构建的 prompt 启动，却从未打开自己的 brief，裁定发现时缺失了它本应' + + '使用的裁定标准', fix: 'relaunch with the same printed prompt — the agent must OPEN the brief ' + 'file the prompt names; that read is the receipt', @@ -905,38 +983,68 @@ const VERIFY_GAP: GapText = { * nobody ran, `rewritten` may not claim the brief never arrived. The * remediation stays per-role — the two rebuild commands differ. */ -const COMBINED_STEP45_GAP: Record, string> = { - 'not-built': - 'neither the verifier nor the reverse auditor was launched with a prompt ' + - 'this skill builds — the posted findings were ruled on, and the misses ' + - 'the rest of the review left were hunted, if at all, without the briefs ' + - 'this skill certifies against', - 'not-launched': - 'both prompts were built, but no agent was launched with either — the ' + - 'posted findings cannot be counted as verified, and the pass that hunts ' + - 'what the rest of the review missed cannot be certified', - rewritten: - 'each ran and opened its brief, but neither was launched with the prompt ' + - 'the CLI built — the launches were written by hand, so the posted ' + - 'findings cannot be counted as verified, and what the agents were ' + - 'actually asked is not what this skill certifies', - 'brief-unread': - 'each was launched with its built prompt and never opened its brief, so ' + - 'the findings were ruled on without the verdict bar, and the audit ran ' + - 'without the gaps-only method it was launched to follow', +const COMBINED_STEP45_GAP: Record< + Exclude, + { en: string; zh: string } +> = { + 'not-built': { + en: + 'neither the verifier nor the reverse auditor was launched with a prompt ' + + 'this skill builds — the posted findings were ruled on, and the misses ' + + 'the rest of the review left were hunted, if at all, without the briefs ' + + 'this skill certifies against', + zh: + '验证 agent 与反向审计 agent 都没有用本 skill 构建的 prompt 启动——发布的' + + '发现即便被裁定过、评审其余部分遗漏的问题即便被搜寻过，也都缺失了本 ' + + 'skill 用以认证的 brief', + }, + 'not-launched': { + en: + 'both prompts were built, but no agent was launched with either — the ' + + 'posted findings cannot be counted as verified, and the pass that hunts ' + + 'what the rest of the review missed cannot be certified', + zh: + '两份 prompt 都已构建，但都没有 agent 用它们启动——发布的发现不能算作已' + + '验证，搜寻评审遗漏问题的工序也无法作证', + }, + rewritten: { + en: + 'each ran and opened its brief, but neither was launched with the prompt ' + + 'the CLI built — the launches were written by hand, so the posted ' + + 'findings cannot be counted as verified, and what the agents were ' + + 'actually asked is not what this skill certifies', + zh: + '两者都运行并打开了各自的 brief，但都不是用 CLI 构建的 prompt 启动的——' + + '启动 prompt 是手写的，发布的发现不能算作已验证，agent 实际被要求做的也' + + '不是本 skill 所认证的内容', + }, + 'brief-unread': { + en: + 'each was launched with its built prompt and never opened its brief, so ' + + 'the findings were ruled on without the verdict bar, and the audit ran ' + + 'without the gaps-only method it was launched to follow', + zh: + '两者都用构建的 prompt 启动，却都从未打开自己的 brief——发现的裁定缺失了' + + '裁定标准，审计也缺失了它本应遵循的只报缺口的方法', + }, }; export interface VerificationReport { /** True when every required Step 4/5 agent ran and read its brief. */ ok: boolean; /** - * Self-explanatory gap lines, shaped to drop straight into - * `unreviewedDimensions` — each carries its own ` — ` reason, so - * `compose-review` renders it verbatim rather than appending the whiff sentence. + * The Step 4/5 gaps, structural — subject and reason apart, in both body + * languages, so `compose-review` never recovers a boundary from rendered + * prose (reparsing was the bug the disclosure entries already fixed). * These reach the POSTED review body: author-facing register, no internal * commands. */ - gaps: string[]; + gaps: Array<{ + subject: string; + reason: string; + subjectZh: string; + reasonZh: string; + }>; /** * The per-shape fix for each gap, in the same order — for stderr, where the * orchestrator reads. Never rendered into the body. @@ -985,7 +1093,7 @@ export function verificationGaps( const { plan, mtimeMs } = readPlan(planPath); const records = readTranscripts(mtimeMs, env, plan.diffPathAbsolute); const built = readRecordedPrompts(planPath); - const gaps: string[] = []; + const gaps: VerificationReport['gaps'] = []; const remediation: string[] = []; // How a step's agents actually got their prompt. The floor needs the four shapes @@ -1096,15 +1204,28 @@ export function verificationGaps( // way — the two rebuild commands differ, and the combined sentence lands in // the posted body while the fixes land on stderr. if (reverse !== 'ok' && verify !== null && verify === reverse) { - gaps.push( - `verification and reverse audit — ${COMBINED_STEP45_GAP[reverse]}`, - ); + gaps.push({ + subject: 'verification and reverse audit', + reason: COMBINED_STEP45_GAP[reverse].en, + subjectZh: '验证与反向审计', + reasonZh: COMBINED_STEP45_GAP[reverse].zh, + }); } else { if (reverse !== 'ok') { - gaps.push(`reverse audit — ${REVERSE_AUDIT_GAP[reverse].gap}`); + gaps.push({ + subject: 'reverse audit', + reason: REVERSE_AUDIT_GAP[reverse].gap, + subjectZh: '反向审计', + reasonZh: REVERSE_AUDIT_GAP[reverse].gapZh, + }); } if (verify !== null && verify !== 'ok') { - gaps.push(`verification — ${VERIFY_GAP[verify].gap}`); + gaps.push({ + subject: 'verification', + reason: VERIFY_GAP[verify].gap, + subjectZh: '验证', + reasonZh: VERIFY_GAP[verify].gapZh, + }); } } diff --git a/packages/core/src/skills/bundled/review/SKILL.md b/packages/core/src/skills/bundled/review/SKILL.md index 6febf3f5be6..f013cfe6933 100644 --- a/packages/core/src/skills/bundled/review/SKILL.md +++ b/packages/core/src/skills/bundled/review/SKILL.md @@ -105,7 +105,7 @@ Based on the parsed `target.type`: Guessing the owner/repo here is not a recoverable mistake — dogfooding this skill against its own PR, the model inferred the fork from the branch's push target, `fetch-pr` answered "Could not resolve to a PullRequest", and the review stopped before reading a line of code. If `gh repo view` and the remote scan disagree, or no remote matches, say so and stop rather than picking one. - Read `.qwen/tmp/qwen-review-pr--fetch.json` for: `worktreePath`, `baseRefName`, `headRefName`, `fetchedSha` (use as the **HEAD commit SHA** for Step 7), `isCrossRepository`, `diffStat` (files / additions / deletions). If the command fails (auth, network, PR not found), inform the user and stop. + Read `.qwen/tmp/qwen-review-pr--fetch.json` for: `worktreePath`, `baseRefName`, `headRefName`, `fetchedSha` (use as the **HEAD commit SHA** for Step 7), `isCrossRepository`, `diffStat` (files / additions / deletions), and `prDescriptionHasHan` (the PR description contains Chinese — every posted inline comment must then be bilingual; see Step 7). If the command fails (auth, network, PR not found), inform the user and stop. Worktree isolation: all subsequent steps (agents, build/test) operate inside `worktreePath`, not the user's working tree. Cache and reports (Step 8) are written to the **main project directory**, not the worktree. @@ -788,6 +788,8 @@ Rationale: an inline comment is the only place GitHub renders a ` ```suggestion ⚠️ **Suggestion text must never appear in the review `body`.** `.github/workflows/qwen-autofix.yml` keeps Suggestions out of the autofix loop by filtering the inline-comment channel on the `**[Suggestion]**` prefix. It does not filter review bodies, so a Suggestion smuggled into `body` would be handed to the autofix bot as actionable work. +**Bilingual comments when the author writes Chinese.** If the Step 1 fetch report says `prDescriptionHasHan: true`, write every inline comment bilingually: the English finding first — marker, description, failure scenario, ` ```suggestion ` block — then the complete Chinese translation collapsed in a `
中文说明…
` block, before the model footer. The severity marker and any ` ```suggestion ` block stay in the English half only (the marker is what tooling filters on; a duplicated suggestion block would render twice). The review `body` needs nothing from you: `submit` composes it from `state`, and its bilingual rendering reads the same plan flag on its own. + **Build the review JSON** with `write_file` to create `.qwen/tmp/qwen-review-{target}-review.json`. It carries three things and **no verdict** — `submit` computes the event and body itself, from the `state` you hand it and the comments you attach, and **refuses a payload that carries `event` or `body`** (a run that skipped the computation and typed its own Approve is exactly what that refusal stops). Every high-confidence Critical or Suggestion finding that maps to a diff line is an entry in `comments`: ````jsonc