Skip to content

Make status predict what run does, in one shared plan - #71

Merged
max-sixty merged 9 commits into
mainfrom
collect-perf
Jul 28, 2026
Merged

max-sixty merged 9 commits into
mainfrom
collect-perf

Conversation

@max-sixty

Copy link
Copy Markdown
Owner

status exists to predict run, and the two carried separate copies of the selection pipeline, each annotated as mirroring the other. They had drifted: run listed tests with the caller's build flags, status with none — so on a feature-gated suite the dry run under-reported against what run would actually execute, which is the one direction a dry run must never be wrong in. You act on status by not looking further.

$ cargo affected status                    # the only form that existed
… no mention of test_extra …

$ cargo affected run -- --features extra
    RUNS test_extra                        # ran a test the dry run never named

status now takes the same post--- passthrough as run and collect, and both commands go through a new plan.rs that owns the shared decision: list tests, diff against every reachable collect_sha, apply config rules, select, classify hit-exact vs hit-with-divergence, assemble the report. run.rs and status.rs keep only what differs for real — which stream, which tense, and whether anything gets executed at the end.

The same pass added the gates that would have caught the drift's smaller siblings: cargo fmt --check and a RUSTDOCFLAGS=-D warnings doc build (a broken intra-doc link compiles fine and fails nothing else — this one found two), plus a cargo msrv verify job, because rust-version = "1.94" is a promise to everyone installing from crates.io and nothing in CI built with anything but current stable.

Reading this

Review commit-by-commit. About 450 of the ±lines are mechanical and carry no semantic content: 113 accumulated rustfmt hunks in 2d49d4e, and 221 pub → pub(crate) declarations in bd6f898. The behavior change itself is roughly 30 lines.

commit what
7d86d3d, fdbf6f0 the substantive refactor — new plan.rs (303 lines), run.rs 373→285, status.rs 279→233
2d49d4e format the repo, and the fmt/docs gates plus .pre-commit-config.yaml that keep it that way
7e6b4d2 the MSRV job, and tests/CLAUDE.md for the scenario conventions
bd6f898 pub(crate) throughout — nothing here is a library
rest module docs, one prune_and_report replacing three copies, named CacheStatus arms

Interfaces

cargo affected status [-- <nextest args>] is the only user-visible change, and it is purely additive — every existing invocation behaves as before. The --report-json schema (SCHEMA_VERSION 1), the CI summary line, and the DB schema are untouched, so there is nothing to migrate.

Testing

153 green (112 unit, 41 functional). status_prediction_uses_run_features was verified failing against the old &[] before being kept, per the repo's regression-test rule; status_accepts_explicit_double_dash_separator covers the clap surface. The binary was also driven end-to-end on a scratch crate through collect → status → edit → status -v → run → clean → status/run on both cache-miss paths.

Two gaps worth naming. Nothing asserts that status and run produce the same selection for a given state — the guarantee is structural (one code path) rather than pinned. And the three new CI jobs have never executed on GitHub; cargo fmt --check, the rustdoc gate and cargo +1.94 check were verified locally, but this PR is the first run of the workflow itself.

Known trade-off

status now runs check_shas_reachable — a few git calls, ~50ms — before printing its database inventory, where it used to print first. Accepted, since status then spends seconds in nextest list anyway.

This was written by Claude Code on behalf of max

max-sixty and others added 9 commits July 28, 2026 13:12
The lint job ran clippy and nothing else, so 113 rustfmt hunks across 20 files
accumulated unnoticed and a broken intra-doc link (`[`new_tests`]` in
`selection.rs`, which needed a `Self::` reference definition) compiled fine.

Adds the gates the sibling repos already run: `cargo fmt --check` and a
rustdoc `-D warnings` pass in CI, plus the pre-commit config worktrunk, prql
and insta share. `.typos.toml` records the two false positives this repo's
prose hits — hyphenated `mis-` prefixes and `raison d'être` — matching how
those repos handle the same thing.

`[lints.rust] unsafe_code = "forbid"` makes the crate's existing property
explicit, and `.tmp/` joins `.gitignore`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`status` exists to predict `run`. That promise was kept by hand: both
commands carried their own copy of the listing/diff/config/selection
pipeline and their own report assembly, each with a comment saying it
mirrored the other.

They had drifted. `run` listed tests with the caller's build flags via
`cargo_build_args(nextest_args)`; `status` listed with none. On any project
with feature-gated tests the dry run therefore predicted a different set
than the run executed — under-reporting, which is the one direction a dry
run must not be wrong in. `new_test.rs` already pinned this for `run`
(`new_test_detection_uses_run_features`); the sibling case for `status` was
never written, so nothing caught it.

The pipeline now lives in `plan.rs` and both commands call it. What differs
between them stays at the call sites because it is real: `run` narrates to
stderr in the present tense and ends by executing nextest, `status` prints a
database inventory to stdout in the conditional. `status` gains the same
post-`--` passthrough `run` and `collect` have, which is what lets it answer
about the build the user actually means.

`status_prediction_uses_run_features` covers it, and fails against the old
`&[]`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit unified the selection pipeline and claimed in plan.rs's
own docstring that the decision "lives here once". Reviewing it found that
only half of it did. `run.rs` and `status.rs` still each decided, from the
same predicate, whether an empty result meant `MissFingerprint` or
`MissNoCoverage` — including their own copies of the
`closest_stored_diff_labels` call — and each classified reachability
separately. Those are decisions wearing rendering's clothes: identical logic,
different prose.

`assess` now returns `CacheState`: `Usable(Reachability)`, or `Miss` with the
three reasons a diff can't be anchored. Both commands `match` it, so a fourth
state is a compile error in each rather than something to remember in the
second file. The split between the two enums is what avoids an
`unreachable!()` in the miss handler — every miss ends the same way, so they
share one exit.

What's left at the call sites is what actually differs: `run` narrates to
stderr in the present tense, `status` to stdout in the conditional and heads
its output with a database inventory. The user-visible text is byte-identical
— `fingerprint.rs` and `clean.rs` assert on it.

Also drops a `region_count` query on the path where nothing is tracked and so
nothing could be counted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`rust-version = "1.94"` is a promise to everyone installing from crates.io,
and nothing in CI built with anything but current stable — so the real
minimum could drift upward and only downstream users would find out. Adds the
`cargo msrv verify` job worktrunk runs, on its own cache since older
toolchains never match the stable one.

`tests/CLAUDE.md` writes down what the functional suite already assumes and
only `main.rs`'s module docs mentioned: unique package names per scenario and
why cargo needs them, the two git settings scratch repos must override,
why assertions are targeted `contains` rather than snapshots (this CLI's
output embeds temp paths, shas and elapsed seconds), the rule that a
regression test must be seen failing first, and the two tripwire scenarios
that exist to catch silent under-selection.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`summary_line`'s catch-all covered MissFingerprint, MissNoCoverage and
ForcedAll — correct today, since all three are full-suite runs, but a new
status would inherit that phrasing silently rather than failing to compile.
`as_str` on the same enum was already exhaustive.

The other `_` arms in src/ destructure Option/Result tuples rather than a
closed domain enum, and stay.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three paths reach the same point from different directions — a --diff with
nothing to recollect, a --diff after its run, and the no-results handler —
and each carried its own copy of the prune call, the >0 guard and the
pluralization. One fact, so one way of stating it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1,300 lines behind a one-line summary, holding the invariant the rest of the
tool rests on: stored ranges and diff hunks are both in the collect_sha's
coordinate system, so a switch to NEW-side line numbers would not fail — it
would silently select the wrong tests.

Also records the two module-wide rules that were only visible per-function:
git failure never degrades to an empty change set (indistinguishable from a
clean tree, so it would read as success while selecting nothing), and
canonicalization has to go opposite ways on macOS and Windows to match the
paths the compiler recorded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The module said it "owns the whole flow so the callers can't drift apart" —
`plan.rs` owns that now, and this module owns the pieces. And
`select_with_reach` justified itself by "the three callers", which became one
when `run` and `status` moved to `plan::plan`; the surviving caller is
`collect --diff`, and the reason `plan` open-codes the same pair (it needs the
ranges again for the report) is worth stating where the next reader will
wonder.

Docs that describe a structure the code no longer has are worse than none —
they read as current.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This package has no library target, so `pub` and `pub(crate)` compile to
the same thing — but `pub` reads as a promise to callers outside the
crate, and there are none. 221 declarations across src/ and the test
helpers now say what they mean, and CLAUDE.md records the convention.

The fixture crate source embedded in the test files keeps its `pub`:
those really are library items, in the scratch crates the scenarios
build.
@max-sixty
max-sixty merged commit 52c1971 into main Jul 28, 2026
8 checks passed
@max-sixty
max-sixty deleted the collect-perf branch July 28, 2026 22:53
@max-sixty max-sixty mentioned this pull request Jul 28, 2026
max-sixty added a commit that referenced this pull request Jul 28, 2026
Ships the collect speedup (#68) and the shared run/status plan (#71).

`collect` exports each test binary's coverage map once rather than once per
test. worktrunk's 1,407-test suite goes from 953 s of CPU to 116 s, and
stored ranges come out a superset of the old ones, never a subset.

`status` and `run` now derive their selection from one plan, so a dry run
can't predict something other than what runs. They had drifted: `run` listed
tests with the caller's build flags and `status` listed with none, so a
feature-gated suite was under-reported. `status` gains the post-`--` nextest
passthrough that `run` and `collect` already had.

Minor rather than patch because the passthrough is new CLI surface, `status`
now selects a different set on feature-gated projects, and a collect whose
recorded source paths land outside the project root fails rather than
storing them.

Also corrects the README, which said the crate wasn't on crates.io. v0.3.2
has been published since #66 landed Trusted Publishing.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants