KEMBAR78
fix: Google: use maxTokens from params by nicolasf · Pull Request #734 · JetBrains/koog · GitHub
Skip to content

Conversation

nicolasf
Copy link
Contributor

@nicolasf nicolasf commented Sep 4, 2025

Current implementation has a hardcoded maxOutputTokens=2048. Use the value from prompt.params.maxTokens similar to the OpenAILLMClient.

Motivation and Context

During execution of structured output requests using Gemini 2.5 I encountered unexpected failures due to reaching MAX_TOKENS.
After checking the GoogleLLMClient implementation I noticed a hardcoded maxOutputTokens = 2048.
Checking the other clients, I'm replicating the same logic that exists on OpenAILLMClient, which applies the value from prompt.params.maxTokens.

Breaking Changes

No breaking changes.


Type of the changes

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tests improvement
  • Refactoring

Checklist

  • The pull request has a description of the proposed change
  • I read the Contributing Guidelines before opening the pull request
  • The pull request uses develop as the base branch
  • Tests for the changes have been added
  • All new and existing tests passed
Additional steps for pull requests adding a new feature
  • An issue describing the proposed change exists
  • The pull request includes a link to the issue
  • The change was discussed and approved in the issue
  • Docs have been added / updated

Copy link
Collaborator

@kpavlov kpavlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @nicolasf, for catching this!
Let's add/modify some test to avoid regressions in the future

@kpavlov kpavlov added the bugfix Something was fixed 🎉 label Sep 5, 2025
Copy link
Collaborator

@kpavlov kpavlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no tests

Current implementation has a hardcoded maxOutputTokens=2048.
Use the value from prompt.params.maxTokens similar to the OpenAILLMClient.
@nicolasf nicolasf force-pushed the fix-google-max-tokens-hardcoded branch from 174924f to bd14e68 Compare September 13, 2025 19:03
@nicolasf
Copy link
Contributor Author

Hi @kpavlov.
I have rebased and added 2 tests.
There's opportunity for improvement by refactoring and separating the logic but I think this is good enough for now.
Let me know if there's anything else that needs improvement.
Thanks.

@nicolasf nicolasf requested a review from kpavlov September 13, 2025 19:08
Copy link
Collaborator

@kpavlov kpavlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @nicolasf
I would also add a check that max tokens value is positive, but in general, it makes things better

@nicolasf
Copy link
Contributor Author

HI @kpavlov. Is it possible to get this merged before 0.5.0 is released?
This would make my dev team's life easier 😄.
Thanks!

@Ololoshechkin Ololoshechkin merged commit 45012a3 into JetBrains:develop Sep 30, 2025
9 of 10 checks passed
@sausti
Copy link

sausti commented Oct 2, 2025

Thanks for fixing this @nicolasf!

@Ololoshechkin, are there plans to release 0.5.0 or 0.4.3 soon (or even a feature or SNAPSHOT build)? We're loving Koog so far, but we cannot migrate to it fully, as this issue prevents the use of Gemini models.

Ololoshechkin pushed a commit that referenced this pull request Oct 2, 2025
karloti pushed a commit to karloti/koog that referenced this pull request Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Something was fixed 🎉

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants