Standardized review — 17-lens rubric

Verdict: REQUEST CHANGES — Critical: 0 · High: 2 · Medium: 12 · Low: 8 · Lenses run: 17/17

Solid PR — the exact-registry lookup over prefix matching is the right call given openai.gpt-oss-120b vs openai.gpt-oss-120b-1:0, and the bedrock/ escape hatch keeps stored configs working. These are review comments, not requested changes I have made — nothing has been pushed. Please take or leave each on its merits; several may be deliberate choices I can't see from the diff, and I've flagged where I think the call is genuinely yours.

Claims I verified rather than assumed: 563 passed on 1.96.2 (exact match) · reverting the routing breaks 15 tests, not 14 (the extra is test_mantle_cost_is_resolvable) — they genuinely pin the behaviour · all 9 lockfiles consistent, nothing beyond litellm 1.90.3→1.96.2, aiohttp 3.14.1→3.14.3, +pydantic-settings 2.15.0 · boto3~=1.34.131 matches the existing aws extra exactly · the vendored cohere patch genuinely applies under 1.96.2 (_SKIP_PATCH=False, handler.embedding is _patched_embedding) and all six private litellm imports still resolve · validate() is a true fixed point.

Unanchored findings

Both sit on lines this diff does not touch, so GitHub can't thread them.

[Medium] [Lens 1 · 7] — bedrock.json:184-197 still makes budget_tokens required for the families this PR says it doesn't apply to

The allOf block makes budget_tokens (minimum 1024) required whenever enable_thinking is true, for every Bedrock model, with no family condition. This PR extends that switch to GPT-5.x / GPT-OSS / Nova 2 and its own description at :69 now says "the budget below does not apply" for exactly those — while :190-191 still describes the field as "Claude's internal reasoning". So two descriptions in the same file disagree, and an operator enabling reasoning on openai.gpt-5.6-terra is blocked by form validation until they type a number the adapter then discards.

The honest limit here is that JSON-Schema can't branch on a free-text model id. Minimally: drop budget_tokens from required (keep default) and retitle it Claude-only. Flagging rather than fixing because changing the form contract is your call.

[Medium] [Lens 17] — The repo's own adapter-ops skill restates the Bedrock rules this PR changed, in six places

These matter more than ordinary stale docs: adapter-ops is the documented path for "adding/removing models to existing adapters", so an engineer or agent following it will reproduce the defect this PR fixes.

Not a finding: provider_capabilities.md:32 is the embedding prefix table, and Mantle is LLM-only, so bedrock/ there is still correct.

Two claims I want to retract before you read the comments

I'd rather flag these than let them stand:

1. There is no CI flake. Some intermediate analysis suggested the routing suite failed 15 tests spontaneously "under network contention". It does not: I measured 5/5 clean runs, and it still passes with egress blocked, because the bundled map carries all 13 Mantle entries — so no registry state can produce bedrock/openai.gpt-5.6-terra. That failure only appears when the routing code is reverted. The registry risk in my first comment is latent, not active.
2. The os.environ write at test_bedrock_mantle_routing.py:254 does not leak. monkeypatch.setenv already recorded the pre-test value, so teardown restores it. It's merely dead.

Lens checklist

The two decisions I think are actually yours

1. The routing oracle. Pin LITELLM_LOCAL_MODEL_COST_MAP=True so routing follows the pinned wheel, or keep an in-repo id set with the registry as fallback? Both have real costs and it's a deployment-shape decision, not a code cleanup.
2. Guardrail on a Mantle model — warn or hard-fail? llm.py:313 already turns ValueError into a UI-visible SdkError. Degrading a moderation control on a stdout warning seems the wrong default to me, but it's a product call.

Also worth confirming: were the non-Bedrock providers (Vertex, Azure, Mistral, Ollama, x2text) exercised against 1.96.2 anywhere? The bump crosses six minors and touches every adapter, justified by a Bedrock fix.

Happy to be wrong on any of these — please push back where I've misread the intent.

Approving — verified against ee8f2f5d2

Re-reviewed the follow-up rather than taking the "fixed in" claims at face value. 15 of the findings are fixed and independently confirmed; 5 were declined or deferred with reasoning I accept.

Verified behaviourally, not by reading the diff:

• Both-shapes bug — BOTH PRESENT = False on the original repro and on a Mantle→Claude family switch. Idempotence still holds and reasoning_effort still round-trips.
• Test hermeticity — the one that mattered: the pin now takes in a full-suite run (source=local, is_env_forced=True, 2980 models, previously remote/False/3054). Self-verifying assert means it fails loudly if that ever regresses.
• Dead field — genuinely removed, not repaired in comment only.
• Lens 17 — all six adapter-ops sites corrected, including dropping Bedrock from the thinking-pattern heading and adding the "check before copying" callout. provider_capabilities.md:32 correctly left alone, since the embedding table is right as it stands.
• budget_tokens — required dropped and retitled, picked up from the summary body without a thread prompting it.

Suite green at 565 passed (+2 new tests, both pinning things that were genuinely unpinned).

On the registry finding — your counter-argument is better than my recommendation, and I'm withdrawing it. I framed litellm.model_cost as a routing oracle; you're right that it is simultaneously the pricing oracle, and that pinning it would freeze audit.py pricing and silently record $0 for any model shipped after the pin, across every provider. Trading a latent routing miss for a guaranteed ongoing revenue miss is the worse default, and it isn't a decision a Bedrock routing PR should make. Pinning it in tests while leaving production live is the right split.

One option neither of us raised, for whenever that issue gets written up: the two oracles could be decoupled — resolve the routing membership check against the bundled map (cached once in a module-level frozenset) while leaving litellm.model_cost on the live fetch for pricing. That gets deterministic, version-pinned routing without freezing cost data. Worth weighing there rather than here.

You were also right to reject my breadcrumb suggestion — "not in the Mantle registry" isn't a signal on its own, since Claude, Titan and Nova take that same path.

Open, by agreement — approving does not close these

1. Guardrail runtime signal — the config-time text is now honest, but a model switch that silently disables an already-configured guardrail still produces no tenant-visible signal. Detail in the thread. Note that thread is marked resolved — that's your call and I've left it that way — so this list is the durable record of the open question, not the thread state.
2. Silent reasoning activation — stored adapters with enable_thinking: true on non-Claude models gain reasoning, latency and token spend on deploy. Pending the production count and release notes; gate rather than log if the count is non-trivial.
3. Cohere patch unbounded-timeout degradation on version mismatch — pre-existing, re-armed by the pin move.
4. us./global. detection warning — agreed shape, deferred to its own change.

Approving on the strength of the code in this PR. Nice work on the follow-up — the replies engaged with the substance rather than just the asks, and pushing back on two of them was the right call.