Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 91 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,23 @@ Per-domain Cargo features drop whole domains **at compile time** (smaller binary

| Set | Where it lives | What it is |
| --- | --- | --- |
| **Contributor** | `[features] default` in `Cargo.toml` | What a bare `cargo check`, `cargo test` and rust-analyzer compile. 9 cheap gates. **353 packages / 3 native builds** (`libsqlite3-sys`, `lzma-sys`, `ring`). |
| **Contributor** | `[features] default` in `Cargo.toml` | What a bare `cargo check`, `cargo test` and rust-analyzer compile. 10 cheap gates. **~353 packages / 3 native builds** (`libsqlite3-sys`, `lzma-sys`, `ring`). |

> **`modules` is in `default`, and it is the one gate here that is not optional.**
> The table below has documented it as Contrib=ON since it landed and
> `scripts/ci/product-features.txt` has always listed it, but it was missing from
> `[features] default` — so a bare `cargo test --lib -- memory::` failed **26**
> tests (582 passed / 26 failed), every one a "null vs module" assertion, because
> `memory::binding::module_provider` took its `#[cfg(not(feature = "modules"))]`
> arm and bound `NullMemoryProvider`. A further 15 module-gated tests did not
> exist at all. With the gate on: **623 passed, 0 failed.** A default set that
> cannot run its own test suite is not an inner loop, so this one stays.
> It is also the cheapest gate in the list — **+9 packages / +5 unique names**
> (`ureq`, `ureq-proto`, `utf8-zero`, `toml_edit`, `toml_write`) and **zero** new
> native builds; the native list is identical with it on and off. Nothing like
> the cohorts that motivated splitting `default` from the product set. It does
> **not** move the kernel floor — that profile is `--no-default-features
> --features flows` and never reads this list.
| **Product** | `scripts/ci/product-features.txt` | What the shipped desktop app has. 16 gates. **540 packages / 7 native builds** (adds `bzip2-sys`, `libgit2-sys`, `libz-sys`, `zstd-sys`). |

`default` used to be the product set, which made the inner loop pay for the whole product on every edit — web3's ethers/secp256k1 cohort, `documents`' zstd/bzip2 native builds (since removed from the graph entirely — the codecs run in a module now), the cpal/hound/arboard/enigo/rdev stack behind `voice`+`inference`, `contacts`' macOS objc2 cohort, `crash-reporting`'s sentry tree, `tui`'s ratatui. Those are default-OFF now. **This did not change what ships**: the shell has set `default-features = false` since #1061 and never inherited `default` anyway.
Expand Down Expand Up @@ -660,6 +676,80 @@ always-on kernel surface, so `features = ["modules"]` there puts a loader plus
one — 305 → 308 packages, which the kernel-floor ratchet caught. It is forwarded
from this crate's own `modules` feature instead.

#### The memory seam — one contract, two live paths (#5560)

Memory is the second module consumer, and it is **half migrated**. Read this
before touching `src/openhuman/memory/`.

**The contract is `tinymemory-api`, and `crate::openhuman::memory::api` is a
re-export of it — not a copy.** `3ee5a3cad` inlined that crate as 10,894 lines
under `src/openhuman/memory/api/`, every file byte-identical to
`vendor/tinymemory/api/src/` apart from doc-comment paths. Nothing behaved
differently, which is what made it worth undoing: the contract is the vocabulary
the host, `ModuleMemoryProvider`, and the separately compiled module all speak,
and the module compiles against the **crate**. A verbatim copy made the host's
`MemoryError`, `Chunk`, `Capabilities` and `MemoryProvider` distinct types from
the ones on the wire. `api::wire` is where that bit hardest — its own docs, and
`modules/memory.rs`, both justify sharing the error table because
reimplementing it "is what would let a `PathEscape` arrive as an `Invalid`" —
and while the host held a private copy of that table the sentence described an
intention rather than the build. `memory/api.rs` is a short `pub use` now;
`memory/api_identity_tests.rs` pins the identity with type equalities, so a
re-inlining fails to compile rather than passing silently.

**`memory::api` is the contract surface, not an alias for the crate.** It
exports only what actually crosses the bus, derived from both directions —
outbound from `modules/memory.rs`, inbound from `modules/memory_host.rs`. Whole
namespaces where the namespace *is* wire vocabulary (`capabilities`, `chunks`,
`error`, `goals`, `health`, `provider` with its `provider::types` payloads,
`recall`, `tool_memory`, `tree`, `types`, `wire`), plus `CONTRACT_VERSION` for
version negotiation. Three exclusions are deliberate and each has a reason:

- **`host`** is re-exported as **two types, not the namespace** — only
`MemoryEvent` and `SpacyResponse` cross the bus. The rest of
`tinymemory_api::host` is the *in-process engine-embedding* seam (the
persisted `MemoryConfig` sections, `MemoryHostConfig`, `EmbeddingProvider`,
`MemoryEventSink`), which the host hands to `tinymemory-core` directly and
which never touches a module.
- **`null`** is the fallback driver `memory::binding` installs when no module is
available — what runs when nothing crosses the bus, so the opposite of
contract. Name `tinymemory_api::null` at the call site.
- **`traits`**, **`version`** and **`is_compatible`** had zero uses in `src/`;
they were alias surface only.

That is the point of the split: `tinymemory-api` is *also* the crate this host
embeds the engine through, and "the module contract" and "the host's own use of
the crate" are different surfaces. Reaching the second one by naming
`tinymemory_api::` directly keeps the difference visible in the source rather
than in someone's memory. **Do not widen `memory::api` back out to the whole
crate** — if a new path needs something not exported there, the question to
answer first is whether it crosses the bus.

**`tinymemory-api` stays; `tinymemory-core` has not left yet.** The API crate is
the host-owned contract and is meant to be a dependency. The *engine* crate is
still linked (1.44 MB of `.text`) because ~71 lines across 38 production files
name `tinymemory_core::` directly, and ~687 more paths reach it through the
twenty-five module re-exports in `memory/mod.rs`. `memory/direct_engine_refs_tests.rs`
is the ratchet over the first number, with every file classified as a re-export
shim, a host-seam installation, or a call that needs a wider bus surface.

**Most of what remains is blocked upstream, not here.** `modules::registry` pins
the TinyMemory module to a released, SHA-256-verified artifact, so a new bus
method is a `tinymemory` release plus a registry re-pin before it is a host
change. Adding a `MemoryProvider` method without that produces a driver that
answers `Unsupported` — strictly worse than the direct call, because the failure
moves from compile time to run time. The concrete gaps (retrieval filters, chunk
reads, an entity-kind filter, source listing, the people domain, and the
`source_scope` task-local) are enumerated in that lint's module docs.

**One trap worth naming: `tinymemory-api` and `tinycortex-api` are two crates
with near-identical types.** The engine's
`tinymemory_core::store::chunks::types::SourceKind` resolves to
`tinycortex_api::chunks::SourceKind`, which is **not** the contract's
`memory::api::chunks::SourceKind`. Swapping one import for the other looks like
a free type carve-out and is a type error — the module does that conversion at
its own boundary.

#### The `tui` gate

The tabbed terminal UI (`openhuman`, or explicitly `openhuman tui` / alias `chat`) lives in `src/openhuman/tui/` and follows the **`mcp`/`voice` facade+stub** pattern: `pub mod tui;` is always compiled; the behavioural submodules (`app`, `render`, `state`, `terminal`, `runner`) are `#[cfg(feature = "tui")]`; and `#[cfg(not(feature = "tui"))] mod stub;` re-exposes the one symbol an always-compiled caller reaches — `run_from_cli` — with a build-fact error body (`"tui feature disabled at compile time … --features tui"`). Bare-command auto-launch requires terminal stdin/stdout and `HostKind::Cli`; Docker, CI, pipes, and `--no-tui` retain the non-TUI CLI path.
Expand Down
17 changes: 16 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,22 @@ proptest = "1"
# gated-off domains, so every CI lane that builds or tests "the product" now
# passes `--features "$(scripts/ci/product-features.sh)"`. If you add a lane,
# decide which of the two sets it is testing and say so.
default = ["media", "skills", "flows", "mcp", "channels", "medulla", "http-server", "scheduler-gate", "file-logging"]
# `modules` is in this list because the memory seam is not optional at test
# time. `memory::binding::module_provider` has a `#[cfg(not(feature =
# "modules"))]` arm that binds `NullMemoryProvider`, so without the gate a bare
# `cargo test --lib -- memory::` fails 26 tests, every one of them a "null vs
# module" assertion, and 15 further module-gated tests do not exist at all
# (582 passed / 26 failed, versus 623 passed / 0 failed with the gate on).
# A default set that cannot run its own test suite is not a usable inner loop.
# The cost is the smallest of any gate here: +9 packages / +5 unique names
# (`ureq`, `ureq-proto`, `utf8-zero`, `toml_edit`, `toml_write`) and **zero**
# new native builds — the native list is byte-identical with the gate on and
# off. That is nothing like the cohorts #4919 moved out of `default`, and it
# also brings the build in line with what AGENTS.md has always documented
# (`modules`: Contrib=ON, Product=ON) and with `scripts/ci/product-features.txt`,
# which already lists it. This does NOT move the kernel floor: that profile is
# `--no-default-features --features flows` and never reads this list.
default = ["media", "skills", "flows", "mcp", "channels", "medulla", "http-server", "scheduler-gate", "file-logging", "modules"]
# HTTP + Socket.IO server transport (#5048): the `/rpc` JSON-RPC endpoint and
# its auth middleware/CORS layer (`core::jsonrpc`, `core::auth`), the `/v1`
# OpenAI-compatible router (`inference::http`), the ad-hoc static-dir file
Expand Down
90 changes: 90 additions & 0 deletions src/openhuman/memory/api.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
//! The **tinybus module contract** for memory — the vocabulary that crosses the
//! bus between this host and the separately compiled TinyMemory module, named
//! as a re-export of the [`tinymemory_api`] crate and never as a copy of it.
//!
//! # Why this file is nine lines and not ten thousand
//!
//! This module used to be a directory holding 10,894 lines that were, file for
//! file, byte-identical to `vendor/tinymemory/api/src/` apart from the paths
//! written inside doc comments. Commit `3ee5a3cad` ("run tiny domains as
//! TinyBus modules") put them there.
//!
//! Nothing behaved differently the day that landed, which is exactly what made
//! it worth undoing. The contract is the vocabulary **three** parties speak:
//! host call sites, [`crate::openhuman::modules::memory::ModuleMemoryProvider`]
//! serialising onto the bus, and the separately compiled TinyMemory module on
//! the far end — and that module compiles against the crate. A verbatim copy
//! made the host's `MemoryError`, `Chunk`, `Capabilities` and `MemoryProvider`
//! *distinct types* from the ones on the wire, kept in step by nothing but
//! whoever remembered to edit both.
//!
//! [`wire`] is where that mattered most. Its module docs, and
//! `modules/memory.rs`, both say the error table is shared by both ends
//! precisely because reimplementing it "is what would let a `PathEscape` arrive
//! as an `Invalid`, silently reclassifying a sandbox escape as a caller
//! mistake". While the host held its own copy of that table the sentence
//! described an intention rather than the build.
//!
//! # What this module exports, and what it deliberately does not
//!
//! It is the contract surface, **not a convenience alias for the crate**. The
//! set below is derived from what actually crosses the bus, in both directions:
//! outbound from `modules/memory.rs`, where `ModuleMemoryProvider` serialises
//! each capability family onto the wire, and inbound from
//! `modules/memory_host.rs`, the host callbacks the module calls back into.
//!
//! The distinction is the point. `tinymemory-api` is also the crate this host
//! embeds the memory *engine* through, and those two roles are not the same
//! surface. Anything the host uses for its own purposes — engine config
//! sections, the driver-fallback provider, trait scaffolding — is reached by
//! naming [`tinymemory_api`] directly at the call site, so that "the module
//! contract" and "the host's own use of the crate" are told apart in the
//! source rather than in someone's memory.
//!
//! Excluded, with the reason:
//!
//! - **`host`** is re-exported as **two types, not the namespace.** It is the
//! engine-embedding seam — `MemoryConfig` and the other persisted config
//! sections, `EmbeddingProvider`, `MemoryHostConfig`, `MemoryEventSink` —
//! which the host hands to `tinymemory-core` in-process and which never
//! touch the bus. Only [`host::MemoryEvent`] and [`host::SpacyResponse`]
//! cross it, inbound, and only those two are exported here.
//! - **`null`** is `NullMemoryProvider`, the fallback bound by
//! `memory::binding` when no module driver is available. It is what runs
//! when nothing crosses the bus, which makes it the opposite of contract.
//! - **`traits`**, **`version`** (as a module path) and **`is_compatible`** had
//! no use anywhere in `src/`; they were alias surface only. Version
//! negotiation is done through [`CONTRACT_VERSION`], which `memory::binding`
//! and `memory::ops::provider` compare against the driver's reported
//! contract, so that constant stays.
//!
//! Everything else is re-exported as a whole namespace on purpose: each of
//! `capabilities`, `chunks`, `error`, `goals`, `health`, `provider` (with its
//! `provider::types` payload vocabulary), `recall`, `tool_memory`, `tree`,
//! `types` and `wire` is wire vocabulary end to end — `provider` alone is the
//! fourteen capability-family traits plus eleven payload types, and naming
//! those twenty-five individually would restate the crate's own module
//! structure without narrowing anything.
//!
//! `memory/api_identity_tests.rs` pins the survivors with type identities, so a
//! future re-inlining fails to compile instead of passing silently.
//!
//! Prefer naming [`tinymemory_api`] directly in new code. These paths exist so
//! the several hundred existing `memory::api::…` call sites did not have to
//! move in the same change that removed the duplicate.

pub use tinymemory_api::{
capabilities, chunks, error, goals, health, provider, recall, tool_memory, tree, types, wire,
CONTRACT_VERSION,
};

/// The inbound half of the seam: the only two `tinymemory_api::host` types that
/// cross the bus, rather than the whole engine-embedding namespace.
///
/// `modules/memory_host.rs` serves both — [`MemoryEvent`] is what the module
/// publishes back into the host's event bus, and [`SpacyResponse`] answers the
/// module's NLP callback. The rest of `tinymemory_api::host` is the in-process
/// engine seam and must be named on the crate.
pub mod host {
pub use tinymemory_api::host::{MemoryEvent, SpacyResponse};
}
Loading
Loading