Conversation
… check Refactor the hot-path session-start and capture hooks to do less synchronous network work, and introduce a disk-backed write queue so per-event inserts no longer block the user. Highlights: - New src/hooks/session-queue.ts: append-only JSONL queue per session with inflight rename, stale recovery, batched INSERT flush, and auth-failure disable state. Flushed on Stop/SubagentStop and SessionEnd. - src/hooks/capture.ts now enqueues rows locally instead of issuing one INSERT per event; flush happens at turn boundaries. - src/hooks/session-start.ts slimmed to local-only work (credentials + context injection). All network work (table setup, placeholder row, queue drain, version check, auto-update) moved to session-start-setup.ts. - New src/hooks/version-check.ts with cached latest-version lookup (TTL) so we don't hit GitHub on every session start. - New src/virtual-path-scope.ts centralizes /sessions/ vs memory routing; pre-tool-use and grep-core consult it for ls/find/grep scoping so sessions and memory are queried in parallel only when the path covers both. - grep-core gains a regex literal prefilter helper so content scans can still leverage a LIKE anchor when a safe substring exists. - Matching changes on the codex side (capture/pre-tool-use/session-start/ session-start-setup/stop) and regenerated bundles for both plugins. - Tests: new session-queue.test.ts and version-check.test.ts; updates to session-start, grep-core, grep-interceptor, deeplake-api, and codex integration tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s, requeue race - session-queue: escape backslashes before single quotes so JSON payloads survive SQL backends with standard_conforming_strings=off. - version-check: strip pre-release tags before Number() so 1.2.3-beta compares deterministically instead of collapsing to NaN. - session-queue: requeueInflight now appends inflight content via appendFileSync unconditionally, removing the existsSync→renameSync TOCTOU window where a concurrent capture append could be overwritten. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The virtual /index.md served from the Deeplake-backed memory path was only listing rows from the `memory` table (summaries), so in workspaces where the memory table is empty or has been dropped (e.g. locomo_benchmark/baseline) the index falsely reported "0 sessions" / "1 sessions" even when the `sessions` table held hundreds of rows. Agents reading the index would conclude memory was empty and give up on retrieval. Extend `buildVirtualIndexContent` to accept both summary and session rows and render them under `## Summaries` and `## Sessions` sections, with a combined header like `273 entries (1 summaries, 272 sessions):`. Update the fallback branch in `readVirtualPathContents` to query both tables in parallel and pass the results to the new builder. Verified against the locomo baseline benchmark: the same three QAs that previously saw a 1-entry index (conv 0 / qa 6, 25, 46) now receive the full listing on the fast-path cat index.md call, and the generated index matches the 272 sessions ingested into the baseline workspace.
Lock in the fix that made `buildVirtualIndexContent` aware of session rows and the fallback path in `readVirtualPathContents` query both tables when /index.md has no physical row. New unit tests for `buildVirtualIndexContent`: - renders both sections with a combined "N entries (X summaries, Y sessions):" header when both tables have rows, with Summaries listed before Sessions - renders only sessions when the memory table is empty (guards the baseline_cloud regression where the old output reported "0 sessions:" despite 272 rows in the sessions table) - stays backwards-compatible for callers that pass only summary rows - produces a well-formed empty index when both inputs are empty New integration tests for `readVirtualPathContents`: - when /index.md has no physical row, the fallback issues three queries (union for exact paths + two parallel fallback queries) and each fallback targets the correct table and LIKE filter - the synthesized index still renders summaries if the sessions-table fallback query rejects One existing test (`reads multiple exact paths in a single query and synthesizes /index.md when needed`) was updated to expect three calls instead of two, matching the new dual-table fallback behavior.
…al QAs Adds integration coverage for the three LoCoMo QAs that cloud baseline got wrong before the /index.md fix landed (conv_0 questions 6, 25, 46): - qa_6 : "When is Melanie planning on going camping?" (gold: June 2023) - qa_25 : "When did Caroline go to the LGBTQ conference?" (10 July 2023) - qa_46 : "Would Melanie be considered an ally..." (Yes, she is supportive) Each QA is driven through `processPreToolUse` twice — once via the Read-tool intercept (`Read /home/.deeplake/memory/index.md`) and once via the Bash intercept (`cat /home/.deeplake/memory/index.md`) — against a DeeplakeApi mock that mirrors the real sessions-only baseline workspace at the time of the regression (memory table empty, 272 rows across conv_0..9 in the sessions table). The assertions verify the synthesized index reports "272 entries (0 summaries, 272 sessions):", contains the specific session file each QA needed (conv_0_session_2 for the camping date, conv_0_session_7 for the conference, conv_0_session_10 for the ally question), and does not regress to "0 sessions:" or "1 sessions:" headers. The suite also exercises the pure builder and the `readVirtualPathContents` fallback against the same 272-row fixture so the regression is caught at the unit, integration, and entry-point boundaries. Tests run hermetically by stubbing the disk-backed session cache so they do not read or write ~/.deeplake/query-cache/. Verified by temporarily reverting the fix on virtual-table-query.ts: all eight assertions fail without the fix (0 sessions: header, missing session paths), then pass cleanly once the fix is restored.
Claude Code hooks replace the tool input with whatever `updatedInput`
they emit. The pre-tool-use hook was always emitting
`{command, description}` — the Bash-tool shape — even when the incoming
tool was Read. The Read implementation then read `updatedInput.file_path`,
found `undefined`, and crashed with:
"The 'path' property must be of type string, got undefined"
Claude wasted a turn (or more) recovering by re-issuing the read as a
Bash `cat`. In the plugin-v8-optimizations-100 run (memory table
populated, 272 summaries), 60 / 100 transcripts contained this error.
In the sessions-only baseline_cloud run it was even worse because the
recovery path hit fix #1's `/index.md` bug on top.
The fix teaches the hook to materialize Read intercepts into a real
file on disk and return the path:
- Add an optional `file_path` field to ClaudePreToolDecision. When
present, main() emits `updatedInput: {file_path}` instead of the
Bash-shaped `{command, description}`.
- Add `writeReadCacheFile(sessionId, virtualPath, content)` which
writes into `~/.deeplake/query-cache/<sessionId>/read/<virtualPath>`,
mirroring the per-session cache the index already uses. Cleanup
reuses the existing session-end path.
- Add `buildReadDecision(file_path, description)` so the call site is
explicit about the Read-tool shape.
- Branch in the direct-read code path: when `input.tool_name ===
"Read"`, write the fetched content via `writeReadCacheFile` and
return `buildReadDecision(...)`. Bash cat / head / tail / wc keep
their existing `echo <content>` shape.
- Thread `writeReadCacheFileFn` through the existing deps so tests
can stub it and stay hermetic.
Test updates:
- `hooks-source.test.ts > reuses cached /index.md content ...` now
asserts `directDecision?.file_path` instead of `.command` for the
Read variant, with a stubbed cache writer that captures the written
content.
- `hooks-source.test.ts > uses direct grep, direct reads, listings ...`
updated the Read assertion the same way.
- `pre-tool-use-baseline-cloud-3qa.test.ts` Read cases now assert
that the decision carries `file_path` (bug #2 guard) while the Bash
cases confirm `command` still exists (bash shape preserved).
Verified: stashing the fix causes all three Read-tool per-QA tests
to fail; restoring the fix makes them pass.
End-to-end verified against locomo_benchmark/baseline (272 sessions,
memory dropped) on a 5-QA subset spanning conv 0 questions 6 / 25 / 29
/ 46 / 62 — five QAs that baseline-local answered correctly and the
original baseline_cloud run got wrong. Post-fix run: 5 / 5 correct,
0 occurrences of "property must be of type string" across the five
transcripts. (Haiku happened to pick Bash over Read for each QA in
this run, so the Read intercept didn't fire in-flight; the unit tests
and the earlier fix1b transcript where Read was attempted cover that
path.)
…ons/* Read Extends the integration test suite for fix #1 and fix #2 with two more QAs — qa_3 (Caroline's research) and qa_29 (Melanie's pottery workshop) — bringing the REAL_QAS pool to five. qa_3 specifically maps to the Read calls that fired in the `baseline_cloud_9qa_read_candidates_fix2` benchmark run (three Read calls, all against memory paths), so its inclusion anchors the test suite against live behavior observed on the sessions-only `locomo_benchmark/baseline` workspace. Adds a dedicated test for the other Read-tool regression surface: a Read against a /sessions/<file>.json path (not only /index.md). The same benchmark run showed haiku calling `Read /home/.deeplake/memory/sessions/conv_0_session_{1,2}.json` directly; the new test feeds that exact shape through `processPreToolUse`, asserts the decision carries `file_path` (not `command`), and verifies the session JSON body is materialized to the read cache at the expected virtual path. Renames the test file from `pre-tool-use-baseline-cloud-3qa.test.ts` to `pre-tool-use-baseline-cloud.test.ts` now that it covers more than three QAs. Verification: 13 / 13 tests pass; temporarily stashing the fix #2 source change makes the new per-QA Read assertions and the /sessions Read assertion all fail (decision.file_path is undefined), restoring the source brings them back to green.
Claude Code's Bash tool merges the child process's stderr into the
tool_result string the model sees. When a user or CI had
HIVEMIND_TRACE_SQL=1 or HIVEMIND_DEBUG=1 exported, every SQL query
issued by the shell bundle during `node shell-bundle -c "..."` wrote
a `[deeplake-sql] query start:` line to stderr — and all of it landed
in Claude's view of the command output, drowning out the real data.
Confirmed on the original baseline_cloud-100 run: 35+ trace lines
across the transcripts, interleaved with the bash command results
Claude was trying to parse. In several QAs the SQL noise replaced the
useful output entirely (exit code 1 + trace lines → Claude concluded
"no matches").
Two-part fix:
1. Move the TRACE_SQL / DEBUG_FILE_LOG env checks out of the top-level
module constants in `src/deeplake-api.ts` and into the `traceSql`
function body. The check now evaluates per-call, so callers that
import the SDK can still flip the env vars at runtime. (Previously
the constants were frozen at module load, so any downstream delete
had no effect.)
2. In `src/shell/deeplake-shell.ts`, detect one-shot mode (`-c` in
argv) up front and `delete process.env[...]` the four trace
variables before doing anything else. Interactive REPL mode keeps
the env untouched so developers still get `[deeplake-sql]` lines
when they set the vars intentionally.
Test coverage in `claude-code/tests/shell-bundle-sql-trace-silence.test.ts`:
- Spawns the built `claude-code/bundle/shell/deeplake-shell.js` with
fake creds and HIVEMIND_TRACE_SQL / DEEPLAKE_TRACE_SQL /
HIVEMIND_DEBUG / DEEPLAKE_DEBUG all set to "1", pointed at an
unreachable API URL with a 200ms query timeout. After the SQL query
fails (expected), asserts stderr is free of `[deeplake-sql]` lines.
- A source-level check confirms `traceSql` reads the env vars inside
the function body (runtime) rather than via a frozen top-level
`const TRACE_SQL`.
Regression verified: stashing both source changes causes the bundle
test to fail with the expected `[deeplake-sql] query fail:` line in
stderr and the source-level test to report the reintroduced top-level
const; restoring the source brings both green.
End-to-end verified against `locomo_benchmark/baseline` on a 6-QA
subset (conv 0 QAs 3 / 11 / 27 / 32 / 59 / 65). Before fix: 2–4 SQL
trace lines leaked into each QA's tool_result stream. After fix: zero
leaks across all six transcripts. qa_3 and qa_11 (already correct with
fix #1 + fix #2) stay correct; the hard QAs (27, 32, 59, 65) continue
to show judge-score variance under Haiku non-determinism but are no
longer looking at SQL noise as their "retrieval result".
`sqlLike(value)` escapes `_` and `%` in the value by prefixing them with
backslashes so callers can interpolate user-controlled strings inside
`LIKE 'pattern'` literals. But the Deeplake SQL backend does not treat
backslash as the LIKE escape character by default — without an explicit
`ESCAPE '\'` clause, `\_` becomes two literal characters in the pattern
instead of a literal `_`, so queries whose paths contain underscores
silently return nothing.
Empirically reproduced on the `locomo_benchmark/baseline` workspace:
grep -l Caroline /home/.deeplake/memory/sessions/*.json
→ returns 20+ session paths (works: path has no underscores past
the final slash, sqlLike produces '/sessions/%.json')
grep -i hike /home/.deeplake/memory/sessions/conv_0_session_*.json
→ returns (no matches) before this fix — because the SQL becomes
path LIKE '/sessions/conv\_0\_session\_%.json' and Deeplake
matches `\_` literally against `_` → zero rows
→ returns real matches after this fix (ESCAPE '\' added, `\_` is
now interpreted as literal `_`, matches the underscored paths)
Same symptom in the 100-QA post-fix baseline_cloud run: 15 / 100 QA
that local baseline answered correctly came back wrong/partial in the
cloud, and the tool-call transcripts show repeated `(no matches)` on
grep commands whose glob mentions `conv_<c>_session_*.json`.
The fix appends ` ESCAPE '\'` to every `LIKE '...'` clause that is
fed from `sqlLike()`:
- src/shell/grep-core.ts:buildPathCondition — both the wildcard path
branch and the directory-prefix branch.
- src/hooks/virtual-table-query.ts:buildDirFilter — per-dir
`path LIKE '<dir>/%'` clauses used by listVirtualPathRowsForDirs.
- src/hooks/virtual-table-query.ts:findVirtualPaths — both the
memoryTable and sessionsTable branches, on both the path and the
filename LIKE clauses.
Codex/Claude Code find fallbacks and `bash-command-compiler`'s
`find_grep` path ultimately call `findVirtualPaths`, so they inherit
the fix without a local change.
Rebuild updates the 8 Claude Code and 8 Codex bundles.
Verified via a targeted reproducer that drives `processPreToolUse`
with the same glob commands against the real baseline workspace: all
three underscored-glob greps return real matches after the fix, where
previously they returned `(no matches)`.
…review truncation Claude Code's Bash tool silently persists any tool_result larger than ~16 KB to disk and replaces it with a 2 KB preview plus a path to the persisted file. The model almost never recovers from that replacement: in the locomo `baseline_cloud_100qa_fix123` run (100 QA, all fixes #1 / #2 / #3 applied), 11 / 14 losing QAs that hit the persist path never read the persisted file even once, and finished on the truncated 2 KB preview — which was rarely enough to carry the answer. Typical triggers from that run: - `grep -r Caroline /home/.deeplake/memory/` → 66 KB of dialogue lines because the name appears in nearly every session. - `for f in /.../sessions/conv_0_session_*.json; do grep ...; done` → 926 KB of concatenated grep output (slow-path shell bundle). - `cat /.../sessions/conv_0_session_*.json` (glob over many files) → tens of KB of JSON. This fix introduces `src/utils/output-cap.ts` with `capOutputForClaude(output, {kind})` and applies it on the plugin's exit paths before Claude Code sees the result: - `grep-direct.ts:handleGrepDirect` — caps grep's combined output. - `bash-command-compiler.ts:executeCompiledBashCommand` — caps the final concatenation of compiled segments (cat / ls / find / grep / find_grep, incl. `&&` and `;` pipelines). - `pre-tool-use.ts` direct read path — caps `cat` / `head` / `tail` Bash intercepts. Read-tool intercepts are unaffected: they write content to disk and return a `file_path`, so no size pressure from Claude Code's preview truncation applies. - `pre-tool-use.ts` direct `ls` and `find` fallbacks — capped too. Cap is 8 KB (CLAUDE_OUTPUT_CAP_BYTES), comfortably under Claude Code's ~16 KB persist threshold and 4× the 2 KB preview the model used to get. When the cap fires, the output is truncated at a line boundary and the tail gets a short footer: ... [grep truncated: 313 more lines (58.4 KB) elided — refine with '| head -N' or a tighter pattern] The footer names the operation (grep / cat / ls / find / bash) and gives the model an actionable next step. Unit tests in `claude-code/tests/output-cap.test.ts` (8 tests): - No-op for inputs that fit the cap, including empty strings. - Byte size after cap is ≤ CLAUDE_OUTPUT_CAP_BYTES. - Truncation aligns to line boundaries; footer line counts add up to the original total. - Single oversized line (no newline) is byte-sliced with a footer. - Custom `maxBytes` is honoured (no silent 1 KB floor). - Default footer kind is "output" when no kind is passed. - A realistic 400-line grep fixture that exceeds 16 KB gets capped above 4 KB and under the cap — strictly more useful than the 2 KB preview. Bundle rebuild propagates the change to the 8 Claude Code and 8 Codex bundles. Verified empirically via `processPreToolUse` against the real `locomo_benchmark/baseline` workspace: grep -r Caroline /home/.deeplake/memory/ before fix #5: ~66 KB of output, Claude Code truncated to 2 KB. after fix #5: ~7.9 KB (313 lines kept, 313 more elided, footer). grep -r 'Caroline|Melanie' /home/.deeplake/memory/ before: ~70 KB. after: ~7.9 KB with footer reporting 391 lines elided. cat /home/.deeplake/memory/sessions/conv_0_session_1.json ~2 KB — unchanged, well under the cap. Expected impact on the 100-QA baseline_cloud benchmark: 11 QAs that lost points purely because of the 2 KB preview now see up to 8 KB of the same grep output. Combined with fix #4 (19 QAs with (no matches) from SQL LIKE under-escaping), the plugin should close the remaining ~7.5 pt gap to the local-files baseline (75.0 %) and likely match or exceed it.
Append per-file thresholds in vitest.config.ts for the two source files that materially changed in this PR, holding them at the same 90 / 90 / 90 / 90 bar already applied to the grep-dual-table files from PR #60: - src/utils/output-cap.ts — new file, fix #5. Currently at 100 / 100 / 100 / 100 under the tests in claude-code/tests/output-cap.test.ts. - src/hooks/virtual-table-query.ts — rewritten for fix #1 (dual-table index generation) and fix #4 (ESCAPE '\' on LIKE clauses). Currently at 98.9 / 93.2 / 95.8 / 98.9 under claude-code/tests/virtual-table-query.test.ts and claude-code/tests/pre-tool-use-baseline-cloud.test.ts. Files left without new thresholds because their changes in this PR are small and localized: - src/hooks/pre-tool-use.ts — added a Read-intercept branch and a writeReadCacheFile helper; the broader file is covered by hooks-source.test.ts which is pre-failing on this branch (unrelated to the fixes in this PR). - src/deeplake-api.ts — moved TRACE_SQL from a module-level const into the traceSql function body (fix #3). - src/shell/deeplake-shell.ts — three env-var deletes in the one-shot entry (fix #3).
…sessions # Conflicts: # claude-code/bundle/capture.js # claude-code/bundle/session-end.js # claude-code/bundle/session-start-setup.js # claude-code/bundle/session-start.js # codex/bundle/capture.js # codex/bundle/session-start-setup.js # codex/bundle/session-start.js # codex/bundle/stop.js # src/hooks/capture.ts # src/hooks/codex/capture.ts # src/hooks/codex/session-start-setup.ts # src/hooks/codex/session-start.ts # src/hooks/codex/stop.ts # src/hooks/session-end.ts # src/hooks/session-start-setup.ts # src/hooks/session-start.ts
…ix #4 Fix #4 (`3d15454`) appended `ESCAPE '\'` to every LIKE clause fed by `sqlLike()` so backslash-escaped `_` / `%` match their literal characters on the Deeplake backend. The existing buildPathFilter glob test still asserted the pre-fix SQL. Update the literal string and the regex so the assertion matches the new SQL shape, and annotate the case with a comment explaining why the ESCAPE clause is required.
The `pull_request.branches:` filter matches on the base branch of a PR. With `[main, dev]` the CI workflow (typecheck + jscpd duplication check + coverage report) silently skipped any PR targeting a long- lived feature branch like `optimizations`. Only "PR Checks" and "Claude PR Review" ran on those PRs, so the coverage and dup report comments never showed up. Dropping the filter runs CI on every PR; the push side stays limited to main/dev so we don't double-run on personal branch pushes.
The merge of `origin/main` pulled in the canonical source refactors for the Codex hooks (session-start / session-start-setup / stop) but the corresponding tests on Davit's `optimizations` branch were written against an intermediate refactor state where helpers like `runCodexSessionStartSetup`, `extractLastAssistantMessage`, `buildCodexStopEntry`, `runCodexStopHook`, and the matching `claude-code/tests/hooks-source.test.ts` imports never made it into the exported surface. CI was failing with 39 `TypeError: X is not a function` errors. Two broken test files are deleted (they never existed on `origin/main` and their coverage is already provided by the canonical suites added by PR #62, which landed on `main` and came in with this merge): - `claude-code/tests/hooks-source.test.ts` (894 LOC, 19 / 30 failing) - `codex/tests/codex-source-hooks.test.ts` (1126 LOC, 20 / 28 failing) The canonical replacements from `main` cover the same ground: - `claude-code/tests/capture-hook.test.ts` - `claude-code/tests/session-start-hook.test.ts` - `claude-code/tests/session-start-setup-hook.test.ts` - `claude-code/tests/session-end-hook.test.ts` - `claude-code/tests/codex-capture-hook.test.ts` - `claude-code/tests/codex-session-start-hook.test.ts` - `claude-code/tests/codex-session-start-setup-hook.test.ts` - `claude-code/tests/codex-stop-hook.test.ts` - `claude-code/tests/codex-wiki-worker.test.ts` Two test files also merged in with Davit-branch test blocks that asserted stale session-start prompt wording. Restored to main's version: - `claude-code/tests/session-start.test.ts` — dropped the "steers recall tasks toward index-first exact file reads" block; main's session-start prompt uses different phrasing. - `codex/tests/codex-integration.test.ts` — restored main's assertions ("Do NOT jump straight to JSONL" instead of "Do NOT jump straight to raw session files"). Verified: `npx vitest run` — 837 / 837 tests pass across 39 files. Per-file coverage thresholds unaffected (output-cap.ts 100%, virtual-table-query.ts 98.9% lines, grep-core.ts / grep-direct.ts / grep-interceptor.ts / session-queue.ts all above their bars).
…ine count Three issues flagged by the automated review on PR #63: 1. `writeReadCacheFile` (src/hooks/pre-tool-use.ts) had no containment guard: `path.join(cacheRoot, session, "read", rel)` resolves `..` segments in `rel`, so a DB-controlled `virtualPath` could escape the per-session cache dir. Added a check that `absPath` stays under `expectedRoot = join(cacheRoot, session, "read")` and throws `"writeReadCacheFile: path escapes cache root: <abs>"` otherwise. Uses `path.sep` so the boundary check is correct on any platform. 2. The inline `/index.md` fallback in `processPreToolUse` (pre-tool- use.ts:334-347) was unreachable after fix #1 landed, and if somehow reached would regenerate the old broken single-table index (queries only `memory`, uses the header "${n} sessions:", omits `## Sessions`). Removed; the dual-table builder in `virtual-table-query.ts` now owns index generation exclusively. 3. `src/utils/output-cap.ts` had a dead `cut += lineBytes` accumulator (would trigger `noUnusedLocals` under strict TS config) and a trailing-newline off-by-one: `output.split("\n")` on `"a\nb\n"` returns `["a", "b", ""]`, so `totalLines` over-counted by 1 whenever the input ended with a newline — which grep and cat both do. The footer reported one extra "elided line" that was the empty terminator, not a real content line. Dropped the dead accumulator and adjusted totalLines to subtract the trailing empty entry. Test coverage: - `claude-code/tests/pre-tool-use-baseline-cloud.test.ts` — 4 new cases on `writeReadCacheFile`: happy path, `../../../etc/passwd` traversal refused (and no file lands anywhere under cacheRoot), absolute-root escape refused, and a path that normalizes back inside the cache (`/sessions/foo/../bar.json`) is still accepted. Plus one integration test that pins the removal of the inline /index.md fallback: `processPreToolUse` must materialize the dual-table builder's content and must NOT issue its own `FROM "memory" WHERE path LIKE '/summaries/%'` SELECT. - `claude-code/tests/output-cap.test.ts` — 2 new cases on the line counting: with a trailing newline the kept-lines + elided-lines sum matches the original line count exactly (no off-by-one), and without a trailing newline the count is still exact. Full suite: 844 / 844 tests passing.
…ed row The jscpd duplication check used to run as a step inside the "Typecheck and Test" job, so the PR checks table only showed a single aggregate row for both. Reviewers couldn't tell at a glance whether duplication passed without opening the combined log. Move jscpd into its own `duplication` job named "Duplication check". Small installation cost (extra `npm install`, runs in parallel with the test job) in exchange for clear attribution on the PR checks table. Artifact upload and the jscpd config stay the same.
PR #63 bot review flagged several source files as under-covered. Added a dedicated branch-coverage suite for the pre-tool-use hook and registered the two now-sufficient files in `vitest.config.ts` so their thresholds are enforced on every run. `claude-code/tests/pre-tool-use-branches.test.ts` — 46 test cases: - Pure helpers: buildAllowDecision, buildReadDecision, rewritePaths, touchesMemory, isSafe (positive + negative paths). - getShellCommand: Grep hit + miss, Read on file + directory, Bash safe + unsafe + non-memory, Glob hit + miss, unknown tool → null. - extractGrepParams: Grep output_mode=count, empty path → "/", Bash delegating to parseBashGrep, non-grep Bash → null, unknown tool → null. - processPreToolUse end-to-end: - returns null for non-memory Bash - returns `[RETRY REQUIRED]` guidance for unsupported commands - falls back to the shell bundle when no config is loaded - Glob + Bash `ls` + Bash `ls -la` long format - ls with both file-level (-rw-) and directory (drwx) entries; also empty-name rows skipped by the `if (!name) continue` guard - cat / head / tail / wc -l / cat | head pipeline - find / find | wc -l - Grep tool delegates to handleGrepDirect; null result falls through to the read/ls branch instead of short-circuiting - direct query throws → shell bundle fallback - Index cache short-circuit: three cases covering the inline readVirtualPathContentsWithCache callback that the bash compiler passes into executeCompiledBashCommand — cache hit, cache miss (writes fresh index), empty cachePaths edge case. Coverage after this suite (measured on pre-tool-use-branches + pre-tool-use-baseline-cloud): src/hooks/pre-tool-use.ts lines 98.9 branches 90.0 funcs 93.8 stmts 98.6 src/hooks/memory-path-utils.ts lines 100 branches 90.9 funcs 100 stmts 100 Both now registered under `coverage.thresholds` at 90 / 90 / 90 / 90 in `vitest.config.ts`, alongside the five existing PR-tracked files. Full suite: 890 / 890 passing (was 844 before this commit).
… paths
CI (HOME=/home/runner) reported two failures on the just-added branch
coverage suite:
AssertionError: expected '/home/emanuele/.deeplake/memory/...' to be
'/sessions/a.json'
The `rewritePaths` and `touchesMemory` assertions hardcoded my local
home path. The real MEMORY_PATH in production is
join(homedir(), ".deeplake", "memory"), so hardcoded absolute paths in
tests don't survive anywhere except my workstation — not CI, not
another developer's machine.
Import `homedir` + `join` from node:os / node:path and build MEM_ABS
once at the top of the file. The two affected cases now use template
strings so the values match whatever home the test runner is using.
The other tests in the suite already use ~-prefixed literals, matched
by the TILDE_PATH branch independently of homedir.
Verified: `env -i HOME=/home/runner PATH=$PATH npx vitest run` —
46 / 46 pass.
|
Claude finished @efenocchi's task in 2m 37s —— View job
SummaryFound 4 issues, all posted as inline comments:
|
Contributor
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 16 files changed
Generated for commit 3ca3ddf. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // Network-level failure (DNS, TCP reset, timeout, etc.) | ||
| if (isTimeoutError(e)) { | ||
| lastError = new Error(`Query timeout after ${QUERY_TIMEOUT_MS}ms`); | ||
| throw lastError; |
There was a problem hiding this comment.
Timeout errors are thrown immediately, bypassing all retries — but timeouts from AbortSignal.timeout() are often transient (brief network congestion, cold-start latency). Skipping retries makes any single 10-second stall a permanent failure for the query.
Fall through to the normal retry path instead of throwing early:
Suggested change
| throw lastError; | |
| lastError = new Error(`Query timeout after ${QUERY_TIMEOUT_MS}ms`); |
(remove the throw lastError; line so the loop continues to retry)
| if (attempt < MAX_RETRIES && RETRYABLE_CODES.has(resp.status)) { | ||
| const retryable403 = | ||
| isSessionInsertQuery(sql) && | ||
| (resp.status === 401 || (resp.status === 403 && (text.length === 0 || isTransientHtml403(text)))); |
There was a problem hiding this comment.
Retrying on HTTP 401 Unauthorized is incorrect: 401 means the bearer token is invalid or expired, which won't resolve itself on retry — it will just exhaust the retry budget and add ~1.4 s of sleep before the permanent failure. The intent here is transient 403s (Cloudflare/nginx gateway pages), not auth failures.
Suggested change
| (resp.status === 401 || (resp.status === 403 && (text.length === 0 || isTransientHtml403(text)))); | |
| (resp.status === 403 && (text.length === 0 || isTransientHtml403(text))); |
| memoryTable: string, | ||
| sessionsTable: string, | ||
| dir: string, | ||
| filenamePattern: string, |
There was a problem hiding this comment.
filenamePattern is interpolated directly into SQL and must be pre-escaped by the caller with sqlLike() — all three current callers do this correctly, but the plain string type gives no indication of the contract, making it easy for a future caller to pass a raw pattern and introduce SQL injection.
Rename the parameter to signal the expectation, e.g.:
Suggested change
| filenamePattern: string, | |
| sqlLikeFilenamePattern: string, |
and update the three call sites from namePattern/filenamePattern to match.
|
|
||
| if (keptLines.length === 0) { | ||
| // A single line is already over budget — take a prefix and mark it. | ||
| const slice = Buffer.from(output, "utf8").slice(0, budget).toString("utf8"); |
There was a problem hiding this comment.
Slicing a UTF-8 Buffer at a byte boundary can split a multi-byte sequence; toString("utf8") then inserts U+FFFD replacement characters, corrupting the output suffix. Use Buffer.from(output).subarray(0, budget) (same effect, non-deprecated) or find the nearest valid character boundary.
Suggested change
| const slice = Buffer.from(output, "utf8").slice(0, budget).toString("utf8"); | |
| const slice = Buffer.from(output).subarray(0, budget).toString("utf8"); |
…use from 0% Two changes driven by PR #64 review bot: 1) `output-cap.ts` — fix the UTF-8-boundary issue flagged on the single-oversized-line path. Naive `Buffer.slice(0, budget)` can split a multi-byte UTF-8 sequence, and the subsequent `toString("utf8")` then leaks U+FFFD replacement characters into the output suffix. Migrate to `Buffer.subarray` (the non-deprecated replacement for `.slice`) and, before decoding, back up to the nearest valid UTF-8 start byte — any byte whose top two bits are `10xxxxxx` is a continuation byte and must not be a boundary. Added two regression cases in `output-cap.test.ts`: - single 20 000-char line of `©` (2 bytes each) — byte budget falls mid-sequence; must produce zero U+FFFD. - multi-line content with multi-byte chars — standard line- boundary truncation; still asserts zero replacement chars. 2) `src/hooks/codex/pre-tool-use.ts` — the Codex pre-tool-use hook sat at 0% coverage. New `codex/tests/codex-pre-tool-use-branches.test.ts` (26 tests) exercises `processCodexPreToolUse` across every routing branch, using the same mock-at-the-network-boundary style as the Claude Code branch coverage suite: - pass-through (non-memory), guide (unsafe command), shell fallback with/without empty result - compiled bash fast-path + the inline `readVirtualPathContentsWithCache` callback (cache hit → SQL only issued for non-cached path) - direct read: cat / head -N / head (default 10) / tail -N / tail (default 10) / wc -l / `cat | head` pipeline - `/index.md` cache hit, cache miss (fresh fetch + cache write), and the inline memory-table fallback when the virtual-path read returns null - ls branch: short + long format with mixed file/dir entries, empty-name rows skipped, empty directory - find / find | wc -l / find no matches → `(no matches)` - grep delegated to handleGrepDirect - direct-query throw → falls back to runVirtualShell Also covers the pure helpers `buildUnsupportedGuidance` and `runVirtualShell` (error path). Coverage moves on PR scope (files changed vs origin/main): lines 87.81% → 95.61% statements 86.29% → 93.15% functions 89.20% → 92.33% branches 79.44% → 86.38% `src/hooks/codex/pre-tool-use.ts` specifically goes 0% → 99.3% lines / 87.3% branches / 81.8% functions / 98.1% statements.
This was referenced Apr 21, 2026
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
2 participants
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.
Summary
Five bug fixes to the pre-tool-use hook and shell bundle that close the 48-point accuracy gap between the local-files LoCoMo baseline and the cloud baseline on a sessions-only Deeplake workspace.
Stacked on top of
optimizations(PR #61). Keeps Davit's grep refactor, adds five independent fixes plus 44 unit/integration tests that fail if any fix regresses.Headline numbers
100-QA LoCoMo run, deterministic first-100 QAs from
locomo10.json, Haiku model, Gemini judge viaopenrouter/google/gemini-2.5-flash. Sessions-only workspacelocomo_benchmark/baseline(272 raw sessions insessions,memorytable dropped).baseline_cloud(original, reference)baseline_cloud_100qa_fix123baseline_cloud_100qa_fix12345baseline-100-subset(local, no plugin)Per-category accuracy (full 100-QA runs)
Signal quality at the tool boundary
[deeplake-sql]lines in tool_result"path must be of type string"errors(no matches)onconv_N_session_*.jsonglobsThe five fixes
#1 —
/index.mdlists session files too (4271baf)Bug: the virtual
/index.mdwas generated from thememorytable only (WHERE path LIKE '/summaries/%'). In workspaces where that table was empty or dropped (e.g.locomo_benchmark/baseline), the index reported0 sessions:or1 sessions:even when thesessionstable had 272 rows. Claude concluded memory was empty and gave up.Fix:
buildVirtualIndexContent(summaryRows, sessionRows)now renders both under## Summaries/## Sessionssections with a combined header (273 entries (1 summaries, 272 sessions):). The fallback path inreadVirtualPathContentsqueries both tables in parallel and passes both sets to the builder.Files:
src/hooks/virtual-table-query.ts.Tests:
claude-code/tests/virtual-table-query.test.tsadds four cases covering the regression, the backwards-compatible single-arg call, and the empty case.claude-code/tests/pre-tool-use-baseline-cloud.test.tsdrives the fullprocessPreToolUseflow against a 272-row fixture and asserts the synthesized index contains every real session path.#2 — Read-tool intercepts return
file_path, notcommand(4c5d50b)Bug: the hook's
updatedInputwas always Bash-shaped ({command, description}). When the incoming tool wasRead, Claude Code's Read implementation looked forupdatedInput.file_path, foundundefined, and crashed with"The 'path' property must be of type string, got undefined". On the pre-fix sessions-only 100-QA run, every memory-path Read call hit this error (9 / 9 tracked cases); on the plugin-v8-optimizations-100 run, 60 / 100 transcripts contained the error.Fix: extended
ClaudePreToolDecisionwith an optionalfile_pathfield. For Read-tool intercepts, the plugin materializes the fetched content viawriteReadCacheFile(sessionId, virtualPath, content)into~/.deeplake/query-cache/<sessionId>/read/<virtualPath>and returns a decision withfile_pathset.main()dispatches onfile_path: if present, emitsupdatedInput: {file_path}; otherwise keeps the historical{command, description}. Bash / Grep / Glob paths are unchanged.Files:
src/hooks/pre-tool-use.ts.Tests:
pre-tool-use-baseline-cloud.test.tsnow asserts Read intercepts produce a decision withfile_path, with content captured through a stubbedwriteReadCacheFileFn.hooks-source.test.tscases updated to match.#3 — Shell bundle silences
[deeplake-sql]trace in one-shot mode (35a7e87)Bug: Claude Code's Bash tool merges a child process's stderr into the
tool_resultstring the model sees. The shell bundle, when invoked asnode shell-bundle -c "…"from the pre-tool-use hook, wrote[deeplake-sql] query start: …lines to stderr wheneverHIVEMIND_TRACE_SQL/HIVEMIND_DEBUGwas set — which in CI / dev shells was frequently the case. The model saw SQL log noise instead ofgrepoutput; on the originalbaseline_cloud-100run, 35+ trace lines leaked across the transcripts.Fix: two parts.
TRACE_SQL/DEBUG_FILE_LOGchecks insrc/deeplake-api.tsout of module-level constants and into thetraceSqlfunction body so callers can flip the env vars at runtime.src/shell/deeplake-shell.ts, when the bundle detects one-shot mode (-cin argv),delete process.env[…]forHIVEMIND_TRACE_SQL,DEEPLAKE_TRACE_SQL,HIVEMIND_DEBUG,DEEPLAKE_DEBUGbefore opening any SQL connection. Interactive REPL mode keeps the env untouched.Files:
src/deeplake-api.ts,src/shell/deeplake-shell.ts.Tests:
claude-code/tests/shell-bundle-sql-trace-silence.test.tsspawns the shipped bundle with the trace vars set, points it at an unreachable API, and asserts stderr is free of[deeplake-sql]. Source-level check confirmstraceSqlreads env at call time, not at module load.#4 —
LIKEclauses that consumesqlLike()output useESCAPE '\'(3d15454)Bug:
sqlLike(value)escapes_and%by prefixing them with\so callers can safely interpolate user-controlled strings intoLIKE 'pattern'literals. But the Deeplake backend does not treat backslash as the LIKE escape character by default — without an explicitESCAPE '\'clause,\_is matched as two literal characters instead of a literal_. Every query whose path or filename contained_(e.g./sessions/conv_0_session_N.json) silently returned zero rows.Observed in the wild:
grep "adoption agency" ~/.deeplake/memory/sessions/conv_0_session_*.jsonreturned(no matches)even though "adoption agency" is in the file — the LIKE pattern/sessions/conv\_0\_session\_%.jsonnever matched any real path.Fix: append
ESCAPE '\'to everyLIKE '...'clause that is fed fromsqlLike(). Covers:src/shell/grep-core.ts:buildPathCondition(wildcard-path and directory-prefix branches).src/hooks/virtual-table-query.ts:buildDirFilter(per-directory filters used bylistVirtualPathRowsForDirs).src/hooks/virtual-table-query.ts:findVirtualPaths(memory- and sessions-table branches, path and filename clauses).The Codex and Claude Code
findfallbacks andbash-command-compiler'sfind_grepsegment call through tofindVirtualPathsand inherit the fix without a local change.Files:
src/shell/grep-core.ts,src/hooks/virtual-table-query.ts.Validation: focused 15-QA subset run (
baseline_cloud_15qa_fix4) on the 15 regressions where local=1 and cloud<1 after fix #1+#2+#3. Pre-fix-4: 1.5 / 15 pts. Post-fix-4: 13.0 / 15 pts, ties the local score exactly. 14 / 15 QAs improved, 1 stayed partial, 0 regressed.#5 — Cap plugin tool output at 8 KB (
2c0d65d)Bug: Claude Code's Bash tool silently persists any
tool_resultlarger than ~16 KB to disk and replaces it with a 2 KB preview plus a path to the persisted file. In thebaseline_cloud_100qa_fix123run, 11 of 14 losing QAs that hit this path never read the persisted file — the 2 KB preview was too small to carry the answer and the model gave up.Typical triggers:
grep -r Caroline /home/.deeplake/memory/(Caroline appears in nearly every session → 66 KB of dialogue),for f in /…/sessions/conv_0_session_*.json; do grep …; done(926 KB of concatenated output through the slow-path shell bundle).Fix:
src/utils/output-cap.tsexportscapOutputForClaude(output, {kind}). If the output fits under 8 KB (CLAUDE_OUTPUT_CAP_BYTES) it is returned unchanged; otherwise it is truncated at the last line boundary that fits under the cap, and a short footer is appended:The footer names the operation (grep / cat / ls / find / bash) and gives the model a concrete next step. The cap is applied on every plugin exit path that can produce a Bash-tool result:
grep-direct.ts:handleGrepDirect(grep output)bash-command-compiler.ts:executeCompiledBashCommand(final concatenation of compiled segments)pre-tool-use.tsdirect read (cat/head/tail),ls, andfindfallbacksRead-tool intercepts are unaffected: they write content to disk and return a
file_path, so no Claude Code preview truncation applies.Files:
src/utils/output-cap.ts,src/hooks/grep-direct.ts,src/hooks/bash-command-compiler.ts,src/hooks/pre-tool-use.ts.Tests:
claude-code/tests/output-cap.test.ts(8 cases) covers the no-op path, line-boundary truncation, single-oversized-line path, custommaxBytes, the default footer kind, and a realistic 400-line grep fixture that exceeds 16 KB and gets capped strictly between 4 KB and 8 KB.Test coverage
44 unit and integration tests across four files, all passing:
claude-code/tests/virtual-table-query.test.ts— 21 tests. Covers fix initial virtual fs implementation #1 at the builder level and thereadVirtualPathContentsfallback (both branches of the "memory empty" / "sessions empty" matrix). Asserts the exact SQL shape per branch.claude-code/tests/pre-tool-use-baseline-cloud.test.ts— 13 tests. Real-QA-anchored integration tests drivingprocessPreToolUseagainst a workspace mock with 272 session rows. Every case mirrors a concrete LoCoMo QA from the benchmark (conv 0 / qa 3, 6, 25, 29, 46). Asserts fix initial virtual fs implementation #1 and fix Feature/enriched capture #2 at the entry point; one dedicated test for Read against a/sessions/<file>.jsonpath (not just/index.md).claude-code/tests/shell-bundle-sql-trace-silence.test.ts— 2 tests. Bundle-level regression guard for fix Feature/integrate hook #3.claude-code/tests/output-cap.test.ts— 8 tests. Byte-accurate truncation assertions for fix Feature/e2e test #5.Each fix was independently verified by stashing the source change and re-running the relevant test file — every source-stash produces a failing test that pinpoints the regression. Verification notes live in the individual commit messages.
Non-determinism caveat (honest)
Haiku and the Gemini judge introduce run-to-run variance that is much larger than the signal we're measuring at 100-QA scale. On the full 100-QA set, the fix 1+2+3+4+5 run scored 68.0 % vs fix 1+2+3's 67.5 % — a 0.5-point net delta. This is not proof that fixes #4 and #5 are net-zero at that scale.
The decisive evidence is in the focused subsets:
grep -r Caroline /home/.deeplake/memory/went from ~66 KB of output truncated to a 2 KB preview (Claude rarely recovered) to a capped 7.9 KB chunk with a footer reporting 313 elided lines.A 14-QA re-run on an identical fix state produced 14.3 % in one run and 53.6 % in another — a 39-point swing from pure Haiku + judge non-determinism. In other words, a single 100-QA run carries ±3–4 points of noise, and the +0.5 pt cross-run delta is well inside that band. The true effect of fixes #4 and #5 on the full 100 is masked by the noise; the per-fix subset tests are the honest measurement.
Plugin workspace cross-check
Same build, same 100-QA set, but against
locomo_benchmark/plugin(272 summaries + 272 sessions) instead of the sessions-only baseline workspace:locomo_benchmark/baseline(sessions only)locomo_benchmark/plugin(sum + ses)Davit's pre-fix
plugin-v8-optimizations-100scored 71.0 % on the canonical 45+55 subset against the same plugin workspace. The fix 1–5 build on the different first-100 subset is statistically indistinguishable.The +2.5 pt from adding summaries is smaller than Davit's observed +11 pt (v8 baseline-workspace → v8 optimizations) because fixes #1–#3 already close most of the sessions-only gap: with the raw-session path working, the summaries are no longer compensating for retrieval-path bugs.
Full analysis lives in
deeplake-cli-locomo-benchmark_plugin/results/ablation_cloud_plugin_fixes.md(benchmark repo).Reproduction
Test plan
npm run buildsucceeds (8 Claude Code + 8 Codex + 1 OpenClaw bundles).npx vitest run claude-code/tests/virtual-table-query.test.ts claude-code/tests/pre-tool-use-baseline-cloud.test.ts claude-code/tests/shell-bundle-sql-trace-silence.test.ts claude-code/tests/output-cap.test.ts— 44 / 44 pass.locomo_benchmark/baselinereaches 68.0 % (vs 27.0 % pre-fix, vs 75.0 % local).fix415-QA regression subset run on a fresh checkout.content_text <#>for pattern-variation robustness;ORDER BYinsidegrep-core.ts:searchDeeplakeTablessubqueries to remove the remaining plugin-side non-determinism fromLIMIT 100.