fix(review): readable bilingual disclosures for lint deferrals and caller prose - #10587
Conversation
…ller prose The review body's disclosure sentences carried two readability defects, both visible on PR #10567's posted round-1 body: - The deferred-checker line stuttered: script-lint's deferral/skip reasons ended with an "— not linted" tail written for a standalone context, the gate spliced them under a "the executable-script lint —" prefix, and the body wrapped the result in a sentence that already opens "Not linted:". Posted: "Not linted (tool limitation, not a blocker): the executable-script lint — ... — not linted." The reasons drop the tail, and the disclosure drops the circular prefix — the wrapper names the fact once, the path and reason carry the rest. - The body's Chinese half presented untranslated English as its translation. Two legs: - The deferral disclosure is machine-built from the report, so it can carry a real translation: `scriptLintGate` now returns bilingual disclosure pairs, the report schema gains an optional `reasonZh` (the actionlint deferral supplies it), and an older CLI's report without one falls back to the English reason in both halves. - Caller-prose "Not reviewed" entries stay untranslatable by construction, and the Chinese label now says so — `未审查(原文为英文):` — instead of presenting an all-English sentence as a translation; the payload keeps its own English full stop rather than closing an English sentence with `。`.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks for the PR, @wenshao — the description doesn't follow this repo's PR template, though, so I have to stop here and ask for a restructure first.
The body carries ## What this PR does, ## What this PR does not change, and ## Testing, but the template requires: What this PR does, Why it's needed, Reviewer Test Plan (with How to verify, Evidence (Before & After), and the Tested on OS table), Risk & Scope, Linked Issues, and a Chinese translation in a <details> block (that last one is present). Please rework the description against .github/pull_request_template.md — the material is largely there already: the motivation currently opens "What this PR does", the test commands sit under ## Testing, and "What this PR does not change" covers much of Risk & Scope, so it mostly needs to move into the right sections — plus a reviewer-facing How to verify (e.g. what the composed disclosure sentences should read on a review body like #10567's) and the Tested on matrix.
For what it's worth, the problem itself is observably real — the exact "Not linted (tool limitation, not a blocker): the executable-script lint — … — not linted." stutter the PR quotes appears verbatim in this bot's round-1 review body on #10567 (review 5061446682, verified via API) — so this gate is purely about the description format at this stage, not about the change.
中文说明
感谢提交,@wenshao —— 但 PR 描述没有遵循本仓库的 PR 模板,所以这里需要先停下来,请重新组织描述。
正文目前有 ## What this PR does、## What this PR does not change 和 ## Testing,而模板要求:What this PR does、Why it's needed、Reviewer Test Plan(含 How to verify、Evidence (Before & After) 和 Tested on 操作系统表格)、Risk & Scope、Linked Issues,以及 <details> 块中的中文翻译(最后一项已具备)。请参照 .github/pull_request_template.md 重写——内容大多已经具备:动机目前写在 "What this PR does" 开头,测试命令在 ## Testing 下,"What this PR does not change" 覆盖了 Risk & Scope 的大部分内容,主要是把内容移到对应小节——另外补一个面向审查者的 How to verify(例如在 #10567 那样的 review 正文上,最终合成的披露语句应该是什么样子)和 Tested on 平台表格。
顺带说明:问题本身是真实可见的——PR 引用的 "Not linted (tool limitation, not a blocker): the executable-script lint — … — not linted." 重复,原样出现在本 bot 于 #10567 发布的第 1 轮 review 正文中(review 5061446682,已用 API 核实)——所以本次打回仅针对描述格式,与代码改动本身无关。
— Qwen Code · qwen3.8-max
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:反向审计——评审时间预算不足,未能开始第 5 轮。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
… for
Five review-suggested pins, each verified by re-running its witness
mutation:
- The comment-grammar fixture now carries a malicious reasonZh, so the
stripCommentGrammar(d.reasonZh) leg is exercised against a live marker
(deleting the call previously survived the suite).
- The pipeline reasonZh is pinned to its Chinese literal — a
toContain('actionlint') fragment was satisfied by the English reason too.
- Both skipped reasons get not.toContain('not linted') pins; re-appending
either tail previously shipped green.
- A two-entry deferred fixture pins the en '; ' and zh ';' joins and the
reasonZh-carrying branch end-to-end — every prior fixture held one entry,
so no join separator was ever observable, and the only full-sentence zh
pin exercised the English-fallback branch.
Description reworked to follow .github/pull_request_template.md: What this PR does / Why it's needed / Reviewer Test Plan (How to verify, Evidence Before & After, Tested on) / Risk & Scope / Linked Issues, with the full Chinese translation in the details block.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): "agent 5": executing the three changed test files — the prerequisite npm run build (worktree had no dist/ ) did not complete within two 10-minute command timeouts, so a…; "agent 2": run the three changed test files at HEAD to confirm the new pins pass (and mutation-prove the new witnesses go red without stripCommentGrammar(d.reasonZh) ) — …; "agent 6b": could not execute the three test suites — the review worktree has no node_modules or built dist/ ( npm ci + npm run build exceeds the remaining tool budg….
Not reviewed: reverse audit — stopped before round 1 by the review time budget.
Test Plan (not a blocker): src/commands/review/script-lint.test.ts — no such file or directory; src/commands/review/script-lint.mock.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory.
中文说明
仅完成部分审查,审查缺口已披露。
未探索到全部深度(达到工具调用预算):"agent 5":executing the three changed test files — the prerequisite npm run build (worktree had no dist/ ) did not complete within two 10-minute command timeouts, so a…;"agent 2":run the three changed test files at HEAD to confirm the new pins pass (and mutation-prove the new witnesses go red without stripCommentGrammar(d.reasonZh) ) — …;"agent 6b":could not execute the three test suites — the review worktree has no node_modules or built dist/ ( npm ci + npm run build exceeds the remaining tool budg…。
未审查:反向审计——评审时间预算不足,未能开始第 1 轮。
Test Plan(非阻断):src/commands/review/script-lint.test.ts — no such file or directory; src/commands/review/script-lint.mock.test.ts — no such file or directory; src/commands/review/compose-review.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — the PR could not be built - workflow run The PR could not be built because 中文 — 判定:❌ 不通过 · PR 构建失败由于 Install/build log— Qwen Code · sandboxed verification |
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献!
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review — no blocking findings. The approach matches what I would have done: fix the strings where they are produced, widen only the channel that needs a real translation, and leave the capping
Testing — evidence below is the PR's own CI on the reviewed commit, fetched via the API; no PR code was executed in this run.
中文说明代码审查——无阻断项。方案与我自己的独立思路一致:在字符串产生处修复,只扩展真正需要译文的通道,封顶用的
测试——以下证据来自该 commit 上 PR 自身的 CI(经 API 获取),本次运行未执行任何 PR 代码。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. The defect was verified against the actual posted body, not taken on faith — both the stutter and the untranslated Chinese half are right there in review 5061446682. The fix goes after the cause instead of the symptom: the reasons no longer restate what the wrapper already says, the one channel that could afford a real translation gets one, and the channel that cannot (caller prose) stops pretending to translate and labels itself honestly. The compatibility story is genuinely two-way, the sanitization now covers both legs, and the tests pin whole composed sentences in both languages — including the join separators that every single-entry fixture left unpinned until now. CI is green on the reviewed commit with nothing pending. This is the kind of change that makes the bot's posted bodies read like someone proofed them; shipping it. 中文说明置信度:5/5——各阶段全部干净,毫不犹豫可合入。 缺陷是对照实际发布的正文核实的,而非凭空采信——重复句与未翻译的中文半区都原样出现在 review 5061446682 中。修复针对成因而非表象:原因不再重复包裹句已经说过的话,唯一承担得起真实译文的通道获得了译文,承担不起的通道(caller 自由文本)不再假装翻译,而是如实标注。兼容性在两个方向上都成立,消毒如今覆盖两条腿,测试整句钉住两种语言的合成结果——包括此前单条目 fixture 一直未能钉住的 join 分隔符。该 commit 的 CI 全绿且无待跑项。这类改动让 bot 发布的正文读起来像有人校对过;合入。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qqqys
left a comment
There was a problem hiding this comment.
Independent verification at head dbf0be21 (review-only pass, no code changes):
- The
scriptLintGateshape change (disclosed→{en, zh}pairs) is contained: the only consumers are the single render site (both halves mapped/joined) and the push site that spreads the gate's array; the two other callers read.criticalsonly. - Sanitization survives the split —
stripCommentGrammarapplies to the English reason and toreasonZh, and the missing-reasonZhfallback reuses the already-stripped English reason; the tests pin newline/backtick/mention/comment-grammar inertness on BOTH halves. - Cap semantics unchanged: disclosed entries still never feed the verdict cap.
- Older reports without
reasonZhstill render (English both halves) since the field is optional.
CI green at this head and ci-bot has approved, so approving from my side as well.
|
Released in v0.23.0. |
What this PR does
Makes the review body's disclosure sentences read cleanly in both languages. The deferred-checker disclosure no longer stutters: script-lint's deferral/skip reasons drop the
— not lintedtail that was written for a standalone context, and the disclosure drops the circularthe executable-script lint —prefix, so the body's "Not linted:" wrapper states the fact once and the path and reason carry the rest. The body's Chinese half stops presenting untranslated English as its translation: the machine-built deferral disclosure becomes genuinely bilingual (scriptLintGatereturns{en, zh}pairs, the report schema gains an optionalreasonZhwhich the actionlint deferral supplies, and a report from an older CLI falls back to the English reason in both halves), while caller-prose "Not reviewed" entries — untranslatable free text by construction — get an honest Chinese label,未审查(原文为英文):, and keep their own English full stop instead of closing an English sentence with。.Why it's needed
Both defects are visible verbatim in the review bot's posted round-1 body on #10567 (review 5061446682). It carried "Not linted (tool limitation, not a blocker): the executable-script lint —
.github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted." — three layers each completing the sentence on their own — and its 中文说明 block rendered the same English sentences with only the labels translated, so the Chinese half added nothing and read like broken translation machinery.Reviewer Test Plan
How to verify
Run the review-command suites from
packages/cli(no build needed):npx vitest run src/commands/review/script-lint.test.ts src/commands/review/script-lint.mock.test.ts src/commands/review/compose-review.test.ts. The composed sentences are pinned whole incompose-review.test.ts("a DEFERRED-only report keeps APPROVE…" and "two deferred entries join per language…"): a deferral must render asNot linted (tool limitation, not a blocker):.github/workflows/ci.yml— source mapping not yet supported.with the Chinese half未检查(工具限制,非阻断):.github/workflows/ci.yml——尚未支持源映射。whenreasonZhis present, and a caller-prose gap must render its Chinese line as未审查(原文为英文):…(pinned in "the marker does not shadow other reverse-audit scopes…"). Each pin was additionally checked by mutation: re-appending either— not lintedtail, substituting the English reason forreasonZh, droppingstripCommentGrammarfrom thereasonZhleg, or altering either join separator turns exactly the corresponding assertion red while the intact source stays green.Evidence (Before & After)
Before (posted on #10567, review 5061446682):
After (pinned by the wired body tests):
Tested on
Environment (optional)
Unit tests only — vitest run from
packages/cli; no runtime or sandbox involved.Risk & Scope
string[]→{en, zh}[]), but it is consumed only bycomposeReview's deferred block; the cappingunreviewedchannel deliberately stays string-typed.reasonZhis optional in the report schema, so a report written by an older CLI still renders (English in both halves).compose-review.ts.Linked Issues
Prompted by the posted review body on #10567 (review 5061446682); no issue to close.
中文说明
本 PR 做了什么
让 review 正文的披露语句在两种语言下都能干净地阅读。deferred-checker 披露不再重复:script-lint 的 deferral/skip 原因去掉了为独立语境写的
— not linted尾巴,披露条目去掉了循环的the executable-script lint —前缀,正文的 "Not linted:" 包裹句只陈述一次事实,路径和原因承载其余信息。正文的中文半区不再把未翻译的英文当译文呈现:机器构建的 deferral 披露变为真正双语(scriptLintGate返回{en, zh}对,report schema 新增可选reasonZh,actionlint deferral 会提供该字段,旧版 CLI 写的 report 两半都回退英文原因);而 caller 自由文本的 "Not reviewed" 条目本质上不可翻译,中文标签如实标注为未审查(原文为英文):,并保留英文句号,不再以。结束英文句子。为什么需要
两个缺陷都原样出现在 review bot 于 #10567 发布的第 1 轮正文中(review 5061446682)。该正文出现了 "Not linted (tool limitation, not a blocker): the executable-script lint —
.github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted." ——三层拼接各自补全了一遍句子——而其中文说明块只翻译了标签、解释部分原样保留英文,中文半区没有信息增量,读起来像翻译机制坏了。审查者测试计划
如何验证
在
packages/cli下运行 review 命令的测试套件(无需构建):npx vitest run src/commands/review/script-lint.test.ts src/commands/review/script-lint.mock.test.ts src/commands/review/compose-review.test.ts。合成句在compose-review.test.ts中被整句钉住("a DEFERRED-only report keeps APPROVE…" 与 "two deferred entries join per language…"):deferral 必须渲染为Not linted (tool limitation, not a blocker):.github/workflows/ci.yml— source mapping not yet supported.,reasonZh存在时中文半区为未检查(工具限制,非阻断):.github/workflows/ci.yml——尚未支持源映射。;caller 自由文本缺口的中文行必须渲染为未审查(原文为英文):…(在 "the marker does not shadow other reverse-audit scopes…" 中钉住)。每个钉住都另经变异验证:把任一— not linted尾巴加回、用英文原因替换reasonZh、从reasonZh分支移除stripCommentGrammar、或改动任一 join 分隔符,都会让对应断言变红,而未变异源码保持全绿。证据(Before & After)
Before(#10567 上已发布的 review 5061446682):
After(由 wired body 测试钉住):
测试平台
环境(可选)
仅单元测试——在
packages/cli下运行 vitest;不涉及运行时或沙箱。风险与范围
string[]→{en, zh}[]),但其唯一消费方是composeReview的 deferred block;封顶通道unreviewed有意保持字符串类型。report schema 中reasonZh为可选字段,旧版 CLI 写的 report 仍可渲染(两半均为英文)。compose-review.ts的内部实现。关联 Issue
由 #10567 上已发布的 review 正文(review 5061446682)触发;无需关闭的 issue。