Skip to content

feat: embedding providers, code quality fixes, and comprehensive test coverage - #1

Merged
devigned merged 6 commits into
mainfrom
code-quality-and-embedding
Jun 3, 2026
Merged

feat: embedding providers, code quality fixes, and comprehensive test coverage#1
devigned merged 6 commits into
mainfrom
code-quality-and-embedding

Conversation

@devigned

@devigned devigned commented Jun 3, 2026

Copy link
Copy Markdown
Owner

Summary

This PR adds server-side embedding support, fixes critical correctness issues found through two rounds of code quality analysis, and significantly expands test coverage.

Embedding Providers (OMS Spec Section 8)

  • LocalEmbedder: In-process via fastembed-rs (all-MiniLM-L6-v2, 384 dims) for local dev/testing
  • OpenAiCompatibleEmbedder: Any OpenAI-compatible /v1/embeddings endpoint (OpenAI, Azure, Ollama, vLLM)
  • NoopEmbedder: Pass-through for callers who manage their own embeddings
  • Auto-embed on write (create/update) and search when no embedding provided
  • Client-supplied embeddings always take priority

P0 Correctness Fixes

  • Upsert scope: Fixed to match all 8 scope columns (was only matching scope_tenant_id)
  • GDPR audit hash integrity: Added redacted column so hash chain remains verifiable after anonymization

P1 Fixes

  • MCP gdpr_purge tool: Added missing tool for spec parity (10 tools total)
  • Atomic bubble_up: Dedupe check + inserts now in single transaction
  • OpenAI embedder hardening: Constructor returns Result, 4xx/5xx/429 error classification, index validation
  • MCP tool naming: graph_traversetraverse_graph (matches HTTP API)
  • Atomic store provisioning: get_or_create_store with INSERT OR IGNORE eliminates TOCTOU race
  • MCP/HTTP response shape normalization: deleted_countdeleted

Test Coverage

  • 144 tests across 5 crates (up from ~77), 0 clippy warnings
  • 20 kd6-core unit tests (FakeEmbedder-based)
  • 13 kd6-embed unit tests (wiremock for HTTP mocking)
  • 17 MCP integration tests (tool registration, server info, gdpr_purge)
  • 47 HTTP server integration tests (inheritance, shared spaces, embedding)
  • Tests use deterministic FakeEmbedder (no model downloads in CI)

Spec Updates

  • Added Embedding Provider Interface (Section 8)
  • Documented client-side vs server-side embedding design

devigned added 4 commits June 3, 2026 13:18
Add Section 8 to the OMS specification defining:
- EmbeddingProvider trait with embed_texts, embed_query, dimensions, model_id
- Three provider types: local (in-process), remote (OpenAI-compatible), noop
- Server-side auto-embedding on write (create/update) and search
- Client-supplied embeddings always take priority over auto-embedding
- Dimension validation rejects mismatched caller-provided vectors

Signed-off-by: David Justice <david@justice.dev>
Implement the EmbeddingProvider trait (kd6-core) and three providers:

- LocalEmbedder: In-process ONNX via fastembed-rs (all-MiniLM-L6-v2,
  384 dimensions). Ideal for local development and testing.
- OpenAiCompatibleEmbedder: Calls any /v1/embeddings endpoint (OpenAI,
  Azure, Ollama, vLLM). Includes error classification (4xx/429/5xx),
  out-of-order index sorting, and dimension/count validation.
- NoopEmbedder: Pass-through that returns errors on embed calls,
  for deployments where callers manage their own embeddings.

Also adds auto_embed_content, auto_embed_query, and auto_embed_update
helpers in kd6-core that skip embedding when a provider is noop or
when the caller supplies their own vectors.

Includes 20 unit tests for kd6-core embedding helpers (FakeEmbedder)
and 13 wiremock-based tests for the OpenAI-compatible provider.

Signed-off-by: David Justice <david@justice.dev>
Address correctness issues in the SQLite backend:

- Upsert scope matching: match all 8 scope columns using COALESCE
  for NULL-safe comparison, persist scope on UPDATE, return original
  created_at from database rather than request time
- GDPR audit hash integrity: add redacted column to audit_log so
  hash chain remains verifiable after anonymization (entries are
  flagged rather than having their hashes broken)
- Atomic bubble_up: extract insert_memory_on_conn helper, run both
  the dedupe check and inserts inside a single BEGIN IMMEDIATE txn
- Atomic store provisioning: add get_or_create_store to OmsProvider
  trait with INSERT OR IGNORE + unique (tenant_id, name) index,
  eliminating TOCTOU race in resolve_store
- Add full-scope upsert index, pagination index, and stores unique
  constraint via new migration

Signed-off-by: David Justice <david@justice.dev>
Wire embedding into both server frontends:

HTTP server (kd6-server):
- AppState carries Arc<dyn EmbeddingProvider>
- Auto-embed on create, update, and search routes
- Dimension validation rejects mismatched caller vectors
- resolve_store uses atomic get_or_create_store

MCP server (kd6-mcp):
- Add gdpr_purge tool (10 tools total, spec parity)
- Rename graph_traverse → traverse_graph (matches HTTP API)
- Normalize response shape (deleted_count → deleted)
- CreateMemoryParams accepts serde_json::Value content and scope fields
- Expose list_tools() for test introspection

Both servers select provider via KD6_EMBEDDING_PROVIDER env var
(local | openai-compatible | none).

Signed-off-by: David Justice <david@justice.dev>
@devigned
devigned force-pushed the code-quality-and-embedding branch from a0113e7 to 7c223a3 Compare June 3, 2026 17:20
Expand test coverage from 77 to 144 tests:

kd6-server (47 tests):
- Embedding integration: auto-embed on write, search, update
- Dimension validation rejects mismatched caller vectors
- Caller-supplied embeddings preserved (not overwritten)
- Vector search returns scored results
- Structured content with arrays embeds correctly
- Inheritance create/delete and bubble-up
- Shared space lifecycle (create, join, leave, delete)
- Noop embedder pass-through behavior

kd6-mcp (17 tests):
- All 10 tools exercised end-to-end
- MCP tool registration and server info smoke tests
- GDPR purge with selective agent data removal
- Error handling for invalid UUIDs and missing resources

All tests use deterministic FakeEmbedder (3-dim, no model downloads)
to ensure fast, reproducible CI runs.

Also updates copilot-instructions.md with current tool count and
adds Squad/LangChain validation scaffold.

Signed-off-by: David Justice <david@justice.dev>
@devigned
devigned force-pushed the code-quality-and-embedding branch from 7c223a3 to d91ec78 Compare June 3, 2026 17:23
Add Makefile with targets mirroring CI:
- make ci: fmt-check → lint → build → test (use before every push)
- make fix: auto-format then run lint/build/test
- make run-server / make run-mcp: convenience targets

Update CI workflow to use 'make ci' so local and CI run identical
steps in the same order.

Signed-off-by: David Justice <david@justice.dev>
@devigned
devigned merged commit 037d351 into main Jun 3, 2026
1 check passed
@devigned
devigned deleted the code-quality-and-embedding branch June 3, 2026 17:36
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