Contain a failed node-view chunk, and stop exporting its loading shell

web_ui/src/app/canvas/exportCanvasPng.ts

@@ -132,12 +132,24 @@ function nextPaint(): Promise<void> {
   });
 }
 
+/** Every Suspense fallback that means "this node has not rendered yet".
+ * Kept as one constant so a third fallback cannot be added without a reader
+ * of the export path seeing it. */
+export const NODE_LOADING_SELECTOR = ".chart-node-placeholder, .scene-node-loading";
+
 function sleep(ms: number): Promise<void> {
   return new Promise((resolve) => setTimeout(resolve, ms));
 }
 
-/** Polls `root` for any still-loading chart node (ChartNodeView.tsx's
- * `.chart-node-placeholder` Suspense fallback) and resolves once none remain.
+/** Polls `root` for any still-loading node and resolves once none remain.
+ *
+ * TWO fallbacks qualify. `.chart-node-placeholder` is ChartNodeView.tsx's own,
+ * and was the only one this waited on. ADR-019's node-view code split added
+ * `.scene-node-loading` (lazyNodeViews.tsx), which is shown while ANY node
+ * kind's chunk loads - and export is precisely when the most chunks are
+ * unloaded, because exportCanvasAsPng disables onlyRenderVisibleElements to
+ * mount the off-viewport nodes, which are exactly the ones whose chunks were
+ * never fetched. Without this the PNG captures "Loading..." shells.
  * Bounded by `timeoutMs` so a genuinely stuck or failed chunk load (offline,
  * a bad deploy) degrades to "capture whatever is there" rather than hanging
  * the export indefinitely - consistent with this module's broader "never
@@ -155,7 +167,7 @@ export async function waitForChartPlaceholdersToClear(
   pollIntervalMs: number = CHART_CHUNK_POLL_INTERVAL_MS,
 ): Promise<void> {
   const deadline = Date.now() + timeoutMs;
-  while (root.querySelector(".chart-node-placeholder") && Date.now() < deadline) {
+  while (root.querySelector(NODE_LOADING_SELECTOR) && Date.now() < deadline) {
     await sleep(pollIntervalMs);
   }
 }

web_ui/src/app/canvas/lazyNodeViews.test.tsx

@@ -0,0 +1,103 @@
+/**
+ * ADR-019: the node views load one chunk per kind. These tests cover the two
+ * ways that can go wrong at runtime, neither of which the split shipped with.
+ *
+ * The failure mode that matters: React.lazy throws a PROMISE, which Suspense
+ * catches - but when that promise REJECTS, Suspense has nothing to do with it
+ * and the rejection propagates uncaught. React then unmounts the whole tree.
+ * One node kind failing to load (a stale asset index after a deploy, a dropped
+ * connection mid-session) would blank the entire canvas and the app chrome
+ * with it. Measured before the boundary existed: the rejection surfaced as an
+ * unhandled error.
+ */
+
+import { lazy, type ComponentType } from "react";
+import { render, screen, waitFor } from "@testing-library/react";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+import * as lazyNodeViews from "./lazyNodeViews";
+
+// The real module's own wrapper shape, applied to controllable chunks. Kept
+// structurally identical to withNodeSuspense so a change there that drops the
+// boundary fails here (see the export-shape test at the bottom, which pins
+// that these really are the same wrapper).
+function neverResolves() {
+  return new Promise<{ default: ComponentType }>(() => {});
+}
+
+describe("a chunk that fails to load", () => {
+  beforeEach(() => {
+    vi.spyOn(console, "error").mockImplementation(() => {});
+  });
+
+  // This is the regression proof. Before NodeChunkBoundary existed, a
+  // rejecting chunk inside only a Suspense boundary surfaced as an UNHANDLED
+  // error - Suspense catches the thrown promise, never its rejection - and
+  // React unmounted the tree. Demonstrating that directly is not possible in
+  // a test file, because the unhandled rejection fails the vitest run itself
+  // (measured: exit 1). The surviving "App chrome" assertion below is the
+  // same claim, stated positively.
+  it("is contained to its own card - the surrounding tree survives", async () => {
+    const Failing = lazyNodeViews.__testing.withNodeSuspense(
+      lazy(() => Promise.reject(new Error("Failed to fetch dynamically imported module"))),
+    );
+
+    render(
+      <div>
+        <h1>App chrome</h1>
+        <Failing />
+      </div>,
+    );
+
+    await waitFor(() => expect(screen.getByRole("alert")).toBeTruthy());
+    // The point of the whole exercise: everything around the bad card is
+    // still mounted.
+    expect(screen.getByText("App chrome")).toBeTruthy();
+    expect(screen.getByRole("alert").textContent).toContain("could not load");
+  });
+
+  it("says how to recover, and does not offer a retry that cannot work", async () => {
+    const Failing = lazyNodeViews.__testing.withNodeSuspense(
+      lazy(() => Promise.reject(new Error("boom"))),
+    );
+    render(<Failing />);
+    await waitFor(() => expect(screen.getByRole("alert")).toBeTruthy());
+    // React.lazy caches the rejection, so re-rendering replays the failure
+    // forever. A retry button here would be a button that never works.
+    expect(screen.queryByRole("button")).toBeNull();
+    expect(screen.getByRole("alert").textContent).toContain("Reload");
+  });
+
+  it("logs the underlying error rather than swallowing it", async () => {
+    const Failing = lazyNodeViews.__testing.withNodeSuspense(
+      lazy(() => Promise.reject(new Error("chunk 404"))),
+    );
+    render(<Failing />);
+    await waitFor(() => expect(screen.getByRole("alert")).toBeTruthy());
+    const logged = (console.error as unknown as { mock: { calls: unknown[][] } }).mock.calls;
+    expect(logged.some((call) => String(call[0]).includes("chunk failed to load"))).toBe(true);
+  });
+});
+
+describe("a chunk that is still loading", () => {
+  it("shows the loading shell, which export polls for", () => {
+    const Pending = lazyNodeViews.__testing.withNodeSuspense(lazy(neverResolves));
+    const { container } = render(<Pending />);
+    // exportCanvasPng.ts's NODE_LOADING_SELECTOR waits on exactly this class;
+    // if it is ever renamed, the PNG export starts capturing loading shells.
+    expect(container.querySelector(".scene-node-loading")).toBeTruthy();
+    expect(container.querySelector('[aria-busy="true"]')).toBeTruthy();
+  });
+});
+
+describe("the exported node views", () => {
+  it("are all wrapped, so no kind can bypass the boundary", () => {
+    const views = Object.entries(lazyNodeViews).filter(([name]) => name.endsWith("NodeView"));
+    // 17 kinds; GroupNodeView backs both frame and container.
+    expect(views.length).toBe(17);
+    for (const [name, view] of views) {
+      expect(typeof view, name).toBe("function");
+      expect((view as ComponentType).displayName, name).toContain("LazyNodeView");
+    }
+  });
+});

