feat(bench): RFC 0031 — storage-side Loki bytes (conservative metric)

[jensholdgaard]

What

Run #5's headline (146.9×) carried a metric asymmetry worth fixing before anything lands in benchmarks.md: Loki's summary.totalBytesProcessed counts decompressed engine-side bytes, while Ourios's bytes_read counts compressed bytes fetched from storage — overstating Loki's storage reads by the chunk compression ratio.

• parse_loki_fetched_bytes — recursively sums compressedBytes / headChunkBytes across the stats tree (querier + ingester, store + head sections), resilient to layout differences across Loki versions. Zeros are legitimate (head-chunk-served queries); a missing stats block errors (the harness's standing honesty rule).
• The indicative report now prints both figures and both gate ratios, with the storage-side one (compressed + head-chunk) marked PRIMARY — the conservative, apples-to-apples number for the §9 entry. Head-chunk bytes (memory-served) are included in the sum rather than ignored: dropping them would understate Loki's data touched when a query is served from the head.
• Report extracted to a helper (too_many_lines).

Why this matters

Self-critique before anyone else does it: the honest recorded claim should survive an adversarial reader asking "did you compare compressed to decompressed?" The answer will now be printed in the run itself. Expectation: the conservative ratio lands meaningfully lower than 146.9× (likely ~20–40× depending on chunk compression) — still comfortably past the provisional 10× margin if the thesis holds.

Note for the §9 fold-in: RFC 0031 §3.6 names totalBytesProcessed as the Loki-side metric — the §9 entry PR should carry a one-line §3.6 amendment recording the storage-side refinement.

After merge

Re-dispatch → run #6 produces the refined, defensible pair of numbers.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Comparative benchmark reports now include Loki storage-read byte measurements.
  • Reports show storage-side bytes alongside total processed bytes and corresponding comparisons.
• Bug Fixes
  • Byte accounting now aggregates values across all relevant response sections.
  • Missing statistics are reported as errors instead of being silently treated as zero.
• Tests
  • Added coverage for aggregated values, valid zero-byte results, and missing statistics.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 53 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: 0d9c04cc-af63-4304-ad11-4cc294d501f6

📥 Commits

Reviewing files that changed from the base of the PR and between 1d53962 and a480091.

📒 Files selected for processing (1)

• crates/ourios-bench/src/comparative.rs

📝 Walkthrough

Walkthrough

Adds recursive parsing of Loki storage-side byte statistics and threads the resulting values through the RFC0031 comparative harness, report calculations, public exports, and unit tests.

Changes

Loki byte accounting

Layer / File(s)	Summary
Fetched-byte parser and exports crates/ourios-bench/src/comparative.rs, crates/ourios-bench/src/lib.rs	Adds LokiFetchedBytes, recursively sums Loki compressedBytes and headChunkBytes, validates the stats block, tests the parser, and re-exports the new API.
Query polling integration crates/ourios-bench/tests/rfc0031_comparative.rs	Extends Loki query results and the comparative polling loop to carry fetched bytes alongside lines and totalBytesProcessed.
Comparative report accounting crates/ourios-bench/tests/rfc0031_comparative.rs	Computes and prints storage-side and processed-byte comparisons, including component values and provisional margins.

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

Sequence Diagram(s)

sequenceDiagram
  participant Loki
  participant QueryParser
  participant ComparativeRunner
  participant IndicativeReport
  Loki->>QueryParser: Return query_range JSON
  QueryParser->>QueryParser: Parse lines, totalBytesProcessed, and fetched bytes
  QueryParser->>ComparativeRunner: Return three measurements
  ComparativeRunner->>IndicativeReport: Pass Loki and Ourios byte values
  IndicativeReport->>IndicativeReport: Compute storage-side and processed-byte comparisons

Loading

Possibly related PRs

• jensholdgaard/ourios#471: Modifies the same RFC0031 Loki query and polling flow.
• jensholdgaard/ourios#472: Adds related Loki stats parsing and comparative-harness wiring.
• jensholdgaard/ourios#474: Sets up the indicative run and parses Loki processed-byte statistics.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is substantive but does not follow the required template and is missing the Related and Checklist sections.	Reformat the PR description to use Summary, Related, and Checklist headings, add RFC/issue links, and fill out the required checklist items.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise, specific, and accurately describes the main change to Loki benchmark accounting.
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 unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch rfc0031-fetched-bytes

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Pull request overview

This PR refines the RFC 0031 comparative benchmark to report Loki “storage-side” bytes (compressed chunk bytes + head-chunk bytes) in addition to Loki’s engine-level totalBytesProcessed, so the benchmark can present a conservative apples-to-apples ratio against Ourios bytes_read.

Changes:

• Add parse_loki_fetched_bytes and LokiFetchedBytes to extract and sum compressedBytes/headChunkBytes recursively from data.stats.
• Update the RFC0031 comparative test to capture both Loki byte figures and print a “PRIMARY” gate based on the storage-side number.
• Extract the indicative report formatting into a helper (print_indicative_report) to reduce clutter.

Reviewed changes

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

File	Description
crates/ourios-bench/tests/rfc0031_comparative.rs	Captures both Loki byte metrics and prints a report with primary gating on the conservative storage-side bytes.
crates/ourios-bench/src/lib.rs	Re-exports the new Loki fetched-bytes parsing API for use by benchmarks/tests.
crates/ourios-bench/src/comparative.rs	Introduces LokiFetchedBytes + parse_loki_fetched_bytes and adds unit tests for cross-section summing and missing-stats behavior.

---

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

[coderabbitai]

🧹 Nitpick comments (1)

crates/ourios-bench/src/comparative.rs (1)

441-498: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

LGTM!

The recursive parser is clean: it correctly sums only compressedBytes/headChunkBytes, ignores decompressedBytes and other fields, treats zero as valid, and errors on missing data.stats. The doc comments explain the non-obvious "why" behind the conservative measurement choice. The test covers multi-section summation, empty stats, and missing stats.

One optional note: sum_stats_fields uses plain += for accumulation, while reference.rs uses saturating_add for its compressed_bytes(). Overflow is practically impossible for byte counts, but saturating_add would be consistent with the existing pattern and would prevent silent wrapping in release builds.

♻️ Optional: use saturating_add for consistency with reference.rs

 fn sum_stats_fields(node: &serde_json::Value, acc: &mut LokiFetchedBytes) {
     let Some(map) = node.as_object() else {
         return;
     };
     for (key, value) in map {
         match (key.as_str(), value.as_u64()) {
-            ("compressedBytes", Some(n)) => acc.compressed_bytes += n,
-            ("headChunkBytes", Some(n)) => acc.head_chunk_bytes += n,
+            ("compressedBytes", Some(n)) => acc.compressed_bytes = acc.compressed_bytes.saturating_add(n),
+            ("headChunkBytes", Some(n)) => acc.head_chunk_bytes = acc.head_chunk_bytes.saturating_add(n),
             _ => sum_stats_fields(value, acc),
         }
     }
 }

🤖 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/comparative.rs` around lines 441 - 498, Update
sum_stats_fields to use saturating_add when accumulating compressed_bytes and
head_chunk_bytes, matching the existing overflow-safe pattern in reference.rs
while preserving the current recursive traversal and field selection.

🤖 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/comparative.rs`:
- Around line 441-498: Update sum_stats_fields to use saturating_add when
accumulating compressed_bytes and head_chunk_bytes, matching the existing
overflow-safe pattern in reference.rs while preserving the current recursive
traversal and field selection.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9d1a1836-13df-493a-bd29-f7053441f28a

📥 Commits

Reviewing files that changed from the base of the PR and between 6604b0c and 1d53962.

📒 Files selected for processing (3)

• crates/ourios-bench/src/comparative.rs
• crates/ourios-bench/src/lib.rs
• crates/ourios-bench/tests/rfc0031_comparative.rs

[Copilot]

Pull request overview

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