chore: fix review comments

Author: harbournick

packages/super-editor/src/document-api-adapters/helpers/list-item-resolver.test.ts

@@ -142,6 +142,40 @@ describe('list-item-resolver', () => {
     expect(result.items[0]?.id).toBe('li-1');
   });
 
+  it('keeps listId stable across within scopes', () => {
+    const editor = makeEditor([
+      makeParagraph({
+        id: 'li-1',
+        text: 'First',
+        numId: 1,
+        ilvl: 0,
+        markerText: '1.',
+        path: [1],
+        numberingType: 'decimal',
+      }),
+      makeParagraph({
+        id: 'li-2',
+        text: 'Second',
+        numId: 1,
+        ilvl: 0,
+        markerText: '2.',
+        path: [2],
+        numberingType: 'decimal',
+      }),
+    ]);
+
+    const unscoped = listListItems(editor);
+    const scoped = listListItems(editor, {
+      within: { kind: 'block', nodeType: 'listItem', nodeId: 'li-2' },
+    });
+
+    const unscopedSecond = unscoped.items.find((item) => item.id === 'li-2');
+    const scopedSecond = scoped.items.find((item) => item.id === 'li-2');
+
+    expect(unscopedSecond?.listId).toBe('1:li-1');
+    expect(scopedSecond?.listId).toBe('1:li-1');
+  });
+
   it('throws TARGET_NOT_FOUND when resolving a stale list address', () => {
     const editor = makeEditor([
       makeParagraph({ id: 'li-1', numId: 1, markerText: '1.', path: [1], numberingType: 'decimal' }),

packages/super-editor/src/document-api-adapters/helpers/list-item-resolver.ts

@@ -192,20 +192,22 @@ export function listListItems(editor: Editor, query?: ListsListQuery): ListsList
 
   const index = getBlockIndex(editor);
   const scope = resolveBlockScopeRange(index, query?.within as BlockNodeAddress | undefined);
-  const candidates = listItemCandidatesInScope(index, scope);
+  const allCandidates = index.candidates.filter((candidate) => candidate.nodeType === 'listItem');
+  const allProjections = allCandidates.map((candidate) => projectListItemCandidate(editor, candidate));
+  const projections = allProjections.filter((projection) => isWithinScope(projection.candidate, scope));
   const safeOffset = query?.offset ?? 0;
   const safeLimit = query?.limit ?? Number.POSITIVE_INFINITY;
   const pageEnd = safeOffset + safeLimit;
   const evaluatedRevision = getRevision(editor);
 
-  // Project all items first so sequence IDs can be computed in a single pass.
-  const allProjections = candidates.map((c) => projectListItemCandidate(editor, c));
+  // Compute sequence IDs from document-wide projections so list identity is
+  // stable across scoped queries.
   const sequenceIds = computeSequenceIdMap(allProjections);
 
   let total = 0;
   const items: ListsListResult['items'] = [];
 
-  for (const projection of allProjections) {
+  for (const projection of projections) {
     if (!matchesListQuery(projection, query)) continue;
 
     const currentIndex = total;

packages/super-editor/src/document-api-adapters/plan-engine/lists-wrappers.test.ts

@@ -637,12 +637,28 @@ describe('lists-wrappers', () => {
     it('removes override when value is null', () => {
       const proj = makeProjection({ numId: 1, level: 0 });
       vi.mocked(resolveListItem).mockReturnValueOnce(proj);
+      editor.converter.numbering.definitions[1] = {
+        elements: [{ name: 'w:lvlOverride', attributes: { 'w:ilvl': '0' } }],
+      };
 
       const result = listsSetValueWrapper(editor, { target: proj.address, value: null });
       expect(result.success).toBe(true);
       expect(ListHelpers.removeLvlOverride).toHaveBeenCalledWith(editor, 1, 0);
     });
 
+    it('returns NO_OP when removing an absent override', () => {
+      const proj = makeProjection({ numId: 1, level: 0 });
+      vi.mocked(resolveListItem).mockReturnValueOnce(proj);
+      editor.converter.numbering.definitions[1] = {
+        elements: [{ name: 'w:abstractNumId', attributes: { 'w:val': '10' } }],
+      };
+
+      const result = listsSetValueWrapper(editor, { target: proj.address, value: null });
+      expect(result.success).toBe(false);
+      expect((result as any).failure.code).toBe('NO_OP');
+      expect(ListHelpers.removeLvlOverride).not.toHaveBeenCalled();
+    });
+
     it('returns dry-run result', () => {
       const proj = makeProjection({ numId: 1 });
       vi.mocked(resolveListItem).mockReturnValueOnce(proj);

packages/super-editor/src/document-api-adapters/plan-engine/lists-wrappers.ts

@@ -132,6 +132,23 @@ function withListTarget(editor: Editor, input: ListTargetInput): ListItemProject
   return resolveListItem(editor, input.target);
 }
 
+function hasLevelOverride(editor: Editor, numId: number, level: number): boolean {
+  const converter = editor as unknown as {
+    converter?: {
+      numbering?: {
+        definitions?: Record<number, { elements?: Array<{ name?: string; attributes?: Record<string, unknown> }> }>;
+      };
+    };
+  };
+  const definition = converter.converter?.numbering?.definitions?.[numId];
+  const ilvl = String(level);
+  return (
+    definition?.elements?.some(
+      (element) => element.name === 'w:lvlOverride' && element.attributes?.['w:ilvl'] === ilvl,
+    ) ?? false
+  );
+}
+
 /**
  * Shared core of setLevel, indent, and outdent.
  * Validates preconditions and performs the level change.
@@ -723,6 +740,10 @@ export function listsSetValueWrapper(
 
   // Remove override
   if (input.value === null) {
+    if (!hasLevelOverride(editor, target.numId, level)) {
+      return toListsFailure('NO_OP', 'No startOverride to remove.', { target: input.target });
+    }
+
     const receipt = executeDomainCommand(
       editor,
       () => {