fix(stella-pipeline,stella-protocol): record verifier degradation per candidate in a fan-out (#1787) - #1951
Merged
Merged
Conversation
… candidate in a fan-out A best-of-N fan-out whose verifier died reported one once-per-run prose caveat for N candidates, and nothing recorded WHICH candidates were judged by the deterministic heuristic. Each candidate now emits one structured ProofStep::VerdictDegraded fact (1-based ordinal + stated reason), keyed by candidate-local state so concurrent candidates cannot race a shared flag; the once-per-run transcript warning is unchanged. ProofStep moves to stella-protocol/src/proof.rs (the crate's ladder.rs pattern, re-exported so every existing path still resolves) because event.rs sits at its file-size ceiling; pipeline.rs stays under its own ceiling by consolidating the two once-per-run AtomicBool notices into VerifierNotices in verifier_stage.rs. Refs #1787
A two-candidate fan-out with a tokenless verifier records one VerdictDegraded fact per candidate (ordinals 1 and 2) beside the single once-per-run transcript warning, and a candidate that degrades on both of its rounds records exactly one fact — per candidate, never per round. CandidateSlot groups the ordinal with its workspace so the fan-out driver stays under the argument-count lint structurally rather than by allow. Regenerates docs/wire for the new ProofStep variant. Refs #1787
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's GuideAdds structured per-candidate verdict degradation tracking and proof steps while refactoring proof vocabulary and verifier notices to reduce god-files and make fan-out verifier behavior observable and race-free. Sequence diagram for per-candidate verdict degradation recordingsequenceDiagram
actor Candidate
participant Pipeline
participant Verifier as ModelVerifier
participant Heuristic as heuristic_fallback
participant Proof as emit_proof
Candidate->>Pipeline: run_candidate
Pipeline->>Pipeline: verifier(degradation, prompt, inputs, spend)
alt [verifier unresolvable]
Pipeline->>Pipeline: warn_verifier_fallback(degradation, reason)
else [verifier response off-protocol]
Pipeline->>Pipeline: warn_verifier_fallback(degradation, reason)
else [verifier call fails or times out]
Pipeline->>Pipeline: warn_verifier_fallback(degradation, reason)
end
Pipeline->>Heuristic: heuristic_fallback(inputs)
Heuristic-->>Pipeline: ModelVerifierVerdict
Note over Pipeline,Proof: Once per run: warn(prose)
Pipeline->>Proof: emit_proof(ProofStep::VerdictDegraded)
Proof-->>Pipeline: recorded once per candidate
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
macanderson
enabled auto-merge (squash)
August 7, 2026 00:58
macanderson
disabled auto-merge
August 7, 2026 01:03
Owner
Author
|
Merge-order note (parallel-PR sweep): this PR conflicts with its sibling from the same batch — #1945 and #1951 both touch |
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
`cargo clippy -p stella-pipeline --all-targets -- -D warnings` fails on b5ab7f8. A compile error in the lib masks the test target entirely, so this reads as one failure and is really four, from two different merges. From #1953 (the #1778 research stage): 1. `management_prompt/tests.rs` — `ModelCallRole::Research` is a new variant and `management_system_block`'s match is exhaustive on purpose (E0004). Research rides the sub-agent primitive, so its system prompt travels on the `SubAgentSpec`, never through `metered_raw_call`: it joins the never-dispatched arm, and `ALL_ROLES` grows to 15. 2. `pipeline.rs` — the new `research` parameter pushed `plan_stage` to 8 arguments, one over clippy's cap. Bundled `budget`/`total` into the `Spend` struct every stage downstream of the fan-out already takes, rather than `#[allow]`-ing the lint. From #1951, which rewrote `tests/verification_hardening.rs` wholesale and dropped three items #1945 had added to it hours earlier — a same-seam clobber, in a file #1951's own subject (per-candidate verifier degradation) never needed to touch: 3. `PassingShell` and `shell_call_result` went with it, leaving the child module `flip_halt_arming.rs` referencing two helpers that exist nowhere in the tree (E0425 ×2). Restored to their original home, which the child reaches through `use super::*`. 4. `a_revision_halts_at_the_step_where_the_tracked_test_flips` went too — the configured-command **witness for #1793**. Deleting it did not fail any gate, because the crate stopped compiling for reason 3 first: #1793 has been shipping with half its witness silently gone. Restored verbatim. Both #1793 witnesses now run and pass. Neither is vacuous: each asserts a scripted-prompt count, so a `PassingShell` that omitted the `[exit code: 0]` marker `flip_halt::exit_status` parses would leave the halt unarmed, the revision would consume the steps scripted beyond the flip, and the count would be wrong. `cargo test -p stella-pipeline`: 605 passed, 0 failed. `cargo clippy -p stella-pipeline --all-targets -- -D warnings`: clean.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…R's CI Merging main into this branch left two defects that neither side carried alone: - `resume_stage.rs` builds `CandidateState` by hand (mirroring `run_candidate`), and main's #1951 added the `degradation` field after this branch's copy was written — so the merge produced a struct literal missing a field. A resume re-enters exactly one candidate, so the record takes ordinal 1, the fan-out's 1-based convention. - The file-size baseline kept main's `pipeline/tests.rs` ceiling, losing this branch's `mod resume;` bump. Regenerated with `scripts/check-file-size.sh --update` rather than hand-edited: the two lines that move are exactly this PR's own, `pipeline.rs` re-tightening 3462 -> 3192 from the `execute_stage.rs` extraction and `pipeline/tests.rs` rising 3 for the irreducible module declaration. Refs #1671
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
… the research stage Third break on main from the same round of parallel merges: #1778 added 'research: &[ResearchFinding]' to plan_stage, taking it to 8 arguments against clippy's limit of 7. The gate runs clippy at -D warnings, so stella-pipeline could not pass it. Fixed structurally, not with an #[allow]: budget and total are exactly the pair the crate's own Spend envelope bundles (stage_budget.rs), and they travel together everywhere else — verifier() already took this shape in #1951. plan_stage takes Spend, plan_with_review builds one before its re-plan loop and reborrows it per iteration, and neither uses the two for anything else. 8 arguments become 7 and the meaning is unchanged. No behavior change.
This was referenced Aug 7, 2026
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
Restoring `a_revision_halts_at_the_step_where_the_tracked_test_flips` in the parent commit took `tests/verification_hardening.rs` to 1557 lines, which `file-size` rejects outright — the baseline takes no new entries. Split rather than exempted, and the split is the one the content was asking for: both #1793 witnesses and the two doubles they share (`PassingShell`, `shell_call_result`) now live in `verification_hardening/flip_halt_arming.rs`, the module already named for the concern. The parent drops to 1434. That the two witnesses were ever in separate files is what let #1951's clobber happen quietly: it rewrote the parent wholesale, taking the configured-command witness and both doubles with it, and nothing failed that named the missing test — the crate had already stopped compiling for the missing doubles. With the cluster in one file the same rewrite is a merge conflict instead of a silent deletion, so the module doc says so. `cargo test -p stella-pipeline`: 605 passed, 0 failed — both witnesses among them, at their new path.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
Restoring `a_revision_halts_at_the_step_where_the_tracked_test_flips` in the parent commit took `tests/verification_hardening.rs` to 1557 lines, which `file-size` rejects outright — the baseline takes no new entries. Split rather than exempted, and the split is the one the content was asking for: both #1793 witnesses and the two doubles they share (`PassingShell`, `shell_call_result`) now live in `verification_hardening/flip_halt_arming.rs`, the module already named for the concern. The parent drops to 1434. That the two witnesses were ever in separate files is what let #1951's clobber happen quietly: it rewrote the parent wholesale, taking the configured-command witness and both doubles with it, and nothing failed that named the missing test — the crate had already stopped compiling for the missing doubles. With the cluster in one file the same rewrite is a merge conflict instead of a silent deletion, so the module doc says so. `cargo test -p stella-pipeline`: 605 passed, 0 failed — both witnesses among them, at their new path.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…nd, incl. a silently deleted #1793 witness (#1971) ## What `main` at b5ab7f8 did not build: `cargo clippy -p stella-pipeline --all-targets -- -D warnings` failed. A compile error in the lib masks the test target, so it read as one failure and was really **four**, from two different merges. ### From #1953 (the #1778 research stage) 1. **`management_prompt/tests.rs` (E0004)** — `ModelCallRole::Research` is a new variant and `management_system_block`'s match is exhaustive by design. `Research` rides the sub-agent primitive, so its system prompt travels on the `SubAgentSpec`, never through `metered_raw_call` — it joins the never-dispatched arm, and `ALL_ROLES` grows to 15. 2. **`pipeline.rs` (`too_many_arguments`)** — the new `research` parameter pushed `plan_stage` to 8 args, one over the cap. Bundled `budget`/`total` into the `Spend` struct every stage downstream of the fan-out already takes, rather than `#[allow]`-ing the lint. `pipeline.rs` is a god file, so the doc comment is written to land the file back at **exactly** its 3462 ceiling — no growth. ### From #1951 — a same-seam clobber #1951 rewrote `tests/verification_hardening.rs` wholesale and dropped three items #1945 had added to it hours earlier, in a file its own subject (per-candidate verifier degradation) never needed to touch: 3. **`PassingShell` + `shell_call_result` (E0425 ×2)** — the child module `flip_halt_arming.rs` was left referencing two doubles that exist nowhere in the tree. 4. **`a_revision_halts_at_the_step_where_the_tracked_test_flips`** — the configured-command **witness for #1793**. Deleting it failed no gate, because the crate had already stopped compiling for reason 3. **#1793 has been shipping with half its witness silently gone.** Restored verbatim from eddf970. ## The structural fix, not just the restore Restoring the witness took the parent file to 1557 lines, which `file-size` rejects outright — and the baseline takes no new entries. So this splits rather than exempts, and the split is the one the content was asking for: **both** #1793 witnesses and the two doubles they share now live in `verification_hardening/flip_halt_arming.rs`, the module already named for the concern. Parent drops to 1434. That the two witnesses were ever in separate files is what let the clobber happen quietly. With the cluster in one file, the same wholesale rewrite is a merge conflict instead of a silent deletion — the module doc records why. ## Witness Not a pure refactor: items 3 and 4 restore two witness tests, and neither passes vacuously. Each asserts a scripted-prompt count, so a `PassingShell` that omitted the `[exit code: 0]` marker `flip_halt::exit_status` parses would leave the halt unarmed, the revision would consume the steps scripted beyond the flip, and the count would be wrong. ``` test ...flip_halt_arming::a_revision_halts_at_the_step_where_the_tracked_test_flips ... ok test ...flip_halt_arming::an_authored_witness_arms_the_revision_flip_halt ... ok ``` - `cargo test -p stella-pipeline` — **605 passed, 0 failed** - `cargo clippy -p stella-pipeline --all-targets -- -D warnings` — clean - `make guards-fast` — green, `file-size` and `god-files` included ## Overlap with #1965 This branch originally also retired the stale `crates/stella-protocol/src/event.rs` baseline entry (1454 lines against a recorded 2965 — a fifth red gate on main). **#1965 landed the same fix while this was in flight**, so that work was dropped here in favour of theirs on rebase; the remaining commit is only the split. No duplicate baseline edit.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…ngress (#1787) (#1982) > **Stacked on #1975.** Based on `unbreak-main-pipeline` because `main` does not > currently compile `stella-pipeline`'s test target; the diff below is the one > commit on top. GitHub retargets this to `main` when #1975 merges. ## What & why The last unbounded ingress into the verdict prompt, and the item #1787 folds in at the end of its body: > Also worth folding in: the trusted evidence summary has no length bound > (`oracle_trace` grows per observation; the diff has a token budget, the > trusted zone does not) — `pipeline/evidence.rs` since the extraction. `verifier_evidence_summary` renders `oracle_trace` in full. That trace gains an observation per verification round, and the repair gate (#1479) keeps granting rounds for as long as a measured budget affords them — so the one channel that grows without limit was also the one channel with no ceiling. Every other input to that prompt is bounded: the diff has a token budget, recall frames have `bound_recalled_frames`, and `Verdict::reasoning` got its cap in #1932. Bounded to the newest 24 observations, with the drop **stated in-band**: ``` oracle_trace=[…76 earlier observation(s) omitted → candidate:pass → candidate:fail → …] ``` Three choices worth naming, because each has a wrong-looking alternative: - **Newest kept, oldest dropped.** The recent runs are what the verdict weighs; a trace clipped from the front would hand the verifier a history that stops before the evidence. - **Stated, not silent.** A trace that silently began mid-run reads as the whole run — the verifier would draw conclusions about a first observation that was not the first. - **Clipped where the value is constructed**, not at a downstream consumer. That is the same "structural, not by convention" rule #1932 applied to `reasoning`, and it is why the stored `LadderSnapshot` and `verdict_provenance` are deliberately untouched: the bound is on the *prompt ingress*, not on the record. 24 is sized far above a normal run (a baseline plus a handful of rounds), so the bound only ever bites a pathological loop. It is a named constant next to its rationale rather than a literal. ## The witness - [x] This PR includes a witness test `a_pathological_oracle_trace_is_clipped_with_the_drop_stated` — a 100-observation trace renders exactly 24 entries behind the `…76 earlier observation(s) omitted →` marker, and still ends on the newest observation. `an_ordinary_oracle_trace_renders_unchanged` is the other half, and the one that matters for regression: a 5-observation trace is asserted **byte-identical to `render_oracle_trace`**, main's own unbounded function, which is still present and still used for provenance. So every prompt the bound does not bite is unchanged to the byte — which also means no verdict-reuse digest (#1431) moves for an ordinary run. Honest note on "fails on main": these pin a bound that does not exist on `main`, so the failure there is that `bounded_oracle_trace` is not defined — the same shape as #1932's witnesses for the `reasoning` cap, and the shape any "add a missing ceiling" change has. The behavioural claim is carried by the second test, which compares against main's function directly rather than against a copied expectation. ## The gate - `cargo test -p stella-pipeline` — 585 + 5 + 2 + 5 + 6 + 4 = **607 passed, 0 failed** - `cargo clippy -p stella-pipeline --all-targets` — clean - `cargo fmt --check -p stella-pipeline` — clean - `scripts/check-file-size.sh` — OK, none grew (`evidence.rs` was extracted from `pipeline.rs` precisely so this kind of channel can be added without touching a god file, and that still holds) Full workspace left to CI. ## Nothing left behind `Refs #1787`, deliberately not `Closes` — this is the folded-in bound only. Item 1 (a provider-parity-aware structured verdict output path, invariant 8) remains open and is being approached from a different angle in #1964; item 2 shipped as #1932 and item 3 as #1951. Refs #1787 ## Summary by Sourcery Bound the oracle trace rendered in the verifier prompt to a fixed number of recent observations and document the truncation in-band while preserving full traces in stored provenance. Enhancements: - Introduce a bounded oracle trace renderer for the verifier prompt, limiting the trusted-zone trace to the newest 24 observations and prefixing output with an omission marker when older entries are dropped. Tests: - Add tests ensuring pathological long oracle traces are clipped with an explicit omission notice and that ordinary short traces remain byte-identical to the existing unbounded rendering.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…a dangling boot.rs doc link (#1985) ## What & why `main`'s required `fmt + clippy + test` job has been **red since #1951** (five consecutive merges), so every open PR inherits a red gate. Four distinct errors are involved. This PR fixes the two that **no open PR covers**. ### 1. `cargo clippy -D warnings` — `plan_stage` has 8 arguments (limit 7) `crates/stella-pipeline/src/pipeline.rs`. #1953 added a `research` parameter, pushing `plan_stage` over `clippy::too_many_arguments`. `Spend<'_>` (`crates/stella-pipeline/src/pipeline/stage_budget.rs`) already groups the `budget`/`total` pair, and **seven** sibling stage methods take it; `plan_stage` was the last one carrying the two loose. Adopting it takes the count to 7. This is the right fix rather than `#[allow(clippy::too_many_arguments)]` because there is no argument that the lint is wrong *here* — the grouping type the lint is asking for already exists — and because `pipeline.rs` is a grandfathered god file closed to growth, which this change **shrinks**. ### 2. `cargo doc -D warnings` — unresolved link in `boot.rs` `crates/stella-cli/src/daemon/boot.rs`. #1939 left ``[`SkipReason::NoResumePoint`]`` in the module doc unresolvable (`no item named `SkipReason` in scope`), which fails `rustdoc::broken_intra_doc_links`. Qualified to the full path — the same remedy #1927 applied to *this same file* for *this same reason* after #1920. That recurrence is filed as #1986. ## What this PR deliberately does NOT fix The other two errors belong to the open PR **#1964**: - `flip_halt_arming.rs` references `PassingShell` / `shell_call_result`, test helpers #1945 landed the test file without. - `crates/stella-pipeline/src/management_prompt/tests.rs`'s exhaustive match omits `ModelCallRole::Research`, which #1953 added. Duplicating them here is how two parallel unbreak PRs collide, so they are left to #1964. **`main` needs both PRs.** This one alone leaves `cargo test` red; #1964 alone leaves clippy and rustdoc red (verified: #1964's own CI still fails both, and its clippy failure is the identical `plan_stage` error). Neither is sufficient on its own, and the merge order does not matter. Because of that, **this PR's own `cargo clippy` and `cargo test` steps will stay red until #1964 merges** — clippy `--all-targets` reaches the missing test helpers once the lib error is gone. That is expected, not a regression. ## The witness - [x] No witness test needed — a lint fix and a doc-link fix, neither a behavior change. Verified the artisanal way: - `RUSTDOCFLAGS="-D warnings" cargo doc -p stella-cli --no-deps` fails on `main` and **exits 0** with this change. - `cargo check -p stella-pipeline --all-targets` reports **only** the three #1964-owned errors; the `plan_stage` arg-count error is gone and both call sites (`pipeline/scope_stage.rs`, `pipeline/tests/management_accounting.rs`) compile. - `cargo fmt --check -p stella-pipeline -p stella-cli` exits 0. ## The gate - [x] `check-file-size.sh` and `check-god-files.sh` pass; `pipeline.rs` shrinks, so no baseline change is needed. - [x] No behavior change, no new flags, no docs pages affected. - [x] No new dependencies. ## Nothing left behind - **#1986** — `boot.rs`'s module doc has now broken `main`'s rustdoc twice in two days by the same mechanism (#1920 → #1927, then #1939 → this PR), because `ci.yml` does not run on a push to `main`. Filed as a handoff. - **#1974** — `CandidateState` is hand-built at two sites, which is how #1951's field addition silently broke PR #1962 at merge time. Noticed in the same investigation. Refs #1953, #1939, #1964
1 task
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…hree things and the merge kept both #1985 and #1971/#1995 independently repaired the breaks #1953 left, converged on the same designs, and landed within minutes of each other. Git merged the two additively rather than conflicting, so `main` at e0fbbe0 carries each fix twice and fails `cargo clippy -p stella-pipeline --all-targets -- -D warnings` three ways: 1. `management_prompt/tests.rs` — `ModelCallRole::Research` appears twice in the same or-pattern (`unreachable_patterns`). Kept one. 2. `pipeline/scope_stage.rs` — both PRs bundled `plan_stage`'s budget+total into `Spend`, but the call site kept #1985's per-iteration reborrow AND the other's hoisted `let mut spend`, now unused (`unused_variables` + `unused_mut`). Kept #1985's: the loop replans after a rejected scope card, and only a reborrow per attempt survives that. 3. `tests/verification_hardening.rs` — both restored `PassingShell` and `shell_call_result` after #1951 deleted them, one into this file and one into its `flip_halt_arming` child, leaving the parent's pair dead (`dead_code` ×3, counting `SHELL_TOOL`). For (3) the two copies were not equivalent, so this is not an arbitrary pick: #1985's are better documented — they name `SHELL_TOOL` as a const distinct from `WRITING_TOOL` and say why the `[exit code: 0]` marker is load-bearing (without it the halt never latches and the arming test passes for no reason). Those are the ones kept. They move to the child, which is where both #1793 witnesses now live, because co-location is what makes the next wholesale rewrite of the parent a merge conflict instead of the silent deletion that started this (#1997). The parent's now-stale `mod` doc is corrected in place rather than left describing a layout that no longer holds. `cargo clippy -p stella-pipeline --all-targets -- -D warnings`: clean.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…ngress (#1787) (#2012) > Supersedes #1982, which was auto-closed when the branch it was stacked on > went away. Same single commit, now rebased directly on `main`. > > Note: `main` currently fails `cargo clippy -p stella-pipeline` with five > pre-existing warnings unrelated to this change (fixed in #2009), so this > PR's clippy step inherits them until that lands. ## What & why The last unbounded ingress into the verdict prompt, and the item #1787 folds in at the end of its body: > Also worth folding in: the trusted evidence summary has no length bound > (`oracle_trace` grows per observation; the diff has a token budget, the > trusted zone does not) — `pipeline/evidence.rs` since the extraction. `verifier_evidence_summary` renders `oracle_trace` in full. That trace gains an observation per verification round, and the repair gate (#1479) keeps granting rounds for as long as a measured budget affords them — so the one channel that grows without limit was also the one channel with no ceiling. Every other input to that prompt is bounded: the diff has a token budget, recall frames have `bound_recalled_frames`, and `Verdict::reasoning` got its cap in #1932. Bounded to the newest 24 observations, with the drop **stated in-band**: ``` oracle_trace=[…76 earlier observation(s) omitted → candidate:pass → candidate:fail → …] ``` Three choices worth naming, because each has a wrong-looking alternative: - **Newest kept, oldest dropped.** The recent runs are what the verdict weighs; a trace clipped from the front would hand the verifier a history that stops before the evidence. - **Stated, not silent.** A trace that silently began mid-run reads as the whole run — the verifier would draw conclusions about a first observation that was not the first. - **Clipped where the value is constructed**, not at a downstream consumer. That is the same "structural, not by convention" rule #1932 applied to `reasoning`, and it is why the stored `LadderSnapshot` and `verdict_provenance` are deliberately untouched: the bound is on the *prompt ingress*, not on the record. 24 is sized far above a normal run (a baseline plus a handful of rounds), so the bound only ever bites a pathological loop. It is a named constant next to its rationale rather than a literal. ## The witness - [x] This PR includes a witness test `a_pathological_oracle_trace_is_clipped_with_the_drop_stated` — a 100-observation trace renders exactly 24 entries behind the `…76 earlier observation(s) omitted →` marker, and still ends on the newest observation. `an_ordinary_oracle_trace_renders_unchanged` is the other half, and the one that matters for regression: a 5-observation trace is asserted **byte-identical to `render_oracle_trace`**, main's own unbounded function, which is still present and still used for provenance. So every prompt the bound does not bite is unchanged to the byte — which also means no verdict-reuse digest (#1431) moves for an ordinary run. Honest note on "fails on main": these pin a bound that does not exist on `main`, so the failure there is that `bounded_oracle_trace` is not defined — the same shape as #1932's witnesses for the `reasoning` cap, and the shape any "add a missing ceiling" change has. The behavioural claim is carried by the second test, which compares against main's function directly rather than against a copied expectation. ## The gate - `cargo test -p stella-pipeline` — 585 + 5 + 2 + 5 + 6 + 4 = **607 passed, 0 failed** - `cargo clippy -p stella-pipeline --all-targets` — clean - `cargo fmt --check -p stella-pipeline` — clean - `scripts/check-file-size.sh` — OK, none grew (`evidence.rs` was extracted from `pipeline.rs` precisely so this kind of channel can be added without touching a god file, and that still holds) Full workspace left to CI. ## Nothing left behind `Refs #1787`, deliberately not `Closes` — this is the folded-in bound only. Item 1 (a provider-parity-aware structured verdict output path, invariant 8) remains open and is being approached from a different angle in #1964; item 2 shipped as #1932 and item 3 as #1951. Refs #1787 ## Summary by Sourcery Bound the oracle trace rendered in verifier evidence summaries and added tests to cover the new trusted-zone length cap. New Features: - Introduce a bounded oracle trace renderer for verifier prompts that limits the trusted evidence summary to the newest observations while indicating omissions in-band. Tests: - Add witness tests ensuring long oracle traces are clipped with an explicit omission marker and that ordinary short traces remain byte-identical to the unbounded renderer.
9 tasks
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…#1976) (#2036) ## What & why Three times a PR has landed on `main` that silently deleted code another PR added to the same file hours earlier, and CI could not see it. The most recent (fixed in #1975): #1951 rewrote `crates/stella-pipeline/src/pipeline/tests/verification_hardening.rs` from a pre-#1945 base, deleting the `PassingShell` double, `shell_call_result`, and the witness `a_revision_halts_at_the_step_where_the_tracked_test_flips` that #1945 had added hours earlier. It was caught **only by luck**: #1945 had also added a `mod flip_halt_arming;` line that survived the rewrite and referenced two of the deleted symbols, so `main` went red. Had #1945 added only the test and its doubles — no new module — the deletion would have compiled clean and silently removed a witness from the tree. *A witness that no longer exists cannot fail*, so nothing downstream would ever have reported it. Same shape on record for #1860 reverting #1836's forwarding in four crates. Both PRs are green against the `main` they branched from, and the merge is textually clean: git has no conflict to report, because one side simply does not contain the other's lines. ### The design decision that matters `scripts/check-deleted-tests.sh` compares the **base branch tip** against the **merge result** — never the PR's branch point. That pair is the whole guard. Comparing against the branch point would miss exactly this defect: a test added to `main` *after* the PR branched is absent from the branch point too, so its disappearance would look like nothing at all. Comparing main's tip against the merged tree asks the question that matters — "did everything main had survive this merge?" — and is quiet on a merely stale branch, because git merges main's own additions in unless the PR's side actively removed them. On a `pull_request` event the checkout is `refs/pull/N/merge`, so `HEAD^1` *is* the base branch tip. The guard needs no PR metadata, and because it compares two **trees** rather than two histories, `fetch-depth: 2` is sufficient — no full-depth clone. ### It asks for an acknowledgement, it does not forbid deletion A removed test is not automatically wrong — renames, folding into a table-driven case, and deliberate removal with the feature covered are all ordinary. So a removal fails **only while unnamed**: writing the test's name in the PR description (or a commit message) passes it. That mechanism is deliberately weak. The goal is not to adjudicate whether a deletion was correct — a script cannot — but to convert an invisible deletion into a sentence a reviewer reads. ### Deliberately NOT a `GATE_STEPS` entry It is the one question here about *two* trees, and a local `make gate` has no second tree to compare, so there is nothing for it to do there. This also means the five-edit gate-parity dance does not apply; `check-gate-parity` still reports 25 steps, unchanged. Closes #1976 ## The witness - [x] This PR includes a witness test (fails on `main`, passes here), **or** - [ ] No witness needed The witness is reproduced from **real history**, not a synthetic fixture — `eddf9700` is #1945 (added the witness), `2a142b26` is #1951 (deleted it): ``` $ PR_BODY="" ./scripts/check-deleted-tests.sh eddf970 2a142b2 check-deleted-tests: FAILED These tests exist in eddf970 but not in the merged tree, and nothing in the PR description or the branch's commit messages names them: a_revision_halts_at_the_step_where_the_tracked_test_flips ``` It names exactly the test that was really lost. The other three required behaviours, all verified: ``` $ PR_BODY="Folded a_revision_halts_at_the_step_where_the_tracked_test_flips into …" \ ./scripts/check-deleted-tests.sh eddf970 2a142b2 check-deleted-tests: OK — 1 removed test(s), each named in the PR description or a commit. $ ./scripts/check-deleted-tests.sh 1feb029 eddf970 # a range that only ADDS tests check-deleted-tests: OK — 6790 test(s) in 1feb029, none lost by the merge. $ ./scripts/check-deleted-tests.sh origin/main HEAD # this very branch check-deleted-tests: OK — 6865 test(s) in origin/main, none lost by the merge. ``` A genuine rename is reported too (the old name is gone), which is intended — naming it in the PR is the whole cost. ## The gate - [x] `shellcheck` — clean - [x] `make guards-fast` — all green, including `check-gate-parity` (25 steps, unchanged) and `check-action-pins` - [x] Docs updated: AGENTS.md § witness tests gains the paragraph, and the CI description now lists the guard among what `ci.yml` adds beyond the gate - [x] `Closes #1976` appears both here and as a commit trailer No Rust changed, so fmt/clippy/test are untouched by this PR. ## Nothing left behind - [x] Filed: see below Two limitations are **measured and documented in the script header** rather than left implicit: 1. **The key is the bare test name, unqualified by file or module.** That makes a test *moved* between modules silently fine (the common legitimate case), at the cost that a duplicated name masks a deletion. Measured: 51 of 6867 distinct test names (0.74%) are duplicated today, nearly all the per-adapter provider suites where one contract is asserted against each vendor under one name. The house style of long sentence-shaped test names is what makes the unqualified key work — it is not a general assumption. 2. **`#[test]` inside a multi-line string fixture is counted.** Two lines in the tree (`witness/density.rs`, `candidate_ws/witness_tools.rs` — code that analyses test code) put `#[test]` at the start of a continuation line inside a string literal. It is symmetric noise, so a difference detector cancels it; it could only false-positive if such a fixture were edited, which lands in the acknowledge path by design. ## Ground-rule check - [x] No I/O added to `stella-core`; no new deps - [x] No new outbound network calls ## Anything reviewers should know? `fetch-depth: 2` on the `check` job's checkout is the one change that touches every CI run. It is one extra commit, not a full clone — deliberately the smallest thing that makes the merge commit's first parent readable. The guard runs on `pull_request` only: on a squash-merged push to `main` there is no merge commit to inspect, and the report would arrive too late to act on anyway. The script self-skips on a non-merge `HEAD` with no explicit base, so running it by hand needs a base ref: `./scripts/check-deleted-tests.sh origin/main`. ## Summary by Sourcery Add a CI guard that detects tests removed by a PR’s merge unless their deletion is explicitly acknowledged, and document this behaviour in agent guidance. CI: - Update the main CI workflow to fetch two commits for pull_request checkouts and run a deleted-test guard that compares the base branch tip to the merge result, passing only when removed tests are named in the PR body. Documentation: - Extend AGENTS.md to describe the deleted-test guard, its scope, and its requirement to mention removed tests in PR descriptions. Chores: - Add the scripts/check-deleted-tests.sh utility to scan test trees across two revisions and enforce acknowledgement of removed tests.
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.
What & why
Item 3 of #1787: verifier degradation was surfaced by a once-per-run
AtomicBoolwarning, so a best-of-N fan-out whose verifier died (or answered off-protocol) reported one caveat for N candidates and recorded nothing about which candidates were judged by the deterministic heuristic.Each candidate now emits one structured
ProofStep::VerdictDegraded { candidate, reason }fact — 1-based ordinal,Oracle::run's convention — the first time its verdict degrades. Dedup state is candidate-local (VerdictDegradationridesCandidateState), never a shared flag reset at candidate boundaries: candidates run concurrently (fanout_concurrency.rs), and a run-wide reset would race nondeterministically. The once-per-run transcript warning is unchanged; the traces view renders the new step (verdict degraded (candidate N): reason), and the replay fold keys it asproof:verdict_degraded:{candidate}.Two file-size-driven structural moves, both following existing exemplars:
ProofStepmoves fromevent.rs(at its ceiling) to a newstella-protocol/src/proof.rs, the exactladder.rspattern — re-exported fromeventand the crate root sostella_protocol::ProofStepandevent::ProofStepnever moved.docs/wire/regenerated.pipeline.rs(also at its ceiling) stays under it by consolidating the two once-per-runAtomicBoolnotices intoVerifierNoticesinverifier_stage.rs, andverifier()now takes the crate's ownSpendenvelope instead of loosebudget/total(thetask_frame.rsidiom).CandidateSlotinfanout_stage.rsgroups a candidate's ordinal with its workspace so the fan-out driver stays underclippy::too_many_argumentsstructurally rather than by#[allow].Refs #1787 — item 1 (provider-parity-aware structured verdict output) remains open; item 2 shipped as #1932.
The witness
main, passes here)a_two_candidate_fanout_records_which_candidates_degraded— the issue's own Verify: two isolated candidates with a tokenless verifier record facts for ordinals 1 and 2 beside exactly one transcript warning; every post-triage scripted reply is tokenless so the assertion holds under any completion order.a_candidate_degrading_on_every_round_records_one_factpins per-candidate (not per-round) emission, with the provider's script consumption asserted so the second escalation provably happened. Both nameProofStep::VerdictDegraded, which does not exist onmain. Plus a wire round-trip pinning the serialized shape (proof.rs) and the variant added towire_contract.rs's exhaustive list.The gate
cargo fmt --checkcargo clippy --workspace --all-targets -- -D warningscargo test -p stella-protocol -p stella-pipeline -p stella-tui(all green; guards-fast, file-size, god-files, invariants, module-reachability, wire-schema all green — CI runs the full workspace)Refs #1787above and as a commit trailer — deliberately notCloses: item 1 stays openNothing left behind
Item 1 of #1787 (structured verdict output with a declared per-provider posture) is untouched and tracked by the still-open #1787.
Summary by Sourcery
Record verifier verdict degradation per candidate in fan-out runs while refactoring proof and verifier wiring to keep file sizes and interfaces within project constraints.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: