Skip to content

test(release-matrix): proposal — pytest suite for release-gate trio#253

Draft
Sujimoshi wants to merge 2 commits into
Avarok-Cybersecurity:mainfrom
Sujimoshi:test/pytest-release-matrix
Draft

test(release-matrix): proposal — pytest suite for release-gate trio#253
Sujimoshi wants to merge 2 commits into
Avarok-Cybersecurity:mainfrom
Sujimoshi:test/pytest-release-matrix

Conversation

@Sujimoshi

@Sujimoshi Sujimoshi commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Motivation

Grew out of review discussion on #249's release-gate pipeline (run_all_models.py + single_gpu_suite.py + gate_results.py). Three points from that review:

  1. gate_results.py's verdict() only floor-checks tps > 0 — it can't catch a real perf regression (e.g. tps dropping 40%) as long as the number stays positive.
  2. The suite's prompts/checks aren't standardized as reusable, individually-inspectable assertions.
  3. The whole pipeline is a set of orchestration scripts, not something pytest/CI can run, filter, or report on natively.

This PR is a proposal, not a drop-in replacement: it adds a new, self-contained pytest module alongside the existing three scripts to make the architecture concrete and discussable.

Architecture

  • deployed_atlas / deployed_atlas_ep2 fixtures own the container lifecycle: build the exact serve command (ported from build_serve_cmd/build_ep2_serve_cmd), wait for "Listening on" in logs, send a warmup request, yield (spec, base_url), then stop + sleep 30s (GB10 unified-memory settle, same constant as INTER_ROUND_SETTLE_SECONDS). Boot failure is a hard pytest.fail for that model, not a silent skip — this fixture is the coverage-enforcement mechanism.
  • Snapshot-based regression check: test_tps_no_regression compares the measured tps against a committed tests/baselines/<label>.json with a 10% tolerance, replacing the tps > 0 floor check. --update-baselines switches the test into write-only mode (no assertion) for a deliberate, reviewed baseline refresh.
  • GPU-topology-aware xdist grouping: every test carries pytest.mark.xdist_group(name=...) keyed to spec.host ("head" / "worker") for single-GPU specs, or "multirank" for EP=2/TP=2/TP+EP specs (which occupy both physical GPUs at once). pytest --dist=loadgroup -n 2 then serializes same-GPU rounds while head/worker run in parallel; multirank rounds run as a separate -m multirank pass.
  • Point debugging: Spec parametrization uses ids=lambda s: s.label, so pytest -k "<label> and test_name" runs exactly one model's one test — no other spec's fixture is even instantiated.

