From 377031bc17b9036a335caef20c26409547726124 Mon Sep 17 00:00:00 2001 From: rldyourmnd Date: Sat, 26 Sep 2026 18:50:20 +0500 Subject: [PATCH] fix(bench): refuse incomparable or insufficient suite reports The comparator only flagged missing scenarios and numeric drift; absent metrics, nonfinite values, failed scenarios, thin samples and backend/impairment profile mismatches compared silently. Each is now a fault:* refusal resolved before numbers are weighed, and --min-samples bounds percentile claims. The c1 checkpoint's noq suite gains the transport-noq feature flag it always required, and the r0-evidence gate is registered with a CLI negative-fixture battery. W0.4 partial. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- crates/rds-bench/src/main.rs | 22 ++- crates/rds-bench/src/report.rs | 250 ++++++++++++++++++++++++++++++--- docs/remediation-progress.md | 31 ++++ scripts/checkpoint.sh | 61 +++++++- 4 files changed, 336 insertions(+), 28 deletions(-) diff --git a/crates/rds-bench/src/main.rs b/crates/rds-bench/src/main.rs index 7a17452..e360369 100644 --- a/crates/rds-bench/src/main.rs +++ b/crates/rds-bench/src/main.rs @@ -8,11 +8,17 @@ use clap::{Parser, Subcommand}; use rds_bench::impair::Impairment; use rds_bench::report::{BenchSuite, CompareOpts, compare, unix_ts}; -fn compare_opts(tol: f64, latency_floor_ms: f64, throughput_floor: f64) -> CompareOpts { +fn compare_opts( + tol: f64, + latency_floor_ms: f64, + throughput_floor: f64, + min_samples: usize, +) -> CompareOpts { CompareOpts { tol, latency_floor_ms, throughput_floor, + min_samples, ..CompareOpts::default() } } @@ -78,7 +84,7 @@ enum Cmd { #[arg(long)] md: Option, }, - /// Compare two suite JSONs; exit 1 if drift exceeds tolerance. + /// Compare two suite JSONs; exit 1 on drift or a comparability fault. Compare { a: PathBuf, b: PathBuf, @@ -91,6 +97,9 @@ enum Cmd { /// Absolute throughput floor in MiB/s. #[arg(long, default_value_t = 3.0)] throughput_floor: f64, + /// Minimum samples per side before percentile claims count. + #[arg(long, default_value_t = 3)] + min_samples: usize, }, } @@ -187,6 +196,7 @@ async fn run(cli: Cli) -> anyhow::Result<()> { tol, latency_floor_ms, throughput_floor, + min_samples, } => { let load = |p: &PathBuf| -> anyhow::Result { let text = std::fs::read_to_string(p).with_context(|| format!("read {p:?}"))?; @@ -194,7 +204,7 @@ async fn run(cli: Cli) -> anyhow::Result<()> { }; let a = load(&a)?; let b = load(&b)?; - let opts = compare_opts(tol, latency_floor_ms, throughput_floor); + let opts = compare_opts(tol, latency_floor_ms, throughput_floor, min_samples); let drift = compare(&a, &b, &opts); if drift.is_empty() { println!( @@ -206,8 +216,10 @@ async fn run(cli: Cli) -> anyhow::Result<()> { return Ok(()); } for d in &drift { - if d.ratio.is_nan() { + if d.metric == "scenario" { println!("MISSING scenario in second suite: {}", d.scenario); + } else if d.ratio.is_nan() { + println!("FAULT {}: {}", d.scenario, d.metric); } else { println!( "DRIFT {} {}: {:.2} → {:.2} (×{:.2})", @@ -215,7 +227,7 @@ async fn run(cli: Cli) -> anyhow::Result<()> { ); } } - anyhow::bail!("{} drift(s) beyond tolerance", drift.len()) + anyhow::bail!("{} drift(s)/fault(s)", drift.len()) } } } diff --git a/crates/rds-bench/src/report.rs b/crates/rds-bench/src/report.rs index 3899665..7678042 100644 --- a/crates/rds-bench/src/report.rs +++ b/crates/rds-bench/src/report.rs @@ -146,14 +146,17 @@ impl BenchSuite { } } -/// One metric that drifted beyond tolerance between two suite runs. +/// One metric that drifted beyond tolerance between two suite runs, +/// or a comparability violation that refuses the pairing. #[derive(Debug)] pub struct Drift { pub scenario: String, + /// `p50`, `p95`, `throughput`, or a `fault:*` comparability violation. pub metric: &'static str, pub a: f64, pub b: f64, - /// `b/a` for ratio metrics; NaN when the scenario is missing in `b`. + /// `b/a` for ratio metrics; NaN for comparability faults and a + /// scenario missing in `b`. pub ratio: f64, } @@ -165,6 +168,13 @@ pub struct Drift { /// regression — wrong path, lost pacing — moves it by an order of /// magnitude. The absolute floor covers sub-10 ms timer noise. /// Latencies are compared in ms, throughput in MiB/s. +/// +/// Comparison is refused rather than silent when a scenario is absent, +/// failed, recorded under a different backend/impairment profile, has +/// fewer than `min_samples` per side, or carries absent/nonfinite +/// metrics. The impairment seed is not part of the profile: the same +/// conditions under a different drop schedule are exactly what a +/// reproducibility gate exists to exercise. pub struct CompareOpts { /// Relative tolerance for the median, e.g. 0.15 = ±15%. pub tol: f64, @@ -175,6 +185,9 @@ pub struct CompareOpts { pub latency_floor_ms: f64, /// Absolute throughput floor in MiB/s. pub throughput_floor: f64, + /// Minimum samples per side before percentile claims count + /// (default 3: fewer cannot support a percentile statement). + pub min_samples: usize, } impl Default for CompareOpts { @@ -184,6 +197,7 @@ impl Default for CompareOpts { tail_factor: 3.0, latency_floor_ms: 10.0, throughput_floor: 3.0, + min_samples: 3, } } } @@ -194,9 +208,40 @@ impl CompareOpts { } } +fn fault(scenario: &str, metric: &'static str) -> Drift { + Drift { + scenario: scenario.into(), + metric, + a: 0.0, + b: 0.0, + ratio: f64::NAN, + } +} + +/// Same measurement conditions: impairment parameters must agree +/// exactly, except `seed`, which legitimately varies between runs of +/// the same profile. +fn same_profile(a: &BenchReport, b: &BenchReport) -> bool { + match (&a.meta.impairment, &b.meta.impairment) { + (None, None) => true, + (Some(x), Some(y)) => { + x.loss == y.loss + && x.delay_ms == y.delay_ms + && x.jitter_ms == y.jitter_ms + && x.rate_mbps == y.rate_mbps + } + _ => false, + } +} + +fn scenario_failed(r: &BenchReport) -> bool { + r.notes.iter().any(|n| n.starts_with("SCENARIO FAILED")) + || matches!(r.attempts, Some((ok, total)) if ok < total) +} + /// Compare p50, p95 and throughput per scenario across two suites. -/// A scenario missing from `b` surfaces as a NaN drift — absence is a -/// failure, not silence. +/// Every refusal or drift is returned — absence, failure, profile +/// mismatch, thin samples and nonfinite values are faults, not silence. pub fn compare(a: &BenchSuite, b: &BenchSuite, opts: &CompareOpts) -> Vec { let mut out = Vec::new(); for ra in &a.reports { @@ -205,15 +250,27 @@ pub fn compare(a: &BenchSuite, b: &BenchSuite, opts: &CompareOpts) -> Vec .iter() .find(|rb| rb.meta.scenario == ra.meta.scenario && rb.meta.path == ra.meta.path) else { - out.push(Drift { - scenario: ra.meta.scenario.clone(), - metric: "scenario", - a: 0.0, - b: 0.0, - ratio: f64::NAN, - }); + out.push(fault(&ra.meta.scenario, "scenario")); continue; }; + if ra.meta.backend != rb.meta.backend { + out.push(fault(&ra.meta.scenario, "fault:backend-mismatch")); + continue; + } + if !same_profile(ra, rb) { + out.push(fault(&ra.meta.scenario, "fault:impairment-mismatch")); + continue; + } + if scenario_failed(ra) || scenario_failed(rb) { + out.push(fault(&ra.meta.scenario, "fault:scenario-failed")); + continue; + } + if let (Some(pa), Some(pb)) = (&ra.rtt, &rb.rtt) + && pa.count.min(pb.count) < opts.min_samples + { + out.push(fault(&ra.meta.scenario, "fault:insufficient-samples")); + continue; + } for (metric, fa, fb, tol, floor) in [ ( "p50", @@ -237,16 +294,24 @@ pub fn compare(a: &BenchSuite, b: &BenchSuite, opts: &CompareOpts) -> Vec opts.throughput_floor, ), ] { - if let (Some(va), Some(vb)) = (fa, fb) - && opts.drifts(va, vb, tol, floor) - { - out.push(Drift { - scenario: ra.meta.scenario.clone(), - metric, - a: va, - b: vb, - ratio: vb / va.max(f64::EPSILON), - }); + match (fa, fb) { + (Some(_), None) | (None, Some(_)) => { + out.push(fault(&ra.meta.scenario, "fault:metric-absent")); + } + (Some(va), Some(vb)) => { + if !va.is_finite() || !vb.is_finite() { + out.push(fault(&ra.meta.scenario, "fault:nonfinite")); + } else if opts.drifts(va, vb, tol, floor) { + out.push(Drift { + scenario: ra.meta.scenario.clone(), + metric, + a: va, + b: vb, + ratio: vb / va.max(f64::EPSILON), + }); + } + } + (None, None) => {} } } } @@ -382,4 +447,147 @@ mod tests { }; assert!(compare(&a, &b, &CompareOpts::default()).is_empty()); } + + fn suite_with(mut r: BenchReport) -> BenchSuite { + r.meta.scenario = "s".into(); + BenchSuite { + tool: "t".into(), + unix_ts: 0, + git: None, + reports: vec![r], + } + } + + fn rtt_report(count: usize, p95_ns: u64) -> BenchReport { + BenchReport { + meta: BenchMeta { + scenario: "s".into(), + backend: "iroh".into(), + path: "direct".into(), + impairment: None, + unix_ts: 0, + git: None, + }, + rtt: Some(Percentiles { + count, + min_ns: 1, + p50_ns: p95_ns / 2, + p95_ns, + p99_ns: p95_ns, + max_ns: p95_ns, + mean_ns: p95_ns as f64 / 2.0, + }), + throughput_mib_s: None, + attempts: None, + metrics: Default::default(), + notes: vec![], + } + } + + fn has_fault(drift: &[Drift], metric: &str) -> bool { + drift.iter().any(|d| d.metric == metric) + } + + #[test] + fn compare_rejects_every_incomparable_profile() { + let a = suite_with(rtt_report(50, 50_000_000)); + for mutate in [ + (|r: &mut BenchReport| r.meta.backend = "noq".into()) as fn(&mut BenchReport), + |r| { + r.meta.impairment = Some(Impairment { + loss: 0.05, + ..Impairment::default() + }) + }, + ] { + let mut b = suite_with(rtt_report(50, 50_000_000)); + mutate(&mut b.reports[0]); + let drift = compare(&a, &b, &CompareOpts::default()); + assert!( + has_fault(&drift, "fault:backend-mismatch") + || has_fault(&drift, "fault:impairment-mismatch"), + "profile change must refuse comparison: {drift:?}" + ); + } + // Same impairment but a different seed is the same profile. + let mut seeded = rtt_report(50, 50_000_000); + seeded.meta.impairment = Some(Impairment { + loss: 0.05, + seed: 1, + ..Impairment::default() + }); + let mut reseeded = rtt_report(50, 50_000_000); + reseeded.meta.impairment = Some(Impairment { + loss: 0.05, + seed: 2, + ..Impairment::default() + }); + assert!( + compare( + &suite_with(seeded), + &suite_with(reseeded), + &CompareOpts::default() + ) + .is_empty(), + "seed differs within one impairment profile: not a fault" + ); + } + + #[test] + fn compare_rejects_failed_thin_and_nonfinite_reports() { + let a = suite_with(rtt_report(50, 50_000_000)); + + let mut failed = suite_with(rtt_report(50, 50_000_000)); + failed.reports[0].attempts = Some((9, 50)); + assert!(has_fault( + &compare(&a, &failed, &CompareOpts::default()), + "fault:scenario-failed" + )); + + let mut noted = suite_with(rtt_report(50, 50_000_000)); + noted.reports[0].notes.push("SCENARIO FAILED: x".into()); + assert!(has_fault( + &compare(&a, ¬ed, &CompareOpts::default()), + "fault:scenario-failed" + )); + + let thin = suite_with(rtt_report(2, 50_000_000)); + assert!(has_fault( + &compare(&a, &thin, &CompareOpts::default()), + "fault:insufficient-samples" + )); + + let mut nan = suite_with(rtt_report(50, 50_000_000)); + nan.reports[0].throughput_mib_s = Some(f64::NAN); + let mut a_tp = suite_with(rtt_report(50, 50_000_000)); + a_tp.reports[0].throughput_mib_s = Some(40.0); + assert!(has_fault( + &compare(&a_tp, &nan, &CompareOpts::default()), + "fault:nonfinite" + )); + } + + #[test] + fn compare_rejects_metrics_present_on_one_side_only() { + let mut ra = rtt_report(50, 50_000_000); + ra.throughput_mib_s = Some(40.0); + let rb = rtt_report(50, 50_000_000); + let drift = compare( + &suite_with(ra.clone()), + &suite_with(rb), + &CompareOpts::default(), + ); + assert!(has_fault(&drift, "fault:metric-absent")); + + // Symmetric absence on both sides is not a fault. + let clean = suite_with(rtt_report(50, 50_000_000)); + assert!( + compare( + &clean, + &suite_with(rtt_report(50, 52_000_000)), + &CompareOpts::default() + ) + .is_empty() + ); + } } diff --git a/docs/remediation-progress.md b/docs/remediation-progress.md index 451391c..440326b 100644 --- a/docs/remediation-progress.md +++ b/docs/remediation-progress.md @@ -31,6 +31,7 @@ Neither increment closes these product gaps or any wave. |---|---|---| | W0.1 | Partial | R01/R10 are agent regressions; R02 is now covered by transactional record/delete regressions; R03/R04 are journal regressions, with failures observed before fixing. R05 is covered by planted-link and directory-substitution tests. R06 failed before the name proof fix; R07 is covered by server expiry checks. R08 has failing-before actual-client Drain/drop regressions and passing framing/grace checks. R09 has failing-before direct/relay candidate regressions and passing family/cancellation checks. Desktop byte/cancellation and lifecycle regressions now cover the W6.1/W6.2 increments; broader native/network qualification remains open. | | W0.2 | Partial; receiver completion barrier | Versioned transfer goodput waits for exact received byte count, BLAKE3 digest and response EOF under one operation deadline. A missing-receipt regression failed before the fix; corrupt/truncated/reordered payloads, invalid receipts, delayed reception and real iroh/noq forwarded-TCP checks cover the boundary. Known-rate calibration, connect/auth/service phase timings and topology/load qualification remain open; see [contract](benchmark-transfer.md). | +| W0.4 | Partial; comparator faults refused | Absent metrics, nonfinite values, failed scenarios, insufficient samples and backend/impairment profile mismatches now refuse comparison (seed excluded from profile). `checkpoint.sh` registers `r0-evidence` with a CLI negative-fixture battery; the c1 noq suite gains its missing `transport-noq` feature flag. Gate invocation and re-qualification of historical reports remain open; see section below. | | W1.1 | Implemented; Linux checks passed | Denylist replacement retains its value without observers; atomic modification preserves concurrent revocations. Subscribe-before-check and initial watchdog snapshot check remove missed-update windows. Durable feed freshness remains W1.4. | | W1.2 | Implemented; Linux checks passed | One authorization state owns admission, replay reservation and watchdog. ACK failure/cancellation closes the connection and releases the grant. Service admission checks live validity/revocation. Connection future teardown runs RAII cleanup. | | W1.3 | Implemented; Linux checks passed | Client trust anchor, per-name domain-separated signatures, exact name/record binding, current validity and volatile anti-rollback. Native directory HTTPS/DNS added; durable revision linkage stays W1.4 and native macOS verification remains open. | @@ -1521,3 +1522,33 @@ fails closed rather than reporting a partial number. Report metadata does not yet record the build profile; that is W0.6 scope. W0.2 stays partial: known-rate calibration, per-phase connect/auth/service timings and topology/load qualification remain open. + +## 2026-09-26 — comparator strictness and r0-evidence gate (W0.4 partial) + +`rds-bench compare` refused only missing scenarios and numeric drift before; +a metric present on one side but absent on the other, a failed scenario, +a backend or impairment-profile mismatch, a nonfinite value or a single-sample +percentile all compared silently. Comparison now returns a `fault:*` refusal +for each of those, matched before any number is weighed; the impairment seed +is deliberately outside the profile so different drop schedules of the same +conditions still compare. `--min-samples` bounds percentile claims (default 3; +fewer cannot support one). Nonfinite values are additionally refused at JSON +parse — serde_json rejects `NaN`, and the comparator check protects library +callers. Unit tests cover every refusal plus the equal-profile clean pair; +CLI negative fixtures exercise the same paths end to end. + +`scripts/checkpoint.sh` gains the registered `r0-evidence` gate (the first +remediation gate): fmt/clippy/test, the rds-bench suite and a generated +negative-fixture battery whose every entry must make `compare` exit 1. The +c1 noq suite line also gained the `--features transport-noq` flag it always +needed — `--backend noq` fails closed without the compiled backend, so the +gate previously could not produce its noq suite at all. Other `r*` gates stay +unregistered until their waves introduce their checks. W0.4 remains partial: +the comparator now enforces its contract, but gate invocation and historical +report re-qualification under the strict rules are open, and the remaining +W0 tasks (relay bench world, capability matrix, receipt schema) are unstarted. + +Local validation: `cargo fmt --check`, `cargo clippy -p rds-bench +--all-targets -- -D warnings`, `cargo test -p rds-bench` (10 tests), the +fixture battery through the built CLI (7 refused, 1 accepted) and +`bash -n scripts/checkpoint.sh`. diff --git a/scripts/checkpoint.sh b/scripts/checkpoint.sh index aea4a41..51e0f76 100755 --- a/scripts/checkpoint.sh +++ b/scripts/checkpoint.sh @@ -98,7 +98,7 @@ c1) || fail "turmoil sim" note "noq bench suite" - cargo run -q -p rds-bench -- run --scenario all --backend noq \ + cargo run -q -p rds-bench --features transport-noq -- run --scenario all --backend noq \ --json "$REPORTS/bench-${TS}-noq.json" --md "$REPORTS/bench-${TS}-noq.md" \ || fail "noq bench suite" @@ -334,10 +334,67 @@ c8) this gate covers the artifact layer; ssh-across-NAT, soak and desktop-smoke rows are attested there" ;; +r0-evidence) + note "gate r0-evidence — remediation measurement truthfulness (W0)" + green_bars + + note "bench self-tests (comparator, verified transfer, world)" + cargo test -p rds-bench || fail "rds-bench tests" + + note "comparator negative fixtures — every refusal must exit 1" + FX="$(mktemp -d)" + python3 - "$FX" <<'PY' || fail "fixture generation" +import json, sys, os +d = sys.argv[1] +def rep(scenario="ping", backend="iroh", path="direct", count=50, + p95=50_000_000, tp=None, attempts=None, notes=None, imp=None): + return {"meta": {"scenario": scenario, "backend": backend, "path": path, + "impairment": imp, "unix_ts": 0, "git": "fixture"}, + "rtt": {"count": count, "min_ns": 1, "p50_ns": p95 // 2, + "p95_ns": p95, "p99_ns": p95, "max_ns": p95, + "mean_ns": p95 / 2}, + "throughput_mib_s": tp, "attempts": attempts, + "metrics": {}, "notes": notes or []} +def suite(name, reports): + json.dump({"tool": "fixture", "unix_ts": 0, "git": "fixture", + "reports": reports}, open(os.path.join(d, name), "w")) +suite("a.json", [rep()]) +suite("same.json", [rep(p95=52_000_000)]) +suite("missing.json", [rep(scenario="other")]) +suite("backend.json", [rep(backend="noq")]) +suite("impairment.json", [rep(imp={"loss": 0.05, "delay_ms": 50, + "jitter_ms": 30, "rate_mbps": None, + "seed": 2})]) +suite("failed.json", [rep(attempts=[9, 50])]) +suite("thin.json", [rep(count=1)]) +suite("nan.json", [rep(tp=float("nan"))]) +suite("absent.json", [{**rep(), "throughput_mib_s": None, + "rtt": None}]) +PY + for bad in missing backend impairment failed thin nan absent; do + if cargo run -q -p rds-bench -- compare "$FX/a.json" "$FX/$bad.json" >/dev/null 2>&1; then + fail "comparator accepted $bad fixture" + fi + done + cargo run -q -p rds-bench -- compare "$FX/a.json" "$FX/same.json" >/dev/null \ + || fail "comparator rejected a clean fixture pair" + rm -rf "$FX" + + write_checkpoint "r0-evidence" "pending review" \ + "- fmt/clippy/test: PASS +- rds-bench unit tests (comparator faults, verified transfer, world): PASS +- comparator CLI negative fixtures refuse: missing scenario, backend / + impairment mismatch, failed scenario, single-sample, NaN, absent + metric: PASS +- comparator CLI accepts an equal-profile clean pair: PASS +- W0.1 regression inventory: see remediation-progress.md table +- W0.3 owned-relay bench world + W0.5 capability matrix + W0.6 receipt + schema: pending their own increments — this gate does not close W0" + ;; *) cat <