-
Notifications
You must be signed in to change notification settings - Fork 94
fix(wiki-worker): bound the session-events DB fallback #328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ import { fileURLToPath } from "node:url"; | |
| import { finalizeSummary, releaseLock, readState } from "../summary-state.js"; | ||
| import { readSessionEventCache } from "../session-event-cache.js"; | ||
| import { buildSessionPath } from "../../utils/session-path.js"; | ||
| import { capLinesByBytes, stampOffset, WIKI_JSONL_MAX_BYTES } from "../wiki-offset.js"; | ||
| import { capLinesByBytes, newRowsFromWindow, stampOffset, WIKI_FALLBACK_MAX_ROWS, WIKI_JSONL_MAX_BYTES } from "../wiki-offset.js"; | ||
| import { redactSecrets } from "../shared/redact.js"; | ||
| import { uploadSummary } from "../upload-summary.js"; | ||
| import { log as _log } from "../../utils/debug.js"; | ||
|
|
@@ -126,12 +126,26 @@ async function main(): Promise<void> { | |
| // mega-sessions). Falls back to the DB whenever the cache is absent | ||
| // (session resumed on another machine), empty, or — once the offset is | ||
| // known — shorter than the offset already summarized. | ||
| const dbFetch = () => query( | ||
| `SELECT message, creation_date FROM "${cfg.sessionsTable}" ` + | ||
| `WHERE path LIKE E'${esc(`/sessions/%${cfg.sessionId}%`)}' ORDER BY creation_date ASC` | ||
| ); | ||
| // Bounded DB fallback: the NEWEST WIKI_FALLBACK_MAX_ROWS rows (reversed to | ||
| // chronological) plus the true `total`. The old unbounded `ORDER BY ASC` | ||
| // materialized the whole fat `message` column (tens of MB, ~30s cold on a | ||
| // mega-session) even though only the newest un-summarized rows are consumed. | ||
| // `count(*)` reads no fat column, so `total` is cheap and lets the offset | ||
| // math (`newRowsFromWindow`) and the stamped offset stay correct. | ||
| const dbFetch = async (): Promise<{ rows: Record<string, unknown>[]; total: number }> => { | ||
| const like = esc(`/sessions/%${cfg.sessionId}%`); | ||
| const cnt = await query(`SELECT count(*) AS n FROM "${cfg.sessionsTable}" WHERE path LIKE E'${like}'`); | ||
| const total = Number(cnt[0]?.["n"] ?? 0); | ||
| if (total === 0) return { rows: [], total: 0 }; | ||
| const r = await query( | ||
| `SELECT message, creation_date FROM "${cfg.sessionsTable}" ` + | ||
| `WHERE path LIKE E'${like}' ORDER BY creation_date DESC LIMIT ${WIKI_FALLBACK_MAX_ROWS}` | ||
| ); | ||
| return { rows: r.reverse(), total }; | ||
| }; | ||
|
Comment on lines
+135
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Both workers implement
The same 📍 Affects 2 files
🤖 Prompt for AI Agents |
||
|
|
||
| let usedLocalCache = false; | ||
| let dbTotal = 0; // true row count when the bounded DB path was used (else 0) | ||
| let rows: Record<string, unknown>[]; | ||
| const cachedLines = readSessionEventCache(cfg.sessionId); | ||
| if (cachedLines && cachedLines.length > 0) { | ||
|
|
@@ -140,15 +154,19 @@ async function main(): Promise<void> { | |
| wlog(`loaded ${rows.length} events from local cache`); | ||
| } else { | ||
| wlog("fetching session events"); | ||
| rows = await dbFetch(); | ||
| const f = await dbFetch(); | ||
| rows = f.rows; | ||
| dbTotal = f.total; | ||
| } | ||
|
|
||
| if (rows.length === 0) { | ||
| wlog("no session events found — exiting"); | ||
| return; | ||
| } | ||
|
|
||
| let jsonlLines = rows.length; | ||
| // The offset high-water (stamped into the summary) must be the TRUE total, not the | ||
| // bounded window length — else the next run's offset regresses and re-summarizes. | ||
| let jsonlLines = usedLocalCache ? rows.length : dbTotal; | ||
|
|
||
| // Derive the server path locally when using the cache (avoids a second | ||
| // self-session `SELECT DISTINCT path` scan); the DB branch keeps its lookup. | ||
|
|
@@ -213,15 +231,20 @@ async function main(): Promise<void> { | |
| // session from the DB so no genuinely-new rows get sliced to nothing. | ||
| if (usedLocalCache && rows.length < prevOffset) { | ||
| wlog(`local cache (${rows.length}) < summarized offset (${prevOffset}) — refetching from DB`); | ||
| rows = await dbFetch(); | ||
| jsonlLines = rows.length; | ||
| const f = await dbFetch(); | ||
| rows = f.rows; | ||
| dbTotal = f.total; | ||
| jsonlLines = dbTotal; | ||
| usedLocalCache = false; | ||
| } | ||
|
|
||
| // Feed the agent only the rows added since the last summary. Reprocessing | ||
| // the full session on every run is what drives ENOBUFS / 120s-timeout | ||
| // failures on long (4000+ event) sessions — a stuck offset re-summarizes | ||
| // everything from scratch. | ||
| const newRows = prevOffset > 0 ? rows.slice(prevOffset) : rows; | ||
| const newRows = usedLocalCache | ||
| ? (prevOffset > 0 ? rows.slice(prevOffset) : rows) | ||
| : newRowsFromWindow(rows, dbTotal, prevOffset); | ||
| if (prevOffset > 0 && newRows.length === 0) { | ||
| wlog(`no new events since last summary (offset=${prevOffset}, total=${jsonlLines}) — skipping`); | ||
| return; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: activeloopai/hivemind
Length of output: 159
🏁 Script executed:
Repository: activeloopai/hivemind
Length of output: 16901
🏁 Script executed:
Repository: activeloopai/hivemind
Length of output: 486
🏁 Script executed:
Repository: activeloopai/hivemind
Length of output: 7317
🌐 Web query:
PostgreSQL LIKE operator backslash default escape E'' standard_conforming_strings documentation💡 Result:
In PostgreSQL, the backslash character () serves two distinct functions that often cause confusion: one as a literal string escape mechanism and another as an escape character specifically for the LIKE operator's pattern matching [1][2][3]. 1. String Literal Parsing and standard_conforming_strings: The configuration parameter standard_conforming_strings controls how backslashes are interpreted within ordinary SQL string literals ('...'). - When standard_conforming_strings is on (the default in modern PostgreSQL), backslashes in ordinary string literals are treated as literal backslash characters [2][4]. - When standard_conforming_strings is off, backslashes in ordinary string literals are treated as escape characters (e.g., '\n' becomes a newline) [2][4]. - To represent backslash escape sequences reliably, you should use E-prefixed string constants (e.g., E'\'), which treat backslashes as escape characters regardless of the standard_conforming_strings setting [2][5]. 2. LIKE Operator Escape Behavior: Regardless of the standard_conforming_strings setting, the LIKE operator always uses the backslash as its default escape character for pattern matching [1][3]. This means that within a LIKE pattern, a backslash is used to escape wildcard characters like '%' and '' [1][6]. - To match a literal backslash in a LIKE clause, you must escape it with another backslash (e.g., '\') [1][5]. - Because of the two-layer interpretation (string parsing + LIKE pattern matching), searching for a literal backslash often requires multiple backslashes, such as '\\' in an ordinary string literal, or '\\' in an E-string literal [3]. - You can override the default LIKE escape character using the ESCAPE clause [1][6]. For example, LIKE '%#%' ESCAPE '#' treats '#' as the escape character instead of the backslash [1][3]. To disable the LIKE escape mechanism entirely, you can use ESCAPE '' [1][7]. In summary, standard_conforming_strings governs how string constants are parsed, whereas the LIKE operator's use of the backslash as an escape character is a fixed behavior of the pattern matching engine [1][3]. Applications requiring reliable handling of backslashes should use E-prefixed string constants and explicitly specify an ESCAPE character for LIKE patterns if needed [1][2].
Citations:
🏁 Script executed:
Repository: activeloopai/hivemind
Length of output: 17833
🏁 Script executed:
Repository: activeloopai/hivemind
Length of output: 15321
🌐 Web query:
PostgreSQL E string literal backslash LIKE pattern default escape documentation💡 Result:
In PostgreSQL, there are two distinct mechanisms involving backslashes: string constant escaping (lexical level) and LIKE pattern escaping (pattern matching level). LIKE Pattern Escape In a LIKE pattern, the default escape character is the backslash () [1][2]. This allows you to match literal underscore (_) or percent sign (%) characters by preceding them with the backslash (e.g., '%') [1][2]. To match a literal backslash, you write two backslashes ('\') [1][2]. You can change this default behavior by using the ESCAPE clause (e.g., LIKE 'pattern' ESCAPE '!') or disable it entirely by using ESCAPE '' [1][2]. PostgreSQL's default of using the backslash as an escape character is a nonstandard extension to the SQL standard, which specifies that there should be no default escape character [1][2]. E String Literal An E string literal (e.g., E'string') is a lexical extension that enables C-style backslash escape sequences (such as \n for newline or \xHH for hexadecimal bytes) [3][4]. Because these sequences are interpreted by the parser when the query is read, a backslash intended to be part of a LIKE pattern must be doubled if it is inside an E string literal [3][4]. For example, to match a literal percent sign using an E string literal, you would write E'\%'—the first backslash escapes the second one for the string constant, leaving a single backslash for the LIKE operator to use as an escape character [1][3]. Important Configuration Note Since PostgreSQL 9.1, the default setting for the configuration parameter standard_conforming_strings is ON [3][5]. This means that backslashes in standard (non-E) string constants are treated as literal characters rather than escape sequences [3][5]. Consequently, it is recommended to use E string literals only when backslash escape sequences are actually needed [3][5].
Citations:
Use plain quotes for these LIKE patterns
esc()already returns a string for a plain SQL literal;E'...'adds an extra backslash-parse step before LIKE sees the pattern, so backslash-containing session IDs/paths can match differently here. Use the plain'${likePat}'form to match the other queries in these files.src/hooks/codex/wiki-worker.ts#L145-L154src/hooks/pi/wiki-worker.ts#L130-L139📍 Affects 2 files
src/hooks/codex/wiki-worker.ts#L138-L158(this comment)src/hooks/pi/wiki-worker.ts#L123-L143🤖 Prompt for AI Agents