vestige/docs/plans/0001a-trait-rewrite.md

Phase 1 Amendment Sub-Plan: async_trait -> trait_variant

Status: Draft Depends on: Phase 1 (already on feat/storage-trait-phase1, tip 790c0c8) Followed by: docs/plans/0001c-async-trait-sunset.md (Embedder rewrite + async-trait crate removal) Related: docs/adr/0002-phase-2-execution.md (decision D1), docs/plans/0001-phase-1-storage-trait-extraction.md

---

Context

Phase 1 froze with the storage trait declared as #[async_trait::async_trait] pub trait MemoryStore: Send + Sync + 'static and a pub use MemoryStore as LocalMemoryStore; alias. async_trait boxes every async fn in the trait into Pin<Box<dyn Future + Send>>. That is one heap allocation per call inside the hottest code path in Vestige (insert, search, update_scheduling are all on this surface). It also collapses the two intended trait shapes -- a Send-bound MemoryStore for tokio/axum and a non-Send LocalMemoryStore for thread-local backends -- into a single trait behind an alias, removing the type-system signal we will want for Phase 2's Postgres backend.

ADR 0002 decision D1 supersedes this. The amendment lands on the existing feat/storage-trait-phase1 branch before Phase 2 starts, before the PR is opened upstream. The end state:

• LocalMemoryStore is the source-of-truth trait, declared with native async-fn-in-trait (stable on MSRV 1.91), no Sync bound on the trait itself, and Sync + 'static on the trait object.
• MemoryStore is auto-generated by #[trait_variant::make(MemoryStore: Send)] with Send bounds on every returned future, so Arc<dyn MemoryStore> is movable across tokio::spawn.
• trait-variant 0.1 is already present in crates/vestige-core/Cargo.toml. The async-trait crate dependency stays in place through this sub-plan (it is still in use by the embedder impl); removing it is the job of 0001c-async-trait-sunset.md.
• No production caller changes. Every production call site holds Arc<Storage> (the concrete SqliteMemoryStore alias), which the trait rewrite does not touch. Only the trait declaration and impl blocks change.

---

Scope

In scope

