Skip to content

feat(memory): W2 — re-export memory value types from TinyCortex (type-unification) - #4529

Merged
senamakel merged 1 commit into
tinyhumansai:mainfrom
senamakel:tinycortex/w2-types
Jul 5, 2026
Merged

senamakel merged 1 commit into
tinyhumansai:mainfrom
senamakel:tinycortex/w2-types

Conversation

@senamakel

@senamakel senamakel commented Jul 5, 2026

Copy link
Copy Markdown
Member

Summary

  • W2 of the TinyCortex memory migration — flip the memory value types to crate re-exports, the type-unification decision from Phase 0 (spec §0.5).
  • memory/traits.rs now pub uses tinycortex::memory::{MemoryCategory, MemoryEntry, MemoryTaint, NamespaceSummary, RecallOpts} instead of defining them. One source of truth; all 30+ consumers + the ~10 impl Memory sites compile unchanged.
  • Keeps the security-critical MemoryTaint fail-closed tests as the host-side W2 seam — now pinning the contract on the crate type.

Problem

Phase 0 found the host memory::traits types and the crate's types are wire-compatible twins (identical fields, derives, serde attrs, and — for MemoryTaint — the same from_db_str fail-closed decode). Maintaining two definitions invites silent drift, most dangerously on MemoryTaint, which drives external-effect-tool gating for tainted subconscious turns. W2 collapses them onto the crate.

Solution

  • Re-export, not redefine. traits.rs deletes the five type definitions and re-exports the crate's. They are drop-in identical (verified field-by-field, incl. #[derive(… Copy … Serialize, Deserialize, Default)] and #[serde(rename_all = "snake_case")]), so every use crate::openhuman::memory::traits::{MemoryEntry, …} site is unchanged. No host impl targets these types (checked — no orphan-rule breakage).
  • Memory trait stays host-defined — deliberately. The crate's Memory trait matches the host's method-for-method except sqlite_conn(), the raw-connection escape hatch the crate omits by design. Because sqlite_conn dispatches per-backend through dyn Memory (only UnifiedMemory returns a real handle; everything else None), a blanket extension trait can't preserve it. So the host trait is kept as-is for now; the hatch (2 external callers) migrates to tinycortex::memory::chunks::with_connection in W3, after which the trait can also become a crate re-export.
  • Security seam. The MemoryTaint tests (…unknown_fails_closed, …defaults_to_internal_for_legacy_rows, snake_case db/serde, MemoryCategory Display) stay in traits.rs and now exercise the re-exported crate type — the W2 security-review item: fail-closed to ExternalSync survives the boundary.

Submission Checklist

  • Tests added/updated — the MemoryTaint/MemoryEntry/MemoryCategory seam tests now verify the crate types (fail-closed taint, legacy-row default, snake_case, Display, round-trips)
  • Diff coverage ≥ 80% — the diff is a re-export swap + retained tests; the changed lines are the pub use and the (unchanged-behaviour) tests around it
  • Coverage matrix updated — N/A: no user-facing feature change; type-identity refactor
  • All affected feature IDs listed — N/A
  • No new external network dependencies
  • Manual smoke checklist updated — N/A: no release-cut surface touched
  • Linked issue closed — N/A: tracks plan #4513

Impact

  • Runtime: none — the crate types are byte-identical to the former host types (same serde wire form, same Default, same fail-closed from_db_str). Existing persisted rows deserialize identically.
  • Security: MemoryTaint provenance is now single-sourced from the crate with its fail-closed default intact, pinned by the retained seam tests.
  • Build: cargo check --lib clean across all 30+ consumers and mock backends.

Related


AI Authored PR Metadata

Commit & Branch

  • Branch: tinycortex/w2-types

Validation Run

  • Focused tests: cargo test --lib openhuman::memory::traits::tests (taint fail-closed + type round-trips)
  • Rust check: cargo check --manifest-path Cargo.toml --lib exit 0 across all consumers

Validation Blocked

  • command: full .husky/pre-push (pnpm compile / pnpm rust:check)
  • error: pre-existing/environmental TS+rust-check failures unrelated to this Rust-only change; pushed with --no-verify per repo policy

Behavior Changes

  • None — type-identity refactor; wire form, Default, and fail-closed decode are unchanged

Parity Contract

  • Legacy behavior preserved: yes — re-exported crate types are byte-identical to the former host types; seam tests pin the MemoryTaint fail-closed contract on the crate type

Summary by CodeRabbit

  • Bug Fixes

    • Standardized memory data handling to preserve existing behavior across the app.
    • Invalid or unknown memory taint values still fail safely, maintaining consistent deserialization behavior.
  • Documentation

    • Updated guidance for database connection access to reflect the current recommended approach.

…/RecallOpts/NamespaceSummary from tinycortex

memory/traits.rs now `pub use`s the crate's memory value types instead of
defining them (type-unification decision, spec §0.5). They are drop-in identical
(fields, derives incl. Copy, serde snake_case, and MemoryTaint's fail-closed
from_db_str), so all 30+ consumers and ~10 impl-Memory sites compile unchanged;
no host impls target these types (no orphan-rule breakage).

The Memory trait stays host-defined because of the sqlite_conn() escape hatch the
crate omits by design (per-backend dispatch through dyn Memory can't be preserved
by a blanket extension). Its 2 external callers migrate to
tinycortex::memory::chunks::with_connection in W3, after which the trait can also
be re-exported.

The MemoryTaint fail-closed tests stay in traits.rs as the W2 security seam —
now pinning fail-closed-to-ExternalSync on the crate type. cargo check --lib
clean across the whole crate.

Claude-Session: https://claude.ai/code/session_01JUTPftwppzuj3TnAnLhi4a
@senamakel
senamakel requested a review from a team July 5, 2026 00:50
@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b52d1567-b61b-4992-8040-3921c331bc48

📥 Commits

Reviewing files that changed from the base of the PR and between 7fd935e and 7b54515.

📒 Files selected for processing (1)
  • src/openhuman/memory/traits.rs

📝 Walkthrough

Walkthrough

Local definitions of MemoryEntry, MemoryCategory, MemoryTaint, RecallOpts, and NamespaceSummary in traits.rs are replaced with re-exports from the tinycortex crate. The host-defined Memory trait remains unchanged. Documentation is updated to note sqlite_conn's retirement in favor of a tinycortex connection helper.

Changes

Memory Type Re-export Unification

Layer / File(s) Summary
Re-export core memory types from tinycortex
src/openhuman/memory/traits.rs
MemoryEntry, MemoryCategory, MemoryTaint, RecallOpts, and NamespaceSummary switch from local struct/enum definitions to pub use tinycortex::memory::{…} re-exports; module docs updated to describe the W2 single-source-of-truth contract.
Update escape-hatch docs and test comment
src/openhuman/memory/traits.rs
Memory::sqlite_conn documentation now states it is retired in W3 in favor of tinycortex::memory::chunks::with_connection; a test comment is revised to reference the W2 security seam for the re-exported taint type.

Estimated code review effort: 2 (Simple) | ~10 minutes

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A hop, a skip, a crate re-export,
Old structs retired, no more to sort.
tinycortex holds the truth we seek,
While traits.rs stays lean and sleek.
Escape hatch closed, W3 draws near —
This bunny thumps a review of cheer! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: re-exporting memory value types from TinyCortex for W2 type unification.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant