From b254bd0ecd84802ea402f297dae2398cd9462e32 Mon Sep 17 00:00:00 2001 From: Grimmer Kang Date: Thu, 20 Aug 2026 08:30:37 +0800 Subject: [PATCH 1/6] feat(sessions): shorten titles from the middle, window lines to the match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The title is this app's primary way to tell sessions apart, and it was the least readable thing in the row. Measured on 125 unique custom titles: median 44 chars against a 35-char cut, 64% over it, and 38% written as `A -> B > C` chains whose newest step sits at the END — exactly what head-only truncation removes. Worst consequence: 48 of 125 titles shared their first 35 characters, so eight different sessions all rendered as `fred-ff nextjs backend and mcp arch`. That is a browsing failure, not a search one. - truncateMiddle keeps both ends: `head … tail`. Budget 35 -> 60, which needs looking at in the real window before it settles. - The full title is on a title= attribute, so hovering shows all of it — previously there was no way to read it at all. - windowAroundMatch moves each capped line's window to the first match while searching, so a row shows WHY it matched instead of filtering in and showing nothing. Applied through one fitToRow() to title, first and last message, branch and last AI reply. Measured: 39% of first prompts and 42% of last prompts are longer than the space they render in. - Issue #138: the matched-prompt line used #999 — the same grey as the first-message line — with a U+2315 marker illegible at 11px (read as "•", then "ρ", by the person who asked for the feature). Now amber, in the search-highlight family, with a worded marker. No change needed for the duplicate-suppression hole (§4.5 R3): once the first/last lines window to the match, suppressing the extra line is correct again. Tests: 82 pass, +8 for the two helpers. Both mutation-verified — reverting truncateMiddle to a head slice fails 2, making windowAroundMatch a no-op fails 2. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvkXq7nfQokaTTnbNP8LhD --- CHANGELOG.md | 8 ++++ docs/session-finding-plan.md | 7 ++++ package.json | 2 +- src/session-search.test.ts | 64 +++++++++++++++++++++++++++++++ src/session-search.ts | 50 ++++++++++++++++++++++++ src/switcher-ui.tsx | 73 +++++++++++++++++++++++++++++++----- 6 files changed, 194 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fc56c1..594a53f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 1.0.86 + +- Feat: session rows are readable again when titles are long + - **Long titles now shorten from the middle** (`head … tail`) instead of losing everything past 35 characters. Measured on a 125-title corpus: median length 44, 64% longer than the old cut, and **48 of them shared their first 35 characters** — eight different sessions all rendered as `fred-ff nextjs backend and mcp arch`. Titles written as `A -> B > C` chains keep their newest step, which is the part that identifies the session + - **Hover a title to read it in full** — the row shows a shortened form, the tooltip shows everything + - **Searching moves each line's window to the match.** Every capped line — title, first and last message, branch, last AI reply — now shows *why* the row matched instead of filtering it in and showing nothing (39% of first prompts and 42% of last prompts were longer than the space they render in) + - **The `match #N` line is no longer mistaken for a message line** ([#138](https://github.com/grimmerk/codev/issues/138)): it was the same grey as the first-message line with an unreadable `⌕` glyph; it is now amber, in the same family as the search highlight, with a labelled marker + ## 1.0.85 - Feat: three ways to browse pinned sessions, from the same `📌 Pinned (N)` header diff --git a/docs/session-finding-plan.md b/docs/session-finding-plan.md index 3b8b127..17ba650 100644 --- a/docs/session-finding-plan.md +++ b/docs/session-finding-plan.md @@ -214,6 +214,13 @@ Fixing the highlight is necessary; fixing identifiability is worth more. a boolean. A title-only match therefore produces no snippet at all — which matters for the 35% of titled sessions with no `/rename` prompt to fall back on. +**Status: shipped** (v1.0.86). `truncateMiddle` and `windowAroundMatch` are pure +helpers in `session-search.ts`; the row applies them through one `fitToRow()` +so every capped line follows the same rule. The title budget went 35 → 60 and is +expected to need tuning against the real window. Issue #138's snippet-line +colour ships with it. R3 needed no separate fix: once the first/last lines +window to the match, suppressing the duplicate `⌕` line is correct again. + **Fixes (one PR):** - **T1 — middle-ellipsis title** (`head … tail`) instead of a head-only slice, plus the full diff --git a/package.json b/package.json index 61fc688..806b9d8 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "CodeV", "productName": "CodeV", - "version": "1.0.85", + "version": "1.0.86", "description": "Quick switcher for VS Code, Cursor, and Claude Code sessions", "repository": { "type": "git", diff --git a/src/session-search.test.ts b/src/session-search.test.ts index 533f9f4..77eec30 100644 --- a/src/session-search.test.ts +++ b/src/session-search.test.ts @@ -5,6 +5,8 @@ import { findPromptMatch, isMinorSession, matchesAllWords, + truncateMiddle, + windowAroundMatch, } from './session-search'; describe('matchesAllWords', () => { @@ -96,3 +98,65 @@ describe('isMinorSession', () => { expect(isMinorSession({ isActive: false }, false, false)).toBe(false); }); }); + +describe('truncateMiddle', () => { + it('keeps both ends, which is where these titles carry meaning', () => { + // Real shape from the corpus: the newest step is at the end. + const t = + 'fred-ff nextjs backend and mcp arch clean up -> nextjs backend arch alternative > canva debug'; + const out = truncateMiddle(t, 60); + expect(out.length).toBe(60); + expect(out.startsWith('fred-ff nextjs')).toBe(true); + expect(out.endsWith('canva debug')).toBe(true); + expect(out).toContain('…'); + }); + + it('distinguishes titles that share a long prefix', () => { + const a = 'fred-ff nextjs backend and mcp arch clean up -> alternative 0'; + const b = 'fred-ff nextjs backend and mcp arch clean up -> canva debug'; + // The old head-only cut rendered both identically at 35 chars. + expect(a.slice(0, 35)).toBe(b.slice(0, 35)); + expect(truncateMiddle(a, 40)).not.toBe(truncateMiddle(b, 40)); + }); + + it('leaves short text untouched', () => { + expect(truncateMiddle('short', 60)).toBe('short'); + expect(truncateMiddle('exactly-ten', 11)).toBe('exactly-ten'); + }); +}); + +describe('windowAroundMatch', () => { + const long = `${'a'.repeat(200)}NEEDLE${'b'.repeat(200)}`; + + it('moves the window so a far-away match is actually visible', () => { + const out = windowAroundMatch(long, ['needle'], 60); + expect(out).toContain('NEEDLE'); + expect(out.startsWith('…')).toBe(true); + expect(out.endsWith('…')).toBe(true); + }); + + it('falls back to the ordinary rendering when nothing matches', () => { + expect(windowAroundMatch(long, ['absent'], 60)).toBe( + truncateMiddle(long, 60), + ); + expect(windowAroundMatch(long, [], 60)).toBe(truncateMiddle(long, 60)); + }); + + it('does not move the window for a match already in view', () => { + const text = `NEEDLE${'x'.repeat(200)}`; + expect(windowAroundMatch(text, ['needle'], 60)).toBe( + truncateMiddle(text, 60), + ); + }); + + it('uses the EARLIEST match when several words hit', () => { + const out = windowAroundMatch(long, ['bbb', 'needle'], 60); + expect(out).toContain('NEEDLE'); + }); + + it('leaves text shorter than the window alone', () => { + expect(windowAroundMatch('tiny NEEDLE', ['needle'], 60)).toBe( + 'tiny NEEDLE', + ); + }); +}); diff --git a/src/session-search.ts b/src/session-search.ts index 23df714..1ea7e03 100644 --- a/src/session-search.ts +++ b/src/session-search.ts @@ -74,3 +74,53 @@ export const isMinorSession = ( !hasPrLink && typeof session.messageCount === 'number' && session.messageCount <= 2; + +/** + * Shorten from the MIDDLE, keeping both ends: `head … tail`. + * + * Head-only truncation is wrong for this app's titles specifically. Measured on + * the reference machine (125 unique custom titles): median 44 chars, 64% longer + * than the old 35-char cut, and 38% written as `A -> B > C` chains whose newest + * step sits at the END — so the cut removed exactly the part that identifies + * the session. Worse, 48 of 125 titles shared their first 35 characters: eight + * different sessions all rendered as `fred-ff nextjs backend and mcp arch`. + */ +export const truncateMiddle = (text: string, max: number): string => { + if (max <= 1 || text.length <= max) return text; + // Bias the head slightly longer — it carries the topic, the tail the latest step. + const head = Math.ceil((max - 1) / 2); + const tail = max - 1 - head; + return `${text.slice(0, head)}…${tail > 0 ? text.slice(text.length - tail) : ''}`; +}; + +/** + * A `max`-char window over `text` that is guaranteed to CONTAIN the first + * search match, with ellipses marking whichever end was cut. + * + * A row can only justify its place in the results if you can see why it + * matched. Every line here is length-capped, so a hit past the cap filtered the + * row in and then showed nothing — measured: 39% of first prompts and 42% of + * last prompts exceed the cap they are rendered at. When nothing matches (or + * the match already sits inside the head window) this falls back to `fallback`, + * which is the ordinary non-search rendering. + */ +export const windowAroundMatch = ( + text: string, + wordsLower: string[], + max: number, + fallback: (text: string, max: number) => string = truncateMiddle, +): string => { + if (text.length <= max) return text; + const lower = text.toLowerCase(); + let at = -1; + for (const w of wordsLower) { + if (!w) continue; + const i = lower.indexOf(w); + if (i !== -1 && (at === -1 || i < at)) at = i; + } + // No match, or already visible in the plain head window: render as usual. + if (at === -1 || at < max - 1) return fallback(text, max); + const start = Math.max(0, at - Math.floor(max / 3)); + const end = Math.min(text.length, start + max - 1); + return `…${text.slice(start, end)}${end < text.length ? '…' : ''}`; +}; diff --git a/src/switcher-ui.tsx b/src/switcher-ui.tsx index b272b6e..af927d6 100644 --- a/src/switcher-ui.tsx +++ b/src/switcher-ui.tsx @@ -10,6 +10,7 @@ import { ListViewSession, mergeSessionsById, } from './session-list-view'; +import { truncateMiddle, windowAroundMatch } from './session-search'; import TerminalTab from './terminal-tab'; type SwitcherMode = 'projects' | 'sessions' | 'terminal'; @@ -18,6 +19,22 @@ export const SERVER_URL = 'http://localhost:55688'; // Unified search-match highlight — high contrast on every row color scheme // (the previous per-site translucent styles were near-invisible on colored text). +// The matched-prompt line answers "why is this row here", so it belongs to the +// search affordance, not to the message lines. It used to be #999 — the same +// grey as the first-message line, one pixel smaller — so it read as a second +// copy of that line; and its U+2315 marker was unreadable at 11px (reported as +// "•" and then "ρ" by the author of the feature). Amber ties it to the +// highlight, and the marker is now a word (issue #138). +const SNIPPET_LINE_STYLE = { color: '#c9a227', fontSize: '11px' } as const; + +const SNIPPET_MARKER_STYLE = { + color: '#1e1e1e', + backgroundColor: '#c9a227', + borderRadius: '2px', + padding: '0 4px', + fontSize: '10px', +} as const; + const SEARCH_HIGHLIGHT_STYLE = { backgroundColor: '#f5b942', color: '#1a1a1a', @@ -631,6 +648,17 @@ function SwitcherApp() { ]); const isSearchingSessions = sessionSearchValue.trim().length > 0; + const searchWordsLower = sessionSearchValue + .toLowerCase() + .split(/\s+/) + .filter(Boolean); + // One rule for every length-capped line in a row: while searching, the window + // moves to the first match so you can see WHY the row is in the results; + // otherwise it keeps both ends, because these titles put the newest step last. + const fitToRow = (text: string, max: number) => + isSearchingSessions + ? windowAroundMatch(text, searchWordsLower, max) + : truncateMiddle(text, max); const hiddenSet = new Set(sessionMarks.hidden); const hasPins = Object.keys(sessionMarks.pins).length > 0; // Which rows appear, in which group, in which order — one pure function so @@ -1918,11 +1946,21 @@ function SwitcherApp() { /> {customTitles[session.sessionId] && ( - + {' '}* @@ -1932,7 +1970,10 @@ function SwitcherApp() { {' '}[] @@ -2043,7 +2084,10 @@ function SwitcherApp() { @@ -2053,7 +2097,10 @@ function SwitcherApp() { @@ -2064,7 +2111,10 @@ function SwitcherApp() { @@ -2084,8 +2134,10 @@ function SwitcherApp() { if (dupFirst || dupLast) return null; return (
- - ⌕ #{m.promptIndex + 1}{' '} + + + match #{m.promptIndex + 1} + {' '} From 404e57b6a6defc00a908e1ad4cab2e844a5d7034 Mon Sep 17 00:00:00 2001 From: Grimmer Kang Date: Thu, 20 Aug 2026 15:04:48 +0800 Subject: [PATCH 2/6] fix(sessions): ask the fallback what it shows, don't model it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #139 review round 1. Four of eight findings are one bug, caught by both bots — and grimmer hit the same thing in the UI while testing, from the other direction. windowAroundMatch promised the returned window CONTAINS the first match, then decided "already visible" with a head-window test (at < max - 1) while its fallback truncated from the MIDDLE. For a 60-char budget a match at index 28 passed that test and landed in the elided middle. Real instance, grimmer's own title: agentic-fred harden again - pr2-1533-v7-readiness — deps ... search "pr2-1533-v7-", match at 28 before agentic-fred harden again - pr…k gate, usage-alias migration after …fred harden again - pr2-1533-v7-readiness — deps refresh, … Fixed by asking the fallback whether it shows the match rather than modelling where it keeps characters. Same lesson as PR #137's last round: every "I assume the other function does X" eventually assumes wrong. Also: the window returned max + 1 characters, having reserved room for one ellipsis while rendering two. A capped line that overruns its cap is not capped. The tests missed both, so they are rewritten rather than extended: - no window test asserted the max budget at all - the "earliest match" test used overlapping candidate windows, so it passed even when centred on the LATER word. Now the candidates are 400 chars apart with an explicit not.toContain Snippet colours reworked after grimmer's UI test: the chip is now exactly SEARCH_HIGHLIGHT_STYLE's amber, so chip and highlighted words read as one system, and the line's text returns to the prompt grey — amber body sat too close to the orange last-message line, and the snippet IS a user prompt, not an assistant reply. CHANGELOG: "median 44, 64% longer than the old cut" read as "44 is 64% longer than 35". It meant 64% OF TITLES run past it. 84 tests. Mutation-verified: restoring the head-window assumption fails 1, dropping the ellipsis budget fails 1. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvkXq7nfQokaTTnbNP8LhD --- CHANGELOG.md | 2 +- src/session-search.test.ts | 35 +++++++++++++++++++++++++++++++--- src/session-search.ts | 29 +++++++++++++++++++++++----- src/switcher-ui.tsx | 39 ++++++++++++++++++++++---------------- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 594a53f..67db9da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## 1.0.86 - Feat: session rows are readable again when titles are long - - **Long titles now shorten from the middle** (`head … tail`) instead of losing everything past 35 characters. Measured on a 125-title corpus: median length 44, 64% longer than the old cut, and **48 of them shared their first 35 characters** — eight different sessions all rendered as `fred-ff nextjs backend and mcp arch`. Titles written as `A -> B > C` chains keep their newest step, which is the part that identifies the session + - **Long titles now shorten from the middle** (`head … tail`) instead of losing everything past 35 characters. Measured on a 125-title corpus: the median title is **44 characters** against that 35-char cut, **64% of titles run past it**, and **48 of them shared their first 35 characters** — eight different sessions all rendered as `fred-ff nextjs backend and mcp arch`. Titles written as `A -> B > C` chains keep their newest step, which is the part that identifies the session - **Hover a title to read it in full** — the row shows a shortened form, the tooltip shows everything - **Searching moves each line's window to the match.** Every capped line — title, first and last message, branch, last AI reply — now shows *why* the row matched instead of filtering it in and showing nothing (39% of first prompts and 42% of last prompts were longer than the space they render in) - **The `match #N` line is no longer mistaken for a message line** ([#138](https://github.com/grimmerk/codev/issues/138)): it was the same grey as the first-message line with an unreadable `⌕` glyph; it is now amber, in the same family as the search highlight, with a labelled marker diff --git a/src/session-search.test.ts b/src/session-search.test.ts index 77eec30..25034f4 100644 --- a/src/session-search.test.ts +++ b/src/session-search.test.ts @@ -135,6 +135,31 @@ describe('windowAroundMatch', () => { expect(out.endsWith('…')).toBe(true); }); + // The band between the fallback's head slice and `max`. A head-window check + // (`at < max - 1`) calls this visible, but truncateMiddle elides the middle, + // so the match lands in the gap — the one thing this helper promises cannot + // happen. Asking the fallback whether it shows the match closes it. + it('reveals a match that the fallback would elide from the middle', () => { + const text = `${'a'.repeat(40)}NEEDLE${'c'.repeat(200)}`; + expect(truncateMiddle(text, 60)).not.toContain('NEEDLE'); + expect(windowAroundMatch(text, ['needle'], 60)).toContain('NEEDLE'); + }); + + // A "capped" line that overruns its cap is not capped. Both ellipses count. + it('never exceeds the budget, ellipses included', () => { + for (const max of [20, 40, 60, 81]) { + for (const words of [['needle'], ['absent'], []]) { + expect(windowAroundMatch(long, words, max).length).toBeLessThanOrEqual( + max, + ); + } + const nearEnd = `${'a'.repeat(300)}NEEDLE`; + expect( + windowAroundMatch(nearEnd, ['needle'], max).length, + ).toBeLessThanOrEqual(max); + } + }); + it('falls back to the ordinary rendering when nothing matches', () => { expect(windowAroundMatch(long, ['absent'], 60)).toBe( truncateMiddle(long, 60), @@ -142,16 +167,20 @@ describe('windowAroundMatch', () => { expect(windowAroundMatch(long, [], 60)).toBe(truncateMiddle(long, 60)); }); - it('does not move the window for a match already in view', () => { + it('does not move the window for a match the fallback already shows', () => { const text = `NEEDLE${'x'.repeat(200)}`; expect(windowAroundMatch(text, ['needle'], 60)).toBe( truncateMiddle(text, 60), ); }); + // Discriminating: the two candidate windows are disjoint, so centring on the + // later word would exclude the earlier one and the assertion would fail. it('uses the EARLIEST match when several words hit', () => { - const out = windowAroundMatch(long, ['bbb', 'needle'], 60); - expect(out).toContain('NEEDLE'); + const text = `${'a'.repeat(150)}FIRST${'b'.repeat(400)}SECOND${'c'.repeat(150)}`; + const out = windowAroundMatch(text, ['second', 'first'], 60); + expect(out).toContain('FIRST'); + expect(out).not.toContain('SECOND'); }); it('leaves text shorter than the window alone', () => { diff --git a/src/session-search.ts b/src/session-search.ts index 1ea7e03..6e1aead 100644 --- a/src/session-search.ts +++ b/src/session-search.ts @@ -111,16 +111,35 @@ export const windowAroundMatch = ( fallback: (text: string, max: number) => string = truncateMiddle, ): string => { if (text.length <= max) return text; + const lower = text.toLowerCase(); let at = -1; + let hit = ''; for (const w of wordsLower) { if (!w) continue; const i = lower.indexOf(w); - if (i !== -1 && (at === -1 || i < at)) at = i; + if (i !== -1 && (at === -1 || i < at)) { + at = i; + hit = w; + } } - // No match, or already visible in the plain head window: render as usual. - if (at === -1 || at < max - 1) return fallback(text, max); + if (at === -1) return fallback(text, max); + + // ASK the fallback whether the match is already on screen rather than + // modelling where it keeps characters. An earlier version assumed a head + // window (`at < max - 1`) while the fallback truncated from the MIDDLE, so a + // match at index 40 of a 60-char budget was declared visible and then landed + // in the elided middle — the one thing this helper promises cannot happen. + const plain = fallback(text, max); + if (plain.toLowerCase().includes(hit)) return plain; + + // Every ellipsis rendered counts against `max`, or a "capped" line silently + // overruns the space the row reserved for it. const start = Math.max(0, at - Math.floor(max / 3)); - const end = Math.min(text.length, start + max - 1); - return `…${text.slice(start, end)}${end < text.length ? '…' : ''}`; + const lead = start > 0 ? '…' : ''; + const roomWithTail = max - lead.length - 1; + if (start + roomWithTail >= text.length) { + return `${lead}${text.slice(start, start + (max - lead.length))}`; + } + return `${lead}${text.slice(start, start + roomWithTail)}…`; }; diff --git a/src/switcher-ui.tsx b/src/switcher-ui.tsx index af927d6..6be6867 100644 --- a/src/switcher-ui.tsx +++ b/src/switcher-ui.tsx @@ -17,24 +17,17 @@ type SwitcherMode = 'projects' | 'sessions' | 'terminal'; // import { fetchVSCodeBasedOpenedWindows, SERVER_URL, deleteRecentProjectRecord } from "./vscode-based-ide-utility" export const SERVER_URL = 'http://localhost:55688'; +// The matched-prompt line answers "why is this row here". It used to be #999 +// with a U+2315 marker unreadable at 11px (reported as "•" and then "ρ" by the +// person who asked for the feature), so it read as a second copy of the +// first-message line — issue #138. The identity now lives in the chip below +// rather than in the line's colour: an amber line body sat too close to the +// orange last-message line, and the snippet IS a user prompt, so the neutral +// prompt grey is also the honest colour for its text. +const SNIPPET_LINE_STYLE = { color: '#999', fontSize: '11px' } as const; + // Unified search-match highlight — high contrast on every row color scheme // (the previous per-site translucent styles were near-invisible on colored text). -// The matched-prompt line answers "why is this row here", so it belongs to the -// search affordance, not to the message lines. It used to be #999 — the same -// grey as the first-message line, one pixel smaller — so it read as a second -// copy of that line; and its U+2315 marker was unreadable at 11px (reported as -// "•" and then "ρ" by the author of the feature). Amber ties it to the -// highlight, and the marker is now a word (issue #138). -const SNIPPET_LINE_STYLE = { color: '#c9a227', fontSize: '11px' } as const; - -const SNIPPET_MARKER_STYLE = { - color: '#1e1e1e', - backgroundColor: '#c9a227', - borderRadius: '2px', - padding: '0 4px', - fontSize: '10px', -} as const; - const SEARCH_HIGHLIGHT_STYLE = { backgroundColor: '#f5b942', color: '#1a1a1a', @@ -43,6 +36,20 @@ const SEARCH_HIGHLIGHT_STYLE = { fontWeight: 600, } as const; +// Deliberately the SAME amber as SEARCH_HIGHLIGHT_STYLE: the chip and the +// highlighted words are one system, so the row reads as "search found this +// here" at a glance. The line's text stays the neutral prompt grey — an amber +// line body sat too close to the orange last-message line, and the snippet IS +// a user prompt, so colouring it as one is also the honest choice. +const SNIPPET_MARKER_STYLE = { + color: SEARCH_HIGHLIGHT_STYLE.color, + backgroundColor: SEARCH_HIGHLIGHT_STYLE.backgroundColor, + borderRadius: '2px', + padding: '0 4px', + fontSize: '10px', + fontWeight: 600, +} as const; + // Boundary header of the expanded minor-sessions group. Sticky: it pins to // the top while scrolled inside the minors zone, so collapsing never // requires scrolling back to the boundary row. From a5c83aac83ebc2fd15926d5a72d8f155a16b3b8f Mon Sep 17 00:00:00 2001 From: Grimmer Kang Date: Thu, 20 Aug 2026 15:49:35 +0800 Subject: [PATCH 3/6] fix(sessions): the window must hold the whole matched word MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #139 review round 2. Four findings, three of them one question about the same line. - A long search word could still be cut (cubic P2, real). Centring the window on the match is not enough: a word can start inside the window and run past its end, so the trailing ellipsis swallows it and the branch fails at the one thing it exists to do. The window now slides forward until the match's tail fits, and when a word is longer than the window at all it is shown from its FIRST character — a cut at the end is unavoidable, a cut at the start would hide where the match begins. - truncateMiddle broke its own cap at the lower boundary (CodeRabbit, review body): max <= 1 returned the whole string, so truncateMiddle('ab', 1) was two characters. Same class as the max + 1 bug last round, other end. Now '' at 0 and the ellipsis alone at 1, which is the only honest thing that fits. - Repeated words (CodeRabbit + cubic x2): reusing the fallback when it shows a LATER occurrence contradicted a docblock promising the FIRST match. Fixed on the docblock, not the code, and the reasoning is on the threads: picking the earliest occurrence only decides where to centre; what the reader is owed is that a match is visible. If the ordinary head+tail rendering already shows one, returning it keeps the highlight AND keeps the row from jumping for no visible reason. Two tests pin both directions so the behaviour is a decision rather than an accident. 89 tests. Mutation-verified: removing the slide-forward fails 1, restoring the old lower boundary fails 1, always windowing fails 2. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvkXq7nfQokaTTnbNP8LhD --- src/session-search.test.ts | 45 ++++++++++++++++++++++++++++++++++++++ src/session-search.ts | 38 ++++++++++++++++++++++++++------ 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/session-search.test.ts b/src/session-search.test.ts index 25034f4..05ad2ff 100644 --- a/src/session-search.test.ts +++ b/src/session-search.test.ts @@ -188,4 +188,49 @@ describe('windowAroundMatch', () => { 'tiny NEEDLE', ); }); + + // A repeated word: the earliest occurrence picks where to centre, but if the + // ordinary rendering already shows a LATER one the reader still gets a + // highlight — and gets it without the text jumping. The contract is "a match + // is visible", not "this particular occurrence is". + it('keeps the ordinary rendering when it already shows some occurrence', () => { + const text = `head NEEDLE ${'m'.repeat(200)} NEEDLE tail`; + const plain = truncateMiddle(text, 60); + expect(plain).toContain('NEEDLE'); + expect(windowAroundMatch(text, ['needle'], 60)).toBe(plain); + }); + + it('windows when NO occurrence survives the ordinary rendering', () => { + const text = `${'a'.repeat(60)}NEEDLE${'b'.repeat(60)}NEEDLE${'c'.repeat(200)}`; + expect(truncateMiddle(text, 60)).not.toContain('NEEDLE'); + expect(windowAroundMatch(text, ['needle'], 60)).toContain('NEEDLE'); + }); + // The windowed branch exists to contain the match, so a long search word + // must not be swallowed by the trailing ellipsis it makes room for. + it('keeps a LONG matched word whole, not just its start', () => { + const word = 'w'.repeat(30); + const text = `${'a'.repeat(200)}${word}${'b'.repeat(200)}`; + const out = windowAroundMatch(text, [word], 60); + expect(out).toContain(word); + expect(out.length).toBeLessThanOrEqual(60); + }); + + it('shows a word longer than the window from its first character', () => { + const word = 'w'.repeat(90); + const text = `${'a'.repeat(200)}${word}${'b'.repeat(200)}`; + const out = windowAroundMatch(text, [word], 60); + expect(out.length).toBeLessThanOrEqual(60); + // Cut at the end is unavoidable; cut at the START would hide where the + // match begins, which is the part that tells you why the row is here. + expect(out.replace(/…/g, '').startsWith('w')).toBe(true); + }); +}); + +describe('truncateMiddle lower boundary', () => { + it('never returns more than max, even at 0 and 1', () => { + expect(truncateMiddle('ab', 0)).toBe(''); + expect(truncateMiddle('ab', 1)).toBe('…'); + expect(truncateMiddle('abcdef', 2).length).toBe(2); + expect(truncateMiddle('abcdef', 3).length).toBe(3); + }); }); diff --git a/src/session-search.ts b/src/session-search.ts index 6e1aead..bfaed2f 100644 --- a/src/session-search.ts +++ b/src/session-search.ts @@ -86,7 +86,11 @@ export const isMinorSession = ( * different sessions all rendered as `fred-ff nextjs backend and mcp arch`. */ export const truncateMiddle = (text: string, max: number): string => { - if (max <= 1 || text.length <= max) return text; + if (max <= 0) return ''; + if (text.length <= max) return text; + // At one character there is no room for head, tail AND a marker; the marker + // is the only honest thing to keep. + if (max === 1) return '…'; // Bias the head slightly longer — it carries the topic, the tail the latest step. const head = Math.ceil((max - 1) / 2); const tail = max - 1 - head; @@ -94,8 +98,14 @@ export const truncateMiddle = (text: string, max: number): string => { }; /** - * A `max`-char window over `text` that is guaranteed to CONTAIN the first - * search match, with ellipses marking whichever end was cut. + * A `max`-char window over `text` that is guaranteed to show A match, with + * ellipses marking whichever end was cut. + * + * A match, not *the first* match: the earliest occurrence is only used to + * decide where to centre the window. If the ordinary rendering already shows + * some later occurrence of the same word, that rendering is returned unchanged + * — the reader still sees a highlight, and they see it in the familiar head+ + * tail shape instead of a window that jumped for no visible reason. * * A row can only justify its place in the results if you can see why it * matched. Every line here is length-capped, so a hit past the cap filtered the @@ -135,11 +145,25 @@ export const windowAroundMatch = ( // Every ellipsis rendered counts against `max`, or a "capped" line silently // overruns the space the row reserved for it. - const start = Math.max(0, at - Math.floor(max / 3)); + // Position roughly a third in, then pull the window forward if the match's + // TAIL would fall outside it. Centring alone is not enough: a long search + // word can start inside the window and still run past its end, so the + // trailing ellipsis swallows it and this branch fails at the one thing it + // exists to do. + const wantEnd = at + hit.length; + let start = Math.max(0, at - Math.floor(max / 3)); + // Reserve the trailing ellipsis while positioning; give it back below if the + // window reaches the end of the text. + let room = max - (start > 0 ? 1 : 0) - 1; + if (start + room < wantEnd) { + // A word longer than the window cannot fit whole — then show it from its + // first character rather than from the middle of it. + start = Math.min(at, Math.max(0, wantEnd - room)); + room = max - (start > 0 ? 1 : 0) - 1; + } const lead = start > 0 ? '…' : ''; - const roomWithTail = max - lead.length - 1; - if (start + roomWithTail >= text.length) { + if (start + room >= text.length) { return `${lead}${text.slice(start, start + (max - lead.length))}`; } - return `${lead}${text.slice(start, start + roomWithTail)}…`; + return `${lead}${text.slice(start, start + room)}…`; }; From 81b741bd09d47862d1275fbf774a4179bd43a960 Mon Sep 17 00:00:00 2001 From: Grimmer Kang Date: Thu, 20 Aug 2026 16:16:42 +0800 Subject: [PATCH 4/6] fix(sessions): match on source offsets, not on a lowercased copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #139 review round 3, both from cubic, both real. - Case folding can change length, so an index taken in a lowercased copy does not address the same character in the source. 'İ'.toLowerCase() is two code units, so a title containing one before the match shifted every later index by a character and the window sheared the first matched character off. The search now runs case-insensitively over the ORIGINAL string, which also makes `hit` the source match rather than the query word — the length the window actually has to fit. Query words are escaped, so a title searched for `a+b(c)` is matched as literal text rather than as a pattern. - windowAroundMatch broke its own cap at tiny budgets: at max = 0 a matching input still returned an ellipsis. There is no useful window in one or two characters and building one spends the whole budget on ellipses, so those delegate to the fallback, which already honours the cap. Third time the cap has been wrong in this helper — max + 1, then truncateMiddle's lower boundary, now this — which is why the budget assertion added in round 1 now runs across every branch. 92 tests. Mutation-verified: restoring lowercased indexing fails 1, dropping the tiny-budget guard fails 1, removing the regex escaping fails 1. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvkXq7nfQokaTTnbNP8LhD --- src/session-search.test.ts | 23 +++++++++++++++++++++++ src/session-search.ts | 26 ++++++++++++++++++++------ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/session-search.test.ts b/src/session-search.test.ts index 05ad2ff..4718e61 100644 --- a/src/session-search.test.ts +++ b/src/session-search.test.ts @@ -224,6 +224,29 @@ describe('windowAroundMatch', () => { // match begins, which is the part that tells you why the row is here. expect(out.replace(/…/g, '').startsWith('w')).toBe(true); }); + // toLowerCase() can CHANGE LENGTH — 'İ' becomes two code units — so an index + // taken in a lowercased copy does not address the same character in the + // source. Slicing by it shears the first matched character off the window. + it('keeps source offsets when case folding changes length', () => { + const text = `${'İ'.repeat(40)}${'a'.repeat(120)}NEEDLE${'b'.repeat(120)}`; + const out = windowAroundMatch(text, ['needle'], 60); + expect(out).toContain('NEEDLE'); + expect(out.length).toBeLessThanOrEqual(60); + }); + + it('treats a query word as literal text, not a pattern', () => { + const text = `${'a'.repeat(200)}a+b(c)${'d'.repeat(200)}`; + expect(windowAroundMatch(text, ['a+b(c)'], 60)).toContain('a+b(c)'); + }); + + it('honours a budget too small for any window', () => { + const text = `${'a'.repeat(200)}NEEDLE${'b'.repeat(200)}`; + for (const max of [0, 1, 2]) { + expect( + windowAroundMatch(text, ['needle'], max).length, + ).toBeLessThanOrEqual(max); + } + }); }); describe('truncateMiddle lower boundary', () => { diff --git a/src/session-search.ts b/src/session-search.ts index bfaed2f..06089be 100644 --- a/src/session-search.ts +++ b/src/session-search.ts @@ -75,6 +75,10 @@ export const isMinorSession = ( typeof session.messageCount === 'number' && session.messageCount <= 2; +/** Escape a user-typed word so it can be matched as a literal. */ +const escapeForRegExp = (word: string): string => + word.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + /** * Shorten from the MIDDLE, keeping both ends: `head … tail`. * @@ -122,26 +126,36 @@ export const windowAroundMatch = ( ): string => { if (text.length <= max) return text; - const lower = text.toLowerCase(); + // Search the ORIGINAL string case-insensitively rather than lowercasing it + // first: `toLowerCase()` can change length (U+0130 'İ' becomes two code + // units), so an index taken in the lowercased copy does not address the same + // character in the source, and slicing by it can shear the first matched + // character off the window. let at = -1; let hit = ''; for (const w of wordsLower) { if (!w) continue; - const i = lower.indexOf(w); - if (i !== -1 && (at === -1 || i < at)) { - at = i; - hit = w; + const found = new RegExp(escapeForRegExp(w), 'i').exec(text); + if (found && (at === -1 || found.index < at)) { + at = found.index; + // The source match, not the query word: its length in the original text + // is what the window has to make room for. + hit = found[0]; } } if (at === -1) return fallback(text, max); + // No useful window exists in one or two characters, and building one would + // spend the whole budget on ellipses; the fallback already honours the cap. + if (max <= 2) return fallback(text, max); + // ASK the fallback whether the match is already on screen rather than // modelling where it keeps characters. An earlier version assumed a head // window (`at < max - 1`) while the fallback truncated from the MIDDLE, so a // match at index 40 of a 60-char budget was declared visible and then landed // in the elided middle — the one thing this helper promises cannot happen. const plain = fallback(text, max); - if (plain.toLowerCase().includes(hit)) return plain; + if (plain.toLowerCase().includes(hit.toLowerCase())) return plain; // Every ellipsis rendered counts against `max`, or a "capped" line silently // overruns the space the row reserved for it. From 2467990d53604f80eef9d30b891ccd737006d292 Mon Sep 17 00:00:00 2001 From: Grimmer Kang Date: Thu, 20 Aug 2026 16:47:27 +0800 Subject: [PATCH 5/6] fix(sessions): one matching rule, offsets translated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #139 review round 4. Last round's fix traded one exotic bug for another, and cubic caught it from both sides. Searching the original text with /i does not fold the way toLowerCase does: the filter accepts 'İ' for the query 'i', the regex does not. So a row could be LISTED by matchesAllWords and then show no highlight at all — a regression of the exact guarantee this helper exists for, introduced while fixing an off-by-one. The rule is now singular again: match with toLowerCase, exactly as matchesAllWords does, then translate the offset into the source by walking the lowercased prefix. Matching semantics and source coordinates were never the same question, and answering both with a regex answered neither. The escaping helper goes away with it, so there is no pattern to inject into either. Also: the repeated-word test never reached the branch it named (CodeRabbit). Its first occurrence sat inside the head slice truncateMiddle keeps, so "earliest elided, later one visible" was never exercised. The fixture now places the earliest occurrence in the elided middle and asserts that explicitly. Third test of mine on these two PRs that claimed more than it verified. New test pins the thing that broke: the filter and the window must agree on Unicode folding. 93 tests. Mutation-verified: removing the offset translation fails 1. Using the query word's length instead of the source span's fails nothing — its effect is unobservable in any case I could construct, so it is recorded as reasoning rather than dressed up with a contrived test. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvkXq7nfQokaTTnbNP8LhD --- src/session-search.test.ts | 16 ++++++++++-- src/session-search.ts | 52 ++++++++++++++++++++++++++------------ 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/src/session-search.test.ts b/src/session-search.test.ts index 4718e61..3c0d99d 100644 --- a/src/session-search.test.ts +++ b/src/session-search.test.ts @@ -194,9 +194,12 @@ describe('windowAroundMatch', () => { // highlight — and gets it without the text jumping. The contract is "a match // is visible", not "this particular occurrence is". it('keeps the ordinary rendering when it already shows some occurrence', () => { - const text = `head NEEDLE ${'m'.repeat(200)} NEEDLE tail`; + // The earliest occurrence must be ELIDED and a later one visible, or the + // test never reaches the branch it names. + const text = `${'a'.repeat(40)}NEEDLE${'b'.repeat(200)}NEEDLE-tail`; const plain = truncateMiddle(text, 60); - expect(plain).toContain('NEEDLE'); + expect(plain.slice(0, 30)).not.toContain('NEEDLE'); // earliest is elided + expect(plain).toContain('NEEDLE'); // a later one survives expect(windowAroundMatch(text, ['needle'], 60)).toBe(plain); }); @@ -234,6 +237,15 @@ describe('windowAroundMatch', () => { expect(out.length).toBeLessThanOrEqual(60); }); + // The row is listed by matchesAllWords, so the window must find the same + // match — a regex with /i folds differently and would show no highlight. + it('agrees with the filter on Unicode case folding', () => { + const text = `${'a'.repeat(200)}İ${'b'.repeat(200)}`; + expect(matchesAllWords(text.toLowerCase(), ['i'])).toBe(true); + const out = windowAroundMatch(text, ['i'], 60); + expect(out).toContain('İ'); + }); + it('treats a query word as literal text, not a pattern', () => { const text = `${'a'.repeat(200)}a+b(c)${'d'.repeat(200)}`; expect(windowAroundMatch(text, ['a+b(c)'], 60)).toContain('a+b(c)'); diff --git a/src/session-search.ts b/src/session-search.ts index 06089be..b6d58cb 100644 --- a/src/session-search.ts +++ b/src/session-search.ts @@ -75,9 +75,25 @@ export const isMinorSession = ( typeof session.messageCount === 'number' && session.messageCount <= 2; -/** Escape a user-typed word so it can be matched as a literal. */ -const escapeForRegExp = (word: string): string => - word.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +/** + * The source index whose lowercased prefix is `lowerIndex` code units long. + * + * `toLowerCase()` can change length ('İ' becomes two code units), so an offset + * found in a lowercased copy does not address the same character in the + * source. Walking the prefix maps one to the other WITHOUT changing what + * counts as a match — matching has to stay `toLowerCase`-based because that is + * what `matchesAllWords` uses to decide the row belongs in the results at all. + * A regex with the `i` flag folds differently (it will not accept 'İ' for 'i'), + * so a row could be listed and then show no highlight. + */ +const sourceIndexOfLowerIndex = (text: string, lowerIndex: number): number => { + let lowerLen = 0; + for (let i = 0; i < text.length; i++) { + if (lowerLen >= lowerIndex) return i; + lowerLen += text[i].toLowerCase().length; + } + return text.length; +}; /** * Shorten from the MIDDLE, keeping both ends: `head … tail`. @@ -126,23 +142,27 @@ export const windowAroundMatch = ( ): string => { if (text.length <= max) return text; - // Search the ORIGINAL string case-insensitively rather than lowercasing it - // first: `toLowerCase()` can change length (U+0130 'İ' becomes two code - // units), so an index taken in the lowercased copy does not address the same - // character in the source, and slicing by it can shear the first matched - // character off the window. - let at = -1; - let hit = ''; + // Match exactly as `matchesAllWords` does, then translate the offset into + // the source (see sourceIndexOfLowerIndex). Two matching rules for one + // question is how a row ends up listed with nothing highlighted. + const lower = text.toLowerCase(); + let atLower = -1; + let wordLen = 0; for (const w of wordsLower) { if (!w) continue; - const found = new RegExp(escapeForRegExp(w), 'i').exec(text); - if (found && (at === -1 || found.index < at)) { - at = found.index; - // The source match, not the query word: its length in the original text - // is what the window has to make room for. - hit = found[0]; + const i = lower.indexOf(w); + if (i !== -1 && (atLower === -1 || i < atLower)) { + atLower = i; + wordLen = w.length; } } + const at = atLower === -1 ? -1 : sourceIndexOfLowerIndex(text, atLower); + // The SOURCE span, not the query word: its length in the original text is + // what the window has to make room for. + const hit = + atLower === -1 + ? '' + : text.slice(at, sourceIndexOfLowerIndex(text, atLower + wordLen)); if (at === -1) return fallback(text, max); // No useful window exists in one or two characters, and building one would From f7a51ce7e8972afcf3886345fa0906283c10e37c Mon Sep 17 00:00:00 2001 From: Grimmer Kang Date: Thu, 20 Aug 2026 18:28:09 +0800 Subject: [PATCH 6/6] fix(sessions): record where each folded unit came from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cubic P2, and it is exact: a query can begin INSIDE a folding expansion. The combining dot is the second half of what 'İ' lowercases to, and a prefix count can only name whole source characters, so it reported the character AFTER 'İ' and the span came back empty. An empty hit is worse than a wrong one. `plain.includes('')` is trivially true, so the fallback was accepted unconditionally and the row's only match could stay hidden — the failure this helper exists to prevent, reached through the code meant to prevent it. Third round in a row on Unicode folding, so this changes the data structure rather than adding another condition: record which source character each folded unit came from, then read both ends off that map. Start and end are answered independently, and the end is inclusive, so a match that begins or ends mid-expansion still yields a non-empty span. The map is built only when folding actually changed the length — for ASCII and CJK the two strings share coordinates and nothing is allocated. 94 tests. Mutation-verified: dropping the origin lookup fails 2, using an exclusive end fails 1. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvkXq7nfQokaTTnbNP8LhD --- src/session-search.test.ts | 12 +++++++++ src/session-search.ts | 54 ++++++++++++++++++++++++-------------- 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/src/session-search.test.ts b/src/session-search.test.ts index 3c0d99d..2dde378 100644 --- a/src/session-search.test.ts +++ b/src/session-search.test.ts @@ -246,6 +246,18 @@ describe('windowAroundMatch', () => { expect(out).toContain('İ'); }); + // A query can begin INSIDE a folding expansion: the combining dot is the + // second half of what 'İ' lowercases to. A prefix count can only name whole + // source characters, so it reported the character AFTER 'İ' and the span came + // back EMPTY — and an empty hit makes `plain.includes(hit)` trivially true, + // so the fallback was always accepted and the only match could stay hidden. + it('resolves a match that starts inside a folding expansion', () => { + const text = `${'a'.repeat(200)}İ${'b'.repeat(200)}`; + const out = windowAroundMatch(text, ['\u0307'], 60); + expect(out).toContain('İ'); + expect(out.length).toBeLessThanOrEqual(60); + }); + it('treats a query word as literal text, not a pattern', () => { const text = `${'a'.repeat(200)}a+b(c)${'d'.repeat(200)}`; expect(windowAroundMatch(text, ['a+b(c)'], 60)).toContain('a+b(c)'); diff --git a/src/session-search.ts b/src/session-search.ts index b6d58cb..59f83d4 100644 --- a/src/session-search.ts +++ b/src/session-search.ts @@ -76,23 +76,28 @@ export const isMinorSession = ( session.messageCount <= 2; /** - * The source index whose lowercased prefix is `lowerIndex` code units long. + * Where a match found in `text.toLowerCase()` lives in `text` itself. * - * `toLowerCase()` can change length ('İ' becomes two code units), so an offset - * found in a lowercased copy does not address the same character in the - * source. Walking the prefix maps one to the other WITHOUT changing what - * counts as a match — matching has to stay `toLowerCase`-based because that is - * what `matchesAllWords` uses to decide the row belongs in the results at all. - * A regex with the `i` flag folds differently (it will not accept 'İ' for 'i'), - * so a row could be listed and then show no highlight. + * Folding is not length-preserving — 'İ' lowercases to two code units — so an + * offset found in the lowercased copy does not address the same character in + * the source. Matching still has to be done with `toLowerCase`, because that is + * what `matchesAllWords` uses to decide the row belongs in the results at all; + * a regex with `/i` folds differently and would list a row that shows no + * highlight. So the offset is translated, not re-derived. + * + * Translating by counting prefix lengths is not enough: a query can begin + * INSIDE an expansion (the combining dot of 'İ'), and a prefix count can only + * name whole source characters, so it reports the character AFTER the one the + * match started in and the span comes back empty. Recording which source + * character each folded unit came from answers both ends exactly. */ -const sourceIndexOfLowerIndex = (text: string, lowerIndex: number): number => { - let lowerLen = 0; +const foldedOrigins = (text: string): number[] => { + const origins: number[] = []; for (let i = 0; i < text.length; i++) { - if (lowerLen >= lowerIndex) return i; - lowerLen += text[i].toLowerCase().length; + const folded = text[i].toLowerCase(); + for (let k = 0; k < folded.length; k++) origins.push(i); } - return text.length; + return origins; }; /** @@ -156,13 +161,22 @@ export const windowAroundMatch = ( wordLen = w.length; } } - const at = atLower === -1 ? -1 : sourceIndexOfLowerIndex(text, atLower); - // The SOURCE span, not the query word: its length in the original text is - // what the window has to make room for. - const hit = - atLower === -1 - ? '' - : text.slice(at, sourceIndexOfLowerIndex(text, atLower + wordLen)); + let at = -1; + let hit = ''; + if (atLower !== -1) { + if (lower.length === text.length) { + // Nothing expanded, so the two strings share coordinates. + at = atLower; + hit = text.slice(at, at + wordLen); + } else { + const origins = foldedOrigins(text); + at = origins[atLower]; + // Inclusive end: the source character the match's LAST folded unit came + // from, so a match that starts or ends inside an expansion still yields + // a non-empty span. + hit = text.slice(at, origins[atLower + wordLen - 1] + 1); + } + } if (at === -1) return fallback(text, max); // No useful window exists in one or two characters, and building one would