Skip to content

feat(bench): RFC 0031 L4 harness — aggregation pair, matrix parser, param picker - #534

Merged
jensholdgaard merged 6 commits into
mainfrom
rfc0031-l4-harness
Jul 15, 2026
Merged

feat(bench): RFC 0031 L4 harness — aggregation pair, matrix parser, param picker#534
jensholdgaard merged 6 commits into
mainfrom
rfc0031-l4-harness

Conversation

@jensholdgaard

Copy link
Copy Markdown
Owner

What

The L4 harness fixture-level slice for RFC 0031 — RFC0031.5 un-stubbed. Base: rfc0002-l4-green-execution (in review as #533 — this PR is stacked and will need a rebase once that merges).

  • ourios_aggregate_answer: runs template_id == {id} | count by param({p}), bucket({w}) and decodes QueryResult.aggregate into an AggKey → count map; bytes_read is the grouped-count scan total (RFC 0002 §6.5 guarantees zero materialize/registry components on this path).
  • parse_loki_matrix: decodes Loki/Prometheus resultType: "matrix" responses into the same AggKey shape. Bucket alignment documented: count_over_time(range[w]) at eval instant t covers (t-w, t], so evaluating at t = (k+1)·w yields the Ourios-equivalent bucket [k·w, (k+1)·w).
  • pick_frequency_pair: a fourth corpus pass (mirrors pick_template_pair) choosing (template_id, param_position, bucket_width) where distinct group values fall in 2..=50 and rows ≥ 4 — rejecting both single-value and kafka-style per-line-unique slots. Every rejection is a loud eprintln!, no silent fallthrough.
  • PairClass::L4 added with GateKind::MustWin; frozen_gate_failures explicitly skips it and the report prints both byte channels labeled "§7 DEFERRED — reported only" — M_L4 stays unfrozen until a real dispatch measures it.

⚠️ Known gap — NOT live-wired into the dispatch loop yet

This slice proves the machinery at the fixture level (a hand-built synthetic Loki matrix response, no container). PairClass::L4 is not yet built into class_pair_specs/the live measurement loop in rfc0031_indicative_comparative_run — a dispatch today still won't touch L4. That wiring is the next slice; flagging it here so nothing assumes L4 is measurable yet.

Checks run

cargo fmt --all --check; workspace cargo clippy --all-targets --all-features -- -D warnings; strict rustdoc on ourios-bench; cargo nextest run -p ourios-bench — 165 passed, 7 skipped (all pre-existing container/dispatch-only ignores). New dep: chrono (already transitively pinned via ourios-querier) added directly for RFC 3339 bucket-key decoding.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@jensholdgaard, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 10 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7007b068-3453-4c77-8a6a-64583ae7c6aa

📥 Commits

Reviewing files that changed from the base of the PR and between 2b46313 and be74f30.

⛔ 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/comparative.rs
  • crates/ourios-bench/src/lib.rs
  • crates/ourios-bench/tests/rfc0031_comparative.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rfc0031-l4-harness

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…aram picker

Un-stubs RFC0031.5 as a fixture-level test: adds ourios_aggregate_answer
(count-by-param(n)-bucket(w) -> AggKey map + bytes) and parse_loki_matrix
(Prometheus matrix decoding, bucket-aligned on eval instant t=(k+1)*w) to
ourios-bench's comparative harness, plus a fourth post-store-build picker
(pick_frequency_pair) that tallies per-template param-slot cardinality to
choose a moderate-cardinality (2..=50 values, >=4 rows) L4 candidate. The
scenario test wires these against a hand-built Loki matrix response (no
container) and asserts compare_aggregations equivalence; M_L4 stays
§7-deferred, so the bytes ratio is printed, never asserted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jensholdgaard
jensholdgaard marked this pull request as ready for review July 15, 2026 09:50
@jensholdgaard
jensholdgaard changed the base branch from rfc0002-l4-green-execution to main July 15, 2026 09:51
@jensholdgaard
jensholdgaard requested a review from Copilot July 15, 2026 09:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR un-stubs the RFC 0031 L4 (frequency aggregation) comparative harness slice in ourios-bench, adding machinery to (1) execute Ourios count by param(n), bucket(w) queries, (2) parse Loki/Prometheus resultType: "matrix" responses into the same grouped-count shape, and (3) pick a fixture-derived L4 (template_id, param, bucket_width) candidate for deterministic equivalence checks.

Changes:

  • Add OuriosAggregateAnswer + ourios_aggregate_answer and Loki matrix parsing (parse_loki_matrix) for L4 grouped-count equivalence.
  • Add L4 pair picker utilities (pick_frequency_pair, pick_bucket_width, regex capture construction) and a new RFC0031.5 fixture-level test.
  • Wire PairClass::L4 into the harness’ reporting/gating shape (explicitly deferred for frozen-gate evaluation).

Reviewed changes

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

Show a summary per file
File Description
crates/ourios-bench/tests/rfc0031_comparative.rs Adds RFC0031.5 L4 fixture test, L4 pair picking utilities, and integrates PairClass::L4 into reporting/skip logic.
crates/ourios-bench/src/lib.rs Re-exports new L4 comparative helpers/types from comparative.
crates/ourios-bench/src/comparative.rs Implements Ourios aggregation extraction (ourios_aggregate_answer) and Loki matrix parsing (parse_loki_matrix) + associated tests.
crates/ourios-bench/Cargo.toml Adds a direct chrono dependency for RFC3339 bucket-key decoding.
Cargo.lock Records the new chrono dependency for ourios-bench.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/ourios-bench/tests/rfc0031_comparative.rs
Comment thread crates/ourios-bench/src/comparative.rs
…nest bytes

param_capture_regex now returns None when neither wildcard neighbour is
a fixed token, instead of emitting an unanchored (?P<value>\S+) that
would match any token and silently corrupt L4 equivalence input.
ourios_aggregate_answer now asserts materialize/registry bytes are
zero per the RFC 0002 §6.5 aggregation contract, rather than assuming
it and risking a silent bytes_read underreport on regression.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread crates/ourios-bench/src/comparative.rs
Loki/Prometheus matrix sample values are always a stringified float
("1", "1.0", "1e+00" are all legal integer encodings); a strict
u64::from_str rejected valid responses. Parse as f64 with the same
finite/non-negative/integral guard the timestamp decode already uses,
plus an explicit u64-range check. Regression test confirmed via
revert: fails without the fix, passes with it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread crates/ourios-bench/src/comparative.rs Outdated
…ix-count check

u64::MAX rounds UP to 2^64 in f64 (not exactly representable), so
'count_f64 > u64::MAX as f64' let a count of exactly 2^64 through —
and the float-to-int cast saturates on overflow rather than erroring,
silently corrupting the decoded count to u64::MAX. Compares against
2f64.powi(64) (exact) with >= instead. Regression test confirmed via
revert: the buggy check saturates to u64::MAX on this input.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread crates/ourios-bench/tests/rfc0031_comparative.rs Outdated
…imension

pick_frequency_pair could select a candidate whose rows all land in
one bucket window, leaving the (bucket, group_key) -> count
equivalence shape's bucket dimension untested and parse_loki_matrix's
bucket-alignment convention unexercised on the corpus. Extracted the
three shape floors (cardinality, row count, now bucket diversity >= 2)
into frequency_shape_rejection, which also brought pick_frequency_pair
back under the 100-line limit. New rejection test with a fixture whose
rows all land in one 1s bucket.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@jensholdgaard
jensholdgaard merged commit 44c292e into main Jul 15, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants