Skip to content

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

Draft
centraldogma99 wants to merge 1 commit into
backnotprop:mainfrom
centraldogma99:fix/annotation-quote-markdown-anchoring
Draft

fix(ui): anchor annotation quotes that carry source markdown#1422
centraldogma99 wants to merge 1 commit into
backnotprop:mainfrom
centraldogma99:fix/annotation-quote-markdown-anchoring

Conversation

@centraldogma99

Copy link
Copy Markdown

Draft: the behavior change is verified, but the scope decision at the end (how far to chase the remaining case) is worth your call before this is ready.

You have probably hit this

You POST a few annotations into a session from a tool or an agent, and most of them come back sidebar-only. No error, no warning in the UI — just comments that never found their sentence. On the session that prompted this, 2 of 11 anchored.

The cause is not text drift. Every one of those 11 quotes was present in the document, exactly once. They failed because they were accurate.

The mismatch

An external annotation's originalText is its only address. POST /api/external-annotations carries no position data (transformPlanInput pins blockId: "external" and both offsets to 0), so applyAnnotationsInternal skips the metas branch entirely and restores through findTextInDOM(originalText) — a verbatim search of the rendered DOM. That is the documented contract, and it is a reasonable one: an external tool cannot compute browser coordinates, so the quote is the address.

But the two ends of that contract look at different representations of the same document:

what it holds
where a tool gets the quote GET /api/plan → the document's markdown`config.ts` still has its backticks
where the quote is matched the rendered DOM → config.ts, no backticks anywhere

Nothing reconciles them, so quoting faithfully is what breaks it. In the sample, the 9 misses were exactly the quotes containing a backtick, an asterisk, or a table pipe; the 2 hits were the only two lines with no inline markup at all.

The change

findTextInDOM already has this shape — literal first, then a retry with the renderer's plain-text transform (emoji shortcodes, smart punctuation) when that misses. This adds two more tiers to the same cascade, least destructive first:

  1. stripInlineMarkdown — code spans, emphasis, strikethrough, links, images, wiki links.
  2. stripTableCellDelimiters — the above plus | and its padding, because a row renders as adjacent <td>s with no text between them: | a | b | reads back as ab, not a b.

Same session: 2 → 10 of 11. A quote that matches the page verbatim returns on tier 1 and never reaches the new code.

The transform is applied to the needle, never the haystack. The search has to end in a DOM Range (which text node, which offset) so a highlight can be painted, and those coordinates only exist in the real DOM — so the rendered text is what stays fixed and the quote is what moves.

Two things worth reviewing closely

Code spans are parked, not unwrapped in place. Their content is literal to the renderer, so ** and __ inside one are ordinary characters. Unwrapping backticks first would hand that content to the emphasis rules — `a__b__c` came out as abc in my first attempt, which a test caught. Spans go behind a sentinel and are restored last.

The new tiers refuse to guess. Stripping shortens the needle and makes it more generic, and searchOnce takes the first hit — so a quote that becomes ambiguous once stripped would silently highlight the wrong paragraph. A highlight on text the comment was never about is worse than no highlight, so tiers 3 and 4 anchor only when the transformed needle is unique. The literal tiers keep first-match-wins; their behavior is untouched.

Known limitation (the scope question)

The 11th quote still misses, for a different reason. Its `config.ts` renders as an ambiguous code-file link, which injects a candidate-count badge into the text stream:

source    | ... | `config.ts` — ... |
rendered    ...   config.ts7 — ...
                           ^ <sup>7</sup>, text the source never had

That is on the haystack side — the renderer added characters, rather than removing syntax — so no transform of the quote can reach it. Fixing it means filtering UI adornments out of the rendered text, which would change what the literal tiers see too. I stopped here rather than widen the blast radius; say the word if you'd rather that were in scope.

Verification

  • New unit tests for both pure transforms (inlineTransforms.test.ts), including the code-span protection case that caught the first implementation.
  • New DOM tests (useAnnotationHighlighter.test.tsx) covering all four branches: inline-markdown quote anchors, table-row quote anchors, ambiguous-after-strip refuses, and a verbatim quote of duplicated text still anchors (the unchanged literal path). Mutation-checked — relaxing the uniqueness demand fails exactly the refusal test.
  • bun run typecheck clean; bun test 4036 pass / 0 fail across 426 files.
  • Verified in a browser against the session that prompted this: replayed the same 12 annotations, 2 anchored before, 10 after.

A note on the test run: three apps/pi-extension/server/network.test.ts port cases fail on bun 1.4.0 (server.listenerCount("error") after a failed bind) and pass on 1.3.14, the version CI pins. Reproduced on a clean tree, so it is unrelated to this change — but it is a real forward-incompatibility someone will hit again.

Per CONTRIBUTING.md: fork → PR, dual Apache-2.0/MIT.

An external annotation's `originalText` is its only address: the POST API
carries no position data, so `useAnnotationHighlighter` restores it by
searching the rendered DOM for that quote verbatim. But the quote's
natural source is `GET /api/plan`, which returns the document's MARKDOWN
— where a code span still has its backticks and emphasis its asterisks.
The rendered DOM has none of that syntax, so a faithful quote of any
line containing inline markup matches nothing and the annotation
degrades to sidebar-only. Following the documented contract exactly is
what makes it fail.

Measured on a real session: 2 of 11 quotes anchored, and the 9 that
missed were precisely the ones whose quote contained a backtick, an
asterisk, or a table pipe.

`findTextInDOM` already retries with the renderer's plain-text transform
when the literal search misses, so this adds two more tiers to that same
cascade: inline markdown (code spans, emphasis, strikethrough, links,
wiki links), then table cell delimiters. Same session: 2 -> 10.

Two things the tiers are careful about.

Code-span content is literal to the renderer, so emphasis delimiters
inside one must survive; unwrapping backticks first would expose them to
the emphasis rules (`a__b__c` in a code span became `abc`). Spans are
parked behind a sentinel and restored last.

Stripping shortens the needle and the search takes the first hit, so a
quote that becomes ambiguous once stripped would silently highlight the
wrong paragraph — worse than not highlighting at all. The new tiers
anchor only when the transformed needle is unique. The literal tiers
keep first-match-wins, unchanged.

Known limitation, out of scope here: a quote whose code span renders as
an AMBIGUOUS code-file link still misses, because that link injects a
candidate-count `<sup>` into the text stream. That is text the source
never had, on the haystack side; reaching it means stripping UI
adornments out of the rendered text, which would change the literal
tiers too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lz3aBzrpHocVFU94YN7QBr
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