fix: track new lines in suggested mode

[harbournick]

No description provided.

[linear-code]

IT-1136

[github-actions]

I need to be upfront: every call to the ecma-spec MCP tools was blocked at the permission layer (I retried ~9 times across ooxml_attributes, ooxml_children, ooxml_element, ooxml_section). So the spec cross-check below is from my knowledge of ECMA-376, not a live MCP verification. If you grant the mcp__ecma-spec__* permissions, I'll re-run it to confirm.

Here's the review based on reading the actual emitted XML in the changed handlers.

---

Status: FAIL

The core of this PR is actually a compliance improvement, but there's one genuine schema-order violation introduced by the new export path, plus a pre-existing non-standard attribute that flows through the changed code.

1. w:rPr can be emitted in the wrong position inside w:pPr — generate-paragraph-properties.js:61

prependParagraphSplitInsertion does pPr.elements.unshift(runProperties) when no w:rPr exists yet. If the paragraph already has paragraph-level properties (w:pStyle, w:jc, w:spacing, etc.), this inserts the paragraph-mark w:rPr at index 0 — i.e. before those elements. Per the CT_PPr sequence, the paragraph-mark w:rPr must come after all paragraph-level properties and before w:sectPr/w:pPrChange. Ironically the same file already respects this ordering rule for sectPr (see the comment at line 123), so this is an inconsistency. The test only covers the empty-pPr case, so the mis-ordering is latent. Fix: insert runProperties just before any w:sectPr/w:pPrChange rather than at the front. (ordering ref)

2. Non-standard w:authorEmail on w:rPrChange — helpers.js:109

