fix(super-editor): allow Backspace to delete empty paragraphs in suggesting mode

devtools/visual-testing/tests/interactions/stories/comments-tcs/backspace-empty-paragraph-suggesting.ts

@@ -0,0 +1,70 @@
+import { defineStory } from '@superdoc-testing/helpers';
+
+const WAIT_MS = 300;
+
+/**
+ * SD-1810: Backspace doesn't delete empty paragraph in suggesting mode
+ *
+ * Bug: When a user creates a new paragraph (Enter) in suggesting mode and
+ * immediately presses Backspace, nothing happens — the empty paragraph stays.
+ *
+ * Root cause: The track changes system intercepts the ReplaceStep via
+ * replaceStep() → markDeletion(). An empty paragraph has no inline content,
+ * so markDeletion finds nothing to mark and the step is silently swallowed.
+ *
+ * This test recreates the exact bug scenario:
+ * 1. Type text, press Enter to create an empty paragraph
+ * 2. Press Backspace — empty paragraph should be removed
+ * 3. Also tests the Enter → Enter → Backspace → Backspace flow (paragraph join)
+ */
+export default defineStory({
+  name: 'backspace-empty-paragraph-suggesting',
+  description: 'Test Backspace on empty paragraphs and paragraph joins in suggesting mode',
+  tickets: ['SD-1810'],
+  startDocument: null,
+  layout: true,
+  comments: 'off',
+  hideCaret: false,
+  hideSelection: false,
+
+  async run(_page, helpers): Promise<void> {
+    const { type, press, setDocumentMode, waitForStable, milestone } = helpers;
+
+    // Step 1: Type initial content
+    await type('Hello World');
+    await waitForStable(WAIT_MS);
+    await milestone('initial-text', 'Document with initial text');
+
+    // Step 2: Switch to suggesting mode
+    await setDocumentMode('suggesting');
+    await waitForStable(WAIT_MS);
+    await milestone('suggesting-mode', 'Switched to suggesting mode');
+
+    // Step 3: Press Enter to create an empty paragraph
+    await press('Enter');
+    await waitForStable(WAIT_MS);
+    await milestone('after-enter', 'New empty paragraph created below');
+
+    // Step 4: Press Backspace — should delete the empty paragraph
+    await press('Backspace');
+    await waitForStable(WAIT_MS);
+    await milestone('after-backspace', 'Empty paragraph removed, cursor back at end of "Hello World"');
+
+    // Step 5: Test Enter → Enter → Backspace → Backspace (join flow)
+    await press('Enter');
+    await waitForStable(WAIT_MS);
+    await press('Enter');
+    await waitForStable(WAIT_MS);
+    await milestone('after-two-enters', 'Two new paragraphs created');
+
+    // Step 6: First Backspace — removes the second empty paragraph
+    await press('Backspace');
+    await waitForStable(WAIT_MS);
+    await milestone('after-first-backspace', 'One empty paragraph removed');
+
+    // Step 7: Second Backspace — joins back with the original paragraph
+    await press('Backspace');
+    await waitForStable(WAIT_MS);
+    await milestone('after-second-backspace', 'Joined back to original paragraph — cursor at end of "Hello World"');
+  },
+});

packages/super-editor/src/core/extensions/keymap-history.test.js

@@ -0,0 +1,202 @@
+import { describe, it, expect, afterEach } from 'vitest';
+import { closeHistory, undoDepth } from 'prosemirror-history';
+import { initTestEditor } from '@tests/helpers/helpers.js';
+import { handleEnter, handleBackspace, handleDelete } from './keymap.js';
+
+describe('keymap history grouping', () => {
+  let editor;
+
+  afterEach(() => {
+    editor?.destroy();
+    editor = null;
+  });
+
+  const insertText = (ed, text) => {
+    const { from } = ed.state.selection;
+    ed.view.dispatch(ed.state.tr.insertText(text, from));
+  };
+
+  /** Simulate closeHistoryOnly (space / Opt+Backspace handler). */
+  const closeHistoryGroup = (ed) => {
+    ed.view.dispatch(closeHistory(ed.view.state.tr));
+  };
+
+  it('Enter creates a new undo group boundary', () => {
+    ({ editor } = initTestEditor({ mode: 'text', content: '<p></p>' }));
+
+    insertText(editor, 'hello');
+    const depthAfterFirstText = undoDepth(editor.state);
+
+    handleEnter(editor);
+
+    insertText(editor, 'world');
+    const depthAfterSecondText = undoDepth(editor.state);
+
+    expect(depthAfterSecondText).toBeGreaterThan(depthAfterFirstText);
+  });
+
+  it('undo after Enter restores text before Enter', () => {
+    ({ editor } = initTestEditor({ mode: 'text', content: '<p></p>' }));
+
+    insertText(editor, 'hello');
+    handleEnter(editor);
+    insertText(editor, 'world');
+
+    const textBefore = editor.state.doc.textContent;
+    expect(textBefore).toContain('hello');
+    expect(textBefore).toContain('world');
+
+    editor.commands.undo();
+    const textAfterUndo = editor.state.doc.textContent;
+    expect(textAfterUndo).toContain('hello');
+  });
+
+  it('Enter creates boundary in suggesting mode', () => {
+    ({ editor } = initTestEditor({
+      mode: 'text',
+      content: '<p></p>',
+      user: { name: 'Tester', email: 'test@test.com' },
+    }));
+
+    editor.commands.enableTrackChanges?.();
+
+    insertText(editor, 'hello');
+    const depthAfterFirstText = undoDepth(editor.state);
+
+    handleEnter(editor);
+
+    insertText(editor, 'world');
+    const depthAfterSecondText = undoDepth(editor.state);
+
+    expect(depthAfterSecondText).toBeGreaterThan(depthAfterFirstText);
+  });
+
+  it('space creates a word-level undo boundary', () => {
+    ({ editor } = initTestEditor({ mode: 'text', content: '<p></p>' }));
+
+    insertText(editor, 'hello');
+    const depthAfterFirstWord = undoDepth(editor.state);
+
+    // Simulate space handler: closeHistory then type space
+    closeHistoryGroup(editor);
+    insertText(editor, ' ');
+
+    insertText(editor, 'world');
+    const depthAfterSecondWord = undoDepth(editor.state);
+
+    expect(depthAfterSecondWord).toBeGreaterThan(depthAfterFirstWord);
+  });
+
+  it('undo after space removes only the last word', () => {
+    ({ editor } = initTestEditor({ mode: 'text', content: '<p></p>' }));
+
+    insertText(editor, 'hello');
+    closeHistoryGroup(editor);
+    insertText(editor, ' world');
+
+    expect(editor.state.doc.textContent).toBe('hello world');
+
+    editor.commands.undo();
+    expect(editor.state.doc.textContent).toBe('hello');
+  });
+
+  it('closeHistory before deletion creates its own undo step', () => {
+    ({ editor } = initTestEditor({ mode: 'text', content: '<p></p>' }));
+
+    insertText(editor, 'hello world');
+    const depthAfterTyping = undoDepth(editor.state);
+
+    // Simulate Opt+Backspace: closeHistory then delete last word
+    closeHistoryGroup(editor);
+    const { from } = editor.state.selection;
+    editor.view.dispatch(editor.state.tr.delete(from - 5, from));
+    const depthAfterDelete = undoDepth(editor.state);
+
+    expect(depthAfterDelete).toBeGreaterThan(depthAfterTyping);
+
+    // Undo should restore the deleted word
+    editor.commands.undo();
+    expect(editor.state.doc.textContent).toBe('hello world');
+  });
+
+  it('Backspace creates a new undo group boundary', () => {
+    ({ editor } = initTestEditor({ mode: 'text', content: '<p></p>' }));
+
+    // Create two paragraphs: type, Enter, type
+    insertText(editor, 'hello');
+    handleEnter(editor);
+    insertText(editor, 'world');
+    const depthBeforeBackspace = undoDepth(editor.state);
+
+    // Move cursor to start of second paragraph so joinBackward succeeds
+    let secondParaStart = null;
+    editor.state.doc.forEach((_node, offset, index) => {
+      if (index === 1) secondParaStart = offset + 1;
+    });
+    editor.view.dispatch(
+      editor.state.tr.setSelection(editor.state.selection.constructor.create(editor.state.doc, secondParaStart)),
+    );
+
+    // Backspace at start of second paragraph → joins paragraphs
+    handleBackspace(editor);
+
+    insertText(editor, ' after');
+    const depthAfterBackspace = undoDepth(editor.state);
+
+    expect(depthAfterBackspace).toBeGreaterThan(depthBeforeBackspace);
+  });
+
+  it('undo after Backspace join restores paragraph break', () => {
+    ({ editor } = initTestEditor({ mode: 'text', content: '<p></p>' }));
+
+    insertText(editor, 'hello');
+    handleEnter(editor);
+    insertText(editor, 'world');
+
+    expect(editor.state.doc.childCount).toBe(2);
+
+    // Move cursor to start of second paragraph
+    let secondParaStart = null;
+    editor.state.doc.forEach((_node, offset, index) => {
+      if (index === 1) secondParaStart = offset + 1;
+    });
+    editor.view.dispatch(
+      editor.state.tr.setSelection(editor.state.selection.constructor.create(editor.state.doc, secondParaStart)),
+    );
+
+    // Backspace joins paragraphs
+    handleBackspace(editor);
+    expect(editor.state.doc.childCount).toBe(1);
+
+    // Undo should restore the paragraph break
+    editor.commands.undo();
+    expect(editor.state.doc.childCount).toBe(2);
+  });
+
+  it('Delete creates a new undo group boundary', () => {
+    ({ editor } = initTestEditor({ mode: 'text', content: '<p></p>' }));
+
+    // Create two paragraphs
+    insertText(editor, 'hello');
+    handleEnter(editor);
+    insertText(editor, 'world');
+    const depthBeforeDelete = undoDepth(editor.state);
+
+    // Move cursor to end of first paragraph so joinForward succeeds
+    let firstParaEnd = null;
+    editor.state.doc.forEach((node, offset, index) => {
+      if (index === 0) firstParaEnd = offset + node.nodeSize - 1;
+    });
+    editor.view.dispatch(
+      editor.state.tr.setSelection(editor.state.selection.constructor.create(editor.state.doc, firstParaEnd)),
+    );
+
+    // Delete at end of first paragraph → joins paragraphs
+    handleDelete(editor);
+
+    insertText(editor, ' after');
+    const depthAfterDelete = undoDepth(editor.state);
+
+    expect(depthAfterDelete).toBeGreaterThan(depthBeforeDelete);
+  });
+});

