fix: Quartz mis-reports pytorch/jax matrix-job test results by cgoea · Pull Request #71 · ROCm/Quartz

cgoea · 2026-08-12T12:20:43Z

Motivation

Two related bugs cause pytorch/jax test results to be silently dropped or mis-reported in status.json:

Cancelled/failed JAX test jobs silently disappear from variants. A calling orchestrator (e.g. rockrel) can wrap TheRock's own build job in a differently-cased ancestor segment. The nested test job has no (py, ref) segment of its own and relies entirely on that uppercase JAX ancestor being recognized — but the matrix-cell regex only matched lowercase jax/torch, so the job was invisible and its status (e.g. cancelled) never made it into the leaf's variants.
Pytorch test leaves report the wrong status per arch. A projected test leaf was inheriting the build run's raw GitHub conclusion verbatim instead of the worst-of rollup of its own variants, so leaves showed status: success while every variant underneath showed failure — observed across 16 linux architectures in pytorch.test.

closes #70

Technical Details

therock_update_status_json.py:
Made _MATRIX_JOB_RE case-insensitive and switched from .search() (first match) to taking the last match in a job's composite name -- a job's own tail segment (closest to the actual job) is authoritative, with an ancestor segment used only as a fallback when the job has no segment of its own.
_refresh_same_run_fanout_tests now recomputes the projected leaf's status via Variant.rollup_status(leaf.variants, leaf.status) instead of copying the build run's raw conclusion, matching how every other variant-carrying leaf derives its status.

Test Plan

Ran the full scripts/receive_therock/tests/ suite.

Test Result

402 passed.

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

HereThereBeDragons · 2026-08-12T13:53:17Z

        "publish_no_rocm_image_ubuntu24_04_rocgdb.yml",
        # Upstream test scaffolding (not Quartz-reported).
        "test_artifacts_structure.yml",
+        # Registered separately in users/cgoea/fix_test_artifacts; excluded


? probably want to remove that :)

Good catch, that was leftover from rebasing against users/cgoea/fix_test_artifacts. Removed in 6f2438a — develop already carries test_component.yml/test_jax_dockerfile.yml without that comment, so this branch's diff for the file is now a no-op.

HereThereBeDragons · 2026-08-12T14:51:34Z

+    # must fall back to the uppercase ancestor instead of being dropped.
+    run = _variant_run(
+        pipeline_type="jax",
+        pipeline_phase="build",


as it is build phase shouldnt it kick out jobs that are "test"?

No -- this is intentional and predates this PR (see test_reusable_matrix_nested_jobs_collapse_to_one_variant_per_cell, already on develop). test_pytorch_wheels.yml/test_linux_jax_wheels.yml run as a reusable workflow_call nested inside the delegated build orchestrator, not as their own workflow_dispatch -- so their jobs land in the same job list, same run id, as the build. _variants_from_jobs deliberately groups every job sharing a (py, ref) cell, build and nested test alike, into one Variant, because that's the only way _refresh_same_run_fanout_tests can see a cell's test outcome before the top-level run finishes. Filtering out "Test"-named jobs here would break that early-projection path. I expanded the docstring on _refresh_same_run_fanout_tests (6f2438a) to spell this out -- let me know if that answers it or if you want the filtering anyway for a different reason.

PyTorch/JAX test coverage runs as a reusable workflow_call nested inside the delegated build orchestrator so build and test jobs share one job list/run id, and _variants_from_jobs intentionally groups them together per py, ref cell so test outcomes can be seen before the top-level run finishes.

HereThereBeDragons · 2026-08-12T15:08:20Z

    ):
        return False

+    projected_status = Variant.rollup_status(leaf.variants, leaf.status)


shouldnt leaf.status become part of the valid rollup?
or should have at least some form of overwriting force if it is cancelled or failure?

Yes, good point -- fixed in 6f2438a. Since leaf.variants is guaranteed non-empty at this call site (checked above), leaf.status was only ever used as an empty-variants fallback and never actually influenced the result -- so a cancelled/failed run whose per-cell variants happened to look clean (e.g. a cell whose job never started and so never shows up in variants at all) would silently report the variants' rollup instead of the real outcome. Changed it to fold leaf.status into the rollup itself: rollup_statuses((*(v.status for v in leaf.variants), leaf.status), leaf.status). Added test_fanout_projection_folds_raw_run_conclusion_into_rollup to cover exactly that (all-success variants, but the run itself was cancelled).

HereThereBeDragons · 2026-08-12T15:09:24Z

    completion is classified as the build phase, so it would otherwise leave
    those same-run test leaves stale.
+
+    `leaf.status` is the *build* run's own top-level GitHub conclusion, which


build?
and where do we do the splitting for pytorch that has build and tests in the same workflow job list (as for testing it uses reusable worklfows and not workflow disaptch)

Fair -- "build" was ambiguous and I'd left a stale reference to a function (_merge_variant_leaf) that doesn't actually exist (it's _merge_matrix_build_leaf). Rewrote the docstring in 6f2438a to answer the splitting question directly: there is no job-name-level split. test_pytorch_wheels.yml/test_linux_jax_wheels.yml run as a reusable workflow_call nested inside the delegated release workflow, not their own workflow_dispatch, so their jobs share the same run id and job list as the entry build, and the registry (WORKFLOW_SPECS) classifies the whole thing as pipeline_phase="build". _variants_from_jobs groups build+test jobs into one Variant per (py, ref) cell regardless, and this function is what re-projects that already-merged variant list onto the separately-tracked test-phase leaf (matched by run_id/run_attempt), since the test-phase's own notification can be classified/arrive independently.

rewriting in a better way

HereThereBeDragons · 2026-08-12T15:10:32Z

also please adjust pr title to something non experts also get an idea what this is doing.

Same-run test-leaf projection ignored the build run's own terminal conclusion unless variants were empty, so a top-level cancellation/ failure could be masked by whatever individual matrix cells happened to report. Fold leaf.status into the rollup instead of only using it as an empty-variants fallback. Also drop a stale comment left over from a rebase referencing an unrelated branch, and clarify the same-run fanout docstring (fix a reference to a function that doesn't exist, and explain why build and test jobs share one job list for these delegated PyTorch/JAX runs).

