fix(converter): preserve theme rFonts on DOCX round-trip (SD-2894)

packages/layout-engine/style-engine/src/cascade.test.ts

@@ -152,6 +152,109 @@ describe('cascade - combineRunProperties', () => {
       bold: true,
     });
   });
+
+  // SD-2894: each `<w:rFonts>` script slot has both a concrete form (`w:ascii`,
+  // `w:hAnsi`, `w:eastAsia`, `w:cs`) and a theme form (`w:asciiTheme`,
+  // `w:hAnsiTheme`, `w:eastAsiaTheme`, `w:cstheme` — note `cstheme` lowercase).
+  // When a higher-priority source supplies one form for a slot, the cascade must
+  // drop the other form from lower-priority sources, or Word resolves the concrete
+  // name as an override and defeats per-script theme resolution.
+  //
+  // The original SD-2894 bug was that only the (ascii, asciiTheme) pair was
+  // dropping correctly; hAnsi/eastAsia/cs leaked through. These tests pin the
+  // full 4-slot dedup so the bug cannot regress for any single slot.
+  describe('SD-2894 four-slot theme/concrete dedup', () => {
+    it('drops concrete `ascii` from lower when higher supplies `asciiTheme`', () => {
+      const result = combineRunProperties([
+        { fontFamily: { ascii: 'Calibri' } },
+        { fontFamily: { asciiTheme: 'majorBidi' } },
+      ]);
+      expect(result.fontFamily).toEqual({ asciiTheme: 'majorBidi' });
+    });
+
+    it('drops concrete `hAnsi` from lower when higher supplies `hAnsiTheme`', () => {
+      const result = combineRunProperties([
+        { fontFamily: { hAnsi: 'Calibri' } },
+        { fontFamily: { hAnsiTheme: 'majorHAnsi' } },
+      ]);
+      expect(result.fontFamily).toEqual({ hAnsiTheme: 'majorHAnsi' });
+    });
+
+    it('drops concrete `eastAsia` from lower when higher supplies `eastAsiaTheme`', () => {
+      const result = combineRunProperties([
+        { fontFamily: { eastAsia: 'SimSun' } },
+        { fontFamily: { eastAsiaTheme: 'majorEastAsia' } },
+      ]);
+      expect(result.fontFamily).toEqual({ eastAsiaTheme: 'majorEastAsia' });
+    });
+
+    it('drops concrete `cs` from lower when higher supplies `cstheme` (lowercase)', () => {
+      const result = combineRunProperties([
+        { fontFamily: { cs: 'Arial' } },
+        { fontFamily: { cstheme: 'majorBidi' } },
+      ]);
+      expect(result.fontFamily).toEqual({ cstheme: 'majorBidi' });
+    });
+
+    it('drops theme `asciiTheme` from lower when higher supplies concrete `ascii`', () => {
+      const result = combineRunProperties([
+        { fontFamily: { asciiTheme: 'majorBidi' } },
+        { fontFamily: { ascii: 'Arial' } },
+      ]);
+      expect(result.fontFamily).toEqual({ ascii: 'Arial' });
+    });
+
+    it('drops theme `hAnsiTheme` from lower when higher supplies concrete `hAnsi`', () => {
+      const result = combineRunProperties([
+        { fontFamily: { hAnsiTheme: 'majorHAnsi' } },
+        { fontFamily: { hAnsi: 'Arial' } },
+      ]);
+      expect(result.fontFamily).toEqual({ hAnsi: 'Arial' });
+    });
+
+    it('drops theme `eastAsiaTheme` from lower when higher supplies concrete `eastAsia`', () => {
+      const result = combineRunProperties([
+        { fontFamily: { eastAsiaTheme: 'majorEastAsia' } },
+        { fontFamily: { eastAsia: 'Arial' } },
+      ]);
+      expect(result.fontFamily).toEqual({ eastAsia: 'Arial' });
+    });
+
+    it('drops theme `cstheme` from lower when higher supplies concrete `cs`', () => {
+      const result = combineRunProperties([
+        { fontFamily: { cstheme: 'majorBidi' } },
+        { fontFamily: { cs: 'Arial' } },
+      ]);
+      expect(result.fontFamily).toEqual({ cs: 'Arial' });
+    });
+
+    it('Athenaintelligence customer shape: all 4 concretes from defaults dropped by inline themes', () => {
+      // Mirrors the customer fixture: docDefaults supply concrete fonts (Arial),
+      // inline rPr supplies theme refs on ascii/hAnsi/cs (no eastAsiaTheme). The
+      // cascade must keep only the theme refs on those three slots; eastAsia
+      // concrete from defaults is independent.
+      const result = combineRunProperties([
+        { fontFamily: { ascii: 'Arial', hAnsi: 'Arial', cs: 'Arial', eastAsia: 'Arial' } },
+        { fontFamily: { asciiTheme: 'majorBidi', hAnsiTheme: 'majorBidi', cstheme: 'majorBidi' } },
+      ]);
+      expect(result.fontFamily).toEqual({
+        asciiTheme: 'majorBidi',
+        hAnsiTheme: 'majorBidi',
+        cstheme: 'majorBidi',
+        eastAsia: 'Arial',
+      });
+    });
+
+    it('exports FONT_SLOT_THEME_PAIRS so callers (super-editor plugin) can stay in sync', async () => {
+      const { FONT_SLOT_THEME_PAIRS } = await import('./cascade.js');
+      expect(FONT_SLOT_THEME_PAIRS).toEqual([
+        ['ascii', 'asciiTheme'],
+        ['hAnsi', 'hAnsiTheme'],
+        ['eastAsia', 'eastAsiaTheme'],
+        ['cs', 'cstheme'],
+      ]);
+    });
+  });
 });
 
 describe('cascade - combineIndentProperties', () => {

packages/layout-engine/style-engine/src/cascade.ts

@@ -104,20 +104,51 @@ function isObject(item: unknown): item is PropertyObject {
  * @param propertiesArray - Ordered list of run property objects.
  * @returns Combined run property object.
  */
+// SD-2894: `<w:rFonts>` exposes four independent script slots (ascii / hAnsi / eastAsia / cs).
+// Each slot can be filled either by a concrete font name (`w:ascii="Calibri"`) or a theme
+// reference (`w:asciiTheme="majorBidi"`). When a higher-priority style supplies one form for a
+// slot, the other form from lower-priority sources is no longer applicable and must be
+// dropped — otherwise the merged run carries both, and Word resolves the concrete name as an
+// override that defeats per-script theme resolution (Latin body text mapped to `majorBidi`
+// then renders as Calibri Light instead of Times New Roman for Hebrew/Arab).
+//
+// Pair `<slot>` with `<slot>Theme`, except for the cs slot whose theme attribute is the
+// lowercase `cstheme` (OOXML inconsistency, not a typo).
+// Exported so super-editor's calculateInlineRunPropertiesPlugin can consume the same
+// list instead of duplicating it (SD-2894 follow-up).
+export const FONT_SLOT_THEME_PAIRS: Array<[keyof RunFontFamilyProperties, keyof RunFontFamilyProperties]> = [
+  ['ascii', 'asciiTheme'],
+  ['hAnsi', 'hAnsiTheme'],
+  ['eastAsia', 'eastAsiaTheme'],
+  ['cs', 'cstheme'],
+];
+
+function dropConflictingFontSlots(
+  target: RunFontFamilyProperties,
+  source: RunFontFamilyProperties,
+): RunFontFamilyProperties {
+  const result = { ...target };
+  for (const [concreteKey, themeKey] of FONT_SLOT_THEME_PAIRS) {
+    if (source[themeKey] != null) {
+      delete result[concreteKey];
+      delete result[themeKey];
+    } else if (source[concreteKey] != null) {
+      delete result[themeKey];
+    }
+  }
+  return result;
+}
+
 export function combineRunProperties(propertiesArray: RunProperties[]): RunProperties {
   return combineProperties(propertiesArray, {
     fullOverrideProps: ['color'],
     specialHandling: {
       fontFamily: (target: Record<string, unknown>, source: Record<string, unknown>): unknown => {
         const fontFamilySource = { ...(source.fontFamily as object) } as RunFontFamilyProperties;
-        const fontFamilyTarget = { ...(target.fontFamily as object) } as RunFontFamilyProperties;
-        if (fontFamilySource.asciiTheme != null) {
-          delete fontFamilyTarget.ascii;
-          delete fontFamilyTarget.asciiTheme;
-        }
-        if (fontFamilySource.ascii != null) {
-          delete fontFamilyTarget.asciiTheme;
-        }
+        const fontFamilyTarget = dropConflictingFontSlots(
+          (target.fontFamily as RunFontFamilyProperties) ?? {},
+          fontFamilySource,
+        );
         return { ...(fontFamilyTarget as object), ...(fontFamilySource as object) };
       },
     },

packages/layout-engine/style-engine/src/ooxml/index.ts

@@ -5,7 +5,12 @@
  * This module is format-aware (docx), but translator-agnostic.
  */
 
-import { combineIndentProperties, combineProperties, combineRunProperties } from '../cascade.js';
+import {
+  combineIndentProperties,
+  combineProperties,
+  combineRunProperties,
+  FONT_SLOT_THEME_PAIRS,
+} from '../cascade.js';
 import type { PropertyObject } from '../cascade.js';
 import type { ParagraphConditionalFormatting, ParagraphProperties, ParagraphTabStop, RunProperties } from './types.ts';
 import type { NumberingProperties } from './numbering-types.ts';
@@ -18,7 +23,7 @@ import type {
   TableCellProperties,
 } from './styles-types.ts';
 
-export { combineIndentProperties, combineProperties, combineRunProperties };
+export { combineIndentProperties, combineProperties, combineRunProperties, FONT_SLOT_THEME_PAIRS };
 export type { PropertyObject };
 export type * from './types.ts';
 export type * from './numbering-types.ts';

packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js

@@ -2,6 +2,7 @@ import { Plugin, TextSelection } from 'prosemirror-state';
 import { Fragment } from 'prosemirror-model';
 import { TableMap } from 'prosemirror-tables';
 import { decodeRPrFromMarks, encodeMarksFromRPr, resolveRunProperties } from '@converter/styles.js';
+import { FONT_SLOT_THEME_PAIRS } from '@superdoc/style-engine/ooxml';
 import {
   calculateResolvedParagraphProperties,
   getResolvedParagraphProperties,
@@ -28,6 +29,85 @@ const RUN_PROPERTIES_DERIVED_FROM_MARKS = new Set([
 
 export const TRANSIENT_HYPERLINK_STYLE_IDS = new Set(['Hyperlink', 'FollowedHyperlink']);
 
+/**
+ * Merge mark-derived concrete fontFamily slots with theme references preserved on the
+ * existing run. For each slot where the existing fontFamily has a theme reference, keep
+ * the theme and drop the concrete value from marks; otherwise take the mark value.
+ *
+ * Only safe to call when the caller has already verified that marks are a re-derivation
+ * of `existing` rather than a user override — otherwise this silently reverts user font
+ * changes. See `marksMatchExistingFontFamily` for the gate.
+ *
+ * SD-2894: when the plugin overrides a run's fontFamily from mark-derived values, the
+ * mark only carries the resolved CSS font name (`ascii: "Calibri Light"`) and forgets the
+ * theme reference (`asciiTheme: "majorBidi"`) that came from the imported `<w:rFonts>`.
+ * Without this merge we'd export concrete font names that defeat Word's per-script theme
+ * resolution.
+ *
+ * AIDEV-NOTE: The gate + merge pair operate at the fontFamily-key level (whole rPr
+ * fontFamily object), not at the per-slot level. A user action that mutates only a
+ * per-script mark attr (`eastAsiaFontFamily` or `csFontFamily`) without changing the
+ * primary `fontFamily` will pass the gate, and this merge will silently restore the
+ * stale theme on that slot. We accept this trade-off because (a) the toolbar's
+ * setFontFamily writes all four slots to the same family, so a real user override
+ * always changes the primary; (b) no current API or paste path produces per-script-only
+ * mark changes against an imported theme run; (c) a per-slot model adds intricate
+ * encoder probes that risk regressing the SD-2894 customer fixture again (see commit
+ * bec7e63ed for the prior per-script regression we just fixed). If a real customer
+ * reproduction surfaces — e.g. a paste-from-Word flow with per-script fonts being
+ * silently reverted — file a follow-up ticket with that fixture and rework this helper
+ * into a per-slot decision then. Codex flagged this on the PR-3225 review.
+ *
+ * @param {Record<string, unknown>|null|undefined} fromMarks
+ * @param {Record<string, unknown>|null|undefined} existing
+ * @returns {Record<string, unknown>}
+ */
+function mergeFontFamilyPreservingThemeRefs(fromMarks, existing) {
+  const merged = { ...(fromMarks || {}) };
+  if (!existing || typeof existing !== 'object') return merged;
+  for (const [concreteKey, themeKey] of FONT_SLOT_THEME_PAIRS) {
+    if (existing[themeKey] != null) {
+      merged[themeKey] = existing[themeKey];
+      delete merged[concreteKey];
+    }
+  }
+  return merged;
+}
+
+/**
+ * True when the mark-derived fontFamily value encodes to the same primary ASCII font as
+ * the run's existing (imported) fontFamily — i.e., marks are a faithful re-derivation,
+ * not a user override.
+ *
+ * The encoder resolves theme references to their CSS font name. So:
+ *   - import case: existing = { asciiTheme: 'majorBidi' }, marks = { ascii: 'Calibri Light' }
+ *     → both encode to fontFamily: 'Calibri Light' → match → preserve theme.
+ *   - user-edit case: existing = { asciiTheme: 'majorBidi' }, marks = { ascii: 'Arial' }
+ *     → existing encodes to 'Calibri Light', marks encode to 'Arial' → mismatch → user
+ *       has overridden, drop the theme and respect the new value.
+ *
+ * Compare ONLY the primary `fontFamily` attr, not the full mark attrs object. Per-script
+ * extras (`csFontFamily`, `eastAsiaFontFamily`) on the mark are derived only when the
+ * input has a *concrete* per-script slot. Existing run-props can carry mixed shapes
+ * (e.g. `{ asciiTheme, hAnsiTheme, cstheme, eastAsia: 'Arial' }`) that encode without
+ * `csFontFamily` while the mark re-decode adds a concrete `cs` and therefore does
+ * include it. A full-attrs JSON compare would falsely flag this as user override and
+ * drop the theme — the regression caught on the SD-2894 customer fixture round-trip.
+ *
+ * @param {{ attrs?: Record<string, unknown> } | null | undefined} markFromMarks
+ * @param {Record<string, unknown>|null|undefined} existingFontFamily
+ * @param {(props: Record<string, unknown>, docx: Record<string, unknown>) => Array<{ attrs?: Record<string, unknown> }>} encode
+ * @param {Record<string, unknown>} docx
+ * @returns {boolean}
+ */
+function marksMatchExistingFontFamily(markFromMarks, existingFontFamily, encode, docx) {
+  if (!existingFontFamily || typeof existingFontFamily !== 'object') return false;
+  if (!markFromMarks?.attrs) return false;
+  const markFromExisting = encode({ fontFamily: existingFontFamily }, docx)?.[0];
+  if (!markFromExisting?.attrs) return false;
+  return markFromMarks.attrs.fontFamily === markFromExisting.attrs.fontFamily;
+}
+
 const RUN_PROPERTY_PRESERVE_META_KEY = 'sdPreserveRunPropertiesKeys';
 const COMPANION_INLINE_KEYS = {
   fontSizeCs: 'fontSize',
@@ -536,10 +616,23 @@ function getInlineRunProperties(
     const valueFromStyles = runPropertiesFromStyles[key];
     if (JSON.stringify(valueFromMarks) !== JSON.stringify(valueFromStyles)) {
       if (key === 'fontFamily') {
-        const markFromStyles = encodeMarksFromRPr({ [key]: valueFromStyles }, editor.converter?.convertedXml ?? {})[0];
-        const markFromMarks = encodeMarksFromRPr({ [key]: valueFromMarks }, editor.converter?.convertedXml ?? {})[0];
+        const docx = editor.converter?.convertedXml ?? {};
+        const markFromStyles = encodeMarksFromRPr({ [key]: valueFromStyles }, docx)[0];
+        const markFromMarks = encodeMarksFromRPr({ [key]: valueFromMarks }, docx)[0];
         if (JSON.stringify(markFromMarks?.attrs) !== JSON.stringify(markFromStyles?.attrs)) {
-          inlineRunProperties[key] = valueFromMarks;
+          // SD-2894 follow-up (PR #3225 review): only preserve theme refs from `existing`
+          // when the marks are a faithful re-derivation of the imported value. If marks
+          // diverge from existing (user picked a new font), respect the user's choice
+          // and drop the stale theme reference.
+          const existingFontFamily = existingRunProperties?.[key];
+          inlineRunProperties[key] = marksMatchExistingFontFamily(
+            markFromMarks,
+            existingFontFamily,
+            encodeMarksFromRPr,
+            docx,
+          )
+            ? mergeFontFamilyPreservingThemeRefs(valueFromMarks, existingFontFamily)
+            : valueFromMarks;
         }
       } else {
         inlineRunProperties[key] = valueFromMarks;