Skip to content

Feat/doubao batch - #1285

Closed
whatattitude wants to merge 186 commits into
QuantumNous:mainfrom
Furion-cn:feat/doubao-batch
Closed

Feat/doubao batch#1285
whatattitude wants to merge 186 commits into
QuantumNous:mainfrom
Furion-cn:feat/doubao-batch

Conversation

@whatattitude

@whatattitude whatattitude commented Jun 22, 2025

Copy link
Copy Markdown

Summary by CodeRabbit

  • New Features

    • Added support for new AI channels: XAI and Doubao Offline.
    • Introduced batch chat capabilities with concurrency and retry management for Volcengine channel.
    • Added request and response persistence for text requests with daily table management.
    • Implemented detailed Prometheus metrics for relay requests, tokens, and errors.
    • Added group- and role-based filtering for users, tokens, channels, and groups.
    • Added health check endpoint and API to toggle request logging.
    • Added billing data export as Excel with token-level filtering.
    • Added support for custom channel endpoints and offline access point addresses.
  • Improvements

    • Enhanced log system with daily partitioning, rotation, enriched caller info, and error classification.
    • Improved error messages and user feedback for upstream request failures.
    • Added mock response middleware for test traffic scenarios.
    • Refined request body parsing and header propagation for robustness and traceability.
    • Enhanced UI components with role-based filtering and export functionality.
    • Added detailed metrics instrumentation across relay helpers and middleware.
    • Extended model ratio and pricing configuration with new mappings.
  • Bug Fixes

    • Fixed handling of empty or malformed request bodies with clearer error messages.
    • Corrected filtering logic to restrict data visibility for non-admin users.
    • Improved request ID generation with stronger randomness and centralized logic.
  • Chores

    • Updated dependencies including Prometheus client, Excel export, and cloud SDKs.
    • Simplified and refocused Docker build workflow targeting Tencent Cloud Registry.
    • Added Redis-based distributed locking and keep-alive management for batch requests.
    • Refactored code structure for better modularity and maintainability.
  • Documentation

    • Updated API routes and handlers for new features and administrative controls.
    • Added UI labels and tooltips for new channel endpoint configurations.

@coderabbitai

coderabbitai Bot commented Jun 22, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

This update introduces extensive enhancements and new features across the backend and frontend. Key changes include: daily-partitioned log and request persistence tables, a robust Prometheus metrics system, expanded billing and quota reporting with Excel export, new AI channel integrations (notably XAI and Doubao offline), improved relay and request handling (with modularized info/helper functions and test traffic support), group and token management refinements, and numerous supporting utilities and endpoints for observability, admin control, and reliability.

Changes

File(s) / Area Change Summary
.github/workflows/docker-image-amd64.yml Workflow renamed, triggers/inputs refactored, registry switched to Tencent, build/push logic simplified.
.gitignore Added ignores for new directories, environment, test, profiling, and log files.
Dockerfile Switched to private image mirrors, added Go proxy, improved dependency tidying, updated Alpine repo URLs.
bin/groups.sql, bin/log_requestid.sql Added groups table; added request_id column to logs.
common/constants.go, common/init.go, common/origin.go, common/time.go Added config flags, group/thread-safe maps, new channel types, origin header utilities, and Beijing time helpers.
common/logger.go Introduced log rotation, enhanced log context, error categorization, and sampling.
controller/* Added/updated endpoints: ping, toggle request log, token/group filtering by role, billing/Excel export, quota with token name, user rate limit config, and error message improvements.
dto/claude.go, dto/error.go, dto/openai_request.go, dto/realtime.go Added Claude DTOs, custom status codes, extended request/response fields, and token details.
go.mod Updated/added/removed dependencies, notably Prometheus, Excelize, Volcengine SDK.
main.go Added config flags, time logging, log passthrough/sampling, central DB, persistence, metrics, and improved startup.
metrics/metrics.go New Prometheus metrics package with counters/histograms and registration helpers.
middleware/* Added request logger, improved request ID, mock/test traffic middleware, enhanced error logging, and context utilities.
model/* Log/request persistence with daily tables, central DB, group/token management, billing aggregation, quota by token, and user rate limit config.
relay/channel/* New/updated adaptors: XAI, Doubao offline, header propagation, mock/test traffic, batch chat, keepalive, and streaming helpers.
relay/common/relay_info.go, relay/constant/* Extended relay info, new API/channel types, updated path mapping.
relay/relay-*.go, relay/relay_adaptor.go, relay/relay_direct.go Modularized info/helper functions, metrics instrumentation, improved error handling, test traffic, and persistence.
router/api-router.go, router/relay-router.go Added endpoints: health check, toggle request log, all tokens, user rate limit config, billing Excel export, and ping.
service/* Improved error messages, test traffic handling, and context-aware token counting.
setting/operation_setting/model-ratio.go, setting/user_usable_group.go Added new model ratio maps, improved logging, and group ID lookup.
web/src/components/*, web/src/pages/*, web/src/helpers/*, web/src/constants/*, web/vite.config.js Frontend: role-based filtering, new columns, token/group management, billing export, new channel types, improved utils, and server config.

Sequence Diagram(s)

Example: Text Relay Request Flow with Metrics and Test Traffic

sequenceDiagram
    participant User
    participant API
    participant Middleware
    participant RelayHelper
    participant Metrics
    participant Model
    participant UpstreamAI

    User->>API: HTTP /v1/chat/completions
    API->>Middleware: RequestLogger, MockResponse, RequestId
    alt X-Test-Traffic header == true
        Middleware-->>User: Mock JSON response
    else
        Middleware->>RelayHelper: TextInfo
        RelayHelper->>RelayHelper: Validate request, prepare relayInfo
        RelayHelper->>Metrics: Increment total counter
        RelayHelper->>UpstreamAI: Forward request
        UpstreamAI-->>RelayHelper: Response
        RelayHelper->>Metrics: Increment success/failure, observe duration
        RelayHelper->>Model: Save request/response if persistence enabled
        RelayHelper-->>User: Upstream response
    end
Loading

Example: Billing Data Export Flow

sequenceDiagram
    participant Admin
    participant WebUI
    participant API
    participant Controller
    participant Model
    participant Excelize

    Admin->>WebUI: Click "Export Billing"
    WebUI->>API: GET /api/data/billing?params...
    API->>Controller: ExportBillingExcel
    Controller->>Model: GetBillingAndExportExcel
    Model->>Excelize: Generate Excel file
    Excelize-->>Model: Excel bytes
    Model-->>Controller: Excel bytes
    Controller-->>API: Excel bytes + headers
    API-->>WebUI: Excel file download
Loading

Possibly related PRs

  • feat: auto分组 #1239: Adds support for an "auto" group aggregating multiple groups and updates group filtering logic; related to group-based access control enhancements.

Poem

🐇
Logs now rotate, metrics bloom,
Billing exports from the server room.
Groups and tokens filtered right,
XAI and Doubao join the fight.
With new endpoints, tests, and time,
This code hops forward—oh, how sublime!
🥕✨

✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@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: 70

🔭 Outside diff range comments (2)
model/channel.go (1)

160-169: Potential security issue: Always exposing sensitive key field.

The modification removes the conditional logic that previously omitted the key field when selectAll is false. This means the sensitive API key will always be returned, regardless of the selectAll parameter intent.

This could expose sensitive channel keys in contexts where they shouldn't be visible. Consider reverting this change or ensuring proper access control:

 func GetChannelById(id int, selectAll bool) (*Channel, error) {
 	channel := Channel{Id: id}
 	var err error = nil
 	if selectAll {
 		err = DB.First(&channel, "id = ?", id).Error
 	} else {
-		err = DB.First(&channel, "id = ?", id).Error
+		err = DB.Omit("key").First(&channel, "id = ?", id).Error
 	}
 	return &channel, err
 }
controller/usedata.go (1)

33-51: Remove duplicate code by reusing GetAllQuotaDates.

The GetBilling function is identical to GetAllQuotaDates except it passes an empty string for token_name.

 func GetBilling(c *gin.Context) {
-	startTimestamp, _ := strconv.ParseInt(c.Query("start_timestamp"), 10, 64)
-	endTimestamp, _ := strconv.ParseInt(c.Query("end_timestamp"), 10, 64)
-	username := c.Query("username")
-	dates, err := model.GetAllQuotaDates(startTimestamp, endTimestamp, username, "")
-	if err != nil {
-		c.JSON(http.StatusOK, gin.H{
-			"success": false,
-			"message": err.Error(),
-		})
-		return
-	}
-	c.JSON(http.StatusOK, gin.H{
-		"success": true,
-		"message": "",
-		"data":    dates,
-	})
-	return
+	// Set token_name to empty string for billing queries
+	c.Request.URL.RawQuery = c.Request.URL.Query().Encode() + "&token_name="
+	GetAllQuotaDates(c)
 }
♻️ Duplicate comments (4)
controller/group.go (1)

38-52: Duplicate security concern: Same filtering vulnerability exists here.

The same strings.Contains vulnerability exists in GetUserGroups as identified in GetGroups.

controller/channel.go (3)

104-121: Same security concern applies to regular mode filtering.

The same partial string matching issue exists here as in the tag mode filtering above.


263-280: Same filtering issues apply to SearchChannels regular mode.

This filtering logic has the same security and duplication concerns as mentioned in previous comments.


233-249: Duplicated filtering logic in SearchChannels function.

The filtering logic is identical to GetAllChannels and has the same security concern with partial string matching.

Consider extracting the channel filtering logic into a helper function to reduce code duplication:

func filterChannelsForUser(channels []*model.Channel, userRole int, username string) []*model.Channel {
	if userRole >= 100 {
		return channels
	}
	
	filteredChannels := make([]*model.Channel, 0)
	for _, channel := range channels {
		groups := strings.Split(channel.Group, ",")
		for _, group := range groups {
			if strings.TrimSpace(group) == username {
				filteredChannels = append(filteredChannels, channel)
				break
			}
		}
	}
	return filteredChannels
}
🧹 Nitpick comments (40)
.gitignore (1)

12-21: Consider optimizing redundant ignore patterns.

The .env* pattern on line 21 makes the specific .env_2 and .env_3 entries on lines 15 and 19 redundant. You can simplify by keeping only the .env* pattern.

-.env_2
-*_test.go
-out.log
-out.log_2
-.env_3
-*.prof
-.env*
+*_test.go
+out.log*
+*.prof
+.env*
bin/log_requestid.sql (1)

1-1: Consider adding an index for query performance.

The ALTER TABLE statement is correct and the column definition is appropriate. However, since request IDs are likely to be used for log lookups and filtering, consider adding an index for better query performance.

ALTER TABLE logs ADD COLUMN request_id VARCHAR(255) DEFAULT '' COMMENT '请求ID';
-- Consider adding an index for performance
CREATE INDEX idx_logs_request_id ON logs(request_id);
controller/playground.go (1)

62-63: Update error message text to reflect the change from group name to group ID.

The change from group name to group ID in error messages improves consistency and stability. However, consider updating the Chinese text "当前分组id" to be more grammatically correct, and verify that API consumers can handle this change.

-		message := fmt.Sprintf("当前分组id %d 下对于模型 %s 无可用渠道", groupId, playgroundRequest.Model)
+		message := fmt.Sprintf("当前分组 (ID: %d) 下对于模型 %s 无可用渠道", groupId, playgroundRequest.Model)
#!/bin/bash
# Description: Verify that the GetGroupId function exists and find other similar error messages
# Expected: Find the function definition and check for consistency

# Search for GetGroupId function definition
ast-grep --pattern 'func GetGroupId($$$) $$$'

# Search for similar error message patterns that might need updating
rg -A 2 -B 2 "当前分组.*无可用渠道"
controller/misc.go (1)

274-282: Review the header value assignment.

The ping endpoint implementation is correct, but the header assignment c.Writer.Header().Set("Retry_request_id", "Retry_request_id") sets the header value to the same string as the header name. This seems unusual - typically you'd set a unique request ID as the value.

Consider whether this should be:

-c.Writer.Header().Set("Retry_request_id", "Retry_request_id")
+c.Writer.Header().Set("Retry_request_id", common.GetRandomString(16)) // or similar unique ID
service/error.go (1)

33-33: Enhanced error reporting with error code inclusion.

The addition of error code to the error message improves debugging capabilities by providing more context about upstream failures.

Consider using a localization framework for error messages instead of hardcoded Chinese text to improve internationalization support:

-		text = fmt.Sprintf("请求上游地址失败,错误信息:%s, code is %s", text, code)
+		text = fmt.Sprintf(common.GetLocalizedMessage("upstream_request_failed"), text, code)
controller/option.go (1)

7-7: Verify import necessity and coupling concerns.

The middleware import introduces coupling between the controller and middleware layers. Consider if this direct dependency is necessary.

middleware/mock.go (1)

13-59: Well-structured mock middleware with room for improvement.

The middleware correctly implements the gin pattern and serves its purpose for test traffic. However, consider making the mock response more flexible for different use cases.

Consider making the mock response configurable rather than hardcoded:

type MockConfig struct {
    ID      string
    Model   string
    Content string
    // other configurable fields
}

func MockResponseWithConfig(config MockConfig) gin.HandlerFunc {
    // implementation with configurable response
}

This would make the middleware more reusable across different testing scenarios.

web/src/pages/Channel/EditChannel.js (1)

541-541: Consider extracting channel type constants.

The conditional logic excludes multiple channel types including the new type 100. Consider defining these magic numbers as named constants to improve maintainability.

Example:

+const CHANNEL_TYPES = {
+  AZURE: 3,
+  CUSTOM: 8,
+  FASTGPT: 22,
+  SUNO: 36,
+  SOMETHING: 45,
+  DOUBAO_OFFLINE: 100
+};

-{inputs.type !== 3 && inputs.type !== 8 && inputs.type !== 22 && inputs.type !== 36 && inputs.type !== 45 && inputs.type !== 100 && (
+{![CHANNEL_TYPES.AZURE, CHANNEL_TYPES.CUSTOM, CHANNEL_TYPES.FASTGPT, CHANNEL_TYPES.SUNO, CHANNEL_TYPES.SOMETHING, CHANNEL_TYPES.DOUBAO_OFFLINE].includes(inputs.type) && (
common/origin.go (1)

1-28: Consider adding input validation for security.

While the current implementation is functionally correct, consider adding validation to ensure the extracted IDs are within reasonable bounds to prevent potential security issues if malicious headers are provided.

Example enhancement:

func GetOriginUserId(c *gin.Context, defaultUserId int) int {
	if originUserId := c.GetHeader("X-Origin-User-ID"); originUserId != "" {
		if userId, err := strconv.Atoi(originUserId); err == nil {
+			// Validate reasonable bounds
+			if userId > 0 && userId < 2147483647 {
				return userId
+			}
		}
	}
	return defaultUserId
}
relay/relay_direct.go (2)

21-23: Clarify the MaxTokens validation threshold.

The validation directRequest.MaxTokens > math.MaxInt32/2 uses an arbitrary threshold without explanation. Consider documenting why this specific limit is chosen or using a named constant.

+const MaxAllowedTokens = math.MaxInt32 / 2
+
 func getAndValidateDirectRequest(c *gin.Context, relayInfo *relaycommon.RelayInfo) (*dto.GeneralOpenAIRequest, error) {
 	if strings.HasPrefix(relayInfo.OriginModelName, "claude") {
 		directRequest := &claude.ClaudeRequest{}
 		err := common.UnmarshalBodyReusable(c, directRequest)
 		if err != nil {
 			return nil, err
 		}
-		if directRequest.MaxTokens > math.MaxInt32/2 {
+		if directRequest.MaxTokens > MaxAllowedTokens {
-			return nil, errors.New("max_tokens is invalid")
+			return nil, errors.New("max_tokens exceeds maximum allowed limit")
 		}

30-30: Improve error message specificity.

The generic error message doesn't indicate which models are supported or provide guidance for users.

-	return nil, errors.New("direct model not support")
+	return nil, errors.New("direct relay mode only supports Claude models")
Dockerfile (1)

20-22: Review redundant go mod tidy commands.

Running go mod tidy both before and after go mod download may be redundant. The first go mod tidy is typically sufficient to clean up the module files before downloading dependencies.

Consider removing the first go mod tidy if the second one after copying all files is sufficient:

 ADD go.mod go.sum ./
-RUN go mod tidy
 RUN go mod download
 
 COPY . .
 COPY --from=builder /build/dist ./web/dist
 RUN go mod tidy

Also applies to: 25-25

dto/error.go (1)

57-57: Add English documentation for international accessibility.

The Chinese comment limits code accessibility for international developers. Consider adding English documentation.

-// 自定义HTTP状态码 (使用非标准状态码范围)
+// Custom HTTP status codes for batch API operations
+// 自定义HTTP状态码 (使用非标准状态码范围)
relay/channel/xai/dto.go (1)

5-14: Consider adding field documentation.

The struct fields could benefit from documentation comments explaining their purpose, especially for fields that may not be self-explanatory.

 // ChatCompletionResponse represents the response from XAI chat completion API
 type ChatCompletionResponse struct {
-	Id                string `json:"id"`
-	Object            string `json:"object"`
-	Created           int64  `json:"created"`
-	Model             string `json:"model"`
-	Choices           []dto.ChatCompletionsStreamResponseChoice `json:"choices"`
-	Usage             *dto.Usage `json:"usage"`
-	SystemFingerprint string     `json:"system_fingerprint"`
+	Id                string                                     `json:"id"`                 // Unique identifier for the response
+	Object            string                                     `json:"object"`             // Object type (typically "chat.completion")
+	Created           int64                                      `json:"created"`            // Unix timestamp of creation
+	Model             string                                     `json:"model"`              // Model used for completion
+	Choices           []dto.ChatCompletionsStreamResponseChoice `json:"choices"`            // Generated completion choices
+	Usage             *dto.Usage                                 `json:"usage"`              // Token usage statistics
+	SystemFingerprint string                                     `json:"system_fingerprint"` // System fingerprint for reproducibility
 }
web/src/components/UsersTable.js (1)

408-411: Simplify the username pattern matching regex.

The current regex pattern is complex and may be error-prone. Consider simplifying it for better maintainability and reducing the risk of edge cases.

-        const usernamePattern = new RegExp(`^${currentUser.username}$|^${currentUser.username}_|_${currentUser.username}$|_${currentUser.username}_`, 'i');
+        const username = currentUser.username;
+        const usernamePattern = new RegExp(`^${username}$|^${username}_|_${username}(_|$)`, 'i');

This simplified pattern is more readable and achieves the same filtering logic.

relay/common/relay_info.go (1)

143-145: Consider making Direct mode logic more explicit.

The Direct mode logic currently uses a simple path prefix check. Consider making this more configurable or explicit for future maintainability.

+// IsDirectModeEnabled checks if direct mode should be enabled for the given path
+func IsDirectModeEnabled(path string) bool {
+	return strings.HasPrefix(path, "/v1/messages")
+}
+
 	// 使用直连模式
-	if strings.HasPrefix(c.Request.URL.Path, "/v1/messages") {
+	if IsDirectModeEnabled(c.Request.URL.Path) {
 		info.Direct = true
 	}

This makes the logic more testable and easier to extend for additional direct mode conditions.

middleware/request-logger.go (1)

14-15: Consider thread-safe configuration management.

The global boolean flag EnableRequestBodyLogging could lead to race conditions in concurrent environments. Consider using atomic operations or a configuration manager.

-var EnableRequestBodyLogging bool = false
+import "sync/atomic"
+
+var enableRequestBodyLogging int32 // 0 = false, 1 = true
+
+func SetRequestBodyLogging(enabled bool) {
+	if enabled {
+		atomic.StoreInt32(&enableRequestBodyLogging, 1)
+	} else {
+		atomic.StoreInt32(&enableRequestBodyLogging, 0)
+	}
+}
+
+func IsRequestBodyLoggingEnabled() bool {
+	return atomic.LoadInt32(&enableRequestBodyLogging) == 1
+}
common/constants.go (1)

295-348: Verify the extensive ChannelBaseURLs array expansion.

The array was expanded by 54 entries (indices 47-100) with mostly empty strings and specific URLs at indices 47 and 100. This seems excessive for just 2 new channel types.

Consider a more efficient approach:

-	"",                                          //47
-	"https://api.x.ai",                          //48
-	"",                                          //49
-	// ... many empty entries ...
-	"https://ark.cn-beijing.volces.com",         //100 - 豆包离线
+	"https://api.x.ai",                          //47 - XAI
+	"",                                          //48-99 reserved
+	"https://ark.cn-beijing.volces.com",         //100 - 豆包离线

Also note: Line 296 has the correct XAI URL, but line 295 shows an empty string for index 47. This appears to be a mismatch.

#!/bin/bash
# Description: Verify the correct URL assignment for XAI channel type
# Expected: Index 47 should have the XAI URL, not index 48

echo "=== Checking XAI channel type usage ==="
rg -A 2 -B 2 "ChannelTypeXai.*47"

echo "=== Checking ChannelBaseURLs access patterns ==="
rg -A 3 "ChannelBaseURLs\[.*\]"
web/src/pages/Token/EditToken.js (2)

1-1: Remove unused import.

useContext is imported but never used in this component.

-import React, { useEffect, useState, useContext } from 'react';
+import React, { useEffect, useState } from 'react';

462-462: Fix typo in name attribute.

The name attribute should be 'group' not 'gruop' for consistency.

-              name='gruop'
+              name='group'
dto/openai_request.go (1)

299-299: Consider using structured logging with appropriate log level.

The debug log could be improved to use structured logging and appropriate log level.

-					common.SysLog(fmt.Sprintf("Parsing audio content: data_ok=%v, format_ok=%v, format=%s, data_length=%d", ok1, ok2, format, len(data)))
+					if common.DebugEnabled {
+						common.SysLog(fmt.Sprintf("Parsing audio content: data_ok=%v, format_ok=%v, format=%s, data_length=%d", ok1, ok2, format, len(data)))
+					}
.github/workflows/docker-image-amd64.yml (2)

26-26: Update docker/login-action to v3

The static analysis tool indicates that v2 is outdated for the current GitHub Actions runner.

-        uses: docker/login-action@v2
+        uses: docker/login-action@v3

40-40: Add newline at end of file

YAMLlint indicates missing newline at end of file.

-          docker push furion-sh.tencentcloudcr.com/furion/new-api:${VERSION}
+          docker push furion-sh.tencentcloudcr.com/furion/new-api:${VERSION}
+
setting/operation_setting/model-ratio.go (1)

498-502: Remove commented-out code

Dead code should be removed to improve readability and maintainability.

-	//if strings.Contains(name, "/") {
-	//	if ratio, ok := CompletionRatio[name]; ok {
-	//		return ratio
-	//	}
-	//}
relay/channel/xai/text.go (2)

77-80: Consider returning the error instead of just logging

The error from closing the response body is logged but not returned, which could mask underlying issues.

 	err := resp.Body.Close()
 	if err != nil {
-		//return service.OpenAIErrorWrapper(err, "close_response_body_failed", http.StatusInternalServerError), nil
-		common.SysError("close_response_body_failed: " + err.Error())
+		return service.OpenAIErrorWrapper(err, "close_response_body_failed", http.StatusInternalServerError), usage
 	}

105-107: Handle multiple header values correctly

The code only sets the first value of each header, which could lose information if headers have multiple values.

 	for k, v := range resp.Header {
-		c.Writer.Header().Set(k, v[0])
+		c.Writer.Header()[k] = v
 	}
controller/relay.go (3)

137-145: Consider extracting metrics tracking to reduce code complexity

The metrics tracking logic adds significant complexity to the main flow. Consider extracting it to separate functions.

func trackRequestMetrics(c *gin.Context, channel *model.Channel, requestModel, group, tokenKey, tokenName string, isRetry bool) {
    channelID := strconv.Itoa(channel.Id)
    if !isRetry {
        metrics.IncrementRelayRequestE2ETotalCounter(channelID, channel.Name, requestModel, group, tokenKey, tokenName, 1)
    } else {
        channelTag := ""
        if channel.Tag != nil {
            channelTag = *channel.Tag
        }
        metrics.IncrementRelayRetryCounter(channelID, channel.Name, channelTag, channel.GetBaseURL(), requestModel, group, 1)
    }
}

380-383: Simplify complex conditional logic

The condition combines multiple checks that could be better structured for readability.

-	if strings.Contains(openaiErr.Error.Message, "deadline exceeded") || strings.Contains(openaiErr.Error.Message, "request canceled") || strings.Contains(openaiErr.Error.Message, "copy_response_body_failed") {
-		common.LogInfo(c, fmt.Sprintf("客户端请求下游超时,不再重试 : %s", openaiErr.Error.Message))
-		return false
-	}
+	clientTimeoutErrors := []string{"deadline exceeded", "request canceled", "copy_response_body_failed"}
+	for _, timeoutErr := range clientTimeoutErrors {
+		if strings.Contains(openaiErr.Error.Message, timeoutErr) {
+			common.LogInfo(c, fmt.Sprintf("客户端请求下游超时,不再重试 : %s", openaiErr.Error.Message))
+			return false
+		}
+	}

357-375: Consider using a map for batch error code checking

The repetitive batch error code checks could be simplified using a map or slice.

-	// 处理自定义的 NewAPI batch 错误码
-	if openaiErr.StatusCode == dto.StatusNewAPIBatchRateLimitExceeded {
-		return false
-	}
-	if openaiErr.StatusCode == dto.StatusNewAPIBatchTimeout {
-		return false
-	}
-	// ... (other similar checks)
+	// 处理自定义的 NewAPI batch 错误码
+	nonRetryableBatchCodes := []int{
+		dto.StatusNewAPIBatchRateLimitExceeded,
+		dto.StatusNewAPIBatchTimeout,
+		dto.StatusNewAPIBatchInternal,
+		dto.StatusNewAPIBatchSubmitted,
+		dto.StatusNewAPIBatchAccepted,
+		dto.StatusRequestConflict,
+	}
+	for _, code := range nonRetryableBatchCodes {
+		if openaiErr.StatusCode == code {
+			return false
+		}
+	}
relay/channel/api_request.go (1)

188-191: Optimize context creation by chaining WithValue calls.

The current implementation creates an intermediate context that's immediately discarded.

Apply this diff to chain the context creation:

-	requestId := c.GetString(onecommon.RequestIdKey)
-	ctx := context.WithValue(c.Request.Context(), onecommon.RequestIdKey, requestId)
-	ctx = context.WithValue(ctx, "gin_context", c)
+	requestId := c.GetString(onecommon.RequestIdKey)
+	ctx := context.WithValue(
+		context.WithValue(c.Request.Context(), onecommon.RequestIdKey, requestId),
+		ginContextKey, c)
relay/relay-text.go (1)

142-145: Consider consistent handling of test traffic for all metrics.

Input token metrics are recorded for test traffic (lines 142-145), but consumption logging is skipped (lines 427-432). This inconsistency could skew metrics.

Consider checking for test traffic earlier and skipping all metrics recording:

+	// Check if this is test traffic early
+	isTestTraffic := c.GetHeader("X-Test-Traffic") == "true"
+	
 	// Record input tokens metric
-	tokenName := c.GetString("token_name")
-	userName := c.GetString("username")
-	metrics.IncrementInputTokens(strconv.Itoa(relayInfo.ChannelId), relayInfo.ChannelName, textRequest.Model, relayInfo.Group, strconv.Itoa(relayInfo.UserId), userName, tokenName, float64(promptTokens))
+	if !isTestTraffic {
+		tokenName := c.GetString("token_name")
+		userName := c.GetString("username")
+		metrics.IncrementInputTokens(strconv.Itoa(relayInfo.ChannelId), relayInfo.ChannelName, textRequest.Model, relayInfo.Group, strconv.Itoa(relayInfo.UserId), userName, tokenName, float64(promptTokens))
+	}

Also applies to: 427-432

relay/channel/xai/adaptor.go (3)

51-68: Extract model name processing logic into a separate function.

The model name processing logic is complex and would benefit from being in a separate function for clarity and testability.

Extract to a helper function:

func processGrok3MiniModel(request *dto.GeneralOpenAIRequest) (model string, reasoningEffort string) {
    model = request.Model
    
    if request.MaxCompletionTokens == 0 && request.MaxTokens != 0 {
        request.MaxCompletionTokens = request.MaxTokens
        request.MaxTokens = 0
    }
    
    suffixes := map[string]string{
        "-high": "high",
        "-low": "low", 
        "-medium": "medium",
    }
    
    for suffix, effort := range suffixes {
        if strings.HasSuffix(model, suffix) {
            return strings.TrimSuffix(model, suffix), effort
        }
    }
    
    return model, ""
}

30-30: Document or log the Size field clearing.

The Size field is silently cleared without any validation or logging.

Add a comment explaining why this is necessary:

 func (a *Adaptor) ConvertImageRequest(c *gin.Context, info *relaycommon.RelayInfo, request dto.ImageRequest) (any, error) {
+	// XAI API doesn't support size parameter for image generation
 	request.Size = ""
 	return request, nil
 }

91-94: Remove commented-out code.

Commented-out code should be removed to maintain code cleanliness.

Remove these lines:

-	//if _, ok := usage.(*dto.Usage); ok && usage != nil {
-	//	usage.(*dto.Usage).CompletionTokens = usage.(*dto.Usage).TotalTokens - usage.(*dto.Usage).PromptTokens
-	//}
-
common/logger.go (2)

204-238: Consider documenting the error classification approach.

The error type classification using string matching works well for the current use case. However, this approach could be fragile if error messages change in the future.

Add a comment explaining the approach and potential limitations:

 // 获取错误类型
+// Note: This function uses string matching to classify errors. If error messages change,
+// the classification logic may need to be updated accordingly.
 func getErrorType(msg string) (string, string) {

171-178: Consider making the caller depth configurable.

The hardcoded caller depth of 3 assumes a specific call stack, which may not be correct for all logging paths.

-func getCallerInfo() string {
-	_, file, line, ok := runtime.Caller(3) // 增加调用栈深度到3,跳过日志函数本身
+func getCallerInfo(skip int) string {
+	_, file, line, ok := runtime.Caller(skip)
 	if !ok {
 		return "unknown:0"
 	}
 	// 返回完整路径
 	return fmt.Sprintf("%s:%d", file, line)
 }

Then update all callers to pass the appropriate skip value.

model/usedata.go (1)

190-273: Document memory implications of large page size.

Processing 100,000 records per page could consume significant memory for large datasets.

Add a comment explaining the trade-off:

 var tempBillingMap = make(map[string]*BillingData) // 用于临时存储聚合结果
+// Note: Using 100k page size for efficiency, but this may consume significant memory
+// for very large datasets. Adjust if memory issues occur.
 pageSize := 100000

Consider making the page size configurable or reducing it if memory becomes an issue.

relay/channel/volcengine/keepalive.go (3)

397-397: Use time.Since() for consistency.

For consistency with the rest of the codebase and better readability.

-		if now.Sub(keepAliveKey.LastTouch) > keepAliveKey.Expiration {
+		if time.Since(keepAliveKey.LastTouch) > keepAliveKey.Expiration {

233-241: Initialize random seed for better randomization.

The code uses rand.Intn() without initializing the random seed, which will produce the same sequence on each program start.

Add random seed initialization in the init() function or at the start of the program:

func init() {
    rand.Seed(time.Now().UnixNano())
}

Alternatively, since Go 1.20, the global random number generator is automatically seeded, so this may not be necessary if you're using Go 1.20+.


217-220: Clarify log message for better understanding.

The log message could be misleading as it suggests the time until keep-alive, but it's actually the delay before the next check round.

 			// 生成随机保活间隔时间
 			randomInterval := kam.generateRandomInterval()
-			common.LogInfo(kam.ctx, fmt.Sprintf("Next keep-alive in %v", randomInterval))
+			common.LogInfo(kam.ctx, fmt.Sprintf("Next keep-alive check round in %v", randomInterval))
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cf4700a and 94e8953.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (91)
  • .github/workflows/docker-image-amd64.yml (1 hunks)
  • .gitignore (1 hunks)
  • Dockerfile (2 hunks)
  • bin/groups.sql (1 hunks)
  • bin/log_requestid.sql (1 hunks)
  • common/constants.go (3 hunks)
  • common/init.go (1 hunks)
  • common/logger.go (4 hunks)
  • common/origin.go (1 hunks)
  • common/time.go (1 hunks)
  • controller/channel-test.go (1 hunks)
  • controller/channel.go (6 hunks)
  • controller/group.go (2 hunks)
  • controller/misc.go (1 hunks)
  • controller/option.go (2 hunks)
  • controller/playground.go (1 hunks)
  • controller/relay.go (5 hunks)
  • controller/token.go (2 hunks)
  • controller/usedata.go (2 hunks)
  • dto/claude.go (1 hunks)
  • dto/error.go (1 hunks)
  • dto/openai_request.go (6 hunks)
  • dto/realtime.go (1 hunks)
  • go.mod (3 hunks)
  • main.go (8 hunks)
  • metrics/metrics.go (1 hunks)
  • middleware/auth.go (2 hunks)
  • middleware/distributor.go (8 hunks)
  • middleware/mock.go (1 hunks)
  • middleware/request-id.go (1 hunks)
  • middleware/request-logger.go (1 hunks)
  • middleware/utils.go (1 hunks)
  • model/channel.go (2 hunks)
  • model/group.go (1 hunks)
  • model/log.go (6 hunks)
  • model/main.go (2 hunks)
  • model/option.go (3 hunks)
  • model/text_request.go (1 hunks)
  • model/token.go (4 hunks)
  • model/usedata.go (10 hunks)
  • relay/channel/adapter.go (1 hunks)
  • relay/channel/api_request.go (4 hunks)
  • relay/channel/aws/adaptor.go (3 hunks)
  • relay/channel/claude/adaptor.go (1 hunks)
  • relay/channel/claude/relay-claude.go (4 hunks)
  • relay/channel/gemini/dto.go (2 hunks)
  • relay/channel/gemini/relay-gemini.go (4 hunks)
  • relay/channel/openai/adaptor.go (3 hunks)
  • relay/channel/openai/helper.go (1 hunks)
  • relay/channel/openai/relay-openai.go (2 hunks)
  • relay/channel/task/suno/adaptor.go (2 hunks)
  • relay/channel/vertex/adaptor.go (1 hunks)
  • relay/channel/volcengine/adaptor.go (4 hunks)
  • relay/channel/volcengine/batchchat.go (1 hunks)
  • relay/channel/volcengine/keepalive.go (1 hunks)
  • relay/channel/volcengine/redis_client.go (1 hunks)
  • relay/channel/xai/adaptor.go (1 hunks)
  • relay/channel/xai/constants.go (1 hunks)
  • relay/channel/xai/dto.go (1 hunks)
  • relay/channel/xai/text.go (1 hunks)
  • relay/common/relay_info.go (6 hunks)
  • relay/constant/api_type.go (2 hunks)
  • relay/constant/relay_mode.go (1 hunks)
  • relay/helper/common.go (1 hunks)
  • relay/relay-audio.go (5 hunks)
  • relay/relay-image.go (4 hunks)
  • relay/relay-mj.go (2 hunks)
  • relay/relay-text.go (9 hunks)
  • relay/relay_adaptor.go (2 hunks)
  • relay/relay_direct.go (1 hunks)
  • relay/relay_embedding.go (5 hunks)
  • relay/relay_rerank.go (5 hunks)
  • relay/relay_task.go (2 hunks)
  • router/api-router.go (4 hunks)
  • router/relay-router.go (2 hunks)
  • service/error.go (1 hunks)
  • service/quota.go (3 hunks)
  • service/token_counter.go (5 hunks)
  • setting/operation_setting/model-ratio.go (5 hunks)
  • setting/user_usable_group.go (1 hunks)
  • web/src/components/LogsTable.js (3 hunks)
  • web/src/components/TokensTable.js (3 hunks)
  • web/src/components/UsersTable.js (3 hunks)
  • web/src/constants/channel.constants.js (1 hunks)
  • web/src/helpers/render.js (1 hunks)
  • web/src/helpers/utils.js (1 hunks)
  • web/src/pages/Channel/EditChannel.js (3 hunks)
  • web/src/pages/Detail/index.js (6 hunks)
  • web/src/pages/Token/EditToken.js (6 hunks)
  • web/src/pages/User/EditUser.js (1 hunks)
  • web/vite.config.js (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
relay/channel/gemini/dto.go (1)
Learnt from: 9Ninety
PR: QuantumNous/new-api#1273
File: relay/channel/gemini/relay-gemini.go:97-116
Timestamp: 2025-06-21T03:37:41.688Z
Learning: In relay/channel/gemini/relay-gemini.go, the thinking budget calculation logic (including the MaxOutputTokens multiplication) was introduced in PR #1247. PR #1273 focused specifically on decoupling the thoughts summary feature from thinking budget settings and did not modify the existing thinking budget behavior.
relay/channel/gemini/relay-gemini.go (1)
Learnt from: 9Ninety
PR: QuantumNous/new-api#1273
File: relay/channel/gemini/relay-gemini.go:97-116
Timestamp: 2025-06-21T03:37:41.688Z
Learning: In relay/channel/gemini/relay-gemini.go, the thinking budget calculation logic (including the MaxOutputTokens multiplication) was introduced in PR #1247. PR #1273 focused specifically on decoupling the thoughts summary feature from thinking budget settings and did not modify the existing thinking budget behavior.
🪛 Hadolint (2.12.0)
Dockerfile

[error] 19-19: Use COPY instead of ADD for files and folders

(DL3020)

🪛 actionlint (1.7.7)
.github/workflows/docker-image-amd64.yml

26-26: the runner of "docker/login-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🪛 YAMLlint (1.37.1)
.github/workflows/docker-image-amd64.yml

[error] 40-40: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (122)
relay/helper/common.go (1)

10-12: LGTM! Import ordering follows Go conventions.

The reordering of imports to place external packages after standard library and internal imports aligns with Go best practices and improves code organization.

web/vite.config.js (1)

55-56: Verify security implications of binding to all interfaces.

Setting host: '0.0.0.0' exposes the development server to external networks, and port 80 typically requires elevated privileges. Ensure this configuration is appropriate for your deployment environment and doesn't introduce security risks in development.

Consider whether this should be conditional based on environment or if additional security measures are needed.

relay/channel/adapter.go (1)

8-9: LGTM! Consistent import ordering across the codebase.

The import reordering aligns with Go best practices and maintains consistency with similar changes made throughout the relay channel adaptors.

web/src/helpers/render.js (1)

209-209: Minor formatting change - no functional impact.

The trailing space addition is a trivial formatting change that doesn't affect the function's behavior.

dto/realtime.go (1)

54-57: LGTM! Clean addition of reasoning tokens tracking.

The new ReasoningTokens field follows the established naming conventions and JSON tag patterns consistently with the existing TextTokens and AudioTokens fields.

web/src/constants/channel.constants.js (1)

112-116: LGTM! New channel option follows established conventions.

The new "豆包离线" channel option maintains consistency with the existing array structure and naming patterns.

relay/constant/relay_mode.go (1)

47-47: Verify the new API path /v1/messages is legitimate.

The addition of /v1/messages to the chat completions relay mode follows the existing pattern correctly. However, please verify that this is a documented and legitimate API endpoint that should be treated as chat completions.

#!/bin/bash
# Description: Search for usage and documentation of the /v1/messages API path
# Expected: Find references to this new API endpoint in the codebase

# Search for references to /v1/messages in the codebase
rg -A 3 -B 3 "/v1/messages"
controller/channel-test.go (1)

162-163: Verify all callers of model.RecordConsumeLog are updated consistently.

The addition of a new parameter (hardcoded to 0) to model.RecordConsumeLog suggests the function signature was extended. Please ensure all other callers throughout the codebase have been updated to match this new signature to prevent compilation errors.

#!/bin/bash
# Description: Find all calls to RecordConsumeLog and verify they use the correct parameter count
# Expected: All calls should have the same parameter pattern

# Search for all RecordConsumeLog function calls
rg -A 5 "RecordConsumeLog\(" --type go

# Search for the function definition to understand the new signature
ast-grep --pattern 'func RecordConsumeLog($$$) $$$'
common/init.go (1)

70-72: Good implementation of the mock response feature flag.

The implementation correctly reads the environment variable, provides appropriate defaults, and includes helpful logging. This follows good practices for feature flag management.

#!/bin/bash
# Description: Verify that MockResponseEnabled variable is properly declared
# Expected: Find the variable declaration in the common package

# Search for MockResponseEnabled variable declaration
rg -A 2 -B 2 "MockResponseEnabled.*=" --type go

# Search for MockResponseEnabled usage in other files
rg "MockResponseEnabled" --type go
model/main.go (2)

10-14: Good import organization following Go conventions.

The reordering of imports to place database drivers after standard and local imports follows Go best practices and improves code organization.


217-220: Proper implementation of Group model migration.

The Group model migration follows the established pattern with appropriate error handling and maintains consistency with other model migrations.

#!/bin/bash
# Description: Verify that the Group struct is properly defined
# Expected: Find the Group struct definition with appropriate GORM tags

# Search for Group struct definition
ast-grep --pattern 'type Group struct {
  $$$
}'

# Search for Group model file
fd -t f "group.go" --exec cat {}
middleware/auth.go (2)

4-4: LGTM: Proper import organization.

The import statements have been properly reordered to follow Go conventions, placing standard library imports before third-party packages.

Also applies to: 10-13


122-123: LGTM: Authentication audit logging added.

The new logging statement appropriately captures authentication events for security auditing. The placement after validation but before context setting is correct.

relay/relay_task.go (2)

19-20: LGTM: Import reordering follows conventions.

The import statement has been properly reordered to place third-party packages at the end.


127-127: Verify function signature compatibility.

An additional zero parameter was added to the model.RecordConsumeLog call. Please ensure this aligns with the updated function signature and that all call sites have been consistently updated.

#!/bin/bash
# Description: Verify all RecordConsumeLog function calls match the new signature
# Expected: All calls should have the same number of parameters

echo "Searching for RecordConsumeLog function definition:"
ast-grep --pattern 'func RecordConsumeLog($$$) $$$'

echo -e "\nSearching for all RecordConsumeLog function calls:"
rg -A 3 "RecordConsumeLog\(" --type go
router/relay-router.go (3)

7-8: LGTM: Import reordering follows conventions.

The import statement has been properly reordered to place third-party packages after local packages.


15-16: LGTM: Health check endpoint added.

The /ping route is appropriately added as a public endpoint for health checking purposes.


67-67: LGTM: Messages relay endpoint added.

The /messages route is properly configured with appropriate middleware (TokenAuth and Distribute) consistent with other relay endpoints.

relay/relay_adaptor.go (2)

29-29: LGTM: XAI package import added.

The import for the new XAI channel package is properly added.


90-93: LGTM: New API type support added.

The two new cases properly extend the adaptor selection:

  • APITypeXai returns a new xai.Adaptor instance for XAI channel support
  • APITypeDoubaoOffline reuses the volcengine.Adaptor, which is reasonable if it's a variant of the VolcEngine API

The implementation follows the established pattern in the switch statement.

bin/groups.sql (1)

1-7: Well-designed database schema for groups management.

The table structure is solid with appropriate constraints:

  • IF NOT EXISTS prevents deployment conflicts
  • Unique constraint on name ensures data integrity
  • UTF8MB4 character set supports full Unicode
  • InnoDB storage engine provides ACID compliance
model/group.go (1)

3-7: Clean model definition with proper ORM mapping.

The Group struct correctly maps to the database schema with:

  • Appropriate GORM tags matching the SQL constraints
  • JSON serialization tags following Go conventions
  • Field types aligned with database column types
web/src/components/TokensTable.js (3)

8-8: Good addition of role-based helper import.

The isRoot import supports the new role-based functionality for token display.


85-89: Enhanced token visibility with creator information.

Adding the creator column provides better administrative oversight for token management.


356-359: Proper role-based API endpoint selection.

The conditional logic appropriately separates admin and user token access:

  • Root users access /api/alltoken/ for comprehensive token management
  • Regular users access /api/token/ for their own tokens only

Verify that the backend properly enforces these access restrictions:

#!/bin/bash
# Description: Verify backend authorization for token endpoints
# Expected: Should find proper authentication/authorization middleware

# Check for alltoken endpoint definition and its middleware
rg -A 10 -B 5 "/api/alltoken|RootGetAllTokens" --type go

# Check for role-based filtering in token controllers
rg -A 15 -B 5 "role.*100|GetAllTokens|isRoot" --type go
relay/channel/task/suno/adaptor.go (1)

68-75: LGTM! Clean retry header propagation implementation.

The retry header forwarding logic is well-implemented and consistent with the broader pattern of header propagation across channel adaptors mentioned in the AI summary.

model/channel.go (1)

25-25: Well-implemented field addition.

The new Endpoint field follows proper Go conventions with appropriate JSON and GORM tags.

web/src/pages/User/EditUser.js (1)

49-74: Excellent security enhancement with proper error handling.

The role-based group filtering is well-implemented:

  • Proper API response validation with success/message/data destructuring
  • Non-super-admin users (role < 100) only see groups containing their username
  • Robust error handling for both API failures and success responses

This aligns well with the backend role-based filtering mentioned in the AI summary.

relay/channel/claude/adaptor.go (1)

70-70: LGTM! Function signature update.

The addition of the gin context parameter to RequestOpenAI2ClaudeMessage aligns with the enhanced functionality mentioned in the AI summary for handling user overrides and thinking token budgets.

relay/channel/xai/constants.go (1)

1-19: LGTM! Clean constants definition for XAI channel.

The model list is well-organized with clear categorization and the channel name follows the established naming convention.

relay/constant/api_type.go (2)

34-35: LGTM! Proper addition of new API type constants.

The constants follow the established naming convention and are correctly positioned in the sequence.


94-97: LGTM! Correct channel type to API type mapping.

The mapping appropriately assigns XAI to its own API type while reusing the VolcEngine adaptor for Doubao offline, which is sensible if they share the same API interface.

web/src/components/LogsTable.js (3)

5-5: LGTM! Appropriate import addition.

Adding getTodayEndTimestamp import to support improved date handling.


502-502: LGTM! Better default timestamp handling.

Using getTodayEndTimestamp() provides a more intuitive default end time (end of current day) for the log query range.


723-728: LGTM! Excellent defensive programming.

The guard clause properly handles cases where the API returns empty data, preventing unnecessary processing and ensuring the loading state is properly reset.

model/option.go (2)

4-4: LGTM! Required import for JSON operations.

The encoding/json import is needed for the new group synchronization logic.


26-37: LGTM! Proper initialization of groups from database.

The function correctly uses mutex locking and loads group data into the global map. Using panic for initialization errors is appropriate as this is a critical startup operation.

relay/channel/vertex/adaptor.go (1)

127-127: LGTM! Proper context propagation.

Adding the context parameter c to claude.RequestOpenAI2ClaudeMessage aligns with the broader pattern of enabling context-aware request processing across relay channel adaptors.

web/src/pages/Channel/EditChannel.js (2)

80-80: LGTM: New endpoint field added correctly.

The endpoint field is properly initialized as an empty string in the originInputs object, consistent with other form fields.


560-578: LGTM: Doubao offline channel UI implementation looks good.

The conditional UI block for channel type 100 is well-implemented:

  • Proper label and tooltip for user guidance
  • Consistent styling with other input sections
  • Correct binding to the endpoint state field
  • Helpful placeholder text with example format
relay/channel/gemini/dto.go (3)

83-84: LGTM: ThinkingConfig field properly defined.

The ThinkingConfig field is correctly defined as an optional pointer type with proper JSON tag, consistent with other optional configuration fields in the struct.


86-88: LGTM: GeminiChatThinkingConfig struct well-defined.

The struct is simple and focused, with appropriate JSON tag for the ThinkingBudget field. The integer type is suitable for token budget counts.


116-116: LGTM: ThoughtsTokenCount field added correctly.

The field is properly typed as int and has the correct JSON tag, consistent with other token count fields in the GeminiUsageMetadata struct.

controller/group.go (1)

7-7: LGTM: Appropriate import addition.

The strings package import is correctly added to support the new filtering logic.

controller/token.go (2)

8-8: LGTM: Appropriate import addition.

The strings package import is correctly added for the filtering logic.


55-80: Verify access control for the new RootGetAllTokens endpoint.

The new RootGetAllTokens function provides unrestricted access to all tokens but lacks explicit role validation within the function. Ensure that the route definition properly restricts this endpoint to super administrators only.

#!/bin/bash
# Verify that RootGetAllTokens is properly protected by admin middleware
echo "Searching for RootGetAllTokens route definition and middleware..."
rg -A 5 -B 5 "RootGetAllTokens"
common/origin.go (2)

9-17: LGTM: GetOriginUserId implementation is robust.

The function correctly:

  • Checks for header existence before processing
  • Handles conversion errors gracefully
  • Falls back to the default value on any failure
  • Uses appropriate header name convention

19-27: LGTM: GetOriginChannelId follows the same solid pattern.

The function is consistent with GetOriginUserId in its implementation approach, error handling, and fallback behavior. The code is clean and well-structured.

relay/relay-mj.go (1)

211-212: Verify the new RecordConsumeLog parameter.

An additional 0 parameter was added to model.RecordConsumeLog calls. Without seeing the function signature, it's unclear what this parameter represents and whether 0 is the appropriate default value.

Please verify that 0 is the correct default value for the new parameter in model.RecordConsumeLog. Run this script to check the function signature:

#!/bin/bash
# Search for RecordConsumeLog function definition to understand the new parameter
ast-grep --pattern 'func RecordConsumeLog($$$) $$$'

Also applies to: 513-514

middleware/request-id.go (2)

16-25: LGTM: Improved request ID handling with existing header check.

The enhancement to check for existing request IDs before generating new ones is a good optimization and prevents ID conflicts in downstream processing.


27-42: LGTM: Secure request ID generation with appropriate trade-offs.

The implementation correctly uses cryptographically secure random data and nanosecond timestamps for uniqueness. While MD5 is not cryptographically secure for sensitive data, it's perfectly acceptable for generating request IDs where the primary concern is uniqueness rather than security.

relay/channel/openai/relay-openai.go (3)

286-288: LGTM: Enhanced observability with detailed logging.

The addition of response headers and usage data logging provides valuable debugging information for monitoring OpenAI API interactions.


306-317: Good Content-Length validation with proper logging.

The Content-Length header validation and correction logic is well-implemented. It properly logs discrepancies and updates the header to match the actual response body length, which helps prevent client-side parsing issues.


317-317: Consider memory usage implications of response handling change.

The change from io.Copy(c.Writer, resp.Body) to io.Copy(c.Writer, bytes.NewReader(responseBody)) means the entire response is now held in memory. For large responses, this could increase memory usage.

However, since the response body was already read into responseBody for parsing earlier in the function, this change doesn't actually increase memory usage and may be more efficient for small to medium responses.

service/quota.go (4)

18-18: LGTM: Import addition for concurrent processing.

The gopool import supports the asynchronous notification functionality later in the file.


180-184: Excellent test traffic handling for billing bypass.

The early exit for test traffic (identified by X-Test-Traffic: true header) is a smart approach to prevent test requests from affecting billing and quota consumption. This aligns with best practices for testing in production-like environments.


174-175: LGTM: Enhanced token tracking with reasoning tokens.

The addition of usage.OutputTokenDetails.ReasoningTokens to the RecordConsumeLog call provides more granular token usage tracking, which is valuable for billing accuracy and analytics.


253-254: Consistent reasoning token tracking in audio quota consumption.

Good consistency in adding reasoning token tracking across different quota consumption functions.

middleware/utils.go (3)

17-24: LGTM: Proper request body capture and restoration.

The implementation correctly reads the request body and restores it using io.NopCloser(bytes.NewBuffer(requestBody)), ensuring that downstream middleware can still access the request body.


26-40: Excellent JSON formatting with proper HTML escaping.

The JSON encoding setup with SetEscapeHTML(false) and SetIndent("", "") ensures that non-ASCII characters (like Chinese) are preserved correctly while maintaining compact formatting. This is particularly important for international applications.


42-54: Comprehensive error logging with request context.

The enhanced logging provides excellent debugging information by including:

  • User ID for request tracking
  • Original error message
  • Formatted request body (when valid JSON)
  • Formatted response body

This significantly improves troubleshooting capabilities.

controller/channel.go (1)

53-56: LGTM: Proper user context extraction for access control.

The extraction of user role and username from the Gin context is implemented correctly for role-based access control.

router/api-router.go (3)

16-16: LGTM: Health check endpoint appropriately public.

The /ping endpoint is correctly implemented as a public health check endpoint, which is a standard practice for monitoring and load balancing.


81-81: LGTM: Request logging toggle properly secured.

The /request_log endpoint is correctly protected with RootAuth() middleware, ensuring only super administrators can toggle request logging functionality.


118-122: LGTM: Root token access properly secured.

The /alltoken route group is correctly protected with RootAuth() middleware, ensuring only super administrators can access all tokens regardless of group restrictions.

web/src/components/UsersTable.js (2)

267-273: LGTM: Consistent role-based filtering implementation.

The role-based filtering correctly restricts non-super administrators (role < 100) to their own group data, maintaining proper access control.


347-351: LGTM: Proper group filtering logic in search.

The search function correctly applies group filtering with proper precedence - explicit group selection takes priority over default user group filtering.

relay/channel/gemini/relay-gemini.go (5)

32-39: LGTM: Proper null-safe ThinkingConfig implementation.

The conditional ThinkingConfig creation correctly handles the case where textRequest.Thinking is nil, preventing null pointer exceptions.


215-226: LGTM: Well-implemented audio content processing.

The audio content handling includes appropriate debug logging and correctly converts the audio data to Gemini's InlineData format with proper MIME type handling.


583-583: LGTM: Enhanced error logging for better debugging.

The addition of raw response body logging when no candidates are returned will significantly improve debugging capabilities for failed requests.


601-601: LGTM: Proper reasoning tokens integration.

The ReasoningTokens assignment correctly integrates with the existing usage tracking system, providing better token accounting for thinking-enabled models.


227-233: Clarify YouTube content MIME type handling.

The YouTube content processing uses part.Text for the MIME type, which seems unusual. Please verify this is the intended design.

#!/bin/bash
# Search for YouTube content type usage patterns to verify the design
ast-grep --pattern 'ContentTypeYoutube'
rg -A 5 -B 5 "ContentTypeYoutube"
relay/common/relay_info.go (2)

23-24: LGTM: Well-structured field additions to RelayInfo.

The new fields ChannelTag, ChannelName, Endpoint, Direct, RetryCount, and Headers are appropriately typed and positioned within the struct, enhancing relay metadata tracking capabilities.

Also applies to: 48-48, 64-66


95-96: LGTM: Proper field initialization in GenRelayInfo.

All new fields are correctly populated from the Gin context, and the Headers map is properly initialized as an empty map to prevent nil pointer issues.

Also applies to: 114-114, 118-119, 136-136

middleware/request-logger.go (2)

23-27: LGTM: Proper sensitive header masking.

The middleware correctly masks Authorization and Cookie headers to prevent logging sensitive authentication data.


108-109: LGTM: Proper newline character handling.

The middleware correctly removes newline characters from log output to maintain clean, single-line log entries.

Also applies to: 121-121, 135-135

model/token.go (2)

19-19: LGTM: User field addition enhances token metadata.

The addition of the User field to the Token struct enables better token management and display of token ownership information.


143-143: Verify removal of user authorization in GetTokenByIds.

The function still accepts userId parameter but no longer uses it for filtering tokens. This removes user-level authorization checks.

#!/bin/bash
# Description: Check how GetTokenByIds is used in the codebase to verify if removing user filtering is safe
# Expected: Function should only be called by admin/root users or the filtering should be restored

echo "=== Searching for GetTokenByIds usage ==="
rg -A 3 -B 2 "GetTokenByIds"

echo "=== Searching for related token access patterns ==="
rg -A 5 "GetTokenBy.*userId"
common/constants.go (3)

29-29: LGTM: MockResponseEnabled flag for test traffic handling.

This boolean flag appropriately enables mock responses for test traffic, supporting the development and testing workflow.


37-38: LGTM: Thread-safe group management with proper synchronization.

The Groups map with RWMutex provides thread-safe access to group data, which is essential for concurrent operations.


241-242: LGTM: New channel type constants follow existing pattern.

The addition of ChannelTypeXai (47) and ChannelTypeDoubaoOffline (100) follows the established numbering scheme and naming convention.

relay/channel/aws/adaptor.go (2)

45-51: LGTM: Proper retry header propagation enhances request tracking.

The addition of retry_request_id and retry header propagation from the original request to the outgoing request enables consistent retry tracking and debugging.


63-63: LGTM: Context-aware request conversion improves functionality.

Passing the Gin context to claude.RequestOpenAI2ClaudeMessage enables context-aware request conversion, supporting enhanced logging and request-scoped operations.

relay/channel/openai/adaptor.go (2)

80-86: LGTM: User and channel ID headers enhance observability.

Adding X-User-ID and X-Channel-ID headers when the respective IDs are non-zero provides valuable context for downstream services and debugging without exposing sensitive information.


149-152: LGTM: Thinking disable option follows established pattern.

The "-disable" suffix handling for the Thinking field follows the same pattern as the existing reasoning effort suffixes (high, low, medium), providing consistent model customization capabilities.

service/token_counter.go (3)

37-37: LGTM: Cleaner iteration syntax improves readability.

The change from for model, _ := range to for model := range when the value is not used is more idiomatic Go code.


166-169: LGTM: Essential nil check prevents runtime panics.

Adding a nil check for request.Messages with a descriptive error message is a crucial defensive programming practice that prevents potential runtime panics.


266-266: LGTM: Context-aware logging enhances observability.

Adding Gin context parameters to token counting functions and using common.LogInfo(ctx, ...) instead of standard logging provides better request tracing and context-aware logging capabilities.

Also applies to: 301-301

go.mod (1)

6-39: LGTM! Dependencies align with new features.

The addition of Prometheus client for metrics, Volcengine SDK for batch chat, and Excelize for Excel exports are appropriate for the described feature enhancements.

web/src/pages/Token/EditToken.js (2)

97-117: Good error handling improvement!

The refactoring from loadGroups to fetchGroups with try-catch error handling is a solid improvement.


208-213: Excellent validation addition!

Adding required group validation before token creation prevents invalid tokens from being created.

relay/channel/volcengine/adaptor.go (1)

63-66: Good defensive programming and routing logic!

The safe handling of the Thinking field and case-insensitive batch model detection are well implemented.

Also applies to: 79-82

main.go (1)

42-60: Well-structured initialization sequence!

The command-line flag support, logging configuration, persistence initialization, and other setup steps are properly organized and follow a logical sequence.

Also applies to: 65-81, 116-124, 140-148, 154-157

relay/relay-audio.go (1)

61-171: Excellent refactoring with comprehensive metrics!

The separation of AudioInfo and AudioHelper improves modularity, and the metrics instrumentation provides great observability with proper error tracking and deferred recording.

dto/openai_request.go (1)

293-323: Well-implemented content parsing enhancements!

The audio content parsing enhancement to support mime_type and the new YouTube content type parsing are properly implemented with appropriate error handling.

middleware/distributor.go (2)

172-203: Excellent request body handling implementation!

The enhanced request body handling properly reads, resets, and processes empty bodies with appropriate default models for different endpoints.


49-57: Good consistency improvement using group IDs.

The change from group names to group IDs in error messages provides better consistency and clarity.

Also applies to: 104-118

model/text_request.go (1)

25-35: Well-structured table design for request persistence!

The TextRequest struct is well-designed with appropriate field types, indexes, and size considerations for different text fields.

relay/relay-image.go (4)

12-21: LGTM!

The added imports are appropriate for the metrics instrumentation functionality.


77-86: Good separation of concerns!

The ImageInfo function cleanly separates request parsing and validation from processing, following the same pattern as other relay files.


88-100: Excellent metrics instrumentation!

The deferred metrics recording ensures accurate tracking of all requests, failures, and durations. The use of funcErr to track errors throughout the function is a clean pattern.


203-205: LGTM!

Status code extraction for metrics recording is properly implemented.

relay/relay_embedding.go (2)

51-63: LGTM!

Metrics instrumentation follows the established pattern consistently.


145-147: LGTM!

Status code extraction is consistent with other relay handlers.

relay/channel/claude/relay-claude.go (3)

63-63: Good addition of context parameter!

Adding the gin.Context parameter enables contextual logging, which improves debugging and monitoring capabilities.


108-122: Well-implemented thinking budget override!

The implementation properly validates the minimum budget requirement (1024 tokens) and provides clear logging for both user-defined and system-calculated values.


726-736: Verify the empty JSON schema for claude-3 models

The response format creates a JSON object schema with empty properties. This seems incomplete and may not achieve the intended behavior.

Could you verify if this empty schema is intentional? If claude-3 models require specific response formatting, the schema should include the expected structure.

 if strings.Contains(claudeReq.Model, "claude-3") {
 	openaiReq.ResponseFormat = &dto.ResponseFormat{
 		Type: "json_object",
 		JsonSchema: &dto.FormatJsonSchema{
 			Schema: map[string]interface{}{
 				"type":       "object",
-				"properties": make(map[string]interface{}),
+				// TODO: Define the actual expected JSON schema properties
+				"properties": map[string]interface{}{
+					// Add required properties here
+				},
 			},
 		},
 	}
 }
relay/relay_rerank.go (1)

43-143: LGTM!

The refactoring maintains consistency with other relay handlers and properly implements metrics tracking.

web/src/pages/Detail/index.js (3)

35-43: Good improvement to default timestamps!

Setting the default time range to the full current day (00:00:00 to 23:59:59) provides a more intuitive default for users.


199-206: Good validation logic!

Requiring a username when a token name is specified ensures proper data filtering since tokens are associated with users.


244-279: Well-implemented billing export!

The export function properly handles the file download with blob response, filename extraction from headers, and appropriate error handling.

relay/channel/xai/text.go (1)

54-59: Verify completion tokens calculation logic

The completion tokens calculation assumes that TotalTokens = PromptTokens + CompletionTokens, but this should be verified against XAI's API documentation.

What is the correct formula for calculating completion tokens from XAI API responses, specifically the relationship between total_tokens, prompt_tokens, and completion_tokens?
common/logger.go (5)

99-146: LGTM! Well-implemented log rotation with proper thread safety.

The custom logWriter implementation correctly handles file size tracking and rotation with mutex protection. The atomic update of gin's default writers ensures thread safety during rotation.


148-169: Clean and efficient old log cleanup implementation.

The function correctly sorts log files by modification time (newest first) and removes old files while keeping the most recent ones. Error handling is appropriate with non-fatal logging of removal failures.


267-295: Excellent metrics integration with comprehensive context extraction.

The error logging properly extracts all relevant context information (channel, model, group, token name) from the gin context and passes it to the metrics system with appropriate defaults.


247-256: ```shell
#!/bin/bash

Search for any occurrences of 'hash_value' in the repository

rg -n 'hash_value'

Search for any gin.Context Set calls referencing 'hash_value'

rg -n 'Set.*hash_value'


---

`252-254`: **Log sampling logic appears to be inverted.**

If `LogSampleRatio` represents the percentage of logs to keep (0-100), the condition should be `<=` not `>`. Currently, a ratio of 10 would keep 90% of logs instead of 10%.

```diff
-			if hashValue > int(LogSampleRatio) {
+			if hashValue > 100 || hashValue > int(LogSampleRatio) {
 				return
 			}

Likely an incorrect or invalid review comment.

model/log.go (2)

144-148: Good implementation of test traffic filtering.

The check for test traffic using the X-Test-Traffic header effectively prevents test requests from being recorded in billing logs.


441-463: Well-implemented helper function for multi-table queries.

The getTableNamesByTimeRange function correctly generates table names for the given time range, handling both single-day and multi-day scenarios efficiently.

model/usedata.go (1)

356-419: Well-implemented Excel export with proper formatting and grouping.

The Excel export functionality correctly implements:

  • Proper column headers and widths
  • Channel-based grouping with subtotals
  • Visual styling for subtotal rows
  • Efficient buffer-based file generation

The error handling is appropriate as style creation errors are properly returned.

metrics/metrics.go (1)

1-216: Excellent Prometheus metrics implementation.

The metrics package is well-structured with:

  • Proper namespace usage ("new_api")
  • Comprehensive metric coverage for all aspects of the system
  • Consistent label usage across metrics
  • Clean helper functions for metric operations
  • Appropriate metric types (counters for counts, histograms for durations)

The exponential buckets for duration histograms (1, 2, 4, 8, ... up to 4096ms) provide good granularity for typical API response times.

dto/claude.go (1)

1-219: Well-designed DTOs for Claude AI integration.

The Claude DTO implementation demonstrates excellent API design:

  • Comprehensive structures covering all Claude API entities
  • Proper use of pointers for nullable fields
  • Convenient helper methods for content manipulation (GetText, SetText, etc.)
  • Type-safe content parsing with appropriate error handling
  • Clean JSON marshaling/unmarshaling support

The design provides a good balance between type safety and flexibility for handling Claude's various content types.

Comment on lines +54 to +60
func GetGroupId(groupName string) int {
id, ok := common.Groups[groupName]
if !ok {
return -1
}
return id
}

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

Add synchronization for thread-safe map access.

The function accesses the global common.Groups map without synchronization, which could lead to race conditions if the map is modified concurrently. Based on the AI summary mentioning GroupRWMutex, consider adding proper read locking.

+import "one-api/common"

 func GetGroupId(groupName string) int {
+	common.GroupRWMutex.RLock()
+	defer common.GroupRWMutex.RUnlock()
 	id, ok := common.Groups[groupName]
 	if !ok {
 		return -1
 	}
 	return id
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In setting/user_usable_group.go around lines 54 to 60, the function GetGroupId
accesses the global common.Groups map without synchronization, risking race
conditions. To fix this, wrap the map access with a read lock using
GroupRWMutex.RLock() before accessing the map and defer GroupRWMutex.RUnlock()
immediately after acquiring the lock to ensure thread-safe read access.

Comment thread web/src/helpers/utils.js
Comment on lines +159 to +163
export function getTodayEndTimestamp() {
var now = new Date();
now.setHours(23, 59, 59, 59);
return Math.floor(now.getTime() / 1000);
}

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

Fix the milliseconds parameter for accurate end-of-day timestamp.

The function has a bug in the setHours call. The fourth parameter should be milliseconds (0-999), not 59.

 export function getTodayEndTimestamp() {
   var now = new Date();
-  now.setHours(23, 59, 59, 59);
+  now.setHours(23, 59, 59, 999);
   return Math.floor(now.getTime() / 1000);
 }

This ensures the timestamp represents the very end of the day (23:59:59.999) rather than 23:59:59.059.

📝 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
export function getTodayEndTimestamp() {
var now = new Date();
now.setHours(23, 59, 59, 59);
return Math.floor(now.getTime() / 1000);
}
export function getTodayEndTimestamp() {
var now = new Date();
now.setHours(23, 59, 59, 999);
return Math.floor(now.getTime() / 1000);
}
🤖 Prompt for AI Agents
In web/src/helpers/utils.js around lines 159 to 163, the setHours call
incorrectly sets the milliseconds parameter to 59, which is invalid. Change the
fourth argument of setHours to 999 to correctly represent the end of the day as
23:59:59.999 for an accurate timestamp.

Comment thread controller/option.go
Comment on lines +112 to +133
// ToggleRequestLog 切换请求体日志的开关状态
func ToggleRequestLog(c *gin.Context) {
var request struct {
Enable bool `json:"enable"`
}
if err := c.ShouldBindJSON(&request); err != nil {
c.JSON(200, gin.H{
"success": false,
"message": "无效的请求参数",
})
return
}

middleware.EnableRequestBodyLogging = request.Enable
c.JSON(200, gin.H{
"success": true,
"message": "请求体日志状态已更新",
"data": gin.H{
"enable": middleware.EnableRequestBodyLogging,
},
})
}

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.

🛠️ Refactor suggestion

🧩 Verification correct

Consider thread safety and architecture improvements for global state management.

Several concerns with the current implementation:

  1. Global State Mutation: Direct modification of middleware.EnableRequestBodyLogging without synchronization could lead to race conditions
  2. Tight Coupling: Controller directly manipulates middleware state
  3. Naming: Function name suggests toggle behavior but actually sets a specific value

Consider these improvements:

// Use atomic operations for thread safety
+import "sync/atomic"

func ToggleRequestLog(c *gin.Context) {
	var request struct {
		Enable bool `json:"enable"`
	}
	if err := c.ShouldBindJSON(&request); err != nil {
		c.JSON(200, gin.H{
			"success": false,
			"message": "无效的请求参数",
		})
		return
	}

-	middleware.EnableRequestBodyLogging = request.Enable
+	// Use atomic operation or proper state management
+	atomic.StoreBool(&middleware.EnableRequestBodyLogging, request.Enable)
	
	c.JSON(200, gin.H{
		"success": true,
		"message": "请求体日志状态已更新",
		"data": gin.H{
-			"enable": middleware.EnableRequestBodyLogging,
+			"enable": atomic.LoadBool(&middleware.EnableRequestBodyLogging),
		},
	})
}

Verify if this endpoint has proper authentication/authorization middleware applied:


🏁 Script executed:

#!/bin/bash
# Description: Check if the ToggleRequestLog endpoint has authentication middleware
# Expected: Should find route registration with auth middleware

rg -A 10 -B 5 "ToggleRequestLog|request_log" --type go

Length of output: 1768


Consider thread safety and architecture improvements for global state management.

Several concerns with the current implementation:

  1. Global State Mutation: Direct modification of middleware.EnableRequestBodyLogging without synchronization could lead to race conditions
  2. Tight Coupling: Controller directly manipulates middleware state
  3. Naming: Function name suggests toggle behavior but actually sets a specific value

Consider these improvements:

// Use atomic operations for thread safety
+import "sync/atomic"

func ToggleRequestLog(c *gin.Context) {
	var request struct {
		Enable bool `json:"enable"`
	}
	if err := c.ShouldBindJSON(&request); err != nil {
		c.JSON(200, gin.H{
			"success": false,
			"message": "无效的请求参数",
		})
		return
	}

-	middleware.EnableRequestBodyLogging = request.Enable
+	// Use atomic operation or proper state management
+	atomic.StoreBool(&middleware.EnableRequestBodyLogging, request.Enable)
	
	c.JSON(200, gin.H{
		"success": true,
		"message": "请求体日志状态已更新",
		"data": gin.H{
-			"enable": middleware.EnableRequestBodyLogging,
+			"enable": atomic.LoadBool(&middleware.EnableRequestBodyLogging),
		},
	})
}

Verify if this endpoint has proper authentication/authorization middleware applied:

#!/bin/bash
# Description: Check if the ToggleRequestLog endpoint has authentication middleware
# Expected: Should find route registration with auth middleware

rg -A 10 -B 5 "ToggleRequestLog|request_log" --type go
🤖 Prompt for AI Agents
In controller/option.go lines 112 to 133, the function ToggleRequestLog directly
modifies the global variable middleware.EnableRequestBodyLogging without
synchronization, risking race conditions, and tightly couples the controller to
middleware internals. Rename the function to reflect that it sets the logging
state rather than toggling it. Refactor to encapsulate the state change behind a
thread-safe method in the middleware package that uses synchronization
primitives like mutexes. Update the controller to call this method instead of
directly modifying the variable. Additionally, verify that this endpoint is
protected by appropriate authentication and authorization middleware to prevent
unauthorized access.

Comment thread middleware/mock.go
Comment on lines +33 to +36
"usage": map[string]interface{}{
"prompt_tokens": 6,
"completion_tokens": 7,
"total_tokens": 154,

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

Fix inconsistent token calculations.

The usage statistics show prompt_tokens: 6 and completion_tokens: 7, but total_tokens: 154. The math doesn't add up (6 + 7 = 13, not 154).

 "usage": map[string]interface{}{
 	"prompt_tokens":     6,
 	"completion_tokens": 7,
-	"total_tokens":      154,
+	"total_tokens":      13,
📝 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
"usage": map[string]interface{}{
"prompt_tokens": 6,
"completion_tokens": 7,
"total_tokens": 154,
"usage": map[string]interface{}{
"prompt_tokens": 6,
"completion_tokens": 7,
"total_tokens": 13,
🤖 Prompt for AI Agents
In middleware/mock.go around lines 33 to 36, the total_tokens value is
inconsistent with the sum of prompt_tokens and completion_tokens. Update
total_tokens to be the correct sum of prompt_tokens and completion_tokens, which
is 13, to fix the token calculation inconsistency.

Comment thread model/option.go
Comment on lines +178 to +202
if key == "GroupRatio" {
groups := make(map[string]int)
err := json.Unmarshal([]byte(option.Value), &groups)
if err != nil {
return err
}

// 将 GroupRatio 数据同步到 group 表中
for groupName, ratio := range groups {
// 查询是否存在该记录
var count int64
DB.Table("groups").Where("name = ?", groupName).Count(&count)

if count > 0 {
// 存在记录,更新 ratio
DB.Table("groups").Where("name = ?", groupName).Update("ratio", ratio)
} else {
// 不存在记录,创建新记录
DB.Table("groups").Create(map[string]interface{}{
"name": groupName,
"ratio": ratio,
})
}
}
}

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.

🛠️ Refactor suggestion

Improve database operations for better reliability and consistency.

The group synchronization logic has several concerns:

  1. No error handling: Database operations lack error checking
  2. No transaction: Multiple database operations should be wrapped in a transaction for atomicity
  3. Race condition potential: Separate count and update/create operations could have timing issues
  4. Raw SQL vs GORM models: Consider using GORM models for consistency

Consider this improved approach:

if key == "GroupRatio" {
	groups := make(map[string]int)
	err := json.Unmarshal([]byte(option.Value), &groups)
	if err != nil {
		return err
	}

+	// Use transaction for atomicity
+	tx := DB.Begin()
+	defer func() {
+		if r := recover(); r != nil {
+			tx.Rollback()
+		}
+	}()

	// Sync GroupRatio data to groups table
	for groupName, ratio := range groups {
-		// Check if record exists
-		var count int64
-		DB.Table("groups").Where("name = ?", groupName).Count(&count)
-
-		if count > 0 {
-			// Update existing record
-			DB.Table("groups").Where("name = ?", groupName).Update("ratio", ratio)
-		} else {
-			// Create new record
-			DB.Table("groups").Create(map[string]interface{}{
-				"name":  groupName,
-				"ratio": ratio,
-			})
-		}
+		// Use GORM's upsert functionality
+		group := Group{Name: groupName, Ratio: ratio}
+		if err := tx.Where("name = ?", groupName).FirstOrCreate(&group).Error; err != nil {
+			tx.Rollback()
+			return err
+		}
+		if err := tx.Model(&group).Update("ratio", ratio).Error; err != nil {
+			tx.Rollback()
+			return err
+		}
	}
+
+	if err := tx.Commit().Error; err != nil {
+		return err
+	}
}
🤖 Prompt for AI Agents
In model/option.go around lines 178 to 202, the current group synchronization
code lacks error handling, does not use transactions, and performs separate
count and update/create operations that can cause race conditions. Refactor this
code to wrap all database operations in a single transaction to ensure
atomicity. Use GORM model methods instead of raw table queries for consistency.
Replace the count check with a single upsert or first-or-create operation to
avoid race conditions, and add error handling after each database call to
properly handle failures.

return json.Unmarshal([]byte(jsonStr), &CompletionRatio)
common.SysLog("Updating completion ratio, " + jsonStr)
err = json.Unmarshal([]byte(jsonStr), &CompletionRatio)
common.SysLog("Updated completion ratio success, " + fmt.Sprintf("%v", CompletionRatio["gemini-2.5-pro-preview-03-25"]))

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

Avoid logging potentially sensitive pricing data

Logging specific model completion ratios could expose sensitive pricing information.

-	common.SysLog("Updated completion ratio success, " + fmt.Sprintf("%v", CompletionRatio["gemini-2.5-pro-preview-03-25"]))
+	common.SysLog("Updated completion ratio successfully")
📝 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
common.SysLog("Updated completion ratio success, " + fmt.Sprintf("%v", CompletionRatio["gemini-2.5-pro-preview-03-25"]))
common.SysLog("Updated completion ratio successfully")
🤖 Prompt for AI Agents
In setting/operation_setting/model-ratio.go at line 489, avoid logging the
specific completion ratio value as it may expose sensitive pricing data. Modify
the log statement to remove or mask the detailed ratio value, instead logging a
generic success message without including the actual completion ratio.

Comment thread relay/channel/xai/text.go
Comment on lines +86 to +91
var response *dto.TextResponse
err = json.NewDecoder(bytes.NewReader(responseBody)).Decode(&response)
if err != nil {
common.SysError("error unmarshalling stream response: " + err.Error())
return nil, 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.

🛠️ Refactor suggestion

Inconsistent error handling between stream and non-stream handlers

The xAIHandler returns errors for body operations while xAIStreamHandler only logs them. Consider making error handling consistent.

Both handlers should have consistent error handling strategies. Either both should return errors or both should log and continue, depending on the desired behavior.

Also applies to: 111-116

🤖 Prompt for AI Agents
In relay/channel/xai/text.go around lines 86 to 91 and 111 to 116, the error
handling is inconsistent between xAIHandler and xAIStreamHandler; one returns
errors while the other only logs them. Decide on a consistent approach for both
handlers—either return errors or log and continue—and update the code
accordingly to ensure uniform error handling behavior in both places.

Comment thread relay/channel/xai/text.go
Comment on lines +92 to +93
response.Usage.CompletionTokens = response.Usage.TotalTokens - response.Usage.PromptTokens
response.Usage.CompletionTokenDetails.TextTokens = response.Usage.CompletionTokens - response.Usage.CompletionTokenDetails.ReasoningTokens

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.

🛠️ Refactor suggestion

Add validation for token calculations to prevent negative values

The token calculations could result in negative values if the API returns unexpected data.

-	response.Usage.CompletionTokens = response.Usage.TotalTokens - response.Usage.PromptTokens
-	response.Usage.CompletionTokenDetails.TextTokens = response.Usage.CompletionTokens - response.Usage.CompletionTokenDetails.ReasoningTokens
+	if response.Usage.TotalTokens >= response.Usage.PromptTokens {
+		response.Usage.CompletionTokens = response.Usage.TotalTokens - response.Usage.PromptTokens
+	} else {
+		common.SysError(fmt.Sprintf("Invalid token counts: total=%d, prompt=%d", response.Usage.TotalTokens, response.Usage.PromptTokens))
+		response.Usage.CompletionTokens = 0
+	}
+	
+	if response.Usage.CompletionTokenDetails != nil && 
+	   response.Usage.CompletionTokens >= response.Usage.CompletionTokenDetails.ReasoningTokens {
+		response.Usage.CompletionTokenDetails.TextTokens = response.Usage.CompletionTokens - response.Usage.CompletionTokenDetails.ReasoningTokens
+	}
📝 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
response.Usage.CompletionTokens = response.Usage.TotalTokens - response.Usage.PromptTokens
response.Usage.CompletionTokenDetails.TextTokens = response.Usage.CompletionTokens - response.Usage.CompletionTokenDetails.ReasoningTokens
if response.Usage.TotalTokens >= response.Usage.PromptTokens {
response.Usage.CompletionTokens = response.Usage.TotalTokens - response.Usage.PromptTokens
} else {
common.SysError(fmt.Sprintf(
"Invalid token counts: total=%d, prompt=%d",
response.Usage.TotalTokens,
response.Usage.PromptTokens,
))
response.Usage.CompletionTokens = 0
}
if response.Usage.CompletionTokenDetails != nil &&
response.Usage.CompletionTokens >= response.Usage.CompletionTokenDetails.ReasoningTokens {
response.Usage.CompletionTokenDetails.TextTokens =
response.Usage.CompletionTokens - response.Usage.CompletionTokenDetails.ReasoningTokens
}
🤖 Prompt for AI Agents
In relay/channel/xai/text.go around lines 92 to 93, the token calculations may
produce negative values if the API returns unexpected data. Add validation
checks after each calculation to ensure the resulting token counts are not
negative. If a negative value is detected, set the token count to zero or handle
it appropriately to maintain data integrity.

Comment on lines +206 to +207
pattern := "batch_result:*"
keys, err := redisClient.Keys(ctx, pattern).Result()

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.

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid using KEYS command in production

The KEYS command blocks the Redis server and can cause performance issues in production with large datasets.

Use SCAN instead of KEYS for production-safe iteration:

-	pattern := "batch_result:*"
-	keys, err := redisClient.Keys(ctx, pattern).Result()
+	var keys []string
+	iter := redisClient.Scan(ctx, 0, "batch_result:*", 100).Iterator()
+	for iter.Next(ctx) {
+		keys = append(keys, iter.Val())
+	}
+	if err := iter.Err(); err != nil {
+		return nil, fmt.Errorf("failed to scan keys from Redis: %w", err)
+	}

Also applies to: 220-221

🤖 Prompt for AI Agents
In relay/channel/volcengine/redis_client.go around lines 206-207 and 220-221,
the code uses the Redis KEYS command which blocks the server and can degrade
performance in production. Replace the KEYS command with the SCAN command to
safely iterate over keys without blocking. Implement a loop using SCAN to fetch
keys matching the pattern incrementally until the iteration is complete.

Comment on lines +78 to +79
fmt.Printf("Warning: Failed to add key %s to keep-alive manager: %v\n", key, err)
}

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.

🛠️ Refactor suggestion

Use structured logging instead of fmt.Printf

Direct stdout printing bypasses the application's logging configuration and makes log aggregation difficult.

-		fmt.Printf("Warning: Failed to add key %s to keep-alive manager: %v\n", key, err)
+		common.LogWarn("Failed to add key to keep-alive manager", fmt.Sprintf("key=%s, error=%v", key, err))

Apply similar changes to all fmt.Printf calls.

Also applies to: 127-127, 160-160, 197-197, 245-245, 253-253

🤖 Prompt for AI Agents
In relay/channel/volcengine/redis_client.go at lines 78-79 and also at lines
127, 160, 197, 245, and 253, replace all fmt.Printf calls with the application's
structured logging method. Identify the appropriate logger instance used in this
file and use it to log warnings or errors with structured fields instead of
printing directly to stdout. This ensures logs are consistent and compatible
with the logging configuration and aggregation tools.

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

Review continued from previous batch...

}

// 写入Redis
key := "batch_result:" + requestID

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.

🛠️ Refactor suggestion

Use constants for Redis key prefixes

Hardcoded key prefixes are scattered throughout the code, making it difficult to maintain and potentially causing key collisions.

const (
    BatchResultKeyPrefix = "batch_result:"
    LockKeyPrefix       = "lock:"
)

Then use these constants:

-	key := "batch_result:" + requestID
+	key := BatchResultKeyPrefix + requestID

Also applies to: 121-121, 154-154, 169-169, 191-191, 206-206, 280-280, 294-294

🤖 Prompt for AI Agents
In relay/channel/volcengine/redis_client.go at lines 69, 121, 154, 169, 191,
206, 280, and 294, replace all hardcoded Redis key prefixes with defined
constants. Define constants such as BatchResultKeyPrefix and LockKeyPrefix at
the top of the file, then use these constants to build Redis keys instead of
string literals. This will improve maintainability and reduce the risk of key
collisions.

Comment on lines +273 to +300

// TryAcquireLock 尝试获取分布式锁
func TryAcquireLock(lockKey string, expiration time.Duration) (bool, error) {
redisClient := getRedisClient()
ctx := context.Background()

// 使用SET命令的NX和EX选项实现分布式锁
result, err := redisClient.SetNX(ctx, "lock:"+lockKey, "locked", expiration).Result()
if err != nil {
return false, fmt.Errorf("failed to acquire lock: %w", err)
}

return result, nil
}

// ReleaseLock 释放分布式锁
func ReleaseLock(lockKey string) error {
redisClient := getRedisClient()
ctx := context.Background()

// 删除锁
err := redisClient.Del(ctx, "lock:"+lockKey).Err()
if err != nil {
return fmt.Errorf("failed to release lock: %w", err)
}

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

🛠️ Refactor suggestion

Implement proper distributed locking with ownership verification

The current lock implementation doesn't verify ownership before release, which could lead to one process releasing another process's lock.

Consider using a proper distributed locking library like github.com/go-redsync/redsync or implement lock ownership verification:

func TryAcquireLock(ctx context.Context, lockKey string, owner string, expiration time.Duration) (bool, error) {
    redisClient := getRedisClient()
    result, err := redisClient.SetNX(ctx, LockKeyPrefix+lockKey, owner, expiration).Result()
    if err != nil {
        return false, fmt.Errorf("failed to acquire lock: %w", err)
    }
    return result, nil
}

func ReleaseLock(ctx context.Context, lockKey string, owner string) error {
    redisClient := getRedisClient()
    // Use Lua script to ensure atomic check-and-delete
    script := `
        if redis.call("get", KEYS[1]) == ARGV[1] then
            return redis.call("del", KEYS[1])
        else
            return 0
        end
    `
    result, err := redisClient.Eval(ctx, script, []string{LockKeyPrefix + lockKey}, owner).Result()
    if err != nil {
        return fmt.Errorf("failed to release lock: %w", err)
    }
    if result.(int64) == 0 {
        return fmt.Errorf("lock not owned by caller")
    }
    return nil
}
🤖 Prompt for AI Agents
In relay/channel/volcengine/redis_client.go around lines 273 to 300, the current
distributed lock implementation does not verify ownership before releasing the
lock, risking one process releasing another's lock. Modify TryAcquireLock to
store a unique owner identifier as the lock value when acquiring the lock.
Update ReleaseLock to accept the owner identifier and use a Lua script to
atomically check if the lock is owned by the caller before deleting it. This
ensures only the lock owner can release the lock, preventing accidental unlocks
by others.

// CreateBatchRequestKey 在发起请求前预先创建Redis key
func CreateBatchRequestKey(requestID string) error {
redisClient := getRedisClient()
ctx := context.Background()

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.

🛠️ Refactor suggestion

Accept context as parameter instead of using context.Background()

All functions use context.Background() which prevents proper context propagation for timeouts and cancellation. This is especially important for Redis operations that might block.

Update all functions to accept context as the first parameter:

-func CreateBatchRequestKey(requestID string) error {
+func CreateBatchRequestKey(ctx context.Context, requestID string) error {
 	redisClient := getRedisClient()
-	ctx := context.Background()

Apply this pattern to all functions in the file.

Also applies to: 88-88, 134-134, 167-167, 189-189, 204-204, 218-218, 232-232, 264-264, 277-277, 291-291

🤖 Prompt for AI Agents
In relay/channel/volcengine/redis_client.go at lines 50, 88, 134, 167, 189, 204,
218, 232, 264, 277, and 291, the code uses context.Background() inside
functions, which prevents proper context propagation. Modify all these functions
to accept a context.Context parameter as their first argument and replace
context.Background() with this passed context to enable timeout and cancellation
propagation for Redis operations.

Comment on lines +33 to +43
if common.RedisEnabled && common.RDB != nil {
redisClient = common.RDB
} else {
// 如果项目Redis未启用,创建一个默认的本地Redis客户端
redisClient = redis.NewClient(&redis.Options{
Addr: "localhost:6379",
Password: "",
DB: 0,
})
}
})

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

Avoid hardcoded Redis configuration fallback

Creating a default Redis client with hardcoded localhost configuration could cause unexpected behavior in production if the main Redis is not available.

 		if common.RedisEnabled && common.RDB != nil {
 			redisClient = common.RDB
 		} else {
-			// 如果项目Redis未启用,创建一个默认的本地Redis客户端
-			redisClient = redis.NewClient(&redis.Options{
-				Addr:     "localhost:6379",
-				Password: "",
-				DB:       0,
-			})
+			// Return nil or panic if Redis is required but not configured
+			panic("Redis is required for batch processing but not configured")
 		}
📝 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 common.RedisEnabled && common.RDB != nil {
redisClient = common.RDB
} else {
// 如果项目Redis未启用,创建一个默认的本地Redis客户端
redisClient = redis.NewClient(&redis.Options{
Addr: "localhost:6379",
Password: "",
DB: 0,
})
}
})
if common.RedisEnabled && common.RDB != nil {
redisClient = common.RDB
} else {
// Return nil or panic if Redis is required but not configured
panic("Redis is required for batch processing but not configured")
}
})
🤖 Prompt for AI Agents
In relay/channel/volcengine/redis_client.go around lines 33 to 43, avoid using a
hardcoded localhost Redis configuration as a fallback when the main Redis is not
enabled or available. Instead, refactor the code to either return an error or
use a configurable Redis address from environment variables or configuration
files to prevent unexpected behavior in production environments.

Comment thread controller/relay.go
Comment on lines +175 to +199
// 处理自定义的 NewAPI batch 错误码
if openaiErr.StatusCode == dto.StatusNewAPIBatchRateLimitExceeded {
common.LogError(c, fmt.Sprintf("origin %d error: %s", openaiErr.StatusCode, openaiErr.Error.Message))
openaiErr.Error.Message = "当前服务端限速已满,请稍后再试"
}
if openaiErr.StatusCode == dto.StatusNewAPIBatchTimeout {
common.LogError(c, fmt.Sprintf("origin %d error: %s", openaiErr.StatusCode, openaiErr.Error.Message))
openaiErr.Error.Message = "未等待到结果,请稍后使用Retry_request_id再次查询"
}
if openaiErr.StatusCode == dto.StatusNewAPIBatchInternal {
common.LogError(c, fmt.Sprintf("origin %d error: %s", openaiErr.StatusCode, openaiErr.Error.Message))
openaiErr.Error.Message = "服务内部错误,请稍后再试"
}
if openaiErr.StatusCode == dto.StatusNewAPIBatchSubmitted {
common.LogError(c, fmt.Sprintf("origin %d error: %s", openaiErr.StatusCode, openaiErr.Error.Message))
openaiErr.Error.Message = "批量请求已提交,但是结果还未出来,请使用Retry_request_id查询结果"
}
if openaiErr.StatusCode == dto.StatusNewAPIBatchAccepted {
common.LogError(c, fmt.Sprintf("origin %d error: %s", openaiErr.StatusCode, openaiErr.Error.Message))
openaiErr.Error.Message = "批量请求已接受,正在处理中,请稍后使用Retry_request_id查询结果"
}
if openaiErr.StatusCode == dto.StatusRequestConflict {
common.LogError(c, fmt.Sprintf("origin %d error: %s", openaiErr.StatusCode, openaiErr.Error.Message))
openaiErr.Error.Message = "请求冲突,有其他请求使用了这个Retry_request_id,请稍后再试"
}

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.

🛠️ Refactor suggestion

Extract repetitive error message handling into a helper function

The error message replacement logic for batch errors is duplicated between Relay and WssRelay functions.

func replaceBatchErrorMessage(openaiErr *dto.OpenAIErrorWithStatusCode) {
    errorMessageMap := map[int]string{
        dto.StatusNewAPIBatchRateLimitExceeded: "当前服务端限速已满,请稍后再试",
        dto.StatusNewAPIBatchTimeout:           "未等待到结果,请稍后使用Retry_request_id再次查询",
        dto.StatusNewAPIBatchInternal:          "服务内部错误,请稍后再试",
        dto.StatusNewAPIBatchSubmitted:         "批量请求已提交,但是结果还未出来,请使用Retry_request_id查询结果",
        dto.StatusNewAPIBatchAccepted:          "批量请求已接受,正在处理中,请稍后使用Retry_request_id查询结果",
        dto.StatusRequestConflict:              "请求冲突,有其他请求使用了这个Retry_request_id,请稍后再试",
    }
    
    if newMessage, exists := errorMessageMap[openaiErr.StatusCode]; exists {
        common.LogError(c, fmt.Sprintf("origin %d error: %s", openaiErr.StatusCode, openaiErr.Error.Message))
        openaiErr.Error.Message = newMessage
    }
}

Then use it in both functions:

-		// 处理自定义的 NewAPI batch 错误码
-		if openaiErr.StatusCode == dto.StatusNewAPIBatchRateLimitExceeded {
-			common.LogError(c, fmt.Sprintf("origin %d error: %s", openaiErr.StatusCode, openaiErr.Error.Message))
-			openaiErr.Error.Message = "当前服务端限速已满,请稍后再试"
-		}
-		// ... (other similar blocks)
+		replaceBatchErrorMessage(openaiErr)

Also applies to: 264-288

🤖 Prompt for AI Agents
In controller/relay.go around lines 175 to 199 and also 264 to 288, the error
message replacement logic for batch errors is duplicated in both Relay and
WssRelay functions. Extract this repetitive logic into a helper function that
takes the openaiErr pointer, uses a map to match status codes to new messages,
logs the original error, and replaces the message accordingly. Then call this
helper function from both Relay and WssRelay to avoid duplication and improve
maintainability.

Comment thread common/logger.go
Comment on lines +140 to +142
// 更新 gin 的 writer
gin.DefaultWriter = io.MultiWriter(os.Stdout, w)
gin.DefaultErrorWriter = io.MultiWriter(os.Stderr, w)

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

Potential race condition when updating gin writers during log rotation.

Updating global gin writers while other goroutines might be actively logging could lead to race conditions or lost log entries.

Consider implementing a more robust solution:

  1. Use atomic pointer swaps for the writers
  2. Or implement a buffered approach where logs are queued during rotation
  3. Or use a logging library that handles rotation internally (e.g., lumberjack)
🤖 Prompt for AI Agents
In common/logger.go around lines 140 to 142, updating gin.DefaultWriter and
gin.DefaultErrorWriter directly can cause race conditions during log rotation.
To fix this, replace the direct assignment with an atomic pointer swap to safely
update the writers without interrupting concurrent logging. Alternatively,
implement a buffered logging mechanism to queue logs during rotation or switch
to a logging library like lumberjack that manages rotation internally and
safely.

Comment thread common/logger.go
Comment on lines +80 to 96

logPath := filepath.Join(*LogDir, fmt.Sprintf("oneapi-%s.log", time.Now().Format("20060102150405")))
fd, err := os.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
if err != nil {
log.Fatal("failed to open log file")
}
gin.DefaultWriter = io.MultiWriter(os.Stdout, fd)
gin.DefaultErrorWriter = io.MultiWriter(os.Stderr, fd)

// 创建一个自定义的 writer,用于检查文件大小
writer := &logWriter{
file: fd,
filepath: logPath,
size: 0,
}

gin.DefaultWriter = io.MultiWriter(os.Stdout, writer)
gin.DefaultErrorWriter = io.MultiWriter(os.Stderr, writer)
}

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.

🛠️ Refactor suggestion

Potential file descriptor leak when SetupLogger is called multiple times.

The function doesn't close existing file descriptors before opening new ones. Since SetupLogger can be called multiple times (via gopool when log count exceeds threshold), this could lead to file descriptor leaks.

Consider storing the current logWriter globally and closing it before creating a new one:

var currentLogWriter *logWriter

func SetupLogger() {
    // ... existing code ...
    
    // Close existing writer if any
    if currentLogWriter != nil {
        currentLogWriter.file.Close()
    }
    
    // ... create new writer ...
    currentLogWriter = writer
}
🤖 Prompt for AI Agents
In common/logger.go around lines 80 to 96, the SetupLogger function opens a new
log file each time it is called without closing the previously opened file
descriptor, causing a potential file descriptor leak. To fix this, introduce a
global variable to hold the current logWriter instance, and before creating a
new logWriter and opening a new file, check if the global variable is not nil
and close its file descriptor. Then assign the new logWriter to this global
variable to ensure only one file descriptor is open at a time.

Comment thread common/logger.go
Comment on lines +250 to +256
if ginCtx, ok := ctx.Value("gin_context").(*gin.Context); ok {
hashValue := ginCtx.GetInt("hash_value")
if hashValue > int(LogSampleRatio) {
return
}
}
}

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

Unsafe type assertion could cause panic.

The code performs type assertions without checking if they're safe, which could cause panics if the context doesn't contain a gin.Context.

-		if ginCtx, ok := ctx.Value("gin_context").(*gin.Context); ok {
+		if val := ctx.Value("gin_context"); val != nil {
+			if ginCtx, ok := val.(*gin.Context); ok {
 			hashValue := ginCtx.GetInt("hash_value")
 			if hashValue > int(LogSampleRatio) {
 				return
 			}
+			}
 		}

Apply similar fix to the second occurrence at lines 276-292.

Also applies to: 276-292

🤖 Prompt for AI Agents
In common/logger.go at lines 250-256 and also at lines 276-292, the code uses
type assertions on context values without verifying their safety, which can
cause panics if the assertion fails. Modify these sections to use the "comma ok"
idiom for type assertions, checking if the assertion succeeded before
proceeding. This ensures the code safely handles cases where the context does
not contain a *gin.Context, preventing runtime panics.

Comment thread common/logger.go
Comment on lines +33 to +34
// 10GB in bytes
maxLogFileSize = 20 * 1024 * 1024 * 1024

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

Fix the comment to match the actual value.

The comment says "10GB in bytes" but the actual value is 20GB (20 * 1024 * 1024 * 1024).

-	// 10GB in bytes
+	// 20GB in bytes
 	maxLogFileSize = 20 * 1024 * 1024 * 1024
📝 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
// 10GB in bytes
maxLogFileSize = 20 * 1024 * 1024 * 1024
// 20GB in bytes
maxLogFileSize = 20 * 1024 * 1024 * 1024
🤖 Prompt for AI Agents
In common/logger.go around lines 33 to 34, the comment above maxLogFileSize
incorrectly states "10GB in bytes" while the value is set to 20GB. Update the
comment to accurately reflect the value by changing it to "20GB in bytes" to
match the assigned size.

Comment thread common/logger.go
Comment on lines +157 to +161
sort.Slice(files, func(i, j int) bool {
fi, _ := os.Stat(files[i])
fj, _ := os.Stat(files[j])
return fi.ModTime().After(fj.ModTime())
})

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

Handle stat errors properly in file sorting.

The current implementation ignores stat errors, which could lead to incorrect file ordering and deletion of the wrong log files.

 	// 按修改时间排序
 	sort.Slice(files, func(i, j int) bool {
 		fi, _ := os.Stat(files[i])
 		fj, _ := os.Stat(files[j])
+		if fi == nil || fj == nil {
+			return false
+		}
 		return fi.ModTime().After(fj.ModTime())
 	})
📝 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
sort.Slice(files, func(i, j int) bool {
fi, _ := os.Stat(files[i])
fj, _ := os.Stat(files[j])
return fi.ModTime().After(fj.ModTime())
})
// 按修改时间排序
sort.Slice(files, func(i, j int) bool {
fi, _ := os.Stat(files[i])
fj, _ := os.Stat(files[j])
if fi == nil || fj == nil {
return false
}
return fi.ModTime().After(fj.ModTime())
})
🤖 Prompt for AI Agents
In common/logger.go around lines 157 to 161, the os.Stat errors are ignored when
sorting files by modification time, which can cause incorrect ordering. Modify
the sort function to check for errors returned by os.Stat for both files; if an
error occurs, handle it appropriately by deciding a consistent ordering or
skipping the file to avoid incorrect comparisons. This ensures that files with
stat errors do not disrupt the sorting logic and prevents potential wrong file
deletions.

@feitianbubu

Copy link
Copy Markdown
Member

173次commit, +6,446 −462, 兄弟这是把家底都提上来了 👍

@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: 2

♻️ Duplicate comments (1)
main.go (1)

202-216: Metrics endpoint security concern remains unaddressed.

The Prometheus metrics endpoint is still exposed without authentication, which could leak sensitive operational data. This issue was previously flagged but hasn't been resolved.

Consider securing the endpoint as suggested in the previous review:

-		go func() {
-			http.Handle("/metrics", promhttp.HandlerFor(gatherersRegistry, promhttp.HandlerOpts{}))
-			metricsPort := "9090"
-			if os.Getenv("METRICS_PORT") != "" {
-				metricsPort = os.Getenv("METRICS_PORT")
-			}
-			log.Println(http.ListenAndServe(fmt.Sprintf("0.0.0.0:%s", metricsPort),
-				nil))
-		}()
+		go func() {
+			mux := http.NewServeMux()
+			// Add authentication middleware here
+			mux.Handle("/metrics", promhttp.HandlerFor(gatherersRegistry, promhttp.HandlerOpts{}))
+			metricsPort := "9090"
+			if os.Getenv("METRICS_PORT") != "" {
+				metricsPort = os.Getenv("METRICS_PORT")
+			}
+			log.Println(http.ListenAndServe(fmt.Sprintf("0.0.0.0:%s", metricsPort), mux))
+		}()
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 492e05f and 74fd9e9.

📒 Files selected for processing (7)
  • controller/rate_limit.go (1 hunks)
  • controller/user_rate_limit.go (1 hunks)
  • main.go (8 hunks)
  • model/central_control.go (1 hunks)
  • model/main.go (6 hunks)
  • model/user.go (1 hunks)
  • router/api-router.go (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • model/main.go
  • router/api-router.go
🔇 Additional comments (4)
model/user.go (1)

230-237: LGTM! Well-implemented user lookup function.

The function follows good practices with input validation, secure password omission, and proper error handling. The Chinese error message is consistent with the codebase style.

controller/user_rate_limit.go (1)

10-45: LGTM! Clean and well-structured controller implementation.

The function properly validates input parameters, handles errors with appropriate HTTP status codes, and returns a consistent JSON response format. The parameter validation ensures all required fields are present before proceeding.

main.go (1)

102-105: Good initialization practices with proper error handling.

The new initialization steps (Central DB, request persistence, keep-alive manager) follow consistent patterns with proper error handling and resource cleanup using defer statements.

Also applies to: 121-129, 144-153

controller/rate_limit.go (1)

44-73: Good parameter validation and default handling.

The validation logic appropriately checks for required parameters and gracefully handles missing optional parameters by using sensible defaults (like the user's default group).

Also applies to: 130-145

Comment thread model/central_control.go
Comment on lines +29 to +36
func GetUserRateLimitConfig(username, groupName, modelName string) (*UserRateLimitConfig, error) {
var config UserRateLimitConfig
err := CENTRAL_DB.Where("site_name = ? AND username = ? AND group_name = ? AND model_name = ?", "newapi-prod-center", username, groupName, modelName).First(&config).Error
if err != nil {
return nil, err
}
return &config, 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.

🛠️ Refactor suggestion

Make the site name configurable instead of hardcoding it.

The hardcoded site name "newapi-prod-center" reduces flexibility and makes the code environment-specific. Consider making this configurable via an environment variable or configuration setting.

-	err := CENTRAL_DB.Where("site_name = ? AND username = ? AND group_name = ? AND model_name = ?", "newapi-prod-center", username, groupName, modelName).First(&config).Error
+	siteName := os.Getenv("SITE_NAME")
+	if siteName == "" {
+		siteName = "newapi-prod-center" // default fallback
+	}
+	err := CENTRAL_DB.Where("site_name = ? AND username = ? AND group_name = ? AND model_name = ?", siteName, username, groupName, modelName).First(&config).Error
📝 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
func GetUserRateLimitConfig(username, groupName, modelName string) (*UserRateLimitConfig, error) {
var config UserRateLimitConfig
err := CENTRAL_DB.Where("site_name = ? AND username = ? AND group_name = ? AND model_name = ?", "newapi-prod-center", username, groupName, modelName).First(&config).Error
if err != nil {
return nil, err
}
return &config, nil
}
func GetUserRateLimitConfig(username, groupName, modelName string) (*UserRateLimitConfig, error) {
var config UserRateLimitConfig
siteName := os.Getenv("SITE_NAME")
if siteName == "" {
siteName = "newapi-prod-center" // default fallback
}
err := CENTRAL_DB.
Where("site_name = ? AND username = ? AND group_name = ? AND model_name = ?", siteName, username, groupName, modelName).
First(&config).
Error
if err != nil {
return nil, err
}
return &config, nil
}
🤖 Prompt for AI Agents
In model/central_control.go around lines 29 to 36, the site name
"newapi-prod-center" is hardcoded in the database query, limiting flexibility.
Refactor the function to accept the site name as a parameter or retrieve it from
a configuration source such as an environment variable. Replace the hardcoded
string with this configurable value to make the code environment-agnostic.

Comment thread controller/rate_limit.go Outdated
Comment on lines +84 to +119
rateLimitInfo.ModelRequestRateLimitEnabled = setting.ModelRequestRateLimitEnabled
rateLimitInfo.ModelRequestRateLimitCount = setting.ModelRequestRateLimitCount
rateLimitInfo.ModelRequestRateLimitSuccessCount = setting.ModelRequestRateLimitSuccessCount
rateLimitInfo.ModelRequestRateLimitDurationMinutes = setting.ModelRequestRateLimitDurationMinutes

// 获取全局API限速配置
rateLimitInfo.GlobalApiRateLimitEnable = common.GlobalApiRateLimitEnable
rateLimitInfo.GlobalApiRateLimitNum = common.GlobalApiRateLimitNum
rateLimitInfo.GlobalApiRateLimitDuration = common.GlobalApiRateLimitDuration

// 获取全局Web限速配置
rateLimitInfo.GlobalWebRateLimitEnable = common.GlobalWebRateLimitEnable
rateLimitInfo.GlobalWebRateLimitNum = common.GlobalWebRateLimitNum
rateLimitInfo.GlobalWebRateLimitDuration = common.GlobalWebRateLimitDuration

// 获取模型倍率
if modelName != "" {
modelRatio, success := operation_setting.GetModelRatio(modelName)
if success {
rateLimitInfo.ModelRatio = modelRatio
}
}

// 获取分组倍率
rateLimitInfo.GroupRatio = setting.GetGroupRatio(group)

// 获取完成倍率
if modelName != "" {
rateLimitInfo.CompletionRatio = operation_setting.GetCompletionRatio(modelName)
}

// 获取缓存倍率
if modelName != "" {
cacheRatio, _ := operation_setting.GetCacheRatio(modelName)
rateLimitInfo.CacheRatio = cacheRatio
}

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.

🛠️ Refactor suggestion

Extract common rate limit info building logic to reduce duplication.

There's significant code duplication between GetRateLimitInfo and GetCurrentUserRateLimitInfo. The logic for building the rate limit info structure (lines 84-119 and 156-191) is nearly identical.

Consider extracting a helper function:

func buildRateLimitInfo(username, group, tokenName, modelName string) RateLimitInfo {
	rateLimitInfo := RateLimitInfo{
		Username:  username,
		Group:     group,
		TokenName: tokenName,
		ModelName: modelName,
	}

	// Common logic for fetching all rate limit settings...
	rateLimitInfo.ModelRequestRateLimitEnabled = setting.ModelRequestRateLimitEnabled
	// ... rest of the common logic

	return rateLimitInfo
}

Then use this helper in both functions to eliminate duplication.

Also applies to: 156-191

🤖 Prompt for AI Agents
In controller/rate_limit.go around lines 84 to 119 and similarly in lines 156 to
191, the code for building the RateLimitInfo struct is duplicated. Extract this
common logic into a new helper function, for example buildRateLimitInfo, which
takes parameters like username, group, tokenName, and modelName, and returns a
fully populated RateLimitInfo struct. Replace the duplicated code in both
GetRateLimitInfo and GetCurrentUserRateLimitInfo with calls to this new helper
function to reduce redundancy and improve maintainability.

@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: 1

♻️ Duplicate comments (3)
relay/channel/volcengine/batchchat.go (3)

110-128: Fix race condition in request slot acquisition.


447-459: Improve goroutine lifecycle management and context usage.


708-709: Use proper logging framework instead of fmt.Printf.

🧹 Nitpick comments (4)
relay/channel/volcengine/batchchat.go (4)

961-961: Use proper logging framework instead of fmt.Printf.

Replace debug logging with the project's logging framework:

-	fmt.Printf("Original volcengine response: %s\n", string(resultJson))
+	common.LogDebug(c, fmt.Sprintf("Original volcengine response: %s", string(resultJson)))

557-557: Consider using crypto/rand for security-sensitive random values.

Using math/rand for timing calculations could be predictable. If these delays are security-sensitive, consider using crypto/rand or ensure math/rand is properly seeded.

+import "crypto/rand"
+import "math/big"

-	waitMilliseconds := rand.Intn(timeRange) + int(MinAsyncTimeout.Milliseconds())
+	// Use crypto/rand for unpredictable timing
+	randomValue, _ := rand.Int(rand.Reader, big.NewInt(int64(timeRange)))
+	waitMilliseconds := int(randomValue.Int64()) + int(MinAsyncTimeout.Milliseconds())

546-592: Consider consolidating similar calculation functions.

The three functions calculateRandomWaitTime, calculateRateLimitWaitTime, and calculateRetryResponseDelay follow identical patterns. Consider creating a generic function to reduce code duplication.

func calculateRandomDuration(min, max time.Duration) time.Duration {
    timeRange := int(max.Milliseconds() - min.Milliseconds())
    if timeRange <= 0 {
        return min
    }
    waitMilliseconds := rand.Intn(timeRange) + int(min.Milliseconds())
    return time.Duration(waitMilliseconds) * time.Millisecond
}

958-1070: Consider optimizing JSON operations in response conversion.

The function performs multiple JSON marshal/unmarshal operations which could impact performance. Consider direct field mapping or streaming JSON parsing for better performance in high-throughput scenarios.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ade4000 and c43937f.

📒 Files selected for processing (5)
  • dto/openai_request.go (6 hunks)
  • metrics/metrics.go (1 hunks)
  • relay/channel/gemini/dto.go (4 hunks)
  • relay/channel/gemini/relay-gemini.go (4 hunks)
  • relay/channel/volcengine/batchchat.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • relay/channel/gemini/dto.go
  • relay/channel/gemini/relay-gemini.go
  • dto/openai_request.go
  • metrics/metrics.go
🔇 Additional comments (2)
relay/channel/volcengine/batchchat.go (2)

26-76: LGTM: Well-structured global configuration and thread-safe state management.

The constants are clearly defined and the global variables use appropriate synchronization primitives (mutexes for client cache, atomic operations for counters).


78-107: LGTM: Correct implementation of thread-safe client caching.

The double-checked locking pattern is properly implemented to prevent race conditions while maintaining performance.

Comment on lines +210 to +544
func DoBatchChatRequest(c *gin.Context, info *relaycommon.RelayInfo, requestBody io.Reader) (*http.Response, error) {
// 获取请求ID
requestID := getRequestID(c)

// 获取 retry header 状态
retryHeaderStatus := "false"
if isRetryRequest(c) {
retryHeaderStatus = "true"
}

// 记录请求开始时间
requestStartTime := time.Now()

// 用于记录最终状态码的变量
var finalStatusCode string
var finalError error

// 使用 defer 确保在所有情况下都记录指标
defer func() {
// 如果没有设置状态码,说明是正常流程
if finalStatusCode == "" {
finalStatusCode = "success"
}

metrics.IncrementBatchRequestCounter(
fmt.Sprintf("%d", info.ChannelId),
info.ChannelName,
info.ChannelTag,
info.BaseUrl,
info.UpstreamModelName,
info.Group,
finalStatusCode,
retryHeaderStatus,
1,
)
metrics.ObserveBatchRequestDuration(
fmt.Sprintf("%d", info.ChannelId),
info.ChannelName,
info.ChannelTag,
info.BaseUrl,
info.UpstreamModelName,
info.Group,
finalStatusCode,
retryHeaderStatus,
time.Since(requestStartTime).Seconds(),
)
}()

// 尝试获取分布式锁,避免重复执行
lockKey := requestID + "_lock"
lockAcquired, err := TryAcquireLock(lockKey, DistributedLockExpiration)
if err != nil {
finalStatusCode = "lock_acquisition_error"
finalError = fmt.Errorf("failed to acquire lock: %w", err)
return nil, finalError
}

if !lockAcquired {
finalStatusCode = "lock_already_acquired"

// 返回内部错误响应
errorResponse := gin.H{
"error": gin.H{
"message": fmt.Sprintf("request %s is already being processed, another request is in progress", requestID),
"type": "internal_error",
"code": "lock_acquisition_failed",
"request_id": requestID,
},
}
errorJson, _ := json.Marshal(errorResponse)
response := &http.Response{
StatusCode: dto.StatusRequestConflict,
Body: io.NopCloser(strings.NewReader(string(errorJson))),
Header: make(http.Header),
}
response.Header.Set("Content-Type", "application/json")
return response, nil
}

// 确保在函数结束时释放锁
defer func() {
if releaseErr := ReleaseLock(lockKey); releaseErr != nil {
common.LogError(c, fmt.Sprintf("Failed to release lock for request %s: %v", requestID, releaseErr))
}
}()

// 检查是否为重试请求
if isRetryRequest(c) {
// 应用重试响应延迟
retryDelay := calculateRetryResponseDelay()
if retryDelay > 0 {
common.LogInfo(c.Request.Context(), fmt.Sprintf("Applying retry response delay: %v for request %s", retryDelay, requestID))
time.Sleep(retryDelay)
}

// 从Redis获取结果,使用当前的requestID(可能是retry_request_id)
resultData, err := GetBatchResultFromRedis(requestID)
if err == nil {
// 检查Result是否为空且状态为pending,这种情况说明第一次请求可能超时了
if resultData.Result == "" && resultData.Status == "pending" {
finalStatusCode = "retry_pending"
common.LogInfo(c.Request.Context(), fmt.Sprintf("Found pending request for %s, returning retry response", requestID))

// 返回重试提示
errorResponse := gin.H{
"error": gin.H{
"message": "Request is still being processed, please retry later",
"type": "request_in_progress",
"code": "request_still_processing",
"request_id": requestID,
},
}
errorJson, _ := json.Marshal(errorResponse)

response := &http.Response{
StatusCode: dto.StatusNewAPIBatchSubmitted, // 203 - 批量请求已提交,需要重试获取结果
Body: io.NopCloser(strings.NewReader(string(errorJson))),
Header: make(http.Header),
}
response.Header.Set("Content-Type", "application/json")
response.Header.Set("Retry_request_id", requestID)
// 添加建议重试时间header
avgDuration := GetBatchRequestAverageDuration()
response.Header.Set("X-Suggested-Retry-After", fmt.Sprintf("%.0f", avgDuration))
c.Writer.Header().Set("Retry_request_id", requestID)
c.Writer.Header().Set("X-Suggested-Retry-After", fmt.Sprintf("%.0f", avgDuration))
return response, nil
} else if resultData.Result != "" {
finalStatusCode = "retry_cache_hit"
// 只有在Result不为空时才处理缓存结果
// 删除Redis中的key
err = DeleteBatchResultFromRedis(requestID)
if err != nil {
common.LogError(c, err.Error())
}

// 先用火山引擎格式解析,再转换为SimpleResponse
openaiResponse, err := convertVolcEngineResponseToOpenAI([]byte(resultData.Result))
if err != nil {
finalStatusCode = "retry_cache_convert_error"
finalError = fmt.Errorf("failed to convert cached response format: %w", err)
return nil, finalError
}

// 将转换后的结果序列化为JSON
openaiResponseJson, err := json.Marshal(openaiResponse)
if err != nil {
finalStatusCode = "retry_cache_marshal_error"
finalError = fmt.Errorf("failed to marshal cached OpenAI response: %w", err)
return nil, finalError
}

// 找到结果,返回并删除key
response := &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(string(openaiResponseJson))),
Header: make(http.Header),
}
response.Header.Set("Content-Type", "application/json")
return response, nil
} else {
// Result为空但状态不是pending,说明是错误状态
common.LogInfo(c.Request.Context(), fmt.Sprintf("Found error status for request %s, continuing with new request", requestID))
// 删除Redis中的key,继续执行新建流程
err = DeleteBatchResultFromRedis(requestID)
if err != nil {
common.LogError(c, err.Error())
}
}
}
// 如果Redis中没有找到结果,继续执行新建流程
}

// 尝试获取请求槽位,如果无法立即获取则等待
if err := acquireRequestSlot(); err != nil {
// 如果无法立即获取槽位,等待可用槽位
// 随机计算等待时间:在100ms-1000ms之间随机选择
waitTime := calculateRateLimitWaitTime()

ctx, cancel := context.WithTimeout(c.Request.Context(), waitTime)
defer cancel()

if waitErr := waitForAvailableSlot(ctx); waitErr != nil {
finalStatusCode = "rate_limit_exceeded"
// 等待超时,返回自定义限流错误
errorResponse := gin.H{
"error": gin.H{
"message": "Request limit reached, please retry later",
"type": "new_api_batch_rate_limit_exceeded",
"code": "new_api_batch_rate_limit_exceeded",
},
}
errorJson, _ := json.Marshal(errorResponse)
response := &http.Response{
StatusCode: dto.StatusNewAPIBatchRateLimitExceeded,
Body: io.NopCloser(strings.NewReader(string(errorJson))),
Header: make(http.Header),
}
response.Header.Set("Content-Type", "application/json")
return response, nil
}
}

// 确保在函数结束时释放槽位
defer releaseRequestSlot()

// 解析请求体
var request dto.GeneralOpenAIRequest
if err := json.NewDecoder(requestBody).Decode(&request); err != nil {
finalStatusCode = "request_decode_error"
finalError = fmt.Errorf("failed to decode request body: %w", err)
return nil, finalError
}
// 转换为豆包批量请求格式
batchRequest, err := convertToBatchRequest(&request, info.Endpoint)
if err != nil {
finalStatusCode = "request_convert_error"
finalError = fmt.Errorf("failed to convert request: %w", err)
return nil, finalError
}

// 检查是否有未支持的参数
checkUnsupportedParameters(&request)

// 使用 channel ID 获取或创建客户端实例
client := GetBatchClient(fmt.Sprintf("%d", info.ChannelId), info.ApiKey)

// 创建带超时的 context,用于异步调用的整体超时
timeoutDuration := getAsyncCallTimeout(c.Request.Context())
asyncCtx, asyncCancel := context.WithTimeout(c.Request.Context(), timeoutDuration)
defer asyncCancel()

// 创建通道用于接收异步结果
resultChan := make(chan interface{}, 1)
errChan := make(chan error, 1)

// 异步发起批量推理请求
go func() {
// 使用独立的context,不受外层asyncCtx影响
independentCtx := context.Background()

result, err := executeBatchRequestWithRedis(independentCtx, client, batchRequest, requestID)

if err != nil {
common.LogError(c, fmt.Sprintf("Async batch request failed for requestID %s: %v", requestID, err))
errChan <- err
return
}
resultChan <- result
}()

// 等待结果或超时
var result interface{}
select {
case result = <-resultChan:
// 成功获取结果
common.LogInfo(c, fmt.Sprintf("Received result for requestID %s", requestID))
case err := <-errChan:
// 发生错误
finalStatusCode = "async_request_failed"
finalError = fmt.Errorf("batch request failed: %w", err)
common.LogError(c, fmt.Sprintf("batch request failed: %v", err))
return nil, finalError
case <-asyncCtx.Done():
// 超时
if asyncCtx.Err() == context.DeadlineExceeded {
finalStatusCode = "async_timeout"
common.LogError(c, fmt.Sprintf("Async call timeout after %v for requestID %s", timeoutDuration, requestID))
c.Writer.Header().Set("Retry_request_id", requestID)

// 返回自定义状态码表示请求已提交
errorResponse := gin.H{
"error": gin.H{
"message": fmt.Sprintf("Async call timeout after %v for requestID %s, please retry later to get the result", timeoutDuration, requestID),
"type": "request_submitted",
"code": "request_submitted",
"request_id": requestID,
},
}
errorJson, _ := json.Marshal(errorResponse)
response := &http.Response{
StatusCode: dto.StatusNewAPIBatchSubmitted, // 203 - 批量请求已提交,需要重试获取结果
Body: io.NopCloser(strings.NewReader(string(errorJson))),
Header: make(http.Header),
}
response.Header.Set("Content-Type", "application/json")
response.Header.Set("Retry_request_id", requestID)
// 添加建议重试时间header
avgDuration := GetBatchRequestAverageDuration()
response.Header.Set("X-Suggested-Retry-After", fmt.Sprintf("%.0f", avgDuration))
c.Writer.Header().Set("Retry_request_id", requestID)
c.Writer.Header().Set("X-Suggested-Retry-After", fmt.Sprintf("%.0f", avgDuration))
return response, nil
}
finalStatusCode = "async_cancelled"
finalError = fmt.Errorf("async call cancelled: %w", asyncCtx.Err())
common.LogError(c, fmt.Sprintf("Async call cancelled for requestID %s: %w", requestID, asyncCtx.Err()))
c.Writer.Header().Set("Retry_request_id", requestID)
return nil, finalError
}

// 将结果转换为JSON
resultJson, err := json.Marshal(result)
if err != nil {
finalStatusCode = "result_marshal_error"
finalError = fmt.Errorf("failed to marshal result: %w", err)
return nil, finalError
}

// 将火山引擎的响应转换为标准的OpenAI格式
openaiResponse, err := convertVolcEngineResponseToOpenAI(resultJson)
if err != nil {
finalStatusCode = "response_convert_error"
finalError = fmt.Errorf("failed to convert response format: %w", err)
return nil, finalError
}

// 将转换后的结果序列化为JSON
openaiResponseJson, err := json.Marshal(openaiResponse)
if err != nil {
finalStatusCode = "openai_response_marshal_error"
finalError = fmt.Errorf("failed to marshal OpenAI response: %w", err)
return nil, finalError
}

// 创建HTTP响应
response := &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(string(openaiResponseJson))),
Header: make(http.Header),
}
response.Header.Set("Content-Type", "application/json")
response.Header.Set("Retry_request_id", requestID)
return response, 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.

🛠️ Refactor suggestion

Consider refactoring this large function into smaller, focused functions.

This function has 334 lines and handles multiple responsibilities (distributed locking, retry logic, rate limiting, async execution, response handling). Breaking it into smaller functions would improve testability, readability, and maintainability.

Consider extracting:

  • Retry request handling logic
  • Rate limiting and slot acquisition
  • Async request execution
  • Response processing and conversion
🤖 Prompt for AI Agents
In relay/channel/volcengine/batchchat.go around lines 210 to 544, the
DoBatchChatRequest function is excessively large and handles multiple concerns
such as distributed locking, retry logic, rate limiting, async execution, and
response processing. To improve readability and maintainability, refactor this
function by extracting smaller focused functions for each major responsibility:
one for handling retry request logic including Redis cache checks, one for rate
limiting and request slot acquisition, one for executing the async batch request
and handling its result, and one for processing and converting the final
response. Replace the corresponding code blocks in the main function with calls
to these new helper functions.

@seefs001 seefs001 closed this Oct 31, 2025
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.

7 participants