Retrieval fixes behind default-off flags: vector type tagging, input types, Cohere rerank, score ordering - #12
Open
StockerMC wants to merge 12 commits into
Open
Retrieval fixes behind default-off flags: vector type tagging, input types, Cohere rerank, score ordering#12StockerMC wants to merge 12 commits into
StockerMC wants to merge 12 commits into
Conversation
Gates the retrieval changes that follow so merging them changes nothing until a flag is set: RETRIEVAL_RERANK_ENABLED for the serving path and RETRIEVAL_DOC_INPUT_TYPE_ENABLED for corpus embedding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hAogJYPShanbHuEWTYXyo
Products and creator videos share one Pinecone index and namespace, but the path that actually indexes videos (background_worker) never wrote the type key the API filters on, so that filter matched nothing. Writing it at index time makes video vectors identifiable; is_creator_video_match also accepts vectors indexed before the key existed, which are identified by video_id. embed-english-v3.0 is asymmetric: corpus content belongs in search_document, live queries in search_query. Corpus writers now go through document_to_embedding, which stays on search_query until RETRIEVAL_DOC_INPUT_TYPE_ENABLED is set, because the existing index was built with search_query and mixing the two degrades retrieval until a full re-embed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hAogJYPShanbHuEWTYXyo
Pinecone similarity ordering was never honoured downstream: the Next.js route concatenated pre-computed matches with vector matches and returned them in created_at order, so retrieval quality could not reach the UI. Behind RETRIEVAL_RERANK_ENABLED, both serving paths now retrieve a wider candidate pool, score pre-computed and vector candidates in one Cohere rerank pass so their scores are comparable, and return the top N in that order. If rerank errors or COHERE_KEY is missing, the request falls back to similarity order instead of failing. The route also emits a matches key with a nested creator_videos when the flag is on: that is the shape the reels page reads, and it never matched what this route returned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hAogJYPShanbHuEWTYXyo
Cohere and Pinecone are mocked, so nothing talks to a live service. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hAogJYPShanbHuEWTYXyo
Prints a product's candidates in similarity order next to rerank order with scores, so the ordering change is visible before enabling the flag. Calls the rerank stage directly, so neither flag needs to be set to run it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hAogJYPShanbHuEWTYXyo
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…failures _client() built a fresh cohere.ClientV2 per call and never closed it; cohere 5.20 gives each instance its own httpx.Client, so every request leaked a connection pool. Cache it against the key it was built from so a rotated or removed COHERE_KEY still takes effect, and expose reset_client_cache() for tests. Client construction also sat outside the try, so an ImportError or a bad CO_API_URL propagated to a 500 instead of returning None and falling back to similarity order, which is what the module docstring promises.
…the fallback match_score is not a column on product_creator_matches, it belongs to the legacy product_matches table, so both fallback sorts evaluated every key to 0 and left rows in created_at DESC order. Sort pre-computed rows on relevance_score (the field background_worker.py writes), falling back to similarity_score, and vector rows on their Pinecone score. The successful rerank path was writing Cohere's 0-1 score over the stored 0-10 relevance_score, so a row scored 8.5 by the keyword scorer came back as 0.31 and failed the min_score=4.0 comparison in utils/relevance.py. Cohere's score now goes to rerank_score and relevance_score is left alone; both endpoints agree on what each key means. The fallback also truncated to min(limit, top_n) and concatenated pre-computed ahead of vector candidates, so a rerank failure with 12 rows and ?limit=50 returned 10 matches and an empty vector list, worse than the flag-off response. The fallback now respects the caller's limit and returns both lists whole; top_n truncation applies only when rerank succeeds. Feeding that path, search_text was initialised to "" and only assigned inside the pinecone_id branch, so an unindexed product always reached rerank with an empty query and always took the truncating fallback. The rerank query is now built from whatever product fields exist, separately from the text vector search embeds, which is unchanged. Ranking moves to utils/creator_ranking.py and lib/creatorRanking.ts so it can be tested without importing API.py, which loads real credentials at import time.
…flag RETRIEVAL_RERANK_ENABLED was carrying two unrelated changes on the frontend. The Next.js route has only ever returned `creators`, while /dashboard/reels reads `result.matches`, so the product-scoped reels view rendered nothing. Emitting `matches` behind the rerank flag meant flipping it turned the view on from empty rather than reordering it, and the flag's documented effect was wrong. The `matches` array now sits behind CREATORS_API_EMIT_MATCHES, which changes the response shape and nothing else. RETRIEVAL_RERANK_ENABLED is ordering only and the two are independent: the UI fix can ship without the ranking change, and either can be reverted alone. Emitting `matches` unconditionally would have been the smaller diff but changes a live response with every flag off. Rows whose creator_videos embed missed are dropped from `matches` instead of being emitted as null, which the reels page then dereferenced; that page also guards the field now. Both flags off leaves the response as it is today: same keys, same order, same count.
…econe call The vectordb fixture imported utils.vectordb inside a patch context, which only patches anything when the module is not already in sys.modules. Running test_job_queue.py first left a real client on the module and the two TestCorpusInputType tests that touch vectordb.co failed. Reload inside the patch context so the module-level clients are rebuilt against the mocks either way. That real client was itself the bigger problem: sync_shopify_products_job imports utils.vectordb from inside its body, and pc.Index(name) resolves the index host through a live call to api.pinecone.io with whatever key .env holds. conftest now stubs both client constructors for the session and pins dummy keys, so the run cannot reach Pinecone or Cohere no matter which files are selected. Verified: test_job_queue.py + test_retrieval.py together, in both orders, 46 passed; test_retrieval.py alone, 31 passed.
…semantics The existing tests passed against a bare MagicMock, so renaming documents to docs or query to q would not have failed anything, and rerank_creator_candidates had no coverage at all. The Cohere fake is now autospecced from the real cohere.ClientV2, so a wrong call shape raises TypeError, and the kwargs are asserted key by key. One test pins that ClientV2.rerank still resolves to V2Client.rerank. New coverage: the fallback keeping every row and the vector list, the caller's limit applying instead of top_n, ordering on relevance_score rather than the legacy match_score, relevance_score surviving while rerank_score is added, the client cache being reused, rebuilt on a key change, skipped when the key is missing, and not defeating client injection, and both flags defaulting off and staying independent. Frontend tests run offline under node --test against the extracted pure module (npm test); no new dependencies. tests/ is excluded from tsconfig because the explicit .ts import extensions node needs are not valid for the app build. Verified each new assertion bites by mutating the source: renaming a Cohere kwarg, writing the Cohere score to relevance_score, truncating the fallback to top_n, sorting on match_score, and emitting rows with a null video each fail. Also containment: a raising rerank inside the ranking stage now degrades to similarity order instead of propagating.
Both were unpinned. ClientV2.rerank resolves to V2Client.rerank through the ClientV2(V2Client, Client) MRO; the v1 method takes Sequence[RerankRequestDocumentsItem] and max_chunks_per_doc, so a major bump could silently change which method the rerank call lands on.
The script had its own copy claiming to match the serving path. Import it so it cannot drift.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merging this is a no-op until a flag is set
Every behaviour change is behind a feature flag that defaults to off. With no new
environment variables set, the deployed system serves the same responses, in the
same order, from the same candidate pool as it does today. The flags exist so the
new path can be turned on locally and evaluated first.
RETRIEVAL_RERANK_ENABLEDCREATORS_API_EMIT_MATCHESmatchesarray (entries carrying a nestedcreator_videos) that the reels view reads. No reordering.RETRIEVAL_DOC_INPUT_TYPE_ENABLEDinput_type=search_documentCOHERE_RERANK_MODELrerank-v3.5RETRIEVAL_CANDIDATE_POOL30RETRIEVAL_TOP_N10Flag helpers:
backend/utils/feature_flags.py,frontend/src/lib/featureFlags.ts.Documented in the README under Retrieval feature flags.
What changed
1. Dead candidate filter.
backend/API.pyfiltered vector matches onmetadata["type"] == "creator_video", butbackend/background_worker.py— theonly path that actually indexes videos — never wrote that key. Only
generate_embeddings_job(backend/jobs/tasks.py:189) did, and nothing enqueuesit, so
vector_matcheswas always empty.backend/background_worker.py:432now writes"type": "creator_video", andbackend/utils/vectordb.py:120writes"type": "product". Products and videosshare one index and namespace, so the key is the reliable discriminator.
is_creator_video_match()(backend/utils/vectordb.py,isCreatorVideoMatch()infrontend/src/lib/vectordb.ts) treats a vector as acreator video when
type == "creator_video", or whentypeis absent andvideo_idis present.the flag off,
backend/API.pykeeps the original strict filter, so the endpointkeeps returning an empty
vector_matchesexactly as it does now.No backfill is required. The
video_idfallback covers every vector indexedbefore the key existed.
2. Embedding input types.
text_to_embeddingusedinput_type="search_query"for everything. embed-english-v3.0 is asymmetric. Corpus writers
(
embed_products,background_worker.py:415,jobs/tasks.py:180) now calldocument_to_embedding(); live query paths are unchanged.3. Cohere Rerank stage.
backend/utils/rerank.pyandfrontend/src/lib/rerank.ts. RetrievesRETRIEVAL_CANDIDATE_POOL(30) candidatesand keeps
RETRIEVAL_TOP_N(10). Every failure mode — missing key, clientconstruction, API error, empty result, out-of-range index — returns null so the
caller keeps similarity order instead of failing the request. The Cohere client is
built once and cached against the key it was built from, not per request.
4. Serving path and ordering. Ranking lives in
backend/utils/creator_ranking.pyand
frontend/src/lib/creatorRanking.ts, imported by both endpoints. Pre-computedmatches (scored 0-10 by the keyword scorer in
backend/utils/relevance.py) andvector matches (Pinecone cosine similarity) are deduplicated by
video_idandscored in one rerank pass. Their native scores are on unrelated scales and
cannot be merged directly; one pass puts every candidate on the same 0-1 scale.
The Cohere score is written to
rerank_score. The storedrelevance_scoreisnever overwritten, because
utils/relevance.pycompares it against a 0-10threshold. Both endpoints agree on what each key means.
If rerank is unavailable, the fallback is pre-computed sorted by
relevance_score(falling back to
similarity_score), then vector sorted by Pinecone score. Thecaller's
limitapplies on that path and nothing is dropped; truncation toRETRIEVAL_TOP_Nhappens only when rerank succeeds.5.
backend/scripts/eval_rerank.py. Prints a product's top N in similarityorder next to rerank order with scores, and how many positions moved. It calls the
rerank stage directly, so neither flag needs to be set. Read-only. I did not run it.
Review fixes in this revision
The rerank flag was not an ordering flag on the frontend.
reels/page.tsx:116reads
result.matches, which the Next.js route has never returned, so theproduct-scoped reels view renders empty today. Emitting
matchesunderRETRIEVAL_RERANK_ENABLEDmeant flipping that flag turned the view on from emptyrather than reordering it, and the flag's documented effect was wrong.
Resolved by splitting it:
matchesnow sits behind its ownCREATORS_API_EMIT_MATCHES, which changes the response shape and nothing else.RETRIEVAL_RERANK_ENABLEDis ordering only. The two are independent — the UI fixcan ship without the ranking change, either can be reverted alone, and neither
flag's name overstates what it does. Emitting
matchesunconditionally would havebeen the smaller diff, but that changes a live response with every flag off, which
is the one thing this PR promises not to do.
Rows whose
creator_videosembed missed are dropped frommatchesrather thanemitted as
null, which the reels page then dereferenced; that page guards thefield as well.
match_scoreis not a column. Both fallback sorts usedmatch_score, which belongs to the legacyproduct_matchestable — never presenton
product_creator_matchesrows, so every key evaluated to 0 and the "fallback tosimilarity order" was a stable no-op preserving
created_at DESC. Both now sort onrelevance_score/similarity_score, the fieldsbackground_worker.pyactuallywrites.
The rerank path clobbered
relevance_score.{**row, "relevance_score": score}overwrote the stored 0-10 keyword score with Cohere's 0-1 score, so a row scored 8.5
came back as 0.31 and failed the
min_score = 4.0comparison inrelevance.py:97.Cohere's score goes to
rerank_scorenow.The failure path silently dropped results.
ordered[:keep]withkeep = min(limit, top_n)meant a rerank failure with 12 pre-computed rows and?limit=50returned 10 matches and an emptiedvector_matches— worse than theflag-off response. The fallback now respects the caller's limit and returns both
lists whole.
Feeding that path,
search_textwas initialised to""and only assigned insidethe
pinecone_idbranch, so a product that was never indexed always reachedrerank_documents("", ...), which short-circuits to None and took the truncatingfallback every time. The rerank query is now built from whatever product fields
exist, separately from the text vector search embeds (which is unchanged).
A fresh
cohere.ClientV2per request, never closed. cohere 5.20 gives eachinstance its own
httpx.Client, so every request leaked a connection pool. Theclient is cached at module level, keyed on the API key so rotation and removal
still take effect, and construction moved inside the
try— anImportErroror abad
CO_API_URLnow returns None and falls back instead of raising a 500, which iswhat the module docstring always claimed.
reset_client_cache()exists for tests;an injected client still bypasses the cache entirely.
Order-dependent test fixture. The
vectordbfixture importedutils.vectordbinside a
patch()context, which only patches when the module is not already insys.modules. It now reloads inside that context.That stale client turned out to be the smaller half of the problem:
sync_shopify_products_jobimportsutils.vectordbfrom inside its body, andpc.Index(name)resolves the index host through a live call to api.pinecone.iowith whatever key
.envholds.conftest.pynow stubs both client constructors forthe session and pins dummy keys, so a test run cannot reach Pinecone or Cohere
regardless of which files are selected.
Tests that can actually fail. The Cohere fake was a bare
MagicMock, sorenaming
documents=todocs=would still have passed, andrerank_creator_candidateshad no coverage at all. The fake is now autospecced fromthe real
cohere.ClientV2, so a wrong call shape raisesTypeError, and the kwargsare asserted key by key. One test pins that
ClientV2.rerankstill resolves toV2Client.rerankthrough theClientV2(V2Client, Client)MRO —requirements.txtpins
cohere>=5.18,<6andpinecone>=8,<9for the same reason, since the v1reranksignature is incompatible.Each new assertion was checked by mutating the source: renaming a Cohere kwarg,
writing the Cohere score to
relevance_score, truncating the fallback totop_n,sorting on
match_score, and emitting rows with a null video all fail the suite.Ranking moved out of
API.pyso it could be tested at all — importingAPI.pyruns
load_dotenv()against real credentials and constructs live clients.Behaviour when the flags are on
RETRIEVAL_RERANK_ENABLED: on a successful rerank the served list is cappedat
min(limit, RETRIEVAL_TOP_N). The UI sends nolimit, so it goes from up to50 date-ordered rows to 10 rerank-ordered ones. Raise
RETRIEVAL_TOP_Nif thatis too aggressive. On failure the list is not capped.
CREATORS_API_EMIT_MATCHES: adds one key. The reels view starts rendering; orderis whatever the rerank flag decides.
Verified vs unverified
Verified by static checks and offline tests only:
python3 -m pytest tests/test_retrieval.py— 68 passed (was 31).python3 -m pytest tests/test_job_queue.py tests/test_retrieval.py— 46 passed,in both file orders. Previously 2 failed.
python3 -m pytest tests/test_retrieval.py tests/test_job_queue.py tests/test_metrics.py tests/test_redis_client.py tests/test_shopify_oauth.py—144 passed. (
test_real_search.py,test_youtube.py,test_discovery.py,test_reels_fetch.pyandtest_improved_discovery.pycall live APIs and weredeliberately not run.)
npm testinfrontend/— 20 passed. Offline,node --test, no newdependencies.
tsc --noEmitandeslintclean on the changed frontend files.cohere==5.20.0ClientV2.rerank(*, model, query, documents, top_n)resolving toV2Client, andcohere-ai@7.20.0rerank({model, query, documents, topN}).Flag-off is still a no-op. The backend keeps the exact
search_textexpression ithad, and
rank_creator_candidatesis only reached insideif rerank_enabled. Thefrontend returns the same keys in the same order with the same values. One
disclosed exception: pre-computed entries in the
creatorsarray now also carryrelevance_scoreandsimilarity_score, two real columns that were being read forsorting anyway. Nothing consumes
creators— the only caller of this route isreels/page.tsx, which readsmatches.Unverified — nothing was executed against Cohere, Pinecone, Supabase, YouTube, or
Gemini, and the backend and worker were never started:
the installed SDK type definitions, not from a response.
rerank-v3.5is not confirmed to be enabled on this Cohere account.produces better matches than the current ordering is exactly what
eval_rerank.pyis there to answer.has not been exercised against real data.
Ambiguity flagged rather than guessed
preserves the keyword scorer's signal is a defensible alternative.
RETRIEVAL_TOP_N=10shrinks the served list on the success path. Reasonable for aswipe feed, worth a second opinion.
CREATORS_API_EMIT_MATCHESis a temporary flag. Once the reels view is confirmedworking in production it should be deleted and
matchesemitted unconditionally,or the page corrected to read
creators/videoand the key dropped entirely.frontend/.env.examplegainedPINECONE_API_KEY,PINECONE_INDEX_NAME, andCOHERE_API_KEY. The route already required them; they were simply undocumented.The transcript pipeline is untouched.