fix(CommandPalette): keep astral characters intact when inserting highlight marks

[arsalan507]

Linked issue

Closes #362

Type of change

• Documentation (updates to the documentation or readme)
• Bug fix (a non-breaking change that fixes an issue)
• Enhancement (improving an existing functionality)
• New feature (a non-breaking change that adds functionality)
• Chore (updates to the build process or auxiliary tools and libraries)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Fuse's indices are UTF-16 code-unit offsets, so a region boundary can land between the two surrogates of an astral character; substring() there put <mark> inside the character and orphaned both halves, rendering as �. As prescribed in #362: boundaries that split a surrogate pair are snapped outward before slicing, so the highlight always covers whole characters.

Also handled in the same pass, per the issue's Related note: the single-character region skip was code-unit based — a lone astral character (two units) was highlighted while a lone BMP character was skipped. It now measures code points, so both are skipped.

Two judgement calls, flagged rather than buried:

• isMatched still compares Fuse's original region length against minTokenLength. Snapping changes where the tags go, not whether a region qualifies — widening could flip borderline regions to highlighted, which felt like a separate behavioural decision. Easy to change if you'd rather measure the snapped region.
• After snapping a start leftward it is clamped to the previous region's end: substring() swaps reversed arguments and would duplicate already-emitted text. I believe this is unreachable with Fuse's merged regions, but it is cheap insurance against an interaction between two adjacent snapped regions.

Tests

All new cases fail against unpatched main and pass with the fix. The reachability repro from #362 is pinned with real fuse.js at ContentSearch's shipped defaults and well-formed input on both sides — label "deployment 😀 pipeline", term "deployment 🚀" (the user searched with the wrong emoji) → indices [[0,11],[13,13]], whose first boundary falls between the surrogates. The indices themselves are asserted, so a fuse upgrade that stops producing a mid-pair boundary fails loudly instead of letting the test pass vacuously. Fixtures reuse the range-edge astral set from #365, and the survival sweep counts characters rather than only scanning for lone surrogates.

test/utils/ + test/components/CommandPalette.spec.ts: green, except two pre-existing skill-manifest.spec.ts failures that reproduce on a clean main checkout; eslint and vue-tsc --noEmit clean.

Sync note

The snapping logic is portable to upstream (the same defect exists there); the useTokenSearch surroundings are b24ui-only per .sync/PORTING.md. I'd plan to port this to nuxt/ui after nuxt/ui#6817 settles, since both touch the same functions.

Checklist

• I have linked an issue or discussion.
• I have updated the documentation accordingly.

[IgorShevchik]

Taking this one. I had a competing patch open in #369 and it is now withdrawn — this is smaller, it covers #375 as well, and the reachability test is better than mine was.

That test in particular: driving real fuse.js and then asserting the indices means a fuse upgrade that stops producing a mid-pair boundary fails loudly instead of quietly leaving the case unexercised. Mine used a hand-written fixture and would have gone vacuous without anyone noticing.

Moving the single-character skip after the snap and the clamp is the neater structure, too. It means a region collapsed by clamping falls out through the existing rule rather than needing a separate guard — I ended up adding one (end > start) to my version to get the same property.

Verified before withdrawing mine: 90,112 exhaustive region pairs against your logic — zero empty <mark></mark>, zero text duplication or loss. Single astral characters are skipped exactly as single BMP ones are.

Both of your flagged judgement calls read right to me. Keeping isMatched on the original region length is the conservative choice — widening it would flip borderline regions to highlighted, which is a separate decision. And the start clamp is not dead code: Fuse does merge and sort, so it is unreachable through the search path, but highlight() is a published export and CommandPaletteGroup.postFilter lets a caller supply its own matches. I fuzzed it — 50k cases with Fuse-shaped regions agree with or without the clamp, 30k with a violated contract disagree in 2059. Worth the two lines.

One thing to fix, and it is in my version too, so this is not a mark against yours. A region that lies past the end of the value slips through the single-character skip: end - start is computed on unclamped numbers, while substring() clamps its own arguments. So the region compares as non-empty, slices to nothing, and emits a bare <mark></mark> — the case the skip is meant to prevent:

value  = "The quarterly revenue report is available"   (40 chars)
region = [126, 132]

→ "The quarterly revenue report is available<mark></mark>"

Worse, nextUnhighlightedRegionStartingIndex is then set past the end, so any legitimate region after it collapses too, and the trailing empty mark drives truncateHTMLFromStart into cutting away almost all the real text.

Not reachable through Fuse — convertMaskToIndices builds offsets by scanning the value, so end <= value.length holds by construction. Reachable the same way the clamp case is: a caller passing its own offsets, for instance a server that computed them against an older revision of the text.

Clamping both boundaries to the value closes it:

start = Math.min(Math.max(start, nextUnhighlightedRegionStartingIndex), value.length)
end = Math.min(end, value.length)

On upstream — the plan in your Sync note is the right one, and it settles something this repository got wrong. #362 and #364 are defects of the same class as #339 and equally not b24ui-specific, but they were about to land here only. .sync/PORTING.md §2 now records what in search.ts is genuinely b24ui-only (the useTokenSearch argument) so the boundary is legible when you get to nuxt/ui.

#369 is rescoped to the useTokenSearch coverage it also carried, with no src/ change. #371 adds grapheme-cluster snapping for #364, which subsumes the surrogate snap here — I will rebase it onto this once it lands, keeping your structure.

Thanks again, and sorry for the duplicated effort — that one is on me for not checking whether you were already on it.

---

Generated by Claude Code

[arsalan507]

#371 landed the insertion-boundary fix more completely than this did — the ordering, the nested-region and non-integer guards, and the grapheme snap all go beyond what is here — so I am closing this as superseded rather than leaving it conflicted.

One piece does not carry over: the reachability test. The fixtures on main are recorded fuse output, so a fuse upgrade that changes those offsets would leave the case passing without exercising the boundary. I have sent that as a test-only PR against current main in #385 — take it or leave it, it stands alone.