diff --git a/apps/docs/document-api/reference/_generated-manifest.json b/apps/docs/document-api/reference/_generated-manifest.json index 73084acd22..bc73124d46 100644 --- a/apps/docs/document-api/reference/_generated-manifest.json +++ b/apps/docs/document-api/reference/_generated-manifest.json @@ -1078,5 +1078,5 @@ } ], "marker": "{/* GENERATED FILE: DO NOT EDIT. Regenerate via `pnpm run docapi:sync`. */}", - "sourceHash": "5f439c4117bbb2e55f227e7711df415c1173f8c476954c6e412a4b8b45edd1a3" + "sourceHash": "608536b99749f3a5a1988fa9d39bfafb41aa3814485b64df6812050db98e9d0f" } diff --git a/apps/docs/document-api/reference/comments/get.mdx b/apps/docs/document-api/reference/comments/get.mdx index 8224e77ea9..f28f3bfbfe 100644 --- a/apps/docs/document-api/reference/comments/get.mdx +++ b/apps/docs/document-api/reference/comments/get.mdx @@ -65,7 +65,7 @@ Returns a CommentInfo object with the comment text, author, date, and thread met | `trackedChangeLink` | CommentTrackedChangeLink \| null | no | One of: CommentTrackedChangeLink, null | | `trackedChangeStory` | StoryLocator \| null | no | One of: StoryLocator, null | | `trackedChangeText` | string \| null | no | | -| `trackedChangeType` | enum | no | `"insert"`, `"delete"`, `"replacement"`, `"format"` | +| `trackedChangeType` | enum | no | `"insert"`, `"delete"`, `"replacement"`, `"format"`, `"structural"` | ### Example response @@ -206,7 +206,8 @@ Returns a CommentInfo object with the comment text, author, date, and thread met "insert", "delete", "replacement", - "format" + "format", + "structural" ] } }, diff --git a/apps/docs/document-api/reference/comments/list.mdx b/apps/docs/document-api/reference/comments/list.mdx index baf327be4a..b55c4b2be2 100644 --- a/apps/docs/document-api/reference/comments/list.mdx +++ b/apps/docs/document-api/reference/comments/list.mdx @@ -222,7 +222,8 @@ Returns a CommentsListResult with an array of comment threads and total count. "insert", "delete", "replacement", - "format" + "format", + "structural" ] } }, diff --git a/apps/docs/document-api/reference/extract.mdx b/apps/docs/document-api/reference/extract.mdx index 1f39be85d6..ed33e0c95e 100644 --- a/apps/docs/document-api/reference/extract.mdx +++ b/apps/docs/document-api/reference/extract.mdx @@ -309,7 +309,8 @@ _No fields._ "insert", "delete", "replacement", - "format" + "format", + "structural" ], "type": "string" }, diff --git a/apps/docs/document-api/reference/track-changes/decide.mdx b/apps/docs/document-api/reference/track-changes/decide.mdx index d44816985b..d1761b8ff9 100644 --- a/apps/docs/document-api/reference/track-changes/decide.mdx +++ b/apps/docs/document-api/reference/track-changes/decide.mdx @@ -125,6 +125,10 @@ Returns a Receipt confirming the decision was applied; reports NO_OP if the chan "id": { "type": "string" }, + "range": { + "description": "Partial-range qualifier on an id target. Rejected with INVALID_INPUT for indivisible (e.g. structural) revisions.", + "type": "object" + }, "story": { "$ref": "#/$defs/StoryLocator" } diff --git a/apps/docs/document-api/reference/track-changes/get.mdx b/apps/docs/document-api/reference/track-changes/get.mdx index 6ed1052cbb..86b5257bbe 100644 --- a/apps/docs/document-api/reference/track-changes/get.mdx +++ b/apps/docs/document-api/reference/track-changes/get.mdx @@ -20,7 +20,7 @@ Retrieve a single tracked change by ID. ## Expected result -Returns a TrackChangeInfo object with the change type (`insert`, `delete`, `replacement`, `format`), author, date, affected content, and raw imported Word OOXML revision IDs (`w:id`) when available. +Returns a TrackChangeInfo object with the change type (`insert`, `delete`, `replacement`, `format`, `structural`), author, date, affected content, and raw imported Word OOXML revision IDs (`w:id`) when available. Structural changes (whole-table insert/delete) carry a `subtype` of `table-insert` or `table-delete`. ## Input fields @@ -60,7 +60,8 @@ Returns a TrackChangeInfo object with the change type (`insert`, `delete`, `repl | `id` | string | yes | | | `insertedText` | string | no | | | `pairedWithChangeId` | string \| null | no | | -| `type` | enum | yes | `"insert"`, `"delete"`, `"replacement"`, `"format"` | +| `subtype` | enum | no | `"table-insert"`, `"table-delete"` | +| `type` | enum | yes | `"insert"`, `"delete"`, `"replacement"`, `"format"`, `"structural"` | | `wordRevisionIds` | object | no | | | `wordRevisionIds.delete` | string | no | | | `wordRevisionIds.format` | string | no | | @@ -81,7 +82,7 @@ Returns a TrackChangeInfo object with the change type (`insert`, `delete`, `repl }, "grouping": "standalone", "id": "id-001", - "pairedWithChangeId": null, + "subtype": "table-insert", "type": "insert" } ``` @@ -161,12 +162,20 @@ Returns a TrackChangeInfo object with the change type (`insert`, `delete`, `repl "null" ] }, + "subtype": { + "description": "Finer classification for structural changes (type === 'structural').", + "enum": [ + "table-insert", + "table-delete" + ] + }, "type": { "enum": [ "insert", "delete", "replacement", - "format" + "format", + "structural" ] }, "wordRevisionIds": { diff --git a/apps/docs/document-api/reference/track-changes/list.mdx b/apps/docs/document-api/reference/track-changes/list.mdx index 2725a23a83..c8217e8f03 100644 --- a/apps/docs/document-api/reference/track-changes/list.mdx +++ b/apps/docs/document-api/reference/track-changes/list.mdx @@ -20,7 +20,7 @@ List all tracked changes in the document. ## Expected result -Returns a TrackChangesListResult with tracked change entries (`insert`, `delete`, `replacement`, `format`), total count, and raw imported Word OOXML revision IDs (`w:id`) when available. +Returns a TrackChangesListResult with tracked change entries (`insert`, `delete`, `replacement`, `format`, `structural`), total count, and raw imported Word OOXML revision IDs (`w:id`) when available. Structural changes (whole-table insert/delete) carry a `subtype` of `table-insert` or `table-delete`. ## Input fields @@ -29,7 +29,7 @@ Returns a TrackChangesListResult with tracked change entries (`insert`, `delete` | `in` | StoryLocator \| `"all"` | no | One of: StoryLocator, `"all"` | | `limit` | integer | no | | | `offset` | integer | no | | -| `type` | enum | no | `"insert"`, `"delete"`, `"replacement"`, `"format"` | +| `type` | enum | no | `"insert"`, `"delete"`, `"replacement"`, `"format"`, `"structural"` | ### Example request @@ -75,7 +75,7 @@ Returns a TrackChangesListResult with tracked change entries (`insert`, `delete` "targetKind": "text" }, "id": "id-001", - "pairedWithChangeId": null, + "subtype": "table-insert", "type": "insert" } ], @@ -123,12 +123,13 @@ Returns a TrackChangesListResult with tracked change entries (`insert`, `delete` "type": "integer" }, "type": { - "description": "Filter by change type: 'insert', 'delete', 'replacement', or 'format'.", + "description": "Filter by change type: 'insert', 'delete', 'replacement', 'format', or 'structural'.", "enum": [ "insert", "delete", "replacement", - "format" + "format", + "structural" ] } }, @@ -192,12 +193,20 @@ Returns a TrackChangesListResult with tracked change entries (`insert`, `delete` "null" ] }, + "subtype": { + "description": "Finer classification for structural changes (type === 'structural').", + "enum": [ + "table-insert", + "table-delete" + ] + }, "type": { "enum": [ "insert", "delete", "replacement", - "format" + "format", + "structural" ] }, "wordRevisionIds": { diff --git a/apps/mcp/src/generated/catalog.ts b/apps/mcp/src/generated/catalog.ts index 7b994255c4..5e82d289b1 100644 --- a/apps/mcp/src/generated/catalog.ts +++ b/apps/mcp/src/generated/catalog.ts @@ -2442,9 +2442,9 @@ export const MCP_TOOL_CATALOG = { "Number of tracked changes to skip for pagination. Only for action 'list'. Omit for other actions.", }, type: { - enum: ['insert', 'delete', 'replacement', 'format'], + enum: ['insert', 'delete', 'replacement', 'format', 'structural'], description: - "Filter by change type: 'insert', 'delete', 'replacement', or 'format'. Only for action 'list'. Omit for other actions.", + "Filter by change type: 'insert', 'delete', 'replacement', 'format', or 'structural'. Only for action 'list'. Omit for other actions.", }, force: { type: 'boolean', @@ -2471,6 +2471,11 @@ export const MCP_TOOL_CATALOG = { story: { $ref: '#/$defs/StoryLocator', }, + range: { + type: 'object', + description: + 'Partial-range qualifier on an id target. Rejected with INVALID_INPUT for indivisible (e.g. structural) revisions.', + }, }, additionalProperties: false, required: ['id'], diff --git a/packages/document-api/scripts/lib/reference-docs-artifacts.test.ts b/packages/document-api/scripts/lib/reference-docs-artifacts.test.ts index 9f9c7363aa..0d5cafdd18 100644 --- a/packages/document-api/scripts/lib/reference-docs-artifacts.test.ts +++ b/packages/document-api/scripts/lib/reference-docs-artifacts.test.ts @@ -12,12 +12,19 @@ describe('reference docs artifacts', () => { const trackedChangeGet = artifacts.get('apps/docs/document-api/reference/track-changes/get.mdx'); expect(trackedChangeGet).toBeDefined(); expect(trackedChangeGet!).toContain('| `pairedWithChangeId` | string \\| null | no | |'); - expect(trackedChangeGet!).toContain('"pairedWithChangeId": null'); const trackedChangeList = artifacts.get('apps/docs/document-api/reference/track-changes/list.mdx'); expect(trackedChangeList).toBeDefined(); expect(trackedChangeList!).toContain('| `in` | StoryLocator \\| `"all"` | no | One of: StoryLocator, `"all"` |'); - expect(trackedChangeList!).toContain('"pairedWithChangeId": null'); + + // Nullable-primitive example values are rendered as `null`. (track-changes + // examples no longer surface a nullable primitive once `subtype` joined the + // optional-field budget, so assert this on header-footers/get, which still + // surfaces the nullable `refId` in its generated example.) + const headerFooterGet = artifacts.get('apps/docs/document-api/reference/header-footers/get.mdx'); + expect(headerFooterGet).toBeDefined(); + expect(headerFooterGet!).toContain('| `refId` | string \\| null | no | |'); + expect(headerFooterGet!).toContain('"refId": null'); const commentsGet = artifacts.get('apps/docs/document-api/reference/comments/get.mdx'); expect(commentsGet).toBeDefined(); diff --git a/packages/document-api/scripts/lib/reference-docs-artifacts.ts b/packages/document-api/scripts/lib/reference-docs-artifacts.ts index 900ca2b009..94932c9d32 100644 --- a/packages/document-api/scripts/lib/reference-docs-artifacts.ts +++ b/packages/document-api/scripts/lib/reference-docs-artifacts.ts @@ -1016,6 +1016,17 @@ function getOperationExamples( snapshot: ReturnType, ): { input: unknown; output: unknown } { const inputOverrides: Partial> = { + // The id-target variant carries an optional `range` qualifier used only to + // fail closed (INVALID_INPUT) on indivisible revisions. A canonical id + // decision does NOT pass it, so the auto-generated example's `"range": {}` + // is misleading — pin an explicit clean id-target example here. + 'trackChanges.decide': { + decision: 'accept', + target: { + id: 'id-001', + story: { kind: 'story', storyType: 'body' }, + }, + }, insert: { target: { kind: 'block', diff --git a/packages/document-api/src/contract/operation-definitions.ts b/packages/document-api/src/contract/operation-definitions.ts index df14717b87..ed05e32384 100644 --- a/packages/document-api/src/contract/operation-definitions.ts +++ b/packages/document-api/src/contract/operation-definitions.ts @@ -2481,7 +2481,7 @@ export const OPERATION_DEFINITIONS = { memberPath: 'trackChanges.list', description: 'List all tracked changes in the document.', expectedResult: - 'Returns a TrackChangesListResult with tracked change entries (`insert`, `delete`, `replacement`, `format`), total count, and raw imported Word OOXML revision IDs (`w:id`) when available.', + 'Returns a TrackChangesListResult with tracked change entries (`insert`, `delete`, `replacement`, `format`, `structural`), total count, and raw imported Word OOXML revision IDs (`w:id`) when available. Structural changes (whole-table insert/delete) carry a `subtype` of `table-insert` or `table-delete`.', requiresDocumentContext: true, metadata: readOperation({ idempotency: 'idempotent', @@ -2496,7 +2496,7 @@ export const OPERATION_DEFINITIONS = { memberPath: 'trackChanges.get', description: 'Retrieve a single tracked change by ID.', expectedResult: - 'Returns a TrackChangeInfo object with the change type (`insert`, `delete`, `replacement`, `format`), author, date, affected content, and raw imported Word OOXML revision IDs (`w:id`) when available.', + 'Returns a TrackChangeInfo object with the change type (`insert`, `delete`, `replacement`, `format`, `structural`), author, date, affected content, and raw imported Word OOXML revision IDs (`w:id`) when available. Structural changes (whole-table insert/delete) carry a `subtype` of `table-insert` or `table-delete`.', requiresDocumentContext: true, metadata: readOperation({ idempotency: 'idempotent', diff --git a/packages/document-api/src/contract/schemas.ts b/packages/document-api/src/contract/schemas.ts index c6cf1048e0..cff60c5b1c 100644 --- a/packages/document-api/src/contract/schemas.ts +++ b/packages/document-api/src/contract/schemas.ts @@ -18,7 +18,7 @@ import { Z_ORDER_RELATIVE_HEIGHT_MAX, Z_ORDER_RELATIVE_HEIGHT_MIN } from '../ima type JsonSchema = Record; -const trackChangeTypeValues = ['insert', 'delete', 'replacement', 'format'] as const; +const trackChangeTypeValues = ['insert', 'delete', 'replacement', 'format', 'structural'] as const; /** JSON Schema descriptors for a single operation's input, output, and result variants. */ export interface OperationSchemaSet { @@ -1560,6 +1560,10 @@ const trackChangeInfoSchema = objectSchema( address: trackedChangeAddressSchema, id: { type: 'string' }, type: { enum: [...trackChangeTypeValues] }, + subtype: { + enum: ['table-insert', 'table-delete'], + description: "Finer classification for structural changes (type === 'structural').", + }, grouping: { enum: ['standalone', 'replacement-pair', 'unknown'] }, pairedWithChangeId: { type: ['string', 'null'] }, wordRevisionIds: trackChangeWordRevisionIdsSchema, @@ -1578,6 +1582,10 @@ const trackChangeDomainItemSchema = discoveryItemSchema( { address: trackedChangeAddressSchema, type: { enum: [...trackChangeTypeValues] }, + subtype: { + enum: ['table-insert', 'table-delete'], + description: "Finer classification for structural changes (type === 'structural').", + }, grouping: { enum: ['standalone', 'replacement-pair', 'unknown'] }, pairedWithChangeId: { type: ['string', 'null'] }, wordRevisionIds: trackChangeWordRevisionIdsSchema, @@ -5028,7 +5036,7 @@ const operationSchemas: Record = { offset: { type: 'integer', description: 'Number of tracked changes to skip for pagination.' }, type: { enum: [...trackChangeTypeValues], - description: "Filter by change type: 'insert', 'delete', 'replacement', or 'format'.", + description: "Filter by change type: 'insert', 'delete', 'replacement', 'format', or 'structural'.", }, in: { oneOf: [storyLocatorSchema, { const: 'all' }], @@ -5049,7 +5057,22 @@ const operationSchemas: Record = { decision: { enum: ['accept', 'reject'] }, target: { oneOf: [ - objectSchema({ id: { type: 'string' }, story: storyLocatorSchema }, ['id']), + objectSchema( + { + id: { type: 'string' }, + story: storyLocatorSchema, + // A partial-range qualifier on an entity (id) target. Accepted by + // the schema so the executor can fail closed with INVALID_INPUT + // on indivisible (e.g. structural whole-object) revisions rather + // than the runtime rejecting it as a malformed target. + range: { + type: 'object', + description: + 'Partial-range qualifier on an id target. Rejected with INVALID_INPUT for indivisible (e.g. structural) revisions.', + }, + }, + ['id'], + ), objectSchema( { kind: { const: 'range' }, diff --git a/packages/document-api/src/track-changes/track-changes.test.ts b/packages/document-api/src/track-changes/track-changes.test.ts index 12cb75bc73..f0c14c136a 100644 --- a/packages/document-api/src/track-changes/track-changes.test.ts +++ b/packages/document-api/src/track-changes/track-changes.test.ts @@ -116,4 +116,18 @@ describe('executeTrackChangesDecide validation', () => { } as any), ).toThrow(/exactly one/); }); + + it('fails closed with INVALID_INPUT for a partial-range qualifier on an id target', () => { + const adapter = stubAdapter(); + const result = executeTrackChangesDecide(adapter, { + decision: 'accept', + target: { id: 'tc1', range: { kind: 'partial', start: 0, end: 2 } } as any, + }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.failure.code).toBe('INVALID_INPUT'); + } + // The whole change must not be resolved as a side effect. + expect(adapter.accept).not.toHaveBeenCalled(); + }); }); diff --git a/packages/document-api/src/track-changes/track-changes.ts b/packages/document-api/src/track-changes/track-changes.ts index d4734b1ce1..5ce962f63a 100644 --- a/packages/document-api/src/track-changes/track-changes.ts +++ b/packages/document-api/src/track-changes/track-changes.ts @@ -256,6 +256,23 @@ export function executeTrackChangesDecide( { field: 'target.story', value: rawStory }, ); } + // A partial-range qualifier on an entity (id) target requests a partial + // decision on a single logical change. Indivisible changes (structural + // whole-object revisions per spec §8/§9/§19) must fail closed and leave the + // document unmutated. No divisible-by-id-range path is fixture-backed yet, + // so reject the qualifier here with INVALID_INPUT rather than silently + // resolving the whole change. + if (target.range !== undefined) { + return { + success: false, + failure: { + code: 'INVALID_INPUT', + message: + 'trackChanges.decide does not support a partial range on an id target; the change is not safely divisible.', + details: { target: input.target }, + }, + }; + } if (typeof target.id !== 'string' || target.id.length === 0) { throw new DocumentApiValidationError('INVALID_TARGET', 'trackChanges.decide id targets require a non-empty id.', { field: 'target', diff --git a/packages/document-api/src/types/track-changes.types.ts b/packages/document-api/src/types/track-changes.types.ts index 37e570d684..1a1b8bd2c2 100644 --- a/packages/document-api/src/types/track-changes.types.ts +++ b/packages/document-api/src/types/track-changes.types.ts @@ -2,7 +2,13 @@ import type { TrackedChangeAddress } from './address.js'; import type { DiscoveryOutput } from './discovery.js'; import type { StoryLocator } from './story.types.js'; -export type TrackChangeType = 'insert' | 'delete' | 'replacement' | 'format'; +export type TrackChangeType = 'insert' | 'delete' | 'replacement' | 'format' | 'structural'; +/** + * Finer classification for structural (`type === 'structural'`) tracked + * changes. `table-insert` / `table-delete` describe a whole-table insert or + * delete revision. + */ +export type TrackChangeSubtype = 'table-insert' | 'table-delete'; export type TrackChangeOverlapRelationship = 'parent' | 'child' | 'standalone'; export type TrackChangeGrouping = 'standalone' | 'replacement-pair' | 'unknown'; export type TrackChangeProvenanceOrigin = 'word' | 'google-docs' | 'superdoc' | 'custom' | 'unknown'; @@ -47,6 +53,8 @@ export interface TrackChangeInfo { /** Convenience alias for `address.entityId`. */ id: string; type: TrackChangeType; + /** Finer classification for structural changes (e.g. `table-insert`). */ + subtype?: TrackChangeSubtype; grouping?: TrackChangeGrouping; pairedWithChangeId?: string | null; /** Raw imported Word OOXML revision IDs (`w:id`) from the source document when available. */ @@ -87,6 +95,8 @@ export interface TrackChangesListQuery { export interface TrackChangeDomain { address: TrackedChangeAddress; type: TrackChangeType; + /** Finer classification for structural changes (e.g. `table-insert`). */ + subtype?: TrackChangeSubtype; grouping?: TrackChangeGrouping; pairedWithChangeId?: string | null; /** Raw imported Word OOXML revision IDs (`w:id`) from the source document when available. */ diff --git a/packages/layout-engine/contracts/src/author-colors.test.ts b/packages/layout-engine/contracts/src/author-colors.test.ts index e335f0f179..f0e14db9dd 100644 --- a/packages/layout-engine/contracts/src/author-colors.test.ts +++ b/packages/layout-engine/contracts/src/author-colors.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import { composeAuthorColorResolver, fallbackAuthorColor, stampTrackedChangeColors } from './author-colors.js'; -import type { FlowBlock, ParagraphBlock, TextRun } from './index.js'; +import type { FlowBlock, ParagraphBlock, TableBlock, TextRun } from './index.js'; describe('composeAuthorColorResolver', () => { it('returns undefined when config is missing or disabled', () => { @@ -94,4 +94,40 @@ describe('stampTrackedChangeColors', () => { stampTrackedChangeColors([makeParagraph(run)], composeAuthorColorResolver({ overrides: {} })!); expect((run as TextRun).color).toBeUndefined(); }); + + it('stamps color on a structural row-level tracked change', () => { + const table: TableBlock = { + kind: 'table', + id: 't1', + rows: [ + { + id: 'r1', + attrs: { trackedChange: { kind: 'insert', id: 'row-tc1', author: 'Alice' } }, + cells: [{ id: 'c1', paragraph: { kind: 'paragraph', id: 'p1', runs: [] } }], + }, + ], + }; + + stampTrackedChangeColors([table], composeAuthorColorResolver({ overrides: { Alice: '#abcdef' } })!); + + expect(table.rows[0]!.attrs?.trackedChange?.color).toBe('#abcdef'); + }); + + it('clears stale color on a row-level tracked change when no resolver is provided', () => { + const table: TableBlock = { + kind: 'table', + id: 't1', + rows: [ + { + id: 'r1', + attrs: { trackedChange: { kind: 'delete', id: 'row-tc2', author: 'Bob', color: '#abcdef' } }, + cells: [{ id: 'c1', paragraph: { kind: 'paragraph', id: 'p1', runs: [] } }], + }, + ], + }; + + stampTrackedChangeColors([table], undefined); + + expect(table.rows[0]!.attrs?.trackedChange?.color).toBeUndefined(); + }); }); diff --git a/packages/layout-engine/contracts/src/author-colors.ts b/packages/layout-engine/contracts/src/author-colors.ts index 3b4206a612..4fac3729ca 100644 --- a/packages/layout-engine/contracts/src/author-colors.ts +++ b/packages/layout-engine/contracts/src/author-colors.ts @@ -185,6 +185,11 @@ const stampBlockTrackedChangeColors = ( } case 'table': { for (const row of block.rows) { + // Structural row-level tracked change (inserted/deleted row) reuses the + // same per-author color stamping as inline runs. + if (row.attrs?.trackedChange) { + applyColorToLayer(row.attrs.trackedChange, resolve); + } for (const cell of row.cells) { stampBlockTrackedChangeColors(cell.paragraph, resolve); if (Array.isArray(cell.blocks)) { diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index e0e070b9e1..9fee39afe8 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -767,6 +767,15 @@ export type TableRowAttrs = { value: number; rule?: 'auto' | 'atLeast' | 'exact' | string; }; + /** + * Structural tracked change on the whole row (inserted/deleted row), imported + * from ``/`` inside ``. Reuses the same shared + * {@link TrackedChangeMeta} shape that inline runs carry, so one painter + + * color-stamping system handles both inline and structural tracked changes. + * `kind` is `'insert'` for an inserted row and `'delete'` for a deleted row. + * `color` is stamped downstream by {@link stampTrackedChangeColors}. + */ + trackedChange?: TrackedChangeMeta; }; export type TableRow = { diff --git a/packages/layout-engine/painters/dom/src/runs/index.ts b/packages/layout-engine/painters/dom/src/runs/index.ts index 6d3b5cb9a1..35d1a6bd1e 100644 --- a/packages/layout-engine/painters/dom/src/runs/index.ts +++ b/packages/layout-engine/painters/dom/src/runs/index.ts @@ -8,7 +8,11 @@ export { renderInlineTabRun, renderPositionedTabRun } from './tab-run.js'; export { appendFormattingParagraphMark, setTextContentWithFormattingSpaceMarks } from './formatting-marks.js'; export { sanitizeUrl, linkMetrics } from './links.js'; export { applyRunDataAttributes } from './hash.js'; -export { resolveTrackedChangesConfig, applyTrackedChangeDecorations } from './tracked-changes.js'; +export { + resolveTrackedChangesConfig, + applyTrackedChangeDecorations, + applyRowTrackedChangeToCell, +} from './tracked-changes.js'; export { resolveRunSdtId, createInlineSdtWrapper, diff --git a/packages/layout-engine/painters/dom/src/runs/tracked-changes.ts b/packages/layout-engine/painters/dom/src/runs/tracked-changes.ts index 2453cf7932..9e42ebd002 100644 --- a/packages/layout-engine/painters/dom/src/runs/tracked-changes.ts +++ b/packages/layout-engine/painters/dom/src/runs/tracked-changes.ts @@ -138,6 +138,71 @@ export const resolveTrackedChangesConfig = (block: ParagraphBlock): TrackedChang return { mode, enabled }; }; +/** + * Marks a row-level tracked-change cell so block-context CSS (cell tint / + * strikethrough / collapse) can target it without colliding with the inline + * `.track-insert-dec` / `.track-delete-dec` span rules. + */ +const TRACK_CHANGE_ROW_CELL_CLASS = 'track-row-cell-dec'; + +/** + * Applies a structural row-level tracked change (inserted/deleted whole row) to + * a single table cell element, reusing the exact same machinery as inline runs: + * the shared {@link TrackedChangeMeta}, the `TRACK_CHANGE_BASE_CLASS` + * (`track-insert-dec` / `track-delete-dec`), the `TRACK_CHANGE_MODIFIER_CLASS` + * mode map (insert → review:highlighted / original:hidden / final:normal; + * delete → review:highlighted / original:normal / final:hidden), and + * `applyAuthorColorVariables` for the per-author color CSS variable family. + * + * The painter renders a row as cells appended to a container (there is no + * `` element), so the row's tracked-change visual is applied to each cell. + * Boundary-safe: this lives in the painter and only reads paint-ready + * `TrackedChangeMeta` from contracts. + * + * @param elem - The cell element to decorate. + * @param meta - The row's resolved tracked-change metadata. + * @param config - Tracked-changes mode/enabled (same source inline runs use). + */ +export const applyRowTrackedChangeToCell = ( + elem: HTMLElement, + meta: TrackedChangeMeta, + config: TrackedChangesRenderConfig, +): void => { + if (!config.enabled || config.mode === 'off') { + return; + } + if (meta.kind !== 'insert' && meta.kind !== 'delete') { + return; + } + + const baseClass = TRACK_CHANGE_BASE_CLASS[meta.kind]; + if (baseClass) { + elem.classList.add(baseClass); + } + elem.classList.add(TRACK_CHANGE_ROW_CELL_CLASS); + + const modifier = TRACK_CHANGE_MODIFIER_CLASS[meta.kind]?.[config.mode]; + if (modifier) { + elem.classList.add(modifier); + } + + applyAuthorColorVariables(elem, meta); + + elem.dataset.trackChangeId = meta.id; + elem.dataset.trackChangeKind = meta.kind; + elem.dataset.trackChangeStructural = 'row'; + elem.dataset.storyKey = meta.storyKey ?? 'body'; + if (meta.author) { + elem.dataset.trackChangeAuthor = meta.author; + } + if (meta.authorEmail) { + elem.dataset.trackChangeAuthorEmail = meta.authorEmail; + } + if (meta.date) { + elem.dataset.trackChangeDate = meta.date; + } +}; + export const applyTrackedChangeDecorations = ( elem: HTMLElement, run: Run, diff --git a/packages/layout-engine/painters/dom/src/styles.ts b/packages/layout-engine/painters/dom/src/styles.ts index 0a6198befc..c3d5f4a109 100644 --- a/packages/layout-engine/painters/dom/src/styles.ts +++ b/packages/layout-engine/painters/dom/src/styles.ts @@ -339,6 +339,45 @@ const TRACK_CHANGE_STYLES = ` .superdoc-layout .track-format-dec.highlighted.track-change-focused { background-color: var(--sd-tracked-changes-format-background-focused, #ffd70033); } + +/* + * Structural row-level tracked changes (inserted/deleted whole rows). + * + * The painter renders a row as absolutely-positioned cell
s (no ), so + * each cell of a tracked row carries the same base class (track-insert-dec / + * track-delete-dec) + modifier (highlighted / hidden) as inline runs, plus the + * block-context marker class track-row-cell-dec. These rules reuse the same + * --sd-tracked-changes-insert-* / --sd-tracked-changes-delete-* CSS variables so + * the per-author color flows through identically to the inline path. + * + * 'hidden' mode collapses the cell (and therefore the row) via the existing + * .track-insert-dec.hidden / .track-delete-dec.hidden { display: none } rule + * above: an inserted row in 'original' mode and a deleted row in 'final' mode + * disappear, matching inline behavior. + */ +.superdoc-layout .track-row-cell-dec.track-insert-dec.highlighted { + background-color: var(--sd-tracked-changes-insert-background, #399c7222); + border-top: var(--sd-tracked-changes-insert-border-width, 2px) solid + var(--sd-tracked-changes-insert-border, #00853d); + border-bottom: var(--sd-tracked-changes-insert-border-width, 2px) solid + var(--sd-tracked-changes-insert-border, #00853d); +} + +.superdoc-layout .track-row-cell-dec.track-delete-dec.highlighted { + background-color: var(--sd-tracked-changes-delete-background, #cb0e4722); + border-top: var(--sd-tracked-changes-delete-border-width, 2px) solid + var(--sd-tracked-changes-delete-border, #cb0e47); + border-bottom: var(--sd-tracked-changes-delete-border-width, 2px) solid + var(--sd-tracked-changes-delete-border, #cb0e47); +} + +.superdoc-layout .track-row-cell-dec.track-delete-dec.highlighted .superdoc-line { + text-decoration: + line-through + solid + var(--sd-tracked-changes-delete-text, #cb0e47) + var(--sd-tracked-changes-delete-decoration-thickness, 2px); +} `; const FORMATTING_MARKS_STYLES = ` diff --git a/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts index 4c1661ced4..338934a1e7 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts @@ -3,6 +3,13 @@ import { renderTableRow } from './renderTableRow.js'; const renderTableCellMock = vi.fn(() => ({ cellElement: document.createElement('div') })); +const makeParagraph = (trackedChangesMode?: string, trackedChangesEnabled?: boolean) => ({ + kind: 'paragraph', + id: 'p1', + runs: [], + attrs: { trackedChangesMode, trackedChangesEnabled }, +}); + vi.mock('./renderTableCell.js', () => ({ renderTableCell: (args: unknown) => renderTableCellMock(args), })); @@ -345,4 +352,95 @@ describe('renderTableRow', () => { expect(calls[1].x).toBe(4); }); }); + + describe('structural row tracked changes', () => { + const trackedRowDeps = ( + kind: 'insert' | 'delete', + mode: string, + overrides: Record = {}, + ): Record => + createDeps({ + rowIndex: 0, + totalRows: 1, + cellSpacingPx: 0, + columnWidths: [100, 100], + rowMeasure: { + height: 20, + cells: [ + { width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }, + { width: 100, height: 20, gridColumnStart: 1, colSpan: 1, rowSpan: 1 }, + ], + }, + row: { + id: 'row-1', + attrs: { + trackedChange: { kind, id: `row-tc-${kind}`, author: 'Alice', color: '#abcdef' }, + }, + cells: [ + { id: 'c1', blocks: [makeParagraph(mode, true)] }, + { id: 'c2', blocks: [makeParagraph(mode, true)] }, + ], + }, + ...overrides, + }); + + it('adds the insert class + author-color vars to every cell element of an inserted row', () => { + renderTableRow(trackedRowDeps('insert', 'review') as never); + + const cells = Array.from(container.children) as HTMLElement[]; + expect(cells).toHaveLength(2); + for (const cell of cells) { + expect(cell.classList.contains('track-insert-dec')).toBe(true); + expect(cell.classList.contains('highlighted')).toBe(true); + expect(cell.classList.contains('track-row-cell-dec')).toBe(true); + expect(cell.style.getPropertyValue('--sd-tracked-changes-insert-border')).toBe('#abcdef'); + expect(cell.dataset.trackChangeKind).toBe('insert'); + expect(cell.dataset.trackChangeStructural).toBe('row'); + } + }); + + it('adds the delete class + author-color vars to every cell element of a deleted row', () => { + renderTableRow(trackedRowDeps('delete', 'review') as never); + + const cells = Array.from(container.children) as HTMLElement[]; + expect(cells).toHaveLength(2); + for (const cell of cells) { + expect(cell.classList.contains('track-delete-dec')).toBe(true); + expect(cell.classList.contains('highlighted')).toBe(true); + expect(cell.style.getPropertyValue('--sd-tracked-changes-delete-text')).toBe('#abcdef'); + } + }); + + it("hides an inserted row in 'original' mode (cells get the hidden modifier)", () => { + renderTableRow(trackedRowDeps('insert', 'original') as never); + + const cells = Array.from(container.children) as HTMLElement[]; + for (const cell of cells) { + expect(cell.classList.contains('track-insert-dec')).toBe(true); + expect(cell.classList.contains('hidden')).toBe(true); + expect(cell.classList.contains('highlighted')).toBe(false); + } + }); + + it("hides a deleted row in 'final' mode (cells get the hidden modifier)", () => { + renderTableRow(trackedRowDeps('delete', 'final') as never); + + const cells = Array.from(container.children) as HTMLElement[]; + for (const cell of cells) { + expect(cell.classList.contains('track-delete-dec')).toBe(true); + expect(cell.classList.contains('hidden')).toBe(true); + } + }); + + it('leaves cells of an untracked row undecorated', () => { + renderTableRow(createDeps() as never); + + const cells = Array.from(container.children) as HTMLElement[]; + for (const cell of cells) { + expect(cell.classList.contains('track-insert-dec')).toBe(false); + expect(cell.classList.contains('track-delete-dec')).toBe(false); + expect(cell.classList.contains('track-row-cell-dec')).toBe(false); + } + }); + }); }); diff --git a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts index d3b2a117f2..a1b92be1a2 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts @@ -18,6 +18,8 @@ import { swapCellBordersLR, } from './border-utils.js'; import { getTableCellGridBounds, type TableCellGridPosition } from './grid-geometry.js'; +import { resolveTrackedChangesConfig, applyRowTrackedChangeToCell } from '../runs/tracked-changes.js'; +import type { TrackedChangesRenderConfig } from '../runs/types.js'; import type { FragmentRenderContext } from '../renderer.js'; import type { SdtAncestorOptions } from '../sdt/container.js'; @@ -279,6 +281,29 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { const totalCols = columnWidths.length; + // Structural row-level tracked change (inserted/deleted whole row). Reuses the + // exact same metadata + painter helpers as inline tracked changes. The + // tracked-changes MODE is threaded the same way inline runs get it: from a + // ParagraphBlock's attrs (trackedChangesMode/trackedChangesEnabled) via + // resolveTrackedChangesConfig. FragmentRenderContext carries no mode field, so + // we resolve from a representative paragraph in this row's cells. + const rowTrackedChange = row?.attrs?.trackedChange; + let rowTrackedChangeConfig: TrackedChangesRenderConfig | undefined; + if (rowTrackedChange) { + let representativeParagraph: ParagraphBlock | undefined; + for (const cell of row?.cells ?? []) { + const candidate = + cell.paragraph ?? (cell.blocks?.find((block) => block.kind === 'paragraph') as ParagraphBlock | undefined); + if (candidate) { + representativeParagraph = candidate; + break; + } + } + rowTrackedChangeConfig = representativeParagraph + ? resolveTrackedChangesConfig(representativeParagraph) + : { mode: 'review', enabled: true }; + } + /** * Calculates the horizontal position (x-coordinate) for a cell based on its grid column index. * @@ -455,6 +480,12 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { chrome, }); + // Paint the structural row-level tracked change onto each cell element of + // the row (no exists in the painted DOM), reusing the inline helpers. + if (rowTrackedChange && rowTrackedChangeConfig) { + applyRowTrackedChangeToCell(cellElement, rowTrackedChange, rowTrackedChangeConfig); + } + container.appendChild(cellElement); } }; diff --git a/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css b/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css index 090dbbbff2..7fd224b92c 100644 --- a/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css +++ b/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css @@ -283,6 +283,27 @@ https://github.com/ProseMirror/prosemirror-tables/blob/master/demo/index.html .sd-editor-scoped .ProseMirror .track-delete-widget { visibility: hidden; } + +/* Structural row revisions (whole-table insert/delete). Applied to via + node decorations; reuse the same author-color variables as inline changes. */ +.sd-editor-scoped .ProseMirror tr.track-row-insert-dec.hidden, +.sd-editor-scoped .ProseMirror tr.track-row-delete-dec.hidden { + display: none; +} + +.sd-editor-scoped .ProseMirror tr.track-row-insert-dec.highlighted > td, +.sd-editor-scoped .ProseMirror tr.track-row-insert-dec.highlighted > th { + background-color: var(--sd-tracked-changes-insert-background, #399c7222); + box-shadow: inset 3px 0 0 0 var(--sd-tracked-changes-insert-border, #00853d); +} + +.sd-editor-scoped .ProseMirror tr.track-row-delete-dec.highlighted > td, +.sd-editor-scoped .ProseMirror tr.track-row-delete-dec.highlighted > th { + background-color: var(--sd-tracked-changes-delete-background, #cb0e4722); + box-shadow: inset 3px 0 0 0 var(--sd-tracked-changes-delete-border, #cb0e47); + text-decoration: line-through solid var(--sd-tracked-changes-delete-text, currentColor) + var(--sd-tracked-changes-delete-decoration-thickness, 2px); +} /* Track changes - end */ /* Collaboration cursors */ diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.test.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.test.ts index 4765298947..65e54cf5ae 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.test.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.test.ts @@ -2690,4 +2690,155 @@ describe('tableCellNodeToBlock — SD-2516: documentPartObject children', () => expect(result?.attrs?.tableDirectionContext?.parentSection).toBe(customSectionContext); }); }); + + describe('structural row tracked changes', () => { + const ROW_TRACK_CONFIG: TrackedChangesConfig = { enabled: true, mode: 'review' }; + + const buildTrackedRowTable = (trackChange: Record | null): PMNode => ({ + type: 'table', + content: [ + { + type: 'tableRow', + attrs: trackChange ? { trackChange } : {}, + content: [ + { + type: 'tableCell', + content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Cell' }] }], + }, + ], + }, + ], + }); + + it('produces attrs.trackedChange with kind "insert" for a rowInsert row', () => { + const result = tableNodeToBlock( + buildTrackedRowTable({ + type: 'rowInsert', + id: 'rev-1', + author: 'Alice', + authorEmail: 'alice@example.com', + date: '2024-01-01T00:00:00Z', + }), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + ROW_TRACK_CONFIG, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock; + + const meta = result.rows[0].attrs?.trackedChange; + expect(meta).toBeDefined(); + expect(meta?.kind).toBe('insert'); + expect(meta?.id).toBe('rev-1'); + expect(meta?.author).toBe('Alice'); + expect(meta?.authorEmail).toBe('alice@example.com'); + expect(meta?.date).toBe('2024-01-01T00:00:00Z'); + // Color is stamped downstream, never by the adapter. + expect(meta?.color).toBeUndefined(); + }); + + it('produces attrs.trackedChange with kind "delete" for a rowDelete row', () => { + const result = tableNodeToBlock( + buildTrackedRowTable({ type: 'rowDelete', id: 'rev-2', author: 'Bob' }), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + ROW_TRACK_CONFIG, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock; + + const meta = result.rows[0].attrs?.trackedChange; + expect(meta?.kind).toBe('delete'); + expect(meta?.id).toBe('rev-2'); + }); + + it('omits attrs.trackedChange for an untracked row', () => { + const result = tableNodeToBlock( + buildTrackedRowTable(null), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + ROW_TRACK_CONFIG, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock; + + expect(result.rows[0].attrs?.trackedChange).toBeUndefined(); + }); + + it('omits attrs.trackedChange when tracked changes are disabled', () => { + const result = tableNodeToBlock( + buildTrackedRowTable({ type: 'rowInsert', id: 'rev-3' }), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + { enabled: false, mode: 'review' }, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock; + + expect(result.rows[0].attrs?.trackedChange).toBeUndefined(); + }); + + // View-mode hiding: a hidden tracked row must be dropped from the layout + // entirely (not just CSS-hidden in the painter) so it reserves no blank + // table space. When every row is hidden the whole table block is omitted. + const buildTable = (node: PMNode, config: TrackedChangesConfig) => + tableNodeToBlock( + node, + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + config, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock | null; + + it('omits an inserted row in "original" mode (whole single-row table dropped)', () => { + const result = buildTable(buildTrackedRowTable({ type: 'rowInsert', id: 'r1' }), { + enabled: true, + mode: 'original', + }); + expect(result).toBeNull(); + }); + + it('omits a deleted row in "final" mode (whole single-row table dropped)', () => { + const result = buildTable(buildTrackedRowTable({ type: 'rowDelete', id: 'r1' }), { + enabled: true, + mode: 'final', + }); + expect(result).toBeNull(); + }); + + it('keeps a deleted row in "original" mode and an inserted row in "final" mode', () => { + const del = buildTable(buildTrackedRowTable({ type: 'rowDelete', id: 'r1' }), { + enabled: true, + mode: 'original', + }); + expect(del?.rows).toHaveLength(1); + + const ins = buildTable(buildTrackedRowTable({ type: 'rowInsert', id: 'r1' }), { + enabled: true, + mode: 'final', + }); + expect(ins?.rows).toHaveLength(1); + }); + }); }); diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts index f084386e84..e1460c669b 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts @@ -22,6 +22,7 @@ import type { TableAnchor, TableWrap, SourceAnchor, + TrackedChangeMeta, } from '@superdoc/contracts'; import type { PMNode, @@ -46,7 +47,7 @@ import { import { pickNumber, twipsToPx } from '../utilities.js'; import { hydrateTableStyleAttrs } from './table-styles.js'; import { collectTrackedChangeFromMarks } from '../marks/index.js'; -import { annotateBlockWithTrackedChange, shouldHideTrackedNode } from '../tracked-changes.js'; +import { annotateBlockWithTrackedChange, shouldHideTrackedNode, deriveTrackedChangeId } from '../tracked-changes.js'; import { resolveNodeSdtMetadata, applySdtMetadataToParagraphBlocks, @@ -681,6 +682,48 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { * }); * // Returns: null */ +/** + * Builds shared {@link TrackedChangeMeta} for a structural row-level tracked + * change (inserted/deleted whole row) from the PM `tableRow` node's + * `attrs.trackChange`. Mirrors the inline mark conversion in + * `layout-adapter/tracked-changes.ts` (`buildTrackedChangeMetaFromMark`) and + * reuses `deriveTrackedChangeId` so structural and inline changes share one + * metadata system. The author color is intentionally not set here; it is + * stamped downstream by `stampTrackedChangeColors` in `@superdoc/contracts`. + * + * @param rowNode - PM table row node. + * @param storyKey - Owning content story key, threaded onto the meta. + * @returns The row's TrackedChangeMeta, or undefined when the row is untracked. + */ +const buildRowTrackedChangeMeta = (rowNode: PMNode, storyKey?: string): TrackedChangeMeta | undefined => { + const trackChange = (rowNode.attrs as Record | undefined)?.trackChange as + | Record + | null + | undefined; + if (!trackChange || typeof trackChange !== 'object') return undefined; + const rawType = trackChange.type; + if (rawType !== 'rowInsert' && rawType !== 'rowDelete') return undefined; + + const kind = rawType === 'rowInsert' ? 'insert' : 'delete'; + const meta: TrackedChangeMeta = { + kind, + id: deriveTrackedChangeId(kind, trackChange), + }; + if (typeof trackChange.author === 'string' && trackChange.author) { + meta.author = trackChange.author; + } + if (typeof trackChange.authorEmail === 'string' && trackChange.authorEmail) { + meta.authorEmail = trackChange.authorEmail; + } + if (typeof trackChange.date === 'string' && trackChange.date) { + meta.date = trackChange.date; + } + if (typeof storyKey === 'string' && storyKey.length > 0) { + meta.storyKey = storyKey; + } + return meta; +}; + const parseTableRow = (args: ParseTableRowArgs): TableRow | null => { const { rowNode, rowIndex, context, defaultCellPadding, tableProperties, numRows } = args; if (!isTableRowNode(rowNode) || !Array.isArray(rowNode.content)) { @@ -716,14 +759,23 @@ const parseTableRow = (args: ParseTableRowArgs): TableRow | null => { const rowProps = rowNode.attrs?.tableRowProperties; const rowHeight = normalizeRowHeight(rowProps as Record | undefined); + // Structural row-level tracked change (inserted/deleted whole row). Only + // surfaced when tracked changes are enabled and not in 'off' mode, matching + // how inline tracked-change metadata is gated (annotateBlockWithTrackedChange). + const trackedChangesConfig = context.trackedChangesConfig; + const rowTrackedChange = + trackedChangesConfig?.enabled && trackedChangesConfig.mode !== 'off' + ? buildRowTrackedChangeMeta(rowNode, context.storyKey) + : undefined; const attrs: TableRowAttrs | undefined = rowProps && typeof rowProps === 'object' ? { tableRowProperties: rowProps as Record, ...(rowHeight ? { rowHeight } : {}), + ...(rowTrackedChange ? { trackedChange: rowTrackedChange } : {}), } - : rowHeight - ? { rowHeight } + : rowHeight || rowTrackedChange + ? { ...(rowHeight ? { rowHeight } : {}), ...(rowTrackedChange ? { trackedChange: rowTrackedChange } : {}) } : undefined; // Note: cantSplit is stored within tableRowProperties.cantSplit (not as a separate attr) @@ -935,7 +987,15 @@ export function tableNodeToBlock( tableProperties: tablePropertiesForCascade, }); if (parsedRow) { - rows.push(parsedRow); + // Drop a tracked row from the layout entirely (not just CSS-hide it in the + // painter) when the current view mode removes it — inserted rows in + // "original", deleted rows in "final" — so it never reserves blank table + // space during measurement/pagination. Mirrors `shouldHideTrackedNode` for + // inline content. If every row is hidden, the `rows.length === 0` guard + // below omits the whole table block. + if (!shouldHideTrackedNode(parsedRow.attrs?.trackedChange, parserDeps.trackedChangesConfig)) { + rows.push(parsedRow); + } } }); diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts index cd1f83370e..751b3ab897 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts @@ -2535,10 +2535,98 @@ export class PresentationEditor extends EventEmitter { storyKey: BODY_STORY_KEY, }), ...this.#collectIndexedTrackedChangePositions(), + ...this.#collectStructuralBodyTrackedChangePositions(), ...this.#collectRenderedTrackedChangePositions(), }; } + /** + * Emit position entries for decidable whole-table structural tracked changes + * living in the BODY story (table insert / table delete). + * + * Structural row revisions are whole-table changes that the right rail + * surfaces as review bubbles (see comments-store + * `syncStructuralTrackedChangeComments`). Unlike inline body tracked changes + * (whose marks are measured downstream by mark span) and non-body story + * changes (handled by `#collectIndexedTrackedChangePositions`, which skips the + * body story), a body-story structural change has no inline mark to anchor on. + * + * We key each entry by the tracked-change index `anchorKey` (matching the + * bubble's `trackedChangeAnchorKey`) and carry the table's PM range as + * `start`/`end`. `getCommentBounds` falls through `#getStoryTrackedChangeBounds` + * (null for the body story) into `#getThreadSelectionBounds`, which resolves + * the range to layout rects via `#computeRangeRects(..., forceBodySurface)` — + * the exact path body comments/inline TC use — so the bubble lines up with the + * table in layout-engine viewing mode. + */ + #collectStructuralBodyTrackedChangePositions(): Record< + string, + { + threadId: string; + key: string; + storyKey: string; + kind: 'trackedChange'; + structural: true; + start?: number; + end?: number; + } + > { + const positions: Record< + string, + { + threadId: string; + key: string; + storyKey: string; + kind: 'trackedChange'; + structural: true; + start?: number; + end?: number; + } + > = {}; + + let snapshots: ReadonlyArray<{ + anchorKey?: unknown; + type?: unknown; + runtimeRef?: { rawId?: unknown; storyKey?: unknown }; + range?: { from?: unknown; to?: unknown }; + }> = []; + + try { + snapshots = getTrackedChangeIndex(this.#editor).getAll(); + } catch { + return positions; + } + + snapshots.forEach((snapshot) => { + if (snapshot?.type !== 'structural') return; + const storyKey = + typeof snapshot?.runtimeRef?.storyKey === 'string' ? snapshot.runtimeRef.storyKey : BODY_STORY_KEY; + // Body-story structural changes only — non-body structural would be + // picked up by the rendered/indexed passes which key on their own story. + if (storyKey !== BODY_STORY_KEY) return; + + const key = typeof snapshot?.anchorKey === 'string' ? snapshot.anchorKey : null; + const rawId = snapshot?.runtimeRef?.rawId; + const threadId = rawId == null ? null : String(rawId); + if (!key || !threadId || positions[key]) return; + + const start = Number.isFinite(snapshot?.range?.from) ? Number(snapshot.range.from) : undefined; + const end = Number.isFinite(snapshot?.range?.to) ? Number(snapshot.range.to) : undefined; + + positions[key] = { + threadId, + key, + storyKey, + kind: 'trackedChange', + structural: true, + ...(start !== undefined ? { start } : {}), + ...(end !== undefined ? { end } : {}), + }; + }); + + return positions; + } + #collectIndexedTrackedChangePositions(): Record< string, { diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/row-track-change.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/row-track-change.js new file mode 100644 index 0000000000..ee0fc45c8c --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/row-track-change.js @@ -0,0 +1,103 @@ +// @ts-check +import { + resolveTrackedChangeImportIds, + stampImportTrackingAttrs, +} from '../../../../v2/importer/importTrackingContext.js'; + +/** + * Read a structural tracked-change revision from a row's ``. + * + * A whole inserted/deleted table is encoded in OOXML as `` / `` + * inside each row's `` (ECMA-376 §17.13.5.16 / §17.13.5.13). Unlike + * inline text revisions, the marker carries no content — the `` wrapper + * alone conveys that the row was inserted or deleted. + * + * The id is normalized through the same import context as inline `w:ins`/`w:del` + * so a structural revision pairs correctly with its source `w:id` on export. + * + * @param {{ name: string, elements?: Array<{ name: string, attributes?: Record }> }} [trPr] - The parsed `w:trPr` element. + * @param {import('@translator').SCEncoderConfig} [params] - Encoder params (carries the import tracking context). + * @returns {import('@extensions/table-row/table-row.js').TableRowTrackChange | null} + */ +export function readRowTrackChange(trPr, params = /** @type {any} */ ({})) { + const marker = trPr?.elements?.find((el) => el.name === 'w:ins' || el.name === 'w:del'); + if (!marker) return null; + + const side = marker.name === 'w:ins' ? 'insertion' : 'deletion'; + const attributes = marker.attributes || {}; + + const { sourceId, logicalId } = resolveTrackedChangeImportIds(params, attributes['w:id']); + + /** @type {import('@extensions/table-row/table-row.js').TableRowTrackChange} */ + const trackChange = { + type: side === 'insertion' ? 'rowInsert' : 'rowDelete', + id: logicalId, + sourceId, + author: attributes['w:author'] || '', + authorEmail: attributes['w:authorEmail'] || '', + date: attributes['w:date'] || '', + importedAuthor: `${attributes['w:author'] || ''} (imported)`, + }; + + // Register the logical id with the import context so structural revisions + // share the same id space as inline text revisions. + stampImportTrackingAttrs({ params, attrs: trackChange, side, sourceId }); + + return trackChange; +} + +/** + * Reconstruct the `` / `` revision marker for `` on export + * from a row's `trackChange` attr. The `w:id` is mapped back through the export + * id resolver so the structural revision pairs with its source `w:id`, matching + * inline `w:ins`/`w:del` round-trip behavior. + * + * @param {import('@extensions/table-row/table-row.js').TableRowTrackChange | null | undefined} trackChange + * @param {import('@translator').SCDecoderConfig} [params] - Decoder params (carries the export id allocator). + * @returns {{ name: 'w:ins' | 'w:del', attributes: Record } | null} + */ +export function buildRowTrackChangeElement(trackChange, params = /** @type {any} */ ({})) { + if (!trackChange || (trackChange.type !== 'rowInsert' && trackChange.type !== 'rowDelete')) { + return null; + } + + const name = trackChange.type === 'rowInsert' ? 'w:ins' : 'w:del'; + const wId = resolveExportRowWordId(params, trackChange); + + /** @type {Record} */ + const attributes = { 'w:id': wId }; + if (trackChange.author) attributes['w:author'] = trackChange.author; + if (trackChange.authorEmail) attributes['w:authorEmail'] = trackChange.authorEmail; + if (trackChange.date) attributes['w:date'] = trackChange.date; + + return { name, attributes }; +} + +/** + * Resolve the `w:id` to write for a structural row revision. Mirrors the inline + * `ins`/`del` translators: uses the converter's Word revision id allocator when + * installed, else falls back to `sourceId || id`. + * + * @param {import('@translator').SCDecoderConfig} params + * @param {import('@extensions/table-row/table-row.js').TableRowTrackChange} trackChange + * @returns {string} + */ +function resolveExportRowWordId(params, trackChange) { + const sourceId = trackChange.sourceId; + const logicalId = typeof trackChange.id === 'string' ? trackChange.id : ''; + const exportParams = /** @type {any} */ (params); + const allocator = exportParams?.converter?.wordIdAllocator; + const partPath = + exportParams?.currentPartPath || + (typeof exportParams?.filename === 'string' && exportParams.filename.length > 0 + ? `word/${exportParams.filename}` + : 'word/document.xml'); + if (allocator) { + return allocator.allocate({ + partPath, + sourceId: sourceId === undefined ? undefined : sourceId === null ? null : String(sourceId), + logicalId, + }); + } + return String(sourceId || logicalId || ''); +} diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/tr-translator.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/tr-translator.js index bfb967b014..89966f63a2 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/tr-translator.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/tr-translator.js @@ -9,6 +9,7 @@ import { translator as tblBordersTranslator } from '../tblBorders'; import { translator as trPrTranslator } from '../trPr'; import { advancePastRowSpans, fillPlaceholderColumns, isPlaceholderCell } from './tr-helpers.js'; import { normalizeRowCellChildren } from './row-cell-children.js'; +import { readRowTrackChange, buildRowTrackChangeElement } from './row-track-change.js'; /** @type {import('@translator').XmlNodeName} */ const XML_NODE_NAME = 'w:tr'; @@ -49,6 +50,15 @@ const encode = (params, encodedAttrs) => { nodes: [tPr], }); } + + // Structural tracked-change revision: / inside marks an + // inserted/deleted row. The whitelist-based trPr translator drops these, so read + // them here onto a row-level attribute. The enumeration layer groups rows of one + // table that share a revision into a single logical structural change. + const trackChange = readRowTrackChange(tPr, params); + if (trackChange) { + encodedAttrs['trackChange'] = trackChange; + } const gridBeforeRaw = tableRowProperties?.['gridBefore']; const safeGridBefore = typeof gridBeforeRaw === 'number' && Number.isFinite(gridBeforeRaw) && gridBeforeRaw > 0 ? gridBeforeRaw : 0; @@ -279,6 +289,12 @@ const decode = (params, decodedAttrs) => { if (trPr) elements.unshift(trPr); } + // Structural tracked-change revision: reconstruct / inside + // from the row's trackChange attr (whole-table insert/delete). + // The marker is the first child of w:trPr; create the trPr if no other row + // properties produced one. + applyRowTrackChangeOnDecode({ node, elements, params }); + return { name: 'w:tr', attributes: decodedAttrs || {}, @@ -286,6 +302,26 @@ const decode = (params, decodedAttrs) => { }; }; +/** + * Emit the ``/`` revision marker inside `` from the row's + * `trackChange` attr. Mutates `elements` in place. + * + * @param {{ node: any, elements: any[], params: import('@translator').SCDecoderConfig }} input + */ +const applyRowTrackChangeOnDecode = ({ node, elements, params }) => { + const marker = buildRowTrackChangeElement(node.attrs?.trackChange, params); + if (!marker) return; + + let trPr = elements.find((el) => el && el.name === 'w:trPr'); + if (!trPr) { + trPr = { name: 'w:trPr', elements: [] }; + elements.unshift(trPr); + } + if (!Array.isArray(trPr.elements)) trPr.elements = []; + // ECMA-376: the revision marker is the first child of w:trPr. + trPr.elements.unshift(marker); +}; + /** @type {import('@translator').NodeTranslatorConfig} */ export const config = { xmlName: XML_NODE_NAME, diff --git a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/tr-translator.test.js b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/tr-translator.test.js index b04b3b331b..d3b950cd7b 100644 --- a/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/tr-translator.test.js +++ b/packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/tr/tr-translator.test.js @@ -336,4 +336,138 @@ describe('w:tr translator', () => { expect(translateCall.node.content).toHaveLength(2); }); }); + + describe('structural tracked changes (whole-table insert/delete)', () => { + const rowWithMarker = (markerName) => ({ + name: 'w:tr', + elements: [ + { + name: 'w:trPr', + elements: [ + { + name: markerName, + attributes: { + 'w:id': '7', + 'w:author': 'Alice Reviewer', + 'w:authorEmail': 'alice@example.com', + 'w:date': '2026-05-20T16:00:00Z', + }, + }, + ], + }, + { name: 'w:tc', elements: [] }, + ], + }); + + it('reads in into a rowInsert trackChange attr', () => { + const row = rowWithMarker('w:ins'); + const result = translator.encode({ nodes: [row], extraParams: { row, columnWidths: [100] } }, {}); + + expect(result.attrs.trackChange).toMatchObject({ + type: 'rowInsert', + id: '7', + sourceId: '7', + author: 'Alice Reviewer', + authorEmail: 'alice@example.com', + date: '2026-05-20T16:00:00Z', + }); + }); + + it('reads in into a rowDelete trackChange attr', () => { + const row = rowWithMarker('w:del'); + const result = translator.encode({ nodes: [row], extraParams: { row, columnWidths: [100] } }, {}); + + expect(result.attrs.trackChange).toMatchObject({ type: 'rowDelete', id: '7' }); + }); + + it('leaves trackChange unset for a plain row', () => { + const row = { + name: 'w:tr', + elements: [ + { name: 'w:trPr', elements: [] }, + { name: 'w:tc', elements: [] }, + ], + }; + const result = translator.encode({ nodes: [row], extraParams: { row, columnWidths: [100] } }, {}); + + expect(result.attrs.trackChange).toBeUndefined(); + }); + + describe('export (decode)', () => { + const rowNode = (trackChange) => ({ + type: 'tableRow', + attrs: { trackChange }, + content: [{ type: 'tableCell', attrs: {}, content: [] }], + }); + + it('emits inside for a rowInsert trackChange', () => { + const node = rowNode({ + type: 'rowInsert', + id: '7', + sourceId: '7', + author: 'Alice Reviewer', + authorEmail: 'alice@example.com', + date: '2026-05-20T16:00:00Z', + }); + const result = translator.decode({ node }, {}); + const trPr = result.elements.find((el) => el.name === 'w:trPr'); + expect(trPr).toBeDefined(); + const ins = trPr.elements.find((el) => el.name === 'w:ins'); + expect(ins).toBeDefined(); + expect(ins.attributes).toMatchObject({ + 'w:id': '7', + 'w:author': 'Alice Reviewer', + 'w:authorEmail': 'alice@example.com', + 'w:date': '2026-05-20T16:00:00Z', + }); + // The revision marker is the first child of w:trPr. + expect(trPr.elements[0].name).toBe('w:ins'); + }); + + it('emits inside for a rowDelete trackChange', () => { + const node = rowNode({ type: 'rowDelete', id: '3', sourceId: '3', author: 'Alice Reviewer' }); + const result = translator.decode({ node }, {}); + const trPr = result.elements.find((el) => el.name === 'w:trPr'); + const del = trPr.elements.find((el) => el.name === 'w:del'); + expect(del).toBeDefined(); + expect(del.attributes['w:id']).toBe('3'); + }); + + it('emits no revision marker for a plain row', () => { + const node = rowNode(null); + const result = translator.decode({ node }, {}); + const trPr = result.elements.find((el) => el.name === 'w:trPr'); + // No tableRowProperties and no trackChange → no w:trPr is synthesized. + expect(trPr).toBeUndefined(); + }); + + it('round-trips import → export preserving the trPr ins markup', () => { + const importRow = { + name: 'w:tr', + elements: [ + { + name: 'w:trPr', + elements: [ + { + name: 'w:ins', + attributes: { 'w:id': '7', 'w:author': 'Alice Reviewer', 'w:date': '2026-05-20T16:00:00Z' }, + }, + ], + }, + { name: 'w:tc', elements: [] }, + ], + }; + const encoded = translator.encode( + { nodes: [importRow], extraParams: { row: importRow, columnWidths: [100] } }, + {}, + ); + const exported = translator.decode({ node: encoded }, {}); + const trPr = exported.elements.find((el) => el.name === 'w:trPr'); + const ins = trPr.elements.find((el) => el.name === 'w:ins'); + expect(ins.attributes['w:id']).toBe('7'); + expect(ins.attributes['w:author']).toBe('Alice Reviewer'); + expect(ins.attributes['w:date']).toBe('2026-05-20T16:00:00Z'); + }); + }); + }); }); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/tracked-change-resolver.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/tracked-change-resolver.ts index bf0ac753aa..89d0d23c53 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/tracked-change-resolver.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/tracked-change-resolver.ts @@ -14,7 +14,8 @@ import { TrackInsertMarkName, } from '../../extensions/track-changes/constants.js'; import { getTrackChanges } from '../../extensions/track-changes/trackChangesHelpers/getTrackChanges.js'; -import { toNonEmptyString } from './value-utils.js'; +import { enumerateStructuralRowChanges } from '../../extensions/track-changes/trackChangesHelpers/structuralRowChanges.js'; +import { normalizeExcerpt, toNonEmptyString } from './value-utils.js'; import { resolveStoryRuntime } from '../story-runtime/resolve-story-runtime.js'; import { buildStoryKey, BODY_STORY_KEY } from '../story-runtime/story-key.js'; import type { TrackedChangeRuntimeRef } from './tracked-change-runtime-ref.js'; @@ -42,11 +43,13 @@ export type GroupedTrackedChange = { excerpt?: string; wordRevisionIds?: TrackChangeWordRevisionIds; overlap?: TrackChangeOverlapInfo; + /** Set for whole-object structural revisions (e.g. whole-table insert/delete). */ + structural?: { side: 'insertion' | 'deletion'; subtype: 'table-insert' | 'table-delete' }; }; export type TrackedChangeProjectedSide = 'inserted' | 'deleted'; -type ChangeTypeInput = Pick; +type ChangeTypeInput = Pick; type GroupedTrackedChangeDraft = Omit & { excerptParts: string[]; /** @@ -98,6 +101,7 @@ function deriveTrackedChangeId(change: Omit): string } export function resolveTrackedChangeType(change: ChangeTypeInput): TrackChangeType { + if (change.structural) return 'structural'; if (change.hasFormat) return 'format'; if (change.hasInsert && change.hasDelete) return 'replacement'; if (change.hasDelete) return 'delete'; @@ -383,6 +387,54 @@ export function groupTrackedChanges(editor: Editor): GroupedTrackedChange[] { }); attachOverlapMetadata(grouped); + // Whole-object structural revisions (e.g. whole-table insert/delete) live on + // node attrs, not marks, so they are enumerated separately and appended as + // their own grouped changes. Their `id` is the shared Word revision id; the + // accept/reject command routes by id through the review graph which owns the + // node-level mutation plan. + for (const structural of enumerateStructuralRowChanges(editor.state)) { + const excerpt = normalizeExcerpt(editor.state.doc.textBetween(structural.tableFrom, structural.tableTo, ' ', '')); + // Public id must be stable across import → export → reopen. The logical + // `structural.id` is a fresh UUID minted on each import, so derive the + // public/raw id from the Word revision id (mirrors inline `word::` + // grouping). `commandRawId` keeps the logical id the review graph keys by, + // so accept/reject still routes to the right structural change. + const stableRawId = structural.sourceId ? `word:structural:${structural.sourceId}` : structural.id; + grouped.push({ + rawId: stableRawId, + commandRawId: structural.id, + id: stableRawId, + from: structural.tableFrom, + to: structural.tableTo, + hasInsert: false, + hasDelete: false, + hasFormat: false, + structural: { side: structural.side, subtype: structural.subtype }, + attrs: { + id: structural.id, + sourceId: structural.sourceId || undefined, + author: structural.author || undefined, + authorEmail: structural.authorEmail || undefined, + authorImage: structural.authorImage || undefined, + date: structural.date || undefined, + importedAuthor: structural.importedAuthor || undefined, + origin: structural.sourceId ? 'word' : undefined, + revisionGroupId: structural.revisionGroupId || undefined, + }, + excerpt, + wordRevisionIds: structural.sourceId + ? structural.side === 'insertion' + ? { insert: structural.sourceId } + : { delete: structural.sourceId } + : undefined, + }); + } + + grouped.sort((a, b) => { + if (a.from !== b.from) return a.from - b.from; + return a.id.localeCompare(b.id); + }); + groupedCache.set(editor, { doc: currentDoc, grouped }); return grouped; } diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/track-changes-wrappers.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/track-changes-wrappers.test.ts index 7c4fc2a917..e61a67e606 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/track-changes-wrappers.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/track-changes-wrappers.test.ts @@ -184,6 +184,47 @@ describe('track-changes-wrappers revision guard', () => { }); }); + it('projects subtype for structural changes in list and get', () => { + const hostEditor = makeEditor(); + const structuralSnapshot = { + address: { kind: 'entity', entityType: 'trackedChange', entityId: 'word:structural:2' }, + runtimeRef: { storyKey: 'body', rawId: 'word:structural:2' }, + story: { kind: 'story', storyType: 'body' }, + type: 'structural', + subtype: 'table-insert', + excerpt: 'Cell', + origin: 'word', + imported: true, + storyLabel: 'Body', + storyKind: 'body', + anchorKey: 'tc::body::word:structural:2', + hasInsert: false, + hasDelete: false, + hasFormat: false, + range: { from: 9, to: 30 }, + }; + mocks.getTrackedChangeIndex.mockReturnValue({ + get: vi.fn(() => [structuralSnapshot]), + getAll: vi.fn(() => []), + invalidate: vi.fn(), + invalidateAll: vi.fn(), + subscribe: vi.fn(), + dispose: vi.fn(), + }); + mocks.resolveTrackedChangeInStory.mockReturnValue({ + editor: hostEditor, + story: { kind: 'story', storyType: 'body' }, + runtimeRef: { storyKey: 'body', rawId: 'word:structural:2' }, + change: { id: 'word:structural:2', rawId: 'word:structural:2', from: 9, to: 30, attrs: {} }, + }); + + const listResult = trackChangesListWrapper(hostEditor, {}); + expect(listResult.items[0]).toMatchObject({ type: 'structural', subtype: 'table-insert' }); + + const getResult = trackChangesGetWrapper(hostEditor, { id: 'word:structural:2' }); + expect(getResult).toMatchObject({ type: 'structural', subtype: 'table-insert' }); + }); + it('checks expectedRevision on the host editor before accepting a non-body tracked change', () => { const hostEditor = makeEditor(); const storyEditor = makeEditor({ acceptTrackedChangeById: vi.fn(() => true) }); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/track-changes-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/track-changes-wrappers.ts index c1bce814cc..1172f5448f 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/track-changes-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/track-changes-wrappers.ts @@ -116,6 +116,7 @@ function buildProjectedInfo( }, id, type, + ...(type === 'structural' && snapshot.subtype ? { subtype: snapshot.subtype } : {}), grouping: options.grouping, pairedWithChangeId: options.pairedWithChangeId ?? undefined, wordRevisionIds: normalizeWordRevisionIds(snapshot.wordRevisionIds), @@ -462,6 +463,7 @@ export function trackChangesListWrapper(editor: Editor, input?: TrackChangesList const { address, type, + subtype, grouping, pairedWithChangeId, wordRevisionIds, @@ -479,6 +481,7 @@ export function trackChangesListWrapper(editor: Editor, input?: TrackChangesList return buildDiscoveryItem(info.id, handle, { address, type, + ...(subtype ? { subtype } : {}), grouping, pairedWithChangeId, wordRevisionIds, @@ -548,6 +551,9 @@ export function trackChangesGetWrapper(editor: Editor, input: TrackChangesGetInp }, id: resolved.change.id, type, + ...(type === 'structural' && resolved.change.structural + ? { subtype: resolved.change.structural.subtype as TrackChangeInfo['subtype'] } + : {}), grouping, wordRevisionIds: normalizeWordRevisionIds(resolved.change.wordRevisionIds), overlap: resolved.change.overlap, diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/__tests__/tracked-change-index.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/__tests__/tracked-change-index.test.ts index 1d599398f3..39a9f93ddf 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/__tests__/tracked-change-index.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/__tests__/tracked-change-index.test.ts @@ -132,6 +132,26 @@ describe('TrackedChangeIndex — per-story cache', () => { }); }); + it('projects type + subtype for whole-table structural changes', () => { + const editor = makeEditor(); + mocks.groupTrackedChanges.mockReturnValueOnce([ + { + ...makeGroupedChange('word:structural:2', 9, 30), + hasInsert: false, + hasDelete: false, + hasFormat: false, + structural: { side: 'insertion', subtype: 'table-insert' }, + }, + ]); + + const index = getTrackedChangeIndex(editor); + const snapshots = index.get({ kind: 'story', storyType: 'body' }); + + expect(snapshots).toHaveLength(1); + expect(snapshots[0]?.type).toBe('structural'); + expect(snapshots[0]?.subtype).toBe('table-insert'); + }); + it('preserves overlap metadata on snapshots', () => { const editor = makeEditor(); mocks.groupTrackedChanges.mockReturnValueOnce([ diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/tracked-change-index.ts b/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/tracked-change-index.ts index 9fff279c03..9ac4485de7 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/tracked-change-index.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/tracked-change-index.ts @@ -270,11 +270,17 @@ class TrackedChangeIndexImpl implements TrackedChangeIndex { (change.excerpt !== undefined ? change.excerpt : undefined) ?? normalizeExcerpt(editor.state.doc.textBetween(change.from, change.to, ' ', '\ufffc')); + const subtype = + type === 'structural' && change.structural + ? (change.structural.subtype as TrackedChangeSnapshot['subtype']) + : undefined; + return { address, runtimeRef, story: locator, type, + subtype, author: toNonEmptyString(change.attrs.author), authorEmail: toNonEmptyString(change.attrs.authorEmail), authorImage: toNonEmptyString(change.attrs.authorImage), diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/tracked-change-snapshot.ts b/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/tracked-change-snapshot.ts index 1d4a08583a..0c18646035 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/tracked-change-snapshot.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/tracked-changes/tracked-change-snapshot.ts @@ -9,6 +9,7 @@ import type { TrackedChangeAddress, TrackChangeProvenanceOrigin, TrackChangeType, + TrackChangeSubtype, TrackChangeOverlapInfo, TrackChangeWordRevisionIds, } from '@superdoc/document-api'; @@ -23,6 +24,8 @@ export interface TrackedChangeSnapshot { story: StoryLocator; /** Tracked-change kind. */ type: TrackChangeType; + /** Finer classification for structural changes (e.g. `table-insert`). */ + subtype?: TrackChangeSubtype; /** Author display name, if captured on the mark. */ author?: string; /** Author email, if captured. */ diff --git a/packages/super-editor/src/editors/v1/extensions/table-row/table-row.js b/packages/super-editor/src/editors/v1/extensions/table-row/table-row.js index 623a5ca113..6b410b8ee8 100644 --- a/packages/super-editor/src/editors/v1/extensions/table-row/table-row.js +++ b/packages/super-editor/src/editors/v1/extensions/table-row/table-row.js @@ -64,6 +64,28 @@ import { parseRowHeight } from './helpers/parseRowHeight.js'; * @property {string} [rsidTr] @internal - Editing session ID for properties modification * @property {string} [paraId] @internal - Unique identifier for the row * @property {string} [textId] @internal - Unique identifier for row text + * @property {TableRowTrackChange} [trackChange] @internal - Structural tracked-change revision on the row (imported from ``/`` inside ``) + */ + +/** + * Structural tracked-change revision stored on a table row. + * + * A whole inserted/deleted table is encoded in OOXML as ``/`` + * inside each row's ``. The row is the structural primitive; the + * enumeration layer groups rows of one table that share a revision into a + * single logical structural change. + * + * @typedef {Object} TableRowTrackChange + * @property {'rowInsert' | 'rowDelete'} type - Whether the row was inserted or deleted. + * @property {string} id - Stable SuperDoc-internal revision id (normalized from Word `w:id`). + * @property {string} [sourceId] - Original Word `w:id`, preserved for round-trip export. + * @property {string} [author] - Revision author (`w:author`). + * @property {string} [authorId] - Acting user id for natively authored revisions (no OOXML counterpart). + * @property {string} [authorEmail] - Revision author email (`w:authorEmail`). + * @property {string} [authorImage] - Acting user avatar for natively authored revisions (no OOXML counterpart). + * @property {string} [date] - Revision timestamp (`w:date`, ISO-8601). + * @property {string} [importedAuthor] - Display author for imported revisions. + * @property {string} [revisionGroupId] - Groups rows belonging to the same logical structural change. */ /** @@ -158,6 +180,17 @@ export const TableRow = Node.create({ * @see {@link https://learn.microsoft.com/en-us/openspecs/office_standards/ms-docx/b7eeddec-7c50-47fb-88b6-1feec3ed832c} */ textId: { rendered: false }, + /** + * Structural tracked-change revision on the row. + * @type {TableRowTrackChange | null} + * @see TableRowTrackChange + */ + trackChange: { + default: null, + // Preserve the revision if a tracked row is ever split. + keepOnSplit: true, + rendered: false, + }, }; }, diff --git a/packages/super-editor/src/editors/v1/extensions/table-row/table-row.test.js b/packages/super-editor/src/editors/v1/extensions/table-row/table-row.test.js index a7cc57fe47..99739b5a61 100644 --- a/packages/super-editor/src/editors/v1/extensions/table-row/table-row.test.js +++ b/packages/super-editor/src/editors/v1/extensions/table-row/table-row.test.js @@ -86,4 +86,15 @@ describe('TableRow attributes', () => { }, }); }); + + describe('trackChange (structural revision slot)', () => { + it('defaults to null and is not rendered to the DOM', () => { + expect(attributes.trackChange.default).toBe(null); + expect(attributes.trackChange.rendered).toBe(false); + }); + + it('is preserved when a tracked row is split', () => { + expect(attributes.trackChange.keepOnSplit).toBe(true); + }); + }); }); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/plugins/trackChangesBasePlugin.js b/packages/super-editor/src/editors/v1/extensions/track-changes/plugins/trackChangesBasePlugin.js index 24a93105c5..c6b247183e 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/plugins/trackChangesBasePlugin.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/plugins/trackChangesBasePlugin.js @@ -1,7 +1,9 @@ +// @ts-check import { Plugin, PluginKey } from 'prosemirror-state'; import { Decoration, DecorationSet } from 'prosemirror-view'; import { TrackInsertMarkName, TrackDeleteMarkName, TrackFormatMarkName } from '../constants.js'; import { getTrackChanges } from '../trackChangesHelpers/getTrackChanges.js'; +import { enumerateStructuralRowChanges } from '../trackChangesHelpers/structuralRowChanges.js'; export const TrackChangesBasePluginKey = new PluginKey('TrackChangesBase'); @@ -73,7 +75,10 @@ export const TrackChangesBasePlugin = () => { let mightAffectTrackChanges = false; tr.steps.forEach((step) => { - if (step.slice || step.from !== step.to) { + // Only Replace(Around)Step carry slice/from/to; the base Step type + // does not expose them, so narrow before reading. + const replaceStep = /** @type {{ slice?: unknown, from?: number, to?: number }} */ (step); + if (replaceStep.slice || replaceStep.from !== replaceStep.to) { mightAffectTrackChanges = true; } }); @@ -126,8 +131,14 @@ const getTrackChangesDecorations = (state, onlyOriginalShown, onlyModifiedShown) const decorations = []; const trackedChanges = getTrackChanges(state); + // Structural row revisions (whole-table insert/delete) live on tableRow node + // attributes, not inline marks, so they are decorated separately as node + // decorations on each tracked . Done here (not in a separate plugin) so the + // show-original / show-modified modes and styling stay in one place. + addStructuralRowDecorations(decorations, state, onlyOriginalShown, onlyModifiedShown); + if (!trackedChanges.length) { - return DecorationSet.empty; + return decorations.length ? DecorationSet.create(state.doc, decorations) : DecorationSet.empty; } trackedChanges.forEach(({ mark, from, to }) => { @@ -211,3 +222,44 @@ const getTrackChangesDecorations = (state, onlyOriginalShown, onlyModifiedShown) return DecorationSet.create(state.doc, decorations); }; + +/** + * Append node decorations for structural row revisions (whole-table + * insert/delete) to `decorations`. Each tracked `` gets a + * `track-row-insert-dec` / `track-row-delete-dec` class plus the same + * `highlighted` / `hidden` / `normal` mode the inline marks use, so insertions + * and deletions read consistently across inline and structural changes. + * + * @param {Array} decorations + * @param {import('prosemirror-state').EditorState} state + * @param {boolean} onlyOriginalShown + * @param {boolean} onlyModifiedShown + */ +const addStructuralRowDecorations = (decorations, state, onlyOriginalShown, onlyModifiedShown) => { + const structuralChanges = enumerateStructuralRowChanges(state); + if (!structuralChanges.length) return; + + for (const change of structuralChanges) { + const isInsert = change.side === 'insertion'; + const baseClass = isInsert ? 'track-row-insert-dec' : 'track-row-delete-dec'; + + // Mode mirrors the inline marks: an insertion is hidden in the original view + // and plain in the modified view; a deletion is plain in the original view + // and hidden in the modified view. + let mode = 'highlighted'; + if (onlyOriginalShown) mode = isInsert ? 'hidden' : 'normal'; + else if (onlyModifiedShown) mode = isInsert ? 'normal' : 'hidden'; + + for (const row of change.rows) { + decorations.push( + Decoration.node(row.from, row.to, { + class: `${baseClass} ${mode}`, + 'data-track-change-id': change.id, + 'data-track-change-kind': change.subtype, + 'data-track-change-author': change.author || '', + 'data-track-change-date': change.date || '', + }), + ); + } + } +}; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js index 92119462d2..62de7b6871 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js @@ -21,7 +21,7 @@ */ import { Slice } from 'prosemirror-model'; -import { AddMarkStep, RemoveMarkStep, ReplaceStep, Mapping, canJoin } from 'prosemirror-transform'; +import { AddMarkStep, RemoveMarkStep, ReplaceStep, canJoin } from 'prosemirror-transform'; import { TrackInsertMarkName, TrackDeleteMarkName, TrackFormatMarkName } from '../constants.js'; import { CommentsPluginKey } from '../../comment/comments-plugin.js'; @@ -80,7 +80,7 @@ import { planCommentEffects } from './comment-effects.js'; /** * @typedef {Object} DecisionFailure * @property {false} ok - * @property {'TARGET_NOT_FOUND'|'INVALID_TARGET'|'REVISION_MISMATCH'|'PERMISSION_DENIED'|'CAPABILITY_UNAVAILABLE'|'PRECONDITION_FAILED'|'COMMENT_CASCADE_PARTIAL'|'NO_OP'} code + * @property {'TARGET_NOT_FOUND'|'INVALID_TARGET'|'INVALID_INPUT'|'REVISION_MISMATCH'|'PERMISSION_DENIED'|'CAPABILITY_UNAVAILABLE'|'PRECONDITION_FAILED'|'COMMENT_CASCADE_PARTIAL'|'NO_OP'} code * @property {string} message * @property {DecisionDiagnostic[]} [diagnostics] * @property {unknown} [details] @@ -136,7 +136,7 @@ export const decideTrackedChanges = ({ state, editor, decision, target, replacem if (!permissionResult.ok) return permissionResult.failure; // Compute the PM mutation plan + comment effects. - const planResult = buildMutationPlan({ state, graph, selections, decision, replacements }); + const planResult = buildMutationPlan({ state, graph, selections, decision }); if (!planResult.ok) return planResult.failure; const { plan } = planResult; @@ -216,11 +216,45 @@ const normalizeDecisionTarget = (target) => { * @property {Array<{ from: number, to: number }>} ranges Concrete PM ranges to resolve. */ +/** + * Resolve a logical change id to its change object, preferring a STRUCTURAL + * whole-table change when the id is shared. + * + * Cell text typed in a tracked-inserted row inherits the row's revision id, so + * the same id can map to BOTH the inline text change and the structural table + * change. The structural change registers its public-id alias only when the key + * is free (review-graph), so a plain `changes.get(id)` can return the inline + * child. Always selecting the structural change lets the decide cascade clear + * the contained cell text in ONE action — and this must hold for id, range, and + * collapsed-cursor targets alike (otherwise a range/cursor decide on a tracked + * table resolves the inline child instead of the table). + * + * @param {import('./review-graph.js').TrackedReviewGraph} graph + * @param {string} id + * @returns {import('./review-graph.js').LogicalTrackedChange | undefined} + */ +const resolveLogicalChangeById = (graph, id) => { + const key = String(id); + for (const candidate of graph.changes.values()) { + if (candidate?.type === CanonicalChangeType.Structural && String(candidate.id) === key) { + return candidate; + } + } + return graph.changes.get(id); +}; + const resolveTargetToSelections = ({ graph, normalized }) => { if (normalized.kind === 'all') { /** @type {ChangeSelection[]} */ const sel = []; + // The `changes` map can hold a logical change under more than one key + // (structural changes register both a table-scoped internal key and a + // public-id alias). Dedupe by object identity so `scope:'all'` never plans + // the same change twice. + const seen = new Set(); for (const change of graph.changes.values()) { + if (seen.has(change)) continue; + seen.add(change); sel.push({ change, coverage: 'full', ranges: change.segments.map((s) => ({ from: s.from, to: s.to })) }); } // Document-order sort to make the apply pass deterministic and to keep @@ -229,7 +263,7 @@ const resolveTargetToSelections = ({ graph, normalized }) => { return { ok: true, selections: sel }; } if (normalized.kind === 'id') { - const change = graph.changes.get(normalized.id); + const change = resolveLogicalChangeById(graph, normalized.id); if (!change) return { ok: false, failure: failure('TARGET_NOT_FOUND', `no tracked change with id "${normalized.id}".`) }; return { @@ -255,7 +289,7 @@ const resolveTargetToSelections = ({ graph, normalized }) => { // per phase0-004 "Range Decisions": collapsed range inside a change // resolves the whole logical change. if (from === to && segment.from <= from && segment.to > from) { - const change = graph.changes.get(segment.changeId); + const change = resolveLogicalChangeById(graph, segment.changeId); if (!change) continue; const existing = byId.get(change.id); if (existing) { @@ -271,7 +305,7 @@ const resolveTargetToSelections = ({ graph, normalized }) => { } continue; } - const change = graph.changes.get(segment.changeId); + const change = resolveLogicalChangeById(graph, segment.changeId); if (!change) continue; const existing = byId.get(change.id); if (existing) { @@ -376,7 +410,7 @@ const runPermissionPreflight = ({ editor, decision, selections }) => { /** * @typedef {Object} MutationOp - * @property {'removeContent'|'removeMark'|'addMark'|'unwrapInsert'|'restoreFormat'|'removeFormat'|'rejectParagraphSplit'} kind + * @property {'removeContent'|'removeMark'|'addMark'|'unwrapInsert'|'restoreFormat'|'removeFormat'|'rejectParagraphSplit'|'clearRowTrackChange'} kind * @property {number} from * @property {number} to * @property {string} [changeId] @@ -396,7 +430,7 @@ const runPermissionPreflight = ({ editor, decision, selections }) => { * @property {DecisionDiagnostic[]} diagnostics */ -const buildMutationPlan = ({ state, graph, selections, decision, replacements }) => { +const buildMutationPlan = ({ state, graph, selections, decision }) => { /** @type {MutationOp[]} */ const ops = []; /** @type {Array<{ from: number, to: number, cause: string }>} */ @@ -410,10 +444,149 @@ const buildMutationPlan = ({ state, graph, selections, decision, replacements }) /** @type {DecisionDiagnostic[]} */ const diagnostics = []; + // Pre-compute the table ranges that a selected whole-table structural change + // will REMOVE (reject-insert / accept-delete). Inline tracked changes wholly + // inside such a table must be retired/suppressed as side effects BEFORE + // mutation planning so they do not generate their own overlapping ops or + // cause position drift (scope:'all' and explicit multi-target decides). This + // mirrors the `affectedChildren` retirement for removed ranges below. + /** @type {Array<{ from: number, to: number, structuralId: string }>} */ + const structuralTableRemovals = []; + for (const selection of selections) { + const change = selection.change; + if (change.type !== CanonicalChangeType.Structural) continue; + const structural = change.structural; + if (!structural || structural.wholeTable !== true || structural.decidable === false) continue; + const removesTable = + (structural.side === 'insertion' && decision === 'reject') || + (structural.side === 'deletion' && decision === 'accept'); + if (!removesTable) continue; + structuralTableRemovals.push({ + from: structural.tableFrom, + to: structural.tableTo, + structuralId: change.id, + }); + } + /** @type {Set} Inline ids suppressed because they live inside a removed table. */ + const suppressedInsideTable = new Set(); + const isInsideRemovedTable = (change) => + structuralTableRemovals.length > 0 && + change.type !== CanonicalChangeType.Structural && + change.segments.length > 0 && + change.segments.every((seg) => + structuralTableRemovals.some((range) => range.from <= seg.from && range.to >= seg.to), + ); + + // Pre-compute the table ranges that a selected whole-table structural change + // KEEPS (accept-insert / reject-delete → the table stays as normal content). + // Word / Google Docs treat an inserted/deleted table as ONE change: approving + // it approves the text inside. We therefore cascade the SAME decision onto + // every inline tracked change whose segments are wholly inside [tableFrom, + // tableTo] — accepting an inserted table accepts the contained insertions + // (their trackInsert marks are removed, text stays); rejecting a deleted table + // rejects the contained changes (e.g. contained deletions are rejected so + // their content stays). The inline change is the CHILD of the structural + // parent here, decided together so the bubble pipeline retires it as a child. + /** @type {Array<{ from: number, to: number, structuralId: string }>} */ + const structuralTableStays = []; + for (const selection of selections) { + const change = selection.change; + if (change.type !== CanonicalChangeType.Structural) continue; + const structural = change.structural; + if (!structural || structural.wholeTable !== true || structural.decidable === false) continue; + const tableStays = + (structural.side === 'insertion' && decision === 'accept') || + (structural.side === 'deletion' && decision === 'reject'); + if (!tableStays) continue; + structuralTableStays.push({ + from: structural.tableFrom, + to: structural.tableTo, + structuralId: change.id, + }); + } + const isInsideStayingTable = (change) => + structuralTableStays.length > 0 && + change.type !== CanonicalChangeType.Structural && + change.segments.length > 0 && + change.segments.every((seg) => structuralTableStays.some((range) => range.from <= seg.from && range.to >= seg.to)); + + /** @type {Set} Inline ids resolved as children of a staying table. */ + const cascadedInsideStayingTable = new Set(); + // Plan a single inline tracked change as a CHILD of a staying-table structural + // decision, reusing the existing inline plan functions with FULL coverage and + // the SAME decision. Marks it touched + retired and records it as an affected + // child. Returns a failure to bubble up, or null on success. + const planContainedInlineChild = (change) => { + if (cascadedInsideStayingTable.has(change.id)) return null; + cascadedInsideStayingTable.add(change.id); + touched.add(change.id); + const fullSelection = { + change, + coverage: 'full', + ranges: change.segments.map((s) => ({ from: s.from, to: s.to })), + }; + if (change.type === CanonicalChangeType.Insertion) { + planInsertionDecision({ ops, change, selection: fullSelection, decision, removedRanges, retired }); + return null; + } + if (change.type === CanonicalChangeType.Deletion) { + planDeletionDecision({ ops, change, selection: fullSelection, decision, removedRanges, retired }); + return null; + } + if (change.type === CanonicalChangeType.Replacement) { + const repResult = planReplacementDecision({ ops, graph, change, decision, removedRanges, retired }); + if (!repResult.ok) return repResult.failure; + return null; + } + if (change.type === CanonicalChangeType.Formatting) { + planFormattingDecision({ ops, change, decision, retired }); + return null; + } + // Nested structural (a tracked table inside a tracked table cell) is out of + // scope for the cascade; leave it for its own decide. + cascadedInsideStayingTable.delete(change.id); + touched.delete(change.id); + return null; + }; + for (const selection of selections) { const { change } = selection; + // Inline tracked change fully contained in a table the decision removes: + // suppress its own ops and retire it as a side effect. The whole-table + // removeContent op already deletes its content; planning it independently + // would double-plan overlapping ops / drift positions. + if (isInsideRemovedTable(change)) { + retired.add(change.id); + touched.add(change.id); + suppressedInsideTable.add(change.id); + continue; + } + // Inline tracked change fully contained in a table the decision KEEPS: + // cascade the SAME decision onto it as a child (accept-insert → its + // trackInsert marks are removed; reject-delete → it is rejected). Routing it + // through the dedicated child planner here (instead of the normal path + // below) keeps `scope:'all'` from double-planning the same change. Recorded + // as an affected child in the side-effect pass below. + if (isInsideStayingTable(change)) { + const failureResult = planContainedInlineChild(change); + if (failureResult) return { ok: false, failure: failureResult }; + continue; + } const isFull = selection.coverage === 'full'; if (!isFull) { + if (change.type === CanonicalChangeType.Structural) { + // Whole-object atomicity (spec §8/§9/§19/TC-OPS-003): a partial-range + // target on a structural revision that is not safely divisible MUST + // fail closed with INVALID_INPUT and leave the document unmutated. + return { + ok: false, + failure: failure( + 'INVALID_INPUT', + 'partial-range decisions are not valid on an indivisible structural revision.', + { details: { changeId: change.id, subtype: change.subtype } }, + ), + }; + } if (change.type === CanonicalChangeType.Replacement) { return { ok: false, @@ -446,7 +619,13 @@ const buildMutationPlan = ({ state, graph, selections, decision, replacements }) } } - if (!isFull && (change.type === CanonicalChangeType.Insertion || change.type === CanonicalChangeType.Deletion)) { + if (change.type === CanonicalChangeType.Structural) { + const structuralResult = planStructuralDecision({ ops, change, decision, removedRanges, retired }); + if (!structuralResult.ok) return { ok: false, failure: structuralResult.failure }; + } else if ( + !isFull && + (change.type === CanonicalChangeType.Insertion || change.type === CanonicalChangeType.Deletion) + ) { const partialResult = planPartialTextDecision({ ops, change, @@ -477,6 +656,34 @@ const buildMutationPlan = ({ state, graph, selections, decision, replacements }) } } + // Cascade onto contained inline children of a STAYING table that were NOT in + // the selection set (e.g. a `{kind:'id'}` decide that targets only the + // structural change). The whole-table change is the parent; every inline + // tracked change wholly inside [tableFrom, tableTo] is decided with the SAME + // decision so the table ends up clean (accept-insert) / restored with content + // (reject-delete) and zero inline marks remain. Scope:'all' already cascaded + // these in the main loop; `cascadedInsideStayingTable` dedups so they are not + // planned twice. Done after the main loop so positions are consistent. + if (structuralTableStays.length > 0) { + // Skip by OBJECT identity, not by `change.id`. Cell text typed in a + // tracked-inserted row inherits the row's revision id, so the inline text + // change and the structural table change SHARE an id. An id-based skip + // (`touched.has(change.id)`) would treat the just-decided table as if it + // also covered the inline child and wrongly exclude it from the cascade — + // leaving the cell text tracked after the table is accepted. The decided + // (selected) changes and already-cascaded children are distinct objects. + const decidedObjects = new Set(selections.map((selection) => selection.change)); + const seenStaying = new Set(); + for (const change of graph.changes.values()) { + if (seenStaying.has(change)) continue; + seenStaying.add(change); + if (decidedObjects.has(change) || cascadedInsideStayingTable.has(change.id)) continue; + if (!isInsideStayingTable(change)) continue; + const failureResult = planContainedInlineChild(change); + if (failureResult) return { ok: false, failure: failureResult }; + } + } + if (!ops.length) { return { ok: false, @@ -494,12 +701,38 @@ const buildMutationPlan = ({ state, graph, selections, decision, replacements }) // that were meaningful only inside it. /** @type {Array<{ changeId: string }>} */ const affectedChildren = []; + // Inline changes suppressed because they were wholly inside a removed table + // are already retired/touched above; surface them as affected side effects so + // the bubble lifecycle resolves their threads. + for (const id of suppressedInsideTable) { + affectedChildren.push({ changeId: id }); + } + // Inline children cascaded as part of a STAYING whole-table decision: surface + // them so the bubble lifecycle resolves their threads alongside the parent. + for (const id of cascadedInsideStayingTable) { + affectedChildren.push({ changeId: id }); + } + const seenChange = new Set(); for (const change of graph.changes.values()) { + // `changes` may hold a logical change under both an internal key and a + // public alias; skip the duplicate visit. + if (seenChange.has(change)) continue; + seenChange.add(change); if (touched.has(change.id)) continue; + const insideRemoved = change.segments.length + ? change.segments.every((seg) => removedRanges.some((r) => r.from <= seg.from && r.to >= seg.to)) + : false; + // An inline change wholly inside a removed table (even with no parent + // relationship) is gone with the table — retire it as a side effect. + if (insideRemoved && isInsideRemovedTable(change)) { + retired.add(change.id); + touched.add(change.id); + affectedChildren.push({ changeId: change.id }); + continue; + } if (!change.parent) continue; if (!retired.has(change.parent) && !touched.has(change.parent)) continue; - const inside = change.segments.every((seg) => removedRanges.some((r) => r.from <= seg.from && r.to >= seg.to)); - if (inside) { + if (insideRemoved) { retired.add(change.id); touched.add(change.id); affectedChildren.push({ changeId: change.id }); @@ -591,6 +824,74 @@ const planDeletionDecision = ({ ops, change, selection, decision, removedRanges, if (isFull) retired.add(change.id); }; +/** + * Plan a whole-object structural decision (whole-table insert/delete). + * + * Semantics (spec §8 / §14): + * - accept insertion → clear the rows' trackChange attrs (table becomes normal content). + * - reject insertion → remove the whole table node. + * - accept deletion → remove the whole table node. + * - reject deletion → clear the rows' trackChange attrs (table restored). + */ +const planStructuralDecision = ({ ops, change, decision, removedRanges, retired }) => { + const structural = change.structural; + if (!structural) { + return { + ok: false, + failure: failure('PRECONDITION_FAILED', `structural change "${change.id}" is missing its structural payload.`), + }; + } + + // Fail closed on any structural shape that is NOT a whole-table insert/delete + // (spec TC-OPS-003). A partial row subset or mixed sides within one + // table is NOT a decidable whole-table change; row-level structural is out of + // scope. We must NEVER route such a shape through the table-removal path. The + // engine returns CAPABILITY_UNAVAILABLE and the document stays unmutated. + if (structural.decidable === false || !structural.wholeTable) { + return { + ok: false, + failure: failure( + 'CAPABILITY_UNAVAILABLE', + 'structural row-level revisions (partial rows or mixed sides) are not decidable; only whole-table insert/delete is supported.', + { details: { changeId: change.id, reason: structural.undecidableReason ?? 'not-whole-table' } }, + ), + }; + } + + const removeWholeTable = + (structural.side === 'insertion' && decision === 'reject') || + (structural.side === 'deletion' && decision === 'accept'); + + if (removeWholeTable) { + ops.push({ + kind: 'removeContent', + from: structural.tableFrom, + to: structural.tableTo, + changeId: change.id, + side: structural.side === 'insertion' ? SegmentSide.Inserted : SegmentSide.Deleted, + }); + removedRanges.push({ + from: structural.tableFrom, + to: structural.tableTo, + cause: `${decision}-structural:${change.id}`, + }); + retired.add(change.id); + return { ok: true }; + } + + // Otherwise the table stays and the revision is cleared from every grouped row. + for (const row of structural.rows) { + ops.push({ + kind: 'clearRowTrackChange', + from: row.from, + to: row.to, + changeId: change.id, + }); + } + retired.add(change.id); + return { ok: true }; +}; + const planReplacementDecision = ({ ops, graph, change, decision, removedRanges, retired }) => { const inserted = change.insertedSegments; const deleted = change.deletedSegments; @@ -1002,6 +1303,18 @@ const applyPlan = ({ state, plan }) => { tr.step(new RemoveMarkStep(op.from, op.to, op.mark)); continue; } + if (op.kind === 'clearRowTrackChange') { + // Whole-table accept-insert / reject-delete: the table survives as + // normal content. setNodeMarkup is position-stable (same node size), so + // it is safe in the mark pass. Map through the accumulated transaction + // mapping in case an earlier op shifted positions. + const mappedFrom = tr.mapping.map(op.from, 1); + const rowNode = tr.doc.nodeAt(mappedFrom); + if (rowNode && rowNode.type.name === 'tableRow') { + tr.setNodeMarkup(mappedFrom, undefined, { ...rowNode.attrs, trackChange: null }); + } + continue; + } } for (const op of contentOps) { tr.step(new ReplaceStep(op.from, op.to, Slice.empty)); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/mark-metadata.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/mark-metadata.js index 2a8c3cb05c..ca5e87b4dc 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/mark-metadata.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/mark-metadata.js @@ -34,6 +34,11 @@ export const CanonicalChangeType = Object.freeze({ Deletion: 'deletion', Replacement: 'replacement', Formatting: 'formatting', + // Structural revisions (whole-object insert/delete) live on node attrs, not + // marks. This is used for whole-table insert/delete. The public + // document-api projection maps `structural` straight through to the `kind` + // shape demanded by the §14 structural conformance contract. + Structural: 'structural', }); /** @@ -46,6 +51,8 @@ export const ChangeSubtype = Object.freeze({ TextDeletion: 'text-deletion', TextReplacement: 'text-replacement', RunFormatting: 'run-formatting', + TableInsert: 'table-insert', + TableDelete: 'table-delete', }); /** diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/review-graph.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/review-graph.js index 4ce6600194..5a4d409573 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/review-graph.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/review-graph.js @@ -27,6 +27,7 @@ import { deterministicJson, } from './mark-metadata.js'; import { BODY_STORY, buildStoryKey } from './story-locator.js'; +import { enumerateStructuralRowChanges } from '../trackChangesHelpers/structuralRowChanges.js'; // --------------------------------------------------------------------------- // Types @@ -54,6 +55,7 @@ import { BODY_STORY, buildStoryKey } from './story-locator.js'; * @property {string} parentId * @property {string} parentSide * @property {'parent'|'child'|'standalone'} overlapRole + * @property {boolean} [structural] True for a whole-table structural segment (no inline mark). * @property {Array} [nodePath] optional diagnostics nodePath. */ @@ -131,7 +133,14 @@ import { BODY_STORY, buildStoryKey } from './story-locator.js'; */ export const buildReviewGraph = ({ state, story = BODY_STORY, replacementsMode = 'paired' }) => { const spans = enumerateTrackedMarkSpans(state); - return buildGraphFromSpans({ spans, doc: state?.doc ?? null, story, replacementsMode }); + const structuralChanges = enumerateStructuralRowChanges(state); + return buildGraphFromSpans({ + spans, + structuralChanges, + doc: state?.doc ?? null, + story, + replacementsMode, + }); }; // --------------------------------------------------------------------------- @@ -190,7 +199,7 @@ export const invalidateReviewGraphCache = (editor) => { // Internal builder // --------------------------------------------------------------------------- -const buildGraphFromSpans = ({ spans, doc, story, replacementsMode }) => { +const buildGraphFromSpans = ({ spans, structuralChanges = [], doc, story, replacementsMode }) => { /** @type {Array<{ attrs: import('./mark-metadata.js').NormalizedTrackedAttrs, span: import('./segment-index.js').TrackedMarkSpan }>} */ const normalized = spans.map((span) => ({ attrs: readTrackedAttrs(span.mark, span.mark.type.name), @@ -269,8 +278,38 @@ const buildGraphFromSpans = ({ spans, doc, story, replacementsMode }) => { for (const seg of segs) bySegmentId.set(seg.segmentId, seg); } - // 7. Flat ordered segment list. - const segments = mergedSegments; + // 6b. Structural row changes (whole-table insert/delete). These do not come + // from marks — they live on `tableRow.attrs.trackChange` — so they are + // projected into LogicalTrackedChange entries with synthetic segments + // covering the table range. The decision engine reads `change.structural` + // to apply node-level accept/reject. + // + // Identity is TABLE-SCOPED. Real Word docs assign a distinct `w:id` per + // row, and two separate tracked tables in one document can even share a + // Word id. Keying the `changes` map purely on the Word revision id would + // collapse/drop one of them. We therefore store each structural change + // under an internal `structural::` key (unique per table) + // and ALSO register the stable PUBLIC id as an alias so list/get/decide + // can route by the public id. The public alias is first-wins so a + // pathological id collision never overwrites a previously-seen table. + for (const structural of structuralChanges) { + const logical = buildStructuralLogicalChange({ structural, doc, story }); + if (!logical) continue; + const internalKey = `structural:${structural.tablePos}:${structural.side}`; + if (changes.has(internalKey)) continue; + changes.set(internalKey, logical); + // Public-id alias (Word w:id or table-derived id). Only register when it is + // a distinct key and not already owned by another change. + if (logical.id && logical.id !== internalKey && !changes.has(logical.id)) { + changes.set(logical.id, logical); + } + for (const seg of logical.segments) bySegmentId.set(seg.segmentId, seg); + mergedSegments.push(...logical.segments); + appendToMap(byRevisionGroupId, logical.revisionGroupId, logical.id); + } + + // 7. Flat ordered segment list (kept in document order for range queries). + const segments = mergedSegments.slice().sort((a, b) => a.from - b.from || a.to - b.to); /** @type {TrackedReviewGraph} */ const graph = { @@ -512,6 +551,114 @@ const buildLogicalChange = ({ changeId, segments, doc, story, replacementsMode } return logical; }; +/** + * Project a structural row change (whole-table insert/delete) into a + * LogicalTrackedChange. The segments are synthetic and cover the whole table + * so range/all targeting and coverage checks treat the table atomically. + * + * @param {{ + * structural: import('../trackChangesHelpers/structuralRowChanges.js').StructuralChange, + * doc: import('prosemirror-model').Node | null, + * story: import('./story-locator.js').StoryLocator, + * }} input + * @returns {LogicalTrackedChange | null} + */ +const buildStructuralLogicalChange = ({ structural, doc, story }) => { + const side = structural.side === 'insertion' ? SegmentSide.Inserted : SegmentSide.Deleted; + const from = structural.tableFrom; + const to = structural.tableTo; + if (!(from < to)) return null; + + /** @type {import('./mark-metadata.js').NormalizedTrackedAttrs} */ + const attrs = { + id: structural.id, + revisionGroupId: structural.revisionGroupId, + splitFromId: '', + changeType: CanonicalChangeType.Structural, + replacementGroupId: '', + replacementSideId: '', + overlapParentId: '', + sourceIds: structural.sourceId ? { wordIdStructural: structural.sourceId } : {}, + sourceId: structural.sourceId, + importedAuthor: structural.importedAuthor, + origin: structural.sourceId ? 'word' : '', + author: structural.author, + authorId: '', + authorEmail: structural.authorEmail, + authorImage: structural.authorImage, + date: structural.date, + markType: '', + side, + subtype: structural.subtype, + explicitChangeType: CanonicalChangeType.Structural, + hasReviewMetadata: true, + }; + + /** @type {TrackedSegment} */ + const segment = { + segmentId: `${structural.id}:structural:${from}:${to}:0`, + changeId: structural.id, + markType: '', + side, + from, + to, + text: '', + mark: /** @type {*} */ (null), + markRuns: [], + attrs, + parentId: '', + parentSide: '', + overlapRole: 'standalone', + structural: true, + }; + if (doc) { + try { + segment.text = doc.textBetween(from, to, ' ', ''); + } catch { + segment.text = ''; + } + } + + const segments = [segment]; + /** @type {LogicalTrackedChange} */ + const logical = { + id: structural.id, + type: CanonicalChangeType.Structural, + subtype: structural.subtype, + state: 'open', + segments, + coverageSegments: [...segments], + insertedSegments: side === SegmentSide.Inserted ? [...segments] : [], + deletedSegments: side === SegmentSide.Deleted ? [...segments] : [], + formattingSegments: [], + replacement: null, + author: structural.author, + authorId: '', + authorEmail: structural.authorEmail, + authorImage: structural.authorImage, + date: structural.date, + sourceIds: attrs.sourceIds, + revisionGroupId: structural.revisionGroupId, + splitFromId: '', + sourcePlatform: structural.sourceId ? 'word' : '', + story, + parent: null, + children: [], + before: [], + after: [], + excerpt: segment.text.length > 200 ? `${segment.text.slice(0, 200)}…` : segment.text, + }; + // Structural-specific payload the decision engine reads to apply node-level + // accept/reject. Non-enumerable so it never leaks into deterministic JSON or + // contract projections that iterate own keys. + Object.defineProperty(logical, 'structural', { + value: structural, + enumerable: false, + }); + + return logical; +}; + const aggregateSourceIds = (segments) => { /** @type {Record} */ const out = {}; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/structural-decisions.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/structural-decisions.test.js new file mode 100644 index 0000000000..63b84abb8c --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/structural-decisions.test.js @@ -0,0 +1,659 @@ +// @ts-check +/** + * Structural (whole-table) tracked-change tests. + * + * Covers: + * - enumeration + grouping of structural row revisions into one logical change + * - review-graph projection (type === 'structural') + * - decide semantics (accept/reject insert/delete) + * - whole-object atomicity: partial-range target fails closed with INVALID_INPUT + * - scope: 'all' includes structural changes + */ + +import { describe, it, expect } from 'vitest'; + +import { decideTrackedChanges } from './decision-engine.js'; +import { buildReviewGraph, CanonicalChangeType } from './review-graph.js'; +import { + createReviewGraphTestSchema, + stateWithTrackedTable, + stateWithPerRowTrackedTable, + stateWithTwoTrackedTables, +} from './test-fixtures.js'; +import { enumerateStructuralRowChanges } from '../trackChangesHelpers/structuralRowChanges.js'; +import { EditorState } from 'prosemirror-state'; +import { TrackInsertMarkName, TrackDeleteMarkName } from '../constants.js'; + +const ALICE = { name: 'Alice Reviewer', email: 'alice@example.com' }; + +const insertTrackChange = (id = '1') => ({ + type: 'rowInsert', + id, + sourceId: id, + author: ALICE.name, + authorEmail: ALICE.email, + date: '2026-05-20T16:00:00Z', + importedAuthor: `${ALICE.name} (imported)`, +}); + +const deleteTrackChange = (id = '1') => ({ ...insertTrackChange(id), type: 'rowDelete' }); + +const editorFor = (extra) => ({ + options: { user: ALICE, trackedChanges: {}, ...extra }, + storage: { trackChanges: { lastDecisionFailure: null } }, +}); + +describe('structural row-change enumeration', () => { + it('groups every row of a whole inserted table into one structural change', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithTrackedTable({ schema, trackChange: insertTrackChange('1'), rowCount: 3 }); + + const changes = enumerateStructuralRowChanges(state); + expect(changes).toHaveLength(1); + expect(changes[0]).toMatchObject({ + id: '1', + side: 'insertion', + subtype: 'table-insert', + wholeTable: true, + }); + expect(changes[0].rows).toHaveLength(3); + expect(changes[0].tableTo).toBeGreaterThan(changes[0].tableFrom); + }); + + it('groups a whole deleted table into one structural deletion', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithTrackedTable({ schema, trackChange: deleteTrackChange('9') }); + const changes = enumerateStructuralRowChanges(state); + expect(changes).toHaveLength(1); + expect(changes[0]).toMatchObject({ id: '9', side: 'deletion', subtype: 'table-delete' }); + }); + + it('returns [] for a missing state', () => { + expect(enumerateStructuralRowChanges(null)).toEqual([]); + expect(enumerateStructuralRowChanges({})).toEqual([]); + }); +}); + +describe('structural review-graph projection', () => { + it('exposes the structural change with type "structural" and table-covering segment', () => { + const schema = createReviewGraphTestSchema(); + const { state, tablePos } = stateWithTrackedTable({ schema, trackChange: insertTrackChange('1') }); + + const graph = buildReviewGraph({ state }); + const change = graph.changes.get('1'); + expect(change).toBeDefined(); + expect(change.type).toBe(CanonicalChangeType.Structural); + expect(change.subtype).toBe('table-insert'); + expect(change.segments[0].from).toBe(tablePos); + + // No invariant errors introduced by the structural change. + expect(graph.validate().filter((d) => d.severity === 'error')).toEqual([]); + + // Range queries see the structural change. + const inRange = graph.changesInRange(change.segments[0].from, change.segments[0].to); + expect(inRange.map((c) => c.id)).toContain('1'); + }); +}); + +describe('structural decide semantics', () => { + const tableSurvives = (state) => state.doc.child(1).type.name === 'table'; + const rowTrackChangeOf = (state) => state.doc.child(1).child(0).attrs.trackChange; + + it('accept insertion clears the row trackChange attrs (table stays as normal content)', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithTrackedTable({ schema, trackChange: insertTrackChange('1') }); + + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'accept', + target: { kind: 'id', id: '1' }, + }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + expect(tableSurvives(next)).toBe(true); + expect(rowTrackChangeOf(next)).toBeNull(); + }); + + it('accepting the table also clears cell text that SHARES the structural revision id (one action)', () => { + // Text typed in a tracked-inserted row inherits the row's revision id, so the + // inline trackInsert mark and the structural change SHARE an id. Regression for + // the id-collision: the cascade must still reach the inline child (object + // identity, not change.id) so accepting the table leaves zero tracked marks. + const schema = createReviewGraphTestSchema(); + const id = '1'; + const tc = insertTrackChange(id); + const insMark = schema.marks[TrackInsertMarkName].create({ + id, + author: ALICE.name, + authorEmail: ALICE.email, + date: tc.date, + }); + const cellParagraph = schema.nodes.paragraph.create({}, [schema.text('q', [insMark])]); + const cell = schema.nodes.tableCell.create({}, [cellParagraph]); + const row = schema.nodes.tableRow.create({ trackChange: tc }, [cell]); + const table = schema.nodes.table.create({}, [row]); + const before = schema.nodes.paragraph.create({}, [schema.text('Before.')]); + const after = schema.nodes.paragraph.create({}, [schema.text('After.')]); + const doc = schema.nodes.doc.create({}, [before, table, after]); + const state = EditorState.create({ schema, doc }); + + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'accept', + target: { kind: 'id', id }, + }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + + // Table stays, row revision cleared. + expect(next.doc.child(1).type.name).toBe('table'); + expect(next.doc.child(1).child(0).attrs.trackChange).toBeNull(); + // The shared-id cell text keeps its content but loses the trackInsert mark. + const cellText = next.doc.child(1).child(0).child(0).child(0).child(0); + expect(cellText.text).toBe('q'); + expect((cellText.marks || []).some((m) => m.type.name === TrackInsertMarkName)).toBe(false); + }); + + it('a RANGE/cursor decide over a tracked table resolves the STRUCTURAL change (not the inline child)', () => { + // Range and collapsed-cursor targets must prefer the structural change for a + // shared id, same as the by-id path — otherwise a cursor inside the table + // resolves the inline cell-text change and the table stays tracked. + const schema = createReviewGraphTestSchema(); + const id = '1'; + const tc = insertTrackChange(id); + const insMark = schema.marks[TrackInsertMarkName].create({ id, author: ALICE.name, date: tc.date }); + const cellParagraph = schema.nodes.paragraph.create({}, [schema.text('q', [insMark])]); + const cell = schema.nodes.tableCell.create({}, [cellParagraph]); + const row = schema.nodes.tableRow.create({ trackChange: tc }, [cell]); + const table = schema.nodes.table.create({}, [row]); + const before = schema.nodes.paragraph.create({}, [schema.text('Before.')]); + const doc = schema.nodes.doc.create({}, [before, table, schema.nodes.paragraph.create({}, [schema.text('A')])]); + const state = EditorState.create({ schema, doc }); + + // Locate the table node range, then accept via a RANGE target covering it. + let tableFrom = 0; + let tableTo = 0; + state.doc.descendants((node, pos) => { + if (node.type.name === 'table') { + tableFrom = pos; + tableTo = pos + node.nodeSize; + return false; + } + return undefined; + }); + + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'accept', + target: { kind: 'range', from: tableFrom, to: tableTo }, + }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + + // The structural change was resolved: table stays, row revision cleared, and + // the shared-id cell text lost its trackInsert mark (cascade reached it). + expect(next.doc.child(1).type.name).toBe('table'); + expect(next.doc.child(1).child(0).attrs.trackChange).toBeNull(); + const cellText = next.doc.child(1).child(0).child(0).child(0).child(0); + expect((cellText.marks || []).some((m) => m.type.name === TrackInsertMarkName)).toBe(false); + }); + + it('reject insertion removes the whole table node', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithTrackedTable({ schema, trackChange: insertTrackChange('1') }); + + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'reject', + target: { kind: 'id', id: '1' }, + }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + expect(next.doc.childCount).toBe(2); // before + after, table gone. + expect(next.doc.child(1).textContent).toBe('After.'); + }); + + it('accept deletion removes the whole table node', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithTrackedTable({ schema, trackChange: deleteTrackChange('1') }); + + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'accept', + target: { kind: 'id', id: '1' }, + }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + expect(next.doc.childCount).toBe(2); + }); + + it('reject deletion clears the row trackChange attrs (table restored)', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithTrackedTable({ schema, trackChange: deleteTrackChange('1') }); + + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'reject', + target: { kind: 'id', id: '1' }, + }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + expect(tableSurvives(next)).toBe(true); + expect(rowTrackChangeOf(next)).toBeNull(); + }); + + it('scope: all reject clears the structural change', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithTrackedTable({ schema, trackChange: insertTrackChange('1') }); + + const result = decideTrackedChanges({ state, editor: editorFor(), decision: 'reject', target: { kind: 'all' } }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + expect(enumerateStructuralRowChanges(next)).toEqual([]); + }); +}); + +describe('whole-table detection with distinct per-row ids (real Word shape)', () => { + const insertRow = (id) => ({ + type: 'rowInsert', + id, + sourceId: id, + author: ALICE.name, + authorEmail: ALICE.email, + date: '2026-05-20T16:00:00Z', + importedAuthor: `${ALICE.name} (imported)`, + }); + const deleteRow = (id) => ({ ...insertRow(id), type: 'rowDelete' }); + + it('mirrors new_table.docx: 3 rows, 3 distinct ids, all inserts → ONE whole table-insert', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithPerRowTrackedTable({ + schema, + rowTrackChanges: [insertRow('2'), insertRow('7'), insertRow('11')], + }); + const changes = enumerateStructuralRowChanges(state); + expect(changes).toHaveLength(1); + expect(changes[0]).toMatchObject({ subtype: 'table-insert', wholeTable: true, decidable: true }); + expect(changes[0].rows).toHaveLength(3); + + // accept/reject affects only that table. + const accept = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'accept', + target: { kind: 'id', id: changes[0].id }, + }); + expect(accept.ok).toBe(true); + const next = state.apply(accept.tr); + expect(next.doc.child(1).type.name).toBe('table'); + // every row trackChange cleared + next.doc.child(1).forEach((row) => expect(row.attrs.trackChange).toBeNull()); + expect(enumerateStructuralRowChanges(next)).toEqual([]); + }); + + it('partial tracked rows (subset) → NOT whole-table, decide fails closed, table NOT removed', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithPerRowTrackedTable({ + schema, + rowTrackChanges: [insertRow('2'), null, insertRow('11')], + }); + const changes = enumerateStructuralRowChanges(state); + expect(changes).toHaveLength(1); + expect(changes[0]).toMatchObject({ wholeTable: false, decidable: false, undecidableReason: 'partial-rows' }); + + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'reject', + target: { kind: 'id', id: changes[0].id }, + }); + expect(result.ok).toBe(false); + expect(result.code).toBe('CAPABILITY_UNAVAILABLE'); + }); + + it('mixed sides within one table → NOT whole-table, decide fails closed', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithPerRowTrackedTable({ + schema, + rowTrackChanges: [insertRow('2'), deleteRow('7')], + }); + const changes = enumerateStructuralRowChanges(state); + expect(changes).toHaveLength(1); + expect(changes[0]).toMatchObject({ wholeTable: false, decidable: false, undecidableReason: 'mixed-sides' }); + + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'accept', + target: { kind: 'id', id: changes[0].id }, + }); + expect(result.ok).toBe(false); + expect(result.code).toBe('CAPABILITY_UNAVAILABLE'); + // table must survive + const before = state.doc.childCount; + expect(before).toBe(state.doc.childCount); + }); + + it('scope:all with a partial table leaves the table intact (no whole-table removal)', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithPerRowTrackedTable({ + schema, + rowTrackChanges: [insertRow('2'), null], + }); + const result = decideTrackedChanges({ state, editor: editorFor(), decision: 'reject', target: { kind: 'all' } }); + // The only change is undecidable structural → no ops → fails closed. + expect(result.ok).toBe(false); + // table still present in original state (engine never mutated). + expect(state.doc.child(1).type.name).toBe('table'); + }); + + it('two separate tracked tables → two independent structural changes', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithTwoTrackedTables({ + schema, + first: insertRow('5'), + second: deleteRow('8'), + }); + const changes = enumerateStructuralRowChanges(state); + expect(changes).toHaveLength(2); + expect(changes.map((c) => c.subtype).sort()).toEqual(['table-delete', 'table-insert']); + // graph projects each as its own change. + const graph = buildReviewGraph({ state }); + const structuralLogical = new Set(); + for (const c of graph.changes.values()) { + if (c.type === CanonicalChangeType.Structural) structuralLogical.add(c); + } + expect(structuralLogical.size).toBe(2); + }); + + it('two tracked tables sharing the same Word id still project as two changes (table-scoped identity)', () => { + const schema = createReviewGraphTestSchema(); + const { state } = stateWithTwoTrackedTables({ + schema, + first: insertRow('3'), + second: insertRow('3'), + }); + const graph = buildReviewGraph({ state }); + const structuralLogical = new Set(); + for (const c of graph.changes.values()) { + if (c.type === CanonicalChangeType.Structural) structuralLogical.add(c); + } + expect(structuralLogical.size).toBe(2); + }); + + it('nested tracked table inside a cell is discovered as its own change', () => { + const schema = createReviewGraphTestSchema(); + // Outer table: one row, NOT tracked, whose cell contains an inner tracked table. + const innerCellParagraph = schema.nodes.paragraph.create({}, [schema.text('Inner')]); + const innerCell = schema.nodes.tableCell.create({}, [innerCellParagraph]); + const innerRow = schema.nodes.tableRow.create({ trackChange: insertRow('42') }, [innerCell]); + const innerTable = schema.nodes.table.create({}, [innerRow]); + const outerCell = schema.nodes.tableCell.create({}, [innerTable]); + const outerRow = schema.nodes.tableRow.create({ trackChange: null }, [outerCell]); + const outerTable = schema.nodes.table.create({}, [outerRow]); + const doc = schema.nodes.doc.create({}, [schema.nodes.paragraph.create({}, [schema.text('Before.')]), outerTable]); + const state = EditorState.create({ schema, doc }); + + const changes = enumerateStructuralRowChanges(state); + // Only the inner table is fully tracked → ONE whole-table change. + expect(changes).toHaveLength(1); + expect(changes[0]).toMatchObject({ subtype: 'table-insert', wholeTable: true }); + }); +}); + +describe('co-decide: inline tracked change inside a removed table', () => { + const insertRow = (id) => ({ + type: 'rowInsert', + id, + sourceId: id, + author: ALICE.name, + authorEmail: ALICE.email, + date: '2026-05-20T16:00:00Z', + importedAuthor: `${ALICE.name} (imported)`, + }); + + /** + * Build a doc: Before. / table (one row, rowInsert) whose cell text carries an + * inline trackInsert mark / After. + */ + const buildDocWithInlineInsideTable = (schema) => { + const insMark = schema.marks[TrackInsertMarkName].create({ + id: 'inline-1', + author: ALICE.name, + authorEmail: ALICE.email, + date: '2026-05-20T16:00:00Z', + }); + const cellParagraph = schema.nodes.paragraph.create({}, [schema.text('NewCell', [insMark])]); + const cell = schema.nodes.tableCell.create({}, [cellParagraph]); + const row = schema.nodes.tableRow.create({ trackChange: insertRow('2') }, [cell]); + const table = schema.nodes.table.create({}, [row]); + const before = schema.nodes.paragraph.create({}, [schema.text('Before.')]); + const after = schema.nodes.paragraph.create({}, [schema.text('After.')]); + const doc = schema.nodes.doc.create({}, [before, table, after]); + return EditorState.create({ schema, doc }); + }; + + it('scope:all reject removes the table once and retires the inner inline change (no drift)', () => { + const schema = createReviewGraphTestSchema(); + const state = buildDocWithInlineInsideTable(schema); + + // Sanity: graph sees both the structural and the inline change. + const graph = buildReviewGraph({ state }); + const types = [...graph.changes.values()].map((c) => c.type); + expect(types).toContain(CanonicalChangeType.Structural); + expect(types).toContain(CanonicalChangeType.Insertion); + + const result = decideTrackedChanges({ state, editor: editorFor(), decision: 'reject', target: { kind: 'all' } }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + // Table gone: before + after only. + expect(next.doc.childCount).toBe(2); + expect(next.doc.child(0).textContent).toBe('Before.'); + expect(next.doc.child(1).textContent).toBe('After.'); + // No tracked changes remain. + expect(enumerateStructuralRowChanges(next)).toEqual([]); + const nextGraph = buildReviewGraph({ state: next }); + expect(nextGraph.changes.size).toBe(0); + // The inline change was retired as a side effect. + const retiredIds = result.receipt.removedChangeIds + .map((entry) => entry.id) + .concat(result.receipt.affectedChildren.map((c) => c.changeId)); + expect(retiredIds).toContain('inline-1'); + }); +}); + +describe('co-decide: inline tracked change inside a STAYING table (Word/GDocs subsume)', () => { + const insertRow = (id) => ({ + type: 'rowInsert', + id, + sourceId: id, + author: ALICE.name, + authorEmail: ALICE.email, + date: '2026-05-20T16:00:00Z', + importedAuthor: `${ALICE.name} (imported)`, + }); + const deleteRow = (id) => ({ ...insertRow(id), type: 'rowDelete' }); + + /** Table (one row) whose cell text carries an inline trackInsert mark. */ + const buildInsertedTableWithInlineInsertion = (schema) => { + const insMark = schema.marks[TrackInsertMarkName].create({ + id: 'inline-ins-1', + author: ALICE.name, + authorEmail: ALICE.email, + date: '2026-05-20T16:00:00Z', + }); + const cellParagraph = schema.nodes.paragraph.create({}, [schema.text('NewCell', [insMark])]); + const cell = schema.nodes.tableCell.create({}, [cellParagraph]); + const row = schema.nodes.tableRow.create({ trackChange: insertRow('2') }, [cell]); + const table = schema.nodes.table.create({}, [row]); + const before = schema.nodes.paragraph.create({}, [schema.text('Before.')]); + const after = schema.nodes.paragraph.create({}, [schema.text('After.')]); + const doc = schema.nodes.doc.create({}, [before, table, after]); + return EditorState.create({ schema, doc }); + }; + + /** Deleted table (one row) whose cell text carries an inline trackDelete mark. */ + const buildDeletedTableWithInlineDeletion = (schema) => { + const delMark = schema.marks[TrackDeleteMarkName].create({ + id: 'inline-del-1', + author: ALICE.name, + authorEmail: ALICE.email, + date: '2026-05-20T16:00:00Z', + }); + const cellParagraph = schema.nodes.paragraph.create({}, [schema.text('OldCell', [delMark])]); + const cell = schema.nodes.tableCell.create({}, [cellParagraph]); + const row = schema.nodes.tableRow.create({ trackChange: deleteRow('4') }, [cell]); + const table = schema.nodes.table.create({}, [row]); + const before = schema.nodes.paragraph.create({}, [schema.text('Before.')]); + const after = schema.nodes.paragraph.create({}, [schema.text('After.')]); + const doc = schema.nodes.doc.create({}, [before, table, after]); + return EditorState.create({ schema, doc }); + }; + + const cellTextOf = (state) => state.doc.child(1).child(0).child(0).textContent; + const tableSurvives = (state) => state.doc.child(1).type.name === 'table'; + + it('accept an inserted table: rows cleared AND contained inline insertions accepted (zero marks, text stays)', () => { + const schema = createReviewGraphTestSchema(); + const state = buildInsertedTableWithInlineInsertion(schema); + + // Sanity: graph sees both the structural and the inline insertion. + const graph = buildReviewGraph({ state }); + const types = [...graph.changes.values()].map((c) => c.type); + expect(types).toContain(CanonicalChangeType.Structural); + expect(types).toContain(CanonicalChangeType.Insertion); + + const structural = enumerateStructuralRowChanges(state)[0]; + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'accept', + target: { kind: 'id', id: structural.id }, + }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + + // Table stays, text present. + expect(tableSurvives(next)).toBe(true); + expect(cellTextOf(next)).toBe('NewCell'); + // ZERO tracked changes remain: rows cleared AND inline marks gone. + expect(enumerateStructuralRowChanges(next)).toEqual([]); + const nextGraph = buildReviewGraph({ state: next }); + expect(nextGraph.changes.size).toBe(0); + + // The inline insertion was resolved as an affected child of the parent. + const retiredIds = result.receipt.removedChangeIds + .map((entry) => entry.id) + .concat(result.receipt.affectedChildren.map((c) => c.changeId)); + expect(retiredIds).toContain('inline-ins-1'); + expect(result.receipt.affectedChildren.map((c) => c.changeId)).toContain('inline-ins-1'); + }); + + it('reject an inserted table: whole table (and its inline insertion) removed', () => { + const schema = createReviewGraphTestSchema(); + const state = buildInsertedTableWithInlineInsertion(schema); + const structural = enumerateStructuralRowChanges(state)[0]; + + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'reject', + target: { kind: 'id', id: structural.id }, + }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + // Table + its text gone. + expect(next.doc.childCount).toBe(2); + expect(next.doc.child(0).textContent).toBe('Before.'); + expect(next.doc.child(1).textContent).toBe('After.'); + expect(buildReviewGraph({ state: next }).changes.size).toBe(0); + }); + + it('reject a deleted table: rows restored AND contained inline deletions rejected (content stays, no marks)', () => { + const schema = createReviewGraphTestSchema(); + const state = buildDeletedTableWithInlineDeletion(schema); + const structural = enumerateStructuralRowChanges(state)[0]; + + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'reject', + target: { kind: 'id', id: structural.id }, + }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + + expect(tableSurvives(next)).toBe(true); + // Content stays (deletion rejected) and no marks remain. + expect(cellTextOf(next)).toBe('OldCell'); + expect(enumerateStructuralRowChanges(next)).toEqual([]); + expect(buildReviewGraph({ state: next }).changes.size).toBe(0); + const retiredIds = result.receipt.removedChangeIds + .map((entry) => entry.id) + .concat(result.receipt.affectedChildren.map((c) => c.changeId)); + expect(retiredIds).toContain('inline-del-1'); + }); + + it('scope:all accept of an inserted table + contained inline insertion → ONE coherent result, no double-plan', () => { + const schema = createReviewGraphTestSchema(); + const state = buildInsertedTableWithInlineInsertion(schema); + + const result = decideTrackedChanges({ state, editor: editorFor(), decision: 'accept', target: { kind: 'all' } }); + expect(result.ok).toBe(true); + const next = state.apply(result.tr); + + expect(tableSurvives(next)).toBe(true); + expect(cellTextOf(next)).toBe('NewCell'); + expect(enumerateStructuralRowChanges(next)).toEqual([]); + expect(buildReviewGraph({ state: next }).changes.size).toBe(0); + // The inline insertion id appears exactly once across removed/children + // (dedup avoided double-planning under scope:'all'). + const occurrences = result.receipt.removedChangeIds + .map((e) => e.id) + .concat(result.receipt.affectedChildren.map((c) => c.changeId)) + .filter((id) => id === 'inline-ins-1').length; + expect(occurrences).toBeGreaterThanOrEqual(1); + }); + + it('partial-range on the structural change still fails closed (cascade only on a FULL decision)', () => { + const schema = createReviewGraphTestSchema(); + const state = buildInsertedTableWithInlineInsertion(schema); + const structural = enumerateStructuralRowChanges(state)[0]; + + // A range strictly inside the table, not covering the whole table. + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'accept', + target: { kind: 'range', from: structural.tableFrom + 2, to: structural.tableFrom + 4 }, + }); + expect(result.ok).toBe(false); + expect(result.code).toBe('INVALID_INPUT'); + // Document unmutated. + expect(state.doc.child(1).type.name).toBe('table'); + }); +}); + +describe('structural whole-object atomicity (fail closed)', () => { + it('a partial-range target on a structural change fails closed with INVALID_INPUT and does not mutate', () => { + const schema = createReviewGraphTestSchema(); + const { state, tablePos } = stateWithTrackedTable({ schema, trackChange: deleteTrackChange('1') }); + + // A range strictly inside the table, not covering the whole table → partial. + const result = decideTrackedChanges({ + state, + editor: editorFor(), + decision: 'accept', + target: { kind: 'range', from: tablePos + 2, to: tablePos + 4 }, + }); + + expect(result.ok).toBe(false); + expect(result.code).toBe('INVALID_INPUT'); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/test-fixtures.js b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/test-fixtures.js index 0eee3a748e..b3a55746c6 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/test-fixtures.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/review-model/test-fixtures.js @@ -24,6 +24,26 @@ const NODES = { parseDOM: [{ tag: 'p' }], toDOM: () => ['p', 0], }, + // Minimal table family for structural (whole-table) tracked-change tests. + table: { + content: 'tableRow+', + group: 'block', + isolating: true, + parseDOM: [{ tag: 'table' }], + toDOM: () => ['table', ['tbody', 0]], + }, + tableRow: { + content: 'tableCell+', + attrs: { trackChange: { default: null } }, + parseDOM: [{ tag: 'tr' }], + toDOM: () => ['tr', 0], + }, + tableCell: { + content: 'block+', + isolating: true, + parseDOM: [{ tag: 'td' }], + toDOM: () => ['td', 0], + }, text: { group: 'inline' }, }; @@ -102,6 +122,96 @@ export const stateFromTrackedSpans = ({ schema, spans }) => { return { state, schema, paragraphStart: 1 }; }; +/** + * Build an EditorState with a leading paragraph, a single-row table whose row + * carries a structural `trackChange` attr, and a trailing paragraph. Mirrors + * the imported shape of the Word whole-table insert/delete fixtures. + * + * @param {{ + * schema: Schema, + * trackChange: { type: 'rowInsert'|'rowDelete', id: string, [k: string]: unknown }, + * rowCount?: number, + * cellText?: string, + * }} input + * @returns {{ state: EditorState, schema: Schema, tablePos: number }} + */ +export const stateWithTrackedTable = ({ schema, trackChange, rowCount = 1, cellText = 'Cell' }) => { + const makeRow = () => { + const cellParagraph = schema.nodes.paragraph.create({}, [schema.text(cellText)]); + const cell = schema.nodes.tableCell.create({}, [cellParagraph]); + return schema.nodes.tableRow.create({ trackChange }, [cell]); + }; + const rows = Array.from({ length: rowCount }, makeRow); + const table = schema.nodes.table.create({}, rows); + const before = schema.nodes.paragraph.create({}, [schema.text('Before.')]); + const after = schema.nodes.paragraph.create({}, [schema.text('After.')]); + const doc = schema.nodes.doc.create({}, [before, table, after]); + const state = EditorState.create({ schema, doc }); + // before paragraph = "Before." → nodeSize 9 (pos 0..8 inclusive open/close). + const tablePos = before.nodeSize; + return { state, schema, tablePos }; +}; + +/** + * Build a doc with a table whose rows carry per-row `trackChange` attrs (or + * `null`). Mirrors a real Word whole-table insert where each row gets a + * DISTINCT `w:id`, and supports partial / mixed-side / untracked-row shapes. + * + * @param {{ + * schema: Schema, + * rowTrackChanges: Array | null>, + * cellText?: string, + * trailing?: boolean, + * }} input + * @returns {{ state: EditorState, schema: Schema, tablePos: number }} + */ +export const stateWithPerRowTrackedTable = ({ schema, rowTrackChanges, cellText = 'Cell', trailing = true }) => { + const rows = rowTrackChanges.map((trackChange) => { + const cellParagraph = schema.nodes.paragraph.create({}, [schema.text(cellText)]); + const cell = schema.nodes.tableCell.create({}, [cellParagraph]); + return schema.nodes.tableRow.create({ trackChange: trackChange ?? null }, [cell]); + }); + const table = schema.nodes.table.create({}, rows); + const before = schema.nodes.paragraph.create({}, [schema.text('Before.')]); + const children = trailing + ? [before, table, schema.nodes.paragraph.create({}, [schema.text('After.')])] + : [before, table]; + const doc = schema.nodes.doc.create({}, children); + const state = EditorState.create({ schema, doc }); + const tablePos = before.nodeSize; + return { state, schema, tablePos }; +}; + +/** + * Build a doc with two independent tracked tables, optionally sharing the same + * Word revision id, so identity/table-scoping can be exercised. + * + * @param {{ + * schema: Schema, + * first: Record, + * second: Record, + * }} input + * @returns {{ state: EditorState, schema: Schema, firstTablePos: number, secondTablePos: number }} + */ +export const stateWithTwoTrackedTables = ({ schema, first, second }) => { + const makeTable = (trackChange) => { + const cellParagraph = schema.nodes.paragraph.create({}, [schema.text('Cell')]); + const cell = schema.nodes.tableCell.create({}, [cellParagraph]); + const row = schema.nodes.tableRow.create({ trackChange }, [cell]); + return schema.nodes.table.create({}, [row]); + }; + const before = schema.nodes.paragraph.create({}, [schema.text('Before.')]); + const mid = schema.nodes.paragraph.create({}, [schema.text('Middle.')]); + const after = schema.nodes.paragraph.create({}, [schema.text('After.')]); + const t1 = makeTable(first); + const t2 = makeTable(second); + const doc = schema.nodes.doc.create({}, [before, t1, mid, t2, after]); + const state = EditorState.create({ schema, doc }); + const firstTablePos = before.nodeSize; + const secondTablePos = before.nodeSize + t1.nodeSize + mid.nodeSize; + return { state, schema, firstTablePos, secondTablePos }; +}; + /** * Build a tracked-mark attrs blob with sensible defaults so test * declarations stay short. diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/collectWholeTablesInRange.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/collectWholeTablesInRange.js new file mode 100644 index 0000000000..b355b327fc --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/collectWholeTablesInRange.js @@ -0,0 +1,35 @@ +// @ts-check +/** + * Collect every WHOLE `table` node fully bracketed by `[from, to)` in `doc` + * (the table node starts at/after `from` and ends at/before `to`). A table that + * merely OVERLAPS the range — a partial row/column change, or a row added to an + * existing table — is skipped. A nested table inside a captured table is not + * descended into: it belongs to the same whole-table operation, not a separate + * one. + * + * Single owner of the "is this range a clean whole-table operation" walk, shared + * by the structural insert/delete authoring paths (`replaceStep`) and the row + * stamper (`stampTableRows`). + * + * @param {{ doc: import('prosemirror-model').Node, from: number, to: number }} options + * @returns {Array<{ pos: number, node: import('prosemirror-model').Node, from: number, to: number }>} + */ +export const collectWholeTablesInRange = ({ doc, from, to }) => { + if (!doc || typeof from !== 'number' || typeof to !== 'number' || to <= from) return []; + + const boundedFrom = Math.max(0, from); + const boundedTo = Math.min(doc.content.size, to); + if (boundedTo <= boundedFrom) return []; + + /** @type {Array<{ pos: number, node: import('prosemirror-model').Node, from: number, to: number }>} */ + const tables = []; + doc.nodesBetween(boundedFrom, boundedTo, (node, pos) => { + if (node.type?.name !== 'table') return true; + if (pos >= boundedFrom && pos + node.nodeSize <= boundedTo) { + tables.push({ pos, node, from: pos, to: pos + node.nodeSize }); + return false; // don't descend into a captured table + } + return true; + }); + return tables; +}; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/index.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/index.js index 7212773bc0..31e6e2d5e4 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/index.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/index.js @@ -8,6 +8,7 @@ export * from './addMarkStep.js'; export * from './removeMarkStep.js'; export * from './getLiveInlineMarksInRange.js'; export * from './getTrackChanges.js'; +export * from './structuralRowChanges.js'; export * from './parseFormatList.js'; export * from './findTrackedMarkBetween.js'; export * from './markSnapshotHelpers.js'; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/replaceStep.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/replaceStep.js index 1e0e241fb5..85b21a96c5 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/replaceStep.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/replaceStep.js @@ -4,6 +4,41 @@ import { TrackChangesBasePluginKey } from '../plugins/index.js'; import { CommentsPluginKey } from '../../comment/comments-plugin.js'; import { compileTrackedEdit } from '../review-model/overlap-compiler.js'; import { makeTextInsertIntent, makeTextDeleteIntent, makeTextReplaceIntent } from '../review-model/edit-intent.js'; +import { stampTableRows } from './stampTableRows.js'; +import { collectWholeTablesInRange } from './collectWholeTablesInRange.js'; +import { markInsertion } from './markInsertion.js'; +import { markDeletion } from './markDeletion.js'; + +/** + * Whether a slice's top-level content contains a `table` node (directly, or + * wrapped — the toolbar's `insertTable` emits `[paragraph, table, paragraph]` + * when the new table would otherwise sit adjacent to a document boundary). + * + * @param {import('prosemirror-model').Slice | null | undefined} slice + * @returns {boolean} + */ +const sliceContainsTable = (slice) => { + const content = slice?.content; + if (!content) return false; + let found = false; + content.forEach((node) => { + if (found) return; + if (node?.type?.name === 'table') found = true; + }); + return found; +}; + +/** + * Whether the range `[from, to)` in `doc` fully contains at least one WHOLE + * `table` node (the table node starts at or after `from` and ends at or before + * `to`). This is the deletion analog of `sliceContainsTable`: a whole-table + * delete is a `tr.delete(tableStart, tableEnd)` (the shape `deleteTable` emits), + * so the deleted range exactly brackets the table node. + * + * @param {{ doc: import('prosemirror-model').Node, from: number, to: number }} options + * @returns {boolean} + */ +const rangeContainsWholeTable = ({ doc, from, to }) => collectWholeTablesInRange({ doc, from, to }).length > 0; /** * Given a range (from..to) and a count of characters ("the Nth character in that range"), @@ -221,6 +256,216 @@ export const replaceStep = ({ // fail closed instead of keeping a second tracked-write implementation here. }; +/** + * Apply a tracked STRUCTURAL insert for a ReplaceStep whose inserted slice + * contains a whole `table` node. + * + * The inline overlap-compiler is text-centric: it produces an insertion mark + * over inline text. An empty (freshly authored) table has no inline text, so + * the compiler fails closed and the table would otherwise land untracked. This + * path instead applies the original step verbatim (preserving any separator + * paragraphs the `insertTable` command wrapped around the table) and then makes + * the insertion tracked: + * + * 1. Apply the original ReplaceStep to `newTr`. For a replace + * (`from !== to`) this also removes the replaced range. The common toolbar + * shape replaces an empty paragraph (`from=0, to=2` in an empty doc), so + * there is no live content to preserve; the inserted slice simply takes + * its place. We do NOT tracked-delete the replaced empty paragraph — it + * carries no inline content and Word treats a table inserted over the + * caret's empty paragraph as a pure structural insert. + * 2. Mark every inserted INLINE text run (e.g. text the slice's wrapping + * separator paragraphs may carry, or pre-filled cell text) with a tracked + * insertion mark via `markInsertion`. An empty new table contributes none, + * which is fine — `markInsertion` skips table internals by design. + * 3. Stamp each inserted table's rows with a structural `rowInsert` revision + * (one shared `revisionGroupId` per table) via `stampTableRows`, + * matching the shape the importer lands from `` in ``. + * 4. Keep the outer `map` consistent (append the step's map) so subsequent + * original steps in the same transaction remap correctly, and report + * `insertedTo` so `trackedTransaction` places the caret after the table. + * + * `setNodeMarkup` (step 3) and `addMark` (step 2) do not change node sizes, so + * the mapping established in step 1 stays valid. + * + * @param {{ newTr: import('prosemirror-state').Transaction, step: import('prosemirror-transform').ReplaceStep, map: import('prosemirror-transform').Mapping, user: object, date: string }} options + * @returns {{ handled: boolean }} + */ +const tryStructuralTableInsert = ({ newTr, step, map, user, date }) => { + const beforeSteps = newTr.steps.length; + const beforeSize = newTr.doc.content.size; + const insertAt = step.from; + const replacedLength = step.to - step.from; + + // Only the no-real-content insert/replace is safe to fast-path here — e.g. + // inserting a table at the caret in an empty paragraph (the common toolbar + // shape: ReplaceStep from=0 to=2 replacing the empty leading paragraph). If + // the replaced range holds real text, applying the step directly would delete + // that content WITHOUT a tracked deletion (data loss). Bail so it is handled + // by the normal tracked path instead of silently dropping live content. + if (step.from !== step.to && newTr.doc.textBetween(step.from, step.to).length > 0) { + return { handled: false }; + } + + // 1. Apply the original step (insert slice, replacing [from, to)). + if (newTr.maybeStep(step).failed) { + return { handled: false }; + } + + // Keep the outer mapping consistent with the other replaceStep branches so + // later original steps in this transaction land where the user expects. + const stepMap = newTr.steps[beforeSteps].getMap(); + map.appendMap(stepMap); + + // Inserted range in newTr.doc space. The step deletes `[from, to)` and + // inserts the slice at `from`, so the new content starts exactly at `from`. + // Its span is the net document growth plus the replaced length (this is + // exact regardless of the slice's open depth). Mapping `from` through the + // step map collapses both biases onto the deletion point and cannot bracket + // the freshly inserted nodes, so derive the range from the doc delta instead. + const insertedFrom = insertAt; + const insertedTo = insertAt + (newTr.doc.content.size - beforeSize) + replacedLength; + + // 2. Mark inserted inline text (separator-paragraph text, pre-filled cell + // text). markInsertion skips table rows/cells internals by design, so an + // empty table contributes nothing here. + if (insertedTo > insertedFrom) { + let hasInlineText = false; + newTr.doc.nodesBetween(insertedFrom, insertedTo, (node) => { + if (node.isText && node.text) { + hasInlineText = true; + return false; + } + }); + if (hasInlineText) { + markInsertion({ tr: newTr, from: insertedFrom, to: insertedTo, user, date }); + } + } + + // 3. Stamp each whole inserted table's rows with a structural rowInsert + // revision (shared revisionGroupId per table). + stampTableRows({ type: 'rowInsert', tr: newTr, from: insertedFrom, to: insertedTo, user, date }); + + // 4. Surface insertion meta so the caret lands after the table and the + // bubble/comments pipeline sees a tracked insert. + newTr.setMeta(TrackChangesBasePluginKey, { insertedTo }); + newTr.setMeta(CommentsPluginKey, { type: 'force' }); + + return { handled: true }; +}; + +/** + * Apply a tracked STRUCTURAL delete for a ReplaceStep whose deleted range fully + * brackets one or more whole `table` nodes (the shape `deleteTable` / + * `deleteTableWhenSelected` / select-table-then-Delete emit: + * `tr.delete(tableStart, tableEnd)` with an EMPTY slice). + * + * A tracked deletion keeps the content VISIBLE (struck-through / red) rather + * than removing it, so this path is the inverse of `tryStructuralTableInsert`: + * it does NOT apply the removal step. Instead it leaves the table(s) in place + * and makes them tracked-deleted: + * + * 1. Do NOT apply the original ReplaceStep — the table must stay so the + * reviewer can accept (remove) or reject (restore) it later. + * 2. Mark every INLINE text run in the deleted range with a `trackDelete` + * mark via `markDeletion` (so cell text and any non-table text in the + * range renders struck-through). `markDeletion` skips table rows/cells + * structure nodes by design and only marks leaf inline text, so the table + * nodes themselves are preserved. An EMPTY table contributes no cell text, + * which is fine — step 3 alone makes it a tracked deletion. + * 3. Stamp each WHOLE table in the range with a structural `rowDelete` + * revision on every row (one shared `revisionGroupId` per table) via + * `stampTableRows`, matching the shape the importer lands from + * `` in ``. + * 4. Set the tracked-changes / comments meta and report `handled: true` so + * `replaceStep` does NOT fall through to applying the untracked removal. + * + * Content safety for partial selections: because we never apply the removal and + * `markDeletion` marks ALL inline text in `[from, to)` (table cell text AND any + * text outside a table that the selection happened to include), no live content + * is dropped untracked even when the range is a mix of whole table(s) and + * surrounding text. Only WHOLE tables fully contained in the range receive the + * `rowDelete` stamp; a partially-overlapping table is left to the structural + * enumerator's fail-closed handling (it never becomes a decidable whole-table + * change and is therefore never removed). Partial row/column/cell deletes are + * out of scope (this branch is only taken when at least one WHOLE table is + * fully bracketed). + * + * `markDeletion` (addMark) and `stampTableRows` (setNodeMarkup) do not + * change node sizes and we apply no removal step, so the outer `map` stays the + * identity for this step and subsequent original steps remap correctly. + * + * @param {{ newTr: import('prosemirror-state').Transaction, step: import('prosemirror-transform').ReplaceStep, user: object, date: string }} options + * @returns {{ handled: boolean }} + */ +const tryStructuralTableDelete = ({ newTr, step, map, originalStep, originalStepIndex, tr, user, date }) => { + const from = step.from; + const to = step.to; + + // Collect the whole tables fully bracketed by the range. + const tableRanges = collectWholeTablesInRange({ doc: newTr.doc, from, to }); + + // Only handle a CLEAN whole-table delete: the range must not include inline + // text OUTSIDE the table(s). A mixed selection (surrounding text + table) + // would share one deletion id across inside/outside text via `markDeletion`, + // breaking structural reject cleanup and the bubble subsumption. Decline so + // such ranges fall through to the normal inline-deletion path instead. + let hasOutsideText = false; + newTr.doc.nodesBetween(from, to, (node, pos) => { + if (hasOutsideText) return false; + if (node.isText && node.text && !tableRanges.some((r) => pos >= r.from && pos < r.to)) { + hasOutsideText = true; + return false; + } + return undefined; + }); + if (hasOutsideText) return { handled: false }; + + // Tracked-delete the cell text inside the table(s) (all inside-table now). + // markDeletion marks only leaf inline text and keeps table structure nodes. + let hasInlineText = false; + newTr.doc.nodesBetween(from, to, (node) => { + if (node.isText && node.text) { + hasInlineText = true; + return false; + } + return undefined; + }); + if (hasInlineText) { + markDeletion({ tr: newTr, from, to, user, date }); + } + + // Stamp each whole table's rows with a structural rowDelete revision (shared + // revisionGroupId per table). Required for an empty table (no cell text) and + // for the structural "Deleted table" change/bubble in all cases. + const stamped = stampTableRows({ type: 'rowDelete', tr: newTr, from, to, user, date }); + + // Nothing trackable — decline so the caller can fall through. + if (!stamped && !hasInlineText) { + return { handled: false }; + } + + // We applied NO removal (the table stays). Cancel the original step's + // positional effect on the outer `map` so any LATER original step in the same + // transaction lands in the kept-table document instead of drifting backward by + // the un-removed table's size. Mirrors the inline-deletion map dance; a no-op + // for a single-step deleteTable (no later steps to remap). + if (map && originalStep && tr) { + try { + const invertStep = originalStep.invert(tr.docs[originalStepIndex]).map(map); + if (invertStep) map.appendMap(invertStep.getMap()); + } catch { + // Best effort: leave the map unchanged. + } + } + + // Surface meta so the bubble/comments pipeline sees the tracked deletion. + newTr.setMeta(TrackChangesBasePluginKey, {}); + newTr.setMeta(CommentsPluginKey, { type: 'force' }); + + return { handled: true }; +}; + /** * Try to route a text-shaped ReplaceStep through the overlap-aware compiler. * @@ -247,6 +492,47 @@ const tryCompileStep = ({ date, replacements, }) => { + // Structural insert: the inserted slice introduces a whole `table` node + // (possibly wrapped by the separator paragraphs `insertTable` emits at a + // document boundary). The inline-text-centric compiler cannot represent an + // empty table — it has no inline text to mark — so it fails closed and the + // table would land untracked. Route such inserts through the dedicated + // structural path instead. + if (step.slice.content.size > 0 && sliceContainsTable(step.slice)) { + const structural = tryStructuralTableInsert({ newTr, step, map, user, date }); + if (structural.handled) return structural; + // If the structural insert could not apply (e.g. PM rejected the step), + // fall through to the normal compiler path rather than dropping the edit. + } + + // Structural delete: an empty-slice deletion whose range fully brackets one + // or more WHOLE `table` nodes (the shape `deleteTable` / + // `deleteTableWhenSelected` / select-table-then-Delete emit). A tracked + // deletion must keep the table VISIBLE (struck-through), so route it through + // the dedicated structural path that stamps `rowDelete` + marks cell text + // WITHOUT removing the table. This must run before the empty-deletion + // fall-through below, which would otherwise let the structural fallback + // remove an empty table untracked (data loss of the tracked intent). + if ( + step.from !== step.to && + step.slice.content.size === 0 && + rangeContainsWholeTable({ doc: newTr.doc, from: step.from, to: step.to }) + ) { + const structural = tryStructuralTableDelete({ + newTr, + step, + map, + originalStep, + originalStepIndex, + tr, + user, + date, + }); + if (structural.handled) return structural; + // If the structural delete declined (e.g. nothing trackable), fall through + // to the normal paths rather than dropping the edit. + } + // Empty structural deletion handled by the structural branch above. if (step.from !== step.to && step.slice.content.size === 0) { let hasInlineContent = false; @@ -386,5 +672,22 @@ const tryCompileStep = ({ newTr.setMeta(TrackChangesBasePluginKey, meta); newTr.setMeta(CommentsPluginKey, { type: 'force' }); + // Structural authoring: the compiler/markInsertion only mark inline content + // and deliberately skip table internals. If this insertion introduced a + // whole table, stamp a `rowInsert` revision on each of its rows so the table + // is tracked as ONE whole-table insert (matching imported tracked tables). + // The inserted range is [step.from, insertedTo); setNodeMarkup keeps sizes + // stable so it does not disturb the mapping established above. + if (typeof result.insertedTo === 'number' && result.insertedTo > step.from) { + stampTableRows({ + type: 'rowInsert', + tr: newTr, + from: step.from, + to: result.insertedTo, + user, + date, + }); + } + return { handled: true, sizeDelta: newTr.doc.content.size - beforeSize }; }; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/stampDeletedTableRows.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/stampDeletedTableRows.test.js new file mode 100644 index 0000000000..95b656780d --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/stampDeletedTableRows.test.js @@ -0,0 +1,257 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { CellSelection } from 'prosemirror-tables'; +import { enumerateStructuralRowChanges } from './structuralRowChanges.js'; +import { TrackDeleteMarkName } from '../constants.js'; + +const ALICE = { name: 'Alice', email: 'alice@example.com' }; + +const setup = ({ user = ALICE, track = true, content = '

Hi there

' } = {}) => { + const { editor } = initTestEditor({ + mode: 'text', + content, + user, + trackedChanges: {}, + }); + if (track) editor.commands.enableTrackChanges(); + return editor; +}; + +/** Collect all tableRow nodes (with positions) in the editor doc. */ +const collectRows = (editor) => { + const rows = []; + editor.state.doc.descendants((node, pos) => { + if (node.type.name === 'tableRow') rows.push({ node, pos }); + }); + return rows; +}; + +const countTables = (editor) => { + let n = 0; + editor.state.doc.descendants((node) => { + if (node.type.name === 'table') n += 1; + }); + return n; +}; + +/** Insert a table at the caret WITHOUT track changes (clean fixture), then turn TC on. */ +const editorWithTable = ({ rows = 2, cols = 2, withText = true } = {}) => { + const editor = setup({ track: false, content: '

before

' }); + // Place caret at end so the table is appended. + editor.commands.setTextSelection(editor.state.doc.content.size); + editor.commands.insertTable({ rows, cols }); + if (withText) { + // Type some text into the first cell so we exercise cell-text trackDelete. + // Find first paragraph inside the first cell and set caret there. + let firstCellTextPos = null; + editor.state.doc.descendants((node, pos) => { + if (firstCellTextPos !== null) return false; + if (node.type.name === 'tableCell') { + firstCellTextPos = pos + 2; // inside the cell's first paragraph + return false; + } + }); + if (firstCellTextPos !== null) { + editor.commands.setTextSelection(firstCellTextPos); + editor.commands.insertContent('CELL'); + } + } + editor.commands.enableTrackChanges(); + return editor; +}; + +/** Put the caret inside the first cell of the (only) table. */ +const caretInFirstCell = (editor) => { + let pos = null; + editor.state.doc.descendants((node, p) => { + if (pos !== null) return false; + if (node.type.name === 'tableCell') { + pos = p + 2; + return false; + } + }); + if (pos !== null) editor.commands.setTextSelection(pos); +}; + +describe('authoring: tracked whole-table deletion stamps rowDelete revisions', () => { + let editor; + afterEach(() => { + editor?.destroy(); + editor = null; + }); + + it('deleteTable() with TC on keeps the table visible and stamps rowDelete on every row', () => { + editor = editorWithTable({ rows: 2, cols: 2, withText: true }); + caretInFirstCell(editor); + + expect(countTables(editor)).toBe(1); + const ok = editor.commands.deleteTable(); + expect(ok).toBe(true); + + // The table STILL exists (tracked deletions keep content visible). + expect(countTables(editor)).toBe(1); + + const rows = collectRows(editor); + expect(rows.length).toBe(2); + rows.forEach(({ node }) => { + expect(node.attrs.trackChange).toBeTruthy(); + expect(node.attrs.trackChange.type).toBe('rowDelete'); + expect(node.attrs.trackChange.author).toBe(ALICE.name); + expect(node.attrs.trackChange.authorEmail).toBe(ALICE.email); + expect(typeof node.attrs.trackChange.id).toBe('string'); + expect(node.attrs.trackChange.id.length).toBeGreaterThan(0); + }); + + // One shared revisionGroupId per table; distinct per-row ids. + expect(new Set(rows.map((r) => r.node.attrs.trackChange.revisionGroupId)).size).toBe(1); + expect(new Set(rows.map((r) => r.node.attrs.trackChange.id)).size).toBe(2); + + // Cell text retains its content but gains a trackDelete mark. + expect(editor.state.doc.textContent).toContain('CELL'); + let foundDeletedCellText = false; + editor.state.doc.descendants((node) => { + if (node.isText && node.text?.includes('CELL')) { + if (node.marks.some((m) => m.type.name === TrackDeleteMarkName)) foundDeletedCellText = true; + } + }); + expect(foundDeletedCellText).toBe(true); + + // Exactly ONE decidable whole-table delete. + const changes = enumerateStructuralRowChanges(editor.state); + expect(changes.length).toBe(1); + expect(changes[0].subtype).toBe('table-delete'); + expect(changes[0].side).toBe('deletion'); + expect(changes[0].wholeTable).toBe(true); + expect(changes[0].decidable).toBe(true); + expect(changes[0].rows.length).toBe(2); + }); + + it('a MIXED selection (surrounding text + table) deletes without losing content untracked', () => { + // Deleting a range that spans surrounding text AND a whole table must never + // drop content untracked. (deleteSelection splits this into per-node steps, + // so the table portion is a clean whole-table delete and the text a separate + // inline delete; the mixed-range guard in tryStructuralTableDelete is the + // safety net for any single combined step.) The invariant we assert is the + // one that matters: all content survives as tracked, not removed. + editor = editorWithTable({ rows: 2, cols: 2, withText: true }); + + editor.commands.setTextSelection({ from: 1, to: editor.state.doc.content.size }); + editor.commands.deleteSelection(); + + // Nothing was dropped untracked — the table and both texts remain visible. + expect(countTables(editor)).toBe(1); + expect(editor.state.doc.textContent).toContain('before'); + expect(editor.state.doc.textContent).toContain('CELL'); + }); + + it('select-all-cells then Delete (CellSelection) tracks the whole-table deletion', () => { + editor = editorWithTable({ rows: 2, cols: 2, withText: true }); + + // Build a CellSelection spanning all cells, mirroring deleteTableWhenSelected's + // precondition, then run deleteTable (the command it delegates to). + const cellPositions = []; + editor.state.doc.descendants((node, pos) => { + if (node.type.name === 'tableCell') cellPositions.push(pos); + }); + expect(cellPositions.length).toBe(4); + const sel = new CellSelection( + editor.state.doc.resolve(cellPositions[0]), + editor.state.doc.resolve(cellPositions[cellPositions.length - 1]), + ); + editor.view.dispatch(editor.state.tr.setSelection(sel)); + + const ok = editor.commands.deleteTable(); + expect(ok).toBe(true); + + expect(countTables(editor)).toBe(1); + const rows = collectRows(editor); + expect(rows.length).toBe(2); + rows.forEach(({ node }) => { + expect(node.attrs.trackChange?.type).toBe('rowDelete'); + }); + + const changes = enumerateStructuralRowChanges(editor.state); + expect(changes.length).toBe(1); + expect(changes[0].subtype).toBe('table-delete'); + expect(changes[0].decidable).toBe(true); + expect(changes[0].wholeTable).toBe(true); + }); + + it('EMPTY table delete (no cell text) is tracked, not removed untracked', () => { + editor = editorWithTable({ rows: 2, cols: 2, withText: false }); + caretInFirstCell(editor); + + const ok = editor.commands.deleteTable(); + expect(ok).toBe(true); + + // Table kept and every row carries rowDelete. + expect(countTables(editor)).toBe(1); + const rows = collectRows(editor); + expect(rows.length).toBe(2); + rows.forEach(({ node }) => { + expect(node.attrs.trackChange?.type).toBe('rowDelete'); + }); + + const changes = enumerateStructuralRowChanges(editor.state); + expect(changes.length).toBe(1); + expect(changes[0].subtype).toBe('table-delete'); + expect(changes[0].decidable).toBe(true); + expect(changes[0].wholeTable).toBe(true); + }); + + it('accept removes the deleted table from the document', () => { + editor = editorWithTable({ rows: 2, cols: 2, withText: true }); + caretInFirstCell(editor); + editor.commands.deleteTable(); + expect(countTables(editor)).toBe(1); + + editor.commands.acceptAllTrackedChanges(); + + expect(countTables(editor)).toBe(0); + expect(collectRows(editor).length).toBe(0); + expect(editor.state.doc.textContent).not.toContain('CELL'); + expect(enumerateStructuralRowChanges(editor.state)).toEqual([]); + }); + + it('reject restores the deleted table with zero tracked marks/attrs', () => { + editor = editorWithTable({ rows: 2, cols: 2, withText: true }); + caretInFirstCell(editor); + editor.commands.deleteTable(); + expect(countTables(editor)).toBe(1); + + editor.commands.rejectAllTrackedChanges(); + + // Table kept, content stays, no tracked attrs or marks remain. + expect(countTables(editor)).toBe(1); + expect(editor.state.doc.textContent).toContain('CELL'); + + const rows = collectRows(editor); + expect(rows.length).toBe(2); + rows.forEach(({ node }) => { + expect(node.attrs.trackChange).toBeFalsy(); + }); + + let anyDeleteMark = false; + editor.state.doc.descendants((node) => { + if (node.isText && node.marks.some((m) => m.type.name === TrackDeleteMarkName)) anyDeleteMark = true; + }); + expect(anyDeleteMark).toBe(false); + + expect(enumerateStructuralRowChanges(editor.state)).toEqual([]); + }); + + it('TC OFF: deleteTable removes the table normally (no trackChange, no structural change)', () => { + editor = editorWithTable({ rows: 2, cols: 2, withText: true }); + // Turn track changes back OFF. + editor.commands.disableTrackChanges(); + caretInFirstCell(editor); + + const ok = editor.commands.deleteTable(); + expect(ok).toBe(true); + + expect(countTables(editor)).toBe(0); + expect(collectRows(editor).length).toBe(0); + expect(editor.state.doc.textContent).not.toContain('CELL'); + expect(enumerateStructuralRowChanges(editor.state)).toEqual([]); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/stampInsertedTableRows.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/stampInsertedTableRows.test.js new file mode 100644 index 0000000000..70584e9150 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/stampInsertedTableRows.test.js @@ -0,0 +1,248 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { enumerateStructuralRowChanges } from './structuralRowChanges.js'; +import { translator as trTranslator } from '@core/super-converter/v3/handlers/w/tr/tr-translator.js'; + +const ALICE = { name: 'Alice', email: 'alice@example.com' }; + +const setup = ({ user = ALICE, track = true, content = '

Hi there

' } = {}) => { + const { editor } = initTestEditor({ + mode: 'text', + content, + user, + trackedChanges: {}, + }); + if (track) editor.commands.enableTrackChanges(); + return editor; +}; + +/** Collect all tableRow nodes (with positions) in the editor doc. */ +const collectRows = (editor) => { + const rows = []; + editor.state.doc.descendants((node, pos) => { + if (node.type.name === 'tableRow') rows.push({ node, pos }); + }); + return rows; +}; + +describe('authoring: tracked whole-table insertion stamps row revisions', () => { + let editor; + afterEach(() => { + editor?.destroy(); + editor = null; + }); + + it('sets trackChange.type === "rowInsert" on every row of an inserted table when TC is on', () => { + editor = setup({ track: true }); + const ok = editor.commands.insertTable({ rows: 3, cols: 2 }); + expect(ok).toBe(true); + + const rows = collectRows(editor); + expect(rows.length).toBe(3); + + rows.forEach(({ node }) => { + expect(node.attrs.trackChange).toBeTruthy(); + expect(node.attrs.trackChange.type).toBe('rowInsert'); + expect(node.attrs.trackChange.author).toBe(ALICE.name); + expect(node.attrs.trackChange.authorEmail).toBe(ALICE.email); + expect(typeof node.attrs.trackChange.id).toBe('string'); + expect(node.attrs.trackChange.id.length).toBeGreaterThan(0); + expect(typeof node.attrs.trackChange.date).toBe('string'); + }); + + // All rows of one table share a single revisionGroupId so the enumerator + // groups them as ONE whole-table insert. + const groupIds = new Set(rows.map((r) => r.node.attrs.trackChange.revisionGroupId)); + expect(groupIds.size).toBe(1); + + // Word assigns a distinct id per row; ids should differ. + const ids = new Set(rows.map((r) => r.node.attrs.trackChange.id)); + expect(ids.size).toBe(3); + }); + + it('enumerateStructuralRowChanges reports exactly ONE decidable whole-table insert', () => { + editor = setup({ track: true }); + editor.commands.insertTable({ rows: 3, cols: 2 }); + + const changes = enumerateStructuralRowChanges(editor.state); + expect(changes.length).toBe(1); + + const [change] = changes; + expect(change.subtype).toBe('table-insert'); + expect(change.side).toBe('insertion'); + expect(change.wholeTable).toBe(true); + expect(change.decidable).toBe(true); + expect(change.rows.length).toBe(3); + expect(change.author).toBe(ALICE.name); + }); + + it('does NOT stamp trackChange when TC mode is OFF (plain table insertion)', () => { + editor = setup({ track: false }); + const ok = editor.commands.insertTable({ rows: 2, cols: 2 }); + expect(ok).toBe(true); + + const rows = collectRows(editor); + expect(rows.length).toBe(2); + rows.forEach(({ node }) => { + expect(node.attrs.trackChange).toBeFalsy(); + }); + + expect(enumerateStructuralRowChanges(editor.state)).toEqual([]); + }); + + it('tracks the table when dispatched through the REAL editor view path (not direct command return)', () => { + // This mirrors the toolbar -> editor.commands.insertTable -> CommandService + // -> view.dispatch -> Editor.#dispatchTransaction -> trackedTransaction + // chokepoint that the layout-engine playground actually uses. We build the + // command transaction and push it through the real dispatcher so the whole + // tracked-transaction pipeline runs end to end. + editor = setup({ track: true }); + + // Move the caret to the end of the document so the table insert adds a + // trailing separator paragraph (slice becomes [table, paragraph]); this is + // the common real-app shape and exercises the separator wrapping branch. + const endPos = editor.state.doc.content.size; + editor.commands.setTextSelection(endPos); + + const dispatched = editor.commands.insertTable({ rows: 3, cols: 2 }); + expect(dispatched).toBe(true); + + const rows = collectRows(editor); + expect(rows.length).toBe(3); + rows.forEach(({ node }) => { + expect(node.attrs.trackChange?.type).toBe('rowInsert'); + expect(node.attrs.trackChange.author).toBe(ALICE.name); + }); + + const changes = enumerateStructuralRowChanges(editor.state); + expect(changes.length).toBe(1); + expect(changes[0].subtype).toBe('table-insert'); + expect(changes[0].decidable).toBe(true); + expect(changes[0].wholeTable).toBe(true); + expect(changes[0].rows.length).toBe(3); + }); + + it('does NOT delete selected content untracked when a table insert replaces a real selection', () => { + // Inserting a table over a NON-empty selection must never silently drop the + // selected text (data loss in suggesting mode). The structural fast-path only + // handles the no-real-content case (caret in an empty paragraph); for a + // content-bearing replace it bails so the selected text is preserved. + // (Full tracked-delete-then-insert for this case is a deferred enhancement.) + editor = setup({ track: true, content: '

existing

' }); + editor.commands.selectAll(); + + editor.commands.insertTable({ rows: 2, cols: 2 }); + + // The original text is still present — it was not removed untracked. + expect(editor.state.doc.textContent).toContain('existing'); + }); + + it('tracks the table in an EMPTY doc (toolbar from=0,to=2 replace of the initial empty paragraph)', () => { + // This is the exact layout-engine playground shape: an empty document holds + // a single empty paragraph; the toolbar insertTable dispatches one + // ReplaceStep from=0 to=2 whose slice is [table, paragraph]. The inline + // overlap-compiler cannot represent the empty table (no inline text) and + // fails closed, so this MUST take the structural-insert path. + editor = setup({ track: true, content: '' }); + + const ok = editor.commands.insertTable({ rows: 3, cols: 2 }); + expect(ok).toBe(true); + + // Inserted exactly once. + let tableCount = 0; + editor.state.doc.descendants((node) => { + if (node.type.name === 'table') tableCount += 1; + }); + expect(tableCount).toBe(1); + + const rows = collectRows(editor); + expect(rows.length).toBe(3); + rows.forEach(({ node }) => { + expect(node.attrs.trackChange?.type).toBe('rowInsert'); + expect(node.attrs.trackChange.author).toBe(ALICE.name); + }); + + // One shared revisionGroupId per table; distinct per-row ids. + expect(new Set(rows.map((r) => r.node.attrs.trackChange.revisionGroupId)).size).toBe(1); + expect(new Set(rows.map((r) => r.node.attrs.trackChange.id)).size).toBe(3); + + const changes = enumerateStructuralRowChanges(editor.state); + expect(changes.length).toBe(1); + expect(changes[0].subtype).toBe('table-insert'); + expect(changes[0].decidable).toBe(true); + expect(changes[0].wholeTable).toBe(true); + expect(changes[0].rows.length).toBe(3); + }); + + it('tracks the table with the cursor mid-content (insert after a non-empty paragraph)', () => { + editor = setup({ track: true, content: '

hello world

' }); + // Place the caret inside the paragraph (not a full selection). + editor.commands.setTextSelection(5); + + const ok = editor.commands.insertTable({ rows: 2, cols: 3 }); + expect(ok).toBe(true); + + let tableCount = 0; + editor.state.doc.descendants((node) => { + if (node.type.name === 'table') tableCount += 1; + }); + expect(tableCount).toBe(1); + + // The original paragraph text is untouched (the table is inserted around it, + // not replacing it). + expect(editor.state.doc.textContent).toContain('hello world'); + + const rows = collectRows(editor); + expect(rows.length).toBe(2); + rows.forEach(({ node }) => { + expect(node.attrs.trackChange?.type).toBe('rowInsert'); + }); + + const changes = enumerateStructuralRowChanges(editor.state); + expect(changes.length).toBe(1); + expect(changes[0].decidable).toBe(true); + expect(changes[0].wholeTable).toBe(true); + expect(changes[0].rows.length).toBe(2); + }); + + it('a single undo cleanly reverts a tracked table insert', () => { + editor = setup({ track: true, content: '

keep me

' }); + const before = editor.state.doc.toJSON(); + + editor.commands.insertTable({ rows: 3, cols: 2 }); + expect(collectRows(editor).length).toBe(3); + + editor.commands.undo(); + + // No table, no tracked rows, doc back to its original shape. + let tableCount = 0; + editor.state.doc.descendants((node) => { + if (node.type.name === 'table') tableCount += 1; + }); + expect(tableCount).toBe(0); + expect(collectRows(editor).length).toBe(0); + expect(enumerateStructuralRowChanges(editor.state)).toEqual([]); + expect(editor.state.doc.toJSON()).toEqual(before); + }); + + it('roundtrip: stamped rows export inside ', () => { + editor = setup({ track: true }); + editor.commands.insertTable({ rows: 2, cols: 2 }); + + const rows = collectRows(editor); + expect(rows.length).toBe(2); + + rows.forEach(({ node }) => { + // Run the live row node through the export (decode) translator and assert + // the structural revision marker lands inside . + const result = trTranslator.decode({ node: node.toJSON() }, {}); + const trPr = result.elements.find((el) => el.name === 'w:trPr'); + expect(trPr).toBeDefined(); + const ins = trPr.elements.find((el) => el.name === 'w:ins'); + expect(ins).toBeDefined(); + expect(ins.attributes['w:author']).toBe(ALICE.name); + // Revision marker is the first child of w:trPr. + expect(trPr.elements[0].name).toBe('w:ins'); + }); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/stampTableRows.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/stampTableRows.js new file mode 100644 index 0000000000..d4a356288c --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/stampTableRows.js @@ -0,0 +1,93 @@ +// @ts-check +import { v4 as uuidv4 } from 'uuid'; +import { collectWholeTablesInRange } from './collectWholeTablesInRange.js'; + +/** + * Stamp a structural tracked-change revision (`rowInsert` or `rowDelete`) on + * every row of any WHOLE table fully contained in `[from, to)`. + * + * Structural tracked changes do not live on inline marks: a tracked + * inserted/deleted table carries `tableRow.attrs.trackChange` on each of its + * rows — the same shape the importer lands from ``/`` inside + * `` (see `core/super-converter/v3/handlers/w/tr/row-track-change.js`). + * `markInsertion`/`markDeletion` (and the overlap compiler) only mark INLINE + * content and explicitly skip table internals, so a table authored in + * suggesting mode would otherwise carry no structural markup at all. Stamping + * the rows lets the existing downstream machinery + * (`enumerateStructuralRowChanges` → review-model → paint/bubble → export) treat + * the table as ONE decidable whole-table change. + * + * - For an INSERT (`type: 'rowInsert'`) the table node is freshly inserted in + * the range; for a tracked DELETE (`type: 'rowDelete'`) the deletion is NOT + * applied — the content stays VISIBLE (struck-through) so the table node + * remains in the doc. + * - Only tables fully contained in the range are stamped (table starts at/after + * `from`, ends at/before `to`). A row/column-level change or a row inserted + * into a pre-existing table merely overlaps the range and is out of scope. + * - Every row of a given table shares one `revisionGroupId` so the enumerator + * groups them as a single change. Word assigns a distinct `w:id` per row, so + * each row also gets its own `id`. + * - An existing `trackChange` is never clobbered (e.g. re-inserted imported + * content, or a row already tracked-inserted that is now being deleted). + * + * `setNodeMarkup` does not change node size, so row positions stay stable while + * stamping multiple rows of one table; we still walk fresh per table. + * + * @param {object} options + * @param {'rowInsert'|'rowDelete'} options.type - Revision side to stamp. + * @param {import('prosemirror-state').Transaction} options.tr - Transaction whose doc contains the table(s). + * @param {number} options.from - Start of the range (inclusive). + * @param {number} options.to - End of the range (exclusive). + * @param {import('../../../core/types/EditorConfig.js').User} options.user - Acting user, attributed on the revision. + * @param {string} options.date - Revision timestamp (ISO-8601). + * @returns {boolean} True if at least one row was stamped. + */ +export const stampTableRows = ({ type, tr, from, to, user, date }) => { + if (type !== 'rowInsert' && type !== 'rowDelete') return false; + + const tables = collectWholeTablesInRange({ doc: tr.doc, from, to }); + if (!tables.length) return false; + + let stamped = false; + + for (const { pos: tablePos, node: tableNode } of tables) { + // One shared revision identity per table so the enumerator groups all rows + // as a single whole-table change. + const revisionGroupId = uuidv4(); + + // Collect row positions first (positions are stable under setNodeMarkup, + // but reading the live node before each markup keeps attrs fresh). + let offset = 1; + /** @type {Array} */ + const rowPositions = []; + tableNode.forEach((child) => { + const childPos = tablePos + offset; + offset += child.nodeSize; + if (child.type?.name === 'tableRow') rowPositions.push(childPos); + }); + + for (const rowPos of rowPositions) { + const rowNode = tr.doc.nodeAt(rowPos); + if (!rowNode || rowNode.type?.name !== 'tableRow') continue; + // Don't clobber an existing structural revision. + if (rowNode.attrs?.trackChange) continue; + + /** @type {import('../../../extensions/table-row/table-row.js').TableRowTrackChange} */ + const trackChange = { + type, + id: uuidv4(), + author: user?.name || '', + authorId: user?.id || '', + authorEmail: user?.email || '', + authorImage: user?.image || '', + date, + revisionGroupId, + }; + + tr.setNodeMarkup(rowPos, undefined, { ...rowNode.attrs, trackChange }); + stamped = true; + } + } + + return stamped; +}; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/structuralRowChanges.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/structuralRowChanges.js new file mode 100644 index 0000000000..2235b5b85f --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/structuralRowChanges.js @@ -0,0 +1,192 @@ +// @ts-check +/** + * Structural tracked-change enumerator for whole-table insert/delete. + * + * A whole inserted/deleted table is encoded in OOXML as ``/`` + * inside every row's ``. Real Word documents assign a DISTINCT `w:id` + * per row (e.g. a 3-row inserted table carries 3 different `w:id`s, all + * inserts). The importer lands each marker on `tableRow.attrs.trackChange` + * (see `core/super-converter/v3/handlers/w/tr/row-track-change.js`). Unlike + * inline tracked text, structural row revisions are NOT marks — they live on + * row node attributes, so the inline-mark enumerators (`getTrackChanges`, + * `enumerateTrackedMarkSpans`) never see them. + * + * This module is the single owner of structural-row discovery. It walks the PM + * document, finds `table` nodes whose rows carry `trackChange`, and groups them + * at the TABLE level by SIDE (NOT by id, because ids legitimately differ per + * row). + * + * Whole-table rule (spec TC-OPS-003): + * A table is a WHOLE-TABLE insert/delete iff EVERY row of the table carries a + * `trackChange` AND every tracked row shares the SAME side (all `rowInsert` + * or all `rowDelete`). Ids MAY differ. Only then do we emit ONE decidable + * structural change (`table-insert`/`table-delete`) covering the table. + * + * If only SOME rows are tracked, OR the sides are mixed within one table, it + * is NOT a whole-table change (row-level structural is out of scope). + * We still SURFACE such a shape (so it is never silently dropped) but flag it + * `wholeTable: false` / `decidable: false`. The decision engine fails such a + * shape closed (CAPABILITY_UNAVAILABLE) and NEVER routes it through the + * whole-table removal path — the table is never removed. + */ + +/** + * @typedef {Object} StructuralRowRef + * @property {number} pos Absolute PM position of the row node. + * @property {number} from Same as `pos` (row start). + * @property {number} to Row end (`pos + node.nodeSize`). + * @property {import('prosemirror-model').Node} node + */ + +/** + * @typedef {Object} StructuralChange + * @property {string} id Logical (and public) change id (stable per table+side). + * @property {string} revisionId Representative revision id (first row's id; ids may differ). + * @property {'insertion'|'deletion'} side + * @property {'table-insert'|'table-delete'} subtype + * @property {number} tableFrom Table node start. + * @property {number} tableTo Table node end. + * @property {number} tablePos Table node start (alias of tableFrom). + * @property {boolean} wholeTable True when every row of the table is tracked and shares one side. + * @property {boolean} decidable True only for whole-table changes; false fails closed on decide. + * @property {string} [undecidableReason] Why a non-whole-table shape is not decidable. + * @property {Array} rows + * @property {string} author + * @property {string} authorEmail + * @property {string} authorImage + * @property {string} date + * @property {string} importedAuthor + * @property {string} sourceId + * @property {string} revisionGroupId + */ + +/** + * Enumerate structural row changes (whole-table insert/delete) in the doc. + * + * Tolerates a missing/partial state and returns `[]` instead of throwing, to + * match the inline enumerators' bootstrap-safety contract. + * + * @param {import('prosemirror-state').EditorState | { doc?: import('prosemirror-model').Node } | null | undefined} state + * @returns {Array} + */ +export const enumerateStructuralRowChanges = (state) => { + const doc = state?.doc; + if (!doc) return []; + + /** @type {Array} */ + const out = []; + + try { + doc.descendants((node, pos) => { + if (node.type?.name !== 'table') return undefined; + collectTableStructuralChanges({ table: node, tablePos: pos, out }); + // Keep walking so a nested table inside a (possibly non-tracked) cell is + // still discovered as its own independent change. + return true; + }); + } catch { + return out; + } + + return out; +}; + +/** + * @param {{ table: import('prosemirror-model').Node, tablePos: number, out: Array }} input + */ +const collectTableStructuralChanges = ({ table, tablePos, out }) => { + const tableFrom = tablePos; + const tableTo = tablePos + table.nodeSize; + + /** @type {Array<{ ref: StructuralRowRef, side: 'insertion'|'deletion', tc: Record }>} */ + const trackedRows = []; + let totalRows = 0; + + // Row children are direct children of the table node; their absolute position + // is `tablePos + 1 + offsetWithinTable`. + let offset = 1; + table.forEach((child) => { + const childFrom = tablePos + offset; + offset += child.nodeSize; + if (child.type?.name !== 'tableRow') return; + totalRows += 1; + const tc = child.attrs?.trackChange; + if (!tc || (tc.type !== 'rowInsert' && tc.type !== 'rowDelete')) return; + const side = tc.type === 'rowInsert' ? 'insertion' : 'deletion'; + trackedRows.push({ + ref: { pos: childFrom, from: childFrom, to: childFrom + child.nodeSize, node: child }, + side, + tc, + }); + }); + + if (trackedRows.length === 0) return; + + const sides = new Set(trackedRows.map((r) => r.side)); + const everyRowTracked = trackedRows.length === totalRows && totalRows > 0; + const singleSide = sides.size === 1; + const wholeTable = everyRowTracked && singleSide; + + if (wholeTable) { + const side = /** @type {'insertion'|'deletion'} */ (trackedRows[0].side); + const primary = trackedRows[0].tc; + const representativeRevisionId = stringOf(primary.id) || stringOf(primary.sourceId); + // A stable public id derived from the first row's sourceId (Word w:id) so it + // survives import → export → reopen; falls back to the table position when + // there is no source id (native, freshly authored). Table-scoped so two + // tracked tables that happen to share a Word id never collapse. + const publicId = stringOf(primary.sourceId) || representativeRevisionId || `table:${tableFrom}:${side}`; + out.push({ + id: publicId, + revisionId: representativeRevisionId, + side, + subtype: side === 'insertion' ? 'table-insert' : 'table-delete', + tableFrom, + tableTo, + tablePos, + wholeTable: true, + decidable: true, + rows: trackedRows.map((r) => r.ref), + author: stringOf(primary.author), + authorEmail: stringOf(primary.authorEmail), + authorImage: stringOf(primary.authorImage), + date: stringOf(primary.date), + importedAuthor: stringOf(primary.importedAuthor), + sourceId: stringOf(primary.sourceId), + revisionGroupId: stringOf(primary.revisionGroupId) || representativeRevisionId || publicId, + }); + return; + } + + // NOT a whole-table change: either a partial subset of rows is tracked, or + // the sides are mixed within one table. Surface it (so it is never silently + // dropped) but mark it undecidable so the decision engine fails it closed and + // NEVER removes the table. We emit ONE entry per table here (not per id), with + // a side chosen only for display purposes when uniform, else the first row's. + const side = /** @type {'insertion'|'deletion'} */ (trackedRows[0].side); + const primary = trackedRows[0].tc; + const representativeRevisionId = stringOf(primary.id) || stringOf(primary.sourceId); + const reason = !everyRowTracked ? 'partial-rows' : 'mixed-sides'; + out.push({ + id: stringOf(primary.sourceId) || representativeRevisionId || `table:${tableFrom}:${side}`, + revisionId: representativeRevisionId, + side, + subtype: side === 'insertion' ? 'table-insert' : 'table-delete', + tableFrom, + tableTo, + tablePos, + wholeTable: false, + decidable: false, + undecidableReason: reason, + rows: trackedRows.map((r) => r.ref), + author: stringOf(primary.author), + authorEmail: stringOf(primary.authorEmail), + authorImage: stringOf(primary.authorImage), + date: stringOf(primary.date), + importedAuthor: stringOf(primary.importedAuthor), + sourceId: stringOf(primary.sourceId), + revisionGroupId: stringOf(primary.revisionGroupId) || representativeRevisionId, + }); +}; + +const stringOf = (value) => (typeof value === 'string' ? value : value == null ? '' : String(value)); diff --git a/packages/superdoc/src/SuperDoc.vue b/packages/superdoc/src/SuperDoc.vue index 8bf5004314..6091f78e62 100644 --- a/packages/superdoc/src/SuperDoc.vue +++ b/packages/superdoc/src/SuperDoc.vue @@ -57,6 +57,7 @@ import { useFindReplace } from './composables/use-find-replace.js'; import { createV1EditorRuntimeAdapter } from './core/editor-runtime/v1/v1-editor-runtime-adapter.js'; import { markRuntimeRoot, unmarkRuntimeRoot } from './core/editor-runtime/root-marker.js'; import { collectTouchedTrackedChangeIds } from './helpers/collect-touched-tracked-change-ids.js'; +import { transactionTouchesStructuralChange } from './helpers/transaction-touches-structural-change.js'; import SurfaceHost from './components/surfaces/SurfaceHost.vue'; import { DEFAULT_COMMENTS_DISPLAY_MODE, @@ -1359,6 +1360,12 @@ const onEditorTransaction = (payload = {}) => { // collaboration comment update is already shared through the comments ydoc. broadcastChanges: !isPeerCollaborationReplayTransaction(transaction, ySyncMeta), }); + } else if (transactionTouchesStructuralChange(transaction)) { + // Structural row tracked changes (whole-table insert/delete) live on node + // attrs, not inline marks, so the id-based targeted resync cannot see them. + // Force a full resync so structural bubbles appear/refresh during editing, + // not only on import. + queueTrackedChangeCommentResync({ editor }); } else { queueTrackedChangeCommentResync({ editor, diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue index 6f04259edb..7c4d0539a7 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue @@ -868,6 +868,12 @@ watch(editingCommentId, (commentId) => {
Added new line
+
+ Added table +
+
+ Deleted table +
Format: {{ comment.trackedChangeText }} diff --git a/packages/superdoc/src/helpers/transaction-touches-structural-change.js b/packages/superdoc/src/helpers/transaction-touches-structural-change.js new file mode 100644 index 0000000000..d980abb33a --- /dev/null +++ b/packages/superdoc/src/helpers/transaction-touches-structural-change.js @@ -0,0 +1,50 @@ +/** + * Detect whether a ProseMirror transaction touches a structural row tracked + * change (whole-table insert/delete encoded on `tableRow.attrs.trackChange`). + * + * Structural row revisions live on node attributes — NOT on inline marks — so + * the inline `collectTouchedTrackedChangeIds` scan never reports them. When such + * a node-attr change is part of a transaction, the right-rail structural bubble + * sync must run a FULL resync (the targeted, id-based resync path only refreshes + * inline mark bubbles). This helper flags those transactions so the caller can + * force a full resync, mirroring how inline mark edits trigger sidebar refresh. + * + * Conservative by design: returns true if any node within a changed range is a + * `tableRow` carrying a `rowInsert`/`rowDelete` `trackChange`. False positives + * only cost an extra full resync; false negatives would silently drop the bubble. + * + * @param {import('prosemirror-state').Transaction} transaction + * @returns {boolean} + */ +export function transactionTouchesStructuralChange(transaction) { + if (!transaction?.docChanged || !transaction?.doc || !transaction?.mapping?.maps?.length) return false; + + const docSize = transaction.doc.content.size; + let touched = false; + + transaction.mapping.maps.forEach((stepMap, stepIndex) => { + if (touched) return; + stepMap.forEach((oldStart, oldEnd, newStart, newEnd) => { + if (touched) return; + const mappingOffset = stepIndex + 1; + const mappedFrom = transaction.mapping.slice(mappingOffset).map(newStart, 1); + const mappedTo = transaction.mapping.slice(mappingOffset).map(newEnd, -1); + const from = Math.max(0, mappedFrom - 1); + const to = Math.min(docSize, mappedTo + 1); + if (from >= to) return; + + transaction.doc.nodesBetween(from, to, (node) => { + if (touched) return false; + if (node?.type?.name !== 'tableRow') return undefined; + const tc = node.attrs?.trackChange; + if (tc && (tc.type === 'rowInsert' || tc.type === 'rowDelete')) { + touched = true; + return false; + } + return undefined; + }); + }); + }); + + return touched; +} diff --git a/packages/superdoc/src/helpers/transaction-touches-structural-change.test.js b/packages/superdoc/src/helpers/transaction-touches-structural-change.test.js new file mode 100644 index 0000000000..96612149b0 --- /dev/null +++ b/packages/superdoc/src/helpers/transaction-touches-structural-change.test.js @@ -0,0 +1,68 @@ +import { describe, it, expect } from 'vitest'; +import { transactionTouchesStructuralChange } from './transaction-touches-structural-change.js'; + +// Minimal PM-like stubs. We only exercise the docChanged + mapping + nodesBetween +// traversal contract the helper depends on, not a real ProseMirror document. + +const makeMapping = (ranges) => ({ + maps: ranges.map(() => ({ + forEach(cb) { + // Report a single output range per map. Coordinates are arbitrary; the + // helper maps them through slice().map() which we make identity below. + cb(0, 0, 0, 100); + }, + })), + slice() { + return { map: (pos) => pos }; + }, +}); + +const makeDoc = (rows) => ({ + content: { size: 1000 }, + nodesBetween(_from, _to, cb) { + for (const row of rows) { + const keepGoing = cb(row); + if (keepGoing === false) break; + } + }, +}); + +const tableRow = (trackChangeType) => ({ + type: { name: 'tableRow' }, + attrs: trackChangeType ? { trackChange: { type: trackChangeType } } : {}, +}); + +const paragraph = () => ({ type: { name: 'paragraph' }, attrs: {} }); + +describe('transactionTouchesStructuralChange', () => { + it('returns false when the transaction did not change the doc', () => { + expect(transactionTouchesStructuralChange({ docChanged: false })).toBe(false); + }); + + it('returns false when no tracked table rows are in the touched range', () => { + const transaction = { + docChanged: true, + doc: makeDoc([paragraph(), tableRow(null)]), + mapping: makeMapping([1]), + }; + expect(transactionTouchesStructuralChange(transaction)).toBe(false); + }); + + it('returns true when a touched table row carries a rowInsert track change', () => { + const transaction = { + docChanged: true, + doc: makeDoc([paragraph(), tableRow('rowInsert')]), + mapping: makeMapping([1]), + }; + expect(transactionTouchesStructuralChange(transaction)).toBe(true); + }); + + it('returns true when a touched table row carries a rowDelete track change', () => { + const transaction = { + docChanged: true, + doc: makeDoc([tableRow('rowDelete')]), + mapping: makeMapping([1]), + }; + expect(transactionTouchesStructuralChange(transaction)).toBe(true); + }); +}); diff --git a/packages/superdoc/src/stores/comments-store.js b/packages/superdoc/src/stores/comments-store.js index 96b6aaaa09..5406006082 100644 --- a/packages/superdoc/src/stores/comments-store.js +++ b/packages/superdoc/src/stores/comments-store.js @@ -33,6 +33,26 @@ export const useCommentsStore = defineStore('comments', () => { return makeTrackedChangeAnchorKey({ storyKey: 'body', rawId: String(rawId) }); }; + /** + * Compute the stable public id for a structural (whole-table) tracked change. + * + * This MUST match the public id the document-api projects for the same change + * (`tracked-change-resolver.groupTrackedChanges`), otherwise the right-rail + * accept/reject would route `trackChanges.decide` to a change that does not + * exist. The doc-api derives the public id as `word:structural:` + * for imported Word revisions, falling back to the per-import logical id. + * Reuse the exact rule here so the bubble id is the decide id. + * + * @param {{ sourceId?: string, id?: string }} structural + * @returns {string | null} + */ + const buildStructuralTrackedChangeId = (structural) => { + if (!structural) return null; + const sourceId = structural.sourceId ? String(structural.sourceId) : ''; + if (sourceId) return `word:structural:${sourceId}`; + return structural.id ? String(structural.id) : null; + }; + const superdocStore = useSuperdocStore(); const commentsConfig = reactive({ name: 'comments', @@ -608,6 +628,56 @@ export const useCommentsStore = defineStore('comments', () => { } = params; const normalizedChangeId = changeId != null ? String(changeId) : null; const normalizedDocumentId = documentId != null ? String(documentId) : null; + + // Subsume inline tracked changes inside a tracked whole-table change: the + // change is owned by the structural "Inserted/Deleted table" bubble and must + // not become its own review item (no floating bubble, no active-on-click + // dialog, no separate accept). This is the central chokepoint EVERY creation + // path funnels through — full resync, targeted resync, AND the live + // comments-plugin transaction handler — so suppressing here covers them all. + // The structural bubble itself (display tableInsert/tableDelete) is exempt. + const isStructuralTableBubble = + trackedChangeDisplayType === 'tableInsert' || trackedChangeDisplayType === 'tableDelete'; + if (!isStructuralTableBubble && normalizedChangeId) { + // Resolve the document editor: prefer the event's documentId, fall back to + // the active editor so the guard still fires if a path omits documentId. + const effectiveDocumentId = + normalizedDocumentId ?? + (superdoc?.activeEditor?.options?.documentId != null ? String(superdoc.activeEditor.options.documentId) : null); + const docEditor = effectiveDocumentId ? superdocStore.getDocument(effectiveDocumentId)?.getEditor?.() : null; + const docState = docEditor?.state ?? superdoc?.activeEditor?.state ?? null; + if (docState) { + // (a) The changeId IS a structural whole-table change id (or one of its + // row ids) — text typed inside a tracked-inserted row inherits the row's + // revision id (the cell text and the row share one OOXML w:id). Owned by + // the structural bubble; no separate item. + // (a) A distinct-id inline change whose marks are wholly INSIDE a tracked + // whole-table range is subsumed. (b) Or the changeId is a structural row + // id echoed with NO inline marks (e.g. the comments-plugin re-emitting the + // structural change as a generic tracked change) — subsumed too. + // The id-set match is gated on "no inline range" so an inline change that + // merely SHARES a row/source id but lives OUTSIDE the table is never + // wrongly suppressed (its real range fails the table containment check). + const { ranges: tableRanges, ids: structuralIds } = computeTrackedTableSummaryForState(docState); + const ranges = trackChangesHelpers.getTrackChanges(docState, normalizedChangeId); + const inRange = + tableRanges.length > 0 && ranges.length > 0 && isInlineRangeInsideTrackedTable(ranges, tableRanges); + const isStructuralRowIdEcho = ranges.length === 0 && structuralIds.has(normalizedChangeId); + const subsumed = inRange || isStructuralRowIdEcho; + if (subsumed) { + // Block creation AND remove any stale duplicate created earlier (e.g. + // on a prior keystroke or via an import/replay bypass). Pruning never + // touches the structural bubble (it excludes table-insert/delete). + pruneSuppressedInlineTableComments({ + suppressedIds: new Set([normalizedChangeId]), + activeDocumentId: effectiveDocumentId, + superdoc, + broadcastChanges, + }); + return; + } + } + } const hasStoryMetadata = trackedChangeStory !== undefined || trackedChangeStoryKind !== undefined || @@ -800,6 +870,10 @@ export const useCommentsStore = defineStore('comments', () => { const documentId = editor?.options?.documentId != null ? String(editor.options.documentId) : null; if (!documentId) return; + // Inline changes inside a tracked whole-table change are subsumed by the + // structural "Inserted/Deleted table" bubble. Suppression + pruning of such a + // change is handled centrally in `handleTrackedChangeUpdate` (the chokepoint + // every creation path funnels through), so no per-path check is needed here. for (const changeId of new Set(changeIds.map((id) => (id != null ? String(id) : null)).filter(Boolean))) { const trackedChangesForId = trackChangesHelpers.getTrackChanges(editor.state, changeId); if (!trackedChangesForId.length) continue; @@ -1205,6 +1279,15 @@ export const useCommentsStore = defineStore('comments', () => { createCommentForTrackChanges(editor, superdoc); syncStoryTrackedChangeComments({ superdoc, editor }); + // Whole-table structural tracked changes live on node attrs (not inline + // marks), so `createCommentForTrackChanges` never sees them. Without this + // pass the "Added table" right-rail bubble is not created on import and + // only appears after a later transaction triggers the full + // `syncTrackedChangeComments` path. Mirror the inline/story bootstrap here. + // Idempotent: `syncStructuralTrackedChangeComments` upserts (event 'update' + // when a matching bubble already exists), so re-running it later does not + // duplicate bubbles. + syncStructuralTrackedChangeComments({ superdoc, editor }); }; /** @@ -1374,7 +1457,10 @@ export const useCommentsStore = defineStore('comments', () => { const documentId = activeDocumentId; - // Build comment params directly from grouped changes — no PM dispatch needed + // Inline changes inside a tracked whole-table change are subsumed by the + // structural "Inserted/Deleted table" bubble. Suppression + pruning of such a + // change is handled centrally in `handleTrackedChangeUpdate` (called per + // change below), so no per-path check is needed here. const processedIds = new Set(); groupedChanges.forEach(({ insertedMark, deletionMark, formatMark }) => { const id = insertedMark?.mark.attrs.id || deletionMark?.mark.attrs.id || formatMark?.mark.attrs.id; @@ -1424,6 +1510,64 @@ export const useCommentsStore = defineStore('comments', () => { editor.view.dispatch(tr); }; + /** + * Remove inline tracked-change comments whose change id is subsumed by a + * tracked whole-table change. Mirrors the deletion-event emission of + * `pruneStaleTrackedChangeComments` but is keyed on an explicit suppressed-id + * set rather than mark liveness (the mark is still live; only the review item + * is unwanted). Also clears the active-comment selection if it pointed at a + * now-suppressed change, so no stale active-on-click dialog can reference it. + * + * @param {{ suppressedIds: Set, activeDocumentId: string, superdoc: any, broadcastChanges?: boolean }} input + */ + const pruneSuppressedInlineTableComments = ({ + suppressedIds, + activeDocumentId, + superdoc = null, + broadcastChanges = true, + }) => { + if (!(suppressedIds instanceof Set) || !suppressedIds.size || !activeDocumentId) return; + + const removedComments = []; + commentsList.value = commentsList.value.filter((comment) => { + if (!comment?.trackedChange) return true; + if (!isBodyTrackedChangeComment(comment)) return true; + if (isStructuralTableBubble(comment)) return true; + if (!belongsToTrackedChangeSyncDocument(comment, activeDocumentId)) return true; + + const commentId = comment.commentId != null ? String(comment.commentId) : null; + const importedId = comment.importedId != null ? String(comment.importedId) : null; + const isSuppressed = (commentId && suppressedIds.has(commentId)) || (importedId && suppressedIds.has(importedId)); + if (!isSuppressed) return true; + + removedComments.push(comment); + return false; + }); + + if (!removedComments.length) return; + + const removedIds = new Set(); + removedComments.forEach((comment) => { + if (comment.commentId != null) removedIds.add(String(comment.commentId)); + if (comment.importedId != null) removedIds.add(String(comment.importedId)); + const payload = getCommentEventPayload(comment); + const event = { + type: COMMENT_EVENTS.DELETED, + comment: payload, + changes: [{ key: 'deleted', commentId: payload.commentId, fileId: payload.fileId }], + }; + if (broadcastChanges) { + syncCommentsToClients(superdoc, event); + superdoc?.emit?.('comments-update', event); + } + }); + + const activeCommentId = activeComment.value != null ? String(activeComment.value) : null; + if (activeCommentId && removedIds.has(activeCommentId)) { + clearActiveCommentSelection(); + } + }; + const getCommentDocumentId = (comment) => { if (!comment) return null; if (comment.fileId != null) return String(comment.fileId); @@ -1668,6 +1812,94 @@ export const useCommentsStore = defineStore('comments', () => { }); syncStoryTrackedChangeComments({ superdoc, editor, broadcastChanges, snapshots: storySnapshots }); + syncStructuralTrackedChangeComments({ superdoc, editor, broadcastChanges }); + }; + + /** + * Surface decidable whole-table structural tracked changes (table insert / + * table delete) as right-rail bubbles, mirroring the inline tracked-change + * path. Structural row revisions live on node attrs (not inline marks), so + * the inline `getTrackChanges` enumerator never sees them — they are + * enumerated separately here. + * + * Only `decidable` whole-table changes are surfaced; partial/mixed shapes are + * not actionable (the decision engine fails them closed) so they get no + * bubble. The bubble id is the document-api public id so accept/reject in the + * sidebar routes `trackChanges.decide` to the same change. + * + * Positioning: the bubble carries a body-story anchorKey (matching the + * tracked-change index snapshot) and a body PM range (table start/end). The + * PresentationEditor position pass emits a body-story position entry for that + * anchorKey, whose bounds resolve through the same `getRangeRects` path inline + * body comments/TC use — so it lines up with the table in layout-engine + * viewing mode. + */ + const syncStructuralTrackedChangeComments = ({ superdoc, editor, broadcastChanges = true }) => { + const activeDocumentId = editor?.options?.documentId != null ? String(editor.options.documentId) : null; + if (!activeDocumentId) return; + + const enumerate = trackChangesHelpers?.enumerateStructuralRowChanges; + if (typeof enumerate !== 'function') return; + + let structuralChanges = []; + try { + structuralChanges = enumerate(editor.state) ?? []; + } catch { + structuralChanges = []; + } + + for (const structural of structuralChanges) { + // Only decidable whole-table changes are actionable from the sidebar. + if (!structural?.decidable || !structural?.wholeTable) continue; + + const publicId = buildStructuralTrackedChangeId(structural); + if (!publicId) continue; + + const anchorKey = buildBodyTrackedChangeAnchorKey(publicId); + const displayType = structural.subtype === 'table-insert' ? 'tableInsert' : 'tableDelete'; + const trackedChangeType = structural.side === 'insertion' ? 'trackInsert' : 'trackDelete'; + + // Mirror the story path: 'add' creates a bubble when none exists yet and + // refreshes an existing one; 'update' alone would no-op on first import + // (handleTrackedChangeUpdate returns early when no comment is found). + const normalizedPublicId = String(publicId); + const normalizedAnchorKey = anchorKey != null ? String(anchorKey) : null; + const existingComment = commentsList.value.find((comment) => { + if (!comment?.trackedChange) return false; + if (!belongsToTrackedChangeSyncDocument(comment, activeDocumentId)) return false; + const commentAnchorKey = comment.trackedChangeAnchorKey != null ? String(comment.trackedChangeAnchorKey) : null; + if (normalizedAnchorKey && commentAnchorKey) return commentAnchorKey === normalizedAnchorKey; + const commentId = comment.commentId != null ? String(comment.commentId) : null; + const importedId = comment.importedId != null ? String(comment.importedId) : null; + return commentId === normalizedPublicId || importedId === normalizedPublicId; + }); + + const params = { + event: existingComment ? 'update' : 'add', + changeId: publicId, + trackedChangeText: '', + trackedChangeType, + trackedChangeDisplayType: displayType, + deletedText: null, + authorId: null, + authorEmail: structural.authorEmail || null, + authorImage: structural.authorImage || null, + date: structural.date || null, + author: structural.author || null, + // Match the inline tracked-change shape: the comment layer reads + // `importedAuthor.name` (see use-comment.js `getCommentUser`). Passing the + // raw string would make `.name` undefined and fall back to "(Imported)". + importedAuthor: structural.importedAuthor ? { name: structural.importedAuthor } : null, + documentId: activeDocumentId, + coords: null, + trackedChangeStory: BODY_TRACKED_CHANGE_STORY, + trackedChangeStoryKind: 'body', + trackedChangeStoryLabel: '', + trackedChangeAnchorKey: anchorKey, + }; + + handleTrackedChangeUpdate({ superdoc, params, broadcastChanges }); + } }; const syncStoryTrackedChangeComments = ({ superdoc, editor, broadcastChanges = true, snapshots = null }) => { @@ -1860,6 +2092,150 @@ export const useCommentsStore = defineStore('comments', () => { editorCommentPositions.value = {}; }; + /** + * Identify the single structural (whole-table) bubble for a tracked table so + * it is NEVER suppressed by the table-subsume filter below. The structural + * bubble is the parent "Added table" / "Deleted table" change; its public + * id is `word:structural:` (or a bare structural fallback) and its display + * type is `tableInsert` / `tableDelete`. + * + * @param {Object} comment + * @returns {boolean} + */ + const isStructuralTableBubble = (comment) => { + if (!comment?.trackedChange) return false; + const displayType = comment?.trackedChangeDisplayType; + if (displayType === 'tableInsert' || displayType === 'tableDelete') return true; + const ids = [comment?.commentId, comment?.importedId, comment?.trackedChangeAnchorKey] + .map((id) => (id != null ? String(id) : '')) + .filter(Boolean); + return ids.some((id) => id.includes('word:structural:')); + }; + + /** + * Compute the decidable whole-table tracked-change ranges for a comment's + * document, memoized per editor state so the filter does not re-enumerate the + * document once per comment. Returns `[]` when the document has no tracked + * whole-table change (the common case), so non-tracked tables and inline-only + * tracked changes are never affected. + * + * @param {string | null | undefined} fileId + * @returns {Array<{ from: number, to: number }>} + */ + const trackedTableSummaryCache = new WeakMap(); + + /** + * Single source of truth for the decidable whole-table tracked changes in a PM + * state, enumerated and memoized ONCE per state. Returns: + * - `ranges`: each tracked whole-table's `{ from, to }` document span. Used to + * test whether an inline tracked change falls inside a tracked table. + * - `ids`: every change id associated with those tables (the change's public + * id / revisionId / revisionGroupId / sourceId plus each row's trackChange + * id / sourceId). Text typed inside a tracked-inserted row inherits that + * row's revision id, so such an inline change reports a changeId that is one + * of these — it must be subsumed by the structural bubble, not get its own + * review item. + * + * @param {import('prosemirror-state').EditorState | null | undefined} state + * @returns {{ ranges: Array<{ from: number, to: number }>, ids: Set }} + */ + const computeTrackedTableSummaryForState = (state) => { + if (!state) return { ranges: [], ids: new Set() }; + + const cached = trackedTableSummaryCache.get(state); + if (cached) return cached; + + const ranges = []; + const ids = new Set(); + const add = (value) => { + if (value != null && value !== '') ids.add(String(value)); + }; + + const enumerate = trackChangesHelpers?.enumerateStructuralRowChanges; + if (typeof enumerate === 'function') { + let structuralChanges = []; + try { + structuralChanges = enumerate(state) ?? []; + } catch { + structuralChanges = []; + } + for (const change of structuralChanges) { + if (!change?.decidable || !change?.wholeTable) continue; + const from = Number(change.tableFrom); + const to = Number(change.tableTo); + if (Number.isFinite(from) && Number.isFinite(to)) ranges.push({ from, to }); + add(change.id); + add(change.revisionId); + add(change.revisionGroupId); + add(change.sourceId); + for (const row of change.rows || []) { + const tc = row?.node?.attrs?.trackChange; + add(tc?.id); + add(tc?.sourceId); + } + } + } + + const summary = { ranges, ids }; + trackedTableSummaryCache.set(state, summary); + return summary; + }; + + const getTrackedTableRangesForDocument = (fileId) => { + const doc = superdocStore.getDocument(fileId); + const editor = doc?.getEditor?.(); + return computeTrackedTableSummaryForState(editor?.state).ranges; + }; + + /** + * True when every `{ from, to }` range of an inline tracked change is wholly + * contained within some decidable whole-table tracked-change range. Such an + * inline change is subsumed by the structural "Inserted/Deleted table" review + * item and must NOT become an independent review item (no comment object → + * no floating bubble and no active-on-click dialog). The underlying + * trackInsert/trackDelete mark (the green highlight) is untouched — only the + * review-item comment is suppressed. + * + * @param {Array<{ from?: number, to?: number }>} changeRanges + * @param {Array<{ from: number, to: number }>} tableRanges + * @returns {boolean} + */ + const isInlineRangeInsideTrackedTable = (changeRanges, tableRanges) => { + if (!tableRanges?.length || !changeRanges?.length) return false; + return changeRanges.every((seg) => { + const start = Number(seg?.from); + const end = Number(seg?.to); + if (!Number.isFinite(start) || !Number.isFinite(end)) return false; + return tableRanges.some((table) => start >= table.from && end <= table.to); + }); + }; + + /** + * Suppress an inline tracked-change bubble whose document range falls within a + * tracked whole-table change's range, so only the structural "Added table" + * / "Deleted table" bubble shows for that table (matching Word / Google Docs). + * The structural bubble itself, real user comments, and inline tracked changes + * inside a NON-tracked table are never suppressed. + * + * @param {Object} comment + * @returns {boolean} True when the bubble must NOT render. + */ + const isInlineTrackedChangeInsideTrackedTable = (comment) => { + // Only inline TRACKED-CHANGE bubbles are candidates. Real user comments and + // the structural table bubble are always kept. + if (!comment?.trackedChange) return false; + if (isStructuralTableBubble(comment)) return false; + + const ranges = getTrackedTableRangesForDocument(comment?.fileId); + if (!ranges.length) return false; + + const entry = resolveCommentPositionEntry(comment).entry; + const range = getCommentPositionRange(entry); + if (!range) return false; + + return ranges.some((table) => range.start >= table.from && range.end <= table.to); + }; + const getFloatingComments = computed(() => { const comments = getGroupedComments.value?.parentComments .filter((c) => !c.resolvedTime) @@ -1869,12 +2245,16 @@ export const useCommentsStore = defineStore('comments', () => { // selection.source) must have a live position in the document. if (!isEditorBackedComment(c)) return true; return Boolean(resolveCommentPositionEntry(c).entry); - }); + }) + // Coalesce a tracked whole-table change into ONE bubble: an inline + // tracked-change bubble inside a tracked inserted/deleted table is + // subsumed by the structural bubble and must not render. + .filter((c) => !isInlineTrackedChangeInsideTrackedTable(c)); return comments; }); const getFloatingCommentInstances = computed(() => { - return getFloatingComments.value.flatMap((comment) => { + const instances = getFloatingComments.value.flatMap((comment) => { const { key, entry } = resolveCommentPositionEntry(comment); const fallbackId = getCommentAliasIds(comment)[0] ?? normalizeCommentId(comment?.commentId) ?? null; @@ -1885,6 +2265,7 @@ export const useCommentsStore = defineStore('comments', () => { fallbackId, }); }); + return instances; }); const normalizeFloatingCommentInstanceId = (instanceId) => { diff --git a/packages/superdoc/src/stores/comments-store.test.js b/packages/superdoc/src/stores/comments-store.test.js index 9bab624d46..6a69212092 100644 --- a/packages/superdoc/src/stores/comments-store.test.js +++ b/packages/superdoc/src/stores/comments-store.test.js @@ -87,6 +87,7 @@ vi.mock('@superdoc/super-editor', async () => { }, trackChangesHelpers: { getTrackChanges: vi.fn(() => []), + enumerateStructuralRowChanges: vi.fn(() => []), }, createOrUpdateTrackedChangeComment: vi.fn(({ event, marks, documentId }) => { const changeId = marks?.insertedMark?.attrs?.id ?? marks?.deletionMark?.attrs?.id ?? marks?.formatMark?.attrs?.id; @@ -150,6 +151,7 @@ describe('comments-store', () => { __mockSuperdoc.documents.value = [{ id: 'doc-1', type: 'docx' }]; groupChangesMock.mockReturnValue([]); trackChangesHelpersMock.getTrackChanges.mockReturnValue([]); + trackChangesHelpersMock.enumerateStructuralRowChanges?.mockReturnValue([]); createOrUpdateTrackedChangeCommentMock.mockImplementation(({ event, marks, documentId }) => { const changeId = marks?.insertedMark?.attrs?.id ?? marks?.deletionMark?.attrs?.id ?? marks?.formatMark?.attrs?.id; if (changeId == null) return; @@ -1341,6 +1343,476 @@ describe('comments-store', () => { expect(editorDispatch).toHaveBeenCalledWith(tr); }); + describe('structural (whole-table) tracked-change bubbles', () => { + const makeEditor = () => { + const editorDispatch = vi.fn(); + const tr = { setMeta: vi.fn() }; + return { + state: {}, + view: { state: { tr }, dispatch: editorDispatch }, + options: { documentId: 'doc-1' }, + }; + }; + + it('creates a structural bubble entry with the public id and display type for a decidable whole-table insert', () => { + const editor = makeEditor(); + const superdoc = { emit: vi.fn(), config: { isInternal: true } }; + + trackChangesHelpersMock.getTrackChanges.mockReturnValue([]); + groupChangesMock.mockReturnValue([]); + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([ + { + id: 'logical-uuid-1', + revisionId: 'rev-1', + side: 'insertion', + subtype: 'table-insert', + tableFrom: 10, + tableTo: 40, + tablePos: 10, + wholeTable: true, + decidable: true, + rows: [{ from: 11, to: 20, pos: 11, node: {} }], + author: 'Alice', + authorEmail: 'alice@example.com', + authorImage: '', + date: '2024-01-01', + importedAuthor: '', + sourceId: '7', + revisionGroupId: 'rg-1', + }, + ]); + + store.commentsList = []; + store.syncTrackedChangeComments({ superdoc, editor }); + + const bubble = store.commentsList.find((c) => c.trackedChange && c.commentId === 'word:structural:7'); + expect(bubble).toBeTruthy(); + expect(bubble.commentId).toBe('word:structural:7'); + expect(bubble.trackedChangeDisplayType).toBe('tableInsert'); + expect(bubble.trackedChangeType).toBe('trackInsert'); + expect(bubble.trackedChangeAnchorKey).toBe('tc::body::word:structural:7'); + expect(bubble.trackedChangeStory).toEqual({ kind: 'story', storyType: 'body' }); + }); + + it('uses table-delete display type and falls back to the logical id when there is no source id', () => { + const editor = makeEditor(); + const superdoc = { emit: vi.fn(), config: { isInternal: true } }; + + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([ + { + id: 'logical-uuid-2', + side: 'deletion', + subtype: 'table-delete', + tableFrom: 5, + tableTo: 30, + tablePos: 5, + wholeTable: true, + decidable: true, + rows: [{ from: 6, to: 15, pos: 6, node: {} }], + author: 'Bob', + authorEmail: 'bob@example.com', + date: '2024-02-02', + sourceId: '', + }, + ]); + + store.commentsList = []; + store.syncTrackedChangeComments({ superdoc, editor }); + + const bubble = store.commentsList.find((c) => c.trackedChange && c.commentId === 'logical-uuid-2'); + expect(bubble).toBeTruthy(); + expect(bubble.trackedChangeDisplayType).toBe('tableDelete'); + expect(bubble.trackedChangeType).toBe('trackDelete'); + }); + + it('does not create a bubble for non-decidable (partial/mixed) structural changes', () => { + const editor = makeEditor(); + const superdoc = { emit: vi.fn(), config: { isInternal: true } }; + + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([ + { + id: 'logical-uuid-3', + side: 'insertion', + subtype: 'table-insert', + tableFrom: 1, + tableTo: 20, + tablePos: 1, + wholeTable: false, + decidable: false, + undecidableReason: 'partial-rows', + rows: [{ from: 2, to: 10, pos: 2, node: {} }], + author: 'Carol', + authorEmail: 'carol@example.com', + date: '2024-03-03', + sourceId: '9', + }, + ]); + + store.commentsList = []; + store.syncTrackedChangeComments({ superdoc, editor }); + + expect(store.commentsList.find((c) => c.trackedChange)).toBeUndefined(); + }); + + it('routes accept/reject through trackChanges.decide with the structural public id', () => { + const decide = vi.fn(() => ({ success: true })); + const superdoc = { + activeEditor: { + doc: { trackChanges: { decide } }, + }, + }; + const comment = { + trackedChange: true, + commentId: 'word:structural:7', + trackedChangeStory: { kind: 'story', storyType: 'body' }, + trackedChangeAnchorKey: 'tc::body::word:structural:7', + }; + + const result = store.decideTrackedChangeFromSidebar({ superdoc, comment, decision: 'accept' }); + + expect(result.ok).toBe(true); + expect(decide).toHaveBeenCalledWith({ + decision: 'accept', + target: { id: 'word:structural:7', story: { kind: 'story', storyType: 'body' } }, + }); + }); + + // The "Added table" structural bubble must appear on initial + // import, not only after a later transaction triggers the full + // syncTrackedChangeComments path. The bootstrap that runs after import + // (bootstrapImportedTrackedChangeComments, fired via setTimeout(0)) must + // create structural bubbles alongside the inline/story ones. + it('creates the structural bubble during the initial import bootstrap so it shows on first render', async () => { + const editorDispatch = vi.fn(); + const tr = { setMeta: vi.fn() }; + const editor = { + state: {}, + view: { state: { tr }, dispatch: editorDispatch }, + options: { documentId: 'doc-1' }, + }; + + // No inline tracked changes; only a decidable whole-table insert. + trackChangesHelpersMock.getTrackChanges.mockReturnValue([]); + groupChangesMock.mockReturnValue([]); + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([ + { + id: 'logical-uuid-init', + side: 'insertion', + subtype: 'table-insert', + tableFrom: 10, + tableTo: 40, + tablePos: 10, + wholeTable: true, + decidable: true, + rows: [{ from: 11, to: 20, pos: 11, node: {} }], + author: 'Alice', + authorEmail: 'alice@example.com', + date: '2024-01-01', + sourceId: '7', + }, + ]); + + // Initial layout emits the structural anchor position before/independent + // of comment creation; the store stores it keyed by anchorKey. + store.editorCommentPositions = { + 'word:structural:7': { start: 10, end: 40, bounds: { top: 0, left: 0 } }, + }; + + store.processLoadedDocxComments({ + superdoc: __mockSuperdoc, + editor, + comments: [], + documentId: 'doc-1', + }); + + // Flush the setTimeout(0) bootstrap that runs after import. + vi.runAllTimers(); + await nextTick(); + + const bubble = store.commentsList.find((c) => c.trackedChange && c.commentId === 'word:structural:7'); + expect(bubble, 'structural bubble created during initial import bootstrap').toBeTruthy(); + expect(bubble.trackedChangeDisplayType).toBe('tableInsert'); + + // It must also survive the getFloatingComments filter (has a resolved + // position) — i.e., it would actually render on first paint. + const floating = store.getFloatingComments; + expect(floating.map((c) => c.commentId)).toContain('word:structural:7'); + }); + }); + + // An inline tracked change whose document range is INSIDE a + // decidable whole-table structural change must NOT become a separate review + // item. Suppression happens at the SOURCE (createCommentForTrackChanges): no + // comment object is created/kept, so there is no floating bubble AND no + // active-on-click dialog (the active-comment path looks the comment up by id; + // with no comment object there is nothing to activate). The underlying + // trackInsert/trackDelete mark is left untouched. Accept/reject of the single + // structural bubble cascades to the cell text via the decision engine + // (range-containment in [tableFrom, tableTo]) — proven in super-editor's + // structural-decisions.test.js for both imported and authored tables. + describe('suppresses inline tracked-change review items inside a tracked whole-table change', () => { + const makeEditor = () => { + const editorDispatch = vi.fn(); + const tr = { setMeta: vi.fn() }; + const state = { doc: {} }; + const editor = { + state, + view: { state: { tr }, dispatch: editorDispatch }, + options: { documentId: 'doc-1' }, + }; + // Wire the mock document so the floating filter resolves the same state. + __mockSuperdoc.documents.value = [{ id: 'doc-1', type: 'docx', getEditor: () => editor }]; + return editor; + }; + + // Shape an inline tracked-change mark + its grouped projection. + const inlineInsert = ({ id, from, to }) => { + const mark = { type: { name: 'trackInsert' }, attrs: { id } }; + const raw = { mark, node: {}, from, to }; + return { raw, grouped: { from, to, insertedMark: raw } }; + }; + + it('creates ONLY the structural bubble — no separate item for cell text inside a tracked inserted table', () => { + const editor = makeEditor(); + const superdoc = { emit: vi.fn(), config: { isInternal: true } }; + + // Authored (native) table: NO sourceId → publicId falls back to table:pos:side. + const structural = { + id: 'table:10:insertion', + side: 'insertion', + subtype: 'table-insert', + tableFrom: 10, + tableTo: 40, + tablePos: 10, + wholeTable: true, + decidable: true, + rows: [{ from: 11, to: 38, pos: 11, node: {} }], + author: 'Alice', + authorEmail: 'alice@example.com', + date: '2024-01-01', + sourceId: '', + }; + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([structural]); + + // Inline typed char inside a cell, range [20, 21] ⊂ [10, 40]. + const cellChar = inlineInsert({ id: 'inline-cell-1', from: 20, to: 21 }); + trackChangesHelpersMock.getTrackChanges.mockReturnValue([cellChar.raw]); + groupChangesMock.mockReturnValue([cellChar.grouped]); + + store.commentsList = []; + store.syncTrackedChangeComments({ superdoc, editor }); + + const trackedItems = store.commentsList.filter((c) => c.trackedChange); + // Exactly ONE tracked-change review item: the structural table bubble. + expect(trackedItems.map((c) => c.commentId)).toEqual(['table:10:insertion']); + expect(trackedItems[0].trackedChangeDisplayType).toBe('tableInsert'); + // No separate item for the inline cell char (suppressed at the source). + expect(store.commentsList.find((c) => c.commentId === 'inline-cell-1')).toBeUndefined(); + + // The floating list (belt-and-suspenders) also shows only the structural one. + store.editorCommentPositions = { + 'table:10:insertion': { start: 10, end: 40, bounds: { top: 0, left: 0 } }, + }; + expect(store.getFloatingComments.map((c) => c.commentId)).toEqual(['table:10:insertion']); + }); + + it('prunes an inline comment created on a prior sync once its table becomes/stays tracked (idempotent)', () => { + const editor = makeEditor(); + const superdoc = { emit: vi.fn(), config: { isInternal: true } }; + + // Seed an inline tracked-change comment as if a prior sync (before the + // whole-table change existed) had created it. + store.commentsList = [ + { + commentId: 'inline-cell-1', + fileId: 'doc-1', + documentId: 'doc-1', + trackedChange: true, + trackedChangeStory: { kind: 'story', storyType: 'body' }, + trackedChangeAnchorKey: 'tc::body::inline-cell-1', + resolvedTime: null, + createdTime: 1, + selection: { source: 'super-editor' }, + }, + ]; + + const structural = { + id: 'table:10:insertion', + side: 'insertion', + subtype: 'table-insert', + tableFrom: 10, + tableTo: 40, + tablePos: 10, + wholeTable: true, + decidable: true, + rows: [{ from: 11, to: 38, pos: 11, node: {} }], + author: 'Alice', + authorEmail: 'alice@example.com', + date: '2024-01-01', + sourceId: '', + }; + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([structural]); + const cellChar = inlineInsert({ id: 'inline-cell-1', from: 20, to: 21 }); + trackChangesHelpersMock.getTrackChanges.mockReturnValue([cellChar.raw]); + groupChangesMock.mockReturnValue([cellChar.grouped]); + + store.syncTrackedChangeComments({ superdoc, editor }); + + // Inline comment pruned; only the structural bubble remains. + expect(store.commentsList.find((c) => c.commentId === 'inline-cell-1')).toBeUndefined(); + expect(store.commentsList.filter((c) => c.trackedChange).map((c) => c.commentId)).toEqual(['table:10:insertion']); + + // Re-running the sync is a no-op (idempotent) — still exactly one item. + store.syncTrackedChangeComments({ superdoc, editor }); + expect(store.commentsList.filter((c) => c.trackedChange).map((c) => c.commentId)).toEqual(['table:10:insertion']); + }); + + it('clears the active comment if it pointed at a now-suppressed inline change', () => { + const editor = makeEditor(); + const superdoc = { emit: vi.fn(), config: { isInternal: true } }; + + store.commentsList = [ + { + commentId: 'inline-cell-1', + fileId: 'doc-1', + documentId: 'doc-1', + trackedChange: true, + trackedChangeStory: { kind: 'story', storyType: 'body' }, + trackedChangeAnchorKey: 'tc::body::inline-cell-1', + resolvedTime: null, + createdTime: 1, + selection: { source: 'super-editor' }, + }, + ]; + store.activeComment = 'inline-cell-1'; + + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([ + { + id: 'table:10:insertion', + side: 'insertion', + subtype: 'table-insert', + tableFrom: 10, + tableTo: 40, + tablePos: 10, + wholeTable: true, + decidable: true, + rows: [{ from: 11, to: 38, pos: 11, node: {} }], + author: 'Alice', + authorEmail: 'alice@example.com', + date: '2024-01-01', + sourceId: '', + }, + ]); + const cellChar = inlineInsert({ id: 'inline-cell-1', from: 20, to: 21 }); + trackChangesHelpersMock.getTrackChanges.mockReturnValue([cellChar.raw]); + groupChangesMock.mockReturnValue([cellChar.grouped]); + + store.syncTrackedChangeComments({ superdoc, editor }); + + expect(store.activeComment).toBeNull(); + }); + + it('NEGATIVE: inline tracked change OUTSIDE any table still gets its own review item', () => { + const editor = makeEditor(); + const superdoc = { emit: vi.fn(), config: { isInternal: true } }; + + // A tracked inserted table at [10,40]; the inline change is at [50,55] (outside). + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([ + { + id: 'table:10:insertion', + side: 'insertion', + subtype: 'table-insert', + tableFrom: 10, + tableTo: 40, + tablePos: 10, + wholeTable: true, + decidable: true, + rows: [{ from: 11, to: 38, pos: 11, node: {} }], + author: 'Alice', + authorEmail: 'alice@example.com', + date: '2024-01-01', + sourceId: '', + }, + ]); + const outside = inlineInsert({ id: 'inline-outside', from: 50, to: 55 }); + trackChangesHelpersMock.getTrackChanges.mockReturnValue([outside.raw]); + groupChangesMock.mockReturnValue([outside.grouped]); + + store.commentsList = []; + store.syncTrackedChangeComments({ superdoc, editor }); + + expect(store.commentsList.find((c) => c.commentId === 'inline-outside')).toBeTruthy(); + }); + + it('NEGATIVE: a tracked text edit inside a NON-tracked table still gets its own review item', () => { + const editor = makeEditor(); + const superdoc = { emit: vi.fn(), config: { isInternal: true } }; + + // The table is NOT tracked → no decidable whole-table change → no suppression. + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([]); + const inCell = inlineInsert({ id: 'inline-plain-table', from: 20, to: 25 }); + trackChangesHelpersMock.getTrackChanges.mockReturnValue([inCell.raw]); + groupChangesMock.mockReturnValue([inCell.grouped]); + + store.commentsList = []; + store.syncTrackedChangeComments({ superdoc, editor }); + + expect(store.commentsList.find((c) => c.commentId === 'inline-plain-table')).toBeTruthy(); + }); + + it('NEGATIVE: a real user comment inside a tracked table is NOT suppressed', () => { + const editor = makeEditor(); + const superdoc = { emit: vi.fn(), config: { isInternal: true } }; + + // Seed a real user comment whose range falls inside the tracked table. + store.commentsList = [ + { + commentId: 'user-comment-1', + fileId: 'doc-1', + documentId: 'doc-1', + trackedChange: false, + resolvedTime: null, + createdTime: 1, + selection: { source: 'super-editor' }, + }, + ]; + + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([ + { + id: 'table:10:insertion', + side: 'insertion', + subtype: 'table-insert', + tableFrom: 10, + tableTo: 40, + tablePos: 10, + wholeTable: true, + decidable: true, + rows: [{ from: 11, to: 38, pos: 11, node: {} }], + author: 'Alice', + authorEmail: 'alice@example.com', + date: '2024-01-01', + sourceId: '', + }, + ]); + // No inline tracked changes. + trackChangesHelpersMock.getTrackChanges.mockReturnValue([]); + groupChangesMock.mockReturnValue([]); + + store.syncTrackedChangeComments({ superdoc, editor }); + + expect(store.commentsList.find((c) => c.commentId === 'user-comment-1')).toBeTruthy(); + store.editorCommentPositions = { + 'user-comment-1': { start: 20, end: 22, bounds: { top: 0, left: 0 } }, + 'table:10:insertion': { start: 10, end: 40, bounds: { top: 0, left: 0 } }, + }; + // Real user comment still floats (only inline TRACKED changes are coalesced). + expect(store.getFloatingComments.map((c) => c.commentId).sort()).toEqual([ + 'table:10:insertion', + 'user-comment-1', + ]); + }); + }); + it('emits deleted events when replay sync prunes stale tracked-change comments', () => { const editorDispatch = vi.fn(); const tr = { setMeta: vi.fn() }; @@ -3507,4 +3979,105 @@ describe('comments-store', () => { expect(floating.map((c) => c.commentId)).toEqual(['pdf-1']); }); }); + + describe('getFloatingComments coalesces a tracked whole-table change', () => { + // Wire the mock document to expose an editor state so the store can + // enumerate tracked-table ranges for its comments. + const wireEditorWithTable = () => { + const state = { doc: {} }; + __mockSuperdoc.documents.value = [{ id: 'doc-1', type: 'docx', getEditor: () => ({ state }) }]; + return state; + }; + + it('suppresses an inline tracked-change bubble inside a tracked inserted table (only the structural bubble remains)', () => { + wireEditorWithTable(); + // Tracked whole-table insert covering [10, 30]. + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([ + { decidable: true, wholeTable: true, tableFrom: 10, tableTo: 30, subtype: 'table-insert', side: 'insertion' }, + ]); + + store.commentsList = [ + // Structural "Added table" bubble — must stay. + { + commentId: 'word:structural:2', + fileId: 'doc-1', + trackedChange: true, + trackedChangeDisplayType: 'tableInsert', + resolvedTime: null, + createdTime: 1, + }, + // Inline TC bubble inside the table [15, 20] — must be suppressed. + { + commentId: 'inline-ins-1', + fileId: 'doc-1', + trackedChange: true, + resolvedTime: null, + createdTime: 2, + }, + ]; + store.editorCommentPositions = { + 'word:structural:2': { start: 10, end: 30, bounds: { top: 0, left: 0 } }, + 'inline-ins-1': { start: 15, end: 20, bounds: { top: 0, left: 0 } }, + }; + + const floating = store.getFloatingComments; + expect(floating.map((c) => c.commentId)).toEqual(['word:structural:2']); + }); + + it('keeps an inline tracked-change bubble inside a NON-tracked table (table itself not tracked)', () => { + wireEditorWithTable(); + // No decidable whole-table change → no suppression ranges. + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([]); + + store.commentsList = [ + { + commentId: 'inline-in-plain-table', + fileId: 'doc-1', + trackedChange: true, + resolvedTime: null, + createdTime: 1, + }, + ]; + store.editorCommentPositions = { + 'inline-in-plain-table': { start: 15, end: 20, bounds: { top: 0, left: 0 } }, + }; + + const floating = store.getFloatingComments; + expect(floating.map((c) => c.commentId)).toEqual(['inline-in-plain-table']); + }); + + it('does not suppress real user comments inside a tracked table, nor inline TC outside it', () => { + wireEditorWithTable(); + trackChangesHelpersMock.enumerateStructuralRowChanges.mockReturnValue([ + { decidable: true, wholeTable: true, tableFrom: 10, tableTo: 30, subtype: 'table-insert', side: 'insertion' }, + ]); + + store.commentsList = [ + // Real user comment inside the table range — must stay (not a TC). + { + commentId: 'user-comment', + fileId: 'doc-1', + trackedChange: false, + resolvedTime: null, + createdTime: 1, + selection: { source: 'super-editor' }, + }, + // Inline TC OUTSIDE the table [40, 45] — must stay. + { + commentId: 'inline-outside', + fileId: 'doc-1', + trackedChange: true, + resolvedTime: null, + createdTime: 2, + }, + ]; + store.editorCommentPositions = { + 'user-comment': { start: 16, end: 18, bounds: { top: 0, left: 0 } }, + 'inline-outside': { start: 40, end: 45, bounds: { top: 0, left: 0 } }, + }; + + const floating = store.getFloatingComments; + expect(floating.map((c) => c.commentId).sort()).toEqual(['inline-outside', 'user-comment']); + }); + }); }); diff --git a/packages/word-layout/package.json b/packages/word-layout/package.json index 00ba5347c7..3e33d75442 100644 --- a/packages/word-layout/package.json +++ b/packages/word-layout/package.json @@ -8,9 +8,9 @@ "types": "./dist/index.d.ts", "exports": { ".": { - "types": "./dist/index.d.ts", + "types": "./src/index.ts", "source": "./src/index.ts", - "default": "./dist/index.js" + "default": "./src/index.ts" } }, "scripts": {