packages/super-editor/src/core/extensions/keymap.js

@@ -1,8 +1,16 @@
+import { closeHistory } from 'prosemirror-history';
 import { Extension } from '../Extension.js';
 import { isIOS } from '../utilities/isIOS.js';
 import { isMacOS } from '../utilities/isMacOS.js';
 
 export const handleEnter = (editor) => {
+  const { view } = editor;
+  // Close the current undo group so this structural action becomes its own undo step.
+  // Note: this fires before the command chain, so if no command succeeds (rare — e.g.
+  // Enter with no valid split target) an empty undo boundary is created. Acceptable
+  // trade-off vs. the complexity of post-hoc closeHistory after commands.first.
+  view?.dispatch?.(closeHistory(view?.state?.tr));
+
   return editor.commands.first(({ commands }) => [
     () => commands.splitRunToParagraph(),
     () => commands.newlineInCode(),
@@ -13,6 +21,10 @@ export const handleEnter = (editor) => {
 };
 
 export const handleBackspace = (editor) => {
+  const { view } = editor;
+  // Close undo group — see comment in handleEnter.
+  view?.dispatch?.(closeHistory(view?.state?.tr));
+
   return editor.commands.first(({ commands, tr }) => [
     () => commands.undoInputRule(),
     () => {
@@ -30,6 +42,10 @@ export const handleBackspace = (editor) => {
 };
 
 export const handleDelete = (editor) => {
+  const { view } = editor;
+  // Close undo group — see comment in handleEnter.
+  view?.dispatch?.(closeHistory(view?.state?.tr));
+
   return editor.commands.first(({ commands }) => [
     () => commands.deleteSkipEmptyRun(),
     () => commands.deleteNextToRun(),

packages/super-editor/src/extensions/block-node/block-node.js

@@ -242,6 +242,26 @@ export const BlockNode = Extension.create({
       tr.setNodeMarkup(pos, undefined, nextAttrs, node.marks);
     };
 
+    /**
+     * Ensures a block node has a unique sdBlockId, assigning a new UUID if the
+     * current ID is missing or already seen. Tracks seen IDs in the provided Set
+     * to detect duplicates (e.g., when tr.split() copies the original paragraph's ID).
+     * @param {ProseMirrorNode} node - The node to check.
+     * @param {Object} nextAttrs - Mutable attrs object to update.
+     * @param {Set<string>} seenIds - Set of IDs already encountered in this traversal.
+     * @returns {boolean} True if the sdBlockId was changed.
+     */
+    const ensureUniqueSdBlockId = (node, nextAttrs, seenIds) => {
+      const currentId = node.attrs?.sdBlockId;
+      let changed = false;
+      if (nodeAllowsSdBlockIdAttr(node) && (nodeNeedsSdBlockId(node) || seenIds.has(currentId))) {
+        nextAttrs.sdBlockId = uuidv4();
+        changed = true;
+      }
+      if (currentId) seenIds.add(currentId);
+      return changed;
+    };
+
     return [
       new Plugin({
         key: BlockNodePluginKey,
@@ -258,14 +278,11 @@ export const BlockNode = Extension.create({
 
           if (!hasInitialized) {
             // Initial pass: assign IDs to all block nodes in document
+            const seenIds = new Set();
             newState.doc.descendants((node, pos) => {
               if (!nodeAllowsSdBlockIdAttr(node) && !nodeAllowsSdBlockRevAttr(node)) return;
               const nextAttrs = { ...node.attrs };
-              let nodeChanged = false;
-              if (nodeAllowsSdBlockIdAttr(node) && nodeNeedsSdBlockId(node)) {
-                nextAttrs.sdBlockId = uuidv4();
-                nodeChanged = true;
-              }
+              let nodeChanged = ensureUniqueSdBlockId(node, nextAttrs, seenIds);
               if (nodeAllowsSdBlockRevAttr(node)) {
                 const rev = ensureBlockRev(node);
                 if (nextAttrs.sdBlockRev !== rev) {
@@ -331,6 +348,9 @@ export const BlockNode = Extension.create({
 
             const docSize = newState.doc.content.size;
             const mergedRanges = mergeRanges(rangesToCheck, docSize);
+            // Track seen sdBlockIds across all ranges to detect duplicates
+            // (e.g., when tr.split() copies the original paragraph's sdBlockId to the new one).
+            const seenBlockIds = new Set();
 
             for (const { from, to } of mergedRanges) {
               const clampedRange = clampRange(from, to, docSize);
@@ -346,11 +366,7 @@ export const BlockNode = Extension.create({
                   if (!nodeAllowsSdBlockIdAttr(node) && !nodeAllowsSdBlockRevAttr(node)) return;
                   if (updatedPositions.has(pos)) return;
                   const nextAttrs = { ...node.attrs };
-                  let nodeChanged = false;
-                  if (nodeAllowsSdBlockIdAttr(node) && nodeNeedsSdBlockId(node)) {
-                    nextAttrs.sdBlockId = uuidv4();
-                    nodeChanged = true;
-                  }
+                  let nodeChanged = ensureUniqueSdBlockId(node, nextAttrs, seenBlockIds);
                   if (nodeAllowsSdBlockRevAttr(node)) {
                     nextAttrs.sdBlockRev = getNextBlockRev(node);
                     nodeChanged = true;
@@ -369,14 +385,11 @@ export const BlockNode = Extension.create({
             }
 
             if (shouldFallbackToFullTraversal) {
+              const fallbackSeenIds = new Set();
               newState.doc.descendants((node, pos) => {
                 if (!nodeAllowsSdBlockIdAttr(node) && !nodeAllowsSdBlockRevAttr(node)) return;
                 const nextAttrs = { ...node.attrs };
-                let nodeChanged = false;
-                if (nodeAllowsSdBlockIdAttr(node) && nodeNeedsSdBlockId(node)) {
-                  nextAttrs.sdBlockId = uuidv4();
-                  nodeChanged = true;
-                }
+                let nodeChanged = ensureUniqueSdBlockId(node, nextAttrs, fallbackSeenIds);
                 if (nodeAllowsSdBlockRevAttr(node)) {
                   nextAttrs.sdBlockRev = getNextBlockRev(node);
                   nodeChanged = true;