fix(review): judge a CI check by its name's latest run, not by any leftover - #7183
Merged
Conversation
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: Real, observed bug with concrete evidence — PR #7150's Direction: Clearly aligned. The presubmit CI classifier is internal infrastructure for Size: Not applicable — Approach: Focused and minimal. The change does exactly one thing: replaces per-run failure judgment with per-name latest-verdict judgment. The Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的真实 bug，有具体证据——PR #7150 的 方向：明确对齐。presubmit CI 分类器是 规模：不适用—— 方案：聚焦且最小化。改动只做一件事：把按 run 判定失败替换为按名字的最新裁决判定。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
…ftover
classifyCi already asks "did this name run" per NAME -- the routing
workflows re-dispatch a name several times per commit -- but it judged
failure per RUN: any single failing conclusion pushed its name into
failedCheckNames, including the cancelled runs those very re-dispatches
displace and the failed attempt a flaky job leaves behind when re-run to
green. Two real reviews were downgraded from Approve over exactly that
("CI failing: route" at #7150; route, review-pr, review-config and four
more at #7171), each on a commit whose every live check was green on the
PR page.
Failure is now judged by the name's latest verdict, ordered by the runs'
own timestamps -- the same evidence GitHub's PR page shows a human.
Skipped/neutral/stale runs stay non-verdicts, so a re-dispatch that
skipped cannot erase a real failure beside it; a name whose only run was
cancelled still fails, because nothing superseded it; and a re-run that
fails after a success fails, because latest wins in both directions. On
equal or absent timestamps the first-listed run keeps the name (the API
lists newest first).
Collaborator
Code ReviewThe implementation matches what I'd have done: aggregate check runs by name, pick the latest verdict by timestamp, judge failure from that single winner. The The loop correctly preserves The No critical issues. No AGENTS.md violations. The change is 25 production lines in one file with 7 new test cases (20 total in the file) covering all relevant edge cases. Real-Scenario TestingRan Both before and after classify PR #7171's current head as
Also ran the full review suite (31 files, 729 tests) — all green, no regressions. 中文说明代码审查实现方案与我的独立方案一致：按名字聚合 check runs，按时间戳选最新裁决，从单一胜出者判定失败。 循环正确保留了非完成 run 的
无关键问题。无 AGENTS.md 违规。改动是一个文件中的 25 行生产代码，7 个新测试用例（文件共 20 个测试）覆盖所有相关边界。 真实场景测试在修复分支和 main 分支上分别对 PR #7171 的 head 运行了 完整 review 测试套件（31 文件，729 测试）也全部通过，无回归。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 4/5 — Clean fix for a real, well-documented bug. The cancelled-then-success pattern that caused the false alarms at #7150 and #7171 is correctly resolved. Minor note: live testing against PR #7171's head shows 3 of 7 names still fail (cancelled as only verdict run, later runs all skipped) — a different edge case this PR doesn't target, not a regression. The code is straightforward — one helper, one refactored loop, 25 production lines. The test suite is thorough: 6 new cases covering the target pattern and its mirror cases (latest wins both ways, lone cancelled still fails, skips don't erase failures). Real-scenario testing against the live API data confirms the fix works as designed. The one thing worth watching: if the cancelled-then-only-skipped pattern starts causing false alarms in practice (a routing job gets cancelled and re-dispatched but the re-dispatch skips all legs), that would be a natural follow-up. But it's a separate problem with a different shape, and bundling it here would have bloated a focused fix. LGTM, approving. ✅ 中文说明置信度: 4/5 — 针对真实、记录充分的 bug 的干净修复。导致 #7150 和 #7171 误报的 cancelled-then-success 模式被正确解决。小提示：对 PR #7171 head 的实际测试显示 7 个名字中仍有 3 个失败（cancelled 是唯一裁决 run，后续 run 全为 skipped）——这是本 PR 不针对的不同边界情况，不是回归。 代码简洁——一个辅助函数，一个重构循环，25 行生产代码。测试套件充分：6 个新用例覆盖目标模式及其镜像情况（最新双向生效、孤立 cancelled 仍失败、skip 不抹失败）。对实际 API 数据的真实场景测试确认修复按设计工作。 值得关注的：如果 cancelled-then-only-skipped 模式在实践中开始产生误报（路由工作被 cancelled 并 re-dispatch 但 re-dispatch 跳过了所有分支），那将是一个自然的后续。但这是一个不同形状的问题，在这里捆绑会使聚焦的修复变得臃肿。 批准。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
previously approved these changes
Jul 18, 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. ✅
wenshao
force-pushed
the
fix/presubmit-superseded-runs
branch
from
July 18, 2026 16:18
d3b2272 to
688b454
Compare
Contributor
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
Collaborator
Author
|
The All three of my open PRs (#7171, #7173, this one) are now rebased onto current main — suites green locally on the rebased heads (735/731/728) — and pushed; CI is re-running against consistent trees. |
doudouOUC
reviewed
Jul 18, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
The fallback branch had no test: with it dropped, two same-name runs whose completed_at is null collapse to insertion order. The winning run is listed second on purpose -- listed first, the fixture passes with or without the fallback (first-seen keeps the name on equal stamps) and pins nothing; listed second, the success can win only through its started_at.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
@qwen-code /triage |
Collaborator
Author
|
I reviewed this against the live evidence it cites. The per-name latest-verdict model is the right shape and the tests are well designed, but the headline case is only partially cured — the cited payload still classifies as What I verified
Finding 1 — the #7171 false alarm is only partially fixed, and the Evidence section doesn't reproduceRunning the new not
Under "skips are not verdicts", nothing ever supersedes that lone cancellation, so the three names stay red on this commit forever. A review re-run against that head would still downgrade with On why the body's live check likely showed Suggested direction: let Whichever way this lands, the Evidence section needs updating — as written it claims a result the code doesn't produce. Finding 2 — latent: cross-workflow name collisions can now mask a real failure
The old code could only over-report across such a collision; latest-wins can under-report: workflow A's VerdictRight model, good pinning discipline in the tests (the deliberately-ordered fallback fixture in particular), and 4 of the 7 false names are genuinely cured. But since the PR's own cited evidence still trips the downgrade, I'd fold the cancelled-superseded-by-skip shape into this PR (plus the Evidence correction) before merge. 🤖 Generated with Claude Code — Claude Fable 5 |
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. |
Collaborator
|
Confidence: 5/5 This is a textbook bug fix: observed problem, clear root cause, minimal change, thorough tests. The classifier was judging failure by any leftover run instead of the latest verdict per name — exactly the kind of stale-data bug that silently erodes trust in automated reviews. The fix is 25 lines of production code doing exactly one thing, with tests that encode the real-world failure patterns from #7150 and #7171. The CI is green, full review suite is green (729 tests), no regressions. LGTM. ✅ 中文说明信心：5/5 这是一个教科书级的 bug 修复：已观测到的问题、清晰的根因、最小化改动、充分的测试。分类器按任意残留 run 判定失败，而不是按每个名字的最新裁决——正是这种悄悄侵蚀自动化 review 信任度的陈旧数据 bug。修复是 25 行生产代码只做一件事，测试编码了 #7150 和 #7171 的真实失败模式。
CI 全绿，完整 review 测试套件全绿（729 测试），无回归。LGTM。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 18, 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. ✅
wenshao
commented
Jul 18, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
— qwen-latest-series-invite-beta-v92 via Qwen Code /review
ytahdn
approved these changes
Jul 18, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
LGTM ✅
Per-run → per-name-latest 判定修复精准，双向对称（success→failure 同样生效），skipped/neutral/stale 正确保持为非裁决。7 个新测试覆盖所有边界。
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
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
Makes
presubmit's CI classifier judge each check by its name's latest run instead of by any single run.classifyCialready answers "did this name run" per name — because this repo's routing workflows re-dispatch a name several times per commit — but it judged failure per run: any one run with a failing conclusion pushed its name intofailedCheckNames, including thecancelledruns those very re-dispatches displace and the failed attempt a flaky job leaves behind after being re-run to green.Failure is now decided by the latest verdict per name, ordered by the runs' own timestamps (
completed_at, falling back tostarted_at) — the same evidence GitHub's PR page shows a human. Skipped/neutral/stale runs remain non-verdicts, so a re-dispatch that skipped cannot erase a real failure beside it; a name whose only run was cancelled still fails, because nothing superseded it; and a re-run that fails after a success fails, because latest wins in both directions. On equal or absent timestamps the first-listed run keeps the name (the API lists newest first).Why it's needed
Two real reviews were downgraded from Approve over checks that were green on the PR page:
⚠️ Downgraded from Approve to Comment: CI failing: routewhileroute's latest runs weresuccess— the "failure" was onecancelledrun displaced by a newer dispatch.route, review-pr, delay-automatic-review, resolve-pr, ack-review-request, authorize, review-config) — every one green or skipped at its latest run, every one carrying a displacedcancelledleftover.A wrong
CI failingreason doesn't just misreport: it downgrades the verdict the review posts, and it sends whoever reads the review chasing failures that do not exist.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review/presubmit.test.ts— 19 tests green, including the new describe block: the feat(review): build every Step 5 auditor of a round in one call, identity first #7150/feat(review): bake the round label into a findings role's identity line and key #7171 shape (cancelled displaced by later success →all_pass), flaky-rerun-to-green →all_pass, success-then-failure →any_failure(latest wins both ways), lone-cancelled →any_failure, later-skip-does-not-erase-failure, and the no-timestamps order fallback.npx vitest run src/commands/review— 31 files, 728 tests green.gh api repos/QwenLM/qwen-code/commits/37131de4d/check-runs --paginate(PR feat(review): bake the round label into a findings role's identity line and key #7171's head) showsroutewith success×2 + cancelled×1 and six more names in the same shape — the exact fixture the regression test encodes.Evidence (Before & After)
Before:
classifyCion #7171's head check-runs returnsany_failurewithfailedCheckNames: [ack-review-request, authorize, delay-automatic-review, resolve-pr, review-config, review-pr, route], and the review is downgraded from Approve.After: the same input returns
all_pass— matching the PR page, where every one of those checks shows green or skipped.Tested on
Risk & Scope
/statuscontexts) is untouched — that endpoint already returns the latest status per context.Linked Issues
Third sighting of this false alarm (route at #7150, seven names at #7171), both documented in those PRs' threads.
中文说明
本 PR 做了什么
让
presubmit的 CI 分类器按每个检查名字的最新一次 run 裁决,而不是被任意一次 run 左右。classifyCi对"该名字是否运行过"早已按名字聚合(本仓库的路由工作流会对同一名字反复 re-dispatch),但失败判定却按单次 run:任何一次带失败 conclusion 的 run 都会把名字推进failedCheckNames——包括恰恰被那些 re-dispatch 取代的cancelledrun,以及 flaky 任务重跑变绿后留下的失败尝试。现在失败由该名字的最新裁决决定,按 run 自身时间戳排序(
completed_at,回退started_at)——与 GitHub PR 页面展示给人类的证据一致。skipped/neutral/stale 仍是"非裁决":跳过的 re-dispatch 不能抹掉旁边的真失败;唯一一次 run 就是 cancelled 的名字仍然算失败(没有任何东西取代它);成功后重跑失败也算失败(最新裁决双向生效)。时间戳相等或缺失时,先列出的 run 保留名字(API 按最新在前排列)。为什么需要
两次真实 review 因为 PR 页面上明明是绿的检查被从 Approve 降级:
⚠️ Downgraded from Approve to Comment: CI failing: route,而route最新的 run 全是success——所谓失败是一次被新 dispatch 取代的cancelled。cancelled残留。错误的
CI failing不止是误报:它会降级 review 发布的 verdict,并让读 review 的人去追根本不存在的失败。验证方式
cd packages/cli && npx vitest run src/commands/review/presubmit.test.ts—— 19 测试全绿,新增描述块覆盖:feat(review): build every Step 5 auditor of a round in one call, identity first #7150/feat(review): bake the round label into a findings role's identity line and key #7171 形状(cancelled 被后来的 success 取代 →all_pass)、flaky 重跑变绿 →all_pass、成功后重跑失败 →any_failure(最新双向生效)、孤立 cancelled →any_failure、后续 skip 不抹真失败、无时间戳时的顺序回退。npx vitest run src/commands/review—— 31 文件 728 测试绿。gh api repos/QwenLM/qwen-code/commits/37131de4d/check-runs --paginate(feat(review): bake the round label into a findings role's identity line and key #7171 的 head)可见routesuccess×2 + cancelled×1 及六个同形名字——正是回归测试编码的 fixture。证据(前后对照)
修复前:对 #7171 head 的 check-runs,
classifyCi返回any_failure,failedCheckNames列出七个路由检查,review 被从 Approve 降级。修复后:同一输入返回
all_pass——与 PR 页面一致(那些检查全部显示绿或 skipped)。风险与范围
/statuscontexts)路径未动——该端点本身就按 context 返回最新状态。关联
同类误报第三次目击(#7150 的 route、#7171 的七个名字),均记录在对应 PR 线程。