Repository navigation
Thinking control for local OpenAI-compatible providers (chat_template_kwargs) - #1
Conversation
…per, merge-precedence bar, baseline-relative success, deploy ordering
…parsed-JSON fixtures, rehearse the cherry-pick
… call sites, streaming and non-streaming main-agent coverage
…record why the intermediate-assertion ask is rejected
…ch-key values in B1
Stage A: local stub, three real call paths, captured bodies and fork/main golden bytes. Auxiliary call_llm does not forward providers.*.extra_body; main-stream and the MoA reference do. A1 is not green. A2 names every path that lacks the key once the block is removed.
The hermetic test runner drops TMPDIR, and the main-agent system prompt embeds HERMES_HOME. A tempdir-based home would make the B5 golden bytes depend on which environment re-ran the probe. Recaptured the Stage A fixtures and the fork/main golden bodies at the pinned path.
Providers that declare thinking_control send the switch and a mapped effort inside chat_template_kwargs. Auxiliary, MoA, and both chat-completions builders call that helper directly. A partial effort_map warns at load; an unmapped effort is dropped, not sent raw.
A1 still fails on the auxiliary path. Stage B tests and the mutation proofs are recorded with the commands that produced them.
…s over VALID_REASONING_EFFORTS, make B5 portable
The printed maps now cover every VALID_REASONING_EFFORTS value, so a partial map warns and the corrected maps do not. Golden comparison replaces system-message text with a fixed placeholder and checks every other field exactly.
…ute the summary request through the helper, keep non-dict caller kwargs; bars B8-B10
A missing reasoning_config now resolves against the model on the request, not the config's main model, so a per-model override applies to aux and MoA. An explicit caller reasoning_config still wins. A non-dict chat_template_kwargs is left as supplied, and one warning names the provider.
The hand-built summary chat.completions.create call now goes through apply_thinking_control, the same helper the aux and main-agent builders use.
The fix-round 2 suite on fafc4d9 is 230 passed, exit 0. B4 was not mutated again.
|
Verdict: APPROVE Review round 2 by crew-reviewer (a separate session on Opus 5.5, not the build seat) at 71b56a5. It re-executed A1-A2 and B1-B10 in its own detached worktree, mutation-proved B4 (i-iv) and B8-B10 (each fails at an assertion, rc=1), and re-ran the round-1 reproduction on the stub socket (aux and MoA carry enable_thinking false for the overridden local model). Broader suites: 156 files, 2,730 passed; the 10 failures reproduce identically on fork/main 31cedb4. Round 1 was CHANGES REQUESTED (fallback resolved against the main model; summary path; non-dict kwargs), resolved by steer ruling 99971d2 and fix round 2. a3f25e6 is a doc-only wording reconcile after the review. Obligation: ryan-claude holds this PR. Not merged; deploy to the running VPS branch is human-gated (work order section 5). |
Summary
T1000's per-model reasoning setting never reached local OpenAI-compatible servers. For a provider without a profile (e.g.
spark), anoneoverride was sent asextra_body.reasoning = {"enabled": false}. vLLM drops that key (it maps only a top-levelreasoning_effort), and TensorFold reads onlychat_template_kwargs. So Flash-Next thought on every T1000 call, and 83% of finn's 1,889 requests in 24 h endedfinish_reason=length. On one small coding prompt, default thinking spent 3,000 tokens and 103 s without an answer; with thinking off it took 887 tokens and 22 s.This adds an opt-in
providers.<name>.thinking_controlblock (kind: chat_template_kwargs,switch_key,effort_key,effort_map) and one helper,agent/thinking_control.py::apply_thinking_control. It is called directly inauxiliary_client._build_call_kwargs(auxiliary tasks and MoA), in both chat-completions builders (main agent, streaming and non-streaming), and in the iteration-limit summary request.reasoning_configresolves for the model being requested (per-model override, then the global effort). An explicit one wins.thinkingwith high/max. An unmapped effort is dropped and a warning is logged.reasoningobject and top-levelreasoning_effortare not sent for these providers.chat_template_kwargswins; a non-dict value is kept and a warning is logged.effort_mapmisses any ofVALID_REASONING_EFFORTS.thinking_controlare unchanged. B5 compares the bodies against fork/main.Finding from the build (
docs/plans/forge/2026-09-29-thinking-control/BUILD-FINDINGS.md): a config-only fix throughproviders.<name>.extra_bodydoes NOT reach auxiliary calls, becausecall_llmcopies only base_url and api_key. That is why this is a code change.Work order:
docs/plans/2026-09-29-local-provider-thinking-control-work-order.md. It was committed before any code and forged 6 rounds on local Flash-Next (record indocs/plans/forge/2026-09-29-thinking-control/). Cards: mesh t_d698d173, models t_43fdb841, t_cfa038b9.Review
Crew loop: ryan-claude steered, Grok (unfenced, commit-only, push blocked) built, and crew-reviewer (a separate session) reviewed.
99971d255c; fix round 2.enable_thinking: false. Broader suites: 156 files, 2,730 passed, 10 failed; all 10 fail identically on fork/main (anthropic SDK and gateway health).Test plan
scripts/run_tests.sh tests/agent/test_thinking_control.py tests/agent/test_auxiliary_client.py tests/agent/test_custom_provider_extra_body.py: 230 passedscripts/probe_provider_wire_body.py --mode stage-bfor {none, low, high, max} x {qwen, ds4}: 4/4 paths eachthinking_control, restart, then add the config forspark(Qwen map) and DS4 (DS4 map). Measure finn'sfinished_reasonsplit before and 24 h after.