fix(web): anchor markdown comments to real raw offsets for formatted selections

[omni-resolve-agent]

Summary

replace the proportional fallback with a markdown-syntax-tolerant matcher that finds the true raw span (content chars must match exactly and in order; inline markup, link/image tails, and block/list/table/fence markup are skipped), and store an empty (0,0) placeholder span - the HTML viewer convention, anchor_content stays the primary locator - when the selection genuinely cannot be located; commit 73ca157 on fix/omni-2882

Root Cause

computeSelectionData() in web/src/shell/TipTapEditorHelpers.ts located the rendered selection in the raw markdown with a verbatim indexOf; when the selection spanned syntax the renderer strips (inline marks, link targets, list/table/heading markup, block separators) it fell back to a proportional length-ratio estimate, storing start_index/end_index that point at different text than the anchored selection

Validation

• Resolve workflow: https://github.com/omnigent-ai/omnigent-internal/actions/runs/34758427375
• Bug: https://linear.app/omnigent/issue/OMNI-2882/bug-markdown-comments-store-raw-offsets-pointing-at-different-text

Demo

Recordings: none — corrected outcome is purely textual (stored start_index/end_index read back from the comments API); the visible journey is identical before and after because the highlight always re-located from anchor_content - before/after stored spans are quoted in the evidence and belong in the PR Demo section

Issues

Closes #4598
Resolves OMNI-2882

[github-actions]

UI Preview for this PR has been removed.

[omnigent-ci]

Polly AI Review

Blocking issues

None. Two candidate concerns were investigated against the full source and downstream consumers; both come out non-blocking:

• Wrong-span false positives from context-free skipping — INLINE_SKIP/GAP_SKIP treat * _ ~ \ [ ] ! < > # | - + : =as invisible regardless of Markdown context, so in theory an anchor could match raw text where those characters render literally (e.g.a!b, foo-bar). This is real but **not blocking**: (1) the tolerant matcher runs *only* after a verbatim indexOffails, i.e. only when the rendered text genuinely differs from raw, and a bare!/-that renders literally would have matched verbatim; (2)start_index/end_indexare non-authoritative hints —anchor_contentis the primary locator, andFileViewer.tsxre-anchors and *rewrites* the offset fromanchor_content` on load, so a slightly-off span self-repairs; (3) it is strictly no worse than the proportional guess it replaces.

• All unlocatable comments collapse to (0,0) — verified this does not break persistence (comments.py validates end_index >= start_index >= 0, which (0,0) satisfies), the agent tool (renders from anchor_content), or highlight re-location. It matches the existing HTML-viewer convention (HtmlCommentViewer.tsx). See the note below for the one residual edge.

Security vulnerabilities

None. No new I/O; both new regexes are literals (no regex-from-input); loop work is hard-capped (i <= ri + 1024, close - ri <= 1024, \d{1,9}). The worst-case O(n·m) retry in findRawSpanTolerant operates on same-origin, already-in-memory editor content (the file the user is editing), not attacker-controlled network input, so there is no meaningful ReDoS/DoS surface.

Non-blocking notes

• (0,0) dedup collision widens slightly. CommentsPanel.tsx and MarkdownCommentPlugin.tsx identify draft comments by (start_index, end_index). Two unlocatable comments in the same file now both sit at (0,0) and collide (wrong card could highlight, or the "already exists" check misfire), where the old proportional hints were usually distinct. Confined to the transient draft path — saved comments disambiguate by comment_id — but the collision surface is genuinely a touch wider than before. Worth a comment or a tie-break.
• GAP_SKIP over-skip. Literal #|-+:= adjacent to a whitespace gap can be consumed as markup, yielding a slightly-off hint or a (0,0) fallback. Bounded by the anchor_content re-anchor; a one-line comment acknowledging the heuristic's limits would help future readers.
• Shared sticky regexes FENCE_RE/ORDERED_RE reset lastIndex immediately before each synchronous .test(), so they're correct today, but the module-level mutable state is a smell that would break under any future reentrant/async use. A local const per call (or a note on the invariant) would harden it.
• Test coverage gap: tests pin the whitespace non-fusing case ("ab" vs "a b") but not the punctuation-as-whitespace false-positive cases, duplicate true/false candidates, or the (0,0) collision path.

Approach

Sound. Two-stage verbatim-then-tolerant matching that stores the true raw span or an inert (0,0) placeholder is a strict improvement over a proportional estimate that confidently pointed at unrelated text, and it correctly leans on anchor_content as the authoritative locator. The one materially stronger alternative — context-aware Markdown token recognition rather than a character-class skip set — is more robust but disproportionate here given offsets are only hints; the heuristic is a reasonable fit for existing repo conventions. The new tests were hand-traced and are self-consistent with the implementation (including the subtle has **formatted, table, list, and \r\n cases).

Summary

A well-targeted fix that replaces a wrong-by-design proportional offset with a syntax-tolerant matcher and a safe placeholder, resting on the verified fact that downstream code treats anchor_content as authoritative and re-anchors offsets on load. No blocking correctness or security issues. The residual items — the widened (0,0) draft-dedup collision, GAP_SKIP over-skip, and shared sticky-regex state — are low-severity hardening notes, not merge blockers. No visual demonstration is required: the outcome is purely the stored offset value, and the highlight re-locates identically from anchor_content before and after, as the author correctly explained.

---

_{Automated review by Polly · workflow run}

[github-actions]

@omni-resolve-agent[bot] this PR was closed by a maintainer. If you think that was a mistake, reply here and ask them to reopen it. /reopen only undoes automated closes. See CONTRIBUTING.md.