21 specs total: 15 single-GPU (SPECS, ported from ROUNDS) + 5 EP=2 (EP2_SPECS) + 0 TP=2 (TP2_SPECS, kept empty per upstream's documented OOM issue) + 1 TP+EP (TPEP_SPECS). EP4_ROUNDS is intentionally excluded (no 4-node driver support yet).

File layout

The suite is split by functional axis under tests/release_matrix/ (the combined file grew to ~970 lines):

  • specs.pySpec dataclass, config constants, the spec matrix
  • docker.py — container lifecycle (build_serve_cmd/build_ep2_serve_cmd, start/wait/stop, warmup, EP=2 readiness)
  • api.py — HTTP-API helpers (chat, text extraction, tps calc, repetition-loop detection, long-context/tool fixtures)
  • assertions.py — the five shared assertion bodies (coherence, fibonacci, tool-call, tps, long-context)
  • baselines.py — baseline load/write helpers

tests/test_release_matrix.py now only holds the deployed_atlas/deployed_atlas_ep2 fixtures and the two test classes. tests/conftest.py (the two pytest hooks) is untouched.

Not included / follow-up

  • Real baseline JSON files. This was drafted without GPU access. tests/baselines/ ships with only a .gitkeep + README — actual <label>.json files need someone to run pytest tests/test_release_matrix.py --update-baselines on real GB10 hardware, then review and commit the resulting diff separately.
  • Migration/removal of run_all_models.py / single_gpu_suite.py / gate_results.py. This PR only adds the new file; it does not touch or delete the existing three scripts. Whether/when to retire them is a separate decision.

Testing

  • python3 -m py_compile tests/test_release_matrix.py tests/conftest.py — OK
  • pytest tests/test_release_matrix.py --collect-only105 tests collected (21 specs × 5 test categories: coherence, fibonacci, tool_calls, tps_no_regression, long_context)
  • pytest tests/test_release_matrix.py -k "27B-dense-nvfp4 and test_tps_no_regression" --collect-only1/105 collected (104 deselected) — confirms point debugging only touches one spec
  • pytest tests/test_release_matrix.py -m multirank --collect-only30/105 collected (75 deselected) — confirms the EP=2/TP=2/TP+EP group is correctly isolated
  • No real docker/GPU execution was performed (no hardware access while drafting) — collection-only, as expected for this draft
  • Re-verified all four checks above after the file split — identical results (105 / 1/105 / 30/105), confirming the split didn't change behavior

🤖 Generated with Claude Code

Sujimoshi and others added 2 commits July 4, 2026 22:39
…gpu_suite/gate_results trio

Grew out of review discussion on Avarok-Cybersecurity#249's release-gate pipeline: the orchestrator
(run_all_models.py) + suite (single_gpu_suite.py) + gate (gate_results.py) trio
only floor-checks tps > 0, has no snapshot regression detection, and is not
wired into pytest/CI as real, individually addressable tests.

Proposes replacing the trio with a single self-contained pytest module:
- deployed_atlas / deployed_atlas_ep2 fixtures own container lifecycle
  (build the serve command, wait for "Listening on", warm up, teardown +
  GB10 unified-memory settle), hard pytest.fail on boot failure instead of
  a silent manifest-based skip.
- test_tps_no_regression compares against a committed tps snapshot with a
  10% tolerance (tests/baselines/<label>.json) instead of gate_results.py's
  tps > 0 floor check; --update-baselines writes a fresh snapshot without
  asserting, for manual review before merge.
- pytest.mark.xdist_group(name=spec.host) keys tests to "head"/"worker"/
  "multirank" so `pytest --dist=loadgroup -n 2` serializes same-GPU rounds
  while head/worker run in parallel.
- Spec parametrization (ids=lambda s: s.label) makes single-model debugging
  a plain `pytest -k "<label> and test_name"`.

Not included in this PR (see description for follow-up plan):
- Real baseline JSON files (no GPU access while drafting this).
- Migration/removal of the existing three scripts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…_matrix/ package

The single-file suite grew to ~970 lines, hard to navigate for a proposal
meant to be reviewed line-by-line. Split by functional axis into
tests/release_matrix/: specs.py (Spec dataclass, config constants, the
SPECS/EP2_SPECS/TP2_SPECS/TPEP_SPECS/MULTIRANK_SPECS matrix), docker.py
(container lifecycle: build_serve_cmd/build_ep2_serve_cmd, start/wait/stop,
warmup, EP=2 readiness), api.py (HTTP-API helpers: chat, text extraction,
tps calculation, repetition-loop detection, long-context/tool fixtures),
assertions.py (the five shared assertion bodies), and baselines.py
(baseline load/write helpers). tests/test_release_matrix.py now only holds
the deployed_atlas/deployed_atlas_ep2 fixtures and the two test classes,
importing everything else from release_matrix.*.

No behavior change: same 21 specs, same 5 test categories per spec, same
105 collected tests, same -k point-debug and -m multirank isolation.
BASELINE_DIR now resolves via the package's parent directory instead of
__file__'s own directory, since it moved one level deeper.

tests/conftest.py is untouched — it only holds the two pytest hooks
(--update-baselines option, multirank marker) and doesn't need splitting.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
tbraun96 pushed a commit that referenced this pull request Jul 5, 2026
The gate's tps bar was liveness-only (avg > 0), so a real perf regression
(e.g. tps dropping 40%) passed as long as the number stayed positive. Add a
committed-baseline regression check: tests/baselines/<label>.json records the
blessed tokens/sec, and a run more than TPS_TOLERANCE (10%) below it fails.

- verdict() stays pure — takes the baseline dict as an argument (SBIO).
- No baseline for a label => liveness-only with a loud note, not a silent pass;
  --require-baselines makes a missing baseline a hard fail for a milestone (PCND:
  the stricter bar is explicit opt-in, mirroring --allow-missing-manifest).
- gate_results.py --update-baselines writes {"tps": avg} per label from the
  present results for a deliberate, reviewed refresh (exit 0, no gating).
- Roster stays the single manifest SSOT in run_all_models.py; baselines key to
  those labels — no second matrix.
- tests/test_gate_results.py extended to 23 tests (regression / within-tolerance
  / no-baseline / require-baselines / dead-server-first / update round-trip).

Baseline-snapshot approach proposed by @Sujimoshi in #253; folded into the
existing gate to keep one SSOT rather than a parallel pytest suite.

Co-Authored-By: Igor Solomakha <sujimoshi@users.noreply.github.com>
@tbraun96

tbraun96 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Thanks for this @Sujimoshi — the critique is spot on. You're right that the gate's tps bar was liveness-only (avg > 0), so a genuine perf regression sailed through as long as the number stayed positive. The committed-baseline snapshot with a tolerance band + an explicit --update-baselines refresh is exactly the right shape for catching that.

I've folded that idea straight into the existing gate on #249 (commit f87bcfa), crediting you as co-author:

  • tests/baselines/<label>.json records the blessed tok/s; a run > TPS_TOLERANCE (10%) below it fails.
  • No baseline yet ⇒ liveness-only with a loud note (so we don't ship a red gate before hardware runs bless the numbers); --require-baselines makes a missing baseline a hard fail for a milestone — mirrors the existing --allow-missing-manifest opt-out.
  • gate_results.py --update-baselines writes {"tps": avg} per label from the results for a deliberate, reviewed refresh.
  • verdict() stays pure (takes the baseline dict as an arg); tests grew to 23 (regression / within-tolerance / no-baseline / require-baselines / update round-trip).

On the rest of the PR — the reason I folded the one idea in rather than merging this as-is: it re-ports the whole matrix (ROUNDSSPECS), the serve-cmd builders, and the five assertion bodies into a second parallel copy while leaving the original trio in place. That leaves two rosters and two sets of builders to keep in sync, which is the SSOT problem #249 was specifically trying to close.

That said — going pytest-native for the suite layer is a legitimate direction and your -k point-debugging + xdist topology grouping are genuinely nice. If you want to take it there, the clean way is a replacement, not an addition:

  1. Import the spec matrix + build_serve_cmd/build_ep2_serve_cmd from the existing modules instead of copying them (one SSOT), and
  2. delete run_all_models.py / single_gpu_suite.py / gate_results.py in the same PR, and
  3. keep an explicit full-roster coverage assertion — since a release gate must fail on "model X was never even attempted", and pytest's -k/-m/sharding all make green-on-subset a first-class feature.

One caveat worth knowing for (3): these tests need GPUs + sudo docker + SSH-to-worker, so CI only ever --collect-onlys them — the coverage guarantee has to be structural, not "CI ran them all".

Really appreciate the sharp eye here — would love to see you take the migration if you're up for it, and happy to review.

AzeezIsh added a commit that referenced this pull request Jul 8, 2026
* tests: coverage-enforcing serve-matrix release gate

run_all_models.py boots each model×quant and writes per-model JSON but never
exits non-zero on failure — so "verified iff the matrix passes" was a human
reading a table, and a model that crashed at boot (no JSON written) was
invisible. A glob-only scorer would report the survivors green: a false ship.

- run_all_models.py now writes all_models_results/_manifest.json — the roster
  it planned to cover, derived from planned_specs() (SSOT for the roster).
- gate_results.py reads that manifest and fails any planned model with no
  passing result. Missing manifest is a hard FAIL unless --allow-missing-manifest
  is passed (explicit, coverage-unenforced). Exit 0 ships, 1 blocks.
- tests/test_gate_results.py (stdlib unittest, no GPU) locks in the bars and the
  planned-but-absent = FAIL property.

* docs: GB10 deployment guide, atlas-release skill, CI/CLA convergence

Two contributor-facing deliverables plus the low-risk convergence they exposed.

GB10 deployment & compatibility guide (docs/GB10_DEPLOYMENT_GUIDE.md):
the one page to read before picking a model — model×quant matrix (single-node +
scale-out), goal→recipe picker, weight/KV quant + OOM ladder, consolidated
known-issues, EP=2 troubleshooting, and what "verified" means. Routes to the
recipes/QUICKSTART/DEPLOYMENT rather than duplicating them; recipe defaults stay
the serve-config SSOT. Linked from the README as the welcoming first-read.

atlas-release skill (.claude/skills/atlas-release/): the build → verify → image →
publish pipeline + upstream-sync PR automation, grounded in the real commands,
containers, tags, and gates already in-tree. De-personalized so any contributor
can run it. (Under .claude/, which is gitignored — force-added so reviewers see
it; drop the path if skills should stay out of the tree.)

Convergence the above surfaced, applied where unambiguous:
- scripts/check-license-headers.sh — the script AGENTS.md / PR template / book
  all referenced but that never existed; thin wrapper over the same
  skywalking-eyes engine CI runs.
- docker/gb10/Dockerfile — ARG ATLAS_GIT_SHA + revision LABEL so the image
  records its commit (release.sh already passed the build-arg into a no-op).
- .github/workflows/cla.yml — allowlist maintainer identities so AI-authored
  maintainer PRs skip the CLA ceremony (external contributors still covered).
- AGENTS.md — file-size cap 250→500 to match the CI SSOT; clippy/license lines
  aligned to CI; model-matrix pointer → the new guide.
- Dead docs/EP2-TROUBLESHOOTING.md links (DEPLOYMENT, book, adr/0007) repointed
  to the guide §7; CONTRIBUTING notes CI-green ≠ shippable; docs/releases/ stub.

* tests: baseline-managed tps regression bar in the release gate

The gate's tps bar was liveness-only (avg > 0), so a real perf regression
(e.g. tps dropping 40%) passed as long as the number stayed positive. Add a
committed-baseline regression check: tests/baselines/<label>.json records the
blessed tokens/sec, and a run more than TPS_TOLERANCE (10%) below it fails.

- verdict() stays pure — takes the baseline dict as an argument (SBIO).
- No baseline for a label => liveness-only with a loud note, not a silent pass;
  --require-baselines makes a missing baseline a hard fail for a milestone (PCND:
  the stricter bar is explicit opt-in, mirroring --allow-missing-manifest).
- gate_results.py --update-baselines writes {"tps": avg} per label from the
  present results for a deliberate, reviewed refresh (exit 0, no gating).
- Roster stays the single manifest SSOT in run_all_models.py; baselines key to
  those labels — no second matrix.
- tests/test_gate_results.py extended to 23 tests (regression / within-tolerance
  / no-baseline / require-baselines / dead-server-first / update round-trip).

Baseline-snapshot approach proposed by @Sujimoshi in #253; folded into the
existing gate to keep one SSOT rather than a parallel pytest suite.

Co-Authored-By: Igor Solomakha <sujimoshi@users.noreply.github.com>

---------

Co-authored-by: Azeez Ishaqui <debaterishaqui@gmail.com>
Co-authored-by: Igor Solomakha <sujimoshi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants