Skip to content

refactor: split memory API surface into a memory-api crate#1261

Merged
geoffjay merged 1 commit into
mainfrom
memory-api-split
Jun 11, 2026
Merged

refactor: split memory API surface into a memory-api crate#1261
geoffjay merged 1 commit into
mainfrom
memory-api-split

Conversation

@geoffjay

Copy link
Copy Markdown
Owner

The cli, tui, and install crates depended on the full memory crate but
only used its HTTP client, domain types, and SQLite migration helpers -
never the lancedb vector store. That coupling put the entire
lancedb/datafusion/arrow graph (495 of the workspace's 573 dependencies)
in the build closure of every API consumer.

Move types, client, entity, and migration into a new memory-api crate
with only lightweight dependencies (serde, reqwest, sea-orm). The memory
service depends on it and re-exports the modules, so its public API and
internal crate:: paths are unchanged. Consumers now depend on memory-api
directly: the cli dependency closure drops from 495 to 348 crates and no
longer waits on the lance graph to compile.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

The cli, tui, and install crates depended on the full memory crate but
only used its HTTP client, domain types, and SQLite migration helpers -
never the lancedb vector store. That coupling put the entire
lancedb/datafusion/arrow graph (495 of the workspace's 573 dependencies)
in the build closure of every API consumer.

Move types, client, entity, and migration into a new memory-api crate
with only lightweight dependencies (serde, reqwest, sea-orm). The memory
service depends on it and re-exports the modules, so its public API and
internal crate:: paths are unchanged. Consumers now depend on memory-api
directly: the cli dependency closure drops from 495 to 348 crates and no
longer waits on the lance graph to compile.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@geoffjay geoffjay added the review-agent Used to invoke a review by an agent tracking this label label Jun 11, 2026
@geoffjay

Copy link
Copy Markdown
Owner Author

This change is part of the following stack:

Change managed by git-spice.

@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.17%. Comparing base (c605c26) to head (bad26e7).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
crates/memory-api/src/lib.rs 0.00% 4 Missing ⚠️
crates/install/src/migrate.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1261      +/-   ##
==========================================
+ Coverage   46.13%   46.17%   +0.04%     
==========================================
  Files         390      390              
  Lines       27293    27293              
  Branches     2501     2548      +47     
==========================================
+ Hits        12592    12603      +11     
+ Misses      14679    14668      -11     
  Partials       22       22              
Flag Coverage Δ
frontend 68.29% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@geoffjay

Copy link
Copy Markdown
Owner Author

Code Review: refactor: split memory API surface into a memory-api crate

Note: GitHub prevents approving your own PR, so posting as a comment. No blocking issues — clean, well-motivated refactor.

Stack position

Targets main directly.

What the PR does

Extracts the lightweight API surface of the memory crate (types, HTTP client, SeaORM entity, migrations) into a new memory-api crate with only lightweight dependencies. The memory service crate re-exports everything so its internal crate::types, crate::entity, and crate::migration paths — and the existing memory::* paths used by any code not yet migrated — remain valid.

Correctness

Re-export chain verified end-to-end:

storage.rs imports:

use crate::{
    entity::memory_entry as mem_entity,
    migration::Migrator,
    types::{CreateMemoryRequest, Memory, MemoryType, VisibilityLevel},
    ...
};

All three paths are satisfied by the new memory/src/lib.rs:

pub(crate) use memory_api::migration;           // crate::migration::Migrator ✓
pub use memory_api::{..., entity, ..., types};  // crate::entity, crate::types ✓

uuid removal from memory/Cargo.toml: Grepping memory/src/ confirms no remaining file imports uuid:: directly. All UUID handling was in the types/entity modules that moved to memory-api. ✓

Dependency consistency: async-trait and uuid are not workspace-managed deps (workspace [dependencies] only has sea-orm and sea-orm-migration). Using them directly in memory-api/Cargo.toml is consistent with memory/Cargo.toml's existing pattern. ✓

Backward compatibility: The memory crate's public API (memory::types, memory::client, memory::entity, memory::apply_migrations_for_path, memory::migration_status_for_path) is fully preserved via re-exports. Any code that imports from memory:: still compiles unchanged. ✓

Consumer updates are complete and consistent: cli, tui, and install all update their Cargo.toml dependency and their import paths in lock-step. No partial migrations. ✓

Design note (non-blocking)

migration is declared pub mod migration in memory-api/src/lib.rs, which makes memory_api::migration::Migrator part of the public API even though it's an implementation detail. This is unavoidable given the design goal: the memory crate needs pub(crate) use memory_api::migration to keep crate::migration::Migrator working inside storage.rs, and a cross-crate re-export requires the source item to be pub. The migration type is harmless to expose (a zero-field struct implementing MigratorTrait), so this is purely cosmetic.

If hiding it ever matters, the alternative would be duplicating the apply_migrations_for_path call-site in the memory crate rather than re-exporting the module — not worth the complexity.

What looks good

  • Motivation is clear and the dep count claim (495 → 348 for cli) is verifiable with cargo tree.
  • Only the right modules move: the lancedb-heavy store, storage, config, error stay in memory.
  • Doc examples in memory/src/lib.rs correctly use memory:: paths (not memory_api::), since they document the re-exported surface. ✓
  • uuid is removed from memory/Cargo.toml without leaving a dangling direct use — no orphaned imports.

Approving.

@geoffjay geoffjay added merge-queue Approved by reviewer, queued for merge by conductor and removed review-agent Used to invoke a review by an agent tracking this label labels Jun 11, 2026
@geoffjay geoffjay merged commit 564e1e2 into main Jun 11, 2026
12 checks passed
@geoffjay geoffjay deleted the memory-api-split branch June 11, 2026 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-queue Approved by reviewer, queued for merge by conductor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant