Skip to content

feat: jimeng task query use correct req_key - #2364

Closed
feitianbubu wants to merge 4768 commits into
QuantumNous:mainfrom
feitianbubu:pr/aea95ab4fbaef36399b380008fcf460a9c04dc28
Closed

feat: jimeng task query use correct req_key#2364
feitianbubu wants to merge 4768 commits into
QuantumNous:mainfrom
feitianbubu:pr/aea95ab4fbaef36399b380008fcf460a9c04dc28

Conversation

@feitianbubu

@feitianbubu feitianbubu commented Dec 3, 2025

Copy link
Copy Markdown
Member

优化即梦任务查询, 严格按官方文档不同模型使用对应的req_key参数执行查询
文档: https://www.volcengine.com/docs/85621/1777001

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling and validation for task request parameters
    • Enhanced authentication key handling for non-new API relays
  • Refactor

    • Centralized task request retrieval for better consistency and error propagation
    • Streamlined model information propagation through request payloads

✏️ Tip: You can customize this high-level summary in your review settings.

feitianbubu and others added 30 commits October 18, 2025 01:48
…-embedding-token-count

fix: gemini batch embedding token not counted
fix: handle JSON parsing for thinking content in ollama stream
…-emotion

豆包语音2.0音色支持情感,情绪,音量
Comment out the debug log for MiniMax TTS Request.
…ort-stream-options

Ali channel support stream options
…ream

feat: openai tts support streaming realtime audio
…ream

feat: doubao tts support streaming realtime audio
multipart/form-data; boundary
…edit

修复豆包图像编辑(图生图)功能
Calcium-Ion and others added 25 commits November 30, 2025 18:48
…emini-image-edit

Revert "Gemini Image系列支持图像编辑"
…ix-nano-banana-err

Revert "fix: nano-banana not compatible imageSize"
…dd-gemini-3-pro-image-preview-oai

Revert "OAI生图接口支持gemini 3 pro image preview"
…dels (Midjourney, Rerank, Suno). Add OpenAPI specifications for backend management and relay interfaces.
…i-turn

feat(gemini): implement markdown image handling in text processing
chore: update the relay openapi file
- Introduced new OpenAI text models in `common/model.go`.
- Added `IsOpenAITextModel` function to check for OpenAI text models.
- Refactored token estimation methods across various channels to use estimated prompt tokens instead of direct prompt token counts.
- Updated related functions and structures to accommodate the new token estimation approach, enhancing overall token management.
fix: try resolve the high concurrency issue to a single host
…ken-counter

feat: refactor token estimation logic
…king-level

feat: gemini 3 thinking level gemini-3-pro-preview-high
@coderabbitai

coderabbitai Bot commented Dec 3, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Added a "req_key" parameter to the FetchTask payload using UpstreamModelName from task properties. Refactored the jimeng adaptor to retrieve task requests via a centralized helper and extract req_key from the request body instead of manual parsing. Simplified key signing logic by removing combined key parsing.

Changes

Cohort / File(s) Summary
Task Controller
controller/task_video.go
Added "req_key" field to FetchTask payload, populated with task.Properties.UpstreamModelName
Jimeng Adaptor
relay/channel/task/jimeng/adaptor.go
Modified BuildRequestBody to use centralized relaycommon.GetTaskRequest() for request retrieval; updated FetchTask to extract req_key from request body and assign to info.UpstreamModelName; removed combined key parsing from sign-on-path logic

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Attention areas:
    • Verify that body["req_key"] extraction handles all expected input formats and error cases correctly
    • Confirm that relaycommon.GetTaskRequest() error propagation is consistent with existing error handling patterns
    • Validate that removal of combined key (ak|sk) parsing doesn't break existing sign-on flows for non-new API relays

Possibly related PRs

Suggested reviewers

  • creamlike1024
  • xyfacai

Poem

