Skip to content

Commit 91f57ad

Browse files
authored
feat(ui): highlight external SSE annotations inline on plan text (#511)
* feat(ui): highlight external SSE annotations inline on plan text External annotations posted via /api/external-annotations previously appeared in the sidebar only. They now highlight the matching `originalText` span in the rendered plan, giving tools (linters, agents) an optional way to attach feedback to specific phrases — while `GLOBAL_COMMENT` (or any annotation without `originalText`) still degrades to sidebar-only. Implementation is a new focused hook that drives the Viewer's existing imperative `applySharedAnnotations` / `removeHighlight` API, reusing the same DOM text-search path that share-URL restoration uses. No protocol change, no schema change to `transformPlanInput`, and App.tsx gains only a single hook call. The hook tracks applied ids with a type+originalText fingerprint so SSE updates correctly remove+reapply, clears its bookkeeping only on plan markdown change (where blocks re-render), and early-returns (preserving state) while diff view or a linked doc overlay is active so SSE removals arriving under those conditions still reconcile when the hook re-enables. For provenance purposes, this commit was AI assisted. * fix(ui): repaint external SSE highlights after share import, clean dead code Addresses three review findings on the external annotation highlight hook: - Share-import wipe: `importFromShareUrl` merges annotations without changing `markdown`, so our `planKey` stayed stable. The share-apply effect in App.tsx calls `clearAllHighlights()` to reset the DOM before applying imported annotations — which also wiped live external SSE highlights. The hook believed they were still painted and never re-drove them, leaving sidebar entries with no visible highlight until the next SSE event. Fix: hook now exposes a `reset()` that clears its applied-set and re-runs the main effect via a counter; App.tsx calls it right after `clearAllHighlights()` in the share-apply path. - Removed a dead `nextIds.has(a.id)` guard inside the apply timer callback. `toAdd` is computed as a subset of `eligible`, and `nextIds` was built from `eligible.map(.id)`, so the guard was vacuously always true. - Removed the stable `viewerRef` from the main effect's dep array; React ref objects have stable identity for the component's lifetime so it was noise. Added a comment noting the intentional omission. For provenance purposes, this commit was AI assisted. * docs(ui): note reset() in useExternalAnnotationHighlights header comment For provenance purposes, this commit was AI assisted.
1 parent b7af16a commit 91f57ad

3 files changed

Lines changed: 123 additions & 4 deletions

File tree

bun.lock

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/editor/App.tsx

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import { useAnnotationDraft } from '@plannotator/ui/hooks/useAnnotationDraft';
5353
import { useArchive } from '@plannotator/ui/hooks/useArchive';
5454
import { useEditorAnnotations } from '@plannotator/ui/hooks/useEditorAnnotations';
5555
import { useExternalAnnotations } from '@plannotator/ui/hooks/useExternalAnnotations';
56+
import { useExternalAnnotationHighlights } from '@plannotator/ui/hooks/useExternalAnnotationHighlights';
5657
import { useFileBrowser } from '@plannotator/ui/hooks/useFileBrowser';
5758
import { isVaultBrowserEnabled } from '@plannotator/ui/utils/obsidian';
5859
import { isFileBrowserEnabled, getFileBrowserSettings } from '@plannotator/ui/utils/fileBrowser';
@@ -386,6 +387,16 @@ const App: React.FC = () => {
386387
const { editorAnnotations, deleteEditorAnnotation } = useEditorAnnotations();
387388
const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations<Annotation>({ enabled: isApiMode });
388389

390+
// Drive DOM highlights for SSE-delivered external annotations. Disabled
391+
// while a linked doc overlay is open (Viewer DOM is hidden) and while the
392+
// plan diff view is active (diff view has its own annotation surface).
393+
const { reset: resetExternalHighlights } = useExternalAnnotationHighlights({
394+
viewerRef,
395+
externalAnnotations,
396+
enabled: isApiMode && !linkedDocHook.isActive && !isPlanDiffActive,
397+
planKey: markdown,
398+
});
399+
389400
// Merge local + SSE annotations, deduping draft-restored externals against
390401
// live SSE versions. Prefer the SSE version when both exist (same source,
391402
// type, and originalText). This avoids the timing issues of an effect-based
@@ -473,10 +484,13 @@ const App: React.FC = () => {
473484
viewerRef.current?.clearAllHighlights();
474485
viewerRef.current?.applySharedAnnotations(pendingSharedAnnotations.filter(a => !a.diffContext));
475486
clearPendingSharedAnnotations();
487+
// `clearAllHighlights` wiped live external SSE highlights too;
488+
// tell the external-highlight bookkeeper to re-apply them.
489+
resetExternalHighlights();
476490
}, 100);
477491
return () => clearTimeout(timer);
478492
}
479-
}, [pendingSharedAnnotations, clearPendingSharedAnnotations]);
493+
}, [pendingSharedAnnotations, clearPendingSharedAnnotations, resetExternalHighlights]);
480494

