fix(triage): exclude test files from core module size gate and distinguish feat from refactor - #6369
Conversation
…guish feat from refactor - Add anti-hallucination rule to SKILL.md preventing invented blocking policies - Stage 0 size calculation now excludes test files (*.test.ts, *.spec.ts, __tests__/) - Only production logic lines count toward the 500-line threshold - feat-type PRs touching core escalate instead of hard-blocking - Add soft large-PR advisory (non-blocking) for >1000 production lines - Update AGENTS.md to match refined policy - Clarify conventional commit matching patterns for feat/refactor detection Closes #6365
|
Thanks for the PR! Template looks good ✓ — all required headings present including bilingual section. Problem: Observed — the triage bot was false-blocking legitimate Direction: Aligned. Distinguishing feat (additive, incrementally reviewable) from refactor (restructuring) is the right conceptual split — they have fundamentally different risk profiles. The anti-hallucination rule is a well-scoped defensive addition. Size: Not applicable — this PR touches only Approach: Scope is tight and every edit is necessary. Three focused changes (test exclusion patterns, feat/refactor distinction, anti-hallucination rule) plus a non-blocking 1000+ advisory. The new The one open item from review — hard-blocking Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ — 所有必需标题和双语部分均存在。 问题:已观测到——triage bot 将测试文件计入 500 行阈值,并对功能新增使用了仅适用于重构的语言,误阻塞了合理的 方向:对齐。区分 feat(增量添加,可增量审查)和 refactor(重构)是正确的概念拆分——两者的风险特征本质不同。反幻觉规则是范围明确的良好防御性补充。 规模:不适用——此 PR 仅修改 方案:范围紧凑,每个编辑都是必要的。三个聚焦的改动(排除测试模式、区分 feat/refactor、反幻觉规则)加上非阻塞的 1000+ 建议。Stage 1 模板中新增的 审查中的一个开放项——对触及 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading diff): To fix the triage bot's false-positive blocking of Diff review:
Consistency check: All three files agree on the exclusion patterns, the Reuse check: N/A — configuration/documentation, no reusable code logic. Review feedback addressed: The three suggestions from @wenshao's review — No critical blockers or AGENTS.md violations found. Real-Scenario TestingN/A — this PR modifies triage skill configuration and AGENTS.md documentation only. No CLI behavior changes to test in tmux. — Qwen Code · qwen3.7-max |
|
This is a clean, well-scoped config fix for a real and observed problem. The triage bot was blocking legitimate The feat/refactor distinction is the right conceptual split. New code (additive, incrementally reviewable) and restructured code (changes the foundation) have genuinely different risk profiles, and the gate should reflect that. The anti-hallucination rule is a tightly scoped defensive addition that references only the escalation criteria already defined in the workflow. The new "Stage 0 escalation prevents auto-approval" clause in Stage 3 closes a real gap. Review feedback was addressed: All three files (SKILL.md, pr-workflow.md, AGENTS.md) are consistent. The exclusion patterns, the refactor-only hard block, and the feat/other escalation path all match across files. One minor note for future maintenance: All stages clean, review feedback addressed, guardrail passes. Approving. ✅ 中文说明这是一个干净、范围明确的配置修复,解决了一个已观测到的真实问题。triage bot 将测试代码计入行数门控,误阻塞了合理的 feat/refactor 的区分是正确的概念拆分。新代码(增量添加,可增量审查)和重构代码(改变基础)的风险特征确实不同,门控应该反映这一点。反幻觉规则是严格限定在工作流中已定义升级标准的防御性补充。Stage 3 中新增的"Stage 0 升级阻止自动批准"条款填补了一个实际漏洞。 审查反馈已解决: 三个文件(SKILL.md、pr-workflow.md、AGENTS.md)保持一致。排除模式、仅 refactor 硬阻塞、feat/其他升级路径在文件间完全匹配。 一个小的后续维护注意: 所有阶段通过,审查反馈已解决,护栏检查通过。批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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. |
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
.qwen/skills/triage/references/pr-workflow.md:64, AGENTS.md:33 |
Test-exclusion patterns omit .tsx variants (*.test.tsx, *.spec.tsx). This repo has many React/Ink test files (e.g. packages/cli/src/ui/App.test.tsx, packages/core/src/providers/__tests__/). A core PR bundling .test.tsx files would have those lines counted toward production logic. Additionally, __tests__/** (pr-workflow.md) vs __tests__/ (AGENTS.md) differ in glob semantics. |
Add *.test.tsx and *.spec.tsx to both files; standardize on __tests__/**. |
.qwen/skills/triage/references/pr-workflow.md:64, AGENTS.md:33 |
"Auto-generated schema declarations" is excluded from size calculation but never defined — no glob pattern, no file convention, no marker. The repo has almost no auto-generated files (only one *.schema.json), so the term is ambiguous and will lead to inconsistent line counts. |
Replace with a concrete pattern (e.g. **/*.schema.json, **/*.generated.ts) or remove the exclusion if nothing in the repo qualifies. |
.qwen/skills/triage/references/pr-workflow.md:74 |
The feat exemption assumes "features add new code; refactors restructure" — but a feat adding 500+ lines to auth/, sandbox/, or providers/ carries architectural risk comparable to a refactor. These areas get only a non-blocking awareness flag. |
Consider treating feat PRs touching auth, sandbox, or providers paths the same as refactor for the hard block, since new security-critical code is as risky as restructured code. |
— glm-5.2 via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Suggestion] The Stage 1 comment template (lines ~183-210) was not updated to include a designated field for the new size-reporting obligations introduced by this paragraph (production-vs-test-vs-schema breakdown, 500+ feat escalation flag, 1000+ line advisory). Without a Size: field in the template, these reporting requirements are likely to be omitted or buried in unrelated sections, making them effectively unenforceable.
Suggested fix: add a Size: field to the Stage 1 template.
— qwen3.7-max via Qwen Code /review
|
Addressed the bounded doc items:
Leaving the auth/sandbox/providers hard-block suggestion as follow-up: that changes the policy boundary for |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM — review feedback addressed, all stages clean. Ready to ship. ✅
| files but changes a line or two each is escalated to a maintainer for | ||
| awareness and otherwise judged under Tier 2's 100%-confidence bar, not | ||
| auto-rejected on file count. | ||
| 2. **Small-scope changes → gate may evaluate, but must be 100% confident.** |
There was a problem hiding this comment.
[Suggestion] Tier 2 label not updated to match pr-workflow.md. AGENTS.md still says "Small-scope changes" while pr-workflow.md correctly updated to "Changes to core not blocked by Tier 1". After this PR, a feat PR with 800 production lines in core reaches Tier 2 — which is not "small-scope."
| 2. **Small-scope changes → gate may evaluate, but must be 100% confident.** | |
| 2. **Changes to core not hard-blocked by Tier 1 → gate may evaluate, but must be 100% confident.** |
— qwen3.7-max via Qwen Code /review
| If `GUARD` is `block`: do **not** run `gh pr review --approve` no matter how clean every stage looked. Escalate to the maintainer instead (the "Genuinely unsure" path below, using `$QWEN_MAINTAINER_HANDLE` if set), and only `--request-changes` if you actually found blocking issues. This overrides the "approve" path. | ||
|
|
||
| All stages genuinely clean **and** `GUARD` is `ok` — approve: | ||
| If Stage 0 escalated the PR for maintainer awareness, do **not** approve automatically; use the "Genuinely unsure" path below. |
There was a problem hiding this comment.
[Suggestion] Stage 0 escalation state has no deterministic check at Stage 3. Unlike the GUARD above (a gh pr view --json + jq expression producing block/ok), this rule is prose-only — no bash snippet, no variable, no marker. An LLM processing Stage 3 (especially on a re-run) has no reliable way to recall Stage 0's decision. The existing GUARD script could be widened to also block when production lines ≥ 500 on a same-repo non-refactor PR, providing a deterministic backstop:
GUARD=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json isCrossRepository,title,additions,deletions \
--jq 'if (.isCrossRepository and (.title | test("^\\s*refactor"; "i"))) then "block"
elif (.title | test("^\\s*refactor"; "i") | not) and .additions + .deletions >= 500 then "escalated"
else "ok" end')— qwen3.7-max via Qwen Code /review
| All stages genuinely clean **and** `GUARD` is `ok` — approve: | ||
| If Stage 0 escalated the PR for maintainer awareness, do **not** approve automatically; use the "Genuinely unsure" path below. | ||
|
|
||
| All stages genuinely clean, `GUARD` is `ok`, and no Stage 0 maintainer escalation remains — approve: |
There was a problem hiding this comment.
[Suggestion] No escalation-clearance path defined. The phrase "no Stage 0 maintainer escalation remains" implies escalation can cease — but the PR provides no criteria for when that happens. If a maintainer replies approvingly to the Stage 1 comment, does the escalation clear? Without a defined exit, the "Genuinely unsure" path could loop: agent escalates → maintainer responds → agent re-escalates because the rule still says "do not approve automatically."
Consider adding: "Escalation is considered resolved when a maintainer has left an approving review or explicitly endorsed the PR in a comment."
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM — re-triage complete, review feedback addressed, all stages clean. Ready to ship. ✅
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Changes align with #6365 root cause — test exclusion, feat/refactor distinction, and anti-hallucination guard are all targeted fixes. The 'other PR types' paragraph adds nuance; will monitor live triage runs to confirm the LLM follows the branching correctly.
What this PR does
Refines the triage bot's Stage 0 "Core Module Protection" gate in three ways:
*.test.ts,*.spec.ts,__tests__/**, and auto-generated schema declarations. Only production logic lines count toward the 500-line threshold.feat-type PRs touching core paths escalate to the maintainer for awareness instead of hard-blocking. Onlyrefactor-type PRs trigger the Tier 1 wall.SKILL.mdprohibiting the bot from inventing blocking policies or thresholds not defined in skill files.Why it's needed
The triage bot was blocking legitimate
feat:PRs (e.g. #6354 — 802 total lines but only ~200 production logic) by counting test files toward a raw 500-line threshold and applying refactor-only language to feature additions. A PR that's 55% test code shouldn't trip a core protection gate designed for large structural refactors.Reviewer Test Plan
How to verify
.qwen/skills/triage/references/pr-workflow.md— confirm the test-exclusion clause, feat/refactor split, and advisory are coherent with the rest of the workflow..qwen/skills/triage/SKILL.mdunder## Rules— confirm it's additive and doesn't conflict with existing rules.AGENTS.mdlines 24–49 — confirm it matches the pr-workflow definition.Evidence (Before & After)
N/A — documentation/config-only change. No code, no TUI.
Tested on
Environment (optional)
N/A
Risk & Scope
feat-type PRs — a large feature touching core could now reach Stage 1 review instead of being stopped at Stage 0. Mitigated by the escalation-to-maintainer requirement.refactor-type blocking behavior is unchanged.中文说明
做了什么
优化 triage bot 的 Stage 0 "核心模块保护"门控:
*.test.ts、*.spec.ts、__tests__/**和自动生成的 schema 声明,只统计生产逻辑行数。feat类型 PR 触及核心路径时升级通知维护者而非硬阻塞,仅refactor类型触发 Tier 1 硬墙。SKILL.md中明确禁止 bot 捏造 skill 文件中未定义的阻塞策略或行数阈值。为什么需要
triage bot 把合理的
feat:PR(如 #6354,总共 802 行但仅约 200 行生产逻辑)误判为超阈值并阻塞,原因是将测试文件计入原始 500 行阈值,并对功能新增使用了仅适用于重构的语言。风险
feat类型 PR 的门控——大型功能现在可到达 Stage 1 审查。通过要求升级通知维护者来缓解。