Repository navigation
fix(anthropic): resize max_tokens for adaptive thinking and thread stop_reason - #256
Conversation
Wibias
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34c5559b7c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Thanks — this is the direction we're adopting over #237, since it fixes both halves of #246: adaptive token sizing and end-to-end Two blockers before merge:
Requested change: preserve explicit |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
This has useful remaining value, but it is not mergeable as-is against current dev. The stop_reason/max_tokens completion plumbing is already present and now conflicts with dev. Please rebase and narrow the patch to the unsuperseded adaptive-thinking token budget work (including the 40192 ceiling and explicit maxOutputTokens coverage). The explicit-output override is a real limit/cost behavior change, so keep focused tests for that policy. Security review found no malware, exfiltration, dependency, or process-execution concern.
1f550b1 to
4d834c6
Compare
…op_reason (issue #246) Two compounding bugs caused claude-fable-5 with effort=max to exhaust its entire 8192-token budget on internal thinking and return zero visible text, reported as a normal completed turn. 1. Adaptive-thinking models now resize max_tokens using the same reasoningBudget + OUTPUT_HEADROOM formula as the non-adaptive branch, so high efforts get up to 32000 tokens instead of the 8192 default. 2. stop_reason is now captured from upstream Anthropic responses (streaming message_delta and non-streaming json.stop_reason) and threaded through the done event. When stop_reason is max_tokens, the bridge emits response.incomplete with max_output_tokens instead of response.completed, so clients can distinguish truncated turns from finished ones.
1. Mirror max-token truncation in batch responses: buildResponseJSON now reads stopReason from the done event and returns status incomplete with max_output_tokens details instead of always completed. 2. Preserve continuation state for truncated responses: onCompletedResponse now fires for incomplete responses so rememberResponseState caches partial output for previous_response_id replay. 3. Preserve explicit maxOutputTokens above the 32k ceiling: when a caller explicitly passes maxOutputTokens above REASONING_MAX_TOKENS_CEILING, the value is respected instead of being silently capped.
…iling for effort=max Addresses two blockers from PR #256 review: 1. Explicit caller maxOutputTokens is now passed through as-is. Previously Math.max(explicitMaxOut, floor) could silently raise a cost-capped limit. 2. For omitted limits, switch from REASONING_MAX_TOKENS_CEILING (32k) to ADAPTIVE_THINKING_CEILING (40192 = 32k budget + 8k OUTPUT_HEADROOM). At effort=max the old formula produced floor=40192 then clamped it back to 32000, leaving zero guaranteed visible-output room. The new ceiling allows 40192, so the model always has OUTPUT_HEADROOM tokens beyond the full reasoning budget. Regression test updated: exact value 40192 asserted (not just >8192). Two new tests cover small explicit limits not being raised.
…TIVE_THINKING_CEILING
4363c1d to
d88b98e
Compare
|
I rebased the current four-commit series onto Local verification on the rebased head:
Security review of the four-file diff found no credential handling, exfiltration, dependency, process-execution, or unsafe I/O changes. Waiting for the refreshed GitHub matrix before the final merge decision. |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Re-reviewed on rebased head d88b98e7. The two requested blockers are resolved: omitted adaptive max requests now reserve the full 8,192-token visible-output headroom at 40,192 total, and explicit maxOutputTokens values are preserved exactly. Streaming/batch truncation behavior is covered, all refreshed Linux/macOS/Windows checks pass, and the four-file diff contains no malware, credential exfiltration, unsafe I/O/process execution, or dependency changes.
…#256) Preserve explicit caller output caps, reserve visible-output headroom for omitted adaptive-thinking limits, and retain max-token truncation coverage.
Summary
Fixes #246.
Two compounding bugs caused
claude-fable-5(and other adaptive-thinking models) witheffort: "max"to exhaust the entire 8192-tokenmax_tokensbudget on internal thinking and return zero visible text, while the proxy reported the turn as a normal completed response.Bug 1:
max_tokensnot resized for adaptive thinkingThe adaptive-thinking branch set
thinking.type: "adaptive"andoutput_config.effortbut never resizedmax_tokens. At high efforts the model can consume the entire budget on internal thinking, leaving 0 tokens for visible output. The non-adaptive branch already handled this withreasoningBudget() + OUTPUT_HEADROOM.Fix: Apply the same resize formula to the adaptive branch. For
maxeffort this bumps from 8192 to 32000 (the existingREASONING_MAX_TOKENS_CEILING).Bug 2:
stop_reasondropped from upstream responsesNeither the streaming SSE parser nor the non-streaming path extracted
stop_reasonfrom the upstream Anthropic response. Thedoneevent only carriedusage, so the bridge always emittedresponse.completedeven when Anthropic returnedstop_reason: "max_tokens". Clients had no way to distinguish a truncated turn from a finished one.Fix:
stop_reasonfrom streamingmessage_delta.delta.stop_reasonand non-streamingjson.stop_reasondoneevent (new optionalstopReasonfield onAdapterEvent)stopReason === "max_tokens", the bridge emitsresponse.incompletewithincomplete_details: { reason: "max_output_tokens" }instead ofresponse.completedThe outbound layer in
src/claude/outbound.tsalready handlesresponse.incompletewithmax_output_tokenscorrectly, mapping it tostop_reason: "max_tokens"in the Anthropic Messages SSE output.Changes
src/types.ts: Add optionalstopReasonto thedoneevent inAdapterEventsrc/adapters/anthropic.ts: Resizemax_tokensfor adaptive thinking; capturestop_reasonin both streaming and non-streaming pathssrc/bridge.ts: Emitresponse.incompletewhenstopReason === "max_tokens"tests/anthropic-reasoning.test.ts: Verify adaptivemax_tokensresize for high efforttests/bridge.test.ts: VerifystopReasonthreading produces correct terminal eventsVerification
bun test --isolateon all anthropic + bridge + claude-messages tests: 100 pass, 0 failbun run typecheck: clean