Skip to content

feat(bench): RFC 0031 — bytes-read measurement channel (§3.6) - #472

Merged
jensholdgaard merged 1 commit into
mainfrom
rfc0031-bytes-channel
Jul 11, 2026
Merged

feat(bench): RFC 0031 — bytes-read measurement channel (§3.6)#472
jensholdgaard merged 1 commit into
mainfrom
rfc0031-bytes-channel

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jul 11, 2026

Copy link
Copy Markdown
Owner

What

Both halves of the RFC 0031 §3.6 primary gate metric — bytes read from storage — the measurement the RFC0031.2–.5 must-win gates ratio. Locally verified end to end.

Ourios half (mostly already existed): QueryStats.bytes_read is folded from the engine's bytes_scanned scan metric on the RFC 0016 path and already live-asserted in the querier's execution tests. This PR exposes it through the comparative harness: OuriosAnswer { lines, bytes_read } via ourios_query_answer — one query yields both the equivalence lines and the measurement. ourios_query_lines becomes a thin wrapper, so the merged RFC0031.1 container test is untouched.

Loki half: parse_loki_bytes_processed reads data.stats.summary.totalBytesProcessed from the same query_range response the streams parser consumes, sharing an extracted parse_loki_root error-response guard (no duplicated Loki-error handling).

Honesty choice

A missing Loki stats block is an error, not a silent 0 — a zero bytes_read would fake a perfect pruning ratio in the L-gates. Same principle as the RFC0031.1 completeness guard.

Tests

  • The fixture round-trip now runs through ourios_query_answer and asserts bytes_read > 0 against the real store (the metric can't silently be 0 for a query that scanned data).
  • The bytes parser covers the value, missing-summary, and Loki-error-response cases.

cargo test -p ourios-bench --lib comparative → 14 passed; fmt + clippy clean.

Next

With both measurement halves in place: the L-gate ratio math (where the §7 must-win margins M_L1..L4 land) → OTel-Demo-scale corpus → the first indicative Ourios-vs-Loki bytes-read number via a workflow_dispatch run.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Comparative benchmarks now report both matching results and storage bytes read.
    • Added parsing for Loki’s total bytes processed metric.
  • Bug Fixes
    • Loki error responses now surface their diagnostic messages.
    • Missing or invalid Loki statistics are rejected instead of silently defaulting.
  • Tests
    • Added coverage for byte-processing metrics, error responses, and storage-read reporting.

Both halves of the RFC 0031 §3.6 primary gate metric, feeding the
RFC0031.2–.5 bytes-read ratios.

Ourios half (mostly existed): QueryStats.bytes_read is already folded
from the engine's bytes_scanned scan metric on the RFC 0016 path and
live-asserted in the querier's execution tests. Expose it through the
comparative harness: `OuriosAnswer { lines, bytes_read }` via
`ourios_query_answer` — one query yields both the equivalence lines and
the measurement; `ourios_query_lines` stays as a thin wrapper (the
RFC0031.1 container test is untouched).

Loki half: `parse_loki_bytes_processed` reads
`data.stats.summary.totalBytesProcessed` from the query_range response,
sharing the extracted `parse_loki_root` error-response guard with
`parse_loki_streams`. A missing stats block is an ERROR, not a silent 0
— a zero would fake a perfect pruning ratio in the L-gates.

Tests: the fixture round-trip now runs through ourios_query_answer and
asserts bytes_read > 0 on the real store; the bytes parser covers the
value, missing-summary, and Loki-error-response cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jensholdgaard
jensholdgaard requested a review from Copilot July 11, 2026 15:44
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review Change Stack

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: e130129e-3082-4ff9-9c1a-825544661ad5

📥 Commits

Reviewing files that changed from the base of the PR and between b26fb7b and 12d1527.

📒 Files selected for processing (2)
  • crates/ourios-bench/src/comparative.rs
  • crates/ourios-bench/src/lib.rs

📝 Walkthrough

Walkthrough

The comparative benchmark query API now returns matched lines and storage bytes read. Loki parsing shares root-level error handling and adds validated extraction of processed-byte statistics, with updated public exports and tests.

Changes

Comparative benchmarking updates

Layer / File(s) Summary
Ourios answer API
crates/ourios-bench/src/comparative.rs, crates/ourios-bench/src/lib.rs
Adds OuriosAnswer, captures bytes_read, preserves the line-only wrapper, and re-exports the new public symbols.
Loki metrics parsing
crates/ourios-bench/src/comparative.rs
Centralizes Loki root parsing and adds validated extraction of totalBytesProcessed.
Comparative validation
crates/ourios-bench/src/comparative.rs
Updates fixture assertions and tests successful, missing-statistics, and Loki-error parsing cases.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • jensholdgaard/ourios#467: Establishes related LineKey comparison types and result-comparison harnesses used by these benchmark changes.
  • jensholdgaard/ourios#468: Modifies the same Ourios comparative query extraction path later wrapped by ourios_query_answer.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is on-topic but misses the required Summary, Related, and Checklist sections from the repository template. Add the missing template sections: Summary, Related (issue/RFC link), and Checklist with fmt, clippy, tests, docs, and RFC linkage status.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately captures the main RFC 0031 bytes-read measurement channel change.
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.
✨ 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 rfc0031-bytes-channel

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.

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 extends the ourios-bench comparative harness to carry and validate the RFC 0031 §3.6 “bytes read” measurement for both Ourios and Loki, enabling future L-gate pruning ratio calculations alongside existing equivalence checks.

Changes:

  • Introduces OuriosAnswer { lines, bytes_read } and ourios_query_answer, with ourios_query_lines becoming a thin wrapper.
  • Adds Loki-side parse_loki_bytes_processed and factors shared Loki error-response handling into parse_loki_root.
  • Updates comparative tests to assert the Ourios bytes-read measurement is non-zero and adds focused tests for Loki bytes parsing/error handling.

Reviewed changes

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

File Description
crates/ourios-bench/src/lib.rs Re-exports the new comparative harness API (OuriosAnswer, ourios_query_answer, parse_loki_bytes_processed).
crates/ourios-bench/src/comparative.rs Adds the Ourios bytes-read measurement channel and Loki bytes-processed parsing, with updated tests to cover both.

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

@jensholdgaard
jensholdgaard merged commit d535099 into main Jul 11, 2026
27 checks passed
@jensholdgaard
jensholdgaard deleted the rfc0031-bytes-channel branch July 11, 2026 15:49
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