Fix warm state-cache materialization hits - #750
Conversation
📝 WalkthroughWalkthroughThe change updates state-cache materialization to track degraded provenance explicitly, preserve resident provenance for matching exact hits, compute observed Lamport values from state, and cover cache, checkpoint, replay, and warm-hit behavior with tests and documentation. ChangesProvenance-aware materialization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant RuntimeHost
participant MaterializeLiveStrategy
participant MaterializeSnapshotCacheResult
participant MaterializeController
RuntimeHost->>MaterializeLiveStrategy: request materialization
MaterializeLiveStrategy->>MaterializeSnapshotCacheResult: resolve exact snapshot
MaterializeSnapshotCacheResult->>MaterializeController: return degraded result and observed Lamport
MaterializeController->>RuntimeHost: publish materialized state
RuntimeHost->>RuntimeHost: select resident or materialized provenance
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Release Preflight
If this PR is from a |
Release Preflight
If this PR is from a |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/domain/RuntimeHost.ts (1)
829-853: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winKeep the provenance basis in sync in
_setMaterializedState_onMaterializednow compares against_materializedGraph?.stateHash,_cachedFrontier, and_cachedCeiling; this path updates_materializedGraphbut leaves the other two stale, so the next materialization can make the wrong preserve/degrade call.🤖 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/domain/RuntimeHost.ts` around lines 829 - 853, Update _setMaterializedState to refresh _cachedFrontier and _cachedCeiling alongside _materializedGraph after materialization, using the corresponding values from result or the established materialized-state basis. Ensure the provenance inputs consumed by _onMaterialized remain synchronized for the next materialization.
🤖 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/MaterializeStrategyRuntime.ts`:
- Around line 40-41: Remove the duplicate MaterializeWrappedStateProvenance
union and import the existing WarpStateSnapshotProvenancePosture type from the
shared port definition. Update any references in MaterializeStrategyRuntime.ts
to use the shared alias while preserving the existing full/degraded behavior.
---
Outside diff comments:
In `@src/domain/RuntimeHost.ts`:
- Around line 829-853: Update _setMaterializedState to refresh _cachedFrontier
and _cachedCeiling alongside _materializedGraph after materialization, using the
corresponding values from result or the established materialized-state basis.
Ensure the provenance inputs consumed by _onMaterialized remain synchronized for
the next materialization.
🪄 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
Run ID: cb8bf8b5-9a88-4f8e-ab24-d7814e9beb56
📒 Files selected for processing (18)
docs/topics/cas-first-memoized-materialization.mdsrc/domain/RuntimeHost.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/MaterializeCheckpointStrategy.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeHelpers.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/MaterializeProvenancePolicy.tssrc/domain/services/controllers/MaterializeSnapshotCacheResult.tssrc/domain/services/controllers/MaterializeStrategyRuntime.tssrc/ports/WarpStateCachePort.tstest/unit/domain/RuntimeHost.snapshotCache.test.tstest/unit/domain/services/controllers/CheckpointController.snapshotCache.test.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tstest/unit/domain/services/controllers/MaterializeController.test.tstest/unit/domain/services/controllers/MaterializeProvenancePolicy.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: preflight
- GitHub Check: test-node (22)
- GitHub Check: coverage-threshold
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/unit/domain/services/controllers/MaterializeProvenancePolicy.test.tstest/unit/domain/RuntimeHost.snapshotCache.test.tssrc/ports/WarpStateCachePort.tssrc/domain/services/controllers/MaterializeHelpers.tssrc/domain/services/controllers/MaterializeCheckpointStrategy.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tstest/unit/domain/services/controllers/CheckpointController.snapshotCache.test.tssrc/domain/services/controllers/MaterializeProvenancePolicy.tssrc/domain/services/controllers/MaterializeStrategyRuntime.tssrc/domain/RuntimeHost.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tssrc/domain/services/controllers/MaterializeLiveStrategy.tstest/unit/domain/services/controllers/MaterializeController.test.tssrc/domain/services/controllers/CheckpointController.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.tssrc/domain/services/controllers/MaterializeSnapshotCacheResult.tssrc/domain/services/controllers/MaterializeController.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/ports/WarpStateCachePort.tssrc/domain/services/controllers/MaterializeHelpers.tssrc/domain/services/controllers/MaterializeCheckpointStrategy.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeProvenancePolicy.tssrc/domain/services/controllers/MaterializeStrategyRuntime.tssrc/domain/RuntimeHost.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/MaterializeSnapshotCacheResult.tssrc/domain/services/controllers/MaterializeController.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/WarpStateCachePort.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/MaterializeHelpers.tssrc/domain/services/controllers/MaterializeCheckpointStrategy.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeProvenancePolicy.tssrc/domain/services/controllers/MaterializeStrategyRuntime.tssrc/domain/RuntimeHost.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/MaterializeSnapshotCacheResult.tssrc/domain/services/controllers/MaterializeController.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/MaterializeHelpers.tssrc/domain/services/controllers/MaterializeCheckpointStrategy.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeProvenancePolicy.tssrc/domain/services/controllers/MaterializeStrategyRuntime.tssrc/domain/RuntimeHost.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/CheckpointController.tssrc/domain/services/controllers/MaterializeSnapshotCacheResult.tssrc/domain/services/controllers/MaterializeController.ts
🔇 Additional comments (17)
src/domain/services/controllers/MaterializeHelpers.ts (1)
110-118: LGTM!src/domain/services/controllers/MaterializeProvenancePolicy.ts (1)
1-58: LGTM!src/domain/services/controllers/MaterializeLiveStrategy.ts (1)
129-139: LGTM!Also applies to: 243-252, 291-291
src/domain/services/controllers/MaterializeCoordinateStrategy.ts (1)
145-145: LGTM!src/domain/services/controllers/MaterializeSnapshotCacheResult.ts (1)
3-3: LGTM!Also applies to: 48-51
test/unit/domain/services/controllers/MaterializeController.snapshotCache.test.ts (1)
58-67: LGTM!Also applies to: 188-217, 244-244, 253-300, 450-450, 484-484
test/unit/domain/services/controllers/MaterializeProvenancePolicy.test.ts (1)
1-79: LGTM!src/domain/services/controllers/MaterializeController.ts (1)
20-20: LGTM!Also applies to: 55-55, 215-228, 232-248, 254-275, 332-333, 342-346, 359-359
src/domain/services/controllers/MaterializeCheckpointStrategy.ts (1)
33-33: LGTM!test/unit/domain/services/controllers/MaterializeController.test.ts (1)
402-424: LGTM!Also applies to: 426-446, 449-468
test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts (1)
251-255: LGTM!src/ports/WarpStateCachePort.ts (1)
15-15: LGTM!src/domain/RuntimeHost.ts (1)
47-51: LGTM!Also applies to: 744-746
src/domain/services/controllers/CheckpointController.ts (1)
29-30: LGTM!Also applies to: 137-140, 195-210
test/unit/domain/RuntimeHost.snapshotCache.test.ts (1)
1-100: LGTM!test/unit/domain/services/controllers/CheckpointController.snapshotCache.test.ts (1)
189-192: LGTM!docs/topics/cas-first-memoized-materialization.md (1)
50-59: LGTM!
|
Addressed both CodeRabbit findings in |
Release Preflight
If this PR is from a |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
1 participant
Refs #734.
Refs #738.
Refs #739.
Summary
RuntimeHostrequests a diff for its resident bitmap view.materializeAt()replay omits it.Root Cause
RuntimeHost.materialize()requests a diff after its bitmap index has been built.MaterializeLiveStrategytreatedwantDiffas a reason to bypass the state cache entirely, so every subsequent materialization replayed patch history even when an exact retained snapshot existed.Simply allowing the exact hit exposed two metadata hazards in the existing snapshot result: it replaced a valid resident provenance index with an empty index, and it reported maximum observed Lamport time as zero. The fix makes exact-hit reuse conditional only at the cache lookup layer, then preserves resident provenance only when hash, frontier, ceiling, and zero-replay facts prove the basis is unchanged. Restored Lamport truth comes from the state itself.
The same provenance comparison also depends on the basis metadata attached to the resident state. Eager patch and sync installation now advance
_cachedFrontierand_cachedCeilingtogether with_materializedGraph; calls without a known coordinate clear that metadata rather than retaining a stale basis.Evidence Contract
The current state-cache payload stores
WarpState, notProvenanceIndex. Cache-only restoration therefore reports degraded provenance instead of presenting an empty index as complete support. An unchanged resident runtime may keep its existing index, but its degraded/full posture is never upgraded by the cache hit.The same rule now covers checkpoint replay. A checkpoint carrying a prefix provenance index retains full posture and extends that index with suffix patches. A cache-backed checkpoint without that index, and the state-only
materializeAt()path, report degraded provenance.Materialized state installation accepts an explicit causal coordinate. Patch and sync controllers supply the coordinate they actually installed; unknown or legacy bases fail closed to
null, preventing a later materialization from preserving provenance against stale coordinate metadata.Validation
npm run test:local: 563 files passed, 6,952 tests passed, 2 skipped.npm run test:coverage:ci: 575 files passed, 1 skipped; 7,026 tests passed, 2 skipped; 92.72% line coverage.npm run typechecknpm run lintnpm run buildnpm run typecheck:consumernpm run lint:semgrepnpm run lint:sludgenpm run lint:docs-topologynpm run typecheck:surface4c2392e0e298.Scope
This removes redundant replay for exact warm hits and fixes the causal metadata contract around those hits. It does not close #738 or #739: full
WarpStatehydration remains, and bounded CAS page-backed observer reads, shared checkpoint/materialization bundles, cache diagnostics, and removal of remaining WARP-local cache policy are still required.