Skip to content

fix(app): render replies that live only in YAML endmatter - #145

Open
moiri-gamboni wants to merge 3 commits into
Lex-Inc:mainfrom
moiri-gamboni:fix/endmatter-reply-rendering
Open

moiri-gamboni wants to merge 3 commits into
Lex-Inc:mainfrom
moiri-gamboni:fix/endmatter-reply-rendering

Conversation

@moiri-gamboni

Copy link
Copy Markdown

Problem

A reply stored in YAML endmatter is written to the file and then never shown. The reviewer types a reply, it appears while the composer is open, and it is gone on the next load — so from the user's side the reply simply vanished.

This is reproducible with Roughdraft's own output, which is what makes it more than a formatting nit:

  1. Open any document with an anchored comment {#c1}.
  2. Click the reply button on that comment, type, save. Roughdraft writes c2: {body, by, at, re: c1} into the endmatter, with no inline marker — the shape the README and the RFM spec both document for replies.
  3. Reload the page. The reply is gone from the rail, though it is still in the file.

The server agrees the reply is real: GET /api/review-index returns it with kind: "reply" and the correct parentId. Only the rail drops it.

Root cause

buildCommentThreadRailItems builds each rail group from group.commentIds, which comes from the anchor element's data-comment-ids. That attribute only lists comments carrying an inline marker.

Endmatter replies have no inline marker by design, so although addEndmatterFeedback parses them into the comments map with the right parentCommentId, they are filtered out of visibleComments before buildCommentThreads ever runs. The thread is built from the root alone and the reply is discarded.

This looks like an incomplete part of #91, whose description says the review rail "now handles unanchored comments, replies, and suggestion metadata in the endmatter-backed format".

Likely the same root cause as #87, which reports it as a bullet-list rendering problem: "I added a comment which was rendering fine, but then when the model made a reply to that comment, it stopped rendering as a comment in the sidebar." The list is incidental; the reply is the trigger.

Fix

Before building threads, pull each anchored root's descendants back in from the comments map, using the existing getCommentDescendantIds helper. Comments already present in the group are skipped, so inline replies are not duplicated, and a group with no anchored roots is unaffected.

Tests

packages/app/src/document-comments.test.ts (new): parses a document whose only reply lives in the endmatter and asserts the rail item contains both c1 and c2, plus a second case asserting no duplication when the reply is also anchored inline. The first test fails on main and passes with the fix.

pnpm test — 377 passed. pnpm lint clean.

Scope

App-side only, and independent of the open runWatch timeout PRs (#121, #122, #126, #139, #144) — no overlapping files.

A comment rail group is built from the anchor's data-comment-ids, which
lists only comments carrying an inline marker. Replies are stored in the
endmatter with a `re` pointer and no marker, so they were parsed into the
comments map and then filtered out before threading: the reply rendered
while it was being composed and disappeared on the next load. Writing a
reply through the app's own reply button and reloading reproduces it.

Pull each anchored root's descendants back in before building threads.
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

@moiri-gamboni is attempting to deploy a commit to the Lex Team on Vercel.

A member of the Team first needs to authorize it.

The rail fix generalizes: any surface that renders from anchor ids drops
replies stored in endmatter, because those ids come from inline markers
and an endmatter reply has none. The selection banner reads
getSelectionCommentIds, so it had the same hole.

Extract the descendant walk as collectAnchoredThreadComments and use it
for both, rather than repeating the filter at each call site.
replyToComment anchored the new reply against the id of the card being
replied to. An endmatter reply has no inline marker, so the lookup
returned null, replyToComment returned early, and clicking Reply on a
nested card did nothing at all -- no composer, no error.

Walk up to the nearest anchored ancestor for the anchor while keeping the
clicked card as the parent, so the reply nests under it rather than being
flattened onto the thread root.
@moiri-gamboni

Copy link
Copy Markdown
Author

Pushed two follow-up commits after finding the same root cause on two more surfaces. The original commit fixed the comment rail only, which left the bug half-fixed.

Selection banner. activeCommentIds comes from getSelectionCommentIds, i.e. the marks under the selection, so it drops endmatter replies exactly like the rail did. Rather than repeat the filter, the descendant walk is now collectAnchoredThreadComments in document-comments.ts and both surfaces call it.

Replying to a reply did nothing. replyToComment anchors the new reply via addCommentIdsToAnchor(editor, commentId, ...), which returns null when the target has no inline mark (PageCard.tsx:296). For an endmatter reply that is always, so the function returned early: clicking Reply on a nested card produced no composer and no error. It now walks up to the nearest anchored ancestor for the anchor while keeping the clicked card as the parent, so the reply nests under it instead of being flattened onto the root.

Reproduction for the second one, using only the app's own output: reply to any anchored comment through the UI (Roughdraft writes re: into the endmatter), reload, then try to reply to that reply. Nothing happens.

New test in packages/app/test/page-card.test.tsx covers the composer opening from an unanchored reply; it fails on main. pnpm test 381 passed, pnpm lint clean.

peterhartree added a commit to peterhartree/roughdraft that referenced this pull request Aug 30, 2026
Port upstream PR Lex-Inc#145 (7ee81bc, 5483e46, f5048d6) by Moïri Gamboni.
Replies stored only in YAML endmatter have no inline anchor, so the
rail dropped them and replying to such a reply failed silently.
Thread building now pulls an anchored root's descendants in via
collectAnchoredThreadComments, and replyToComment walks up to the
nearest anchored ancestor for the anchor while keeping the clicked
card as the parent.

Co-Authored-By: Claude <noreply@anthropic.com>
jmatthewpryor added a commit to jmatthewpryor/roughdraft that referenced this pull request Sep 11, 2026
… bump to 0.1.11-patched.2

Adapts Lex-Inc#149 to a per-watch undici Agent instead of a process-wide
singleton: the singleton kept a pooled keep-alive socket to a server that
had since restarted, so the next watch in the same process failed with
UND_ERR_SOCKET "other side closed". The Agent is now created per watch in
both the CLI and the MCP watch tool and closed in a finally.

Fixes a pre-existing race in two CLI tests that posted the review event as
soon as the watch request had been sent, before the server had registered
the waiter; they now poll /api/review-events/status for a watcher first.

Documents the new patches, the Lex-Inc#149 adaptation, the merge-conflict notes,
and the new "handoff after watcher lost" state in the screenshot guide.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
felciano added a commit to felciano/roughdraft that referenced this pull request Oct 31, 2026
…y in YAML endmatter

From Lex-Inc#145, unmerged upstream as of 2026-10-31.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LXZP8LMTVHbkEsmdNZzjyf

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant