diff --git a/scripts/receive_therock/tests/therock_update_status_json_test.py b/scripts/receive_therock/tests/therock_update_status_json_test.py index ffa42f7fb..465509324 100644 --- a/scripts/receive_therock/tests/therock_update_status_json_test.py +++ b/scripts/receive_therock/tests/therock_update_status_json_test.py @@ -1625,10 +1625,12 @@ def test_pytorch_build_variants_from_matrix_job_names() -> None: assert all(v.run_id == 555 and v.status is Status.success for v in variants) -def test_reusable_matrix_nested_jobs_collapse_to_one_variant_per_cell() -> None: - # A reusable-workflow matrix expands each cell into several nested jobs that - # all carry the cell's prefix; they roll up into one variant (worst-of - # status, not-terminal until every nested job finishes). +def test_build_leaf_excludes_nested_test_subjobs() -> None: + # A reusable-workflow build run's job list carries both the cell's build + # sub-job and its nested per-arch test sub-jobs. The build leaf must reflect + # the build sub-job alone: an in-progress (or failed) nested test job belongs + # to the test leaf and must not drag the build cell's status or completion + # (the #82 regression, where the build leaf absorbed test cells). run = _variant_run( pipeline_type="pytorch", pipeline_phase="build", @@ -1644,8 +1646,8 @@ def test_reusable_matrix_nested_jobs_collapse_to_one_variant_per_cell() -> None: variants = tusj._derive_variants(run) assert len(variants) == 1 assert variants[0].matrix == {"py": "3.12", "torch": "release/2.10"} - assert variants[0].status is Status.in_progress - assert variants[0].completed_at is None + assert variants[0].status is Status.success + assert variants[0].completed_at is not None def test_jax_build_variants_use_jax_ref_axis() -> None: @@ -1952,12 +1954,19 @@ def test_fanout_projection_folds_raw_run_conclusion_into_rollup() -> None: # matching (already covered by # test_completed_fanout_build_refreshes_same_run_test_leaves), so it # doesn't need a platform mismatch to make its point. + # The py 3.12 build sub-job succeeded (a test job for that cell only exists + # because its build did): the run-level "cancelled" comes from some *other* + # cell that never started, not from this cell's build. cancelled_build = _variant_run( pipeline_type="pytorch", pipeline_phase="build", run_id=903, conclusion="cancelled", jobs=[ + _job( + "Build | py 3.12 | torch release/2.10 / Build", + conclusion="success", + ), _job( "Build | py 3.12 | torch release/2.10 / Test | gfx110X-all", conclusion="success", @@ -1971,14 +1980,26 @@ def test_fanout_projection_folds_raw_run_conclusion_into_rollup() -> None: leaf = doc.pipelines.pytorch.test["linux"]["gfx110X-all"] assert leaf.status is Status.cancelled + # The build leaf keeps its own success: the cancelled run conclusion folds + # into the test rollup (above), but must not drag down a build cell whose + # build sub-job succeeded (#82). + build_leaf = doc.pipelines.pytorch.build["linux"] + assert build_leaf.status is Status.success + assert build_leaf.variants is not None + assert {v.matrix.get("py") for v in build_leaf.variants} == {"3.12"} + assert build_leaf.variants[0].status is Status.success -def test_variant_job_name_matches_uppercase_ancestor_segment() -> None: - # A calling orchestrator (e.g. rockrel) can wrap TheRock's own build job - # in a differently-cased ancestor segment, e.g. + +def test_build_leaf_own_tail_ref_wins_and_test_only_cell_excluded() -> None: + # A calling orchestrator (e.g. rockrel) wraps TheRock's own build job in a + # differently-cased ancestor segment carrying a looser ref: # "Release | py 3.12 | JAX 0.11.0 / Build | py 3.12 | jax rocm-jaxlib-v0.11.0" - # The build job's own tail (lowercase, full ref) must still win over that - # ancestor. A nested test sub-job has no (py, ref) segment of its own and - # must fall back to the uppercase ancestor instead of being dropped. + # The build job's own tail (lowercase, full ref) must win over that ancestor. + # A py 3.13 cell that expands into a nested test sub-job ONLY (no build job of + # its own) is a test cell, not a build cell: it borrows the uppercase ancestor + # ref and must be excluded from the build leaf entirely -- the #82 regression, + # where that phantom cell doubled the build leaf and let its cancelled test + # flip the build status. run = _variant_run( pipeline_type="jax", pipeline_phase="build", @@ -1997,10 +2018,144 @@ def test_variant_job_name_matches_uppercase_ancestor_segment() -> None: ) variants = tusj._derive_variants(run) by_py = {v.matrix["py"]: v for v in variants} + assert set(by_py) == {"3.12"} assert by_py["3.12"].matrix["jax_ref"] == "rocm-jaxlib-v0.11.0" assert by_py["3.12"].status is Status.success - assert by_py["3.13"].matrix["jax_ref"] == "0.11.0" - assert by_py["3.13"].status is Status.cancelled + + +def test_jax_rockrel_build_leaf_not_flipped_by_cancelled_test() -> None: + # The #82 shape end-to-end: a rockrel-orchestrated jax build run whose nested + # test sub-job was cancelled. The test job borrows the uppercase ancestor ref + # ("JAX 0.10.2" -> "0.10.2") while the build job's own tail carries the full + # ref ("rocm-jaxlib-v0.10.2"); before the fix those landed as two cells and + # the cancelled test flipped the build. The build leaf must now stay success + # and carry exactly one cell. + doc = StatusDocument() + run = _variant_run( + pipeline_type="jax", + pipeline_phase="build", + run_id=930, + conclusion="success", + jobs=[ + _job( + "build_jax_wheels / Release | py 3.12 | JAX 0.10.2 / " + "Build | py 3.12 | jax rocm-jaxlib-v0.10.2" + ), + _job( + "build_jax_wheels / Release | py 3.12 | JAX 0.10.2 / " + "Test | gfx94X-dcgpu | linux-gfx942-1gpu / Test JAX | gfx94X-dcgpu", + conclusion="cancelled", + ), + ], + ) + tusj._merge_run_into_document(doc, run, tusj._create_leaf(run)) + + build_leaf = doc.pipelines.jax.build["linux"] + assert build_leaf.status is Status.success + assert len(build_leaf.variants) == 1 + assert build_leaf.variants[0].matrix == { + "py": "3.12", + "jax_ref": "rocm-jaxlib-v0.10.2", + } + + +def test_test_snapshot_finalizes_same_run_build_leaf() -> None: + # Shared-run topology: a pytorch/jax build workflow calls its test workflow + # via workflow_call, so the reusable test's own notify carries the whole + # parent run's job list -- the finished build sub-jobs included. The build + # leaf can be stuck in_progress (its finalizing notify not yet fired) when a + # test-phase notify arrives mid-run with the build sub-job already terminal. + # That test snapshot must finalize the same-run build leaf early. + doc = StatusDocument() + + building = _variant_run( + pipeline_type="pytorch", + pipeline_phase="build", + run_id=910, + conclusion=None, + jobs=[ + _job( + "Build | py 3.12 | torch release/2.10 / Build", + conclusion=None, + completed=None, + ), + ], + ) + tusj._merge_run_into_document(doc, building, tusj._create_leaf(building)) + assert doc.pipelines.pytorch.build["linux"].status is Status.in_progress + + # The nested test workflow (same run id) reports: the build sub-job has now + # finished, plus its own still-running test job. + test_run = _variant_run( + pipeline_type="pytorch", + pipeline_phase="test", + architectures=["gfx942"], + run_id=910, + conclusion=None, + jobs=[ + _job("Build | py 3.12 | torch release/2.10 / Build"), + _job( + "Build | py 3.12 | torch release/2.10 / Test | gfx942", + conclusion=None, + completed=None, + ), + ], + ) + tusj._merge_run_into_document(doc, test_run, tusj._create_leaf(test_run)) + + build_leaf = doc.pipelines.pytorch.build["linux"] + assert build_leaf.status is Status.success + assert build_leaf.completed_at is not None + assert len(build_leaf.variants) == 1 + assert build_leaf.variants[0].matrix == {"py": "3.12", "torch": "release/2.10"} + # The test snapshot's still-running test job must not appear in the build + # leaf; the test leaf tracks it instead. + assert doc.pipelines.pytorch.test["linux"]["gfx942"].status is Status.in_progress + + # Finalizing the build leaf early must not mark the pipeline done: the + # rollup still shows the build as success but pytorch -- and the run + # overall -- stays in_progress while the test cell runs. + assert doc.summary.linux.pytorch.build.status is Status.success + assert doc.summary.linux.pytorch.test.in_progress == 1 + assert doc.summary.overall_status is Status.in_progress + + +def test_standalone_test_run_does_not_touch_build_leaf() -> None: + # A standalone test dispatch (test_pytorch_wheels_full.yml) has its OWN run + # id, distinct from the build run's. Its snapshot must never rewrite the real + # build run's leaf -- the mirror is guarded to the shared-run id. + doc = StatusDocument() + build = _variant_run( + pipeline_type="pytorch", + pipeline_phase="build", + run_id=920, + jobs=[_job("Build | py 3.12 | torch release/2.10 / Build")], + ) + tusj._merge_run_into_document(doc, build, tusj._create_leaf(build)) + assert doc.pipelines.pytorch.build["linux"].status is Status.success + + standalone_test = _variant_run( + pipeline_type="pytorch", + pipeline_phase="test", + architectures=["gfx942"], + run_id=921, + conclusion="failure", + jobs=[ + _job( + "Build | py 3.12 | torch release/2.10 / Test | gfx942", + conclusion="failure", + ), + ], + ) + tusj._merge_run_into_document( + doc, standalone_test, tusj._create_leaf(standalone_test) + ) + + after = doc.pipelines.pytorch.build["linux"] + assert after.status is Status.success + assert len(after.variants) == 1 + assert after.variants[0].run_id == 920 + assert after.variants[0].status is Status.success def test_skip_workflow_names_are_all_disregarded(tmp_path: Path) -> None: diff --git a/scripts/receive_therock/therock_status_document.py b/scripts/receive_therock/therock_status_document.py index da029ba06..11c388f70 100644 --- a/scripts/receive_therock/therock_status_document.py +++ b/scripts/receive_therock/therock_status_document.py @@ -376,7 +376,7 @@ def merge_matrix_test_leaf(existing: "RunLeaf | None", new: "RunLeaf") -> "RunLe """Merge fan-out test variants by matrix cell instead of replacing the whole leaf (mirrors `_merge_variant_leaf`, but also carries forward `run_id`/`run_attempt`/timestamps so later comparisons -- e.g. - `_refresh_same_run_fanout_tests` in therock_update_status_json.py, its + `_refresh_same_run_tests_from_build` in therock_update_status_json.py, its other caller -- can still identify the owning run). Every completion notification for a shared-entry-run matrix (pytorch/jax diff --git a/scripts/receive_therock/therock_update_status_json.py b/scripts/receive_therock/therock_update_status_json.py index d917d5d96..120f7380e 100644 --- a/scripts/receive_therock/therock_update_status_json.py +++ b/scripts/receive_therock/therock_update_status_json.py @@ -348,7 +348,12 @@ def _update_document_metadata( # follows in the same job name (e.g. "linux-gfx942-1gpu-..." above, which # names a *different* family string than the job's own "Test | gfx94X-dcgpu" # segment) and reintroduce the cross-arch conflation this exists to prevent. -_TEST_ARCH_JOB_RE = re.compile(rf"Test\s*\|\s*(?P{GPU_FAMILY_TOKEN})") +# Case-insensitive (like `_MATRIX_JOB_RE`): this regex is also the build-vs-test +# partition signal in `_is_test_subjob`, so a differently-cased "test |" segment +# must not be misread as a build sub-job. +_TEST_ARCH_JOB_RE = re.compile( + rf"Test\s*\|\s*(?P{GPU_FAMILY_TOKEN})", re.IGNORECASE +) # pipeline_type -> the matrix axis key used in the variant (reference schema: # pytorch cells key the ref as "torch", jax cells as "jax_ref"). @@ -398,6 +403,14 @@ def _job_matches_arch(job_name: str, arch: str) -> bool: return not named or arch in named +def _is_test_subjob(job_name: str) -> bool: + """True if the job is a nested test sub-job (carries a "Test | " + segment of its own). A build sub-job never does. Used to partition a + shared-run job list into its build half and its test half so a build leaf + never absorbs test cells and vice versa.""" + return bool(_TEST_ARCH_JOB_RE.search(job_name)) + + def _job_archs(workflow_run: WorkflowRunRecord) -> frozenset[str]: """Distinct architectures named in any job's own "Test | " segment.""" jobs = ( @@ -409,7 +422,11 @@ def _job_archs(workflow_run: WorkflowRunRecord) -> frozenset[str]: def _variants_from_jobs( - workflow_run: WorkflowRunRecord, axis_key: str, *, arch: str | None = None + workflow_run: WorkflowRunRecord, + axis_key: str, + *, + arch: str | None = None, + phase: str | None = None, ) -> list[Variant]: """One variant per (py, ref) matrix cell parsed from job names. @@ -417,12 +434,24 @@ def _variants_from_jobs( architectures (see `_TEST_ARCH_JOB_RE`). Passing `arch` scopes the cell to that architecture's own jobs plus any arch-agnostic job, so one architecture's result can never roll up into another's variant. + + In a shared-run workflow (a build workflow that calls its test workflow via + `workflow_call`), the notify job list carries BOTH the build sub-jobs and + the nested test sub-jobs. `phase` (a classification `pipeline_phase`) + partitions that list: "build" keeps only the build sub-jobs (dropping nested + "Test | " jobs), "test"/"test-full" keep only the test sub-jobs. + Without it a build leaf would absorb the test cells, doubling cells and + letting a failed test flip the build status. """ jobs = ( workflow_run.api_jobs if workflow_run.api_jobs is not None else workflow_run.jobs ) + if phase == "build": + jobs = [j for j in jobs if not _is_test_subjob(j.name)] + elif phase in ("test", "test-full"): + jobs = [j for j in jobs if _is_test_subjob(j.name)] if arch is not None: jobs = [j for j in jobs if _job_matches_arch(j.name, arch)] cells: dict[tuple[str, str], list[WorkflowJobRecord]] = {} @@ -512,15 +541,22 @@ def _derive_variants( ) -> list[Variant] | None: """Matrix-cell variants for fan-out pipelines (pytorch/jax py x ref). - See `_variants_from_jobs` for what `arch` scopes. Workflows in + See `_variants_from_jobs` for what `arch` and `phase` scope. Workflows in `_SKIP_WORKFLOW_NAMES` never reach here: `update_status_json` returns before deriving a leaf for them at all. """ - axis_key = _VARIANT_AXIS_KEY.get(workflow_run.classification.pipeline_type) + cls = workflow_run.classification + axis_key = _VARIANT_AXIS_KEY.get(cls.pipeline_type) if axis_key is None: return None - variants = _variants_from_jobs(workflow_run, axis_key, arch=arch) + variants = _variants_from_jobs( + workflow_run, axis_key, arch=arch, phase=cls.pipeline_phase + ) if not variants: + # Phase-blind fallback for single-cell test runs lacking the py|ref + # axis. Safe for the build partition: it fires only before any build + # job exists, when no test job exists yet (tests need builds), so the + # whole-run status it reads cannot be test-influenced. variants = _variants_from_inputs(workflow_run, axis_key) return variants or None @@ -535,7 +571,7 @@ def _create_leaf( `arch` is only ever set when this run reports *multiple* architectures (see `_merge_run_into_document`), so `workflow_run`'s own conclusion is a whole-run aggregate across all of them, not this one arch's outcome. - Unlike `_refresh_same_run_fanout_tests`'s single-arch case, it must not be + Unlike `_refresh_same_run_tests_from_build`'s single-arch case, it must not be folded into the rollup as a vote here: doing so would broadcast one shared status onto every architecture -- exactly the leakage `arch` scoping exists to prevent. It is used only as the fallback when this @@ -564,7 +600,7 @@ def _create_leaf( ) -def _refresh_same_run_fanout_tests( +def _refresh_same_run_tests_from_build( doc: StatusDocument, workflow_run: WorkflowRunRecord, leaf: RunLeaf ) -> bool: """Refresh same-run test leaves from a completed fan-out workflow snapshot. @@ -573,16 +609,15 @@ def _refresh_same_run_fanout_tests( is invoked as a reusable `workflow_call` nested inside the delegated release workflow -- not dispatched as its own top-level run -- so its jobs land in the *same* run id, job list, and webhook notifications as the entry build. - There is no job-name-level split between "build" and "test" jobs: the - registry classifies the whole run as `pipeline_type`/`pipeline_phase="build"` - (see `WORKFLOW_SPECS`), and `_variants_from_jobs` already groups every job - sharing a (py, ref) cell -- build and nested test alike -- into one - `Variant` per cell (see `test_reusable_matrix_nested_jobs_collapse_to_one_variant_per_cell`). - Early notifications can project that job-list snapshot into per-arch test - leaves (keyed by the same run id) while some cells are still in progress; - the final notification is still classified as the build phase, so without - this function those same-run test leaves would go stale once the build - itself is done. + The registry classifies the whole run as + `pipeline_type`/`pipeline_phase="build"` (see `WORKFLOW_SPECS`), but its job + list carries both the build sub-jobs and the nested test sub-jobs; + `_variants_from_jobs(..., phase="test")` keeps only the latter (jobs with a + "Test | " segment of their own), so this projects the run's test half + into per-arch test leaves keyed by the same run id -- while some cells are + still in progress. The final notification is still classified as the build + phase, so without this function those same-run test leaves would go stale + once the build itself is done. Each matching test leaf is refreshed under three constraints, all guarding against this coarse build-run snapshot corrupting finer-grained state: @@ -632,7 +667,9 @@ def _refresh_same_run_fanout_tests( continue if (existing.run_attempt or 0) != (leaf.run_attempt or 0): continue - arch_variants = _variants_from_jobs(workflow_run, axis_key, arch=arch) + arch_variants = _variants_from_jobs( + workflow_run, axis_key, arch=arch, phase="test" + ) if not arch_variants: continue statuses = [v.status for v in arch_variants] @@ -659,6 +696,66 @@ def _refresh_same_run_fanout_tests( return wrote +def _refresh_same_run_build_from_test( + doc: StatusDocument, workflow_run: WorkflowRunRecord +) -> bool: + """Finalize a same-run build leaf from a nested test-phase snapshot (mirror + of `_refresh_same_run_tests_from_build`). + + In the shared-run topology a pytorch/jax build workflow calls its test + workflow via `workflow_call`, so the reusable test's own notify_quartz + (reclassified to `pipeline_phase="test"` via `reporting_workflow`) carries a + job list spanning the *whole* parent run -- the finished build sub-jobs + included. The build-phase notify that finalizes the build leaf may not fire + until run completion, leaving that leaf stuck `in_progress` while a test + notify arrives mid-run with the build sub-jobs already terminal. This + projects the build half of that snapshot onto the same-run build leaf so it + finalizes early instead of waiting. + + Guarded to the shared-run case only: the build leaf itself carries no run id + (its cells aggregate across per-cell runs, so `_merge_variant_leaf` drops + it), but each of its variants does. At least one existing build cell must + carry *this* run id, proving the test notify shares the run that produced + those build cells. A standalone test dispatch + (`test_pytorch_wheels_full.yml`) has its own run id, so no build cell + matches and the real build run's leaf is left untouched. Build status is + rolled up from the build sub-jobs alone (`phase="build"`), so a failed test + cell can never flip the build leaf. + """ + cls = workflow_run.classification + axis_key = _VARIANT_AXIS_KEY.get(cls.pipeline_type) + if cls.pipeline_phase not in ("test", "test-full") or axis_key is None: + return False + + pipeline = getattr(doc.pipelines, cls.pipeline_type) + existing = pipeline.build.get(cls.platform) + run_id = workflow_run.workflow_run_id + if existing is None or not existing.variants: + return False + if not any(v.run_id == run_id for v in existing.variants): + return False + + build_variants = _variants_from_jobs(workflow_run, axis_key, phase="build") + if not build_variants: + return False + + status = Variant.rollup_status(build_variants, Status.in_progress) + starts = [v.started_at for v in build_variants if v.started_at] + ends = [v.completed_at for v in build_variants if v.completed_at] + candidate = RunLeaf( + run_id=workflow_run.workflow_run_id, + run_attempt=workflow_run.run_attempt, + status=status, + started_at=min(starts) if starts else existing.started_at, + completed_at=(max(ends) if status.is_terminal and ends else None), + variants=build_variants, + ) + if not existing.should_replace(candidate): + return False + pipeline.build[cls.platform] = merge_matrix_test_leaf(existing, candidate) + return True + + def _rocm_build_run_id(doc: StatusDocument, platform: str) -> int | None: """Run id of the winning `rocm.build` leaf for `platform` (the run that owns the platform's artifact URL block), or None if no build has landed yet.""" @@ -937,7 +1034,8 @@ def _merge_run_into_document( _update_platform_urls( doc, workflow_run, leaf_accepted=leaf_accepted, prev_owner=prev_url_owner ) - refreshed_tests = _refresh_same_run_fanout_tests(doc, workflow_run, leaf) + refreshed_tests = _refresh_same_run_tests_from_build(doc, workflow_run, leaf) + refreshed_build = _refresh_same_run_build_from_test(doc, workflow_run) freeze_requested_architectures( doc, @@ -952,6 +1050,12 @@ def _merge_run_into_document( cls.pipeline_type, workflow_run.workflow_run_id, ) + if refreshed_build: + log.info( + "finalized same-run %s build leaf from nested test snapshot run_id=%s", + cls.pipeline_type, + workflow_run.workflow_run_id, + ) rebuild_summary(doc)