Skip to content

fix(models): tolerate non-mapping when_thinking_enabled.extra_body on the reasoning-contract path - #7039

Merged
WillemJiang merged 2 commits into
bytedance:mainfrom
BlueX888:fix/factory-contract-nonmapping-extrabody
Oct 18, 2026
Merged

WillemJiang merged 2 commits into
bytedance:mainfrom
BlueX888:fix/factory-contract-nonmapping-extrabody

Conversation

@BlueX888

Copy link
Copy Markdown
Contributor

Why

A config.yaml model profile that declares a reasoning: contract together with a truthy non-mapping when_thinking_enabled.extra_body loads fine but makes the model completely unusable:

models:
  - name: glm-4.6
    use: langchain_openai:ChatOpenAI
    model: glm-4.6
    reasoning:
      thinking: optional        # any contract dialect: auto/anthropic/... triggers it
    when_thinking_enabled:
      extra_body: oops           # or `extra_body: true` — a typo like writing the
                                 # payload as a string instead of a mapping

ModelConfig.when_thinking_enabled is dict | None with unvalidated nested values, and the reasoning-contract validator's _lookup_dotted simply returns None for such a shape, so the profile passes config load. Then every create_chat_model call for that model crashes with AttributeError: '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_body the same way it already treats an absent or null one: as carrying no payload shape.

  • _infer_dialect (the dialect: auto heuristic) guards its extra_body read with the same isinstance(..., dict) check _chat_template_kwargs has always used, so a non-mapping value falls through dialect detection like a null one instead of crashing.
  • _apply_contract_thinking_settings guards the shared chat_template_kwargs lookup the same way, which is read unconditionally before either thinking branch.

The enable direction still forwards the template's extra_body value to the provider unchanged, exactly like the legacy path; the disable direction keeps synthesizing the dialect payload (it reads the template's extra_body only to infer the dialect and mirror the vLLM switch spelling; it never forwards or merges the template's extra_body value into the settings). Behavior for every profile that already built is unchanged — the guards only replace the AttributeError with 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 UI — page / component / setting / interaction under frontend/
  • Backend API — endpoint / SSE event / request-response shape under backend/app
  • Agents / LangGraph — agent node, graph wiring, langgraph.json, or prompt change
  • Sandbox — docker/ or sandboxed execution
  • Skills — change under skills/
  • Dependencies — new/upgraded entry in backend/pyproject.toml or frontend/package.json
  • Default behavior change — changes existing behavior without the user opting in
  • Docs / tests / CI only — no runtime behavior change

None 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

  • Test path: backend/tests/test_model_factory.py::test_contract_non_mapping_template_values_build_on_both_thinking_paths, parametrized over dialect: auto / dialect: anthropic × thinking enabled / disabled, built with the existing _contract_model / _patch_factory / _capturing_class helpers next to the other contract-path cases.
  • Did it go red on main and green on this branch? Yes. On main all 4 cases fail with AttributeError: 'str' object has no attribute 'get' — the dialect: auto cases inside _infer_dialect (extra_body.get("thinking")), the explicit-dialect cases at the contract path's chat_template_kwargs lookup. 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:

uv run --frozen --project backend python -m pytest backend/tests/test_model_factory.py -q
  -> 181 passed (177 pre-existing + 4 new)

uv run --frozen --project backend python -m pytest backend/tests/test_reasoning_contract.py backend/tests/test_lead_agent_model_resolution.py backend/tests/test_models_router_reasoning.py -q
  -> 142 passed

cd backend && uv run ruff check . && uv run ruff format --check .
  -> all checks passed, 2135 files already formatted

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.

  • I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output.

@github-actions github-actions Bot added area:backend Gateway / runtime / core backend under backend/ risk:medium Medium risk: regular code changes size/S PR changes 20-100 lines labels Oct 18, 2026
@github-actions github-actions Bot added area:docs Documentation and Markdown only area:frontend Next.js frontend under frontend/ labels Oct 18, 2026
@WillemJiang WillemJiang added this to the 2.2.0 milestone Oct 18, 2026
@WillemJiang
WillemJiang merged commit 12e619c into bytedance:main Oct 18, 2026
17 checks passed
@willem-bd willem-bd mentioned this pull request Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Gateway / runtime / core backend under backend/ area:docs Documentation and Markdown only area:frontend Next.js frontend under frontend/ risk:medium Medium risk: regular code changes size/S PR changes 20-100 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants