Add reasoningEffort to setModel/session.model.switchTo across all SDKs#712
Add reasoningEffort to setModel/session.model.switchTo across all SDKs#712
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for passing an optional reasoningEffort when switching models mid-session (session.model.switchTo / setModel / set_model) across the Node.js, Python, Go, and .NET SDKs, aligning with the runtime capability introduced in the companion runtime PR.
Changes:
- Extend each SDK’s session-level “switch model” API to accept optional
reasoningEffort. - Update generated RPC parameter types in each language to serialize
reasoningEffortasreasoningEfforton the wire. - Update inline docs/examples to demonstrate setting
reasoningEffortduring a model switch.
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| python/copilot/session.py | Adds keyword-only reasoning_effort to set_model() and forwards it to RPC. |
| python/copilot/generated/rpc.py | Extends SessionModelSwitchToParams to include optional reasoning_effort serialization. |
| nodejs/src/session.ts | Extends setModel() to accept an options bag and forwards reasoningEffort. |
| nodejs/src/generated/rpc.ts | Extends SessionModelSwitchToParams to include optional reasoningEffort union type. |
| go/session.go | Adds SetModelOptions and forwards ReasoningEffort (as optional) to RPC. |
| go/rpc/generated_rpc.go | Extends SessionModelSwitchToParams and request building to include reasoningEffort. |
| dotnet/src/Session.cs | Adds optional reasoningEffort parameter and forwards it to RPC. |
| dotnet/src/Generated/Rpc.cs | Extends the generated request type and SwitchToAsync signature to include reasoningEffort. |
dotnet/src/Generated/Rpc.cs
Outdated
| public async Task<SessionModelSwitchToResult> SwitchToAsync(string modelId, string? reasoningEffort = null, CancellationToken cancellationToken = default) | ||
| { | ||
| var request = new SessionModelSwitchToRequest { SessionId = _sessionId, ModelId = modelId }; | ||
| var request = new SessionModelSwitchToRequest { SessionId = _sessionId, ModelId = modelId, ReasoningEffort = reasoningEffort }; | ||
| return await CopilotClient.InvokeRpcAsync<SessionModelSwitchToResult>(_rpc, "session.model.switchTo", [request], cancellationToken); | ||
| } |
There was a problem hiding this comment.
ModelApi.SwitchToAsync now takes (string modelId, string? reasoningEffort = null, CancellationToken cancellationToken = default), which similarly breaks any existing callers that passed a CancellationToken as the second positional argument. Consider adding a backward-compatible overload SwitchToAsync(string modelId, CancellationToken cancellationToken = default) that forwards to the new method (or otherwise avoid inserting parameters before cancellationToken in public APIs).
python/copilot/session.py
Outdated
| >>> await session.set_model("claude-sonnet-4.6", reasoning_effort="high") | ||
| """ | ||
| await self.rpc.model.switch_to(SessionModelSwitchToParams(model_id=model)) | ||
| await self.rpc.model.switch_to(SessionModelSwitchToParams(model_id=model, reasoning_effort=reasoning_effort)) |
There was a problem hiding this comment.
This call is long enough that it will be reformatted by ruff format (the repo enforces ruff format --check in CI with a 100 character line length). To avoid CI formatting diffs/failures, wrap the SessionModelSwitchToParams(...) construction and/or the switch_to(...) call across multiple lines in the style used elsewhere in this file.
| await self.rpc.model.switch_to(SessionModelSwitchToParams(model_id=model, reasoning_effort=reasoning_effort)) | |
| await self.rpc.model.switch_to( | |
| SessionModelSwitchToParams( | |
| model_id=model, | |
| reasoning_effort=reasoning_effort, | |
| ) | |
| ) |
go/session.go
Outdated
| func (s *Session) SetModel(ctx context.Context, model string, opts ...*SetModelOptions) error { | ||
| params := &rpc.SessionModelSwitchToParams{ModelID: model} | ||
| if len(opts) > 0 && opts[0] != nil && opts[0].ReasoningEffort != "" { | ||
| re := opts[0].ReasoningEffort | ||
| params.ReasoningEffort = &re | ||
| } |
There was a problem hiding this comment.
SetModel accepts a variadic opts ...*SetModelOptions, but only opts[0] is ever read. This makes it easy to accidentally pass multiple option structs and have all but the first silently ignored. Consider either validating len(opts) <= 1 (and returning an error if more are provided) or changing the API shape to avoid a variadic when only a single options value is supported.
patniko
force-pushed
the
fix/setmodel-reasoning-effort-and-byok
branch
from
fbffd30 to
2910573
Compare
✅ Cross-SDK Consistency ReviewThis PR demonstrates excellent cross-SDK consistency for adding Consistency Highlights✅ Feature parity: All SDKs now support API Signatures (all semantically equivalent)
Note: Go Breaking Change
Additional FeatureThis PR also consistently adds Recommendation: Approve — this PR maintains excellent consistency across all SDK implementations. 🚀
|
patniko
force-pushed
the
fix/setmodel-reasoning-effort-and-byok
branch
from
2910573 to
f35bf5c
Compare
✅ SDK Consistency Review: APPROVED with one minor observationGreat work on maintaining consistency across all four SDKs! This PR successfully adds the ✅ What's Consistent
📋 Minor ObservationI noticed that the generated RPC files include a new Recommendation: If Verdict: The
|
patniko
force-pushed
the
fix/setmodel-reasoning-effort-and-byok
branch
from
f35bf5c to
f6ee586
Compare
✅ Cross-SDK Consistency Review: PASSEDI've reviewed this PR for consistency across all four SDK implementations (Node.js, Python, Go, and .NET). The changes maintain excellent cross-SDK consistency. SummaryAll four SDKs now support the
Implementation Details
Each approach follows the established conventions for that language while maintaining semantic equivalence. No consistency issues found. 🎉
|
patniko
force-pushed
the
fix/setmodel-reasoning-effort-and-byok
branch
from
f6ee586 to
518256d
Compare
|
@SteveSandersonMS need to get #4408 in and released first. |
dotnet/src/Generated/Rpc.cs
Outdated
|
|
||
| /// <summary>Calls "session.model.switchTo".</summary> | ||
| public async Task<SessionModelSwitchToResult> SwitchToAsync(string modelId, CancellationToken cancellationToken = default) | ||
| public async Task<SessionModelSwitchToResult> SwitchToAsync(string modelId, SessionModelSwitchToRequestReasoningEffort? reasoningEffort, CancellationToken cancellationToken = default) |
There was a problem hiding this comment.
Just a note, the reasoning effort is currently a string? everywhere else (getting list of models, creating/ resuming session, etc), so feels a bit weird that its an enum here.
As it would require us to parse the string? that the SDK returns into this enum to change it mid session
Wouldnt be an issue if it was an enum throughout SDK
All four SDKs now support passing reasoningEffort when switching models
mid-session via setModel(). The parameter is optional and backward-compatible.
- Node.js: setModel(model, { reasoningEffort? })
- Python: set_model(model, *, reasoning_effort=None)
- Go: SetModel(ctx, model, opts ...*SetModelOptions)
- .NET: SetModelAsync(model, reasoningEffort?, cancellationToken?)
Fixes #687
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SteveSandersonMS
force-pushed
the
fix/setmodel-reasoning-effort-and-byok
branch
from
518256d to
c0c4635
Compare
✅ Cross-SDK Consistency Review: PASSEDThis PR demonstrates excellent consistency across all four SDK implementations. The Consistency Analysis✅ API Signatures (accounting for language conventions)
✅ Implementation Details
✅ RPC Layer ConsistencyAll SDKs correctly pass the parameter to the underlying RPC method:
RecommendationNo consistency issues found. This PR maintains excellent feature parity across all SDKs. The implementation respects language-specific conventions while ensuring the same functionality is available everywhere. ✅
|
…, fix .NET overloads - Go: use variadic ...SetModelOptions for backward compatibility - TypeScript: reuse ReasoningEffort type instead of inline union - .NET: remove redundant default on reasoningEffort param, make CancellationToken optional in both overloads - Revert unrelated dotnet/test/RpcTests.cs change Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
✅ Cross-SDK Consistency ReviewGreat work on maintaining consistency across all four SDK implementations! The API Consistency ✓All four SDKs follow the expected language conventions:
Implementation Details ✓
Minor Suggestion: Test CoverageThe PR includes a test in Node.js (
This would ensure the feature is equally well-tested across all implementations and help catch any regression in the future. Summary: This PR maintains excellent cross-SDK consistency. The only recommendation is to add tests for Python, Go, and .NET matching the Node.js test coverage.
|
|
@patniko I'm trying to work out how well this is going to fit for custom providers/BYOK. Relates to @IeuanWalker's point above too. Having a fixed set of values works for OpenAI/CAPI (though the set might expand over time, or maybe some newer models won't support all of these values), but not for custom providers. Do you think we should redefine reasoning effort to accept arbitrary strings? That way provider-specific effort values can be accommodated, and we don't falsely imply that these specific reasoning effort values are always supported for all models. We can also offer a set of consts for SDK consumers that define well-known OpenAI/CAPI values. If so I need to make a change in the underlying runtime which I've already started in the background, so please let me know what you think. |
|
@SteveSandersonMS strings sound good for a longer term approach. Thanks! |
Summary
All four SDKs now support passing
reasoningEffortwhen switching models mid-session viasetModel(). The parameter is optional and fully backward-compatible.API Changes
setModel(model, options?: { reasoningEffort? })set_model(model, *, reasoning_effort=None)SetModel(ctx, model, opts ...*SetModelOptions)SetModelAsync(model, reasoningEffort?, cancellationToken?)Usage Examples
Files Changed
nodejs/src/session.ts+nodejs/src/generated/rpc.tspython/copilot/session.py+python/copilot/generated/rpc.pygo/session.go+go/rpc/generated_rpc.godotnet/src/Session.cs+dotnet/src/Generated/Rpc.csCompanion PR
Fixes #687