Conversation
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant new feature: a real-time data dashboard. This dashboard provides administrators and users with global view permissions a comprehensive, live overview of system performance and usage. It integrates various data points such as key metrics, active sessions, user and provider rankings, model distribution, and traffic trends into an intuitive, full-screen interface, enhancing operational visibility and monitoring capabilities. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📊 PR Size AnalysisThis PR is XL with 1,422 lines changed across 17 files. Large PRs are harder to review and more likely to introduce bugs. 🔀 Suggested Split:Based on the changes, this PR could be split into:
Why Split?
NoteIf splitting isn't practical due to tight coupling between these changes, consider at least:
🤖 Automated analysis by Claude AI |
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary
This PR implements a real-time data dashboard ("big screen") feature with comprehensive monitoring capabilities. However, there is 1 critical runtime error that will cause the dashboard to fail completely, along with several high and medium priority issues.
🔍 Issues Found
- Critical (🔴): 1 issue
- High (🟠): 2 issues
- Medium (🟡): 3 issues
- Low (🟢): 0 issues
🎯 Priority Actions
- [CRITICAL] Add missing
findDailyModelLeaderboard()function insrc/repository/leaderboard.ts- the dashboard will crash without this - [HIGH] Fix timezone inconsistency in
getOverviewMetrics()- uses client timezone instead of configuredTZenvironment variable - [HIGH] Add error handling for parallel data fetching in
getDashboardRealtimeData()- current implementation will fail silently or crash if any data source fails - [MEDIUM] Fix hardcoded slice limit (20) for activity stream - should be configurable or use constant
- [MEDIUM] Add proper null checks for provider rankings lookup in slot data merging
- [MEDIUM] Avoid parsing numeric data as strings then converting back to numbers in trend data processing
💡 General Observations
Correctness Issues:
- Missing critical function implementation will cause immediate runtime failure
- Timezone handling inconsistency between overview metrics and leaderboard queries
- Insufficient error handling in parallel async operations
Performance Concerns:
- Parallel queries are good, but no timeout or circuit breaker pattern
- Multiple array transformations and lookups could be optimized
Code Quality:
- Good use of TypeScript interfaces and type safety
- Good separation of concerns between data fetching and UI
- I18n support properly implemented across all locales
- Well-structured component hierarchy in the UI layer
Testing Gap: No test files included for new server actions or repository methods
🤖 Automated review by Claude AI - focused on identifying issues for improvement
🔒 Security Scan Results✅ No security vulnerabilities detected This PR has been scanned against OWASP Top 10, CWE Top 25, and common security anti-patterns. No security issues were identified in the code changes. Scanned Categories
🛡️ Security HighlightsStrong Points:
📋 OWASP Top 10 Coverage
✅ Security Posture: STRONGRecommendation: APPROVED from security perspective 🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new real-time data dashboard feature, including backend actions to fetch data and a comprehensive frontend implementation. The changes are well-structured, with new i18n files for localization and efficient data fetching using parallel requests.
My review focuses on correctness and maintainability. I've identified a critical issue in the data processing for the trend chart, which would prevent it from rendering correctly. I've also found a UI bug in the user rankings component that could break the layout, and a couple of medium-severity issues related to code maintainability, such as a very large component file and an inconsistent comment language.
Overall, this is a great feature addition. Addressing the identified issues will improve the robustness and maintainability of the new dashboard.
src/actions/dashboard-realtime.ts
Outdated
| const trendData = | ||
| statisticsResult.ok && statisticsResult.data?.chartData | ||
| ? statisticsResult.data.chartData.map((item) => ({ | ||
| hour: typeof item.hour === "number" ? item.hour : parseInt(String(item.hour), 10), | ||
| value: | ||
| typeof item.requests === "number" | ||
| ? item.requests | ||
| : parseInt(String(item.requests), 10), | ||
| })) | ||
| : Array.from({ length: 24 }, (_, i) => ({ hour: i, value: 0 })); |
There was a problem hiding this comment.
The logic for calculating trendData is incorrect. The item from statisticsResult.data.chartData is of type ChartDataItem, which does not have hour or requests properties. Instead, it has a date property (as an ISO string) and dynamic properties for costs and calls per user/key (e.g., 'user-1_calls'). This will result in trendData being an array of { hour: NaN, value: NaN }, breaking the trend chart on the dashboard.
The logic needs to be updated to correctly parse the hour from the date string and aggregate the total requests by summing up all *_calls properties for each data point.
| const trendData = | |
| statisticsResult.ok && statisticsResult.data?.chartData | |
| ? statisticsResult.data.chartData.map((item) => ({ | |
| hour: typeof item.hour === "number" ? item.hour : parseInt(String(item.hour), 10), | |
| value: | |
| typeof item.requests === "number" | |
| ? item.requests | |
| : parseInt(String(item.requests), 10), | |
| })) | |
| : Array.from({ length: 24 }, (_, i) => ({ hour: i, value: 0 })); | |
| const trendData = | |
| statisticsResult.ok && statisticsResult.data?.chartData | |
| ? statisticsResult.data.chartData.map((item) => { | |
| const hour = new Date(item.date).getUTCHours(); | |
| const value = Object.keys(item) | |
| .filter((key) => key.endsWith("_calls")) | |
| .reduce((sum, key) => sum + (Number(item[key]) || 0), 0); | |
| return { hour, value }; | |
| }) | |
| : Array.from({ length: 24 }, (_, i) => ({ hour: i, value: 0 })); |
src/actions/provider-slots.ts
Outdated
| name: provider.name, | ||
| usedSlots, | ||
| totalSlots: provider.limitConcurrentSessions ?? 0, | ||
| totalVolume: 0, // 会由前端从排行榜数据中填充，或由统一接口提供 |
There was a problem hiding this comment.
This comment is in Chinese, while the rest of the file, including other comments and code, is in English. For consistency and readability for all contributors, comments should be in English.
Additionally, the comment states that the frontend will populate totalVolume, but it's actually the getDashboardRealtimeData server action that orchestrates this. The comment could be clarified.
| totalVolume: 0, // 会由前端从排行榜数据中填充，或由统一接口提供 | |
| totalVolume: 0, // This will be populated by the calling action from leaderboard data. |
| "use client"; | ||
|
|
||
| import React, { useState, useEffect, useRef } from "react"; | ||
| import { | ||
| Activity, | ||
| Server, | ||
| Zap, | ||
| DollarSign, | ||
| Clock, | ||
| AlertTriangle, | ||
| Globe, | ||
| Moon, | ||
| Sun, | ||
| RefreshCw, | ||
| ArrowUp, | ||
| ArrowDown, | ||
| Wifi, | ||
| Layers, | ||
| Shield, | ||
| User, | ||
| PieChart as PieIcon, | ||
| } from "lucide-react"; | ||
| import { | ||
| AreaChart, | ||
| Area, | ||
| XAxis, | ||
| YAxis, | ||
| ResponsiveContainer, | ||
| PieChart, | ||
| Pie, | ||
| Cell, | ||
| Tooltip, | ||
| Legend, | ||
| } from "recharts"; | ||
| import { motion, AnimatePresence } from "framer-motion"; | ||
| import useSWR from "swr"; | ||
| import { useTranslations, useLocale } from "next-intl"; | ||
| import { getDashboardRealtimeData } from "@/actions/dashboard-realtime"; | ||
| import type { DashboardRealtimeData } from "@/actions/dashboard-realtime"; | ||
|
|
||
| /** | ||
| * ============================================================================ | ||
| * 配置与常量 | ||
| * ============================================================================ | ||
| */ | ||
| const COLORS = { | ||
| models: ["#ff6b35", "#00d4ff", "#ffd60a", "#00ff88", "#a855f7"], | ||
| }; | ||
|
|
||
| const THEMES = { | ||
| dark: { | ||
| bg: "bg-[#0a0a0f]", | ||
| text: "text-[#e6e6e6]", | ||
| card: "bg-[#1a1a2e]/60 backdrop-blur-md border border-white/5", | ||
| accent: "text-[#ff6b35]", | ||
| border: "border-white/5", | ||
| }, | ||
| light: { | ||
| bg: "bg-[#fafafa]", | ||
| text: "text-[#1a1a1a]", | ||
| card: "bg-white/80 backdrop-blur-md border border-black/5 shadow-sm", | ||
| accent: "text-[#ff5722]", | ||
| border: "border-black/5", | ||
| }, | ||
| }; | ||
|
|
||
| /** | ||
| * ============================================================================ | ||
| * 实用组件：粒子背景 (Canvas) | ||
| * ============================================================================ | ||
| */ | ||
| const ParticleBackground = ({ themeMode }: { themeMode: string }) => { | ||
| const canvasRef = useRef<HTMLCanvasElement>(null); | ||
|
|
||
| useEffect(() => { | ||
| const canvas = canvasRef.current; | ||
| if (!canvas) return; | ||
|
|
||
| const ctx = canvas.getContext("2d"); | ||
| if (!ctx) return; | ||
|
|
||
| let animationFrameId: number; | ||
| let particles: Array<{ | ||
| x: number; | ||
| y: number; | ||
| vx: number; | ||
| vy: number; | ||
| size: number; | ||
| alpha: number; | ||
| }> = []; | ||
|
|
||
| const resize = () => { | ||
| canvas.width = window.innerWidth; | ||
| canvas.height = window.innerHeight; | ||
| }; | ||
| window.addEventListener("resize", resize); | ||
| resize(); | ||
|
|
||
| const createParticles = () => { | ||
| const count = window.innerWidth < 1000 ? 50 : 80; | ||
| particles = []; | ||
| for (let i = 0; i < count; i++) { | ||
| particles.push({ | ||
| x: Math.random() * canvas.width, | ||
| y: Math.random() * canvas.height, | ||
| vx: (Math.random() - 0.5) * 0.3, | ||
| vy: (Math.random() - 0.5) * 0.3, | ||
| size: Math.random() * 2 + 0.5, | ||
| alpha: Math.random() * 0.4 + 0.1, | ||
| }); | ||
| } | ||
| }; | ||
| createParticles(); | ||
|
|
||
| const render = () => { | ||
| ctx.clearRect(0, 0, canvas.width, canvas.height); | ||
| const particleColor = themeMode === "dark" ? "255, 107, 53" : "2, 119, 189"; | ||
|
|
||
| particles.forEach((p) => { | ||
| p.x += p.vx; | ||
| p.y += p.vy; | ||
|
|
||
| if (p.x < 0) p.x = canvas.width; | ||
| if (p.x > canvas.width) p.x = 0; | ||
| if (p.y < 0) p.y = canvas.height; | ||
| if (p.y > canvas.height) p.y = 0; | ||
|
|
||
| ctx.beginPath(); | ||
| ctx.arc(p.x, p.y, p.size, 0, Math.PI * 2); | ||
| ctx.fillStyle = `rgba(${particleColor}, ${p.alpha})`; | ||
| ctx.fill(); | ||
| }); | ||
|
|
||
| animationFrameId = requestAnimationFrame(render); | ||
| }; | ||
| render(); | ||
|
|
||
| return () => { | ||
| window.removeEventListener("resize", resize); | ||
| cancelAnimationFrame(animationFrameId); | ||
| }; | ||
| }, [themeMode]); | ||
|
|
||
| return ( | ||
| <canvas | ||
| ref={canvasRef} | ||
| className="absolute top-0 left-0 w-full h-full pointer-events-none z-0" | ||
| /> | ||
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * ============================================================================ | ||
| * 实用组件：数字滚动动画 | ||
| * ============================================================================ | ||
| */ | ||
| const CountUp = ({ | ||
| value, | ||
| prefix = "", | ||
| suffix = "", | ||
| decimals = 0, | ||
| className = "", | ||
| }: { | ||
| value: number; | ||
| prefix?: string; | ||
| suffix?: string; | ||
| decimals?: number; | ||
| className?: string; | ||
| }) => { | ||
| const [displayValue, setDisplayValue] = useState(value); | ||
| useEffect(() => { | ||
| const start = displayValue; | ||
| const end = value; | ||
| if (start === end) return; | ||
| const duration = 1000; | ||
| const startTime = performance.now(); | ||
| const animate = (currentTime: number) => { | ||
| const elapsed = currentTime - startTime; | ||
| const progress = Math.min(elapsed / duration, 1); | ||
| const ease = 1 - Math.pow(1 - progress, 4); | ||
| const current = start + (end - start) * ease; | ||
| setDisplayValue(current); | ||
| if (progress < 1) requestAnimationFrame(animate); | ||
| }; | ||
| requestAnimationFrame(animate); | ||
| }, [value, displayValue]); | ||
| return ( | ||
| <span className={`font-mono ${className}`}> | ||
| {prefix} | ||
| {displayValue.toFixed(decimals)} | ||
| {suffix} | ||
| </span> | ||
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * ============================================================================ | ||
| * 核心业务组件 | ||
| * ============================================================================ | ||
| */ | ||
|
|
||
| // 1. 顶部核心指标 | ||
| const MetricCard = ({ | ||
| title, | ||
| value, | ||
| subValue, | ||
| icon: Icon, | ||
| type = "neutral", | ||
| theme, | ||
| }: { | ||
| title: string; | ||
| value: React.ReactNode; | ||
| subValue?: string; | ||
| icon: React.ComponentType<{ size: number; className?: string }>; | ||
| type?: string; | ||
| theme: (typeof THEMES)[keyof typeof THEMES]; | ||
| }) => { | ||
| const isPositive = type === "positive"; | ||
| const isNegative = type === "negative"; | ||
| return ( | ||
| <motion.div | ||
| initial={{ opacity: 0, scale: 0.95 }} | ||
| animate={{ opacity: 1, scale: 1 }} | ||
| className={`relative overflow-hidden rounded-lg p-4 flex flex-col justify-between h-full ${theme.card} hover:border-orange-500/30 transition-colors group`} | ||
| > | ||
| <div className="absolute -top-6 -right-6 w-24 h-24 bg-orange-500/10 rounded-full blur-2xl" /> | ||
| <div className="flex justify-between items-start z-10"> | ||
| <span className={`text-xs uppercase tracking-wider font-semibold ${theme.text} opacity-50`}> | ||
| {title} | ||
| </span> | ||
| <Icon size={16} className={`${theme.accent} opacity-80`} /> | ||
| </div> | ||
| <div className="flex items-end gap-3 mt-1 z-10"> | ||
| <div className="relative"> | ||
| {type === "pulse" && ( | ||
| <span className="absolute -left-2.5 top-1/2 -translate-y-1/2 flex h-1.5 w-1.5"> | ||
| <span className="animate-ping absolute inline-flex h-full w-full rounded-full bg-orange-400 opacity-75"></span> | ||
| <span className="relative inline-flex rounded-full h-1.5 w-1.5 bg-orange-500"></span> | ||
| </span> | ||
| )} | ||
| <span className={`text-3xl font-bold font-mono tracking-tight ${theme.text}`}> | ||
| {value} | ||
| </span> | ||
| </div> | ||
| </div> | ||
| <div className="mt-1 flex items-center text-[10px] font-medium z-10"> | ||
| {subValue && ( | ||
| <span | ||
| className={`flex items-center gap-0.5 ${isPositive ? "text-[#00ff88]" : isNegative ? "text-[#ff006e]" : "text-gray-400"}`} | ||
| > | ||
| {isPositive ? <ArrowUp size={10} /> : isNegative ? <ArrowDown size={10} /> : null} | ||
| {subValue} | ||
| </span> | ||
| )} | ||
| </div> | ||
| </motion.div> | ||
| ); | ||
| }; | ||
|
|
||
| // 2. 实时活动流 | ||
| const ActivityStream = ({ | ||
| activities, | ||
| theme, | ||
| t, | ||
| }: { | ||
| activities: Array<{ | ||
| id: string; | ||
| user: string; | ||
| model: string; | ||
| provider: string; | ||
| latency: number; | ||
| status: number; | ||
| }>; | ||
| theme: (typeof THEMES)[keyof typeof THEMES]; | ||
| t: (key: string) => string; | ||
| }) => { | ||
| return ( | ||
| <div className="h-full flex flex-col"> | ||
| <div | ||
| className={`text-xs font-bold mb-2 flex items-center gap-2 ${theme.text} uppercase tracking-wider px-1`} | ||
| > | ||
| <Zap size={12} className="text-yellow-400" /> | ||
| {t("sections.activity")} | ||
| </div> | ||
| <div className="flex-1 overflow-hidden relative rounded-lg border border-white/5 bg-black/20"> | ||
| <div | ||
| className={`grid grid-cols-12 gap-2 text-[10px] font-mono opacity-50 py-2 px-3 border-b border-white/5 ${theme.text}`} | ||
| > | ||
| <div className="col-span-2">{t("headers.user")}</div> | ||
| <div className="col-span-3">{t("headers.model")}</div> | ||
| <div className="col-span-3">{t("headers.provider")}</div> | ||
| <div className="col-span-2 text-right">{t("headers.latency")}</div> | ||
| <div className="col-span-2 text-right">{t("headers.status")}</div> | ||
| </div> | ||
|
|
||
| <div className="space-y-0.5 relative h-full overflow-y-auto no-scrollbar p-1"> | ||
| <AnimatePresence initial={false}> | ||
| {activities.map((item) => ( | ||
| <motion.div | ||
| key={item.id} | ||
| initial={{ opacity: 0, x: 20, backgroundColor: "rgba(255, 107, 53, 0.2)" }} | ||
| animate={{ opacity: 1, x: 0, backgroundColor: "rgba(255,255,255,0.02)" }} | ||
| exit={{ opacity: 0 }} | ||
| transition={{ duration: 0.3 }} | ||
| className={`grid grid-cols-12 gap-2 p-2 rounded text-[10px] font-mono items-center hover:bg-white/10 transition-colors`} | ||
| > | ||
| <div className={`col-span-2 truncate font-bold text-orange-400`}>{item.user}</div> | ||
| <div className={`col-span-3 truncate text-gray-300`}>{item.model}</div> | ||
| <div className={`col-span-3 truncate text-gray-500`}>{item.provider}</div> | ||
| <div | ||
| className={`col-span-2 text-right ${item.latency > 1000 ? "text-red-400" : "text-green-400"}`} | ||
| > | ||
| {item.latency}ms | ||
| </div> | ||
| <div className="col-span-2 text-right flex justify-end"> | ||
| <span | ||
| className={`px-1.5 rounded-sm ${ | ||
| item.status === 200 | ||
| ? "bg-green-500/10 text-green-500" | ||
| : "bg-red-500/10 text-red-500" | ||
| }`} | ||
| > | ||
| {item.status} | ||
| </span> | ||
| </div> | ||
| </motion.div> | ||
| ))} | ||
| </AnimatePresence> | ||
| </div> | ||
| </div> | ||
| <style jsx>{` | ||
| .no-scrollbar::-webkit-scrollbar { | ||
| display: none; | ||
| } | ||
| .no-scrollbar { | ||
| -ms-overflow-style: none; | ||
| scrollbar-width: none; | ||
| } | ||
| `}</style> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| // 3. 供应商并发插槽 | ||
| const ProviderQuotas = ({ | ||
| providers, | ||
| theme, | ||
| t, | ||
| }: { | ||
| providers: Array<{ | ||
| name: string; | ||
| usedSlots: number; | ||
| totalSlots: number; | ||
| }>; | ||
| theme: (typeof THEMES)[keyof typeof THEMES]; | ||
| t: (key: string) => string; | ||
| }) => { | ||
| return ( | ||
| <div className="h-full flex flex-col"> | ||
| <div | ||
| className={`text-xs font-bold mb-3 flex items-center gap-2 ${theme.text} uppercase tracking-wider`} | ||
| > | ||
| <Server size={12} className="text-blue-400" /> | ||
| {t("sections.providerQuotas")} | ||
| </div> | ||
| <div className="flex-1 flex flex-col justify-around gap-2"> | ||
| {providers.map((p, idx) => { | ||
| const percent = p.totalSlots > 0 ? (p.usedSlots / p.totalSlots) * 100 : 0; | ||
| const isCritical = percent > 90; | ||
| const isWarning = percent > 70; | ||
|
|
||
| return ( | ||
| <div key={idx} className="flex flex-col gap-1"> | ||
| <div className="flex justify-between items-end text-[10px]"> | ||
| <span className={`font-mono font-bold ${theme.text}`}>{p.name}</span> | ||
| <span className={`font-mono ${isCritical ? "text-red-400" : "text-gray-400"}`}> | ||
| {p.usedSlots}/{p.totalSlots} Slots | ||
| </span> | ||
| </div> | ||
| <div className="h-2.5 w-full bg-gray-700/30 rounded-sm overflow-hidden relative flex gap-[1px]"> | ||
| <div | ||
| className={`h-full absolute left-0 top-0 transition-all duration-1000 ease-out ${ | ||
| isCritical | ||
| ? "bg-gradient-to-r from-red-500 to-red-400" | ||
| : isWarning | ||
| ? "bg-gradient-to-r from-yellow-500 to-orange-500" | ||
| : "bg-gradient-to-r from-blue-600 to-cyan-400" | ||
| }`} | ||
| style={{ width: `${percent}%` }} | ||
| /> | ||
| <div className="absolute inset-0 w-full h-full flex"> | ||
| {Array.from({ length: 20 }).map((_, i) => ( | ||
| <div key={i} className="flex-1 border-r border-black/20 h-full" /> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| })} | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| // 4. 用户排行 | ||
| const UserRankings = ({ | ||
| users, | ||
| theme, | ||
| t, | ||
| }: { | ||
| users: Array<{ | ||
| userId: number; | ||
| userName: string; | ||
| totalCost: number; | ||
| totalRequests: number; | ||
| }>; | ||
| theme: (typeof THEMES)[keyof typeof THEMES]; | ||
| t: (key: string) => string; | ||
| }) => { | ||
| return ( | ||
| <div className="h-full flex flex-col relative"> | ||
| <div | ||
| className={`text-xs font-bold mb-3 flex items-center gap-2 ${theme.text} uppercase tracking-wider`} | ||
| > | ||
| <User size={12} className="text-purple-400" /> | ||
| {t("sections.userRank")} | ||
| <span className="ml-auto flex h-2 w-2"> | ||
| <span className="animate-ping absolute inline-flex h-2 w-2 rounded-full bg-green-400 opacity-75"></span> | ||
| <span className="relative inline-flex rounded-full h-2 w-2 bg-green-500"></span> | ||
| </span> | ||
| </div> | ||
|
|
||
| <div className="flex-1 space-y-2 overflow-hidden"> | ||
| {users.slice(0, 5).map((user, index) => ( | ||
| <motion.div | ||
| key={user.userId} | ||
| layout | ||
| transition={{ type: "spring", stiffness: 300, damping: 30 }} | ||
| className={`flex items-center gap-3 p-2 rounded border ${ | ||
| index === 0 | ||
| ? "bg-gradient-to-r from-orange-500/20 to-transparent border-orange-500/30" | ||
| : theme.border + " bg-white/5" | ||
| }`} | ||
| > | ||
| <div | ||
| className={` | ||
| w-6 h-6 rounded flex items-center justify-center text-xs font-bold | ||
| ${ | ||
| index === 0 | ||
| ? "bg-[#ff6b35] text-white shadow-lg shadow-orange-500/50" | ||
| : index === 1 | ||
| ? "bg-gray-400 text-black" | ||
| : index === 2 | ||
| ? "bg-orange-800 text-white" | ||
| : "bg-gray-800 text-gray-400" | ||
| } | ||
| `} | ||
| > | ||
| {index + 1} | ||
| </div> | ||
|
|
||
| <div className="flex-1 min-w-0"> | ||
| <div className="flex justify-between items-center"> | ||
| <span className={`text-xs font-bold truncate ${theme.text}`}>{user.userName}</span> | ||
| <span className="text-[10px] text-gray-500 font-mono"> | ||
| ${user.totalCost.toFixed(2)} | ||
| </span> | ||
| </div> | ||
| <div className="flex justify-between items-center mt-1"> | ||
| <div className="w-16 h-1 bg-gray-700 rounded-full overflow-hidden"> | ||
| <motion.div | ||
| className="h-full bg-purple-500" | ||
| initial={{ width: 0 }} | ||
| animate={{ width: `${(user.totalCost / 100) * 100}%` }} | ||
| /> | ||
| </div> | ||
| <span className="text-[9px] text-gray-500">{user.totalRequests} reqs</span> | ||
| </div> | ||
| </div> | ||
| </motion.div> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| // 5. 供应商排行 | ||
| const ProviderRanking = ({ | ||
| providers, | ||
| theme, | ||
| t, | ||
| }: { | ||
| providers: Array<{ | ||
| providerId: number; | ||
| providerName: string; | ||
| totalTokens: number; | ||
| }>; | ||
| theme: (typeof THEMES)[keyof typeof THEMES]; | ||
| t: (key: string) => string; | ||
| }) => { | ||
| return ( | ||
| <div className="h-full flex flex-col"> | ||
| <div | ||
| className={`text-xs font-bold mb-3 flex items-center gap-2 ${theme.text} uppercase tracking-wider`} | ||
| > | ||
| <Shield size={12} className="text-green-400" /> | ||
| {t("sections.providerRank")} | ||
| </div> | ||
| <div className="flex-1 space-y-2"> | ||
| {providers.slice(0, 5).map((p, i) => ( | ||
| <div | ||
| key={p.providerId} | ||
| className="flex items-center justify-between p-2 rounded bg-white/5 border border-white/5" | ||
| > | ||
| <div className="flex items-center gap-2"> | ||
| <span className="text-[10px] text-gray-500 font-mono w-3">0{i + 1}</span> | ||
| <span className={`text-xs font-semibold ${theme.text}`}>{p.providerName}</span> | ||
| </div> | ||
| <div className="text-right"> | ||
| <div className={`text-xs font-mono ${theme.accent}`}> | ||
| {p.totalTokens.toLocaleString()} | ||
| </div> | ||
| <div className="text-[9px] text-gray-500">Tokens</div> | ||
| </div> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| // 6. 模型分布 | ||
| const ModelDistribution = ({ | ||
| data, | ||
| theme, | ||
| t, | ||
| }: { | ||
| data: Array<{ | ||
| model: string; | ||
| totalRequests: number; | ||
| }>; | ||
| theme: (typeof THEMES)[keyof typeof THEMES]; | ||
| t: (key: string) => string; | ||
| }) => { | ||
| const chartData = data.map((item) => ({ | ||
| name: item.model, | ||
| value: item.totalRequests, | ||
| })); | ||
|
|
||
| return ( | ||
| <div className="h-full flex flex-col"> | ||
| <div | ||
| className={`text-xs font-bold mb-1 flex items-center gap-2 ${theme.text} uppercase tracking-wider`} | ||
| > | ||
| <PieIcon size={12} className="text-indigo-400" /> | ||
| {t("sections.modelDist")} | ||
| </div> | ||
| <div className="flex-1 min-h-0 flex items-center"> | ||
| <ResponsiveContainer width="100%" height="100%"> | ||
| <PieChart> | ||
| <Pie | ||
| data={chartData} | ||
| innerRadius={40} | ||
| outerRadius={60} | ||
| paddingAngle={2} | ||
| dataKey="value" | ||
| stroke="none" | ||
| > | ||
| {chartData.map((_, index) => ( | ||
| <Cell key={`cell-${index}`} fill={COLORS.models[index % COLORS.models.length]} /> | ||
| ))} | ||
| </Pie> | ||
| <Tooltip | ||
| contentStyle={{ backgroundColor: "#0a0a0f", borderColor: "#333", fontSize: "12px" }} | ||
| itemStyle={{ color: "#fff" }} | ||
| /> | ||
| <Legend | ||
| layout="vertical" | ||
| verticalAlign="middle" | ||
| align="right" | ||
| iconSize={8} | ||
| wrapperStyle={{ fontSize: "10px", color: "#999" }} | ||
| /> | ||
| </PieChart> | ||
| </ResponsiveContainer> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * ============================================================================ | ||
| * 主应用 | ||
| * ============================================================================ | ||
| */ | ||
| export default function BigScreenPage() { | ||
| const t = useTranslations("bigScreen"); | ||
| const locale = useLocale(); | ||
| const [themeMode, setThemeMode] = useState("dark"); | ||
| const [currentTime, setCurrentTime] = useState(new Date()); | ||
|
|
||
| const theme = THEMES[themeMode as keyof typeof THEMES]; | ||
|
|
||
| // 时钟 | ||
| useEffect(() => { | ||
| const timer = setInterval(() => setCurrentTime(new Date()), 1000); | ||
| return () => clearInterval(timer); | ||
| }, []); | ||
|
|
||
| // 使用 SWR 获取数据，2秒刷新 | ||
| const { data, error, mutate } = useSWR( | ||
| "dashboard-realtime", | ||
| async () => { | ||
| const result = await getDashboardRealtimeData(); | ||
| if (!result.ok) { | ||
| throw new Error(result.error || "Failed to fetch data"); | ||
| } | ||
| return result.data; | ||
| }, | ||
| { | ||
| refreshInterval: 2000, // 2秒刷新 | ||
| revalidateOnFocus: false, | ||
| } | ||
| ); | ||
|
|
||
| // 处理数据 | ||
| const metrics = data?.metrics || { | ||
| concurrentSessions: 0, | ||
| todayRequests: 0, | ||
| todayCost: 0, | ||
| avgResponseTime: 0, | ||
| todayErrorRate: 0, | ||
| }; | ||
|
|
||
| const activities = (data?.activityStream || []).map((item) => ({ | ||
| id: item.id, | ||
| user: item.user, | ||
| model: item.model, | ||
| provider: item.provider, | ||
| latency: item.latency, | ||
| status: item.status, | ||
| })); | ||
|
|
||
| const users = data?.userRankings || []; | ||
| const providers = data?.providerSlots || []; | ||
| const providerRankings = data?.providerRankings || []; | ||
| const modelDist = data?.modelDistribution || []; | ||
| const trendData = data?.trendData || []; | ||
|
|
||
| return ( | ||
| <div | ||
| className={`relative w-full h-screen overflow-hidden transition-colors duration-500 font-sans selection:bg-orange-500/30 ${theme.bg}`} | ||
| > | ||
| <ParticleBackground themeMode={themeMode} /> | ||
|
|
||
| <div className="relative z-10 flex flex-col h-full p-4 gap-4 max-w-[1920px] mx-auto"> | ||
| {/* Header */} | ||
| <header className="flex justify-between items-center pb-2 border-b border-white/5"> | ||
| <div className="flex flex-col"> | ||
| <h1 className={`text-2xl font-bold tracking-widest font-space ${theme.text}`}> | ||
| {t("title")} | ||
| </h1> | ||
| <p className={`text-[10px] tracking-[0.5em] uppercase opacity-50 ${theme.text} mt-1`}> | ||
| {t("subtitle")} | ||
| </p> | ||
| </div> | ||
|
|
||
| <div className="flex items-center gap-6"> | ||
| <div className={`text-right hidden md:block ${theme.text}`}> | ||
| <div className="text-xl font-mono font-bold tabular-nums"> | ||
| {currentTime.toLocaleTimeString()} | ||
| </div> | ||
| </div> | ||
| <div className="h-6 w-[1px] bg-white/10" /> | ||
| <div className="flex gap-2"> | ||
| <button className={`p-1.5 rounded hover:bg-white/5 ${theme.text}`}> | ||
| <Globe size={18} /> | ||
| </button> | ||
| <button | ||
| onClick={() => setThemeMode(themeMode === "dark" ? "light" : "dark")} | ||
| className={`p-1.5 rounded hover:bg-white/5 ${theme.text}`} | ||
| > | ||
| {themeMode === "dark" ? <Moon size={18} /> : <Sun size={18} />} | ||
| </button> | ||
| <button | ||
| onClick={() => mutate()} | ||
| className={`p-1.5 rounded hover:bg-white/5 ${theme.text}`} | ||
| > | ||
| <RefreshCw size={18} /> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </header> | ||
|
|
||
| {/* Top Metrics Row */} | ||
| <div className="grid grid-cols-6 gap-3 h-[120px]"> | ||
| <MetricCard | ||
| title={t("metrics.concurrent")} | ||
| value={metrics.concurrentSessions} | ||
| subValue="Live" | ||
| type="pulse" | ||
| icon={Wifi} | ||
| theme={theme} | ||
| /> | ||
| <MetricCard | ||
| title={t("metrics.activeSessions")} | ||
| value={activities.length} | ||
| subValue="Stable" | ||
| type="neutral" | ||
| icon={Layers} | ||
| theme={theme} | ||
| /> | ||
| <MetricCard | ||
| title={t("metrics.requests")} | ||
| value={<CountUp value={metrics.todayRequests} />} | ||
| subValue="Today" | ||
| type="positive" | ||
| icon={Activity} | ||
| theme={theme} | ||
| /> | ||
| <MetricCard | ||
| title={t("metrics.cost")} | ||
| value={<CountUp value={metrics.todayCost} prefix="$" decimals={2} />} | ||
| subValue="Budget" | ||
| type="neutral" | ||
| icon={DollarSign} | ||
| theme={theme} | ||
| /> | ||
| <MetricCard | ||
| title={t("metrics.latency")} | ||
| value={`${metrics.avgResponseTime}ms`} | ||
| subValue="Avg" | ||
| type="positive" | ||
| icon={Clock} | ||
| theme={theme} | ||
| /> | ||
| <MetricCard | ||
| title={t("metrics.errorRate")} | ||
| value={`${metrics.todayErrorRate.toFixed(2)}%`} | ||
| subValue={metrics.todayErrorRate > 2 ? "High" : "Normal"} | ||
| type={metrics.todayErrorRate > 2 ? "negative" : "neutral"} | ||
| icon={AlertTriangle} | ||
| theme={theme} | ||
| /> | ||
| </div> | ||
|
|
||
| {/* Main Content Grid */} | ||
| <div className="flex-1 grid grid-cols-12 gap-4 min-h-0"> | ||
| {/* LEFT COL */} | ||
| <div className="col-span-3 flex flex-col gap-4 h-full"> | ||
| <div className={`flex-[3] ${theme.card} rounded-lg p-4 overflow-hidden`}> | ||
| <UserRankings users={users} theme={theme} t={t} /> | ||
| </div> | ||
| <div className={`flex-[2] ${theme.card} rounded-lg p-4 overflow-hidden`}> | ||
| <ProviderRanking providers={providerRankings} theme={theme} t={t} /> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* MIDDLE COL */} | ||
| <div className="col-span-5 flex flex-col gap-4 h-full"> | ||
| <div className={`flex-[2] ${theme.card} rounded-lg p-4`}> | ||
| <ProviderQuotas providers={providers} theme={theme} t={t} /> | ||
| </div> | ||
| <div className={`flex-[2] ${theme.card} rounded-lg p-4`}> | ||
| <ModelDistribution data={modelDist} theme={theme} t={t} /> | ||
| </div> | ||
| <div | ||
| className={`flex-[1] ${theme.card} rounded-lg p-4 relative flex flex-col justify-end`} | ||
| > | ||
| <div | ||
| className={`absolute top-3 left-3 text-[10px] font-bold uppercase tracking-wider ${theme.text} opacity-50`} | ||
| > | ||
| {t("sections.requestTrend")} | ||
| </div> | ||
| <div className="h-16 w-full"> | ||
| <ResponsiveContainer width="100%" height="100%"> | ||
| <AreaChart data={trendData}> | ||
| <defs> | ||
| <linearGradient id="grad1" x1="0" y1="0" x2="0" y2="1"> | ||
| <stop offset="0%" stopColor="#ff6b35" stopOpacity={0.2} /> | ||
| <stop offset="100%" stopColor="#ff6b35" stopOpacity={0} /> | ||
| </linearGradient> | ||
| </defs> | ||
| <Area | ||
| type="monotone" | ||
| dataKey="value" | ||
| stroke="#ff6b35" | ||
| fill="url(#grad1)" | ||
| strokeWidth={2} | ||
| /> | ||
| </AreaChart> | ||
| </ResponsiveContainer> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* RIGHT COL */} | ||
| <div className="col-span-4 h-full"> | ||
| <div className={`h-full ${theme.card} rounded-lg p-0 overflow-hidden flex flex-col`}> | ||
| <div className="p-3 pb-0"> | ||
| <ActivityStream activities={activities} theme={theme} t={t} /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Footer */} | ||
| <footer | ||
| className={`h-6 flex items-center justify-between text-[9px] uppercase tracking-wider opacity-40 ${theme.text}`} | ||
| > | ||
| <span>{t("status.normal")}</span> | ||
| <span> | ||
| {t("status.lastUpdate")}: {error ? "Error" : "2s ago"} | ||
| </span> | ||
| </footer> | ||
| </div> | ||
|
|
||
| <style jsx global>{` | ||
| @import url("https://fonts.googleapis.com/css2?family=JetBrains+Mono:wght@400;700&family=Space+Grotesk:wght@400;600;700&display=swap"); | ||
| .font-mono { | ||
| font-family: "JetBrains Mono", monospace; | ||
| } | ||
| .font-space { | ||
| font-family: "Space Grotesk", sans-serif; | ||
| } | ||
| `}</style> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
This file is over 800 lines long and contains multiple distinct React components (ParticleBackground, CountUp, MetricCard, ActivityStream, ProviderQuotas, UserRankings, ProviderRanking, ModelDistribution) in addition to the main page component. This makes the file difficult to read, navigate, and maintain.
For better code organization and maintainability, it is highly recommended to extract each of these components into its own file within a components directory (e.g., src/app/[locale]/internal/dashboard/big-screen/components/).
This will make the main page component much cleaner and easier to understand, as it will focus on layout and data flow, while the individual components will encapsulate their own logic and presentation.
| <motion.div | ||
| className="h-full bg-purple-500" | ||
| initial={{ width: 0 }} | ||
| animate={{ width: `${(user.totalCost / 100) * 100}%` }} |
There was a problem hiding this comment.
The progress bar width in the UserRankings component is calculated as width: ${(user.totalCost / 100) * 100}%. This simplifies to `width: `${user.totalCost}%. Since user.totalCost is an absolute dollar value, this will cause the UI to break for any user with a cost over $100. The width should be normalized relative to the top user's cost to represent a proper progress bar.
| animate={{ width: `${(user.totalCost / 100) * 100}%` }} | |
| animate={{ width: `${(user.totalCost / (users[0]?.totalCost || 1)) * 100}%` }} |
🔍 Detailed Code Review FindingsI've identified 6 significant issues that should be addressed before merging this PR. 🔴 CRITICAL ISSUEsrc/actions/dashboard-realtime.ts, Line 122Missing function causes runtime error Why this is a problem: The code imports and calls Suggested fix: Add the complete function to export async function findDailyModelLeaderboard(): Promise<ModelLeaderboardEntry[]> {
const timezone = getEnvConfig().TZ;
const rankings = await db
.select({
model: messageRequest.model,
totalRequests: sql<number>\`count(*)::double precision\`,
totalCost: sql<string>\`COALESCE(sum(\${messageRequest.costUsd}), 0)\`,
totalTokens: sql<number>\`COALESCE(
sum(
\${messageRequest.inputTokens} +
\${messageRequest.outputTokens} +
COALESCE(\${messageRequest.cacheCreationInputTokens}, 0) +
COALESCE(\${messageRequest.cacheReadInputTokens}, 0)
)::double precision,
0::double precision
)\`,
successRate: sql<number>\`COALESCE(
count(CASE WHEN \${messageRequest.errorMessage} IS NULL OR \${messageRequest.errorMessage} = '' THEN 1 END)::double precision
/ NULLIF(count(*)::double precision, 0),
0::double precision
)\`,
})
.from(messageRequest)
.where(
and(
isNull(messageRequest.deletedAt),
sql\`(\${messageRequest.createdAt} AT TIME ZONE \${timezone})::date = (CURRENT_TIMESTAMP AT TIME ZONE \${timezone})::date\`
)
)
.groupBy(messageRequest.model)
.orderBy(desc(sql\`count(*)\`));
return rankings.map((entry) => ({
model: entry.model ?? 'Unknown',
totalRequests: entry.totalRequests,
totalCost: parseFloat(entry.totalCost),
totalTokens: entry.totalTokens,
successRate: entry.successRate ?? 0,
}));
}🟠 HIGH PRIORITY ISSUES1. src/repository/overview.ts, Lines 25-28Timezone inconsistency causes incorrect "today" boundary Why this is a problem: This function uses JavaScript's
Suggested fix: export async function getOverviewMetrics(): Promise<OverviewMetrics> {
const timezone = getEnvConfig().TZ;
const [result] = await db
.select({
requestCount: count(),
totalCost: sum(messageRequest.costUsd),
avgDuration: avg(messageRequest.durationMs),
errorCount: sum(sql<number>\`CASE WHEN \${messageRequest.statusCode} >= 400 THEN 1 ELSE 0 END\`),
})
.from(messageRequest)
.where(
and(
isNull(messageRequest.deletedAt),
sql\`(\${messageRequest.createdAt} AT TIME ZONE \${timezone})::date = (CURRENT_TIMESTAMP AT TIME ZONE \${timezone})::date\`
)
);
// ... rest of the function remains the same
}2. src/actions/dashboard-realtime.ts, Lines 118-126Insufficient error handling for parallel data fetching Why this is a problem: The code uses Suggested fix: Use const [
overviewResult,
activeSessionsResult,
userRankingsResult,
providerRankingsResult,
providerSlotsResult,
modelRankingsResult,
statisticsResult,
] = await Promise.allSettled([
getOverviewData(),
getActiveSessions(),
findDailyLeaderboard(),
findDailyProviderLeaderboard(),
getProviderSlots(),
findDailyModelLeaderboard(),
getUserStatistics("today"),
]);
// Extract data with fallbacks
const overviewData = overviewResult.status === 'fulfilled' && overviewResult.value.ok
? overviewResult.value.data
: null;
if (\!overviewData) {
logger.error("Failed to get overview data", {
reason: overviewResult.status === 'rejected' ? overviewResult.reason : overviewResult.value.error
});
return { ok: false, error: "获取概览数据失败" };
}
const userRankings = userRankingsResult.status === 'fulfilled' ? userRankingsResult.value : [];
const providerRankings = providerRankingsResult.status === 'fulfilled' ? providerRankingsResult.value : [];
// ... etc for other data sources with fallbacks🟡 MEDIUM PRIORITY ISSUES3. src/actions/dashboard-realtime.ts, Line 147Magic number for activity stream limit should be a constant Why this is a problem: The hardcoded Suggested fix: // At the top of the file
const ACTIVITY_STREAM_LIMIT = 20;
const MODEL_DISTRIBUTION_LIMIT = 10;
// Line 147
const activityStream: ActivityStreamEntry[] = activeSessionsResult.ok
? activeSessionsResult.data.slice(0, ACTIVITY_STREAM_LIMIT).map((session) => ({
// ...
}))
: [];4. src/actions/dashboard-realtime.ts, Lines 158-164Provider rankings lookup lacks explicit null handling Why this is a problem: The code uses Suggested fix: Add logging for better observability: const providerSlots: ProviderSlotInfo[] = providerSlotsResult.ok
? providerSlotsResult.data.map((slot) => {
const rankingData = providerRankings.find((p) => p.providerId === slot.providerId);
if (\!rankingData) {
logger.debug("Provider has slots but no traffic", {
providerId: slot.providerId,
providerName: slot.name
});
}
return {
...slot,
totalVolume: rankingData?.totalTokens ?? 0,
};
})
: [];5. src/actions/dashboard-realtime.ts, Lines 167-175Inefficient data transformation with redundant type conversions Why this is a problem: The code uses
The Suggested fix: Simplify using const trendData =
statisticsResult.ok && statisticsResult.data?.chartData
? statisticsResult.data.chartData.map((item) => ({
hour: Number(item.hour) || 0,
value: Number(item.requests) || 0,
}))
: Array.from({ length: 24 }, (_, i) => ({ hour: i, value: 0 }));6. src/repository/overview.ts, Line 37SQL aggregation for error count can be simplified Recommendation: The current errorCount: sql<number>\`count(*) FILTER (WHERE \${messageRequest.statusCode} >= 400)\`,This is more readable and aligns with SQL:2003 standard. |
CRITICAL fixes: - findDailyModelLeaderboard() already exists in leaderboard.ts (verified) HIGH priority fixes: - Fix timezone inconsistency in getOverviewMetrics() - use SQL AT TIME ZONE - Add Promise.allSettled for robust parallel data fetching with fallbacks - Add comprehensive error logging for partial failures MEDIUM priority fixes: - Replace magic numbers with constants (ACTIVITY_STREAM_LIMIT, MODEL_DISTRIBUTION_LIMIT) - Add logging for provider rankings lookup with null checks - Correct trend data calculation from ChartDataItem structure - Simplify error count SQL using FILTER syntax Code quality improvements: - Translate Chinese comment to English in provider-slots.ts - Fix user ranking progress bar normalization (relative to top user) - Remove unused imports (XAxis, YAxis, locale, DashboardRealtimeData) - Fix TypeScript type errors in error handling paths All type checks pass ✅
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary
This PR adds a real-time data dashboard ("big screen") feature with comprehensive monitoring capabilities. The implementation includes parallel data aggregation, real-time updates via SWR, and a polished UI with animations. However, there are several medium-to-critical issues that should be addressed before merge.
🔍 Issues Found
- Critical (🔴): 1 issue
- High (🟠): 4 issues
- Medium (🟡): 5 issues
- Low (🟢): 1 issue
🎯 Priority Actions
- Fix division by zero in UserRankings component (line 471) - Could cause UI corruption with NaN/Infinity width values
- Add Error Boundary protection - Critical for production monitoring dashboards to prevent white-screen crashes
- Improve error handling for SWR failures - Users need prominent warnings when data fetching fails
- Optimize N+1 Redis queries in provider-slots.ts - Use batch operations for better performance
- Fix external font loading - Use Next.js font optimization instead of blocking @import
💡 General Observations
Strengths:
- Good use of Promise.allSettled for partial failure tolerance
- Proper permission checks across all actions
- Comprehensive logging and debug statements
Concerns:
- Missing error boundaries for production resilience
- Some performance optimization opportunities (N+1 queries, redundant slicing)
- TypeScript type safety could be improved in dynamic property access
- External resource loading not optimized for Next.js
🤖 Automated review by Claude AI - focused on identifying issues for improvement
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
File: src/app/[locale]/internal/dashboard/big-screen/page.tsx:471
🔴 Critical: Potential Division by Zero
Why this is a problem: This line calculates progress bar width using users[0]?.totalCost as the denominator. If the first user has totalCost: 0, this causes division by zero resulting in Infinity or NaN. The fallback to || 1 only works when users[0] is undefined/null, not when totalCost === 0.
Suggested fix:
// Current (buggy):
animate={{ width: \`\${(user.totalCost / (users[0]?.totalCost || 1)) * 100}%\` }}
// Fixed:
animate={{ width: \`\${users[0]?.totalCost ? (user.totalCost / users[0].totalCost) * 100 : 0}%\` }}
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
File: src/app/[locale]/internal/dashboard/big-screen/page.tsx:1-825
🟠 High: No Error Boundary Protection
Why this is a problem: The big-screen dashboard is a complex component with Canvas rendering, animations, and real-time data fetching. If any component throws an error (e.g., Canvas API failure, malformed data), the entire page crashes with a white screen. For a production monitoring dashboard, this is unacceptable - users need clear error messages, not browser confusion.
Suggested fix: Wrap the component with an Error Boundary:
// Create: src/app/[locale]/internal/dashboard/big-screen/error-boundary.tsx
'use client';
import React from 'react';
export class DashboardErrorBoundary extends React.Component<
{ children: React.ReactNode },
{ hasError: boolean; error?: Error }
> {
constructor(props: { children: React.ReactNode }) {
super(props);
this.state = { hasError: false };
}
static getDerivedStateFromError(error: Error) {
return { hasError: true, error };
}
render() {
if (this.state.hasError) {
return (
<div className="flex items-center justify-center h-screen bg-[#0a0a0f] text-white">
<div className="text-center">
<h1 className="text-2xl font-bold mb-4">Dashboard Error</h1>
<p className="text-gray-400">{this.state.error?.message}</p>
<button onClick={() => window.location.reload()} className="mt-4 px-4 py-2 bg-orange-500 rounded">
Reload
</button>
</div>
</div>
);
}
return this.props.children;
}
}
// In layout.tsx: wrap {children} with <DashboardErrorBoundary>
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
File: src/app/[locale]/internal/dashboard/big-screen/page.tsx:607-620
🟠 High: Silent Data Fetching Failures
Why this is a problem: When getDashboardRealtimeData() throws an error, SWR sets the error state, but the only indication is tiny footer text (error ? "Error" : "2s ago"). Users won't notice data is stale or failing. The dashboard continues showing old cached data without any prominent warning.
Suggested fix:
// Add prominent error banner
{error && (
<div className="fixed top-4 right-4 bg-red-500/90 text-white px-4 py-2 rounded-lg shadow-lg z-50 flex items-center gap-2">
<AlertTriangle size={16} />
<span className="text-sm font-semibold">Data fetch failed. Showing cached data.</span>
</div>
)}
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
File: src/app/[locale]/internal/dashboard/big-screen/page.tsx:815
🟡 Medium: Blocking External Font Resource
Why this is a problem: Loading Google Fonts via @import in inline styles blocks rendering and is an anti-pattern in Next.js. This causes Flash of Unstyled Text (FOUT) and slower initial page load. For a real-time dashboard needing instant rendering, this is a performance issue.
Suggested fix: Use Next.js font optimization:
// Create fonts.ts:
import { JetBrains_Mono, Space_Grotesk } from 'next/font/google';
export const jetbrainsMono = JetBrains_Mono({
subsets: ['latin'],
weight: ['400', '700'],
variable: '--font-jetbrains',
display: 'swap',
});
export const spaceGrotesk = Space_Grotesk({
subsets: ['latin'],
weight: ['400', '600', '700'],
variable: '--font-space',
display: 'swap',
});
// In page.tsx:
import { jetbrainsMono, spaceGrotesk } from './fonts';
// Add to className: \${jetbrainsMono.variable} \${spaceGrotesk.variable}
// Update CSS: .font-mono -> font-[var(--font-jetbrains)]
// Remove the <style jsx global> block entirely
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
File: src/app/[locale]/internal/dashboard/big-screen/page.tsx:736-737
🟡 Medium: Hardcoded Error Rate Threshold
Why this is a problem: The error rate threshold 2 is hardcoded in two places. This is a business logic constant that should be extracted to make it easier to modify and maintain. If the acceptable error rate changes, developers need to update multiple locations, risking inconsistency.
Suggested fix:
// At top of file with other constants (after line 45):
const ERROR_RATE_THRESHOLD = 2; // percentage
// Replace lines 736-737:
subValue={metrics.todayErrorRate > ERROR_RATE_THRESHOLD ? "High" : "Normal"}
type={metrics.todayErrorRate > ERROR_RATE_THRESHOLD ? "negative" : "neutral"}
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
File: src/actions/provider-slots.ts:71-85
🟡 Medium: N+1 Query Pattern
Why this is a problem: Lines 71-85 execute SessionTracker.getProviderSessionCount() in a loop via Promise.all. If there are 50 providers, this makes 50 Redis calls. While parallel execution helps, this is a classic N+1 query pattern that could be optimized with a single batch operation.
Suggested fix: Add batch method to SessionTracker:
// In session-tracker.ts, add batch method:
static async getMultipleProviderSessionCounts(providerIds: number[]): Promise<Map<number, number>> {
const redis = getRedisClient();
if (!redis) return new Map();
const pipeline = redis.pipeline();
providerIds.forEach(id => {
pipeline.zcard(\`provider:\${id}:active_sessions\`);
});
const results = await pipeline.exec();
const counts = new Map<number, number>();
providerIds.forEach((id, index) => {
counts.set(id, Number(results?.[index]?.[1] || 0));
});
return counts;
}
// In provider-slots.ts:
const providerIds = providerList.map(p => p.id);
const sessionCounts = await SessionTracker.getMultipleProviderSessionCounts(providerIds);
const slotInfoList = providerList.map(provider => ({
providerId: provider.id,
name: provider.name,
usedSlots: sessionCounts.get(provider.id) ?? 0,
totalSlots: provider.limitConcurrentSessions ?? 0,
totalVolume: 0,
}));
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
File: src/actions/dashboard-realtime.ts:242-244
🟡 Medium: Unsafe Dynamic Property Access
Why this is a problem: Line 244 uses item[key] where item is from statisticsData.chartData, but the code assumes all keys ending in _calls are numbers. If the data structure changes or contains unexpected keys, this could silently return undefined or cause runtime errors. TypeScript doesn't catch this due to dynamic property access.
Suggested fix:
// Add type guard and null check:
const value = Object.keys(item)
.filter((key) => key.endsWith("_calls"))
.reduce((sum, key) => {
const val = item[key as keyof typeof item];
return sum + (typeof val === 'number' ? val : 0);
}, 0);
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
File: src/actions/dashboard-realtime.ts:207-208, 262-265
🟡 Medium: Inefficient Data Slicing
Why this is a problem: The code slices activeSessions to ACTIVITY_STREAM_LIMIT (20 items) on line 208, then maps over all sliced items. If getActiveSessions() returns 1000+ items, you're loading and processing all items only to keep 20. This wastes memory and CPU cycles.
Suggested fix: Add limit parameter to getActiveSessions:
// In active-sessions.ts, modify signature:
export async function getActiveSessions(limit?: number): Promise<ActionResult<ActiveSessionInfo[]>> {
// ... existing code ...
const sessions = await getSessionsFromRedis();
return {
ok: true,
data: limit ? sessions.slice(0, limit) : sessions,
};
}
// In dashboard-realtime.ts line 125:
getActiveSessions(ACTIVITY_STREAM_LIMIT),
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
File: src/actions/dashboard-realtime.ts:212-217
🟢 Low: Inconsistent Null Handling Style
Why this is a problem: The code uses different patterns for null handling: || "Unknown" (lines 212-213) vs || 0 (line 214) vs || 200 (line 215) vs parseFloat(...|| "0") (line 216). While functionally correct, this inconsistency makes code harder to maintain and review.
Suggested fix: Use nullish coalescing operator ?? consistently:
model: session.model ?? "Unknown",
provider: session.providerName ?? "Unknown",
latency: session.durationMs ?? 0,
status: session.statusCode ?? 200,
cost: parseFloat(session.costUsd ?? "0"),
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
File: src/actions/dashboard-realtime.ts:174, 186
🟠 High: Inconsistent Empty Data Handling
Why this is a problem: Line 174 logs a warning when activeSessions is empty (\!activeSessions.length), but line 186 does NOT log a warning when providerSlots is empty, only when the action failed. This inconsistency means one data source triggers warnings for legitimate empty states while another doesn't. Empty data is not necessarily an error - it could be valid (e.g., no active sessions at 3 AM).
Suggested fix: Be consistent - either warn for all empty states or none:
// Option 1: Remove empty data warnings (recommended - empty is often valid):
if (activeSessionsResult.status === "rejected") {
logger.warn("Failed to get active sessions", { reason: activeSessionsResult.reason });
}
// Option 2: Add empty warnings for all data sources consistently:
if (activeSessionsResult.status === "rejected" || \!activeSessions.length) {
logger.debug("Active sessions empty or failed", { ... }); // Use debug, not warn
}
if (providerSlotsResult.status === "rejected" || \!providerSlots.length) {
logger.debug("Provider slots empty or failed", { ... });
}✨ 新增功能 - 实现 Live Stream 混合数据源 (Redis + Database) - 优化 Traffic Trend 图表展示和实时截断 - 添加语言循环切换功能 🐛 Bug 修复 - 修复 Model Distribution 数据为空问题 (COALESCE originalModel) - 修复进行中请求显示 0ms 耗时问题 - 修复语言切换器无响应问题 🎨 UI/UX 优化 - Traffic Trend 提升至中间位置，突出重要性 - 添加坐标轴标签 (XAxis: 小时, YAxis: 请求数) - 实时截断未来时间数据点 (避免误导性 0 值) - Provider Slot 按占用率排序，最多显示 3 个 - 供应商排行改为按金额降序排序 - 合并 CONCURRENCY 和 ACTIVE SESSIONS 指标 - 过滤未设置并发限额的供应商 📦 数据层改进 - 新增 activity-stream.ts repository - 使用窗口函数优化 session 查询 - 修复 Model Distribution 查询 NULL 处理 - 实时耗时计算 (durationMs ?? now - startTime) 🔧 技术优化 - Live Stream 最多 20 条 (活跃 + 历史混合) - Traffic Trend 2 秒自动刷新 - 语言切换支持 5 种语言循环 - 所有修改通过 TypeScript 类型检查
🔒 Security Scan Results✅ No security vulnerabilities detected This PR has been scanned against OWASP Top 10, CWE Top 25, and common security anti-patterns. No security issues were identified in the code changes. Scanned Categories
Key Security Features Verified✅ SQL Injection Prevention
✅ Authorization Controls
✅ Data Sanitization
✅ Client-Side Security
✅ Error Handling
✅ Data Integrity
Code Quality Observations (Non-Security)The following are not security vulnerabilities but good coding practices observed:
🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage |
| .orderBy(providers.priority, providers.id); | ||
|
|
||
| // 并行获取每个供应商的并发数 | ||
| const slotInfoList = await Promise.all( |
There was a problem hiding this comment.
🟠 High: Potential N+1 query in provider slots retrieval
Why this is a problem: Line 71-85 uses Promise.all to query Redis session counts for each provider individually. This creates N Redis queries (one per provider) instead of a single batch operation, causing performance degradation when there are many providers.
Suggested fix:
// Add a batch method to SessionTracker
// In SessionTracker class:
static async getMultipleProviderSessionCounts(providerIds: number[]): Promise<Map<number, number>> {
const pipeline = redis.pipeline();
providerIds.forEach(id => pipeline.scard(`provider:${id}:sessions`));
const results = await pipeline.exec();
const countMap = new Map<number, number>();
providerIds.forEach((id, i) => {
countMap.set(id, (results?.[i]?.[1] as number) ?? 0);
});
return countMap;
}
// Then use it:
const providerIds = providerList.map(p => p.id);
const sessionCounts = await SessionTracker.getMultipleProviderSessionCounts(providerIds);
const slotInfoList = providerList.map(provider => ({
providerId: provider.id,
name: provider.name,
usedSlots: sessionCounts.get(provider.id) ?? 0,
totalSlots: provider.limitConcurrentSessions ?? 0,
totalVolume: 0,
}));
| * | ||
| * @param limit 最大返回条数（默认 20） | ||
| */ | ||
| export async function findRecentActivityStream(limit = 20): Promise<ActivityStreamItem[]> { |
There was a problem hiding this comment.
🟠 High: Missing input validation for limit parameter
Why this is a problem: The limit parameter has no validation. A malicious or buggy caller could pass negative numbers, zero, or extremely large values, causing database performance issues or unexpected behavior.
Suggested fix:
export async function findRecentActivityStream(limit = 20): Promise<ActivityStreamItem[]> {
// Validate and sanitize limit
const sanitizedLimit = Math.max(1, Math.min(limit, 100)); // Clamp between 1 and 100
try {
// ... rest of code using sanitizedLimit instead of limit
const activeSessionRequests = await db
.select({
// ...
})
// ...
.limit(sanitizedLimit * 2);
// ...
}
}
| } | ||
|
|
||
| // 4. 按创建时间降序排序并去重 | ||
| const uniqueItems = new Map<number, ActivityStreamItem>(); |
There was a problem hiding this comment.
🟡 Medium: Inefficient deduplication using Map
Why this is a problem: Lines 208-213 use a Map for deduplication, but the code already fetches distinct data (active sessions + non-overlapping recent requests). The deduplication adds unnecessary overhead and could mask logic bugs where duplicates shouldn't exist.
Suggested fix:
// Remove unnecessary deduplication since queries already ensure uniqueness
// Active sessions query: one record per session (filtered by rowNum = 1)
// Recent requests query: explicitly excludes active session IDs
// These two sets are mutually exclusive, so no duplicates possible
const sortedItems = activityItems
.sort((a, b) => b.startTime - a.startTime)
.slice(0, limit);
// Optional: Add assertion to catch logic bugs in development
if (process.env.NODE_ENV === 'development') {
const ids = new Set(activityItems.map(item => item.id));
if (ids.size !== activityItems.length) {
logger.warn('[ActivityStream] Unexpected duplicates detected', {
totalItems: activityItems.length,
uniqueIds: ids.size,
});
}
}
| * ============================================================================ | ||
| * 实用组件：粒子背景 (Canvas) | ||
| * ============================================================================ | ||
| */ |
There was a problem hiding this comment.
🟡 Medium: Potential memory leak in client-side component
Why this is a problem: The ParticleBackground component creates animation frames but the cleanup in useEffect may not cancel all pending frames if multiple resize events occur rapidly. This could cause memory leaks and performance degradation over time.
Suggested fix:
useEffect(() => {
const canvas = canvasRef.current;
if (\!canvas) return;
const ctx = canvas.getContext('2d');
if (\!ctx) return;
let animationFrameId: number | null = null;
let isCleanedUp = false;
const resize = () => {
canvas.width = window.innerWidth;
canvas.height = window.innerHeight;
};
const render = () => {
if (isCleanedUp) return; // Guard against cleanup race conditions
ctx.clearRect(0, 0, canvas.width, canvas.height);
// ... particle rendering logic
animationFrameId = requestAnimationFrame(render);
};
window.addEventListener('resize', resize);
resize();
createParticles();
render();
return () => {
isCleanedUp = true;
window.removeEventListener('resize', resize);
if (animationFrameId \!== null) {
cancelAnimationFrame(animationFrameId);
}
};
}, [themeMode]);
| }); | ||
|
|
||
| // 处理供应商插槽数据（合并流量数据 + 过滤未设置限额 + 按占用率排序 + 限制最多3个） | ||
| const providerSlotsWithVolume: ProviderSlotInfo[] = providerSlots |
There was a problem hiding this comment.
🟠 High: Database query inside map iterator
Why this is a problem: Line 229 performs a .find() operation for each provider slot in the map. This is an O(n*m) operation where n is the number of provider slots and m is the number of provider rankings. For large datasets, this becomes a performance bottleneck.
Suggested fix:
// Pre-build a lookup map for O(1) access
const providerRankingMap = new Map(
providerRankings.map(p => [p.providerId, p])
);
const providerSlotsWithVolume: ProviderSlotInfo[] = providerSlots
.filter((slot) => slot.totalSlots > 0)
.map((slot) => {
const rankingData = providerRankingMap.get(slot.providerId);
if (!rankingData) {
logger.debug("Provider has slots but no traffic", {
providerId: slot.providerId,
providerName: slot.name,
});
}
return {
...slot,
totalVolume: rankingData?.totalTokens ?? 0,
};
})
// ... rest of the chain
| ? statisticsResult.value.data | ||
| : null; | ||
|
|
||
| // 记录部分失败的数据源 |
There was a problem hiding this comment.
🟡 Medium: Inconsistent error handling for PromiseSettledResult
Why this is a problem: The code logs warnings for partial failures but doesn't handle the root causes. For example, activityStreamResult could fail silently and return an empty array, masking Redis/database connection issues that should be escalated.
Suggested fix:
// Add severity levels and escalation for critical failures
const failures = {
critical: [] as string[],
warning: [] as string[],
};
if (activityStreamResult.status === "rejected") {
failures.critical.push(`activity_stream: ${activityStreamResult.reason}`);
logger.error("Critical: Failed to get activity stream", {
reason: activityStreamResult.reason,
stack: activityStreamResult.reason?.stack,
});
}
// ... similar for other critical data sources
// If we have critical failures, include them in the response
if (failures.critical.length > 0) {
logger.error("Dashboard has critical data source failures", {
userId: session.user.id,
failures: failures.critical,
});
// Optional: Return partial failure status
// return { ok: false, error: 'Partial data unavailable', details: failures };
}
| and(isNull(messageRequest.deletedAt), inArray(messageRequest.sessionId, activeSessionIds)) | ||
| ) | ||
| .orderBy(desc(messageRequest.createdAt)) | ||
| .limit(limit * 2); // 获取足够的数据，后面会过滤 |
There was a problem hiding this comment.
🟡 Medium: Hardcoded magic numbers without constants
Why this is a problem: Lines 103 and 176 use hardcoded numbers (limit * 2 and remaining) without explanation. The * 2 multiplier is particularly problematic - why 2? This makes the code harder to maintain and could lead to over-fetching.
Suggested fix:
// Add explanatory constants at the top of the file
const WINDOW_FUNCTION_OVERFETCH_MULTIPLIER = 2; // Fetch 2x limit to account for filtering by rowNum
const MAX_ACTIVITY_STREAM_OVERFETCH = 100; // Cap to prevent excessive DB load
export async function findRecentActivityStream(limit = 20): Promise<ActivityStreamItem[]> {
const sanitizedLimit = Math.max(1, Math.min(limit, 100));
try {
// ...
if (activeSessionIds.length > 0) {
const fetchLimit = Math.min(
sanitizedLimit * WINDOW_FUNCTION_OVERFETCH_MULTIPLIER,
MAX_ACTIVITY_STREAM_OVERFETCH
);
const activeSessionRequests = await db
.select({ /* ... */ })
.limit(fetchLimit); // More explicit than `limit * 2`
// ...
}
}
}
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary
This PR adds a comprehensive realtime data dashboard feature with multi-source data aggregation, internationalization support, and visual monitoring capabilities. The implementation is largely well-structured with good use of Promise.allSettled for fault tolerance, but contains several performance and reliability issues that should be addressed.
🔍 Issues Found
- Critical (🔴): 0 issues
- High (🟠): 3 issues
- Medium (🟡): 4 issues
- Low (🟢): 0 issues
🎯 Priority Actions
-
Fix N+1 Redis queries in provider slots - Implement batch Redis operations using pipelining to avoid sequential queries for each provider (src/actions/provider-slots.ts:71)
-
Add input validation for limit parameter - Prevent negative/excessive values that could cause database performance degradation (src/repository/activity-stream.ts:62)
-
Optimize nested loops with Map lookups - Replace O(n*m) .find() operations with O(1) Map.get() for provider ranking lookups (src/actions/dashboard-realtime.ts:226)
-
Improve error handling escalation - Add severity levels to distinguish critical failures from warnings, preventing silent data loss (src/actions/dashboard-realtime.ts:172)
-
Fix potential memory leak in canvas animation - Add proper cleanup guards and animation frame cancellation (src/app/[locale]/internal/dashboard/big-screen/page.tsx:73)
💡 General Observations
Strengths:
- Good use of Promise.allSettled for graceful degradation
- Comprehensive internationalization coverage (5 locales)
- Well-documented TypeScript interfaces
- Proper permission checks before data access
Patterns to improve:
- Several instances of hardcoded magic numbers without explanatory constants
- Inconsistent error handling between critical and non-critical failures
- Performance optimizations needed for database/Redis query patterns
- Some unnecessary deduplication logic that could mask bugs
Testing recommendations:
- Load test with 50+ providers to verify Redis pipeline optimization
- Test dashboard behavior when Redis/DB connections fail
- Verify memory stability of canvas animations over extended periods
- Test with malicious limit values (negative, zero, MAX_SAFE_INTEGER)
🤖 Automated review by Claude AI - focused on identifying issues for improvement
1 participant
No description provided.