fix(wiki-worker): read current session from a local cache, not a full DB re-scan#311
Merged
Merged
Conversation
… DB re-scan The wiki-worker rebuilt each session's JSONL by re-`SELECT`ing the entire `message` column for the current session on every periodic (~every 50 events) and session-end trigger: SELECT message, creation_date FROM sessions WHERE path LIKE '%<sessionId>%' ORDER BY creation_date That `path LIKE '%...%'` is an unindexed full scan of the fat `message` column for the one session the client is *already* appending to. On a long mega-session it re-materializes the whole history on a cold backend — measured in prod at 15,234 rows / 72 MB, ~1.6-2.9s total_us with only ~180ms exec_us, rss ~1GB (org 5a000e2c); and 4,711 rows / 28 MB, 13-16s cold (org 1ad8b15e). The pooler's DISCARD ALL evicts the warm handle between statements, so every trigger re-pays the cold load. A secondary `SELECT DISTINCT path ... LIKE` scan of the same self-session added ~1.1s. The client already owns this data: capture writes each normalized event to the sessions table. Now it also mirrors that exact line into a local append-only cache (~/.claude/hooks/session-cache/<id>.jsonl). The wiki-worker reads the cache (a few-ms local file read, zero backend load) instead of the fat scan, and derives the server path locally via buildSessionPath instead of the DISTINCT-path scan. The cache is a strict optimization, never a source of truth: the worker falls back to the DB SELECT whenever the cache is absent (session resumed on another machine), empty, or shorter than the offset already summarized (an incomplete local copy). Master opt-out: HIVEMIND_SESSION_EVENT_CACHE=0. Stale caches are pruned (14-day TTL) at session-end. Scope: claude-code path only. cursor/ and hermes/ wiki-workers carry the same pattern and can adopt this module as a fast follow. Tests: session-event-cache unit tests (roundtrip, opt-out, prune); wiki-worker local-cache path (no self-session SELECTs, DB fallback, short-cache refetch); capture wiring (append after successful INSERT only).
📝 WalkthroughWalkthroughAdds a best-effort per-session JSONL event cache, writes cache entries after successful Claude, Cursor, and Hermes inserts, prunes stale caches, and updates all wiki workers to prefer cached events with database fallback and resume-offset safeguards. ChangesSession cache and wiki worker flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CaptureHook
participant Database
participant SessionCache
participant WikiWorker
participant SummaryState
CaptureHook->>Database: INSERT captured event
Database-->>CaptureHook: successful INSERT
CaptureHook->>SessionCache: appendSessionEvent(sessionId, line)
WikiWorker->>SessionCache: readSessionEventCache(sessionId)
alt cache is available and sufficient
SessionCache-->>WikiWorker: ordered event lines
WikiWorker->>WikiWorker: buildSessionPath(orgName, sessionId)
else cache unavailable or shorter than prevOffset
WikiWorker->>Database: fetch session events and path
Database-->>WikiWorker: event rows and server path
end
WikiWorker->>SummaryState: finalize summary with offset
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 11 files changed
Generated for commit 14b8017. |
Apply the same self-session-fetch elimination to the cursor/ and hermes/ wiki-worker forks: their capture hooks now mirror each event into the shared local cache (session-event-cache.ts), and their workers read it instead of re-`SELECT`ing the fat `message` column (`WHERE path LIKE '%<sessionId>%'`) and the secondary `SELECT DISTINCT path` on every periodic / session-end trigger. Server path is derived locally via buildSessionPath; DB fallback and the short-cache safety net match the claude-code path. orgName is threaded through both spawn configs. Tests: cursor/hermes wiki-worker suites gain a cache-path case (no self-session SELECTs, path derived locally) and a DB-fallback case; the cursor/hermes capture-hook tests now mock session-event-cache (hermetic, no real ~/.claude writes).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/hooks/cursor/wiki-worker.ts (1)
120-168: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftCache-load + path-derivation block duplicated across worker variants.
This exact block (dbFetch closure, cache-preference check,
buildSessionPathvsSELECT DISTINCT pathbranching) is repeated verbatim insrc/hooks/hermes/wiki-worker.ts. Extracting it into a shared helper (e.g.loadSessionEventsWithCache(cfg, dbFetch)) would reduce triple-maintenance risk as this logic evolves (it already needs a fix — see the lines 210-218 comment).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/cursor/wiki-worker.ts` around lines 120 - 168, Extract the duplicated session-event loading and server-path derivation logic from the worker into a shared helper, such as loadSessionEventsWithCache, and reuse it from both wiki-worker variants. The helper should accept cfg and the dbFetch callback, preserve cache preference and fallback behavior, return the loaded rows, cache usage, JSONL count, and derived path, and keep existing logging and empty-result handling consistent.tests/cursor/cursor-wiki-worker.test.ts (1)
174-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test for the offset-mismatch (safety-net) refetch path.
Good coverage for cache-hit and cache-absent, but the new "cache shorter than
prevOffset→ refetch from DB" branch (worker lines 210-218) isn't exercised here. This is the exact path with the stale-jsonlServerPathbug flagged insrc/hooks/cursor/wiki-worker.ts; a test would have caught it.Based on path instructions, tests should assert on specific values — a suggested test would mock
readCacheMockto return fewer lines than a seededprevOffset(via an existing summary row) and assert the DBSELECT message, creation_datequery fires andSRC=reflects the DB-derived path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/cursor/cursor-wiki-worker.test.ts` around lines 174 - 226, Add a runWorker test covering the cache-offset mismatch safety-net path: have readCacheMock return fewer events than the prevOffset established by a mocked existing summary row. Assert that fetchMock receives a SELECT message, creation_date query and that the generated prompt’s SRC value uses the DB-derived session path rather than the stale cache path.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/cursor/wiki-worker.ts`:
- Around line 210-218: Update the safety-net branch in the cursor worker so the
DB fallback refreshes jsonlServerPath alongside rows and jsonlLines, using
resolveDbPath() to derive the authoritative stored path. In the existing else
branch, replace the duplicated inline path query with resolveDbPath() as well,
preserving the current local-cache path behavior when no fallback occurs.
In `@src/hooks/hermes/wiki-worker.ts`:
- Around line 210-218: Update the safety-net refetch branch in the wiki worker
so that after dbFetch replaces rows for an incomplete local cache,
jsonlServerPath is also recomputed from the authoritative DB-backed data. Keep
the existing cache-length check and refetch behavior unchanged, and mirror the
corresponding cursor worker handling.
---
Nitpick comments:
In `@src/hooks/cursor/wiki-worker.ts`:
- Around line 120-168: Extract the duplicated session-event loading and
server-path derivation logic from the worker into a shared helper, such as
loadSessionEventsWithCache, and reuse it from both wiki-worker variants. The
helper should accept cfg and the dbFetch callback, preserve cache preference and
fallback behavior, return the loaded rows, cache usage, JSONL count, and derived
path, and keep existing logging and empty-result handling consistent.
In `@tests/cursor/cursor-wiki-worker.test.ts`:
- Around line 174-226: Add a runWorker test covering the cache-offset mismatch
safety-net path: have readCacheMock return fewer events than the prevOffset
established by a mocked existing summary row. Assert that fetchMock receives a
SELECT message, creation_date query and that the generated prompt’s SRC value
uses the DB-derived session path rather than the stale cache path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7240734d-e73b-4898-947d-37c345d7ca26
📒 Files selected for processing (10)
src/hooks/cursor/capture.tssrc/hooks/cursor/spawn-wiki-worker.tssrc/hooks/cursor/wiki-worker.tssrc/hooks/hermes/capture.tssrc/hooks/hermes/spawn-wiki-worker.tssrc/hooks/hermes/wiki-worker.tstests/cursor/cursor-capture-hook.test.tstests/cursor/cursor-wiki-worker.test.tstests/hermes/hermes-capture-hook.test.tstests/hermes/hermes-wiki-worker.test.ts
Comment on lines
+210
to
+218
| // 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; | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stale jsonlServerPath after safety-net DB refetch.
When the cache-shorter-than-offset safety net fires, rows/jsonlLines are re-derived from the DB, but jsonlServerPath (computed earlier at lines 155-159 via buildSessionPath, 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
+ const resolveDbPath = async (): Promise<string> => {
+ const pathRows = await query(
+ `SELECT DISTINCT path FROM "${cfg.sessionsTable}" ` +
+ `WHERE path LIKE '${esc(`/sessions/%${cfg.sessionId}%`)}' LIMIT 1`
+ );
+ return pathRows.length > 0
+ ? pathRows[0].path as string
+ : `/sessions/unknown/${cfg.sessionId}.jsonl`;
+ };
if (usedLocalCache && rows.length < prevOffset) {
wlog(`local cache (${rows.length}) < summarized offset (${prevOffset}) — refetching from DB`);
rows = await dbFetch();
jsonlLines = rows.length;
+ jsonlServerPath = await resolveDbPath();
}(and reuse resolveDbPath() in the existing else branch above instead of duplicating the query inline.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // 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; | |
| } | |
| const resolveDbPath = async (): Promise<string> => { | |
| const pathRows = await query( | |
| `SELECT DISTINCT path FROM "${cfg.sessionsTable}" ` + | |
| `WHERE path LIKE '${esc(`/sessions/%${cfg.sessionId}%`)}' LIMIT 1` | |
| ); | |
| return pathRows.length > 0 | |
| ? pathRows[0].path as string | |
| : `/sessions/unknown/${cfg.sessionId}.jsonl`; | |
| }; | |
| // 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; | |
| jsonlServerPath = await resolveDbPath(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/cursor/wiki-worker.ts` around lines 210 - 218, Update the
safety-net branch in the cursor worker so the DB fallback refreshes
jsonlServerPath alongside rows and jsonlLines, using resolveDbPath() to derive
the authoritative stored path. In the existing else branch, replace the
duplicated inline path query with resolveDbPath() as well, preserving the
current local-cache path behavior when no fallback occurs.
Comment on lines
+210
to
+218
| // 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; | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stale jsonlServerPath after safety-net DB refetch (mirrors cursor worker).
Identical issue to src/hooks/cursor/wiki-worker.ts lines 210-218: jsonlServerPath stays locally-derived even after this branch refetches authoritative rows from the DB — precisely in the "resumed on another machine" scenario this branch is meant to guard against.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/hermes/wiki-worker.ts` around lines 210 - 218, Update the
safety-net refetch branch in the wiki worker so that after dbFetch replaces rows
for an incomplete local cache, jsonlServerPath is also recomputed from the
authoritative DB-backed data. Keep the existing cache-length check and refetch
behavior unchanged, and mirror the corresponding cursor worker handling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (measured in prod)
Every periodic (~every 50 events) and session-end summary trigger, the wiki-worker rebuilt the session JSONL by re-
SELECTing the entiremessagecolumn for the current session:path LIKE '%...%'is an unindexed full scan of the fatmessagecolumn — for the one session the client is already appending to. On a long "mega-session" it re-materializes the whole history on a cold backend:5a000e2c(admin's Org)ac1124001ad8b15e(Examen AI)c8bb9e95The pooler's
DISCARD ALLevicts the warm pg-deeplake handle between statements, so every trigger re-pays the cold load. A secondarySELECT DISTINCT path ... LIKE '%<sessionId>%'scan of the same self-session added ~1.1s. Two prod orgs confirm the pattern.This is a client query-pattern issue, not a backend rebuild/commit-bloat issue (both tables are healthy — 21 active commits on
5a000e2c).Fix
The client already owns this data — capture writes every normalized event to the sessions table. Now it also mirrors that exact line into a local append-only cache (
~/.claude/hooks/session-cache/<id>.jsonl, new modulesession-event-cache.ts). The wiki-worker:SELECT messagescan;buildSessionPathinstead of the secondarySELECT DISTINCT pathscan.The cache lines are row-for-row identical to the DB
messagecolumn (capture builds oneJSON.stringify(entry)line and writes it to both), so the summarizer's offset bookkeeping is unchanged.Safety — strict optimization, DB stays source of truth
Falls back to the DB
SELECT(with the existing retry loop) whenever the cache is:Master opt-out:
HIVEMIND_SESSION_EVENT_CACHE=0. Stale caches pruned (14-day TTL) at session-end; a freshly-written current-session cache is never touched.Scope — all three agents
Commit 1 (
82d3ae3e): claude-code path. Commit 2 (0254a720): the identical cursor/ and hermes/ wiki-worker forks now share the samesession-event-cache.tsmodule (capture appends; worker reads with DB fallback;orgNamethreaded through both spawn configs).Complementary server-side lever (out of scope, separate pg-deeplake/pooler change): keep the pg-deeplake in-process handle warm across
DISCARD ALLso cold re-loads don't recur fleet-wide.Tests
session-event-cache.test.ts— append/read roundtrip, one-line-per-event with embedded newlines, opt-out flag, TTL prune, non-.jsonlskip.wiki-worker-local-cache.test.ts(+ cursor/hermes wiki-worker suites) — cache path issues no self-sessionSELECTs and derives the path locally; DB fallback when cache absent/empty; refetch when cache shorter than offset.Full suite: all previously-green tests stay green (pre-existing failures are unrelated —
tests/shared/graph/*needs the nativetree-sitter-pythondep, andflush-memory"no-op without network" is login/network-state dependent).🤖 Generated with Claude Code
Summary by CodeRabbit