From 59e4e001e350224ce06122f218aa77dc9649682d Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 10 Aug 2026 13:54:56 +0300 Subject: [PATCH 1/2] feat(memory): carve the inert diff types out from behind `git-diff` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git-diff` gated the whole `memory::diff` module, so a host that did not want libgit2 in its dependency graph could not so much as *name* a `CrossSourceDiff`. That is more than the feature needs to gate: `types.rs` and `source.rs` are `serde`/`std`-only and reach no `git2` symbol — only `ledger.rs` and `ledger_helpers.rs` do. `pub mod diff` is now always compiled. Ungated: `types`, `source`, and their re-exports. Gated on `git-diff`: `ledger` + `ledger_helpers` (the two that touch git2), `checkpoint` / `diff` / `snapshot` (whose impls are written against `Ledger`), and `DiffEngine` itself — its inherent methods live in those modules, so an ungated engine would be a handle with nothing to call. The distinction is describe-vs-compute: without the feature a host can pass a diff around, match on a `ChangeKind`, and implement `SnapshotItemSource`; it simply cannot produce one. This unblocks a `memory-git` gate in OpenHuman, whose always-on subconscious profile renders `CrossSourceDiff`/`ChangeKind` into prompts. Stubbing those types host-side instead would mean two definitions of one serde shape drifting apart silently — which is why OpenHuman's own gate guidance says to put a domain's inert types in a dependency-free submodule and gate only behaviour. Two `#[cfg(not(feature = "git-diff"))]` tests pin the carve-out, because the disabled build is the only thing that can catch it regressing: re-gating these types compiles fine with the feature on and only breaks downstream. They construct and serde-round-trip the types rather than just naming them, so a gated-away derive fails too. The pre-existing `types`/`source` unit tests now run in the disabled build as well. Verified both ways: `--features obsidian,persona,sync` (43 → the git-backed tests compile out, 14 inert ones run) and with `git-diff,wiki-git` added (43 diff tests pass, unchanged). Co-authored-by: Medulla --- src/memory/diff/mod.rs | 72 +++++++++++++++++++++++++++++++++++++++++- src/memory/mod.rs | 24 ++++++++------ 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/src/memory/diff/mod.rs b/src/memory/diff/mod.rs index 04c4a37..5377084 100644 --- a/src/memory/diff/mod.rs +++ b/src/memory/diff/mod.rs @@ -40,18 +40,47 @@ //! - `DiffEngine::diff_since_checkpoint` (cross-source) //! - `DiffEngine::cleanup` +//! ## Type carve-out — what compiles WITHOUT `git-diff` +//! +//! `types` and `source` are `serde`/`std`-only: they name no git concept and +//! reach no `git2` symbol. They stay **ungated**, so a host that does not want +//! libgit2 in its dependency graph can still describe a diff — pass a +//! `CrossSourceDiff` around, match on a `ChangeKind`, implement a +//! `SnapshotItemSource` — it simply cannot *compute* one. +//! +//! That distinction is what makes the gate usable downstream. OpenHuman's +//! always-on subconscious profile renders `CrossSourceDiff` / `ChangeKind` in +//! prompts, and stubbing those types rather than sharing them would mean two +//! definitions of the same serde shape drifting apart silently. The rule +//! (OpenHuman's AGENTS.md, "Compile-time domain gates") is: put a domain's +//! inert types in a dependency-free submodule and leave it ungated; gate only +//! the behaviour. +//! +//! Gated on `git-diff`: `ledger` + `ledger_helpers` (the only two modules that +//! touch `git2` directly), `checkpoint` / `diff` / `snapshot` (whose impls are +//! all written against `Ledger`), and `DiffEngine` itself — the engine's inherent +//! methods live in those modules, so an ungated `DiffEngine` would be a handle +//! with nothing to call. + +#[cfg(feature = "git-diff")] use std::path::PathBuf; +#[cfg(feature = "git-diff")] pub mod checkpoint; // Keep the established `memory::diff::diff` path for downstream callers. +#[cfg(feature = "git-diff")] #[allow(clippy::module_inception)] pub mod diff; +#[cfg(feature = "git-diff")] pub mod ledger; +#[cfg(feature = "git-diff")] mod ledger_helpers; +#[cfg(feature = "git-diff")] pub mod snapshot; pub mod source; pub mod types; +#[cfg(feature = "git-diff")] pub use ledger::{Ledger, SnapshotMeta}; pub use source::{extract_item_id, InMemoryItemSource, SnapshotItemSource}; pub use types::{ @@ -66,11 +95,13 @@ pub use types::{ /// injected [`SnapshotItemSource`] that yields a source's already-ingested /// items. All operations are synchronous; git mutations serialise through a /// process-global lock inside the [`Ledger`]. +#[cfg(feature = "git-diff")] pub struct DiffEngine { workspace: PathBuf, items: S, } +#[cfg(feature = "git-diff")] impl DiffEngine { /// Construct an engine rooted at `workspace`, reading items from `items`. pub fn new(workspace: impl Into, items: S) -> Self { @@ -96,6 +127,45 @@ impl DiffEngine { } } -#[cfg(test)] +#[cfg(all(test, feature = "git-diff"))] #[path = "engine_tests.rs"] mod tests; + +/// Proves the type carve-out holds in the build that motivates it. +/// +/// The whole point of leaving `types`/`source` ungated is that a host without +/// libgit2 can still name a diff. Only the disabled build can catch a regression +/// here — re-gating them would compile fine everywhere else and only break +/// downstream, which is exactly the failure this test exists to make loud. +#[cfg(all(test, not(feature = "git-diff")))] +mod carve_out_tests { + use super::types::{ChangeKind, CrossSourceDiff, DiffSummary, SnapshotItem}; + use super::SnapshotItemSource; + + #[test] + fn inert_diff_types_are_available_without_the_git_diff_feature() { + // Constructed field-by-field, and round-tripped through serde, because + // these types exist to cross a boundary: a host renders them and stores + // them. Merely naming them would not catch a derive being gated away. + let diff = CrossSourceDiff { + checkpoint_id: Some("ckpt_1".into()), + computed_at_ms: 0, + summary: DiffSummary::default(), + per_source: Vec::new(), + }; + let json = serde_json::to_string(&diff).expect("CrossSourceDiff serialises"); + assert!(json.contains("ckpt_1")); + let _kind = ChangeKind::Added; + } + + #[test] + fn the_item_source_trait_can_still_be_implemented_without_git() { + struct Empty; + impl SnapshotItemSource for Empty { + fn items_for_source(&self, _source_id: &str) -> Vec { + Vec::new() + } + } + assert!(Empty.items_for_source("anything").is_empty()); + } +} diff --git a/src/memory/mod.rs b/src/memory/mod.rs index 6614394..a07a56e 100644 --- a/src/memory/mod.rs +++ b/src/memory/mod.rs @@ -14,8 +14,10 @@ //! - [`tree`]: summary-tree mechanics (append, seal, summarise, retrieve). //! - [`queue`]: async job model (extract, append, seal, flush, backfill). //! - [`retrieval`]: vector / keyword / graph / tree / hybrid search. -//! - `diff`: git-backed source snapshots, diffs, checkpoints, read markers -//! (feature `git-diff`; gates the heavy native `git2`/libgit2 dependency). +//! - `diff`: git-backed source snapshots, diffs, checkpoints, read markers. +//! Its inert `types`/`source` submodules are always compiled; computing a +//! diff needs feature `git-diff`, which gates the heavy native +//! `git2`/libgit2 dependency. //! - [`entities`] / [`graph`]: entity files and derived co-occurrence graph. //! - [`goals`] / [`tool_memory`]: specialized long-term memory surfaces. //! - [`conversations`] / [`archivist`]: transcript storage and tree archival. @@ -41,10 +43,11 @@ //! any unrecognised persisted value decodes as //! [`types::MemoryTaint::ExternalSync`] — the more restrictive setting — so //! policy gates never under-trust content of unknown provenance. -//! - **Feature-gated modules add no default-build cost.** `diff`, `providers`, -//! and `persona` are compiled out entirely unless their feature is enabled (see -//! the crate-level feature-flag docs in `lib.rs`); code in this module must -//! not assume they are present. +//! - **Feature-gated modules add no default-build cost.** `providers` and +//! `persona` are compiled out entirely unless their feature is enabled, and +//! `diff` keeps only its inert `types`/`source` submodules without `git-diff` +//! (see the crate-level feature-flag docs in `lib.rs`). Code in this module +//! must not assume any of them is present. // ── Shared contracts ──────────────────────────────────────────────────────── pub mod config; @@ -64,9 +67,12 @@ pub mod chunks; pub mod conversations; /// Git-backed source snapshots, diffs, checkpoints, and read markers. /// -/// Gated behind the `git-diff` feature: the entire module (and the heavy native -/// `git2`/libgit2 dependency it needs) compiles out when the feature is off. -#[cfg(feature = "git-diff")] +/// Always compiled, but mostly hollow without the `git-diff` feature: the inert +/// `types` and `source` submodules are `serde`/`std`-only and stay available so +/// a host can still *describe* a diff, while everything that computes one — +/// `Ledger`, `DiffEngine`, and the checkpoint/snapshot/diff operations — is +/// gated along with the heavy native `git2`/libgit2 dependency it needs. See +/// the module's own docs for why the split falls where it does. pub mod diff; pub mod entities; /// Shared filesystem primitives (crash-safe atomic writes). From be7b395354271082953d2594765aded73975b54c Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Mon, 10 Aug 2026 15:26:16 +0300 Subject: [PATCH 2/2] test(memory): move the carve-out tests to a sibling file and round-trip them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on #141: - The tests were an inline `mod` in `mod.rs`; every other test module in this directory is a `#[path = "*_tests.rs"]` sibling. Now they match. - The serde test only serialised. These types exist to cross a boundary, so a `Deserialize` derive that got gated away would not have failed it — it now round-trips and asserts the restored fields. Co-authored-by: Medulla --- src/memory/diff/carve_out_tests.rs | 38 ++++++++++++++++++++++++++++ src/memory/diff/mod.rs | 40 +++++------------------------- 2 files changed, 44 insertions(+), 34 deletions(-) create mode 100644 src/memory/diff/carve_out_tests.rs diff --git a/src/memory/diff/carve_out_tests.rs b/src/memory/diff/carve_out_tests.rs new file mode 100644 index 0000000..dccc2a2 --- /dev/null +++ b/src/memory/diff/carve_out_tests.rs @@ -0,0 +1,38 @@ +//! The `git-diff`-disabled half of the diff module's contract. +//! +//! Sibling test file rather than an inline `mod`, matching this directory's +//! convention (`ledger_tests.rs`, `source_tests.rs`, `types_tests.rs`, …). + +use super::types::{ChangeKind, CrossSourceDiff, DiffSummary, SnapshotItem}; +use super::SnapshotItemSource; + +#[test] +fn inert_diff_types_are_available_without_the_git_diff_feature() { + // Round-tripped, not just serialised: these types exist to cross a + // boundary, so a `Deserialize` derive that got gated away has to fail here + // too. Serialising alone would only exercise half the pair. + let diff = CrossSourceDiff { + checkpoint_id: Some("ckpt_1".into()), + computed_at_ms: 0, + summary: DiffSummary::default(), + per_source: Vec::new(), + }; + let json = serde_json::to_string(&diff).expect("CrossSourceDiff serialises"); + let restored: CrossSourceDiff = + serde_json::from_str(&json).expect("CrossSourceDiff deserialises"); + assert_eq!(restored.checkpoint_id.as_deref(), Some("ckpt_1")); + assert_eq!(restored.computed_at_ms, 0); + assert!(restored.per_source.is_empty()); + let _kind = ChangeKind::Added; +} + +#[test] +fn the_item_source_trait_can_still_be_implemented_without_git() { + struct Empty; + impl SnapshotItemSource for Empty { + fn items_for_source(&self, _source_id: &str) -> Vec { + Vec::new() + } + } + assert!(Empty.items_for_source("anything").is_empty()); +} diff --git a/src/memory/diff/mod.rs b/src/memory/diff/mod.rs index 5377084..4736239 100644 --- a/src/memory/diff/mod.rs +++ b/src/memory/diff/mod.rs @@ -134,38 +134,10 @@ mod tests; /// Proves the type carve-out holds in the build that motivates it. /// /// The whole point of leaving `types`/`source` ungated is that a host without -/// libgit2 can still name a diff. Only the disabled build can catch a regression -/// here — re-gating them would compile fine everywhere else and only break -/// downstream, which is exactly the failure this test exists to make loud. +/// libgit2 can still name a diff. Only the disabled build can catch a +/// regression here — re-gating them would compile fine everywhere else and +/// only break downstream, which is exactly the failure this exists to make +/// loud. #[cfg(all(test, not(feature = "git-diff")))] -mod carve_out_tests { - use super::types::{ChangeKind, CrossSourceDiff, DiffSummary, SnapshotItem}; - use super::SnapshotItemSource; - - #[test] - fn inert_diff_types_are_available_without_the_git_diff_feature() { - // Constructed field-by-field, and round-tripped through serde, because - // these types exist to cross a boundary: a host renders them and stores - // them. Merely naming them would not catch a derive being gated away. - let diff = CrossSourceDiff { - checkpoint_id: Some("ckpt_1".into()), - computed_at_ms: 0, - summary: DiffSummary::default(), - per_source: Vec::new(), - }; - let json = serde_json::to_string(&diff).expect("CrossSourceDiff serialises"); - assert!(json.contains("ckpt_1")); - let _kind = ChangeKind::Added; - } - - #[test] - fn the_item_source_trait_can_still_be_implemented_without_git() { - struct Empty; - impl SnapshotItemSource for Empty { - fn items_for_source(&self, _source_id: &str) -> Vec { - Vec::new() - } - } - assert!(Empty.items_for_source("anything").is_empty()); - } -} +#[path = "carve_out_tests.rs"] +mod carve_out_tests;