Add comprehensive tests for reasoning effort edge case handling#49
Merged
SantiagoDePolonia merged 3 commits intoclaude/fix-thinking-max-tokens-vdp8gfrom Jan 26, 2026
Merged
Conversation
Co-authored-by: SantiagoDePolonia <16936376+SantiagoDePolonia@users.noreply.github.com>
Co-authored-by: SantiagoDePolonia <16936376+SantiagoDePolonia@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix MaxTokens and Effort checking based on review feedback
Add comprehensive tests for reasoning effort edge case handling
Jan 26, 2026
Contributor
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
14eca86
into
claude/fix-thinking-max-tokens-vdp8g
1 check passed
SantiagoDePolonia
added a commit
that referenced
this pull request
Jan 26, 2026
* fix: ensure MaxTokens >= budget when extended thinking is enabled When extended thinking is enabled via the reasoning parameter, Anthropic requires max_tokens to be at least the thinking budget_tokens. This fix ensures MaxTokens is bumped to the budget value when it's lower, in both ChatRequest and ResponsesRequest conversion paths. * Add logging for MaxTokens adjustment in Anthropic extended thinking (#50) * Initial plan * feat: add logging for MaxTokens adjustment in Anthropic provider Add informative log statements when MaxTokens is adjusted to meet Anthropic extended thinking requirements. Logs include original value, adjusted value, and the reason for adjustment. Co-authored-by: SantiagoDePolonia <16936376+SantiagoDePolonia@users.noreply.github.com> * refactor: extract MaxTokens logging into helper function Extract duplicate logging statements into a dedicated helper function to improve code maintainability and ensure consistency across both ChatCompletion and StreamChatCompletion flows. Co-authored-by: SantiagoDePolonia <16936376+SantiagoDePolonia@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: SantiagoDePolonia <16936376+SantiagoDePolonia@users.noreply.github.com> * Add comprehensive tests for reasoning effort edge case handling (#49) * Initial plan * test: add comprehensive tests for reasoning effort edge cases Co-authored-by: SantiagoDePolonia <16936376+SantiagoDePolonia@users.noreply.github.com> * fix: improve test code quality based on review feedback Co-authored-by: SantiagoDePolonia <16936376+SantiagoDePolonia@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: SantiagoDePolonia <16936376+SantiagoDePolonia@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: SantiagoDePolonia <16936376+SantiagoDePolonia@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review feedback questioned whether
req.Reasoning.Effortneeds explicit nil checking. SinceEffortis astring(not*string), it cannot be nil—unset values default to"". The existing checkreq.Reasoning.Effort != ""already handles all edge cases correctly.Changes
ReasoningobjectEffortstringChatRequestandResponsesRequestpathsExample behavior validated
No implementation changes required—existing logic is correct and now has full test coverage.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.