docs(rfc-0001): specify the operator-driven alias-index write path - #151
Conversation
Resolve the RFC 0001 §9 open question "alias index creation mechanism" with operator-driven + audited aliasing, the mechanism §3.1 (no silent merges) demands. §6.7: replace the "no creation event / produced out of band" deferral with the alias write path — the alias model (per-tenant equivalence class of template_ids, cross-leaf only, canonical representative), the alias_asserted / alias_retracted audit events (WAL-before-ack durable per §3.4), the per-tenant alias-map projection folded from the durable event log, eventual-consistency / staleness semantics, the resolves_to reader contract (RFC0002.9), and reversibility. Adds a Mermaid assert→audit→materialize→query sequence diagram and defers automatic inference to a future propose→operator-confirm layer. §5.3: add acceptance criteria RFC0001.12–RFC0001.16 (durable assertion + map appearance; resolves_to set expansion; cross-tenant isolation per §3.7; retraction; non-aliased id resolves to itself). §8 maps them to an alias write-path test entry. §9: mark the open question RESOLVED, noting automatic inference as remaining future work and the physical alias-map file/format as an RFC 0005 storage decision. Status: red -> specified (new criteria re-enter the ladder; precedent: the RFC 0003 served-binary amendment). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 48 minutes and 3 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ 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 (1)
📝 WalkthroughWalkthroughRFC 0001 status set to specified; adds RFC0001.12–RFC0001.16 acceptance scenarios and replaces the alias index placeholder with an operator-driven audited write path (alias_asserted/alias_retracted), per-tenant projection folding, resolves_to expansion rules, retraction semantics, and corresponding tests. ChangesAlias Index Mechanism Specification & Acceptance Scenarios
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
This PR updates RFC 0001 to specify the operator-driven, audited write path for the drift-alias-index (resolving the former §9 open question), and updates the RFC’s maturity/status accordingly.
Changes:
- RFC frontmatter status moved to
specifiedand new alias-focused acceptance criteria (RFC0001.12–.16) were added to §5.3. - §6.7 now specifies the alias model, audit events (
alias_asserted/alias_retracted), projection/materialization semantics, and theresolves_to(X)reader contract (with an added Mermaid sequence diagram). - §8 test traceability and §9 open-questions checklist were updated to reflect the resolved alias-write-path decision.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/rfcs/0001-template-miner.md (1)
1296-1300:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
AuditEventTypecontract is internally inconsistent after introducing alias events.
§6.4still definesAuditEventTypeas only the three template-widening variants, but§6.7addsalias_asserted/alias_retractedon the same audit stream. This leaves the normative event schema ambiguous for implementers and tests.Proposed doc fix
{ - event_type: AuditEventType, # enum: - # template_widened - # template_type_expanded - # template_widening_rejected_degenerate + event_type: AuditEventType, # enum: + # template_widened + # template_type_expanded + # template_widening_rejected_degenerate + # alias_asserted + # alias_retracted tenant_id: TenantId, - template_id: u64, - old_version: u32, - new_version: u32, - old_template: String, # canonical form, with <*> for wildcards - new_template: String, + # template_* events: + template_id: u64, + old_version: u32, + new_version: u32, + old_template: String, # canonical form, with <*> for wildcards + new_template: String, ... + # alias_* events use alias payload per §6.7: + # representative_id, member_ids, actor, reason, timestamp }Also applies to: 1543-1562
🤖 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 `@docs/rfcs/0001-template-miner.md` around lines 1296 - 1300, The AuditEventType enum and its usage are inconsistent: update the normative schema so the event_type field and the AuditEventType definition (referenced in §6.4) explicitly include the alias_asserted and alias_retracted variants (or clearly document a separate alias audit stream) and reconcile §6.7 to reference the same enum; update all occurrences (including examples and schema blocks around AuditEventType, §6.4, and §6.7) so tests and implementers see a single authoritative definition that lists template_widened, template_type_expanded, template_widening_rejected_degenerate, alias_asserted, and alias_retracted (or state the intentional separation if you choose a separate stream) and ensure the event_type field examples and any normative language reflect that choice.
🤖 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 `@docs/rfcs/0001-template-miner.md`:
- Around line 1546-1562: The fenced code block showing the audit event structure
(keys like event_type, tenant_id, representative_id, member_ids, actor, reason,
timestamp) is missing a language tag; update the opening ``` to include a
language identifier (e.g., change ``` to ```text) so the markdown linter (MD040)
recognizes the block and suppresses the noisy warning.
---
Outside diff comments:
In `@docs/rfcs/0001-template-miner.md`:
- Around line 1296-1300: The AuditEventType enum and its usage are inconsistent:
update the normative schema so the event_type field and the AuditEventType
definition (referenced in §6.4) explicitly include the alias_asserted and
alias_retracted variants (or clearly document a separate alias audit stream) and
reconcile §6.7 to reference the same enum; update all occurrences (including
examples and schema blocks around AuditEventType, §6.4, and §6.7) so tests and
implementers see a single authoritative definition that lists template_widened,
template_type_expanded, template_widening_rejected_degenerate, alias_asserted,
and alias_retracted (or state the intentional separation if you choose a
separate stream) and ensure the event_type field examples and any normative
language reflect that choice.
🪄 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: 90376458-ac37-4f92-9768-89e13cfd30a1
📒 Files selected for processing (1)
docs/rfcs/0001-template-miner.md
…e detail Maintainer review of #151: the numerically-smallest-member selection is an implementation detail, not a contract — it may evolve (e.g. operator- designated representative) without changing alias-set semantics, since resolves_to expands by membership (RFC0001.13). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…emetry
Make the canonical representative a derived display convenience
(min member), with membership the only contract-bearing notion;
define the asserted set as the full union {representative_id} ∪
member_ids so retraction is well-defined for any member including
the canonical. Add alias_assertions_total / alias_retractions_total
to the §6.8 telemetry table, tag the audit-event fence as text, and
extend the staleness discussion to cover retraction over-inclusion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Resolves the RFC 0001 §9 open question "alias index creation mechanism" (the drift-alias-index write path). DOC-ONLY — no Rust.
The maintainer chose (2026-06-07) operator-driven + audited aliasing over automatic-inference and "deferred entirely", precisely because §3.1 forbids silent template merges ("every merge emits an audit event; explicit"). An auto-aliased cross-semantic merge is a silent merge by another name.
The mechanism:
resolves_to(n)→template_id IN (alias set).What changed (all in
docs/rfcs/0001-template-miner.md)template_ids, cross-leaf only, canonical = numerically smallest representative; cross-version stays thetemplate_versionaxis), thealias_asserted/alias_retractedaudit events (field list in §6.4 schema style), materialization + storage venue analysis, eventual-consistency/staleness semantics, theresolves_toreader contract, reversibility, a Mermaid assert→audit→materialize→query sequence diagram, and the automatic-inference future-work note.New §5 acceptance criteria
resolves_to(rep)returns all members; expansion is by the set (rep or member); a non-member resolves to{C}.alias_retractedand removes membership on rebuild.template_id = X, RFC0001.6).Storage venue choice + RFC 0005 split
Of three venues — (a) per-tenant projection from the durable audit/alias event log (chosen), (b) tenant-root
aliases.parquet/JSON written directly, (c) extend the partitionManifest— (a) wins: it reuses the §6.4 audit infra (no new write plane), matches the tenant scope of the data (§3.7), and keeps an append-only/replayable source of truth (the event log) with a rebuildable map cache. (b) is rejected as the source of truth (a directly-mutated file has no audit trail) but survives as the serialization format. (c) is rejected because the manifest is partition-scoped while alias sets are tenant-global.RFC 0001 owns the model + write path + criteria. RFC 0005 (storage) owns the physical alias-map file/format and the snapshot cadence (the staleness bound). Sibling to the issue #147 split.
Status transition
red→specified. Following the RFC 0003 served-binary amendment precedent: adding new acceptance criteria re-enters the ladder atspecifieduntil RFC0001.12–.16 have failing test stubs (red) and then pass (green).Invariants touched
§3.1 (no silent merges — aliasing is explicit + audited + reversible), §3.4 (WAL-before-ack — alias events inherit the §6.4 durability barrier), §3.7 (per-tenant — alias sets are tenant-scoped, RFC0001.14 locks isolation). Reader contract stays consistent with RFC 0002 RFC0002.9.
Verification
mdbook buildsucceeds; the only warning is the pre-existing mdbook-mermaid version note (built against 0.5.0, called from 0.5.2). The new Mermaid sequence diagram renders (class="mermaid"present in the built HTML). DOC-ONLY change — no cargo run needed.🤖 Generated with Claude Code
Summary by CodeRabbit