Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 36 additions & 5 deletions .github/workflows/qwen-code-pr-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,17 @@ jobs:
# would retire a still-accurate badge on a transient classifier
# failure or a dry run.
DOCS_ONLY_MEDIUM=""
# One implementation for both downgrades' arithmetic: the docs-only
# branch and the micro tightening below both halve with the same
# 90-minute floor, and two verbatim copies once let a one-sided
# divisor edit diverge them silently while the comments still
# claimed they matched.
halve_budget_floor() {
EFFECTIVE_TIMEOUT_MINUTES=$(( EFFECTIVE_TIMEOUT_MINUTES / 2 ))
if [ "$EFFECTIVE_TIMEOUT_MINUTES" -lt 90 ]; then
EFFECTIVE_TIMEOUT_MINUTES=90
fi
}
if [ "${AUTO_REVIEW:-false}" = "true" ]; then
# Fetch + classify through the shared wrapper so this gate and
# ci.yml's profile gate cannot drift on the classifier's input
Expand All @@ -1068,15 +1079,35 @@ jobs:
DOCS_ONLY_MEDIUM=true
# Medium measures at one-third to one-half of high, so halve
# the size-aware budget with a 90-minute floor.
EFFECTIVE_TIMEOUT_MINUTES=$(( EFFECTIVE_TIMEOUT_MINUTES / 2 ))
if [ "$EFFECTIVE_TIMEOUT_MINUTES" -lt 90 ]; then
EFFECTIVE_TIMEOUT_MINUTES=90
fi
halve_budget_floor
echo "PR #${PR_NUMBER} is docs-only; automatic review runs at --effort medium (${EFFECTIVE_TIMEOUT_MINUTES}-minute budget)."
else
DOCS_ONLY_MEDIUM=false
fi
fi
# A micro automatic review — total churn (additions + deletions)
# strictly below 25 — keeps its full high-effort posting run: a
# medium downgrade would drop the inline comments a source fix
# deserves, and the pipeline itself is what shrinks on a micro
# diff (file-scoped suites, a one-round reverse audit). The
# threshold rides the skill's sweep floor in the direction that
# cannot over-tighten: SWEEP_FLOOR (budget.ts) gates a
# source-weighted measure, max(srcDiffLines, diffLines/8), and
# total churn < 25 implies that measure is < 25 — so every diff
Comment thread
wenshao marked this conversation as resolved.
Resolved
# this tightens is one whose sweep and reverse-audit loop the
# skill has already shrunk. What a micro run must not keep is a
# multi-hour kill switch: the same halve-with-floor the docs
# downgrade uses, so a hung run dies at the scale of its work.
# Skipped when the size lookup failed (an unknown size must not
# tighten anything), on explicit runs (their timeout is the
# caller's), and for docs-only runs, which are already halved.
if [ "${AUTO_REVIEW:-false}" = "true" ] \
&& [ "$DOCS_ONLY_MEDIUM" != "true" ] \
&& [ -n "${PR_SIZE_LINES:-}" ] \
Comment thread
wenshao marked this conversation as resolved.
Resolved
&& [ "$PR_SIZE_LINES" -lt 25 ]; then
Comment thread
wenshao marked this conversation as resolved.
Resolved
halve_budget_floor
echo "PR #${PR_NUMBER} is a micro diff (${PR_SIZE_LINES} changed lines); the automatic review keeps --effort high and inline posting, with a tightened ${EFFECTIVE_TIMEOUT_MINUTES}-minute budget."
fi
echo "docs_only_medium=$DOCS_ONLY_MEDIUM" >> "$GITHUB_OUTPUT"

if ! PR_DATA="$(gh pr view "$PR_NUMBER" --repo "$REPO" --json state,headRefOid --jq '[.state, .headRefOid] | @tsv')"; then
Expand Down Expand Up @@ -1359,7 +1390,7 @@ jobs:
GH_TOKEN: '${{ secrets.CI_BOT_PAT }}'
PR_NUMBER: '${{ steps.context.outputs.pr_number }}'
EXPECTED_HEAD_SHA: '${{ steps.review.outputs.expected_head_sha }}'
COMPLETION_LINE: "${{ steps.review.outputs.completion_line || '(no relayable \"Review complete:\" line in the run output — see the run log)' }}"
COMPLETION_LINE: '${{ steps.review.outputs.completion_line || ''(no relayable "Review complete:" line in the run output — see the run log)'' }}'
RUN_URL: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}'
run: |-
set -euo pipefail
Expand Down
132 changes: 123 additions & 9 deletions scripts/tests/qwen-pr-review-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1431,15 +1431,15 @@ describe('docs-only medium gate', () => {
});

function floorSource() {
const anchor = run.indexOf('# Medium measures at one-third to one-half');
expect(anchor).toBeGreaterThan(-1);
const start = run.indexOf('EFFECTIVE_TIMEOUT_MINUTES=$((', anchor);
// The YAML parser strips the block scalar's base indentation, so the
// floor's closing `fi` sits at four spaces in the parsed text.
const end = run.indexOf('\n fi', start) + '\n fi'.length;
// The arithmetic lives in ONE function shared by the docs-only branch
// and the micro tightening; extract the definition plus one call, so
// these cases execute the same implementation both branches run.
const start = run.indexOf('halve_budget_floor() {');
const endAnchor = '\n}';
const end = run.indexOf(endAnchor, start) + endAnchor.length;
expect(start).toBeGreaterThan(-1);
expect(end).toBeGreaterThan(start);
return run.slice(start, end);
return `${run.slice(start, end)}\nhalve_budget_floor`;
}

