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:
📝 WalkthroughWalkthroughThe PR updates user-setting persistence and cache behavior, expands task request and billing normalization, changes Ollama tool-call handling, adjusts server and web session flows, and refreshes repository maintenance docs and JSON allowlist entries. ChangesUser settings and cache writes
Relay task parsing and task adapters
Ollama tool-call conversion
Server lifecycle and web session gating
Repository policy and sync maintenance
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@relay/channel/task/taskcommon/protocol_config.go`:
- Line 58: The ResultURLPaths default slice in protocol_config.go is duplicated
across multiple prefix groups, so refactor it to be built programmatically from
shared leaf keys and prefixes instead of hardcoding every combination. Update
the default construction near the ProtocolConfig defaults to use a helper like
defaultResultURLPaths() (or equivalent) that iterates over the repeated prefix
set and appends the common leaf keys, while preserving the existing top-level
entries at the end. This will keep the ResultURLPaths list consistent and make
future changes to the leaf key set easier to maintain.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 369ff2e1-3a16-4ed2-9329-e9c4cdbb64e4
📒 Files selected for processing (2)
relay/channel/task/taskcommon/path_config_test.gorelay/channel/task/taskcommon/protocol_config.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Backend checks
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrappers incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly callingencoding/json;encoding/jsontypes likejson.RawMessageandjson.Numbermay still be used as types.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+ simultaneously: prefer GORM methods over raw SQL, avoid directAUTO_INCREMENT/SERIAL, use cross-DB quoting/boolean helpers when raw SQL is unavoidable, avoid DB-specific functions/operators without fallbacks, and ensure migrations work on all three databases.
For request structs parsed from client JSON and later re-marshaled to upstream providers, especially in relay/convert paths, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved and omitted-only fields remainnil.
**/*.go: Usecommon.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson, andcommon.GetJsonTypefromcommon/json.gofor all JSON marshal/unmarshal operations; do not directly callencoding/jsonin business code.
Keep all database code fully compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless cross-DB-safe, and use the provided DB-specific helpers/flags for quoting, booleans, and branching.
For request structs parsed from client JSON and re-marshaled to upstream providers, especially in relay/convert paths, use pointer types withomitemptyfor optional scalar fields so explicit zero/false values are preserved.
Files:
relay/channel/task/taskcommon/protocol_config.gorelay/channel/task/taskcommon/path_config_test.go
relay/channel/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
When implementing a new relay channel, confirm whether the provider supports
StreamOptions; if it does, add that channel tostreamSupportedChannels.When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/task/taskcommon/protocol_config.gorelay/channel/task/taskcommon/path_config_test.go
🔇 Additional comments (1)
relay/channel/task/taskcommon/path_config_test.go (1)
274-298: LGTM! Good coverage for the newmetadata.urlextraction path, and it complements the existingresult.video_urltest.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
relay/channel/ollama/stream.go (1)
145-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated "thinking" decode logic into a helper.
The same trim/unmarshal/fallback logic for
Thinkingis repeated three times (streaming path, non-stream per-line loop, non-stream single-body fallback), differing only in where the result is written.♻️ Suggested refactor
func decodeOllamaThinking(thinking json.RawMessage) string { if len(thinking) == 0 { return "" } raw := strings.TrimSpace(string(thinking)) if raw == "" || raw == "null" { return "" } var content string if err := common.Unmarshal(thinking, &content); err == nil { return content } return raw }Then replace each call site, e.g.:
- if chunk.Message != nil && len(chunk.Message.Thinking) > 0 { - raw := strings.TrimSpace(string(chunk.Message.Thinking)) - if raw != "" && raw != "null" { - var thinkingContent string - if err := common.Unmarshal(chunk.Message.Thinking, &thinkingContent); err == nil { - delta.Choices[0].Delta.SetReasoningContent(thinkingContent) - } else { - delta.Choices[0].Delta.SetReasoningContent(raw) - } - } - } + if chunk.Message != nil { + if tc := decodeOllamaThinking(chunk.Message.Thinking); tc != "" { + delta.Choices[0].Delta.SetReasoningContent(tc) + } + }Also applies to: 236-248, 268-280
🤖 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/ollama/stream.go` around lines 145 - 157, The Ollama thinking decode logic is duplicated across the streaming and non-streaming paths, so extract the trim/unmarshal/fallback behavior into a shared helper like decodeOllamaThinking and use it from the chunk handling in stream.go; keep the helper responsible only for turning chunk.Message.Thinking into the final string, and have each call site write that result into delta.Choices[0].Delta.SetReasoningContent or the equivalent non-stream target.relay/common/relay_utils.go (1)
253-256: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent trimming vs.
ValidateMultipartDirect.This sibling normalization path copies
req.Imageintoreq.Imageswithoutstrings.TrimSpace, while the newly-changedValidateMultipartDirect(lines 176-177) trims. For consistency (and since a dedicated test was just added to verify trimming behavior), consider applying the same trim here.🧹 Suggested fix
if len(req.Images) == 0 && strings.TrimSpace(req.Image) != "" { // 兼容单图上传 - req.Images = []string{req.Image} + req.Images = []string{strings.TrimSpace(req.Image)} }🤖 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 253 - 256, The single-image normalization in the `req.Images` fallback is inconsistent with `ValidateMultipartDirect` because it copies `req.Image` without trimming first. Update the `req.Images` assignment in the `relay_utils` normalization path to use the same `strings.TrimSpace` behavior as `ValidateMultipartDirect`, so `req.Image` is trimmed before being wrapped into the slice.controller/user.go (2)
696-746: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate read-modify-write blocks for
sidebar_modulesandlanguage.Both branches repeat the same fetch/mutate/persist pattern. Consider extracting a small helper, e.g.
updateUserSettingField(userId int, mutate func(*dto.UserSetting)) error, to reduce duplication.♻️ Suggested consolidation
- if sidebarModules, sidebarExists := requestData["sidebar_modules"]; sidebarExists { - userId := c.GetInt("id") - user, err := model.GetUserById(userId, false) - if err != nil { - common.ApiError(c, err) - return - } - - // 获取当前用户设置 - currentSetting := user.GetSetting() - - // 更新sidebar_modules字段 - if sidebarModulesStr, ok := sidebarModules.(string); ok { - currentSetting.SidebarModules = sidebarModulesStr - } - - if err := model.UpdateUserSetting(user.Id, currentSetting); err != nil { - common.ApiErrorI18n(c, i18n.MsgUpdateFailed) - return - } - - common.ApiSuccessI18n(c, i18n.MsgUpdateSuccess, nil) - return - } - - // 检查是否是语言偏好更新请求 - if language, langExists := requestData["language"]; langExists { - userId := c.GetInt("id") - user, err := model.GetUserById(userId, false) - if err != nil { - common.ApiError(c, err) - return - } - - // 获取当前用户设置 - currentSetting := user.GetSetting() - - // 更新language字段 - if langStr, ok := language.(string); ok { - currentSetting.Language = langStr - } - - if err := model.UpdateUserSetting(user.Id, currentSetting); err != nil { - common.ApiErrorI18n(c, i18n.MsgUpdateFailed) - return - } - - common.ApiSuccessI18n(c, i18n.MsgUpdateSuccess, nil) - return - } + if sidebarModules, sidebarExists := requestData["sidebar_modules"]; sidebarExists { + if err := updateUserSettingField(c, func(s *dto.UserSetting) { + if v, ok := sidebarModules.(string); ok { + s.SidebarModules = v + } + }); err != nil { + return + } + return + } + + if language, langExists := requestData["language"]; langExists { + if err := updateUserSettingField(c, func(s *dto.UserSetting) { + if v, ok := language.(string); ok { + s.Language = v + } + }); err != nil { + return + } + return + }🤖 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/user.go` around lines 696 - 746, The `sidebar_modules` and `language` branches in the user settings update flow duplicate the same read-modify-write logic. Extract the repeated fetch via `model.GetUserById`, mutate `user.GetSetting()`, and persist with `model.UpdateUserSetting` into a small helper (for example, a function used by this handler) so both cases reuse the same path and only differ in the field mutation.
691-746: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAvoid full-blob writes for user settings
controller/user.gostill reads the entiresettingJSON, mutates one field, and writes the whole blob back. Concurrent updates to other setting fields can be lost becausemodel.UpdateUserSettingreplaces the full column. Patch only the changed key or add merge/optimistic-locking semantics.🤖 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/user.go` around lines 691 - 746, The user settings update paths in controller/user.go still read the full setting blob via GetUserById/GetSetting and then overwrite it with UpdateUserSetting, which can clobber concurrent changes. Update the sidebar_modules and language branches so they only patch the modified key, or add merge/optimistic-locking behavior inside model.UpdateUserSetting so unchanged setting fields are preserved.model/user.go (1)
544-565: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueExtra round-trip:
currentis only used to anchorModel().
currentis loaded solely to giveDB.Model(¤t)a primary key; its fields are never read afterward. This could be simplified toDB.Model(&User{Id: user.Id})to skip the initialSELECT, at the cost of losing the "row must exist" pre-check (which would then surface as a no-opUPDATEmatching zero rows instead ofErrRecordNotFound). If existence-checking is intentional, consider a comment noting why the extra query is kept.🤖 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 `@model/user.go` around lines 544 - 565, The Update method in User loads a full current record only to pass it into DB.Model, which adds an unnecessary SELECT. Simplify the anchoring in Update by using a lightweight User instance with the existing Id when calling DB.Model, or if the pre-check is intentional, keep the current lookup and add a brief comment explaining why the extra DB.First(¤t, user.Id) is required. Use the User.Update method and the current/newUser variables to locate the change.
🤖 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/ollama/stream_test.go`:
- Around line 19-95: The new tool-call coverage only exercises
ollamaChatHandler, but the streaming path in ollamaStreamHandler has the same
tool-call conversion and finish-reason override logic and is currently untested.
Add a streaming-focused test alongside TestOllamaChatHandlerNonStreamToolCalls
that feeds a streamed Ollama response through ollamaStreamHandler and asserts
the emitted OpenAI output includes the converted tool call, generated
ID/type/function fields, and the ToolCalls finish reason.
In `@relay/channel/task/ali/adaptor.go`:
- Around line 223-248: `firstTaskImage` is preferring `req.Image` before
`req.InputReference`, which can select the wrong source for mixed requests.
Update `firstTaskImage` in the adaptor to check `req.InputReference` first, then
fall back to non-empty entries in `req.Images`, and only use `req.Image` last so
it stays consistent with `ValidateMultipartDirect` and the `secondTaskImage`
helper logic.
- Around line 250-291: normalizeWan27I2VInput currently accepts any non-empty
Input.Media, but WAN2.7 i2v must include a first_frame. Update this function to
validate aliReq.Input.Media after normalization and reject requests that only
contain last_frame or driving_audio without any first_frame. Keep the existing
media-building logic and the legacy-field cleanup, but make the final validation
explicitly require a first_frame entry before returning nil.
---
Outside diff comments:
In `@controller/user.go`:
- Around line 696-746: The `sidebar_modules` and `language` branches in the user
settings update flow duplicate the same read-modify-write logic. Extract the
repeated fetch via `model.GetUserById`, mutate `user.GetSetting()`, and persist
with `model.UpdateUserSetting` into a small helper (for example, a function used
by this handler) so both cases reuse the same path and only differ in the field
mutation.
- Around line 691-746: The user settings update paths in controller/user.go
still read the full setting blob via GetUserById/GetSetting and then overwrite
it with UpdateUserSetting, which can clobber concurrent changes. Update the
sidebar_modules and language branches so they only patch the modified key, or
add merge/optimistic-locking behavior inside model.UpdateUserSetting so
unchanged setting fields are preserved.
In `@model/user.go`:
- Around line 544-565: The Update method in User loads a full current record
only to pass it into DB.Model, which adds an unnecessary SELECT. Simplify the
anchoring in Update by using a lightweight User instance with the existing Id
when calling DB.Model, or if the pre-check is intentional, keep the current
lookup and add a brief comment explaining why the extra DB.First(¤t,
user.Id) is required. Use the User.Update method and the current/newUser
variables to locate the change.
In `@relay/channel/ollama/stream.go`:
- Around line 145-157: The Ollama thinking decode logic is duplicated across the
streaming and non-streaming paths, so extract the trim/unmarshal/fallback
behavior into a shared helper like decodeOllamaThinking and use it from the
chunk handling in stream.go; keep the helper responsible only for turning
chunk.Message.Thinking into the final string, and have each call site write that
result into delta.Choices[0].Delta.SetReasoningContent or the equivalent
non-stream target.
In `@relay/common/relay_utils.go`:
- Around line 253-256: The single-image normalization in the `req.Images`
fallback is inconsistent with `ValidateMultipartDirect` because it copies
`req.Image` without trimming first. Update the `req.Images` assignment in the
`relay_utils` normalization path to use the same `strings.TrimSpace` behavior as
`ValidateMultipartDirect`, so `req.Image` is trimmed before being wrapped into
the slice.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: c5ef4535-8758-4734-b901-1ac07e74e1ee
📒 Files selected for processing (20)
.agents/upstream-sync/new-api-sync.mdcontroller/subscription.gocontroller/user.gomain.gomodel/log.gomodel/user.gomodel/user_cache.gomodel/user_update_test.gorelay/channel/ollama/stream.gorelay/channel/ollama/stream_test.gorelay/channel/task/ali/adaptor.gorelay/channel/task/ali/adaptor_wan27_test.gorelay/channel/task/ali/constants.gorelay/channel/task/taskcommon/path_config_test.gorelay/channel/task/taskcommon/protocol_config.gorelay/common/relay_utils.gorelay/common/relay_utils_test.gorouter/api-router.gotools/jsonwrapcheck/allowlist.txtweb/default/src/routes/_authenticated/route.tsx
💤 Files with no reviewable changes (1)
- tools/jsonwrapcheck/allowlist.txt
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Backend checks
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrappers incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly callingencoding/json;encoding/jsontypes likejson.RawMessageandjson.Numbermay still be used as types.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+ simultaneously: prefer GORM methods over raw SQL, avoid directAUTO_INCREMENT/SERIAL, use cross-DB quoting/boolean helpers when raw SQL is unavoidable, avoid DB-specific functions/operators without fallbacks, and ensure migrations work on all three databases.
For request structs parsed from client JSON and later re-marshaled to upstream providers, especially in relay/convert paths, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved and omitted-only fields remainnil.
**/*.go: Usecommon.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson, andcommon.GetJsonTypefromcommon/json.gofor all JSON marshal/unmarshal operations; do not directly callencoding/jsonin business code.
Keep all database code fully compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless cross-DB-safe, and use the provided DB-specific helpers/flags for quoting, booleans, and branching.
For request structs parsed from client JSON and re-marshaled to upstream providers, especially in relay/convert paths, use pointer types withomitemptyfor optional scalar fields so explicit zero/false values are preserved.
Files:
relay/channel/task/ali/constants.gorouter/api-router.gorelay/common/relay_utils.gorelay/common/relay_utils_test.gorelay/channel/ollama/stream_test.gocontroller/subscription.gomodel/user_update_test.gomain.gomodel/log.gorelay/channel/task/taskcommon/path_config_test.gorelay/channel/task/taskcommon/protocol_config.gorelay/channel/task/ali/adaptor_wan27_test.gomodel/user.gomodel/user_cache.gorelay/channel/ollama/stream.gorelay/channel/task/ali/adaptor.gocontroller/user.go
relay/channel/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
When implementing a new relay channel, confirm whether the provider supports
StreamOptions; if it does, add that channel tostreamSupportedChannels.When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/task/ali/constants.gorelay/channel/ollama/stream_test.gorelay/channel/task/taskcommon/path_config_test.gorelay/channel/task/taskcommon/protocol_config.gorelay/channel/task/ali/adaptor_wan27_test.gorelay/channel/ollama/stream.gorelay/channel/task/ali/adaptor.go
web/default/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
web/default/src/**/*.{ts,tsx}: 前端页面文本与组件内文案必须支持 i18n：React 组件中应使用useTranslation()取得t，并通过t()渲染用户可见文本；子组件也应自行使用useTranslation()保持独立性。
禁止使用两层及以上嵌套三元表达式；复杂逻辑应改用if-else、提前返回或抽取函数。
控制函数圈复杂度，复杂逻辑应拆成更小的函数；变量与函数命名应有意义并遵循驼峰等常规命名约定。
TypeScript 代码应避免使用any，优先使用具体类型或unknown；参数与返回值应显式标注类型；仅类型用途的导入应使用import type。
修改 TypeScript 或 TSX 代码后必须执行类型检查，并修复所有类型错误，不得遗留。
对象非必要不要解构，尤其是组件 props；优先直接使用props.xxx以保持代码清晰。
组件应使用函数式组件与 Hooks，遵循单一职责；组件 props 必须有明确类型（接口或类型别名）。
单文件超过约 200 行时应考虑拆分子组件或抽取自定义 Hooks；类型定义可与组件同文件或放在同模块的types中。
在 React 中应合理使用useMemo、useCallback、React.memo，避免在渲染路径中创建新对象或数组；必要时进行代码分割与动态import。
React Query 的数据获取应使用useQuery、变更应使用useMutation；每个查询需配置唯一queryKey，并在成功后对相关查询执行invalidateQueries；服务端错误应统一交给handleServerError。
Axios 请求应使用项目统一的api实例；GET 请求默认去重，特殊请求可显式关闭；认证与通用错误应在拦截器中统一处理。
服务端错误应统一使用handleServerError，展示层应使用toast.error等统一方式；文案需走 i18n；路由级错误应由errorComponent承接；表单错误应通过form.setError等方式映射到字段。
样式应以 Tailwind 工具类为主，动态类名使用cn()合并；非动态场景避免内联样式；响应式采用移动优先与 Tailwind 断点，主题与暗色模式通过 CSS 变量与dark:处理。
应使用语义化 HTML、正确关联label与输入、保证键盘可操作与合理焦点顺序；必要时添加 ARIA 属性，装饰性图标应使用aria-hidden="true"。
认证与权限应在路由与接口层校验；前后端都应做数据校验（如 Zod）；敏感信息不得落前端存储；避免使用dangerouslySetInnerHTML；跨域与 Cookie 需配合withCredentials并按后端要求处理 CSRF。
组件测试应使用 React Testing Library，关注交互与行为，避免测试实现细节；关键流程可补充集成与 E2E 测试。
环境变量应通过.env读取，并使用VITE_前缀；代码中不得硬编码密钥。
Files:
web/default/src/routes/_authenticated/route.tsx
web/default/src/routes/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
路由应使用 TanStack Router，并通过
createFileRoute定义；搜索参数应使用 Zod schema +validateSearch校验；认证与重定向应放在beforeLoad中；导航应优先使用useNavigate或Link，避免直接操作window.location。
Files:
web/default/src/routes/_authenticated/route.tsx
web/default/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bunas the preferred package manager and script runner for the frontend inweb/default/(bun install,bun run dev,bun run build, andbun run i18n:*).
Files:
web/default/src/routes/_authenticated/route.tsx
🪛 Betterleaks (1.6.0)
.agents/upstream-sync/new-api-sync.md
[high] 12-12: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
.agents/upstream-sync/new-api-sync.md
[grammar] ~55-~55: Use a hyphen to join words.
Context: ...unters; add critical rate limit for self update. | ported | Added `UpdateUserSett...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.22.1)
.agents/upstream-sync/new-api-sync.md
[warning] 55-55: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 55-55: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 55-55: Table column count
Expected: 7; Actual: 8; Too many cells, extra data will be missing
(MD056, table-column-count)
🔇 Additional comments (24)
.agents/upstream-sync/new-api-sync.md (2)
55-55: 📐 Maintainability & Code Quality | ⚡ Quick winEscape the regex pipe in the verification cell.
The
|inside thego testregex is being parsed as another table column, which breaks the Markdown table layout.Fix
-| `go test ./model -run "TestUserUpdateDoesNotOverwriteAccountingFields|TestUpdateUserSettingOnlyUpdatesSetting"`; broader focused Go test set; `go run ./tools/jsonwrapcheck`; `git diff --check`. | Preserves explicit quota mutation paths and token/cache invalidation logic. +| `go test ./model -run "TestUserUpdateDoesNotOverwriteAccountingFields\|TestUpdateUserSettingOnlyUpdatesSetting"`; broader focused Go test set; `go run ./tools/jsonwrapcheck`; `git diff --check`. | Preserves explicit quota mutation paths and token/cache invalidation logic.Source: Linters/SAST tools
9-13: 📐 Maintainability & Code Quality | ⚡ Quick winKeep the reviewed baseline and queue in sync.
917a2cff...is now recorded as the last reviewed commit, but the queue still lists a pending entry that includes917a2cff. That makes the sync log internally inconsistent and leaves the next incremental pass ambiguous.Please confirm whether this range should stop before
917a2cffor whether the baseline should remain on the prior commit.Also applies to: 58-58
web/default/src/routes/_authenticated/route.tsx (1)
41-57: LGTM!relay/channel/task/taskcommon/protocol_config.go (1)
37-59: LGTM!Also applies to: 81-81
relay/channel/task/taskcommon/path_config_test.go (1)
299-327: LGTM!router/api-router.go (1)
86-86: LGTM!relay/channel/ollama/stream.go (2)
45-72: LGTM!
202-331: LGTM!relay/common/relay_utils_test.go (1)
14-33: LGTM!relay/channel/task/ali/adaptor.go (2)
209-217: LGTM!
422-511: LGTM! The rest ofconvertToAliRequest's integration withfirstTaskImageandnormalizeWan27I2VInputmatches the added test coverage.relay/channel/task/ali/constants.go (1)
3-13: LGTM!relay/channel/task/ali/adaptor_wan27_test.go (1)
1-131: LGTM! Good coverage of the new WAN2.7 media-building, legacy-field-clearing, explicit-metadata-media, and error-path behaviors.relay/common/relay_utils.go (1)
174-178: 🗄️ Data Integrity & IntegrationNo precedence bug here.
firstTaskImage()already prefersreq.Image, so a request with bothimageandinput_referencehas a consistent winner in the WAN2.7 path.> Likely an incorrect or invalid review comment.controller/subscription.go (1)
89-95: LGTM! Consistent with the newmodel.UpdateUserSettingpersistence pattern used elsewhere. Note the same read-modify-write-whole-blob caveat flagged incontroller/user.go(UpdateSelf) applies here too, sincecurrent := user.GetSetting()is read once and the full struct is written back.controller/user.go (3)
3-5: LGTM!Also applies to: 38-38, 182-182, 565-565, 603-603, 876-876, 927-927
691-694: LGTM!Also applies to: 749-758
1341-1341: LGTM! Persisting only thesettingcolumn viamodel.UpdateUserSettingcorrectly avoids clobbering other columns (e.g. quota) compared to the previousSetSetting+Update(false)pattern.model/user.go (1)
82-116: LGTM!Also applies to: 170-170
model/user_update_test.go (1)
1-92: LGTM! Good regression coverage for the accounting-field-preservation fix.model/user_cache.go (2)
67-77: LGTM!Also applies to: 109-109, 246-252
79-100: 🩺 Stability & AvailabilityRole changes already invalidate the user cache
user.Update(false)callers don’t persistRole, and the promote/demote path clears bothInvalidateUserCacheandInvalidateUserTokensCacheafter saving, soupdateUserRoleCacheisn’t needed here.> Likely an incorrect or invalid review comment.model/log.go (2)
15-16: LGTM!
528-542: 🩺 Stability & AvailabilityNo issue here
LogQuotaDataonly updates the in-memory quota cache under a mutex; it does not do external I/O or background work, so calling it inline here should not add meaningful request latency or expose a new panic risk.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
relay/common/relay_info.go (1)
740-766: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve nil for
duration: null.nullis currently decoded intoDuration = &0, which drops the optional-field distinction and can re-emit as an explicitduration: 0.🤖 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_info.go` around lines 740 - 766, Preserve the nil/optional distinction in TaskSubmitReq.UnmarshalJSON: the current Duration decoding path treats duration:null as a zero value and assigns a pointer, which can later serialize as an explicit 0. Update the duration handling in UnmarshalJSON to detect null before attempting int/string parsing and leave t.Duration unset when aux.Duration is null; keep the existing parsing behavior for numeric and string values in TaskSubmitReq.Source: Coding guidelines
web/default/src/routes/_authenticated/route.tsx (1)
41-61: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep cached sessions usable through transient
/api/user/selffailures — rethrowing every non-401 aborts authenticated navigation and sends users to the global error page during brief backend/network hiccups, even when a cached user is already present. If that fallback is undesirable, treat non-401 failures as non-fatal here instead of exitingbeforeLoad.🤖 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/routes/_authenticated/route.tsx` around lines 41 - 61, The authenticated route’s beforeLoad logic is treating every non-401 getSelf() failure as fatal, which breaks navigation when a cached session is still valid. Update the getSelf()/res handling in the route component to only redirect/reset auth on an actual 401, and treat transient non-401 errors as non-fatal by falling back to the existing cached user instead of throwing. Keep the logic localized around getSelf(), auth.reset(), auth.setUser(), and the redirect call.
🤖 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 `@web/default/src/features/channels/lib/channel-form.ts`:
- Around line 452-459: The channel defaults in getChannelTypeScopedFieldDefaults
use unexplained magic numbers for type checks, so replace the raw 45 and 18
comparisons with named channel-type constants defined alongside the other
channel type symbols in channel-form.ts or the shared channel constants module.
Update the ternary logic in getChannelTypeScopedFieldDefaults to reference those
constants so the base_url and other defaults remain unchanged but the mapping is
self-documenting and easier to maintain.
---
Outside diff comments:
In `@relay/common/relay_info.go`:
- Around line 740-766: Preserve the nil/optional distinction in
TaskSubmitReq.UnmarshalJSON: the current Duration decoding path treats
duration:null as a zero value and assigns a pointer, which can later serialize
as an explicit 0. Update the duration handling in UnmarshalJSON to detect null
before attempting int/string parsing and leave t.Duration unset when
aux.Duration is null; keep the existing parsing behavior for numeric and string
values in TaskSubmitReq.
In `@web/default/src/routes/_authenticated/route.tsx`:
- Around line 41-61: The authenticated route’s beforeLoad logic is treating
every non-401 getSelf() failure as fatal, which breaks navigation when a cached
session is still valid. Update the getSelf()/res handling in the route component
to only redirect/reset auth on an actual 401, and treat transient non-401 errors
as non-fatal by falling back to the existing cached user instead of throwing.
Keep the logic localized around getSelf(), auth.reset(), auth.setUser(), and the
redirect call.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 79864c9c-2d8d-4050-a2c3-cbcadd1cd23b
📒 Files selected for processing (55)
.agents/skills/classic-to-default-sync/SKILL.md.agents/skills/i18n-translate/SKILL.md.agents/skills/shadcn-ui/SKILL.md.agents/skills/shadcn-ui/vendor/shadcn/SKILL.md.agents/skills/shadcn-ui/vendor/shadcn/UPSTREAM.txt.agents/skills/shadcn-ui/vendor/shadcn/cli.md.agents/skills/shadcn-ui/vendor/shadcn/customization.md.agents/skills/shadcn-ui/vendor/shadcn/mcp.md.agents/skills/shadcn-ui/vendor/shadcn/rules/base-vs-radix.md.agents/skills/shadcn-ui/vendor/shadcn/rules/composition.md.agents/skills/shadcn-ui/vendor/shadcn/rules/forms.md.agents/skills/shadcn-ui/vendor/shadcn/rules/icons.md.agents/skills/shadcn-ui/vendor/shadcn/rules/styling.md.agents/skills/vercel-react-best-practices/AGENTS.md.agents/upstream-sync/new-api-sync.md.gitignoreAGENTS.mdCLAUDE.mdcontroller/user.gomain.gomodel/user.gomodel/user_update_test.gorelay/channel/task/ali/adaptor.gorelay/channel/task/ali/adaptor_wan27_test.gorelay/channel/task/doubao/adaptor.gorelay/channel/task/gemini/adaptor.gorelay/channel/task/gemini/billing.gorelay/channel/task/hailuo/adaptor.gorelay/channel/task/jimeng/adaptor.gorelay/channel/task/kling/adaptor.gorelay/channel/task/sora/adaptor.gorelay/channel/task/taskcommon/generic_billing.gorelay/channel/task/taskcommon/generic_billing_test.gorelay/channel/task/taskcommon/request_body.gorelay/channel/task/taskcommon/request_body_test.gorelay/channel/task/vertex/adaptor.gorelay/channel/task/vidu/adaptor.gorelay/common/relay_info.gorelay/common/relay_info_test.gorelay/common/relay_utils.gorelay/common/relay_utils_test.goservice/convert.goservice/convert_test.gotools/jsonwrapcheck/allowlist.txtweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/default/src/features/channels/lib/channel-form.tsweb/default/src/features/playground/index.tsxweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/routes/_authenticated/route.tsxweb/default/tests/channel-form.test.ts
💤 Files with no reviewable changes (17)
- .agents/skills/shadcn-ui/vendor/shadcn/UPSTREAM.txt
- .agents/skills/shadcn-ui/vendor/shadcn/rules/icons.md
- .agents/skills/shadcn-ui/vendor/shadcn/customization.md
- .gitignore
- .agents/skills/i18n-translate/SKILL.md
- .agents/skills/shadcn-ui/vendor/shadcn/SKILL.md
- .agents/skills/shadcn-ui/vendor/shadcn/mcp.md
- .agents/skills/classic-to-default-sync/SKILL.md
- .agents/skills/shadcn-ui/SKILL.md
- .agents/skills/shadcn-ui/vendor/shadcn/rules/base-vs-radix.md
- .agents/skills/shadcn-ui/vendor/shadcn/rules/forms.md
- .agents/skills/shadcn-ui/vendor/shadcn/cli.md
- .agents/skills/shadcn-ui/vendor/shadcn/rules/composition.md
- .agents/skills/vercel-react-best-practices/AGENTS.md
- .agents/skills/shadcn-ui/vendor/shadcn/rules/styling.md
- tools/jsonwrapcheck/allowlist.txt
- .agents/upstream-sync/new-api-sync.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
web/default/**/*.{json,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In
web/default/, usebunas the preferred package manager and script runner (bun install,bun run dev,bun run build,bun run i18n:*).
Files:
web/default/tests/channel-form.test.tsweb/default/src/i18n/locales/ru.jsonweb/default/src/features/channels/lib/channel-form.tsweb/default/src/i18n/locales/zh.jsonweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/features/playground/index.tsxweb/default/src/routes/_authenticated/route.tsx
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Project identity references, branding, metadata, attributions, and protected names related to
max-apiandMAX API Nextmust not be modified, deleted, renamed, or replaced anywhere in the repository.
Files:
web/default/tests/channel-form.test.tsCLAUDE.mdAGENTS.mdrelay/common/relay_info_test.goweb/default/src/i18n/locales/ru.jsonweb/default/src/features/channels/lib/channel-form.tsrelay/channel/task/vertex/adaptor.gorelay/channel/task/gemini/billing.goweb/default/src/i18n/locales/zh.jsonweb/default/src/i18n/locales/en.jsonrelay/channel/task/gemini/adaptor.goweb/default/src/i18n/locales/fr.jsonrelay/channel/task/sora/adaptor.gorelay/channel/task/taskcommon/request_body_test.goweb/default/src/i18n/locales/vi.jsonrelay/channel/task/jimeng/adaptor.goweb/default/src/i18n/locales/ja.jsonservice/convert_test.goweb/default/src/features/playground/index.tsxrelay/channel/task/kling/adaptor.gorelay/common/relay_utils_test.gorelay/common/relay_utils.gorelay/channel/task/taskcommon/generic_billing_test.gorelay/common/relay_info.gomain.gorelay/channel/task/taskcommon/generic_billing.goservice/convert.goweb/default/src/routes/_authenticated/route.tsxmodel/user_update_test.gorelay/channel/task/ali/adaptor_wan27_test.gomodel/user.gorelay/channel/task/taskcommon/request_body.gorelay/channel/task/hailuo/adaptor.gorelay/channel/task/vidu/adaptor.gorelay/channel/task/doubao/adaptor.gorelay/channel/task/ali/adaptor.gocontroller/user.go
**/*.{go,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When working on tiered/dynamic billing (expression-based pricing), read and follow
pkg/billingexpr/expr.mdbefore changing the expression system.
Files:
CLAUDE.mdAGENTS.mdrelay/common/relay_info_test.gorelay/channel/task/vertex/adaptor.gorelay/channel/task/gemini/billing.gorelay/channel/task/gemini/adaptor.gorelay/channel/task/sora/adaptor.gorelay/channel/task/taskcommon/request_body_test.gorelay/channel/task/jimeng/adaptor.goservice/convert_test.gorelay/channel/task/kling/adaptor.gorelay/common/relay_utils_test.gorelay/common/relay_utils.gorelay/channel/task/taskcommon/generic_billing_test.gorelay/common/relay_info.gomain.gorelay/channel/task/taskcommon/generic_billing.goservice/convert.gomodel/user_update_test.gorelay/channel/task/ali/adaptor_wan27_test.gomodel/user.gorelay/channel/task/taskcommon/request_body.gorelay/channel/task/hailuo/adaptor.gorelay/channel/task/vidu/adaptor.gorelay/channel/task/doubao/adaptor.gorelay/channel/task/ali/adaptor.gocontroller/user.go
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType); do not directly callencoding/jsonfor marshal/unmarshal logic.
All database code must be fully compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions, avoid directAUTO_INCREMENT/SERIAL, and ensure any raw SQL or migrations use the project’s cross-DB helpers and fallbacks.
Files:
relay/common/relay_info_test.gorelay/channel/task/vertex/adaptor.gorelay/channel/task/gemini/billing.gorelay/channel/task/gemini/adaptor.gorelay/channel/task/sora/adaptor.gorelay/channel/task/taskcommon/request_body_test.gorelay/channel/task/jimeng/adaptor.goservice/convert_test.gorelay/channel/task/kling/adaptor.gorelay/common/relay_utils_test.gorelay/common/relay_utils.gorelay/channel/task/taskcommon/generic_billing_test.gorelay/common/relay_info.gomain.gorelay/channel/task/taskcommon/generic_billing.goservice/convert.gomodel/user_update_test.gorelay/channel/task/ali/adaptor_wan27_test.gomodel/user.gorelay/channel/task/taskcommon/request_body.gorelay/channel/task/hailuo/adaptor.gorelay/channel/task/vidu/adaptor.gorelay/channel/task/doubao/adaptor.gorelay/channel/task/ali/adaptor.gocontroller/user.go
{dto,relay}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types with
omitemptyso explicit zero/false values are preserved.
Files:
relay/common/relay_info_test.gorelay/channel/task/vertex/adaptor.gorelay/channel/task/gemini/billing.gorelay/channel/task/gemini/adaptor.gorelay/channel/task/sora/adaptor.gorelay/channel/task/taskcommon/request_body_test.gorelay/channel/task/jimeng/adaptor.gorelay/channel/task/kling/adaptor.gorelay/common/relay_utils_test.gorelay/common/relay_utils.gorelay/channel/task/taskcommon/generic_billing_test.gorelay/common/relay_info.gorelay/channel/task/taskcommon/generic_billing.gorelay/channel/task/ali/adaptor_wan27_test.gorelay/channel/task/taskcommon/request_body.gorelay/channel/task/hailuo/adaptor.gorelay/channel/task/vidu/adaptor.gorelay/channel/task/doubao/adaptor.gorelay/channel/task/ali/adaptor.go
web/default/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
web/default/src/**/*.{ts,tsx}: 前端页面文本与组件内文案必须支持 i18n：React 组件中应使用useTranslation()取得t，并通过t()渲染用户可见文本；子组件也应自行使用useTranslation()保持独立性。
禁止使用两层及以上嵌套三元表达式；复杂逻辑应改用if-else、提前返回或抽取函数。
控制函数圈复杂度，复杂逻辑应拆成更小的函数；变量与函数命名应有意义并遵循驼峰等常规命名约定。
TypeScript 代码应避免使用any，优先使用具体类型或unknown；参数与返回值应显式标注类型；仅类型用途的导入应使用import type。
修改 TypeScript 或 TSX 代码后必须执行类型检查，并修复所有类型错误，不得遗留。
对象非必要不要解构，尤其是组件 props；优先直接使用props.xxx以保持代码清晰。
组件应使用函数式组件与 Hooks，遵循单一职责；组件 props 必须有明确类型（接口或类型别名）。
单文件超过约 200 行时应考虑拆分子组件或抽取自定义 Hooks；类型定义可与组件同文件或放在同模块的types中。
在 React 中应合理使用useMemo、useCallback、React.memo，避免在渲染路径中创建新对象或数组；必要时进行代码分割与动态import。
React Query 的数据获取应使用useQuery、变更应使用useMutation；每个查询需配置唯一queryKey，并在成功后对相关查询执行invalidateQueries；服务端错误应统一交给handleServerError。
Axios 请求应使用项目统一的api实例；GET 请求默认去重，特殊请求可显式关闭；认证与通用错误应在拦截器中统一处理。
服务端错误应统一使用handleServerError，展示层应使用toast.error等统一方式；文案需走 i18n；路由级错误应由errorComponent承接；表单错误应通过form.setError等方式映射到字段。
样式应以 Tailwind 工具类为主，动态类名使用cn()合并；非动态场景避免内联样式；响应式采用移动优先与 Tailwind 断点，主题与暗色模式通过 CSS 变量与dark:处理。
应使用语义化 HTML、正确关联label与输入、保证键盘可操作与合理焦点顺序；必要时添加 ARIA 属性，装饰性图标应使用aria-hidden="true"。
认证与权限应在路由与接口层校验；前后端都应做数据校验（如 Zod）；敏感信息不得落前端存储；避免使用dangerouslySetInnerHTML；跨域与 Cookie 需配合withCredentials并按后端要求处理 CSRF。
组件测试应使用 React Testing Library，关注交互与行为，避免测试实现细节；关键流程可补充集成与 E2E 测试。
环境变量应通过.env读取，并使用VITE_前缀；代码中不得硬编码密钥。
Files:
web/default/src/features/channels/lib/channel-form.tsweb/default/src/features/playground/index.tsxweb/default/src/routes/_authenticated/route.tsx
web/default/src/features/**/lib/**/*.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
表单应使用 React Hook Form + Zod：在功能模块的
lib/下定义 schema，并用z.infer导出表单类型；useForm应配合@hookform/resolvers/zod进行校验。
Files:
web/default/src/features/channels/lib/channel-form.ts
web/default/src/features/**
📄 CodeRabbit inference engine (web/default/AGENTS.md)
功能模块应放在
src/features/<feature>/，并按需包含components/、lib/、hooks/、api.ts、types.ts、constants.ts等；通用组件应放在src/components/，通用工具与类型应放在src/lib/。
Files:
web/default/src/features/channels/lib/channel-form.tsweb/default/src/features/playground/index.tsx
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/task/vertex/adaptor.gorelay/channel/task/gemini/billing.gorelay/channel/task/gemini/adaptor.gorelay/channel/task/sora/adaptor.gorelay/channel/task/taskcommon/request_body_test.gorelay/channel/task/jimeng/adaptor.gorelay/channel/task/kling/adaptor.gorelay/channel/task/taskcommon/generic_billing_test.gorelay/channel/task/taskcommon/generic_billing.gorelay/channel/task/ali/adaptor_wan27_test.gorelay/channel/task/taskcommon/request_body.gorelay/channel/task/hailuo/adaptor.gorelay/channel/task/vidu/adaptor.gorelay/channel/task/doubao/adaptor.gorelay/channel/task/ali/adaptor.go
web/default/src/routes/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
路由应使用 TanStack Router，并通过
createFileRoute定义；搜索参数应使用 Zod schema +validateSearch校验；认证与重定向应放在beforeLoad中；导航应优先使用useNavigate或Link，避免直接操作window.location。
Files:
web/default/src/routes/_authenticated/route.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: MAX-API-Next/MAX-API
Timestamp: 2026-07-05T14:56:59.365Z
Learning: Do not modify, delete, replace, or remove protected project information related to `max-api` or `max-api-next` anywhere in the repository, including documentation, metadata, configs, code comments, and package/import paths.
Learnt from: CR
Repo: MAX-API-Next/MAX-API
Timestamp: 2026-07-05T14:56:59.365Z
Learning: All non-project process documents must be created and kept under `.tmp/`; do not place scratch notes, drafts, review outputs, or other internal process material in repository root, `docs/`, `.github/`, or source directories unless they are intended as public documentation.
🔇 Additional comments (38)
web/default/tests/channel-form.test.ts (1)
6-6: LGTM!Also applies to: 137-151
CLAUDE.md (1)
146-146: LGTM!AGENTS.md (1)
146-146: LGTM!web/default/src/features/playground/index.tsx (2)
214-214: LGTM!
155-158: 📐 Maintainability & Code QualityNo i18n sync issue. The new strings are already present in all locale files.
relay/common/relay_utils_test.go (1)
14-54: LGTM!relay/common/relay_utils.go (1)
131-136: LGTM!Also applies to: 171-179, 256-256
relay/channel/task/hailuo/adaptor.go (1)
151-156: LGTM!relay/common/relay_info_test.go (1)
6-6: LGTM!Also applies to: 42-53
relay/channel/task/taskcommon/request_body_test.go (1)
1-95: LGTM!service/convert_test.go (1)
39-48: LGTM!Also applies to: 76-88, 93-122
relay/channel/task/kling/adaptor.go (1)
378-390: LGTM!relay/channel/task/taskcommon/generic_billing_test.go (1)
163-166: LGTM!Also applies to: 229-229, 238-238
relay/common/relay_info.go (1)
688-735: LGTM!relay/channel/task/taskcommon/request_body.go (1)
91-186: LGTM!Also applies to: 252-262
relay/channel/task/vidu/adaptor.go (1)
233-247: LGTM!web/default/src/i18n/locales/ru.json (1)
3991-3992: LGTM!relay/channel/task/vertex/adaptor.go (1)
164-166: LGTM!relay/channel/task/gemini/billing.go (1)
86-88: LGTM!web/default/src/i18n/locales/zh.json (1)
3991-3992: LGTM!relay/channel/task/gemini/adaptor.go (1)
94-96: LGTM!relay/channel/task/sora/adaptor.go (1)
109-112: LGTM!web/default/src/i18n/locales/en.json (1)
3991-3992: LGTM!web/default/src/i18n/locales/fr.json (1)
3991-3992: LGTM!web/default/src/i18n/locales/ja.json (1)
3991-3992: LGTM!model/user_update_test.go (1)
1-166: LGTM!relay/channel/task/ali/adaptor_wan27_test.go (1)
1-223: LGTM!model/user.go (3)
85-85: LGTM!Also applies to: 174-174
556-572: 🗄️ Data Integrity & IntegrationCache refresh is safe here.
updateUserCache()only refreshesGroup,Status,Username, andSetting;Quotais updated separately by the atomic quota-delta paths, so this change does not repopulate stale billing data.> Likely an incorrect or invalid review comment.
111-114: 🗄️ Data Integrity & IntegrationNo issue:
ensureUserUpdateMatchedTxalready handles noop updates. It falls back to a lookup by user id whenRowsAffected == 0, so unchanged MySQL updates won’t be turned into用户不存在.> Likely an incorrect or invalid review comment.relay/channel/task/doubao/adaptor.go (1)
482-483: LGTM!controller/user.go (1)
927-927: LGTM!Also applies to: 1025-1027
web/default/src/i18n/locales/vi.json (1)
3991-3992: LGTM!relay/channel/task/jimeng/adaptor.go (1)
393-398: LGTM!main.go (1)
210-242: LGTM!relay/channel/task/taskcommon/generic_billing.go (1)
123-124: LGTM!Also applies to: 194-215
service/convert.go (1)
38-38: LGTM!Also applies to: 53-53, 644-663
relay/channel/task/ali/adaptor.go (1)
196-196: LGTM!Also applies to: 235-253, 444-533, 551-552
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
main.go (3)
236-238: 🚀 Performance & Scalability | 🔵 TrivialNo fallback when
srv.Shutdownfails.If
srv.Shutdown(ctx)returns an error (e.g., timeout), the error is only logged; there's no fallback such assrv.Close()to forcibly terminate remaining connections before proceeding to the quota-cache save and process exit.♻️ Suggested fallback
if err := srv.Shutdown(ctx); err != nil { common.SysError(fmt.Sprintf("server forced to shutdown: %v", err)) + _ = srv.Close() }🤖 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 `@main.go` around lines 236 - 238, The shutdown path in `srv.Shutdown(ctx)` only logs failures and then continues, so add a forced fallback in the same shutdown flow when it returns an error. Update the shutdown handling around `srv.Shutdown` to attempt `srv.Close()` after logging the shutdown error, then continue with the existing quota-cache save and exit sequence. Keep the fix localized to the server shutdown logic so the fallback is triggered only when graceful shutdown fails.
233-233: 🧹 Nitpick | 🔵 TrivialVerify deployment grace periods cover the new worst-case shutdown duration.
Default
SHUTDOWN_TIMEOUT_SECONDS(120s) plus defaultQUOTA_DATA_CACHE_SAVE_TIMEOUT_SECONDS(30s) sum to up to 150s of shutdown time. Ensure container/orchestrator termination grace periods (e.g., KubernetesterminationGracePeriodSeconds) are increased accordingly, otherwise SIGKILL may cut the graceful shutdown short.Also applies to: 240-240
🤖 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 `@main.go` at line 233, The shutdown path in main.go now allows up to 150s total by combining shutdownTimeout and QUOTA_DATA_CACHE_SAVE_TIMEOUT_SECONDS, so update the deployment/orchestrator termination grace period to cover that worst case. Check the service/container config that governs terminationGracePeriodSeconds or equivalent and raise it accordingly so the graceful shutdown in the main shutdown flow and cache save can finish before SIGKILL.
236-264: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAsync quota-cache save can race with the deferred
CloseDB()on timeout.When
runWithTimeout(saveTimeout, model.SaveQuotaDataCache)times out, the spawned goroutine keeps executingmodel.SaveQuotaDataCachein the background afterrunWithTimeoutreturnsfalse.main()then proceeds to"server exited"and returns, which triggers the deferredmodel.CloseDB()(registered near the top ofmain()). This creates a race: the leaked goroutine may still be writing to the database pool concurrently with (or after) it being closed, risking failed writes or noisy errors during shutdown.Consider either: not closing the DB until the save goroutine truly completes (e.g., track completion via a shared
sync.WaitGroup/channel thatCloseDBwaits on), or accepting the timeout but skipping/cancelling the save via a context-awareSaveQuotaDataCachevariant instead of leaving it to run detached.🤖 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 `@main.go` around lines 236 - 264, The detached timeout wrapper in runWithTimeout can leave model.SaveQuotaDataCache running after main() has moved on to the deferred model.CloseDB(), creating a shutdown race. Update the shutdown flow around runWithTimeout and model.SaveQuotaDataCache so the DB is not closed until the save work is definitely finished, or make the save cancellable/stop on timeout instead of continuing in the background. Use the existing runWithTimeout, SaveQuotaDataCache, and CloseDB symbols to coordinate completion before logging "server exited".
🤖 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/common/relay_utils.go`:
- Around line 175-179: Trim req.InputReference before using it to populate
req.Images in the relay utility logic, because the current check in the Images
assignment path can treat whitespace-only input_reference as a valid image and
produce an empty entry. Update the conditional around the existing
req.InputReference / req.Image fallback so it only assigns to Images when the
trimmed value is non-empty, matching the behavior already used for the image
fallback branch and preserving correct HasImage() behavior.
---
Outside diff comments:
In `@main.go`:
- Around line 236-238: The shutdown path in `srv.Shutdown(ctx)` only logs
failures and then continues, so add a forced fallback in the same shutdown flow
when it returns an error. Update the shutdown handling around `srv.Shutdown` to
attempt `srv.Close()` after logging the shutdown error, then continue with the
existing quota-cache save and exit sequence. Keep the fix localized to the
server shutdown logic so the fallback is triggered only when graceful shutdown
fails.
- Line 233: The shutdown path in main.go now allows up to 150s total by
combining shutdownTimeout and QUOTA_DATA_CACHE_SAVE_TIMEOUT_SECONDS, so update
the deployment/orchestrator termination grace period to cover that worst case.
Check the service/container config that governs terminationGracePeriodSeconds or
equivalent and raise it accordingly so the graceful shutdown in the main
shutdown flow and cache save can finish before SIGKILL.
- Around line 236-264: The detached timeout wrapper in runWithTimeout can leave
model.SaveQuotaDataCache running after main() has moved on to the deferred
model.CloseDB(), creating a shutdown race. Update the shutdown flow around
runWithTimeout and model.SaveQuotaDataCache so the DB is not closed until the
save work is definitely finished, or make the save cancellable/stop on timeout
instead of continuing in the background. Use the existing runWithTimeout,
SaveQuotaDataCache, and CloseDB symbols to coordinate completion before logging
"server exited".
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 920beb58-eef8-4f1a-b79b-fe0acbc48fc2
📒 Files selected for processing (14)
main.gomain_test.gorelay/channel/ollama/stream.gorelay/common/relay_utils.gorelay/common/relay_utils_test.goweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/default/src/features/channels/constants.tsweb/default/src/features/channels/lib/channel-form.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: All JSON marshal/unmarshal operations in business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType); do not directly callencoding/jsonthere.
Database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions, avoid raw SQL unless cross-database handling is included, use shared reserved-word/boolean helpers, avoid database-specific functions/operators/types without fallback, and ensure migrations work on all three databases.
For request structs parsed from client JSON and re-marshaled to upstream providers, especially in relay/convert paths, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved.
**/*.go: All JSON marshal/unmarshal operations in business code must use the wrappers incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/json.
Use GORM abstractions (Create,Find,Where,Updates, etc.) instead of raw SQL wherever possible, and let GORM handle primary key generation rather than usingAUTO_INCREMENTorSERIALdirectly.
When raw SQL is unavoidable, use database-specific column quoting and the shared helpers/flags (commonGroupCol,commonKeyCol,common.UsingPostgreSQL,common.UsingSQLite,common.UsingMySQL) to keep the SQL compatible across PostgreSQL, MySQL, and SQLite.
Do not use database-specific SQL features without a cross-DB fallback: avoid MySQL-only functions, PostgreSQL-only operators/JSONB features, SQLite-incompatibleALTER COLUMN, and database-specific column types without fallback.
Request structs parsed from client JSON and re-marshaled to upstream providers (especially in relay/convert paths) must use pointer fields withomitemptyfor optional scalars so explicit zero/false v...
Files:
main_test.gorelay/common/relay_utils_test.gomain.gorelay/common/relay_utils.gorelay/channel/ollama/stream.go
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Do not modify, delete, replace, or remove protected project identifiers and references related to
max-apiorMAX API Nextanywhere in the repository.
Files:
main_test.goweb/default/src/i18n/locales/en.jsonweb/default/src/features/channels/constants.tsweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/features/channels/lib/channel-form.tsweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/zh.jsonrelay/common/relay_utils_test.gomain.gorelay/common/relay_utils.goweb/default/src/i18n/locales/vi.jsonrelay/channel/ollama/stream.go
web/default/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
web/default/src/**/*.{ts,tsx}: 前端页面文本与组件内文案必须支持 i18n：React 组件中应使用useTranslation()取得t，并通过t()渲染用户可见文本；子组件也应自行使用useTranslation()保持独立性。
禁止使用两层及以上嵌套三元表达式；复杂逻辑应改用if-else、提前返回或抽取函数。
控制函数圈复杂度，复杂逻辑应拆成更小的函数；变量与函数命名应有意义并遵循驼峰等常规命名约定。
TypeScript 代码应避免使用any，优先使用具体类型或unknown；参数与返回值应显式标注类型；仅类型用途的导入应使用import type。
修改 TypeScript 或 TSX 代码后必须执行类型检查，并修复所有类型错误，不得遗留。
对象非必要不要解构，尤其是组件 props；优先直接使用props.xxx以保持代码清晰。
组件应使用函数式组件与 Hooks，遵循单一职责；组件 props 必须有明确类型（接口或类型别名）。
单文件超过约 200 行时应考虑拆分子组件或抽取自定义 Hooks；类型定义可与组件同文件或放在同模块的types中。
在 React 中应合理使用useMemo、useCallback、React.memo，避免在渲染路径中创建新对象或数组；必要时进行代码分割与动态import。
React Query 的数据获取应使用useQuery、变更应使用useMutation；每个查询需配置唯一queryKey，并在成功后对相关查询执行invalidateQueries；服务端错误应统一交给handleServerError。
Axios 请求应使用项目统一的api实例；GET 请求默认去重，特殊请求可显式关闭；认证与通用错误应在拦截器中统一处理。
服务端错误应统一使用handleServerError，展示层应使用toast.error等统一方式；文案需走 i18n；路由级错误应由errorComponent承接；表单错误应通过form.setError等方式映射到字段。
样式应以 Tailwind 工具类为主，动态类名使用cn()合并；非动态场景避免内联样式；响应式采用移动优先与 Tailwind 断点，主题与暗色模式通过 CSS 变量与dark:处理。
应使用语义化 HTML、正确关联label与输入、保证键盘可操作与合理焦点顺序；必要时添加 ARIA 属性，装饰性图标应使用aria-hidden="true"。
认证与权限应在路由与接口层校验；前后端都应做数据校验（如 Zod）；敏感信息不得落前端存储；避免使用dangerouslySetInnerHTML；跨域与 Cookie 需配合withCredentials并按后端要求处理 CSRF。
组件测试应使用 React Testing Library，关注交互与行为，避免测试实现细节；关键流程可补充集成与 E2E 测试。
环境变量应通过.env读取，并使用VITE_前缀；代码中不得硬编码密钥。
Files:
web/default/src/features/channels/constants.tsweb/default/src/features/channels/lib/channel-form.ts
web/default/src/features/**/constants.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
各 feature 的
constants.ts中，成功/错误/提示类消息常量应仅保存 i18n 键，展示时必须通过t()渲染；状态/选项的 label 应统一使用labelKey或label（二选一且同一 feature 内保持一致）；新增此类常量时应同步登记翻译键。
Files:
web/default/src/features/channels/constants.ts
web/default/src/features/**
📄 CodeRabbit inference engine (web/default/AGENTS.md)
功能模块应放在
src/features/<feature>/，并按需包含components/、lib/、hooks/、api.ts、types.ts、constants.ts等；通用组件应放在src/components/，通用工具与类型应放在src/lib/。
Files:
web/default/src/features/channels/constants.tsweb/default/src/features/channels/lib/channel-form.ts
web/default/src/features/**/lib/**/*.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
表单应使用 React Hook Form + Zod：在功能模块的
lib/下定义 schema，并用z.infer导出表单类型；useForm应配合@hookform/resolvers/zod进行校验。
Files:
web/default/src/features/channels/lib/channel-form.ts
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.When implementing a new channel, verify whether the provider supports
StreamOptions, and if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/ollama/stream.go
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: MAX-API-Next/MAX-API
Timestamp: 2026-07-05T15:35:20.202Z
Learning: Use `bun` as the preferred package manager and script runner for the frontend in `web/default/` (`bun install`, `bun run dev`, `bun run build`, `bun run i18n:*`).
Learnt from: CR
Repo: MAX-API-Next/MAX-API
Timestamp: 2026-07-05T15:35:20.202Z
Learning: Do not modify, delete, replace, or remove protected project information related to `max-api` or `max-api-next`, including references in README files, licenses, metadata, HTML, Go module paths, Docker/CI configs, comments, docs, and changelogs.
Learnt from: CR
Repo: MAX-API-Next/MAX-API
Timestamp: 2026-07-05T15:35:20.202Z
Learning: All non-project process documents must be created and kept under `.tmp/`; local agent workflow files such as `.agents/skills/` and `.agents/upstream-sync/` are private workspace materials and must not be committed to the public repository.
🔇 Additional comments (14)
main_test.go (1)
1-21: LGTM!web/default/src/i18n/locales/zh.json (1)
951-951: LGTM!Also applies to: 3991-3992
relay/common/relay_utils_test.go (1)
35-54: LGTM!web/default/src/i18n/locales/vi.json (1)
951-951: LGTM!Also applies to: 3991-3992
relay/channel/ollama/stream.go (1)
24-31: LGTM!Based on learnings, JSON marshal/unmarshal in business code should use
common.Marshal/common.Unmarshalwrappers rather thanencoding/jsondirectly — this change correctly complies with that requirement.Also applies to: 202-333
Source: Coding guidelines
main.go (1)
214-224: 🩺 Stability & AvailabilityNo extra return needed after
common.FatalLogcommon.FatalLogcallsos.Exit(1), so thenet.Listenfailure branch already terminates and won't reachsrv.Serve(ln).> Likely an incorrect or invalid review comment.web/default/src/i18n/locales/en.json (1)
951-951: LGTM!Also applies to: 3991-3992
web/default/src/i18n/locales/fr.json (1)
951-951: LGTM!Also applies to: 3991-3992
web/default/src/features/channels/constants.ts (1)
82-83: 📐 Maintainability & Code QualityNo duplicate channel-type constants
web/default/src/features/channels/lib/channel-form.tsimportsCHANNEL_TYPE_XUNFEIandCHANNEL_TYPE_VOLCENGINEfromweb/default/src/features/channels/constants.ts, so there’s a single source of truth here.> Likely an incorrect or invalid review comment.web/default/src/i18n/locales/ja.json (2)
951-951: LGTM!
3991-3992: LGTM!web/default/src/features/channels/lib/channel-form.ts (1)
22-23: LGTM!Also applies to: 454-465
web/default/src/i18n/locales/ru.json (2)
951-951: LGTM!
3991-3992: LGTM!
1 participant
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)