Add runtime-backed strand forks - #795
Conversation
|
Warning Review limit reached
Next review available in: 33 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 Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe PR adds ChangesRuntime forking and strand lanes
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant WorldlineLane
participant RuntimeLaneAdapter
participant DraftTimeline
participant Observer
Runtime->>WorldlineLane: fork(name)
WorldlineLane->>RuntimeLaneAdapter: create strand from fork coordinate
RuntimeLaneAdapter->>DraftTimeline: create draft reading target
Observer->>RuntimeLaneAdapter: start observation
RuntimeLaneAdapter->>DraftTimeline: read using pinned tick
DraftTimeline-->>Observer: readings and receipt
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/application/RuntimeLaneAdapter.ts`:
- Around line 94-115: Wrap the body of forkWorldlineLane in the same
activity.run guard used by captureWorldlineCoordinate, so direct lane fork calls
are tracked and reject with E_RUNTIME_CLOSED when the runtime is closing. Ensure
the guard covers createForkTick, createDraftTimeline, and createStrandLane,
while preserving the existing Runtime.fork behavior through idempotent nesting.
- Around line 68-81: Update bindWorldlineLaneRuntime so the fork closure
captures only activity, owner, parent, and timeline, not the full options object
or options.lane. Introduce or reuse a lane-free record for these fields, pass it
to forkWorldlineLane, and narrow forkWorldlineLane’s parameter type accordingly
while keeping bindLaneRuntime associated with the original lane.
In `@src/domain/WarpWorldline.ts`:
- Around line 69-76: Extract WarpStrandOpticBasis from WarpWorldline.ts into a
dedicated src/domain/WarpStrandOpticBasis.ts file and update
DraftTimelineRuntime imports to use it. Also extract DraftReadingTarget from
DraftTimelineRuntime.ts into a dedicated file and update RuntimeLaneAdapter
imports accordingly; remove the original inline exports while preserving both
public type APIs.
In `@test/unit/application/RuntimeLaneAdapter.test.ts`:
- Around line 77-79: In the test using requireLaneRuntime(lane).fork, replace
the non-null assertion after expect(fork).not.toBeNull() with explicit runtime
narrowing, matching the sibling test’s if (fork === null) throw pattern before
invoking fork. Preserve the existing 'try-admin-role' call and assertions.
- Around line 182-190: Replace the nested ternary assertion in the test around
the first and second results with the file’s existing Set-of-tick-ids idiom,
matching the direct pin-check pattern used near lines 279-282. Preserve the
expectation that both yielded values have the same tick ID without rechecking
their done state.
In `@test/unit/domain/services/strand/StrandService.test.ts`:
- Around line 716-730: Replace the Reflect.apply invocation in the malformed
explicit parent frontier test with a direct service.create call preceded by
`@ts-expect-error`, preserving compile-time validation of the invalid input.
Update the second rejection assertion to also require context.field to equal
baseFrontier, matching the stable error contract asserted for the first case.
🪄 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: cc0d264f-85d3-4d45-9b4c-e172fa403a2d
📒 Files selected for processing (20)
docs/topics/reference.mdindex.tssrc/application/Runtime.tssrc/application/RuntimeLaneAdapter.tssrc/domain/WarpWorldline.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/api/LaneRuntime.tssrc/domain/api/TickRuntime.tssrc/domain/api/TimelineRuntime.tssrc/domain/services/controllers/StrandController.tssrc/domain/services/strand/StrandCoordinator.tssrc/domain/services/strand/StrandDescriptorValidation.tssrc/domain/types/StrandDescriptor.tstest/conformance/v18FirstUseOpticsHonesty.test.tstest/integration/application/Runtime.fork.integration.test.tstest/type-check/v19-consumer.tstest/unit/application/Runtime.test.tstest/unit/application/RuntimeLaneAdapter.test.tstest/unit/domain/services/strand/StrandService.test.tstest/unit/scripts/v19-public-api-boundary.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: v19 base/head performance
- GitHub Check: test-node (22)
- GitHub Check: preflight
- GitHub Check: coverage-threshold
🧰 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:
index.tstest/integration/application/Runtime.fork.integration.test.tssrc/domain/api/TickRuntime.tssrc/domain/types/StrandDescriptor.tstest/unit/domain/services/strand/StrandService.test.tssrc/domain/services/controllers/StrandController.tstest/unit/scripts/v19-public-api-boundary.test.tssrc/domain/api/LaneRuntime.tssrc/domain/api/TimelineRuntime.tstest/unit/application/RuntimeLaneAdapter.test.tstest/type-check/v19-consumer.tssrc/domain/services/strand/StrandCoordinator.tssrc/domain/services/strand/StrandDescriptorValidation.tstest/conformance/v18FirstUseOpticsHonesty.test.tstest/unit/application/Runtime.test.tssrc/application/Runtime.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/WarpWorldline.tssrc/application/RuntimeLaneAdapter.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/api/TickRuntime.tssrc/domain/types/StrandDescriptor.tssrc/domain/services/controllers/StrandController.tssrc/domain/api/LaneRuntime.tssrc/domain/api/TimelineRuntime.tssrc/domain/services/strand/StrandCoordinator.tssrc/domain/services/strand/StrandDescriptorValidation.tssrc/application/Runtime.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/WarpWorldline.tssrc/application/RuntimeLaneAdapter.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/api/TickRuntime.tssrc/domain/types/StrandDescriptor.tssrc/domain/services/controllers/StrandController.tssrc/domain/api/LaneRuntime.tssrc/domain/api/TimelineRuntime.tssrc/domain/services/strand/StrandCoordinator.tssrc/domain/services/strand/StrandDescriptorValidation.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/WarpWorldline.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/api/TickRuntime.tssrc/domain/types/StrandDescriptor.tssrc/domain/services/controllers/StrandController.tssrc/domain/api/LaneRuntime.tssrc/domain/api/TimelineRuntime.tssrc/domain/services/strand/StrandCoordinator.tssrc/domain/services/strand/StrandDescriptorValidation.tssrc/domain/api/DraftTimelineRuntime.tssrc/domain/WarpWorldline.ts
🔇 Additional comments (30)
src/application/Runtime.ts (4)
1-6: LGTM!
22-42: LGTM!
73-85: LGTM!
96-131: LGTM!src/domain/api/LaneRuntime.ts (1)
5-8: LGTM!Also applies to: 19-20
index.ts (1)
14-17: LGTM!docs/topics/reference.md (1)
42-69: LGTM!test/unit/application/Runtime.test.ts (4)
22-50: LGTM!Also applies to: 71-79
105-134: LGTM!
136-158: LGTM!
187-210: LGTM!test/type-check/v19-consumer.ts (1)
16-16: LGTM!Also applies to: 27-28, 90-90
src/application/RuntimeLaneAdapter.ts (4)
2-12: LGTM!Also applies to: 25-28, 40-51
117-156: LGTM!
230-230: LGTM!Also applies to: 251-251, 274-274, 291-291, 333-333, 350-350, 367-367, 392-392, 415-415, 444-444
185-209: 🩺 Stability & AvailabilityNo change needed.
prepareStrandOpticdoes not surfaceE_OPTIC_NO_BOUNDED_BASIS; the only bounded-basis failure increateDraftReadingTargetis the nullstate.forkedAtpath, which fork-created draft strands do not reach.> Likely an incorrect or invalid review comment.test/unit/application/RuntimeLaneAdapter.test.ts (1)
136-181: LGTM!Also applies to: 191-203
src/domain/types/StrandDescriptor.ts (1)
92-97: LGTM!test/unit/domain/services/strand/StrandService.test.ts (1)
698-714: LGTM!src/domain/WarpWorldline.ts (2)
18-20: LGTM!Also applies to: 37-40, 88-90, 107-109, 127-129, 155-162, 192-204, 250-259, 311-318, 327-334, 352-442
336-350: 🩺 Stability & AvailabilityNo change needed: the fork bootstrap is already guarded by the activity queue.
The checkpoint bootstrap retries on
E_OPTIC_NO_BOUNDED_BASIS, and concurrentRuntime.fork(...)calls for the same runtime are serialized throughRuntimeActivity.run.> Likely an incorrect or invalid review comment.src/domain/api/DraftTimelineRuntime.ts (1)
2-3: LGTM!Also applies to: 13-16, 26-26, 41-47, 102-103, 120-146, 148-166, 281-298, 300-309
src/domain/api/TickRuntime.ts (1)
23-31: LGTM!src/domain/api/TimelineRuntime.ts (1)
15-20: LGTM!Also applies to: 46-63
test/conformance/v18FirstUseOpticsHonesty.test.ts (1)
138-141: LGTM!test/integration/application/Runtime.fork.integration.test.ts (1)
1-85: LGTM!src/domain/services/controllers/StrandController.ts (1)
38-38: LGTM!src/domain/services/strand/StrandCoordinator.ts (1)
17-17: LGTM!Also applies to: 184-188
src/domain/services/strand/StrandDescriptorValidation.ts (1)
117-161: LGTM!test/unit/scripts/v19-public-api-boundary.test.ts (1)
29-29: 🎯 Functional CorrectnessNo action needed for
RuntimeForkOptions.
Release Preflight
If this PR is from a |
Release Preflight
If this PR is from a |
Closes #792.
Adds
Runtime.fork()with same-runtime ownership, exact captured-frontier persistence, runtime-backed strand writes, and checkpoint-tail bounded strand observations. A first fork establishes a missing checkpoint through the composition root; observations never full-materialize the graph.Validation: full pre-push gate, 7,338/2 skipped coverage tests at 92.96% lines, consumer/declaration/reference checks, real-Git fork integration, and 27/28 integration files in the aggregate run plus 7/7 on the resource-timed traversal file in isolation.