diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes-overlap.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes-overlap.test.js
index a680c5dccf..1215d3c463 100644
--- a/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes-overlap.test.js
+++ b/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes-overlap.test.js
@@ -19,6 +19,7 @@ import { beforeEach, afterEach, describe, expect, it } from 'vitest';
import { TextSelection } from 'prosemirror-state';
import { TrackInsertMarkName, TrackDeleteMarkName } from './constants.js';
import { initTestEditor } from '@tests/helpers/helpers.js';
+import { handleBackspace, handleDelete } from '@core/extensions/keymap.js';
import { buildReviewGraph, CanonicalChangeType } from './review-model/review-graph.js';
const ALICE = { name: 'Alice', email: 'alice@example.com' };
@@ -215,3 +216,149 @@ describe('overlap wired: insertTrackedChange delegates to compiler', () => {
expect(ok).toBe(false);
});
});
+
+describe('overlap wired: deletion spanning a paragraph boundary (SD-3386)', () => {
+ let editor;
+ afterEach(() => {
+ editor?.destroy();
+ editor = null;
+ });
+
+ const TWO_PARAGRAPHS = '
First line of text
Second line of text
';
+
+ // Selection from the start of paragraph 1 (pos 1) through the middle of
+ // paragraph 2 ("Second|" → 21 + 6 = 27). deleteSelection routes through
+ // deleteRange, which emits a ReplaceStep whose slice is a structural
+ // paragraph re-join shell with no inline content.
+ const deleteAcrossBoundary = () => {
+ editor.commands.command(({ tr, dispatch }) => {
+ tr.setSelection(TextSelection.create(tr.doc, 1, 27));
+ tr.deleteSelection();
+ if (dispatch) dispatch(tr);
+ return true;
+ });
+ };
+
+ const paragraphTexts = (doc) => {
+ const texts = [];
+ doc.forEach((node) => texts.push(node.textContent));
+ return texts;
+ };
+
+ it('marks the range deleted without splitting the trailing paragraph', () => {
+ editor = setup(ALICE, TWO_PARAGRAPHS);
+ deleteAcrossBoundary();
+
+ // No spurious paragraph split: still exactly two paragraphs with the
+ // original text (the deletion is only marked, not applied).
+ expect(editor.state.doc.childCount).toBe(2);
+ expect(paragraphTexts(editor.state.doc)).toEqual(['First line of text', 'Second line of text']);
+
+ // One logical deletion — not a replacement wrapping an empty block shell.
+ const graph = graphFor(editor);
+ expect(graph.changes.size).toBe(1);
+ const change = Array.from(graph.changes.values())[0];
+ expect(change.type).toBe(CanonicalChangeType.Deletion);
+ });
+
+ it('accept removes the marked content and all tracked marks', () => {
+ editor = setup(ALICE, TWO_PARAGRAPHS);
+ deleteAcrossBoundary();
+
+ const ok = editor.commands.acceptTrackedChangesBetween(0, editor.state.doc.content.size);
+ expect(ok).toBe(true);
+ expect(editor.storage.trackChanges?.lastDecisionFailure ?? null).toBeNull();
+
+ // Deleted content is gone, remainder of paragraph 2 survives.
+ expect(paragraphTexts(editor.state.doc)).toEqual(['', 'ond line of text']);
+
+ // No tracked-change state (marks/decorations) survives the decision.
+ const graph = graphFor(editor);
+ expect(graph.changes.size).toBe(0);
+ });
+
+ it('reject restores the content and removes all tracked marks', () => {
+ editor = setup(ALICE, TWO_PARAGRAPHS);
+ deleteAcrossBoundary();
+
+ const graph = graphFor(editor);
+ const changeId = Array.from(graph.changes.keys())[0];
+ const ok = editor.commands.rejectTrackedChangeById(changeId);
+ expect(ok).toBe(true);
+ expect(editor.storage.trackChanges?.lastDecisionFailure ?? null).toBeNull();
+
+ // Original two-paragraph content is intact and unmarked.
+ expect(editor.state.doc.childCount).toBe(2);
+ expect(paragraphTexts(editor.state.doc)).toEqual(['First line of text', 'Second line of text']);
+ expect(graphFor(editor).changes.size).toBe(0);
+
+ let hasTrackedMarks = false;
+ editor.state.doc.descendants((node) => {
+ if (node.marks.some((m) => m.type.name === TrackInsertMarkName || m.type.name === TrackDeleteMarkName)) {
+ hasTrackedMarks = true;
+ }
+ });
+ expect(hasTrackedMarks).toBe(false);
+ });
+});
+
+describe('overlap wired: cross-paragraph deletion through the real keymap path (SD-3386)', () => {
+ let editor;
+ afterEach(() => {
+ editor?.destroy();
+ editor = null;
+ });
+
+ // The keymap path differs from a raw command dispatch: handleBackspace tags
+ // the transaction with inputType 'deleteContentBackward' and deleteSelection
+ // sets the post-step selection, which can land inside the structural shell
+ // slice that the tracked compile never inserts. Mapping that position back
+ // through a falsely-mirrored invert map produced `Position NaN` and the
+ // dispatch fallback then applied the deletion untracked.
+ const setupTwoLines = () => {
+ ({ editor } = initTestEditor({
+ mode: 'text',
+ content: 'Line 1
Line 2
',
+ user: ALICE,
+ trackedChanges: {},
+ }));
+ editor.commands.enableTrackChanges();
+
+ let p2TextStart = null;
+ editor.state.doc.descendants((node, pos) => {
+ if (node.isText && node.text === 'Line 2') p2TextStart = pos;
+ });
+ // User selection from the start of "Line 1" through "Lin" of line 2,
+ // dispatched on its own like a mouse selection.
+ editor.commands.command(({ tr, dispatch }) => {
+ tr.setSelection(TextSelection.create(tr.doc, 1, p2TextStart + 3));
+ if (dispatch) dispatch(tr);
+ return true;
+ });
+ };
+
+ const paragraphTexts = (doc) => {
+ const texts = [];
+ doc.forEach((node) => texts.push(node.textContent));
+ return texts;
+ };
+
+ it.each([
+ ['Backspace', handleBackspace],
+ ['Delete', handleDelete],
+ ])('%s tracks the deletion instead of hard-deleting', (_label, handler) => {
+ setupTwoLines();
+ const handled = handler(editor);
+ expect(handled).toBe(true);
+
+ // Content survives as a tracked deletion — never hard-deleted.
+ expect(paragraphTexts(editor.state.doc)).toEqual(['Line 1', 'Line 2']);
+ const graph = graphFor(editor);
+ expect(graph.changes.size).toBe(1);
+ expect(Array.from(graph.changes.values())[0].type).toBe(CanonicalChangeType.Deletion);
+
+ // Caret lands at the left edge of the tracked deletion, inside paragraph 1.
+ expect(editor.state.selection.empty).toBe(true);
+ expect(editor.state.selection.from).toBeLessThan(editor.state.doc.firstChild.nodeSize);
+ });
+});
diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes.js b/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes.js
index e839e0b236..10738b5adb 100644
--- a/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes.js
+++ b/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes.js
@@ -96,7 +96,9 @@ const dispatchReviewDecision = ({ editor, state, dispatch, decision, target }) =
originalId: event.changeId,
}).some(({ mark }) => mark.attrs?.splitFromId === event.changeId);
if (successorsPresent) continue;
- editor.emit('commentsUpdate', event);
+ // Carry the decision so resolved bubbles can report accepted vs
+ // rejected instead of assuming every resolution was an accept.
+ editor.emit('commentsUpdate', { ...event, decision });
}
const touched =
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 a963cd7462..3da0e5ffbf 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
@@ -171,6 +171,27 @@ const normalizeReplaceStepSingleCharDelete = ({ step, doc }) => {
}
};
+/**
+ * Whether a slice carries any inline leaf content (text, hard breaks, inline
+ * widgets). A slice without inline leaves is a purely structural shell — e.g.
+ * the paragraph re-join slice `deleteRange` produces for a deletion that spans
+ * a paragraph boundary (`` with open ends).
+ *
+ * @param {import('prosemirror-model').Slice} slice
+ * @returns {boolean}
+ */
+const sliceHasInlineLeafContent = (slice) => {
+ let found = false;
+ slice.content.descendants((node) => {
+ if (found) return false;
+ if (node.isInline && (node.isText || node.isLeaf)) {
+ found = true;
+ return false;
+ }
+ });
+ return found;
+};
+
/**
* Replace step.
* @param {object} options Replace step options.
@@ -555,6 +576,14 @@ const tryCompileStep = ({
if (!hasInlineContent) return { handled: false };
}
+ // A non-empty slice without inline leaf content is a structural re-join
+ // shell, not user-visible replacement content. `deleteRange` emits one for
+ // a deletion that spans a paragraph boundary; compiling it as a replacement
+ // would insert the empty block shell at the range end and split the
+ // trailing paragraph (SD-3386). Track it as a plain deletion instead.
+ const isStructuralShellDelete =
+ step.from !== step.to && step.slice.content.size > 0 && !sliceHasInlineLeafContent(step.slice);
+
// Build the intent. Pure inserts and pure deletes use the matching intent
// type; mixed (text-replace) carries the original slice.
let intent;
@@ -570,7 +599,7 @@ const tryCompileStep = ({
source,
preserveExistingReviewState,
});
- } else if (step.from !== step.to && step.slice.content.size === 0) {
+ } else if (step.from !== step.to && (step.slice.content.size === 0 || isStructuralShellDelete)) {
intent = makeTextDeleteIntent({
from: step.from,
to: step.to,
@@ -641,7 +670,17 @@ const tryCompileStep = ({
map.appendMap(invertStep.getMap());
const mirrorIndex = map.maps.length - 1;
for (let i = beforeSteps; i < newTr.steps.length; i += 1) {
- map.appendMap(newTr.steps[i].getMap(), mirrorIndex);
+ // Mirror pairing assumes the compiled steps re-apply the original
+ // step's position effect (the condensed insert in the replace path).
+ // A structural-shell delete compiles to mark steps only — pairing
+ // their empty maps as mirrors of the invert map corrupts position
+ // recovery (Position NaN) for positions inside the never-inserted
+ // shell, so append those without mirrors (SD-3386).
+ if (isStructuralShellDelete) {
+ map.appendMap(newTr.steps[i].getMap());
+ } else {
+ map.appendMap(newTr.steps[i].getMap(), mirrorIndex);
+ }
}
} else {
for (let i = beforeSteps; i < newTr.steps.length; i += 1) {
@@ -676,7 +715,11 @@ const tryCompileStep = ({
// fall back to a shaped step the comments plugin already understands.
meta.step = { slice: { content: { content: result.insertedNodes } } };
}
- if (result.selection?.kind === 'near' && stepWasNormalized && !result.insertedMark) {
+ // Structural-shell deletes also need the explicit override: the original
+ // step's post-step selection can sit inside the shell slice that was never
+ // inserted, so mapping it back is meaningless — honor the compiler's caret
+ // hint (left edge of the tracked deletion) instead.
+ if (result.selection?.kind === 'near' && (stepWasNormalized || isStructuralShellDelete) && !result.insertedMark) {
meta.selectionPos = result.selection.pos;
}
newTr.setMeta(TrackChangesBasePluginKey, meta);
diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js
index 5574d352f1..ba5f71651e 100644
--- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js
+++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js
@@ -767,6 +767,7 @@ describe('CommentDialog.vue', () => {
email: superdocStoreStub.user.email,
name: superdocStoreStub.user.name,
superdoc: expect.any(Object),
+ decision: 'accept',
});
expect(superdocStub.focus).toHaveBeenCalledTimes(1);
@@ -778,6 +779,50 @@ describe('CommentDialog.vue', () => {
expect(superdocStub.focus).toHaveBeenCalledTimes(2);
});
+ it('does not resolve the tracked-change thread when the decision fails (SD-3386)', async () => {
+ const { wrapper, baseComment } = await mountDialog({
+ baseCommentOverrides: {
+ trackedChange: true,
+ trackedChangeType: 'trackDelete',
+ trackedChangeText: 'Removed',
+ },
+ });
+ commentsStoreStub.decideTrackedChangeFromSidebar.mockReturnValueOnce({ ok: true, success: false });
+
+ const header = wrapper.findComponent(CommentHeaderStub);
+ header.vm.$emit('reject');
+ await nextTick();
+ expect(baseComment.resolveComment).not.toHaveBeenCalled();
+ });
+
+ it('labels a rejected tracked change as Rejected, not Accepted (SD-3386)', async () => {
+ const { wrapper } = await mountDialog({
+ baseCommentOverrides: {
+ trackedChange: true,
+ trackedChangeType: 'trackDelete',
+ trackedChangeText: 'Removed',
+ resolvedTime: Date.now(),
+ trackedChangeDecision: 'reject',
+ },
+ });
+
+ expect(wrapper.find('.resolved-badge').text()).toContain('Rejected');
+ });
+
+ it('labels an accepted tracked change as Accepted', async () => {
+ const { wrapper } = await mountDialog({
+ baseCommentOverrides: {
+ trackedChange: true,
+ trackedChangeType: 'trackInsert',
+ trackedChangeText: 'Added',
+ resolvedTime: Date.now(),
+ trackedChangeDecision: 'accept',
+ },
+ });
+
+ expect(wrapper.find('.resolved-badge').text()).toContain('Accepted');
+ });
+
it('renders hyperlink additions without a format label', async () => {
const { wrapper } = await mountDialog({
baseCommentOverrides: {
diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue
index 7c4d0539a7..9e8e5e5fa3 100644
--- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue
+++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue
@@ -279,7 +279,8 @@ const isDialogActive = computed(() => {
/* ── Step 1: Resolved badge ── */
const resolvedBadgeLabel = computed(() => {
if (!props.comment.resolvedTime) return null;
- return props.comment.trackedChange ? 'Accepted' : 'Resolved';
+ if (!props.comment.trackedChange) return 'Resolved';
+ return props.comment.trackedChangeDecision === 'reject' ? 'Rejected' : 'Accepted';
});
/* ── Pending new comment (brand-new, not a reply) ── */
@@ -597,26 +598,31 @@ const handleAddComment = () => {
const handleReject = () => {
const customHandler = proxy.$superdoc.config.onTrackedChangeBubbleReject;
+ // Custom handlers always resolve so the bubble disappears from
+ // getFloatingComments (SD-2049). The internal decision path only resolves
+ // when the decision actually applied — otherwise the tracked marks are
+ // still in the document and the thread must stay open (SD-3386).
+ let decisionApplied = true;
if (props.comment.trackedChange && typeof customHandler === 'function') {
customHandler(props.comment, proxy.$superdoc.activeEditor);
} else if (props.comment.trackedChange) {
- commentsStore.decideTrackedChangeFromSidebar({
+ const result = commentsStore.decideTrackedChangeFromSidebar({
superdoc: proxy.$superdoc,
comment: props.comment,
decision: 'reject',
});
+ decisionApplied = Boolean(result?.ok) && result?.success !== false;
} else {
commentsStore.deleteComment({ superdoc: proxy.$superdoc, commentId: props.comment.commentId });
}
- // Always resolve tracked changes so resolvedTime is set and the bubble
- // disappears from getFloatingComments — even when a custom handler is used (SD-2049).
- if (props.comment.trackedChange) {
+ if (props.comment.trackedChange && decisionApplied) {
props.comment.resolveComment({
id: superdocStore.user.id,
email: superdocStore.user.email,
name: superdocStore.user.name,
superdoc: proxy.$superdoc,
+ decision: 'reject',
});
}
@@ -632,26 +638,33 @@ const handleReject = () => {
const handleResolve = () => {
const customHandler = proxy.$superdoc.config.onTrackedChangeBubbleAccept;
+ // Custom handlers always resolve so the bubble disappears from
+ // getFloatingComments (SD-2049). The internal decision path only resolves
+ // when the decision actually applied — otherwise the tracked marks are
+ // still in the document and the thread must stay open (SD-3386).
+ let decisionApplied = true;
if (props.comment.trackedChange && typeof customHandler === 'function') {
customHandler(props.comment, proxy.$superdoc.activeEditor);
} else {
if (props.comment.trackedChange) {
- commentsStore.decideTrackedChangeFromSidebar({
+ const result = commentsStore.decideTrackedChangeFromSidebar({
superdoc: proxy.$superdoc,
comment: props.comment,
decision: 'accept',
});
+ decisionApplied = Boolean(result?.ok) && result?.success !== false;
}
}
- // Always resolve so resolvedTime is set and the bubble disappears
- // from getFloatingComments — even when a custom handler is used (SD-2049).
- props.comment.resolveComment({
- id: superdocStore.user.id,
- email: superdocStore.user.email,
- name: superdocStore.user.name,
- superdoc: proxy.$superdoc,
- });
+ if (decisionApplied) {
+ props.comment.resolveComment({
+ id: superdocStore.user.id,
+ email: superdocStore.user.email,
+ name: superdocStore.user.name,
+ superdoc: proxy.$superdoc,
+ ...(props.comment.trackedChange ? { decision: 'accept' } : {}),
+ });
+ }
// Always cleanup the dialog state
nextTick(() => {
diff --git a/packages/superdoc/src/components/CommentsLayer/use-comment.js b/packages/superdoc/src/components/CommentsLayer/use-comment.js
index 8f87f316b2..d420e954a3 100644
--- a/packages/superdoc/src/components/CommentsLayer/use-comment.js
+++ b/packages/superdoc/src/components/CommentsLayer/use-comment.js
@@ -102,6 +102,9 @@ export default function useComment(params) {
const resolvedById = ref(params.resolvedById || null);
const resolvedByEmail = ref(params.resolvedByEmail || null);
const resolvedByName = ref(params.resolvedByName || null);
+ // For tracked-change threads: which decision resolved the thread
+ // ('accept' | 'reject'). Null for plain comments and legacy payloads.
+ const trackedChangeDecision = ref(params.trackedChangeDecision || null);
/**
* Mark this conversation as resolved with UTC date
@@ -109,14 +112,18 @@ export default function useComment(params) {
* @param {String} id The actor id of the user marking this conversation as done
* @param {String} email The email of the user marking this conversation as done
* @param {String} name The name of the user marking this conversation as done
+ * @param {'accept' | 'reject'} [decision] The tracked-change decision that resolved this thread
* @returns {void}
*/
- const resolveComment = ({ id, email, name, superdoc }) => {
+ const resolveComment = ({ id, email, name, superdoc, decision }) => {
if (resolvedTime.value) return;
resolvedTime.value = Date.now();
resolvedById.value = id ?? null;
resolvedByEmail.value = email;
resolvedByName.value = name;
+ if (decision === 'accept' || decision === 'reject') {
+ trackedChangeDecision.value = decision;
+ }
const emitData = { type: comments_module_events.RESOLVED, comment: getValues() };
propagateUpdate(superdoc, emitData);
@@ -318,6 +325,7 @@ export default function useComment(params) {
trackedChangeStoryKind: trackedChangeStoryKind.value,
trackedChangeStoryLabel: trackedChangeStoryLabel.value,
trackedChangeAnchorKey: trackedChangeAnchorKey.value,
+ trackedChangeDecision: trackedChangeDecision.value,
deletedText: deletedText.value,
resolvedTime: resolvedTime.value,
resolvedById: resolvedById.value,
@@ -360,6 +368,7 @@ export default function useComment(params) {
trackedChangeStoryKind,
trackedChangeStoryLabel,
trackedChangeAnchorKey,
+ trackedChangeDecision,
resolvedTime,
resolvedById,
resolvedByEmail,
diff --git a/packages/superdoc/src/stores/comments-store.js b/packages/superdoc/src/stores/comments-store.js
index 5a0796319b..3f1615482a 100644
--- a/packages/superdoc/src/stores/comments-store.js
+++ b/packages/superdoc/src/stores/comments-store.js
@@ -317,6 +317,7 @@ export const useCommentsStore = defineStore('comments', () => {
resolvedById: comment.resolvedById ?? null,
resolvedByEmail: comment.resolvedByEmail ?? null,
resolvedByName: comment.resolvedByName ?? null,
+ trackedChangeDecision: comment.trackedChangeDecision ?? null,
});
}
// Sets the resolved state to null so it can be restored in the comments sidebar
@@ -324,6 +325,7 @@ export const useCommentsStore = defineStore('comments', () => {
comment.resolvedById = null;
comment.resolvedByEmail = null;
comment.resolvedByName = null;
+ comment.trackedChangeDecision = null;
};
const restoreResolvedMetadata = (comment) => {
@@ -335,6 +337,7 @@ export const useCommentsStore = defineStore('comments', () => {
comment.resolvedById = snapshot.resolvedById ?? null;
comment.resolvedByEmail = snapshot.resolvedByEmail ?? null;
comment.resolvedByName = snapshot.resolvedByName ?? null;
+ comment.trackedChangeDecision = snapshot.trackedChangeDecision ?? null;
return true;
};
@@ -918,6 +921,7 @@ export const useCommentsStore = defineStore('comments', () => {
id: params.resolvedById ?? superdoc?.user?.id ?? null,
email: params.resolvedByEmail ?? superdoc?.user?.email ?? null,
name: params.resolvedByName ?? superdoc?.user?.name ?? null,
+ decision: params.decision ?? null,
superdoc,
};
@@ -1732,6 +1736,7 @@ export const useCommentsStore = defineStore('comments', () => {
comment.resolvedById = resolutionSnapshot.resolvedById ?? null;
comment.resolvedByEmail = resolutionSnapshot.resolvedByEmail ?? null;
comment.resolvedByName = resolutionSnapshot.resolvedByName ?? null;
+ comment.trackedChangeDecision = resolutionSnapshot.trackedChangeDecision ?? null;
restoredComments.push(comment);
return true;
}
diff --git a/packages/superdoc/src/stores/comments-store.test.js b/packages/superdoc/src/stores/comments-store.test.js
index 46c42e28a4..cda3442f65 100644
--- a/packages/superdoc/src/stores/comments-store.test.js
+++ b/packages/superdoc/src/stores/comments-store.test.js
@@ -678,6 +678,7 @@ describe('comments-store', () => {
id: 'reviewer-id',
email: 'reviewer@example.com',
name: 'Reviewer',
+ decision: null,
superdoc,
});
expect(existingComment.resolvedTime).not.toBeNull();