[lexical-table] Bug Fix: insert column after the correct cell in rows covered by a rowSpan

[LeSingh1]

Description

$insertTableColumnAtNode walks left from the insertion column to find the cell in each row to insert after. The walk steps by a fixed stride:

prevCellIndex -= currentCell.__colSpan;

currentCell never changes inside the loop, so it always steps by the anchor cell's colSpan. When that is greater than 1 the walk jumps over grid columns it never inspects, and lands on the wrong cell — or falls off the left edge and appends to the end of the row.

Two failure modes, both reproducible:

row0: [A, B, V(rowSpan=2), X(rowSpan=2, colSpan=2)]   // cols A=0 B=1 V=2 X=3-4
row1: [C0, C1]                                        // cols 0-1, 2-4 covered
$insertTableColumnAtNode(X, true)                     // insertAfterColumn = 4

The walk goes 4 → 2 → 0, skipping C1, so row1 becomes ['C0', '', 'V', 'X', 'X', 'C1'] instead of ['C0', 'C1', 'V', 'X', 'X', ''].

row0: [P, A(rowSpan=2), C(rowSpan=2, colSpan=2)]      // cols P=0 A=1 C=2-3
row1: [B]                                             // col 0, cols 1-3 covered
$insertTableColumnAtNode(C, true)                     // insertAfterColumn = 3

The walk goes 3 → 1 → -1 and appends, even though row1 owns B at column 0.

Fix

Derive the insertion point from the row map the way $unmergeCellNode already does in this file: scan rowMap from column 0 up to insertAfterColumn, skipping by each entry's colSpan, and keep the last entry whose startRow is this row. That entry is the cell to insert after; when there is none, every column to the left is covered by a rowSpan from an earlier row, so the new cell is the row's first child and is added with $insertFirst.

The prepend/append distinction now falls out of the scan instead of being a special case, and the labeled continue rowLoop is gone. The setColSpan branch is unchanged.

Test plan

Three tests in LexicalTableUtils.test.ts, using the file's shared buildEditorFromExtensions editor: the original rowSpan=2 alignment case plus both grids above.

Teeth-checked by reverting only LexicalTableUtils.ts:

product code	result
main	2 failed / 39 passed
the earlier $insertFirst-only version of this PR	2 failed / 39 passed (the other two)
this fix	41 passed

packages/lexical-table is 151/151; the full unit suite is 4969 passed / 1 skipped; tsc --noEmit, eslint and prettier clean.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
lexical	Ready	Preview	Aug 5, 2026 5:41am
lexical-playground	Ready	Preview	Aug 5, 2026 5:41am

[mayrang]

The fix looks right — $insertFirst matches the existing guard a few lines above (insertAfterColumn < 0) and the $getGridTexts helper makes the column-alignment assertion easy to read. Left a small note on the test pattern inline.

[mayrang]

Nit: since this is a new describe block, it might be worth using buildEditorFromExtensions + using editor — handles cleanup automatically and the codebase is moving in this direction. The same package already uses it in TableImportExtension.test.ts. Something like:

const tableExtension = defineExtension({
  name: 'test',
  nodes: [TableNode, TableCellNode, TableRowNode],
});

test('inserts the new cell...', () => {
  using editor = buildEditorFromExtensions(tableExtension);
  editor.update(() => { /* setup */ }, { discrete: true });
  editor.update(() => { /* action */ }, { discrete: true });
  editor.read(() => { /* assertions */ });
});

Not a blocker — the fix itself is correct.

[etrepum]

Here's a Claude review, it looks like there are some additional edge cases that could be addressed here (unedited claude output below)

---

Thanks for the fix — the reported case is real and the regression test genuinely fails on main and passes with this change (verified locally). However, the new $insertFirst branch relies on a leftward walk that is itself buggy, and that makes this fix a regression for a neighboring class of grids. Findings ranked by severity, all verified by running counterexamples against this PR's head:

1. 🔴 Regression: the prepend fires spuriously when the walk skips a real row-start cell (LexicalTableUtils.ts:569-576)

The walk above the changed branch decrements by a fixed stride:

prevCellIndex -= currentCell.__colSpan; // L563: always the ORIGINAL cell's span

currentCell never changes inside the loop, so when the anchor cell has colSpan > 1, the walk steps by that width past cells it never inspected — including cells whose startRow === i. It can then fall off the left edge (prevCellIndex < 0) even though the row does own a cell left of the insertion column. The new comment ("Every cell to the left in this row is covered by a rowSpan") is not true in that case, and prepending corrupts the grid — while the old append happened to be correct there.

Verified repro (passes on main, fails with this PR):

row0: [P, A(rowSpan=2), C(rowSpan=2, colSpan=2)]   → grid cols: P=0, A=1, C=2-3
row1: [B]                                          → grid: B=0, cols 1-3 covered
$insertTableColumnAtNode(C, true)                  → insertAfterColumn = 3

Walk for row 1: index 3 (C) → 3 - 2 = 1 (A) → 1 - 2 = -1 — it skips B at column 0 because it subtracts C's colSpan (2) instead of A's (1). Result:

	before this PR	with this PR
row1 grid	['B', 'A', 'C', 'C', ''] ✅	['', 'A', 'C', 'C', 'B'] ❌ (new cell steals column 0, B pushed to column 4)

2. 🔴 Root cause left unfixed: the walk lands on the wrong cell even when it doesn't hit −1 (LexicalTableUtils.ts:563)

The same fixed-stride bug misplaces the insertion without ever reaching the patched branch. Verified repro (fails identically before and after this PR):

row0: [A, B, V(rowSpan=2), X(rowSpan=2, colSpan=2)]  → cols A=0, B=1, V=2, X=3-4
row1: [C0, C1]                                       → cols 0, 1; cols 2-4 covered
$insertTableColumnAtNode(X, true)                    → insertAfterColumn = 4

Walk for row 1: 4 (X) → 4 - 2 = 2 (V) → 2 - 2 = 0 (C0) — skipping C1 at index 1. The new cell is inserted after C0, so row 1 becomes ['C0', '', 'V', 'X', 'X', 'C1'] instead of ['C0', 'C1', 'V', 'X', 'X', ''].

So fixing only the else branch patches one symptom of the walk while the walk itself still mis-traverses.

3. 🟡 Suggested fix: derive the insertion point from the map, like $unmergeCellNode already does

This exact problem — "find the rightmost cell owned by row i left of a column, or prepend if there is none" — is already solved correctly in the same file by $unmergeCellNode (~L1007-1035): scan rowMap[i] left-to-right below the target column, track the last entry with startRow === i as insertAfterCell, and use $insertFirst when it stays null, insertAfter otherwise. Rewriting this walk in that shape fixes #1 and #2 at once, makes the prepend/append distinction fall out naturally instead of being a special case, and removes the labeled continue rowLoop. Something like:

let insertAfterCell: null | TableCellNode = null;
for (let column = 0; column <= insertAfterColumn; column++) {
  const entry = rowMap[column];
  if (entry.startRow === i) {
    insertAfterCell = entry.cell;
  }
  column += entry.cell.__colSpan - 1;
}
if (insertAfterCell === null) {
  $insertFirst(loopRow, $createTableCellNodeForInsertTableColumn(headerState));
} else {
  insertAfterCell.insertAfter($createTableCellNodeForInsertTableColumn(headerState));
}

(modulo the existing currentStartColumn + colSpan - 1 > insertAfterColumn → setColSpan case, which stays as-is).

4. 🟡 Test coverage: add a colSpan > 1 covering-cell variant

The new test only exercises all-colSpan=1 grids, so it stays green while repro #1 fails. Adding the #1 grid (or any variant where the covering rowSpan cell has colSpan ≥ 2 and the spanned row still owns a cell) to the new describe block would have caught this.

5. 🔵 Heads-up: this test file was refactored on main and this PR will conflict

main commit 782b6e9 (#8833) rewrote LexicalTableUtils.test.ts to a single shared buildEditorFromExtensions(defineExtension({dependencies: [TableExtension], ...})) setup with afterEach(() => editor.dispose()) and $assertNodeType helpers. This PR (based on an older main) adds a fourth copy of the createEditor + editor._headless = true boilerplate and will conflict on rebase. After rebasing, the new describe block can drop its local beforeEach entirely and use the shared editor + $assertNodeType($getRoot().getFirstChild(), $isTableNode) like the rest of the file (as @mayrang also suggested).

---

Summary: the direction is right — prepending is correct when the row is genuinely fully covered — but as written the change trades one wrong-alignment case for another. Fixing the walk at L563 (ideally by adopting the $unmergeCellNode scan pattern) makes the prepend sound, and a colSpan > 1 test variant will lock it in.

[LeSingh1]

@etrepum @mayrang — both repros were real, thanks for running them.

Rebased onto main and replaced the leftward walk with the $unmergeCellNode scan: walk rowMap from column 0 to insertAfterColumn skipping by each entry's colSpan, keep the last entry with startRow === i, then insertAfter it or $insertFirst when there is none. The setColSpan branch is unchanged and the labeled continue is gone. Test block now uses the shared buildEditorFromExtensions editor and $assertNodeType.

Both of your grids are now tests alongside the original one, and I retitled the PR since the fix is no longer just the prepend case.

Teeth-checked by reverting only LexicalTableUtils.ts: on main 2 of the 3 fail (repro 1 passes there, as you predicted); with the old $insertFirst hunk the other 2 fail. All 3 pass with the scan. packages/lexical-table is 151/151, full unit suite 4969 passed / 1 skipped, tsc and lint clean.