feat(rfc0017.1): audit leaf creation via TemplateChange::Created - #273
Merged
Conversation
RFC 0017 §3.1 — a read-time template registry must recover a leaf's v1
tokens after the originating rows age out, but initial creation was
unaudited. Add the audit event so derivation (.2) is complete:
- ourios-core: TemplateChange::Created { new_version, new_template };
event_kind ordinal 6 / event_type "template_created" (append-only per
RFC 0005 §3.7, existing 0-5 unchanged). Not a merge.
- ourios-parquet: writer maps Created to (new_version, new_template) with
old_* NULL (no prior template); reader reconstructs Created before the
old_* require (they are NULL for this kind). Round-trip test added.
- ourios-miner: create_new_leaf emits Created (canonical tokens via
format_template) on the same AuditSink path as the widening events,
for both the fresh-leaf and lossy-zone allocation sites.
Contract change (surfaced): RFC0001.1 ("fresh-leaf creation does not emit
an audit event") is superseded by RFC 0017 §3.1 and overturned in its
tests; creation remains a non-merge so merges_total is unchanged. Widening
/ rejection tests filter the new leading Created event (drain_changes
helper) to stay decoupled from the per-leaf creation count; the creation
contract has dedicated coverage (RFC0017.1 + fresh_leaf_emits_created_event).
Fills the RFC0017.1 red stubs (core append-only schema + miner emit) and
removes their #[ignore].
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 45 minutes and 37 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 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 refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. 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 (4)
📝 WalkthroughWalkthroughThe PR introduces a new ChangesTemplateChange::Created audit event end-to-end
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ingest_string
participant create_new_leaf
rect rgba(70, 130, 180, 0.5)
Note over ingest_string,create_new_leaf: Fresh-leaf allocation path
Caller->>ingest_string: raw log line
ingest_string->>create_new_leaf: raw, record, tokens (no candidate match)
create_new_leaf->>create_new_leaf: insert leaf into self.tenants (borrow scoped + released)
create_new_leaf->>create_new_leaf: compute canonical template string
end
rect rgba(60, 179, 113, 0.5)
Note over create_new_leaf,audit_sink: Audit emission (version 1)
create_new_leaf->>audit_sink: emit AuditEvent(TemplateChange::Created{new_template})
audit_sink-->>create_new_leaf: ok
end
create_new_leaf-->>ingest_string: leaf_id
ingest_string-->>Caller: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new audit event for fresh leaf creation so a future read-time template registry (RFC 0017) can recover a leaf’s version-1 tokens even after originating rows age out. This extends the audit stream contract in an append-only way (new event_kind ordinal + new event_type) and wires the event through miner emit + Parquet writer/reader, with updated RFC/acceptance coverage.
Changes:
ourios-core: IntroducesTemplateChange::Created { new_version, new_template }plusEVENT_KIND_TEMPLATE_CREATED = 6/EVENT_TYPE_TEMPLATE_CREATED = "template_created", and maps kind/type/merge-count behavior accordingly.ourios-miner: EmitsTemplateChange::Createdfromcreate_new_leaf(both normal and lossy-zone allocations) and updates/extends tests to account for the new leading creation event.ourios-parquet: WritesCreatedwithold_* = NULL, readsCreatedbefore requiringold_*, and adds a round-trip test for the new variant.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/rfcs/0017-template-registry-query-rendering.md | Documents RFC 0017’s amendment to RFC 0001 regarding creation audit emission. |
| crates/ourios-core/src/audit.rs | Adds TemplateChange::Created and new kind/type constants + mapping. |
| crates/ourios-core/tests/rfc0017_audit.rs | Pins append-only ordinal/type contract and Created’s kind/type + merge semantics. |
| crates/ourios-miner/src/cluster.rs | Emits template_created on leaf allocation; introduces drain_changes helper for tests. |
| crates/ourios-miner/tests/rfc0017_template_created.rs | New miner-side acceptance test ensuring leaf allocation emits Created. |
| crates/ourios-miner/tests/rfc_internal.rs | Updates superseded RFC0001.1 scenario and filters out Created for widening-focused scenarios. |
| crates/ourios-miner/tests/hazards.rs | Filters out Created so widening assertions remain focused. |
| crates/ourios-parquet/src/audit_record_batch.rs | Writes Created with NULL old_* columns and empty list fields. |
| crates/ourios-parquet/src/audit_reader.rs | Reads Created correctly before requiring old_* columns. |
| crates/ourios-parquet/tests/audit_round_trip.rs | Adds a Parquet writer→reader round-trip test for template_created. |
💡 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ourios-miner/src/cluster.rs (1)
1434-1437:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace stale “no audit event” wording with “no non-creation change.”
Creation now emits
TemplateChange::Created; these comments/messages still describe the old RFC0001.1 contract while the assertions intentionally filter Created viadrain_changes.Suggested wording update
- /// RFC0001.1: this path **does not** emit an audit event — - /// `template_count` already reflects the allocation and - /// `merges_total` is reserved for widening / type-expansion - /// events on existing leaves. + /// RFC 0017 §3.1: this path emits a `TemplateChange::Created` + /// audit event for the version-1 template. Creation is not a + /// merge, so `merges_total` remains reserved for widening / + /// type-expansion events on existing leaves.- // Fresh-leaf creation does NOT emit an audit event - // (RFC0001.1) — even with non-empty slot_types. + // Fresh-leaf creation emits `Created`; this assertion only + // checks there are no non-creation template-change events.- // rather than widening). No audit events. + // rather than widening). No non-creation audit events.- // no audit event, no + // no non-creation audit event, no ... - "lossy attach emits no audit event" + "lossy attach emits no non-creation audit event"Also applies to: 2951-2953, 3467-3469, 4107-4124
🤖 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-miner/src/cluster.rs` around lines 1434 - 1437, The comment block describing the audit event behavior is outdated and no longer reflects the current implementation where creation events now emit TemplateChange::Created. Update the comment wording from stating "does not emit an audit event" to clarify that "does not emit a non-creation change" since assertions intentionally filter out Created events via drain_changes. Apply this same wording update to all other occurrences of this pattern at lines 2951-2953, 3467-3469, and 4107-4124 in the cluster.rs file to ensure consistency across the codebase.
🧹 Nitpick comments (1)
crates/ourios-miner/src/cluster.rs (1)
2771-2789: ⚡ Quick winPin the Created payload to the leaf’s canonical template.
This test would still pass if
new_templatewere any non-empty placeholder. Compare it with the created leaf’s canonical template so the RFC0017 recovery payload is actually specified.Suggested stronger assertion
+ let templates = cluster.templates_for(&t); - for (event, id) in events.iter().zip([id_a, id_b]) { + for (event, (id, raw)) in events + .iter() + .zip([(id_a, "user 42 logged in"), (id_b, "GET /home 200")]) + { let AuditPayload::Template { template_id, + triggering_line_hash, + triggering_line_sample, change: TemplateChange::Created { new_version, new_template, @@ }; assert_eq!(*template_id, id); assert_eq!(*new_version, 1, "a leaf is created at version 1"); - assert!( - !new_template.is_empty(), - "creation carries the initial tokens", - ); + let snapshot = templates + .iter() + .find(|snapshot| snapshot.template_id == id) + .expect("created template id has a leaf snapshot"); + let expected_template = format_template(&snapshot.template); + assert_eq!( + new_template, &expected_template, + "creation carries the canonical initial tokens", + ); + assert_eq!(*triggering_line_hash, hash_triggering_line(raw.as_bytes())); + assert_eq!(triggering_line_sample.as_deref(), Some(raw)); }🤖 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-miner/src/cluster.rs` around lines 2771 - 2789, The assertion for new_template in the AuditPayload::Template/Created destructuring only checks that new_template is non-empty, which is insufficient for validating the RFC0017 recovery payload specification. Instead of just asserting that new_template is not empty, compare it against the canonical template of the created leaf using assert_eq to ensure the payload contains the correct expected template content. You will need to access the created leaf's canonical template data and compare it directly with the new_template variable extracted from the payload.
🤖 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-miner/src/cluster.rs`:
- Around line 1434-1437: The comment block describing the audit event behavior
is outdated and no longer reflects the current implementation where creation
events now emit TemplateChange::Created. Update the comment wording from stating
"does not emit an audit event" to clarify that "does not emit a non-creation
change" since assertions intentionally filter out Created events via
drain_changes. Apply this same wording update to all other occurrences of this
pattern at lines 2951-2953, 3467-3469, and 4107-4124 in the cluster.rs file to
ensure consistency across the codebase.
---
Nitpick comments:
In `@crates/ourios-miner/src/cluster.rs`:
- Around line 2771-2789: The assertion for new_template in the
AuditPayload::Template/Created destructuring only checks that new_template is
non-empty, which is insufficient for validating the RFC0017 recovery payload
specification. Instead of just asserting that new_template is not empty, compare
it against the canonical template of the created leaf using assert_eq to ensure
the payload contains the correct expected template content. You will need to
access the created leaf's canonical template data and compare it directly with
the new_template variable extracted from the payload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b255da2c-09de-45ec-9564-a7ebf7f73f5a
📒 Files selected for processing (10)
crates/ourios-core/src/audit.rscrates/ourios-core/tests/rfc0017_audit.rscrates/ourios-miner/src/cluster.rscrates/ourios-miner/tests/hazards.rscrates/ourios-miner/tests/rfc0017_template_created.rscrates/ourios-miner/tests/rfc_internal.rscrates/ourios-parquet/src/audit_reader.rscrates/ourios-parquet/src/audit_record_batch.rscrates/ourios-parquet/tests/audit_round_trip.rsdocs/rfcs/0017-template-registry-query-rendering.md
Copilot: two test comments still described the pre-RFC0017.1 contract
("fresh-leaf creation does NOT emit an audit event" / "No audit events").
Leaf creation now emits a Created event; drain_changes filters it, so the
assertions check for no widening/type-expansion/rejection events. Comments
updated to match.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot flagged that TemplateChange::Created carried new_version while being documented as always 1 — a representable invalid state the writer/ reader would persist/accept. Rather than validate it at the serialization boundary (the suggestion), drop the field: a leaf is always born at v1, so the variant now carries only new_template and the invariant is unrepresentable (nothing to validate — the strongest form). The writer supplies the canonical on-disk new_version via the new TEMPLATE_INITIAL_VERSION constant; the reader does not read it back into the variant. Updated all construction/match sites + RFC §3.1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot doc-accuracy nits (no behavior change): - create_new_leaf doc + the module-doc decision-tree bullet now say leaf creation emits a TemplateCreated event (RFC 0017 §3.1), not 'no audit event' (the superseded RFC0001.1 wording). - the parquet round-trip test doc and the miner emit-test module doc no longer claim new_version is carried/survives — the variant omits it; the on-disk new_version=1 is supplied by the writer. - normalize the RFC's conceptual 'TemplateCreated' references to the concrete event_type 'template_created' to avoid code/schema cross-ref confusion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot: the module decision-tree bullet named a 'TemplateCreated' type that doesn't exist. Use the concrete in-memory variant (TemplateChange::Created) + on-disk event_type (template_created) so the doc cross-references the code and Parquet schema. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
.1— audit leaf creation (TemplateChange::Created)First green slice of RFC 0017. A read-time template registry (
.2) must recover a leaf's version-1 tokens after the originating rows age out, but the miner never audited initial leaf creation — only widenings. This adds the missing event.Changes
ourios-core(audit.rs): newTemplateChange::Created { new_version, new_template };event_kindordinal 6 /event_typetemplate_created— append-only per RFC 0005 §3.7 (existing0–5unchanged, old readers tolerate via theUnknownpath). Not a merge.ourios-parquet: writer mapsCreatedto(new_version, new_template)withold_version/old_templateNULL (no prior template); reader handlesCreatedbefore theold_*requires (which would reject those NULLs for the widening kinds). Writer→reader round-trip test added.ourios-miner(cluster.rs):create_new_leafemitsCreated(canonical tokens viaformat_template) on the sameAuditSink/ WAL-before-ack path as the widening events — covering both the fresh-leaf and lossy-zone allocation sites.Contract change (explicitly surfaced)
RFC 0017 §3.1 supersedes RFC0001.1 ("fresh-leaf creation does not emit an audit event"). Leaf creation now emits a
template_createdevent. It remains a non-merge (merges_totalunchanged), so RFC 0001's merge-counting contract is untouched. The RFC0001.1 acceptance test (rfc_internal.rs) is overturned to assert the amended contract; the amendment is recorded in RFC 0017 §1.Widening/type-expansion/rejection tests now filter the leading
Createdevent (drain_changeshelper / inline filters) so they stay decoupled from the per-leaf creation count — not a weakening: the creation contract has dedicated coverage (fresh_leaf_emits_created_event+ the RFC0017.1 acceptance tests).Tests
#[ignore].CreatedParquet round-trip.ourios-core/ourios-parquet/ourios-minertests green locally;cargo fmt --check+clippy -D warningsclean.Next:
.2(derive_template_registry).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
template_created, kind ordinal6) with canonical template content and an initial version invariant.Bug Fixes
Tests
Documentation