fix(super-editor): guard cached paragraph props lookup in indent commands (SD-3083)

packages/super-editor/src/editors/v1/core/commands/textIndent.js

@@ -1,4 +1,7 @@
-import { getResolvedParagraphProperties } from '@extensions/paragraph/resolvedPropertiesCache.js';
+import {
+  getResolvedParagraphProperties,
+  calculateResolvedParagraphProperties,
+} from '@extensions/paragraph/resolvedPropertiesCache.js';
 import { ptToTwips } from '@converter/helpers';
 
 const defaultIncrementPoints = 36;
@@ -12,7 +15,10 @@ const defaultIncrementPoints = 36;
  * @note Increments by the default value (36 points by default)
  * @note Creates initial indent if none exists
  */
-export const increaseTextIndent = () => modifyIndentation((node) => calculateNewIndentation(node, 1));
+export const increaseTextIndent = () =>
+  modifyIndentation((node, resolvedProps) => calculateNewIndentation(node, 1, resolvedProps), {
+    resolveProps: true,
+  });
 
 /**
  * Decrease text indentation
@@ -23,7 +29,10 @@ export const increaseTextIndent = () => modifyIndentation((node) => calculateNew
  * @note Decrements by the default value (36 points by default)
  * @note Removes indentation completely if it reaches 0 or below
  */
-export const decreaseTextIndent = () => modifyIndentation((node) => calculateNewIndentation(node, -1));
+export const decreaseTextIndent = () =>
+  modifyIndentation((node, resolvedProps) => calculateNewIndentation(node, -1, resolvedProps), {
+    resolveProps: true,
+  });
 
 /**
  * Set text indentation
@@ -42,7 +51,7 @@ export const setTextIndentation = (points) => modifyIndentation(() => ptToTwips(
  * @category Command
  * @returns {Function} Command function
  * @example
- * unsetTextIndent()
+ * unsetTextIndentation()
  * @note Removes inline indentation from the selected nodes
  */
 export const unsetTextIndentation = () => modifyIndentation(() => null);
@@ -51,10 +60,11 @@ export const unsetTextIndentation = () => modifyIndentation(() => null);
  * Calculate new indentation based on delta
  * @param {import('prosemirror-model').Node} node - The paragraph node
  * @param {number} delta - The delta to apply (positive to increase, negative to decrease)
+ * @param {object} resolvedProps - Resolved paragraph properties (cache hit or freshly computed)
  * @returns {number|null} New left indentation in twips, or null if no indentation
  */
