Skip to content

run-sweep: gate full-sweep PRs behind a sequential canary#1503

Merged
Oseltamivir merged 17 commits into
mainfrom
sweep-canary-gate
May 27, 2026
Merged

run-sweep: gate full-sweep PRs behind a sequential canary#1503
Oseltamivir merged 17 commits into
mainfrom
sweep-canary-gate

Conversation

@Oseltamivir

@Oseltamivir Oseltamivir commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

When a PR carries full-sweep-enabled, pick the lowest-conc single-node benchmark entry as a canary and run it before fanning out the full sweep. If the canary fails, the eight fan-out jobs are skipped to save cluster time on shared failures (bad image tag, removed CLI flag, etc.).

Design choices:

  • Canary candidacy is restricted to single_node['1k1k' | '8k1k'] and excludes entries with run-eval: true, so the canary is always a pure benchmark smoke test using the existing single-node template. For an evals-only: true changelog entry, the generated benchmark matrices are empty and the canary skips naturally.
  • The canary entry is removed from the regular fan-out's matrix (via remaining-search-space-config) only when the canary actually succeeded. On canary skip or canary-select failure, the regular fan-out falls back to the full search-space-config so coverage is preserved.
  • The fan-out gate proceeds only on canary-sweep.result == 'success' || == 'skipped' (allowlist, not != 'failure'), so a cancelled canary also blocks fan-out — preventing the case where in-flight fan-out jobs continue after the user explicitly cancels the canary.
  • The eight fan-out jobs gate on !cancelled() && …, not always() && …. always() is immune to workflow cancellation, so under the old gate any matrix entry whose runner had been assigned would launch and run to completion after gh run cancel, and queued matrix entries kept picking up runners. !cancelled() evaluates false once the workflow is cancelled (by user, by concurrency, or upstream), so the queued matrix stops spawning and in-flight entries are cancelled along with the workflow. collect-results and the housekeeping jobs keep always() on purpose so partial-result aggregation still runs on cancel.
  • A new non-canary-full-sweep-enabled label runs the full sweep without the canary gate (escape hatch when the canary is flaky or unrepresentative). The three sweep labels are now mutually exclusive. utils/find_reusable_sweep_run.py and utils/merge_with_reuse.sh accept either full-sweep-enabled or non-canary-full-sweep-enabled as a "full sweep" for /reuse-sweep-run and merge_with_reuse paths.
  • Non-full-sweep PRs, draft PRs, pushes to main, and the reuse path all behave identically to before via existing gates.

The aggregated results_bmk artifact picks up both the canary's row and the regular fan-out's rows via the existing bmk_* glob — each entry appears exactly once.

Docker-tag-monitor research block split out to #1538.
Documentation for the new label and reuse behavior is included in AGENTS.md, .github/workflows/README.md, KLAUD_DEBUG.md, and .claude/commands/klaud-pr-status-html.md.

Test plan

T1/T2/T3/T6/T7/T8 are live Run Sweep runs on this branch, each triggered by a synchronize event with a deliberately-chosen changelog target. gptoss-fp4-h100-vllm was used for T1/T2/T6/T7/T8 (h100 has the shortest CI queue); kimik2.5-int4-h100-vllm (agentic-only) was used for T3. T5 is workflow-logic inspection; T9/T10 are local validation.

T6 and T7 are the empirical bug-and-fix pair for the cancellation regression: T6 reproduces the bug on the old always() && gate, T7 verifies the fix on the new !cancelled() && gate. They share the same procedure (let canary succeed → gh run cancel mid-fan-out) and differ only by the gate they ran against.

  • T1 — Canary success → fan-out runs with dedup (run 26467367124)

    • Result: canary-sweep ran the lowest-conc 1k1k entry (tp=2, conc=4) and succeeded. Downstream single-node-1k1k ran against remaining-search-space-config (the canary entry de-duped). Final tally: 41 success, 7 skipped, 0 failures.
    • Catches: jq remove_one correctness and the (canary == success && remaining) || full matrix ternary.
  • T2 — Canary failure → fan-out skipped (run 26471353341)

    • Setup: image override to vllm/vllm-openai:nonexistent-tag-T2-canary-failure-test.
    • Result: canary-sweep.result == 'failure'; all 8 fan-out jobs (single-node 1k1k/8k1k, single-node agentic/eval, all multi-node variants) and collect-results correctly skipped via the success || skipped allowlist.
    • Catches: the value-prop of this PR — gate actually blocks fan-out on shared failure.
  • T3 — No canary candidate → fan-out on full search space (run 26471597565)

    • Setup: switched changelog target to kimik2.5-int4-h100-vllm (agentic-only config; no fixed-seq-len entries → empty single_node['1k1k'] and ['8k1k'] arrays).
    • Result: canary-select succeeded but returned []; canary-sweep skipped via the != '' && != '[]' guard; agentic fan-out ran on the full search space (ternary fell through canary-sweep.result == 'success'); 1k1k/8k1k/eval/multi-node all correctly skipped because their matrices are empty for this config.
    • Catches: the fallback branch of the ternary — broken matrices wouldn't silently skip single-node sweeps.
    • Post-fix route check: run 26531946908 repeated the empty-canary path under the !cancelled() gate: canary-sweep skipped and the agentic fan-out was admitted. The run is not counted as green validation because its downstream agentic benchmark jobs failed.
  • T5 — Push to main → canary never fires

    • Method: workflow-logic inspection only (no live run).
    • Expected: canary-select.if requires github.event_name == 'pull_request', so on a push event it evaluates false → canary-sweep skipped → every sweep-* job runs via the success || skipped allowlist on the full search-space-config. Identical to pre-PR push-to-main behavior.
  • T6 — Workflow cancel mid-fan-out under old always() && gate (regression repro) (run 26524225403)

    • Setup: commit d51965fe, fan-out gates were always() && needs.setup.result == 'success' && …. Canary completed success at tp=2, conc=4; once the fan-out matrix had 19 in_progress + 12 queued + 10 skipped/done jobs, ran gh run cancel 26524225403.
    • Result, T+7 min after cancel: 24 success, 6 skipped, 0 cancelled, 0 failures. All 12 originally-queued matrix entries picked up runners after the cancel signal and finished with conclusion=success. The run never honored the cancel.
    • Catches: confirmed the always() gate ignores workflow cancellation — the reason for moving to !cancelled() && ….
  • T7 — Workflow cancel mid-fan-out under new !cancelled() && gate (fix verification) (run 26525033406)

    • Setup: commit e166509d, fan-out gates are now !cancelled() && …. Same procedure as T6 on the same canary entry (gptoss-fp4-h100-vllm, tp=2 conc=4): waited for canary to succeed and the fan-out matrix to reach 19 in_progress + 12 queued, then ran gh run cancel 26525033406.
    • Result: at T+13 s, queued went 12 → 0 and cancelled jumped 0 → 12 — every queued matrix entry was cancelled before any could pick up a runner (the exact opposite of T6, where queued held at 12 and all 12 picked up runners). Final tally: 32 cancelled, 10 success, 6 skipped, 0 failures, run.conclusion = cancelled. The 10 success rows are setup/canary/check-newline (already done pre-cancel) plus a small number of nested sub-jobs that finished inside GitHub's runner-shutdown grace window — none were post-cancel matrix pickups.
    • Catches: that !cancelled() actually fires on user-issued gh run cancel and blocks queued matrix entries from spawning new runners. Side-by-side with T6 this is the direct empirical receipt that the always() → !cancelled() swap fixes the cancellation regression.
  • T8 — non-canary-full-sweep-enabled label → canary skipped, full fan-out under final gate (run 26530059401)

    • Setup: commit 13272b92, after the !cancelled() fix; swapped the PR label from full-sweep-enabled to non-canary-full-sweep-enabled.
    • Result: canary-select and canary-sweep skipped; fan-out ran on the full search space; final tally: 40 success, 9 skipped, 0 failures. The results_bmk aggregate contains 28 rows, 28 unique rows.
    • Catches: the new label takes the no-canary path, and a skipped canary dependency still permits full fan-out under the final !cancelled() expression.
  • T9 — Reuse path accepts both full-sweep labels (unit test, utils/test_find_reusable_sweep_run.py)

    • Method: added test_main_accepts_non_canary_full_sweep_label (PR carrying only non-canary-full-sweep-enabledreuse-enabled=true) and test_main_rejects_pr_with_neither_full_sweep_label (PR carrying neither label → RuntimeError mentioning both label names). All 13 reuse tests pass.
    • Catches: the find_reusable_sweep_run.py / merge_with_reuse.sh change that lets a PR labeled with the new no-canary escape hatch still approve /reuse-sweep-run and merge via utils/merge_with_reuse.sh.
  • T10 — Final local validation

    • Result: actionlint .github/workflows/run-sweep.yml, bash -n utils/merge_with_reuse.sh, and python -m pytest utils/test_find_reusable_sweep_run.py utils/test_validate_reusable_sweep_artifacts.py utils/matrix_logic/ -q succeeded (166 passed).

