Skip to content

Replay targeted cold node properties#774

Merged
flyingrobots merged 1 commit into
mainfrom
v19/targeted-node-property-replay
Jul 25, 2026
Merged

Replay targeted cold node properties#774
flyingrobots merged 1 commit into
mainfrom
v19/targeted-node-property-replay

Conversation

@flyingrobots

Copy link
Copy Markdown
Member

Summary

  • replay only one proven-live node property bag when a partial materialization has no usable property root
  • preserve exact-coordinate LWW ordering, safe property keys, immutable results, and acquisition cleanup without constructing WarpState
  • document the remaining upstream PatchCollector writer-chain residency boundary and keep unreleased descriptor schemas out of the v19 contract

Issue

Refs #738

This is an incremental bounded-read slice. It intentionally does not close #738: neighborhoods, list reads, checkpoint creation, and other compatibility operations still reconstruct whole state, and PatchCollector may still buffer one writer chain.

Test plan

  • npx vitest run test/unit/domain/services/controllers/TargetedNodePropertyReplay.test.ts test/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.ts (23 passed)
  • npm run typecheck
  • npm run lint
  • npm run test:coverage:ci (7,371 passed; 2 skipped; all ratchets held)
  • npx vitest run test/unit test/integration (7,453 passed; 2 skipped)
  • pre-push IRONCLAD static gates and stable-unit shards (all passed)
  • git diff --check

ADR checks

  • This PR does not implement ADR 2 without satisfying ADR 3
  • If this PR touches persisted op formats, I linked the ADR 3 readiness issue
  • If this PR touches wire compatibility, I confirmed canonical-only ops are still rejected on the wire pre-cutover
  • If this PR touches schema constants, I confirmed patch and checkpoint namespaces remain distinct

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c23329ef-fd7b-432e-9be8-af2ad0231d17

📥 Commits

Reviewing files that changed from the base of the PR and between 571389a and ae394f6.

📒 Files selected for processing (6)
  • CHANGELOG.md
  • docs/topics/cas-first-memoized-materialization.md
  • src/domain/services/controllers/MaterializeLiveStrategy.ts
  • src/domain/services/controllers/TargetedNodePropertyReplay.ts
  • test/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.ts
  • test/unit/domain/services/controllers/TargetedNodePropertyReplay.test.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout. (5)
  • GitHub Check: test-node (22)
  • GitHub Check: coverage-threshold
  • GitHub Check: test-deno
  • GitHub Check: type-firewall-lint
  • 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 introduce any, as any, as unknown as, unknown (outside adapters), Record<string, unknown> (outside adapters), *Like placeholder types, JSON.parse/JSON.stringify (outside adapters), fetch (outside adapters), process.env (outside adapters), @ts-ignore, or z.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 create utils.ts, helpers.ts, misc.ts, or common.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 as Uint8Array; Buffer belongs in infrastructure adapters.

Files:

  • src/domain/services/controllers/TargetedNodePropertyReplay.ts
  • src/domain/services/controllers/MaterializeLiveStrategy.ts
