Skip to content

fix(#3996): preserve Gemini user thinking budgets outside title generation - #4028

Draft
aheritier wants to merge 1 commit into
workspace-media-namingfrom
gemini-direct-image-output
Draft

fix(#3996): preserve Gemini user thinking budgets outside title generation#4028
aheritier wants to merge 1 commit into
workspace-media-namingfrom
gemini-direct-image-output

Conversation

@aheritier

@aheritier aheritier commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

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 test

Issue 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.

@aheritier aheritier added area/providers/gemini Google Gemini provider support area/runtime Runtime engine, agent loop execution, tool dispatch, loop detection area/tui For features/issues/fixes related to the TUI kind/fix PR fixes a bug (maps to fix:). Use on PRs only. labels Aug 22, 2026
@aheritier
aheritier force-pushed the gemini-direct-image-output branch from b4dbc21 to 84d8c20 Compare August 24, 2026 15:13
@aheritier
aheritier force-pushed the gemini-direct-image-output branch from 84d8c20 to a89ad52 Compare August 25, 2026 08:00
@aheritier
aheritier force-pushed the gemini-direct-image-output branch from a89ad52 to 1b7dc6c Compare August 25, 2026 08:17
@aheritier
aheritier force-pushed the gemini-direct-image-output branch from 1b7dc6c to 9b1176c Compare August 25, 2026 08:33
@aheritier
aheritier force-pushed the gemini-direct-image-output branch from 9b1176c to 53d7a5c Compare August 25, 2026 20:24
@aheritier
aheritier force-pushed the gemini-direct-image-output branch from 53d7a5c to c56cc55 Compare August 26, 2026 11:10
@aheritier
aheritier force-pushed the gemini-direct-image-output branch 3 times, most recently from 5339887 to c56cc55 Compare September 1, 2026 16:07
@aheritier
aheritier force-pushed the gemini-direct-image-output branch from c56cc55 to 2c8d6b1 Compare September 1, 2026 16:30
…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.
@aheritier
aheritier force-pushed the gemini-direct-image-output branch from 2c8d6b1 to 97877c1 Compare September 1, 2026 19:58

@aheritier aheritier left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/providers/gemini Google Gemini provider support area/runtime Runtime engine, agent loop execution, tool dispatch, loop detection area/tui For features/issues/fixes related to the TUI kind/fix PR fixes a bug (maps to fix:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant