Skip to content

fix: always include max_completion_tokens for OpenAI compatible providers #6267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

roomote[bot]
Copy link

@roomote roomote bot commented Jul 27, 2025

This PR fixes an issue where the response context limit was randomly ignored when using OpenAI Compatible API providers like koboldcpp.

Problem

When using OpenAI Compatible API providers, the max_completion_tokens parameter was only included when includeMaxTokens was explicitly set to true. When undefined, the parameter was omitted, causing providers like koboldcpp to fall back to their default limit (512 tokens), which is insufficient for generating complete diffs.

Solution

  • Changed the default behavior to include max_completion_tokens when includeMaxTokens is undefined (treating undefined as true for backward compatibility)
  • Added validation to only include positive max token values (avoiding invalid -1 values)
  • Updated tests to reflect the new default behavior
  • Added specific tests for OpenAI compatible provider scenarios

Testing

  • All existing tests pass
  • Added new tests to verify the fix works correctly
  • Tested with both includeMaxTokens: false (explicitly disabled) and undefined (default enabled)

Fixes #6265


Important

Ensure max_completion_tokens is always included for OpenAI compatible providers unless explicitly disabled, with updated tests in openai.spec.ts.

  • Behavior:
    • Always include max_completion_tokens for OpenAI compatible providers unless includeMaxTokens is explicitly false in openai.ts.
    • Treat includeMaxTokens as true when undefined for backward compatibility.
    • Validate to include only positive max_completion_tokens values.
  • Testing:
    • Updated tests in openai.spec.ts to reflect new default behavior.
    • Added tests for scenarios with OpenAI compatible providers, including when includeMaxTokens is false or undefined.
  • Misc:
    • Minor updates to comments and logic in addMaxTokensIfNeeded() in openai.ts.

This description was created by Ellipsis for e83f5cc. You can customize this summary. It will automatically update as commits are pushed.

…ders

- Changed default behavior to include max_completion_tokens when includeMaxTokens is undefined
- Added validation to only include positive max token values
- Updated tests to reflect new default behavior
- Added tests for OpenAI compatible provider scenarios

Fixes #6265
@roomote roomote bot requested review from mrubens, cte and jr as code owners July 27, 2025 01:26
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Jul 27, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 27, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jul 29, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jul 29, 2025
@daniel-lxs
Copy link
Collaborator

Closing for now, I don't think this is the right solution

@daniel-lxs daniel-lxs closed this Jul 30, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 30, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PR - Needs Preliminary Review size:M This PR changes 30-99 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

response context limit randomly ignored
3 participants