-
Notifications
You must be signed in to change notification settings - Fork 95
fix(wiki-worker): read current session from a local cache, not a full DB re-scan #311
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 |
|---|---|---|
|
|
@@ -17,6 +17,8 @@ import { execFileSync } from "node:child_process"; | |
| import { dirname, join } from "node:path"; | ||
| 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 { uploadSummary } from "../upload-summary.js"; | ||
| import { log as _log } from "../../utils/debug.js"; | ||
|
|
@@ -35,6 +37,7 @@ interface WorkerConfig { | |
| sessionsTable: string; | ||
| sessionId: string; | ||
| userName: string; | ||
| orgName: string; | ||
| project: string; | ||
| pluginVersion?: string; | ||
| tmpDir: string; | ||
|
|
@@ -114,27 +117,55 @@ function cleanup(): void { | |
|
|
||
| async function main(): Promise<void> { | ||
| try { | ||
| // 1. Fetch session events from sessions table | ||
| wlog("fetching session events"); | ||
| const rows = await query( | ||
| // 1. Load session events. Prefer the local per-session event cache the | ||
| // capture hook appends to as the session runs — it is row-for-row | ||
| // identical to the sessions-table `message` column but avoids re-scanning | ||
| // the entire fat `message` column on the backend for THIS session on every | ||
| // periodic / session-end trigger (the dominant cold-start cost on long | ||
| // 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` | ||
| ); | ||
|
|
||
| let usedLocalCache = false; | ||
| let rows: Record<string, unknown>[]; | ||
| const cachedLines = readSessionEventCache(cfg.sessionId); | ||
| if (cachedLines && cachedLines.length > 0) { | ||
| rows = cachedLines.map(message => ({ message })); | ||
| usedLocalCache = true; | ||
| wlog(`loaded ${rows.length} events from local cache`); | ||
| } else { | ||
| wlog("fetching session events"); | ||
| rows = await dbFetch(); | ||
| } | ||
|
|
||
| if (rows.length === 0) { | ||
| wlog("no session events found — exiting"); | ||
| return; | ||
| } | ||
|
|
||
| const jsonlLines = rows.length; | ||
| let jsonlLines = rows.length; | ||
|
|
||
| const pathRows = await query( | ||
| `SELECT DISTINCT path FROM "${cfg.sessionsTable}" ` + | ||
| `WHERE path LIKE '${esc(`/sessions/%${cfg.sessionId}%`)}' LIMIT 1` | ||
| ); | ||
| const jsonlServerPath = pathRows.length > 0 | ||
| ? pathRows[0].path as string | ||
| : `/sessions/unknown/${cfg.sessionId}.jsonl`; | ||
| // Derive the server path locally when using the cache (avoids a second | ||
| // self-session `SELECT DISTINCT path` scan); the DB branch keeps its lookup. | ||
| let jsonlServerPath: string; | ||
| if (usedLocalCache) { | ||
| jsonlServerPath = buildSessionPath( | ||
| { userName: cfg.userName, orgName: cfg.orgName, workspaceId: cfg.workspaceId }, | ||
| cfg.sessionId, | ||
| ); | ||
| } else { | ||
| const pathRows = await query( | ||
| `SELECT DISTINCT path FROM "${cfg.sessionsTable}" ` + | ||
| `WHERE path LIKE '${esc(`/sessions/%${cfg.sessionId}%`)}' LIMIT 1` | ||
| ); | ||
| jsonlServerPath = pathRows.length > 0 | ||
| ? pathRows[0].path as string | ||
| : `/sessions/unknown/${cfg.sessionId}.jsonl`; | ||
| } | ||
|
|
||
| // 2. Determine how many rows were already summarized (resumed session). | ||
| // The sidecar count is authoritative: finalizeSummary writes it after every | ||
|
|
@@ -176,6 +207,15 @@ async function main(): Promise<void> { | |
| if (sidecarCount > prevOffset) prevOffset = sidecarCount; | ||
| } | ||
|
|
||
| // Safety net: a local cache shorter than the summarized offset is an | ||
| // incomplete copy (session resumed on another machine) — re-load the full | ||
| // 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; | ||
| } | ||
|
|
||
|
Comment on lines
+210
to
+218
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. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Stale Identical issue to 🤖 Prompt for AI Agents |
||
| // 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 | ||
|
|
||
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 | 🟠 Major | ⚡ Quick win
Stale
jsonlServerPathafter safety-net DB refetch.When the cache-shorter-than-offset safety net fires,
rows/jsonlLinesare re-derived from the DB, butjsonlServerPath(computed earlier at lines 155-159 viabuildSessionPath, under the assumption the cache is authoritative) is never recomputed. The exact scenario that triggers this branch — a session resumed on another machine — is also the scenario most likely to make the locally-derived path diverge from the actual DB-stored path, since the whole point of the DB fallback here is "don't trust local assumptions."🔧 Proposed fix: recompute path when falling back
(and reuse
resolveDbPath()in the existingelsebranch above instead of duplicating the query inline.)📝 Committable suggestion
🤖 Prompt for AI Agents