Add embedding-based semantic search (third retrieval channel)

[ZmeiGorynych]

Summary

• Add a third retrieval channel to search: dense embedding similarity via litellm, RRF-fused with the existing tantivy + entity-overlap BM25 channels. Entity hits — previously tantivy-only — are now RRF-fused across channels 2 and 3.
• Persist embeddings in a sidecar table keyed by (canonical_id, embedding_model_name). Refresh inline on slayer ingest, edit_model, and save_memory; SHA256 content_hash makes idempotent re-runs cheap.
• Gate behind the optional embedding_search extra (litellm + numpy). When the extra is missing, no API key is configured, or there are no rows for the active model, the channel emits one warning and search degrades gracefully via tantivy + BM25.
• Refactor delete_model / delete_datasource cascade into the StorageBackend ABC so embedding cleanup is backend-agnostic; backends implement only the row primitives.

Configuration

• Default model: openai/text-embedding-3-small. Override via SLAYER_EMBEDDING_MODEL. Provider credentials (OPENAI_API_KEY, etc.) read by litellm directly.
• Switching the active model leaves prior rows inert; re-run slayer ingest (or re-save memories) to populate the new model's rows.
• Per-entity embed failures (rate limits, transient network errors, bad keys) are non-fatal and surface as warnings.

Test plan

• Unit suite green: poetry run pytest -m "not integration" — 2348 passed.
• Lint clean: poetry run ruff check slayer/ tests/.
• Manual smoke (with extra): poetry install -E embedding_search, set OPENAI_API_KEY, slayer datasources create demo --ingest, slayer memory save "...", then slayer search --question "..." — confirm channel 3 surfaces both memory and relevant entity.
• Manual smoke (without extra): fresh venv, poetry install (no extra), same search — confirm the response has a warning about missing embedding support and tantivy + BM25 hits still surface.
• Manual smoke (model change): with the extra installed, set SLAYER_EMBEDDING_MODEL=openai/text-embedding-3-large, re-run search — confirm channel 3 emits a "no embedding rows" warning, then re-ingest and re-run to confirm hits return.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Three‑channel semantic search (BM25 + full‑text + optional embeddings) with fused ranking; new REST /search, CLI search (+ refresh-samples), MCP tool and client search API.
  • Per‑column sampled snapshots persisted and refreshable; embedding sidecar persistence with cascade-delete behavior.

• Changes

  • Memory retrieval unified into search (separate recall surfaces removed); save memory now triggers best‑effort embedding refresh.
  • Schema version bump for models (v6) and related migration notes.

• Tests

  • Extensive new unit and end‑to‑end tests for search, embeddings, sampled refresh, and migrations.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a unified semantic search surface (BM25 + tantivy + optional embeddings fused via RRF), per-column sampled profiling and writeback, an embeddings sidecar with persistence and cascade deletes, ingestion/edit refresh hooks, storage ABC/backend changes, new API/CLI/MCP/Client search surfaces, v6 schema bump, removal of recall_memories, and extensive tests and docs.

Changes

Semantic search, embeddings, and sampled cache

