fix: gemini batch embedding token not counted - #2061
Conversation
WalkthroughA new public handler Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
relay/helper/valid_request.go (1)
325-332: Consider adding validation for the Requests field.The handler follows the pattern of
GetAndValidateGeminiEmbeddingRequest, but consider adding defensive validation to ensure theRequestsarray is not empty, similar to howGetAndValidateRerankRequestvalidates its fields (lines 93-98).Apply this diff to add validation:
func GetAndValidateGeminiBatchEmbeddingRequest(c *gin.Context) (*dto.GeminiBatchEmbeddingRequest, error) { request := &dto.GeminiBatchEmbeddingRequest{} err := common.UnmarshalBodyReusable(c, request) if err != nil { return nil, err } + if len(request.Requests) == 0 { + return nil, errors.New("requests is required and cannot be empty") + } return request, nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relay/helper/valid_request.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
relay/helper/valid_request.go (3)
dto/request_common.go (1)
Request(8-12)dto/gemini.go (1)
GeminiBatchEmbeddingRequest(381-383)common/gin.go (1)
UnmarshalBodyReusable(32-53)
🔇 Additional comments (2)
relay/helper/valid_request.go (2)
25-31: ---Routing and batch handler implementation is correct; token counting aggregation verified.
The routing logic properly separates
:embedContentand:batchEmbedContentsrequests into distinct handlers. The newGetAndValidateGeminiBatchEmbeddingRequestfunction follows the established pattern, and theGeminiBatchEmbeddingRequestDTO correctly aggregates token counts across all batch requests in itsGetTokenCountMeta()implementation (iterating over theRequestsslice). This integration withrelay/gemini_handler.goensures token counting works correctly for batch operations, addressing the PR's stated fix.
325-332: No issues found — GeminiBatchEmbeddingRequest properly implements the Request interface.The verification confirms that
dto.GeminiBatchEmbeddingRequestcorrectly implements theRequestinterface with all required methods:
IsStream()returns false for non-streamed batch requestsGetTokenCountMeta()(lines 390–401) aggregates token count metadata from nested requests by iterating throughr.Requestsand combining their textSetModelName()(lines 404–410) propagates the model name to nested requestsThe token counting functionality for batch embeddings is properly implemented.
…-embedding-token-count fix: gemini batch embedding token not counted
Summary by CodeRabbit