fix(everos): voyage rerank + timezone import after EverCore rename#7
Conversation
…verCore rename
Two defects survived the methods/evermemos/ → methods/EverCore/ rename
and made retrieval return empty results (HTTP 200 with episodes=[]):
1. rerank_voyage.py was dropped on rename — RERANK_PROVIDER=voyage in .env
then raises "Unsupported provider: voyage" in the factory. Rewritten
from scratch (252 LOC) using rerank_deepinfra.py as a template, with
Voyage-specific request shape (plain query/documents, no Qwen wrapping)
and response mapping (data: [{index, relevance_score}] → standard
{results: [{index, score, rank}]}).
2. episodic_memory_qdrant_repository.py used timezone.utc in
datetime.fromtimestamp() but only imported `datetime`. NameError on
every vector search. One-line fix in the import statement.
3. rerank_service.py factory branch for "voyage" added between deepinfra
and the raise. Defaults base_url to https://api.voyageai.com/v1/rerank
when env override is empty.
Verified live: 60 ES candidates → Qdrant vector_search 12 ms → Voyage
rerank 15 results @ 0.4492 top score → 15 episodes returned in 5050 ms.
Backups on database:
- rerank_service.py.bak.1779028699
- episodic_memory_qdrant_repository.py.bak.1779028653
- .env*.bak.1779028576 (env LLM_BASE_URL drift; not in scope for this repo)
|
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 Pull Request integriert die Voyage AI Rerank-API als neuer Provider durch eine neue Service-Klasse ( ChangesVoyage Rerank Service 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 |
Issues are disabled on this fork, so the tracking list lives in-repo. Documents every patch that exists only in XInfty/EverOS_Qdrant and must be re-applied after each upstream sync: - Voyage rerank provider + factory branch - timezone import in episodic Qdrant repo - 13× response_format json_schema strict across LLM call-sites - Profile extractor post-parse coercion - Intentional opt-outs (tool_pre_compress stays on json_object) Each section has a one-liner grep so a future contributor (or CI guard) can verify the patch is still present after a merge from upstream. Concrete history motivating this file: - 2026-05-12: response_format=json_object patches got exposed by a model swap; migrated 13/15 sites to strict json_schema. - 2026-05-15: rerank_voyage.py + factory branch silently dropped during the methods/evermemos/ → methods/EverCore/ rename. Retrieval returned empty for three days before being noticed. Restored in #7.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7804ba3e19
ℹ️ 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".
| config = VoyageRerankConfig( | ||
| api_key=api_key, | ||
| base_url=base_url or 'https://api.voyageai.com/v1/rerank', | ||
| model=model, |
There was a problem hiding this comment.
Select a Voyage model when provider is
voyage
When RERANK_PROVIDER=voyage, this branch still forwards the shared RERANK_MODEL value unchanged, but the repo default is Qwen/Qwen3-Reranker-4B (methods/EverCore/env.template:121), which is not a Voyage rerank model. In that common configuration, Voyage returns a request error and reranking degrades/fails (especially with fallback disabled), so switching providers is broken unless users manually discover and override the model. Set a provider-specific default (e.g., rerank-2.5) when the model is unset or still on the non-Voyage default.
Useful? React with 👍 / 👎.
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/agentic_layer/rerank_service.py`:
- Around line 177-187: The Voyage branch currently always forwards the shared
model value to VoyageRerankConfig, preventing VoyageRerankConfig's internal
default (rerank-2.5) from being used; modify the branch so that
VoyageRerankConfig is constructed without a model argument when no explicit
model was provided (i.e., only pass model=model when model is not None/empty),
or explicitly pass None so the config can apply its default; update the
instantiation that creates VoyageRerankConfig in the provider=="voyage" branch
(referencing VoyageRerankConfig and VoyageRerankService) accordingly.
In `@methods/EverCore/src/agentic_layer/rerank_voyage.py`:
- Around line 217-219: The debug log currently prints the full query string
(logger.debug with query and len(all_texts)), which can expose tenant data;
update the logging in the rerank_voyage logic (look for logger.debug and the
variable query in rerank_voyage or surrounding function) to avoid logging raw
query text and instead log only non-sensitive metadata such as query length and
a deterministic hash (e.g., SHA-256 of query) plus num_texts (len(all_texts));
ensure you compute the hash from query safely and include a short label like
"query_len" and "query_hash" in the logger.debug call rather than the full query
content.
- Around line 62-71: The _ensure_session method can create multiple
aiohttp.ClientSession objects concurrently and leak them; guard session creation
with a single async lock/semaphore so only one coroutine creates and assigns
self.session at a time (e.g., acquire an existing self._session_lock or the same
Semaphore used elsewhere before checking/creating). Inside the lock, re-check
"if self.session is None or self.session.closed" then create ClientSession; if
you must replace an existing session ensure the old session is properly closed
with await self.session.close() before overwriting. Update _ensure_session to
use the lock/semaphore and the re-check pattern to prevent races and leaks.
🪄 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: 824f4c1b-b352-4ace-9b07-f36280bb91f7
📒 Files selected for processing (3)
methods/EverCore/src/agentic_layer/rerank_service.pymethods/EverCore/src/agentic_layer/rerank_voyage.pymethods/EverCore/src/infra_layer/adapters/out/search/repository/episodic_memory_qdrant_repository.py
| elif provider.lower() == "voyage": | ||
| config = VoyageRerankConfig( | ||
| api_key=api_key, | ||
| base_url=base_url or 'https://api.voyageai.com/v1/rerank', | ||
| model=model, | ||
| timeout=timeout, | ||
| max_retries=max_retries, | ||
| batch_size=batch_size, | ||
| max_concurrent_requests=max_concurrent, | ||
| ) | ||
| return VoyageRerankService(config) |
There was a problem hiding this comment.
Der Voyage-Branch neutralisiert den provider-spezifischen Modell-Default.
Hier wird immer das geteilte model übergeben; dadurch greift VoyageRerankConfig-Default (rerank-2.5) nie. Für provider="voyage" sollte ohne explizites Modell auf den Voyage-Default zurückgefallen werden.
🔧 Vorschlag zur Behebung
elif provider.lower() == "voyage":
+ resolved_model = model
+ if not resolved_model or resolved_model == HybridRerankConfig.model:
+ resolved_model = VoyageRerankConfig().model
config = VoyageRerankConfig(
api_key=api_key,
base_url=base_url or 'https://api.voyageai.com/v1/rerank',
- model=model,
+ model=resolved_model,
timeout=timeout,
max_retries=max_retries,
batch_size=batch_size,
max_concurrent_requests=max_concurrent,
)🤖 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 `@methods/EverCore/src/agentic_layer/rerank_service.py` around lines 177 - 187,
The Voyage branch currently always forwards the shared model value to
VoyageRerankConfig, preventing VoyageRerankConfig's internal default
(rerank-2.5) from being used; modify the branch so that VoyageRerankConfig is
constructed without a model argument when no explicit model was provided (i.e.,
only pass model=model when model is not None/empty), or explicitly pass None so
the config can apply its default; update the instantiation that creates
VoyageRerankConfig in the provider=="voyage" branch (referencing
VoyageRerankConfig and VoyageRerankService) accordingly.
| logger.debug( | ||
| f"Voyage reranking, query: {query!r}, num_texts={len(all_texts)}" | ||
| ) |
There was a problem hiding this comment.
Roh-Query wird geloggt und kann Tenant-Daten exponieren.
Das Debug-Log enthält den kompletten Query-Text. In Multi-Tenant-Betrieb sollte hier nur Metadaten (z. B. Länge/Hash) geloggt werden.
🔒 Vorschlag zur Behebung
logger.debug(
- f"Voyage reranking, query: {query!r}, num_texts={len(all_texts)}"
+ "Voyage reranking gestartet: num_texts=%d, query_len=%d",
+ len(all_texts),
+ len(query),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.debug( | |
| f"Voyage reranking, query: {query!r}, num_texts={len(all_texts)}" | |
| ) | |
| logger.debug( | |
| "Voyage reranking gestartet: num_texts=%d, query_len=%d", | |
| len(all_texts), | |
| len(query), | |
| ) |
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 218-218: Logging statement uses f-string
(G004)
🤖 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 `@methods/EverCore/src/agentic_layer/rerank_voyage.py` around lines 217 - 219,
The debug log currently prints the full query string (logger.debug with query
and len(all_texts)), which can expose tenant data; update the logging in the
rerank_voyage logic (look for logger.debug and the variable query in
rerank_voyage or surrounding function) to avoid logging raw query text and
instead log only non-sensitive metadata such as query length and a deterministic
hash (e.g., SHA-256 of query) plus num_texts (len(all_texts)); ensure you
compute the hash from query safely and include a short label like "query_len"
and "query_hash" in the logger.debug call rather than the full query content.
…Lock CodeRabbit finding #3: _ensure_session() could be entered concurrently by multiple coroutines (e.g. parallel rerank batches via gather), each seeing self.session is None and creating its own aiohttp.ClientSession. The losers of the race were never tracked and leaked. Add self._session_lock = asyncio.Lock() in __init__ and use a double-checked pattern inside _ensure_session: a fast path when the session is already alive, a lock-protected slow path with re-check before instantiation. Findings #1 (Voyage model default) and #2 (raw query in debug log) are intentionally deferred — production .env always sets RERANK_MODEL and DEBUG-level logs are not enabled by default in this environment.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c8bbb6bb6
ℹ️ 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".
| """Low-level reranking; ``instruction`` is ignored — Voyage uses the | ||
| query/documents pair directly.""" |
There was a problem hiding this comment.
Preserve rerank instruction in Voyage requests
VoyageRerankService.rerank_documents explicitly ignores the instruction argument, so any call site that supplies custom ranking guidance loses that behavior when RERANK_PROVIDER=voyage. For example, search_mem_service passes a skill-specific instruction to bias domain/method matching, but this provider drops it and ranks only on raw query text, which can materially reduce retrieval precision and makes Voyage behavior inconsistent with the existing vLLM/DeepInfra implementations that honor instruction.
Useful? React with 👍 / 👎.
| config = VoyageRerankConfig( | ||
| api_key=api_key, | ||
| base_url=base_url or 'https://api.voyageai.com/v1/rerank', | ||
| model=model, |
There was a problem hiding this comment.
Select a Voyage-compatible default model
The new voyage branch forwards the shared RERANK_MODEL unchanged, but the repository default remains Qwen/Qwen3-Reranker-4B in env.template, which is not a Voyage reranker model. With RERANK_PROVIDER=voyage and no explicit model override, requests will hit Voyage with an invalid model and fail with 4xx errors, breaking reranking in the default configuration for this provider.
Useful? React with 👍 / 👎.
| batch_size = self.config.batch_size or 100 | ||
| batches = [ | ||
| documents[i : i + batch_size] for i in range(0, len(documents), batch_size) |
There was a problem hiding this comment.
Guard against non-positive Voyage batch sizes
This implementation does not validate self.config.batch_size before building batches. If RERANK_BATCH_SIZE is set to a negative value, range(0, len(documents), batch_size) produces no batches, so no API calls are made and the code silently returns zero-filled scores, effectively disabling reranking without an explicit error.
Useful? React with 👍 / 👎.
| if isinstance(result, Exception): | ||
| logger.error(f"Voyage rerank batch {i} failed: {result}") | ||
| all_scores.extend([-100.0] * len(batches[i])) | ||
| continue |
There was a problem hiding this comment.
Raise on Voyage batch failures instead of fabricating scores
When a Voyage batch request errors out, the code logs it and appends synthetic -100.0 scores, then continues as if reranking succeeded. If all batches fail (e.g., invalid API key, outage, 429/5xx), callers still receive a normal-looking rerank result, so fallback/error paths are never triggered and production can silently serve degraded ordering instead of surfacing the failure.
Useful? React with 👍 / 👎.
Summary
Two custom patches survived the upstream rename
methods/evermemos/→methods/EverCore/only in the production deployment but not in this fork:RERANK_PROVIDER=voyagein production.envandRERANK_FALLBACK_PROVIDER=none(fail-fast policy), every search raisedUnsupported provider: voyagefrom the rerank-factory. Rewritten (252 LOC) usingrerank_deepinfra.pyas a template, adapted for Voyage's plain-query/documents request anddata: [{index, relevance_score}]response shape.episodic_memory_qdrant_repository.py:247(tz=timezone.utcwithoutfrom datetime import timezone). One-line fix.rerank_service.pyforvoyageprovider added betweendeepinfraand theelse: raise RerankError.Symptom was HTTP 200 +
episodes=[](not a hang) — silent fail mode that took three days to notice.Test plan
databaseafter deployment: 60 ES candidates → Qdrantvector_search12 ms → Voyage rerank 15 results @ 0.4492 top score → 15 episodes returned in 5050 ms totaleveros,everos-thot/ptah/anubis/sachmet) cleanrerank_voyage.pyupstream to EverMind-AI/EverOS so it doesn't get re-dropped on future renamesRelated