Follow-ups (separate PR)

Raised by the code reviewer and intentionally scoped out of this PR to keep it focused on the canary gate + cancellation fix + label-reuse plumbing:

  • collect-results predicate omits sweep-agentic / sweep-multi-node-agentic. Pre-existing bug, not introduced here — visible in T3 (run 26471597565) where collect-results was skipped even though the agentic fan-out ran. Should be fixed by adding both job names to the OR-chain at .github/workflows/run-sweep.yml around line 524.

The originally-listed docs follow-up (non-canary-full-sweep-enabled not in AGENTS.md / workflow README) has been folded into this PR (commit e3f40988).

Merge note

perf-changelog.yaml on this branch is identical to main after merging origin/main to pick up PR #1569's new MI355X entries. This is now a workflow/docs-only PR: Run Sweep does not trigger on the current head via pull_request: paths: [perf-changelog.yaml], and a standard squash merge will not trigger the push-to-main sweep or result ingestion either. No /reuse-sweep-run approval is needed for the final diff.

For audit purposes, the post-fix T8 redo (run 26530059401, commit 13272b92) is green and its commit remains in this PR's history. It would be eligible reuse evidence only if a later revision reintroduced a real perf-changelog.yaml change before merge.


Note

Medium Risk
Changes core run-sweep.yml orchestration (canary gating, job if conditions, new label matrix); mistakes could skip sweeps, duplicate work, or break merge-time artifact reuse.

Overview
full-sweep-enabled PR sweeps now run a sequential single-node canary before the full matrix: canary-select picks the lowest-concurrency non-eval entry from 1k1k/8k1k, canary-sweep runs it, and fan-out jobs wait for canary success or skip; on canary success, that entry is removed from the single-node 1k1k/8k1k matrices via remaining-search-space-config.

Fan-out job gates switch from always() to !cancelled() so workflow cancel stops queued matrix jobs from spawning (fix for post-gh run cancel runner pickup).

Adds non-canary-full-sweep-enabled for full sweeps without the canary; sweep-enabled, full-sweep-enabled, and non-canary-full-sweep-enabled are mutually exclusive. Reuse paths (find_reusable_sweep_run.py, merge_with_reuse.sh) and docs accept either full-sweep label for /reuse-sweep-run.

Reviewed by Cursor Bugbot for commit ce6a610. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread .github/workflows/run-sweep.yml Dismissed
@Oseltamivir Oseltamivir changed the title Sequential canary for full-sweep PRs + pre-flight research in docker-tag-monitor run-sweep: gate full-sweep PRs behind a sequential canary May 20, 2026
Oseltamivir added a commit that referenced this pull request May 26, 2026
Per PR #1503 test plan (T1 — canary success → fan-out runs with dedup).
The minimaxm2.5-fp8-b200-vllm config has both single_node 1k1k and 8k1k
scenarios, so canary-select has lowest-conc candidates to pick from. The
master config image (vllm/vllm-openai:v0.21.0) is known-good, so the
canary is expected to succeed and the fan-out should run against
remaining-search-space-config (canary entry removed → no duplicate run).
Comment thread perf-changelog.yaml Outdated
Oseltamivir added a commit that referenced this pull request May 26, 2026
Per PR #1503 test plan (T1 — canary success → fan-out runs with dedup).
The minimaxm2.5-fp8-b200-vllm config has both single_node 1k1k and 8k1k
scenarios, so canary-select has lowest-conc candidates to pick from. The
master config image (vllm/vllm-openai:v0.21.0) is known-good, so the
canary is expected to succeed and the fan-out should run against
remaining-search-space-config (canary entry removed → no duplicate run).
Comment thread .github/workflows/run-sweep.yml
Oseltamivir added a commit that referenced this pull request May 26, 2026
Per PR #1503 test plan (T1 — canary success → fan-out runs with dedup).
The minimaxm2.5-fp8-b200-vllm config has both single_node 1k1k and 8k1k
scenarios, so canary-select has lowest-conc candidates to pick from. The
master config image (vllm/vllm-openai:v0.21.0) is known-good, so the
canary is expected to succeed and the fan-out should run against
remaining-search-space-config (canary entry removed → no duplicate run).
@Oseltamivir Oseltamivir added non-canary-full-sweep-enabled Run the full sweep without the canary gate (full search space, no trim) full-sweep-enabled evals-only and removed full-sweep-enabled non-canary-full-sweep-enabled Run the full sweep without the canary gate (full search space, no trim) labels May 26, 2026
Comment thread perf-changelog.yaml Outdated
When a PR carries `full-sweep-enabled` (and not `evals-only`), pick the
lowest-conc single-node benchmark entry as a canary and run it before
fanning out the full sweep. If the canary fails, the eight fan-out jobs
are skipped to save cluster time on shared failures (bad image tag,
removed CLI flag, etc.).

Design choices:
- Canary candidacy is restricted to single_node['1k1k' | '8k1k'] and
  excludes entries with run-eval: true, so the canary is always a pure
  benchmark smoke test using the existing single-node template.
- The canary entry is removed from the regular fan-out's matrix (via
  remaining-search-space-config) only when the canary actually succeeded.
  On canary skip / cancel / canary-select failure, the regular fan-out
  falls back to the full search-space-config so coverage is preserved.
- The fan-out gate blocks only on `canary-sweep.result == 'failure'` --
  every other state (success, skipped, cancelled) proceeds, so a bug in
  the canary mechanism never blocks the rest of the sweep.
- Non-full-sweep PRs, draft PRs, pushes to main, and the reuse path all
  behave identically to before via existing gates.

The aggregated results_bmk artifact picks up both the canary's row and
the regular fan-out's rows via the existing bmk_* glob -- each entry
appears exactly once.
Previously the fan-out gate was `needs.canary-sweep.result != 'failure'`,
which let `cancelled` (and any unknown future result) fall through. A
cancelled canary then ran the FULL fan-out matrix — including the canary's
own entry — without canary validation, the worst-case outcome.

Replace with an explicit allowlist:
  (result == 'success' || result == 'skipped')

- success: canary passed → fan-out runs with deduped (remaining) matrix
- skipped: no canary candidate (multi-node-only / evals-only) → fan-out
  runs with full matrix (no canary to dedup against)
- failure / cancelled / anything else: fan-out blocked

The matrix-ternary `result == 'success' && remaining-search-space-config
|| full-search-space-config` already had the right shape and is untouched.

Applies to all 8 fan-out jobs (single/multi-node 1k1k+8k1k, agentic,
multi-node-agentic, evals, multi-node-evals).
…nary)

New label that triggers the full sweep without running the canary gate.
Acts as an escape hatch when:
- The canary is known to be flaky / unreliable for this config
- The user wants the full sweep without the canary delay
- The canary's chosen entry is not a representative smoke test

