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-api/src/provider/chunks.rs b/crates/tinymemory-api/src/provider/chunks.rs index 1a8f96d0..feff6fdf 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. //! @@ -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; @@ -40,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. /// @@ -67,6 +70,118 @@ 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)) + } + + /// 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/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..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, 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 @@ -31,6 +33,46 @@ 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. +/// +/// ## `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` @@ -79,6 +121,154 @@ 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 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 chunks it already has — a page of retrieval hits, a + /// screen of a browser — without re-running extraction over the text. + /// + /// 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. + /// + /// # 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 + /// 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 + /// + /// [`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")) + } + + /// 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..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, EntityRef, ExportPage, ExportRecord, FlushOutcome, - ImportOutcome, IngestItem, IngestOutcome, MaintenanceReport, ResetOutcome, SnapshotRef, + 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 65b94e84..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 99 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 71dade98..f3eb2e48 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. @@ -128,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. @@ -169,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. @@ -197,6 +211,14 @@ 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"; + /// `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. @@ -260,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; 99] = [ +pub const METHODS: [&str; 109] = [ methods::DRIVER_ID, methods::CAPABILITIES, methods::HEALTH, @@ -360,6 +382,16 @@ 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, + 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 d4b5356c..f9b40c0d 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)] @@ -330,6 +354,83 @@ 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, +} + +/// 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 @@ -450,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 @@ -606,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-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..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 { @@ -67,6 +68,50 @@ 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) +} + +/// 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)) } @@ -147,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 8f81f89b..7836d96b 100644 --- a/crates/tinymemory-core/src/store/entities.rs +++ b/crates/tinymemory-core/src/store/entities.rs @@ -166,32 +166,239 @@ 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( + rusqlite::params![kind, i64::try_from(limit).unwrap_or(i64::MAX)], + index_row, + )? + .collect::>>()?; + Ok(rows) +} + +/// 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. +/// +/// 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. +/// +/// # 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 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 +/// 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([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![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-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-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/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/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 b46674ae..62715f93 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,181 @@ pub(crate) fn install(connection: Connection) { tinymemory_core::nlp_host::set_nlp_host(host); } +// ── Seams no bus interface serves ─────────────────────────────────────────── +// +// 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. +// +// 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 +// +// 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. 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. +// 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 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 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, + "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] 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" + ); +} + #[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 4cd2c94e..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, @@ -78,8 +94,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}; @@ -134,7 +151,28 @@ 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 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( &config.memory, @@ -153,10 +191,176 @@ 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); + + // ── 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 } +/// 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 +410,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", @@ -248,6 +460,9 @@ mod exports { "ChunkDetail", "StorageKinds", "ChunkEmbeddings", + "CountChunks", + "ListChunkDetails", + "SourceTotals", // Retrieval. "FastRetrieve", "CoverWindow", @@ -289,6 +504,9 @@ mod exports { "Entities", "EntityEdges", "TouchEntities", + "TopEntities", + "ChunkEntities", + "EntityChunkIds", "KvGet", "KvPut", "KvDelete", @@ -305,6 +523,7 @@ mod exports { "DeleteToolRule", "AcceptSourceItems", "ForgetSource", + "ForgetMatching", "Reembed", "Compact", "Consolidate", @@ -316,7 +535,11 @@ mod exports { "BackfillInProgress", "FlushPending", "ResetDerivedIndex", + "PurgeAll", "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..c5c12a51 100644 --- a/crates/tinymemory-module/src/service/mod.rs +++ b/crates/tinymemory-module/src/service/mod.rs @@ -28,10 +28,13 @@ //! SeedFromAddressBook() -> AddressBookSeedOutcome //! //! ListChunks(query, scope) -> [Chunk] +//! CountChunks(query, scope) -> u64 //! GetChunk(chunk_id) -> Option //! 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) @@ -47,6 +50,16 @@ //! 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_ids, kinds) -> [ChunkEntityOccurrence] +//! EntityChunkIds(entity_id, limit) -> [String] +//! +//! ForgetMatching(selector) -> ForgetOutcome +//! PurgeAll() -> PurgeOutcome //! ``` //! //! # Source scope crosses as an argument, never as ambient state @@ -120,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, ExportPage, ExportRecord, FlushOutcome, ImportOutcome, IngestItem, - IngestOutcome, MaintenanceReport, QueueFailure, QueueStats, ResetOutcome, SnapshotRef, + 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, @@ -141,7 +157,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, @@ -403,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 @@ -1427,6 +1462,204 @@ 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 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 + /// 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_ids, kinds.as_deref()) + .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) + } + + /// 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 49bdab7f..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::{ @@ -614,6 +614,9 @@ const EXPECTED_METHODS: &[&str] = &[ "ChunkDetail", "StorageKinds", "ChunkEmbeddings", + "CountChunks", + "ListChunkDetails", + "SourceTotals", // Retrieval. "FastRetrieve", "CoverWindow", @@ -653,6 +656,9 @@ const EXPECTED_METHODS: &[&str] = &[ "Entities", "EntityEdges", "TouchEntities", + "TopEntities", + "ChunkEntities", + "EntityChunkIds", "KvGet", "KvPut", "KvDelete", @@ -669,6 +675,7 @@ const EXPECTED_METHODS: &[&str] = &[ "DeleteToolRule", "AcceptSourceItems", "ForgetSource", + "ForgetMatching", "Reembed", "Compact", "Consolidate", @@ -680,7 +687,10 @@ const EXPECTED_METHODS: &[&str] = &[ "BackfillInProgress", "FlushPending", "ResetDerivedIndex", + "PurgeAll", "RecallNamespaceRecent", + "SummaryForest", + "RecentLeaves", ]; #[tokio::test] @@ -1041,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", @@ -1163,9 +1194,15 @@ 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}; + use tinymemory_api::provider::chunks::{ + ChunkDetail, ChunkEmbedding, ChunkListRow, ChunkQuery, SourceTotal, + }; use tinymemory_api::provider::types::{IngestItem, IngestOutcome}; let ingest = IngestItem { @@ -1183,6 +1220,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(),)) @@ -1232,6 +1273,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 @@ -1241,6 +1286,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", @@ -1267,6 +1335,58 @@ 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" + ); + + // 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 } @@ -1411,6 +1531,34 @@ 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", + (vec!["chunk-1".to_string()], Option::>::None), + ) + .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) { @@ -1450,6 +1598,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 a05569b3..5ebc3982 100644 --- a/crates/tinymemory-tinycortex/src/engine/mod.rs +++ b/crates/tinymemory-tinycortex/src/engine/mod.rs @@ -32,8 +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, + 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 @@ -41,18 +42,20 @@ use tinymemory_api::provider::types::{ #[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; -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, @@ -586,23 +589,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(), @@ -907,6 +906,262 @@ 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()) + } +} + +/// 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] @@ -1024,6 +1279,96 @@ 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_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_ids, &kinds) + }) + .await?; + Ok(rows + .into_iter() + .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()) + } + + 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")] @@ -1116,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( @@ -1214,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] @@ -1544,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 @@ -1965,6 +2431,71 @@ 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`, `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, + offset, + 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, + offset, + source_scope: scope_to_engine(scope), + exclude_dropped, + }) +} + #[async_trait] impl MemoryChunks for TinycortexProvider { async fn list_chunks( @@ -1972,29 +2503,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 +2511,82 @@ 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 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/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 b62693db..e7f6cd0c 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. @@ -1673,6 +1751,97 @@ 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".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].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".to_string()], None) + .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 +1927,1456 @@ 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()); +} + +/// 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 8401346b..84c994b7 160000 --- a/vendor/tinycortex +++ b/vendor/tinycortex @@ -1 +1 @@ -Subproject commit 8401346b574cacb1dc0cf6b36bc608ff5ef9f6f5 +Subproject commit 84c994b71eeeb1df1dd5669253353e19e2c2b62c