feat(embedding): apply asymmetric query/document prefixes to embed calls - #1858
feat(embedding): apply asymmetric query/document prefixes to embed calls#1858NullSense wants to merge 2 commits into
Conversation
Indexing and querying call the same embed() the same way, so a provider has no way to tell which side of retrieval a text is on. Models trained with asymmetric prefixes (E5, BGE, Nomic, NVIDIA Nemotron) are meant to see "query: " on the search side and "passage: " (or similar) on the stored side; repowise sends both bare. Measured A/B against a live corpus (Nemotron-3-Embed-1B, ~1,200 wiki pages, 11 queries): hit@5 went 3/11 -> 7/11, MRR 0.236 -> 0.332. Adds kind: str = "query" | "document" through Embedder.embed(), VectorStore.embed_texts()/search()/search_many(), and every provider and store that implements them, plus resolve_embed_prefix() reading REPOWISE_EMBED_QUERY_PREFIX / REPOWISE_EMBED_DOC_PREFIX (empty by default, so an unconfigured deployment's requests stay byte-identical). Decision near-duplicate matching (find_duplicate_decision and its two siblings) compares decision text to other decision text and needs both sides on the same prefix -- its own docstring already says so. Those three call sites now pass kind="document" explicitly rather than inheriting search()'s new "query" default, which would have silently broken dedup for exactly the kind of asymmetric-prefix deployment this PR is meant to support.
The full unit suite surfaced test doubles across tests/unit/cli, tests/unit/persistence, and tests/unit/server/mcp whose embed() or embed_texts() signature didn't accept the new kind parameter. Two classes of breakage: a bare TypeError swallowed by a broad except Exception (crud/decisions.py's bulk_upsert_decisions caught it and silently fell back to per-decision embedding -- 48 embedder round trips for 48 decisions instead of one batch, and semantic dedup/ supersession stopped merging paraphrases entirely), and a direct TypeError surfaced through lancedb_store.search(). Fixed by accepting kind (unused, deleted) in each double. No production code changed in this commit.
|
✅ Health of changed files: 5.5 → 5.6 (+0.1) 📋 At a glance Files & modules (2)
✅ Health gate: passed 📌 Before you merge
🎯 Blast radius (symbols whose signature this PR changed, and who calls them)
🔎 More signals (3)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (2 modules)"]
m_packages["packages (13 files)"]:::changed
m_tests["tests (1 file)"]:::changed
end
d_packages["packages"]
m_tests -->|2 files| d_packages
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (36 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (5)
2 more
💀 Dead code (1 finding)
👀 Suggested reviewers @RaghavChamadiya 📊 See the full report for this PR |
Ayush7614
left a comment
Ayush7614
left a comment
There was a problem hiding this comment.
Verified subset: 107 passed (test_openai_embedder, test_answer_question_embedding, test_why_search_payload, test_keyless_vector_leg, test_retrieval_leg_visibility, test_embed_kind_prefix).\n\nChange is valid and working: adds kind='query'|'document' through Embedder.embed() → VectorStore.embed_texts/search/search_many → all providers, with resolve_embed_prefix() reading REPOWISE_EMBED_QUERY_PREFIX/REPOWISE_EMBED_DOC_PREFIX (empty by default → byte-identical). Decision dedup call sites correctly pass kind='document' to keep symmetric embedding. Blast-radius flagged (25 files, 8 dirs) is expected for a protocol-signature change — no new issues spotted, handling of empty text and invalid kind looks correct. LGTM. Note: full tests/unit not re-verified in CI yet due to prior test-double fixes, recommend confirming full suite green before merge.
2 participants
Summary
embed()method, called identically for indexing and for querying — nothing says which side a text is on, and no provider prefixes anything. Addskind: str = "query" | "document"throughEmbedder.embed(),VectorStore.embed_texts()/search()/search_many(), and every store/provider that calls them, plusresolve_embed_prefix()readingREPOWISE_EMBED_QUERY_PREFIX/REPOWISE_EMBED_DOC_PREFIX(empty by default, so existing deployments are byte-identical).query:/passage:prefixes; hit@10 4/11 → 7/11; MRR 0.236 → 0.332.find_duplicate_decision,find_related_decisions,find_related_decisions_many) now passkind="document"explicitly, so they keep embedding symmetrically once an asymmetric prefix is configured — left alone, they'd have silently picked up the new defaultkind="query"through the sameVectorStore.search()real document search uses, breaking near-duplicate matching.Related Issues
None.
Test Plan
uv run pytest tests/unit/test_persistence/test_openai_embedder.py tests/unit/server/mcp/test_answer_question_embedding.py tests/unit/server/mcp/test_why_search_payload.py tests/unit/server/mcp/test_keyless_vector_leg.py tests/unit/server/mcp/test_retrieval_leg_visibility.py -q— 92 passed (every file touched to fix akind-signature break, plus the new prefix tests)uv run ruff check .uv run pytest tests/unit/ -q(full suite) — not verified. A second commit on this branch (test: accept kind= in every embed()/embed_texts() test double) fixesembed()/embed_texts()test doubles intests/unit/cli,tests/unit/persistence, andtests/unit/server/mcpwhose signature didn't accept the newkindparameter, per that commit's own message; I have not independently re-run the full suite to confirm it's green afterward, so I'm not claiming that result here. No production code changes in that second commit.uv run repowise risk main..HEAD:riskflags this as elevated on diff shape (25 files, 8 dirs) — expected for a protocol-signature change that touches every caller, and consistent with the full-suite verification gap noted above rather than a new finding.(
impacted-tests/health --fileneed a completedrepowise initagainst this clone; that indexing run did not finish in time and is omitted rather than faked.)The wrinkle: decision dedup
find_duplicate_decision/find_related_decisions/find_related_decisions_manycompare decision text to other decision text, and their own docstring already states the invariant this depends on: "the stored vector and the query side always embed the same shape." Decision vectors are stored viaupsert_decision_vectors, which callsembed_texts()at its"document"default, so the dedup lookup has to search at"document"too — but those three functions look up existing decisions through the same genericVectorStore.search()/search_many()real document search uses, whose default kind is"query". All three call sites now passkind="document"explicitly.Checklist
Written with AI assistance; measurements and tests were run and verified locally.