ci: split model vars for autofix, triage, and review workflows - #7282
ci: split model vars for autofix, triage, and review workflows#7282yiliang114 wants to merge 3 commits into
Conversation
|
Superseded by clean branch based on latest main |
ReviewNoting up front that this PR is closed and superseded. Its diff is two unrelated bodies of work, which now live separately:
I diffed both: the classifier here differs from #7251's head in exactly one place (the file-predicate ordering), and #7283 is line-for-line identical to the model half here, just rebased. So the findings below still apply to the live PRs — targets noted per item. Verified as correct
Findings1. [Medium — #7251] Verified: Sizing it against history: of the last 300 commits on Suggest either widening the type gate to 2. [Medium-Low — #7251] The file-predicate reorder changed behavior and is untested Between this PR and #7251's head the predicate became 3. [Low — #7251]
4. [Low — #7251] Combined with 5. [Low — #7251]
6. [Nit — #7251]
7. [Low — #7283] The body says "all three CI workflows", but five share the variable On
|
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
left a comment
There was a problem hiding this comment.
Hi @yiliang114, thanks for the PR!
This PR doesn't follow the PR template. Several required sections are missing:
## What this PR doesand## Why it's needed(currently## Motivation/## Changes— close, but the template headings help reviewers find info quickly)## Reviewer Test Planwith### Evidence (Before & After)and### Tested ontable## Risk & Scope## Linked Issues
Could you restructure the PR body to follow the template? It helps reviewers and maintainers process PRs faster.
Also — the PR title says "split model vars for autofix, triage, and review workflows" but the diff includes an entirely separate feature: a release notes classifier (classify-release-notes.mjs, classify-release-notes.yml, test file, release.yml update) totaling ~170 production lines and ~170 test lines. This isn't mentioned in the PR body at all. These look like two independent changes that would be easier to review and revert as separate PRs. Could you split them?
中文说明
感谢提交!
这个 PR 没有遵循 PR 模板,缺少以下必要部分:
## What this PR does和## Why it's needed(当前用的是## Motivation/## Changes,接近但模板标题有助于 review 者快速定位信息)## Reviewer Test Plan(含### Evidence (Before & After)和### Tested on表格)## Risk & Scope## Linked Issues
请按照模板重新组织 PR 正文。
另外,PR 标题写的是"split model vars for autofix, triage, and review workflows",但 diff 中包含了一个完全不同的功能:release notes 分类器(classify-release-notes.mjs、classify-release-notes.yml、测试文件、release.yml 更新),总计约 170 行生产代码和约 170 行测试代码。这在 PR 正文中完全没有提到。这看起来是两个独立的改动,拆成两个 PR 会更容易 review 和回滚。
— Qwen Code · qwen3.7-max
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
| GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}' | ||
| OPENAI_API_KEY: '${{ secrets.AUTOFIX_OPENAI_API_KEY }}' | ||
| OPENAI_BASE_URL: '${{ secrets.AUTOFIX_OPENAI_BASE_URL || secrets.OPENAI_BASE_URL }}' | ||
| OPENAI_MODEL: '${{ vars.QWEN_PR_REVIEW_MODEL }}' | ||
| OPENAI_MODEL: '${{ vars.QWEN_AUTOFIX_MODEL || vars.QWEN_PR_REVIEW_MODEL }}' |
There was a problem hiding this comment.
[Suggestion] The fallback expression vars.QWEN_AUTOFIX_MODEL || vars.QWEN_PR_REVIEW_MODEL is repeated across 6 step-level env: blocks in this workflow. The workflow-level env: block (line 89) already consolidates an identical-shaped vars.* fallback (AUTOFIX_BOT), so this one could live there too. — Concrete cost: if the fallback chain changes (variable renamed, third fallback added, or shared variable retired), all 6 sites must be updated in lockstep; missing one leaves a single job resolving a different model, producing silent model-version skew. The two different env-var names (OPENAI_MODEL at 4 sites, MODEL at 2) make a grep-and-replace slightly less trivial.
| GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}' | |
| OPENAI_API_KEY: '${{ secrets.AUTOFIX_OPENAI_API_KEY }}' | |
| OPENAI_BASE_URL: '${{ secrets.AUTOFIX_OPENAI_BASE_URL || secrets.OPENAI_BASE_URL }}' | |
| OPENAI_MODEL: '${{ vars.QWEN_PR_REVIEW_MODEL }}' | |
| OPENAI_MODEL: '${{ vars.QWEN_AUTOFIX_MODEL || vars.QWEN_PR_REVIEW_MODEL }}' | |
| AUTOFIX_MODEL: '${{ vars.QWEN_AUTOFIX_MODEL || vars.QWEN_PR_REVIEW_MODEL }}' |
Add this to the workflow-level env: block, then reference ${{ env.AUTOFIX_MODEL }} in each step.
— qwen3.8-max-preview via Qwen Code /review
Motivation
All three CI workflows (autofix, triage, review) shared a single
QWEN_PR_REVIEW_MODELvariable, making it impossible to run different models for different tasks. This change splits them into independent variables with fallback to the shared one.Changes
qwen-autofix.yml: usevars.QWEN_AUTOFIX_MODEL || vars.QWEN_PR_REVIEW_MODELqwen-triage.yml: usevars.QWEN_TRIAGE_MODEL || vars.QWEN_PR_REVIEW_MODELqwen-code-pr-review.yml: unchanged, continues usingvars.QWEN_PR_REVIEW_MODELRepo variables already configured:
QWEN_AUTOFIX_MODEL= qwen3.8-max-previewQWEN_TRIAGE_MODEL= qwen3.8-max-previewQWEN_PR_REVIEW_MODEL= qwen3.7-maxHow to verify
After merge, trigger a review and an autofix run — confirm they pick up different models from the run logs.