From b494eb5758687b1576543b02da5da43e4ede33b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?oliver=20k=C3=B6nig?= Date: Tue, 5 May 2026 10:01:38 +0000 Subject: [PATCH 1/4] [docs] feat: add bump-dependency skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: oliver könig --- skills/bump-dependency/SKILL.md | 356 ++++++++++++++++++++++++++++++++ 1 file changed, 356 insertions(+) create mode 100644 skills/bump-dependency/SKILL.md diff --git a/skills/bump-dependency/SKILL.md b/skills/bump-dependency/SKILL.md new file mode 100644 index 00000000000..18e24f5de7e --- /dev/null +++ b/skills/bump-dependency/SKILL.md @@ -0,0 +1,356 @@ +--- +name: bump-dependency +description: Bump a pinned dependency (TransformerEngine, Megatron-LM, NRX, etc.), regenerate the lockfile, open a PR, and drive it to green by attaching a watchdog to the "CICD NeMo" workflow and quarantining failing functional tests as flaky until the run is green. +when_to_use: Bumping a dependency pin in `pyproject.toml` or `uv.lock` and shepherding the PR to green. 'bump TE', 'bump transformer-engine', 'update TE pin', 'bump submodule', 'update lock file', 'bump dependency PR', 'watch CI for a bump', 'quarantine flaky tests after bump', 'run all tests for this bump'. +--- + +# Bump Dependency + +End-to-end workflow for shipping a dependency bump in Megatron Bridge. +Optimised for the case where TE, MCore, or another GPU-heavy pin moves +forward — which often surfaces flakes that have to be quarantined before +the PR can land. + +The pipeline is always: **edit → relock → push → /ok to test → watchdog → +quarantine on red → re-trigger → repeat until green**. + +## When to reach for this skill + +- Bumping a git-source pin in `pyproject.toml` `override-dependencies` + (e.g. `transformer-engine @ git+...@`). +- Bumping the `3rdparty/Megatron-LM` submodule. +- Any change that touches `uv.lock` and needs the full L0 + L1 matrix to + prove out before merge. + +For pure dep additions/removals without a CI loop, the +`build-and-dependency` skill is enough. + +## Required context + +Read first, then follow the steps below: + +- @CONTRIBUTING.md — PR title/label policy, DCO sign-off +- @skills/build-and-dependency/SKILL.md — `uv lock` mechanics, container choice +- @skills/cicd/SKILL.md — how `copy-pr-bot` and `/ok to test` work +- @skills/testing/SKILL.md — `active/` vs `flaky/` directory layout + +## Step 1 — Worktree and edit + +```bash +# From the Megatron-Bridge repo root +git worktree add .claude/worktrees/ -b origin/main +git submodule update --init 3rdparty/Megatron-LM # required before `uv lock` +``` + +Edit the pin. For TE the canonical knob is the override line in +`pyproject.toml`: + +```toml +override-dependencies = [ + ... + "transformer-engine @ git+https://github.com/NVIDIA/TransformerEngine.git@", + ... +] +``` + +Use a **branch name** (`release_v2.15`) only when you want to track a +moving tip; use a full SHA for reproducibility. TE branches use +`release_vX.Y` (underscore), not `release/vX.Y`. Verify with +`git ls-remote https://github.com/NVIDIA/TransformerEngine.git`. + +## Step 2 — Regenerate the lockfile + +`uv.lock` is Linux + CUDA only. Run inside the project image: + +```bash +docker run --rm \ + -v $(pwd):/opt/Megatron-Bridge \ + -v $HOME/.cache/uv:/root/.cache/uv \ + -w /opt/Megatron-Bridge \ + megatron-bridge:latest \ + bash -c 'uv lock' +``` + +Confirm only the intended packages moved: + +```bash +git diff --stat pyproject.toml uv.lock +``` + +If the diff carries changes you didn't ask for (transitive movements you +can't explain), stop and investigate before pushing. + +## Step 3 — Commit and push + +```bash +git add pyproject.toml uv.lock +git commit -S -s -m "[build] chore: bump to " +git push -u origin +``` + +PR title format per @CONTRIBUTING.md: `[build] chore: bump to `. +Sign-off (`-s`) is required; signed commits (`-S`) let `copy-pr-bot` +trigger CI without needing `/ok to test` for every push. + +## Step 4 — Open the PR + +PR body goes through a tmpfile to preserve formatting. Wrap it in a +`
` block: + +```bash +cat > /tmp/pr-body.md <<'EOF' +
Claude summary + +## What +- Bump to . +- Regenerate `uv.lock`. + +## Lockfile delta +``` +Updated -> +``` + +## Test plan +- [ ] L0 CI green +- [ ] L1 CI green (label `needs-more-tests` applied) + +## Quarantined tests (this bump) +_None yet — will be appended as flakes are identified during CI iteration._ + +
+EOF + +gh pr create \ + --repo NVIDIA-NeMo/Megatron-Bridge \ + --base main \ + --head \ + --title "[build] chore: bump to " \ + --body-file /tmp/pr-body.md \ + --label "ci,area:build,needs-review,needs-more-tests" +``` + +The `needs-more-tests` label is **mandatory** for a bump — it expands the +matrix from L0 to L0+L1 (see @skills/testing/SKILL.md tier table). L2 is +schedule-only and cannot be triggered by labels. + +`gh pr edit` is unreliable. To update a PR's title or body later, use the +REST API directly: + +```bash +gh api -X PATCH "repos/NVIDIA-NeMo/Megatron-Bridge/pulls/" \ + -F "body=@/tmp/pr-body.md" + +gh api -X PATCH "repos/NVIDIA-NeMo/Megatron-Bridge/pulls/" \ + -f "title=[build] chore: bump to " +``` + +## Step 5 — Trigger CI on the exact SHA + +Even with a signed commit, post `/ok to test` on the SHA you actually +want exercised so any cached / cancelled run is re-fired: + +```bash +SHA=$(git rev-parse HEAD) +gh pr comment --repo NVIDIA-NeMo/Megatron-Bridge --body "/ok to test $SHA" +``` + +Use the **full** SHA (`git rev-parse HEAD`), never the short form. + +## Step 6 — Attach the watchdog (always; never a cronjob) + +For a bump PR you want a single live process that emits per-job state +changes for the **CICD NeMo** workflow only. Other workflows (docs, +wheel, copyright, install-test) are noise here — the gate that decides +green-or-red for a bump is `CICD NeMo`. + +**Always attach a watchdog with the Monitor tool. Never schedule wakeups +or cronjobs for this loop.** A watchdog gives you: + +- Sub-minute reaction time on every job transition. +- A single live process — no scattered scheduled-wakeup state to reason + about. +- Natural early termination via `TaskStop` once the run is green. + +### Watchdog script + +Save to `/tmp/watchdog-.sh` and chmod +x: + +```bash +#!/usr/bin/env bash +# Watchdog: monitor "CICD NeMo" runs on pull-request/ and emit +# per-job state changes. Stays alive across re-runs (new commits). +set -u +PR= +REPO=NVIDIA-NeMo/Megatron-Bridge +BRANCH="pull-request/$PR" + +prev_run_id="" +declare -A prev_state + +emit() { echo "[$(date -u +%H:%M:%SZ)] $*"; } + +while true; do + run_json=$(gh run list --repo "$REPO" --workflow "CICD NeMo" \ + --branch "$BRANCH" --limit 1 \ + --json databaseId,status,conclusion,headSha 2>/dev/null || echo "[]") + run_id=$(echo "$run_json" | jq -r '.[0].databaseId // empty') + run_status=$(echo "$run_json" | jq -r '.[0].status // empty') + run_conclusion=$(echo "$run_json" | jq -r '.[0].conclusion // empty') + run_sha=$(echo "$run_json" | jq -r '.[0].headSha // empty') + + if [[ -z "$run_id" ]]; then + sleep 30; continue + fi + + if [[ "$run_id" != "$prev_run_id" ]]; then + emit "RUN ${run_id} STARTED sha=${run_sha:0:8} status=${run_status}" + prev_run_id="$run_id" + unset prev_state + declare -A prev_state + fi + + jobs_json=$(gh run view "$run_id" --repo "$REPO" --json jobs 2>/dev/null || echo "{}") + while IFS=$'\t' read -r name status conclusion; do + [[ -z "$name" ]] && continue + cur="${status}/${conclusion}" + if [[ "${prev_state[$name]:-}" != "$cur" ]]; then + case "$status" in + completed) + emit "JOB ${name} -> ${conclusion}" ;; + in_progress) + if [[ -z "${prev_state[$name]:-}" || "${prev_state[$name]}" == "queued/" ]]; then + emit "JOB ${name} -> in_progress" + fi ;; + esac + prev_state[$name]="$cur" + fi + done < <(echo "$jobs_json" | jq -r '.jobs[]? | [.name, .status, (.conclusion // "")] | @tsv') + + if [[ "$run_status" == "completed" ]]; then + emit "RUN ${run_id} COMPLETED conclusion=${run_conclusion}" + fi + + sleep 60 +done +``` + +### Arming the watchdog + +```text +Monitor( + description="CICD NeMo run state changes on PR ", + command="bash /tmp/watchdog-.sh", + persistent=true, + timeout_ms=3600000 +) +``` + +`persistent: true` keeps it alive across re-runs (you'll push more +commits when quarantining flakes). Stop it with `TaskStop()` +once the run is green. + +### Why never a cronjob / scheduled wakeup + +- Cronjobs run blind — they fire on a clock, not on an event. You'll + either over-poll (cache miss every wake-up) or miss long stalls. +- Wakeups can't easily fan out to "tell me whenever a job transitions" + — they only resume the agent on a fixed interval. +- A persistent Monitor surfaces every job edge in real time and exits + cleanly when the work is done. + +## Step 7 — Quarantine on red, then iterate + +When a `JOB -> failure` event fires: + +1. Skim the logs to confirm it's a flake / pre-existing issue, not the + bump itself: + + ```bash + RUN_ID= + gh run view "$RUN_ID" --repo NVIDIA-NeMo/Megatron-Bridge --log-failed > /tmp/run.log + wc -l /tmp/run.log + tail -200 /tmp/run.log + ``` + + If the failure is caused by the bump (real regression, not a flake), + **stop quarantining** — fix the underlying issue or revert the bump. + Quarantining a real regression hides the very signal the bump PR + exists to surface. + +2. Move the launch script to `flaky/` on the matching hardware target + (see @skills/testing/SKILL.md): + + ```bash + git mv tests/functional_tests/launch_scripts/h100/active/_.sh \ + tests/functional_tests/launch_scripts/h100/flaky/_.sh + + # If the GB200 variant also failed: + git mv tests/functional_tests/launch_scripts/gb200/active/_.sh \ + tests/functional_tests/launch_scripts/gb200/flaky/_.sh + ``` + + Map a CI job name (e.g. `gb200_L0_Launch_models_foo`) to its launch + script via: + + - prefix `gb200_` → `gb200/active/`, otherwise `h100/active/` + - the rest is the script's basename without `.sh` + +3. Append the test to the PR description's **Quarantined tests** + section, with a one-line reason and a follow-up tracking link if you + have one. This is the durable record of what this bump deferred. + +4. Commit, push, retrigger: + + ```bash + git commit -S -s -m "[ci] chore: quarantine flaky for bump" + git push + SHA=$(git rev-parse HEAD) + gh pr comment --repo NVIDIA-NeMo/Megatron-Bridge --body "/ok to test $SHA" + ``` + +5. Update the PR body via `gh api PATCH` so the quarantine list stays + current. + +The watchdog is persistent — it will pick up the new run automatically +and emit `RUN STARTED` for the new attempt. + +## Step 8 — Stop when green + +`RUN COMPLETED conclusion=success` is the exit condition. Then: + +```bash +# Sanity check +gh pr checks --repo NVIDIA-NeMo/Megatron-Bridge | awk '{print $2}' | sort | uniq -c + +# Tear down +TaskStop() + +# Tick the boxes in the PR body +gh api -X PATCH "repos/NVIDIA-NeMo/Megatron-Bridge/pulls/" -F "body=@/tmp/pr-body.md" +``` + +## Common pitfalls + +| Symptom | Cause | Fix | +|---|---|---| +| `uv lock` errors with "not a Python project" on `3rdparty/Megatron-LM` | Submodule not initialised in the worktree | `git submodule update --init 3rdparty/Megatron-LM` | +| CI never starts on a new push | Commit not GPG-signed and no `/ok to test` for the new SHA | Post `/ok to test $(git rev-parse HEAD)` | +| Watchdog goes silent for 30+ min | `gh` rate-limited or auth expired | Bump poll interval; `gh auth status`; restart Monitor | +| Quarantine commit doesn't trigger a new run | Pushed but didn't post `/ok to test` for the new SHA | Always re-post on the new SHA | +| Job name doesn't match a script in `active/` | `gb200_` prefix is the hardware indicator, not part of the filename | Strip `gb200_` and look in `gb200/active/` | +| Wrong TE branch ref (`release/v2.15`) silently resolves nothing | TE uses `release_vX.Y` with an underscore | Verify with `git ls-remote` before locking | +| Lockfile diff includes unrelated CVE-pinned packages | `override-dependencies` carries floors that float | Re-run lock and accept; don't try to revert those | + +## Anti-patterns + +- **Cron / scheduled wakeups for this loop.** Always Monitor. +- **Polling all workflows.** Filter to `CICD NeMo` — the rest are noise + for a bump. +- **Quarantining a real regression** to "make CI green." That defeats + the purpose of the bump PR. Only quarantine if the failure reproduces + on `main` or is clearly unrelated infrastructure. +- **`gh pr edit`** for title/body. Use `gh api PATCH`. +- **HEREDOC in `gh pr create --body`.** Always go through a tmpfile + + `--body-file`. +- **Bundling unrelated changes** (feature work, refactors) into a bump + PR. Bumps should stay surgical so CI failures attribute cleanly. From d93e61b9aa50e732fcdf697b6e168162a66fc417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?oliver=20k=C3=B6nig?= Date: Tue, 5 May 2026 10:16:15 +0000 Subject: [PATCH 2/4] [docs, ci] fix: document full-test-suite label as the L2 trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: oliver könig --- .github/workflows/cicd-main.yml | 2 +- CONTRIBUTING.md | 7 +++++-- skills/bump-dependency/SKILL.md | 7 +++++-- skills/cicd/SKILL.md | 2 +- skills/testing/SKILL.md | 2 +- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/.github/workflows/cicd-main.yml b/.github/workflows/cicd-main.yml index 7d108cb92f4..e20c7ac618e 100644 --- a/.github/workflows/cicd-main.yml +++ b/.github/workflows/cicd-main.yml @@ -673,7 +673,7 @@ jobs: test-data-path: ${{ needs.pre-flight.outputs.test_data_path }} runner: ${{ matrix.runner }} - # L2: runs on schedule (nightly/weekly) and workflow_dispatch only + # L2: runs on schedule (nightly/weekly), workflow_dispatch, and PRs labeled `full-test-suite` cicd-functional-tests-l2: strategy: fail-fast: false diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d0174ab2eae..012a000a905 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -202,11 +202,13 @@ When you create a pull request, **add labels immediately** so reviewers and CI c 3. **`docs-only`** — if the PR touches only documentation (no code changes); this skips most CI jobs 4. **`needs-review`** — when the PR is ready for review 5. **`needs-more-tests`** — if the change needs additional test coverage; triggers both L0 and L1 CI -6. **`high-complexity`** — if the PR is large, touches many files, or is prone to merge conflicts +6. **`full-test-suite`** — if the change has high blast radius (TE/MCore bumps, kernel changes, FP8 paths); pulls L2 into the PR run on top of L0+L1 +7. **`high-complexity`** — if the PR is large, touches many files, or is prone to merge conflicts Add risk labels when applicable: - `breaking-change` — if any public API, CLI argument, config key, or function signature changes - `needs-more-tests` — if the change needs additional test coverage (also triggers L1 CI) +- `full-test-suite` — if the change warrants the full L2 matrix (heavy: VL models, ckpt conversion, quantization) Properly labeled PRs get faster reviews and avoid sitting in the triage queue. @@ -254,6 +256,7 @@ Apply only when risk affects review or merge behavior: | `breaking-change` | Public behavior or API compatibility changes | | `high-complexity` | Harder to merge: prone to conflicts and needs additional test coverage | | `needs-more-tests` | Requires additional test coverage; triggers both L0 and L1 CI test tiers | +| `full-test-suite` | Pulls the L2 functional matrix (VL models, ckpt conversion, heavy quantization) into the PR run on top of L0+L1 | ### Area Labels @@ -358,7 +361,7 @@ Functional tests are placed in tiered launcher scripts inside [`tests/functional |------|--------|---------|---------| | **L0** | `L0_Launch_*.sh` | Every PR, main push, schedule | Core smoke tests — must be fast and stable | | **L1** | `L1_Launch_*.sh` | Main push + schedule; PRs labeled `needs-more-tests` | Broader model/recipe coverage | -| **L2** | `L2_Launch_*.sh` | Schedule / `workflow_dispatch` only | VL models, checkpoint conversion, heavy quantization | +| **L2** | `L2_Launch_*.sh` | Schedule / `workflow_dispatch`; PRs labeled `full-test-suite` | VL models, checkpoint conversion, heavy quantization | When adding a new launcher script, always start with the **L0** tier so it runs on every PR. A maintainer will adjust the tier later if the test is too slow or better suited for nightly coverage. diff --git a/skills/bump-dependency/SKILL.md b/skills/bump-dependency/SKILL.md index 18e24f5de7e..ac31c6c4a94 100644 --- a/skills/bump-dependency/SKILL.md +++ b/skills/bump-dependency/SKILL.md @@ -130,8 +130,11 @@ gh pr create \ ``` The `needs-more-tests` label is **mandatory** for a bump — it expands the -matrix from L0 to L0+L1 (see @skills/testing/SKILL.md tier table). L2 is -schedule-only and cannot be triggered by labels. +matrix from L0 to L0+L1 (see @skills/testing/SKILL.md tier table). For a +high-blast-radius bump (TE, MCore submodule, anything that touches CUDA +kernels), also add `full-test-suite` to pull L2 into the PR run — L2 +covers VL models, checkpoint conversion, and heavy quantization which +otherwise only run on schedule. `gh pr edit` is unreliable. To update a PR's title or body later, use the REST API directly: diff --git a/skills/cicd/SKILL.md b/skills/cicd/SKILL.md index 39aba290650..4c132d2f15f 100644 --- a/skills/cicd/SKILL.md +++ b/skills/cicd/SKILL.md @@ -46,7 +46,7 @@ pre-flight └── cicd-container-build ├── unit-tests-core ├── unit-tests-diffusion - └── functional-tests (L0 always; L1 with needs-more-tests label; L2 on schedule) + └── functional-tests (L0 always; L1 with needs-more-tests label; L2 on schedule or full-test-suite label) ``` - Slack notifications are sent on completion for scheduled and nightly runs. diff --git a/skills/testing/SKILL.md b/skills/testing/SKILL.md index fd20fad9319..9cb72a24c6a 100644 --- a/skills/testing/SKILL.md +++ b/skills/testing/SKILL.md @@ -30,7 +30,7 @@ scripts are named `{Tier}_{Description}.sh` (e.g., `L0_Launch_training.sh`). |---|---|---| | L0 | Every PR, every push to `main`, schedule | Yes — PR cannot merge if L0 fails | | L1 | Push to `main`, schedule, PRs with `needs-more-tests` label | Yes | -| L2 | Schedule and `workflow_dispatch` only | Yes (when triggered) | +| L2 | Schedule, `workflow_dispatch`, PRs with `full-test-suite` label | Yes (when triggered) | | flaky | `workflow_dispatch` with `test_suite=all` only | No — failures are informational | H100 and GB200 each have independent L0/L1/L2/flaky jobs. Moving a script to From e08e832d8b1adff9067eb205d373eb48a06e9c6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?oliver=20k=C3=B6nig?= Date: Tue, 5 May 2026 10:17:45 +0000 Subject: [PATCH 3/4] [docs] fix: register bump-dependency skill in toctree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: oliver könig --- docs/skills-index.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/skills-index.md b/docs/skills-index.md index 24127651f0f..c5445e2cdc0 100644 --- a/docs/skills-index.md +++ b/docs/skills-index.md @@ -10,6 +10,7 @@ and verification steps. :maxdepth: 1 skills/build-and-dependency/SKILL +skills/bump-dependency/SKILL skills/testing/SKILL skills/cicd/SKILL skills/mlm-bridge-training/SKILL From 6ba360d4956f07b3b4bb107ca0197e5dd00e4076 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?oliver=20k=C3=B6nig?= Date: Tue, 5 May 2026 12:10:08 +0000 Subject: [PATCH 4/4] [docs] refactor: trim bump-dependency skill duplications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: oliver könig --- skills/bump-dependency/SKILL.md | 158 ++++++++++++-------------------- 1 file changed, 61 insertions(+), 97 deletions(-) diff --git a/skills/bump-dependency/SKILL.md b/skills/bump-dependency/SKILL.md index ac31c6c4a94..390d993ed60 100644 --- a/skills/bump-dependency/SKILL.md +++ b/skills/bump-dependency/SKILL.md @@ -32,16 +32,19 @@ Read first, then follow the steps below: - @CONTRIBUTING.md — PR title/label policy, DCO sign-off - @skills/build-and-dependency/SKILL.md — `uv lock` mechanics, container choice - @skills/cicd/SKILL.md — how `copy-pr-bot` and `/ok to test` work -- @skills/testing/SKILL.md — `active/` vs `flaky/` directory layout +- @skills/testing/SKILL.md — `active/` vs `flaky/` directory layout, `git mv` quarantine recipe ## Step 1 — Worktree and edit +Create a worktree off `main` per @CLAUDE.md. Then, **before any `uv lock`**: + ```bash -# From the Megatron-Bridge repo root -git worktree add .claude/worktrees/ -b origin/main -git submodule update --init 3rdparty/Megatron-LM # required before `uv lock` +git submodule update --init 3rdparty/Megatron-LM ``` +The submodule must be initialised in the worktree or `uv lock` errors +with "not a Python project" on the MCore path. + Edit the pin. For TE the canonical knob is the override line in `pyproject.toml`: @@ -60,45 +63,49 @@ moving tip; use a full SHA for reproducibility. TE branches use ## Step 2 — Regenerate the lockfile -`uv.lock` is Linux + CUDA only. Run inside the project image: - -```bash -docker run --rm \ - -v $(pwd):/opt/Megatron-Bridge \ - -v $HOME/.cache/uv:/root/.cache/uv \ - -w /opt/Megatron-Bridge \ - megatron-bridge:latest \ - bash -c 'uv lock' -``` - -Confirm only the intended packages moved: +Run `uv lock` inside the project container per +@skills/build-and-dependency/SKILL.md "Regenerating uv.lock". Then +confirm only the intended packages moved: ```bash git diff --stat pyproject.toml uv.lock ``` If the diff carries changes you didn't ask for (transitive movements you -can't explain), stop and investigate before pushing. +can't explain), stop and investigate before pushing. Note that +`override-dependencies` carries CVE floors that float — unrelated +packages bumping by a patch version is expected; accept those, don't +revert them. ## Step 3 — Commit and push +Sign-off + signed-commit + PR title format per @CONTRIBUTING.md and +@skills/cicd/SKILL.md "Commit and PR Workflow". For a bump: + ```bash git add pyproject.toml uv.lock git commit -S -s -m "[build] chore: bump to " git push -u origin ``` -PR title format per @CONTRIBUTING.md: `[build] chore: bump to `. -Sign-off (`-s`) is required; signed commits (`-S`) let `copy-pr-bot` -trigger CI without needing `/ok to test` for every push. +A signed commit (`-S`) lets `copy-pr-bot` trigger CI without manual +`/ok to test` for the first push — but you'll still post `/ok to test` +on every subsequent SHA in this loop (Step 5). ## Step 4 — Open the PR -PR body goes through a tmpfile to preserve formatting. Wrap it in a -`
` block: +Title and labels per @CONTRIBUTING.md. Two bump-specific requirements: -```bash -cat > /tmp/pr-body.md <<'EOF' +- Apply `needs-more-tests` — **mandatory** for a bump; expands the matrix + from L0 to L0+L1. +- For a high-blast-radius bump (TE, MCore submodule, anything that + touches CUDA kernels), also apply `full-test-suite` to pull L2 into + the PR run. L2 covers VL models, checkpoint conversion, and heavy + quantization which otherwise only run on schedule. + +The PR body template — this is the durable record of the bump: + +```markdown
Claude summary ## What @@ -118,46 +125,19 @@ Updated -> _None yet — will be appended as flakes are identified during CI iteration._
-EOF - -gh pr create \ - --repo NVIDIA-NeMo/Megatron-Bridge \ - --base main \ - --head \ - --title "[build] chore: bump to " \ - --body-file /tmp/pr-body.md \ - --label "ci,area:build,needs-review,needs-more-tests" ``` -The `needs-more-tests` label is **mandatory** for a bump — it expands the -matrix from L0 to L0+L1 (see @skills/testing/SKILL.md tier table). For a -high-blast-radius bump (TE, MCore submodule, anything that touches CUDA -kernels), also add `full-test-suite` to pull L2 into the PR run — L2 -covers VL models, checkpoint conversion, and heavy quantization which -otherwise only run on schedule. - -`gh pr edit` is unreliable. To update a PR's title or body later, use the -REST API directly: - -```bash -gh api -X PATCH "repos/NVIDIA-NeMo/Megatron-Bridge/pulls/" \ - -F "body=@/tmp/pr-body.md" - -gh api -X PATCH "repos/NVIDIA-NeMo/Megatron-Bridge/pulls/" \ - -f "title=[build] chore: bump to " -``` +To update the PR title or body later, use `gh api -X PATCH +"repos/NVIDIA-NeMo/Megatron-Bridge/pulls/" -F "body=@/tmp/pr-body.md"` +— never `gh pr edit`. ## Step 5 — Trigger CI on the exact SHA -Even with a signed commit, post `/ok to test` on the SHA you actually -want exercised so any cached / cancelled run is re-fired: - -```bash -SHA=$(git rev-parse HEAD) -gh pr comment --repo NVIDIA-NeMo/Megatron-Bridge --body "/ok to test $SHA" -``` - -Use the **full** SHA (`git rev-parse HEAD`), never the short form. +Trigger mechanics live in @skills/cicd/SKILL.md "How CI Is Triggered". +For this loop the rule is simple: **on every new SHA you push, post +`/ok to test $(git rev-parse HEAD)`** as a PR comment, even if your +commits are signed. This guarantees the run targets the SHA you actually +want exercised and re-fires anything that got cancelled or cached. ## Step 6 — Attach the watchdog (always; never a cronjob) @@ -265,8 +245,7 @@ once the run is green. When a `JOB -> failure` event fires: -1. Skim the logs to confirm it's a flake / pre-existing issue, not the - bump itself: +1. **Triage the failure — is it the bump or a flake?** Skim the logs: ```bash RUN_ID= @@ -275,60 +254,47 @@ When a `JOB -> failure` event fires: tail -200 /tmp/run.log ``` - If the failure is caused by the bump (real regression, not a flake), - **stop quarantining** — fix the underlying issue or revert the bump. + This is the bump-specific judgement call: only quarantine if the + failure reproduces on `main` or is clearly unrelated infrastructure. + If the failure is caused by the bump (real regression), **stop + quarantining** — fix the underlying issue or revert the bump. Quarantining a real regression hides the very signal the bump PR exists to surface. -2. Move the launch script to `flaky/` on the matching hardware target - (see @skills/testing/SKILL.md): - - ```bash - git mv tests/functional_tests/launch_scripts/h100/active/_.sh \ - tests/functional_tests/launch_scripts/h100/flaky/_.sh - - # If the GB200 variant also failed: - git mv tests/functional_tests/launch_scripts/gb200/active/_.sh \ - tests/functional_tests/launch_scripts/gb200/flaky/_.sh - ``` - - Map a CI job name (e.g. `gb200_L0_Launch_models_foo`) to its launch - script via: +2. **Move the launch script to `flaky/`** per @skills/testing/SKILL.md + "Moving a Test to Flaky". Map a CI job name to its launch script via: - prefix `gb200_` → `gb200/active/`, otherwise `h100/active/` - the rest is the script's basename without `.sh` -3. Append the test to the PR description's **Quarantined tests** - section, with a one-line reason and a follow-up tracking link if you - have one. This is the durable record of what this bump deferred. +3. **Append to the PR body's Quarantined tests section** with a one-line + reason and a follow-up tracking link if you have one. This is the + durable record of what this bump deferred — the section exists + precisely so a reviewer can see at a glance which flakes were + side-stepped to land the bump. -4. Commit, push, retrigger: +4. **Commit, push, retrigger**: ```bash git commit -S -s -m "[ci] chore: quarantine flaky for bump" git push - SHA=$(git rev-parse HEAD) - gh pr comment --repo NVIDIA-NeMo/Megatron-Bridge --body "/ok to test $SHA" + gh pr comment --repo NVIDIA-NeMo/Megatron-Bridge \ + --body "/ok to test $(git rev-parse HEAD)" ``` -5. Update the PR body via `gh api PATCH` so the quarantine list stays - current. +5. **Update the PR body** via `gh api PATCH` so the quarantine list + stays current. -The watchdog is persistent — it will pick up the new run automatically -and emit `RUN STARTED` for the new attempt. +The watchdog is persistent — it picks up the new run automatically and +emits `RUN STARTED` for the new attempt. Loop back to step 1. ## Step 8 — Stop when green `RUN COMPLETED conclusion=success` is the exit condition. Then: ```bash -# Sanity check gh pr checks --repo NVIDIA-NeMo/Megatron-Bridge | awk '{print $2}' | sort | uniq -c - -# Tear down TaskStop() - -# Tick the boxes in the PR body gh api -X PATCH "repos/NVIDIA-NeMo/Megatron-Bridge/pulls/" -F "body=@/tmp/pr-body.md" ``` @@ -336,13 +302,11 @@ gh api -X PATCH "repos/NVIDIA-NeMo/Megatron-Bridge/pulls/" -F "body=@/tmp/pr- | Symptom | Cause | Fix | |---|---|---| -| `uv lock` errors with "not a Python project" on `3rdparty/Megatron-LM` | Submodule not initialised in the worktree | `git submodule update --init 3rdparty/Megatron-LM` | -| CI never starts on a new push | Commit not GPG-signed and no `/ok to test` for the new SHA | Post `/ok to test $(git rev-parse HEAD)` | -| Watchdog goes silent for 30+ min | `gh` rate-limited or auth expired | Bump poll interval; `gh auth status`; restart Monitor | -| Quarantine commit doesn't trigger a new run | Pushed but didn't post `/ok to test` for the new SHA | Always re-post on the new SHA | -| Job name doesn't match a script in `active/` | `gb200_` prefix is the hardware indicator, not part of the filename | Strip `gb200_` and look in `gb200/active/` | | Wrong TE branch ref (`release/v2.15`) silently resolves nothing | TE uses `release_vX.Y` with an underscore | Verify with `git ls-remote` before locking | | Lockfile diff includes unrelated CVE-pinned packages | `override-dependencies` carries floors that float | Re-run lock and accept; don't try to revert those | +| Signed first push triggers CI but later pushes don't | `copy-pr-bot` re-trusts on each new SHA only via `/ok to test` once you're past the first signed commit in this loop | Always re-post `/ok to test $(git rev-parse HEAD)` per Step 5 | +| Watchdog goes silent for 30+ min | `gh` rate-limited or auth expired | Bump poll interval; `gh auth status`; restart Monitor | +| Job name doesn't map to a script in `active/` | `gb200_` prefix is the hardware indicator, not part of the filename | Strip `gb200_` and look in `gb200/active/` | ## Anti-patterns