Skip to content

fix: preserve explicit zero values in native relay requests - #3069

Merged
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
seefs001:fix/gemini-field-ignore
Mar 1, 2026
Merged

fix: preserve explicit zero values in native relay requests#3069
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
seefs001:fix/gemini-field-ignore

Conversation

@seefs001

@seefs001 seefs001 commented Mar 1, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

Release Notes

  • Documentation

    • Added guidance for handling optional request parameters to ensure explicit zero values are preserved when sent upstream while omitting unspecified fields.
  • Tests

    • Added tests to verify that explicit zero values are correctly preserved during request serialization and deserialization.
  • Bug Fixes

    • Improved handling of optional numeric and boolean fields across API requests to properly distinguish between absent and explicitly zero-valued parameters, enhancing compatibility with upstream services.

@coderabbitai

coderabbitai Bot commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

PR adds documentation Rule 6 on preserving explicit zero values in request DTOs using pointers with omitempty, then systematically converts scalar fields in request DTO structs from value types to pointer types (Stream, MaxTokens, TopP, etc.), updates relay handlers and channel adaptors to safely access these pointers via the lo utility library with appropriate defaults, and adds zero-value preservation tests.

Changes

Cohort / File(s) Summary
Documentation
.cursor/rules/project.mdc, AGENTS.md, CLAUDE.md
New Rule 6 guidance on handling optional scalar fields in request DTOs using pointers with omitempty, detailing nil vs explicit zero-valued field semantics.
Core DTO Struct Conversions
dto/openai_request.go, dto/claude.go, dto/gemini.go, dto/embedding.go, dto/openai_image.go, dto/audio.go, dto/rerank.go
Multiple scalar fields (Stream, MaxTokens, TopP, TopK, N, etc.) converted from value types to pointer types; updated related methods (IsStream, GetTokenCountMeta, GetMaxTokens) to handle nil pointers safely.
Gemini Config DTO
dto/gemini.go
GeminiChatGenerationConfig fields (TopP, TopK, MaxOutputTokens, CandidateCount, ResponseLogprobs, Seed) converted to pointers; UnmarshalJSON refactored to handle both snake_case and camelCase variants with nil-aware logic; added ImageConfig field.
Zero-Value Preservation Tests
dto/gemini_generation_config_test.go, dto/openai_request_zero_value_test.go
New tests verifying explicit zero values are preserved through unmarshal/marshal cycles for GeneralOpenAIRequest, OpenAIResponsesRequest, and GeminiChatRequest fields.
Relay Test Updates
controller/channel-test.go
Updated test request construction to wrap pointer fields with lo.ToPtr (N, TopN, Stream, MaxTokens, MaxCompletionTokens).
Relay Token-Count Logic
controller/relay.go, relay/claude_handler.go, service/convert.go
Updated pricing and token-count calculations to use pointer-safe access via lo.FromPtrOr and lo.FromPtr for nil-aware dereferencing.
Ali Channel Adaptors
relay/channel/ali/image.go, relay/channel/ali/image_wan.go, relay/channel/ali/rerank.go, relay/channel/ali/text.go
Pointer-safe N and TopP handling using lo.FromPtrOr with defaults; TopN field signature changed in AliRerankParameters from pointer to value.
AWS, Baidu, Cloudflare, Cohere, Coze, Dify Channel Adaptors
relay/channel/aws/dto.go, relay/channel/baidu/relay-baidu.go, relay/channel/cloudflare/relay_cloudflare.go, relay/channel/cohere/relay-cohere.go, relay/channel/coze/relay-coze.go, relay/channel/dify/relay-dify.go
Replaced value checks with nil-pointer checks; updated field assignments to use lo.FromPtrOr for safe defaults (TopP, TopK, Stream, TopN).
Claude Relay Handler
relay/channel/claude/relay-claude.go
Refactored MaxTokens, TopP, TopK, Stream assignments to use pointer-based fields with nil-aware defaults; updated thinking/adaptive branches to dereference pointers; added nil safety for budget calculations.
Codex, Gemini, Mistral, Ollama, OpenAI, Perplexity Channel Adaptors
relay/channel/codex/adaptor.go, relay/channel/gemini/adaptor.go, relay/channel/gemini/relay-gemini.go, relay/channel/mistral/text.go, relay/channel/ollama/relay-ollama.go, relay/channel/openai/adaptor.go, relay/channel/perplexity/adaptor.go, relay/channel/perplexity/relay-perplexity.go
Pointer-aware N/SampleCount/Dimensions/MaxTokens/MaxOutputTokens handling using lo.FromPtrOr; conditional field assignments for nil/zero values; MaxOutputTokens set to nil instead of 0 in codex.
Replicate, SiliconFlow, Tencent, Vertex Channel Adaptors
relay/channel/replicate/adaptor.go, relay/channel/siliconflow/adaptor.go, relay/channel/tencent/relay-tencent.go, relay/channel/vertex/adaptor.go, relay/channel/vertex/dto.go
Safe pointer dereferencing for image N; VertexAIClaudeRequest fields (MaxTokens, Stream, TopP, TopK) converted to pointers; Stream/TopP handling updated to conditional nil checks.
Volcengine, X.AI, Xunfei, Zhipu Channel Adaptors
relay/channel/volcengine/adaptor.go, relay/channel/xai/adaptor.go, relay/channel/xunfei/relay-xunfei.go, relay/channel/zhipu/adaptor.go, relay/channel/zhipu/relay-zhipu.go, relay/channel/zhipu_4v/adaptor.go, relay/channel/zhipu_4v/relay-zhipu_v4.go
Speed/TopP/TopK/MaxTokens/N handled via lo.FromPtrOr with defaults; conditional MaxTokens assignments; pointer-based TopP coercion (e.g., >= 1 → 0.99).
Relay Helper & Handler Logic
relay/helper/valid_request.go, relay/image_handler.go, relay/compatible_handler.go, service/openaicompat/chat_to_responses.go
Image N field handled as pointer with nil-aware defaults; Stream validation using lo.FromPtrOr; MaxOutputTokens conditionally set when input tokens provided; TopP and max-tokens validation using pointer-safe access.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~35 minutes

Possibly related PRs

Suggested reviewers

  • Calcium-Ion

Poem

🐰 Pointers bloom where zeros roamed,
Each scalar now has nil a home,
With lo.ToPtr and defaults bound,
Zero values safe and sound,
The relay hops with omitempty grace! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.00% 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 objective of the changeset: converting optional scalar fields in request DTOs to use pointer types to preserve explicit zero values in JSON serialization.

✏️ 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: 6

Caution

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

⚠️ Outside diff range comments (11)
relay/channel/claude/relay-claude.go (4)

86-88: ⚠️ Potential issue | 🟡 Minor

Use common.Unmarshal instead of json.Unmarshal.

As per coding guidelines, all JSON unmarshal operations must use wrapper functions from common/json.go.

🔧 Suggested fix
 			// 解析 UserLocation JSON
 			var userLocationMap map[string]interface{}
-			if err := json.Unmarshal(textRequest.WebSearchOptions.UserLocation, &userLocationMap); err == nil {
+			if err := common.Unmarshal(textRequest.WebSearchOptions.UserLocation, &userLocationMap); err == nil {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/claude/relay-claude.go` around lines 86 - 88, Replace the
direct call to json.Unmarshal when decoding
textRequest.WebSearchOptions.UserLocation into userLocationMap with the
project's wrapper common.Unmarshal; specifically, locate the block that declares
var userLocationMap map[string]interface{} and currently calls
json.Unmarshal(textRequest.WebSearchOptions.UserLocation, &userLocationMap) and
change it to use common.Unmarshal(...) handling the returned error the same way
(check error != nil). Ensure imports are updated to remove "encoding/json" if no
longer used and to reference the common package used for JSON helpers.

821-825: ⚠️ Potential issue | 🟡 Minor

Use common.Marshal instead of json.Marshal.

As per coding guidelines, all JSON marshal operations must use wrapper functions from common/json.go.

🔧 Suggested fix
 	case types.RelayFormatOpenAI:
 		openaiResponse := ResponseClaude2OpenAI(&claudeResponse)
 		openaiResponse.Usage = *claudeInfo.Usage
-		responseData, err = json.Marshal(openaiResponse)
+		responseData, err = common.Marshal(openaiResponse)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/claude/relay-claude.go` around lines 821 - 825, Replace the
direct json.Marshal call with the project's wrapper by calling common.Marshal on
the OpenAI response: in the block that builds openaiResponse (using
ResponseClaude2OpenAI, claudeResponse, claudeInfo, openaiResponse) swap
json.Marshal(openaiResponse) for common.Marshal(openaiResponse) and keep the
existing error handling (return types.NewError(err,
types.ErrorCodeBadResponseBody)) so behavior remains identical while adhering to
the common/json.go wrapper.

374-377: ⚠️ Potential issue | 🟡 Minor

Use common.Unmarshal instead of json.Unmarshal.

As per coding guidelines, all JSON unmarshal operations must use wrapper functions from common/json.go.

🔧 Suggested fix
 				if message.ToolCalls != nil {
 					for _, toolCall := range message.ParseToolCalls() {
 						inputObj := make(map[string]any)
-						if err := json.Unmarshal([]byte(toolCall.Function.Arguments), &inputObj); err != nil {
+						if err := common.Unmarshal([]byte(toolCall.Function.Arguments), &inputObj); err != nil {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/claude/relay-claude.go` around lines 374 - 377, Replace the
direct call to json.Unmarshal when parsing toolCall.Function.Arguments with the
project wrapper common.Unmarshal: call
common.Unmarshal([]byte(toolCall.Function.Arguments), &inputObj) and handle the
returned error the same way (using the existing err variable and logging via
common.SysLog). Ensure the common package is imported/available in
relay-claude.go and reference the existing symbols ParseToolCalls,
toolCall.Function.Arguments, and inputObj when making the change.

510-510: ⚠️ Potential issue | 🟡 Minor

Use common.Marshal instead of json.Marshal.

As per coding guidelines, all JSON marshal operations must use wrapper functions from common/json.go.

🔧 Suggested fix
 		case "tool_use":
-			args, _ := json.Marshal(message.Input)
+			args, _ := common.Marshal(message.Input)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/claude/relay-claude.go` at line 510, Replace the direct call to
json.Marshal for serializing message.Input with the project's wrapper
common.Marshal: change the call in the code that assigns to args (currently
args, _ := json.Marshal(message.Input)) to use common.Marshal(message.Input),
capture and handle the returned error instead of discarding it, and propagate or
log the error appropriately where args is used (referencing the args variable
and message.Input and using common.Marshal in place of json.Marshal).
relay/channel/minimax/adaptor.go (1)

58-67: ⚠️ Potential issue | 🟡 Minor

Use common.Unmarshal and common.Marshal instead of json package.

As per coding guidelines, all JSON marshal/unmarshal operations must use wrapper functions from common/json.go.

🔧 Suggested fix
+	"github.com/QuantumNous/new-api/common"
 	"github.com/QuantumNous/new-api/dto"
 	// 同步扩展字段的厂商自定义metadata
 	if len(request.Metadata) > 0 {
-		if err := json.Unmarshal(request.Metadata, &minimaxRequest); err != nil {
+		if err := common.Unmarshal(request.Metadata, &minimaxRequest); err != nil {
 			return nil, fmt.Errorf("error unmarshalling metadata to minimax request: %w", err)
 		}
 	}

-	jsonData, err := json.Marshal(minimaxRequest)
+	jsonData, err := common.Marshal(minimaxRequest)
 	if err != nil {
 		return nil, fmt.Errorf("error marshalling minimax request: %w", err)
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/minimax/adaptor.go` around lines 58 - 67, Replace direct json
package calls with the project's JSON wrappers: use common.Unmarshal to decode
request.Metadata into minimaxRequest (keeping the same error wrapping message)
and use common.Marshal to encode minimaxRequest into jsonData (again preserving
the error wrap). Update the two sites where json.Unmarshal and json.Marshal are
used—refer to minimaxRequest and request.Metadata—to call
common.Unmarshal(request.Metadata, &minimaxRequest) and
common.Marshal(minimaxRequest) and propagate errors with the existing fmt.Errorf
messages.
relay/channel/tencent/relay-tencent.go (2)

196-196: ⚠️ Potential issue | 🟡 Minor

Use common.Marshal() instead of json.Marshal.

Direct usage of json.Marshal in getTencentSign violates coding guidelines.

Proposed fix
-	payload, _ := json.Marshal(req)
+	payload, _ := common.Marshal(req)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/tencent/relay-tencent.go` at line 196, Replace the direct call
to json.Marshal in getTencentSign with the project's common.Marshal helper: use
common.Marshal(req) to produce payload (error-handling as appropriate) instead
of json.Marshal(req); update the payload variable assignment and handle the
returned error from common.Marshal consistently with surrounding error handling
in getTencentSign.

143-143: ⚠️ Potential issue | 🟡 Minor

Use common.Unmarshal() instead of json.Unmarshal.

Direct usage of json.Unmarshal violates coding guidelines.

Proposed fix
-	err = json.Unmarshal(responseBody, &tencentSb)
+	err = common.Unmarshal(responseBody, &tencentSb)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/tencent/relay-tencent.go` at line 143, Replace the direct call
to json.Unmarshal(responseBody, &tencentSb) with the project-standard
common.Unmarshal helper: call common.Unmarshal(responseBody, &tencentSb) (or the
equivalent common.UnmarshalBytes) to parse responseBody into tencentSb so the
code follows the coding guidelines and centralized error handling; update
imports if necessary to remove "encoding/json" and ensure common is imported.
relay/channel/ali/rerank.go (2)

67-67: ⚠️ Potential issue | 🟡 Minor

Use common.Marshal() instead of json.Marshal.

Direct usage of json.Marshal violates coding guidelines.

Proposed fix
-	jsonResponse, err := json.Marshal(rerankResponse)
+	jsonResponse, err := common.Marshal(rerankResponse)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/ali/rerank.go` at line 67, Replace the direct call to
json.Marshal with the project's wrapper common.Marshal for serializing
rerankResponse: change json.Marshal(rerankResponse) to
common.Marshal(rerankResponse), update imports to remove encoding/json if no
longer used and add the common package import, and keep the existing error
handling around the marshal call intact (i.e., still check the returned error
and handle it as before). Ensure the variable names (jsonResponse,
rerankResponse) and surrounding logic in rerank.go remain unchanged except for
swapping the marshal function.

43-43: ⚠️ Potential issue | 🟡 Minor

Use common.Unmarshal() instead of json.Unmarshal.

Direct usage of json.Unmarshal violates coding guidelines. As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions from common/json.go".

Proposed fix
-	err = json.Unmarshal(responseBody, &aliResponse)
+	err = common.Unmarshal(responseBody, &aliResponse)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/ali/rerank.go` at line 43, Replace the direct call to
json.Unmarshal(responseBody, &aliResponse) with the project's JSON wrapper: call
common.Unmarshal(responseBody, &aliResponse) (and ensure the common package is
imported), handling and returning the same error value as before; update any
error checks around aliResponse unmarshal to use the error returned by
common.Unmarshal in rerank.go so behavior remains identical.
relay/helper/valid_request.go (1)

155-169: ⚠️ Potential issue | 🟠 Major

Negative multipart n can overflow into a huge unsigned value.

String2Int("-1") becomes -1, and casting to uint yields a very large number, bypassing the defaulting logic.

🔧 Suggested fix
-imageRequest.N = common.GetPointer(uint(common.String2Int(formData.Get("n"))))
+nVal := common.String2Int(formData.Get("n"))
+if nVal <= 0 {
+	imageRequest.N = common.GetPointer(uint(1))
+} else {
+	imageRequest.N = common.GetPointer(uint(nVal))
+}
 ...
-if imageRequest.N == nil || *imageRequest.N == 0 {
-	imageRequest.N = common.GetPointer(uint(1))
-}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/helper/valid_request.go` around lines 155 - 169, The code casts the
parsed signed int from common.String2Int(formData.Get("n")) directly to uint
causing negative values (e.g., "-1") to become huge unsigned numbers and bypass
the default; fix by first storing the parsed value in a signed int (e.g.,
parsedN := common.String2Int(...)), then check if parsedN <= 0 and set
imageRequest.N = common.GetPointer(uint(1)) in that case, otherwise set
imageRequest.N = common.GetPointer(uint(parsedN)); update the logic around
imageRequest.N assignment in the function handling form parsing (references:
imageRequest.N, common.String2Int, common.GetPointer) so negatives and zero are
guarded against before any uint cast.
relay/channel/ollama/relay-ollama.go (1)

520-522: ⚠️ Potential issue | 🟡 Minor

Use common.Unmarshal instead of json.Unmarshal.

This violates the coding guideline requiring all JSON unmarshal operations to use wrapper functions from common/json.go.

Proposed fix
-	if err := json.Unmarshal(body, &versionResp); err != nil {
+	if err := common.Unmarshal(body, &versionResp); err != nil {

As per coding guidelines: "Use common.Unmarshal() from common/json.go for all JSON marshal/unmarshal operations in business code. Do NOT directly import or call encoding/json for marshal/unmarshal in business code."

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

In `@relay/channel/ollama/relay-ollama.go` around lines 520 - 522, Replace the
direct call to json.Unmarshal with the project's wrapper common.Unmarshal: call
common.Unmarshal(body, &versionResp) and propagate the error similarly (e.g.,
return "", fmt.Errorf("解析响应失败: %v", err)) so the behavior/format stays the same;
update imports to remove direct usage of encoding/json if no longer needed and
ensure the file imports the package that exposes Unmarshal (common) so the
change affects the json unmarshal in the function that handles the version
response (where versionResp and body are used).
🧹 Nitpick comments (6)
relay/channel/replicate/adaptor.go (1)

5-5: Direct encoding/json usage violates coding guidelines.

Line 5 imports encoding/json and line 114 calls json.Unmarshal directly. Per coding guidelines, all JSON operations must use wrapper functions from common/json.go.

Proposed fix

Remove the direct encoding/json import and replace json.Unmarshal with common.Unmarshal:

 import (
 	"bytes"
-	"encoding/json"
 	"errors"
 	"fmt"
 	if len(request.OutputFormat) > 0 {
 		var outputFormat string
-		if err := json.Unmarshal(request.OutputFormat, &outputFormat); err == nil && strings.TrimSpace(outputFormat) != "" {
+		if err := common.Unmarshal(request.OutputFormat, &outputFormat); err == nil && strings.TrimSpace(outputFormat) != "" {
 			inputPayload["output_format"] = outputFormat
 		}
 	}

As per coding guidelines: "Do NOT directly import or call encoding/json for marshal/unmarshal in business code."

Also applies to: 114-117

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

In `@relay/channel/replicate/adaptor.go` at line 5, Remove the direct import of
"encoding/json" and replace the direct call to json.Unmarshal (used around the
block that includes function/method invoking json.Unmarshal) with the wrapper
common.Unmarshal from common/json.go; update the import list to include the
common package instead of encoding/json and ensure any error handling around the
Unmarshal call remains the same while calling common.Unmarshal(...) where
json.Unmarshal(...) was used.
controller/relay.go (1)

266-276: Logic is correct; consider consistent helper usage for readability.

The pointer-safe access via lo.FromPtrOr correctly handles the DTO fields that are now pointers. However, line 276 uses lo.FromPtr while other cases use lo.FromPtrOr(..., uint(0)). Both are functionally equivalent for *uint (returning 0 when nil), but using a consistent pattern improves readability.

♻️ Optional: use consistent helper for ClaudeRequest case
 case *dto.ClaudeRequest:
-	meta.MaxTokens = int(lo.FromPtr(r.MaxTokens))
+	meta.MaxTokens = int(lo.FromPtrOr(r.MaxTokens, uint(0)))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@controller/relay.go` around lines 266 - 276, The ClaudeRequest branch
currently uses lo.FromPtr(r.MaxTokens) while other branches use
lo.FromPtrOr(..., uint(0)); update the dto.ClaudeRequest handling to use
lo.FromPtrOr(r.MaxTokens, uint(0)) when setting meta.MaxTokens so the
pointer-safe helper usage is consistent with the other cases (leave the
assignment to meta.MaxTokens and the int cast as-is).
service/openaicompat/chat_to_responses.go (1)

370-373: Redundant pointer conversion for TopP.

The current code dereferences the pointer with lo.FromPtr, then immediately wraps it back in a pointer with common.GetPointer. This is equivalent to just copying the pointer value.

♻️ Simplified TopP handling
 	var topP *float64
 	if req.TopP != nil {
-		topP = common.GetPointer(lo.FromPtr(req.TopP))
+		v := *req.TopP
+		topP = &v
 	}

Or if you want to keep using helpers:

 	var topP *float64
 	if req.TopP != nil {
-		topP = common.GetPointer(lo.FromPtr(req.TopP))
+		topP = lo.ToPtr(*req.TopP)
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@service/openaicompat/chat_to_responses.go` around lines 370 - 373, The TopP
handling is performing a redundant dereference and rewrap: in the block that
defines topP, remove the lo.FromPtr + common.GetPointer dance and simply copy
the pointer (assign topP = req.TopP) so you preserve the original *float64;
update the code around the topP variable initialization (referencing topP and
req.TopP, and remove lo.FromPtr and common.GetPointer usage) to eliminate the
unnecessary conversion.
relay/image_handler.go (1)

140-142: Redundant condition check.

Since imageN is a uint that defaults to 1 and can only be overwritten by dereferencing a non-nil pointer, the condition imageN > 0 will always be true. Consider removing the conditional check.

♻️ Suggested simplification
-	if imageN > 0 {
-		logContent = append(logContent, fmt.Sprintf("生成数量 %d", imageN))
-	}
+	logContent = append(logContent, fmt.Sprintf("生成数量 %d", imageN))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/image_handler.go` around lines 140 - 142, The conditional check `if
imageN > 0` is redundant; remove the `if` and always append the generated-count
entry to `logContent` by calling `logContent = append(logContent,
fmt.Sprintf("生成数量 %d", imageN))` unconditionally (update the code around
`imageN`/`logContent` in `image_handler.go` accordingly).
dto/embedding.go (1)

11-20: Consider aligning EmbeddingOptions with Rule 6 pattern.

EmbeddingOptions has Seed and TopK as non-pointer int with omitempty, which means explicit zero values would be silently dropped during marshal. If this struct is used in upstream relay contexts, consider converting these to *int for consistency with Rule 6.

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

In `@dto/embedding.go` around lines 11 - 20, EmbeddingOptions currently declares
Seed and TopK as non-pointer ints so zero values are omitted by json.Marshal due
to `omitempty`; change Seed and TopK to pointer types (`*int`) in the
EmbeddingOptions struct so explicit zero values are preserved (consistent with
Rule 6), update any constructors/usage sites that set or read Seed and TopK to
handle nil vs non-nil correctly, and add nil-checks or dereferencing where these
fields are consumed (e.g., places referencing EmbeddingOptions.Seed or .TopK).
dto/openai_request_zero_value_test.go (1)

37-50: Assert exact zero/false values, not only key presence.

These checks currently prove that keys survive marshaling, but not that values remain 0/false. Add value assertions to fully lock the behavior.

🧪 Suggested test hardening
 require.True(t, gjson.GetBytes(encoded, "stream").Exists())
+require.Equal(t, false, gjson.GetBytes(encoded, "stream").Bool())

 require.True(t, gjson.GetBytes(encoded, "max_tokens").Exists())
+require.Equal(t, float64(0), gjson.GetBytes(encoded, "max_tokens").Num())

 require.True(t, gjson.GetBytes(encoded, "top_p").Exists())
+require.Equal(t, float64(0), gjson.GetBytes(encoded, "top_p").Num())

Also applies to: 69-72

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

In `@dto/openai_request_zero_value_test.go` around lines 37 - 50, Replace
presence-only assertions in dto/openai_request_zero_value_test.go with
exact-value assertions using gjson.GetBytes(encoded, "<key>").Int() or .Bool()
as appropriate: assert numeric keys (max_tokens, max_completion_tokens, top_p,
top_k, n, frequency_penalty, presence_penalty, seed, logprobs, top_logprobs,
dimensions) equal 0 and boolean keys (stream, return_images,
return_related_questions) equal false using
require.Equal/require.False/require.Zero; update the similar checks at the later
block mentioned (lines 69-72) the same way so the test verifies zero/false
values rather than mere key existence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@relay/channel/aws/dto.go`:
- Around line 97-109: The mapping currently treats explicit zero values as
absent because each gate checks "*req.<Field> != 0" before copying; change the
logic in the block that sets novaReq.InferenceConfig so each field is included
when the request pointer is non-nil (remove the "*... != 0" checks and only test
for nil), and update NovaInferenceConfig's scalar fields to be pointer types (or
remove omitempty) so zeros are preserved when marshaling; ensure
novaReq.InferenceConfig = &NovaInferenceConfig{} is allocated once and then
assign pointer values (e.g., set MaxTokens to a pointer to int when
req.MaxTokens != nil) so explicit 0 values are propagated.

In `@relay/channel/gemini/relay-gemini.go`:
- Around line 216-219: The current check `if textRequest.Seed != nil &&
*textRequest.Seed != 0` drops an explicitly provided zero seed; change the guard
to only test for nil (i.e., `if textRequest.Seed != nil`) and then convert and
assign the seed as you do now (`geminiSeed :=
int64(lo.FromPtr(textRequest.Seed))` and `geminiRequest.GenerationConfig.Seed =
common.GetPointer(geminiSeed)`), so an explicit 0 value is preserved when
converting from textRequest to geminiRequest.GenerationConfig.Seed.

In `@relay/channel/openai/adaptor.go`:
- Around line 318-321: The code treats an explicit max_completion_tokens: 0 as
missing because it compares dereferenced-or-default values; change the condition
to check pointer nils instead so a user-specified zero is preserved. Replace the
current if that uses lo.FromPtrOr on request.MaxCompletionTokens with a
nil-check: only copy request.MaxTokens into request.MaxCompletionTokens when
request.MaxCompletionTokens == nil and request.MaxTokens != nil, then set
request.MaxTokens = nil (refer to request.MaxCompletionTokens and
request.MaxTokens in adaptor.go).

In `@relay/channel/siliconflow/adaptor.go`:
- Around line 56-59: The code treats sfRequest.BatchSize==0 as "unset" and
therefore overwrites an explicit zero with request.N; change the absence check
to use a sentinel (e.g. initialize BatchSize to -1 where sfRequest is created)
and only copy from request.N when sfRequest.BatchSize == -1, or alternatively
add a boolean flag like BatchSizeSet and check that instead; update the
conditional around sfRequest.BatchSize, request.N and lo.FromPtr(request.N) in
the adaptor.go block so explicit zero values are preserved.

In `@relay/channel/vertex/adaptor.go`:
- Line 307: Replace the direct call to json.Unmarshal(request.ExtraBody, &extra)
with the project's wrapper common.Unmarshal(request.ExtraBody, &extra),
preserving the existing error-checking logic; update the import usage (remove
direct json usage if unused) and ensure the error return from common.Unmarshal
is handled exactly as before where it currently checks err == nil around
request.ExtraBody and the variable extra.

In `@service/convert.go`:
- Around line 711-719: The conversion currently ignores explicit zero values
because it checks `> 0`; remove the `> 0` tests and only check for non-nil
before assigning so that zero is preserved: for
`geminiRequest.GenerationConfig.TopP` and `TopK` and `MaxOutputTokens` check `!=
nil` and then set `openaiRequest.TopP =
lo.ToPtr(*geminiRequest.GenerationConfig.TopP)`, `openaiRequest.TopK =
lo.ToPtr(int(*geminiRequest.GenerationConfig.TopK))`, and
`openaiRequest.MaxTokens =
lo.ToPtr(*geminiRequest.GenerationConfig.MaxOutputTokens)` respectively (apply
same change to the other similar block referenced).

---

Outside diff comments:
In `@relay/channel/ali/rerank.go`:
- Line 67: Replace the direct call to json.Marshal with the project's wrapper
common.Marshal for serializing rerankResponse: change
json.Marshal(rerankResponse) to common.Marshal(rerankResponse), update imports
to remove encoding/json if no longer used and add the common package import, and
keep the existing error handling around the marshal call intact (i.e., still
check the returned error and handle it as before). Ensure the variable names
(jsonResponse, rerankResponse) and surrounding logic in rerank.go remain
unchanged except for swapping the marshal function.
- Line 43: Replace the direct call to json.Unmarshal(responseBody, &aliResponse)
with the project's JSON wrapper: call common.Unmarshal(responseBody,
&aliResponse) (and ensure the common package is imported), handling and
returning the same error value as before; update any error checks around
aliResponse unmarshal to use the error returned by common.Unmarshal in rerank.go
so behavior remains identical.

In `@relay/channel/claude/relay-claude.go`:
- Around line 86-88: Replace the direct call to json.Unmarshal when decoding
textRequest.WebSearchOptions.UserLocation into userLocationMap with the
project's wrapper common.Unmarshal; specifically, locate the block that declares
var userLocationMap map[string]interface{} and currently calls
json.Unmarshal(textRequest.WebSearchOptions.UserLocation, &userLocationMap) and
change it to use common.Unmarshal(...) handling the returned error the same way
(check error != nil). Ensure imports are updated to remove "encoding/json" if no
longer used and to reference the common package used for JSON helpers.
- Around line 821-825: Replace the direct json.Marshal call with the project's
wrapper by calling common.Marshal on the OpenAI response: in the block that
builds openaiResponse (using ResponseClaude2OpenAI, claudeResponse, claudeInfo,
openaiResponse) swap json.Marshal(openaiResponse) for
common.Marshal(openaiResponse) and keep the existing error handling (return
types.NewError(err, types.ErrorCodeBadResponseBody)) so behavior remains
identical while adhering to the common/json.go wrapper.
- Around line 374-377: Replace the direct call to json.Unmarshal when parsing
toolCall.Function.Arguments with the project wrapper common.Unmarshal: call
common.Unmarshal([]byte(toolCall.Function.Arguments), &inputObj) and handle the
returned error the same way (using the existing err variable and logging via
common.SysLog). Ensure the common package is imported/available in
relay-claude.go and reference the existing symbols ParseToolCalls,
toolCall.Function.Arguments, and inputObj when making the change.
- Line 510: Replace the direct call to json.Marshal for serializing
message.Input with the project's wrapper common.Marshal: change the call in the
code that assigns to args (currently args, _ := json.Marshal(message.Input)) to
use common.Marshal(message.Input), capture and handle the returned error instead
of discarding it, and propagate or log the error appropriately where args is
used (referencing the args variable and message.Input and using common.Marshal
in place of json.Marshal).

In `@relay/channel/minimax/adaptor.go`:
- Around line 58-67: Replace direct json package calls with the project's JSON
wrappers: use common.Unmarshal to decode request.Metadata into minimaxRequest
(keeping the same error wrapping message) and use common.Marshal to encode
minimaxRequest into jsonData (again preserving the error wrap). Update the two
sites where json.Unmarshal and json.Marshal are used—refer to minimaxRequest and
request.Metadata—to call common.Unmarshal(request.Metadata, &minimaxRequest) and
common.Marshal(minimaxRequest) and propagate errors with the existing fmt.Errorf
messages.

In `@relay/channel/ollama/relay-ollama.go`:
- Around line 520-522: Replace the direct call to json.Unmarshal with the
project's wrapper common.Unmarshal: call common.Unmarshal(body, &versionResp)
and propagate the error similarly (e.g., return "", fmt.Errorf("解析响应失败: %v",
err)) so the behavior/format stays the same; update imports to remove direct
usage of encoding/json if no longer needed and ensure the file imports the
package that exposes Unmarshal (common) so the change affects the json unmarshal
in the function that handles the version response (where versionResp and body
are used).

In `@relay/channel/tencent/relay-tencent.go`:
- Line 196: Replace the direct call to json.Marshal in getTencentSign with the
project's common.Marshal helper: use common.Marshal(req) to produce payload
(error-handling as appropriate) instead of json.Marshal(req); update the payload
variable assignment and handle the returned error from common.Marshal
consistently with surrounding error handling in getTencentSign.
- Line 143: Replace the direct call to json.Unmarshal(responseBody, &tencentSb)
with the project-standard common.Unmarshal helper: call
common.Unmarshal(responseBody, &tencentSb) (or the equivalent
common.UnmarshalBytes) to parse responseBody into tencentSb so the code follows
the coding guidelines and centralized error handling; update imports if
necessary to remove "encoding/json" and ensure common is imported.

In `@relay/helper/valid_request.go`:
- Around line 155-169: The code casts the parsed signed int from
common.String2Int(formData.Get("n")) directly to uint causing negative values
(e.g., "-1") to become huge unsigned numbers and bypass the default; fix by
first storing the parsed value in a signed int (e.g., parsedN :=
common.String2Int(...)), then check if parsedN <= 0 and set imageRequest.N =
common.GetPointer(uint(1)) in that case, otherwise set imageRequest.N =
common.GetPointer(uint(parsedN)); update the logic around imageRequest.N
assignment in the function handling form parsing (references: imageRequest.N,
common.String2Int, common.GetPointer) so negatives and zero are guarded against
before any uint cast.

---

Nitpick comments:
In `@controller/relay.go`:
- Around line 266-276: The ClaudeRequest branch currently uses
lo.FromPtr(r.MaxTokens) while other branches use lo.FromPtrOr(..., uint(0));
update the dto.ClaudeRequest handling to use lo.FromPtrOr(r.MaxTokens, uint(0))
when setting meta.MaxTokens so the pointer-safe helper usage is consistent with
the other cases (leave the assignment to meta.MaxTokens and the int cast as-is).

In `@dto/embedding.go`:
- Around line 11-20: EmbeddingOptions currently declares Seed and TopK as
non-pointer ints so zero values are omitted by json.Marshal due to `omitempty`;
change Seed and TopK to pointer types (`*int`) in the EmbeddingOptions struct so
explicit zero values are preserved (consistent with Rule 6), update any
constructors/usage sites that set or read Seed and TopK to handle nil vs non-nil
correctly, and add nil-checks or dereferencing where these fields are consumed
(e.g., places referencing EmbeddingOptions.Seed or .TopK).

In `@dto/openai_request_zero_value_test.go`:
- Around line 37-50: Replace presence-only assertions in
dto/openai_request_zero_value_test.go with exact-value assertions using
gjson.GetBytes(encoded, "<key>").Int() or .Bool() as appropriate: assert numeric
keys (max_tokens, max_completion_tokens, top_p, top_k, n, frequency_penalty,
presence_penalty, seed, logprobs, top_logprobs, dimensions) equal 0 and boolean
keys (stream, return_images, return_related_questions) equal false using
require.Equal/require.False/require.Zero; update the similar checks at the later
block mentioned (lines 69-72) the same way so the test verifies zero/false
values rather than mere key existence.

In `@relay/channel/replicate/adaptor.go`:
- Line 5: Remove the direct import of "encoding/json" and replace the direct
call to json.Unmarshal (used around the block that includes function/method
invoking json.Unmarshal) with the wrapper common.Unmarshal from common/json.go;
update the import list to include the common package instead of encoding/json
and ensure any error handling around the Unmarshal call remains the same while
calling common.Unmarshal(...) where json.Unmarshal(...) was used.

In `@relay/image_handler.go`:
- Around line 140-142: The conditional check `if imageN > 0` is redundant;
remove the `if` and always append the generated-count entry to `logContent` by
calling `logContent = append(logContent, fmt.Sprintf("生成数量 %d", imageN))`
unconditionally (update the code around `imageN`/`logContent` in
`image_handler.go` accordingly).

In `@service/openaicompat/chat_to_responses.go`:
- Around line 370-373: The TopP handling is performing a redundant dereference
and rewrap: in the block that defines topP, remove the lo.FromPtr +
common.GetPointer dance and simply copy the pointer (assign topP = req.TopP) so
you preserve the original *float64; update the code around the topP variable
initialization (referencing topP and req.TopP, and remove lo.FromPtr and
common.GetPointer usage) to eliminate the unnecessary conversion.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 01c63e1 and 0e9198e.

📒 Files selected for processing (52)
  • .cursor/rules/project.mdc
  • AGENTS.md
  • CLAUDE.md
  • controller/channel-test.go
  • controller/relay.go
  • dto/audio.go
  • dto/claude.go
  • dto/embedding.go
  • dto/gemini.go
  • dto/gemini_generation_config_test.go
  • dto/openai_image.go
  • dto/openai_request.go
  • dto/openai_request_zero_value_test.go
  • dto/rerank.go
  • relay/channel/ali/image.go
  • relay/channel/ali/image_wan.go
  • relay/channel/ali/rerank.go
  • relay/channel/ali/text.go
  • relay/channel/aws/dto.go
  • relay/channel/baidu/relay-baidu.go
  • relay/channel/claude/relay-claude.go
  • relay/channel/cloudflare/relay_cloudflare.go
  • relay/channel/codex/adaptor.go
  • relay/channel/cohere/relay-cohere.go
  • relay/channel/coze/relay-coze.go
  • relay/channel/dify/relay-dify.go
  • relay/channel/gemini/adaptor.go
  • relay/channel/gemini/relay-gemini.go
  • relay/channel/minimax/adaptor.go
  • relay/channel/mistral/text.go
  • relay/channel/ollama/relay-ollama.go
  • relay/channel/openai/adaptor.go
  • relay/channel/perplexity/adaptor.go
  • relay/channel/perplexity/relay-perplexity.go
  • relay/channel/replicate/adaptor.go
  • relay/channel/siliconflow/adaptor.go
  • relay/channel/tencent/relay-tencent.go
  • relay/channel/vertex/adaptor.go
  • relay/channel/vertex/dto.go
  • relay/channel/volcengine/adaptor.go
  • relay/channel/xai/adaptor.go
  • relay/channel/xunfei/relay-xunfei.go
  • relay/channel/zhipu/adaptor.go
  • relay/channel/zhipu/relay-zhipu.go
  • relay/channel/zhipu_4v/adaptor.go
  • relay/channel/zhipu_4v/relay-zhipu_v4.go
  • relay/claude_handler.go
  • relay/compatible_handler.go
  • relay/helper/valid_request.go
  • relay/image_handler.go
  • service/convert.go
  • service/openaicompat/chat_to_responses.go

Comment thread relay/channel/aws/dto.go
Comment on lines +97 to +109
if (req.MaxTokens != nil && *req.MaxTokens != 0) || (req.Temperature != nil && *req.Temperature != 0) || (req.TopP != nil && *req.TopP != 0) || (req.TopK != nil && *req.TopK != 0) || req.Stop != nil {
novaReq.InferenceConfig = &NovaInferenceConfig{}
if req.MaxTokens != 0 {
novaReq.InferenceConfig.MaxTokens = int(req.MaxTokens)
if req.MaxTokens != nil && *req.MaxTokens != 0 {
novaReq.InferenceConfig.MaxTokens = int(*req.MaxTokens)
}
if req.Temperature != nil && *req.Temperature != 0 {
novaReq.InferenceConfig.Temperature = *req.Temperature
}
if req.TopP != 0 {
novaReq.InferenceConfig.TopP = req.TopP
if req.TopP != nil && *req.TopP != 0 {
novaReq.InferenceConfig.TopP = *req.TopP
}
if req.TopK != 0 {
novaReq.InferenceConfig.TopK = req.TopK
if req.TopK != nil && *req.TopK != 0 {
novaReq.InferenceConfig.TopK = *req.TopK

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

Explicit zero values are still being filtered out in Nova mapping.

Line 97/99/105/108 still gate on *req.<field> != 0, so requests that explicitly set 0 are treated as omitted. This contradicts the PR goal and changes behavior for clients relying on explicit zero.

Proposed fix
- if (req.MaxTokens != nil && *req.MaxTokens != 0) || (req.Temperature != nil && *req.Temperature != 0) || (req.TopP != nil && *req.TopP != 0) || (req.TopK != nil && *req.TopK != 0) || req.Stop != nil {
+ if req.MaxTokens != nil || req.Temperature != nil || req.TopP != nil || req.TopK != nil || req.Stop != nil {
     novaReq.InferenceConfig = &NovaInferenceConfig{}
-    if req.MaxTokens != nil && *req.MaxTokens != 0 {
+    if req.MaxTokens != nil {
         novaReq.InferenceConfig.MaxTokens = int(*req.MaxTokens)
     }
-    if req.Temperature != nil && *req.Temperature != 0 {
+    if req.Temperature != nil {
         novaReq.InferenceConfig.Temperature = *req.Temperature
     }
-    if req.TopP != nil && *req.TopP != 0 {
+    if req.TopP != nil {
         novaReq.InferenceConfig.TopP = *req.TopP
     }
-    if req.TopK != nil && *req.TopK != 0 {
+    if req.TopK != nil {
         novaReq.InferenceConfig.TopK = *req.TopK
     }

Also, to truly preserve zero in outbound JSON, NovaInferenceConfig scalar fields should be pointers (or omitempty removed), otherwise zeros are still dropped at marshal time.

📝 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 (req.MaxTokens != nil && *req.MaxTokens != 0) || (req.Temperature != nil && *req.Temperature != 0) || (req.TopP != nil && *req.TopP != 0) || (req.TopK != nil && *req.TopK != 0) || req.Stop != nil {
novaReq.InferenceConfig = &NovaInferenceConfig{}
if req.MaxTokens != 0 {
novaReq.InferenceConfig.MaxTokens = int(req.MaxTokens)
if req.MaxTokens != nil && *req.MaxTokens != 0 {
novaReq.InferenceConfig.MaxTokens = int(*req.MaxTokens)
}
if req.Temperature != nil && *req.Temperature != 0 {
novaReq.InferenceConfig.Temperature = *req.Temperature
}
if req.TopP != 0 {
novaReq.InferenceConfig.TopP = req.TopP
if req.TopP != nil && *req.TopP != 0 {
novaReq.InferenceConfig.TopP = *req.TopP
}
if req.TopK != 0 {
novaReq.InferenceConfig.TopK = req.TopK
if req.TopK != nil && *req.TopK != 0 {
novaReq.InferenceConfig.TopK = *req.TopK
if req.MaxTokens != nil || req.Temperature != nil || req.TopP != nil || req.TopK != nil || req.Stop != nil {
novaReq.InferenceConfig = &NovaInferenceConfig{}
if req.MaxTokens != nil {
novaReq.InferenceConfig.MaxTokens = int(*req.MaxTokens)
}
if req.Temperature != nil {
novaReq.InferenceConfig.Temperature = *req.Temperature
}
if req.TopP != nil {
novaReq.InferenceConfig.TopP = *req.TopP
}
if req.TopK != nil {
novaReq.InferenceConfig.TopK = *req.TopK
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/aws/dto.go` around lines 97 - 109, The mapping currently treats
explicit zero values as absent because each gate checks "*req.<Field> != 0"
before copying; change the logic in the block that sets novaReq.InferenceConfig
so each field is included when the request pointer is non-nil (remove the "*...
!= 0" checks and only test for nil), and update NovaInferenceConfig's scalar
fields to be pointer types (or remove omitempty) so zeros are preserved when
marshaling; ensure novaReq.InferenceConfig = &NovaInferenceConfig{} is allocated
once and then assign pointer values (e.g., set MaxTokens to a pointer to int
when req.MaxTokens != nil) so explicit 0 values are propagated.

Comment on lines +216 to +219
if textRequest.Seed != nil && *textRequest.Seed != 0 {
geminiSeed := int64(lo.FromPtr(textRequest.Seed))
geminiRequest.GenerationConfig.Seed = common.GetPointer(geminiSeed)
}

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

Explicit seed: 0 is currently dropped during OpenAI→Gemini conversion.

The != 0 guard turns an explicitly provided zero into “absent”. If zero is intentional, this breaks request fidelity.

🔧 Suggested fix
-if textRequest.Seed != nil && *textRequest.Seed != 0 {
-	geminiSeed := int64(lo.FromPtr(textRequest.Seed))
+if textRequest.Seed != nil {
+	geminiSeed := int64(*textRequest.Seed)
 	geminiRequest.GenerationConfig.Seed = common.GetPointer(geminiSeed)
 }
📝 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 textRequest.Seed != nil && *textRequest.Seed != 0 {
geminiSeed := int64(lo.FromPtr(textRequest.Seed))
geminiRequest.GenerationConfig.Seed = common.GetPointer(geminiSeed)
}
if textRequest.Seed != nil {
geminiSeed := int64(*textRequest.Seed)
geminiRequest.GenerationConfig.Seed = common.GetPointer(geminiSeed)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/gemini/relay-gemini.go` around lines 216 - 219, The current
check `if textRequest.Seed != nil && *textRequest.Seed != 0` drops an explicitly
provided zero seed; change the guard to only test for nil (i.e., `if
textRequest.Seed != nil`) and then convert and assign the seed as you do now
(`geminiSeed := int64(lo.FromPtr(textRequest.Seed))` and
`geminiRequest.GenerationConfig.Seed = common.GetPointer(geminiSeed)`), so an
explicit 0 value is preserved when converting from textRequest to
geminiRequest.GenerationConfig.Seed.

Comment on lines +318 to 321
if lo.FromPtrOr(request.MaxCompletionTokens, uint(0)) == 0 && lo.FromPtrOr(request.MaxTokens, uint(0)) != 0 {
request.MaxCompletionTokens = request.MaxTokens
request.MaxTokens = 0
request.MaxTokens = 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

max_completion_tokens: 0 is treated as missing and overwritten.

The current condition merges explicit zero with nil, so user intent can be lost when max_tokens is present.

🔧 Suggested fix
-if lo.FromPtrOr(request.MaxCompletionTokens, uint(0)) == 0 && lo.FromPtrOr(request.MaxTokens, uint(0)) != 0 {
+if request.MaxCompletionTokens == nil && lo.FromPtrOr(request.MaxTokens, uint(0)) != 0 {
 	request.MaxCompletionTokens = request.MaxTokens
 	request.MaxTokens = nil
}
📝 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 lo.FromPtrOr(request.MaxCompletionTokens, uint(0)) == 0 && lo.FromPtrOr(request.MaxTokens, uint(0)) != 0 {
request.MaxCompletionTokens = request.MaxTokens
request.MaxTokens = 0
request.MaxTokens = nil
}
if request.MaxCompletionTokens == nil && lo.FromPtrOr(request.MaxTokens, uint(0)) != 0 {
request.MaxCompletionTokens = request.MaxTokens
request.MaxTokens = nil
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/openai/adaptor.go` around lines 318 - 321, The code treats an
explicit max_completion_tokens: 0 as missing because it compares
dereferenced-or-default values; change the condition to check pointer nils
instead so a user-specified zero is preserved. Replace the current if that uses
lo.FromPtrOr on request.MaxCompletionTokens with a nil-check: only copy
request.MaxTokens into request.MaxCompletionTokens when
request.MaxCompletionTokens == nil and request.MaxTokens != nil, then set
request.MaxTokens = nil (refer to request.MaxCompletionTokens and
request.MaxTokens in adaptor.go).

Comment on lines 56 to +59
if sfRequest.BatchSize == 0 {
sfRequest.BatchSize = request.N
if request.N != nil {
sfRequest.BatchSize = lo.FromPtr(request.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

Preserve explicit batch_size: 0 instead of treating it as missing.

This fallback uses sfRequest.BatchSize == 0 as the absence check, so an explicitly provided zero can be overwritten by n, which breaks the explicit-zero preservation intent.

💡 Proposed fix
  sfRequest.Model = request.Model
  sfRequest.Prompt = request.Prompt
  // 优先使用image_size/batch_size,否则使用OpenAI标准的size/n
  if sfRequest.ImageSize == "" {
  	sfRequest.ImageSize = request.Size
  }
- if sfRequest.BatchSize == 0 {
- 	if request.N != nil {
- 		sfRequest.BatchSize = lo.FromPtr(request.N)
- 	}
- }
+ _, hasBatchSize := request.Extra["batch_size"]
+ if !hasBatchSize && sfRequest.BatchSize == 0 && request.N != nil {
+ 	sfRequest.BatchSize = lo.FromPtr(request.N)
+ }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/siliconflow/adaptor.go` around lines 56 - 59, The code treats
sfRequest.BatchSize==0 as "unset" and therefore overwrites an explicit zero with
request.N; change the absence check to use a sentinel (e.g. initialize BatchSize
to -1 where sfRequest is created) and only copy from request.N when
sfRequest.BatchSize == -1, or alternatively add a boolean flag like BatchSizeSet
and check that instead; update the conditional around sfRequest.BatchSize,
request.N and lo.FromPtr(request.N) in the adaptor.go block so explicit zero
values are preserved.

@@ -305,7 +306,7 @@ func (a *Adaptor) ConvertOpenAIRequest(c *gin.Context, info *relaycommon.RelayIn
var extra map[string]any
if err := json.Unmarshal(request.ExtraBody, &extra); 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 | 🟡 Minor

Use common.Unmarshal() instead of json.Unmarshal.

Direct usage of json.Unmarshal violates coding guidelines. As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions from common/json.go".

Proposed fix
-			if err := json.Unmarshal(request.ExtraBody, &extra); err == nil {
+			if err := common.Unmarshal(request.ExtraBody, &extra); err == nil {
📝 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(request.ExtraBody, &extra); err == nil {
if err := common.Unmarshal(request.ExtraBody, &extra); err == nil {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@relay/channel/vertex/adaptor.go` at line 307, Replace the direct call to
json.Unmarshal(request.ExtraBody, &extra) with the project's wrapper
common.Unmarshal(request.ExtraBody, &extra), preserving the existing
error-checking logic; update the import usage (remove direct json usage if
unused) and ensure the error return from common.Unmarshal is handled exactly as
before where it currently checks err == nil around request.ExtraBody and the
variable extra.

Comment thread service/convert.go
Comment on lines +711 to 719
if geminiRequest.GenerationConfig.TopP != nil && *geminiRequest.GenerationConfig.TopP > 0 {
openaiRequest.TopP = lo.ToPtr(*geminiRequest.GenerationConfig.TopP)
}
if geminiRequest.GenerationConfig.TopK > 0 {
openaiRequest.TopK = int(geminiRequest.GenerationConfig.TopK)
if geminiRequest.GenerationConfig.TopK != nil && *geminiRequest.GenerationConfig.TopK > 0 {
openaiRequest.TopK = lo.ToPtr(int(*geminiRequest.GenerationConfig.TopK))
}
if geminiRequest.GenerationConfig.MaxOutputTokens > 0 {
openaiRequest.MaxTokens = geminiRequest.GenerationConfig.MaxOutputTokens
if geminiRequest.GenerationConfig.MaxOutputTokens != nil && *geminiRequest.GenerationConfig.MaxOutputTokens > 0 {
openaiRequest.MaxTokens = lo.ToPtr(*geminiRequest.GenerationConfig.MaxOutputTokens)
}

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

Gemini→OpenAI conversion currently drops explicit zero values.

These > 0 guards discard non-nil zero inputs, so explicit 0 is not preserved through conversion.

🔧 Suggested fix
-if geminiRequest.GenerationConfig.TopP != nil && *geminiRequest.GenerationConfig.TopP > 0 {
+if geminiRequest.GenerationConfig.TopP != nil {
 	openaiRequest.TopP = lo.ToPtr(*geminiRequest.GenerationConfig.TopP)
}
-if geminiRequest.GenerationConfig.TopK != nil && *geminiRequest.GenerationConfig.TopK > 0 {
+if geminiRequest.GenerationConfig.TopK != nil {
 	openaiRequest.TopK = lo.ToPtr(int(*geminiRequest.GenerationConfig.TopK))
}
-if geminiRequest.GenerationConfig.MaxOutputTokens != nil && *geminiRequest.GenerationConfig.MaxOutputTokens > 0 {
+if geminiRequest.GenerationConfig.MaxOutputTokens != nil {
 	openaiRequest.MaxTokens = lo.ToPtr(*geminiRequest.GenerationConfig.MaxOutputTokens)
}
-if geminiRequest.GenerationConfig.CandidateCount != nil && *geminiRequest.GenerationConfig.CandidateCount > 0 {
+if geminiRequest.GenerationConfig.CandidateCount != nil {
 	openaiRequest.N = lo.ToPtr(*geminiRequest.GenerationConfig.CandidateCount)
}

Also applies to: 724-726

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

In `@service/convert.go` around lines 711 - 719, The conversion currently ignores
explicit zero values because it checks `> 0`; remove the `> 0` tests and only
check for non-nil before assigning so that zero is preserved: for
`geminiRequest.GenerationConfig.TopP` and `TopK` and `MaxOutputTokens` check `!=
nil` and then set `openaiRequest.TopP =
lo.ToPtr(*geminiRequest.GenerationConfig.TopP)`, `openaiRequest.TopK =
lo.ToPtr(int(*geminiRequest.GenerationConfig.TopK))`, and
`openaiRequest.MaxTokens =
lo.ToPtr(*geminiRequest.GenerationConfig.MaxOutputTokens)` respectively (apply
same change to the other similar block referenced).

@Calcium-Ion
Calcium-Ion merged commit 6b9296c into QuantumNous:main Mar 1, 2026
1 check passed
ennnnny pushed a commit to ennnnny/new-api that referenced this pull request Mar 17, 2026
…nore

fix: preserve explicit zero values in native relay requests
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.

2 participants