〰️ Split Output into Output[]

packages/myst-cli/src/process/notebook.ts

@@ -7,7 +7,7 @@
   ICell,
   IMimeBundle,
   INotebookContent,
   INotebookMetadata,
   IOutput,
   MultilineString,
 } from '@jupyterlab/nbformat';
@@ -104,8 +104,8 @@
     ...PAGE_FRONTMATTER_KEYS,
     // Include aliased entries for page frontmatter keys
     ...Object.entries(FRONTMATTER_ALIASES)
       .filter(([_, value]) => PAGE_FRONTMATTER_KEYS.includes(value))
       .map(([key, _]) => key),
   ]);
   const frontmatter = validatePageFrontmatter(
     filteredMetadata ?? {},
@@ -165,17 +165,16 @@
           value: ensureString(cell.source),
         };
 
-        // Embed outputs in an output block
-        const output: { type: 'output'; id: string; data: IOutput[] } = {
-          type: 'output',
-          id: nanoid(),
-          data: [],
-        };
-
-        if (cell.outputs && (cell.outputs as IOutput[]).length > 0) {
-          output.data = cell.outputs as IOutput[];
-        }
-        return acc.concat(blockParent(cell, [code, output]));
+        const outputs = (cell.outputs as IOutput[]).map((output) => {
+          // TODO: output-refactoring -- embed this in an `outputs node` in future
+          const result: { type: 'output'; id: string; data: IOutput[] } = {
+            type: 'output',
+            id: nanoid(),
+            data: [output],
+          };
+          return result;
+        });
+        return acc.concat(blockParent(cell, [code, ...outputs]));
       }
       return acc;
     },

packages/myst-cli/src/transforms/crossReferences.ts

@@ -1,14 +1,16 @@
 import type { VFile } from 'vfile';
 import { selectAll } from 'unist-util-select';
+import { visit, SKIP } from 'unist-util-visit';
 import type { FrontmatterParts, GenericNode, GenericParent, References } from 'myst-common';
-import { RuleId, fileWarn, plural, selectMdastNodes } from 'myst-common';
+import { RuleId, fileWarn, plural, selectMdastNodes, liftChildren } from 'myst-common';
 import { computeHash, tic } from 'myst-cli-utils';
 import { addChildrenFromTargetNode } from 'myst-transforms';
 import type { PageFrontmatter } from 'myst-frontmatter';
 import type { CrossReference, Dependency, Link, SourceFileKind } from 'myst-spec-ext';
 import type { ISession } from '../session/types.js';
 import { loadFromCache, writeToCache } from '../session/cache.js';
 import type { SiteAction, SiteExport } from 'myst-config';
+import type { IOutput } from '@jupyterlab/nbformat';
 
 export const XREF_MAX_AGE = 1; // in days
 
@@ -32,6 +34,104 @@ export type MystData = {
   references?: References;
 };
 
