feat: ionet integrate - #2105
Conversation
# Conflicts: # controller/channel.go # go.mod # model/main.go # relay/channel/ollama/dto.go # relay/channel/ollama/relay-ollama.go # web/src/components/table/channels/modals/EditChannelModal.jsx # web/src/i18n/locales/en.json # web/src/i18n/locales/zh.json
# Conflicts: # controller/channel.go # relay/channel/ollama/dto.go # relay/channel/ollama/relay-ollama.go # web/src/components/table/channels/modals/EditChannelModal.jsx # web/src/i18n/locales/en.json # web/src/i18n/locales/zh.json
WalkthroughThis pull request introduces comprehensive IO.NET deployment management and Ollama model operations. It adds backend Go client libraries, API handlers, database models, frontend pages with modal forms, hooks for state management, and route configuration. Integrations span from low-level HTTP clients to high-level deployment lifecycle management, container operations, pricing estimation, and hardware/location queries. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/pages/Setting/Personal/SettingsSidebarModulesUser.jsx (1)
101-112: UI/config mismatch: add deployment to Admin section modules so it’s user‑toggleable.You populate
defaultConfig.admin.deployment, but the Admin section module list doesn’t include it, so it can’t be toggled.modules: [ { key: 'channel', title: t('渠道管理'), description: t('API渠道配置') }, { key: 'models', title: t('模型管理'), description: t('AI模型配置') }, + { key: 'deployment', title: t('模型部署'), description: t('模型部署管理') }, { key: 'redemption', title: t('兑换码管理'), description: t('兑换码生成管理'), },Also consider removing
console.logstatements or guarding them behind a debug flag to keep console clean in production.Also applies to: 320-338
web/src/components/table/channels/modals/EditChannelModal.jsx (1)
489-494: Guard JSON.parse on model_mapping (crash fix).
Invalid JSON from DB will throw and break the modal.- if (data.model_mapping !== '') { - data.model_mapping = JSON.stringify( - JSON.parse(data.model_mapping), - null, - 2, - ); - } + if (data.model_mapping !== '') { + try { + data.model_mapping = JSON.stringify(JSON.parse(data.model_mapping), null, 2); + } catch (e) { + console.warn('Invalid model_mapping JSON, keeping raw string:', e); + // keep original string to let user fix it + } + }
🧹 Nitpick comments (70)
web/src/components/table/channels/ChannelsColumnDefs.jsx (3)
93-101: Minor hardening for window.openAdd noreferrer to the feature string to drop the Referer header. Optional.
- window.open(targetUrl, '_blank', 'noopener'); + window.open(targetUrl, '_blank', 'noopener,noreferrer');
102-129: Make IO.NET tag accessible and conditionally clickableAdd link semantics and keyboard support; only clickable when deployment_id exists; adjust cursor/aria accordingly.
- return ( + const isNavigable = Boolean(ionetMeta?.deployment_id); + return ( <Space spacing={6}> {typeTag} <Tooltip content={ <div className='max-w-xs'> <div className='text-xs text-gray-600'>{t('来源于 IO.NET 部署')}</div> - {ionetMeta?.deployment_id && ( + {ionetMeta?.deployment_id && ( <div className='text-xs text-gray-500 mt-1'> {t('部署 ID')}: {ionetMeta.deployment_id} </div> )} </div> } > - <span> + <span> <Tag color='purple' type='light' - className='cursor-pointer' - onClick={handleNavigate} + className={isNavigable ? 'cursor-pointer' : 'cursor-not-allowed opacity-60'} + onClick={isNavigable ? handleNavigate : undefined} + role='link' + tabIndex={0} + aria-disabled={!isNavigable} + aria-label={isNavigable ? t('打开 IO.NET 部署详情') : t('未绑定 IO.NET 部署')} + onKeyDown={(e) => { + if (!isNavigable) return; + if (e.key === 'Enter' || e.key === ' ') handleNavigate(e); + }} > IO.NET </Tag> </span> </Tooltip> </Space> );
46-55: Signature change verified as safe; recommend Map refactoringThe renderType signature change to (type, record = {}, t) is isolated to ChannelsColumnDefs.jsx. The callsite at line 338 correctly passes all three arguments. Each other file has its own renderType definition, so no breaking changes.
The Map misuse remains valid: type2label is created with
new Map()but accessed via bracket notation like a plain object.- let type2label = new Map(); - for (let i = 0; i < CHANNEL_OPTIONS.length; i++) { - type2label[CHANNEL_OPTIONS[i].value] = CHANNEL_OPTIONS[i]; - } - type2label[0] = { value: 0, label: t('未知类型'), color: 'grey' }; + const type2label = { 0: { value: 0, label: t('未知类型'), color: 'grey' } }; + for (const opt of CHANNEL_OPTIONS) { + type2label[opt.value] = opt; + }pkg/ionet/jsonutil_test.go (2)
10-29: Broaden assertions and cover container_config fieldsAdd checks for container_config (image_url, traffic_port, etc.) and assert exact values; this prevents silent regressions in struct tags and decoding.
Apply:
assert.True(t, resp.Data.CreatedAt.Equal(expectedTime)) assert.Equal(t, 15, resp.Data.ComputeMinutesServed) assert.Equal(t, 45, resp.Data.ComputeMinutesRemaining) + assert.Equal(t, "example", resp.Data.ContainerConfig.ImageURL) + assert.Equal(t, 0, resp.Data.ContainerConfig.TrafficPort) + assert.Len(t, resp.Data.ContainerConfig.Entrypoint, 0) + assert.Empty(t, resp.Data.ContainerConfig.EnvVariables)
31-51: Add offset and millis-only timestamp casesConsider a subtest table with timestamps like "2025-10-03T06:46:25+08:00" and "2025-10-03T06:46:25.316Z" to ensure decodeWithFlexibleTimes normalizes to UTC consistently.
web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx (3)
53-55: Remove unused model-loading logic and related stateModels are fetched but never rendered; avoids extra network load every open.
Apply:
- const [models, setModels] = useState([]); - const [loadingModels, setLoadingModels] = useState(false); + // (removed) models state not needed for rename @@ - // Load available models - const loadModels = async () => { - setLoadingModels(true); - try { - const res = await API.get('/api/models/?page_size=1000'); - if (res.data.success) { - const items = res.data.data.items || res.data.data || []; - const modelOptions = items.map(model => ({ - label: `${model.model_name} (${model.vendor?.name || 'Unknown'})`, - value: model.model_name, - model_id: model.id, - })); - setModels(modelOptions); - } - } catch (error) { - console.error('Failed to load models:', error); - showError(t('加载模型列表失败')); - } - setLoadingModels(false); - }; + // (removed) loadModels @@ - useEffect(() => { - if (visible) { - loadModels(); - } - }, [visible]); + // (removed) model loading on openAlso delete unused imports: InputNumber, Select, Divider.
Also applies to: 85-105, 134-140
116-121: Trim and validate name before PUTPrevent sending whitespace-only names.
- const res = await API.put(`/api/deployments/${editingDeployment.id}/name`, { - name: values.deployment_name, - }); + const name = (values.deployment_name || '').trim(); + if (!name) { showError(t('请输入部署名称')); return; } + const res = await API.put(`/api/deployments/${editingDeployment.id}/name`, { name });
60-68: Drop unused resource option constantscpuOptions/memoryOptions/gpuOptions are dead code here.
web/src/components/table/model-deployments/modals/UpdateConfigModal.jsx (3)
68-75: Initialize from correct source fields (image_url likely under registry_config)Create flow uses registry_config.image_url; mirror that here to avoid blank prefill.
- image_url: deployment.container_config?.image_url || '', + image_url: deployment.registry_config?.image_url + ?? deployment.container_config?.image_url + ?? '',
24-36: Remove unused UI importsInputNumber (standalone), TextArea, Switch are not used.
- InputNumber, Typography, Card, Space, Divider, Button, Banner, Tag, Collapse, - TextArea, - Switch,
107-110: Entrypoint parsing is space-split; note quoted args limitationIf users paste quoted commands, splitting on spaces breaks. Consider JSON array input or a chips-style editor later.
web/src/components/table/model-deployments/DeploymentsActions.jsx (2)
86-93: Use a single translatable string with interpolationConcatenation harms localization. Prefer a message with a count variable.
- content={`${t('确定要删除选中的')} ${selectedKeys.length} ${t('个部署吗？此操作不可逆。')}`} + content={t('确定要删除选中的 {{count}} 个部署吗？此操作不可逆。', { count: selectedKeys.length })}
73-114: Minor: redundant disabled props inside guarded blockButtons are already hidden when none selected. You can drop disabled checks here to simplify.
web/src/components/table/model-deployments/DeploymentsFilters.jsx (1)
39-45: Reset-submit can be synchronousSemi Form typically allows immediate submit after reset; setTimeout is likely unnecessary.
- formApiRef.current.reset(); - setTimeout(() => { - formApiRef.current.submitForm(); - }, 0); + formApiRef.current.reset(); + formApiRef.current.submitForm();web/src/components/table/model-deployments/modals/ExtendDurationModal.jsx (5)
57-84: Replace mock cost with real estimator; avoid hard-coded currency.Wire this to the backend price‑estimation (or reuse useDeploymentResources.calculatePrice) and drop the fixed 0.5/USDC. Keep numbers numeric in state and format at render.
Example minimal change sketch (fallback to mock if API data absent):
- const calculateEstimatedCost = async (hours) => { + const calculateEstimatedCost = async (hours) => { if (!deployment?.id || hours <= 0) { setEstimatedCost(null); return; } setCostLoading(true); try { - const hourlyRate = 0.5; // Mock hourly rate per GPU - const gpuCount = deployment.hardware_quantity || 1; - const cost = hours * hourlyRate * gpuCount; - setEstimatedCost({ - totalCost: cost.toFixed(2), - hourlyRate: hourlyRate.toFixed(2), - gpuCount, - currency: 'USDC' - }); + // Prefer backend price estimation if inputs are available + const gpuCount = Number(deployment?.hardware_quantity ?? deployment?.gpus_per_container ?? 1); + let estimation = null; + try { + const body = { + hardware_id: deployment?.hardware_id, + location_ids: deployment?.location_ids, + gpus_per_container: gpuCount, + duration_hours: hours, + replica_count: Number(deployment?.replica_count ?? 1), + }; + if (body.hardware_id && body.location_ids?.length) { + const { data } = await API.post('/api/deployments/price-estimation', body); + if (data?.success) estimation = data.data; + } + } catch (_) { /* fall back to local calc */ } + const hourlyRate = Number(estimation?.hourly_rate ?? 0.5); + const currency = estimation?.currency ?? 'USDC'; + const totalCost = Number(estimation?.total_cost ?? hours * hourlyRate * gpuCount); + setEstimatedCost({ totalCost, hourlyRate, gpuCount, currency }); } catch (error) { console.error('Failed to calculate cost:', error); setEstimatedCost(null); } finally { setCostLoading(false); } };
108-112: Pass AxiosError to showError to preserve 401 handling.Building a string bypasses the helper’s 401 redirect/cleanup. Forward the original error, and optionally show a friendly message separately.
- } catch (error) { - showError(t('延长时长失败') + ': ' + (error.response?.data?.message || error.message)); + } catch (error) { + showError(error); } finally {
124-126: Localize literals and use interpolation for units.Avoid hard‑coded “0分钟/小时” to keep i18n consistent.
- const currentRemainingTime = deployment?.time_remaining || '0分钟'; - const newTotalTime = `${currentRemainingTime} + ${durationHours}小时`; + const currentRemainingTime = deployment?.time_remaining || t('0分钟'); + const newTotalTime = t('{{current}} + {{hours}}{{unit}}', { + current: currentRemainingTime, + hours: durationHours, + unit: t('小时'), + });
271-281: Avoid mixing “$” with a currency code.Show either symbol or code, not both. Prefer code for clarity.
- <Text strong>${estimatedCost.hourlyRate} {estimatedCost.currency}</Text> + <Text strong>{estimatedCost.hourlyRate} {estimatedCost.currency}</Text> ... - <Text strong className="text-lg text-green-600"> - ${estimatedCost.totalCost} {estimatedCost.currency} - </Text> + <Text strong className="text-lg text-green-600"> + {estimatedCost.totalCost} {estimatedCost.currency} + </Text>
20-34: Remove unused import.InputNumber is not used directly (Form.InputNumber is). Drop it.
- InputNumber,web/src/components/table/model-deployments/modals/ViewLogsModal.jsx (5)
253-256: Include UTF‑8 charset for log downloads.Prevents mojibake for non‑ASCII logs.
- const blob = new Blob([logText], { type: 'text/plain' }); + const blob = new Blob([logText], { type: 'text/plain;charset=utf-8' });
528-537: Open external links safely.Add noopener/noreferrer to prevent tab‑nabbing.
- onClick={() => window.open(containerDetails.public_url, '_blank')} + onClick={() => window.open(containerDetails.public_url, '_blank', 'noopener,noreferrer')}
425-433: Expose “ALL” stream option (code already supports it).UI only allows stdout/stderr; add “all” to match fetchLogs behavior.
<Radio.Group type="button" size="small" value={streamFilter} onChange={handleStreamChange} > <Radio value="stdout">STDOUT</Radio> <Radio value="stderr">STDERR</Radio> + <Radio value="all">ALL</Radio> </Radio.Group>No code changes needed elsewhere; resolveStreamValue and fetchLogs already handle 'all'.
Also applies to: 117-121
135-139: Preserve standardized auth handling in errors.Forward the AxiosError instead of a concatenated string (same for other catch blocks in this file).
- } catch (error) { - showError(t('获取日志失败') + ': ' + (error.response?.data?.message || error.message)); + } catch (error) { + showError(error); } finally {Apply similarly at Lines 166-169 and 186-189.
373-381: UsebodyStyleinstead ofheightprop for Semi Modal.Semi Modal does support the
heightprop, but the documented best practice is to constrain modal body height viabodyStyle— e.g.,bodyStyle={{ overflow: 'auto', maxHeight: '600px' }}— rather than applying height at the Modal level. This gives better control over the body content area and aligns with Semi's recommended patterns.web/src/hooks/model-deployments/useDeploymentResources.js (3)
20-23: Consolidate imports from helpers.Minor cleanup.
-import { API } from '../../helpers'; -import { showError } from '../../helpers'; +import { API, showError } from '../../helpers';
164-205: Align calculatePrice payload with creation flow and surface errors.Add optional currency parameter for parity with CreateDeployment, and show user‑visible errors on catch.
- const calculatePrice = useCallback(async (params) => { + const calculatePrice = useCallback(async (params) => { const { locationIds, hardwareId, gpusPerContainer, durationHours, - replicaCount + replicaCount, + currency, // optional } = params; @@ try { setLoadingPrice(true); const requestData = { location_ids: locationIds, hardware_id: hardwareId, gpus_per_container: gpusPerContainer, duration_hours: durationHours, replica_count: replicaCount, + ...(currency ? { currency: currency.toUpperCase?.() ?? currency } : {}), + // For backends expecting alternate fields; harmless if ignored: + duration_type: 'hour', + duration_qty: durationHours, + hardware_qty: gpusPerContainer, }; @@ - } catch (error) { - console.error('Price calculation error:', error); + } catch (error) { + console.error('Price calculation error:', error); + showError(error); setPriceEstimation(null); return null; } finally {
141-162: Surface errors to users when loading replicas fails.Currently only console.error on catch; consider showError for parity with other loaders.
- } catch (error) { - console.error('Load available replicas error:', error); + } catch (error) { + console.error('Load available replicas error:', error); + showError(error); setAvailableReplicas([]); return [];web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx (2)
20-31: Remove unused imports.useRef and Title are unused.
-import React, { useState, useRef } from 'react'; +import React, { useState } from 'react'; @@ -const { Text, Title } = Typography; +const { Text } = Typography;
55-57: Handle empty requiredText.If neither name nor id is present, disable the name step and keep button gated by checkboxes only, or block confirmation entirely until an identifier is available.
- const requiredText = deployment?.container_name || deployment?.id || ''; - const isConfirmed = confirmText === requiredText && acknowledged && (type !== 'danger' || doubleConfirmed); + const requiredText = deployment?.container_name || deployment?.id || ''; + const nameRequired = Boolean(requiredText); + const isConfirmed = + (!nameRequired || confirmText === requiredText) && + acknowledged && + (type !== 'danger' || doubleConfirmed); @@ - <Input + <Input value={confirmText} onChange={setConfirmText} placeholder={t('输入容器名称确认')} + disabled={!nameRequired} style={{ borderColor: confirmText === requiredText ? '#52c41a' : undefined }} /> - {confirmText && confirmText !== requiredText && ( + {nameRequired && confirmText && confirmText !== requiredText && (Also applies to: 190-206
docs/ionet-client.md (1)
1-6: Format the example and include auth/body; avoid bare URL.
- Current snippet violates MD034 (bare URL) and lacks headers/body context.
- Suggest a concise cURL + JSON example.
Apply this rewrite:
-Request URL -https://api.io.solutions/v1/io-cloud/clusters/654fc0a9-0d4a-4db4-9b95-3f56189348a2/update-name -Request Method -PUT - -{"status":"succeeded","message":"Cluster name updated successfully"} +### Update cluster name + +```bash +curl -X PUT \ + "https://api.io.solutions/v1/io-cloud/clusters/{cluster_id}/update-name" \ + -H "Authorization: Bearer $IONET_API_KEY" \ + -H "Content-Type: application/json" \ + -d '{"name":"my-cluster"}' +``` + +Example response: + +```json +{ "status": "succeeded", "message": "Cluster name updated successfully" } +```web/src/hooks/common/useSidebar.js (1)
71-83: Deep‑merge stored admin config with defaults to surface new modules for existing installs.If SidebarModulesAdmin exists but predates ‘deployment’, users won’t see the new module. Merge with defaults.
const adminConfig = useMemo(() => { if (statusState?.status?.SidebarModulesAdmin) { try { - const config = JSON.parse(statusState.status.SidebarModulesAdmin); - return config; + const config = JSON.parse(statusState.status.SidebarModulesAdmin); + return mergeDeep(defaultAdminConfig, config); } catch (error) { return defaultAdminConfig; } } return defaultAdminConfig; }, [statusState?.status?.SidebarModulesAdmin]);Add helper (outside this block):
function mergeDeep(target, source) { if (typeof target !== 'object' || target === null) return source; const out = Array.isArray(target) ? [...target] : { ...target }; for (const [k, v] of Object.entries(source || {})) { out[k] = v && typeof v === 'object' && !Array.isArray(v) ? mergeDeep(out[k] ?? {}, v) : v; } return out; }web/src/pages/Setting/Operation/SettingsSidebarModulesAdmin.jsx (1)
61-70: Deployment module wiring looks correct; dedupe default structures.Addition in initial state, reset defaults, fallback defaults, and UI modules is consistent. Consider extracting a single getDefaultAdminModules() to avoid drift.
Example:
const defaultAdminModules = () => ({ chat:{enabled:true,playground:true,chat:true}, console:{enabled:true,detail:true,token:true,log:true,midjourney:true,task:true}, personal:{enabled:true,topup:true,personal:true}, admin:{enabled:true,channel:true,models:true,deployment:true,redemption:true,user:true,setting:true}, });Then reuse it in state init, reset, and JSON-parse fallback.
Also applies to: 100-130, 170-202, 253-256
web/src/components/layout/SiderBar.jsx (1)
48-49: Align deployment menu item path with routerMap.The item uses
to: '/deployment'but routerMap maps to/console/deployment. The wrapper resolves by itemKey, so it works, but keep paths consistent for readability.- to: '/deployment', + to: '/console/deployment',Reduce role-gating duplication for this item.
Visibility already respects
isModuleVisible('admin', 'deployment')and the Admin section gate. Consider removing the extraclassName: isAdmin() ? '' : 'tableHiddle'for this new item to lean on the centralized permissions system. Based on learnings.Also applies to: 161-166
pkg/ionet/jsonutil.go (1)
61-71: Support more timestamp shapes (space separator) commonly seen in APIs.Add space‑separated layouts alongside the existing T‑separator ones.
layouts := []string{ - "2006-01-02T15:04:05.999999999", - "2006-01-02T15:04:05.999999", - "2006-01-02T15:04:05", + "2006-01-02T15:04:05.999999999", + "2006-01-02T15:04:05.999999", + "2006-01-02T15:04:05", + "2006-01-02 15:04:05.999999999", + "2006-01-02 15:04:05.999999", + "2006-01-02 15:04:05", }Additionally, consider routing JSON (un)marshal via common/json to keep behavior consistent project‑wide.
pkg/ionet/client_test.go (1)
141-220: Nice param assertions for price estimation; consider adding a header test.Add a small test to assert Authorization header is set on requests (if client code sets it), using MockHTTPClient.MatchedBy to inspect
req.Headers["Authorization"].web/src/components/model-deployments/DeploymentAccessGuard.jsx (1)
196-227: Refactor hover handling to use React state.The inline
onMouseEnter/onMouseLeavehandlers that directly mutatee.target.stylecan fail when hovering over child elements (like the Settings icon on line 224). Whene.targetis the child element rather than the div, styles are applied incorrectly.Use React state to manage hover styling:
+ const [isHovered, setIsHovered] = useState(false); + <div onClick={handleGoToSettings} style={{ display: 'inline-flex', alignItems: 'center', gap: '8px', cursor: 'pointer', padding: '12px 24px', borderRadius: '8px', fontSize: '16px', fontWeight: '500', color: 'var(--semi-color-primary)', - background: 'var(--semi-color-fill-0)', + background: isHovered ? 'var(--semi-color-fill-1)' : 'var(--semi-color-fill-0)', border: '1px solid var(--semi-color-border)', transition: 'all 0.2s ease', - textDecoration: 'none' + textDecoration: 'none', + transform: isHovered ? 'translateY(-1px)' : 'translateY(0)', + boxShadow: isHovered ? '0 2px 8px rgba(0, 0, 0, 0.1)' : 'none', }} - onMouseEnter={(e) => { - e.target.style.background = 'var(--semi-color-fill-1)'; - e.target.style.transform = 'translateY(-1px)'; - e.target.style.boxShadow = '0 2px 8px rgba(0, 0, 0, 0.1)'; - }} - onMouseLeave={(e) => { - e.target.style.background = 'var(--semi-color-fill-0)'; - e.target.style.transform = 'translateY(0)'; - e.target.style.boxShadow = 'none'; - }} + onMouseEnter={() => setIsHovered(true)} + onMouseLeave={() => setIsHovered(false)} > <Settings size={18} /> {t('前往设置页面')} </div>web/src/components/settings/ModelDeploymentSetting.jsx (1)
44-50: Consider adding key filtering for robustness.The parsing logic directly assigns any key from the API response to
newInputswithout checking if it's an expected key. This differs from the similar logic inuseModelDeploymentSettings.js(lines 42-50) which useshasOwnPropertyto filter known keys. While not immediately problematic, this could allow unexpected API keys to enter component state.Apply this diff to match the pattern in
useModelDeploymentSettings.js:data.forEach((item) => { if (item.key.endsWith('Enabled') || item.key.endsWith('enabled')) { newInputs[item.key] = toBoolean(item.value); - } else { + } else if (newInputs.hasOwnProperty(item.key)) { newInputs[item.key] = item.value; } });web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
43-53: Add labels for new/active columns to avoid fallback keys in UI.Keys like container_name, time_remaining, hardware_info aren’t labeled; UI falls back to raw keys.
const columnLabels = { - [columnKeys.deployment_name]: t('部署名称'), + [columnKeys.container_name]: t('容器名称'), + [columnKeys.time_remaining]: t('剩余时间'), + [columnKeys.hardware_info]: t('硬件信息'), + [columnKeys.deployment_name]: t('部署名称'), // legacy [columnKeys.model_name]: t('模型名称'), [columnKeys.status]: t('状态'), [columnKeys.instance_count]: t('实例数量'), [columnKeys.resource_config]: t('资源配置'), [columnKeys.created_at]: t('创建时间'), [columnKeys.updated_at]: t('更新时间'), [columnKeys.actions]: t('操作'), };Based on learnings.
web/src/components/table/model-deployments/DeploymentsTable.jsx (2)
132-144: Missing dependency: updateDeploymentName in columns useMemo.Stale closure risk if the handler changes.
}, [ t, COLUMN_KEYS, startDeployment, restartDeployment, deleteDeployment, + updateDeploymentName, syncDeploymentToChannel, setEditingDeployment, setShowEdit, refresh, activePage, deployments, ]);
146-150: Default to showing unknown/new columns instead of hiding when no visibility entry exists.Current filter treats undefined as false, unintentionally hiding newly added columns.
- const getVisibleColumns = () => { - return allColumns.filter((column) => visibleColumns[column.key]); - }; + const getVisibleColumns = () => { + return allColumns.filter((column) => visibleColumns[column.key] !== false); + };web/src/components/table/model-deployments/modals/ViewDetailsModal.jsx (2)
125-126: Use fetched details.status for display (fallback to prop only before load).Prevents stale/incorrect status in modal.
- const statusConfig = getStatusConfig(deployment?.status); + const statusConfig = getStatusConfig(details?.status ?? deployment?.status ?? '');And keep the existing rendering as-is.
Also applies to: 200-205
103-106: Clipboard fallback for browsers without navigator.clipboard.Minor robustness win.
const handleCopyId = () => { - navigator.clipboard.writeText(deployment?.id); - showSuccess(t('ID已复制到剪贴板')); + const text = deployment?.id ?? ''; + if (navigator.clipboard?.writeText) { + navigator.clipboard.writeText(text).then( + () => showSuccess(t('ID已复制到剪贴板')), + () => showError(t('复制失败')) + ); + } else { + const ta = document.createElement('textarea'); + ta.value = text; + document.body.appendChild(ta); + ta.select(); + try { document.execCommand('copy'); showSuccess(t('ID已复制到剪贴板')); } + catch { showError(t('复制失败')); } + finally { document.body.removeChild(ta); } + } };web/src/pages/Setting/Model/SettingModelDeployment.jsx (1)
197-201: Remove debug log before merge.Stray console.log in settings init.
- console.log('Setting inputs from props:', currentInputs);relay/channel/ollama/relay-ollama.go (4)
289-326: Add timeout, use bytes.NewReader, and unify error handling.
- Use an HTTP client with a sane timeout; current client has none.
- Prefer bytes.NewReader(requestBody) over converting []byte to string.
- Set Accept: application/json and keep error messages consistent (English).
- client := &http.Client{} + client := &http.Client{ Timeout: 30 * time.Second } - request, err := http.NewRequest("GET", url, nil) + request, err := http.NewRequest("GET", url, nil) if err != nil { - return nil, fmt.Errorf("创建请求失败: %v", err) + return nil, fmt.Errorf("failed to create request: %w", err) } + request.Header.Set("Accept", "application/json") ... - return nil, fmt.Errorf("请求失败: %v", err) + return nil, fmt.Errorf("request failed: %w", err) ... - return nil, fmt.Errorf("服务器返回错误 %d: %s", response.StatusCode, string(body)) + return nil, fmt.Errorf("server returned %d: %s", response.StatusCode, string(body)) ... - return nil, fmt.Errorf("读取响应失败: %v", err) + return nil, fmt.Errorf("read response failed: %w", err) ... - return nil, fmt.Errorf("解析响应失败: %v", err) + return nil, fmt.Errorf("unmarshal response failed: %w", err)
329-367: Prefer idiomatic timeouts and bytes reader; add Accept header.Use time.Minute units and bytes.NewReader; include Accept header.
- client := &http.Client{ - Timeout: 30 * 60 * 1000 * time.Millisecond, // 30分钟超时，支持大模型 - } + client := &http.Client{ Timeout: 30 * time.Minute } - request, err := http.NewRequest("POST", url, strings.NewReader(string(requestBody))) + request, err := http.NewRequest("POST", url, bytes.NewReader(requestBody)) if err != nil { - return fmt.Errorf("创建请求失败: %v", err) + return fmt.Errorf("failed to create request: %w", err) } request.Header.Set("Content-Type", "application/json") + request.Header.Set("Accept", "application/json")
370-435: Scanner default buffer may truncate; support long lines and idiomatic timeout.
- bufio.Scanner caps tokens at 64KB; Ollama progress lines can exceed this. Increase buffer.
- Use time.Hour and bytes.NewReader; add Accept.
- client := &http.Client{ - Timeout: 60 * 60 * 1000 * time.Millisecond, // 1小时超时，支持超大模型 - } + client := &http.Client{ Timeout: time.Hour } - request, err := http.NewRequest("POST", url, strings.NewReader(string(requestBody))) + request, err := http.NewRequest("POST", url, bytes.NewReader(requestBody)) ... request.Header.Set("Content-Type", "application/json") + request.Header.Set("Accept", "application/json") ... - scanner := bufio.NewScanner(response.Body) + scanner := bufio.NewScanner(response.Body) + buf := make([]byte, 0, 256*1024) + scanner.Buffer(buf, 2*1024*1024) // allow up to 2MB per line
438-473: DELETE with body: add timeout, bytes reader, and Accept; improve errors.
- Provide a client timeout; use bytes.NewReader; set Accept header; standardize error text.
- client := &http.Client{} - request, err := http.NewRequest("DELETE", url, strings.NewReader(string(requestBody))) + client := &http.Client{ Timeout: 30 * time.Second } + request, err := http.NewRequest("DELETE", url, bytes.NewReader(requestBody)) if err != nil { - return fmt.Errorf("创建请求失败: %v", err) + return fmt.Errorf("failed to create request: %w", err) } request.Header.Set("Content-Type", "application/json") + request.Header.Set("Accept", "application/json") ... - return fmt.Errorf("请求失败: %v", err) + return fmt.Errorf("request failed: %w", err) ... - return fmt.Errorf("删除模型失败 %d: %s", response.StatusCode, string(body)) + return fmt.Errorf("delete model failed %d: %s", response.StatusCode, string(body))web/src/hooks/model-deployments/useEnhancedDeploymentActions.jsx (5)
41-43: Remove unused variable.operationKey is computed but never used.
- const operationKey = `extend_${deploymentId}`; try { setOperationLoading('extend', deploymentId, true);
66-77: Return a defined value when success is false.When API returns success: false, the function currently returns undefined. Prefer returning null/throwing for consistency.
- if (response.data.success) { - return response.data.data; - } + if (response.data.success) return response.data.data; + throw new Error(response.data.message || 'getDeploymentDetails failed');
84-95: Make URLSearchParams conversion explicit.Use params.toString() to avoid accidental "[object URLSearchParams]" if a custom toString is overridden.
- const response = await API.get(`/api/deployments/${deploymentId}/logs?${params}`); + const response = await API.get(`/api/deployments/${deploymentId}/logs?${params.toString()}`);
199-219: Harden timestamp formatting for mixed types.API timestamps may be epoch seconds/ms or ISO strings. Normalize to avoid "Invalid Date".
- const logText = logs.logs.map(log => - `[${new Date(log.timestamp).toISOString()}] [${log.level}] ${log.source ? `[${log.source}] ` : ''}${log.message}` - ).join('\n'); + const toISO = (ts) => { + if (typeof ts === 'number') { + const ms = ts > 1e12 ? ts : ts * 1000; + return new Date(ms).toISOString(); + } + const d = new Date(ts); + return isNaN(d.getTime()) ? new Date().toISOString() : d.toISOString(); + }; + const logText = logs.logs.map(log => + `[${toISO(log.timestamp)}] [${log.level}] ${log.source ? `[${log.source}] ` : ''}${log.message}` + ).join('\n');
126-143: Surface non-success as errors for delete action.Mirror other methods: throw on success: false so callers can aggregate failures.
- if (response.data.success) { - showSuccess(t('容器销毁请求已提交')); - return response.data.data; - } + if (response.data.success) { + showSuccess(t('容器销毁请求已提交')); + return response.data.data; + } + throw new Error(response.data.message || 'deleteDeployment failed');pkg/ionet/examples/basic_usage.go (1)
181-216: Repeat nil guard and simplify rename flow.Ensure deployments != nil before accessing index 0; handle errors uniformly.
- if len(deployments.Deployments) > 0 { + if deployments != nil && len(deployments.Deployments) > 0 {pkg/ionet/client.go (3)
83-96: Normalize baseURL to avoid double slashes and ease endpoint joins.Trim trailing slash once; endpoints can consistently start with '/'.
-func NewClientWithConfig(apiKey, baseURL string, httpClient HTTPClient) *Client { +func NewClientWithConfig(apiKey, baseURL string, httpClient HTTPClient) *Client { if baseURL == "" { baseURL = DefaultBaseURL } + baseURL = strings.TrimRight(baseURL, "/") if httpClient == nil { httpClient = NewDefaultHTTPClient(DefaultTimeout) } return &Client{ BaseURL: baseURL, APIKey: apiKey, HTTPClient: httpClient, } }Add import:
-import ( +import ( "bytes" "encoding/json" "fmt" "net/http" "net/url" "strconv" "time" + "strings" )
110-121: Skip X-API-KEY when empty (some endpoints may not require it).Avoid sending empty auth headers.
- headers := map[string]string{ - "X-API-KEY": c.APIKey, + headers := map[string]string{ "Content-Type": "application/json", "Accept": "application/json", } + if c.APIKey != "" { + headers["X-API-KEY"] = c.APIKey + }
161-193: Extend query param support (int64, float64, slices).Common options include int64/float64 and []int/[]string.
for key, value := range params { if value != nil { switch v := value.(type) { case string: if v != "" { values.Add(key, v) } case int: if v != 0 { values.Add(key, strconv.Itoa(v)) } + case int64: + if v != 0 { values.Add(key, strconv.FormatInt(v, 10)) } + case float64: + if v != 0 { values.Add(key, strconv.FormatFloat(v, 'f', -1, 64)) } case bool: values.Add(key, strconv.FormatBool(v)) case time.Time: if !v.IsZero() { values.Add(key, v.Format(time.RFC3339)) } + case []string: + for _, s := range v { if s != "" { values.Add(key, s) } } + case []int: + for _, n := range v { if n != 0 { values.Add(key, strconv.Itoa(n)) } } } } }web/src/hooks/model-deployments/useDeploymentsData.jsx (3)
84-89: Avoid in-place mutation; map to a new array for React state.Immutability reduces surprises and aids change detection.
- const setDeploymentFormat = (deployments) => { - for (let i = 0; i < deployments.length; i++) { - deployments[i].key = deployments[i].id; - } - setDeployments(deployments); - }; + const setDeploymentFormat = (items) => { + const withKeys = (items || []).map((d) => ({ ...d, key: d.id })); + setDeployments(withKeys); + };
155-187: Always clear loading via finally.Minor: moving setLoading(false) into a finally ensures it runs on all paths.
- ) => { - setLoading(true); - try { + ) => { + setLoading(true); + try { ... - } catch (error) { + } catch (error) { ... - } - setLoading(false); + } finally { + setLoading(false); + } };
322-344: Normalize baseUrl; guard against whitespace-only values.Trim and validate before use.
- const rawUrl = String(activeContainer.public_url).trim(); - const baseUrl = rawUrl.replace(/\/+$/, ''); - if (!baseUrl) { + const rawUrl = String(activeContainer.public_url || '').trim(); + const baseUrl = rawUrl.replace(/\/+$/, ''); + if (!baseUrl) { showError(t('容器访问地址无效')); return; }relay/channel/ollama/dto.go (1)
75-83: Consider parsing modified_at as time.Time (optional).
If you’ll sort or filter by time, prefer time.Time with tolerant decoding; otherwise string is fine.-type OllamaModel struct { - ... - ModifiedAt string `json:"modified_at"` - ... -} +type OllamaModel struct { + ... + ModifiedAt time.Time `json:"modified_at"` + ... +}web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (3)
52-63: Harden random key generation.
Math.random is weak. Fall back to crypto.getRandomValues when randomUUID is unavailable.-const generateRandomKey = () => { - try { - if (typeof crypto !== 'undefined' && crypto.randomUUID) { - return `ionet-${crypto.randomUUID().replace(/-/g, '')}`; - } - } catch (error) { - // ignore - } - return `ionet-${Math.random().toString(36).slice(2)}${Math.random() - .toString(36) - .slice(2)}`; -}; +const generateRandomKey = () => { + try { + if (typeof crypto !== 'undefined') { + if (crypto.randomUUID) { + return `ionet-${crypto.randomUUID().replace(/-/g, '')}`; + } + if (crypto.getRandomValues) { + const buf = new Uint8Array(16); + crypto.getRandomValues(buf); + return `ionet-${Array.from(buf).map(b => b.toString(16).padStart(2,'0')).join('')}`; + } + } + } catch {} + return `ionet-${Math.random().toString(36).slice(2)}${Math.random().toString(36).slice(2)}`; +};
130-151: Debounce and cancel price estimation to avoid stale updates.
Rapid changes can race responses and flicker UI. Use a debounce and axios CancelToken/AbortController.- useEffect(() => { + useEffect(() => { if ( selectedHardwareId && selectedLocationIds.length > 0 && gpusPerContainer > 0 && durationHours > 0 && replicaCount > 0 ) { - calculatePrice(); + const h = setTimeout(() => calculatePrice(true), 300); + return () => clearTimeout(h); } else { setPriceEstimation(null); } }, [selectedHardwareId, selectedLocationIds, gpusPerContainer, durationHours, replicaCount, priceCurrency]); - const calculatePrice = async () => { + const calculatePrice = async (debounced = false) => { + const controller = new AbortController(); + const signal = controller.signal; + // cancel in-flight on next call + calculatePrice._abort?.(); + calculatePrice._abort = () => controller.abort(); try { setLoadingPrice(true); const requestData = { location_ids: selectedLocationIds, hardware_id: selectedHardwareId, gpus_per_container: gpusPerContainer, duration_hours: durationHours, replica_count: replicaCount, currency: priceCurrency?.toUpperCase?.() || priceCurrency, duration_type: 'hour', duration_qty: durationHours, hardware_qty: gpusPerContainer, }; - const response = await API.post('/api/deployments/price-estimation', requestData); + const response = await API.post('/api/deployments/price-estimation', requestData, { signal, skipErrorHandler: true }); if (response.data.success) { setPriceEstimation(response.data.data); } else { showError(t('价格计算失败: ') + response.data.message); setPriceEstimation(null); } } catch (error) { - console.error('Price calculation error:', error); + if (error.name !== 'CanceledError' && error.name !== 'AbortError') { + console.error('Price calculation error:', error); + } setPriceEstimation(null); } finally { setLoadingPrice(false); } };Also applies to: 395-423
429-443: Normalize env entries before submit.
Trim keys/values and drop empty keys to avoid sending malformed envs.- const envVars = {}; - envVariables.forEach(env => { - if (env.key && env.value) { - envVars[env.key] = env.value; - } - }); + const envVars = {}; + envVariables.forEach(({ key, value }) => { + const k = String(key || '').trim(); + const v = String(value || '').trim(); + if (k) envVars[k] = v; + }); - const secretEnvVars = {}; - secretEnvVariables.forEach(env => { - if (env.key && env.value) { - secretEnvVars[env.key] = env.value; - } - }); + const secretEnvVars = {}; + secretEnvVariables.forEach(({ key, value }) => { + const k = String(key || '').trim(); + const v = String(value || '').trim(); + if (k) secretEnvVars[k] = v; + });Also applies to: 452-473
pkg/ionet/hardware.go (1)
189-196: Be resilient to wrapped responses.
Some endpoints return { "data": {...} }. Support both shapes.- var hardwareType HardwareType - if err := json.Unmarshal(resp.Body, &hardwareType); err != nil { + var hardwareType HardwareType + var wrapped struct{ Data HardwareType `json:"data"` } + if err := json.Unmarshal(resp.Body, &hardwareType); err != nil { + if err := json.Unmarshal(resp.Body, &wrapped); err != nil { + return nil, fmt.Errorf("failed to parse hardware type: %w", err) + } + hardwareType = wrapped.Data + } return &hardwareType, nilApply the same pattern for GetLocation and GetLocationAvailability.
pkg/ionet/container.go (1)
183-201: Add cancellation for streaming (optional).
Pass context to allow callers to stop streaming and to bound total duration.- func (c *Client) StreamContainerLogs(deploymentID, containerID string, opts *GetLogsOptions, callback func(*LogEntry) error) error { + func (c *Client) StreamContainerLogs(ctx context.Context, deploymentID, containerID string, opts *GetLogsOptions, callback func(*LogEntry) error) error { ... - for { + for { + select { + case <-ctx.Done(): + return ctx.Err() + default: + } ... time.Sleep(2 * time.Second) }Also applies to: 254-259
web/src/components/table/channels/modals/OllamaModelModal.jsx (1)
325-344: Optional SSE hardening: add no-cache headers.Helps with proxies and some browsers for event-stream.
const fetchHeaders = { 'Content-Type': 'application/json', Accept: 'text/event-stream', + 'Cache-Control': 'no-cache', + Pragma: 'no-cache', 'New-API-User': String(userId), ...authHeaders, };web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (1)
318-326: Guard completed_percent scale (0–1 vs 0–100).If API returns 0–1, current rounding yields 0%/1%. Normalize when <= 1.
- const parsed = typeof rawValue === 'string' - ? parseFloat(rawValue.replace(/[^0-9.+-]/g, '')) - : Number(rawValue ?? 0); - const percentUsed = Number.isFinite(parsed) - ? Math.min(100, Math.max(0, Math.round(parsed))) + const parsed = typeof rawValue === 'string' + ? parseFloat(rawValue.replace(/[^0-9.+-]/g, '')) + : Number(rawValue ?? 0); + const normalized = Number.isFinite(parsed) && parsed <= 1 ? parsed * 100 : parsed; + const percentUsed = Number.isFinite(normalized) + ? Math.min(100, Math.max(0, Math.round(normalized))) : null;Confirm the API returns completed_percent in 0–100. If it's 0–1, this guard is required. Based on learnings.
Also applies to: 334-343
controller/deployment.go (1)
228-246: Consider including human-friendly name in GetDeployment.Currently deployment_name = details.ID. If a display/name exists (cluster name), return it for consistent UI.
pkg/ionet/deployment.go (1)
55-66: Avoid sending empty/zero-valued query params by default.Passing zero/empty values can confuse APIs and break caching.
Filter params when building the map:
- if opts != nil { - params["status"] = opts.Status - params["location_id"] = opts.LocationID - params["page"] = opts.Page - params["page_size"] = opts.PageSize - params["sort_by"] = opts.SortBy - params["sort_order"] = opts.SortOrder - } + if opts != nil { + if s := strings.TrimSpace(opts.Status); s != "" { params["status"] = s } + if opts.LocationID > 0 { params["location_id"] = opts.LocationID } + if opts.Page > 0 { params["page"] = opts.Page } + if opts.PageSize > 0 { params["page_size"] = opts.PageSize } + if s := strings.TrimSpace(opts.SortBy); s != "" { params["sort_by"] = s } + if s := strings.TrimSpace(opts.SortOrder); s != "" { params["sort_order"] = s } + }pkg/ionet/types.go (1)
96-103: Consider narrowing EnvVariables to map[string]string if API guarantees strings.Looser typing complicates consumers and marshaling. If values can be non-string, keep as-is; otherwise prefer map[string]string for safety and DX.
Confirm the IO.NET API allows non-string env var values.
| "instance_count": d.HardwareQuantity, | ||
| "resource_config": map[string]interface{}{ | ||
| "cpu": "", | ||
| "memory": "", | ||
| "gpu": strconv.Itoa(d.HardwareQuantity), | ||
| }, | ||
| "description": "", | ||
| } |
There was a problem hiding this comment.
Inconsistent instance_count/name between list and detail responses.
- List items (mapIoNetDeployment) set instance_count from HardwareQuantity.
- GetDeployment sets instance_count from TotalContainers.
- ExtendDeployment maps Name to deploymentID (not the cluster name), so container_name becomes the ID on the refreshed item.
This yields confusing UI deltas (counts/names change across pages).
- Align instance_count across endpoints. Prefer TotalContainers when available; otherwise derive consistently.
- In ExtendDeployment, set Name from details (if present) or omit updating name to avoid regressions.
Proposed patches:
- Use TotalContainers when available in mapIoNetDeployment (fallback to HardwareQuantity):
- "instance_count": d.HardwareQuantity,
+ "instance_count": func() int { if d.Replicas > 0 { return d.Replicas }; if d.HardwareQuantity > 0 { return d.HardwareQuantity }; return 0 }(),- In ExtendDeployment mapping, keep Name consistent:
- Name: deploymentID,
+ Name: details.ID, // or details.ClusterName if availableAnd adjust mapIoNetDeployment to derive container_name from a stable field. Based on learnings.
Also applies to: 149-161, 198-206
🤖 Prompt for AI Agents
In controller/deployment.go around lines 100-107 (also address similar logic at
149-161 and 198-206): mapIoNetDeployment currently sets instance_count from
HardwareQuantity and container_name/Name from mutable fields, while
GetDeployment uses TotalContainers and ExtendDeployment overwrites Name with
deployment ID; update mapIoNetDeployment to set instance_count = TotalContainers
when >0 else fallback to HardwareQuantity, derive container_name from a stable
immutable field (e.g., cluster or service name) instead of mutable ID, and
modify ExtendDeployment mapping so Name is set from the detailed deployment
response if present (or skip updating Name) to avoid replacing the cluster name
with the deployment ID; ensure all three affected ranges use the same
TotalContainers fallback logic and consistent Name/container_name source.
| @@ -0,0 +1,7 @@ | |||
| Request URL | |||
| https://api.io.solutions/v1/io-cloud/clusters/654fc0a9-0d4a-4db4-9b95-3f56189348a2/update-name | |||
There was a problem hiding this comment.
🧩 Analysis chain
Verify endpoint path vs client base URL.
Client sets DefaultEnterpriseBaseURL to an enterprise/caas prefix; docs use /v1/io-cloud/ without enterprise/caas. Confirm which is correct and align.
🏁 Script executed:
#!/bin/bash
# Show client base URLs and any update-name routes to confirm doc path
rg -n --glob '!**/node_modules/**' -C2 'Default(Enterprise)?BaseURL|io-cloud|update-name' pkg router || trueLength of output: 1818
Fix documentation endpoint path to match client base URL.
The docs example at line 2 omits the /caas segment. The client's DefaultBaseURL and DefaultEnterpriseBaseURL both include /caas at the end, so the actual constructed endpoint would be https://api.io.solutions/v1/io-cloud/caas/clusters/{clusterID}/update-name, not the path shown in the docs. Update the example to include /caas.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
2-2: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In docs/ionet-client.md around line 2, the example endpoint is missing the /caas
segment; update the URL to include /caas so it matches the client's
DefaultBaseURL/DefaultEnterpriseBaseURL (use
https://api.io.solutions/v1/io-cloud/caas/clusters/654fc0a9-0d4a-4db4-9b95-3f56189348a2/update-name).
| // GetContainerLogsRaw retrieves the raw text logs for a specific container | ||
| func (c *Client) GetContainerLogsRaw(deploymentID, containerID string, opts *GetLogsOptions) (string, error) { | ||
| endpoint, err := buildLogEndpoint(deploymentID, containerID, opts) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| resp, err := c.makeRequest("GET", endpoint, nil) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to get container logs: %w", err) | ||
| } | ||
|
|
||
| return string(resp.Body), nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Clarify logs format (raw text vs JSON).
GetContainerLogsRaw returns text, StreamContainerLogs expects JSON. Confirm the endpoint behavior or add content-type detection and fallback.
Also applies to: 230-244
🌐 Web query:
IO.NET logs API: does /deployment/{id}/log/{containerId} return JSON (with has_more/next_cursor) or plain text lines? Is there a query flag to request JSON (e.g., format=json)?
💡 Result:
I can look that up — can you share the exact API documentation link or the service name (IO.NET, IONET, io.net, etc.)? I couldn't find a definitive reference for /deployment/{id}/log/{containerId} without the provider/docs.
I need to investigate the codebase to understand how these functions actually work and what format they handle. Let me search for the relevant code sections.
🏁 Script executed:
#!/bin/bash
# Find the GetContainerLogsRaw and StreamContainerLogs functions and their full implementations
rg -n -A 20 "func.*GetContainerLogsRaw" pkg/ionet/container.go
echo "---"
rg -n -A 20 "func.*StreamContainerLogs" pkg/ionet/container.goLength of output: 1519
🏁 Script executed:
#!/bin/bash
# Check the makeRequest function to understand what it returns
rg -n -B 5 -A 15 "func.*makeRequest" pkg/ionet/container.goLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look for any types related to GetLogsOptions, log responses, or streaming
fd -e go -t f | xargs rg -l "GetLogsOptions|has_more|next_cursor" pkg/ionet/Length of output: 184
🏁 Script executed:
#!/bin/bash
# Search for tests related to logs to understand expected format
fd -e go -t f -p "*test*" | xargs rg -l "GetContainerLogsRaw|StreamContainerLogs" Length of output: 361
I need to inspect the type definitions and tests to understand the actual log format. Let me continue investigating.
🏁 Script executed:
#!/bin/bash
# Find LogEntry type definition and response structures
rg -n -B 2 -A 10 "type LogEntry" pkg/ionet/types.go
echo "---"
rg -n -B 2 -A 10 "type.*Response" pkg/ionet/types.go
echo "---"
rg -n -B 2 -A 8 "type GetLogsOptions" pkg/ionet/types.goLength of output: 4966
🏁 Script executed:
#!/bin/bash
# Find makeRequest in client files
fd -e go -t f | xargs rg -n "func.*makeRequest" | head -20Length of output: 119
🏁 Script executed:
#!/bin/bash
# Check container_test.go for test cases showing actual usage
rg -n -B 3 -A 15 "TestGetContainerLogsRaw\|TestStreamContainerLogs" pkg/ionet/container_test.goLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for JSON unmarshaling or log parsing logic
rg -n "json.Unmarshal|LogEntry|log.*json" pkg/ionet/container.goLength of output: 261
I need to see the full StreamContainerLogs implementation and understand how it actually parses the response body.
🏁 Script executed:
#!/bin/bash
# Get full StreamContainerLogs implementation (from line 185 onwards)
rg -n -A 50 "^func.*StreamContainerLogs" pkg/ionet/container.go | head -60Length of output: 1618
🏁 Script executed:
#!/bin/bash
# Get context around line 162 (log parsing)
sed -n '150,175p' pkg/ionet/container.goLength of output: 657
🏁 Script executed:
#!/bin/bash
# Get context around line 323 (json unmarshal)
sed -n '310,340p' pkg/ionet/container.goLength of output: 578
🏁 Script executed:
#!/bin/bash
# Search for how the response is actually being handled - look for bufio or scanner usage
rg -n "bufio\|Scanner\|split\|json.Decoder" pkg/ionet/container.goLength of output: 0
Let me find the ContainerLogs type and the decodeWithFlexibleTimes function to understand the actual format expectations.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find ContainerLogs type
rg -n -B 2 -A 10 "type ContainerLogs" pkg/ionet/types.go
Length of output: 465
🏁 Script executed:
#!/bin/bash
# Find decodeWithFlexibleTimes function
rg -n -B 2 -A 20 "func.*decodeWithFlexibleTimes" pkg/ionet/Length of output: 1265
🏁 Script executed:
#!/bin/bash
# Search for buildLogEndpoint to understand what endpoint is being called
rg -n -B 2 -A 15 "func.*buildLogEndpoint" pkg/ionet/Length of output: 994
Endpoint returns JSON with pagination metadata—fix function format handling or clarify documentation.
The endpoint /deployment/{id}/log/{containerId} returns JSON with has_more and next_cursor fields (per ContainerLogs struct in types.go). StreamContainerLogs correctly parses this via decodeWithFlexibleTimes(), but GetContainerLogsRaw returns string(resp.Body) directly, which returns the raw JSON as a string—not plain text as the name suggests.
- GetContainerLogsRaw (lines 169–181): Returns raw JSON response body as string
- StreamContainerLogs (lines 185–244): Unmarshals JSON into
ContainerLogsstruct with pagination fields
Either update GetContainerLogsRaw to parse and return structured data (or plain text if that's the intent), or update documentation to clarify it returns JSON. The current inconsistency risks callers mishandling the response format.
🤖 Prompt for AI Agents
pkg/ionet/container.go lines 168-181: GetContainerLogsRaw currently returns the
raw JSON body as a string while the endpoint returns a paginated JSON object
(has_more, next_cursor) per ContainerLogs; change the function to return a
parsed ContainerLogs object (update signature to (types.ContainerLogs, error)),
decode the response using decodeWithFlexibleTimes(resp.Body, &containerLogs) or
json.Unmarshal as appropriate, and return the parsed struct (or an empty struct
with error on failure); ensure callers are updated or add a new function
name/alias if you must preserve the old signature.
| "one-api/pkg/ionet" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Go module path import.
The import path "one-api/pkg/ionet" may not match this repository's module path (likely github.com/QuantumNous/new-api). Update to the module path declared in go.mod.
🏁 Script executed:
#!/bin/bash
# Show module path and occurrences of "one-api/pkg/ionet"
rg -n '^module\s+' go.mod
rg -n --no-heading "one-api/pkg/ionet" -C2Length of output: 257
Update import path in basic_usage.go to match module declaration.
The import "one-api/pkg/ionet" at line 6 does not match the module path declared in go.mod. Update to github.com/QuantumNous/new-api/pkg/ionet.
"github.com/QuantumNous/new-api/pkg/ionet"
)
🤖 Prompt for AI Agents
In pkg/ionet/examples/basic_usage.go around lines 6 to 7, the import path
"one-api/pkg/ionet" does not match the module path in go.mod; replace that
import with "github.com/QuantumNous/new-api/pkg/ionet" so the package resolves
correctly, update any other occurrences in the file if present, and run go build
or go vet to confirm imports are fixed.
| const handleConfirm = () => { | ||
| if (isConfirmed) { | ||
| onConfirm(); | ||
| handleCancel(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Don’t close the dialog immediately on confirm.
This hides in‑flight status and errors. Let the parent close on success or await the promise and only close if it succeeds.
- const handleConfirm = () => {
- if (isConfirmed) {
- onConfirm();
- handleCancel();
- }
- };
+ const handleConfirm = async () => {
+ if (!isConfirmed) return;
+ try {
+ const result = await onConfirm?.();
+ // Parent controls visibility; only auto-close if explicitly truthy or not false
+ if (result !== false) handleCancel();
+ } catch (e) {
+ // Keep dialog open so errors can be shown
+ }
+ };📝 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.
| const handleConfirm = () => { | |
| if (isConfirmed) { | |
| onConfirm(); | |
| handleCancel(); | |
| } | |
| }; | |
| const handleConfirm = async () => { | |
| if (!isConfirmed) return; | |
| try { | |
| const result = await onConfirm?.(); | |
| // Parent controls visibility; only auto-close if explicitly truthy or not false | |
| if (result !== false) handleCancel(); | |
| } catch (e) { | |
| // Keep dialog open so errors can be shown | |
| } | |
| }; |
🤖 Prompt for AI Agents
In web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx
around lines 65–70, the handleConfirm callback currently closes the dialog
immediately after calling onConfirm, which hides in‑flight status and errors;
make handleConfirm asynchronous: set a local "pending" state before calling
onConfirm, await the result, only call handleCancel if the awaited call
succeeds, catch and surface errors (e.g., set a local error state or call an
onError prop) and clear pending on failure so the dialog stays open, and ensure
UI elements are disabled/indicate loading while pending.
| rules={[ | ||
| { required: true, message: t('请输入部署名称') }, | ||
| { | ||
| pattern: /^[a-zA-Z0-9-_\u4e00-\u9fa5]+$/, | ||
| message: t('部署名称只能包含字母、数字、横线、下划线和中文') | ||
| }, | ||
| ]} |
There was a problem hiding this comment.
Fix regex: current character class allows unintended punctuation
[a-zA-Z0-9-\u4e00-\u9fa5] makes a range 9-, admitting symbols like : ; < = > @ [ \ ] ^ _. Escape or move '-' to ends.
Apply:
- pattern: /^[a-zA-Z0-9-_\u4e00-\u9fa5]+$/,
+ pattern: /^[A-Za-z0-9_\u4e00-\u9fa5-]+$/,📝 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.
| rules={[ | |
| { required: true, message: t('请输入部署名称') }, | |
| { | |
| pattern: /^[a-zA-Z0-9-_\u4e00-\u9fa5]+$/, | |
| message: t('部署名称只能包含字母、数字、横线、下划线和中文') | |
| }, | |
| ]} | |
| rules={[ | |
| { required: true, message: t('请输入部署名称') }, | |
| { | |
| pattern: /^[A-Za-z0-9_\u4e00-\u9fa5-]+$/, | |
| message: t('部署名称只能包含字母、数字、横线、下划线和中文') | |
| }, | |
| ]} |
🤖 Prompt for AI Agents
In web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx
around lines 184-190 the character class [a-zA-Z0-9-_\u4e00-\u9fa5] creates an
unintended range because the hyphen sits between 0 and _, allowing extra
punctuation; fix by escaping the hyphen (e.g. \-) or moving it to the start or
end of the class (e.g. [-... ] or [...-]) so the class only permits letters,
digits, underscore, hyphen and Chinese characters, then keep the rest of the
pattern and validation message unchanged.
| // Auto refresh functionality | ||
| useEffect(() => { | ||
| if (autoRefresh && visible) { | ||
| autoRefreshRef.current = setInterval(() => { | ||
| fetchLogs(); | ||
| }, 5000); | ||
| } else { | ||
| if (autoRefreshRef.current) { | ||
| clearInterval(autoRefreshRef.current); | ||
| autoRefreshRef.current = null; | ||
| } | ||
| } | ||
|
|
||
| return () => { | ||
| if (autoRefreshRef.current) { | ||
| clearInterval(autoRefreshRef.current); | ||
| } | ||
| }; | ||
| }, [autoRefresh, visible, selectedContainerId, streamFilter, following]); | ||
|
|
There was a problem hiding this comment.
Fix stale interval closure in auto-refresh.
Add deployment.id to deps; otherwise it keeps polling the old deployment after switching.
- }, [autoRefresh, visible, selectedContainerId, streamFilter, following]);
+ }, [autoRefresh, visible, selectedContainerId, streamFilter, following, deployment?.id]);🤖 Prompt for AI Agents
In web/src/components/table/model-deployments/modals/ViewLogsModal.jsx around
lines 288 to 307, the auto-refresh effect is missing the deployment.id in its
dependency array which causes the interval to continue polling logs for the
previous deployment after switching; add the current deployment.id (or the
prop/state variable that holds it, e.g., deployment?.id or deploymentId) to the
dependency list and ensure the existing cleanup clears the interval when that id
changes so the effect re-creates a fresh interval for the new deployment.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (8)
web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
79-93: Implement temporary state to make Cancel functional.Changes are applied immediately via
onVisibleColumnsChange, so Cancel cannot revert user actions. The previous review comments recommended using temporary state that is only committed on Confirm.Consider this pattern:
+import React, { useMemo, useState, useEffect } from 'react'; const ColumnSelectorModal = ({ ... }) => { + const [tempVisibleColumns, setTempVisibleColumns] = useState(visibleColumns); + + // Sync temp state when modal opens and enforce required columns + useEffect(() => { + if (visible) { + const next = { ...visibleColumns }; + columnOptions.forEach(({ key, required }) => { + if (required) next[key] = true; + }); + setTempVisibleColumns(next); + } + }, [visible, visibleColumns, columnOptions]); const handleColumnVisibilityChange = (key, checked) => { const column = columnOptions.find((option) => option.key === key); if (column?.required) return; - onVisibleColumnsChange({ - ...visibleColumns, + setTempVisibleColumns({ + ...tempVisibleColumns, [key]: checked, }); }; - const handleConfirm = () => onCancel(); + const handleConfirm = () => { + onVisibleColumnsChange(tempVisibleColumns); + onCancel(); + }; + + const handleCancel = () => { + // Revert temp state on cancel + const next = { ...visibleColumns }; + columnOptions.forEach(({ key, required }) => { + if (required) next[key] = true; + }); + setTempVisibleColumns(next); + onCancel(); + };Update all handlers to use
tempVisibleColumnsandsetTempVisibleColumns. Based on learnings.web/src/components/table/channels/modals/OllamaModelModal.jsx (2)
751-754: Checkbox onChange receives event, not boolean — selection toggling will break.Use e.target.checked; current code passes the event object as checked.
Apply this diff:
- <Checkbox - checked={selectedModelIds.includes(model.id)} - onChange={(checked) => handleToggleModel(model.id, checked)} - /> + <Checkbox + checked={selectedModelIds.includes(model.id)} + onChange={(e) => + handleToggleModel(model.id, !!e?.target?.checked) + } + />
360-426: SSE may drop the last event on EOF — process trailing buffer and flush decoder.When the stream ends without a trailing newline, the final 'data:' line in buffer isn’t parsed. Flush the decoder and handle the leftover buffer once.
Apply this patch near the end of the read loop:
- while (true) { + while (true) { const { done, value } = await reader.read(); if (done) break; buffer += decoder.decode(value, { stream: true }); const lines = buffer.split('\n'); buffer = lines.pop() || ''; for (const line of lines) { ... } } - // 正常结束流 + // 处理可能未以换行结束的最后一条消息 + try { + const tail = buffer + decoder.decode(); + const lastLine = tail && tail.startsWith('data: ') ? tail : ''; + if (lastLine) { + const eventData = lastLine.substring(6); + if (eventData === '[DONE]') { + setPullLoading(false); + setPullProgress(null); + setEventSource(null); + } else { + const data = JSON.parse(eventData); + if (data.status) { + setPullProgress(data); + } else if (data.error) { + showError(data.error); + } else if (data.message) { + showSuccess(data.message); + setPullModelName(''); + } + } + } + } catch (e) { + console.error('Failed to parse trailing SSE data:', e); + } + // 正常结束流 setPullLoading(false); setPullProgress(null); setEventSource(null); await refreshModels();web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx (2)
226-234: Rename-only modal: fix CTA label and disable submit when no IDThis modal only supports renaming; showing “创建” misleads and yields an error when no ID. Force “更新” and guard the action when not editing.
- <Button + <Button theme='solid' type='primary' loading={loading} - onClick={() => formRef.current?.submitForm()} + disabled={!isEdit} + onClick={() => formRef.current?.submitForm()} > <Save size={16} className='mr-1' /> - {isEdit ? t('更新') : t('创建')} + {t('更新')} </Button>
187-195: Fix regex: hyphen in class creates unintended rangeHyphen between 0 and _ creates a 0-_ range, allowing punctuation. Move hyphen to the end (or escape).
- pattern: /^[a-zA-Z0-9-_\u4e00-\u9fa5]+$/, + pattern: /^[A-Za-z0-9_\u4e00-\u9fa5-]+$/,controller/deployment.go (3)
60-109: Unify instance_count; prefer replicas/containers over GPU qtymapIoNetDeployment sets instance_count from HardwareQuantity, which often equals GPU count, not container count. Use Replicas when present, else fallback.
func mapIoNetDeployment(d ionet.Deployment) map[string]interface{} { @@ - hardwareInfo := fmt.Sprintf("%s %s x%d", d.BrandName, d.HardwareName, d.HardwareQuantity) + hardwareInfo := fmt.Sprintf("%s %s x%d", d.BrandName, d.HardwareName, d.HardwareQuantity) + // Prefer replicas (containers); fallback to hardware quantity if replicas unavailable. + instanceCount := d.Replicas + if instanceCount <= 0 { + instanceCount = d.HardwareQuantity + } @@ - "instance_count": d.HardwareQuantity, + "instance_count": instanceCount,
235-246: Return a stable, human-readable name in GetDeploymentUsing details.ID for deployment_name confuses the UI. Prefer details.Name (or cluster name) with ID fallback; also expose container_name consistently.
- data := map[string]interface{}{ - "id": details.ID, - "deployment_name": details.ID, + name := strings.TrimSpace(details.Name) + if name == "" { + name = details.ID + } + data := map[string]interface{}{ + "id": details.ID, + "deployment_name": name, + "container_name": name,
374-385: ExtendDeployment mapping overwrites name and misreports countsName is set to deploymentID and HardwareQuantity to TotalGPUs, causing UI churn. Map from details consistently (use name; use containers/replicas for counts).
data := mapIoNetDeployment(ionet.Deployment{ ID: details.ID, Status: details.Status, - Name: deploymentID, + Name: details.Name, // fallback to details.ID if details.Name empty - CompletedPercent: float64(details.CompletedPercent), - HardwareQuantity: details.TotalGPUs, + CompletedPercent: float64(details.CompletedPercent), + // Prefer containers/replicas semantics for instance_count; pass containers here. + HardwareQuantity: details.TotalContainers, BrandName: details.BrandName, HardwareName: details.HardwareName, ComputeMinutesServed: details.ComputeMinutesServed, ComputeMinutesRemaining: details.ComputeMinutesRemaining, CreatedAt: details.CreatedAt, })
🧹 Nitpick comments (14)
web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
60-69: Align reset with app defaults: legacy columns should default to hidden.The hook defaults hide legacy columns (deployment_name, model_name, etc.), but this reset makes all columns visible. Consider either accepting a
defaultVisibleColumnsprop from the caller or importing the defaults from the hook.Example approach using a prop:
+// Caller would pass: defaultVisibleColumns={INITIAL_DEFAULTS_FROM_HOOK} const handleReset = () => { - const defaults = columnOptions.reduce((acc, { key }) => { - acc[key] = true; - return acc; - }, {}); - onVisibleColumnsChange({ - ...visibleColumns, - ...defaults, - }); + // Apply app defaults, ensuring required columns stay true + const reset = { ...defaultVisibleColumns }; + columnOptions.forEach(({ key, required }) => { + if (required) reset[key] = true; + }); + onVisibleColumnsChange(reset); };Based on learnings.
web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (3)
389-400: Consider extracting hardcoded badge styles.The provider badge uses inline hardcoded colors. Extracting these to a theme constant would improve maintainability.
Define a constant at the top of the file:
const PROVIDER_BADGE_STYLE = { borderColor: 'rgba(59, 130, 246, 0.4)', backgroundColor: 'rgba(59, 130, 246, 0.08)', color: '#2563eb', };Then use it in the render:
<div className="flex items-center gap-1.5 rounded-full border px-2 py-0.5 text-[10px] font-medium uppercase tracking-wide" - style={{ - borderColor: 'rgba(59, 130, 246, 0.4)', - backgroundColor: 'rgba(59, 130, 246, 0.08)', - color: '#2563eb', - }} + style={PROVIDER_BADGE_STYLE} >
536-568: Minor: Empty onClick handlers for disabled actions.Disabled actions use
onClick: () => {}which creates unnecessary function objects. Consider usingonClick: undefinedfor disabled states.Example:
case 'deploying': return { icon: <FaClock className="text-xs" />, text: t('部署中'), - onClick: () => {}, + onClick: undefined, type: 'secondary', theme: 'light', disabled: true, };
346-364: Consider grouping related parameters.The function accepts 15+ parameters. While acceptable for a configuration function, grouping related parameters could improve readability.
Example grouping:
export const getDeploymentsColumns = ({ t, COLUMN_KEYS, // Group action handlers actions: { startDeployment, restartDeployment, deleteDeployment, updateDeploymentName, }, // Group modal handlers modals: { setEditingDeployment, setShowEdit, onViewLogs, onExtendDuration, onViewDetails, onUpdateConfig, onSyncToChannel, }, // Group data data: { activePage, deployments, }, refresh, }) => {web/src/components/table/channels/modals/ModelSelectModal.jsx (3)
49-55: Harden name extraction; avoid “[object Object]” pollution.For non-string objects without model_name, you currently String(...) them, which yields “[object Object]” and leaks into lists. Return empty string instead; optionally consider common fields like id/name/model as fallbacks.
Apply this diff:
- const getModelName = (model) => { + const getModelName = (model) => { if (!model) return ''; if (typeof model === 'string') return model; - if (typeof model === 'object' && model.model_name) return model.model_name; - return String(model ?? ''); + if (typeof model === 'object') { + if (model.model_name) return model.model_name; + if (model.id) return String(model.id); + if (model.name) return String(model.name); + if (model.model) return String(model.model); + return ''; + } + return String(model ?? ''); }; - const normalizedModels = (models || []) + const normalizedModels = (models || []) .map(getModelName) - .filter((name) => typeof name === 'string' && name.trim() !== ''); + .filter((name) => typeof name === 'string' && name.trim() !== '');Also applies to: 64-68
202-213: Ensure panels default to expanded, matching the comment.You compute allActiveKeys but pass defaultActiveKey={[]}. Use the computed keys.
Apply this diff:
- <Collapse - key={`${categoryKeyPrefix}_${categoryEntries.length}`} - defaultActiveKey={[]} - > + <Collapse + key={`${categoryKeyPrefix}_${categoryEntries.length}`} + defaultActiveKey={allActiveKeys} + >
264-271: Optional: Guard Tabs when no items to avoid invalid activeKey.When both new and existing lists are empty, tabList is [], but activeKey remains 'new'. Consider hiding Tabs or defaulting activeTab to undefined when tabList.length === 0.
Also applies to: 141-159
web/src/components/table/channels/modals/EditChannelModal.jsx (2)
214-220: Harden window.open for security.Add noreferrer alongside noopener to prevent referrer leakage.
- window.open(targetUrl, '_blank', 'noopener'); + window.open(targetUrl, '_blank', 'noopener,noreferrer');
2961-2971: Avoid duplicate model fetches after Ollama pull.onModelsUpdate triggers fetchUpstreamModelList, and success branch in Ollama modal may also refetch. Ensure only one path refetches to reduce load.
Also applies to: 2972-2997
web/src/components/table/channels/modals/OllamaModelModal.jsx (1)
402-414: Remove redundant refetch to avoid double load.You call fetchModels(); and then await refreshModels() which also fetches. Keep just one.
- showSuccess(data.message); - setPullModelName(''); - setPullProgress(null); - setPullLoading(false); - setEventSource(null); - await fetchModels(); - if (onModelsUpdate) { - onModelsUpdate(); - } - await refreshModels(); + showSuccess(data.message); + setPullModelName(''); + setPullProgress(null); + setPullLoading(false); + setEventSource(null); + await refreshModels(); // handles fetch + onModelsUpdate onceweb/src/components/table/model-deployments/modals/EditDeploymentModal.jsx (1)
52-55: Remove unused model-loading logicmodels/loadingModels and loadModels effect are unused in this rename-only modal. Drop them to reduce complexity.
- const [models, setModels] = useState([]); - const [loadingModels, setLoadingModels] = useState(false); + // (removed) unused: models, loadingModels - const loadModels = async () => { - setLoadingModels(true); - try { - const res = await API.get('/api/models/?page_size=1000'); - if (res.data.success) { - const items = res.data.data.items || res.data.data || []; - const modelOptions = items.map((model) => ({ - label: `${model.model_name} (${model.vendor?.name || 'Unknown'})`, - value: model.model_name, - model_id: model.id, - })); - setModels(modelOptions); - } - } catch (error) { - console.error('Failed to load models:', error); - showError(t('加载模型列表失败')); - } - setLoadingModels(false); - }; + // (removed) unused: loadModels - useEffect(() => { - if (visible) { - loadModels(); - } - }, [visible]); + // (removed) unused: models loading on openAlso applies to: 85-105, 137-143
web/src/hooks/model-deployments/useDeploymentsData.jsx (1)
387-403: De-duplicate rename logic; reuse enhanced actionsupdateDeploymentName here duplicates the same operation found in web/src/hooks/model-deployments/useEnhancedDeploymentActions.jsx. Centralize to one place to avoid drift.
+import useEnhancedDeploymentActions from './useEnhancedDeploymentActions'; @@ - const updateDeploymentName = async (deploymentId, newName) => { - try { - const res = await API.put(`/api/deployments/${deploymentId}/name`, { name: newName }); - if (res.data.success) { - showSuccess(t('部署名称更新成功')); - await refresh(); - return true; - } else { - showError(res.data.message); - return false; - } - } catch (error) { - console.error(error); - showError(t('更新部署名称失败')); - return false; - } - }; + const { updateDeploymentName } = useEnhancedDeploymentActions();web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (2)
168-193: Debounce price estimation to reduce API churnEffect triggers API on every keystroke/change; add a small debounce.
+ const priceTimerRef = useRef(null); @@ - useEffect(() => { + useEffect(() => { if (!visible) { return; } if ( selectedHardwareId && selectedLocationIds.length > 0 && gpusPerContainer > 0 && durationHours > 0 && replicaCount > 0 ) { - calculatePrice(); + clearTimeout(priceTimerRef.current); + priceTimerRef.current = setTimeout(() => { + calculatePrice(); + }, 300); } else { setPriceEstimation(null); } }, [ selectedHardwareId, selectedLocationIds, gpusPerContainer, durationHours, replicaCount, priceCurrency, visible, ]);
803-809: Skip required rule when builtin image is selected (optional)image_url is disabled in builtin mode and set programmatically; you can drop the required rule when imageMode === 'builtin' to avoid unnecessary validation.
- <Form.Input + <Form.Input field="image_url" @@ - rules={[{ required: true, message: t('请输入镜像地址') }]} + rules={imageMode === 'builtin' ? [] : [{ required: true, message: t('请输入镜像地址') }]} disabled={imageMode === 'builtin'}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
controller/deployment.go(1 hunks)web/src/components/table/channels/modals/EditChannelModal.jsx(12 hunks)web/src/components/table/channels/modals/ModelSelectModal.jsx(1 hunks)web/src/components/table/channels/modals/OllamaModelModal.jsx(1 hunks)web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx(1 hunks)web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx(1 hunks)web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx(1 hunks)web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx(1 hunks)web/src/helpers/render.jsx(2 hunks)web/src/hooks/model-deployments/useDeploymentsData.jsx(1 hunks)web/src/pages/ModelDeployment/index.jsx(1 hunks)web/src/pages/Setting/Model/SettingModelDeployment.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/pages/Setting/Model/SettingModelDeployment.jsx
- web/src/pages/ModelDeployment/index.jsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
PR: QuantumNous/new-api#1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the applyModelMapping function transforms the models list by replacing original model names (mapping values) with display names (mapping keys). The database stores this transformed list containing mapped keys. On channel load, data.models contains these mapped display names, making the initialization filter if (data.models.includes(key)) correct.
Applied to files:
web/src/components/table/channels/modals/ModelSelectModal.jsx
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
PR: QuantumNous/new-api#1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the database stores mapped keys (display names) in the models field after applying model mapping transformations. When loading a channel, data.models contains the mapped keys, not the original model names. The filtering logic if (data.models.includes(key)) in the initialization is correct.
Applied to files:
web/src/components/table/channels/modals/ModelSelectModal.jsx
🧬 Code graph analysis (8)
web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx (2)
web/src/hooks/model-deployments/useDeploymentsData.jsx (2)
editingDeployment(40-42)refresh(235-237)web/src/helpers/utils.jsx (2)
showError(122-151)showSuccess(157-159)
web/src/components/table/channels/modals/OllamaModelModal.jsx (2)
web/src/helpers/utils.jsx (3)
showError(122-151)getUserIdFromLocalStorage(61-66)showSuccess(157-159)web/src/helpers/auth.jsx (1)
authHeader(24-33)
web/src/hooks/model-deployments/useDeploymentsData.jsx (4)
web/src/hooks/common/useTableCompactMode.js (2)
setCompactMode(34-40)useTableCompactMode(29-58)web/src/hooks/model-deployments/useEnhancedDeploymentActions.jsx (2)
deleteDeployment(127-143)updateDeploymentName(146-164)web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (2)
columns(365-706)newName(218-218)web/src/helpers/utils.jsx (2)
showError(122-151)showSuccess(157-159)
web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (4)
web/src/hooks/model-deployments/useDeploymentsData.jsx (4)
updateDeploymentName(387-403)COLUMN_KEYS(96-113)restartDeployment(279-292)startDeployment(264-277)web/src/hooks/model-deployments/useEnhancedDeploymentActions.jsx (1)
updateDeploymentName(146-164)web/src/components/table/model-deployments/modals/ViewDetailsModal.jsx (1)
handleCopyId(103-106)web/src/helpers/utils.jsx (2)
showSuccess(157-159)timestamp2string(192-218)
web/src/components/table/channels/modals/EditChannelModal.jsx (1)
web/src/components/table/channels/modals/OllamaModelModal.jsx (1)
OllamaModelModal(166-804)
web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
web/src/hooks/model-deployments/useDeploymentsData.jsx (1)
visibleColumns(121-145)
controller/deployment.go (5)
common/constants.go (3)
OptionMapRWMutex(38-38)OptionMap(37-37)StartTime(12-12)common/gin.go (3)
ApiErrorMsg(110-115)ApiError(103-108)ApiSuccess(117-123)pkg/ionet/types.go (10)
Client(8-12)Deployment(131-146)ListDeploymentsOptions(283-290)ContainerConfig(46-53)UpdateClusterNameRequest(257-259)UpdateDeploymentRequest(233-243)ExtendDurationRequest(246-248)DeploymentRequest(35-43)PriceEstimationRequest(187-197)GetLogsOptions(293-301)pkg/ionet/client.go (2)
NewClient(79-81)NewEnterpriseClient(74-76)common/page_info.go (1)
GetPageQuery(41-82)
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (2)
web/src/hooks/model-deployments/useDeploymentResources.js (1)
calculatePrice(164-205)web/src/helpers/utils.jsx (4)
a(259-259)showError(122-151)showSuccess(157-159)copy(72-95)
🔇 Additional comments (7)
web/src/helpers/render.jsx (2)
75-75: LGTM! Server icon import added correctly.The import follows the existing pattern and is properly placed with other lucide-react icon imports.
117-118: LGTM! Deployment icon case implemented correctly.The new 'deployment' case follows the established pattern and correctly returns the Server icon with appropriate styling. The icon choice is semantically appropriate for deployment management functionality.
web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
100-100: Checkbox onChange handlers correctly use Semi UI's Event signature.The handlers properly extract
checkedfrome.target.checked, addressing the previous review concern about Semi UI's onChange receiving an Event object rather than a boolean.Also applies to: 114-115
web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (3)
20-61: LGTM! Comprehensive imports.The imports are well-organized and cover all the necessary dependencies for the deployment columns UI.
365-708: Well-structured column definitions.The column definitions are comprehensive and well-organized:
- Clear separation of rendering logic for each column type
- Appropriate use of conditional rendering based on deployment status
- Good visual hierarchy with icons and tags
- Action dropdown provides context-appropriate operations
The implementation effectively handles the complex lifecycle states of deployments.
498-501: ****The
onUpdateConfigcallback is correctly designed to handle multiple action types, including deletion. When called with'delete'as the second parameter (line 500), it triggers the parent component's confirmation dialog, which then callsdeleteDeployment()upon confirmation (DeploymentsTable.jsx lines 88–100). ThedeleteDeploymentprop is not available in DeploymentsColumnDefs, and the confirmation flow resides in the parent component—this architecture is appropriate and working as intended.Likely an incorrect or invalid review comment.
controller/deployment.go (1)
321-349: No action required—routes are intentionally planned for future implementation.The commented-out routes in
router/api-router.go(lines 289–294) explicitly document that start/restart/stop and batch operations are planned but not yet implemented. The ionet client library does not provideStartDeployment()orRestartDeployment()methods, so the absence of handlers and routes is consistent and intentional, not an oversight. The code is in a stable, coherent state with clear documentation of planned future work.Likely an incorrect or invalid review comment.
| const isIonetLocked = isIonetChannel && isEdit; | ||
|
|
||
| const handleInputChange = (name, value) => { | ||
| if (isIonetChannel && isEdit && ['type', 'key', 'base_url'].includes(name)) { | ||
| return; | ||
| } | ||
| if (formApiRef.current) { |
There was a problem hiding this comment.
Ionet lock enforcement is inconsistent — disable all key inputs.
handleInputChange already ignores edits to ['type','key','base_url'] when Ionet-managed, but several UI key inputs remain editable, causing confusing UX. Disable them when isIonetLocked.
Apply these diffs:
// Batch textarea for keys
<Form.TextArea
field='key'
label={t('密钥')}
...
- onChange={(value) => handleInputChange('key', value)}
- disabled={isIonetLocked}
+ onChange={(value) => handleInputChange('key', value)}
+ disabled={isIonetLocked}
...
/>
// Vertex JSON manual input (type 41)
<Form.TextArea
field='key'
...
- onChange={(value) => handleInputChange('key', value)}
+ onChange={(value) => handleInputChange('key', value)}
+ disabled={isIonetLocked}
...
/>
// Generic single-line key input
<Form.Input
field='key'
...
- onChange={(value) => handleInputChange('key', value)}
+ onChange={(value) => handleInputChange('key', value)}
+ disabled={isIonetLocked}
...
/>Also ensure any additional key inputs introduced later follow the same pattern.
Also applies to: 1487-1509, 1656-1690, 1755-1811, 1841-1889
🤖 Prompt for AI Agents
In web/src/components/table/channels/modals/EditChannelModal.jsx around lines
384 to 390 (and also apply same changes to blocks at 1487-1509, 1656-1690,
1755-1811, 1841-1889): the Ionet lock is only enforced in handleInputChange for
['type','key','base_url'] which leaves several key input fields editable; update
the JSX for each affected input/control to set disabled={isIonetLocked} (or add
the equivalent prop/attribute for non-input controls) for all fields that
represent channel keys/credentials (type, key, base_url and any other related
key inputs), and ensure any future key inputs follow this pattern by
centralizing the isIonetLocked check or adding a shared helper to compute
disabled state and applying it to each relevant input/control.
| const parsePercentValue = (value) => { | ||
| if (value === null || value === undefined) return null; | ||
| if (typeof value === 'string') { | ||
| const parsed = parseFloat(value.replace(/[^0-9.+-]/g, '')); | ||
| return Number.isFinite(parsed) ? parsed : null; | ||
| } | ||
| if (typeof value === 'number') { | ||
| return Number.isFinite(value) ? value : null; | ||
| } | ||
| return null; | ||
| }; |
There was a problem hiding this comment.
Regex pattern could allow malformed numbers.
The regex /[^0-9.+-]/g permits multiple decimal points or signs (e.g., "1.2.3" or "+-5"). While the isFinite check catches invalid results, the parsing could be more robust.
Consider using a stricter pattern or validation:
const parsePercentValue = (value) => {
if (value === null || value === undefined) return null;
if (typeof value === 'string') {
- const parsed = parseFloat(value.replace(/[^0-9.+-]/g, ''));
+ // Match a valid number format: optional sign, digits, optional decimal and more digits
+ const match = value.match(/^[+-]?\d+\.?\d*$/);
+ if (!match) return null;
+ const parsed = parseFloat(match[0]);
return Number.isFinite(parsed) ? parsed : null;
}
if (typeof value === 'number') {
return Number.isFinite(value) ? value : null;
}
return null;
};📝 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.
| const parsePercentValue = (value) => { | |
| if (value === null || value === undefined) return null; | |
| if (typeof value === 'string') { | |
| const parsed = parseFloat(value.replace(/[^0-9.+-]/g, '')); | |
| return Number.isFinite(parsed) ? parsed : null; | |
| } | |
| if (typeof value === 'number') { | |
| return Number.isFinite(value) ? value : null; | |
| } | |
| return null; | |
| }; | |
| const parsePercentValue = (value) => { | |
| if (value === null || value === undefined) return null; | |
| if (typeof value === 'string') { | |
| // Match a valid number format: optional sign, digits, optional decimal and more digits | |
| const match = value.match(/^[+-]?\d+\.?\d*$/); | |
| if (!match) return null; | |
| const parsed = parseFloat(match[0]); | |
| return Number.isFinite(parsed) ? parsed : null; | |
| } | |
| if (typeof value === 'number') { | |
| return Number.isFinite(value) ? value : null; | |
| } | |
| return null; | |
| }; |
🤖 Prompt for AI Agents
In web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx around
lines 125 to 135, the parsePercentValue function strips all non-number
characters with /[^0-9.+-]/g which can leave malformed numbers like "1.2.3" or
"+-5"; change the implementation to first normalize the input (trim, remove
commas and trailing %), then use a stricter regex to extract a single optional
sign followed by digits with an optional single decimal fraction (e.g.
/^[+-]?\d+(\.\d+)?$/ or capturing with match), validate that the match exists,
and only then parseFloat on the matched substring so malformed inputs are
rejected and only well-formed numeric strings are returned.
| const handleCopyId = () => { | ||
| navigator.clipboard.writeText(record.id); | ||
| showSuccess(t('ID已复制到剪贴板')); | ||
| }; |
There was a problem hiding this comment.
Missing error handling for clipboard operation.
The clipboard API call is not awaited and lacks error handling. It can fail in insecure contexts or when permissions are denied, yet the success message displays immediately.
Apply this fix:
- const handleCopyId = () => {
- navigator.clipboard.writeText(record.id);
- showSuccess(t('ID已复制到剪贴板'));
+ const handleCopyId = async () => {
+ try {
+ await navigator.clipboard.writeText(record.id);
+ showSuccess(t('ID已复制到剪贴板'));
+ } catch (error) {
+ showError(t('复制失败'));
+ }
};📝 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.
| const handleCopyId = () => { | |
| navigator.clipboard.writeText(record.id); | |
| showSuccess(t('ID已复制到剪贴板')); | |
| }; | |
| const handleCopyId = async () => { | |
| try { | |
| await navigator.clipboard.writeText(record.id); | |
| showSuccess(t('ID已复制到剪贴板')); | |
| } catch (error) { | |
| showError(t('复制失败')); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx around
lines 242 to 245, the clipboard write operation is called without awaiting or
error handling so the success toast shows regardless of failure; change
handleCopyId to await navigator.clipboard.writeText(record.id) inside a
try/catch, call showSuccess only when the await resolves, and in the catch block
call showError (or show a fallback toast) with a helpful message; optionally
handle environments without navigator.clipboard by falling back to a legacy copy
method or showing an error.
| {record.compute_minutes_remaining !== undefined && percentRemaining !== null && ( | ||
| <div className="text-[10px]" style={{ color: theme.textColor }}> | ||
| {t('剩余')} {record.compute_minutes_remaining} {t('分钟')} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Redundant time display.
After showing formatted time ("约 2天 5小时"), displaying raw minutes ("剩余 3125 分钟") is redundant and clutters the UI. The formatted time is more user-friendly.
Consider removing the raw minutes display:
</span>
)}
</div>
- {record.compute_minutes_remaining !== undefined && percentRemaining !== null && (
- <div className="text-[10px]" style={{ color: theme.textColor }}>
- {t('剩余')} {record.compute_minutes_remaining} {t('分钟')}
- </div>
- )}
</div>
);
},📝 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.
| {record.compute_minutes_remaining !== undefined && percentRemaining !== null && ( | |
| <div className="text-[10px]" style={{ color: theme.textColor }}> | |
| {t('剩余')} {record.compute_minutes_remaining} {t('分钟')} | |
| </div> | |
| )} | |
| </span> | |
| )} | |
| </div> | |
| </div> | |
| ); | |
| }, |
🤖 Prompt for AI Agents
In web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx around
lines 452 to 456, remove the conditional JSX block that renders the raw minutes
display ("剩余 {record.compute_minutes_remaining} 分钟") because it's redundant with
the formatted time; delete the entire {record.compute_minutes_remaining !==
undefined && percentRemaining !== null && (...) } block (or replace it with an
optional tooltip showing raw minutes only on hover if you prefer) so only the
user-friendly formatted time remains in the UI.
| const columnOptions = useMemo( | ||
| () => [ | ||
| { key: columnKeys.container_name, label: t('容器名称'), required: true }, | ||
| { key: columnKeys.status, label: t('状态') }, | ||
| { key: columnKeys.time_remaining, label: t('剩余时间') }, | ||
| { key: columnKeys.hardware_info, label: t('硬件配置') }, | ||
| { key: columnKeys.created_at, label: t('创建时间') }, | ||
| { key: columnKeys.actions, label: t('操作'), required: true }, | ||
| ], | ||
| [columnKeys, t], | ||
| ); |
There was a problem hiding this comment.
Include all available columns from the hook, not just the default-visible ones.
The hook defines 11 columns but this modal only exposes 6. The missing legacy columns (deployment_name, model_name, instance_count, resource_config, updated_at) cannot be toggled by users, limiting the modal's utility.
Add the legacy columns to columnOptions:
const columnOptions = useMemo(
() => [
{ key: columnKeys.container_name, label: t('容器名称'), required: true },
{ key: columnKeys.status, label: t('状态') },
{ key: columnKeys.time_remaining, label: t('剩余时间') },
{ key: columnKeys.hardware_info, label: t('硬件配置') },
{ key: columnKeys.created_at, label: t('创建时间') },
+ { key: columnKeys.deployment_name, label: t('部署名称') },
+ { key: columnKeys.model_name, label: t('模型名称') },
+ { key: columnKeys.instance_count, label: t('实例数量') },
+ { key: columnKeys.resource_config, label: t('资源配置') },
+ { key: columnKeys.updated_at, label: t('更新时间') },
{ key: columnKeys.actions, label: t('操作'), required: true },
],
[columnKeys, t],
);Based on learnings.
🤖 Prompt for AI Agents
In web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx
around lines 31 to 41, the columnOptions array currently lists only 6
default-visible columns but omits the legacy columns defined by the hook
(deployment_name, model_name, instance_count, resource_config, updated_at),
preventing users from toggling them; update the columnOptions to include entries
for those five legacy columns (with appropriate key names from columnKeys and
translated labels via t(...)) and set required flags only where needed so all 11
hook-provided columns are available in the modal.
| // Batch operations | ||
| const batchDeleteDeployments = async () => { | ||
| if (selectedKeys.length === 0) return; | ||
|
|
||
| try { | ||
| const ids = selectedKeys.map(deployment => deployment.id); | ||
| const res = await API.post('/api/deployments/batch_delete', { ids }); | ||
| if (res.data.success) { | ||
| showSuccess(t('批量删除成功')); | ||
| setSelectedKeys([]); | ||
| await refresh(); | ||
| } else { | ||
| showError(res.data.message); | ||
| } | ||
| } catch (error) { | ||
| console.error(error); | ||
| showError(t('批量删除失败')); | ||
| } | ||
| }; | ||
|
|
||
| const batchStartDeployments = async () => { | ||
| if (selectedKeys.length === 0) return; | ||
|
|
||
| try { | ||
| const ids = selectedKeys.map(deployment => deployment.id); | ||
| const res = await API.post('/api/deployments/batch_start', { ids }); | ||
| if (res.data.success) { | ||
| showSuccess(t('批量启动成功')); | ||
| setSelectedKeys([]); | ||
| await refresh(); | ||
| } else { | ||
| showError(res.data.message); | ||
| } | ||
| } catch (error) { | ||
| console.error(error); | ||
| showError(t('批量启动失败')); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Batch API mismatch: endpoints not implemented
batchDeleteDeployments/batchStartDeployments hit /api/deployments/batch_(delete|start), but controller exposes no such routes. Implement or remove these for now to avoid 404/405 in UI.
🏁 Script executed:
#!/bin/bash
rg -nP "/api/deployments/batch_(delete|start)" -g "controller/**|router/**"Length of output: 148
🏁 Script executed:
#!/bin/bash
# Find controller/router files or directories
fd -t f -e js -e ts -e jsx -e tsx | grep -iE "(controller|router)" | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search entire codebase for batch_delete endpoint definition
rg -nP "batch_delete|batch_start" --type=js --type=ts --type=jsx --type=tsx -iLength of output: 27
🏁 Script executed:
#!/bin/bash
# Check common API route patterns in root and src directories
find . -maxdepth 3 -type f \( -name "*.js" -o -name "*.ts" -o -name "*.jsx" -o -name "*.tsx" \) | xargs rg -l "api/deployments" 2>/dev/null | head -10Length of output: 0
🏁 Script executed:
#!/bin/bash
# List directory structure to understand layout
ls -la
echo "---"
find . -maxdepth 2 -type d | head -20Length of output: 2548
🏁 Script executed:
#!/bin/bash
# Search for batch_delete and batch_start without type restrictions
rg -n "batch_delete|batch_start" -i --max-count=50Length of output: 727
🏁 Script executed:
#!/bin/bash
# Search for POST routes related to deployments
rg -n "post.*deployments|router\.post.*deployments" -i --max-count=50Length of output: 1770
🏁 Script executed:
#!/bin/bash
# Look for API endpoint definitions in common locations
rg -n "/api/deployments" --max-count=50Length of output: 5205
🏁 Script executed:
#!/bin/bash
# Check if controller functions exist
rg -n "BatchDeleteDeployments|BatchStartDeployments" Length of output: 207
🏁 Script executed:
#!/bin/bash
# Show context around the commented routes
sed -n '290,296p' router/api-router.goLength of output: 382
Remove batch operation calls or implement and enable the endpoints
The frontend calls POST /api/deployments/batch_delete and /api/deployments/batch_start (lines 411, 430), but these routes are commented out in router/api-router.go (lines 292–294). This will cause 404 errors when users attempt batch operations. Either uncomment and implement the controller functions, or remove these frontend calls entirely.
🤖 Prompt for AI Agents
In web/src/hooks/model-deployments/useDeploymentsData.jsx around lines 405–443
the batchDeleteDeployments and batchStartDeployments functions call POST
/api/deployments/batch_delete and /api/deployments/batch_start which are
commented out in router/api-router.go causing 404s; fix by either (A) enabling
backend support: uncomment the two routes in router/api-router.go, implement
corresponding controller handlers that accept { ids: number[] }, perform the
delete/start logic, return a JSON { success: boolean, message?: string } and
proper error handling/status codes, and add unit/integration tests; or (B)
remove the frontend batch endpoints: delete these batch* functions and any UI
batch-action wiring and instead iterate selectedKeys to call the existing
single-item delete/start endpoints (or adapt UI to not offer batch actions),
update tests and remove related i18n messages if unused.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
router/api-router.go (1)
260-298: Deployment routes are comprehensive, but past frontend integration issue remains.The deployment API surface is well-structured with proper validation and auth. The commented start/stop/restart routes (lines 292-295) are flagged in past reviews as a frontend-backend integration bug—this is still pending resolution.
controller/deployment.go (1)
109-158: Inconsistent instance_count and name mapping across endpoints (known issue).As flagged in past reviews:
mapIoNetDeploymentusesHardwareQuantityforinstance_count(line 149), butGetDeploymentusesTotalContainers(line 290), andExtendDeploymentoverwritesNamewithdeploymentID(line 426) instead of preserving the cluster name. This causes confusing UI deltas when data refreshes.Based on learnings.
🧹 Nitpick comments (1)
web/src/pages/Setting/Model/SettingModelDeployment.jsx (1)
187-197: Remove commented code.The commented Text component should be removed to keep the codebase clean.
Apply this diff:
- {/*<Text */} - {/* type="secondary" */} - {/* size="small"*/} - {/* style={{ */} - {/* display: 'block', */} - {/* marginBottom: '20px',*/} - {/* color: 'var(--semi-color-text-2)'*/} - {/* }}*/} - {/*>*/} - {/* {t('配置模型部署服务提供商的API密钥和启用状态')}*/} - {/*</Text>*/} -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
controller/deployment.go(1 hunks)router/api-router.go(2 hunks)web/src/pages/Setting/Model/SettingModelDeployment.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
router/api-router.go (3)
controller/channel.go (3)
OllamaPullModel(1647-1707)OllamaPullModelStream(1710-1789)OllamaDeleteModel(1792-1852)middleware/auth.go (1)
AdminAuth(163-167)controller/deployment.go (17)
GetAllDeployments(177-212)SearchDeployments(214-265)TestIoNetConnection(42-89)GetHardwareTypes(490-508)GetLocations(510-532)GetAvailableReplicas(534-568)GetPriceEstimation(570-589)CheckClusterNameAvailability(591-614)CreateDeployment(464-488)GetDeployment(267-313)GetDeploymentLogs(616-674)ListDeploymentContainers(676-729)GetContainerDetails(731-780)UpdateDeployment(370-398)UpdateDeploymentName(315-368)ExtendDeployment(400-437)DeleteDeployment(439-462)
controller/deployment.go (5)
common/constants.go (3)
OptionMapRWMutex(38-38)OptionMap(37-37)StartTime(12-12)common/gin.go (3)
ApiErrorMsg(110-115)ApiError(103-108)ApiSuccess(117-123)pkg/ionet/types.go (9)
Client(8-12)APIError(268-272)Deployment(131-146)ListDeploymentsOptions(283-290)ContainerConfig(46-53)UpdateClusterNameRequest(257-259)DeploymentRequest(35-43)PriceEstimationRequest(187-197)GetLogsOptions(293-301)pkg/ionet/client.go (2)
NewClient(79-81)NewEnterpriseClient(74-76)common/page_info.go (1)
GetPageQuery(41-82)
web/src/pages/Setting/Model/SettingModelDeployment.jsx (2)
web/src/components/settings/ModelDeploymentSetting.jsx (3)
useTranslation(27-27)loading(33-33)inputs(28-31)web/src/helpers/utils.jsx (4)
showError(122-151)showSuccess(157-159)compareObjects(298-315)showWarning(153-155)
🔇 Additional comments (17)
web/src/pages/Setting/Model/SettingModelDeployment.jsx (4)
49-108: LGTM! Backend API testing approach is secure.The API key validation now correctly uses the backend endpoint
/api/deployments/test-connection, which addresses the previous security concern about exposing keys to client-side scripts and CORS issues. The error handling is comprehensive, covering network failures and API errors with localized messages.
110-148: Solid form submission logic with proper validation.The pre-validation check ensures data integrity before submission, and the use of
structuredClonefor state updates is a good modern practice. The batch update approach withPromise.allefficiently handles multiple setting changes.
150-170: LGTM! Proper state synchronization.The effect correctly syncs props to local state with sensible defaults and uses
structuredCloneto prevent unintended reference sharing.
229-242: Good security practice with password mode.Using
mode="password"for the API key input and properly disabling it when the feature is not enabled demonstrates good UX and security practices.router/api-router.go (1)
152-154: LGTM! Ollama channel integration routes are properly wired.The three Ollama endpoints (pull, streaming pull, delete) are correctly mapped to their controller handlers with appropriate AdminAuth protection.
controller/deployment.go (12)
14-40: LGTM! Client initialization helpers are well-structured.The authentication and client factory functions use proper read locking, clear error messaging, and consistent (value, bool) return patterns.
42-89: LGTM! Connection testing handler properly validates API keys.The function correctly handles API errors, provides fallback logic for computing available hardware, and returns meaningful diagnostics.
91-107: LGTM! Parameter validators follow clean patterns.Both functions consistently validate and extract required path parameters with appropriate error responses.
117-126: Verify i18n requirements for hardcoded Chinese text.The time remaining formatting uses hardcoded Chinese strings ("小时", "分钟", "已完成"). Confirm whether this is intentional for a Chinese-only deployment or if these strings should be externalized for localization.
If localization is needed, consider:
- Moving strings to a translation/i18n service
- Accepting a locale parameter or reading from user preferences
- Providing fallback English strings
160-175: LGTM! Status aggregation logic is clean.The function properly initializes counts, normalizes status strings, and aggregates deployment statuses.
177-265: LGTM! List and search handlers are well-implemented.Both functions properly handle pagination and filtering. Note that
SearchDeploymentsperforms in-memory keyword filtering after fetching results, which is acceptable for typical deployment counts but may need optimization if volumes grow significantly.
267-398: LGTM! Deployment retrieval and update handlers are robust.All three functions properly validate inputs, handle errors, and provide clear responses. The name update flow includes proper availability checking before committing changes.
400-462: LGTM! Extend and delete handlers are functional.Both functions properly validate inputs and return appropriate responses. (The
Namefield inconsistency inExtendDeploymentat line 426 is noted in past reviews.)
464-532: LGTM! Deployment creation and resource endpoints are clean.All three functions properly delegate to the IoNet client, handle errors appropriately, and provide well-structured responses.
534-614: LGTM! Resource query endpoints properly validate inputs.All three functions correctly validate required parameters, provide sensible defaults where appropriate, and return clean responses.
616-674: LGTM! Log retrieval handler is well-designed.The function properly validates required parameters, caps limits at a reasonable threshold (1000), and gracefully handles optional time range parsing.
676-780: LGTM! Container inspection handlers are robust.Both functions properly validate inputs, handle nil responses gracefully, and consistently format event timestamps and status strings.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
web/src/components/table/model-deployments/DeploymentsTable.jsx (2)
182-186: Store keys instead of full row objects in selectedKeys state.The
onChangehandler storesselectedRows(full objects) insetSelectedKeys. This violates naming conventions and separation of concerns. While batch operations currently work by mapping.id, the state variable name implies it should hold keys (IDs) not objects.Apply this diff to align with naming and best practices:
rowSelection={{ - onChange: (selectedRowKeys, selectedRows) => { - setSelectedKeys(selectedRows); + onChange: (selectedRowKeys) => { + setSelectedKeys(selectedRowKeys); }, }}Then update batch operations in the hook to use IDs directly instead of extracting from row objects.
87-103: Confirm handler ignores 'destroy' operation, leaving it as a no-op.The
handleUpdateConfigfunction (lines 87-95) allows both'delete'and'destroy'operations to trigger the confirmation dialog, buthandleConfirmAction(lines 97-103) only executesdeleteDeploymentwhenconfirmOperation === 'delete'. Users confirming a 'destroy' action will see the dialog close with no effect.Apply this diff to handle both operations:
const handleConfirmAction = () => { - if (selectedDeployment && confirmOperation === 'delete') { + if (selectedDeployment && (confirmOperation === 'delete' || confirmOperation === 'destroy')) { deleteDeployment(selectedDeployment.id); } setShowConfirmDialog(false); setSelectedDeployment(null); };Alternatively, remove the 'destroy' branch from the code if only 'delete' is supported.
web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (3)
392-396: Remove redundant raw minutes display.After showing formatted time ("约 2天 5小时"), displaying raw minutes ("剩余 3125 分钟") is redundant and clutters the UI. The formatted time is more user-friendly.
Remove the redundant display:
</span> )} </div> - {record.compute_minutes_remaining !== undefined && percentRemaining !== null && ( - <div className="text-[10px]" style={{ color: theme.textColor }}> - {t('剩余')} {record.compute_minutes_remaining} {t('分钟')} - </div> - )} </div> ); },
117-127: Regex permits malformed numbers like "1.2.3" or "+-5".The regex
/[^0-9.+-]/gstrips non-numeric characters but allows multiple decimal points or signs. WhileNumber.isFinitecatches invalid results, stricter validation would prevent parsing ambiguity.Apply this diff for stricter parsing:
const parsePercentValue = (value) => { if (value === null || value === undefined) return null; if (typeof value === 'string') { - const parsed = parseFloat(value.replace(/[^0-9.+-]/g, '')); + const match = value.trim().match(/^[+-]?\d+(\.\d+)?$/); + if (!match) return null; + const parsed = parseFloat(match[0]); return Number.isFinite(parsed) ? parsed : null; } if (typeof value === 'number') { return Number.isFinite(value) ? value : null; } return null; };
209-212: Missing error handling for clipboard operation.
navigator.clipboard.writeTextis not awaited and lacks error handling. This can fail in insecure contexts or when permissions are denied, yet the success message displays immediately regardless of outcome.Apply this diff:
- const handleCopyId = () => { - navigator.clipboard.writeText(record.id); - showSuccess(t('ID已复制到剪贴板')); + const handleCopyId = async () => { + try { + await navigator.clipboard.writeText(record.id); + showSuccess(t('ID已复制到剪贴板')); + } catch (error) { + showError(t('复制失败')); + } };web/src/hooks/model-deployments/useDeploymentsData.jsx (2)
274-302: Frontend calls non-existent start/restart endpoints.
startDeployment(lines 274-287) andrestartDeployment(lines 289-302) callPOST /api/deployments/:id/startandPOST /api/deployments/:id/restart, but these routes are commented out inrouter/api-router.goand the corresponding controller functions do not exist. Users clicking start/restart buttons will receive 404 errors.Option A: Implement the backend endpoints by uncommenting routes in
router/api-router.goand addingStartDeploymentandRestartDeploymenthandlers incontroller/deployment.go.Option B: Remove these functions and hide the start/restart UI buttons until backend support is available.
416-452: Batch operations call non-existent endpoints.
batchDeleteDeployments(lines 416-433) andbatchStartDeployments(lines 435-452) callPOST /api/deployments/batch_deleteandPOST /api/deployments/batch_start. These routes are commented out inrouter/api-router.go(lines 292-294), causing 404 errors when users attempt batch actions.Option A: Uncomment and implement the batch endpoints in the backend with proper handlers.
Option B: Remove these functions and iterate
selectedKeysto call single-item endpoints, or disable batch actions in the UI.controller/deployment.go (1)
109-158: Inconsistent instance_count mapping causes UI confusion.
mapIoNetDeployment(line 149) setsinstance_countfromHardwareQuantity, butGetDeployment(line 290) usesTotalContainers. This inconsistency causes the same deployment to show different counts in list vs. detail views. Additionally,ExtendDeployment(line 426) overwritesNamewithdeploymentID, replacing the user-friendly container name with the ID.Fix 1: Align
instance_countto preferTotalContainerswith fallback:func mapIoNetDeployment(d ionet.Deployment) map[string]interface{} { + instanceCount := d.HardwareQuantity + if d.Replicas > 0 { + instanceCount = d.Replicas + } // ... - "instance_count": d.HardwareQuantity, + "instance_count": instanceCount,Fix 2: In
ExtendDeployment, preserve the original deployment name instead of using the ID:data := mapIoNetDeployment(ionet.Deployment{ ID: details.ID, Status: details.Status, - Name: deploymentID, + Name: details.ClusterName, // or fetch from original detailsBased on learnings.
🧹 Nitpick comments (1)
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (1)
666-717: Complex location filtering logic – consider adding inline comments.The
useEffect(lines 666-717) filtersselectedLocationIdsbased onavailableReplicasorlocationswith multiple conditional paths. While the logic appears correct, adding brief inline comments explaining the filtering strategy would improve maintainability.Example:
// When hardware changes, valid locations are limited to those with available replicas const validLocationIds = availableReplicas.length > 0 ? availableReplicas.map((item) => item.location_id) : locations.map((location) => location.id);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
controller/deployment.go(1 hunks)web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx(1 hunks)web/src/components/table/model-deployments/DeploymentsTable.jsx(1 hunks)web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx(1 hunks)web/src/hooks/model-deployments/useDeploymentsData.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (2)
web/src/hooks/model-deployments/useDeploymentResources.js (1)
calculatePrice(164-205)web/src/helpers/utils.jsx (4)
a(259-259)showError(122-151)showSuccess(157-159)copy(72-95)
web/src/hooks/model-deployments/useDeploymentsData.jsx (3)
web/src/hooks/common/useTableCompactMode.js (2)
setCompactMode(34-40)useTableCompactMode(29-58)web/src/hooks/model-deployments/useEnhancedDeploymentActions.jsx (2)
deleteDeployment(127-143)updateDeploymentName(146-164)web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (1)
columns(294-646)
web/src/components/table/model-deployments/DeploymentsTable.jsx (5)
web/src/components/table/model-deployments/index.jsx (2)
deploymentsData(33-33)deploymentsData(39-74)web/src/hooks/model-deployments/useDeploymentsData.jsx (17)
deleteDeployment(304-317)refresh(245-247)syncDeploymentToChannel(319-395)COLUMN_KEYS(96-114)startDeployment(274-287)restartDeployment(289-302)activePage(33-33)deployments(31-31)visibleColumns(130-155)compactMode(28-28)pageSize(34-34)deploymentCount(36-36)handlePageSizeChange(258-264)handlePageChange(250-255)handleRow(455-459)loading(32-32)searching(35-35)web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (2)
getDeploymentsColumns(276-649)getDeploymentsColumns(276-649)web/src/components/common/ui/CardTable.jsx (1)
CardTable(42-232)web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx (1)
ConfirmationDialog(40-281)
controller/deployment.go (5)
common/constants.go (3)
OptionMapRWMutex(38-38)OptionMap(37-37)StartTime(12-12)common/gin.go (3)
ApiErrorMsg(110-115)ApiError(103-108)ApiSuccess(117-123)pkg/ionet/types.go (11)
Client(8-12)APIError(268-272)Deployment(131-146)ListDeploymentsOptions(283-290)ContainerConfig(46-53)UpdateClusterNameRequest(257-259)UpdateDeploymentRequest(233-243)ExtendDurationRequest(246-248)DeploymentRequest(35-43)PriceEstimationRequest(187-197)GetLogsOptions(293-301)pkg/ionet/client.go (2)
NewClient(79-81)NewEnterpriseClient(74-76)common/page_info.go (1)
GetPageQuery(41-82)
web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (2)
web/src/helpers/utils.jsx (2)
showSuccess(157-159)timestamp2string(192-218)web/src/hooks/model-deployments/useDeploymentsData.jsx (3)
COLUMN_KEYS(96-114)restartDeployment(289-302)startDeployment(274-287)
🔇 Additional comments (1)
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (1)
534-604: LGTM: Robust submission handling with proper validation and error handling.The
handleSubmitfunction correctly:
- Validates and filters environment variables
- Ensures builtin mode has OLLAMA_API_KEY
- Cleans entrypoint/args arrays
- Constructs nested payload with container_config and registry_config
- Handles success/error responses appropriately
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/components/table/channels/ChannelsColumnDefs.jsx (1)
77-87: Consider using theparseMaybeJSONhelper for consistency.The JSON parsing logic here is correct and defensive. However,
OllamaModalModal.jsxuses aparseMaybeJSONhelper for similar parsing ofother_info. Consider extracting or reusing this helper for consistency across the codebase.+import { parseMaybeJSON } from '../../../helpers'; + const renderType = (type, record = {}, t) => { const channelInfo = record?.channel_info; // ... existing code ... let ionetMeta = null; if (record?.other_info) { - try { - const parsed = JSON.parse(record.other_info); + const parsed = parseMaybeJSON(record.other_info); if (parsed && typeof parsed === 'object' && parsed.source === 'ionet') { ionetMeta = parsed; } - } catch (error) { - // ignore invalid metadata - } }Note: Only apply this if
parseMaybeJSONis available in your helpers and returnsnull/undefinedfor invalid JSON.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/src/components/table/channels/ChannelsColumnDefs.jsx(3 hunks)web/src/components/table/channels/modals/EditChannelModal.jsx(13 hunks)web/src/components/table/channels/modals/OllamaModelModal.jsx(1 hunks)web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx
- web/src/components/table/channels/modals/OllamaModelModal.jsx
- web/src/components/table/channels/modals/EditChannelModal.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/components/table/channels/ChannelsColumnDefs.jsx (1)
web/src/components/table/channels/modals/OllamaModelModal.jsx (1)
parsed(95-95)
🔇 Additional comments (2)
web/src/components/table/channels/ChannelsColumnDefs.jsx (2)
46-47: LGTM: Function signature improved for flexibility.The signature change to accept the full
recordobject instead of justchannelInfoprovides better access to additional metadata (likeother_infofor IO.NET integration) while maintaining backward compatibility through optional chaining.
338-338: LGTM: Call site correctly updated.The call to
renderTypenow passes the fullrecordobject, which correctly matches the updated function signature and enables IO.NET metadata access.
| const handleNavigate = (event) => { | ||
| event?.stopPropagation?.(); | ||
| if (!ionetMeta?.deployment_id) { | ||
| return; | ||
| } | ||
| const targetUrl = `/console/deployment?deployment_id=${ionetMeta.deployment_id}`; | ||
| window.open(targetUrl, '_blank', 'noopener'); | ||
| }; |
There was a problem hiding this comment.
Improve UX when deployment_id is missing.
The IO.NET tag is always rendered as clickable (cursor-pointer class at line 121) and has an onClick handler, but the handler does nothing when deployment_id is missing (lines 95-97). This creates a confusing user experience where the cursor suggests interactivity but clicking has no effect.
Consider conditionally applying the cursor style and click handler:
<span>
<Tag
color='purple'
type='light'
- className='cursor-pointer'
- onClick={handleNavigate}
+ className={ionetMeta?.deployment_id ? 'cursor-pointer' : ''}
+ onClick={ionetMeta?.deployment_id ? handleNavigate : undefined}
>
IO.NET
</Tag>
</span>Also applies to: 102-129
🤖 Prompt for AI Agents
In web/src/components/table/channels/ChannelsColumnDefs.jsx around lines 93-100
(and similarly for the block covering 102-129), the IO.NET tag is always styled
as clickable and wired with an onClick that no-ops when ionetMeta.deployment_id
is missing; update the JSX so the cursor-pointer class and the onClick handler
are only applied when ionetMeta?.deployment_id is present (otherwise omit
onClick and use a non-interactive class or an explicit disabled/aria-disabled
state and optional tooltip/title explaining why it’s not clickable) so the
visual affordance matches actual behavior and accessibility is preserved.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx (1)
51-56: The past review concern remains unaddressed: don't close immediately on confirm.This still closes the dialog before
onConfirmcompletes, hiding in-flight status and errors from the user—especially problematic for destructive operations.Apply the previously suggested fix to make
handleConfirmasync and only close on success:- const handleConfirm = () => { - if (isConfirmed) { - onConfirm(); - handleCancel(); - } - }; + const handleConfirm = async () => { + if (!isConfirmed) return; + try { + const result = await onConfirm?.(); + // Parent controls visibility; only auto-close if explicitly truthy or not false + if (result !== false) handleCancel(); + } catch (e) { + // Keep dialog open so errors can be shown + } + };router/api-router.go (1)
293-295: Restore deployment lifecycle endpoints or remove the UI actions.
The deployment Start/Stop/Restart buttons in the frontend still POST to/api/deployments/{id}/startand/api/deployments/{id}/restart, but these routes remain commented out, so every click 404s. Either wire the handlers back up (and implement them) or remove/disable those UI actions until the backend supports them; otherwise the feature is broken in production.controller/deployment.go (2)
109-158: Address the instance_count inconsistency and hardcoded localization.Two issues persist:
instance_count mapping inconsistency (from past review): Line 149 uses
d.HardwareQuantitybutGetDeployment(Line 290) usesdetails.TotalContainers. This creates UI confusion when list and detail views show different counts.Hardcoded Chinese text (Lines 117-126): Time-remaining strings use Chinese characters (小时, 分钟, 已完成) without internationalization support.
Apply this diff to address the instance_count inconsistency:
- "instance_count": d.HardwareQuantity, + "instance_count": func() int { + if d.Replicas > 0 { return d.Replicas } + return d.HardwareQuantity + }(),For the Chinese text, extract strings to a localization layer or make the language configurable. Based on learnings.
401-438: Fix Name and HardwareQuantity mapping in ExtendDeployment.Two issues from past review persist:
Line 427:
Nameis set todeploymentID(the path parameter) instead of the deployment's actual name fromdetails. This causescontainer_namein the response to become the deployment ID.Line 429:
HardwareQuantityis set todetails.TotalGPUs, but semanticallyHardwareQuantityshould represent the count of hardware units, not total GPU count. This confuses the mapping logic.Apply this diff:
data := mapIoNetDeployment(ionet.Deployment{ ID: details.ID, Status: details.Status, - Name: deploymentID, + Name: details.ClusterName, // or details.Name if available CompletedPercent: float64(details.CompletedPercent), - HardwareQuantity: details.TotalGPUs, + HardwareQuantity: details.TotalContainers, // or derive from GPUsPerContainer + Replicas: details.TotalContainers, BrandName: details.BrandName, HardwareName: details.HardwareName, ComputeMinutesServed: details.ComputeMinutesServed, ComputeMinutesRemaining: details.ComputeMinutesRemaining, CreatedAt: details.CreatedAt, })Verify the correct field name for the cluster name in the DeploymentDetails struct. Based on learnings.
🧹 Nitpick comments (2)
web/src/components/table/model-deployments/modals/ExtendDurationModal.jsx (2)
280-295: Consider extracting nested ternaries to helper functions.The nested ternaries handling multiple naming conventions (snake_case vs. PascalCase) are hard to scan. A small helper would improve readability:
const safeGet = (obj, ...keys) => { for (const key of keys) { const val = obj?.[key]; if (typeof val === 'number') return val; } return undefined; }; const estimatedTotalCost = safeGet( priceData, 'estimated_cost', 'EstimatedCost' ) ?? safeGet(breakdown, 'total_cost', 'TotalCost'); const hourlyRate = safeGet(breakdown, 'hourly_rate', 'HourlyRate'); const computeCost = safeGet(breakdown, 'compute_cost', 'ComputeCost');
318-319: Consider extracting the disabled condition for clarity.The compound boolean expression could be a named constant for better readability:
+const isExtendDisabled = + !deployment?.id || detailsLoading || !durationHours || durationHours < 1; + okButtonProps={{ - disabled: - !deployment?.id || detailsLoading || !durationHours || durationHours < 1, + disabled: isExtendDisabled, }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
controller/channel.go(4 hunks)controller/deployment.go(1 hunks)relay/channel/ollama/relay-ollama.go(2 hunks)router/api-router.go(2 hunks)web/src/components/table/channels/ChannelsColumnDefs.jsx(5 hunks)web/src/components/table/channels/ChannelsTable.jsx(3 hunks)web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx(1 hunks)web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx(1 hunks)web/src/components/table/model-deployments/modals/ExtendDurationModal.jsx(1 hunks)web/src/hooks/channels/useChannelsData.jsx(3 hunks)web/src/i18n/locales/en.json(1 hunks)web/src/i18n/locales/fr.json(1 hunks)web/src/i18n/locales/ru.json(1 hunks)web/src/i18n/locales/zh.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/components/table/channels/ChannelsColumnDefs.jsx
- web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx
🧰 Additional context used
🧬 Code graph analysis (8)
web/src/hooks/channels/useChannelsData.jsx (2)
web/src/helpers/api.js (6)
res(224-224)res(225-225)res(268-268)res(269-269)API(29-37)API(29-37)web/src/helpers/utils.jsx (4)
showInfo(161-163)copy(72-95)showSuccess(157-159)showError(122-151)
router/api-router.go (3)
controller/channel.go (4)
OllamaPullModel(1647-1707)OllamaPullModelStream(1710-1789)OllamaDeleteModel(1792-1852)OllamaVersion(1855-1903)middleware/auth.go (1)
AdminAuth(163-167)controller/deployment.go (17)
GetAllDeployments(177-212)SearchDeployments(214-265)TestIoNetConnection(42-89)GetHardwareTypes(491-509)GetLocations(511-533)GetAvailableReplicas(535-569)GetPriceEstimation(571-590)CheckClusterNameAvailability(592-615)CreateDeployment(465-489)GetDeployment(267-314)GetDeploymentLogs(617-675)ListDeploymentContainers(677-730)GetContainerDetails(732-781)UpdateDeployment(371-399)UpdateDeploymentName(316-369)ExtendDeployment(401-438)DeleteDeployment(440-463)
controller/channel.go (5)
constant/channel.go (2)
ChannelTypeOllama(8-8)ChannelBaseURLs(60-117)relay/channel/ollama/relay-ollama.go (5)
FetchOllamaModels(289-326)PullOllamaModel(329-367)PullOllamaModelStream(370-444)DeleteOllamaModel(447-482)FetchOllamaVersion(484-530)model/channel.go (1)
GetChannelById(332-347)relay/channel/ollama/dto.go (1)
OllamaPullResponse(97-102)common/json.go (1)
Marshal(21-23)
web/src/components/table/channels/ChannelsTable.jsx (1)
web/src/hooks/channels/useChannelsData.jsx (1)
checkOllamaVersion(756-815)
relay/channel/ollama/relay-ollama.go (2)
relay/channel/ollama/dto.go (5)
OllamaModel(75-81)OllamaTagsResponse(71-73)OllamaPullRequest(92-95)OllamaPullResponse(97-102)OllamaDeleteRequest(104-106)common/json.go (2)
Unmarshal(9-11)Marshal(21-23)
web/src/components/table/model-deployments/modals/ExtendDurationModal.jsx (3)
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (2)
durationHours(150-150)calculatePrice(504-532)web/src/hooks/model-deployments/useDeploymentResources.js (1)
calculatePrice(164-205)web/src/helpers/utils.jsx (2)
showError(122-151)showSuccess(157-159)
controller/deployment.go (5)
common/constants.go (3)
OptionMapRWMutex(38-38)OptionMap(37-37)StartTime(12-12)common/gin.go (3)
ApiErrorMsg(110-115)ApiError(103-108)ApiSuccess(117-123)pkg/ionet/types.go (11)
Client(8-12)APIError(268-272)Deployment(131-146)ListDeploymentsOptions(283-290)ContainerConfig(46-53)UpdateClusterNameRequest(257-259)UpdateDeploymentRequest(233-243)ExtendDurationRequest(246-248)DeploymentRequest(35-43)PriceEstimationRequest(187-197)GetLogsOptions(293-301)pkg/ionet/client.go (2)
NewClient(79-81)NewEnterpriseClient(74-76)common/page_info.go (1)
GetPageQuery(41-82)
web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx (6)
web/src/components/table/model-deployments/modals/ExtendDurationModal.jsx (2)
Typography(42-42)handleCancel(260-266)web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (1)
Typography(45-45)web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx (2)
Typography(42-42)title(58-58)web/src/components/table/model-deployments/modals/ViewDetailsModal.jsx (1)
Typography(48-48)web/src/components/table/model-deployments/modals/ViewLogsModal.jsx (1)
Typography(49-49)web/src/components/table/model-deployments/modals/UpdateConfigModal.jsx (2)
Typography(49-49)handleCancel(145-152)
🔇 Additional comments (13)
web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx (1)
58-96: LGTM: Modal UI is well-structured.The modal properly wires
loadingto the OK button, disables it until validation passes, and provides inline error feedback. Autofocus on the input improves accessibility.web/src/components/table/model-deployments/modals/ExtendDurationModal.jsx (2)
101-203: LGTM: Robust price calculation with race-condition handling.The
requestIdpattern (lines 152-154, 175-177, 189-191, 199-201) correctly prevents stale price estimates from appearing if the user changes duration rapidly. Input validation and error handling are thorough.
408-425: LGTM: Quick presets are well-implemented.The preset buttons correctly sync both
durationHoursstate and the form value (lines 415-418), and the conditional display (hours vs. days) improves readability.web/src/components/table/channels/ChannelsTable.jsx (1)
60-60: LGTM! Clean prop threading for Ollama version checking.The
checkOllamaVersionfunction is properly wired through the component: destructured from props, passed to column definitions, and included in the memoization dependencies. This enables the Ollama version checking feature in the channels table UI.Also applies to: 86-86, 108-108
web/src/hooks/channels/useChannelsData.jsx (2)
37-37: LGTM! Necessary import for modal footer buttons.The
Buttoncomponent is correctly imported from Semi UI to support the custom modal footer in thecheckOllamaVersionfunction.
1173-1173: LGTM! Function properly exported from the hook.The
checkOllamaVersionfunction is correctly included in the hook's return object, making it available to components that use this hook.controller/deployment.go (7)
14-40: LGTM: Clean helper pattern for client initialization.The API key retrieval and client initialization helpers are well-structured with appropriate read-lock usage for the global OptionMap.
42-89: LGTM: Robust connection testing with proper error handling.The connection test handler correctly validates the API key by calling
GetMaxGPUsPerContainerand handles both generic and API-specific errors appropriately. The fallback logic fortotalAvailableis sensible.
316-369: LGTM: Proper name validation before update.The availability check (Lines 345-354) before attempting the name update is good practice, preventing errors and providing clear feedback when a name is already taken.
617-675: LGTM: Well-bounded log retrieval.The 1000-entry limit cap (Lines 643-645) is a good safeguard against excessive data retrieval. Time parsing and parameter handling are appropriate.
677-730: LGTM: Defensive nil handling.The nil check on Line 695 prevents potential panics if the API returns unexpected data.
91-107: LGTM: Remaining handlers follow consistent patterns.The parameter validators (Lines 91-107), status computation (Lines 160-175), and standard CRUD handlers (Lines 177-212, 371-399, 440-489, 732-781) are well-structured with appropriate error handling. The auxiliary data endpoints (Lines 491-615) correctly validate inputs and handle edge cases.
Note:
GetLocations(Line 512) usesgetIoClientrather thangetIoEnterpriseClient, which differs from other endpoints—verify this is intentional based on IoNet API requirements.Also applies to: 160-212, 371-399, 440-489, 491-615, 732-781
267-314: The review comment is incorrect—DeploymentDetail has no Name field.The
DeploymentDetailstruct inpkg/ionet/types.go(lines 69–87) does not includeNameorClusterNamefields. Settingdeployment_nametodetails.IDis the correct and only available option. The reference tomapIoNetDeploymentappears to compare different deployment types with different struct definitions.Likely an incorrect or invalid review comment.
| func SearchDeployments(c *gin.Context) { | ||
| pageInfo := common.GetPageQuery(c) | ||
| client, ok := getIoEnterpriseClient(c) | ||
| if !ok { | ||
| return | ||
| } | ||
|
|
||
| status := strings.ToLower(strings.TrimSpace(c.Query("status"))) | ||
| keyword := strings.TrimSpace(c.Query("keyword")) | ||
|
|
||
| dl, err := client.ListDeployments(&ionet.ListDeploymentsOptions{ | ||
| Status: status, | ||
| Page: pageInfo.GetPage(), | ||
| PageSize: pageInfo.GetPageSize(), | ||
| SortBy: "created_at", | ||
| SortOrder: "desc", | ||
| }) | ||
| if err != nil { | ||
| common.ApiError(c, err) | ||
| return | ||
| } | ||
|
|
||
| filtered := make([]ionet.Deployment, 0, len(dl.Deployments)) | ||
| if keyword == "" { | ||
| filtered = dl.Deployments | ||
| } else { | ||
| kw := strings.ToLower(keyword) | ||
| for _, d := range dl.Deployments { | ||
| if strings.Contains(strings.ToLower(d.Name), kw) { | ||
| filtered = append(filtered, d) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| items := make([]map[string]interface{}, 0, len(filtered)) | ||
| for _, d := range filtered { | ||
| items = append(items, mapIoNetDeployment(d)) | ||
| } | ||
|
|
||
| total := dl.Total | ||
| if keyword != "" { | ||
| total = len(filtered) | ||
| } | ||
|
|
||
| data := gin.H{ | ||
| "page": pageInfo.GetPage(), | ||
| "page_size": pageInfo.GetPageSize(), | ||
| "total": total, | ||
| "items": items, | ||
| } | ||
| common.ApiSuccess(c, data) | ||
| } |
There was a problem hiding this comment.
Client-side filtering breaks pagination semantics.
Lines 253-256 adjust total to the filtered count, but pagination parameters (page, page_size) operate on the pre-filtered dataset from the API. This creates incorrect pagination metadata:
- If the API returns 100 deployments (page 1) but only 10 match
keyword,totalbecomes 10 whilepageandpage_sizestill reflect the original pagination. - Subsequent page requests will use the wrong offsets.
Consider one of these solutions:
- Remove total adjustment and document that
totalreflects unfiltered count when keyword is present. - Fetch all deployments and paginate the filtered results client-side (inefficient).
- Add server-side keyword search support to the IoNet client if the API supports it.
Proposed quick fix (option 1):
- total := dl.Total
- if keyword != "" {
- total = len(filtered)
- }
+ // Note: total reflects unfiltered count; actual filtered count is len(items)
+ total := dl.Total
data := gin.H{
"page": pageInfo.GetPage(),
"page_size": pageInfo.GetPageSize(),
"total": total,
"items": items,
+ "filtered_count": len(filtered),
}📝 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 SearchDeployments(c *gin.Context) { | |
| pageInfo := common.GetPageQuery(c) | |
| client, ok := getIoEnterpriseClient(c) | |
| if !ok { | |
| return | |
| } | |
| status := strings.ToLower(strings.TrimSpace(c.Query("status"))) | |
| keyword := strings.TrimSpace(c.Query("keyword")) | |
| dl, err := client.ListDeployments(&ionet.ListDeploymentsOptions{ | |
| Status: status, | |
| Page: pageInfo.GetPage(), | |
| PageSize: pageInfo.GetPageSize(), | |
| SortBy: "created_at", | |
| SortOrder: "desc", | |
| }) | |
| if err != nil { | |
| common.ApiError(c, err) | |
| return | |
| } | |
| filtered := make([]ionet.Deployment, 0, len(dl.Deployments)) | |
| if keyword == "" { | |
| filtered = dl.Deployments | |
| } else { | |
| kw := strings.ToLower(keyword) | |
| for _, d := range dl.Deployments { | |
| if strings.Contains(strings.ToLower(d.Name), kw) { | |
| filtered = append(filtered, d) | |
| } | |
| } | |
| } | |
| items := make([]map[string]interface{}, 0, len(filtered)) | |
| for _, d := range filtered { | |
| items = append(items, mapIoNetDeployment(d)) | |
| } | |
| total := dl.Total | |
| if keyword != "" { | |
| total = len(filtered) | |
| } | |
| data := gin.H{ | |
| "page": pageInfo.GetPage(), | |
| "page_size": pageInfo.GetPageSize(), | |
| "total": total, | |
| "items": items, | |
| } | |
| common.ApiSuccess(c, data) | |
| } | |
| func SearchDeployments(c *gin.Context) { | |
| pageInfo := common.GetPageQuery(c) | |
| client, ok := getIoEnterpriseClient(c) | |
| if !ok { | |
| return | |
| } | |
| status := strings.ToLower(strings.TrimSpace(c.Query("status"))) | |
| keyword := strings.TrimSpace(c.Query("keyword")) | |
| dl, err := client.ListDeployments(&ionet.ListDeploymentsOptions{ | |
| Status: status, | |
| Page: pageInfo.GetPage(), | |
| PageSize: pageInfo.GetPageSize(), | |
| SortBy: "created_at", | |
| SortOrder: "desc", | |
| }) | |
| if err != nil { | |
| common.ApiError(c, err) | |
| return | |
| } | |
| filtered := make([]ionet.Deployment, 0, len(dl.Deployments)) | |
| if keyword == "" { | |
| filtered = dl.Deployments | |
| } else { | |
| kw := strings.ToLower(keyword) | |
| for _, d := range dl.Deployments { | |
| if strings.Contains(strings.ToLower(d.Name), kw) { | |
| filtered = append(filtered, d) | |
| } | |
| } | |
| } | |
| items := make([]map[string]interface{}, 0, len(filtered)) | |
| for _, d := range filtered { | |
| items = append(items, mapIoNetDeployment(d)) | |
| } | |
| // Note: total reflects unfiltered count; actual filtered count is len(items) | |
| total := dl.Total | |
| data := gin.H{ | |
| "page": pageInfo.GetPage(), | |
| "page_size": pageInfo.GetPageSize(), | |
| "total": total, | |
| "items": items, | |
| "filtered_count": len(filtered), | |
| } | |
| common.ApiSuccess(c, data) | |
| } |
🤖 Prompt for AI Agents
In controller/deployment.go around lines 214-265 the handler applies client-side
keyword filtering but overrides total with the filtered count which breaks
pagination semantics; instead keep total as the API-returned dl.Total regardless
of keyword (remove or disable the block that sets total = len(filtered) when
keyword != ""), and add a short comment that total reflects the unfiltered
dataset when keyword filtering is done client-side.
| client := &http.Client{} | ||
| request, err := http.NewRequest("GET", url, nil) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("创建请求失败: %v", err) | ||
| } | ||
|
|
||
| // Ollama 通常不需要 Bearer token，但为了兼容性保留 | ||
| if apiKey != "" { | ||
| request.Header.Set("Authorization", "Bearer "+apiKey) | ||
| } | ||
|
|
||
| response, err := client.Do(request) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("请求失败: %v", err) |
There was a problem hiding this comment.
Add a timeout when fetching Ollama tags
FetchOllamaModels runs inside request handlers (e.g., FetchUpstreamModels / FetchModels). With the zero-timeout http.Client, a dead or slow Ollama endpoint will block the gin worker forever, taking the whole API path down. Please give this client a bounded timeout (similar to the other Ollama helpers) so we fail fast instead of hanging the request thread.
- client := &http.Client{}
+ client := &http.Client{
+ Timeout: 30 * time.Second,
+ }📝 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.
| client := &http.Client{} | |
| request, err := http.NewRequest("GET", url, nil) | |
| if err != nil { | |
| return nil, fmt.Errorf("创建请求失败: %v", err) | |
| } | |
| // Ollama 通常不需要 Bearer token，但为了兼容性保留 | |
| if apiKey != "" { | |
| request.Header.Set("Authorization", "Bearer "+apiKey) | |
| } | |
| response, err := client.Do(request) | |
| if err != nil { | |
| return nil, fmt.Errorf("请求失败: %v", err) | |
| client := &http.Client{ | |
| Timeout: 30 * time.Second, | |
| } | |
| request, err := http.NewRequest("GET", url, nil) | |
| if err != nil { | |
| return nil, fmt.Errorf("创建请求失败: %v", err) | |
| } | |
| // Ollama 通常不需要 Bearer token，但为了兼容性保留 | |
| if apiKey != "" { | |
| request.Header.Set("Authorization", "Bearer "+apiKey) | |
| } | |
| response, err := client.Do(request) | |
| if err != nil { | |
| return nil, fmt.Errorf("请求失败: %v", err) |
🤖 Prompt for AI Agents
In relay/channel/ollama/relay-ollama.go around lines 292 to 305, the http.Client
is created with no timeout which can hang gin workers; change the client to use
a bounded timeout (match the timeout used in other Ollama helpers, e.g. set
http.Client{Timeout: X * time.Second}) so requests fail fast; ensure you import
time if missing and choose a sensible timeout value consistent with other
helpers, apply the same timeout for this client before calling client.Do.
| client := &http.Client{} | ||
| request, err := http.NewRequest("DELETE", url, strings.NewReader(string(requestBody))) | ||
| if err != nil { | ||
| return fmt.Errorf("创建请求失败: %v", err) | ||
| } | ||
|
|
||
| request.Header.Set("Content-Type", "application/json") | ||
| if apiKey != "" { | ||
| request.Header.Set("Authorization", "Bearer "+apiKey) | ||
| } | ||
|
|
||
| response, err := client.Do(request) | ||
| if err != nil { | ||
| return fmt.Errorf("请求失败: %v", err) | ||
| } | ||
| defer response.Body.Close() | ||
|
|
||
| if response.StatusCode != http.StatusOK { |
There was a problem hiding this comment.
Apply the same timeout discipline to deletes
DeleteOllamaModel also instantiates an http.Client without a timeout, so a stalled /api/delete call will hang our handler indefinitely. Please mirror the bounded timeout used elsewhere (e.g., FetchOllamaVersion) to keep the API responsive.
- client := &http.Client{}
+ client := &http.Client{
+ Timeout: 30 * time.Second,
+ }📝 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.
| client := &http.Client{} | |
| request, err := http.NewRequest("DELETE", url, strings.NewReader(string(requestBody))) | |
| if err != nil { | |
| return fmt.Errorf("创建请求失败: %v", err) | |
| } | |
| request.Header.Set("Content-Type", "application/json") | |
| if apiKey != "" { | |
| request.Header.Set("Authorization", "Bearer "+apiKey) | |
| } | |
| response, err := client.Do(request) | |
| if err != nil { | |
| return fmt.Errorf("请求失败: %v", err) | |
| } | |
| defer response.Body.Close() | |
| if response.StatusCode != http.StatusOK { | |
| client := &http.Client{ | |
| Timeout: 30 * time.Second, | |
| } | |
| request, err := http.NewRequest("DELETE", url, strings.NewReader(string(requestBody))) | |
| if err != nil { | |
| return fmt.Errorf("创建请求失败: %v", err) | |
| } | |
| request.Header.Set("Content-Type", "application/json") | |
| if apiKey != "" { | |
| request.Header.Set("Authorization", "Bearer "+apiKey) | |
| } | |
| response, err := client.Do(request) | |
| if err != nil { | |
| return fmt.Errorf("请求失败: %v", err) | |
| } | |
| defer response.Body.Close() | |
| if response.StatusCode != http.StatusOK { |
🤖 Prompt for AI Agents
In relay/channel/ollama/relay-ollama.go around lines 459-476, the code creates
an http.Client with no timeout causing DeleteOllamaModel to potentially hang;
change the client to use the same bounded timeout used elsewhere (e.g., the
timeout used by FetchOllamaVersion) by instantiating the client with a Timeout
field (or by using context.WithTimeout for the request) so the delete call is
bounded; ensure you reference the same timeout constant/value used in other
functions, propagate/cancel the context if used, and keep the existing error
handling and response.Body.Close.
| if (response.data.success) { | ||
| showSuccess(t('容器时长延长成功')); | ||
| onSuccess?.(response.data.data); | ||
| handleCancel(); | ||
| } |
There was a problem hiding this comment.
Consider separating extension success from callback errors.
If onSuccess throws, the catch block will report "延长时长失败" even though the extension API call succeeded. The user's deployment was extended, but the UI callback failed.
Consider handling callback errors separately:
if (response.data.success) {
showSuccess(t('容器时长延长成功'));
- onSuccess?.(response.data.data);
- handleCancel();
+ handleCancel();
+ try {
+ onSuccess?.(response.data.data);
+ } catch (callbackError) {
+ console.error('onSuccess callback error:', callbackError);
+ }
}🤖 Prompt for AI Agents
In web/src/components/table/model-deployments/modals/ExtendDurationModal.jsx
around lines 244-248, the code treats any error thrown by onSuccess as if the
API extension failed; change the flow so the API success path always shows the
success toast and calls handleCancel, then invoke onSuccess separately inside
its own try/catch (await if it can return a promise), and handle/log any errors
from onSuccess without showing the "延长时长失败" error to the user (e.g.,
console.error or a non-blocking warning) so callback failures do not override
the actual API success.
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 (1)
web/src/i18n/locales/fr.json (1)
1511-1519: Placeholder mismatches found in French translations—fix requiredVerification revealed 2 placeholder mismatches between English and French translations:
- "确定要删除所选的 {{count}} 个令牌吗？_one" — English has
{{count}}, French is missing it- "确定要删除所选的 {{count}} 个模型吗？_one" — English has
{{count}}, French is missing itThese occur in deletion confirmation dialogs and will cause runtime errors or display literal tokens in the French UI. Add the missing
{{count}}placeholder to the French translations.Note: The specific line ranges cited (1511–1519, 2241–2248, 2339–2355) do not contain these mismatches; they are clean.
♻️ Duplicate comments (8)
web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (3)
117-127: Stricter numeric parsing for percent values/[^0-9.+-]/g allows malformed strings (e.g., 1.2.3). Use a validated match before parseFloat.
- if (typeof value === 'string') { - const parsed = parseFloat(value.replace(/[^0-9.+-]/g, '')); - return Number.isFinite(parsed) ? parsed : null; - } + if (typeof value === 'string') { + const s = value.trim(); + const m = s.match(/^[+-]?\d+(\.\d+)?$/); + if (!m) return null; + const parsed = parseFloat(m[0]); + return Number.isFinite(parsed) ? parsed : null; + }
418-422: Redundant raw minutes lineThe “剩余 X 分钟” line duplicates the human-friendly time above; consider removing to declutter.
- {showProgress && showRemainingMeta && ( - <div className="text-[10px]" style={{ color: theme.textColor }}> - {t('剩余')} {record.compute_minutes_remaining} {t('分钟')} - </div> - )} + {/* raw minutes hidden to reduce noise */}
207-213: Clipboard write lacks await/fallbackUse the shared copy() helper to handle insecure contexts and failures; only toast on success.
- const handleCopyId = () => { - navigator.clipboard.writeText(record.id); - showSuccess(t('ID已复制到剪贴板')); - }; + const handleCopyId = async () => { + const ok = await copy(record.id); + if (ok) showSuccess(t('ID已复制到剪贴板')); + else showError(t('复制失败，请手动选择文本复制')); + };Add import at top:
-import { - timestamp2string, - showSuccess, - showError, -} from '../../../helpers'; +import { + timestamp2string, + showSuccess, + showError, + copy, +} from '../../../helpers';controller/deployment.go (3)
424-435: Name/instance_count mapping drift after ExtendDeploymentYou map Name to deploymentID and instance_count elsewhere differs from detail responses. Use the returned details to keep fields consistent.
data := mapIoNetDeployment(ionet.Deployment{ ID: details.ID, Status: details.Status, - Name: deploymentID, + Name: details.Name, - CompletedPercent: float64(details.CompletedPercent), - HardwareQuantity: details.TotalGPUs, + CompletedPercent: float64(details.CompletedPercent), + HardwareQuantity: details.TotalGPUs, BrandName: details.BrandName, HardwareName: details.HardwareName, ComputeMinutesServed: details.ComputeMinutesServed, ComputeMinutesRemaining: details.ComputeMinutesRemaining, CreatedAt: details.CreatedAt, })Additionally, consider aligning mapIoNetDeployment to set instance_count from a container/replica field when available rather than HardwareQuantity to match GetDeployment. [Based on learnings]
214-265: Client-side filtering breaks pagination semanticstotal is set to len(filtered) when keyword != "", but page/page_size are from upstream, causing inconsistent metadata. Keep total = dl.Total and optionally add filtered_count.
- total := dl.Total - if keyword != "" { - total = len(filtered) - } + // total reflects unfiltered count from upstream to keep pagination consistent with page/page_size + total := dl.Total @@ - "total": total, + "total": total, + "filtered_count": len(filtered),
177-212: Missing lifecycle endpoints for start/restart and batch operationsThe controller does not expose StartDeployment/RestartDeployment or batch delete/start handlers, yet the UI calls them. Either implement these routes or remove the UI actions.
#!/bin/bash # Show any deployment lifecycle endpoints currently registered rg -nP 'func\\s+(Start|Restart|Batch(Delete|Start))Deployment' controller --glob='**/*.go' rg -nP '"deployments"\\s*' router --glob='**/*.go' -n -A4 -B2I can draft minimal handlers that validate path/body and return ApiSuccess with a queued-operation message if you decide to implement them now.
Also applies to: 214-265
web/src/hooks/model-deployments/useDeploymentsData.jsx (2)
273-302: Frontend calls to start/restart endpoints without backend handlersstartDeployment and restartDeployment hit POST /api/deployments/:id/(start|restart). No corresponding controller handlers are present in controller/deployment.go, so users will see 404/405. Implement backend endpoints or hide these actions for now.
Run to verify:
#!/bin/bash rg -nP "func\\s+(Start|Restart)Deployment\\(" controller rg -nP "/deployments/\\:id/(start|restart)" -n router --glob='**/*.go'
415-433: Batch delete endpoint likely missingbatchDeleteDeployments posts to /api/deployments/batch_delete, but no batch handler exists in controller/deployment.go (and was previously commented in router). Implement the batch backend or remove this call/UI until ready.
#!/bin/bash rg -nP "batch_delete" -S rg -nP "Batch.*Deployments|batch.*deployments" controller router --glob='**/*.go'
🧹 Nitpick comments (11)
web/src/hooks/model-deployments/useDeploymentsData.jsx (1)
83-90: Avoid mutating API arrays in placesetDeploymentFormat mutates deployments[i].key directly. Use a mapped copy to prevent accidental shared-state bugs.
-const setDeploymentFormat = (deployments) => { - for (let i = 0; i < deployments.length; i++) { - deployments[i].key = deployments[i].id; - } - setDeployments(deployments); -}; +const setDeploymentFormat = (list) => { + setDeployments((Array.isArray(list) ? list : []).map(d => ({ ...d, key: d.id }))); +};pkg/ionet/client.go (2)
110-121: Add Accept header and guard empty endpoint joinsInclude Accept: application/json and normalize URL join to avoid double slashes or missing slashes.
headers := map[string]string{ "X-API-KEY": c.APIKey, "Content-Type": "application/json", + "Accept": "application/json", } @@ - URL: c.BaseURL + endpoint, + URL: strings.TrimRight(c.BaseURL, "/") + "/" + strings.TrimLeft(endpoint, "/"),Add import:
-import ( +import ( "bytes" "encoding/json" "fmt" + "strings"
33-71: Expose context for cancellations/timeoutsConsider a Context-aware Do method (or per-request context) so callers can cancel long-running requests beyond DefaultTimeout. Keep current method for compatibility.
web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (2)
55-115: Avoid calling t() at module scope; make status config dynamicSTATUS_TAG_CONFIG invokes t() during module evaluation, risking stale translations and double t() in renderStatus. Generate labels inside a factory that receives t.
-const STATUS_TAG_CONFIG = { - running: { color: 'green', label: t('运行中'), icon: <FaPlay size={12} className='text-green-600' /> }, - ... -}; +const getStatusTagConfig = (t) => ({ + running: { color: 'green', label: '运行中', icon: <FaPlay size={12} className='text-green-600' /> }, + ... +}); @@ -const renderStatus = (status, t) => { +const renderStatus = (status, t) => { const normalizedStatus = normalizeStatus(status); - const config = STATUS_TAG_CONFIG[normalizedStatus] || DEFAULT_STATUS_CONFIG; + const config = (getStatusTagConfig(t)[normalizedStatus]) || DEFAULT_STATUS_CONFIG; const statusText = typeof status === 'string' ? status : ''; - const labelText = config.label ? t(config.label) : statusText || t('未知状态'); + const labelText = config.label ? t(config.label) : statusText || t('未知状态'); ... };And update the single call site accordingly (no other changes needed).
Also applies to: 189-205
276-297: restartDeployment prop is unusedYou pass restartDeployment but never render a restart action. Remove the prop or add the action when backend is available.
Also applies to: 566-597
web/src/i18n/locales/zh.json (1)
297-300: Secret scanner false-positive: Basic auth in example"例如: socks5://user:pass@host:port" triggers basic-auth credential scanners. Mask the example to reduce noise.
- "例如: socks5://user:pass@host:port": "例如: socks5://user:pass@host:port", + "例如: socks5://user:*****@host:port": "例如: socks5://user:*****@host:port",web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (3)
171-196: Price estimation recomputes on every small change; debounce for UXRapid field edits can spam /price-estimation requests. Debounce calculatePrice by ~300ms or batch with a single effect using a timeout.
-import React, { useState, useEffect, useMemo, useRef } from 'react'; +import React, { useState, useEffect, useMemo, useRef } from 'react'; +import { debounce } from 'lodash-es'; @@ - useEffect(() => { + const debouncedCalc = useMemo(() => debounce(() => calculatePrice(), 300), [selectedHardwareId, selectedLocationIds, gpusPerContainer, durationHours, replicaCount, priceCurrency]); + useEffect(() => { if (!visible) { return; } @@ - calculatePrice(); + debouncedCalc(); } else { setPriceEstimation(null); } - }, [ + }, [ selectedHardwareId, selectedLocationIds, gpusPerContainer, durationHours, replicaCount, priceCurrency, visible, ]); + useEffect(() => () => debouncedCalc.cancel(), [debouncedCalc]);If lodash is undesirable, implement a small in-file debounce with setTimeout.
Also applies to: 504-532
1173-1210: Env var editors: avoid empty rows leaking to payloadYou filter empties, good. Consider trimming keys/values and preventing duplicate keys in UI to reduce backend validation load.
Also applies to: 1224-1248, 1254-1333
1150-1166: Traffic port validation messageAdd a validation rule to surface “端口号必须在1-65535之间” when out of range.
<Form.InputNumber field="traffic_port" ... - min={1} - max={65535} + min={1} + max={65535} + rules={[{ validator: (_, v) => (v >= 1 && v <= 65535) ? '' : t('端口号必须在1-65535之间') }]}web/src/i18n/locales/en.json (1)
940-951: Placeholder style is mixed (${...} vs {{...}}) — confirm runtime handlingMost strings use {{var}} but this block uses ${version}. Ensure the renderer interpolates ${...} too; otherwise UI may show raw tokens.
If unsupported, switch to {{version}} for consistency.
web/src/i18n/locales/fr.json (1)
729-734: Duplicate-key guard for fr.json as wellNo duplicate was flagged here, but given the file size, add a duplicate-key check to avoid silent overrides (same risk as en.json).
Use the duplicate detector script shared in the en.json comment; it checks both en/fr.
Also applies to: 2181-2199
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
controller/deployment.go(1 hunks)pkg/ionet/client.go(1 hunks)web/src/components/table/model-deployments/DeploymentsActions.jsx(1 hunks)web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx(1 hunks)web/src/components/table/model-deployments/DeploymentsFilters.jsx(1 hunks)web/src/components/table/model-deployments/index.jsx(1 hunks)web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx(1 hunks)web/src/hooks/model-deployments/useDeploymentsData.jsx(1 hunks)web/src/i18n/locales/en.json(83 hunks)web/src/i18n/locales/fr.json(85 hunks)web/src/i18n/locales/zh.json(84 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/components/table/model-deployments/DeploymentsFilters.jsx
- web/src/components/table/model-deployments/DeploymentsActions.jsx
🧰 Additional context used
🧬 Code graph analysis (6)
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (2)
web/src/hooks/model-deployments/useDeploymentResources.js (1)
calculatePrice(164-205)web/src/helpers/utils.jsx (4)
a(259-259)showError(122-151)showSuccess(157-159)copy(72-95)
web/src/components/table/model-deployments/index.jsx (6)
web/src/components/table/model-deployments/DeploymentsTable.jsx (2)
deploymentsData(37-60)DeploymentsTable(36-245)web/src/hooks/model-deployments/useDeploymentsData.jsx (16)
useDeploymentsData(26-507)useDeploymentsData(26-507)refresh(245-247)editingDeployment(40-42)showEdit(39-39)closeEdit(76-81)showColumnSelector(158-158)visibleColumns(130-155)COLUMN_KEYS(96-114)selectedKeys(45-45)batchDeleteDeployments(416-433)compactMode(28-28)formInitValues(57-60)searchDeployments(208-242)loading(32-32)searching(35-35)web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
ColumnSelectorModal(23-125)web/src/components/common/ui/CardPro.jsx (1)
CardPro(44-174)web/src/components/table/model-deployments/DeploymentsActions.jsx (1)
DeploymentsActions(24-107)web/src/components/table/model-deployments/DeploymentsFilters.jsx (1)
DeploymentsFilters(24-127)
pkg/ionet/client.go (1)
pkg/ionet/types.go (5)
Client(8-12)HTTPRequest(20-25)HTTPResponse(28-32)HTTPClient(15-17)APIError(268-272)
web/src/hooks/model-deployments/useDeploymentsData.jsx (4)
web/src/hooks/common/useTableCompactMode.js (2)
setCompactMode(34-40)useTableCompactMode(29-58)web/src/hooks/model-deployments/useEnhancedDeploymentActions.jsx (2)
deleteDeployment(127-143)updateDeploymentName(146-164)web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (1)
columns(294-669)web/src/helpers/utils.jsx (2)
showError(122-151)showSuccess(157-159)
controller/deployment.go (5)
common/constants.go (3)
OptionMapRWMutex(38-38)OptionMap(37-37)StartTime(12-12)common/gin.go (3)
ApiErrorMsg(110-115)ApiError(103-108)ApiSuccess(117-123)pkg/ionet/types.go (11)
Client(8-12)APIError(268-272)Deployment(131-146)ListDeploymentsOptions(283-290)ContainerConfig(46-53)UpdateClusterNameRequest(257-259)UpdateDeploymentRequest(233-243)ExtendDurationRequest(246-248)DeploymentRequest(35-43)PriceEstimationRequest(187-197)GetLogsOptions(293-301)pkg/ionet/client.go (2)
NewClient(79-81)NewEnterpriseClient(74-76)common/page_info.go (1)
GetPageQuery(41-82)
web/src/components/table/model-deployments/DeploymentsColumnDefs.jsx (2)
web/src/helpers/utils.jsx (2)
showSuccess(157-159)timestamp2string(192-218)web/src/hooks/model-deployments/useDeploymentsData.jsx (2)
COLUMN_KEYS(96-114)startDeployment(274-287)
🪛 Biome (2.1.2)
web/src/i18n/locales/zh.json
[error] 1299-1300: The key 失败 was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
web/src/i18n/locales/en.json
[error] 920-920: The key 失败 was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🪛 Checkov (3.2.334)
web/src/i18n/locales/zh.json
[medium] 299-300: Basic Auth Credentials
(CKV_SECRET_4)
web/src/i18n/locales/en.json
[medium] 302-303: Basic Auth Credentials
(CKV_SECRET_4)
web/src/i18n/locales/fr.json
[medium] 305-306: Basic Auth Credentials
(CKV_SECRET_4)
🔇 Additional comments (3)
web/src/components/table/model-deployments/index.jsx (1)
101-139: Page composition looks solidModal wiring, pagination via createCardProPagination, and data plumbing from useDeploymentsData are coherent. No issues spotted.
web/src/i18n/locales/fr.json (2)
3-9: Now I need to verify whether the corresponding_oneand_otherentries exist for these pluralized strings.Based on my verification, I can now generate the rewritten review comment:
Remove or complete
_manypluralization suffixes in French locale; French uses only one/other for standard messagesFrench uses plural categories "one" (for 1 and 0) and "other" (for all other numbers). While CLDR v38 added a
_manycategory for French, it is specifically for compact number handling only. For regular message translations infr.json, only_oneand_othersuffixes are appropriate.Issues:
- Multiple
_manyentries have empty translations (lines 17, 443, 446, 855, etc.), preventing proper localization- Non-empty
_manyentries (lines 4, 8, 439, 842, 858, 884, 907, 1631, 1634) are unnecessary for standard pluralizationAction: Remove all
_manyentries or, if they must exist for compact number support, ensure all have translations. Verify corresponding_oneand_otherforms exist for each pluralized key.
16-18: No issues remain—all empty French translations have been filled from EnglishThe 331 empty string values in fr.json have been filled with English translations, and no empty strings remain in the file. The specific line ranges cited in the review comment (16–18, 75, 90–95, 178–180) now contain proper translations instead of empty values. The fallback suppression issue is fully resolved.
| // Search deployments | ||
| const searchDeployments = async (searchTerms) => { | ||
| setSearching(true); | ||
| try { | ||
| const { searchKeyword, searchStatus } = searchTerms; | ||
| const params = new URLSearchParams({ | ||
| p: '1', | ||
| page_size: pageSize.toString(), | ||
| }); | ||
|
|
||
| if (searchKeyword?.trim()) { | ||
| params.append('keyword', searchKeyword.trim()); | ||
| } | ||
| if (searchStatus && searchStatus !== 'all') { | ||
| params.append('status', searchStatus); | ||
| } | ||
|
|
||
| const res = await API.get(`/api/deployments/search?${params}`); | ||
| const { success, message, data } = res.data; | ||
|
|
||
| if (success) { | ||
| const items = extractItems(data); | ||
| setActivePage(1); | ||
| setDeploymentCount(data.total || items.length); | ||
| setDeploymentFormat(items); | ||
| } else { | ||
| showError(message); | ||
| setDeployments([]); | ||
| } | ||
| } catch (error) { | ||
| console.error('Search error:', error); | ||
| showError(t('搜索失败')); | ||
| setDeployments([]); | ||
| } | ||
| setSearching(false); | ||
| }; |
There was a problem hiding this comment.
Paging loses search filters; results revert to unfiltered list
After searchDeployments loads page 1, subsequent handlePageChange/handlePageSizeChange call loadDeployments (no keyword), dropping filters. Persist last search params and page through /api/deployments/search while active.
Apply minimal state and routing fix:
@@
- const searchDeployments = async (searchTerms) => {
+ const [lastSearch, setLastSearch] = useState(null);
+ const searchDeployments = async (searchTerms) => {
setSearching(true);
try {
@@
const res = await API.get(`/api/deployments/search?${params}`);
@@
if (success) {
+ setLastSearch({ keyword: searchKeyword?.trim() || '', status: searchStatus || 'all' });
const items = extractItems(data);
setActivePage(1);
@@
setSearching(false);
};
@@
- const handlePageChange = (page) => {
+ const handlePageChange = async (page) => {
setActivePage(page);
- if (!searching) {
- loadDeployments(page, pageSize);
- }
+ if (lastSearch && (lastSearch.keyword || (lastSearch.status && lastSearch.status !== 'all'))) {
+ const params = new URLSearchParams({ p: String(page), page_size: String(pageSize) });
+ if (lastSearch.keyword) params.append('keyword', lastSearch.keyword);
+ if (lastSearch.status && lastSearch.status !== 'all') params.append('status', lastSearch.status);
+ try {
+ const res = await API.get(`/api/deployments/search?${params}`);
+ const { success, data } = res.data;
+ if (success) {
+ setDeploymentCount(data.total || extractItems(data).length);
+ setDeploymentFormat(extractItems(data));
+ }
+ } catch (e) { /* no-op; toasts already elsewhere */ }
+ return;
+ }
+ loadDeployments(page, pageSize);
};
@@
- const handlePageSizeChange = (size) => {
+ const handlePageSizeChange = async (size) => {
setPageSize(size);
setActivePage(1);
- if (!searching) {
- loadDeployments(1, size);
- }
+ if (lastSearch && (lastSearch.keyword || (lastSearch.status && lastSearch.status !== 'all'))) {
+ await handlePageChange(1);
+ return;
+ }
+ loadDeployments(1, size);
};Also applies to: 249-265
🤖 Prompt for AI Agents
In web/src/hooks/model-deployments/useDeploymentsData.jsx around lines 207 to
242 (and similarly 249-265), the search flow loads page 1 but subsequent
pagination/size changes call loadDeployments (unfiltered) and drop the search
filters; persist the last search params (keyword, status, page, page_size) in
state (e.g., searchParams state) and in the router query, update
searchDeployments to save those params and call the /api/deployments/search
endpoint with them, and modify handlePageChange and handlePageSizeChange to
detect active searchParams and call searchDeployments with merged page/page_size
instead of loadDeployments so paging keeps filters; also ensure
setActivePage/setPageSize and URL query sync are updated when search is
executed.
| "已完成": "Completed", | ||
| "失败": "Failed", | ||
| "部署请求中": "Deployment requested", | ||
| "终止请求中": "Termination requested", | ||
| "已销毁": "Destroyed", | ||
| "运行命令 (Command)": "Run command (Command)", |
There was a problem hiding this comment.
🧩 Analysis chain
Duplicate key "失败" (Failed) — earlier value is shadowed
JSON object has "失败": "Failed" defined earlier and again here. Only the last wins; the earlier is ignored. Remove one to avoid subtle UI bugs.
Apply this minimal diff to drop the later duplicate:
"运行中": "Running",
"已完成": "Completed",
- "失败": "Failed",
"部署请求中": "Deployment requested",
"终止请求中": "Termination requested",
"已销毁": "Destroyed",Optionally, run this repo-safe check to list any duplicate keys across locale files:
🏁 Script executed:
#!/bin/bash
# Detect duplicate keys inside JSON objects (preserves order) for both locales.
python - <<'PY'
import sys, json
from collections import OrderedDict
def dup_keys(path):
with open(path, 'rb') as f:
txt=f.read().decode('utf-8')
dups=[]
stack=[]
key_counts={}
i=0
# naive tokenizer to record keys preserving duplicates
import re
for m in re.finditer(r'"((?:\\.|[^"\\])*)"\s*:', txt):
key=m.group(1)
key_counts[key]=key_counts.get(key,0)+1
for k,c in key_counts.items():
if c>1: dups.append(k)
if dups:
print(f"[DUP] {path}: {len(dups)} duplicate keys")
for k in sorted(dups):
print(" -", k)
else:
print(f"[OK] {path}: no duplicate keys")
for p in ["web/src/i18n/locales/en.json","web/src/i18n/locales/fr.json"]:
try: dup_keys(p)
except FileNotFoundError: print(f"[MISS] {p}")
PYLength of output: 267
Now let me verify the exact locations of the duplicate "失败" keys in the en.json file:
Let me search for this file first:
Actually, let me try a different approach - search the codebase for both occurrences:
---
🏁 Script executed:
```shell
#!/bin/bash
# Find all occurrences of "失败" in en.json with line numbers and context
grep -n "失败" web/src/i18n/locales/en.json
Length of output: 6356
🏁 Script executed:
#!/bin/bash
# Find the en.json file
find . -name "en.json" -type f 2>/dev/null | head -20Length of output: 30
🏁 Script executed:
#!/bin/bash
# Use ripgrep to find all "失败" key definitions with line numbers
rg '"失败"\s*:' -nLength of output: 360
Let me verify the exact context around lines 2181-2186 to confirm the review snippet is accurate:
---
🏁 Script executed:
```shell
#!/bin/bash
# Extract lines 2181-2186 from en.json to verify the context
sed -n '2181,2186p' web/src/i18n/locales/en.json
Length of output: 189
Duplicate key "失败" (Failed) — earlier value is shadowed
JSON object has "失败": "Failed" defined at line 724 and again at line 2182. Only the last wins; the earlier is ignored. Remove one to avoid subtle UI bugs.
Apply this minimal diff to drop the later duplicate:
"已完成": "Completed",
- "失败": "Failed",
"部署请求中": "Deployment requested",
"终止请求中": "Termination requested",
"已销毁": "Destroyed",Optionally, run this repo-safe check to list any duplicate keys across locale files:
#!/bin/bash
python - <<'PY'
import sys, json, re
def dup_keys(path):
with open(path, 'rb') as f:
txt = f.read().decode('utf-8')
key_counts = {}
for m in re.finditer(r'"((?:\\.|[^"\\])*)"\s*:', txt):
key = m.group(1)
key_counts[key] = key_counts.get(key, 0) + 1
dups = [k for k, c in key_counts.items() if c > 1]
if dups:
print(f"[DUP] {path}: {len(dups)} duplicate keys")
for k in sorted(dups):
print(" -", k)
else:
print(f"[OK] {path}: no duplicate keys")
for p in ["web/src/i18n/locales/en.json", "web/src/i18n/locales/fr.json"]:
try:
dup_keys(p)
except FileNotFoundError:
print(f"[MISS] {p}")
PY🤖 Prompt for AI Agents
In web/src/i18n/locales/en.json around lines 2181-2186 there is a duplicated key
"失败" (Failed) which also appears earlier at line ~724; remove the later
occurrence (the entry at lines 2181-2186 containing "失败": "Failed") so the key
is only defined once; optionally run the provided duplicate-key check across
locale files to confirm no other duplicates remain.
| "来源于 IO.NET 部署": "来源于 IO.NET 部署", | ||
| "某些配置更改可能需要几分钟才能生效。": "某些配置更改可能需要几分钟才能生效。", |
There was a problem hiding this comment.
🧩 Analysis chain
Duplicate key(s) in translation object
Key “失败” appears multiple times; only the last one is used, earlier ones are ignored. Deduplicate to avoid surprises.
I can generate a normalized zh.json with duplicates merged if desired.
Also applies to: 2169-2170
🏁 Script executed:
#!/bin/bash
# List duplicate keys with counts (rough)
jq -r '.translation | keys[]' web/src/i18n/locales/zh.json | sort | uniq -d | head -50Length of output: 3
I need to verify the actual structure and find duplicates correctly.
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the structure and find duplicates
echo "=== JSON structure check ==="
jq 'keys' web/src/i18n/locales/zh.json | head -20
echo -e "\n=== Looking for duplicate keys (correct approach) ==="
jq 'keys[]' web/src/i18n/locales/zh.json | sort | uniq -d | head -20
echo -e "\n=== Count of '失败' key occurrences ==="
grep -o '"失败"' web/src/i18n/locales/zh.json | wc -l
echo -e "\n=== Lines around 1299-1300 ==="
sed -n '1295,1305p' web/src/i18n/locales/zh.json
echo -e "\n=== Lines around 2169-2170 ==="
sed -n '2165,2175p' web/src/i18n/locales/zh.json
Length of output: 740
I need to check if "失败" actually appears as a duplicate key in the raw file.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find all lines with "失败" as a key
echo "=== All lines containing '失败' key ==="
grep -n '"失败"' web/src/i18n/locales/zh.json
echo -e "\n=== Count of unique lines with '失败' as key (key:value pairs) ==="
grep -n '"失败"' web/src/i18n/locales/zh.json | wc -l
echo -e "\n=== Verify jq sees no duplicates ==="
jq '.translation | keys[] | select(. == "失败")' web/src/i18n/locales/zh.json | wc -l
Length of output: 186
Correct the duplicate key locations; "失败" appears at lines 718 and 2169, not 1299-1300
The duplicate key "失败" is confirmed, but the cited line 1299-1300 is incorrect. The actual duplicates appear at lines 718 and 2169. When JSON objects have duplicate keys, only the last value is retained; the earlier one at line 718 is silently ignored. Remove one of these duplicate entries to avoid confusion.
🧰 Tools
🪛 Biome (2.1.2)
[error] 1299-1300: The key 失败 was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🤖 Prompt for AI Agents
In web/src/i18n/locales/zh.json around lines 718 and 2169, there are duplicate
JSON keys "失败" (the reviewer comment incorrectly cited lines 1299-1300); JSON
will keep only the last occurrence, silently dropping the earlier one — remove
or consolidate one of the duplicate "失败" entries so each key is unique, keeping
the intended translation (or merge/rename if both meanings differ), and re-run a
JSON lint to ensure the file remains valid.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/table/channels/modals/ModelSelectModal.jsx (1)
206-213: Unused variable and comment/code mismatch.The variable
allActiveKeysis generated but never used, and the comment states "确保都展开" (ensure all are expanded), butdefaultActiveKey={[]}on line 213 means panels start collapsed.If panels should start expanded, apply this diff:
const allActiveKeys = categoryEntries.map( (_, index) => `${categoryKeyPrefix}_${index}`, ); return ( <Collapse key={`${categoryKeyPrefix}_${categoryEntries.length}`} - defaultActiveKey={[]} + defaultActiveKey={allActiveKeys} >Otherwise, remove the unused variable and update the comment to reflect the intended collapsed behavior.
🧹 Nitpick comments (2)
web/src/components/table/channels/modals/ModelSelectModal.jsx (2)
68-80: Consider memoizing derived model lists for better search performance.The derived values (
normalizedModels,filteredModels,newModels,existingModels,newModelsByCategory,existingModelsByCategory) are recomputed on every render, including on each keystroke during search. For large model lists (hundreds+), this could cause noticeable lag.Consider memoizing these computations:
+const normalizedModels = useMemo( + () => (models || []) + .map(getModelName) + .filter((name) => typeof name === 'string' && name.trim() !== ''), + [models] +); -const safeKeyword = keyword ? keyword.toLowerCase() : ''; -const normalizedModels = (models || []) - .map(getModelName) - .filter((name) => typeof name === 'string' && name.trim() !== ''); +const safeKeyword = useMemo( + () => keyword ? keyword.toLowerCase() : '', + [keyword] +); +const filteredModels = useMemo( + () => normalizedModels.filter((name) => + name.toLowerCase().includes(safeKeyword) + ), + [normalizedModels, safeKeyword] +); -const filteredModels = normalizedModels.filter((name) => - name.toLowerCase().includes(safeKeyword), -); +const { newModels, existingModels } = useMemo(() => ({ + newModels: filteredModels.filter((model) => !normalizedSelected.includes(model)), + existingModels: filteredModels.filter((model) => normalizedSelected.includes(model)) +}), [filteredModels, normalizedSelected]); -const newModels = filteredModels.filter((model) => !normalizedSelected.includes(model)); -const existingModels = filteredModels.filter((model) => - normalizedSelected.includes(model), -);Similarly for
newModelsByCategoryandexistingModelsByCategory(lines 141-142).Also applies to: 141-142
90-96: Remove redundantselecteddependency.The effect depends on both
newModels.lengthandselected, butnewModels.lengthalready captures changes toselected(throughnormalizedSelected). Includingselecteddirectly may cause an extra effect run without changing behavior.useEffect(() => { if (visible) { const hasNewModels = newModels.length > 0; setActiveTab(hasNewModels ? 'new' : 'existing'); } -}, [visible, newModels.length, selected]); +}, [visible, newModels.length]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/components/table/channels/modals/ModelSelectModal.jsx(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
Repo: QuantumNous/new-api PR: 1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the applyModelMapping function transforms the models list by replacing original model names (mapping values) with display names (mapping keys). The database stores this transformed list containing mapped keys. On channel load, data.models contains these mapped display names, making the initialization filter if (data.models.includes(key)) correct.
Applied to files:
web/src/components/table/channels/modals/ModelSelectModal.jsx
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
Repo: QuantumNous/new-api PR: 1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the database stores mapped keys (display names) in the models field after applying model mapping transformations. When loading a channel, data.models contains the mapped keys, not the original model names. The filtering logic if (data.models.includes(key)) in the initialization is correct.
Applied to files:
web/src/components/table/channels/modals/ModelSelectModal.jsx
🔇 Additional comments (1)
web/src/components/table/channels/modals/ModelSelectModal.jsx (1)
20-20: Previous re-render loop issue resolved.The critical re-render loop has been fixed correctly.
normalizedSelectedis now memoized based onselected(lines 56-59), preventing it from being recreated on every render. The effect on lines 83-87 properly depends on the memoized value, ensuring state syncs only whenvisibleor the actualselectedprop changes.Also applies to: 49-59, 83-87
# Conflicts: # web/bun.lock # web/src/components/table/channels/modals/EditChannelModal.jsx # web/src/components/table/channels/modals/ModelSelectModal.jsx # web/src/i18n/locales/en.json # web/src/i18n/locales/fr.json # web/src/i18n/locales/ru.json # web/src/i18n/locales/zh.json
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
web/src/hooks/channels/useChannelsData.jsx (1)
778-837: Missing translation keys need to be added.As noted in the past review, the following translation keys are missing from the locale files:
'暂无可复制的版本信息'(line 792)'已复制版本号'(line 798)'复制版本号'(line 811)These keys should be added to all locale files (zh.json, en.json, fr.json, ru.json) to ensure proper localization.
Consider using
Modal.destroy()instead ofModal.destroyAll().Line 816 uses
Modal.destroyAll()which destroys all open modals, not just the current one. If other modals are open when the user clicks "关闭", they will be unexpectedly closed.🔎 Proposed fix
The Semi UI
Modal.info()returns a modal instance with adestroy()method. Store the instance and calldestroy()on it:- Modal.info({ + const modal = Modal.info({ title: t('Ollama 版本信息'), content: infoMessage, centered: true, footer: ( <div className='flex justify-end gap-2'> <Button type='tertiary' onClick={handleCopyVersion}> {t('复制版本号')} </Button> <Button type='primary' theme='solid' - onClick={() => Modal.destroyAll()} + onClick={() => modal.destroy()} > {t('关闭')} </Button> </div> ),web/src/components/table/channels/ChannelsColumnDefs.jsx (1)
121-130: Improve UX whendeployment_idis missing.The IO.NET tag is always rendered with
cursor-pointerand anonClickhandler, but clicking does nothing whendeployment_idis missing (the handler returns early at line 99-100). This creates a confusing user experience where the cursor suggests interactivity but clicking has no effect.🔎 Proposed fix
<span> <Tag color='purple' type='light' - className='cursor-pointer' - onClick={handleNavigate} + className={ionetMeta?.deployment_id ? 'cursor-pointer' : ''} + onClick={ionetMeta?.deployment_id ? handleNavigate : undefined} > IO.NET </Tag> </span>web/src/components/table/channels/modals/EditChannelModal.jsx (1)
1943-1998: IO.NET lock enforcement is inconsistent — disable remaining key inputs.The
handleInputChangeguard prevents edits tokeywhenisIonetLockedis true, but the UI inputs remain visually editable, causing confusing UX. The Vertex manual input textarea (lines 1943-1998) and the generic key Form.Input (lines 2028-2076) are missing thedisabled={isIonetLocked}prop.🔎 Proposed fix for Vertex manual input (around line 1943)
<Form.TextArea field='key' label={...} placeholder={...} rules={...} autoComplete='new-password' onChange={(value) => handleInputChange('key', value) } + disabled={isIonetLocked} extraText={...} autosize showClear />🔎 Proposed fix for generic key input (around line 2028)
<Form.Input field='key' label={...} placeholder={...} rules={...} autoComplete='new-password' onChange={(value) => handleInputChange('key', value) } + disabled={isIonetLocked} extraText={...} showClear />Also applies to: 2028-2076
🧹 Nitpick comments (1)
controller/channel.go (1)
212-261: Consider extracting common Ollama setup logic.The pattern of extracting
baseURLand the first key fromchannel.Keyis repeated across multiple Ollama handlers. Consider refactoring this into a helper function to improve maintainability.🔎 Suggested refactor
Create a helper function:
// getOllamaChannelConfig extracts baseURL and key for Ollama channel operations func getOllamaChannelConfig(channel *model.Channel) (baseURL string, key string) { baseURL = constant.ChannelBaseURLs[channel.Type] if channel.GetBaseURL() != "" { baseURL = channel.GetBaseURL() } key = strings.Split(channel.Key, "\n")[0] return }Then replace repeated code blocks with:
baseURL, key := getOllamaChannelConfig(channel)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
controller/channel.gogo.modrouter/api-router.goweb/src/App.jsxweb/src/components/layout/SiderBar.jsxweb/src/components/table/channels/ChannelsColumnDefs.jsxweb/src/components/table/channels/modals/EditChannelModal.jsxweb/src/components/table/channels/modals/ModelSelectModal.jsxweb/src/helpers/render.jsxweb/src/hooks/channels/useChannelsData.jsx
🚧 Files skipped from review as they are similar to previous changes (6)
- web/src/helpers/render.jsx
- web/src/App.jsx
- go.mod
- web/src/components/table/channels/modals/ModelSelectModal.jsx
- router/api-router.go
- web/src/components/layout/SiderBar.jsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
Repo: QuantumNous/new-api PR: 1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the applyModelMapping function transforms the models list by replacing original model names (mapping values) with display names (mapping keys). The database stores this transformed list containing mapped keys. On channel load, data.models contains these mapped display names, making the initialization filter if (data.models.includes(key)) correct.
Applied to files:
web/src/components/table/channels/modals/EditChannelModal.jsx
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
Repo: QuantumNous/new-api PR: 1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the database stores mapped keys (display names) in the models field after applying model mapping transformations. When loading a channel, data.models contains the mapped keys, not the original model names. The filtering logic if (data.models.includes(key)) in the initialization is correct.
Applied to files:
web/src/components/table/channels/modals/EditChannelModal.jsx
🧬 Code graph analysis (2)
web/src/components/table/channels/ChannelsColumnDefs.jsx (2)
web/src/components/table/channels/modals/OllamaModelModal.jsx (1)
parsed(95-95)web/src/hooks/channels/useChannelsData.jsx (1)
checkOllamaVersion(778-837)
web/src/hooks/channels/useChannelsData.jsx (3)
web/src/helpers/api.js (6)
res(224-224)res(225-225)res(301-301)res(302-302)API(29-37)API(29-37)web/src/helpers/secureApiCall.js (2)
data(55-55)error(33-33)web/src/helpers/utils.jsx (3)
showInfo(161-163)showSuccess(157-159)showError(122-151)
🔇 Additional comments (24)
controller/channel.go (7)
14-14: LGTM: Ollama package import added.The import is properly added and used throughout the file for Ollama integration.
20-25: LGTM: Metadata field enhancement.The
Metadatafield addition toOpenAIModelis well-designed with proper JSON tags and flexible typing for Ollama model metadata.
1031-1055: LGTM: Ollama model fetching logic.The Ollama-specific handling correctly fetches models and returns names. This follows the same pattern as
FetchUpstreamModelsand would benefit from the same refactoring mentioned earlier.
1719-1780: LGTM: Model pull functionality.The
OllamaPullModelhandler properly validates inputs, checks channel type, and delegates to the Ollama package. Error handling is appropriate.
1782-1862: LGTM: SSE streaming implementation is correct.The
OllamaPullModelStreamhandler properly implements Server-Sent Events with:
- Correct SSE headers
- Proper data formatting with
data:prefix- Periodic flushing for real-time updates
- [DONE] marker for stream completion
- Error handling within the stream
1864-1925: LGTM: Model deletion functionality.The
OllamaDeleteModelhandler follows the established pattern with proper validation and error handling.
1927-1976: LGTM: Version fetching functionality.The
OllamaVersionhandler correctly retrieves and returns Ollama service version information with appropriate validation.web/src/hooks/channels/useChannelsData.jsx (2)
38-38: LGTM!Import addition is appropriate for the new modal footer buttons.
1196-1196: LGTM!Properly exports
checkOllamaVersionas part of the hook's return object.web/src/components/table/channels/ChannelsColumnDefs.jsx (5)
50-51: LGTM!Good refactor to accept the full
recordobject, enabling access to additional channel data likeother_info.
81-91: LGTM!Robust JSON parsing with try/catch to handle invalid metadata gracefully.
289-289: LGTM!Parameter addition properly threads
checkOllamaVersionthrough for use in the operations menu.
389-389: LGTM!Updated call correctly passes the full
recordtorenderType.
623-630: LGTM!Good addition of the "测活" (version check) menu item for Ollama channels (type 4). The conditional rendering based on
record.type === 4is appropriate.web/src/components/table/channels/modals/EditChannelModal.jsx (10)
58-58: LGTM!Import for the new OllamaModelModal component.
184-184: LGTM!State for controlling OllamaModelModal visibility.
219-237: LGTM!Proper state initialization for IO.NET channel tracking with appropriate reset in useEffect when not in edit mode.
239-245: LGTM!Navigation handler for opening the associated IO.NET deployment page. The guard clause for missing
deployment_idprevents navigation errors.
426-431: LGTM!Good implementation of the lock mechanism that prevents editing of
type,key, andbase_urlfor IO.NET-managed channels.
652-670: LGTM!Robust parsing of IO.NET metadata from
other_infowith proper error handling. Correctly identifies ionet-managed channels by checking forsource === 'ionet'.
678-732: LGTM!Good addition of the
silentoption tofetchUpstreamModelListto support fetching models without immediately opening the modal picker. This enables the OllamaModelModal to trigger a refresh without disrupting the current UI state.
1675-1696: LGTM!Well-designed IO.NET channel banner with informative message and optional deployment navigation button. The conditional rendering of the button based on
deployment_idpresence is correct.
2533-2542: LGTM!Appropriate conditional rendering of the Ollama model management button only for Ollama channels (type 4) in edit mode.
3166-3192: LGTM!Well-integrated OllamaModelModal with proper callbacks:
onModelsUpdatetriggers a silent refresh of the model listonApplyModelscorrectly merges incoming models with existing ones using Set for deduplication- Form state is properly synchronized via both
handleInputChangeandformApiRef.current.setValue
…and web UI model deployment components.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
pkg/ionet/hardware.go (1)
44-53: MaxGPUs incorrectly set to the requested gpuCount.This assigns the per-container GPU request to
MaxGPUs, which should represent hardware capability. The API response (availableReplicaPayload) doesn't include max GPU info, so either leave it as 0 or fetch the capability from hardware metadata.🔎 Suggested fix
return AvailableReplica{ LocationID: item.ID, LocationName: item.Name, HardwareID: hardwareID, HardwareName: "", AvailableCount: item.AvailableReplicas, - MaxGPUs: gpuCount, + MaxGPUs: 0, // Hardware capability unknown from this endpoint }pkg/ionet/container.go (1)
124-149: GetContainerLogs loses pagination metadata from API response.
GetContainerLogscallsGetContainerLogsRaw, then parses the response as newline-separated text. If the API returns JSON withhas_moreandnext_cursorfields (as indicated byContainerLogsstruct), this parsing approach discards pagination metadata and may incorrectly split JSON content.Consider either:
- Parse the raw response as JSON into
ContainerLogsdirectly- Document that this method is for plain-text log endpoints only
🧹 Nitpick comments (9)
web/src/hooks/model-deployments/useModelDeploymentSettings.js (2)
35-66: Consider memoizinggetSettingswithuseCallback.
getSettingsis exposed asrefreshin the return value. If consumers use it as a dependency in their own effects, it will trigger on every render since it's recreated each time. Wrapping it inuseCallbackwould provide a stable reference.🔎 Suggested refactor
- const getSettings = async () => { + const getSettings = useCallback(async () => { try { setLoading(true); const res = await API.get('/api/option/'); const { success, data } = res.data; if (success) { const newSettings = { 'model_deployment.ionet.enabled': false, 'model_deployment.ionet.api_key': '', }; data.forEach((item) => { if (item.key.endsWith('enabled')) { newSettings[item.key] = toBoolean(item.value); } else if (newSettings.hasOwnProperty(item.key)) { newSettings[item.key] = item.value || ''; } }); setSettings(newSettings); } } catch (error) { console.error('Failed to get model deployment settings:', error); } finally { setLoading(false); } - }; + }, []); useEffect(() => { getSettings(); - }, []); + }, [getSettings]);
47-53: Broad key matching could pick up unintended settings.
item.key.endsWith('enabled')will match any key ending with "enabled", potentially including future settings not related to io.net. Consider using an explicit check:- if (item.key.endsWith('enabled')) { + if (item.key === 'model_deployment.ionet.enabled') {web/src/components/table/model-deployments/DeploymentsFilters.jsx (1)
39-45: Consider using a callback-based approach instead ofsetTimeout.The
setTimeout(0)pattern to defersubmitFormafterreset()is fragile and relies on timing. Semi UI's FormApi might provide a callback or you could usesetValueswith explicit values followed by an immediate submit:const handleReset = () => { if (!formApiRef.current) return; formApiRef.current.setValues(formInitValues); searchDeployments(formInitValues); };This approach is more predictable than relying on the event loop.
web/src/components/model-deployments/DeploymentAccessGuard.jsx (1)
259-273: Inconsistent translation language between UI states.The disabled state (lines 49, 112, 122, etc.) uses Chinese translation keys like
'加载设置中...'and'模型部署服务未启用', while the connection error state uses English keys like'Checking io.net connection...','API key expired', and'Go to settings'. This inconsistency may lead to mixed-language UI if translation files are incomplete.Consider using a consistent approach—either all Chinese keys or all English keys—across the component.
pkg/ionet/types.go (1)
96-102: Inconsistent type forEnvVariablesacross config structs.
DeploymentContainerConfig.EnvVariablesusesmap[string]interface{}(line 99) whileContainerConfig.EnvVariables(line 48) usesmap[string]string. This inconsistency could cause type conversion issues when processing environment variables across different contexts.Consider aligning both to use
map[string]stringif the API always returns string values, or document why the difference exists.pkg/ionet/client.go (1)
188-189: Boolean query params added unconditionally.Unlike other types that check for zero values, boolean parameters are always added to the query string. If
falseshould not appear in the URL (some APIs treat parameter presence as truthy), this could cause unexpected behavior.🔎 Suggested fix
case bool: - values.Add(key, strconv.FormatBool(v)) + if v { + values.Add(key, strconv.FormatBool(v)) + }pkg/ionet/hardware.go (1)
151-155: Inconsistent JSON decoding approach.
GetHardwareType,GetLocation, andGetLocationAvailabilityusejson.Unmarshaldirectly, while other methods usedecodeDataordecodeDataWithFlexibleTimes. If these endpoints also wrap responses in{"data": ...}or contain timestamps, parsing may fail.Consider using the appropriate decode helper for consistency with other methods.
Also applies to: 173-177, 195-199
pkg/ionet/container.go (1)
224-226: Hardcoded 2-second polling interval without backoff.The fixed
time.Sleep(2 * time.Second)may be too aggressive for the API or too slow for real-time needs. Consider making this configurable or implementing exponential backoff when no new logs are received.// Example: configurable or adaptive interval pollInterval := 2 * time.Second if len(logs.Logs) == 0 { pollInterval = min(pollInterval*2, 30*time.Second) // backoff } time.Sleep(pollInterval)pkg/ionet/deployment.go (1)
343-346: Add a code comment explaining the raw boolean response format from the external API.The external IoEnterprise API returns a raw boolean rather than a wrapped object (which is unusual but valid JSON). Document this in the code so future maintainers understand why the unmarshaling pattern differs from typical REST API designs:
// Note: IoEnterprise API returns a raw boolean, not a wrapped object var availabilityResp bool if err := json.Unmarshal(resp.Body, &availabilityResp); err != nil { return false, fmt.Errorf("failed to parse cluster name availability response: %w", err) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.gitignorepkg/ionet/client.gopkg/ionet/container.gopkg/ionet/deployment.gopkg/ionet/hardware.gopkg/ionet/jsonutil.gopkg/ionet/types.goweb/src/components/model-deployments/DeploymentAccessGuard.jsxweb/src/components/table/model-deployments/DeploymentsActions.jsxweb/src/components/table/model-deployments/DeploymentsFilters.jsxweb/src/components/table/model-deployments/index.jsxweb/src/hooks/model-deployments/useModelDeploymentSettings.jsweb/src/pages/ModelDeployment/index.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/components/table/model-deployments/DeploymentsActions.jsx
- web/src/components/table/model-deployments/index.jsx
🧰 Additional context used
🧬 Code graph analysis (5)
web/src/components/model-deployments/DeploymentAccessGuard.jsx (6)
web/src/pages/Setting/Model/SettingModelDeployment.jsx (3)
Typography(32-32)useTranslation(35-35)loading(37-37)web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx (4)
Typography(42-42)useTranslation(50-50)loading(52-52)title(58-58)web/src/components/layout/SiderBar.jsx (1)
useTranslation(54-54)web/src/hooks/model-deployments/useDeploymentsData.jsx (2)
useTranslation(27-27)loading(32-32)web/src/pages/Setting/index.jsx (2)
useTranslation(52-52)navigate(53-53)web/src/components/settings/ModelDeploymentSetting.jsx (2)
useTranslation(27-27)loading(33-33)
pkg/ionet/jsonutil.go (1)
common/json.go (2)
Unmarshal(9-11)Marshal(21-23)
web/src/components/table/model-deployments/DeploymentsFilters.jsx (1)
web/src/hooks/model-deployments/useDeploymentsData.jsx (4)
searchDeployments(208-242)formInitValues(57-60)formApi(70-70)loading(32-32)
pkg/ionet/hardware.go (3)
pkg/ionet/types.go (9)
Client(8-12)AvailableReplicasResponse(166-168)AvailableReplica(156-163)MaxGPUResponse(171-174)HardwareType(303-317)MaxGPUInfo(177-183)LocationsResponse(333-336)Location(320-330)LocationAvailability(339-345)controller/deployment.go (1)
GetAvailableReplicas(535-569)common/json.go (1)
Unmarshal(9-11)
pkg/ionet/deployment.go (3)
pkg/ionet/types.go (17)
Client(8-12)DeploymentRequest(35-43)DeploymentResponse(63-66)RegistryConfig(56-60)ContainerConfig(46-53)ListDeploymentsOptions(282-289)DeploymentList(149-153)Deployment(131-146)DeploymentDetail(69-87)UpdateDeploymentRequest(232-242)UpdateDeploymentResponse(250-253)ExtendDurationRequest(245-247)PriceEstimationRequest(186-196)PriceEstimationResponse(199-204)PriceBreakdown(207-213)UpdateClusterNameRequest(256-258)UpdateClusterNameResponse(261-264)common/json.go (1)
Unmarshal(9-11)controller/deployment.go (6)
GetDeployment(267-314)UpdateDeployment(371-399)ExtendDeployment(401-438)DeleteDeployment(440-463)GetPriceEstimation(571-590)CheckClusterNameAvailability(592-615)
🔇 Additional comments (17)
web/src/hooks/model-deployments/useModelDeploymentSettings.js (1)
88-122: Well-structured connection testing with comprehensive error handling.The
testConnectionfunction properly handles various error scenarios (network errors, API failures) and normalizes them throughbuildConnectionError. Good use ofuseCallbackfor memoization.web/src/pages/ModelDeployment/index.jsx (1)
25-50: Clean component composition with proper separation of concerns.The page correctly delegates settings management to the hook and access control to the guard component. Props are passed appropriately.
web/src/components/table/model-deployments/DeploymentsFilters.jsx (1)
57-127: Well-structured responsive form layout.The form uses a clean responsive design pattern with Tailwind classes for mobile/desktop breakpoints. Props are appropriately wired to Semi UI Form components.
web/src/components/model-deployments/DeploymentAccessGuard.jsx (1)
44-54: Clear state machine handling for access control.The component correctly prioritizes states: loading → disabled → connection loading → connection error → children. Each state has appropriate UI feedback.
Also applies to: 254-264, 374-374
pkg/ionet/types.go (1)
1-279: Well-structured type definitions for the IO.NET API client.The data models are comprehensive and well-organized with appropriate JSON tags, optional field handling via
omitempty, and proper use of pointer types for nullable time fields. TheAPIErrorimplementation correctly satisfies theerrorinterface.pkg/ionet/jsonutil.go (2)
11-26: Performance note: double JSON serialization.The
decodeWithFlexibleTimesfunction performs unmarshal → normalize → marshal → unmarshal. While this approach correctly handles inconsistent timestamp formats from the API, it has O(2n) serialization overhead. This is acceptable for API response sizes but worth noting if processing large payloads.
89-92: Timezone conversion to UTC may lose original timezone info.The normalization converts parsed times to UTC. If the original timezone is meaningful for downstream processing (e.g., displaying local times to users), consider preserving the original zone or documenting this behavior.
// Current: converts to UTC return parsed.UTC().Format(time.RFC3339Nano), truepkg/ionet/client.go (2)
34-71: LGTM: Clean HTTP client implementation.The
DefaultHTTPClient.Domethod properly handles request creation, header injection, response reading, and cleanup withdefer resp.Body.Close().
198-209: No action required. The IO.NET API documentation confirms thatlocation_idsand similar array parameters are expected as JSON-encoded arrays (e.g.,?location_ids=[1,2,3]). The current implementation correctly JSON-marshals[]intand[]stringslices inbuildQueryParams, matching the documented API specification.pkg/ionet/hardware.go (1)
73-110: Good defensive handling in ListHardwareTypes.The fallback name generation, total calculation when missing, and mapping logic are well-implemented.
pkg/ionet/container.go (3)
110-115: Time parameters passed without RFC3339 formatting.
opts.StartTimeandopts.EndTimeare passed directly tobuildQueryParams. ThebuildQueryParamsfunction inclient.gohandles*time.Timeby formatting to RFC3339, so this should work correctly.
185-222: StreamContainerLogs now correctly uses buildLogEndpoint.The implementation addresses the previous concern about duplicate query building logic. The cursor update and endpoint reconstruction logic is correct.
269-302: ExecuteInContainer: Verify API supports this endpoint.The
/execendpoint for container command execution may not be available in all environments or may require specific permissions. The fallback to return raw response body whenoutputfield is missing is a reasonable defensive measure.pkg/ionet/deployment.go (4)
80-84: Derived fields assumption documented.The comment "Assuming 1:1 mapping for now" for
Replicas = HardwareQuantityis helpful. Consider adding a TODO or linking to documentation if the actual mapping is expected to differ.
316-320: Verify ComputeCost calculation logic.
ComputeCostis calculated asTotalCostUSDC - IonetFee - CurrencyConversionFee. Confirm this accurately represents compute cost (the API may have other components like network/storage that should be excluded from compute).ComputeCost: pricingData.TotalCostUSDC - pricingData.IonetFee - pricingData.CurrencyConversionFee,
11-53: Comprehensive input validation in DeployContainer.All required fields are validated before the API call, with clear error messages. This defensive approach prevents unnecessary API calls with invalid requests.
236-255: Well-handled duration type normalization.The switch statement correctly handles various duration type aliases (hour/hours/hourly, day/days/daily, etc.) and computes appropriate
durationHoursForRate. The fallback to "hourly" ensures a valid default.
| .gomodcache/ | ||
| .gocache-temp | ||
| .gopath |
There was a problem hiding this comment.
Remove duplicate .gomodcache/ entry.
Line 26 duplicates the .gomodcache/ pattern from line 19. The new patterns .gocache-temp and .gopath are appropriate additions for a Go project, but remove the redundant entry to keep the file clean.
🔎 Proposed fix
-web/bun.lock
-
-electron/node_modules
-electron/dist
-data/
-.gomodcache/
-.gocache-temp
-.gopath
+web/bun.lock
+
+electron/node_modules
+electron/dist
+data/
+.gocache-temp
+.gopathCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .gitignore around lines 26 to 28, there is a duplicated `.gomodcache/` entry
(already present at line 19); remove the redundant `.gomodcache/` line and keep
the new `.gocache-temp` and `.gopath` entries so the file contains each pattern
only once.
| onMouseEnter={(e) => { | ||
| e.target.style.background = 'var(--semi-color-fill-1)'; | ||
| e.target.style.transform = 'translateY(-1px)'; | ||
| e.target.style.boxShadow = '0 2px 8px rgba(0, 0, 0, 0.1)'; | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| e.target.style.background = 'var(--semi-color-fill-0)'; | ||
| e.target.style.transform = 'translateY(0)'; | ||
| e.target.style.boxShadow = 'none'; | ||
| }} |
There was a problem hiding this comment.
Use e.currentTarget instead of e.target to avoid styling child elements.
When the mouse enters/leaves the div, if the pointer is over a child element (like the Settings icon or text), e.target will reference that child, causing the style to be incorrectly applied to it. Use e.currentTarget to always reference the element with the handler attached.
🔎 Proposed fix
onMouseEnter={(e) => {
- e.target.style.background = 'var(--semi-color-fill-1)';
- e.target.style.transform = 'translateY(-1px)';
- e.target.style.boxShadow = '0 2px 8px rgba(0, 0, 0, 0.1)';
+ e.currentTarget.style.background = 'var(--semi-color-fill-1)';
+ e.currentTarget.style.transform = 'translateY(-1px)';
+ e.currentTarget.style.boxShadow = '0 2px 8px rgba(0, 0, 0, 0.1)';
}}
onMouseLeave={(e) => {
- e.target.style.background = 'var(--semi-color-fill-0)';
- e.target.style.transform = 'translateY(0)';
- e.target.style.boxShadow = 'none';
+ e.currentTarget.style.background = 'var(--semi-color-fill-0)';
+ e.currentTarget.style.transform = 'translateY(0)';
+ e.currentTarget.style.boxShadow = 'none';
}}🤖 Prompt for AI Agents
In web/src/components/model-deployments/DeploymentAccessGuard.jsx around lines
221-230, the mouse event handlers use e.target which can refer to child elements
(like icons or text) and results in styling being applied to the wrong element;
change both handlers to use e.currentTarget so the styles are applied to the
element the listener is attached to (the wrapper div) and not to any child
nodes, leaving the rest of the inline style assignments unchanged.
…text, and a new description component.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx (2)
187-195: Fix deployment name validation regex to avoid unintended charactersThe current pattern
[a-zA-Z0-9-_\u4e00-\u9fa5]creates a range between0and_, allowing extra punctuation. Move-to the end (or escape it) so only letters, digits, underscore, hyphen, and CJK are accepted.Suggested change
- { - pattern: /^[a-zA-Z0-9-_\u4e00-\u9fa5]+$/, + { + pattern: /^[A-Za-z0-9_\u4e00-\u9fa5-]+$/, message: t( '部署名称只能包含字母、数字、横线、下划线和中文', ), },
220-234: Footer still exposes a misleading “创建” path for a rename-only modalEven after making
isEdita strict boolean and forcing the title to “重命名部署”, the footer still shows “创建” when!isEditand always submits the form.handleSubmitimmediately rejects non-edit submissions with “无效的部署信息”, so users see a “创建” button that can only error.To keep this strictly a rename modal:
- Always label the primary action as “更新”.
- Disable the button when there is no
editingDeployment.id.Suggested change
- <Button - theme='solid' - type='primary' - loading={loading} - onClick={() => formRef.current?.submitForm()} - > - <Save size={16} className='mr-1' /> - {isEdit ? t('更新') : t('创建')} - </Button> + <Button + theme='solid' + type='primary' + loading={loading} + disabled={!isEdit || !editingDeployment?.id} + onClick={() => formRef.current?.submitForm()} + > + <Save size={16} className='mr-1' /> + {t('更新')} + </Button>web/src/components/table/model-deployments/modals/ViewLogsModal.jsx (1)
288-306: Auto-refresh interval still not tied to deployment idThe auto-refresh
useEffectdoesn’t depend ondeployment?.id, so if you switch to a different deployment while the modal stays open, the existing interval keeps callingfetchLogsfor the old deployment until another dependency changes.Add the current deployment id to the dependency array so the interval is torn down and recreated when the deployment changes:
- }, [autoRefresh, visible, selectedContainerId, streamFilter, following]); + }, [autoRefresh, visible, selectedContainerId, streamFilter, following, deployment?.id]);The existing cleanup logic already clears the previous interval.
web/src/components/table/model-deployments/modals/ExtendDurationModal.jsx (1)
230-258: Clarify success vs failure for extend API andonSuccesscallbackTwo issues in
handleExtend:
- If the API returns
success: falsein a 2xx response, nothing is shown to the user.- Any error thrown by
onSuccessis caught and surfaced as “延长时长失败…”, even though the extension may have completed successfully.That can both hide backend errors and mislead users when only the UI callback fails.
Consider:
- Adding an explicit
elseto show an error based onresponse.data.message.- Calling
onSuccessafterhandleCancelin its owntry/catchso callback failures are logged but don’t override the “成功” toast.Example restructuring
- const response = await API.post( - `/api/deployments/${deployment.id}/extend`, - { - duration_hours: Math.round(durationHours), - }, - ); - - if (response.data.success) { - showSuccess(t('容器时长延长成功')); - onSuccess?.(response.data.data); - handleCancel(); - } + const response = await API.post( + `/api/deployments/${deployment.id}/extend`, + { duration_hours: Math.round(durationHours) }, + ); + + if (response.data.success) { + showSuccess(t('容器时长延长成功')); + handleCancel(); + try { + onSuccess?.(response.data.data); + } catch (callbackError) { + console.error('onSuccess callback error:', callbackError); + } + } else { + const msg = response.data.message || ''; + showError( + t('延长时长失败') + (msg ? `: ${msg}` : ''), + ); + }web/src/components/table/model-deployments/DeploymentsActions.jsx (1)
36-44: Fallback to edit modal remains problematic (duplicate concern)The fallback logic still opens the edit/rename modal when
setShowCreateModalis not provided. While the parent component does provide this prop (making the fallback unreachable in practice), the dead-end code path should be removed or replaced with proper error handling as previously recommended.web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
31-41: Missing legacy columns in selector (duplicate concern)The
columnOptionsarray only includes 6 columns but the hook defines 11 total columns (including legacy ones:deployment_name,model_name,instance_count,resource_config,updated_at). Users cannot toggle these legacy columns via the modal. As previously noted, add entries for all columns defined in the hook'sCOLUMN_KEYS.
🧹 Nitpick comments (4)
web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx (1)
60-84: Remove unused resource/model loading logic from a rename-only modal
cpuOptions,memoryOptions,gpuOptions, themodelsstate, andloadModels()(plus the/api/modelscall in thevisibleeffect) are never used in the JSX. This adds an unnecessary network request every time the rename modal opens and increases complexity for no functional gain.Consider deleting:
- The resource option arrays.
models/loadingModelsstate andloadModels.- The
useEffectthat callsloadModels().This keeps the component focused on renaming and avoids extra API traffic.
Also applies to: 85-104, 137-142
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (1)
534-604: Avoid treatingonSuccess/onCancelfailures as API failures in create flowIn
handleSubmit, any exception thrown byonSuccessoronCancelis caught and surfaced as “容器创建失败…”, even when the/api/deploymentscall succeeded. That misleads users and makes real backend issues harder to spot.You can mirror the pattern suggested for
UpdateConfigModal:
- Keep the API call and
showSuccessin the maintry.- After a successful response, call
onSuccess(and possibly close the modal) inside a separatetry/catchthat logs callback errors without overwriting the success toast.This keeps the success/failure boundary aligned with the backend result.
web/src/components/table/model-deployments/modals/ViewDetailsModal.jsx (2)
103-107: Use existing copy helper instead of rawnavigator.clipboard
handleCopyIdcallsnavigator.clipboard.writeTextwithout handling failures. Since the project already exposes acopyhelper with fallbacks, consider reusing it here so users on unsupported/denied clipboard environments get graceful behavior and consistent toasts.Example:
const handleCopyId = async () => { if (!deployment?.id) return; const ok = await copy(deployment.id); if (ok) { showSuccess(t('ID已复制到剪贴板')); } else { showError(t('复制失败，请手动选择文本复制')); } };
113-126: Derive status from fetched details rather than the initial deployment prop
statusConfigis computed fromdeployment?.status, but once details are loaded you also havedetails.status, which may have changed since the table row was rendered. The tag under “基本信息 → 状态” will stay stale.Prefer using
details.statuswhendetailsis present, with a fallback todeployment?.statusif needed.Also applies to: 200-207
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
web/src/components/model-deployments/DeploymentAccessGuard.jsxweb/src/components/table/model-deployments/DeploymentsActions.jsxweb/src/components/table/model-deployments/DeploymentsDescription.jsxweb/src/components/table/model-deployments/index.jsxweb/src/components/table/model-deployments/modals/ColumnSelectorModal.jsxweb/src/components/table/model-deployments/modals/ConfirmationDialog.jsxweb/src/components/table/model-deployments/modals/CreateDeploymentModal.jsxweb/src/components/table/model-deployments/modals/EditDeploymentModal.jsxweb/src/components/table/model-deployments/modals/ExtendDurationModal.jsxweb/src/components/table/model-deployments/modals/UpdateConfigModal.jsxweb/src/components/table/model-deployments/modals/ViewDetailsModal.jsxweb/src/components/table/model-deployments/modals/ViewLogsModal.jsxweb/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.json
✅ Files skipped from review due to trivial changes (1)
- web/src/i18n/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx
- web/src/components/model-deployments/DeploymentAccessGuard.jsx
🧰 Additional context used
🧬 Code graph analysis (8)
web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
web/src/hooks/model-deployments/useDeploymentsData.jsx (1)
visibleColumns(130-155)
web/src/components/table/model-deployments/DeploymentsDescription.jsx (2)
web/src/components/common/ui/CompactModeToggle.jsx (1)
CompactModeToggle(30-57)web/src/hooks/common/useTableCompactMode.js (1)
setCompactMode(34-40)
web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx (4)
web/src/components/table/model-deployments/DeploymentsDescription.jsx (1)
Typography(25-25)web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (1)
Typography(45-45)web/src/components/table/model-deployments/modals/UpdateConfigModal.jsx (1)
Typography(49-49)web/src/helpers/utils.jsx (2)
showError(122-151)showSuccess(157-159)
web/src/components/table/model-deployments/modals/ViewLogsModal.jsx (1)
web/src/helpers/utils.jsx (4)
showError(122-151)a(259-259)copy(72-95)timestamp2string(192-218)
web/src/components/table/model-deployments/DeploymentsActions.jsx (2)
web/src/components/layout/SiderBar.jsx (1)
selectedKeys(64-64)web/src/hooks/model-deployments/useDeploymentsData.jsx (3)
selectedKeys(45-45)batchDeleteDeployments(416-433)refresh(245-247)
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (1)
web/src/helpers/utils.jsx (4)
a(259-259)showError(122-151)showSuccess(157-159)copy(72-95)
web/src/components/table/model-deployments/index.jsx (6)
web/src/components/table/model-deployments/DeploymentsTable.jsx (2)
deploymentsData(37-60)DeploymentsTable(36-245)web/src/hooks/model-deployments/useDeploymentsData.jsx (10)
useDeploymentsData(26-507)useDeploymentsData(26-507)refresh(245-247)visibleColumns(130-155)COLUMN_KEYS(96-114)batchDeleteDeployments(416-433)formInitValues(57-60)searchDeployments(208-242)loading(32-32)searching(35-35)web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
ColumnSelectorModal(23-127)web/src/components/common/ui/CardPro.jsx (1)
CardPro(44-174)web/src/components/table/model-deployments/DeploymentsActions.jsx (1)
DeploymentsActions(23-107)web/src/components/table/model-deployments/DeploymentsFilters.jsx (1)
DeploymentsFilters(24-128)
web/src/components/table/model-deployments/modals/ViewDetailsModal.jsx (1)
web/src/helpers/utils.jsx (3)
showError(122-151)showSuccess(157-159)timestamp2string(192-218)
🔇 Additional comments (7)
web/src/i18n/locales/ru.json (1)
2247-2264: New io.net / deployment i18n keys look consistentThe added Russian translations for the deployment/io.net keys (
"模型部署"through"重试连接") preserve interpolation placeholders and accurately reflect the Chinese source strings. No structural JSON issues spotted around this block.web/src/components/table/model-deployments/DeploymentsDescription.jsx (1)
27-44: DeploymentsDescription component is clean and idiomaticProps are passed through correctly, i18n keys are wired, and the layout integrates with
CompactModeToggleas expected. No changes needed here.web/src/i18n/locales/ja.json (1)
2136-2153: LGTM: Japanese translations for model deployment featureThe new translation keys properly support the IO.NET model deployment feature with appropriate Japanese localization. The translations cover all necessary UI messages for settings, connection status, and error states.
web/src/i18n/locales/zh.json (1)
2214-2231: LGTM: Chinese translations for model deployment featureThe new translation keys provide proper Chinese localization for the IO.NET model deployment feature, covering settings, connection status, and user guidance messages. The additions are consistent with the existing translation structure.
web/src/components/table/model-deployments/index.jsx (1)
33-151: Well-structured page compositionThe
DeploymentsPagecomponent cleanly orchestrates the deployments UI by:
- Managing local create modal state while consuming shared state from the hook
- Properly wiring all modals (Edit, Create, ColumnSelector) with their required props
- Composing the CardPro layout with description, actions, filters, and pagination
- Threading translation function consistently through all child components
The separation of concerns and prop flow are clear and maintainable.
web/src/i18n/locales/vi.json (1)
2747-2764: New io.net deployment Vietnamese strings look consistent and correctThe added translations correctly capture the Chinese source meaning, maintain style consistency with surrounding Vietnamese entries, and introduce no interpolation or formatting issues. Nothing to change here.
web/src/i18n/locales/fr.json (1)
2237-2254: French io.net deployment translations are accurate and idiomaticThe new French entries faithfully reflect the original Chinese strings, read naturally in a UI context, and introduce no placeholder or formatting issues. All good from an i18n standpoint.
| const handleReset = () => { | ||
| const defaults = columnOptions.reduce((acc, { key }) => { | ||
| acc[key] = true; | ||
| return acc; | ||
| }, {}); | ||
| onVisibleColumnsChange({ | ||
| ...visibleColumns, | ||
| ...defaults, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Reset logic doesn't match hook's default visibility states
The handleReset function sets all columns to true, but the hook's default state hides legacy columns (sets them to false). This causes reset to diverge from the application's intended defaults. Consider accepting a defaultVisibleColumns prop from the parent or importing the hook's default map to ensure consistency.
🤖 Prompt for AI Agents
In web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx
around lines 60 to 69, the handleReset currently sets every column key to true
which conflicts with the hook's intended default visibility (some legacy columns
default to false); update handleReset to use the actual default visibility map
instead of forcing true — either accept a defaultVisibleColumns prop from the
parent and call onVisibleColumnsChange({ ...visibleColumns,
...defaultVisibleColumns }), or import/consume the hook's defaultVisibleColumns
map and merge that in so reset restores the hook's real defaults rather than
enabling every column.
| const handleUpdate = async () => { | ||
| try { | ||
| const formValues = formRef.current ? await formRef.current.validate() : {}; | ||
| setLoading(true); | ||
|
|
||
| // Prepare the update payload | ||
| const payload = {}; | ||
|
|
||
| if (formValues.image_url) payload.image_url = formValues.image_url; | ||
| if (formValues.traffic_port) payload.traffic_port = formValues.traffic_port; | ||
| if (formValues.registry_username) payload.registry_username = formValues.registry_username; | ||
| if (formValues.registry_secret) payload.registry_secret = formValues.registry_secret; | ||
| if (formValues.command) payload.command = formValues.command; | ||
|
|
||
| // Process entrypoint | ||
| if (formValues.entrypoint) { | ||
| payload.entrypoint = formValues.entrypoint.split(' ').filter(cmd => cmd.trim()); | ||
| } | ||
|
|
||
| // Process environment variables | ||
| if (envVars.length > 0) { | ||
| payload.env_variables = envVars.reduce((acc, env) => { | ||
| if (env.key && env.value !== undefined) { | ||
| acc[env.key] = env.value; | ||
| } | ||
| return acc; | ||
| }, {}); | ||
| } | ||
|
|
||
| // Process secret environment variables | ||
| if (secretEnvVars.length > 0) { | ||
| payload.secret_env_variables = secretEnvVars.reduce((acc, env) => { | ||
| if (env.key && env.value !== undefined) { | ||
| acc[env.key] = env.value; | ||
| } | ||
| return acc; | ||
| }, {}); | ||
| } | ||
|
|
||
| const response = await API.put(`/api/deployments/${deployment.id}`, payload); | ||
|
|
||
| if (response.data.success) { | ||
| showSuccess(t('容器配置更新成功')); | ||
| onSuccess?.(response.data.data); | ||
| handleCancel(); | ||
| } | ||
| } catch (error) { | ||
| showError(t('更新配置失败') + ': ' + (error.response?.data?.message || error.message)); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Handle non-success responses and separate callback failures from API failures
Right now:
- If
response.data.successisfalse, the user gets no feedback. - Any error thrown by
onSuccessis caught and reported as “更新配置失败”, even though the API call may have succeeded.
That can both hide real backend errors and mislead users when only the UI callback fails.
Consider:
- Adding an
elsebranch to showresponse.data.message(or a fallback). - Calling
onSuccessin its owntry/catchafter handling the API result, so callback errors are logged but don’t show a misleading “更新配置失败”.
One possible refactor
- const response = await API.put(`/api/deployments/${deployment.id}`, payload);
-
- if (response.data.success) {
- showSuccess(t('容器配置更新成功'));
- onSuccess?.(response.data.data);
- handleCancel();
- }
+ const response = await API.put(
+ `/api/deployments/${deployment.id}`,
+ payload,
+ );
+
+ if (response.data.success) {
+ showSuccess(t('容器配置更新成功'));
+ handleCancel();
+ try {
+ onSuccess?.(response.data.data);
+ } catch (callbackError) {
+ // Avoid masking API success with callback errors
+ console.error('onSuccess callback error:', callbackError);
+ }
+ } else {
+ showError(
+ t('更新配置失败') +
+ (response.data.message ? `: ${response.data.message}` : ''),
+ );
+ }🤖 Prompt for AI Agents
In web/src/components/table/model-deployments/modals/UpdateConfigModal.jsx
around lines 92 to 143, the code currently ignores API responses where
response.data.success === false and conflates exceptions from the onSuccess
callback with API failures; update the flow so that after awaiting the PUT you
check if response.data.success is true then showSuccess and call onSuccess
inside its own try/catch (log or show a non-fatal message for callback errors
without reporting the overall update as failed), and add an else branch that
calls showError with response.data.message (or a sensible fallback) so users see
backend error messages when the API returns success: false. Ensure API
exceptions still go to the existing catch that shows "更新配置失败" and retain the
finally block that sets loading to false.
…updated text, and a new description component." This reverts commit 8b75cb5.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
web/src/components/table/model-deployments/DeploymentsActions.jsx (1)
38-46: Fallback still opens wrong modal for creation.This concern was already raised in the previous review. The fallback to
setEditingDeploymentandsetShowEditleads to opening the edit/rename modal instead of the proper create modal, resulting in a dead-end submit flow.web/src/components/model-deployments/DeploymentAccessGuard.jsx (1)
221-230: Usee.currentTargetinstead ofe.targetto avoid styling child elements.When the mouse enters/leaves the wrapper,
e.targetcan reference a child element (Settings icon or text), causing styles to be applied incorrectly. Usee.currentTargetto always reference the element with the handler attached.🔎 Proposed fix
onMouseEnter={(e) => { - e.target.style.background = 'var(--semi-color-fill-1)'; - e.target.style.transform = 'translateY(-1px)'; - e.target.style.boxShadow = '0 2px 8px rgba(0, 0, 0, 0.1)'; + e.currentTarget.style.background = 'var(--semi-color-fill-1)'; + e.currentTarget.style.transform = 'translateY(-1px)'; + e.currentTarget.style.boxShadow = '0 2px 8px rgba(0, 0, 0, 0.1)'; }} onMouseLeave={(e) => { - e.target.style.background = 'var(--semi-color-fill-0)'; - e.target.style.transform = 'translateY(0)'; - e.target.style.boxShadow = 'none'; + e.currentTarget.style.background = 'var(--semi-color-fill-0)'; + e.currentTarget.style.transform = 'translateY(0)'; + e.currentTarget.style.boxShadow = 'none'; }}web/src/components/table/model-deployments/modals/ViewLogsModal.jsx (1)
288-306: Fix stale closure in auto-refresh interval.The auto-refresh effect continues polling logs for the previous deployment after switching because
deployment?.idis missing from the dependency array. When a user switches to a different deployment, the interval keeps fetching logs for the staledeployment.id.🔎 Fix: Add deployment?.id to dependencies
- }, [autoRefresh, visible, selectedContainerId, streamFilter, following]); + }, [autoRefresh, visible, selectedContainerId, streamFilter, following, deployment?.id]);web/src/components/table/model-deployments/modals/ExtendDurationModal.jsx (1)
244-248: Separate callback errors from API success.If
onSuccessthrows an exception, the catch block reports "延长时长失败" even though the API call succeeded. This misleads users—the deployment was extended, but the UI callback failed.🔎 Handle callback separately
if (response.data.success) { showSuccess(t('容器时长延长成功')); - onSuccess?.(response.data.data); handleCancel(); + try { + onSuccess?.(response.data.data); + } catch (callbackError) { + console.error('onSuccess callback error:', callbackError); + } }web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (2)
31-41: Missing legacy columns limits user control.The modal only exposes 6 columns but the hook defines 11. Users cannot toggle the 5 legacy columns (deployment_name, model_name, instance_count, resource_config, updated_at), limiting the modal's utility.
Add the missing legacy columns to
columnOptions:const columnOptions = useMemo( () => [ { key: columnKeys.container_name, label: t('容器名称'), required: true }, { key: columnKeys.status, label: t('状态') }, { key: columnKeys.time_remaining, label: t('剩余时间') }, { key: columnKeys.hardware_info, label: t('硬件配置') }, { key: columnKeys.created_at, label: t('创建时间') }, + { key: columnKeys.deployment_name, label: t('部署名称') }, + { key: columnKeys.model_name, label: t('模型名称') }, + { key: columnKeys.instance_count, label: t('实例数量') }, + { key: columnKeys.resource_config, label: t('资源配置') }, + { key: columnKeys.updated_at, label: t('更新时间') }, { key: columnKeys.actions, label: t('操作'), required: true }, ], [columnKeys, t], );Based on learnings.
60-69: Reset logic doesn't match hook's default visibility states.
handleResetsets all columns totrue, but the hook's defaults hide legacy columns (set them tofalse). This causes reset to diverge from the application's intended defaults.Consider accepting a
defaultVisibleColumnsprop or importing the hook's default map to ensure consistency.web/src/components/table/model-deployments/modals/UpdateConfigModal.jsx (1)
92-143: Handle non-success responses and separate callback failures.Two issues:
- If
response.data.successisfalse, the user gets no feedback (no else branch)- Any error thrown by
onSuccessis caught and reported as "更新配置失败", even though the API call may have succeeded🔎 Recommended fix
const response = await API.put(`/api/deployments/${deployment.id}`, payload); if (response.data.success) { showSuccess(t('容器配置更新成功')); - onSuccess?.(response.data.data); handleCancel(); + try { + onSuccess?.(response.data.data); + } catch (callbackError) { + console.error('onSuccess callback error:', callbackError); + } + } else { + showError( + t('更新配置失败') + + (response.data.message ? `: ${response.data.message}` : ''), + ); }
🧹 Nitpick comments (5)
web/src/components/table/model-deployments/DeploymentsActions.jsx (2)
78-85: Remove redundant disabled check.The
disabled={selectedKeys.length === 0}on line 81 is redundant since the button is only rendered whenhasSelectedis true (i.e.,selectedKeys.length > 0). The disabled condition can never evaluate to true.🔎 Proposed fix
<Button type='danger' className='flex-1 md:flex-initial' - disabled={selectedKeys.length === 0} size='small' > {t('批量删除')} ({selectedKeys.length}) </Button>
70-77: Consider using interpolation for translation.The content string concatenates multiple
t()calls, which can make translation more difficult since translators don't see the full sentence context. Consider using a single translation key with interpolation for better maintainability.🔎 Example refactor
<Popconfirm title={t('确认删除')} - content={`${t('确定要删除选中的')} ${selectedKeys.length} ${t('个部署吗？此操作不可逆。')}`} + content={t('确定要删除选中的 {{count}} 个部署吗？此操作不可逆。', { count: selectedKeys.length })} okText={t('删除')} cancelText={t('取消')} okType='danger' onConfirm={handleBatchDelete} >Note: This assumes your translation library supports interpolation. Adjust the syntax based on your i18n implementation.
web/src/components/model-deployments/DeploymentAccessGuard.jsx (1)
204-234: Use a<Button>component for better accessibility.The clickable div lacks keyboard navigation and screen reader support. Users relying on keyboard navigation or assistive technologies cannot interact with this element.
🔎 Proposed fix using Semi UI Button
- <div + <Button + theme="borderless" onClick={handleGoToSettings} + icon={<Settings size={18} />} style={{ display: 'inline-flex', alignItems: 'center', gap: '8px', - cursor: 'pointer', padding: '12px 24px', borderRadius: '8px', fontSize: '16px', fontWeight: '500', color: 'var(--semi-color-primary)', background: 'var(--semi-color-fill-0)', border: '1px solid var(--semi-color-border)', transition: 'all 0.2s ease', - textDecoration: 'none' }} - onMouseEnter={(e) => { - e.target.style.background = 'var(--semi-color-fill-1)'; - e.target.style.transform = 'translateY(-1px)'; - e.target.style.boxShadow = '0 2px 8px rgba(0, 0, 0, 0.1)'; - }} - onMouseLeave={(e) => { - e.target.style.background = 'var(--semi-color-fill-0)'; - e.target.style.transform = 'translateY(0)'; - e.target.style.boxShadow = 'none'; - }} > - <Settings size={18} /> {t('前往设置页面')} - </div> + </Button>Note: You may need to adjust the styling or use CSS classes to achieve the desired hover effects with the Button component.
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (1)
534-604: Consider validating required fields before submission.The submit handler builds a complex payload but doesn't explicitly validate that critical fields like
hardware_idandlocation_idsare present before making the API call. While the form has validation rules, adding explicit checks could provide clearer error messages.Optional validation enhancement
const handleSubmit = async (values) => { try { + // Explicit validation for critical fields + if (!values.hardware_id || !values.location_ids?.length) { + showError(t('请完成必填的硬件和位置配置')); + return; + } + setSubmitting(true); // ... rest of the functionweb/src/components/table/model-deployments/modals/ViewDetailsModal.jsx (1)
103-106: Consider using the sharedcopyhelper for consistency.The modal imports the
copyhelper buthandleCopyIdusesnavigator.clipboard.writeTextdirectly. Using the shared helper would provide consistent fallback behavior across the app.Use shared copy helper
- const handleCopyId = () => { - navigator.clipboard.writeText(deployment?.id); - showSuccess(t('ID已复制到剪贴板')); + const handleCopyId = async () => { + const copied = await copy(deployment?.id); + if (copied) { + showSuccess(t('ID已复制到剪贴板')); + } else { + showError(t('复制失败')); + } };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
web/src/components/model-deployments/DeploymentAccessGuard.jsxweb/src/components/table/model-deployments/DeploymentsActions.jsxweb/src/components/table/model-deployments/index.jsxweb/src/components/table/model-deployments/modals/ColumnSelectorModal.jsxweb/src/components/table/model-deployments/modals/ConfirmationDialog.jsxweb/src/components/table/model-deployments/modals/CreateDeploymentModal.jsxweb/src/components/table/model-deployments/modals/EditDeploymentModal.jsxweb/src/components/table/model-deployments/modals/ExtendDurationModal.jsxweb/src/components/table/model-deployments/modals/UpdateConfigModal.jsxweb/src/components/table/model-deployments/modals/ViewDetailsModal.jsxweb/src/components/table/model-deployments/modals/ViewLogsModal.jsx
🚧 Files skipped from review as they are similar to previous changes (3)
- web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx
- web/src/components/table/model-deployments/index.jsx
- web/src/components/table/model-deployments/modals/EditDeploymentModal.jsx
🧰 Additional context used
🧬 Code graph analysis (4)
web/src/components/table/model-deployments/modals/UpdateConfigModal.jsx (4)
web/src/components/model-deployments/DeploymentAccessGuard.jsx (1)
Typography(26-26)web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (1)
Typography(45-45)web/src/components/table/model-deployments/modals/ViewLogsModal.jsx (1)
Typography(49-49)web/src/helpers/utils.jsx (2)
showSuccess(157-159)showError(122-151)
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (2)
web/src/hooks/model-deployments/useDeploymentResources.js (6)
hardwareTypes(25-25)hardwareTotalAvailable(26-26)locations(27-27)availableReplicas(29-29)priceEstimation(30-30)calculatePrice(164-205)web/src/helpers/utils.jsx (3)
a(259-259)showError(122-151)copy(72-95)
web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
web/src/hooks/model-deployments/useDeploymentsData.jsx (1)
visibleColumns(130-155)
web/src/components/table/model-deployments/modals/ViewLogsModal.jsx (1)
web/src/helpers/utils.jsx (4)
showError(122-151)a(259-259)copy(72-95)timestamp2string(192-218)
🔇 Additional comments (15)
web/src/components/model-deployments/DeploymentAccessGuard.jsx (6)
20-26: LGTM!Imports are well-organized and all dependencies are properly used throughout the component.
28-42: LGTM!Component definition and hooks are properly structured. The navigation handler correctly routes to the settings page with the appropriate tab query parameter.
44-54: LGTM!Loading state is properly implemented with Semi UI's built-in loading animation.
254-264: LGTM!Connection loading state is properly implemented with appropriate conditional logic.
266-372: LGTM!Connection error state is well-implemented with proper error type handling and appropriate user actions. Optional chaining is used correctly for safe property access.
374-377: LGTM!Guard pattern correctly returns children when all checks pass. Clean default export.
web/src/components/table/model-deployments/modals/ViewLogsModal.jsx (1)
245-286: LGTM: Download and copy functions are well-implemented.Both
downloadLogsandcopyAllLogsproperly:
- Check for empty logs before proceeding
- Use
filteredLogsas the source when available- Provide appropriate user feedback
- Handle the clipboard API with fallback in the
copyhelperweb/src/components/table/model-deployments/modals/ExtendDurationModal.jsx (1)
101-203: LGTM: Race condition handling in price calculation is well-implemented.The
calculatePricefunction properly guards against race conditions usingcostRequestIdRef:
- Generates a unique
requestIdfor each request- Checks if the request is still current before updating state
- Only updates loading state if the request hasn't been superseded
This prevents stale price data from appearing when parameters change rapidly.
web/src/components/table/model-deployments/modals/ColumnSelectorModal.jsx (1)
100-100: LGTM: Checkbox onChange correctly uses event signature.The Checkbox onChange handlers properly extract the checked state from
e.target.checked, matching Semi UI's Event-based signature.Also applies to: 114-115
web/src/components/table/model-deployments/modals/UpdateConfigModal.jsx (1)
64-90: LGTM: Form initialization properly handles deployment data.The effect correctly:
- Initializes form values from
deployment.container_config- Joins entrypoint array for display
- Maps env_variables object to key-value pairs array
- Resets when modal visibility or deployment changes
web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (3)
197-244: LGTM: Image mode switching preserves user data correctly.The effect properly handles switching between builtin Ollama and custom image modes:
- Saves custom image/port/env vars before switching to builtin
- Restores saved values when switching back to custom
- Auto-generates Ollama API key for builtin mode
- Uses
prevImageModeRefto detect actual mode changesThis prevents data loss during mode switches.
381-451: LGTM: Race condition handling in location loading.The
loadLocationsfunction properly useslocationRequestIdRefto:
- Generate unique request IDs
- Ignore stale responses
- Only update state for current requests
- Clear state on new requests
Similar pattern is correctly applied in
loadAvailableReplicas.
666-717: Effect properly filters invalid location selections.This effect ensures that selected location IDs remain valid when:
- Hardware changes (locations list updates)
- Replicas data changes (availability updates)
It correctly filters out invalid selections and updates both state and form, preventing invalid submissions.
web/src/components/table/model-deployments/modals/ViewDetailsModal.jsx (2)
93-101: LGTM: Effect properly manages data lifecycle.The effect correctly:
- Fetches details and containers when modal opens
- Guards against missing
deployment?.id- Clears state when modal closes
- Includes both
visibleanddeployment?.idin dependenciesThis prevents stale data and unnecessary fetches.
113-123: Status mapping provides excellent user experience.The
getStatusConfigfunction:
- Maps backend status strings to user-friendly labels
- Provides semantic colors (green for success, red for errors)
- Includes emoji icons for quick visual scanning
- Has a sensible fallback for unknown statuses
* wip ionet integrate * wip ionet integrate * wip ionet integrate * ollama wip * wip * feat: ionet integration & ollama manage * fix merge conflict * wip * fix: test conn cors * wip * fix ionet * fix ionet * wip * fix model select * refactor: Remove `pkg/ionet` test files and update related Go source and web UI model deployment components. * feat: Enhance model deployment UI with styling improvements, updated text, and a new description component. * Revert "feat: Enhance model deployment UI with styling improvements, updated text, and a new description component." This reverts commit 8b75cb5.
2 participants
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.