Conversation
- 修复:Key 日限额优先于 User 日额度检查\n- 修复:checkCostLimits 对 user 类型的文案标识不一致\n- 测试:补齐限额相关单测,新增 quota 覆盖率配置\n\n验证:\n- bun run lint\n- bun run typecheck\n- bun run test\n- bun run build\n- bun run test:coverage\n- bun run test:coverage:quota
|
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. 📝 WalkthroughWalkthrough此 PR 增强了速率限制和配额管理功能。核心变化包括重组速率限制检查逻辑、引入新的密钥级日配额检查、添加语义增强以改进错误消息,以及扩展单元测试覆盖和新的 Vitest 配额配置文件。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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! 此拉取请求旨在解决一个关键的限额管理缺陷,即当用户未设置每日限额时,Key 的每日限额会失效。通过调整限额检查的执行顺序,确保 Key 级别的每日限额始终优先于用户级别进行评估。此外,还改进了限额超出的错误提示信息,使其更加准确。最重要的是,此 PR 大幅增强了限额系统的测试覆盖率,引入了针对各种限额场景(包括不同实体类型、缓存行为和重置模式)的详细单元测试,以提高系统的健壮性和可靠性。 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
你好,感谢你的贡献。这次的 Pull Request 修复了一个关键的限额检查漏洞,确保了在用户未设置限额时,Key 的限额能够正确生效。你通过调整检查优先级,补全了 Key 的每日限额检查,这是一个重要的修正。
此外,你还统一了限额提示信息,并增加了非常全面的单元测试,覆盖了多种限额场景、缓存未命中和数据库回退等情况。为限额相关的代码建立独立的测试覆盖率配置也是一个很好的实践,有助于长期维护代码质量。
代码整体质量很高。我只发现了一个可以改进的地方,即在 rate-limit-guard.ts 中存在一些重复的错误处理逻辑,建议可以提取成一个辅助函数来优化。
总体来说,这是一次出色的修复和增强!
| // 7. Key 每日限额(Key 独有的每日预算)- null 表示无限制 | ||
| const keyDailyCheck = await RateLimitService.checkCostLimits(key.id, "key", { | ||
| limit_5h_usd: null, | ||
| limit_daily_usd: key.limitDailyUsd, | ||
| daily_reset_mode: key.dailyResetMode, | ||
| daily_reset_time: key.dailyResetTime, | ||
| limit_weekly_usd: null, | ||
| limit_monthly_usd: null, | ||
| }); | ||
|
|
||
| if (!keyDailyCheck.allowed) { | ||
| logger.warn(`[RateLimit] Key daily limit exceeded: key=${key.id}, ${keyDailyCheck.reason}`); | ||
|
|
||
| const { currentUsage, limitValue } = parseLimitInfo(keyDailyCheck.reason!); | ||
|
|
||
| const resetInfo = getResetInfoWithMode("daily", key.dailyResetTime, key.dailyResetMode); | ||
| // rolling 模式没有 resetAt,使用 24 小时后作为 fallback | ||
| const resetTime = | ||
| resetInfo.resetAt?.toISOString() ?? | ||
| new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(); | ||
|
|
||
| const { getLocale } = await import("next-intl/server"); | ||
| const locale = await getLocale(); | ||
| const message = await getErrorMessageServer( | ||
| locale, | ||
| ERROR_CODES.RATE_LIMIT_DAILY_QUOTA_EXCEEDED, | ||
| { | ||
| current: currentUsage.toFixed(4), | ||
| limit: limitValue.toFixed(4), | ||
| resetTime, | ||
| } | ||
| ); | ||
|
|
||
| throw new RateLimitError( | ||
| "rate_limit_error", | ||
| message, | ||
| "daily_quota", | ||
| currentUsage, | ||
| limitValue, | ||
| resetTime, | ||
| null | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/unit/lib/rate-limit/cost-limits.test.ts (1)
142-246: LGTM!全面覆盖了缓存降级和滚动窗口场景这些测试用例全面验证了:
- Redis cache miss 时的数据库降级逻辑(lines 142-160)
- 总消费限额的各种边界情况(lines 162-202)
- 用户每日消费在 fixed 和 rolling 模式下的检查(lines 204-246)
- ZSET 缓存回填机制(lines 229-246)
Line 243-245 验证 pipeline 命令序列的做法很好,确保了 ZSET 正确重建。
可选改进: Line 166 使用
undefined as any是为了测试边界情况,虽然在测试代码中可接受,但如果类型允许可考虑更精确的类型处理。src/app/v1/_lib/proxy/rate-limit-guard.ts (1)
263-273: 可选优化:考虑提取重复的 locale 获取逻辑。文件中多次重复
await import("next-intl/server")和getLocale()调用。可以考虑在函数顶部一次性获取 locale,或提取一个辅助函数来构建本地化错误消息。不过这是已有模式,不阻塞本次修复。
🔎 可选:提取辅助函数
// 在文件顶部或 ensure 方法开头 async function buildRateLimitMessage( errorCode: string, params: Record<string, string> ): Promise<string> { const { getLocale } = await import("next-intl/server"); const locale = await getLocale(); return getErrorMessageServer(locale, errorCode, params); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (9)
.gitignorepackage.jsonsrc/app/v1/_lib/proxy/rate-limit-guard.tssrc/lib/rate-limit/service.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/lib/rate-limit/service-extra.test.tstests/unit/lib/rate-limit/time-utils.test.tstests/unit/proxy/rate-limit-guard.test.tsvitest.quota.config.ts
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use 2-space indentation in all code files
Files:
src/lib/rate-limit/service.tspackage.jsontests/unit/lib/rate-limit/time-utils.test.tsvitest.quota.config.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/proxy/rate-limit-guard.test.tssrc/app/v1/_lib/proxy/rate-limit-guard.tstests/unit/lib/rate-limit/service-extra.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use double quotes for strings instead of single quotes
Use trailing commas in multi-line structures
Enforce maximum line length of 100 characters
Use path alias@/*to reference files from./src/*directory
**/*.{ts,tsx,js,jsx}: Use Biome for linting and formatting with 2-space indent, double quotes, trailing commas, and 100 character max line length
Use path alias@/*to reference files in./src/*directory
Files:
src/lib/rate-limit/service.tstests/unit/lib/rate-limit/time-utils.test.tsvitest.quota.config.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/proxy/rate-limit-guard.test.tssrc/app/v1/_lib/proxy/rate-limit-guard.tstests/unit/lib/rate-limit/service-extra.test.ts
src/lib/rate-limit/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Rate Limiting must track dimensions: RPM, cost (5h/week/month), concurrent sessions at User, Key, and Provider levels using Redis Lua scripts
Use Redis with Lua scripts for atomic rate limiting operations
Files:
src/lib/rate-limit/service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode for type safety
Use readonly or const assertions for immutable data structures
Files:
src/lib/rate-limit/service.tstests/unit/lib/rate-limit/time-utils.test.tsvitest.quota.config.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/proxy/rate-limit-guard.test.tssrc/app/v1/_lib/proxy/rate-limit-guard.tstests/unit/lib/rate-limit/service-extra.test.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Hash API keys using SHA-256 before storing in database, never store plaintext keys
Mask API keys and sensitive data in application logs
Validate required environment variables at startup with clear error messages
Files:
src/lib/rate-limit/service.tssrc/app/v1/_lib/proxy/rate-limit-guard.ts
src/lib/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use connection pooling for database and Redis connections
Files:
src/lib/rate-limit/service.ts
**/*.{tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use next-intl for internationalization with 5 locales: en, ja, ru, zh-CN, zh-TW
Files:
package.json
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for unit testing with Node environment, coverage thresholds: 50% lines/functions, 40% branches
Files:
tests/unit/lib/rate-limit/time-utils.test.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/proxy/rate-limit-guard.test.tstests/unit/lib/rate-limit/service-extra.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Ensure test database names contain 'test' keyword for safety validation
Files:
tests/unit/lib/rate-limit/time-utils.test.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/proxy/rate-limit-guard.test.tstests/unit/lib/rate-limit/service-extra.test.ts
src/app/v1/_lib/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Guard pipeline must execute in order: ProxyAuthenticator, SensitiveWordGuard, VersionGuard, ProxySessionGuard, ProxyRateLimitGuard, ProxyProviderResolver
Files:
src/app/v1/_lib/proxy/rate-limit-guard.ts
src/app/v1/_lib/proxy/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/app/v1/_lib/proxy/**/*.ts: Implement guard pipeline pattern for cross-cutting concerns in request processing (auth, rate limiting, session)
Use undici library for HTTP requests instead of node-fetch for better performance
Files:
src/app/v1/_lib/proxy/rate-limit-guard.ts
src/**/*{guard,auth}*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use constant-time comparison for API key validation to prevent timing attacks
Files:
src/app/v1/_lib/proxy/rate-limit-guard.ts
src/app/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement Content-Security-Policy headers for XSS prevention
Files:
src/app/v1/_lib/proxy/rate-limit-guard.ts
src/app/v1/_lib/proxy/**/*guard*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement fail-open strategy: allow requests when Redis is unavailable
Files:
src/app/v1/_lib/proxy/rate-limit-guard.ts
src/app/v1/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use Hono router for ultrafast, lightweight routing in proxy endpoints
Files:
src/app/v1/_lib/proxy/rate-limit-guard.ts
🧠 Learnings (11)
📚 Learning: 2026-01-03T09:08:20.573Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T09:08:20.573Z
Learning: Applies to src/lib/rate-limit/**/*.ts : Rate Limiting must track dimensions: RPM, cost (5h/week/month), concurrent sessions at User, Key, and Provider levels using Redis Lua scripts
Applied to files:
src/lib/rate-limit/service.tstests/unit/lib/rate-limit/time-utils.test.tstests/unit/lib/rate-limit/cost-limits.test.tstests/unit/proxy/rate-limit-guard.test.tssrc/app/v1/_lib/proxy/rate-limit-guard.tstests/unit/lib/rate-limit/service-extra.test.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to **/*.test.{ts,tsx} : Use Vitest for unit testing with Node environment, coverage thresholds: 50% lines/functions, 40% branches
Applied to files:
package.jsontests/unit/lib/rate-limit/time-utils.test.tsvitest.quota.config.ts.gitignore
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to **/*.test.ts : Ensure test database names contain 'test' keyword for safety validation
Applied to files:
package.jsonvitest.quota.config.ts
📚 Learning: 2026-01-03T09:08:20.573Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T09:08:20.573Z
Learning: Applies to tests/integration/**/*.test.{ts,tsx} : Integration tests requiring database must be placed in `tests/integration/` and use test database with 'test' in the name
Applied to files:
package.jsonvitest.quota.config.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to tests/integration/**/*.test.ts : Place integration tests requiring database in `tests/integration/` (excluded by default)
Applied to files:
package.jsonvitest.quota.config.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/lib/rate-limit/**/*.ts : Use Redis with Lua scripts for atomic rate limiting operations
Applied to files:
tests/unit/lib/rate-limit/cost-limits.test.tssrc/app/v1/_lib/proxy/rate-limit-guard.tstests/unit/lib/rate-limit/service-extra.test.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/**/*{message,response,log}*.ts : Log request duration, token usage, and cost to message_request table for analytics
Applied to files:
tests/unit/lib/rate-limit/cost-limits.test.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/app/v1/_lib/proxy/**/*.ts : Implement guard pipeline pattern for cross-cutting concerns in request processing (auth, rate limiting, session)
Applied to files:
tests/unit/proxy/rate-limit-guard.test.tssrc/app/v1/_lib/proxy/rate-limit-guard.ts
📚 Learning: 2026-01-03T09:08:20.573Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T09:08:20.573Z
Learning: Applies to src/app/v1/_lib/**/*.ts : Guard pipeline must execute in order: ProxyAuthenticator, SensitiveWordGuard, VersionGuard, ProxySessionGuard, ProxyRateLimitGuard, ProxyProviderResolver
Applied to files:
tests/unit/proxy/rate-limit-guard.test.tssrc/app/v1/_lib/proxy/rate-limit-guard.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to src/app/v1/_lib/proxy/**/*guard*.ts : Implement fail-open strategy: allow requests when Redis is unavailable
Applied to files:
tests/unit/proxy/rate-limit-guard.test.ts
📚 Learning: 2026-01-03T09:08:49.019Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T09:08:49.019Z
Learning: Applies to **/*.{tsx,json} : Use next-intl for internationalization with 5 locales: en, ja, ru, zh-CN, zh-TW
Applied to files:
.gitignore
🧬 Code graph analysis (3)
tests/unit/lib/rate-limit/time-utils.test.ts (1)
src/lib/rate-limit/time-utils.ts (7)
normalizeResetTime(286-289)getTimeRangeForPeriodWithMode(88-104)getResetInfoWithMode(219-232)getTTLForPeriodWithMode(155-165)getTTLForPeriod(113-147)getSecondsUntilMidnight(295-314)getDailyResetTime(319-337)
vitest.quota.config.ts (2)
scripts/sync-settings-keys.js (1)
path(9-9)scripts/validate-migrations.js (1)
__dirname(18-18)
tests/unit/proxy/rate-limit-guard.test.ts (1)
src/app/v1/_lib/proxy/rate-limit-guard.ts (1)
ProxyRateLimitGuard(20-473)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Greptile Review
- GitHub Check: pr-description
- GitHub Check: pr-label
- GitHub Check: 🌐 API Tests
- GitHub Check: pr-review
- GitHub Check: Docker Build Test
- GitHub Check: check-codex-status
- GitHub Check: dev-build-deploy
🔇 Additional comments (21)
.gitignore (1)
15-15: LGTM!新增的配额覆盖率输出目录忽略规则正确添加了
/coverage-quota忽略规则,与新增的配额测试配置(vitest.quota.config.ts和test:coverage:quota脚本)保持一致。src/lib/rate-limit/service.ts (1)
251-254: LGTM!修复了用户限额类型标识错误的问题此更改将类型名称逻辑提取为独立变量
typeName,确保在 fast-path 中正确标识 User 类型(避免错误标注为"供应商")。这正是 PR 描述中提到的核心修复:统一限额提示信息。代码逻辑清晰,与数据库降级路径(line 466)保持一致。
package.json (1)
21-21: LGTM!新增配额测试覆盖率脚本新增的
test:coverage:quota脚本正确配置了配额专用的测试覆盖率运行,使用独立的vitest.quota.config.ts配置文件,符合 PR 目标中提到的"新增限额专用覆盖率配置"。tests/unit/lib/rate-limit/cost-limits.test.ts (2)
1-67: LGTM!测试基础设施设置完善测试 mock 设置非常全面:
- Pipeline mock(lines 3-21)能够追踪命令执行,便于验证 ZSET 操作
- Redis client mock(lines 23-31)提供完整的方法集
- Statistics mock(lines 37-51)覆盖固定窗口和滚动窗口查询
- 使用 fake timers 确保时间相关测试的确定性
这为后续的限额检查测试提供了坚实的基础。
123-140: LGTM!关键测试验证了 User 类型标识修复此测试用例(lines 123-140)直接验证了 PR 的核心修复:确保 User 类型在 fast-path 中正确显示为 "User" 而非错误的"供应商"。line 139 的断言明确检查了错误消息中的类型名称。
这与
src/lib/rate-limit/service.ts的 line 251 修复相呼应。tests/unit/lib/rate-limit/time-utils.test.ts (1)
1-59: LGTM!时间工具函数测试覆盖全面此测试文件完整验证了
time-utils模块的关键功能:
normalizeResetTime的容错处理(lines 24-28)- 滚动窗口的时间范围计算(lines 30-35)
- 不同模式下的 TTL 计算(lines 43-49)
- 每日重置时间的计算逻辑(lines 51-58)
使用 fake timers(lines 15-22)确保了测试的确定性和可重复性,这是测试时间相关代码的最佳实践。
根据学习记录:符合 Vitest 单元测试规范,使用 Node 环境运行。
vitest.quota.config.ts (1)
1-45: 配置结构良好,覆盖率阈值设置合理。配额测试配置文件设置正确:
- 路径别名
@和server-only配置正确- 测试环境使用
node,符合编码规范- 覆盖率阈值(80% lines/functions/statements,70% branches)高于默认阈值,适合关键限流代码路径
isolate、mockReset、restoreMocks、clearMocks配置确保测试隔离性src/app/v1/_lib/proxy/rate-limit-guard.ts (3)
27-28: 注释更新正确反映了新的检查顺序。检查顺序更新为 5-8(短期)和 9-12(中长期),正确体现了新增的 Key 每日限额检查。
242-284: Key 每日限额检查实现正确,这是本 PR 的核心修复。新增的 Key 每日限额检查逻辑:
- 正确传递
daily_reset_mode和daily_reset_time配置rolling模式下使用 24 小时作为resetTime的 fallback 是合理的- 错误类型
daily_quota与 User 每日检查保持一致此修复确保当
user.dailyQuota = null时,key.limitDailyUsd仍能生效。
286-327: User 每日额度检查逻辑保持不变,仅调整了执行顺序。User 每日检查现在作为第 8 步执行(在 Key 每日检查之后),保留了原有的
user.dailyQuota !== null判断,确保用户未设置限额时跳过此检查。tests/unit/lib/rate-limit/service-extra.test.ts (6)
1-42: Mock 设置结构良好,pipeline 模拟完整。
makePipeline工厂函数正确模拟了 Redis pipeline 的链式调用模式,包括eval、get、incrbyfloat、expire、zremrangebyscore、zcard、zadd和exec方法。pipelineCalls数组用于追踪调用记录,便于断言验证。
82-104: 测试设置完善,时间控制和 Mock 重置正确。
- 使用
vi.useFakeTimers()和vi.setSystemTime()确保时间相关测试的确定性beforeEach中重置所有 Mock 和pipelineCalls数组afterEach恢复真实计时器,避免影响其他测试
106-165: Session 限制测试覆盖全面。测试用例覆盖了:
limit <= 0时放行- Key 并发达到上限时拦截
- Provider 并发未达上限时放行
- Redis 非 ready 时 Fail Open 策略
符合
src/app/v1/_lib/proxy/**/*guard*.ts的 Fail Open 设计原则。基于 learnings 中的编码规范。
166-181: Fixed 和 Rolling 模式的成本追踪测试正确区分了不同的 Redis 数据结构。
- Fixed 模式验证使用
STRING + TTL(incrbyfloat+expire)- Rolling 模式验证使用
ZSET Lua 脚本(eval)这与 learnings 中"Use Redis with Lua scripts for atomic rate limiting operations"的要求一致。
279-301: Daily rolling cache miss 场景测试验证了 DB 回退和 ZSET 预热逻辑。测试确保当 Redis 缓存不存在时(
exists返回 0),系统能够:
- 从 DB 查询历史成本记录
- 使用
zadd预热 ZSET 缓存- 正确计算并返回限额超限状态
380-395: 错误处理测试验证了单个查询失败时的跳过逻辑。当 pipeline 返回中某个查询出错时,该项保持默认值 0,其余项正常解析。这确保了部分 Redis 错误不会导致整个批量查询失败。
tests/unit/proxy/rate-limit-guard.test.ts (5)
1-42: Mock 设置完整,覆盖所有依赖模块。正确模拟了:
@/lib/rate-limit中的RateLimitService各方法@/lib/loggernext-intl/server的getLocale@/lib/utils/error-messages的ERROR_CODES和getErrorMessageServer
43-98:createSession工厂函数设计合理,便于测试数据复用。默认值设置为"无限制"状态(
rpm: null,dailyQuota: null等),允许测试用例按需覆盖特定字段。这符合测试的最小化原则。
110-140: 核心测试用例:验证 PR 的主要修复。此测试确保当
user.dailyQuota = null时,key.limitDailyUsd仍能正确拦截请求:
- 验证
checkUserDailyCost未被调用(因为 Key 检查先失败)- 验证传递给
checkCostLimits的参数包含正确的daily_reset_mode和daily_reset_time这直接验证了 PR 描述中的问题场景。
142-191: Key 优先级和 User 回退测试完善。
- 测试 2 验证 Key 每日限额优先于 User 每日限额检查
- 测试 3 验证当 Key 未设置每日限额时,User 每日限额仍能正常拦截
这确保了修复不会破坏原有的 User 每日限额功能。
193-409: 完整覆盖所有 12 个限额检查场景。测试用例覆盖了:
- Key/User 总限额 (
usd_total)- Key 并发 Session (
concurrent_sessions)- User RPM (
rpm)- Key/User 5h (
usd_5h)- Key/User 每日 (
daily_quota)- Key/User 周 (
usd_weekly)- Key/User 月 (
usd_monthly)- 所有限额通过时放行
使用
mockResolvedValueOnce链式调用正确模拟了检查顺序。
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: XL
- Lines changed: 1219
- Files changed: 9
- Split suggestion: Consider splitting into (1) rate-limit behavior fixes (
src/app/v1/_lib/proxy/rate-limit-guard.ts,src/lib/rate-limit/service.ts) and (2) test additions + coverage config (tests/unit/**,vitest.quota.config.ts).
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
There was a problem hiding this comment.
Code Review Summary
This PR successfully fixes a critical bug where Key daily limits were not enforced when User daily quotas were set to null. The implementation is clean, well-tested, and follows existing patterns.
PR Size: XL
- Lines changed: 1,219 (1,211 additions, 8 deletions)
- Files changed: 9
Split Recommendation: This PR is appropriately sized. While it's XL due to extensive test coverage (1,111 test lines), the core logic change is minimal (~44 lines). The tests are essential for ensuring the fix works correctly and should remain in this PR.
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 |
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Excellent (1,111 new test lines with dedicated coverage config)
- Code clarity - Good
Key Strengths
-
Minimal, Focused Change: The core fix adds exactly one daily limit check for Keys in the correct order (step 7, before User daily check at step 8).
-
Pattern Consistency: The new code duplicates the existing User daily quota check pattern (lines 242-283), ensuring maintainability.
-
Comprehensive Test Coverage:
- 1,111 lines of unit tests
- Dedicated vitest.quota.config.ts with 80% coverage threshold
- Tests explicitly cover the bug scenario (Key limit active, User limit null)
- Tests cover both cache hit/miss and rolling/fixed modes
-
Error Message Fix: Correctly distinguishes User from Provider in error messages (service.ts:465).
-
Documentation Updated: Comment block correctly reflects new check order (5-8 instead of 5-7).
Automated review by Claude AI
There was a problem hiding this comment.
Reviewed PR #531 and pulled the full diff + changed file list.
- Applied the
size/XLlabel. - Submitted a PR review summary comment (no high-confidence issues found in the diff lines).
- No inline review comments were posted because nothing met the “must-fix” bar within the changed lines.
背景
当用户层面限额被取消(如 user.dailyQuota = null)时,当前代理限流链路未检查 Key 的每日限额(key.limitDailyUsd),导致 Key 层限额不生效。
Root Cause:
PR #499 将新用户的默认 dailyQuota 从 100 改为 null (无限制),暴露了限流检查顺序的缺陷:原逻辑在用户每日额度(步骤 7)检查中遇到 null 时直接跳过,从不检查 Key 每日限额。
Related Work:
变更
核心变更
1. src/app/v1/_lib/proxy/rate-limit-guard.ts
2. src/lib/rate-limit/service.ts
3. 测试文件(新增)
4. vitest.quota.config.ts(新增)
影响范围
Breaking Changes
✅ 无破坏性变更
验证
测试场景
场景 1: 用户限额为 null,Key 限额为 10
场景 2: Key 限额为 null,用户限额为 10
场景 3: Key 限额超限,用户限额正常
关联
Description enhanced by Claude AI