Skip to content

Commit 6cc8247

Browse files
feat: story-aware bookmark addressing (SD-2358) (#2971)
* feat(document-api): add story-aware bookmark contract * feat(super-editor): add cross-story bookmark discovery helpers * feat(super-editor): add story-aware bookmark wrappers * feat(super-editor): support bookmark targets in navigateTo * feat: export navigation address types from public APIs * feat: expose SuperDoc navigateTo wrapper for presentation navigation * test(behavior): cover bookmark navigation and header roundtrip flows * fix(super-editor): use live note session editors for bookmark discovery * fix(super-editor): normalize default header slot bookmark navigation * fix(super-editor): use story revisions for story-scoped bookmark flows * fix(super-editor): scope bookmark rename duplicate check by story key The bookmarkExistsAnywhere exclusion previously matched on bookmarkId alone, so renaming a bookmark could silently collide with a same-named bookmark in another story whose id happened to match. Scope the exclusion to (storyKey, bookmarkId) so cross-story id collisions no longer mask real duplicate names. * fix(super-editor): skip unresolved stories during bookmark listing Document-wide bookmarksList iterates story keys discovered from the bookmark index; if a story (e.g. a header/footer part) can't be resolved at list time, resolveStoryRuntime throws STORY_NOT_FOUND and the whole list call fails. Catch that specific error per-story and return no items for it so a stale or missing part doesn't break listing the rest of the document. * fix(super-editor): prefer live header/footer sessions in bookmark discovery * docs(document-api): clarify story-aware navigateTo support * docs(document-api): clarify bookmark story resolution semantics * fix(super-editor): align bookmark header activation with session API * fix(super-editor): resolve unqualified bookmark mutations document-wide * docs(document-api): narrow unqualified bookmark lookup semantics * fix(super-editor): use aggregate revisions for document-wide bookmark reads * fix(document-api): allow story on text target schema * fix(super-editor): reject ambiguous unqualified bookmark mutations When a bookmark rename or remove target omits `story` but the name exists in multiple stories, throw INVALID_INPUT asking the caller to disambiguate instead of silently picking the first match. * fix(super-editor): reject ambiguous unqualified bookmark mutations When a bookmark rename or remove target omits `story` but the name exists in multiple stories, throw INVALID_INPUT asking the caller to disambiguate instead of silently picking the first match. * refactor(super-editor): extract header/footer variant normalization Move normalizeVariant out of the doc-api adapter helper into a dedicated module under presentation-editor/header-footer/, where its sole non-adapter caller (HeaderFooterSessionManager) lives. The slot-materialization helper re-exports it for existing import sites. * test(super-editor): assert navigateTo returns false for missing bookmarks * fix(super-editor): exit header surface when navigating to body bookmark Calling navigateTo for a body-story bookmark while a header/footer surface is active left the header editor focused, so the selection update was applied to the wrong editor. Exit the active story surface before delegating to goToAnchor so navigation lands on the body editor. * fix(super-editor): scope bookmark discovery IDs by story key Document-wide bookmark listings collide when the same name exists in multiple stories (e.g. body and a header part). buildBookmarkDiscoveryItem now accepts an optional idScope and URL-encodes each segment so each story's bookmarks get distinct, stable IDs. bookmarksListWrapper passes the story key through when iterating snapshots. * fix(document-api): harden bookmark mutation revisions and id allocation
1 parent 018ffb1 commit 6cc8247

28 files changed

Lines changed: 3532 additions & 164 deletions

packages/document-api/src/bookmarks/bookmarks.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ function makeAdapter(): BookmarksAdapter {
2020
}
2121

2222
const validTarget = { kind: 'entity', entityType: 'bookmark', name: 'bm1' };
23+
const validStory = { kind: 'story', storyType: 'footnote', noteId: 'fn-1' } as const;
2324

2425
describe('bookmarks validation', () => {
2526
// ── Target validation ───────────────────────────────────────────────
@@ -65,6 +66,15 @@ describe('bookmarks validation', () => {
6566
}),
6667
).toThrow(DocumentApiValidationError);
6768
});
69+
70+
it('throws INVALID_INPUT when target.story is invalid', () => {
71+
const adapter = makeAdapter();
72+
expect(() =>
73+
executeBookmarksGet(adapter, {
74+
target: { kind: 'entity', entityType: 'bookmark', name: 'bm1', story: 'body' } as any,
75+
}),
76+
).toThrow(DocumentApiValidationError);
77+
});
6878
});
6979

7080
// ── Input validation ────────────────────────────────────────────────
@@ -136,6 +146,18 @@ describe('bookmarks validation', () => {
136146
expect(adapter.list).toHaveBeenCalledWith(query);
137147
});
138148

