CI: trim per-PR simulator scene coverage - #1823
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (22)
📝 WalkthroughWalkthroughSelected simulator tests now use manual-execution markers for ChangesSimulator execution policy
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This change only adjusts simulator test selection while retaining representative Per-PR coverage and daily execution for migrated cases; no actionable merge-blocking risk remains after normal checks and review. Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
0cf9476 to
b329975
Compare
doraemonmj
changed the title
Review — CI: trim per-PR simulator scene coverageReviewed Real goal vs stated goalThey match. This is pure test-selection metadata: 59 test files gain a Churn: 61 files, +124/−13 = 137 lines. 59 of them are a one-line annotation; the other two are the Sim workflows. No Representative-coverage claims: spot-checked, they hold
Every added marker lists only Must fix1.
|
pre 31687370652 |
final 31993266388 |
|
|---|---|---|
Run pytest scene tests, 4 lanes |
1329 s (22m09s) | 744 s (12m24s) |
| dedicated DFX steps, 4 lanes | 132 s | 284 s |
| pytest work, total | 1461 s | 1028 s |
| whole Sim jobs, 4 lanes | 2228 s (37m08s) | 1614 s (26m54s) |
Net pytest-work reduction is 433 s (7m13s, ~30 %), not 9m45s / 44 %. The DFX steps grew 2–5× per lane (a2a3 dep_gen 4 s→10 s, a2a3 HBG dep_gen 3 s→17 s, a5 PMU 2 s→9 s) — consistent with kernel compilation for those cases migrating out of the xdist-parallel main step into the serial -p no:xdist tail. The table does disclose the exclusion, but the headline number is computed as if the excluded steps had not grown.
The whole-job aggregate still shows a real win (−10m14s), so the conclusion survives — only the framing needs correcting. Worth noting the pre-change a2a3-ubuntu job (698 s) is an outlier: ~220 s more non-pytest time than its macOS sibling, so single-run deltas are noisy.
5. docs/ci.md:110-111 is now inaccurate
It states: "Per-PR scene-test jobs keep the default --manual exclude, so moving a case to Daily does not require a second workflow exclusion list." After this PR the per-PR DFX steps pin --manual include, so marking a case that lives under a DFX-smoke path manual does not move it to Daily — it keeps running in per-PR. That is exactly the invariant the next contributor will rely on. Please update the paragraph in this commit (per .claude/rules/doc-consistency.md §1, §4).
Consider
6. manual: True instead of a platform list
tests/st/host_build_graph_wide_dispatch/test_host_build_graph_wide_dispatch.py:85. Equivalent today (the case is sim-only), but it is the one marker the "all newly added shared-case markers are platform-scoped" guardrail does not literally cover, and it would silently exclude onboard if a2a3/a5 were ever added to platforms. ["a2a3sim", "a5sim"] costs nothing.
7. _PER_PR_SIM_CASES is unvalidated
tests/st/runtime_fatal_codes/test_runtime_fatal_codes.py:221. A typo in either name silently marks all 11 cases manual and zeroes per-PR Sim fatal-code coverage — a green-CI failure shape (cf. .claude/rules/ci-change-detection.md §7). One assert _PER_PR_SIM_CASES <= CASES.keys() closes it.
8. The residual risk is asymmetric between arches, and the guardrail table hides it
For the shared cases that declare a5sim but no a5 — runtime_fatal_codes (9 of 11), host_build_graph_validation (3 of 4), collectives all_to_all / allgather / broadcast / reduce_scatter / group_reservation — there is no onboard-a5 fallback, so on a5 they become Daily-only. On a2a3 the per-PR onboard lane still covers them. The "Onboard Per-PR" column is unchanged precisely because those cases were never in a5 onboard, so it reads as a clean bill of health when it isn't. One sentence in the description would fix that. (TestConsecutiveGroupReservation is the only class in its file, so group reservation drops to a2a3-onboard-only in per-PR.)
9. The DFX steps hardcode --manual include while their siblings honor $MANUAL_MODE
Every other pytest invocation in these reusable workflows uses --manual "$MANUAL_MODE". A caller passing the valid input manual_mode: only now gets the main step running only manual cases while the DFX steps run everything. Either derive the value or add a comment on why these are pinned.
10. Latent local footgun (pre-existing pattern, newly reachable)
The DFX test_run overrides post-validate every platform-matching case — for case in self.CASES: if st_platform in case["platforms"]: self._post_validate(...) (test_dep_gen_chain.py:142-144, test_chip_swimlane.py:109, test_pmu.py:95, and the sibling files) — replicating the platform filter but not --case or --manual. With three of four chain cases now manual, pytest .../dfx/dep_gen/ --platform a2a3sim --enable-dep-gen (default --manual exclude) asserts on output dirs for cases that never ran and fails with a misleading no output dir for case 'n_64_no_chain' created this run.
No CI lane trips it: the DFX steps pass --manual include, and no workflow passes --case (grep over .github/). Fixing #1 removes the reachable path; the --case half is pre-existing and out of scope here.
Verdict
Request changes, for #1 (three dep-chain cases silently leave per-PR Sim, contradicting the stated design) and #2 (hardcoded parametrize list can silently drop future cases). Both are small edits. Everything else is description accuracy (#4, #5, #8) or hardening.
The approach itself is sound: it reuses an existing, documented mechanism rather than inventing a second exclusion list, keeps the full corpus in Daily, correctly scopes every marker to simulator platforms, and the wall-clock win is real even after correcting the arithmetic.
Move simulator-only lifecycle, error, stress, scale, and redundant coverage to the manual set retained by daily CI. Keep representative paths and dedicated DFX smoke validation in per-PR CI while leaving onboard selection unchanged. Run dep-gen smoke targets over their complete directories and make DFX post-validation reuse the same platform, case, and manual filters as scene execution. Preserve the full HBG invalid-input matrix, validate retained fatal-code case names, and document the dedicated-DFX and platform-fallback semantics.
b329975 to
1b2ef52
Compare
|
@ChaoWao Addressed all ten points from the review:
The final-head GitHub Actions run is green, including all four Sim lanes and all Onboard lanes. The PR description has also been updated with the final selection counts, corrected timing, and validation results. |
Re-review — force-push
|
| Previous finding | Resolution — verified |
|---|---|
| Must #1 — three dep-chain cases left per-PR Sim entirely because the step targeted a file | Both TMR dep_gen steps now target .../dfx/dep_gen/, so test_dep_gen_chain.py is collected and its three overflow cases are back in per-PR Sim. |
Must #2 — host_build_graph_validation hardcoded parametrize list |
Reverted entirely; the file is no longer in the diff, so the full four-case invalid-input matrix stays in per-PR on both simulator platforms. |
| Should #3 — total loss of three validation cases with no onboard fallback | Moot, same revert. |
| Should #4 — timing table excluded the steps that absorbed the work | Table now separates main / DFX / total / whole-job. I recomputed all eight cells from the Actions API and every one matches exactly: main 1329 s→841 s (22m09s→14m01s), DFX 132 s→308 s (2m12s→5m08s), pytest work 1461 s→1149 s (24m21s→19m09s, −21 %), whole jobs 2228 s→1721 s (37m08s→28m41s, −23 %). The per-lane main steps (205/252/183/201 s) also match the quoted 3m25s / 4m12s / 3m03s / 3m21s. |
Should #5 — docs/ci.md:110-111 became inaccurate |
Rewritten, and it now states the exact rule the code implements including the only passthrough. docs/testing.md and the testing-skill table updated to match. |
Consider #6 — manual: True instead of a platform list |
Now ["a2a3sim", "a5sim"] in host_build_graph_wide_dispatch. Across the whole diff all 74 added markers are sim-scoped; no bare True remains. |
Consider #7 — _PER_PR_SIM_CASES unvalidated |
assert _PER_PR_SIM_CASES <= CASES.keys() at module scope, with the offending names in the message. |
| Consider #8 — asymmetric A5 residual risk hidden by the guardrail table | Called out in the body and, better, encoded in both docs: onboard coverage survives only when the case declares that onboard platform. |
Consider #9 — DFX steps pinned --manual include while their siblings honour $MANUAL_MODE |
Now `--manual "${{ inputs.manual_mode == 'only' && 'only' |
Consider #10 — post-validate loop ignored --case / --manual |
Fixed properly, and generalised past this PR's own scope: the filter is extracted as SceneTestCase._matching_cases() and all ten validators route through it, including test_scope_stats.py, which this PR does not otherwise touch. Grepping the tree, the only remaining for case in ...CASES loops are the two authoritative ones inside scene_test.py. |
Verified independently as well: the manual-selection unit tests pass locally (32 passed), and all four chip_swimlane/*.py files sit inside the directory the dedicated step targets on both architectures — so test_sync_start_drain_phases and test_sync_start_early_local_owner keep full per-PR coverage and merely moved out of the parallel sweep. That retires the early-dispatch concern from the first pass entirely.
The net saving fell from the ~7m13s the previous head would have delivered to 5m12s, because coverage was restored in three places. That is the right direction for the trade.
On the refactor
test_run now computes matched first and skips before build_callable, then hands the same list to run_class_cases. Behaviour-preserving for the selection itself, with two side effects that are both improvements: a class filtered out by --case no longer pays a kernel compile, and because pytest.skip() raises, a subclass validator placed after super().test_run(...) can no longer run against a class that never executed. _matching_cases also tightened case.get("platforms", []) to case["platforms"], matching what test_run always did.
Remaining — all Consider, nothing blocking
-
The A2/A3
host_build_graphdep_gen step still namestest_dep_gen.pyrather than its directory. No gap today (that directory holds exactly one file), but a filename target silently missing siblings is the precise defect this revision fixed for TMR, and the summary says the dep-gen targets "now cover the complete directories" while one still does not. One-word change. -
test_dep_gen_chain.pyis now the only half-marked file.n_65_single_overflowstays non-manual, so it runs twice per PR: plain in the main sweep, and again with--enable-dep-genin the DFX step. If the dep-gen-off run is the point — a different orchestration build, and the golden still checks the barrier — a comment saying so would settle it; otherwise mark it and make the file uniform. -
TestChipSwimlaneMixed._validate_dump_func_idsstill hardcodesTestChipSwimlaneMixed_default. It is the one validator in these files that does not derive its label from_matching_cases. Safe today (single-case class, and the class only runs when that case matched), but a second case would make it silently validate the wrong output directory. -
The skip-before-
build_callableordering is load-bearing and unrecorded. One present-tense line at the skip — no case has executed yet, so a subclass validator aftersuper().test_run()cannot see cases that did not run — would stop a future edit from quietly moving it back down.
ℹ️ pto-isa pin: pinned to
f51c92f610827daad0ddfb383072e03d514b4ae9(moved with the rebase). No pto-isa header references changed — advisory only.
Verdict
Approve. Both must-fixes and all three should-fixes from the previous pass are resolved, the fix for the post-validate mismatch went further than asked and removed the pattern repo-wide, the timing numbers are now exactly reproducible from the Actions API, and the docs state the real rule rather than the old one. The four remaining items are cosmetic or future-proofing.
…view `network1` / `network2` / `network3`, but only the span vocabulary moved. The rest of the tree still called L4 a pod, so one level had two names. This converts the remainder and fixes the three findings from hw-native-sys#1877's review. `pod` meant three different things, so each occurrence was judged rather than swept: * `POD` — Plain Old Data, the C++ term codestyle.md rule 8 is built on. 114 occurrences, untouched. * `SceneTestLevel.POD = 4` — the L4 scene level, now `NETWORK1`, together with its `@scene_level()` call sites. * lowercase `pod` — the level word, in identifiers, paths, CI job names and prose. Converted. Renamed with history: the three `.github/actions/pod-*` composite actions, `_st-pod.yml`, and `tests/st/.../l4_pod/` with its test file. Job names follow (`st-pod-onboard-a2a3` -> `st-network1-onboard-a2a3`), which is safe because the active ruleset enforces deletion, non-fast-forward, pull_request and linear history and carries no required status checks — so no PR waits on a check that stops reporting. Fixture names move together with every signature that requests them (`st_pod_peer` -> `st_network1_peer` and siblings); a missed one is a fixture-not-found at collection, so all five L4 tests were collected to check. `POD_*` are 28 variables read from a `.env` on each runner machine, and `_st-network1.yml` accepts only keys matching a prefix. Renaming repo-side while those files still say `POD_` would make the filter drop every key — silently, because it `continue`s. Those files are not in this repository. So the reader now accepts both spellings and exports every key as `NETWORK1_*`. The rest of the job and the tests see exactly one name, and a machine can be converted whenever, in either order. `POD_ENV_FILE` still works for the same reason. Drop the `POD_` branch once no `.env` uses it. **`set_level_prefix` could dangle a live name pointer.** `SpanScope` keeps the `const char *` it was handed and dereferences it in its destructor, and rebinding reassigned the strings it points into. A process constructing Workers at two levels also relabelled the first Worker's spans mid-run. The first non-empty word now wins and later ones are refused; `level_prefix()` reports what is actually bound, and Python compares the two and warns, because one process has one vocabulary and that is worth saying rather than leaving in a trace. **docs/dfx/host-trace.md documented only `host.*`** and still claimed names do not encode the emitting level — pointing at hw-native-sys#1793 for the fix that hw-native-sys#1877 was. The table now uses `<level>.` and names the four possible words. **test_strace_timing.py lost its retired-name coverage.** The fixture was called `old` because it stood for an older log; hw-native-sys#1877's rename converted its contents to the current names, leaving `_RETIRED_WORDS` — added by that same PR to read archived logs — with no test at all. Restored as its own case, plus one covering `span_family` across every level word, `ext.`, and an unknown leader. - `pytest tests/ut/py -m "not requires_hardware"` — 1520 passed, 0 failed. - `ctest -LE requires_hardware --timeout 300` — 101/101. - All 51 local `uses:` references in `.github/` resolve, checked against the working tree; a stale one is this change's only way to redden CI. - All five L4 tests collect, which is what proves the fixture renames. - End to end on a2a3sim: a real trace still splits into 13 chip and 9 host spans. - `pre-commit` clean except clang-tidy, whose hook venv cannot import simpler and fails identically on untouched files. The longer variable names pushed one `pytest.fail` message in conftest.py past the 120-column limit; the message is now built from a local rather than nested inside the f-string. `docs/hierarchical-level-runtime.md` is the one place that maps a level word onto a physical entity, so it is where `network1` is explained as the layer commonly deployed as a pod. Nothing else in the tree repeats the mapping. Rebasing onto hw-native-sys#1823 conflicted in three docs: it rewrote the `manual` marker rows that this change had only reworded, in `.claude/skills/testing/SKILL.md`, `docs/testing.md` and `docs/ci.md`. Both sides are kept — the expanded DFX-step wording with the level word applied. That PR also added prose calling the level POD, which this converts too.
…view `network1` / `network2` / `network3`, but only the span vocabulary moved. The rest of the tree still called L4 a pod, so one level had two names. This converts the remainder and fixes the three findings from hw-native-sys#1877's review. `pod` meant three different things, so each occurrence was judged rather than swept: * `POD` — Plain Old Data, the C++ term codestyle.md rule 8 is built on. 114 occurrences, untouched. * `SceneTestLevel.POD = 4` — the L4 scene level, now `NETWORK1`, together with its `@scene_level()` call sites. * lowercase `pod` — the level word, in identifiers, paths, CI job names and prose. Converted. Renamed with history: the three `.github/actions/pod-*` composite actions, `_st-pod.yml`, and `tests/st/.../l4_pod/` with its test file. Job names follow (`st-pod-onboard-a2a3` -> `st-network1-onboard-a2a3`), which is safe because the active ruleset enforces deletion, non-fast-forward, pull_request and linear history and carries no required status checks — so no PR waits on a check that stops reporting. Fixture names move together with every signature that requests them (`st_pod_peer` -> `st_network1_peer` and siblings); a missed one is a fixture-not-found at collection, so all five L4 tests were collected to check. `POD_*` are 28 variables read from a `.env` on each runner machine, and `_st-network1.yml` accepts only keys matching a prefix. Renaming repo-side while those files still say `POD_` would make the filter drop every key — silently, because it `continue`s. Those files are not in this repository. So the reader now accepts both spellings and exports every key as `NETWORK1_*`. The rest of the job and the tests see exactly one name, and a machine can be converted whenever, in either order. `POD_ENV_FILE` still works for the same reason. Drop the `POD_` branch once no `.env` uses it. **`set_level_prefix` could dangle a live name pointer.** `SpanScope` keeps the `const char *` it was handed and dereferences it in its destructor, and rebinding reassigned the strings it points into. A process constructing Workers at two levels also relabelled the first Worker's spans mid-run. The first non-empty word now wins and later ones are refused; `level_prefix()` reports what is actually bound, and Python compares the two and warns, because one process has one vocabulary and that is worth saying rather than leaving in a trace. **docs/dfx/host-trace.md documented only `host.*`** and still claimed names do not encode the emitting level — pointing at hw-native-sys#1793 for the fix that hw-native-sys#1877 was. The table now uses `<level>.` and names the four possible words. **test_strace_timing.py lost its retired-name coverage.** The fixture was called `old` because it stood for an older log; hw-native-sys#1877's rename converted its contents to the current names, leaving `_RETIRED_WORDS` — added by that same PR to read archived logs — with no test at all. Restored as its own case, plus one covering `span_family` across every level word, `ext.`, and an unknown leader. - `pytest tests/ut/py -m "not requires_hardware"` — 1520 passed, 0 failed. - `ctest -LE requires_hardware --timeout 300` — 101/101. - All 51 local `uses:` references in `.github/` resolve, checked against the working tree; a stale one is this change's only way to redden CI. - All five L4 tests collect, which is what proves the fixture renames. - End to end on a2a3sim: a real trace still splits into 13 chip and 9 host spans. - `pre-commit` clean except clang-tidy, whose hook venv cannot import simpler and fails identically on untouched files. The longer variable names pushed one `pytest.fail` message in conftest.py past the 120-column limit; the message is now built from a local rather than nested inside the f-string. `docs/hierarchical-level-runtime.md` is the one place that maps a level word onto a physical entity, so it is where `network1` is explained as the layer commonly deployed as a pod. Nothing else in the tree repeats the mapping. Three findings from the hw-native-sys#1880 review are folded in. A proxy URL may carry `user:password@`, and both the local reachability check and the peer-side one in network1-stage wrote the whole value to the job log; the parse that already stripped the credential is now a `proxy_hostport` helper the error message shares, so a failure still names the address without the secret. The a2a3 reproduction commands in three skills carried `-m "not sdma"` but not `--exclude-level 4`, which CI pairs it with, so a single-machine reproduction would collect the level-4 tests that need the two-machine job. One `POD` survived in an _st-network1.yml comment describing that job's test result. Rebasing onto hw-native-sys#1823 conflicted in three docs: it rewrote the `manual` marker rows that this change had only reworded, in `.claude/skills/testing/SKILL.md`, `docs/testing.md` and `docs/ci.md`. Both sides are kept — the expanded DFX-step wording with the level word applied. That PR also added prose calling the level POD, which this converts too.
…view `network1` / `network2` / `network3`, but only the span vocabulary moved. The rest of the tree still called L4 a pod, so one level had two names. This converts the remainder and fixes the three findings from hw-native-sys#1877's review. `pod` meant three different things, so each occurrence was judged rather than swept: * `POD` — Plain Old Data, the C++ term codestyle.md rule 8 is built on. 114 occurrences, untouched. * `SceneTestLevel.POD = 4` — the L4 scene level, now `NETWORK1`, together with its `@scene_level()` call sites. * lowercase `pod` — the level word, in identifiers, paths, CI job names and prose. Converted. Renamed with history: the three `.github/actions/pod-*` composite actions, `_st-pod.yml`, and `tests/st/.../l4_pod/` with its test file. Job names follow (`st-pod-onboard-a2a3` -> `st-network1-onboard-a2a3`), which is safe because the active ruleset enforces deletion, non-fast-forward, pull_request and linear history and carries no required status checks — so no PR waits on a check that stops reporting. Fixture names move together with every signature that requests them (`st_pod_peer` -> `st_network1_peer` and siblings), as does the type one of them returns (`PodPeer` -> `Network1Peer`); a missed fixture is a fixture-not-found at collection, so all five L4 tests were collected to check. `POD_*` are 28 variables read from a `.env` on each runner machine, and `_st-network1.yml` accepts only keys matching a prefix. Renaming repo-side while those files still say `POD_` would make the filter drop every key — silently, because it `continue`s. Those files are not in this repository. So the reader now accepts both spellings and exports every key as `NETWORK1_*`. The rest of the job and the tests see exactly one name, and a machine can be converted whenever, in either order. `POD_ENV_FILE` still works for the same reason. Drop the `POD_` branch once no `.env` uses it. The `a2a3pod` runner label is machine-side for the same reason and gets no such escape: a job matches every label in its list, so it cannot accept either spelling. It stays until those machines are relabelled, and the workflows and docs/ci.md say why it reads differently from everything around it. **`set_level_prefix` could dangle a live name pointer.** `SpanScope` keeps the `const char *` it was handed and dereferences it in its destructor, and rebinding reassigned the strings it points into. A process constructing Workers at two levels also relabelled the first Worker's spans mid-run. The first non-empty word now wins and later ones are refused; `level_prefix()` reports what is actually bound, and Python compares the two and warns, because one process has one vocabulary and that is worth saying rather than leaving in a trace. **docs/dfx/host-trace.md documented only `host.*`** and still claimed names do not encode the emitting level — pointing at hw-native-sys#1793 for the fix that hw-native-sys#1877 was. The table now uses `<level>.` and names the four possible words. **test_strace_timing.py lost its retired-name coverage.** The fixture was called `old` because it stood for an older log; hw-native-sys#1877's rename converted its contents to the current names, leaving `_RETIRED_WORDS` — added by that same PR to read archived logs — with no test at all. Restored as its own case, plus one covering `span_family` across every level word, `ext.`, and an unknown leader. - `pytest tests/ut/py -m "not requires_hardware"` — 1581 passed, 0 failed. - `ctest -LE requires_hardware --timeout 300` — 101/101. - All 55 local `uses:` references in `.github/` resolve, checked against the working tree; a stale one is this change's only way to redden CI. - All five L4 tests collect, which is what proves the fixture renames. - End to end on a2a3sim: a real trace still splits into 13 chip and 9 host spans. - `pre-commit` clean except clang-tidy, whose hook venv cannot import simpler and fails identically on untouched files. The longer variable names pushed one `pytest.fail` message in conftest.py past the 120-column limit; the message is now built from a local rather than nested inside the f-string. `docs/hierarchical-level-runtime.md` is the one place that maps a level word onto a physical entity, so it is where `network1` is explained as the layer commonly deployed as a pod. Nothing else in the tree repeats the mapping. Three findings from the hw-native-sys#1880 review are folded in. A proxy URL may carry `user:password@`, and both the local reachability check and the peer-side one in network1-stage wrote the whole value to the job log; the parse that already stripped the credential is now a `proxy_hostport` helper the error message shares, so a failure still names the address without the secret. The a2a3 reproduction commands in three skills carried `-m "not sdma"` but not `--exclude-level 4`, which CI pairs it with, so a single-machine reproduction would collect the level-4 tests that need the two-machine job. One `POD` survived in an _st-network1.yml comment describing that job's test result. `.claude/skills/testing/SKILL.md`, `docs/testing.md` and `docs/ci.md` carry both hw-native-sys#1823's expanded `manual`-marker and pre-commit-build wording and the level word, which this change also applies to the prose that PR added calling the level POD.
`network1` / `network2` / `network3`, but only the span vocabulary moved. The rest of the tree still called L4 a pod, so one level had two names. This converts the remainder and fixes the three findings from #1877's review. `pod` meant three different things, so each occurrence was judged rather than swept: * `POD` — Plain Old Data, the C++ term codestyle.md rule 8 is built on. 114 occurrences, untouched. * `SceneTestLevel.POD = 4` — the L4 scene level, now `NETWORK1`, together with its `@scene_level()` call sites. * lowercase `pod` — the level word, in identifiers, paths, CI job names and prose. Converted. Renamed with history: the three `.github/actions/pod-*` composite actions, `_st-pod.yml`, and `tests/st/.../l4_pod/` with its test file. Job names follow (`st-pod-onboard-a2a3` -> `st-network1-onboard-a2a3`), which is safe because the active ruleset enforces deletion, non-fast-forward, pull_request and linear history and carries no required status checks — so no PR waits on a check that stops reporting. Fixture names move together with every signature that requests them (`st_pod_peer` -> `st_network1_peer` and siblings), as does the type one of them returns (`PodPeer` -> `Network1Peer`); a missed fixture is a fixture-not-found at collection, so all five L4 tests were collected to check. `POD_*` are 28 variables read from a `.env` on each runner machine, and `_st-network1.yml` accepts only keys matching a prefix. Renaming repo-side while those files still say `POD_` would make the filter drop every key — silently, because it `continue`s. Those files are not in this repository. So the reader now accepts both spellings and exports every key as `NETWORK1_*`. The rest of the job and the tests see exactly one name, and a machine can be converted whenever, in either order. `POD_ENV_FILE` still works for the same reason. Drop the `POD_` branch once no `.env` uses it. The `a2a3pod` runner label is machine-side for the same reason and gets no such escape: a job matches every label in its list, so it cannot accept either spelling. It stays until those machines are relabelled, and the workflows and docs/ci.md say why it reads differently from everything around it. **`set_level_prefix` could dangle a live name pointer.** `SpanScope` keeps the `const char *` it was handed and dereferences it in its destructor, and rebinding reassigned the strings it points into. A process constructing Workers at two levels also relabelled the first Worker's spans mid-run. The first non-empty word now wins and later ones are refused; `level_prefix()` reports what is actually bound, and Python compares the two and warns, because one process has one vocabulary and that is worth saying rather than leaving in a trace. **docs/dfx/host-trace.md documented only `host.*`** and still claimed names do not encode the emitting level — pointing at #1793 for the fix that #1877 was. The table now uses `<level>.` and names the four possible words. **test_strace_timing.py lost its retired-name coverage.** The fixture was called `old` because it stood for an older log; #1877's rename converted its contents to the current names, leaving `_RETIRED_WORDS` — added by that same PR to read archived logs — with no test at all. Restored as its own case, plus one covering `span_family` across every level word, `ext.`, and an unknown leader. - `pytest tests/ut/py -m "not requires_hardware"` — 1581 passed, 0 failed. - `ctest -LE requires_hardware --timeout 300` — 101/101. - All 55 local `uses:` references in `.github/` resolve, checked against the working tree; a stale one is this change's only way to redden CI. - All five L4 tests collect, which is what proves the fixture renames. - End to end on a2a3sim: a real trace still splits into 13 chip and 9 host spans. - `pre-commit` clean except clang-tidy, whose hook venv cannot import simpler and fails identically on untouched files. The longer variable names pushed one `pytest.fail` message in conftest.py past the 120-column limit; the message is now built from a local rather than nested inside the f-string. `docs/hierarchical-level-runtime.md` is the one place that maps a level word onto a physical entity, so it is where `network1` is explained as the layer commonly deployed as a pod. Nothing else in the tree repeats the mapping. Three findings from the #1880 review are folded in. A proxy URL may carry `user:password@`, and both the local reachability check and the peer-side one in network1-stage wrote the whole value to the job log; the parse that already stripped the credential is now a `proxy_hostport` helper the error message shares, so a failure still names the address without the secret. The a2a3 reproduction commands in three skills carried `-m "not sdma"` but not `--exclude-level 4`, which CI pairs it with, so a single-machine reproduction would collect the level-4 tests that need the two-machine job. One `POD` survived in an _st-network1.yml comment describing that job's test result. `.claude/skills/testing/SKILL.md`, `docs/testing.md` and `docs/ci.md` carry both #1823's expanded `manual`-marker and pre-commit-build wording and the level word, which this change also applies to the prose that PR added calling the level POD.
2 participants
Summary
manualselection.manual_mode: include.--case, and--manualselection as scene execution.No kernels, test parameters, runtime configuration, golden checks, or timeout thresholds are changed.
What moves to Daily
Selection after this PR
Collection was run from final commit
1b2ef52cagainst the 275 discoveredexamples/tests/stitems:--manual exclude)--manual only)Shared cases keep Onboard coverage when they declare the corresponding Onboard platform. A small set of cases declares A5 Sim but not A5 Onboard (runtime-fatal variants and selected collective/group-reservation cases); those cases become A5 Daily-only and remain covered on A2/A3 Onboard where declared. The marker and testing documentation now call out this platform-fallback behavior explicitly.
Timing
Final-head run 32119518132 passed all four Sim lanes. The comparison is against pre-change run 31687370652 and separates the main scene step from the dedicated DFX tail so that the total comparison is apples-to-apples.
Run pytest scene testsstepsThe final main pytest lanes took 3m25s (A2/A3 Ubuntu), 4m12s (A2/A3 macOS), 3m03s (A5 Ubuntu), and 3m21s (A5 macOS), all below the 5-minute target. Complete jobs still take 6m24s–8m41s because setup and the serial dedicated DFX tail remain; further scene downselection alone is insufficient to make every whole job sub-five-minute.
Validation
--manual include; A2/A3excludeandonlyselection were also verified.git diff --check upstream/main...HEAD: passed.Final-head GitHub Actions CI passed, including pre-commit, packaging, profiling flags, unit tests, all Onboard lanes, and all four Sim lanes.