Skip to content

Thinking control for local OpenAI-compatible providers (chat_template_kwargs) - #1

Merged
ryanmitchellneely merged 20 commits into
mainfrom
ryan/local-thinking-control
Sep 29, 2026
Merged

ryanmitchellneely merged 20 commits into
mainfrom
ryan/local-thinking-control

Conversation

@ryanmitchellneely

Copy link
Copy Markdown
Owner

Summary

T1000's per-model reasoning setting never reached local OpenAI-compatible servers. For a provider without a profile (e.g. spark), a none override was sent as extra_body.reasoning = {"enabled": false}. vLLM drops that key (it maps only a top-level reasoning_effort), and TensorFold reads only chat_template_kwargs. So Flash-Next thought on every T1000 call, and 83% of finn's 1,889 requests in 24 h ended finish_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_control block (kind: chat_template_kwargs, switch_key, effort_key, effort_map) and one helper, agent/thinking_control.py::apply_thinking_control. It is called directly in auxiliary_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.

  • A missing reasoning_config resolves for the model being requested (per-model override, then the global effort). An explicit one wins.
  • Efforts go through a per-family map. Qwen raises on an unknown effort; DS4 uses thinking with high/max. An unmapped effort is dropped and a warning is logged.
  • The legacy reasoning object and top-level reasoning_effort are not sent for these providers.
  • Caller chat_template_kwargs wins; a non-dict value is kept and a warning is logged.
  • A config-load warning fires when an effort_map misses any of VALID_REASONING_EFFORTS.
  • Providers without thinking_control are 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 through providers.<name>.extra_body does NOT reach auxiliary calls, because call_llm copies 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 in docs/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.

  • Round 1: CHANGES REQUESTED. The fallback resolved against the main model; the summary path was uncovered; non-dict caller kwargs were replaced. Steer ruling 99971d255c; fix round 2.
  • Round 2: Verdict: APPROVE. The reviewer re-executed A1-A2 and B1-B10, mutation-proved B4 (i-iv) and B8-B10 (each fails at an assertion), and re-ran the round-1 reproduction on the socket: the aux and MoA bodies now carry 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 passed
  • scripts/probe_provider_wire_body.py --mode stage-b for {none, low, high, max} x {qwen, ds4}: 4/4 paths each
  • B5 golden check with a different home and frozen date
  • Deploy (human-gated, work order section 5): rehearse the cherry-pick onto the running branch, verify the running loader ignores thinking_control, restart, then add the config for spark (Qwen map) and DS4 (DS4 map). Measure finn's finished_reason split before and 24 h after.

Ryan Neely added 20 commits September 29, 2026 14:18
…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
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.
@ryanmitchellneely

Copy link
Copy Markdown
Owner Author

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).

@ryanmitchellneely
ryanmitchellneely merged commit dfd624b into main Sep 29, 2026
40 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant