mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 11:01:16 +01:00
fix(ui): open focused orphaned document annotation threads (#18950)
## Thinking Path > - Paperclip manages tasks and their saved documents. > - Review threads keep their comments when a document revision removes their selection. > - A supported document/thread link opens the annotation panel and selects the thread. > - The panel then removed every orphaned thread, including that selected target. > - This change keeps the exact focused orphan visible so its history can be read. > - Other unselected orphaned threads keep their existing display policy. ## Linked Issues or Issue Description **What happened?** A direct link to a resolved document annotation opened the panel but omitted its thread after the selection became orphaned. The server still retained the complete review comments. **Expected behavior** The selected thread and complete comments remain readable through the existing deep link. Viewing the history must not reopen, reply to or reanchor the thread. **Steps to reproduce** Create and resolve a review thread. Save a revision that removes its selected text. Open its supported document/thread link in a fresh view. The original panel shows other threads but filters out the requested one. Related #14648 fixes duplicate annotation positions. It does not repair this focused-thread filter. ## What Changed - Allow the exact focused orphaned thread through the existing panel filter. - Recompute that list when focus changes. Preserve the display policy for unrelated orphans. - Add a rendered hash-to-wrapper-to-panel regression with a long first comment, a later resolution note, focus changes and a fresh remount/query client. ## Verification - Untouched-production rendered regression failed because the focused target card was missing:1 failed,18 excluded by selection. - Five related annotation/hash/consumer files passed49/49 tests on this worktree's own locked graph. - UI typecheck, UI build, token gates, module boundaries and whitespace passed. - The rendered regression preserves resolved/orphaned state and confirms no create, reply or status mutation. It reads the full first comment after a fresh remount and hides the old orphan after focus moves away. - API results, Markdown rendering and selection overlay use the existing component-test fixtures. This is a rendered consumer proof, not a full browser/server rebase journey. - Initial dependency/build-prerequisite failures are retained locally. Normal install/shared/SDK preparation resolved them; no unrelated source change was made. - Repository-wide tests/typecheck/build beyond the affected UI were not repeated locally for this two-line presentation fix. Current-head required CI and substantive review remain pending. No map entry or command changes are needed. This restores the existing document-link/read-history contract without adding syntax, access or reattachment behavior. ## Risks A focused orphan now exposes its existing comment card and controls. The existing server authorization and mutation rules still apply. This change does not move the anchor, change resolution state, reveal an unselected orphan, or fetch a thread outside the already-authorized list. No style values or dependency changes are added. ## Model Used OpenAI `gpt-6.1-sol` through Codex assisted with diagnosis, implementation and testing. The exact context-window size is not exposed. Tool use and code execution were enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will require all Paperclip CI gates to be green before merge (currently pending) - [x] I will require Greptile 5/5 with no unresolved findings before merge (currently pending) - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
612d9931a6
commit
cf51936587
2 files changed
+42
-4
No files matched your search
@@ -127,19 +127,17 @@ export function AnnotationPanelBody(props: AnnotationPanelProps) {
|
||||
onReplyAdded: (threadId) => setReplyDrafts((current) => ({ ...current, [threadId]: "" })),
|
||||
});
|
||||
|
||||
// Show every thread that can be anchored in the document (orphaned threads have
|
||||
// lost their anchor). Filters were removed in favour of a single simple list.
|
||||
// Sort in document order (top-to-bottom) — not by recency — so the comment list
|
||||
// stays congruent with the highlights as you scroll the document.
|
||||
const visibleThreads = useMemo(
|
||||
() =>
|
||||
props.threads
|
||||
.filter((thread) => thread.anchorState !== "orphaned")
|
||||
.filter((thread) => thread.anchorState !== "orphaned" || thread.id === props.focusedThreadId)
|
||||
.sort((a, b) =>
|
||||
(a.normalizedStart - b.normalizedStart)
|
||||
|| (a.markdownStart - b.markdownStart)
|
||||
|| (new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime())),
|
||||
[props.threads],
|
||||
[props.threads, props.focusedThreadId],
|
||||
);
|
||||
|
||||
|
||||
|
||||
@@ -484,6 +484,46 @@ describe("IssueDocumentAnnotations", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("opens a resolved orphaned deep-link target after revision and fresh reload without changing history", async () => {
|
||||
const originalBody = "Original review detail that must remain readable after its selection was removed. ".repeat(5)
|
||||
+ "The complete first comment ends here.";
|
||||
const target = makeThread({ id: "orphan-target", status: "resolved", anchorState: "orphaned",
|
||||
currentRevisionId: "rev-5", currentRevisionNumber: 5, resolvedByUserId: "user-1", resolvedAt: new Date() });
|
||||
target.comments = [{ ...target.comments[0]!, body: originalBody },
|
||||
{ ...target.comments[0]!, id: "resolution-note", body: "The saved policy addresses this review." }];
|
||||
mockAnnotationsApi.list.mockResolvedValue([target,
|
||||
makeThread({ id: "unrelated-orphan", status: "resolved", anchorState: "orphaned" }),
|
||||
makeThread({ id: "remaining-thread", anchorState: "stale" })]);
|
||||
const doc = makeDoc({ latestRevisionId: "rev-5", latestRevisionNumber: 5, body: "# Updated policy\n\nThe original selection is gone." });
|
||||
for (let mount = 0; mount < 2; mount++) {
|
||||
const root = createRoot(container);
|
||||
const queryClient = makeQueryClient();
|
||||
const render = (locationHash: string) => root.render(
|
||||
<QueryClientProvider client={queryClient}><Harness doc={doc} locationHash={locationHash} /></QueryClientProvider>,
|
||||
);
|
||||
await act(() => render("#document-plan&thread=orphan-target&comment=comment-1"));
|
||||
await waitFor(() => {
|
||||
const card = container.querySelector('[data-thread-id="orphan-target"][data-focused]');
|
||||
expect(card).not.toBeNull();
|
||||
expect(card!.getAttribute("data-anchor-state")).toBe("orphaned");
|
||||
expect(card!.getAttribute("data-status")).toBe("resolved");
|
||||
expect(card!.textContent).toContain(originalBody);
|
||||
expect(card!.textContent).toContain("The saved policy addresses this review.");
|
||||
});
|
||||
expect(container.querySelector('[data-thread-id="unrelated-orphan"]')).toBeNull();
|
||||
expect(mockAnnotationsApi.create).not.toHaveBeenCalled();
|
||||
expect(mockAnnotationsApi.addComment).not.toHaveBeenCalled();
|
||||
expect(mockAnnotationsApi.updateStatus).not.toHaveBeenCalled();
|
||||
await act(() => render("#document-plan&thread=remaining-thread"));
|
||||
await waitFor(() => {
|
||||
expect(container.querySelector('[data-thread-id="remaining-thread"][data-focused]')).not.toBeNull();
|
||||
expect(container.querySelector('[data-thread-id="orphan-target"]')).toBeNull();
|
||||
});
|
||||
await unmountActiveRoots();
|
||||
queryClient.clear();
|
||||
}
|
||||
});
|
||||
|
||||
it("shows a disabled reason in the panel when the draft is dirty", async () => {
|
||||
mockAnnotationsApi.list.mockResolvedValue([makeThread()]);
|
||||
const root = createRoot(container);
|
||||
|
||||
Reference in new issue
Block a user