perf(verify): name the tests 5 falcon-core verification steps actually rely on - #343
Open
avrabe wants to merge 13 commits into
Open
perf(verify): name the tests 5 falcon-core verification steps actually rely on#343avrabe wants to merge 13 commits into
avrabe wants to merge 13 commits into
Conversation
…y rely on Measured, not guessed. `cargo test -p falcon-core` is 60 tests / 51s, and the verification sweep runs it as a WHOLE-CRATE step 8 times — ~408s of one sweep spent re-running the same suite for artifacts that each care about 1-3 tests. Converts 5 of those 8 to named filters. Every one was verified locally to execute >0 tests and pass: partitioned 3 passed 7.44s (FV-FALCON-PART-001, debug + release) gnss 3 passed 15.65s (FV-FALCON-GNSS-002) prearm 2 passed 0.15s (FV-FALCON-PREARM-003) battery 2 passed 0.53s (FV-FALCON-BATTERY-002) sag_punch 1 passed 0.62s (FV-FALCON-BATTERY-002, "sag-compensated") range 1 passed 1.87s (FV-FALCON-RANGEDRV-001) partitioned 3 passed 0.27s (--release variant) 6 whole-crate steps (>=306s) -> 7 named steps (31s measured). This is #262 part 2. Part 1 (the `release:` backfill) shipped as TRACE-P01 in v1.132. Whole-crate executable steps repo-wide: 70 -> 65. Beyond speed, this is the traceability point: a whole-crate step says "something in this crate passes", a named step says "THIS test verifies this requirement" — and the gate already fails a named step that runs 0 tests (`cargo_tests_passed(...) == 0`, pulseengine.eu#89), so named steps are self-checking against evidence drift in a way whole-crate steps are not. NOT converted, deliberately: - FV-FALCON-NOTCH-001 — the only notch test in falcon-core (`notch_reduces_rate_loop_noise_under_rotor_vibration`) is `#[ignore]`d pending #290, so naming it would run 0 tests and correctly FAIL the gate. That is a real evidence gap, reported on #290 rather than hidden here. - FV-FALCON-FLOAT-001, FV-FALCON-MAVLINK-003 — the intended test set is not unambiguous from the artifact; left whole-crate rather than guessed. `rivet validate` PASS (warnings 352 -> 350). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG
This was referenced Aug 6, 2026
rivet hygiene: backfill release: fields + convert whole-crate verification steps to named tests
#262
Open
avrabe
enabled auto-merge (squash)
August 7, 2026 19:05
avrabe
added a commit
that referenced
this pull request
Aug 8, 2026
…350) The sweep has grown into its own ceiling. Measured across one day, same 156-artifact sweep: #340 ~63m success #344 1h30m21s FAILURE — killed at timeout-minutes: 90 #344 (earlier) cancelled #342 failed twice the same way #343 success twice Variance now exceeds headroom, so a REQUIRED check is decided by runner load rather than by whether anything is wrong. And a timeout surfaces as `fail`, indistinguishable from a real failure unless you read the duration — which already cost a diagnosis cycle. Worst of all, it was blocking the two PRs meant to improve this gate (#342 fail-open fix, #343 sweep speedup). Raising the ceiling does not weaken the gate: it is being KILLED, not failing. Landing it here because this PR already owns this file and is itself blocked by the timeout it fixes. THIS IS A STOPGAP and the third reactive bump (60 -> 90 -> 150). The trend is the real problem: 45m in July, >=90m now. #350 tracks it, and names the next measurement — the compile-vs-test split ON the runner. The sweep spans 64 crates with 29 `--release` steps, so compilation is the likely dominant cost; whole-crate -> named conversion (#343/#262) is right for traceability but, measured, will not close a 30-minute gap on its own (the non-falcon-core offenders run in 0-5s). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG
avrabe
added a commit
that referenced
this pull request
Sep 2, 2026
…te (#342) * fix(ci): a Verify-Filter matching zero artifacts must not pass the gate The verification gate is a REQUIRED check, and it could be silenced by a one-word change to the PR body. `scripts/run-falcon-verification.py` exits 0 when its filter matches nothing — correctly, in isolation: no artifact ran, so none failed. But the gate's verdict is exactly that exit code (`steps.verify.outcome`), so a `Verify-Filter:` that matches zero artifacts turns a required safety gate green while verifying nothing. It even renders "✅ Rivet verification gate — 0/0 passed" into the sticky PR comment, so the deception is not visible to a reviewer skimming it. The PR body is attacker-controllable. The INJECTION risk was already handled (env-bound, quoted, never interpolated into run:) — but the SEMANTIC trust of the filter was not: an untrusted string decides how much verification happens, and "none" was an accepted answer. Caught empirically, not by review: `(has-tag "traceability")` reads perfectly plausible and matches 0 of the 195 sw-verification artifacts, because that tag lives on sw-reqs, not on verifications. I had put exactly that filter on #341. Fix: after the sweep, fail if the script reports `0 artifact(s) matched`. Verified locally: (has-tag "traceability") -> 0 artifacts -> guard fires (has-tag "oci") -> 1 artifact -> no false positive (has-tag "falcon") -> 156 artifacts (the default) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG * fix(ci): raise the verification-gate timeout 90 -> 150 (STOPGAP, see #350) The sweep has grown into its own ceiling. Measured across one day, same 156-artifact sweep: #340 ~63m success #344 1h30m21s FAILURE — killed at timeout-minutes: 90 #344 (earlier) cancelled #342 failed twice the same way #343 success twice Variance now exceeds headroom, so a REQUIRED check is decided by runner load rather than by whether anything is wrong. And a timeout surfaces as `fail`, indistinguishable from a real failure unless you read the duration — which already cost a diagnosis cycle. Worst of all, it was blocking the two PRs meant to improve this gate (#342 fail-open fix, #343 sweep speedup). Raising the ceiling does not weaken the gate: it is being KILLED, not failing. Landing it here because this PR already owns this file and is itself blocked by the timeout it fixes. THIS IS A STOPGAP and the third reactive bump (60 -> 90 -> 150). The trend is the real problem: 45m in July, >=90m now. #350 tracks it, and names the next measurement — the compile-vs-test split ON the runner. The sweep spans 64 crates with 29 `--release` steps, so compilation is the likely dominant cost; whole-crate -> named conversion (#343/#262) is right for traceability but, measured, will not close a 30-minute gap on its own (the non-falcon-core offenders run in 0-5s). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG * fix(ci): my zero-match guard emitted INVALID YAML — backslash continuation at column 0 Self-inflicted, and it broke the very gate the guard was meant to strengthen. The guard's error message used a backslash-continued string whose continuation lines started at COLUMN 0: echo "::error::Verify-Filter matched 0 ... — \ refusing to pass a gate that verified nothing. Fix the filter or drop the \ override to use the default." Inside a `run: |` block scalar, a line at column 0 TERMINATES the block. The whole workflow file was therefore invalid YAML from the moment I added the guard, which is almost certainly why this PR's verification gate failed twice — NOT the 90-minute timeout I attributed it to in the previous commit. `python3 -c "yaml.safe_load(...)"` reports: ScannerError: while scanning a simple key, line 234 ... could not find expected ':' Fixed by keeping the message on one line, with a comment saying why, so the next person does not "tidy" it back into a continuation. Lesson worth the comment: I verified this file for the thing I was thinking about (no `${{ }}` inside `run:` — which I DID check, with a parser) and not for whether it still parsed at all. Both checks now run. YAML parses: OK, timeout-minutes=150 ${{ }} inside run: blocks: none Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Measured, not guessed. Part 2 of #262 (part 1, the
release:backfill, shipped as TRACE-P01 in v1.132).The number
cargo test -p falcon-coreis 60 tests / 51s, and the verification sweep runs it as a whole-crate step 8 times — roughly 408s of every sweep re-running one suite for artifacts that each care about 1–3 tests.What changed
5 of those 8 steps now name their tests. Each was run locally and verified to execute >0 tests and pass:
partitionedgnssprearmbatterysag_punchrange6 whole-crate steps (≥306s) → 7 named steps (31s measured).
Whole-crate executable steps repo-wide: 70 → 65.
Why it matters beyond speed
A whole-crate step says "something in this crate passes". A named step says "this test verifies this requirement" — and the gate already fails a named step that runs 0 tests (
cargo_tests_passed(...) == 0, pulseengine.eu#89), so named steps are self-checking against evidence drift; whole-crate steps are not.Deliberately NOT converted
falcon-core,notch_reduces_rate_loop_noise_under_rotor_vibration, is#[ignore]d pending Notch closed-loop hover is a non-deterministic oracle: chaotically fragile under rotor-line vibration (kernel-agnostic; mechanism unknown) #290.Naming it would run 0 tests and correctly fail the gate. That means the artifact’s
cargo test -p falcon-corestep contributes nothing to NOTCH-P01 today — it runs 60unrelated tests. Reported on Notch closed-loop hover is a non-deterministic oracle: chaotically fragile under rotor-line vibration (kernel-agnostic; mechanism unknown) #290 rather than papered over here.
unambiguous from the artifact. Left whole-crate rather than guessed.
Evidence
rivet validate→ PASS (warnings 352 → 350)🤖 Generated with Claude Code
https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG