Skip to content

feat: 适配 Gemini 多模态参数及 OpenRouter 逻辑 - #3102

Closed
YanXi9999 wants to merge 1 commit into
QuantumNous:mainfrom
YanXi9999:feat-my-update
Closed

feat: 适配 Gemini 多模态参数及 OpenRouter 逻辑#3102
YanXi9999 wants to merge 1 commit into
QuantumNous:mainfrom
YanXi9999:feat-my-update

Conversation

@YanXi9999

@YanXi9999 YanXi9999 commented Mar 3, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

Release Notes

  • New Features
    • Added Google Gemini API integration with support for chat, embeddings, and image generation.
    • Added thinking and reasoning capabilities for Gemini models with configurable budgets.
    • Added function and tool calling support for Gemini models.
    • Added OpenRouter channel support.

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Introduces comprehensive Gemini API integration across a new DTO module and relay handler. The DTO module defines 40+ types for Gemini requests/responses, including chat, embeddings, and image generation, with custom JSON unmarshalling for field-name flexibility. The relay module translates OpenAI-style requests to Gemini API calls, with support for streaming, tool calling, thinking configurations, and media handling.

Changes

Cohort / File(s) Summary
Gemini DTO Module
dto/gemini.go
Defines 40+ public types for Gemini API integration: chat/embedding/image request-response structures, tool configurations, safety settings, thinking configs, file/media handling, and usage metadata. Includes custom UnmarshalJSON implementations for flexible field naming (snake_case/camelCase) and helper methods (GetTokenCountMeta, IsStream, SetModelName, GetTools).
Gemini Relay Integration
relay/channel/gemini/relay-gemini.go
Transforms OpenAI-style requests to Gemini API calls with support for streaming, thinking budgets, tool conversion, and response mapping. Handles MIME whitelisting, thinking adaptor orchestration, content parsing (system/user/tool messages), and streaming response translation. Includes utilities for embedding, image generation, and FetchGeminiModels for model enumeration.
OpenRouter Constants
relay/channel/openrouter/constant.go
Declares two exported variables: ModelList (empty string slice) and ChannelName ("openrouter").

Sequence Diagram(s)

sequenceDiagram
    participant Client as OpenAI Client
    participant Relay as Gemini Relay
    participant DTO as DTO Layer
    participant API as Gemini API
    
    Client->>Relay: OpenAI ChatCompletion Request
    Relay->>DTO: Parse to GeneralOpenAIRequest
    Relay->>Relay: CovertOpenAI2Gemini()
    Relay->>Relay: Map temperature, top_p, max_tokens
    Relay->>Relay: Convert tools/function calls
    Relay->>Relay: ThinkingAdaptor() - adjust thinking budget
    Relay->>DTO: Create GeminiChatRequest
    Relay->>API: Send Gemini API Request
    API->>API: Process with thinking/tools
    API-->>Relay: Streaming Response Chunks
    Relay->>Relay: Map Gemini response to OpenAI format
    Relay->>Relay: Handle tool calls, reasoning, content
    Relay->>Relay: Translate finish_reason, compute usage
    Relay-->>Client: OpenAI-style Streaming Response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • seefs001

Poem

🐰 Hop, hop! Gemini's gates swing wide today,
DTOs dance in camelCase array,
From OpenAI requests to Gemini's shore,
Streaming and thinking and so much more! ✨
bzzt bzzt goes the token counter's cheer,
New models await—let's get coding here!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: adding Gemini multimodal parameter support and OpenRouter logic integration across three files.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Desktop/电信/测试/newapi/new-api-main/dto/gemini.go`:
- Around line 548-550: The loop over r.Requests currently assumes each element
is non-nil and calls request.GetTokenCountMeta() (and another method later at
the same loop where request is dereferenced); add a nil guard at the top of the
loop (e.g., if request == nil { continue }) so any null entries in r.Requests
are skipped before calling request.GetTokenCountMeta() and the subsequent
request methods referenced around lines 562–564, preventing panics when payloads
include null requests.
- Around line 133-140: The prefix checks on r.Tools are whitespace-sensitive; in
the function handling tool parsing (the block using r.Tools, common.Unmarshal
and logger.LogError) trim leading/trailing whitespace/newlines before calling
strings.HasPrefix so JSON like "  [ ... ]" or "\n{...}" is recognized; replace
uses of strings.HasPrefix(string(r.Tools), ...) with checks against
strings.HasPrefix(strings.TrimSpace(string(r.Tools)), ...) (or equivalent
bytes.TrimSpace) and then unmarshal the original or trimmed bytes as appropriate
to preserve data.
- Around line 167-170: The DTO uses non-pointer scalar fields so zero-values
(false/0) get omitted on re-marshal; change IncludeThoughts to *bool,
SampleCount and OutputDimensionality to *int (and any other optional scalar like
ThinkingLevel if it must preserve an explicit empty value—change to *string) in
the gemini DTOs (symbols: IncludeThoughts, ThinkingBudget, ThinkingLevel,
SampleCount, OutputDimensionality), keep the json:"...,omitempty" tags, and
update all code that constructs or reads these fields to handle nil vs non-nil
(use nil to mean unset, dereference safely when present and provide defaults
where the upstream call expects concrete values).

In `@Desktop/电信/测试/newapi/new-api-main/relay/channel/gemini/relay-gemini.go`:
- Around line 249-252: The current logic sets adaptorWithExtraBody = true
whenever extraBody["google"] exists, which erroneously disables the default
ThinkingAdaptor; change it so adaptorWithExtraBody is only set when the google
map contains a thinking config key (accept both "thinking_config" and
"thinkingConfig" variants), e.g. inspect googleBody for
"thinking_config"/"thinkingConfig" (and normalize snake/camel case) before
setting adaptorWithExtraBody and skipping ThinkingAdaptor; keep the existing
UpstreamModelName "-nothinking" check intact and ensure image_config alone does
not trigger adaptorWithExtraBody.
- Around line 1037-1042: The code is generating a fresh UUID for each chunk
(fmt.Sprintf("call_%s", common.GetUUID())) which prevents later
aggregation/merging logic from recognizing the same tool call; change the ID to
be stable per logical function call by using the original call identifier (e.g.,
item.FunctionCall.ID or an existing callID variable passed into this flow) when
constructing the FunctionResponse (ID, Type, Function:
dto.FunctionResponse{...}) so repeated chunks share the same ID; if an original
call ID is not available, derive a deterministic ID (e.g., hash of FunctionName
+ Arguments) instead of calling common.GetUUID() so the ID remains consistent
across chunks and aligns with the code that maps/merges tool calls.
- Around line 578-585: The parsing loop advances the local variable text (text =
text[closeIdx+1:]) when a markdown image is found but never appends any trailing
remainder when hasMarkdownImage is true, so trailing text is dropped; update the
logic around hasMarkdownImage/parts to, after processing images, check if text
!= "" and append a dto.GeminiPart{Text: text} (using the same dto.GeminiPart
shape you use for the no-image branch) so that the leftover substring after
closeIdx is preserved; reference the variables and symbols text, closeIdx,
hasMarkdownImage, parts, dto.GeminiPart and part.Text to locate and modify the
code.
- Around line 1633-1635: The code appends nextPageToken directly into the url
string (see variable url and nextPageToken), which can break if the token
contains reserved characters; update the code to percent-escape the token or,
better, build the URL using net/url: parse the base URL, call
urlObj.Query().Set("pageToken", nextPageToken) (or use
url.QueryEscape(nextPageToken)), then reassign url = urlObj.String(); add the
necessary import for net/url and ensure the same approach is used wherever
pageToken is appended.
- Around line 1242-1261: The loop currently mixes thought and non-thought text
into one slice and when any part.Thought is true it writes everything to
choice.Delta.SetReasoningContent, hiding normal output; fix by maintaining two
separate slices (e.g., thoughtTexts and normalTexts) and append to thoughtTexts
when part.Thought is true and to normalTexts for normal branches (including
ExecutableCode/CodeExecutionResult/Text); after the loop, always call
choice.Delta.SetContentString(strings.Join(normalTexts, "\n")) and if there were
any thoughts call choice.Delta.SetReasoningContent(strings.Join(thoughtTexts,
"\n")), ensuring SetReasoningContent only contains true thought content and
SetContentString always contains non-thought content (use the same symbols
part.Thought, part.ExecutableCode, part.CodeExecutionResult,
choice.Delta.SetReasoningContent, choice.Delta.SetContentString).
- Around line 468-471: Several spots in relay-gemini.go use encoding/json
directly (e.g., json.Unmarshal called on contentStr into contentMap/contentSlice
and other marshal/unmarshal sites); replace those direct calls with the
repository wrappers: use common.Unmarshal([]byte(contentStr), &contentMap) or
common.Unmarshal([]byte(contentStr), &contentSlice) and use
common.Marshal(value) where json.Marshal was used. Locate occurrences by
searching for json.Unmarshal/json.Marshal and the variables contentStr,
contentMap, contentSlice (and the other call sites reported) and swap each call
to the corresponding common.Unmarshal/common.Marshal signature, keeping error
handling unchanged.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 196f534 and 35b8bb1.

📒 Files selected for processing (3)
  • Desktop/电信/测试/newapi/new-api-main/dto/gemini.go
  • Desktop/电信/测试/newapi/new-api-main/relay/channel/gemini/relay-gemini.go
  • Desktop/电信/测试/newapi/new-api-main/relay/channel/openrouter/constant.go

Comment on lines +133 to +140
if strings.HasPrefix(string(r.Tools), "[") {
// is array
if err := common.Unmarshal(r.Tools, &tools); err != nil {
logger.LogError(nil, "error_unmarshalling_tools: "+err.Error())
return nil
}
} else if strings.HasPrefix(string(r.Tools), "{") {
// is object

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.

⚠️ Potential issue | 🟡 Minor

GetTools is whitespace-sensitive and may miss valid JSON.

Line 133 and Line 139 check raw prefix without trimming. Leading spaces/newlines will bypass both object/array branches.

🔧 Proposed fix
- if strings.HasPrefix(string(r.Tools), "[") {
+ toolsStr := strings.TrimSpace(string(r.Tools))
+ if strings.HasPrefix(toolsStr, "[") {
    ...
- } else if strings.HasPrefix(string(r.Tools), "{") {
+ } else if strings.HasPrefix(toolsStr, "{") {
    ...
}
📝 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.

Suggested change
if strings.HasPrefix(string(r.Tools), "[") {
// is array
if err := common.Unmarshal(r.Tools, &tools); err != nil {
logger.LogError(nil, "error_unmarshalling_tools: "+err.Error())
return nil
}
} else if strings.HasPrefix(string(r.Tools), "{") {
// is object
toolsStr := strings.TrimSpace(string(r.Tools))
if strings.HasPrefix(toolsStr, "[") {
// is array
if err := common.Unmarshal(r.Tools, &tools); err != nil {
logger.LogError(nil, "error_unmarshalling_tools: "+err.Error())
return nil
}
} else if strings.HasPrefix(toolsStr, "{") {
// is object
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Desktop/电信/测试/newapi/new-api-main/dto/gemini.go` around lines 133 - 140, The
prefix checks on r.Tools are whitespace-sensitive; in the function handling tool
parsing (the block using r.Tools, common.Unmarshal and logger.LogError) trim
leading/trailing whitespace/newlines before calling strings.HasPrefix so JSON
like "  [ ... ]" or "\n{...}" is recognized; replace uses of
strings.HasPrefix(string(r.Tools), ...) with checks against
strings.HasPrefix(strings.TrimSpace(string(r.Tools)), ...) (or equivalent
bytes.TrimSpace) and then unmarshal the original or trimmed bytes as appropriate
to preserve data.

Comment on lines +167 to +170
IncludeThoughts bool `json:"includeThoughts,omitempty"`
ThinkingBudget *int `json:"thinkingBudget,omitempty"`
// TODO Conflict with thinkingbudget.
ThinkingLevel string `json:"thinkingLevel,omitempty"`

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "gemini.go" -path "*/dto/*" -type f

Repository: QuantumNous/new-api

Length of output: 128


🏁 Script executed:

wc -l ./dto/gemini.go

Repository: QuantumNous/new-api

Length of output: 82


🏁 Script executed:

sed -n '165,175p' ./dto/gemini.go

Repository: QuantumNous/new-api

Length of output: 500


🏁 Script executed:

sed -n '485,492p' ./dto/gemini.go

Repository: QuantumNous/new-api

Length of output: 329


🏁 Script executed:

sed -n '508,515p' ./dto/gemini.go

Repository: QuantumNous/new-api

Length of output: 404


🏁 Script executed:

# Check the struct definitions and their context
sed -n '160,180p' ./dto/gemini.go
echo "---"
sed -n '480,495p' ./dto/gemini.go
echo "---"
sed -n '500,520p' ./dto/gemini.go

Repository: QuantumNous/new-api

Length of output: 2059


🏁 Script executed:

# Check if these are relay request structs and how they're used
grep -n "type GeminiThinkingConfig\|type GeminiImageParameters\|type GeminiEmbeddingRequest" ./dto/gemini.go
echo "---"
# Check for relay/convert references
grep -n "relay\|convert" ./dto/gemini.go | head -20
echo "---"
# Check if file imports or uses relay patterns
head -50 ./dto/gemini.go

Repository: QuantumNous/new-api

Length of output: 1778


🏁 Script executed:

# Check which structs embed or use these types, and if they're relay request DTOs
grep -n "GeminiThinkingConfig\|GeminiImageParameters\|GeminiEmbeddingRequest" ./dto/gemini.go | head -30
echo "---"
# Check if these DTOs are used in relay/convert paths
find ./relay -name "*.go" -type f | xargs grep -l "GeminiThinkingConfig\|GeminiImageParameters\|GeminiEmbeddingRequest" 2>/dev/null | head -10

Repository: QuantumNous/new-api

Length of output: 1149


🏁 Script executed:

# Check how these are used in relay paths
sed -n '340,350p' ./dto/gemini.go
echo "---"
sed -n '475,495p' ./dto/gemini.go
echo "---"
# Check relay usage
grep -A 5 -B 5 "GeminiEmbeddingRequest\|GeminiImageParameters" ./relay/channel/gemini/adaptor.go | head -40

Repository: QuantumNous/new-api

Length of output: 1810


🏁 Script executed:

# Check the relay adaptor and how GeminiEmbeddingRequest and GeminiImageParameters are used
cat ./relay/channel/gemini/adaptor.go | head -100

Repository: QuantumNous/new-api

Length of output: 2854


🏁 Script executed:

# Check if GeminiEmbeddingRequest is used in relay conversion
grep -A 20 "GeminiEmbeddingRequest" ./relay/channel/gemini/adaptor.go
echo "---"
# Check the relay conversion for embeddings
grep -n "ConvertEmbedding\|Embedding" ./relay/channel/gemini/adaptor.go

Repository: QuantumNous/new-api

Length of output: 390


🏁 Script executed:

sed -n '196,260p' ./relay/channel/gemini/adaptor.go

Repository: QuantumNous/new-api

Length of output: 2394


🏁 Script executed:

# Check how GeminiThinkingConfig and GeminiChatRequest are used in relay
grep -n "ConvertGeminiRequest" ./relay/channel/gemini/adaptor.go -A 10
echo "---"
# Verify these structs are parsed from client JSON
grep -B 5 "GeminiChatRequest\|GeminiImageRequest" ./relay/channel/gemini/relay-gemini.go | head -30

Repository: QuantumNous/new-api

Length of output: 1089


🏁 Script executed:

# Check how the requests are parsed from client JSON and marshaled
grep -n "common.Unmarshal\|json.Unmarshal" ./relay/channel/gemini/relay-gemini.go | head -10
echo "---"
# Check the relay handler entry point
sed -n '1,50p' ./relay/channel/gemini/relay-gemini.go

Repository: QuantumNous/new-api

Length of output: 2168


🏁 Script executed:

# Check where GeminiEmbeddingRequest is actually used and parsed
grep -rn "GeminiEmbeddingRequest" ./relay/ --include="*.go"
echo "---"
# Check if it's parsed from client input
grep -rn "GeminiEmbeddingRequest" ./controller/ --include="*.go"

Repository: QuantumNous/new-api

Length of output: 449


🏁 Script executed:

sed -n '325,345p' ./relay/helper/valid_request.go
echo "---"
sed -n '220,240p' ./relay/gemini_handler.go

Repository: QuantumNous/new-api

Length of output: 1119


Use pointer types for optional scalar fields in relay request DTOs with omitempty.

Line 167 (IncludeThoughts bool), line 487 (SampleCount int), and line 510 (OutputDimensionality int) drop explicit false/0 on re-marshal to upstream Gemini API, breaking zero-value preservation for client-supplied values.

🔧 Proposed fix
- IncludeThoughts bool `json:"includeThoughts,omitempty"`
+ IncludeThoughts *bool `json:"includeThoughts,omitempty"`

- SampleCount      int    `json:"sampleCount,omitempty"`
+ SampleCount      *int   `json:"sampleCount,omitempty"`

- OutputDimensionality int               `json:"outputDimensionality,omitempty"`
+ OutputDimensionality *int              `json:"outputDimensionality,omitempty"`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Desktop/电信/测试/newapi/new-api-main/dto/gemini.go` around lines 167 - 170, The
DTO uses non-pointer scalar fields so zero-values (false/0) get omitted on
re-marshal; change IncludeThoughts to *bool, SampleCount and
OutputDimensionality to *int (and any other optional scalar like ThinkingLevel
if it must preserve an explicit empty value—change to *string) in the gemini
DTOs (symbols: IncludeThoughts, ThinkingBudget, ThinkingLevel, SampleCount,
OutputDimensionality), keep the json:"...,omitempty" tags, and update all code
that constructs or reads these fields to handle nil vs non-nil (use nil to mean
unset, dereference safely when present and provide defaults where the upstream
call expects concrete values).

Comment on lines +548 to +550
for _, request := range r.Requests {
meta := request.GetTokenCountMeta()
if meta != nil && meta.CombineText != "" {

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.

⚠️ Potential issue | 🟠 Major

Guard nil elements in Requests to prevent panic.

Line 549 and Line 563 call methods on request items without nil checks. A payload containing null in requests will panic.

🔧 Proposed fix
for _, request := range r.Requests {
+ if request == nil {
+   continue
+ }
  meta := request.GetTokenCountMeta()
  ...
}

for _, req := range r.Requests {
+ if req == nil {
+   continue
+ }
  req.SetModelName(modelName)
}

Also applies to: 562-564

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Desktop/电信/测试/newapi/new-api-main/dto/gemini.go` around lines 548 - 550, The
loop over r.Requests currently assumes each element is non-nil and calls
request.GetTokenCountMeta() (and another method later at the same loop where
request is dereferenced); add a nil guard at the top of the loop (e.g., if
request == nil { continue }) so any null entries in r.Requests are skipped
before calling request.GetTokenCountMeta() and the subsequent request methods
referenced around lines 562–564, preventing panics when payloads include null
requests.

Comment on lines +249 to +252
if googleBody, ok := extraBody["google"].(map[string]interface{}); ok {
if !strings.HasSuffix(info.UpstreamModelName, "-nothinking") {
adaptorWithExtraBody = true
// check error param name like thinkingConfig, should be thinking_config

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.

⚠️ Potential issue | 🟠 Major

extra_body.google currently disables default ThinkingAdaptor even without thinking_config.

Line 251 sets adaptorWithExtraBody = true for any extra_body.google block, so Line 354 gets skipped even when only image_config is provided.

🔧 Proposed fix
- if googleBody, ok := extraBody["google"].(map[string]interface{}); ok {
-   if !strings.HasSuffix(info.UpstreamModelName, "-nothinking") {
-     adaptorWithExtraBody = true
+ if googleBody, ok := extraBody["google"].(map[string]interface{}); ok {
+   if !strings.HasSuffix(info.UpstreamModelName, "-nothinking") {
      ...
      if thinkingConfig, ok := googleBody["thinking_config"].(map[string]interface{}); ok {
+       adaptorWithExtraBody = true
        ...
      }
   }

Also applies to: 353-355

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Desktop/电信/测试/newapi/new-api-main/relay/channel/gemini/relay-gemini.go`
around lines 249 - 252, The current logic sets adaptorWithExtraBody = true
whenever extraBody["google"] exists, which erroneously disables the default
ThinkingAdaptor; change it so adaptorWithExtraBody is only set when the google
map contains a thinking config key (accept both "thinking_config" and
"thinkingConfig" variants), e.g. inspect googleBody for
"thinking_config"/"thinkingConfig" (and normalize snake/camel case) before
setting adaptorWithExtraBody and skipping ThinkingAdaptor; keep the existing
UpstreamModelName "-nothinking" check intact and ensure image_config alone does
not trigger adaptorWithExtraBody.

Comment on lines +468 to +471
if err := json.Unmarshal([]byte(contentStr), &contentMap); err != nil {
// 2. 如果失败,尝试解析为 JSON 数组
var contentSlice []interface{}
if err := json.Unmarshal([]byte(contentStr), &contentSlice); err == nil {

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the relay-gemini.go file
find . -name "relay-gemini.go" -type f

Repository: QuantumNous/new-api

Length of output: 174


🏁 Script executed:

#!/bin/bash
# List files in the common directory to find json.go
find . -path "*/common/*.go" -type f | head -20

Repository: QuantumNous/new-api

Length of output: 509


🏁 Script executed:

#!/bin/bash
# Check the content of common/json.go to verify wrapper functions exist
if [ -f "common/json.go" ]; then
  head -100 common/json.go
fi

Repository: QuantumNous/new-api

Length of output: 817


🏁 Script executed:

#!/bin/bash
# Check the file size and read the imports section
wc -l ./relay/channel/gemini/relay-gemini.go
head -50 ./relay/channel/gemini/relay-gemini.go

Repository: QuantumNous/new-api

Length of output: 1426


🏁 Script executed:

#!/bin/bash
# Check the specific lines mentioned in the review
echo "=== Line 468 context ==="
sed -n '465,475p' ./relay/channel/gemini/relay-gemini.go

echo "=== Line 503 context ==="
sed -n '500,510p' ./relay/channel/gemini/relay-gemini.go

echo "=== Line 1031 context ==="
sed -n '1028,1038p' ./relay/channel/gemini/relay-gemini.go

echo "=== Line 1593 context ==="
sed -n '1590,1600p' ./relay/channel/gemini/relay-gemini.go

Repository: QuantumNous/new-api

Length of output: 1619


Replace direct encoding/json marshal/unmarshal calls with common wrappers.

Lines 468, 471, 503, 1031, and 1593 directly use json.Marshal/json.Unmarshal in business code, violating the repository JSON handling rule. Replace with common.Unmarshal() and common.Marshal() respectively.

🔧 Proposed fix
- if err := json.Unmarshal([]byte(contentStr), &contentMap); err != nil {
+ if err := common.Unmarshal([]byte(contentStr), &contentMap); err != nil {

- if err := json.Unmarshal([]byte(contentStr), &contentSlice); err == nil {
+ if err := common.Unmarshal([]byte(contentStr), &contentSlice); err == nil {

- if json.Unmarshal([]byte(call.Function.Arguments), &args) != nil {
+ if common.Unmarshal([]byte(call.Function.Arguments), &args) != nil {

- argsBytes, err = json.Marshal(item.FunctionCall.Arguments)
+ argsBytes, err = common.Marshal(item.FunctionCall.Arguments)

- jsonResponse, jsonErr := json.Marshal(openAIResponse)
+ jsonResponse, jsonErr := common.Marshal(openAIResponse)

Per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions in common/json.go. Do NOT directly import or call encoding/json in business code."

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

Suggested change
if err := json.Unmarshal([]byte(contentStr), &contentMap); err != nil {
// 2. 如果失败,尝试解析为 JSON 数组
var contentSlice []interface{}
if err := json.Unmarshal([]byte(contentStr), &contentSlice); err == nil {
if err := common.Unmarshal([]byte(contentStr), &contentMap); err != nil {
// 2. 如果失败,尝试解析为 JSON 数组
var contentSlice []interface{}
if err := common.Unmarshal([]byte(contentStr), &contentSlice); err == nil {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Desktop/电信/测试/newapi/new-api-main/relay/channel/gemini/relay-gemini.go`
around lines 468 - 471, Several spots in relay-gemini.go use encoding/json
directly (e.g., json.Unmarshal called on contentStr into contentMap/contentSlice
and other marshal/unmarshal sites); replace those direct calls with the
repository wrappers: use common.Unmarshal([]byte(contentStr), &contentMap) or
common.Unmarshal([]byte(contentStr), &contentSlice) and use
common.Marshal(value) where json.Marshal was used. Locate occurrences by
searching for json.Unmarshal/json.Marshal and the variables contentStr,
contentMap, contentSlice (and the other call sites reported) and swap each call
to the corresponding common.Unmarshal/common.Marshal signature, keeping error
handling unchanged.

Comment on lines +578 to +585
text = text[closeIdx+1:]
}
// 添加剩余文本或原始文本(如果没有找到 markdown 图片)
if !hasMarkdownImage {
parts = append(parts, dto.GeminiPart{
Text: part.Text,
})
}

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.

⚠️ Potential issue | 🟠 Major

Trailing text after markdown data-image is dropped.

After parsing markdown images, text is advanced (Line 578), but trailing remainder is never appended when at least one image was found.

🔧 Proposed fix
- // 添加剩余文本或原始文本(如果没有找到 markdown 图片)
- if !hasMarkdownImage {
+ // 添加剩余文本或原始文本
+ if hasMarkdownImage {
+   if text != "" {
+     parts = append(parts, dto.GeminiPart{Text: text})
+   }
+ } else {
    parts = append(parts, dto.GeminiPart{
      Text: part.Text,
    })
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Desktop/电信/测试/newapi/new-api-main/relay/channel/gemini/relay-gemini.go`
around lines 578 - 585, The parsing loop advances the local variable text (text
= text[closeIdx+1:]) when a markdown image is found but never appends any
trailing remainder when hasMarkdownImage is true, so trailing text is dropped;
update the logic around hasMarkdownImage/parts to, after processing images,
check if text != "" and append a dto.GeminiPart{Text: text} (using the same
dto.GeminiPart shape you use for the no-image branch) so that the leftover
substring after closeIdx is preserved; reference the variables and symbols text,
closeIdx, hasMarkdownImage, parts, dto.GeminiPart and part.Text to locate and
modify the code.

Comment on lines +1037 to +1042
ID: fmt.Sprintf("call_%s", common.GetUUID()),
Type: "function",
Function: dto.FunctionResponse{
Arguments: string(argsBytes),
Name: item.FunctionCall.FunctionName,
},

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.

⚠️ Potential issue | 🟠 Major

Random tool-call IDs break cross-chunk aggregation semantics.

Line 1037 generates a new UUID for every chunk, while Line 1375+ expects stable IDs to map/merge tool calls. The ID map can’t correlate repeated chunks of the same function call.

Also applies to: 1370-1383

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Desktop/电信/测试/newapi/new-api-main/relay/channel/gemini/relay-gemini.go`
around lines 1037 - 1042, The code is generating a fresh UUID for each chunk
(fmt.Sprintf("call_%s", common.GetUUID())) which prevents later
aggregation/merging logic from recognizing the same tool call; change the ID to
be stable per logical function call by using the original call identifier (e.g.,
item.FunctionCall.ID or an existing callID variable passed into this flow) when
constructing the FunctionResponse (ID, Type, Function:
dto.FunctionResponse{...}) so repeated chunks share the same ID; if an original
call ID is not available, derive a deterministic ID (e.g., hash of FunctionName
+ Arguments) instead of calling common.GetUUID() so the ID remains consistent
across chunks and aligns with the code that maps/merges tool calls.

Comment on lines +1242 to +1261
} else if part.Thought {
isThought = true
texts = append(texts, part.Text)
} else {
if part.ExecutableCode != nil {
texts = append(texts, "```"+part.ExecutableCode.Language+"\n"+part.ExecutableCode.Code+"\n```\n")
} else if part.CodeExecutionResult != nil {
texts = append(texts, "```output\n"+part.CodeExecutionResult.Output+"\n```\n")
} else {
if part.Text != "\n" {
texts = append(texts, part.Text)
}
}
}
}
if isThought {
choice.Delta.SetReasoningContent(strings.Join(texts, "\n"))
} else {
choice.Delta.SetContentString(strings.Join(texts, "\n"))
}

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.

⚠️ Potential issue | 🟠 Major

Streaming path conflates normal content with reasoning content.

If any part.Thought exists, Line 1258 writes all collected text to reasoning_content, including non-thought text. This can hide assistant output from content.

🔧 Proposed fix
- var texts []string
- isTools := false
- isThought := false
+ var contentTexts []string
+ var reasoningTexts []string
+ isTools := false

...
- } else if part.Thought {
-   isThought = true
-   texts = append(texts, part.Text)
+ } else if part.Thought {
+   reasoningTexts = append(reasoningTexts, part.Text)
  } else {
    ...
-   texts = append(texts, part.Text)
+   contentTexts = append(contentTexts, part.Text)
  }

- if isThought {
-   choice.Delta.SetReasoningContent(strings.Join(texts, "\n"))
- } else {
-   choice.Delta.SetContentString(strings.Join(texts, "\n"))
- }
+ if len(reasoningTexts) > 0 {
+   choice.Delta.SetReasoningContent(strings.Join(reasoningTexts, "\n"))
+ }
+ if len(contentTexts) > 0 {
+   choice.Delta.SetContentString(strings.Join(contentTexts, "\n"))
+ }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Desktop/电信/测试/newapi/new-api-main/relay/channel/gemini/relay-gemini.go`
around lines 1242 - 1261, The loop currently mixes thought and non-thought text
into one slice and when any part.Thought is true it writes everything to
choice.Delta.SetReasoningContent, hiding normal output; fix by maintaining two
separate slices (e.g., thoughtTexts and normalTexts) and append to thoughtTexts
when part.Thought is true and to normalTexts for normal branches (including
ExecutableCode/CodeExecutionResult/Text); after the loop, always call
choice.Delta.SetContentString(strings.Join(normalTexts, "\n")) and if there were
any thoughts call choice.Delta.SetReasoningContent(strings.Join(thoughtTexts,
"\n")), ensuring SetReasoningContent only contains true thought content and
SetContentString always contains non-thought content (use the same symbols
part.Thought, part.ExecutableCode, part.CodeExecutionResult,
choice.Delta.SetReasoningContent, choice.Delta.SetContentString).

Comment on lines +1633 to +1635
if nextPageToken != "" {
url = fmt.Sprintf("%s?pageToken=%s", url, nextPageToken)
}

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.

⚠️ Potential issue | 🟡 Minor

pageToken should be query-escaped before URL construction.

Line 1634 concatenates pageToken directly into the URL. Reserved characters in the token can break request parsing.

🔧 Proposed fix
- if nextPageToken != "" {
-   url = fmt.Sprintf("%s?pageToken=%s", url, nextPageToken)
- }
+ if nextPageToken != "" {
+   url = fmt.Sprintf("%s?pageToken=%s", url, urlpkg.QueryEscape(nextPageToken))
+ }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Desktop/电信/测试/newapi/new-api-main/relay/channel/gemini/relay-gemini.go`
around lines 1633 - 1635, The code appends nextPageToken directly into the url
string (see variable url and nextPageToken), which can break if the token
contains reserved characters; update the code to percent-escape the token or,
better, build the URL using net/url: parse the base URL, call
urlObj.Query().Set("pageToken", nextPageToken) (or use
url.QueryEscape(nextPageToken)), then reassign url = urlObj.String(); add the
necessary import for net/url and ensure the same approach is used wherever
pageToken is appended.

@YanXi9999 YanXi9999 closed this Mar 3, 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