Layer / File(s)	Summary
Data & migrations slayer/core/models.py, slayer/storage/migrations.py, slayer/storage/v6_migration.py, pyproject.toml	Add Column.sampled, bump SlayerModel default version to 6, register v5→v6 no-op migrator and Embedding entity; add tantivy and optional litellm/numpy extras and embedding_search.
Embeddings subsystem slayer/embeddings/models.py, slayer/embeddings/client.py, slayer/embeddings/ranker.py, slayer/embeddings/service.py, slayer/embeddings/__init__.py	Persisted Embedding model; lazy litellm-based async embed client with query cache; NumPy top-k cosine ranker; EmbeddingService for refresh/read with content-hash skipping and best-effort warnings; limited re-exports.
Storage & backends slayer/storage/base.py, slayer/storage/sqlite_storage.py, slayer/storage/yaml_storage.py, slayer/storage/join_sync.py	Extend StorageBackend ABC with embedding sidecar methods and update_column_sampled; implement embedding tables/files, CRUD, prefix deletion; cascade deletes for models/datasources/memories; delegate changes in JoinSync.
Search orchestration slayer/search/render.py, slayer/search/index.py, slayer/search/rrf.py, slayer/search/service.py, slayer/search/__init__.py	Renderers for entities/memories, in-memory Tantivy corpus/index, RRF fusion, SearchService implementing BM25 memory ranker, tantivy full-text, optional embedding similarity, fused SearchResponse (memories / example_queries / entities).
Engine profiling & ingestion slayer/engine/profiling.py, slayer/engine/ingestion.py	Centralized profiling helpers for Column.sampled, refresh helpers and edit/ingest hooks to backfill/persist sampled values and invoke embedding refresh best-effort.
Surfaces & clients slayer/api/server.py, slayer/cli.py, slayer/client/slayer_client.py, slayer/mcp/server.py	Add POST /search endpoint and SearchRequest, slayer search CLI + refresh-samples, SlayerClient.search, MCP search tool; remove /memories/recall, CLI memory recall, and recall_memories client; update help/docs wiring.
Memories write-side slayer/memories/*	Narrow memory package to write-only surfaces (save_memory, forget_memory), remove recall models/APIs, and perform best-effort embedding refresh during saves.
Docs & config docs/**, CLAUDE.md, .claude/skills/slayer-overview.md	Document semantic search design, channels, fusion rules, embedding gating and failure modes, sampled cache semantics and refresh triggers, migration notes, and CLI usage.
Tests tests/**	Add/extend tests: embeddings client/service/storage, search render/index/rrf/service, profiling/writeback, surfaces (API/CLI/MCP/client), migrations (v6), sampled persistence; remove/adjust recall-related tests.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant API as FastAPI /search
  participant SearchService
  participant Storage as StorageBackend
  participant Embeddings as EmbeddingService
  Client->>API: POST /search {entities, query, question}
  API->>SearchService: search(...)
  SearchService->>Storage: load models, memories, corpus metadata
  alt question present
    SearchService->>SearchService: build in-memory tantivy corpus & run full-text
  end
  alt embeddings enabled and question present
    SearchService->>Embeddings: fetch_corpus()
    SearchService->>Embeddings: embed_question(question)
    Embeddings->>Storage: read/write embedding rows (refresh)
    SearchService->>SearchService: compute cosine/top-k + fuse via RRF
  end
  SearchService-->>API: SearchResponse (memories, example_queries, entities, warnings)
  API-->>Client: JSON

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• MotleyAI/slayer#100: Prior work on memory/recall surface that this change supersedes/removes.
• MotleyAI/slayer#67: Related SlayerModel schema changes and migrations.
• MotleyAI/slayer#77: Related schema versioning and migration adjustments.

Suggested reviewers

• whimo
• AivanF

Poem

I burrow through indices with fluffy feet,
Fuse ranks with a twitch—RRF makes results neat.
I sample columns, stash snapshots so sweet,
Embed tiny vectors where memories meet.
Recall naps—now Search hops in to greet! 🐇

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch egor/dev-1386-semantic-search-using-embeddings

[coderabbitai]

Actionable comments posted: 13

🧹 Nitpick comments (8)

slayer/storage/v6_migration.py (1)

15-15: ⚡ Quick win

Use keyword arguments in migration registration decorator.

Line 15 uses two positional arguments; please switch to named arguments for consistency with repository Python style rules.

As per coding guidelines **/*.py: Use keyword arguments for functions with more than 1 parameter.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/storage/v6_migration.py` at line 15, The decorator usage
`@register_migration`("SlayerModel", 5) should use keyword arguments per style
rules; update the decorator call to use named parameters (e.g.,
register_migration(name="SlayerModel", version=5)) so the migration registration
is explicit and consistent with functions that take multiple parameters.

slayer/embeddings/client.py (1)

111-111: ⚡ Quick win

Use keyword args for the multi-parameter internal call.

Line 111 should pass texts explicitly for consistency with repo conventions.

💡 Suggested fix

-    result = await embed_batch([text], model=resolved_model)
+    result = await embed_batch(texts=[text], model=resolved_model)

As per coding guidelines **/*.py: Use keyword arguments for functions with more than 1 parameter.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/embeddings/client.py` at line 111, Call embed_batch using keyword
arguments to follow the repo convention: change the call that currently uses
positional args to pass texts explicitly (e.g., use texts=[text],
model=resolved_model). Update the invocation of embed_batch in
slayer/embeddings/client.py (the line with result = await embed_batch([text],
model=resolved_model)) so it uses the texts= keyword while keeping
model=resolved_model and the same variables (text, resolved_model).

