Skip to content

Commit cf0681a

Browse files
committed
feat(runner): Ensure frame.space is always set and add inHandler flag
Audit and fix frame creation to ensure space is always available from the result cell context: 1. Modified pushFrameFromCause() to: - Extract space from unsafe_binding and set on frame.space - Accept inHandler boolean parameter (replaces event field) - This makes frame.space available as fallback in cell.ts 2. Renamed Frame.event to Frame.inHandler: - More accurate semantic: indicates handler context, not event data - Used for per-frame counter-based ID generation fallback - Simpler boolean check vs checking event truthiness 3. Updated CauseContainer to store space: - Container now holds id, space, and cause for sharing across siblings - ensureLink() checks if container has both id and space before deriving - Space from container takes precedence over frame space 4. Updated event handler in runner.ts: - Pass inHandler: true to pushFrameFromCause() - Frame now has both space (from processCell.space) and inHandler flag Benefits: - frame.space fallback in cell.ts now works correctly - clearer semantics with inHandler vs event - space always available from result cell in runner contexts - CauseContainer properly shares space across siblings
1 parent 787d887 commit cf0681a

File tree

6 files changed

+37
-22
lines changed

6 files changed

+37
-22
lines changed

docs/specs/recipe-construction/rollout-plan.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
isn't there, e.g. because we need to create a link to the cell (when passed
4343
into `anotherCell.set()` for example). We want to encourage .for use in
4444
ambiguous cases.
45-
- [ ] Add space and event to Frame
45+
- [x] Add space and event to Frame
4646
- [ ] First merge of OpaqueRef and RegularCell
4747
- [ ] Add methods that allow linking to node invocations
4848
- [ ] `setPreExisting` can be deprecated (used in toOpaqueRef which itself

packages/runner/src/builder/recipe.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,13 +399,17 @@ export function pushFrame(frame?: Frame): Frame {
399399
export function pushFrameFromCause(
400400
cause: any,
401401
unsafe_binding?: UnsafeBinding,
402+
inHandler: boolean = false,
402403
): Frame {
403404
const frame = {
404405
parent: getTopFrame(),
405406
cause,
406407
generatedIdCounter: 0,
407408
opaqueRefs: new Set(),
409+
// Extract space from unsafe_binding if available and set it on frame
410+
...(unsafe_binding?.space && { space: unsafe_binding.space }),
408411
...(unsafe_binding ? { unsafe_binding } : {}),
412+
...(inHandler && { inHandler }),
409413
};
410414
frames.push(frame);
411415
return frame;

packages/runner/src/builder/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ export type Frame = {
260260
cause?: unknown;
261261
generatedIdCounter: number;
262262
space?: MemorySpace;
263-
event?: unknown;
263+
inHandler?: boolean;
264264
opaqueRefs: Set<OpaqueRef<any>>;
265265
unsafe_binding?: UnsafeBinding;
266266
};

packages/runner/src/cell.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ export class CellImpl<T> implements ICell<T>, IStreamable<T> {
278278
} else {
279279
// Fail by default
280280
throw new Error(
281-
"Cannot set cause: cell already has a cause or link. Pass true as second parameter to allow this as a suggestion.",
281+
"Cannot set cause: cell already has a cause or link.",
282282
);
283283
}
284284
}
@@ -301,14 +301,22 @@ export class CellImpl<T> implements ICell<T>, IStreamable<T> {
301301
* @throws Error if not in a handler context and no cause was provided
302302
*/
303303
private ensureLink(): void {
304-
// If we already have a full link (id and space), nothing to do
305-
if (this._causeContainer.id && this._link.space) {
304+
// If we already have a full link (id and space) in the container, just copy
305+
// it over to our link.
306+
if (this._causeContainer.id && this._causeContainer.space) {
307+
this._link = {
308+
...this._link,
309+
id: this._causeContainer.id,
310+
space: this._causeContainer.space,
311+
};
306312
return;
307313
}
308314

309-
// Check if we're in a handler context
315+
// Otherwise, let's attempt to derive the id:
316+
310317
const frame = getTopFrame();
311318

319+
// We must be in a frame context to derive the id.
312320
if (!frame) {
313321
throw new Error(
314322
"Cannot create cell link: no frame context.\n" +
@@ -329,9 +337,11 @@ export class CellImpl<T> implements ICell<T>, IStreamable<T> {
329337
);
330338
}
331339

340+
// Used passed in cause (via .for()), for events fall back to per-frame
341+
// counter.
332342
const cause = this._causeContainer.cause ??
333-
(frame.event ? { count: frame.generatedIdCounter++ } : undefined);
334-
// TODO(seefeld): Implement no-cause-but-in-handler case
343+
(frame.inHandler ? { count: frame.generatedIdCounter++ } : undefined);
344+
335345
if (!cause) {
336346
throw new Error(
337347
"Cannot create cell link: not in a handler context and no cause was provided.\n" +
@@ -342,19 +352,16 @@ export class CellImpl<T> implements ICell<T>, IStreamable<T> {
342352
);
343353
}
344354

345-
// Create an entity ID from the cause
346-
// Include frame.cause in the source for determinism
355+
// Create an entity ID from the cause, including the frame's
347356
const id = toURI(createRef({ frame: cause }, cause));
348357

349358
// Populate the id in the shared causeContainer
350359
// All siblings will see this update
351360
this._causeContainer.id = id;
352361
this._causeContainer.space = space;
353362

354-
// Update this cell's link with the space if it doesn't have one
355-
if (!this._link.space) {
356-
this._link = { ...this._link, id, space };
357-
}
363+
// Update this cell's link
364+
this._link = { ...this._link, id, space };
358365
}
359366

360367
get space(): MemorySpace {

packages/runner/src/runner.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -969,13 +969,17 @@ export class Runner implements IRunner {
969969
tx,
970970
);
971971

972-
const frame = pushFrameFromCause(cause, {
973-
recipe,
974-
materialize: (path: readonly PropertyKey[]) =>
975-
processCell.getAsQueryResult(path),
976-
space: processCell.space,
977-
tx,
978-
});
972+
const frame = pushFrameFromCause(
973+
cause,
974+
{
975+
recipe,
976+
materialize: (path: readonly PropertyKey[]) =>
977+
processCell.getAsQueryResult(path),
978+
space: processCell.space,
979+
tx,
980+
},
981+
true, // Set the event on the frame
982+
);
979983

980984
const argument = module.argumentSchema
981985
? inputsCell.asSchema(module.argumentSchema).get()

packages/runner/test/cell-optional-link.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ describe("Cell with Optional Link", () => {
209209
pushFrame({
210210
cause: { type: "handler-cause" },
211211
space,
212-
event: "test-event",
212+
inHandler: true,
213213
generatedIdCounter: 0,
214214
opaqueRefs: new Set(),
215215
});

0 commit comments

Comments
 (0)