Skip to content

feat(parquet): AuditReader::open_bytes for the RFC 0019 Store read path - #291

Merged
jensholdgaard merged 1 commit into
mainfrom
rfc0019-audit-reader-open-bytes
Jun 26, 2026
Merged

feat(parquet): AuditReader::open_bytes for the RFC 0019 Store read path#291
jensholdgaard merged 1 commit into
mainfrom
rfc0019-audit-reader-open-bytes

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jun 26, 2026

Copy link
Copy Markdown
Owner

What

RFC 0019 slice-2a prerequisite: AuditReader::open_bytes(Bytes) — so the querier's audit scan can read audit Parquet through the Store seam (Store::get_blocking → bytes → here) on local or S3, rather than std::fs.

How

  • Mirrors the existing Reader::open_bytes: builds the ParquetRecordBatchReader from in-memory bytes::Bytes with the same §3.7 baseline-REQUIRED-column check, skipping row-vs-path validation (like open_file). No filesystem access → no Io error.
  • Factors the builder + schema-check into a private from_chunk_reader<R: ChunkReader> helper shared by open_file (a File) and open_bytes (Bytes) — no duplication.

Tests

  • audit_reader_open_bytes_round_trips: write events via AuditWriter, read the object's bytes, parse through open_bytes, assert full AuditEvent equality with what was written (parity with the on-disk path).
  • cargo clippy -p ourios-parquet --all-targets --all-features -- -D warnings clean; cargo fmt --all --check green.

Context

Bounded, additive prerequisite for slice 2a (the querier Store migration). The §3.7 audit-isolation decision for that migration is settled (maintainer): rely on the #290 segment-wise prefix-scoped Store::list for tenant isolation, dropping the local-only symlink-canonicalize backstop (object storage — the production source of truth — has no symlinks; the residual local risk implies bucket write access). No §5 scenario flips green here; RFC 0019 stays red.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added support for loading audit data directly from in-memory bytes, in addition to reading from disk.
    • Audit reads now use a shared code path so behavior stays consistent across input sources.
  • Tests
    • Added coverage to confirm audit data round-trips correctly when read from in-memory bytes.

@jensholdgaard
jensholdgaard requested a review from Copilot June 26, 2026 06:57
@coderabbitai

coderabbitai Bot commented Jun 26, 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: 50ecf09b-1191-4e5e-b308-d9143a0dd285

📥 Commits

Reviewing files that changed from the base of the PR and between e71ed61 and cc69d63.

📒 Files selected for processing (2)
  • crates/ourios-parquet/src/audit_reader.rs
  • crates/ourios-parquet/tests/audit_round_trip.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/ourios-parquet/tests/audit_round_trip.rs
  • crates/ourios-parquet/src/audit_reader.rs

📝 Walkthrough

Walkthrough

AuditReader now accepts in-memory audit parquet bytes through a new open_bytes entry point. The shared chunk-reader constructor now accepts an explicit file path, and a new test reads written audit data back from bytes and compares it to the original events.

Changes

Audit bytes-backed reader

Layer / File(s) Summary
Bytes-backed reader setup
crates/ourios-parquet/src/audit_reader.rs
AuditReader::open_bytes forwards bytes::Bytes through the shared chunk-reader path, and the shared constructor now takes an explicit file path while storing that value on the reader.
Round-trip test
crates/ourios-parquet/tests/audit_round_trip.rs
A test writes audit events, reads the produced parquet file as bytes, parses it with AuditReader::open_bytes, and compares the decoded events to the originals.

Sequence Diagram(s)

sequenceDiagram
  participant Test
  participant AuditWriter
  participant FileSystem
  participant AuditReader
  participant from_chunk_reader

  Test->>AuditWriter: write three_variants("acme")
  AuditWriter->>FileSystem: write audit parquet file
  Test->>FileSystem: read raw bytes
  Test->>AuditReader: open_bytes(bytes)
  AuditReader->>from_chunk_reader: open_bytes(bytes, "<object-store>")
  from_chunk_reader-->>AuditReader: parsed AuditReader
  AuditReader-->>Test: decoded events
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hopped through bytes with twitchy nose,
and traced the audit where parquet flows.
A memory burrow, neat and bright,
returned the events in one soft bite.
🐰✨ Hop-happy audit trail!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly names the new open_bytes API and the RFC 0019 store read path.
Description check ✅ Passed It covers the change, rationale, tests, and RFC context, but it doesn't follow the template's exact Summary/Related/Checklist structure.
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 rfc0019-audit-reader-open-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

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

Adds an in-memory audit Parquet read entry point to support the RFC 0019 querier migration onto the Store seam (reading via Store::get_blocking → bytes) instead of directly from std::fs.

Changes:

  • Introduces AuditReader::open_bytes(Bytes) for reading audit Parquet from in-memory bytes.
  • Refactors shared Parquet-builder + required-column schema validation into from_chunk_reader<R: ChunkReader>, used by both open_file and open_bytes.
  • Adds a round-trip test asserting open_bytes produces identical AuditEvents to what AuditWriter wrote.

Reviewed changes

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

File Description
crates/ourios-parquet/src/audit_reader.rs Adds open_bytes and factors common ChunkReader-based reader construction with required-column checks.
crates/ourios-parquet/tests/audit_round_trip.rs Adds a round-trip test covering the new in-memory open_bytes audit read path.

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

Comment thread crates/ourios-parquet/src/audit_reader.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/tests/audit_round_trip.rs`:
- Around line 194-217: The round-trip coverage for AuditReader::open_bytes is
currently only an integration-style example in audit_round_trip.rs, but it needs
to live next to the AuditReader implementation and be expressed as a property
test. Move the test into src/audit_reader.rs alongside
AuditReader::open_bytes/open_file, and rewrite it using proptest so it checks
the reconstruction invariant across generated event sequences rather than a
single fixed fixture. Keep the assertion that bytes read from the written audit
object round-trip through AuditReader::open_bytes and match the original events.
🪄 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: e8351657-c165-4cd7-aca3-99a3baf5a734

📥 Commits

Reviewing files that changed from the base of the PR and between dcf8917 and e71ed61.

📒 Files selected for processing (2)
  • crates/ourios-parquet/src/audit_reader.rs
  • crates/ourios-parquet/tests/audit_round_trip.rs

Comment thread crates/ourios-parquet/tests/audit_round_trip.rs
…d path

Add `AuditReader::open_bytes(Bytes)` (mirroring `Reader::open_bytes`): build the
audit reader from in-memory bytes with the same §3.7 baseline-column check,
sharing a private `from_chunk_reader` helper with `open_file`. This is the
prerequisite for the querier's audit scan to read through the `Store` seam
(`Store::get_blocking` → bytes → here) on local or S3 (RFC 0019 §3.3 / slice 2a),
rather than `std::fs`. Round-trip test asserts parity with the on-disk path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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 2 out of 2 changed files in this pull request and generated no new comments.

@jensholdgaard
jensholdgaard merged commit 428f825 into main Jun 26, 2026
22 checks passed
@jensholdgaard
jensholdgaard deleted the rfc0019-audit-reader-open-bytes branch June 26, 2026 08:22
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