Skip to content

SD-2570 - fix: document API tracked changes not fully rendered on document#2825

Open
chittolinag wants to merge 3 commits intomainfrom
gabriel/sd-2570-bug-missing-content-inserted-by-ai-simplified-fix
Open

SD-2570 - fix: document API tracked changes not fully rendered on document#2825
chittolinag wants to merge 3 commits intomainfrom
gabriel/sd-2570-bug-missing-content-inserted-by-ai-simplified-fix

Conversation

@chittolinag
Copy link
Copy Markdown
Contributor

@chittolinag chittolinag commented Apr 15, 2026

Issue

Tracked text inserted programmatically with editor.doc.insert(..., { changeMode: 'tracked' }) was truncated in the document view when the inserted string contained newline characters (\n). Only the content before the first newline was rendered, even though the full text was preserved in comment bubbles and export.

Proposed solution

Align renderer run slicing with measurer behavior for inline newlines. The renderer now expands text runs containing \n into text/break/text segments before slicing line ranges, so measured line indices map correctly and all multiline tracked content renders in the document.

@linear
Copy link
Copy Markdown

linear bot commented Apr 15, 2026

@chittolinag chittolinag marked this pull request as ready for review April 15, 2026 20:34
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: 9781989a3e

ℹ️ 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".

if ((run as TextRun).text && typeof (run as TextRun).text === 'string' && (run as TextRun).text.includes('\n')) {
const textRun = run as TextRun;
const segments = textRun.text.split('\n');
let cursor = textRun.pmStart ?? 0;
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 Avoid fabricating PM positions for newline-split runs

The newline expansion path seeds cursor with textRun.pmStart ?? 0, which means runs that originally had no PM mapping are rewritten with synthetic pmStart/pmEnd values. Those fabricated offsets are later emitted as data-pm-* attributes during rendering, so in documents rendered without real PM coordinates the caret/click mapping can resolve to incorrect positions (often near the document start) instead of using the no-position fallback path. Preserve pmStart/pmEnd as undefined when the source run has no PM metadata.

Useful? React with 👍 / 👎.

};

export const sliceRunsForLine = (block: ParagraphBlock, line: Line): Run[] => {
const runs = expandRunsForInlineNewlines(block.runs as Run[]);
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 Expand inline-newline runs only once per paragraph

This call now recomputes newline expansion for the full paragraph run list every time a single line is rendered. Since renderLine invokes sliceRunsForLine per measured line, long paragraphs incur repeated full-array scans and allocations, turning rendering into avoidable O(lines × runs) work and causing noticeable paint slowdowns on large documents with tracked changes. Cache or precompute the expanded run array per paragraph/layout pass instead of rebuilding it per line.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@artem-harbour artem-harbour left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants