Fix live materialization state-cache hits - #708
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughUpdates live materialization to use state-cache-backed exact, predecessor, and replay paths, extracts snapshot helpers, constrains checkpoint suffix streaming, and refreshes related tests, docs, and changelog entries. ChangesWARP State-Cache Materialization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
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: 3
🤖 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/MaterializeLiveStrategy.ts`:
- Around line 153-161: The cache-assisted materialization flow is dropping the
requested diff flag, so `materialize({ wantDiff: true })` can return a cache hit
without computing diff data. Update `tryResolveSnapshotCache`,
`tryResolveExactSnapshot`, `tryResolvePredecessorSnapshot`, and the replay path
that currently hardcodes `wantDiff: false` so `wantDiff` is either threaded
through or used to disable cache resolution when true. Make sure the
`MaterializeLiveStrategy` paths stay consistent with `fromScratch()`,
`fromFrontier()`, and `fromCheckpoint()` by preserving diff computation whenever
`wantDiff` is requested.
- Around line 66-69: The fallback path in MaterializeLiveStrategy.fromCheckpoint
is not fully bound to the requested coordinate, so it can rebuild state from a
newer checkpoint and publish/cache it under an older frontier. Update the
checkpoint replay flow to respect the coordinate.frontier everywhere in this
path: ensure streamPatchesSince(checkpoint) is constrained to the same frontier
passed from the caller, and keep the returned/persisted materialized snapshot
keyed to that exact coordinate. Use the existing fromCheckpoint, loadCheckpoint,
and streamPatchesSince logic as the main touchpoints.
In `@src/domain/services/controllers/MaterializeSnapshotCacheResult.ts`:
- Around line 20-27: Replace the positional receipts boolean with a named
options object in both receiptsAllowSnapshot and canUseSnapshot to avoid
boolean-trap parameters. Update the helper and exported function signatures to
accept an options object with a clearly named receipts field, then adjust the
internal check and all call sites accordingly. Keep the type guard behavior of
canUseSnapshot unchanged while using the new options shape.
🪄 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: 24d55d53-a45f-4fd3-b055-98f777c32dd7
📒 Files selected for processing (8)
CHANGELOG.mddocs/topics/README.mddocs/topics/cas-first-memoized-materialization.mdsrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/MaterializeSnapshotCacheResult.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: test-node (22)
- GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (4)
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/domain/services/controllers/MaterializeSnapshotCacheResult.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.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/MaterializeSnapshotCacheResult.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.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/MaterializeSnapshotCacheResult.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.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/domain/services/controllers/MaterializeSnapshotCacheResult.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.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 (2)
src/domain/services/controllers/MaterializeSnapshotCacheResult.ts (1)
1-19: LGTM!Also applies to: 34-47
src/domain/services/controllers/MaterializeCoordinateStrategy.ts (1)
10-13: LGTM!Also applies to: 103-104, 114-114
Release Preflight
If this PR is from a |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/domain/services/controllers/MaterializeLiveStrategy.ts (1)
47-69: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftValidate checkpoint compatibility before seeding a captured live coordinate.
coordinate.frontieris captured beforeloadCheckpoint(), thenfromCheckpoint()starts fromcheckpoint.stateand publishes under that olderfrontier. If the checkpoint has advanced beyond or outside the captured coordinate, the bounded suffix stream cannot remove already-applied patches, so this can still cache/return state under the wrong WARP coordinate. Only use the checkpoint when it is a compatible predecessor ofcoordinate; otherwise fall back tofromFrontier(coordinate, opts).Also applies to: 79-90
🤖 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/services/controllers/MaterializeLiveStrategy.ts` around lines 47 - 69, `MaterializeLiveStrategy.replayCurrentCoordinate` is using `loadCheckpoint()` without verifying that the checkpoint is compatible with the captured `coordinate.frontier`, which can seed `fromCheckpoint()` with an invalid WARP coordinate. Update the checkpoint path to validate that the checkpoint is a predecessor/compatible with `coordinate` before calling `fromCheckpoint(checkpoint, opts, coordinate.frontier)`, and if it is not, fall back to `fromFrontier(coordinate, opts)` instead. Keep the check local to `replayCurrentCoordinate` (and any related checkpoint-handling code in this flow) so only safe checkpoints are reused.
🤖 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.
Duplicate comments:
In `@src/domain/services/controllers/MaterializeLiveStrategy.ts`:
- Around line 47-69: `MaterializeLiveStrategy.replayCurrentCoordinate` is using
`loadCheckpoint()` without verifying that the checkpoint is compatible with the
captured `coordinate.frontier`, which can seed `fromCheckpoint()` with an
invalid WARP coordinate. Update the checkpoint path to validate that the
checkpoint is a predecessor/compatible with `coordinate` before calling
`fromCheckpoint(checkpoint, opts, coordinate.frontier)`, and if it is not, fall
back to `fromFrontier(coordinate, opts)` instead. Keep the check local to
`replayCurrentCoordinate` (and any related checkpoint-handling code in this
flow) so only safe checkpoints are reused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0d981ead-51b2-4a5b-9cb0-b61aa86caa77
📒 Files selected for processing (7)
CHANGELOG.mdsrc/domain/capabilities/PatchCollector.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/MaterializeSnapshotCacheResult.tstest/unit/domain/RuntimePatchCollector.stream.test.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: test-node (22)
- GitHub Check: coverage-threshold
- GitHub Check: type-firewall-lint
- 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/unit/domain/RuntimePatchCollector.stream.test.tssrc/domain/services/controllers/MaterializeSnapshotCacheResult.tssrc/domain/capabilities/PatchCollector.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.tssrc/domain/services/controllers/MaterializeLiveStrategy.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/domain/services/controllers/MaterializeSnapshotCacheResult.tssrc/domain/capabilities/PatchCollector.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeLiveStrategy.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/MaterializeSnapshotCacheResult.tssrc/domain/capabilities/PatchCollector.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeLiveStrategy.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/MaterializeSnapshotCacheResult.tssrc/domain/capabilities/PatchCollector.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeLiveStrategy.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 (6)
test/unit/domain/RuntimePatchCollector.stream.test.ts (1)
52-78: LGTM!Also applies to: 80-107
test/unit/domain/services/controllers/MaterializeController.snapshotCache.test.ts (1)
238-262: LGTM!Also applies to: 264-305
src/domain/services/controllers/MaterializeSnapshotCacheResult.ts (1)
14-38: LGTM!Also applies to: 41-54
src/domain/services/controllers/MaterializeCoordinateStrategy.ts (1)
103-104: LGTM!Also applies to: 114-114
src/domain/services/controllers/MaterializeLiveStrategy.ts (1)
55-55: LGTM!Also applies to: 94-114, 176-229
src/domain/capabilities/PatchCollector.ts (1)
30-78: LGTM!Also applies to: 133-144
|
@codex Self-audit finding from Code Lawyer pass:
|
Release Preflight
If this PR is from a |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/domain/services/controllers/MaterializeLiveStrategy.ts (1)
47-69: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAvoid replaying from a checkpoint loaded after the target frontier.
Line 47 captures the live coordinate, but Line 67 can load a checkpoint written after that coordinate.
fromCheckpoint()then reduces fromcheckpoint.stateand returns it under the oldercoordinate.frontier, so a cache miss can still publish/cache state under the wrong coordinate. Re-read the replay frontier after loading the checkpoint, or only use checkpoints proven to be compatible predecessors of the requested coordinate.🐛 One localized way to keep the checkpoint base at-or-before the replay coordinate
- return await this.replayCurrentCoordinate(coordinate, opts); + return await this.replayCurrentCoordinate(opts); } private async replayCurrentCoordinate( - coordinate: WarpStateCoordinate, opts: MaterializeLiveOptions, ): Promise<MaterializeResult> { const checkpoint = await this.runtime.deps.patches.loadCheckpoint(); + const frontier = await this.runtime.deps.patches.getFrontier(); + if (frontier.size === 0) { + return await this.runtime.emptyResult(null, frontier); + } + const coordinate = this.snapshotCoordinate(frontier); if (checkpoint !== null && checkpoint !== undefined && isCurrentCheckpointSchema(checkpoint.schema)) { return await this.fromCheckpoint(checkpoint, opts, coordinate.frontier); }🤖 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/services/controllers/MaterializeLiveStrategy.ts` around lines 47 - 69, MaterializeLiveStrategy.replayCurrentCoordinate is using a checkpoint that may have been written after the live coordinate was captured in the earlier snapshot flow, which can cause state to be returned under the wrong frontier. Update the checkpoint path in replayCurrentCoordinate to verify the loaded checkpoint is at-or-before the requested WarpStateCoordinate.frontier, or re-read/compare the replay frontier after load before calling fromCheckpoint, so only compatible predecessor checkpoints are used.
🤖 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/unit/domain/services/controllers/MaterializeController.snapshotCache.test.ts`:
- Around line 264-288: Update the receipt-path snapshot cache bypass tests in
MaterializeController.snapshotCache.test.ts so they also assert that
MaterializeController.materialize does not call stateCache.put when receipts are
requested. The current test only checks the read-side behavior via
stateCache.getExact, stateCache.getBestCompatiblePredecessor, and
patches.collectForFrontier; add a write-side expectation for stateCache.put to
remain unused in the receipt-backed path. Apply the same tightening to the other
receipt-related test block referenced in the comment so both receipt scenarios
fully verify the cache bypass contract.
---
Duplicate comments:
In `@src/domain/services/controllers/MaterializeLiveStrategy.ts`:
- Around line 47-69: MaterializeLiveStrategy.replayCurrentCoordinate is using a
checkpoint that may have been written after the live coordinate was captured in
the earlier snapshot flow, which can cause state to be returned under the wrong
frontier. Update the checkpoint path in replayCurrentCoordinate to verify the
loaded checkpoint is at-or-before the requested WarpStateCoordinate.frontier, or
re-read/compare the replay frontier after load before calling fromCheckpoint, so
only compatible predecessor checkpoints are used.
🪄 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: 5bacec68-c959-4293-ada4-dcae0620ee51
📒 Files selected for processing (4)
CHANGELOG.mdsrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeLiveStrategy.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: test-node (22)
- GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (4)
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/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeLiveStrategy.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/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeLiveStrategy.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/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeLiveStrategy.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/domain/services/controllers/MaterializeCoordinateStrategy.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.tssrc/domain/services/controllers/MaterializeLiveStrategy.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 (2)
src/domain/services/controllers/MaterializeCoordinateStrategy.ts (1)
80-82: LGTM!Also applies to: 101-108, 112-139
src/domain/services/controllers/MaterializeLiveStrategy.ts (1)
74-114: LGTM!Also applies to: 148-167, 176-229
Release Preflight
If this PR is from a |
1 similar comment
Release Preflight
If this PR is from a |
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 `@src/domain/services/controllers/MaterializeController.ts`:
- Around line 257-265: The receipt-path cache bypass is only applied in
_buildResult(), but _emptyResult() still goes through _wrapState() and
_publishSnapshot(), so zero-patch live paths can still repopulate
WarpStateCachePort. Update MaterializeController’s empty-result flow to be
receipt-aware by either adding a publishSnapshot flag through the runtime or
skipping _publishSnapshot() when params.reduced.receipts is present, using the
existing _emptyResult(), _wrapState(), and _publishSnapshot() paths as the main
touchpoints.
🪄 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: 105e1fff-7766-42e1-b630-ea84bfea5c4c
📒 Files selected for processing (6)
CHANGELOG.mdsrc/domain/capabilities/PatchCollector.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/warp/RuntimePatchCollector.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: type-firewall
- GitHub Check: coverage-threshold
- GitHub Check: test-node (22)
- GitHub Check: preflight
🧰 Additional context used
📓 Path-based instructions (4)
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/domain/warp/RuntimePatchCollector.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/capabilities/PatchCollector.tssrc/domain/services/controllers/MaterializeLiveStrategy.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/warp/RuntimePatchCollector.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/capabilities/PatchCollector.tssrc/domain/services/controllers/MaterializeLiveStrategy.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/warp/RuntimePatchCollector.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/capabilities/PatchCollector.tssrc/domain/services/controllers/MaterializeLiveStrategy.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/domain/warp/RuntimePatchCollector.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/capabilities/PatchCollector.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.tssrc/domain/services/controllers/MaterializeLiveStrategy.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
Release Preflight
If this PR is from a |
Code Lawyer Activity Summary
Validation:
|
1 participant
Closes #709.
Summary
This PR turns the June 29 state-cache work into a real live-materialization cache path.
WarpStateCachePortis available.frontier: null.Root Cause
The v18.2.0 state-cache adapter was wired into Git-backed runtimes, but live materialization still used the old null-frontier path. Because
MaterializeController._publishSnapshot()skips null frontiers, normal live materialization did not reliably populate or hit the state cache. Exact cache hits also flowed through the normal result builder, which republished the cached state instead of returning directly.Validation
npm run typechecknpx vitest run test/unit/domain/services/controllers/MaterializeController.snapshotCache.test.ts test/unit/domain/services/controllers/MaterializeController.test.ts test/unit/domain/services/controllers/MaterializeController.stateSession.test.ts test/unit/domain/services/controllers/CheckpointController.snapshotCache.test.tsnpm run lintnpm run test:localnpm run buildRelease Note
After merge, cut a patch release so XYPH can consume the corrected live-materialization cache behavior from npm.