Feat/add thinking budget to gemini #5395
Conversation
WalkthroughThis pull request adds support for the Changes
Sequence DiagramsequenceDiagram
participant UI as UI Config
participant Init as Initialization
participant Client as API Client
UI->>Init: Pass thinkingBudget parameter
Init->>Init: Parse thinkingBudget to integer
Init->>Client: Create client with config
Client->>Client: Set generationConfig.thinkingConfig
Note over Client: thinkingBudget applied<br/>for Gemini 2.5 models
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/components/nodes/chatmodels/ChatGoogleGenerativeAI/ChatGoogleGenerativeAI.ts (1)
231-251: Consider adding validation for thinkingBudget values.The code parses
thinkingBudgetbut doesn't validate that it's -1 (dynamic), 0 (disabled), or a positive integer as documented. Invalid values like negative numbers (other than -1) or non-numeric strings would parse to NaN or invalid values.Apply this diff to add validation:
if (thinkingBudget) obj.thinkingBudget = parseInt(thinkingBudget, 10) + const parsedBudget = parseInt(thinkingBudget, 10) + if (!isNaN(parsedBudget) && (parsedBudget === -1 || parsedBudget >= 0)) { + obj.thinkingBudget = parsedBudget + } + }Alternatively, rely on the API to validate and return errors for invalid values.
packages/components/nodes/chatmodels/ChatGoogleGenerativeAI/FlowiseChatGoogleGenerativeAI.ts (1)
665-665: Consider adding validation for thinkingBudget.Similar to how
temperature,topP, andtopKare validated in the constructor (lines 625-641), consider adding validation to ensurethinkingBudgetis -1, 0, or a positive integer.Apply this diff to add validation:
this.thinkingBudget = fields.thinkingBudget + if (this.thinkingBudget !== undefined && this.thinkingBudget !== -1 && this.thinkingBudget < 0) { + throw new Error('`thinkingBudget` must be -1 (dynamic), 0 (disabled), or a positive integer') + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/components/nodes/chatmodels/ChatGoogleGenerativeAI/ChatGoogleGenerativeAI.ts(3 hunks)packages/components/nodes/chatmodels/ChatGoogleGenerativeAI/FlowiseChatGoogleGenerativeAI.ts(4 hunks)packages/components/nodes/chatmodels/ChatGoogleGenerativeAI/utils/common.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build (ubuntu-latest, 18.15.0)
- GitHub Check: build
🔇 Additional comments (4)
packages/components/nodes/chatmodels/ChatGoogleGenerativeAI/utils/common.ts (1)
51-52: LGTM! Documentation clarifies default behavior.The comment helpfully documents that
convertAuthorToRolereturns 'user' as default instead of throwing an error, which aligns with the implementation at line 76.packages/components/nodes/chatmodels/ChatGoogleGenerativeAI/ChatGoogleGenerativeAI.ts (1)
177-188: LGTM! Clear parameter definition for Gemini 2.5 thinking budget.The input parameter is well-defined with a clear description of valid values (-1 for dynamic, 0 to disable, or positive integer) and appropriate conditional visibility for Gemini 2.5 models.
packages/components/nodes/chatmodels/ChatGoogleGenerativeAI/FlowiseChatGoogleGenerativeAI.ts (2)
178-179: LGTM! Clear interface definition for thinkingBudget.The interface field is properly typed and documented with the supported values for Gemini 2.5 thinking models.
605-605: LGTM! Field declaration follows existing pattern.
| if (this.thinkingBudget !== undefined) { | ||
| ;(this.client.generationConfig as any).thinkingConfig = { | ||
| ...(this.thinkingBudget !== undefined ? { thinkingBudget: this.thinkingBudget } : {}) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Simplify redundant conditional check.
The inner ternary operator is unnecessary since we're already inside an if (this.thinkingBudget !== undefined) block.
Apply this diff to simplify:
if (this.thinkingBudget !== undefined) {
;(this.client.generationConfig as any).thinkingConfig = {
- ...(this.thinkingBudget !== undefined ? { thinkingBudget: this.thinkingBudget } : {})
+ thinkingBudget: this.thinkingBudget
}
}Note: The type cast to any is acceptable here since thinkingConfig is a new API feature that may not yet be in the TypeScript definitions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (this.thinkingBudget !== undefined) { | |
| ;(this.client.generationConfig as any).thinkingConfig = { | |
| ...(this.thinkingBudget !== undefined ? { thinkingBudget: this.thinkingBudget } : {}) | |
| } | |
| } | |
| if (this.thinkingBudget !== undefined) { | |
| ;(this.client.generationConfig as any).thinkingConfig = { | |
| thinkingBudget: this.thinkingBudget | |
| } | |
| } |
🤖 Prompt for AI Agents
In
packages/components/nodes/chatmodels/ChatGoogleGenerativeAI/FlowiseChatGoogleGenerativeAI.ts
around lines 685-689, the inner ternary is redundant because you're already
inside an if checking this.thinkingBudget !== undefined; replace the current
assignment with a direct object assignment (e.g., (this.client.generationConfig
as any).thinkingConfig = { thinkingBudget: this.thinkingBudget }) to simplify
the code while keeping the any cast as noted.
| if (this.thinkingBudget !== undefined) { | ||
| ;(this.client.generationConfig as any).thinkingConfig = { | ||
| ...(this.thinkingBudget !== undefined ? { thinkingBudget: this.thinkingBudget } : {}) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Simplify redundant conditional check.
Same issue as in the constructor: the inner ternary operator is unnecessary since we're already inside an if (this.thinkingBudget !== undefined) block.
Apply this diff to simplify:
if (this.thinkingBudget !== undefined) {
;(this.client.generationConfig as any).thinkingConfig = {
- ...(this.thinkingBudget !== undefined ? { thinkingBudget: this.thinkingBudget } : {})
+ thinkingBudget: this.thinkingBudget
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (this.thinkingBudget !== undefined) { | |
| ;(this.client.generationConfig as any).thinkingConfig = { | |
| ...(this.thinkingBudget !== undefined ? { thinkingBudget: this.thinkingBudget } : {}) | |
| } | |
| } | |
| if (this.thinkingBudget !== undefined) { | |
| ;(this.client.generationConfig as any).thinkingConfig = { | |
| thinkingBudget: this.thinkingBudget | |
| } | |
| } |
🤖 Prompt for AI Agents
In
packages/components/nodes/chatmodels/ChatGoogleGenerativeAI/FlowiseChatGoogleGenerativeAI.ts
around lines 696 to 700, the inner ternary checking this.thinkingBudget is
redundant because the surrounding if already ensures it's defined; replace the
current assignment to this.client.generationConfig.thinkingConfig with a direct
object that sets thinkingBudget to this.thinkingBudget (i.e., assign {
thinkingBudget: this.thinkingBudget }) so the extra conditional is removed and
the code is simplified.
Summary by CodeRabbit
New Features
thinkingBudgetparameter to ChatGoogleGenerativeAI node for Gemini 2.5 models, enabling users to control the number of thinking tokens (-1 for dynamic allocation, 0 to disable, or custom positive integer).