fix(provider): apply reasoning_effort to ZAI providers via SetZaiThinking#3198
fix(provider): apply reasoning_effort to ZAI providers via SetZaiThinking#3198tmchow wants to merge 4 commits intotailcallhq:mainfrom
Conversation
785dbea to
c46eb03
Compare
| fn transform(&mut self, mut context: Self::Value) -> Self::Value { | ||
| context.messages.iter_mut().for_each(|message| { | ||
| if let ContextMessage::Text(text) = &mut **message { | ||
| text.reasoning_details = None; |
There was a problem hiding this comment.
What about other reasoning fields?
There was a problem hiding this comment.
I scoped this to reasoning_details to match the sibling DropReasoningDetails transformer in forge_domain (clears reasoning_details + context.reasoning only). The other reasoning-adjacent field on TextMessage is thought_signature, which only flows through dto/google/response.rs and is left alone by the sibling. Happy to clear it too if you'd prefer cross-provider parity here, just wanted to flag the scope intentionally matched the sibling before broadening.
@laststylebender14 asked what happens when reasoning.effort is set to None. Add an inline comment near the .map(|_| true) line walking through why the outer enabled becomes None and no thinking field is added, and point at the existing test that locks the case.
f9aeca9 to
bad7354
Compare
Use &self.agent.provider directly inside the method instead of passing it through as a function argument from the caller. Addresses reviewer feedback.
|
@laststylebender14 thanks for the review.
Fixed in 5f5f14d. Method now reads
I scoped this to |
Summary
Apply
:reasoning-effortto z.ai providers (GLM 5.1 et al). Today the user can run:reasoning-effort noneor:reasoning-effort highagainst GLM 5.1 and observe no behavior change because the per-effort setting is silently dropped before the request reaches the z.ai pipeline. This PR threads the user's intent through to z.ai'sthinking: {type: enabled|disabled}shape.Importance
z.ai's API only exposes thinking as on/off (
{"type": "enabled"|"disabled"}), but Forge's internal model maps:reasoning-efforttoEffort::None|Minimal|Low|Medium|High|XHigh|Max. Two pieces of code were dropping the user's intent:crates/forge_app/src/dto/openai/transformers/pipeline.rs:62-67gatesSetReasoningEffortonREQUESTY | GITHUB_COPILOT | deepseek | NVIDIA. ZAI is excluded by design (z.ai doesn't acceptreasoning_effort), so its parallel transformerSetZaiThinkingis the only path that should be touching reasoning for these providers.crates/forge_app/src/dto/openai/transformers/zai_reasoning.rsonly mappedreasoning.enabledtothinking. The user's:reasoning-effortsetting reaches the request asreasoning.effortwithenabled: None, soSetZaiThinkingdid nothing and the request shipped withoutthinking. GLM 5.1 defaults to thinking ON, so the user got no behavior change for eithernoneorhigh.The
Effort::Nonepath additionally triggered an upstream short-circuit:Context::is_reasoning_supported()(crates/forge_domain/src/context.rs:644-647) treatsEffort::Noneas a strong opt-out, after whichDropReasoningDetails(gated atcrates/forge_app/src/orch.rs:209) wiped the entire reasoning config beforeSetZaiThinkingever ran.Closes #3074
Testing
cargo test -p forge_app zai_reasoning-> 13 tests pass (8 existing + 5 new effort-coverage tests)cargo test -p forge_app drop_message_reasoning-> new transformer test passescargo test -p forge_app-> full crate suite passescargo clippy -p forge_app --all-features --all-targets -- -D warnings-> cleancargo clippy --all-features --workspace -- -D clippy::string_slice -D clippy::indexing_slicing -D clippy::disallowed_methods-> cleanThis contribution was developed with AI assistance.