feat(store/turso): shared-source libSQL backend rebased on upstream v0.10.57 #1
Loading…
Reference in a new issue
No description provided.
Delete branch "rebase/turso-on-upstream-v0.10.57"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Rebases the Turso/libSQL backend onto upstream rtk-ai/icm v0.10.57 (76 commits ahead of the fork point), while eliminating 14,000+ lines of duplicate code by switching to a
#[path]-shared-source approach.Tier: T1 (changes under
hosts/nixos/only indirectly; the icm binary is rebuilt from this fork — no flake.nix changes in this PR)What changed
Upstream sync (commit
2b71bc5)Upstream had a major backend-pluggability sprint (PRs #301, #302, #304):
Storeenum +dispatch!macro for runtime backend selectionICM_DB_BACKENDenv var (sqlite/postgres/opensearch)pgvector), OpenSearch (BM25+knn)icm serve(#291),icm forget+icm listin hooks (#252)Turso was forward-ported as a 4th additive backend.
Shared-source redesign (commit
6c30a75) — upstream-mergeableBefore:
turso_store.rs(7,398 lines) +turso_schema.rs(847 lines) = verbatim copies ofstore.rs/schema.rswithrusqlite::→dbcompat::substitution.After:
#[path]-based compilation —store.rsandschema.rsare compiled once as shared source, included into two provider contexts:Net change: −8,022 lines (302 inserted, 8,324 deleted).
The only code changes to
store.rsandschema.rs:use rusqlite::→use super::sql::(provider alias)collect_rows<T>signature:MappedRows<'_, F>→impl Iterator<Item = sql::Result<T>>(works for both rusqlite's lifetime-bearing type and dbcompat'sIntoIter)Auto-detect fix (in
6c30a75)BackendKind::from_env()precedence (matches production wiring in Hermes/orchestra/modules/icm.nix):ICM_DB_BACKENDexplicit env var always winsTURSO_DATABASE_URL/LIBSQL_URLauto-selects turso (turso feature only,#[cfg(feature = "turso")])Production wiring never sets
ICM_DB_BACKEND— onlyTURSO_DATABASE_URL. This fix is required for the binary to work without config changes.Bug fixes (commit
9aab56c)schema.rstests:super::storeresolves tosqlite_backend::schema's parent, not the backend wrapper. Fixed tosuper::super::storeperf_fts_search_100: libsql async-wrapped-in-sync is 2–3× slower than native rusqlite; raised ceiling from 1 s to 5 slibsqlTLS feature: required even for plainhttp://URLs (Builder::new_remote panics without it)Test results
Smoke test vs local sqld
All core operations verified against
sqld 0.24.33athttp://127.0.0.1:28080(NOT production at 100.98.32.20:8080):memoir create/memoir liststore/recall(memory)facts set/facts getfeedback record/feedback listtranscript start-session/record/show/search/statsKnown constraint
--features backend-sqlite,tursocompiles and type-checks clean, but cannot link in one test binary:libsql-ffiandlibsqlite3-sysboth bundle the sqlite3 amalgamation and produce duplicate symbols under mold/lld. This is a known upstream libsql constraint. Production usage is always one-or-the-other.Also on upstream GitHub PR #262
The shared-source approach (upstream-mergeable, per the coordinator's request) will also be pushed to https://github.com/rtk-ai/icm/pull/262 once this Forgejo review approves.
Fixes the production TURSO_DATABASE_URL auto-detect regression + eliminates 14k lines of duplicates.
remembersubcommand 0ee0e2a555/rememberskill now callsicm remember0cc540930dBare byte slices such as `&text[text.len() - 2000..]` panic when the cut lands inside a multi-byte UTF-8 char ('→' is 3 bytes). The PreCompact hook crashed on a transcript containing '→' — the root cause was the raw slice in the extraction fallback (extract.rs). The compact-path truncation was already char-safe (#110), but seven other sites still sliced on raw byte offsets and abort on multilingual/emoji content. Add `truncate_tail_at_char_boundary` next to the existing `truncate_at_char_boundary` and route every raw slice through the two helpers: - extract.rs raw-text extraction fallback (the reproduced crash) - PostToolUse async enqueue + `extract enqueue` 8 KB tail caps - compact-path 4 KB tail (inline boundary loop de-duplicated) - hook-event note (200), extract-patterns summary (120), claude JSON parse error (200), generic `truncate()` Add two tail-helper tests covering '→' and mixed ASCII/arrow input. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>`icm list` now mirrors `icm recall`'s output flags so external tooling (TakoIA-style agents, scripts that build a per-topic memory map, etc.) can enumerate a topic programmatically without piping through a separate parser. ### CLI surface ``` icm list --topic <T> --all --sort {weight|created|accessed} \ --format {human|toon|json|toml} --limit <N> ``` - `human` (default) is the legacy multi-line view kept verbatim so terminal users see no regression. - `toon`, `json`, `toml` reuse `recall_format::render` so the structured output is byte-for-byte consistent with `icm recall`. - `--limit N` truncates *after* sorting so the top-N by weight (or by created / accessed) is what gets emitted. ### TOML format New `RecallFormat::Toml` variant. Emits `[[memories]]` arrays with `id`, `topic`, `importance`, `weight`, `access_count`, `created_at`, `last_accessed`, `summary`, and (when present) `keywords` / `raw_excerpt` / `score`. The 384-dim embedding vector is intentionally omitted — useless in a human-readable TOML. Available on both `icm recall --format toml` and `icm list --format toml` for consistency, as the issue suggested. ### Smoke (release binary) ``` $ icm list --all --format toml [[memories]] id = "01K..." topic = "takoia/agent/B" importance = "low" ... $ icm list --all --format toon --limit 2 memories[2]{id,topic,importance,weight,summary}: 01K..., takoia/agent/B, low, 1.000, uses SAML 01K..., takoia/agent/A, high, 1.000, uses Postgres ``` ### Tests - 2 new unit tests in `recall_format.rs` (`toml_round_trips`, `toml_empty_list_renders_clean`). - Existing `recall_format` tests still pass. - Workspace cargo test + `clippy --workspace --all-targets -- -D warnings` clean. Closes #269. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Adds an opt-in `[archive]` config block. When enabled, the `hook prompt` and `hook post` paths tee every user message and tool output into the existing FTS5-indexed `sessions`/`messages` tables — keyed by the host agent's session id so re-fires within one Claude Code / Codex / Gemini conversation land under a single row. Also exposes `icm sessions {search,list,show,stats,forget}` as the UX-friendly entry point. Internally it delegates to the same handlers as `icm transcript …` (which stays for direct manipulation: start, record, etc.) — the new surface is the read-mostly subset that matters for the issue. Why a separate `[archive]` block (not under `[extraction]`): - Extraction = curated/lossy: distilled facts into Memory. - Archive = verbatim/lossless: every event into the sessions table. Both share the same hook ingestion point but answer different questions ("what did we decide" vs "what was actually said"). Per the issue, retention policy and secret scrubbing are open questions — landing the off-by-default switch keeps the door open for those follow-ups without forcing them now. Public API: - `TranscriptStore::ensure_session(id, agent, project, metadata)`: idempotent insert keyed by the external session id. INSERT OR IGNORE so re-firing the hook is a no-op for the sessions row. - `ArchiveConfig { enabled, max_bytes_per_event }` with sane default cap of 32 KB per event. - `icm sessions search|list|show|stats|forget` CLI surface. Tests: - `test_ensure_session_is_idempotent` (store). - 10 unit tests in `crates/icm-cli/src/archive.rs`: cap_bytes boundaries (multibyte), session id resolution (stdin / transcript path / absent), record_event noop-when-disabled, skip-when-no-id, persist-under-external-id, truncation at byte cap. - `perf_session_archive_search_2k_messages` (store): regression bell for FTS5 latency over 2k archived messages (debug budget 1500ms; release: <100ms). Smoke (release binary, fake HOME): - `[archive].enabled = true` → `hook post` and `hook prompt` both archive their events under `sess-001`; `icm sessions list` shows the row; `icm sessions search bash`/`snapshot` returns the verbatim content; `icm sessions show sess-001` replays chronologically. - `cargo test --workspace` green (537 tests). - `cargo clippy --workspace --all-targets -- -D warnings` clean.MVP scope (per the issue, which flagged the full direction as RFC + phased): land schema + CRUD + CLI now; defer auto-population-from-extraction and entity-resolution heuristics to follow-up RFCs. A new `facts` table keyed by `(entity, key)` answers exact factual questions ("which GCP project for X", "what version of Y", "what host is Z on") with a primary-key lookup — distinct from semantic recall (probabilistic, can miss or down-rank a sharp query) and the verbatim transcript archive (lossless but unranked). Supersession as a first-class concept (issue bonus): updating a fact marks the previous row `superseded_at = now` and inserts a new active row. `icm facts history` shows the full chain. This gives ICM a home for "fact changed" — orthogonal to dedup (PR #249, "same fact stated twice"). Public API: - `icm_core::FactsStore` trait: - `set_fact(entity, key, value, source)` — idempotent on unchanged value, supersedes on change. - `get_fact(entity, key)` — active-row lookup. - `list_facts(entity, prefix?)` — alphabetic by key. - `history(entity, key)` — full chain, newest first. - `forget_fact(entity, key)` — hard delete of active + history. - `facts_stats()` — counts + top entities. - `icm_core::{Fact, FactsStats}` types. - SQLite schema with `UNIQUE INDEX … WHERE superseded_at IS NULL` so the "one active row per slot" invariant is enforced at the DB layer. - `icm facts {set,get,list,history,forget,stats}` CLI surface. Tests (icm-store): - set/get roundtrip, list alpha, prefix filter, supersede keeps history, idempotent unchanged-value set, forget cascades, stats breakdown, rejects empty entity/key. - `perf_facts_get_at_10k_under_5ms`: bench invariant. Debug budget 5ms/lookup; release: well under 1ms. Smoke (release binary, fake HOME): - set creates, supersede returns "superseded: old -> new", same-value re-set returns "unchanged"; history shows the chain with timestamps + status; get returns the active value plus provenance on stderr; stats breaks down active vs total. - 100 process-level `facts get` calls: ~5ms/call avg, dominated by process spawn + DB open; in-process lookup is sub-ms per the perf test.Adds a hook-driven, no-MCP equivalent of Context-Engine's `record_code_area` tool: every time Claude Code / Codex / Gemini / Copilot calls `Edit` / `Write` / `MultiEdit` / `NotebookEdit`, the already-installed PostToolUse hook (`icm hook post`) extracts `tool_input.file_path` and upserts a row in a new `code_areas` table. Same `(project, file_path)` increments `touch_count` instead of duplicating, so the table grows in *file count*, not edit count. - New table `code_areas(id, project, file_path, description, session_id, tool_name, touch_count, first_touched_at, last_touched_at)` with a `UNIQUE(project, file_path)` constraint driving the upsert. - `SqliteStore::upsert_code_area` — ON CONFLICT bumps `touch_count`, refreshes `last_touched_at`/`session_id`/`tool_name`, and only overwrites `description` when the caller passes `Some` (preserves the most recent meaningful hint). - `SqliteStore::list_code_areas` — filter by `project`, exact or suffix `file_path`, `since` timestamp; ordered by `last_touched_at DESC`. - `extract_tool_input_file_path()` in `main.rs` covers the three shapes we've seen across Claude Code 1.x / 2.x, Codex, and Gemini (`tool_input.file_path`, top-level `file_path`, `tool_input.arguments.file_path`). - `cmd_hook_post` calls `upsert_code_area` for matching tool names **before** the extract counter — independent of the throttle, never blocking the hot path, errors swallowed (telemetry must never fail the hook). - New `icm code-areas` CLI command with `--in-file`, `--project`, `--since`, `--limit`, `--format {table,json}`. - `description` is `None` in the MVP. Once #165 (LLM-summarized briefing) lands, the same provider infrastructure can feed a diff summary into `description` opt-in. - No new dependencies. Hook + transcript + sqlite plumbing was already in place; the patch reuses all of it. - Source MCP tool (`Context-Engine-AI/Context-Engine`) is source-available proprietary; ICM ships an Apache-2.0 equivalent from scratch. - 7 new unit tests in `icm-store` (upsert idempotency, touch_count increment, description preservation/overwrite, project + path-suffix filters, `since` filter, ordering, count). - 513 tests pass across the workspace, `cargo clippy --workspace --all-targets -- -D warnings` clean. - Manual smoke against the release binary: 3 hook payloads (`Edit`, `Write`, `MultiEdit`) → `code-areas` reports 2 unique paths, `auth.rs` has touch_count=2, `Bash` payload correctly ignored, `--in-file` and `--format json` both work. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>/remember-sessionskill for session checkpointing (#251) d2edca3488`icm recall`, `icm list`, `icm stats`, `icm topics`, `icm health` used to require a writable SQLite connection — they opened the DB with WAL journaling, ran the schema migration on every open, and the recall path itself wrote `last_accessed` / `access_count`. In a read-only Codex automation or any `chmod -w` sandbox, every read-like command aborted with "attempt to write a readonly database". This PR adds: - `SqliteStore::open_readonly(path)` — opens with the URI form `file:<path>?mode=ro&immutable=1`. The `immutable=1` flag is essential: plain `SQLITE_OPEN_READ_ONLY` is not enough because SQLite still tries to create / refresh the `-shm` / `-wal` companion files, which fails when the parent dir is non-writable. `immutable=1` tells SQLite the file will not change for the lifetime of the connection and disables WAL bookkeeping entirely. - A `readonly: bool` field on `SqliteStore`. `maybe_auto_decay`, `update_access`, and `batch_update_access` (the writes that the recall path triggers) short-circuit to `Ok(())` so recall stays best-effort. `apply_decay` (user-called via `icm decay`) returns a clear `IcmError::ReadOnly("apply_decay")` instead of a cryptic SQLite error. - A new `--read-only` global CLI flag and `ICM_READONLY=1` env var. `read_only_requested(cli_flag)` centralizes the truthy semantics ("1", "true", any non-empty / non-"0" value enables; absent or "0" disables). - Same `immutable=1` URI is reused inside `SqliteStore::read_stored_embedding_dims` (the helper from #267 that peeks the stored dim before the writable open). Otherwise the no-embeddings + chmod -w combination still failed on the peek itself. Public API: - `IcmError::ReadOnly(String)` new variant carrying the rejected operation name. - `SqliteStore::open_readonly(path)` and `SqliteStore::is_readonly()`. - `--read-only` CLI flag (global) + `ICM_READONLY=1` env var. Tests (icm-store): - `open_readonly_errors_on_missing_file` - `open_readonly_can_read_existing_db` - `read_only_recall_path_skips_access_bookkeeping` — `maybe_auto_decay` / `update_access` / `batch_update_access` are no-ops; verified by re-opening writable afterwards and asserting `access_count` and `last_accessed` are unchanged. - `read_only_apply_decay_returns_readonly_error` - `read_only_mutation_attempts_are_rejected_by_sqlite` — defense-in-depth: even a write method that isn't explicitly gated must fail because the connection itself is RO. Tests (icm-cli): - 5 `read_only_requested_tests` covering the CLI/env truthy matrix (flag wins, env "1" enables, env "0" disables, env empty disables, neither = writable). Smoke (release binary, fake HOME): - Seed 2 memories writable. `chmod -R a-w` the data dir. Then: `--read-only recall "hello"` returns the matching memory; `--read-only stats` prints the count; `ICM_READONLY=1 list --all` enumerates both rows; an attempted `store` is rejected with "attempt to write a readonly database" and exit code 1.Two related issues in the Ollama summarizer path that surfaced through `icm extract-pending`: 1. **qwen3 / deepseek-r1 / phi4-reasoning / etc. silently produced empty output.** These "thinking" Ollama families emit a `<think>…</think>` block by default. With our 400-token `num_predict` budget the block consumes the entire response, so the visible part after `</think>` is empty — surfacing only as the opaque "provider returned empty output" line. Fix: detect known thinking families and send `"think": false` in the Ollama `/api/generate` body. Ollama ignores the flag on non-thinking models, so the option is safe when set on a positive match. New `is_thinking_model` helper pattern-matches `qwen3*`, `qwen3-coder`, `deepseek-r1*`, `granite3-think`, `phi4-reasoning`, `smollm3`. 2. **Silent fallback to `qwen2.5:0.5b` when no model was set.** The previous `unwrap_or("qwen2.5:0.5b")` masked a real misconfiguration: the user's config sets `[extraction.summarizer] model = "qwen3:8b"`, but if the value didn't reach the request (config not loaded, lookup error), `OllamaSummarizer::summarize` happily used a totally different 0.5B model and the failure looked like a model quality problem. Fix: refuse explicitly and point to the config key + the `--model` override. The caller now sees the real error instead of a useless small model running. Also: when Ollama does come back empty, log the model name, `num_predict`, and whether `think` suppression fired — so the OpenCode plugin / cron drain (which runs detached with `stdio: "ignore"`) leaves a debuggable trail. Tests: - `is_thinking_model_matches_qwen3_family` (incl. case + tag). - `is_thinking_model_matches_other_thinking_families` (deepseek-r1, granite3-think, phi4-reasoning, smollm3). - `is_thinking_model_skips_non_thinking_families` (qwen2.5, llama3.2, mistral, gemma3, empty).* fix(embeddings): apply e5 query/passage instruction prefixes multilingual-e5 models are trained to expect "query: " on search queries and "passage: " on stored documents; omitting the prefixes degrades retrieval (per the intfloat/multilingual-e5 model card). ICM embedded all text raw, so e5 ran out-of-distribution on every store and recall. Apply the prefixes in the embedder, model-aware: - embed() / embed_batch() prepend the document ("passage: ") prefix - a new Embedder::embed_query() prepends the query ("query: ") prefix, used by the CLI recall and MCP recall paths Only e5-family models (MultilingualE5 Small/Base/Large) are affected; every other model is byte-identical to before. The default impl of embed_query() delegates to embed(), so non-e5 Embedder impls are unchanged. Deterministic IR eval on a real 424-memory corpus (12 lexically-distinct paraphrase queries, same targets, only the prefix differs): recall@1 8->9/12, recall@5 10->12/12, MRR 0.764->0.850, no regressions. Near-verbatim queries (already easy) are unaffected. Migration: existing e5 stores must be re-embedded once with `icm embed --force`; a mix of prefixed and unprefixed vectors is worse than neither. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style: cargo fmt Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: patrick <patrick@rtk-ai.app>icm forgetandicm listtoicm_block(#252) b698ee6517Build + smoke test results (all green)
nix build .#icm— PASS (exit 0)cargo build --no-default-features --features "turso,tui"— PASS (exit 0, 1m43s)Smoke test — PASS
Local libSQL file mode (no
TURSO_DATABASE_URLset) — exercises the libSQL-local code path without touching the production sqld at 100.98.32.20:8080. Store → recall round-trip confirmed.Also verified
cargo check -p icm-cli, backend-sqlite): OK — no regression to the upstream default path.ICM_DB_BACKENDerror handling: correctly rejects unknown values (the binary only hastursocompiled in; requestingsqlitegives a clear "not compiled with its Cargo feature" error, not a crash).rebase: port turso/libSQL backend onto upstream v0.10.57to feat(store/turso): shared-source libSQL backend rebased on upstream v0.10.579aab56c99dtoedef56a0527bdc87523dto66dfa57be766dfa57be7to60d568806bSuperseded by the
flakebranch: upstream icm 0.10.57 now ships a native runtime-selectable PostgreSQL backend (upstream issue #301), so the custom libSQL/Turso backend is no longer needed. The shared store migrated to postgres+pgvector on cfx-mgmt (nixos-llm PR #168);flakecarries only the Nix flake packaging plus an opt-in OpenAI-compatible remote embedder — both candidates for upstreaming. sqld is retired; final sqld state archived on Unraid.View command line instructions
Manual merge helper
Use this merge commit message when completing the merge manually.
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.