ci: execution routes, affected-package tests, and one guards job - #1841
Conversation
The fixture loader returned `None` for a missing package and only panicked with a hint when `OBC_REQUIRE_FIXTURES` was set, so every one of the sixteen call sites turned that `None` into an `.expect` message that named no setup command. `file()` now returns the path or panics with the exact `obc fixtures sync` command, `file_in` names a profile outside `test`, and the environment variable is gone from the loader, the justfile, the workflow, the registry command and the assistant-places verifier. `sim-peak-view` leaves the `test` sync profile: no test reads it, only the simulator does at runtime, and it stays in the `sim` profile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both nextest steps and the doctest step ran `--workspace --all-features` and then filtered execution, so a three-crate change still built every crate. The plan already names the selected suites; `tools/ci/rust_packages.py` turns it into `-p NAME` flags for one tier, prints `--workspace` when the change can alter what any package compiles to, and prints nothing when the tier selected no package. An empty set skips the tier; it never falls back to the workspace. `tools/ci/test.sh` carries the job's body, one section per step, so the same file serves CI and a local run. The registry reads a `tools/ci/*.sh` script as the job body it is, and a computed package list counts as the whole root workspace for job coverage, which is what it is on a foundation change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven guard jobs each paid for a checkout to run one fast Python script. They are now seven steps of one `guards` job, which also carries the registry drift check, its unit tests and the fixture policy. No guard's logic moved and each step keeps its own name, so a failure still reads the same. The UI snapshot sweep and the builder pytest leave the `test` job's serial path for `ui-snapshots` and `builder-python`, each gated on its own job name and each restoring the shared host-tests cache. The builder job now fails, rather than silently skipping its packer half, when the binary or corpus is missing. Routing expectations that changed, all from the split: - python.repository-tools now routes to `guards`, python.builder to `builder-python`. - `builder-python` compiles obc-pack and `ui-snapshots` compiles obc-sim, so both are routes for those packages, exactly as `clippy` and `fmt` are. A foundational Rust change therefore starts both jobs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`obc test -p PKG` ran Cargo's harness while CI ran nextest, so a developer and CI could disagree about a merged target. The focused arm now runs nextest and then the same scope's doctests. `obc test -p PKG --lib` is the library-only loop: one target, no doctests, and Cargo itself rejects a package with no library. `COPERNICUS_ATTRIBUTION` and `SOURCE_DATASET` move from the host DEM tool to the `obc-elevation` leaf every consumer already depends on. The application drops its `obc-dem` dev-dependency, which existed only so one unit test could read the wording. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Names the one explicit route for each of the three ignored tests and the five manual commands, and says there is no expensive tier. Records that bounded fixture suites are ordinary work that fails with the sync command, that the `test` job compiles the packages the plan selected, that the sweep is its own job with a wider job-level trigger than its suite, and that a focused `obc test -p` runs on nextest with a `--lib` form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (31)
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 |
CI evidence — run 35080781943, conclusion successThis run is the gate for the workflow shape. It selects Job list
|
| # | Step | Result | Seconds |
|---|---|---|---|
| 1 | Set up job | success | 3 |
| 2 | Run actions/checkout@v4 | success | 2 |
| 3 | Run actions-rust-lang/setup-rust-toolchain@v1 | success | 7 |
| 4 | Run Swatinem/rust-cache@v2 | success | 14 |
| 5 | Run ./.github/actions/setup-host-deps | success | 17 |
| 6 | Install native LLVM coverage tools | success | 2 |
| 7 | Run taiki-e/install-action@v2 | success | 2 |
| 8 | Cache content-addressed developer fixtures | success | 0 |
| 9 | cargo nextest (fast hermetic binaries) | success | 64 |
| 10 | cargo nextest (captured fixture binaries) | success | 7 |
| 11 | cargo test --doc (selected packages, all features) | success | 3 |
| 12 | cargo test (obc-formats, default features = the device's shape) | success | 1 |
| 13 | Run actions/setup-python@v5 | success | 0 |
| 14 | Install Python test reporter | success | 5 |
| 15 | firmware resource/dependency tool unit tests | success | 1 |
| 16 | firmware production dependency direction | success | 0 |
| 17 | obc-bench --check (render + read-counter golden gate) | success | 7 |
| 18 | firmware-tools coverage | success | 0 |
| 19 | Upload firmware-tools coverage | success | 1 |
| 20 | Upload firmware-tools Python test results | success | 1 |
| 21 | Rust component coverage | success | 7 |
| 22 | Publish Rust coverage summary | success | 0 |
| 23 | Upload Rust coverage and remaining native logs | success | 1 |
| 24 | Upload Rust test results | success | 1 |
| 25 | Upload Rust fixture test results | success | 1 |
Measured against the epic's baseline
The epic recorded the test job at 302 s and 304 s, with the fast tier at 97 s and 99 s and the
sweep at 66 s inside the job.
| Measure | Epic baseline | This run |
|---|---|---|
test job, whole |
302 s / 304 s | 153 s |
| fast nextest step | 97 s / 99 s | 64 s |
| captured-fixture nextest step | 14 s | 7 s |
sweep, on the test job's serial path |
66 s | 0 s (own 144 s job, in parallel) |
| builder pytest preparation, on that serial path | 33 s | 0 s (own 151 s job, in parallel) |
Both nextest steps still compiled the whole workspace on this run, so the fast tier's 64 s is a
cache-warmth difference, not the narrowing. What the job shape bought is the 99 s of sweep and
builder preparation that left the serial path. guards runs the seven guard scripts, the
registry drift check, the registry unit tests and the fixture policy in 42 s on one runner,
where seven jobs each paid for their own checkout.
|
@coderabbitai review |
|
The per-pull-request ratchet reads one LCOV report over the whole workspace and fails any critical file it never compiled, so compiling only the selected packages made `tools/coverage_report.py` report the five critical components as unmeasured and fail the job. A leaf-crate plan omits obc-link and obc-storage outright. Both nextest sections and the doctest section go back to `--workspace --all-features` with the tier filter, `tools/ci/rust_packages.py` is deleted, and the registry's unexpanded-variable accommodation goes with it. Reading a `tools/ci/*.sh` script as a job body stays: the commands live there. llvm-cov instrumentation is now on the two nextest steps only. It was job-wide, so the doctests, the default-feature formats run and the bench golden gate were instrumented too, and the bench release binary's profile data would have merged into the ratchet's evidence. Each section also runs in a subshell, so one section's environment cannot reach the next. The sweep step carries its own `ci.ui-snapshots` condition again. The job still gates on its own name, so routing, `validate-filters` and the aggregate are unchanged, but the sweep itself runs only for the suite that owns the rendering triggers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… suite Replaces the package-narrowing description with the reason it cannot narrow: the coverage ratchet needs one report over the whole workspace. Records that only the two nextest sections are instrumented, and that the sweep step runs for `ci.ui-snapshots` alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review round 1 addressed — run 35084445187 on
|
| # | Step | Result | Seconds |
|---|---|---|---|
| 9 | cargo nextest (fast hermetic binaries) | success | 76 |
| 10 | cargo nextest (captured fixture binaries) | success | 10 |
| 11 | cargo test --doc (workspace, all features) | success | 3 |
| 12 | cargo test (obc-formats, default features = the device's shape) | success | 1 |
| 15 | firmware resource/dependency tool unit tests | success | 0 |
| 16 | firmware production dependency direction | success | 1 |
| 17 | obc-bench --check (render + read-counter golden gate) | success | 7 |
| 21 | Rust component coverage | success | 9 |
| 24 | Upload Rust test results | success | 1 |
| 25 | Upload Rust fixture test results | success | 1 |
Whole job 178 s against the epic's 302 s and 304 s baseline. guards 34 s, ui-snapshots 157 s
and builder-python 157 s in parallel. Every job in the run succeeded and the ci aggregate
passed.
Checks re-run on this head
python3 tools/suite_registry.py check # 83 suites, 285 discovered execution units
python3 tools/suite_registry.py validate-filters # 21 workflow jobs, 15 audited selection classes
python3 -m unittest discover -s tools/tests # Ran 205 tests ... OK
bash -n tools/ci/test.sh
cargo check -p obc-app -p obc-pack -p obc-bake --locked
cargo test -p obc-app --lib copernicus # copernicus_wording_matches_obc_elevation ... ok
cargo fmt --all -- --check
python3 docs/build_docs.py --check-links # all internal anchor links resolve
Line counts for the branch: 32 files changed, 391 insertions, 256 deletions.
|
@coderabbitai review |
|
Two wording errands from the review. The Outcome section reported the `test` job as "302 s to 178 s". 178 s was not a recorded measurement. The record now carries all three measured figures: 302 s before the epic, 153 s on TS-B's head when it shipped (#1841), and 177-194 s on `develop` push runs after every child landed, measured 2026-09-16. "The docs job still runs on them" read as though `build_docs.py` checks `CLAUDE.md` and `AGENTS.md`. It does not read them at all. They are owned by the documentation route so the paths are not unowned; the check that actually reads them is the unconditional `guards` job — the planner's own `check` plus the tool unit tests, which pin the agent-prose change class. Corrected in the `ci.docs` comment, the `TEST_POLICY_PATTERNS` comment and the change-selection table. Validation: `python3 tools/test_plan.py check` OK; `validate-filters` agrees on 22 jobs over 15 audited paths; `python3 -m unittest discover -s tools/tests` 159 tests OK; `python3 docs/build_docs.py --check-links` all internal anchor links resolve. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 participant
TS-B of epic #1816. All six numbered sections of the issue are done.
What changed, in plain English
Every test now has one explicit route, and the routes are written down. There is no expensive
tier and nothing to build one for: the whole repository has three ignored tests and five manual
Cargo-example commands, and each already had a route.
docs/testing.mdnow carries the completetable. The words "heavy" and
obc test heavyappear nowhere indocs/testing.mdorCONTRIBUTING.md, as the issue predicted — confirmed by grep, nothing to remove.A missing fixture now fails with the command that fixes it.
obc_fixtures::file()used toreturn
None, and it only panicked with a helpful hint whenOBC_REQUIRE_FIXTURESwas set. Allsixteen call sites turned that
Noneinto an.expectwhose message named no setup command, so aplain local run panicked with no way forward.
file()now returns the path or panics withRun \obc fixtures sync test`;file_in(profile, …)names a different profile for the one caller outside thetestprofile. The environment variable is gone from the loader,tools/justfile, the workflow, the registry command andfixtures/verify-assistant-places.py.sim-peak-view(98 MiB) leaves thetestsync profile: no test reads it, only the simulator does at run time, and it stays in thesim` profile.The
testjob does not narrow what it compiles, and it cannot yet. The first round of thispull request compiled only the packages the plan selected. That breaks the per-pull-request
coverage ratchet:
tools/coverage_report.pywalksgit ls-filesfor the five critical components(format-protocol-codecs, crc, storage, dfu, boot) and fails any file the run never compiled, so a
leaf-crate plan such as
-p obc-bench -p obc-formats -p obc-vectors, which omits obc-link andobc-storage outright, reports them as unmeasured and fails the job. The epic's rule is to keep the
ratchet as TS5 accepted it, so the narrowing is gone: both nextest steps and the doctest step are
--workspace --all-featureswith the tier filter, andtools/ci/rust_packages.pyis deleted. Themeasured 302 s to 153 s comes from the serial-path split, not from compiling less. Whether the
ratchet moves to develop pushes, which would unlock narrowing, is deferred to TS-C2 and the owner.
One job body file.
tools/ci/test.shholds thetestjob's five sections (fast nextest,fixture nextest, doctests, default-feature formats, bench golden). CI runs one section per step so
each keeps its own step name and result artifact;
obc check testruns the same file (allsections except the fixture tier, which needs a download).
Job shape. Seven guard jobs, each paying for a checkout to run one fast Python script, are now
seven steps of one unconditional
guardsjob, which also carries the registry drift check, theregistry unit tests and the fixture policy. No guard's logic moved. The UI sweep and the builder
pytest left the
testjob's serial path forui-snapshotsandbuilder-python. The builder jobnow fails when the packer binary or the corpus is missing instead of skipping its packer half.
Local runner and the attribution move.
obc test -p PKGruns nextest — the runner CI uses —and then the same scope's doctests.
obc test -p PKG --libis the library-only loop: one target,no doctests; Cargo itself rejects
--libon a package with no library, so no extra check wasneeded.
COPERNICUS_ATTRIBUTIONandSOURCE_DATASETmoved fromhost/obc-demto thefirmware/obc-elevationleaf every consumer already depends on, andobc-appdropped theobc-demdev-dependency that existed only so one unit test could read the wording.Expected job sets that changed, and why
tools/tests/test_suite_registry.pyasserts exact job sets per change class. Every change belowfollows from the job split; none of them is a policy decision I made on my own.
python.repository-tools["fixture-registry", "selection"]["guards", "selection"]python.builder["test"]["builder-python"]ci.ui-snapshots["ui-snapshots"]— addedrust.obc-sim["clippy", "fmt", "test", "ui-snapshots"]— addedbuilder-pythonandui-snapshotsbuilder-pythonandui-snapshotsbuilder-pythonandui-snapshotsThe sweep's selection budget is unchanged. The
ui-snapshotsjob gates on its own job name,so every suite that routes to it can start the job — and because the job compiles the simulator,
that includes each package the simulator is built from. The sweep step carries
contains(fromJSON(needs.selection.outputs.plan).selected_suite_ids, 'ci.ui-snapshots'), exactlywhere that condition lived before, so the render and manifest check still runs only for the suite
that owns the rendering triggers.
workflow_jobs()reads job-level keys only, so routing,validate-filtersand the aggregate see no change, and decision D5 is preserved as written.Accepted costs
failure at once; one job stops at the first. The trade is six runner set-ups per pull request
against a second iteration when two guards break together.
ui-snapshotsandbuilder-pythonjobs start for every package they compile, so afoundational Rust change now provisions two more runners.
ui-snapshotsskips its own sweepstep unless
ci.ui-snapshotsis selected, so what it costs in that case is the set-up, not the106 s render.
Registry changes (small, and TS-C replaces them)
scan_workflowreads atools/ci/*.shscript as the job body it is and records its commandlines, applying the same
export/assignment/comment rules the workflow loop already applied.Without this the extracted commands would vanish from discovery and their suites would go dead.
Scope is deliberately
tools/ci/only:builder/build-wasm-bridges.shand friends are buildtools, and their invocation line stays the owned unit.
OBC_FIXTURE_ROOT="$(python3 …)"inside the job body is read as shell state, not as a command.run --level,run --affected,gatesandcargo-filterare untouched and still pass theirtests.
Measurements
llvm-cov instrumentation, on this machine, final head. Both runs used the same fixed package
set (
obc-app obc-route obc-render obc-reader obc-storage obc-link obc-formats) and the same fastfilter. I did not measure the whole workspace: 19 GiB free with three agents building, and a
second instrumented workspace build was not a risk worth taking for a ratio. Reported as measured,
not extrapolated.
cargo llvm-cov show-envfirmware/obc-app/src/lib.rsInstrumentation costs nothing measurable at execution time. It costs on compilation:
cargo-llvm-cov sets
cfg(coverage)and coverageRUSTFLAGS, so the instrumented artifacts are aseparate build, and a one-crate edit cost 3.4× here. CI caches the instrumented artifacts under
the shared
host-testskey, so a pull request restores them and pays the incremental figure, notthe cold one. The instrumentation stays; TS5's ratchet keeps it.
llvm-cov scope. Instrumentation is on the two nextest steps only. A local run of the
formats-defaultsection added zerotarget/*.profrawfiles, which is the shape everyuninstrumented section now has; previously the flag was job-wide and the bench gate's release
binary wrote profile data into the ratchet's evidence.
Verification
Commands run, all from the worktree, all passing unless noted:
python3 -m unittest discover -s tools/testsneedstools/requirements-test.txtinstalled;without it three unrelated tests error on missing
tree_sitterandxmlrunneron this machine.I ran the whole set in a scratch virtual environment with those pinned requirements.
Acceptance check:
grep -rn OBC_REQUIRE_FIXTURESover the tree returns nothing, andcargo test -p obc-route --features external-fixtures --test nav_fixtureswith an empty fixtureroot fails with
No UI sweep was run: no rendering file changed.
Line counts
32 files changed, 391 insertions, 256 deletions. The one new file is
tools/ci/test.sh(80);.github/workflows/ci.ymlis 123 added against 132 deleted, andtools/suite_registry.pyis19 against 3.
Left out, deliberately
tools/suite_registry.pystays authoritative; TS-C1/TS-C2 replace it.all out of scope for this issue.
obc check testdoes not run the fixture tier. It runs the same file, not every section: thefixture tier needs a sync, and a local gate should not start a download.
Corrections to the issue
request; it needs one LCOV report over the whole workspace. The narrowing is dropped here and
the question is deferred, as described above.
fixtures/verify-assistant-places.pyalso setOBC_REQUIRE_FIXTURES; the issue did not list it.The acceptance grep would have failed without it.
host/obc-bakehad noobc-elevationdependency, so the attribution move adds one.Closes #1820
🤖 Generated with Claude Code