fix(scanner): attribute a marker to the test it annotates (REQ-326, #892, #787) - #897
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📐 Rivet artifact delta
Graphgraph LR
REQ_326["REQ-326"]:::added
classDef added fill:#d4edda,stroke:#28a745,color:#155724
classDef removed fill:#f8d7da,stroke:#dc3545,color:#721c24
classDef modified fill:#fff3cd,stroke:#ffc107,color:#856404
classDef overflow fill:#e2e3e5,stroke:#6c757d,color:#495057,stroke-dasharray: 3 3
Added
Posted by |
, #787) `find_enclosing_function` only ever scanned BACKWARDS, so a marker written in the conventional place — on the line above the `#[test]` it annotates — was attributed to the function before it. Reported twice, from meld (#892) and earlier as #787. Three lines reproduce it: #[test] fn first_unrelated_test() {} // rivet: verifies REQ-001 -> reported first_unrelated_test #[test] fn the_test_that_actually_verifies_it() {} The damage is narrow and bad. The requirement MAPPING stays correct, so `rivet verify` still advances and coverage percentages are unaffected. What is wrong is the EVIDENCE LINE, which names a different test than the one that verifies the requirement — and for a reader auditing the right-hand side of the V, that line is the entire product. Following it leads to a test that does not test the thing. Fixed by walking FORWARD first, but only across lines that may legitimately separate a marker from the item it annotates: attributes, comments, blank lines. The bound is what makes it correct rather than merely different. A marker inside a function body has real code on the next line, so the walk stops there and the existing backward scan returns the enclosing function — which is the shell convention REQ-319 depends on. Negative-controlled both ways at --lib scope, which is what shows the bound is load-bearing rather than decorative: remove the forward walk 1 failed (the reported bug returns) make the walk unbounded 2 failed (in-body case AND the shell case) Worth recording how a mistake in this commit was caught. Inserting these tests spliced a doc comment into the middle of REQ-319's own doc comment, orphaning its `rivet: verifies` line onto the wrong function. `cargo test` and `clippy` both passed — doc comments concatenate harmlessly — and only `rivet coverage --tests` caught it, by reporting REQ-319 against a test that has nothing to do with shell scanning. The tool found a defect the compiler could not see, which is the case for the tool existing. Confirmed with cargo fmt --check, clippy --all-targets -D warnings on 1.97.0, cargo test --workspace (exit 0, 66 ok, 0 failed), the full cli_commands suite (189 passed), rivet validate, rivet docs check, yamllint — all exit 0. Implements: REQ-326 Verifies: REQ-326
…(REQ-326)
The PR-diff mutation gate found five survivors, every one a `||`-to-`&&` in the
`is_separator` chain. The fixtures used `#[test]` and nothing else, so no test
could distinguish any other disjunct.
The fix is not more fixtures for eight disjuncts — it is that eight was wrong:
#[attr] #!inner # comment all start with '#', so one check covers three
pub async sit on the `fn` line the pattern already
matches, so as separate lines they never occur
Four remain — blank, `#`, `//`, `@` — each genuinely distinct and each now
exercised by its own case, plus a case proving a NON-separator stops the walk so
the backward scan wins. Without that last one the other four would also pass
under an unbounded walk, which is the mistake the earlier round already made
once.
Negative-controlled at --lib, the scope the gate uses: turning any one of the
four into `&&` reddens two tests. Baseline 16 pass.
Worth stating plainly: the gate did not just find missing tests here, it found
dead predicate. Three of the eight disjuncts could never change an outcome and
two could never be reached, so no test could have killed those mutants — the
honest fix was to delete them rather than to write tests asserting behaviour
that does not exist.
Confirmed with cargo fmt --check, clippy --all-targets -D warnings on 1.97.0,
cargo test --workspace (exit 0, 66 ok), the full cli_commands suite (189
passed), rivet validate — all exit 0. REQ-326 now shows 3 test markers, each
correctly attributed.
Verifies: REQ-326
avrabe
force-pushed
the
fix/req-326-marker-attribution
branch
from
September 7, 2026 03:55
5f864a5 to
7b14dfc
Compare
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
1 participant
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.
Closes #892. Closes #787.
find_enclosing_functiononly ever scanned backwards, so a marker written inthe conventional place — on the line above the
#[test]it annotates — wasattributed to the function before it. Reported twice, from meld (#892) and
earlier as #787.
The damage is narrow and bad
The requirement mapping stays correct —
rivet verifystill advances, andcoverage percentages are unaffected. What's wrong is the evidence line, which
names a different test than the one that verifies the requirement.
For a reader auditing the right-hand side of the V, that line is the product.
Following it leads to a test that does not test the thing.
Why the fix is bounded
Forward walk, but only across lines that may legitimately separate a marker from
the item it annotates: attributes, comments, blank lines. A marker inside a
body has real code on the next line, so the walk stops and the existing backward
scan returns the enclosing function — which is the shell convention REQ-319
depends on.
Negative-controlled both ways, which is what shows the bound is load-bearing:
A mistake in this commit, and what caught it
Inserting these tests spliced a doc comment into the middle of REQ-319's own
doc comment, orphaning its
rivet: verifiesline onto the wrong function.cargo testpassed.clippypassed. Doc comments concatenate harmlessly, sonothing in the Rust toolchain could see it. Only
rivet coverage --testscaughtit — by reporting REQ-319 against a test with nothing to do with shell scanning.
The tool found a defect the compiler could not. That's worth more than the fix.
Gates
cargo fmt --check·clippy --all-targets -D warningson 1.97.0 ·cargo test --workspace(exit 0, 66 ok) · fullcli_commands(189 passed) ·rivet validate·rivet docs check·yamllint— all exit 0.REQ-326 →
verified, 2 test markers, correctly attributed.