Conversation
问题：Date 对象在 Server Action 传输时被序列化为 UTC， 后端使用服务器时区解析导致时间偏移，当天凌晨记录查不到。 修复：改用字符串直接传递本地时间，避免 Date 序列化的时区问题。 closes #198 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
用户输入分组名称后直接点击保存按钮，输入值不会被添加到标签数组中。 现在添加 onBlur 处理器，在输入框失焦时自动提交待处理的输入值。 Fixes #201 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- 新增系统设置选项，允许配置模型重定向时的计费模型来源 - 支持两种模式：重定向前（原始模型，默认）和重定向后（实际模型） - 使用记录表"模型"列改为"计费模型"列，显示实际计费模型 - 请求详情对话框中添加当前计费模式说明 - 修复 Drizzle 快照链冲突问题（0023/0024 快照父引用错误） - 完整支持 5 种语言的 i18n 翻译（zh-CN, en, ja, ru, zh-TW） 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- 恢复 0022_simple_stardust.sql 迁移文件（枚举类型定义） - 恢复 0022_snapshot.json 快照文件 - 修复 _journal.json 中的迁移条目顺序（idx 22-26） - 修复 0023_snapshot.json 的 prevId 指向正确的父快照 - 删除孤儿文件 0023_daily_limit_partial_indexes.sql 修复后的迁移链： idx 22: 0022_simple_stardust (枚举类型) idx 23: 0023_cheerful_shocker (超时默认值) idx 24: 0023_safe_christian_walker (MCP passthrough) idx 25: 0024_update_provider_timeout_defaults (超时配置) idx 26: 0025_hard_violations (billing_model_source) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
问题：0025_hard_violations 的 when 时间戳 (1764128773876) 小于 0024 的时间戳 (1764206400000)，导致 Drizzle 按时间 顺序执行时跳过了这个迁移。 修复：将 0025 的 when 值改为 1764300000000，确保它在 0024 之后执行。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
问题： - 两个 0023_* 文件导致编号冲突 - journal 时间戳乱序 - 多次手动修改导致迁移链不一致 解决方案： - 删除冲突迁移文件 (0020-0025) - 使用 db:generate 重新生成单一迁移 0020_glossy_grandmaster - 迁移内 SQL 使用幂等语法 (IF NOT EXISTS, DO EXCEPTION) - 迁移内包含一次性清理旧记录的逻辑 兼容性： - 新安装：正常执行所有迁移 - 从 0019 升级：执行新迁移 - 从冲突版本升级：清理旧记录 + 幂等迁移 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
问题：0025_hard_violations 的 when 时间戳 (1764128773876) 小于 0024 的时间戳 (1764206400000)，导致 Drizzle 按时间 顺序执行时跳过了这个迁移。 修复：将 0025 的 when 值改为 1764300000000，确保它在 0024 之后执行。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- 修复 i18n 参数不匹配: billingDescription 使用 {original}/{current} 替代 {model}
- 修复使用记录表格中重定向标记被 truncate 隐藏的问题
- 简化 error-details-dialog 中的模型重定向 UI: 从三列网格改为紧凑单行设计
- 新增 billingOriginal/billingRedirected i18n 键用于计费标识
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
问题：当请求重试并切换供应商后，统计仍归属到首次选择的失败供应商 修复： - updateMessageRequestDetails 支持更新 providerId 字段 - 请求成功后更新 provider_id 到最终处理的供应商 - 统计查询使用 providerChain 最后一项（兼容历史数据） Closes #204 🤖 Generated with [Claude Code](https://claude.ai/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
辅助端点失败不应影响供应商健康状态，避免并发 count_tokens 请求 导致供应商被误熔断的问题。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
问题：翻译键路径不匹配导致返回键本身字符串，Badge 显示超长文本挤压模型名
修复：
- t("modelRedirect.redirected") → t("logs.modelRedirect.redirected")
- t("modelRedirect.actualModelTooltip") → t("logs.details.modelRedirect.actualModelTooltip")
- t("modelRedirect.originalModelTooltip") → t("logs.details.modelRedirect.originalModelTooltip")
🤖 Generated with [Claude Code](https://claude.ai/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
- 移除重定向 Badge 中的"已重定向"文字，只保留箭头图标 - 移除有问题的 i18n Tooltip - 点击图标可打开请求详情弹窗并自动滚动到重定向部分 - ErrorDetailsDialog 支持外部控制打开状态 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- 移除未使用的 isError 变量 (error-details-dialog.tsx) - 移除未使用的 getActiveSessions 导入 (dashboard-realtime.ts) - 移除未使用的 Layers 导入 (big-screen/page.tsx) - 修复 CountUp 组件 useEffect 依赖警告，使用 ref 跟踪前值 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
当供应商 A 配置了模型重定向后失败，切换到供应商 B 时， 系统错误地使用供应商 A 的重定向模型名请求供应商 B。 修复方案： - ModelRedirector.apply() 使用 getOriginalModel() 获取真正的原始模型 - 当新供应商没有对应重定向规则时，自动重置到原始模型 - 新增 resetToOriginal() 方法处理模型和 Gemini URL 路径重置 - session.ts 新增 originalUrlPathname 字段支持 Gemini 路径重置 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
旧正则 `cache_control.*limit.*blocks` 无法匹配 Anthropic API 返回的错误 "A maximum of 4 blocks with cache_control may be provided"，因为消息中 blocks 在 cache_control 之前，且使用 maximum 而非 limit。 新正则支持两种顺序和两个关键词变体。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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 addresses several critical bug fixes and introduces new features to enhance the system's stability, accuracy, and configurability. Key improvements include a consolidated and idempotent database migration, the introduction of a configurable billing model source to manage costs more flexibly, and significant refinements to the proxy's model redirection and error handling logic. Additionally, it improves the accuracy of usage statistics and streamlines date filtering in the dashboard. 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 5,568 lines changed across 51 files. 📋 Analysis BreakdownMigration Files (bulk of deletions):
Actual Feature/Bug Fix Changes (~39 files):
🔀 Suggested SplitBased on the changes, this PR could be split into:
Why Split?
🤖 Automated analysis by Claude AI |
![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 mix of bug fixes, new features, and significant refactoring. The consolidation of database migrations into a single idempotent file is a good step for maintainability, though it appears to have removed the daily limit feature, which needs clarification. The introduction of a configurable billing model source is a valuable new feature. Most importantly, this PR contains critical fixes for the proxy logic, especially regarding model redirection during provider failover and accurate cost attribution in retry scenarios. The UI/UX has also seen several nice improvements. My review focuses on potential regressions from the migration consolidation and suggests minor improvements for maintainability.
| -- 幂等迁移: 合并 0020-0025 的所有变更 | ||
| -- 此迁移可以在任何状态下安全执行（新安装、从 0019 升级、从冲突版本升级） | ||
|
|
||
| -- Step 0: 清理冲突的旧迁移记录（一次性，仅影响从冲突版本升级的用户） | ||
| DELETE FROM drizzle.__drizzle_migrations WHERE hash IN ( | ||
| '0020_next_juggernaut', | ||
| '0021_daily_cost_limits', | ||
| '0022_simple_stardust', | ||
| '0023_cheerful_shocker', | ||
| '0023_safe_christian_walker', | ||
| '0024_update_provider_timeout_defaults', | ||
| '0025_hard_violations' | ||
| ); | ||
| --> statement-breakpoint | ||
|
|
||
| -- Step 1: 创建枚举类型（幂等） | ||
| DO $$ BEGIN | ||
| CREATE TYPE "public"."daily_reset_mode" AS ENUM('fixed', 'rolling'); | ||
| EXCEPTION | ||
| WHEN duplicate_object THEN null; | ||
| END $$; | ||
| --> statement-breakpoint | ||
|
|
||
| -- Step 2: 更新 providers 表默认值（幂等，无条件安全） | ||
| ALTER TABLE "providers" ALTER COLUMN "first_byte_timeout_streaming_ms" SET DEFAULT 0;--> statement-breakpoint | ||
| ALTER TABLE "providers" ALTER COLUMN "streaming_idle_timeout_ms" SET DEFAULT 0;--> statement-breakpoint | ||
| ALTER TABLE "providers" ALTER COLUMN "request_timeout_non_streaming_ms" SET DEFAULT 0;--> statement-breakpoint | ||
|
|
||
| -- Step 3: 添加 keys 表字段（幂等） | ||
| ALTER TABLE "keys" ADD COLUMN IF NOT EXISTS "daily_reset_mode" "daily_reset_mode" DEFAULT 'fixed' NOT NULL;--> statement-breakpoint | ||
|
|
||
| -- Step 4: 添加 providers 表字段（幂等） | ||
| ALTER TABLE "providers" ADD COLUMN IF NOT EXISTS "mcp_passthrough_type" varchar(20) DEFAULT 'none' NOT NULL;--> statement-breakpoint | ||
| ALTER TABLE "providers" ADD COLUMN IF NOT EXISTS "mcp_passthrough_url" varchar(512);--> statement-breakpoint | ||
| ALTER TABLE "providers" ADD COLUMN IF NOT EXISTS "daily_reset_mode" "daily_reset_mode" DEFAULT 'fixed' NOT NULL;--> statement-breakpoint | ||
|
|
||
| -- Step 5: 添加 system_settings 表字段（幂等） | ||
| ALTER TABLE "system_settings" ADD COLUMN IF NOT EXISTS "billing_model_source" varchar(20) DEFAULT 'original' NOT NULL;--> statement-breakpoint | ||
|
|
||
| -- Step 6: 添加 users 表字段（幂等） | ||
| ALTER TABLE "users" ADD COLUMN IF NOT EXISTS "limit_5h_usd" numeric(10, 2);--> statement-breakpoint | ||
| ALTER TABLE "users" ADD COLUMN IF NOT EXISTS "limit_weekly_usd" numeric(10, 2);--> statement-breakpoint | ||
| ALTER TABLE "users" ADD COLUMN IF NOT EXISTS "limit_monthly_usd" numeric(10, 2);--> statement-breakpoint | ||
| ALTER TABLE "users" ADD COLUMN IF NOT EXISTS "limit_concurrent_sessions" integer; |
There was a problem hiding this comment.
This consolidated migration appears to have removed the limit_daily_usd and daily_reset_time columns from the keys and providers tables. These were part of the now-deleted 0021_daily_cost_limits.sql migration. The associated partial indexes from drizzle/0023_daily_limit_partial_indexes.sql are also missing.
This seems to be an intentional feature removal, but it's a significant change that isn't mentioned in the pull request description. If this was unintentional, it's a critical omission that could lead to loss of functionality. Could you please clarify if this feature is being deprecated? If not, these columns and their indexes should be added back to this migration file.
| -- 幂等迁移: 合并 0020-0025 的所有变更 | ||
| -- 此迁移可以在任何状态下安全执行（新安装、从 0019 升级、从冲突版本升级） | ||
|
|
||
| -- Step 0: 清理冲突的旧迁移记录（一次性，仅影响从冲突版本升级的用户） | ||
| DELETE FROM drizzle.__drizzle_migrations WHERE hash IN ( | ||
| '0020_next_juggernaut', | ||
| '0021_daily_cost_limits', | ||
| '0022_simple_stardust', | ||
| '0023_cheerful_shocker', | ||
| '0023_safe_christian_walker', | ||
| '0024_update_provider_timeout_defaults', | ||
| '0025_hard_violations' | ||
| ); | ||
| --> statement-breakpoint | ||
|
|
||
| -- Step 1: 创建枚举类型（幂等） | ||
| DO $$ BEGIN | ||
| CREATE TYPE "public"."daily_reset_mode" AS ENUM('fixed', 'rolling'); | ||
| EXCEPTION | ||
| WHEN duplicate_object THEN null; | ||
| END $$; | ||
| --> statement-breakpoint | ||
|
|
||
| -- Step 2: 更新 providers 表默认值（幂等，无条件安全） | ||
| ALTER TABLE "providers" ALTER COLUMN "first_byte_timeout_streaming_ms" SET DEFAULT 0;--> statement-breakpoint | ||
| ALTER TABLE "providers" ALTER COLUMN "streaming_idle_timeout_ms" SET DEFAULT 0;--> statement-breakpoint | ||
| ALTER TABLE "providers" ALTER COLUMN "request_timeout_non_streaming_ms" SET DEFAULT 0;--> statement-breakpoint | ||
|
|
||
| -- Step 3: 添加 keys 表字段（幂等） | ||
| ALTER TABLE "keys" ADD COLUMN IF NOT EXISTS "daily_reset_mode" "daily_reset_mode" DEFAULT 'fixed' NOT NULL;--> statement-breakpoint | ||
|
|
||
| -- Step 4: 添加 providers 表字段（幂等） | ||
| ALTER TABLE "providers" ADD COLUMN IF NOT EXISTS "mcp_passthrough_type" varchar(20) DEFAULT 'none' NOT NULL;--> statement-breakpoint | ||
| ALTER TABLE "providers" ADD COLUMN IF NOT EXISTS "mcp_passthrough_url" varchar(512);--> statement-breakpoint | ||
| ALTER TABLE "providers" ADD COLUMN IF NOT EXISTS "daily_reset_mode" "daily_reset_mode" DEFAULT 'fixed' NOT NULL;--> statement-breakpoint | ||
|
|
||
| -- Step 5: 添加 system_settings 表字段（幂等） | ||
| ALTER TABLE "system_settings" ADD COLUMN IF NOT EXISTS "billing_model_source" varchar(20) DEFAULT 'original' NOT NULL;--> statement-breakpoint | ||
|
|
||
| -- Step 6: 添加 users 表字段（幂等） | ||
| ALTER TABLE "users" ADD COLUMN IF NOT EXISTS "limit_5h_usd" numeric(10, 2);--> statement-breakpoint | ||
| ALTER TABLE "users" ADD COLUMN IF NOT EXISTS "limit_weekly_usd" numeric(10, 2);--> statement-breakpoint | ||
| ALTER TABLE "users" ADD COLUMN IF NOT EXISTS "limit_monthly_usd" numeric(10, 2);--> statement-breakpoint | ||
| ALTER TABLE "users" ADD COLUMN IF NOT EXISTS "limit_concurrent_sessions" integer; |
There was a problem hiding this comment.
The data migration logic from the deleted drizzle/0024_update_provider_timeout_defaults.sql file seems to be missing. The old file contained an UPDATE statement to normalize streaming_idle_timeout_ms for existing providers. While this new migration correctly sets the DEFAULT to 0, it doesn't update existing rows, which could leave inconsistent data in the database for existing installations.
Please consider adding the following UPDATE statement to this migration script to ensure data consistency for users who are upgrading:
-- 批量更新流式静默期超时
-- 规则：
-- - 小于 60000ms (60s) 且大于 0 的 → 改为 60000
-- - 等于 0（不限制）的 → 不操作
-- - 大于等于 60000 的 → 不操作
UPDATE "providers"
SET "streaming_idle_timeout_ms" = 60000
WHERE "streaming_idle_timeout_ms" > 0
AND "streaming_idle_timeout_ms" < 60000
AND "deleted_at" IS NULL;
🔒 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. 📋 Detailed AnalysisChanges Reviewed
Scanned Categories
🔍 Specific Findings AnalysisSQL Query Construction (src/repository/usage-logs.ts, src/repository/provider.ts)
Timezone Variable (Environment Config)
Time String Normalization (src/repository/usage-logs.ts)
React Components (UI changes)
🛡️ Security PostureStrong - This PR follows secure coding practices:
🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage |
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary
This PR consolidates multiple migration files (0020-0025) into a single idempotent migration and includes bug fixes for provider statistics, model redirection, and various UI/UX improvements. The changes are generally well-structured, but there are a few issues that should be addressed.
🔍 Issues Found
- Critical (🔴): 0 issues
- High (🟠): 1 issue
- Medium (🟡): 3 issues
- Low (🟢): 0 issues
🎯 Priority Actions
- 🟠 SQL Query Safety (
src/repository/provider.ts): The new SQL query that extractsproviderIdfromprovider_chainJSONB using(provider_chain->-1->>'providerId')::intcould throw an exception if the JSON structure is malformed or theproviderIdfield contains a non-integer value. Consider adding a fallback:
COALESCE(
CASE
WHEN (provider_chain->-1->>'providerId') ~ '^[0-9]+$'
THEN (provider_chain->-1->>'providerId')::int
ELSE NULL
END,
provider_id
)- 🟡 Missing Input Validation (
src/repository/usage-logs.ts:75-84): ThenormalizeLocalTimeStrfunction only validates string length but doesn't validate the actual date format. Malformed date strings could pass through and cause SQL errors.
function normalizeLocalTimeStr(input: string): string {
// Validate format before processing
const dateTimeRegex = /^\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}(:\d{2})?$/;
if (!dateTimeRegex.test(input)) {
throw new Error(`Invalid date format: ${input}`);
}
const normalized = input.replace("T", " ");
if (normalized.length === 16) {
return normalized + ":00";
}
return normalized;
}-
🟡 Migration Cleanup Safety (
drizzle/0020_glossy_grandmaster.sql): The migration deletes records fromdrizzle.__drizzle_migrationsbased on hash values. While the comment indicates this is a one-time cleanup for conflicting versions, there's no safeguard if this migration is accidentally re-run or if the hash values match future migrations. Consider adding a version check or timestamp condition. -
🟡 Incomplete ProviderChain Type Handling (
src/repository/provider.ts): The SQL usesprovider_chain->-1->>'providerId'but the TypeScriptProviderChainItemtype insrc/types/message.tsdefines the field asid, notproviderId. This inconsistency could lead to the query always falling back toprovider_id:
// ProviderChainItem has:
id: number; // The field name is 'id', not 'providerId'The SQL should use provider_chain->-1->>'id' instead.
💡 General Observations
- The consolidation of migrations 0020-0025 into a single idempotent migration is a good approach for handling version conflicts
- The model redirector changes for handling provider switches are well-implemented
- The addition of
billingModelSourcesystem setting provides good configurability for cost calculation
🤖 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.
⚠️ Critical Bug Identified
File: src/repository/provider.ts
The SQL query uses (mr.provider_chain->-1->>'providerId')::int to extract the provider ID from the chain, but the ProviderChainItem interface (in src/types/message.ts) defines the field as id, NOT providerId:
export interface ProviderChainItem {
id: number; // <-- The actual field name
name: string;
// ...
}Impact: The SQL condition (mr.provider_chain->-1->>'providerId')::int = p.id will always fail because providerId doesn't exist in the JSON structure. This means:
- Provider statistics will only count requests where
provider_chainis NULL or empty - All requests with retry/failover chains will be excluded from statistics
- The
latest_callCTE will produce incorrect results
Suggested fix:
Replace all occurrences of ->>'providerId' with ->>'id':
-- Change from:
(mr.provider_chain->-1->>'providerId')::int = p.id
-- To:
(mr.provider_chain->-1->>'id')::int = p.idThis should be fixed in the following lines of src/repository/provider.ts:
- Line 380
- Line 383
- Line 392
- Line 395
- Line 401
合并迁移文件时遗漏了 keys 和 providers 表的每日限额相关字段，导致新安装用户缺少这些列。 添加的字段： - keys.limit_daily_usd: 每日消费限额 - keys.daily_reset_time: 每日重置时间 - providers.limit_daily_usd: 供应商每日限额 - providers.daily_reset_time: 供应商重置时间 使用 IF NOT EXISTS 确保幂等性，所有升级场景都能正常工作。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
📊 PR Size AnalysisThis PR is XL with 5,572 lines changed (574 additions, 4,998 deletions) across 51 files. Large PRs are harder to review and more likely to introduce bugs. 📝 Analysis NotesLooking at the changes, I notice that a significant portion of the deletions (~4,500+ lines) come from:
The actual feature/fix changes appear smaller, but the PR still bundles multiple distinct concerns. 🔀 Suggested SplitBased on the changes, this PR could be split into:
Why Split?
Alternative ApproachIf splitting is not practical, at minimum consider:
🤖 Automated analysis by Claude AI |
🔒 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. Changes AnalyzedThis PR adds comprehensive usage logs functionality with the following security-relevant components:
Scanned Categories
OWASP Top 10 Coverage
🛡️ Security PostureStrong - The codebase follows security best practices:
🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage |
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary
This PR consolidates database migrations (0020-0025) into a single idempotent migration file and adds billing model source configuration feature. The migration consolidation approach is sound.
🔍 Issues Found
- Critical (🔴): 1 issue
- High (🟠): 0 issues
- Medium (🟡): 0 issues
- Low (🟢): 1 issue
🎯 Priority Actions
- Critical: The migration file
0020_glossy_grandmaster.sqlis missing thebilling_model_sourcecolumn addition forsystem_settingstable, but the code (schema.ts, repository files, types) expects this column to exist. This will cause runtime errors.
💡 General Observations
The migration consolidation strategy is well-designed with proper idempotent operations using IF NOT EXISTS. However, the new billing_model_source feature added in this PR requires a corresponding migration entry that is currently missing.
🤖 Automated review by Claude AI - focused on identifying issues for improvement
|
|
||
| -- Step 5: 添加 system_settings 表字段（幂等） | ||
| ALTER TABLE "system_settings" ADD COLUMN IF NOT EXISTS "billing_model_source" varchar(20) DEFAULT 'original' NOT NULL;--> statement-breakpoint | ||
|
|
There was a problem hiding this comment.
🔴 Critical: Missing billing_model_source column in migration
Why this is a problem: The schema at src/drizzle/schema.ts adds billingModelSource column to system_settings table, and the repository code (src/repository/system-config.ts) queries this field. However, this migration file does not add this column. This will cause database errors for users running this migration:
SELECT ... billing_model_source ...will fail with "column does not exist"INSERTstatements including this field will fail
Suggested fix: Add this line after Step 5 in the migration:
-- Step 5.1: 添加 system_settings 计费模型来源字段（幂等）
ALTER TABLE "system_settings" ADD COLUMN IF NOT EXISTS "billing_model_source" varchar(20) DEFAULT 'original' NOT NULL;--> statement-breakpoint
| * 支持输入格式: "YYYY-MM-DDTHH:mm" 或 "YYYY-MM-DD HH:mm:ss" | ||
| */ | ||
| function normalizeLocalTimeStr(input: string): string { | ||
| // 处理 datetime-local 格式: "2025-11-26T00:00" → "2025-11-26 00:00:00" |
There was a problem hiding this comment.
🟢 Low: Missing input validation in normalizeLocalTimeStr function
Why this is a problem: The function assumes the input string is always in a valid format (YYYY-MM-DDTHH:mm or YYYY-MM-DD HH:mm:ss). If an unexpected format is passed (e.g., malformed string, empty string, or different format), it will silently produce incorrect results instead of failing early. This could lead to incorrect SQL queries.
Suggested fix:
function normalizeLocalTimeStr(input: string): string {
// Validate input format
if (!input || typeof input !== "string") {
throw new Error("Invalid time string: input is empty or not a string");
}
// Validate expected formats: "YYYY-MM-DDTHH:mm" (16 chars) or "YYYY-MM-DD HH:mm:ss" (19 chars)
const trimmed = input.trim();
if (trimmed.length < 16) {
throw new Error(`Invalid time string format: ${input}`);
}
// 处理 datetime-local 格式: "2025-11-26T00:00" → "2025-11-26 00:00:00"
const normalized = trimmed.replace("T", " ");
// 如果没有秒数，补充 ":00"
if (normalized.length === 16) {
return normalized + ":00";
}
return normalized;
}
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary (Updated)
This PR adds a new "billing model source" configuration feature that allows administrators to control whether billing uses the original model (before redirection) or the redirected model (after redirection). It also consolidates database migrations, improves time zone handling for log filters, and fixes UI/UX issues.
🔍 Issues Found
- Critical (🔴): 0 issues
- High (🟠): 0 issues
- Medium (🟡): 0 issues
- Low (🟢): 1 issue
🎯 Priority Actions
- Low: Consider adding input validation to
normalizeLocalTimeStrfunction insrc/repository/usage-logs.tsto handle edge cases gracefully
💡 General Observations
Well-implemented features:
- The billing model source feature is properly integrated across the stack (schema, repository, actions, UI)
- Migration consolidation is well-designed with idempotent operations
- Time zone handling refactoring from
Dateobjects to local time strings avoids serialization issues - The model redirector reset logic handles provider switching correctly
- CountUp animation fix using
useRefprevents stale closure issues - TagInput
onBlurhandler improves UX by committing pending values
Code Quality:
- Consistent use of TypeScript types (
BillingModelSourcetype) - Proper i18n support across all 5 languages
- UI accessibility improvements with scroll-to-section functionality
🤖 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.
📋 Code Review Summary (Updated)
This PR adds a new "billing model source" configuration feature that allows administrators to control whether billing uses the original model (before redirection) or the redirected model (after redirection). It also consolidates database migrations, improves time zone handling for log filters, and fixes UI/UX issues.
🔍 Issues Found
- Critical (🔴): 0 issues
- High (🟠): 0 issues
- Medium (🟡): 0 issues
- Low (🟢): 1 issue
🎯 Priority Actions
- Low: Consider adding input validation to
normalizeLocalTimeStrfunction insrc/repository/usage-logs.tsto handle edge cases gracefully
💡 General Observations
Well-implemented features:
- The billing model source feature is properly integrated across the stack (schema, repository, actions, UI)
- Migration consolidation is well-designed with idempotent operations
- Time zone handling refactoring from
Dateobjects to local time strings avoids serialization issues - The model redirector reset logic handles provider switching correctly
- CountUp animation fix using
useRefprevents stale closure issues - TagInput
onBlurhandler improves UX by committing pending values
Code Quality:
- Consistent use of TypeScript types (
BillingModelSourcetype) - Proper i18n support across all 5 languages
- UI accessibility improvements with scroll-to-section functionality
🤖 Automated review by Claude AI - focused on identifying issues for improvement
1 participant
Summary
This PR fixes multiple bugs and consolidates database migrations into a single idempotent migration file.
Problem
Additionally, the migration files (0020-0025) had conflicts causing upgrade issues for users on different versions.
Solution
0020_glossy_grandmaster.sql)IF NOT EXISTSand exception handling for idempotent operationsChanges
0020_glossy_grandmaster.sqlthat:daily_reset_modeenum type (idempotent)Testing
Related Issues
Closes #204
Closes #201
Closes #198