Repository navigation
fix(gateway): forward reasoning_effort to Together AI - #3367
Merged
Merged
Conversation
The together-ai case in prepare-request-body never wrote reasoning_effort
and broke before the default case's generic forwarder, so the parameter
was silently dropped for every Together mapping — including the gateway's
own auto-routing default. deepseek-v4-pro declared reasoningEfforts for
together-ai that could never take effect.
Verified live against api.together.ai: Together is OpenAI-compatible on
reasoning_effort, but disabling thinking is not uniform. The gpt-oss and
Gemma deployments validate reasoning_effort (naming their accepted
literals in the 400) and take "none" there, while thinking is ignored.
The DeepSeek V4 Pro, MiniMax M3 and Kimi deployments accept any
reasoning_effort string without complaint and keep thinking on; only
thinking: { type: "disabled" } turns it off. Mappings on the second stack
now set requiresDisableThinkingParam.
Each mapping's reasoningEfforts is set from measured behaviour:
upstream-validated sets for gpt-oss (low/medium/high) and Gemma
(none/low/medium/high), and for the rest the tiers that measurably change
the reasoning length (deepseek-v4-pro: none/xhigh/max; minimax-m3 and
kimi-k2.6/k3: none only).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
WalkthroughTogether AI request construction now forwards supported ChangesTogether AI reasoning
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant prepareRequestBody
participant TogetherAI
Client->>prepareRequestBody: Send reasoning_effort
prepareRequestBody->>TogetherAI: Forward effort or thinking.disabled
TogetherAI-->>Client: Return model response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/actions/src/prepare-request-body.spec.ts (1)
4811-4842: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the broad
anyassertion.Use a narrow request-body type for
reasoning_effortandthinking. The tests only require these fields.Proposed fix
- ) as Promise<any>; + ) as Promise<{ + reasoning_effort?: string; + thinking?: { type: "disabled" }; + }>;As per coding guidelines,
**/*.{ts,tsx}must not useanyoras anyunless absolutely necessary.🤖 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 `@packages/actions/src/prepare-request-body.spec.ts` around lines 4811 - 4842, Replace the broad `as Promise<any>` assertion in togetherReasoning with a narrow request-body type containing only reasoning_effort and thinking, and use that type for the promise assertion. Preserve the existing test inputs and behavior while removing any usage.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@packages/actions/src/prepare-request-body.spec.ts`:
- Around line 4811-4842: Replace the broad `as Promise<any>` assertion in
togetherReasoning with a narrow request-body type containing only
reasoning_effort and thinking, and use that type for the promise assertion.
Preserve the existing test inputs and behavior while removing any usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 77218cf2-765a-4938-a7dc-0f7262433b85
📒 Files selected for processing (8)
packages/actions/src/prepare-request-body.spec.tspackages/actions/src/prepare-request-body.tspackages/models/src/models.tspackages/models/src/models/deepseek.tspackages/models/src/models/google.tspackages/models/src/models/minimax.tspackages/models/src/models/moonshot.tspackages/models/src/models/openai.ts
There was a problem hiding this comment.
Pull request overview
This PR fixes Together AI support for the OpenAI-compatible reasoning_effort parameter in the gateway request builder, and updates Together-specific model catalog metadata so reasoning_effort: "none" can correctly disable thinking per Together’s serving stack behavior.
Changes:
- Forward
reasoning_effortfortogether-ai(and translate"none"to eitherreasoning_effort: "none"orthinking: { type: "disabled" }depending on mapping). - Add
requiresDisableThinkingParamto provider mappings and set Together mappings’reasoningEffortsbased on observed Together behavior. - Add unit tests covering Together’s “none” translation and forwarding behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/actions/src/prepare-request-body.ts | Adds Together-specific forwarding/translation for reasoning_effort, and treats Together as a provider that must preserve "none" for downstream handling. |
| packages/actions/src/prepare-request-body.spec.ts | Adds unit tests asserting Together reasoning_effort forwarding and "none" translation behavior. |
| packages/models/src/models.ts | Adds requiresDisableThinkingParam to the mapping interface to drive Together “disable thinking” translation behavior. |
| packages/models/src/models/openai.ts | Declares Together reasoningEfforts for gpt-oss mappings (validated low/medium/high). |
| packages/models/src/models/google.ts | Declares Together reasoningEfforts for Gemma mapping (validated none/low/medium/high). |
| packages/models/src/models/deepseek.ts | Updates Together DeepSeek-V4-Pro reasoningEfforts and sets requiresDisableThinkingParam. |
| packages/models/src/models/minimax.ts | Declares Together MiniMax-M3 reasoningEfforts and sets requiresDisableThinkingParam. |
| packages/models/src/models/moonshot.ts | Declares Together Kimi K2.6/K3 reasoningEfforts and sets requiresDisableThinkingParam. |
Suppressed comments (1)
packages/models/src/models/moonshot.ts:708
- The comment says Together's deployment "accepts any reasoning_effort string without validating it", but reasoningEfforts is exposed via /v1/models as the exact accepted reasoning_effort values for a mapping (apps/gateway/src/models/models.ts:82-90). Listing only ["none"] here is likely to under-report what the provider will accept, which can mislead clients relying on the model catalog. Consider either omitting reasoningEfforts (unknown/unbounded), or enumerating the accepted values and clarifying in the comment which tiers actually change behavior.
// Together's deployment accepts any reasoning_effort string without
// validating it, and no tier measurably changes the reasoning length,
// so `none` — honoured through the `thinking` switch rather than
// reasoning_effort — is the only effort this mapping really applies.
reasoningEfforts: ["none"],
requiresDisableThinkingParam: true,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // (which they validate) actually turns it off. Mappings on the second | ||
| // stack set `requiresDisableThinkingParam`; either way only mappings | ||
| // that declare `none` in `reasoningEfforts` can disable at all. | ||
| if (supportsReasoning && reasoning_effort !== undefined) { |
Comment on lines
+51
to
+56
| // Together's deployment accepts any reasoning_effort string without | ||
| // validating it, and no tier measurably changes the reasoning length, | ||
| // so `none` — honoured through the `thinking` switch rather than | ||
| // reasoning_effort — is the only effort this mapping really applies. | ||
| reasoningEfforts: ["none"], | ||
| requiresDisableThinkingParam: true, |
Comment on lines
+468
to
+473
| // Together's deployment accepts any reasoning_effort string without | ||
| // validating it, and no tier measurably changes the reasoning length, | ||
| // so `none` — honoured through the `thinking` switch rather than | ||
| // reasoning_effort — is the only effort this mapping really applies. | ||
| reasoningEfforts: ["none"], | ||
| requiresDisableThinkingParam: true, |
Comment on lines
+319
to
+325
| // Together's deployment accepts any reasoning_effort string without | ||
| // validating it, and only the top tiers measurably change behaviour: | ||
| // xhigh and max roughly double the reasoning tokens, while | ||
| // low/medium/high land on the provider default. `none` is honoured | ||
| // through the `thinking` switch, not through reasoning_effort. | ||
| reasoningEfforts: ["none", "xhigh", "max"], | ||
| requiresDisableThinkingParam: true, |
steebchen
added this pull request to the merge queue
Aug 2, 2026
This was referenced Aug 2, 2026
This was referenced Aug 8, 2026
Closed
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.
Fixes #3361.
The bug
case "inference.net": case "together-ai":inpackages/actions/src/prepare-request-body.tsforwardedresponse_format,temperature,max_tokens,top_pand the penalties, thenbreaked before thedefaultcase's genericreasoning_effortforwarder.reasoning_effortwas therefore never sent to Together AI for any model — including the gateway's own auto-routing default anddeepseek-v4-pro's declaredreasoningEfforts: ["high", "max"], which was dead on arrival.What the upstream API actually does
Probed live against
api.together.ai(not from docs). Together is OpenAI-compatible onreasoning_effort, but disabling thinking is not uniform — reasoning models run on two serving stacks with different switches:reasoning_effortopenai/gpt-oss-120b,openai/gpt-oss-20blow/medium/highnoneis rejected)google/gemma-4-31b-it'none','low','medium','high')reasoning_effort: "none"→ 0 reasoning tokensdeepseek-ai/DeepSeek-V4-Proxhigh/maxroughly double reasoning tokens,low/medium/highland on the defaultthinking: { type: "disabled" }MiniMaxAI/MiniMax-M3,moonshotai/Kimi-K2.6,moonshotai/Kimi-K3thinking: { type: "disabled" }The two switches are not interchangeable: Gemma ignores
thinking(5 runs, reasoning continued at ~1.5k tokens), and DeepSeek V4 Pro / Kimi K2.6 ignorereasoning_effort: "none"(still reasoned).thinking.typeis validated by the second stack — a bad value 400s withunknown variant ... expected one of enabled, disabled, adaptive.The fix
reasoning_effortverbatim from thetogether-aicase, per the repo's no-downgrade rule.together-aitohandlesNoneNativelysononesurvives to the switch.noneper serving stack, driven by a new per-mappingrequiresDisableThinkingParamflag (sibling to the existingrequiresEnableThinking), so the DeepSeek/MiniMax/Kimi mappings emitthinking: { type: "disabled" }and Gemma emitsreasoning_effort: "none".reasoningEffortsfrom measured behaviour rather than assumption, replacingdeepseek-v4-pro's unverified["high", "max"].Testing
prepare-request-body.spec.tscovering both stacks, thenonetranslation, and the drop when a mapping does not declarenone.pnpm test:unitforpackages/actions+packages/models: 546 passed.pnpm buildandpnpm formatclean.TEST_MODELS=... FULL_MODE=true pnpm test:e2e), which expands one case per declared effort tier. All pass: gpt-oss-120b/20blow/medium/high, gemmanone/low/medium/high, deepseek-v4-pronone/xhigh/max, andnonefor minimax-m3, kimi-k2.6, kimi-k3.Pre-existing failures (not from this change)
together-ai/gemma-4-31b-ittimes out at 60s intermittently across unrelated suites (JSON output, streaming, tool calls) and on the plain no-parameter baseline when probed directly — its effort tiers each pass in one run and time out in another, in a different combination each time.together-ai/gpt-oss-120bfails tool-calling and Responses tool-calling. Neither of those suites sendsreasoning_effortat all.Out of scope
moonshotai/Kimi-K2.5andzai-org/GLM-4.7on Together returnUnable to access non-serverless modelfor every request — those mappings need a dedicated endpoint and appear unusable as configured. Left untouched here; worth a separate look.🤖 Generated with Claude Code
Summary by CodeRabbit
none,low,medium,high,xhigh, andmaxtiers where applicable.