Run queries returned by lift (#255)

* add stream() helper
* run handler recipes that are more than single opaque refs
* remove debugger calls
* apply closure alias bumps to nested recipes
* documented closure unwrapping a bit better
* json serialize non-id cells as {"/": ""}, mostly for debugging
* fix import in test
* fix references with path when returning a recipe from a node
* remove extra logging
* lifted functions can now return recipes and they'll get executed!
* added test to make sure we don't regenerate recipe when not necessary

Author: seefeldb

typescript/packages/common-builder/src/index.ts

@@ -1,4 +1,4 @@
-export { opaqueRef as cell } from "./opaque-ref.js";
+export { opaqueRef as cell, stream } from "./opaque-ref.js";
 export {
   createNodeFactory,
   derive,

typescript/packages/common-builder/src/opaque-ref.ts

@@ -137,6 +137,10 @@ export function opaqueRef<T>(value?: Opaque<T> | T): OpaqueRef<T> {
   return top;
 }
 
+export function stream<T>(): OpaqueRef<T> {
+  return opaqueRef<T>({ $stream: true } as T);
+}
+
 export function createShadowRef(ref: OpaqueRef<any>): ShadowRef {
   return { shadowOf: ref };
 }

typescript/packages/common-runner/src/cell.ts

@@ -376,7 +376,7 @@ export function cell<T>(value?: T, cause?: any): CellImpl<T> {
     toJSON: () =>
       typeof entityId?.toJSON === "function"
         ? entityId.toJSON()
-        : (entityId as { "/": string }),
+        : (entityId as { "/": string }) ?? { "/": "" },
     get value(): T {
       return value as T;
     },
@@ -772,7 +772,6 @@ export function createQueryResultProxy<T>(
         };
         ref.cell.send(value);
         ref.cell.sourceCell = valueCell;
-        if (Array.isArray(valueCell.get())) debugger;
 
         log?.writes.push(ref);
 
@@ -835,7 +834,6 @@ function makeOpaqueRef(
   let ref = opaqueRefs.find((p) => arrayEqual(valuePath, p.path))?.opaqueRef;
   if (!ref) {
     ref = opaqueRef();
-    for (const key of valuePath) ref = ref.key(key);
     ref.setPreExisting({ $alias: { cell: valueCell, path: valuePath } });
     opaqueRefs.push({ path: valuePath, opaqueRef: ref });
   }

typescript/packages/common-runner/src/runner.ts

@@ -8,7 +8,6 @@ import {
   isModule,
   isRecipe,
   isAlias,
-  isOpaqueRef,
   isStreamAlias,
   popFrame,
   recipeFromFrame,
@@ -28,14 +27,15 @@ import {
 import { Action, schedule, addEventHandler } from "./scheduler.js";
 import {
   extractDefaultValues,
-  mapBindingsToCell,
+  unwrapOneLevelAndBindtoCell,
   findAllAliasedCells,
   followAliases,
   mergeObjects,
   sendValueToBinding,
   staticDataToNestedCells,
   deepCopy,
   unsafe_noteParentOnRecipes,
+  containsOpaqueRef,
 } from "./utils.js";
 import { getModuleByRef } from "./module.js";
 import { type AddCancel, type Cancel, useCancelGroup } from "./cancel.js";
@@ -180,7 +180,9 @@ export function run<T, R = any>(
   });
 
   // Send "query" to results to the result cell
-  resultCell.send(mapBindingsToCell<R>(recipe.result as R, processCell));
+  resultCell.send(
+    unwrapOneLevelAndBindtoCell<R>(recipe.result as R, processCell),
+  );
 
   // [unsafe closures:] For recipes from closures, add a materialize factory
   if (recipe[unsafe_originalRecipe])
@@ -308,7 +310,7 @@ function instantiateJavaScriptNode(
   addCancel: AddCancel,
   recipe: Recipe,
 ) {
-  const inputs = mapBindingsToCell(
+  const inputs = unwrapOneLevelAndBindtoCell(
     inputBindings as { [key: string]: any },
     processCell,
   );
@@ -318,7 +320,7 @@ function instantiateJavaScriptNode(
   // written.
   const reads = findAllAliasedCells(inputs, processCell);
 
-  const outputs = mapBindingsToCell(outputBindings, processCell);
+  const outputs = unwrapOneLevelAndBindtoCell(outputBindings, processCell);
   const writes = findAllAliasedCells(outputs, processCell);
 
   let fn = (
@@ -383,21 +385,14 @@ function instantiateJavaScriptNode(
       const result = fn(inputsCell.getAsQueryResult([]));
 
       // If handler returns a graph created by builder, run it
-      // TODO: Handle case where the result is a structure with possibly
-      // multiple such nodes
-      if (isOpaqueRef(result)) {
-        const resultNode = result;
-
-        // Recipe that assigns the result of the returned node to "result"
+      if (containsOpaqueRef(result)) {
         const resultRecipe = recipeFromFrame(
           "event handler result",
           undefined,
-          () => ({
-            result: resultNode,
-          }),
+          () => result,
         );
 
-        const resultCell = run(resultRecipe, {});
+        const resultCell = run(resultRecipe);
         addCancel(cancels.get(resultCell));
       }
 
@@ -411,6 +406,8 @@ function instantiateJavaScriptNode(
     const inputsCell = cell(inputs);
     inputsCell.freeze(); // Freezes the bindings, not aliased cells.
 
+    let resultCell: CellImpl<any> | undefined;
+
     const action: Action = (log: ReactivityLog) => {
       const inputsProxy = inputsCell.getAsQueryResult([], log);
 
@@ -420,9 +417,28 @@ function instantiateJavaScriptNode(
           processCell.getAsQueryResult(path, log),
       } satisfies UnsafeBinding);
       const result = fn(inputsProxy);
-      popFrame(frame);
 
-      sendValueToBinding(processCell, outputs, result, log);
+      if (containsOpaqueRef(result)) {
+        const resultRecipe = recipeFromFrame(
+          "action result",
+          undefined,
+          () => result,
+        );
+
+        resultCell = run(resultRecipe, undefined, resultCell);
+        addCancel(cancels.get(resultCell));
+
+        sendValueToBinding(
+          processCell,
+          outputs,
+          { cell: resultCell, path: [] },
+          log,
+        );
+      } else {
+        sendValueToBinding(processCell, outputs, result, log);
+      }
+
+      popFrame(frame);
     };
 
     addCancel(schedule(action, { reads, writes } satisfies ReactivityLog));
@@ -445,8 +461,14 @@ function instantiateRawNode(
   // Built-ins can define their own scheduling logic, so they'll
   // implement parts of the above themselves.
 
-  const mappedInputBindings = mapBindingsToCell(inputBindings, processCell);
-  const mappedOutputBindings = mapBindingsToCell(outputBindings, processCell);
+  const mappedInputBindings = unwrapOneLevelAndBindtoCell(
+    inputBindings,
+    processCell,
+  );
+  const mappedOutputBindings = unwrapOneLevelAndBindtoCell(
+    outputBindings,
+    processCell,
+  );
 
   // For `map` and future other node types that take closures, we need to
   // note the parent recipe on the closure recipes.
@@ -474,11 +496,11 @@ function instantiatePassthroughNode(
   processCell: CellImpl<any>,
   addCancel: AddCancel,
 ) {
-  const inputs = mapBindingsToCell(inputBindings, processCell);
+  const inputs = unwrapOneLevelAndBindtoCell(inputBindings, processCell);
   const inputsCell = cell(inputs);
   const reads = findAllAliasedCells(inputs, processCell);
 
-  const outputs = mapBindingsToCell(outputBindings, processCell);
+  const outputs = unwrapOneLevelAndBindtoCell(outputBindings, processCell);
   const writes = findAllAliasedCells(outputs, processCell);
 
   const action: Action = (log: ReactivityLog) => {
@@ -496,12 +518,12 @@ function instantiateIsolatedNode(
   processCell: CellImpl<any>,
   addCancel: AddCancel,
 ) {
-  const inputs = mapBindingsToCell(inputBindings, processCell);
+  const inputs = unwrapOneLevelAndBindtoCell(inputBindings, processCell);
   const reads = findAllAliasedCells(inputs, processCell);
   const inputsCell = cell(inputs);
   inputsCell.freeze();
 
-  const outputs = mapBindingsToCell(outputBindings, processCell);
+  const outputs = unwrapOneLevelAndBindtoCell(outputBindings, processCell);
   const writes = findAllAliasedCells(outputs, processCell);
 
   if (!isJavaScriptModuleDefinition(module.implementation))
@@ -561,14 +583,18 @@ function instantiateRecipeNode(
   addCancel: AddCancel,
 ) {
   if (!isRecipe(module.implementation)) throw new Error(`Invalid recipe`);
-  const inputs = mapBindingsToCell(inputBindings, processCell);
+  const recipe = unwrapOneLevelAndBindtoCell(
+    module.implementation,
+    processCell,
+  );
+  const inputs = unwrapOneLevelAndBindtoCell(inputBindings, processCell);
   const resultCell = cell(undefined, {
     recipe: module.implementation,
     parent: processCell,
     inputBindings,
     outputBindings,
   });
-  run(module.implementation, inputs, resultCell);
+  run(recipe, inputs, resultCell);
   sendValueToBinding(processCell, outputBindings, {
     cell: resultCell,
     path: [],

typescript/packages/common-runner/src/utils.ts

@@ -7,6 +7,7 @@ import {
   Recipe,
   UnsafeBinding,
   unsafe_materializeFactory,
+  isOpaqueRef,
 } from "@commontools/common-builder";
 import {
   cell,
@@ -171,24 +172,50 @@ export function setNestedValue(
   return true;
 }
 
-// Turn local aliases into explicit aliases to named cell.
-export function mapBindingsToCell<T>(binding: T, cell: CellImpl<any>): T {
+/**
+ * Unwraps one level of aliases, and
+ * - binds top-level aliases to passed cell
+ * - reduces wrapping count of closure cells by one
+ *
+ * This is used for arguments to nodes (which can be recipes, e.g. for map) and
+ * for the recipe in recipe nodes.
+ *
+ * An alias will go through these stages:
+ * - { $alias: { cell: 1, path: ["a"] } }
+ *   = Nested two layers deep, an argment for a nested recipe
+ * - { $alias: { path: ["a"] } }
+ *   = One layer deep, e.g. a recipe that will be passed to `run`
+ * - { $alias: { cell: <cell>, path: ["a"] } }
+ *   = Unwrapped, executing the recipe
+ *
+ * @param binding - The binding to unwrap.
+ * @param cell - The cell to bind to.
+ * @returns The unwrapped binding.
+ */
+export function unwrapOneLevelAndBindtoCell<T>(
+  binding: T,
+  cell: CellImpl<any>,
+): T {
   function convert(binding: any, processStatic = false): any {
     if (isStatic(binding) && !processStatic)
       return markAsStatic(convert(binding, true));
     else if (isAlias(binding)) {
       if (typeof binding.$alias.cell === "number")
         if (binding.$alias.cell === 1)
+          // Moved to the next-to-top level. Don't assign a cell, so that on
+          // next unwrap, the right cell be assigned.
           return { $alias: { path: binding.$alias.path } };
         else
           return {
+            // Otherwise decrease count by one
             $alias: {
               cell: binding.$alias.cell - 1,
               path: binding.$alias.path,
             },
           };
       else
         return {
+          // Bind to passed cell, if there isn't already one
           $alias: {
             cell: binding.$alias.cell ?? cell,
             path: binding.$alias.path,
@@ -494,7 +521,6 @@ export function normalizeToCells(
           value[i] = { cell: cell(value[i]), path: [] } satisfies CellReference;
           value[i].cell.entityId = itemId;
           value[i].cell.sourceCell = parentCell;
-          if (Array.isArray(parentCell.get())) debugger;
 
           preceedingItemId = itemId;
           log?.writes.push(value[i]);
@@ -560,6 +586,13 @@ export function isEqualCellReferences(
   );
 }
 
+export function containsOpaqueRef(value: any): boolean {
+  if (isOpaqueRef(value)) return true;
+  if (typeof value === "object" && value !== null)
+    return Object.values(value).some(containsOpaqueRef);
+  return false;
+}
+
 export function deepCopy(value: any): any {
   if (isCell(value) || isRendererCell(value)) return value;
   if (typeof value === "object" && value !== null)

typescript/packages/common-runner/test/recipes.test.ts

@@ -15,7 +15,6 @@ describe("Recipe Runner", () => {
       },
     );
 
-    console.log("simpleRecipe", JSON.stringify(simpleRecipe.toJSON(), null, 2));
     const result = run(simpleRecipe, { value: 5 });
 
     await idle();
@@ -198,11 +197,79 @@ describe("Recipe Runner", () => {
     await idle();
     expect(values).toEqual([
       [1, 1, 0],
-      [3, 1, 0], // That's the first logger called again when counter changes
+      // Next is the first logger called again when counter changes, since this
+      // is now a long running charmlet:
+      [3, 1, 0],
       [3, 2, 0],
     ]);
   });
 
+  it("should handle recipes returned by lifted functions", async () => {
+    const x = cell(2);
+    const y = cell(3);
+
+    const runCounts = {
+      multiply: 0,
+      multiplyGenerator: 0,
+      multiplyGenerator2: 0,
+    };
+
+    const multiply = lift<{ x: number; y: number }>(({ x, y }) => {
+      runCounts.multiply++;
+      return x * y;
+    });
+
+    const multiplyGenerator = lift<{ x: number; y: number }>((args) => {
+      runCounts.multiplyGenerator++;
+      return multiply(args);
+    });
+
+    const multiplyGenerator2 = lift<{ x: number; y: number }>(({ x, y }) => {
+      runCounts.multiplyGenerator2++;
+      // Now passing literals, so will hardcode values in recipe and hence
+      // re-run when values change
+      return multiply({ x, y });
+    });
+
+    const multiplyRecipe = recipe<{ x: number; y: number }>(
+      "multiply",
+      (args) => {
+        return {
+          result1: multiplyGenerator(args),
+          result2: multiplyGenerator2(args),
+        };
+      },
+    );
+
+    const result = run(multiplyRecipe, { x, y });
+
+    await idle();
+
+    expect(result.getAsQueryResult()).toMatchObject({
+      result1: 6,
+      result2: 6,
+    });
+
+    expect(runCounts).toMatchObject({
+      multiply: 2,
+      multiplyGenerator: 1,
+      multiplyGenerator2: 1,
+    });
+
+    x.send(3);
+    await idle();
+    expect(result.getAsQueryResult()).toMatchObject({
+      result1: 9,
+      result2: 9,
+    });
+
+    expect(runCounts).toMatchObject({
+      multiply: 4,
+      multiplyGenerator: 1, // Did not re-run, since we didn't read the values!
+      multiplyGenerator2: 2,
+    });
+  });
+
   it("should support referenced modules", async () => {
     addModuleByRef(
       "double",