Comments new line fix

api/crates/infrastructure/src/documents/realtime/hub.rs

@@ -343,6 +343,17 @@ impl Hub {
             txt_new.get_string(&txn)
         };
 
+        // Autosave PUTs re-send markdown the room already has. Replacing the
+        // whole YText (delete + insert) and broadcasting would invalidate
+        // client editor decorations even when the string is unchanged.
+        {
+            let txt = room.doc.get_or_insert_text("content");
+            let txn = room.doc.transact();
+            if txt.get_string(&txn) == new_markdown {
+                return Ok(());
+            }
+        }
+
         let update_bytes = {
             let txt = room.doc.get_or_insert_text("content");
             let mut txn = room.doc.transact_mut();

api/crates/infrastructure/src/documents/realtime/redis/engine.rs

@@ -431,12 +431,21 @@ impl RealtimeEngineTrait for RedisRealtimeEngine {
             .hydration_service
             .hydrate(&uuid, HydrationOptions::default())
             .await?;
-        let update_bytes = {
+        let new_markdown = {
             let txt_new = doc.get_or_insert_text("content");
             let txn_new = doc.transact();
-            let new_markdown = txt_new.get_string(&txn_new);
-            drop(txn_new);
-
+            txt_new.get_string(&txn_new)
+        };
+        // Autosave PUTs re-send markdown already present in the doc. Skip the
+        // YText wipe+rewrite so connected editors keep decorations intact.
+        {
+            let txt = hydrated.doc.get_or_insert_text("content");
+            let txn = hydrated.doc.transact();
+            if txt.get_string(&txn) == new_markdown {
+                return Ok(());
+            }
+        }
+        let update_bytes = {
             let txt = hydrated.doc.get_or_insert_text("content");
             let mut txn = hydrated.doc.transact_mut();
             let len = txt.len(&txn);

app/src/features/document-comments/index.ts

@@ -1,6 +1,7 @@
 export { CommentsPanel } from './ui/CommentsPanel'
 export {
   buildCommentMarker,
+  buildCommentMarkerInsertion,
   createCommentId,
   createCommentMarkerId,
   findUnknownCommentMarkers,

app/src/features/document-comments/lib/thread-range.test.ts

@@ -4,7 +4,12 @@ import type { DocumentCommentThread } from '@/entities/document'
 
 import type { DocumentEditorApi } from '@/features/plugins'
 
-import { findCommentThreadRange, getCommentThreadLine } from './thread-range'
+import {
+  findCommentThreadRange,
+  getCommentThreadLine,
+  getLineEndOffset,
+  isCommentMarkerAloneOnLine,
+} from './thread-range'
 
 function commentThread(
   overrides: Partial<DocumentCommentThread> = {},
@@ -43,25 +48,39 @@ const rangeEditor = {
 } as DocumentEditorApi
 
 describe('comment thread range lookup', () => {
-  it('uses the live marker before stale offsets when quote matching fails', () => {
-    const content = 'changed text<!--comment:thread-1-->'
-    const markerIndex = content.indexOf('<!--comment:thread-1-->')
-
+  it('uses stored offsets when adjacent quote matching fails', () => {
     expect(
       findCommentThreadRange(
         commentThread({
           quote: 'original text',
           startOffset: 0,
           endOffset: 8,
         }),
+        'changed text<!--comment:thread-1-->',
+        rangeEditor,
+      ),
+    ).toEqual({
+      startLineNumber: 0,
+      startColumn: 8,
+      endLineNumber: 0,
+      endColumn: 8,
+    })
+  })
+
+  it('finds the quote on the previous line when the marker is alone on a line', () => {
+    const content = 'alpha target beta\n<!--comment:thread-1-->'
+
+    expect(
+      findCommentThreadRange(
+        commentThread({ quote: 'target' }),
         content,
         rangeEditor,
       ),
     ).toEqual({
-      startLineNumber: markerIndex,
-      startColumn: '<!--comment:thread-1-->'.length,
-      endLineNumber: markerIndex,
-      endColumn: markerIndex + '<!--comment:thread-1-->'.length,
+      startLineNumber: 6,
+      startColumn: 'target'.length,
+      endLineNumber: 6,
+      endColumn: 6 + 'target'.length,
     })
   })
 })
@@ -78,9 +97,32 @@ describe('comment thread line lookup', () => {
       .toBe(3)
   })
 
-  it('falls back to stored line metadata when the marker is missing', () => {
+  it('points at the content line when the marker is alone on the next line', () => {
+    const content = 'target text\n<!--comment:thread-1-->'
+
+    expect(getCommentThreadLine(commentThread(), content)).toBe(1)
+  })
+})
+
+describe('marker line helpers', () => {
+  it('detects markers alone on a line', () => {
     expect(
-      getCommentThreadLine(commentThread({ startLineNumber: 4 }), 'target'),
-    ).toBe(4)
+      isCommentMarkerAloneOnLine(
+        'hello\n<!--comment:thread-1-->\nworld',
+        '<!--comment:thread-1-->',
+      ),
+    ).toBe(true)
+    expect(
+      isCommentMarkerAloneOnLine(
+        'hello<!--comment:thread-1-->',
+        '<!--comment:thread-1-->',
+      ),
+    ).toBe(false)
+  })
+
+  it('computes line end offsets', () => {
+    expect(getLineEndOffset('a\nbb\nc', 1)).toBe(1)
+    expect(getLineEndOffset('a\nbb\nc', 2)).toBe(4)
+    expect(getLineEndOffset('a\nbb\nc', 3)).toBe(6)
   })
 })

app/src/features/document-comments/lib/thread-range.ts

@@ -2,14 +2,45 @@ import type { DocumentCommentThread } from '@/entities/document'
 
 import type { DocumentEditorApi, DocumentEditorRange } from '@/features/plugins'
 
+import { COMMENT_MARKER_WRAP_BREAK } from '../model/comments-store'
+
+const LONE_MARKER_LINE = /^\u200B?<!--comment:[A-Za-z0-9_-]+-->$/
+
+function markerAnchorStart(content: string, markerIndex: number) {
+  if (
+    markerIndex > 0 &&
+    content[markerIndex - 1] === COMMENT_MARKER_WRAP_BREAK
+  ) {
+    return markerIndex - 1
+  }
+  return markerIndex
+}
+
+/** True when the marker is the only content on its line (optional leading ZWSP). */
+export function isCommentMarkerAloneOnLine(
+  content: string,
+  marker: string,
+  markerIndex = content.indexOf(marker),
+) {
+  if (markerIndex < 0) return false
+  const lineStart = content.lastIndexOf('\n', markerIndex - 1) + 1
+  const nextNl = content.indexOf('\n', markerIndex)
+  const lineEnd = nextNl < 0 ? content.length : nextNl
+  return LONE_MARKER_LINE.test(content.slice(lineStart, lineEnd))
+}
+
 export function findCommentMarkerRange(
   thread: DocumentCommentThread,
   content: string,
   editor: DocumentEditorApi,
 ): DocumentEditorRange | null {
   const markerIndex = content.indexOf(thread.marker)
   if (markerIndex < 0) return null
-  return editor.getRangeFromOffset(markerIndex, thread.marker.length)
+  const start = markerAnchorStart(content, markerIndex)
+  return editor.getRangeFromOffset(
+    start,
+    markerIndex + thread.marker.length - start,
+  )
 }
 
 export function findCommentThreadRange(
@@ -19,14 +50,41 @@ export function findCommentThreadRange(
 ): DocumentEditorRange | null {
   const markerIndex = content.indexOf(thread.marker)
   if (markerIndex >= 0 && thread.quote) {
-    const quoteStart = Math.max(0, markerIndex - thread.quote.length)
-    if (content.slice(quoteStart, markerIndex) === thread.quote) {
+    const quoteEnd = markerAnchorStart(content, markerIndex)
+    const quoteStart = Math.max(0, quoteEnd - thread.quote.length)
+    if (content.slice(quoteStart, quoteEnd) === thread.quote) {
       return editor.getRangeFromOffset(quoteStart, thread.quote.length)
     }
+
+    // Markers live on their own hidden line after the content line. Find the
+    // quote on the previous line so highlights stay on the real text.
+    if (isCommentMarkerAloneOnLine(content, thread.marker, markerIndex)) {
+      const markerLineStart = content.lastIndexOf('\n', markerIndex - 1) + 1
+      const prevLineEnd = Math.max(0, markerLineStart - 1)
+      const prevLineStart =
+        prevLineEnd > 0 ? content.lastIndexOf('\n', prevLineEnd - 1) + 1 : 0
+      const prevLine = content.slice(prevLineStart, prevLineEnd)
+      const quotePos = prevLine.lastIndexOf(thread.quote)
+      if (quotePos >= 0) {
+        return editor.getRangeFromOffset(
+          prevLineStart + quotePos,
+          thread.quote.length,
+        )
+      }
+    }
   }
 
   if (markerIndex >= 0) {
-    return editor.getRangeFromOffset(markerIndex, thread.marker.length)
+    // Prefer stored offsets over highlighting the hidden marker itself.
+    if (
+      typeof thread.startOffset === 'number' &&
+      typeof thread.endOffset === 'number' &&
+      thread.endOffset > thread.startOffset
+    ) {
+      const length = thread.endOffset - thread.startOffset
+      return editor.getRangeFromOffset(thread.startOffset, length)
+    }
+    return findCommentMarkerRange(thread, content, editor)
   }
 
   if (
@@ -56,7 +114,11 @@ export function getCommentThreadLine(
 ) {
   const markerIndex = content.indexOf(thread.marker)
   if (markerIndex >= 0) {
-    return content.slice(0, markerIndex).split('\n').length
+    const markerLine = content.slice(0, markerIndex).split('\n').length
+    if (isCommentMarkerAloneOnLine(content, thread.marker, markerIndex)) {
+      return Math.max(1, markerLine - 1)
+    }
+    return markerLine
   }
 
   if (thread.startLineNumber && Number.isFinite(thread.startLineNumber)) {
@@ -65,3 +127,19 @@ export function getCommentThreadLine(
 
   return null
 }
+
+/** 1-based line number → offset of that line's trailing newline, or content.length at EOF. */
+export function getLineEndOffset(content: string, lineNumber: number) {
+  const target = Math.max(1, Math.floor(lineNumber))
+  let line = 1
+  let index = 0
+  while (line < target && index < content.length) {
+    const nl = content.indexOf('\n', index)
+    if (nl < 0) return content.length
+    index = nl + 1
+    line += 1
+  }
+  if (index >= content.length) return content.length
+  const nl = content.indexOf('\n', index)
+  return nl < 0 ? content.length : nl
+}

app/src/features/document-comments/model/comments-store.test.ts

@@ -55,6 +55,16 @@ describe('comment markers', () => {
     )
   })
 
+  it('does not leave a blank line when a marker occupied its own line', () => {
+    const marker = buildCommentMarker('owned')
+
+    expect(stripCommentMarkers(`alpha\n${marker}\nbeta`, [marker])).toBe(
+      'alpha\nbeta',
+    )
+    expect(stripCommentMarkers(`alpha\n${marker}`, [marker])).toBe('alpha')
+    expect(stripCommentMarkers(`${marker}\nbeta`, [marker])).toBe('beta')
+  })
+
   it('finds valid comment markers in content once', () => {
     const marker = buildCommentMarker('owned')
     const content = `${marker} text ${marker} <!--comment:bad marker-->`

app/src/features/document-comments/model/comments-store.ts

@@ -65,13 +65,25 @@ export function buildCommentMarker(id: string) {
   return `<!--comment:${id}-->`
 }
 
+/** Zero-width space kept for legacy inline markers during migration. */
+export const COMMENT_MARKER_WRAP_BREAK = '\u200B'
+
+export function buildCommentMarkerInsertion(id: string) {
+  // Markers sit on their own line so they don't steal wrap width from content.
+  return `\n${buildCommentMarker(id)}`
+}
+
 export function parseCommentMarkerId(marker: string) {
   const match = /^<!--comment:([A-Za-z0-9_-]+)-->$/.exec(marker)
   return match?.[1] ?? null
 }
 
 const COMMENT_MARKER_PATTERN = /<!--comment:[A-Za-z0-9_-]+-->/g
 
+function escapeRegExp(value: string) {
+  return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+}
+
 export function findCommentMarkers(content: string) {
   const matches = content.match(COMMENT_MARKER_PATTERN)
   return matches ? Array.from(new Set(matches)) : []
@@ -94,6 +106,12 @@ export function stripCommentMarkers(
   for (const marker of markers) {
     if (!marker || seen.has(marker)) continue
     seen.add(marker)
+    const escaped = escapeRegExp(marker)
+    // Drop markers that occupy a whole line so we don't leave a blank line.
+    out = out.replace(new RegExp(`\\n\\u200B?${escaped}(?=\\n|$)`, 'g'), '')
+    out = out.replace(new RegExp(`^\\u200B?${escaped}\\n`), '')
+    out = out.replace(new RegExp(`^\\u200B?${escaped}$`), '')
+    out = out.split(`\u200B${marker}`).join('')
     out = out.split(marker).join('')
   }
   return out