Repository navigation
feat(core): stage a streamed inventory without handling a journal key (#445) - #1012
Conversation
…#445) Staging still took the journal key, the committed manifest and the root binding from its caller, and the commit took a committed manifest and a journal directory that it compared with the staged one by spelling. begin_streamed_capture reads the binding from the committed root, routes the journal key through the key-epoch registry, loads the committed manifest by the exact root-named path under that key and starts the stage-one capture over it. The returned StagingSession stages page by page under that key and one journal directory value, so the caller never handles a key and never spells the directory twice. commit_streamed_inventory_capture loses its committed-manifest and journal-directory inputs: it promotes into the directory the capture was staged in and loads the committed manifest from the root under the routed key while the lock is held.
🤖 CodeAnt AI — Review Status
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Reviewer's GuideThe PR moves streamed inventory staging behind an authority-owned Sequence diagram for streamed inventory staging and commitsequenceDiagram
participant Caller
participant Authority
participant Root as CommittedRoot
participant Registry as KeyEpochRegistry
participant Journal
participant Lock as RootLock
Caller->>Authority: begin_streamed_capture(fs, provider, layout, begin)
Authority->>Root: load_catalog()
Root-->>Authority: live_migration binding
Authority->>Registry: resolve_journal_key(journal)
Registry-->>Authority: routed key
Authority->>Journal: load_authoritative_manifest(root-named path)
Journal-->>Authority: committed manifest
Authority->>Journal: StreamedCapture::begin(token, total)
Authority-->>Caller: StagingSession(key, journal_dir)
loop Each page
Caller->>Authority: StagingSession::push_page(entries)
Authority->>Journal: StreamedCapture::push_page(entries)
Journal-->>Authority: updated staging capture
Authority-->>Caller: StagingSession
end
Caller->>Authority: StagingSession::finish()
Authority->>Journal: StreamedCapture::finish()
Journal-->>Caller: StagedCapture
Caller->>Authority: commit_streamed_inventory_capture(capture)
Authority->>Lock: acquire root lock
Authority->>Registry: resolve_journal_key(root binding)
Registry-->>Authority: routed key
Authority->>Root: load_authoritative_manifest(root-named path)
Root-->>Authority: committed manifest
Authority->>Journal: promote_staged_inventory_fenced(staged directory)
Authority->>Root: publish successor and advance binding
Authority->>Lock: release root lock
Authority-->>Caller: committed manifest
Flow diagram for streamed capture refusal and commit safetyflowchart TD
A[begin_streamed_capture] --> B{Bound live migration?}
B -- No --> X[NoLiveMigration]
B -- Yes --> C[resolve_journal_key]
C --> D{Journal route valid?}
D -- No --> Y[JournalRoute error]
D -- Yes --> E[load_authoritative_manifest]
E --> F[StreamedCapture::begin]
F --> G[StagingSession stages pages lock-free]
G --> H[finish -> StagedCapture]
H --> I[commit_streamed_inventory_capture]
I --> J[Root lock]
J --> K[resolve_journal_key and load_authoritative_manifest]
K --> L{Owner and revision still valid?}
L -- No --> M[Refuse before promotion; keep staged files]
L -- Yes --> N[promote_staged_inventory_fenced]
N --> O[Publish successor and advance root binding]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
codeant-ai
Bot
added
the
size:L
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
[check-pr-size] PR size is over the target tier (normal profile): 10 files, 520 meaningful lines, 3 commits — limit ≤8 files / ≤400 lines / ≤6 commits. Consider splitting into smaller, independently reviewable PRs. |
|
Warning Review limit reachedEnable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Next included review available in 6 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 67 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe change adds an authority-managed staging session for streamed inventory capture. It routes the journal key and reads binding and manifest data from the root. Commit now uses the staged capture’s directory and loads the committed manifest from the root. ChangesRoot-routed streamed capture
Priority: ⬇️ Low Merge Risk: 🔵 Low · up to Clarify when staged files are removed so callers can implement failure cleanup and commit retries consistently. This bounded documentation issue does not block merging.
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: qnbs/WorldScript-Studio/.coderabbit.yaml
- Review profile: CHILL
- Plan: Essentials
- Run ID:
13816b07-02bc-4fac-93f4-3986f63e3ff1
📒 Files selected for processing (8)
CHANGELOG.mdcrates/worldscript-secure-storage/src/authority.rscrates/worldscript-secure-storage/src/lib.rscrates/worldscript-secure-storage/tests/gate4d_journal_route_test.rscrates/worldscript-secure-storage/tests/gate4d_root_binding_test.rsdocs/native/R15-SECURE-STORAGE-CONTRACT.mddocs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.mddocs/native/r15/GATE4D-SLICE-B-GAP-MATRIX.md
Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.
CodeAnt PR Risk: Low Risk
Assessed commit: |
…tention of a finished one (#445) The contract paragraph on streamed captures said both that a handled failure removes the pages staged so far and that staged files are removed only after the root has committed. The first applies while a capture is being staged, the second to a finished capture through its promotion and commit; the paragraph now says which is which.
|
@codex review |
|
@CodeAnt-AI review |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6f6894b39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…without a key (#445) begin_streamed_capture read the whole catalog (load_catalog) only to take the live-migration binding out of the root, which for a catalog of a million records materialises it before the first page of a memory-bounded capture. It now reads the committed root alone. StagedCapture::discard required a journal context and so a journal key, which a caller that staged through a session never holds; it only ever needed the file system, so it takes that.
|
@codex review |
|
@CodeAnt-AI review |
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Passed
See analysis details in CodeScene
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| gate4d_root_binding_test.rs | 6.81 → 7.02 | Code Duplication |
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
CodeAnt Nitpicks1 code suggestion1. A replacement can land after the digest read but before
|
Final disposition census — head
|
| Finding | Disposition |
|---|---|
| CodeRabbit (Minor), contract: the paragraph said both that a handled failure removes the staged pages and that staged files are removed only after the root commit | VALID_AND_FIXED in d6f6894b — it now separates the cleanup of an incomplete capture (while staging) from the retention of a finished one (through promotion and commit, unless discarded); documentation only |
Codex P2: begin_streamed_capture loaded the whole catalog only to take the live-migration binding from the root, which contradicts a memory-bounded capture |
VALID_AND_FIXED in 56c89a81 — it reads the committed root alone; observable: a catalog page that no longer verifies makes load_catalog fail (control) while the session still starts; mutation-checked |
Codex P2: StagedCapture::discard required a journal context and so a key, which a session caller never holds |
VALID_AND_FIXED in 56c89a81 — it takes the file system only; tested with a session-staged capture and StdFs alone |
CodeAnt nitpick: a file can be replaced between the digest read and the removal in discard |
DUPLICATE of the INVALID_WITH_EVIDENCE classification on #1010 (no unlink by handle in DurableFs; the directory is private, randomly named and never authority; stated as a limit in the evidence) |
Decisions flagged on the admission and not changed by review: the session reads the manifest and binding itself (beyond the QNB-11 text); StreamedInventoryCapture changes shape; the session holds the routed key for its lifetime; staging stays lock-free.
Proof: the stage-two (a) cases pass unchanged in substance against the smaller input and now stage through a session; a session is refused with a stale token and with no bound migration; a commit after the journal moved on is refused before any write with the staged files kept; the route refuses the session with a revoked or unregistered epoch and a route to another key; a session starts with an unreadable catalog page; a finished capture is discarded with no key. Mutation-checked for the key route, the binding requirement, the committed revision and the root-only start. The whole crate suite (50 passing results), clippy -D warnings, fmt --check, docs:check.
Scope: the key-routed staging session and the smaller commit input only. No C2, reclamation, inheriting pages or graphify. PRODUCTION_AUTHORITY_SWITCH_ALLOWED = NO.
1 participant
User description
What
Stage two (b) of the streaming capture: the staging stage no longer takes anything from its caller that the root already names. After #1011,
StreamedCapture::beginandpush_pagestill took the journal key (through the context), the committed manifest and the root binding from the caller, and the commit took a committed manifest and a journal directory that it compared with the staged one by spelling (a finding deferred on #1011).begin_streamed_captureNoLiveMigrationif none), routes the journal key through the key-epoch registry (JournalRouteerrors), loads the committed manifest by the exact root-named path under that key, and starts the stage-one capture over it (so the token, the open inventory and the announced total are checked as before). The caller supplies the journal source (directory and write operation), the owner's token and the announced total. Nothing is created and no lock is takenStagingSession::push_page/finishcommit_streamed_inventory_captureDecisions to review (mine, flagged on the admission): the session reads the manifest and the binding itself, which goes beyond the QNB-11 text (it names only the key);
StreamedInventoryCapturechanges shape (S2a had no caller); the session holds the routed key for its lifetime and the commit routes again, so a key that went stale in between is refused at promotion; staging stays lock-free.Boundary matrix
Open(Tampered)), a stale token, a journal that moved on since the capture began (StaleMigrationOwnerbefore any write, staged files kept)Not in this PR
The C2 driver, reclaiming abandoned pending directories, inheriting unchanged pages, the write barrier. No production authority switch.
Verification
Local: the stage-two cases of #1011 run unchanged in substance against the smaller input, and now stage through a session with no key, manifest or binding in the caller's hands (the happy path still equals the in-memory capture of the same pages and verifies); a session is refused before anything is created with a stale token and with no bound migration; a commit after the owner moved the journal on is refused before any write with the staged files kept; the route refuses the session with a revoked or unregistered epoch and with a route to another key, beside the other journal-owner operations. Mutation-checked: a fixed journal key in the session, no binding requirement, and a wrong committed revision in the commit each fail the test that owns them. The whole crate suite (50 passing results),
cargo clippy --all-targets -D warnings,cargo fmt --check,pnpm run docs:check. Size: 8 files, about 415 meaningful lines, 1 commit.Part of #359 and #445 (Linear QNB-11). Admission: #359 issuecomment-6069756829.
PRODUCTION_AUTHORITY_SWITCH_ALLOWED = NO.Summary by Sourcery
Route streamed inventory staging from authenticated root state and remove redundant journal authority inputs from the capture API.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores:
Summary by cubic
Staging a streamed inventory no longer takes the journal key, the committed manifest, or the root binding from its caller, and the commit no longer takes a committed manifest or a journal directory.
begin_streamed_capturereads the live migration binding from the committed root alone (catalog pages are not read, so a capture of a million-record inventory does not first materialise it), routes the journal key through the key-epoch registry, loads the committed manifest from the exact root-named path, and returns aStagingSessionthat stages page by page under a held key and journal directory.commit_streamed_inventory_capturepromotes into the directory the capture was staged in and loads the committed manifest from the root under the routed key while the root lock is held. Discarding a finished capture now needs only the file system, since a session caller never held a key. Docs now separate the cleanup of an incomplete capture (a handled failure removes pages staged so far) from the retention of a finished one (a failed promotion or commit keeps the staged files until the root commits).Behavior changes
Decisions to review
StreamedInventoryCapturechanges shape; stage two (a) had no caller.Written for commit 56c89a8. Summary will update on new commits.
Summary by CodeRabbit
CodeAnt-AI Description
Route streamed inventory capture from the committed root
What Changed
Impact
✅ No journal key needed to stage inventory✅ Catalog pages stay unloaded during capture startup✅ Staged pages remain available for commit retries💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.