fix(ui): anchor annotation quotes that carry source markdown

packages/ui/hooks/useAnnotationHighlighter.test.tsx

@@ -353,3 +353,126 @@ describe('useAnnotationHighlighter math annotations', () => {
     host.remove();
   });
 });
+
+// --- Source-markdown quotes anchoring against rendered text -----------------
+
+function QuoteHarness({ annotations }: { annotations: Annotation[] }) {
+  const containerRef = useRef<HTMLDivElement | null>(null);
+  const hook = useAnnotationHighlighter({
+    containerRef,
+    annotations: [],
+    selectedAnnotationId: null,
+    mode: 'comment',
+    onAddAnnotation: () => {},
+  });
+
+  React.useEffect(() => {
+    // Mirrors the external-annotation restore path: no startMeta/endMeta, so
+    // the quote is the only address the annotation has.
+    const timer = setTimeout(() => hook.applyAnnotations(annotations), 0);
+    return () => clearTimeout(timer);
+  }, [annotations]);
+
+  return (
+    <div ref={containerRef}>
+      {/* What the renderer produces: no backticks, no asterisks, and table
+          cells with nothing between them. */}
+      <p data-block-id="block-1">
+        The <code>config.ts</code> switch is <strong>required</strong> here.
+      </p>
+      <p data-block-id="block-2">A duplicated phrase.</p>
+      <p data-block-id="block-3">A duplicated phrase.</p>
+      <table>
+        <tbody>
+          <tr data-block-id="block-4">
+            <td>WP5</td>
+            <td>mcpAuth.ts</td>
+          </tr>
+        </tbody>
+      </table>
+    </div>
+  );
+}
+
+const quoteAnnotation = (id: string, originalText: string): Annotation => ({
+  id,
+  blockId: 'external',
+  startOffset: 0,
+  endOffset: 0,
+  type: AnnotationType.COMMENT,
+  text: 'a remark',
+  originalText,
+  createdA: 1,
+});
+
+async function mountQuotes(annotations: Annotation[]) {
+  const host = document.createElement('div');
+  document.body.appendChild(host);
+  const root = createRoot(host);
+  await act(async () => {
+    root.render(<QuoteHarness annotations={annotations} />);
+  });
+  await act(async () => {
+    await new Promise(resolve => setTimeout(resolve, 20));
+  });
+  return {
+    host,
+    cleanup: () => {
+      act(() => root.unmount());
+      host.remove();
+    },
+  };
+}
+
+describe.if(hasDom)('useAnnotationHighlighter — source-markdown quotes', () => {
+  test('anchors a quote whose inline markdown the renderer consumed', async () => {
+    // The quote is verbatim from the SOURCE, where the code span still has
+    // its backticks and the emphasis its asterisks. Before the strip tiers
+    // this found nothing and the annotation stayed sidebar-only.
+    const { host, cleanup } = await mountQuotes([
+      quoteAnnotation('a1', 'The `config.ts` switch is **required** here.'),
+    ]);
+    try {
+      expect(host.querySelectorAll('[data-bind-id="a1"]').length).toBeGreaterThan(0);
+    } finally {
+      cleanup();
+    }
+  });
+
+  test('anchors a table row, whose cells render with nothing between them', async () => {
+    const { host, cleanup } = await mountQuotes([
+      quoteAnnotation('a2', '| WP5 | `mcpAuth.ts` |'),
+    ]);
+    try {
+      expect(host.querySelectorAll('[data-bind-id="a2"]').length).toBeGreaterThan(0);
+    } finally {
+      cleanup();
+    }
+  });
+
+  test('refuses to guess when the stripped quote is no longer unique', async () => {
+    // Stripping shortens the needle, and the search takes the first hit — so a
+    // quote that becomes ambiguous must anchor nowhere rather than highlight
+    // whichever paragraph happens to come first.
+    const { host, cleanup } = await mountQuotes([
+      quoteAnnotation('a3', 'A **duplicated** phrase.'),
+    ]);
+    try {
+      expect(host.querySelectorAll('[data-bind-id="a3"]').length).toBe(0);
+    } finally {
+      cleanup();
+    }
+  });
+
+  test('still anchors a quote that matches the page verbatim', async () => {
+    // The literal tier is unchanged: first-match-wins, no uniqueness demand.
+    const { host, cleanup } = await mountQuotes([
+      quoteAnnotation('a4', 'A duplicated phrase.'),
+    ]);
+    try {
+      expect(host.querySelectorAll('[data-bind-id="a4"]').length).toBeGreaterThan(0);
+    } finally {
+      cleanup();
+    }
+  });
+});

packages/ui/hooks/useAnnotationHighlighter.ts

@@ -11,7 +11,7 @@ import type { Annotation, EditorMode, ImageAttachment } from '../types';
 import { AnnotationType } from '../types';
 import type { QuickLabel } from '../utils/quickLabels';
 import { getIdentity } from '../utils/identity';
-import { transformPlainText } from '../utils/inlineTransforms';
+import { stripInlineMarkdown, stripTableCellDelimiters, transformPlainText } from '../utils/inlineTransforms';
 
 // --- Exported state types ---
 
@@ -469,16 +469,61 @@ export function useAnnotationHighlighter({
       return null;
     };
 
-    // First try the literal text. If that misses, re-try with the same
-    // transform the renderer applies to plain text (emoji shortcodes +
-    // smart punctuation) so annotations made before those transforms
-    // shipped can still re-bind to their target after reload.
+    // The tiers below make the needle shorter and more generic, and
+    // `searchOnce` takes the FIRST hit — so a stripped quote that now appears
+    // in two places would silently highlight the wrong one. A highlight on
+    // text the comment was never about is worse than no highlight, so the
+    // permissive tiers refuse to guess: they anchor only when the transformed
+    // needle is unique. The literal tiers keep first-match-wins, which is the
+    // behavior that already shipped.
+    const searchIfUnique = (needle: string): Range | null => {
+      const container = containerRef.current;
+      if (!needle || !container) return null;
+      const collapse = (v: string) => v.replace(/\s+/g, ' ').trim();
+      const haystack = collapse(container.textContent || '');
+      const target = collapse(needle);
+      if (!target) return null;
+      let count = 0;
+      let from = haystack.indexOf(target);
+      while (from !== -1 && count < 2) {
+        count += 1;
+        from = haystack.indexOf(target, from + target.length);
+      }
+      if (count !== 1) return null;
+      return searchOnce(needle);
+    };
+
+    // Tiers, least destructive first. Each re-tries the search with one more
+    // of the transforms the renderer applies, so a quote taken from the
+    // document's SOURCE can still find its RENDERED target. A quote that
+    // matches the page verbatim returns on the first tier and never reaches
+    // the rest.
     const direct = searchOnce(searchText);
     if (direct) return direct;
 
+    // Emoji shortcodes + smart punctuation, so annotations made before those
+    // transforms shipped still re-bind to their target after reload.
     const transformed = transformPlainText(searchText);
     if (transformed !== searchText) {
-      return searchOnce(transformed);
+      const viaTransform = searchOnce(transformed);
+      if (viaTransform) return viaTransform;
+    }
+
+    // Inline markdown the renderer consumes. An external annotation quotes
+    // the markdown a tool read from the server, where `code` still carries
+    // its backticks; the DOM has none of that syntax.
+    const stripped = stripInlineMarkdown(searchText);
+    if (stripped !== searchText) {
+      const viaStrip = searchIfUnique(stripped);
+      if (viaStrip) return viaStrip;
+    }
+
+    // Last: a table row, whose cells render as adjacent elements with nothing
+    // between them. Deleting the padding around a `|` mangles ordinary prose,
+    // which is why it is only reached once everything else has failed.
+    const withoutCells = stripTableCellDelimiters(searchText);
+    if (withoutCells !== stripped) {
+      return searchIfUnique(withoutCells);
     }
 
     return null;

packages/ui/utils/inlineTransforms.test.ts

@@ -1,5 +1,5 @@
 import { describe, test, expect } from 'bun:test';
-import { transformPlainText } from './inlineTransforms';
+import { stripInlineMarkdown, stripTableCellDelimiters, transformPlainText } from './inlineTransforms';
 
 describe('transformPlainText — emoji shortcodes', () => {
   test('replaces known shortcode with unicode emoji', () => {
@@ -46,3 +46,54 @@ describe('transformPlainText — smart punctuation', () => {
     expect(transformPlainText("he said 'hi'")).toBe('he said ‘hi’');
   });
 });
+
+describe('stripInlineMarkdown', () => {
+  test('unwraps code spans, which is what an external quote carries', () => {
+    expect(stripInlineMarkdown('`config.ts` — `ENABLED_TOOLS` branch')).toBe(
+      'config.ts — ENABLED_TOOLS branch',
+    );
+    expect(stripInlineMarkdown('``a `b` c``')).toBe('a `b` c');
+  });
+
+  test('unwraps emphasis and strikethrough', () => {
+    expect(stripInlineMarkdown('**R2.** the ~~old~~ __new__ path')).toBe('R2. the old new path');
+    expect(stripInlineMarkdown('***both***')).toBe('both');
+  });
+
+  test('keeps link and image text, drops the target', () => {
+    expect(stripInlineMarkdown('see [the plan](./plan.md)')).toBe('see the plan');
+    expect(stripInlineMarkdown('![a diagram](x.png)')).toBe('a diagram');
+    expect(stripInlineMarkdown('[[Design]] and [[Design|alias]]')).toBe('Design and Design');
+  });
+
+  test('does not unwrap emphasis that lives inside a code span', () => {
+    // Backticks are consumed first, so the underscores in a code span are
+    // literal content and must survive as themselves.
+    expect(stripInlineMarkdown('`a__b__c`')).toBe('a__b__c');
+  });
+
+  test('leaves prose without inline syntax untouched', () => {
+    expect(stripInlineMarkdown('plain sentence, nothing to strip')).toBe(
+      'plain sentence, nothing to strip',
+    );
+  });
+
+  test('leaves table cell delimiters alone', () => {
+    // Separate tier: deleting the padding around a pipe mangles prose, so it
+    // is not part of the general strip.
+    expect(stripInlineMarkdown('| a | b |')).toBe('| a | b |');
+  });
+});
+
+describe('stripTableCellDelimiters', () => {
+  test('renders a row the way adjacent cells concatenate', () => {
+    // `| a | b |` renders as <td>a</td><td>b</td> — no text between the cells,
+    // so the padding has to go with the pipe, not just the pipe.
+    expect(stripTableCellDelimiters('| a | b |')).toBe('ab');
+    expect(stripTableCellDelimiters('| `x.ts` | **WP5** |')).toBe('x.tsWP5');
+  });
+
+  test('includes the inline strip', () => {
+    expect(stripTableCellDelimiters('`a`')).toBe('a');
+  });
+});

packages/ui/utils/inlineTransforms.ts

@@ -39,3 +39,59 @@ function smartypants(s: string): string {
 export function transformPlainText(text: string): string {
   return smartypants(replaceEmoji(text));
 }
+
+/**
+ * Strip the inline markdown syntax the renderer consumes, so a quote taken
+ * from a document's SOURCE can be matched against its RENDERED text.
+ *
+ * An external annotation's `originalText` is a quote, and its natural source
+ * is the markdown a tool read from the server — where `` `config.ts` `` still
+ * carries its backticks. The rendered DOM has none of that syntax, so a
+ * faithful quote of a line containing any inline markup finds nothing and the
+ * annotation degrades to sidebar-only.
+ *
+ * Deliberately conservative and lossy in one direction only: it removes
+ * delimiters, never content. Both helpers are last-resort restore tiers, tried
+ * only after a literal search has already failed, so a quote that matches the
+ * page verbatim never reaches them.
+ */
+export function stripInlineMarkdown(text: string): string {
+  // Code-span content is literal to the renderer, so emphasis delimiters
+  // inside one are ordinary characters and must survive. Unwrapping the
+  // backticks first would expose them to the rules below, so the content is
+  // parked out of reach and restored at the end.
+  const spans: string[] = [];
+  const SENTINEL = '\u0000';
+  const parked = text.replace(/``([\s\S]+?)``|`([^`]+)`/g, (_match, double, single) => {
+    spans.push(double ?? single);
+    return `${SENTINEL}${spans.length - 1}${SENTINEL}`;
+  });
+
+  const stripped = parked
+    .replace(/!\[([^\]]*)\]\([^)]*\)/g, '$1')
+    .replace(/\[\[([^\]|]+)(?:\|[^\]]*)?\]\]/g, '$1')
+    .replace(/\[([^\]]+)\]\([^)]*\)/g, '$1')
+    .replace(/\*\*\*([^*]+)\*\*\*/g, '$1')
+    .replace(/\*\*([^*]+)\*\*/g, '$1')
+    .replace(/__([^_]+)__/g, '$1')
+    .replace(/~~([^~]+)~~/g, '$1');
+
+  return stripped.replace(
+    new RegExp(`${SENTINEL}(\\d+)${SENTINEL}`, 'g'),
+    (_match, index) => spans[Number(index)] ?? '',
+  );
+}
+
+/**
+ * `stripInlineMarkdown` plus the cell delimiters of a table row.
+ *
+ * A row renders as sibling `<td>` elements with no text between them, so the
+ * rendered text of `| a | b |` is `ab` — the pipes AND the padding around them
+ * have to go, not just the pipes. Kept separate from `stripInlineMarkdown`
+ * because deleting the whitespace around a `|` mangles ordinary prose that
+ * happens to contain one; it is only ever worth trying when everything else
+ * has already failed.
+ */
+export function stripTableCellDelimiters(text: string): string {
+  return stripInlineMarkdown(text).replace(/\s*\|\s*/g, '');
+}