fix(anthropic): forward --thinking to extended thinking API#233
fix(anthropic): forward --thinking to extended thinking API#233wangwllu wants to merge 5 commits into
Conversation
The Anthropic provider accepted `--thinking <effort>` (and the matching
`thinking` config field) without error, but never forwarded the value to
the API. Requests went out with no `thinking` block, so users who set a
reasoning level got no extended thinking on Claude Opus / Sonnet.
Two changes:
1. Pass `requestOptions.reasoningEffort` from `generateTextWithModelId`
into `completeAnthropicText`, mirroring how the OpenAI path uses it.
2. In `completeAnthropicText`, map the OpenAI effort enum to a pi-ai
`ThinkingLevel` and forward it as `reasoning`. When the model came
from `createSyntheticModel` (`reasoning: false` by default), opt the
model into thinking so the pi-ai adapter does not silently drop the
block.
Verified end-to-end against a custom `ANTHROPIC_BASE_URL` (Claude Opus
4.7 via a proxy that exposes the synthetic-model code path):
--thinking low -> budget_tokens: 2048
--thinking medium -> budget_tokens: 8192
--thinking high -> budget_tokens: 15360
--thinking xhigh -> budget_tokens: 15360
Before: `has_thinking: false` for every effort level.
|
Codex review: needs maintainer review before merge. Reviewed June 6, 2026, 5:09 AM ET / 09:09 UTC. Summary Reproducibility: yes. Source inspection on current main shows Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the provider-scoped text and streaming implementation after normal maintainer review, and track document-attachment thinking separately if full Anthropic coverage is required. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection on current main shows Is this the best way to solve the issue? Yes. The provider-scoped merge helper preserves OpenAI defaults while forwarding explicit CLI and per-attempt reasoning, and the Anthropic helper avoids enabling thinking on registered unsupported models. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against f40e26b1330e. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
…hetic models only Addresses the two P1 review findings on steipete#233: 1. Streaming path was uncovered. `--stream auto` enables streaming in TTY, so `summarize` from a terminal would call `streamSimple` without the `reasoning` option and the thinking block would never be sent. Thread the same handling through `streamAnthropic`-style dispatch in `generate-text-stream.ts`. 2. Earlier override flipped `reasoning: true` on ANY model whose pi-ai metadata reported `reasoning: false`. That includes registered Claude 3 / 3.5 models where extended thinking is unsupported, so users with a global `thinking` setting would go from successful no-thinking requests to API rejections. Restrict the override to *synthetic* models (`tryGetModel` miss — typical for custom `ANTHROPIC_BASE_URL` proxies in front of newer Claude versions). Shared helper `prepareAnthropicReasoning` is extracted to keep both dispatch sites in sync. Tests cover the four invariants: - no effort -> model and reasoning untouched - `none` -> off (no reasoning forwarded) - registered supported model -> reasoning forwarded, model metadata preserved - registered unsupported model (`reasoning: false`) -> metadata NOT mutated - synthetic model -> `reasoning: true` flipped so pi-ai forwards thinking Verified end-to-end against a custom `ANTHROPIC_BASE_URL` (Claude Opus 4.7 via proxy) on both `--stream on` and `--stream off`; outbound body now carries `thinking: {type: "enabled", budget_tokens: 15360, display: "summarized"}` in both paths.
|
Thanks for the thorough review — addressed both P1 findings in cdcea8f. Streaming path ( Unsupported-model metadata ( Extracted Re-verified end-to-end with a fetch wrapper logging outbound bodies:
Happy to also update the |
|
@clawsweeper re-review Addressed the P1 finding from your last verdict in commit 27c5d61: |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Addressed your latest P1 ( Provider-scope helper
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Addressed both new P1s from your previous review in commit 1.
2. URL markdown path now provider-scoped — Replaced the unscoped Coverage:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Current-head proof (commit 3d7cc96)Per ClawSweeper's latest review, here are the two outbound-request captures requested. Capture method: one-shot Node script ( Artifact 1 — CLI
|
Ready for maintainer reviewClawSweeper verdict on
The patch went through five iterations to converge:
The current behavior:
Two outbound-request artifacts attached at #issuecomment-4638051618 (captured via fetch-mock + production @steipete — tagging since this is your repo. Happy to address any further feedback. PR can be reviewed against current main (the branch was based before the pi-ai 0.78.0 bump but no logical conflicts). |
Proof
Latest head 3d7cc96 proof: see comment #4638051618 — outbound request captures for both (a) CLI
--thinking xhighcorrectly forwarding thethinkingblock to Anthropic and (b) persistedopenai.thinkingnot leaking into Anthropic requests.Problem
The Anthropic provider accepts
--thinking <effort>(and the matchingthinkingconfig field) without error, but never forwards the value to the API. Requests go out with nothinkingblock, so users who set a reasoning level onanthropic/...models get no extended thinking and don't know it.Reproduces on
mainagainst any Claude model:summarize "https://example.com" --model anthropic/claude-opus-4-5 --thinking xhigh --verboseVerbose log shows
model=anthropic/claude-opus-4-5but the outbound request has nothinkingfield.Root cause
Two layers:
generateTextWithModelIdinsrc/llm/generate-text.tsreadsrequestOptions.reasoningEffortfor the OpenAI path but never threads it intocompleteAnthropicText.completeAnthropicTextinsrc/llm/providers/anthropic.tscalls pi-ai'scompleteSimplewithout areasoningoption. Even when threaded through, models built viacreateSyntheticModeldefault toreasoning: false, which makes the pi-ai Anthropic adapter silently drop the block (seestreamSimpleAnthropic— it gates onmodel.reasoning).Fix
requestOptions?.reasoningEffortfromgenerateTextWithModelIdintocompleteAnthropicText, mirroring how the OpenAI path uses it.completeAnthropicText, map the OpenAI effort enum (low/medium/high/xhigh;noneis treated as off) to a pi-aiThinkingLeveland forward it asreasoning. When the model came fromcreateSyntheticModel(reasoning: false), opt the model into thinking so the pi-ai adapter does not silently drop the block.Verification
Tested end-to-end against a custom
ANTHROPIC_BASE_URL(Claude Opus 4.7 via a proxy that exposes the synthetic-model code path). The outbound request body (captured with afetchmonkey-patch wrapper) before vs after this PR:--thinkinghas_thinking: falsehas_thinking: false(unchanged)lowhas_thinking: falsethinking: {enabled, budget_tokens: 2048}mediumhas_thinking: falsethinking: {enabled, budget_tokens: 8192}highhas_thinking: falsethinking: {enabled, budget_tokens: 15360}xhighhas_thinking: falsethinking: {enabled, budget_tokens: 15360}(With
max_tokens=16384the pi-ai SDK caps thinking budget atmax_tokens − 1024, hencehighandxhighboth land at 15360 — that's SDK behavior, not this patch.)Summaries still return correctly in all cases.
Notes / scope
completeAnthropicTextis patched.completeAnthropicDocumentbuilds its request body by hand (no pi-ai dispatch) — extending thinking support there would be a separate, larger change touching the manual payload assembly.--thinking's CLI help text still reads "OpenAI reasoning effort"; happy to update it in this PR if you'd like the wording to acknowledge the Anthropic path now works.