Skip to content

strip :THINKING in e2ee_round_trip, drop per-model defaults - #2

Open
sirouk wants to merge 4 commits into
chutesai:mainfrom
sirouk:fix/thinking-minimal
Open

strip :THINKING in e2ee_round_trip, drop per-model defaults#2
sirouk wants to merge 4 commits into
chutesai:mainfrom
sirouk:fix/thinking-minimal

Conversation

@sirouk

@sirouk sirouk commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Supersedes #1.

Root path of all requests is e2ee_round_trip, so that's where the
:THINKING handling lives. Strip the suffix from model, set
thinking/enable_thinking in chat_template_kwargs, re-encode the body.
Discovery never sees :THINKING, and claude_handler / responses_handler
inherit the behavior for free.

Per-model defaults (GLM-4.7 / Kimi-K2.5 / DeepSeek-V3.2-Speciale on,
MiMo-V2-Flash off) are not duplicated here. Users can set
chat_template_kwargs directly in the request body to override; it's
not the proxy's job to replicate every edge case from the chutes-api
router.

Credit to Snow for reporting the original bug and opening #1, and
to Ben for the review that straightened this out.

@stargazer617

stargazer617 commented Apr 25, 2026

Copy link
Copy Markdown

I believe this does not handle the X-Enable-Thinking header. But it should be an easy fix, checking for the header with ngx.req.get_headers() and enabling chat_template_kwargs.thinking and chat_template_kwargs.enable_thinking, the same way :THINKING is handled.

@stargazer617

stargazer617 commented Apr 25, 2026

Copy link
Copy Markdown

Also doesn't seem to normalize thinking and enable_thinking keys. So sending "chat_template_kwargs": { "thinking": true } wouldn't enable reasoning for models using the enable_thinking kwarg, and vice versa. This should also be an easy fix, it could be done the way it was done in #1 :

if kwargs.thinking ~= nil and kwargs.enable_thinking == nil then
  kwargs.enable_thinking = kwargs.thinking
end
if kwargs.enable_thinking ~= nil and kwargs.thinking == nil then
  kwargs.thinking = kwargs.enable_thinking
end

@cxmplex

cxmplex commented Apr 28, 2026

Copy link
Copy Markdown

@stargazer617

Also doesn't seem to normalize thinking and enable_thinking keys. So sending "chat_template_kwargs": { "thinking": true } wouldn't enable reasoning for models using the enable_thinking kwarg, and vice versa. This should also be an easy fix, it could be done the way it was done in #1 :

if kwargs.thinking ~= nil and kwargs.enable_thinking == nil then
  kwargs.enable_thinking = kwargs.thinking
end
if kwargs.enable_thinking ~= nil and kwargs.thinking == nil then
  kwargs.thinking = kwargs.enable_thinking
end

There shouldn't be any downside to sending both thinking and enable_thinking which this does. If the user sends something like "chat_template_kwargs": { "thinking": true } when they needed enable_thinking they should either 1.) fix that and just send both, 2.) use the :THINKING suffix.

There's too many model specific exceptions in regards to defaults. If there's a specific APP or something where this is needed, please let me know.

@cxmplex

cxmplex commented Apr 28, 2026

Copy link
Copy Markdown

@stargazer617 @sirouk

I believe this does not handle the X-Enable-Thinking header. But it should be an easy fix, checking for the header with ngx.req.get_headers() and enabling chat_template_kwargs.thinking and chat_template_kwargs.enable_thinking, the same way :THINKING is handled.

This could be added to match chutes-api.

@cxmplex

cxmplex commented Apr 28, 2026

Copy link
Copy Markdown

@sirouk add it like this

If :THINKING is not present, you do not touch chat kwargs (current behavior)

If :THINKING is present, you set both chat kwargs to true (current behavior)

If X-Enable-Thinking is not present, you do not touch chat kwargs

If X-Enable-Thinking is present, and it is true (bool), you set both chat kwargs to true (need to implement)

If X-Enable-Thinking is present, and it is false (bool), you set both chat kwargs to false (need to implement)

@stargazer617

stargazer617 commented Apr 28, 2026

Copy link
Copy Markdown

@cxmplex

@stargazer617

Also doesn't seem to normalize thinking and enable_thinking keys. So sending "chat_template_kwargs": { "thinking": true } wouldn't enable reasoning for models using the enable_thinking kwarg, and vice versa. This should also be an easy fix, it could be done the way it was done in #1 :

if kwargs.thinking ~= nil and kwargs.enable_thinking == nil then
  kwargs.enable_thinking = kwargs.thinking
end
if kwargs.enable_thinking ~= nil and kwargs.thinking == nil then
  kwargs.thinking = kwargs.enable_thinking
end

There shouldn't be any downside to sending both thinking and enable_thinking which this does. If the user sends something like "chat_template_kwargs": { "thinking": true } when they needed enable_thinking they should either 1.) fix that and just send both, 2.) use the :THINKING suffix.

There's too many model specific exceptions in regards to defaults. If there's a specific APP or something where this is needed, please let me know.

While there aren't any downsides to the current implementation, I still believe it would be good practice to normalize these two kwargs so that sending one of them to the endpoint sends the other as well. I understand that there are many models with specific exceptions, but most models seem to use either thinking or enable_thinking when it comes to reasoning, so this should be set and forget, rather than being a maintenance burden later down the line. For example, Qwen and GLM use enable_thinking, while Kimi, MiMo and Deepseek use thinking. Given how the instructions on the support server recommend adding "chat_template_kwargs": { "thinking": true } to request body to enable thinking, and how the main Chutes API normalizes these two kwargs as well, I believe it would be better to do the same here. The issue isn't with how the kwargs are handled when :THINKING is present, but how they are handled when neither the suffix nor the header is present. As in, what happens when users manually set chat_template_kwargs with only one of the keys. The current PR only handles the :THINKING suffix case by injecting both. But if a user sends a manual request with only thinking: true (following the recommendation in the discord server), and the model actually needs enable_thinking, it does not enable reasoning, where the main API would have normalized it. A less technical user migrating from the main API to the proxy would find their old workflow doesn't work here because the proxy does not have full parity with the API. I believe it is better to do it correctly now, rather than silently alter the behaviour of the proxy compared to the API. That said, I agree that keeping per model defaults here is unnecessary. As both kwargs are synonymous simply mirroring the other when one is present should be enough, which is six lines of code.

@sirouk

sirouk commented May 5, 2026

Copy link
Copy Markdown
Contributor Author
  • :THINKING present: strips suffix, sets both chat_template_kwargs.thinking and chat_template_kwargs.enable_thinking to true.
  • X-Enable-Thinking present as true: sets both to true.
  • X-Enable-Thinking present as false: sets both to false.
  • Manual body with only thinking: fills enable_thinking.
  • Manual body with only enable_thinking: fills thinking.
  • No per-model defaults added back.

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.

3 participants