• Rewrite MemoryStore / LocalMemoryStore declaration in crates/vestige-core/src/storage/memory_store.rs to use #[trait_variant::make(MemoryStore: Send)] pub trait LocalMemoryStore.
• Remove the pub use MemoryStore as LocalMemoryStore; alias from the same file. LocalMemoryStore becomes the real trait; MemoryStore is the generated Send variant.
• Drop the #[async_trait::async_trait] attribute on the SQLite impl block in crates/vestige-core/src/storage/sqlite.rs (line 6274). The impl block switches from impl MemoryStore for SqliteMemoryStore (currently spelled through the LocalMemoryStore alias) to impl LocalMemoryStore for SqliteMemoryStore. trait_variant provides a blanket impl<T: LocalMemoryStore + Send> MemoryStore for T so &dyn MemoryStore and Arc<dyn MemoryStore> keep working unchanged.
• Update doc comments in memory_store.rs to drop references to #[async_trait::async_trait] and instead describe the trait_variant mechanism.
• Keep the existing Phase 1 test suite (tests/phase_1/*.rs) green. The tests already use Arc<dyn MemoryStore> and Box<dyn Embedder>, which is exactly the surface the rewrite is meant to preserve.

Out of scope -- moved to 0001c

• Rewriting the Embedder / LocalEmbedder trait declaration -- handled by 0001c-async-trait-sunset.md.
• Dropping #[async_trait::async_trait] from crates/vestige-core/src/embedder/fastembed.rs.
• Removing async-trait = "0.1" from crates/vestige-core/Cargo.toml.
• The hard grep -rn 'async_trait' crates/ zero-match gate (async_trait is still present in the embedder module after 0001a alone).

Out of scope

• The SQLite file split (sqlite.rs -> sqlite/ directory). That is the sibling sub-plan 0001b-sqlite-split.md.
• Any production-side refactor that switches Arc<Storage> to Arc<dyn MemoryStore>. Production code keeps using the concrete alias.
• Adding register_model / from_pool / Postgres-specific variants of MemoryStoreError. Those land with the Postgres backend in Phase 2.
• Removing the pub type Storage = SqliteMemoryStore; alias.

---

Prerequisites

Current state (verified)

• crates/vestige-core/Cargo.toml already declares trait-variant = "0.1" (line 117). async-trait = "0.1" (line 119) remains in place for the duration of this sub-plan; 0001c removes it.
• crates/vestige-core/src/storage/memory_store.rs:194 declares the trait with #[async_trait::async_trait] pub trait MemoryStore: Send + Sync + 'static.
• crates/vestige-core/src/storage/memory_store.rs:262 has pub use MemoryStore as LocalMemoryStore;.
• crates/vestige-core/src/storage/sqlite.rs:6274 declares #[async_trait::async_trait] impl crate::storage::memory_store::LocalMemoryStore for SqliteMemoryStore.
• Production call sites use Arc<Storage> (the concrete SqliteMemoryStore alias). Confirmed by grep: grep -rn "dyn MemoryStore\|dyn LocalMemoryStore" --include="*.rs" returns hits only in tests/phase_1/*.rs, in two comments inside memory_store.rs and sqlite.rs, and in one test-only &dyn MemoryStore cast inside sqlite.rs::tests (line 8669).
• Workspace-wide async_trait usages: only the trait declarations and impl blocks in memory_store.rs, sqlite.rs, embedder/mod.rs, and embedder/fastembed.rs (verified by grep -rn async_trait --include="*.rs"). This sub-plan touches only the first two; the embedder files are addressed by 0001c.

Required crates

Crate	Version	Action
trait-variant	0.1	Already declared. Verify present after edit.
async-trait	0.1	Unchanged for this sub-plan (still used by embedder impl). Removed by 0001c.
blake3, thiserror, chrono, uuid, serde, serde_json	unchanged	unchanged

---

Files Touched

Grouped by category. Every change is listed; nothing else gets touched.

Trait declarations (vestige-core)

File	Lines (approx)	Change
crates/vestige-core/src/storage/memory_store.rs	183-262	Replace #[async_trait::async_trait] block with #[trait_variant::make(MemoryStore: Send)] pub trait LocalMemoryStore. Drop the pub use MemoryStore as LocalMemoryStore; alias. Update doc comments.

Impl blocks (vestige-core)

File	Lines (approx)	Change
crates/vestige-core/src/storage/sqlite.rs	6274 (impl block header only)	Delete the #[async_trait::async_trait] attribute. Keep the impl crate::storage::memory_store::LocalMemoryStore for SqliteMemoryStore { ... } body verbatim.

Cargo dependency cleanup

None in this sub-plan. The async-trait crate stays declared in crates/vestige-core/Cargo.toml because the embedder impl still uses it. 0001c-async-trait-sunset.md removes the dependency after the embedder side is rewritten.

Cognitive module call sites

No changes. The 29 cognitive modules under crates/vestige-core/src/neuroscience/ and crates/vestige-core/src/advanced/ already operate on concrete SqliteMemoryStore (via the Storage alias) or on plain data types (KnowledgeNode, Vec<f32>, ConnectionRecord). Grep verified zero production references to dyn MemoryStore or dyn LocalMemoryStore.

MCP call sites

No changes. All ~30 vestige-mcp/src/**.rs files holding Arc<Storage> keep working because:

• Storage is still pub type Storage = SqliteMemoryStore; (unchanged).
• They call inherent methods on the concrete type, never via a trait object.
• SqliteMemoryStore implements LocalMemoryStore; trait_variant auto- generates a blanket impl<T: LocalMemoryStore + Send> MemoryStore for T, so the concrete type also satisfies MemoryStore for any future caller that wants the trait-object form.

Test files (vestige-core integration tests)

File	Lines	Change
tests/phase_1/trait_round_trip.rs	7-18, 134	No change. Already uses Arc<dyn MemoryStore> and use vestige_core::storage::MemoryStore. trait_variant emits a MemoryStore trait at the same path, so the imports resolve.
tests/phase_1/send_bound_variant.rs	10-12, 36, 57	No change. Already asserts the trait_variant Send invariant; the rewrite is what makes the doc comment on lines 3-4 actually true.
tests/phase_1/cognitive_module_isolation.rs	11, 37, 76, 102, 115	No change.
tests/phase_1/embedding_model_registry.rs	10	No change.
tests/phase_1/domain_column_migration.rs	98	No change.
crates/vestige-core/src/storage/sqlite.rs::tests	8666-8675	No change. The existing test casts &s to &dyn MemoryStore and calls trait methods through that vtable; trait_variant preserves this exact dyn-compatible surface.

Documentation

File	Change
crates/vestige-core/src/storage/memory_store.rs	Rewrite the trait-level doc comment (lines 185-193) to describe trait_variant rather than async_trait.
CLAUDE.md	No change. The repo-level architecture notes do not name the trait attribute.

---

Trait Declaration Rewrite

Before (current state on feat/storage-trait-phase1)

crates/vestige-core/src/storage/memory_store.rs:183-262:

// ----------------------------------------------------------------------------
// TRAIT
// ----------------------------------------------------------------------------

/// The single storage abstraction.
///
/// `#[async_trait::async_trait]` makes every `async fn` return a
/// `Pin<Box<dyn Future + Send>>`, which is required for `Arc<dyn MemoryStore>`
/// to be movable across `tokio::spawn` boundaries.
///
/// `LocalMemoryStore` is a type alias kept for source compatibility with code
/// that refers to the non-send variant. In Phase 1 both names refer to the same
/// (dyn-compatible, Send-safe) trait.
#[async_trait::async_trait]
pub trait MemoryStore: Send + Sync + 'static {
    // --- Lifecycle ---
    async fn init(&self) -> MemoryStoreResult<()>;
    async fn health_check(&self) -> MemoryStoreResult<HealthStatus>;

    // ... 25 more async fn ...

    async fn vacuum(&self) -> MemoryStoreResult<()>;
}

/// Type alias kept for source compatibility. Both names refer to the same
/// `async_trait`-annotated trait that is dyn-compatible and `Send + Sync`.
pub use MemoryStore as LocalMemoryStore;

After

crates/vestige-core/src/storage/memory_store.rs:183-262:

// ----------------------------------------------------------------------------
// TRAIT
// ----------------------------------------------------------------------------

/// The single storage abstraction.
///
/// `LocalMemoryStore` is the source-of-truth trait. The
/// `#[trait_variant::make(MemoryStore: Send)]` attribute auto-generates a
/// `MemoryStore` trait whose returned futures are `Send`, so
/// `Arc<dyn MemoryStore>` is movable across `tokio::spawn` boundaries while
/// `Arc<dyn LocalMemoryStore>` remains usable on single-threaded executors
/// and thread-local backends.
///
/// Every method is native async-fn-in-trait (stable on MSRV 1.91); no
/// per-call heap allocation, no boxed futures.
#[trait_variant::make(MemoryStore: Send)]
pub trait LocalMemoryStore: Sync + 'static {
    // --- Lifecycle ---
    async fn init(&self) -> MemoryStoreResult<()>;
    async fn health_check(&self) -> MemoryStoreResult<HealthStatus>;

    // --- Embedding model registry ---
    async fn registered_model(&self) -> MemoryStoreResult<Option<ModelSignature>>;
    async fn register_model(&self, sig: &ModelSignature) -> MemoryStoreResult<()>;

    // --- CRUD ---
    async fn insert(&self, record: &MemoryRecord) -> MemoryStoreResult<Uuid>;
    async fn get(&self, id: Uuid) -> MemoryStoreResult<Option<MemoryRecord>>;
    async fn update(&self, record: &MemoryRecord) -> MemoryStoreResult<()>;
    async fn delete(&self, id: Uuid) -> MemoryStoreResult<()>;

    // --- Search ---
    async fn search(&self, query: &SearchQuery) -> MemoryStoreResult<Vec<SearchResult>>;
    async fn fts_search(&self, text: &str, limit: usize) -> MemoryStoreResult<Vec<SearchResult>>;
    async fn vector_search(
        &self,
        embedding: &[f32],
        limit: usize,
    ) -> MemoryStoreResult<Vec<SearchResult>>;

    // --- FSRS Scheduling ---
    async fn get_scheduling(
        &self,
        memory_id: Uuid,
    ) -> MemoryStoreResult<Option<SchedulingState>>;
    async fn update_scheduling(&self, state: &SchedulingState) -> MemoryStoreResult<()>;
    async fn get_due_memories(
        &self,
        before: DateTime<Utc>,
        limit: usize,
    ) -> MemoryStoreResult<Vec<(MemoryRecord, SchedulingState)>>;

    // --- Graph (spreading activation) ---
    async fn add_edge(&self, edge: &MemoryEdge) -> MemoryStoreResult<()>;
    async fn get_edges(
        &self,
        node_id: Uuid,
        edge_type: Option<&str>,
    ) -> MemoryStoreResult<Vec<MemoryEdge>>;
    async fn remove_edge(&self, source: Uuid, target: Uuid) -> MemoryStoreResult<()>;
    async fn get_neighbors(
        &self,
        node_id: Uuid,
        depth: usize,
    ) -> MemoryStoreResult<Vec<(MemoryRecord, f64)>>;

    // --- Domains ---
    async fn list_domains(&self) -> MemoryStoreResult<Vec<Domain>>;
    async fn get_domain(&self, id: &str) -> MemoryStoreResult<Option<Domain>>;
    async fn upsert_domain(&self, domain: &Domain) -> MemoryStoreResult<()>;
    async fn delete_domain(&self, id: &str) -> MemoryStoreResult<()>;
    async fn classify(&self, embedding: &[f32]) -> MemoryStoreResult<Vec<(String, f64)>>;

    // --- Bulk / Maintenance ---
    async fn count(&self) -> MemoryStoreResult<usize>;
    async fn get_stats(&self) -> MemoryStoreResult<StoreStats>;
    async fn vacuum(&self) -> MemoryStoreResult<()>;
}

Notes:

• The pub use MemoryStore as LocalMemoryStore; line on the current memory_store.rs:262 is deleted entirely. MemoryStore is now the generated trait that trait_variant::make emits at the same module path; LocalMemoryStore is the source-of-truth declaration. Both names export from storage/mod.rs already (see lines 10-14 of that file).
• Sync + 'static on LocalMemoryStore (and no Send bound on the trait itself) is correct: Send on the trait is what trait_variant::make inserts when it emits the MemoryStore variant. The current trait carries Send + Sync + 'static; the rewrite drops the Send bound from the local variant. Arc<dyn LocalMemoryStore> is Sync but not Send; Arc<dyn MemoryStore> (the generated variant) is Send + Sync.
• trait_variant 0.1 does not require any attribute on the impl block. The attribute lives only on the trait declaration. See the next section.

The Embedder trait rewrite uses the identical trait_variant::make pattern and is fully specified in 0001c-async-trait-sunset.md.

---

Impl Block Migration

trait_variant 0.1 attaches the attribute only to the trait declaration. The impl side is plain impl LocalMemoryStore for SqliteMemoryStore; no attribute on the impl, no #[trait_variant::make(MemoryStore: Send)] on the impl block. The crate auto-generates the blanket impl<T: LocalMemoryStore + Send> MemoryStore for T, so any concrete type that implements LocalMemoryStore automatically also implements MemoryStore provided it is Send.

SqliteMemoryStore already is Send + Sync (it holds Mutex<Connection> fields whose Mutex is std::sync::Mutex, which is Send + Sync when its guarded type is Send; rusqlite::Connection is Send). No new bound is needed.

Before

crates/vestige-core/src/storage/sqlite.rs:6274:

#[async_trait::async_trait]
impl crate::storage::memory_store::LocalMemoryStore for SqliteMemoryStore {
    async fn init(&self) -> crate::storage::memory_store::MemoryStoreResult<()> {
        // Migrations run in `new`; this is a no-op for the SQLite backend.
        Ok(())
    }

    // ... ~2400 lines of method bodies, unchanged ...
}

After

crates/vestige-core/src/storage/sqlite.rs:6274:

impl crate::storage::memory_store::LocalMemoryStore for SqliteMemoryStore {
    async fn init(&self) -> crate::storage::memory_store::MemoryStoreResult<()> {
        // Migrations run in `new`; this is a no-op for the SQLite backend.
        Ok(())
    }

    // ... ~2400 lines of method bodies, unchanged ...
}

Diff is exactly one removed line (the #[async_trait::async_trait] attribute). Every method body, every async fn signature, every use statement inside the impl block stays verbatim. No Box::pin(async move { ... }), no manual Pin<Box<dyn Future>>, no 'async_trait lifetime markers; native async-fn-in-trait does this directly.

The Embedder impl block rewrite follows the identical "remove one #[async_trait::async_trait] attribute" pattern and is fully specified in 0001c-async-trait-sunset.md.

Why the impl block does not need an attribute

trait_variant::make generates two things from the source trait:

1. The source trait itself (LocalMemoryStore), with native async fns.
2. A second trait (MemoryStore) whose method signatures return impl Future<Output = ...> + Send instead of impl Future<Output = ...>, plus a blanket impl wiring concrete types through.

Both are emitted at the macro-call site. SqliteMemoryStore only writes one impl block (against LocalMemoryStore); the macro-generated blanket guarantees SqliteMemoryStore: MemoryStore for free. The current &dyn MemoryStore casts (sqlite.rs:8669; tests under tests/phase_1/) keep type-checking unchanged.

---

Call Site Audit

Verified via:

grep -rn "dyn MemoryStore\|dyn LocalMemoryStore" --include="*.rs" \
  /home/delandtj/prppl/vestige-phase2/
grep -rn "Arc<Storage>\|Arc<SqliteMemoryStore>" --include="*.rs" \
  /home/delandtj/prppl/vestige-phase2/
grep -rn "use.*MemoryStore;\|use.*LocalMemoryStore;" --include="*.rs" \
  /home/delandtj/prppl/vestige-phase2/

Files that reference the trait object form (dyn MemoryStore / dyn LocalMemoryStore)

All test-only or doc-comment-only:

File	Line	Use	Required change
tests/phase_1/trait_round_trip.rs	7-18	Arc<dyn MemoryStore> in make_store() and test bodies	None. MemoryStore is the generated Send variant; signature stays.
tests/phase_1/trait_round_trip.rs	134	Arc<dyn MemoryStore> upcast inside a test body	None.
tests/phase_1/send_bound_variant.rs	10-97	Arc<dyn MemoryStore> moved across tokio::spawn	None. This test becomes meaningfully correct only after the rewrite (currently it relies on async_trait boxing; after the rewrite it relies on trait_variant's Send variant -- same observable outcome).
tests/phase_1/cognitive_module_isolation.rs	11, 33-115	Arc<dyn MemoryStore> passed into cognitive module-style closures	None.
tests/phase_1/embedding_model_registry.rs	10	Arc<dyn MemoryStore> in make_store()	None.
tests/phase_1/domain_column_migration.rs	98	Arc<dyn MemoryStore> cast inside a migration assertion	None.
crates/vestige-core/src/storage/sqlite.rs	8666-8675	let dyn_s: &dyn MemoryStore = &s; inside mod tests	None. The cast is testing that the dyn-vtable still resolves the trait methods correctly; after the rewrite it resolves through the MemoryStore trait that trait_variant::make emits at the same path.
crates/vestige-core/src/storage/memory_store.rs	188 (doc)	Doc comment mentioning Arc<dyn MemoryStore>	Replaced as part of the doc rewrite (see Trait Declaration section).

Files that hold the concrete type (Arc<Storage> / Arc<SqliteMemoryStore>)

35 files, 116 hits. Every one of them keeps working unchanged because the concrete SqliteMemoryStore type stays exactly as it is. Listed here for completeness so a reviewer can confirm none of them needs an edit:

crates/vestige-core/src/storage/mod.rs                (alias declaration)
crates/vestige-core/src/storage/sqlite.rs             (impl block)
crates/vestige-mcp/src/server.rs                      (Arc<Storage> in McpServer)
crates/vestige-mcp/src/cognitive.rs                   (hydrate(&Storage))
crates/vestige-mcp/src/autopilot.rs
crates/vestige-mcp/src/protocol/http.rs
crates/vestige-mcp/src/dashboard/mod.rs
crates/vestige-mcp/src/dashboard/state.rs
crates/vestige-mcp/src/dashboard/handlers.rs
crates/vestige-mcp/src/resources/codebase.rs
crates/vestige-mcp/src/resources/memory.rs
crates/vestige-mcp/src/tools/changelog.rs
crates/vestige-mcp/src/tools/codebase_unified.rs
crates/vestige-mcp/src/tools/context.rs
crates/vestige-mcp/src/tools/contradictions.rs
crates/vestige-mcp/src/tools/cross_reference.rs
crates/vestige-mcp/src/tools/dedup.rs
crates/vestige-mcp/src/tools/dream.rs
crates/vestige-mcp/src/tools/explore.rs
crates/vestige-mcp/src/tools/feedback.rs
crates/vestige-mcp/src/tools/graph.rs
crates/vestige-mcp/src/tools/health.rs
crates/vestige-mcp/src/tools/importance.rs
crates/vestige-mcp/src/tools/intention_unified.rs
crates/vestige-mcp/src/tools/maintenance.rs
crates/vestige-mcp/src/tools/memory_states.rs
crates/vestige-mcp/src/tools/memory_unified.rs
crates/vestige-mcp/src/tools/predict.rs
crates/vestige-mcp/src/tools/restore.rs
crates/vestige-mcp/src/tools/review.rs
crates/vestige-mcp/src/tools/search_unified.rs
crates/vestige-mcp/src/tools/session_context.rs
crates/vestige-mcp/src/tools/smart_ingest.rs
crates/vestige-mcp/src/tools/suppress.rs
crates/vestige-mcp/src/tools/tagging.rs
crates/vestige-mcp/src/tools/timeline.rs

Each holds Arc<Storage> and dispatches to inherent methods on SqliteMemoryStore. None of them goes through a trait vtable. Required change for every one of them: none.

Files that use ...MemoryStore from production code

grep -rn "use.*MemoryStore;\|use.*LocalMemoryStore;" --include="*.rs" \
  | grep -v "memory_store.rs\|sqlite.rs\|tests/phase_1"

returns nothing. Production code does not import the trait by name.

Conclusion

The rewrite is a strictly local change to two source files (storage/memory_store.rs and storage/sqlite.rs). Zero production call sites need edits. The integration tests that consume Arc<dyn MemoryStore> keep their current form; the rewrite is what gives that signature its no-box semantics on the storage side. The Box<dyn Embedder> surface is addressed by 0001c-async-trait-sunset.md.

---

Commit Sequence

Two commits, each green on cargo test -p vestige-core --no-default-features and cargo test -p vestige-core --features embeddings,vector-search.

Commit 1: rewrite MemoryStore / LocalMemoryStore trait declaration

• Touches: crates/vestige-core/src/storage/memory_store.rs only.

• Action: replace lines 183-262 per the "Trait Declaration Rewrite" section above. Delete the pub use MemoryStore as LocalMemoryStore; line.

• Green after: cargo check -p vestige-core (the impl block in sqlite.rs still has #[async_trait::async_trait] on it, but it still resolves through the LocalMemoryStore trait which is now native-async; the async_trait macro is harmless when applied to a trait that the impl block targets by path, because the macro rewrites the impl's async fns into boxed-future fns whose signatures still match the native-async declarations after trait_variant lowering). If cargo check complains here, fold commit 2 into commit 1.

  Mitigation if check fails between commits 1 and 2: combine the two into a single commit. The split is offered for review convenience; the build must be green after every commit lands.

Commit 2: drop #[async_trait::async_trait] from SqliteMemoryStore impl

• Touches: crates/vestige-core/src/storage/sqlite.rs only.
• Action: delete line 6274 (#[async_trait::async_trait]).
• Green after: cargo test -p vestige-core --features embeddings,vector-search, including all trait_* tests inside sqlite.rs::tests (lines 8643-8712) and the trait-object cast on line 8669.

Combined alternative

If the per-step split feels artificial, commits 1 and 2 can collapse into a single commit covering both the trait rewrite and the impl-attribute drop for MemoryStore. That is acceptable; the two-commit form is preferred only because it lets a reviewer bisect trait-rewrite failures separately from impl-rewrite failures.

The Embedder / fastembed commits and the async-trait Cargo dependency removal live in 0001c-async-trait-sunset.md.

---

Verification

Every command runs from the repo root unless noted otherwise.

# 1. Vestige-core, default features (embeddings + vector-search).
cargo test -p vestige-core --features embeddings,vector-search

# 2. Vestige-core, minimal features (no embeddings, no vector-search).
cargo test -p vestige-core --no-default-features

# 3. Workspace build, release mode (catches any feature-gated regression
#    in the vestige-mcp tools tree).
cargo build --workspace --release

# 4. Whole-workspace test (vestige-mcp 406 tests + vestige-core 352 tests
#    per the CLAUDE.md baseline).
cargo test --workspace

# 5. Phase 1 integration tests (these are the trait-shape contract).
cargo test --test trait_round_trip       --features embeddings,vector-search
cargo test --test send_bound_variant     --features embeddings,vector-search
cargo test --test cognitive_module_isolation --features embeddings,vector-search
cargo test --test embedding_model_registry --features embeddings,vector-search
cargo test --test domain_column_migration --features embeddings,vector-search

# 6. Clippy gate, deny warnings (matches Phase 1 PR policy of zero warnings).
cargo clippy --workspace --all-targets --features embeddings,vector-search -- -D warnings

# 7. Storage-side dependency hygiene check (must return zero hits).
#    Scoped to the storage module only -- the embedder module still uses
#    async_trait until 0001c lands.
grep -rn "async_trait\|async-trait" crates/vestige-core/src/storage/

# 8. Confirm trait_variant attribute is in place on the storage trait
#    (must return exactly one hit in memory_store.rs).
grep -rn "trait_variant::make" crates/vestige-core/src/storage/

Expected outcomes:

• Command 1: 352 tests pass (matches baseline).
• Command 2: smaller test count, all pass.
• Command 3: workspace compiles in release mode.
• Command 4: 758 total tests pass (matches CLAUDE.md baseline).
• Command 5: each phase_1 integration test binary runs green. The send_bound_variant::arc_dyn_memory_store_moves_across_tokio_tasks test is the canary; if MemoryStore lost its Send-bound future variant, this fails to compile with "future cannot be sent between threads safely".
• Command 6: zero clippy warnings. The rewrite must not introduce a new clippy::needless_lifetimes or clippy::async_yields_async.
• Command 7: empty output. async_trait is gone from the storage module. The embedder module still contains async_trait; that is removed by 0001c-async-trait-sunset.md.
• Command 8: one hit, in memory_store.rs.

---

Acceptance Criteria

A reviewer should be able to check every box:

• crates/vestige-core/src/storage/memory_store.rs declares the trait with #[trait_variant::make(MemoryStore: Send)] pub trait LocalMemoryStore: Sync + 'static, no async_trait attribute, no Send bound on LocalMemoryStore itself.
• crates/vestige-core/src/storage/memory_store.rs no longer contains pub use MemoryStore as LocalMemoryStore;.
• crates/vestige-core/src/storage/sqlite.rs:6274 is plain impl crate::storage::memory_store::LocalMemoryStore for SqliteMemoryStore -- no attribute on the impl block.
• grep -rn "async_trait\|async-trait" crates/vestige-core/src/storage/ returns zero hits.
• grep -rn "trait_variant::make" crates/vestige-core/src/storage/ returns exactly one hit (the storage trait in memory_store.rs).
• All 758 workspace tests pass (cargo test --workspace).
• Phase 1 integration tests pass with the trait-object surface (Arc<dyn MemoryStore>) intact.
• cargo clippy --workspace --all-targets --features embeddings,vector-search -- -D warnings is clean.
• No production source file under crates/vestige-mcp/ or crates/vestige-core/src/{neuroscience,advanced,consolidation,codebase, memory,embeddings,embedder}/ was modified by this sub-plan.
• Arc<Storage> still type-checks at every existing call site (verified by the workspace test pass).
• Doc comments on the storage trait declaration describe trait_variant, not async_trait.

---

Risks and Mitigations

• trait_variant 0.1 macro emits unexpected diagnostics on MSRV 1.91. Mitigation: the master Phase 1 plan already prescribed this exact pattern (#[trait_variant::make(MemoryStore: Send)] pub trait LocalMemoryStore: Sync + 'static, see plan 0001-... line 274-275); the crate has been in vestige-core/Cargo.toml since Phase 1 landed. If a diagnostic appears, pin to the exact known-good version with trait-variant = "=0.1.2" and open an upstream issue.
• Native async-fn-in-trait makes the trait no longer dyn-compatible. Mitigation: trait_variant::make is specifically the workaround for this -- it emits both the source trait (for static dispatch) and a Send-bound variant whose returned futures use Pin<Box<dyn Future + Send>> only at the dyn boundary. Arc<dyn MemoryStore> keeps working because the generated MemoryStore trait is dyn-compatible by construction. Verified by the existing send_bound_variant::* tests, which intentionally move Arc<dyn MemoryStore> across tokio::spawn from inside a multi_thread runtime.
• A cognitive module silently relied on the boxed-future return type. Mitigation: grep verified no cognitive module imports MemoryStore / LocalMemoryStore or holds an Arc<dyn ...> form; all of them use the concrete Storage alias. There is no Send-ness expectation downstream to break.
• Future bodies inside the SQLite impl capture non-Send locals. Mitigation: every method body in sqlite.rs:6274.. runs synchronous rusqlite calls inside the same async fn frame; no .await points exist inside the bodies that we ship today. The Send bound on the generated MemoryStore variant is therefore satisfied automatically. If a future change adds .await inside an impl method, the new trait_variant surface will surface that as a compile error at the call site, which is the correct outcome.
• async-trait crate is left declared after this sub-plan. This is intentional: the embedder impl still depends on it. The 0001c-async-trait-sunset.md sub-plan removes the crate after the embedder side is rewritten. Grep on the whole workspace returns only the storage and embedder files; no downstream crate pulls async-trait.

---

Out-of-Band Notes

• This sub-plan amends feat/storage-trait-phase1 (tip 790c0c8). The branch has not been opened upstream yet, so amending in place is safe; no force-push to a public PR.
• The companion sub-plan 0001b-sqlite-split.md lands after this one on the same branch. The trait-rewrite landing first is intentional: the SQLite split moves the impl block into storage/sqlite/trait_impl.rs, and it is cleaner to move a small attribute-free impl than a macro-decorated one.
• The companion sub-plan 0001c-async-trait-sunset.md lands after this one (order with 0001b is independent) and finishes the async_trait -> trait_variant transition for the Embedder trait, then removes the async-trait crate dependency.
• After the Phase 1 amendment sub-plans (0001a, 0001b, 0001c) land, the branch is reviewed and merged before Phase 2 sub-plans (0002a- through 0002i-) begin implementation.