feat(models): auto disable/enable models by channel availability - #6456
feat(models): auto disable/enable models by channel availability#6456LaplaceOrange wants to merge 11 commits into
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 automatic model enable/disable reconciliation based on available channels, lifecycle triggers, manual batch endpoints, model status filtering, and frontend controls, actions, badges, and translations. ChangesModel channel availability automation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant ModelsUI
participant ModelsAPI
participant AvailabilityService
Admin->>ModelsUI: change automation switch or choose batch action
ModelsUI->>ModelsAPI: update option or POST batch endpoint
ModelsAPI->>AvailabilityService: reconcile model channel availability
AvailabilityService-->>ModelsAPI: return disabled/enabled counts
ModelsAPI-->>ModelsUI: return result
ModelsUI-->>Admin: show result and refresh model list
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
service/model_channel_availability.go (1)
27-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
forceFulldoc comment overstates its effect.The comment says
forceFull=true always evaluates all models; otherwise only when disable switch is on, but the model scan at lines 58-80 always covers every row regardless offorceFull; the flag only affects whether a zero-change result gets logged (119-124), and_ = forceFullat 126 is a no-op once the flag is already used. Worth tightening the comment (or the design) so future readers don't assume partial-scope scans exist.Also applies to: 113-126
🤖 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 `@service/model_channel_availability.go` around lines 27 - 47, Update the documentation for syncModelChannelAvailability and SyncModelChannelAvailabilityFull so forceFull is described accurately: model evaluation already scans all rows, while the flag only controls zero-change result logging. Remove the misleading “partial-skip” wording and eliminate the redundant _ = forceFull statement if it is no longer needed after documenting the actual behavior.web/src/features/models/lib/model-actions.ts (1)
278-349: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared logic between the two batch-availability handlers.
handleBatchDisableModelsNoChannelsandhandleBatchEnableModelsWithChannelsduplicate the same try/success-check/toast/invalidate/catch flow, differing only by the API call, the result field name, and the i18n messages.♻️ Proposed shared helper
+async function runBatchAvailabilityAction( + apiCall: () => Promise<{ success: boolean; message?: string; data?: { disabled: number; enabled: number } }>, + countField: 'disabled' | 'enabled', + messages: { success: (count: number) => string; empty: string; failure: string }, + queryClient?: QueryClient, + onSuccess?: (count: number) => void +): Promise<void> { + try { + const response = await apiCall() + if (response.success) { + const count = response.data?.[countField] ?? 0 + if (count > 0) { + toast.success(messages.success(count)) + } else { + toast.info(messages.empty) + } + queryClient?.invalidateQueries({ queryKey: modelsQueryKeys.lists() }) + onSuccess?.(count) + } else { + toast.error(response.message || messages.failure) + } + } catch (error: unknown) { + toast.error((error as Error)?.message || messages.failure) + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/features/models/lib/model-actions.ts` around lines 278 - 349, Extract the duplicated try/success-check/toast/invalidate/catch flow from handleBatchDisableModelsNoChannels and handleBatchEnableModelsWithChannels into a shared private helper. Parameterize the helper with the batch API call, result count, success/empty/error messages, and operation-specific toast text, then have both handlers delegate to it while preserving their existing callbacks and query invalidation.
🤖 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 `@controller/channel.go`:
- Around line 1097-1100: Update the sync guard in UpdateChannel to compare only
channel.Models against originChannel.Models, removing the ineffective
channel.Status comparison while preserving the existing
SyncModelChannelAvailability("channel.update") call.
In `@controller/model_sync.go`:
- Around line 437-441: Update chooseStatus to preserve a legitimate fallback
status of 0 instead of returning hardcoded status 1 when both inputs are zero,
while preserving existing call-site behavior including chooseStatus(up.Status,
1). In the status overwrite flow around local.AutoDisabledByRule, clear the
marker only when the selected status genuinely differs from the existing local
status; otherwise preserve the disabled state and marker.
In `@service/model_channel_availability.go`:
- Around line 189-200: Update the manual batch enable UPDATE for enableAutoIDs
in manualSyncModelChannelAvailability to include auto_disabled_by_rule = true in
its WHERE clause, mirroring the sync enable path’s defensive condition. Preserve
the existing update behavior while preventing it from re-enabling a marker
cleared concurrently by ClearModelAutoDisabledByRule.
In `@web/src/features/models/components/models-primary-buttons.tsx`:
- Around line 74-81: Gate the system-wide batch actions behind confirmation by
updating the relevant DropdownMenuItem onClick handlers to call setConfirmAction
instead of invoking handleBatchDisableNoChannels or
handleBatchEnableWithChannels directly. Add a Dialog matching the existing
DataTableBulkActions confirmation pattern, and invoke the selected handler only
after explicit confirmation.
In `@web/src/i18n/locales/ru.json`:
- Around line 509-510: Translate the values for the Russian locale keys
"Auto-disable models with no available channels" and "Auto-enable models
disabled by this setting when a channel recovers" into Russian, while preserving
the keys and JSON structure.
In `@web/src/i18n/locales/vi.json`:
- Around line 509-510: Translate the Vietnamese locale values for “Auto-disable
models with no available channels” and “Auto-enable models disabled by this
setting when a channel recovers” in the locale entries, replacing the English
text with natural Vietnamese while preserving the keys unchanged.
In `@web/src/i18n/locales/zh-TW.json`:
- Around line 509-510: Translate the values for “Auto-disable models with no
available channels” and “Auto-enable models disabled by this setting when a
channel recovers” in the zh-TW locale to natural Traditional Chinese, while
preserving both existing keys and JSON validity.
In `@web/src/i18n/locales/zh.json`:
- Line 511: Update the Chinese translations for the batch model enable/disable
strings, including the related entries around the second referenced location, to
preserve the distinction between recovered channels and unavailable channels.
Replace wording that only indicates configured channels with wording equivalent
to “models with recovered channels” and “models with no available channels,”
while keeping the existing automation meaning.
---
Nitpick comments:
In `@service/model_channel_availability.go`:
- Around line 27-47: Update the documentation for syncModelChannelAvailability
and SyncModelChannelAvailabilityFull so forceFull is described accurately: model
evaluation already scans all rows, while the flag only controls zero-change
result logging. Remove the misleading “partial-skip” wording and eliminate the
redundant _ = forceFull statement if it is no longer needed after documenting
the actual behavior.
In `@web/src/features/models/lib/model-actions.ts`:
- Around line 278-349: Extract the duplicated
try/success-check/toast/invalidate/catch flow from
handleBatchDisableModelsNoChannels and handleBatchEnableModelsWithChannels into
a shared private helper. Parameterize the helper with the batch API call, result
count, success/empty/error messages, and operation-specific toast text, then
have both handlers delegate to it while preserving their existing callbacks and
query invalidation.
🪄 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 Plus
Run ID: 0a5c2210-e9bb-491b-9a72-9a1835bc3825
📒 Files selected for processing (31)
.gitignorecommon/constants.gocontroller/channel.gocontroller/model_meta.gocontroller/model_sync.gocontroller/option.gomodel/model_meta.gomodel/option.gorouter/api-router.goservice/channel.goservice/model_channel_availability.goservice/model_channel_availability_test.goservice/task_billing_test.goweb/src/features/models/api.tsweb/src/features/models/components/drawers/model-mutate-drawer.tsxweb/src/features/models/components/models-availability-switches.tsxweb/src/features/models/components/models-columns.tsxweb/src/features/models/components/models-primary-buttons.tsxweb/src/features/models/components/models-table.tsxweb/src/features/models/constants.tsweb/src/features/models/lib/model-actions.tsweb/src/features/models/types.tsweb/src/features/system-settings/models/index.tsxweb/src/features/system-settings/types.tsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.json
|
@bigsongeth 求审批 |
|
抱歉,我不是本项目的维护者,只是提过一个 PR 的外部贡献者,没有仓库权限,审批不了 🙏 能 review / merge 的是维护者 Calcium-Ion 和 seefs001,建议找他们。 另外顺手提一句:这个 PR 目前跟 main 已经冲突了(mergeable: CONFLICTING),CodeRabbit 那轮也还留着 8 条 actionable comments。先 rebase 并把这些处理掉,应该更容易推进。 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@model/option.go`:
- Around line 210-213: Refactor the paired-flag handling around updateOptionMap
and the load/bulk reconciliation paths to canonicalize enabled values
consistently, including "1", and reconcile AutomaticEnableModelEnabled with
AutomaticDisableModelEnabled independent of processing order. Normalize both
flags before persistence, clear and persist the child when the parent is
disabled, and persist the resulting pair atomically after bulk updates and
loadOptionsFromDatabase so in-memory and database state remain synchronized.
In `@web/src/i18n/locales/fr.json`:
- Line 5229: Update the French translation for the “No disabled models with
available channels found” key to preserve the source meaning: disabled models
with available channels. Remove the auto-disabled and recovered-channel wording
that duplicates the nearby translation, while leaving the neighboring locale
entries unchanged.
In `@web/src/i18n/locales/ru.json`:
- Line 5229: Update the Russian translation value for the “No disabled models
with available channels found” key to describe generic disabled models with
available channels, removing references to automatic disabling and recovered
channels while preserving the key’s intended meaning.
In `@web/src/i18n/locales/vi.json`:
- Line 5229: Update the Vietnamese translation value for “No disabled models
with available channels found” to preserve the key’s broader meaning, referring
to all disabled models with available channels without adding the narrower
“automatically disabled” condition.
In `@web/src/i18n/locales/zh-TW.json`:
- Line 5229: Update the translation value for “No disabled models with available
channels found” in the zh-TW locale to directly express that no disabled models
with available channels were found, removing the narrower references to
rule-based disabling and recovered channels.
🪄 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 Plus
Run ID: 2d5f525f-c908-4729-be24-f56bd18c5aa9
📒 Files selected for processing (16)
controller/model_meta.gocontroller/model_sync.gomodel/option.goservice/model_channel_availability.goservice/model_channel_availability_test.goservice/task_billing_test.goweb/src/features/models/components/models-availability-switches.tsxweb/src/features/models/components/models-primary-buttons.tsxweb/src/features/models/lib/model-actions.tsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes (10)
- service/task_billing_test.go
- web/src/i18n/locales/en.json
- service/model_channel_availability.go
- web/src/features/models/components/models-primary-buttons.tsx
- web/src/features/models/components/models-availability-switches.tsx
- controller/model_sync.go
- web/src/features/models/lib/model-actions.ts
- web/src/i18n/locales/zh.json
- service/model_channel_availability_test.go
- controller/model_meta.go
91336b1 to
7f8f1b0
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
model/model_meta.go (1)
261-279: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove or use the unused
parseModelStatusFilterwrapper.
golangci-lintreports this function as unused, while callers now useparseModelStatusFilterSpec. Delete the stale compatibility wrapper or migrate a real status-only caller to it.🤖 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/model_meta.go` around lines 261 - 279, Remove the unused parseModelStatusFilter wrapper from model metadata parsing, since callers now use parseModelStatusFilterSpec directly. Do not migrate callers unless an actual status-only caller exists; retain parseModelStatusFilterSpec and its current behavior unchanged.Source: Linters/SAST tools
controller/model_meta.go (1)
152-188: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFlatten the unnecessary
elsefor early-return style.The
if statusOnly { ...; return }branch already returns, so the followingelsejust adds nesting for the rest of the function. As per coding guidelines,**/*.{go,ts,tsx}: "prefer early returns, clear branches, and well-named local variables over deep nesting or layered control flow."♻️ Proposed restructure
- } else { - // 名称冲突检查 - if dup, err := model.IsModelNameDuplicated(m.Id, m.ModelName); err != nil { - common.ApiError(c, err) - return - } else if dup { - common.ApiErrorMsg(c, "模型名称已存在") - return - } + } + // 名称冲突检查 + if dup, err := model.IsModelNameDuplicated(m.Id, m.ModelName); err != nil { + common.ApiError(c, err) + return + } else if dup { + common.ApiErrorMsg(c, "模型名称已存在") + return + } - // Preserve previous status to detect explicit status changes. - var prev model.Model - _ = model.DB.Select("id", "status", "model_name", "name_rule").Where("id = ?", m.Id).First(&prev).Error + // Preserve previous status to detect explicit status changes. + var prev model.Model + _ = model.DB.Select("id", "status", "model_name", "name_rule").Where("id = ?", m.Id).First(&prev).Error - if err := m.Update(); err != nil { - common.ApiError(c, err) - return - } + if err := m.Update(); err != nil { + common.ApiError(c, err) + return + } ... - common.ApiSuccess(c, &m) - return - } + common.ApiSuccess(c, &m) + 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/model_meta.go` around lines 152 - 188, Remove the unnecessary else wrapping the metadata update path after the statusOnly branch, since that branch already returns. Dedent the duplicate-name validation, previous-model lookup, update, synchronization, pricing refresh, and success response logic while preserving their existing behavior.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@controller/model_meta.go`:
- Around line 152-188: Remove the unnecessary else wrapping the metadata update
path after the statusOnly branch, since that branch already returns. Dedent the
duplicate-name validation, previous-model lookup, update, synchronization,
pricing refresh, and success response logic while preserving their existing
behavior.
In `@model/model_meta.go`:
- Around line 261-279: Remove the unused parseModelStatusFilter wrapper from
model metadata parsing, since callers now use parseModelStatusFilterSpec
directly. Do not migrate callers unless an actual status-only caller exists;
retain parseModelStatusFilterSpec and its current behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 92f9bd1a-0e07-41ab-9790-f477b7357f50
📒 Files selected for processing (8)
.gitignorecommon/constants.gocontroller/channel.gocontroller/model_meta.gocontroller/model_sync.gocontroller/option.gomodel/model_meta.gomodel/option.go
🚧 Files skipped from review as they are similar to previous changes (5)
- .gitignore
- common/constants.go
- controller/option.go
- controller/model_sync.go
- controller/channel.go
|
@Calcium-Ion @seefs001 求审核awa |
|
@coderabbitai review |
✅ Action performedReview finished.
|
f0164d4 to
d6b4a1f
Compare
…ntumNous#6456) When model metadata has no available channels, auto-disable it; re-enable only models previously disabled by this rule when channels recover. Adds admin switches, status badges/filters, and batch availability actions. Upstream: QuantumNous#6456
Reconcile model metadata status against enabled channels and abilities. Add AutomaticDisableModelEnabled / AutomaticEnableModelEnabled options, batch admin actions, status filters/badges, and coverage for channel lifecycle hooks. Preserve manual disable decisions via auto_disabled_by_rule.
Keep feature commits code-only by excluding local output, playwright artifacts, server logs, temp helpers, and i18n report dumps.
Guard UpdateChannel sync on models only, preserve disabled status=0 in chooseStatus, clear auto markers only on real status change, harden manual auto-enable WHERE, require batch-action confirmation, and improve locale wording for recovered vs unavailable channels.
Only recover auto-disabled models in batch enable, pair enable switch with disable (runtime + persistence), avoid duplicate pricing refresh, re-evaluate after manual status edits, and complete locale strings.
# Conflicts: # service/task_billing_test.go
d6b4a1f to
8c31127
Compare
|
感谢贡献,但是这个功能改的太大了,要不先电邮和我沟通一下想法 i@caion.me |
Make channel and model mutations transactional and cache-coherent. Reject stale upstream results, coordinate batched model status changes, and cover retry, locking, cache invalidation, and UI behavior with regression tests.
Important
📝 变更描述 / Description
当模型元数据已配置,但没有可用渠道(或渠道被禁用)时,前台仍可能展示“可用”模型,导致调用失败。
本 PR 增加“按渠道可用性自动启停模型元数据状态”能力:
AutomaticDisableModelEnabled:无可用渠道时自动禁用模型AutomaticEnableModelEnabled:仅恢复被规则自动禁用、且渠道已恢复的模型models.auto_disabled_by_ruleEnabled/Disabled/Auto-enabled/Auto-disabled本地验证:
go test ./service -run ModelChannelAvailability通过;页面手动验证开关、筛选与批量菜单可见。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
UI 截图(/models/metadata)
模型页 + 自动开关 + 状态 Badge
claude-3显示Auto-disabled(无可用渠道)gpt-4显示Enabled(渠道test-channel可用)批量操作菜单
Disable Models with No ChannelsEnable Models with Available Channelsclaude-3,Total=1API 抽查(root 登录后)
POST /api/models/batch_disable_no_channels→ successPOST /api/models/batch_enable_with_channels→ success,曾恢复 1 个有渠道模型AutomaticDisableModelEnabled=trueAutomaticEnableModelEnabled=true备注
auto_disabled_by_rule仍为 true,Badge 显示Auto-enabled,便于识别规则托管状态。Summary by CodeRabbit