✨ Display full free roster on dashboard - #324
Conversation
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Sorry @sheepdestroyer, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Warning Review limit reached
Next review available in:56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughA new OpenRouter inspection script and centralized free-model discovery flow support adaptive roster registration, rate-limit-triggered refreshes, best-model caching, and dashboard display of model scores, capabilities, and activation status. ChangesFree model roster
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant LiteLLM
participant OpenRouter
Client->>Router: Request agent-* model
Router->>LiteLLM: Proxy request
LiteLLM-->>Router: Return HTTP 429
Router->>OpenRouter: Fetch free-model metadata
Router->>LiteLLM: Synchronize roster
Router->>Router: Invalidate best-model cache
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request refactors the OpenRouter free model roster synchronization logic by extracting model fetching into a dedicated helper, implementing opportunistic roster syncs on rate limits (HTTP 429), and adding a Free Model Roster table to the dashboard UI. The review feedback highlights several critical improvements: introducing an asyncio.Lock to serialize and throttle concurrent roster syncs to prevent hammering the API, using a safe fallback value for context_length instead of defaulting to 0 (which would break upstream requests), and applying defensive .get() calls with fallbacks when rendering the dashboard table to avoid potential KeyError or TypeError exceptions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| FREE_MODEL_CACHE_TTL = 3600 # Refresh cache every 1 hour | ||
| _registered_free_models: Dict[str, Set[str]] = {} | ||
| _last_roster_sync: float = 0.0 |
There was a problem hiding this comment.
Define an asyncio.Lock to prevent concurrent roster sync executions when multiple requests trigger opportunistic syncs simultaneously.
| _last_roster_sync: float=0.0 | |
| _last_roster_sync: float=0.0 | |
| _roster_sync_lock=asyncio.Lock() |
| async def maybe_trigger_roster_sync(force: bool = False): | ||
| """Opportunistically refresh the OpenRouter roster if ratelimited or after TTL.""" | ||
| global _last_roster_sync | ||
| now = time.monotonic() | ||
| # 5-minute throttle for roster sync | ||
| if not force and (now - _last_roster_sync < 300): | ||
| return | ||
| master_key = os.getenv("LITELLM_MASTER_KEY") | ||
| if master_key: | ||
| logger.info(f"Triggering opportunistic roster sync (force={force})") | ||
| await sync_adaptive_router_roster(master_key) | ||
| # Invalidate cache to ensure dashboard gets fresh data | ||
| global free_model_cache | ||
| free_model_cache["data"] = None |
There was a problem hiding this comment.
Use the _roster_sync_lock to serialize and throttle opportunistic roster syncs. This prevents multiple concurrent requests from hammering the OpenRouter API and causing database deadlocks or duplicate model registrations during a rate-limit storm.
asyncdefmaybe_trigger_roster_sync(force: bool=False):
"""Opportunistically refresh the OpenRouter roster if ratelimited or after TTL."""global_last_roster_syncnow=time.monotonic()
# 5-minute throttle for roster syncifnotforceand (now-_last_roster_sync<300):
returnif_roster_sync_lock.locked():
logger.info("Roster sync already in progress — skipping opportunistic trigger")
returnasyncwith_roster_sync_lock:
# Re-check throttle inside lock in case another task just finished syncingnow=time.monotonic()
ifnotforceand (now-_last_roster_sync<300):
returnmaster_key=os.getenv("LITELLM_MASTER_KEY")
ifmaster_key:
logger.info(f"Triggering opportunistic roster sync (force={force})")
awaitsync_adaptive_router_roster(master_key)
# Invalidate cache to ensure dashboard gets fresh dataglobalfree_model_cachefree_model_cache["data"] =None| "id": mid, | ||
| "name": m.get("name", mid), | ||
| "score": score, | ||
| "context_length": m.get("context_length") or 0, |
There was a problem hiding this comment.
Defaulting context_length to 0 when it is missing or falsy will cause the model to be registered in LiteLLM with max_tokens: 0 and max_input_tokens: 0. This will break all upstream requests to these models. Default to a safe fallback value like 262144 instead.
| "context_length": m.get("context_length") or0, | |
| "context_length": m.get("context_length") or262144, |
| <td style="padding:10px 8px;font-size:12px;font-weight:600;">{m['name']}<br><span style="font-size:10px;opacity:0.4;font-family:monospace;">{mid}</span></td> | ||
| <td style="padding:10px 8px;text-align:center;font-weight:bold;color:#fbbf24;">{m['score']:.1f}</td> | ||
| <td style="padding:10px 8px;text-align:center;opacity:0.7;font-size:11px;">{m['context_length']//1000}k</td> |
There was a problem hiding this comment.
Use defensive .get() calls with safe fallbacks when rendering the dashboard table to prevent potential KeyError or TypeError if any fields are missing or malformed in the persisted roster JSON file.
| <tdstyle="padding:10px 8px;font-size:12px;font-weight:600;">{m['name']}<br><spanstyle="font-size:10px;opacity:0.4;font-family:monospace;">{mid}</span></td> | |
| <tdstyle="padding:10px 8px;text-align:center;font-weight:bold;color:#fbbf24;">{m['score']:.1f}</td> | |
| <tdstyle="padding:10px 8px;text-align:center;opacity:0.7;font-size:11px;">{m['context_length']//1000}k</td> | |
| <tdstyle="padding:10px 8px;font-size:12px;font-weight:600;">{m.get('name', mid)}<br><spanstyle="font-size:10px;opacity:0.4;font-family:monospace;">{mid}</span></td> | |
| <tdstyle="padding:10px 8px;text-align:center;font-weight:bold;color:#fbbf24;">{m.get('score', 0.0):.1f}</td> | |
| <tdstyle="padding:10px 8px;text-align:center;opacity:0.7;font-size:11px;">{(m.get('context_length') or262144)//1000}k</td> |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@router/main.py`:
- Around line 3469-3477: Update the HTML row construction to apply html.escape
to the external model fields m['name'] and mid before interpolating them into
the markup. Preserve their displayed values and existing formatting while
ensuring both fields are safely escaped.
- Around line 2679-2682: Remove the unreachable status_code 429 handling from
the stream error handler around maybe_trigger_roster_sync, including the
model_name check there. Preserve or relocate roster synchronization for 429
responses within the existing non-200 response handling branch, where the
response status is available.
- Around line 1630-1652: Update the model filtering logic in
sync_adaptive_router_roster to skip entries where supported_parameters does not
contain "tools". Place the has_tools check before pricing and free_models.append
so non-tool models are not registered in agent-* deployments, while preserving
the existing denylist and tool-capable model handling.
- Around line 1911-1927: The maybe_trigger_roster_sync function needs
single-flight protection because force=True bypasses the throttle and can start
overlapping sync_adaptive_router_roster runs. Add an in-progress guard around
the sync and ensure it is cleared reliably after completion, so concurrent
forced calls coalesce or return while one sync is active while preserving the
existing throttle and cache invalidation behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82b037f1-8c66-4383-b897-ca9ad6f3775c
⛔ Files ignored due to path filters (2)
mock.logis excluded by!**/*.logserver.logis excluded by!**/*.log
📒 Files selected for processing (2)
check_openrouter.pyrouter/main.py
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| if hasattr(ex, "status_code") and getattr(ex, "status_code") == 429: | ||
| if model_name.startswith("agent-"): | ||
| await maybe_trigger_roster_sync(force=True) | ||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Inspect the relevant ranges in router/main.py
python3 - <<'PY'from pathlib import Pathp = Path("router/main.py")lines = p.read_text().splitlines()for start, end in [(2588, 2615), (2665, 2695), (2718, 2745)]: print(f"\n--- {p}:{start}-{end} ---") for i in range(start, min(end, len(lines)) + 1): print(f"{i:5d}: {lines[i-1]}")PY# Find other uses of status_code in streaming/error handling nearby
rg -n 'status_code|aiter_bytes|maybe_trigger_roster_sync|429' router/main.pyRepository: sheepdestroyer/LLM-Routing
Length of output: 10027
🏁 Script executed:
#!/bin/bashset -euo pipefail
# Identify the HTTP client and related imports/usages
python3 - <<'PY'from pathlib import Pathp = Path("router/main.py")lines = p.read_text().splitlines()for i, line in enumerate(lines, 1): if 'import httpx' in line or 'from httpx' in line or 'AsyncClient' in line or 'client =' in line: print(f"{i:5d}: {line}")PY# Search for any exception/status_code patterns in the repository
rg -n 'hasattr\(.*status_code|getattr\(.*status_code|\.status_code\)' router main.py .Repository: sheepdestroyer/LLM-Routing
Length of output: 2089
🌐 Web query:
httpx streaming response aiter_bytes exception status_code transport decode errors
💡 Result:
When using httpx streaming responses with aiter_bytes, you may encounter several categories of exceptions and behaviors related to transport, decoding, and stream lifecycle management. Exceptions and Error Handling DecodingError: This exception is raised when the response content is malformed or cannot be decompressed (e.g., gzip, brotli, or zstd errors) [1][2][3]. If you are working with compressed streams, ensure the necessary dependencies (like httpx[zstd] or brotli) are installed [2]. StreamError Subclasses: - StreamConsumed: Raised if you attempt to iterate over a stream that has already been consumed [4][3]. Note that aiter_raw and aiter_bytes consume the stream, and aiter_raw is typically only available once [4][5]. - StreamClosed: Raised if you try to stream content after the underlying request connection has been closed [3]. - ResponseNotRead: This occurs if you attempt to access properties like response.content after a streaming request without having fully read the content (e.g., via aread) [6][3]. Transport and Asyncio Exceptions: - asyncio.CancelledError: When using asynchronous streaming, it is important to handle asyncio.CancelledError. Recent versions of httpx have addressed issues where cancellation during stream iteration required explicit handling to ensure proper generator cleanup [7]. - Connection/Read Errors: Standard transport errors (e.g., ReadError, ConnectError) may be raised during iteration if the network connection is interrupted while fetching chunks [1][3]. Key Differences in Streaming Methods - aiter_bytes: Iterates over the decoded (decompressed) content of the response [4][5]. It automatically handles Content-Encoding (e.g., gzip, deflate) [8][5]. - aiter_raw: Iterates over the raw, compressed bytes-on-the-wire [4][5]. This is generally not needed for end-users and should be used with caution as it is only available once [4]. Status Codes The status_code of the response is available regardless of whether the body has been streamed [9]. To enforce status code validation (e.g., raising an exception for 4xx or 5xx errors), you should explicitly call response.raise_for_status before or after processing the stream [9][1]. Best Practices - Always use a context manager (e.g., async with client.stream(...) as response:) to ensure the connection is closed properly after streaming completes [6]. - If you need to access response.content or response.text after streaming, you must either call await response.aread or manually buffer the chunks [6]. - Use try-except blocks around your iteration to catch specific httpx exceptions like DecodingError or network-related ReadError [1][2].
Citations:
- 1: https://github.com/encode/httpx/blob/def4778d/httpx/_exceptions.py
- 2: https://github.com/encode/httpx/blob/master/httpx/_decoders.py
- 3: https://github.com/encode/httpx/blob/master/httpx/_exceptions.py
- 4: Why does aiter_raw raise StreamConsumed but aiter_bytes doesn't ? encode/httpx#2123
- 5: https://github.com/encode/httpx/blob/master/httpx/%5Fmodels.py
- 6: ResponseNotRead and aiter_bytes encode/httpx#3534
- 7: https://github.com/encode/httpx/issues/2960
- 8: https://deepwiki.com/encode/httpx/4.6-content-encoding-and-decoding
- 9: https://www.python-httpx.org/api/
Remove the dead 429 check in the stream error handler.r.aiter_bytes() failures won’t expose a status_code, so this branch can’t fire here, and the upstream 429 path is already handled below. If roster sync should happen on 429, keep it in the non-200 response branch instead.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 2679-2679: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
🤖 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 `@router/main.py` around lines 2679 - 2682, Remove the unreachable status_code
429 handling from the stream error handler around maybe_trigger_roster_sync,
including the model_name check there. Preserve or relocate roster
synchronization for 429 responses within the existing non-200 response handling
branch, where the response status is available.
Source: Linters/SAST tools
Uh oh!
There was an error while loading. Please reload this page.
0a85fdb to
7a5b945Compare
- Integrated OpenRouter free model fetching with Artificial Analysis scoring. - Added detailed "Free Model Roster" table to the dashboard UI. - Implemented opportunistic roster refresh on HTTP 429 rate-limits. - Refactored model selection to use a unified scoring and tool-detection logic. - Verified backend with pytest and frontend with Playwright. Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@router/main.py`:
- Line 2019: Remove the unresolved merge-conflict markers around the affected
section of router/main.py, including the matching ======= and >>>>>>> markers,
and retain the correct implementation from the conflicting branches so the
module parses as valid Python.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| raise HTTPException(status_code=502, detail="Model proxy failed") | ||
| <<<<<<< HEAD |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the unresolved merge conflict marker.
Line 2019 contains <<<<<<< HEAD. This is not valid Python. The whole router/main.py module fails to parse, so the router cannot start. Ruff reports invalid-syntax at this exact line. Check for the matching ======= and >>>>>>> markers further down and resolve the conflict.
🐛 Proposed fix
-<<<<<<< HEAD
async def maybe_trigger_roster_sync(force: bool = False):🧰 Tools
🪛 GitHub Actions: Run Tests / 0_test.txt
[error] 2019-2019: Pytest collection failed because router/main.py contains an unresolved Git merge-conflict marker '<<<<<<< HEAD', causing SyntaxError: invalid syntax. Resolve the conflict markers before rerunning 'CONFIG_PATH=router/config.yaml PYTHONPATH=. pytest --ignore=tests/test_agy_behavior.py --ignore=tests/test_agy_tiers.py --ignore=tests/test_antigravity.py'.
🪛 Ruff (0.16.1)
[warning] 2019-2019: Expected a statement
(invalid-syntax)
[warning] 2019-2019: Expected a statement
(invalid-syntax)
[warning] 2019-2019: Expected a statement
(invalid-syntax)
[warning] 2019-2019: Expected a statement
(invalid-syntax)
🤖 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 `@router/main.py` at line 2019, Remove the unresolved merge-conflict markers
around the affected section of router/main.py, including the matching =======
and >>>>>>> markers, and retain the correct implementation from the conflicting
branches so the module parses as valid Python.
Source: Linters/SAST tools
7a5b945 to
0c1a7c4CompareUh oh!
There was an error while loading. Please reload this page.
🎯 What:
📊 Coverage:
sync_adaptive_router_roster,get_best_free_model, andexecute_proxylogic via existing and new test scenarios.✨ Result:
Fixes#290
PR created automatically by Jules for task 950286063644622268 started by @sheepdestroyer
Summary by CodeRabbit
New Features
Bug Fixes