fix(qdrant): route service-level vector repo lookups through env-aware factory#6
Conversation
…e factory PR #5 made the service start with VECTOR_STORE_BACKEND=qdrant by gating the Milvus lifespan and lazifying BaseMilvusRepository's collection property. But the service-layer write/read path was still hard-coded to the Milvus repositories: 5 service classes either called ``EpisodicMemoryMilvusRepository()`` directly or asked the DI container for ``get_bean_by_type(EpisodicMemoryMilvusRepository)``. With the Milvus collections never loaded, every insert / search through that path raised ``ValueError("Collection instance not created, please call ensure_loaded() first")`` — which the calling services swallowed as ``ERROR`` logs while returning success with ``indexed=0``. Net effect: data continued to land in MongoDB, but never reached Qdrant, and search returned empty results. This commit introduces ``core/oxm/vector_backend_router.py`` — a thin factory with one ``get_<memory>_repo()`` per memory type. Each factory reads ``VECTOR_STORE_BACKEND`` (case-insensitive, normalised) and resolves the appropriate Qdrant or Milvus repo via the DI container, preserving singleton bean scope. Direct instantiation is the fallback for unit-test contexts without a DI scan. Service-layer sites updated: - ``agentic_layer/search_mem_service.py`` — 4 hard-coded ``*MilvusRepository()`` direct instantiations now call the factory; field names keep their historical ``_milvus_`` token for backward compatibility with the rest of the service. - ``agentic_layer/memory_manager.py`` — the ``match mem_type`` switch that picked one of 5 ``MilvusRepository`` beans now routes to the factory. - ``agentic_layer/profile_search_service.py`` — the lazy ``milvus_repo`` property now resolves through ``get_user_profile_repo()``. - ``memory_layer/profile_indexer/profile_indexer.py`` — same lazy property, same factory call. - ``biz_layer/mem_sync.py`` — constructor defaults for ``foresight_milvus_repo`` and ``atomic_fact_milvus_repo`` now come from the factory. - ``biz_layer/mem_memorize.py`` — the inline ``agent_skill_milvus_repo`` lookup inside ``upsert_records`` sync now uses ``get_agent_skill_repo()``. Both backends already expose the same public surface (``vector_search``, ``create_and_save_*``, ``delete_by_*``), so callers need no further changes.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughDie Änderungen führen eine zentralisierte Backend-Router-Infrastruktur ein, die die Auswahl zwischen Qdrant- und Milvus-Vector-Store-Implementierungen basierend auf einer Umgebungsvariable ermöglicht. Sechs Service-Schichten werden aktualisiert, um diese Factory-basierte Auflösung anstelle direkter Dependency-Injection zu verwenden, während Feldnamen für Rückwärtskompatibilität beibehalten werden. ChangesVector Backend Router Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@methods/EverCore/src/core/oxm/vector_backend_router.py`:
- Around line 34-38: Die Funktion _backend() normalisiert zwar den ENV-Wert,
validiert aber nicht, ob er "qdrant" oder "milvus" ist; aktualisiere _backend()
so dass nach dem Lesen und Normalisieren des ENV-Werts eine Überprüfung gegen
ein erlaubtes Set = {"qdrant","milvus"} erfolgt und bei ungültigem Wert eine
klare Ausnahme (z.B. ValueError) oder ein aussagekräftiger Fehler-Log mit dem
fehlerhaften Wert geworfen/geloggt wird, statt stillschweigend auf "milvus"
zurückzufallen; referenziere dabei die Funktion _backend() und die
Umgebungsvariable VECTOR_STORE_BACKEND und stelle sicher, dass die Fehlermeldung
den ungültigen Wert enthält.
- Line 31: Replace the deprecated typing.Type import and uses with the built-in
type: change the import line "from typing import Any, Type" to "from typing
import Any" (or drop typing import entirely if unused) and update any
annotations that use Type[...] to use the built-in "type[...]"/"type" (e.g.,
change "Type[Foo]" to "type[Foo]" or just "type" where appropriate) — look for
usages in this module such as Type in function signatures or variable
annotations and update them accordingly.
- Around line 45-57: In _resolve, narrow and log DI failures instead of silently
swallowing them: import BeanNotFoundError from core.di.exceptions, call
get_bean_by_type(cls) inside a try that catches ImportError and
BeanNotFoundError (not a bare Exception), log the exception details (include the
cls and the exception) before falling back to returning cls(), so failures in
get_bean_by_type are visible in logs.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 431cb57b-9ea9-4999-adc2-2161697ccfdb
📒 Files selected for processing (7)
methods/EverCore/src/agentic_layer/memory_manager.pymethods/EverCore/src/agentic_layer/profile_search_service.pymethods/EverCore/src/agentic_layer/search_mem_service.pymethods/EverCore/src/biz_layer/mem_memorize.pymethods/EverCore/src/biz_layer/mem_sync.pymethods/EverCore/src/core/oxm/vector_backend_router.pymethods/EverCore/src/memory_layer/profile_indexer/profile_indexer.py
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f57f43ca3e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Addresses 5 of the 6 PR-6 review findings (Codex P1×3 + CodeRabbit
minor×2). The remaining one (CodeRabbit analysis-only on get_bean_by_type
exception scope) requires no code change — the fallback in ``_resolve``
already converts the missing-bean case into direct instantiation, so the
broad ``except Exception`` is intentional.
P1 fixes — Milvus-compat write API on Qdrant repositories
Codex correctly identified that the previous factory swap could route
``ProfileIndexer.insert_batch(...)``, ``sync_foresight/atomic_fact.insert(...)``
and ``mem_memorize.agent_skill insert/delete`` calls to Qdrant repos that
never had those methods. The Qdrant repos exposed only
``create_and_save_*`` + native ``upsert(PointStruct)`` — different
signatures, different entity shape.
Instead of rewriting every service-layer call (large diff, easy to miss
sites), this commit adds a thin compatibility layer to
``BaseQdrantRepository`` so all six concrete Qdrant repos inherit it:
- ``insert(entity_dict, flush=False) -> str`` — translates a
Milvus-converter output (``{"id", "vector", payload-fields…}``) into
a ``PointStruct`` via the same ``mongo_id_to_qdrant_id`` mapping the
sweep uses, upserts, and returns the resulting Qdrant point id. The
raw Mongo id is preserved as ``payload.mongo_id`` for round-trip.
- ``insert_batch(entity_dicts, flush=False) -> UpdateResult`` — batch
version. Entities without a vector or id are silently skipped (the
source converter is supposed to guard, but the pipeline sometimes
hands partial docs in early-lifecycle states).
- The ``flush`` parameter is accepted and ignored — Qdrant has no
separate flush; ``upsert(wait=True)`` already covers durability.
This keeps the existing service-layer ``MilvusConverter.from_mongo()``
→ ``milvus_repo.insert()`` pipeline intact while transparently routing
to Qdrant when the env flag is set.
CodeRabbit minors
- ``typing.Type`` → builtin ``type`` (Python 3.9+ idiom, ruff UP035).
- ``VECTOR_STORE_BACKEND`` validation: an unrecognised value now logs a
warning and falls back to ``milvus`` instead of silently routing
there. Catches typos like ``VECTOR_STORE_BACKEND=qdarnt``.
|
@coderabbitai review All 5 findings addressed in 035444c:
The remaining CR analysis (get_bean_by_type exception scope) needs no change: the broad except in _resolve is intentional because we want to fall through to direct instantiation in unit-test contexts that have no DI scan. |
|
Triggering a fresh review now. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 035444c355
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| self.agent_case_milvus_repo = get_agent_case_repo() | ||
| self.agent_skill_milvus_repo = get_agent_skill_repo() |
There was a problem hiding this comment.
After SearchMemoryService starts resolving agent_case_milvus_repo/agent_skill_milvus_repo via the backend router, Qdrant mode returns point UUIDs in id, but this service still treats id as a Mongo _id for backfill (_fetch_agent_cases_by_ids / _fetch_agent_skills_by_ids) and for hybrid fusion dedupe. In Qdrant deployments this causes vector/hybrid agent-case and agent-skill results to be dropped because Mongo lookups miss, even though vector hits were found.
Useful? React with 👍 / 👎.
| } | ||
| payload["mongo_id"] = str(mongo_id) | ||
| return qmodels.PointStruct( | ||
| id=mongo_id_to_qdrant_id(mongo_id), |
There was a problem hiding this comment.
The new Milvus-compat insert adapter rewrites Mongo IDs to UUID5 point IDs, but delete call sites in routed services still pass raw Mongo IDs into delete_by_id. That mismatch means update/delete sync flows can fail or silently miss deletions in Qdrant mode; in mem_memorize this is especially harmful because delete and insert are inside one try, so a delete failure skips the subsequent upserts and leaves vectors stale.
Useful? React with 👍 / 👎.
| case MemoryType.EPISODIC_MEMORY: | ||
| milvus_repo = get_bean_by_type(EpisodicMemoryMilvusRepository) | ||
| milvus_repo = get_episodic_repo() | ||
| case MemoryType.AGENT_CASE: | ||
| milvus_repo = get_bean_by_type(AgentCaseMilvusRepository) | ||
| milvus_repo = get_agent_case_repo() | ||
| case MemoryType.AGENT_SKILL: |
There was a problem hiding this comment.
This routing change also affects MemoryManager hybrid retrieval: _search_hybrid deduplicates keyword/vector hits by id, but keyword hits use ES document IDs while Qdrant vector hits now carry point UUIDs, so the same memory is no longer recognized as duplicate. In Qdrant mode this inflates duplicate candidates before rerank and can crowd out distinct results in the final top-k.
Useful? React with 👍 / 👎.
| self.episodic_milvus_repo = get_episodic_repo() | ||
| self.profile_milvus_repo = get_user_profile_repo() |
There was a problem hiding this comment.
By routing episodic_milvus_repo through get_episodic_repo(), Qdrant mode now feeds search_mem_service results where timestamp is already a datetime, but _search_episodic_memories only handles numeric timestamps and otherwise sets timestamp=None. This makes vector episodic results lose their timestamp field in Qdrant deployments, which breaks time display/sorting behavior even when the underlying hit is valid.
Useful? React with 👍 / 👎.
1 participant
Problem
PR #5 enabled the service to start with
VECTOR_STORE_BACKEND=qdrantby gating the Milvus lifespan and lazifying
BaseMilvusRepository'scollection property. But the service-layer write/read path was still
hard-coded to the Milvus repositories: 5 service classes either called
EpisodicMemoryMilvusRepository()directly or asked the DI containerfor
get_bean_by_type(EpisodicMemoryMilvusRepository).With the Milvus collections never loaded, every insert / search through
that path raised
ValueError("Collection instance not created, please call ensure_loaded() first")— which the calling services swallowed as
ERRORlogs while returningindexed=0and treating the operation as successful.Net effect on a live cutover deployment: data continued to land in
MongoDB (the source of truth), but never reached Qdrant. Search through
the service layer returned empty results because the same broken path
was used for reads.
Concretely measured on the production cutover host:
v1_episodic_memories(Mongo) were not presentin the corresponding Qdrant collection (108 docs missing after ~12 h).
profile_indexerloggedProfile Milvus indexing completed: indexed=0on every save with a
UserProfileCollection Collection instance not createdtraceback right next to it.
Fix
Introduce
core/oxm/vector_backend_router.py— a thin factory with oneget_<memory>_repo()per memory type. Each factory readsVECTOR_STORE_BACKEND(case-insensitive, normalised) and resolves theappropriate Qdrant or Milvus repo via the DI container, preserving
singleton bean scope. Direct instantiation is the fallback for unit-test
contexts without a DI scan.
Service-layer sites updated:
agentic_layer/search_mem_service.py— 4 hard-coded*MilvusRepository()direct instantiations call the factory now;field names keep their historical
_milvus_token for calleecompatibility with the rest of the service.
agentic_layer/memory_manager.py— thematch mem_typeswitchthat picked one of 5
MilvusRepositorybeans now routes to thefactory.
agentic_layer/profile_search_service.py— the lazymilvus_repoproperty resolves through
get_user_profile_repo().memory_layer/profile_indexer/profile_indexer.py— same lazyproperty, same factory call.
biz_layer/mem_sync.py— constructor defaults forforesight_milvus_repoandatomic_fact_milvus_repocome fromthe factory.
biz_layer/mem_memorize.py— the inlineagent_skill_milvus_repolookup inside the
upsert_recordssync usesget_agent_skill_repo().Both backends expose the same public surface (
vector_search,create_and_save_*,delete_by_*), so callers need no furtherchanges. Field / variable names keep their
_milvus_token — thatkeeps the diff minimal and avoids a rename cascade through the
existing methods; renaming is a follow-up.
Test plan
VECTOR_STORE_BACKEND=qdrant(alreadyvalidated by PR fix(qdrant): allow service to boot with VECTOR_STORE_BACKEND=qdrant #5) and now also writes to Qdrant on memory ingest.
profile_indexerindexed=N>0for a new profile.were written to Mongo between cutover and this fix (idempotent via
uuid5 mapping, no double-embedding cost).