fix(#3996): preserve Gemini user thinking budgets outside title generation - #4028
fix(#3996): preserve Gemini user thinking budgets outside title generation#4028aheritier wants to merge 1 commit into
Conversation
b4dbc21 to
84d8c20
Compare
84d8c20 to
a89ad52
Compare
a89ad52 to
1b7dc6c
Compare
1b7dc6c to
9b1176c
Compare
9b1176c to
53d7a5c
Compare
53d7a5c to
c56cc55
Compare
5339887 to
c56cc55
Compare
c56cc55 to
2c8d6b1
Compare
…ation Title generation requests must omit thinkingConfig so image-capable Gemini models can produce a plain text title. Keep the explicit no-thinking behavior for MCP sampling and ordinary requests unchanged, including Gemini 3 minimum reasoning settings and Gemini 2.5 zero-budget suppression.
2c8d6b1 to
97877c1
Compare
aheritier
left a comment
There was a problem hiding this comment.
Reviewed the three-dot delta vs workspace-media-naming (b18b517): 1 commit, 2 files (+70/-2). CI is green for 97877c1 (13 check-runs, incl. lint, build-and-test, CodeQL, windows-tests; publish jobs skipped). Locally: build + GOOS=windows build clean, go test -race green on pkg/model/provider/gemini, pkg/sessiontitle, pkg/model/provider, golangci-lint 0 issues, and the new test is load-bearing (reverting the production hunk fails title_generation_omits_thinking_config).
[blocking] Gemini 3 title generation loses the safeguard the comment still documents. client.go:455-458 returns before the Gemini-3 branch at :464-472, which set ThinkingLevelLow and raise MaxOutputTokens to a 200-token floor. Title generation clones with MaxTokens=20 (sessiontitle/generator.go:32,:148 -> clone.go:56-58), so the effective config for gemini-3-flash goes from {ThinkingLevel:LOW, MaxOutputTokens:200} to {ThinkingConfig:nil, MaxOutputTokens:20}. Per the comment retained at :460-463, Gemini 3+ models always think, so an omitted thinkingConfig means default thinking against a 20-token cap -> MAX_TOKENS with no visible text, which generateOnce reports as "empty title output" (generator.go:164-167) and Generate then burns every fallback (:135). Please narrow the omission to models that actually reject thinkingConfig (image-output models), or keep the MaxOutputTokens floor when omitting it, and add a title case asserting that floor.
[blocking] Title/body describe this change inversely. The only production change gates the existing NoThinking branch on GeneratingTitle(); the user-budget branch (:479-486) is untouched, so ordinary/image-output requests are byte-identical to base. The body claims the opposite ("preserves ... for ordinary image-output requests while retaining the special title-generation behavior") and lists modality/guard/runtime-TUI validation that is not in this delta. The commit body is accurate — please align title + body (and note that Compacting() is deliberately unaffected: compactor.go:202-206 never sets NoThinking).
[should-fix] return config at :457 also bypasses the structured-output block at :488-491 (verified: title+structured yields ResponseMIMEType "" vs "application/json" on the non-title path). Harmless today only because sessiontitle passes WithStructuredOutput(nil) (generator.go:147). Prefer scoping the condition: if c.ModelOptions.NoThinking() && !c.ModelOptions.GeneratingTitle() { ... }.
[should-fix] :428 doc comment says BuildConfig for the unexported buildConfig — and it's an unrelated drive-by edit in an otherwise focused diff.
[optional] The added cases use gemini-3-flash / gemini-2.5-flash; the motivating image-capable model (e.g. gemini-2.5-flash-image) isn't exercised, and nothing pins that ordinary image-model requests still carry the user's budget.
Checked and clean: no transport split (buildConfig has no apiSurface branching); ResponseModalities is applied after buildConfig (:763-768) so the #4020 guard is intact; the reranking caller (:881) overrides ThinkingConfig explicitly; no clone shared-state mutation (clone.go copies by value, -race clean); thinking_budget: 0 still disables thinking; modelerrors untouched. The pkg/mcp clientSession rename appears exactly once in the stack (9ffb3af, #4024 lane) — no duplication.
What
Preserves Gemini user thinking budgets for ordinary image-output requests while retaining the special title-generation behavior, with guard and integration tests.
Why
Image-output requests must honor the user's configured reasoning budget; only title generation should use its constrained policy.
Validation
task test; Gemini client, modality, guard, and runtime/TUI integration tests.Test instructions
Run:
task testIssue a Gemini image-output request with a non-default user thinking budget and inspect the outgoing request. Expected: the configured budget is preserved. Trigger session title generation separately; expected: its constrained title-generation policy remains unchanged.