fix: preserve paragraph runtime ids and reject empty insert refs

Author: harbournick

packages/document-api/src/insert/insert.test.ts

@@ -0,0 +1,43 @@
+import { describe, it, expect, vi } from 'vitest';
+import { executeInsert, type InsertInput } from './insert.js';
+import type { SelectionMutationAdapter } from '../selection-mutation.js';
+import type { WriteAdapter } from '../write/write.js';
+
+// ---------------------------------------------------------------------------
+// Stub adapters — validation runs before any adapter method is called, so
+// these stubs only need to exist (they should never be invoked in error cases).
+// ---------------------------------------------------------------------------
+
+function createStubSelectionAdapter(): SelectionMutationAdapter {
+  return { execute: vi.fn() } as unknown as SelectionMutationAdapter;
+}
+
+function createStubWriteAdapter(): WriteAdapter {
+  return { write: vi.fn(), insertStructured: vi.fn(), replaceStructured: vi.fn() } as unknown as WriteAdapter;
+}
+
+// ---------------------------------------------------------------------------
+// ref validation
+// ---------------------------------------------------------------------------
+
+describe('executeInsert: ref validation', () => {
+  it('rejects empty string ref', () => {
+    const input: InsertInput = { value: 'hello', ref: '' } as unknown as InsertInput;
+    expect(() => executeInsert(createStubSelectionAdapter(), createStubWriteAdapter(), input)).toThrow(
+      'ref must be a non-empty string',
+    );
+  });
+
+  it('rejects non-string ref', () => {
+    const input = { value: 'hello', ref: 42 } as unknown as InsertInput;
+    expect(() => executeInsert(createStubSelectionAdapter(), createStubWriteAdapter(), input)).toThrow(
+      'ref must be a non-empty string',
+    );
+  });
+
+  it('does not reject undefined ref (untargeted insert is valid)', () => {
+    const writeAdapter = createStubWriteAdapter();
+    (writeAdapter.write as ReturnType<typeof vi.fn>).mockReturnValue({ success: true, ref: 'r1' });
+    expect(() => executeInsert(createStubSelectionAdapter(), writeAdapter, { value: 'hello' })).not.toThrow();
+  });
+});

packages/document-api/src/insert/insert.ts

@@ -151,8 +151,8 @@ function validateTextInsertInput(input: Record<string, unknown>): void {
     });
   }
 
-  if (ref !== undefined && typeof ref !== 'string') {
-    throw new DocumentApiValidationError('INVALID_TARGET', `ref must be a string, got ${typeof ref}.`, {
+  if (ref !== undefined && (typeof ref !== 'string' || ref === '')) {
+    throw new DocumentApiValidationError('INVALID_TARGET', 'ref must be a non-empty string.', {
       field: 'ref',
       value: ref,
     });

packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.test.ts

@@ -307,16 +307,19 @@ describe('buildBlockIndex', () => {
       expect(index.candidates[0].nodeId).toMatch(/^para-auto-[0-9a-f]{8}$/);
     });
 
-    it('assigns deterministic fallback id when sdBlockId is a volatile UUID', () => {
+    it('keeps volatile sdBlockId as primary id for session stability', () => {
+      // Volatile (UUID-like) sdBlockIds are preferred over deterministic
+      // fallback IDs because the fallback hashes nodeType + traversal path,
+      // which shifts when siblings are inserted/moved or a paragraph is
+      // restyled to heading/list-item. The UUID stays stable for the session.
+      const volatileUuid = '7701a615-4ad8-45b5-922c-2a32114df4c8';
       const index = indexFromNodes({
         typeName: 'paragraph',
-        attrs: { sdBlockId: '7701a615-4ad8-45b5-922c-2a32114df4c8' },
+        attrs: { sdBlockId: volatileUuid },
         offset: 7,
       });
       expect(index.candidates).toHaveLength(1);
-      expect(index.candidates[0].nodeId).toMatch(/^para-auto-[0-9a-f]{8}$/);
-      // Volatile sdBlockId registered as alias
-      expect(index.byId.get('paragraph:7701a615-4ad8-45b5-922c-2a32114df4c8')).toBeDefined();
+      expect(index.candidates[0].nodeId).toBe(volatileUuid);
     });
 
     it('keeps non-volatile sdBlockId as primary id for paragraphs', () => {

packages/super-editor/src/document-api-adapters/helpers/node-address-resolver.ts

@@ -117,7 +117,15 @@ function resolveLegacyTableIdentity(attrs: BlockIdAttrs): string | undefined {
   return toId(attrs.paraId) ?? toId(attrs.blockId) ?? toId(attrs.id) ?? toId(attrs.uuid);
 }
 
-function resolveRuntimeBlockIdentity(
+/**
+ * Resolves a runtime block identity for **table-like** nodes.
+ *
+ * Non-volatile sdBlockId is preferred; otherwise a deterministic fallback
+ * (hashed from nodeType + traversal path) is used. This is correct for tables
+ * because they keep a stable nodeType across mutations — their sdBlockId may
+ * change when ProseMirror replaces the node during property edits.
+ */
+function resolveTableRuntimeIdentity(
   nodeType: BlockNodeType,
   attrs: BlockIdAttrs,
   pos: number,
@@ -130,6 +138,25 @@ function resolveRuntimeBlockIdentity(
   return buildFallbackBlockNodeId(nodeType, pos, path);
 }
 
+/**
+ * Resolves a runtime block identity for **paragraph-like** nodes
+ * (paragraph, heading, listItem).
+ *
+ * Always prefers sdBlockId — even a volatile (UUID-like) one — because the
+ * deterministic fallback hashes nodeType + traversal path, both of which shift
+ * during ordinary edits: sibling inserts/moves change the path, and restyles
+ * (paragraph → heading/listItem) change the nodeType. The sdBlockId stays
+ * stable for the session lifetime.
+ */
+function resolveParagraphRuntimeIdentity(
+  nodeType: BlockNodeType,
+  attrs: BlockIdAttrs,
+  pos: number,
+  path?: TraversalPath,
+): string | undefined {
+  return toId(attrs.sdBlockId) ?? buildFallbackBlockNodeId(nodeType, pos, path);
+}
+
 export function resolveBlockNodeId(
   node: ProseMirrorNode,
   pos: number,
@@ -138,10 +165,9 @@ export function resolveBlockNodeId(
 ): string | undefined {
   if (node.type.name === 'paragraph') {
     const attrs = node.attrs as ParagraphAttrs | undefined;
-    // paraId (imported from DOCX) is the primary identity for paragraphs. This
-    // preserves historical IDs across DOCX round-trips. Non-volatile sdBlockId
-    // is preferred over deterministic fallback for freshly created nodes.
-    return toId(attrs?.paraId) ?? resolveRuntimeBlockIdentity(nodeType, (attrs ?? {}) as BlockIdAttrs, pos, path);
+    // paraId (imported from DOCX) is the primary identity for paragraphs —
+    // preserves historical IDs across DOCX round-trips.
+    return toId(attrs?.paraId) ?? resolveParagraphRuntimeIdentity(nodeType, (attrs ?? {}) as BlockIdAttrs, pos, path);
   }
 
   if (nodeType === 'tableOfContents') {
@@ -162,7 +188,7 @@ export function resolveBlockNodeId(
   // UUID sdBlockId exists, expose a deterministic fallback instead so session
   // addresses remain reusable across fresh document opens.
   if (typeName === 'table' || typeName === 'tableCell' || typeName === 'tableHeader') {
-    return resolveLegacyTableIdentity(attrs) ?? resolveRuntimeBlockIdentity(nodeType, attrs, pos, path);
+    return resolveLegacyTableIdentity(attrs) ?? resolveTableRuntimeIdentity(nodeType, attrs, pos, path);
   }
 
   // NOTE: Migration surface for the stable-addresses plan.

packages/super-editor/src/document-api-adapters/plan-engine/plan-wrappers.ts

@@ -827,6 +827,10 @@ function insertStructuredInner(editor: Editor, input: InsertInput, options?: Mut
   let resolvedRange: ResolvedTextTarget;
   let effectiveTarget: TextAddress;
 
+  if (ref !== undefined && ref === '') {
+    throw new DocumentApiAdapterError('INVALID_TARGET', 'ref must be a non-empty string.', { ref });
+  }
+
   if (target) {
     const resolved = resolveSelectionTarget(editor, target);
     resolvedRange = { from: resolved.absFrom, to: resolved.absTo };