+/**
+ * Convert between MyST AST versions either side of the `output` refactoring work
+ *
+ * "past" AST is upgraded to "contemporary" AST.
+ * "future" AST is downgraded to "contemporary AST".
+ *
+ * where "contemporary` AST is immediately after #1661 merges"
+ *
+ * These two changes allow us to continue to publish AST that will mostly work with old mystmd/myst-theme deployments, whilst being ready for a future breaking change that we can anticipate.
+ *
+ * After these upgrades/downgrades, we ensure that we have the following pseudo-schema:
+ *
+ * type CodeBlock = {
+ *   type: "block";
+ *   kind: "notebook-code",
+ *   children: [
+ *     Code,
+ *     Output,
+ *     ...,
+ *     Output
+ *   ]
+ * }
+ * type Output = {
+ *   type: "output";
+ *   children: GenericNode[];
+ *   visibility: ...;
+ *   data: IOutput[1];
+ * }
+ *
+ */
+function upgradeAndDowngradeMystData(data: MystData): MystData {
+  const makeUniqueLabel = (label: string | undefined, index: number): string | undefined => {
+    if (label === undefined) {
+      return undefined;
+    }
+    if (index === 0) {
+      return label;
+    } else {
+      return `${label}_${index}`;
+    }
+  };
+
+  // TODO: output-refactoring -- rewrite this function
+  visit(
+    data.mdast as any,
+    'output',
+    (node: GenericNode, index: number | null, parent: GenericParent | null) => {
+      // Case 1: "old" schema with >1 data per Output
+      // Upgrade old schema to have >1 data per output
+      if (parent && node.data && node.data.length > 1) {
+        const outputs = node.data.map((outputData: IOutput, idx: number) => {
+          // Take the unique ID from the first node
+          const auxData = {
+            identifier: makeUniqueLabel(node.identifier, idx),
+            html_id: makeUniqueLabel(node.html_id, idx),
+            id: makeUniqueLabel(node.id, idx),
+          };
+          return {
+            type: 'output',
+            visibility: node.visibility,
+            data: [outputData],
+            children: [], // FIXME: ignoring children here
+            ...auxData,
+          };
+        });
+        parent.children[index!] = outputs;
+        return SKIP;
+      }
+      // Case 2: "future" AST
+      // 1. delete new `jupyter_output` field of `Output`
+      // 2. restore `Output.data`
+      // 3. duplicate `Outputs.visibility` to `Output`, with `Output` taking precedence
+      // 4. erase `Outputs` type
+      else if (parent && parent.type === 'outputs' && 'jupyter_output' in node) {
+        // Erase the `Outputs` node
+        parent.type = '__lift__';
+
+        // Downgrade `jupyter_output` (1) and (2)
+        node.data = [node.jupyter_output];
+        node.jupyter_output = undefined;
+
+        // Duplicate `visibility` onto `Output` children (3)
+        node.visibility = node.visibility ?? parent.visibility;
+
+        // Take unique ID from parent
+        if (index === 0) {
+          node.identifier = parent.identifier;
+          node.html_id = parent.html_id;
+        }
+      }
+    },
+  );
+
+  // Erase lifted outputs
+  liftChildren(data.mdast as any, '__lift__');
+  return data;
+}
+
 async function fetchMystData(
   session: ISession,
   dataUrl: string | undefined,
@@ -48,7 +148,8 @@ async function fetchMystData(
     try {
       const resp = await session.fetch(dataUrl);
       if (resp.ok) {
-        const data = (await resp.json()) as MystData;
+        const data = upgradeAndDowngradeMystData((await resp.json()) as MystData);
+
         writeToCache(session, filename, JSON.stringify(data));
         return data;
       }

packages/myst-cli/src/transforms/outputs.ts

@@ -41,6 +41,7 @@ export async function transformOutputsToCache(
     outputs
       .filter((output) => output.visibility !== 'remove')
       .map(async (output) => {
+        // TODO: output-refactoring -- drop to single output in future
         output.data = await minifyCellOutput(output.data as IOutput[], cache.$outputs, {
           computeHash,
           maxCharacters: opts?.minifyMaxCharacters,
@@ -77,6 +78,7 @@ export function transformFilterOutputStreams(
     const outputs = selectAll('output', block) as GenericNode[];
     // There should be only one output in the block
     outputs.forEach((output) => {
+      // TODO: output-refactoring -- drop to single output in future
       output.data = output.data.filter((data: IStream | MinifiedMimeOutput) => {
         if (
           (stderr !== 'show' || blockRemoveStderr) &&
@@ -193,6 +195,7 @@ export function transformOutputsToFile(
   const cache = castSession(session);
 
   outputs.forEach((node) => {
+    // TODO: output-refactoring -- drop to single output in future
     walkOutputs(node.data, (obj) => {
       const { hash } = obj;
       if (!hash || !cache.$outputs[hash]) return undefined;
@@ -236,13 +239,15 @@ export function reduceOutputs(
   const outputs = selectAll('output', mdast) as GenericNode[];
   const cache = castSession(session);
   outputs.forEach((node) => {
+    // TODO: output-refactoring -- drop to single output in future
     if (!node.data?.length && !node.children?.length) {
       node.type = '__delete__';
       return;
     }
     node.type = '__lift__';
     if (node.children?.length) return;
     const selectedOutputs: { content_type: string; hash: string }[] = [];
+    // TODO: output-refactoring -- drop to single output in future
     node.data.forEach((output: MinifiedOutput) => {
       let selectedOutput: { content_type: string; hash: string } | undefined;
       walkOutputs([output], (obj: any) => {

packages/myst-directives/src/code.ts

@@ -212,7 +212,7 @@ export const codeCellDirective: DirectiveSpec = {
     };
     const output = {
       type: 'output',
-      id: nanoid(),
+      children: [],
       data: [],
     };
     const block: GenericNode = {

packages/myst-execute/package.json

@@ -37,6 +37,7 @@
     "myst-cli-utils": "^2.0.11",
     "myst-common": "^1.7.2",
     "node-fetch": "^3.3.0",
+    "unist-util-remove": "^3.1.1",
     "unist-util-select": "^4.0.3",
     "vfile": "^5.3.7",
     "which": "^4.0.0"

packages/myst-execute/src/execute.ts

@@ -1,4 +1,5 @@
 import { select, selectAll } from 'unist-util-select';
+import { remove } from 'unist-util-remove';
 import type { Logger } from 'myst-cli-utils';
 import type { PageFrontmatter, KernelSpec } from 'myst-frontmatter';
 import type { Kernel, KernelMessage, Session, SessionManager } from '@jupyterlab/services';
@@ -282,10 +283,19 @@ function applyComputedOutputsToNodes(
     const thisResult = computedResult.shift();
 
     if (isCellBlock(matchedNode)) {
-      // Pull out output to set data
-      const output = select('output', matchedNode) as unknown as { data: IOutput[] };
-      // Set the output array to empty if we don't have a result (e.g. due to a kernel error)
-      output.data = thisResult === undefined ? [] : (thisResult as IOutput[]);
+      // Pull out code node
+      const code = select('code', matchedNode);
+
+      // Remove outputs
+      remove(matchedNode, { cascade: false }, 'output');
+
+      // Generate outputs
+      const outputs = ((thisResult as IOutput[]) ?? []).map((data) => {
+        return { type: 'output', children: [], data: [data] as any };
+      });
+      // Ensure that whether this fails or succeeds, we write to `children` (e.g. due to a kernel error)
+      // TODO: output-refactoring -- contain these nodes in `outputs`
+      matchedNode.children = [code as any, ...outputs];
     } else if (isInlineExpression(matchedNode)) {
       // Set data of expression to the result, or empty if we don't have one
       matchedNode.result = // TODO: FIXME .data

packages/myst-execute/tests/execute.yml

@@ -67,9 +67,6 @@ cases:
                   enumerator: 1
                   html_id: nb-cell-0-code
                 - type: output
-                  id: T7FMDqDm8dM2bOT1tKeeM
-                  identifier: nb-cell-0-output
-                  html_id: nb-cell-0-output
                   data:
                     - output_type: stream
                       name: stdout
@@ -171,9 +168,6 @@ cases:
               enumerator: 1
               html_id: nb-cell-0-code
             - type: output
-              id: T7FMDqDm8dM2bOT1tKeeM
-              identifier: nb-cell-0-output
-              html_id: nb-cell-0-output
               data:
                 - output_type: stream
                   name: stdout
@@ -195,9 +189,6 @@ cases:
               enumerator: 1
               html_id: nb-cell-0-code
             - type: output
-              id: T7FMDqDm8dM2bOT1tKeeM
-              identifier: nb-cell-0-output
-              html_id: nb-cell-0-output
               data:
                 - output_type: error
                   # Note this traceback can be different on various machines
@@ -254,9 +245,6 @@ cases:
               enumerator: 1
               html_id: nb-cell-0-code
             - type: output
-              id: T7FMDqDm8dM2bOT1tKeeM
-              identifier: nb-cell-0-output
-              html_id: nb-cell-0-output
               data:
                 - output_type: error
                   # Note this traceback can be different on various machines
@@ -313,7 +301,4 @@ cases:
               enumerator: 1
               html_id: nb-cell-0-code
             - type: output
-              id: T7FMDqDm8dM2bOT1tKeeM
-              identifier: nb-cell-0-output
-              html_id: nb-cell-0-output
               data:

packages/myst-execute/tests/run.spec.ts

@@ -96,6 +96,7 @@ casesList.forEach(({ title, cases }) => {
             expect.arrayContaining([expect.stringMatching(throws)]),
           );
         }
+        console.log(JSON.stringify(after, null, 2));
         expect(before).toMatchObject(after);
       },
       { timeout: 30_000 },