cgoea · 2026-08-13T12:53:16Z

Addressed all four review comments in 6f2438a:

Removed the stale rebase-leftover comment referencing users/cgoea/fix_test_artifacts.
Folded the run's own top-level conclusion into the same-run test-leaf rollup (it was previously only used as an empty-variants fallback and never actually applied), with a new regression test.
Expanded/fixed the _refresh_same_run_fanout_tests docstring to answer the build/test job-list splitting question and fix a reference to a function that doesn't exist.
Renamed the PR title to something clearer for non-experts.

Replied inline to each thread with more detail.

HereThereBeDragons · 2026-08-13T14:54:31Z

    assert leaf.variants[0].status is Status.failure


+def test_fanout_projection_folds_raw_run_conclusion_into_rollup() -> None:


i think here is an error

if i understand the test correctly:
pytorch test for gfx110X-all

have a in_progress run on linux
have a successful test run but cancel the entire pytorch testing run on windows

now at the end when checking the status of linux should copy from windows and show cancelled?

that does not make sense because windows and linux have separate pytorch workflows. so this kind of rollup should only happen within a platform.

No, no. agree it reads as confusing, since it looks like it's testing platform mixing when it's really testing field-independence. the reason the test sets cancelled_build.classification.platform = "windows" while the stale leaf is "linux" (both keyed to the same run_id=903) is to deliberately prove the matching predicate really is just (run_id, run_attempt) and doesn't silently depend on platform equality. I'll drop the platform override in my new test (default to "linux", matching the stale leaf).

and doesn't silently depend on platform equality

but it should depend on the platform. pytorch test on windows are totally independent of linux - even for the same arch

test_fanout_projection_folds_raw_run_conclusion_into_rollup doesn't need a platform mismatch to make its point (that's already covered by test_completed_fanout_build_refreshes_same_run_test_leaves), and it read as if a windows run could leak into a linux leaf. Drop it there, and add a comment on the older test explaining why the mismatch is intentional where it remains.

…Quartz into users/cgoea/fix_matrix_job

…trix_job # Conflicts: # scripts/receive_therock/tests/therock_update_status_json_test.py

43cd224, fix: Quartz mis-reports pytorch/jax matrix-job test results (#71), Ciprian Goea (ciprian.goea@amd.com), Thu Aug 13 19:35:28 2026 +0300

cgoea added 2 commits August 12, 2026 11:53

fix: _MATRIX_JOB_RE should be lower/uppercase agnostic

393383d

add regression tests for matrix-job fixes

927d15c

cgoea requested a review from a team August 12, 2026 12:20

cgoea changed the title ~~Users/cgoea/fix matrix job~~ fix: _MATRIX_JOB_RE changed to case insensitive & _refresh_same_run_fanout_tests recomputes leaf status Aug 12, 2026

cgoea changed the base branch from main to develop August 12, 2026 12:38

HereThereBeDragons reviewed Aug 12, 2026

View reviewed changes

cgoea changed the title ~~fix: _MATRIX_JOB_RE changed to case insensitive & _refresh_same_run_fanout_tests recomputes leaf status~~ fix: Quartz mis-reports pytorch/jax matrix-job test results Aug 13, 2026

cgoea added 2 commits August 13, 2026 16:03

address comments from reviewer

c19a9e1

make comment more clear

d9ecc99

cgoea requested a review from HereThereBeDragons August 13, 2026 13:11

HereThereBeDragons reviewed Aug 13, 2026

View reviewed changes

cgoea added 2 commits August 13, 2026 18:36

test: clarify the deliberate platform mismatch

7e45584

HereThereBeDragons approved these changes Aug 13, 2026

View reviewed changes

cgoea added 2 commits August 13, 2026 19:17

Merge branch 'users/cgoea/fix_matrix_job' of https://github.com/ROCm/…

d6f55af

…Quartz into users/cgoea/fix_matrix_job

Merge remote-tracking branch 'origin/develop' into users/cgoea/fix_ma…

a36a860

…trix_job # Conflicts: # scripts/receive_therock/tests/therock_update_status_json_test.py

cgoea merged commit 43cd224 into develop Aug 13, 2026
1 check passed

cgoea deleted the users/cgoea/fix_matrix_job branch August 13, 2026 16:35

quartz-sync-github-app Bot pushed a commit that referenced this pull request Aug 13, 2026

Merge develop (43cd224) into main

4e4d260

43cd224, fix: Quartz mis-reports pytorch/jax matrix-job test results (#71), Ciprian Goea (ciprian.goea@amd.com), Thu Aug 13 19:35:28 2026 +0300

		assert leaf.variants[0].status is Status.failure


		def test_fanout_projection_folds_raw_run_conclusion_into_rollup() -> None:

Uh oh!

Conversation

cgoea commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HereThereBeDragons commented Aug 12, 2026

Uh oh!

cgoea commented Aug 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cgoea commented Aug 12, 2026 •

edited

Loading