add-gateway-model: check the provider's docs for rejected parameters - #155
Merged
Conversation
GPT-6 Astra was registered with correct pricing, correct aliases, correct
`responses_api_for_tools`, green unit tests — and 400ed on every single
request, because it rejects `temperature` and nothing in the process ever
sent it one. The skill listed `supports_temperature=False` among the
optional fields but gave no way to find out that it was needed.
Three traps, now written down:
* restrictions don't travel down a family — copying the closest
sibling's ModelConfig is right for pricing shape and request shaping
and wrong for this;
* langchain-openai strips `temperature` itself for names starting with
"gpt-5" and only those, so the gpt-5.6 family worked without the flag
and the first gpt-6 model did not — "the sibling works" proves
nothing;
* reasoning-first flagships are where providers keep dropping the
field (Opus 4.7+, o-series, gpt-5, GPT-6), so assume it's rejected
until a live call says otherwise.
Adds a step 8: send one real request. test_provider_usage_integration
already does exactly that through the chat-controller path with
temperature=0, so point its NEW_CHAT_MODELS entry at the new model and
run it — locally, or via the workflow_dispatch "live-provider-usage" CI
job for anyone without provider keys. If the keys aren't available, the
PR has to say the registration is unverified rather than let silence
imply otherwise.
Also updates NEW_CHAT_MODELS to the actual newest models (gpt-6-astra,
claude-fable-5-1 — it still pointed at gpt-5.6-luna and Sonnet 5), and
fixes that CI job's OpenRouter key: the suite reads OPENROUTER_API_KEY
but the workflow only exported NOUS_API_KEY, so the job died in its own
key check. It now accepts either secret name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019GiCnGpGxLEusqVUS6zUHw
Verification belongs on the provider's API reference, not on a billable call: the reference page, the provider's reasoning-model guide (which lists the unsupported sampling params once for the whole family) and the release changelog are where a removed field is written down, and reading them is a 30-second step in the same visit that confirms pricing. Step 8 is now that read, with what to cite in the PR. The live suite stays mentioned as the fallback for contradictory docs, explicitly not part of a routine addition. Also states the default when the docs are still a stub: register `supports_temperature=False`, since omitting a field the model would have accepted costs a knob no client varies while sending one it rejects costs every request. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GiCnGpGxLEusqVUS6zUHw
adambalogh
marked this pull request as ready for review
September 4, 2026 19:28
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
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.
Why
GPT-6 Astra was added with correct pricing, correct aliases, correct
responses_api_for_tools, and a green unit suite — and 400ed on every request, because it rejectstemperature(fix: #154). The skill already listedsupports_temperature=Falseamong the optional fields, but nothing in the process said to go find out whether it was needed. Pricing gets looked up as a matter of course; the supported-parameter list is on the same docs and nobody reads it.What changed
.claude/skills/add-gateway-model/SKILL.md— new section "Parameter restrictions are the thing that bites", plus a step 8: read the provider's API docs for the model's supported parameters and set the matching flags. Three traps, written down:ModelConfigis right for pricing shape, request shaping andresponses_api_for_tools, and wrong for this.temperaturefor names starting withgpt-5, and only those — so the whole gpt-5.6 family worked without the flag and the firstgpt-6-*model did not. "The sibling works" proves nothing; the sibling may be surviving on a provider-package special case keyed to its name.Where to look, for the specific model: its own reference page, the provider's reasoning-model guide (which lists the unsupported sampling params once for the whole family rather than per model), and the release changelog — where removals land first and often only, in the weeks after a launch. Cite what you found in the PR body next to the pricing citation.
And the default when the docs are still a stub: register
supports_temperature=False. Omitting a field the model would have accepted costs a sampling knob no client varies (every client pinstemperature: 0.0); sending one it rejects costs 100% of its requests.The live suite (
test_provider_usage_integration.py,workflow_dispatchonly) stays mentioned as the fallback for contradictory docs, explicitly not part of a routine model addition.test_provider_usage_integration.py—NEW_CHAT_MODELSnow points at the actual newest models per provider (gpt-6-astra,claude-fable-5-1); it still listedgpt-5.6-lunaand Sonnet 5, so a dispatch was exercising last quarter's flagships..github/workflows/test.yml— thelive-provider-usagejob exportedNOUS_API_KEY, but the suite readsOPENROUTER_API_KEY(the provider was renamed), so the job failed in its own key-check step before running anything. It now accepts either secret name:${{ secrets.OPENROUTER_API_KEY || secrets.NOUS_API_KEY }}. Worth a maintainer's eye — I can't see which secret the repo actually holds; if it's neither, the check step now says so clearly instead of failing later.Merge order
Land after #154. The skill's closing line ("
llm_backendpassesNonefor a flagged field, which every langchain-<provider> package turns into omit-the-key") describes the behavior that PR introduces for non-Anthropic providers.Verification
make lintclean; unit suite green (403 passed, 6 skipped, 117 subtests); workflow YAML parses. No live provider calls were made.🤖 Generated with Claude Code
https://claude.ai/code/session_019GiCnGpGxLEusqVUS6zUHw