feat: support VolcEngine Agent Plan and Coding Plan - #6693
Conversation
WalkthroughThe change adds VolcEngine Agent Plan and Coding Plan channels, including discovery, endpoint mapping, request signing, relay routing, frontend configuration, localized guidance, and admin request metadata logging. ChangesVolcEngine channel support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Admin
participant ChannelController
participant VolcEngineAPI
participant ChannelUI
Admin->>ChannelController: fetch models
ChannelController->>VolcEngineAPI: discover plans or endpoints
VolcEngineAPI-->>ChannelController: return models and mappings
ChannelController-->>ChannelUI: return model_mapping
ChannelUI->>ChannelUI: merge selected mappings
ChannelUI-->>Admin: display selectable models
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
All branch, description, file, and commit checks now pass on the clean upstream-based branch. The remaining failure is account/action-level:\n\n1. max-daily-forks reports 9 because anti-slop calculates the maximum historical 24-hour fork window across all owned forks, rather than forks created in the current 24 hours.\n2. The merge-ratio check then aborts while using the workflow GITHUB_TOKEN for a global author search: "The listed users cannot be searched either because the users do not exist or you do not have permission to view the users."\n\nCould a maintainer please add the exempt label and rerun the PR Check? The code-related checks report 1 emoji, no blocked paths, all final newlines present, and 9 added comments within the limit of 10. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
relay/channel/volcengine/adaptor.go (1)
330-336: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSet the plan API key on audio requests.
Line 330 runs after plan credential parsing removes the management credentials. The normalized API key has no
|, so this branch exits at Line 336 without settingAuthorization.
GetRequestURLstill routes plan audio requests to/v1/audio/speech. Those requests reach the upstream service without authentication.Proposed fix
if info.RelayMode == constant.RelayModeAudioSpeech { parts := strings.Split(info.ApiKey, "|") if len(parts) == 2 { req.Set("Authorization", "Bearer;"+parts[1]) + } else if info.ChannelType == channelconstant.ChannelTypeVolcEngineAgentPlan || + info.ChannelType == channelconstant.ChannelTypeVolcEngineCodingPlan { + req.Set("Authorization", "Bearer "+apiKey) } req.Set("Content-Type", "application/json") return nil }Add regression cases for both plan channel types.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/volcengine/adaptor.go` around lines 330 - 336, Update the audio request authorization logic in the RelayModeAudioSpeech branch of the request setup method to set the normalized plan API key as the Bearer credential when ApiKey no longer contains management credentials, while preserving existing parsed-key handling. Add regression coverage for both plan channel types and verify their audio requests include Authorization when GetRequestURL routes them to /v1/audio/speech.relay/common/relay_utils.go (1)
50-74: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove URL fragments before logging.
A URL fragment can contain a token or other credential.
parsedURL.String()preserves it. ClearparsedURL.Fragmentand add a regression test for a URL such ashttps://upstream.test/v1#token=secret.Proposed fix
if parsedURL.User != nil { parsedURL.User = url.User("***masked***") changed = true } + if parsedURL.Fragment != "" { + parsedURL.Fragment = "" + changed = true + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/common/relay_utils.go` around lines 50 - 74, Update the URL sanitization flow around parsedURL.String() to clear parsedURL.Fragment before returning the sanitized URL, ensuring fragments containing credentials are removed from logs. Add a regression test covering https://upstream.test/v1#token=secret and verify the resulting URL has no fragment.
🧹 Nitpick comments (13)
relay/channel/volcengine/adaptor_test.go (1)
50-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the Agent Plan image-edits route.
GetRequestURLsendsRelayModeImagesEditsthrough the same Agent Planimages/generationsURL. Add anImagesEditscase so a future change cannot split these routes.As per coding guidelines, “Backend tests must protect real behavior, API contracts, billing/accounting invariants, compatibility, or regression paths.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/volcengine/adaptor_test.go` around lines 50 - 53, Add a test case for RelayModeImagesEdits with ChannelTypeVolcEngineAgentPlan in the GetRequestURL test cases, expecting the same Agent Plan images/generations URL as RelayModeImagesGenerations. Keep the existing generation case unchanged and ensure both modes are explicitly covered.Source: Coding guidelines
controller/channel_upstream_update.go (5)
223-225: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the no-op reassignment.
nextMappingis already an initialized empty map at this point. The assignment changes nothing.♻️ Proposed cleanup
- if len(nextMapping) == 0 { - nextMapping = map[string]string{} - } if mapsEqual(existingMapping, nextMapping) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/channel_upstream_update.go` around lines 223 - 225, Remove the len(nextMapping) == 0 conditional and its reassignment, leaving the already initialized nextMapping unchanged.
661-668: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueGuard the built-in endpoint lookup.
Line 667 writes
inner.ModelMapping[name]without checking presence.inner.Modelspasses throughnormalizeModelNames, so a future change to that normalization would silently store an empty target here.🛡️ Proposed guard
for _, name := range normalizeModelNames(inner.Models) { + innerID, ok := inner.ModelMapping[name] + if !ok || innerID == "" { + continue + } if _, exists := seen[name]; !exists { modelNames = append(modelNames, name) seen[name] = struct{}{} } // Built-in endpoints win model-name conflicts. - modelMapping[name] = inner.ModelMapping[name] + modelMapping[name] = innerID }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/channel_upstream_update.go` around lines 661 - 668, Guard the modelMapping assignment in the loop over normalizeModelNames(inner.Models) by checking whether inner.ModelMapping contains name before writing it; only store the mapped target when the lookup succeeds, while preserving the existing built-in conflict precedence and modelNames deduplication.
1392-1405: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueNote the synchronous discovery call in the admin apply path.
Each manual apply that adds models triggers a fresh VolcEngine discovery. That performs two paginated, signed management requests before the response returns. The auto-sync path already persisted the mapping for previously detected models.
Consider reusing the persisted mapping when the selected additions are already covered, and fetch only when a selected model has no mapping.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/channel_upstream_update.go` around lines 1392 - 1405, The admin apply path should avoid repeating synchronous VolcEngine discovery when persisted mappings already cover the selected additions. Update the logic around fetchChannelUpstreamModelDiscovery and applyDiscoveredModelMapping to reuse the channel’s persisted mapping, invoking discovery only when at least one selected model lacks a mapping, while preserving the existing mapping application and error propagation behavior.
796-823: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate the OpenAI model list parsing.
Lines 809-811 call
parseOpenAIModelIDsfor VolcEngine. Lines 813-823 then repeat almost the same logic for the remaining channel types; the only difference is the Geminimodels/prefix trim. Gemini already returns earlier at Line 728, so the branch at Line 818 is unreachable.Route all remaining types through
parseOpenAIModelIDsand drop the dead Gemini branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/channel_upstream_update.go` around lines 796 - 823, Update the model-list parsing after the VolcEngine handling to route all remaining channel types through parseOpenAIModelIDs, removing the duplicated response unmarshalling and normalization logic. Delete the unreachable Gemini-specific models/ prefix trimming branch while preserving the existing earlier Gemini handling and error behavior.
238-248: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
maps.Equalfor string maps.
mapsEqualduplicatesmaps.Equal[string, string]behavior. Replace the helper withmaps.Equal(existingMapping, nextMapping)and add"maps"to the import block.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/channel_upstream_update.go` around lines 238 - 248, Remove the redundant mapsEqual helper and use maps.Equal(existingMapping, nextMapping) at its call sites instead. Add the standard "maps" import and preserve the existing string-map comparison behavior.web/src/features/channels/lib/__tests__/model-mapping-discovery.test.ts (1)
24-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for malformed existing mapping input.
mergeDiscoveredModelMappinghas explicit branches for invalid JSON, a non-object value, an array, and non-string entry values (web/src/features/channels/lib/model-mapping-discovery.tsLines 36-50). None of those branches is covered. Add one case that passes an invalid JSON string and asserts the discovered mapping is still produced.💚 Proposed test
test('ignores an unparsable existing mapping', () => { const result = mergeDiscoveredModelMapping( '{not json', { current: 'ep-current' }, ['current'], [] ) assert.deepEqual(JSON.parse(result), { current: 'ep-current' }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/features/channels/lib/__tests__/model-mapping-discovery.test.ts` around lines 24 - 60, Add a test case in the mergeDiscoveredModelMapping suite that passes malformed existing JSON, such as '{not json', with a selected discovered endpoint, then assert the parsed result contains the discovered mapping despite the invalid input.web/src/features/channels/lib/model-mapping-discovery.ts (2)
21-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
normalizeModelNameListis defined twice with identical bodies. Both copies map throughnormalizeModelName, drop empty values, and deduplicate through aSet. Keeping two copies risks the dialog and the merge logic normalizing model names differently after a future change.
web/src/features/channels/lib/model-mapping-discovery.ts#L21-L27: export this function so it becomes the single definition.web/src/features/channels/components/dialogs/fetch-models-dialog.tsx#L52-L54: delete the local definition and import the exported one from../../lib.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/features/channels/lib/model-mapping-discovery.ts` around lines 21 - 27, Export normalizeModelNameList from web/src/features/channels/lib/model-mapping-discovery.ts as the single shared definition. In web/src/features/channels/components/dialogs/fetch-models-dialog.tsx, remove the local duplicate and import normalizeModelNameList from ../../lib.
68-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueName the endpoint prefix.
'ep-'is a protocol-level marker shared with the Go implementation (controller/channel_upstream_update.goLines 218 and 273). Declare it once as a named constant in this module and export it, so the meaning is explicit at both use sites here.♻️ Proposed change
+const VOLCENGINE_ENDPOINT_ID_PREFIX = 'ep-' + export function mergeDiscoveredModelMapping(Then use
target.trim().startsWith(VOLCENGINE_ENDPOINT_ID_PREFIX).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/features/channels/lib/model-mapping-discovery.ts` around lines 68 - 76, Declare and export a module-level constant for the protocol endpoint prefix `ep-` in model-mapping-discovery.ts, then replace the inline string in the mapping cleanup condition with that constant. Use the named constant consistently at both relevant use sites in the module.web/src/features/channels/components/dialogs/fetch-models-dialog.tsx (1)
414-518: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider extracting the dialog body into a subcomponent.
The if/else chain removes the nested ternaries, which is an improvement. The file is now around 546 lines, well past the guideline threshold of roughly 200 lines for a component file. Extract the model selection body (search, tabs, summary) into its own component to keep this file focused on fetch and save orchestration.
As per coding guidelines: "组件文件超过约 200 行时，应考虑拆分子组件或提取自定义 Hook。"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/features/channels/components/dialogs/fetch-models-dialog.tsx` around lines 414 - 518, Extract the model-selection dialog body currently assigned through dialogBody into a focused subcomponent, including the search input, New/Existing/Removed tabs, category rendering, and selected-model summary. Pass the required state, handlers, translations, model collections, and renderModelCategory behavior as props, then render the subcomponent from the parent’s activeChannel/customFetcher and loading flow while keeping fetch/save orchestration in the parent component.Source: Coding guidelines
controller/channel_upstream_update_test.go (1)
144-177: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a BytePlus region case.
The table covers
ark.cn-beijing.volces.comand a custom host. It does not coverhttps://ark.ap-southeast.bytepluses.com, which the channel UI offers as a VolcEngine base URL. Add that case once the management host and signing region for that region are decided. See the comment oncontroller/channel_upstream_update.goLines 443-463.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/channel_upstream_update_test.go` around lines 144 - 177, Extend TestBuildVolcEngineManagementURL with a BytePlus ap-southeast base URL case, using the management host and signing region defined by the implementation requirements in channel_upstream_update.go. Assert the expected management URL preserves the BytePlus endpoint mapping and action/version query parameters.web/src/features/channels/components/drawers/channel-mutate-drawer.tsx (1)
2770-2792: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the literal channel type numbers with named constants.
Types
61and62appear at Lines 1282, 2770, 2780, 2786, and 2818. The file already imports named constants such asCHANNEL_TYPE_ADVANCED_CUSTOM. AddCHANNEL_TYPE_VOLCENGINE_AGENT_PLANandCHANNEL_TYPE_VOLCENGINE_CODING_PLANinweb/src/features/channels/constants.tsand use them here. The plan URLs are also duplicated inline; move them next to the constants.As per coding guidelines: "复杂逻辑应拆分为小函数，命名应有意义".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/features/channels/components/drawers/channel-mutate-drawer.tsx` around lines 2770 - 2792, Replace channel type literals 61 and 62 throughout the channel mutate drawer, including the currentType checks and related branches, with CHANNEL_TYPE_VOLCENGINE_AGENT_PLAN and CHANNEL_TYPE_VOLCENGINE_CODING_PLAN imported from the channel constants. Define these constants in constants.ts and move the plan-specific Anthropic and OpenAI URLs there, then reuse the named URL constants instead of inline duplicates.Source: Coding guidelines
controller/channel.go (1)
1320-1336: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the discovery response builder.
FetchUpstreamModels(Lines 243-260) andFetchModelsbuild the same response shape. A shared helper keeps thedataandmodel_mappingcontract in one place.♻️ Proposed helper
func writeUpstreamModelDiscoveryResponse(c *gin.Context, discovery upstreamModelDiscovery) { response := gin.H{"success": true, "message": "", "data": discovery.Models} if len(discovery.ModelMapping) > 0 { response["model_mapping"] = discovery.ModelMapping } c.JSON(http.StatusOK, response) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/channel.go` around lines 1320 - 1336, Extract the duplicated success-response construction from FetchUpstreamModels and FetchModels into a shared writeUpstreamModelDiscoveryResponse helper accepting gin.Context and upstreamModelDiscovery. Replace both inline response blocks with calls to the helper, preserving the conditional model_mapping field and existing response contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@constant/channel.go`:
- Around line 61-63: Update the ChannelTypeDummy constant in the channel type
declaration to an explicit unused sentinel value after
ChannelTypeVolcEngineCodingPlan, such as 63, so the controller/model.go
registration loop bounded by ChannelTypeDummy includes each channel exactly
once.
In `@controller/channel_upstream_update.go`:
- Around line 443-463: Update buildVolcEngineManagementURL and the related
fetchVolcEngineEndpointSource signing flow to recognize
https://ark.ap-southeast.bytepluses.com, mapping it to the BytePlus OpenAPI
management host and ap-southeast region instead of the cn-beijing VolcEngine
values. Preserve existing cn-beijing behavior, or explicitly reject endpoint
discovery only when a cn-beijing request is made against the BytePlus base URL.
- Around line 826-851: Update fetchChannelUpstreamModelDiscovery and its
downstream fetch flow to resolve channel.GetNextEnabledKey() only once and reuse
the same key, credential, and management-credential decision. Pass the resolved
values into fetchChannelUpstreamModelIDs or move the fetchVolcEngineEndpoints
branch there, ensuring multi-key and polling channels do not advance or select a
different key during the same discovery operation.
In `@web/src/features/channels/components/dialogs/fetch-models-dialog.tsx`:
- Around line 193-203: The empty discovered-model mapping currently prevents
endpoint mapping cleanup. In fetch-models-dialog.tsx (193-203), compute
nextModelMapping via mergeDiscoveredModelMapping unconditionally and include
model_mapping in updateChannel only when it differs from
activeChannel.model_mapping. In channel-mutate-drawer.tsx (4864-4883), likewise
call mergeDiscoveredModelMapping unconditionally and set model_mapping only when
the merged value differs from form.getValues('model_mapping').
In `@web/src/features/channels/components/drawers/channel-mutate-drawer.tsx`:
- Around line 1282-1288: The default model initialization in the currentType
61/62 branch incorrectly applies ark-code-latest to Agent Plan as well as Coding
Plan. Update the type-specific logic so type 62 retains its coding-plan default,
while type 61 uses the correct agent-plan model name or leaves models empty when
no verified default exists; preserve the existing non-empty models behavior.
- Around line 4864-4883: Update both form.setValue calls in the onModelsSelected
handler to pass shouldDirty: true and shouldValidate: true, matching the
existing form behavior for models and model_mapping updates.
In `@web/src/features/channels/lib/model-mapping-discovery.ts`:
- Around line 39-46: Update the currentMapping construction in the model-mapping
parsing flow to store trimmed keys and values, not just validate them with
trim(). Normalize both sides consistently with normalizeChannelModelMapping so
the cleanup loop comparing against previousSet recognizes padded aliases and
model names.
In `@web/src/features/usage-logs/components/dialogs/details-dialog.tsx`:
- Around line 780-785: Update the API DetailRow label in the details dialog to
use the existing t() translation function, and add the corresponding translation
key with “API” to every supported locale file where it is missing.
---
Outside diff comments:
In `@relay/channel/volcengine/adaptor.go`:
- Around line 330-336: Update the audio request authorization logic in the
RelayModeAudioSpeech branch of the request setup method to set the normalized
plan API key as the Bearer credential when ApiKey no longer contains management
credentials, while preserving existing parsed-key handling. Add regression
coverage for both plan channel types and verify their audio requests include
Authorization when GetRequestURL routes them to /v1/audio/speech.
In `@relay/common/relay_utils.go`:
- Around line 50-74: Update the URL sanitization flow around parsedURL.String()
to clear parsedURL.Fragment before returning the sanitized URL, ensuring
fragments containing credentials are removed from logs. Add a regression test
covering https://upstream.test/v1#token=secret and verify the resulting URL has
no fragment.
---
Nitpick comments:
In `@controller/channel_upstream_update_test.go`:
- Around line 144-177: Extend TestBuildVolcEngineManagementURL with a BytePlus
ap-southeast base URL case, using the management host and signing region defined
by the implementation requirements in channel_upstream_update.go. Assert the
expected management URL preserves the BytePlus endpoint mapping and
action/version query parameters.
In `@controller/channel_upstream_update.go`:
- Around line 223-225: Remove the len(nextMapping) == 0 conditional and its
reassignment, leaving the already initialized nextMapping unchanged.
- Around line 661-668: Guard the modelMapping assignment in the loop over
normalizeModelNames(inner.Models) by checking whether inner.ModelMapping
contains name before writing it; only store the mapped target when the lookup
succeeds, while preserving the existing built-in conflict precedence and
modelNames deduplication.
- Around line 1392-1405: The admin apply path should avoid repeating synchronous
VolcEngine discovery when persisted mappings already cover the selected
additions. Update the logic around fetchChannelUpstreamModelDiscovery and
applyDiscoveredModelMapping to reuse the channel’s persisted mapping, invoking
discovery only when at least one selected model lacks a mapping, while
preserving the existing mapping application and error propagation behavior.
- Around line 796-823: Update the model-list parsing after the VolcEngine
handling to route all remaining channel types through parseOpenAIModelIDs,
removing the duplicated response unmarshalling and normalization logic. Delete
the unreachable Gemini-specific models/ prefix trimming branch while preserving
the existing earlier Gemini handling and error behavior.
- Around line 238-248: Remove the redundant mapsEqual helper and use
maps.Equal(existingMapping, nextMapping) at its call sites instead. Add the
standard "maps" import and preserve the existing string-map comparison behavior.
In `@controller/channel.go`:
- Around line 1320-1336: Extract the duplicated success-response construction
from FetchUpstreamModels and FetchModels into a shared
writeUpstreamModelDiscoveryResponse helper accepting gin.Context and
upstreamModelDiscovery. Replace both inline response blocks with calls to the
helper, preserving the conditional model_mapping field and existing response
contract.
In `@relay/channel/volcengine/adaptor_test.go`:
- Around line 50-53: Add a test case for RelayModeImagesEdits with
ChannelTypeVolcEngineAgentPlan in the GetRequestURL test cases, expecting the
same Agent Plan images/generations URL as RelayModeImagesGenerations. Keep the
existing generation case unchanged and ensure both modes are explicitly covered.
In `@web/src/features/channels/components/dialogs/fetch-models-dialog.tsx`:
- Around line 414-518: Extract the model-selection dialog body currently
assigned through dialogBody into a focused subcomponent, including the search
input, New/Existing/Removed tabs, category rendering, and selected-model
summary. Pass the required state, handlers, translations, model collections, and
renderModelCategory behavior as props, then render the subcomponent from the
parent’s activeChannel/customFetcher and loading flow while keeping fetch/save
orchestration in the parent component.
In `@web/src/features/channels/components/drawers/channel-mutate-drawer.tsx`:
- Around line 2770-2792: Replace channel type literals 61 and 62 throughout the
channel mutate drawer, including the currentType checks and related branches,
with CHANNEL_TYPE_VOLCENGINE_AGENT_PLAN and CHANNEL_TYPE_VOLCENGINE_CODING_PLAN
imported from the channel constants. Define these constants in constants.ts and
move the plan-specific Anthropic and OpenAI URLs there, then reuse the named URL
constants instead of inline duplicates.
In `@web/src/features/channels/lib/__tests__/model-mapping-discovery.test.ts`:
- Around line 24-60: Add a test case in the mergeDiscoveredModelMapping suite
that passes malformed existing JSON, such as '{not json', with a selected
discovered endpoint, then assert the parsed result contains the discovered
mapping despite the invalid input.
In `@web/src/features/channels/lib/model-mapping-discovery.ts`:
- Around line 21-27: Export normalizeModelNameList from
web/src/features/channels/lib/model-mapping-discovery.ts as the single shared
definition. In
web/src/features/channels/components/dialogs/fetch-models-dialog.tsx, remove the
local duplicate and import normalizeModelNameList from ../../lib.
- Around line 68-76: Declare and export a module-level constant for the protocol
endpoint prefix `ep-` in model-mapping-discovery.ts, then replace the inline
string in the mapping cleanup condition with that constant. Use the named
constant consistently at both relevant use sites in the module.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 73870367-6682-4d2d-8453-7af721e03f8c
📒 Files selected for processing (44)
common/api_type.gocommon/endpoint_type.goconstant/channel.goconstant/context_key.gocontroller/channel-test.gocontroller/channel.gocontroller/channel_test_internal_test.gocontroller/channel_upstream_update.gocontroller/channel_upstream_update_test.gocontroller/relay.gorelay/channel/api_request.gorelay/channel/api_request_test.gorelay/channel/volcengine/adaptor.gorelay/channel/volcengine/adaptor_test.gorelay/channel/volcengine/sign.gorelay/channel/volcengine/sign_test.gorelay/common/relay_info.gorelay/common/relay_info_plan_test.gorelay/common/relay_info_test.gorelay/common/relay_utils.gorelay/common/relay_utils_test.goservice/log_info_generate.goservice/log_info_generate_test.goservice/task_billing.goweb/scripts/sync-i18n.mjsweb/src/features/channels/components/dialogs/fetch-models-dialog.tsxweb/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/src/features/channels/constants.tsweb/src/features/channels/lib/__tests__/model-mapping-discovery.test.tsweb/src/features/channels/lib/__tests__/volcengine-plan-channel.test.tsweb/src/features/channels/lib/channel-type-config.tsweb/src/features/channels/lib/channel-utils.tsweb/src/features/channels/lib/index.tsweb/src/features/channels/lib/model-mapping-discovery.tsweb/src/features/channels/types.tsweb/src/features/usage-logs/components/dialogs/details-dialog.tsxweb/src/features/usage-logs/types.tsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.json
| ChannelTypeVolcEngineAgentPlan = 61 | ||
| ChannelTypeVolcEngineCodingPlan = 62 | ||
| ChannelTypeDummy // this one is only for count, do not add any channel after this |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find usages of ChannelTypeDummy to assess whether its aliasing with
# ChannelTypeVolcEngineCodingPlan has functional impact.
rg -n -C3 --type=go '\bChannelTypeDummy\b'Repository: QuantumNous/new-api
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -u
echo "== candidate files =="
fd -a 'channel\.go$' . || true
echo "== ChannelType constants =="
rg -n -C4 'ChannelType(VolcEngineAgentPlan|VolcEngineCodingPlan|Dummy|NewAPI)|type\s+Channel' . || true
echo "== all symbol references =="
rg -n -C2 '\bChannelTypeDummy\b|\bChannelTypeVolcEngineCodingPlan\b' . || true
echo "== const block excerpt =="
if [ -f constant/channel.go ]; then
nl -ba constant/channel.go | sed -n '1,90p'
fi
echo "== programmatic const values for const block =="
python3 - <<'PY'
from pathlib import Path
p = Path('constant/channel.go')
if p.exists():
lines = p.read_text().splitlines()
in_block = False
vals = {}
for i,line in enumerate(lines,1):
s=line.strip()
if s.startswith('const ('):
in_block=True
continue
if in_block and s == ')':
break
if in_block and '=' in s:
name = s.split()[0]
_, rest = s.split('=',1)
vals[name]=rest.strip()
elif in_block and s and not s.startswith('//') and not s.startswith('*'):
vals[s.split()[0]]=vals[list(vals)[-1]][0] if vals else None
for name in ['ChannelTypeVolcEngineAgentPlan','ChannelTypeVolcEngineCodingPlan','ChannelTypeDummy']:
print(name, vals.get(name))
PYRepository: QuantumNous/new-api
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -u
echo "== all ChannelTypeDummy references with grep =="
grep -RInwo '\bChannelTypeDummy\b' . || true
echo "== controller/model.go relevant loop =="
nl -ba controller/model.go | sed -n '70,110p'
echo "== constants surrounding ChannelType constants =="
nl -ba constant/channel.go | sed -n '44,66p'Repository: QuantumNous/new-api
Length of output: 452
Replace the ChannelTypeDummy alias before using it as a count bound.
ChannelTypeDummy still inherits the previous const value, so it equals constant.ChannelTypeVolcEngineCodingPlan (62). Since controller/model.go iterates for i := 1; i <= constant.ChannelTypeDummy; i++, each route registration includes 62 twice. Give ChannelTypeDummy an unused sentinel value (for example, 63) so the loop count remains correct.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@constant/channel.go` around lines 61 - 63, Update the ChannelTypeDummy
constant in the channel type declaration to an explicit unused sentinel value
after ChannelTypeVolcEngineCodingPlan, such as 63, so the controller/model.go
registration loop bounded by ChannelTypeDummy includes each channel exactly
once.
| func buildVolcEngineManagementURL(baseURL string, action string) (*url.URL, error) { | ||
| parsedBaseURL, err := url.Parse(baseURL) | ||
| if err != nil || parsedBaseURL.Scheme == "" || parsedBaseURL.Host == "" { | ||
| return nil, fmt.Errorf("invalid VolcEngine base URL: %s", baseURL) | ||
| } | ||
| managementScheme := parsedBaseURL.Scheme | ||
| managementHost := parsedBaseURL.Host | ||
| if parsedBaseURL.Hostname() == "ark.cn-beijing.volces.com" { | ||
| managementScheme = "https" | ||
| managementHost = "ark.cn-beijing.volcengineapi.com" | ||
| } | ||
| return &url.URL{ | ||
| Scheme: managementScheme, | ||
| Host: managementHost, | ||
| Path: "/", | ||
| RawQuery: url.Values{ | ||
| "Action": []string{action}, | ||
| "Version": []string{"2024-01-01"}, | ||
| }.Encode(), | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
VolcEngine Ark BytePlus ap-southeast OpenAPI management endpoint host and region for ListEndpoints
💡 Result:
For the BytePlus ModelArk ListEndpoints operation in the ap-southeast-1 region, the OpenAPI management endpoint host is ark.ap-southeast-1.byteplusapi.com [1][2]. The region is ap-southeast-1 [1][3].
Citations:
- 1: https://docs.byteplus.com/en/docs/ModelArk/1262430
- 2: https://docs.byteplus.com/en/docs/modelark/1262430
- 3: https://docs.byteplus.com/en/docs/ModelArk/2191806
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files matching channel-upstream =="
fd -a 'channel_upstream_update.go|channel-mutate-drawer.tsx' . | sed 's#^\./##'
echo
echo "== target function context =="
file=$(fd 'channel_upstream_update.go' . | head -n1)
if [ -n "${file:-}" ]; then
sed -n '410,730p' "$file" | nl -ba -v410
fi
echo
echo "== drawer URL references =="
drawer=$(fd 'channel-mutate-drawer.tsx' . | head -n1)
if [ -n "${drawer:-}" ]; then
rg -n -C 3 'ark\.ap-southeast\.bytepluses\.com|ark\.cn-beijing\.volces\.com|VolcEngine|volc|byteplus' "$drawer"
fi
echo
echo "== all Volc/BytePlus/OpenAPI region references =="
rg -n 'ark\.ap-southeast|ark\.cn-beijing|bytepluses|byteplusapi|byte\.com|cn-beijing|ap-southeast|ap\.southeast|volcengineapi|Region|RegionID|RegionId' .Repository: QuantumNous/new-api
Length of output: 419
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=$(fd 'channel_upstream_update.go' . | head -n1)
drawer=$(fd 'channel-mutate-drawer.tsx' . | head -n1)
echo "== target function context =="
sed -n '410,730p' "$file"
echo
echo "== drawer URL references =="
rg -n -C 4 'ark\.ap-southeast\.bytepluses\.com|ark\.cn-beijing\.volces\.com|VolcEngine|volc|byteplus' "$drawer"Repository: QuantumNous/new-api
Length of output: 17687
Map the VolcEngine BytePlus base URL to the management host and region.
When the base URL is https://ark.ap-southeast.bytepluses.com, buildVolcEngineManagementURL returns ark.ap-southeast.bytepluses.com/?Action=... instead of the BytePlus OpenAPI host. fetchVolcEngineEndpointSource also signs ListEndpoints and plan discovery with cn-beijing, which does not match the BytePlus ap-southeast region. Map this base URL to its BytePlus OpenAPI endpoint/region, or reject supported endpoint discovery only for explicit cn-beijing requests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/channel_upstream_update.go` around lines 443 - 463, Update
buildVolcEngineManagementURL and the related fetchVolcEngineEndpointSource
signing flow to recognize https://ark.ap-southeast.bytepluses.com, mapping it to
the BytePlus OpenAPI management host and ap-southeast region instead of the
cn-beijing VolcEngine values. Preserve existing cn-beijing behavior, or
explicitly reject endpoint discovery only when a cn-beijing request is made
against the BytePlus base URL.
| func fetchChannelUpstreamModelDiscovery(channel *model.Channel) (upstreamModelDiscovery, error) { | ||
| if channel != nil && channel.Type == constant.ChannelTypeVolcEngine { | ||
| key, _, apiErr := channel.GetNextEnabledKey() | ||
| if apiErr != nil { | ||
| return upstreamModelDiscovery{}, fmt.Errorf("获取渠道密钥失败: %w", apiErr) | ||
| } | ||
| key = strings.TrimSpace(key) | ||
| credential, err := volcengine.ParseAPIKeyCredential(key) | ||
| if err != nil { | ||
| return upstreamModelDiscovery{}, sanitizeFetchModelsError(err, key) | ||
| } | ||
| if credential.HasManagementCredential() { | ||
| baseURL := constant.ChannelBaseURLs[channel.Type] | ||
| if channel.GetBaseURL() != "" { | ||
| baseURL = channel.GetBaseURL() | ||
| } | ||
| return fetchVolcEngineEndpoints(channel, baseURL, key) | ||
| } | ||
| } | ||
|
|
||
| models, err := fetchChannelUpstreamModelIDs(channel) | ||
| if err != nil { | ||
| return upstreamModelDiscovery{}, err | ||
| } | ||
| return upstreamModelDiscovery{Models: models}, nil | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Determine whether GetNextEnabledKey rotates keys between calls.
ast-grep run --pattern 'func ($_ *Channel) GetNextEnabledKey($$$) { $$$ }' --lang go model
rg -n -C5 'func .*GetNextEnabledKey' --type=goRepository: QuantumNous/new-api
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -u
echo "Tracked files named channel:"
git ls-files | rg '(^|/)channel.*\.go$|(^|/)channel.*\.ts$|(^|/)channel.*\.tsx$' | head -200
echo
echo "Find candidate Go files for GetNextEnabledKey:"
rg -n "GetNextEnabledKey|type Channel|func .*Channel" --type go . \
| head -300
echo
echo "Locate channel model/controller files:"
git ls-files | rg 'controller/channel_upstream_update|model/channel|channel.*\.go$' | head -200
echo
echo "Search channel.go definitions:"
for f in $(git ls-files | rg 'channel\.go$' | head -50); do
echo "--- $f"
wc -l "$f"
rg -n "type Channel|func .*GetNextEnabledKey|GetNextEnabledKey" "$f" | head -80
doneRepository: QuantumNous/new-api
Length of output: 46077
🏁 Script executed:
#!/bin/bash
set -u
echo "--- model/channel.go GetKeys/GetNextEnabledKey/locking/update lines 175-260"
sed -n '175,260p' model/channel.go | cat -n
echo
echo "--- controller/channel_upstream_update.go fetchChannelUpstreamModelIDs/fetchVolcEngine/fetchChannelUpstreamModelDiscovery lines 634-851"
sed -n '634,851p' controller/channel_upstream_update.go | cat -n
echo
echo "--- read-only semantic check for duplicate rotated key calls in VolcEngine upstream path"
python3 - <<'PY'
import re
from pathlib import Path
channel_update = Path('controller/channel_upstream_update.go').read_text()
model_channel = Path('model/channel.go').read_text()
patt = re.compile(r'controller/channel_upstream_update.go:\d+(?:-\d+)?:[\s\S]+?)\nfunc |$')
func = re.search(r'(?ms)^func fetchChannelUpstreamModelDiscovery\(channel \*model\.Channel\).*?\n^\}', channel_update)
ids = re.search(r'(?ms)^func fetchChannelUpstreamModelIDs\(channel \*model\.Channel\).*?\n^\}', channel_update)
print("func fetchChannelUpstreamModelDiscovery found:", bool(func))
print("func fetchChannelUpstreamModelIDs found:", bool(ids))
if func:
text = func.group().splitlines()
starts = next(i for i,l in enumerate(text) if 'func fetchChannelUpstreamModelDiscovery' in l)+1
line_no = 1
calls = []
for line in text:
if 'GetNextEnabledKey()' in line or 'ParseAPIKeyCredential(key)' in line or 'HasManagementCredential()' in line:
calls.append((line_no, line.strip()))
line_no += 1
print("fetchChannelUpstreamModelDiscovery relevant calls:")
for call in calls:
print(f" {call[0]}: {call[1]}")
fetch_call = [l for line_no,l in calls if 'GetNextEnabledKey()' in l]
print("fetchChannelUpstreamModelDiscovery GetNextEnabledKey calls:", len(fetch_call))
if ids:
text = ids.group().splitlines()
line_no = 1
calls = []
for line in text:
if 'GetNextEnabledKey()' in line or 'ParseAPIKeyCredential(key)' in line or 'HasManagementCredential()' in line:
calls.append((line_no, line.strip()))
line_no += 1
print("fetchChannelUpstreamModelIDs relevant calls:")
for call in calls:
print(f" {call[0]}: {call[1]}")
getkeys = re.search(r'(?ms)^func \(channel \*Channel\) GetKeys\(\).*?\n^\}', model_channel)
getnext = re.search(r'(?ms)^func \(channel \*Channel\) GetNextEnabledKey\(\).*?\n^\}', model_channel)
print("GetKeys found:", bool(getkeys))
print("GetNextEnabledKey found:", bool(getnext))
if getkeys and getnext:
print("GetKeys source:")
print(getkeys.group())
print("\nGetNextEnabledKey source:")
print(getnext.group())
PYRepository: QuantumNous/new-api
Length of output: 13680
🏁 Script executed:
#!/bin/bash
set -u
echo "--- model/channel.go GetNextEnabledKey polling continuation lines 220-250"
sed -n '220,250p' model/channel.go | cat -n
echo
echo "--- model/channel.go locks and multi-key update/status references lines 617-655"
sed -n '617,655p' model/channel.go | cat -n
sed -n '700,735p' model/channel.go | cat -n
echo
echo "--- deterministic multi-key rotation probe from source logic"
python3 - <<'PY'
import re
from pathlib import Path
model = Path('model/channel.go').read_text()
discovery = Path('controller/channel_upstream_update.go').read_text()
getnext = re.search(r'(?ms)^func \(channel \*Channel\) GetNextEnabledKey\(\).*?\n^\}', model)
fetch_chain = re.search(r'(?ms)^func fetchChannelUpstreamModelDiscovery\(channel \*model\.Channel\).*?\n^\}', discovery)
ids = re.search(r'(?ms)^func fetchChannelUpstreamModelIDs\(channel \*model\.Channel\).*?\n^\}', discovery)
print("GetNextEnabledKey IsMultiKey branch:", bool(re.search(r'if !channel\.ChannelInfo\.IsMultiKey \{[\s\S]*?return channel\.Key, 0, nil', getnext.group() if getnext else '')))
print("GetNextEnabledKey Random branch:", bool(re.search(r'constant\.MultiKeyModeRandom:[^}]*(?:selectedIdx|rand\.Intn)', getnext.group() if getnext else '')))
print("GetNextEnabledKey Polling branch status update:", bool(re.search(r'case constant\.MultiKeyModePolling:.*?MultiKeyPollingIndex\s*=\s*\([^)]*MultiKeyPollingIndex', re.search(r'(?ms)^func \(channel \*Channel\) GetNextEnabledKey\(\).*?\n^\}', model).group() if re.search(r'(?ms)^func \(channel \*Channel\) GetNextEnabledKey\(\).*?\n^\}', model) else '')))
print("fetchChannelUpstreamModelDiscovery Volcengine GetNextEnabledKey count:", re.findall(r'channel\.GetNextEnabledKey\(\)', fetch_chain.group() if fetch_chain else ''))
print("fetchChannelUpstreamModelDiscovery Volcengine ParseAPIKeyCredential count:", re.findall(r'volcengine\.ParseAPIKeyCredential\(key\)', fetch_chain.group() if fetch_chain else ''))
print("fetchChannelUpstreamModelDiscovery Volcengine HasManagementCredential count:", re.findall(r'HasManagementCredential\(\)', fetch_chain.group() if fetch_chain else ''))
print("fetchChannelUpstreamModelIDs Volcengine GetNextEnabledKey count:", re.findall(r'channel\.GetNextEnabledKey\(\)', ids.group() if ids else ''))
print("fetchChannelUpstreamModelIDs Volcengine ParseAPIKeyCredential count:", re.findall(r'volcengine\.ParseAPIKeyCredential\(key\)', ids.group() if ids else ''))
print("fetchChannelUpstreamModelIDs Volcengine HasManagementCredential count:", re.findall(r'HasManagementCredential\(\)', ids.group() if ids else ''))
PYRepository: QuantumNous/new-api
Length of output: 5194
🏁 Script executed:
#!/bin/bash
set -u
echo "--- GetNextEnabledKey complete polling return/update"
python3 - <<'PY'
import re
from pathlib import Path
s = Path('model/channel.go').read_text()
m = re.search(r'(?ms)^func \(channel \*Channel\) GetNextEnabledKey\(\).*?\n^\}', s)
block = m.group().splitlines()
for i, line in enumerate(block, 1):
if i >= 20 and i <= 110:
print(f"{23+i}: {line}")
PY
echo
echo "--- static count of getnext calls and credential branches in Volc upstream discovery chain"
python3 - <<'PY'
import re
from pathlib import Path
s = Path('controller/channel_upstream_update.go').read_text()
discovery = re.search(r'(?ms)^func fetchChannelUpstreamModelDiscovery\(channel \*model\.Channel\).*?\n^\}', s)
ids = re.search(r'(?ms)^func fetchChannelUpstreamModelIDs\(channel \*model\.Channel\).*?\n^\}', s)
patt_getnext = re.compile(r'channel\.GetNextEnabledKey\(\)')
patt_parse = re.compile(r'volcengine\.ParseAPIKeyCredential\(key\)')
patt_mgmt = re.compile(r'HasManagementCredential\(\)')
if discovery:
print("fetchChannelUpstreamModelDiscovery")
print("GetNextEnabledKey", len(patt_getnext.findall(discovery.group())))
print("ParseAPIKeyCredential", len(patt_parse.findall(discovery.group())))
print("HasManagementCredential", len(patt_mgmt.findall(discovery.group())))
if ids:
print("fetchChannelUpstreamModelIDs")
print("GetNextEnabledKey", len(patt_getnext.findall(ids.group())))
print("ParseAPIKeyCredential", len(patt_parse.findall(ids.group())))
print("HasManagementCredential", len(patt_mgmt.findall(ids.group())))
PYRepository: QuantumNous/new-api
Length of output: 3073
Resolve the VolcEngine key once and reuse it.
fetchChannelUpstreamModelDiscovery parses channel.GetNextEnabledKey() and checks HasManagementCredential(), then discards that result and calls fetchChannelUpstreamModelIDs, which calls GetNextEnabledKey() again and repeats the same management branch. Multi-key channels can return a different key between these calls, and polling channels update the current polling index on each call. Resolve the key once and pass it down, or move fetchVolcEngineEndpoints into fetchChannelUpstreamModelIDs and reuse that result.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/channel_upstream_update.go` around lines 826 - 851, Update
fetchChannelUpstreamModelDiscovery and its downstream fetch flow to resolve
channel.GetNextEnabledKey() only once and reuse the same key, credential, and
management-credential decision. Pass the resolved values into
fetchChannelUpstreamModelIDs or move the fetchVolcEngineEndpoints branch there,
ensuring multi-key and polling channels do not advance or select a different key
during the same discovery operation.
| const nextModelMapping = mergeDiscoveredModelMapping( | ||
| activeChannel.model_mapping, | ||
| discoveredModelMapping, | ||
| selectedModels, | ||
| existingModels | ||
| ) | ||
| const response = await updateChannel(activeChannel.id, { | ||
| models: modelsString, | ||
| ...(Object.keys(discoveredModelMapping).length > 0 | ||
| ? { model_mapping: nextModelMapping } | ||
| : {}), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The empty-discovered-mapping guard skips endpoint mapping cleanup in both frontend write paths. mergeDiscoveredModelMapping performs two jobs: it adds discovered ep- mappings for selected models, and it removes ep- mappings for models the user deselected. Both call sites gate the whole merge on Object.keys(discoveredModelMapping).length > 0, so when a fetch returns no mapping the removal never reaches the channel. Stale ep- entries then point at endpoints for models the channel no longer serves. The backend runs the equivalent removal unconditionally in applyDiscoveredModelMapping (controller/channel_upstream_update.go Lines 1400-1404).
web/src/features/channels/components/dialogs/fetch-models-dialog.tsx#L193-L203: computenextModelMappingfirst, then includemodel_mappingin theupdateChannelpayload whenever it differs fromactiveChannel.model_mapping, instead of gating on a non-emptydiscoveredModelMapping.web/src/features/channels/components/drawers/channel-mutate-drawer.tsx#L4864-L4883: callmergeDiscoveredModelMappingunconditionally and setmodel_mappingwhen the merged value differs fromform.getValues('model_mapping'), instead of skipping the merge whendiscoveredModelMappingis empty.
📍 Affects 2 files
web/src/features/channels/components/dialogs/fetch-models-dialog.tsx#L193-L203(this comment)web/src/features/channels/components/drawers/channel-mutate-drawer.tsx#L4864-L4883
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/channels/components/dialogs/fetch-models-dialog.tsx` around
lines 193 - 203, The empty discovered-model mapping currently prevents endpoint
mapping cleanup. In fetch-models-dialog.tsx (193-203), compute nextModelMapping
via mergeDiscoveredModelMapping unconditionally and include model_mapping in
updateChannel only when it differs from activeChannel.model_mapping. In
channel-mutate-drawer.tsx (4864-4883), likewise call mergeDiscoveredModelMapping
unconditionally and set model_mapping only when the merged value differs from
form.getValues('model_mapping').
| if (currentType === 61 || currentType === 62) { | ||
| form.setValue('base_url', '') | ||
| if (!form.getValues('models').trim()) { | ||
| form.setValue('models', 'ark-code-latest') | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Verify the seeded default model for the agent plan.
Lines 1284-1286 seed ark-code-latest for both type 61 (Agent Plan) and type 62 (Coding Plan). The backend discovery uses different actions per type: ListArkAgentPlanModel for 61 and ListArkCodingPlanModel for 62 (controller/channel_upstream_update.go Lines 684-687). ark-code-latest reads as a coding-plan model name.
If the agent plan exposes a different model, seed the correct name for type 61, or seed nothing and require the user to fetch models.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/channels/components/drawers/channel-mutate-drawer.tsx`
around lines 1282 - 1288, The default model initialization in the currentType
61/62 branch incorrectly applies ark-code-latest to Agent Plan as well as Coding
Plan. Update the type-specific logic so type 62 retains its coding-plan default,
while type 61 uses the correct agent-plan model name or leaves models empty when
no verified default exists; preserve the existing non-empty models behavior.
| onModelsSelected={(models, discoveredModelMapping) => { | ||
| const previousModels = parseModelsString( | ||
| form.getValues('models') || '' | ||
| ) | ||
| form.setValue('models', formatModelsArray(models)) | ||
| if ( | ||
| discoveredModelMapping && | ||
| Object.keys(discoveredModelMapping).length > 0 | ||
| ) { | ||
| form.setValue( | ||
| 'model_mapping', | ||
| mergeDiscoveredModelMapping( | ||
| form.getValues('model_mapping'), | ||
| discoveredModelMapping, | ||
| models, | ||
| previousModels | ||
| ) | ||
| ) | ||
| } | ||
| }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Set shouldDirty and shouldValidate on these setValue calls.
Other form.setValue calls in this file pass { shouldDirty: true, shouldValidate: true } (Lines 794-801, 1301-1304, 4838-4841). These two calls omit the options, so the form does not mark models or model_mapping as dirty and does not revalidate them after the dialog fills them.
🐛 Proposed fix
- form.setValue('models', formatModelsArray(models))
+ form.setValue('models', formatModelsArray(models), {
+ shouldDirty: true,
+ shouldValidate: true,
+ })
if (
discoveredModelMapping &&
Object.keys(discoveredModelMapping).length > 0
) {
form.setValue(
'model_mapping',
mergeDiscoveredModelMapping(
form.getValues('model_mapping'),
discoveredModelMapping,
models,
previousModels
- )
+ ),
+ { shouldDirty: true, shouldValidate: true }
)
}📝 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.
| onModelsSelected={(models, discoveredModelMapping) => { | |
| const previousModels = parseModelsString( | |
| form.getValues('models') || '' | |
| ) | |
| form.setValue('models', formatModelsArray(models)) | |
| if ( | |
| discoveredModelMapping && | |
| Object.keys(discoveredModelMapping).length > 0 | |
| ) { | |
| form.setValue( | |
| 'model_mapping', | |
| mergeDiscoveredModelMapping( | |
| form.getValues('model_mapping'), | |
| discoveredModelMapping, | |
| models, | |
| previousModels | |
| ) | |
| ) | |
| } | |
| }} | |
| onModelsSelected={(models, discoveredModelMapping) => { | |
| const previousModels = parseModelsString( | |
| form.getValues('models') || '' | |
| ) | |
| form.setValue('models', formatModelsArray(models), { | |
| shouldDirty: true, | |
| shouldValidate: true, | |
| }) | |
| if ( | |
| discoveredModelMapping && | |
| Object.keys(discoveredModelMapping).length > 0 | |
| ) { | |
| form.setValue( | |
| 'model_mapping', | |
| mergeDiscoveredModelMapping( | |
| form.getValues('model_mapping'), | |
| discoveredModelMapping, | |
| models, | |
| previousModels | |
| ), | |
| { shouldDirty: true, shouldValidate: true } | |
| ) | |
| } | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/channels/components/drawers/channel-mutate-drawer.tsx`
around lines 4864 - 4883, Update both form.setValue calls in the
onModelsSelected handler to pass shouldDirty: true and shouldValidate: true,
matching the existing form behavior for models and model_mapping updates.
| currentMapping = Object.fromEntries( | ||
| Object.entries(parsed).filter( | ||
| (entry): entry is [string, string] => | ||
| typeof entry[1] === 'string' && | ||
| entry[0].trim() !== '' && | ||
| entry[1].trim() !== '' | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Trim the retained keys and values.
The filter tests entry[0].trim() and entry[1].trim(), but the entry is stored untrimmed. A stored mapping of {" alias ": " ep-x "} therefore keeps the padding. The removal loop at Lines 68-76 compares previousSet, which holds normalized names, against these untrimmed keys, so the cleanup misses them.
The backend normalizes both sides in normalizeChannelModelMapping (controller/channel_upstream_update.go Lines 171-179). Match that behavior.
🐛 Proposed fix
currentMapping = Object.fromEntries(
Object.entries(parsed)
.filter(
(entry): entry is [string, string] =>
typeof entry[1] === 'string' &&
entry[0].trim() !== '' &&
entry[1].trim() !== ''
)
+ .map(([source, target]) => [source.trim(), target.trim()])
)📝 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.
| currentMapping = Object.fromEntries( | |
| Object.entries(parsed).filter( | |
| (entry): entry is [string, string] => | |
| typeof entry[1] === 'string' && | |
| entry[0].trim() !== '' && | |
| entry[1].trim() !== '' | |
| ) | |
| ) | |
| currentMapping = Object.fromEntries( | |
| Object.entries(parsed) | |
| .filter( | |
| (entry): entry is [string, string] => | |
| typeof entry[1] === 'string' && | |
| entry[0].trim() !== '' && | |
| entry[1].trim() !== '' | |
| ) | |
| .map(([source, target]) => [source.trim(), target.trim()]) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/channels/lib/model-mapping-discovery.ts` around lines 39 -
46, Update the currentMapping construction in the model-mapping parsing flow to
store trimmed keys and values, not just validate them with trim(). Normalize
both sides consistently with normalizeChannelModelMapping so the cleanup loop
comparing against previousSet recognizes padded aliases and model names.
| {requestMetadata.api_type != null && ( | ||
| <DetailRow | ||
| label='API' | ||
| value={String(requestMetadata.api_type)} | ||
| mono | ||
| /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Translate the API label.
Line 782 renders a user-facing literal. Route this label through t() and add the key to the supported locale files if it does not already exist.
Proposed fix
- label='API'
+ label={t('API')}📝 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.
| {requestMetadata.api_type != null && ( | |
| <DetailRow | |
| label='API' | |
| value={String(requestMetadata.api_type)} | |
| mono | |
| /> | |
| {requestMetadata.api_type != null && ( | |
| <DetailRow | |
| label={t('API')} | |
| value={String(requestMetadata.api_type)} | |
| mono | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/usage-logs/components/dialogs/details-dialog.tsx` around
lines 780 - 785, Update the API DetailRow label in the details dialog to use the
existing t() translation function, and add the corresponding translation key
with “API” to every supported locale file where it is missing.
Source: Coding guidelines
|
提交PR请先看模版，不接受coding plan |
2 participants
变更说明 / Description\n\n新增 VolcEngine Agent Plan 与 Coding Plan 独立渠道，复用现有适配器并固定官方路由，避免套餐请求落到普通方舟 API。\n\n同时补充 Endpoint 模型发现与名称到推理 Endpoint 的映射：过滤 Stopped 状态，自定义与内置 Endpoint 按模型名称去重且内置优先。日志详情会记录脱敏后的上游请求地址与方法，并修复 Agent Plan 图片生成路由。\n\n## 变更类型 / Type of change\n\n- [x] 新功能 (New feature)\n- [x] Bug 修复 (Bug fix)\n\n## 关联任务 / Related Issue\n\n- VolcEngine Agent Plan / Coding Plan support\n\n## ✅ 提交前检查项 / Checklist\n\n- [x] 人工确认：已逐项审阅变更与冲突合并结果。\n- [x] 非重复提交：已检查现有 Issues 与 PRs。\n- [x] 变更理解：已确认渠道路由、鉴权、模型发现及映射优先级。\n- [x] 范围聚焦：分支基于上游 main，仅包含本功能相关改动。\n- [x] 本地验证：针对性 Go 测试、前端类型检查、变更文件格式检查与 i18n 同步均已通过。\n- [x] 安全合规：管理凭据不会写入日志，上游 URL 会在持久化前脱敏。\n\n## 验证 / Verification\n\n- Messages、Chat Completions、Responses 与 Agent Plan Images 路由测试通过。\n- Endpoint 签名、分页、Stopped 过滤、内置优先去重及模型映射测试通过。\n- 普通 VolcEngine 路由与旧 Coding Plan 配置兼容测试通过。\n- 七种语言同步报告均为零缺失、零额外项、零未翻译项。
Summary by CodeRabbit
New Features
Bug Fixes