fix: 增加 RPM 管理功能;删除新建用户默认限额#499
Conversation
- Remove DEFAULT constraints from rpm_limit and daily_limit_usd columns in database schema - Update transformers to not fallback to default values for null quotas - Modify rate limit guard to explicitly check for null values (null = unlimited) - Add "unlimited" quick value option (0) in limit rule picker UI - Add i18n translations for unlimited option across all locales - Update validation schemas to allow null values for limits - Fix null safety issues in quota progress bar calculations BREAKING CHANGE: New users will no longer automatically receive default quotas. Admins must explicitly assign quotas to users who need rate limiting. Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <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! 此拉取请求旨在优化用户速率限制(RPM 和每日配额)的管理和解释方式。通过引入 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
|
There was a problem hiding this comment.
Code Review
这个 PR 很好地解决了 dailyQuota 为 0 时归一化为 null 的问题,并为此添加了全面的单元测试,值得称赞。
不过,在审查代码时,我发现新引入的 rpm 字段在处理“无限制”场景时存在一个关键问题。当前实现中使用 0 来表示无限制,但这在后端被解释为“每分钟 0 次请求”的硬限制,会无意中禁用用户的 API 访问。我已经在相关代码位置提出了具体的修改建议,将 0 转换为 null 来正确表示“无限制”。
除此之外,代码的整体结构和实现都很好,特别是国际化和 UI 组件的更新非常完整。修复了 rpm 的问题后,这个功能就更完善了。
| case "limitRpm": | ||
| emitChange("rpm", 0); // 0 = 无限制 | ||
| return; |
| case "limitRpm": | ||
| emitChange("rpm", Math.floor(value)); // RPM 应为整数 | ||
| return; |
| tags: [], | ||
| expiresAt: undefined, | ||
| providerGroup: PROVIDER_GROUP.DEFAULT, | ||
| rpm: 0, |
| tags: user.tags || [], | ||
| expiresAt: user.expiresAt ?? undefined, | ||
| providerGroup: normalizeProviderGroup(user.providerGroup), | ||
| rpm: user.rpm ?? 0, |
There was a problem hiding this comment.
Code Review Summary
This PR implements a significant architectural change to user quota management by removing default quotas and treating null values as "unlimited". The implementation is well-designed and thoroughly tested.
PR Size: XL
- Lines changed: 2656 (2501 additions + 155 deletions)
- Files changed: 33
The PR size is appropriate given this is a schema-breaking change affecting the database, type system, business logic, and UI across multiple locales.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
No significant issues identified in this PR.
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Excellent
- Code clarity - Good
Key Strengths
-
Comprehensive Test Coverage: New test file
transformers.test.ts(235 lines) thoroughly covers all edge cases including null, undefined, zero, and string values for bothrpmanddailyQuotafields. -
Proper Null Handling: Rate limit guard correctly wraps quota checks with null guards (e.g.,
if (user.rpm !== null)andif (user.dailyQuota !== null)), ensuring unlimited quotas skip enforcement entirely. -
Type System Consistency: Type changes propagate correctly throughout the codebase - from database schema to TypeScript interfaces to API responses to UI components.
-
Migration Safety: Database migration only drops defaults; existing data is preserved. The transformer normalizes legacy zero values to null at runtime.
-
Validation Layers: Multi-layered validation ensures data integrity:
- Database:
numeric(10, 2)for dailyQuota,integerfor rpm - Zod schemas: Min/max validation with proper i18n error messages
- Transformer: Normalizes
<= 0tonullfor dailyQuota, preserves0for rpm
- Database:
-
I18n Completeness: All 5 locales (en, ja, ru, zh-CN, zh-TW) updated consistently with new "unlimited" terminology.
-
UI/UX: Batch edit dialog and limit rule picker properly handle null values with "unlimited" quick-select options.
Validation Performed
I performed comprehensive validation including:
- Full context reads of critical files (transformers, rate-limit-guard, schemas)
- Cross-reference checks for null handling across the codebase
- Verification that database constraints align with application logic
- Confirmation that test coverage matches all transformer edge cases
- Review of i18n consistency across all locales
- Analysis of migration impact and backwards compatibility
All potential issues were investigated and validated as either:
- Intentional design decisions (e.g.,
0rpm = blocked,nullrpm = unlimited) - Properly handled elsewhere (e.g., database type safety prevents NaN)
- Already tested (e.g., zero normalization explicitly documented and tested)
Automated review by Claude AI
将 rpm = 0 转换为 null 表示无限制,与 dailyQuota 的处理保持一致。 之前 rpm = 0 会被后端解释为"每分钟 0 次请求",会禁用用户 API 访问。 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
本次 PR 的核心目标是移除新建用户的默认配额,并将 0 值标准化为 null 以表示“无限制”,从而解决之前存在的歧义。您通过修改数据库结构、更新数据转换层、调整类型定义、适配限流逻辑以及更新 UI,非常全面地完成了这一重构。代码结构清晰,逻辑严谨,特别是为 transformers 新增了单元测试,这是一个非常好的实践。
我的审查意见主要集中在一些可以提升代码可读性和一致性的细节上,并发现了一个国际化文件中可能存在的疏漏。总体而言,这是一次高质量的提交。
I am having trouble creating individual review comments. Click here to see my feedback.
messages/zh-TW/dashboard.json (478-480)
您似乎意外地刪除了 quotas.labels 下的 all, warning, 和 exceeded 這幾個翻譯鍵。這些鍵在其他語言版本中仍然存在,並且可能被前端的額度篩選功能使用。請恢復這些翻譯以避免功能異常或顯示問題。
src/app/[locale]/dashboard/_components/user/forms/user-edit-section.tsx (230)
为了与 number | null 类型定义保持一致并提高代码清晰度,建议在移除 RPM 限制时直接传递 null 而不是 0。虽然系统目前能将 0 正确处理为“无限制”,但使用 null 能更明确地表达“未设置”或“无限”的意图。
emitChange("rpm", null); // null = 无限制
src/app/[locale]/dashboard/_components/user/forms/user-edit-section.tsx (263)
在处理 RPM 限制时,当用户从 LimitRulePicker 选择“无限制”(对应的值为 0)时,建议直接 emitChange 为 null,而不是 0。这与类型系统 (number | null) 更为一致,也使得意图更清晰。
emitChange("rpm", value > 0 ? Math.floor(value) : null); // RPM 应为整数
src/repository/_shared/transformers.ts (15-24)
rpm 和 dailyQuota 字段的转换逻辑可以简化。if (...) return null; 的检查是多余的,因为后续的 Number() 和 Number.parseFloat() 已经可以正确处理 null 和 undefined 的情况,并最终通过 > 0 的判断返回 null。移除这个冗余的检查可以让代码更简洁。
rpm: (() => {
const parsed = Number(dbUser?.rpm);
return parsed > 0 ? parsed : null;
})(),
dailyQuota: (() => {
const parsed = Number.parseFloat(dbUser?.dailyQuota);
return parsed > 0 ? parsed : null;
})(),| value={ | ||
| user.quota?.rpm && user.quota.rpm.limit > 0 | ||
| user.quota?.rpm && user.quota.rpm.limit !== null && user.quota.rpm.limit > 0 | ||
| ? (user.quota.rpm.current / user.quota.rpm.limit) * 100 |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Unlimited RPM/Daily quota renders as null / $0.00
File: src/app/[locale]/dashboard/quotas/users/_components/user-quota-list-item.tsx:161
Why this is a problem: getUserLimitUsage() now returns limit: number | null (and 0 is treated as “no limit” in the rate-limit layer). This component updated the progress calculation to guard limit !== null, but the summary line above still renders {current} / {limit} (RPM) and formatCurrency(limit) (daily cost) even when the limit is null/0, producing misleading UI like 0 / null or $12.34 / $0.00 for an unlimited quota.
Suggested fix:
// RPM summary: treat null/<=0 as unlimited
{user.quota?.rpm && user.quota.rpm.limit !== null && user.quota.rpm.limit > 0 ? (
<span className="font-medium">
{user.quota.rpm.current} / {user.quota.rpm.limit}
</span>
) : (
<span className="text-muted-foreground">{t("noLimitSet")}</span>
)}
// Daily cost summary: same idea
{user.quota?.dailyCost &&
user.quota.dailyCost.limit !== null &&
user.quota.dailyCost.limit > 0 ? (
<span className="font-medium">
{formatCurrency(user.quota.dailyCost.current, currencyCode)} /{" "}
{formatCurrency(user.quota.dailyCost.limit, currencyCode)}
</span>
) : (
<span className="text-muted-foreground">{t("noLimitSet")}</span>
)}There was a problem hiding this comment.
Code Review Summary
This PR is XL and spans DB migration/schema defaults, server actions, proxy rate limiting, dashboard UI, i18n, and tests for user RPM/daily quota normalization. The main risk is behavior drift across layers when “unlimited” is represented as null/0.
PR Size: XL
- Lines changed: 2656
- Files changed: 33
- Split suggestions:
- DB migration + Drizzle schema changes
- Server-side RPM/dailyQuota normalization + type changes
- Dashboard UI + i18n additions
- Test-only PR for transformer coverage
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
src/app/[locale]/dashboard/quotas/users/_components/user-quota-list-item.tsx:161[LOGIC-BUG] Unlimitedrpm/dailyCostlimits render asnull/$0.00in the quota summary when the limit isnull/0(Confidence: 95).
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Identified open PR
#499and applied labelsize/XL(2656 lines changed, 33 files). - Posted 1 inline review comment on
src/app/[locale]/dashboard/quotas/users/_components/user-quota-list-item.tsx:161about unlimitedrpm/dailyCostlimits rendering asnull/$0.00in the quota summary, with a concretetsxfix snippet. - Submitted the required PR review summary via
gh pr review --comment.
Summary
Removes default quota assignments for new users and normalizes zero values to null for proper "unlimited" quota handling.
Problem
Previously, new users were automatically assigned default quotas (RPM: 60, Daily: $100) at database and application levels. This created issues:
rpm_limit DEFAULT 60anddaily_limit_usd DEFAULT '100.00'toUser()treated "0" string values as 0 instead of null (unlimited)Solution
Core Changes
Database Schema (Migration 0042)
drizzle/0042_legal_harrier.sql- Removed defaults fromusers.rpm_limitandusers.daily_limit_usdcolumnssrc/drizzle/schema.ts- Changed from.default(60)and.default('100.00')to nullable without defaultsTransformer Logic (
src/repository/_shared/transformers.ts)toUser.rpm: Changed fromdbUser?.rpm || 60todbUser?.rpm ?? null(no fallback)toUser.dailyQuota: Added normalization - parses string, returnsnullif<= 0, otherwise parsed number"0"→null(unlimited)"0.00"→null(unlimited)"100.00"→100null/undefined→nullType System Updates (
src/types/user.ts)User.rpm:number→number | nullUser.dailyQuota:number→number | nullRate Limiting (
src/app/v1/_lib/proxy/rate-limit-guard.ts)if (user.dailyQuota \!== null)- skips check when unlimitednullas unlimitedUser Management UI
Supporting Changes
src/actions/users.ts- Updated all user CRUD functions to handle nullable quotas, removed default value assignmentssrc/lib/constants/user.constants.ts- RemovedUSER_DEFAULTS.RPMandUSER_DEFAULTS.DAILY_QUOTAconstantssrc/lib/validation/schemas.ts- Changed RPM validation from.optional().default(60)to.nullable().optional()src/repository/_shared/transformers.test.ts(NEW) - Comprehensive Vitest coverage for transformer edge casesMigration Impact
Breaking Changes
nullinstead of 60/100user.rpmanduser.dailyQuotausageData Migration
rpm_limit = 60anddaily_limit_usd = 100.00are not modifiedINSERTstatements will getNULLby defaultnullat application runtime viatoUser()Testing
Automated Tests
src/repository/_shared/transformers.test.ts(235 lines)toUser.rpmandtoUser.dailyQuotaManual Testing Checklist
bun run build- verify production build succeedsbun run test- verify all tests passVerification
Per PR description:
DSN= bun run testbun run lintbun run typecheckbun run buildDescription enhanced by Claude AI
原始说明 (Original Chinese Description)
变更说明
src/repository/_shared/transformers.test.ts(Vitest),重点覆盖toUser.rpm与toUser.dailyQuota的边界行为,并补齐transformers.ts其余导出函数的基础覆盖。toUser.dailyQuota:将解析结果<= 0(含 "0" / "0.00")统一归一化为null。验证
DSN= bun run testbun run lintbun run typecheckbun run build