From b4b849dca845e56f67492d6c13ef0d08af2b95ea Mon Sep 17 00:00:00 2001 From: Hamish-2569 Date: Thu, 15 Oct 2026 22:09:34 -0400 Subject: [PATCH] fix(history): cut snippets and indexed text without splitting a surrogate pair history--search's snippet keeps 80 UTF-16 units each side of a match, and history--semantic_search's keeps the first 400 of a message hit's text. The semantic index gets the first maxChars units (32,000 by default) of each message's and summary's text, and semantic_search rebuilds a message hit's text with the same cut before taking its snippet. Each was cut with String.prototype.slice, so a cut that fell inside a surrogate pair, half of an emoji for instance, left a lone surrogate at the start or end of what it cut. Both search tools return their snippets in a JSON result, where such a half shows as an escape such as \ude00, half of a character, and that result stays in the agent's context, which later requests carry. The indexer sends its text to the index service in JSON the same way. With maxChars at 400 or under, the rebuilt text's lone surrogate reached the snippet too, since a text that short is the snippet whole. - safeSlice, which already backs an end off one unit rather than keep only the first half of a pair, now also moves a start that falls on a low surrogate forward past it. No half of a pair faces a cut, whether or not its other half is there; a start of 0 and an end at or past the length cut nothing, so the string's own first and last units stay. Every caller before this one passes a start of 0, which it leaves as it was. - snippetOf (semantic_search) and snippetAround (search, by text or by regular expression) cut with safeSlice, and so does snippetAround's fallback for a match with no position, which no caller reaches today. - The maxChars cut uses safeSlice where the indexer builds a message's and a summary's text and where semantic_search rebuilds a message hit's, so the text indexed and the text rebuilt stay the same. A summary hit's snippet is still the one the index service sends, which this module passes on as it comes. Tests, on Linux, Node 24.22.0: history-module-ux searches, by text and by regular expression, a message whose 80 units before the match start on the second half of a pair and whose 80 after it end on the first half of one, and each snippet keeps 79 units on each side. A second message ends within 80 units of the match, so its snippet runs to the message's end, through safeSlice's early return, and only its start moves. history-semantic-review-findings runs semantic_search over a message whose 400th unit is the first half of a pair, and its snippet keeps 399; with maxChars at 400, the same message's snippet is its first 399 units, and the text indexed for a message and for a summary keeps 399. safe-slice pins each side of both rules: a start on a pair's second half moves, one on another unit doesn't, and a start of 0 keeps a lone low surrogate; an end inside a pair moves back, one just after a whole pair doesn't, and one at or past the length keeps the string's last unit. Red-first on main's src with these three test files: 5 of their 53 tests fail, the four that pin a cut in history-module-ux and history-semantic-review-findings and safe-slice's start on a pair's second half; safe-slice's other five pass on main. Removals: each of 6 pieces put back on its own fails at least one test, and so does each of four changes to safeSlice's rules. The history tests, with channel-label-history and safe-slice, pass 184 of 184. Co-Authored-By: Claude Opus 5.5 --- changelog.d/history-snippets.fixed.md | 1 + src/modules/history/index.ts | 17 +++++--- src/modules/history/semantic.ts | 5 ++- src/safe-slice.ts | 10 ++++- test/history-module-ux.test.ts | 19 +++++++++ test/history-semantic-review-findings.test.ts | 39 +++++++++++++++++++ test/safe-slice.test.ts | 35 +++++++++++++++++ 7 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 changelog.d/history-snippets.fixed.md create mode 100644 test/safe-slice.test.ts diff --git a/changelog.d/history-snippets.fixed.md b/changelog.d/history-snippets.fixed.md new file mode 100644 index 000000000..9ce0001dc --- /dev/null +++ b/changelog.d/history-snippets.fixed.md @@ -0,0 +1 @@ +- The snippets the history tools cut, and the text they send the semantic index, no longer begin or end on half of a surrogate pair, half of an emoji for instance. `history--search` keeps up to 80 UTF-16 units each side of a match, `history--semantic_search` up to 400 of a message hit's text, and the semantic index up to `maxChars` (32,000 by default) of a message's or a summary's text, each one fewer at a cut that would split a pair. A summary hit's snippet is still the one the index service sends. The internal `safeSlice` now also moves a start that falls on a low surrogate forward past it. diff --git a/src/modules/history/index.ts b/src/modules/history/index.ts index 6a763bdb1..9be608655 100644 --- a/src/modules/history/index.ts +++ b/src/modules/history/index.ts @@ -50,6 +50,7 @@ import type { EventResponse, ProcessState } from '../../types/module.js'; import type { SearchWorkerMessage, SearchWorkerMatch } from './search-regex-worker.js'; import type { ChannelRegistry } from '../../mcpl/channel-registry.js'; import { SemanticIndexClient, SemanticIndexer, messageIndexText, type SemanticIndexConfig, type SyncReport, type PendingChanges } from './semantic.js'; +import { safeSlice } from '../../safe-slice.js'; // ============================================================================ // Tool input shapes @@ -208,9 +209,13 @@ const SEMANTIC_DEFAULT_LIMIT = 10; const SEMANTIC_MAX_LIMIT = 50; /** Largest k sent to the embed-service when over-fetching past dropped hits (its own cap is 200). */ const SEMANTIC_MAX_FETCH = 200; -/** Local snippet: the leading SEMANTIC_SNIPPET_CHARS of the current text. */ +/** + * Local snippet: the leading SEMANTIC_SNIPPET_CHARS of the current text, one + * fewer where the last would split a surrogate pair, since the snippet reaches + * the agent's context. + */ function snippetOf(text: string): string { - return text.length <= SEMANTIC_SNIPPET_CHARS ? text : `${text.slice(0, SEMANTIC_SNIPPET_CHARS)}…`; + return text.length <= SEMANTIC_SNIPPET_CHARS ? text : `${safeSlice(text, 0, SEMANTIC_SNIPPET_CHARS)}…`; } const SEMANTIC_SNIPPET_CHARS = 400; @@ -1525,7 +1530,7 @@ export class HistoryModule implements Module { if (msgId !== undefined) { const m = cm.getMessage(msgId); if (!m) return 'offBranch'; - const text = messageIndexText(m, includePrivate).slice(0, this.semanticCfg!.maxChars ?? 32_000); + const text = safeSlice(messageIndexText(m, includePrivate), 0, this.semanticCfg!.maxChars ?? 32_000); if (!text) return 'stale'; currentText.set(h.id, text); return 'ok'; @@ -2331,12 +2336,12 @@ function buildAuthorFilter(include: AuthorSpec | undefined, exclude: AuthorSpec /** ~SNIPPET_CONTEXT_CHARS of surrounding context on each side of a match, or * the first ~SNIPPET_FALLBACK_CHARS of content when no meaningful match * position is given (kept simple on purpose — this is a readability aid, - * not a highlighting engine). */ + * not a highlighting engine). Neither end splits a surrogate pair. */ function snippetAround(text: string, index: number, length: number): string { - if (index < 0) return text.slice(0, SNIPPET_FALLBACK_CHARS); + if (index < 0) return safeSlice(text, 0, SNIPPET_FALLBACK_CHARS); const start = Math.max(0, index - SNIPPET_CONTEXT_CHARS); const end = Math.min(text.length, index + length + SNIPPET_CONTEXT_CHARS); const prefix = start > 0 ? '…' : ''; const suffix = end < text.length ? '…' : ''; - return prefix + text.slice(start, end) + suffix; + return prefix + safeSlice(text, start, end) + suffix; } diff --git a/src/modules/history/semantic.ts b/src/modules/history/semantic.ts index da4938a77..8149e187c 100644 --- a/src/modules/history/semantic.ts +++ b/src/modules/history/semantic.ts @@ -26,6 +26,7 @@ import type { ContextManager, StoredMessage } from '@animalabs/context-manager'; import type { ContentBlock } from '@animalabs/membrane'; +import { safeSlice } from '../../safe-slice.js'; export interface SemanticIndexConfig { /** Base URL of the embed-service, e.g. `http://100.90.161.34:8804`. */ @@ -194,7 +195,7 @@ export function messageToItem(msg: StoredMessage, cfg: { includePrivateTools?: b const tsMs = msg.timestamp.getTime(); return { id: `msg:${String(msg.id)}`, - text: text.slice(0, cfg.maxChars ?? 32_000), + text: safeSlice(text, 0, cfg.maxChars ?? 32_000), ts: tsMs / 1000, channel: channelOf(msg), kind: 'message', @@ -221,7 +222,7 @@ export function summaryToItem(s: SummaryLike, cfg: { maxChars?: number }): Index if (!text) return null; return { id: `sum:${s.id}`, - text: text.slice(0, cfg.maxChars ?? 32_000), + text: safeSlice(text, 0, cfg.maxChars ?? 32_000), ts: s.startMs / 1000, kind: 'summary', level: s.level, diff --git a/src/safe-slice.ts b/src/safe-slice.ts index 9339e0818..04b9090e4 100644 --- a/src/safe-slice.ts +++ b/src/safe-slice.ts @@ -2,9 +2,17 @@ * Slice a string without splitting surrogate pairs. * * Regular String.prototype.slice can cut in the middle of a surrogate pair, - * producing invalid UTF-16. This function adjusts the end index to avoid that. + * producing invalid UTF-16. Here no half of a pair faces a cut: a start on a + * low surrogate moves forward past it, and an end just after a high surrogate + * moves back before it, whether or not the other half is there. A start of 0 + * and an end at or past the length cut nothing, so the string's own first and + * last units stay as they are. */ export function safeSlice(str: string, start: number, end: number): string { + // A low surrogate where the start cuts is a pair's second half, or a lone half: either way, skip it + const first = str.charCodeAt(start); + if (start > 0 && first >= 0xdc00 && first <= 0xdfff) start += 1; + if (end >= str.length) return str.slice(start); // If we're about to cut between a high and low surrogate, back up one char diff --git a/test/history-module-ux.test.ts b/test/history-module-ux.test.ts index cd0ae9491..e32ba052b 100644 --- a/test/history-module-ux.test.ts +++ b/test/history-module-ux.test.ts @@ -210,6 +210,25 @@ describe('HistoryModule UX: wholeWord', () => { }); }); +describe('HistoryModule UX: snippets', () => { + it('keep up to 80 units each side of a match, one fewer where the 80th would split a surrogate pair', async () => { + // One code point, two UTF-16 units. With the "x" first, 80 units before the match start on the second half of + // a pair, and with the "z" after it, 80 units after the match end on the first half of one: each end moves in. + const grin = '\u{1F600}'; + const before = `x${grin.repeat(100)}y`; + const { mod } = stub([ + mk({ id: 's1', ms: min(1), text: `${before}needlez${grin.repeat(100)}` }), + // Here the message ends within 80 units of the match, so the snippet runs to its end and only its start moves. + mk({ id: 's2', ms: min(2), text: `${before}needlez` }), + ]); + const snippets = async (input: Record) => + data(await call(mod, 'search', input)).matches.map((m: any) => m.snippet); + const expected = [`…${grin.repeat(39)}yneedlez${grin.repeat(39)}…`, `…${grin.repeat(39)}yneedlez`]; + assert.deepEqual(await snippets({ query: 'needle' }), expected); + assert.deepEqual(await snippets({ query: 'needle', regex: true }), expected); + }); +}); + describe('HistoryModule UX: order + scannedThrough', () => { it('order:newest scans the most recent maxScan and returns newest-first, with a to: resume point', async () => { const { mod } = stub(fixture); diff --git a/test/history-semantic-review-findings.test.ts b/test/history-semantic-review-findings.test.ts index 92775e236..2063e2c20 100644 --- a/test/history-semantic-review-findings.test.ts +++ b/test/history-semantic-review-findings.test.ts @@ -332,6 +332,45 @@ describe('semantic sync: Greptile #173 findings', () => { await m.stop(); }); + it("a hit's snippet keeps up to 400 units of its text, one fewer where the 400th would split a surrogate pair", async () => { + // One code point, two UTF-16 units, so after the "x" the 400th unit is the first half of the 200th of them. + const grin = '\u{1F600}'; + const cm = new StubCm([msg('e', T0, `x${grin.repeat(250)}`)]); + const m = new HistoryModule({ semantic: { url: svc.url, namespace: 'n', syncIntervalMs: 0, maxSyncBeforeSearch: 0 } }); + m.bind(cm as unknown as ContextManager); + await m.syncSemanticIndex(); + const res = await m.handleToolCall({ id: 'c', name: 'semantic_search', input: { query: 'x' } }); + const hit = (res.data as { hits: Array<{ id: string; snippet: string }> }).hits.find((h) => h.id === 'msg:e'); + assert.ok(hit, 'the message is a hit'); + assert.equal(hit.snippet, `x${grin.repeat(199)}…`); + await m.stop(); + }); + + it("with maxChars at 400, a hit's snippet is its text cut there at a whole character", async () => { + // maxChars cuts the current text the snippet is built from. After the "x", the 400th unit is the first half of a pair. + const grin = '\u{1F600}'; + const cm = new StubCm([msg('e', T0, `x${grin.repeat(250)}`)]); + const m = new HistoryModule({ semantic: { url: svc.url, namespace: 'n', syncIntervalMs: 0, maxSyncBeforeSearch: 0, maxChars: 400 } }); + m.bind(cm as unknown as ContextManager); + await m.syncSemanticIndex(); + const res = await m.handleToolCall({ id: 'c', name: 'semantic_search', input: { query: 'x' } }); + const hit = (res.data as { hits: Array<{ id: string; snippet: string }> }).hits.find((h) => h.id === 'msg:e'); + assert.ok(hit, 'the message is a hit'); + assert.equal(hit.snippet, `x${grin.repeat(199)}`); + await m.stop(); + }); + + it("the text indexed is cut at maxChars at a whole character, a message's and a summary's", async () => { + const grin = '\u{1F600}'; + const summary = { id: 's1', level: 1, content: `x${grin.repeat(250)}`, tokens: 3, startMs: T0, endMs: T0 + 1, firstSequence: 0, lastSequence: 1, createdMs: T0 + 5 }; + const m = new HistoryModule({ semantic: { url: svc.url, namespace: 'n', syncIntervalMs: 0, maxChars: 400 } }); + m.bind(new StubCm([msg('e', T0, `x${grin.repeat(250)}`)], [summary]) as unknown as ContextManager); + await m.syncSemanticIndex(); + assert.equal(svc.items.get('msg:e')?.text, `x${grin.repeat(199)}`); + assert.equal(svc.items.get('sum:s1')?.text, `x${grin.repeat(199)}`); + await m.stop(); + }); + it('G3#2 queued edits for messages that left the branch do not spend the sync budget', async () => { const cm = new StubCm(Array.from({ length: 300 }, (_, i) => msg(`m${i}`, T0 + i * 3_600_000, `message ${i}`))); const m = mod(cm); diff --git a/test/safe-slice.test.ts b/test/safe-slice.test.ts new file mode 100644 index 000000000..8eebd0b9e --- /dev/null +++ b/test/safe-slice.test.ts @@ -0,0 +1,35 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { safeSlice } from '../src/safe-slice.js'; + +// One code point, two UTF-16 units: a high surrogate, then a low one. +const grin = '\u{1F600}'; +const [high, low] = [grin[0]!, grin[1]!]; + +describe('safeSlice: no half of a surrogate pair faces a cut', () => { + it('a start on the second half of a pair moves forward past it', () => { + assert.equal(safeSlice(`a${grin}b`, 2, 4), 'b'); + }); + + it('a start on any other unit stays where it is', () => { + assert.equal(safeSlice(`ab${grin}c`, 1, 5), `b${grin}c`); + assert.equal(safeSlice(`a${grin}b`, 1, 4), `${grin}b`); + }); + + it("a start of 0 cuts nothing, so the string's own first unit stays, a lone low surrogate included", () => { + assert.equal(safeSlice(`${low}ab`, 0, 3), `${low}ab`); + }); + + it('an end inside a pair moves back before it', () => { + assert.equal(safeSlice(`a${grin}b`, 0, 2), 'a'); + }); + + it('an end just after a whole pair stays where it is', () => { + assert.equal(safeSlice(`a${grin}b`, 0, 3), `a${grin}`); + }); + + it("an end at or past the length cuts nothing, so the string's own last unit stays, a lone high surrogate included", () => { + assert.equal(safeSlice(`a${grin}`, 0, 3), `a${grin}`); + assert.equal(safeSlice(`a${high}`, 0, 10), `a${high}`); + }); +});