diff --git a/packages/core/src/repowise/core/analysis/decisions/semantic_match.py b/packages/core/src/repowise/core/analysis/decisions/semantic_match.py index f75a2cd34..181151490 100644 --- a/packages/core/src/repowise/core/analysis/decisions/semantic_match.py +++ b/packages/core/src/repowise/core/analysis/decisions/semantic_match.py @@ -357,7 +357,13 @@ async def find_duplicate_decision( if not query: return None try: - results = await store.search(query, limit=SEARCH_FETCH) + # kind="document": this is decision text compared against other + # decision text (near-duplicate matching), not a natural-language + # question against stored documents. Stored decision vectors always + # embed at the document prefix (see upsert_decision_vectors); an + # asymmetric-prefix embedder that got the default "query" here would + # search with the wrong framing and silently miss real duplicates. + results = await store.search(query, limit=SEARCH_FETCH, kind="document") except Exception: return None @@ -397,7 +403,8 @@ async def find_related_decisions( if not query: return [] try: - results = await store.search(query, limit=SEARCH_FETCH) + # kind="document" — same reasoning as find_duplicate_decision above. + results = await store.search(query, limit=SEARCH_FETCH, kind="document") except Exception: return [] return _related_from_results(results, lo=lo, hi=hi, exclude_ids=exclude_ids, limit=limit) @@ -452,7 +459,10 @@ async def find_related_decisions_many( try: # Empty query texts still occupy their slot (keeps results aligned) # but must not reach the embedder — some providers reject "". - all_results = await store.search_many([q or " " for q in queries], limit=SEARCH_FETCH) + # kind="document" — same reasoning as find_duplicate_decision above. + all_results = await store.search_many( + [q or " " for q in queries], limit=SEARCH_FETCH, kind="document" + ) except Exception: return [[] for _ in items] if len(all_results) != len(items): diff --git a/packages/core/src/repowise/core/persistence/vector_store/_base.py b/packages/core/src/repowise/core/persistence/vector_store/_base.py index 301177d80..24c97f70e 100644 --- a/packages/core/src/repowise/core/persistence/vector_store/_base.py +++ b/packages/core/src/repowise/core/persistence/vector_store/_base.py @@ -195,15 +195,27 @@ async def embed_batch(self, items: list[tuple[str, str, dict]]) -> None: for page_id, text, metadata in items: await self.embed_and_upsert(page_id, text, metadata) - async def embed_texts(self, texts: list[str]) -> list[list[float]] | None: + async def embed_texts( + self, texts: list[str], *, kind: str = "document" + ) -> list[list[float]] | None: """Embed *texts* in batched embedder requests, without upserting. Lets a caller that needs the raw vectors (e.g. decision dedup, which - searches *and* upserts the same text) pay for one batched embedding - instead of one round-trip per item. Returns ``None`` when the backend - holds no embedder — callers must fall back to the per-item text APIs. - Chunked so a large input can't blow the embedder's per-request token - cap; each text is capped at :data:`EMBED_TEXT_MAX_CHARS`. + searches *and* upserts the same text — pass ``kind="document"``, the + default, so both directions embed identically) pay for one batched + embedding instead of one round-trip per item. Returns ``None`` when + the backend holds no embedder — callers must fall back to the + per-item text APIs. Chunked so a large input can't blow the + embedder's per-request token cap; each text is capped at + :data:`EMBED_TEXT_MAX_CHARS`. + + Args: + kind: ``"query"`` or ``"document"`` — forwarded to the embedder + so a directional model (see + :func:`repowise.core.providers.embedding.base.resolve_embed_prefix`) + applies the right framing. A caller embedding a natural- + language question to search against stored pages must pass + ``kind="query"``. """ embedder = getattr(self, "_embedder", None) if embedder is None: @@ -212,7 +224,7 @@ async def embed_texts(self, texts: list[str]) -> list[list[float]] | None: return [] out: list[list[float]] = [] for _chunk, capped_texts in iter_embed_chunks([("", t, {}) for t in texts]): - out.extend(await embedder.embed(capped_texts)) + out.extend(await embedder.embed(capped_texts, kind=kind)) return out async def search_by_vector( @@ -239,11 +251,26 @@ async def upsert_vectors(self, items: list[tuple[str, list[float], dict]]) -> bo return False @abstractmethod - async def search(self, query: str, limit: int = 10) -> list[SearchResult]: - """Embed *query* and return the *limit* nearest pages.""" + async def search( + self, query: str, limit: int = 10, *, kind: str = "query" + ) -> list[SearchResult]: + """Embed *query* and return the *limit* nearest pages. + + Args: + kind: ``"query"`` or ``"document"``, forwarded to the embedder + (see :meth:`embed_texts`). The default matches what every + existing caller wants — *query* is a natural-language + question being matched against stored documents. Decision + near-duplicate lookup is the one caller that isn't: it is + comparing decision text to other decision text, symmetric by + construction, so it passes ``kind="document"`` to match the + prefix the candidate text was — or will be — stored under. + """ ... - async def search_many(self, queries: list[str], limit: int = 10) -> list[list[SearchResult]]: + async def search_many( + self, queries: list[str], limit: int = 10, *, kind: str = "query" + ) -> list[list[SearchResult]]: """Batch variant of :meth:`search` — one result list per query, aligned by index. @@ -253,13 +280,16 @@ async def search_many(self, queries: list[str], limit: int = 10) -> list[list[Se Backends override this to embed *all* queries in a single embedder call — the network round-trip dominates each search, so batching the embedding turns N round-trips into 1. + + Args: + kind: See :meth:`search`; forwarded unchanged to every query. """ import asyncio as _asyncio if not queries: return [] results = await _asyncio.gather( - *(self.search(q, limit=limit) for q in queries), return_exceptions=True + *(self.search(q, limit=limit, kind=kind) for q in queries), return_exceptions=True ) return [r if isinstance(r, list) else [] for r in results] diff --git a/packages/core/src/repowise/core/persistence/vector_store/in_memory.py b/packages/core/src/repowise/core/persistence/vector_store/in_memory.py index 559693417..d91a99234 100644 --- a/packages/core/src/repowise/core/persistence/vector_store/in_memory.py +++ b/packages/core/src/repowise/core/persistence/vector_store/in_memory.py @@ -66,10 +66,12 @@ async def upsert_vectors(self, items: list[tuple[str, list[float], dict]]) -> bo self._store[page_id] = (list(vector), dict(metadata)) return True - async def search(self, query: str, limit: int = 10) -> list[SearchResult]: + async def search( + self, query: str, limit: int = 10, *, kind: str = "query" + ) -> list[SearchResult]: if not self._store: return [] - q_vecs = await self._embedder.embed([query]) + q_vecs = await self._embedder.embed([query], kind=kind) return self._search_by_vector(q_vecs[0], limit) async def search_by_vector(self, vector: list[float], limit: int = 10) -> list[SearchResult]: @@ -77,13 +79,15 @@ async def search_by_vector(self, vector: list[float], limit: int = 10) -> list[S return [] return self._search_by_vector(vector, limit) - async def search_many(self, queries: list[str], limit: int = 10) -> list[list[SearchResult]]: + async def search_many( + self, queries: list[str], limit: int = 10, *, kind: str = "query" + ) -> list[list[SearchResult]]: """One embedder call for all queries, then local scoring per query.""" if not queries: return [] if not self._store: return [[] for _ in queries] - q_vecs = await self._embedder.embed(list(queries)) + q_vecs = await self._embedder.embed(list(queries), kind=kind) return [self._search_by_vector(q_vec, limit) for q_vec in q_vecs] async def delete(self, page_id: str) -> None: diff --git a/packages/core/src/repowise/core/persistence/vector_store/lancedb_store.py b/packages/core/src/repowise/core/persistence/vector_store/lancedb_store.py index 2775aaec3..fe3bdff6b 100644 --- a/packages/core/src/repowise/core/persistence/vector_store/lancedb_store.py +++ b/packages/core/src/repowise/core/persistence/vector_store/lancedb_store.py @@ -263,12 +263,14 @@ async def upsert_vectors(self, items: list[tuple[str, list[float], dict]]) -> bo await self._upsert_rows(rows) return True - async def search(self, query: str, limit: int = 10) -> list[SearchResult]: + async def search( + self, query: str, limit: int = 10, *, kind: str = "query" + ) -> list[SearchResult]: await self._ensure_connected() if self._table is None: return [] - q_vecs = await self._embedder.embed([query]) + q_vecs = await self._embedder.embed([query], kind=kind) return await self._search_by_vector([float(v) for v in q_vecs[0]], limit, query=query) async def search_by_vector(self, vector: list[float], limit: int = 10) -> list[SearchResult]: @@ -277,14 +279,16 @@ async def search_by_vector(self, vector: list[float], limit: int = 10) -> list[S return [] return await self._search_by_vector([float(v) for v in vector], limit) - async def search_many(self, queries: list[str], limit: int = 10) -> list[list[SearchResult]]: + async def search_many( + self, queries: list[str], limit: int = 10, *, kind: str = "query" + ) -> list[list[SearchResult]]: """One embedder call for all queries; the vector lookups are local.""" if not queries: return [] await self._ensure_connected() if self._table is None: return [[] for _ in queries] - q_vecs = await self._embedder.embed(list(queries)) + q_vecs = await self._embedder.embed(list(queries), kind=kind) out: list[list[SearchResult]] = [] for query, q_vec in zip(queries, q_vecs, strict=True): try: diff --git a/packages/core/src/repowise/core/persistence/vector_store/pgvector_store.py b/packages/core/src/repowise/core/persistence/vector_store/pgvector_store.py index 07ccc56cb..7d5989343 100644 --- a/packages/core/src/repowise/core/persistence/vector_store/pgvector_store.py +++ b/packages/core/src/repowise/core/persistence/vector_store/pgvector_store.py @@ -114,8 +114,10 @@ async def upsert_vectors(self, items: list[tuple[str, list[float], dict]]) -> bo await session.commit() return True - async def search(self, query: str, limit: int = 10) -> list[SearchResult]: - q_vecs = await self._embedder.embed([query]) + async def search( + self, query: str, limit: int = 10, *, kind: str = "query" + ) -> list[SearchResult]: + q_vecs = await self._embedder.embed([query], kind=kind) return await self.search_by_vector(q_vecs[0], limit) async def search_by_vector(self, vector: list[float], limit: int = 10) -> list[SearchResult]: @@ -150,11 +152,13 @@ async def search_by_vector(self, vector: list[float], limit: int = 10) -> list[S for r in raw ] - async def search_many(self, queries: list[str], limit: int = 10) -> list[list[SearchResult]]: + async def search_many( + self, queries: list[str], limit: int = 10, *, kind: str = "query" + ) -> list[list[SearchResult]]: """One embedder call for all queries; per-query SELECTs share a session.""" if not queries: return [] - q_vecs = await self._embedder.embed(list(queries)) + q_vecs = await self._embedder.embed(list(queries), kind=kind) from sqlalchemy.sql import text as sa_text diff --git a/packages/core/src/repowise/core/providers/embedding/base.py b/packages/core/src/repowise/core/providers/embedding/base.py index ed442ffac..e643f6c27 100644 --- a/packages/core/src/repowise/core/providers/embedding/base.py +++ b/packages/core/src/repowise/core/providers/embedding/base.py @@ -66,6 +66,38 @@ def resolve_embedding_timeout( return selected +_EMBED_PREFIX_ENV: dict[str, str] = { + "query": "REPOWISE_EMBED_QUERY_PREFIX", + "document": "REPOWISE_EMBED_DOC_PREFIX", +} + + +def resolve_embed_prefix(kind: str) -> str: + """The literal text to prepend before embedding a batch of *kind*. + + Some embedding models (e.g. asymmetric-instruction models like + NVIDIA's Nemotron-3-Embed) are trained with a fixed ``"query: "`` / + ``"passage: "`` (or similar) framing and never apply it themselves — the + caller has to. Most models (OpenAI, Gemini, ...) want the raw text and + would be actively hurt by an uninvited prefix. + + Reading ``REPOWISE_EMBED_QUERY_PREFIX`` / ``REPOWISE_EMBED_DOC_PREFIX`` + keeps this opt-in and per-deployment: unset (the default) resolves to + ``""`` for both kinds, so an embedder that calls this is inert unless the + operator explicitly configures a model that needs it. + + Args: + kind: ``"query"`` or ``"document"`` — which side of retrieval *texts* + are on. Anything else is a caller bug, not a misconfiguration, so + it raises rather than silently defaulting. + """ + try: + var = _EMBED_PREFIX_ENV[kind] + except KeyError: + raise ValueError(f"kind must be 'query' or 'document', got {kind!r}") from None + return os.environ.get(var, "") + + def _report_invalid_timeout(var: str, raw: str, default: float) -> None: """Say it where the user will actually see it. @@ -96,11 +128,17 @@ def dimensions(self) -> int: """Number of dimensions in the embedding vector.""" ... - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: """Embed a batch of texts. Args: texts: Non-empty list of strings to embed. + kind: ``"query"`` or ``"document"`` — which side of retrieval + *texts* are on. Implementations that don't need the + distinction (most do not) accept and ignore it; it exists so + an asymmetric-instruction model can apply the right prefix + (see :func:`resolve_embed_prefix`) without every call site + needing to know which models care. Returns: List of unit-length float vectors, one per input string. @@ -125,7 +163,8 @@ class MockEmbedder: dimensions: int = 8 - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + del kind # deterministic hash is direction-agnostic results: list[list[float]] = [] for text in texts: digest = hashlib.sha256(text.encode()).digest() diff --git a/packages/core/src/repowise/core/providers/embedding/edenai.py b/packages/core/src/repowise/core/providers/embedding/edenai.py index a13315eed..4d193fa5e 100644 --- a/packages/core/src/repowise/core/providers/embedding/edenai.py +++ b/packages/core/src/repowise/core/providers/embedding/edenai.py @@ -101,12 +101,18 @@ def __init__( def dimensions(self) -> int: return self._dimensions - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: """Embed a batch of texts using Eden AI. Runs the synchronous SDK call in a thread pool to avoid blocking the asyncio event loop. + + Args: + texts: Non-empty list of strings to embed. + kind: Accepted for Embedder-protocol parity; unused. None of the + models in ``_DIMS`` are configured with a directional prefix. """ + del kind if not texts: return [] diff --git a/packages/core/src/repowise/core/providers/embedding/gemini.py b/packages/core/src/repowise/core/providers/embedding/gemini.py index 0d9bd8fce..a8d06df77 100644 --- a/packages/core/src/repowise/core/providers/embedding/gemini.py +++ b/packages/core/src/repowise/core/providers/embedding/gemini.py @@ -79,7 +79,7 @@ def __init__( def dimensions(self) -> int: return self._output_dimensionality - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: """Embed a batch of texts using Gemini. Runs the synchronous SDK call in a thread pool to avoid blocking the @@ -87,10 +87,15 @@ async def embed(self, texts: list[str]) -> list[list[float]]: Args: texts: Non-empty list of strings to embed. + kind: Accepted for Embedder-protocol parity with directional + embedders; Gemini's ``task_type`` already encodes + query-vs-document framing (see ``__init__``), so this is + ignored here rather than doubly applied. Returns: List of unit-length (L2-normalized) float vectors. """ + del kind if not texts: return [] diff --git a/packages/core/src/repowise/core/providers/embedding/ollama.py b/packages/core/src/repowise/core/providers/embedding/ollama.py index bbc87b1bf..8d603462c 100644 --- a/packages/core/src/repowise/core/providers/embedding/ollama.py +++ b/packages/core/src/repowise/core/providers/embedding/ollama.py @@ -84,8 +84,16 @@ def __init__( def dimensions(self) -> int: return self._dimensions - async def embed(self, texts: list[str]) -> list[list[float]]: - """Embed a batch of texts using Ollama's native API.""" + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + """Embed a batch of texts using Ollama's native API. + + Args: + texts: Non-empty list of strings to embed. + kind: Accepted for Embedder-protocol parity; unused. Ollama + embedding models exposed here have no configured directional + prefix, so nothing is applied. + """ + del kind if not texts: return [] diff --git a/packages/core/src/repowise/core/providers/embedding/openai.py b/packages/core/src/repowise/core/providers/embedding/openai.py index 11a5a1269..f0da1589f 100644 --- a/packages/core/src/repowise/core/providers/embedding/openai.py +++ b/packages/core/src/repowise/core/providers/embedding/openai.py @@ -32,7 +32,7 @@ import os from typing import Any, ClassVar -from repowise.core.providers.embedding.base import resolve_embedding_timeout +from repowise.core.providers.embedding.base import resolve_embed_prefix, resolve_embedding_timeout class OpenAIEmbedder: @@ -123,7 +123,7 @@ def _resolve_dimensions(cls, dimensions: int | None, model: str) -> tuple[int, i def dimensions(self) -> int: return self._dimensions - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: """Embed a batch of texts using OpenAI. Runs the synchronous SDK call in a thread pool to avoid blocking the @@ -131,6 +131,13 @@ async def embed(self, texts: list[str]) -> list[list[float]]: Args: texts: Non-empty list of strings to embed. + kind: ``"query"`` or ``"document"``. Prepends + ``REPOWISE_EMBED_QUERY_PREFIX`` / ``REPOWISE_EMBED_DOC_PREFIX`` + (empty by default) to every text before sending — see + :func:`repowise.core.providers.embedding.base.resolve_embed_prefix`. + Inert against a model that doesn't need directional framing; + required for one (like Nemotron-3-Embed) that does and never + applies it server-side. Returns: List of L2-normalized float vectors. @@ -138,6 +145,10 @@ async def embed(self, texts: list[str]) -> list[list[float]]: if not texts: return [] + prefix = resolve_embed_prefix(kind) + if prefix: + texts = [f"{prefix}{t}" for t in texts] + model = self._model timeout = self._timeout request_dimensions = self._request_dimensions diff --git a/packages/core/src/repowise/core/providers/embedding/openrouter.py b/packages/core/src/repowise/core/providers/embedding/openrouter.py index b6caf5299..54a85156e 100644 --- a/packages/core/src/repowise/core/providers/embedding/openrouter.py +++ b/packages/core/src/repowise/core/providers/embedding/openrouter.py @@ -85,12 +85,18 @@ def __init__( def dimensions(self) -> int: return self._dimensions - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: """Embed a batch of texts using OpenRouter. Runs the synchronous SDK call in a thread pool to avoid blocking the asyncio event loop. + + Args: + texts: Non-empty list of strings to embed. + kind: Accepted for Embedder-protocol parity; unused. None of the + models in ``_DIMS`` are configured with a directional prefix. """ + del kind if not texts: return [] diff --git a/packages/server/src/repowise/server/mcp_server/_answer_pipeline.py b/packages/server/src/repowise/server/mcp_server/_answer_pipeline.py index 0bdf3bb82..ebe3184b0 100644 --- a/packages/server/src/repowise/server/mcp_server/_answer_pipeline.py +++ b/packages/server/src/repowise/server/mcp_server/_answer_pipeline.py @@ -263,7 +263,9 @@ async def question_vector(ctx: Any, question: str) -> list[float] | None: return cached[1] try: - vectors = await asyncio.wait_for(store.embed_texts([question]), timeout=_EMBED_TIMEOUT_S) + vectors = await asyncio.wait_for( + store.embed_texts([question], kind="query"), timeout=_EMBED_TIMEOUT_S + ) except TimeoutError: # The A18 case, and the one worth naming separately: the embedder is # configured, reachable and healthy, and simply did not answer inside diff --git a/packages/server/src/repowise/server/mcp_server/tool_why.py b/packages/server/src/repowise/server/mcp_server/tool_why.py index 4004766e5..037461974 100644 --- a/packages/server/src/repowise/server/mcp_server/tool_why.py +++ b/packages/server/src/repowise/server/mcp_server/tool_why.py @@ -699,7 +699,7 @@ async def _semantic_lanes(ctx: Any, query: str) -> tuple[list, list]: raw: list | None = None with contextlib.suppress(Exception): - vectors = await ctx.vector_store.embed_texts([query]) + vectors = await ctx.vector_store.embed_texts([query], kind="query") if vectors: raw = await ctx.vector_store.search_by_vector(vectors[0], limit=_SEMANTIC_WINDOW) if raw is None: diff --git a/tests/unit/cli/test_embedder_resolution.py b/tests/unit/cli/test_embedder_resolution.py index b3e7a0329..38e957176 100644 --- a/tests/unit/cli/test_embedder_resolution.py +++ b/tests/unit/cli/test_embedder_resolution.py @@ -38,7 +38,8 @@ class _WideEmbedder: dimensions = 1536 - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + del kind return [[0.0] * 1535 + [1.0] for _ in texts] @@ -447,7 +448,8 @@ def test_two_real_widths_are_never_compared( class _MisreportingEmbedder: dimensions = 768 # the hardcoded guess, not what it actually emits - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + del kind return [[0.0] * 1024 for _ in texts] monkeypatch.setattr( diff --git a/tests/unit/persistence/test_decision_semantic_dedup.py b/tests/unit/persistence/test_decision_semantic_dedup.py index e174144de..c66775e16 100644 --- a/tests/unit/persistence/test_decision_semantic_dedup.py +++ b/tests/unit/persistence/test_decision_semantic_dedup.py @@ -35,7 +35,8 @@ class _KeywordEmbedder: dimensions = 8 - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + del kind out: list[list[float]] = [] for t in texts: tl = t.lower() @@ -202,7 +203,8 @@ def __init__(self) -> None: self.calls = 0 self._axis_by_text: dict[str, int] = {} - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + del kind self.calls += 1 out: list[list[float]] = [] for t in texts: diff --git a/tests/unit/persistence/test_decision_supersession.py b/tests/unit/persistence/test_decision_supersession.py index c1ca7a94f..d34934d2b 100644 --- a/tests/unit/persistence/test_decision_supersession.py +++ b/tests/unit/persistence/test_decision_supersession.py @@ -47,7 +47,8 @@ class _TopicEmbedder: dimensions = 4 - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + del kind out: list[list[float]] = [] for t in texts: tl = t.lower() diff --git a/tests/unit/persistence/test_vector_store.py b/tests/unit/persistence/test_vector_store.py index c0e47f66d..3dffc0f64 100644 --- a/tests/unit/persistence/test_vector_store.py +++ b/tests/unit/persistence/test_vector_store.py @@ -362,7 +362,8 @@ class _FixedDimEmbedder: def __init__(self, dimensions: int) -> None: self.dimensions = dimensions - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + del kind out: list[list[float]] = [] for i, _ in enumerate(texts): vec = [0.0] * self.dimensions @@ -423,7 +424,8 @@ def __init__(self, fail_on_call: int | None = None) -> None: self.calls: list[list[str]] = [] self._fail_on_call = fail_on_call - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + del kind self.calls.append(list(texts)) if self._fail_on_call is not None and len(self.calls) == self._fail_on_call: raise RuntimeError("simulated 400 max_tokens_per_request") diff --git a/tests/unit/server/mcp/test_answer_decision_rows.py b/tests/unit/server/mcp/test_answer_decision_rows.py index 8516fd33b..96e571c29 100644 --- a/tests/unit/server/mcp/test_answer_decision_rows.py +++ b/tests/unit/server/mcp/test_answer_decision_rows.py @@ -74,14 +74,16 @@ def __init__(self, n_decisions: int, n_pages: int = 40) -> None: ] self.limits: list[int] = [] - async def embed_texts(self, texts: list[str]) -> list[list[float]]: + async def embed_texts(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + del kind return [[0.1, 0.2, 0.3] for _ in texts] async def search_by_vector(self, vector, limit: int = 10) -> list[SearchResult]: self.limits.append(limit) return self._rows[:limit] - async def search(self, query: str, limit: int = 10) -> list[SearchResult]: + async def search(self, query: str, limit: int = 10, *, kind: str = "query") -> list[SearchResult]: + del kind self.limits.append(limit) return self._rows[:limit] diff --git a/tests/unit/server/mcp/test_answer_question_embedding.py b/tests/unit/server/mcp/test_answer_question_embedding.py index 2057cfa0d..23257f1e9 100644 --- a/tests/unit/server/mcp/test_answer_question_embedding.py +++ b/tests/unit/server/mcp/test_answer_question_embedding.py @@ -37,9 +37,9 @@ def __init__(self) -> None: def dimensions(self) -> int: return self._inner.dimensions - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: self.batches.append(list(texts)) - return await self._inner.embed(texts) + return await self._inner.embed(texts, kind=kind) @pytest.fixture diff --git a/tests/unit/server/mcp/test_embedder_resolution.py b/tests/unit/server/mcp/test_embedder_resolution.py index 6732b9d34..4b3872651 100644 --- a/tests/unit/server/mcp/test_embedder_resolution.py +++ b/tests/unit/server/mcp/test_embedder_resolution.py @@ -139,7 +139,8 @@ def test_custom_registered_embedder_is_honoured(monkeypatch): class _FakeEmbedder: dimensions = 4 - async def embed(self, texts): + async def embed(self, texts, *, kind="document"): + del kind return [[0.0, 0.0, 0.0, 1.0] for _ in texts] register_embedder("fake-test-embedder", lambda **kw: _FakeEmbedder()) diff --git a/tests/unit/server/mcp/test_keyless_vector_leg.py b/tests/unit/server/mcp/test_keyless_vector_leg.py index b5d515a30..f2d7cb402 100644 --- a/tests/unit/server/mcp/test_keyless_vector_leg.py +++ b/tests/unit/server/mcp/test_keyless_vector_leg.py @@ -37,7 +37,8 @@ class _RealisticEmbedder: dimensions = 4 - async def embed(self, texts: list[str]) -> list[list[float]]: + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + del kind out = [] for text in texts: raw = [float(text.count(c)) for c in "abcd"] diff --git a/tests/unit/server/mcp/test_retrieval_leg_visibility.py b/tests/unit/server/mcp/test_retrieval_leg_visibility.py index 68bdc4667..dcb40f1ba 100644 --- a/tests/unit/server/mcp/test_retrieval_leg_visibility.py +++ b/tests/unit/server/mcp/test_retrieval_leg_visibility.py @@ -29,7 +29,8 @@ def __init__(self, *, embed_hangs=False, search_hangs=False): self._embed_hangs = embed_hangs self._search_hangs = search_hangs - async def embed_texts(self, texts): + async def embed_texts(self, texts, *, kind="document"): + del kind if self._embed_hangs: await asyncio.sleep(60) return [[0.1, 0.2, 0.3]] diff --git a/tests/unit/server/mcp/test_why_search_payload.py b/tests/unit/server/mcp/test_why_search_payload.py index ab836c860..5ef474386 100644 --- a/tests/unit/server/mcp/test_why_search_payload.py +++ b/tests/unit/server/mcp/test_why_search_payload.py @@ -170,11 +170,11 @@ async def test_search_embeds_the_query_once(session, setup_mcp, monkeypatch): original = vs.embed_texts - async def _counting(texts): + async def _counting(texts, *, kind="document"): calls.append("embed_texts") - return await original(texts) + return await original(texts, kind=kind) - async def _forbidden(query, limit=10): + async def _forbidden(query, limit=10, *, kind="query"): calls.append(f"search:{query}") return [] diff --git a/tests/unit/test_persistence/test_embed_kind_prefix.py b/tests/unit/test_persistence/test_embed_kind_prefix.py new file mode 100644 index 000000000..b5f00c4fe --- /dev/null +++ b/tests/unit/test_persistence/test_embed_kind_prefix.py @@ -0,0 +1,224 @@ +"""``kind="query"|"document"`` through the embedding stack. + +Sits alongside ``test_embedding_timeout_resolution.py`` for the same reason: +one cross-cutting contract — here, which side of retrieval a text is on — +that every embedder and every store must honour identically, so it belongs +in one place rather than duplicated per provider. + +The empty-by-default behaviour (``resolve_embed_prefix`` returns ``""`` for +both kinds until an operator opts in) is what makes threading ``kind`` +through ``embed()``/``embed_texts()``/``search()`` safe to ship: every +existing request stays byte-identical unless ``REPOWISE_EMBED_QUERY_PREFIX`` +/ ``REPOWISE_EMBED_DOC_PREFIX`` is set. +""" + +from __future__ import annotations + +import hashlib +import math + +import pytest + +from repowise.core.analysis.decisions.semantic_match import ( + decision_vector_item, + find_duplicate_decision, + find_related_decisions, + find_related_decisions_many, + upsert_decision_vectors, +) +from repowise.core.persistence.vector_store.in_memory import InMemoryVectorStore +from repowise.core.providers.embedding.base import MockEmbedder, resolve_embed_prefix + +# --------------------------------------------------------------------------- +# resolve_embed_prefix +# --------------------------------------------------------------------------- + + +def _clear(monkeypatch): + monkeypatch.delenv("REPOWISE_EMBED_QUERY_PREFIX", raising=False) + monkeypatch.delenv("REPOWISE_EMBED_DOC_PREFIX", raising=False) + + +def test_empty_by_default_for_both_kinds(monkeypatch): + _clear(monkeypatch) + assert resolve_embed_prefix("query") == "" + assert resolve_embed_prefix("document") == "" + + +def test_reads_the_matching_env_var(monkeypatch): + _clear(monkeypatch) + monkeypatch.setenv("REPOWISE_EMBED_QUERY_PREFIX", "query: ") + monkeypatch.setenv("REPOWISE_EMBED_DOC_PREFIX", "passage: ") + assert resolve_embed_prefix("query") == "query: " + assert resolve_embed_prefix("document") == "passage: " + + +def test_query_prefix_does_not_leak_into_document(monkeypatch): + _clear(monkeypatch) + monkeypatch.setenv("REPOWISE_EMBED_QUERY_PREFIX", "query: ") + assert resolve_embed_prefix("document") == "" + + +def test_invalid_kind_raises(): + with pytest.raises(ValueError, match="kind must be 'query' or 'document'"): + resolve_embed_prefix("passage") + + +# --------------------------------------------------------------------------- +# MockEmbedder — kind is accepted but never changes the vector +# --------------------------------------------------------------------------- + + +async def test_mock_embedder_ignores_kind(): + emb = MockEmbedder() + doc_vec = (await emb.embed(["hello"], kind="document"))[0] + query_vec = (await emb.embed(["hello"], kind="query"))[0] + default_vec = (await emb.embed(["hello"]))[0] + assert doc_vec == query_vec == default_vec + + +# --------------------------------------------------------------------------- +# VectorStore wiring — upsert stays "document", search moves to "query" +# --------------------------------------------------------------------------- + + +class _KindSpyEmbedder(MockEmbedder): + """Records the ``kind`` every ``embed()`` call was made with.""" + + def __init__(self) -> None: + self.calls: list[str] = [] + + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + self.calls.append(kind) + return await super().embed(texts, kind=kind) + + +async def test_upsert_embeds_as_document(): + spy = _KindSpyEmbedder() + store = InMemoryVectorStore(spy) + await store.embed_and_upsert("p1", "some content", {}) + assert spy.calls == ["document"] + + +async def test_search_embeds_the_query_as_query(): + spy = _KindSpyEmbedder() + store = InMemoryVectorStore(spy) + await store.embed_and_upsert("p1", "some content", {}) + spy.calls.clear() + + await store.search("some question", limit=5) + assert spy.calls == ["query"] + + +async def test_search_many_embeds_every_query_as_query(): + spy = _KindSpyEmbedder() + store = InMemoryVectorStore(spy) + await store.embed_and_upsert("p1", "some content", {}) + spy.calls.clear() + + await store.search_many(["q1", "q2"], limit=5) + assert spy.calls == ["query"] # one batched embedder call for both queries + + +async def test_search_accepts_an_explicit_document_kind(): + # The escape hatch decision dedup uses — see the tests below. + spy = _KindSpyEmbedder() + store = InMemoryVectorStore(spy) + await store.embed_and_upsert("p1", "some content", {}) + spy.calls.clear() + + await store.search("some content", limit=5, kind="document") + assert spy.calls == ["document"] + + +async def test_embed_texts_defaults_to_document(): + spy = _KindSpyEmbedder() + store = InMemoryVectorStore(spy) + await store.embed_texts(["some text"]) + assert spy.calls == ["document"] + + +async def test_embed_texts_forwards_an_explicit_query_kind(): + spy = _KindSpyEmbedder() + store = InMemoryVectorStore(spy) + await store.embed_texts(["some text"], kind="query") + assert spy.calls == ["query"] + + +# --------------------------------------------------------------------------- +# Decision dedup — the symmetric near-duplicate path stays on "document", +# even though it goes through the same VectorStore.search()/search_many() +# every asymmetric document-search caller uses. +# +# MockEmbedder can't prove this: it deliberately ignores kind, so a bug that +# silently reverted every call site below to the default "query" kind would +# not fail with it. This directional double makes kind change the vector — a +# strong, orthogonal shift, not a subtle one — so a kind mismatch is +# guaranteed to fall below DEFAULT_DEDUP_TAU (0.83) while a kind match on +# identical text is guaranteed to be an exact 1.0. +# --------------------------------------------------------------------------- + + +class _DirectionalEmbedder: + """Deterministic embedder whose vector depends on both *text* and *kind*. + + Vector = [10, 0, f1, f2] for "document", [0, 10, f1, f2] for "query", + L2-normalised. (f1, f2) is a small deterministic fingerprint of *text*. + The kind axes dominate (weight 10 vs. |f| <= 1), so two vectors for the + same text but different kind are nearly orthogonal (cosine ~0.01-0.02), + while two vectors for the same text and the same kind are identical. + """ + + dimensions: int = 4 + + async def embed(self, texts: list[str], *, kind: str = "document") -> list[list[float]]: + axis = 0 if kind == "document" else 1 + out: list[list[float]] = [] + for text in texts: + digest = hashlib.sha256(text.encode()).digest() + f1 = digest[0] / 127.5 - 1.0 + f2 = digest[1] / 127.5 - 1.0 + raw = [0.0, 0.0, f1, f2] + raw[axis] = 10.0 + norm = math.sqrt(sum(x * x for x in raw)) + out.append([x / norm for x in raw]) + return out + + +async def _seeded_store() -> InMemoryVectorStore: + store = InMemoryVectorStore(_DirectionalEmbedder()) + item = decision_vector_item("d1", title="Use Redis for session cache") + assert item is not None + await upsert_decision_vectors(store, [item]) + return store + + +async def test_find_duplicate_decision_matches_identical_text(): + store = await _seeded_store() + found = await find_duplicate_decision(store, title="Use Redis for session cache") + assert found == "d1" + + +async def test_find_duplicate_decision_would_miss_under_the_query_kind(): + # Proves the double actually discriminates kind, and pins the failure + # mode a regression to the pre-fix hardcoded kind="query" would hit. + store = await _seeded_store() + missed = await store.search("Use Redis for session cache", limit=50, kind="query") + assert missed == [] or missed[0].score < 0.83 + + +async def test_find_related_decisions_matches_identical_text(): + store = await _seeded_store() + related = await find_related_decisions( + store, title="Use Redis for session cache", lo=0.5, hi=1.01 + ) + assert ("d1", pytest.approx(1.0)) in [(rid, pytest.approx(score)) for rid, score in related] + + +async def test_find_related_decisions_many_matches_identical_text(): + store = await _seeded_store() + [related] = await find_related_decisions_many( + store, [("Use Redis for session cache", "", set())], lo=0.5, hi=1.01 + ) + assert related and related[0][0] == "d1" + assert related[0][1] == pytest.approx(1.0) diff --git a/tests/unit/test_persistence/test_openai_embedder.py b/tests/unit/test_persistence/test_openai_embedder.py index ab066965b..9ebce9fd1 100644 --- a/tests/unit/test_persistence/test_openai_embedder.py +++ b/tests/unit/test_persistence/test_openai_embedder.py @@ -188,6 +188,77 @@ def fake_create(**kwargs): assert captured[0]["input"] == ["test text"] +# --------------------------------------------------------------------------- +# kind="query"|"document" — REPOWISE_EMBED_QUERY_PREFIX / _DOC_PREFIX +# --------------------------------------------------------------------------- + + +async def _captured_input(emb: OpenAIEmbedder, *, kind: str) -> list[str]: + captured: dict = {} + + def fake_create(**kwargs): + captured.update(kwargs) + return _make_mock_response([[1.0] + [0.0] * (emb.dimensions - 1)]) + + with patch("openai.OpenAI") as mock_client: + mock_client.return_value.embeddings.create.side_effect = fake_create + await emb.embed(["hello"], kind=kind) + return captured["input"] + + +async def test_no_prefix_configured_leaves_the_request_byte_identical(monkeypatch): + # A fresh embedder per call: OpenAIEmbedder caches its SDK client on first + # use, so a second call on the same instance would talk to the first + # call's mock, not the one this call just configured. + monkeypatch.delenv("REPOWISE_EMBED_QUERY_PREFIX", raising=False) + monkeypatch.delenv("REPOWISE_EMBED_DOC_PREFIX", raising=False) + assert await _captured_input(OpenAIEmbedder(api_key="k"), kind="query") == ["hello"] + assert await _captured_input(OpenAIEmbedder(api_key="k"), kind="document") == ["hello"] + + +async def test_query_prefix_applied_only_to_query_kind(monkeypatch): + monkeypatch.setenv("REPOWISE_EMBED_QUERY_PREFIX", "query: ") + monkeypatch.delenv("REPOWISE_EMBED_DOC_PREFIX", raising=False) + assert await _captured_input(OpenAIEmbedder(api_key="k"), kind="query") == ["query: hello"] + assert await _captured_input(OpenAIEmbedder(api_key="k"), kind="document") == ["hello"] + + +async def test_document_prefix_applied_only_to_document_kind(monkeypatch): + monkeypatch.delenv("REPOWISE_EMBED_QUERY_PREFIX", raising=False) + monkeypatch.setenv("REPOWISE_EMBED_DOC_PREFIX", "passage: ") + assert await _captured_input(OpenAIEmbedder(api_key="k"), kind="document") == [ + "passage: hello" + ] + assert await _captured_input(OpenAIEmbedder(api_key="k"), kind="query") == ["hello"] + + +async def test_prefix_applies_to_every_text_in_the_batch(monkeypatch): + monkeypatch.setenv("REPOWISE_EMBED_QUERY_PREFIX", "query: ") + monkeypatch.delenv("REPOWISE_EMBED_DOC_PREFIX", raising=False) + emb = OpenAIEmbedder(api_key="k", dimensions=1) + captured: dict = {} + + def fake_create(**kwargs): + captured.update(kwargs) + return _make_mock_response([[1.0], [1.0]]) + + with patch("openai.OpenAI") as mock_client: + mock_client.return_value.embeddings.create.side_effect = fake_create + await emb.embed(["alpha", "beta"], kind="query") + + assert captured["input"] == ["query: alpha", "query: beta"] + + +async def test_default_kind_is_document(): + # embed()'s default kind matches the Embedder protocol's default, so a + # caller that doesn't think about retrieval direction gets the document + # framing — the safer default, since most existing callers upsert. + import inspect + + sig = inspect.signature(OpenAIEmbedder.embed) + assert sig.parameters["kind"].default == "document" + + # --------------------------------------------------------------------------- # Forwarding the width to the API # ---------------------------------------------------------------------------