Unify checkpoints with retained materializations - #780
Conversation
📝 WalkthroughWalkthroughThis change replaces new legacy checkpoint index-tree publication with exact retained Git-CAS materializations, adds bundle-backed index reads, stores replay and provenance roots, updates schema migration to v19 materializations, and preserves legacy checkpoint loading through compatibility paths. ChangesCheckpoint materialization lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/migrations/v17.0.0/checkpoint-schema-upgrade.ts (1)
152-172: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRedundant full-state hash computation during migration.
computeStateHash(payload.state, ...)is computed here to build the retained materialization, butcreateCheckpointEnvelope(viacheckpointCreate.ts) recomputes the same hash internally since nostateHashServiceis passed through. For large migrated states this hashes the full state twice per checkpoint.♻️ Proposed fix: pass the already-computed hash through `stateHashService`
const upgradedCheckpointSha = await createCheckpointEnvelope({ checkpointStore, graphName: options.graphName, state: payload.state, frontier: payload.frontier, parents: (await options.persistence.getNodeInfo(previousCheckpointSha)).parents, expectedCheckpointSha: previousCheckpointSha, codec, crypto, materialization, + stateHashService: { compute: async () => stateHash }, ...(payload.provenanceIndex === undefined ? {} : { provenanceIndex: payload.provenanceIndex }), });🤖 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 `@scripts/migrations/v17.0.0/checkpoint-schema-upgrade.ts` around lines 152 - 172, Update the migration flow around retainMigratedCheckpoint and createCheckpointEnvelope to pass the already-computed stateHash through the stateHashService option, ensuring checkpointCreate.ts reuses it instead of recomputing the payload.state hash. Preserve the existing retained-materialization behavior and hash value.src/infrastructure/adapters/GitCasMaterializationWorkspace.ts (1)
161-181: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftDerive workspace root members from canonical root metadata.
These stages write retained roots as
roots/<name>, whileMaterializationRootsand the bundle decoder both derive members from the domain-ownedMATERIALIZATION_ROOT_NAMES. The current list can miss a newly retained root or retain a stale literal again.🤖 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 `@src/infrastructure/adapters/GitCasMaterializationWorkspace.ts` around lines 161 - 181, Update workspaceMembers and appendWorkspaceRoot to derive root paths from the canonical MATERIALIZATION_ROOT_NAMES metadata instead of hardcoding individual root names. Map each canonical root name to the corresponding MaterializationWorkspaceRoots value, preserving requireRoots validation and parseRoot handling while ensuring newly added or removed retained roots stay synchronized.
🤖 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 `@src/domain/services/controllers/CheckpointController.ts`:
- Around line 217-238: Update the _indexStore workspace flow in
CheckpointController so a successful workspace.promote call is followed by
workspace.release before returning. Preserve the existing
releaseWorkspaceAfterFailure cleanup in the catch path and return the promotion
result after successful cleanup.
In `@src/infrastructure/adapters/CborCheckpointStoreAdapter.ts`:
- Around line 300-325: Update _readBundleLayout to use the shared
materializationMismatch(...) helper for both state-hash and live-coordinate
validation failures, removing its duplicated PersistenceError construction.
Relocate or export the helper from the shared publication module so the publish
and load paths use the same error construction and messages.
In `@src/infrastructure/adapters/CheckpointMaterializationPublication.ts`:
- Around line 16-25: Update the validation in
CheckpointMaterializationPublication to compare
materialization.coordinate.frontier() with record.frontier as order-independent
maps, and throw materializationMismatch when they differ. Preserve the existing
stateHash and live-coordinate ceiling checks.
In `@src/infrastructure/adapters/GitCasMaterializationProvenanceSupport.ts`:
- Around line 63-78: Update the MaterializationRoots domain object to expose a
withRoot(name, root) method that returns a copy with only the named root
replaced while preserving all other roots. Refactor replaceProvenanceSupportRoot
and the analogous replaceReplayBasisRoot implementation to delegate to this
method instead of rebuilding MaterializationRoots with every field.
In `@src/infrastructure/adapters/GitCasMaterializationSnapshotReader.ts`:
- Line 26: Extract the shared 1024 * 1024 descriptor size bound into a common
exported constant, then update GitCasMaterializationStoreAdapter,
GitCasMaterializationSnapshotReader, and
GitCasMaterializationCacheDiagnosticsAdapter to import and use it for their
descriptor maxBytes values. Remove the duplicated local definitions while
preserving the existing limit.
In `@test/helpers/MemoryRuntimeStorageAdapter.ts`:
- Around line 153-178: Extract the repeated candidate-normalization logic from
commitNodeWithTree, writeBlob, and writeTree into a shared helper that accepts
the history result and corresponding fallback operation, returning the candidate
when it is a Git object ID and otherwise invoking the fallback. Update all three
wrappers to use this helper while preserving commit publication behavior.
In `@test/unit/domain/services/controllers/CheckpointController.test.ts`:
- Around line 247-277: Extend the checkpoint test around createCheckpoint to
initialize host['_indexStore'] and mock the workspace lifecycle used by
_retainCheckpointMaterialization, including openWorkspace,
prepareMaterializationIndexRoots, promote, and release. Assert the index-store
branch is selected, the retained materialization is published, and
workspace.release() is called; keep the existing no-rebuild assertions where
applicable.
In `@test/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.ts`:
- Around line 263-290: Add a load-path test alongside the existing publish
mismatch cases that commits checkpoint metadata with a state-hash or
live-coordinate mismatch over an otherwise valid bundle, then loads it through
the checkpoint store and asserts rejection with code
E_CHECKPOINT_MATERIALIZATION_MISMATCH. Exercise the _readBundleLayout validation
rather than publishCheckpoint’s requireCheckpointMaterialization guard.
- Around line 219-230: Update the test around record() and loadBasis() so
record() exposes the logical and property root handles it writes, then assert
basis.indexRoot and basis.propertyRoot equal those exact handle strings rather
than only checking non-null values. Preserve the existing empty
indexShardHandles and frontier assertions.
In `@test/unit/scripts/checkpoint-schema-upgrade.test.ts`:
- Around line 401-437: The unavailable-roots materialization setup is duplicated
across the tests. In test/unit/scripts/checkpoint-schema-upgrade.test.ts lines
401-437, extract the MaterializationCoordinate, all-unavailable
MaterializationRoots, and materializationStore.retain logic from
createCurrentCheckpoint into a shared test/helpers factory accepting the store,
frontier, state, crypto, and codec; in
test/unit/scripts/v16-to-v17-upgrade.test.ts lines 82-109, replace the inline
setup with that helper and preserve the existing checkpoint behavior.
---
Outside diff comments:
In `@scripts/migrations/v17.0.0/checkpoint-schema-upgrade.ts`:
- Around line 152-172: Update the migration flow around retainMigratedCheckpoint
and createCheckpointEnvelope to pass the already-computed stateHash through the
stateHashService option, ensuring checkpointCreate.ts reuses it instead of
recomputing the payload.state hash. Preserve the existing
retained-materialization behavior and hash value.
In `@src/infrastructure/adapters/GitCasMaterializationWorkspace.ts`:
- Around line 161-181: Update workspaceMembers and appendWorkspaceRoot to derive
root paths from the canonical MATERIALIZATION_ROOT_NAMES metadata instead of
hardcoding individual root names. Map each canonical root name to the
corresponding MaterializationWorkspaceRoots value, preserving requireRoots
validation and parseRoot handling while ensuring newly added or removed retained
roots stay synchronized.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a0911340-dff4-46f7-9240-3f9d57a186a6
📒 Files selected for processing (53)
CHANGELOG.mdscripts/migrations/v17.0.0/CheckpointMaterializationMigration.tsscripts/migrations/v17.0.0/checkpoint-schema-upgrade.tsscripts/migrations/v17.0.0/openCheckpointMigrationStore.tssrc/domain/RuntimeHost.tssrc/domain/materialization/MaterializationIndexProfile.tssrc/domain/materialization/UnavailableMaterializationRoots.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/MaterializationIndexRoots.tssrc/domain/services/controllers/MaterializationRetention.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/domain/services/index/LogicalIndexReader.tssrc/domain/services/index/PropertyIndexReader.tssrc/domain/services/optic/CheckpointShardFactReader.tssrc/domain/services/optic/CheckpointTailBasisLoader.tssrc/domain/services/optic/CheckpointTailBasisVerifier.tssrc/domain/services/state/checkpointCreate.tssrc/domain/services/state/checkpointLoad.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.tssrc/infrastructure/adapters/CborIndexStoreAdapter.tssrc/infrastructure/adapters/CheckpointMaterializationPublication.tssrc/infrastructure/adapters/GitCasMaterializationProvenanceSupport.tssrc/infrastructure/adapters/GitCasMaterializationReplayBasis.tssrc/infrastructure/adapters/GitCasMaterializationSnapshotReader.tssrc/infrastructure/adapters/GitCasMaterializationStoreAdapter.tssrc/infrastructure/adapters/GitCasMaterializationStoreValidation.tssrc/infrastructure/adapters/GitCasMaterializationWorkspace.tssrc/infrastructure/adapters/GitCasRepositoryAdapter.tssrc/ports/CheckpointStorePort.tssrc/ports/IndexStorePort.tssrc/ports/MaterializationWorkspacePort.tstest/helpers/InMemoryCheckpointStore.tstest/helpers/MemoryRuntimeStorageAdapter.tstest/helpers/MockIndexStorage.tstest/helpers/WarpGraphMockPersistence.tstest/unit/domain/RuntimeHost.snapshotCache.test.tstest/unit/domain/WarpGraph.checkpoint.test.tstest/unit/domain/materialization/MaterializationIndexProfile.test.tstest/unit/domain/services/controllers/CheckpointController.test.tstest/unit/domain/services/controllers/MaterializeController.propertyRootRetention.test.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tstest/unit/domain/services/controllers/MaterializeController.test.tstest/unit/domain/services/optic/CheckpointShardFactReader.manifest.test.tstest/unit/domain/services/optic/CheckpointTailBasisVerifier.test.tstest/unit/domain/services/optic/CheckpointTailReadIdentityBuilder.test.tstest/unit/domain/warp/hydrateCheckpointIndex.regression.test.tstest/unit/infrastructure/CborIndexStoreAdapter.test.tstest/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.tstest/unit/ports/CheckpointStorePort.test.tstest/unit/ports/IndexStorePort.test.tstest/unit/scripts/checkpoint-schema-upgrade.test.tstest/unit/scripts/v16-to-v17-upgrade.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: test-node (22)
- GitHub Check: coverage-threshold
- GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,js,jsx}: Do not introduceany,as any,as unknown as,unknown(outside adapters),Record<string, unknown>(outside adapters),*Likeplaceholder types,JSON.parse/JSON.stringify(outside adapters),fetch(outside adapters),process.env(outside adapters),@ts-ignore, orz.any()in core code; use validated boundary models and ports instead.
Use constructor-injected ports for external capabilities; do not rely on ambient dependencies for I/O, clocks, persistence, or entropy.
Do not createutils.ts,helpers.ts,misc.ts, orcommon.ts; name files after the actual concept they model.
Prefer one file per class, type, or object; if a file accumulates peer concepts, split it.
Keep helper corridors, fake shape trust, transitional duplication, and compile-time theater out of the codebase; runtime-honest TypeScript must reflect actual behavior.
No enum usage; prefer runtime-backed domain forms and unions.
Do not use boolean trap parameters; prefer named option objects or separate methods.
Avoid magic strings or numbers when a named constant should exist.
Keep domain bytes asUint8Array;Bufferbelongs in infrastructure adapters.
Files:
src/infrastructure/adapters/GitCasRepositoryAdapter.tssrc/domain/materialization/UnavailableMaterializationRoots.tssrc/domain/materialization/MaterializationIndexProfile.tssrc/infrastructure/adapters/GitCasMaterializationWorkspace.tssrc/domain/services/index/LogicalIndexReader.tssrc/ports/MaterializationWorkspacePort.tssrc/domain/services/state/checkpointCreate.tssrc/infrastructure/adapters/CheckpointMaterializationPublication.tssrc/infrastructure/adapters/GitCasMaterializationReplayBasis.tssrc/domain/services/state/checkpointLoad.tssrc/domain/services/controllers/MaterializationIndexRoots.tssrc/ports/IndexStorePort.tssrc/domain/services/controllers/MaterializeController.tssrc/infrastructure/adapters/GitCasMaterializationProvenanceSupport.tssrc/domain/RuntimeHost.tssrc/infrastructure/adapters/GitCasMaterializationSnapshotReader.tssrc/domain/services/optic/CheckpointTailBasisVerifier.tssrc/infrastructure/adapters/GitCasMaterializationStoreValidation.tssrc/ports/CheckpointStorePort.tssrc/domain/services/controllers/MaterializationRetention.tssrc/domain/services/index/PropertyIndexReader.tssrc/domain/services/optic/CheckpointTailBasisLoader.tssrc/domain/services/optic/CheckpointShardFactReader.tssrc/infrastructure/adapters/GitCasMaterializationStoreAdapter.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/infrastructure/adapters/CborIndexStoreAdapter.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not use direct imports fromsrc/infrastructure/**insrc/domain/**orsrc/ports/**; depend on a port instead.
Do not use direct Node built-ins insrc/domain/**orsrc/ports/**; use a port instead.
Files:
src/infrastructure/adapters/GitCasRepositoryAdapter.tssrc/domain/materialization/UnavailableMaterializationRoots.tstest/unit/domain/services/optic/CheckpointTailReadIdentityBuilder.test.tsscripts/migrations/v17.0.0/CheckpointMaterializationMigration.tstest/unit/domain/materialization/MaterializationIndexProfile.test.tssrc/domain/materialization/MaterializationIndexProfile.tssrc/infrastructure/adapters/GitCasMaterializationWorkspace.tstest/unit/domain/warp/hydrateCheckpointIndex.regression.test.tstest/helpers/WarpGraphMockPersistence.tstest/unit/domain/RuntimeHost.snapshotCache.test.tssrc/domain/services/index/LogicalIndexReader.tssrc/ports/MaterializationWorkspacePort.tssrc/domain/services/state/checkpointCreate.tssrc/infrastructure/adapters/CheckpointMaterializationPublication.tstest/helpers/MockIndexStorage.tstest/unit/ports/IndexStorePort.test.tstest/helpers/InMemoryCheckpointStore.tstest/unit/domain/services/controllers/MaterializeController.propertyRootRetention.test.tsscripts/migrations/v17.0.0/openCheckpointMigrationStore.tstest/unit/scripts/v16-to-v17-upgrade.test.tstest/unit/domain/services/controllers/MaterializeController.test.tssrc/infrastructure/adapters/GitCasMaterializationReplayBasis.tssrc/domain/services/state/checkpointLoad.tstest/unit/domain/WarpGraph.checkpoint.test.tssrc/domain/services/controllers/MaterializationIndexRoots.tssrc/ports/IndexStorePort.tssrc/domain/services/controllers/MaterializeController.tssrc/infrastructure/adapters/GitCasMaterializationProvenanceSupport.tstest/unit/domain/services/optic/CheckpointTailBasisVerifier.test.tssrc/domain/RuntimeHost.tssrc/infrastructure/adapters/GitCasMaterializationSnapshotReader.tssrc/domain/services/optic/CheckpointTailBasisVerifier.tstest/unit/domain/services/controllers/CheckpointController.test.tssrc/infrastructure/adapters/GitCasMaterializationStoreValidation.tssrc/ports/CheckpointStorePort.tstest/unit/ports/CheckpointStorePort.test.tssrc/domain/services/controllers/MaterializationRetention.tstest/unit/domain/services/optic/CheckpointShardFactReader.manifest.test.tstest/helpers/MemoryRuntimeStorageAdapter.tstest/unit/infrastructure/CborIndexStoreAdapter.test.tssrc/domain/services/index/PropertyIndexReader.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tssrc/domain/services/optic/CheckpointTailBasisLoader.tsscripts/migrations/v17.0.0/checkpoint-schema-upgrade.tstest/unit/scripts/checkpoint-schema-upgrade.test.tssrc/domain/services/optic/CheckpointShardFactReader.tssrc/infrastructure/adapters/GitCasMaterializationStoreAdapter.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/infrastructure/adapters/CborIndexStoreAdapter.tstest/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.ts
src/domain/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/domain/**/*.{ts,tsx,js,jsx}: Insrc/domain/**, do not useDate.now(),new Date(),Date(),performance.now(),Math.random(),crypto.randomUUID(),crypto.getRandomValues(),setTimeout,setInterval, rawnew Error(...)/new TypeError(...), or direct imports from Node built-ins; time, entropy, and external capabilities must enter through ports or parameters, and domain errors should extendWarpError.
Construct domain objects only in core when doing so establishes validated runtime truth; do not build infrastructure adapters, host APIs, persistence implementations, wall clocks, or entropy sources inside core.
Prefer discriminated unions and explicit result types instead of boolean-flag bags, and model expected failures as return values rather than exceptions.
src/domain/must not import host APIs or Node-specific globals; hexagonal architecture boundaries are mandatory.
Domain code must not use the wall clock directly; time must enter through a port or parameter.
Files:
src/domain/materialization/UnavailableMaterializationRoots.tssrc/domain/materialization/MaterializationIndexProfile.tssrc/domain/services/index/LogicalIndexReader.tssrc/domain/services/state/checkpointCreate.tssrc/domain/services/state/checkpointLoad.tssrc/domain/services/controllers/MaterializationIndexRoots.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/RuntimeHost.tssrc/domain/services/optic/CheckpointTailBasisVerifier.tssrc/domain/services/controllers/MaterializationRetention.tssrc/domain/services/index/PropertyIndexReader.tssrc/domain/services/optic/CheckpointTailBasisLoader.tssrc/domain/services/optic/CheckpointShardFactReader.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/MaterializeSessionBridge.ts
src/domain/**/!(*.test).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit domain concepts with validated constructors,
Object.freeze, andinstanceofdispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.
Files:
src/domain/materialization/UnavailableMaterializationRoots.tssrc/domain/materialization/MaterializationIndexProfile.tssrc/domain/services/index/LogicalIndexReader.tssrc/domain/services/state/checkpointCreate.tssrc/domain/services/state/checkpointLoad.tssrc/domain/services/controllers/MaterializationIndexRoots.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/RuntimeHost.tssrc/domain/services/optic/CheckpointTailBasisVerifier.tssrc/domain/services/controllers/MaterializationRetention.tssrc/domain/services/index/PropertyIndexReader.tssrc/domain/services/optic/CheckpointTailBasisLoader.tssrc/domain/services/optic/CheckpointShardFactReader.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/MaterializeSessionBridge.ts
src/ports/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
In
src/ports/**, do not import Node built-ins orsrc/infrastructure/**; ports must stay abstract and depend only on boundary-safe types.
Files:
src/ports/MaterializationWorkspacePort.tssrc/ports/IndexStorePort.tssrc/ports/CheckpointStorePort.ts
🧠 Learnings (1)
📚 Learning: 2026-03-08T19:50:17.519Z
Learnt from: flyingrobots
Repo: git-stunts/git-warp PR: 65
File: CHANGELOG.md:88-88
Timestamp: 2026-03-08T19:50:17.519Z
Learning: Follow the Keep a Changelog convention for CHANGELOG.md. Allow duplicate subheadings across versions (e.g., '### Added', '### Fixed'). Configure markdownlint MD024 with {"siblings_only": true} to avoid cross-version false positives.
Applied to files:
CHANGELOG.md
🔇 Additional comments (67)
test/helpers/MockIndexStorage.ts (1)
10-10: LGTM!Also applies to: 49-62, 85-98
src/domain/materialization/MaterializationIndexProfile.ts (1)
1-36: LGTM!src/domain/materialization/UnavailableMaterializationRoots.ts (1)
1-18: LGTM!src/ports/CheckpointStorePort.ts (1)
7-7: LGTM!Also applies to: 18-18, 39-40, 50-51
src/ports/IndexStorePort.ts (1)
44-47: LGTM!Also applies to: 106-109, 120-125
src/ports/MaterializationWorkspacePort.ts (1)
6-14: LGTM!Also applies to: 16-21
src/domain/services/state/checkpointCreate.ts (1)
13-13: LGTM!Also applies to: 24-24, 49-57, 77-77
src/domain/services/state/checkpointLoad.ts (1)
22-22: LGTM!Also applies to: 35-36, 65-66
src/infrastructure/adapters/CheckpointMaterializationPublication.ts (1)
1-15: LGTM!Also applies to: 29-53, 55-57
test/helpers/InMemoryCheckpointStore.ts (1)
48-49: LGTM!Also applies to: 124-125
test/unit/ports/CheckpointStorePort.test.ts (1)
31-34: LGTM!Also applies to: 64-67
test/unit/ports/IndexStorePort.test.ts (1)
17-20: LGTM!Also applies to: 59-75
test/unit/domain/materialization/MaterializationIndexProfile.test.ts (1)
1-22: LGTM!CHANGELOG.md (1)
44-48: LGTM!Also applies to: 113-116
scripts/migrations/v17.0.0/CheckpointMaterializationMigration.ts (1)
9-28: LGTM!scripts/migrations/v17.0.0/checkpoint-schema-upgrade.ts (1)
30-30: LGTM!Also applies to: 49-49, 191-198
scripts/migrations/v17.0.0/openCheckpointMigrationStore.ts (1)
6-31: LGTM!test/unit/domain/WarpGraph.checkpoint.test.ts (1)
366-401: LGTM!test/unit/domain/RuntimeHost.snapshotCache.test.ts (1)
65-93: LGTM!test/unit/infrastructure/CborIndexStoreAdapter.test.ts (1)
194-210: LGTM!Also applies to: 268-279
src/infrastructure/adapters/CborCheckpointStoreAdapter.ts (6)
1-6: LGTM!Also applies to: 43-50, 62-83
96-127: LGTM!
129-158: LGTM!
164-197: LGTM!
218-244: LGTM!
419-421: LGTM!src/infrastructure/adapters/GitCasMaterializationProvenanceSupport.ts (2)
24-61: LGTM!
80-85: LGTM!src/infrastructure/adapters/GitCasMaterializationReplayBasis.ts (1)
77-90: LGTM!src/infrastructure/adapters/GitCasMaterializationSnapshotReader.ts (2)
63-91: LGTM!
94-113: LGTM!src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts (3)
69-118: LGTM!
191-207: LGTM!
464-476: LGTM!src/infrastructure/adapters/GitCasMaterializationStoreValidation.ts (1)
11-26: LGTM!src/infrastructure/adapters/GitCasRepositoryAdapter.ts (1)
192-192: LGTM!test/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.ts (3)
2-92: LGTM!
94-159: LGTM!
365-377: LGTM!src/infrastructure/adapters/CborIndexStoreAdapter.ts (1)
8-8: LGTM!Also applies to: 194-212, 231-243, 282-326, 427-439
test/helpers/WarpGraphMockPersistence.ts (1)
11-11: LGTM!Also applies to: 40-49
src/domain/services/index/LogicalIndexReader.ts (1)
22-22: LGTM!Also applies to: 109-138
src/domain/services/index/PropertyIndexReader.ts (2)
280-311: LGTM!Also applies to: 340-353
355-372: 🎯 Functional CorrectnessNo change needed.
decodePropertyShardalready defaults its key resolver tocomputeShardKey, so the root path can passmaterializationPropertyShardKeywithout a signature mismatch.src/domain/services/optic/CheckpointShardFactReader.ts (1)
16-19: LGTM!Also applies to: 42-45, 63-70, 86-97, 115-120, 145-157, 183-236, 254-268, 277-292
src/domain/services/optic/CheckpointTailBasisLoader.ts (1)
3-5: LGTM!Also applies to: 28-31, 56-65, 78-128, 233-249
src/domain/services/optic/CheckpointTailBasisVerifier.ts (1)
43-43: LGTM!Also applies to: 60-62
test/unit/domain/services/optic/CheckpointShardFactReader.manifest.test.ts (1)
5-5: LGTM!Also applies to: 23-29, 43-88, 413-416
test/unit/domain/services/optic/CheckpointTailBasisVerifier.test.ts (1)
10-10: LGTM!Also applies to: 137-148, 171-172
test/unit/domain/services/optic/CheckpointTailReadIdentityBuilder.test.ts (1)
67-70: LGTM!src/domain/services/controllers/CheckpointController.ts (3)
195-216: Reuse-check for cached retained materialization looks correct.Reusing
h._retainedMaterializationonly whenstateHash/coordinatematch and both bounded roots are non-unavailable(permittingempty) matches howresolvePropertyRoot/resolveIndexRoottreat "existing" roots elsewhere. No issue here.
158-172: LGTM!
38-51: LGTM!Also applies to: 64-65, 83-84, 308-309
src/domain/RuntimeHost.ts (1)
72-72: LGTM!Also applies to: 185-188, 267-267, 441-441, 565-565, 911-911
src/domain/services/controllers/MaterializationIndexRoots.ts (2)
29-50: LGTM!Also applies to: 150-160
70-95: 🎯 Functional CorrectnessReturn an empty materialization root when there are no retained index shards.
LogicalIndexBuildServicealways emits at leastMetaShard(s),LabelShard, edge shards, andReceiptShard, so zero shard count is not reachable from the currentbuildShardspath.> Likely an incorrect or invalid review comment.src/domain/services/controllers/MaterializationRetention.ts (2)
71-85: 🚀 Performance & ScalabilityConfirm the write-amplification impact of always retaining materializations now.
Previously, a live materialize whose reduction produced no
roots(non-session-backed reducer) skipped retention entirely. Now the only skip condition isfrontier === null; with a non-null frontier and no session roots,rootsfalls back tounavailableMaterializationRoots()andmaterializations.retain(request)still runs. That means every live materialize call on this path now performs a CAS write purely to recordstateHash/replayBasis, where before it did none. This looks intentional per the PR's stated goal of unifying checkpoint/live materialization, but please confirm this additional per-materialize I/O cost was accounted for (e.g., in the referenced full-suite/perf validation), since it affects the hot materialize path rather than just checkpoint creation.
3-3: LGTM!src/domain/services/controllers/MaterializeController.ts (1)
317-323: LGTM!Also applies to: 415-418
src/domain/services/controllers/MaterializeSessionBridge.ts (1)
117-142: LGTM!Also applies to: 201-240
test/unit/domain/services/controllers/CheckpointController.test.ts (1)
318-332: LGTM!test/unit/domain/services/controllers/MaterializeController.propertyRootRetention.test.ts (1)
113-113: LGTM!Also applies to: 132-138
test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts (1)
550-571: LGTM!Also applies to: 796-797
test/unit/domain/services/controllers/MaterializeController.test.ts (1)
9-9: LGTM!Also applies to: 166-166
test/unit/domain/warp/hydrateCheckpointIndex.regression.test.ts (1)
9-9: LGTM!Also applies to: 58-58
test/helpers/MemoryRuntimeStorageAdapter.ts (2)
76-97: LGTM!
144-197: 🩺 Stability & AvailabilityNo action needed.
The fallback helper is not exercised by the test path using it here, and there’s no path reading back content written through the disconnected
fallbackObjectsinstance.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/helpers/MemoryRuntimeStorageAdapter.ts`:
- Around line 169-174: Update writeTree and the associated object-type tracking
in MemoryRuntimeStorageAdapter so every normalized tree OID, including
fallbackObjects.writeTree results, is recorded as type tree. Ensure
readObjectType consults this tracking before defaulting non-publication OIDs to
blob, preserving valid tree types for fixture CAS graphs.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ba6001e9-cf0d-434e-ba19-b7d6856467f5
📒 Files selected for processing (25)
src/domain/materialization/MaterializationRoots.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.tssrc/infrastructure/adapters/CheckpointMaterializationPublication.tssrc/infrastructure/adapters/GitCasMaterializationCacheDiagnosticsAdapter.tssrc/infrastructure/adapters/GitCasMaterializationDescriptor.tssrc/infrastructure/adapters/GitCasMaterializationProvenanceSupport.tssrc/infrastructure/adapters/GitCasMaterializationReplayBasis.tssrc/infrastructure/adapters/GitCasMaterializationSnapshotReader.tssrc/infrastructure/adapters/GitCasMaterializationStoreAdapter.tstest/helpers/MemoryRuntimeStorageAdapter.tstest/helpers/retainUnavailableMaterialization.tstest/integration/api/checkpoint.test.tstest/unit/domain/materialization/MaterializationIdentity.test.tstest/unit/domain/services/LogicalIndexReader.test.tstest/unit/domain/services/PropertyIndex.test.tstest/unit/domain/services/controllers/CheckpointController.test.tstest/unit/domain/services/controllers/MaterializeController.stateSession.fixtures.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tstest/unit/infrastructure/CborIndexStoreAdapter.test.tstest/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.tstest/unit/scripts/checkpoint-schema-upgrade.test.tstest/unit/scripts/v16-to-v17-upgrade.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: coverage-threshold
- GitHub Check: test-node (22)
- GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Do not use direct imports fromsrc/infrastructure/**insrc/domain/**orsrc/ports/**; depend on a port instead.
Do not use direct Node built-ins insrc/domain/**orsrc/ports/**; use a port instead.
Files:
test/helpers/retainUnavailableMaterialization.tstest/unit/domain/materialization/MaterializationIdentity.test.tssrc/infrastructure/adapters/GitCasMaterializationDescriptor.tstest/unit/domain/services/LogicalIndexReader.test.tstest/unit/domain/services/PropertyIndex.test.tssrc/domain/services/controllers/MaterializeController.tstest/unit/domain/services/controllers/MaterializeController.stateSession.fixtures.tstest/unit/scripts/v16-to-v17-upgrade.test.tssrc/infrastructure/adapters/GitCasMaterializationReplayBasis.tssrc/infrastructure/adapters/CheckpointMaterializationPublication.tstest/helpers/MemoryRuntimeStorageAdapter.tssrc/infrastructure/adapters/GitCasMaterializationCacheDiagnosticsAdapter.tssrc/infrastructure/adapters/GitCasMaterializationProvenanceSupport.tstest/unit/domain/services/controllers/CheckpointController.test.tssrc/infrastructure/adapters/GitCasMaterializationSnapshotReader.tssrc/domain/services/controllers/CheckpointController.tstest/unit/scripts/checkpoint-schema-upgrade.test.tssrc/domain/materialization/MaterializationRoots.tstest/unit/infrastructure/CborIndexStoreAdapter.test.tstest/integration/api/checkpoint.test.tssrc/infrastructure/adapters/GitCasMaterializationStoreAdapter.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.tstest/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.ts
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,js,jsx}: Do not introduceany,as any,as unknown as,unknown(outside adapters),Record<string, unknown>(outside adapters),*Likeplaceholder types,JSON.parse/JSON.stringify(outside adapters),fetch(outside adapters),process.env(outside adapters),@ts-ignore, orz.any()in core code; use validated boundary models and ports instead.
Use constructor-injected ports for external capabilities; do not rely on ambient dependencies for I/O, clocks, persistence, or entropy.
Do not createutils.ts,helpers.ts,misc.ts, orcommon.ts; name files after the actual concept they model.
Prefer one file per class, type, or object; if a file accumulates peer concepts, split it.
Keep helper corridors, fake shape trust, transitional duplication, and compile-time theater out of the codebase; runtime-honest TypeScript must reflect actual behavior.
No enum usage; prefer runtime-backed domain forms and unions.
Do not use boolean trap parameters; prefer named option objects or separate methods.
Avoid magic strings or numbers when a named constant should exist.
Keep domain bytes asUint8Array;Bufferbelongs in infrastructure adapters.
Files:
src/infrastructure/adapters/GitCasMaterializationDescriptor.tssrc/domain/services/controllers/MaterializeController.tssrc/infrastructure/adapters/GitCasMaterializationReplayBasis.tssrc/infrastructure/adapters/CheckpointMaterializationPublication.tssrc/infrastructure/adapters/GitCasMaterializationCacheDiagnosticsAdapter.tssrc/infrastructure/adapters/GitCasMaterializationProvenanceSupport.tssrc/infrastructure/adapters/GitCasMaterializationSnapshotReader.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/materialization/MaterializationRoots.tssrc/infrastructure/adapters/GitCasMaterializationStoreAdapter.tssrc/domain/services/controllers/MaterializeSessionBridge.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.ts
src/domain/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/domain/**/*.{ts,tsx,js,jsx}: Insrc/domain/**, do not useDate.now(),new Date(),Date(),performance.now(),Math.random(),crypto.randomUUID(),crypto.getRandomValues(),setTimeout,setInterval, rawnew Error(...)/new TypeError(...), or direct imports from Node built-ins; time, entropy, and external capabilities must enter through ports or parameters, and domain errors should extendWarpError.
Construct domain objects only in core when doing so establishes validated runtime truth; do not build infrastructure adapters, host APIs, persistence implementations, wall clocks, or entropy sources inside core.
Prefer discriminated unions and explicit result types instead of boolean-flag bags, and model expected failures as return values rather than exceptions.
src/domain/must not import host APIs or Node-specific globals; hexagonal architecture boundaries are mandatory.
Domain code must not use the wall clock directly; time must enter through a port or parameter.
Files:
src/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/materialization/MaterializationRoots.tssrc/domain/services/controllers/MaterializeSessionBridge.ts
src/domain/**/!(*.test).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit domain concepts with validated constructors,
Object.freeze, andinstanceofdispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.
Files:
src/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/materialization/MaterializationRoots.tssrc/domain/services/controllers/MaterializeSessionBridge.ts
🔇 Additional comments (34)
test/unit/domain/materialization/MaterializationIdentity.test.ts (1)
131-142: LGTM!Also applies to: 144-149
test/unit/domain/services/LogicalIndexReader.test.ts (1)
160-164: LGTM!Also applies to: 410-418, 420-425
test/unit/domain/services/PropertyIndex.test.ts (1)
11-11: LGTM!Also applies to: 102-109
test/unit/domain/services/controllers/CheckpointController.test.ts (1)
53-54: LGTM!Also applies to: 68-69, 112-116, 213-214, 258-326, 367-381
src/domain/materialization/MaterializationRoots.ts (1)
30-44: LGTM!Also applies to: 55-67, 86-103, 111-117
test/unit/infrastructure/CborIndexStoreAdapter.test.ts (2)
123-134: LGTM!
418-425: LGTM!test/unit/scripts/checkpoint-schema-upgrade.test.ts (1)
32-32: LGTM!Also applies to: 399-421
test/unit/scripts/v16-to-v17-upgrade.test.ts (1)
18-18: LGTM!Also applies to: 82-87
test/unit/infrastructure/adapters/CborCheckpointStoreAdapter.test.ts (6)
104-173: LGTM!
175-198: LGTM!
200-258: LGTM!
320-327: LGTM!
390-443: LGTM!
445-594: LGTM!src/domain/services/controllers/CheckpointController.ts (3)
217-241: LGTM!
298-324: LGTM!
195-216: 🗄️ Data Integrity & IntegrationNo change needed
h._retainedMaterializationis populated byRuntimeHost._onMaterialized(result)viaretainedMaterializationFrom(result)after materialization, and is cleared during initialization and set-materialized-state.src/domain/services/controllers/MaterializeController.ts (1)
318-327: LGTM!Also applies to: 415-422
src/domain/services/controllers/MaterializeSessionBridge.ts (1)
118-150: LGTM!Also applies to: 209-227
test/unit/domain/services/controllers/MaterializeController.stateSession.fixtures.ts (1)
1-142: LGTM!test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts (1)
11-26: LGTM!Also applies to: 404-427, 449-449, 459-467, 692-693
src/infrastructure/adapters/CborCheckpointStoreAdapter.ts (2)
123-159: LGTM!Also applies to: 301-324
64-69: 🗄️ Data Integrity & IntegrationNo change needed.
CborCheckpointStoreAdapterconstructors already supplycrypto, and the test/helper facade shape includesgetMemberReference,iterateMemberReferences, andputOrdered.src/infrastructure/adapters/CheckpointMaterializationPublication.ts (1)
6-32: LGTM!Also applies to: 60-70
src/infrastructure/adapters/GitCasMaterializationCacheDiagnosticsAdapter.ts (1)
26-42: LGTM!Also applies to: 164-169
test/helpers/MemoryRuntimeStorageAdapter.ts (1)
87-88: LGTM!Also applies to: 156-168, 195-200
src/infrastructure/adapters/GitCasMaterializationDescriptor.ts (1)
13-13: LGTM!src/infrastructure/adapters/GitCasMaterializationProvenanceSupport.ts (1)
8-8: LGTM!Also applies to: 63-68
src/infrastructure/adapters/GitCasMaterializationReplayBasis.ts (1)
9-9: LGTM!Also applies to: 77-88, 103-107
src/infrastructure/adapters/GitCasMaterializationSnapshotReader.ts (1)
16-16: LGTM!Also applies to: 62-90
src/infrastructure/adapters/GitCasMaterializationStoreAdapter.ts (1)
49-49: LGTM!Also applies to: 87-87, 98-118, 167-224, 449-452
test/integration/api/checkpoint.test.ts (1)
32-64: LGTM!Also applies to: 88-100, 111-124, 142-184
test/helpers/retainUnavailableMaterialization.ts (1)
1-29: LGTM!
Release Preflight
If this PR is from a |
Release Preflight
If this PR is from a |
1 participant
Summary
Why
Checkpoint creation still duplicated whole-state and index assembly after materialization. That increased CPU, memory residency, object writes, and the number of storage representations that had to remain mutually consistent. The v19 path now gives runtime reads, checkpoint recovery, provenance, and bounded logical/property reads one retained git-cas representation with explicit integrity checks.
User impact
Current checkpoints are cheaper to create and reopen, and their bounded-read roots are exact page-backed references rather than copied checkpoint assets. Old released Git-tree checkpoints remain readable. Very old schema-2/schema-3 checkpoint migration is intentionally no longer supported in v19.
Validation
Part of #739.