Repository navigation
fix(models): tolerate non-mapping when_thinking_enabled.extra_body on the reasoning-contract path - #7039
Merged
WillemJiang merged 2 commits intoOct 18, 2026
Conversation
… the reasoning-contract path
WillemJiang
approved these changes
Oct 18, 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.
Why
A
config.yamlmodel profile that declares areasoning:contract together with a truthy non-mappingwhen_thinking_enabled.extra_bodyloads fine but makes the model completely unusable:ModelConfig.when_thinking_enabledisdict | Nonewith unvalidated nested values, and the reasoning-contract validator's_lookup_dottedsimply returnsNonefor such a shape, so the profile passes config load. Then everycreate_chat_modelcall for that model crashes withAttributeError: 'str' object has no attribute 'get'— both thinking directions, every dialect. Since the lead agent, subagents, title generation and summarization all build their models through this factory, the profile breaks the whole backend for that model.The legacy enable path (a profile without a
reasoning:contract) already forwards non-mapping template values unchanged (test_legacy_non_mapping_template_values_are_forwarded_not_rejected), and this PR guards the reasoning-contract path's two reads. The legacy disable direction has its own unguarded reads at base and is fixed by a sibling PR targeting_apply_legacy_thinking_settings; this change does not touch that path.What changed
The reasoning-contract path now treats a truthy non-mapping
when_thinking_enabled.extra_bodythe same way it already treats an absent or null one: as carrying no payload shape._infer_dialect(thedialect: autoheuristic) guards itsextra_bodyread with the sameisinstance(..., dict)check_chat_template_kwargshas always used, so a non-mapping value falls through dialect detection like a null one instead of crashing._apply_contract_thinking_settingsguards the sharedchat_template_kwargslookup the same way, which is read unconditionally before either thinking branch.The enable direction still forwards the template's
extra_bodyvalue to the provider unchanged, exactly like the legacy path; the disable direction keeps synthesizing the dialect payload (it reads the template'sextra_bodyonly to infer the dialect and mirror the vLLM switch spelling; it never forwards or merges the template'sextra_bodyvalue into the settings). Behavior for every profile that already built is unchanged — the guards only replace theAttributeErrorwith the "value is absent" treatment that null values already received.This is the contract-path counterpart of the earlier null-template-keys fix (#6838) and of the legacy-path non-mapping fix, which guarded the same heuristics with the same pattern.
Surface area
frontend/backend/applanggraph.json, or prompt changedocker/or sandboxed executionskills/backend/pyproject.tomlorfrontend/package.jsonNone of the boxes applies: this is a crash fix confined to
backend/packages/harness/deerflow/models/factory.py; every configuration that previously built produces byte-identical constructor settings.Bug fix verification
backend/tests/test_model_factory.py::test_contract_non_mapping_template_values_build_on_both_thinking_paths, parametrized overdialect: auto/dialect: anthropic× thinking enabled / disabled, built with the existing_contract_model/_patch_factory/_capturing_classhelpers next to the other contract-path cases.mainand green on this branch? Yes. Onmainall 4 cases fail withAttributeError: 'str' object has no attribute 'get'— thedialect: autocases inside_infer_dialect(extra_body.get("thinking")), the explicit-dialect cases at the contract path'schat_template_kwargslookup. On this branch all 4 pass and additionally assert the built payload (enable direction forwards the non-mapping value unchanged; disable direction synthesizes the dialect payload).Validation
Backend, from the repo root:
AI assistance
Tool(s) used: Claude Code
How you used it: traced the reported crash to the two unguarded reads on the reasoning-contract path, wrote the parametrized regression test and the minimal guard fix, and checked both against the factory's existing null-value handling conventions.