test(parquet): green RFC0013.2 + .5 local acceptance scenarios

[jensholdgaard]

RFC 0013 green — formalizes the two §5 scenarios the merged Store seam now satisfies on the local backend (test-only).

What

• RFC0013.2 (local backend regresses nothing): a 200-row round-trip through the Store-backed Writer + Reader — the same LocalFileSystem seam the existing RFC0005/0009 suites (round_trip, sizing, partition_layout, manifest, compaction) now run through by default.
• RFC0013.5 (tenant isolation across the prefix): two tenants' data in one store land under distinct data/tenant_id=<tenant>/… key sub-prefixes, and a read of one tenant's partition surfaces only that tenant's rows.

Both un-#[ignore]d; cargo test shows 2 passed, 6 ignored (CI default run green).

Remaining stubs (accurately re-noted)

• .8 forward-compat: §3.9 (absent→default, unknown→ignored, missing-required→error) is already exercised over the Store seam by the colocated reader tests rfc0005_2/3/4 (which read through Store now). A duplicate variant-schema integration test is deferred — low marginal value.
• .6 WAL-stays-local: red until the server wires the object-store backend (production write-path wiring, out of current scope).
• .1/.3/.4/.7: red pending live S3-compatible storage (the integration approach is the open question in the PR thread).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Implemented RFC0013 test scenarios for multi-tenant partition isolation and object store round-trip operations.
  • Added test helpers for deterministic record generation and store write validation.
  • Enhanced coverage to verify tenant data separation prevents unauthorized cross-tenant access.

[coderabbitai]

📝 Walkthrough

Walkthrough

Activates two previously ignored RFC0013 integration tests in rfc0013_object_store.rs: RFC0013.2 (200-record Store-backed write/read round-trip) and RFC0013.5 (two-tenant partition isolation). Adds rec_for and write_through_store test helpers. Updates the RFC0013.8 stub with a deferred todo! and expanded rationale comment.

Changes

RFC0013 Object Store Tests

Layer / File(s)	Summary
Test helper utilities crates/ourios-parquet/tests/rfc0013_object_store.rs	Adds rec_for for deterministic MinedRecord construction and write_through_store to write a record batch via Writer and return the published object path.
RFC0013.2: Store-backed round-trip crates/ourios-parquet/tests/rfc0013_object_store.rs	Replaces the ignored stub with an active test writing 200 records through the local Store-backed Writer and asserting byte-for-byte equality on read_all().
RFC0013.5: Tenant partition isolation crates/ourios-parquet/tests/rfc0013_object_store.rs	Replaces the ignored stub with an active test writing two tenants into the same bucket, asserting each object path falls under the tenant's data/tenant_id=<id>/... prefix, and that reading one tenant's partition returns only that tenant's rows.
RFC0013.8: Updated deferred stub crates/ourios-parquet/tests/rfc0013_object_store.rs	Updates the ignored RFC0013.8 stub to a todo! with an expanded comment noting forward-compat schema behavior is already covered by existing reader tests through the Store seam.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• jensholdgaard/ourios#230: Introduced the original #[ignore] todo!() RFC0013.2/RFC0013.5/RFC0013.8 stubs in the same rfc0013_object_store.rs file that this PR directly replaces.
• jensholdgaard/ourios#231: Added the Store async put/get/delete local I/O surface and object-key resolution that the new RFC0013.2 and RFC0013.5 tests depend on to persist and read Parquet objects.
• jensholdgaard/ourios#44: Introduced PartitionKey/percent_encode_tenant and Writer behavior whose tenant-path derivation is validated by the new RFC0013.5 path-prefix assertions.

Poem

🐇 Hoppity-hop through the object store lane,
Two tenants now tested, no rows go astray!
Round-trips confirmed, 200 records in line,
Each partition is clean, every prefix divine.
The stubs that once slept now wake up and run —
A rabbit's best day: green tests, every one! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: enabling and implementing RFC0013.2 and RFC0013.5 test scenarios for the local backend.
Description check	✅ Passed	The description covers all required template sections: What section details the changes, Related section references RFC 0013, and a Checklist section acknowledges testing (though not all boxes marked). The description is comprehensive and addresses the PR objectives.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 rfc0013-green-local-scenarios

---

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 and usage tips.}

[Copilot]

Pull request overview

Adds green (non-ignored) acceptance tests for RFC 0013’s local object-store seam in ourios-parquet, validating that the Store-backed Writer/Reader round-trip and tenant prefix isolation behave correctly on the LocalFileSystem backend.

Changes:

• Adds deterministic MinedRecord generation plus a helper to write records through the Store-backed Writer.
• Implements RFC0013.2 as a 200-row Writer → Reader round-trip assertion on the local backend.
• Implements RFC0013.5 to verify per-tenant key-prefix layout and that reading tenant X’s partition returns only tenant X’s rows.

---

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

[coderabbitai]

🧹 Nitpick comments (1)

crates/ourios-parquet/tests/rfc0013_object_store.rs (1)

157-166: ⚡ Quick win

Consider adding symmetric read verification for tenant-y.

The test thoroughly verifies tenant-x's isolation (path prefix and read filtering), but does not symmetrically verify tenant-y's read. Adding a parallel read assertion for tenant-y would strengthen the isolation guarantee and ensure the test pattern generalizes.

✨ Suggested symmetric verification

     assert!(
         gx.iter().all(|r| r.tenant_id.as_str() == "tenant-x"),
         "no tenant-y rows leak into a tenant-x read",
     );
+
+    // Symmetric check: reading tenant-y surfaces only tenant-y rows.
+    let gy = Reader::open_partition(&ypath, py)
+        .expect("open y")
+        .read_all()
+        .expect("read y");
+    assert_eq!(gy.len(), y.len());
+    assert!(
+        gy.iter().all(|r| r.tenant_id.as_str() == "tenant-y"),
+        "no tenant-x rows leak into a tenant-y read",
+    );
 }

🤖 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/tests/rfc0013_object_store.rs` around lines 157 - 166,
Add symmetric read verification for tenant-y after the existing tenant-x
isolation checks. Follow the same pattern used for tenant-x: open tenant-y's
partition using the ypath and py variables, call read_all() on the Reader,
assert that the returned data length matches the expected tenant-y data length,
and verify that all returned rows have tenant_id equal to "tenant-y" with an
assertion message indicating that no tenant-x rows leak into the tenant-y read.
This symmetric verification strengthens the isolation guarantee by confirming
that the test pattern works bidirectionally for both tenants.

🤖 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-parquet/tests/rfc0013_object_store.rs`:
- Around line 157-166: Add symmetric read verification for tenant-y after the
existing tenant-x isolation checks. Follow the same pattern used for tenant-x:
open tenant-y's partition using the ypath and py variables, call read_all() on
the Reader, assert that the returned data length matches the expected tenant-y
data length, and verify that all returned rows have tenant_id equal to
"tenant-y" with an assertion message indicating that no tenant-x rows leak into
the tenant-y read. This symmetric verification strengthens the isolation
guarantee by confirming that the test pattern works bidirectionally for both
tenants.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4338b891-4d99-4c4e-8c38-104112d28cec

📥 Commits

Reviewing files that changed from the base of the PR and between 806595a and 5992671.

📒 Files selected for processing (1)

• crates/ourios-parquet/tests/rfc0013_object_store.rs