Repository navigation
fix(annotate): folder sessions export each comment once - #1696
Merged
Merged
Conversation
In a folder annotate session (and any session submitted while a linked document is open) the open document's comments were exported twice: once from the host's live state under the session heading, and again from useLinkedDoc.getDocAnnotations(), which also carries the active document. Plan review submitted with a linked doc open also dropped the plan's own comments, since the stashed plan has no source path. useLinkedDoc gains getFeedbackDocuments() (root once, every other document once) and packages/editor/feedbackDocuments.ts resolves the export sections once for both export paths (annotationsOutput and the submitted payload). Folder sessions title the per-file section 'Folder Document Feedback' instead of 'documents referenced in the plan'.
… a self-opened root copy Review follow-ups for the folder duplicate-export fix: - exportLinkedDocAnnotations now renders each document through the same entry renderer as exportAnnotations (renderAnnotationEntries): document order (block-2 before block-10), [In diff content], quick labels with their tip and a Label Summary, and threaded replies. Plain comment, deletion and global output is unchanged. - getFeedbackDocuments() no longer deletes a cache entry under the root's own path: it is a copy opened from the root itself (an HTML Home link) with its own comments, which the counts include, so it is exported under its path. - The payload no longer opens on a blank line when only secondary sections are present.
backnotprop
force-pushed
the
fix/folder-annotate-duplicate-export
branch
from
October 5, 2026 02:06
b45b3fc to
4923927
Compare
…g the root export's pre-#1696 order
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Target: 0.28.1
Bug
In a folder annotate session, every comment on the open document was exported twice: once under
# Folder Feedbackand again under# Linked Document Feedback … documents referenced in the plan. It goes back to at least 0.27.25 and affects every host, because the export is built in the browser.Root cause
useLinkedDoc.getDocAnnotations()includes the active document's live state (the panel and the counts need it). Both export paths (annotationsOutputandgetCurrentFeedbackPayload→buildCompleteAnnotateFeedback) exported the live annotations under the session heading, then exportedgetDocAnnotations()again under the linked heading. A folder session always has a document open, so it always duplicated. The same mistake hit any session submitted while a linked document was open. In plan review that also dropped the plan's own comments, because the stashed plan has nosourceFilePath.Two more defects sat in the same path:
exportLinkedDocAnnotations, a separate and weaker renderer. It sorted byblockId.localeCompare, so block-10 came before block-2. It had no[In diff content]label, no quick-label heading, tip or Label Summary, and no reply threading.getDocAnnotations()overwrote that copy's cache entry with the stashed root. Its comments then vanished from the export while a linked doc was open. Main printed "User reviewed the document and has no feedback." for that case, shown below.Fix
One split for every export path.
useLinkedDoc.getFeedbackDocuments()is additive. It returns{ root, documents }:rootis the stashed root while a linked document is open (otherwise null), anddocumentsholds every other document once, by path.getDocAnnotations()is unchanged.One section resolver.
packages/editor/feedbackDocuments.ts→resolveFeedbackSections()is used by bothannotationsOutputand the submitted payload.# Folder Document Feedback, with "The following feedback is on files in the reviewed folder, grouped by file." The plan and file headings are unchanged.One renderer.
renderAnnotationEntriesinparser.tsis now whatexportAnnotationsuses, andexportLinkedDocAnnotationsuses it too, one heading level down.sortAnnotationsInDocumentOrdersorts by block position; without blocks it compares ids numerically.labelSummaryBlockis shared as well.No leading blank line. The payload no longer starts with
\nwhen only secondary sections are present.Plain plan review, linked-doc output: what changes
Plain comments, deletions and globals are byte-identical to main. This is pinned by a test that passes against main's
parser.ts. Linked documents only gain fidelity they were missing:[In diff content]instead of a line label (they never had a real one).[Label] Feedback on: …plus the tip, and the document gets a### Label Summary. Before, they printed as a plain> Labelquote.**Replies:**instead of being numbered as separate entries.Counts:
feedbackAnnotationCountis unchanged and now matches what is emitted. Each section's "N pieces" line counts what it prints, the same way the root export counts.Feedback archive: the record's
annotationsarray (the client'sallAnnotations) was never duplicated. The record'sfeedbacktext was, and now isn't (see below).Tests
packages/ui/utils/parser.linkedDocParity.test.ts(new): a 12-block document with comments on block 10 and block 2, a reply, a diff comment, and a quick label with a tip.[In diff content], the quick-label heading, tip and### Label Summary.packages/editor/feedbackDocuments.test.ts:# Folder Document Feedback(no leading newline).useLinkedDoc.crossFile.test.tsx(already on the DOM_TESTS list):back().openLoaded(ROOT_PATH)from the root, add s1, open another doc, go back.getFeedbackDocuments()now carries s1 anddocAnnotationCountis 1.bun test(6039 pass),bun run typecheck, and the DOM suites fromtest.yml(1465 pass; the Windows-step server tests in that file are excluded, since they are not DOM suites) are all green.Headless verification
Built with
bun run --cwd apps/review build && bun run build:hook, for main (b6702ed) and for this branch. Each run usedbun apps/hook/server/index.ts annotate <dir>/ --jsonwith a tempPLANNOTATOR_DATA_DIRand headless Chromium (--use-mock-keychain --password-store=basic).Folder session. A seed session opened
a.md(12 paragraphs) to record version history. Paragraph 4 was then edited. In the second session the reviewer:Replies have no human-UI producer (only agents create them, through WebMCP or the external-annotations API), so the export tests cover them rather than the browser run.
Before (main): every comment appears twice. The second copy is in string order (line 21 before line 5) and has lost its diff label and quick-label heading.
Archive record: 5 annotations; the paragraph-11 comment and the diff comment each appear in
feedback2×.After (this branch): each comment appears once, in document order, at full fidelity, and the payload opens on its heading.
Archive record: 5 annotations; the paragraph-11 comment and the diff comment each appear 1×.
Self-link (file session).
index.mdlinks to itself ([Home](index.md)) and toother.md. The reviewer clicked Home, commented on the opened copy, clicked Other, and submitted.Before (main):
After:
Not changed / follow-up
In the self-copy state,
getDocAnnotations()(the panel's All files view) still shows the stashed root under that path instead of the copy, though the counts include the copy. The export no longer depends on it. Fixing the panel is a separate change.