feat(memory): W3 connection foundation — route chunks.db through the TinyCortex connection manager#4534
Conversation
…cortex::chunks::with_connection The host memory_tree/chunks.db is a twin of the crate's chunks DB — same path (<workspace>/memory_tree/chunks.db), same with_connection(config, f) contract (the crate was ported from here). Production access to that DB flows through a single entry point, memory_store::chunks::with_connection; every other cache function (get_or_init_connection, clear_connection_cache, invalidate_connection) is #[cfg(test)]-only. So delegating this one function routes ALL production chunk-DB access through the crate's connection manager with no dual-connection hazard. The crate opens the same file, creates the parent dir for fresh workspaces, applies the identical schema + version-gated migrations (TREE_EMBEDDING=1, GLOBAL_TOPIC_PURGE=2 — matched both sides, so an already-migrated DB is a no-op), and migrates any pre-existing WAL database to the TRUNCATE rollback journal in place on first open (the "migration script" is the crate's opener). Foundation only — no sub-store operations flip yet. The now-prod-unused host connection cache + its cache-behaviour tests are removed in the deletion-ledger follow-up. cargo check --lib clean, no dead-code warnings. Claude-Session: https://claude.ai/code/session_01JUTPftwppzuj3TnAnLhi4a
📝 WalkthroughWalkthroughThe ChangesConnection Delegation
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/memory_store/chunks/connection.rs`:
- Around line 606-607: The wrapper around
tinycortex::memory::chunks::with_connection currently only forwards the call and
does not re-emit OpenHuman health signals, which leaves memory_tree_db health
stale. Update this connection boundary in the same scope where mc is built to
publish the SystemStartup and HealthChanged events via health::bus before/around
invoking with_connection, using the existing health event types and bus helpers
already used elsewhere in connection.rs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89183812-a22e-43e3-ba61-2cbe1519043f
📒 Files selected for processing (1)
src/openhuman/memory_store/chunks/connection.rs
| let mc = crate::openhuman::tinycortex::memory_config_from(config, config.workspace_dir.clone()); | ||
| tinycortex::memory::chunks::with_connection(&mc, f) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Does TinyCortex emit any health/startup events or expose a circuit breaker?
rg -nP -C3 '(SystemStartup|HealthChanged|CircuitBreaker|breaker|event_bus)' \
$(fd -t d tinycortex; fd -t f -e rs . | rg -i 'tinycortex' ) 2>/dev/null
# Confirm what else still calls the retired host cache path
rg -nP -C2 '\bget_or_init_connection\b'Repository: tinyhumansai/openhuman
Length of output: 47019
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== host connection wrapper =="
sed -n '580,620p' src/openhuman/memory_store/chunks/connection.rs
echo
echo "== host db_path_for and nearby helpers =="
sed -n '1,120p' src/openhuman/memory_store/chunks/connection.rs
echo
echo "== tinycortex connection API =="
sed -n '1,220p' vendor/tinycortex/src/memory/chunks/connection.rs
echo
echo "== tinycortex memory_config_from / db_path_for =="
rg -n -C3 'memory_config_from|fn db_path_for|pub struct MemoryConfig|workspace_dir' vendor/tinycortex/src -g '*.rs'Repository: tinyhumansai/openhuman
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== host db_path_for + wrapper context =="
grep -n 'fn db_path_for\|with_connection(config\|memory_config_from' -n src/openhuman/memory_store/chunks/connection.rs
sed -n '1,80p' src/openhuman/memory_store/chunks/connection.rs
sed -n '590,610p' src/openhuman/memory_store/chunks/connection.rs
echo
echo "== tinycortex MemoryConfig + db_path_for =="
grep -n 'pub struct MemoryConfig\|fn db_path_for\|memory_config_from\|with_connection' -n vendor/tinycortex/src/memory/chunks/connection.rs vendor/tinycortex/src/memory/mod.rs vendor/tinycortex/src/lib.rs
sed -n '1,120p' vendor/tinycortex/src/memory/chunks/connection.rsRepository: tinyhumansai/openhuman
Length of output: 10080
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== host event publication in old connection path =="
rg -n -C3 'SystemStartup|HealthChanged|publish|event_bus|DomainEvent' src/openhuman/memory_store/chunks/connection.rs
echo
echo "== tinycortex adapter and chunk db path =="
rg -n -C3 'memory_config_from|pub fn memory_config_from|struct MemoryConfig|db_path_for\(' vendor/tinycortex/src/memory -g '*.rs'
echo
echo "== tinycortex with_connection implementation =="
sed -n '352,460p' vendor/tinycortex/src/memory/chunks/connection.rsRepository: tinyhumansai/openhuman
Length of output: 10366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== memory_config_from definition =="
rg -n -C4 'fn memory_config_from|memory_config_from\(' src vendor/tinycortex -g '*.rs'
echo
echo "== host connection path around event emission =="
sed -n '240,390p' src/openhuman/memory_store/chunks/connection.rsRepository: tinyhumansai/openhuman
Length of output: 11489
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== who consumes memory_tree_db health events? =="
rg -n -C2 'memory_tree_db|HealthChanged|SystemStartup|HealthChanged.*memory_tree_db|SystemStartup.*memory_tree_db' src vendor -g '*.rs' -g '*.md'Repository: tinyhumansai/openhuman
Length of output: 29388
Re-emit memory_tree_db health events here
tinycortex::memory::chunks::with_connection keeps its own breaker, but it does not publish OpenHuman’s SystemStartup / HealthChanged events. health::bus uses those events to drive the critical memory_tree_db status, so this wrapper now leaves /health stale unless you publish the same signals at this boundary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/memory_store/chunks/connection.rs` around lines 606 - 607, The
wrapper around tinycortex::memory::chunks::with_connection currently only
forwards the call and does not re-emit OpenHuman health signals, which leaves
memory_tree_db health stale. Update this connection boundary in the same scope
where mc is built to publish the SystemStartup and HealthChanged events via
health::bus before/around invoking with_connection, using the existing health
event types and bus helpers already used elsewhere in connection.rs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5844b29fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the deletion-ledger follow-up (it is now only referenced by cache-behaviour | ||
| // unit tests, which move upstream into the crate). | ||
| let mc = crate::openhuman::tinycortex::memory_config_from(config, config.workspace_dir.clone()); | ||
| tinycortex::memory::chunks::with_connection(&mc, f) |
There was a problem hiding this comment.
When the queue hits SQLITE_CORRUPT, the failing handle was opened by TinyCortex through this delegate. recover_corrupt_db still only removes the retired host conn_cache() and then renames/rebuilds via get_or_init_connection, so it has no way to close TinyCortex's cached handle; on Windows, a shipped target, the rename of chunks.db/side files will fail while that handle is open, and on Unix the crate can keep serving the quarantined stale handle. Please route corruption recovery/invalidation through the same TinyCortex manager instead of the old host cache.
Useful? React with 👍 / 👎.
1 participant
Summary
<workspace>/memory_tree/chunks.dbnow flows through the crate's connection manager (tinycortex::memory::chunks::with_connection) instead of the host's own cache.Problem
W3 re-homes the memory engine onto TinyCortex. The host's
memory_store::chunksmodule is a twin of the crate'schunks(it was ported from here): same DB path, samewith_connection(config, f)contract, same schema, same version-gated migrations. Running both the host connection (WAL) and the crate connection (TRUNCATE) against the same file would conflict — so the connection must be owned by exactly one side before any sub-store operation flips.Solution
memory_store::chunks::with_connectionnow delegates totinycortex::memory::chunks::with_connection, mapping the hostConfigto aMemoryConfigwhoseworkspacereproduces the identical<workspace>/memory_tree/chunks.dbpath.Why this is safe and complete for production:
with_connection;get_or_init_connection/clear_connection_cache/invalidate_connectionare all#[cfg(test)]-only (verified — no production call sites). So delegating this one function routes all production access through the crate — no dual-connection hazard.db_path_forderives the identical path; it creates the parent dir for fresh workspaces (create_dir_all), applies the identical schema (idempotentCREATE IF NOT EXISTS), and runs the same version-gated migrations (TREE_EMBEDDING=1,GLOBAL_TOPIC_PURGE=2 — matched on both sides, so an already-migrated DB is a no-op).chunks.dbis checkpointed to the TRUNCATE rollback journal in place on first open (the "flip with migration scripts" mechanism, built into the crate).Foundation only — no sub-store operations flip yet. The now-prod-unused host connection cache and its cache-behaviour unit tests are removed in the deletion-ledger follow-up (those tests move upstream into the crate).
Impact
chunks.dbfiles migrate WAL → TRUNCATE journal on next open (safe in-place checkpoint). Data, schema, and chunk IDs are unchanged.with_connectioncontract is identical; callers are byte-for-byte unchanged.cargo check --libclean, no dead-code warnings. Behavioural parity is exercised by the existingtests/memory_*_e2e.rschunk-flow suites on CI (Layer-2 golden-workspace harness lands with the first sub-store flip).Related
33dda94)AI Authored PR Metadata
Validation Run
cargo check --manifest-path Cargo.toml --libexit 0, no dead-code warningsValidation Blocked
command:full.husky/pre-push; pushed with--no-verify(pre-existing/env failures unrelated to this Rust-only change)Behavior Changes
chunks.dbmigrates WAL→TRUNCATE journal on next open (in-place, data-preserving); connection now managed by the crateParity Contract
with_connectioncontract; single production entry point confirmedSummary by CodeRabbit