feat(dashboard): 添加数据看板模型消耗分布显示模式切换功能 #2739
Conversation
- 在模型消耗分布标签页添加 $/Token 切换按钮 - 在数据看板设置中新增默认显示模式配置项 - Token 模式使用数据库 token_used 字段
WalkthroughThe PR adds a display mode feature allowing users to toggle between quota/amount and token views in data dashboard charts. Changes include backend option handling, frontend settings UI with localStorage persistence, component-level display mode toggle, and data processing logic to support both modes with conditional rendering. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 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/pages/Setting/Dashboard/SettingsDataDashboard.jsx (1)
98-105: Bug: localStorage is written with stale state values.The
useEffectwrites to localStorage usinginputs.DataExportDefaultTimeandinputs.DataExportDefaultDisplayMode, but at this pointinputsstill holds the previous state values. The new values fromprops.optionsare set viasetInputs(currentInputs)but React state updates are asynchronous, so the subsequent localStorage writes use stale data.🐛 Proposed fix
useEffect(() => { const currentInputs = {}; for (let key in props.options) { if (Object.keys(inputs).includes(key)) { currentInputs[key] = props.options[key]; } } setInputs(currentInputs); setInputsRow(structuredClone(currentInputs)); refForm.current.setValues(currentInputs); localStorage.setItem( 'data_export_default_time', - String(inputs.DataExportDefaultTime), + String(currentInputs.DataExportDefaultTime || 'hour'), ); localStorage.setItem( 'data_export_default_display_mode', - String(inputs.DataExportDefaultDisplayMode || 'QUOTA'), + String(currentInputs.DataExportDefaultDisplayMode || 'QUOTA'), ); }, [props.options]);
🧹 Nitpick comments (5)
web/src/components/dashboard/ChartsPanel.jsx (1)
50-63: Consider adding accessibility attributes to the toggle.The display mode toggle has a
titlefor hover tooltip but lacks anaria-labelfor screen reader users. Addingrole="button"andaria-labelwould improve accessibility.♿ Suggested improvement
<div className='flex items-center gap-1 bg-[var(--semi-color-fill-0)] rounded px-1.5 py-1 cursor-pointer hover:bg-[var(--semi-color-fill-1)] transition-colors' onClick={() => onDisplayModeChange(displayMode === 'QUOTA' ? 'TOKENS' : 'QUOTA')} title={displayMode === 'QUOTA' ? t('切换为 Token') : t('切换为金额')} + role='button' + aria-label={displayMode === 'QUOTA' ? t('切换为 Token') : t('切换为金额')} + tabIndex={0} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + onDisplayModeChange(displayMode === 'QUOTA' ? 'TOKENS' : 'QUOTA'); + } + }} >web/src/hooks/dashboard/useDashboardCharts.jsx (4)
301-302: Questionable data field assignments for rawQuota and rawTokenUsed.When
displayMode === 'TOKENS', bothrawQuotaandrawTokenUsedare set toaggregated?.tokenUsed || 0. This meansrawQuotaloses the actual quota value, andrawTokenUsedis0when in QUOTA mode. This could cause issues if the display mode is toggled without refetching data, as the underlying raw values would be mode-dependent rather than preserving both values.Consider always storing both raw values:
♻️ Suggested fix
return { Time: time, Model: model, - rawQuota: displayMode === 'TOKENS' ? (aggregated?.tokenUsed || 0) : (aggregated?.quota || 0), - rawTokenUsed: displayMode === 'TOKENS' ? (aggregated?.tokenUsed || 0) : 0, + rawQuota: aggregated?.quota || 0, + rawTokenUsed: aggregated?.tokenUsed || 0, Usage: displayValue, };
307-321: DuplicatevalueFielddeclaration.
valueFieldis declared identically at lines 307 and 321. While valid due to block scoping, this duplicates the mode-switching logic. Consider extracting it to a single declaration at the beginning of the data processing block.♻️ Suggested refactor
+ const valueField = displayMode === 'TOKENS' ? 'rawTokenUsed' : 'rawQuota'; + const renderValue = displayMode === 'TOKENS' ? renderNumber : (v) => renderQuota(v, 4); + chartTimePoints.forEach((time) => { // ... existing code ... }); - const valueField = displayMode === 'TOKENS' ? 'rawTokenUsed' : 'rawQuota'; const timeSum = timeData.reduce((sum, item) => sum + item[valueField], 0); // ... const totalValue = displayMode === 'TOKENS' ? totalTokens : totalQuota; const totalDisplay = displayMode === 'TOKENS' ? renderNumber(totalValue) : renderQuota(totalValue, 2); - // 生成 tooltip 配置 - const renderValue = displayMode === 'TOKENS' ? renderNumber : (v) => renderQuota(v, 4); - const valueField = displayMode === 'TOKENS' ? 'rawTokenUsed' : 'rawQuota'; + // 生成 tooltip 配置 (uses valueField and renderValue from above)
448-500: Duplicated tooltip configuration logic.The tooltip configuration at lines 454-493 is nearly identical to lines 322-361 within
updateChartData. This duplication increases maintenance burden and risk of divergence.Consider extracting the tooltip generation into a shared helper function:
♻️ Suggested refactor
// Extract as a helper function within the hook or in helpers/dashboard.jsx const createTooltipConfig = (displayMode, t) => { const renderValue = displayMode === 'TOKENS' ? renderNumber : (v) => renderQuota(v, 4); const valueField = displayMode === 'TOKENS' ? 'rawTokenUsed' : 'rawQuota'; return { mark: { content: [ { key: (datum) => datum['Model'], value: (datum) => renderValue(datum[valueField] || 0), }, ], }, dimension: { content: [ { key: (datum) => datum['Model'], value: (datum) => datum[valueField] || 0, }, ], updateContent: (array) => { array.sort((a, b) => b.value - a.value); let sum = 0; for (let i = 0; i < array.length; i++) { if (array[i].key == '其他') continue; let value = parseFloat(array[i].value); if (isNaN(value)) value = 0; if (array[i].datum && array[i].datum.TimeSum) { sum = array[i].datum.TimeSum; } array[i].value = renderValue(value); } array.unshift({ key: t('总计'), value: renderValue(sum) }); return array; }, }, }; };
342-344: Use strict equality for string comparison.Line 342 uses
==for comparingarray[i].keywith'其他'. Prefer===for strict equality.- if (array[i].key == '其他') { + if (array[i].key === '其他') {
1 participant
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.