web_ui/src/app/canvas/lazyNodeViews.tsx

@@ -27,7 +27,7 @@
 // component renders, so an empty fallback would collapse the card and let
 // edges snap to a zero-size box for a frame or two.
 
-import { lazy, Suspense, type ComponentType } from "react";
+import { Component, lazy, Suspense, type ComponentType, type ReactNode } from "react";
 
 /** The shell shown while a kind's chunk is still loading. */
 function NodeChunkFallback() {
@@ -38,26 +38,85 @@ function NodeChunkFallback() {
   );
 }
 
+/** Shown when a chunk could not be fetched at all.
+ *
+ * There is deliberately no retry button. React.lazy caches the REJECTION as
+ * well as the resolution, so re-rendering the same lazy component replays the
+ * failure forever - a retry that cannot work is worse than none. Reloading
+ * re-runs the import from scratch, which is the only thing that can recover. */
+function NodeChunkError() {
+  return (
+    <div className="scene-node scene-node-loading" role="alert">
+      <div className="scene-node-title">
+        This card could not load. Reload the app to try again.
+      </div>
+    </div>
+  );
+}
+
+/**
+ * Catches a failed chunk fetch for ONE node.
+ *
+ * Without it a rejected import propagates uncaught and React unmounts the
+ * whole tree - one node kind failing to load (a stale index after a deploy, a
+ * dropped connection mid-session) would blank the entire canvas and the app
+ * chrome with it. Verified: a rejecting lazy() inside only a Suspense
+ * boundary surfaces as an unhandled error, because Suspense catches the
+ * PROMISE, never the rejection.
+ *
+ * A class component because that is the only thing React lets be an error
+ * boundary. Per node, for the same reason the Suspense boundary is per node:
+ * the blast radius of one bad chunk should be one card.
+ */
+class NodeChunkBoundary extends Component<{ children: ReactNode }, { failed: boolean }> {
+  state = { failed: false };
+
+  static getDerivedStateFromError() {
+    return { failed: true };
+  }
+
+  componentDidCatch(error: unknown) {
+    // Left visible rather than swallowed: a chunk that will not load is a
+    // deploy/network problem worth seeing in the console, and this component
+    // cannot surface it any other way.
+    console.error("node view chunk failed to load", error);
+  }
+
+  render() {
+    return this.state.failed ? <NodeChunkError /> : this.props.children;
+  }
+}
+
 /**
  * Wrap a lazily-imported node component in its own Suspense boundary.
  *
  * React Flow's nodeTypes map wants a plain component, and `lazy()` returns
  * one that throws a promise - so the boundary has to live between them,
  * here, rather than at any call site.
  */