149+
it('delegates to adapter.list with a valid story filter', () => {
150+
const adapter = makeAdapter();
151+
const query = { in: validStory };
152+
executeBookmarksList(adapter, query);
153+
expect(adapter.list).toHaveBeenCalledWith(query);
154+
});
155+
156+
it('throws INVALID_INPUT when query.in is invalid', () => {
157+
const adapter = makeAdapter();
158+
expect(() => executeBookmarksList(adapter, { in: 'body' as any })).toThrow(DocumentApiValidationError);
159+
});
160+
139161
it('delegates to adapter.list without query', () => {
140162
const adapter = makeAdapter();
141163
executeBookmarksList(adapter);
@@ -150,6 +172,13 @@ describe('bookmarks validation', () => {
150172
executeBookmarksGet(adapter, input as any);
151173
expect(adapter.get).toHaveBeenCalledWith(input);
152174
});
175+
176+
it('delegates to adapter.get for a story-qualified target', () => {
177+
const adapter = makeAdapter();
178+
const input = { target: { ...validTarget, story: validStory } };
179+
executeBookmarksGet(adapter, input);
180+
expect(adapter.get).toHaveBeenCalledWith(input);
181+
});
153182
});
154183

155184
describe('executeBookmarksRemove', () => {

packages/document-api/src/bookmarks/bookmarks.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { MutationOptions } from '../write/write.js';
22
import { normalizeMutationOptions } from '../write/write.js';
33
import { DocumentApiValidationError } from '../errors.js';
44
import { assertTargetPresent } from '../validation-primitives.js';
5+
import { validateStoryLocator } from '../validation/story-validator.js';
56
import type {
67
BookmarkAddress,
78
BookmarkGetInput,
@@ -43,13 +44,19 @@ function validateBookmarkTarget(target: unknown, operationName: string): asserts
4344
{ target },
4445
);
4546
}
47+
if (t.story !== undefined) {
48+
validateStoryLocator(t.story, `${operationName}.target.story`);
49+
}
4650
}
4751

4852
// ---------------------------------------------------------------------------
4953
// Execute wrappers
5054
// ---------------------------------------------------------------------------
5155

5256
export function executeBookmarksList(adapter: BookmarksAdapter, query?: BookmarkListInput): BookmarksListResult {
57+
if (query?.in !== undefined) {
58+
validateStoryLocator(query.in, 'bookmarks.list.in');
59+
}
5360
return adapter.list(query);
5461
}
5562

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Position } from '../types/base.js';
22
import type { TextTarget } from '../types/address.js';
3+
import type { StoryLocator } from '../types/story.types.js';
34
import type { AdapterMutationFailure } from '../types/adapter-result.js';
45
import type { DiscoveryOutput } from '../types/discovery.js';
56

@@ -11,6 +12,18 @@ export interface BookmarkAddress {
1112
kind: 'entity';
1213
entityType: 'bookmark';
1314
name: string;
15+
/**
16+
* Story containing this bookmark. Omit for body (backward compatible).
17+
*
18+
* When omitted, bookmark operations resolve by `name` across the whole
19+
* document. If multiple stories contain the same bookmark name, omitted-story
20+
* lookup resolves to the first match returned by the implementation's
21+
* document-wide bookmark scan; callers must not rely on a specific cross-story
22+
* ordering rule. Prefer the `address` returned by bookmark read operations
23+
* (`bookmarks.list`, `bookmarks.get`) because those responses populate
24+
* `story` for non-body bookmarks.
25+
*/
26+
story?: StoryLocator;
1427
}
1528

1629
// ---------------------------------------------------------------------------
@@ -20,6 +33,8 @@ export interface BookmarkAddress {
2033
export interface BookmarkListInput {
2134
limit?: number;
2235
offset?: number;
36+
/** Restrict listing to a specific story. Omit to search document-wide. */
37+
in?: StoryLocator;
2338
}
2439

2540
export interface BookmarkGetInput {

packages/document-api/src/contract/contract.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,26 @@ describe('document-api contract catalog', () => {
145145
).toEqual(['forbid', 'allow']);
146146
});
147147

148+
it('allows story-scoped text targets for bookmark inserts', () => {
149+
const schemas = buildInternalContractSchemas();
150+
const bookmarkInsertInput = schemas.operations['bookmarks.insert'].input as {
151+
properties?: {
152+
at?: { $ref?: string };
153+
};
154+
};
155+
const textTarget = schemas.$defs?.TextTarget as {
156+
properties?: Record<string, unknown>;
157+
required?: string[];
158+
additionalProperties?: boolean;
159+
};
160+
161+
expect(bookmarkInsertInput.properties?.at?.$ref).toBe('#/$defs/TextTarget');
162+
expect(textTarget.properties).toHaveProperty('story');
163+
expect((textTarget.properties?.story as { $ref?: string }).$ref).toBe('#/$defs/StoryLocator');
164+
expect(textTarget.required).toEqual(['kind', 'segments']);
165+
expect(textTarget.additionalProperties).toBe(false);
166+
});
167+
148168
it('accepts both object and array SDFragment in structural insert content schema', () => {
149169
const schemas = buildInternalContractSchemas();
150170
const insertInput = schemas.operations.insert.input as { oneOf?: Array<{ properties?: Record<string, unknown> }> };

packages/document-api/src/contract/schemas.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ const SHARED_DEFS: Record<string, JsonSchema> = {
231231
{
232232
kind: { const: 'text' },
233233
segments: { type: 'array', items: ref('TextSegment'), minItems: 1 },
234+
story: ref('StoryLocator'),
234235
},
235236
['kind', 'segments'],
236237
),
@@ -2520,10 +2521,12 @@ function buildContentControlSchemas(): Record<ContentControlOperationId, Operati
25202521
// ---------------------------------------------------------------------------
25212522

25222523
// --- Shared patterns ---
2523-
const refListQuerySchema = objectSchema({
2524+
const refListQueryProperties = {
25242525
limit: { type: 'integer', minimum: 1 },
25252526
offset: { type: 'integer', minimum: 0 },
2526-
});
2527+
} satisfies Record<string, JsonSchema>;
2528+
2529+
const refListQuerySchema = objectSchema(refListQueryProperties);
25272530

25282531
const discoveryOutputSchema: JsonSchema = { type: 'object' };
25292532

@@ -2567,7 +2570,12 @@ function refConfigSchemas(): { output: JsonSchema; success: JsonSchema; failure:
25672570

25682571
// --- Bookmark schemas ---
25692572
const bookmarkAddressSchema: JsonSchema = objectSchema(
2570-
{ kind: { const: 'entity' }, entityType: { const: 'bookmark' }, name: { type: 'string' } },
2573+
{
2574+
kind: { const: 'entity' },
2575+
entityType: { const: 'bookmark' },
2576+
name: { type: 'string' },
2577+
story: ref('StoryLocator'),
2578+
},
25712579
['kind', 'entityType', 'name'],
25722580
);
25732581

@@ -6993,7 +7001,10 @@ const operationSchemas: Record<OperationId, OperationSchemaSet> = {
69937001
// Bookmarks
69947002
// -------------------------------------------------------------------------
69957003
'bookmarks.list': {
6996-
input: refListQuerySchema,
7004+
input: objectSchema({
7005+
...refListQueryProperties,
7006+
in: storyLocatorSchema,
7007+
}),
69977008
output: discoveryOutputSchema,
69987009
},
69997010
'bookmarks.get': {

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { BlockNodeType } from './base.js';
22
import type { StoryLocator } from './story.types.js';
3+
import type { BookmarkAddress } from '../bookmarks/bookmarks.types.js';
34

45
export type Range = {
56
/** Inclusive start offset (0-based, UTF-16 code units). */
@@ -118,6 +119,12 @@ export type EntityType = 'comment' | 'trackedChange';
118119
export type CommentAddress = {
119120
kind: 'entity';
120121
entityType: 'comment';
122+
/**
123+
* Comment navigation is currently body-scoped only.
124+
*
125+
* Unlike bookmark and tracked-change navigation, `navigateTo()` does not yet
126+
* accept a `story` locator for comments.
127+
*/
121128
entityId: string;
122129
};
123130

@@ -145,6 +152,10 @@ export type EntityAddress = CommentAddress | TrackedChangeAddress;
145152
* queries (e.g. `query.match`, `find`, `getNode`) as the `nodeId`.
146153
*
147154
* When `nodeType` is omitted, the lookup searches across all block types.
155+
*
156+
* Block navigation is currently body-scoped only. For non-body stories such
157+
* as headers, footers, footnotes, and endnotes, `navigateTo()` currently
158+
* supports story-aware bookmark and tracked-change targets instead.
148159
*/
149160
export type BlockNavigationAddress = {
150161
kind: 'block';
@@ -156,8 +167,13 @@ export type BlockNavigationAddress = {
156167
* Union of all address types accepted by `navigateTo()`.
157168
*
158169
* Supports navigation to:
159-
* - Blocks by `nodeId` (paragraphs, headings, tables, images, SDTs)
160-
* - Comments by `entityId`
161-
* - Tracked changes by `entityId`
170+
* - Blocks by `nodeId` in the body story
171+
* - Bookmarks by `name`, optionally scoped to a `story`
172+
* - Comments by `entityId` in the body story
173+
* - Tracked changes by `entityId`, optionally scoped to a `story`
174+
*
175+
* Story-aware navigation is currently supported for bookmarks and tracked
176+
* changes. Block and comment targets remain body-only until the runtime gains
177+
* equivalent non-body resolution paths.
162178
*/
163-
export type NavigableAddress = BlockNavigationAddress | CommentAddress | TrackedChangeAddress;
179+
export type NavigableAddress = BlockNavigationAddress | BookmarkAddress | CommentAddress | TrackedChangeAddress;

0 commit comments

Comments
 (0)