-
Notifications
You must be signed in to change notification settings - Fork 3k
perf(ci): run docs-only automatic reviews at medium effort #8648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
3537b25
perf(ci): run docs-only automatic reviews at medium effort
f178af5
perf(ci): address review feedback on the docs-only medium gate
119845d
perf(ci): harden the docs-only gate against round-2 review findings
f87dec5
perf(ci): fix the medium Request-changes swallow and the stale docs b…
2c648b1
perf(ci): never let a failed lookup mint or keep a stale docs badge
e3a20ec
perf(ci): make docs_only_medium three-valued and pin the untested guards
6196fd9
perf(ci): bind the docs badge to the reviewed head and retire it on f…
77a1cd0
chore(ci): merge origin/main into docs-only medium-effort review branch
qwen-code-dev-bot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| #!/usr/bin/env bash | ||
| # Fetch a PR's changed files and classify its CI profile in one step. | ||
| # | ||
| # The jq projection below is the input contract of classify-profile.mjs | ||
| # (it reads `filename`, `status`, `previous_filename` per JSONL entry). | ||
| # Both ci.yml's profile gate and qwen-code-pr-review.yml's docs-only | ||
| # downgrade consume the classification through THIS script, so the contract | ||
| # lives in exactly one place — a divergence between the two call sites once | ||
| # meant the same PR could classify differently in each workflow, silently, | ||
| # because both fall back to `full` on their own errors. | ||
| # | ||
| # Usage: classify-pr-profile.sh <owner/repo> <pr-number> | ||
| # Prints the profile (docs_only | github_ci_only | full) on stdout. | ||
| # Exit codes: 0 classified; 2 file listing failed; 3 classifier failed. | ||
| set -euo pipefail | ||
|
|
||
| repo="${1:?usage: classify-pr-profile.sh <owner/repo> <pr-number>}" | ||
| pr="${2:?usage: classify-pr-profile.sh <owner/repo> <pr-number>}" | ||
|
|
||
| # mktemp + trap, not a fixed name: the self-hosted pool is persistent and | ||
| # shared, so a predictable path is a leftover-file landmine, and ci.yml's | ||
| # gate and the review gate can run concurrently for the same PR — two | ||
| # writers interleaving one JSONL would classify one job against the other | ||
| # job's file list. | ||
| tmp="${RUNNER_TEMP:-${TMPDIR:-/tmp}}" | ||
| files="$(mktemp "${tmp}/classify-pr-${pr}-files.XXXXXX")" | ||
| trap 'rm -f "$files"' EXIT | ||
|
|
||
| if ! gh api --paginate "repos/${repo}/pulls/${pr}/files" \ | ||
| --jq '.[] | {filename, status, previous_filename}' > "${files}"; then | ||
| exit 2 | ||
| fi | ||
|
|
||
| # The list-files endpoint caps at 3,000 entries. A truncated listing can be | ||
| # all docs while an omitted later entry is source, so any mismatch against | ||
| # the PR's own changed-file count conservatively classifies as `full`. | ||
| declared="$(gh api "repos/${repo}/pulls/${pr}" --jq '.changed_files')" || exit 2 | ||
| retrieved="$(wc -l < "${files}")" | ||
|
wenshao marked this conversation as resolved.
|
||
| if [ "${retrieved}" -ne "${declared}" ]; then | ||
| echo "classify-pr-profile: retrieved ${retrieved} file entries but PR declares ${declared}; classifying full." >&2 | ||
| echo "full" | ||
| exit 0 | ||
| fi | ||
|
wenshao marked this conversation as resolved.
|
||
|
|
||
| node "$(dirname "$0")/classify-profile.mjs" "${files}" || exit 3 | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2026 Qwen Team | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| // Executes the real classify-pr-profile.sh with a stubbed `gh` (and, for the | ||
| // classifier-failure case, a stubbed `node`) on PATH. The wrapper's own | ||
| // comment declares its jq projection the single home of the classifier's | ||
| // input contract — these tests are what make that claim enforceable: dropping | ||
| // `status` from the projection turns a renamed source→docs file into a plain | ||
|
wenshao marked this conversation as resolved.
|
||
| // docs path (classifyFileEntry consults `previous_filename` only when | ||
| // `status === "renamed"`), which downgraded a source PR in the probe that | ||
| // motivated this file. The exit-code contract (2 listing / 3 classifier) is | ||
| // consumed by both ci.yml and the review workflow's docs-only gate. | ||
|
|
||
| import { test } from 'node:test'; | ||
| import assert from 'node:assert/strict'; | ||
| import { | ||
| chmodSync, | ||
| mkdirSync, | ||
| mkdtempSync, | ||
| rmSync, | ||
| writeFileSync, | ||
| } from 'node:fs'; | ||
| import { execFileSync } from 'node:child_process'; | ||
| import { tmpdir } from 'node:os'; | ||
| import { dirname, join } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| const here = dirname(fileURLToPath(import.meta.url)); | ||
| const wrapper = join(here, 'classify-pr-profile.sh'); | ||
|
|
||
| function run(scenario, { stubNodeFailure = false } = {}) { | ||
| const dir = mkdtempSync(join(tmpdir(), 'classify-pr-profile-')); | ||
| const bin = join(dir, 'bin'); | ||
| mkdirSync(bin); | ||
| const write = (name, body) => { | ||
| const p = join(bin, name); | ||
| writeFileSync(p, body); | ||
| chmodSync(p, 0o755); | ||
| }; | ||
| // The gh stub serves the two calls the wrapper makes: the paginated file | ||
| // listing and the PR object's changed_files count. For the listing it | ||
| // applies the wrapper's OWN `--jq` argument to a full API-shaped fixture | ||
| // with real jq — so the projection (the input contract this wrapper exists | ||
| // to be the single home of) is genuinely under test: dropping `status` or | ||
| // `previous_filename` from it changes what the classifier sees and turns | ||
| // the renamed-source scenario red, instead of the stub hardcoding the | ||
| // projected output and keeping every projection mutant green. | ||
| write( | ||
| 'gh', | ||
| [ | ||
| '#!/bin/bash', | ||
| 'jqfilter=""; prev=""', | ||
| 'for a in "$@"; do if [ "$prev" = "--jq" ]; then jqfilter="$a"; fi; prev="$a"; done', | ||
| 'case "$*" in', | ||
| ' *"/files"*)', | ||
| ' case "$SCENARIO" in', | ||
| ' list-fail) exit 1 ;;', | ||
| ' docs-only) FIXTURE=\'[{"filename":"docs/users/a.md","status":"modified","previous_filename":null,"sha":"x","additions":1},{"filename":"README.md","status":"modified","previous_filename":null,"sha":"y","additions":1}]\' ;;', | ||
| ' renamed-source) FIXTURE=\'[{"filename":"docs/new.md","status":"renamed","previous_filename":"packages/core/src/runtime.ts","sha":"z","additions":0}]\' ;;', | ||
| ' truncated) FIXTURE=\'[{"filename":"docs/users/a.md","status":"modified","previous_filename":null,"sha":"x","additions":1}]\' ;;', | ||
| ' declared-fails) FIXTURE=\'[{"filename":"docs/users/a.md","status":"modified","previous_filename":null,"sha":"x","additions":1}]\' ;;', | ||
| ' *) exit 9 ;;', | ||
| ' esac', | ||
| ' printf \'%s\' "$FIXTURE" | jq -c "$jqfilter" ;;', | ||
| ' *"repos/"*)', | ||
| ' case "$SCENARIO" in', | ||
| ' truncated) echo 5 ;;', | ||
| ' declared-fails) exit 1 ;;', | ||
| ' docs-only) echo 2 ;;', | ||
| ' renamed-source) echo 1 ;;', | ||
| ' *) exit 9 ;;', | ||
| ' esac ;;', | ||
| ' *) exit 9 ;;', | ||
| 'esac', | ||
| 'exit 0', | ||
| ].join('\n') + '\n', | ||
| ); | ||
| if (stubNodeFailure) { | ||
| write('node', '#!/bin/bash\nexit 1\n'); | ||
| } | ||
| try { | ||
| const stdout = execFileSync('bash', [wrapper, 'o/r', '42'], { | ||
| encoding: 'utf8', | ||
| env: { | ||
| ...process.env, | ||
| PATH: `${bin}:${process.env.PATH}`, | ||
| SCENARIO: scenario, | ||
| RUNNER_TEMP: dir, | ||
| }, | ||
| }); | ||
| return { code: 0, stdout: stdout.trim() }; | ||
| } catch (e) { | ||
| return { code: e.status, stdout: `${e.stdout ?? ''}`.trim() }; | ||
| } finally { | ||
| rmSync(dir, { recursive: true, force: true }); | ||
| } | ||
| } | ||
|
|
||
| test('classifies a docs-only listing as docs_only', () => { | ||
| assert.deepEqual(run('docs-only'), { code: 0, stdout: 'docs_only' }); | ||
| }); | ||
|
|
||
| test('a renamed source→docs file classifies full (the projection carries status/previous_filename)', () => { | ||
| assert.deepEqual(run('renamed-source'), { code: 0, stdout: 'full' }); | ||
| }); | ||
|
|
||
| test('a listing shorter than the PR-declared changed_files classifies full (3,000-file cap)', () => { | ||
| assert.deepEqual(run('truncated'), { code: 0, stdout: 'full' }); | ||
| }); | ||
|
|
||
| test('exit 2 when the file listing fails', () => { | ||
| assert.equal(run('list-fail').code, 2); | ||
| }); | ||
|
|
||
| test('exit 3 when the classifier fails', () => { | ||
| assert.equal(run('docs-only', { stubNodeFailure: true }).code, 3); | ||
| }); | ||
|
|
||
| test('exit 2 when the changed_files fetch fails after a successful listing', () => { | ||
| // The truncation guard's precondition: a swallowed failure here leaves | ||
| // `declared` empty and the guard silently skipped (probed mutant | ||
| // `|| exit 2` → `|| true` classified a docs first page as docs_only). | ||
| const r = run('declared-fails'); | ||
| assert.equal(r.code, 2); | ||
| }); | ||
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| #!/usr/bin/env bash | ||
| # Upsert a marker-identified bot comment on a PR/issue. | ||
| # | ||
| # One implementation of the marker+author upsert protocol, shared by the | ||
| # docs-only relay and the stale-badge supersede step in | ||
| # qwen-code-pr-review.yml (the previous per-step copies had already drifted: | ||
| # one had retry/null-guards/dynamic login, the other none). The lookup is | ||
| # author-scoped — only comments by the authenticated login are upsert | ||
| # targets, so a participant posting the marker can never capture the upsert. | ||
| # | ||
| # A FAILED lookup is never treated as an EMPTY result: posting on a failed | ||
| # listing is how a transient 5xx mints a permanent duplicate (later runs | ||
| # PATCH only the `last` match), so every prerequisite — the authenticated | ||
| # login and the listing — is re-resolved inside the retry loop, and an | ||
| # attempt whose prerequisites failed retries instead of falling through to | ||
| # POST. On --update-only, a failed lookup exits 1 (the caller's warning | ||
| # path), never the no-op success reserved for a lookup that genuinely | ||
| # found nothing. | ||
| # | ||
| # Usage: upsert-bot-comment.sh <owner/repo> <issue-number> <marker> <body-file> [--update-only] | ||
| # --update-only: PATCH an existing bot-authored marker comment if present; | ||
| # succeed as a no-op when none exists (never POSTs). For | ||
| # superseding a badge without minting one where none was. | ||
| # Exit codes: 0 posted/updated/no-op; 1 all attempts failed. | ||
| set -euo pipefail | ||
|
|
||
| repo="${1:?usage: upsert-bot-comment.sh <owner/repo> <issue-number> <marker> <body-file> [--update-only]}" | ||
| number="${2:?missing issue number}" | ||
| marker="${3:?missing marker}" | ||
| body_file="${4:?missing body file}" | ||
| update_only="${5:-}" | ||
|
|
||
| body="$(cat "${body_file}")" | ||
|
|
||
| for _attempt in 1 2 3; do | ||
| if bot_login="$(gh api user --jq '.login')" \ | ||
| && [ -n "${bot_login}" ] \ | ||
| && listing="$(gh api "repos/${repo}/issues/${number}/comments" \ | ||
| --method GET \ | ||
| --paginate \ | ||
| -F per_page=100)" \ | ||
| && existing_id="$(printf '%s' "${listing}" \ | ||
| | jq -sr --arg bot "${bot_login}" --arg marker "${marker}" '[.[][] | ||
| | select((.user.login // "") == $bot) | ||
| | select((.body // "") | contains($marker))] | ||
| | last | .id // empty')"; then | ||
|
wenshao marked this conversation as resolved.
|
||
| if [ -n "${existing_id}" ]; then | ||
| if gh api --method PATCH \ | ||
| "repos/${repo}/issues/comments/${existing_id}" \ | ||
| -f body="${body}" >/dev/null; then | ||
| echo "updated comment ${existing_id}" | ||
| exit 0 | ||
| fi | ||
| elif [ "${update_only}" = "--update-only" ]; then | ||
| echo "no existing comment; nothing to update" | ||
| exit 0 | ||
| elif gh api "repos/${repo}/issues/${number}/comments" \ | ||
| -f body="${body}" >/dev/null; then | ||
| echo "posted new comment" | ||
| exit 0 | ||
| fi | ||
| fi | ||
| sleep 10 | ||
| done | ||
| echo "all attempts failed" >&2 | ||
| exit 1 | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.