feat(memory): opt-in persistent personal memory (--memory) - #1190
Draft
mikenorgate wants to merge 53 commits into
Draft
mikenorgate wants to merge 53 commits into
mikenorgate wants to merge 53 commits into
Conversation
Kimchi Code Review
Summary📊 Review Score: 84/100 (overall code quality — 0 lowest, 100 highest) 🧪 Tests: yes — Good test coverage for the visible changes: 📝 Found 7 issue(s). See inline comments for details. What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
![kimchi-review[bot]](https://avatars.githubusercontent.com/in/3223788?s=60&v=4){kind=small}
There was a problem hiding this comment.
📊 Review Score: 84/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 4/5 (1 = trivial, 5 = very complex)
🧪 Tests: yes — Good test coverage for the visible changes: stripMemoryArgs and boolean =-form normalization in src/cli-args.test.ts, the CLI shell's output routing / exit codes / non-TTY reset decline in src/commands/memory.test.ts, and the admin core in src/extensions/memory/admin.test.ts with dependency-injected fake backends. package.json adds memory:check (Bun-runtime acceptance against the real gateway) and memory:measure. The terminal-bench memory kwarg / build_cli_flags override in agent.py has no accompanying test.
📝 Found 7 issue(s). See inline comments for details.
Storage foundation for the memory extension (Phase 2 POC chunk 1): - shims/better-sqlite3: better-sqlite3 API surface over bun:sqlite, lazy driver resolution so the module stays loadable under Node (vitest). Bound via root dependency + pnpm override; mem0ai peer-resolves it. Upstream tracking: oven-sh/bun#36712. mem0 usage verified by grep: exec/prepare(run,get,all)/transaction/close only. - src/extensions/memory/backend.ts: Mem0 Memory construction with the SQLite MemoryVectorStore (hybrid BM25 + entity + semantic retrieval; the langchain adapter is semantic-only) and remote embeddings via the kimchi gateway (text-embedding-3-small, 1536 dims; extraction kimi-k3). - disableMem0Telemetry(): mem0 OSS phones home to PostHog by default — disabled before first module load (MEM0_TELEMETRY env, read at module scope). Explicit operator opt-in wins. - backend.check.ts (pnpm run memory:check): Bun acceptance check — store-level hybrid surface (port of the spike sqlite-shim-test) + full round-trip with real remote embeddings. ALL CHECKS PASSED. - backend.test.ts: 10 vitest tests — config wiring, endpoint overrides, scope-id path validation, Node runtime guard, telemetry opt-out. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Phase 2 POC chunk 2 — the extension itself: - index.ts: opt-in via --memory; digest retrieval grounded in the session opening prompt; value gate (threshold + top-N + token caps); empty digest is the normal outcome — nothing is injected when nothing clears the bar. The digest rides on before_agent_start as a byte-stable system-prompt suffix (stable prefix, zero mid-session cache breaks); recomputed only after compaction. Failures degrade to no-memory with a logged error, never breaking the session. - inject.ts: pure digest builder with composition logging (facts, considered, belowThreshold, overCap, overBudget, tokensEstimated) — the input for the chunk-4 value measurement. - tools.ts: memory_search pull tool (deps-injected, dap.ts pattern). - config.ts: scope, threshold 0.3, top-5, 2k-token cap. - cli-args.ts: --memory flag in CLI_OPTIONS + SessionCliArgs; cli.ts: extension registered. 20 vitest tests; typecheck clean. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Phase 2 POC chunk 3 — turning sessions into stored memories without blocking the harness: - capture.ts: captures at session_before_compact (the compacted-away span, where knowledge gets summarized away) and session_shutdown. Job files are written atomically under the scope dir; the worker is spawned detached + unref'd, fire-and-forget. Two spawn modes: under `bun run` the worker script runs directly; in compiled binaries process.execPath is the kimchi binary, so the worker routes as the `memory-capture` subcommand (cli.ts, before telemetry/session setup). - capture-worker.ts: dedupe by message hash (captured-hashes.json, crash-resumable, idempotent re-runs) -> <=4-message windows (the spike's gateway 524 mitigation) -> extraction LLM tuned to durable user facts -> conservative force-DELETE+ADD supersede -> add(infer: false). Gateway client retries 429/5xx/network honoring retry-after; other 4xx fails fast. - supersede.ts: findSupersededIds — one batched conservative judge call per window; only explicit changed-value evidence deletes. 35 vitest tests; typecheck clean. Co-Authored-By: Kimchi <noreply@kimchi.dev>
The digest previously appeared on turn 2 (async compute after the first before_agent_start), which is a mid-session prompt change — a cache break attributable to memory. before_agent_start handlers can be async, so turn 1 now awaits the digest: the provider-facing prefix carries it from the very first request and stays byte-stable across all turns. Three-state model (uncomputed / empty / set): an empty digest injects nothing for the whole session, with exactly one search per session; compaction resets to recomputation. createMemoryExtension(deps) factory (dap.ts pattern) makes the searcher and flag gate injectable; extension.test.ts drives the handlers through the shared __mocks__/extension-api fixture and pins the cache contract: byte-identical prompt across turns, single search, post-compaction recompute, degrade-to-no-memory on failure. 42 tests across the memory extension; typecheck clean. Co-Authored-By: Kimchi <noreply@kimchi.dev>
memory:measure seeds a real store (remote embeddings) and runs related and unrelated queries through the same value gate the extension uses, reporting injection rate, no-injection rate, and the top-score ranges. First calibration run at threshold 0.3: 100% no-injection on unrelated queries (top 0.142-0.231), 50% injection on related (0.193-0.376) — the ranges overlap, so the threshold errs toward precision (the user's explicit requirement that auto-injection earn its tokens). Digest misses are covered by the memory_search pull tool. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Chunk 4 verification findings and fixes, all surfaced by the dogfood
run (two --print sessions, capture worker in between):
- cli-args.ts stripMemoryArgs + cli.ts wiring: pi-mono's parser treats
unknown --flags greedily and consumed the next argument, so
--memory "prompt" ate the prompt and print mode exited silently.
Kimchi parses --memory itself; pi must never see it (the
stripMultiModelArgs pattern). Also pi.registerFlag("memory") in the
extension so applyExtensionFlagValues accepts the flag.
- capture-worker.ts: new URL("/chat/completions", baseURL) dropped the
gateway base path (/openai/v1) — relative join instead. The worker
now captures: session A -> 2 facts stored, hashes marked, job
cleaned up.
- __mocks__/extension-api.ts: registerFlag added to the shared mock.
- terminal-bench-2 kimchi_agent: memory kwarg (default off) appends
--memory for the memory-on arm of A/B runs. No Python test
scaffolding exists for the agent kwargs (same as disable-compaction).
Dogfood acceptance: preference stated in session A recalled in fresh
session B. The digest correctly stayed empty for this phrasing (2 hits
below the 0.3 bar — as memory:measure predicted) and the memory_search
pull tool covered the recall — the value gate errs precision-first.
42 tests pass; typecheck clean.
Co-Authored-By: Kimchi <noreply@kimchi.dev>
mem0ai bundles lazy per-provider imports (ollama, groq, qdrant, etc.) that are not installed; the bundler fails to resolve them. The memory extension uses only the openai embedder/LLM and the built-in SQLite store, so none are ever loaded at runtime — mark them external. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Format/organize-imports drift (15 files, auto-fixed), non-null assertions in inject.test.ts replaced with narrowing throws, and the pre-existing noDelete in mcp-adapter's test annotated per the config.ts convention. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Benchmark artifact builds run on memory-constrained CI pods where tsc OOMs before the compile step. The benchmark/memory branch already carries this guard; mirror it here so the tb21-pattern worktree build (fetched by ref from the benchmark pipeline) can skip typecheck — the ref's own CI still typechecks every src change. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Chunk 1+2 of the accuracy plan (benchmark investigation findings): - chatWithRetry: an unparseable (prose) extraction or supersede-judge response is retried once with a CRITICAL FORMAT REMINDER appended to the system prompt before failing. Investigation root cause: kimi-k3 answered a session question with prose and one bad response killed the whole capture job (2311e44b: zero facts). - parseIdArray: the supersede judge parse gets the same defensive slice-parse + retry treatment as extraction. - EXTRACTION_SYSTEM_PROMPT: require itemized values as their own facts (counts, prices, assignments, dates, measurements) and explicit change statements on updated values — feeds supersede and fixes the stale-count and lost-assignment failure modes (69fee5aa, 7161e7e2). 45 memory tests pass; typecheck clean. Co-Authored-By: Kimchi <noreply@kimchi.dev>
These files were auto-fixed by lint:fix but missed the earlier style commit staging (9fc437a3) — format-only changes, no behavior. Co-Authored-By: Kimchi <noreply@kimchi.dev>
…floor) Chunk 3 of the accuracy plan. The investigation confirmed a needle retrieved at 0.197 was dropped by the 0.3 bar (9a707b82: chocolate-cake fact captured, retrieved, gated out). Calibration puts unrelated-query top scores at 0.142-0.231, so 0.2 is the tightest cut that admits the confirmed miss; the noise trade is re-validated via memory:measure before the benchmark re-run (retreat to 0.22-0.25 if noise breaches). inject.test.ts fixture updated deliberately (weak fact 0.29 -> 0.19: the old value is above the new bar). config.ts also carries the inert TURN_RECALL_* constants landed early for the multi-turn chunk. Co-Authored-By: Kimchi <noreply@kimchi.dev>
…meout
Chunk 4 of the accuracy plan (user: address the DB lock that blocks
concurrent sessions):
- backend.ts: historyStore { provider: sqlite, historyDbPath } pinned
to the same .kimchi memory directory as the vector store — mem0's
default was a cwd-relative memory.db (the stray repo-root file; the
lock contention the local investigation hit).
- shims/better-sqlite3: WAL journal mode (readers proceed during
writes) + busy_timeout=5000 (writers wait instead of failing fast) —
the same busy_timeout the repo's cursor.ts integration sets; skipped
for readonly connections which cannot change journal mode.
- backend.check.ts PART C: 8 parallel writers + 8 concurrent readers on
one store — zero lock errors; history db lands next to the store; no
files created relative to cwd. ALL CHECKS PASSED.
Co-Authored-By: Kimchi <noreply@kimchi.dev>
Chunk 5 of the accuracy plan (user design: identify additional information to load based on user prompts and model responses, keep the digest lean): - Turns 2+: the drift signal is the new user prompt PLUS the last assistant response (the model may drive the conversation). A free lexical-coverage gate (condition A) decides when a retrieval is worth an embedding call; a per-session cap (5) bounds the cost. Condition B (BM25 vs store) deferred — the mem0 facade does not expose keywordSearch; noted in the plan decision log. - Only NEW facts deliver (delivery ledger deduped by fact hash, seeded from the initial digest) as hidden steer messages (pi.sendMessage display:false, deliverAs:steer) — conversation-tail appends, so the system-prompt prefix stays byte-stable: zero mid-session cache breaks. - Compaction resets the ledger and budget: earlier steers may have been compacted away, so re-delivery is allowed. - Searcher failures degrade to no-recall, logged once (a throwing factory now marks the session failed instead of retrying per turn). - Gate/recall decisions logged (skip: covered / delivered / nothing-new) for the memory:measure skip-retrieve ratio. 50 memory tests pass; typecheck clean. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Narrowing throws replace the last non-null assertions; formatter and import-order fixes from the Chunk 5 files. Co-Authored-By: Kimchi <noreply@kimchi.dev>
…calls Validation finding: the same haystack produced different captured facts across runs (the needle count fact and the chocolate-cake fact appeared and disappeared between captures) — chatJson never set a temperature, so the gateway default applied. The spike judge always used temperature 0. At temperature 0 the remaining needle misses are deterministic, not variance: one-pass 4-message windowed extraction over ~50-session haystacks consistently drops 1-in-50-session needles. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Implements the approved windowing design (see .kimchi/plans/capture-windowing-design.md): - User-only capture jobs: assistant content is excluded by the extraction prompt anyway, costs ~half the extraction tokens, and dilutes needles (the proven failure mode). Big real session: 544k -> 131k chars. - windowByBudget packs consecutive user messages up to ~2k chars per extraction window (proven-safe size; typically 1-2 messages per window). Oversized single messages extract whole. Chronological order preserved — supersede correctness depends on newer facts arriving after older ones (reverse order was rejected during design). - Extraction prompt: treat agent-prompt content as text to analyze, not messages addressed to the model (the validation run caught kimi-k3 conversing with the content instead of extracting). - Window extraction failures skip-and-continue instead of aborting the whole job; unmarked hashes retry on the next capture spawn. Validation (the big real coding session, 122 user messages): 103 durable facts (~17x fewer windows than the flat W=4 path, ~8.3 min vs ~2.4h estimate), memory:check ALL PASS, 55 tests green. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Models deprecate and per-user gateway access varies, so the extraction model is now resolved at capture-worker start: KIMCHI_MEMORY_EXTRACTION_MODEL env override first; otherwise the first available preference from [glm-5.3-flash, deepseek-v4-flash-0731, glm-5.3, kimi-k3] against the gateway's live /models list (model-roles-style graceful fallback). An unreachable model list falls back to the top preference; a reachable list with no preference errors clearly. Measured: quality holds at flash tier (the weight-loss needle extracted verbatim with date context); latency is unchanged vs kimi-k3 (~10s per extraction call) — the tier is not the bottleneck, the call count is. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Co-Authored-By: Kimchi <noreply@kimchi.dev>
… capture Three levers against capture latency (call-count-bound, not model-bound — the flash experiment proved the tier was a red herring): 1+2. Chunked pipeline: extraction windows run in parallel (concurrency 4, bounded by mapWithConcurrency) within chunks of 8, then ONE supersede judge pass per chunk instead of per window. Adds land before the judge; the judge prompt now states the new facts are in chronological order and that identical facts are not replacements — within-chunk supersede keeps the right direction and cannot self-delete. 3. Incremental capture: before_agent_start drains uncaptured user messages once at least 10 accumulate (runtime mark makes batches non-overlapping; the worker hash ledger dedupes restarts; compaction re-derives the mark). The shutdown tail shrinks to the last few turns, shrinking the next-session staleness race with it. Measured on the benchmark-size job (5 windows): 1m41s -> 1m06s (~1.5x — parallelism works; per-call gateway latency caps the gain at this size). Quality identical: 28 facts, the weight-loss needle verbatim. 67 tests green. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Implements the assistant-message memory plan (see .kimchi/plans/assistant-message-memory.md): - Structural gate: assistant turns enter capture jobs only when pure text — no toolCall blocks (work product, excluded whole), no thinking blocks (messageText now filters to type=text), ≤1000 chars. The cheap pre-filter against the ~93% work-product share of real sessions. - Two extraction passes per window: user-stated facts (the validated prompt, unchanged — zero regression risk) plus a cautious ASSISTANT_FACTS pass (agent-aware: assistant messages are AI output — requirements: engagement evidence — the user asked, or accepted/acted on; attribution; skip on doubt). Pure-user windows skip the second call entirely, so user-only sessions pay nothing extra. Co-Authored-By: Kimchi <noreply@kimchi.dev>
The gate excluded oversized turns entirely — but an answer's key statement (the count, the recommendation) sits at its start, so the egg-count needle never reached extraction. Truncate at the bound instead: retains the key statements, bounds the volume. Also: the user-facts extraction prompt gains the quoted-advice rule — when the user references what the assistant told them (quotes, "things you told me"), capture those as conversation-established facts. Validated: 5 facts extracted from the framed message vs 0 before. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Implements the approved two-scope design (see .kimchi/plans/two-scope-memory.md), with the 95-fact audit as the validated classification baseline: - scope.ts: resolveProjectScope — git remote → owner/name (GitLab subgroup paths kept whole), repo-dir fallback, null outside repos; sanitized segments, spawnSync arg-array - Tagged extraction: the audit-validated scope-tag section in BOTH passes, appended ONLY when a project scope exists (suppressed otherwise — the home-dir audit finding); parseTaggedFacts tolerates bare arrays (all personal); strict-retry suffix now shape-neutral - Two-store routing: personal always + project from the job scope; tagged facts add to their store; supersede per store (a project fact never supersedes a personal one); tag counts logged for the promotion follow-on - Shared ledger + pending dir at the memory root (one-time rename migration from personal/) — the benchmark drain path updated to match - scoped-searcher.ts: personal + project search merged by score (createScopedSearcher from the session cwd captured at first agent start; project-store failure degrades to personal-only); memory_search labels provenance ([personal]/[project]); mergeScopedResults pure-tested Co-Authored-By: Kimchi <noreply@kimchi.dev>
The two-store dogfood caught two compliance failures: (1) the base
prompts array-format respond line conflicted with the tag sections
object instruction — the model returned a bare array with tags embedded
in the fact strings (16 mis-routed facts, 0 to the project store);
scopedPrompt now strips the base line so the object instruction is the
only format directive; (2) the model also returns fact-objects
([{"fact": "...", "scope": "project"}]) and markdown lists
("- [project] fact") — parseTaggedFacts now handles all three shapes,
routing by the scope field, the string prefix, or the list prefix.
tryParseObject guards on the tagged fields so single fact-objects
fall through to the array path.
Dogfood re-run: 7 facts captured from a project-scoped job — all
correctly routed to the project store, prefixes stripped, zero
pollution of the personal store. 95 tests green.
Co-Authored-By: Kimchi <noreply@kimchi.dev>
P1 (injection framing): the digest and turn-recall steers are wrapped in the harness <system-reminder> convention with an explicit data-never- instructions clause; memory_search output carries the same framing; the assistant-pass extraction prompt gains the treat-as-text clause; the --memory wording discloses that extraction/embedding run via the kimchi gateway (storage stays local). P2 (capture race): capture workers serialize — each spawn acquires the root capture lock (proper-lockfile, mtime-refreshed, stealable after 15 min) and drains ALL pending jobs oldest-first. The concurrent-worker ledger race is structurally impossible, orphaned job files get retried by the next spawn, and jobs older than 7 days are swept (the reaper). Belt-and-braces: the hash ledger merges on save, adds are guarded by an exact-duplicate check (mem0 getAll, local read), and the digest dedupes identical fact texts. P3s: 10s bounded wait on the turn-1 digest and recall searches (a hung gateway degrades to no-memory); spawn 'error' listener on the detached worker; ACP branch parses the stripped args list; --flag=true coerced to real booleans at the parseCliArgs layer (repo-wide); parseTaggedFacts routes unprefixed entries per the unsure->project rule when a project scope exists; tests use the shared createContext() mock; dead parseFactsResponse removed; personal-store path unified (digestDbPath); shared runCaptureWorkerMain entrypoint for cli.ts + import.meta.main; chatJson consolidated onto fetchWithRetry (Cloudflare 520-522 added to the shared retryable set); KIMCHI_SKIP_TYPECHECK warns when skipping; shims/ added to the lint scope; logOnce renamed to logDegrade. Docs: durable design rationale under /docs/memory-extension.md; the citing comments in config/backend/supersede/backend.check repointed. Verification: lint + typecheck + 10,107 unit tests; memory:check (Bun acceptance, remote embeddings, 8-writer concurrency) ALL PASSED; lock serialization spot-checked (a second worker waits out a held lock and drains cleanly). Co-Authored-By: Kimchi <noreply@kimchi.dev>
Live-session finding: told "my dog's name is fred", the model replied it has no memory-write tool and cannot store it — it knew about memory_search but nothing said capture happens on its own. A constant `## Memory` section now goes out on every start whenever memory is enabled, even when the digest is empty: facts established in conversation are captured automatically at session end, "remember this" needs no action, memory_search is the retrieval path. Constant bytes, so the stable-prefix cache contract is unaffected; an empty digest now injects the notice instead of nothing. The memory_search description gains the same read-only note. Co-Authored-By: Kimchi <noreply@kimchi.dev>
…set) `kimchi memory` (CLI subcommand, always available) and `/memory` (in-session, when --memory is on) share one admin core (src/extensions/memory/admin.ts): overview (storage path, per-store stats, pending jobs), list (newest 50 by default across all stores, scope-labeled, paginated, --json), search (ranked, ids shown), delete (by id, resolved across all stores — no --scope needed), and reset (scoped deleteAll for personal/project; --scope all wipes the memory root under the capture lock, keeping only the lock artifacts). Deletion is user-only: the model keeps read-only memory_search, and the always-on notice now points users at /memory when they ask to forget or review a stored fact. Resets confirm interactively (the update command readline confirm, extracted to commands/_helpers.ts; the native ui.confirm dialog in-session) or take --yes. Supporting changes: the drain lock moved to lock.ts (acquireCaptureLock) and is shared by the worker and reset; the shared extension-api mock gains getRegisteredCommand and the context mock gains createCommandContext; "memory" joins the telemetry subcommand allowlist. Verified: lint + typecheck + 10,133 unit tests (26 new for the admin core and command); live CLI overview/list against real stores; live reset/delete smoke against an isolated memory root. Co-Authored-By: Kimchi <noreply@kimchi.dev>
The in-session /memory command displayed lists in ctx.ui.editor — an editable buffer, which implied editing the list had an effect (it didn't; edits were discarded). Output now renders as a read-only widget above the input (setWidget, the same surface as the agents list): it stays visible while you type the next command — the /memory delete <id> workflow — and clears when an agent turn resumes. Very long output is capped at 30 lines with a paging hint; single-line results stay notifications. Co-Authored-By: Kimchi <noreply@kimchi.dev>
…ection /memory list and /memory search now open an interactive panel (McpPanel-style component via ctx.ui.custom) instead of dumping text: page through facts with arrows/j/k (PgUp/PgDn, g/G), delete the selected fact with `d`, quit with q/Esc/Ctrl+C. Deleting by selection removes the list → copy id → delete round-trip; the CLI grammar is unchanged. Deletion in the panel is optimistic (the row disappears immediately) and restores with an error notice if the store rejects it. The admin core exports the operations the panel needs (adminListFacts, adminSearchFacts, adminDeleteFacts); opList/opSearch/opDelete now delegate to them — one source of truth. The window math adapts to the terminal height and keeps the cursor centered (computeMemoryWindow). Non-panel output (overview, errors) stays the read-only widget, now capped at the TUI's actual 10-line widget limit so our hint line replaces its "... (widget truncated)". Single-line results remain notifications. Verified: lint + typecheck + 10,145 unit tests (12 new — window math, cursor movement incl. shift+g/G, page keys, optimistic delete + restore, quit keys, empty state, mount wiring through ui.custom). Co-Authored-By: Kimchi <noreply@kimchi.dev>
…reset Three terminal-level scenarios over the shared fixture (isolated HOME, fake OpenAI server, --memory sessions) with a deterministic seeded store: - memory-panel-browse-delete: /memory list opens the interactive browser over the seeded facts, the cursor moves with the arrow keys, `d` deletes the selected fact (the notice names what was deleted), quitting returns to the prompt, and re-opening shows the deletion persisted. - memory-overview-widget: the bare /memory command renders store stats as the read-only widget, and the widget clears once an agent turn resumes. - memory-delete-reset: /memory delete removes a fact by id, the overview reflects the remaining count, and /memory reset confirms through the native dialog (Enter = Yes) leaving the store at zero facts. Seeding (support/memory-seed.ts, spawned under bun): facts are written straight into the MemoryVectorStore the Memory class reads — verified that admin list, the panel, and deletes all operate on them — with hand-made unit vectors, so the tests need no network and no capture pipeline. support/memory-e2e.ts resolves the script via KIMCHI_REPO_ROOT (the runner compiles tests into .tui-test/cache, breaking import.meta.url paths). All three scenarios pass (4.3s / 2.8s / 3.3s). Co-Authored-By: Kimchi <noreply@kimchi.dev>
…iene
Applies all 12 findings from the pre-PR verification adjudication:
P1 — capture pipeline orchestration now has automated coverage: added
injectable seams to the worker (CaptureBackend interface, createBackend
factory, and llm options on RunCaptureWorkerOptions; default factories
preserve production behavior exactly) and 10 tests in capture.test.ts:
end-to-end drain (extract → add → hash-mark → job removal), ledger
idempotence (re-spawned duplicate job is a no-op), crash-resume (failed
add leaves the job and hashes unmarked; the retry drain reprocesses),
poison-job removal without blocking the drain, the 7-day stale sweep,
oldest-first drain order, the exact-duplicate guard, and
wireMemoryCapture's session_shutdown deterministic job-file writing +
worker spawn.
P2 — .gitignore now covers benchmark/memory-spike/{data,dist}/ and
Library/ (the 265MB dataset and build output are documented external
prerequisites; the stray Library/ cache is deleted); stripMemoryArgs
(the guard against --memory eating the prompt) gets unit tests.
P3s — normalizeMem0SearchResults extracted in backend.ts and used at
all five mem0 search-unwrap sites; the E2E seed script imports
MEMORY_EMBEDDING_DIMS instead of hardcoding 1536; the panel's
failed-delete restore uses the original index (no reordering) with an
order-preservation test; the scoped extraction prompt's
respond-line-swap is asserted; buildTurnRecall's cap/truncation/
ledger-filter branches get unit tests; 520/521/522 added to the retry
table; the `kimchi memory` CLI shell gets tests (json/text routing,
exit codes, non-TTY confirm decline); the /memory command's --json and
non-UI branches get tests; the useTemplate lint warning is fixed.
A PR description draft recording the mem0ai + shim dependency decision
(removal criteria, override blast radius) lives in
.kimchi/docs/pr-description-memory-extension.md (git-ignored).
vitest.config.ts now excludes tests/e2e/**: the tui-test E2E files
collide with vitest's fork-pool IPC when loaded in plain vitest (the
deterministic "Unexpected call to process.send()" crash — verified: all
14 unit suites pass under the same invocation; the three E2E scenarios
pass under their dedicated tui-test runner, one file per process). The
exclude is a no-op for every existing script (`pnpm run test` uses
--dir src; E2E runs via run-tui-e2e.js) and prevents a bare root
`vitest run` from tripping over them. Verified via probe that vitest
respects the exclude for explicitly-passed file paths.
Co-Authored-By: Kimchi <noreply@kimchi.dev>
The capture worker's extraction model now resolves through the auto router (/v1/route — the same service the kimchi-dev/auto model uses interactively) instead of a hardcoded preference list as the primary mechanism. One routing call per worker run, using a fixed transcript-free query that mirrors the extraction system prompt's task definition (no user data, deterministic). Resolution order: KIMCHI_MEMORY_EXTRACTION_MODEL override → the router's best_model validated against the gateway's live model list (an unreachable list cannot validate, so the recommendation is used unvalidated — a router-endorsed model is at least as likely-right as the top hardcoded preference) → the existing flash-tier preference order (router failure or off-list recommendation) → the top preference when the model list is unreachable → throw only when the list is reachable and nothing matches. The router and models-list calls run in parallel; routeQuery's built-in 5s timeout and total-failure semantics mean the router can never fail a drain. DEFAULT_ROUTER_ENDPOINT is now exported from router-config.ts (single source of truth; KIMCHI_ROUTER_ENDPOINT honored the same as interactive routing). Verified: lint + typecheck green; 154 memory tests (3 new router-path tests: primary recommendation used, off-list falls back to preferences, unvalidated use when the list is down); full suite 10,619 passing. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Benchmark replay runs anchor the assistant's clock to a dataset timeline so relative-time questions resolve against baked-in history rather than the real wall clock. Inert when unset; real-time consumers (TLS, session timestamps) are unaffected — only the system prompt's Current date line changes. Co-Authored-By: Kimchi <noreply@kimchi.dev>
The full benchmark's dominant failure cause was capture timing: serialized jobs (~24s each, ~18 min for a benchmark haystack) outran the drain budget, so answers living in the latest sessions failed at 42% vs 16% for the earliest (2.6x answer-position gradient tracking the capture backlog). Restructures the drain into three phases under the SAME single lock (the P2 ledger-race fix is unchanged — one lock-holder, no cross-process concurrency): A. Claim pass (sequential, no LLM): each job's messages filter against the shared ledger plus this drain's in-memory claims — overlapping jobs (before_compact + its shutdown superset, incremental batches) dedupe; the first job to claim a message owns it for this drain. B. Parallel extraction: every window from every job in ONE bounded-concurrency queue (MEMORY_DRAIN_CONCURRENCY=6, env-overridable KIMCHI_MEMORY_DRAIN_CONCURRENCY) — the LLM phase is the throughput bottleneck. A failed window retries once; a second failure leaves hashes unmarked for a future drain (the pre-pipeline drain relied on a later superset job re-carrying the messages; the claim pass now dedupes those, so the retry lives here instead). C. Sequential commit in job order (chronological — supersede correctness depends on newer facts arriving after older ones) with backends created once per drain, then ONE batched supersede judge pass per store across the whole drain (MEMORY_SUPERSEDE_BATCH_FACTS=40 caps the judge prompt) — a single chronological view makes cross-session conflicts explicit; per-job judges never saw two sessions' facts side by side. Judge failures are fail-soft (adds already committed, supersede is a quality pass, not a correctness invariant). Removes the obsolete MEMORY_CAPTURE_CHUNK_WINDOWS/MEMORY_CAPTURE_CONCURRENCY constants; the docs tuning table is updated. All 48 existing orchestration tests pass unchanged; 4 new tests cover drain-wide extraction concurrency, chronological commit order under out-of-order extraction, the single batched judge with chronological fact listing, and claim-pass dedup of overlapping jobs. Co-Authored-By: Kimchi <noreply@kimchi.dev>
…nd recording dates
The benchmark's temporal-reasoning killer: extraction captured facts as
timeless statements ("planning a trip to Seattle") while the questions
need when — 33 abstentions of "I don't have any DATED records" even
though the replay framing carried the dates ("Quick update from
2023/05/26").
Two signals, because the date lives in different places for the
benchmark and real sessions:
- Recording dates (real sessions): CaptureMessage gains an optional
date (YYYY-MM-DD from the session entry timestamp, local time via
the en-CA locale — same format as the system prompt's Current-date
line), and the extraction transcript renders a [YYYY-MM-DD] prefix
per line. Deliberately excluded from messageHash: the same message
re-passed with a different recording date keeps its identity.
- Conversation-text dates (the benchmark's main fix): both extraction
prompts instruct date-stamping when temporally meaningful as a
strict prefix — "As of 2023-05-26, planning a trip to Seattle" —
normalized from any form the conversation uses (dated updates,
"today", "last week", "in June"). Priority rule: a date the
conversation explicitly states wins over the recording-date line
prefix (the benchmark's replay framing dates beat the wall-clock
entry timestamps); timeless facts (long-held preferences) stay
undated.
The KIMCHI_CONTEXT_DATE context-clock override (f2aab89) is not used
by the capture path — the harness anchors only the question session;
the replay sessions' dates live in the conversation text, which the
prompt mining now uses.
Verified: lint + typecheck green; 161 memory tests (3 new: entry
timestamp → date enrichment with absent/unparseable fallbacks, the
transcript date prefix with undated lines rendering bare, prompt
assertions for the As-of instruction on both passes).
Co-Authored-By: Kimchi <noreply@kimchi.dev>
…ogs, wider search
The benchmark's knowledge-update killer (29 failures): both old and new
values surface with no recency signal ("two conflicting entries: 25 new
postcards… 17 new postcards"). The judge prompt documents the supersede
contract but the store kept both and retrieval ranked them equally.
Three changes:
- Judge prompt sharpening: a new VALUE for the same subject is a
change — counts, statuses, locations, and preferences that differ
between an old memory and a new fact mean the value changed; delete
the old memory, do not keep both as complementary details ("25 new
postcards, up from 17" replaces "17 new postcards"). With the As-of
date stamps landed, the prompt adds the recency tie-break: when both
state values for the same subject, the fact with the LATER "As of"
date, or the explicit "up from / replaced / now" phrasing, is the
newer state.
- Judge outcome logs: every judge pass logs "N new fact(s),
M candidate(s), K deleted" — the diagnostic that separates the
failure modes on a benchmark rerun (no judge line = the search
found no candidates; a line with 0 deleted = the judge declined).
- Candidate search widened from topK 5 to 8 (SUPERSEDE_SEARCH_TOPK) —
the old value must surface as a candidate before the judge can
delete it.
SUPERSEDE_SYSTEM_PROMPT is now exported for prompt assertions. The
docs tuning table carries the new constant.
Verified: lint + typecheck green; 57 capture tests (2 new: the judge
prompt's value-change language, and the cross-session conflict case —
session 1 "17 postcards", session 2 "25, up from 17", one drain, old
fact deleted, new fact kept, all candidate searches at topK 8).
Co-Authored-By: Kimchi <noreply@kimchi.dev>
The benchmark's minor failure mode (22/167): facts retrieved (token overlap up to 1.00) but the model hedged anyway. One clause in the framed ## User memory preamble: "They are the user's own recorded memories: when they answer the question, rely on them directly." Kept to one line — the digest already pays framing tokens. Verified: lint + typecheck green; full unit suite 10,628 passing; the three TUI E2E memory scenarios green against the rebuilt binary (4.1s / 2.9s / 3.0s) — the needle sanity holds end to end with the pipelined worker compiled in. Co-Authored-By: Kimchi <noreply@kimchi.dev>
…over-proxy
Follow-up finding on the date-stamp work: a recording date is a proxy
for when a statement was true, valid only while recording ≈ conversation
time. The previous fallback rule ("use the [date] line prefix when the
conversation states no date") turned that proxy into fake explicit
evidence — and the judge's later-As-of-wins tie-break then let a
recording-stamped fact beat a conversation-dated one. The inversion is
real in two worlds: benchmark replays (recording 2026, conversation
2023) and production imported chat history (recording = import time,
conversation years earlier). Faking the process clock breaks TLS
(libfaketime) and anchoring replay sessions perturbs the scores, so the
fix is extension-side, two-part:
- Extraction prompts (both passes): stamp ONLY dates the conversation
explicitly states or implies — relative expressions anchor against
the [YYYY-MM-DD] line prefix, explicitly stated dates always
preferred — and when the conversation gives no date signal, the fact
stays undated; never write the recording date into the fact text.
Undated facts still order internally via the drain's chronological
commit order (the judge's listing), which is exactly the
recording-valid-in-production case ("I have 30 eggs" Monday vs
"I have 20 eggs" Thursday).
- Supersede judge: explicit-over-proxy precedence — a conversation-
dated fact is never superseded by an undated fact or by recording
time alone, unless the other fact explicitly says it supersedes
("up from", "replaced", "now I have"). Between two conversation-
dated facts, the later As-of date still wins.
Also removes the bloat the stamped prefixes added to fact text (the
paper-date retrieval flip). The [YYYY-MM-DD] transcript line prefix
stays — it anchors relative expressions and gives the extraction model
context; it just never becomes fact content on its own.
Verified: lint + typecheck green; 163 memory tests passing (prompt
assertions extended for the no-fallback rule and the precedence
clause).
Co-Authored-By: Kimchi <noreply@kimchi.dev>
…xes removed
The 66.7% rerun finding: the previous guard ("never write the recording
date into the fact") contradicted itself — the same prompt still said
"anchoring relative expressions against the [YYYY-MM-DD] line prefix",
and that prefix IS the recording date (session entry timestamps = the
2026 wall clock). So "we've currently got 30 dozen eggs" dutifully
anchored "currently" to 2026-09-12 and wrote "As of 2026-09-12, I have
30 dozen eggs (360 eggs)" — nine such facts in the egg trial's store.
Once a recording-derived date is in fact text, the supersede guard
can't catch it: the judge sees it as strong explicit evidence and
correctly prefers it over the true conversation-dated "As of
2023-01-11". The guard guarded a signal that had already been
laundered.
The fix closes the channel at the input:
- renderWindow is deliberately date-prefix-free: the extraction
transcript is plain "role: content" — recording dates never reach
the LLM, so no instruction can launder them into fact text. The
conversation's own dates travel in the message text (replay
framing "Quick update from 2023/01/11"; real users state dates the
same way).
- CaptureMessage drops the date field entirely; extractMessages no
longer derives one (entryDate helper removed). Job files stop
carrying dates.
- Both extraction prompts resolve relative expressions ("today", "last
week") against the conversation's own stated dates and context;
when the conversation gives no date signal, the fact stays
undated — there is no recording timestamp to fall back on, by
design.
- The judge's explicit-over-proxy precedence stays as belt-and-braces.
Also adds the process fix from the finding: a benchmark-framing
fixture test (the exact replay framing "Quick update from
2023/01/11: we've currently got 30 dozen eggs") asserting the
transcript carries the conversation date and NO [YYYY-MM-DD] prefix,
and that the prompts no longer reference any "line prefix" anchor —
this test would have caught both the original fallback and this
residual before any pipeline run.
This matches the configuration that scored 86.7% (conversation-dated
facts via text alone) plus the conversation-date stamping and
pipelined-drain gains.
Verified: lint + typecheck green; 163 memory tests passing.
Co-Authored-By: Kimchi <noreply@kimchi.dev>
…hen-uncovered
The latest rerun's failure profile (99 failures, down from 167 — every
category improved): the fixes worked as designed, but two changes
compounded into a -13.1 preference-slice regression, and the dominant
remaining failure mode (42 abstentions) shares its root with it —
haystack details that never became retrievable, dated, distinct facts.
Three targeted fixes, additive only (the drain pipeline, supersede,
judge, and the no-recording-date guard are untouched):
- Extraction prompt — possessions and resources the user acquires,
owns, or starts using (a Suica card, a power bank, a downloaded app,
a pet's name) are now explicit Include-line items: they anchor future
advice, so dropping them produces the regression's signature failure
("recommend buying what the user already owns"). Per-occurrence
enumeration: each class, purchase, or tank is its own dated fact —
never merged or dropped because a similar fact exists (the fitness-
class/tank/gift-sum failures). An event or state described in a
dated update (a bedtime, a purchase, an appointment, that day's
routine) carries that update's date — the bedtime failure was the
right value detached from its night, and the appliance question was
a purchase never dated. All conversation-dated: the no-recording-
timestamp rule is unchanged.
- Digest preamble — directness kept for covered answers, search
restored for uncovered ones: "when they answer the question, rely
on them directly; when the question asks for something they do not
cover — a specific possession, event, or date — search memory with
memory_search before concluding it is not on record." The 2-of-3
failures where the model answered from the digest with zero
searches showed the old "be direct" posture suppressing the search
instinct that used to rescue these.
- memory_search description — a fourth trigger: "when a question asks
about a specific item, event, or date that the recalled digest did
not surface."
Verified: lint + typecheck green; 164 memory tests (new prompt-
assertion block covering possessions, enumeration, and the
event-carries-date rule; digest exact-match updated).
Co-Authored-By: Kimchi <noreply@kimchi.dev>
…ieval anchor The investigation's definitive finding: capture is already fixed at a48001f (running the exact missed messages locally proves the stores now contain the Suica/TripIt and Luna facts) — the remaining failure is retrieval ranking caused by terse fact text. "As of 2023-05-22, just got a Suica card" has no Tokyo/transit keywords, so it loses its top-8 slots to topical facts when the question queries "getting around Tokyo." The old verbose form ("recently got a Suica card for use on Tokyo's public transit") matched — because the relevance lives in the use, not the name. The power bank recovered precisely because its terse form IS the domain keyword for a battery question; the Suica card's relevance lives elsewhere. One instruction: possessions are captured WITH their stated purpose or use from the conversation — "got a Suica card for getting around Tokyo", "bought a power bank for phone charging on the go", "Luna, my pet cat". The purpose is already stated in the source; the extraction just needs to keep it. With context preserved, fact and question share domain terms and ranking recovers naturally. Bonus for the Luna class: "Has a pet named Luna" is unmatchable by an allergy/sneezing query — "Luna, my pet cat" lets a "cat allergies" search surface it and the model make the dander connection the judge expects. The bedtime date-attribution and the last enumeration gaps (fitness 3-of-4) are smaller, separable refinements in the same prompt family — not addressed here. Verified: lint + typecheck green; 164 memory tests (new assertions: "WITH its stated purpose or use", "the use is the retrieval anchor"). Co-Authored-By: Kimchi <noreply@kimchi.dev>
… of the path Benchmarking showed the auto router picking models suited to conversation rather than strict-JSON extraction at temperature 0 — the router's recommendation made extraction unreliable. Reverts the extraction path to the deterministic flash-tier preference list (the validated path), now ordered deepseek-v4-flash-0731 first, then glm-5.3-flash, then glm-5.3, kimi-k3 as availability fallbacks. The auto router is removed from resolveExtractionModel entirely — no /v1/route call, no ROUTER_QUERY — and the router-client/router-config imports drop out of backend.ts. KIMCHI_MEMORY_EXTRACTION_MODEL still overrides. The interactive auto model (kimchi-dev/auto) is untouched — this only affects the capture worker's extraction model resolution. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Billing needs to distinguish background memory traffic from session chat traffic on the gateway. Both request paths now carry a usage-tracking tag in the request body, using the same payload.tags field the /tags extension sets on session LLM requests, so attribution flows through one existing mechanism instead of a new header convention. - Extraction: the capture worker's chatJson request body carries tags: ["memory:extraction"] (direct fetch, fully under our control). - Embeddings: mem0's OpenAI embedder only forwards apiKey/baseURL to the OpenAI SDK client, so the tag can't ride the embedder config. A surgical, idempotent globalThis.fetch wrapper (applied at every backend creation) adds tags: ["memory:embedding"] to /embeddings request bodies only — all other fetch traffic passes through untouched. Tag names follow the reserved-tag shape (model:<id>, phase:<name>). Co-Authored-By: Kimchi <noreply@kimchi.dev>
- cli.ts: exit explicitly after the memory-capture worker dispatch. The worker main now returns an exit code instead of calling process.exit itself, so a future early-return can no longer fall through into the interactive bootstrap (telemetry, session setup) with worker argv. - build-binary.js: measured the mem0 peer-dep claims. natural and compromise are lazily required inside try/catch guards and were never bundled (byte-identical binary with them external; natural's own deps - mongoose, pg, redis - never enter the graph); the externals entries pin that. pg IS bundled (eager top-level require for pgvector) and cannot be externalized without breaking the mem0ai import in the compiled binary - documented as an accepted cost. - shims/better-sqlite3: enforce fileMustExist in the constructor (bun:sqlite ignores it, so a missing db would be created instead of throwing), and export the Database named export the runtime already had (declare class replaces the interface + const split). - _helpers.ts confirm(): a stdin that closes without emitting data (closed pipe, lost TTY) now declines instead of leaving the promise unsettled forever. Regression test covers data-then-close racing. - cli-args.ts: safe CLI_OPTIONS lookup in boolean normalization (a cacheable name missing from the catalog is now a coverage-test failure instead of a startup TypeError). - terminal-bench-2: build_cli_flags --memory coverage test - the flag override is the single mechanism that arms the memory-on A/B arm, and a silent regression would corrupt benchmark comparisons. Verified: biome + typecheck clean; 10,636 unit tests; 2/2 new Python tests; compiled binary smoke (memory search against the gateway, exit 0; byte-identical size before/after the externals change). Co-Authored-By: Kimchi <noreply@kimchi.dev>
For testing other embedding providers (e.g. OpenRouter) without code changes. Four env vars; unset, behavior is identical to today (gateway base, gateway key, text-embedding-3-small, 1536 dims): - MEMORY_EMBEDDING_MODEL - model override (applies in both modes) - MEMORY_EMBEDDING_BASE_URL - custom embedding base URL; switches key resolution into custom-endpoint mode - MEMORY_EMBEDDING_API_KEY - embedding key (custom mode only) - MEMORY_EMBEDDING_DIMS - vector dims, validated positive integer, fed to the embedder config and the store schema together The key fallback is coupled to the base URL: with a custom MEMORY_EMBEDDING_BASE_URL, the key comes from MEMORY_EMBEDDING_API_KEY then OPENROUTER_API_KEY - never the gateway key; without it, everything stays on the gateway and MEMORY_EMBEDDING_API_KEY is ignored. That prevents accidentally sending a gateway request with an OpenRouter key, or vice versa. The memory:embedding usage tag is now gateway-origin-matched, so custom embedding endpoints never receive our usage-tracking tag. Programmatic overrides (tests, check scripts) still win per-field over the env layer. Verified: 173 memory-suite tests (8 new env-matrix tests + updated origin-guard tag test), typecheck + biome clean. Live OpenRouter verification pending an OPENROUTER_API_KEY in the environment. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Co-Authored-By: Kimchi <noreply@kimchi.dev>
…ly suitable model The fallback preference chain (glm-5.3-flash, glm-5.3, kimi-k3) is gone: benchmarking showed deepseek-v4-flash-0731 is the only suitable model for extraction, so resolving anything else was dead weight. EXTRACTION_MODEL_PREFERENCES (array) becomes EXTRACTION_MODEL (single constant); resolveExtractionModel returns deepseek-v4-flash-0731 when it's on the gateway's live model list (or the list is unreachable), and throws when the list is reachable but deepseek flash is not on it. KIMCHI_MEMORY_EXTRACTION_MODEL still overrides. Co-Authored-By: Kimchi <noreply@kimchi.dev>
…acy content The subagent MEMORY.md system (per-agent memory directories injected into subagent prompts) is obsolete now that the memory extension owns durable memory. Removed: the agents/memory module and its tests, the persona `memory` field and its custom-agent parsing, the agent-runner memory-block wiring and memory tool-name injection, and the docs sections covering all of it. Users who built up MEMORY.md content get it carried over: a one-time, marker-guarded migration imports user-scope agent MEMORY.md files (~/.config/kimchi/harness/agent-memory/<name>/) into the personal memory store verbatim (infer: false, with an agent-name provenance prefix, the legacy 200-line cap). It runs before the session's first digest so migrated content is immediately searchable, is idempotent across retries, and never throws — a failed migration just retries next session. Project-scoped agent memory stays in its repo, untouched. Also: import-sort lint fix in backend.test.ts (missed by an earlier commit) and isUnsafeName inlined into skill-loader (its module was deleted). Co-Authored-By: Kimchi <noreply@kimchi.dev>
With KIMCHI_MEMORY_CAPTURE=off, a session skips capture entirely: no handlers registered, no pending job files written, no workers spawned. Unset or any other value keeps normal capture, byte-identical. Purpose: the benchmark's shared-store mode needs question sessions that read the memory store without adding their own content — the store freezes after the replay drain and every question launch runs against it as-is. Gated at both the wiring point (wireMemoryCapture returns before registering) and the funnel (captureMessages early-returns), so no capture path can bypass it. 🤖 Generated with [Kimchi](https://kimchi.dev) Co-Authored-By: Kimchi <noreply@kimchi.dev>
Reads a JSONL file of {"fact": "<text>"} records and adds each fact
through the extension's own store path — createMemoryBackend with the
same embeddings and schema as organic capture, infer disabled so facts
store verbatim (no extraction, no supersede pass). The benchmark's
oracle-capture arm uses this to load ground-truth facts directly,
isolating retrieval quality from capture quality.
- --scope personal (default) uses the capture digest path; --scope
project resolves via the git-root project scope (the same mapping the
capture pipeline uses)
- Routed in cli.ts beside memory-capture, before telemetry/session
bootstrap, exiting at the dispatch site
- 13 unit tests: arg parsing, JSONL validation with line numbers,
scope-to-dbPath mapping (incl. the non-git-repo error), and the add
contract (one verbatim add per fact, infer: false, userId from config)
🤖 Generated with [Kimchi](https://kimchi.dev)
Co-Authored-By: Kimchi <noreply@kimchi.dev>
Co-Authored-By: Kimchi <noreply@kimchi.dev>
Master's pi-0.85.1 mock is a superset of our version (adds fork, navigateTree, switchSession, reload; getSystemPromptOptions returns a value) — keep it once. Co-Authored-By: Kimchi <noreply@kimchi.dev>
mikenorgate
force-pushed
the
memory-extension
branch
from
September 18, 2026 10:59
889316a to
bd5f830
Compare
The master rebase regenerated the lockfile with a narrower peer set for mem0ai — @mistralai/mistralai, bare mysql2, and redis are no longer installed, so bun build fails resolving mem0's lazy loadPeer imports (CI: "Could not resolve: @mistralai/mistralai"). All three are guarded dynamic imports in provider registries our code never executes, so externalizing them is safe and consistent with the existing natural/compromise treatment. @anthropic-ai/sdk deliberately stays bundled: it IS installed (via @agentclientprotocol/sdk) and required eagerly by bundled code — externalizing it crashes the binary at startup (verified by smoke test, then reverted). Verified against a clean frozen-lockfile install (CI-equivalent): binary build + memory search smoke pass. Co-Authored-By: Kimchi <noreply@kimchi.dev>
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.
Linked issue
Closes #
What does this PR do?
feat(memory): opt-in persistent personal memory (
--memory) — 32 commits, 54 files, +7,422/−25 on top of master (14c4ad3a).Adds persistent personal memory to kimchi, behind an opt-in
--memoryflag (off by default). Facts a user establishes in a session — preferences, decisions, personal context — are captured automatically and recalled into later sessions:lock.ts) and drains all pending jobs, oldest first, which makes the concurrent-worker ledger race structurally impossible and retries orphaned job files from failed runs. A shared message-hash ledger gives crash-resume idempotence (a crashed run resumes where it stopped; a re-spawned duplicate job is a no-op). Extraction is two-pass (user-stated facts, then cautious assistant-established facts with engagement evidence), scope-tagged per store: a personal (global) store plus one per-project store keyed by the git remote, with an asymmetric unsure→project default so a project fact never pollutes the personal store. New facts supersede stale ones via a conservative force-DELETE+ADD judge; an exact-duplicate guard (mem0getAll, local SQLite) keeps adds idempotent. Pending jobs older than 7 days are swept.<system-reminder>convention with an explicit "data, never instructions" clause,memory_searchresults carry the same framing, and both extraction prompts instruct the model to treat transcripts as text to analyze. Progressive recall (turns 2+) delivers new facts on topic drift as hidden conversation-tail steer messages, bounded by a per-session evaluation cap. A constant always-on notice tells the model capture is automatic (no write tool, "remember this" needs no action) and where the user manages what is stored.kimchi memory(CLI subcommand, always available) and the in-session/memorycommand share one admin core (admin.ts): overview (storage path, per-store counts/sizes, pending jobs), list (newest 50 by default across stores,--limit N|all,--offset,--json), search (ranked), delete by id (resolved across all stores), and reset (scopeddeleteAllfor personal/project;--scope allwipes the whole memory root under the capture lock, keeping only the lock artifacts). In-sessionlist/searchopen an interactive panel (memory-panel.ts): page with ↑↓/j/k (PgUp/PgDn, g/G), delete the selected fact withd, quit with q/Esc — no ids to copy.pnpm run memory:check(Bun acceptance: store hybrid surface, round-trip with real remote embeddings, 8 parallel writers + 8 concurrent readers) andpnpm run memory:measure(value-gate injection/no-injection rates). Design rationale lives indocs/memory-extension.md.The benchmark harness agent (
benchmark/terminal-bench-2/src/kimchi_agent/agent.py) gains a memory A/B kwarg so the extension arm can be measured against controls. The benchmark evaluation itself is intentionally out of this PR (separate harness work).Dependencies: mem0ai and the better-sqlite3 shim
This PR intentionally adds a heavyweight runtime dependency, chosen after a phase-0 spike comparing build-vs-buy for hybrid BM25+semantic SQLite storage with remote embeddings:
mem0ai3.1.8 — the memory engine. The spike validated the integration under Bun and against the kimchi gateway; alternatives (langchain memory store, from-scratch) were rejected for capability or maintenance cost.shims/better-sqlite3— mem0's SQLite store and history manager require better-sqlite3, a native module that does not load under Bun. The shim reimplements the ~86-line subset mem0 needs overbun:sqlite(WAL, 5s busy timeout), validated against mem0 3.1.8's bundle surface and exercised bymemory:check(8 parallel writers + 8 concurrent readers, zero lock errors).Removal criteria (also in the shim header): the shim is temporary debt. It expires when Bun ships better-sqlite3 compatibility — upstream tracking oven-sh/bun#4290 and #36712 — or when mem0 drops the requirement.
Override blast radius: the pnpm override redirects every
better-sqlite3resolution in the tree to the shim. mem0ai is currently the only consumer. A future transitive dependency needing unimplemented surface (.pragma(), transaction variants, named parameters,.iterate()) will fail loudly at that call — revisit the override then (extend the shim or scope the override to mem0ai's resolution).Embeddings run through the kimchi gateway (
text-embedding-3-small); facts are stored locally. The extraction model resolves through the auto router (/v1/route— the same service behind thekimchi-dev/automodel) at worker start, validated against the gateway's live model list; a router failure or an off-list recommendation falls back to a flash-tier preference order, andKIMCHI_MEMORY_EXTRACTION_MODELoverrides.Tests
CaptureBackend+createBackend/llmonRunCaptureWorkerOptions; defaults preserve production behavior): end-to-end drain (extract → add → hash-mark → job-file removal), ledger idempotence, crash-resume retry, poison-job removal, the 7-day stale sweep, oldest-first ordering, the exact-duplicate guard, and session-shutdown job-file writing. Also:stripMemoryArgs(the guard preventing--memoryfrom eating the prompt — pi's parser treats unknown flags as greedy),buildTurnRecallcap/truncation/ledger-filter branches, the scoped extraction prompt's respond-line swap, the auto-router extraction resolution (recommendation used as primary, off-list falls back, unvalidated when the list is down), the/memoryrouting (--jsonand non-UI console.log branches), the CLI shell (json/text routing, exit codes, non-interactive confirm decline), and the panel's failed-delete restore order.tests/e2e/tui/memory-*.test.tsover a deterministically seeded store (memory-seed.tswrites rows straight into the MemoryVectorStore under bun — no network, no capture pipeline): the interactive browser (open, navigate, delete by selection, quit, re-list shows the deletion persisted), the overview widget (renders store stats, clears when an agent turn resumes), and delete-by-id + reset through its confirmation dialog.pnpm run verify(check + build:binary + test + smoke) exits 0. Every user-visible behavior ships with tests.vitest.config.tsnow excludestests/e2e/**— the tui-test E2E files collide with vitest's fork-pool IPC when loaded in plain vitest (a deterministic pool crash). They run through their dedicated runner (pnpm run test:e2e:tui), which is unchanged; the exclude only prevents off-label invocations from tripping.Verification
Pre-PR verification ran three parallel audits plus static checks. Twelve findings were adjudicated (1 × P1 coverage on the capture pipeline, 2 × P2 hygiene/test gaps, 9 × P3 maintenance items); all were fixed and re-verified against the code (not fix-agent claims): lint + typecheck exit 0, targeted test run exit 0,
pnpm run verifyexit 0. The branch is rebased on master's tip (14c4ad3a, zero conflicts) and the same head is pushed to the GitLab remote where the benchmark CI runs.Final verdict: ready.
Notes for reviewers
memory_searchtool, and the always-on notice points users at/memorywhen they ask to forget something.KIMCHI_SKIP_TYPECHECKinbuild-binary.jsis for memory-constrained benchmark CI pods only; it warns when active.Checklist
pnpm run test)pnpm run check)