Repository navigation
Stop reconstructing a budget refusal from the prose it just printed - #7476
Conversation
`std.observation` models `TimedOut { budget, elapsed }`, but nothing could ever
emit one: both budget arms held the typed `{elapsed_ms, budget_ms}` pair and
immediately flattened it with `format!` into `ClaimOutcome::RuntimeError`. The
floor then recovered the classification by SUBSTRING-MATCHING the very message
the seed had produced —
falsifier_failure_mode: d.contains("eval budget exceeded")
— one fact in two representations, the second guessed back from the first. Its
fallback arm is `WitnessRed`, so rewording an error message silently demoted a
budget refusal to a witness failure: two states whose remedies differ (re-basis a
dated ceiling vs. fix the witness).
`ClaimOutcome::TimedOut { elapsed_ms, budget_ms, kind }` keeps the pair as data.
`BudgetKind` keeps the two clocks apart — `Cpu` is thread CPU (the stride-poll
metric, so a witness slowed by cold I/O is not misclassified), `Wall` is
whole-receipt time counting subprocess I/O. Collapsing them would relocate the
state-space conflation rather than end it. `ClaimResult.budget_refusal` carries
it to the receipt (`ok`/`detail` are a lossy flattening, which is why the mode
had to be sniffed from prose), and `witness_cost_seed_timed_out_event` projects a
real `ObservationOutcome::TimedOut`.
WHY THIS MATTERS BEYOND TIDINESS: a killed row's recorded wall is its CEILING,
not its cost. `resolution_divergence_silent_pick_gate_keystone_holds` is killed
at 900000ms and records a flat 900794ms — a censored measurement wearing the
shape of a completed one. That is why the witness row-cost basis cannot seed the
corpus's most expensive row: under the 2x comparator a basis of 900794 pins it
`WithinBasis` below ~1.8M ms forever, enshrining a deadline ceiling as normal
cost. This is the prerequisite for seeding that row honestly.
NOT DONE HERE, stated rather than hidden: `BudgetKind` is not a field on
`ObservationOutcome.TimedOut`, which carries budget and elapsed only. Extending
that sum is a change to load-bearing substrate (`std.observation`) this lane has
no mandate for, so the clock is surfaced in the seed-side detail and the batch
failure mode instead. An ObservationEvent alone does not say which clock expired.
Dissolve-on: a consumer needing the clock from the event is the argument for
taking the field to `std.observation`, not for widening it here.
VERIFIED BY EXECUTION:
- `budget_kill_classifies_structurally_not_by_message_text` — RED control: the
detail contains NONE of the substrings the string classifier looks for, and
the test first asserts `falsifier_failure_mode` really would return
`WitnessRed` for it, so the control is not vacuous; the batch still reports
`BudgetExceeded`, and a plain witness red still reports `WitnessRed`.
- `cpu_and_wall_kills_do_not_collapse_into_one_state` — identical numbers on
different clocks must not be the same value.
- `.dag`: two new witnesses in witness_row_cost_projection_witness_test, green
by execution, each proven discriminating by perturbation — a wrong budget
value goes RED, and treating the TimedOut arm as anything else goes RED,
with the restored file PASS.
- claim_executor suite: 36 passed, 0 failed. All three bins build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…misclassified
`review 45220` caught a real regression I introduced, on the exact path this
change exists to fix. Both findings verified against the code and fixed.
FINDING 1 — discovery batches missed the structural path. `discovery_claim_result`
hardcoded `budget_refusal: None`, so a discovery batch never carried the typed
refusal even when `summary.witness_outcomes` held a `ClaimOutcome::TimedOut`.
Combined with this PR's new detail wording — which deliberately contains none of
`falsifier_failure_mode`'s substrings — a budget kill on the discovery path fell
through to `WitnessRed`. That is the same §5 misclassification the PR removes,
reintroduced in new prose, and it landed on the primary path:
`resolution_divergence_silent_pick_gate_keystone_holds` IS a discovery row, so
the incident the PR cites would have stayed misclassified.
The fact was available and I failed to read it — `witness_outcomes` carries the
typed per-witness `ClaimOutcome`. `discovery_budget_refusal` lifts the first kill
out of it. BOTH arms are fixed, not just the one flagged: the `Err` arm (receipt
projection refused) had the same hole, and its own comment already states the
rule — a receipt refusal ADDS a cause, it never erases one already established.
A batch that blew its budget and then failed to project its receipt is still a
budget kill.
FINDING 2 — `TimedOut` was missing from the `std.observation` import list in
`gunbc.witness_row_cost` (only `Done`/`Refused`/`Failed` were imported). It
compiled and executed green anyway via bare-reference resolution, which is
exactly the accidental-coverage fragility the import-strip cascade thread
documents: green by pool-membership coincidence, not by binding. Imported
explicitly, matching every other module that builds this outcome.
VERIFIED BY EXECUTION:
- `discovery_budget_kill_classifies_structurally_on_the_falsifier_path` — a RED
control proven discriminating: with the fix reverted it FAILS on "discovery
batch must lift the budget kill out of witness_outcomes"; restored it passes.
It first asserts `falsifier_failure_mode` really would return `WitnessRed` for
the new detail text, so the control is not vacuous, exercises BOTH projection
arms (Ok and receipt-refused), and asserts an ordinary discovery red is NOT
dragged into `BudgetExceeded`.
- `.dag` witnesses still green by execution with the explicit import.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Finding 1 — discovery batches missed the structural pathConfirmed exactly as described. The compounding part is what makes this bad rather than merely incomplete: this PR replaced the old budget prose, so the new detail ( The fact was available and I simply failed to read it — I fixed both arms, not only the one flagged. The Finding 2 — missing
|
| state | result |
|---|---|
fix reverted (budget_refusal: None) |
FAILED — "discovery batch must lift the budget kill out of witness_outcomes" |
| fix restored | ok |
It also asserts up front that falsifier_failure_mode really would return WitnessRed for the new detail text, so the control cannot pass vacuously; exercises both projection arms (Ok and receipt-refused); and asserts an ordinary discovery red is not dragged into BudgetExceeded. .dag witnesses remain green by execution with the explicit import.
— sent from quiet-ferret-16
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
I considered arguing the grain — DESIGN §7's frontier is per-module ("each module is either self-emitted or seed-retained"), and What landed
On the dissolve-on gap you identifiedThat was the sharpest part of the finding and I'd got it wrong. Module green by execution with the note added; full — sent from quiet-ferret-16 |
`review 45231` is correct: five exhaustive matches on ClaimOutcome in
src/v1/tests/ had no arm for the new variant, so the v1-compiler-tests crate
failed to compile with E0004.
VERIFICATION GAP THAT LET THIS THROUGH, recorded because the gap is the real
finding: I built the three bins and ran the claim_executor bin suite, and treated
that as coverage. It was not — v1-compiler-tests is a separate crate that neither
`cargo build --bin` nor `cargo test --bin` compiles. The workspace test build
that WOULD have caught it is itself blocked by a pre-existing E0432 in
v1-stage0-std-core (confirmed on main with my diff stashed), and I let that
known-red stand in for "cannot check tests here" instead of targeting the crate
directly with `-p v1-compiler-tests`, which works fine.
The five arms render the budget kill distinctly from RUNTIMEERR — a budget kill
is not a runtime fault — and the four label-producing files print both numbers so
a caller comparing labels cannot mistake the elapsed ceiling for a completed
duration.
PROVEN DISCRIMINATING, not merely green: with one arm removed the crate fails
with `E0004: non-exhaustive patterns: &ClaimOutcome::TimedOut { .. } not
covered` at union_resolve_receipts_test.rs:100; restored, it Finishes.
Note these targets do not run in CI (CI runs no cargo test), so nothing would
have redded on merge — the breakage would have surfaced as a local dev build
failure for whoever touched them next. That makes the reviewer's catch the only
gate that existed here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The verification gap, which is the more useful findingI built the three bins and ran the Worse, I had already seen them. They appeared in my very first The workspace test build that would have caught this is blocked by a pre-existing The fixThe budget kill renders distinctly from Proven discriminating rather than merely green:
One thing worth flagging about the gateThese targets do not run in CI — CI runs no — sent from quiet-ferret-16 |
…ng produces (#7482) * Read the walk's own infra fault as data, and drop a matcher arm nothing produces Sibling of #7476, found by looking for the same shape elsewhere: a typed fact formatted into prose and then recovered by grepping that prose. THE ROUND-TRIP. `handle.join()` returning `Err(_)` means a claim thread panicked — the walk KNOWS this. That bool became `format!("batch=N infra=thread_panic")`, and `falsifier_failure_mode` recovered it with `d.contains("infra=")`. The panic PAYLOAD is genuinely lost (`Err(_)` discards it), but *that a thread panicked* is not, so it now travels as `InfraFault::ClaimThreadPanicked { batch_index }` and `falsifier_failure_mode_with_faults` consults it before any text path. The rendered line goes back to being for humans. A MATCHER ARM THAT COULD NEVER FIRE. `d.contains("failed to spawn")` is deleted: no producer reachable from this input emits it. The interpreter's spawn failure says "failed to execute '{argv0}': {e}"; the in-tree producers of "failed to spawn" are a different bin, test helpers, and panic messages — and a panic payload is discarded before it could become a detail. A substring its own input cannot contain only looks like coverage. KEPT AS TEXT, DELIBERATELY. "Resource temporarily unavailable" and "sccache" are genuinely EXTERNAL text arriving as an io::Error Display through `failed to execute '{argv0}': {e}` (EAGAIN renders as the former). Matching them here is not principled, and the comment says so: typing them belongs at the interpreter boundary where the io::Error is caught. Not silently left as if it were the same kind of thing as the fact above. ALSO FIXES A LIVE MAIN RED, unrelated to the above but caught by it. `cargo test --bin claim_executor` does not compile on clean main: #7470 added the `finalization_record` helper with a `ClaimResult` literal while #7476 added the required `budget_refusal` field. Both merged, git saw no textual conflict, and CI runs no cargo test, so nothing caught it. Verified by stashing this branch's diff and building unmodified origin/main (E0063 at claim_executor.rs:4526). My field made it required, so the one-line fix rides here. VERIFIED BY EXECUTION, both crates this time — the omission that let review 45231 find a compile break last round: - claim_executor bin suite: 40 passed, 0 failed. - `cargo build --tests -p v1-compiler-tests`: Finished. - `falsifier_failure_mode_classifies_three_arms` now asserts BOTH directions, so the move from text to value is proven, not assumed: the panic text ALONE classifies WitnessRed, an observed fault classifies Infra even when no text hints at it, and no fault means no Infra whatever the prose says. - Proven discriminating: with the typed arm neutered the test FAILS; restored it passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * WIP: affected set is red again --------- Co-authored-by: Brian Searls <briansearls1@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
std.observationmodelsTimedOut { budget, elapsed }— but nothing could ever emit one.Both budget arms held the typed
{elapsed_ms, budget_ms}pair and immediately flattened it withformat!intoClaimOutcome::RuntimeError. The floor then recovered the classification by substring-matching the very message the seed had just printed:One fact in two representations, the second guessed back from the first. And the classifier's fallback arm is
WitnessRed— so rewording an error message silently demoted a budget refusal to a witness failure, two states whose remedies differ (re-basis a dated ceiling vs. fix the witness).The change
ClaimOutcome::TimedOut { elapsed_ms, budget_ms, kind }keeps the pair as data.BudgetKindkeeps the two clocks apart.Cpuis thread CPU time (the stride-poll metric, so a witness slowed by cold I/O or governor time-slicing isn't misclassified);Wallis whole-receipt time, counting subprocess I/O. Collapsing them would relocate the state-space conflation rather than end it.ClaimResult.budget_refusalcarries the fact to the receipt —ok/detailare a lossy flattening ofClaimOutcome, which is precisely why the mode had to be sniffed out of prose.witness_cost_seed_timed_out_eventthen projects a realObservationOutcome::TimedOut.Why this matters beyond tidiness
A killed row's recorded wall is its ceiling, not its cost.
resolution_divergence_silent_pick_gate_keystone_holdsis killed at 900000ms and records a flat900794ms— a censored measurement wearing the shape of a completed one.That is why the witness row-cost basis (#7475) cannot seed the corpus's most expensive row: under the 2× comparator a basis of 900794 pins it
WithinBasisbelow ~1.8M ms forever, enshrining a deadline ceiling as its normal cost. This PR is the prerequisite for seeding that row honestly.Not done here, stated rather than hidden
BudgetKindis not a field onObservationOutcome.TimedOut, which carriesbudgetandelapsedonly. Extending that sum changes load-bearing substrate (std.observation) that this lane has no mandate for, so the clock is surfaced in the seed-side detail and the batch failure mode instead.The consequence, stated plainly: an
ObservationEventalone does not say which clock expired. Dissolve-on — a consumer that needs the clock from the event itself is the argument for taking the field tostd.observation, not for widening it here.Verified by execution
Rust
budget_kill_classifies_structurally_not_by_message_text— a RED control whose detail string contains none of the substrings the string classifier looks for. It first assertsfalsifier_failure_modereally would returnWitnessRedfor that text, so the control isn't vacuous; the batch nonetheless reportsBudgetExceeded, and a plain witness red still reportsWitnessRed.cpu_and_wall_kills_do_not_collapse_into_one_state— identical numbers on different clocks must not be equal values.claim_executorsuite: 36 passed, 0 failed. All three bins build..dag— two new witnesses inwitness_row_cost_projection_witness_test.dag, green by execution and each proven discriminating by perturbation:900000→999999)TimedOutarm treated as anything elsePure Bootstrap receipt (
review 45226)witness_cost_timed_out_seed_deferral_noterecords the hand-Rust deferral DESIGN §7 requires, matching the sibling shape ingunbc.floor_component_receipt:ClaimOutcomesum in the already-seed-retainedcli_runmodule, theBudgetKindlabels, theBudgetRefusalprojection field, and the discovery lift. No new Rust file, no parallel authority.format!-into-RuntimeErrorsites andfalsifier_failure_mode's substring recovery. The seed no longer decides what a budget message means by reading its own prose — so this is net-negative on the seed's judgment surface..dag: the outcome vocabulary, the receipt shape, and theMillisecondcarrier, which the seed obtains by callingmillisecond(count:)through the interpreter rather than constructing.ZeroHandMaintainedRust, roadmap nodev1-zero-hand-maintained-rust(gunbc.v1_deletion_planv1_exit_finish_lines) — verified present..daginstead of interpreted in the seed bin (the witness-realization lane), at which point the deadline is enforced by the realized runner and the event is constructed directly.The review correctly noted the existing dissolve-on did not cover the Rust realization: that trigger concerns widening
std.observation.ObservationOutcomewith the clock kind, this one deletes the seed transport, and neither implies the other. Both are now recorded separately.Full
claim_executorsuite after all review fixes: 37 passed, 0 failed.