feat: Image-Aware Model Routing - #5593
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an image-aware model routing feature to the gateway. A virtual entry model name is mapped to either a vision model or coding model based on whether the last ChangesImage-Aware Model Routing
CI and Deployment Infrastructure
Sequence Diagram(s)sequenceDiagram
participant Client
participant Distribute as Distribute()
participant IAAR as ApplyImageAwareRouting
participant GetRule as GetImageAwareRouteRule
participant Detect as detectImageInLastUserMessage
participant Parse as hasImageInLastUserMessage
participant ChannelSel as Channel Selection
participant RelayHandler as OpenAI/Claude Handler
participant RouteHint as RouteHint()
Client->>Distribute: request (model=auto-coder)
Distribute->>IAAR: modelRequest
IAAR->>GetRule: "auto-coder"
GetRule-->>IAAR: {VisionModel: glm-4v, CodingModel: glm-4}
IAAR->>Detect: request body bytes
Detect->>Parse: raw JSON body
Parse-->>Detect: image: true
Detect-->>IAAR: true
IAAR->>IAAR: modelRequest.Model = "glm-4v"<br/>set context keys
IAAR-->>Distribute: model rewritten
Distribute->>ChannelSel: model: "glm-4v"
ChannelSel-->>Distribute: channel selected
Distribute->>RelayHandler: forward request
RelayHandler->>RouteHint: c, RelayInfo
RouteHint-->>RelayHandler: "> [Route: auto-coder → glm-4v (image detected)]"
RelayHandler->>Client: response with hint prefix<br/>+ X-Routed-Model headers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/image-aware-routing.md (1)
116-119:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate log-display wording to match current behavior.
This section says the entry model is not shown in the log page, but this PR stack includes entry/actual model display in usage logs. Please align the docs to avoid operator confusion.
📝 Suggested doc diff
-- 如需在日志中追溯入口名,可查看 context 中的 `image_aware_entry_model` 字段(当前版本仅在内部记录,未在日志页面显示) +- 日志页面会显示实际模型;在启用路由信息展示时,也可查看入口模型与路由结果(同时 `image_aware_entry_model` 仍会记录在上下文元数据中)🤖 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 `@docs/image-aware-routing.md` around lines 116 - 119, Update the documentation in the image-aware-routing.md file to reflect the current logging behavior. The current text states that the image_aware_entry_model field is not displayed in the log page ("未在日志页面显示"), but this PR adds entry/actual model display to usage logs. Replace or modify the clause that says the entry model information is not shown in the log page to indicate that it is now available in the usage logs, ensuring operators are aware they can view both the entry model and actual model in the logs.
🧹 Nitpick comments (4)
web/default/src/features/keys/types.ts (1)
45-52: Inconsistent boolean preprocessing across the schema.The numeric preprocess pattern (converting
1→true,0→false) is also used forcross_group_retry, but other boolean fields likeunlimited_quotaandmodel_limits_enableddon't use it. This inconsistency suggests these specific fields may handle a backend behavior that others don't, or the preprocessing is overly defensive. Verify ifmodel_route_notifyactually receives numeric values from the backend, and if so, ensure consistent preprocessing across all affected boolean fields.🤖 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/default/src/features/keys/types.ts` around lines 45 - 52, The boolean preprocessing pattern that converts numeric values (1 to true, 0 to false) is applied inconsistently across the schema. The fields model_route_notify and cross_group_retry use this numeric preprocessing, while unlimited_quota and model_limits_enabled do not. Verify whether model_route_notify actually receives numeric values from the backend. If it does, apply the same numeric preprocessing pattern to unlimited_quota and model_limits_enabled to ensure consistency. If model_route_notify does not receive numeric values from the backend, remove the unnecessary numeric preprocessing from both model_route_notify and cross_group_retry to keep the schema clean and maintainable.setting/operation_setting/image_aware_routing.go (1)
19-21: ⚡ Quick winHide
ImageAwareModelRoutingbehind accessors to preserve lock guarantees.Exporting a mutable
mapallows external packages to bypassimageAwareModelRoutingLock, which can introduce races/panics under concurrent reads/writes. Prefer keeping the map unexported and exposing only locked helper methods.♻️ Suggested encapsulation diff
-var ImageAwareModelRouting = map[string]ImageAwareRouteRule{} +var imageAwareModelRouting = map[string]ImageAwareRouteRule{} func ImageAwareModelRouting2JSONString() string { imageAwareModelRoutingLock.RLock() defer imageAwareModelRoutingLock.RUnlock() - data, err := common.Marshal(ImageAwareModelRouting) + data, err := common.Marshal(imageAwareModelRouting) if err != nil { return "{}" } return string(data) } func UpdateImageAwareModelRoutingByJSONString(value string) error { newMap := make(map[string]ImageAwareRouteRule) if value != "" { if err := common.Unmarshal([]byte(value), &newMap); err != nil { return err } } imageAwareModelRoutingLock.Lock() - ImageAwareModelRouting = newMap + imageAwareModelRouting = newMap imageAwareModelRoutingLock.Unlock() return nil } func GetImageAwareRouteRule(model string) (ImageAwareRouteRule, bool) { imageAwareModelRoutingLock.RLock() defer imageAwareModelRoutingLock.RUnlock() - rule, ok := ImageAwareModelRouting[model] + rule, ok := imageAwareModelRouting[model] return rule, ok }🤖 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 `@setting/operation_setting/image_aware_routing.go` around lines 19 - 21, The exported variable ImageAwareModelRouting allows direct access to the map by external packages, bypassing the imageAwareModelRoutingLock mutex and risking race conditions. Change ImageAwareModelRouting to unexported (rename to imageAwareModelRouting with lowercase) and create exported accessor methods (such as GetImageAwareRouting, SetImageAwareRouting, or similar) that properly acquire and release the imageAwareModelRoutingLock when reading or writing to the map. This ensures all access to the map is protected by the mutex.docs/PR-description.md (2)
42-44: ⚡ Quick winAdd language specifier to code block.
For better syntax highlighting and documentation clarity, specify a language for the code block.
📝 Suggested fix
-``` +```text image_aware_routing: entry=auto-coder has_image=true -> routed=glm-4.6v notify=true</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/PR-description.mdaround lines 42 - 44, The code block containing the
image_aware_routing example is missing a language specifier. Add "text" as the
language specifier to the opening backticks of the code block that contains
"image_aware_routing: entry=auto-coder has_image=true -> routed=glm-4.6v
notify=true". Change the opening triple backticks fromtotext to enable
proper syntax highlighting and improve documentation clarity.</details> <!-- cr-comment:v1:a656a72efa703b23601b5ded --> --- `35-39`: _⚡ Quick win_ **Add language specifier to code block.** For better syntax highlighting and documentation clarity, specify a language for the code block. <details> <summary>📝 Suggested fix</summary> ```diff -``` +```text model_name=glm-4.6v prompt_tokens=41355 image_aware_entry_model=auto-coder // 含图 → 视觉模型 model_name=glm-5 prompt_tokens=79 image_aware_entry_model=auto-coder // 纯文本 → 编程模型 model_name=glm-5 prompt_tokens=41188 image_aware_entry_model=auto-coder // 后续纯文本轮,仍回编程模型 ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/PR-description.mdaround lines 35 - 39, The code block starting at line
35 in docs/PR-description.md is missing a language specifier. Add the language
identifier "text" immediately after the opening triple backticks (change ``` toas shown in the suggested fix.
🤖 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 `@relay/channel/claude/relay-claude.go`:
- Around line 955-966: The code at line 965 uses the standard library
`json.Marshal` directly when handling the openaiResponse object, but the project
has a standardized JSON marshalling contract through `common.Marshal()` defined
in common/json.go. Replace the `json.Marshal(openaiResponse)` call with
`common.Marshal(openaiResponse)` to ensure consistency with the project's JSON
wrapper contract.
In `@relay/compatible_handler.go`:
- Around line 198-202: In the compatible_handler.go file, the X-Route-Reason
header is being set with a hardcoded value of "image_detected" in the block that
checks for ContextKeyImageAwareEntryModel. Instead of always using
"image_detected", the logic should determine and set the appropriate reason
value based on whether an image was actually detected. If this code block
handles the image-detected case, the header value is correct, but if this block
is for the no-image route case, change the header value to reflect the actual
routing reason (not "image_detected").
In
`@web/default/src/features/system-settings/operations/image-aware-routing-rule-drawer.tsx`:
- Around line 63-69: The onSave callback in ImageAwareRoutingRuleDrawerProps is
typed as synchronous but the caller provides an async function, causing the
drawer to close immediately without waiting for the save operation to complete,
which hides failures and creates unhandled promise rejections. Change the onSave
type signature to return a Promise (make it async-compatible), then locate the
code that calls onSave and closes the drawer (around lines 113-114 in the
component where onOpenChange is called), and await the onSave result before
closing the drawer.
In
`@web/default/src/features/system-settings/operations/image-aware-routing-section.tsx`:
- Around line 109-131: The handleDelete and handleSave functions derive
nextRules from the current rules snapshot, but the UI actions that trigger these
functions remain clickable while a persist operation is pending. This allows
rapid consecutive actions to send competing payloads that can overwrite each
other. Introduce a loading or busy state variable to track when a persist
operation is in progress, then disable the delete and save buttons (or the
actions that trigger handleDelete and handleSave) while this pending state is
true, ensuring that only one mutation can occur at a time.
In `@web/default/src/i18n/locales/en.json`:
- Line 2482: The translation key "Model {{model}}" is defined twice in the
en.json file, creating a duplicate key that can cause ambiguous parser behavior
and silent value overrides. Remove the duplicate translation entry for "Model
{{model}}" to ensure each translation key is defined only once in the file.
In `@web/default/src/i18n/locales/zh.json`:
- Around line 2480-2482: The JSON key "Model {{model}}" appears twice in the
zh.json localization file, with a duplicate entry at the location shown in the
diff. Remove the duplicate "Model {{model}}" entry from lines 2480-2482 to
ensure only one definition exists in the file, preventing key shadowing and
confusion during future maintenance.
---
Outside diff comments:
In `@docs/image-aware-routing.md`:
- Around line 116-119: Update the documentation in the image-aware-routing.md
file to reflect the current logging behavior. The current text states that the
image_aware_entry_model field is not displayed in the log page ("未在日志页面显示"), but
this PR adds entry/actual model display to usage logs. Replace or modify the
clause that says the entry model information is not shown in the log page to
indicate that it is now available in the usage logs, ensuring operators are
aware they can view both the entry model and actual model in the logs.
---
Nitpick comments:
In `@docs/PR-description.md`:
- Around line 42-44: The code block containing the image_aware_routing example
is missing a language specifier. Add "text" as the language specifier to the
opening backticks of the code block that contains "image_aware_routing:
entry=auto-coder has_image=true -> routed=glm-4.6v notify=true". Change the
opening triple backticks from ``` to ```text to enable proper syntax
highlighting and improve documentation clarity.
- Around line 35-39: The code block starting at line 35 in
docs/PR-description.md is missing a language specifier. Add the language
identifier "text" immediately after the opening triple backticks (change ``` to
```text) to enable proper syntax highlighting and improve documentation clarity
as shown in the suggested fix.
In `@setting/operation_setting/image_aware_routing.go`:
- Around line 19-21: The exported variable ImageAwareModelRouting allows direct
access to the map by external packages, bypassing the imageAwareModelRoutingLock
mutex and risking race conditions. Change ImageAwareModelRouting to unexported
(rename to imageAwareModelRouting with lowercase) and create exported accessor
methods (such as GetImageAwareRouting, SetImageAwareRouting, or similar) that
properly acquire and release the imageAwareModelRoutingLock when reading or
writing to the map. This ensures all access to the map is protected by the
mutex.
In `@web/default/src/features/keys/types.ts`:
- Around line 45-52: The boolean preprocessing pattern that converts numeric
values (1 to true, 0 to false) is applied inconsistently across the schema. The
fields model_route_notify and cross_group_retry use this numeric preprocessing,
while unlimited_quota and model_limits_enabled do not. Verify whether
model_route_notify actually receives numeric values from the backend. If it
does, apply the same numeric preprocessing pattern to unlimited_quota and
model_limits_enabled to ensure consistency. If model_route_notify does not
receive numeric values from the backend, remove the unnecessary numeric
preprocessing from both model_route_notify and cross_group_retry to keep the
schema clean and maintainable.
🪄 Autofix (Beta)
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
Run ID: cf21c45b-26ab-4b8b-a26a-fc1e3d93d801
📒 Files selected for processing (32)
constant/context_key.gocontroller/token.godocs/PR-description.mddocs/image-aware-routing.mdmiddleware/auth.gomiddleware/distributor.gomiddleware/image_aware_routing.gomiddleware/image_aware_routing_test.gomodel/option.gomodel/token.gorelay/channel/claude/relay-claude.gorelay/channel/openai/helper.gorelay/channel/openai/relay-openai.gorelay/common/relay_info.gorelay/compatible_handler.gorelay/helper/route_hint.goservice/log_info_generate.gosetting/operation_setting/image_aware_routing.goweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/keys/types.tsweb/default/src/features/system-settings/operations/image-aware-routing-rule-drawer.tsxweb/default/src/features/system-settings/operations/image-aware-routing-section.tsxweb/default/src/features/system-settings/operations/index.tsxweb/default/src/features/system-settings/operations/section-registry.tsxweb/default/src/features/system-settings/types.tsweb/default/src/features/usage-logs/components/columns/common-logs-columns.tsxweb/default/src/features/usage-logs/components/model-badge.tsxweb/default/src/features/usage-logs/lib/format.tsweb/default/src/features/usage-logs/types.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/zh.json
| "Model Route Notify": "模型路由提示", | ||
| "Show a hint in the response when the request is auto-routed to a different model.": "当请求被自动路由到其他模型时,在响应中显示一条提示。", | ||
| "Model {{model}}": "模型 {{model}}", |
There was a problem hiding this comment.
Remove the duplicate Model {{model}} entry.
This key is already defined earlier in the file, so re-adding it here creates a duplicate JSON key and silently shadows the original translation. Keep only one definition to avoid confusing future edits.
🤖 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/default/src/i18n/locales/zh.json` around lines 2480 - 2482, The JSON key
"Model {{model}}" appears twice in the zh.json localization file, with a
duplicate entry at the location shown in the diff. Remove the duplicate "Model
{{model}}" entry from lines 2480-2482 to ensure only one definition exists in
the file, preventing key shadowing and confusion during future maintenance.
Map a virtual entry model to a vision model (last user message has an image) or coding model (no image). The rewrite runs before channel selection, so the real model name drives selection, affinity, billing, and retry. Adds response headers, token-level ModelRouteNotify hint, admin routing-rule drawer, and usage-log entry-model badge. Note: AI-generated/assisted contribution.
- use common.Marshal instead of encoding/json in claude relay - set X-Route-Reason based on whether an image was detected - await async onSave before closing the routing-rule drawer - guard concurrent mutations in routing section (disable while pending) - remove duplicate i18n key, unexport routing map, trim comments
31b6231 to
b642e05
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@relay/channel/openai/relay-openai.go`:
- Line 259: In the relay-openai.go file at the line calling
common.Marshal(simpleResponse), instead of discarding the error with an
underscore, capture the error return value and check if it is non-nil. If the
marshaling fails, handle the error appropriately (such as logging it and
returning early or setting an appropriate error response) to prevent the code
from continuing with a potentially stale or invalid responseBody, which could
cause the route-hint injection logic to be silently skipped.
In `@setting/operation_setting/image_aware_routing.go`:
- Around line 30-40: The UpdateImageAwareModelRoutingByJSONString function needs
to validate the deserialized routing rules before updating the global state.
After successfully unmarshaling the JSON payload into newMap using
common.Unmarshal, add validation logic to check that each ImageAwareRouteRule
entry in the newMap has non-empty values for the model key itself and all
required fields (vision_model and coding_model). Return an error if any rule has
empty values before proceeding to acquire the imageAwareModelRoutingLock and
update the global imageAwareModelRouting variable. This ensures invalid
configurations with empty model references cannot be swapped into the global
state.
🪄 Autofix (Beta)
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
Run ID: fbf1aace-572a-4c9d-972f-b401a615e1ca
📒 Files selected for processing (32)
constant/context_key.gocontroller/token.godocs/PR-description.mddocs/image-aware-routing.mdmiddleware/auth.gomiddleware/distributor.gomiddleware/image_aware_routing.gomiddleware/image_aware_routing_test.gomodel/option.gomodel/token.gorelay/channel/claude/relay-claude.gorelay/channel/openai/helper.gorelay/channel/openai/relay-openai.gorelay/common/relay_info.gorelay/compatible_handler.gorelay/helper/route_hint.goservice/log_info_generate.gosetting/operation_setting/image_aware_routing.goweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/keys/types.tsweb/default/src/features/system-settings/operations/image-aware-routing-rule-drawer.tsxweb/default/src/features/system-settings/operations/image-aware-routing-section.tsxweb/default/src/features/system-settings/operations/index.tsxweb/default/src/features/system-settings/operations/section-registry.tsxweb/default/src/features/system-settings/types.tsweb/default/src/features/usage-logs/components/columns/common-logs-columns.tsxweb/default/src/features/usage-logs/components/model-badge.tsxweb/default/src/features/usage-logs/lib/format.tsweb/default/src/features/usage-logs/types.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/zh.json
✅ Files skipped from review due to trivial changes (2)
- web/default/src/i18n/locales/zh.json
- web/default/src/i18n/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (25)
- web/default/src/features/system-settings/operations/index.tsx
- service/log_info_generate.go
- web/default/src/features/keys/components/api-keys-mutate-drawer.tsx
- middleware/auth.go
- web/default/src/features/usage-logs/types.ts
- relay/compatible_handler.go
- constant/context_key.go
- relay/common/relay_info.go
- controller/token.go
- middleware/distributor.go
- relay/channel/openai/helper.go
- relay/helper/route_hint.go
- web/default/src/features/keys/lib/api-key-form.ts
- model/option.go
- web/default/src/features/usage-logs/lib/format.ts
- middleware/image_aware_routing_test.go
- web/default/src/features/system-settings/operations/section-registry.tsx
- web/default/src/features/system-settings/types.ts
- web/default/src/features/usage-logs/components/model-badge.tsx
- model/token.go
- web/default/src/features/system-settings/operations/image-aware-routing-rule-drawer.tsx
- web/default/src/features/system-settings/operations/image-aware-routing-section.tsx
- web/default/src/features/usage-logs/components/columns/common-logs-columns.tsx
- middleware/image_aware_routing.go
- relay/channel/claude/relay-claude.go
| message := &simpleResponse.Choices[0].Message | ||
| if message.IsStringContent() { | ||
| message.SetStringContent(hint + message.StringContent()) | ||
| responseBody, _ = common.Marshal(simpleResponse) |
There was a problem hiding this comment.
Handle marshal failure instead of silently ignoring it.
If common.Marshal(simpleResponse) fails, the code currently swallows the error and continues with potentially stale responseBody, which can silently skip the route-hint injection.
Suggested fix
- responseBody, _ = common.Marshal(simpleResponse)
+ responseBody, err = common.Marshal(simpleResponse)
+ if err != nil {
+ return nil, types.NewOpenAIError(err, types.ErrorCodeBadResponseBody, http.StatusInternalServerError)
+ }📝 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.
| responseBody, _ = common.Marshal(simpleResponse) | |
| responseBody, err = common.Marshal(simpleResponse) | |
| if err != nil { | |
| return nil, types.NewOpenAIError(err, types.ErrorCodeBadResponseBody, http.StatusInternalServerError) | |
| } |
🤖 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/openai/relay-openai.go` at line 259, In the relay-openai.go
file at the line calling common.Marshal(simpleResponse), instead of discarding
the error with an underscore, capture the error return value and check if it is
non-nil. If the marshaling fails, handle the error appropriately (such as
logging it and returning early or setting an appropriate error response) to
prevent the code from continuing with a potentially stale or invalid
responseBody, which could cause the route-hint injection logic to be silently
skipped.
| func UpdateImageAwareModelRoutingByJSONString(value string) error { | ||
| newMap := make(map[string]ImageAwareRouteRule) | ||
| if value != "" { | ||
| if err := common.Unmarshal([]byte(value), &newMap); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| imageAwareModelRoutingLock.Lock() | ||
| imageAwareModelRouting = newMap | ||
| imageAwareModelRoutingLock.Unlock() | ||
| return nil |
There was a problem hiding this comment.
Validate routing rule payload before swapping global state.
UpdateImageAwareModelRoutingByJSONString accepts entries with empty model key / vision_model / coding_model. In middleware/image_aware_routing.go (Line 29-33 in the provided snippet), those values are used directly to rewrite modelRequest.Model, so invalid config can route requests to an empty/invalid model and break request handling.
Proposed fix
func UpdateImageAwareModelRoutingByJSONString(value string) error {
newMap := make(map[string]ImageAwareRouteRule)
if value != "" {
if err := common.Unmarshal([]byte(value), &newMap); err != nil {
return err
}
+ for entryModel, rule := range newMap {
+ if entryModel == "" || rule.VisionModel == "" || rule.CodingModel == "" {
+ return errors.New("image-aware routing rule requires non-empty entry model, vision_model, and coding_model")
+ }
+ }
}
imageAwareModelRoutingLock.Lock()
imageAwareModelRouting = newMap
imageAwareModelRoutingLock.Unlock()
return nil
} import (
+ "errors"
"sync"
"github.com/QuantumNous/new-api/common"
)📝 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.
| func UpdateImageAwareModelRoutingByJSONString(value string) error { | |
| newMap := make(map[string]ImageAwareRouteRule) | |
| if value != "" { | |
| if err := common.Unmarshal([]byte(value), &newMap); err != nil { | |
| return err | |
| } | |
| } | |
| imageAwareModelRoutingLock.Lock() | |
| imageAwareModelRouting = newMap | |
| imageAwareModelRoutingLock.Unlock() | |
| return nil | |
| func UpdateImageAwareModelRoutingByJSONString(value string) error { | |
| newMap := make(map[string]ImageAwareRouteRule) | |
| if value != "" { | |
| if err := common.Unmarshal([]byte(value), &newMap); err != nil { | |
| return err | |
| } | |
| for entryModel, rule := range newMap { | |
| if entryModel == "" || rule.VisionModel == "" || rule.CodingModel == "" { | |
| return errors.New("image-aware routing rule requires non-empty entry model, vision_model, and coding_model") | |
| } | |
| } | |
| } | |
| imageAwareModelRoutingLock.Lock() | |
| imageAwareModelRouting = newMap | |
| imageAwareModelRoutingLock.Unlock() | |
| return nil | |
| } |
🤖 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 `@setting/operation_setting/image_aware_routing.go` around lines 30 - 40, The
UpdateImageAwareModelRoutingByJSONString function needs to validate the
deserialized routing rules before updating the global state. After successfully
unmarshaling the JSON payload into newMap using common.Unmarshal, add validation
logic to check that each ImageAwareRouteRule entry in the newMap has non-empty
values for the model key itself and all required fields (vision_model and
coding_model). Return an error if any rule has empty values before proceeding to
acquire the imageAwareModelRoutingLock and update the global
imageAwareModelRouting variable. This ensures invalid configurations with empty
model references cannot be swapped into the global state.
- Add docker-image-dev.yml: builds Dockerfile.dev multi-arch and pushes
to ${DOCKERHUB_USERNAME}/new-api:{dev,dev-<sha>} on push / dispatch.
- Add docker-compose.deploy.yml: pulls the published dev image (with
DOCKERHUB_USERNAME override) instead of building locally.
- workflow: push to ghcr.io/<owner>/new-api:{dev,dev-<sha>} via GITHUB_TOKEN,
no Docker Hub secrets needed.
- compose: reference ghcr.io/gentle-lijie/new-api:dev.
amd64/arm64 each build on their native runner in parallel (no QEMU),
then a manifest job merges them into ghcr.io/<owner>/new-api:{dev,dev-<sha>}.
Much faster than a single emulated multi-arch build.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/docker-image-dev.yml:
- Around line 34-37: The checkout action step named "Check out (shallow)" that
uses actions/checkout@v4 is missing the persist-credentials configuration. Add
persist-credentials: false to the with section of this action alongside the
existing fetch-depth parameter to prevent Git credentials from being persisted
in the workflow environment.
🪄 Autofix (Beta)
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
Run ID: 9d8beb49-d462-4549-81b2-008374dc5363
📒 Files selected for processing (2)
.github/workflows/docker-image-dev.ymldocker-compose.deploy.yml
✅ Files skipped from review due to trivial changes (1)
- docker-compose.deploy.yml
| - name: Check out (shallow) | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 1 |
There was a problem hiding this comment.
Add persist-credentials: false to limit credential exposure.
The checkout action persists Git credentials by default, which could be accessed by subsequent steps or artifacts. Since this workflow doesn't need to push commits, disable credential persistence.
- name: Check out (shallow)
uses: actions/checkout@v4
with:
fetch-depth: 1
+ persist-credentials: false📝 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.
| - name: Check out (shallow) | |
| uses: actions/checkout@v4 | |
| with: | |
| fetch-depth: 1 | |
| - name: Check out (shallow) | |
| uses: actions/checkout@v4 | |
| with: | |
| fetch-depth: 1 | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 34-37: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 35-35: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/docker-image-dev.yml around lines 34 - 37, The checkout
action step named "Check out (shallow)" that uses actions/checkout@v4 is missing
the persist-credentials configuration. Add persist-credentials: false to the
with section of this action alongside the existing fetch-depth parameter to
prevent Git credentials from being persisted in the workflow environment.
Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/docker-image-dev.yml:
- Around line 31-33: The Docker image workflow lacks concurrency protection for
the `:dev` tag publishing, allowing older workflow runs to finish later and
overwrite newer `:dev` manifests. Add concurrency configuration to the
build_single_arch job and any other jobs that publish mutable `:dev*` tags to
ensure only one run executes at a time and newer runs cancel in-progress older
runs. Use a stable concurrency group identifier based on the branch or workflow
context, and configure the concurrency setting to automatically cancel previous
runs when a new run starts, preventing race conditions where stale images
overwrite current ones.
- Line 78: Replace all floating GitHub Actions version tags with their
corresponding immutable commit SHAs in the workflow file at lines 78, 107, 124,
and 131 to prevent silent upstream changes. Change docker/build-push-action@v6
to use SHA 10e90e3645eae34f1e60eeb005ba3a3d33f178e8, docker/login-action@v3 to
c94ce9fb468520275223c153574b00df6fe4bcc9, docker/setup-buildx-action@v3 to
8d2750c68a42422c14e847fe6c8ac0403b4cbd6f, and actions/checkout@v4 to
34e114876b0b11c390a56381ad16ebd13914f8d5 by replacing the `@vN` tag suffix with
the full commit SHA.
🪄 Autofix (Beta)
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
Run ID: c6667ba6-411d-4861-8e03-87cb58f66b12
📒 Files selected for processing (1)
.github/workflows/docker-image-dev.yml
| build_single_arch: | ||
| name: Build & push (${{ matrix.arch }}) [native] | ||
| strategy: |
There was a problem hiding this comment.
Protect :dev publishing from out-of-order workflow races.
Both jobs publish mutable :dev* tags, but there is no workflow concurrency guard. Older runs can finish later and overwrite the latest :dev manifest, which impacts deployment consumers of :dev.
Suggested minimal fix
+concurrency:
+ group: docker-image-dev-${{ github.ref }}
+ cancel-in-progress: true
+
jobs:
build_single_arch:Also applies to: 98-100, 133-145
🤖 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 @.github/workflows/docker-image-dev.yml around lines 31 - 33, The Docker
image workflow lacks concurrency protection for the `:dev` tag publishing,
allowing older workflow runs to finish later and overwrite newer `:dev`
manifests. Add concurrency configuration to the build_single_arch job and any
other jobs that publish mutable `:dev*` tags to ensure only one run executes at
a time and newer runs cancel in-progress older runs. Use a stable concurrency
group identifier based on the branch or workflow context, and configure the
concurrency setting to automatically cancel previous runs when a new run starts,
preventing race conditions where stale images overwrite current ones.
|
|
||
| - name: Build & push single-arch | ||
| id: build | ||
| uses: docker/build-push-action@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Unpinned action references in workflow files:"
rg -nP 'uses:\s*[^@\s]+@[vV]?\d+(\.\d+)?$' .github/workflows/*.yml .github/workflows/*.yaml || true
echo
echo "Resolve current tag SHAs (for pinning):"
for spec in \
"actions/checkout v4" \
"docker/build-push-action v6" \
"docker/login-action v3" \
"docker/setup-buildx-action v3"
do
action="${spec% *}"
tag="${spec##* }"
echo "== $action@$tag =="
gh api "repos/${action}/git/ref/tags/${tag}" --jq '.object.sha'
doneRepository: QuantumNous/new-api
Length of output: 2933
Pin GitHub Actions to immutable commit SHAs.
Lines 78, 107, 124, and 131 use floating version tags (@v6/@v4/@v3). Floating tags permit silent upstream changes in CI and violate security policy.
Resolve to commit SHAs:
actions/checkout@v4→34e114876b0b11c390a56381ad16ebd13914f8d5docker/build-push-action@v6→10e90e3645eae34f1e60eeb005ba3a3d33f178e8docker/login-action@v3→c94ce9fb468520275223c153574b00df6fe4bcc9docker/setup-buildx-action@v3→8d2750c68a42422c14e847fe6c8ac0403b4cbd6f
🧰 Tools
🪛 zizmor (1.25.2)
[error] 78-78: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/docker-image-dev.yml at line 78, Replace all floating
GitHub Actions version tags with their corresponding immutable commit SHAs in
the workflow file at lines 78, 107, 124, and 131 to prevent silent upstream
changes. Change docker/build-push-action@v6 to use SHA
10e90e3645eae34f1e60eeb005ba3a3d33f178e8, docker/login-action@v3 to
c94ce9fb468520275223c153574b00df6fe4bcc9, docker/setup-buildx-action@v3 to
8d2750c68a42422c14e847fe6c8ac0403b4cbd6f, and actions/checkout@v4 to
34e114876b0b11c390a56381ad16ebd13914f8d5 by replacing the `@vN` tag suffix with
the full commit SHA.
Source: Linters/SAST tools
Switch workflow from Dockerfile.dev (backend-only) to the production Dockerfile, which builds web/default + classic with bun and embeds them into the Go binary. The single :3000 port now serves both API and frontend. Drop web/** from paths-ignore so frontend changes trigger a rebuild.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/docker-image-dev.yml (3)
49-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin GitHub Actions to immutable commit SHAs.
Floating version tags (
@v4/@v3/@v6) permit silent upstream changes and violate security policy. Pin to commit SHAs to prevent supply-chain attacks.🔒 Proposed fix to pin all actions
- name: Check out (shallow) - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 1- name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 + uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3- name: Log in to GHCR - uses: docker/login-action@v3 + uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3 with: registry: ghcr.io- name: Build & push single-arch id: build - uses: docker/build-push-action@v6 + uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6 with:Also applies to: 67-67, 70-70, 78-78
🤖 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 @.github/workflows/docker-image-dev.yml at line 49, Replace the floating version tag (`@v4`) in the actions/checkout action with an immutable commit SHA to prevent silent upstream changes and security vulnerabilities. Apply the same fix to all other GitHub Actions in the workflow that currently use floating version tags (the comment also mentions lines 67, 70, and 78), pinning each to their respective commit SHAs instead of version tags like `@v3`, `@v6`, etc.
48-51:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
persist-credentials: falseto limit credential exposure.The checkout action persists Git credentials by default, making them accessible to subsequent steps or artifacts. This workflow doesn't push commits, so credential persistence is unnecessary.
🛡️ Proposed fix to disable credential persistence
- name: Check out (shallow) uses: actions/checkout@v4 with: fetch-depth: 1 + persist-credentials: false🤖 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 @.github/workflows/docker-image-dev.yml around lines 48 - 51, The actions/checkout@v4 action is persisting Git credentials by default, creating an unnecessary security risk since this workflow only builds and pushes Docker images without performing any Git push operations. Add the persist-credentials: false parameter to the with section of the checkout action to disable credential persistence and reduce the attack surface.
31-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winProtect
:devpublishing from out-of-order workflow races.Multiple concurrent workflow runs can publish mutable
:dev*tags without ordering guarantees. Older runs finishing later will overwrite the latest:devmanifest, impacting deployment consumers (e.g., docker-compose.deploy.yml pulls:dev).🔒 Proposed fix to add workflow-level concurrency control
Add before the
jobs:section:+concurrency: + group: docker-image-dev-${{ github.ref }} + cancel-in-progress: true + jobs: build_single_arch:🤖 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 @.github/workflows/docker-image-dev.yml around lines 31 - 33, The workflow lacks concurrency control at the job level, allowing multiple concurrent runs to publish mutable `:dev` tags simultaneously without ordering guarantees, causing older runs to potentially overwrite newer ones. Add a concurrency configuration section before the jobs section in the workflow file to ensure sequential execution of builds that publish to the `:dev` tag, using the default branch or appropriate concurrency group identifier to prevent races and ensure only the latest build's manifest is published.
🤖 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 @.github/workflows/docker-image-dev.yml:
- Line 81: There is a mismatch between the Dockerfile reference in the
docker-image-dev workflow and the documentation in docker-compose.deploy.yml.
The workflow file currently references `./Dockerfile` but
docker-compose.deploy.yml indicates the CI builds from `Dockerfile.dev`. Verify
which Dockerfile is the correct one to use for the development image build, then
update either the file parameter in the docker-image-dev workflow (the `file:`
field pointing to `./Dockerfile`) or the comment in docker-compose.deploy.yml to
ensure consistency across both files.
---
Duplicate comments:
In @.github/workflows/docker-image-dev.yml:
- Line 49: Replace the floating version tag (`@v4`) in the actions/checkout action
with an immutable commit SHA to prevent silent upstream changes and security
vulnerabilities. Apply the same fix to all other GitHub Actions in the workflow
that currently use floating version tags (the comment also mentions lines 67,
70, and 78), pinning each to their respective commit SHAs instead of version
tags like `@v3`, `@v6`, etc.
- Around line 48-51: The actions/checkout@v4 action is persisting Git
credentials by default, creating an unnecessary security risk since this
workflow only builds and pushes Docker images without performing any Git push
operations. Add the persist-credentials: false parameter to the with section of
the checkout action to disable credential persistence and reduce the attack
surface.
- Around line 31-33: The workflow lacks concurrency control at the job level,
allowing multiple concurrent runs to publish mutable `:dev` tags simultaneously
without ordering guarantees, causing older runs to potentially overwrite newer
ones. Add a concurrency configuration section before the jobs section in the
workflow file to ensure sequential execution of builds that publish to the
`:dev` tag, using the default branch or appropriate concurrency group identifier
to prevent races and ensure only the latest build's manifest is published.
🪄 Autofix (Beta)
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
Run ID: 7bae94e4-7489-44b7-ad31-ae02ef912209
📒 Files selected for processing (1)
.github/workflows/docker-image-dev.yml
| uses: docker/build-push-action@v6 | ||
| with: | ||
| context: . | ||
| file: ./Dockerfile |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check Dockerfile existence and verify docker-compose.deploy.yml references
echo "=== Dockerfiles in repository ==="
fd -t f '^Dockerfile' -d 1
echo -e "\n=== docker-compose.deploy.yml references ==="
rg -n 'Dockerfile' docker-compose.deploy.yml || echo "No direct Dockerfile reference in services"
echo -e "\n=== Verify comment accuracy in docker-compose.deploy.yml ==="
rg -n -B2 -A2 'Dockerfile.dev|from Dockerfile' docker-compose.deploy.ymlRepository: QuantumNous/new-api
Length of output: 608
Verify and fix Dockerfile reference mismatch.
The workflow builds from ./Dockerfile (line 81), but docker-compose.deploy.yml documents that the CI builds from Dockerfile.dev. Both files exist in the repository. Confirm which Dockerfile should be used and update whichever is incorrect—either this workflow or the comment in docker-compose.deploy.yml (line 3).
🤖 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 @.github/workflows/docker-image-dev.yml at line 81, There is a mismatch
between the Dockerfile reference in the docker-image-dev workflow and the
documentation in docker-compose.deploy.yml. The workflow file currently
references `./Dockerfile` but docker-compose.deploy.yml indicates the CI builds
from `Dockerfile.dev`. Verify which Dockerfile is the correct one to use for the
development image build, then update either the file parameter in the
docker-image-dev workflow (the `file:` field pointing to `./Dockerfile`) or the
comment in docker-compose.deploy.yml to ensure consistency across both files.
51fdfc5 to
2b6f1df
Compare
Image-Aware Model Routing
Note
This PR was initially generated with AI assistance and subsequently reviewed and refined manually.
This PR resolves issue #5589
Overview
新增图片感知模型路由(Image-Aware Routing)能力。
管理员可以配置一个虚拟入口模型(例如
auto-coder),用户请求该模型时,网关在进行渠道选择前,会检查当前请求中最后一条user消息是否包含图片内容,并根据结果自动将模型重写为对应的视觉模型或文本模型。支持识别:
image_urlimage由于模型改写发生在 Distributor 选路之前,因此后续的渠道匹配、模型亲和性、计费统计及重试逻辑均基于最终路由后的真实模型执行。
Routing Logic
路由判断仅基于当前请求中最后一条
role=user消息:系统不维护任何会话状态,也不会扫描历史消息中的图片内容。
因此:
Observability
为提升路由可观测性,新增以下能力:
Response Headers
所有响应增加以下 Header:
X-Route-Entry-ModelX-Routed-ModelX-Route-Reason用于展示入口模型、实际路由模型以及路由原因。
In-Response Route Notification
新增配置项
ModelRouteNotify(新 APIKey 默认开启,位于 APIkey 的高级设置内)。开启后,系统会在响应内容中插入路由提示,例如:
支持:
Logging
新增路由相关日志字段:
同时在用量记录中展示:
Admin UI
管理后台新增向导式配置界面。
配置项包括:
管理员无需手动编辑 JSON 即可完成配置。
Validation
已完成以下验证:
go build ./...go test ./middleware/Summary by CodeRabbit
Release Notes
New Features
usermessage includes an image.Documentation
Tests
usermessage.