[codex] Gate providers and add Bedrock discovery#37
Conversation
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughAdds Bedrock model discovery and AWS pricing support, wires Bedrock providers into the registry and live config, adds provider-auth enforcement to ChangesBedrock Source, Auth Enforcement, and OpenRouter Filtering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sources/openrouter.py (1)
139-155: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCompute inline benchmark ranks from routable models only.
_live_traitsis ranked before the unavailable models are filtered out. Those dropped models still consumebench_*_rankslots, so top-k policies can reject a live offer solely because an unavailable model ranked ahead of it in the discarded set.Suggested fix
- live = {m["id"]: self._traits_for(m) for m in self._models_snapshot if m.get("id")} + live = { + m["id"]: self._traits_for(m) + for m in self._models_snapshot + if m.get("id") and self._model_available(m["id"]) + } self._add_ranks(live) self._live_traits = live🤖 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 `@sources/openrouter.py` around lines 139 - 155, The `pricing` flow in `sources.openrouter.py` builds and ranks `_live_traits` from the full snapshot before unavailable models are removed, which lets unroutable models consume `bench_*_rank` slots. Filter the snapshot to routable/available models first, then build `live` and call `_add_ranks` on that filtered set so ranks used by `_offer_for`, `offers_sync`, and `market_book` only reflect models that can actually be served.
🤖 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 `@config.live.lua`:
- Around line 96-97: The fallback `base_url` in `config.live.lua` is hardcoded
to `us-east-1`, which can mismatch `BEDROCK_REGION`. Update the `base_url`
fallback to be derived from `BEDROCK_REGION` so model discovery and pricing stay
in the same region. Use the existing `BEDROCK_MANTLE_BASE_URL` override in the
same config block, and make the fallback construction align with the region used
by `BedrockSource`.
In `@llm_router_host.py`:
- Around line 603-619: The auth-unconfigured handling in llm_router_host.py is
overwriting and later deleting existing disable states for providers that were
disabled for other reasons. Update the missing-provider loop and the cleanup
loop around the disabled map so _AUTH_UNCONFIGURED is only added when no other
disable reason exists, and only removed when the current entry was created by
this auth marker. Keep the logic scoped to the existing disabled dictionary
entries and preserve any non-auth disable reason unchanged.
In `@sources/bedrock.py`:
- Around line 220-223: The fallback in the Bedrock refresh flow is too broad:
when `_prices_by_family()` fails, `prices` is replaced with an empty dict and
every discovered model is treated as unpriced. Update the logic in the Bedrock
pricing refresh path (around `Bedrock` and `_prices_by_family`) so a pricing
fetch failure does not clear the catalog; instead preserve the last known
pricing data or skip the refresh for that region and surface the failure without
dropping existing Bedrock rows/offers.
In `@sources/openrouter.py`:
- Around line 65-79: The `_url_for` helper currently accepts any absolute URL,
which allows `links.details` from the `/models` payload to redirect requests off
OpenRouter. Update `_url_for` in `sources.openrouter.py` to only allow absolute
URLs that match the OpenRouter origin/base host, and reject or ignore foreign
hosts; keep the existing normalization for relative `/api/v1/...` paths before
joining with `self._base_url`. Ensure `_get` continues to call `_url_for` so the
refresh flow only fetches same-origin detail links.
---
Outside diff comments:
In `@sources/openrouter.py`:
- Around line 139-155: The `pricing` flow in `sources.openrouter.py` builds and
ranks `_live_traits` from the full snapshot before unavailable models are
removed, which lets unroutable models consume `bench_*_rank` slots. Filter the
snapshot to routable/available models first, then build `live` and call
`_add_ranks` on that filtered set so ranks used by `_offer_for`, `offers_sync`,
and `market_book` only reflect models that can actually be served.
🪄 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: bdf7c567-bbf5-4be8-9157-69d6a387d875
📒 Files selected for processing (10)
.env.exampleconfig.live.luallm_router_host.pysources/__init__.pysources/bedrock.pysources/openrouter.pytests/test_async_concurrency.pytests/test_live_wiring.pytests/test_provider_filter_flow.pytests/test_sources.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_sources.py`:
- Around line 537-544: The cache-retention check around source.pricing() and
source.offers_sync("bedrock_mantle_market") is currently vacuous if the first
refresh returns no offers. Add a precondition in this test after the initial
refresh and before simulating the 500 response so it verifies the cached offers
are non-empty (or otherwise meaningfully populated) before asserting they are
preserved after the failed refresh. Use the existing source.pricing(),
source.offers_sync(), and pytest.raises(RuntimeError) flow to keep the test
focused on cache retention.
🪄 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: 8b4ccc65-de36-4869-b41e-23a31e49926b
📒 Files selected for processing (6)
config.live.luallm_router_host.pysources/bedrock.pysources/openrouter.pytests/test_live_wiring.pytests/test_sources.py
🚧 Files skipped from review as they are similar to previous changes (4)
- config.live.lua
- llm_router_host.py
- sources/openrouter.py
- sources/bedrock.py
MuncleUscles
force-pushed
the
codex/provider-availability-bedrock
branch
2 times, most recently
from
8cdf6fc to
42bdeb0
Compare
MuncleUscles
force-pushed
the
codex/provider-availability-bedrock
branch
from
42bdeb0 to
81aed10
Compare
…s a pre-merge branch commit)
2 participants
Summary
Core dependency
Validation