docs(rfc0036): validate — in-repo before/after (1.43×) + baseline no-regression (§9.26/§9.27)

[jensholdgaard]

Flips RFC 0036 → validated on the resolved comparative arm (the user chose "B + cheap in-repo before/after" after the baseline run surfaced that the comparative harness can't exercise RFC 0036).

What landed

1. In-repo before/after test (rfc0036_2_materialization_before_after, ourios-querier) — the RFC0036.2 materialization diagnostic on a genuinely-compacted store: the sort takes a one-service window from materialising the whole hour (unsorted, 1/1 groups, 100.5 MB) to a contiguous minority (sorted, 2/6 groups, 70.0 MB) for the identical 100-row answer — a 1.43× materialization-bytes win, measured from footer survivor-chunk bytes (not the misleading count-scan bytes_read). Modest and honest: the compacted file is ~2× larger on disk, and §2.2's registry floor is why the gate is the scanned-row-group bound (enforced by the untouched rfc0036_2_window_materialization_bound).
2. §9.26 (authoritative baseline-8vcpu-32gib, HEAD 5e5aa66) — the frozen RFC 0031 dispatch reran on post-RFC-0036 main: every measured frozen gate passes in §9.24's band (no regression). Key finding: build_comparative_store writes one ingest file per partition → compact_partition no-ops → RFC 0036's sort never runs → every ourios_bytes_read is byte-identical to §9.24. A multi-file-per-partition harness (which would re-base RFC 0031's frozen-gate store) is future work, not a validated blocker.
3. §9.27 — the in-repo before/after record. Plus a stale §9.25 test-name fix (the forced_spill rename).
4. RFC 0036 status green → validated with the evidence inline; accepted is a maintainer flip.

Invariants / hazards

Hazard #4 (small-file / pruning granularity) — the 1.43× win is the §3.3 pruning-granularity-over-bytes trade, made visible. No schema change; no production code touched (test + docs only).

Verification

• cargo fmt --all --check, cargo clippy --all-targets --all-features -- -D warnings, cargo doc --workspace --no-deps --all-features, mdbook build — all exit 0
• cargo test -p ourios-querier --all-features --test it rfc0036_2 — 2 passed; before/after numbers reproduced (100,520,155 → 70,018,075 bytes, 1.43×)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 25 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: 0ae31e9c-83f7-4858-b1d5-0309fd5b7058

📥 Commits

Reviewing files that changed from the base of the PR and between 6ec8345 and f75d80c.

📒 Files selected for processing (3)

• crates/ourios-querier/tests/it/rfc0036_window_materialization.rs
• docs/benchmarks.md
• docs/rfcs/0036-write-side-layout.md

📝 Walkthrough

Walkthrough

Changes

The RFC0036.2 integration test now creates an uncompacted baseline, measures predicted and live row-group scanning, and compares it with a compacted store for identical query results, pruning, and materialization bytes.

RFC0036 materialization validation

Layer / File(s)	Summary
Dataset and window measurement crates/ourios-querier/tests/it/rfc0036_window_materialization.rs	Adds round-robin ingest data generation and a helper that compares footer-predicted survivor row groups and bytes with live query scan statistics.
Before-and-after materialization test crates/ourios-querier/tests/it/rfc0036_window_materialization.rs	Adds coverage comparing uncompacted and compacted stores, asserting equal rows, full scanning before compaction, pruning after compaction, and lower materialization bytes after compaction.

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

Possibly related PRs

• jensholdgaard/ourios#586: Introduces the RFC0036.2 window-materialization test area expanded by this PR.
• jensholdgaard/ourios#590: Adds related row-group pruning and footer-derived materialization checks in the same test file.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title is specific and matches the PR’s main change: validating RFC0036 with an in-repo before/after materialization comparison.
Description check	✅ Passed	The description is detailed and covers the change, rationale, and verification, though it does not follow the repository’s exact template headings.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch rfc0036-materialization-beforeafter

---

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

Advances RFC 0036 (“write-side layout”) from green to validated by adding in-repo before/after evidence for the RFC0036.2 materialization-bytes diagnostic, and records the baseline comparative rerun results (showing no regression and documenting that the current comparative harness doesn’t exercise the RFC 0036 sorting path).

Changes:

• Mark RFC 0036 as validated and document the supporting baseline + in-repo evidence.
• Add benchmark write-up sections §9.26 (baseline rerun + harness finding) and §9.27 (in-repo before/after).
• Add an integration test that measures RFC0036.2 window-materialization before/after on an uncompacted vs compacted store.

Reviewed changes

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

File	Description
docs/rfcs/0036-write-side-layout.md	Updates RFC 0036 status to validated and embeds the new evidence narrative.
docs/benchmarks.md	Adds benchmark result sections §9.26/§9.27 and fixes the forced-spill test name reference.
crates/ourios-querier/tests/it/rfc0036_window_materialization.rs	Adds a deterministic in-repo before/after materialization-bytes test and shared measurement helper.

---

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

crates/ourios-querier/tests/it/rfc0036_window_materialization.rs:432

• The “before” description says the uncompacted store’s row group(s) span “the whole hour”, but in this fixture the important property is that the uncompacted parquet is a single row group spanning all services (and therefore can’t be pruned for this query). Rewording makes the invariants match what the test actually asserts (before_total == 1).

///   rotation, no `sorting_columns`. Every row group spans the whole
///   hour and every service, so nothing prunes: the window query
///   materialises the *entire* file (`scanned == total`).

crates/ourios-querier/tests/it/rfc0036_window_materialization.rs:481

• This comment says the uncompacted ingest file’s row group(s) span “the whole hour”, but the test is really relying on “no pruning” because the uncompacted store is a single row group covering all services (and overlapping the window). Rewording avoids a misleading “hour” claim and matches the explicit before_total==1 assertion just below.

    // the row groups a `service == TARGET AND time ∈ window` scan can NOT
    // prune; on the unsorted ingest file every row group spans the whole hour
    // and every service, so none prune — the survivors are the *whole file*. ---

[Copilot]

Pull request overview

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