feat: drop reasoning when tools present but reasoning_with_tool_calls unsupported

[sammaji]

Summary

Some models that support reasoning do not support using reasoning alongside tool calls. This PR introduces a reasoning_with_tool_calls capability flag so that reasoning parameters can be selectively dropped when tools are present but the model doesn't support that combination.

Changes

• Added SupportsReasoningWithToolCalls field to modelParametersParseResult, allowing datasheets to explicitly declare whether a model supports reasoning when tool calls are also present.
• In extractSupportedParams, reasoning_with_tool_calls is added as a supported param by default (when the field is nil), preserving backward compatibility. It is only excluded when explicitly set to false.
• Updated dropUnsupportedParams in the compat plugin to drop reasoning not only when the model doesn't support reasoning at all, but also when tools are present and the model doesn't support reasoning_with_tool_calls.
• Updated a stale comment referencing buildSupportedOutputsIndex to correctly reference extractSupportedParams.

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

Send a request with both reasoning and tools parameters to a model whose datasheet has supports_reasoning_with_tool_calls set to false. Verify that the reasoning parameter is dropped and the request proceeds without error.

Send the same request to a model without supports_reasoning_with_tool_calls set (i.e., nil) and verify that reasoning is preserved, confirming backward-compatible behavior.

go test ./...

Breaking changes

• Yes
• No

Related issues

Security considerations

None.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 806e0902-9330-4063-8026-6839022e44ff

📥 Commits

Reviewing files that changed from the base of the PR and between 0fb111e and d037479.

📒 Files selected for processing (3)

• framework/modelcatalog/datasheet/types.go
• plugins/compat/dropparams.go
• plugins/compat/dropparams_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• plugins/compat/dropparams.go
• plugins/compat/dropparams_test.go
• framework/modelcatalog/datasheet/types.go

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added support for reasoning with tool calls—models can now leverage reasoning capabilities when using tools and function calling.

• Tests

  • Added validation tests for reasoning behavior with tool interactions.

Walkthrough

A new optional boolean field SupportsReasoningWithToolCalls is added to the model datasheet. extractSupportedParams includes reasoning_with_tool_calls in the supported list. dropUnsupportedParams is extended to drop Reasoning when tools are present and reasoning_with_tool_calls is unsupported. A test validates this interaction.

Changes

reasoning_with_tool_calls Capability Support

Layer / File(s)	Summary
Datasheet field and extraction logic framework/modelcatalog/datasheet/types.go	Adds SupportsReasoningWithToolCalls *bool to modelParametersParseResult and updates extractSupportedParams to append reasoning_with_tool_calls when the field is nil or true; fixes the struct comment to reference extractSupportedParams.
Drop Reasoning when tool calls unsupported plugins/compat/dropparams.go	Computes a hasSupportedTools flag and extends dropUnsupportedParams to also drop params.Reasoning when params.Tools is non-nil and isSupported["reasoning_with_tool_calls"] is false, in addition to the existing check for reasoning unsupported.
Test coverage for reasoning and tool interactions plugins/compat/dropparams_test.go	Introduces TestDropUnsupportedParams_ChatReasoningWithUnsupportedTools to verify correct parameter dropping when both reasoning and tools are present under different supported-capability configurations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A bunny hops through the params with care,
New flag supports_reasoning_with_tool_calls declared there.
If tools are around and reasoning can't play,
The drop logic kindly brushes it away.
Nil means permissive, false means refrain—
The catalog speaks, and the compat obeys! 🛠️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding logic to drop reasoning when tools are present but reasoning_with_tool_calls is unsupported.
Description check	✅ Passed	The PR description comprehensively covers all required template sections with clear explanations of the changes, type of change, affected areas, testing instructions, and completed checklists.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 06-23-fix_function_call_with_reasoning_effort_for_chat_completion_for_openai_models

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

_{Comment @coderabbitai help to get the list of available commands.}

[sammaji]

• feat: drop reasoning when tools present but reasoning_with_tool_calls unsupported #4630 👈 (View in Graphite)
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Confidence Score: 4/5

The ChatRequest path is correctly implemented and tested; the ResponsesRequest reasoning drop (line 161) still lacks the new reasoning_with_tool_calls guard, so the fix is incomplete for the Responses API.

The hasSupportedTools variable neatly sidesteps the ordering concern from the previous review, and the ChatRequest logic is correct. However, the ResponsesRequest path at line 161 still only gates reasoning on !isSupported["reasoning"] — a model with supports_reasoning_with_tool_calls: false sending both Reasoning and Tools through the Responses API will not have reasoning stripped, which can cause a provider-side error. The test suite also has no coverage for the Responses path and is missing the backward-compat sub-case.

plugins/compat/dropparams.go — the ResponsesRequest reasoning-drop block (around line 161) needs the same hasSupportedTools && !isSupported["reasoning_with_tool_calls"] guard that was added to the ChatRequest path.

Important Files Changed

Filename	Overview
framework/modelcatalog/datasheet/types.go	Adds SupportsReasoningWithToolCalls *bool field and corresponding extractSupportedParams logic; nil-default adds the param for backward compatibility, false excludes it. Comment fix is correct.
plugins/compat/dropparams.go	Adds hasSupportedTools pre-check and expanded reasoning-drop condition for ChatRequest. The ResponsesRequest path (line 161) still uses the single-condition form and will not drop reasoning on the Responses API path when tools are present and reasoning_with_tool_calls is unsupported — as flagged in the previous review round.
plugins/compat/dropparams_test.go	New test validates tools-unsupported and tools-supported-without-reasoning_with_tool_calls scenarios for ChatRequest; missing the backward-compat case (reasoning_with_tool_calls in supported set → reasoning preserved), and no coverage for the ResponsesRequest path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[dropUnsupportedParams called] --> B{req.ChatRequest != nil?}
    B -- No --> G{req.ResponsesRequest != nil?}
    B -- Yes --> C[hasSupportedTools = len Tools > 0 AND isSupported tools]
    C --> D{params.Reasoning != nil?}
    D -- No --> E[skip reasoning drop]
    D -- Yes --> F{!isSupported reasoning\nOR\nhasSupportedTools AND !isSupported reasoning_with_tool_calls}
    F -- true --> FA[drop reasoning]
    F -- false --> E
    E --> H[... other param drops ...]
    H --> I{len Tools > 0 AND !isSupported tools?}
    I -- Yes --> J[drop tools]
    I -- No --> K[keep tools]

    G -- Yes --> L{params.Reasoning != nil AND !isSupported reasoning?}
    L -- true --> M[drop reasoning\n⚠️ missing reasoning_with_tool_calls check]
    L -- false --> N[keep reasoning]
    N --> O{len Tools > 0 AND !isSupported tools?}
    M --> O
    O -- Yes --> P[drop tools]
    O -- No --> Q[keep tools]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[dropUnsupportedParams called] --> B{req.ChatRequest != nil?}
    B -- No --> G{req.ResponsesRequest != nil?}
    B -- Yes --> C[hasSupportedTools = len Tools > 0 AND isSupported tools]
    C --> D{params.Reasoning != nil?}
    D -- No --> E[skip reasoning drop]
    D -- Yes --> F{!isSupported reasoning\nOR\nhasSupportedTools AND !isSupported reasoning_with_tool_calls}
    F -- true --> FA[drop reasoning]
    F -- false --> E
    E --> H[... other param drops ...]
    H --> I{len Tools > 0 AND !isSupported tools?}
    I -- Yes --> J[drop tools]
    I -- No --> K[keep tools]

    G -- Yes --> L{params.Reasoning != nil AND !isSupported reasoning?}
    L -- true --> M[drop reasoning\n⚠️ missing reasoning_with_tool_calls check]
    L -- false --> N[keep reasoning]
    N --> O{len Tools > 0 AND !isSupported tools?}
    M --> O
    O -- Yes --> P[drop tools]
    O -- No --> Q[keep tools]

Loading

_{Reviews (3): Last reviewed commit: "feat: drop reasoning when tools present ..." | Re-trigger Greptile}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

plugins/compat/dropparams.go (1)

160-163: 🎯 Functional Correctness | 🟠 Major

Add reasoning_with_tool_calls check to ResponsesRequest path for consistency.

The ResponsesRequest path (lines 160–163) only checks isSupported["reasoning"] before dropping the Reasoning parameter, but the ChatRequest path (lines 71–77) conditionally drops it when tools are present and reasoning_with_tool_calls is unsupported. Since ResponsesParameters also has a Tools field (used at lines 144, 152, 184, 188), the same conditional logic should apply:

