fix(ui): aggregate comments across body + headers/footers/notes (SD-2938)

packages/document-api/src/comments/comments.types.ts

@@ -1,8 +1,17 @@
 import type { CommentAddress, CommentStatus, TextTarget } from '../types/index.js';
 import type { DiscoveryOutput } from '../types/discovery.js';
+import type { StoryLocator } from '../types/story.types.js';
 
 export type { CommentStatus } from '../types/index.js';
 
+/**
+ * Scope marker used by {@link CommentsListQuery.in} to request comments
+ * across every story (body + headers + footers + footnotes + endnotes).
+ * Equivalent to a multi-story aggregate list.
+ */
+export const COMMENTS_IN_ALL = 'all' as const;
+export type CommentsInAll = typeof COMMENTS_IN_ALL;
+
 export interface CommentInfo {
   address: CommentAddress;
   commentId: string;
@@ -22,6 +31,13 @@ export interface CommentsListQuery {
   includeResolved?: boolean;
   limit?: number;
   offset?: number;
+  /**
+   * Story scope.
+   * - `undefined` (default): body only (backward compatible).
+   * - A {@link StoryLocator}: only that story.
+   * - `'all'`: flat list across body + every non-body story.
+   */
+  in?: StoryLocator | CommentsInAll;
 }
 
 /**

packages/document-api/src/index.ts

@@ -1402,7 +1402,8 @@ export type {
   GoToCommentInput,
   SetCommentActiveInput,
 } from './comments/comments.js';
-export type { CommentInfo, CommentsListQuery, CommentsListResult } from './comments/comments.types.js';
+export type { CommentInfo, CommentsListQuery, CommentsListResult, CommentsInAll } from './comments/comments.types.js';
+export { COMMENTS_IN_ALL } from './comments/comments.types.js';
 export { DocumentApiValidationError } from './errors.js';
 export { textReceiptToSDReceipt, buildStructuralReceipt } from './receipt-bridge.js';
 export type { StructuralReceiptParams } from './receipt-bridge.js';

packages/document-api/src/types/address.ts

@@ -119,13 +119,9 @@ export type EntityType = 'comment' | 'trackedChange';
 export type CommentAddress = {
   kind: 'entity';
   entityType: 'comment';
-  /**
-   * Comment navigation is currently body-scoped only.
-   *
-   * Unlike bookmark and tracked-change navigation, `navigateTo()` does not yet
-   * accept a `story` locator for comments.
-   */
   entityId: string;
+  /** Story containing this comment. Omit for body (backward compatible). */
+  story?: StoryLocator;
 };
 
 export type TrackedChangeAddress = {
@@ -169,11 +165,11 @@ export type BlockNavigationAddress = {
  * Supports navigation to:
  * - Blocks by `nodeId` in the body story
  * - Bookmarks by `name`, optionally scoped to a `story`
- * - Comments by `entityId` in the body story
+ * - Comments by `entityId`, optionally scoped to a `story`
  * - Tracked changes by `entityId`, optionally scoped to a `story`
  *
- * Story-aware navigation is currently supported for bookmarks and tracked
- * changes. Block and comment targets remain body-only until the runtime gains
- * equivalent non-body resolution paths.
+ * Story-aware navigation is supported for bookmarks, tracked changes, and
+ * comments. Block targets remain body-only until the runtime gains equivalent
+ * non-body resolution paths.
  */
 export type NavigableAddress = BlockNavigationAddress | BookmarkAddress | CommentAddress | TrackedChangeAddress;

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

@@ -8140,7 +8140,7 @@ export class PresentationEditor extends EventEmitter {
           return await this.#navigateToBookmark(target);
         }
         if (target.entityType === 'comment') {
-          return await this.#navigateToComment(target.entityId, options);
+          return await this.#navigateToComment(target.entityId, resolveStoryKeyFromAddress(target.story), options);
         }
         if (target.entityType === 'trackedChange') {
           return await this.#navigateToTrackedChange(
@@ -8222,24 +8222,45 @@ export class PresentationEditor extends EventEmitter {
 
   async #navigateToComment(
     entityId: string,
+    storyKey: string | undefined,
     options: { behavior?: ScrollBehavior; block?: 'start' | 'center' | 'end' | 'nearest' } = {},
   ): Promise<boolean> {
     const editor = this.#editor;
     if (!editor) return false;
 
+    const behavior = options.behavior ?? 'auto';
+    const block = options.block ?? 'center';
+
+    // Non-body comments live in stories whose anchors aren't reachable
+    // through the body editor's `setCursorById`. Fall back to scrolling
+    // the rendered DOM element directly. `findRenderedCommentElements`
+    // is already story-aware. When the story isn't currently painted
+    // (e.g. a footnote that hasn't been expanded), the lookup returns
+    // an empty array and we surface a structured `false` rather than
+    // landing the cursor in the wrong place.
+    if (storyKey && storyKey !== BODY_STORY_KEY) {
+      const elements = findRenderedCommentElements(this.#painterHost, entityId, storyKey);
+      const candidate = elements[0];
+      if (!candidate) return false;
+      try {
+        candidate.scrollIntoView({ behavior, block, inline: 'nearest' });
+        return true;
+      } catch {
+        return false;
+      }
+    }
+
     const setCursorById = editor.commands?.setCursorById;
     if (typeof setCursorById !== 'function') return false;
 
     if (!setCursorById(entityId, { preferredActiveThreadId: entityId, activeCommentId: entityId })) {
       return false;
     }
 
-    // Scroll the viewport — setCursorById places the cursor but doesn't
-    // scroll in presentation mode where DomPainter renders the output.
-    await this.scrollToPositionAsync(editor.state.selection.from, {
-      behavior: options.behavior ?? 'auto',
-      block: options.block ?? 'center',
-    });
+    // Scroll the viewport. `setCursorById` places the cursor but
+    // doesn't scroll in presentation mode where DomPainter renders
+    // the output.
+    await this.scrollToPositionAsync(editor.state.selection.from, { behavior, block });
     return true;
   }
 

packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/comments-wrappers.test.ts

@@ -30,9 +30,20 @@ vi.mock('../helpers/adapter-utils.js', async () => {
   };
 });
 
+vi.mock('../tracked-changes/enumerate-stories.js', () => ({
+  enumerateRevisionCapableStories: vi.fn(() => [{ kind: 'story', storyType: 'body' }]),
+}));
+
+vi.mock('../story-runtime/resolve-story-runtime.js', () => ({
+  resolveStoryRuntime: vi.fn(),
+}));
+
 import { listCommentAnchors } from '../helpers/comment-target-resolver.js';
 import { resolveTextTarget } from '../helpers/adapter-utils.js';
 import { executeDomainCommand } from './plan-wrappers.js';
+import { enumerateRevisionCapableStories } from '../tracked-changes/enumerate-stories.js';
+import { resolveStoryRuntime } from '../story-runtime/resolve-story-runtime.js';
+import type { StoryLocator } from '@superdoc/document-api';
 
 function makeAnchor(
   overrides: Partial<CommentAnchor> & { commentId: string; pos: number; end: number },
@@ -840,3 +851,180 @@ describe('comments-wrappers: addCommentHandler multi-segment targets', () => {
     expect(editor.commands!.addComment).not.toHaveBeenCalled();
   });
 });
+
+describe('comments-wrappers: cross-section listing (SD-2938)', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    // Earlier suites set per-test mockReturnValue on listCommentAnchors
+    // to seed anchors for "c1". clearAllMocks wipes call history but
+    // not implementations, so reset to the default empty list here.
+    vi.mocked(listCommentAnchors).mockReturnValue([]);
+    vi.mocked(enumerateRevisionCapableStories).mockReturnValue([{ kind: 'story', storyType: 'body' }]);
+  });
+
+  const headerLocator: StoryLocator = {
+    kind: 'story',
+    storyType: 'headerFooterPart',
+    refId: 'rId6',
+  };
+
+  it('returns body-only items when query.in is undefined (backward compatible)', () => {
+    const editor = makeEditor([{ commentId: 'c-body', commentText: 'body comment' }]);
+    const wrapper = createCommentsWrapper(editor);
+
+    const result = wrapper.list();
+
+    expect(result.items).toHaveLength(1);
+    expect(result.items[0]!.id).toBe('c-body');
+    expect(result.items[0]!.address.story).toBeUndefined();
+    expect(enumerateRevisionCapableStories).not.toHaveBeenCalled();
+    expect(resolveStoryRuntime).not.toHaveBeenCalled();
+  });
+
+  it('aggregates body + header comments and stamps story on the header item when in: "all"', () => {
+    const bodyEditor = makeEditor([{ commentId: 'c-body', commentText: 'body comment', createdTime: 1 }]);
+    const headerEditor = makeEditor([{ commentId: 'c-header', commentText: 'header comment', createdTime: 2 }]);
+
+    vi.mocked(enumerateRevisionCapableStories).mockReturnValue([{ kind: 'story', storyType: 'body' }, headerLocator]);
+    vi.mocked(resolveStoryRuntime).mockReturnValue({ editor: headerEditor } as never);
+
+    const wrapper = createCommentsWrapper(bodyEditor);
+    const result = wrapper.list({ in: 'all' });
+
+    const ids = result.items.map((i) => i.id);
+    expect(ids).toEqual(['c-body', 'c-header']);
+
+    const bodyItem = result.items.find((i) => i.id === 'c-body')!;
+    const headerItem = result.items.find((i) => i.id === 'c-header')!;
+
+    expect(bodyItem.address.story).toBeUndefined();
+    expect(headerItem.address.story).toEqual(headerLocator);
+  });
+
+  it('returns only the targeted story when query.in is a non-body locator', () => {
+    const bodyEditor = makeEditor([{ commentId: 'c-body', commentText: 'body comment' }]);
+    const headerEditor = makeEditor([{ commentId: 'c-header', commentText: 'header comment' }]);
+
+    vi.mocked(resolveStoryRuntime).mockReturnValue({ editor: headerEditor } as never);
+
+    const wrapper = createCommentsWrapper(bodyEditor);
+    const result = wrapper.list({ in: headerLocator });
+
+    expect(result.items).toHaveLength(1);
+    expect(result.items[0]!.id).toBe('c-header');
+    expect(result.items[0]!.address.story).toEqual(headerLocator);
+    expect(enumerateRevisionCapableStories).not.toHaveBeenCalled();
+  });
+
+  it('skips a non-body story when its runtime cannot be resolved', () => {
+    const bodyEditor = makeEditor([{ commentId: 'c-body', commentText: 'body comment' }]);
+
+    vi.mocked(enumerateRevisionCapableStories).mockReturnValue([{ kind: 'story', storyType: 'body' }, headerLocator]);
+    vi.mocked(resolveStoryRuntime).mockImplementation(() => {
+      throw new Error('story runtime missing');
+    });
+
+    const wrapper = createCommentsWrapper(bodyEditor);
+    const result = wrapper.list({ in: 'all' });
+
+    expect(result.items).toHaveLength(1);
+    expect(result.items[0]!.id).toBe('c-body');
+  });
+
+  it('deduplicates if the same comment id appears in body and a non-body store', () => {
+    const bodyEditor = makeEditor([{ commentId: 'c-shared', commentText: 'body copy', createdTime: 1 }]);
+    const headerEditor = makeEditor([{ commentId: 'c-shared', commentText: 'header copy', createdTime: 2 }]);
+
+    vi.mocked(enumerateRevisionCapableStories).mockReturnValue([{ kind: 'story', storyType: 'body' }, headerLocator]);
+    vi.mocked(resolveStoryRuntime).mockReturnValue({ editor: headerEditor } as never);
+
+    const wrapper = createCommentsWrapper(bodyEditor);
+    const result = wrapper.list({ in: 'all' });
+
+    expect(result.items).toHaveLength(1);
+    expect(result.items[0]!.id).toBe('c-shared');
+    // Body wins because body is enumerated first.
+    expect(result.items[0]!.address.story).toBeUndefined();
+  });
+
+  it('prefers the story-stamped record when body has only a definition (no anchor)', () => {
+    // OOXML imports flatten every comment definition into the body's
+    // converter.comments. For a comment anchored in the header, the
+    // body pass yields a definition-only record (no target). The
+    // header pass yields the same id with target + story. The merge
+    // must keep the header version so UI actions route by story.
+    const bodyEditor = makeEditor([{ commentId: 'c-header-only', commentText: 'lives in header' }]);
+    const headerEditor = makeEditor([{ commentId: 'c-header-only', commentText: 'lives in header' }]);
+
+    vi.mocked(listCommentAnchors).mockImplementation(((editor: unknown) => {
+      if (editor === bodyEditor) return [] as never;
+      return [makeAnchor({ commentId: 'c-header-only', pos: 0, end: 6 })] as never;
+    }) as never);
+
+    vi.mocked(enumerateRevisionCapableStories).mockReturnValue([{ kind: 'story', storyType: 'body' }, headerLocator]);
+    vi.mocked(resolveStoryRuntime).mockReturnValue({ editor: headerEditor } as never);
+
+    const wrapper = createCommentsWrapper(bodyEditor);
+    const result = wrapper.list({ in: 'all' });
+
+    expect(result.items).toHaveLength(1);
+    const item = result.items[0]!;
+    expect(item.id).toBe('c-header-only');
+    expect(item.address.story).toEqual(headerLocator);
+    expect(item.target).toBeDefined();
+    expect(item.target?.story).toEqual(headerLocator);
+  });
+
+  it('disposes a non-cacheable runtime returned during the in: all walk', () => {
+    const bodyEditor = makeEditor();
+    const headerEditor = makeEditor();
+    const dispose = vi.fn();
+
+    vi.mocked(enumerateRevisionCapableStories).mockReturnValue([{ kind: 'story', storyType: 'body' }, headerLocator]);
+    vi.mocked(resolveStoryRuntime).mockReturnValue({
+      editor: headerEditor,
+      cacheable: false,
+      dispose,
+    } as never);
+
+    const wrapper = createCommentsWrapper(bodyEditor);
+    wrapper.list({ in: 'all' });
+
+    expect(dispose).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not dispose a cacheable runtime during the in: all walk', () => {
+    const bodyEditor = makeEditor();
+    const headerEditor = makeEditor();
+    const dispose = vi.fn();
+
+    vi.mocked(enumerateRevisionCapableStories).mockReturnValue([{ kind: 'story', storyType: 'body' }, headerLocator]);
+    vi.mocked(resolveStoryRuntime).mockReturnValue({
+      editor: headerEditor,
+      cacheable: true,
+      dispose,
+    } as never);
+
+    const wrapper = createCommentsWrapper(bodyEditor);
+    wrapper.list({ in: 'all' });
+
+    expect(dispose).not.toHaveBeenCalled();
+  });
+
+  it('disposes a non-cacheable runtime returned for a single-story query', () => {
+    const bodyEditor = makeEditor();
+    const headerEditor = makeEditor();
+    const dispose = vi.fn();
+
+    vi.mocked(resolveStoryRuntime).mockReturnValue({
+      editor: headerEditor,
+      cacheable: false,
+      dispose,
+    } as never);
+
+    const wrapper = createCommentsWrapper(bodyEditor);
+    wrapper.list({ in: headerLocator });
+
+    expect(dispose).toHaveBeenCalledTimes(1);
+  });
+});