tests/test_embeddings_storage.py (1)

32-38: ⚡ Quick win

Include JoinSyncStorage in this matrix too.

These tests only exercise the raw YAML/SQLite backends, but this PR also adds embedding-method delegation in JoinSyncStorage. Adding a wrapped-backend case here would catch a broken passthrough on the adapter path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_embeddings_storage.py` around lines 32 - 38, The parametrized
storage fixture currently yields only YAMLStorage and SQLiteStorage; add cases
that yield JoinSyncStorage wrappers to exercise delegation. Update the fixture
(storage) to include wrapped variants (e.g., for YAML and SQLite) or extend
params to include "join_yaml"/"join_sqlite", and when those values are seen,
construct JoinSyncStorage(inner=YAMLStorage(...)) and
JoinSyncStorage(inner=SQLiteStorage(...)) so tests exercise JoinSyncStorage's
passthrough behavior alongside YAMLStorage and SQLiteStorage.

tests/test_search_index.py (1)

151-163: 💤 Low value

Weak assertions don't verify the documented expectation.

The comment states "Both groups should be non-empty" but the assertions only check isinstance(..., list), which is always true. Consider asserting len(memory_hits) > 0 and len(entity_hits) > 0 to match the documented expectation.

Suggested fix

-    # Both groups should be non-empty for "customer" since memory 2 mentions
-    # "customers" and entities include customers/customer_id.
-    assert isinstance(memory_hits, list)  # type sanity
-    assert isinstance(entity_hits, list)
+    # Both groups should be non-empty for "customer" since memory 2 mentions
+    # "customers" and entities include customers/customer_id.
+    assert len(memory_hits) > 0, "expected at least one memory hit"
+    assert len(entity_hits) > 0, "expected at least one entity hit"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_search_index.py` around lines 151 - 163, The test
test_kind_field_split_for_memory_vs_entity documents that both memory_hits and
entity_hits should be non-empty but only asserts their types; update the
assertions in this test to assert len(memory_hits) > 0 and len(entity_hits) > 0
(instead of isinstance checks) so the behavior of search_index /
build_in_memory_index is actually verified for the "customer" query.

slayer/engine/profiling.py (1)

87-96: 💤 Low value

Minor: redundant or None in data_source handling.

On line 94, model.data_source or None is redundant since data_source is already Optional[str]. If it's None, the expression evaluates to None; if it's an empty string, converting to None might be intentional for disambiguation.

If the intent is to treat empty string as None, this is fine. Otherwise, it can be simplified.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/engine/profiling.py` around lines 87 - 96, The use of
"model.data_source or None" when calling engine.execute is redundant because
model.data_source is already Optional[str]; update the call in the try block
(the SlayerQuery.model_validate / engine.execute usage) to pass
model.data_source directly (i.e., remove the "or None" coalesce) unless you
explicitly want empty-string values converted to None—in that case add a short
comment clarifying that behavior.

slayer/search/service.py (1)

597-608: 💤 Low value

Accessing private method _list_all_model_identities outside storage layer.

The _collect_index_corpus method accesses storage._list_all_model_identities(), which is a private method (prefixed with _). While this works since SearchService is part of the same codebase, it couples the service to storage internals.

Consider whether a public method like list_all_models() or list_model_identities() would be more appropriate for the storage ABC, or document this as an intentional internal coupling.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/search/service.py` around lines 597 - 608, _collect_index_corpus is
calling the private storage method _list_all_model_identities which couples
SearchService to storage internals; replace that direct private call with a
public storage API (e.g., add and use list_model_identities or list_all_models
on the storage ABC) and update implementations accordingly so
_collect_index_corpus calls storage.list_model_identities() (keep using
storage.list_datasources() and storage.get_model(name, data_source=ds)
unchanged); alternatively, if this coupling is intentional, add a clear
comment/docstring on SearchService._collect_index_corpus noting the deliberate
use of the private API.

slayer/search/index.py (1)

121-169: 💤 Low value

Consider extracting entity-type renderers to reduce cognitive complexity.

SonarCloud flagged cognitive complexity of 16 (limit 15). While the function is readable as-is, extracting the inner loops for columns/measures/aggregations into helper functions would address this and improve testability.

However, given the function's straightforward iteration pattern and the minor threshold violation, this is low priority.

