Bump tinymemory across the #18 arc (tinycortex in lockstep) - #5587
Conversation
vendor/tinymemory: dc3a725 -> f8bd9af (85 commits: the whole tinymemory#18 restructure -- contract crate, per-engine features, sync moved off the engine, conformance suite -- plus the follow-up fixes through tinyhumansai#62). vendor/tinycortex: 5fdeac9 -> 8401346, exactly the SHA tinymemory vendors; the two must move together because tinycortex-api is now a re-export of the TinyMemory contract rather than a duplicate of it (tinymemory#18 §A1). Both cargo workspaces gain the `[patch."https://github.com/tinyhumansai/tinymemory"]` table mapping tinymemory-api to the submodule path. The new tinycortex takes that crate by git; without the entry cargo resolves the git copy and the path copy as two distinct crates and type identity splits at the seam. The app workspace is its own cargo world and needs its own copy of the table. Everything else was verified ready ahead of this bump by the three-repo audit: all 102 tinymemory_core paths this tree names resolve against the new pin (the four compatibility surfaces tinymemory#60 added exist for exactly this bump), the episodic_insert Result<i64> change was already absorbed (store_golden.rs, hook_impl.rs), and the contacts/memory-git feature forwards already target the new feature names. The bump includes tinymemory#61 deliberately: its {toolkit}:{connection_id} tree-scope parity commit is required content -- an older SHA would reintroduce the tinyhumansai#5473 unreachable-items regression through the engine-free sync path. Locks regenerated by resolution only (cargo metadata); the compile gate is CI's.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (7)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThis change updates TinyMemory submodules and Cargo resolution, reuses the active memory client during Composio cleanup, refreshes memory allowlists and fixtures, and lowers the dependency-count calibration from 268 to 264 unique crate names. ChangesTinyMemory integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The PR updates the vendored tinymemory and tinycortex pins plus Cargo resolution compatibility; no actionable merge-blocking risk remains beyond normal CI and review. Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
the engine-module rename in the memory allowlist Two CI findings on the tinymemory bump, both the bump doing its job: 1. Feature-Gate Smoke: "kernel floor IMPROVED but was not ratcheted — flows resolves 264 names, limit still 268". The profile got SMALLER: tinymemory#43 (§D2) moved `axum` from tinymemory-core's normal dependencies to dev-only (its four references were all inside `mod tests`), and that takes the whole tail out of openhuman's always-on kernel profile -- axum, axum-core, axum-macros, matchit, httpdate, serde_path_to_error leave (-6). Two enter, tinymemory-sources and tinymemory-sync, the crates tinyhumansai#46/tinyhumansai#48 split relocated code into, so packages stay flat. Net -4 names. `scripts/kernel-floor.limits` is lowered 268 -> 264 with the history entry written in the file's own terms (measured on CI Linux; macOS resolves 265 per the documented skew), and the dep-sim calibration assertion moves with it. Verified by diffing the resolved crate set old-pin vs new-pin: exactly the six out, two in, listed above. 2. Core Coverage: three `bypass_allowlist` tests failed. tinymemory#18 §C1 renamed `core/src/tinycortex/` to `core/src/engine/`, so the two allowlist entries for the engine seam pointed at a file that no longer exists; and §B1 added `core/src/sync/pipelines/host.rs`, the engine-free sync runner -- an adapter over `MemoryClient` shaped exactly like the engine seam, with the same two needles. Entries moved to the new path and added for the new seam with the same justifications, each re-verified against the code: every `from_workspace_dir` hit in both files sits inside an inline `#[cfg(test)]` module, and `client_if_ready` is the production seam beneath the contract, not above it. All six scanner tests pass, including the one that fails on any entry that no longer matches. 12,093 tests passed on CI before these; the three scanner tests now join them.
…hange
Core Coverage failed on `memory_golden_fixture_e2e`: the fixture's
recorded DDL no longer matched what the code produces.
MIGRATION STORY (the review rule this fixture's own docs demand for any
diff that touches it): THERE IS NO MIGRATION. Nothing structural
changed. SQLite stores `CREATE TABLE` statements verbatim in
`sqlite_master`, comments included, and tinymemory#18 §C1 renamed the
module a comment inside `conversation_segments` names:
-- Per-session sequence numbers from tinycortex::memory::archivist::store
-- Per-session sequence numbers from crate::engine::backend::archivist::store
Verified rather than asserted: stripping SQL comments from both
manifests and diffing the object rows gives 125 objects before and 125
after, byte-identical -- every table, column, type, default, constraint
and index unchanged. An existing user workspace needs nothing; its
`sqlite_master` text differs from a fresh one only in that comment, and
no code reads it.
Regenerated with `scripts/regen-memory-golden-fixture.sh` rather than by
hand-editing the manifest, because the manifest is recomputed FROM the
committed `.db` blobs -- editing it alone cannot pass, by design. The
`.db` diffs are that regeneration.
The manifest header also picks up `memory::store::golden` ->
`memory::store_golden`, matching where the generator actually lives.
cargo test --test memory_golden_fixture_e2e: 3 passed
cargo test --test memory_golden_parity_e2e: 1 passed
`memory_source_status_counts_reader_and_composio_prefixes` failed with pending=2 where it expected 1. The assertion is right and the fixture was stale: it marked a chunk embedded by setting `mem_tree_chunks.embedding`, the LEGACY column the engine never writes. That column being dead is exactly the defect tinymemory#59 item 2 fixed: counting `embedding IS NULL` reported every chunk pending forever, so a healthy source showed `chunks_pending == chunks_synced` and the memory-sources UI never cleared. The status query counts against the live `mem_tree_chunk_embeddings` sidecar now, so the fixture writes there instead -- one row per (chunk, model signature), which is what "this chunk has an embedding" means today. No production behaviour changes here; the bump brings the corrected query and this makes the test's setup mean what it always intended. cargo test raw_coverage_all -- memory_source_status_counts: 1 passed
Composio's cleanup-target discovery opened its own MemoryClient via from_workspace_dir for the notion sync-state read. That constructor is not just an open: it starts an ingestion worker at construction, so every delete_connection with clear_memory put a second worker and a second write connection on the live store — the exact hazard the bypass allowlist names for that constructor, allowlisted at the time as "cleanup runs off a config workspace with no live binding". The discovery now takes &MemoryClientRef as a parameter. The RPC path resolves the live process client (active_memory_client, the same Config-derived workspace the handler's config came from), and the tests pass a client bound to their own temp workspace — the injection seam they were already using the config for. The allowlist entry moves to the one resolution site in connections.rs and names what it replaced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Pushed 828bcf2 — a wiring defect the seam audit surfaced: Composio's cleanup-target discovery constructed its own The discovery now takes Verified: bypass-allowlist ratchet 6/6 (staleness check included), composio suite 428/428, |
How this change flows2 changed behaviours across 1 relationship. The code graph does not know these behaviours yet — normal for newly added code, and a cold index otherwise. 10 further behaviours left out to keep the diagram readable. flowchart LR
n0["MemoryCleanupTarget<br/>changed"]:::changed
n1["gmail_memory_sources_for_connection<br/>changed"]:::changed
n1 -->|uses| n0
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
The submodule bump the three-repo audit verified ready. Draft: the compile gate is this CI run — locks were regenerated by resolution only.
vendor/tinymemorydc3a725→f8bd9afvendor/tinycortex5fdeac9→8401346Plus the
[patch."https://github.com/tinyhumansai/tinymemory"]table in both cargo workspaces (root +app/src-tauri) mappingtinymemory-apito the submodule path — without it cargo resolves the git copy and the path copy as two distinct crates and type identity splits.Pre-verified by the audit (both-sides file:line, before this PR existed):
tinymemory_core::paths this tree names resolve against the new pin — the four compat surfaces tinymemory#60 added exist for exactly this bump (module alias serving 35 call sites,SyncAuditEntryseam re-export,extract_item_idrestored, dualSyncStateStoreimpl)episodic_insert → Result<i64>already absorbed (store_golden.rs:224,hook_impl.rs:34/65)contacts/memory-gitfeature forwards already target the new names{toolkit}:{connection_id}tree-scope parity is required content — an older SHA reintroduces the Connector syncs stopped feeding the memory tree after the TinyCortex migration #5473 unreachable-items regression through the engine-free sync pathSYNC_ALREADY_RUNNINGnote is displayed, never parsed; concurrent same-connection syncs now skip instead of double-runningUnblocks: opencompany#1113 (its vendored tinymemory rides this tree).
Summary by CodeRabbit
Bug Fixes
Dependency Updates
Tests
Maintenance