Conversation
English: - Dashboard: Quota Management→Quotas, User Management→Users, Documentation→Docs - Settings: Configuration→Config, Sensitive Words→Filters, Client Update Reminder→Updates, Data Management→Data, API Documentation→API Docs, Error Rules→Errors Russian: - Dashboard: Документация→Доки - Settings: Конфигурация→Конфиг, Прайс-лист→Цены, Чувствительные слова→Фильтры, Напоминание об обновлении→Обновления, Управление данными→Данные, API документация→API док., Правила ошибок→Ошибки, Обратная связь→Отзывы Japanese: - Dashboard: ダッシュボード→ホーム, ユーザー管理→ユーザー, ドキュメント→ドキュ, システム設定→設定 - Settings: プロバイダー→供給元, センシティブワード→フィルター, クライアント更新リマインダー→更新通知, データ管理→データ, APIドキュメント→API文書, エラールール→エラー, フィードバック→報告
修复正则表达式以同时匹配 "tokens...maximum" 和 "maximum...tokens" 两种格式。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
fix: 改进 prompt_limit 错误规则的正则匹配
Visual presentation modes:
1. Overlay (对比模式)
- 关注点: 个体差异 ("谁更强")
- 逻辑: 独立渲染,基准线均为 0
- 用途: 数值直接对比 (e.g., User A vs User B)
2. Stacked (堆叠模式)
- 关注点: 整体构成 ("整体怎样")
- 逻辑: 累加渲染,后续层基于前一层
- 用途: 总量及占比分析 (e.g., Total Spend composition)
Add ability to override error responses and status codes when error rules match: - New DB fields: override_response (JSONB) and override_status_code - Runtime override logic in error-handler with validation (400-599 range) - New validator for Claude API error format with 10KB size limit - Frontend: override section component with real-time JSON validation - Frontend: error rule tester to preview override behavior - Support empty message fallback to original error - Backward compatible: no behavior change without override config 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
根本原因:前端传递小数形式的 bucketSizeMinutes(如 0.25 表示 15 秒), 但后端使用 parseInt() 解析,导致 0.25 被转换为 0, 在除法运算中产生 Infinity,最终导致 Invalid Date 错误。 修复: - API 路由:使用 parseFloat 代替 parseInt 解析 bucketSizeMinutes - 可用性服务:增加最小值保护(0.25 分钟 = 15 秒)防止除零错误 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…lose #230 The createActionRoute function was passing the entire request body object as a single argument to Server Actions. This broke actions that expect positional parameters (e.g., `getUserStatistics(timeRange)` receiving `{ timeRange: "hour" }` instead of `"hour"`). The fix extracts parameters from the request body based on the schema keys and passes them as positional arguments to match the action function signatures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Change multi-parameter case to pass entire body object - Fixes #230 for single-param actions (e.g., getUserStatistics) - Preserves compatibility for editUser/editKey/editProvider signatures
- Add Number.isNaN check in route.ts to handle invalid parseFloat input - Add Number.isNaN check in availability-service.ts as defense-in-depth - Fixes edge case where Math.max(0.25, NaN) returns NaN, causing Invalid Date error
…id-date Fix #227: 修复可用性监控 15 分钟时间范围的 Invalid Date 错误
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant new capability by allowing administrators to configure custom error overriding rules. This feature enables the system to intercept specific upstream error messages and return tailored, user-friendly responses and HTTP status codes to clients, while still logging the original detailed errors for debugging. Complementary improvements include UI/UX refinements, better data visualization options in the dashboard, and several backend stability enhancements. 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 3,502 lines changed across 33 files. Large PRs are harder to review and more likely to introduce bugs. 🔀 Suggested Split:Based on the changes, this PR could be split into:
Why Split?
Note on Generated FilesThe 🤖 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. 📋 OWASP Top 10 Coverage
🛡️ Security Features ImplementedThis PR actually improves security posture by:
📝 Code Quality Observations
✅ Security Posture: StrongThe error override feature is well-designed with multiple security layers. No action required before merge. 🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature for error handling, allowing custom override responses and status codes for matched error rules. This includes a new UI for configuration and a tester for validation. Additionally, there are several UI/UX improvements, such as making navigation labels more concise, adding a stacked/overlay mode to charts, and fixing a bug in the availability monitoring chart. However, I've identified a critical regression in the API action adapter (src/lib/api/action-adapter-openapi.ts) that incorrectly handles single-argument server actions, which could break existing functionality. My review includes a detailed explanation and a suggested fix for this issue.
| // 如果 requestSchema 是对象类型,提取 keys 作为参数顺序 | ||
| let args: unknown[]; | ||
| if (requestSchema instanceof z.ZodObject) { | ||
| const schemaShape = requestSchema.shape; | ||
| const keys = Object.keys(schemaShape); | ||
| if (keys.length === 0) { | ||
| // 没有参数 | ||
| args = []; | ||
| } else if (keys.length === 1) { | ||
| // 单个参数,直接传递值 | ||
| args = [body[keys[0] as keyof typeof body]]; | ||
| } else { | ||
| // 多个参数场景 - 保持原有行为传递整个 body 对象 | ||
| // 因为存在 editUser(userId, data) 这类签名,无法从 schema 区分 | ||
| args = [body]; | ||
| } | ||
| } else { | ||
| // 非对象 schema,直接传递整个 body | ||
| args = [body]; | ||
| } | ||
|
|
||
| const rawResult = await action(...args); |
There was a problem hiding this comment.
The new logic for handling single-parameter server actions appears to be incorrect. The else if (keys.length === 1) block unwraps the single property from the request body, which will break actions that expect a single object as their parameter (e.g., myAction(input: { message: string })). For instance, the newly added testErrorRuleAction in this same pull request would be called with a string argument instead of the expected object, causing a runtime error. The previous behavior of passing the entire body object was more robust for this common case.
// If requestSchema is an object type, determine arguments
let args: unknown[];
if (requestSchema instanceof z.ZodObject) {
const keys = Object.keys(requestSchema.shape);
if (keys.length === 0) {
// Handle actions with no parameters
args = [];
} else {
// For actions with one or more parameters, pass the entire body object.
// This is safer and supports both `action({ a, b })` and actions that
// might destructure a single object argument.
args = [body];
}
} else {
// Non-object schema, pass the entire body as a single argument
args = [body];
}
const rawResult = await action(...args);
ding113
left a comment
There was a problem hiding this comment.
📝 Documentation Review
📊 Issues Summary
- Critical (🔴): 1 - Must fix before merge
- High (🟠): 0 - Should fix before merge
- Medium (🟡): 0 - Consider addressing
- Low (🟢): 0 - Optional improvements
⚡ Priority Fixes
🔴 Missing Historical Records in CHANGELOG.md
This PR removes 9 previously documented bug fixes from the [Unreleased] section without preserving them:
- #220: Fix session binding not updating after provider failover
- #219: Fix provider availability monitoring page sorting order
- #216: Multiple fixes (JSONB field name error, response validation, login redirect, translations, DialogContent width)
- #210: Fix model test disclaimer display order
- #207: Fix provider statistics, group settings persistence
Impact: Historical changelog entries are permanently lost. Users and contributors reviewing the changelog will not see these fixes documented.
Suggested Fix: Either:
- Create a versioned release section (e.g.,
## [1.x.x] - 2024-11-28) and move these fixes there before removing them from[Unreleased] - Keep these entries in the
[Unreleased]section alongside the new fixes (#231, #232)
Reference: Keep a Changelog recommends maintaining all notable changes until they are released under a version tag.
📋 Review Coverage
- Technical accuracy - No issues
- Completeness - 1 issue (missing historical entries)
- Code examples - N/A
- Links and references - No issues
- Clarity and organization - No issues
💡 General Observations
The PR description is well-structured and comprehensive, but the CHANGELOG.md changes contradict the stated goal of "consolidating multiple features and bug fixes since the last release" - instead, it removes documentation of previous fixes. Consider:
- If this is a release PR, add a versioned section header before clearing
[Unreleased] - The features mentioned in the PR description (Error Response Override, Chart Visualization Modes) should also be added to CHANGELOG.md under
### Added
🤖 Automated docs review by Claude AI
📝 Documentation Review📊 Issues Summary
⚡ Priority Fixes🔴 Missing Historical Records in CHANGELOG.md This PR removes 9 previously documented bug fixes from the
Impact: Historical changelog entries are permanently lost. Users and contributors reviewing the changelog will not see these fixes documented. Suggested Fix: Either:
Reference: Keep a Changelog recommends maintaining all notable changes until they are released under a version tag. 📋 Review Coverage
💡 General ObservationsThe PR description is well-structured and comprehensive, but the CHANGELOG.md changes contradict the stated goal of "consolidating multiple features and bug fixes since the last release" - instead, it removes documentation of previous fixes. Consider:
🤖 Automated docs review by Claude AI |
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary
This is a release PR consolidating multiple features including Error Response Override, Chart Visualization Modes, and bug fixes for API action adapter and availability monitoring. The code quality is generally good with proper validation and error handling.
🔍 Issues Found
- Critical (🔴): 0 issues
- High (🟠): 1 issue
- Medium (🟡): 2 issues
- Low (🟢): 1 issue
🎯 Priority Actions
-
[🟠 High] action-adapter-openapi.ts:233 - The single-parameter extraction logic using
Object.keys(schemaShape)relies on property ordering which, while practically stable in V8, is not guaranteed by the JavaScript specification. Consider explicitly sorting keys or documenting the assumption. -
[🟡 Medium] drizzle.config.ts:4-8 - The new env file loading loop doesn't use
override: falseoption fordotenv.config(), meaning later files will NOT override earlier ones. The comment says "priority order" but the implementation does the opposite - first match wins, so .env.development.local takes priority only if loaded first AND subsequent loads don't override. Consider addingoverride: trueto reverse the effective priority. -
[🟡 Medium] availability-service.ts:182-187 - The NaN handling uses
Number.isNaN()which is correct, but the minimum bucket size of 0.25 minutes (15 seconds) is very small. With high-volume data, this could create thousands of buckets causing performance issues. Consider a larger minimum like 1 minute. -
[🟢 Low] error-override-validator.ts:78 - The
isValidErrorOverrideResponsetype guard function duplicates validation logic by callingvalidateErrorOverrideResponse. This is fine but could note in comments that the two functions share logic.
💡 General Observations
- Good use of validation at multiple layers (repository, detector, action) for the error override feature
- The atomic cache replacement pattern in ErrorRuleDetector.reload() is well implemented
- Proper ReDoS protection with safe-regex is maintained
- Translation keys are consistently added across all locales
🤖 Automated review by Claude AI - focused on identifying issues for improvement
|
|
||
| // 如果 requestSchema 是对象类型,提取 keys 作为参数顺序 | ||
| let args: unknown[]; | ||
| if (requestSchema instanceof z.ZodObject) { |
There was a problem hiding this comment.
🟠 High: Object.keys() does not guarantee property order
Why this is a problem: While V8 maintains insertion order for string keys, the JS spec does not guarantee ordering. This could cause parameter order mismatch in edge cases.
Suggested fix:
const keys = Object.keys(schemaShape).sort(); // Explicitly sort for deterministic order| const rawBucketSize = | ||
| explicitBucketSize ?? determineOptimalBucketSize(requests.length, timeRangeMinutes); | ||
| const bucketSizeMinutes = Number.isNaN(rawBucketSize) | ||
| ? determineOptimalBucketSize(requests.length, timeRangeMinutes) |
There was a problem hiding this comment.
🟡 Medium: Minimum bucket size of 0.25 minutes may cause performance issues
Why this is a problem: With 15-second buckets on high-volume data, this could create thousands of buckets causing memory and CPU performance issues during aggregation.
Suggested fix:
const bucketSizeMinutes = Number.isNaN(rawBucketSize)
? determineOptimalBucketSize(requests.length, timeRangeMinutes)
: Math.max(1, rawBucketSize); // Use 1 minute as minimum for safety| // Priority: .env.development.local > .env.local > .env.development > .env | ||
| const envFiles = [".env.development.local", ".env.local", ".env.development", ".env"]; | ||
| for (const envFile of envFiles) { | ||
| config({ path: envFile }); |
There was a problem hiding this comment.
🟡 Medium: dotenv config() priority order is inverted from comment
Why this is a problem: The comment says "Priority: .env.development.local > .env.local > ..." but dotenv.config() by default does NOT override already-set variables. So loading in this order means .env.development.local is loaded first and its values are set, then subsequent files cannot override them. The actual priority is first-file-wins, which matches Next.js priority.
However, if DSN is already set in an earlier file, later files won't override it. Consider whether this is intentional.
Suggested clarification: Update the comment to clarify the "first wins" behavior, or use override: true if you want later files to take precedence:
for (const envFile of envFiles) {
config({ path: envFile, override: false }); // First file wins (Next.js style)
}
Summary
Release PR consolidating multiple features and bug fixes since the last release.
Changes
Features
drizzle.config.tsto follow Next.js env file priority order (.env.development.local>.env.local>.env.development>.env)Bug Fixes
prompt_limiterror rule regex pattern matchingInternationalization
Database
0021_broad_black_panther.sqladdingoverride_response(jsonb) andoverride_status_code(integer) columns toerror_rulestableFiles Changed
src/app/v1/_lib/proxy/error-handler.ts- Error override application logicsrc/app/[locale]/settings/error-rules/- UI for error override configurationsrc/lib/error-override-validator.ts- Override response validationsrc/lib/api/action-adapter-openapi.ts- API adapter fixsrc/lib/availability/availability-service.ts- Availability monitoring fixsrc/drizzle/schema.ts- Schema updates for error override fieldsTesting
🤖 Generated with Claude Code