fix(track-changes): address PR review on block-level structural tracked changes

- Move painter-targeting CSS into BLOCK_TRACK_CHANGE_STYLES in the painter's
  styles module so document visuals live behind the painter boundary; keep
  contenteditable-side rules scoped under .sd-editor-scoped .ProseMirror tr.
- Drop stale isBlockLevel entries from previous state before merging in
  freshly-computed block entries, so accept/reject clears the bubble.
- renderDOM now emits data-track-change-id and data-track-change-operation
  alongside data-track-change so HTML round-trips preserve enough state for
  getBlockTrackedChanges and acceptTrackedChangeById to resolve the row.

Adds regression tests: stale-entry pruning in comments-plugin apply(),
renderDOM/parseDOM round-trip in blockTrackedChangeAttr, and a styles.ts
guard that fails if a bare [data-track-change=...] selector regresses.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: shri-scale

packages/layout-engine/painters/dom/src/styles.test.ts

@@ -70,4 +70,19 @@ describe('ensureTrackChangeStyles', () => {
     );
     expect(cssText).not.toMatch(/track-format-dec\.highlighted\.track-change-focused\s*\{[\s\S]*border-bottom-width:/);
   });
+
+  it('block-level tracked-change rules are scoped to .superdoc-layout, not bare selectors', () => {
+    ensureTrackChangeStyles(document);
+
+    const styleEl = document.querySelector('[data-superdoc-track-change-styles="true"]');
+    const cssText = styleEl?.textContent ?? '';
+
+    // Scoped selectors present
+    expect(cssText).toContain(".superdoc-layout [data-track-change='delete']");
+    expect(cssText).toContain(".superdoc-layout [data-track-change='insert']");
+
+    // No bare `[data-track-change=...] {` selector — would leak to PM mirror
+    // and any host page that uses the same attribute name.
+    expect(cssText).not.toMatch(/^\s*\[data-track-change=/m);
+  });
 });

packages/layout-engine/painters/dom/src/styles.ts

