feat: model hub visibility & features + review fixes#16
feat: model hub visibility & features + review fixes#16sheepdestroyer wants to merge 18 commits into
Conversation
Reviewer's GuideImplements full Model Hub visibility and capability metadata for Ollama, OpenRouter, and agent groups by enriching LiteLLM deployment registration, aligning synthetic routing model context lengths, wiring LiteLLM config into the router pod, hardening verification scripts with httpx and status checks, and fixing CI/workflow + startup behaviors per prior review feedback. Sequence diagram for Ollama DB registration during router startupsequenceDiagram
participant Lifespan as lifespan
participant Router as RouterApp
participant Config as LiteLLMConfigFile
participant DB as PostgresDB
participant Admin as LiteLLMAdmin
Lifespan->>Router: _register_ollama_models_in_db(litellm_master_key)
alt [no LITELLM_MASTER_KEY]
Router-->>Lifespan: return (skip registration)
else [master key present]
Router->>Config: load config.yaml via LITELLM_CONFIG_PATH
alt [config parsed]
Router-->>Router: collect ollama-deepseek-* model_list entries
else [config missing/invalid]
Router-->>Router: fall back to static Ollama model definitions
end
Router->>DB: _purge_stale_deployments('ollama-deepseek-%')
DB-->>Router: deleted stale LiteLLM_ProxyModelTable rows
Router->>Admin: POST /model/new (for each ollama-deepseek model)
loop each ollama_models payload
Admin-->>Router: 200/201 or error
end
Router-->>Lifespan: log Ollama DB registration summary
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
More reviews will be available in 12 minutes and 42 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (26)
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements to the routing and model registration pipeline. Key changes include updating model context lengths in the documentation and router, defining explicit model_info capabilities and costs in litellm/config.yaml, and implementing dynamic registration of Ollama models in the database within router/main.py to ensure custom metadata takes precedence. Additionally, verification scripts have been refactored to use httpx instead of urllib.request. Feedback on these changes highlights two improvement opportunities: adding a missing response.raise_for_status() call in verify_direct_ollama_cooldown.py to handle HTTP errors properly, and adding a defensive type check in router/main.py when iterating over model_list to prevent potential AttributeError 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.
| response = httpx.get(METRICS_URL, timeout=5.0) | ||
| lines = response.text.splitlines() |
There was a problem hiding this comment.
The response.raise_for_status() call is missing here, unlike in verify_ollama_cooldown.py. Without it, non-2xx HTTP responses (such as 500 Internal Server Error) will not raise an exception, causing the function to silently fail and return 0 instead of printing the HTTP error.
response = httpx.get(METRICS_URL, timeout=5.0)
response.raise_for_status()
lines = response.text.splitlines()| for item in litellm_config["model_list"]: | ||
| model_name = item.get("model_name", "") | ||
| if model_name.startswith("ollama-deepseek-"): | ||
| # Create a clean deep copy to avoid mutating configuration structures | ||
| ollama_models.append(copy.deepcopy(item)) |
There was a problem hiding this comment.
If model_list contains any non-dictionary items (e.g., due to parsing issues or manual edits), calling item.get() will raise an AttributeError and fail the entire configuration loading process. Adding a defensive check to ensure item is a dictionary prevents this.
| for item in litellm_config["model_list"]: | |
| model_name = item.get("model_name", "") | |
| if model_name.startswith("ollama-deepseek-"): | |
| # Create a clean deep copy to avoid mutating configuration structures | |
| ollama_models.append(copy.deepcopy(item)) | |
| for item in litellm_config["model_list"]: | |
| if isinstance(item, dict): | |
| model_name = item.get("model_name", "") | |
| if model_name.startswith("ollama-deepseek-"): | |
| # Create a clean deep copy to avoid mutating configuration structures | |
| ollama_models.append(copy.deepcopy(item)) |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
verify_direct_ollama_cooldown.py, consider mirroringverify_ollama_cooldown.pyby callingresponse.raise_for_status()inget_triage_request_count()so that HTTP errors don’t get silently treated as zero metrics. - In
verify_ollama_routing.py, you may want to special-casehttpx.HTTPStatusErrorand includee.response.textin the error output (similar to the other verification scripts) to make routing failures easier to diagnose.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `verify_direct_ollama_cooldown.py`, consider mirroring `verify_ollama_cooldown.py` by calling `response.raise_for_status()` in `get_triage_request_count()` so that HTTP errors don’t get silently treated as zero metrics.
- In `verify_ollama_routing.py`, you may want to special-case `httpx.HTTPStatusError` and include `e.response.text` in the error output (similar to the other verification scripts) to make routing failures easier to diagnose.
## Individual Comments
### Comment 1
<location path="litellm/config.yaml" line_range="23-24" />
<code_context>
- langfuse
detailed_debug: false
drop_params: true
+ public_model_groups:
+ - openrouter-auto
+ - llm-routing-ollama
+ - ollama-deepseek-v4-pro
</code_context>
<issue_to_address>
**issue (bug_risk):** The `public_model_groups` list indentation will likely break YAML parsing.
YAML sequence items must be indented further than their key. Here the `- openrouter-auto` line is aligned with `public_model_groups:`, so it won’t be parsed as that key’s value. Instead, indent the list items one level deeper, e.g.:
```yaml
public_model_groups:
- openrouter-auto
- llm-routing-ollama
- ollama-deepseek-v4-pro
```
so the list is correctly associated with `public_model_groups`.
</issue_to_address>
### Comment 2
<location path="scripts/verification/verify_direct_ollama_cooldown.py" line_range="26" />
<code_context>
- for line in lines:
- if line.startswith("triage_requests_total"):
- return int(float(line.split()[1]))
+ response = httpx.get(METRICS_URL, timeout=5.0)
+ lines = response.text.splitlines()
+ for line in lines:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Metrics fetch ignores HTTP status codes, which can hide server-side errors.
In `get_triage_request_count`, non-2xx responses (e.g., 500) are still parsed, so an error page that happens to contain similar text could be misinterpreted as valid metrics. Consider calling `response.raise_for_status()` (as in `verify_ollama_cooldown.py`) so failures on the metrics endpoint are surfaced instead of silently producing misleading counts.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| public_model_groups: | ||
| - openrouter-auto |
There was a problem hiding this comment.
issue (bug_risk): The public_model_groups list indentation will likely break YAML parsing.
YAML sequence items must be indented further than their key. Here the - openrouter-auto line is aligned with public_model_groups:, so it won’t be parsed as that key’s value. Instead, indent the list items one level deeper, e.g.:
public_model_groups:
- openrouter-auto
- llm-routing-ollama
- ollama-deepseek-v4-proso the list is correctly associated with public_model_groups.
| for line in lines: | ||
| if line.startswith("triage_requests_total"): | ||
| return int(float(line.split()[1])) | ||
| response = httpx.get(METRICS_URL, timeout=5.0) |
There was a problem hiding this comment.
suggestion (bug_risk): Metrics fetch ignores HTTP status codes, which can hide server-side errors.
In get_triage_request_count, non-2xx responses (e.g., 500) are still parsed, so an error page that happens to contain similar text could be misinterpreted as valid metrics. Consider calling response.raise_for_status() (as in verify_ollama_cooldown.py) so failures on the metrics endpoint are surfaced instead of silently producing misleading counts.
…le null content safely
…d fix SAST/lint comments
…oldown for llm-routing-ollama on rate limit
LiteLLM Community Edition's deployment cooldown is unreliable for single-deployment model groups and fallback-target groups. The previous approach (multi-deployment replicas, allowed_fails_policy) did not reliably cool down llm-routing-ollama, causing crashloops when Ollama was rate-limited. New approach: the triage router manages Ollama cooldowns internally. When Ollama fails (429/502/503), the router activates a 5-minute cooldown (configurable via OLLAMA_COOLDOWN_SECONDS env var). During cooldown, all Ollama requests are immediately rejected: - Auto modes: silently fall back to the free tier - Direct/fallback mode: return 429 so LiteLLM skips to openrouter-auto Changes: - router/main.py: Add _ollama_cooldown_until state, cooldown check before Ollama proxy call, and activation on failure. Add Prometheus metrics (ollama_cooldown_active, ollama_cooldown_remaining_seconds). - litellm/config.yaml: Remove test-fallback-model, remove duplicate llm-routing-ollama deployment (127.0.0.2), restore Ollama api_base to production (api.ollama.com), remove enterprise-only allowed_fails_policy. - README.md: Update sequence diagrams, Fallback and Cooldown Behavior section, and metrics table to document router-side cooldown.
…ent verification scripts
…ing in sed, expected model asserts in tests, synced metrics documentation
…fecycles, and handle transient exception status codes
… breaker Valkey calls, generic error detail messages, SSE stream token counting, secure YAML rendering, and robust verification script exit codes
- Decouple agy_proxy from main by introducing CooldownPersistence protocol and passing client/cooldown_persistence parameters. - Manage http client lifetime cleanly in agy_proxy. - Reset Valkey client state and cache last init attempt on exceptions to enforce the 5s retry cooldown. - Add isinstance(msg, dict) guards to prevent crashes on malformed payloads. - Use incremental UTF-8 decoder in stream generator to prevent character corruption. - Remove dead should_close_client variables and conditions. - Guard test_antigravity_connection when agentapi is missing. - Fix typo in README.
… Table
- ci: add httpx to test workflow pip install for test_a2_verify.py
- config: add public_model_groups list to litellm_settings so all agent-*,
openrouter-auto, llm-routing-ollama, and ollama-deepseek-* groups appear
in the LiteLLM Model Hub Table UI
- config: add full model_info to all model_list entries (supports_vision,
supports_reasoning, supports_function_calling, mode, max_tokens,
max_input_tokens, is_public_model_group)
- config: set llm-routing-ollama and ollama-deepseek-v4-{pro,flash} context
windows to 512K (524288 tokens), up from 131K/262K
- config: add DeepSeek API equivalent per-token costs to ollama models for
Langfuse cost tracking:
ollama-deepseek-v4-pro: $1.74/1M input, $3.48/1M output
ollama-deepseek-v4-flash: $0.14/1M input, $0.28/1M output
- router: enrich /model/new roster sync payload with model_info (features,
context length from OpenRouter API, is_public_model_group) for all
dynamically registered agent-* tiers
- router: add _register_ollama_models_in_db() — registers ollama-deepseek
models via /model/new at startup so their model_info wins over LiteLLM's
internal ollama_chat provider lookup (which returns null/false for unknown
models in /model_group/info aggregation)
…t capabilities, align returned context lengths, and refactor verify scripts to httpx)
…_status in metrics fetch, detailed HTTP routing diagnostics, and defensive config type checking)
0495855 to
d22047a
Compare
Description
This PR contains the complete implementation for Model Hub visibility and features, incorporating all fixes and feedback from the review of PR #14 and PR #15.
Key Changes
1. Model Hub Visibility & Feature Metadata
/model/newadmin endpoint at startup. This ensures that their capabilities (vision, reasoning, function calling), token limits, and pricing metadata win over LiteLLM's internal defaults (which returnnull/falsefor unrecognized providers likeollama_chatin model group aggregation).llm-routing-*models to match their target downstream limits.litellm/config.yamlso they appear correctly in the Model Hub UI.2. Address Code Reviews (PR #14 & PR #15)
_register_ollama_models_in_db()withLITELLM_ADMIN_URLenv var (defaulting tohttp://127.0.0.1:4000).config.yamlusing theLITELLM_CONFIG_PATHenvironment variable (with fallback checks and a safe hardcoded schema fallback if parsing fails).LITELLM_MASTER_KEYis missing/empty, preventing unnecessary HTTP requests.pod.yamlto mountlitellm-configin thellm-triage-routercontainer and setLITELLM_CONFIG_PATH=/config/litellm_dir/config.yaml.response.raise_for_status()to metrics parsing inverify_ollama_cooldown.pyto prevent masking real HTTP issues.3. CI / Workflow Fixes
branches: [ master ]underpull_requestin.github/workflows/test.ymlso that CI runs and verifies PRs targeting other branches (such asvalkey-global-cooldown-cache-and-cleanups-resolved).Verification & Testing
./start-stack.sh --full-rebuildand verified dynamic Ollama DB registration succeeds.test_circuit_breaker.py,verify_breaker.py, andtest_a2_verify.py) compile and pass successfully.verify_direct_ollama_cooldown.py) functions correctly.Summary by Sourcery
Implement complete Model Hub visibility and capability metadata for OpenRouter and Ollama models, and address related review feedback, configuration, verification, and CI issues.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Deployment:
Documentation:
Tests: