Skip to content

Commit 75d7226

Browse files
seefeldbclaude
andcommitted
fix(scheduler): use mightWrite for dependency chain and topological sort
When a lift function returns undefined on first run, no write is recorded in the journal. This broke the dependency chain in pull mode because: 1. updateDependents only looked at actual writes 2. topologicalSort only used actual writes for ordering Fix: Initialize mightWrite from declared writes during subscribe, and use mightWrite (instead of just log.writes) in both updateDependents and topologicalSort. Adds test case that reproduces the nested lift scenario where inner lift initially returns undefined. Fixes counter-when-unless-operators test which has nested lifts where the inner lift initially returns undefined. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 487ef74 commit 75d7226

File tree

2 files changed

+176
-2
lines changed

2 files changed

+176
-2
lines changed

packages/runner/src/scheduler.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,13 @@ export class Scheduler {
690690
const reads = sortAndCompactPaths(log.reads);
691691
const writes = sortAndCompactPaths(log.writes);
692692
this.dependencies.set(action, { reads, writes });
693+
694+
// Initialize mightWrite with declared writes from subscribe
695+
// This ensures dependency chain can be built even before action runs
696+
if (writes.length > 0) {
697+
this.updateMightWrite(action, { reads, writes });
698+
}
699+
693700
return reads;
694701
}
695702

@@ -723,8 +730,12 @@ export class Scheduler {
723730
const otherLog = this.dependencies.get(otherAction);
724731
if (!otherLog) continue;
725732

733+
// Use mightWrite if available (accumulates all paths action has ever written)
734+
// This ensures dependency chain is built even if first run writes undefined
735+
const otherWrites = this.mightWrite.get(otherAction) ?? otherLog.writes;
736+
726737
// Check if otherAction writes to this entity we're reading
727-
for (const write of otherLog.writes) {
738+
for (const write of otherWrites) {
728739
if (
729740
read.space === write.space &&
730741
read.id === write.id &&
@@ -1059,6 +1070,7 @@ export class Scheduler {
10591070
const sorted = topologicalSort(
10601071
new Set(dirtyMembers),
10611072
this.dependencies,
1073+
this.mightWrite,
10621074
this.actionParent,
10631075
);
10641076

@@ -1141,6 +1153,7 @@ export class Scheduler {
11411153
const sorted = topologicalSort(
11421154
new Set(dirtyMembers),
11431155
this.dependencies,
1156+
this.mightWrite,
11441157
this.actionParent,
11451158
);
11461159

@@ -1662,6 +1675,7 @@ export class Scheduler {
16621675
const order = topologicalSort(
16631676
workSet,
16641677
this.dependencies,
1678+
this.mightWrite,
16651679
this.actionParent,
16661680
);
16671681

@@ -1753,6 +1767,7 @@ export class Scheduler {
17531767
function topologicalSort(
17541768
actions: Set<Action>,
17551769
dependencies: WeakMap<Action, ReactivityLog>,
1770+
mightWrite: WeakMap<Action, IMemorySpaceAddress[]>,
17561771
actionParent?: WeakMap<Action, Action>,
17571772
): Action[] {
17581773
const graph = new Map<Action, Set<Action>>();
@@ -1768,7 +1783,8 @@ function topologicalSort(
17681783
for (const actionA of actions) {
17691784
const log = dependencies.get(actionA);
17701785
if (!log) continue;
1771-
const { writes } = log;
1786+
// Use mightWrite if available (includes declared writes even if first run writes undefined)
1787+
const writes = mightWrite.get(actionA) ?? log.writes;
17721788
const graphA = graph.get(actionA)!;
17731789
for (const write of writes) {
17741790
for (const actionB of actions) {

packages/runner/test/scheduler.test.ts

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4357,3 +4357,161 @@ describe("parent-child action ordering", () => {
43574357
expect(childRunCount).toBe(0);
43584358
});
43594359
});
4360+
4361+
describe("pull mode with references", () => {
4362+
let storageManager: ReturnType<typeof StorageManager.emulate>;
4363+
let runtime: Runtime;
4364+
let tx: IExtendedStorageTransaction;
4365+
4366+
beforeEach(() => {
4367+
storageManager = StorageManager.emulate({ as: signer });
4368+
runtime = new Runtime({
4369+
apiUrl: new URL(import.meta.url),
4370+
storageManager,
4371+
});
4372+
runtime.scheduler.enablePullMode();
4373+
tx = runtime.edit();
4374+
});
4375+
4376+
afterEach(async () => {
4377+
await tx.commit();
4378+
await runtime?.dispose();
4379+
await storageManager?.close();
4380+
});
4381+
4382+
it("should propagate dirtiness through references (nested lift scenario)", async () => {
4383+
// This test reproduces the nested lift pattern where:
4384+
// - Inner lift reads source, writes to innerOutput
4385+
// - outerInput cell contains a REFERENCE to innerOutput
4386+
// - Outer lift reads outerInput (following ref to innerOutput), writes to outerOutput
4387+
// - Effect reads outerOutput
4388+
//
4389+
// When source changes:
4390+
// 1. Inner lift is marked dirty
4391+
// 2. Outer lift should be marked dirty because it reads (via reference) what inner writes
4392+
// 3. Effect should run and see updated value
4393+
4394+
const source = runtime.getCell<string[]>(
4395+
space,
4396+
"nested-ref-source",
4397+
undefined,
4398+
tx,
4399+
);
4400+
source.set([]);
4401+
4402+
const innerOutput = runtime.getCell<string | undefined>(
4403+
space,
4404+
"nested-ref-inner-output",
4405+
undefined,
4406+
tx,
4407+
);
4408+
innerOutput.set(undefined);
4409+
4410+
// This cell holds a REFERENCE to innerOutput (simulating how lift passes results)
4411+
const outerInput = runtime.getCell<string | undefined>(
4412+
space,
4413+
"nested-ref-outer-input",
4414+
undefined,
4415+
tx,
4416+
);
4417+
// Set it to be a reference pointing to innerOutput
4418+
outerInput.set(undefined);
4419+
4420+
const outerOutput = runtime.getCell<string>(
4421+
space,
4422+
"nested-ref-outer-output",
4423+
undefined,
4424+
tx,
4425+
);
4426+
outerOutput.set("default");
4427+
4428+
const effectResult = runtime.getCell<string>(
4429+
space,
4430+
"nested-ref-effect-result",
4431+
undefined,
4432+
tx,
4433+
);
4434+
effectResult.set("");
4435+
4436+
await tx.commit();
4437+
tx = runtime.edit();
4438+
4439+
let innerRuns = 0;
4440+
let outerRuns = 0;
4441+
let effectRuns = 0;
4442+
4443+
// Inner lift: arr => arr[0] (returns undefined when array is empty)
4444+
const innerLift: Action = (actionTx) => {
4445+
innerRuns++;
4446+
const arr = source.withTx(actionTx).get() ?? [];
4447+
const firstItem = arr[0]; // Returns undefined when empty!
4448+
innerOutput.withTx(actionTx).send(firstItem);
4449+
};
4450+
4451+
// Outer lift: (name, firstItem) => name || firstItem || "default"
4452+
// For this test, we'll read innerOutput directly to simulate following the reference
4453+
const outerLift: Action = (actionTx) => {
4454+
outerRuns++;
4455+
const firstItem = innerOutput.withTx(actionTx).get();
4456+
const result = firstItem || "default";
4457+
outerOutput.withTx(actionTx).send(result);
4458+
};
4459+
4460+
// Effect: sink that captures the output
4461+
const effect: Action = (actionTx) => {
4462+
effectRuns++;
4463+
const val = outerOutput.withTx(actionTx).get();
4464+
effectResult.withTx(actionTx).send(val ?? "");
4465+
};
4466+
4467+
// Subscribe in order: inner, outer, effect
4468+
runtime.scheduler.subscribe(
4469+
innerLift,
4470+
{
4471+
reads: [source.getAsNormalizedFullLink()],
4472+
writes: [innerOutput.getAsNormalizedFullLink()],
4473+
},
4474+
{ scheduleImmediately: true },
4475+
);
4476+
await runtime.idle();
4477+
4478+
runtime.scheduler.subscribe(
4479+
outerLift,
4480+
{
4481+
reads: [innerOutput.getAsNormalizedFullLink()],
4482+
writes: [outerOutput.getAsNormalizedFullLink()],
4483+
},
4484+
{ scheduleImmediately: true },
4485+
);
4486+
await runtime.idle();
4487+
4488+
runtime.scheduler.subscribe(
4489+
effect,
4490+
{
4491+
reads: [outerOutput.getAsNormalizedFullLink()],
4492+
writes: [effectResult.getAsNormalizedFullLink()],
4493+
},
4494+
{ scheduleImmediately: true, isEffect: true },
4495+
);
4496+
await runtime.idle();
4497+
4498+
// Initial state: source is [], innerOutput is undefined, outerOutput is "default"
4499+
expect(innerRuns).toBe(1);
4500+
expect(outerRuns).toBe(1);
4501+
expect(effectRuns).toBe(1);
4502+
expect(effectResult.get()).toBe("default");
4503+
4504+
// Now change source to ["apple"]
4505+
source.withTx(tx).send(["apple"]);
4506+
await tx.commit();
4507+
tx = runtime.edit();
4508+
await runtime.idle();
4509+
4510+
// With fix: All should run because dependency chain is now properly built
4511+
// (mightWrite preserves declared writes, enabling correct topological ordering)
4512+
expect(innerRuns).toBe(2);
4513+
expect(outerRuns).toBe(2);
4514+
expect(effectRuns).toBe(2);
4515+
expect(effectResult.get()).toBe("apple");
4516+
});
4517+
});

0 commit comments

Comments
 (0)