createRunPropertiesChangeElement writes 'w:authorEmail' into the w: namespace. CT_TrackChange (the base for w:rPrChange, w:ins, etc.) only defines w:id, w:author, and w:date — there is no w:authorEmail. This is pre-existing (it's an unchanged context line, not introduced here), and the new createParagraphSplitInsertionElement correctly omits it. Flagging only because the changed function still emits it; worth dropping or moving to a foreign namespace. (ref)

What's correct / improved (for the record):

• w:ins inside the paragraph-mark w:rPr is exactly the right OOXML mechanism for a tracked paragraph-mark (split) insertion, and it's correctly unshifted to the front of w:rPr (the EG_*TrackChanges group leads CT_ParaRPr).
• The w:id decimal coercion (getTrackFormatChangeWordId → /^\d+$/) is a real compliance fix: w:id is ST_DecimalNumber, so the old 'format-1'-style ids were invalid and are now valid integers.
• The w:pPrChange import/decode tests correctly keep SuperDoc-only sd:* metadata in a foreign namespace and never write it into w:-namespaced output — the spec-sanctioned extension approach.

The headline issue is #1 (ordering); #2 is pre-existing cleanup.

[cubic-dev-ai]

cubic analysis

3 issues found across 17 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/p/helpers/generate-paragraph-properties.js">

<violation number="1" location="packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/p/helpers/generate-paragraph-properties.js:24">
P2: Descendant traversal can clear a previously found paragraph-split mark, causing paragraph split tracking to be skipped.</violation>

<violation number="2" location="packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/p/helpers/generate-paragraph-properties.js:61">
P2: `w:rPr` is prepended to `w:pPr`, which can break paragraph-property element ordering.</violation>
</file>

<file name="packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js">

<violation number="1" location="packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js:992">
P1: `rejectParagraphSplit` performs `tr.join(...)` in the position-stable mark-op pass, which can invalidate positions for later ops in the same decision transaction.</violation>
</file>

Linked issue analysis

Linked issue: IT-1136: Track Changes: paragraph split (Enter) and paragraph-style changes are applied directly, not recorded as tracked changes

Status	Acceptance criteria	Notes
✅	Pressing Enter (split run to paragraph) in Suggesting mode records a tracked paragraph-split formatting change that can be accepted or rejected	split-run.js adds addTrackedParagraphSplitMark to create a trackFormat mark for the split; tests in split-run.test.js assert the change is recorded, and that it can be rejected and reverses the split.
✅	Paragraph-split tracked marks include paragraphSplit snapshots and are exported as a Word-visible paragraph insertion (w:ins) in paragraph properties when exporting a non-final doc (using the Word revision id allocator when present)	helpers detect paragraphSplit snapshots and create w:ins elements for export; generate-paragraph-properties prepends the insertion into w:pPr; tests validate the w:ins element and allocator usage.
✅	Import restores paragraphSplit snapshots from paragraph mark insertion metadata so imported docs regain the tracked-split representation	markImporter now maps inline paragraph insertion metadata to paragraphSplit snapshots on import; tests verify the snapshot restoration path.
✅	UI surfaces paragraph splits as a ‘new line’ tracked-change display and Comment dialog shows an 'Added new line' message	tracked-change-display returns a paragraphSplit display type and tests cover it; CommentDialog template and tests were updated to render the 'Added new line' display.

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P1: rejectParagraphSplit performs tr.join(...) in the position-stable mark-op pass, which can invalidate positions for later ops in the same decision transaction.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/super-editor/src/editors/v1/extensions/track-changes/review-model/decision-engine.js, line 992:

<comment>`rejectParagraphSplit` performs `tr.join(...)` in the position-stable mark-op pass, which can invalidate positions for later ops in the same decision transaction.</comment>

<file context>
@@ -947,6 +989,13 @@ const applyPlan = ({ state, plan }) => {
         tr.step(new RemoveMarkStep(op.from, op.to, op.mark));
         continue;
       }
+      if (op.kind === 'rejectParagraphSplit' && op.mark) {
+        tr.step(new RemoveMarkStep(op.from, op.to, op.mark));
+        if (!rejectParagraphSplitAt(tr, op.from, op.anchor)) {
</file context>

[cubic-dev-ai]

P2: w:rPr is prepended to w:pPr, which can break paragraph-property element ordering.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/p/helpers/generate-paragraph-properties.js, line 61:

<comment>`w:rPr` is prepended to `w:pPr`, which can break paragraph-property element ordering.</comment>

<file context>
@@ -1,5 +1,66 @@
+  if (!Array.isArray(runProperties.elements)) runProperties.elements = [];
+  const hasParagraphInsertion = runProperties.elements.some((element) => element?.name === 'w:ins');
+  if (!hasParagraphInsertion) runProperties.elements.unshift(insertionElement);
+  if (!existingRunProperties) pPr.elements.unshift(runProperties);
+  return pPr;
+}
</file context>

Suggested change

	if (!existingRunProperties) pPr.elements.unshift(runProperties);
	if (!existingRunProperties) {
	const anchorIdx = pPr.elements.findIndex((element) => element?.name === 'w:sectPr' || element?.name === 'w:pPrChange');
	if (anchorIdx === -1) pPr.elements.push(runProperties);
	else pPr.elements.splice(anchorIdx, 0, runProperties);
	}

[cubic-dev-ai]

P2: Descendant traversal can clear a previously found paragraph-split mark, causing paragraph split tracking to be skipped.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/super-editor/src/editors/v1/core/super-converter/v3/handlers/w/p/helpers/generate-paragraph-properties.js, line 24:

<comment>Descendant traversal can clear a previously found paragraph-split mark, causing paragraph split tracking to be skipped.</comment>

<file context>
@@ -1,5 +1,66 @@
+    let found = null;
+    node.descendants((child) => {
+      const childMarks = Array.isArray(child?.marks) ? child.marks : [];
+      found = childMarks.find((mark) => isParagraphSplitTrackFormatMark(mark)) || null;
+      return !found;
+    });
</file context>

Suggested change

	found = childMarks.find((mark) => isParagraphSplitTrackFormatMark(mark)) || null;
	return !found;
	const match = childMarks.find((mark) => isParagraphSplitTrackFormatMark(mark));
	if (match) found = match;
	return !found;

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7cb843fabb

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Map paragraph-split positions before joining

When a user rejects two or more paragraphSplit formatting changes in one operation (for example rejectAllTrackedChanges after pressing Enter twice in suggesting mode), the first tr.join changes document positions, but the loop continues using the original op.from/op.to for the later ops. Those stale offsets can make the next RemoveMarkStep/join target the wrong paragraph or fail the whole decision, unlike the previous formatting-only ops which did not change document size.

Useful? React with 👍 / 👎.