feat: alias-index write path v1 — alias events persisted + querier-derived map (#148, per #183) - #184
Merged
Merged
Conversation
…RFC 0005 §3.7, #148) Adds the three OPTIONAL alias columns the 2026-06-12 amendment pins (alias_representative_id, alias_member_ids LIST<UInt64> with the empty-list-vs-NULL distinction, alias_actor), maps AliasAsserted / AliasRetracted to event_kind 4 / 5 with member_ids stored verbatim and the "" <-> NULL reason rule, applies the §3.7 encoding-policy rows for the new columns, and retires the writer's interim AliasEventNotYetPersistable rejection. Schema change is §3.8 rule 1 (additive OPTIONAL); the schema-pin fixture moves in lockstep. The colocated expect-error test flips per the RFC-gated contract change (CLAUDE.md §6.2), citing the amendment in its doc comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
|
Warning Review limit reached
More reviews will be available in 18 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds alias-event persistence and tolerant unknown-event reads; extends audit Parquet schema/writers/readers for alias fields; introduces shared audit-file discovery; derives alias maps from audit stream at compile time when queries use resolves_to; makes Querier::run_query accept an optional alias_map and updates tests. ChangesAlias Event Persistence and Derivation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the RFC 0005 §3.7 / §3.7.1 alias-index v1 end-to-end: persist alias audit events in Parquet, make the reader forward-compatible with unknown event_kinds, and derive the per-tenant AliasMap in the querier when resolves_to(...) is used.
Changes:
- Extend the audit Parquet schema with
alias_*columns and implement writer/reader support for kinds 4–5 (alias_asserted/alias_retracted), including"" ↔ NULLforreason. - Replace unknown-ordinal hard-failure with
AuditPayload::Unknown { event_kind, event_type }and preserve the envelope on read-then-write. - Add querier-side storage derivation of the alias map (scanning
audit/tenant_id=...), and wireQuerier::run_query(..., alias_map: Option<&AliasMap>)soNonetriggers derivation only when needed.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-querier/tests/rfc0005_14_alias_derivation.rs | New integration tests pin §3.7.1 derived-map ordering and tenant isolation. |
| crates/ourios-querier/tests/rfc0005_13.rs | Update run_query calls for the new Option<&AliasMap> parameter. |
| crates/ourios-querier/tests/rfc0002_dsl.rs | Update run_query calls; add storage-backed RFC0002.9 resolves_to test using derived map. |
| crates/ourios-querier/tests/rfc0001_time_preserved.rs | Update run_query call signature for alias map injection. |
| crates/ourios-querier/tests/rfc0001_query_semantics.rs | Update run_query call signature for alias map injection. |
| crates/ourios-querier/src/lib.rs | Wire alias_map: Option<&AliasMap> and derive from storage when resolves_to is present. |
| crates/ourios-querier/src/drift.rs | Refactor drift audit-file discovery to use shared audit_scan. |
| crates/ourios-querier/src/compile.rs | Add uses_resolves_to helper to avoid unnecessary alias derivation scans. |
| crates/ourios-querier/src/audit_scan.rs | New shared audit subtree walk: tenant isolation, canonical-path backstop, optional day-prune, lexicographic file ordering. |
| crates/ourios-querier/src/alias_store.rs | New v1 alias-map derivation by scanning and folding alias events with §3.7.1 total order. |
| crates/ourios-parquet/tests/schema_pin.rs | Update pinned audit schema field list to include alias_* columns. |
| crates/ourios-parquet/tests/audit_round_trip.rs | Add RFC0005.14 round-trip coverage for alias events and raw-column NULL discipline. |
| crates/ourios-parquet/src/lib.rs | Add audit_columns::ALIAS_* constants and extend audit_schema() with new optional fields. |
| crates/ourios-parquet/src/audit_writer.rs | Apply §3.7 encoding policy for alias_actor and alias_member_ids list leaf stats/page-index behavior. |
| crates/ourios-parquet/src/audit_record_batch.rs | Retire alias-event rejection; encode kinds 4–5 into new columns; support writing AuditPayload::Unknown envelope-only. |
| crates/ourios-parquet/src/audit_reader.rs | Decode kinds 4–5; add unknown-ordinal tolerance via AuditPayload::Unknown; add LIST<UInt64> decoding for alias_member_ids. |
| crates/ourios-core/src/audit.rs | Add AuditPayload::Unknown and adjust event_type() to return &str (preserve unknown strings). |
| crates/ourios-core/src/alias.rs | Ensure alias fold ignores AuditPayload::Unknown by construction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ourios-parquet/src/audit_reader.rs (1)
301-324:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDefer OPTIONAL payload decoding until after
event_kinddispatch.Lines 301-324 still parse every payload group up front, so the unknown-kind fallback on Lines 374-380 is not actually sufficient. A pre-amendment file that omits OPTIONAL columns like
positions_widened/slots_expandedstill hard-fails before row dispatch, and a future unknown kind can also fail if irrelevant payload columns contain data this reader doesn't understand. That breaks the reader-compat contract this change is trying to introduce.Please gate payload decoding by the named kind being reconstructed, or make the optional column decoders lazy/non-failing until a row actually needs that column. As per coding guidelines, "Parquet readers MUST handle absent columns (old files) and unknown columns (future files) without error."
Also applies to: 374-380
🤖 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/audit_reader.rs` around lines 301 - 324, The code eagerly decodes OPTIONAL payload columns (e.g., positions_widened_lists via decode_positions_column, slots_expanded_lists via decode_slots_column, optional_string/optional_u64/optional_u32 calls that set template_id, old_version, new_version, old_template, new_template, triggering_line_hash, reason, compaction_*, alias_*) before dispatching on event_kind, causing failures for pre-amendment or future files; fix by deferring or making lazy those decodes: move the payload-specific decoders into the event_kind match arms so you only call decode_positions_column, decode_slots_column and the optional_* decoders when reconstructing kinds that actually need them, or change those decoders to return a non-failing lazy wrapper/closure (e.g., Option<Result<...>> or a thunk) that only performs parsing when accessed; ensure the unknown-kind fallback remains reachable and that optional_* helpers tolerate missing columns until invoked.Source: Coding guidelines
🧹 Nitpick comments (1)
crates/ourios-querier/src/alias_store.rs (1)
40-83: ⚡ Quick winAdd colocated unit tests for the ordering and tenant-mismatch contract.
This module owns the §3.7.1 ordering glue plus the row-level tenant backstop, but it has no in-file tests. Please add a small
#[cfg(test)]block here that pins same-timestamp ordering and foreign-tenant rejection next to the implementation.As per coding guidelines,
crates/**/src/**/*.rs: Unit tests must be colocated next to the code for anything non-trivial.🤖 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-querier/src/alias_store.rs` around lines 40 - 83, Add a #[cfg(test)] mod tests next to derive_alias_map that exercises the ordering and tenant-backstop: create temporary audit files under a temp bucket_root and tenant partition (use audit_scan::audit_files expectations and AuditReader::open_file/read_all format) — one test writes two files with AliasAsserted events that share the same timestamp but are created in a lexicographic file order so calling derive_alias_map(bucket_root, tenant) yields an AliasMap whose events preserve file+row ordering (assert the resulting AliasMap ordering or resulting alias resolution); another test writes a file containing an AuditEvent whose tenant_id != tenant and asserts derive_alias_map returns Err(QueryError::Storage) with a message mentioning the offending path; keep tests colocated in the same file and use AliasMap::from_events/derive_alias_map/AuditReader symbols to construct and validate behavior.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-querier/src/audit_scan.rs`:
- Around line 48-49: The tenant_dir path may be a symlink allowing traversal
outside the intended audit tree; before walking or canonicalizing it, resolve
and verify its real location by canonicalizing the bucket_root.join("audit")
tree and the tenant_dir (or explicitly reject if symlink_metadata shows
tenant_dir is a symlink), then ensure the canonicalized tenant_dir starts_with
the canonicalized audit root; update the logic around the tenant_dir variable
and any later starts_with checks (affecting the block that canonicalizes
tenant_root and the run_drift caller) to use the canonical audit root or reject
symlinked tenant roots up front.
In `@crates/ourios-querier/src/lib.rs`:
- Around line 501-515: The code derives an alias map
(alias_store::derive_alias_map) before letting compile::compile perform query
validation/window-resolution, which can cause unnecessary audit-tree access for
invalid queries; change the control flow so you validate the query first (call a
validation/preflight function on compile—e.g. extract and call a
compile::validate_query or a new compile::preflight_validate that performs
unsupported-stage and window-resolution checks with tenant, now_unix_nano and
default_window_nanos) and only when that validation succeeds proceed to call
alias_store::derive_alias_map when compile::uses_resolves_to(&query.predicate)
is true, then pass the derived or empty AliasMap into compile::compile; if
compile::compile currently contains validation logic, split that
validation/window-resolution into a callable helper and invoke it before
derive_alias_map.
In `@crates/ourios-querier/tests/rfc0005_14_alias_derivation.rs`:
- Around line 203-219: The test currently can't prove T2's derived alias map was
applied because T2 only has an A data row; add a data row in T2 for the aliased
class (e.g., a B row) so that after writing the audit event with
write_audit_file_named(alias_asserted(...)) the
resolves_to_a_rows(bucket.path(),"T2").await will reflect folding (expect count
>1). Update the fixture by calling write_all or write_data_rows for T2 to
include the extra B row and change the assertion on resolves_to_a_rows for "T2"
accordingly so the test fails if alias derivation isn't applied.
---
Outside diff comments:
In `@crates/ourios-parquet/src/audit_reader.rs`:
- Around line 301-324: The code eagerly decodes OPTIONAL payload columns (e.g.,
positions_widened_lists via decode_positions_column, slots_expanded_lists via
decode_slots_column, optional_string/optional_u64/optional_u32 calls that set
template_id, old_version, new_version, old_template, new_template,
triggering_line_hash, reason, compaction_*, alias_*) before dispatching on
event_kind, causing failures for pre-amendment or future files; fix by deferring
or making lazy those decodes: move the payload-specific decoders into the
event_kind match arms so you only call decode_positions_column,
decode_slots_column and the optional_* decoders when reconstructing kinds that
actually need them, or change those decoders to return a non-failing lazy
wrapper/closure (e.g., Option<Result<...>> or a thunk) that only performs
parsing when accessed; ensure the unknown-kind fallback remains reachable and
that optional_* helpers tolerate missing columns until invoked.
---
Nitpick comments:
In `@crates/ourios-querier/src/alias_store.rs`:
- Around line 40-83: Add a #[cfg(test)] mod tests next to derive_alias_map that
exercises the ordering and tenant-backstop: create temporary audit files under a
temp bucket_root and tenant partition (use audit_scan::audit_files expectations
and AuditReader::open_file/read_all format) — one test writes two files with
AliasAsserted events that share the same timestamp but are created in a
lexicographic file order so calling derive_alias_map(bucket_root, tenant) yields
an AliasMap whose events preserve file+row ordering (assert the resulting
AliasMap ordering or resulting alias resolution); another test writes a file
containing an AuditEvent whose tenant_id != tenant and asserts derive_alias_map
returns Err(QueryError::Storage) with a message mentioning the offending path;
keep tests colocated in the same file and use
AliasMap::from_events/derive_alias_map/AuditReader symbols to construct and
validate behavior.
🪄 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: ca6b2570-df30-481e-b467-adde0f902446
📒 Files selected for processing (18)
crates/ourios-core/src/alias.rscrates/ourios-core/src/audit.rscrates/ourios-parquet/src/audit_reader.rscrates/ourios-parquet/src/audit_record_batch.rscrates/ourios-parquet/src/audit_writer.rscrates/ourios-parquet/src/lib.rscrates/ourios-parquet/tests/audit_round_trip.rscrates/ourios-parquet/tests/schema_pin.rscrates/ourios-querier/src/alias_store.rscrates/ourios-querier/src/audit_scan.rscrates/ourios-querier/src/compile.rscrates/ourios-querier/src/drift.rscrates/ourios-querier/src/lib.rscrates/ourios-querier/tests/rfc0001_query_semantics.rscrates/ourios-querier/tests/rfc0001_time_preserved.rscrates/ourios-querier/tests/rfc0002_dsl.rscrates/ourios-querier/tests/rfc0005_13.rscrates/ourios-querier/tests/rfc0005_14_alias_derivation.rs
…e envelope (RFC 0005 §3.7) The reader rebuilds AliasAsserted / AliasRetracted payloads from the alias_* columns (empty member list != NULL; NULL reason decodes to the in-memory empty string; an empty actor is a writer-invariant error — aliasing is never anonymous, RFC 0001 §6.7). The UnknownEventKind hard error is replaced by the now-pinned §3.7 tolerance rule: an ordinal above the known range surfaces as the new envelope-only AuditPayload::Unknown variant (the ParamType::Unknown discipline applied to the kind enum), preserved verbatim on read-then-write; folds over named kinds (AliasMap::apply, the drift filter) ignore it by construction. The forged-ordinal reader test flips from expect-error to expect-opaque-event per the RFC-gated contract change (#183, CLAUDE.md §6.2), with a round-trip test alongside. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…_scan Pure relocation of drift.rs's tenant-rooted audit-tree walk (canonical escape backstop, canonical-path de-dup, conservative day prune) into a shared module, with the day window now optional and the resolved file set sorted lexicographically — the file-path component of the RFC 0005 §3.7.1 total fold order the alias-map derivation needs next. The two prune unit tests move with the helpers, unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… 0005 §3.7.1, #148) run_query's alias parameter becomes Option<&AliasMap>: None — the production default — folds the requesting tenant's map from its audit/ partition at compile time (alias_store::derive_alias_map), reading kinds 4/5 via AuditReader in the §3.7.1 total order (timestamp, file path lexicographic, within-file row index — the shared walk's sorted file set + in-file row order under a stable sort by timestamp) and handing the events to ourios-core::alias::AliasMap::from_events, whose RFC 0001 §6.7 semantics this does not restate. Some(map) stays the test/operator override, bypassing storage. Queries with no resolves_to skip the audit scan entirely. A row claiming another tenant under the tenant's partition root is a loud Storage error, not a silent drop (CLAUDE.md §3.7). Test updates are mechanical Option-wrapping at the call sites; no assertion changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(a) round-trip through the real audit writer/reader: member set
verbatim (order + duplicate preserved), empty-list retraction != NULL,
actor, the "" <-> NULL reason rule, and raw-column NULL discipline per
kind (§3.8 rule 6). (b) the §3.7.1 derived fold: assert-then-retract by
event time, plus both directions of the cross-file same-timestamp
tiebreak — one event per file, files renamed into a crafted
lexicographic order so the outcome is decided by the file-path
component of the total order and nothing else. (c) tenant isolation:
a second tenant's stored alias events never fold into the requesting
tenant's derived map (CLAUDE.md §3.7). (d) storage-backed RFC0002.9:
the assertion is written through ParquetAuditSink and run_query(None)
derives the map end-to-end — resolves_to(A) returns A ∪ {B} while
template_id == A stays exactly A; the injected-map RFC0002.9 test
stays alongside, unweakened.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…efore alias derivation Review round 1: - a symlinked audit/tenant_id=... root is now rejected outright (the per-file starts_with backstop trusted the symlink's own resolution) - run_query validates the query before paying the alias-derivation IO, so compile errors precede Storage errors deterministically - tenant-mismatch error renders tenant IDs with Display, not Debug - tests: symlinked-root rejection, error precedence, and the T2 isolation test now proves its own map was derived (resolves_to == 2) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jensholdgaard
force-pushed
the
feat/148-alias-index-write-path-v1
branch
from
June 12, 2026 10:17
507b3e3 to
2043f85
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ourios-querier/src/compile.rs (1)
93-145:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
validateonly enforces part of the new pre-derivation error contract.Lines 99-135 reject unsupported stages and resolve the window/limit, but predicate-level compile failures still happen later in paths like
string_call_column,attr_match, andcolumn_comparison. A query such asresolves_to(10) and trace_id contains "x"will passvalidate, hituses_resolves_to, derive the alias map from storage, and only then fail asInvalidQuery. Ifrun_queryis meant to guarantee “invalid query before alias-scan IO,” this needs a pure predicate-validation pass folded intovalidate, not just stage validation.🤖 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-querier/src/compile.rs` around lines 93 - 145, validate currently only checks stages/window/limit but lets predicate-level errors slip through until alias-map derivation; add a pure predicate validation pass inside validate that walks the query predicates and runs the same checks used by string_call_column, attr_match, column_comparison, and uses_resolves_to (or extract their pure parts into non-IO helpers) so any predicate that would later produce QueryError::InvalidQuery is caught before alias lookup; ensure these new validation helpers do not touch storage/alias_map and call them from validate before resolving window/limit return.
♻️ Duplicate comments (1)
crates/ourios-querier/src/audit_scan.rs (1)
48-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate the tenant root before the walk can return successfully.
Line 51 seeds the traversal with
tenant_dirbefore it has been checked against the canonical bucket root, and Line 84 can returnOk([])before Lines 93-116 ever run. That means a symlinkedaudit/tenant_id=<enc>can still masquerade as a normal empty scan when no.parquetfiles are found (or the window prunes them all), andrun_driftwill treat that as “no data” instead of a tenancy failure. Move the canonical tenant-root check ahead of the walk/early return so the scan fails before any foreign subtree is traversed or silently accepted. As per coding guidelines,Every code path that touches data must take a tenant ID; every Parquet file partitioned by tenant; every template tree scoped per tenant.Also applies to: 84-116
🤖 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-querier/src/audit_scan.rs` around lines 48 - 52, The tenant directory is seeded into the traversal before it is validated against the canonical bucket root, allowing a symlinked audit/tenant_id=<enc> to be treated as empty; move and perform the canonical-tenant-root check for tenant_dir (the path built from bucket_root.join("audit").join(format!("tenant_id={enc}"))) immediately after construction and before creating stack/files or returning early (before the logic that can return Ok([])); if the canonicalized tenant_dir is not under the canonical bucket_root, return an error so the subsequent walk (stack loop and the code that collects .parquet files/filters used by run_drift) never proceeds on an unvalidated subtree.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.
Outside diff comments:
In `@crates/ourios-querier/src/compile.rs`:
- Around line 93-145: validate currently only checks stages/window/limit but
lets predicate-level errors slip through until alias-map derivation; add a pure
predicate validation pass inside validate that walks the query predicates and
runs the same checks used by string_call_column, attr_match, column_comparison,
and uses_resolves_to (or extract their pure parts into non-IO helpers) so any
predicate that would later produce QueryError::InvalidQuery is caught before
alias lookup; ensure these new validation helpers do not touch storage/alias_map
and call them from validate before resolving window/limit return.
---
Duplicate comments:
In `@crates/ourios-querier/src/audit_scan.rs`:
- Around line 48-52: The tenant directory is seeded into the traversal before it
is validated against the canonical bucket root, allowing a symlinked
audit/tenant_id=<enc> to be treated as empty; move and perform the
canonical-tenant-root check for tenant_dir (the path built from
bucket_root.join("audit").join(format!("tenant_id={enc}"))) immediately after
construction and before creating stack/files or returning early (before the
logic that can return Ok([])); if the canonicalized tenant_dir is not under the
canonical bucket_root, return an error so the subsequent walk (stack loop and
the code that collects .parquet files/filters used by run_drift) never proceeds
on an unvalidated subtree.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 84ab8b35-7340-4245-8331-c2a4bf11d05f
📒 Files selected for processing (15)
crates/ourios-core/src/alias.rscrates/ourios-core/src/audit.rscrates/ourios-parquet/src/audit_reader.rscrates/ourios-parquet/src/audit_record_batch.rscrates/ourios-parquet/tests/audit_round_trip.rscrates/ourios-querier/src/alias_store.rscrates/ourios-querier/src/audit_scan.rscrates/ourios-querier/src/compile.rscrates/ourios-querier/src/drift.rscrates/ourios-querier/src/lib.rscrates/ourios-querier/tests/rfc0001_query_semantics.rscrates/ourios-querier/tests/rfc0001_time_preserved.rscrates/ourios-querier/tests/rfc0002_dsl.rscrates/ourios-querier/tests/rfc0005_13.rscrates/ourios-querier/tests/rfc0005_14_alias_derivation.rs
🚧 Files skipped from review as they are similar to previous changes (12)
- crates/ourios-core/src/alias.rs
- crates/ourios-querier/tests/rfc0001_time_preserved.rs
- crates/ourios-parquet/tests/audit_round_trip.rs
- crates/ourios-querier/tests/rfc0005_13.rs
- crates/ourios-querier/src/alias_store.rs
- crates/ourios-querier/tests/rfc0001_query_semantics.rs
- crates/ourios-querier/tests/rfc0002_dsl.rs
- crates/ourios-querier/src/lib.rs
- crates/ourios-querier/src/drift.rs
- crates/ourios-parquet/src/audit_reader.rs
- crates/ourios-parquet/src/audit_record_batch.rs
- crates/ourios-core/src/audit.rs
…row index in messages Copilot round 2, wording only: the run_query comment no longer implies all invalid-query errors precede the audit scan (predicate compilation needs the map), and the non-nullable-element conversion messages say "batch row" — the index is batch-local, not file-global. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the #148 alias-index write path v1, spec-first to the just-merged #183 amendment (RFC 0005 §3.7 / §3.7.1) + RFC 0001 §6.7. Four phased commits, each independently gated.
What lands
1.
ourios-parquet— write side (RFC 0005 §3.7 amendment 2026-06-12)alias_representative_id(UInt64),alias_member_ids(LIST<UInt64>),alias_actor(Utf8).AliasAsserted/AliasRetractedmap toevent_kind4 / 5 with the paired canonicalevent_typestrings (ordinals imported fromourios-core, never redefined).member_idsstored verbatim (no sort/dedup — the semantic value is the set{representative_id} ∪ member_ids, folded by consumers); empty list is valid and distinct from NULL; in-memory empty-stringreason↔ on-disk NULL ("" ↔ NULL).alias_representative_idkeeps the page-index default (same shape astemplate_id);alias_member_idslist leaf +alias_actordowngraded to chunk stats.AliasEventNotYetPersistablewriter rejection is retired per the amendment.2.
ourios-parquet/ourios-core— read sidealias_actoris a loud writer-invariant error — aliasing is never anonymous, RFC 0001 §6.7).UnknownEventKindhard error is replaced by the now-pinned §3.7 tolerance rule: an ordinal above the known range surfaces as the new envelope-onlyAuditPayload::Unknown { event_kind, event_type }variant (theParamType::Unknowndiscipline applied to the kind enum), preserved verbatim on read-then-write. Folds over named kinds (AliasMap::apply, the RFC 0010 drift filter) ignore it by construction — the compiler enforces the new arm at every exhaustive match site.3.
ourios-querier— the §3.7.1 v1 derivationdrift.rs's tenant-rooted audit walk (canonical-path tenant-escape backstop, canonical de-dup, conservative day prune) is extracted into a sharedaudit_scanmodule; the day window is now optional and the file set returns in lexicographic path order.alias_store::derive_alias_map: scans the tenant'saudit/partition root (no window — the fold covers the whole alias history; alias events are rare operator actions, §3.7.1), filters kinds 4/5, orders events by the §3.7.1 total order(timestamp, file path lexicographic, within-file row index)— sorted file walk + in-file row order under a stable sort by timestamp — and folds viaourios_core::alias::AliasMap::from_events(RFC 0001 §6.7 semantics owned there, not restated).Storageerror, not a silent drop.4. Wiring choice (called out per the issue):
run_query's alias parameter becomesOption<&AliasMap>—None(the production default) selects the §3.7.1 storage derivation, skipped entirely when the query has noresolves_to;Some(map)remains the test/operator override that bypasses storage. This was the most natural fit forcompile.rs's structure:compile()keeps taking a plain&AliasMap, andrun_queryresolves the source before compiling. Existing test call sites were updated mechanically (Some(&…)wrapping) — no assertion changes.Tests (RFC0005.14 + #148 step 3)
AuditWriter→AuditReader: member set verbatim (order + duplicate preserved), empty-list retraction ≠ NULL, actor,"" ↔ NULLreason, plus raw-column NULL discipline per kind (§3.8 rule 6).a.parquet/b.parquetso identical timestamps force the outcome onto the file-path component of the total order; the mirrored case proves the tiebreak (not luck) decides.ParquetAuditSink,run_query(…, None)derives the map end-to-end —resolves_to(A)returns A ∪ {B} whiletemplate_id == Astays exactly A. The injected-map RFC0002.9 test stays alongside, unweakened.RFC-gated test flips (CLAUDE.md §6.2 — explicit approval surface)
Two existing tests asserted exactly the behaviour the #183 amendment removes, and flip rather than weaken (each cites the amendment in its doc comment):
alias_events_are_rejected_pending_the_rfc_0005_split→alias_events_build_a_batch_with_kinds_4_and_5(the writer rejection is retired by the amendment).UnknownEventKind-error → expect-opaque-AuditPayload::Unknown(the documented deferral was "until a real new variant lands"; kinds 4–5 are that variant and the amendment pins the tolerance rule). AnUnknownround-trip test lands alongside.Invariants / hazards addressed
resolves_toonly saw an in-memory injected map. Nothing is auto-inferred;bare template_id ==never follows alias chains (pinned by tests).None, no migration) plus the rule 6 required-by-convention conventions for kinds 4–5, writer-enforced and test-pinned. The schema-pin fixture (RFC0005.10) moves in lockstep. Reader forward-compat is strengthened (unknown ordinals no longer fail files).Verification (per commit and at HEAD)
cargo test --all-features(exit 0, 0 failed),cargo fmt --all --check,cargo clippy --all-targets --all-features -- -D warnings,cargo doc --workspace --no-deps --all-features,cargo bench -p ourios-bench --no-run.Closes the storage/querier slices of #148 (steps 2–3); per #183.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests