Skip to content

fix: preserve explicit false Gemini includeThoughts - #5362

Closed
Q1Xuan wants to merge 2 commits into
QuantumNous:mainfrom
Q1Xuan:codex/gemini-include-thoughts-cloud-test
Closed

fix: preserve explicit false Gemini includeThoughts#5362
Q1Xuan wants to merge 2 commits into
QuantumNous:mainfrom
Q1Xuan:codex/gemini-include-thoughts-cloud-test

Conversation

@Q1Xuan

@Q1Xuan Q1Xuan commented Jun 7, 2026

Copy link
Copy Markdown

变更描述 / Description

Gemini thinkingConfig.includeThoughts 是可选布尔参数,需要区分“没有传入”和“明确传入 false”。原实现使用 boolomitempty,客户端明确设置 includeThoughts: falseinclude_thoughts: false 后,重新序列化转发时会把该字段省略,导致上游无法收到显式关闭配置。

本次将 GeminiThinkingConfig.IncludeThoughts 改为 *bool,并同步调整 Gemini thinking adaptor 与 extra_body.google.thinking_config 的合并逻辑。这样只有在请求实际提供该字段,或由 thinking_budget 推导出该字段时才覆盖配置,可以保留显式 false,也避免未传 include_thoughts 时用默认零值误覆盖已有设置。

变更类型 / Type of change

  • Bug 修复 (Bug fix)
  • 新功能 (New feature)
  • 性能优化 / 重构 (Refactor)
  • 文档更新 (Documentation)

关联任务 / Related Issue

✅ 提交前检查项 / Checklist

  • 人工确认: 我已整理并撰写此描述,没有直接粘贴未经处理的输出。
  • 非重复提交: 我已搜索现有 Issues 与 PRs,确认不是重复提交。
  • Bug fix 说明: 此修复针对可选布尔字段显式 false 被序列化省略的问题。
  • 变更理解: 我已理解这些更改的工作原理及可能影响。
  • 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
  • 验证: 已通过 fork 上的 GitHub Actions 目标测试。
  • 安全合规: 代码中无敏感凭据,且符合项目代码规范。

运行证明 / Proof of Work

Fork GitHub Actions targeted test passed:

go test ./dto ./relay/channel/gemini

Run: https://github.com/Q1Xuan/new-api/actions/runs/27093099180

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

GeminiThinkingConfig.IncludeThoughts changes from bool to *bool to distinguish unset, true, and explicitly false states. DTO unmarshalling adapts to the pointer type. Relay adaptors now use pointer-aware assignments and conditional merges. Two new tests validate the explicit false behavior through serialization and conversion flows.

Changes

Nullable IncludeThoughts for state preservation

Layer / File(s) Summary
DTO type contract and serialization
dto/gemini.go, dto/gemini_generation_config_test.go
GeminiThinkingConfig.IncludeThoughts becomes *bool (was bool). UnmarshalJSON assigns the snake_case include_thoughts value directly to the pointer field. New test validates explicit false is preserved through unmarshal-marshal-unmarshal cycles in both camelCase and snake_case JSON shapes.
Relay adaptor pointer-safe conversion logic
relay/channel/gemini/relay-gemini.go, relay/channel/gemini/relay_gemini_usage_test.go
ThinkingAdaptor sets IncludeThoughts via common.GetPointer(true) for -thinking-<budget>, -thinking, and effort-suffix branches. CovertOpenAI2Gemini derives IncludeThoughts from thinking_budget (true if positive, false if zero/negative), preserves provided include_thoughts as a pointer, and only merges non-nil values to avoid clearing unset fields. Integration test verifies explicit false from extra_body.google.thinking_config.include_thoughts is preserved in the converted Gemini config.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • QuantumNous/new-api#2274: Modifies dto/gemini.go GeminiThinkingConfig JSON unmarshalling for include_thoughts field; this PR additionally changes the field type to *bool and implements pointer-safe preservation of explicit false.
  • QuantumNous/new-api#2344: Updates Gemini "thinking" configuration handling in relay/channel/gemini/relay-gemini.go for GenerationConfig.ThinkingConfig.IncludeThoughts; this PR switches to pointer-safe nil/false handling while that PR adds thinking-level-suffix parsing.

Poem

🐰 A nibble here, a pointer there,
Explicit false gets proper care,
Three states now shine: unset, true, nay,
No silent loss along the way! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and accurately describes the main change: making the GeminiThinkingConfig.IncludeThoughts field a pointer to preserve explicit false values instead of omitting them.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
relay/channel/gemini/relay-gemini.go (1)

267-281: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject non-integer thinking_budget before casting

Line 270 truncates JSON numbers via int(v), so values like 0.9 become 0, and then Lines 274-277 can incorrectly force IncludeThoughts=false. This silently violates the “must be an integer” contract.

Suggested fix
 					if thinkingBudget, exists := thinkingConfig["thinking_budget"]; exists {
 						switch v := thinkingBudget.(type) {
 						case float64:
+							if v != float64(int(v)) {
+								return nil, errors.New("extra_body.google.thinking_config.thinking_budget must be an integer")
+							}
 							budgetInt := int(v)
 							tempThinkingConfig.ThinkingBudget = common.GetPointer(budgetInt)
 							if budgetInt > 0 {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@relay/channel/gemini/relay-gemini.go` around lines 267 - 281, The code
currently casts a float64 thinkingBudget to int causing values like 0.9 to
truncate; update the parsing logic for thinkingConfig["thinking_budget"] (where
you handle thinkingBudget and set tempThinkingConfig.ThinkingBudget and
tempThinkingConfig.IncludeThoughts) to reject non-integer numeric values before
casting: when thinkingBudget is a float64 verify that float64(int(v)) == v (or
otherwise check for fractional part) and return the existing error
"extra_body.google.thinking_config.thinking_budget must be an integer" if it has
a fractional part; only then convert to int, set
tempThinkingConfig.ThinkingBudget, and set IncludeThoughts based on >0 as
currently implemented. Ensure this preserves the current behavior for exact
integer floats and still errors for non-integer floats and non-numeric types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@relay/channel/gemini/relay-gemini.go`:
- Around line 267-281: The code currently casts a float64 thinkingBudget to int
causing values like 0.9 to truncate; update the parsing logic for
thinkingConfig["thinking_budget"] (where you handle thinkingBudget and set
tempThinkingConfig.ThinkingBudget and tempThinkingConfig.IncludeThoughts) to
reject non-integer numeric values before casting: when thinkingBudget is a
float64 verify that float64(int(v)) == v (or otherwise check for fractional
part) and return the existing error
"extra_body.google.thinking_config.thinking_budget must be an integer" if it has
a fractional part; only then convert to int, set
tempThinkingConfig.ThinkingBudget, and set IncludeThoughts based on >0 as
currently implemented. Ensure this preserves the current behavior for exact
integer floats and still errors for non-integer floats and non-numeric types.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9385d831-68cc-4de8-82d2-761b38250d21

📥 Commits

Reviewing files that changed from the base of the PR and between 4ca47ee and c0a7369.

📒 Files selected for processing (4)
  • dto/gemini.go
  • dto/gemini_generation_config_test.go
  • relay/channel/gemini/relay-gemini.go
  • relay/channel/gemini/relay_gemini_usage_test.go

@Q1Xuan Q1Xuan closed this Jun 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant