feat(llm): dynamic OpenRouter model catalog for context/max tokens - #11284
Conversation
Add a TTL-cached OpenRouter models catalog for context_length and max_completion_tokens, and clamp gateway completion budgets against live provider ceilings instead of hardcoding them. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_9e191b8f-99b7-4654-a777-bbdb4ae22ff9) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6eacaad7a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| with httpx.Client(timeout=10.0, transport=self._transport) as client: | ||
| response = client.get(self._models_url, headers=headers) |
There was a problem hiding this comment.
Keep catalog refresh off the gateway event loop
On the first OpenRouter request and every TTL refresh, this synchronous httpx.Client.get() is reached from the async _attempt_provider and _prepared_streaming_iterator paths. A slow catalog response can therefore block the gateway event loop for up to the configured timeout, freezing concurrent completions and health checks; in _attempt_provider it also occurs after the provider deadline was calculated, allowing the overall request to exceed that deadline. Use asynchronous HTTP or refresh/offload the cache outside request shaping.
AGENTS.md reference: backend/AGENTS.md:L276-L278
Useful? React with 👍 / 👎.
| except Exception: | ||
| logger.warning('OpenRouter model catalog refresh failed; keeping stale entries', exc_info=True) | ||
| self._expires_at = now + min(60, self._ttl_seconds) | ||
| return |
There was a problem hiding this comment.
Record the catalog refresh fail-open
When the catalog request fails, this branch continues with stale metadata—or no metadata on the initial load—so completion limits may remain outdated or unclamped and OpenRouter can reject requests that the new clamp was intended to protect. The warning is not recorded through the shared fallback metric, leaving operators unable to measure this degraded correctness path; call record_fallback with the llm_gateway component and an appropriate degraded outcome.
AGENTS.md reference: backend/AGENTS.md:L334-L334
Useful? React with 👍 / 👎.
| return apply_openrouter_completion_clamp( | ||
| provider_request, | ||
| provider=provider_ref.provider, | ||
| model=provider_ref.model, | ||
| ) |
There was a problem hiding this comment.
Isolate catalog I/O from the unit-test paths
For every OpenRouter request, this unconditional global-catalog call also runs in existing unit files such as test_llm_gateway_executor.py, test_llm_gateway_openai_compatible.py, and test_public_shared_conversation_chat.py, none of which replaces the catalog. Since test.sh executes files in isolated processes, each file gets a cold cache and can issue its own live GET /api/v1/models; unavailable networking adds the 10-second failure delay, while a reachable service makes the unit suite depend on mutable external data. Inject or centrally fake the catalog for these unit paths so live-provider coverage remains in integration tests.
AGENTS.md reference: backend/AGENTS.md:L227-L227
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
4 issues found across 3 files
Confidence score: 2/5
- In
backend/utils/llm/openrouter_model_catalog.py, the cache-freshness checks gate on nonempty models, so after a cold-start refresh failure the 60-second backoff is bypassed and refresh is retried on every request; this can create repeated upstream calls and request-path instability — make_expires_atenforcement independent of cache contents. - In
backend/utils/llm/openrouter_model_catalog.py(_fetch_models()called from async_provider_request), using synchronoushttpx.Client.get()can block the event loop for up to the timeout, increasing tail latency and reducing gateway throughput under load — switch to an async client path or run blocking I/O off the loop. - In
backend/llm_gateway/gateway/executor.py, clamp key construction viaopenrouter_provider_model_nameonly covers a subset of vendor prefixes, so many OpenRouter models may never get clamped and could exceed intended limits — broaden/normalize key mapping across vendors and add coverage tests for non-gemini/gpt/o* models. - In
backend/utils/llm/openrouter_model_catalog.py, refresh failures currently only warn and then silently use stale/empty metadata, making degraded unclamped behavior hard to detect operationally — add explicit metrics/counters (and alerting hooks) for refresh failures and fallback usage.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/utils/llm/openrouter_model_catalog.py">
<violation number="1" location="backend/utils/llm/openrouter_model_catalog.py:76">
P1: A cold-cache refresh failure is retried on every OpenRouter request instead of observing the 60-second backoff, because both freshness guards require nonempty models. Honor `_expires_at` independently of cache contents so an unavailable catalog does not repeatedly delay requests.</violation>
<violation number="2" location="backend/utils/llm/openrouter_model_catalog.py:85">
P2: When the catalog refresh fails, this only logs a warning and silently falls back to stale (or empty) metadata, so completion limits can stay outdated/unclamped without any way to measure this degraded path in metrics. Consider recording this fallback through the shared `record_fallback` mechanism used elsewhere for the `llm_gateway` component so operators can observe when clamping is running on stale data.</violation>
<violation number="3" location="backend/utils/llm/openrouter_model_catalog.py:96">
P2: The new OpenRouter clamp injects a synchronous, blocking HTTP fetch into the async gateway serving path. `_fetch_models()` uses `httpx.Client.get()` (10s timeout) inside `_provider_request`, which runs on the asyncio event loop via `_attempt_provider`. When the process-local catalog is cold or the 1h TTL has elapsed, that blocking network call stalls the whole event loop for up to 10 seconds, freezing every concurrent request the gateway is handling. Consider fetching/refreshing the catalog asynchronously (or in a background task before it's needed), so the common request path never performs a blocking network round-trip.</violation>
</file>
<file name="backend/llm_gateway/gateway/executor.py">
<violation number="1" location="backend/llm_gateway/gateway/executor.py:441">
P2: The clamp only takes effect for a narrow set of models. The lookup key is built by `openrouter_provider_model_name`, which only vendor-prefixes gemini*/gpt*/o1/o3/o4 models; for any other OpenRouter vendor (anthropic, meta-llama, deepseek, mistralai, etc.) it returns the bare model, which won't match the catalog keys (full API ids like `anthropic/claude-...`), so `clamp_completion_tokens` finds no limits and the requested values pass through unclamped. The feature is fail-open (no error), but completion-ceiling protection won't apply to most third-party providers. If coverage is intended beyond the allowlisted families, the model-id resolution used for the catalog lookup should be robust to all vendor prefixes rather than the current subset.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| def _ensure_fresh(self) -> None: | ||
| now = time.monotonic() | ||
| if self._models and now < self._expires_at: |
There was a problem hiding this comment.
P1: A cold-cache refresh failure is retried on every OpenRouter request instead of observing the 60-second backoff, because both freshness guards require nonempty models. Honor _expires_at independently of cache contents so an unavailable catalog does not repeatedly delay requests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/utils/llm/openrouter_model_catalog.py, line 76:
<comment>A cold-cache refresh failure is retried on every OpenRouter request instead of observing the 60-second backoff, because both freshness guards require nonempty models. Honor `_expires_at` independently of cache contents so an unavailable catalog does not repeatedly delay requests.</comment>
<file context>
@@ -0,0 +1,194 @@
+
+ def _ensure_fresh(self) -> None:
+ now = time.monotonic()
+ if self._models and now < self._expires_at:
+ return
+ with self._lock:
</file context>
| api_key = os.getenv('OPENROUTER_API_KEY', '').strip() | ||
| if api_key: | ||
| headers['Authorization'] = f'Bearer {api_key}' | ||
| with httpx.Client(timeout=10.0, transport=self._transport) as client: |
There was a problem hiding this comment.
P2: The new OpenRouter clamp injects a synchronous, blocking HTTP fetch into the async gateway serving path. _fetch_models() uses httpx.Client.get() (10s timeout) inside _provider_request, which runs on the asyncio event loop via _attempt_provider. When the process-local catalog is cold or the 1h TTL has elapsed, that blocking network call stalls the whole event loop for up to 10 seconds, freezing every concurrent request the gateway is handling. Consider fetching/refreshing the catalog asynchronously (or in a background task before it's needed), so the common request path never performs a blocking network round-trip.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/utils/llm/openrouter_model_catalog.py, line 96:
<comment>The new OpenRouter clamp injects a synchronous, blocking HTTP fetch into the async gateway serving path. `_fetch_models()` uses `httpx.Client.get()` (10s timeout) inside `_provider_request`, which runs on the asyncio event loop via `_attempt_provider`. When the process-local catalog is cold or the 1h TTL has elapsed, that blocking network call stalls the whole event loop for up to 10 seconds, freezing every concurrent request the gateway is handling. Consider fetching/refreshing the catalog asynchronously (or in a background task before it's needed), so the common request path never performs a blocking network round-trip.</comment>
<file context>
@@ -0,0 +1,194 @@
+ api_key = os.getenv('OPENROUTER_API_KEY', '').strip()
+ if api_key:
+ headers['Authorization'] = f'Bearer {api_key}'
+ with httpx.Client(timeout=10.0, transport=self._transport) as client:
+ response = client.get(self._models_url, headers=headers)
+ response.raise_for_status()
</file context>
| @@ -38,6 +38,7 @@ | |||
| RouteServingClass, | |||
There was a problem hiding this comment.
P2: The clamp only takes effect for a narrow set of models. The lookup key is built by openrouter_provider_model_name, which only vendor-prefixes gemini*/gpt*/o1/o3/o4 models; for any other OpenRouter vendor (anthropic, meta-llama, deepseek, mistralai, etc.) it returns the bare model, which won't match the catalog keys (full API ids like anthropic/claude-...), so clamp_completion_tokens finds no limits and the requested values pass through unclamped. The feature is fail-open (no error), but completion-ceiling protection won't apply to most third-party providers. If coverage is intended beyond the allowlisted families, the model-id resolution used for the catalog lookup should be robust to all vendor prefixes rather than the current subset.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/llm_gateway/gateway/executor.py, line 441:
<comment>The clamp only takes effect for a narrow set of models. The lookup key is built by `openrouter_provider_model_name`, which only vendor-prefixes gemini*/gpt*/o1/o3/o4 models; for any other OpenRouter vendor (anthropic, meta-llama, deepseek, mistralai, etc.) it returns the bare model, which won't match the catalog keys (full API ids like `anthropic/claude-...`), so `clamp_completion_tokens` finds no limits and the requested values pass through unclamped. The feature is fail-open (no error), but completion-ceiling protection won't apply to most third-party providers. If coverage is intended beyond the allowlisted families, the model-id resolution used for the catalog lookup should be robust to all vendor prefixes rather than the current subset.</comment>
<file context>
@@ -434,7 +435,11 @@ def _provider_request(
+ return apply_openrouter_completion_clamp(
+ provider_request,
+ provider=provider_ref.provider,
+ model=provider_ref.model,
+ )
</file context>
| try: | ||
| models = self._fetch_models() | ||
| except Exception: | ||
| logger.warning('OpenRouter model catalog refresh failed; keeping stale entries', exc_info=True) |
There was a problem hiding this comment.
P2: When the catalog refresh fails, this only logs a warning and silently falls back to stale (or empty) metadata, so completion limits can stay outdated/unclamped without any way to measure this degraded path in metrics. Consider recording this fallback through the shared record_fallback mechanism used elsewhere for the llm_gateway component so operators can observe when clamping is running on stale data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/utils/llm/openrouter_model_catalog.py, line 85:
<comment>When the catalog refresh fails, this only logs a warning and silently falls back to stale (or empty) metadata, so completion limits can stay outdated/unclamped without any way to measure this degraded path in metrics. Consider recording this fallback through the shared `record_fallback` mechanism used elsewhere for the `llm_gateway` component so operators can observe when clamping is running on stale data.</comment>
<file context>
@@ -0,0 +1,194 @@
+ try:
+ models = self._fetch_models()
+ except Exception:
+ logger.warning('OpenRouter model catalog refresh failed; keeping stale entries', exc_info=True)
+ self._expires_at = now + min(60, self._ttl_seconds)
+ return
</file context>
|
Thanks for this — the dynamic catalog is a clean approach to keeping provider limits accurate without hardcoding them in route config. Reviewed all 3 changed files: backend/utils/llm/openrouter_model_catalog.py — Solid implementation. Thread-safe TTL cache with double-checked locking (lines 74-89), fail-open design that retains stale entries when refresh fails (line 85), and defensive parsing that correctly rejects bools from backend/llm_gateway/gateway/executor.py — The clamp is applied as the final transform in backend/tests/unit/test_openrouter_model_catalog.py — Good coverage across the core paths: limit loading, clamping to provider ceiling, non-OpenRouter passthrough, stale retention on refresh failure, and non-JSON rejection. The The 5 new tests pass (verified locally). The Backend unit suite failure is present on the parent branch (#11283 Leaving for human maintainer review: this is a stacked PR making an architectural decision about whether Omi should source LLM model limits dynamically from OpenRouter rather than hardcoding them, and it touches provider routing in the gateway executor. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
Resolution: `_provider_request` applies both sides — sanitize the params OpenAI rejects for GPT-5.6 (parent branch), then clamp the completion budget to the catalog's limits (this branch). Neither replaces the other. Also allowlist the catalog's OPENROUTER_API_KEY read in the direct-provider coverage guardrail: it fetches /api/v1/models for context and completion limits, so it is metadata rather than product text and does not belong on a gateway lane. Verified: backend/.venv/bin/python -m pytest on every tests/unit file matching openrouter or llm_gateway — 360 passed. Failure-Class: none Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f87afe96-9d40-4082-b559-a4e1da91d3d9) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6e45d4625
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def _ensure_fresh(self) -> None: | ||
| now = time.monotonic() | ||
| if self._models and now < self._expires_at: |
There was a problem hiding this comment.
Honor the retry window when the first refresh fails
When the initial catalog fetch fails, _models remains empty, so this condition and its locked duplicate ignore the _expires_at retry window assigned by the exception branch. Every OpenRouter request carrying an output cap therefore starts another synchronized 10-second fetch instead of backing off for up to 60 seconds; because request shaping runs on the async gateway path, a catalog-only outage can repeatedly block and serialize otherwise viable completions. Track freshness independently of whether the cache contains entries so the failed-refresh window is honored.
AGENTS.md reference: backend/AGENTS.md:L276-L278
Useful? React with 👍 / 👎.
…sts (#11360) ## Summary Reverts the two OpenRouter PRs: #11283 (managed product text → OpenRouter Luna) and #11284 (dynamic OpenRouter model catalog). **No `OPENROUTER_API_KEY` is provisioned.** As merged, every managed product-text feature — chat, memories, knowledge graph, conversation processing, goals, notifications, wrapped — resolves to an OpenRouter route and would fail at the provider, and `llm_gateway/routers/health.py` requires that key before reporting ready, so the gateway would never pass readiness. Routing config returns exactly to its pre-#11283 state (`git diff` against the commit before that merge is empty for `model_config.py`, `llm_gateway/config/`, `health.py` and `clients.py`). ## Kept The SSOT work from #11286 and #11325 stays: `/v1/knowledge-graph/extract`, `/v1/memories/extract`, `/v1/connectors/synthesize`, `/v1/conversations/topic`, `/v1/users/ai-profile/synthesize` and deterministic KG ids. Those route through `get_llm(feature)`, so they follow whatever provider `model_config` names — now direct OpenAI/Anthropic again — and keep working. ## Product invariants affected - INV-AGENT-* - INV-CHAT-1 - INV-MEM-1 ## Failure class (fixes) Failure-Class: none ## Test plan - [x] `backend/test.sh` over the 41 gateway/qos/openrouter/SSOT-endpoint test files — all pass file-by-file (one pre-existing fast-unit CPU-time guard trip on `test_llm_gateway_deploy_contract.py`, 13/13 assertions pass, unrelated to this diff) - [x] Routing config byte-identical to pre-#11283 for `model_config.py`, `llm_gateway/config/`, `health.py`, `clients.py` - [ ] Dev backend deploy reports ready without `OPENROUTER_API_KEY` <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/BasedHardware/omi/pull/11360?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **High Risk** > Touches core LLM routing, gateway provider execution, readiness, and BYOK behavior for all managed text features; wrong config would break chat and background LLM workloads at scale. > > **Overview** > **Reverts managed product text routing from OpenRouter back to direct providers** so the stack can run without `OPENROUTER_API_KEY` and gateway `/ready` no longer blocks on that credential. > > `model_config` and gateway **generated route overrides** again send most features to **direct OpenAI** (`gpt-5.6-luna` / `gpt-5-nano`), **Gemini** for former flash-lite workloads, **Anthropic** for `chat_agent`, and **OpenRouter** only for `wrapped_analysis`. Inventory, route artifacts, and cost cards are aligned with that map; OpenRouter Luna/nano rate cards are removed. > > The gateway **executor** drops OpenRouter-specific request shaping: no BYOK vendor remapping on OpenRouter routes, no OpenRouter completion clamp, and GPT-5.6 sanitization applies only to `openai` provider refs. **Health** reports `managed_chat_provider: openai` and requires `OPENAI_API_KEY` instead of OpenRouter. > > **Deleted** the dynamic OpenRouter model catalog and shared vendor-prefix helpers; synthetics/replay harnesses register **`openai`** fakes directly again. QoS and gateway unit/integration tests are updated to match the pre–OpenRouter managed-text expectations. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3b0aa79. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…okens (BasedHardware#11284)" This reverts merge commit 52ed1b8 (PR BasedHardware#11284) via -m 1. The catalog reads OPENROUTER_API_KEY to clamp completion budgets against OpenRouter's model limits. With managed text going back to direct OpenAI it has no caller, and no OpenRouter key exists to serve it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
OpenRouterModelCatalogthat fetches/caches OpenRouterGET /api/v1/modelsforcontext_length,max_completion_tokens, andsupported_parameters.backend-ssot-openrouter).Test plan
tests/unit/test_openrouter_model_catalog.pyviatest.shtests/unit/test_llm_gateway_executor.pystill greenOPENROUTER_API_KEY, confirmopenai/gpt-5.6-lunaresolves with non-null context/max completionMade with Cursor
Failure class (fixes)
Failure-Class: none
The
fix:commits here come from the merged parent branch; this PR adds the dynamicOpenRouter model catalog and its clamp.