-function calculateNewIndentation(node, delta) {
-  let { indent } = getResolvedParagraphProperties(node);
+function calculateNewIndentation(node, delta, resolvedProps) {
+  let { indent } = resolvedProps ?? {};
   let { left } = indent || {};
 
   const increment = ptToTwips(delta * defaultIncrementPoints);
@@ -72,18 +82,33 @@ function calculateNewIndentation(node, delta) {
 
 /** * Modify indentation of selected paragraph nodes
  * @param {Function} calcFunc - Function to calculate new indentation
+ * @param {object} [options]
+ * @param {boolean} [options.resolveProps=false] - When true, resolve paragraph
+ *   properties (cache hit or compute on miss) and pass them to calcFunc. Only
+ *   needed by commands that read the current indent (increase/decrease).
  * @returns {Function} Command function
  */
-function modifyIndentation(calcFunc) {
-  return ({ state, dispatch }) => {
+function modifyIndentation(calcFunc, { resolveProps = false } = {}) {
+  return ({ editor, state, dispatch }) => {
     const tr = state.tr;
 
     const { from, to } = state.selection;
     const results = [];
 
     state.doc.nodesBetween(from, to, (node, pos) => {
       if (node.type.name === 'paragraph') {
-        let left = calcFunc(node);
+        // For increase/decrease, we need the resolved value (style cascade
+        // included) to compute a correct delta. The cache misses on paragraphs
+        // the rendering pass hasn't visited yet (fresh load, freshly-inserted
+        // nodes), so fall back to computing on demand. A bare `?? {}` guard
+        // would stop the crash but silently drop any style-derived indent
+        // baseline, turning a crash into a wrong delta.
+        const resolvedProps = resolveProps
+          ? getResolvedParagraphProperties(node) ||
+            calculateResolvedParagraphProperties(editor ?? {}, node, state.doc.resolve(pos))
+          : undefined;
+
+        let left = calcFunc(node, resolvedProps);
         if (Number.isNaN(left)) {
           results.push(false);
           return false;

packages/super-editor/src/editors/v1/core/commands/textIndent.test.js

@@ -3,11 +3,15 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { Schema } from 'prosemirror-model';
 import { EditorState, TextSelection } from 'prosemirror-state';
 import { increaseTextIndent, decreaseTextIndent, setTextIndentation, unsetTextIndentation } from './textIndent.js';
-import { getResolvedParagraphProperties } from '@extensions/paragraph/resolvedPropertiesCache.js';
+import {
+  getResolvedParagraphProperties,
+  calculateResolvedParagraphProperties,
+} from '@extensions/paragraph/resolvedPropertiesCache.js';
 import { ptToTwips } from '@converter/helpers';
 
 vi.mock('@extensions/paragraph/resolvedPropertiesCache.js', () => ({
   getResolvedParagraphProperties: vi.fn((node) => node.attrs.paragraphProperties || {}),
+  calculateResolvedParagraphProperties: vi.fn((_editor, node) => node.attrs.paragraphProperties || {}),
 }));
 
 vi.mock('@converter/helpers', () => ({
@@ -38,9 +42,10 @@ const createState = (paragraphAttrs) => {
   return EditorState.create({ doc, selection });
 };
 
-const runCommand = (command, state) => {
+const runCommand = (command, state, editor = {}) => {
   let nextState = state;
   const dispatched = command({
+    editor,
     state,
     dispatch: (tr) => {
       nextState = state.apply(tr);
@@ -90,4 +95,83 @@ describe('text indent commands', () => {
     const finalNode = afterUnset.doc.firstChild;
     expect(finalNode.attrs.paragraphProperties.indent).toBeUndefined();
   });
+
+  describe('uncached paragraphs (cache miss fallback)', () => {
+    it('increaseTextIndent does not throw and applies a default increment', () => {
+      const state = createState({ paragraphProperties: {} });
+      getResolvedParagraphProperties.mockReturnValueOnce(undefined);
+      calculateResolvedParagraphProperties.mockReturnValueOnce({});
+
+      expect(() => runCommand(increaseTextIndent(), state)).not.toThrow();
+      expect(calculateResolvedParagraphProperties).toHaveBeenCalledTimes(1);
+    });
+
+    it('decreaseTextIndent does not throw and clears indent when no resolved indent exists', () => {
+      const state = createState({ paragraphProperties: {} });
+      getResolvedParagraphProperties.mockReturnValueOnce(undefined);
+      calculateResolvedParagraphProperties.mockReturnValueOnce({});
+
+      const { dispatched, nextState } = runCommand(decreaseTextIndent(), state);
+
+      expect(dispatched).toBe(true);
+      const updated = nextState.doc.firstChild;
+      expect(updated.attrs.paragraphProperties.indent).toBeUndefined();
+    });
+
+    it('setTextIndentation and unsetTextIndentation skip the resolve fallback entirely', () => {
+      const state = createState({ paragraphProperties: {} });
+
+      runCommand(setTextIndentation(10), state);
+      runCommand(unsetTextIndentation(), state);
+
+      expect(calculateResolvedParagraphProperties).not.toHaveBeenCalled();
+      expect(getResolvedParagraphProperties).not.toHaveBeenCalled();
+    });
+
+    it('increaseTextIndent honors style-derived indent on cache miss', () => {
+      // Paragraph has no inline indent; the style cascade resolves to a
+      // 36pt left indent. Increase should produce 36pt + 36pt, not 36pt
+      // alone (which would silently drop the inherited baseline).
+      const inheritedLeft = ptToTwips(36);
+      const state = createState({ paragraphProperties: {} });
+      getResolvedParagraphProperties.mockReturnValueOnce(undefined);
+      calculateResolvedParagraphProperties.mockReturnValueOnce({ indent: { left: inheritedLeft } });
+
+      const { dispatched, nextState } = runCommand(increaseTextIndent(), state);
+
+      expect(dispatched).toBe(true);
+      const updated = nextState.doc.firstChild;
+      expect(updated.attrs.paragraphProperties.indent.left).toBe(inheritedLeft + ptToTwips(36));
+    });
+
+    it('decreaseTextIndent honors style-derived indent on cache miss', () => {
+      // Symmetric to the increase case. A paragraph inheriting 72pt should
+      // decrement to 36pt - not clear, which would happen if the fallback
+      // dropped the inherited baseline and reduced from 0.
+      const inheritedLeft = ptToTwips(72);
+      const state = createState({ paragraphProperties: {} });
+      getResolvedParagraphProperties.mockReturnValueOnce(undefined);
+      calculateResolvedParagraphProperties.mockReturnValueOnce({ indent: { left: inheritedLeft } });
+
+      const { dispatched, nextState } = runCommand(decreaseTextIndent(), state);
+
+      expect(dispatched).toBe(true);
+      const updated = nextState.doc.firstChild;
+      expect(updated.attrs.paragraphProperties.indent.left).toBe(inheritedLeft - ptToTwips(36));
+    });
+
+    it('cache hit short-circuits the compute-on-miss fallback', () => {
+      // Inverse of the set/unset opt-out test. Verifies the production
+      // `||` short-circuit: when the cache is populated, the fallback
+      // must not run. A future refactor that always computes (e.g. for
+      // freshness) would silently double the work and break this guard.
+      const state = createState({ paragraphProperties: {} });
+      getResolvedParagraphProperties.mockReturnValueOnce({ indent: { left: ptToTwips(36) } });
+
+      runCommand(increaseTextIndent(), state);
+
+      expect(getResolvedParagraphProperties).toHaveBeenCalledTimes(1);
+      expect(calculateResolvedParagraphProperties).not.toHaveBeenCalled();
+    });
+  });
 });

tests/behavior/tests/toolbar/paragraph-indent.spec.ts

@@ -0,0 +1,53 @@
+import { test, expect, type SuperDocFixture } from '../../fixtures/superdoc.js';
+
+test.use({ config: { toolbar: 'full', showSelection: true } });
+
+async function getFirstParagraphIndentLeft(superdoc: SuperDocFixture): Promise<number | undefined> {
+  return superdoc.page.evaluate(() => {
+    const editor = (window as any).editor;
+    return editor?.state?.doc?.firstChild?.attrs?.paragraphProperties?.indent?.left;
+  });
+}
+
+test('Increase Indent toolbar button on a fresh paragraph adds indent without crashing', async ({ superdoc }) => {
+  await superdoc.type('Indent me');
+  await superdoc.waitForStable();
+
+  expect(await getFirstParagraphIndentLeft(superdoc)).toBeUndefined();
+
+  await superdoc.page.locator('[data-item="btn-indentright"]').click();
+  await superdoc.waitForStable();
+
+  const left = await getFirstParagraphIndentLeft(superdoc);
+  expect(typeof left).toBe('number');
+  expect(left).toBeGreaterThan(0);
+});
+
+test('Decrease Indent removes the indent applied by Increase Indent', async ({ superdoc }) => {
+  await superdoc.type('Round trip');
+  await superdoc.waitForStable();
+
+  await superdoc.page.locator('[data-item="btn-indentright"]').click();
+  await superdoc.waitForStable();
+  expect(await getFirstParagraphIndentLeft(superdoc)).toBeGreaterThan(0);
+
+  await superdoc.page.locator('[data-item="btn-indentleft"]').click();
+  await superdoc.waitForStable();
+  expect(await getFirstParagraphIndentLeft(superdoc)).toBeUndefined();
+});
+
+test('Repeated Increase Indent compounds the left indent', async ({ superdoc }) => {
+  await superdoc.type('Compounding');
+  await superdoc.waitForStable();
+
+  await superdoc.page.locator('[data-item="btn-indentright"]').click();
+  await superdoc.waitForStable();
+  const afterOne = await getFirstParagraphIndentLeft(superdoc);
+
+  await superdoc.page.locator('[data-item="btn-indentright"]').click();
+  await superdoc.waitForStable();
+  const afterTwo = await getFirstParagraphIndentLeft(superdoc);
+
+  expect(afterOne).toBeGreaterThan(0);
+  expect(afterTwo).toBeGreaterThan(afterOne!);
+});