refactor: route local inference through shared service - #425
Merged
Conversation
📝 WalkthroughWalkthroughThe change routes embeddings and reranking through a shared inference service. It adds reranker health, readiness, model discovery, and failure handling. It removes in-process embedding fallback and warmup gates. Retrieval floors now use model-specific defaults and global reranker enablement. ChangesShared inference routing and lifecycle
Reranker contract and execution
Retrieval floors and failure handling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant APIWorkers
participant InferenceService
participant SearchRoute
CLI->>InferenceService: Start or use shared inference service
APIWorkers->>InferenceService: Request embeddings
SearchRoute->>InferenceService: Request reranking
InferenceService-->>SearchRoute: Return model-aware scores
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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: 5
🧹 Nitpick comments (6)
reflexio/server/llm/rerank/common.py (1)
83-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSort
__all__to satisfy RUF022.Ruff reports
__all__is not sorted. Apply isort-style ordering.♻️ Proposed ordering
__all__ = [ - "RERANK_ENABLED_ENV_VAR", "ENGLISH_RERANK_MODEL", - "MULTILINGUAL_RERANK_MODEL", "ENGLISH_RERANK_REVISION", - "MULTILINGUAL_RERANK_REVISION", - "RERANK_MODEL_REVISIONS", - "RERANK_FLOOR_DEFAULTS", - "RERANK_MODEL", + "MULTILINGUAL_RERANK_MODEL", + "MULTILINGUAL_RERANK_REVISION", + "RERANK_ENABLED_ENV_VAR", + "RERANK_FLOOR_DEFAULTS", + "RERANK_MODEL", + "RERANK_MODEL_REVISIONS", "CrossEncoderUnavailableError", "reranker_enabled", "reranker_model_for_embedding", "reranker_revision", "resolve_retrieval_floor", ]🤖 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 `@reflexio/server/llm/rerank/common.py` around lines 83 - 97, Sort the entries in the __all__ declaration in common.py using isort-style ordering to satisfy RUF022, while preserving all existing exports.Source: Linters/SAST tools
tests/server/llm/test_embedding_service_rerank.py (1)
40-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider covering the
disabledreranker health status.
_Runner.statusreturns only"ready"or"unavailable". The productionCrossEncoderRunner.statusalso returns"disabled". No test requests/health/rerankwhileREFLEXIO_RERANK_ENABLED=false, so thedisabledvalue of theRerankHealthResponse.statusLiteral is never exercised.Add that assertion to pin the readiness contract that
resolve_inference_service_capabilitiesconsumes.Also applies to: 99-112
🤖 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 `@tests/server/llm/test_embedding_service_rerank.py` around lines 40 - 41, Extend the reranker health tests around _Runner.status and the /health/rerank request to cover REFLEXIO_RERANK_ENABLED=false, asserting that RerankHealthResponse.status is "disabled". Keep the existing ready and unavailable assertions unchanged, and ensure the test exercises the status consumed by resolve_inference_service_capabilities.tests/server/services/test_unified_search_temporal.py (1)
203-208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared reranker constant.
relevance_floorbindsscore_pairs_with_model, so the patch target is valid. ImportENGLISH_RERANK_MODELand return it instead of duplicating the literal model name.🤖 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 `@tests/server/services/test_unified_search_temporal.py` around lines 203 - 208, Update the test’s fake_score in the unified search temporal test to import and return the shared ENGLISH_RERANK_MODEL constant instead of duplicating the reranker model string; keep the existing score list and relevance_floor patch target unchanged.tests/server/services/retrieval/test_relevance_floor.py (1)
197-220: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for an unknown reranker model with a null floor.
These two tests pin the known-model default and the explicit
0.0override. No test covers a discovered model that is absent fromRERANK_FLOOR_DEFAULTSwhile the configured floor isNone. That combination is the default configuration and currently escapes the fail-open guard inreflexio/server/services/retrieval/relevance_floor.pyLine 160.Add a test that returns an unrecognized model from
score_pairs_with_modeland asserts the unfiltered pool is returned.🤖 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 `@tests/server/services/retrieval/test_relevance_floor.py` around lines 197 - 220, Add a test alongside test_nullable_floor_uses_discovered_model_default that patches score_pairs_with_model to return an unrecognized model with scores, invokes apply_relevance_floors with a None floor, and asserts the original unfiltered item pool is returned. Use the existing arms and assertion style, specifically covering the unknown-model/null-floor path.tests/server/services/search/test_rerank_integration.py (1)
43-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the fake scorer tolerant of punctuation.
document.lower().split()keeps trailing punctuation on a token. A seeded content such as"The user loves pizza."yields the tokenpizza., which does not matchfood_terms. Every document then scores-1.0. The sort is stable, so the relevant profiles still appear first by input order, andtest_rerank_surfaces_relevant_profile_above_irrelevantpasses without exercising the scoring logic.Strip punctuation before matching, or match with a substring test.
♻️ Proposed refactor
def json(self) -> dict: food_terms = {"pasta", "pizza", "ramen", "peanuts"} return { "data": [ { "index": index, "score": 1.0 - if food_terms.intersection(document.lower().split()) + if food_terms.intersection(re.findall(r"[a-z]+", document.lower())) else -1.0, } for index, document in enumerate(self._documents) ] }Add the import at the top of the file:
import re🤖 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 `@tests/server/services/search/test_rerank_integration.py` around lines 43 - 55, Update the fake scorer’s json method to normalize punctuation before checking food_terms, ensuring inputs such as “pizza.” match the existing terms and produce the relevant score. Use the suggested re import or an equivalent substring-based match, while preserving the current scoring and response structure.reflexio/server/llm/rerank/cross_encoder_reranker.py (1)
99-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCatch all
httpx.TransportErrorsubclasses for colocated outages.
httpx.WriteError,httpx.ReadError, andhttpx.PoolTimeoutcurrently use the catch-all branch and setreport_failure=True. Catchhttpx.TransportErrorso all transport failures use the colocated-outage rule.♻️ Proposed refactor
- except ( - httpx.ConnectError, - httpx.ConnectTimeout, - httpx.ReadTimeout, - httpx.RemoteProtocolError, - ) as exc: + except httpx.TransportError as exc: raise CrossEncoderUnavailableError( f"Rerank service request failed at {url}: {exc}", report_failure=not expected_local_unavailability, ) from exc🤖 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 `@reflexio/server/llm/rerank/cross_encoder_reranker.py` around lines 99 - 121, Update the exception handling around the rerank service request to catch httpx.TransportError, including WriteError, ReadError, and PoolTimeout, in the colocated-outage branch. Apply the expected_local_unavailability rule when setting report_failure, while preserving the existing handling for HTTPStatusError and non-transport errors.
🤖 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 `@reflexio/server/llm/model_defaults.py`:
- Line 490: Update the startup diagnostics to use
REFLEXIO_EMBEDDING_SERVICE_URL: in reflexio/server/llm/model_defaults.py lines
490-490, report remote inference when the configured URL is non-local and
colocated inference otherwise; apply the same distinction in
reflexio/cli/commands/services.py lines 83-84 and replace the stale ChromaDB
fallback matrix there.
In `@reflexio/server/llm/providers/embedding_service_provider.py`:
- Around line 95-99: Update embedding_service_url() to normalize the
REFLEXIO_EMBEDDING_SERVICE_URL value by stripping whitespace before each
presence check, so whitespace-only values are treated as unset and do not route
to inference_service_url(). Add a regression test covering a whitespace-only
environment value.
- Around line 76-84: Update inference_service_url() to parse and validate
REFLEXIO_EMBEDDING_SERVICE_URL: permit http only for an exact loopback host,
require https for every other host, and reject invalid or unsupported schemes
while preserving normalized trailing-slash handling. Document this HTTPS
requirement and the loopback-only HTTP exception in .env.example at lines
106-112.
In `@reflexio/server/services/retrieval/relevance_floor.py`:
- Around line 143-160: Move the per-arm resolve_retrieval_floor calls in
_apply_floors into the existing try block that catches
CrossEncoderUnavailableError, resolving all floors before entering the results
loop. Preserve the current fail-open return of unfiltered RelevanceFloorResult
values when either model scoring or floor resolution raises
CrossEncoderUnavailableError.
In `@tests/e2e_tests/test_service_mode_embedding_e2e.py`:
- Around line 216-226: Update the embedding daemon startup coverage in
_EmbeddingDaemon.start and the reranker health assertion to allow enough time
for both model downloads, including the cross-encoder prewarm. Increase the
startup timeout beyond 90 seconds and include the /health/rerank response
details in the readiness assertion failure so prewarm errors are diagnosable.
---
Nitpick comments:
In `@reflexio/server/llm/rerank/common.py`:
- Around line 83-97: Sort the entries in the __all__ declaration in common.py
using isort-style ordering to satisfy RUF022, while preserving all existing
exports.
In `@reflexio/server/llm/rerank/cross_encoder_reranker.py`:
- Around line 99-121: Update the exception handling around the rerank service
request to catch httpx.TransportError, including WriteError, ReadError, and
PoolTimeout, in the colocated-outage branch. Apply the
expected_local_unavailability rule when setting report_failure, while preserving
the existing handling for HTTPStatusError and non-transport errors.
In `@tests/server/llm/test_embedding_service_rerank.py`:
- Around line 40-41: Extend the reranker health tests around _Runner.status and
the /health/rerank request to cover REFLEXIO_RERANK_ENABLED=false, asserting
that RerankHealthResponse.status is "disabled". Keep the existing ready and
unavailable assertions unchanged, and ensure the test exercises the status
consumed by resolve_inference_service_capabilities.
In `@tests/server/services/retrieval/test_relevance_floor.py`:
- Around line 197-220: Add a test alongside
test_nullable_floor_uses_discovered_model_default that patches
score_pairs_with_model to return an unrecognized model with scores, invokes
apply_relevance_floors with a None floor, and asserts the original unfiltered
item pool is returned. Use the existing arms and assertion style, specifically
covering the unknown-model/null-floor path.
In `@tests/server/services/search/test_rerank_integration.py`:
- Around line 43-55: Update the fake scorer’s json method to normalize
punctuation before checking food_terms, ensuring inputs such as “pizza.” match
the existing terms and produce the relevant score. Use the suggested re import
or an equivalent substring-based match, while preserving the current scoring and
response structure.
In `@tests/server/services/test_unified_search_temporal.py`:
- Around line 203-208: Update the test’s fake_score in the unified search
temporal test to import and return the shared ENGLISH_RERANK_MODEL constant
instead of duplicating the reranker model string; keep the existing score list
and relevance_floor patch target unchanged.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8208be87-bf43-4941-a9e5-a8861000a38f
📒 Files selected for processing (51)
.env.exampleREADME.mdreflexio/cli/README.mdreflexio/cli/commands/services.pyreflexio/cli/commands/setup_cmd.pyreflexio/cli/run_services.pyreflexio/lib/_profiles.pyreflexio/models/config_schema.pyreflexio/server/__main__.pyreflexio/server/api.pyreflexio/server/llm/_litellm_embedding.pyreflexio/server/llm/embedding_service.pyreflexio/server/llm/litellm_client.pyreflexio/server/llm/model_defaults.pyreflexio/server/llm/providers/embedder_warmup.pyreflexio/server/llm/providers/embedding_service_provider.pyreflexio/server/llm/rerank/__init__.pyreflexio/server/llm/rerank/common.pyreflexio/server/llm/rerank/cross_encoder_model.pyreflexio/server/llm/rerank/cross_encoder_reranker.pyreflexio/server/routes/search.pyreflexio/server/routes/system.pyreflexio/server/services/playbook/components/reviewer.pyreflexio/server/services/retrieval/relevance_floor.pyreflexio/server/services/unified_search_service.pytests/cli/test_run_services_workers.pytests/cli/test_service_builders.pytests/cli/test_services_first_run.pytests/conftest.pytests/e2e_tests/test_service_mode_embedding_e2e.pytests/models/test_retrieval_floor_config.pytests/server/llm/rerank/test_cross_encoder_device.pytests/server/llm/rerank/test_prewarm.pytests/server/llm/rerank/test_remote_service.pytests/server/llm/test_embedder_warmup.pytests/server/llm/test_embedding_service_provider.pytests/server/llm/test_embedding_service_rerank.pytests/server/llm/test_litellm_client_unit.pytests/server/llm/test_local_embedding_provider.pytests/server/llm/test_model_defaults.pytests/server/llm/test_service_only_inference_boundary.pytests/server/routes/test_health_warm_gate.pytests/server/routes/test_rerank_profile_route.pytests/server/services/retrieval/test_relevance_floor.pytests/server/services/search/test_rerank_integration.pytests/server/services/storage/sqlite_storage/test_profiles_atomicity_characterization_integration.pytests/server/services/storage/test_lineage_b3d_interactions_integration.pytests/server/services/storage/test_lineage_b3f_profile_atomic_integration.pytests/server/services/test_unified_search_floor.pytests/server/services/test_unified_search_temporal.pytests/server/test_recycle_smoke_integration.py
💤 Files with no reviewable changes (4)
- reflexio/server/llm/litellm_client.py
- tests/server/llm/test_embedder_warmup.py
- tests/server/routes/test_health_warm_gate.py
- reflexio/server/llm/providers/embedder_warmup.py
Remove API-process embedding and reranker execution, discover both service models from one health response, and add nullable model-specific retrieval floors. Keep cloud embedding providers direct and make colocated inference a supervised HTTP service.
yyiilluu
force-pushed
the
codex/service-only-inference
branch
from
August 7, 2026 16:57
1e00565 to
75627df
Compare
yilu331
added a commit
that referenced
this pull request
Aug 7, 2026
…centroid (#436) ## Problem 4 e2e tests fail on `main`: ``` RuntimeError: rerun agent playbook has no centroid embedding aggregator.py:1773 ``` - `test_playbook_workflows.py` — 3 tests - `test_openclaw_integration.py` — 1 test They pass at `b0e0755` and fail from `c24e1e7` onward, so this is a regression, not an environment quirk. ## Root cause Two changes that were each fine alone. **`159d8ab` (#410)** added the invariant: on the rerun path, a saved agent playbook must carry an embedding to persist as a cluster centroid. Safe at the time — local embeddings were computed **in-process**, so `saved_fb.embedding` was always populated and the raise was unreachable outside a genuine bug. **`c24e1e7` (#425)** removed in-process local inference ("service-only inference boundary"). `embedding_provider_mode` now returns `local_service` for any local model with no env vars set, so embedding goes over HTTP to `127.0.0.1:8072`. With the embedder unreachable, `SQLiteStorage` deliberately degrades: ``` Embedding unavailable for document text; continuing without vector ``` …and saves the playbook with `embedding=None`. The invariant then fires and rolls back the entire aggregation. An assertion written to catch a *programming error* now trips on an *infrastructure state*. This is not narrow: `reflexio/lib/_generation.py:73` sets `rerun=True` for every `run_playbook_aggregation()` call, so the branch is the normal path, not an edge case. ## Why it wasn't caught - This repo runs no CI workflows. - Enterprise `ci-fast.yml:104` runs `--ignore=tests/e2e_tests/`. - The e2e tier only runs in `release.yml`. - The invariant had **no test coverage at all** — `grep 'no centroid embedding' tests/` returns nothing. ## Fix Mock mode is the case that has no centroid *by construction*: it clusters by trigger rather than by vector (the `MOCK_LLM_RESPONSE` branch in `get_clusters`), so a centroid was never meaningful there. Skip the cluster bookkeeping instead of aborting. Every other caller still raises — a centroid-less cluster row would silently break the incremental re-aggregation that table exists to feed. Production behaviour outside mock mode is unchanged. Adds the three cases the invariant never had: - the raise still fires outside mock mode - mock mode reaches the save, then skips the centroid write - the happy path still records the cluster Verified the new tests fail against the pre-fix aggregator with the exact `RuntimeError`. ## Verification - OSS e2e tier: **47 passed, 87 skipped** (was 4 failed / 43 passed) - OSS unit tier: **4368 passed, 9 skipped** - ruff + pyright clean ## Worth a second opinion The mock-mode carve-out fixes the tests, but the underlying mismatch is broader: storage treats a missing embedding as *degrade and continue*, while the aggregator treats it as *fatal*. In any deployment where the embedding service is unreachable, `run_playbook_aggregation()` now hard-fails and rolls back rather than degrading. That may well be intended — failing loudly beats silently writing a useless centroid — but it changed behaviour without discussion when #425 landed, so the owners of #410/#425 should confirm which semantics they want.
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
1 participant
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.
Summary
Changes
docs/superpowers/plans/2026-06-26-profile-module-cutover.md.ProfileGenerationServicefromprofile.serviceandProfileExtractorfromprofile.components.extractor.open_source/reflexioat the OSS profile module cutover commit.Related PRs
Test Plan
uv run pytest open_source/reflexio/tests/server/services/profile/test_profile_module_contract.py open_source/reflexio/tests/server/services/profile/test_profile_extractor.py open_source/reflexio/tests/server/services/profile/test_profile_generation_service.py open_source/reflexio/tests/server/services/profile/test_profile_consolidator.py open_source/reflexio/tests/server/services/extraction/test_resume_worker.py reflexio_ext/tests/server/services/profile/test_profile_extractor.py -q -o 'addopts='->122 passeduv run pytest open_source/reflexio/tests/lib/test_profiles_unit.py open_source/reflexio/tests/lib/test_generation_unit.py open_source/reflexio/tests/eval/extraction/test_extraction_eval.py -q -o 'addopts='->71 passed, 2 skippeduv run ruff check open_source/reflexio/reflexio/server/services/profile open_source/reflexio/reflexio/server/services/generation_service.py open_source/reflexio/reflexio/server/services/extraction/resume_worker.py open_source/reflexio/reflexio/lib/_profiles.py open_source/reflexio/reflexio/lib/_generation.py open_source/reflexio/tests/server/services/profile open_source/reflexio/tests/server/services/extraction/test_resume_worker.py reflexio_ext/tests/server/services/profile/test_profile_extractor.pyuv run pyright open_source/reflexio/reflexio/server/services/profile open_source/reflexio/reflexio/server/services/generation_service.py open_source/reflexio/reflexio/server/services/extraction/resume_worker.py open_source/reflexio/reflexio/lib/_profiles.py open_source/reflexio/reflexio/lib/_generation.py open_source/reflexio/tests/server/services/profile open_source/reflexio/tests/server/services/extraction/test_resume_worker.py reflexio_ext/tests/server/services/profile/test_profile_extractor.py->0 errorsuv run python -c "import reflexio; print('import OK')"rg -n -P "profile\.profile_generation_service(?!_utils)|profile\.profile_extractor|profile_generation_service\.py|profile_extractor\.py" open_source/reflexio/reflexio open_source/reflexio/tests reflexio_ext --glob '*.py' --glob '*.md' --glob '*.mdx'-> only the removed-file assertion in the new contract testSummary by CodeRabbit
Documentation
Bug Fixes
Tests
Rebase status