From 7eac0462a8e3e0b002a5f98844f83cb52b77158a Mon Sep 17 00:00:00 2001 From: Jens Holdgaard Pedersen Date: Tue, 26 May 2026 13:57:25 +0200 Subject: [PATCH 1/3] feat(bench): clap CLI + JSON results-file writer (PR-J1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the bench binary to drive `run()` and lands the §3.6 JSON results-file writer. `main.rs` is no longer the red-gate scaffold — `cargo run -p ourios-bench --` / `just thesis-bench` now parse the §3.7 flag surface, run the enabled gates, write the results JSON, print a summary, and exit non-zero on a C1 correctness failure. Lands: - `Cargo.toml` — adds `clap` (derive). The CNCF-Rust- observability standard (Vector / Quickwit / OpenObserve / GreptimeDB all use it; the Rust analogue to Go's cobra); `ourios-server` will reuse it. - `src/main.rs` — clap `Cli` for the §3.7 flags (`--corpus`, `--results-dir`, `--bucket-dir`, `--keep-parquet`, `--hardware-kind` / `--allow-unknown-hardware`, `--update-benchmarks-md`, `--gates a1,c1,c2`). `--hardware-kind` is `required_unless_present` `--allow-unknown-hardware`; `--keep-parquet` `requires` `--bucket-dir`; `--gates` collapses to a `GateSet` (empty ⇒ all). Drives `run`, writes the JSON, prints a per-gate summary, exits non-zero only on a C1 reconstruction mismatch (§3.4.2) — A1/C2 gate outcomes are reported, not exit-gating. - `src/report.rs` — `write_results_json` serialises a `ResultsFile` to `/-.json` (colon-free name for cross-platform validity; bounded collision-retry suffix per §3.6). Re-exported from the crate root. Verified end-to-end on the seed corpus (`--gates a1,c1 --hardware-kind dev-laptop`): writes a valid results JSON, C1 = 1.000000 PASS, audit stream produces 5 KB (so the A1 audit-writer path is exercised), exit 0. A1 reports FAIL on the seed corpus as expected — 77 lines → ~1 MB Parquet (footer / dictionary overhead dwarfs the data), which is exactly why real §9 numbers need a millions-of-lines corpus and why A1-fail doesn't gate the exit. Un-`#[ignore]`'d / added tests: - RFC0006.5 (`main.rs`): `--hardware-kind` required unless `--allow-unknown-hardware`; parse-time rejection. - RFC0006.6 (`main.rs`): `--gates` scopes the measurement; default is all. - `--keep-parquet` requires `--bucket-dir` (parse-time). - RFC0006.4 JSON half (`report.rs`): results round-trip through disk with every §3.6 key; colon-free filename; collision suffix. Not in this PR: - The `--update-benchmarks-md` §9 markdown appender — parsed but warns "not implemented"; the JSON lands regardless. The §9 in-place rewrite (RFC0006.4 second sub-test, RFC0006.6 §9-untouched assertion) lands in PR-J2. - C2 — still `NotImplemented`; a default `--gates` (all) run errors on C2 until it lands. RFC0006.3 stays ignored. Verification (CLAUDE.md §6.6): - cargo fmt --all --check — clean. - cargo clippy --all-targets --all-features -- -D warnings — clean. - cargo test --all-features — 258 passed / 22-ish ignored (was 252; +3 CLI tests, +3 results-writer tests). Maturity gate: RFC 0006 stays `red` until C2 + the §9 appender land (RFC0006.3 / RFC0006.4-§9 / RFC0006.7 still ignored). Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 121 ++++++++++++ crates/ourios-bench/Cargo.toml | 7 + crates/ourios-bench/src/lib.rs | 27 +-- crates/ourios-bench/src/main.rs | 302 ++++++++++++++++++++++++++++-- crates/ourios-bench/src/report.rs | 166 ++++++++++++++++ 5 files changed, 595 insertions(+), 28 deletions(-) create mode 100644 crates/ourios-bench/src/report.rs diff --git a/Cargo.lock b/Cargo.lock index 900c916e..c2f348cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -25,6 +25,56 @@ dependencies = [ "libc", ] +[[package]] +name = "anstream" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "824a212faf96e9acacdbd09febd34438f8f711fb84e09a8916013cd7815ca28d" +dependencies = [ + "anstyle", + "anstyle-parse", + "anstyle-query", + "anstyle-wincon", + "colorchoice", + "is_terminal_polyfill", + "utf8parse", +] + +[[package]] +name = "anstyle" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "940b3a0ca603d1eade50a4846a2afffd5ef57a9feac2c0e2ec2e14f9ead76000" + +[[package]] +name = "anstyle-parse" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "52ce7f38b242319f7cabaa6813055467063ecdc9d355bbb4ce0c68908cd8130e" +dependencies = [ + "utf8parse", +] + +[[package]] +name = "anstyle-query" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" +dependencies = [ + "windows-sys", +] + +[[package]] +name = "anstyle-wincon" +version = "3.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" +dependencies = [ + "anstyle", + "once_cell_polyfill", + "windows-sys", +] + [[package]] name = "anyhow" version = "1.0.102" @@ -232,6 +282,52 @@ dependencies = [ "windows-link", ] +[[package]] +name = "clap" +version = "4.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ddb117e43bbf7dacf0a4190fef4d345b9bad68dfc649cb349e7d17d28428e51" +dependencies = [ + "clap_builder", + "clap_derive", +] + +[[package]] +name = "clap_builder" +version = "4.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "714a53001bf66416adb0e2ef5ac857140e7dc3a0c48fb28b2f10762fc4b5069f" +dependencies = [ + "anstream", + "anstyle", + "clap_lex", + "strsim", +] + +[[package]] +name = "clap_derive" +version = "4.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2ce8604710f6733aa641a2b3731eaa1e8b3d9973d5e3565da11800813f997a9" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "clap_lex" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" + +[[package]] +name = "colorchoice" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" + [[package]] name = "const-random" version = "0.1.18" @@ -548,6 +644,12 @@ version = "3.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8bb03732005da905c88227371639bf1ad885cc712789c011c31c5fb3ab3ccf02" +[[package]] +name = "is_terminal_polyfill" +version = "1.70.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" + [[package]] name = "itertools" version = "0.14.0" @@ -758,6 +860,12 @@ version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" +[[package]] +name = "once_cell_polyfill" +version = "1.70.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" + [[package]] name = "opentelemetry" version = "0.32.0" @@ -805,6 +913,7 @@ dependencies = [ name = "ourios-bench" version = "0.0.0" dependencies = [ + "clap", "ourios-core", "ourios-miner", "ourios-parquet", @@ -1072,6 +1181,12 @@ version = "0.4.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" +[[package]] +name = "strsim" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" + [[package]] name = "syn" version = "2.0.117" @@ -1160,6 +1275,12 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" +[[package]] +name = "utf8parse" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" + [[package]] name = "uuid" version = "1.23.1" diff --git a/crates/ourios-bench/Cargo.toml b/crates/ourios-bench/Cargo.toml index 36a97d8e..5a8cb7a5 100644 --- a/crates/ourios-bench/Cargo.toml +++ b/crates/ourios-bench/Cargo.toml @@ -24,6 +24,13 @@ path = "src/lib.rs" ourios-core = { path = "../ourios-core" } ourios-miner = { path = "../ourios-miner" } ourios-parquet = { path = "../ourios-parquet" } +# CLI argument parsing for the §3.7 flag surface. `clap` is the +# CNCF-Rust-observability standard (Vector, Quickwit, +# OpenObserve, GreptimeDB all use it) — the Rust analogue to +# Go's cobra. `derive` keeps the parser declarative; the binary +# is operator-facing, exactly where clap's `--help` / validation +# earn their weight. `ourios-server` will reuse it. +clap = { version = "4", features = ["derive"] } # Serde + serde_json for the §3.6 ResultsFile JSON contract. # `derive` gives the struct→JSON path; `std` covers the rest. serde = { version = "1", default-features = false, features = ["derive", "std"] } diff --git a/crates/ourios-bench/src/lib.rs b/crates/ourios-bench/src/lib.rs index d6905c07..061afe12 100644 --- a/crates/ourios-bench/src/lib.rs +++ b/crates/ourios-bench/src/lib.rs @@ -2,24 +2,24 @@ //! (A1 compression, C1 reconstruction, C2 template-count //! convergence). //! -//! **Implementation status (PR-I2):** the A1 (compression) and +//! **Implementation status (PR-J1):** the A1 (compression) and //! C1 (reconstruction) gates are live — [`run`] computes them //! end-to-end, in any combination, in a single miner pass, and -//! returns a populated [`ResultsFile`]. C2 still returns +//! returns a populated [`ResultsFile`]. The CLI (RFC 0006 +//! §3.7) in `main.rs` drives `run` and writes the §3.6 JSON +//! results file via [`write_results_json`]. C2 still returns //! [`BenchError::NotImplemented`] when selected via //! `config.gates`; its `#[ignore]`'d test stubs in //! `tests/{c2,reproducibility}.rs` get un-ignored when it -//! lands. The CLI parser (RFC 0006 §3.7) and the -//! `docs/benchmarks.md` §9 result-file writer ([`ResultsFile`] -//! → disk / markdown) also remain unwritten — `main.rs` is the -//! red-stage scaffold and the binary path doesn't yet drive -//! [`run`]. +//! lands. The `docs/benchmarks.md` §9 markdown appender (the +//! `--update-benchmarks-md` path) is the remaining `report` +//! piece and lands in a follow-up. //! -//! Per RFC 0006 §3.2 the eventual module layout is `corpus`, -//! `harness`, `a1`, `c1`, `c2`, `report`. Those modules land -//! incrementally. PR-I1 extracted `corpus`, `harness`, `c1`; -//! PR-I2 added `a1`; `c2` / `report` remain unwritten until -//! their respective implementation PRs. +//! Per RFC 0006 §3.2 the module layout is `corpus`, `harness`, +//! `a1`, `c1`, `c2`, `report`. PR-I1 extracted `corpus`, +//! `harness`, `c1`; PR-I2 added `a1`; PR-J1 added `report` +//! (JSON half); `c2` remains unwritten until its +//! implementation PR. #![deny(unsafe_code)] @@ -30,6 +30,9 @@ mod a1; mod c1; mod corpus; mod harness; +mod report; + +pub use report::write_results_json; /// Configuration for one bench invocation. /// diff --git a/crates/ourios-bench/src/main.rs b/crates/ourios-bench/src/main.rs index ff52c334..4067b76f 100644 --- a/crates/ourios-bench/src/main.rs +++ b/crates/ourios-bench/src/main.rs @@ -1,24 +1,294 @@ //! `ourios-bench` binary entry point. //! -//! Once implemented, this binary will be a thin wrapper that -//! parses CLI arguments into a [`ourios_bench::BenchConfig`] -//! and calls [`ourios_bench::run`]. The CLI surface is pinned -//! by RFC 0006 §3.7. **Today this file is the Red-gate -//! scaffold**: `main()` prints a banner to stderr and exits -//! non-zero without touching the library. Argument parsing -//! and the call into `run` land in the PR-H2 follow-up -//! together with the test stubs that exercise them. +//! Parses the RFC 0006 §3.7 flag surface into a +//! [`ourios_bench::BenchConfig`], drives +//! [`ourios_bench::run`], writes the §3.6 JSON results file, +//! prints a human summary, and maps the outcome to a process +//! exit code (a C1 reconstruction mismatch is a hard failure +//! per §3.4.2). +//! +//! The `--update-benchmarks-md` §9 markdown appender is not +//! implemented yet — the flag is accepted (so the surface +//! matches §3.7) but currently only warns; the JSON results +//! file is written regardless. +use std::path::PathBuf; use std::process::ExitCode; +use clap::{Parser, ValueEnum}; +use ourios_bench::{BenchConfig, BenchError, GateSet, run, write_results_json}; + +/// RFC 0006 §1 hardware baseline tag, surfaced in the +/// `--allow-unknown-hardware` warning so an operator knows +/// what to run on for comparable numbers. +const BASELINE_HARDWARE_TAG: &str = "baseline-8vcpu-32gib (8 vCPU / 32 GiB / gp3-class SSD)"; + +/// RFC 0006 §3.7 thesis-gate bench harness CLI. +#[derive(Parser, Debug)] +#[command( + name = "ourios-bench", + about = "RFC 0006 thesis-gate bench harness (A1 compression / C1 reconstruction / C2 convergence)" +)] +struct Cli { + /// Directory of `*.txt` corpus files to load. + #[arg(long, default_value = "testdata/corpus")] + corpus: PathBuf, + /// Where the §3.6 JSON results file lands. + #[arg(long, default_value = "benchmarks/results")] + results_dir: PathBuf, + /// Parquet writer `bucket_root`. Defaults to a fresh temp + /// dir, cleaned up on exit unless `--keep-parquet`. + #[arg(long)] + bucket_dir: Option, + /// Keep the Parquet output for inspection. Requires + /// `--bucket-dir` (a scratch dir's path isn't reported, so + /// keeping it would be unfindable). + #[arg(long, requires = "bucket_dir")] + keep_parquet: bool, + /// §3.5 hardware-kind annotation. Required unless + /// `--allow-unknown-hardware`. + #[arg(long, required_unless_present = "allow_unknown_hardware")] + hardware_kind: Option, + /// Tag the results `hardware_kind = "unknown"` instead of + /// requiring `--hardware-kind`. + #[arg(long)] + allow_unknown_hardware: bool, + /// Append / rewrite the `docs/benchmarks.md` §9 sub-heading + /// (not implemented yet — see crate docs). + #[arg(long)] + update_benchmarks_md: bool, + /// Comma-separated subset of gates to compute. Default: all. + #[arg(long, value_enum, value_delimiter = ',')] + gates: Vec, +} + +/// One thesis gate, as named on the `--gates` flag. +#[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)] +enum Gate { + #[value(name = "a1")] + A1, + #[value(name = "c1")] + C1, + #[value(name = "c2")] + C2, +} + +impl Cli { + /// Collapse the `--gates` list into a [`GateSet`]. An empty + /// list (flag omitted) means all gates, per §3.7. + fn gate_set(&self) -> GateSet { + if self.gates.is_empty() { + return GateSet::all(); + } + GateSet { + a1: self.gates.contains(&Gate::A1), + c1: self.gates.contains(&Gate::C1), + c2: self.gates.contains(&Gate::C2), + } + } + + fn into_config(self) -> BenchConfig { + let gates = self.gate_set(); + BenchConfig { + corpus_dir: self.corpus, + results_dir: self.results_dir, + bucket_dir: self.bucket_dir, + keep_parquet: self.keep_parquet, + // `None` here ⇒ `run` tags `hardware_kind = + // "unknown"`; reachable only with + // `--allow-unknown-hardware` (clap enforces the + // flag otherwise). + hardware_kind: self.hardware_kind, + update_benchmarks_md: self.update_benchmarks_md, + gates, + } + } +} + fn main() -> ExitCode { - eprintln!( - "ourios-bench: RFC 0006 Red-gate scaffold — argument parser and harness are \ - not implemented yet. Track progress on the maturity-model bump in \ - `docs/rfcs/0006-bench-harness.md` §7.", + let cli = Cli::parse(); + match run_bench(cli) { + Ok(code) => code, + Err(e) => { + eprintln!("ourios-bench: {e}"); + ExitCode::from(2) + } + } +} + +fn run_bench(cli: Cli) -> Result { + if cli.hardware_kind.is_none() { + // Reachable only via --allow-unknown-hardware (clap + // requires one of the two). Name the §1 baseline so an + // operator knows what to run on for comparable numbers. + eprintln!( + "ourios-bench: warning: --allow-unknown-hardware set; results are tagged \ + hardware_kind=\"unknown\". For numbers comparable to the thesis gates, run on the \ + §1 baseline ({BASELINE_HARDWARE_TAG}) and pass --hardware-kind.", + ); + } + let keep_parquet_path = cli.keep_parquet.then(|| cli.bucket_dir.clone()).flatten(); + let update_md = cli.update_benchmarks_md; + let results_dir = cli.results_dir.clone(); + + let config = cli.into_config(); + let results = run(&config)?; + let path = write_results_json(&results, &results_dir)?; + eprintln!("ourios-bench: results written to {}", path.display()); + + if let Some(bucket) = keep_parquet_path { + eprintln!( + "ourios-bench: --keep-parquet set; Parquet output retained at {}", + bucket.display(), + ); + } + print_summary(&results); + if update_md { + eprintln!( + "ourios-bench: warning: --update-benchmarks-md is not implemented yet (the §9 \ + markdown appender lands in a follow-up PR); JSON results written only.", + ); + } + + // §3.4.2: a non-lossy reconstruction mismatch is a + // correctness failure, not just a degraded number — exit + // non-zero so CI / a `/bench` run surfaces it. A1 / C2 + // gate outcomes are *reported* (in the JSON + summary) but + // don't fail the process; whether a missed compression + // target pauses the project is the §7 escalation rule's + // human judgment, not a build red. + if let Some(c1) = &results.c1 { + if !c1.pass { + let failed = c1.non_lossy_total - c1.non_lossy_reconstruct_ok; + eprintln!( + "ourios-bench: C1 FAILED — {failed} of {} non-lossy row(s) did not reconstruct \ + byte-for-byte (RFC 0006 §3.4.2 / CLAUDE.md §3.3)", + c1.non_lossy_total, + ); + return Ok(ExitCode::from(1)); + } + } + Ok(ExitCode::SUCCESS) +} + +/// Print a one-line-per-gate human summary to stdout. The +/// machine-readable form is the JSON results file; this is +/// just operator feedback. +fn print_summary(results: &ourios_bench::ResultsFile) { + println!( + "corpus {} — {} line(s), {} file(s), {} raw byte(s) [{}]", + results.corpus.directory, + results.corpus.total_lines, + results.corpus.total_files, + results.corpus.raw_bytes, + results.hardware_kind, ); - // Exit non-zero so a `just thesis-bench` invocation in the - // scaffold window can't be mistaken for a successful - // benchmark run. - ExitCode::from(2) + if let Some(a1) = &results.a1 { + println!( + " A1 compression: ourios {:.3}× vs zstd-19 {:.3}× → delta {:.3}× (target ≥ {:.1}×) — {}", + a1.ourios_ratio, + a1.zstd_ratio, + a1.delta, + a1.target_delta, + if a1.pass { "PASS" } else { "FAIL" }, + ); + } + if let Some(c1) = &results.c1 { + println!( + " C1 reconstruction: {:.6} ({}/{} non-lossy rows; lossy ratio {:.4}) — {}", + c1.rate, + c1.non_lossy_reconstruct_ok, + c1.non_lossy_total, + c1.lossy_flag_ratio, + if c1.pass { "PASS" } else { "FAIL" }, + ); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// RFC0006.5 — `--hardware-kind` is required unless + /// `--allow-unknown-hardware`. clap rejects the bare + /// invocation at parse time, before any measurement runs. + #[test] + fn hardware_kind_required_unless_allow_unknown() { + let bare = Cli::try_parse_from(["ourios-bench"]); + assert!( + bare.is_err(), + "missing --hardware-kind must be a usage error" + ); + + let allowed = Cli::try_parse_from(["ourios-bench", "--allow-unknown-hardware"]) + .expect("--allow-unknown-hardware satisfies the requirement"); + assert!( + allowed.hardware_kind.is_none(), + "hardware_kind stays None under --allow-unknown-hardware (run tags it \"unknown\")", + ); + + let explicit = + Cli::try_parse_from(["ourios-bench", "--hardware-kind", "baseline-8vcpu-32gib"]) + .expect("explicit --hardware-kind parses"); + assert_eq!( + explicit.hardware_kind.as_deref(), + Some("baseline-8vcpu-32gib") + ); + } + + /// RFC0006.6 — `--gates` scopes the measurement; omitting + /// it means all three. + #[test] + fn gates_flag_scopes_the_measurement() { + let all = Cli::try_parse_from(["ourios-bench", "--allow-unknown-hardware"]) + .expect("parse") + .gate_set(); + assert_eq!(all, GateSet::all(), "default is all gates"); + + let c1_only = + Cli::try_parse_from(["ourios-bench", "--allow-unknown-hardware", "--gates", "c1"]) + .expect("parse") + .gate_set(); + assert_eq!( + c1_only, + GateSet { + a1: false, + c1: true, + c2: false, + }, + "--gates c1 selects only C1", + ); + + let a1_c2 = Cli::try_parse_from([ + "ourios-bench", + "--allow-unknown-hardware", + "--gates", + "a1,c2", + ]) + .expect("parse") + .gate_set(); + assert_eq!( + a1_c2, + GateSet { + a1: true, + c1: false, + c2: true, + }, + "--gates a1,c2 selects A1 and C2 (comma-separated)", + ); + } + + /// `--keep-parquet` requires `--bucket-dir` at the clap + /// layer (a scratch dir's path isn't reported). Pins the + /// early rejection so the friendlier `requires` message + /// fires before `run`'s internal backstop. + #[test] + fn keep_parquet_requires_bucket_dir() { + let err = + Cli::try_parse_from(["ourios-bench", "--allow-unknown-hardware", "--keep-parquet"]); + assert!( + err.is_err(), + "--keep-parquet without --bucket-dir is a usage error" + ); + } } diff --git a/crates/ourios-bench/src/report.rs b/crates/ourios-bench/src/report.rs new file mode 100644 index 00000000..989c22e2 --- /dev/null +++ b/crates/ourios-bench/src/report.rs @@ -0,0 +1,166 @@ +//! §3.6 results-file writer. +//! +//! Serialises a [`ResultsFile`] to a per-run JSON file under +//! the results directory. The §9 `docs/benchmarks.md` +//! appender (the `--update-benchmarks-md` path) is a separate +//! follow-up — this module only owns the machine-readable +//! JSON artifact, which lands on every run regardless of the +//! markdown flag. + +use std::path::{Path, PathBuf}; + +use crate::{BenchError, ResultsFile}; + +/// Write `results` as pretty JSON to `results_dir`, returning +/// the path written. The file name is +/// `-.json` per §3.1, with a numeric +/// suffix (`-1`, `-2`, …) appended on collision so two runs +/// landing in the same millisecond on the same commit don't +/// clobber each other (the §3.6 collision-retry rule). +/// +/// # Errors +/// +/// [`BenchError::Report`] when the directory can't be created, +/// the results can't be serialised, or the file write fails. +pub fn write_results_json( + results: &ResultsFile, + results_dir: &Path, +) -> Result { + std::fs::create_dir_all(results_dir).map_err(|e| BenchError::Report { + detail: format!("create_dir_all({}): {e}", results_dir.display()), + })?; + + let stem = file_stem(&results.timestamp, &results.git_sha); + let mut path = results_dir.join(format!("{stem}.json")); + // Bounded collision retry. A few thousand same-ms same-sha + // runs is already pathological; cap the suffix search so a + // filesystem returning a persistent error from `exists` + // can't spin forever. + for counter in 1..=10_000u32 { + if !path.exists() { + break; + } + path = results_dir.join(format!("{stem}-{counter}.json")); + } + + let json = serde_json::to_string_pretty(results).map_err(|e| BenchError::Report { + detail: format!("serialise results: {e}"), + })?; + std::fs::write(&path, json).map_err(|e| BenchError::Report { + detail: format!("write({}): {e}", path.display()), + })?; + Ok(path) +} + +/// File-name stem `-`, with `:` from the +/// RFC3339 timestamp replaced by `-`. Colons are illegal in +/// filenames on Windows (and awkward on some tooling), so the +/// on-disk name uses a colon-free form even though the +/// `timestamp` field inside the JSON keeps canonical RFC3339. +fn file_stem(timestamp: &str, git_sha: &str) -> String { + format!("{}-{}", timestamp.replace(':', "-"), git_sha) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{A1Result, C1Result, CorpusStats, OuriosStats, ZstdStats}; + + fn sample_results() -> ResultsFile { + ResultsFile { + rfc: "RFC 0006".to_string(), + rfc_version: "v1".to_string(), + timestamp: "2026-05-26T14:30:00.123Z".to_string(), + git_sha: "abc1234".to_string(), + hardware_kind: "baseline-8vcpu-32gib".to_string(), + corpus: CorpusStats { + directory: "testdata/corpus".to_string(), + total_lines: 100, + total_files: 2, + raw_bytes: 4096, + }, + ourios: OuriosStats { + data_parquet_bytes: 300, + audit_parquet_bytes: 0, + total_parquet_bytes: 300, + }, + zstd: ZstdStats { + level: 19, + compressed_bytes: 1024, + }, + a1: Some(A1Result { + ourios_ratio: 13.6, + zstd_ratio: 4.0, + delta: 3.4, + target_delta: 3.0, + pass: true, + }), + c1: Some(C1Result { + non_lossy_total: 100, + non_lossy_reconstruct_ok: 100, + rate: 1.0, + lossy_flag_ratio: 0.0, + pass: true, + }), + c2: None, + } + } + + /// RFC0006.4 (JSON half): a written results file parses + /// back to an equal `ResultsFile` and carries the §3.6 + /// required keys. Pins the on-disk contract downstream + /// analysis depends on. + #[test] + fn results_json_round_trips_through_disk() { + let tmp = tempfile::TempDir::new().expect("temp dir"); + let original = sample_results(); + let path = write_results_json(&original, tmp.path()).expect("write"); + + assert!(path.exists(), "results file written"); + let text = std::fs::read_to_string(&path).expect("read back"); + // Every §3.6 required key is present on disk. + for key in [ + "rfc", + "rfc_version", + "timestamp", + "git_sha", + "hardware_kind", + "corpus", + "ourios", + "zstd", + "a1", + "c1", + "c2", + ] { + assert!(text.contains(&format!("\"{key}\"")), "missing key {key}"); + } + let parsed: ResultsFile = serde_json::from_str(&text).expect("parse"); + assert_eq!(parsed, original, "round-trip preserves every field"); + } + + /// The on-disk name is colon-free (RFC3339 colons → `-`) so + /// it's valid on every filesystem, and it embeds the git + /// sha. + #[test] + fn file_name_is_colon_free_and_embeds_sha() { + let tmp = tempfile::TempDir::new().expect("temp dir"); + let path = write_results_json(&sample_results(), tmp.path()).expect("write"); + let name = path.file_name().unwrap().to_string_lossy(); + assert!(!name.contains(':'), "no colons in filename: {name}"); + assert!(name.contains("abc1234"), "filename embeds git sha: {name}"); + assert!(name.ends_with(".json")); + } + + /// A second write on the same `(timestamp, sha)` gets a + /// distinct suffixed file rather than clobbering the first + /// — the §3.6 collision-retry rule. + #[test] + fn collision_appends_a_suffix() { + let tmp = tempfile::TempDir::new().expect("temp dir"); + let r = sample_results(); + let first = write_results_json(&r, tmp.path()).expect("first"); + let second = write_results_json(&r, tmp.path()).expect("second"); + assert_ne!(first, second, "second run gets a distinct path"); + assert!(first.exists() && second.exists(), "both files survive"); + } +} From 36f5eb31d35459e7cfc94e773f30cc18da997c3e Mon Sep 17 00:00:00 2001 From: Jens Holdgaard Pedersen Date: Tue, 26 May 2026 14:54:49 +0200 Subject: [PATCH 2/3] fixup! feat(bench): clap CLI + JSON results-file writer (PR-J1) Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/ourios-bench/src/main.rs | 18 +++++++ crates/ourios-bench/src/report.rs | 82 +++++++++++++++++++++++-------- docs/rfcs/0006-bench-harness.md | 30 ++++++++--- 3 files changed, 101 insertions(+), 29 deletions(-) diff --git a/crates/ourios-bench/src/main.rs b/crates/ourios-bench/src/main.rs index 4067b76f..95fc930f 100644 --- a/crates/ourios-bench/src/main.rs +++ b/crates/ourios-bench/src/main.rs @@ -203,6 +203,24 @@ fn print_summary(results: &ourios_bench::ResultsFile) { if c1.pass { "PASS" } else { "FAIL" }, ); } + if let Some(c2) = &results.c2 { + // `pass = None` is the §3.4.3 abstention (corpus + // < 1 M lines) — surface it as ABSTAIN, not a silent + // omission. (C2 isn't computed yet; this line is ready + // for when it lands.) + let verdict = match c2.pass { + Some(true) => "PASS", + Some(false) => "FAIL", + None => "ABSTAIN (corpus < 1 M lines)", + }; + let ratio = c2 + .convergence_ratio + .map_or_else(|| "n/a".to_string(), |r| format!("{r:.3}")); + println!( + " C2 convergence: ratio {ratio} (end template count {}, sample cadence {}) — {verdict}", + c2.template_count_at_end, c2.sample_cadence, + ); + } } #[cfg(test)] diff --git a/crates/ourios-bench/src/report.rs b/crates/ourios-bench/src/report.rs index 989c22e2..5365b3d5 100644 --- a/crates/ourios-bench/src/report.rs +++ b/crates/ourios-bench/src/report.rs @@ -7,21 +7,37 @@ //! JSON artifact, which lands on every run regardless of the //! markdown flag. +use std::fs::OpenOptions; +use std::io::Write; use std::path::{Path, PathBuf}; use crate::{BenchError, ResultsFile}; +/// Upper bound on collision-suffix candidates tried before +/// giving up. A few thousand results files sharing one +/// `(timestamp-ms, git_sha)` is already pathological; the cap +/// stops a wedged filesystem from spinning forever. +const MAX_COLLISION_CANDIDATES: u32 = 10_000; + /// Write `results` as pretty JSON to `results_dir`, returning /// the path written. The file name is -/// `-.json` per §3.1, with a numeric +/// `-.json` per §3.6, with a numeric /// suffix (`-1`, `-2`, …) appended on collision so two runs /// landing in the same millisecond on the same commit don't -/// clobber each other (the §3.6 collision-retry rule). +/// clobber each other. +/// +/// Each candidate is created with `OpenOptions::create_new` +/// (atomic "create iff absent") and retried on +/// `AlreadyExists`, so the file is never clobbered — neither +/// by a TOCTOU race against a concurrent run nor by the +/// suffix budget running out (that returns an error rather +/// than overwriting `-.json`). /// /// # Errors /// /// [`BenchError::Report`] when the directory can't be created, -/// the results can't be serialised, or the file write fails. +/// the results can't be serialised, the file write fails, or +/// all [`MAX_COLLISION_CANDIDATES`] name candidates are taken. pub fn write_results_json( results: &ResultsFile, results_dir: &Path, @@ -31,25 +47,44 @@ pub fn write_results_json( })?; let stem = file_stem(&results.timestamp, &results.git_sha); - let mut path = results_dir.join(format!("{stem}.json")); - // Bounded collision retry. A few thousand same-ms same-sha - // runs is already pathological; cap the suffix search so a - // filesystem returning a persistent error from `exists` - // can't spin forever. - for counter in 1..=10_000u32 { - if !path.exists() { - break; - } - path = results_dir.join(format!("{stem}-{counter}.json")); - } - let json = serde_json::to_string_pretty(results).map_err(|e| BenchError::Report { detail: format!("serialise results: {e}"), })?; - std::fs::write(&path, json).map_err(|e| BenchError::Report { - detail: format!("write({}): {e}", path.display()), - })?; - Ok(path) + + for counter in 0..=MAX_COLLISION_CANDIDATES { + let path = if counter == 0 { + results_dir.join(format!("{stem}.json")) + } else { + results_dir.join(format!("{stem}-{counter}.json")) + }; + // `create_new` is atomic: it fails with `AlreadyExists` + // rather than truncating an existing file, closing the + // check-then-write race. + match OpenOptions::new().write(true).create_new(true).open(&path) { + Ok(mut file) => { + file.write_all(json.as_bytes()) + .map_err(|e| BenchError::Report { + detail: format!("write({}): {e}", path.display()), + })?; + return Ok(path); + } + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {} + Err(e) => { + return Err(BenchError::Report { + detail: format!("create({}): {e}", path.display()), + }); + } + } + } + + Err(BenchError::Report { + detail: format!( + "exhausted {} results-file name candidates for stem {stem} under {} — every \ + [-N].json is taken", + MAX_COLLISION_CANDIDATES + 1, + results_dir.display(), + ), + }) } /// File-name stem `-`, with `:` from the @@ -118,7 +153,12 @@ mod tests { assert!(path.exists(), "results file written"); let text = std::fs::read_to_string(&path).expect("read back"); - // Every §3.6 required key is present on disk. + // Every §3.6 required key is present as a top-level + // object key (parse to `Value` rather than substring- + // matching, which would false-positive if a key name + // appeared inside a string value). + let value: serde_json::Value = serde_json::from_str(&text).expect("parse to value"); + let obj = value.as_object().expect("top level is a JSON object"); for key in [ "rfc", "rfc_version", @@ -132,7 +172,7 @@ mod tests { "c1", "c2", ] { - assert!(text.contains(&format!("\"{key}\"")), "missing key {key}"); + assert!(obj.contains_key(key), "missing top-level key {key}"); } let parsed: ResultsFile = serde_json::from_str(&text).expect("parse"); assert_eq!(parsed, original, "round-trip preserves every field"); diff --git a/docs/rfcs/0006-bench-harness.md b/docs/rfcs/0006-bench-harness.md index a3a76a38..01b76532 100644 --- a/docs/rfcs/0006-bench-harness.md +++ b/docs/rfcs/0006-bench-harness.md @@ -118,7 +118,8 @@ This RFC pins: across hardware classes don't masquerade as code regressions. - The output format: a per-run JSON results file under - `benchmarks/results/-.json`, and a + `benchmarks/results/-.json` + (filename colons replaced by `-`; see §3.6), and a human-readable summary appended to `docs/benchmarks.md` §9 under a date-stamped sub-heading. - The invocation surface: `cargo run -p ourios-bench --` or @@ -445,16 +446,29 @@ the explicit `--allow-unknown-hardware` opt-in so a forgotten Each bench invocation writes one results JSON to: ```text -benchmarks/results/-.json +benchmarks/results/-[-N].json ``` -The timestamp is RFC3339 with millisecond precision (e.g. -`2026-05-22T14:30:00.123Z`) so two runs on the same commit -within the same wall-clock second produce different filenames. +The name embeds the run's millisecond-precision RFC3339 +timestamp with the `:` separators replaced by `-` (so +`2026-05-22T14:30:00.123Z` becomes +`2026-05-22T14-30-00.123Z`). The colon substitution is +required: `:` is illegal in filenames on Windows and awkward +for shell / tooling elsewhere, so the on-disk *name* is +colon-free even though the `timestamp` **field inside** the +JSON keeps canonical RFC3339 (colons included). Two runs on +the same commit in the same wall-clock second still produce +distinct names via the millisecond component. + Even at millisecond precision two runs *can* theoretically -collide on a fast machine; the bench detects the conflict at -write time and retries with the next millisecond's timestamp, -emitting a warning to stderr. The directory `benchmarks/` will be created at the repo root by +collide on a fast machine. The writer creates each candidate +with an atomic `create_new` ("create iff absent") open and, +on `AlreadyExists`, appends a numeric suffix (`-1`, `-2`, …) +until it finds a free name — rather than re-deriving the +timestamp. This closes the check-then-write race against a +concurrent run and never clobbers an existing file; if the +suffix budget is exhausted the write fails loudly rather than +overwriting. The directory `benchmarks/` will be created at the repo root by the implementation PR that lands the `ourios-bench` crate. That same PR adds a `.gitignore` entry ignoring `benchmarks/results/` except for a `.gitkeep` and the specific runs the maintainer From 0e6dbbc3a61495caf89d3814ed90b92be9db43a0 Mon Sep 17 00:00:00 2001 From: Jens Holdgaard Pedersen Date: Tue, 26 May 2026 15:05:05 +0200 Subject: [PATCH 3/3] fixup! feat(bench): clap CLI + JSON results-file writer (PR-J1) Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/ourios-bench/src/report.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/ourios-bench/src/report.rs b/crates/ourios-bench/src/report.rs index 5365b3d5..70585ffd 100644 --- a/crates/ourios-bench/src/report.rs +++ b/crates/ourios-bench/src/report.rs @@ -37,7 +37,9 @@ const MAX_COLLISION_CANDIDATES: u32 = 10_000; /// /// [`BenchError::Report`] when the directory can't be created, /// the results can't be serialised, the file write fails, or -/// all [`MAX_COLLISION_CANDIDATES`] name candidates are taken. +/// all `MAX_COLLISION_CANDIDATES + 1` name candidates are +/// taken (the unsuffixed `.json` plus +/// `-1 ..= -MAX`). pub fn write_results_json( results: &ResultsFile, results_dir: &Path,