feat: Add Overview dashboard page and fix UI layout issues#44
Conversation
- Add database query functions for completions and embeddings statistics - Add time series data aggregation with configurable bucket intervals - Add model distribution statistics - Create /admin/stats/overview endpoint with time range support - Support ranges: 1m, 5m, 10m, 30m, 1h, 4h, 12h Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add Overview page with real-time statistics display - Add Recharts dependency for data visualization - Implement charts: request trend, latency, model distribution, token usage, success rate - Add summary cards with key metrics - Add time range selector (1m/5m/10m/30m/1h/4h/12h) - Enable Overview nav item in sidebar - Set Overview as default landing page - Add i18n translations for en-US and zh-CN Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…onses - Add specific test logic for Anthropic using /messages endpoint - Add fallback for openai-responses when /models is unavailable - Return informative messages for providers without model list support - Show appropriate UI feedback in manage models dialog Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Wrap page in flex container with h-svh - Add border-b to header - Make secondary sidebar fixed height - Enable scrolling only in content area Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add min-w-0 and flex-1 to allow content section to shrink - Add truncate class to provider name and URL - Add shrink-0 to buttons container to prevent compression Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add border-b to AppHeader across all pages - Wrap pages in flex container with h-svh - Enable scrolling only in content area below header - Ensure consistent layout structure across all routes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
图表有点问题：
|
| ) { | ||
| logger.debug("getCompletionsTimeSeries", startTime, endTime, bucketSeconds); | ||
| const result = await db.execute(sql` | ||
| SELECT |
- Add explicit types in model-distribution.tsx to avoid implicit any - Add comments explaining type assertions in db/index.ts for raw SQL queries - Fix useMemo dependency warning in manage-models-dialog.tsx by memoizing remoteModels Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use NOW() - INTERVAL in SQL queries instead of JavaScript Date objects to avoid timezone mismatch with 'timestamp without time zone' columns. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
/gemini review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 Walkthrough漫步本次更改为后端引入了新的管理员统计概览端点，添加了供应商特定的测试策略模式，以及完整的前端仪表板，展示请求趋势、延迟、模型分布和Token使用情况的多个图表。新增数据库查询函数支持时间序列统计聚合。 变更
时序图sequenceDiagram
actor User
participant Frontend as 前端 (OverviewPage)
participant API as 后端 API<br/>(stats 端点)
participant DB as 数据库
User->>Frontend: 访问概览页面或<br/>选择时间范围
activate Frontend
Frontend->>Frontend: 初始化 useOverviewStats<br/>(timeRange)
deactivate Frontend
Frontend->>API: GET /stats/overview<br/>?range={timeRange}
activate API
API->>API: 验证范围参数<br/>映射到秒数/桶大小
API->>DB: getCompletionsStats()
API->>DB: getEmbeddingsStats()
API->>DB: getCompletionsModelDistribution()
API->>DB: getEmbeddingsModelDistribution()
API->>DB: getCompletionsTimeSeries()
API->>DB: getEmbeddingsTimeSeries()
activate DB
DB-->>API: 返回统计聚合数据
deactivate DB
API->>API: 合并数据、计算<br/>成功率、聚合Token
API-->>Frontend: 返回完整概览对象
deactivate API
Frontend->>Frontend: 缓存数据至 React Query
Frontend->>Frontend: 渲染仪表板：<br/>摘要卡片 + 5 个图表
Frontend-->>User: 显示可视化统计数据
估算代码审查工作量🎯 4 (复杂) | ⏱️ ~45 分钟 此变更涉及多个层级的新功能实现（后端、数据库、前端），包含复杂的数据聚合逻辑、新的 React 组件库集成、以及国际化支持。需要特别关注数据库模块中的重复函数声明，以及后端 API 中供应商测试策略的多分支逻辑。 诗
🚥 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
Comment |
✅ Actions performedReview triggered.
|
![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 'Overview' dashboard feature, providing comprehensive statistics for API usage. The backend now includes a new /admin/stats/overview endpoint that fetches detailed metrics such as total requests, success rates, average latency, time-to-first-token (TTFT), model distribution, and token usage for both completions and embeddings. This data is retrieved from the database using several new SQL queries that support time-range filtering and bucketing. On the frontend, a new 'Overview' page has been added, accessible via the sidebar, which visualizes these statistics using recharts components like line, area, pie, and bar charts. The page includes summary cards, time range selection, and loading/error states. Additionally, the pull request refactors provider connection testing logic to handle specific provider types (e.g., Anthropic, openai-responses) that may not support standard model listing, providing appropriate messages to the user. Review comments highlighted the need to align client-side time series bucket generation with the database's bucketing logic to prevent data misalignment and suggested a more robust, extensible pattern for provider connection testing instead of nested if statements. Another comment pointed out a mathematical inaccuracy in averaging durations for mixed completions and embeddings, recommending a weighted average based on request counts.
| const now = Date.now(); | ||
| const bucketCount = Math.ceil(config.seconds / config.bucketSeconds); | ||
| for (let i = 0; i < bucketCount; i++) { | ||
| // Calculate bucket time from start (now - rangeSeconds) | ||
| const bucketTime = new Date( | ||
| now - config.seconds * 1000 + i * config.bucketSeconds * 1000, | ||
| ); | ||
| const key = bucketTime.toISOString(); | ||
| timeSeriesMap.set(key, { | ||
| timestamp: key, | ||
| completionsCount: 0, | ||
| embeddingsCount: 0, | ||
| completionsFailed: 0, | ||
| embeddingsFailed: 0, | ||
| avgDuration: 0, | ||
| avgTTFT: 0, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The current implementation for generating time series buckets in JavaScript may not align with the buckets generated by the database. Date.now() in the application and NOW() in PostgreSQL are not synchronized, which can lead to misaligned bucket timestamps. This could result in data points from the database not matching any generated buckets, leading to empty or incomplete charts.
To ensure alignment, you should round the current time down to the nearest bucket interval before generating the series of buckets. This makes the client-side bucket generation deterministic and consistent with the database's floor logic.
| const now = Date.now(); | |
| const bucketCount = Math.ceil(config.seconds / config.bucketSeconds); | |
| for (let i = 0; i < bucketCount; i++) { | |
| // Calculate bucket time from start (now - rangeSeconds) | |
| const bucketTime = new Date( | |
| now - config.seconds * 1000 + i * config.bucketSeconds * 1000, | |
| ); | |
| const key = bucketTime.toISOString(); | |
| timeSeriesMap.set(key, { | |
| timestamp: key, | |
| completionsCount: 0, | |
| embeddingsCount: 0, | |
| completionsFailed: 0, | |
| embeddingsFailed: 0, | |
| avgDuration: 0, | |
| avgTTFT: 0, | |
| }); | |
| } | |
| const bucketMillis = config.bucketSeconds * 1000; | |
| // Align start time to the bucket to match DB bucketing and prevent mismatches. | |
| const startTime = Math.floor((Date.now() - config.seconds * 1000) / bucketMillis) * bucketMillis; | |
| const bucketCount = Math.ceil(config.seconds / config.bucketSeconds); | |
| for (let i = 0; i < bucketCount; i++) { | |
| const bucketTime = new Date(startTime + i * bucketMillis); | |
| const key = bucketTime.toISOString(); | |
| timeSeriesMap.set(key, { | |
| timestamp: key, | |
| completionsCount: 0, | |
| embeddingsCount: 0, | |
| completionsFailed: 0, | |
| embeddingsFailed: 0, | |
| avgDuration: 0, | |
| avgTTFT: 0, | |
| }); | |
| } |
| if (existing.avgDuration > 0) { | ||
| existing.avgDuration = | ||
| (existing.avgDuration + Number(row.avg_duration)) / 2; | ||
| } else { | ||
| existing.avgDuration = Number(row.avg_duration); | ||
| } |
There was a problem hiding this comment.
Averaging two averages, as done here for avgDuration, is mathematically correct only if the number of items in both groups is identical. Since the number of completions and embeddings requests within a time bucket can differ, this calculation will produce an incorrect overall average duration.
To fix this, you should calculate a weighted average using the request counts for both completions and embeddings, which are available.
const completionsCount = existing.completionsCount;
const embeddingsCount = Number(row.total);
if (completionsCount > 0) {
// Both completions and embeddings might have data, calculate weighted average
const totalDuration =
existing.avgDuration * completionsCount +
Number(row.avg_duration) * embeddingsCount;
const totalCount = completionsCount + embeddingsCount;
existing.avgDuration = totalCount > 0 ? totalDuration / totalCount : 0;
} else {
// Only embeddings have data for this bucket
existing.avgDuration = Number(row.avg_duration);
}| if (provider.type === "anthropic") { | ||
| const baseUrl = provider.baseUrl.endsWith("/") | ||
| ? provider.baseUrl.slice(0, -1) | ||
| : provider.baseUrl; | ||
|
|
||
| const response = await fetch(`${baseUrl}/messages`, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| "anthropic-version": provider.apiVersion || "2023-06-01", | ||
| ...(provider.apiKey && { "x-api-key": provider.apiKey }), | ||
| }, | ||
| body: JSON.stringify({ | ||
| model: "claude-3-haiku-20240307", // Use a common model for testing | ||
| messages: [{ role: "user", content: "Hi" }], | ||
| max_tokens: 1, | ||
| }), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| const text = await response.text(); | ||
| // Check if the error is just about invalid model (which means auth is working) | ||
| if (response.status === 400 && text.includes("model")) { | ||
| return { | ||
| success: true, | ||
| message: "Connection successful (API key valid)", | ||
| models: [], | ||
| }; | ||
| } | ||
| throw new Error(`API error: ${response.status} ${text}`); | ||
| } | ||
|
|
||
| return { | ||
| success: true, | ||
| message: "Connection successful", | ||
| models: [], | ||
| }; | ||
| } | ||
|
|
||
| // For openai-responses, try the standard /models endpoint first | ||
| // since most deployments share the same OpenAI account | ||
| if (provider.type === "openai-responses") { | ||
| const client = new OpenAI({ | ||
| baseURL: provider.baseUrl, | ||
| apiKey: provider.apiKey || "not-required", | ||
| }); | ||
|
|
||
| try { | ||
| const models = await client.models.list(); | ||
| return { | ||
| success: true, | ||
| models: models.data.map((m) => ({ | ||
| id: m.id, | ||
| owned_by: m.owned_by, | ||
| })), | ||
| }; | ||
| } catch { | ||
| // If /models doesn't work, just report success for connection test | ||
| return { | ||
| success: true, | ||
| message: "Connection configured (models endpoint not available)", | ||
| models: [], | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for testing provider connections is growing complex with if statements for different provider types. This approach can become difficult to maintain as more provider-specific logic is added.
Consider refactoring this into a more extensible pattern, such as a strategy pattern or a map of test functions. This would separate the logic for each provider and make the main function cleaner and easier to extend.
For example:
// Define a type for test functions
type ProviderTestFn = (provider: Provider) => Promise<any>;
// Map provider types to their specific test functions
const providerTestHandlers: Partial<Record<ProviderType, ProviderTestFn>> = {
'anthropic': testAnthropicConnection,
'openai-responses': testOpenAIResponsesConnection,
};
// In the route handler
const testFn = providerTestHandlers[provider.type] ?? testDefaultOpenAIConnection;
return await testFn(provider);![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/settings/manage-models-dialog.tsx (1)
126-126:savedModelNames已计算但未在filteredRemoteModels中使用。Line 126 计算了
savedModelNames，但在filteredRemoteModels（Lines 128-133）的过滤逻辑中没有排除已保存的模型。根据注释 "Filter remote models based on search query and exclude already saved models"，应该添加排除逻辑。🐛 建议修复
const filteredRemoteModels = useMemo(() => { return remoteModels.filter((model) => { const matchesSearch = model.id.toLowerCase().includes(searchQuery.toLowerCase()) - return matchesSearch + const notSaved = !savedModelNames.has(model.id) + return matchesSearch && notSaved }) -}, [remoteModels, searchQuery]) +}, [remoteModels, searchQuery, savedModelNames])
🤖 Fix all issues with AI agents
In `@backend/src/api/admin/providers.ts`:
- Around line 214-221: The catch block that currently swallows all errors and
returns { success: true, message: "Connection configured (models endpoint not
available)", models: [] } must be made error-aware: inspect the caught error
inside that catch, log or record the error details, and only treat it as the
"models endpoint not available" success case for specific conditions (e.g., HTTP
404 / endpoint-missing or a recognized API-format error); for network/DNS/auth
failures or other unexpected exceptions return { success: false, message:
<error.message or descriptive text>, models: [] } or rethrow so callers can
handle it. Update the catch around the connection test (the block returning
success/message/models) to distinguish endpoint-not-found vs real connection
errors and include the original error details in logs and the returned message
when failing.
♻️ Duplicate comments (1)
backend/src/db/index.ts (1)
1156-1169: 时间序列查询使用原始 SQL。这与之前的评审意见一致：尽量避免使用 raw SQL。但考虑到 Drizzle ORM 对复杂时间分桶查询的支持有限，此处使用原始 SQL 是合理的权衡。
建议添加注释说明为何此处需要原始 SQL，并确保表名使用 schema 引用而非硬编码字符串，以避免表名重命名时的不一致。
♻️ 建议使用 schema 引用表名
FROM completions - FROM ${sql.identifier(schema.CompletionsTable) + -- 或者在文件顶部定义常量 + const COMPLETIONS_TABLE = 'completions';虽然目前表名不太可能改变，但使用常量或 schema 引用可提高可维护性。
Also applies to: 1194-1206
🧹 Nitpick comments (18)
frontend/src/pages/settings/providers-settings-page.tsx (1)
249-266: Flex 布局和溢出处理实现得当。正确使用了以下技术解决长 URL 溢出问题：
min-w-0允许 flex 子元素收缩到内容以下shrink-0保护图标和按钮区域不被压缩truncate配合min-w-0实现文本截断可选改进：为被截断的文本添加
title属性以提升可访问性，让用户悬停时能看到完整内容。♻️ 可选优化
- <span className="truncate text-base font-medium">{provider.name}</span> + <span className="truncate text-base font-medium" title={provider.name}>{provider.name}</span>- <div className="text-muted-foreground mt-1 truncate text-sm"> + <div className="text-muted-foreground mt-1 truncate text-sm" title={provider.baseUrl}>frontend/src/pages/overview/time-range-select.tsx (1)
22-38:onValueChange存在类型安全问题。
Select的onValueChange回调接收的是string类型，但onChange期望的是TimeRange类型。虽然运行时不会有问题（因为SelectItem的值都是有效的TimeRange），但缺少类型断言会使 TypeScript 的类型检查失效。♻️ 建议的修复方案
export function TimeRangeSelect({ value, onChange }: TimeRangeSelectProps) { const { t } = useTranslation() return ( - <Select value={value} onValueChange={onChange}> + <Select value={value} onValueChange={(v) => onChange(v as TimeRange)}> <SelectTrigger className="w-[140px]"> <SelectValue placeholder={t('pages.overview.selectTimeRange')} /> </SelectTrigger>frontend/src/routes/settings/route.tsx (1)
39-60:NavContent作为内联函数组件可能影响性能。
NavContent在每次RouteComponent渲染时都会创建新的函数引用。考虑到 PR 评论中提到的性能问题（特别是展开/收起侧边栏时的卡顿），建议将其提取为独立组件或使用useMemo/useCallback进行优化。♻️ 建议的优化方案
+const NavContent = ({ navItems, matchRoute }: { + navItems: { icon: React.ReactNode; title: string; href: string }[]; + matchRoute: ReturnType<typeof useMatchRoute>; +}) => ( + <nav className="space-y-2"> + {navItems.map((item) => { + const isActive = !!matchRoute({ to: item.href, fuzzy: true }) + return ( + <Link + key={item.href} + to={item.href} + className={cn( + 'flex items-center gap-3 rounded-lg px-3 py-2.5 text-sm font-medium transition-colors', + isActive + ? 'bg-primary text-primary-foreground' + : 'text-muted-foreground hover:bg-muted hover:text-foreground' + )} + > + {item.icon} + {item.title} + </Link> + ) + })} + </nav> +) function RouteComponent() { // ... existing code ... - const NavContent = () => ( - <nav className="space-y-2"> - {/* ... */} - </nav> - )或者使用
useMemo:const NavContent = useMemo(() => ( <nav className="space-y-2"> {navItems.map((item) => { const isActive = !!matchRoute({ to: item.href, fuzzy: true }) return ( <Link key={item.href} to={item.href} /* ... */> {item.icon} {item.title} </Link> ) })} </nav> ), [navItems, matchRoute])backend/src/db/index.ts (1)
1054-1054:sql.raw()存在潜在的 SQL 注入风险。虽然当前
rangeSeconds来自受信任的RANGE_CONFIG，但使用sql.raw(String(rangeSeconds))绕过了参数化查询的保护。如果未来代码重构导致用户输入直接传入，可能产生安全漏洞。建议使用参数化方式或验证输入：
♻️ 建议的修复方案
可以使用 PostgreSQL 的
make_interval函数配合参数化查询：-sql`${schema.CompletionsTable.createdAt} >= NOW() - INTERVAL '${sql.raw(String(rangeSeconds))} seconds'` +sql`${schema.CompletionsTable.createdAt} >= NOW() - make_interval(secs => ${rangeSeconds})`或者在函数入口处添加验证：
export async function getCompletionsStats(rangeSeconds: number) { // Validate rangeSeconds is a positive integer within expected bounds if (!Number.isInteger(rangeSeconds) || rangeSeconds <= 0 || rangeSeconds > 86400) { throw new Error('Invalid rangeSeconds'); } // ... }Also applies to: 1087-1087, 1115-1115, 1138-1138
backend/src/api/admin/stats.ts (2)
31-33: 建议使用 Elysia 的错误处理机制。抛出普通的
Error会导致 500 错误，建议使用 Elysia 的error函数返回适当的 HTTP 状态码。♻️ 建议的修复方案
+import { Elysia, t, error } from "elysia"; // ... if (!config) { - throw new Error(`Invalid range: ${rangeKey}`); + return error(400, { message: `Invalid range: ${rangeKey}` }); }不过由于查询参数已使用
t.Union验证，理论上此分支不会被触发，可以考虑移除或保留作为防御性编程。
140-148: 持续时间的平均计算可能不准确。当合并 completions 和 embeddings 的平均持续时间时，简单地使用
(a + b) / 2没有考虑两者的请求数量差异。如果 completions 有 1000 个请求而 embeddings 只有 10 个，这种平均方式会产生误导性结果。♻️ 建议使用加权平均
// Fill in embeddings data for (const row of embeddingsTimeSeries) { const key = new Date(row.bucket).toISOString(); const existing = timeSeriesMap.get(key); if (existing) { + const embeddingsCount = Number(row.total); existing.embeddingsCount = Number(row.total); existing.embeddingsFailed = Number(row.failed); - // Average the duration if completions already has data - if (existing.avgDuration > 0) { - existing.avgDuration = - (existing.avgDuration + Number(row.avg_duration)) / 2; + // Weighted average of duration based on request counts + if (existing.completionsCount > 0 && embeddingsCount > 0) { + const totalCount = existing.completionsCount + embeddingsCount; + existing.avgDuration = + (existing.avgDuration * existing.completionsCount + + Number(row.avg_duration) * embeddingsCount) / totalCount; } else { existing.avgDuration = Number(row.avg_duration); } } }backend/src/api/admin/providers.ts (1)
163-175: 建议为 fetch 请求添加超时控制。当前的 fetch 请求没有超时机制，如果 Anthropic API 无响应，请求可能会无限期挂起。
♻️ 建议的修改
- const response = await fetch(`${baseUrl}/messages`, { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); + + const response = await fetch(`${baseUrl}/messages`, { method: "POST", headers: { "Content-Type": "application/json", "anthropic-version": provider.apiVersion || "2023-06-01", ...(provider.apiKey && { "x-api-key": provider.apiKey }), }, body: JSON.stringify({ model: "claude-3-haiku-20240307", // Use a common model for testing messages: [{ role: "user", content: "Hi" }], max_tokens: 1, }), + signal: controller.signal, }); + + clearTimeout(timeoutId);frontend/src/components/app/app-sidebar.tsx (1)
23-27: Overview 导航项添加正确。新的 Overview 导航项遵循了现有的导航项模式。
注意：
navItems数组在模块加载时使用i18n.t()解析翻译，这意味着运行时切换语言不会更新导航标题。这是现有代码的模式，建议在未来考虑将navItems移入组件内部或使用useMemo配合useTranslation来支持动态语言切换。frontend/src/pages/overview/summary-cards.tsx (1)
12-22: 成功率计算逻辑正确，但建议添加useMemo优化性能。当前计算逻辑在每次渲染时都会重新执行。考虑到 PR 评论中提到的性能问题（特别是展开/收起侧边栏时的卡顿），建议将
overallSuccessRate和cards数组的计算用useMemo包裹。♻️ 建议的优化
+import { useMemo } from 'react' import { ActivityIcon, ClockIcon, GaugeIcon, ZapIcon } from 'lucide-react' import { useTranslation } from 'react-i18next' // ... export function SummaryCards({ data }: SummaryCardsProps) { const { t } = useTranslation() const { summary } = data - // Calculate overall success rate - const totalCompleted = - (summary.completionsCount * summary.completionsSuccessRate) / 100 + - (summary.embeddingsCount * summary.embeddingsSuccessRate) / 100 - const overallSuccessRate = - summary.totalRequests > 0 ? (totalCompleted / summary.totalRequests) * 100 : 100 - - const cards = [ + const { overallSuccessRate, cards } = useMemo(() => { + const totalCompleted = + (summary.completionsCount * summary.completionsSuccessRate) / 100 + + (summary.embeddingsCount * summary.embeddingsSuccessRate) / 100 + const rate = + summary.totalRequests > 0 ? (totalCompleted / summary.totalRequests) * 100 : 100 + + return { + overallSuccessRate: rate, + cards: [ // ... cards array - ] + ], + } + }, [summary, t])frontend/src/pages/settings/manage-models-dialog.tsx (1)
37-40: 类型断言可以简化。当前的类型断言略显冗长，可以使用更简洁的方式。
♻️ 简化类型检查
- const supportsRemoteModels = !UNSUPPORTED_REMOTE_MODEL_TYPES.includes( - provider.type as (typeof UNSUPPORTED_REMOTE_MODEL_TYPES)[number] - ) + const supportsRemoteModels = !(UNSUPPORTED_REMOTE_MODEL_TYPES as readonly string[]).includes(provider.type)frontend/src/pages/overview/charts/token-usage-chart.tsx (1)
13-26: 建议使用useMemo缓存chartData。
chartData在每次渲染时都会重新创建，且涉及 i18n 翻译调用。根据 PR 评论中提到的性能问题，建议进行 memoization。♻️ 建议的优化
+import { useMemo } from 'react' import { useTranslation } from 'react-i18next' // ... export function TokenUsageChart({ data }: TokenUsageChartProps) { const { t } = useTranslation() - const chartData = [ + const chartData = useMemo(() => [ { name: t('pages.overview.tokens.prompt'), value: data.promptTokens, }, // ... - ] + ], [data.promptTokens, data.completionTokens, data.embeddingTokens, t])frontend/src/pages/overview/charts/success-rate-chart.tsx (2)
14-30: 建议使用useMemo缓存chartData计算。
chartData涉及对整个data数组的 map 操作和计算，应该进行 memoization 以避免不必要的重计算，特别是考虑到 PR 评论中提到的性能问题。♻️ 建议的优化
+import { useMemo } from 'react' import { format } from 'date-fns' // ... export function SuccessRateChart({ data }: SuccessRateChartProps) { const { t } = useTranslation() - const chartData = data.map((item: OverviewStats['timeSeries'][number]) => { + const chartData = useMemo(() => data.map((item) => { // ... calculation logic - }) + }), [data])
52-67: 图表配置合理，但dot={false}可能影响数据点识别。当数据点较少时，禁用点标记可能使用户难以准确定位数据。如果时间范围较短（如 1m），考虑根据数据点数量动态决定是否显示点标记。
frontend/src/pages/overview/charts/requests-trend-chart.tsx (2)
14-18: 考虑使用useMemo缓存chartData转换结果。PR 评论中提到侧边栏展开/收起时存在性能问题。由于数据转换在每次渲染时都会执行，建议使用
useMemo来优化：♻️ 建议的优化
+import { useMemo } from 'react' import { format } from 'date-fns' import { useTranslation } from 'react-i18next' ... export function RequestsTrendChart({ data }: RequestsTrendChartProps) { const { t } = useTranslation() - const chartData = data.map((item: OverviewStats['timeSeries'][number]) => ({ - timestamp: item.timestamp, - completions: item.completionsCount, - embeddings: item.embeddingsCount, - })) + const chartData = useMemo( + () => + data.map((item: OverviewStats['timeSeries'][number]) => ({ + timestamp: item.timestamp, + completions: item.completionsCount, + embeddings: item.embeddingsCount, + })), + [data] + )
30-37: Tooltip 样式可以抽取为共享常量。此 contentStyle 配置在多个图表组件中重复出现。考虑将其提取为共享常量以保持一致性并简化维护。
frontend/src/pages/overview/charts/model-distribution.tsx (1)
37-41: 建议使用useMemo缓存chartData。与其他图表组件一致，建议对数据转换进行缓存以提升性能。
♻️ 建议的优化
+import { useMemo } from 'react' import { useTranslation } from 'react-i18next' ... - const chartData: ChartDataItem[] = data.map((item: ModelDistributionItem) => ({ - name: item.model, - value: item.count, - type: item.type, - })) + const chartData = useMemo<ChartDataItem[]>( + () => + data.map((item: ModelDistributionItem) => ({ + name: item.model, + value: item.count, + type: item.type, + })), + [data] + )frontend/src/pages/overview/index.tsx (1)
59-65: 考虑为错误状态添加重试功能。当前错误状态仅显示错误信息，用户需要手动刷新页面。可以考虑添加重试按钮以提升用户体验。
♻️ 建议的优化
+ const { data, isLoading, error, refetch } = useOverviewStats(timeRange) if (error) { return ( <div className="flex h-[50vh] items-center justify-center"> - <p className="text-destructive">{t('pages.overview.fetchError')}</p> + <div className="text-center"> + <p className="text-destructive">{t('pages.overview.fetchError')}</p> + <button + onClick={() => refetch()} + className="mt-4 text-sm text-primary hover:underline" + > + {t('common.retry')} + </button> + </div> </div> ) }frontend/src/pages/overview/charts/latency-chart.tsx (1)
14-18: 建议使用useMemo缓存chartData。与其他图表组件保持一致，建议对数据转换进行缓存。
♻️ 建议的优化
+import { useMemo } from 'react' import { format } from 'date-fns' ... - const chartData = data.map((item: OverviewStats['timeSeries'][number]) => ({ - timestamp: item.timestamp, - duration: Math.round(item.avgDuration), - ttft: Math.round(item.avgTTFT), - })) + const chartData = useMemo( + () => + data.map((item: OverviewStats['timeSeries'][number]) => ({ + timestamp: item.timestamp, + duration: Math.round(item.avgDuration), + ttft: Math.round(item.avgTTFT), + })), + [data] + )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
backend/src/api/admin/index.tsbackend/src/api/admin/providers.tsbackend/src/api/admin/stats.tsbackend/src/db/index.tsfrontend/package.jsonfrontend/src/components/app/app-sidebar.tsxfrontend/src/i18n/locales/en-US.jsonfrontend/src/i18n/locales/zh-CN.jsonfrontend/src/pages/overview/charts/latency-chart.tsxfrontend/src/pages/overview/charts/model-distribution.tsxfrontend/src/pages/overview/charts/requests-trend-chart.tsxfrontend/src/pages/overview/charts/success-rate-chart.tsxfrontend/src/pages/overview/charts/token-usage-chart.tsxfrontend/src/pages/overview/index.tsxfrontend/src/pages/overview/summary-cards.tsxfrontend/src/pages/overview/time-range-select.tsxfrontend/src/pages/overview/use-overview-stats.tsfrontend/src/pages/settings/manage-models-dialog.tsxfrontend/src/pages/settings/providers-settings-page.tsxfrontend/src/routes/_dashboard/index.tsxfrontend/src/routes/_dashboard/route.tsxfrontend/src/routes/apps/route.tsxfrontend/src/routes/embeddings/route.tsxfrontend/src/routes/requests/route.tsxfrontend/src/routes/settings/route.tsx
🧰 Additional context used
🧬 Code graph analysis (14)
frontend/src/pages/overview/charts/model-distribution.tsx (2)
frontend/src/pages/overview/use-overview-stats.ts (1)
OverviewStats(25-25)frontend/src/components/ui/tooltip.tsx (1)
Tooltip(48-48)
frontend/src/pages/overview/charts/token-usage-chart.tsx (1)
frontend/src/pages/overview/use-overview-stats.ts (1)
OverviewStats(25-25)
frontend/src/pages/overview/summary-cards.tsx (2)
frontend/src/pages/overview/use-overview-stats.ts (1)
OverviewStats(25-25)frontend/src/components/ui/card.tsx (4)
Card(35-35)CardHeader(35-35)CardTitle(35-35)CardContent(35-35)
frontend/src/routes/_dashboard/route.tsx (1)
frontend/src/components/app/app-header.tsx (5)
AppHeader(7-13)AppHeaderPart(23-29)AppSidebarTrigger(15-17)AppSidebarSeparator(19-21)AppHeaderTitle(35-41)
frontend/src/pages/overview/time-range-select.tsx (2)
frontend/src/pages/overview/use-overview-stats.ts (1)
TimeRange(6-6)frontend/src/components/ui/select.tsx (5)
Select(140-140)SelectTrigger(148-148)SelectValue(149-149)SelectContent(141-141)SelectItem(143-143)
backend/src/api/admin/stats.ts (1)
backend/src/db/index.ts (6)
getCompletionsStats(1038-1067)getEmbeddingsStats(1073-1098)getCompletionsModelDistribution(1104-1121)getEmbeddingsModelDistribution(1127-1144)getCompletionsTimeSeries(1151-1182)getEmbeddingsTimeSeries(1189-1218)
frontend/src/routes/embeddings/route.tsx (1)
frontend/src/components/app/app-header.tsx (5)
AppHeader(7-13)AppHeaderPart(23-29)AppSidebarTrigger(15-17)AppSidebarSeparator(19-21)AppHeaderTitle(35-41)
frontend/src/pages/settings/providers-settings-page.tsx (1)
frontend/src/components/ui/card.tsx (1)
CardContent(35-35)
frontend/src/pages/overview/charts/requests-trend-chart.tsx (2)
frontend/src/pages/overview/use-overview-stats.ts (1)
OverviewStats(25-25)frontend/src/components/ui/tooltip.tsx (1)
Tooltip(48-48)
frontend/src/pages/overview/use-overview-stats.ts (2)
frontend/src/lib/api.ts (1)
api(21-21)frontend/src/lib/error.ts (1)
formatError(3-16)
frontend/src/pages/overview/charts/success-rate-chart.tsx (2)
frontend/src/pages/overview/use-overview-stats.ts (1)
OverviewStats(25-25)frontend/src/components/ui/tooltip.tsx (1)
Tooltip(48-48)
frontend/src/pages/overview/charts/latency-chart.tsx (2)
frontend/src/pages/overview/use-overview-stats.ts (1)
OverviewStats(25-25)frontend/src/components/ui/tooltip.tsx (1)
Tooltip(48-48)
frontend/src/pages/settings/manage-models-dialog.tsx (1)
frontend/src/lib/api.ts (1)
api(21-21)
backend/src/api/admin/index.ts (1)
backend/src/api/admin/stats.ts (1)
adminStats(24-198)
🔇 Additional comments (32)
frontend/package.json (1)
50-50: 依赖添加合理。recharts 是一个成熟的 React 图表库，用于支持新的 Overview 仪表板页面。版本
^3.6.0使用语义化版本范围是合理的做法。frontend/src/routes/apps/route.tsx (1)
19-31: 布局重构实现正确。采用
h-svh+flex-col容器配合min-h-0 flex-1 overflow-y-auto的滚动区域是处理固定头部 + 可滚动内容的最佳实践：
min-h-0确保 flex 子元素可以正确收缩flex-1使内容区域填充剩余空间overflow-y-auto启用垂直滚动此模式与 PR 中其他路由文件保持一致。
frontend/src/routes/requests/route.tsx (1)
21-34: 布局调整与其他路由保持一致。实现与
/apps路由采用相同的布局模式，确保了整个应用的视觉和交互一致性。frontend/src/routes/embeddings/route.tsx (1)
19-30: LGTM!布局模式与
/apps、/requests等路由一致，保持了代码库的统一性。frontend/src/i18n/locales/en-US.json (1)
342-368: LGTM! Overview 页面的国际化键值完整且组织良好。新增的
pages.overview命名空间下的键值与前端组件需求一致，时间范围选项与TimeRange类型定义匹配。frontend/src/routes/settings/route.tsx (1)
63-107: LGTM! 响应式布局结构清晰。移动端的 Sheet 导航和桌面端的固定侧边栏实现合理，
h-svh配合flex布局解决了视口高度问题，overflow-y-auto确保内容可滚动。backend/src/db/index.ts (1)
1174-1181: 类型断言的使用是合理的。对于原始 SQL 查询返回的结果使用类型断言是必要的，注释清楚解释了原因。PostgreSQL 聚合函数返回字符串以保持精度的行为也已正确记录。
Also applies to: 1211-1217
backend/src/api/admin/stats.ts (3)
100-119: 时间桶生成可能与数据库时间不对齐。使用客户端
Date.now()生成时间桶，而数据库查询使用NOW()。如果服务器与数据库之间存在时钟偏差，可能导致某些数据点无法匹配到预生成的桶中。建议验证：
- 服务器和数据库是否在同一时区配置下运行
- 是否存在明显的时钟偏差
或者考虑从数据库查询当前时间来生成桶：
// 可选：从数据库获取当前时间以确保一致性 const [{ now }] = await db.execute(sql`SELECT NOW() as now`);
39-54: LGTM! 并行数据获取是正确的优化。使用
Promise.all并行获取所有统计数据可以显著减少总体响应时间。数据库函数的调用方式正确。
178-196: LGTM! 查询参数验证完善。使用
t.Union配合t.Literal限制range参数的有效值，提供了良好的类型安全和运行时验证。backend/src/api/admin/index.ts (1)
10-10: LGTM!adminStats 模块的导入和集成遵循了现有的路由模式，正确地放置在受保护的 admin 路由组内。
Also applies to: 31-31
backend/src/api/admin/providers.ts (2)
264-270: LGTM!Anthropic 不支持模型列表 API，提前返回带有
unsupported标志的 400 响应是正确的处理方式，便于前端显示适当的提示信息。
287-293: LGTM!对于 openai-responses 类型，在 /models 端点不可用时返回带有
unsupported标志的响应，与 Anthropic 的处理方式保持一致。frontend/src/i18n/locales/zh-CN.json (3)
11-11: LGTM!侧边栏 Overview 翻译添加正确。
334-337: LGTM!远程模型不支持场景的翻译完整且准确。
343-369: LGTM!概览页面的翻译键完整，涵盖了标题、指标、图表和时间范围选择器所需的所有文案。
frontend/src/components/app/app-sidebar.tsx (1)
2-2: LGTM!ChartPieIcon 导入正确。
frontend/src/pages/overview/use-overview-stats.ts (1)
1-25: LGTM!React Query hook 实现良好：
TimeRange类型与后端 API 的 range 参数保持一致queryKey正确包含range参数以实现缓存隔离- 30 秒的
refetchInterval符合 PR 目标中的自动刷新需求- 错误处理使用
formatError进行规范化OverviewStats类型推导简洁实用frontend/src/pages/overview/summary-cards.tsx (1)
51-66: UI 结构清晰，实现良好。响应式网格布局使用
grid-cols-2 md:grid-cols-4适配不同屏幕尺寸，Card 组件结构合理。frontend/src/pages/settings/manage-models-dialog.tsx (3)
72-74: 良好的useMemo使用，避免了不必要的数组引用变化。正确地对
remoteModels进行了 memoization，防止下游组件不必要的重渲染。
200-265: 远程模型标签页的多状态处理完整且用户体验良好。正确处理了以下状态：
- 不支持远程列表的 provider 类型
- 后端返回消息提示
- 获取失败并提供重试按钮
- 正常加载和展示列表
UI 反馈清晰，用户引导明确。
17-18: 常量定义正确，无需扩展。
openai-responses不需要添加到此列表。虽然其模型列表端点在某些部署中可能不可用，但这是条件性的，而非绝对不支持。后端 API 已正确处理此情况：当端点不可用时返回错误消息（含unsupported: true标志），前端则通过remoteModelsMessage向用户显示该消息。相比之下，anthropic始终不支持模型列表（无端点存在），因此应保持在此常量中。当前设计允许根据实际部署能力动态支持模型列表。frontend/src/pages/overview/charts/token-usage-chart.tsx (1)
44-51: Tooltip 已正确设置背景色，符合 PR 评论中的反馈。
contentStyle配置了backgroundColor: 'hsl(var(--background))'和边框样式，解决了 koitococo 反馈的 tooltip 缺少背景导致可读性差的问题。frontend/src/routes/_dashboard/route.tsx (1)
19-31: 布局结构修复正确，与 PR 目标一致。
- 使用
h-svh确保容器占满视口高度border-b为头部添加了一致的边框样式min-h-0 flex-1 overflow-y-auto正确实现了可滚动内容区域这与 AI 摘要中提到的其他路由（如
apps/route.tsx和requests/route.tsx）的布局模式保持一致。frontend/src/pages/overview/charts/success-rate-chart.tsx (1)
42-50: Tooltip 样式与其他图表组件保持一致。
contentStyle配置正确，使用了主题变量，解决了可读性问题。frontend/src/routes/_dashboard/index.tsx (1)
1-9: LGTM!路由配置简洁明了，正确地将 OverviewPage 设置为仪表板的默认组件，并保留了错误处理。
frontend/src/pages/overview/charts/model-distribution.tsx (2)
26-32: 空数据检查实现良好。正确处理了空数据场景，提供了用户友好的本地化提示信息。
55-57: 标签渲染逻辑正确处理了 null 值。使用
name ?? ''和percent ?? 0安全地处理了可能的 null/undefined 值。frontend/src/pages/overview/index.tsx (2)
16-52: LoadingSkeleton 组件实现良好。骨架屏布局与实际内容布局一致，提供了良好的加载体验。由于该组件不接收 props，可以考虑将其移到组件外部或使用
React.memo避免不必要的重新创建。
67-132: 页面布局结构清晰，组件职责分明。响应式网格布局合理，各图表组件接收正确的数据切片。时间范围选择器位置适当。整体实现符合预期。
frontend/src/pages/overview/charts/latency-chart.tsx (2)
29-30: 双 Y 轴设计合理。将 duration 和 ttft 分配到不同的 Y 轴上，可以更好地展示两个可能具有不同数量级的指标。
40-57: 图表配置正确，样式一致。Line 组件配置合理：
dot={false}减少视觉噪音，strokeWidth={2}保证可见性，使用主题色变量保持风格统一。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Frontend fixes (koitococo's feedback): - Add chart-styles.ts with shared tooltip styles including box-shadow for better readability - Add CSS rules to remove recharts focus outline on chart elements - Wrap all chart components with React.memo() to prevent unnecessary re-renders - Add useMemo in OverviewPage to stabilize data references during sidebar toggle Backend fixes (gemini-code-assist & coderabbitai feedback): - Fix time bucket alignment by flooring start time to bucket boundary - Fix weighted average calculation for duration (was incorrectly averaging two averages) - Refactor provider test logic using strategy pattern for better maintainability - Improve catch block error handling to distinguish 404/405 from real connection errors Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
/gemini review the latest commit change, does it resolve the previously raised issues? |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive Overview dashboard, providing valuable real-time statistics about platform usage. The addition of various charts for requests, latency, and token usage is a great enhancement. I'm also impressed with the significant UI layout improvements across the application, such as the fixed headers and scrollable content areas, which create a much more polished and consistent user experience. The refactoring of the provider connection testing logic is well-executed and makes the system more robust and easier to maintain. My review includes a few suggestions to further improve code quality, focusing on reducing duplication, enhancing accessibility, and increasing consistency in the new components.
| // Check if it's a 404/405 (endpoint not available) vs real connection error | ||
| const error = e as Error & { status?: number }; | ||
| const errorMessage = error.message || ""; | ||
|
|
||
| // These indicate the endpoint doesn't exist but connection works | ||
| if ( | ||
| error.status === 404 || | ||
| error.status === 405 || | ||
| errorMessage.includes("404") || | ||
| errorMessage.includes("405") || | ||
| errorMessage.includes("Not Found") || | ||
| errorMessage.includes("Method Not Allowed") | ||
| ) { | ||
| return { | ||
| success: true, | ||
| message: "Connection configured (models endpoint not available)", | ||
| models: [], | ||
| }; | ||
| } | ||
|
|
||
| // Re-throw actual connection errors to be handled by outer catch | ||
| throw e; | ||
| } |
There was a problem hiding this comment.
The error handling logic to detect if a models endpoint is unavailable (checking for 404/405 errors) is duplicated here and in the /remote-models endpoint handler (lines 353-372). To improve maintainability and reduce code duplication, you could extract this logic into a shared helper function.
For example:
function isOpenAIModelEndpointError(error: Error & { status?: number }): boolean {
const errorMessage = error.message || "";
return (
error.status === 404 ||
error.status === 405 ||
errorMessage.includes("404") ||
errorMessage.includes("405") ||
errorMessage.includes("Not Found") ||
errorMessage.includes("Method Not Allowed")
);
}You could then use this helper in both places to simplify the code.
| const COLORS = [ | ||
| 'hsl(var(--chart-1))', | ||
| 'hsl(var(--chart-2))', | ||
| 'hsl(var(--chart-3))', | ||
| 'hsl(var(--chart-4))', | ||
| 'hsl(var(--chart-5))', | ||
| 'hsl(221.2, 83.2%, 53.3%)', | ||
| 'hsl(212, 95%, 68%)', | ||
| 'hsl(216, 92%, 60%)', | ||
| 'hsl(210, 98%, 78%)', | ||
| 'hsl(212, 97%, 87%)', | ||
| ] |
There was a problem hiding this comment.
The COLORS array uses a mix of CSS variables for the first five colors and hardcoded HSL values for the rest. For better theming support and consistency, consider defining all these colors as CSS variables in your styles.css file (e.g., --chart-6, --chart-7, etc.) and referencing them here.
| completions: Math.round(completionsSuccessRate * 100) / 100, | ||
| embeddings: Math.round(embeddingsSuccessRate * 100) / 100, |
There was a problem hiding this comment.
You are rounding the success rates to two decimal places here, but the Tooltip formatter on line 48 rounds it again to one decimal place. To simplify, you could pass the unrounded success rates to the chart data and perform the rounding only in the formatter.
| completions: Math.round(completionsSuccessRate * 100) / 100, | |
| embeddings: Math.round(embeddingsSuccessRate * 100) / 100, | |
| completions: completionsSuccessRate, | |
| embeddings: embeddingsSuccessRate, |
| .recharts-wrapper:focus, | ||
| .recharts-wrapper *:focus { | ||
| outline: none; | ||
| } | ||
|
|
||
| .recharts-surface:focus { | ||
| outline: none; | ||
| } |
There was a problem hiding this comment.
Removing the focus outline has negative accessibility implications, as it makes it difficult for keyboard-only users to navigate the charts. Instead of removing the outline completely, please consider styling the focus state to be visible but less intrusive. For example, you could use a subtle box-shadow or a different color outline that fits your design while still providing a clear focus indicator.
- Extract error detection logic into shared isModelEndpointUnavailable() helper - Define chart colors 6-10 as CSS variables for consistent theming - Simplify success rate rounding by only formatting in tooltip - Improve focus state accessibility with focus-visible styling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@backend/src/api/admin/providers.ts`:
- Around line 361-377: Current logic only treats provider.type ===
"openai-responses" and "anthropic" specially when
isModelEndpointUnavailable(error) is true, causing other OpenAI-compatible
providers to return 502; change the conditional to detect all OpenAI-compatible
providers (e.g., using or adding a helper like isOpenAICompatible(provider.type)
that returns true for "openai", "azure", "ollama", "openai-responses", etc.)
and, when isModelEndpointUnavailable(error) is true, return status(400,
{...unsupported:true}) instead of falling through to the status(502) branch;
alternatively, if any providers must be exempted, add a clear code comment
explaining why those types are excluded and explicitly list them in the
condition.
In `@frontend/src/pages/overview/charts/model-distribution.tsx`:
- Around line 12-23: Update the COLORS constant in model-distribution.tsx to use
the same HSL CSS variable pattern as other chart components: replace each
'var(--color-chart-N)' entry in the COLORS array with the corresponding
'hsl(var(--chart-N))' form (e.g., change COLORS entries to
'hsl(var(--chart-1))', 'hsl(var(--chart-2))', etc.) so the file matches the
style used by success-rate-chart.tsx, requests-trend-chart.tsx,
token-usage-chart.tsx, and latency-chart.tsx.
♻️ Duplicate comments (1)
frontend/src/pages/overview/charts/success-rate-chart.tsx (1)
18-34: 数据转换逻辑正确，之前的双重舍入问题已修复。成功率计算逻辑正确，并且不再在此处进行舍入，仅在 Tooltip 格式化时进行一次舍入（第 48 行），符合之前的 review 建议。
🧹 Nitpick comments (4)
frontend/src/pages/overview/charts/model-distribution.tsx (1)
38-39: 将类型定义移到组件外部以避免重复创建。
ChartDataItem和ModelDistributionItem类型定义在组件函数体内，会在每次渲染时重新创建。建议将其移至组件外部，与ModelDistributionChartProps接口放在一起。♻️ 建议的重构
interface ModelDistributionChartProps { data: OverviewStats['modelDistribution'] } +type ChartDataItem = { name: string | null; value: number; type: 'chat' | 'embedding' } +type ModelDistributionItem = OverviewStats['modelDistribution'][number] + const COLORS = [ 'var(--color-chart-1)', // ... ] export const ModelDistributionChart = memo(function ModelDistributionChart({ data, }: ModelDistributionChartProps) { const { t } = useTranslation() if (data.length === 0) { // ... } - type ChartDataItem = { name: string | null; value: number; type: 'chat' | 'embedding' } - type ModelDistributionItem = OverviewStats['modelDistribution'][number] - const chartData: ChartDataItem[] = data.map((item: ModelDistributionItem) => ({frontend/src/pages/overview/charts/success-rate-chart.tsx (1)
13-16: 缺少空数据状态处理。与
ModelDistributionChart不同，此组件没有处理data.length === 0的情况。当数据为空时，图表可能显示异常或空白区域而没有友好提示。建议添加空数据状态的处理以保持一致性。♻️ 建议添加空数据处理
export const SuccessRateChart = memo(function SuccessRateChart({ data, }: SuccessRateChartProps) { const { t } = useTranslation() + if (data.length === 0) { + return ( + <div className="flex h-[300px] items-center justify-center text-muted-foreground"> + {t('pages.overview.noData')} + </div> + ) + } + const chartData = data.map((item: OverviewStats['timeSeries'][number]) => {backend/src/api/admin/providers.ts (2)
23-30: 考虑复用现有的 schema 类型。
Provider接口与数据库 schema 中的类型可能存在重复。可以考虑从@/db/schema导入现有类型，或使用Pick/Omit从已有类型派生，以保持类型定义的单一来源。
70-70: 硬编码的模型名称可能会过时。
claude-3-haiku-20240307是一个特定版本的模型，未来可能被废弃或重命名。建议将其提取为常量或配置项，便于后续维护更新。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/src/api/admin/providers.tsfrontend/src/pages/overview/charts/model-distribution.tsxfrontend/src/pages/overview/charts/success-rate-chart.tsxfrontend/src/styles.css
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/styles.css
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/pages/overview/charts/success-rate-chart.tsx (2)
frontend/src/pages/overview/use-overview-stats.ts (1)
OverviewStats(25-25)frontend/src/pages/overview/charts/chart-styles.ts (3)
tooltipContentStyle(7-13)tooltipLabelStyle(18-22)tooltipItemStyle(27-29)
🔇 Additional comments (5)
frontend/src/pages/overview/charts/model-distribution.tsx (1)
47-73: 图表渲染逻辑清晰，实现正确。组件正确使用了
ResponsiveContainer实现响应式布局，Pie组件配置合理，标签格式化处理了null情况。memo包装有助于性能优化。frontend/src/pages/overview/charts/success-rate-chart.tsx (1)
36-72: 图表渲染实现合理。LineChart 配置正确，XAxis 和 Tooltip 的时间格式化清晰，YAxis 的 domain 设置为 [0, 100] 确保百分比显示合理。使用共享的 tooltip 样式保持了视觉一致性。
backend/src/api/admin/providers.ts (3)
96-132: 实现正确，错误处理逻辑符合预期。函数正确使用了
isModelEndpointUnavailable辅助函数区分端点不可用和真正的连接错误，并将后者重新抛出由外层处理。
156-170: 策略模式实现清晰，符合之前的重构建议。
providerTestHandlersmap 和getProviderTestFn选择器的实现干净利落，使代码更易于扩展和维护。新增 provider 类型时只需在 map 中添加对应的测试函数即可。
310-318: 测试端点正确集成了策略模式。路由处理器通过
getProviderTestFn获取对应的测试函数，代码简洁且易于扩展。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const response = await fetch(`${baseUrl}/messages`, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| "anthropic-version": provider.apiVersion || "2023-06-01", | ||
| ...(provider.apiKey && { "x-api-key": provider.apiKey }), | ||
| }, | ||
| body: JSON.stringify({ | ||
| model: "claude-3-haiku-20240307", // Use a common model for testing | ||
| messages: [{ role: "user", content: "Hi" }], | ||
| max_tokens: 1, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
缺少请求超时设置，可能导致连接测试无限挂起。
fetch 调用没有设置超时，如果目标服务器无响应，请求可能会无限期挂起，影响用户体验和系统资源。
🔧 建议添加 AbortController 实现超时
async function testAnthropicConnection(
provider: Provider,
): Promise<ProviderTestResult> {
const baseUrl = provider.baseUrl.endsWith("/")
? provider.baseUrl.slice(0, -1)
: provider.baseUrl;
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 10000); // 10 second timeout
+
+ try {
const response = await fetch(`${baseUrl}/messages`, {
method: "POST",
headers: {
"Content-Type": "application/json",
"anthropic-version": provider.apiVersion || "2023-06-01",
...(provider.apiKey && { "x-api-key": provider.apiKey }),
},
body: JSON.stringify({
model: "claude-3-haiku-20240307",
messages: [{ role: "user", content: "Hi" }],
max_tokens: 1,
}),
+ signal: controller.signal,
});
+ } finally {
+ clearTimeout(timeoutId);
+ }| if (!response.ok) { | ||
| const text = await response.text(); | ||
| // Check if the error is just about invalid model (which means auth is working) | ||
| if (response.status === 400 && text.includes("model")) { | ||
| return { | ||
| success: true, | ||
| message: "Connection successful (API key valid)", | ||
| models: [], | ||
| }; | ||
| } | ||
| throw new Error(`API error: ${response.status} ${text}`); |
There was a problem hiding this comment.
错误检测逻辑较为脆弱。
第 79 行通过检查 text.includes("model") 来判断认证是否成功。这种方式过于宽泛，任何包含 "model" 字样的 400 错误都会被误判为认证成功。建议使用更精确的错误码或错误类型匹配。
🔧 建议更精确的错误检测
if (!response.ok) {
const text = await response.text();
- // Check if the error is just about invalid model (which means auth is working)
- if (response.status === 400 && text.includes("model")) {
+ // Check for specific Anthropic error types that indicate auth is working
+ // Error types: invalid_request_error with model-related messages
+ if (response.status === 400) {
+ try {
+ const errorBody = JSON.parse(text);
+ if (errorBody.error?.type === "invalid_request_error" &&
+ errorBody.error?.message?.toLowerCase().includes("model")) {
+ return {
+ success: true,
+ message: "Connection successful (API key valid)",
+ models: [],
+ };
+ }
+ } catch {
+ // If JSON parsing fails, fall through to throw
+ }
+ }
- return {
- success: true,
- message: "Connection successful (API key valid)",
- models: [],
- };
- }
throw new Error(`API error: ${response.status} ${text}`);
}📝 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.
| if (!response.ok) { | |
| const text = await response.text(); | |
| // Check if the error is just about invalid model (which means auth is working) | |
| if (response.status === 400 && text.includes("model")) { | |
| return { | |
| success: true, | |
| message: "Connection successful (API key valid)", | |
| models: [], | |
| }; | |
| } | |
| throw new Error(`API error: ${response.status} ${text}`); | |
| if (!response.ok) { | |
| const text = await response.text(); | |
| // Check for specific Anthropic error types that indicate auth is working | |
| // Error types: invalid_request_error with model-related messages | |
| if (response.status === 400) { | |
| try { | |
| const errorBody = JSON.parse(text); | |
| if (errorBody.error?.type === "invalid_request_error" && | |
| errorBody.error?.message?.toLowerCase().includes("model")) { | |
| return { | |
| success: true, | |
| message: "Connection successful (API key valid)", | |
| models: [], | |
| }; | |
| } | |
| } catch { | |
| // If JSON parsing fails, fall through to throw | |
| } | |
| } | |
| throw new Error(`API error: ${response.status} ${text}`); | |
| } |
| } catch (e) { | ||
| const error = e as Error & { status?: number }; | ||
|
|
||
| // For openai-responses, the /models endpoint might not be available | ||
| if ( | ||
| provider.type === "openai-responses" && | ||
| isModelEndpointUnavailable(error) | ||
| ) { | ||
| return status(400, { | ||
| error: "Models list endpoint not available for this provider. Please configure models manually.", | ||
| unsupported: true, | ||
| }); | ||
| } | ||
|
|
||
| return status(502, { | ||
| error: e instanceof Error ? e.message : "Unknown error", | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "isModelEndpointUnavailable" --type ts -B 2 -A 2Repository: EM-GeekLab/NexusGate
Length of output: 1122
🏁 Script executed:
# Also search for the definition of isModelEndpointUnavailable
rg -n "isModelEndpointUnavailable" --type tsRepository: EM-GeekLab/NexusGate
Length of output: 351
🏁 Script executed:
# Check the broader context around the provider handling
fd "providers.ts" --type fRepository: EM-GeekLab/NexusGate
Length of output: 177
🏁 Script executed:
cat -n backend/src/api/admin/providers.ts | sed -n '36,50p'Repository: EM-GeekLab/NexusGate
Length of output: 661
🏁 Script executed:
cat -n backend/src/api/admin/providers.ts | sed -n '115,135p'Repository: EM-GeekLab/NexusGate
Length of output: 745
🏁 Script executed:
cat -n backend/src/api/admin/providers.ts | sed -n '330,390p'Repository: EM-GeekLab/NexusGate
Length of output: 2253
🏁 Script executed:
# Let me check what provider types are defined
rg -n "provider.type" backend/src/api/admin/providers.ts | head -20Repository: EM-GeekLab/NexusGate
Length of output: 347
🏁 Script executed:
# Check for provider type definitions/constants
fd "provider" --type f | grep -E "(constant|type|enum)" | head -10Repository: EM-GeekLab/NexusGate
Length of output: 99
🏁 Script executed:
# Look at the broader test function to understand the pattern
cat -n backend/src/api/admin/providers.ts | sed -n '100,135p'Repository: EM-GeekLab/NexusGate
Length of output: 1243
🏁 Script executed:
# Look at the test function for standard OpenAI providers
cat -n backend/src/api/admin/providers.ts | sed -n '135,180p'Repository: EM-GeekLab/NexusGate
Length of output: 1694
🏁 Script executed:
# Check the getProviderTestFn to understand which test function is used for each provider type
cat -n backend/src/api/admin/providers.ts | sed -n '155,200p'Repository: EM-GeekLab/NexusGate
Length of output: 1604
🏁 Script executed:
# Let me check if there are any comments or documentation about why openai-responses is treated specially
cat -n backend/src/api/admin/providers.ts | sed -n '1,100p'Repository: EM-GeekLab/NexusGate
Length of output: 3580
🏁 Script executed:
# Check what provider types exist
rg -n "ProviderTypeEnum" --type ts -A 15Repository: EM-GeekLab/NexusGate
Length of output: 6184
🏁 Script executed:
# Let me check if there's any documentation about why only openai-responses gets this special treatment
cat -n backend/src/api/admin/providers.ts | sed -n '96,100p'Repository: EM-GeekLab/NexusGate
Length of output: 266
🏁 Script executed:
# Check the database schema to understand the intent
cat -n backend/src/db/schema.ts | sed -n '156,165p'Repository: EM-GeekLab/NexusGate
Length of output: 557
其他 OpenAI 兼容 provider 的 /models 端点不可用时会返回 502 错误。
当前仅对 openai-responses 和 anthropic 类型进行了端点不可用的特殊处理（返回 400）。如果其他 OpenAI 兼容 provider（如 openai、azure、ollama）的 /models 端点不可用或返回 404/405，将返回 502 错误而非更友好的 400 提示。建议统一处理所有 OpenAI 兼容 provider 的端点不可用情况，或在代码注释中说明为何某些 provider 被豁免。
🤖 Prompt for AI Agents
In `@backend/src/api/admin/providers.ts` around lines 361 - 377, Current logic
only treats provider.type === "openai-responses" and "anthropic" specially when
isModelEndpointUnavailable(error) is true, causing other OpenAI-compatible
providers to return 502; change the conditional to detect all OpenAI-compatible
providers (e.g., using or adding a helper like isOpenAICompatible(provider.type)
that returns true for "openai", "azure", "ollama", "openai-responses", etc.)
and, when isModelEndpointUnavailable(error) is true, return status(400,
{...unsupported:true}) instead of falling through to the status(502) branch;
alternatively, if any providers must be exempted, add a clear code comment
explaining why those types are excluded and explicitly list them in the
condition.
| const COLORS = [ | ||
| 'var(--color-chart-1)', | ||
| 'var(--color-chart-2)', | ||
| 'var(--color-chart-3)', | ||
| 'var(--color-chart-4)', | ||
| 'var(--color-chart-5)', | ||
| 'var(--color-chart-6)', | ||
| 'var(--color-chart-7)', | ||
| 'var(--color-chart-8)', | ||
| 'var(--color-chart-9)', | ||
| 'var(--color-chart-10)', | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 首先检查success-rate-chart.tsx是否存在并查看其颜色变量使用方式
find . -type f -name "success-rate-chart.tsx" 2>/dev/nullRepository: EM-GeekLab/NexusGate
Length of output: 123
🏁 Script executed:
# 搜索所有图表组件中的颜色变量使用
rg "var\(--[a-z\-]*chart" --type ts --type tsx -nRepository: EM-GeekLab/NexusGate
Length of output: 91
🏁 Script executed:
# 查找CSS变量定义
rg "--color-chart-|--chart-" --type css --type scss -nRepository: EM-GeekLab/NexusGate
Length of output: 512
🏁 Script executed:
# 检查目标文件的颜色数组定义
cat frontend/src/pages/overview/charts/model-distribution.tsx | head -30Repository: EM-GeekLab/NexusGate
Length of output: 922
🏁 Script executed:
# 查看success-rate-chart.tsx中的颜色变量使用
cat ./frontend/src/pages/overview/charts/success-rate-chart.tsx | head -50Repository: EM-GeekLab/NexusGate
Length of output: 1989
🏁 Script executed:
# 使用正确的rg语法搜索颜色变量使用
rg "var\(--.*chart" -nRepository: EM-GeekLab/NexusGate
Length of output: 2501
🏁 Script executed:
# 查找CSS文件中的颜色变量定义
fd -e css -e scss | xargs grep -l "chart" 2>/dev/nullRepository: EM-GeekLab/NexusGate
Length of output: 87
统一图表组件的 CSS 变量引用方式
虽然颜色变量已正确定义，但 model-distribution.tsx 使用 var(--color-chart-X) 方式不同于其他图表组件。success-rate-chart.tsx、requests-trend-chart.tsx、token-usage-chart.tsx 和 latency-chart.tsx 都统一使用 hsl(var(--chart-X)) 格式。建议调整此文件的 COLORS 数组以匹配项目中其他图表组件的做法，改为 'hsl(var(--chart-1))' 等格式，确保代码风格一致。
🤖 Prompt for AI Agents
In `@frontend/src/pages/overview/charts/model-distribution.tsx` around lines 12 -
23, Update the COLORS constant in model-distribution.tsx to use the same HSL CSS
variable pattern as other chart components: replace each 'var(--color-chart-N)'
entry in the COLORS array with the corresponding 'hsl(var(--chart-N))' form
(e.g., change COLORS entries to 'hsl(var(--chart-1))', 'hsl(var(--chart-2))',
etc.) so the file matches the style used by success-rate-chart.tsx,
requests-trend-chart.tsx, token-usage-chart.tsx, and latency-chart.tsx.
Keep strategy pattern refactoring for provider tests and isModelEndpointUnavailable helper function from feature branch. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@backend/src/api/admin/providers.ts`:
- Around line 134-154: The testDefaultOpenAIConnection function doesn't handle
the case where the /models endpoint is unavailable (client.models.list), causing
a 502 to bubble up; update testDefaultOpenAIConnection to mirror the error
handling in testOpenAIResponsesConnection by wrapping the client.models.list
call in a try/catch, detect endpoint-unavailable errors and return a
ProviderTestResult with success: false and a clear reason/message (and any
relevant error details) instead of throwing; ensure you reference the OpenAI
instance creation and the client.models.list call so the catch covers that
operation and returns the standardized failure shape used elsewhere.
In `@backend/src/db/index.ts`:
- Line 1116: The interval construction currently injects rangeSeconds with
sql.raw (sql`${schema.CompletionsTable.createdAt} >= NOW() - INTERVAL
'${sql.raw(String(rangeSeconds))} seconds'`); replace this with a parameterized
expression that binds rangeSeconds instead of raw-inserting it — e.g. pass
rangeSeconds as a bound parameter and build the INTERVAL on the SQL side (or use
make_interval) so the predicate still compares schema.CompletionsTable.createdAt
to NOW() - interval but without using sql.raw(String(rangeSeconds)).
- Line 1139: 在使用 EmbeddingsTable.createdAt 的过滤表达式中不要把 rangeSeconds 通过 sql.raw
直接插入；把 rangeSeconds 强制为整数并作为参数绑定传入，然后在 SQL 里使用参数化的间隔构造（例如把数字参数拼接到 "seconds"
单位的表达式中），替换掉 sql`${schema.EmbeddingsTable.createdAt} >= NOW() - INTERVAL
'${sql.raw(String(rangeSeconds))} seconds'`，确保只传入经验证的数字参数 rangeSeconds
而不是原始字符串以防注入。
♻️ Duplicate comments (7)
backend/src/db/index.ts (4)
1157-1183: 避免使用原始 SQL，改用 Drizzle ORM 查询构建器此处使用
db.execute()执行原始 SQL，降低了类型安全性和可维护性。同时sql.raw()也存在注入风险。建议重构为使用 Drizzle ORM 的查询构建器，可以利用
sql模板实现时间桶分组：♻️ 建议的重构方案
export async function getCompletionsTimeSeries( rangeSeconds: number, bucketSeconds: number, ) { logger.debug("getCompletionsTimeSeries", rangeSeconds, bucketSeconds); - const result = await db.execute(sql` - SELECT - to_timestamp(floor(extract(epoch from created_at) / ${bucketSeconds}) * ${bucketSeconds}) AS bucket, - COUNT(*) AS total, - SUM(CASE WHEN status = 'completed' THEN 1 ELSE 0 END) AS completed, - SUM(CASE WHEN status = 'failed' THEN 1 ELSE 0 END) AS failed, - COALESCE(AVG(CASE WHEN duration > 0 THEN duration END), 0) AS avg_duration, - COALESCE(AVG(CASE WHEN status = 'completed' AND ttft > 0 THEN ttft END), 0) AS avg_ttft - FROM completions - WHERE deleted = false - AND created_at >= NOW() - INTERVAL '${sql.raw(String(rangeSeconds))} seconds' - GROUP BY bucket - ORDER BY bucket ASC - `); - // ... - return result as unknown as { ... }[]; + const bucketExpr = sql`to_timestamp(floor(extract(epoch from ${schema.CompletionsTable.createdAt}) / ${bucketSeconds}) * ${bucketSeconds})`; + return await db + .select({ + bucket: bucketExpr, + total: count(schema.CompletionsTable.id), + completed: sql<number>`SUM(CASE WHEN ${schema.CompletionsTable.status} = 'completed' THEN 1 ELSE 0 END)::int`, + failed: sql<number>`SUM(CASE WHEN ${schema.CompletionsTable.status} = 'failed' THEN 1 ELSE 0 END)::int`, + avgDuration: sql<number>`COALESCE(AVG(CASE WHEN ${schema.CompletionsTable.duration} > 0 THEN ${schema.CompletionsTable.duration} END), 0)`, + avgTtft: sql<number>`COALESCE(AVG(CASE WHEN ${schema.CompletionsTable.status} = 'completed' AND ${schema.CompletionsTable.ttft} > 0 THEN ${schema.CompletionsTable.ttft} END), 0)`, + }) + .from(schema.CompletionsTable) + .where( + and( + not(schema.CompletionsTable.deleted), + sql`${schema.CompletionsTable.createdAt} >= NOW() - (${rangeSeconds} * INTERVAL '1 second')`, + ), + ) + .groupBy(bucketExpr) + .orderBy(asc(bucketExpr)); }
1195-1219: 同样需要重构为 ORM 查询构建器与
getCompletionsTimeSeries相同，建议使用 Drizzle ORM 查询构建器替代原始 SQL，以提升类型安全性和可维护性。
1055-1055: SQL 注入风险：避免使用sql.raw()处理用户输入
sql.raw(String(rangeSeconds))绕过了参数化查询的保护。虽然 TypeScript 将rangeSeconds类型定义为number，但在运行时无法保证类型安全。如果上游调用方传入未经验证的输入，可能导致 SQL 注入。建议使用参数化的方式构建 INTERVAL：
🔒 建议的修复方案
- sql`${schema.CompletionsTable.createdAt} >= NOW() - INTERVAL '${sql.raw(String(rangeSeconds))} seconds'`, + sql`${schema.CompletionsTable.createdAt} >= NOW() - (${rangeSeconds} * INTERVAL '1 second')`,
1088-1088: 同样的 SQL 注入风险与
getCompletionsStats相同的问题，建议统一修复：🔒 建议的修复方案
- sql`${schema.EmbeddingsTable.createdAt} >= NOW() - INTERVAL '${sql.raw(String(rangeSeconds))} seconds'`, + sql`${schema.EmbeddingsTable.createdAt} >= NOW() - (${rangeSeconds} * INTERVAL '1 second')`,backend/src/api/admin/providers.ts (3)
62-74: 缺少请求超时设置，可能导致连接测试无限挂起。
fetch调用没有设置超时，如果目标服务器无响应，请求可能会无限期挂起。建议使用AbortController添加超时机制：🔧 建议的修改
async function testAnthropicConnection( provider: Provider, ): Promise<ProviderTestResult> { const baseUrl = provider.baseUrl.endsWith("/") ? provider.baseUrl.slice(0, -1) : provider.baseUrl; + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); + + try { const response = await fetch(`${baseUrl}/messages`, { method: "POST", headers: { "Content-Type": "application/json", "anthropic-version": provider.apiVersion || "2023-06-01", ...(provider.apiKey && { "x-api-key": provider.apiKey }), }, body: JSON.stringify({ model: "claude-3-haiku-20240307", messages: [{ role: "user", content: "Hi" }], max_tokens: 1, }), + signal: controller.signal, }); + // ... rest of the function + } finally { + clearTimeout(timeoutId); + }
76-86: 错误检测逻辑较为脆弱。第 79 行通过
text.includes("model")判断认证是否成功过于宽泛。建议解析 JSON 响应并检查 Anthropic 特定的错误类型：🔧 建议更精确的错误检测
if (!response.ok) { const text = await response.text(); - if (response.status === 400 && text.includes("model")) { - return { - success: true, - message: "Connection successful (API key valid)", - models: [], - }; - } + if (response.status === 400) { + try { + const errorBody = JSON.parse(text); + if (errorBody.error?.type === "invalid_request_error" && + errorBody.error?.message?.toLowerCase().includes("model")) { + return { + success: true, + message: "Connection successful (API key valid)", + models: [], + }; + } + } catch { + // JSON 解析失败则继续抛出错误 + } + } throw new Error(`API error: ${response.status} ${text}`); }
365-373: 其他 OpenAI 兼容 provider 的/models端点不可用时会返回 502 错误。当前仅对
openai-responses类型进行了端点不可用的特殊处理。如果openai、azure、ollama等 provider 的/models端点不可用或返回 404/405，将返回 502 错误而非更友好的 400 提示。建议统一处理所有可能的端点不可用情况：
🔧 建议的修改
} catch (e) { const error = e as Error & { status?: number }; - // For openai-responses, the /models endpoint might not be available - if ( - provider.type === "openai-responses" && - isModelEndpointUnavailable(error) - ) { + // Handle cases where /models endpoint is not available + if (isModelEndpointUnavailable(error)) { return status(400, { error: "Models list endpoint not available for this provider. Please configure models manually.", unsupported: true, }); } return status(502, { error: e instanceof Error ? e.message : "Unknown error", }); }
🧹 Nitpick comments (3)
backend/src/db/index.ts (1)
1039-1040: 建议添加输入参数验证所有统计函数都接受
rangeSeconds和bucketSeconds参数，但没有验证其有效性。建议在函数入口处添加验证：
rangeSeconds <= 0会导致查询无数据或意外行为bucketSeconds <= 0在时间序列函数中会导致除零错误🛡️ 建议的验证逻辑
// 可以创建一个辅助函数用于参数验证 function validateStatsParams(rangeSeconds: number, bucketSeconds?: number) { if (rangeSeconds <= 0) { throw new Error("rangeSeconds must be positive"); } if (bucketSeconds !== undefined && bucketSeconds <= 0) { throw new Error("bucketSeconds must be positive"); } }backend/src/api/admin/providers.ts (2)
17-32: 接口定义清晰，策略模式设计良好。
ProviderTestResult和ProviderTestFn类型定义清晰，策略模式的引入使得针对不同 provider 的测试逻辑更易维护和扩展。考虑一个小改进：本地
Provider接口与@/db中的Provider类型存在重复定义。可以考虑直接复用数据库导出的类型，使用Pick选取所需字段：import type { Provider as DbProvider } from "@/db"; type Provider = Pick<DbProvider, 'id' | 'name' | 'type' | 'baseUrl' | 'apiKey' | 'apiVersion'>;这样可以确保类型定义与数据库 schema 保持同步。
103-106: 可选：为 OpenAI 客户端配置超时以加快测试端点反馈速度。OpenAI SDK 默认超时为 10 分钟，但对于连接测试场景，可考虑设置更短的超时时间以提供更快的反馈：
const client = new OpenAI({ baseURL: provider.baseUrl, apiKey: provider.apiKey || "not-required", timeout: 10000, // 10 seconds });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
backend/src/api/admin/providers.tsbackend/src/db/index.tsfrontend/package.jsonfrontend/src/i18n/locales/en-US.jsonfrontend/src/i18n/locales/zh-CN.json
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/api/admin/providers.ts (3)
backend/src/db/index.ts (1)
Provider(41-41)frontend/src/pages/settings/providers-columns.tsx (1)
Provider(11-21)backend/src/db/schema.ts (1)
ProviderTypeEnumType(166-166)
🔇 Additional comments (10)
frontend/src/i18n/locales/zh-CN.json (3)
11-11: LGTM!侧边栏"Overview"的翻译"概览"准确且符合现有命名规范。
350-358: LGTM!翻译准确，且正确使用了中文引号「」。错误消息的表述与文件中现有模式一致。
359-385: LGTM!Overview 页面的翻译完整且准确。技术术语（如 Token、TTFT）保留英文是中文技术界面的常见做法。时间单位翻译正确，"向量化"与文件中其他位置的用法保持一致。
frontend/src/i18n/locales/en-US.json (3)
11-11: LGTM!新增的侧边栏 Overview 条目遵循现有的命名规范，与其他侧边栏项保持一致。
349-352: LGTM!新增的错误处理和状态消息遵循现有的命名规范，提供了清晰的用户提示：
- 告知用户某些 provider 类型不支持自动获取模型列表
- 引导用户使用手动添加方式
- 提供重试选项
这些改进增强了用户体验。
357-384: LGTM!Overview 页面的国际化键结构设计合理：
- 时间范围选项使用简洁的格式 (1m, 5m, 10m 等)
- 指标分组清晰 (metrics, charts, tokens)
- 为 avgLatency 和 avgTTFT 提供了描述性文本 (avgLatencyDesc, avgTTFTDesc)，有助于用户理解
键命名与现有模式保持一致，翻译准确。
backend/src/api/admin/providers.ts (4)
34-49: 辅助函数提取合理，解决了代码重复问题。
isModelEndpointUnavailable提取了检测端点不可用的逻辑，在测试函数和/remote-models路由中复用，符合 DRY 原则。
100-132: 错误处理逻辑正确。
testOpenAIResponsesConnection正确区分了端点不可用（返回成功状态）和真正的连接错误（重新抛出异常），符合预期行为。
156-170: 策略模式实现简洁且易于扩展。使用
providerTestHandlersmap 和getProviderTestFn选择器实现的策略模式清晰简洁，新增 provider 类型时只需在 map 中添加对应的测试函数即可。
310-318: 测试路由集成简洁。使用
getProviderTestFn动态获取测试函数的方式使路由逻辑更加简洁，符合开闭原则。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /** | ||
| * Test standard OpenAI-compatible provider connection (openai, azure, ollama). | ||
| * Uses the /models endpoint to verify connection and list available models. | ||
| */ | ||
| async function testDefaultOpenAIConnection( | ||
| provider: Provider, | ||
| ): Promise<ProviderTestResult> { | ||
| const client = new OpenAI({ | ||
| baseURL: provider.baseUrl, | ||
| apiKey: provider.apiKey || "not-required", | ||
| }); | ||
|
|
||
| const models = await client.models.list(); | ||
| return { | ||
| success: true, | ||
| models: models.data.map((m) => ({ | ||
| id: m.id, | ||
| owned_by: m.owned_by, | ||
| })), | ||
| }; | ||
| } |
There was a problem hiding this comment.
标准 OpenAI 兼容 provider 未处理端点不可用情况。
testDefaultOpenAIConnection 用于 openai、azure、ollama 等 provider，但未像 testOpenAIResponsesConnection 那样处理 /models 端点不可用的情况。如果这些 provider 的 /models 端点不可用，会直接返回 502 错误。
建议统一处理逻辑：
🔧 建议的修改
async function testDefaultOpenAIConnection(
provider: Provider,
): Promise<ProviderTestResult> {
const client = new OpenAI({
baseURL: provider.baseUrl,
apiKey: provider.apiKey || "not-required",
});
- const models = await client.models.list();
- return {
- success: true,
- models: models.data.map((m) => ({
- id: m.id,
- owned_by: m.owned_by,
- })),
- };
+ try {
+ const models = await client.models.list();
+ return {
+ success: true,
+ models: models.data.map((m) => ({
+ id: m.id,
+ owned_by: m.owned_by,
+ })),
+ };
+ } catch (e) {
+ const error = e as Error & { status?: number };
+ if (isModelEndpointUnavailable(error)) {
+ return {
+ success: true,
+ message: "Connection configured (models endpoint not available)",
+ models: [],
+ };
+ }
+ throw e;
+ }
}🤖 Prompt for AI Agents
In `@backend/src/api/admin/providers.ts` around lines 134 - 154, The
testDefaultOpenAIConnection function doesn't handle the case where the /models
endpoint is unavailable (client.models.list), causing a 502 to bubble up; update
testDefaultOpenAIConnection to mirror the error handling in
testOpenAIResponsesConnection by wrapping the client.models.list call in a
try/catch, detect endpoint-unavailable errors and return a ProviderTestResult
with success: false and a clear reason/message (and any relevant error details)
instead of throwing; ensure you reference the OpenAI instance creation and the
client.models.list call so the catch covers that operation and returns the
standardized failure shape used elsewhere.
| .where( | ||
| and( | ||
| not(schema.CompletionsTable.deleted), | ||
| sql`${schema.CompletionsTable.createdAt} >= NOW() - INTERVAL '${sql.raw(String(rangeSeconds))} seconds'`, |
There was a problem hiding this comment.
同样需要修复 sql.raw() 问题
请应用与上述相同的参数化修复方案。
🤖 Prompt for AI Agents
In `@backend/src/db/index.ts` at line 1116, The interval construction currently
injects rangeSeconds with sql.raw (sql`${schema.CompletionsTable.createdAt} >=
NOW() - INTERVAL '${sql.raw(String(rangeSeconds))} seconds'`); replace this with
a parameterized expression that binds rangeSeconds instead of raw-inserting it —
e.g. pass rangeSeconds as a bound parameter and build the INTERVAL on the SQL
side (or use make_interval) so the predicate still compares
schema.CompletionsTable.createdAt to NOW() - interval but without using
sql.raw(String(rangeSeconds)).
| .where( | ||
| and( | ||
| not(schema.EmbeddingsTable.deleted), | ||
| sql`${schema.EmbeddingsTable.createdAt} >= NOW() - INTERVAL '${sql.raw(String(rangeSeconds))} seconds'`, |
There was a problem hiding this comment.
同样需要修复 sql.raw() 问题
请应用与上述相同的参数化修复方案。
🤖 Prompt for AI Agents
In `@backend/src/db/index.ts` at line 1139, 在使用 EmbeddingsTable.createdAt
的过滤表达式中不要把 rangeSeconds 通过 sql.raw 直接插入；把 rangeSeconds 强制为整数并作为参数绑定传入，然后在 SQL
里使用参数化的间隔构造（例如把数字参数拼接到 "seconds" 单位的表达式中），替换掉
sql`${schema.EmbeddingsTable.createdAt} >= NOW() - INTERVAL
'${sql.raw(String(rangeSeconds))} seconds'`，确保只传入经验证的数字参数 rangeSeconds
而不是原始字符串以防注入。
2 participants
Summary
Changes
New Features
Bug Fixes
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
版本发布说明
新功能
改进
✏️ Tip: You can customize this high-level summary in your review settings.