Skip to content

SD-3386 - fix: deletion handling across paragraph boundaries#3664

Open
chittolinag wants to merge 2 commits into
mainfrom
gabriel/sd-3386-bug-multi-line-tracked-change-selection-inserts-spurious
Open

SD-3386 - fix: deletion handling across paragraph boundaries#3664
chittolinag wants to merge 2 commits into
mainfrom
gabriel/sd-3386-bug-multi-line-tracked-change-selection-inserts-spurious

Conversation

@chittolinag
Copy link
Copy Markdown
Contributor

@chittolinag chittolinag commented Jun 5, 2026

Issue

In suggesting mode, deleting a selection that spans two paragraphs pushed the remainder of the second paragraph onto a new line, as if Enter was pressed. On the real Backspace/Delete keymap path the tracked interception could crash position mapping (RangeError: Position NaN), and the dispatch fallback then deleted the text untracked. Accepting or rejecting the change failed silently, leaving the inline decoration painted, and the hover bubble reported "Accepted" even after a reject.

Proposed solution

  • ProseMirror describes a cross-paragraph deletion as a ReplaceStep carrying a structural paragraph re-join shell; the tracked-changes compiler misread that shell as replacement content, inserted it at the selection end (splitting the paragraph), and minted a malformed replacement the decision engine refuses. replaceStep.js now compiles slices without inline leaf content as plain tracked deletions, skips the invert/mirror mapping pairing for that mark-only compile (the source of the NaN), and emits an explicit caret hint at the deletion's left edge.
  • Decision resolve events now carry which decision was taken; the comment model stores it and the resolved badge shows "Rejected" vs "Accepted". The bubble's accept/reject no longer mark the thread resolved when the underlying editor decision did not apply.
  • Regression tests cover the command path, the real keymap path (Backspace and Delete), the accept/reject lifecycle, and the badge/resolve behavior.

@linear-code
Copy link
Copy Markdown

linear-code Bot commented Jun 5, 2026

SD-3386

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...perdoc/src/components/CommentsLayer/use-comment.js 80.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@chittolinag chittolinag marked this pull request as ready for review June 5, 2026 21:48
@chittolinag chittolinag requested a review from a team as a code owner June 5, 2026 21:48
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 737a2ae4be

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +584 to +585
const isStructuralShellDelete =
step.from !== step.to && step.slice.content.size > 0 && !sliceHasInlineLeafContent(step.slice);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve block-only replacement content

When a non-empty selection is replaced with a block-only slice that has no inline descendants (for example DOCX paste/import content containing a mathBlock, passthroughBlock, or an empty block SDT), this predicate classifies the step as a structural-shell delete and routes it through makeTextDeleteIntent, so the selected text is marked deleted but the inserted block is silently dropped. The special case should be limited to ProseMirror's paragraph re-join shell (or explicitly exclude block leaf/atom content), otherwise suggesting mode loses valid replacement content instead of inserting or failing closed.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants