feat(lib): re-export git2 for hosts that inspect a ledger - #147
Conversation
`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 <medulla@tinyhumans.ai>
…ip them 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 <medulla@tinyhumans.ai>
TinyCortex owns every libgit2 call in the memory stack -- the diff ledger, the wiki mirror and the persona git-history reader. Hosts had to declare their own git2 to assert on the artifacts this crate writes, which risks a second major pin; `links = "git2"` makes that a hard cargo error rather than a warning. Re-export the binding under the two git features instead. Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change keeps diff types and sources available without Git support. It gates Git-backed modules and ChangesGit diff feature carve-out
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to This adds a feature-gated public git2 re-export without changing runtime behavior or feature-off behavior; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
PR babysitter status Head: CI: all 7 required GitHub Actions checks are Non-required check Review threads: 0 unresolved. Reviews requesting changes: 0. Local validation on head
Mergeability: Status: READY_FOR_APPROVAL. Handing off to pr-approval-reviewer. |
Summary
Re-export
git2from the crate root under#[cfg(any(feature = "git-diff", feature = "wiki-git"))], so a host can read back a ledger this crate wrote without declaring its owngit2dependency.Problem
TinyCortex owns every libgit2 call in the memory stack — the diff ledger (
memory::diff), the wiki mirror (memory::store::content::wiki_git), and the persona git-history reader (memory::persona::readers::git_history). Nothing else in the stack opens a repository.Hosts still had to declare
git2themselves to assert on the artifacts this crate produces. OpenHuman did exactly that: agit2entry in[dependencies]plusdep:git2on itsmemory-gitgate, whose only consumer in the entire crate was onegit2::Repository::openin an integration test.That is not merely redundant. A host pin is free to drift off this crate's major, and
git2setslinks = "git2"— two majors in one graph is a hard cargo error, not a warning. The failure would surface as an unrelatable link conflict at build time.Solution
pub use git2;at the crate root, gated on the two features that already pull the dependency in, with a doc comment saying what it is for and what it is not: reach for it only where a caller genuinely has to inspect a ledger or mirror this crate produced — an integration test asserting on commits, tags or trees. Ordinary callers stay on the typed API.No behaviour change, no new dependency, no change to the feature-off build (the re-export compiles out with the features that carry
git2).Impact
git2declarations, leaving this crate the sole libgit2 link in the graph. The companion PRs do that intinymemory-coreandopenhuman.Validation
cargo check --all-targets(default features) — clean.cargo check --features git-diff,wiki-git,obsidian,persona,sync --all-targets— clean.cargo fmt --check— clean.git2dependency removed and its test moved ontotinycortex::git2,cargo test --features memory-git --test memory_artifacts_e2epasses (2 tests), andcargo tree -i git2shows this crate as the only parent.Summary by CodeRabbit
New Features
git-difffeature is enabled.Documentation
Tests