feat(bench): land C2 template-count convergence (PR-J2) - #54
Conversation
The third and final writer-side thesis gate. C2 measures whether the template count plateaus within 2× of its steady-state by 1 M lines (§3.4.3). All three gates (A1, C1, C2) now run in any combination in a single miner pass. Key design: the live template count at any point equals the number of distinct non-NO_TEMPLATE `template_id`s seen so far — templates are monotonic (never unmerged) and every template is stamped onto the records of the lines that use it. So C2 is a pure stream accumulator over the harness callback (like C1), needing no cluster or harness change. Lands: - `src/c2.rs` — `C2Accumulator`: tracks distinct template ids, samples the count at the §3.4.3 cadence (N = max(1, ceil(total_lines / 1024)), curve ≤ 1024 points, final line always sampled), and at finalize computes SS (end count), count-at-1M (sample closest to line 1_000_000, floor tie-break), convergence ratio, and pass (ratio ≥ 0.5 on ≥ 1 M-line corpora; abstain → pass=None below 1 M). - `src/lib.rs` — `run()` drops the C2 NotImplemented guard and wires the C2 accumulator into the shared miner pass. The obsolete `c2_still_returns_not_implemented` marker test is replaced by `no_gates_enabled_is_a_cli_error` (the remaining gate-selection contract). Crate / `run` / `BenchError` docs updated to PR-J2 status. - `tests/c2.rs` — `rfc0006_3_c2_abstains_on_short_corpus` un-ignored (seed corpus, fast, real miner → abstain). The ≥ 1 M-line end-to-end (`rfc0006_3_c2_gate_passes_on_stable_corpus`) gets a real bounded-alphabet generator but stays `#[ignore]`'d as a heavy on-demand check (>1M miner ingests; §3.7 "bench runs on-demand, not per-PR"). Verified passing via `--ignored` (0.77s release). Test coverage: - `src/c2.rs` colocated unit tests drive the convergence math at ≥ 1 M-line scale fast (synthetic ids, no miner/disk): cadence/curve-length, stable-corpus pass, short-corpus abstain, non-converging (linear growth) fail. - `tests/c2.rs` abstention runs the real miner end-to-end. Now a default all-gates CLI run completes on the seed corpus (A1 FAIL on the tiny corpus as expected, C1 PASS, C2 ABSTAIN) instead of erroring on C2. Verification (CLAUDE.md §6.6): - cargo fmt --all --check — clean. - cargo clippy --all-targets --all-features -- -D warnings — clean. - cargo test --all-features — 263 passed / 21 ignored (was 258; +4 c2 unit tests + 1 un-ignored abstention − the removed marker test, net new C2 coverage). - The heavy ≥1M C2 test passes via `cargo test -- --ignored`. Maturity gate: with all three writer-side gates green, the only remaining RFC 0006 work is the `docs/benchmarks.md` §9 markdown appender (the `--update-benchmarks-md` path) and the RFC0006.7 reproducibility test. The `specified → green` flip follows once those land. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughImplements the C2 gate: adds C2Accumulator to sample distinct-template counts during mining, wires accumulator into the harness ingest path, finalizes C2Result after mining, updates gate validation/docs, and adds unit and integration tests exercising stable and short corpora. ChangesC2 Gate Implementation & Harness Integration
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Implements the RFC 0006 writer-side C2 gate (template-count convergence) in ourios-bench, wiring it into the existing single-pass harness so A1/C1/C2 can run in any combination from one miner ingest.
Changes:
- Added
C2Accumulatorstream accumulator and convergence metric computation (sampling cadence, SS, count@1M, ratio, pass/abstain). - Updated
run()to enable C2, removed the priorNotImplementedguard, and adjusted the “no gates enabled” contract/test. - Updated C2 integration tests: real-miner short-corpus abstention now runs by default; ≥1M stable-corpus E2E remains
#[ignore]with an on-demand generator.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/ourios-bench/src/c2.rs | New C2 accumulator implementation + colocated unit tests for cadence/ratio math |
| crates/ourios-bench/src/lib.rs | Wires C2 into the shared miner pass; updates docs and gate-selection test |
| crates/ourios-bench/tests/c2.rs | Updates C2 integration tests (heavy stable-corpus generator; un-ignores short-corpus abstention) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/ourios-bench/src/c2.rs (1)
227-249: 💤 Low valueTest name and assertion don't match actual behavior.
With 2M lines and linear growth (one new template per line), the closest sample to 1M is at ~1,000,448 lines (cadence ≈ 1954), giving:
count_1m ≈ 1,000,448template_count_at_end = 2,000,000ratio ≈ 0.5002Since 0.5002 ≥ 0.5,
pass == Some(true). The test name says "fails_the_gate" but linear growth lands at the pass boundary. The comment mentions "push it under by minting faster in the back half" but that logic isn't implemented.Consider either:
- Rename to
linear_growth_at_boundaryand addassert_eq!(r.pass, Some(true));to document the actual boundary behavior, or- Mint faster in the second half (e.g., 2 templates/line after 1M) to actually fail the gate and assert
pass == Some(false).🤖 Prompt for 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. In `@crates/ourios-bench/src/c2.rs` around lines 227 - 249, The test's name/assertion doesn't match behavior: linear growth (one new template per line) with total=2_000_000 actually yields pass == Some(true). Rename the test from non_converging_curve_fails_the_gate to linear_growth_at_boundary (or similar), and update the assertions to reflect reality by adding assert_eq!(r.pass, Some(true)); (keep the existing ratio check). Update the test comment to remove the incorrect "fails the gate" wording and reference C2Accumulator::new, C2Accumulator::observe, C2Accumulator::finalize, and r.pass so future readers understand the observed boundary behavior.
🤖 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.
Nitpick comments:
In `@crates/ourios-bench/src/c2.rs`:
- Around line 227-249: The test's name/assertion doesn't match behavior: linear
growth (one new template per line) with total=2_000_000 actually yields pass ==
Some(true). Rename the test from non_converging_curve_fails_the_gate to
linear_growth_at_boundary (or similar), and update the assertions to reflect
reality by adding assert_eq!(r.pass, Some(true)); (keep the existing ratio
check). Update the test comment to remove the incorrect "fails the gate" wording
and reference C2Accumulator::new, C2Accumulator::observe,
C2Accumulator::finalize, and r.pass so future readers understand the observed
boundary behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a6e5ac1-5141-4ce8-b17c-199f1134021a
📒 Files selected for processing (3)
crates/ourios-bench/src/c2.rscrates/ourios-bench/src/lib.rscrates/ourios-bench/tests/c2.rs
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
The third and final writer-side thesis gate. C2 measures whether the template count plateaus within 2× of its steady-state value by 1 M lines (§3.4.3). All three gates (A1, C1, C2) now run in any combination in a single miner pass.
Key design: the live template count at any point equals the number of distinct non-
NO_TEMPLATEtemplate_ids seen so far — templates are monotonic (never unmerged) and every template is stamped onto the records of the lines that use it. So C2 is a pure stream accumulator over the harness callback (like C1), needing no cluster or harness change.Lands
src/c2.rs—C2Accumulator: tracks distinct template ids, samples the count at the §3.4.3 cadence (N = max(1, ceil(total_lines / 1024)), curve ≤ 1024 points, final line always sampled), and at finalize computes SS (end count), count-at-1M (sample closest to line1_000_000, floor tie-break), convergence ratio, and pass (ratio ≥ 0.5on ≥ 1 M-line corpora; abstain →pass=Nonebelow 1 M).src/lib.rs—run()drops the C2NotImplementedguard and wires the C2 accumulator into the shared miner pass. The obsoletec2_still_returns_not_implementedmarker test is replaced byno_gates_enabled_is_a_cli_error. Crate /run/BenchErrordocs updated to PR-J2 status.tests/c2.rs—rfc0006_3_c2_abstains_on_short_corpusun-ignored (seed corpus, fast, real miner → abstain). The ≥ 1 M-line end-to-end test gets a real bounded-alphabet generator but stays#[ignore]'d as a heavy on-demand check (>1 M miner ingests; consistent with §3.7 "bench runs on-demand, not per-PR"). Verified passing via--ignored(0.77 s release).Test coverage
src/c2.rscolocated unit tests drive the convergence math at ≥ 1 M-line scale fast (synthetic ids, no miner/disk): cadence/curve-length, stable-corpus pass, short-corpus abstain, non-converging (linear growth) fail.tests/c2.rsabstention runs the real miner end-to-end.A default all-gates CLI run now completes on the seed corpus (A1 FAIL on the tiny corpus as expected, C1 PASS, C2 ABSTAIN) instead of erroring on C2.
Test plan
cargo fmt --all --check— clean.cargo clippy --all-targets --all-features -- -D warnings— clean.cargo test --all-features— 263 passed / 21 ignored (+4 C2 unit tests, +1 un-ignored abstention, −1 removed marker).cargo test -p ourios-bench -- --ignored.Maturity stage
With all three writer-side gates green, the only remaining RFC 0006 work is the
docs/benchmarks.md§9 markdown appender (--update-benchmarks-md) and the RFC0006.7 reproducibility test. Thespecified → greenflip follows once those land.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Bug Fixes