feat(bench): clap CLI + JSON results-file writer (PR-J1) by jensholdgaard · Pull Request #53 · jensholdgaard/ourios

jensholdgaard · 2026-05-26T11:57:43Z

Summary

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. --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 outcomes are reported, not exit-gating.
src/report.rs — write_results_json serialises a ResultsFile to <results-dir>/<timestamp>-<sha>.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

cargo run -p ourios-bench -- --gates a1,c1 --hardware-kind dev-laptop on the seed corpus: writes a valid results JSON, C1 = 1.000000 PASS, audit stream produces ~5 KB (A1 audit-writer path 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.

Tests

RFC0006.5 (main.rs): --hardware-kind required unless --allow-unknown-hardware; parse-time rejection.
RFC0006.6 (main.rs): --gates scopes the measurement; default 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

--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.

Test plan

cargo fmt --all --check — clean.
cargo clippy --all-targets --all-features -- -D warnings — clean.
cargo test --all-features — 258 passed (+3 CLI, +3 results-writer).
Manual end-to-end run (above).

Maturity stage

RFC 0006 stays red until C2 + the §9 appender land.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Full CLI for running benchmarks with standard argument parsing, configurable inputs, hardware annotation, gate selection, and correctness-aware exit codes
- Results writer exposed for use by external callers
Bug Fixes / Reliability
- Safe, collision-resistant JSON result file writing with retry-on-collision
Tests
- Added CLI validation and results-file round-trip/collision tests
Documentation
- RFC updated to require colon-free timestamp filenames and explicit collision-handling policy

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 `<results-dir>/<timestamp>-<sha>.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) <noreply@anthropic.com>

coderabbitai · 2026-05-26T12:00:08Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9b0e276b-d8cb-4cdb-8b85-2dceec4aa31d

📥 Commits

Reviewing files that changed from the base of the PR and between 36f5eb3 and 0e6dbbc.

📒 Files selected for processing (1)

crates/ourios-bench/src/report.rs

📝 Walkthrough

Walkthrough

Adds a clap-based CLI for ourios-bench, maps CLI args into BenchConfig (including gate selection), runs benchmarks via run(), writes ResultsFile as pretty JSON with collision-safe filenames and retries, updates RFC 0006 filename contract, and re-exports write_results_json.

Changes

CLI and Results Reporting Implementation

Layer / File(s)	Summary
Module setup and public API `crates/ourios-bench/src/lib.rs`	RFC 0006 status comment updated; `report` module declared and `pub use report::write_results_json;` re-exported.
Cargo dependency: clap `crates/ourios-bench/Cargo.toml`	Adds `clap = { version = "4", features = ["derive"] }` dependency used by the CLI.
CLI parsing infrastructure `crates/ourios-bench/src/main.rs`	Defines `Cli` with corpus/results/bucket options, `--hardware-kind` requirement logic, `--allow-unknown-hardware`, `--update-benchmarks-md`, and `--gates`.
Gate selection and configuration `crates/ourios-bench/src/main.rs`	Introduces `Gate` enum and implements `Cli::gate_set()` and `Cli::into_config()` to produce a `BenchConfig` (empty `--gates` ⇒ all gates).
Benchmark execution and output `crates/ourios-bench/src/main.rs`	Replaces `main()` with CLI parsing, `run_bench()` orchestration, correctness-gated exit codes (fail on C1 non-lossy mismatch), human-readable per-gate summary, and unit tests for CLI validation and option semantics.
JSON results writing and output handling `crates/ourios-bench/src/report.rs`	Adds `write_results_json()` that serializes `ResultsFile` to pretty JSON, uses a colon-free `<timestamp>-<git_sha>` stem, and performs atomic `create_new` writes with `-N` suffix retry on collisions; includes `file_stem()` helper and tests for round-trip, filename format, and collision behavior.
RFC filename contract `docs/rfcs/0006-bench-harness.md`	Updates §3.6 and related text to require colon-free timestamp filenames and specify atomic create-new with numeric suffix collision handling (`[-N]`).

Sequence Diagram

sequenceDiagram
  participant CLI
  participant Clap as ClapParse
  participant IntoCfg as Cli::into_config
  participant BenchConfig
  participant Runner as ourios_bench::run
  participant ResultsFile
  participant Writer as write_results_json
  participant Printer as print_summary
  participant Exit as ExitCode

  CLI->>Clap: parse args (clap derive)
  Clap->>IntoCfg: validate & convert
  IntoCfg->>BenchConfig: produce BenchConfig
  BenchConfig->>Runner: execute selected gates
  Runner->>ResultsFile: return ResultsFile
  ResultsFile->>Writer: serialize & attempt atomic write (create_new)
  Writer-->>CLI: return PathBuf
  ResultsFile->>Printer: produce human summary
  Printer-->>CLI: print summary and warnings
  CLI->>Exit: exit (1 on C1 failure, 2 on run error, 0 otherwise)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

jensholdgaard/ourios#49: Scaffolding PR for ourios-bench that this change extends into a full CLI and reporting implementation.
jensholdgaard/ourios#50: Related deferred JSON round-trip/reporting work that this PR implements via write_results_json.
jensholdgaard/ourios#51: Implementation of C1 reconstruction pipeline whose outputs are used for correctness-gated exit behavior here.

Poem

🐰 I parsed the flags with careful care,

gates marched onward through the harness air,
JSON tucked safe with colons turned to dashes,
retries danced so files need not clash,
a rabbit cheers — benchmarks now write in flashes.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: adding a clap-based CLI and JSON results-file writer to the bench binary.
Description check	✅ Passed	The PR description covers all required sections: Summary, Related (RFC links), and a comprehensive Checklist showing format/linting/testing/documentation completion.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/ourios-bench-cli

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ourios-bench/src/report.rs`:
- Around line 39-51: The current check-then-write in write_results_json is
TOCTOU-unsafe and can clobber an existing file; instead, attempt to atomically
create the file with
std::fs::OpenOptions::new().write(true).create_new(true).open(&path) inside the
same counter loop (retrying on Err(e) if e.kind() ==
io::ErrorKind::AlreadyExists by incrementing counter and recomputing path),
write the serialized serde_json::to_string_pretty(results) into the opened file,
map any non-AlreadyExists IO error to BenchError::Report (include path.display()
and the error), and if the loop exhausts all candidates return a
BenchError::Report indicating exhaustion; reference symbols: write_results_json,
results_dir, stem, counter, BenchError::Report, serde_json::to_string_pretty,
and std::fs::OpenOptions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5de0c191-d195-4932-be70-0e62f1951e60

📥 Commits

Reviewing files that changed from the base of the PR and between d1f4045 and 7eac046.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (4)

crates/ourios-bench/Cargo.toml
crates/ourios-bench/src/lib.rs
crates/ourios-bench/src/main.rs
crates/ourios-bench/src/report.rs

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.

coderabbitai Bot reviewed May 26, 2026

View reviewed changes

Comment thread crates/ourios-bench/src/report.rs Outdated

jensholdgaard requested a review from Copilot May 26, 2026 12:18

Copilot started reviewing on behalf of jensholdgaard May 26, 2026 12:21 View session

Copilot AI reviewed May 26, 2026

jensholdgaard requested a review from Copilot May 26, 2026 12:22

Copilot started reviewing on behalf of jensholdgaard May 26, 2026 12:22 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

Comment thread crates/ourios-bench/src/report.rs Outdated

Comment thread crates/ourios-bench/src/report.rs Outdated

Comment thread crates/ourios-bench/src/report.rs

Comment thread crates/ourios-bench/src/main.rs

jensholdgaard requested a review from Copilot May 26, 2026 12:48

Copilot started reviewing on behalf of jensholdgaard May 26, 2026 12:48 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

Comment thread crates/ourios-bench/src/report.rs Outdated

Comment thread crates/ourios-bench/src/report.rs Outdated

fixup! feat(bench): clap CLI + JSON results-file writer (PR-J1)

36f5eb3

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jensholdgaard requested a review from Copilot May 26, 2026 12:56

Copilot started reviewing on behalf of jensholdgaard May 26, 2026 12:56 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

Comment thread crates/ourios-bench/src/report.rs Outdated

fixup! feat(bench): clap CLI + JSON results-file writer (PR-J1)

0e6dbbc

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jensholdgaard requested a review from Copilot May 26, 2026 13:21

Copilot started reviewing on behalf of jensholdgaard May 26, 2026 13:22 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

jensholdgaard merged commit b75b81c into main May 26, 2026
10 checks passed

This was referenced May 26, 2026

feat(bench): §9 benchmarks.md results appender (PR-J3) #55

Merged

feat(bench): close RFC 0006 — reproducibility + forged-mismatch, flip to green (PR-J4) #56

Merged

ci(bench): add on-demand thesis-gate bench workflow (PR-K1) #57

Merged

coderabbitai Bot mentioned this pull request Jul 5, 2026

feat(bench): rfc 0024 green — --calibrate pass discharges §5.1/.2 #361

Merged

Conversation

jensholdgaard commented May 26, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Lands

Verified end-to-end

Tests

Not in this PR

Test plan

Maturity stage

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jensholdgaard commented May 26, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 26, 2026 •

edited

Loading