Read retained node liveness without whole-state materialization#757
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 20 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 (5)
📝 WalkthroughWalkthroughAdds bounded trie-backed node-liveness reads for retained materialization roots, wires them through runtime and query layers, preserves fallback to full state materialization, and adds unit, controller, integration, and documentation coverage. ChangesRetained node-liveness reads
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant QueryReads
participant RuntimeHost
participant MaterializeController
participant MaterializeLiveStrategy
participant TrieMaterializationReader
QueryReads->>RuntimeHost: query node presence
RuntimeHost->>MaterializeController: readLiveNodePresence(nodeId)
MaterializeController->>MaterializeLiveStrategy: readNodePresence(nodeId)
MaterializeLiveStrategy->>TrieMaterializationReader: hasNode(nodeAliveHandle, nodeId)
TrieMaterializationReader-->>QueryReads: boolean or null
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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
🤖 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/materialization/TrieMaterializationReader.ts`:
- Around line 18-27: Update the TrieMaterializationReader constructor to
validate options, store, codec, and optional geometry using the domain’s
established validation errors before accessing their properties, then call
Object.freeze(this) after initialization. Preserve the default
TrieGeometry.default16way() behavior and use instanceof-based dispatch where
runtime type selection is required.
🪄 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: 97e7550d-9194-4985-a095-5a20375f05e8
📒 Files selected for processing (14)
docs/topics/cas-first-memoized-materialization.mdsrc/domain/RuntimeHost.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeDeps.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/QueryReads.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/warp/RuntimeHostBoot.tssrc/ports/MaterializationReadPort.tstest/integration/api/materialization.retainedResume.test.tstest/unit/domain/materialization/TrieMaterializationReader.test.tstest/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.tstest/unit/domain/services/controllers/QueryController.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)
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/MaterializationReadPort.tssrc/domain/services/controllers/MaterializeDeps.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/services/controllers/QueryReads.tssrc/domain/RuntimeHost.tssrc/domain/warp/RuntimeHostBoot.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeLiveStrategy.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/MaterializationReadPort.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/ports/MaterializationReadPort.tstest/unit/domain/materialization/TrieMaterializationReader.test.tssrc/domain/services/controllers/MaterializeDeps.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/materialization/TrieMaterializationReader.tstest/unit/domain/services/controllers/QueryController.test.tssrc/domain/services/controllers/QueryReads.tssrc/domain/RuntimeHost.tstest/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.tstest/integration/api/materialization.retainedResume.test.tssrc/domain/warp/RuntimeHostBoot.tssrc/domain/services/controllers/MaterializeController.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/MaterializeDeps.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/services/controllers/QueryReads.tssrc/domain/RuntimeHost.tssrc/domain/warp/RuntimeHostBoot.tssrc/domain/services/controllers/MaterializeController.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/MaterializeDeps.tssrc/domain/services/controllers/ReadGraphHost.tssrc/domain/materialization/TrieMaterializationReader.tssrc/domain/services/controllers/QueryReads.tssrc/domain/RuntimeHost.tssrc/domain/warp/RuntimeHostBoot.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeLiveStrategy.ts
🪛 ast-grep (0.44.1)
test/integration/api/materialization.retainedResume.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🔇 Additional comments (14)
src/ports/MaterializationReadPort.ts (1)
1-6: LGTM!src/domain/materialization/TrieMaterializationReader.ts (1)
1-16: LGTM!Also applies to: 29-45
src/domain/services/controllers/MaterializeDeps.ts (1)
1-30: LGTM!test/unit/domain/materialization/TrieMaterializationReader.test.ts (1)
1-40: LGTM!src/domain/services/controllers/MaterializeController.ts (1)
55-57: LGTM!Also applies to: 79-79, 164-166
src/domain/services/controllers/MaterializeLiveStrategy.ts (1)
16-17: LGTM!Also applies to: 72-88, 349-368
test/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.ts (1)
1-260: LGTM!src/domain/warp/RuntimeHostBoot.ts (1)
8-8: LGTM!Also applies to: 38-38, 76-76, 340-340, 357-362, 391-393
src/domain/RuntimeHost.ts (1)
405-407: LGTM!Also applies to: 428-431
src/domain/services/controllers/ReadGraphHost.ts (1)
31-31: LGTM!src/domain/services/controllers/QueryReads.ts (1)
125-130: LGTM!test/integration/api/materialization.retainedResume.test.ts (1)
1-2: LGTM!Also applies to: 23-24, 75-119, 153-160
test/unit/domain/services/controllers/QueryController.test.ts (1)
121-121: LGTM!Also applies to: 184-196
docs/topics/cas-first-memoized-materialization.md (1)
105-112: LGTM!Also applies to: 174-176
Release Preflight
If this PR is from a |
Self-Code Review@codex please confirm the review disposition below.
Review conclusionAfter the correction, I found no remaining correctness, lifecycle, public-surface, or Markdown violations in Verification
|
Release Preflight
If this PR is from a |
Release Preflight
If this PR is from a |
Independent Code Lawyer Review@codex please confirm the independent review disposition.
Residual riskConcurrent frontier advancement during one exact membership operation and deliberately corrupted retained trie pages are not newly exercised by this slice. The operation snapshots the frontier before acquisition, and existing trie readers fail closed on malformed pages; these remain bounded follow-up test opportunities rather than merge blockers. Verification
|
Release Preflight
If this PR is from a |
1 participant
Summary
MaterializationReadPortand trie-backed reader for independently retained node-liveness rootsRuntimeHost.hasNode()through operation-scoped retained materialization acquisitions before the explicit legacy full-state fallback_cachedStateIssue scope
This advances #738 under the #734 storage-ownership campaign. Both issues remain open: cold retained-handle misses and graph reads other than exact node liveness can still reconstruct a complete
WarpState.Invariants
Source evidence
src/ports/MaterializationReadPort.ts#3-5@56ed3ce16acd36cada7446c4d3dfcd8b0d036f30]src/domain/materialization/TrieMaterializationReader.ts#10-43@56ed3ce16acd36cada7446c4d3dfcd8b0d036f30]src/domain/services/controllers/MaterializeLiveStrategy.ts#72-86@56ed3ce16acd36cada7446c4d3dfcd8b0d036f30]src/domain/services/controllers/QueryReads.ts#124-132@56ed3ce16acd36cada7446c4d3dfcd8b0d036f30]src/domain/warp/RuntimeHostBoot.ts#357-361@56ed3ce16acd36cada7446c4d3dfcd8b0d036f30]test/integration/api/materialization.retainedResume.test.ts#76-108@56ed3ce16acd36cada7446c4d3dfcd8b0d036f30]Validation
npm run lintnpm run typechecknpm run typecheck:policynpm run typecheck:surface(0 errors, 0 warnings)npm run build && npm run typecheck:consumernpm run lint:semgrepnpm run lint:sludgenpm run test:local(7,031 passed, 2 skipped)Structural audit note
Graft reports the internal
MaterializeDepsandMaterializePersistencedeclarations as removed and re-added because they moved to a dedicated source-size-compliant module and are re-exported from their original module. The package publication-surface validator reports zero changes, errors, or warnings.