fix: prevent title generation from hanging with Gemini 3 models#1602
fix: prevent title generation from hanging with Gemini 3 models#1602dgageot merged 1 commit intodocker:mainfrom
Conversation
There was a problem hiding this comment.
Code Review Summary
✅ No bugs found in the changed code.
This PR properly addresses the Gemini 3 model hanging issue by:
-
Correctly implementing thinking override logic - The new
buildConfig()code properly checks if thinking is explicitly disabled viaModelOptions.Thinking()and setsThinkingBudget=0before falling back to config-based thinking settings -
Proper nil safety - All pointer dereferences are guarded with nil checks (e.g.,
thinking != nil && !*thinking) -
Good context management - The timeout in
generator.gocorrectly wraps the entire title generation operation with proper cleanup viadefer cancel() -
Complete fix for FromModelOptions - The thinking option is now properly preserved when cloning providers
-
Comprehensive test coverage - Tests cover all three scenarios: thinking explicitly disabled, explicitly enabled, and not set
The implementation is well-documented and follows Go best practices for pointer handling, context management, and error prevention.
When using Gemini 3 models (e.g., gemini-3-flash-preview), title generation would hang because thinking was not being properly disabled. The low max_tokens (20) used for title generation left no room for thinking tokens, causing the API to hang. Changes: - Set ThinkingBudget=0 in Gemini client when ModelOptions.Thinking() is false, which completely disables thinking at the API level - Add 30-second timeout to title generation as a safety net - Fix FromModelOptions to preserve the thinking setting when cloning providers Assisted-By: cagent
4e42950 to
412c3e8
Compare
When using Gemini 3 models (e.g., gemini-3-flash-preview), title generation would hang because thinking was not being properly disabled. The low max_tokens (20) used for title generation left no room for thinking tokens, causing the API to hang.
Changes:
Assisted-By: cagent