Skip to content

Commit e9ac7cc

Browse files
bsboddenclaude
andcommitted
fix(llm-router): address Copilot review comments
Applied Copilot security/quality recommendations: 1. Fixed mutable default arguments - Changed `connection_kwargs: Dict[str, Any] = {}` to `Optional[Dict[str, Any]] = None` in LLMRouter.__init__ and SemanticRouter.__init__ to avoid shared state across instances. 2. Added empty routes guard - Added `if not self.routes: return []` check in both sync and async `_get_route_matches()` to prevent ValueError when calling `max(route.distance_threshold for route in self.routes)` on empty sequence. These changes address the same issues that were fixed in the original LLMRouter implementation (commits e1cd469 and 9cba561), ensuring consistency across the refactored codebase. All 86 router tests passing. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent fed62ee commit e9ac7cc

File tree

2 files changed

+13
-2
lines changed

2 files changed

+13
-2
lines changed

redisvl/extensions/llm_router/__init__.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def __init__(
7979
redis_url: str = "redis://localhost:6379",
8080
overwrite: bool = False,
8181
cost_optimization: bool = False,
82-
connection_kwargs: Dict[str, Any] = {},
82+
connection_kwargs: Optional[Dict[str, Any]] = None,
8383
**kwargs,
8484
):
8585
"""Initialize LLMRouter (deprecated, use SemanticRouter).
@@ -110,6 +110,10 @@ def __init__(
110110

111111
routing_config = RoutingConfig(cost_optimization=True)
112112

113+
# Handle mutable default
114+
if connection_kwargs is None:
115+
connection_kwargs = {}
116+
113117
# Call parent __init__
114118
super().__init__(
115119
name=name,

redisvl/extensions/router/semantic.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def __init__(
5656
redis_client: Optional[SyncRedisClient] = None,
5757
redis_url: str = "redis://localhost:6379",
5858
overwrite: bool = False,
59-
connection_kwargs: Dict[str, Any] = {},
59+
connection_kwargs: Optional[Dict[str, Any]] = None,
6060
**kwargs,
6161
):
6262
"""Initialize the SemanticRouter.
@@ -96,6 +96,9 @@ def __init__(
9696
if routing_config is None:
9797
routing_config = RoutingConfig()
9898

99+
if connection_kwargs is None:
100+
connection_kwargs = {}
101+
99102
super().__init__(
100103
name=name,
101104
routes=routes,
@@ -386,6 +389,8 @@ def _get_route_matches(
386389
max_k: int = 1,
387390
) -> List[RouteMatch]:
388391
"""Get route response from vector db"""
392+
if not self.routes:
393+
return []
389394

390395
# what's interesting about this is that we only provide one distance_threshold for a range query not multiple
391396
# therefore you might take the max_threshold and further refine from there.
@@ -1438,6 +1443,8 @@ async def _get_route_matches(
14381443
max_k: int = 1,
14391444
) -> List[RouteMatch]:
14401445
"""Get route response from vector db (async)"""
1446+
if not self.routes:
1447+
return []
14411448

14421449
# what's interesting about this is that we only provide one distance_threshold for a range query not multiple
14431450
# therefore you might take the max_threshold and further refine from there.

0 commit comments

Comments
 (0)