-	if params.Reasoning != nil && !isSupported["reasoning"] {
-		params.Reasoning = nil
-		dropped = append(dropped, "reasoning")
+	if params.Reasoning != nil {
+		if !isSupported["reasoning"] || params.Tools != nil && !isSupported["reasoning_with_tool_calls"] {
+			params.Reasoning = nil
+			dropped = append(dropped, "reasoning")
+		}
 	}

🤖 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 `@plugins/compat/dropparams.go` around lines 160 - 163, The reasoning parameter
handling in the ResponsesRequest path (lines 160–163) is inconsistent with the
ChatRequest path. In the ChatRequest path, reasoning is dropped only when both
tools are present AND reasoning_with_tool_calls is unsupported. Update the logic
for params.Reasoning in the ResponsesRequest path to apply the same conditional
check: drop reasoning only if params.Tools is not empty AND
isSupported["reasoning_with_tool_calls"] is false, similar to how it's handled
in the ChatRequest section. This ensures consistent behavior across both paths
since ResponsesParameters also has a Tools field.

🧹 Nitpick comments (2)

plugins/compat/dropparams.go (1)

71-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add explicit parentheses for clarity and clarify the comment.

While the operator precedence is correct (&& binds tighter than ||), explicit parentheses would improve readability. Also, the comment mentions "reasoning_effort" but the code drops the entire Reasoning parameter, which may contain additional reasoning-related fields.

♻️ Proposed improvements

 	if params.Reasoning != nil {
-		// for chat completions, some models do not support reasoning_effort
-		// with tools
-		if !isSupported["reasoning"] || params.Tools != nil && !isSupported["reasoning_with_tool_calls"] {
+		// Drop reasoning parameter when: (1) reasoning is unsupported, or
+		// (2) tools are present but reasoning_with_tool_calls is unsupported
+		if !isSupported["reasoning"] || (params.Tools != nil && !isSupported["reasoning_with_tool_calls"]) {
 			params.Reasoning = nil
 			dropped = append(dropped, "reasoning")
 		}

🤖 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 `@plugins/compat/dropparams.go` around lines 71 - 77, In the condition checking
params.Reasoning support, add explicit parentheses around the logical AND
operation to improve readability: wrap the params.Tools != nil &&
!isSupported["reasoning_with_tool_calls"] portion in parentheses to make the OR
precedence explicit. Additionally, update the comment above the condition to
accurately reflect what is being dropped—the entire params.Reasoning parameter
(which may contain multiple reasoning-related fields) rather than just
"reasoning_effort", ensuring the comment accurately describes the behavior of
setting params.Reasoning to nil and appending "reasoning" to the dropped list.

framework/modelcatalog/datasheet/types.go (1)

480-482: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Simplify the nil-check condition.

The condition can be simplified from:

parsed.SupportsReasoningWithToolCalls == nil || (parsed.SupportsReasoningWithToolCalls != nil && *parsed.SupportsReasoningWithToolCalls)

to:

parsed.SupportsReasoningWithToolCalls == nil || *parsed.SupportsReasoningWithToolCalls

This is safe because Go's || operator short-circuits—if the first condition is true, the second part (with the dereference) won't be evaluated.

♻️ Proposed simplification

-	if parsed.SupportsReasoningWithToolCalls == nil || (parsed.SupportsReasoningWithToolCalls != nil && *parsed.SupportsReasoningWithToolCalls) {
+	if parsed.SupportsReasoningWithToolCalls == nil || *parsed.SupportsReasoningWithToolCalls {
 		addParam("reasoning_with_tool_calls")
 	}

🤖 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 `@framework/modelcatalog/datasheet/types.go` around lines 480 - 482, The
nil-check condition for parsed.SupportsReasoningWithToolCalls in the if
statement is unnecessarily verbose. Simplify the condition by removing the
redundant nil check from the second part of the OR expression. Since Go's ||
operator short-circuits, if the first condition
(parsed.SupportsReasoningWithToolCalls == nil) is false, the pointer will be
safe to dereference in the second part. Replace the entire condition with just
parsed.SupportsReasoningWithToolCalls == nil ||
*parsed.SupportsReasoningWithToolCalls, keeping the addParam call with
reasoning_with_tool_calls unchanged.

🤖 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.

Outside diff comments:
In `@plugins/compat/dropparams.go`:
- Around line 160-163: The reasoning parameter handling in the ResponsesRequest
path (lines 160–163) is inconsistent with the ChatRequest path. In the
ChatRequest path, reasoning is dropped only when both tools are present AND
reasoning_with_tool_calls is unsupported. Update the logic for params.Reasoning
in the ResponsesRequest path to apply the same conditional check: drop reasoning
only if params.Tools is not empty AND isSupported["reasoning_with_tool_calls"]
is false, similar to how it's handled in the ChatRequest section. This ensures
consistent behavior across both paths since ResponsesParameters also has a Tools
field.

---

Nitpick comments:
In `@framework/modelcatalog/datasheet/types.go`:
- Around line 480-482: The nil-check condition for
parsed.SupportsReasoningWithToolCalls in the if statement is unnecessarily
verbose. Simplify the condition by removing the redundant nil check from the
second part of the OR expression. Since Go's || operator short-circuits, if the
first condition (parsed.SupportsReasoningWithToolCalls == nil) is false, the
pointer will be safe to dereference in the second part. Replace the entire
condition with just parsed.SupportsReasoningWithToolCalls == nil ||
*parsed.SupportsReasoningWithToolCalls, keeping the addParam call with
reasoning_with_tool_calls unchanged.

In `@plugins/compat/dropparams.go`:
- Around line 71-77: In the condition checking params.Reasoning support, add
explicit parentheses around the logical AND operation to improve readability:
wrap the params.Tools != nil && !isSupported["reasoning_with_tool_calls"]
portion in parentheses to make the OR precedence explicit. Additionally, update
the comment above the condition to accurately reflect what is being dropped—the
entire params.Reasoning parameter (which may contain multiple reasoning-related
fields) rather than just "reasoning_effort", ensuring the comment accurately
describes the behavior of setting params.Reasoning to nil and appending
"reasoning" to the dropped list.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c5e40eb2-b0d0-4b34-b3ed-e31d3b70f5b8

📥 Commits

Reviewing files that changed from the base of the PR and between 1c846ce and 85cc7c7.

📒 Files selected for processing (2)

• framework/modelcatalog/datasheet/types.go
• plugins/compat/dropparams.go

The merge-base changed after approval.

[Pratham-Mishra04]

Merge activity

• Jun 30, 5:01 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 30, 5:02 PM UTC: @Pratham-Mishra04 merged this pull request with Graphite.