chore: review fixes

Author: harbournick

packages/super-editor/src/core/Editor.ts

@@ -36,7 +36,10 @@ import { prepareCommentsForExport, prepareCommentsForImport } from '@extensions/
 import DocxZipper from '@core/DocxZipper.js';
 import { generateCollaborationData } from '@extensions/collaboration/collaboration.js';
 import { useHighContrastMode } from '../composables/use-high-contrast-mode.js';
-import { scheduleReconcile } from '@extensions/collaboration/part-sync/part-reconcile-scheduler.js';
+import {
+  scheduleReconcile,
+  reconcileImmediately,
+} from '@extensions/collaboration/part-sync/part-reconcile-scheduler.js';
 import { setImageNodeSelection } from './helpers/setImageNodeSelection.js';
 import { canRenderFont } from './helpers/canRenderFont.js';
 import {
@@ -1492,16 +1495,21 @@ export class Editor extends EventEmitter<EditorEventMap> {
    * If we are replacing data and have a valid provider, listen for synced event
    * so that we can initialize the data
    */
-  initializeCollaborationData(): void {
+  initializeCollaborationData(afterInsert?: () => void): void {
     if (!this.options.isNewFile || !this.options.collaborationProvider) return;
     const provider = this.options.collaborationProvider;
 
+    const doInsert = () => {
+      this.#insertNewFileData();
+      afterInsert?.();
+    };
+
     const postSyncInit = () => {
       provider.off?.('synced', postSyncInit);
-      this.#insertNewFileData();
+      doInsert();
     };
 
-    if (provider.synced) this.#insertNewFileData();
+    if (provider.synced) doInsert();
     // If we are not sync'd yet, wait for the event then insert the data
     else provider.on?.('synced', postSyncInit);
   }
@@ -3191,8 +3199,7 @@ export class Editor extends EventEmitter<EditorEventMap> {
     this.initDefaultStyles();
 
     if (this.options.ydoc && this.options.collaborationProvider) {
-      scheduleReconcile(this, 'replaceFile');
-      this.initializeCollaborationData();
+      this.initializeCollaborationData(() => reconcileImmediately(this));
     } else {
       this.#insertNewFileData();
     }

packages/super-editor/src/core/presentation-editor/PresentationEditor.ts

@@ -2641,7 +2641,7 @@ export class PresentationEditor extends EventEmitter {
     const PART_INVALIDATION: Record<string, { clearCaches: boolean }> = {
       styles: { clearCaches: true },
       numbering: { clearCaches: true },
-      themeColors: { clearCaches: true },
+      theme: { clearCaches: true },
     };
 
     const handlePartChanged = (payload: { partId: string; changedPaths: string[]; source: string }) => {

packages/super-editor/src/extensions/collaboration/part-sync/part-reconcile-scheduler.js

@@ -152,6 +152,22 @@ export function scheduleReconcile(editor, _reason) {
   }
 }
 
+/**
+ * Run a reconcile immediately, bypassing the debounce/maxWait timers.
+ *
+ * Use this after operations like `replaceFile` where structured channels must
+ * be consistent before any other client can observe the new state.
+ *
+ * Automatically marks the editor dirty so the reconcile is not skipped.
+ *
+ * @param {object} editor
+ * @returns {Promise<void>}
+ */
+export async function reconcileImmediately(editor) {
+  markDirty(editor);
+  await executeReconcile(editor);
+}
+
 /**
  * Clear all pending timers and per-editor state. Called on editor destroy.
  *

packages/super-editor/src/extensions/collaboration/part-sync/part-reconcile-scheduler.test.js

@@ -1,5 +1,10 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import { scheduleReconcile, destroyReconcileState, markDirty } from './part-reconcile-scheduler.js';
+import {
+  scheduleReconcile,
+  destroyReconcileState,
+  markDirty,
+  reconcileImmediately,
+} from './part-reconcile-scheduler.js';
 import { publishPartSections } from './part-sync-engine.js';
 import { getOoxmlPartSpecs, invalidateDiscoveredSpecs } from './part-spec-registry.js';
 
@@ -485,4 +490,61 @@ describe('part-reconcile-scheduler', () => {
       expect(() => markDirty(null)).not.toThrow();
     });
   });
+
+  describe('reconcileImmediately', () => {
+    it('runs exportDocx and publishPartSections synchronously (no timer)', async () => {
+      const editor = createMockEditor();
+
+      await reconcileImmediately(editor);
+
+      expect(editor.exportDocx).toHaveBeenCalledOnce();
+      expect(editor.exportDocx).toHaveBeenCalledWith({ getUpdatedDocs: true });
+      expect(publishPartSections).toHaveBeenCalledTimes(2);
+    });
+
+    it('does not require a prior markDirty call', async () => {
+      const editor = createMockEditor();
+
+      // reconcileImmediately marks dirty internally
+      await reconcileImmediately(editor);
+
+      expect(editor.exportDocx).toHaveBeenCalledOnce();
+    });
+
+    it('cancels any pending debounced reconcile timers', async () => {
+      const editor = createMockEditor();
+
+      markDirty(editor);
+      scheduleReconcile(editor, 'pending');
+
+      // Immediate reconcile runs and clears timers
+      await reconcileImmediately(editor);
+      expect(editor.exportDocx).toHaveBeenCalledOnce();
+
+      editor.exportDocx.mockClear();
+      publishPartSections.mockClear();
+
+      // Advancing past debounce should NOT fire again (no new dirty)
+      await advanceAndFlush(DEBOUNCE_MS + MAX_WAIT_MS);
+      expect(editor.exportDocx).not.toHaveBeenCalled();
+    });
+
+    it('skips if editor is destroyed', async () => {
+      const editor = createMockEditor();
+      editor.isDestroyed = true;
+
+      await reconcileImmediately(editor);
+
+      expect(editor.exportDocx).not.toHaveBeenCalled();
+    });
+
+    it('skips if ydoc is missing', async () => {
+      const editor = createMockEditor();
+      editor.options.ydoc = null;
+
+      await reconcileImmediately(editor);
+
+      expect(editor.exportDocx).not.toHaveBeenCalled();
+    });
+  });
 });

packages/super-editor/src/extensions/collaboration/part-sync/part-spec-registry.js

@@ -34,6 +34,7 @@ import { translator as docDefaultsTranslator } from '../../../core/super-convert
 import { translator as latentStylesTranslator } from '../../../core/super-converter/v3/handlers/w/latentStyles/latentStyles-translator.js';
 import { translator as styleTranslator } from '../../../core/super-converter/v3/handlers/w/style/style-translator.js';
 import { incrementRevision } from '../../../document-api-adapters/plan-engine/revision-tracker.js';
+import { translator as numberingTranslator } from '../../../core/super-converter/v3/handlers/w/numbering/numbering-translator.js';
 
 // ---------------------------------------------------------------------------
 // Factory: xml-js parts with a single 'root' section
@@ -388,7 +389,26 @@ export const CONTENT_TYPES_SPEC = {
 // OOXML part specs (shared channel: ooxmlPartModels)
 // ---------------------------------------------------------------------------
 
-export const NUMBERING_SPEC = createXmlPartSpec('numbering', 'word/numbering.xml');
+export const NUMBERING_SPEC = {
+  ...createXmlPartSpec('numbering', 'word/numbering.xml'),
+
+  afterApply: (editor) => {
+    const converter = editor.converter;
+    if (!converter) return;
+
+    const part = converter.parts?.['word/numbering.xml'] ?? converter.convertedXml?.['word/numbering.xml'];
+    const rootElement = part?.elements?.[0];
+
+    // Re-translate raw XML into the model the layout engine reads.
+    // If the part was deleted (no root element), clear to empty model so
+    // collaborators don't keep stale list definitions.
+    converter.translatedNumbering = rootElement
+      ? numberingTranslator.encode({ nodes: [rootElement] })
+      : { abstracts: {}, definitions: {} };
+
+    incrementRevision(editor);
+  },
+};
 export const SETTINGS_SPEC = createXmlPartSpec('settings', 'word/settings.xml');
 export const DOCUMENT_RELS_SPEC = createXmlPartSpec('documentRels', 'word/_rels/document.xml.rels');
 export const FOOTNOTES_SPEC = createXmlPartSpec('footnotes', 'word/footnotes.xml');

packages/super-editor/src/extensions/collaboration/part-sync/part-spec-registry.test.js

@@ -23,6 +23,13 @@ vi.mock('../../../document-api-adapters/plan-engine/revision-tracker.js', () =>
   incrementRevision: vi.fn(),
 }));
 
+const mockNumberingTranslator = vi.hoisted(() => ({
+  encode: vi.fn(() => ({ abstracts: { 0: {} }, definitions: { 1: {} } })),
+}));
+vi.mock('../../../core/super-converter/v3/handlers/w/numbering/numbering-translator.js', () => ({
+  translator: mockNumberingTranslator,
+}));
+
 import {
   STYLES_SPEC,
   NUMBERING_SPEC,
@@ -38,6 +45,7 @@ import {
   CUSTOM_PROPS_SPEC,
   CORE_PROPS_SPEC,
   FONT_TABLE_RELS_SPEC,
+  THEME_SPEC,
   HEADER_FOOTER_RELS_SPEC,
   HEADER_FOOTER_CONTENT_SPEC,
   CONTENT_TYPES_SPEC,
@@ -119,6 +127,10 @@ describe('Registry functions', () => {
     expect(result).toEqual({ spec: NUMBERING_SPEC, section: 'root' });
   });
 
+  it('THEME_SPEC.id is "theme" (must match PresentationEditor PART_INVALIDATION key)', () => {
+    expect(THEME_SPEC.id).toBe('theme');
+  });
+
   it('resolveOoxmlPartKey("unknown/something") returns null', () => {
     expect(resolveOoxmlPartKey('unknown/something')).toBeNull();
   });
@@ -221,6 +233,69 @@ describe('createXmlPartSpec (NUMBERING_SPEC)', () => {
     NUMBERING_SPEC.applySection(converter, 'root', value);
     expect(converter.convertedXml['word/numbering.xml'].elements[0]).toBe(value);
   });
+
+  describe('afterApply', () => {
+    it('re-translates numbering XML into converter.translatedNumbering', () => {
+      const rootElement = { name: 'w:numbering', elements: [{ name: 'w:abstractNum' }] };
+      const converter = {
+        convertedXml: { 'word/numbering.xml': { elements: [rootElement] } },
+        translatedNumbering: null,
+      };
+      const editor = { converter };
+
+      NUMBERING_SPEC.afterApply(editor, ['root']);
+
+      expect(mockNumberingTranslator.encode).toHaveBeenCalledWith({ nodes: [rootElement] });
+      expect(converter.translatedNumbering).toEqual({ abstracts: { 0: {} }, definitions: { 1: {} } });
+    });
+
+    it('calls incrementRevision', () => {
+      const rootElement = { name: 'w:numbering' };
+      const converter = {
+        convertedXml: { 'word/numbering.xml': { elements: [rootElement] } },
+        translatedNumbering: null,
+      };
+      const editor = { converter };
+
+      NUMBERING_SPEC.afterApply(editor, ['root']);
+
+      expect(incrementRevision).toHaveBeenCalledWith(editor);
+    });
+
+    it('skips translation when converter is missing', () => {
+      const editor = { converter: null };
+      expect(() => NUMBERING_SPEC.afterApply(editor, ['root'])).not.toThrow();
+      expect(mockNumberingTranslator.encode).not.toHaveBeenCalled();
+    });
+
+    it('clears translatedNumbering to empty model when numbering XML is deleted', () => {
+      const converter = {
+        convertedXml: { 'word/numbering.xml': { elements: [] } },
+        translatedNumbering: { abstracts: { 0: { levels: {} } }, definitions: { 1: {} } },
+      };
+      const editor = { converter };
+
+      NUMBERING_SPEC.afterApply(editor, ['root']);
+
+      expect(mockNumberingTranslator.encode).not.toHaveBeenCalled();
+      expect(converter.translatedNumbering).toEqual({ abstracts: {}, definitions: {} });
+      expect(incrementRevision).toHaveBeenCalledWith(editor);
+    });
+
+    it('reads from converter.parts when convertedXml is missing the part', () => {
+      const rootElement = { name: 'w:numbering' };
+      const converter = {
+        parts: { 'word/numbering.xml': { elements: [rootElement] } },
+        convertedXml: {},
+        translatedNumbering: null,
+      };
+      const editor = { converter };
+
+      NUMBERING_SPEC.afterApply(editor, ['root']);
+
+      expect(mockNumberingTranslator.encode).toHaveBeenCalledWith({ nodes: [rootElement] });
+    });
+  });
 });
 
 // ---------------------------------------------------------------------------