feat(sse): retry without thinking when a model rejects it outright

[cryptiklemur]

A request that carries a thinking block dies with a 400 when the upstream rejects thinking by
naming the model instead of the offending field.

Ollama does this for Instruct-only models. Qwen3-Coder-30B-A3B-Instruct has no thinking mode,
and a /v1/messages request with thinking comes back as:

{"error":{"message":"\"Qwen3-Coder:latest\" does not support thinking","type":"invalid_request_error"}}

base.ts already has a 400-recovery chain, but neither detector can see anything to strip here.
findOffendingField needs a literal field name from KNOWN_OFFENDING_FIELDS, and
detectUnsupportedParam needs the phrase Unsupported parameter. This body has neither, so the
error goes straight back to the client. Every Claude Code request against a self-hosted
non-thinking model fails.

what changed

area	change
providerFieldStrips.ts	isUnsupportedThinkingError() plus REASONING_REQUEST_FIELDS
base.ts	a third branch in the 400 chain, between the literal-field strip and auto-learn
tests	4 new cases covering the match, near-misses, and that the existing detectors miss it

The branch drops whichever reasoning field the body actually carries rather than assuming one,
since an OpenAI-compatible target can arrive with reasoning_effort, reasoning, thinking or
think depending on the translation path. strippedFields already guards against a retry loop,
so each field is dropped at most once per request.

how to test

node --import tsx/esm --test tests/unit/provider-field-strips.test.ts
node --import tsx/esm --test tests/unit/param-filters-auto-learn.test.ts
npm run typecheck:core

All pass. Prettier and ESLint are clean on the changed lines.

limitations, please read

This is unit-tested, not verified against a live model. The regex is built from a real 400
captured from Ollama 0.34.0, but the Ollama instance was taken down before the patch was
written, so the retry path has not been exercised end to end. Someone with a non-thinking model
loaded should confirm the retry actually succeeds rather than merely firing.

base.ts has 6 pre-existing @typescript-eslint/no-unused-vars errors on lines 36, 50, 51 and
522-523. They are untouched and present on release/v3.8.51, left alone to keep this diff to
the fix.

[diegosouzapw]

Conceito aprovado, bem contido. Falta teste do branch e um rebase.

A premissa confere no tip: open-sse/executors/base.ts:1612-1670 tem só dois detectores na cadeia de recuperação de 400 — findOffendingField (precisa de nome literal em KNOWN_OFFENDING_FIELDS) e detectUnsupportedParam (precisa da frase Unsupported parameter). Um corpo "Qwen3-Coder:latest" does not support thinking não casa nenhum, e o 400 vai cru para o cliente. Buraco real.

Verifiquei o que mais me preocupava e está certo: não há tempestade de retry. O branch é else if dentro do bloco de 400, sem continue/urlIndex--, e strippedFields impede repetição — no máximo um fetch extra por request. E o regex reasoning\b não casa reasoning_effort (_ é word char), então não colide com o campo errado. Bem pensado.

Pendência 1 — zero teste do branch. tests/unit/provider-field-strips.test.ts testa só o detector puro. Nenhum teste toca base.ts: não há prova de que o retry dispara, de que dispara só uma vez, nem de que o body do retry sai sem o campo. Pela Hard Rule #18 isso não fecha. Há precedente fácil: tests/unit/context-editing-relays.test.ts já mocka exatamente essa cadeia de 400 com mockFetchErrorThenOk.

Pendência 2 — regressão pequena mas real. Em base.ts:1648-1652, quando isUnsupportedThinkingError casa mas nenhum campo de reasoning está no body (caso do Gemini, onde o thinking vive em generationConfig.thinkingConfig), o branch não faz nada e engole o fallback de auto-learn que o else rodaria. Melhor cair no else quando reasoningFields.length === 0.

Pendência 3 — prettier vermelho no arquivo de teste (linha em branco dupla antes do primeiro teste novo, e um assert.equal quebrado em 3 linhas que cabe em 1). Reprova o gate de formatação.

Seu CI vermelho é herdado, não defeito seu — provei rodando tests/unit/context-editing-relays.test.ts no tip atual: 4/4 verdes. É drift de base velha (sua base é de 16/09); um rebase resolve.

Decisão de produto: o cliente pediu thinking explicitamente e recebe resposta sem thinking, com uma linha de log. É coerente com o resto da cadeia de strip, mas é escolha de produto — vale confirmar.

[cryptiklemur]

thanks for the detailed read, all three are addressed and pushed at 5dd0ad7fd.

1. test for the branch. added tests/unit/base-thinking-unsupported-retry.test.ts, built on the mockFetchErrorThenOk pattern from context-editing-relays.test.ts like you suggested. four cases:

• a 400 naming the model strips reasoning_effort and retries exactly once
• the retry strips only the reasoning field actually on the body, not every REASONING_REQUEST_FIELDS entry
• an unrelated 400 does not strip or retry
• a thinking-unsupported 400 with no reasoning field falls through to auto-learn

2. the gemini regression. good catch. the branch now guards on thinkingFields.length > 0, so when the body carries no reasoning field the else runs and auto-learn gets its shot. that is the fourth test above.

3. prettier. npx prettier --check is clean on all four touched files.

base drift. merged release/v3.8.51 twice, most recently to pick up the base-red repairs in 7cc454d93 and 24fc202d9. locally base-thinking-unsupported-retry, provider-field-strips and context-editing-relays are 17/17 green on the merged tree.

the product call. you are right that it is a product choice, and my read is that a working answer without thinking beats a raw 400 the client cannot act on. that said, i do not want to decide it for you. happy to gate it behind a per-provider setting (something like strictThinking, default off) so anyone who wants the 400 to surface can keep it. say the word and i will add it in this pr or a follow-up, whichever you prefer.

[diegosouzapw]

Re-homed to release/v3.8.52: v3.8.51 entered its release freeze, so the branch now belongs to the release captain and development continues on the next cycle. Nothing is wrong with this PR — it just needed a live base. No action needed from you; CI will re-run against the new base.