From 373a88098cb626f3c3b428014b2f208301fa40ce Mon Sep 17 00:00:00 2001 From: Shanu Date: Mon, 24 Aug 2026 23:49:16 +0530 Subject: [PATCH 1/6] Give the host every read it still runs raw SQL for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit openhuman#5560 wants zero direct engine references in the host. What was left was not routing — it was six reads the contract could not express, so five handlers kept a SQLite handle to answer them. These are those six. `MemoryChunks::count_chunks` answers the unpaged total a paging caller needs. The filter is the listing's own: tinycortex#157 factors the WHERE-clause construction out and the count reuses it verbatim, so a count and the page it labels cannot drift. Deliberately NOT defaulted to `list_chunks(..).len()` — that compiles, passes a naive test, and silently returns the driver's row cap as if it were the total. Three `MemoryEntities` members return what the occurrence index actually holds. `entities()` was not widened to fit: it is namespace-scoped and hotness-ranked, and the index is neither, so bending one member to serve both would have made its contract a lie. Two honest members beat one fudged one. Two `MemoryTree` members walk the sealed summary forest and its recent leaves, which the tree family could describe one node at a time but never enumerate. Two verdicts recorded rather than members added: the doctor's chunk count is already `store_stats` — its blocker is that `run` is sync while the member is async, which is a host restructure, not a contract gap. And the obsidian registry is host filesystem policy that happens to live in the engine crate, like redact and safety before it; the host can own it. CONTRACT_VERSION goes (2, 2) -> (3, 0). Every member here lands on a family a driver may already advertise, and negotiation is family-granular — a driver advertising `Chunks` at 2.2 would bind and then be asked for a method it has never heard of. The major half refuses that bind instead of discovering it at the call. Worth saying plainly: #85/#86/#89/#90 added methods to advertised families and stayed minor. That was wrong by this rule. Nothing was bound across the gap because those releases and their hosts moved in lockstep, but it is drift, not precedent. METHODS 99 -> 103, registered in all four places and verified by sequence, not by eye. Loader E2E passes against the built cdylib. Co-Authored-By: Claude Opus 5 --- crates/tinymemory-api/src/provider/chunks.rs | 35 ++ crates/tinymemory-api/src/provider/content.rs | 119 +++- .../tinymemory-api/src/provider/knowledge.rs | 133 ++++- crates/tinymemory-api/src/provider/mod.rs | 6 +- crates/tinymemory-bus/src/lib.rs | 2 +- crates/tinymemory-bus/src/names.rs | 21 +- crates/tinymemory-bus/src/provider/types.rs | 44 ++ crates/tinymemory-bus/src/tree.rs | 171 +++++- crates/tinymemory-bus/src/version.rs | 2 +- crates/tinymemory-bus/src/version_tests.rs | 18 +- .../tinymemory-core/src/store/chunks/store.rs | 10 + crates/tinymemory-core/src/store/entities.rs | 143 ++++- crates/tinymemory-module/src/lib.rs | 7 + crates/tinymemory-module/src/service/mod.rs | 111 +++- crates/tinymemory-module/tests/module_e2e.rs | 54 ++ .../tinymemory-tinycortex/src/engine/mod.rs | 393 +++++++++++- .../tests/full_provider_conformance.rs | 565 ++++++++++++++++++ vendor/tinycortex | 2 +- 18 files changed, 1777 insertions(+), 59 deletions(-) diff --git a/crates/tinymemory-api/src/provider/chunks.rs b/crates/tinymemory-api/src/provider/chunks.rs index 1a8f96d0..de1bb049 100644 --- a/crates/tinymemory-api/src/provider/chunks.rs +++ b/crates/tinymemory-api/src/provider/chunks.rs @@ -32,6 +32,7 @@ use async_trait::async_trait; +use crate::capabilities::Capability; use crate::chunks::Chunk; use crate::error::MemoryError; use crate::provider::types::SourceScope; @@ -67,6 +68,40 @@ pub trait MemoryChunks: Send + Sync { scope: Option<&SourceScope>, ) -> Result, MemoryError>; + /// How many chunks `query` matches, ignoring its `limit` and `offset`. + /// + /// The predicate is [`Self::list_chunks`]'s, exactly: same filters, same + /// `scope`, same fail-closed reading of an empty allowlist. Only the page + /// bounds are dropped, because a total that moved as the caller paged + /// through it would not be a total. + /// + /// # Why this is a member and not the caller's arithmetic + /// + /// A caller rendering "showing 20 of 431" cannot derive 431 from a page: it + /// would have to list the whole match set unbounded, which is the query the + /// row limit exists to prevent, and it would still be capped by the + /// driver's own ceiling — silently, so 10,000 would read as the truth. The + /// count has to be answered where the `WHERE` clause is. + /// + /// The two must be built from one predicate driver-side. A count that + /// disagrees with the list beside it points the caller at pages that hold + /// nothing, which is worse than not offering a count at all. + /// + /// # Errors + /// + /// [`MemoryError::Unsupported`] from a driver that implements this family + /// but predates this member — it is deliberately not derived from + /// [`Self::list_chunks`] by default, because that default would silently + /// answer with the driver's row cap instead of the real total. Otherwise + /// backend failures only; no match yields `0`. + async fn count_chunks( + &self, + _query: &ChunkQuery, + _scope: Option<&SourceScope>, + ) -> Result { + Err(MemoryError::unsupported(Capability::Chunks)) + } + /// One chunk by id. /// /// # Errors diff --git a/crates/tinymemory-api/src/provider/content.rs b/crates/tinymemory-api/src/provider/content.rs index d343e171..ff7b9b19 100644 --- a/crates/tinymemory-api/src/provider/content.rs +++ b/crates/tinymemory-api/src/provider/content.rs @@ -22,7 +22,7 @@ use crate::capabilities::Capability; use crate::chunks::Chunk; use crate::error::MemoryError; use crate::provider::types::{IngestItem, IngestOutcome, SourceScope}; -use crate::tree::{IngestRequest, QueryResult, TreeStatus}; +use crate::tree::{IngestRequest, QueryResult, SummaryForest, TreeLeaf, TreeStatus}; use crate::types::{NamespaceDocumentInput, NamespaceRetrievalContext, StoredMemoryDocument}; /// Bulk content ingestion — the driver owns chunking and embedding. @@ -192,6 +192,21 @@ pub trait MemoryDocuments: Send + Sync { /// implicitly on ingest because the **host** owns scheduling. A driver runs one /// step when asked; it does not get to install its own background loop. This is /// the same rule as the engine's `queue::run_once`. +/// +/// # Navigating one node, and walking the whole forest +/// +/// [`Self::drill_down`] addresses a node by id and returns it with its direct +/// children — enough to descend a tree a caller is already inside. +/// [`Self::summary_forest`] and [`Self::recent_leaves`] answer the question +/// that has no starting id: what trees exist, how they nest, and what content +/// hangs off them. Both are here rather than in +/// [`MemoryRetrieval`](crate::provider::MemoryRetrieval) because neither ranks +/// and neither takes a query; they are structure, not results. +/// +/// The embedded driver happens to serve the two from different storage — the +/// markdown time tree on disk, the sealed summary forest in tables — and the +/// contract deliberately does not encode that split. See +/// [`crate::tree`] for the shapes and why they are described separately there. #[async_trait] pub trait MemoryTree: Send + Sync { /// Append raw content to the ingestion buffer for later sealing. @@ -246,4 +261,106 @@ pub trait MemoryTree: Send + Sync { /// /// Backend failures only. async fn cascade(&self, namespace: &str) -> Result; + + /// Walk every sealed summary the store holds, across every tree. + /// + /// # Why [`Self::drill_down`] cannot answer this + /// + /// `drill_down` starts from a node id and returns that node with its + /// direct children. A caller that wants the whole forest has no id to + /// start from — that is what it is asking for — and no way to discover + /// one, because nothing else in the contract enumerates trees. Walking it + /// by repeated `drill_down` would also be one round trip per node, over a + /// bus, to rebuild a shape the driver already has in one table. + /// + /// [`crate::provider::MemoryRetrieval::retrieve_children`] does not answer + /// it either, for a different reason: it *ranks*. It needs a seed node and + /// returns scored hits without a parent link, which is a reading list + /// rather than a graph. + /// + /// # `scope` is a predicate, not a post-filter + /// + /// The allowlist must be applied **inside** the driver's query for the + /// reasons in [`SourceScope`], and this member is the one where getting it + /// wrong is least visible: an unscoped forest walk hands back every source + /// in the store at once, which is precisely the shape a per-turn source + /// gate exists to prevent. `None` means unrestricted and must be a + /// decision, not a default the caller drifted into. + /// + /// A driver returns nodes whose tree the scope allows. It may therefore + /// return a node whose `parent_id` names one it withheld; see + /// [`crate::tree::TreeSummary::parent_id`] for what a caller does with + /// that. + /// + /// # Bounds + /// + /// `limit` caps the nodes returned and the driver clamps it to its own + /// cap — a caller cannot raise the ceiling by asking for more, the same + /// rule [`crate::provider::ChunkQuery::limit`] carries. Hitting either + /// bound sets [`SummaryForest::truncated`] rather than erroring. + /// + /// Tombstoned summaries are never returned. A driver that keeps them + /// filters them out here; "deleted" is not a state a caller has to know + /// about to draw a graph. + /// + /// # Errors + /// + /// [`MemoryError::Unsupported`] from a driver that has a tree family but + /// cannot enumerate it — deliberately not an empty forest, because a + /// driver with trees reporting none is a lie a caller would render as an + /// empty store. Backend failures otherwise; a store that has sealed + /// nothing returns an empty, untruncated forest, which is true of it. + async fn summary_forest( + &self, + _limit: usize, + _scope: Option<&SourceScope>, + ) -> Result { + Err(MemoryError::unsupported(Capability::Tree)) + } + + /// The most recent leaves, each with the summary that sealed it, newest + /// first. + /// + /// The forest's bottom edge. [`Self::summary_forest`] returns the summary + /// nodes and the child ids they sealed over; this returns the leaves + /// themselves with the back-pointer that says which summary claimed them, + /// so a caller can attach content to the structure without one lookup per + /// leaf. + /// + /// # Why not [`crate::provider::MemoryChunks::list_chunks`] + /// + /// That returns the same rows and drops the link: a [`Chunk`] does not say + /// which summary sealed it, and the link is what makes a leaf part of a + /// tree rather than a loose row. It is also the half that changes without + /// the chunk changing — a leaf gains a parent when the scheduler seals it, + /// long after ingest. + /// + /// Both halves are separate calls rather than one combined read because + /// the two bounds are separate: a caller may want the whole forest + /// skeleton and only the newest few hundred leaves, and folding them into + /// one response would make the smaller bound pay for the larger. + /// + /// # Bounds and scope + /// + /// As [`Self::summary_forest`]: `limit` is clamped by the driver, and + /// `scope` is applied inside the query, before the limit, so a disallowed + /// source cannot starve permitted ones out of the page. + /// + /// [`TreeLeaf::preview`] is a label, capped at + /// [`crate::tree::LEAF_PREVIEW_CHARS`] characters. Bodies are + /// [`crate::provider::MemoryChunks::chunk_detail`]'s job, one row at a + /// time; a forest-sized read carrying whole bodies would not fit a frame. + /// + /// # Errors + /// + /// [`MemoryError::Unsupported`] on the same terms as + /// [`Self::summary_forest`]. Backend failures otherwise; a store with no + /// leaves returns an empty vector. + async fn recent_leaves( + &self, + _limit: usize, + _scope: Option<&SourceScope>, + ) -> Result, MemoryError> { + Err(MemoryError::unsupported(Capability::Tree)) + } } diff --git a/crates/tinymemory-api/src/provider/knowledge.rs b/crates/tinymemory-api/src/provider/knowledge.rs index cc979cda..77fa674c 100644 --- a/crates/tinymemory-api/src/provider/knowledge.rs +++ b/crates/tinymemory-api/src/provider/knowledge.rs @@ -16,7 +16,7 @@ use async_trait::async_trait; use crate::error::MemoryError; use crate::graph::{GraphEdge, GraphNode, GraphView, GraphViewQuery}; -use crate::provider::types::{DiffReport, EntityHit, SnapshotRef}; +use crate::provider::types::{DiffReport, EntityHit, EntityOccurrence, SnapshotRef}; use crate::types::{GraphRelationRecord, MemoryKvRecord}; /// How many edges the default [`MemoryGraph::graph_view`] traversal scans per @@ -31,6 +31,31 @@ use crate::types::{GraphRelationRecord, MemoryKvRecord}; pub const INBOUND_SCAN_LIMIT: usize = 4_096; /// The entity index: who and what the stored memory is about. +/// +/// ## Two readings of one index, and why both are here +/// +/// [`Self::entities`] and [`Self::entity_edges`] read the index the way an +/// agent does: inside one namespace, ranked by what is warm. +/// [`Self::top_entities`], [`Self::chunk_entities`] and +/// [`Self::entity_chunk_ids`] read it the way a browser does: across the whole +/// store, ranked by what is actually there, and joined back to the chunks the +/// observations came from. Neither reading is derivable from the other — each +/// method says which assumption breaks — so the family carries both rather +/// than one call with a mode flag. +/// +/// ## Why the second three are defaulted and the first three are not +/// +/// The first three have been in this trait since it existed; every driver that +/// compiles implements them. The second three arrived later, and making them +/// required would break every out-of-tree driver at its next `cargo build` for +/// a capability it may genuinely not have — the trait equivalent of a +/// non-additive wire change, which this contract does not make. +/// +/// So they default to [`MemoryError::Unsupported`], carrying +/// `entities.` rather than the bare family name: the family *is* +/// supported, and an operator reading "unsupported capability: entities" from +/// a driver whose entity list works would be chasing the wrong thing. A driver +/// that can answer these should override them; the embedded engine does. #[async_trait] pub trait MemoryEntities: Send + Sync { /// List entities in a namespace, ranked by hotness when `query` is `None` @@ -79,6 +104,112 @@ pub trait MemoryEntities: Send + Sync { namespace: &str, entity_ids: &[String], ) -> Result<(), MemoryError>; + + /// The most-observed entities in the **whole store**, optionally narrowed + /// to one kind. + /// + /// # Why this is not [`Self::entities`] with a wider scope + /// + /// [`Self::entities`] is namespace-scoped and hotness-ranked. This is + /// neither: it reads the occurrence index as it stands — every namespace + /// at once, ordered by how often each entity was indexed — which is what + /// "who and what does this store know about at all" asks for. A caller + /// cannot assemble that from the namespace-scoped call: it would have to + /// enumerate namespaces and merge their rankings on hotness, and hotness + /// is a per-driver, per-namespace number that does not survive a merge. + /// + /// Rows are [`EntityOccurrence`] rather than [`EntityHit`] for the reason + /// that type's docs give — the index holds a surface sample and a count, + /// and no hotness at all. + /// + /// `kind` is validated, not merely applied: an unrecognised kind is + /// [`MemoryError::Invalid`], never an empty vector, because a misspelled + /// filter that matched nothing is indistinguishable from a store that + /// holds nothing. That is + /// [`crate::provider::MemoryRetrieval::search_entities`]'s rule, kept the + /// same here so one filter does not behave two ways. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a `kind` the driver does not recognise, + /// otherwise backend failures. An empty index yields an empty vector. + async fn top_entities( + &self, + kind: Option<&str>, + limit: usize, + ) -> Result, MemoryError> { + // Discarded rather than underscore-prefixed in the signature: the + // parameter names are what rustdoc shows a driver author, and + // `_kind` reads as vestigial where `kind` reads as the contract. + let _ = (kind, limit); + Err(MemoryError::unsupported_raw("entities.top_entities")) + } + + /// Every entity indexed against one chunk, most-observed first. + /// + /// The inverse of [`Self::entity_chunk_ids`], and the only read in this + /// family that starts from content instead of from an entity: it is how a + /// caller labels a chunk it already has — a retrieval hit, a row in a + /// browser — without re-running extraction over the text. + /// + /// A summary node's id is accepted too, and answered from the same index. + /// The parameter is named for the common case, not to exclude the other: + /// refusing an id the index can answer would send the caller to raw SQL + /// for the difference. + /// + /// One entity may appear more than once, once per distinct + /// [`EntityOccurrence::surface`], because the two forms are the evidence a + /// caller has for how this chunk actually named it. Deduplicating by id + /// here would throw that away and leave `surface` meaning "whichever row + /// sorted last". + /// + /// # Why there is no `limit` + /// + /// Every other list in this family is bounded by one, and this one is + /// already bounded by the thing it reads: a chunk's rows are what that + /// chunk's own extraction produced. There is no query for a caller to + /// narrow and no ranking for a cut-off to respect — a truncated answer + /// would simply be a wrong description of the chunk. + /// + /// # Errors + /// + /// Backend failures only. An unknown `chunk_id` yields an empty vector + /// rather than [`MemoryError::NotFound`], for [`Self::entity_edges`]'s + /// reason: "nothing was extracted from it" and "there is no such chunk" + /// are the same answer to this question. + async fn chunk_entities(&self, chunk_id: &str) -> Result, MemoryError> { + let _ = chunk_id; + Err(MemoryError::unsupported_raw("entities.chunk_entities")) + } + + /// The ids of the chunks one entity was observed in, newest first. + /// + /// The inverse of [`Self::chunk_entities`], and the member that makes an + /// entity usable as a filter: a caller that has resolved a name to a + /// canonical id gets the content behind it and reads that content through + /// [`crate::provider::MemoryChunks`], which is where chunk bodies belong. + /// Returning the chunks themselves would duplicate that family's shape + /// here and double the bytes for a caller that already holds them. + /// + /// [`Self::entity_edges`] does not cover this and cannot: it answers + /// entity-to-entity, and there is no path from an edge back to the text + /// the co-occurrence was observed in. + /// + /// **Chunks only.** A driver that also indexes derived nodes — summaries, + /// rollups — leaves them out: they are not chunks, and a caller filtering + /// a chunk list by these ids would find ids that match nothing. + /// + /// # Errors + /// + /// Backend failures only; an unknown `entity_id` yields an empty vector. + async fn entity_chunk_ids( + &self, + entity_id: &str, + limit: usize, + ) -> Result, MemoryError> { + let _ = (entity_id, limit); + Err(MemoryError::unsupported_raw("entities.entity_chunk_ids")) + } } /// The key/value and relation graph tier. diff --git a/crates/tinymemory-api/src/provider/mod.rs b/crates/tinymemory-api/src/provider/mod.rs index 35146e06..6df598bf 100644 --- a/crates/tinymemory-api/src/provider/mod.rs +++ b/crates/tinymemory-api/src/provider/mod.rs @@ -91,7 +91,7 @@ pub use retrieval::{ RetrievalNodeKind, RetrievalResponse, SourceRetrievalQuery, }; pub use types::{ - ChangeKind, DiffReport, EntityHit, EntityRef, ExportPage, ExportRecord, FlushOutcome, - ImportOutcome, IngestItem, IngestOutcome, MaintenanceReport, ResetOutcome, SnapshotRef, - SourceChange, SourceItem, SourceScope, + ChangeKind, DiffReport, EntityHit, EntityOccurrence, EntityRef, ExportPage, ExportRecord, + FlushOutcome, ImportOutcome, IngestItem, IngestOutcome, MaintenanceReport, ResetOutcome, + SnapshotRef, SourceChange, SourceItem, SourceScope, }; diff --git a/crates/tinymemory-bus/src/lib.rs b/crates/tinymemory-bus/src/lib.rs index 65b94e84..6553ff16 100644 --- a/crates/tinymemory-bus/src/lib.rs +++ b/crates/tinymemory-bus/src/lib.rs @@ -2,7 +2,7 @@ //! the members that carry them. //! //! TinyMemory ships as a loadable `TinyBus` module: `crates/tinymemory-module` -//! exports one object with 99 members on it, built as a `cdylib`. A host that +//! exports one object with 105 members on it, built as a `cdylib`. A host that //! loads it — OpenHuman — can call into it but cannot `use` anything out of it, //! so the payload vocabulary has to be published as an ordinary library. This //! is that library. diff --git a/crates/tinymemory-bus/src/names.rs b/crates/tinymemory-bus/src/names.rs index 71dade98..958dc968 100644 --- a/crates/tinymemory-bus/src/names.rs +++ b/crates/tinymemory-bus/src/names.rs @@ -94,6 +94,10 @@ pub mod methods { pub const SEAL: &str = "Seal"; /// `Cascade` — cascade. pub const CASCADE: &str = "Cascade"; + /// `SummaryForest` — every sealed summary in the store, with its tree. + pub const SUMMARY_FOREST: &str = "SummaryForest"; + /// `RecentLeaves` — the newest leaves and the summaries that sealed them. + pub const RECENT_LEAVES: &str = "RecentLeaves"; // Entities, relations and the namespaced key/value store. /// `Entities` — entities. @@ -104,6 +108,12 @@ pub mod methods { pub const TOUCH_ENTITIES: &str = "TouchEntities"; /// `SearchEntities` — search entities. pub const SEARCH_ENTITIES: &str = "SearchEntities"; + /// `TopEntities` — the store-wide entity index, most-observed first. + pub const TOP_ENTITIES: &str = "TopEntities"; + /// `ChunkEntities` — every entity indexed against one chunk. + pub const CHUNK_ENTITIES: &str = "ChunkEntities"; + /// `EntityChunkIds` — the chunks one entity was observed in. + pub const ENTITY_CHUNK_IDS: &str = "EntityChunkIds"; /// `Relations` — relations. pub const RELATIONS: &str = "Relations"; /// `PutRelation` — put relation. @@ -197,6 +207,9 @@ pub mod methods { pub const STORAGE_KINDS: &str = "StorageKinds"; /// `ChunkEmbeddings` — chunk embeddings. pub const CHUNK_EMBEDDINGS: &str = "ChunkEmbeddings"; + /// `CountChunks` — how many chunks `ListChunks` matches, page bounds + /// ignored. + pub const COUNT_CHUNKS: &str = "CountChunks"; // The scored retrieval surface. /// `FastRetrieve` — fast retrieve. @@ -260,7 +273,7 @@ pub mod methods { /// The order matters: `tinybus`'s `Interface::members()` returns declaration /// order, and the module compares the two sequences directly rather than as /// sets, so a reordering is caught alongside an addition or a removal. -pub const METHODS: [&str; 99] = [ +pub const METHODS: [&str; 105] = [ methods::DRIVER_ID, methods::CAPABILITIES, methods::HEALTH, @@ -360,6 +373,12 @@ pub const METHODS: [&str; 99] = [ methods::RETRIEVE_LEAVES, methods::RECALL_NAMESPACE_SCORED, methods::SEARCH_ENTITIES, + methods::COUNT_CHUNKS, + methods::TOP_ENTITIES, + methods::CHUNK_ENTITIES, + methods::ENTITY_CHUNK_IDS, + methods::SUMMARY_FOREST, + methods::RECENT_LEAVES, ]; #[cfg(test)] diff --git a/crates/tinymemory-bus/src/provider/types.rs b/crates/tinymemory-bus/src/provider/types.rs index d4b5356c..a5d17c4e 100644 --- a/crates/tinymemory-bus/src/provider/types.rs +++ b/crates/tinymemory-bus/src/provider/types.rs @@ -330,6 +330,50 @@ pub struct EntityHit { pub mentions: u32, } +/// One row of the entity **occurrence** index: an entity as it was actually +/// observed, and how many observations are behind the row. +/// +/// ## Why this is not [`EntityHit`] +/// +/// [`EntityHit`] answers "what is this namespace about, and what is warm right +/// now": it is namespace-scoped, ranked by a driver-computed hotness, and its +/// [`EntityRef::name`] is a canonical display name the driver stands behind. +/// This answers a different question — "what is in the index" — and every one +/// of those three properties differs: it spans the whole store, it ranks by +/// raw observation count, and it carries a [`Self::surface`], which is one of +/// the literal forms the source text used and nothing more. +/// +/// Folding the two together would need a field to lie. A surface placed in +/// `name` reads as canonical to every caller that renders it, and a hotness +/// synthesised for an index row would rank on a number no decay curve +/// produced. Two shapes, each true about its own query, is the cheaper answer. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct EntityOccurrence { + /// Canonical, driver-stable entity id — the same id space as + /// [`EntityRef::id`], so an id read here can be passed straight back to an + /// entity-keyed call. + pub entity_id: String, + /// Entity kind as a wire string (`person`, `email`, `topic`, …), the same + /// open vocabulary as [`EntityRef::kind`]: a kind this build does not + /// recognise must still round-trip. + pub kind: String, + /// One surface form the entity was observed under — a sample, not a name. + /// + /// Which sample is the driver's choice, and it may change as rows are + /// added, so this is for showing a caller how the text read, never for + /// identity. Empty is legitimate: an index that records occurrences + /// without keeping the source form has nothing truthful to put here. + #[serde(default)] + pub surface: String, + /// How many indexed observations this row aggregates. + /// + /// The unit is the driver's occurrence row, not "times the word appeared": + /// an index keyed per `(entity, node)` counts a node once no matter how + /// often the entity is named inside it, while one keyed per span counts + /// every span. Compare within one driver's results only. + pub mentions: u32, +} + /// Identity of a captured snapshot. /// /// The engine's own snapshot type additionally carries the git commit SHA and diff --git a/crates/tinymemory-bus/src/tree.rs b/crates/tinymemory-bus/src/tree.rs index b6c6bd55..cd3bc93f 100644 --- a/crates/tinymemory-bus/src/tree.rs +++ b/crates/tinymemory-bus/src/tree.rs @@ -1,7 +1,13 @@ -//! Domain types for the markdown time-based summary tree. +//! Domain types for the summary trees. //! -//! Organises summaries as a time hierarchy: root → year → month → day → hour -//! (leaf). Ported from OpenHuman's `memory_tree/tree_runtime/types.rs`. +//! Two shapes live here, and the file is ordered that way. The first is the +//! **markdown time tree**: summaries organised as a time hierarchy, root → year +//! → month → day → hour (leaf), ported from OpenHuman's +//! `memory_tree/tree_runtime/types.rs`. The second, below +//! [`node_id_to_path`], is the **sealed summary forest**: one tree per ingest +//! source, levelled by seal generation. They are navigated by different members +//! of the same family — see the section comment further down for why one cannot +//! answer for the other. use chrono::{DateTime, Datelike, Timelike, Utc}; use serde::{Deserialize, Serialize}; @@ -207,6 +213,165 @@ pub fn node_id_to_path(node_id: &str) -> PathBuf { } } +// ── The sealed summary forest ───────────────────────────────────────────── +// +// Everything above describes the *markdown time tree*: one node per hour, day, +// month and year, addressed as `2024/03/15/09`, one `.md` file each, navigated +// by `MemoryTree::drill_down`. The types below describe a second shape — the +// sealed summary **forest**: one tree per ingest source rather than one per +// calendar, levelled by seal generation rather than by calendar unit, and with +// no calendar-shaped node id for `drill_down` to address it by. +// +// The contract does not require a driver to keep the two apart. The embedded +// engine happens to — the markdown tree is files, the forest is tables — and a +// driver with a single structure answers both surfaces from it. What the +// contract does require is that both are reachable, because a host that can +// reach only the first has to read the second out of the driver's storage to +// draw it, which is the split-brain this contract exists to end. + +/// Maximum length, in characters, of [`TreeLeaf::preview`]. +/// +/// Fixed here rather than left to each driver because the preview is a *label* +/// and the caller lays it out: a driver that returned whole bodies would blow +/// the frame budget on a forest-sized read, and one that returned forty +/// characters would silently truncate a caller that had budgeted for more. A +/// caller that wants the body asks +/// `MemoryChunks::chunk_detail` for the one leaf it is +/// showing, which is a single row rather than every row. +pub const LEAF_PREVIEW_CHARS: usize = 200; + +/// One sealed summary node, with the tree it belongs to denormalised onto it. +/// +/// # Why not a ranked hit +/// +/// This overlaps `RetrievalHit` in almost every field and is deliberately not +/// it. A hit carries a `score`, which is meaningless for a structural walk — +/// nothing was ranked and there was no query to rank against — and it carries +/// no `parent_id`, because a ranked list is flat and has no reason to. The +/// parent link is the whole point here: it is the edge a caller draws a graph +/// from, and reconstructing it from each node's `child_ids` means holding the +/// entire forest in memory first, which is exactly what a truncated read +/// cannot do. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct TreeSummary { + /// Stable summary-node id, unique across the store. + pub id: String, + /// Id of the tree this node was sealed into. + pub tree_id: String, + /// The owning tree's kind — `source`, `topic`, `global`, …. + /// + /// Open vocabulary, and a plain string for the same reason + /// `MemorySourceSink::accept_source_items` takes + /// its `source_kind` as one: the set belongs to the driver and grows + /// without a contract change. A caller that does not recognise a kind must + /// still render the node. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub tree_kind: String, + /// The owning tree's scope — what it covers, e.g. `slack:#eng`, + /// `github:acme/widget`. Empty when the driver does not scope its trees. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub tree_scope: String, + /// Seal generation: `1` for a summary over raw leaves, `2` over `1`, and so + /// on. Never `0` — a leaf is a [`TreeLeaf`], not a summary at level zero. + pub level: u32, + /// Parent summary id, or `None` while this node is its tree's current root. + /// + /// A `Some` that names a node **absent from the same read** is expected + /// rather than a fault: the parent may sit beyond the read's bound, or the + /// scope may allow this node's tree and not its parent's. A caller + /// building edges must treat an unresolvable parent as a root, which is + /// what the host's Memory tab does. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub parent_id: Option, + /// The children sealed under this node, fixed at seal time: leaf ids at + /// level 1, lower-level summary ids above it. + /// + /// Not every level-1 child id resolves to a [`TreeLeaf`]. A document tree + /// seals over logical units — a commit, an issue, a page — whose ids never + /// existed as chunk rows, so a caller must label an unresolved child from + /// the id itself rather than assuming a lookup will find it. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub child_ids: Vec, + /// Inclusive start of the time span this node's children cover. + pub time_range_start: DateTime, + /// Inclusive end of the time span this node's children cover. + pub time_range_end: DateTime, +} + +/// One leaf and the summary it was sealed under. +/// +/// The back-pointer is why this is not `MemoryChunks::list_chunks`: a chunk row +/// says nothing about which summary claimed it, and that link is the edge +/// between the forest's bottom level and the content under it. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct TreeLeaf { + /// The leaf's chunk id — the same id `MemoryChunks` addresses it by. + pub chunk_id: String, + /// The summary that sealed over this leaf, or `None` when nothing has + /// sealed it yet. + /// + /// `None` is the normal state of freshly-ingested content, not an error: + /// sealing is a scheduled step the host drives, so an unsealed leaf is one + /// the scheduler has not reached. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub parent_summary_id: Option, + /// The logical source this leaf came from. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub source_id: String, + /// A label for the leaf: its first non-empty line, truncated to + /// [`LEAF_PREVIEW_CHARS`] characters. + /// + /// Characters, not bytes — a byte cut would split a multi-byte codepoint, + /// and a driver that answered with invalid UTF-8 would fail to encode + /// rather than return a short label. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub preview: String, + /// Inclusive start of the leaf's time coverage. + pub time_range_start: DateTime, + /// Inclusive end of the leaf's time coverage. + pub time_range_end: DateTime, +} + +/// A bounded walk of the sealed summaries in a store. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct SummaryForest { + /// The nodes, ordered by tree, then level, then seal time. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub summaries: Vec, + /// Whether the walk stopped at a bound rather than at the end of the store. + /// + /// A bound is not an error — the same reading `GraphView::truncated` + /// takes — but it is not a uniform thinning either. The order is + /// tree-major, so a truncated walk drops **whole trees** off the tail + /// rather than sampling across them: a caller that renders this as the + /// complete picture is showing a store with sources missing, and one that + /// counts nodes from it is counting a prefix. Say so in the UI, or raise + /// the bound and read again. + /// + /// Always serialized, unlike the fields above: a caller that has to notice + /// this must not have it disappear from the payload when it is `false`, + /// because "absent" and "not truncated" are then the same bytes and the + /// only reading left is the optimistic one. + #[serde(default)] + pub truncated: bool, +} + +/// First non-empty line of `content`, truncated to [`LEAF_PREVIEW_CHARS`]. +/// +/// Here rather than in each driver so two drivers cannot disagree about what a +/// preview is, and so the host is not left re-deriving it from a body the +/// forest read deliberately does not carry. +pub fn leaf_preview(content: &str) -> String { + content + .lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .unwrap_or("") + .chars() + .take(LEAF_PREVIEW_CHARS) + .collect() +} + #[cfg(test)] #[path = "tree_tests.rs"] mod tests; diff --git a/crates/tinymemory-bus/src/version.rs b/crates/tinymemory-bus/src/version.rs index 798f721a..7a666402 100644 --- a/crates/tinymemory-bus/src/version.rs +++ b/crates/tinymemory-bus/src/version.rs @@ -60,7 +60,7 @@ /// added to a family a driver may already advertise** (negotiation is /// family-granular, not method-granular, so that case cannot be made minor-safe /// by negotiation alone). -pub const CONTRACT_VERSION: (u16, u16) = (2, 2); +pub const CONTRACT_VERSION: (u16, u16) = (3, 0); /// Whether a driver speaking `remote` can be bound against this build. /// diff --git a/crates/tinymemory-bus/src/version_tests.rs b/crates/tinymemory-bus/src/version_tests.rs index 19ce415b..c334d793 100644 --- a/crates/tinymemory-bus/src/version_tests.rs +++ b/crates/tinymemory-bus/src/version_tests.rs @@ -7,10 +7,20 @@ use super::*; #[test] -fn contract_version_is_two_two() { - // (2, 2): the `episodic` family was added, which the version rule makes a - // minor bump — capability negotiation is what keeps an older driver safe. - assert_eq!(CONTRACT_VERSION, (2, 2)); +fn contract_version_is_three_zero() { + // (3, 0): `count_chunks`, the three entity-occurrence members and the two + // tree-forest members were added to families a driver may ALREADY + // advertise. The rule makes that a major bump and not a minor one, and the + // reason is the whole point of the rule: negotiation is family-granular, + // so a driver advertising `Chunks` at (2, 2) would be bound and then asked + // for a method it has never heard of. The major half is what refuses that + // bind instead of discovering it at the call. + // + // Note for anyone reading the history: #85/#86/#89/#90 also added methods + // to advertised families and stayed on the minor half. That was wrong by + // this rule; those releases and their hosts moved in lockstep so nothing + // was bound across the gap, but it is drift, not precedent. + assert_eq!(CONTRACT_VERSION, (3, 0)); } #[test] diff --git a/crates/tinymemory-core/src/store/chunks/store.rs b/crates/tinymemory-core/src/store/chunks/store.rs index 96c436fd..59115e77 100644 --- a/crates/tinymemory-core/src/store/chunks/store.rs +++ b/crates/tinymemory-core/src/store/chunks/store.rs @@ -67,6 +67,16 @@ pub fn count_chunks(config: &Config) -> Result { crate::engine::backend::chunks::count_chunks(&engine_config(config)) } +/// How many chunks [`list_chunks`] would return for `query`, ignoring its +/// `limit` and `offset`. +/// +/// Filtered, unlike [`count_chunks`] above, and built from the listing's own +/// `WHERE` clause engine-side rather than from a second copy of it — a total +/// that disagrees with the page it accompanies is worse than no total. +pub fn count_chunks_matching(config: &Config, query: &ListChunksQuery) -> Result { + crate::engine::backend::chunks::count_chunks_matching(&engine_config(config), query) +} + pub fn extraction_coverage(config: &Config) -> Result { crate::engine::backend::chunks::extraction_coverage(&engine_config(config)) } diff --git a/crates/tinymemory-core/src/store/entities.rs b/crates/tinymemory-core/src/store/entities.rs index 8f81f89b..b3453939 100644 --- a/crates/tinymemory-core/src/store/entities.rs +++ b/crates/tinymemory-core/src/store/entities.rs @@ -166,32 +166,151 @@ pub fn count_entity_index(config: &Config) -> Result { index(config)?.count_entity_index() } -/// Most frequently observed entities, with recency as the tie-breaker. -pub fn top_entities(config: &Config, limit: usize) -> Result> { +/// One aggregated row of `mem_tree_entity_index`, named for the columns it +/// holds rather than for what a caller might render. +/// +/// [`TopEntity`] carries the same four values under a `name` that is really a +/// `MAX(surface)` sample. That reading is fine where the caller only needs a +/// label, and wrong where it crosses the driver contract, which promises a +/// canonical name under `name`. This type keeps `surface` called `surface` so +/// the promise is not made by accident. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct EntityIndexRow { + /// Canonical entity id. + pub entity_id: String, + /// Stable entity kind string, as stored. + pub entity_kind: String, + /// An observed surface form — a sample of one row in the group, not a + /// canonical name. + pub surface: String, + /// Number of index rows aggregated into this one. + pub mentions: u32, +} + +/// Store-wide entity rows, most-observed first, optionally one kind only. +/// +/// Deliberately **not** tree-scoped: `namespace_entities` answers the scoped +/// question, and this one exists for the workspace-wide view where the caller +/// is asking what the store holds at all. Recency breaks ties, so two entities +/// seen the same number of times order by which was seen last. +/// +/// `kind` is matched against the stored `entity_kind` verbatim; validating it +/// belongs to the caller, which knows the vocabulary it accepts. +pub fn top_entity_rows( + config: &Config, + kind: Option<&str>, + limit: usize, +) -> Result> { let memory = memory_config_from(config, config.workspace_dir().clone()); let connection = crate::engine::backend::chunks::shared_connection(&memory)?; let guard = connection.lock(); + // The `?1 IS NULL OR` form keeps one prepared statement for both the + // filtered and unfiltered call, rather than concatenating SQL per call. let mut statement = guard.prepare( "SELECT entity_id, entity_kind, MAX(surface), COUNT(*) FROM mem_tree_entity_index + WHERE (?1 IS NULL OR entity_kind = ?1) GROUP BY entity_id, entity_kind ORDER BY COUNT(*) DESC, MAX(timestamp_ms) DESC - LIMIT ?1", + LIMIT ?2", )?; let rows = statement - .query_map([i64::try_from(limit).unwrap_or(i64::MAX)], |row| { - let mentions: i64 = row.get(3)?; - Ok(TopEntity { - id: row.get(0)?, - kind: row.get(1)?, - name: row.get(2)?, - mentions: u32::try_from(mentions.max(0)).unwrap_or(u32::MAX), - }) - })? + .query_map( + rusqlite::params![kind, i64::try_from(limit).unwrap_or(i64::MAX)], + index_row, + )? .collect::>>()?; Ok(rows) } +/// The entity rows recorded against one tree node, most-observed first. +/// +/// Grouped by surface as well as by id: one entity seen under two forms is two +/// rows, because the form is the evidence of how this node's text named it. +/// +/// The count is `1` for every row under the current schema — the primary key +/// is `(entity_id, node_id)`, so one node cannot hold two rows for the same +/// entity — and is reported anyway: it is the same aggregate +/// [`top_entity_rows`] returns, and an index that later keys occurrences per +/// span would make it meaningful without a shape change here. +pub fn node_entity_rows(config: &Config, node_id: &str) -> Result> { + let memory = memory_config_from(config, config.workspace_dir().clone()); + let connection = crate::engine::backend::chunks::shared_connection(&memory)?; + let guard = connection.lock(); + let mut statement = guard.prepare( + "SELECT entity_id, entity_kind, surface, COUNT(*) + FROM mem_tree_entity_index + WHERE node_id = ?1 + GROUP BY entity_id, entity_kind, surface + ORDER BY COUNT(*) DESC, entity_id ASC", + )?; + let rows = statement + .query_map(rusqlite::params![node_id], index_row)? + .collect::>>()?; + Ok(rows) +} + +/// Ids of the **leaf** nodes one entity was observed in, newest first. +/// +/// `node_kind = 'leaf'` is the filter the scorer's write path defines: it +/// stamps `leaf` for a scored chunk and `summary` for a summariser-curated +/// node. Summary nodes are excluded because their ids are not chunk ids — a +/// caller filtering a chunk list by them would match nothing. +/// +/// `GROUP BY` rather than `SELECT DISTINCT` so the sort key is a selected +/// aggregate: the primary key already makes `node_id` unique per entity, but a +/// `DISTINCT` ordered by an unselected column is the kind of query that +/// depends on how permissive the engine happens to be. +pub fn entity_leaf_node_ids(config: &Config, entity_id: &str, limit: usize) -> Result> { + let memory = memory_config_from(config, config.workspace_dir().clone()); + let connection = crate::engine::backend::chunks::shared_connection(&memory)?; + let guard = connection.lock(); + let mut statement = guard.prepare( + "SELECT node_id, MAX(timestamp_ms) AS seen_at + FROM mem_tree_entity_index + WHERE entity_id = ?1 AND node_kind = 'leaf' + GROUP BY node_id + ORDER BY seen_at DESC + LIMIT ?2", + )?; + let rows = statement + .query_map( + rusqlite::params![entity_id, i64::try_from(limit).unwrap_or(i64::MAX)], + |row| row.get::<_, String>(0), + )? + .collect::>>()?; + Ok(rows) +} + +/// Read one aggregated row in the shape both grouped queries above select. +fn index_row(row: &rusqlite::Row<'_>) -> rusqlite::Result { + let mentions: i64 = row.get(3)?; + Ok(EntityIndexRow { + entity_id: row.get(0)?, + entity_kind: row.get(1)?, + surface: row.get(2)?, + mentions: u32::try_from(mentions.max(0)).unwrap_or(u32::MAX), + }) +} + +/// Most frequently observed entities, with recency as the tie-breaker. +/// +/// The [`TopEntity`] view of [`top_entity_rows`], kept for callers that were +/// written against it. New callers should take the rows: this shape puts a +/// `MAX(surface)` sample in a field called `name`, and the two are not the +/// same claim. +pub fn top_entities(config: &Config, limit: usize) -> Result> { + Ok(top_entity_rows(config, None, limit)? + .into_iter() + .map(|row| TopEntity { + id: row.entity_id, + kind: row.entity_kind, + name: row.surface, + mentions: row.mentions, + }) + .collect()) +} + #[cfg(test)] #[path = "entities_tests.rs"] mod tests; diff --git a/crates/tinymemory-module/src/lib.rs b/crates/tinymemory-module/src/lib.rs index 4cd2c94e..3412cb16 100644 --- a/crates/tinymemory-module/src/lib.rs +++ b/crates/tinymemory-module/src/lib.rs @@ -248,6 +248,7 @@ mod exports { "ChunkDetail", "StorageKinds", "ChunkEmbeddings", + "CountChunks", // Retrieval. "FastRetrieve", "CoverWindow", @@ -289,6 +290,9 @@ mod exports { "Entities", "EntityEdges", "TouchEntities", + "TopEntities", + "ChunkEntities", + "EntityChunkIds", "KvGet", "KvPut", "KvDelete", @@ -317,6 +321,9 @@ mod exports { "FlushPending", "ResetDerivedIndex", "RecallNamespaceRecent", + // Tree, structural: the forest walk and its leaf edge. + "SummaryForest", + "RecentLeaves", ], signals = [], // The host's embedder is deliberately NOT declared as `requires`. That diff --git a/crates/tinymemory-module/src/service/mod.rs b/crates/tinymemory-module/src/service/mod.rs index be448492..eb4b609c 100644 --- a/crates/tinymemory-module/src/service/mod.rs +++ b/crates/tinymemory-module/src/service/mod.rs @@ -28,6 +28,7 @@ //! SeedFromAddressBook() -> AddressBookSeedOutcome //! //! ListChunks(query, scope) -> [Chunk] +//! CountChunks(query, scope) -> u64 //! GetChunk(chunk_id) -> Option //! ChunkDetail(chunk_id) -> Option //! ChunkEmbeddings(chunk_ids, model_signature) -> [ChunkEmbedding] @@ -47,6 +48,13 @@ //! RetrieveSource(query, scope) -> RetrievalResponse //! RetrieveChildren(node_id, max_depth, query, limit, scope) -> [RetrievalHit] //! RetrieveLeaves(chunk_ids, scope) -> [RetrievalHit] +//! +//! SummaryForest(limit, scope) -> SummaryForest +//! RecentLeaves(limit, scope) -> [TreeLeaf] +//! +//! TopEntities(kind, limit) -> [EntityOccurrence] +//! ChunkEntities(chunk_id) -> [EntityOccurrence] +//! EntityChunkIds(entity_id, limit) -> [String] //! ``` //! //! # Source scope crosses as an argument, never as ambient state @@ -120,9 +128,9 @@ use tinymemory_api::error::MemoryError; use tinymemory_api::goals::GoalsDoc; use tinymemory_api::health::MemoryHealth; use tinymemory_api::provider::types::{ - DiffReport, EntityHit, ExportPage, ExportRecord, FlushOutcome, ImportOutcome, IngestItem, - IngestOutcome, MaintenanceReport, QueueFailure, QueueStats, ResetOutcome, SnapshotRef, - SourceItem, SourceScope, StoreStats, + DiffReport, EntityHit, EntityOccurrence, ExportPage, ExportRecord, FlushOutcome, ImportOutcome, + IngestItem, IngestOutcome, MaintenanceReport, QueueFailure, QueueStats, ResetOutcome, + SnapshotRef, SourceItem, SourceScope, StoreStats, }; // `MemoryCore`, `MemoryRecall` and `MemoryPortability` are deliberately not // imported: they are supertraits of `MemoryProvider`, so their methods are @@ -141,7 +149,7 @@ use tinymemory_api::provider::retrieval::{ use tinymemory_api::provider::MemoryProvider; use tinymemory_api::recall::OwnedRecallOpts; use tinymemory_api::tool_memory::ToolMemoryRule; -use tinymemory_api::tree::{IngestRequest, QueryResult, TreeStatus}; +use tinymemory_api::tree::{IngestRequest, QueryResult, SummaryForest, TreeLeaf, TreeStatus}; use tinymemory_api::types::{ GraphRelationRecord, MemoryCategory, MemoryEntry, MemoryKvRecord, MemoryTaint, NamespaceDocumentInput, NamespaceMemoryHit, NamespaceRetrievalContext, NamespaceSummary, @@ -1427,6 +1435,101 @@ impl MemoryService { ensure_response_fits(&matches, "SearchEntities")?; Ok(matches) } + + /// How many chunks `ListChunks` matches, with its page bounds ignored. + /// + /// Declared here, at the end, rather than beside `ListChunks`: member order + /// is the wire order this module serves, and `tinymemory_bus::METHODS` is + /// compared against it as a sequence, so a new member is appended rather + /// than filed with its family. + /// + /// Not size-checked. The ceiling exists for responses that carry content; + /// this one is a number, and no query can make it bigger. + async fn count_chunks(&self, query: ChunkQuery, scope: Option) -> BusResult { + require_family!(self, as_chunks, Capability::Chunks) + .count_chunks(&query, scope.as_ref()) + .await + .map_err(|error| into_bus_error(&error)) + } + + /// The store-wide entity index, most-observed first — see `Entities` for + /// the namespace-scoped, hotness-ranked read these three do not replace. + /// + /// Appended here rather than filed beside `Entities` for the reason + /// `count_chunks` gives above: member order is wire order. + async fn top_entities( + &self, + kind: Option, + limit: usize, + ) -> BusResult> { + let rows = require_family!(self, as_entities, Capability::Entities) + .top_entities(kind.as_deref(), limit) + .await + .map_err(|error| into_bus_error(&error))?; + ensure_response_fits(&rows, "TopEntities")?; + Ok(rows) + } + + /// Every entity indexed against one chunk. + /// + /// Size-checked even though the contract gives it no `limit`: the bound is + /// one chunk's extraction, which is the driver's number rather than the + /// caller's, and an over-large frame the host cannot decode is a worse + /// answer than a named refusal naming the method. + async fn chunk_entities(&self, chunk_id: String) -> BusResult> { + let rows = require_family!(self, as_entities, Capability::Entities) + .chunk_entities(&chunk_id) + .await + .map_err(|error| into_bus_error(&error))?; + ensure_response_fits(&rows, "ChunkEntities")?; + Ok(rows) + } + + /// The chunks one entity was observed in, as ids. + async fn entity_chunk_ids(&self, entity_id: String, limit: usize) -> BusResult> { + let ids = require_family!(self, as_entities, Capability::Entities) + .entity_chunk_ids(&entity_id, limit) + .await + .map_err(|error| into_bus_error(&error))?; + ensure_response_fits(&ids, "EntityChunkIds")?; + Ok(ids) + } + + /// Every sealed summary in the store, with the tree each belongs to. + /// + /// Appended here rather than filed beside `DrillDown` for the reason + /// `count_chunks` gives above: member order is wire order. + /// + /// Size-checked, and it is the method most likely to hit the ceiling: the + /// caller's `limit` bounds *nodes*, not bytes, and a store of long-scoped + /// trees can put a forest-sized walk over a frame. A named refusal telling + /// the caller to lower the bound beats a frame the host cannot decode. + async fn summary_forest( + &self, + limit: usize, + scope: Option, + ) -> BusResult { + let forest = require_family!(self, as_tree, Capability::Tree) + .summary_forest(limit, scope.as_ref()) + .await + .map_err(|error| into_bus_error(&error))?; + ensure_response_fits(&forest, "SummaryForest")?; + Ok(forest) + } + + /// The newest leaves and the summaries that sealed them. + async fn recent_leaves( + &self, + limit: usize, + scope: Option, + ) -> BusResult> { + let leaves = require_family!(self, as_tree, Capability::Tree) + .recent_leaves(limit, scope.as_ref()) + .await + .map_err(|error| into_bus_error(&error))?; + ensure_response_fits(&leaves, "RecentLeaves")?; + Ok(leaves) + } } /// The response-size ceiling for a method that returns a list of entries. diff --git a/crates/tinymemory-module/tests/module_e2e.rs b/crates/tinymemory-module/tests/module_e2e.rs index 49bdab7f..371d5f44 100644 --- a/crates/tinymemory-module/tests/module_e2e.rs +++ b/crates/tinymemory-module/tests/module_e2e.rs @@ -614,6 +614,7 @@ const EXPECTED_METHODS: &[&str] = &[ "ChunkDetail", "StorageKinds", "ChunkEmbeddings", + "CountChunks", // Retrieval. "FastRetrieve", "CoverWindow", @@ -653,6 +654,9 @@ const EXPECTED_METHODS: &[&str] = &[ "Entities", "EntityEdges", "TouchEntities", + "TopEntities", + "ChunkEntities", + "EntityChunkIds", "KvGet", "KvPut", "KvDelete", @@ -681,6 +685,8 @@ const EXPECTED_METHODS: &[&str] = &[ "FlushPending", "ResetDerivedIndex", "RecallNamespaceRecent", + "SummaryForest", + "RecentLeaves", ]; #[tokio::test] @@ -1163,6 +1169,10 @@ async fn query_and_maintenance_families_dispatch_typed_requests() { portability_and_lifecycle_round_trip(&bus).await; } +#[allow( + clippy::too_many_lines, + reason = "one linear bus round trip: ingest, then every chunk read it enables, asserted in call order" +)] async fn ingest_and_chunks_round_trip(bus: &tinybus::Proxy) -> String { use tinymemory_api::chunks::DataSource; use tinymemory_api::provider::chunks::{ChunkDetail, ChunkEmbedding, ChunkQuery}; @@ -1267,6 +1277,25 @@ async fn ingest_and_chunks_round_trip(bus: &tinybus::Proxy) -> String { .call("ChunkEmbeddings", (vec![chunk_id.clone()], "test:8")) .await .expect("ChunkEmbeddings"); + // The count is asked over the wire with the same query the list used. This + // workspace holds far fewer chunks than the default page, so the two must + // agree exactly — a count answered from an unfiltered `SELECT COUNT(*)`, + // or one that let the page bounds through, would not. + let total: u64 = bus + .call( + "CountChunks", + ( + ChunkQuery::default(), + Option::::None, + ), + ) + .await + .expect("CountChunks"); + assert_eq!( + total, + chunks.len() as u64, + "CountChunks must agree with the ListChunks page it accompanies" + ); chunk_id } @@ -1411,6 +1440,31 @@ async fn tree_and_entities_round_trip(bus: &tinybus::Proxy) { bus.call::<()>("TouchEntities", ("project", vec!["person:alice"])) .await .expect("TouchEntities"); + + // The occurrence-index reads. Shape over the wire is what is under test — + // an empty index is a legitimate answer to all three — so these assert the + // decode and the argument tuples, which is what a host gets wrong. + let _: Vec = bus + .call("TopEntities", (Option::::None, 8_usize)) + .await + .expect("TopEntities"); + let _: Vec = bus + .call("ChunkEntities", ("chunk-1",)) + .await + .expect("ChunkEntities"); + let _: Vec = bus + .call("EntityChunkIds", ("person:alice", 8_usize)) + .await + .expect("EntityChunkIds"); + // A kind the extractor's vocabulary does not hold is a caller mistake, not + // an empty store: the module must refuse it rather than answer with `[]`. + let refused: Result, _> = bus + .call("TopEntities", (Some("not-a-kind".to_string()), 8_usize)) + .await; + assert!( + refused.is_err(), + "an unknown entity kind must be refused, not answered with an empty index" + ); } async fn maintenance_and_diff_round_trip(bus: &tinybus::Proxy) { diff --git a/crates/tinymemory-tinycortex/src/engine/mod.rs b/crates/tinymemory-tinycortex/src/engine/mod.rs index a05569b3..cd444d73 100644 --- a/crates/tinymemory-tinycortex/src/engine/mod.rs +++ b/crates/tinymemory-tinycortex/src/engine/mod.rs @@ -32,9 +32,9 @@ use tinymemory_api::host::{ }; use tinymemory_api::mandatory::MemoryTraitProvider; use tinymemory_api::provider::types::{ - EntityHit, EntityRef, ExportPage, ExportRecord, FlushOutcome, ImportOutcome, IngestItem, - IngestOutcome, MaintenanceReport, QueueFailure, QueueStats, ResetOutcome, SourceItem, - SourceScope, StoreStats, + EntityHit, EntityOccurrence, EntityRef, ExportPage, ExportRecord, FlushOutcome, ImportOutcome, + IngestItem, IngestOutcome, MaintenanceReport, QueueFailure, QueueStats, ResetOutcome, + SourceItem, SourceScope, StoreStats, }; // Diff-family value types, used only by the `MemoryDiff` impl below — which is // compiled out without the git-backed snapshot store. @@ -52,7 +52,9 @@ use tinymemory_api::provider::{ }; use tinymemory_api::recall::OwnedRecallOpts; use tinymemory_api::tool_memory::ToolMemoryRule; -use tinymemory_api::tree::{IngestRequest, QueryResult, TreeStatus}; +use tinymemory_api::tree::{ + IngestRequest, QueryResult, SummaryForest, TreeLeaf, TreeStatus, TreeSummary, +}; use tinymemory_api::types::{ GraphRelationRecord, MemoryCategory, MemoryEntry, MemoryKvRecord, MemoryTaint, NamespaceDocumentInput, NamespaceMemoryHit, NamespaceRetrievalContext, NamespaceSummary, @@ -907,6 +909,243 @@ impl MemoryTree for TinycortexProvider { .map_err(|error| Self::other("cascade tree", error))?; Self::cross(&status, "convert tree status") } + + async fn summary_forest( + &self, + limit: usize, + scope: Option<&SourceScope>, + ) -> Result { + /// This driver's own ceiling on one forest walk. + /// + /// The contract says the driver clamps, and this is where. It is not a + /// quota anyone should meet: it is the bound that stops a single read + /// from naming every summary in a store that has been ingesting for a + /// year, which the module would then have to refuse for overrunning a + /// frame. + const MAX_FOREST_SUMMARIES: usize = 20_000; + + /// Epoch milliseconds as the contract's timestamp. + /// + /// A row outside the representable range floors at the epoch rather + /// than failing the whole walk: one nonsense timestamp is a bad node, + /// not a bad read. + fn at_ms(ms: i64) -> chrono::DateTime { + chrono::DateTime::from_timestamp_millis(ms) + .unwrap_or(chrono::DateTime::::UNIX_EPOCH) + } + + let limit = limit.clamp(1, MAX_FOREST_SUMMARIES); + let scope = scope.cloned(); + blocking(self.config.clone(), "walk summary forest", move |config| { + tinymemory_core::store::chunks::store::with_connection(config, |conn| { + // The scope predicate applies to the *tree*, not to each + // summary: a summary's only source attribution is the tree it + // sealed into. Resolving the allowed trees first and binding + // their ids into the row query keeps the filter inside SQL and + // ahead of the LIMIT — filtering afterwards would let a + // withheld tree eat the caller's budget. + let allowed_tree_ids = match &scope { + None => None, + Some(scope) => { + // `allows_source_id` is the contract's own published + // predicate. The retrieval ranker uses a looser + // tree-scope variant that additionally accepts a bare + // `mem_src:` allow entry; this is deliberately the + // stricter of the two. Over-denying hides a node from a + // graph, which the user can see; under-denying leaks + // one, which the user cannot. + let mut stmt = conn.prepare("SELECT id, scope FROM mem_tree_trees")?; + let mut ids = Vec::new(); + let rows = stmt.query_map([], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) + })?; + for row in rows { + let (id, tree_scope) = row?; + if scope.allows_source_id(&tree_scope) { + ids.push(id); + } + } + Some(ids) + } + }; + + // An empty allowlist denies everything — `SourceScope`'s + // fail-closed rule — and so does a scope that matched no tree. + // Both are an empty forest that is *not* truncated: nothing was + // withheld by a bound, so the caller must not be told to ask + // again with a bigger one. + if allowed_tree_ids.as_ref().is_some_and(Vec::is_empty) { + return Ok(SummaryForest::default()); + } + + // One row over the limit, so truncation is observed rather than + // inferred. A store holding exactly `limit` nodes is complete, + // and `rows.len() == limit` alone cannot tell that apart from a + // store holding one more. + let probe = i64::try_from(limit.saturating_add(1)).unwrap_or(i64::MAX); + let mut sql = String::from( + "SELECT s.id, s.tree_id, s.tree_kind, t.scope, s.level, s.parent_id, + s.child_ids_json, s.time_range_start_ms, s.time_range_end_ms + FROM mem_tree_summaries s + JOIN mem_tree_trees t ON t.id = s.tree_id + WHERE s.deleted = 0", + ); + let mut bound: Vec> = Vec::new(); + if let Some(ids) = &allowed_tree_ids { + sql.push_str(" AND s.tree_id IN ("); + for (index, id) in ids.iter().enumerate() { + if index > 0 { + sql.push(','); + } + sql.push('?'); + bound.push(Box::new(id.clone())); + } + sql.push(')'); + } + // Tree-major, so a truncated walk loses whole trees off the + // tail rather than thinning every tree by a little. That is the + // honest way to cut a forest — a caller can see a source is + // missing, where it cannot see that every tree is short some + // nodes — and it is what `SummaryForest::truncated` documents. + sql.push_str(" ORDER BY s.tree_id, s.level, s.sealed_at_ms LIMIT ?"); + bound.push(Box::new(probe)); + let params = bound + .iter() + .map(|value| value.as_ref() as &dyn rusqlite::ToSql) + .collect::>(); + + let mut summaries = conn + .prepare(&sql)? + .query_map(params.as_slice(), |row| { + let child_ids_json: String = row.get(6)?; + Ok(TreeSummary { + id: row.get(0)?, + tree_id: row.get(1)?, + tree_kind: row.get(2)?, + tree_scope: row.get(3)?, + // The column is signed and the contract is not. + // The sealer never writes a negative level, so a + // corrupt row floors at zero rather than wrapping + // to four billion and sorting last. + level: u32::try_from(row.get::<_, i64>(4)?.max(0)).unwrap_or(u32::MAX), + // A node that is its tree's current root stores + // NULL; the empty string is the same state written + // by an older path. Both must read as "no parent" + // or the caller draws an edge to a node named "". + parent_id: row.get::<_, Option>(5)?.filter(|id| !id.is_empty()), + // A malformed blob is a broken row, not a broken + // read: the node still belongs in the graph, it + // just contributes no child edges. + child_ids: serde_json::from_str(&child_ids_json).unwrap_or_default(), + time_range_start: at_ms(row.get(7)?), + time_range_end: at_ms(row.get(8)?), + }) + })? + .collect::>>()?; + + let truncated = summaries.len() > limit; + summaries.truncate(limit); + Ok(SummaryForest { + summaries, + truncated, + }) + }) + .map_err(|error| anyhow::anyhow!("walk summary forest: {error}")) + }) + .await + } + + async fn recent_leaves( + &self, + limit: usize, + scope: Option<&SourceScope>, + ) -> Result, MemoryError> { + // Row selection delegates to `list_chunks` rather than a second + // hand-written `SELECT`, and that is the important part: it already + // applies the source allowlist in SQL ahead of the LIMIT, including the + // tags-aware rule that lets through content carrying no memory-source + // provenance at all. A second copy of that predicate here would be free + // to drift from the one every other chunk read goes through, and a + // source gate that drifts fails open. + let query = tinymemory_core::store::chunks::ListChunksQuery { + source_scope: scope.map(|scope| scope.allow.iter().cloned().collect::>()), + limit: Some(limit), + exclude_dropped: true, + ..Default::default() + }; + let chunks = blocking(self.config.clone(), "list recent leaves", move |config| { + tinymemory_core::store::chunks::list_chunks(config, &query) + }) + .await?; + if chunks.is_empty() { + return Ok(Vec::new()); + } + + // The parent link lives on the chunk row but not in the chunk model, so + // it is a second read keyed by the ids the first one returned. It + // cannot widen the result: every id here already passed the scope + // predicate above, and this query filters to exactly those ids. A leaf + // sealed between the two reads comes back unattached — the answer the + // first read was true for, and a state the caller already has to handle + // for content the scheduler has not reached. + let mut ids = Vec::with_capacity(chunks.len()); + for chunk in &chunks { + ids.push(chunk.id.clone()); + } + let parents = blocking(self.config.clone(), "read leaf parents", move |config| { + tinymemory_core::store::chunks::store::with_connection(config, |conn| { + // Built by hand rather than by `repeat_n(..).join(..)` so the + // bind order and the placeholder count come from one loop: + // a mismatch between them is a runtime SQL error, not a + // compile-time one. + let mut placeholders = String::with_capacity(ids.len() * 2); + for index in 0..ids.len() { + if index > 0 { + placeholders.push(','); + } + placeholders.push('?'); + } + let sql = format!( + "SELECT id, parent_summary_id FROM mem_tree_chunks WHERE id IN ({placeholders})" + ); + let params = ids + .iter() + .map(|id| id as &dyn rusqlite::ToSql) + .collect::>(); + let parents = conn + .prepare(&sql)? + .query_map(params.as_slice(), |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, Option>(1)?.filter(|id| !id.is_empty()), + )) + })? + .collect::>>>( + )?; + Ok(parents) + }) + .map_err(|error| anyhow::anyhow!("read leaf parents: {error}")) + }) + .await?; + + Ok(chunks + .into_iter() + .map(|chunk| { + let (time_range_start, time_range_end) = chunk.metadata.time_range; + TreeLeaf { + parent_summary_id: parents.get(&chunk.id).cloned().flatten(), + source_id: chunk.metadata.source_id, + // The shared helper rather than a local truncation: two + // drivers disagreeing about what a preview is shows up as a + // label that changes length when the driver changes. + preview: tinymemory_api::tree::leaf_preview(&chunk.content), + chunk_id: chunk.id, + time_range_start, + time_range_end, + } + }) + .collect()) + } } #[async_trait] @@ -1024,6 +1263,73 @@ impl MemoryEntities for TinycortexProvider { }) .await } + + async fn top_entities( + &self, + kind: Option<&str>, + limit: usize, + ) -> Result, MemoryError> { + // Parsed rather than passed through, because the column stores whatever + // string the writer used: an unrecognised filter would match no rows and + // arrive as "this store knows about nothing". Same rule and same parser + // as `MemoryRetrieval::search_entities`, and re-emitting `as_str` keeps + // the comparison against the canonical spelling the index writes. + let kind = match kind { + Some(kind) => Some( + tinymemory_core::tree::score::extract::EntityKind::parse(kind) + .map_err(|_| MemoryError::Invalid(format!("unknown entity kind: {kind}")))? + .as_str() + .to_string(), + ), + None => None, + }; + let rows = blocking(self.config.clone(), "read top entities", move |config| { + tinymemory_core::store::entities::top_entity_rows(config, kind.as_deref(), limit) + }) + .await?; + Ok(rows + .into_iter() + .map(|row| EntityOccurrence { + entity_id: row.entity_id, + kind: row.entity_kind, + surface: row.surface, + mentions: row.mentions, + }) + .collect()) + } + + async fn chunk_entities(&self, chunk_id: &str) -> Result, MemoryError> { + let node_id = chunk_id.to_string(); + let rows = blocking(self.config.clone(), "read chunk entities", move |config| { + tinymemory_core::store::entities::node_entity_rows(config, &node_id) + }) + .await?; + Ok(rows + .into_iter() + .map(|row| EntityOccurrence { + entity_id: row.entity_id, + kind: row.entity_kind, + surface: row.surface, + mentions: row.mentions, + }) + .collect()) + } + + async fn entity_chunk_ids( + &self, + entity_id: &str, + limit: usize, + ) -> Result, MemoryError> { + let entity_id = entity_id.to_string(); + blocking( + self.config.clone(), + "read entity chunk ids", + move |config| { + tinymemory_core::store::entities::entity_leaf_node_ids(config, &entity_id, limit) + }, + ) + .await + } } #[cfg(feature = "memory-git")] @@ -1965,6 +2271,44 @@ fn scope_to_engine(scope: Option<&SourceScope>) -> Option> { scope.map(|scope| scope.allow.iter().cloned().collect()) } +/// Convert a contract chunk query into the engine's. +/// +/// Shared by `list_chunks` and `count_chunks` so the two ask the engine the +/// same question. Two copies of this conversion would compile identically +/// today and drift the first time a filter is added to one of them — and the +/// symptom of that drift is a total that no amount of paging can reach. +/// +/// The page bounds are carried across unchanged; dropping them for the count +/// is the engine's job, because that is where the `LIMIT` is appended. +fn chunk_query_to_engine( + query: &ChunkQuery, + scope: Option<&SourceScope>, +) -> Result { + let ChunkQuery { + source_kind, + source_id, + owner, + since_ms, + until_ms, + limit, + offset, + exclude_dropped, + } = query.clone(); + Ok(tinymemory_core::store::chunks::ListChunksQuery { + source_kind: source_kind + .map(|kind| TinycortexProvider::cross(&kind, "convert source kind")) + .transpose()?, + source_id, + owner, + since_ms, + until_ms, + limit, + offset, + source_scope: scope_to_engine(scope), + exclude_dropped, + }) +} + #[async_trait] impl MemoryChunks for TinycortexProvider { async fn list_chunks( @@ -1972,29 +2316,7 @@ impl MemoryChunks for TinycortexProvider { query: &ChunkQuery, scope: Option<&SourceScope>, ) -> Result, MemoryError> { - let ChunkQuery { - source_kind, - source_id, - owner, - since_ms, - until_ms, - limit, - offset, - exclude_dropped, - } = query.clone(); - let engine_query = tinymemory_core::store::chunks::ListChunksQuery { - source_kind: source_kind - .map(|kind| Self::cross(&kind, "convert source kind")) - .transpose()?, - source_id, - owner, - since_ms, - until_ms, - limit, - offset, - source_scope: scope_to_engine(scope), - exclude_dropped, - }; + let engine_query = chunk_query_to_engine(query, scope)?; let chunks = blocking(self.config.clone(), "list chunks", move |config| { tinymemory_core::store::chunks::list_chunks(config, &engine_query) }) @@ -2002,6 +2324,23 @@ impl MemoryChunks for TinycortexProvider { Self::cross(&chunks, "convert chunks") } + async fn count_chunks( + &self, + query: &ChunkQuery, + scope: Option<&SourceScope>, + ) -> Result { + // Same conversion as the listing, then the engine's counting sibling, + // which builds its `WHERE` clause from the listing's own and simply + // never appends the `LIMIT`. The page bounds therefore travel and are + // ignored, rather than being cleared here where a later filter could + // be cleared with them by accident. + let engine_query = chunk_query_to_engine(query, scope)?; + blocking(self.config.clone(), "count chunks", move |config| { + tinymemory_core::store::chunks::count_chunks_matching(config, &engine_query) + }) + .await + } + async fn get_chunk(&self, chunk_id: &str) -> Result, MemoryError> { let id = chunk_id.to_string(); let chunk = blocking(self.config.clone(), "get chunk", move |config| { diff --git a/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs b/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs index b62693db..e1cfd103 100644 --- a/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs +++ b/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs @@ -1673,6 +1673,94 @@ async fn tree_entities_and_maintenance_execute_real_workspace_transitions() { .expect("query touched entity"); assert!(touched[0].hotness > 0.0); + // The occurrence-index reads. + // + // Two entities, both on `chunk-1`, both seen once. That is enough to pin + // every property the three members promise and the namespace-scoped + // `entities` above does not: no namespace argument, a count instead of a + // hotness, a surface instead of a name, and a join back to the chunk. + let store_wide = entities + .top_entities(None, 10) + .await + .expect("store-wide entity index"); + assert_eq!(store_wide.len(), 2); + // Ordered by count, and both counts are 1 — so this asserts membership, + // not position. Asserting an order the SQL breaks ties on by timestamp, + // when both rows carry the same timestamp, would be a flaky test. + let alice = store_wide + .iter() + .find(|row| row.entity_id == "person:alice") + .expect("the seeded person is in the index"); + assert_eq!(alice.kind, "person"); + assert_eq!(alice.surface, "Alice"); + assert_eq!(alice.mentions, 1); + + let people_only = entities + .top_entities(Some("person"), 10) + .await + .expect("kind-filtered entity index"); + assert_eq!(people_only.len(), 1); + assert_eq!(people_only[0].entity_id, "person:alice"); + + // The filter is validated, not applied blindly: an unknown kind that + // matched nothing would read as an empty store. + assert!( + matches!( + entities.top_entities(Some("not-a-kind"), 10).await, + Err(tinymemory_api::error::MemoryError::Invalid(_)) + ), + "an unrecognised kind must be refused rather than answered with []" + ); + + let of_chunk = entities + .chunk_entities("chunk-1") + .await + .expect("entities of one chunk"); + assert_eq!(of_chunk.len(), 2); + // Equal counts break by entity id ascending, which is deterministic here. + assert_eq!(of_chunk[0].entity_id, "organization:tinymemory"); + assert_eq!(of_chunk[0].surface, "TinyMemory"); + assert!(entities + .chunk_entities("no-such-chunk") + .await + .expect("an unknown chunk is not an error") + .is_empty()); + + let of_entity = entities + .entity_chunk_ids("person:alice", 10) + .await + .expect("chunks of one entity"); + assert_eq!(of_entity, vec!["chunk-1".to_string()]); + assert!(entities + .entity_chunk_ids("person:nobody", 10) + .await + .expect("an unknown entity is not an error") + .is_empty()); + + // Summary nodes live in the same index and are not chunks. Indexing the + // same entity against one must not add its node id to the chunk list — + // a caller filtering a chunk list by these ids would find nothing behind + // it. + assert_eq!( + tinymemory_core::store::entities::index_entities( + &config, + &indexed[..1], + "summary-1", + "summary", + 1_700_000_100_000, + Some("project"), + ) + .expect("seed a summary-node occurrence"), + 1 + ); + assert_eq!( + entities + .entity_chunk_ids("person:alice", 10) + .await + .expect("chunks of one entity, with a summary node indexed"), + vec!["chunk-1".to_string()], + ); + let maintenance = provider.as_maintenance().expect("Maintenance"); let reembed = maintenance.reembed().await.expect("reembed"); assert_eq!(reembed.operation, "reembed"); @@ -1758,3 +1846,480 @@ async fn diff_captures_and_compares_real_source_snapshots() { assert_eq!(report.changes[0].item_id, "item-1"); assert_eq!(report.changes[0].kind, ChangeKind::Modified); } + +/// The count answers the same question the page does. +/// +/// `count_chunks` exists because a caller rendering "20 of 431" cannot derive +/// 431 from a page, and the only host-side way to get it — list everything and +/// measure — is the unbounded query the row limit exists to prevent. So the +/// total is computed where the `WHERE` clause is, and what has to be pinned is +/// that it is computed from *that* `WHERE` clause. +/// +/// Three failure shapes are each asserted apart, because the plausible wrong +/// implementations differ: +/// +/// - a count wired to the engine's unfiltered `count_chunks` returns the whole +/// table and looks right until a filter is applied, so the filtered count is +/// required to be strictly smaller than the unfiltered one; +/// - a count that reuses the listing's SQL wholesale keeps its `LIMIT`, so the +/// same query paged one row at a time must not move it; +/// - a count that skips the scope clause reports rows a scoped caller is not +/// allowed to see, which is the source gate failing open in a number. +/// +/// The rows are seeded through the store rather than the ingest pipeline: the +/// point here is which rows a predicate matches, and the pipeline decides +/// chunk boundaries, which would make the expected totals a property of the +/// chunker instead of the filter. +#[tokio::test(flavor = "multi_thread")] +async fn the_chunk_count_matches_the_page_and_ignores_its_bounds() { + use tinymemory_api::chunks::{Chunk, Metadata, SourceKind}; + use tinymemory_api::provider::types::SourceScope; + use tinymemory_api::provider::{ChunkQuery, MemoryProvider}; + + let workspace = tempfile::tempdir().expect("workspace"); + let provider = provider_over(workspace.path()); + let config = provider_config(workspace.path(), serde_json::Value::Null); + let timestamp = chrono::DateTime::from_timestamp(1_700_000_000, 0).expect("timestamp"); + let seed = |id: &str, kind: SourceKind, source_id: &str, tags: Vec, seq: u32| Chunk { + id: id.into(), + content: format!("Body of {id}."), + metadata: Metadata { + tags, + ..Metadata::point_in_time(kind, source_id, "owner", timestamp) + }, + token_count: 3, + seq_in_source: seq, + created_at: timestamp, + partial_message: false, + }; + // Four documents and one chat, so the kind filter has something to drop; + // one of the documents is source-attributed, so the scope does too. + let seeded = vec![ + seed("count-doc-0", SourceKind::Document, "doc-source", vec![], 0), + seed("count-doc-1", SourceKind::Document, "doc-source", vec![], 1), + seed("count-doc-2", SourceKind::Document, "doc-source", vec![], 2), + seed( + "count-doc-scoped", + SourceKind::Document, + "mem_src:src-a:item-1", + vec!["memory_sources".into()], + 0, + ), + seed("count-chat-0", SourceKind::Chat, "chat-source", vec![], 0), + ]; + assert_eq!( + tinymemory_core::store::chunks::store::upsert_chunks(&config, &seeded) + .expect("seed chunks"), + seeded.len(), + ); + + let chunks = provider.as_chunks().expect("Chunks"); + // A limit well above the seeded rows: the listing this is compared against + // must not be the truncated one, or the equality would hold for the wrong + // reason. + let documents = ChunkQuery { + source_kind: Some(SourceKind::Document), + limit: Some(1_000), + ..ChunkQuery::default() + }; + let listed = chunks + .list_chunks(&documents, None) + .await + .expect("list documents"); + let counted = chunks + .count_chunks(&documents, None) + .await + .expect("count documents"); + assert_eq!( + listed.len(), + 4, + "four of the five seeded rows are documents" + ); + assert_eq!(counted as usize, listed.len()); + + let everything = ChunkQuery { + limit: Some(1_000), + ..ChunkQuery::default() + }; + let all = chunks + .count_chunks(&everything, None) + .await + .expect("count everything"); + assert_eq!(all as usize, seeded.len()); + assert!( + counted < all, + "the filter must reach the count; an unfiltered count would report {all} for both" + ); + + // Same predicate, one row at a time: the page moves, the total does not. + let second_page = ChunkQuery { + limit: Some(1), + offset: Some(2), + ..documents.clone() + }; + assert_eq!( + chunks + .list_chunks(&second_page, None) + .await + .expect("list one row") + .len(), + 1 + ); + assert_eq!( + chunks + .count_chunks(&second_page, None) + .await + .expect("count with page bounds"), + counted, + "limit and offset must not change what the count reports" + ); + + // The scope is applied by both, identically. `src-b` allows nothing that + // was ingested under `src-a`, and the unattributed rows stay visible — + // the engine's fail-closed rule, which the count has to share or it + // reports rows the caller may not see. + let scope = SourceScope::new(["src-b"]); + let scoped = chunks + .list_chunks(&documents, Some(&scope)) + .await + .expect("list scoped documents"); + assert_eq!(scoped.len(), 3, "the source-attributed row is out of scope"); + assert_eq!( + chunks + .count_chunks(&documents, Some(&scope)) + .await + .expect("count scoped documents") as usize, + scoped.len() + ); + + // No match is a zero, not an error. + let unmatched = ChunkQuery { + source_id: Some("no-such-source".into()), + ..ChunkQuery::default() + }; + assert!(chunks + .list_chunks(&unmatched, None) + .await + .expect("list nothing") + .is_empty()); + assert_eq!( + chunks + .count_chunks(&unmatched, None) + .await + .expect("count nothing"), + 0 + ); +} + +/// The forest walk and its leaf edge — the two structural tree reads. +/// +/// Nothing here can pass vacuously. Both members default to +/// `MemoryError::Unsupported` on the trait, so every `expect` below is a +/// claim about this engine rather than about the contract's fallback, and a +/// build that dropped either implementation fails on the first call. +/// +/// What is pinned: the owning tree's kind and scope arrive denormalised onto +/// each node; the parent link survives (it is the edge a caller draws a graph +/// from, and the one thing `retrieve_children` does not carry); the source +/// allowlist is applied to trees *and* to leaves; an empty allowlist denies +/// rather than waves through; a bound reports itself as `truncated` instead of +/// erroring or lying; a tombstoned summary is invisible; and a leaf carries the +/// summary that sealed it plus a preview capped at `LEAF_PREVIEW_CHARS`. +#[tokio::test(flavor = "multi_thread")] +async fn the_summary_forest_and_its_leaves_read_through_the_contract() { + use chrono::{TimeZone, Utc}; + use tinymemory_api::chunks::{chunk_id, Chunk, Metadata, SourceKind}; + use tinymemory_api::provider::types::SourceScope; + use tinymemory_api::provider::MemoryProvider; + use tinymemory_api::tree::LEAF_PREVIEW_CHARS; + use tinymemory_core::store::chunks::store::{upsert_chunks, with_connection}; + use tinymemory_core::store::trees::store::{insert_summary_tx, insert_tree}; + use tinymemory_core::store::trees::{SummaryNode, Tree, TreeKind, TreeStatus as TreeActivity}; + + const BASE_MS: i64 = 1_700_000_000_000; + + let workspace = tempfile::tempdir().expect("workspace"); + let provider = provider_over(workspace.path()); + let config = provider_config(workspace.path(), serde_json::Value::Null); + + let at = |offset_ms: i64| { + Utc.timestamp_millis_opt(BASE_MS + offset_ms) + .single() + .expect("timestamp") + }; + + // Two source trees. Scoping is only testable with more than one, and the + // whole point of the forest walk is that it spans them. + for (id, scope) in [("tree-alpha", "src-alpha"), ("tree-beta", "src-beta")] { + insert_tree( + &config, + &Tree { + id: id.into(), + kind: TreeKind::Source, + scope: scope.into(), + root_id: None, + max_level: 2, + ask: None, + status: TreeActivity::Active, + created_at: at(0), + last_sealed_at: Some(at(0)), + }, + ) + .expect("insert tree"); + } + + // Leaves. The `memory_sources` tag is what makes a chunk source-attributed + // — without it the allowlist lets the row through by design — so the two + // scoped leaves carry it and the assertions below are about the predicate + // rather than about an exemption from it. + let leaves = [ + ( + "src-alpha", + 0u32, + "Alpha leaf one\nand a second line nobody labels with", + ), + ("src-alpha", 1, "Alpha leaf two"), + ("src-beta", 0, "Beta leaf one"), + ] + .into_iter() + .enumerate() + .map(|(index, (source, seq, content))| { + let ts = at(i64::try_from(index).unwrap_or(0) * 1_000); + Chunk { + id: chunk_id(SourceKind::Chat, source, seq, content), + content: content.to_string(), + metadata: Metadata { + source_kind: SourceKind::Chat, + source_id: source.into(), + owner: "owner".into(), + timestamp: ts, + time_range: (ts, ts), + tags: vec!["memory_sources".into()], + source_ref: None, + path_scope: None, + }, + token_count: 8, + seq_in_source: seq, + created_at: ts, + partial_message: false, + } + }) + .collect::>(); + assert_eq!( + upsert_chunks(&config, &leaves).expect("persist leaves"), + leaves.len() + ); + + // Four summaries: an L1 and its L2 parent in alpha, an L1 in beta, and a + // tombstone that must never surface. + let summary = |id: &str, + tree_id: &str, + level: u32, + parent: Option<&str>, + children: Vec, + deleted: bool| SummaryNode { + id: id.into(), + tree_id: tree_id.into(), + tree_kind: TreeKind::Source, + level, + parent_id: parent.map(str::to_string), + child_ids: children, + content: format!("seal of {id}"), + token_count: 32, + entities: Vec::new(), + topics: Vec::new(), + time_range_start: at(0), + time_range_end: at(2_000), + score: 0.5, + sealed_at: at(i64::from(level) * 10), + deleted, + embedding: None, + doc_id: None, + version_ms: None, + }; + let alpha_leaf_ids = leaves[..2] + .iter() + .map(|chunk| chunk.id.clone()) + .collect::>(); + let seeded = [ + summary( + "s-alpha-1", + "tree-alpha", + 1, + Some("s-alpha-2"), + alpha_leaf_ids.clone(), + false, + ), + summary( + "s-alpha-2", + "tree-alpha", + 2, + None, + vec!["s-alpha-1".into()], + false, + ), + summary( + "s-beta-1", + "tree-beta", + 1, + None, + vec![leaves[2].id.clone()], + false, + ), + summary("s-beta-gone", "tree-beta", 1, None, Vec::new(), true), + ]; + with_connection(&config, |conn| { + let tx = conn.unchecked_transaction()?; + for node in &seeded { + insert_summary_tx(&tx, node, None, "test")?; + } + // The seal writes this column when it claims a leaf; written directly + // here because running the real sealer needs a summarisation model, and + // what is under test is the read, not the summariser. + for id in &alpha_leaf_ids { + tx.execute( + "UPDATE mem_tree_chunks SET parent_summary_id = ?1 WHERE id = ?2", + rusqlite::params!["s-alpha-1", id], + )?; + } + tx.commit()?; + Ok(()) + }) + .expect("seed summaries and their leaf claims"); + + let tree = provider.as_tree().expect("Tree"); + + // ── The whole forest ──────────────────────────────────────────────────── + let forest = tree + .summary_forest(100, None) + .await + .expect("walk the forest"); + assert!( + !forest.truncated, + "a walk that reached the end of the store is not truncated" + ); + assert_eq!( + forest.summaries.len(), + 3, + "the tombstoned summary is not a node a caller has to know about" + ); + let alpha_1 = forest + .summaries + .iter() + .find(|node| node.id == "s-alpha-1") + .expect("the L1 node is in the walk"); + assert_eq!(alpha_1.tree_id, "tree-alpha"); + assert_eq!( + alpha_1.tree_scope, "src-alpha", + "the tree's scope is denormalised onto the node; a caller that had to \ + join for it would be reading the driver's tables again" + ); + assert_eq!(alpha_1.tree_kind, "source"); + assert_eq!(alpha_1.level, 1); + assert_eq!( + alpha_1.parent_id.as_deref(), + Some("s-alpha-2"), + "the parent link is the edge; without it this is a list, not a graph" + ); + assert_eq!(alpha_1.child_ids, alpha_leaf_ids); + assert_eq!(alpha_1.time_range_start, at(0)); + assert_eq!(alpha_1.time_range_end, at(2_000)); + assert!( + forest.summaries.iter().all(|node| node.id != "s-beta-gone"), + "a tombstone must not reach the caller" + ); + + // ── A bound reports itself ────────────────────────────────────────────── + let clipped = tree + .summary_forest(1, None) + .await + .expect("walk one node of the forest"); + assert_eq!(clipped.summaries.len(), 1); + assert!( + clipped.truncated, + "a walk stopped by the bound says so, rather than reading as a store \ + with one summary in it" + ); + + // ── Scope is a predicate, on trees ────────────────────────────────────── + let alpha_only = tree + .summary_forest(100, Some(&SourceScope::new(["src-alpha"]))) + .await + .expect("scoped walk"); + assert_eq!(alpha_only.summaries.len(), 2); + assert!( + alpha_only + .summaries + .iter() + .all(|node| node.tree_id == "tree-alpha"), + "a scoped walk answers for the allowed trees only" + ); + + let denied = tree + .summary_forest(100, Some(&SourceScope::default())) + .await + .expect("an empty allowlist is an answer, not an error"); + assert!( + denied.summaries.is_empty(), + "an empty allowlist denies everything — it is not 'unrestricted'" + ); + assert!( + !denied.truncated, + "nothing was withheld by a bound, so asking again with a bigger one \ + would change nothing" + ); + + // ── The leaf edge ─────────────────────────────────────────────────────── + let recent = tree.recent_leaves(100, None).await.expect("recent leaves"); + assert_eq!(recent.len(), 3); + assert!( + recent + .windows(2) + .all(|pair| pair[0].time_range_start >= pair[1].time_range_start), + "newest first, as the member promises" + ); + let claimed = recent + .iter() + .find(|leaf| leaf.chunk_id == alpha_leaf_ids[0]) + .expect("the first alpha leaf is in the page"); + assert_eq!( + claimed.parent_summary_id.as_deref(), + Some("s-alpha-1"), + "the summary that sealed a leaf is the fact `list_chunks` cannot report" + ); + assert_eq!(claimed.source_id, "src-alpha"); + assert_eq!( + claimed.preview, "Alpha leaf one", + "the preview is the first line, not the whole body" + ); + assert!(recent + .iter() + .all(|leaf| leaf.preview.chars().count() <= LEAF_PREVIEW_CHARS)); + let unclaimed = recent + .iter() + .find(|leaf| leaf.chunk_id == leaves[2].id) + .expect("the beta leaf is in the page"); + assert!( + unclaimed.parent_summary_id.is_none(), + "a leaf nothing has sealed is unattached, which is a state and not a \ + fault" + ); + + // ── Scope is a predicate, on leaves too ───────────────────────────────── + let scoped_leaves = tree + .recent_leaves(100, Some(&SourceScope::new(["src-alpha"]))) + .await + .expect("scoped leaves"); + assert_eq!(scoped_leaves.len(), 2); + assert!( + scoped_leaves + .iter() + .all(|leaf| leaf.source_id == "src-alpha"), + "the allowlist is applied inside the query, not after the limit" + ); + assert!(tree + .recent_leaves(100, Some(&SourceScope::default())) + .await + .expect("an empty allowlist is an answer here too") + .is_empty()); +} diff --git a/vendor/tinycortex b/vendor/tinycortex index 8401346b..928843f5 160000 --- a/vendor/tinycortex +++ b/vendor/tinycortex @@ -1 +1 @@ -Subproject commit 8401346b574cacb1dc0cf6b36bc608ff5ef9f6f5 +Subproject commit 928843f594c449451b575b69ebb6c385f03a6f79 From e46ffa21635f9cb0e24ab8b433c5f9ff87d5b805 Mon Sep 17 00:00:00 2001 From: Shanu Date: Tue, 25 Aug 2026 00:07:52 +0530 Subject: [PATCH 2/6] Take tinycortex's drift fix, and stop naming a link rustdoc resolves itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gitlink moves onto tinycortex#157's branch, which now also carries the clippy drift `stable` picked up since that repo's last green run — five `chunks_exact` with a constant size plus three smaller lints, none of them in code either branch wrote. `chunks.rs`'s module doc spelled out a link target rustdoc already resolves from the path, which `-D warnings` treats as an error. Co-Authored-By: Claude Opus 5 --- crates/tinymemory-api/src/provider/chunks.rs | 2 +- vendor/tinycortex | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/tinymemory-api/src/provider/chunks.rs b/crates/tinymemory-api/src/provider/chunks.rs index de1bb049..d27ee119 100644 --- a/crates/tinymemory-api/src/provider/chunks.rs +++ b/crates/tinymemory-api/src/provider/chunks.rs @@ -1,6 +1,6 @@ //! The chunks family: direct read access to the stored chunk tier. //! -//! A driver advertising [`Capability::Chunks`](crate::capabilities::Capability::Chunks) +//! A driver advertising [`Capability::Chunks`] //! can list and fetch individual chunks, and hand back the embedding vectors it //! holds for them. //! diff --git a/vendor/tinycortex b/vendor/tinycortex index 928843f5..3679a92c 160000 --- a/vendor/tinycortex +++ b/vendor/tinycortex @@ -1 +1 @@ -Subproject commit 928843f594c449451b575b69ebb6c385f03a6f79 +Subproject commit 3679a92c03a674c11d9b76d89db46fadf6daed22 From f3c7d97ca44edcc6a7c58a0f5fb839788d764db2 Mon Sep 17 00:00:00 2001 From: Shanu Date: Tue, 25 Aug 2026 00:56:36 +0530 Subject: [PATCH 3/6] feat(bus): carry mail headers across the contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ingest_email` rendered every stored thread without a `To:` or `Cc:` line, repeated the thread's subject on every message, and dropped `List-Unsubscribe` entirely, because `IngestItem` had nowhere to put them. The driver said so in a comment and accepted the loss: those are display headers, and the bodies — what retrieval matches on — crossed intact. `List-Unsubscribe` is not a display header. It is the input an unsubscribe flow reads back out of stored mail, so dropping it does not make that flow less pretty, it makes it impossible. A host with mail ingest cannot move onto the contract while the contract cannot carry it. So widen `IngestItem` with `to`, `cc`, `subject` and `list_unsubscribe`. All four are `#[serde(default)]` and skipped when empty, which is the version rule's "new optional wire field" — minor-safe on its own, and here riding on the major bump this branch already carries. `subject` is optional rather than required because a reply keeps the thread's subject and only a renamed thread differs; absent means the thread subject stands, so the field being new leaves no mail unlabelled. The assertions go at both boundaries deliberately. The conformance test proves the driver's mapping; the loader E2E proves the same headers survive a real serialize/deserialize through the dlopen'd cdylib. Only the second would catch a shape that encoded them and then lost them in transit — and that failure is invisible from `written > 0`, which a dropped-header ingest still answers happily. The `vendor/tinycortex` pin also advances one commit, from `3679a92` to `2e7b57e`. Both are on the same in-flight tinycortex branch; this only catches the pin up to that branch's head so CI here builds what that PR actually proposes. The pin moves to tinycortex `main` once that merges. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HZ3Y9zHmLQL313GAq7Jm5J --- crates/tinymemory-api/src/null_tests.rs | 4 + crates/tinymemory-bus/src/provider/types.rs | 24 ++++++ crates/tinymemory-documents/src/ingest/mod.rs | 4 + crates/tinymemory-module/tests/module_e2e.rs | 35 +++++++++ .../tinymemory-tinycortex/src/engine/mod.rs | 24 +++--- .../tinymemory-tinycortex/src/engine/test.rs | 4 + .../tests/full_provider_conformance.rs | 78 +++++++++++++++++++ vendor/tinycortex | 2 +- 8 files changed, 160 insertions(+), 15 deletions(-) diff --git a/crates/tinymemory-api/src/null_tests.rs b/crates/tinymemory-api/src/null_tests.rs index 9ae79428..36d2621a 100644 --- a/crates/tinymemory-api/src/null_tests.rs +++ b/crates/tinymemory-api/src/null_tests.rs @@ -254,6 +254,10 @@ fn every_optional_method_fails_with_its_advertised_family_name() { author: None, channel_label: None, platform: None, + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, }; assert_unsupported(block_on(driver.ingest_document(ingest)), Capability::Ingest); diff --git a/crates/tinymemory-bus/src/provider/types.rs b/crates/tinymemory-bus/src/provider/types.rs index a5d17c4e..ccd9c8ac 100644 --- a/crates/tinymemory-bus/src/provider/types.rs +++ b/crates/tinymemory-bus/src/provider/types.rs @@ -160,6 +160,30 @@ pub struct IngestItem { /// name, which is right for every new caller. #[serde(default, skip_serializing_if = "Option::is_none")] pub platform: Option, + /// Recipients, for a mail item. Rendered as the `To:` line. + /// + /// Empty for every non-mail source, which is why this is a plain `Vec` + /// rather than an `Option` — "no recipients" and "not mail" are the same + /// statement to every reader of it. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub to: Vec, + /// Carbon copies, rendered as the `Cc:` line. Empty as above. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub cc: Vec, + /// This message's own subject, when it differs from the thread's. + /// + /// Absent means the thread subject stands, which is the common case: a + /// reply carries the thread's subject and only a renamed thread differs. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub subject: Option, + /// The `List-Unsubscribe` header, verbatim. + /// + /// Not decoration: it is the input an unsubscribe flow reads back out of + /// stored mail, so a pipeline that drops it makes that flow impossible + /// rather than merely less pretty. Absent for mail that carries no such + /// header, and for everything that is not mail. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub list_unsubscribe: Option, /// Provenance taint. The **host** stamps this; a driver must persist what it /// is given and must never assign or upgrade it. #[serde(default)] diff --git a/crates/tinymemory-documents/src/ingest/mod.rs b/crates/tinymemory-documents/src/ingest/mod.rs index 2d511ca4..2e5d8e89 100644 --- a/crates/tinymemory-documents/src/ingest/mod.rs +++ b/crates/tinymemory-documents/src/ingest/mod.rs @@ -136,6 +136,10 @@ impl<'a> DocumentIntake<'a> { author: None, channel_label: None, platform: None, + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, }; let outcome = ingest.ingest_document(item).await?; Ok(IntakeReceipt { diff --git a/crates/tinymemory-module/tests/module_e2e.rs b/crates/tinymemory-module/tests/module_e2e.rs index 371d5f44..573e6678 100644 --- a/crates/tinymemory-module/tests/module_e2e.rs +++ b/crates/tinymemory-module/tests/module_e2e.rs @@ -1193,6 +1193,10 @@ async fn ingest_and_chunks_round_trip(bus: &tinybus::Proxy) -> String { author: None, channel_label: None, platform: None, + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, }; let outcome: IngestOutcome = bus .call("IngestDocument", (ingest.clone(),)) @@ -1242,6 +1246,10 @@ async fn ingest_and_chunks_round_trip(bus: &tinybus::Proxy) -> String { author: Some("alice@example.com".into()), channel_label: Some("Adapter ship date".into()), platform: None, + to: vec!["carol@example.com".into()], + cc: vec!["dave@example.com".into()], + subject: Some("Re: adapter, renamed".into()), + list_unsubscribe: Some("".into()), }],), ) .await @@ -1251,6 +1259,29 @@ async fn ingest_and_chunks_round_trip(bus: &tinybus::Proxy) -> String { "the mail path must reach the pipeline, not just route" ); + // The mail headers, asserted after a real serialize/deserialize across the + // loaded module rather than only against the driver. `List-Unsubscribe` is + // the one that matters most: it is the input an unsubscribe flow reads back + // out of stored mail, so a shape that dropped it in transit would still + // answer `written > 0` above and look like a healthy ingest. + let stored: Option = bus + .call("GetChunk", (mail.ids[0].clone(),)) + .await + .expect("GetChunk for the stored mail"); + let stored = stored.expect("the id `IngestEmail` reported must resolve"); + for header in [ + "To: carol@example.com", + "Cc: dave@example.com", + "Subject: Re: adapter, renamed", + "List-Unsubscribe: ", + ] { + assert!( + stored.content.contains(header), + "`{header}` must survive the crossing: {}", + stored.content + ); + } + let chunks: Vec = bus .call( "ListChunks", @@ -1504,6 +1535,10 @@ async fn maintenance_and_diff_round_trip(bus: &tinybus::Proxy) { author: None, channel_label: None, platform: None, + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, }; let _: IngestOutcome = bus .call("IngestDocument", (changed,)) diff --git a/crates/tinymemory-tinycortex/src/engine/mod.rs b/crates/tinymemory-tinycortex/src/engine/mod.rs index cd444d73..9631a384 100644 --- a/crates/tinymemory-tinycortex/src/engine/mod.rs +++ b/crates/tinymemory-tinycortex/src/engine/mod.rs @@ -588,23 +588,19 @@ impl MemoryIngest for TinycortexProvider { // Same rule as the chat mapping: the speaking role when // the caller distinguishes it, the owner otherwise. from: item.author.unwrap_or(item.owner), - // `IngestItem` carries no recipient list and no - // per-message subject, so the rendered thread has no - // `To:`/`Cc:` lines and every message repeats the - // thread's subject. Those are display headers in the - // canonical markdown; the bodies, their order and their - // timestamps — what retrieval actually matches on — - // cross intact. Carrying them would mean widening - // `IngestItem`, whose literals are exhaustive at every - // construction site in every host, for fields only this - // path reads. - to: Vec::new(), - cc: Vec::new(), - subject: thread_subject.clone(), + to: item.to, + cc: item.cc, + // A reply carries the thread's subject; only a renamed + // thread differs, which is what the per-item field is + // for. + subject: item.subject.unwrap_or_else(|| thread_subject.clone()), sent_at: item.timestamp.unwrap_or_else(Utc::now), body: item.content, source_ref: item.source_ref.map(|source_ref| source_ref.value), - list_unsubscribe: None, + // Carried verbatim: an unsubscribe flow reads this back + // out of stored mail, so dropping it makes that flow + // impossible rather than merely less complete. + list_unsubscribe: item.list_unsubscribe, }, ) .collect(), diff --git a/crates/tinymemory-tinycortex/src/engine/test.rs b/crates/tinymemory-tinycortex/src/engine/test.rs index 7d27a3c8..c62aed45 100644 --- a/crates/tinymemory-tinycortex/src/engine/test.rs +++ b/crates/tinymemory-tinycortex/src/engine/test.rs @@ -43,6 +43,10 @@ fn ingest_item(content: &str, mime: Option<&str>, taint: MemoryTaint) -> IngestI author: None, channel_label: None, platform: None, + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, } } diff --git a/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs b/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs index e1cfd103..94cb4d14 100644 --- a/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs +++ b/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs @@ -183,6 +183,10 @@ async fn maintenance_diagnostics_read_the_store_rather_than_their_defaults() { author: None, channel_label: None, platform: None, + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, }) .await .expect("ingest a document"); @@ -1061,6 +1065,10 @@ async fn ingest_chunks_and_retrieval_cover_success_and_validation_without_networ author: None, channel_label: None, platform: None, + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, }; assert!(matches!( ingest.ingest_document(invalid).await, @@ -1091,6 +1099,10 @@ async fn ingest_chunks_and_retrieval_cover_success_and_validation_without_networ author: None, channel_label: None, platform: None, + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, }) .await .expect("successful deterministic ingest"); @@ -1118,6 +1130,10 @@ async fn ingest_chunks_and_retrieval_cover_success_and_validation_without_networ author: Some("assistant".into()), channel_label: Some("Agent session #1".into()), platform: Some("agent".into()), + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, }]) .await .expect("successful chat ingest"); @@ -1314,6 +1330,10 @@ async fn a_repeated_source_reports_its_gate_rather_than_a_dropped_chunk() { author: None, channel_label: None, platform: None, + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, }; let first = ingest @@ -1391,6 +1411,10 @@ async fn an_email_thread_keeps_its_per_message_headers() { author: Some(author.into()), channel_label: Some("Adapter ship date".into()), platform: None, + to: Vec::new(), + cc: Vec::new(), + subject: None, + list_unsubscribe: None, }; let outcome = ingest @@ -1405,6 +1429,17 @@ async fn an_email_thread_keeps_its_per_message_headers() { "The review is done, so Thursday holds.", 1_700_000_100, ), + IngestItem { + to: vec!["carol@example.com".into()], + cc: vec!["dave@example.com".into()], + subject: Some("Re: adapter, renamed".into()), + list_unsubscribe: Some("".into()), + ..message( + "carol@example.com", + "Renaming the thread so the ship date is findable.", + 1_700_000_200, + ) + }, ]) .await .expect("email ingest"); @@ -1426,6 +1461,49 @@ async fn an_email_thread_keeps_its_per_message_headers() { "the sender header is what a per-message citation resolves against: {}", stored.content ); + + // The headers the third message carries are not decoration. `To:`/`Cc:` + // are what "who else saw this" resolves against, a per-message `Subject:` + // is how a renamed thread stays findable under its new name, and + // `List-Unsubscribe:` is the input an unsubscribe flow reads back out of + // stored mail — a pipeline that drops it makes that flow impossible, not + // merely less complete. So this asserts they survive the crossing rather + // than trusting that they do. + let thread = stored_thread_text(&provider, &outcome.ids).await; + for header in [ + "To: carol@example.com", + "Cc: dave@example.com", + "Subject: Re: adapter, renamed", + "List-Unsubscribe: ", + ] { + assert!( + thread.contains(header), + "`{header}` must survive the crossing: {thread}" + ); + } + // And a message that names no subject of its own still inherits the + // thread's, so the field being optional does not leave mail unlabelled. + assert!( + thread.contains("Subject: Adapter ship date"), + "a message with no subject of its own keeps the thread's: {thread}" + ); +} + +/// Every chunk the outcome named, concatenated, so a header assertion does not +/// depend on which chunk the splitter happened to put it in. +async fn stored_thread_text( + provider: &impl tinymemory_api::provider::MemoryProvider, + ids: &[String], +) -> String { + let chunks = provider.as_chunks().expect("Chunks"); + let mut text = String::new(); + for id in ids { + if let Some(chunk) = chunks.get_chunk(id).await.expect("read stored chunk") { + text.push_str(&chunk.content); + text.push('\n'); + } + } + text } /// Flushing twice inside one window schedules the work once, and says so. diff --git a/vendor/tinycortex b/vendor/tinycortex index 3679a92c..2e7b57ee 160000 --- a/vendor/tinycortex +++ b/vendor/tinycortex @@ -1 +1 @@ -Subproject commit 3679a92c03a674c11d9b76d89db46fadf6daed22 +Subproject commit 2e7b57eea7188a9c96e7baee0a0d2cfb6c8f6b7c From 77e971005a0bdac7f7be272a3fd5891ce2ac3a51 Mon Sep 17 00:00:00 2001 From: Shanu Date: Tue, 25 Aug 2026 03:45:04 +0530 Subject: [PATCH 4/6] Close the remaining contract, registration and runtime gaps for openhuman#5560 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The host still links the memory engine directly because five of its own reads and writes have no member on the bus, and because two engine-owned runtime behaviours have no owner once the host's in-process engine is deleted. Both halves are here. ## Contract `ChunkQuery` grows six filters — `ids`, `source_kinds`, `source_ids`, `entity_ids`, `entity_kinds` and `content_contains` — every one `#[serde(default, skip_serializing_if = ...)]`, so an older peer on either side of the boundary neither sends nor misses them. Four trait members join them, all defaulted to `Unsupported` so no existing driver stops compiling: `MemoryChunks::{list_chunk_details, source_totals}`, `MemorySourceSink::forget_matching` and `MemoryMaintenance::purge_all`. `ChunkListRow` is deliberately `ChunkDetail` without its body. `ChunkDetail`'s docs promise that a `None` body means the vault read failed, and a listing can only honour that promise by reading every file or by lying, so it does neither. `MemoryEntities::chunk_entities` is amended rather than added: it now takes a batch of chunk ids and an optional kind filter, and answers `ChunkEntityOccurrence` rows that each name their own chunk. Widening it is legitimate only because it has never shipped — it was added on this branch, so no released host calls the single-id form — and its position in the member sequence is unchanged, which is what the drift assertion pins. The four new methods are registered in all four places that have to agree (interface impl, module manifest, `tinymemory_bus::METHODS`, and the E2E's expected set), in declaration order, because the drift test compares sequences rather than sets. ## Runtime The module now starts the engine's queue worker pool itself. Until now the only `queue::start` in any tree was the host's, against the second in-process engine the host also booted — so a host that deletes that engine, which is the entire point of loading this module, silently turns `flush_pending`, `retry_failed`, the re-embed backfill and the ingest path's `extract_chunk` into no-ops. Ingestion keeps reporting success and the content is simply never indexed. Two degradations come with it and are documented rather than hidden: the pool runs unthrottled, because the scheduler gate is a host seam no bus interface serves, and its shutdown hook is dropped, so locks held at exit are reclaimed by lease expiry on the next start instead of released cleanly. `claim_queue_pool` exists because `queue::start`'s `Once` is process-global while the pool is bound to one workspace: a second workspace in one process is not a second pool, it is a store with nothing draining its queue, and that now logs as an error instead of being invisible. `tinycortex/contacts` replaces `tinycortex/people` on the module's dependency, since upstream declares `contacts = ["people", ...]`. Without it `SeedFromAddressBook` compiles down to the stub that returns an empty contact list, so a refresh reports success and imports nobody — the failure mode that looks like an empty address book rather than a missing feature. The four objc2 crates behind the gate sit under a macOS target table upstream, so Linux and Windows artifacts still compile none of them. The E2E's `SeedFromAddressBook` assertion accepts a platform permission denial: with `contacts` on, that call reaches the real address book, and on macOS that is a per-application privacy grant a test runner may not hold. A denial is the address book answering, so what is asserted is that the call routes and returns under a contract error name — never that this particular host granted access. `vendor/tinycortex` advances to the engine queries these members forward to. --- crates/tinymemory-api/src/provider/chunks.rs | 82 +- .../tinymemory-api/src/provider/knowledge.rs | 105 +- crates/tinymemory-api/src/provider/mod.rs | 11 +- crates/tinymemory-api/src/provider/records.rs | 112 +- crates/tinymemory-bus/src/lib.rs | 2 +- crates/tinymemory-bus/src/names.rs | 15 +- crates/tinymemory-bus/src/provider/chunks.rs | 167 +++ crates/tinymemory-bus/src/provider/types.rs | 151 +++ .../tinymemory-core/src/store/chunks/store.rs | 80 +- crates/tinymemory-core/src/store/entities.rs | 112 +- .../src/store/retrieval/mod.rs | 6 + crates/tinymemory-module/Cargo.lock | 73 ++ crates/tinymemory-module/Cargo.toml | 19 +- crates/tinymemory-module/src/host.rs | 174 +++ crates/tinymemory-module/src/host_test.rs | 42 +- crates/tinymemory-module/src/lib.rs | 150 ++- crates/tinymemory-module/src/service/mod.rs | 152 ++- crates/tinymemory-module/src/service/test.rs | 73 +- crates/tinymemory-module/tests/module_e2e.rs | 79 +- .../tinymemory-tinycortex/src/engine/mod.rs | 292 +++++- .../tests/full_provider_conformance.rs | 987 +++++++++++++++++- vendor/tinycortex | 2 +- 22 files changed, 2784 insertions(+), 102 deletions(-) diff --git a/crates/tinymemory-api/src/provider/chunks.rs b/crates/tinymemory-api/src/provider/chunks.rs index d27ee119..feff6fdf 100644 --- a/crates/tinymemory-api/src/provider/chunks.rs +++ b/crates/tinymemory-api/src/provider/chunks.rs @@ -41,7 +41,9 @@ use crate::provider::types::SourceScope; // — they cross the module boundary, and a host that only makes calls must be // able to name them without compiling this trait — and re-exported here so // every historical path keeps resolving and the types stay the same types. -pub use tinymemory_bus::provider::chunks::{ChunkDetail, ChunkEmbedding, ChunkQuery}; +pub use tinymemory_bus::provider::chunks::{ + ChunkDetail, ChunkEmbedding, ChunkListRow, ChunkQuery, SourceTotal, +}; /// Direct read access to the chunk tier. /// @@ -102,6 +104,84 @@ pub trait MemoryChunks: Send + Sync { Err(MemoryError::unsupported(Capability::Chunks)) } + /// The same rows [`Self::list_chunks`] returns, each carrying the stored + /// facts a listing renders beside it. + /// + /// Same predicate, same `scope`, same newest-first order, same page + /// bounds — a caller can swap one for the other without re-sorting, and + /// [`Self::count_chunks`] labels either. + /// + /// # Why this is not `list_chunks` plus a call per row + /// + /// A browser page shows a chunk's vault path, its lifecycle state, and + /// whether it has been embedded. Assembling those from + /// [`Self::chunk_detail`] is one call per row — fifty to a thousand bus + /// round trips for one screen, and each of those trips also reads the + /// chunk's body off disk to fill a field the list will not display. That + /// is precisely the fan-out [`ChunkDetail`]'s own docs exist to argue + /// against, reintroduced one level up. + /// + /// # Why the rows are not `ChunkDetail` + /// + /// [`ChunkListRow`] is [`ChunkDetail`] minus its body, and the missing + /// field is the point: `ChunkDetail::body` promises that `None` means the + /// vault read *failed*, which a list can only honour by reading every + /// file or by lying. That type's docs carry the full argument. + /// + /// # Errors + /// + /// [`MemoryError::Unsupported`] from a driver that implements this family + /// but not this member — not defaulted to [`Self::list_chunks`] with empty + /// detail, which would report every row as unembedded and pathless. + /// [`MemoryError::Invalid`] for a [`ChunkQuery`] filter the driver cannot + /// apply, per that type's docs. Otherwise backend failures; no match + /// yields an empty vector. + async fn list_chunk_details( + &self, + query: &ChunkQuery, + scope: Option<&SourceScope>, + ) -> Result, MemoryError> { + let _ = (query, scope); + Err(MemoryError::unsupported(Capability::Chunks)) + } + + /// What the driver holds per logical source, newest source first. + /// + /// One row per `(source_kind, source_id)` group, ordered by + /// [`SourceTotal::most_recent_ms`] descending — the same ordering + /// [`Self::list_chunks`] uses, so a browser showing sources above chunks + /// does not flip between two notions of "first". `limit` caps the rows and + /// is clamped to the driver's own ceiling, exactly as + /// [`ChunkQuery::limit`] is. + /// + /// `scope` filters the chunks the groups are computed *from*, not the + /// groups afterwards: a scoped caller must not learn a forbidden source + /// exists by seeing its total, and must not see permitted sources carrying + /// counts that include rows it cannot read. + /// + /// # Why this is a member and not a fold over a chunk page + /// + /// A group is not a row in any table, so the only way to derive it is to + /// list every chunk in the store and group them client-side — the + /// unbounded query the page limit exists to prevent, and one that would + /// silently answer from the driver's row cap instead of the whole store. + /// It is [`Self::count_chunks`]'s argument applied to a `GROUP BY`: the + /// aggregate has to be computed where the rows are. + /// + /// # Errors + /// + /// [`MemoryError::Unsupported`] from a driver that implements this family + /// but not this member. Otherwise backend failures; an empty store yields + /// an empty vector. + async fn source_totals( + &self, + limit: usize, + scope: Option<&SourceScope>, + ) -> Result, MemoryError> { + let _ = (limit, scope); + Err(MemoryError::unsupported(Capability::Chunks)) + } + /// One chunk by id. /// /// # Errors diff --git a/crates/tinymemory-api/src/provider/knowledge.rs b/crates/tinymemory-api/src/provider/knowledge.rs index 77fa674c..c03f6b79 100644 --- a/crates/tinymemory-api/src/provider/knowledge.rs +++ b/crates/tinymemory-api/src/provider/knowledge.rs @@ -16,7 +16,9 @@ use async_trait::async_trait; use crate::error::MemoryError; use crate::graph::{GraphEdge, GraphNode, GraphView, GraphViewQuery}; -use crate::provider::types::{DiffReport, EntityHit, EntityOccurrence, SnapshotRef}; +use crate::provider::types::{ + ChunkEntityOccurrence, DiffReport, EntityHit, EntityOccurrence, SnapshotRef, +}; use crate::types::{GraphRelationRecord, MemoryKvRecord}; /// How many edges the default [`MemoryGraph::graph_view`] traversal scans per @@ -56,6 +58,21 @@ pub const INBOUND_SCAN_LIMIT: usize = 4_096; /// supported, and an operator reading "unsupported capability: entities" from /// a driver whose entity list works would be chasing the wrong thing. A driver /// that can answer these should override them; the embedded engine does. +/// +/// ## `chunk_entities` changed shape after it was written, and that was allowed +/// +/// It landed taking one `chunk_id` and returning [`EntityOccurrence`]. It now +/// takes a batch and returns [`ChunkEntityOccurrence`]. Re-cutting a member's +/// signature is normally out of bounds here — it breaks every driver at once, +/// and family-granular version negotiation cannot see it — and it is +/// legitimate exactly once, because this member has never been in a release. +/// It was added after `v1.4.0` and ships for the first time alongside this +/// change, so no driver anywhere implements the one-chunk form. +/// +/// The alternative was to keep it and add a batched member beside it, leaving +/// two ways to ask one question and a per-chunk one no caller should reach +/// for. Correcting an unreleased shape costs nothing; carrying it costs every +/// reader after. #[async_trait] pub trait MemoryEntities: Send + Sync { /// List entities in a namespace, ranked by hotness when `query` is `None` @@ -145,40 +162,82 @@ pub trait MemoryEntities: Send + Sync { Err(MemoryError::unsupported_raw("entities.top_entities")) } - /// Every entity indexed against one chunk, most-observed first. + /// Every entity indexed against these chunks, most-observed first. /// /// The inverse of [`Self::entity_chunk_ids`], and the only read in this /// family that starts from content instead of from an entity: it is how a - /// caller labels a chunk it already has — a retrieval hit, a row in a - /// browser — without re-running extraction over the text. + /// caller labels chunks it already has — a page of retrieval hits, a + /// screen of a browser — without re-running extraction over the text. /// - /// A summary node's id is accepted too, and answered from the same index. - /// The parameter is named for the common case, not to exclude the other: + /// Summary-node ids are accepted too, and answered from the same index. + /// `chunk_ids` is named for the common case, not to exclude the other: /// refusing an id the index can answer would send the caller to raw SQL /// for the difference. /// - /// One entity may appear more than once, once per distinct - /// [`EntityOccurrence::surface`], because the two forms are the evidence a - /// caller has for how this chunk actually named it. Deduplicating by id - /// here would throw that away and leave `surface` meaning "whichever row - /// sorted last". - /// - /// # Why there is no `limit` + /// # Why this takes a batch + /// + /// It is asked once per rendered list, not once per chunk opened. A + /// caller labelling fifteen hundred rows one call at a time makes fifteen + /// hundred bus round trips to read one index — exactly the fan-out + /// [`ChunkDetail`]'s docs were written to prevent, at the scale that makes + /// it fatal rather than merely wasteful. The caller bounds the work by + /// choosing the batch, which is why there is still no `limit` (see below); + /// a driver that considers a batch too large refuses it with + /// [`MemoryError::Invalid`] rather than answering for part of it, because + /// a refusal is visible to the caller and a truncation is not. + /// + /// Rows are [`ChunkEntityOccurrence`] rather than [`EntityOccurrence`] + /// because a flat list over many chunks has no other way back to the chunk + /// a row describes. **Group by [`ChunkEntityOccurrence::chunk_id`]; never + /// index by position.** A chunk the extractor has not reached contributes + /// no rows at all, so the result covers fewer chunks than were asked for + /// and says nothing about their order. + /// + /// One entity may still appear more than once for the same chunk, once per + /// distinct [`EntityOccurrence::surface`], because the two forms are the + /// evidence a caller has for how that chunk actually named it. + /// Deduplicating by id here would throw that away and leave `surface` + /// meaning "whichever row sorted last". + /// + /// # Filtering by kind + /// + /// `None` returns every kind. `Some` narrows to the kinds listed, and the + /// list is **validated, not merely applied**: an unrecognised kind is + /// [`MemoryError::Invalid`], never an empty result, because a misspelled + /// filter that matched nothing is indistinguishable from a chunk nothing + /// was extracted from. That is [`Self::top_entities`]'s rule, kept the same + /// here so one filter does not behave two ways. + /// + /// `Some(&[])` is a filter admitting no kind and yields an empty vector. + /// It is not a second spelling of `None` — `None` is already how a caller + /// says "no filter", so reading an empty slice as "everything" would leave + /// the `Option` meaning nothing. + /// + /// # Why there is still no `limit` /// /// Every other list in this family is bounded by one, and this one is - /// already bounded by the thing it reads: a chunk's rows are what that - /// chunk's own extraction produced. There is no query for a caller to - /// narrow and no ranking for a cut-off to respect — a truncated answer - /// would simply be a wrong description of the chunk. + /// bounded by the thing it reads: a chunk's rows are what that chunk's own + /// extraction produced, and the caller chose how many chunks to ask about. + /// There is no ranking for a cut-off to respect — a truncated answer would + /// silently describe some of the batch and not the rest, with nothing on + /// the wire to say which. /// /// # Errors /// - /// Backend failures only. An unknown `chunk_id` yields an empty vector - /// rather than [`MemoryError::NotFound`], for [`Self::entity_edges`]'s - /// reason: "nothing was extracted from it" and "there is no such chunk" - /// are the same answer to this question. - async fn chunk_entities(&self, chunk_id: &str) -> Result, MemoryError> { - let _ = chunk_id; + /// [`MemoryError::Invalid`] for an unrecognised kind, or for a batch the + /// driver declines to answer whole. Otherwise backend failures: unknown + /// ids yield no rows rather than + /// [`MemoryError::NotFound`], for [`Self::entity_edges`]'s reason — + /// "nothing was extracted from it" and "there is no such chunk" are the + /// same answer to this question. + /// + /// [`ChunkDetail`]: crate::provider::ChunkDetail + async fn chunk_entities( + &self, + chunk_ids: &[String], + kinds: Option<&[String]>, + ) -> Result, MemoryError> { + let _ = (chunk_ids, kinds); Err(MemoryError::unsupported_raw("entities.chunk_entities")) } diff --git a/crates/tinymemory-api/src/provider/mod.rs b/crates/tinymemory-api/src/provider/mod.rs index 6df598bf..de1a99da 100644 --- a/crates/tinymemory-api/src/provider/mod.rs +++ b/crates/tinymemory-api/src/provider/mod.rs @@ -74,7 +74,9 @@ pub mod retrieval; pub use tinymemory_bus::provider::types; pub use audit::{audit_provider, CapabilityAudit}; -pub use chunks::{ChunkDetail, ChunkEmbedding, ChunkQuery, MemoryChunks}; +pub use chunks::{ + ChunkDetail, ChunkEmbedding, ChunkListRow, ChunkQuery, MemoryChunks, SourceTotal, +}; pub use content::{MemoryDocuments, MemoryIngest, MemoryTree}; pub use driver::MemoryProvider; pub use episodic::{ConversationSegment, EpisodicEvent, EpisodicTurn, EventKind, MemoryEpisodic}; @@ -91,7 +93,8 @@ pub use retrieval::{ RetrievalNodeKind, RetrievalResponse, SourceRetrievalQuery, }; pub use types::{ - ChangeKind, DiffReport, EntityHit, EntityOccurrence, EntityRef, ExportPage, ExportRecord, - FlushOutcome, ImportOutcome, IngestItem, IngestOutcome, MaintenanceReport, ResetOutcome, - SnapshotRef, SourceChange, SourceItem, SourceScope, + ChangeKind, ChunkEntityOccurrence, DiffReport, EntityHit, EntityOccurrence, EntityRef, + ExportPage, ExportRecord, FlushOutcome, ForgetOutcome, ForgetSelector, ImportOutcome, + IngestItem, IngestOutcome, MaintenanceReport, PurgeOutcome, ResetOutcome, SnapshotRef, + SourceChange, SourceItem, SourceScope, }; diff --git a/crates/tinymemory-api/src/provider/records.rs b/crates/tinymemory-api/src/provider/records.rs index 952d3f7a..80ce4509 100644 --- a/crates/tinymemory-api/src/provider/records.rs +++ b/crates/tinymemory-api/src/provider/records.rs @@ -9,18 +9,19 @@ //! //! [`MemorySourceSink`] receives already-fetched items — the host owns //! credentials, OAuth, rate limits, and the schedule. [`MemoryMaintenance`] -//! exposes four operations the host's existing scheduler calls; no driver +//! exposes the operations the host's existing scheduler calls; no driver //! installs a background task of its own. Both follow the same rule as the //! engine's `queue::run_once`, and both are why a driver never needs to see //! configuration or a keychain. use async_trait::async_trait; +use crate::capabilities::Capability; use crate::error::MemoryError; use crate::goals::GoalsDoc; use crate::provider::types::{ - FlushOutcome, IngestOutcome, MaintenanceReport, QueueFailure, QueueStats, ResetOutcome, - SourceItem, StoreStats, + FlushOutcome, ForgetOutcome, ForgetSelector, IngestOutcome, MaintenanceReport, PurgeOutcome, + QueueFailure, QueueStats, ResetOutcome, SourceItem, StoreStats, }; use crate::tool_memory::ToolMemoryRule; use crate::types::MemoryTaint; @@ -118,14 +119,63 @@ pub trait MemorySourceSink: Send + Sync { /// /// Backend failures only. async fn forget_source(&self, source_id: &str) -> Result; + + /// Remove whatever [`ForgetSelector`] names, and report what went with it. + /// + /// # How this differs from [`Self::forget_source`] + /// + /// [`Self::forget_source`] is the whole-source disconnect: one logical id, + /// every kind it appears under, one number back. This is the selective + /// path, and each of its arms is something that call cannot express — a + /// single chunk, a kind-qualified source, a family of derived source ids + /// under one prefix, everything one owner brought in. Widening + /// `forget_source` to cover them would mean four `Option` arguments where + /// at most one may ever be set, on a call that deletes. + /// + /// A driver implementing both must keep them consistent: a + /// [`ForgetSelector::Source`] naming the only kind a source has must + /// remove exactly what `forget_source` would. + /// + /// # Why the outcome is not a count + /// + /// Deleting chunks can strand the summary trees derived from them, and + /// cleaning a stranded tree is work that happens with no chunk removed at + /// all. [`ForgetOutcome`] keeps the two counts apart so a caller can tell + /// "nothing matched" from "nothing was left but the summaries". + /// + /// # Errors + /// + /// [`MemoryError::Unsupported`] from a driver that implements this family + /// but not this member — deliberately not defaulted onto + /// [`Self::forget_source`] for the [`ForgetSelector::Source`] arm, because + /// a default that quietly ignored `source_kind` would delete across kinds + /// a caller had narrowed away from. + /// + /// [`MemoryError::Invalid`] for a `source_kind` the driver does not + /// recognise, never an outcome of zero: on a delete, a zero the caller + /// reads as "already gone" is worse than a refusal. + /// + /// Otherwise backend failures. Idempotent — a selector that matches + /// nothing removes nothing and returns an all-zero [`ForgetOutcome`]. + async fn forget_matching( + &self, + selector: &ForgetSelector, + ) -> Result { + let _ = selector; + Err(MemoryError::unsupported(Capability::Sources)) + } } /// Periodic upkeep the host's scheduler drives. /// -/// All four operations must be safe to call repeatedly and safe to interrupt: +/// Every operation here must be safe to call repeatedly and safe to interrupt: /// the scheduler may invoke them on a timer, and a desktop process can exit at /// any point. A driver that cannot bound the work should do a slice per call /// and report progress in [`MaintenanceReport`]. +/// +/// [`Self::purge_all`] is the one member no scheduler may drive. It sits here +/// because this is where the optional operator-triggered mutations live, not +/// because it is upkeep; its own docs say why no other family could hold it. #[async_trait] pub trait MemoryMaintenance: Send + Sync { /// Recompute embeddings for content whose embedding is missing or stale. @@ -312,4 +362,58 @@ pub trait MemoryMaintenance: Send + Sync { async fn reset_derived_index(&self) -> Result { Ok(ResetOutcome::default()) } + + /// Delete everything this driver has stored. + /// + /// The operator's factory reset: every chunk, every derived row, every + /// queue entry. The inverse of [`Self::reset_derived_index`], which is + /// safe precisely because it deletes only what it can rebuild. Nothing + /// here is rebuildable, and a caller reaching it has already asked a + /// human. + /// + /// # Where the wipe stops + /// + /// At the storage the driver owns. A driver that keeps content outside its + /// database in a place of its own choosing clears that too: leaving it + /// behind orphans bytes nothing will ever reference again. A directory the + /// *host* created, configured, and hands the driver a path into is the + /// host's to remove, and a driver deleting host-owned directories is + /// reaching past its own storage into somewhere it cannot reason about. + /// The embedded driver's content vault is the second kind. + /// + /// # Why this is on maintenance and not on portability + /// + /// A wipe reads like the companion of import and export, and that is the + /// wrong home for it: [`crate::provider::MemoryPortability`] is a + /// **mandatory** supertrait, so putting it there would oblige every driver + /// that compiles to implement a destructive whole-store delete — including + /// the ones with nothing to wipe, and the ones fronting a store that must + /// never be wiped through this contract at all. This family is optional + /// and already holds the operator-triggered mutations + /// ([`Self::reset_derived_index`], [`Self::flush_pending`]), so a driver + /// declines by not advertising rather than by implementing a stub. + /// + /// # Why this defaults to a refusal + /// + /// The rest of this family defaults to an empty result, and that is honest + /// for a *read* that under-claims: "nothing to report" is true of a driver + /// with no queue. A `purge_all` defaulting to `rows_deleted: 0` would + /// report a completed wipe from a driver that deleted nothing, to a caller + /// whose next act is telling the user their memory is gone. It is the same + /// distinction [`crate::null::NullMemoryProvider`] draws when it overrides + /// the two mutating defaults above rather than inheriting them. + /// + /// # Errors + /// + /// [`MemoryError::Unsupported`] from a driver that cannot — or must not — + /// destroy its store on request; that is the correct answer for one + /// fronting a shared or externally-owned backend. + /// + /// Otherwise backend failures. A partial wipe is a failure and not a + /// smaller success: a driver that cannot make this atomic reports what it + /// managed in the error, rather than returning `Ok` over a store that is + /// now half there. + async fn purge_all(&self) -> Result { + Err(MemoryError::unsupported(Capability::Maintenance)) + } } diff --git a/crates/tinymemory-bus/src/lib.rs b/crates/tinymemory-bus/src/lib.rs index 6553ff16..9b988117 100644 --- a/crates/tinymemory-bus/src/lib.rs +++ b/crates/tinymemory-bus/src/lib.rs @@ -2,7 +2,7 @@ //! the members that carry them. //! //! TinyMemory ships as a loadable `TinyBus` module: `crates/tinymemory-module` -//! exports one object with 105 members on it, built as a `cdylib`. A host that +//! exports one object with 109 members on it, built as a `cdylib`. A host that //! loads it — OpenHuman — can call into it but cannot `use` anything out of it, //! so the payload vocabulary has to be published as an ordinary library. This //! is that library. diff --git a/crates/tinymemory-bus/src/names.rs b/crates/tinymemory-bus/src/names.rs index 958dc968..f3eb2e48 100644 --- a/crates/tinymemory-bus/src/names.rs +++ b/crates/tinymemory-bus/src/names.rs @@ -138,6 +138,8 @@ pub mod methods { pub const ACCEPT_SOURCE_ITEMS: &str = "AcceptSourceItems"; /// `ForgetSource` — forget source. pub const FORGET_SOURCE: &str = "ForgetSource"; + /// `ForgetMatching` — forget everything one selector names. + pub const FORGET_MATCHING: &str = "ForgetMatching"; // The long-term goals document. /// `Goals` — goals. @@ -179,6 +181,8 @@ pub mod methods { pub const FLUSH_PENDING: &str = "FlushPending"; /// `ResetDerivedIndex` — drop derived state and schedule its rebuild. pub const RESET_DERIVED_INDEX: &str = "ResetDerivedIndex"; + /// `PurgeAll` — erase every row the driver holds. + pub const PURGE_ALL: &str = "PurgeAll"; // The people store: ranking, handles, scores and interactions. /// `ListPeople` — list people. @@ -210,6 +214,11 @@ pub mod methods { /// `CountChunks` — how many chunks `ListChunks` matches, page bounds /// ignored. pub const COUNT_CHUNKS: &str = "CountChunks"; + /// `ListChunkDetails` — the metadata `ChunkDetail` returns, for a whole + /// page at once. + pub const LIST_CHUNK_DETAILS: &str = "ListChunkDetails"; + /// `SourceTotals` — one row per source, with what it contributed. + pub const SOURCE_TOTALS: &str = "SourceTotals"; // The scored retrieval surface. /// `FastRetrieve` — fast retrieve. @@ -273,7 +282,7 @@ pub mod methods { /// The order matters: `tinybus`'s `Interface::members()` returns declaration /// order, and the module compares the two sequences directly rather than as /// sets, so a reordering is caught alongside an addition or a removal. -pub const METHODS: [&str; 105] = [ +pub const METHODS: [&str; 109] = [ methods::DRIVER_ID, methods::CAPABILITIES, methods::HEALTH, @@ -379,6 +388,10 @@ pub const METHODS: [&str; 105] = [ methods::ENTITY_CHUNK_IDS, methods::SUMMARY_FOREST, methods::RECENT_LEAVES, + methods::LIST_CHUNK_DETAILS, + methods::SOURCE_TOTALS, + methods::FORGET_MATCHING, + methods::PURGE_ALL, ]; #[cfg(test)] diff --git a/crates/tinymemory-bus/src/provider/chunks.rs b/crates/tinymemory-bus/src/provider/chunks.rs index 0be7aef9..55195a2e 100644 --- a/crates/tinymemory-bus/src/provider/chunks.rs +++ b/crates/tinymemory-bus/src/provider/chunks.rs @@ -38,6 +38,28 @@ use crate::chunks::{Chunk, SourceKind}; /// /// Every field is optional and they compose with AND. The default matches /// everything the scope allows, bounded by the driver's own safety cap. +/// +/// # An empty collection is "no constraint", never "match nothing" +/// +/// The collection filters default to empty, and `Default` has to keep meaning +/// "everything the scope allows" — so an empty `Vec` places no constraint at +/// all. A caller that narrowed a list of ids down to none must therefore skip +/// the call rather than send it: an empty [`Self::ids`] reads as "no id +/// filter" and answers with the whole store. +/// +/// # A driver that cannot apply a filter refuses the query +/// +/// These fields are additive on the wire, which is exactly what makes them +/// invisible to a driver that has not implemented them — and a driver that +/// accepts a filter without applying it returns the rows the caller asked to +/// exclude. On [`Self::content_contains`] or [`Self::entity_ids`] those are +/// the rows a scoped browser was told not to show, so the failure is not a +/// wider page, it is a leak. +/// +/// So a driver that cannot honour a filter answers +/// [`MemoryError::Invalid`](crate::error::MemoryError::Invalid) naming it. +/// Refusing is recoverable; silently widening the result is not, because +/// nothing downstream can tell that it happened. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct ChunkQuery { /// Restrict to one source kind. @@ -65,6 +87,74 @@ pub struct ChunkQuery { /// Drop chunks marked dropped by the lifecycle. #[serde(default)] pub exclude_dropped: bool, + /// Restrict to this explicit set of chunk ids. + /// + /// For a caller that already holds the ids — retrieval hits it wants the + /// stored rows behind, a selection it is re-reading — rather than a filter + /// over content. Ids the store does not hold contribute no row, so the + /// result may be shorter than the input and must not be indexed by + /// position against it. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub ids: Vec, + /// Restrict to any of these source kinds. + /// + /// The set form of [`Self::source_kind`] and not a replacement for it: + /// both are applied, so a scalar naming one kind and a set naming another + /// match nothing at all. A caller that wants several kinds leaves the + /// scalar unset. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub source_kinds: Vec, + /// Restrict to any of these logical source ids. + /// + /// The set form of [`Self::source_id`], read the same way: both apply. + /// Exact ids only — a prefix is not a source id, and honouring one here + /// would quietly make `mem_src:x` select `mem_src:x-archive` too. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub source_ids: Vec, + /// Restrict to chunks the entity index has indexed against any of these + /// entity ids. + /// + /// Ids live in [`EntityRef::id`]'s space, so a name resolved through the + /// entity or retrieval families can be handed straight back here. + /// + /// This reads a **derived** index rather than the text: a chunk whose + /// extraction has not run yet is absent even though its body names the + /// entity. A caller rendering "chunks about X" against a store that is + /// still indexing should say so rather than report that there are none. + /// + /// A chunk matching several of these entities is still **one** row. The + /// join multiplies rows and the driver collapses them; a caller must not + /// have to discover that adding a filter grew its page. + /// + /// [`EntityRef::id`]: crate::provider::types::EntityRef::id + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub entity_ids: Vec, + /// Restrict to chunks carrying at least one indexed entity of any of these + /// kinds (`person`, `organization`, `topic`, …). + /// + /// The same open vocabulary as [`EntityRef::kind`] and the same collapse + /// as [`Self::entity_ids`]. It composes with `entity_ids` by AND like + /// everything else, which is an intersection and not a union: a query + /// naming both matches chunks holding one of those entities *and* one of + /// those kinds, and the two need not be the same observation. + /// + /// [`EntityRef::kind`]: crate::provider::types::EntityRef::kind + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub entity_kinds: Vec, + /// Restrict to chunks whose stored text contains this substring. + /// + /// A literal substring and not a query language: `%` and `_` match + /// themselves, and there is no tokenisation, stemming, or ranking. Case is + /// folded for ASCII only — which is what the stores behind this actually + /// do, and promising full Unicode folding here would be a promise a + /// SQLite `LIKE` cannot keep. + /// + /// It scans the text the driver holds inline, which for a chunk whose body + /// was written to the content vault is the stored preview and not the + /// whole document. So this narrows a browse; it does not replace + /// `MemoryRecall`, which is what "search my memory" should reach for. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub content_contains: Option, } /// One chunk's stored embedding. @@ -112,3 +202,80 @@ pub struct ChunkDetail { /// a *particular* space has it is `MemoryChunks::chunk_embeddings`. pub has_embedding: bool, } + +/// One row of a chunk *listing*: a [`ChunkDetail`] without its body. +/// +/// # Why a listing is not `Vec` +/// +/// [`ChunkDetail::body`] carries a contract a list cannot honour. `None` there +/// means **the vault read failed**, not "we did not look". Filling a page of +/// details truthfully would mean opening every row's file in the content vault +/// — fifty to a thousand of them for one screen of a browser — and filling it +/// with `None` instead would report every row as a failed read to a caller +/// whose next move is to tell the user their content is unreadable. Either the +/// list is unusably slow or the field lies; there is no third reading. +/// +/// So the body is *absent* rather than empty. Everything else a list renders — +/// where the body lives, whether the row is still active, whether it has been +/// embedded — sits in a column beside the chunk and costs nothing to return, +/// which is the same one-trip argument [`ChunkDetail`] itself is built on. +/// +/// The two are deliberately **not** interchangeable on the wire even though +/// this one is a subset of that one: decode a `ChunkListRow` as a +/// [`ChunkDetail`] and `body` defaults to `None`, turning "not read" into +/// "read failed". A caller that wants a body asks `MemoryChunks::chunk_detail` +/// for the single row it is opening. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +pub struct ChunkListRow { + /// The chunk row, exactly as `MemoryChunks::list_chunks` would return it. + pub chunk: Chunk, + /// Path of the body in the content vault, when it has one. + /// + /// Where the body *is*, never what it *says* — a caller can show that a + /// row is backed by a file, or open that one file, without the list having + /// paid for every read. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub content_path: Option, + /// Lifecycle state (`active`, `dropped`, …); `None` when unrecorded. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub lifecycle_status: Option, + /// Whether an embedding vector exists for this chunk in **any** space. + /// + /// [`ChunkDetail::has_embedding`]'s reading exactly, and signature-blind + /// for the same reason: a list is asking "has this been embedded at all". + pub has_embedding: bool, +} + +/// One logical source and what the driver holds for it. +/// +/// The unit a memory browser lists above the chunks. A source is not a row in +/// any table — it is the `(source_kind, source_id)` group the chunk rows fall +/// into — so this is an aggregate, and a caller cannot assemble it from a +/// chunk page: it would have to list every chunk in the store to group them, +/// which is the query the page limit exists to prevent. +/// +/// # What is deliberately not here +/// +/// No display name. Turning `gmail:alice@example.com|bob@example.com` into +/// "bob@example.com" requires knowing which address is the user's, and that is +/// host policy resting on host state — the same line redaction and the source +/// safety rules already sit on. A driver that guessed would be guessing about +/// a person. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct SourceTotal { + /// Which kind of source this group is. + pub source_kind: SourceKind, + /// The logical source id the group is keyed by. + pub source_id: String, + /// Chunks the driver holds for the group. + pub chunk_count: u64, + /// Source time of the newest chunk in the group, epoch milliseconds. + /// + /// Not an `Option`, unlike the store-wide `most_recent_chunk_ms` on + /// [`StoreStats`]: a group exists only because a chunk fell into it, so + /// there is always a newest one. A source holding nothing is not a zero + /// row here, it is absent from the list. + /// + /// [`StoreStats`]: crate::provider::types::StoreStats + pub most_recent_ms: i64, +} diff --git a/crates/tinymemory-bus/src/provider/types.rs b/crates/tinymemory-bus/src/provider/types.rs index ccd9c8ac..f9b40c0d 100644 --- a/crates/tinymemory-bus/src/provider/types.rs +++ b/crates/tinymemory-bus/src/provider/types.rs @@ -398,6 +398,39 @@ pub struct EntityOccurrence { pub mentions: u32, } +/// One [`EntityOccurrence`] together with the chunk it was observed in. +/// +/// ## Why the chunk id is on the row +/// +/// Asking one chunk what it is about needs no such field: the chunk id was the +/// argument, and every row answers for it. Asking fifteen hundred chunks in +/// one call returns a single flat list, and without the id on each row there +/// is no way back from a row to the content it describes. +/// +/// The alternative shape — a list per chunk, or a map keyed by chunk id — was +/// rejected twice over. It encodes the grouping in the type, so every chunk +/// the extractor has not reached yet costs an empty vector on the wire; and a +/// map keyed by chunk id serialises as a JSON object whose keys are +/// caller-supplied text, which is the encoding [`super::chunks::ChunkEmbedding`] +/// is a list to avoid. +/// +/// The occurrence is **flattened** rather than nested, so the wire form is an +/// [`EntityOccurrence`] object carrying one extra `chunk_id` key. A caller that +/// already decodes occurrences reads these under the same field names. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct ChunkEntityOccurrence { + /// The chunk — or the summary node — this observation came from. + /// + /// Rows are **not** unique by this: one chunk contributes one row per + /// distinct `(entity, surface)` it was observed under, for + /// [`EntityOccurrence::surface`]'s reason. Group by it; never index by + /// position against the ids that were asked for. + pub chunk_id: String, + /// The observation itself. + #[serde(flatten)] + pub occurrence: EntityOccurrence, +} + /// Identity of a captured snapshot. /// /// The engine's own snapshot type additionally carries the git commit SHA and @@ -518,6 +551,99 @@ pub struct SourceItem { pub tags: Vec, } +/// Which stored content a selective forget removes. +/// +/// ## Why an enum and not a struct of options +/// +/// The four arms are mutually exclusive and each maps 1:1 onto a delete the +/// engine already implements — by chunk id, by exact source, by source-id +/// prefix, by owner. A struct of `Option` fields would admit combinations none +/// of those deletes has a meaning for (`chunk_id` *and* `owner`; a prefix *and* +/// an exact id), and the driver would have to invent a precedence rule and +/// document it, for a **destructive** call where guessing wrong deletes the +/// wrong content. The enum makes the illegal combinations unrepresentable +/// instead of merely discouraged. +/// +/// ## Why `source_kind` is a wire string +/// +/// The same reason `MemorySourceSink::accept_source_items` takes one: the set +/// of source kinds belongs to the host's sync machinery and grows without a +/// contract change. A driver parses it and answers +/// [`MemoryError::Invalid`](crate::error::MemoryError::Invalid) for a kind it +/// does not recognise — never an outcome of zero. On a read a silent zero is a +/// misleading empty list; here it is an operator told their content was +/// already gone when nothing was even looked at. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "by", rename_all = "snake_case")] +pub enum ForgetSelector { + /// One chunk, by id. + /// + /// The narrowest arm, and the only one that does not name a source: a + /// caller removing a single row it is looking at has the id and nothing + /// else. An id the store does not hold removes nothing and is not an + /// error, matching every other idempotent delete in this contract. + Chunk { + /// The chunk to remove. + chunk_id: String, + }, + /// Everything stored under one exact `(source_kind, source_id)`. + /// + /// Exact, never a prefix, so sibling sources sharing a leading segment are + /// untouched — that is what [`Self::SourcePrefix`] is for, and conflating + /// the two is how a single disconnect takes a workspace with it. + Source { + /// Kind of the source, as a wire string. + source_kind: String, + /// The exact logical source id. + source_id: String, + }, + /// Everything whose source id begins with `source_id_prefix`, under one + /// kind. + /// + /// The disconnect path for a provider that files one logical connection + /// under many derived ids. The prefix is matched literally: it is not a + /// pattern, so `%` and `_` in a provider id mean themselves. + SourcePrefix { + /// Kind of the sources, as a wire string. + source_kind: String, + /// Literal prefix the source ids must start with. + source_id_prefix: String, + }, + /// Everything owned by one owner, under one kind. + /// + /// Owner is the account the content came in through, so this is the arm a + /// caller reaches for when one connection of several is removed and the + /// others must survive on the same source. + Owner { + /// Kind of the sources, as a wire string. + source_kind: String, + /// The owner whose content is removed. + owner: String, + }, +} + +/// What a selective forget removed. +/// +/// Two counts because they are two different deletions, not a total and a +/// part. Chunks are what the caller asked to remove; a summary tree is +/// *derived* from chunks and is cleaned only once every chunk under it has +/// gone, so a forget that removes rows may clean no tree and a forget that +/// removes none may still clean one left stranded by an earlier partial +/// delete. Summed into one number, neither case can be told from the other. +/// +/// A count rather than the `bool` the single-source path used before it: an +/// exact-source delete can orphan at most one tree, but a prefix or owner +/// selector spans many sources and can orphan several, and a `bool` would have +/// to report three as "yes". +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct ForgetOutcome { + /// Chunk rows removed, together with the per-chunk side rows and content + /// files that hang off them. + pub chunks_removed: u64, + /// Summary trees cascaded away because nothing was left under them. + pub trees_cleaned: u64, +} + /// Outcome of one maintenance operation. /// /// A single shape covers reembed, compact, consolidate, and doctor because the @@ -674,6 +800,31 @@ pub struct ResetOutcome { pub jobs_enqueued: u64, } +/// What wiping the whole store deleted. +/// +/// One field, and a struct rather than a bare `u64`, for two reasons that both +/// only show up later. The unit is named where an integer return would leave +/// it to a call site to remember — these are *rows*, across every table the +/// driver owns, not chunks. And a wipe is the one operation whose reporting +/// will want to grow: a driver that later counts the vault files it discarded, +/// or the tables it truncated, adds a field, where a bare integer would have +/// to be replaced and every caller changed with it. +/// +/// Deliberately **not** a [`ResetOutcome`]. That one deletes derived rows and +/// schedules their re-derivation, so its three counts describe work that +/// continues; nothing continues after this. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct PurgeOutcome { + /// Database rows the driver deleted, summed across its own tables. + /// + /// Rows only, and deliberately not a second count of files: a file count + /// is not comparable between drivers, and a driver that keeps bodies + /// in-row would report zero and read as having done less than one that + /// does not. What a wipe reaches beyond the database is the driver's own + /// question — `MemoryMaintenance::purge_all` says where that line falls. + pub rows_deleted: u64, +} + #[cfg(test)] #[path = "types_tests.rs"] mod tests; diff --git a/crates/tinymemory-core/src/store/chunks/store.rs b/crates/tinymemory-core/src/store/chunks/store.rs index 59115e77..4cb3aa19 100644 --- a/crates/tinymemory-core/src/store/chunks/store.rs +++ b/crates/tinymemory-core/src/store/chunks/store.rs @@ -1,6 +1,6 @@ //! `Config` and transaction adapters for tinycortex chunk persistence. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use anyhow::Result; use rusqlite::Transaction; @@ -11,8 +11,9 @@ use crate::store::content::StagedChunk; use crate::Config; pub use crate::engine::backend::chunks::{ - ListChunksQuery, RawRef, CHUNK_STATUS_ADMITTED, CHUNK_STATUS_BUFFERED, CHUNK_STATUS_DROPPED, - CHUNK_STATUS_PENDING_EXTRACTION, CHUNK_STATUS_SEALED, RAW_FILE_GATE_KIND, + ChunkDetailRow, ListChunksQuery, RawRef, SourceTotal, CHUNK_STATUS_ADMITTED, + CHUNK_STATUS_BUFFERED, CHUNK_STATUS_DROPPED, CHUNK_STATUS_PENDING_EXTRACTION, + CHUNK_STATUS_SEALED, RAW_FILE_GATE_KIND, }; pub fn upsert_chunks(config: &Config, chunks: &[Chunk]) -> Result { @@ -77,6 +78,40 @@ pub fn count_chunks_matching(config: &Config, query: &ListChunksQuery) -> Result crate::engine::backend::chunks::count_chunks_matching(&engine_config(config), query) } +/// The same page [`list_chunks`] returns, carrying the per-row facts an +/// inspection view renders beside each chunk. +/// +/// One statement per page, not [`get_chunk`] plus four side-table reads per +/// row. The caller this exists for renders pages of up to a thousand rows, and +/// the per-row shape would make that five thousand queries — which is why the +/// contract has a list member here and a detail member for the single-row case +/// rather than one of them looped. +/// +/// The predicate is [`list_chunks`]'s own, built engine-side by the same filter +/// builder [`count_chunks_matching`] uses, so a page of details, a page of +/// chunks and the total beside them cannot disagree about which rows match. +pub fn list_chunk_details(config: &Config, query: &ListChunksQuery) -> Result> { + crate::engine::backend::chunks::list_chunk_details(&engine_config(config), query) +} + +/// Per-source chunk totals, most recently written source first. +/// +/// The `GROUP BY source_kind, source_id` a source browser opens with. Derived +/// caller-side it is a full-table listing measured in memory — the unbounded +/// query the row limit exists to prevent — so it is answered where the +/// aggregate is. `limit` bounds the number of *sources*, not of chunks, +/// because the source is the row the caller renders. +/// +/// `source_scope` is [`list_chunks`]'s allowlist, applied the same way: a +/// scoped caller must not learn that a source exists by seeing its total. +pub fn source_totals( + config: &Config, + limit: Option, + source_scope: Option<&HashSet>, +) -> Result> { + crate::engine::backend::chunks::source_totals(&engine_config(config), limit, source_scope) +} + pub fn extraction_coverage(config: &Config) -> Result { crate::engine::backend::chunks::extraction_coverage(&engine_config(config)) } @@ -157,6 +192,45 @@ pub fn delete_orphaned_source_tree(config: &Config, kind: SourceKind, id: &str) crate::engine::backend::chunks::delete_orphaned_source_tree(&engine_config(config), kind, id) } +/// Delete one chunk by id, with the score, entity-index and embedding rows +/// hanging off it and its body in the content vault. +/// +/// The per-id sibling of [`delete_chunks_by_source`], and not expressible +/// through it: a chunk id is not a source id, and deleting the chunk's source +/// would take every other chunk of that source with it. A caller that removes +/// a single row without this cascades nothing, and the orphaned side rows keep +/// the chunk visible to entity and score reads that never look at +/// `mem_tree_chunks`. +/// +/// `0` means no such chunk, which is the same end state as a successful delete +/// and is reported apart only so a caller can tell the user whether it did +/// anything. +pub fn delete_chunk_by_id(config: &Config, chunk_id: &str) -> Result { + crate::engine::backend::chunks::delete_chunk_by_id(&engine_config(config), chunk_id) +} + +/// Empty the chunk tier and everything derived from it, in one transaction. +/// +/// The opposite end of the scale from [`delete_chunks_by_source`]: no +/// selector, no survivors. It is deliberately *not* [`delete_chunks_by_owner`] +/// over every owner — the derived tables (summaries, trees, buffers, jobs, the +/// ingest gates) are keyed by things a chunk-shaped delete cannot enumerate, +/// so a per-source sweep leaves them behind and the store comes back holding a +/// tree over chunks that no longer exist. +/// +/// One transaction because a partial wipe is worse than no wipe: a caller that +/// saw an error and retried against a store whose gates were cleared but whose +/// chunks were not would re-ingest nothing and be told the source was already +/// there. +/// +/// Returns the number of **chunk** rows removed, the same unit the selective +/// deletes above return — not the sum over every table emptied, which would +/// change meaning each time the purge learns about another one. Content files +/// on disk go with them; this count is the database half. +pub fn purge_all(config: &Config) -> Result { + crate::engine::backend::chunks::purge_all(&engine_config(config)) +} + #[path = "connection.rs"] mod connection; pub(crate) use connection::recover_corrupt_db; diff --git a/crates/tinymemory-core/src/store/entities.rs b/crates/tinymemory-core/src/store/entities.rs index b3453939..7836d96b 100644 --- a/crates/tinymemory-core/src/store/entities.rs +++ b/crates/tinymemory-core/src/store/entities.rs @@ -223,7 +223,35 @@ pub fn top_entity_rows( Ok(rows) } -/// The entity rows recorded against one tree node, most-observed first. +/// One aggregated occurrence row, carrying the node it was observed on. +/// +/// [`EntityIndexRow`] deliberately has no node: it is the shape of a query +/// that groups *across* nodes, and a node id there would have to be a sample +/// of one row in the group. This is the shape of a query that groups *within* +/// each node, so the node is part of the key rather than a sample, and a +/// caller reading many nodes at once can tell whose row it is holding. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct NodeEntityRow { + /// The tree node — a chunk id, or a summary id where the caller asked for + /// one. + pub node_id: String, + /// Canonical entity id. + pub entity_id: String, + /// Stable entity kind string, as stored. + pub entity_kind: String, + /// An observed surface form on this node. + pub surface: String, + /// Number of index rows aggregated into this one. + pub mentions: u32, +} + +/// Defensive cap on ids bound into one `IN (?,?,…)`, far below SQLite's +/// `SQLITE_MAX_VARIABLE_NUMBER` (32 766) and the same window the engine's own +/// batched chunk read uses. +const MAX_NODE_BATCH: usize = 500; + +/// The entity rows recorded against a set of tree nodes, most-observed first +/// within each node. /// /// Grouped by surface as well as by id: one entity seen under two forms is two /// rows, because the form is the evidence of how this node's text named it. @@ -233,23 +261,83 @@ pub fn top_entity_rows( /// entity — and is reported anyway: it is the same aggregate /// [`top_entity_rows`] returns, and an index that later keys occurrences per /// span would make it meaningful without a shape change here. -pub fn node_entity_rows(config: &Config, node_id: &str) -> Result> { +/// +/// # Why a set of nodes rather than one +/// +/// The caller labelling a page of chunks has as many nodes as the page has +/// rows. One node per call turns a 1 500-row contacts graph into 1 500 round +/// trips, and across the module bus each of those is a message rather than a +/// function call. The single-node case is this call with a slice of one. +/// +/// `node_ids` is windowed so no single statement approaches the bound-parameter +/// limit; the windows are read under one connection lock, so a concurrent write +/// cannot land between them. +/// +/// An empty `kinds` means *no kind filter*, not *no kinds*. That is the +/// deliberate opposite of the source allowlist, which denies on empty: a scope +/// is a gate and fails closed, while this is a narrowing and its empty form is +/// what a caller that built the list from nothing passes. Widening here shows +/// rows the caller could already see; failing closed would silently empty a +/// page instead. +pub fn node_entity_rows( + config: &Config, + node_ids: &[String], + kinds: &[String], +) -> Result> { + if node_ids.is_empty() { + return Ok(Vec::new()); + } let memory = memory_config_from(config, config.workspace_dir().clone()); let connection = crate::engine::backend::chunks::shared_connection(&memory)?; let guard = connection.lock(); - let mut statement = guard.prepare( - "SELECT entity_id, entity_kind, surface, COUNT(*) - FROM mem_tree_entity_index - WHERE node_id = ?1 - GROUP BY entity_id, entity_kind, surface - ORDER BY COUNT(*) DESC, entity_id ASC", - )?; - let rows = statement - .query_map(rusqlite::params![node_id], index_row)? - .collect::>>()?; + let mut rows = Vec::with_capacity(node_ids.len()); + for window in node_ids.chunks(MAX_NODE_BATCH) { + let nodes = placeholders(window.len()); + let kind_clause = if kinds.is_empty() { + String::new() + } else { + format!(" AND entity_kind IN ({})", placeholders(kinds.len())) + }; + // `node_id` leads the sort so a caller re-mapping the result by node + // sees each node's rows contiguously; within a node the order is the + // single-node query's, unchanged. + let sql = format!( + "SELECT node_id, entity_id, entity_kind, surface, COUNT(*) + FROM mem_tree_entity_index + WHERE node_id IN ({nodes}){kind_clause} + GROUP BY node_id, entity_id, entity_kind, surface + ORDER BY node_id ASC, COUNT(*) DESC, entity_id ASC" + ); + let bound = window + .iter() + .chain(kinds.iter()) + .map(|value| rusqlite::types::Value::Text(value.clone())) + .collect::>(); + let mut statement = guard.prepare(&sql)?; + let window_rows = statement + .query_map(rusqlite::params_from_iter(bound), |row| { + let mentions: i64 = row.get(4)?; + Ok(NodeEntityRow { + node_id: row.get(0)?, + entity_id: row.get(1)?, + entity_kind: row.get(2)?, + surface: row.get(3)?, + mentions: u32::try_from(mentions.max(0)).unwrap_or(u32::MAX), + }) + })? + .collect::>>()?; + rows.extend(window_rows); + } Ok(rows) } +/// `?,?,…` for `count` bound values. +fn placeholders(count: usize) -> String { + std::iter::repeat_n("?", count) + .collect::>() + .join(",") +} + /// Ids of the **leaf** nodes one entity was observed in, newest first. /// /// `node_kind = 'leaf'` is the filter the scorer's write path defines: it diff --git a/crates/tinymemory-core/src/store/retrieval/mod.rs b/crates/tinymemory-core/src/store/retrieval/mod.rs index 4c260a10..2940d646 100644 --- a/crates/tinymemory-core/src/store/retrieval/mod.rs +++ b/crates/tinymemory-core/src/store/retrieval/mod.rs @@ -132,6 +132,12 @@ impl RetrievalFacade { offset: None, source_scope: None, exclude_dropped: false, + // The six list/substring predicates the contract's filtered + // listing added are not part of a param-tag search: this path + // narrows by source, owner, time and tag only, and an empty + // predicate means unfiltered, so the defaults are the right + // answer rather than a placeholder. + ..Default::default() }; let rows = list_chunks(config, &query)?; let Some(required) = filters.tags_all_of.as_ref() else { diff --git a/crates/tinymemory-module/Cargo.lock b/crates/tinymemory-module/Cargo.lock index dbdd0562..e269f2c3 100644 --- a/crates/tinymemory-module/Cargo.lock +++ b/crates/tinymemory-module/Cargo.lock @@ -100,6 +100,15 @@ dependencies = [ "hybrid-array", ] +[[package]] +name = "block2" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdeb9d870516001442e364c5220d3574d2da8dc765554b4a617230d33fa58ef5" +dependencies = [ + "objc2", +] + [[package]] name = "bumpalo" version = "3.20.3" @@ -299,6 +308,16 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "dispatch2" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e0e367e4e7da84520dedcac1901e4da967309406d1e51017ae1abfb97adbd38" +dependencies = [ + "bitflags", + "objc2", +] + [[package]] name = "displaydoc" version = "0.2.7" @@ -957,6 +976,56 @@ dependencies = [ "autocfg", ] +[[package]] +name = "objc2" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a12a8ed07aefc768292f076dc3ac8c48f3781c8f2d5851dd3d98950e8c5a89f" +dependencies = [ + "objc2-encode", +] + +[[package]] +name = "objc2-contacts" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b034b578389f89a85c055eacc8d8b368be5f04a6c1b07f672bf3aec21d0ef621" +dependencies = [ + "block2", + "objc2", + "objc2-foundation", +] + +[[package]] +name = "objc2-core-foundation" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a180dd8642fa45cdb7dd721cd4c11b1cadd4929ce112ebd8b9f5803cc79d536" +dependencies = [ + "bitflags", + "dispatch2", + "objc2", +] + +[[package]] +name = "objc2-encode" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ef25abbcd74fb2609453eb695bd2f860d389e457f67dc17cafc8b8cbc89d0c33" + +[[package]] +name = "objc2-foundation" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3e0adef53c21f888deb4fa59fc59f7eb17404926ee8a6f59f5df0fd7f9f3272" +dependencies = [ + "bitflags", + "block2", + "libc", + "objc2", + "objc2-core-foundation", +] + [[package]] name = "once_cell" version = "1.21.4" @@ -1736,12 +1805,16 @@ version = "0.1.1" dependencies = [ "anyhow", "async-trait", + "block2", "chrono", "dirs 5.0.1", "futures", "git2", "hex", "log", + "objc2", + "objc2-contacts", + "objc2-foundation", "parking_lot", "rand", "regex", diff --git a/crates/tinymemory-module/Cargo.toml b/crates/tinymemory-module/Cargo.toml index 5c93e21d..d35bdffc 100644 --- a/crates/tinymemory-module/Cargo.toml +++ b/crates/tinymemory-module/Cargo.toml @@ -33,10 +33,25 @@ tinymemory = { path = "../tinymemory" } # loads this binary compiles neither. tinymemory-core = { path = "../tinymemory-core", features = ["memory-git"] } tinymemory-tinycortex = { path = "../tinymemory-tinycortex", features = ["memory-git"] } -# `people` is enabled here rather than inherited: the module serves the +# `contacts` is enabled here rather than inherited: the module serves the # `MemoryPeople` family directly off the engine's people store, so it needs the # gate on even though `tinymemory-core` only re-exports the domain. -tinycortex = { version = "0.1", features = ["people"] } +# +# It replaces `people` rather than joining it. Upstream declares +# `contacts = ["people", ...]`, so naming both would state the same requirement +# twice and invite one of them to be edited without the other. +# +# What the wider gate buys is `SeedFromAddressBook` actually seeding. Without +# `contacts`, `people::address_book`'s macOS implementation compiles out and the +# stub in its place returns an empty contact list — so a refresh reports success +# and imports nobody, which is the failure mode that looks like an empty address +# book rather than a missing feature. That is newly load-bearing: the shipping +# desktop build turns `contacts` on for the in-process engine it also boots, and +# the host's whole people domain is a glob re-export of that engine, so the day +# it is deleted macOS address-book seeding survives only if this module carries +# it. The four objc2 crates behind the gate sit under a macOS target table +# upstream, so a Linux or Windows artifact still compiles none of them. +tinycortex = { version = "0.1", features = ["contacts"] } tinyagents = { version = "2.1" } # TinyBus provides the typed service interface and the dynamic module host ABI. # Reached by path now that this crate is its own workspace root: the nested diff --git a/crates/tinymemory-module/src/host.rs b/crates/tinymemory-module/src/host.rs index b46674ae..264502d1 100644 --- a/crates/tinymemory-module/src/host.rs +++ b/crates/tinymemory-module/src/host.rs @@ -1,5 +1,6 @@ //! Host-owned runtime services used by the compiled memory engine. +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use async_trait::async_trait; @@ -124,6 +125,179 @@ pub(crate) fn install(connection: Connection) { tinymemory_core::nlp_host::set_nlp_host(host); } +// ── Seams no bus interface serves ─────────────────────────────────────────── +// +// This module serves five of the host's seams: the embedder and the chat model +// have host interfaces of their own, and the event sink, the error reporter and +// spaCy cross the bus through `BusRuntimeHost` above. The other four — +// `composio_host`, `config_loader`, and the two stubbed below — have no callback +// channel at all. +// +// `config_loader` survives that honestly: every path returns +// `Err("no ConfigLoader installed …")`, so a module-mode loop fails with a named +// cause instead of quietly keeping a stale snapshot. `composio_host` is loud on +// the two paths that do work — `list_connections` and `execute` — and quiet on +// its two probes: unwired, `api_key` reads `None` and `is_available` reads +// `false`, which a caller cannot tell apart from "the user has no Composio +// connections". +// +// **The two below were quiet on every path.** With nothing installed, +// `scheduler_gate::current_policy()` reads `Normal` and `wait_for_capacity()` +// returns instantly — background LLM work runs flat out no matter what the user +// asked for — and `shutdown::register` drops the ingest queue's lock-release +// hook behind a `log::debug!`. That is precisely the outcome the host's own +// `install_memory_host_seams` comment says the seams exist to prevent: a sync +// run that looks empty rather than broken. Silence is the bug; these stubs are +// the fix. The two `composio_host` probes are the same bug one size down, and +// are left alone here only because closing them means inventing a bus interface +// this crate owns one half of. +// +// # Why stubs and not bus proxies +// +// Not a surface-budget choice — the trait shapes rule a proxy out. +// `SchedulerGate::current_policy` is sync and a bus call is not; `resume_notify` +// hands back a `tokio::sync::Notify`, a runtime primitive with no wire form; and +// `ShutdownHost::register` takes a Rust closure, which cannot be serialised at +// all. Mirroring the policy locally would need a signal this module does not +// declare (`signals = []`), a host half this crate does not own, and a cache +// that is wrong between ticks. +// +// # Why not synthesise a policy from the module's own config +// +// `ModuleConfig` carries `SchedulerGateConfig`, so `mode = off` looks +// answerable from here. It is not. The gate is *live* state — the user's toggle, +// AC power, CPU pressure, whether anyone is signed in — and the module holds the +// config it was loaded with. A module that paused itself on a stale `off` would +// stay paused after the user switched background AI back on, with no channel to +// learn otherwise and no way out short of a restart. Guessing is worse than not +// answering. +// +// # So: identical behaviour, no longer silent +// +// Each stub returns exactly what the unwired path returned, so installing them +// changes no scheduling and wedges nothing, and each reports once per process +// the first time anything actually consults it. Nothing in this process consults +// them today — the module starts none of the background loops (`queue::start`, +// `start_periodic_sync` and `start_workspace_periodic_sync` are all called +// host-side) — so the report fires on the day someone moves one in here, which +// is the day the gap stops being theoretical. +// +// Note also what is deliberately *not* built here: a module-local registry that +// banked shutdown hooks and drained them on the module's own `Shutdown` method. +// Nothing calls `MemoryProvider::shutdown()` on the way out of the host process, +// so those hooks would still never run — and they would stop reporting. That +// trades a loud gap for a quiet one. + +/// Latched so the gap is reported once per process, not once per job claim — +/// `wait_for_capacity` is consulted before every claim, and an unlatched report +/// would page on every poll. Same guard `queue::worker` puts on its own +/// storage-failure reports. +static GATE_REPORTED: AtomicBool = AtomicBool::new(false); + +/// Latched for the same reason: one hook dropped means every later one is too. +static SHUTDOWN_REPORTED: AtomicBool = AtomicBool::new(false); + +/// What the missing scheduler gate costs, in the terms a reader of the log needs. +const GATE_UNSERVED: &str = "scheduler gate unserved in module mode: background memory work in \ + this process runs ungated, ignoring the host's background-AI \ + throttle (user toggle, AC power, CPU pressure, signed-out)"; + +/// What the missing shutdown host costs. +const SHUTDOWN_UNSERVED: &str = "shutdown host unserved in module mode: a memory shutdown hook \ + was dropped, so in-flight queue job locks are not released on a \ + clean exit and the next launch waits out the lease instead"; + +/// Log and report an unserved seam once per process. +fn report_unserved_once(latch: &AtomicBool, message: &'static str, operation: &'static str) { + if latch.swap(true, Ordering::SeqCst) { + return; + } + log::error!("[tinymemory:module] {message}"); + // The error reporter reaches the host by spawning onto the module runtime, + // and two of the three call sites below are sync methods a future caller + // could reach from a plain thread. The log line above is unconditional; + // only the telemetry needs a runtime to exist, so the gap is never silent + // even when the report cannot be sent. + if tokio::runtime::Handle::try_current().is_ok() { + tinymemory_core::observability::report_error_or_expected( + message, + "memory", + operation, + &[("mode", "module")], + ); + } +} + +/// The host's background-AI throttle, which this module cannot observe. +/// +/// Answers exactly what an uninstalled gate answered — see the section comment +/// above for why it must not answer anything else — and says so out loud the +/// first time it is asked. +#[derive(Debug)] +pub(crate) struct UnservedSchedulerGate; + +#[async_trait] +impl tinymemory_core::scheduler_gate::SchedulerGate for UnservedSchedulerGate { + fn current_policy(&self) -> tinymemory_core::scheduler_gate::Policy { + report_unserved_once(&GATE_REPORTED, GATE_UNSERVED, "scheduler_gate"); + tinymemory_core::scheduler_gate::Policy::Normal + } + + fn resume_notify(&self) -> Arc { + report_unserved_once(&GATE_REPORTED, GATE_UNSERVED, "scheduler_gate"); + Arc::clone(IDLE_NOTIFY.get_or_init(|| Arc::new(tokio::sync::Notify::new()))) + } + + async fn wait_for_capacity(&self) -> Option> { + report_unserved_once(&GATE_REPORTED, GATE_UNSERVED, "scheduler_gate"); + None + } +} + +/// A `Notify` nobody ever fires. +/// +/// A `select!` on it simply never takes that arm, so the queue loops fall back +/// on their own tick cadence — which is what they did with no gate installed at +/// all. One per process rather than one per call, because every caller has to +/// receive the same handle for a wait on it to mean anything. +static IDLE_NOTIFY: std::sync::OnceLock> = std::sync::OnceLock::new(); + +/// The host's shutdown sequencer, which this module has no way to reach. +#[derive(Debug)] +pub(crate) struct UnservedShutdownHost; + +impl tinymemory_core::shutdown::ShutdownHost for UnservedShutdownHost { + fn register(&self, hook: tinymemory_core::shutdown::ShutdownHook) { + // Dropped, not banked: there is no moment inside this module at which it + // could be awaited. The hard-kill path is what remains — leases expire + // and startup recovery reclaims them — so this is a degradation, not a + // loss of data, and the report is classified accordingly. + drop(hook); + report_unserved_once(&SHUTDOWN_REPORTED, SHUTDOWN_UNSERVED, "shutdown_host"); + } +} + +/// Install the two seams this module can only stub, and name the gap at setup. +/// +/// Kept separate from [`install`] on purpose: that function wires the seams the +/// host genuinely serves over the bus, and folding these in would blur the +/// difference between "wired" and "wired to nothing". +pub(crate) fn install_unserved_seams() { + tinymemory_core::scheduler_gate::set_scheduler_gate(Arc::new(UnservedSchedulerGate)); + tinymemory_core::shutdown::set_shutdown_host(Arc::new(UnservedShutdownHost)); + // One line, once per process — `setup` runs exactly once. It is a warning + // rather than a debug line because in module mode this is true on every + // boot, and a reader of the log should not have to diff seam lists to find + // out that the throttle and the graceful lock release are not in effect. + log::warn!( + "[tinymemory:module] four host seams are unserved in module mode: composio_host and \ + config_loader are absent and their work paths fail with a named cause; scheduler_gate \ + and shutdown are stubs that keep the unwired behaviour and report once when consulted. \ + Background-AI throttling and graceful queue-lock release are not honoured inside this \ + process" + ); +} + #[cfg(test)] #[path = "host_test.rs"] mod test; diff --git a/crates/tinymemory-module/src/host_test.rs b/crates/tinymemory-module/src/host_test.rs index 1ec89dbf..b22bb34e 100644 --- a/crates/tinymemory-module/src/host_test.rs +++ b/crates/tinymemory-module/src/host_test.rs @@ -11,6 +11,8 @@ struct HostSeamsRestore { event_sink: Option>, error_reporter: Option>, nlp_host: Option>, + scheduler_gate: Option>, + shutdown_host: Option>, } impl HostSeamsRestore { @@ -19,6 +21,8 @@ impl HostSeamsRestore { event_sink: tinymemory_core::events::event_sink(), error_reporter: tinymemory_core::observability::error_reporter(), nlp_host: tinymemory_core::nlp_host::nlp_host(), + scheduler_gate: tinymemory_core::scheduler_gate::scheduler_gate(), + shutdown_host: tinymemory_core::shutdown::shutdown_host(), } } } @@ -37,6 +41,14 @@ impl Drop for HostSeamsRestore { Some(host) => tinymemory_core::nlp_host::set_nlp_host(host), None => tinymemory_core::nlp_host::clear_nlp_host(), } + match self.scheduler_gate.take() { + Some(gate) => tinymemory_core::scheduler_gate::set_scheduler_gate(gate), + None => tinymemory_core::scheduler_gate::clear_scheduler_gate(), + } + match self.shutdown_host.take() { + Some(host) => tinymemory_core::shutdown::set_shutdown_host(host), + None => tinymemory_core::shutdown::clear_shutdown_host(), + } } } @@ -237,16 +249,44 @@ async fn runtime_callbacks_and_spacy_cross_the_bus_with_their_full_payloads() { assert!(expected_error); } +/// Kept as one test rather than two on purpose: both installs write +/// process-global seams, and a second test that captured, installed and +/// asserted in parallel with this one could have its assertion land after this +/// one's `HostSeamsRestore` had already put the globals back. #[tokio::test] -async fn install_wires_all_three_runtime_host_seams() { +async fn install_wires_every_seam_this_module_can_supply() { let _restore = HostSeamsRestore::capture(); let (connection, _callbacks) = bus_with_runtime_host().await; + // The pair `setup` calls, in the order it calls them. super::install(connection); + super::install_unserved_seams(); assert!(tinymemory_core::events::event_sink().is_some()); assert!(tinymemory_core::observability::error_reporter().is_some()); assert!(tinymemory_core::nlp_host::nlp_host().is_some()); + // The two that used to be left out entirely, and so degraded in silence + // instead of failing with a named cause the way `config_loader` does. + assert!(tinymemory_core::scheduler_gate::scheduler_gate().is_some()); + assert!(tinymemory_core::shutdown::shutdown_host().is_some()); +} + +#[test] +fn the_unserved_stubs_answer_exactly_what_an_unwired_seam_answered() { + use tinymemory_core::scheduler_gate::SchedulerGate; + use tinymemory_core::shutdown::ShutdownHost; + + // Loud, not different. A stub that answered anything else would change + // scheduling as a side effect of loading the module — and with no channel + // to the host's live gate, any other answer would be a guess that goes + // stale the moment the user toggles background AI. + assert_eq!( + super::UnservedSchedulerGate.current_policy(), + tinymemory_core::scheduler_gate::Policy::Normal + ); + // Registering with nowhere to run reports and drops; it must never panic. + let hook: tinymemory_core::shutdown::ShutdownHook = Box::new(|| Box::pin(async {})); + super::UnservedShutdownHost.register(hook); } #[tokio::test] diff --git a/crates/tinymemory-module/src/lib.rs b/crates/tinymemory-module/src/lib.rs index 3412cb16..c82979e0 100644 --- a/crates/tinymemory-module/src/lib.rs +++ b/crates/tinymemory-module/src/lib.rs @@ -78,8 +78,9 @@ pub use embedding::{ pub use host::{RUNTIME_HOST_BUS_NAME, RUNTIME_HOST_INTERFACE, RUNTIME_HOST_OBJECT_PATH}; pub use service::{BUS_NAME, OBJECT_PATH}; +use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Arc; +use std::sync::{Arc, OnceLock}; use tinybus::{Connection, Error as BusError, Result as BusResult}; @@ -135,6 +136,12 @@ async fn setup(connection: Connection, mut config: ModuleConfig) -> BusResult<() &config, ))); host::install(connection.clone()); + // The seams no bus interface serves. Two of the four degraded in silence + // rather than with a named cause; see the section comment on + // `host::install_unserved_seams` for why they are stubbed here rather than + // proxied or synthesised. Installed with the rest, before the store exists, + // so nothing can consult a seam this process has not yet decided about. + host::install_unserved_seams(); let client = tinymemory_core::store::factories::create_memory_client_with_local_ai( &config.memory, @@ -153,10 +160,133 @@ async fn setup(connection: Connection, mut config: ModuleConfig) -> BusResult<() setup_error("create memory store") })?; + // After the store, never before: `queue::start` recovers stale locks as its + // first act, which opens the queue database, and the factory above is what + // creates the workspace it lives in. + start_queue_pool(&config); + let provider = provider::provider(&config, Arc::new(client)); service::serve(&connection, Arc::new(provider), config).await } +/// The workspace whose queue this process's worker pool drains. +/// +/// The pool is bound to one workspace — every `queue::store` entry point +/// resolves its database through `engine_config`, which roots at +/// `config.workspace_dir()` — while the `Once` inside `queue::start` is +/// process-global. Those two facts together are the trap this cell exists for: +/// a second `start` under a different workspace is not a second pool, it is a +/// silent no-op leaving that store's queue with nothing draining it. Recording +/// which workspace won makes that case loud instead of invisible. +static QUEUE_POOL_WORKSPACE: OnceLock = OnceLock::new(); + +/// What [`claim_queue_pool`] found when asked to start a pool. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum QueuePoolClaim { + /// Nothing had claimed the pool; this caller starts it. + Start, + /// A pool is already draining this workspace's queue, so there is nothing + /// to do and nothing wrong. + AlreadyDraining, + /// A pool is running, but rooted somewhere else. This store's queue has + /// nothing draining it and cannot be given a pool of its own. + Foreign, +} + +/// Decide whether this caller is the one that starts the pool. +/// +/// Split out from [`start_queue_pool`] so the decision can be asserted without +/// spawning four job workers and a daily scheduler into a test process, and +/// because `queue::start`'s own `Once` is not observable from here at all — a +/// second call to it is indistinguishable from a first that worked. +pub(crate) fn claim_queue_pool(workspace: &Path) -> QueuePoolClaim { + match QUEUE_POOL_WORKSPACE.set(workspace.to_path_buf()) { + Ok(()) => QueuePoolClaim::Start, + // `set` hands the rejected value back, so the comparison needs no + // second read and cannot race with a concurrent claim. + Err(rejected) => { + if QUEUE_POOL_WORKSPACE.get() == Some(&rejected) { + QueuePoolClaim::AlreadyDraining + } else { + QueuePoolClaim::Foreign + } + } + } +} + +/// Start the engine's queue worker pool for this process. +/// +/// # Why the module has to own this +/// +/// Every enqueue this driver makes is inert without a pool draining it, and the +/// enqueues are not incidental: `FlushPending` and `RetryFailed` schedule work +/// rather than doing it, the re-embed backfill is a queued job, and the ingest +/// path's `extract_chunk` is *how ingested content becomes retrievable at all*. +/// Until now the only `queue::start` call in any tree was the host's, made +/// against the second, in-process engine the host also booted. A host that +/// deletes that engine — which is the entire point of loading this module — +/// turns all four into permanent no-ops with no error anywhere: ingestion still +/// reports success and the content is simply never indexed. So the pool moves +/// in here, alongside the engine that needs it. +/// +/// # Two things it does not get in module mode +/// +/// Stated rather than hidden, because this is a real product degradation the +/// host does not have today. The pool consults +/// [`tinymemory_core::scheduler_gate`] before every claim and registers a +/// [`tinymemory_core::shutdown`] hook to release in-flight job locks. This +/// module serves neither seam — see the section comment on +/// `host::install_unserved_seams` for why neither can be proxied — so both are +/// stubs, and the consequences follow: +/// +/// - **It runs unthrottled.** `wait_for_capacity` returns immediately, so +/// background memory work in this process ignores the host's background-AI +/// throttle: the user's toggle, AC power, CPU pressure, signed-out. On a +/// laptop that means the queue drains at full tilt on battery, which the +/// host's in-process engine would not do. +/// - **Its shutdown hook is dropped.** A clean exit therefore leaves `running` +/// rows locked. They are reclaimed by lease expiry at the next start — +/// `recover_stale_locks` is the first thing `queue::start` does, and +/// `queue::worker` documents that as the hard-kill path — so the cost is one +/// lease of latency after a restart, not lost work. +/// +/// Closing either properly needs a `SchedulerGate` bus interface this crate +/// owns only one half of, which is separate work. Until then the stubs report +/// once per process the first time the pool consults them. +fn start_queue_pool(config: &ModuleConfig) { + match claim_queue_pool(&config.workspace_dir) { + QueuePoolClaim::Start => { + // Warn, not debug: it is true on every boot in module mode, and a + // reader of the log should not have to know which seams are stubbed + // to find out that the throttle is not in effect. + log::warn!( + "[tinymemory:module] starting the memory queue worker pool in this process. \ + It runs unthrottled — the scheduler gate is unserved here, so background \ + memory work ignores the host's background-AI throttle, AC power and CPU \ + pressure — and its graceful lock-release hook is dropped, so locks held at \ + exit are reclaimed by lease expiry on the next start" + ); + tinymemory_core::queue::start(Arc::new( + tinymemory_tinycortex::engine::EngineRuntimeConfig::from(config), + )); + } + QueuePoolClaim::AlreadyDraining => { + log::debug!( + "[tinymemory:module] the queue worker pool for this workspace is already running" + ); + } + QueuePoolClaim::Foreign => { + log::error!( + "[tinymemory:module] a queue worker pool is already running for a different \ + workspace in this process, and `queue::start` is guarded process-wide, so the \ + store just opened has nothing draining its queue: ingested content will not be \ + indexed and flushes and retries will not run. One module process serves one \ + workspace" + ); + } + } +} + /// Claim this process's single setup slot. /// /// `setup` installs **process-global** host callbacks, so it is not @@ -206,10 +336,18 @@ mod exports { tinybus_module::module_export! { setup = super::setup, config = super::ModuleConfig, - // Two, not one: a recall that triggers an embed makes an outbound call + // Eight, derived rather than picked. Two are the floor this module has + // always needed: a recall that triggers an embed makes an outbound call // while still inside its own inbound call, so a single worker would - // deadlock on the first semantic query. - worker_threads = 2, + // deadlock on the first semantic query. `setup` now also starts the + // engine's queue pool — four job workers plus the daily scheduler — and + // those five run the engine's SQLite claim and settle synchronously + // inside their async loops, so a busy one occupies a runtime thread + // outright instead of yielding it. Two plus five is seven; the eighth + // is what drives a job's own outbound embed while the rest are busy. At + // two, a draining queue would starve inbound dispatch and the module + // would stop answering recalls until the queue emptied. + worker_threads = 8, provides = ["ai.tinyhumans.tinymemory.Memory"], methods = [ "DriverId", @@ -249,6 +387,8 @@ mod exports { "StorageKinds", "ChunkEmbeddings", "CountChunks", + "ListChunkDetails", + "SourceTotals", // Retrieval. "FastRetrieve", "CoverWindow", @@ -309,6 +449,7 @@ mod exports { "DeleteToolRule", "AcceptSourceItems", "ForgetSource", + "ForgetMatching", "Reembed", "Compact", "Consolidate", @@ -320,6 +461,7 @@ mod exports { "BackfillInProgress", "FlushPending", "ResetDerivedIndex", + "PurgeAll", "RecallNamespaceRecent", // Tree, structural: the forest walk and its leaf edge. "SummaryForest", diff --git a/crates/tinymemory-module/src/service/mod.rs b/crates/tinymemory-module/src/service/mod.rs index eb4b609c..c5c12a51 100644 --- a/crates/tinymemory-module/src/service/mod.rs +++ b/crates/tinymemory-module/src/service/mod.rs @@ -33,6 +33,8 @@ //! ChunkDetail(chunk_id) -> Option //! ChunkEmbeddings(chunk_ids, model_signature) -> [ChunkEmbedding] //! StorageKinds() -> [String] +//! ListChunkDetails(query, scope) -> [ChunkListRow] +//! SourceTotals(limit, scope) -> [SourceTotal] //! //! ListActiveFacets() / ListAllFacets() -> [ProfileFacet] //! GetFacet(key) / FacetsByType(type) -> facet(s) @@ -53,8 +55,11 @@ //! RecentLeaves(limit, scope) -> [TreeLeaf] //! //! TopEntities(kind, limit) -> [EntityOccurrence] -//! ChunkEntities(chunk_id) -> [EntityOccurrence] +//! ChunkEntities(chunk_ids, kinds) -> [ChunkEntityOccurrence] //! EntityChunkIds(entity_id, limit) -> [String] +//! +//! ForgetMatching(selector) -> ForgetOutcome +//! PurgeAll() -> PurgeOutcome //! ``` //! //! # Source scope crosses as an argument, never as ambient state @@ -128,14 +133,17 @@ use tinymemory_api::error::MemoryError; use tinymemory_api::goals::GoalsDoc; use tinymemory_api::health::MemoryHealth; use tinymemory_api::provider::types::{ - DiffReport, EntityHit, EntityOccurrence, ExportPage, ExportRecord, FlushOutcome, ImportOutcome, - IngestItem, IngestOutcome, MaintenanceReport, QueueFailure, QueueStats, ResetOutcome, - SnapshotRef, SourceItem, SourceScope, StoreStats, + ChunkEntityOccurrence, DiffReport, EntityHit, EntityOccurrence, ExportPage, ExportRecord, + FlushOutcome, ForgetOutcome, ForgetSelector, ImportOutcome, IngestItem, IngestOutcome, + MaintenanceReport, PurgeOutcome, QueueFailure, QueueStats, ResetOutcome, SnapshotRef, + SourceItem, SourceScope, StoreStats, }; // `MemoryCore`, `MemoryRecall` and `MemoryPortability` are deliberately not // imported: they are supertraits of `MemoryProvider`, so their methods are // already callable on the trait object. -use tinymemory_api::provider::chunks::{ChunkDetail, ChunkEmbedding, ChunkQuery}; +use tinymemory_api::provider::chunks::{ + ChunkDetail, ChunkEmbedding, ChunkListRow, ChunkQuery, SourceTotal, +}; use tinymemory_api::provider::episodic::{ConversationSegment, EpisodicEvent, EpisodicTurn}; use tinymemory_api::provider::people::{ AddressBookSeedOutcome, PersonHandle, PersonInteraction, PersonRecord, PersonScore, @@ -411,6 +419,25 @@ impl MemoryService { } })?; + // No queue worker pool is started for this store, and that is a finding + // rather than an omission. The engine's queue is rooted at the + // workspace, not at the store subtree: every `queue::store` entry point + // resolves its database through `engine_config`, which is + // `memory_config_from(config, config.workspace_dir())`, while + // `memory_subdir` reaches only `UnifiedMemory::new_with_memory_dir`. One + // module process serves one workspace — `claim_process_setup` refuses a + // second `setup` — so every store opened here shares the one queue + // `setup` already started a pool for, and a second `queue::start` would + // be a silent no-op besides: its guard is a process-global `Once`. + // + // Calling `crate::start_queue_pool` here anyway would be correct and + // would make that invariant enforced rather than argued. It is left out + // because it would start a real four-worker pool inside the unit tests + // that exercise this method, whose workspaces are temporary directories + // deleted while the workers still poll them — the workers then mark the + // store degraded process-wide, which later tests read. The invariant is + // asserted instead by `crate::claim_queue_pool`, which is the same + // decision without the tasks. let provider = crate::provider::provider(&opener.config, Arc::new(client)); opener.instrumentation.before_registration()?; opener @@ -1470,15 +1497,36 @@ impl MemoryService { Ok(rows) } - /// Every entity indexed against one chunk. + /// Every entity indexed against a batch of chunks. + /// + /// The batch is the point. The caller this exists for is drawing a graph + /// over a page of chunks, and one id per call is one round trip per chunk — + /// a page of a thousand becomes a thousand calls for a single view. + /// `kinds` narrows to the kinds that caller will actually render, so the + /// frame carries the rows it asked for instead of the whole index of every + /// chunk in the page. + /// + /// Rows come back as [`ChunkEntityOccurrence`] rather than + /// [`EntityOccurrence`] because over a batch a flat list has no other way + /// back to the chunk each row describes — see the contract, which says to + /// group by `chunk_id` and never index by position. + /// + /// Widening the arguments is only legitimate because this member has never + /// shipped: it was added on this branch, so no released host calls the + /// single-id form. Its position in the member sequence is unchanged, which + /// is what the drift assertion pins. /// /// Size-checked even though the contract gives it no `limit`: the bound is - /// one chunk's extraction, which is the driver's number rather than the - /// caller's, and an over-large frame the host cannot decode is a worse - /// answer than a named refusal naming the method. - async fn chunk_entities(&self, chunk_id: String) -> BusResult> { + /// the extraction of the chunks named, which is the driver's number rather + /// than the caller's, and an over-large frame the host cannot decode is a + /// worse answer than a named refusal naming the method. + async fn chunk_entities( + &self, + chunk_ids: Vec, + kinds: Option>, + ) -> BusResult> { let rows = require_family!(self, as_entities, Capability::Entities) - .chunk_entities(&chunk_id) + .chunk_entities(&chunk_ids, kinds.as_deref()) .await .map_err(|error| into_bus_error(&error))?; ensure_response_fits(&rows, "ChunkEntities")?; @@ -1530,6 +1578,88 @@ impl MemoryService { ensure_response_fits(&leaves, "RecentLeaves")?; Ok(leaves) } + + /// What `ChunkDetail` returns, for a whole page in one read. + /// + /// Appended here rather than filed beside `ListChunks` for the reason + /// `count_chunks` gives above: member order is wire order. + /// + /// It is not `ChunkDetail` in a loop, and the difference is not stylistic. + /// One detail is several engine reads, so a thousand-row page done that way + /// is several thousand queries behind a thousand round trips. Sharing + /// `ListChunks`' own filter is the other half: a page and the details + /// describing it cannot disagree about which chunks are in it. + /// + /// Size-checked, and it is the chunk method most likely to trip the + /// ceiling: a row carries chunk text, so the limit that bounds rows does + /// not bound bytes. + async fn list_chunk_details( + &self, + query: ChunkQuery, + scope: Option, + ) -> BusResult> { + let rows = require_family!(self, as_chunks, Capability::Chunks) + .list_chunk_details(&query, scope.as_ref()) + .await + .map_err(|error| into_bus_error(&error))?; + ensure_response_fits(&rows, "ListChunkDetails")?; + Ok(rows) + } + + /// One row per source, with what that source put in the store. + /// + /// Aggregated by the driver because the alternative is listing every chunk + /// and grouping caller-side, which crosses the whole store to compute a + /// handful of counts — and crosses it as content, which is what the + /// response ceiling is there to stop. + /// + /// Size-checked for the same reason `TopEntities` is: `limit` bounds rows, + /// and the ceiling is a property of the frame rather than of the row count. + async fn source_totals( + &self, + limit: usize, + scope: Option, + ) -> BusResult> { + let totals = require_family!(self, as_chunks, Capability::Chunks) + .source_totals(limit, scope.as_ref()) + .await + .map_err(|error| into_bus_error(&error))?; + ensure_response_fits(&totals, "SourceTotals")?; + Ok(totals) + } + + /// Forget everything one selector names. + /// + /// One door rather than one member per shape — a chunk, a source, a source + /// prefix, an owner. The four deletions differ only in which rows they + /// match, and four members would be four chances for one of them to leave + /// behind a side table the others clear. + /// + /// Not size-checked: the response counts what went, and no selector can + /// make a count bigger. + async fn forget_matching(&self, selector: ForgetSelector) -> BusResult { + require_family!(self, as_sources, Capability::Sources) + .forget_matching(&selector) + .await + .map_err(|error| into_bus_error(&error)) + } + + /// Erase every row this driver holds. + /// + /// Filed under maintenance rather than sources because it is scoped to no + /// source: it is the "wipe this store" a host offers behind a confirmation, + /// and the driver's half of that is every table at once. What it does not + /// touch is the filesystem — the content directory belongs to the host, and + /// a driver deleting host directories would be reaching past its own + /// storage into somewhere it cannot reason about. + /// + /// Not size-checked, for the reason `forget_matching` gives above. + async fn purge_all(&self) -> BusResult { + require_family!(self, as_maintenance, Capability::Maintenance) + .purge_all() + .await + .map_err(|error| into_bus_error(&error)) + } } /// The response-size ceiling for a method that returns a list of entries. diff --git a/crates/tinymemory-module/src/service/test.rs b/crates/tinymemory-module/src/service/test.rs index 02bfb2b2..62b86764 100644 --- a/crates/tinymemory-module/src/service/test.rs +++ b/crates/tinymemory-module/src/service/test.rs @@ -451,6 +451,77 @@ async fn the_open_store_cap_is_reached_through_successful_opens() { ); } +/// The queue worker pool is claimed once per process, and a store under a +/// second workspace is refused loudly rather than left with no pool. +/// +/// Asserted through `claim_queue_pool` rather than `start_queue_pool` on +/// purpose. Starting the pool for real spawns four job workers and a daily +/// scheduler against a temporary directory the test deletes while they are +/// still polling it; they then mark the store degraded process-wide, which +/// every later test that reads health would inherit. The claim is the whole of +/// the decision — what follows it is one call into `tinymemory-core`, whose own +/// `Once` guards it a second time. +/// +/// The three outcomes are asserted in one test because the cell behind them is +/// a process-global `OnceLock`: split across three tests they would race, and +/// only the first to run would see `Start`. +#[test] +fn the_queue_pool_is_claimed_once_and_a_foreign_workspace_is_refused() { + let workspace = std::path::Path::new("/tinymemory-module/queue-pool-claim"); + let elsewhere = std::path::Path::new("/tinymemory-module/queue-pool-elsewhere"); + + assert_eq!( + crate::claim_queue_pool(workspace), + crate::QueuePoolClaim::Start, + "the first claim must be the one that starts the pool" + ); + assert_eq!( + crate::claim_queue_pool(workspace), + crate::QueuePoolClaim::AlreadyDraining, + "a second claim for the same workspace must not start a second pool" + ); + assert_eq!( + crate::claim_queue_pool(elsewhere), + crate::QueuePoolClaim::Foreign, + "a claim for another workspace must be named, not silently swallowed — \ + `queue::start` would no-op and that store's queue would never drain" + ); +} + +/// A second store opens normally, and needs no pool of its own to do it. +/// +/// The pairing with the test above is the point. `queue::start` is guarded by a +/// process-global `Once`, so the obvious failure of moving the pool into the +/// module is a second store silently getting no worker at all. It cannot happen +/// here: the engine's queue is rooted at the workspace — `queue::store` resolves +/// its database through `engine_config`, which is `memory_config_from(config, +/// config.workspace_dir())` — while `memory_subdir` reaches only +/// `UnifiedMemory::new_with_memory_dir`. Both stores below therefore share the +/// one queue `setup` started a pool for. +#[tokio::test] +async fn a_second_store_opens_under_the_one_workspace_queue() { + use std::sync::Arc; + + let workspace = tempfile::tempdir().expect("tempdir"); + let connection = test_connection().await; + let config = test_config(workspace.path()); + let _embedding_host = EmbeddingHostRestore::install(connection.clone(), &config); + let opener = test_opener(connection, config); + let service = super::MemoryService::root(test_provider(), Arc::clone(&opener)); + + let first = service + .open_store("profile-one".to_string()) + .await + .expect("the first store must open"); + let second = service + .open_store("profile-two".to_string()) + .await + .expect("a second store must open rather than panic or be refused"); + + assert_ne!(first, second, "each subtree gets its own object path"); + assert_eq!(opener.served.lock().await.len(), 2); +} + /// Every method the service implements must also be declared in the manifest. /// /// The manifest's `methods` list is admission surface: the host may only call a @@ -544,7 +615,7 @@ fn the_served_members_are_exactly_the_published_contract() { .map(|member| (*member).to_string()) .collect(); - // Reported as differences rather than as a 97-element inequality, so the + // Reported as differences rather than as a 109-element inequality, so the // failure names the method that moved instead of printing both lists. let missing: Vec<&String> = served.iter().filter(|m| !published.contains(m)).collect(); assert!( diff --git a/crates/tinymemory-module/tests/module_e2e.rs b/crates/tinymemory-module/tests/module_e2e.rs index 573e6678..02608fcb 100644 --- a/crates/tinymemory-module/tests/module_e2e.rs +++ b/crates/tinymemory-module/tests/module_e2e.rs @@ -42,7 +42,7 @@ use tinybus::broker::Broker; use tinybus::module::ModuleHost; use tinybus::transport::memory::MemoryBus; -use tinybus::{Connection, Result as BusResult}; +use tinybus::{Connection, Error as BusError, Result as BusResult}; use tinymemory_api::capabilities::{Capabilities, Capability}; use tinymemory_api::types::{MemoryCategory, MemoryEntry, MemoryTaint}; use tinymemory_module::{ @@ -615,6 +615,8 @@ const EXPECTED_METHODS: &[&str] = &[ "StorageKinds", "ChunkEmbeddings", "CountChunks", + "ListChunkDetails", + "SourceTotals", // Retrieval. "FastRetrieve", "CoverWindow", @@ -673,6 +675,7 @@ const EXPECTED_METHODS: &[&str] = &[ "DeleteToolRule", "AcceptSourceItems", "ForgetSource", + "ForgetMatching", "Reembed", "Compact", "Consolidate", @@ -684,6 +687,7 @@ const EXPECTED_METHODS: &[&str] = &[ "BackfillInProgress", "FlushPending", "ResetDerivedIndex", + "PurgeAll", "RecallNamespaceRecent", "SummaryForest", "RecentLeaves", @@ -1047,10 +1051,31 @@ async fn people_and_profile_round_trip(bus: &tinybus::Proxy) { .call("ListPeople", (Some(8_usize),)) .await .expect("ListPeople"); - let _: tinymemory_api::provider::people::AddressBookSeedOutcome = bus - .call("SeedFromAddressBook", ()) - .await - .expect("SeedFromAddressBook"); + // `SeedFromAddressBook` is the one member here that reaches outside the + // process for its answer: with `contacts` on it opens the platform address + // book, and on macOS that is a per-application privacy grant the test + // runner may not hold. A denial is the address book answering, not the + // module failing to route, so the assertion is that the call reaches the + // driver and comes back under a contract error — never that this host + // happens to have granted Contacts access. + match bus + .call::("SeedFromAddressBook", ()) + .await + { + Ok(_) => {} + Err(BusError::MethodFailed { name, message }) + if message.contains("contacts access denied") => + { + assert!( + name.starts_with("ai.tinyhumans.tinymemory.Error."), + "a denial must still come back under a contract error name, got {name}" + ); + eprintln!( + "SeedFromAddressBook: address book access not granted on this host — {message}" + ); + } + Err(error) => panic!("SeedFromAddressBook: {error:?}"), + } bus.call::<()>( "UpsertProviderFacet", @@ -1175,7 +1200,9 @@ async fn query_and_maintenance_families_dispatch_typed_requests() { )] async fn ingest_and_chunks_round_trip(bus: &tinybus::Proxy) -> String { use tinymemory_api::chunks::DataSource; - use tinymemory_api::provider::chunks::{ChunkDetail, ChunkEmbedding, ChunkQuery}; + use tinymemory_api::provider::chunks::{ + ChunkDetail, ChunkEmbedding, ChunkListRow, ChunkQuery, SourceTotal, + }; use tinymemory_api::provider::types::{IngestItem, IngestOutcome}; let ingest = IngestItem { @@ -1328,6 +1355,39 @@ async fn ingest_and_chunks_round_trip(bus: &tinybus::Proxy) -> String { "CountChunks must agree with the ListChunks page it accompanies" ); + // The detail list answers the page's own filter in one read rather than in + // a `ChunkDetail` loop, so it has to describe exactly the page `ListChunks` + // returned. A detail list built on a second predicate would not. + let details: Vec = bus + .call( + "ListChunkDetails", + ( + ChunkQuery::default(), + Option::::None, + ), + ) + .await + .expect("ListChunkDetails"); + assert_eq!( + details.len(), + chunks.len(), + "ListChunkDetails must describe the same page ListChunks returned" + ); + + // Shape over the wire is what is under test here — a workspace with one + // source is a legitimate answer — so this asserts the decode and the + // argument tuple, which is what a host gets wrong. + let _: Vec = bus + .call( + "SourceTotals", + ( + 16_usize, + Option::::None, + ), + ) + .await + .expect("SourceTotals"); + chunk_id } @@ -1479,8 +1539,11 @@ async fn tree_and_entities_round_trip(bus: &tinybus::Proxy) { .call("TopEntities", (Option::::None, 8_usize)) .await .expect("TopEntities"); - let _: Vec = bus - .call("ChunkEntities", ("chunk-1",)) + let _: Vec = bus + .call( + "ChunkEntities", + (vec!["chunk-1".to_string()], Option::>::None), + ) .await .expect("ChunkEntities"); let _: Vec = bus diff --git a/crates/tinymemory-tinycortex/src/engine/mod.rs b/crates/tinymemory-tinycortex/src/engine/mod.rs index 9631a384..5ebc3982 100644 --- a/crates/tinymemory-tinycortex/src/engine/mod.rs +++ b/crates/tinymemory-tinycortex/src/engine/mod.rs @@ -32,23 +32,24 @@ use tinymemory_api::host::{ }; use tinymemory_api::mandatory::MemoryTraitProvider; use tinymemory_api::provider::types::{ - EntityHit, EntityOccurrence, EntityRef, ExportPage, ExportRecord, FlushOutcome, ImportOutcome, - IngestItem, IngestOutcome, MaintenanceReport, QueueFailure, QueueStats, ResetOutcome, - SourceItem, SourceScope, StoreStats, + ChunkEntityOccurrence, EntityHit, EntityOccurrence, EntityRef, ExportPage, ExportRecord, + FlushOutcome, ForgetOutcome, ForgetSelector, ImportOutcome, IngestItem, IngestOutcome, + MaintenanceReport, PurgeOutcome, QueueFailure, QueueStats, ResetOutcome, SourceItem, + SourceScope, StoreStats, }; // Diff-family value types, used only by the `MemoryDiff` impl below — which is // compiled out without the git-backed snapshot store. #[cfg(feature = "memory-git")] use tinymemory_api::provider::types::{ChangeKind, DiffReport, SnapshotRef, SourceChange}; use tinymemory_api::provider::{ - AddressBookSeedOutcome, ChunkDetail, ChunkEmbedding, ChunkQuery, ConversationSegment, - CoverWindowQuery, EntityMatch, EpisodicEvent, EpisodicTurn, EventKind, FacetType, - FastRetrieveQuery, MemoryChunks, MemoryCore, MemoryDiff, MemoryDocuments, MemoryEntities, - MemoryEpisodic, MemoryGoals, MemoryGraph, MemoryIngest, MemoryMaintenance, MemoryPeople, - MemoryPortability, MemoryProfile, MemoryProvider, MemoryRecall, MemoryRetrieval, + AddressBookSeedOutcome, ChunkDetail, ChunkEmbedding, ChunkListRow, ChunkQuery, + ConversationSegment, CoverWindowQuery, EntityMatch, EpisodicEvent, EpisodicTurn, EventKind, + FacetType, FastRetrieveQuery, MemoryChunks, MemoryCore, MemoryDiff, MemoryDocuments, + MemoryEntities, MemoryEpisodic, MemoryGoals, MemoryGraph, MemoryIngest, MemoryMaintenance, + MemoryPeople, MemoryPortability, MemoryProfile, MemoryProvider, MemoryRecall, MemoryRetrieval, MemorySourceSink, MemoryToolMemory, MemoryTree, PersonHandle, PersonInteraction, PersonRecord, PersonScore, ProfileFacet, RankedPerson, ResolvedPerson, RetrievalHit, RetrievalResponse, - SourceRetrievalQuery, UserState, + SourceRetrievalQuery, SourceTotal, UserState, }; use tinymemory_api::recall::OwnedRecallOpts; use tinymemory_api::tool_memory::ToolMemoryRule; @@ -1144,6 +1145,25 @@ impl MemoryTree for TinycortexProvider { } } +/// Validate entity-kind wire strings and re-emit them in the index's spelling. +/// +/// The same parser and the same rule `MemoryEntities::top_entities` applies to +/// its single kind: an unrecognised one is an error rather than a filter that +/// matches nothing, because a misspelling and an empty index produce the same +/// empty answer and the caller acts on it either way. Re-emitting `as_str` +/// settles the spelling on the one the index writes, so a kind that reached +/// the caller through some other vocabulary still matches. +fn canonical_entity_kinds(kinds: &[String]) -> Result, MemoryError> { + kinds + .iter() + .map(|kind| { + tinymemory_core::tree::score::extract::EntityKind::parse(kind) + .map(|parsed| parsed.as_str().to_string()) + .map_err(|_| MemoryError::Invalid(format!("unknown entity kind: {kind}"))) + }) + .collect() +} + #[async_trait] impl MemoryEntities for TinycortexProvider { async fn entities( @@ -1294,19 +1314,42 @@ impl MemoryEntities for TinycortexProvider { .collect()) } - async fn chunk_entities(&self, chunk_id: &str) -> Result, MemoryError> { - let node_id = chunk_id.to_string(); + async fn chunk_entities( + &self, + chunk_ids: &[String], + kinds: Option<&[String]>, + ) -> Result, MemoryError> { + let kinds = match kinds { + // No filter. The store spells that as an empty kind list, the same + // way every plural `ChunkQuery` filter does. + None => Vec::new(), + // A filter admitting no kind, which the contract answers with an + // empty vector. It has to be answered here rather than passed + // down, because the store would read the same empty list as + // *unfiltered* — the right reading there and the wrong one for an + // `Option` whose `None` already says "no filter". The two readings + // meet at this line and nowhere else. + Some([]) => return Ok(Vec::new()), + Some(kinds) => canonical_entity_kinds(kinds)?, + }; + let node_ids = chunk_ids.to_vec(); let rows = blocking(self.config.clone(), "read chunk entities", move |config| { - tinymemory_core::store::entities::node_entity_rows(config, &node_id) + tinymemory_core::store::entities::node_entity_rows(config, &node_ids, &kinds) }) .await?; Ok(rows .into_iter() - .map(|row| EntityOccurrence { - entity_id: row.entity_id, - kind: row.entity_kind, - surface: row.surface, - mentions: row.mentions, + .map(|row| ChunkEntityOccurrence { + // The index calls this a node id because summaries live in the + // same table; every row here came back under an id the caller + // asked for, so tagging it as the chunk is not a widening. + chunk_id: row.node_id, + occurrence: EntityOccurrence { + entity_id: row.entity_id, + kind: row.entity_kind, + surface: row.surface, + mentions: row.mentions, + }, }) .collect()) } @@ -1418,6 +1461,20 @@ impl MemoryDiff for TinycortexProvider { } } +/// Read a contract source-kind wire string in the engine's vocabulary. +/// +/// The contract carries the kind as text because the host's sync machinery +/// grows kinds without a contract change; this engine stores three of them and +/// has to say so rather than match nothing. A rejected kind is +/// [`MemoryError::Invalid`] and never an outcome of zero — on the delete paths +/// this serves, a zero would tell an operator their content was already gone +/// when nothing had been looked at. +fn parse_source_kind( + kind: &str, +) -> Result { + tinymemory_core::store::chunks::SourceKind::parse(kind).map_err(MemoryError::Invalid) +} + #[async_trait] impl MemorySourceSink for TinycortexProvider { async fn accept_source_items( @@ -1516,6 +1573,93 @@ impl MemorySourceSink for TinycortexProvider { .await?; Ok(u64::try_from(documents.saturating_add(chunks)).unwrap_or(u64::MAX)) } + + async fn forget_matching( + &self, + selector: &ForgetSelector, + ) -> Result { + // The kind arrives as a wire string and is parsed before any delete + // runs, rather than inside the blocking closure: that closure's error + // channel is `anyhow`, which would surface a kind this driver does not + // recognise as a store failure. On a destructive call the difference + // decides what an operator does next — retry, or fix the argument. + // + // `trees_cleaned` is only ever non-zero for the exact-source arm, and + // that is a property of the engine rather than an omission here. Its + // delete already cascades the trees whose scope it emptied; the extra + // sweep is the *legacy* cleanup for a source whose chunks went in an + // earlier, partial delete and left a tree behind. That question can + // only be asked of one source id — a prefix or an owner names a set, + // and there is no orphaned scope to name for a set. + let removed = |count: usize| u64::try_from(count).unwrap_or(u64::MAX); + let outcome = match selector { + ForgetSelector::Chunk { chunk_id } => { + let chunk_id = chunk_id.clone(); + let count = blocking(self.config.clone(), "forget one chunk", move |config| { + tinymemory_core::store::chunks::delete_chunk_by_id(config, &chunk_id) + }) + .await?; + ForgetOutcome { + chunks_removed: removed(count), + trees_cleaned: 0, + } + } + ForgetSelector::Source { + source_kind, + source_id, + } => { + let kind = parse_source_kind(source_kind)?; + let source_id = source_id.clone(); + blocking(self.config.clone(), "forget one source", move |config| { + let count = tinymemory_core::store::chunks::delete_chunks_by_source( + config, kind, &source_id, + )?; + let cleaned = tinymemory_core::store::chunks::delete_orphaned_source_tree( + config, kind, &source_id, + )?; + Ok(ForgetOutcome { + chunks_removed: u64::try_from(count).unwrap_or(u64::MAX), + trees_cleaned: u64::from(cleaned), + }) + }) + .await? + } + ForgetSelector::SourcePrefix { + source_kind, + source_id_prefix, + } => { + let kind = parse_source_kind(source_kind)?; + let prefix = source_id_prefix.clone(); + let count = blocking( + self.config.clone(), + "forget a source prefix", + move |config| { + tinymemory_core::store::chunks::delete_chunks_by_source_prefix( + config, kind, &prefix, + ) + }, + ) + .await?; + ForgetOutcome { + chunks_removed: removed(count), + trees_cleaned: 0, + } + } + ForgetSelector::Owner { source_kind, owner } => { + let kind = parse_source_kind(source_kind)?; + let owner = owner.clone(); + let count = blocking(self.config.clone(), "forget one owner", move |config| { + tinymemory_core::store::chunks::delete_chunks_by_owner(config, kind, &owner) + }) + .await?; + ForgetOutcome { + chunks_removed: removed(count), + trees_cleaned: 0, + } + } + }; + Ok(outcome) + } } #[async_trait] @@ -1846,6 +1990,26 @@ impl MemoryMaintenance for TinycortexProvider { .await } + async fn purge_all(&self) -> Result { + // The opposite end of the scale from `reset_derived_index` above, and + // the contrast is the whole reason both exist: that one is guaranteed + // to keep `mem_tree_chunks`, this one is guaranteed not to. Neither is + // a safer spelling of the other, so a caller has to pick, and the two + // names say which it picked. + // + // The database is the whole of this call. Content files on disk stay + // the caller's — the vault root is a host path the driver is handed, + // and a driver deleting directories under it would be acting on + // filesystem policy it does not own. + let chunks_removed = blocking(self.config.clone(), "purge the store", move |config| { + tinymemory_core::store::chunks::purge_all(config) + }) + .await?; + Ok(PurgeOutcome { + rows_deleted: u64::try_from(chunks_removed).unwrap_or(u64::MAX), + }) + } + async fn backfill_in_progress(&self) -> Result { // A process-global the backfill chain owns, not a column — and not one // this engine can narrow, since `tinymemory_core::queue` tracks the @@ -2269,21 +2433,39 @@ fn scope_to_engine(scope: Option<&SourceScope>) -> Option> { /// Convert a contract chunk query into the engine's. /// -/// Shared by `list_chunks` and `count_chunks` so the two ask the engine the -/// same question. Two copies of this conversion would compile identically -/// today and drift the first time a filter is added to one of them — and the -/// symptom of that drift is a total that no amount of paging can reach. +/// Shared by `list_chunks`, `count_chunks` and `list_chunk_details` so all +/// three ask the engine the same question. Two copies of this conversion would +/// compile identically today and drift the first time a filter is added to one +/// of them — and the symptom of that drift is a total that no amount of paging +/// can reach. /// /// The page bounds are carried across unchanged; dropping them for the count /// is the engine's job, because that is where the `LIMIT` is appended. +/// +/// The destructure is exhaustive on purpose. A field added to [`ChunkQuery`] +/// and forgotten here does not fail — it silently widens every query that used +/// it, which is a wrong answer rather than an error, and the caller has no way +/// to tell. Binding every field by name makes that a build failure instead. +/// +/// The plural filters carry their empty form through as *no filter*, matching +/// the engine, and that is the deliberate opposite of `source_scope`, which +/// denies when empty. A scope is a gate and fails closed; these are +/// narrowings, and a narrowing that failed closed on an empty list would +/// silently blank a page a caller assembled from nothing. fn chunk_query_to_engine( query: &ChunkQuery, scope: Option<&SourceScope>, ) -> Result { let ChunkQuery { + ids, source_kind, + source_kinds, source_id, + source_ids, owner, + entity_ids, + entity_kinds, + content_contains, since_ms, until_ms, limit, @@ -2291,11 +2473,20 @@ fn chunk_query_to_engine( exclude_dropped, } = query.clone(); Ok(tinymemory_core::store::chunks::ListChunksQuery { + ids, source_kind: source_kind .map(|kind| TinycortexProvider::cross(&kind, "convert source kind")) .transpose()?, + source_kinds: source_kinds + .iter() + .map(|kind| TinycortexProvider::cross(kind, "convert source kind")) + .collect::, MemoryError>>()?, source_id, + source_ids, owner, + entity_ids, + entity_kinds, + content_contains, since_ms, until_ms, limit, @@ -2337,6 +2528,65 @@ impl MemoryChunks for TinycortexProvider { .await } + async fn list_chunk_details( + &self, + query: &ChunkQuery, + scope: Option<&SourceScope>, + ) -> Result, MemoryError> { + // The same conversion the page and the total use, so a caller that + // renders "20 of 431" out of `count_chunks` and fills the table from + // here is looking at one predicate rather than three that agree today. + let engine_query = chunk_query_to_engine(query, scope)?; + let rows = blocking(self.config.clone(), "list chunk details", move |config| { + tinymemory_core::store::chunks::list_chunk_details(config, &engine_query) + }) + .await?; + // The engine's row is field-for-field the contract's, body included — + // which is to say body-excluded: neither type carries one, for the + // reason `ChunkListRow`'s own docs give. Crossed rather than moved + // because a host that resolves the contract crate twice has two + // `Chunk` types with one shape, the hazard every other conversion here + // is written against. + rows.into_iter() + .map(|row| { + Ok(ChunkListRow { + chunk: Self::cross(&row.chunk, "convert chunk")?, + content_path: row.content_path, + lifecycle_status: row.lifecycle_status, + has_embedding: row.has_embedding, + }) + }) + .collect() + } + + async fn source_totals( + &self, + limit: usize, + scope: Option<&SourceScope>, + ) -> Result, MemoryError> { + let allowed = scope_to_engine(scope); + let totals = blocking(self.config.clone(), "read source totals", move |config| { + // The engine takes `Option` so an internal caller can ask + // for its default page; the contract does not offer that spelling, + // and passing the caller's number through unchanged keeps the + // clamp in one place — the engine's, which is also the one + // `ChunkQuery::limit` is clamped by. + tinymemory_core::store::chunks::source_totals(config, Some(limit), allowed.as_ref()) + }) + .await?; + totals + .into_iter() + .map(|total| { + Ok(SourceTotal { + source_kind: Self::cross(&total.source_kind, "convert source kind")?, + source_id: total.source_id, + chunk_count: total.chunk_count, + most_recent_ms: total.last_timestamp_ms, + }) + }) + .collect() + } + async fn get_chunk(&self, chunk_id: &str) -> Result, MemoryError> { let id = chunk_id.to_string(); let chunk = blocking(self.config.clone(), "get chunk", move |config| { diff --git a/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs b/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs index 94cb4d14..e7f6cd0c 100644 --- a/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs +++ b/crates/tinymemory-tinycortex/tests/full_provider_conformance.rs @@ -1791,15 +1791,18 @@ async fn tree_entities_and_maintenance_execute_real_workspace_transitions() { ); let of_chunk = entities - .chunk_entities("chunk-1") + .chunk_entities(&["chunk-1".to_string()], None) .await .expect("entities of one chunk"); assert_eq!(of_chunk.len(), 2); // Equal counts break by entity id ascending, which is deterministic here. - assert_eq!(of_chunk[0].entity_id, "organization:tinymemory"); - assert_eq!(of_chunk[0].surface, "TinyMemory"); + assert_eq!(of_chunk[0].occurrence.entity_id, "organization:tinymemory"); + assert_eq!(of_chunk[0].occurrence.surface, "TinyMemory"); + // The single-id call is the batched one with a slice of one, and it still + // has to say which chunk each row came from. + assert!(of_chunk.iter().all(|row| row.chunk_id == "chunk-1")); assert!(entities - .chunk_entities("no-such-chunk") + .chunk_entities(&["no-such-chunk".to_string()], None) .await .expect("an unknown chunk is not an error") .is_empty()); @@ -2401,3 +2404,979 @@ async fn the_summary_forest_and_its_leaves_read_through_the_contract() { .expect("an empty allowlist is an answer here too") .is_empty()); } + +/// One chunk row, ready to be seeded straight into the store. +/// +/// The rows in the tests below go in through the store rather than the ingest +/// pipeline, for the reason +/// `the_chunk_count_matches_the_page_and_ignores_its_bounds` gives: what is +/// under test is which rows a predicate matches, and the pipeline decides chunk +/// boundaries, which would make every expected number a property of the chunker +/// instead of the filter. +fn chunk_row( + id: &str, + kind: tinymemory_api::chunks::SourceKind, + source_id: &str, + timestamp_ms: i64, +) -> tinymemory_api::chunks::Chunk { + let timestamp = chrono::DateTime::from_timestamp_millis(timestamp_ms).expect("timestamp"); + tinymemory_api::chunks::Chunk { + id: id.into(), + content: format!("Body of {id}."), + metadata: tinymemory_api::chunks::Metadata::point_in_time( + kind, source_id, "owner", timestamp, + ), + token_count: 3, + seq_in_source: 0, + created_at: timestamp, + partial_message: false, + } +} + +/// One canonical entity, ready to be indexed against a node. +fn canonical_entity( + canonical_id: &str, + kind: tinymemory_core::engine::backend::store::entity_index::EntityKind, + surface: &str, +) -> tinymemory_core::engine::backend::store::entity_index::CanonicalEntity { + tinymemory_core::engine::backend::store::entity_index::CanonicalEntity { + canonical_id: canonical_id.into(), + kind, + surface: surface.into(), + span_start: 0, + span_end: surface.len() as u32, + score: 1.0, + } +} + +/// An entity filter matches a chunk once, however many of the asked-for +/// entities that chunk mentions. +/// +/// This is the one filter in the family that reaches a second table, and the +/// obvious way to write it — `INNER JOIN mem_tree_entity_index` — multiplies +/// the chunk row by the number of matching index rows. The host's own SQL +/// carries a `SELECT DISTINCT` and a `COUNT(*)` over a subquery precisely to +/// undo that, and the two are separate spellings of one predicate: drop either +/// and the page and the total disagree, silently, and only for chunks that +/// mention more than one of the filtered entities. A semi-join (`EXISTS`) +/// cannot multiply in the first place, which is why the shared filter builder +/// has to use one. +/// +/// So the assertion is not "two rows came back" — a de-duplicating listing +/// passes that with a multiplying count beside it. It is that the same +/// `ChunkQuery` produces the same total through `count_chunks`, through +/// `list_chunks`, and through `list_chunk_details`. +#[tokio::test(flavor = "multi_thread")] +async fn an_entity_filter_matches_a_chunk_once_however_many_entities_it_mentions() { + use tinymemory_api::chunks::SourceKind; + use tinymemory_api::provider::{ChunkQuery, MemoryProvider}; + use tinymemory_core::engine::backend::store::entity_index::{CanonicalEntity, EntityKind}; + + let workspace = tempfile::tempdir().expect("workspace"); + let provider = provider_over(workspace.path()); + let config = provider_config(workspace.path(), serde_json::Value::Null); + + let seeded = vec![ + chunk_row( + "ent-both", + SourceKind::Document, + "ent-source", + 1_700_000_003_000, + ), + chunk_row( + "ent-one", + SourceKind::Document, + "ent-source", + 1_700_000_002_000, + ), + chunk_row( + "ent-none", + SourceKind::Document, + "ent-source", + 1_700_000_001_000, + ), + ]; + assert_eq!( + tinymemory_core::store::chunks::store::upsert_chunks(&config, &seeded).expect("seed"), + seeded.len() + ); + // `ent-both` mentions two of the two filtered entities; `ent-one` mentions + // one; `ent-none` mentions an entity of a different kind, so it is in the + // table but out of both filters. + let index = |node: &str, entities: &[CanonicalEntity]| { + tinymemory_core::store::entities::index_entities( + &config, + entities, + node, + "leaf", + 1_700_000_000_000, + Some("project"), + ) + .expect("seed the entity index") + }; + index( + "ent-both", + &[ + canonical_entity("person:alice", EntityKind::Person, "Alice"), + canonical_entity("person:bob", EntityKind::Person, "Bob"), + ], + ); + index( + "ent-one", + &[canonical_entity( + "person:alice", + EntityKind::Person, + "Alice", + )], + ); + index( + "ent-none", + &[canonical_entity( + "organization:acme", + EntityKind::Organization, + "Acme", + )], + ); + + let chunks = provider.as_chunks().expect("Chunks"); + let by_entity = ChunkQuery { + entity_ids: vec!["person:alice".into(), "person:bob".into()], + limit: Some(1_000), + ..ChunkQuery::default() + }; + let listed = chunks + .list_chunks(&by_entity, None) + .await + .expect("list by entity"); + assert_eq!( + listed.iter().filter(|chunk| chunk.id == "ent-both").count(), + 1, + "a chunk mentioning two of the filtered entities is still one chunk" + ); + assert_eq!(listed.len(), 2, "ent-none mentions neither"); + + let counted = chunks + .count_chunks(&by_entity, None) + .await + .expect("count by entity"); + assert_eq!( + counted as usize, + listed.len(), + "the total must not multiply where the page does not" + ); + + let detailed = chunks + .list_chunk_details(&by_entity, None) + .await + .expect("detail rows by entity"); + assert_eq!( + detailed.len() as u64, + counted, + "the detail listing and the total answer one predicate, not two that \ + happen to agree on the unfiltered case" + ); + let mut detailed_ids = detailed + .iter() + .map(|row| row.chunk.id.as_str()) + .collect::>(); + detailed_ids.sort_unstable(); + assert_eq!(detailed_ids, vec!["ent-both", "ent-one"]); + + // The kind filter reaches the same table by the same join and is subject + // to the same multiplication: `ent-both` carries two `person` rows. + let by_kind = ChunkQuery { + entity_kinds: vec!["person".into()], + limit: Some(1_000), + ..ChunkQuery::default() + }; + let by_kind_rows = chunks + .list_chunk_details(&by_kind, None) + .await + .expect("detail rows by entity kind"); + assert_eq!(by_kind_rows.len(), 2); + assert_eq!( + chunks + .count_chunks(&by_kind, None) + .await + .expect("count by entity kind") as usize, + by_kind_rows.len() + ); + + // An id nothing was indexed under is an empty page, not every chunk: a + // filter dropped on the way to the engine reads exactly like no filter. + let unmatched = ChunkQuery { + entity_ids: vec!["person:nobody".into()], + limit: Some(1_000), + ..ChunkQuery::default() + }; + assert!(chunks + .list_chunk_details(&unmatched, None) + .await + .expect("list nothing") + .is_empty()); +} + +/// The detail row reports the embedding sidecar, and the content filter +/// matches text rather than a pattern. +/// +/// Two failures that both look like working code: +/// +/// - `has_embedding` read from `mem_tree_chunks.embedding` is `false` for every +/// chunk in a live store. That column is a migration artefact nothing has +/// written since embeddings moved to `mem_tree_chunk_embeddings`, so the +/// host's own SQL — `CASE WHEN c.embedding IS NULL THEN 0 ELSE 1 END` — is a +/// constant `0` wearing a `CASE`. The seeded chunk here has a sidecar row and +/// no legacy blob, which is what every real chunk looks like. +/// - `content_contains` handed to `LIKE` unescaped turns `%` and `_` in the +/// caller's text into wildcards. Nobody notices until a user searches for +/// `100%` or a `snake_case` identifier and gets rows that do not contain +/// what they typed — a false positive, which is the failure a search box +/// cannot recover from. +#[tokio::test(flavor = "multi_thread")] +async fn detail_rows_carry_the_embedding_sidecar_and_match_content_literally() { + use tinymemory_api::chunks::SourceKind; + use tinymemory_api::provider::{ChunkQuery, MemoryProvider}; + + let workspace = tempfile::tempdir().expect("workspace"); + let provider = provider_over(workspace.path()); + let config = provider_config(workspace.path(), serde_json::Value::Null); + + let mut embedded = chunk_row( + "lit-embedded", + SourceKind::Document, + "lit", + 1_700_000_004_000, + ); + embedded.content = "100% sure about this".into(); + embedded.metadata.tags = vec!["alpha".into(), "beta".into()]; + let mut spelled_out = chunk_row( + "lit-spelled", + SourceKind::Document, + "lit", + 1_700_000_003_000, + ); + spelled_out.content = "100 percent sure about this".into(); + let mut underscored = chunk_row("lit-under", SourceKind::Document, "lit", 1_700_000_002_000); + underscored.content = "the snake_case identifier".into(); + let mut single_char = chunk_row("lit-any", SourceKind::Document, "lit", 1_700_000_001_000); + single_char.content = "the snakeXcase identifier".into(); + + let seeded = vec![embedded, spelled_out, underscored, single_char]; + assert_eq!( + tinymemory_core::store::chunks::store::upsert_chunks(&config, &seeded).expect("seed"), + seeded.len() + ); + tinymemory_core::store::chunks::set_chunk_embedding(&config, "lit-embedded", &[0.5, 0.25]) + .expect("write one embedding sidecar row"); + + let chunks = provider.as_chunks().expect("Chunks"); + let everything = ChunkQuery { + limit: Some(1_000), + ..ChunkQuery::default() + }; + let rows = chunks + .list_chunk_details(&everything, None) + .await + .expect("detail rows"); + assert_eq!(rows.len(), seeded.len()); + let embedded_row = rows + .iter() + .find(|row| row.chunk.id == "lit-embedded") + .expect("the embedded chunk is listed"); + assert!( + embedded_row.has_embedding, + "a chunk with a row in mem_tree_chunk_embeddings has an embedding, \ + whatever the legacy blob column says" + ); + assert!( + rows.iter() + .filter(|row| row.chunk.id != "lit-embedded") + .all(|row| !row.has_embedding), + "and a chunk without one does not" + ); + + // The rest of the row is what makes this member worth a round trip at all: + // a caller that has to fetch these separately is back to five reads a row. + assert_eq!( + embedded_row.chunk.metadata.source_kind, + SourceKind::Document + ); + assert_eq!(embedded_row.chunk.metadata.source_id, "lit"); + assert_eq!(embedded_row.chunk.metadata.owner, "owner"); + assert_eq!( + embedded_row.chunk.metadata.timestamp.timestamp_millis(), + 1_700_000_004_000 + ); + assert_eq!(embedded_row.chunk.token_count, 3); + assert_eq!(embedded_row.lifecycle_status.as_deref(), Some("admitted")); + assert_eq!( + embedded_row.chunk.metadata.tags, + vec!["alpha".to_string(), "beta".to_string()], + "tags arrive decoded, not as the stored JSON text" + ); + assert_eq!(embedded_row.chunk.content, "100% sure about this"); + assert!( + embedded_row.content_path.is_none(), + "an inline chunk has no vault path, and the list must not invent one" + ); + + // `%` is a literal. Read as a wildcard it also matches "100 percent sure", + // which is the row a user searching for "100%" must not be shown. + let percent = ChunkQuery { + content_contains: Some("100% sure".into()), + limit: Some(1_000), + ..ChunkQuery::default() + }; + let percent_rows = chunks + .list_chunk_details(&percent, None) + .await + .expect("literal percent"); + assert_eq!( + percent_rows + .iter() + .map(|row| row.chunk.id.as_str()) + .collect::>(), + vec!["lit-embedded"], + "% must not match ' percent'" + ); + assert_eq!( + chunks + .count_chunks(&percent, None) + .await + .expect("count literal percent"), + percent_rows.len() as u64 + ); + + // `_` is a literal too. Read as a wildcard it also matches "snakeXcase". + let underscore = ChunkQuery { + content_contains: Some("snake_case".into()), + limit: Some(1_000), + ..ChunkQuery::default() + }; + assert_eq!( + chunks + .list_chunk_details(&underscore, None) + .await + .expect("literal underscore") + .iter() + .map(|row| row.chunk.id.as_str()) + .collect::>(), + vec!["lit-under"], + "_ must not match any single character" + ); + + // The id filter is the recall-hydration path: N ids in, those rows out. + let by_ids = ChunkQuery { + ids: vec!["lit-under".into(), "lit-any".into()], + limit: Some(1_000), + ..ChunkQuery::default() + }; + let mut hydrated = chunks + .list_chunk_details(&by_ids, None) + .await + .expect("hydrate by id") + .into_iter() + .map(|row| row.chunk.id) + .collect::>(); + hydrated.sort(); + assert_eq!( + hydrated, + vec!["lit-any".to_string(), "lit-under".to_string()] + ); +} + +/// Source totals bound sources, count chunks, and apply the scope. +/// +/// Three things a caller cannot check for itself. The `limit` bounds the +/// number of *sources* — bound to chunks instead and a store with one busy +/// source returns one row and looks empty. The count is an aggregate over the +/// whole source, not over the page the browser happens to be showing. And the +/// allowlist is the same one `list_chunks` applies: a scoped caller that must +/// not see a source's rows must not learn the source exists from its total +/// either. +#[tokio::test(flavor = "multi_thread")] +async fn source_totals_bound_sources_and_apply_the_scope() { + use tinymemory_api::chunks::SourceKind; + use tinymemory_api::provider::types::SourceScope; + use tinymemory_api::provider::MemoryProvider; + + let workspace = tempfile::tempdir().expect("workspace"); + let provider = provider_over(workspace.path()); + let config = provider_config(workspace.path(), serde_json::Value::Null); + + let mut scoped = chunk_row( + "tot-scoped", + SourceKind::Document, + "mem_src:src-a:item-1", + 1_700_000_500_000, + ); + scoped.metadata.tags = vec!["memory_sources".into()]; + let seeded = vec![ + chunk_row("tot-a-0", SourceKind::Document, "doc-a", 1_700_000_100_000), + chunk_row("tot-a-1", SourceKind::Document, "doc-a", 1_700_000_200_000), + chunk_row("tot-a-2", SourceKind::Document, "doc-a", 1_700_000_300_000), + chunk_row("tot-b-0", SourceKind::Document, "doc-b", 1_700_000_400_000), + chunk_row("tot-c-0", SourceKind::Chat, "chat-a", 1_700_000_050_000), + scoped, + ]; + assert_eq!( + tinymemory_core::store::chunks::store::upsert_chunks(&config, &seeded).expect("seed"), + seeded.len() + ); + + let chunks = provider.as_chunks().expect("Chunks"); + let totals = chunks + .source_totals(1_000, None) + .await + .expect("every source total"); + assert_eq!(totals.len(), 4, "six chunks across four distinct sources"); + assert_eq!( + totals + .iter() + .map(|total| total.source_id.as_str()) + .collect::>(), + vec!["mem_src:src-a:item-1", "doc-b", "doc-a", "chat-a"], + "most recently written source first" + ); + let busiest = totals + .iter() + .find(|total| total.source_id == "doc-a") + .expect("doc-a is a source"); + assert_eq!(busiest.chunk_count, 3); + assert_eq!(busiest.most_recent_ms, 1_700_000_300_000); + assert_eq!(busiest.source_kind, SourceKind::Document); + // Same source id under two kinds would be two rows; `chat-a` proves the + // kind is part of the group key rather than decoration on it. + assert_eq!( + totals + .iter() + .find(|total| total.source_id == "chat-a") + .expect("chat-a is a source") + .source_kind, + SourceKind::Chat + ); + + let bounded = chunks + .source_totals(2, None) + .await + .expect("bounded source totals"); + assert_eq!( + bounded.len(), + 2, + "the bound is on sources; two chunks would be one source here" + ); + assert_eq!(bounded[0].source_id, "mem_src:src-a:item-1"); + + // The engine's fail-closed rule, unchanged: `src-b` allows nothing + // ingested under `src-a`, and content with no source provenance at all is + // outside the predicate and stays visible. + let scoped_totals = chunks + .source_totals(1_000, Some(&SourceScope::new(["src-b"]))) + .await + .expect("scoped source totals"); + assert_eq!(scoped_totals.len(), 3); + assert!( + scoped_totals + .iter() + .all(|total| total.source_id != "mem_src:src-a:item-1"), + "a scoped caller must not learn a forbidden source exists from its total" + ); +} + +/// Each `forget_matching` selector removes what it names and nothing else. +/// +/// Four selectors over one door, and the door is the whole reason for the +/// shape: the alternative is four members, three of which differ from the +/// others only by which column the predicate reads. What that buys — and what +/// this test exists for — is that the four arms are wired to four different +/// deletes, and a mis-wired arm is silent. An `Owner` routed to the +/// source-prefix delete matches nothing and reports `0`, which reads exactly +/// like "there was nothing to remove"; routed to the exact-source delete it +/// removes the wrong rows and still reports a plausible number. +/// +/// The per-chunk arm additionally has to cascade. A bare +/// `DELETE FROM mem_tree_chunks` leaves the entity-index row behind, and the +/// chunk keeps appearing in every entity read that never joins back to the +/// chunk table — a deleted chunk that is still findable by name. +#[tokio::test(flavor = "multi_thread")] +async fn each_forget_selector_removes_what_it_names_and_leaves_its_siblings() { + use tinymemory_api::chunks::SourceKind; + use tinymemory_api::error::MemoryError; + use tinymemory_api::provider::types::ForgetSelector; + use tinymemory_api::provider::MemoryProvider; + use tinymemory_core::engine::backend::store::entity_index::EntityKind; + + let workspace = tempfile::tempdir().expect("workspace"); + let provider = provider_over(workspace.path()); + let config = provider_config(workspace.path(), serde_json::Value::Null); + + let mut alice = chunk_row( + "own-alice", + SourceKind::Document, + "owned-a", + 1_700_000_001_000, + ); + alice.metadata.owner = "alice".into(); + let mut bob = chunk_row( + "own-bob", + SourceKind::Document, + "owned-b", + 1_700_000_002_000, + ); + bob.metadata.owner = "bob".into(); + let seeded = vec![ + chunk_row( + "del-0", + SourceKind::Document, + "del-source", + 1_700_000_010_000, + ), + chunk_row( + "del-1", + SourceKind::Document, + "del-source", + 1_700_000_011_000, + ), + chunk_row( + "del-2", + SourceKind::Document, + "del-source", + 1_700_000_012_000, + ), + chunk_row( + "pfx-one", + SourceKind::Document, + "pfx:one", + 1_700_000_020_000, + ), + chunk_row( + "pfx-two", + SourceKind::Document, + "pfx:two", + 1_700_000_021_000, + ), + chunk_row( + "pfx-other", + SourceKind::Document, + "other", + 1_700_000_022_000, + ), + alice, + bob, + ]; + assert_eq!( + tinymemory_core::store::chunks::store::upsert_chunks(&config, &seeded).expect("seed"), + seeded.len() + ); + assert_eq!( + tinymemory_core::store::entities::index_entities( + &config, + &[canonical_entity( + "person:carol", + EntityKind::Person, + "Carol" + )], + "del-1", + "leaf", + 1_700_000_011_000, + Some("project"), + ) + .expect("seed an occurrence on the chunk about to be deleted"), + 1 + ); + + let sources = provider.as_sources().expect("Sources"); + let chunks = provider.as_chunks().expect("Chunks"); + let entities = provider.as_entities().expect("Entities"); + + // ── One chunk ─────────────────────────────────────────────────────────── + let one = sources + .forget_matching(&ForgetSelector::Chunk { + chunk_id: "del-1".into(), + }) + .await + .expect("forget one chunk"); + assert_eq!(one.chunks_removed, 1); + assert_eq!( + one.trees_cleaned, 0, + "a chunk id names no source scope, so there is no orphaned tree to \ + report" + ); + assert!(chunks + .get_chunk("del-1") + .await + .expect("read the deleted chunk") + .is_none()); + for sibling in ["del-0", "del-2"] { + assert!( + chunks + .get_chunk(sibling) + .await + .expect("read a sibling") + .is_some(), + "{sibling} shares a source with the deleted chunk and must survive" + ); + } + assert!( + entities + .entity_chunk_ids("person:carol", 10) + .await + .expect("read the occurrence index") + .is_empty(), + "the occurrence index must not keep naming a chunk that is gone" + ); + // Deleting it again is `0`, not an error — the end state is the same. + assert_eq!( + sources + .forget_matching(&ForgetSelector::Chunk { + chunk_id: "del-1".into(), + }) + .await + .expect("forget it twice") + .chunks_removed, + 0 + ); + + // ── One exact source ──────────────────────────────────────────────────── + let source = sources + .forget_matching(&ForgetSelector::Source { + source_kind: "document".into(), + source_id: "del-source".into(), + }) + .await + .expect("forget one source"); + assert_eq!(source.chunks_removed, 2, "the two survivors of that source"); + for gone in ["del-0", "del-2"] { + assert!(chunks + .get_chunk(gone) + .await + .expect("read a removed chunk") + .is_none()); + } + + // ── A source prefix ───────────────────────────────────────────────────── + let prefix = sources + .forget_matching(&ForgetSelector::SourcePrefix { + source_kind: "document".into(), + source_id_prefix: "pfx:".into(), + }) + .await + .expect("forget a source prefix"); + assert_eq!(prefix.chunks_removed, 2); + assert!( + chunks + .get_chunk("pfx-other") + .await + .expect("read the unprefixed chunk") + .is_some(), + "the prefix is a prefix, not a substring or a wildcard" + ); + + // ── One owner ─────────────────────────────────────────────────────────── + let owner = sources + .forget_matching(&ForgetSelector::Owner { + source_kind: "document".into(), + owner: "alice".into(), + }) + .await + .expect("forget one owner"); + assert_eq!(owner.chunks_removed, 1); + assert!(chunks + .get_chunk("own-alice") + .await + .expect("read the removed owner's chunk") + .is_none()); + assert!( + chunks + .get_chunk("own-bob") + .await + .expect("read the other owner's chunk") + .is_some(), + "an owner delete that reached the source column would take bob too" + ); + + // A kind this engine does not store is refused rather than answered with + // a zero. On a destructive call the two readings send an operator opposite + // ways: one says "fix the argument", the other says "it was already gone". + assert!( + matches!( + sources + .forget_matching(&ForgetSelector::Source { + source_kind: "not-a-kind".into(), + source_id: "del-source".into(), + }) + .await, + Err(MemoryError::Invalid(_)) + ), + "an unrecognised source kind must not read as nothing to remove" + ); +} + +/// Purging clears the chunk tier and everything keyed to it, in one call. +/// +/// The operation exists because the caller cannot assemble it: the derived +/// tables are keyed by tree ids and job ids a chunk-shaped delete cannot +/// enumerate, so sweeping every source leaves summaries and trees standing over +/// chunks that no longer exist. That half-wiped store is worse than the full +/// one — recall walks a tree whose leaves are gone, and re-ingest is refused by +/// gates whose content was deleted. +/// +/// What is pinned is that one call is enough: after it, every read in the +/// contract that touches the tier answers empty, and the reported row count +/// covers the derived rows as well as the chunks. A caller that had to issue +/// this table by table could stop halfway; a driver that wipes table by table +/// outside a transaction can too. +#[tokio::test(flavor = "multi_thread")] +async fn purging_clears_the_chunk_tier_and_everything_keyed_to_it() { + use tinymemory_api::chunks::SourceKind; + use tinymemory_api::provider::{ChunkQuery, MemoryProvider}; + use tinymemory_core::engine::backend::store::entity_index::EntityKind; + + let workspace = tempfile::tempdir().expect("workspace"); + let provider = provider_over(workspace.path()); + let config = provider_config(workspace.path(), serde_json::Value::Null); + + let seeded = vec![ + chunk_row( + "purge-0", + SourceKind::Document, + "purge-source", + 1_700_000_001_000, + ), + chunk_row( + "purge-1", + SourceKind::Document, + "purge-source", + 1_700_000_002_000, + ), + chunk_row("purge-2", SourceKind::Chat, "purge-chat", 1_700_000_003_000), + ]; + assert_eq!( + tinymemory_core::store::chunks::store::upsert_chunks(&config, &seeded).expect("seed"), + seeded.len() + ); + assert_eq!( + tinymemory_core::store::entities::index_entities( + &config, + &[ + canonical_entity("person:dave", EntityKind::Person, "Dave"), + canonical_entity("topic:migration", EntityKind::Topic, "migration"), + ], + "purge-0", + "leaf", + 1_700_000_001_000, + Some("project"), + ) + .expect("seed the entity index"), + 2 + ); + + let chunks = provider.as_chunks().expect("Chunks"); + let entities = provider.as_entities().expect("Entities"); + let maintenance = provider.as_maintenance().expect("Maintenance"); + let everything = ChunkQuery { + limit: Some(1_000), + ..ChunkQuery::default() + }; + assert_eq!( + chunks + .count_chunks(&everything, None) + .await + .expect("count before"), + seeded.len() as u64, + "nothing below means anything if the store was empty to begin with" + ); + + let outcome = maintenance.purge_all().await.expect("purge the store"); + assert!( + outcome.rows_deleted >= seeded.len() as u64, + "a purge that reported less than it removed would let a caller tell a \ + user their store was already empty: got {}", + outcome.rows_deleted + ); + + assert_eq!( + chunks + .count_chunks(&everything, None) + .await + .expect("count after"), + 0 + ); + assert!(chunks + .list_chunks(&everything, None) + .await + .expect("list after") + .is_empty()); + assert!(chunks + .list_chunk_details(&everything, None) + .await + .expect("detail rows after") + .is_empty()); + assert!(chunks + .source_totals(1_000, None) + .await + .expect("source totals after") + .is_empty()); + assert!( + entities + .top_entities(None, 10) + .await + .expect("entity index after") + .is_empty(), + "a purge that left the occurrence index standing would keep naming \ + entities extracted from chunks that no longer exist" + ); + assert_eq!( + maintenance + .store_stats() + .await + .expect("store stats after") + .chunks, + 0 + ); + + // Purging an empty store is a no-op, not a failure: the end state is the + // one that was asked for. + assert_eq!( + maintenance + .purge_all() + .await + .expect("purge again") + .rows_deleted, + 0 + ); +} + +/// Occurrences read for many chunks at once stay attached to their own chunk. +/// +/// The member takes a set because its callers have one — a page of rows to +/// label, a contacts graph of fifteen hundred nodes. Answering that by looping +/// the single-chunk read is fifteen hundred bus messages, which is why the +/// signature carries the set rather than the caller. +/// +/// The regression a batched read invites is the row losing its owner. The index +/// column is `node_id`, and the two ways to drop it are both easy to write and +/// impossible to see afterwards: not selecting it at all and stamping every row +/// with `chunk_ids[0]`, or concatenating per-chunk results and letting the +/// caller assume input order survived. Either way every entity in the page +/// appears to belong to the first chunk in it. +#[tokio::test(flavor = "multi_thread")] +async fn occurrences_read_in_a_batch_stay_attached_to_their_own_chunk() { + use tinymemory_api::error::MemoryError; + use tinymemory_api::provider::MemoryProvider; + use tinymemory_core::engine::backend::store::entity_index::{CanonicalEntity, EntityKind}; + + let workspace = tempfile::tempdir().expect("workspace"); + let provider = provider_over(workspace.path()); + let config = provider_config(workspace.path(), serde_json::Value::Null); + + let index = |node: &str, entities: &[CanonicalEntity], at_ms: i64| { + tinymemory_core::store::entities::index_entities( + &config, + entities, + node, + "leaf", + at_ms, + Some("project"), + ) + .expect("seed the entity index") + }; + index( + "batch-a", + &[ + canonical_entity("person:erin", EntityKind::Person, "Erin"), + canonical_entity("organization:acme", EntityKind::Organization, "Acme"), + ], + 1_700_000_001_000, + ); + index( + "batch-b", + &[canonical_entity( + "person:frank", + EntityKind::Person, + "Frank", + )], + 1_700_000_002_000, + ); + // A third chunk nobody asks about, so "returned everything in the table" + // is distinguishable from "returned what was asked for". + index( + "batch-c", + &[canonical_entity( + "person:grace", + EntityKind::Person, + "Grace", + )], + 1_700_000_003_000, + ); + + let entities = provider.as_entities().expect("Entities"); + let asked = ["batch-a".to_string(), "batch-b".to_string()]; + let rows = entities + .chunk_entities(&asked, None) + .await + .expect("occurrences for two chunks"); + assert_eq!(rows.len(), 3, "two on the first chunk, one on the second"); + // `Option` rather than an unwrap: an entity missing from the result reads + // as `None` against the expected chunk instead of panicking out of the + // closure, so the assertion below names which entity went missing. + let owner_of = |entity_id: &str| { + rows.iter() + .find(|row| row.occurrence.entity_id == entity_id) + .map(|row| row.chunk_id.as_str()) + }; + assert_eq!(owner_of("person:erin"), Some("batch-a")); + assert_eq!(owner_of("organization:acme"), Some("batch-a")); + assert_eq!( + owner_of("person:frank"), + Some("batch-b"), + "a row stamped with the first requested id would say batch-a here" + ); + assert!( + rows.iter() + .all(|row| row.occurrence.entity_id != "person:grace"), + "only the chunks that were asked about" + ); + + // The kind filter narrows without losing the attachment. + let people = entities + .chunk_entities(&asked, Some(&["person".to_string()])) + .await + .expect("people only"); + assert_eq!(people.len(), 2); + assert!(people.iter().all(|row| row.occurrence.kind == "person")); + assert_eq!( + people + .iter() + .map(|row| row.chunk_id.as_str()) + .collect::>(), + ["batch-a", "batch-b"].into_iter().collect() + ); + + // An empty request is an empty answer, not the whole index — and so is a + // kind filter that admits no kind, which the store would otherwise read as + // the unfiltered case. + assert!(entities + .chunk_entities(&[], None) + .await + .expect("no chunks asked about") + .is_empty()); + assert!( + entities + .chunk_entities(&asked, Some(&[])) + .await + .expect("a filter admitting no kind is an answer, not a fault") + .is_empty(), + "Some(&[]) is not a second spelling of None" + ); + + // And the kind filter is validated rather than applied blindly, for + // `top_entities`' reason: a misspelled kind that matched nothing would read + // as "these chunks mention nobody", which is an answer the caller acts on. + assert!(matches!( + entities + .chunk_entities(&asked, Some(&["not-a-kind".to_string()])) + .await, + Err(MemoryError::Invalid(_)) + )); +} diff --git a/vendor/tinycortex b/vendor/tinycortex index 2e7b57ee..745ff33e 160000 --- a/vendor/tinycortex +++ b/vendor/tinycortex @@ -1 +1 @@ -Subproject commit 2e7b57eea7188a9c96e7baee0a0d2cfb6c8f6b7c +Subproject commit 745ff33e38972e51873084da7841dd3c57a97cb3 From 38e03fb90db769b5ce987927d603b0e53bf6c0c8 Mon Sep 17 00:00:00 2001 From: Shanu Date: Tue, 25 Aug 2026 03:57:05 +0530 Subject: [PATCH 5/6] Advance vendor/tinycortex to the reviewed engine head Picks up the corrected nullability documentation on the lifecycle column and the test that pins it. No query changes, so nothing on this side moves; the gitlink is bumped to keep the two branches describing the same engine rather than leaving this one testing against a superseded commit. This still points at a pull-request head. When tinycortex#157 merges, re-point it at the merge SHA before this PR merges. --- vendor/tinycortex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/tinycortex b/vendor/tinycortex index 745ff33e..ef09f096 160000 --- a/vendor/tinycortex +++ b/vendor/tinycortex @@ -1 +1 @@ -Subproject commit 745ff33e38972e51873084da7841dd3c57a97cb3 +Subproject commit ef09f0964fe9e00d04ec305f3956ffe9ff9bbb41 From ff07568912974987a82df2e5ffac729ae8423060 Mon Sep 17 00:00:00 2001 From: Shanu Date: Tue, 25 Aug 2026 11:24:14 +0530 Subject: [PATCH 6/6] Serve ComposioHost and ConfigLoader inside the module, and re-pin onto merged tinycortex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine reaches its host through nine seam traits installed as process globals. This module installed seven of them; the host installs eight. The two nobody installed here were `ComposioHost` and `ConfigLoader`, and that gap is what stands between OpenHuman and deleting the second in-process engine it currently boots. It fails quietly, which is the reason to close it rather than document it: `require_composio_host()` returns `Err` and `is_available()` returns `false` when unset, so composio sync would simply stop, and the first symptom would be a user noticing their synced mail or chat had gone stale. `BusComposioHost` proxies the four members to an interface the host serves, following the shape `ChatHost` and `EmbeddingHost` already use. The engine's `&Config` never travels — the module supplies its own on this side of the proxy — and an unserved host is reported once per process through the already installed `ErrorReporter` rather than being swallowed. `is_available` answers `true` whenever it could not ask, because a wrong `false` is a silent "not signed in" skip while a wrong `true` is a named failure one call later. `ModuleConfigLoader` answers from the `ModuleConfig` this module is already handed rather than proxying to the host. Asking the host to re-read a config the module was given would let the two disagree about which workspace is being worked in, and `reload_snapshot` refuses a snapshot anchored at a different `config_path` for the same reason: answering it with this module's config would silently hand a caller another workspace's data. ## What is deliberately NOT here Ownership of the periodic composio sync loop does not move into this module, and that is a finding rather than an omission. Three things block it, each verified at source and each written into `lib.rs` at the line someone would try to add the call: - `sync/pipelines/host.rs`'s `composio_config` takes its direct branch only when `config.composio().mode == "direct"`, and `EngineRuntimeConfig` answers `ComposioMode::default()` with no session token, so backend mode fails and direct mode is never selected. `ComposioHost::api_key` cannot rescue this: it is consulted inside the branch that is not taken. - `run_composio_connection_with_caps` begins at `global::client_if_ready()`, which is `None` here — this module builds its store through `create_memory_client_with_local_ai` and never touches the global slot. - `EngineRuntimeConfig::memory_sync_interval_secs()` is `Some(0)`, which `effective_interval_secs` defines as manual-only, so both the composio and workspace loops would skip every source with no error at all. Until those are closed the loop stays where it is, host-side, against the engine the host still boots. `vendor/tinycortex` moves to tinycortex#157's merge commit on `main`, so this branch no longer points at a pull-request head. --- crates/tinymemory-module/src/composio.rs | 381 ++++++++++++++++++ crates/tinymemory-module/src/composio_test.rs | 265 ++++++++++++ crates/tinymemory-module/src/config_loader.rs | 186 +++++++++ .../src/config_loader_test.rs | 96 +++++ crates/tinymemory-module/src/host.rs | 76 ++-- crates/tinymemory-module/src/lib.rs | 84 +++- vendor/tinycortex | 2 +- 7 files changed, 1047 insertions(+), 43 deletions(-) create mode 100644 crates/tinymemory-module/src/composio.rs create mode 100644 crates/tinymemory-module/src/composio_test.rs create mode 100644 crates/tinymemory-module/src/config_loader.rs create mode 100644 crates/tinymemory-module/src/config_loader_test.rs diff --git a/crates/tinymemory-module/src/composio.rs b/crates/tinymemory-module/src/composio.rs new file mode 100644 index 00000000..c843c1e6 --- /dev/null +++ b/crates/tinymemory-module/src/composio.rs @@ -0,0 +1,381 @@ +//! Composio stays host-side; only the request crosses. +//! +//! # What this seam is for +//! +//! The engine's memory-sync layer needs four things from Composio: which +//! connections the signed-in user has, the ability to run one tool against one +//! of them, the direct-mode API key, and a cheap "is any of this wired up?" +//! probe. It needs none of the rest of the integration — OAuth, the backend +//! session, the toolkit allowlist, HMAC-verified trigger fan-out, or the choice +//! between backend-proxied and direct mode. `tinymemory_core::composio_host` +//! draws that line, and this module is the module-mode implementation of the +//! engine's half. +//! +//! # Why a proxy and not an answer from `ModuleConfig` +//! +//! Because none of the four is a *value*; all four are live host state. The +//! connection list changes when the user completes an OAuth flow in a browser, +//! the direct key changes on a `set_api_key` RPC, and neither restarts +//! anything. A load-time snapshot would report the state as it was when the +//! module loaded and keep reporting it for the life of the process — which for +//! `is_available` means telling the sync layer "not signed in" about a user who +//! signed in a minute ago, and the sync layer treats that as *skip silently*. +//! That is the looks-empty-rather-than-broken failure this whole seam exists to +//! prevent, so the answer has to come from the host at call time. +//! +//! It is the same reasoning [`crate::embedding`] gives for keeping the embed +//! host-side, and the opposite of the one [`crate::config_loader`] gives for +//! answering the config locally — the deciding question in both directions is +//! whether the host holds something the module cannot be handed once. +//! +//! # The credential does cross here, unlike everywhere else +//! +//! [`crate::embedding`] refuses to carry an inference key and this crate's +//! module docs say the module carries no credentials. `ApiKey` is the exception +//! and it is worth naming rather than hiding: the engine's +//! `sync::pipelines::host::composio_config` builds its **own** HTTP client from +//! the direct-mode key, so unlike an embed there is no host-side call to route +//! the work through. A `None` here is therefore not a degraded answer the +//! caller works around — it is "direct-mode sync cannot run at all". +//! +//! The property that survives is the one that was actually load-bearing: +//! [`crate::config::ModuleConfig`] still has nowhere to *hold* a credential, so +//! the key exists in this address space only for the duration of one call and +//! only when the sync layer asked for it. Narrowing this further means moving +//! the direct-mode sync client behind an `Execute`-shaped method, which is a +//! change to the engine's contract rather than something to smuggle in through +//! one of its two halves. + +use std::sync::atomic::{AtomicBool, Ordering}; + +use async_trait::async_trait; +use tinybus::Connection; +use tinymemory_core::composio_host::{ComposioConnection, ComposioExecuteResponse, ComposioHost}; + +use crate::host::report_unserved_once; + +/// Well-known name the host serves its Composio integration under. +pub const COMPOSIO_HOST_BUS_NAME: &str = "ai.tinyhumans.tinymemory.ComposioHost"; + +/// Object path the host serves it at. +pub const COMPOSIO_HOST_OBJECT_PATH: &str = "/ai/tinyhumans/tinymemory/ComposioHost"; + +/// Interface the host serves at [`COMPOSIO_HOST_OBJECT_PATH`]. +/// +/// Equal to [`COMPOSIO_HOST_BUS_NAME`] by convention, but a separate constant +/// for the same reason [`crate::embedding::EMBEDDING_HOST_INTERFACE`] is: one +/// addresses a peer, the other selects a dispatch table on that peer's object. +pub const COMPOSIO_HOST_INTERFACE: &str = "ai.tinyhumans.tinymemory.ComposioHost"; + +/// Every connection the signed-in user has, active or not. +pub const LIST_CONNECTIONS_METHOD: &str = "ListConnections"; + +/// Run one Composio tool against one connection. +/// +/// Takes `(tool, arguments, entity_id, connection_id)`. The last two travel +/// even though backend mode ignores both: which mode is in force is resolved +/// host-side at call time, and omitting them would silently drop the connection +/// pin the moment a user switched to direct mode. +pub const EXECUTE_METHOD: &str = "Execute"; + +/// The direct-mode Composio API key, or `None` when direct mode is unset. +pub const API_KEY_METHOD: &str = "ApiKey"; + +/// Whether *some* viable Composio client resolves host-side right now. +pub const IS_AVAILABLE_METHOD: &str = "IsAvailable"; + +/// Latched so the gap is reported once per process rather than once per sync +/// tick — the periodic scheduler consults this seam on every tick, and an +/// unlatched report would page on every one of them. Same guard the scheduler +/// gate and the shutdown host in `crate::host` put on theirs. +static COMPOSIO_REPORTED: AtomicBool = AtomicBool::new(false); + +/// What an unserved Composio host costs, in the terms a reader of the log +/// needs. +const COMPOSIO_UNSERVED: &str = "composio host unserved in module mode: this host serves no \ + `ai.tinyhumans.tinymemory.ComposioHost` interface, so memory \ + sync cannot list connections, run a Composio tool, or resolve a \ + direct-mode key — every synced source stops updating"; + +/// What a probe made from outside a Tokio runtime costs. +/// +/// `api_key` and `is_available` are synchronous on the engine's trait and a bus +/// call is not, so they need a runtime handle to bridge onto. Every caller in +/// the engine reaches them from inside one; a caller that did not would get a +/// silent `None`/`false` without this. +const COMPOSIO_NO_RUNTIME: &str = "composio host probe made outside a Tokio runtime: the \ + synchronous `api_key`/`is_available` probes bridge onto the \ + module runtime to reach the host, and without one they cannot \ + ask — direct-mode sync will report its key as unconfigured"; + +/// The Composio integration, reached over the module's connection. +pub struct BusComposioHost { + connection: Connection, + /// Cleared the first time a call proves the host serves no Composio + /// interface at all. + /// + /// Not a cache of the *user's* Composio state — that is deliberately never + /// cached, see the module docs. This records one structural fact about the + /// host, which cannot change while the process runs: tinybus never unloads + /// a library and a host that did not serve the interface at load will not + /// grow one. Recording it turns every later probe into a local answer + /// instead of a round trip that is already known to fail. + host_serves: AtomicBool, +} + +// `Connection` is not `Debug`, and `ComposioHost` requires it. Rendering the +// connection would say nothing useful anyway, and this type's only other field +// is a latch. +impl std::fmt::Debug for BusComposioHost { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("BusComposioHost") + .field("host_serves", &self.host_serves.load(Ordering::SeqCst)) + .finish_non_exhaustive() + } +} + +impl BusComposioHost { + /// Build the host bridge over the module connection. + /// + /// Takes no configuration on purpose: everything this seam answers is live + /// host state, so there is nothing about it worth capturing at load time. + #[must_use] + pub fn new(connection: Connection) -> Self { + Self { + connection, + host_serves: AtomicBool::new(true), + } + } + + /// Call one member of the host's Composio interface. + /// + /// # Errors + /// + /// The named-unserved message when the host exports no such interface, + /// otherwise the bus failure with the member that produced it. Never + /// carries `arguments`: a Composio tool call's arguments are mail queries, + /// document bodies and connection pins, and an error string is not a place + /// for any of them. + async fn call( + &self, + member: &'static str, + arguments: impl serde::Serialize + Send, + ) -> Result + where + R: serde::de::DeserializeOwned, + { + let proxy = self + .connection + .proxy( + COMPOSIO_HOST_BUS_NAME, + COMPOSIO_HOST_OBJECT_PATH, + COMPOSIO_HOST_INTERFACE, + ) + .map_err(|error| self.classify(member, &error))?; + proxy + .call(member, arguments) + .await + .map_err(|error| self.classify(member, &error)) + } + + /// Turn a bus failure into a message, and name a structural one out loud. + /// + /// Two failures wear the same clothes on this seam and must not be + /// conflated: "the user has no Composio connections" is an ordinary answer, + /// while "this host exports no Composio interface" is a build mismatch that + /// stops every synced source updating. The second is what + /// `report_unserved_once` exists for. + fn classify(&self, member: &'static str, error: &tinybus::Error) -> String { + if is_unserved(error) { + self.host_serves.store(false, Ordering::SeqCst); + report_unserved_once(&COMPOSIO_REPORTED, COMPOSIO_UNSERVED, "composio_host"); + return format!("{COMPOSIO_UNSERVED} (calling {member}: {error})"); + } + format!("composio host call {member} failed: {error}") + } + + /// Ask the host one argument-free question from a synchronous caller. + /// + /// `Some` is the host's answer; `None` means it could not be asked, which + /// each probe below turns into its own fallback. + /// + /// # Why an OS thread and not `block_in_place` + /// + /// `ComposioHost::api_key` and `ComposioHost::is_available` are synchronous + /// on the engine's trait — they are consulted from inside + /// `composio_config` and `ProviderContext::from_config`, neither of which + /// can `await` — and the host serves both as ordinary async bus members. So + /// something has to bridge, and the two candidates behave differently under + /// the runtime flavours this code can find itself on. + /// `tokio::task::block_in_place` panics outright on a current-thread + /// runtime, and this module cannot prove its caller's flavour: the shipped + /// module declares eight worker threads, but nothing stops an in-process + /// harness from driving this code on a current-thread runtime, and a probe + /// that aborted the process would be a far worse failure than the one it + /// was asked about. A fresh thread with a `Handle` works identically under + /// both, and costs one spawn on a path that is about to make a network call + /// anyway. + /// + /// It does occupy the calling thread until the host answers. That is + /// bounded by the bus's own call deadline and happens at most twice per + /// sync tick, against a runtime sized at eight workers — but it is the + /// reason these two are probes and not a general-purpose synchronous call + /// helper, and why nothing else in this file uses this path. + fn probe(&self, member: &'static str) -> Option + where + R: serde::de::DeserializeOwned + Send + 'static, + { + // Already proven unserved: answer locally rather than spawn a thread to + // rediscover it. The report has fired; a second one would be noise. + if !self.host_serves.load(Ordering::SeqCst) { + return None; + } + let Ok(handle) = tokio::runtime::Handle::try_current() else { + report_unserved_once(&COMPOSIO_REPORTED, COMPOSIO_NO_RUNTIME, "composio_host"); + return None; + }; + let connection = self.connection.clone(); + let joined = std::thread::scope(|scope| { + scope + .spawn(move || { + handle.block_on(async move { + let proxy = connection.proxy( + COMPOSIO_HOST_BUS_NAME, + COMPOSIO_HOST_OBJECT_PATH, + COMPOSIO_HOST_INTERFACE, + )?; + proxy.call::(member, ()).await + }) + }) + .join() + }); + match joined { + Ok(Ok(answer)) => Some(answer), + Ok(Err(error)) => { + // Classified *before* the log call, not inside it. `classify` + // latches the seam and fires the once-per-process report, and + // `log::debug!` does not evaluate its arguments when debug + // logging is off — which is every shipped build. Folding the + // two together would make the report depend on the log level. + let named = self.classify(member, &error); + log::debug!("[tinymemory:module] {named}"); + None + } + // A panic inside the bridge thread. Nothing here can panic today, + // but a probe that returned a plausible answer after one would be + // worse than one that says it could not tell. + Err(_) => { + log::error!( + "[tinymemory:module] composio host probe {member} panicked; \ + answering as unreachable" + ); + None + } + } + } +} + +/// Whether `error` means "this host exports no such interface". +/// +/// Matched on [`tinybus::Error::wire_name`] rather than on the enum, because a +/// remote failure is reconstructed as `MethodFailed { name, message }` on this +/// side — the structured variants exist on the *raising* side only, and +/// matching them here would silently never fire. +/// +/// The four names are the whole "nobody is listening" family: no peer owns the +/// name, the peer exports no such object, the object has no such interface, and +/// the interface has no such member. The last one is what an older host with a +/// newer module actually produces. +fn is_unserved(error: &tinybus::Error) -> bool { + matches!( + error.wire_name(), + "ai.tinyhumans.tinybus.Error.NameHasNoOwner" + | "ai.tinyhumans.tinybus.Error.UnknownObject" + | "ai.tinyhumans.tinybus.Error.UnknownInterface" + | "ai.tinyhumans.tinybus.Error.UnknownMethod" + ) +} + +#[async_trait] +impl ComposioHost for BusComposioHost { + /// The user's connections, as the host sees them right now. + /// + /// The `config` argument is dropped rather than forwarded. In module mode + /// it is the *engine's* config, built from the `ModuleConfig` this module + /// was loaded with, and it is not the host's — sending it would ask the + /// host to resolve a Composio client against a config it did not write. + /// `ChatHost` makes the same call for the same reason: `Complete` carries a + /// role and a request and nothing else. + async fn list_connections( + &self, + _config: &tinymemory_core::Config, + ) -> Result, String> { + self.call(LIST_CONNECTIONS_METHOD, ()).await + } + + /// Run `tool`, host-side, against `connection_id`. + /// + /// A provider that answers `successful: false` is **not** an error — that + /// rides back in the [`ComposioExecuteResponse`], because the sync layer + /// bills a completed round trip either way. + async fn execute( + &self, + _config: &tinymemory_core::Config, + tool: &str, + arguments: Option, + entity_id: &str, + connection_id: Option<&str>, + ) -> Result { + log::debug!("[tinymemory:module] composio execute tool={tool}"); + self.call( + EXECUTE_METHOD, + ( + tool.to_string(), + arguments, + entity_id.to_string(), + connection_id.map(str::to_string), + ), + ) + .await + } + + /// The direct-mode key, or `None` when direct mode is unset *or* the host + /// could not be asked. + /// + /// The two are not distinguishable through this signature, and that is + /// tolerable here only because the caller turns both into the same named + /// failure: `composio_config` reports "Composio direct API key is not + /// configured" and refuses to build a client. An unreachable host is + /// additionally reported once through the error reporter by + /// `probe`, so the log distinguishes what the return value cannot. + fn api_key(&self, _config: &tinymemory_core::Config) -> Option { + self.probe::>(API_KEY_METHOD).flatten() + } + + /// Whether the sync layer should treat the user as signed in. + /// + /// # An unreachable host answers *yes*, deliberately + /// + /// This probe has no error channel, so an unreachable host has to be + /// reported as one of the two real answers, and the two are not + /// symmetrical. A wrong `false` makes `ProviderContext::from_config` return + /// `None`, which the sync layer logs at debug and treats as "the user is + /// not signed in" — the run reports nothing to do and looks healthy while + /// no memory is being synced at all. A wrong `true` costs one more call, + /// which reaches `execute` and fails with a named cause that says + /// the Composio host is unserved. + /// + /// One of those is discoverable from a log and the other is not, so this + /// answers `true` whenever it could not ask — including when the host is + /// already known to serve no Composio interface, where the goal is + /// precisely to let the next call fail loudly. Only a host that actually + /// answered `false` reads as "not signed in". + fn is_available(&self, _config: &tinymemory_core::Config) -> bool { + self.probe::(IS_AVAILABLE_METHOD).unwrap_or(true) + } +} + +#[cfg(test)] +#[path = "composio_test.rs"] +mod test; diff --git a/crates/tinymemory-module/src/composio_test.rs b/crates/tinymemory-module/src/composio_test.rs new file mode 100644 index 00000000..85f8f27e --- /dev/null +++ b/crates/tinymemory-module/src/composio_test.rs @@ -0,0 +1,265 @@ +//! Tests for the host-owned Composio bridge over an in-memory TinyBus. +//! +//! Every test here runs multi-threaded with an explicit worker count, and both +//! halves of that matter. `api_key` and `is_available` block their calling +//! thread while the host answers, so the broker and the fake host's dispatch +//! loop need a worker that is not the one waiting — and `worker_threads` +//! defaults to the core count, which on a one-core CI box would leave exactly +//! one. Pinning it makes the test independent of the machine rather than +//! hanging on the small ones. + +use tinybus::broker::Broker; +use tinybus::transport::memory::MemoryBus; +use tinybus::{Connection, Result as BusResult}; +use tinymemory_core::composio_host::{ComposioConnection, ComposioExecuteResponse, ComposioHost}; + +use super::{ + BusComposioHost, COMPOSIO_HOST_BUS_NAME, COMPOSIO_HOST_OBJECT_PATH, COMPOSIO_UNSERVED, +}; +use crate::config::ModuleConfig; + +/// What the fake host was asked to execute. +#[derive(Debug)] +struct Executed { + tool: String, + arguments: Option, + entity_id: String, + connection_id: Option, +} + +struct FakeComposioHost { + executed: tokio::sync::mpsc::UnboundedSender, + api_key: Option, + available: bool, +} + +#[tinybus::interface(name = "ai.tinyhumans.tinymemory.ComposioHost")] +impl FakeComposioHost { + async fn list_connections(&self) -> BusResult> { + std::future::ready(()).await; + Ok(vec![ComposioConnection { + id: "connection-1".to_string(), + toolkit: "Gmail".to_string(), + status: "ACTIVE".to_string(), + created_at: None, + account_email: Some("user@example.com".to_string()), + workspace: None, + username: None, + }]) + } + + async fn execute( + &self, + tool: String, + arguments: Option, + entity_id: String, + connection_id: Option, + ) -> BusResult { + std::future::ready(()).await; + let _ = self.executed.send(Executed { + tool, + arguments, + entity_id, + connection_id, + }); + Ok(ComposioExecuteResponse { + data: serde_json::json!({ "messages": 2 }), + successful: true, + error: None, + cost_usd: 0.25, + markdown_formatted: Some("two messages".to_string()), + }) + } + + async fn api_key(&self) -> BusResult> { + std::future::ready(()).await; + Ok(self.api_key.clone()) + } + + async fn is_available(&self) -> BusResult { + std::future::ready(()).await; + Ok(self.available) + } +} + +async fn bus_with_composio_host( + api_key: Option<&str>, + available: bool, +) -> (Connection, tokio::sync::mpsc::UnboundedReceiver) { + let bus = MemoryBus::new(); + let broker = Broker::new(); + let _broker_task = broker.spawn(bus.clone()); + let (executed, receiver) = tokio::sync::mpsc::unbounded_channel(); + let host = Connection::connect(bus.connect().await.expect("host transport")) + .await + .expect("host connection"); + host.serve_at( + COMPOSIO_HOST_OBJECT_PATH.try_into().expect("object path"), + FakeComposioHost { + executed, + api_key: api_key.map(str::to_string), + available, + }, + ) + .await + .expect("serve composio host"); + host.request_name(COMPOSIO_HOST_BUS_NAME) + .await + .expect("claim composio host name"); + std::mem::forget(host); + let module = Connection::connect(bus.connect().await.expect("module transport")) + .await + .expect("module connection"); + (module, receiver) +} + +/// A bare connection with nobody serving the Composio name. +async fn bus_without_composio_host() -> Connection { + let bus = MemoryBus::new(); + let broker = Broker::new(); + let _broker_task = broker.spawn(bus.clone()); + Connection::connect(bus.connect().await.expect("transport")) + .await + .expect("connection") +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn connections_execute_and_probes_all_cross_the_composio_bridge() { + let (connection, mut executed) = bus_with_composio_host(Some("direct-key"), true).await; + let bridge = BusComposioHost::new(connection); + let config = tinymemory_tinycortex::engine::EngineRuntimeConfig::from(&ModuleConfig::default()); + + let connections = bridge + .list_connections(&config) + .await + .expect("host lists connections"); + assert_eq!(connections.len(), 1); + assert_eq!(connections[0].id, "connection-1"); + // The engine normalises the slug itself; the bridge must not do it for the + // host, or a toolkit would arrive pre-mangled on one path only. + assert_eq!(connections[0].toolkit, "Gmail"); + assert!(connections[0].is_active()); + + let response = bridge + .execute( + &config, + "GMAIL_FETCH_EMAILS", + Some(serde_json::json!({ "max_results": 5 })), + "entity-7", + Some("connection-1"), + ) + .await + .expect("host executes the tool"); + assert!(response.successful); + assert!((response.cost_usd - 0.25).abs() < f64::EPSILON); + assert_eq!(response.markdown_formatted.as_deref(), Some("two messages")); + assert_eq!(response.data["messages"], 2); + + // Every argument the engine passed reaches the host unchanged. `entity_id` + // and `connection_id` matter most: backend mode ignores both, so a bridge + // that dropped them would look correct until a user switched to direct + // mode and their connection pin silently disappeared. + let recorded = executed.try_recv().expect("execute reached the host"); + assert_eq!(recorded.tool, "GMAIL_FETCH_EMAILS"); + assert_eq!(recorded.entity_id, "entity-7"); + assert_eq!(recorded.connection_id.as_deref(), Some("connection-1")); + assert_eq!( + recorded.arguments, + Some(serde_json::json!({ "max_results": 5 })) + ); + + assert_eq!(bridge.api_key(&config).as_deref(), Some("direct-key")); + assert!(bridge.is_available(&config)); + + let rendered = format!("{bridge:?}"); + assert!(rendered.contains("BusComposioHost"), "{rendered}"); + assert!(!rendered.contains("Connection"), "{rendered}"); +} + +/// A served host that answers "no" is the only thing that reads as "no". +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_host_that_answers_no_is_believed() { + let (connection, _executed) = bus_with_composio_host(None, false).await; + let bridge = BusComposioHost::new(connection); + let config = tinymemory_tinycortex::engine::EngineRuntimeConfig::from(&ModuleConfig::default()); + + assert!(bridge.api_key(&config).is_none()); + assert!(!bridge.is_available(&config)); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn an_unserved_host_is_named_and_the_probes_bias_towards_a_loud_failure() { + let bridge = BusComposioHost::new(bus_without_composio_host().await); + let config = tinymemory_tinycortex::engine::EngineRuntimeConfig::from(&ModuleConfig::default()); + + let error = bridge + .list_connections(&config) + .await + .expect_err("no composio host is served"); + assert!(error.contains(COMPOSIO_UNSERVED), "{error}"); + assert!(error.contains(COMPOSIO_HOST_BUS_NAME), "{error}"); + assert!(error.contains(super::LIST_CONNECTIONS_METHOD), "{error}"); + + // The structural fact is latched, so later probes answer locally instead of + // re-dialling a name that is known to have no owner. + assert!(format!("{bridge:?}").contains("host_serves: false")); + + // `None` here becomes "Composio direct API key is not configured" one frame + // up, which is a named refusal rather than a silent skip. + assert!(bridge.api_key(&config).is_none()); + // And this stays `true` on purpose: a `false` would make the sync layer + // report "not signed in" and skip quietly, where `true` lets the next call + // fail with the unserved message asserted above. + assert!(bridge.is_available(&config)); + + let execute_error = bridge + .execute(&config, "GMAIL_FETCH_EMAILS", None, "entity-7", None) + .await + .expect_err("no composio host is served"); + assert!(execute_error.contains(COMPOSIO_UNSERVED), "{execute_error}"); +} + +/// An `Execute` failure must never quote what was executed. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn an_execute_failure_never_carries_its_arguments() { + let bridge = BusComposioHost::new(bus_without_composio_host().await); + let config = tinymemory_tinycortex::engine::EngineRuntimeConfig::from(&ModuleConfig::default()); + + let error = bridge + .execute( + &config, + "GMAIL_FETCH_EMAILS", + Some(serde_json::json!({ "query": "from:accountant@example.com" })), + "entity-7", + Some("connection-1"), + ) + .await + .expect_err("no composio host is served"); + assert!(!error.contains("accountant@example.com"), "{error}"); + assert!(!error.contains("query"), "{error}"); +} + +#[test] +fn only_the_nobody_is_listening_family_reads_as_unserved() { + let unserved = |name: &str| { + super::is_unserved(&tinybus::Error::MethodFailed { + name: name.to_string(), + message: "irrelevant".to_string(), + }) + }; + + // Every remote failure arrives as `MethodFailed`, so these four names are + // the only evidence this side gets that nobody is listening. `UnknownMethod` + // is the one an older host with a newer module actually produces. + assert!(unserved("ai.tinyhumans.tinybus.Error.NameHasNoOwner")); + assert!(unserved("ai.tinyhumans.tinybus.Error.UnknownObject")); + assert!(unserved("ai.tinyhumans.tinybus.Error.UnknownInterface")); + assert!(unserved("ai.tinyhumans.tinybus.Error.UnknownMethod")); + + // A host that ran the method and failed is a working seam having a bad day: + // reporting it as unserved would latch the bridge off for the rest of the + // process over one expired session. + assert!(!unserved("ai.tinyhumans.tinymemory.Error.Host")); + assert!(!unserved("ai.tinyhumans.tinybus.Error.Failed")); + assert!(!unserved("ai.tinyhumans.tinybus.Error.Timeout")); +} diff --git a/crates/tinymemory-module/src/config_loader.rs b/crates/tinymemory-module/src/config_loader.rs new file mode 100644 index 00000000..da3cd8f8 --- /dev/null +++ b/crates/tinymemory-module/src/config_loader.rs @@ -0,0 +1,186 @@ +//! The engine's config loader, answered from what the module was handed. +//! +//! # Why this one is *not* a bus proxy +//! +//! Every other seam in this crate goes to the host, and the reason is always +//! the same: the host holds live state the module cannot be handed once — a +//! credential, an inference route, the user's Composio connections. This seam +//! is the one where that reasoning runs the other way. +//! +//! The module is handed [`crate::config::ModuleConfig`] at load. It is the +//! host's own configuration, already resolved by the host's loader with the +//! host's env overrides and migrations applied, already narrowed to what the +//! engine reads, and already the thing every engine call in this process runs +//! against — `provider::provider` and the queue worker pool both take their +//! [`EngineRuntimeConfig`] from it. Asking the host to re-read a config the +//! module was handed would introduce a *second* answer to a question that +//! already has one, and the interesting case is not when the two agree. +//! +//! They can disagree in both directions. `ConfigLoader::load` is documented to +//! "follow the ambient environment", so a host with more than one workspace can +//! answer for a different one than the module is bound to — and the module's +//! store, queue and summary tree are all rooted at +//! `ModuleConfig::workspace_dir`. A loader that answered from somewhere else +//! would hand a sync loop in this process a config pointing at another user's +//! workspace, which is precisely the cross-workspace leak the engine's own +//! `get_source_in` exists to avoid. +//! +//! # What this costs, stated rather than hidden +//! +//! `tinymemory_core::config_loader`'s whole purpose is *freshness*: background +//! loops re-load so a mid-session settings change takes effect on the next tick +//! rather than the next restart, and `ProviderContext::execute` re-reads on +//! every call so a `composio.mode` toggle is honoured immediately. Answering +//! from the load-time snapshot gives up exactly that. A user who changes a +//! setting after this module loaded gets the old value from anything in this +//! process until the host reloads the module. +//! +//! That is a real degradation and it is reported once per process the first +//! time anything consults this loader — `report_unserved_once`, the same +//! latch-and-report the scheduler-gate and shutdown stubs use. Closing it +//! properly means a host-pushed config signal (this module declares +//! `signals = []`), not a bus *pull*: a pull would re-introduce the two-answers +//! problem above while still being stale between ticks. +//! +//! # One gap this loader cannot paper over +//! +//! `EngineRuntimeConfig::memory_sync_interval_secs` answers `Some(0)`, and +//! the contract reads `Some(0)` as **manual only**. So a periodic sync loop +//! started inside this process would consider every source manual and skip it — +//! silently, which is the failure class this migration keeps producing. +//! +//! It is left as it is on purpose. `ModuleConfig` carries no cadence field, so +//! answering anything else would mean this module *guessing* at a user setting +//! it was never told — the same argument `crate::host` gives for refusing to +//! synthesise a scheduler-gate policy from `ModuleConfig::scheduler_gate`, and +//! the same conclusion: guessing is worse than not answering. The honest fix is +//! for the host to send the cadence in `ModuleConfig`, at which point this +//! loader answers it without further change. Until then, nothing in this +//! process starts a periodic sync loop, and this note is why. + +use std::sync::atomic::AtomicBool; +use std::sync::Arc; + +use async_trait::async_trait; +use tinymemory_core::config_loader::ConfigLoader; +// The trait, not only the alias: `config_path` is reached as a METHOD on both +// sides of the comparison below, and `EngineRuntimeConfig` also has a field of +// that name. Without the trait in scope the method call resolves to nothing and +// rustc points at the field, which would compare a path against a path-shaped +// field on a different type. +use tinymemory_api::host::MemoryHostConfig; +use tinymemory_core::Config; +use tinymemory_tinycortex::engine::EngineRuntimeConfig; + +use crate::config::ModuleConfig; +use crate::host::report_unserved_once; + +/// Latched so the degradation is named once per process rather than once per +/// call — `ProviderContext::execute` reloads on *every* Composio action, and an +/// unlatched report would page per tool call. +static LOADER_REPORTED: AtomicBool = AtomicBool::new(false); + +/// What answering locally costs, in the terms a reader of the log needs. +const CONFIG_LOADER_FROZEN: &str = "config loader answered from the module's load-time snapshot: \ + this module re-reads no config file, so a settings change \ + made after it loaded (Composio mode, sync cadence, a memory \ + source switched off) does not reach the engine in this \ + process until the host reloads the module"; + +/// Refusal message for a snapshot this module was not loaded for. +/// +/// Names no path. A workspace path identifies a user, and a module error +/// crosses the bus into logs that are not this module's to decide about — the +/// same rule [`ModuleConfig::validate`] follows. +const FOREIGN_SNAPSHOT: &str = "config loader was asked to re-read a snapshot from a different \ + workspace than the one this module was loaded for; this module \ + serves exactly one workspace and will not answer for another"; + +/// The engine's [`ConfigLoader`], served from [`ModuleConfig`]. +#[derive(Debug)] +pub struct ModuleConfigLoader { + /// Behind an `Arc` because `reload_snapshot` hands back a shared handle and + /// `load` hands back an owned one; keeping one canonical value means the + /// two can never answer differently. + snapshot: Arc, +} + +impl ModuleConfigLoader { + /// Build the loader from the config this module was handed. + /// + /// # The credential is dropped here too + /// + /// `EngineRuntimeConfig::from` clones `ModuleConfig::memory` wholesale, and + /// that struct carries `agentmemory_secret` — a bearer token for a remote + /// memory backend. `setup` already strips it before this is built, so this + /// clears nothing in practice today. It is here because this type's whole + /// job is to *hand the config back out*, repeatedly, to any engine code + /// that asks: a future caller that built a loader before the strip, or from + /// a config that never went through `setup`, would turn one missed ordering + /// into a token handed to every consumer. Defence in depth costs one line + /// and removes a whole class of ordering bug. + #[must_use] + pub fn new(config: &ModuleConfig) -> Self { + let mut snapshot = EngineRuntimeConfig::from(config); + snapshot.memory.agentmemory_secret = None; + Self { + snapshot: Arc::new(snapshot), + } + } +} + +#[async_trait] +impl ConfigLoader for ModuleConfigLoader { + /// The config this module was loaded with. + /// + /// A `Box`, not an `Arc`, because the contract's callers include config + /// *migrations* that need `&mut`. Those writes land on the copy and go + /// nowhere: `EngineRuntimeConfig::save` is a no-op, since this module has + /// no config file to write and inventing one would put a second writer on + /// the host's. The composio source-caps migration is the one caller that + /// notices — it re-runs each time rather than recording that it ran. + /// + /// # Errors + /// + /// Never. The answer is a clone of a value this module already holds; there + /// is no read to fail. The `Result` is the contract's, shaped for a host + /// that reads a file. + async fn load(&self) -> Result, String> { + report_unserved_once(&LOADER_REPORTED, CONFIG_LOADER_FROZEN, "config_loader"); + let owned: Box = Box::new((*self.snapshot).clone()); + Ok(owned) + } + + /// Re-read the config `snapshot` came from — which, here, is this one. + /// + /// The contract distinguishes this from `load` because it + /// follows the ambient environment and can land on a different workspace + /// than the caller is working in. In this module both answer from the same + /// value, so the distinction collapses — except for the check below, which + /// is the one thing the distinction still buys. + /// + /// # Errors + /// + /// `FOREIGN_SNAPSHOT` when `snapshot` was loaded from a different + /// workspace. Answering with this module's config would be worse than + /// failing: the caller asked to re-read *its* config and would silently get + /// another workspace's, which is how a sync run writes one user's data into + /// another user's store. The paths are compared rather than the values + /// because `config_path` is what the contract itself calls the anchor. + async fn reload_snapshot(&self, snapshot: &Config) -> Result, String> { + report_unserved_once(&LOADER_REPORTED, CONFIG_LOADER_FROZEN, "config_loader"); + if snapshot.config_path() != self.snapshot.config_path() { + return Err(FOREIGN_SNAPSHOT.to_string()); + } + // Annotated rather than `Arc::clone`d: the field is an + // `Arc` and the contract wants an + // `Arc`, so the binding's type is what drives the + // unsizing coercion. `Arc::clone` would infer the concrete type and fail. + let shared: Arc = self.snapshot.clone(); + Ok(shared) + } +} + +#[cfg(test)] +#[path = "config_loader_test.rs"] +mod test; diff --git a/crates/tinymemory-module/src/config_loader_test.rs b/crates/tinymemory-module/src/config_loader_test.rs new file mode 100644 index 00000000..6c646c3b --- /dev/null +++ b/crates/tinymemory-module/src/config_loader_test.rs @@ -0,0 +1,96 @@ +//! Tests for the module-side config loader. + +use std::path::PathBuf; + +use tinymemory_api::host::MemoryConfig; +use tinymemory_core::config_loader::ConfigLoader; +use tinymemory_tinycortex::engine::EngineRuntimeConfig; + +use super::{ModuleConfigLoader, FOREIGN_SNAPSHOT}; +use crate::config::ModuleConfig; + +fn module_config(workspace: &str) -> ModuleConfig { + ModuleConfig { + workspace_dir: PathBuf::from(workspace), + memory_sources: serde_json::json!([{ "id": "gmail:1", "kind": "composio" }]), + ..ModuleConfig::default() + } +} + +#[tokio::test] +async fn load_answers_from_the_module_config() { + let loader = ModuleConfigLoader::new(&module_config("/tmp/module-workspace")); + + let config = loader.load().await.expect("the module always has a config"); + + assert_eq!( + config.workspace_dir(), + &PathBuf::from("/tmp/module-workspace") + ); + // The anchor `reload_snapshot` compares on, derived rather than configured: + // the module has no config file of its own, so the path it reports has to + // be the one the engine would look for inside its workspace. + assert_eq!( + config.config_path(), + &PathBuf::from("/tmp/module-workspace/config.toml") + ); + // The source registry has to survive verbatim: the periodic loops decide + // which sources are enabled by decoding exactly this value, and an empty + // one reads as "no source has an entry yet", which silently re-enables + // sources the user switched off. + let sources = config + .memory_sources_json() + .expect("memory sources round-trip"); + assert_eq!(sources[0]["id"], "gmail:1"); +} + +/// The one field that would smuggle a credential back out. +#[tokio::test] +async fn the_loader_hands_back_no_carried_credential() { + let mut config = module_config("/tmp/module-workspace"); + config.memory = MemoryConfig { + agentmemory_secret: Some("remote-backend-token".to_string()), + ..MemoryConfig::default() + }; + + let loader = ModuleConfigLoader::new(&config); + + // The input still carries it — so this asserts that the loader's copy + // diverged, not that the fixture was empty to begin with. + assert!(config.memory.agentmemory_secret.is_some()); + let answered = loader.load().await.expect("the module always has a config"); + assert!(answered.memory().agentmemory_secret.is_none()); +} + +#[tokio::test] +async fn reloading_our_own_snapshot_answers_with_the_module_config() { + let loader = ModuleConfigLoader::new(&module_config("/tmp/module-workspace")); + let snapshot = loader.load().await.expect("load"); + + let reloaded = loader + .reload_snapshot(&*snapshot) + .await + .expect("our own snapshot is re-readable"); + + assert_eq!( + reloaded.workspace_dir(), + &PathBuf::from("/tmp/module-workspace") + ); +} + +#[tokio::test] +async fn reloading_a_foreign_snapshot_is_refused_without_naming_a_path() { + let loader = ModuleConfigLoader::new(&module_config("/tmp/module-workspace")); + let foreign = EngineRuntimeConfig::from(&module_config("/tmp/somebody-elses-workspace")); + + let error = loader + .reload_snapshot(&foreign) + .await + .expect_err("a snapshot from another workspace is refused"); + + assert_eq!(error, FOREIGN_SNAPSHOT); + // A workspace path identifies a user, and this string travels back across + // the bus into logs this module does not own. + assert!(!error.contains("somebody-elses-workspace"), "{error}"); + assert!(!error.contains("module-workspace"), "{error}"); +} diff --git a/crates/tinymemory-module/src/host.rs b/crates/tinymemory-module/src/host.rs index 264502d1..62715f93 100644 --- a/crates/tinymemory-module/src/host.rs +++ b/crates/tinymemory-module/src/host.rs @@ -127,30 +127,23 @@ pub(crate) fn install(connection: Connection) { // ── Seams no bus interface serves ─────────────────────────────────────────── // -// This module serves five of the host's seams: the embedder and the chat model -// have host interfaces of their own, and the event sink, the error reporter and -// spaCy cross the bus through `BusRuntimeHost` above. The other four — -// `composio_host`, `config_loader`, and the two stubbed below — have no callback -// channel at all. +// This module serves seven of the host's nine seams. Six cross the bus: the +// embedder and the chat model have host interfaces of their own, `composio_host` +// has one too (see `crate::composio`), and the event sink, the error reporter +// and spaCy share `BusRuntimeHost` above. The seventh, `config_loader`, is +// answered locally from the `ModuleConfig` this module was handed, for the +// reason its own module docs give: proxying it would ask the host to re-read a +// config the module already has, and the interesting case is the one where the +// two answers disagree. // -// `config_loader` survives that honestly: every path returns -// `Err("no ConfigLoader installed …")`, so a module-mode loop fails with a named -// cause instead of quietly keeping a stale snapshot. `composio_host` is loud on -// the two paths that do work — `list_connections` and `execute` — and quiet on -// its two probes: unwired, `api_key` reads `None` and `is_available` reads -// `false`, which a caller cannot tell apart from "the user has no Composio -// connections". -// -// **The two below were quiet on every path.** With nothing installed, -// `scheduler_gate::current_policy()` reads `Normal` and `wait_for_capacity()` -// returns instantly — background LLM work runs flat out no matter what the user -// asked for — and `shutdown::register` drops the ingest queue's lock-release -// hook behind a `log::debug!`. That is precisely the outcome the host's own -// `install_memory_host_seams` comment says the seams exist to prevent: a sync -// run that looks empty rather than broken. Silence is the bug; these stubs are -// the fix. The two `composio_host` probes are the same bug one size down, and -// are left alone here only because closing them means inventing a bus interface -// this crate owns one half of. +// The two below are what is left, and both were quiet on every path. With +// nothing installed, `scheduler_gate::current_policy()` reads `Normal` and +// `wait_for_capacity()` returns instantly — background LLM work runs flat out no +// matter what the user asked for — and `shutdown::register` drops the ingest +// queue's lock-release hook behind a `log::debug!`. That is precisely the +// outcome the host's own `install_memory_host_seams` comment says the seams +// exist to prevent: a sync run that looks empty rather than broken. Silence is +// the bug; these stubs are the fix. // // # Why stubs and not bus proxies // @@ -176,11 +169,11 @@ pub(crate) fn install(connection: Connection) { // // Each stub returns exactly what the unwired path returned, so installing them // changes no scheduling and wedges nothing, and each reports once per process -// the first time anything actually consults it. Nothing in this process consults -// them today — the module starts none of the background loops (`queue::start`, -// `start_periodic_sync` and `start_workspace_periodic_sync` are all called -// host-side) — so the report fires on the day someone moves one in here, which -// is the day the gap stops being theoretical. +// the first time anything actually consults it. The queue worker pool now runs +// in here (`crate::start_queue_pool`) and consults both, so the scheduler-gate +// report fires on every boot that drains a job — which is the honest signal that +// the throttle is not in effect. The two periodic sync loops are still started +// host-side, so nothing in this process reaches them through that path yet. // // Note also what is deliberately *not* built here: a module-local registry that // banked shutdown hooks and drained them on the module's own `Shutdown` method. @@ -207,17 +200,27 @@ const SHUTDOWN_UNSERVED: &str = "shutdown host unserved in module mode: a memory was dropped, so in-flight queue job locks are not released on a \ clean exit and the next launch waits out the lease instead"; -/// Log and report an unserved seam once per process. -fn report_unserved_once(latch: &AtomicBool, message: &'static str, operation: &'static str) { +/// Log and report a seam degradation once per process. +/// +/// Shared with [`crate::composio`] and [`crate::config_loader`], which have +/// their own latches and their own messages but need exactly this behaviour: +/// one `log::error!` unconditionally, one classified report when a runtime +/// exists to send it on, and nothing at all on every later call. Each caller +/// owns its latch so one seam going quiet never silences another. +pub(crate) fn report_unserved_once( + latch: &AtomicBool, + message: &'static str, + operation: &'static str, +) { if latch.swap(true, Ordering::SeqCst) { return; } log::error!("[tinymemory:module] {message}"); // The error reporter reaches the host by spawning onto the module runtime, - // and two of the three call sites below are sync methods a future caller - // could reach from a plain thread. The log line above is unconditional; - // only the telemetry needs a runtime to exist, so the gap is never silent - // even when the report cannot be sent. + // and several call sites are sync methods a caller could reach from a plain + // thread — the scheduler-gate stub below and both Composio probes. The log + // line above is unconditional; only the telemetry needs a runtime to exist, + // so the gap is never silent even when the report cannot be sent. if tokio::runtime::Handle::try_current().is_ok() { tinymemory_core::observability::report_error_or_expected( message, @@ -290,9 +293,8 @@ pub(crate) fn install_unserved_seams() { // boot, and a reader of the log should not have to diff seam lists to find // out that the throttle and the graceful lock release are not in effect. log::warn!( - "[tinymemory:module] four host seams are unserved in module mode: composio_host and \ - config_loader are absent and their work paths fail with a named cause; scheduler_gate \ - and shutdown are stubs that keep the unwired behaviour and report once when consulted. \ + "[tinymemory:module] two host seams are unserved in module mode: scheduler_gate and \ + shutdown are stubs that keep the unwired behaviour and report once when consulted. \ Background-AI throttling and graceful queue-lock release are not honoured inside this \ process" ); diff --git a/crates/tinymemory-module/src/lib.rs b/crates/tinymemory-module/src/lib.rs index c82979e0..231ae94d 100644 --- a/crates/tinymemory-module/src/lib.rs +++ b/crates/tinymemory-module/src/lib.rs @@ -43,6 +43,15 @@ //! [`config::ModuleConfig::strip_host_credentials`], not merely asserted about a //! field list. "Carried verbatim" carries credentials verbatim too. //! +//! The claim is about *configuration*, and there is exactly one place it stops +//! there: [`composio`]'s `ApiKey` fetches the user's direct-mode Composio key +//! from the host for the duration of one call. It is stated here rather than +//! buried because the difference matters — the engine's `composio_config` +//! builds its own HTTP client from that key, so unlike an embed there is no +//! host-side call to route the work through, and refusing it would mean +//! direct-mode memory sync simply cannot run. Nothing stores it; there is still +//! no field it could be stored in. +//! //! # Scope: the complete TinyMemory API //! //! The module boundary mirrors every capability family in `tinymemory_api`. @@ -63,14 +72,21 @@ )] pub mod chat; +pub mod composio; pub mod config; +pub mod config_loader; pub mod embedding; mod host; mod provider; mod service; pub use chat::{CHAT_HOST_BUS_NAME, CHAT_HOST_INTERFACE, CHAT_HOST_OBJECT_PATH}; +pub use composio::{ + BusComposioHost, API_KEY_METHOD, COMPOSIO_HOST_BUS_NAME, COMPOSIO_HOST_INTERFACE, + COMPOSIO_HOST_OBJECT_PATH, EXECUTE_METHOD, IS_AVAILABLE_METHOD, LIST_CONNECTIONS_METHOD, +}; pub use config::ModuleConfig; +pub use config_loader::ModuleConfigLoader; pub use embedding::{ BusEmbeddingHost, BusEmbeddingProvider, EMBEDDING_HOST_BUS_NAME, EMBEDDING_HOST_INTERFACE, EMBEDDING_HOST_OBJECT_PATH, @@ -135,12 +151,27 @@ async fn setup(connection: Connection, mut config: ModuleConfig) -> BusResult<() connection.clone(), &config, ))); + // Composio is host state end to end — the connection list, the direct key, + // and whether any client resolves at all change on an OAuth completion or a + // `set_api_key` RPC with nothing restarting — so this one is a proxy and + // holds no snapshot. See `composio` for why the direct-mode key is the one + // credential that does cross. + tinymemory_core::composio_host::set_composio_host(Arc::new(composio::BusComposioHost::new( + connection.clone(), + ))); + // The config loader is the opposite call, and deliberately: it is answered + // from `config` — which is this line's whole argument — rather than asking + // the host to re-read what it already handed over. It goes *after* the + // credential strip above, because this is the seam that hands the config + // back out to the engine repeatedly. + tinymemory_core::config_loader::set_config_loader(Arc::new(ModuleConfigLoader::new(&config))); host::install(connection.clone()); - // The seams no bus interface serves. Two of the four degraded in silence - // rather than with a named cause; see the section comment on - // `host::install_unserved_seams` for why they are stubbed here rather than - // proxied or synthesised. Installed with the rest, before the store exists, - // so nothing can consult a seam this process has not yet decided about. + // The two seams no bus interface serves, and no local answer can honestly + // stand in for. Both degraded in silence rather than with a named cause; + // see the section comment on `host::install_unserved_seams` for why they + // are stubbed here rather than proxied or synthesised. Installed with the + // rest, before the store exists, so nothing can consult a seam this process + // has not yet decided about. host::install_unserved_seams(); let client = tinymemory_core::store::factories::create_memory_client_with_local_ai( @@ -165,6 +196,49 @@ async fn setup(connection: Connection, mut config: ModuleConfig) -> BusResult<() // creates the workspace it lives in. start_queue_pool(&config); + // ── The periodic sync loops are deliberately NOT started here ─────────── + // + // This is the obvious next line to write — the queue pool moved in here for + // exactly the reason the sync loops would, and `composio_host` and + // `config_loader` are now installed above, which is what a reader would + // check first. It does not work yet, and it would fail *quietly*, so the + // reasons are written down rather than left to be rediscovered. + // + // `tinymemory_core::sync::composio::start_periodic_sync` dispatches through + // `sync::pipelines::host::run_composio_connection_with_caps`, and three + // separate things in that path have no answer in this process: + // + // 1. **The pipeline reads credentials off the `Config`, not off the seam.** + // `composio_config` takes the direct-mode branch only when + // `config.composio().mode == "direct"` and otherwise needs + // `config.session_token()`. `EngineRuntimeConfig` answers + // `ComposioMode::default()` (mode `""`) and `Ok(None)`, so backend mode + // fails with "backend bearer token is not configured" and direct mode is + // never selected at all. `ComposioHost::api_key` cannot rescue this: the + // seam is consulted *inside* the direct branch that is not taken. The + // real fix is to route the pipeline's own HTTP client through + // `ComposioHost::execute`, which is a change to the engine's contract. + // + // 2. **`crate::global::client_if_ready()` is `None` here.** That is the + // first line of every pipeline run. This module builds its store through + // `create_memory_client_with_local_ai`, which does not touch the global + // slot, and calling `global::init` would build a *second* client via + // `MemoryClient::from_workspace_dir` — different embedding routes, a + // second ingestion worker over the same SQLite file. + // + // 3. **The cadence reads as "manual only".** + // `EngineRuntimeConfig::memory_sync_interval_secs()` is `Some(0)`, which + // the contract defines as manual-only, so both loops would skip every + // source on every tick. This is the one that would be invisible: no + // error, no warning, just a sync that never fires. See + // `config_loader`'s module docs for why the loader does not invent a + // different number. + // + // A fourth consequence is worth knowing even once those are fixed: this + // module's scheduler gate is a stub that always reads `Normal`, so a sync + // loop in here would not honour the "signed out" and "user disabled" pauses + // that `periodic_pause_reason` exists to apply. + let provider = provider::provider(&config, Arc::new(client)); service::serve(&connection, Arc::new(provider), config).await } diff --git a/vendor/tinycortex b/vendor/tinycortex index ef09f096..84c994b7 160000 --- a/vendor/tinycortex +++ b/vendor/tinycortex @@ -1 +1 @@ -Subproject commit ef09f0964fe9e00d04ec305f3956ffe9ff9bbb41 +Subproject commit 84c994b71eeeb1df1dd5669253353e19e2c2b62c