fix(ci): key the metric-history baseline on the executing stage - #2702
Open
yueming-yuan wants to merge 1 commit into
Open
fix(ci): key the metric-history baseline on the executing stage#2702yueming-yuan wants to merge 1 commit into
yueming-yuan wants to merge 1 commit into
Conversation
RunIdentity took its suite from registry.suite -- the stage a test declares in register_*_ci(suite=...) -- rather than the stage the run actually executed on. The two are the same today, because a test only ever runs in the job whose name equals its suite, so the gate has been correct by coincidence. That coincidence ends as soon as a test can be dispatched to a second GPU generation. A test homed on stage-c-8-gpu-h200 and executed on B200 would append Blackwell numbers to the Hopper series; mxfp8/nvfp4 values sit nowhere near their BF16/FP8 counterparts, so the gate would compare across populations and either fire constantly or widen until it detects nothing. The write path is narrower but reachable: a PR carrying both `nightly` and a dispatch label resolves to nightly cadence, which writes, and would poison the stored baseline for good. evaluate_gate now takes a required executing_suite. Required rather than defaulted to registry.suite: a silent fallback restores exactly the bug, and its symptom is slow data corruption rather than a crash. That costs ~40 updated call sites in the gate tests, all mechanical. No behaviour change -- run_suite passes args.suite, which filter_tests already guarantees equals every selected test's declared suite.
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Chain 3/5 — stacked on
b200-ci-hardware-axis.RunIdentitytook its suite fromregistry.suite— the stage a test declares inregister_*_ci(suite=...)— rather than the stage the run actually executed on. The two are the same today, because a test only ever runs in the job whose name equals its suite, so the gate has been correct by coincidence.That coincidence ends as soon as a test can be dispatched to a second GPU generation (4/5). A test homed on
stage-c-8-gpu-h200and executed on B200 would append Blackwell numbers to the Hopper series; mxfp8/nvfp4 values sit nowhere near their BF16/FP8 counterparts, so the gate would compare across populations and either fire constantly or widen until it detects nothing — losing the regression detection that already works on Hopper, as a side effect of adding Blackwell.The write path is narrower but reachable: a PR carrying both
nightlyand a dispatch label resolves to nightly cadence, which writes, and would poison the stored baseline for good.evaluate_gatenow takes a requiredexecuting_suite. Required rather than defaulted toregistry.suite: a silent fallback restores exactly the bug, and its symptom is slow data corruption rather than a crash. That costs ~40 updated call sites in the gate tests, all mechanical — the production change is 4 lines.Two new tests pin the point: an absorbed run writes under the executing stage, and the two stages keep independent series (0.42 recorded against Blackwell does not regress against Hopper's 0.81).
No behaviour change —
run_suitepassesargs.suite, whichfilter_testsalready guarantees equals every selected test's declared suite.🤖 Generated with Claude Code