feat(rfc0017.2): derive_template_registry from the audit stream - #274
Merged
Conversation
RFC 0017 section 3.2 — the read-time template registry, mirroring alias_store::derive_alias_map: fold a tenant's template_created / template_widened / template_type_expanded audit events (deterministic timestamp, path, row order) into HashMap<(template_id, version), Vec<OwnedToken>>. template_created keys at TEMPLATE_INITIAL_VERSION (the variant omits the version); widened/type_expanded at new_version; rejections contribute nothing. Keying by (id, version) means a later widening never clobbers an earlier version's tokens (section 3.5). - ourios-miner: add tree::parse_template (inverse of format_template), co-locating the canonical encode/decode pair; format_template moved from cluster.rs to tree.rs (pub) so both live next to OwnedToken. Round-trip unit test. - ourios-querier: depend on ourios-miner (for OwnedToken + the registry, and reconstruct::render in .3); new template_registry module exposing derive_template_registry + TemplateRegistry. - Fills the RFC0017.2 stub (realistic space-joined templates via the production ParquetAuditSink write path). .5 stays ignored until .3. Also corrects the RFC section 3.2 canonical-form prose: it is the space-joined form format_template writes, not a JSON-array encoding (follow the code). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough
ChangesRFC0017 Template Registry Derivation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements RFC 0017 slice .2 by adding a querier-side, read-time template registry derived from the tenant audit stream, plus the miner-side canonical encode/decode pair needed to round-trip templates between audit storage and in-memory tokens.
Changes:
- Add
ourios_querier::derive_template_registryandTemplateRegistry = HashMap<(u64, u32), Vec<OwnedToken>>, folded from template audit events in RFC-defined total order. - Add
ourios_miner::tree::parse_templateand move/exposetree::format_templatealongsideOwnedToken, with round-trip unit tests. - Fill the RFC0017.2 test stub using
ParquetAuditSink, keeping RFC0017.5 ignored until the rendering slice lands.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| docs/rfcs/0017-template-registry-query-rendering.md | Updates §3.2 prose to match the actual canonical space-joined template encoding and parsing path. |
| crates/ourios-querier/tests/rfc0017_registry.rs | Implements the RFC0017.2 green test using production audit persistence + derivation; keeps RFC0017.5 ignored. |
| crates/ourios-querier/src/template_registry.rs | New module deriving (template_id, version) -> tokens from the audit stream using the shared audit_scan walk. |
| crates/ourios-querier/src/lib.rs | Wires the new module and re-exports derive_template_registry / TemplateRegistry. |
| crates/ourios-querier/Cargo.toml | Adds ourios-miner dependency to reuse OwnedToken and template parsing/formatting. |
| crates/ourios-miner/src/tree.rs | Adds parse_template, makes format_template public, and adds round-trip tests. |
| crates/ourios-miner/src/cluster.rs | Switches to importing format_template from tree.rs (removes local helper). |
| Cargo.lock | Records the new ourios-querier -> ourios-miner dependency. |
💡 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
🧹 Nitpick comments (1)
crates/ourios-querier/tests/rfc0017_registry.rs (1)
62-112: ⚡ Quick winBuild fixture templates with
format_templateto lock canonical parity.The fixture currently hardcodes canonical strings while claiming they match miner output. Constructing them with
ourios_miner::tree::format_templatemakes that contract self-enforcing and prevents drift if canonical formatting changes.Proposed refactor
-use ourios_miner::tree::OwnedToken; +use ourios_miner::tree::{OwnedToken, format_template}; @@ TemplateChange::Created { - new_template: "user <*>".to_owned(), + new_template: format_template(&[fixed("user"), OwnedToken::Wildcard]), }, @@ old_version: 1, new_version: 2, - old_template: "user <*>".to_owned(), - new_template: "user <*> <*>".to_owned(), + old_template: format_template(&[fixed("user"), OwnedToken::Wildcard]), + new_template: format_template(&[ + fixed("user"), + OwnedToken::Wildcard, + OwnedToken::Wildcard, + ]), positions_widened: vec![2], }, @@ TemplateChange::Created { - new_template: "GET <*>".to_owned(), + new_template: format_template(&[fixed("GET"), OwnedToken::Wildcard]), }, @@ old_version: 1, new_version: 2, - old_template: "GET <*>".to_owned(), - new_template: "GET <*>".to_owned(), + old_template: format_template(&[fixed("GET"), OwnedToken::Wildcard]), + new_template: format_template(&[fixed("GET"), OwnedToken::Wildcard]), slots_expanded: Vec::new(), }, @@ TemplateChange::RejectedDegenerate { version: 2, - current_template: "user <*> <*>".to_owned(), - would_be_template: "<*> <*> <*>".to_owned(), + current_template: format_template(&[ + fixed("user"), + OwnedToken::Wildcard, + OwnedToken::Wildcard, + ]), + would_be_template: format_template(&[ + OwnedToken::Wildcard, + OwnedToken::Wildcard, + OwnedToken::Wildcard, + ]), would_be_positions: vec![0], },🤖 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/tests/rfc0017_registry.rs` around lines 62 - 112, The fixture currently hardcodes template strings like "user <*>", "user <*> <*>", "GET <*>", and "<*> <*> <*>" in the TemplateChange enum variants (Created, Widened, TypeExpanded, RejectedDegenerate), which can drift if canonical formatting changes. Instead of hardcoding these strings, construct them using the ourios_miner::tree::format_template function to ensure the test fixtures match the actual miner output and maintain canonical parity automatically.
🤖 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-miner/src/tree.rs`:
- Around line 359-389: The parse_template and format_template round-trip
invariant needs property-based testing to cover broader token combinations
beyond the manual test cases. Add a new property test using proptest that
generates arbitrary OwnedToken combinations and verifies that parsing the result
of format_template always returns the original token sequence. This should be
implemented alongside the existing parse_template_inverts_format_template and
parse_template_empty_string_is_empty_template tests to ensure the invariant
holds across edge cases and various token shapes.
In `@crates/ourios-querier/src/template_registry.rs`:
- Around line 50-118: The function derive_template_registry lacks module-local
unit tests despite being non-trivial according to coding guidelines. Add focused
in-file tests in the template_registry.rs module that verify key behaviors: (1)
the tie-break ordering of audit events by timestamp and file/row order, and (2)
the row-vs-path tenant mismatch failure that correctly returns a
QueryError::Storage when an audit event claims a different tenant than expected.
Ensure tests exercise both the happy path (successful fold of template history)
and the error condition (tenant validation).
- Around line 81-83: The `derive_template_registry` function lacks unit test
coverage despite being a complex function (lines 50-118) that handles folding
audit events with error handling and version logic. Add comprehensive unit tests
for the `derive_template_registry` function in a tests module adjacent to the
function definition. The tests should cover key scenarios including successful
audit event processing, error handling paths, and version logic to ensure the
function behaves correctly across different inputs and edge cases.
---
Nitpick comments:
In `@crates/ourios-querier/tests/rfc0017_registry.rs`:
- Around line 62-112: The fixture currently hardcodes template strings like
"user <*>", "user <*> <*>", "GET <*>", and "<*> <*> <*>" in the TemplateChange
enum variants (Created, Widened, TypeExpanded, RejectedDegenerate), which can
drift if canonical formatting changes. Instead of hardcoding these strings,
construct them using the ourios_miner::tree::format_template function to ensure
the test fixtures match the actual miner output and maintain canonical parity
automatically.
🪄 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: 2af30f2d-d730-44dd-979c-82c031e21a23
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
crates/ourios-miner/src/cluster.rscrates/ourios-miner/src/tree.rscrates/ourios-querier/Cargo.tomlcrates/ourios-querier/src/lib.rscrates/ourios-querier/src/template_registry.rscrates/ourios-querier/tests/rfc0017_registry.rsdocs/rfcs/0017-template-registry-query-rendering.md
CodeRabbit (testing guidelines): - ourios-miner: add a proptest round-trip for the template encode/decode invariant (parse_template . format_template == id) over arbitrary token sequences, alongside the example-based tests (CLAUDE.md §6.2 "reconstruction is always a property test"). Adds proptest dev-dep. - ourios-querier: factor the pure fold out of derive_template_registry into fold_registry, and add module-local unit tests for it (version keying: created→v1, widened/type_expanded→new_version; rejection skip; later version doesn't clobber; same-(id,version) last-wins by timestamp). The row-vs-path tenant backstop is defense-in-depth: AuditWriter rejects a tenant/partition mismatch at write time, so a foreign-tenant row is unreachable through the supported write path (documented note; mirrors the untested-at-unit-level backstop in alias_store::derive_alias_map). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 21, 2026
This was referenced Jul 7, 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.
RFC 0017 green
.2—derive_template_registrySecond green slice. The read-time template registry, derived from the tenant audit stream exactly like
alias_store::derive_alias_map(sameaudit_scanwalk, same(timestamp, path, row)total fold order).Changes
ourios-miner— addtree::parse_template(the inverse offormat_template): split the canonical space-joinedlit … <*>string back intoVec<OwnedToken>. Movedformat_templatefromcluster.rsintotree.rs(nowpub) so the canonical encode/decode pair lives together next toOwnedToken, with a round-trip unit test. The 6 in-crate callers are unchanged (import).ourios-querier— now depends onourios-miner(forOwnedToken+ the registry, andreconstruct::renderin.3; no arrow/DataFusion type crosses). Newtemplate_registrymodule exposingderive_template_registry(bucket_root, tenant) -> Result<TemplateRegistry, QueryError>andTemplateRegistry = HashMap<(u64, u32), Vec<OwnedToken>>.Behaviour
Fold per template event:
template_created→ key(id, TEMPLATE_INITIAL_VERSION)(the variant omits the version);template_widened/template_type_expanded→ key(id, new_version);template_widening_rejected_degenerate→ nothing (no version bump, no token change).Keying by
(id, version)is what makes a later widening never clobber an earlier version's tokens — the prerequisite for version-correct rendering (.5,.3). Same tenant-row backstop as the alias derivation (RFC 0005 §3.9).Tests
created+widened+type_expanded+rejectedevents via the productionParquetAuditSink, derives, and asserts every(id, version)is present including v1, v2 doesn't clobber v1, and the rejection adds no entry..5stays#[ignore]d until.3(it needs the render path).parse_template ∘ format_template.format_templatewrites, not a JSON array — follow the code).All
ourios-miner/ourios-queriertests green;fmt --check+clippy -D warningsclean.Next:
.3(query-time rendering —LogBodythree zones viareconstruct::render).🤖 Generated with Claude Code
Summary by CodeRabbit