From 8cdd684a8e7aa57444093e9a24f75860e3eff074 Mon Sep 17 00:00:00 2001 From: NovakPAai Date: Sun, 2 Aug 2026 23:18:11 +0300 Subject: [PATCH] perf: stop the search index from freezing the event loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildSearchIndex re-parsed every session with detail — a findSessionFile lookup plus a full detail load (sync fs read + JSON.parse per line) each — in one synchronous tick, on whichever request happened to miss the 60s cache. Measured on a 900-session history: a 3.6s hard stall of the event loop, which also stalls every other API call and the terminal WebSocket data pump. The analytics job already solved this exact problem with chunk+yield (_scheduleAnalyticsRecompute); search never got the same treatment. Chunking per session alone wasn't enough: real histories have a heavy tail (median session ~0.1MB here, but codex transcripts up to 78MB), and readLines slurps the whole file into a string before splitting — one such session blocks for seconds regardless of the outer chunk size. - buildSearchIndex is async, processes sessions in small chunks (8 — each item is far heavier than the computeSessionCost calls analytics batches 80-at-a-time) and yields via setImmediate between them. - JSONL sessions over SEARCH_STREAM_THRESHOLD (4MB) are read line-by-line off a stream that yields every 2000 lines, so one huge transcript can't block either. Nothing is truncated — this only changes *when* the work happens, not what gets indexed. - getSearchIndex is stale-while-revalidate (mirroring getCostAnalytics): a >60s-old index is still overwhelmingly accurate, so it's served instantly while the refresh runs in the background. Only a genuine cold start awaits. - Concurrent rebuilds dedupe into one in-flight job, so a burst of searches during a rebuild no longer queues N full-history scans. - Collapsed six near-identical per-format if/else branches into a SEARCH_DETAIL_LOADERS lookup table, and the two JSONL readers now share one per-line parser so they can't drift. Snippet cap is a named constant instead of a repeated 500. - searchFullText is now async; /api/search and the `codbash search` CLI command updated accordingly. Measured before/after on the same 900-session history: worst event-loop stall 3622ms -> 60ms cold build 3.6s -> 3.0s warm search ~14ms (unchanged) Search results verified byte-identical to origin/main across 6 queries (496 result rows) via a git-worktree differential run. --- bin/cli.js | 17 +- src/data.js | 290 ++++++++++++++++++----------- src/server.js | 7 +- test/search-index-chunking.test.js | 127 +++++++++++++ 4 files changed, 322 insertions(+), 119 deletions(-) create mode 100644 test/search-index-chunking.test.js diff --git a/bin/cli.js b/bin/cli.js index 99c8971..33a0573 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -170,10 +170,14 @@ switch (command) { process.exit(1); } const sessions = loadSessions(); - const results = searchFullText(query, sessions); - if (results.length === 0) { - console.log(`\n No results for "${query}"\n`); - } else { + // searchFullText is async (the index build yields between chunks). This + // switch is top-level CJS, so no top-level await — run it in an IIFE. + (async () => { + const results = await searchFullText(query, sessions); + if (results.length === 0) { + console.log(`\n No results for "${query}"\n`); + return; + } console.log(`\n \x1b[36m\x1b[1m${results.length} sessions\x1b[0m matching "${query}"\n`); for (const r of results.slice(0, 15)) { const s = sessions.find(x => x.id === r.sessionId); @@ -188,7 +192,10 @@ switch (command) { } if (results.length > 15) console.log(`\n \x1b[2m... and ${results.length - 15} more\x1b[0m`); console.log(''); - } + })().catch(e => { + console.error(` Search failed: ${e.message}`); + process.exit(1); + }); break; } diff --git a/src/data.js b/src/data.js index a74166a..9e6da84 100644 --- a/src/data.js +++ b/src/data.js @@ -4839,118 +4839,167 @@ let searchIndex = null; let searchIndexBuiltAt = 0; const INDEX_TTL = 60000; // rebuild every 60s -function buildSearchIndex(sessions) { - const startMs = Date.now(); - const index = []; +const SEARCH_SNIPPET_LEN = 500; + +// Formats whose messages come from a bespoke loader rather than the generic +// JSONL reader. Each entry takes (sessionId, file) — loaders that don't need +// the file simply ignore it — and returns `{ messages: [{role, content}] }`. +// A lookup table instead of six near-identical if/else branches: the branches +// differed only by loader name, so a fix to one (snippet length, the +// isSystemMessage filter) silently missed the other five. +const SEARCH_DETAIL_LOADERS = { + qwen: (id, file) => loadQwenDetail(id, file), + kilo: (id) => loadKiloCliDetail(id), + opencode: (id) => loadOpenCodeDetail(id), + kiro: (id) => loadKiroDetail(id), + 'kiro-cli': (id) => loadKiroCliDetail(id), + cursor: (id) => loadCursorDetail(id), + pi: (id, file) => loadPiDetail(id, file), + // Copilot Chat (VS Code JSON) and Copilot CLI need their own loaders; the + // generic JSONL branch would mis-parse them and index nothing. + copilot: (id) => loadCopilotCliDetail(id), + 'copilot-chat': (id) => loadCopilotDetail(id), +}; - for (const s of sessions) { - if (!s.has_detail) continue; +// Indexable {role, content} pairs from a loader's messages: drop empties and +// system noise, cap each message at SEARCH_SNIPPET_LEN. (isSystemMessage only +// tests short prefixes/exact strings, so filtering before or after the slice +// is equivalent — the pre-refactor qwen branch did it in the other order.) +function _searchTextsFromMessages(messages) { + const texts = []; + for (const msg of (messages || [])) { + if (msg.content && !isSystemMessage(msg.content)) { + texts.push({ role: msg.role, content: msg.content.slice(0, SEARCH_SNIPPET_LEN) }); + } + } + return texts; +} - const found = findSessionFile(s.id, s.project); - if (!found) continue; +// One JSONL line → an indexable {role, content} pair, or null. Shared by the +// sync and streaming readers so the two can't drift. +function _searchTextFromJsonlLine(line, format) { + try { + const entry = JSON.parse(line); + let role, content; - try { - if (found.format === 'qwen') { - const detail = loadQwenDetail(s.id, found.file); - const texts = (detail.messages || []).map(function(m) { - return { role: m.role, content: (m.content || '').slice(0, 500) }; - }).filter(function(m) { - return m.content && !isSystemMessage(m.content); - }); - if (texts.length > 0) { - const fullText = texts.map(t => t.content).join(' ').toLowerCase(); - index.push({ sessionId: s.id, texts, fullText }); - } - continue; - } + if (format === 'claude') { + if (entry.type !== 'user' && entry.type !== 'assistant') return null; + role = entry.type; + content = extractContent((entry.message || {}).content); + } else { + if (entry.type !== 'response_item' || !entry.payload) return null; + role = entry.payload.role; + if (role !== 'user' && role !== 'assistant') return null; + content = extractContent(entry.payload.content); + } - const texts = []; + if (content && !isSystemMessage(content)) { + return { role, content: content.slice(0, SEARCH_SNIPPET_LEN) }; + } + } catch {} + return null; +} - if (found.format === 'kilo') { - const detail = loadKiloCliDetail(s.id); - for (const msg of detail.messages) { - if (msg.content && !isSystemMessage(msg.content)) { - texts.push({ role: msg.role, content: msg.content.slice(0, 500) }); - } - } - } else if (found.format === 'opencode') { - const detail = loadOpenCodeDetail(s.id); - for (const msg of detail.messages) { - if (msg.content && !isSystemMessage(msg.content)) { - texts.push({ role: msg.role, content: msg.content.slice(0, 500) }); - } - } - } else if (found.format === 'kiro') { - const detail = loadKiroDetail(s.id); - for (const msg of detail.messages) { - if (msg.content && !isSystemMessage(msg.content)) { - texts.push({ role: msg.role, content: msg.content.slice(0, 500) }); - } - } - } else if (found.format === 'kiro-cli') { - const detail = loadKiroCliDetail(s.id); - for (const msg of detail.messages) { - if (msg.content && !isSystemMessage(msg.content)) { - texts.push({ role: msg.role, content: msg.content.slice(0, 500) }); - } - } - } else if (found.format === 'cursor') { - const detail = loadCursorDetail(s.id); - for (const msg of detail.messages) { - if (msg.content && !isSystemMessage(msg.content)) { - texts.push({ role: msg.role, content: msg.content.slice(0, 500) }); - } - } - } else if (found.format === 'pi') { - const detail = loadPiDetail(s.id, found.file); - for (const msg of detail.messages) { - if (msg.content && !isSystemMessage(msg.content)) { - texts.push({ role: msg.role, content: msg.content.slice(0, 500) }); - } - } - } else if (found.format === 'copilot-chat' || found.format === 'copilot') { - // Copilot Chat (VS Code JSON) and Copilot CLI use bespoke loaders; the - // generic JSONL branch below would mis-parse them and index nothing. - const detail = found.format === 'copilot' - ? loadCopilotCliDetail(s.id) - : loadCopilotDetail(s.id); - for (const msg of (detail.messages || [])) { - if (msg.content && !isSystemMessage(msg.content)) { - texts.push({ role: msg.role, content: msg.content.slice(0, 500) }); - } - } - } else { - const lines = readLines(found.file); +// Indexable pairs straight from a raw JSONL file (claude / codex formats). +function _searchTextsFromJsonl(file, format) { + const texts = []; + for (const line of readLines(file)) { + const t = _searchTextFromJsonlLine(line, format); + if (t) texts.push(t); + } + return texts; +} + +// Above which a JSONL session is read as a stream instead of slurped whole. +// Chunking the index *per session* still leaves one stall as long as the +// biggest single session takes: real histories have a heavy tail (a median +// session is ~0.1MB but codex transcripts run to tens of MB), and slurping one +// of those via readLines — whole file into a string, split, filter — blocks +// for seconds no matter how small the outer chunk is. +const SEARCH_STREAM_THRESHOLD = 4 * 1024 * 1024; + +// Same as _searchTextsFromJsonl but reads line-by-line off a stream and yields +// to the event loop periodically, so indexing one huge transcript can't freeze +// the terminal WebSocket. Nothing is truncated — this is purely about *when* +// the work happens, so search results are identical either way. +async function _searchTextsFromJsonlStreaming(file, format) { + const readline = require('readline'); + const texts = []; + let sinceYield = 0; + const rl = readline.createInterface({ + input: fs.createReadStream(file, { encoding: 'utf8' }), + crlfDelay: Infinity, + }); + try { + for await (const raw of rl) { + const line = raw.replace(/\r$/, ''); + if (!line) continue; + const t = _searchTextFromJsonlLine(line, format); + if (t) texts.push(t); + if (++sinceYield >= 2000) { + sinceYield = 0; + await new Promise(r => setImmediate(r)); + } + } + } finally { + rl.close(); + } + return texts; +} - for (const line of lines) { - try { - const entry = JSON.parse(line); - let role, content; - - if (found.format === 'claude') { - if (entry.type !== 'user' && entry.type !== 'assistant') continue; - role = entry.type; - content = extractContent((entry.message || {}).content); - } else { - if (entry.type !== 'response_item' || !entry.payload) continue; - role = entry.payload.role; - if (role !== 'user' && role !== 'assistant') continue; - content = extractContent(entry.payload.content); - } +// One session's index entry, or null when it has nothing searchable. Async so +// an oversized JSONL transcript can be streamed with yields rather than +// slurped in one blocking read. +async function _indexSession(s) { + const found = findSessionFile(s.id, s.project); + if (!found) return null; + try { + const loader = SEARCH_DETAIL_LOADERS[found.format]; + let texts; + if (loader) { + texts = _searchTextsFromMessages(loader(s.id, found.file).messages); + } else if (_fileSize(found.file) > SEARCH_STREAM_THRESHOLD) { + texts = await _searchTextsFromJsonlStreaming(found.file, found.format); + } else { + texts = _searchTextsFromJsonl(found.file, found.format); + } + if (texts.length === 0) return null; + // Pre-compute lowercase full text for fast matching + return { sessionId: s.id, texts, fullText: texts.map(t => t.content).join(' ').toLowerCase() }; + } catch { + return null; + } +} - if (content && !isSystemMessage(content)) { - texts.push({ role, content: content.slice(0, 500) }); - } - } catch {} - } - } +function _fileSize(file) { + try { return fs.statSync(file).size; } catch { return 0; } +} - if (texts.length > 0) { - // Pre-compute lowercase full text for fast matching - const fullText = texts.map(t => t.content).join(' ').toLowerCase(); - index.push({ sessionId: s.id, texts, fullText }); - } - } catch {} +// Build the index in small chunks, yielding to the event loop between them. +// +// Each session here means a findSessionFile() lookup plus a full detail +// load (sync fs reads + JSON.parse per line). Doing all of them in one +// synchronous tick froze the loop for seconds on a large history — stalling +// every other request AND the terminal WebSocket data pump. Same chunk+yield +// shape as _scheduleAnalyticsRecompute, for the same reason. +async function buildSearchIndex(sessions) { + const startMs = Date.now(); + const index = []; + // Small chunk on purpose: each item is a whole detail load, an order of + // magnitude heavier than the computeSessionCost calls the analytics job + // batches 80-at-a-time. Oversized JSONL sessions additionally yield from + // *inside* _indexSession (see SEARCH_STREAM_THRESHOLD). + const CHUNK = 8; + + for (let i = 0; i < sessions.length; i += CHUNK) { + const chunk = sessions.slice(i, i + CHUNK); + for (const s of chunk) { + if (!s.has_detail) continue; + const entry = await _indexSession(s); + if (entry) index.push(entry); + } + // Yield so terminal output and other API calls stay responsive. + await new Promise(r => setImmediate(r)); } const elapsed = Date.now() - startMs; @@ -4958,19 +5007,36 @@ function buildSearchIndex(sessions) { return index; } -function getSearchIndex(sessions) { - const now = Date.now(); - if (!searchIndex || (now - searchIndexBuiltAt) > INDEX_TTL) { - searchIndex = buildSearchIndex(sessions); - searchIndexBuiltAt = now; - } +// In-flight build, so a burst of searches during a rebuild shares one job +// instead of queueing N duplicate full-history scans. +let _searchIndexBuilding = null; + +function _rebuildSearchIndex(sessions) { + if (_searchIndexBuilding) return _searchIndexBuilding; + _searchIndexBuilding = buildSearchIndex(sessions) + .then(index => { + searchIndex = index; + searchIndexBuiltAt = Date.now(); + return index; + }) + .finally(() => { _searchIndexBuilding = null; }); + return _searchIndexBuilding; +} + +// Stale-while-revalidate, mirroring getCostAnalytics: a >60s-old index is +// still overwhelmingly accurate for search, so serve it instantly and refresh +// in the background. Only the very first build (nothing cached yet) awaits — +// and even that now yields between chunks rather than blocking outright. +async function getSearchIndex(sessions) { + if (!searchIndex) return await _rebuildSearchIndex(sessions); + if ((Date.now() - searchIndexBuiltAt) > INDEX_TTL) _rebuildSearchIndex(sessions); return searchIndex; } -function searchFullText(query, sessions) { +async function searchFullText(query, sessions) { if (!query || query.length < 2) return []; const q = query.toLowerCase(); - const index = getSearchIndex(sessions); + const index = await getSearchIndex(sessions); const results = []; for (const entry of index) { diff --git a/src/server.js b/src/server.js index bf9c9ea..fbe4239 100644 --- a/src/server.js +++ b/src/server.js @@ -690,8 +690,11 @@ function startServer(host, port, openBrowser = true) { else if (req.method === 'GET' && pathname === '/api/search') { const q = parsed.searchParams.get('q') || ''; const sessions = loadSessions(); - const results = searchFullText(q, sessions); - json(res, results); + // searchFullText is async: the index build yields to the event loop + // between chunks so a cold rebuild can't stall the terminal WebSocket. + searchFullText(q, sessions) + .then(results => json(res, results)) + .catch(e => json(res, { error: e.message }, 500)); } // ── Session cost ────────────────────── diff --git a/test/search-index-chunking.test.js b/test/search-index-chunking.test.js new file mode 100644 index 0000000..08ccf33 --- /dev/null +++ b/test/search-index-chunking.test.js @@ -0,0 +1,127 @@ +'use strict'; + +// The search index is built from EVERY session with detail — a findSessionFile +// lookup plus a full detail load (sync fs + JSON.parse per line) each. Doing +// that in one synchronous tick froze the event loop for seconds on a large +// history, stalling other requests and the terminal WebSocket data pump. +// buildSearchIndex now chunks with setImmediate yields (same shape as +// _scheduleAnalyticsRecompute) and getSearchIndex serves stale-while-revalidate. +// +// Source-level contract tests, same style as running-agents-external.test.js: +// data.js reaches into the real ~/.claude tree on load, so we assert on the +// source rather than driving the real indexer. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +function src(rel) { + return fs.readFileSync(path.join(__dirname, '..', rel), 'utf8'); +} + +function fn(source, name) { + const m = source.match(new RegExp('(?:async )?function ' + name + '\\([\\s\\S]*?\\n\\}')); + assert.ok(m, name + ' should exist'); + return m[0]; +} + +// ── Chunked build, yielding to the event loop ─────────────────────────────── + +test('buildSearchIndex is async and yields between chunks', () => { + const body = fn(src('src/data.js'), 'buildSearchIndex'); + assert.match(body, /^async function/, 'must be async so it can yield mid-build'); + assert.match(body, /setImmediate/, 'must yield to the event loop between chunks'); + assert.match(body, /CHUNK/, 'must process sessions in bounded chunks'); +}); + +test('getSearchIndex serves a stale index instead of blocking on rebuild', () => { + const body = fn(src('src/data.js'), 'getSearchIndex'); + assert.match(body, /^async function/, 'must be async'); + // Only the cold path (no index at all) awaits; a stale index is returned + // immediately while the rebuild runs in the background. + assert.match(body, /if \(!searchIndex\) return await/, + 'cold start must await the first build'); + assert.match(body, /return searchIndex/, + 'a stale index must be returned without awaiting the refresh'); +}); + +test('concurrent rebuilds are deduped into one in-flight job', () => { + const source = src('src/data.js'); + assert.match(source, /_searchIndexBuilding/, 'must track the in-flight build'); + const body = fn(source, '_rebuildSearchIndex'); + assert.match(body, /if \(_searchIndexBuilding\) return _searchIndexBuilding/, + 'a second caller must join the in-flight build, not start another'); +}); + +test('callers await the now-async search', () => { + assert.match(fn(src('src/data.js'), 'searchFullText'), /await getSearchIndex/, + 'searchFullText must await the index'); + assert.match(src('src/server.js'), /searchFullText\(q, sessions\)\s*\n\s*\.then/, + '/api/search must resolve the promise before responding'); + assert.match(src('bin/cli.js'), /await searchFullText/, + 'the CLI search command must await too'); +}); + +// ── Per-format dispatch collapsed to a lookup table ───────────────────────── + +test('bespoke per-format loaders live in one table, not copy-pasted branches', () => { + const source = src('src/data.js'); + assert.match(source, /const SEARCH_DETAIL_LOADERS = \{/, 'loader table should exist'); + const table = source.match(/const SEARCH_DETAIL_LOADERS = \{[\s\S]*?\n\};/)[0]; + // Every format that previously had its own if/else branch must still resolve. + for (const format of ['qwen', 'kilo', 'opencode', 'kiro', 'kiro-cli', 'cursor', 'pi', 'copilot', 'copilot-chat']) { + assert.ok( + table.includes("'" + format + "'") || new RegExp('(^|\\s)' + format + ':').test(table), + 'format ' + format + ' must still have a loader' + ); + } + // Kilo and Kiro are different agents with different loaders — an easy + // one-character mixup when collapsing the branches. + assert.match(table, /kilo:\s*\(id\)\s*=>\s*loadKiloCliDetail/, 'kilo must map to the Kilo loader'); + assert.match(table, /'kiro-cli':\s*\(id\)\s*=>\s*loadKiroCliDetail/, 'kiro-cli must map to the Kiro loader'); +}); + +test('snippet length is one named constant, not repeated magic numbers', () => { + const source = src('src/data.js'); + assert.match(source, /const SEARCH_SNIPPET_LEN = 500/, 'snippet cap should be a named constant'); + for (const name of ['_searchTextsFromMessages', '_searchTextFromJsonlLine']) { + assert.match(fn(source, name), /SEARCH_SNIPPET_LEN/, name + ' must use the constant'); + } +}); + +test('the generic JSONL path still distinguishes claude from codex', () => { + const body = fn(src('src/data.js'), '_searchTextFromJsonlLine'); + assert.match(body, /format === 'claude'/, 'claude entries are typed user/assistant'); + assert.match(body, /response_item/, 'codex entries are wrapped in response_item payloads'); +}); + +// ── Oversized transcripts stream instead of blocking ──────────────────────── + +test('a large JSONL session is streamed, not slurped whole', () => { + const source = src('src/data.js'); + assert.match(source, /const SEARCH_STREAM_THRESHOLD/, 'a size threshold should be named'); + const body = fn(source, '_indexSession'); + assert.match(body, /SEARCH_STREAM_THRESHOLD/, 'must branch on file size'); + assert.match(body, /_searchTextsFromJsonlStreaming/, 'oversized files take the streaming path'); +}); + +test('the streaming reader yields mid-file so one huge session cannot freeze the loop', () => { + const body = fn(src('src/data.js'), '_searchTextsFromJsonlStreaming'); + assert.match(body, /^async function/, 'must be async'); + assert.match(body, /createReadStream/, 'must stream rather than readFileSync the whole file'); + assert.match(body, /setImmediate/, 'must yield to the event loop while reading'); +}); + +test('both JSONL readers share one line parser so they cannot drift', () => { + const source = src('src/data.js'); + for (const name of ['_searchTextsFromJsonl', '_searchTextsFromJsonlStreaming']) { + assert.match(fn(source, name), /_searchTextFromJsonlLine/, + name + ' must delegate to the shared per-line parser'); + } +}); + +test('_indexSession is awaited by the builder', () => { + assert.match(fn(src('src/data.js'), 'buildSearchIndex'), /await _indexSession/, + 'the builder must await the now-async per-session indexer'); +});