it.each([
Expand Down Expand Up @@ -1641,7 +1641,7 @@ describe('docs-only gate and relay, executed', () => {
return runStep.slice(start, end);
}

function runGate({ autoReview, wrapper }) {
function runGate({ autoReview, wrapper, prSizeLines, timeoutMinutes }) {
const dir = mkdtempSync(join(tmpdir(), 'docs-gate-'));
try {
const stub = join(dir, '.github/scripts/ci');
Expand All @@ -1655,7 +1655,8 @@ describe('docs-only gate and relay, executed', () => {
`AUTO_REVIEW=${autoReview}`,
'REPO=o/r',
'PR_NUMBER=42',
'EFFECTIVE_TIMEOUT_MINUTES=360',
`EFFECTIVE_TIMEOUT_MINUTES=${timeoutMinutes ?? 360}`,
...(prSizeLines === undefined ? [] : [`PR_SIZE_LINES=${prSizeLines}`]),
`GITHUB_OUTPUT="${gho}"`,
gateSource(),
'printf "timeout=%s" "$EFFECTIVE_TIMEOUT_MINUTES"',
Expand All @@ -1679,6 +1680,119 @@ describe('docs-only gate and relay, executed', () => {
expect(r.stdout).toContain('timeout=180');
});

it("tightens a micro diff's budget without touching its effort or posting", () => {
// Below the review skill's sweep floor (25 changed lines) the automatic
// run keeps --effort high and its inline comments — the pipeline itself
// shrinks on a micro diff — but its kill switch halves to the same
// 90-minute floor the docs downgrade uses.
Comment thread
wenshao marked this conversation as resolved.
Resolved
const r = runGate({
autoReview: 'true',
wrapper: '#!/bin/bash\necho full\n',
prSizeLines: 24,
timeoutMinutes: 180,
});
expect(r.output).toBe('docs_only_medium=false');
expect(r.stdout).toContain('micro diff (24 changed lines)');
expect(r.stdout).toContain('keeps --effort high');
expect(r.stdout).toContain('timeout=90');
});

it('micro tightening floors at 90 minutes', () => {
const r = runGate({
autoReview: 'true',
wrapper: '#!/bin/bash\necho full\n',
prSizeLines: 10,
timeoutMinutes: 100,
});
expect(r.stdout).toContain('timeout=90');
});

it('twenty-five changed lines is not micro — churn rides the sweep floor conservatively', () => {
// The gate measures TOTAL churn while the skill's SWEEP_FLOOR gates a
// source-weighted measure; churn < 25 implies the weighted measure is
// < 25, so the tightening only ever lands on diffs whose pipeline the
// skill has already shrunk. 25 itself must not tighten.
const r = runGate({
autoReview: 'true',
wrapper: '#!/bin/bash\necho full\n',
prSizeLines: 25,
timeoutMinutes: 180,
});
expect(r.stdout).not.toContain('micro diff');
expect(r.stdout).toContain('timeout=180');
});

it('both downgrades share one halve-with-floor implementation', () => {
// Two verbatim copies once let a one-sided divisor edit diverge micro
// runs from docs-only runs while the comments claimed they matched —
// probe: a / 2 → / 3 mutant survived every test because both micro
// inputs land on the floor under any divisor ≥ 2. One named function,
// called from both branches, makes the invariant structural.
const gate = gateSource();
expect(gate.match(/halve_budget_floor\(\)/g)).toHaveLength(1);
expect(gate.match(/halve_budget_floor$/gm)).toHaveLength(2);
expect(
gate.match(
/EFFECTIVE_TIMEOUT_MINUTES=\$\(\( EFFECTIVE_TIMEOUT_MINUTES \/ 2 \)\)/g,
),
).toHaveLength(1);
});

it('a docs-only micro diff is halved once, by the docs gate, not twice', () => {
const r = runGate({
autoReview: 'true',
wrapper: '#!/bin/bash\necho docs_only\n',
prSizeLines: 10,
timeoutMinutes: 360,
});
expect(r.output).toBe('docs_only_medium=true');
expect(r.stdout).toContain('timeout=180');
expect(r.stdout).not.toContain('micro diff');
});

it('a manually requested review is never tightened, whatever its size', () => {
// Production-reachable: an @qwen-code /review comment without --timeout
// populates PR_SIZE_LINES but is not an automatic review — its budget
// is the caller's. A mutant dropping the AUTO_REVIEW guard survived the
// suite until this pin.
const r = runGate({
autoReview: 'false',
wrapper: '#!/bin/bash\necho full\n',
prSizeLines: 10,
timeoutMinutes: 180,
});
expect(r.stdout).not.toContain('micro diff');
expect(r.stdout).toContain('timeout=180');
});

it('a failed docs classification still tightens a micro automatic run', () => {
// DOCS_ONLY_MEDIUM stays '' when the classifier fails; the micro guard
// keys on != "true", not = "false" — a mutant conflating the two kept
// 180 on exactly the runs the tightening exists for.
const r = runGate({
autoReview: 'true',
wrapper: '#!/bin/bash\nexit 2\n',
prSizeLines: 10,
timeoutMinutes: 180,
});
expect(r.output).toBe('docs_only_medium=');
expect(r.stdout).toContain('micro diff (10 changed lines)');
expect(r.stdout).toContain('timeout=90');
});

it('an unknown size never tightens — and neither does an explicit run', () => {
// PR_SIZE_LINES is unset when the size lookup failed or when the caller
// passed --timeout (the size block is skipped); both must keep the
// budget they have.
const r = runGate({
autoReview: 'true',
wrapper: '#!/bin/bash\necho full\n',
timeoutMinutes: 180,
});
expect(r.stdout).not.toContain('micro diff');
expect(r.stdout).toContain('timeout=180');
});

it('keeps the full review for a full classification', () => {
const r = runGate({
autoReview: 'true',
Expand Down
Loading