♻️ Optional refactor sketch

def _collect_model_children_pairs(
    model: SlayerModel,
    model_canonical: str,
) -> List[Tuple[str, str, str]]:
    """Extract column/measure/aggregation pairs for a single model."""
    out: List[Tuple[str, str, str]] = []
    for column in model.columns:
        if column.hidden:
            continue
        out.append((
            f"{model_canonical}.{column.name}", "column",
            render_column_text(model=model, column=column),
        ))
    for measure in model.measures:
        if measure.name is None:
            continue
        out.append((
            f"{model_canonical}.{measure.name}", "measure",
            render_measure_text(model=model, measure=measure),
        ))
    for aggregation in model.aggregations:
        out.append((
            f"{model_canonical}.{aggregation.name}", "aggregation",
            render_aggregation_text(model=model, aggregation=aggregation),
        ))
    return out

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/search/index.py` around lines 121 - 169, The function
_collect_render_pairs exceeds SonarCloud's cognitive complexity threshold due to
the inlined loops for model children; extract the column/measure/aggregation
handling into a helper (e.g., _collect_model_children_pairs) that accepts a
SlayerModel and its canonical id and returns List[Tuple[str,str,str]>; replace
the three inner loops inside _collect_render_pairs with a single call to that
helper and ensure the helper applies the same filters (skip hidden columns and
measures with name None) and uses render_column_text, render_measure_text, and
render_aggregation_text so behavior remains identical.

tests/test_search_writeback.py (1)

170-198: 💤 Low value

Unused monkeypatch fixture parameter.

The monkeypatch fixture is declared in the function signature but never used in this test. Consider removing it to avoid confusion.

♻️ Proposed fix

 `@pytest.mark.asyncio`
 async def test_inspect_model_writes_back_on_sampled_miss(
-    sqlite_table_setup, monkeypatch,
+    sqlite_table_setup,
 ) -> None:

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_search_writeback.py` around lines 170 - 198, The test function
test_inspect_model_writes_back_on_sampled_miss declares an unused monkeypatch
fixture parameter; remove monkeypatch from the function signature (or use it if
there was an intended patch) so the test signature becomes async def
test_inspect_model_writes_back_on_sampled_miss(sqlite_table_setup) -> None: and
update any call sites if needed; ensure no other references to monkeypatch exist
in this function.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/skills/slayer-overview.md:
- Line 35: Update the search description that currently calls it "two-channel"
to document three retrieval channels: entity-overlap BM25 over memories, tantivy
full-text over memories ∪ entities, and dense embedding similarity (optional)
over memories ∪ entities; keep the note that the query-bearing memories are
partitioned into `example_queries` and reference the optional embedding behavior
consistent with docs/concepts/search.md so the `search` overview matches the
full design.

In `@CLAUDE.md`:
- Around line 115-116: Update the CLAUDE.md synopsis for the slayer search CLI
to match the semantic-search docs by changing "two-channel" to "three-channel"
and adding the new flag to the usage string and description: include
--max-example-queries in the usage for `slayer search [--entity ENT ...]
[--query JSON_OR_@FILE] [--question TEXT] [--max-memories N] [--max-entities N]
[--max-example-queries N] [--format json|text]` and mention the third channel
(example queries) in the short description for the `slayer search` command so it
aligns with the semantic-search section.

In `@docs/concepts/ingestion.md`:
- Line 160: Update the sentence describing embedding refresh to clarify that
setting SLAYER_EMBEDDING_MODEL is optional: when the embedding_search extra is
installed and either a SLAYER_EMBEDDING_MODEL is configured or the runtime
default embedding model (e.g., openai/text-embedding-3-small) is used, the
embedding refresh will re-run for the datasource doc plus every visible model
and its visible children; mention SLAYER_EMBEDDING_MODEL as an optional override
so readers understand the default behavior and that re-ingests remain cheap due
to the SHA256 content_hash on each row.

In `@slayer/cli.py`:
- Around line 605-607: Calls to helper functions _run_search_refresh_samples,
_run_search_query, and _refresh_samples_async currently pass multiple positional
arguments; update each call to use keyword arguments (e.g.,
_run_search_refresh_samples(args=..., storage=...), _run_search_query(args=...,
storage=...), _refresh_samples_async(args=..., storage=...)) so the parameter
order is explicit and follows the repository Python guideline for functions with
more than one parameter.
- Around line 624-626: The loop that calls storage.get_model(model_name,
data_source=ds) currently continues silently when m is None, which lets typos in
a user-specified --model filter go unnoticed; change this by tracking unresolved
model names (e.g., collect model_name when storage.get_model returns None) and
after the loop, if the user provided --model and any unresolved names exist,
surface them to the user (print/log a clear error or warning and exit non-zero)
instead of proceeding to a “completed successfully” message; update the code
paths that reference model_name, m, storage.get_model and the --model handling
to fail-fast or fail loudly when requested models are missing.

In `@slayer/client/slayer_client.py`:
- Line 253: Update the stale docstring that currently reads "Two-channel
semantic search over memories + canonical entities." in slayer_client.py to
reflect the new optional third channel (embeddings) added by this PR: change
wording to indicate "Two- or three-channel semantic search" and enumerate the
channels (memories, canonical entities, and optional embeddings when the
embeddings channel/parameter is enabled), and mention any relevant parameter
name that enables the third channel so the docstring matches actual retrieval
behavior.

In `@slayer/embeddings/client.py`:
- Around line 108-120: The cache implementation using _QUERY_CACHE and
_QUERY_CACHE_MAX currently behaves FIFO because hits don't update recency;
modify the hit path in the function that looks up key (before returning cached)
to refresh recency by moving the key to the most-recent position (e.g., remove
and reinsert the key or use collections.OrderedDict and call move_to_end) so the
eviction logic that pops next(iter(_QUERY_CACHE)) effectively implements LRU;
keep the existing embed_batch, vec extraction, and eviction code but ensure hits
update the cache order before returning.

In `@slayer/mcp/server.py`:
- Around line 1892-1896: The current gate only marks column upserts and a few
source/filter changes, so pure measure/aggregation/join edits never set
model_level_change and thus skip handle_edit_refresh in edit_model; update the
edit-path to treat non-column semantic edits as model-level changes by setting
model_level_change = True (or otherwise marking the change) when encountering
edits to measures, aggregations, joins or similar non-column metadata, and
ensure changed_columns and model_level_change are passed into the existing
post-save/refresh logic (handle_edit_refresh) so the semantic-search/index
refresh path runs for these edits (refer to changed_columns, model_level_change,
handle_edit_refresh, and edit_model to locate where to set the flag and
propagate it).
- Around line 2574-2594: The docstring for the search function is out of
date—update the Slayer MCP `search` docstring to describe the new optional
embedding retrieval channel and how its results are fused with the existing
channels; explicitly mention that search now has a third (optional) embeddings
channel, when it runs, that fused ranking uses Reciprocal Rank Fusion (k=60)
across channels (and that entity hits remain channel-2 only), and keep the
existing notes about partitioning query-bearing memories into example_queries vs
memories (SlayerQuery) and capping behavior; reference the `search` function,
the embedding retrieval path, Reciprocal Rank Fusion (k=60), and
`SlayerQuery`/`example_queries` so readers can find the related logic.
- Around line 2128-2139: The post-save call to handle_edit_refresh should be
made truly best-effort: wrap the await handle_edit_refresh(...) call in a
try/except so any exception is caught and does not abort after the model is
persisted, log a warning including the exception details and context (engine,
saved_model.name, data_source), and capture the return value (e.g., errors =
await handle_edit_refresh(...)); if that return contains non-empty errors, log
them as warnings and append them to the MCP response warnings payload (or to a
warnings list on the current save result/context) so the API response surfaces
these refresh warnings instead of dropping them.
- Around line 1258-1279: The code collects measure profiles via
_collect_measure_profile(model=model, engine=engine) into measure_profile but
never persists them; update the logic after measure_profile is assigned to
iterate its entries and call storage.update_column_sampled(...) for each
measured column (similar to the loop that persists dim profile values),
formatting values with the same _format_dim_profile_value or a suitable
formatter and catching exceptions to log warnings; reference
_collect_measure_profile, measure_profile, storage.update_column_sampled,
_format_dim_profile_value and Column.sampled so the persisted sampled values are
saved for numeric/date/time columns.

