fix: only send reasoning.effort to models that support it#54
fix: only send reasoning.effort to models that support it#54shehabyasser-scale wants to merge 3 commits into
Conversation
Validated live, with a same-model A/BBoth halves of the guard are now confirmed against real runs, using the same agent model on both sides so the only variable is this patch. Without the guard — GAIA, 2026-07-24, gpt-4o agent
715 HTTP 400s — roughly 30% of every inference call that run made, thrown away on a parameter the model does not accept. With the guard — swe-atlas-qna, 2026-07-25, gpt-4o agent
Zero 400s. Zero non-200 responses of any kind. The negative case is confirmed tooThe predicate must not quietly strip
So: non-reasoning models lose the parameter, reasoning models keep it. One thing worth flagging for reviewThe guard is five copy-pasted lines in five separate bench agents, with no shared helper and no unit test — the evidence above is all end-to-end. That is a fair reviewer objection. I did not refactor it into a helper because these files are the candidate programs under optimization: the optimizer edits them, and a shared import outside the candidate repo would change what the candidate can see and modify. Worth a deliberate decision rather than an accident, though. This is also no longer optional. Per #58, |
gaia and swe-bench-pro send `reasoning: {effort: ...}` unconditionally. A
non-reasoning model rejects it with HTTP 400, so every call fails and the
case is scored as an honest-looking task failure. Measured on a live gaia
run: 715 of 2376 calls returned 400 (514 evaluation, 201 finalization).
Gate it on the model being reasoning-capable. No behavior change for
reasoning models; gpt-4o stops 400ing.
Scoped to the two agents that have no gate at all. officeqa, tau3 and
swe-atlas-qna gained `if "fireworks" not in self._api_model` in b7a5b80,
so their hunks are dropped here rather than rewritten. That gate is
provider-shaped rather than capability-shaped and still sends
reasoning_effort to any non-Fireworks non-reasoning model such as Azure
gpt-4o, which is raised as review on the PR rather than changed unilaterally.
Refs #51.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
ccc7d07 to
ec708e9
Compare
28085f3 to
4c0cd19
Compare
|
Restacked and narrowed from five agents to two.
One thing to flag rather than change unilaterally, since it is your code and a judgement call: if "fireworks" not in self._api_model:
kwargs["reasoning_effort"] = "medium"That gate is provider-shaped, not capability-shaped. It correctly spares Fireworks-served open models, but it still sends _model = self._api_model.lower()
if _model.startswith(("gpt-5", "o1", "o3", "o4")) or "codex" in _model:which also excludes Live measurement behind the PR, unchanged: gpt-4o without a guard on gaia burned 715 HTTP 400s of 2376 calls (514 evaluation + 201 finalization); gpt-4o with the guard on swe-atlas-qna logged Related and separate, worth its own look: gaia is the one benchmark now pointed at a Fireworks model while still on the Responses API with a hosted Full suite matches base: 11 pre-existing failures, none new. |
Restores the hunks dropped when this PR was restacked. b7a5b80 added `if "fireworks" not in self._api_model` to officeqa, tau3 and swe-atlas-qna, which is provider-shaped rather than capability-shaped: it correctly spares Fireworks-served open models and still sends `reasoning_effort` to any non-Fireworks model that cannot accept it. That is not theoretical. Measured today against the configured Azure endpoint, running the swe-atlas-qna seed agent over its 50-case held-out split: gpt-5.3-codex, provider gate 50/50 BadRequestError 400 "The requested operation is unsupported" (that model has no chat/completions surface on this resource at all) gpt-4o, provider gate 50/50 BadRequestError 400 "Unrecognized request argument supplied: reasoning_effort" gpt-4o, capability gate 0 BadRequestError, agent inference succeeded So the provider gate makes these three benchmarks unrunnable against any non-Fireworks upstream, which is every endpoint we currently have. The capability form also excludes `fireworks_ai/*`, so it is a strict superset of the current behaviour and changes nothing for the default configuration. officeqa gates two arguments on that one condition. Only `reasoning_effort` moves: `parallel_tool_calls` is a separate axis that Fireworks rejects and gpt-4o supports, so it keeps the provider check. Refs #51. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
|
Restoring the three hunks I dropped, because the gap turned out to be load-bearing rather than stylistic. I said above that the provider gate "will bite whoever points this at OpenAI or Azure". It bit today, and it is worse than I described: it makes three of the five benchmarks unrunnable on any non-Fireworks upstream. Measured against the configured Azure endpoint, running the swe-atlas-qna seed agent over its full 50-case held-out split:
Two separate things conspire, and the second is the one worth knowing:
So the intersection of "model this endpoint serves on Chat Completions" and "model the provider gate does not break" is empty. The capability gate is the only thing that makes swe-atlas-qna, tau3 and officeqa runnable here. The change is a strict superset of current behaviour: officeqa needed care. It gates two arguments on the one condition. Only Verified after the change: all three files compile, and the follow-on run reached the verifier stage with zero |
The four Chat Completions agents send `max_tokens` unconditionally. Every gpt-5 model rejects it: "Unsupported parameter: 'max_tokens' is not supported with this model. Use 'max_completion_tokens' instead." So even with the reasoning_effort gate corrected, these agents could not target any modern OpenAI reasoning model, only Fireworks-served models and gpt-4o. Select the parameter name from the same capability test, and fold the two copies of that test into one `_is_reasoning_model` helper per agent so they cannot drift apart. browsecomp-plus's reasoning_effort check moves onto the helper as well; it had the same provider-shaped gate as the other three. Verified live against the configured Azure endpoint, sending exactly the shapes the patched agents now build: gpt-4o ['max_tokens'] 200 gpt-5.4-mini-2026-03-17 ['max_completion_tokens','reasoning_effort'] 200 and the predicate classifies the suite's own targets correctly: `fireworks_ai/deepseek-v4-flash` and `fireworks_ai/gpt-oss-120b` are both non-reasoning, so the default configuration keeps the legacy shape and is unaffected. gaia and swe-bench-pro are untouched here: they use the Responses API, whose `max_output_tokens` has no such split. Refs #51. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Why this is now load-bearing, not defensive
This started as cleanup for a 400 seen during a local gpt-4o swap. It is now a prerequisite, because gpt-4o is the only usable eval model on the configured Azure resource.
Probing the endpoint directly, only two deployments answer —
gpt-5.3-codexandgpt-4o. Everything else, including the committedgpt-5.4-mini-2026-03-17that four of the five benchmarks point at, returns404 DeploymentNotFound(details in #58). So moving swe-atlas-qna to gpt-4o is the only way to get it running at all, and without this guard every one of its agent calls trades a 404 for:The change
Each of the five bench agents built an unconditional
"reasoning": {"effort": ...}into its Responses request. Now it is sent only when the model supports it:The predicate runs on
_api_model, which ismodel_name.removeprefix("openai/")(e.g.atlas_agent/agent.py:80), so theopenai/prefix carried inbuild.yamlnever defeats thestartswith.Both branches matter and both are covered by the run below:
gpt-4o→ noreasoningkey → the 400 cannot occur.gpt-5.3-codex(producer) → matches on"codex"→ still receiveseffort.gpt-5.4-mini-2026-03-17→ matches ongpt-5. So no reasoning model silently loses its effort setting.Verification
Live combined run in flight on swe-atlas-qna with a gpt-4o agent, on a branch carrying this plus #51/#52/#53. The check is
session/artifacts/inference/requests/*.jsonl: zero records with status 400 /unsupported_parameter, against 322/322 upstream failures in the last run. Posting the counts here before merge.Greptile Summary
This PR replaces a provider-name heuristic (
"fireworks" not in model) with a capability-based predicate (_is_reasoning_model) that guardsreasoning.effort(and themax_completion_tokensvsmax_tokensparameter name) on all six bench agents. The fix is a prerequisite for running swe-atlas-qna against gpt-4o, which was returning HTTP 400unsupported_parameteron every call due to the unconditionalreasoningblock.browsecomp-plus,officeqa,atlas,tau3) define a shared_is_reasoning_model()helper; the two Responses-API agents (gaia,swe-bench-pro) inline the same predicate. Both are correct but create six maintenance points for the same logic.officeqacorrectly splits the old combined guard into two independent checks so thatgpt-4ogetsparallel_tool_calls=False(which it supports) withoutreasoning_effort(which it rejects).Confidence Score: 5/5
Safe to merge — the predicate is correct, both API shapes (Chat Completions and Responses) are handled, and the officeqa guard split is accurate.
The change is narrowly scoped: a boolean predicate replaces a provider-name string check in six agents, and the affected parameter (reasoning.effort) is simply omitted rather than set to a wrong value when the model does not support it. The logic has been verified against live runs described in the PR. The only structural gap is that two of the six agents inline the predicate instead of using the helper defined in the other four, which is a maintainability concern but not a correctness issue.
Files Needing Attention: gaia/agent.py and swe-bench-pro/agent.py inline the predicate rather than using a helper — worth unifying in a follow-up if new reasoning model families need to be added.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Agent builds request dict] --> B{_is_reasoning_model?} B -- "Yes: gpt-5*, o1*, o3*, o4*, *codex*" --> C[token key = max_completion_tokens] B -- "No: e.g. gpt-4o" --> D[token key = max_tokens] C --> E[Add reasoning effort param] D --> F[Skip reasoning param] E --> G{Uses Responses API?} F --> G G -- "Yes: gaia / swe-bench-pro" --> H[reasoning effort string] G -- "No: browsecomp / officeqa / atlas / tau3" --> I[reasoning_effort string] H --> J[Send request] I --> J J --> K{officeqa only} K -- "fireworks in model" --> L[skip parallel_tool_calls] K -- "not fireworks" --> M[parallel_tool_calls = False] L --> N[API call] M --> NReviews (4): Last reviewed commit: "fix: pick the token-limit parameter by m..." | Re-trigger Greptile