Cleanup code in advance of initial beta release v0.1.0

ARCHITECTURE.md

@@ -476,12 +476,14 @@ pdv_kernel/
     namespace.py         # PDVNamespace (protected dict), PDVApp, pdv_namespace()
     serialization.py     # Type detection, format writers (npy, parquet, json, module, gui, namelist, lib)
     environment.py       # Path utilities, working dir management, project root logic
-    errors.py            # PDVError, PDVPathError, PDVKeyError, PDVProtectedNameError, PDVSerializationError, PDVScriptError, PDVCommError, PDVVersionError, PDVSchemaError
+    errors.py            # PDVError, PDVPathError, PDVKeyError, PDVProtectedNameError, PDVSerializationError, PDVScriptError, PDVVersionError
     modules.py           # Custom type handler registry and dispatch (@pdv.handle() decorator)
     namelist_utils.py    # Fortran namelist and TOML parsing utilities
     checksum.py          # Content-based XXH3-128 Merkle-tree checksum for PDVTree (tree_checksum())
+    tree_loader.py       # Shared two-pass tree-index loader used by project.load and module.register handlers
     handlers/
         __init__.py
+        _helpers.py      # Shared register-validation, namelist resolution, and gui-attach helpers
         lifecycle.py     # pdv.init, pdv.ready handlers
         project.py       # pdv.project.load, pdv.project.save handlers
         tree.py          # pdv.tree.list, pdv.tree.get, pdv.tree.resolve_file handlers
@@ -539,6 +541,7 @@ All other `pdv_*` names in the namespace are an error. Internal implementation f
 - **`__getitem__(key)`**: If key is not in the in-memory dict, consults the lazy-load registry. If the key exists in the registry (i.e., it was populated from a save directory), fetches the data file from the save directory into the working directory and loads it into memory. Then returns the value. If neither in-memory nor in the registry, raises `KeyError`.
 - **`__setitem__(key, value)`**: Sets the value in the in-memory dict. Emits `pdv.tree.changed` push notification.
 - **`__delitem__(key)`**: Removes from in-memory dict and from the lazy-load registry. Emits `pdv.tree.changed`.
+- **`set_quiet(key, value)`**: Same dot-path traversal as `__setitem__` (creating intermediate `PDVTree` containers as needed) but bypasses the change notification. Used by bulk loaders (`pdv.project.load`, `pdv.module.register`) to populate the tree without flooding the comm channel — callers typically emit a single `pdv.project.loaded` push when bulk load completes.
 - **Path notation**: Both `pdv_tree['key']` and `pdv_tree['parent.child.grandchild']` are supported as a convenience. Dot-separated paths are resolved recursively.
 - **`run_script(path, **kwargs)`**: Loads and executes the script at `path`, passing `pdv_tree` and `**kwargs` to its `run()` function.
 
@@ -1360,9 +1363,10 @@ electron/
         project-file-sync.ts    ← Tree file synchronization between working/save dirs
         module-manager.ts       ← Module import/installation pipeline
         module-runtime.ts       ← Module bind/setup helpers: lib file copy, sys.path setup, script registration
-        module-window-manager.ts ← Module GUI popup BrowserWindow lifecycle
-        gui-editor-window-manager.ts  ← GUI editor popup BrowserWindow lifecycle
-        gui-viewer-window-manager.ts  ← Standalone GUI viewer BrowserWindow lifecycle
+        base-window-manager.ts    ← Generic per-key BrowserWindow lifecycle base class shared by the three popup managers
+        module-window-manager.ts  ← Module GUI popup BrowserWindow (subclass of BaseWindowManager)
+        gui-editor-window-manager.ts  ← GUI editor popup BrowserWindow (subclass of BaseWindowManager)
+        gui-viewer-window-manager.ts  ← Standalone GUI viewer BrowserWindow (subclass of BaseWindowManager)
         ipc-register-kernels.ts           ← IPC handlers: kernel lifecycle + execution
         ipc-register-project.ts           ← IPC handlers: project save/load/new
         ipc-register-modules.ts           ← IPC handlers: module import/install

electron/knip.json

@@ -4,6 +4,9 @@
     "main/bootstrap.ts",
     "preload.ts",
     "renderer/src/main.tsx",
+    "renderer/src/module-window-main.tsx",
+    "renderer/src/gui-editor-main.tsx",
+    "renderer/src/gui-viewer-main.tsx",
     "renderer/vite.config.ts"
   ],
   "project": [
@@ -14,7 +17,8 @@
     "electronmon"
   ],
   "ignore": [
-    "renderer/src/vite-env.d.ts"
+    "renderer/src/vite-env.d.ts",
+    "main/pdv-protocol.ts"
   ],
   "ignoreExportsUsedInFile": true
 }

electron/main/base-window-manager.ts

@@ -0,0 +1,175 @@
+/**
+ * base-window-manager.ts — Generic per-key BrowserWindow manager.
+ *
+ * Shared infrastructure for the three popup-window managers:
+ * {@link GuiEditorWindowManager}, {@link GuiViewerWindowManager}, and
+ * {@link ModuleWindowManager}. Each subclass differs only in:
+ *
+ * - the renderer HTML file it loads,
+ * - the BrowserWindow dimensions/title it constructs, and
+ * - the typed context attached to each window.
+ *
+ * The base class owns:
+ * - the `windows` map keyed by per-subclass identity (alias / treePath),
+ * - the `contexts` map keyed by `webContents.id` for IPC sender lookup,
+ * - open / close / closeAll lifecycle, and
+ * - broadcast to all live windows.
+ *
+ * Subclasses implement {@link openWindow} via a typed public `open(...)`
+ * that builds a context and calls {@link openWindowInternal}.
+ */
+
+import { BrowserWindow, type BrowserWindowConstructorOptions } from "electron";
+import * as path from "path";
+
+/**
+ * Static and per-call configuration for one BrowserWindow.
+ *
+ * Subclasses return one of these from {@link BaseWindowManager.buildConfig}
+ * to describe the window they want to open.
+ */
+export interface ChildWindowConfig {
+  /**
+   * Renderer-side HTML file basename to load (e.g. `"module-window.html"`).
+   * The base class resolves it against the dev server in development and
+   * against `renderer/dist/` in production.
+   */
+  htmlFile: string;
+  /**
+   * BrowserWindow constructor options for the new child window.
+   *
+   * The base class always overrides `show: false` (the window is shown
+   * after content is loaded) and supplies a fresh `webPreferences` block
+   * with the manager's preload path; subclasses should not set those.
+   */
+  windowOptions: Omit<BrowserWindowConstructorOptions, "show" | "webPreferences">;
+}
+
+/**
+ * Generic per-key BrowserWindow manager.
+ *
+ * @typeParam TKey - The lookup key for child windows (typically `string`,
+ *   either a module alias or a tree path).
+ * @typeParam TContext - The typed per-window context attached to each
+ *   window for IPC sender lookup.
+ */
+export abstract class BaseWindowManager<TKey extends string, TContext> {
+  protected readonly windows = new Map<TKey, BrowserWindow>();
+  protected readonly contexts = new Map<number, TContext>();
+
+  constructor(protected readonly preloadPath: string) {}
+
+  /**
+   * Build the per-call window configuration. Implemented by each subclass
+   * to return its dimensions, title, and HTML file. Receives the key and
+   * context so titles can be derived from either.
+   */
+  protected abstract buildConfig(
+    key: TKey,
+    context: TContext
+  ): ChildWindowConfig;
+
+  /**
+   * Open a child window for the given key, or focus an existing one.
+   *
+   * Subclasses expose a typed public `open(...)` that constructs the
+   * context and delegates here.
+   *
+   * @throws {Error} When the renderer content fails to load.
+   */
+  protected async openWindowInternal(key: TKey, context: TContext): Promise<void> {
+    const existing = this.windows.get(key);
+    if (existing && !existing.isDestroyed()) {
+      existing.focus();
+      return;
+    }
+
+    const config = this.buildConfig(key, context);
+    const child = new BrowserWindow({
+      ...config.windowOptions,
+      show: false,
+      webPreferences: {
+        preload: this.preloadPath,
+        contextIsolation: true,
+        nodeIntegration: false,
+        sandbox: false,
+      },
+    });
+
+    const webContentsId = child.webContents.id;
+    this.windows.set(key, child);
+    this.contexts.set(webContentsId, context);
+
+    child.on("closed", () => {
+      this.windows.delete(key);
+      this.contexts.delete(webContentsId);
+    });
+
+    const devServerUrl = process.env.VITE_DEV_SERVER_URL;
+    if (process.env.NODE_ENV === "development" && devServerUrl) {
+      const base = devServerUrl.replace(/\/$/, "");
+      await child.loadURL(`${base}/${config.htmlFile}`);
+    } else {
+      const rendererDir = path.join(
+        __dirname,
+        "..",
+        "..",
+        "renderer",
+        "dist"
+      );
+      await child.loadFile(path.join(rendererDir, config.htmlFile));
+    }
+
+    child.show();
+  }
+
+  /**
+   * Close a child window by key. Returns true when a live window was
+   * found and asked to close. The actual map cleanup happens via the
+   * `closed` event handler attached in {@link openWindowInternal}.
+   */
+  close(key: TKey): boolean {
+    const win = this.windows.get(key);
+    if (!win || win.isDestroyed()) {
+      this.windows.delete(key);
+      return false;
+    }
+    win.close();
+    return true;
+  }
+
+  /**
+   * Close all open child windows and clear lookup maps.
+   *
+   * Maps are cleared after the close loop instead of inside it to avoid
+   * mutating during iteration; the per-window `closed` handler is also a
+   * no-op on the cleared map.
+   */
+  closeAll(): void {
+    for (const win of this.windows.values()) {
+      if (!win.isDestroyed()) {
+        win.close();
+      }
+    }
+    this.windows.clear();
+    this.contexts.clear();
+  }
+
+  /**
+   * Look up the typed context for a given `webContents.id`. Used by IPC
+   * handlers that need to know which child window an `event.sender`
+   * belongs to.
+   */
+  getContextForSender(webContentsId: number): TContext | null {
+    return this.contexts.get(webContentsId) ?? null;
+  }
+
+  /** Send a push message to every live child window. */
+  broadcastToAll(channel: string, payload: unknown): void {
+    for (const win of this.windows.values()) {
+      if (!win.isDestroyed()) {
+        win.webContents.send(channel, payload);
+      }
+    }
+  }
+}

electron/main/comm-router.ts

@@ -12,9 +12,11 @@
  * 2. **Push notifications** (CommRouter.onPush) — registers listeners for
  *    unsolicited kernel-initiated messages (e.g. pdv.tree.changed).
  *
- * CommRouter is stateless between attach() / detach() cycles. Call attach()
- * once per kernel session and detach() before discarding the router or when
- * the kernel dies.
+ * CommRouter retains pending-request and push-handler state across the
+ * lifetime of the instance. Call attach() once per kernel session to bind
+ * to a kernel's iopub stream and detach() before discarding the router or
+ * when the kernel dies. Pending requests are rejected on detach to avoid
+ * dangling promises.
  *
  * See Also
  * --------
@@ -26,6 +28,7 @@
 import * as crypto from "crypto";
 import {
   PDVMessage,
+  PDVErrorPayload,
   getAppVersion,
   PDV_COMM_TARGET,
   isPDVMessage,
@@ -53,6 +56,45 @@ export class PDVCommError extends Error {
   }
 }
 
+/**
+ * Validate and narrow an error response payload to {@link PDVErrorPayload}.
+ *
+ * ARCHITECTURE.md §3.5 specifies that every error envelope must carry both
+ * `code` and `message` as strings. Earlier code defaulted missing fields to
+ * `"unknown"` / `"PDV error"`, which silently masked malformed errors. This
+ * validator surfaces them as a typed PDVCommError with a synthetic
+ * `protocol.malformed_error` code and the raw payload preview in the message,
+ * so contract violations are visible during development.
+ *
+ * @param raw - Untyped payload from a status='error' PDVMessage.
+ * @param msg - The full enclosing PDVMessage (used as the error response).
+ * @returns A validated PDVErrorPayload, or a synthetic one describing the
+ *   malformed payload when validation fails.
+ */
+function parsePDVErrorPayload(raw: unknown, msg: PDVMessage): PDVErrorPayload {
+  if (
+    raw !== null &&
+    typeof raw === "object" &&
+    typeof (raw as { code?: unknown }).code === "string" &&
+    typeof (raw as { message?: unknown }).message === "string"
+  ) {
+    return raw as PDVErrorPayload;
+  }
+  let preview: string;
+  try {
+    preview = JSON.stringify(raw);
+  } catch {
+    preview = String(raw);
+  }
+  console.error(
+    `[CommRouter] Malformed error payload on ${msg.type} (msg_id=${msg.msg_id}): ${preview}`
+  );
+  return {
+    code: "protocol.malformed_error",
+    message: `Malformed error payload received from kernel (${msg.type}): ${preview}`,
+  };
+}
+
 /**
  * Thrown when a PDV request receives no response within the configured
  * timeout window.
@@ -400,14 +442,8 @@ export class CommRouter {
     clearTimeout(pending.timer);
 
     if (msg.status === "error") {
-      const payload = msg.payload as { code?: string; message?: string };
-      pending.reject(
-        new PDVCommError(
-          payload.message ?? "PDV error",
-          payload.code ?? "unknown",
-          msg
-        )
-      );
+      const payload = parsePDVErrorPayload(msg.payload, msg);
+      pending.reject(new PDVCommError(payload.message, payload.code, msg));
     } else {
       pending.resolve(msg);
     }
@@ -462,31 +498,4 @@ export class CommRouter {
     );
     this.pushHandlers.clear();
   }
-
-  /**
-   * Handle a raw incoming comm message from the Jupyter client.
-   *
-   * @deprecated Use attach() / _handleIopubMessage() instead.
-   *   This method is kept for backward compatibility with the original skeleton
-   *   interface but delegates to _handleIopubMessage() after wrapping the data.
-   *
-   * @param data - Raw data payload from the Jupyter comm channel.
-   */
-  handleIncoming(data: unknown): void {
-    // Wrap in a synthetic comm_msg JupyterMessage for _handleIopubMessage.
-    const synthetic: JupyterMessage = {
-      header: {
-        msg_id: crypto.randomUUID(),
-        username: "pdv",
-        session: "",
-        msg_type: "comm_msg",
-        version: "5.3",
-        date: new Date().toISOString(),
-      },
-      parent_header: {},
-      metadata: {},
-      content: { comm_id: this.commId ?? "", data },
-    };
-    this._handleIopubMessage(synthetic);
-  }
 }

electron/main/config.test.ts

@@ -44,6 +44,7 @@ describe("ConfigStore", () => {
       showPrivateVariables: false,
       showModuleVariables: false,
       showCallableVariables: false,
+      autoRefreshNamespace: false,
       settings: {
         appearance: {
           themeName: "Dark+ (VSCode)",
@@ -89,6 +90,7 @@ describe("ConfigStore", () => {
       showPrivateVariables: true,
       showModuleVariables: true,
       showCallableVariables: false,
+      autoRefreshNamespace: false,
       theme: "dark",
       settings: {
         appearance: {
@@ -109,6 +111,7 @@ describe("ConfigStore", () => {
       showPrivateVariables: false,
       showModuleVariables: false,
       showCallableVariables: false,
+      autoRefreshNamespace: false,
       settings: {
         appearance: {
           themeName: "Dark+ (VSCode)",