-function withNodeSuspense<P extends object>(Component: ComponentType<P>): ComponentType<P> {
+function withNodeSuspense<P extends object>(Loaded: ComponentType<P>): ComponentType<P> {
   function LazyNodeView(props: P) {
+    // Boundary OUTSIDE Suspense: Suspense catches the thrown promise, the
+    // boundary catches its rejection. Nested the other way the rejection
+    // escapes past the boundary entirely.
     return (
-      <Suspense fallback={<NodeChunkFallback />}>
-        <Component {...props} />
-      </Suspense>
+      <NodeChunkBoundary>
+        <Suspense fallback={<NodeChunkFallback />}>
+          <Loaded {...props} />
+        </Suspense>
+      </NodeChunkBoundary>
     );
   }
   // Keeps React DevTools and any test that queries by displayName readable.
-  LazyNodeView.displayName = `LazyNodeView(${Component.displayName || "chunk"})`;
+  LazyNodeView.displayName = `LazyNodeView(${Loaded.displayName || "chunk"})`;
   return LazyNodeView;
 }
 
+// Exposed for lazyNodeViews.test.tsx only. The wrapper's behaviour on a
+// failed chunk is the thing worth testing, and it cannot be reached through
+// the real exports without a real failing chunk.
+export const __testing = { withNodeSuspense }
+
 // Named exports, so each import() is unwrapped to a default for lazy().
 export const ArtifactNodeView = withNodeSuspense(
   lazy(() => import("./ArtifactNodeView").then((m) => ({ default: m.ArtifactNodeView }))),

web_ui/src/app/styles.css

@@ -8512,6 +8512,26 @@ mark.document-view-search-match-current {
   border-radius: 8px;
 }
 
+/* ADR-019 node-view code split: the shell shown while a node kind's chunk
+   loads (lazyNodeViews.tsx). It shipped with NO styling at all, so the
+   fallback collapsed to whatever .scene-node alone gives - React Flow has
+   already positioned and sized the node by then, and an unsized card lets
+   edges snap to a near-zero box for a frame or two. Mirrors
+   .chart-node-placeholder below, which is the same idea for chart chunks. */
+.scene-node-loading {
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  min-width: 260px;
+  min-height: 120px;
+  padding: 16px;
+  color: var(--gl-surface-text-muted);
+  font-size: 11px;
+  text-align: center;
+  border: 1px dashed var(--gl-surface-border);
+  border-radius: 8px;
+}
+
 .chart-node-placeholder {
   display: flex;
   align-items: center;