In `@slayer/search/__init__.py`:
- Around line 4-7: Update the package docstring in slayer/search/__init__.py so
it accurately describes SearchService as a three-channel orchestrator
(entity-overlap BM25 over memories, tantivy full-text over the unioned corpus,
plus an optional embedding-based retrieval channel) and mention that results are
fused (e.g., via RRF) and that the embedding channel is conditional; update any
brief usage notes to reflect the optional embeddings flag and fusion behavior.
Use the symbol SearchService to locate the orchestrator and edit the top-level
module docstring accordingly.

In `@tests/integration/test_integration.py`:
- Around line 3135-3143: The assertion currently allows unrelated hits like
"test_sqlite.customers.region"; tighten it so the test requires at least one
actual amount column. In the block that computes column_hits and expected,
remove "test_sqlite.customers.region" from expected and assert that there exists
a hit with id equal to "test_sqlite.orders.amount" or that endswith(".amount")
(i.e., replace the current combined any(...) or any(...endswith(".amount"))
check with a single assertion requiring any(h.id.endswith(".amount") or h.id ==
"test_sqlite.orders.amount") against column_hits) so only actual amount columns
satisfy the test.

---

Nitpick comments:
In `@slayer/embeddings/client.py`:
- Line 111: Call embed_batch using keyword arguments to follow the repo
convention: change the call that currently uses positional args to pass texts
explicitly (e.g., use texts=[text], model=resolved_model). Update the invocation
of embed_batch in slayer/embeddings/client.py (the line with result = await
embed_batch([text], model=resolved_model)) so it uses the texts= keyword while
keeping model=resolved_model and the same variables (text, resolved_model).

In `@slayer/engine/profiling.py`:
- Around line 87-96: The use of "model.data_source or None" when calling
engine.execute is redundant because model.data_source is already Optional[str];
update the call in the try block (the SlayerQuery.model_validate /
engine.execute usage) to pass model.data_source directly (i.e., remove the "or
None" coalesce) unless you explicitly want empty-string values converted to
None—in that case add a short comment clarifying that behavior.

In `@slayer/search/index.py`:
- Around line 121-169: The function _collect_render_pairs exceeds SonarCloud's
cognitive complexity threshold due to the inlined loops for model children;
extract the column/measure/aggregation handling into a helper (e.g.,
_collect_model_children_pairs) that accepts a SlayerModel and its canonical id
and returns List[Tuple[str,str,str]>; replace the three inner loops inside
_collect_render_pairs with a single call to that helper and ensure the helper
applies the same filters (skip hidden columns and measures with name None) and
uses render_column_text, render_measure_text, and render_aggregation_text so
behavior remains identical.

In `@slayer/search/service.py`:
- Around line 597-608: _collect_index_corpus is calling the private storage
method _list_all_model_identities which couples SearchService to storage
internals; replace that direct private call with a public storage API (e.g., add
and use list_model_identities or list_all_models on the storage ABC) and update
implementations accordingly so _collect_index_corpus calls
storage.list_model_identities() (keep using storage.list_datasources() and
storage.get_model(name, data_source=ds) unchanged); alternatively, if this
coupling is intentional, add a clear comment/docstring on
SearchService._collect_index_corpus noting the deliberate use of the private
API.

In `@slayer/storage/v6_migration.py`:
- Line 15: The decorator usage `@register_migration`("SlayerModel", 5) should use
keyword arguments per style rules; update the decorator call to use named
parameters (e.g., register_migration(name="SlayerModel", version=5)) so the
migration registration is explicit and consistent with functions that take
multiple parameters.

In `@tests/test_embeddings_storage.py`:
- Around line 32-38: The parametrized storage fixture currently yields only
YAMLStorage and SQLiteStorage; add cases that yield JoinSyncStorage wrappers to
exercise delegation. Update the fixture (storage) to include wrapped variants
(e.g., for YAML and SQLite) or extend params to include
"join_yaml"/"join_sqlite", and when those values are seen, construct
JoinSyncStorage(inner=YAMLStorage(...)) and
JoinSyncStorage(inner=SQLiteStorage(...)) so tests exercise JoinSyncStorage's
passthrough behavior alongside YAMLStorage and SQLiteStorage.

In `@tests/test_search_index.py`:
- Around line 151-163: The test test_kind_field_split_for_memory_vs_entity
documents that both memory_hits and entity_hits should be non-empty but only
asserts their types; update the assertions in this test to assert
len(memory_hits) > 0 and len(entity_hits) > 0 (instead of isinstance checks) so
the behavior of search_index / build_in_memory_index is actually verified for
the "customer" query.

In `@tests/test_search_writeback.py`:
- Around line 170-198: The test function
test_inspect_model_writes_back_on_sampled_miss declares an unused monkeypatch
fixture parameter; remove monkeypatch from the function signature (or use it if
there was an intended patch) so the test signature becomes async def
test_inspect_model_writes_back_on_sampled_miss(sqlite_table_setup) -> None: and
update any call sites if needed; ensure no other references to monkeypatch exist
in this function.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 877745c5-8486-4efa-a343-3b3508cfd27e

📥 Commits

Reviewing files that changed from the base of the PR and between 013fca7 and 2dfadb7.

⛔ Files ignored due to path filters (1)

• poetry.lock is excluded by !**/*.lock

📒 Files selected for processing (61)

• .claude/skills/slayer-overview.md
• CLAUDE.md
• docs/concepts/ingestion.md
• docs/concepts/memories.md
• docs/concepts/models.md
• docs/concepts/queries.md
• docs/concepts/references.md
• docs/concepts/schema-drift.md
• docs/concepts/search.md
• docs/configuration/storage.md
• pyproject.toml
• slayer/api/server.py
• slayer/cli.py
• slayer/client/slayer_client.py
• slayer/core/models.py
• slayer/embeddings/__init__.py
• slayer/embeddings/client.py
• slayer/embeddings/models.py
• slayer/embeddings/ranker.py
• slayer/embeddings/service.py
• slayer/engine/ingestion.py
• slayer/engine/profiling.py
• slayer/mcp/server.py
• slayer/memories/__init__.py
• slayer/memories/models.py
• slayer/memories/ranker.py
• slayer/memories/service.py
• slayer/search/__init__.py
• slayer/search/index.py
• slayer/search/render.py
• slayer/search/rrf.py
• slayer/search/service.py
• slayer/storage/base.py
• slayer/storage/join_sync.py
• slayer/storage/migrations.py
• slayer/storage/sqlite_storage.py
• slayer/storage/v6_migration.py
• slayer/storage/yaml_storage.py
• tests/integration/test_integration.py
• tests/test_embeddings_client.py
• tests/test_embeddings_service.py
• tests/test_embeddings_storage.py
• tests/test_engine_profiling.py
• tests/test_memories_cli.py
• tests/test_memories_client.py
• tests/test_memories_inspect.py
• tests/test_memories_mcp.py
• tests/test_memories_rest.py
• tests/test_memories_storage.py
• tests/test_migrations.py
• tests/test_search_index.py
• tests/test_search_render.py
• tests/test_search_rrf.py
• tests/test_search_service.py
• tests/test_search_surfaces.py
• tests/test_search_three_channel.py
• tests/test_search_writeback.py
• tests/test_storage_sampled.py
• tests/test_storage_type_refinement.py
• tests/test_v4_migration.py
• tests/test_v6_migration.py

[coderabbitai]

♻️ Duplicate comments (1)

CLAUDE.md (1)

115-116: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix CLI synopsis to reflect the third channel and complete flag set.

This line still says "two-channel" and omits --max-example-queries from the usage string, creating inconsistency with the semantic-search section above (line 94) which describes three channels and with line 96 which lists the flag.

📝 Proposed documentation fix

-- `slayer search [--entity ENT ...] [--query JSON_OR_@FILE] [--question TEXT] [--max-memories N] [--max-entities N] [--format json|text]` (DEV-1375) — two-channel semantic search over memories + canonical entities. See [docs/concepts/search.md](docs/concepts/search.md).
+- `slayer search [--entity ENT ...] [--query JSON_OR_@FILE] [--question TEXT] [--max-memories N] [--max-example-queries N] [--max-entities N] [--format json|text]` (DEV-1375/DEV-1386) — up to three-channel semantic search over memories + canonical entities (BM25 + tantivy + optional embeddings). See [docs/concepts/search.md](docs/concepts/search.md).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CLAUDE.md` around lines 115 - 116, Update the CLI synopsis string in the
`slayer search` usage line to reflect the third channel and include the missing
flag: change "two-channel semantic search over memories + canonical entities" to
"three-channel semantic search over memories + canonical entities + examples"
and add the `--max-example-queries N` (or `--max-example-queries` form used
elsewhere) into the flags list for the `slayer search [--entity ...] [--query
...] [--question ...] [--max-memories N] [--max-entities N] [--format
json|text]` synopsis so it matches the semantic-search section and the flags
listed elsewhere.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@CLAUDE.md`:
- Around line 115-116: Update the CLI synopsis string in the `slayer search`
usage line to reflect the third channel and include the missing flag: change
"two-channel semantic search over memories + canonical entities" to
"three-channel semantic search over memories + canonical entities + examples"
and add the `--max-example-queries N` (or `--max-example-queries` form used
elsewhere) into the flags list for the `slayer search [--entity ...] [--query
...] [--question ...] [--max-memories N] [--max-entities N] [--format
json|text]` synopsis so it matches the semantic-search section and the flags
listed elsewhere.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d608308b-cf0a-4405-b0d3-7cd5ea8f5967

📥 Commits

Reviewing files that changed from the base of the PR and between 2dfadb7 and eccfa88.

📒 Files selected for processing (5)

• CLAUDE.md
• docs/concepts/queries.md
• docs/concepts/references.md
• pyproject.toml
• tests/integration/test_integration.py

✅ Files skipped from review due to trivial changes (2)

• docs/concepts/queries.md
• docs/concepts/references.md

🚧 Files skipped from review as they are similar to previous changes (2)

• pyproject.toml
• tests/integration/test_integration.py

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@slayer/cli.py`:
- Around line 523-526: The refresh-samples command is ignoring the shared
--format flag; update the refresh-samples handler to check args.format and emit
JSON when args.format == "json" (otherwise keep text), i.e. where the command
currently prints text for results replace the unconditional text output with a
conditional that serializes the result to JSON when args.format == "json" (use
json.dumps) and otherwise prints the existing text format; also apply the same
change to the analogous block referenced at the other location (around lines
640-649) so both refresh paths honor args.format.
- Line 4: Remove the top-level "import asyncio" and instead import run_sync from
slayer.async_utils, then replace the two calls to asyncio.run(...) in
slayer/cli.py with run_sync(...) (use run_sync from slayer.async_utils), passing
the coroutine via the named parameter (e.g., coro=YourAsyncEntry(...)) and
convert any positional arguments you were passing into keyword arguments on that
coroutine call; update both occurrences (the ones previously calling asyncio.run
at the two spots mentioned) to use run_sync and remove the now-unused asyncio
import.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3dfe7cb2-60e0-4d19-a25e-431b59b443e5

📥 Commits

Reviewing files that changed from the base of the PR and between eccfa88 and a1faf7d.

📒 Files selected for processing (5)

• slayer/cli.py
• slayer/search/index.py
• tests/test_embeddings_client.py
• tests/test_embeddings_service.py
• tests/test_search_three_channel.py

🚧 Files skipped from review as they are similar to previous changes (4)

• tests/test_embeddings_client.py
• slayer/search/index.py
• tests/test_search_three_channel.py
• tests/test_embeddings_service.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@slayer/mcp/server.py`:
- Line 2628: Update the docstring summary line that currently reads "Two-channel
semantic search over memories + canonical entities." to accurately reflect the
implementation described in the body (three channels); specifically change that
first-line summary to mention "Three-channel semantic search" (or a short phrase
like "Three-channel semantic search over memories, canonical entities, and the
third channel described below") so the one-line summary matches the docstring
body for the function whose docstring begins with "Two-channel semantic search
over memories + canonical entities."

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a04e748b-defc-4366-94f8-e1e9a4d43553

📥 Commits

Reviewing files that changed from the base of the PR and between ca8d67f and dfa47f0.

📒 Files selected for processing (10)

• .claude/skills/slayer-overview.md
• CLAUDE.md
• docs/concepts/ingestion.md
• slayer/cli.py
• slayer/client/slayer_client.py
• slayer/embeddings/client.py
• slayer/mcp/server.py
• slayer/search/__init__.py
• slayer/storage/v6_migration.py
• tests/integration/test_integration.py

✅ Files skipped from review due to trivial changes (2)

• .claude/skills/slayer-overview.md
• docs/concepts/ingestion.md

🚧 Files skipped from review as they are similar to previous changes (4)

• slayer/storage/v6_migration.py
• slayer/embeddings/client.py
• tests/integration/test_integration.py
• slayer/cli.py

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.6% Duplication on New Code

See analysis details on SonarQube Cloud