Behavior matrix:
  sweep-enabled                  - trims to max(conc) per parallelism, with canary
  full-sweep-enabled             - full intermediate conc sweep, with canary
  non-canary-full-sweep-enabled  - full intermediate conc sweep, NO canary

How it works (no changes to existing label semantics):
- Setup gate triggers on any of the 3 labels (was: 2)
- canary-select gate still requires `full-sweep-enabled` (exact array match,
  so `non-canary-full-sweep-enabled` does NOT match) → canary skips → all
  fan-out jobs run on full search space via the `== 'skipped'` allowlist
- TRIM_CONC env is unchanged — only `sweep-enabled` enables trim, so the
  new label correctly behaves as "full sweep"
- The reject-conflicting-labels step is now a 3-way exclusion: at most
  one of {sweep-enabled, full-sweep-enabled, non-canary-full-sweep-enabled}
- The same gate updates apply to the comment-visualizer job
- Concurrency-group filter excludes the new label too so toggling it
  uses the same `'active'` group key as the other sweep labels
@SemiAnalysisAI SemiAnalysisAI deleted a comment from github-actions Bot May 27, 2026
@SemiAnalysisAI SemiAnalysisAI deleted a comment from github-actions Bot May 27, 2026
@SemiAnalysisAI SemiAnalysisAI deleted a comment from github-actions Bot May 27, 2026
@SemiAnalysisAI SemiAnalysisAI deleted a comment from github-actions Bot May 27, 2026
@SemiAnalysisAI SemiAnalysisAI deleted a comment from github-actions Bot May 27, 2026
@SemiAnalysisAI SemiAnalysisAI deleted a comment from github-actions Bot May 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

Used by T6/T7 (cancellation-regression repro and !cancelled() fix
verification, runs 26524225403 and 26525033406). Both tests are
complete and recorded in the PR description; the entry is no longer
needed.
@Oseltamivir Oseltamivir added non-canary-full-sweep-enabled Run the full sweep without the canary gate (full search space, no trim) and removed full-sweep-enabled labels May 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

DO NOT MERGE. Verifies empty-canary-config path on HEAD's !cancelled() gate.
Agentic-only config → canary-select returns [] → canary-sweep skipped via
inner guard → agentic fan-out on full search space.
@Oseltamivir Oseltamivir added full-sweep-enabled and removed non-canary-full-sweep-enabled Run the full sweep without the canary gate (full search space, no trim) labels May 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d35e0b7. Configure here.

Comment thread perf-changelog.yaml Outdated
@github-actions

Copy link
Copy Markdown
Contributor

DO NOT MERGE. Verifies canary success → fan-out + dedup path on HEAD's
!cancelled() gate.
@github-actions

Copy link
Copy Markdown
Contributor

AGENTS.md: list all three sweep labels in the table; clarify multi-label rejection.
.github/workflows/README.md: reuse section accepts either full-sweep label;
fail-closed wording covers both.
KLAUD_DEBUG.md / klaud-pr-status-html.md: mention the new label alongside
full-sweep-enabled.
Final cleanup. T8 redo (run 26530059401, commit 13272b9, success) is the
reusable green sweep for /reuse-sweep-run.
@Oseltamivir Oseltamivir enabled auto-merge (squash) May 27, 2026 22:10
@Oseltamivir Oseltamivir disabled auto-merge May 27, 2026 22:32
Resolves conflicts with PR #1576 (trim-to-min). For the
multi-conflicting-labels guardrail in run-sweep.yml and the matching
prose in AGENTS.md, keep this branch's three-label structure
(sweep-enabled / full-sweep-enabled / non-canary-full-sweep-enabled)
and the canary-gate description for full-sweep-enabled, and update the
sweep-enabled wording from "max(conc)" → "min(conc)" to match the
new trim_conc behavior on main. utils/process_changelog.py auto-merged
cleanly to the new lowest-`conc` implementation.
@Oseltamivir Oseltamivir merged commit bfad6df into main May 27, 2026
4 checks passed
@Oseltamivir Oseltamivir deleted the sweep-canary-gate branch May 27, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

2 participants