🐰 A key hops through the request stream,
No parsing, just a centered dream,
From properties to payload sent,
The task flow now has less to lent! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: jimeng task query use correct req_key' directly describes the main change: fixing the jimeng task query to use the correct req_key parameter, which aligns with the actual modifications to req_key handling across the codebase.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b9f6a5 and cf1ba14.

📒 Files selected for processing (2)
  • controller/task_video.go (1 hunks)
  • relay/channel/task/jimeng/adaptor.go (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
controller/task_video.go (1)
model/task.go (1)
  • Properties (75-79)
relay/channel/task/jimeng/adaptor.go (1)
relay/common/relay_utils.go (1)
  • GetTaskRequest (62-72)
🔇 Additional comments (4)
relay/channel/task/jimeng/adaptor.go (3)

123-126: LGTM! Good refactoring to centralized helper.

Using relaycommon.GetTaskRequest(c) instead of direct context extraction improves maintainability and ensures consistent error handling across the codebase.


244-244: LGTM! Good simplification of signing logic.

Using the pre-parsed a.accessKey and a.secretKey (set in Init at lines 89-94) instead of re-parsing the combined key improves efficiency and reduces code duplication.


166-166: Add validation for non-empty body.ReqKey.

While info.UpstreamModelName is set from body.ReqKey to enable correct req_key propagation to task queries, there's no validation that body.ReqKey is non-empty. If the original request has an empty model field, this could propagate empty values through the system, eventually causing FetchTask failures.

Consider adding validation before this assignment:

+	if body.ReqKey == "" {
+		return nil, fmt.Errorf("req_key cannot be empty")
+	}
 	info.UpstreamModelName = body.ReqKey
⛔ Skipped due to learnings
Learnt from: RedwindA
Repo: QuantumNous/new-api PR: 1517
File: controller/channel.go:196-196
Timestamp: 2025-08-06T17:09:08.221Z
Learning: In the channel management system, channel keys are validated as required during channel creation, so they are guaranteed to be non-empty in functions like FetchUpstreamModels. Additional validation for empty keys in downstream functions is not needed.
controller/task_video.go (1)

76-80: Verify impact on existing tasks without UpstreamModelName.

The addition of req_key derived from task.Properties.UpstreamModelName may break status queries for tasks created before this change. Verify that:

  1. Existing pending tasks have UpstreamModelName populated, or tasks fail gracefully when it's empty.
  2. The validation in adaptor.go (lines 223-227) handles empty/missing req_key appropriately—either by rejecting strictly or providing a fallback.
  3. Whether a migration is needed for backward compatibility with tasks created before this field was added.

Comment on lines +223 to +227
if reqKey, ok := body["req_key"].(string); !ok {
return nil, fmt.Errorf("invalid req_key")
} else {
payload["req_key"] = reqKey
}

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 | 🟠 Major

Enhance validation to reject empty req_key.

The validation checks that req_key exists and is a string, but doesn't verify it's non-empty. An empty string would pass validation but likely cause upstream API failures.

Apply this diff to add empty string validation:

-	if reqKey, ok := body["req_key"].(string); !ok {
+	if reqKey, ok := body["req_key"].(string); !ok || reqKey == "" {
-		return nil, fmt.Errorf("invalid req_key")
+		return nil, fmt.Errorf("invalid or empty req_key")
 	} else {
 		payload["req_key"] = reqKey
 	}
📝 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 reqKey, ok := body["req_key"].(string); !ok {
return nil, fmt.Errorf("invalid req_key")
} else {
payload["req_key"] = reqKey
}
if reqKey, ok := body["req_key"].(string); !ok || reqKey == "" {
return nil, fmt.Errorf("invalid or empty req_key")
} else {
payload["req_key"] = reqKey
}
🤖 Prompt for AI Agents
In relay/channel/task/jimeng/adaptor.go around lines 223 to 227, the current
validation only checks that body["req_key"] is a string but allows an empty
string; update the check to reject empty strings as well by verifying reqKey !=
"" and returning an error (e.g., fmt.Errorf("invalid req_key")) if it's empty,
otherwise assign payload["req_key"] = reqKey.

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.