diff --git a/crates/bench-runner/src/service.rs b/crates/bench-runner/src/service.rs index cae2e57..06ec4a9 100644 --- a/crates/bench-runner/src/service.rs +++ b/crates/bench-runner/src/service.rs @@ -189,6 +189,15 @@ fn one_request(port: u16) -> std::io::Result<()> { /// One open-loop load session against a freshly spawned server. pub fn run_session(cfg: &ServiceCfg, port: u16) -> Result { + ensure!( + cfg.rate.is_finite() && cfg.rate > 0.0, + "arrival rate must be finite and positive (got {})", + cfg.rate + ); + ensure!( + cfg.count > 0 && cfg.workers > 0, + "count and workers must be > 0" + ); let mut child = spawn_server(cfg, port)?; // Warm up closed-loop (untimed) so the first timed request doesn't eat // page faults / cold caches. diff --git a/crates/ledger/src/lib.rs b/crates/ledger/src/lib.rs index 3e0793e..4b8fd8c 100644 --- a/crates/ledger/src/lib.rs +++ b/crates/ledger/src/lib.rs @@ -217,12 +217,30 @@ impl Ledger { .and_then(|b| b.get("env_fingerprint")) .and_then(|f| f.get("workload")) .cloned(); + // Pull the sanitizer gate out so the report can refuse to + // mint a clean ROI for an `accepted` row that predates the + // machine-enforced sanitizer gate and was overturned by + // audit (phase2-comrak-010). The gates JSON is stored + // separately; read it here. + let sanitizers_clean = self + .conn + .query_row( + "SELECT gates FROM attempts WHERE run_id = ?1", + [run_id], + |r| r.get::<_, Option>(0), + ) + .ok() + .flatten() + .and_then(|g| serde_json::from_str::(&g).ok()) + .and_then(|g| g.get("sanitizers_clean").and_then(|v| v.as_bool())) + .unwrap_or(false); Ok(Some(serde_json::json!({ "run_id": run_id, "target": target, "playbook_class": class, "hypothesis": hypothesis, "verdict": verdict, + "sanitizers_clean": sanitizers_clean, "timestamp": ts, "speedup_median": bench.as_ref().and_then(|b| b.get("speedup_median").cloned()), "speedup_ci": bench.as_ref().and_then(|b| b.get("speedup_ci").cloned()), diff --git a/crates/report/src/main.rs b/crates/report/src/main.rs index fc4a023..c22e6dc 100644 --- a/crates/report/src/main.rs +++ b/crates/report/src/main.rs @@ -69,14 +69,27 @@ fn main() -> Result<()> { "**Target:** {target} · **Playbook class:** {class} · **Verdict:** `{verdict}`\n\n\ **Workload:** {workload}\n\n", )); - // A report for anything the pipeline did not accept must say so before - // any dollar figure — the ROI below is a measured effect, not a - // shippable saving. - if verdict != "accepted" { + // A report for anything the pipeline did not accept — OR an `accepted` + // row that is not sanitizer-clean (a pre-sanitizer-gate accept the + // audit overturned, e.g. phase2-comrak-010) — must say so before any + // dollar figure. The ROI below is a measured effect, not a shippable + // saving. This closes the hole where the immutable ledger's historical + // accept could mint a clean ROI for a patch that was never shipped. + let sanitizers_clean = row["sanitizers_clean"].as_bool().unwrap_or(false); + let shippable = verdict == "accepted" && sanitizers_clean; + if !shippable { + let reason = if verdict != "accepted" { + format!("its verdict is `{verdict}`, not `accepted`") + } else { + "it is `accepted` but NOT sanitizer-clean — this is a pre-sanitizer-gate accept that \ + the audit overturned (the immutable ledger preserves the original verdict; \ + `sanitizers_clean:false` is the disqualifying signal)" + .to_string() + }; md.push_str(&format!( - "> ⚠️ **Not shipped.** This attempt's verdict is `{verdict}`, not `accepted`. Any ROI\n\ - > below is the *measured* effect — it did not clear the significance/tier bar and is\n\ - > **not** a committed saving. Shown for completeness; the ledger records it as-is.\n\n", + "> ⚠️ **Not shipped.** This attempt is not a committed saving because {reason}. Any ROI\n\ + > below is the *measured* effect only; it did not clear the significance/tier/sanitizer\n\ + > bar. Shown for completeness; the ledger records it as-is.\n\n", )); } diff --git a/results/adversarial-review.md b/results/adversarial-review.md new file mode 100644 index 0000000..2a510a6 --- /dev/null +++ b/results/adversarial-review.md @@ -0,0 +1,59 @@ +# Adversarial review — whole package + +A hostile pass over the entire project: every headline claim re-checked +against the ledger, the trust boundary attacked directly, the newest +code scrutinized hardest, and the statistics re-validated. Two real +defects found and fixed; the load-bearing guarantees held. + +## Findings + +### 1. FIXED — the ROI generator minted a clean pitch for an overturned patch +The report generator trusted the ledger's `verdict` field. `phase2-comrak-010` +is immutably `accepted` (it predates the machine-enforced sanitizer gate; +the audit overturned it for a LeakSanitizer leak and it was never banked). +`report --run-id phase2-comrak-010` therefore produced a clean +**$16,886/year** ROI report for a memory-leaking patch — exactly the kind +of self-contradiction a hostile reviewer would seize on. + +**Fix:** the report now treats an `accepted` row that is not +`sanitizers_clean` as **not shippable** and prints a "Not shipped" warning +before any dollar figure. Because the pipeline now enforces ASan/UBSan on +every accept, all legitimate wins are `sanitizers_clean: true`; only the +historical overturned row is `accepted + false`, so the rule cleanly +separates them. Regression-checked: `phase3-cjson-002` still renders clean. + +### 2. FIXED — load generator could panic on degenerate inputs +`bench-runner service --rate 0` (or `--count 0`) reached +`Duration::from_secs_f64(1.0/0.0)` → panic. **Fix:** `run_session` now +rejects a non-finite/non-positive rate and zero count/workers with a clear +error. + +## Attacks that were correctly refused (no change needed) + +- **Ledger append-only:** `UPDATE`/`DELETE` on a copy both refused by the + SQLite triggers. +- **Patch boundary:** `propose_patch` refused `../` traversal and `.git` + access; `read_target_source` refused traversal. A patch can only touch + `targets//workspace`. +- **Accepted-row invariant:** all 10 `accepted` rows have green upstream + tests + golden replay and a bootstrap CI lower bound ≥ 1.02. +- **Statistics:** every batch and service A/A calibration is a null result + (0 false positives); every injected-5% regression is detected (≥95%). + The service load generator is coordinated-omission-correct and its + injection test resolves a known 5% latency regression 10/10. + +## Verification run + +`cargo test --workspace` (all green), `clippy -D warnings` (clean), +`cargo fmt --check` (clean). Every case-study and CLAUDE.md figure +re-checked against `results/ledger.sqlite` and the calibrated bench — +consistent, with the compound tokei figure stated as an approximation +(CIs don't multiply) and p99 service latency explicitly not claimed. + +## Standing (not defects) + +- `phase2-comrak-010` remains `accepted` in the ledger by design — the + ledger is append-only and records what happened; the overturn lives in + the case study and is now enforced by the report generator too. +- `phase0-comrak-002` (mimalloc) is `needs-human-review`, awaiting a ruling + (see `results/rulings/`).