Skip to content

fix(scanner): read verifies markers from shell scripts (REQ-319, #870) - #872

Open
avrabe wants to merge 1 commit into
mainfrom
fix/req-319-shell-markers
Open

fix(scanner): read verifies markers from shell scripts (REQ-319, #870)#872
avrabe wants to merge 1 commit into
mainfrom
fix/req-319-shell-markers

Conversation

@avrabe

@avrabe avrabe commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Closes #870.

coverage --tests read # rivet: verifies REQ-X from a Python file but not
the identical comment from a shell script
. detect_language had no entry for
sh at all, so scan_file returned before any pattern ran — the file was
skipped in silence.

a.rs   // rivet: verifies REQ-RS-001   →  found
b.py   #  rivet: verifies REQ-PY-001   →  found
c.sh   #  rivet: verifies REQ-SH-001   →  No test markers found

This costs evidence, not tidiness

Not every falsifiable check is a unit test, and the ones that aren't tend to
guard the nastiest failures. Your two examples — a gate refusing any workflow
that redirects key material into a file (proved by rejecting four shapes,
including the exact line your own deposit workflow shipped), and one asserting
the installer warns when a different binary wins the PATH lookup — both run in
CI, both carry negative controls, and both are the sole evidence for their
requirement. Which therefore could not reach verified by any honest route.

The fix

Shell gets its own language category rather than an alias for python: the
comment pattern is identical but the enclosing-function regex is not
(name() { and function name, not def).

Also a shebang fallback for extensionless files, since a gate is frequently
tools/no-key-on-disk rather than …​.sh. Consulted only when a file has no
extension, so a tree of images and binaries isn't read looking for one — and the
test pins that a plain text file with no shebang is still skipped.

Both halves negative-controlled:

remove the sh mapping       →  extension test FAILED
remove the shebang fallback →  extensionless test FAILED

On #871 — part (1) is already fixed

Part (1) shipped in v0.34.0 (REQ-294). Verified on current main:

swe3-refines-swe2   sw-detail-design   0   0   n/a%
V-closure: sw-detail-design (all 2 rules)   n/a%  [0/0]
JSON: percentage: null, empty_scope: true

That's exactly the shape your suggestion (1) asks for, including the JSON null.
You're on scry's vendored copy, which predates the release — worth a bump.

Part (2) is real and unaddressed and is now REQ-320: there's no way to subset
an embedded preset's rules, so adopting aspice for SWE.1/SWE.6 necessarily
inherits SWE.2/3/4 rows you can never satisfy. Your workaround — a checked-in
declaration of unmodelled levels, gated in CI, failing both when an empty
population is undeclared and when a declaration goes stale — is the tell that
this is a mechanism rivet should own.

I've deliberately grouped it with REQ-309 (exempt-when-field) and REQ-313
(declared-blocked criteria) rather than solving it standalone: three
near-identical mechanisms for "this is declared, not forgotten" would be worse
than one.

Gates

cargo fmt --check · clippy --all-targets -D warnings on 1.97.0 ·
cargo test --workspace (exit 0, 65 ok, 0 failed) · rivet validate ·
rivet docs check — all exit 0. REQ-319 → implemented, 2 test markers.

`coverage --tests` read `# rivet: verifies REQ-X` from a Python file but not the
identical comment from a shell script. `detect_language` had no entry for `sh`
at all, so `scan_file` returned before any pattern ran and the file was skipped
in silence.

Reproduced with three files in one directory, one marker each: the .rs and .py
markers were found, the .sh one reported "No test markers found".

This costs evidence, not tidiness. Not every falsifiable check is a unit test,
and the ones that are not tend to guard the nastiest failures. The reporter's
two examples are a gate refusing any workflow that redirects key material into
a file — proved by rejecting four shapes, including the exact line their own
deposit workflow shipped — and one asserting the installer warns when a
different binary wins the PATH lookup. Both run in CI, both carry negative
controls proving they can go red, and both are the sole evidence for their
requirement, which therefore could not reach `verified` by any honest route.

Shell gets its own language category rather than an alias for python: the
comment pattern is identical but the enclosing-function regex is not
(`name() {` and `function name`, not `def`).

Also added a shebang fallback for EXTENSIONLESS files, since a gate is
frequently `tools/no-key-on-disk` rather than `tools/no-key-on-disk.sh`. It is
consulted only when a file has no extension, so a tree of images and binaries is
not read looking for one, and the test pins that a plain text file with no
shebang is still skipped.

Both halves negative-controlled: removing the sh mapping reddens the extension
test, removing the shebang fallback reddens the extensionless one.

Also files REQ-320 for #871. Part (1) of that issue — an empty-population rule
rendering as 100.0% — was ALREADY FIXED in v0.34.0 by REQ-294, verified on
current main: the per-rule table and the V-closure lines both render `n/a%` and
the JSON emits `percentage: null` with `empty_scope: true`, which is the shape
the issue asks for. The reporter is on scry's vendored copy, which predates it.
Part (2), no way to subset an embedded preset's rules, is real and unaddressed;
grouped with REQ-309 and REQ-313 rather than solved standalone, because three
near-identical mechanisms for "this is declared, not forgotten" would be worse
than one.

Confirmed with cargo fmt --check, clippy --all-targets -D warnings on 1.97.0,
cargo test --workspace (exit 0, 65 ok), rivet validate, rivet docs check — all
exit 0.

Implements: REQ-319
Refs: REQ-320, REQ-294
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.83784% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rivet-core/src/test_scanner.rs 87.83% 9 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

Copy link
Copy Markdown

📐 Rivet artifact delta

Change Count
Added 2
Removed 0
Modified 0
Downstream impacted (depth ≤ 5) 0

Graph

graph LR
  REQ_319["REQ-319"]:::added
  REQ_320["REQ-320"]:::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
Loading
Added
  • REQ-319
  • REQ-320

📎 Full HTML dashboard attached as workflow artifact rivet-delta-pr-872download from the workflow run.

Posted by rivet-delta workflow. The graph shows only changed artifacts; open the HTML dashboard (above) for full context.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: b3cc8a4 Previous: 9d9a697 Ratio
store_insert/100 83438 ns/iter (± 2949) 66482 ns/iter (± 3182) 1.26
store_insert/1000 890745 ns/iter (± 9791) 718522 ns/iter (± 3126) 1.24
store_lookup/100 2068 ns/iter (± 32) 1461 ns/iter (± 4) 1.42
store_lookup/1000 26434 ns/iter (± 107) 17816 ns/iter (± 33) 1.48
store_lookup/10000 355702 ns/iter (± 1284) 258802 ns/iter (± 2208) 1.37
store_by_type/100 95 ns/iter (± 0) 75 ns/iter (± 1) 1.27
store_by_type/1000 95 ns/iter (± 0) 75 ns/iter (± 0) 1.27
store_by_type/10000 94 ns/iter (± 0) 75 ns/iter (± 0) 1.25
schema_load_and_merge 1511704 ns/iter (± 28167) 1183968 ns/iter (± 12458) 1.28
link_graph_build/100 157920 ns/iter (± 842) 126045 ns/iter (± 264) 1.25
link_graph_build/1000 1902548 ns/iter (± 16006) 1497016 ns/iter (± 23213) 1.27
link_graph_build/10000 27180072 ns/iter (± 1386691) 21409503 ns/iter (± 500407) 1.27
validate/100 504193 ns/iter (± 3302) 344980 ns/iter (± 1286) 1.46
validate/1000 18318731 ns/iter (± 142208) 10667637 ns/iter (± 38321) 1.72
validate/10000 1521291383 ns/iter (± 11816313) 753996489 ns/iter (± 5648610) 2.02
traceability_matrix/100 4355 ns/iter (± 70) 3209 ns/iter (± 30) 1.36
traceability_matrix/1000 59664 ns/iter (± 475) 33994 ns/iter (± 115) 1.76
traceability_matrix/10000 821192 ns/iter (± 3415) 554924 ns/iter (± 2007) 1.48
diff/100 62556 ns/iter (± 370) 47962 ns/iter (± 113) 1.30
diff/1000 712804 ns/iter (± 3944) 516450 ns/iter (± 1949) 1.38
diff/10000 7730356 ns/iter (± 152272) 6242821 ns/iter (± 139231) 1.24
query/100 1114 ns/iter (± 11) 765 ns/iter (± 3) 1.46
query/1000 14044 ns/iter (± 311) 10410 ns/iter (± 33) 1.35
query/10000 318601 ns/iter (± 5604) 170185 ns/iter (± 625) 1.87
document_parse/10 23671 ns/iter (± 333) 16350 ns/iter (± 41) 1.45
document_parse/100 170454 ns/iter (± 984) 111521 ns/iter (± 478) 1.53
document_parse/1000 1565569 ns/iter (± 30845) 1038384 ns/iter (± 8076) 1.51

This comment was automatically generated by workflow using github-action-benchmark.

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.

coverage --tests does not read markers from shell scripts, so gate-verified requirements cannot close the V

1 participant