refactor(routing)!: remove latency-aware router - #162
Merged
eric-liu-nvidia merged 1 commit intoJul 28, 2026
Merged
Conversation
Signed-off-by: Eric Liu <zengyuanl@nvidia.com>
|
WalkthroughThe pull request removes the latency-aware router and its backend, configuration, profile, route-bundle support, health metrics, alerts, documentation, exports, and dedicated tests. Remaining routing and endpoint error-contract tests are updated accordingly. ChangesLatency Service Removal
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 2
🧹 Nitpick comments (3)
tests/test_outcome_metrics.py (1)
274-277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate
_canned_app()asFastAPI.
build_switchyard_appreturnsFastAPI;-> objectdiscards that under strict mypy for no benefit.♻️ Proposed typing tightening
-def _canned_app() -> object: +def _canned_app() -> FastAPI: return build_switchyard_app( Switchyard(backend=_CannedBackend(), translator=TranslationEngine()) )Requires
from fastapi import FastAPIin the import block.As per coding guidelines: "provide type hints throughout; code must pass strict mypy".
🤖 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/test_outcome_metrics.py` around lines 274 - 277, The _canned_app() return annotation is too broad for strict mypy. Import FastAPI and annotate _canned_app() with FastAPI, preserving its existing build_switchyard_app implementation.Source: Coding guidelines
tests/test_inference_e2e.py (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
pytest-mock'smockerfixture instead ofunittest.mock.patch.The repository standard for general mocking in
tests/ispytest-mock, which also removes the nestedwithand guarantees teardown ordering.♻️ Proposed switch to `mocker`
- def test_post_dispatch_exception_returns_json_500(self, app: FastAPI) -> None: + def test_post_dispatch_exception_returns_json_500( + self, app: FastAPI, mocker: MockerFixture + ) -> None: """An exception raised AFTER dispatch (e.g. during result serialization) must surface as a JSON 500 envelope, not FastAPI's plain-text 500.""" class _Unserializable: def model_dump(self) -> None: raise RuntimeError("serialization exploded") - with patch( - "switchyard.lib.endpoints.openai_chat_endpoint.dispatch_chat_request", - new_callable=AsyncMock, - return_value=_Unserializable(), - ): - with TestClient(app, raise_server_exceptions=False) as client: - response = client.post( - "/v1/chat/completions", - json={"model": "any-model", "messages": [{"role": "user", "content": "hi"}]}, - ) + mocker.patch( + "switchyard.lib.endpoints.openai_chat_endpoint.dispatch_chat_request", + new_callable=mocker.AsyncMock, + return_value=_Unserializable(), + ) + with TestClient(app, raise_server_exceptions=False) as client: + response = client.post( + "/v1/chat/completions", + json={"model": "any-model", "messages": [{"role": "user", "content": "hi"}]}, + )Then drop the
unittest.mockimport and addfrom pytest_mock import MockerFixtureunder aTYPE_CHECKINGguard.As per coding guidelines: "Use
respxfor HTTP mocking,pytest-mockfor general mocking".Also applies to: 549-553
🤖 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/test_inference_e2e.py` at line 16, Replace unittest.mock.patch usage in the affected tests with the pytest-mock mocker fixture, removing nested patch context managers while preserving existing mocked behavior and teardown scope. Remove the unittest.mock import, and add pytest_mock.MockerFixture under a TYPE_CHECKING guard for fixture annotations.Source: Coding guidelines
tests/test_upstream_error_passthrough.py (1)
190-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRe-anchor the
provider-labelledERROR_SOURCE_HEADERassertion here.Only the switchyard-labelled branch survives (Line 317). The provider-labelled branch lost its wire-level coverage with the removed latency-backed tests, even though it is reproducible on this Rust-native 401 path with one assertion.
💚 Proposed coverage restore
assert response.status_code == 401 + assert response.headers[ERROR_SOURCE_HEADER] == "provider" assert response.json() == {🤖 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/test_upstream_error_passthrough.py` around lines 190 - 197, Restore wire-level coverage in the 401 response test by asserting the response’s ERROR_SOURCE_HEADER carries the provider label alongside the existing status and JSON assertions. Keep this scoped to the provider-labelled upstream error path represented by the response fixture.
🤖 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 `@examples/prometheus/README.md`:
- Around line 28-30: Update the Kubernetes discovery guidance in
examples/prometheus/README.md (lines 28-30) and the related comment in
examples/prometheus/prometheus.yml (lines 21-23) to reference a complete
Kubernetes scrape job, including the required role, rather than suggesting
kubernetes_sd_configs is a direct static-target block swap.
In `@tests/test_error_source_annotation.py`:
- Around line 8-10: Update the module docstring in
tests/test_error_source_annotation.py to correct the grammar, replacing “from
the ctx stamps a backend sets” with equivalent wording such as “from ctx stamps
set by a backend.”
---
Nitpick comments:
In `@tests/test_inference_e2e.py`:
- Line 16: Replace unittest.mock.patch usage in the affected tests with the
pytest-mock mocker fixture, removing nested patch context managers while
preserving existing mocked behavior and teardown scope. Remove the unittest.mock
import, and add pytest_mock.MockerFixture under a TYPE_CHECKING guard for
fixture annotations.
In `@tests/test_outcome_metrics.py`:
- Around line 274-277: The _canned_app() return annotation is too broad for
strict mypy. Import FastAPI and annotate _canned_app() with FastAPI, preserving
its existing build_switchyard_app implementation.
In `@tests/test_upstream_error_passthrough.py`:
- Around line 190-197: Restore wire-level coverage in the 401 response test by
asserting the response’s ERROR_SOURCE_HEADER carries the provider label
alongside the existing status and JSON assertions. Keep this scoped to the
provider-labelled upstream error path represented by the response fixture.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6b69bf94-c98b-4efb-a103-541b0ed62f91
📒 Files selected for processing (49)
AGENTS.mdCHANGELOG.mdcrates/switchyard-components/tests/stats_accumulator.rscrates/switchyard-py/src/core_bindings/context.rscrates/switchyard-py/src/core_bindings/response.rsdocs/internal/latency_service_routing.mddocs/internal/metrics_reference.mdexamples/latency_service_caller_required.yamlexamples/prometheus/README.mdexamples/prometheus/prometheus.ymlexamples/prometheus/switchyard.rules.yamlswitchyard/__init__.pyswitchyard/cli/launch_command.pyswitchyard/cli/launchers/claude_code_launcher.pyswitchyard/cli/route_bundle.pyswitchyard/cli/switchyard_cli.pyswitchyard/lib/affinity_pin_store.pyswitchyard/lib/backends/__init__.pyswitchyard/lib/backends/health_poller.pyswitchyard/lib/backends/latency_service_llm_backend.pyswitchyard/lib/config/__init__.pyswitchyard/lib/config/latency_service_backend_config.pyswitchyard/lib/endpoints/prometheus_emitter.pyswitchyard/lib/endpoints/stats_endpoint.pyswitchyard/lib/endpoints/upstream_error.pyswitchyard/lib/profiles/__init__.pyswitchyard/lib/profiles/escalation_router_profile_config.pyswitchyard/lib/profiles/latency_service.pyswitchyard/lib/proxy_context.pyswitchyard/lib/redis_pin_store.pyswitchyard/lib/session_affinity.pyswitchyard/lib/tracing.pytests/e2e/test_latency_service_llm_backend.pytests/readme/test_readme.pytests/test_error_source_annotation.pytests/test_inference_e2e.pytests/test_latency_service_health_metrics.pytests/test_latency_service_llm_backend.pytests/test_latency_service_spans.pytests/test_latency_service_stats_metrics.pytests/test_outcome_metrics.pytests/test_passthrough_profile_e2e.pytests/test_redis_pin_store.pytests/test_route_bundle.pytests/test_route_selection_headers.pytests/test_session_affinity.pytests/test_stream_close_chain.pytests/test_stream_leak_repro.pytests/test_upstream_error_passthrough.py
💤 Files with no reviewable changes (16)
- switchyard/lib/profiles/latency_service.py
- examples/latency_service_caller_required.yaml
- docs/internal/latency_service_routing.md
- switchyard/lib/backends/health_poller.py
- switchyard/lib/config/latency_service_backend_config.py
- switchyard/lib/config/init.py
- switchyard/init.py
- switchyard/lib/backends/latency_service_llm_backend.py
- tests/test_latency_service_health_metrics.py
- tests/test_latency_service_stats_metrics.py
- tests/e2e/test_latency_service_llm_backend.py
- switchyard/lib/backends/init.py
- tests/test_latency_service_spans.py
- tests/test_latency_service_llm_backend.py
- switchyard/lib/profiles/init.py
- switchyard/cli/route_bundle.py
grahamking
approved these changes
Jul 28, 2026
messiaen
approved these changes
Jul 28, 2026
eric-liu-nvidia
deleted the
eric-liu/switch-1111-delete-latency-aware-router-code-for-switchyard-02
branch
July 28, 2026 16:41
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
4 participants
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
Removes the latency-aware router ("latency service") from Switchyard for the 0.2+ line, per SWITCH-1111.
The router depended on NVIDIA Inference Hub's latency endpoint and schema, which is specific to Inference Hub. Per the team decision, Switchyard should not carry this: Inference Hub can pin the current commit indefinitely, and deployments needing multi-endpoint / load- or latency-aware routing should move to Dynamo (backend-load / KV-cache-aware routing with failover) or an external load balancer such as Traefik / HAProxy.
What changed (49 files, +275 / −7,274)
latency_service_llm_backend.py,health_poller.py,latency_service_backend_config.py,latency_service.py), 5 latency-only test files, the internal routing doc, and the example YAML.__init__.pyre-exports acrossbackends/config/profiles/ top-level) and stripped thelatency_serviceroute type out ofcli/route_bundle.py(dispatch, key-sets, builders, type inference, aliases, plus an orphaned helper).test_redis_pin_store.py's shared-infra coverage onto the escalation router's_build_affinity_l2, and trimmed 4 mixed test files down to their non-latency tests.[Unreleased] → Removedentry (0.1.0 history preserved), trimmedmetrics_reference.md, the threeexamples/prometheus/*files, and theAGENTS.mdsource-tree map; scrubbed stale docstrings/comments naming the deleted class across shared source.Test coverage
A few generic behaviors were previously covered only through latency-vehicled tests; these were re-vehicled onto in-process passthrough-style backends:
switchyard_client_responses_total, operational routes (/metrics,/health,/v1/models) do not.invalid_valueinto a provider-compatible 400; no surviving backend reproduces that, so an invalid role now surfaces as a 500internal_error. The re-vehicled test pins this current 500 behavior and documents where a 400 could be restored (follow-up if desired).Verification
ruff✅ ·mypy --strict✅ ·pytest1735 passed, 35 skipped ✅ ·mkdocs build --strict✅ ·cargo fmt --check/clippy -D warnings/test --workspace✅One deselected test (
test_verify.py::…::test_secrets_file_wins_when_no_cli_or_env) is a pre-existing, environment-specific config-leak failure — it reproduces on a clean tree and is unrelated to this change.🤖 Generated with Claude Code
Summary by CodeRabbit
Removed
Documentation
Bug Fixes