@@ -313,6 +313,34 @@ const TRACK_CHANGE_STYLES = `
 .superdoc-layout .track-format-dec.highlighted.track-change-focused {
   background-color: var(--sd-tracked-changes-format-background-focused, #ffd70033);
 }
+
+/*
+ * Block-level tracked changes (row granularity for v1).
+ *
+ * The DomPainter stamps data-track-change on each cell <div> of a tracked
+ * row (no <tr> exists in layout mode; cells are absolutely positioned).
+ * Scoping to .superdoc-layout keeps these rules out of the contenteditable
+ * mirror; the editor stylesheet handles the PM <tr> path separately.
+ */
+.superdoc-layout [data-track-change='delete'] {
+  background-color: var(--sd-block-tracked-deleted-bg, rgba(203, 14, 71, 0.18));
+  outline:
+    var(--sd-block-tracked-deleted-outline-width, 2px)
+    dashed
+    var(--sd-block-tracked-deleted-outline, #cb0e47);
+  outline-offset: var(--sd-block-tracked-deleted-outline-offset, -1px);
+  text-decoration: line-through;
+  text-decoration-color: var(--sd-block-tracked-deleted-outline, #cb0e47);
+}
+
+.superdoc-layout [data-track-change='insert'] {
+  background-color: var(--sd-block-tracked-inserted-bg, rgba(57, 156, 114, 0.18));
+  outline:
+    var(--sd-block-tracked-inserted-outline-width, 2px)
+    dashed
+    var(--sd-block-tracked-inserted-outline, #00853d);
+  outline-offset: var(--sd-block-tracked-inserted-outline-offset, -1px);
+}
 `;
 
 const FORMATTING_MARKS_STYLES = `

packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css

@@ -286,35 +286,30 @@ https://github.com/ProseMirror/prosemirror-tables/blob/master/demo/index.html
 /* Track changes - end */
 
 /*
- * Block-level tracked changes (row granularity for v1).
+ * Block-level tracked changes (row granularity for v1) — contenteditable path.
  *
- * The `data-track-change` attribute is stamped in two places:
- *   1. ProseMirror's contenteditable <tr> via the schema's renderDOM
- *      (hidden in layout-engine mode but kept consistent).
- *   2. The DomPainter's per-cell <div> elements (no <tr> exists in layout
- *      mode; cells are absolutely positioned). Every cell of a tracked row
- *      carries the attribute → a continuous red/green strip across the row.
- *
- * Selectors are intentionally minimal so the rule applies wherever the painter
- * places the cell — across re-renders and table-fragment boundaries.
+ * The data-track-change attribute is stamped on PM's <tr> via the schema's
+ * renderDOM. The painter-side stamping (per-cell <div> in layout mode) is
+ * styled by BLOCK_TRACK_CHANGE_STYLES in
+ * packages/layout-engine/painters/dom/src/styles.ts to keep document visuals
+ * inside the painter's stylesheet boundary.
  */
-[data-track-change='delete'] {
-  background-color: var(--sd-block-tracked-deleted-bg, rgba(203, 14, 71, 0.18)) !important;
-  outline: var(--sd-block-tracked-deleted-outline-width, 2px) dashed var(--sd-block-tracked-deleted-outline, #cb0e47) !important;
-  outline-offset: var(--sd-block-tracked-deleted-outline-offset, -1px) !important;
+.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] {
+  background-color: var(--sd-block-tracked-deleted-bg, rgba(203, 14, 71, 0.18));
+  outline: var(--sd-block-tracked-deleted-outline-width, 2px) dashed var(--sd-block-tracked-deleted-outline, #cb0e47);
+  outline-offset: var(--sd-block-tracked-deleted-outline-offset, -1px);
 }
 
 .sd-editor-scoped .ProseMirror tr[data-track-change='delete'] td,
-.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] th,
-.superdoc-table-fragment [data-track-change='delete'] {
+.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] th {
   text-decoration: line-through;
   text-decoration-color: var(--sd-block-tracked-deleted-outline, #cb0e47);
 }
 
-[data-track-change='insert'] {
-  background-color: var(--sd-block-tracked-inserted-bg, rgba(57, 156, 114, 0.18)) !important;
-  outline: var(--sd-block-tracked-inserted-outline-width, 2px) dashed var(--sd-block-tracked-inserted-outline, #00853d) !important;
-  outline-offset: var(--sd-block-tracked-inserted-outline-offset, -1px) !important;
+.sd-editor-scoped .ProseMirror tr[data-track-change='insert'] {
+  background-color: var(--sd-block-tracked-inserted-bg, rgba(57, 156, 114, 0.18));
+  outline: var(--sd-block-tracked-inserted-outline-width, 2px) dashed var(--sd-block-tracked-inserted-outline, #00853d);
+  outline-offset: var(--sd-block-tracked-inserted-outline-offset, -1px);
 }
 /* Block-level tracked changes - end */
 

packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js

@@ -50,4 +50,31 @@ describe('comments-plugin — block-level tracked-change registration', () => {
     expect(tracked['OP1']).toBeUndefined();
     expect(tracked['r1']).toBeUndefined();
   });
+
+  it('drops a block-level entry once its row is no longer tracked (resolved op)', () => {
+    const table = schema.nodes.table.create({ sdBlockId: 't1' }, [makeRow('delete', 'r1', 'OP1')]);
+    installDoc([table]);
+    expect(CommentsPluginKey.getState(editor.state).trackedChanges['OP1']).toBeDefined();
+
+    // Walk the doc to find the tracked row, then clear its trackChange attr
+    // through a transaction. This exercises the apply() reducer's stale-entry
+    // pruning rather than recomputing state from init().
+    let rowPos = null;
+    let rowNode = null;
+    editor.state.doc.descendants((node, pos) => {
+      if (node.type.name === 'tableRow' && node.attrs.trackChange) {
+        rowPos = pos;
+        rowNode = node;
+        return false;
+      }
+      return true;
+    });
+    expect(rowPos).not.toBeNull();
+
+    const tr = editor.state.tr.setNodeMarkup(rowPos, undefined, { ...rowNode.attrs, trackChange: null });
+    editor.view.dispatch(tr);
+
+    const pluginState = CommentsPluginKey.getState(editor.state);
+    expect(pluginState.trackedChanges['OP1']).toBeUndefined();
+  });
 });

packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js

@@ -555,10 +555,16 @@ export const CommentsPlugin = Extension.create({
                 isBlockLevel: true,
               };
             }
-            // Merge: keep existing inline entries, add block-level entries.
-            // Block-level keys (operationId / row-id) shouldn't collide with
-            // inline ones (mark-id), but if they ever do, inline wins.
-            pluginState.trackedChanges = { ...blockTracked, ...pluginState.trackedChanges };
+            // Drop stale block-level entries from the previous state first.
+            // getBlockTrackedChanges is the source of truth for block-level
+            // tracking, so anything no longer in the doc (accepted/rejected)
+            // must not leak back via the merge. Inline entries are owned by
+            // handleTrackedChangeTransaction and are kept as-is.
+            const previousInline = {};
+            for (const [k, v] of Object.entries(pluginState.trackedChanges || {})) {
+              if (!v?.isBlockLevel) previousInline[k] = v;
+            }
+            pluginState.trackedChanges = { ...previousInline, ...blockTracked };
           }
 
           // Check for changes in the actively selected comment

packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js

@@ -20,7 +20,14 @@ export const blockTrackedChangeAttrSpec = {
     },
     renderDOM: (attrs) => {
       const tc = attrs?.trackChange;
-      return tc ? { 'data-track-change': tc.kind } : {};
+      if (!tc) return {};
+      const out = { 'data-track-change': tc.kind };
+      // Emit id + operationId so HTML round-trips (clipboard, getHTML/setContent,
+      // collaboration patches) preserve enough to resolve the change via
+      // getBlockTrackedChanges and accept/reject it.
+      if (tc.id) out['data-track-change-id'] = tc.id;
+      if (tc.operationId) out['data-track-change-operation'] = tc.operationId;
+      return out;
     },
   },
 };

packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js

@@ -26,13 +26,36 @@ describe('blockTrackedChangeAttrSpec', () => {
     expect(blockTrackedChangeAttrSpec.trackChange.parseDOM(el)).toBeNull();
   });
 
-  it('renderDOM emits data-track-change when set; nothing when null', () => {
+  it('renderDOM emits kind, id, and operationId when present', () => {
     expect(
       blockTrackedChangeAttrSpec.trackChange.renderDOM({
         trackChange: { kind: 'insert', id: 'r1', operationId: 'op1' },
       }),
-    ).toEqual({ 'data-track-change': 'insert' });
+    ).toEqual({
+      'data-track-change': 'insert',
+      'data-track-change-id': 'r1',
+      'data-track-change-operation': 'op1',
+    });
+  });
+
+  it('renderDOM omits optional id / operationId when missing', () => {
+    expect(
+      blockTrackedChangeAttrSpec.trackChange.renderDOM({
+        trackChange: { kind: 'delete' },
+      }),
+    ).toEqual({ 'data-track-change': 'delete' });
+  });
+
+  it('renderDOM emits nothing when trackChange is null or absent', () => {
     expect(blockTrackedChangeAttrSpec.trackChange.renderDOM({ trackChange: null })).toEqual({});
     expect(blockTrackedChangeAttrSpec.trackChange.renderDOM({})).toEqual({});
   });
+
+  it('renderDOM and parseDOM round-trip preserve id and operationId', () => {
+    const original = { kind: 'delete', id: 'row-42', operationId: 'op-99' };
+    const rendered = blockTrackedChangeAttrSpec.trackChange.renderDOM({ trackChange: original });
+    const el = document.createElement('tr');
+    for (const [k, v] of Object.entries(rendered)) el.setAttribute(k, v);
+    expect(blockTrackedChangeAttrSpec.trackChange.parseDOM(el)).toEqual(original);
+  });
 });