fix(open-knowledge): Backspace/Delete at a list-item boundary orphani…

.changeset/list-item-backspace-delete-merge.md

@@ -0,0 +1,5 @@
+---
+"@inkeep/open-knowledge": patch
+---
+
+Fixes `Backspace`/`Delete` at a list-item boundary orphaning rows (inkeep/open-knowledge#609). Merging two list items — e.g. backspacing at the start of a checked task item into the item above, or deleting forward across a nested/top-level boundary — could leave the merged text as a bare paragraph with no bullet or checkbox, or silently re-nest it at the wrong depth. Backspace and Delete now merge the two items' text directly, keeping the surviving item's list marker and nesting intact.

packages/app/src/editor/list-keymap-merge.test.ts

@@ -0,0 +1,204 @@
+/**
+ * Backspace/Delete merge at a listItem boundary (#609 cause 2 — "orphan"
+ * rows from deleting, as distinct from cause 1's paste mis-placement fixed
+ * in handle-paste.list-placement.test.ts).
+ *
+ * StarterKit's ListKeymap sub-extension (@tiptap/extension-list, wired via
+ * shared.ts's `listKeymap` option) targets the fragmented BulletList/
+ * OrderedList/TaskList/TaskItem schema. Against this repo's unified single
+ * `listItem` schema, two of its branches misbehave:
+ *
+ *   - Backspace at the start of an item whose PRECEDING sibling has a nested
+ *     sublist takes `liftListItem` instead of merging text, which lifts the
+ *     merged item clean out of the list into a bare paragraph with no
+ *     bullet/checkbox — the orphan row users see.
+ *   - Delete at the end of the last item in a nested sublist, merging a
+ *     shallower next top-level item, re-nests that item at the wrong depth
+ *     and drops its `checked` attr, silently rewriting document structure.
+ *
+ * list.ts's ListItemNode now binds Backspace/Delete itself (priority 101,
+ * ahead of ListKeymap's default 100) using prosemirror-commands'
+ * `joinTextblockBackward`/`joinTextblockForward` — these descend through
+ * container nodes to the actual textblocks and merge only those, never
+ * re-parenting or dropping the surviving item's attrs, falling back to
+ * `joinBackward`/`joinForward` only when there's no adjacent list item to
+ * join into (first/last item in the list).
+ *
+ * Each test mounts a real TipTap editor over the core schema and dispatches
+ * a real `keydown` through `view.someProp('handleKeyDown', ...)` — the same
+ * path a live keypress takes — rather than calling a PM command directly,
+ * so the assertions cover the actual registered keymap-plugin race.
+ */
+
+import { afterAll, afterEach, beforeAll, describe, expect, test } from 'bun:test';
+import { MarkdownManager, sharedExtensions } from '@inkeep/open-knowledge-core';
+import { Editor, type JSONContent } from '@tiptap/core';
+import { TextSelection } from '@tiptap/pm/state';
+import { installDomGlobals } from './walk-currency-test-harness';
+
+const mdManager = new MarkdownManager({ extensions: sharedExtensions });
+
+let restoreDomGlobals: (() => void) | null = null;
+const editors: Editor[] = [];
+
+beforeAll(() => {
+  restoreDomGlobals = installDomGlobals();
+});
+
+afterAll(() => {
+  restoreDomGlobals?.();
+  restoreDomGlobals = null;
+});
+
+afterEach(() => {
+  while (editors.length > 0) editors.pop()?.destroy();
+});
+
+function mountEditor(md: string): Editor {
+  const host = document.createElement('div');
+  document.body.appendChild(host);
+  const editor = new Editor({
+    element: host,
+    content: mdManager.parse(md) as JSONContent,
+    extensions: [...sharedExtensions],
+  });
+  editors.push(editor);
+  return editor;
+}
+
+/** Doc position at the start or end of the nth listItem's first paragraph. */
+function itemPos(editor: Editor, n: number, where: 'start' | 'end'): number {
+  let i = 0;
+  let found: number | null = null;
+  editor.state.doc.descendants((node, pos) => {
+    if (found !== null) return false;
+    if (node.type.name === 'listItem') {
+      if (i === n) {
+        const para = node.firstChild;
+        found = where === 'start' ? pos + 2 : pos + 2 + (para ? para.content.size : 0);
+        return false;
+      }
+      i++;
+    }
+    return true;
+  });
+  if (found === null) throw new Error(`listItem ${n} not found`);
+  return found;
+}
+
+function setCaret(editor: Editor, pos: number): void {
+  editor.view.dispatch(editor.state.tr.setSelection(TextSelection.create(editor.state.doc, pos)));
+}
+
+function press(editor: Editor, key: string): unknown {
+  const event = new KeyboardEvent('keydown', { key, code: key, bubbles: true, cancelable: true });
+  return editor.view.someProp('handleKeyDown', (f) => f(editor.view, event));
+}
+
+function serialize(editor: Editor): string {
+  return mdManager.serialize(editor.getJSON() as JSONContent);
+}
+
+/** No bare/unmarked line sitting where a list item used to be. */
+function hasOrphanParagraph(md: string): boolean {
+  return /\n\n[a-z]/i.test(md);
+}
+
+const TASK_SEED = ['- [ ] alpha', '- [ ] bravo', '- [x] charlie', '- [ ] delta', '- [ ] echo'].join(
+  '\n',
+);
+
+describe('list keymap — Backspace/Delete merge at an item boundary', () => {
+  test('Backspace at item start merges text into the previous flat item, no orphan', () => {
+    const editor = mountEditor(TASK_SEED);
+    setCaret(editor, itemPos(editor, 2, 'start')); // start of "charlie"
+    expect(press(editor, 'Backspace')).toBe(true);
+
+    const md = serialize(editor);
+    expect(md).toContain('- [ ] bravocharlie');
+    expect(hasOrphanParagraph(md)).toBe(false);
+  });
+
+  test('Backspace merging into a previous item that has a nested sublist keeps the merged text inside the list (no bare orphan paragraph)', () => {
+    const editor = mountEditor('- [ ] top\n  - [ ] child\n- [ ] next');
+    setCaret(editor, itemPos(editor, 2, 'start')); // start of "next" (top-level, after "top"+"child")
+    expect(press(editor, 'Backspace')).toBe(true);
+
+    const md = serialize(editor);
+    // "next" must land inside the list (merged onto the deepest preceding
+    // textblock), never as a bare paragraph outside any list item.
+    expect(hasOrphanParagraph(md)).toBe(false);
+    expect(md).toContain('childnext');
+  });
+
+  test('Delete at the end of a nested item merges the next shallower item at the correct depth, checked attr intact', () => {
+    const editor = mountEditor('- [ ] a\n  - [ ] b\n  - [ ] c\n- [ ] d');
+    setCaret(editor, itemPos(editor, 2, 'end')); // end of "c" (nested, sibling of b)
+    expect(press(editor, 'Delete')).toBe(true);
+
+    const md = serialize(editor);
+    // "d" merges onto "c" at the nested depth — no bare re-nested plain
+    // bullet, no orphan line at the wrong indent.
+    expect(md).toContain('  - [ ] cd');
+    expect(hasOrphanParagraph(md)).toBe(false);
+  });
+
+  test('a ranged selection spanning two items still deletes via the normal path (unaffected)', () => {
+    const editor = mountEditor(TASK_SEED);
+    const from = itemPos(editor, 1, 'start'); // start of "bravo"
+    const to = itemPos(editor, 2, 'start'); // start of "charlie"
+    editor.view.dispatch(
+      editor.state.tr.setSelection(TextSelection.create(editor.state.doc, from, to)),
+    );
+    editor.view.dispatch(editor.state.tr.deleteSelection());
+
+    const md = serialize(editor);
+    expect(md).toContain('- [x] charlie');
+    expect(md).not.toContain('bravo');
+    expect(hasOrphanParagraph(md)).toBe(false);
+  });
+});
+
+describe('list keymap — pre-existing behavior preserved', () => {
+  test('Backspace on the empty line after a list merges back in (no stray bullet)', () => {
+    const editor = mountEditor('- item one\n');
+    setCaret(editor, editor.state.doc.content.size - 1);
+    press(editor, 'Enter'); // empty second item
+    press(editor, 'Enter'); // lifts out to a plain paragraph below the list
+    press(editor, 'Backspace'); // must rejoin the list, not spawn a bullet
+
+    const md = serialize(editor).trim();
+    expect(md).toBe('- item one');
+    expect(md).not.toMatch(/^- *$/m);
+  });
+
+  test('Backspace on an empty nested item removes it (does not toggle the bullet)', () => {
+    const editor = mountEditor('- top\n  - sub\n');
+    setCaret(editor, editor.state.doc.content.size - 1);
+    press(editor, 'Enter'); // empty nested item after "sub"
+    press(editor, 'Backspace'); // removes the empty item, back to "sub"
+
+    const md = serialize(editor);
+    expect(md).toMatch(/^- top\n {2}- sub\n?$/m);
+    expect(md.match(/- sub/g)).toHaveLength(1);
+  });
+
+  test('Backspace at the very start of the first item in a list lifts it out to a plain paragraph', () => {
+    const editor = mountEditor('- alpha\n- bravo\n');
+    setCaret(editor, itemPos(editor, 0, 'start'));
+    press(editor, 'Backspace');
+
+    const md = serialize(editor);
+    expect(md).toContain('alpha');
+    expect(md).toContain('- bravo');
+  });
+
+  test('Enter at end of a task item still creates a new task item', () => {
+    const editor = mountEditor('- [ ] sf\n');
+    setCaret(editor, itemPos(editor, 0, 'end'));
+    press(editor, 'Enter');
+
+    const md = serialize(editor);
+    expect(md).toMatch(/^- \[ \] sf\n- \[ \] ?$/m);
+  });
+});

packages/app/tests/stress/list-keymap.e2e.ts

@@ -343,6 +343,64 @@ test.describe('OQ1: Tab/Shift-Tab scoping by cursor context', () => {
     expect(ytext).not.toMatch(/^- *$/m);
   });
 
+  // #609 cause 2 — deleting/merging list items (as distinct from cause 1's
+  // paste mis-placement, covered by handle-paste.list-placement.test.ts).
+  // StarterKit's ListKeymap (its joinItemBackward/liftListItem branches
+  // target the fragmented BulletList/TaskItem schema) could lift a merged
+  // item clean out of the list into a bare, unmarked paragraph, or re-nest
+  // it at the wrong depth with `checked` dropped. list.ts's ListItemNode
+  // now binds Backspace/Delete itself via joinTextblockBackward/Forward at
+  // priority 101 (ahead of ListKeymap's default 100).
+  test('Backspace merging into a previous item with a nested sublist does not orphan the merged item', async ({
+    page,
+    api,
+  }) => {
+    const docName = uniqueDocName('bksp-merge-sublist');
+    await openDoc(api, page, docName);
+    await seedMarkdown(api, page, docName, '- [ ] top\n  - [ ] child\n- [ ] next\n');
+
+    await page.locator('.ProseMirror:not(.composer-prosemirror)').focus();
+    // Click into "next" — a top-level item whose preceding sibling ("top")
+    // has a nested sublist.
+    await page.locator('.ProseMirror li', { hasText: 'next' }).first().click();
+    await page.keyboard.press('Home');
+    await waitForPmSelectionInNode(page, 'listItem');
+
+    await page.keyboard.press('Backspace');
+
+    // "next" must merge as text into the list (onto "child", the deepest
+    // preceding textblock) — never survive as a bare paragraph with no
+    // bullet/checkbox sitting outside any list item.
+    await expect.poll(() => getYText(page)).toContain('childnext');
+    const ytext = await getYText(page);
+    expect(ytext).not.toMatch(/\n\nnext/);
+    expect(ytext).toContain('- [ ] top');
+  });
+
+  test('Delete at the end of a nested item merges the next top-level item at the correct depth', async ({
+    page,
+    api,
+  }) => {
+    const docName = uniqueDocName('del-merge-depth');
+    await openDoc(api, page, docName);
+    await seedMarkdown(api, page, docName, '- [ ] a\n  - [ ] b\n  - [ ] c\n- [ ] d\n');
+
+    await page.locator('.ProseMirror:not(.composer-prosemirror)').focus();
+    await page.locator('.ProseMirror li', { hasText: /^c$/ }).first().click();
+    await page.keyboard.press('End');
+    await waitForPmSelectionInNode(page, 'listItem');
+
+    await page.keyboard.press('Delete');
+
+    // "d" merges onto "c" at the nested depth, keeping "c"'s checkbox on the
+    // surviving item and its indentation — not re-nested as a bare bullet
+    // at the wrong depth.
+    await expect.poll(() => getYText(page)).toMatch(/^ {2}- \[ \] cd$/m);
+    const ytext = await getYText(page);
+    expect(ytext).toContain('- [ ] a');
+    expect(ytext).not.toMatch(/^- d$/m);
+  });
+
   test.fixme('Tab inside a codeBlock inserts 2 spaces', async ({ page, api }) => {
     const docName = uniqueDocName('tab-codeblock');
     await openDoc(api, page, docName);

packages/core/src/extensions/list.ts

@@ -19,6 +19,13 @@
  */
 
 import { findParentNode, InputRule, mergeAttributes, Node, wrappingInputRule } from '@tiptap/core';
+import {
+  chainCommands,
+  joinTextblockBackward,
+  joinTextblockForward,
+  joinBackward as pmJoinBackward,
+  joinForward as pmJoinForward,
+} from '@tiptap/pm/commands';
 import type { NodeType, Node as PmNode } from '@tiptap/pm/model';
 import { liftListItem as pmLiftListItem, wrapInList as pmWrapInList } from '@tiptap/pm/schema-list';
 import type { EditorState, Transaction } from '@tiptap/pm/state';
@@ -315,8 +322,18 @@ export const ListNode = Node.create({
 // default priority (100) matches stock TipTap and lets our splitListItem
 // run first; a previous `priority: 60` here regressed Enter on every list
 // type.
+//
+// priority: 101 — one above default so this extension's Backspace/Delete
+// (joinTextblockBackward/Forward, see addKeyboardShortcuts) run before
+// StarterKit's ListKeymap sub-extension (also default 100, registered later
+// in sharedExtensions but stable-sorted ahead of same-priority extensions
+// added earlier). ListKeymap's joinItemBackward/liftListItem branches target
+// the fragmented BulletList/TaskItem schema; against this unified listItem
+// schema they can lift a same-depth sibling clean out of the list (issue
+// #609 orphan rows) before this extension's handler ever runs.
 export const ListItemNode = Node.create({
   name: 'listItem',
+  priority: 101,
   content: 'paragraph block*',
   defining: true,
 
@@ -458,6 +475,38 @@ export const ListItemNode = Node.create({
   addKeyboardShortcuts() {
     return {
       Enter: () => this.editor.commands.splitListItem(this.name),
+      // Backspace/Delete at a listItem boundary — bypass the upstream
+      // ListKeymap extension's joinItemBackward/joinItemForward and
+      // has-sublist/depth-mismatch branches (@tiptap/extension-list, wired
+      // via StarterKit's `listKeymap` option in shared.ts). Those branches
+      // assume the fragmented BulletList/OrderedList/TaskList/TaskItem
+      // schema; against this unified single-`listItem` schema they can
+      // `liftListItem` a same-depth sibling clean out of the list entirely
+      // (previous item has a nested sublist → the merged-in item becomes a
+      // bare paragraph with no bullet) or re-nest a merged item at the wrong
+      // depth with its `checked` attr dropped (Delete across a depth
+      // change) — both read by users as "orphan" list rows (issue #609).
+      //
+      // `joinTextblockBackward`/`joinTextblockForward` (prosemirror-commands)
+      // are the general-purpose fix: at a boundary between two container
+      // nodes (listItem/listItem, or listItem/nested-list), they descend via
+      // lastChild/firstChild to the actual textblocks and merge only those —
+      // never re-parenting or dropping the surviving item's attrs. Only
+      // engage them when the cursor is inside a listItem so plain-document
+      // Backspace/Delete (paragraphs, headings, etc.) keeps the stock
+      // joinBackward/joinForward behavior. Falling back to joinBackward/
+      // joinForward keeps the existing lift-out-of-list behavior for the
+      // first/last item in a list (no adjacent item to join into).
+      Backspace: () => {
+        if (!isCursorInsideListItem(this.editor.state)) return false;
+        const { state, view } = this.editor;
+        return chainCommands(joinTextblockBackward, pmJoinBackward)(state, view.dispatch, view);
+      },
+      Delete: () => {
+        if (!isCursorInsideListItem(this.editor.state)) return false;
+        const { state, view } = this.editor;
+        return chainCommands(joinTextblockForward, pmJoinForward)(state, view.dispatch, view);
+      },
       Tab: () => {
         // Only handle Tab when the cursor is inside a listItem — otherwise
         // pass through so other extensions (e.g., table) can handle it.
@@ -482,6 +531,15 @@ export const ListItemNode = Node.create({
   },
 });
 
+/** True when the selection's anchor sits inside a `listItem` ancestor. */
+function isCursorInsideListItem(state: EditorState): boolean {
+  const { $from } = state.selection;
+  for (let d = $from.depth; d > 0; d--) {
+    if ($from.node(d).type.name === 'listItem') return true;
+  }
+  return false;
+}
+
 /**
  * Combined export for registration in shared.ts.
  * Register both ListNode and ListItemNode to get the full list experience.