Skip to content

Comment count mismatch: overlapping comment silently clobbers existing mark, orphaning its store entry #830

Description

@mrangelmarino

Symptom

With a single visible comment in the document, the chat-panel chips read "Process 2 comments" / "Review 2 comments" while the status bar reads "1 comment". Traversing threads in the Comment Review panel cycles through what looks like the same comment twice.

Root cause (confirmed via Playwright repro against the built app)

Adding a comment whose range fully overlaps an existing comment mark silently replaces the old mark in the document (ProseMirror marks of the same type exclude each other by default, and setCommentcommands.setMark does nothing to prevent it). The old thread's entry stays behind in useCommentStore.pendingComments, so the store and the document diverge:

after adding c1 on "hello world":        marks: [c1]      store: [c1]
after adding c2 on the same range:       marks: [c2]      store: [c1, c2]   ← c1 is now a ghost
after adding c3 partially overlapping:   marks: [c2, c3]  store: [c1, c2, c3]  (partial overlap splits, both survive)

The two UI surfaces then disagree because they count different sources:

  • Chat-panel chips: pendingComments.filter(c => !c.resolved).length (store) → 2
  • Status bar: getComments(editor).length (live doc marks, StatusBar.tsx:92) → 1
  • CommentReviewPanel iterates the store, so it shows both threads — anchored to the same text, hence "the same comment twice"

Why full overlap is the common case

The AI add_comment tool resolves nodeId targets to the entire node range (executors/document.ts:465-473). Two add_comment calls on the same node (e.g. the model re-commenting a paragraph across turns) produce exactly-equal ranges → guaranteed clobber. The user UI path can also hit it by commenting a selection that covers an existing comment.

Data-loss angle

The divergence doesn't just miscount — it destroys thread A either way:

  • Immediately: A's mark (highlight/anchor) is gone from the doc.
  • Later: the next tab-switch save runs mergeCommentsForPersistence, which drops unresolved store entries with no live mark — thread A, including any replies, is silently deleted. The bug "self-heals" by discarding data, which is why it's easy to miss.

Repro

  1. npm run build, launch out/main/index.js (isolated HOME to dodge the single-instance lock)
  2. In an untitled doc: editor.chain().setTextSelection({from: 1, to: 12}).setComment({id: 'c1', comment: 'first'}).run()
  3. Repeat with the same range and id: 'c2'
  4. window.__prose_tools.getCommentStore() → 2 entries; comment marks in editor.state.doc → only c2

Fix directions

  1. Prevent the clobber: in setComment (and/or executeAddComment), detect existing comment marks in the target range. Either block with a notification pointing at the existing thread ("reply instead"), or treat it as an explicit replace and remove the old thread's store entry in the same operation. For the AI path, add_comment on an already-commented node should probably return the existing thread id so the model replies rather than duplicates.
  2. Unify the counts: chips, status bar, and review panel should derive from one selector (e.g. unresolved store entries that have a live mark) so any future divergence can't render contradictory UI.
  3. Optional hardening: restoreComments restoring two threads onto the same text has the same clobber semantics (second addMark wins) — worth a warning or reconciliation there too.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    Status
    Do First

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions