Merged
Conversation
Summary by CodeRabbit
WalkthroughThis PR adds engineering documentation that defines the stable authoring boundary between the shared runtime substrate and engine-local domain semantics. It includes a new canonical contract document, updates the documentation index, and clarifies the authoring constraint in the M13 roadmap milestone. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/engine-authoring-boundary.md`:
- Around line 196-201: The "Next Step" section currently instructs the reader to
"write the focused runtime-vs-engine contract note" even though this file is
that contract; update the paragraph and bullet list so it reads as the
post-contract decision point: remove or replace the first bullet ("write the
focused runtime-vs-engine contract note") and rephrase the opening sentence to
"With this contract in place, the next M13 step is narrower:" (or similar), then
ensure bullets only cover: decide whether the contract makes a reduced-copy
proof likely enough, and only then choose whether M14 still needs a full
fourth-engine or can use a smaller engine-slice proof; preserve references to
M13/M14 and the decision sequencing in the updated text.
- Around line 1-201: Add an explicit pointer from the runtime-extraction-roadmap
document to the new engine-authoring-boundary document by updating the section
that describes M13 ("Internal Engine Authoring Contract") to reference or link
to engine-authoring-boundary.md as the canonical M13 authoring contract (look
for the M13 heading/title in runtime-extraction-roadmap.md). Also update any
mention in status.md that currently claims the roadmap defines the boundary so
it instead points readers to engine-authoring-boundary.md as the authoritative
source (search for the phrase that references the roadmap/boundary in
status.md).
In `@docs/status.md`:
- Line 220: Update docs/status.md: the current "next roadmap step" referencing
M13 and runtime-extraction-roadmap.md is stale because this PR added that
contract; change the single-line text (around the M13 mention) to the next
actionable step (for example: "M13: validate contract implementation / run
contract validation tests and prepare M14 decision gate" or explicitly move to
"M14 decision gate") so the file reflects that the contract document exists and
lists the next work item (contract validation or M14 decision gate) as the
actionable next step.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 35b70265-4e12-4245-823b-c3646e17c4de
📒 Files selected for processing (3)
docs/README.mddocs/engine-authoring-boundary.mddocs/status.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: semgrep-cloud-platform/scan
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep documentation up to date with the code and design. If a change affects behavior, invariants, failure modes, operational semantics, testing strategy, or implementation sequencing, update the relevant docs in the same task or PR.
Files:
docs/README.mddocs/status.mddocs/engine-authoring-boundary.md
docs/status.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep
docs/status.mdcurrent as the single-file progress snapshot for the repository. Update it whenever milestone state, implementation coverage, or the recommended next step materially changes.
Files:
docs/status.md
🧠 Learnings (2)
📚 Learning: 2026-03-12T15:18:53.086Z
Learnt from: CR
Repo: skel84/allocdb PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-12T15:18:53.086Z
Learning: Applies to **/*.md : Keep documentation up to date with the code and design. If a change affects behavior, invariants, failure modes, operational semantics, testing strategy, or implementation sequencing, update the relevant docs in the same task or PR.
Applied to files:
docs/README.mddocs/status.md
📚 Learning: 2026-03-12T15:18:53.086Z
Learnt from: CR
Repo: skel84/allocdb PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-12T15:18:53.086Z
Learning: Applies to docs/status.md : Keep [`docs/status.md`](./docs/status.md) current as the single-file progress snapshot for the repository. Update it whenever milestone state, implementation coverage, or the recommended next step materially changes.
Applied to files:
docs/status.md
🔇 Additional comments (1)
docs/README.md (1)
20-20: Good docs index update.Adding the new boundary doc to the Engineering Docs index is the right integration point.
Comment on lines
+1
to
+201
| # Engine Authoring Boundary | ||
|
|
||
| ## Purpose | ||
|
|
||
| This document closes the first `M13` question: after the `M12` micro-extractions, what is the | ||
| stable boundary between the shared runtime substrate and engine-local semantics? | ||
|
|
||
| This is an internal authoring rule, not a public framework story. | ||
|
|
||
| ## Decision | ||
|
|
||
| The shared runtime owns only mechanically shared durability and bounded-state substrate. | ||
|
|
||
| Everything that defines domain meaning stays engine-local. | ||
|
|
||
| That means: | ||
|
|
||
| - use the extracted runtime crates where the seam is already proven | ||
| - keep new engine semantics local by default | ||
| - treat any new generic abstraction above the current substrate as suspect until repeated pressure | ||
| proves otherwise | ||
|
|
||
| ## What The Shared Runtime Owns Today | ||
|
|
||
| The shared runtime currently owns only the modules that are already shared on `main`. | ||
|
|
||
| ### Shared and stable enough now | ||
|
|
||
| - `allocdb-retire-queue` | ||
| - bounded retirement queue discipline | ||
| - no domain meaning beyond ordered retirement bookkeeping | ||
| - `allocdb-wal-frame` | ||
| - WAL frame versioning | ||
| - frame header/footer validation | ||
| - checksum verification | ||
| - torn-tail and corruption detection at the frame level | ||
| - `allocdb-wal-file` | ||
| - append-only durable file handle | ||
| - replace/rewrite discipline | ||
| - truncation and reopen behavior | ||
|
|
||
| ### What these modules are allowed to know | ||
|
|
||
| Only substrate concerns: | ||
|
|
||
| - bytes | ||
| - lengths | ||
| - checksums | ||
| - file paths and file handles | ||
| - bounded queue mechanics | ||
| - ordering and truncation discipline | ||
|
|
||
| These modules must not know: | ||
|
|
||
| - command schemas | ||
| - result codes | ||
| - resource, bucket, pool, or hold semantics | ||
| - snapshot schemas | ||
| - engine-specific invariants | ||
|
|
||
| ## What Stays Engine-Local | ||
|
|
||
| Each engine still owns the parts that define the database itself. | ||
|
|
||
| ### Domain contract surface | ||
|
|
||
| Keep local: | ||
|
|
||
| - command enums | ||
| - command codecs above raw frame transport | ||
| - result codes and read models | ||
| - config surfaces tied to domain semantics | ||
|
|
||
| ### Persistence schema | ||
|
|
||
| Keep local: | ||
|
|
||
| - snapshot encoding and decoding | ||
| - snapshot file wrappers while file formats still differ | ||
| - engine-specific recovery error surfaces | ||
|
|
||
| ### State machine semantics | ||
|
|
||
| Keep local: | ||
|
|
||
| - apply rules | ||
| - invariants | ||
| - derived indexes | ||
| - logical-slot effects such as refill, expiry, revoke, reclaim, or fencing | ||
| - any internal command semantics above raw WAL framing | ||
|
|
||
| ### Recovery entry points | ||
|
|
||
| Keep local: | ||
|
|
||
| - top-level recovery APIs | ||
| - replay orchestration that depends on engine-specific command decoding | ||
| - operational logging tied to one engine's semantics | ||
|
|
||
| ## Authoring Rules For Future Work | ||
|
|
||
| ### Rule 1: Start local unless the seam is already proven | ||
|
|
||
| When adding a new engine or engine slice: | ||
|
|
||
| - use the shared runtime crates only for seams already extracted | ||
| - keep new runtime-adjacent code local until at least two engines want the same thing in the same | ||
| shape | ||
|
|
||
| ### Rule 2: Do not generalize state-machine APIs | ||
|
|
||
| Do not introduce: | ||
|
|
||
| - generic state-machine traits | ||
| - generic apply pipelines | ||
| - generic snapshot schemas | ||
| - generic recovery entry points | ||
|
|
||
| Those layers still carry domain meaning and would create abstraction debt faster than maintenance | ||
| relief. | ||
|
|
||
| ### Rule 3: Extract only below the semantic line | ||
|
|
||
| A module is a good runtime candidate only if it can stay below the line where domain meaning starts. | ||
|
|
||
| Good examples: | ||
|
|
||
| - bytes-on-disk framing | ||
| - bounded retirement bookkeeping | ||
| - file rewrite/truncate mechanics | ||
|
|
||
| Bad examples: | ||
|
|
||
| - "generic reserve/confirm/release" APIs | ||
| - "generic bucket/pool/resource" models | ||
| - "generic engine config" layers | ||
|
|
||
| ### Rule 4: Prefer duplication over dishonest abstraction | ||
|
|
||
| If a candidate seam requires: | ||
|
|
||
| - engine-specific branches | ||
| - feature flags that mirror engine names | ||
| - generic types that only one engine can actually use | ||
|
|
||
| then it is not ready. | ||
|
|
||
| ### Rule 5: New extractions need multi-engine pressure | ||
|
|
||
| Do not extract a new runtime module unless at least one of these is true: | ||
|
|
||
| - the code is already mechanically identical across engines | ||
| - the same fix or improvement is landing independently in multiple engines | ||
| - a new engine authoring pass clearly pays less copy-paste by using the shared layer | ||
|
|
||
| ## Current Boundary Map | ||
|
|
||
| ### Shared runtime | ||
|
|
||
| - `allocdb-retire-queue` | ||
| - `allocdb-wal-frame` | ||
| - `allocdb-wal-file` | ||
|
|
||
| ### Deferred seams | ||
|
|
||
| - `snapshot_file` | ||
| - deferred because the seam is still only clean inside the `quota-core` / `reservation-core` | ||
| pair | ||
| - bounded collections beyond `retire_queue` | ||
| - still need proof that the common surface is stable enough | ||
| - recovery helpers above file/frame mechanics | ||
| - still too tied to engine-local replay contracts | ||
|
|
||
| ### Explicit non-goals | ||
|
|
||
| - no public database-building library claim yet | ||
| - no renaming the repository around framework identity | ||
| - no generic engine kit above the current substrate | ||
|
|
||
| ## Practical Consequence | ||
|
|
||
| A future engine author should think in this order: | ||
|
|
||
| 1. write engine-local semantics first | ||
| 2. consume the existing shared runtime only for proven substrate | ||
| 3. copy new runtime-adjacent code locally if the seam is not already explicit | ||
| 4. extract later only if repeated pressure proves the boundary | ||
|
|
||
| That keeps the repository honest: | ||
|
|
||
| - shared where the code is actually shared | ||
| - local where the semantics are still the database | ||
|
|
||
| ## Next Step | ||
|
|
||
| With this boundary in place, the next `M13` step is narrower: | ||
|
|
||
| 1. write the focused runtime-vs-engine contract note | ||
| 2. decide whether that contract already makes a reduced-copy proof likely enough | ||
| 3. only then choose whether `M14` still needs a full fourth-engine or can use a smaller engine | ||
| slice proof |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether roadmap includes/links the new boundary contract and reflects canonical ownership.
fd runtime-extraction-roadmap.md
rg -n -C2 'engine-authoring-boundary|semantic line|M13|authoring boundary|canonical' docs/runtime-extraction-roadmap.md docs/status.md docs/README.mdRepository: skel84/allocdb
Length of output: 2922
Update docs/runtime-extraction-roadmap.md to reference engine-authoring-boundary.md as the canonical M13 authoring contract.
The roadmap doc currently describes M13 as "Internal Engine Authoring Contract" but does not reference or delegate to the newly added engine-authoring-boundary.md file. Meanwhile, status.md still refers to the roadmap as the location where this boundary is defined. Per the coding guidelines, documentation must stay aligned with design and code changes. The roadmap should explicitly reference the boundary contract document so readers know where the canonical authoring rules live.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/engine-authoring-boundary.md` around lines 1 - 201, Add an explicit
pointer from the runtime-extraction-roadmap document to the new
engine-authoring-boundary document by updating the section that describes M13
("Internal Engine Authoring Contract") to reference or link to
engine-authoring-boundary.md as the canonical M13 authoring contract (look for
the M13 heading/title in runtime-extraction-roadmap.md). Also update any mention
in status.md that currently claims the roadmap defines the boundary so it
instead points readers to engine-authoring-boundary.md as the authoritative
source (search for the phrase that references the roadmap/boundary in
status.md).
Comment on lines
+196
to
+201
| With this boundary in place, the next `M13` step is narrower: | ||
|
|
||
| 1. write the focused runtime-vs-engine contract note | ||
| 2. decide whether that contract already makes a reduced-copy proof likely enough | ||
| 3. only then choose whether `M14` still needs a full fourth-engine or can use a smaller engine | ||
| slice proof |
There was a problem hiding this comment.
“Next Step” section contradicts this newly added contract doc.
Line 198 says the next step is to write the contract note, but this file is already that contract. Please update this section to the post-contract decision point to avoid sequencing ambiguity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/engine-authoring-boundary.md` around lines 196 - 201, The "Next Step"
section currently instructs the reader to "write the focused runtime-vs-engine
contract note" even though this file is that contract; update the paragraph and
bullet list so it reads as the post-contract decision point: remove or replace
the first bullet ("write the focused runtime-vs-engine contract note") and
rephrase the opening sentence to "With this contract in place, the next M13 step
is narrower:" (or similar), then ensure bullets only cover: decide whether the
contract makes a reduced-copy proof likely enough, and only then choose whether
M14 still needs a full fourth-engine or can use a smaller engine-slice proof;
preserve references to M13/M14 and the decision sequencing in the updated text.
| - PR `#107` merged the `M10` quota-engine proof on `main`, and PRs `#116`, `#117`, and `#118` merged the full `M11` reservation-core chain on `main`: the repository now has a second and third deterministic engine with bounded command sets, logical-slot refill/expiry, and snapshot/WAL recovery proofs | ||
| - PRs `#132`, `#133`, and `#134` merged the first `M12` runtime extractions on `main`: `retire_queue`, `wal`, and `wal_file` are now shared internal substrate instead of copied engine-local modules, while `M12-T04` closed as a defer decision because `snapshot_file` is still only a clean seam inside the `quota-core` / `reservation-core` pair and `allocdb-core` keeps the simpler file format | ||
| - the next roadmap step is now `M13`: define the internal engine authoring boundary in `runtime-extraction-roadmap.md` and stop extraction pressure until that contract is written down | ||
| - the next roadmap step is now `M13`: define the internal engine authoring boundary in `runtime-extraction-roadmap.md` and stop extraction pressure until that contract is written down; the authoring rule is to keep shared runtime below the semantic line and keep command surfaces, snapshot schemas, recovery entry points, and state-machine meaning engine-local |
There was a problem hiding this comment.
docs/status.md next-step text is stale after this PR.
Line 220 still says the next step is to define the boundary contract, but this PR already introduces that contract document. Please advance this line to the next actionable M13 step (e.g., contract validation / M14 decision gate) so status remains a true snapshot.
As per coding guidelines: "Keep docs/status.md current as the single-file progress snapshot for the repository. Update it whenever milestone state, implementation coverage, or the recommended next step materially changes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/status.md` at line 220, Update docs/status.md: the current "next roadmap
step" referencing M13 and runtime-extraction-roadmap.md is stale because this PR
added that contract; change the single-line text (around the M13 mention) to the
next actionable step (for example: "M13: validate contract implementation / run
contract validation tests and prepare M14 decision gate" or explicitly move to
"M14 decision gate") so the file reflects that the contract document exists and
lists the next work item (contract validation or M14 decision gate) as the
actionable next step.
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
1 participant
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.
Summary
Test plan
Closes #125
Refs #120
Refs #126