feat(embedding): dual-model registry (MiniLM + paraphrase) per ADR-043

[ohdearquant]

Summary

Slice 7/9 from PR #399 (v022-polish show). Stacked on slice 5 (recall-knobs).

Multi-model embedding support across the runtime + storage + memory stack. Workspace dual-embedding now reachable end-to-end.

What's changed

khive-runtime

• RuntimeConfig.additional_embedding_models: Vec<EmbeddingModel>
• Replaces single OnceCell<embedder> with HashMap<model_name, embedder>
• default_embedder_name() + embedder(name) public methods
• KHIVE_ADDITIONAL_EMBEDDING_MODELS env-var parsing
• configured_embedding_models() helper enumerates active set

khive-db

• V16 migration: add embedding_model TEXT NOT NULL DEFAULT '<default>' column to vectors table with backfill + composite index
• VectorStore.insert / search scoped by embedding_model

khive-storage

• VectorRecord carries model tag
• Vector search params include model scope

khive-pack-memory

• recall + remember accept optional embedding_model arg
• Validation: must be a registered model name

kkernel

• engine list now returns real loaded models (no longer empty Vec)
• engine migrate / drift-check still return not-implemented ([c21 follow-up] engine migrate: implement EmbedMigrationWorker + actual queueing (ADR-043 D2-D6) #380/[c20 follow-up] ADR-043 §8 startup backfill (steps 2-4) #385)

Notes

• 17 files changed, +634/-159 lines
• Tests rebaselined for V16 (failed_migration_rolls_back tests V17 now; store_ddl_then_event_migration_is_idempotent expects V16 head)
• Lattice gap status: N/A — lattice-embed 0.2.4 already exposes both MiniLM + paraphrase as 384-d local models with EmbeddingRoutingConfig primitives

Dependencies

Depends on #406 (slice 5: recall-knobs). Base branch is slice/v022/05-recall-knobs — will auto-rebase when slice 5 merges.

Test plan

• cargo build --workspace passes
• cargo test --workspace passes
• cargo clippy --workspace --all-targets -- -D warnings passes
• cargo fmt --all -- --check passes
• CI green

[ohdearquant]

Verdict: REJECT
Findings: 1 Critical, 3 High, 1 Medium

Findings

[Critical] V16 Ships a Schema Shape That ADR-043 Still Rejects

Evidence: docs/adr/ADR-043-embedding-model-migration.md:110 says each vec_<engine> table gains embedding_model_id; docs/adr/ADR-043-embedding-model-migration.md:113 defines it as a BLOB FK to _embedding_models; docs/adr/ADR-043-embedding-model-migration.md:118 requires backfilling existing rows with the active model id. The PR instead adds embedding_model TEXT NOT NULL DEFAULT 'all-minilm-l6-v2' and indexes (subject_id, embedding_model) at crates/khive-db/src/migrations.rs:936 and crates/khive-db/src/migrations.rs:937. The migration ledger also still stops at V15 in docs/adr/ADR-015-schema-migrations.md:46, while the process requires new schema migrations to claim the next ledger version in the ADR at docs/adr/ADR-015-schema-migrations.md:63.

Why this matters: This is an accepted ADR/code mismatch on a production schema migration. Either the string-tag design is the new decision, or the FK registry design is. Shipping V16 while the accepted ADR says the opposite leaves future migrations, kkernel engine state, and vector cleanup without a stable contract.

Suggested fix: Amend ADR-043 and ADR-015 in this PR to explicitly adopt the string-tag V16 design, or change V16 to implement the accepted embedding_model_id FK/backfill contract.

[High] V16 Does Not Correctly Backfill Existing Non-Default or sqlite-vec Rows

Evidence: The only regular-table backfill value is the hard-coded default at crates/khive-db/src/migrations.rs:936, so every existing regular vec_* row is tagged as all-minilm-l6-v2 regardless of the table/model it came from. Runtime configuration can select a different default model through KHIVE_EMBEDDING_MODEL at crates/khive-runtime/src/runtime.rs:182, and vector stores are opened per resolved model key/name at crates/khive-runtime/src/runtime.rs:374. For existing sqlite-vec tables, the open path does not backfill at all: missing embedding_model causes the whole table to be dropped at crates/khive-db/src/backend.rs:323 and crates/khive-db/src/backend.rs:324.

Why this matters: A migrated deployment with BGE/paraphrase vectors will either tag them as MiniLM or delete them on open. Model-scoped recall then filters on embedding_model = ? and can silently lose the vector side of recall.

Suggested fix: Backfill from the active registry row or from the model key represented by each vector table. For sqlite-vec, implement an explicit rebuild/copy/reindex plan with auditability instead of silently dropping old tables.

[High] Non-Default Memory Vectors Are Not Deleted With Their Notes

Evidence: remember.embedding_model reaches create_note_with_decay_for_embedding_model at crates/khive-pack-memory/src/handlers.rs:429, and the runtime inserts that note vector into vectors_for_model(token, model_name) at crates/khive-runtime/src/operations.rs:943 and crates/khive-runtime/src/operations.rs:945. Note deletion only deletes from self.vectors(token) when the default embedding model is configured at crates/khive-runtime/src/operations.rs:1392 and crates/khive-runtime/src/operations.rs:1403.

Why this matters: Memories embedded with embedding_model="paraphrase" leave orphaned paraphrase vectors after note deletion. Those stale rows can continue participating in vector candidate sets until later note filtering removes them, wasting recall budget and violating the model-scoped cleanup semantics this PR adds.

Suggested fix: Persist the vector model used for a note or delete across all registered model stores for that subject id. Add a regression that remembers with a non-default model, deletes the note, and proves the non-default vector row is gone.

[High] Unknown remember.embedding_model Errors After Persisting the Note

Evidence: The memory handler passes the raw optional model string through at crates/khive-pack-memory/src/handlers.rs:440. create_note_inner writes the note at crates/khive-runtime/src/operations.rs:912 and the FTS row at crates/khive-runtime/src/operations.rs:919 before model resolution happens in embed_with_model at crates/khive-runtime/src/operations.rs:943. Unknown names are rejected by resolve_embedding_model at crates/khive-runtime/src/runtime.rs:441 and crates/khive-runtime/src/runtime.rs:452.

Why this matters: A typo in remember(embedding_model=...) returns an error but still leaves a memory note and text index row behind. That is a write-after-failure bug, and it makes the requested unknown-model validation non-atomic.

Suggested fix: Resolve and validate embedding_model before any note/FTS/vector write, ideally at the start of create_note_inner or in handle_remember. Add a regression that an unknown model leaves no note, FTS row, or vector row.

[Medium] KHIVE_ADDITIONAL_EMBEDDING_MODELS Silently Drops Unknown Names

Evidence: RuntimeConfig::default reads KHIVE_ADDITIONAL_EMBEDDING_MODELS at crates/khive-runtime/src/runtime.rs:186 and delegates to parse_embedding_model_list; that parser uses filter_map at crates/khive-runtime/src/runtime.rs:542 and crates/khive-runtime/src/runtime.rs:544, so invalid names disappear without an error or warning.

Why this matters: The format is otherwise robust for comma/whitespace-separated values, but a config typo produces a runtime with fewer models than the operator requested. The later UnknownModel error is clear, but it occurs at request time rather than configuration time.

Suggested fix: Parse this env var as Result<Vec<EmbeddingModel>, RuntimeError> and fail startup or emit a prominent warning listing invalid names. Keep the current whitespace/empty-entry behavior.

Looks Right

• Local checkout matches the requested feature commit: 7402dda feat(embedding): dual-model registry (MiniLM + paraphrase) per ADR-043.
• V16 is append-only in the Rust migration array at crates/khive-db/src/migrations.rs:497, and migration tests are rebaselined to V16: rollback now uses bad V17 at crates/khive-db/src/migrations.rs:1277, and the event-DDL idempotency test expects V16 at crates/khive-db/src/migrations.rs:1586.
• The regular-table composite index exists as idx_{t}_subject_model ON {t}(subject_id, embedding_model) at crates/khive-db/src/migrations.rs:937.
• Vector search is model-scoped: the SQL subquery includes embedding_model = ?4 at crates/khive-db/src/stores/vectors.rs:419, and exact candidate scoring also includes e.embedding_model = ?3 at crates/khive-db/src/stores/vectors.rs:586.
• Backward-compatible default embedding calls still use the configured default and return Unconfigured("embedding_model") when no default exists at crates/khive-runtime/src/retrieval.rs:48 and crates/khive-runtime/src/retrieval.rs:75.
• embedder(name) returns RuntimeError::UnknownModel for unregistered names through resolve_embedding_model / map lookup at crates/khive-runtime/src/runtime.rs:464 and crates/khive-runtime/src/runtime.rs:470.
• recall.embedding_model reaches model-specific embedding and vector search through collect_recall_candidates at crates/khive-pack-memory/src/handlers.rs:307 and crates/khive-pack-memory/src/handlers.rs:314.
• kkernel engine list no longer returns only a stub/default; it queries all _embedding_models rows via khive_db::query_embedding_models at crates/kkernel/src/engine.rs:205 and crates/kkernel/src/engine.rs:209.
• Concurrent embedder loading is threadsafe: each immutable registry entry owns a tokio::sync::OnceCell at crates/khive-runtime/src/runtime.rs:210, and model construction goes through get_or_init at crates/khive-runtime/src/runtime.rs:472.
• The per-call HashMap lookup is not a material performance concern compared with embedding inference; service construction and result caching remain behind per-model OnceCell/CachedEmbeddingService.

Commands Run

• git diff --stat origin/slice/v022/05-recall-knobs..HEAD: confirmed 17 changed files, +634/-159.
• git log --oneline --decorate -5: confirmed HEAD is 7402dda on slice/v022/07-dual-embedding.
• git diff --check origin/slice/v022/05-recall-knobs..HEAD: passed.
• cargo fmt --all --manifest-path crates/Cargo.toml -- --check: passed.
• RUSTC_WRAPPER= CARGO_BUILD_RUSTC_WRAPPER= cargo test --offline --manifest-path crates/Cargo.toml -p khive-db --all-features migrations::tests:: -- --nocapture: passed, 10 migration tests.
• RUSTC_WRAPPER= CARGO_BUILD_RUSTC_WRAPPER= cargo test --offline --manifest-path crates/Cargo.toml -p khive-db --all-features vector_filter_contract -- --nocapture: passed, 3 contract tests.
• RUSTC_WRAPPER= CARGO_BUILD_RUSTC_WRAPPER= cargo test --offline --manifest-path crates/Cargo.toml -p khive-runtime --all-features -- --nocapture: passed, 262 unit + 18 integration tests; 2 ignored model-loading tests.
• RUSTC_WRAPPER= CARGO_BUILD_RUSTC_WRAPPER= cargo test --offline --manifest-path crates/Cargo.toml -p khive-pack-memory --all-features -- --nocapture: passed, 61 unit + 27 integration tests.
• RUSTC_WRAPPER= CARGO_BUILD_RUSTC_WRAPPER= cargo test --offline --manifest-path crates/Cargo.toml -p kkernel --all-features engine::tests -- --nocapture: passed, 6 engine tests.
• gh pr view 407 --json ... --repo ohdearquant/khive: failed because this sandbox cannot connect to api.github.com.

What I Did Not Check

• I did not run full cargo test --workspace or clippy; I ran targeted crate gates for the changed migration/runtime/memory/kkernel surfaces.
• I could not fetch live PR metadata from GitHub because network access is unavailable in this environment.

Re-Review Guidance

Re-review broadly after fixes. The migration/ADR decision must be settled first; then rerun model-scoped remember/recall/delete tests against both default and non-default embedding models.

Domain utility: SKIPPED — no mcp__lore tools are available in this environment, so I used the repository ADRs, local diff, and targeted Rust gates.

[ohdearquant]

Codex findings addressed

Verdict: REJECT → fixes pushed (per Ocean direction: amend ADR + fix data-safety bugs)

Critical: ADR-043 vs V16 schema mismatch

Fixed by amending ADR-043 to formalize the string-tag design that V16 ships. Commit 78f0a6e.

The ADR §1.1 (vector store column addition) now describes the actual implementation:

• embedding_model TEXT NOT NULL DEFAULT 'all-minilm-l6-v2' column on regular vec_* tables
• Composite index (subject_id, embedding_model) for scoped recall
• Rationale for TEXT vs BLOB FK (hot-path join cost, end-to-end shape consistency with kkernel/env-var/registry, _embedding_models.model_id as natural join key)

ADR-015 ledger amended to claim V16 (vector_embedding_model_tag) with a cluster-22 amendment note explaining the addition.

High 3: Atomic unknown-model validation

Fixed. handle_remember now calls resolve_embedding_model(Some(model_name)) BEFORE create_note_with_decay_for_embedding_model. The resolver is synchronous (no model load) and checks registry membership only. Unknown name → error returned before any note/FTS/vector row is written.

High 2: Scoped delete across all model stores

Fixed. New public method KhiveRuntime::registered_embedding_model_names() enumerates the embedder registry. delete_note now iterates over every registered model and calls vectors_for_model(token, name).delete(id) for each — both on hard delete and on soft delete.

Medium: KHIVE_ADDITIONAL_EMBEDDING_MODELS silent drop

Fixed. parse_embedding_model_list now emits tracing::warn for non-empty raw names that fail to parse, instead of silently filtering them out. Operator typos surface at startup. Function still returns a Vec (partial validity → functional runtime) — the warning is unambiguous.

High 1: V16 backfill / sqlite-vec rebuild — deferred

The codex finding about hard-coded 'all-minilm-l6-v2' backfill and silent sqlite-vec table drop is acknowledged in ADR-043 §1.1 final paragraph as a documented limitation with operator backup warning. Implementing a per-table model-inferred backfill + sqlite-vec INSERT-INTO-SELECT-FROM rebuild requires a careful multi-step migration with verification — tracked separately rather than added to this PR.

Operators are warned: back up before upgrading any production deployment with persisted non-default embeddings.

Verification

• cargo build --workspace: pass
• cargo test -p khive-runtime -p khive-pack-memory --lib: 262 + 62 = 324 tests pass
• cargo fmt --all --check: pass
• deno fmt --check docs/adr/ADR-015...md docs/adr/ADR-043...md: pass

Commits: 78f0a6e (fixes), daa3c6b (cargo fmt), f188955 (deno fmt).

Ready for re-review.

[ohdearquant]

Codex round 2 verdict

REQUEST CHANGES → fixes pushed in 55c1f4a

Round 2 surfaced 2 new findings (0 Critical, 1 High, 1 Medium):

High — ADR-043 internal inconsistency

Round 1's §1.1 amendment landed cleanly, but the rest of the ADR still contained normative text for the rejected FK design:

• §8 (Backward compat) described embedding_model_id BLOB migration + CHECK rebuild
• Alternatives table rejected per-record model_id (which is exactly what V16 ships)
• Migration version section claimed version=5 + BLOB FK

Fixed: §8 rewritten to describe the actual V14 + V16 split that shipped. Alternatives table row struck through with "Superseded by V16 (2026-05-25)" pointing to §1.1 for rationale. Migration version section split into V14 (cluster-20) and V16 (v022-polish) sub-sections with the actual SQL each runs.

Medium — Runtime-level atomicity gap

Round 1 fix to handle_remember was correct but the runtime layer (create_note_inner in khive-runtime/src/operations.rs) still had the write-after-failure pattern: note → FTS → embed (where unknown model errored).

Fixed: resolve_embedding_model now called at start of create_note_inner BEFORE any note/FTS/vector write. Direct Rust callers (other packs, integration tests) get atomicity at the lowest layer. The handler-level validation remains as an earlier error boundary.

Verification

• cargo test -p khive-runtime -p khive-pack-memory --lib: 262 + 62 tests pass
• cargo fmt --check: clean
• deno fmt --check on ADR-043: clean

Confirmed in round 2's "Looks Right" section: ADR-015 V16 row landed, sqlite-vec deferral documented, handle_remember validates before pack write, delete_note iterates all model stores, env var emits tracing::warn, vector search remains model-scoped, migration tests rebaselined for V16.

Ready for round 3 (or APPROVE).

The codex round 2 full review file was generated locally — see commit 55c1f4a for the fix details. Findings are summarized above.