Fix two stale ROCm assertions; skip Apple fixtures the build cannot run - #620
Merged
Merged
Conversation
Follow-up to #619, which left 59 failures in a full sweep on the Strix Halo box. None were ROCm lane defects; all were test-side. This closes 42 of them and narrows the rest to one file. ROCm — two stale assertions (the 2 non-Apple failures) `test_autodiff_rocm_matmul_composed` compared `last_backward_execution` with `==` over the whole dict. E2E-REAL-6 added six tracer-authority fields (`family`, `frontend_authority`, four `*_consumer`) and changed `implementation` to `family_plugin_composition`, so the test failed on additive metadata while the numerics passed. Seven sibling tests already assert the new shape; this was the only one left on the old one. Now asserts the fields the proof depends on by key — additive-tolerant, and still fail-closed because a dropped key raises KeyError. `test_rocm_activation_rope_codegen` grepped for `kind must be gelu, silu, or relu`, a pass-body string. Rejection has since moved to the ODS attribute constraint (Decision #21a, EnumAttr over unvalidated StrAttr), so the op is refused before the generator — correct, but the new diagnostic named only "ROCm activation kind" and dropped the legal set, which Decision #21 wants a diagnostic to carry. Rather than weaken the test to match a worse message, `ROCM_ActivationKindAttr` now names its values, matching `ROCM_ReductionAttr` two declarations above it, and the test asserts the op name and all three legal kinds: error: 'tessera_rocm.activation' op attribute 'kind' failed to satisfy constraint: ROCm activation kind: 'gelu', 'silu', or 'relu' Apple — 57 fixtures failed where they should have skipped `tessera-opt` registers the Apple pipelines only under -DTESSERA_BUILD_APPLE_BACKEND=ON. The ROCm/x86 boxes build it OFF, so these fixtures died on `Unknown command line argument '-tessera-lower-to-apple_cpu-full'` (or the `--pass-pipeline` parser's equivalent). The existing guard asks whether the binary exists; it never asked whether that binary has the Apple backend in it. A capability the build lacks must skip, not fail — 57 red that say nothing about Apple crowd out real signal. `skip_if_apple_pipeline_unregistered()` is called at the point of invocation and is narrow in three ways so it cannot hide a defect: * fires only on the two "pipeline is not registered" signatures; * only when the Apple backend is genuinely absent — if it IS built, an unregistered pipeline is a registration regression and stays loud; * ignores any other nonzero exit, so the negative fixtures still assert their own diagnostics. A file-level skipif was tried first and rejected: it also disabled 54 fixtures in those files that pass fine without the Apple backend. Verified on the Strix Halo box (gfx1151), which has no Apple backend: - probe both directions: Apple pipeline absent -> False, `tessera-lower-to-rocm` -> True (so False is real signal, not a stuck probe), and CMakeCache agrees independently of --help - **guard is a no-op when the backend is present**: with the probe forced True to simulate the Mac, the fixtures FAIL rather than skip — the property that keeps this from silently disabling the Apple suite - full sweep: 59 failed -> 17 failed / 16344 passed - ROCm unaffected: `-k "rocm or gfx"` still 2828 passed, 0 crashes; `check-tessera-rocm` 63/63; ruff clean, mypy 0 Not fixed here, and why: the remaining 17 (all `test_apple_value_target_ir.py`) fail through the Python front door (`KeyError: 'compiler_path'`, `target_ir_artifact` vs `value_target_ir`) and a `--help` introspection check, not through a pipeline invocation. Guarding those needs an Apple build to confirm they still run, and this host cannot build the Apple backend — a guard written blind there could mask a real Apple regression. They belong on the Mac. Five fixtures that previously "passed" here now skip. They asserted a nonzero exit and were getting one from the missing CLI flag rather than from the rejection under test — false passes, not lost coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c3beecdcb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sessment
P2 — probe the executable that produced the failure.
`tessera_opt_registers()` resolved `tessera-opt` itself via
`compiler_tool.tessera_opt_path()`, a THIRD resolution scheme: the five fixtures
disagree on how they find the binary (`TESSERA_OPT` vs `TESSERA_OPT_PATH`, and
different default build dirs). On a host with several build trees the guard
could therefore inspect a different binary than the one that failed — and the
dangerous direction is not the missed skip but the inverted one: skipping a
genuine registration regression in an Apple-enabled binary because some other
preferred build lacks the backend.
`tessera_opt_registers(flag, opt_path)`, `apple_backend_in_tessera_opt(opt_path)`,
`apple_backend_configured_in_build(opt_path)` and
`skip_if_apple_pipeline_unregistered(proc, opt_path)` now all take the invoked
path, cache by it, and every call site passes its own `_OPT`. The path is
required, not defaulted, so a caller cannot silently get the wrong binary.
Verified the probe follows the path: same flag, different answer per binary --
`tessera-lower-to-x86` is True for `build/.../tessera-opt` and False for
`tessera-rocm-opt`. Guard behaviour unchanged (17 failed / 49 passed / 68
skipped, as before).
P1 — record the required cross-backend test assessment (AGENTS.md:81-85).
This is shared test infrastructure and the original commit updated none of the
four backend plans. Added owning item `CI-BACKEND-CAPABILITY-SKIP-2026-08-23`
to the Apple plan and a sibling-outcome entry to the other three. The outcomes
are measured, not asserted:
* ROCm — parity validated / no change. It already has both halves:
`lit.cfg.py:126` derives `tessera-rocm-backend` from the same
`tessera-opt --help` probe, and the ROCm pytest fixtures gate on
`_tessera_opt_path()` / `_rocm_wmma_runtime_available()`.
* NVIDIA — not applicable / no exposure. Measured on a host with
`TESSERA_BUILD_NVIDIA_BACKEND:BOOL=OFF`: `pytest -k nvidia -m "not slow"`
gives 496 passed, 14 skipped, 0 failed, so those fixtures already skip.
* x86 — not applicable by design. The x86 executable pass is ALWAYS
registered so it fails closed with a rebuild diagnostic, which is why
`lit.cfg.py:137` probes the dialect directly with a `!tessera_x86.tile`
fixture instead of `--help`; that also guards the assertions-enabled
regression behind Decision #19, so it is kept deliberately.
The Apple entry also records what this does NOT close (the remaining 17
front-door failures) and that it requires a Mac run, since exact-device Apple
evidence is unchanged and none is claimed.
Gates: generated-doc drift 26/26 in sync; claim_lint/audit/governance 323
passed; ruff + mypy clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gstoner
pushed a commit
that referenced
this pull request
Aug 23, 2026
#620 and this PR each added an entry immediately after the H1 of all four backend plans, so they collided in every one. Both entries are kept: they are distinct work items and neither supersedes the other -- `CI-BACKEND-CAPABILITY-SKIP-2026-08-23` (Apple-owned pytest capability gate, from #620) and `ROCM-CI-HSACO-SERIALIZE-2026-08-23` (this PR's host-free ROCm CI lane). Text-only conflict; no logic was involved on either side. Resolved by merge rather than rebase deliberately, so the two already-pushed commits are not rewritten under a reviewer who has read them. Verified after resolution: both item IDs present in all four plans with no markers left; generated-doc drift 26/26 in sync; ci-workflow contract + serialization proof 20 passed; ruff + mypy clean. Full sweep on the merged content: 16347 passed / 17 failed -- the 17 are the Apple front-door failures #620 explicitly deferred to a Mac run, unchanged and unrelated to this lane. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Follow-up to #619, which fixed the ROCm
lld invocation failedroot cause but left 59 failures in a full sweep on the Strix Halo box. I checked all 59 rather than assuming they shared a cause: none are ROCm lane defects — all are test-side. This closes 42 and narrows the rest to one file.ROCm — two stale assertions
test_autodiff_rocm_matmul_composedcomparedlast_backward_executionwith==over the whole dict. E2E-REAL-6 added six tracer-authority fields (family,frontend_authority, four*_consumer) and changedimplementationtofamily_plugin_composition, so it failed on additive metadata while the numerics passed. Seven sibling tests already assert the new shape — this was the only one left on the old one. Now asserts the fields the proof depends on by key: additive-tolerant, still fail-closed (a dropped key raisesKeyError).test_rocm_activation_rope_codegengrepped forkind must be gelu, silu, or relu, a pass-body string. Rejection has since moved to the ODS attribute constraint — Decision #21a, EnumAttr over unvalidated StrAttr — so the op is refused before the generator ever runs. Correct, but the new diagnostic named only"ROCm activation kind"and dropped the legal set, which Decision #21 wants a diagnostic to carry.Rather than weaken the test to match a worse message,
ROCM_ActivationKindAttrnow names its values — matchingROCM_ReductionAttrtwo declarations above it in the same file:Apple — 57 fixtures failed where they should have skipped
tessera-optregisters the Apple pipelines only under-DTESSERA_BUILD_APPLE_BACKEND=ON. The ROCm/x86 boxes build it OFF, so these fixtures died onUnknown command line argument '-tessera-lower-to-apple_cpu-full'(or the--pass-pipelineparser's equivalent).The existing guard asks whether the binary exists; it never asked whether that binary has the Apple backend in it. A capability the build lacks must skip, not fail — 57 red that say nothing about Apple crowd out real signal, which is exactly how a total ROCm serializer outage went unnoticed until #619.
skip_if_apple_pipeline_unregistered()is called at the point of invocation and is narrow in three ways so it cannot hide a defect:A file-level
skipifwas tried first and rejected: it also disabled 54 fixtures in those files that pass fine without the Apple backend.Evidence
Strix Halo (gfx1151, Ubuntu 24.04/WSL2) — a host with no Apple backend.
-k "rocm or gfx" -m "not slow"ninja -C build check-tessera-rocmpython/tessera/+ changed test files) / mypyProbe validated both directions, so a
Falseis real signal and not a stuck probe:Falsetessera-lower-to-rocm(control)TrueTESSERA_BUILD_APPLE_BACKEND(independent of--help)OFF— agreesThe property that matters: with the probe forced
Trueto simulate the Mac, the fixtures FAIL rather than skip. That is what keeps this from silently disabling the Apple suite on the box that can actually run it.Not fixed here, deliberately
The remaining 17 (all
test_apple_value_target_ir.py) fail through the Python front door (KeyError: 'compiler_path',target_ir_artifactvsvalue_target_ir) and a--helpintrospection check — not through a pipeline invocation. Guarding those needs an Apple build to confirm they still run, and this host cannot build the Apple backend; a guard written blind there could mask a real Apple regression. They belong on the Mac.Five fixtures that previously "passed" here now skip. They asserted a nonzero exit and were getting one from the missing CLI flag rather than from the rejection under test — false passes, not lost coverage.
Claim integrity
All ROCm/gfx1151 evidence was produced on the box with that device. No Apple claim is made: this host cannot build or run the Apple backend, so the Apple change is verified only as "these fixtures skip here, and provably do not skip when the backend is present." Whether they still pass on the Mac is unverified here and needs a Mac run before that is asserted.
🤖 Generated with Claude Code