src/domain/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/domain/**/*.{ts,tsx,js,jsx}: In src/domain/**, do not use Date.now(), new Date(), Date(), performance.now(), Math.random(), crypto.randomUUID(), crypto.getRandomValues(), setTimeout, setInterval, raw new 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 extend WarpError.
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/TargetedNodePropertyReplay.ts
  • src/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, and instanceof dispatch; domain objects should be runtime-backed nouns, not ad hoc shape bags.

Files:

  • src/domain/services/controllers/TargetedNodePropertyReplay.ts
  • src/domain/services/controllers/MaterializeLiveStrategy.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Do not use direct imports from src/infrastructure/** in src/domain/** or src/ports/**; depend on a port instead.
Do not use direct Node built-ins in src/domain/** or src/ports/**; use a port instead.

Files:

  • src/domain/services/controllers/TargetedNodePropertyReplay.ts
  • test/unit/domain/services/controllers/TargetedNodePropertyReplay.test.ts
  • src/domain/services/controllers/MaterializeLiveStrategy.ts
  • test/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.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)
src/domain/services/controllers/TargetedNodePropertyReplay.ts (1)

1-86: LGTM!

test/unit/domain/services/controllers/TargetedNodePropertyReplay.test.ts (1)

1-213: LGTM!

src/domain/services/controllers/MaterializeLiveStrategy.ts (1)

15-15: LGTM!

Also applies to: 33-33, 100-105, 405-430

test/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.ts (1)

17-18: LGTM!

Also applies to: 29-31, 49-53, 219-282, 304-305, 318-319, 332-332, 364-364, 449-466

docs/topics/cas-first-memoized-materialization.md (1)

150-156: LGTM!

Also applies to: 233-237

CHANGELOG.md (1)

74-78: LGTM!

Also applies to: 106-108


📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Cold live-node property reads can now retrieve properties even when retained property data is unavailable.
    • Reads target only the requested node and return its latest valid property values without loading the full graph state.
  • Bug Fixes

    • Improved handling of unavailable or empty property roots.
    • Ensured failed property replays clean up resources and report errors correctly.
  • Documentation

    • Clarified bounded-memory behavior, compatibility fallbacks, and property-root availability in the materialization documentation.

Walkthrough

Cold live node-property reads now fall back to exact-coordinate replay of one node’s NodePropSet operations when retained property roots are unavailable. The reducer produces a validated, deterministic LWW property bag without hydrating whole state, and documentation/tests cover the behavior.

Changes

Cold property replay

Layer / File(s) Summary
Targeted property reducer
src/domain/services/controllers/TargetedNodePropertyReplay.ts, test/unit/domain/services/controllers/TargetedNodePropertyReplay.test.ts
Adds targeted NodePropSet replay with LWW selection, value validation, sorted frozen output, and focused tests.
Cold-read fallback wiring
src/domain/services/controllers/MaterializeLiveStrategy.ts, test/unit/domain/services/controllers/MaterializeController.liveNodeRead.test.ts
Uses retained properties when available and targeted replay otherwise, with coverage for exact patch tips, empty results, dependency avoidance, and cleanup failures.
Behavior and schema documentation
docs/topics/cas-first-memoized-materialization.md, CHANGELOG.md
Documents partial-handle property roots, bounded targeted replay, and schema v5 retained or explicitly empty roots.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MaterializeController
  participant MaterializeLiveStrategy
  participant PatchCollector
  MaterializeController->>MaterializeLiveStrategy: request live node properties
  MaterializeLiveStrategy->>MaterializeLiveStrategy: try retained property root
  MaterializeLiveStrategy->>PatchCollector: stream exact-coordinate patches
  PatchCollector-->>MaterializeLiveStrategy: targeted patch chain
  MaterializeLiveStrategy-->>MaterializeController: frozen node property bag
Loading

Possibly related PRs

  • git-stunts/git-warp#773: Adds the bounded partial live-materialization model that this targeted property replay builds upon.

Poem

I’m a bunny with patches, hopping light,
One node’s small bag comes out just right.
LWW leaves the winning key,
Frozen and sorted for all to see.
No giant state cloud fills the night—
Just a tidy replay, quick and bright!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning This PR only adds targeted node-property replay and does not satisfy the broader #738 goals of replacing full-state caches and handle-based bounded reads. Split this slice from #738 or add the missing cache/handle replacement and broader bounded-read behavior before marking the issue compliant.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately captures the main change: targeted replay of cold node properties.
Description check ✅ Passed The description matches the template with Summary, Issue, Test plan, and checked ADR items.
Out of Scope Changes check ✅ Passed The changes stay focused on targeted cold property replay, documentation, tests, and related changelog updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Release Preflight

  • package version: 18.2.1
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If this PR is from a release/* branch and merges to main, Main Push Release Branch Check will run final preflight and create v18.2.1. A maintainer who is a JSR @git-stunts scope member must then dispatch the Release workflow manually.

@flyingrobots
flyingrobots merged commit 70571ab into main Jul 25, 2026
17 checks passed
@flyingrobots
flyingrobots deleted the v19/targeted-node-property-replay branch July 25, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace RuntimeHost full state with CAS-backed materialization handles

1 participant