diff --git a/.sync/PORTING.md b/.sync/PORTING.md index 804dda02..a88c2bcf 100644 --- a/.sync/PORTING.md +++ b/.sync/PORTING.md @@ -187,6 +187,72 @@ material. Reproduce its *intent* in b24ui by editing files under `src/` only. `test/components/CommandPalette.spec.ts`, which fails if the fifth argument stops being forwarded from `processGroupItems`. Port the intent and keep the fifth argument; trust those two specs over this paragraph. +- **`utils/search.ts` cuts on grapheme clusters, at both boundaries.** Fuse + reports `indices` as UTF-16 code-unit offsets and the truncation counter walks + code points, so either boundary can land inside a character the reader sees as + one. Two failure modes, and the second is worse: splitting a surrogate pair + orphans both halves and renders as `οΏ½` (#362 β€” reproduced with real fuse.js at + `ContentSearch`'s options, 8 of 66 live matches over emoji-bearing labels), + while splitting a *cluster* yields a **different** character with nothing to + signal the loss (#364 β€” πŸ‡ΊπŸ‡Έ cut by one code point re-pairs into πŸ‡ΈπŸ‡Ί, a + different country). `createClusterSnapper(value)` moves each boundary off the + straddled cluster before the slice: `generateHighlightedText` uses both ends of + it, `truncateHTMLFromStart` only `.toEnd()`, since a cut has one side. Four + details are load-bearing and easy to drop as noise: + - **The bookkeeping around the snap** β€” the clamps on `start` and `end`, the + `end > start` test, the integer filter and the sort. Every one of them + exists because `substring()` reports no errors: it swaps a range it finds + reversed and clamps one it finds out of range, so an ordering mistake here + surfaces as duplicated text rather than a throw. Snapping alone produces + all four mistakes β€” adjacent regions meeting inside one cluster snap past + each other; a region nested in an earlier one ends behind the cursor; a + region past the end of the value slices to nothing while still comparing + as non-empty, giving a bare ``; and a region the clamps leave + empty does the same. The sort keeps regions in order (unordered, the + clamps swallow each one that arrives after a later one) and puts the + longest of an equal-start pair first, so the outer region is marked whole. + The filter drops non-integer bounds, which `Fuse` never emits but + `postFilter` may: `NaN` compares false against everything, including + `end > start`, and then lands in the cursor where `substring(NaN)` reads as + `substring(0)` and repeats the whole value. + - **The `< U+0300` screen.** Nothing below it can continue a cluster (CRLF + aside, handled explicitly), so ASCII and Latin-1 boundaries never reach + `Intl.Segmenter`. It buys nothing above the floor, which includes Cyrillic + (U+0430) and CJK β€” measured at 979 characters, ASCII is +1% against the + pre-fix cost while both of those are +2.5-2.9 Β΅s. For a product localised + into Russian, treat the screen as covering markup and Latin identifiers, not + the body text. + - **One segmenter view per value.** Building `segment(value)` per boundary + instead of once made a value carrying many match regions linear in the + number of regions rather than paid once: 8.1 ms against 0.6 ms over 1600 + boundaries. + - **The 8192-character guard.** `Segments.containing()` scans: a few Β΅s up to + ~8k, two orders of magnitude worse at 100k. Past the guard only the + surrogate snap applies: `οΏ½` is still prevented, but **every** multi-code-point + cluster loses protection, not just flags β€” the same degradation as a runtime + without `Intl.Segmenter`. What the guard measures is the *marked-up* string, + not the value: truncation runs after the tags are inserted, so the threshold + is crossed 13 characters earlier than a reading of `value.length` suggests. + + Upstream has no equivalent β€” inferred from this file's history, not + re-inspected β€” so replaying upstream's `generateHighlightedText` or + `truncateHTMLFromStart` verbatim reverts it, and the failure is quiet: the + output stays well-formed HTML and only the glyph changes. Note this sits + directly below the `useTokenSearch` divergence and shares the same + `indices.forEach` body; one careless port reverts both. Guarded by + `describe('mark insertion')`, `describe('grapheme clusters')` and + `describe('degraded paths')` in `test/utils/search.spec.ts` β€” every constant + and every branch above was verified by removing it and watching a named test + fail. Two of those fixtures look pointless and are not: the CRLF pair is the + only cluster rule `Intl.Segmenter` never sees, since the fast-path screen + answers it first; and the unpaired-surrogate strings are the only input that + can catch a surrogate range constant being *widened* β€” every other fixture + holds a real pair, which only pins the narrowing direction. + `test/bench/search.bench.ts` covers the two constants no unit test can observe + β€” advisory only, it asserts nothing and CI does not run it. Beware the fixture + trap those tests document: a run of bare emoji modifiers is **one** cluster, + not many, so counting characters with `'\u{1F3FF}'.repeat(n)` asserts the wrong + thing. - **`skills/` is b24ui-authored β€” never replay upstream skill or doc prose into it.** The package was seeded from nuxt/ui's skill, and every defect the #93 audit found was an inherited upstream idiom rather than an ordinary typo: @@ -362,3 +428,4 @@ forward, since every commit between the two would then never be judged. - 2026-08-12 β€” the sync is manual by decision; the automation is removed. Deleted `.sync/PLAN.md` (the dispatcher/porter/on-merge design, its phase plan and its cron) and `.sync/RUNBOOK.md` (an incident playbook whose every row diagnosed one of those workflows). Dropped `sync_enabled` from the ledger β€” a kill-switch for a dispatcher that will not exist reads as "the sync is off" to anyone who finds it, which was already misleading while this file's own procedure ran twelve ports past it β€” and `stats`, Phase-4 telemetry that was never written to (`noop_ratio: 0` against an actual 47/226). Folded the one runbook row that survives manual work into Β§6: a cursor SHA that vanishes under an upstream force-push must be moved to the nearest surviving ancestor with a tracking issue, never skipped forward. Β§6 now spells out the procedure that was previously only implied by the workflows β€” parent-order reconstruction, verbatim diffs, the gate order with `docs:generate` and `deploy.yml`'s env, ledger reconciliation including the last-entry case, and the `behind` rebase. Also corrected `color-map.json`: `warning` mapped to `air-primary-alert`, the same token as `error`, so the table said the two upstream colors were interchangeable; `air-primary-warning` exists and is used 50 times in `src/theme/`. Last reviewed: 2026-08-12. - 2026-08-12 β€” rebuilt `icon-map.json` and gave it a guard (the content of the closed PR #67, verified rather than imported). The map is now *derived*: for every icon key both sides define β€” `src/theme/icons.ts` upstream, `src/runtime/dictionary/icons.ts` here β€” the row is (upstream's lucide name β†’ whatever our dictionary maps that key to), 37 pairs from a 43Γ—39 key intersection at cursor `3dbca02`. Beware the obvious shortcut when re-checking this: the installed `@nuxt/ui@4.8.2` in `node_modules` (pulled in transitively by `nuxtseo-layer-devtools`) is **older than the sync cursor** and is missing keys β€” three separate reviewers read it and concluded `star` was fabricated and the intersection was 36. Read the raw file at the cursor SHA instead. The derivation turned up three errors in the values #67 proposed, each of which resolves to a real icon and so would have failed no import: `i-lucide-rotate-cw` for what upstream calls `i-lucide-rotate-ccw` (`reload`), `i-lucide-circle-check` for `copyCheck`'s `i-lucide-copy-check`, and `i-lucide-refresh-cw`, which no upstream key uses. It also surfaced seven derivable pairs #67 missed β€” `drag`, `panelClose`, `panelOpen`, `star`, `stop`, `copyCheck`, `reload` β€” and, separately, `i-lucide-terminal`, the **only** `i-lucide-*` literal upstream hardcodes under `src/` (`src/theme/prose/code-icon.ts`), which neither the old map nor #67 had even though `prose/CodeIcon.vue` has answered it all along. `error` and `success` gained judgement rows rather than staying unmapped: our `caution` carries a `// this for error` comment, and `copyCheck` already owns the glyph `success` would want. The five entries #67 dropped (`activity`, `arrow-up-to-line`, `house`, `settings`, `user`) are kept β€” they match no key on either side, which is the hardcoded-literal case the map exists for. **Correcting the record on the five values #67 changed** (`check`, `chevronDown`, `chevronUp`, `minus`, `x`): they are wrong because the map must agree with the dictionary, *not* β€” as an earlier draft of this entry claimed β€” because the library never renders them. It does. `Checkbox.vue` renders `main/CheckIcon` and `actions/Minus20Icon`, `Badge.vue` renders `actions/Cross20Icon`, `Button.vue` renders `outline/ChevronDownSIcon`; roughly half of the icon paths under `src/` are hardcoded in components that never read the dictionary, which is #380. That discovery also reshaped the guard: `test/utils/icon-map.spec.ts` allows any icon used anywhere in `src/` rather than only the dictionary's β€” the narrower rule rejected `terminal`, a correct row β€” while separately requiring every *derived* row to equal what its semantic key resolves to. That last check is the one with teeth: without it, pointing `i-lucide-check` at another icon the dictionary genuinely uses passed every other assertion. It guards wrong rows, not stale ones; nothing here notices if upstream renames a default. No `.sync/log/` or ledger entry, since this is not a port of an upstream commit β€” same as #343, #346, #351 and #377. Last reviewed: 2026-08-12. - 2026-08-12 β€” coverage for #363: gave the Β§2 **`highlight()` takes a fifth `useTokenSearch` argument** invariant a guard. It was recorded during the review of #347 but left untested, and the bullet said so. The parameter and the token-search logic around it are b24ui-only β€” `c502157b` added `tokens`/`minTokenLength` and `6743f793` the parameter itself, both against the file's old name `src/runtime/utils/fuse.ts`, both shipped in v2.8.0. Immediately before `c502157b` the function took four parameters and computed no `minTokenLength` at all, which is what marks it as locally authored; the `Upstream:` trailer convention is too sparse (16 of ~3200 commits) to carry an inference either way. The later port `557a5178` renamed `fuse.ts` to `search.ts` and carried the divergence across, so a pickaxe on the current path returns only that port β€” pass `--follow` to see the two commits that introduced it. Until now it had no test at all, so replaying upstream's four-parameter signature would have dropped a shipped feature with nothing going red. Upstream itself has not been re-inspected; treat "upstream has no such parameter" as an inference from b24ui's own history. Last reviewed: 2026-08-12. +- 2026-08-13 β€” fix of #364: the Β§2 **`utils/search.ts` cuts on grapheme clusters** invariant. Cutting by code point is not enough β€” a flag is two regional indicators, a family emoji several joined by ZWJ β€” and slicing inside one yields a *different* character rather than a broken one, with nothing to signal the loss. `Intl.Segmenter`'s `containing()` was chosen on measurement: segmenting the whole string costs 455 Β΅s at 979 characters and 52 ms at 100k, and a fixed Β±64 window is constant-time but wrong β€” a run of flags is longer than the window, so it starts mid-run and re-pairs the indicators, reproducing the very bug (28 disagreements in 1044 probes). Worth recording that the *snap* was the easy half: every defect review turned up was in the bookkeeping around it, and each one reached the user as duplicated or vanished text rather than as an error, because `substring()` swaps a reversed range and clamps an out-of-range one instead of throwing. Four, in the order they were found β€” a region the clamps left empty emitted a bare `` with the highlight lost; a region past the end of the value bypassed that guard, since the comparison did not clamp where `substring()` did; a region nested inside an earlier one ended behind the cursor and had its overlap emitted three times; and a non-integer bound (`NaN` in particular, which compares false against every guard including `end > start`) landed in the cursor, where `substring(NaN)` reads as `substring(0)` and repeats the whole value. All four are guarded, each by a test verified to fail when its guard is removed. That verification is worth repeating whenever this code is touched: it is what showed the CRLF carve-out and the widening direction of all four surrogate range constants to be uncovered β€” nine mutations passing the whole file β€” and both are now fixtured. `indices` are sorted before use β€” a no-op for Fuse, which sorts, merges and integer-bounds them itself, but `highlight()` is a published export and `postFilter` lets a caller supply its own; the tie-break puts the longest of an equal-start pair first so the outer region is marked whole rather than split across two ``s. One clamp went the other way: mutation testing showed `Math.min(…, value.length)` on `start` was unreachable β€” `start` can only exceed the value by exceeding `end`, which is checked β€” so it was removed rather than left as an untested guard. Last reviewed: 2026-08-13. diff --git a/src/runtime/utils/search.ts b/src/runtime/utils/search.ts index 1a5bd4f5..b0898cd0 100644 --- a/src/runtime/utils/search.ts +++ b/src/runtime/utils/search.ts @@ -13,10 +13,134 @@ function escapeHTML(str: string): string { return str.replace(/[&<>"']/g, char => htmlEscapes[char]!) } +const HIGH_SURROGATE_START = 0xD800 +const HIGH_SURROGATE_END = 0xDBFF +const LOW_SURROGATE_START = 0xDC00 +const LOW_SURROGATE_END = 0xDFFF + +// Nothing below U+0300 can continue a grapheme cluster: the range holds no +// combining marks, no joiners, no surrogates and no regional indicators. CRLF is +// the one pair that lives there, and UAX #29 keeps it together. Screening on this +// keeps ASCII and Latin-1 boundaries β€” the overwhelming majority β€” from ever +// reaching the segmenter. +const CLUSTER_CONTINUATION_FLOOR = 0x0300 +const CARRIAGE_RETURN = 0x000D +const LINE_FEED = 0x000A + +// `Segments.containing()` scans, so its cost grows with the value: a few +// microseconds up to ~8k, two orders of magnitude worse at 100k. Search snippets +// are short. Past this length the grapheme snap is skipped and only the +// surrogate-pair snap applies: `οΏ½` is still prevented, but every +// multi-code-point cluster loses protection β€” flags, ZWJ sequences, combining +// marks alike β€” the same degradation as a runtime without `Intl.Segmenter`. +const GRAPHEME_SNAP_MAX_LENGTH = 8192 + +// True when `index` points at the low half of a surrogate pair β€” i.e. cutting +// the string there would slice an astral character in two. +function splitsSurrogatePair(value: string, index: number): boolean { + if (index <= 0 || index >= value.length) { + return false + } + + const low = value.charCodeAt(index) + + if (low < LOW_SURROGATE_START || low > LOW_SURROGATE_END) { + return false + } + + const high = value.charCodeAt(index - 1) + + return high >= HIGH_SURROGATE_START && high <= HIGH_SURROGATE_END +} + +let graphemeSegmenter: Intl.Segmenter | null | undefined + +function getGraphemeSegmenter(): Intl.Segmenter | null { + if (graphemeSegmenter === undefined) { + graphemeSegmenter = typeof Intl !== 'undefined' && typeof Intl.Segmenter === 'function' + // Grapheme segmentation does not vary by locale; leaving it undefined + // avoids depending on whatever the host default happens to be. + ? new Intl.Segmenter(undefined, { granularity: 'grapheme' }) + : null + } + + return graphemeSegmenter +} + +interface ClusterSpan { + index: number + length: number +} + +/** + * Moves string offsets off the middle of a character. + * + * Code points are not the unit a reader sees: a flag is two regional + * indicators, a family emoji several code points joined by ZWJ, and `ΰ€•ΰ€Ώ` a + * consonant plus a vowel sign. Cutting inside one yields a *different* visible + * character rather than a broken one β€” πŸ‡ΊπŸ‡Έ cut by one code point re-pairs into + * πŸ‡ΈπŸ‡Ί, a different country (#364) β€” so the cut has to move to a cluster edge. + * + * Bound to one `value` because segmenting it is the expensive part. + * `Segments.containing()` is called once per boundary, and a value can carry + * hundreds of match regions: rebuilding the segmenter view for each of them + * cost 8.1 ms over 1600 boundaries, against 0.6 ms when it is built once. + */ +function createClusterSnapper(value: string) { + // `null` once resolved to "no segmenter for this value"; `undefined` while + // still unresolved, so a value that never needs snapping never builds one. + let segments: Intl.Segments | null | undefined + + function straddling(index: number): ClusterSpan | undefined { + if (index <= 0 || index >= value.length) { + return undefined + } + + const current = value.charCodeAt(index) + const previous = value.charCodeAt(index - 1) + + if (current < CLUSTER_CONTINUATION_FLOOR && previous < CLUSTER_CONTINUATION_FLOOR) { + return previous === CARRIAGE_RETURN && current === LINE_FEED + ? { index: index - 1, length: 2 } + : undefined + } + + if (segments === undefined) { + const segmenter = value.length <= GRAPHEME_SNAP_MAX_LENGTH ? getGraphemeSegmenter() : null + + segments = segmenter ? segmenter.segment(value) : null + } + + if (!segments) { + return splitsSurrogatePair(value, index) ? { index: index - 1, length: 2 } : undefined + } + + const segment = segments.containing(index) + + return segment && segment.index !== index + ? { index: segment.index, length: segment.segment.length } + : undefined + } + + return { + /** The start of the cluster straddling `index`, or `index` if it is already on an edge. */ + toStart(index: number): number { + return straddling(index)?.index ?? index + }, + /** The end of the cluster straddling `index`, or `index` if it is already on an edge. */ + toEnd(index: number): number { + const cluster = straddling(index) + + return cluster ? cluster.index + cluster.length : index + } + } +} + function truncateHTMLFromStart(html: string, maxLength: number) { - let truncated = '' + let keptLength = 0 let totalLength = 0 let insideTag = false + let didTruncate = false // Iterate through the HTML string in reverse order, one code point at a time. // Indexing by UTF-16 code unit would slice an astral character (emoji, most @@ -25,6 +149,11 @@ function truncateHTMLFromStart(html: string, maxLength: number) { // `<` and `>` are always single code units, so tag tracking is unaffected. const chars = Array.from(html) + // Characters are retained until the one that overruns the budget, which is + // dropped along with everything the scan has not reached β€” so the result is + // always a suffix of `html`. Counting the retained code units rather than + // building the string up front leaves an index to snap, and drops the + // quadratic prepend on the way. for (let i = chars.length - 1; i >= 0; i--) { const char = chars[i]! @@ -32,7 +161,7 @@ function truncateHTMLFromStart(html: string, maxLength: number) { insideTag = true } else if (char === '<') { insideTag = false - truncated = char + truncated + keptLength += char.length continue } @@ -41,22 +170,36 @@ function truncateHTMLFromStart(html: string, maxLength: number) { } if (totalLength <= maxLength) { - truncated = char + truncated + keptLength += char.length } else { // If we've reached the max length, we break out of the loop // to prevent further processing of the string - truncated = '...' + truncated + didTruncate = true break } } - return truncated + if (!didTruncate) { + return html + } + + // A code point is not what the reader sees. Snap the cut past any grapheme + // cluster it lands inside, so the visible character after the ellipsis is the + // one the author wrote rather than its tail. + return '...' + html.slice(createClusterSnapper(html).toEnd(html.length - keptLength)) } -// Escape an FTS snippet to safe HTML while preserving the `` highlight tags. -// The tag is intentionally hardcoded β€” exposing it as a parameter would let a -// caller smuggle through arbitrary tags (e.g. `