481495
const handleTaterModeChange = (enabled: boolean) => {
482496
setTaterMode(enabled);
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
import { useCallback, useEffect, useRef, useState } from 'react';
2+
import type { Annotation } from '../types';
3+
import { AnnotationType } from '../types';
4+
import type { ViewerHandle } from '../components/Viewer';
5+
6+
/**
7+
* Bridges SSE-delivered external annotations into the Viewer's imperative
8+
* highlight API so tools can POST annotations with `originalText` and have
9+
* them highlight real spans of the rendered plan.
10+
*
11+
* The Viewer's `applySharedAnnotations` already searches the DOM for
12+
* `originalText` and dedupes against already-applied marks, so this hook
13+
* just needs to drive it when the external list changes.
14+
*
15+
* - Annotations without `originalText` (or `GLOBAL_COMMENT`) stay sidebar-only.
16+
* - Annotations with `diffContext` are skipped (diff view owns those).
17+
* - On plan markdown change the applied set is cleared so re-rendered blocks
18+
* get re-highlighted.
19+
* - Callers can invoke the returned `reset()` to force a full re-apply — used
20+
* by the share-import path in App.tsx after it calls `clearAllHighlights()`,
21+
* which would otherwise leave our bookkeeping stale against a wiped DOM.
22+
* - Disabled state no-ops WITHOUT clearing the applied set. This preserves the
23+
* bookkeeping while the Viewer DOM is hidden (diff view / linked doc) so that
24+
* any SSE removals that arrive while hidden are correctly reconciled when the
25+
* hook re-enables.
26+
*/
27+
export function useExternalAnnotationHighlights(params: {
28+
viewerRef: React.RefObject<ViewerHandle | null>;
29+
externalAnnotations: Annotation[];
30+
enabled: boolean;
31+
/** Bump to force a full re-apply (e.g. plan markdown changed and blocks re-rendered). */
32+
planKey: string;
33+
}): { reset: () => void } {
34+
const { viewerRef, externalAnnotations, enabled, planKey } = params;
35+
36+
// Tracks annotation IDs currently materialized as DOM highlights, along
37+
// with a fingerprint so updates trigger remove+reapply.
38+
const appliedRef = useRef<Map<string, string>>(new Map());
39+
40+
// Bumped to force the main effect to treat every current external as a
41+
// fresh application target — used by `reset()` below.
42+
const [resetCount, setResetCount] = useState(0);
43+
44+
// Clear tracking when plan content changes — the Viewer re-parses blocks
45+
// and wipes marks, so our bookkeeping is stale.
46+
useEffect(() => {
47+
appliedRef.current.clear();
48+
}, [planKey]);
49+
50+
useEffect(() => {
51+
if (!enabled) return;
52+
53+
const viewer = viewerRef.current;
54+
if (!viewer) return;
55+
56+
const eligible = externalAnnotations.filter(
57+
a => a.type !== AnnotationType.GLOBAL_COMMENT && !a.diffContext && a.originalText,
58+
);
59+
const applied = appliedRef.current;
60+
61+
// Removals: previously applied but no longer present, or fingerprint changed.
62+
const toRemove: string[] = [];
63+
for (const [id, fp] of applied) {
64+
const match = eligible.find(a => a.id === id);
65+
if (!match || fingerprint(match) !== fp) {
66+
toRemove.push(id);
67+
}
68+
}
69+
toRemove.forEach(id => {
70+
viewer.removeHighlight(id);
71+
applied.delete(id);
72+
});
73+
74+
// Additions: eligible but not yet applied (includes re-adds from updates).
75+
const toAdd = eligible.filter(a => !applied.has(a.id));
76+
if (toAdd.length === 0) return;
77+
78+
// Paint delay matches the existing draft/share restore pattern —
79+
// ensures blocks are mounted before we walk the DOM.
80+
const timer = setTimeout(() => {
81+
const v = viewerRef.current;
82+
if (!v) return;
83+
v.applySharedAnnotations(toAdd);
84+
toAdd.forEach(a => applied.set(a.id, fingerprint(a)));
85+
}, 100);
86+
87+
return () => clearTimeout(timer);
88+
// viewerRef is a stable ref object and intentionally omitted from deps.
89+
}, [externalAnnotations, enabled, planKey, resetCount]);
90+
91+
// Forget everything we've tracked and force a full re-apply on the next
92+
// effect run. Callers invoke this after an external action has wiped the
93+
// Viewer DOM out from under us (e.g. `clearAllHighlights()` during share
94+
// import) so live externals get repainted.
95+
const reset = useCallback(() => {
96+
appliedRef.current.clear();
97+
setResetCount(c => c + 1);
98+
}, []);
99+
100+
return { reset };
101+
}
102+
103+
function fingerprint(a: Annotation): string {
104+
return `${a.type}\u0000${a.originalText}`;
105+
}

0 commit comments

Comments
 (0)