fix(review): deprioritize Maven generated test sources - #8405
Merged
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the follow-up! Template looks good ✓ Problem: real and substantiated. The Java heading prioritization shipped this morning in PR 8379 classifies Direction: aligned — direct follow-up to a feature merged today, fixing a gap found in its own review. Internal Size: not core modules ( Approach: exactly the minimal change — one Risk: no elevated risk signals — neither file matches the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢这个 follow-up！ 模板完整 ✓ 问题：真实且有依据。今早随 PR 8379 上线的 Java 标题路径优先级会把 方向：对齐——是对今天刚合并功能的直接跟进，修复的是该功能自身 review 中发现的缺口。属于 规模：非核心模块（ 方案：恰好是最小改动——在 风险：无升级风险信号——两个文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code reviewIndependent baseline first: given only the title and motivation, the obvious fix is to teach the The regex change is correct. The new The regression case is load-bearing, not ceremonial. It feeds eleven generated-test paths plus one production path through No blockers, no convention violations, nothing to flag. TestingUnattended CI run — no PR code was built or executed here; the evidence below is the PR's own CI on the reviewed commit, read via the API. At fetch time the one check that pins this change — Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Bot-orchestration checks (triage, precheck-pr, resolve-pr, label, authorize, tmux/verify gates) omitted from the table — they don't exercise this change. The claim under review is a pure string-classification change, and the added regression test exercises exactly that logic — once the ubuntu suite lands green, CI substantiates it; no sandboxed lane is needed to settle a deterministic unit-tested classifier. 中文说明代码审查先说独立基线：只看标题和动机，最直接的修法就是让 正则改动是正确的。新增的 回归用例是真正起作用的，不是摆设。它把 11 条生成测试路径加 1 条生产路径送入 无阻塞项，无规范违规，没有其他需要指出的问题。 测试无人值守 CI 运行——此处未构建或执行任何 PR 代码；以下证据是通过 API 读取的、该 PR 自身在被审 commit 上的 CI 结果。抓取时，唯一能钉住本改动的检查—— （CI 明细表见上方英文部分；机器人编排类检查未列入。） 本次审查的结论是一个纯字符串分类改动，而新增回归用例恰好验证的就是这段逻辑——待 ubuntu 套件变绿后，CI 即可证实该结论；对一个有确定性单测覆盖的分类器，无需沙箱验证通道来定论。 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 5/5 — a four-line fix for a real gap found in the review of a feature that merged this morning, carrying a regression test that genuinely pins the change. Honest read: I verified the problem against the base code rather than accepting the PR's framing — the shipped classifier really does miss Verdict: approve — but CI is still in flight. The unit suite that pins this change ( 中文说明置信度：5/5 —— 用 4 行改动修复了今早合并功能在 review 中被发现的真实缺口，且附带真正能钉住改动的回归测试。 坦率地说：我没有直接接受 PR 的叙述，而是在基线代码上验证了问题——现有的分类器确实匹配不到 结论：approve——但 CI 仍在运行。能钉住本改动的单测套件（ — Qwen Code · qwen3.8-max-preview Reviewed at |
Contributor
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. |
qwen-code-ci-bot
approved these changes
Aug 3, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
approved these changes
Aug 3, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
Test Plan (not a blocker): 43 tests pass — this review observed 16989 passed; 43 passed — this review observed 16989 passed.
中文说明
未发现问题。LGTM！✅
Test Plan（非阻断）：43 tests pass — this review observed 16989 passed; 43 passed — this review observed 16989 passed。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
Collaborator
Author
ReviewOverviewAdds 4 production lines, 8 test lines. Direct follow-up to #8379. Correctness — verifiedI reproduced the classifier and
So the new row is genuinely load-bearing — and specifically it is Confirmed no other branch of Sibling shapes spot-checked against the new regex — all classify as expected:
Alternation ordering is safe: JS backtracks across alternatives, so Coverage assessmentFor the paths that can actually reach this rule ( Suggestions (non-blocking)1. The alternation is now collapsible — The three branches share a prefix, and collapsing them also undoes the Prettier multi-line wrap the addition forced: /(?:^|\/)(?:target|build)\/generated(?:-(?:test-)?sources)?\//i.test(p)I verified this is behaviourally identical to the PR's regex across the cases above plus 2. Its only consumer is Conventions / style / risk
VerdictLGTM. Minimal, correct, and the regression test fails without the production change. The two suggestions are optional. 中文概述在 正确性 —— 已验证我把分类器和 边界形态抽查全部符合预期：多模块 覆盖面对于能触发该规则的路径（仅 建议（非阻塞）
规范 / 风险风格、许可头、 结论LGTM。改动最小、正确，且回归用例在没有生产代码改动时会失败。两条建议均为可选。 |
yiliang114
approved these changes
Aug 3, 2026
Collaborator
|
Released in v0.21.4. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Treats Maven Compiler Plugin generated test sources as generated output when
/reviewprioritizes Java paths for the rule heading. It also adds a cap-sensitive regression case with more than ten generated test paths plus one production path.Why it's needed
Maven writes generated test annotation sources under
target/generated-test-sources/test-annotations/**. These paths were previously treated as production sources, so a generated-test-heavy change could consume all ten named heading slots and hide the actual production Java path behind the truncation count.Reviewer Test Plan
How to verify
Run
cd packages/cli && npx vitest run src/commands/review/lib/path-rules.test.ts. Confirm all 43 tests pass. In the Maven generated test output case, confirm the heading retainsHot.javaand truncates a generated test path after the ten-path cap. Removing thegenerated-test-sourcesclassifier alternative should make that regression test fail because the generated paths occupy every named slot.Evidence (Before & After)
N/A — non-UI path-prioritization change.
Tested on
Environment (optional)
macOS Darwin, Node.js 22.22.2. Focused Vitest suite: 43/43 passed. Prettier, ESLint, and
git diff --checkpassed. Repository and CLI build/typecheck were attempted but are currently blocked by unrelated baseline dependency and type errors in channel, ACP, and Ink selection code.Risk & Scope
targetorbuilddirectory namedgenerated-test-sourcesis deprioritized in the Java rule heading; this is consistent with treating build output as non-production source.Linked Issues
Follow-up to #8379 and #8379 (comment).
中文说明
本 PR 做了什么
当
/review为 Java 规则标题排列路径优先级时，将 Maven Compiler Plugin 生成的测试源码视为生成产物。同时增加一个对上限敏感的回归用例：超过十条生成测试路径，再加一条生产路径。为什么需要
Maven 会把测试注解处理生成的源码写入
target/generated-test-sources/test-annotations/**。此前这些路径会被当成生产源码，因此当变更包含大量生成测试文件时，它们可能占满标题中十个可见路径槽位，把真正的生产 Java 路径隐藏在截断计数之后。Reviewer Test Plan
如何验证
运行
cd packages/cli && npx vitest run src/commands/review/lib/path-rules.test.ts，确认 43 个测试全部通过。在 Maven generated test output 用例中，确认标题保留Hot.java，并在十条路径上限之后截断生成测试路径。删除分类器中的generated-test-sources分支后，该回归测试应失败，因为生成路径会占满全部可见槽位。证据（Before & After）
N/A — 非 UI 的路径优先级改动。
测试平台
环境（可选）
macOS Darwin，Node.js 22.22.2。定向 Vitest：43/43 通过。Prettier、ESLint 和
git diff --check通过。已尝试仓库及 CLI 的 build/typecheck，但目前被 channel、ACP 和 Ink selection 代码中与本改动无关的基线依赖及类型错误阻断。风险与范围
target或build目录下、名为generated-test-sources的路径会在 Java 规则标题中降级；这与将构建产物视为非生产源码的现有行为一致。关联问题
#8379 的 follow-up，对应 https://github.com/QwenLM/qwen-code/pull/8379#discussion_r3700729787。