feat(parquet): store-backed Writer + Reader ctors for the RFC 0019 compactor

[jensholdgaard]

What

The foundational data-seam piece of the RFC 0019 compactor migration (slice 2b). The compactor must read inputs and write the consolidated file through a configured Store (local or S3), but Writer::open / Reader::open_partition construct Store::local internally. This adds the S3-capable seam additively — the live ingest path (RecordSink) is already on Store; this brings the compactor's writer/reader along.

• Writer::open_in(store: &Store, partition) (+ open_in_with_zstd_level) — opens a writer on an already-built Store. Writer::open(bucket_root, partition) now delegates to it after building the local store, so the ~40 local Writer::open(path, …) call sites are untouched (the no-churn precedent from feat(querier): migrate the audit + scan read paths onto Store (rfc0019 2a) #292's Querier::new).
• WrittenFile gains key + bytes_written — the backend-agnostic address and size, so the compactor can name the consolidated file and report bytes without stat-ing a local path (which an S3 writer can't do). The existing path field stays (absolute for local; the key rendered as a path for the store ctor).
• Reader::open_partition_bytes(bytes, partition) — the store-backed counterpart of open_partition (read via Store::get_blocking → these bytes), keeping the RFC 0005 §3.9 row-vs-path validation so a mis-partitioned compaction input still aborts (RFC0009.5).

Why

Independently-mergeable prereq for PR B (compaction.rs + compactor.rs + main.rs onto Store), so that data compaction can target S3. Pairs with #293 (Store list_with_sizes_blocking / delete_blocking).

Invariants

• §3.9 / RFC0009.5 row-vs-path validation is preserved on the new bytes reader (test: open_partition_bytes_rejects_a_mismatched_partition).
• §3.5 no on-disk format change — same schema/encoding; the store ctor only changes where the bytes are put.
• §3.4 atomic-publish convention unchanged (local temp-rename; S3 put is atomic).

Tests

open_in_writes_through_a_store_and_reports_key_and_size (store round-trip, key/bytes_written correctness) and open_partition_bytes_rejects_a_mismatched_partition (§3.9 abort). All existing ourios-parquet suites pass unchanged.

Local gate

cargo fmt --all --check, cargo clippy -p ourios-parquet --all-targets --all-features -D warnings, RUSTDOCFLAGS=-D warnings cargo doc -p ourios-parquet, cargo test -p ourios-parquet, cargo check --workspace --all-targets — all green.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added support for opening Parquet data from in-memory bytes when the partition is already known.
  • Improved file-writing support for storage-backed outputs, including clearer object naming and byte-size reporting.

• Bug Fixes

  • Validation now happens earlier when using invalid compression settings.
  • Reading partitioned data now better detects mismatches between the expected and actual partition.

• Documentation

  • Updated output-file details to better explain local vs storage-backed paths.

[coderabbitai]

Warning

Review limit reached

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

More reviews will be available in 33 minutes and 34 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

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 credits.

🚦 How do rate 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 see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d7fe4155-b561-41c0-a458-b1f988d91bb1

📥 Commits

Reviewing files that changed from the base of the PR and between 3e8d212 and 132f92b.

📒 Files selected for processing (2)

• crates/ourios-parquet/src/reader.rs
• crates/ourios-parquet/src/writer.rs

📝 Walkthrough

Walkthrough

Adds WrittenFile.key and WrittenFile.bytes_written to the writer's close result, refactors Writer::open_in path construction and ZSTD validation ordering, introduces Reader::open_partition_bytes for opening Parquet from in-memory bytes under a known PartitionKey, and adds two integration tests covering store-backed write and partition mismatch rejection.

Writer and Reader store-backed bytes API

Layer / File(s)	Summary
WrittenFile struct extension and docs crates/ourios-parquet/src/writer.rs	WrittenFile gains pub key: String and pub bytes_written: u64; field docs clarify how path differs for local vs store-backed writers and why bytes_written is needed for store-backed backends. Writer struct and open rustdocs updated.
Writer open_in, ZSTD validation, and close changes crates/ourios-parquet/src/writer.rs	ZSTD level validation moved before filesystem side effects; open_in/open_in_with_zstd_level rewritten to derive final_path from the object key for store-backed writers and clone Store into the struct; close computes bytes_written and populates both new WrittenFile fields.
Reader::open_partition_bytes API crates/ourios-parquet/src/reader.rs	New pub fn open_partition_bytes(bytes, partition, key) delegates to from_bytes for Parquet init, records key as file_path, sets partition for row-vs-path validation in read_all.
Integration tests crates/ourios-parquet/src/writer.rs	Two tests: one asserts WrittenFile.key Hive prefix/.parquet suffix and bytes_written equality with stored object size; the other asserts Reader::open_partition_bytes rejects bytes under a mismatched partition.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• jensholdgaard/ourios#44: Introduced the PartitionKey and initial writer surface that WrittenFile.key and bytes_written extend.
• jensholdgaard/ourios#232: Added Reader::open_bytes and the shared from_bytes initialization flow that open_partition_bytes delegates into.
• jensholdgaard/ourios#233: Modified the same from_bytes/file_path diagnostic path in reader.rs that open_partition_bytes now builds on.

Poem

🐇 A key and a count now travel with each file,
bytes_written recorded, worth every while.
The partition checks bytes from the store,
mismatched keys rejected at the door.
Hop hop — the writer and reader align! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly names the main change: store-backed Writer/Reader constructors for the RFC 0019 compactor.
Description check	✅ Passed	The description covers the change, motivation, invariants, tests, and local verification; only the template headings are not followed exactly.
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 rfc0019-writer-reader-store-seam

---

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

Adds a Store-backed construction seam to the ourios-parquet writer/reader so the RFC 0019 compactor can read/write Parquet through an already-configured Store (local or S3) without changing existing local call sites.

Changes:

• Introduces Writer::open_in / open_in_with_zstd_level to write via an existing Store, while keeping Writer::open delegating to the local-store path.
• Extends WrittenFile with backend-agnostic output identifiers (key) and size reporting (bytes_written).
• Adds Reader::open_partition_bytes to validate/read partitioned Parquet data from in-memory bytes (store fetch).

Reviewed changes

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

File	Description
crates/ourios-parquet/src/writer.rs	Adds Store-backed writer constructors and augments WrittenFile with key/size metadata; includes new unit tests for store round-trip.
crates/ourios-parquet/src/reader.rs	Adds a partition-aware “read from bytes” constructor to support store-backed compaction reads while preserving §3.9 validation.

Comments suppressed due to low confidence (1)

crates/ourios-parquet/src/writer.rs:176

• Writer::open_with_zstd_level no longer validates zstd_level before creating the partition directory / opening the store. As a result, an invalid level now has observable filesystem side-effects (empty partition dir) and does more work before returning WriterError::Parquet, which contradicts the prior “fail fast” contract described in the removed comment block.

        // Ensure the store root (and the partition dir) exist:
        // `Store::local` canonicalises `bucket_root`, which must
        // therefore exist; the object-store `put` on close creates any
        // remaining parents.
        let dir = partition.data_path(bucket_root);

---

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

crates/ourios-parquet/src/writer.rs (1)

825-904: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make the store-backed reconstruction test property-based.

This adds good seam coverage, but it exercises only one fixed record shape. Since this is a Parquet writer reconstruction path, please add a proptest variant alongside the deterministic store/key assertions. Also fix the RFC0009.5 typo in the mismatch test comment while touching this block. As per coding guidelines, **/crates/ourios-{miner,parquet,querier}/**/*.rs: “Use property tests (proptest) for anything with an invariant: the template miner, the Parquet writer, the query planner. Reconstruction is always a property test.”

🤖 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-parquet/src/writer.rs` around lines 825 - 904, The store-backed
reconstruction coverage in Writer::open_in and Reader::open_partition_bytes is
still only using one fixed record shape, so add a proptest-based variant that
exercises the same key/size/reconstruction invariants across generated records
while keeping the existing deterministic assertions. Reuse the existing
Writer::open_in, WrittenFile, and Reader::open_partition_bytes flow, but
parameterize the records so the reconstruction path is property-tested as
required for the Parquet writer. While touching the
open_partition_bytes_rejects_a_mismatched_partition test, correct the RFC0009.5
typo in the comment.

Source: Coding guidelines

🤖 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.

Inline comments:
In `@crates/ourios-parquet/src/reader.rs`:
- Around line 112-137: Clean up the docs for Reader::open_partition_bytes and
Reader::read_all: replace the stray RFC0009.5 reference with the same RFC 0005
§3.9 wording used elsewhere, and update the read_all documentation to explicitly
mention open_partition_bytes as another entry point that triggers the same
partition validation path.

In `@crates/ourios-parquet/src/writer.rs`:
- Around line 243-247: Update the rustdoc for the `final_path()` accessor in
`writer.rs` to reflect store-backed behavior: it should describe the returned
path as the object key rendered as a path for store-backed writes, not an
absolute store-root-joined filesystem path. Keep the documentation aligned with
the `final_path` construction in the writer logic and mention `WrittenFile::key`
as the canonical value for S3/store-backed reads so callers are not misled.

---

Nitpick comments:
In `@crates/ourios-parquet/src/writer.rs`:
- Around line 825-904: The store-backed reconstruction coverage in
Writer::open_in and Reader::open_partition_bytes is still only using one fixed
record shape, so add a proptest-based variant that exercises the same
key/size/reconstruction invariants across generated records while keeping the
existing deterministic assertions. Reuse the existing Writer::open_in,
WrittenFile, and Reader::open_partition_bytes flow, but parameterize the records
so the reconstruction path is property-tested as required for the Parquet
writer. While touching the open_partition_bytes_rejects_a_mismatched_partition
test, correct the RFC0009.5 typo in the comment.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 380b7a79-bac2-4692-945c-abfce44b0580

📥 Commits

Reviewing files that changed from the base of the PR and between fb313c2 and 3e8d212.

📒 Files selected for processing (2)

• crates/ourios-parquet/src/reader.rs
• crates/ourios-parquet/src/writer.rs

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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