feat(providers): add configurable weekly reset time for rate limiting#695
feat(providers): add configurable weekly reset time for rate limiting#695ylxmf2005 wants to merge 12 commits intoding113:devfrom
Conversation
Add the ability to configure which day of the week and time (HH:mm) the weekly cost window resets for providers. Changes: - Add weeklyResetDay (0-6) and weeklyResetTime (HH:mm) columns to providers table - Extend time-utils.ts to support custom weekly reset day/time parameters - Update rate-limit service to use configurable weekly reset with Redis key suffix - Add Weekly Reset Settings UI in provider limits section - Add i18n translations for all 5 languages (en, zh-CN, zh-TW, ja, ru) - Update related tests for new function signatures Default behavior remains Monday 00:00 for backward compatibility. Closes ding113#694
📝 WalkthroughWalkthrough为按提供商配置的每周重置添加数据库字段、表单与类型、UI 文案、验证与单元测试，并将周重置参数贯穿到速率限制时间计算、Redis 键命名/TTL、以及费用检查与追踪路径中；另含少量未使用导入/依赖调整与 FUNDING 文件新增。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @ylxmf2005, 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 significantly enhances the flexibility of provider rate limiting by allowing administrators to define a custom weekly reset day and time. This change moves away from a fixed Monday 00:00 reset, enabling more tailored quota management for individual providers. The implementation includes comprehensive updates across the database schema, backend logic for time calculations and Redis interactions, frontend user interface for configuration, and robust type and validation checks to ensure data integrity and a seamless user experience. 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
|
github-actions
bot
added
enhancement
![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 successfully implements configurable weekly reset times for provider rate limiting, moving away from the hardcoded Monday 00:00 reset. The changes are comprehensive, touching the database schema, backend logic, frontend UI, and tests. The implementation for calculating custom time windows and TTLs is well-handled.
I've found one critical issue in the rate-limiting service where the Redis key for checking weekly limits for keys and users was constructed incorrectly, which would cause weekly limits for them to not function as expected. I've provided a specific comment with a suggested fix for this.
Overall, this is a great feature addition that enhances the flexibility of the rate-limiting system.
Additional Comments (3)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/lib/rate-limit/service.ts
Line: 1292:1293
Comment:
`getCurrentCostBatch` reads from old Redis key format `cost_weekly` but `trackCost` writes to new format `cost_weekly_{day}_{HHmm}` (line 769). This mismatch will cause weekly cost tracking to always return 0 after providers configure custom weekly reset times.
```suggestion
// Weekly - use provider-specific reset config
const weeklyResetDay = provider.weeklyResetDay ?? 1;
const weeklyResetTime = provider.weeklyResetTime ?? "00:00";
const wSuffix = RateLimitService.resolveWeeklyReset(weeklyResetDay, weeklyResetTime).suffix;
pipeline.get(`provider:${providerId}:cost_weekly_${wSuffix}`);
queryMeta.push({ providerId, period: "weekly", isRolling: false });
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/lib/rate-limit/service.ts
Line: 1221:1223
Comment:
`dailyResetConfigs` parameter needs to include weekly reset config for proper key generation. The function needs access to `weeklyResetDay` and `weeklyResetTime` per provider to construct correct Redis keys.
```suggestion
static async getCurrentCostBatch(
providerIds: number[],
dailyResetConfigs: Map<number, { resetTime?: string | null; resetMode?: string | null }>,
weeklyResetConfigs: Map<number, { resetDay?: number | null; resetTime?: string | null }>
): Promise<
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/lib/rate-limit/service.ts
Line: 1292:1293
Comment:
Update to use new Redis key format with weekly reset suffix to match `trackCost` behavior (line 769 uses `cost_weekly_{day}_{HHmm}`)
```suggestion
// Weekly - need provider weekly reset config to construct correct key
// TODO: Add weeklyResetConfigs parameter to function signature
pipeline.get(`provider:${providerId}:cost_weekly_1_0000`); // Currently hardcoded to Monday 00:00
queryMeta.push({ providerId, period: "weekly", isRolling: false });
```
How can I resolve this? If you propose a fix, please make it concise. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR adds database schema and UI for configurable weekly reset time for provider rate limiting but the implementation is incomplete. The newly added weeklyResetDay and weeklyResetTime fields exist in the database and UI but are NOT used in the actual rate limiting logic.
PR Size: XL
- Lines changed: 3394 (3352 additions + 42 deletions)
- Files changed: 22
Recommendation: This is a large PR that mixes database migration, UI changes, and i18n updates. Consider splitting future PRs into smaller, focused changes.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 2 | 0 | 0 | 0 |
| Implementation | 1 | 1 | 0 | 0 |
Critical Issues (Must Fix)
1. [LOGIC-BUG][CRITICAL] Weekly reset fields added to database but not used in rate limiting logic
The PR adds weekly_reset_day and weekly_reset_time columns to the providers table (drizzle/0060_minor_bloodstorm.sql:2-3) but these fields are never passed to or used by the rate limiting system.
Evidence:
src/lib/rate-limit/time-utils.ts:65-70-getTimeRangeForPeriod()for weekly period is UNCHANGED and still hardcoded to Monday 00:00src/lib/rate-limit/service.ts:162-checkCostLimits()weekly limit has no resetTime/resetDay parameterssrc/lib/rate-limit/service.ts:707-708-trackCost()usescost_weeklyRedis key without reset time suffix (unlike daily which usescost_daily_{HHmm})
Why this is a problem: Users can configure weekly reset settings in the UI, but the configuration will be silently ignored. The weekly reset will always occur on Monday at 00:00 regardless of what the user configures.
Suggested fix: Add weeklyResetDay and weeklyResetTime parameters to the time utils functions, similar to how daily reset works:
// src/lib/rate-limit/time-utils.ts
export async function getTimeRangeForPeriod(
period: TimePeriod,
resetTime = "00:00",
weeklyResetDay?: number, // ADD THIS
weeklyResetTime?: string // ADD THIS
): Promise<TimeRange> {
// ...
case "weekly": {
const day = weeklyResetDay ?? 1; // Default Monday
const time = normalizeResetTime(weeklyResetTime ?? "00:00");
const zonedNow = toZonedTime(now, timezone);
const zonedStartOfWeek = getCustomWeeklyResetTime(zonedNow, day, time, timezone);
startTime = fromZonedTime(zonedStartOfWeek, timezone);
break;
}
}2. [LOGIC-BUG][CRITICAL] Missing Redis key suffix for weekly reset configuration
The Redis key for weekly cost tracking is cost_weekly (src/lib/rate-limit/service.ts:707-708) without any reset time suffix. This means if weekly reset configuration is implemented, all providers with different weekly reset times will share the same Redis key, causing data corruption.
Evidence:
- Daily fixed mode uses
cost_daily_{HHmm}suffix (service.ts:702) - Daily rolling mode uses
cost_daily_rollingsuffix (service.ts:677) - Weekly uses
cost_weeklywith NO suffix (service.ts:707)
Why this is a problem: When multiple providers have different weekly reset configurations (e.g., Provider A resets Friday 18:00, Provider B resets Monday 00:00), they will overwrite each other's cost data in Redis.
Suggested fix: Follow the daily reset pattern and include reset config in the Redis key:
// src/lib/rate-limit/service.ts:119-136
private static resolveWeeklyReset(resetDay?: number, resetTime?: string): { day: number; time: string; suffix: string } {
const day = resetDay ?? 1; // Default Monday
const time = normalizeResetTime(resetTime ?? "00:00");
const suffix = `${day}_${time.replace(":", "")}`; // e.g., "1_0000" for Monday 00:00
return { day, time, suffix };
}
// Then use it like:
const weeklyKey = `provider:${providerId}:cost_weekly_${weeklyReset.suffix}`;High Priority Issues (Should Fix)
1. [IMPLEMENTATION-INCOMPLETE][HIGH] Frontend UI added but backend implementation missing
The PR adds UI components for weekly reset configuration (src/app/[locale]/settings/providers/_components/forms/provider-form/sections/limits-section.tsx) and i18n translations for 5 languages, but the backend doesn't actually use these values.
Impact: This creates a misleading user experience where users think they're configuring weekly resets, but the configuration has no effect. This violates the principle of least astonishment and will lead to user confusion and support tickets.
Suggested fix: Either:
- Option A: Complete the implementation to actually use the fields
- Option B: Remove the UI and wait until the backend is ready
Review Coverage
- Logic and correctness - 2 Critical bugs found
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Tests updated for daily reset but NO tests for weekly reset
- Code clarity - Good
Overall Assessment: The PR adds database schema and UI for a feature that isn't actually implemented in the business logic. This is a blocker that must be fixed before merging. The database migration and i18n work is good, but the core functionality is missing.
Automated review by Claude AI
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/actions/providers.ts (2)
458-464:⚠️ Potential issue | 🟡 MinoraddProvider 入参类型缺少 weekly_reset_day / weekly_reset_time
当前入参类型未包含周重置字段，调用方若使用对象字面量会触发多余属性检查或直接漏传。建议补齐类型定义。
建议修改
@@ limit_weekly_usd?: number | null; + weekly_reset_day?: number | null; + weekly_reset_time?: string | null; limit_monthly_usd?: number | null;
1376-1383:⚠️ Potential issue | 🟠 Major批量用量的 weekly resetAt 未使用供应商配置
rangeWeekly 已按供应商配置计算，但 resetWeekly 仍使用默认周一 00:00，会导致自定义周重置的 resetAt 展示错误。建议传入 weeklyResetDay/weeklyResetTime。
建议修改
@@ - const resetWeekly = await getResetInfo("weekly"); + const resetWeekly = await getResetInfo( + "weekly", + "00:00", + provider.weeklyResetDay ?? undefined, + provider.weeklyResetTime ?? undefined + );src/lib/rate-limit/service.ts (1)
1377-1409:⚠️ Potential issue | 🟠 MajorLeaseService 未接收 weeklyResetDay，导致周窗口重置时间失效
当前代码在 service.ts 中计算了 weeklyResetDay 但未传入 LeaseService.getCostLease，导致周窗口时间范围计算始终使用默认周一。建议修改步骤：
- 在 GetCostLeaseParams 接口中增加
weeklyResetDay?: number和weeklyResetTime?: string字段- 在 getCostLease 调用处传递这两个参数
- 在 refreshCostLeaseFromDb 中将这两个参数转发给 getLeaseTimeRange
🤖 Fix all issues with AI agents
In `@drizzle/0060_minor_bloodstorm.sql`:
- Around line 1-3: The migration was handwritten instead of using the project
generator; remove the manual SQL file (drizzle/0060_minor_bloodstorm.sql), make
the corresponding schema changes in src/drizzle/schema.ts (add the providers
fields weekly_reset_day and weekly_reset_time and remove the default from
notification_target_bindings.schedule_timezone), then run bun run db:generate to
produce the proper migration SQL and commit the generated migration and updated
schema.ts; do not re-add handwritten SQL files.
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/limits-section.tsx:
- Around line 247-302: The time input placeholder "00:00" is hardcoded and
should use i18n; update the Input in the Weekly Reset Settings (the Input with
id set via isEdit ? "edit-weekly-reset-time" : "weekly-reset-time", value
state.rateLimit.weeklyResetTime, and onChange dispatching
"SET_WEEKLY_RESET_TIME") to use
t("sections.rateLimit.weeklyResetTime.placeholder") instead of the literal
"00:00", then add that translation key with appropriate strings for the five
supported languages in the locale files.
In `@src/lib/rate-limit/service.ts`:
- Around line 264-277: The weekly key format is inconsistent: reads use
cost_weekly_{day}_{HHmm} but writes/backfill still use cost_weekly and ignore
weeklyResetDay/time; update all code paths that compute periodKey (the snippet
using RateLimitService.resolveDailyReset and resolveWeeklyReset and the variable
periodKey) plus write/backfill functions to use the unified key pattern
cost_weekly_{day}_{HHmm} and set TTL computed from resolveWeeklyReset; implement
a migration on reads (in getCurrentCost/getCurrentCostBatch and similar readers)
that checks the old key, if present reads it, writes the value into the new
cost_weekly_{day}_{HHmm} key with proper TTL, then deletes the old key, and
ensure all DB-backfill routines also write the new key format and TTL so weekly
custom start day/time work consistently.
- Around line 179-185: The hardcoded user-facing label name: "周" in the
rate-limit entry must use i18n; replace the literal with a translation lookup
(e.g., call the existing i18n/t function or import the translator used across
the app) so the object using limits.limit_weekly_usd and weeklyReset.{day,time}
provides a localized name (e.g., i18n.t('rate_limit.period.week') or the
project's equivalent key); ensure you add/verify the corresponding i18n key in
resources and use the same translation helper used elsewhere in
src/lib/rate-limit/service.ts.
In `@src/lib/validation/schemas.ts`:
- Around line 419-430: The schema validators weekly_reset_day
(z.coerce.number()... .int/.min/.max) and weekly_reset_time
(z.string().regex(...)) currently use hard-coded Chinese error messages; replace
those string literals with i18n keys (or calls to the project's translation
helper) so validation errors return consistent i18n keys instead of raw text —
update the .int, .min, .max and .regex call arguments to reference the
appropriate i18n key constants or t('...') keys used elsewhere in the codebase
and ensure nullable().optional() behavior is preserved.
- Around line 419-430: Update the UpdateProviderSchema in
src/lib/validation/schemas.ts to include the same weekly_reset_day and
weekly_reset_time fields as CreateProviderSchema so update requests don't drop
them; add weekly_reset_day with
z.coerce.number().int("每周重置日必须是整数").min(0,"每周重置日范围为 0-6").max(6,"每周重置日范围为
0-6").nullable().optional() and weekly_reset_time with
z.string().regex(/^([01]?[0-9]|2[0-3]):[0-5][0-9]$/,"重置时间格式必须为
HH:mm").nullable().optional(), matching the exact validators used in
CreateProviderSchema.
🧹 Nitpick comments (2)
drizzle/meta/0060_snapshot.json (1)
1773-1784: 建议在数据库层补充 weekly_reset_day / weekly_reset_time 约束这两列目前为可空且无 CHECK 约束，若存在手工 SQL/历史回填，越界值会进入库。建议在迁移里补充约束（day 0–6、time 为 HH:mm），以提升数据一致性与可诊断性。
src/types/provider.ts (1)
92-95: 明确 weeklyResetDay/weeklyResetTime 的 null 与 undefined 语义建议在类型层面补充注释（或统一类型别名）来区分 null 与 undefined 的含义，避免创建/更新时语义混淆（例如 null=显式使用默认值，undefined=不设置/不更新）。
建议修改
@@ - weeklyResetDay: number | null; - weeklyResetTime: string | null; + // null 表示使用系统默认周一 00:00 + weeklyResetDay: number | null; + // null 表示使用系统默认时间 00:00 + weeklyResetTime: string | null; @@ - weekly_reset_day?: number | null; - weekly_reset_time?: string | null; + // undefined 表示沿用默认；null 表示显式使用系统默认 + weekly_reset_day?: number | null; + weekly_reset_time?: string | null; @@ - weekly_reset_day?: number | null; - weekly_reset_time?: string | null; + // undefined 表示不更新；null 表示显式使用系统默认 + weekly_reset_day?: number | null; + weekly_reset_time?: string | null;Based on learnings: In TypeScript interfaces, explicitly document and enforce distinct meanings for null and undefined. Example: for numeric limits like limitTotalUsd, use 'number | null | undefined' when null signifies explicitly unlimited (e.g., matches DB schema or special UI logic) and undefined signifies 'inherit default'. This pattern should be consistently reflected in type definitions across related fields to preserve semantic clarity between database constraints and UI behavior.
Also applies to: 268-271, 341-343
src/app/[locale]/settings/providers/_components/forms/provider-form/sections/limits-section.tsx
Show resolved
Hide resolved
Critical fixes:
- Fix Redis key mismatch for keys/users weekly limits (Gemini review)
Only providers have configurable weekly reset, so keys/users now use
'cost_weekly' without suffix instead of 'cost_weekly_{day}_{HHmm}'
- Fix cache warming for weekly to differentiate entity types
Providers use weekly_${suffix}, keys/users use plain 'weekly'
- Fix getCurrentCost and getCurrentCostBatch for weekly suffix
Added weeklyResetDay/weeklyResetTime params to getCurrentCost
Extended resetConfigs parameter type in getCurrentCostBatch
High priority fixes:
- Add weekly_reset_day/time fields to UpdateProviderSchema (CodeRabbit)
- Add weekly_reset_day/time fields to addProvider input type (CodeRabbit)
- Fix getProviderLimitUsageBatch resetWeekly to use provider config
Test updates:
- Update cost-limits.test.ts to use correct Redis key for users
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx
Line: 214:216
Comment:
`weekly_reset_day` and `weekly_reset_time` missing from form submission payload. UI allows users to set these values but they're never saved.
```suggestion
limit_weekly_usd: state.rateLimit.limitWeeklyUsd,
weekly_reset_day: state.rateLimit.weeklyResetDay,
weekly_reset_time: state.rateLimit.weeklyResetTime,
limit_monthly_usd: state.rateLimit.limitMonthlyUsd,
```
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/rate-limit/service.ts (1)
1424-1463:⚠️ Potential issue | 🟠 MajorLease 路径未传入 weeklyResetDay，周窗口可能按默认周一起算
checkCostLimitsWithLease 解析 weeklyReset 但仅把 resetTime 传给 LeaseService；若 provider 配置了非默认周起始日，租约窗口与实际周重置可能不一致。建议扩展 LeaseService 接口/实现以接收 weeklyResetDay/time 并用于时间窗口计算。
🤖 Fix all issues with AI agents
In `@src/lib/rate-limit/service.ts`:
- Around line 811-813: The DB fallback in getCurrentCost currently ignores
weeklyResetDay and weeklyResetTime, causing Redis misses to be backfilled with
incorrect TTLs; update getCurrentCost to pass weeklyResetDay and weeklyResetTime
through the DB query and TTL calculation paths (including any helper functions
used for computing time ranges/ttl) so the DB lookup and the Redis backfill use
the same weekly reset parameters; ensure the same change is applied to the
analogous logic around lines 882-894 (the secondary DB fallback/backfill branch)
so both DB query and Redis write include weeklyResetDay/weeklyResetTime and
respect resetMode/DailyResetMode.
- Around line 512-527: The weekly provider branch builds a periodKey with a
weekly suffix but still computes ttl via getTTLForPeriodWithMode without passing
weeklyResetDay/time, causing mismatched expirations; update the weekly branch in
RateLimitService (around resolveWeeklyReset usage) to call a TTL helper that
includes the weekly config (either call getTTLForPeriod with the resolved
weeklyResetDay/weeklyResetTime or extend getTTLForPeriodWithMode to accept the
weekly reset params) so the TTL calculation uses the same suffix/time used for
periodKey.
Fixed unused imports across 11 files to pass CI lint check. These were pre-existing issues in the codebase, not related to the weekly reset config feature.
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx
Line: 214:216
Comment:
`weekly_reset_day` and `weekly_reset_time` missing from form submission
```suggestion
limit_weekly_usd: state.rateLimit.limitWeeklyUsd,
weekly_reset_day: state.rateLimit.weeklyResetDay,
weekly_reset_time: state.rateLimit.weeklyResetTime,
limit_monthly_usd: state.rateLimit.limitMonthlyUsd,
```
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[locale]/dashboard/availability/_components/endpoint/probe-terminal.tsx (1)
81-85:⚠️ Potential issue | 🟠 Major自动滚动在新日志到达时不会触发
当前
useEffect的依赖数组仅包含autoScroll和userScrolled，但logs属性变化（新日志到达）时不会触发 effect 重新执行，与注释"新日志到达自动滚动"的意图不符。应在依赖数组中添加logs.length以确保新日志到达时自动滚动到底部。建议修复
- }, [autoScroll, userScrolled]); + }, [autoScroll, userScrolled, logs.length]);
🧹 Nitpick comments (2)
src/app/[locale]/dashboard/availability/_components/endpoint/latency-curve.tsx (1)
124-126: 建议：将硬编码的 "OK"/"FAIL" 字符串国际化。根据编码规范，所有面向用户的字符串应使用 i18n。当前代码中
"OK"和"FAIL"是硬编码的。♻️ 建议的修复
- <div className={cn("text-xs", data?.ok ? "text-emerald-500" : "text-rose-500")}> - {data?.statusCode || (data?.ok ? "OK" : "FAIL")} - </div> + <div className={cn("text-xs", data?.ok ? "text-emerald-500" : "text-rose-500")}> + {data?.statusCode || (data?.ok ? t("statusOk") : t("statusFail"))} + </div>需要在对应的 i18n 文件中添加
statusOk和statusFail键。src/app/[locale]/dashboard/_components/user/forms/limit-rule-picker.tsx (1)
103-112: 建议为未来维护添加 useMemo 保护。当前
availableTypes直接从模块常量LIMIT_TYPE_OPTIONS派生，依赖数组[open]已足够。但若后续改为动态生成（如基于 props 或 state），该依赖会失效导致未重置默认值。建议使用useMemo包装availableTypes并将其加入依赖数组，提升代码的防御性和可维护性。建议改动
- const availableTypes = LIMIT_TYPE_OPTIONS; + const availableTypes = useMemo(() => LIMIT_TYPE_OPTIONS, []); const [type, setType] = useState<LimitType | "">(""); @@ -110,1 +110,1 @@ setError(null); - }, [open]); + }, [open, availableTypes]);
The form UI collected weeklyResetDay and weeklyResetTime values but they were never included in the submission payload, causing the feature to silently fail. Also adds comprehensive test coverage for configurable weekly reset: - time-utils: 8 tests for custom day/time window calculation - cost-limits: 6 tests for Redis key format and service behavior - providers-usage: verify parameter passing
Fixes issues where DB fallback in getCurrentCost and checkCostLimitsFromDatabase would ignore custom weekly reset configuration, causing: - Incorrect time range calculation (defaulting to Monday 00:00) - Wrong TTL calculation for Redis cache warming Changes: - getCurrentCost: pass weeklyResetDay/time to getTimeRangeForPeriodWithMode - getCurrentCost: pass weeklyResetDay/time to getTTLForPeriodWithMode - checkCostLimitsFromDatabase: use getTTLForPeriod with weekly params for provider
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts
Show resolved
Hide resolved
src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/lib/rate-limit/service.ts`:
- Around line 1441-1451: checkCostLimitsWithLease computes weeklyReset
(weeklyReset.day and .time) but does not pass weeklyReset.day into
LeaseService.getCostLease because getLeaseTimeRange/GetCostLeaseParams lack a
weeklyResetDay; update the GetCostLeaseParams interface to include
weeklyResetDay, propagate that field through getLeaseTimeRange into
LeaseService.getCostLease, and ensure getLeaseTimeRange forwards weeklyResetDay
to getTimeRangeForPeriodWithMode (or alternatively call
getTimeRangeForPeriodWithMode directly from checkCostLimitsWithLease) so custom
provider weekly reset days are used when computing the weekly window.
…errors - Change period names from Chinese to English (5小时→5-hour, 每日→daily, 周→weekly, 月→monthly) - Change type name from Chinese to English (供应商→Provider) - Change error message template from Chinese to English This makes API error messages language-neutral and consistent.
- Remove hardcoded Chinese messages from weekly_reset_day and weekly_reset_time validation - Add TIME_FORMAT_INVALID and WEEKLY_RESET_DAY_INVALID error codes - Add translations for all 5 languages (en, zh-CN, zh-TW, ja, ru) - Let Zod's default errors be translated by the zod-i18n system
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)
src/lib/rate-limit/service.ts (1)
1321-1406:⚠️ Potential issue | 🟠 Major批量周成本读取缺少旧 key 兼容，可能返回 0
getCurrentCostBatch 只读cost_weekly_{suffix}，若 Redis 仍保留旧cost_weekly，批量结果会被读成 0 且不会迁移，导致周成本展示不准。建议双读旧 key 并在命中时回填新 key（含 TTL）。建议改法（示例）
- const queryMeta: Array<{ - providerId: number; - period: "5h" | "daily" | "weekly" | "monthly"; - isRolling: boolean; - }> = []; + const queryMeta: Array<{ + providerId: number; + period: "5h" | "daily" | "weekly" | "weekly_legacy" | "monthly"; + isRolling: boolean; + }> = []; @@ - const weeklySuffix = RateLimitService.resolveWeeklyReset( + const weeklySuffix = RateLimitService.resolveWeeklyReset( config?.weeklyResetDay, config?.weeklyResetTime ).suffix; + const weeklyKey = `provider:${providerId}:cost_weekly_${weeklySuffix}`; + const weeklyLegacyKey = `provider:${providerId}:cost_weekly`; @@ - pipeline.get(`provider:${providerId}:cost_weekly_${weeklySuffix}`); - queryMeta.push({ providerId, period: "weekly", isRolling: false }); + pipeline.get(weeklyKey); + queryMeta.push({ providerId, period: "weekly", isRolling: false }); + pipeline.get(weeklyLegacyKey); + queryMeta.push({ providerId, period: "weekly_legacy", isRolling: false }); @@ switch (meta.period) { @@ case "weekly": providerData.costWeekly = cost; break; + case "weekly_legacy": + if (providerData.costWeekly === 0 && cost > 0) { + providerData.costWeekly = cost; + } + break;
🤖 Fix all issues with AI agents
In `@src/lib/rate-limit/service.ts`:
- Around line 170-186: The costLimits array is using hardcoded English display
strings for the CostLimit.name (e.g., "5-hour", "daily", "weekly", "monthly")
and corresponding reason templates—replace these with i18n keys and call the
project translation helper (e.g., use the i18n get/translate function) to fetch
localized strings instead of hardcoding; update CostLimit entries in costLimits
and the other affected spots (around the referenced blocks ~296-301, ~553-557,
~1517-1522) to store either i18n keys or already-translated text, and change any
code that builds user-facing "reason" templates to use translated strings,
ensuring the new keys are added to all five locales (zh-CN, zh-TW, en, ja, ru).
src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts
Show resolved
Hide resolved
src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsx
Show resolved
Hide resolved
src/app/[locale]/settings/providers/_components/forms/provider-form/sections/limits-section.tsx
Show resolved
Hide resolved
src/app/[locale]/settings/providers/_components/forms/provider-form/sections/limits-section.tsx
Show resolved
Hide resolved
Review Comments Resolution SummaryAll review comments have been resolved. Here's the summary: Fixed Issues ✅
Not Applicable / By Design 📋
New i18n Error Codes Added 🌐
Translations added for all 5 languages: en, zh-CN, zh-TW, ja, ru |
The production code was changed to return English error messages
("spending limit exceeded") but the test assertions still expected
Chinese strings. This fixes the mismatch for weekly limit tests.
src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx
Show resolved
Hide resolved
- Keys use fixed Monday 00:00 (only providers support configurable weekly reset) - Form defaults null values to Monday 00:00 on edit
1 participant
Summary
Added configurable weekly reset time (day of week + HH:mm) for provider rate limiting. Administrators can now specify when the weekly cost window resets for each provider, moving beyond the previous hardcoded Monday 00:00 behavior.
This implementation follows the existing daily reset configuration pattern and maintains full backward compatibility with nullable fields defaulting to Monday 00:00.
Changes
Database schema:
weeklyResetDay(integer, 0-6 where 0=Sunday) toproviderstable insrc/drizzle/schema.ts:225weeklyResetTime(varchar(5), HH:mm format) toproviderstable insrc/drizzle/schema.ts:226drizzle/0060_minor_bloodstorm.sqlwith nullable columnsBackend logic:
getTimeRangeForPeriod,getTTLForPeriod,getResetInfoinsrc/lib/rate-limit/time-utils.ts:67-195to accept optionalweeklyResetDayandweeklyResetTimeparametersresolveWeeklyReset()helper insrc/lib/rate-limit/service.ts:119-136to generate Redis key suffixcost_weeklytocost_weekly_{day}_{HHmm}insrc/lib/rate-limit/service.ts:769checkCostLimitsinterface insrc/lib/rate-limit/service.ts:150-187to accept weekly reset configtrackCostto pass weekly config through proxy layer insrc/app/v1/_lib/proxy/response-handler.ts:2007Frontend UI:
weeklyResetDayandweeklyResetTimetoRateLimitStateinsrc/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts:58-59SET_WEEKLY_RESET_DAYandSET_WEEKLY_RESET_TIMEreducer actions inprovider-form-context.tsx:185-187sections/limits-section.tsx:248-290with day-of-week Select and time InputInternationalization:
weeklyResetDay,weeklyResetTime, day names (Sunday-Saturday), and section title translationsmessages/{en,zh-CN,zh-TW,ja,ru}/settings/providers/form/sections.jsonType safety:
Provider,ProviderDisplay,CreateProviderData,UpdateProviderDatainterfaces insrc/types/provider.ts:92-95,180-182,268-270,333-335weekly_reset_day(0-6 integer) andweekly_reset_time(HH:mm regex) insrc/lib/validation/schemas.ts:826-837Server actions:
src/actions/providers.ts:537-539to map weekly reset fieldsgetProviderLimitUsageBatchparameter type insrc/actions/providers.ts:1293-1296to include weekly configTesting
Updated existing tests for new function signatures:
tests/unit/lib/rate-limit/cost-limits.test.ts:128to use new Redis key formatcost_weekly_1_0000(Monday 00:00)tests/unit/actions/providers-usage.test.ts:51-57,193to include weekly reset parameters in mock functionsManual testing verification:
Testing checklist for reviewers:
Related Issue
Closes #694
Greptile Overview
Greptile Summary
Added configurable weekly reset time (day of week + HH:mm) for provider rate limiting, moving from hardcoded Monday 00:00 to administrator-controlled reset schedules. The implementation:
Architecture:
weekly_reset_day(0-6) andweekly_reset_time(HH:mm) columnscost_weeklytocost_weekly_{day}_{HHmm}for providers only (keys/users continue using non-suffixed keys)time-utils.tsto accept optional weekly reset parameters with Monday 00:00 defaultsresolveWeeklyReset()helper to generate consistent Redis key suffixesData Flow:
getTimeRangeForPeriod(),getTTLForPeriod(),getResetInfo()for accurate window calculationsgetCurrentCostBatch()correctly handle per-provider weekly configurationsUI/UX:
Testing:
cost_weekly_1_0000)Backward Compatibility:
Confidence Score: 5/5
Important Files Changed
weeklyResetDay(integer) andweeklyResetTime(varchar(5)) columns to providers tablecost_weekly_5_1800), only for providers (keys/users use default)Sequence Diagram
sequenceDiagram participant User participant UI as Provider Form UI participant Action as Server Action participant DB as PostgreSQL participant Proxy as Request Proxy participant RateLimit as RateLimitService participant Redis participant TimeUtils as Time Utils Note over User,TimeUtils: Configuration Phase User->>UI: Set weekly reset (e.g., Friday 18:00) UI->>UI: Dispatch SET_WEEKLY_RESET_DAY (5) UI->>UI: Dispatch SET_WEEKLY_RESET_TIME ("18:00") User->>UI: Submit form UI->>Action: editProvider/addProvider(weeklyResetDay: 5, weeklyResetTime: "18:00") Action->>Action: Validate schema (0-6, HH:mm) Action->>DB: UPDATE providers SET weekly_reset_day=5, weekly_reset_time='18:00' DB-->>Action: Success Action-->>UI: Provider updated Note over User,TimeUtils: Runtime - Request Processing User->>Proxy: API Request Proxy->>DB: Fetch provider config DB-->>Proxy: Provider (weeklyResetDay: 5, weeklyResetTime: "18:00") Proxy->>RateLimit: checkCostLimits(providerId, limits) RateLimit->>RateLimit: resolveWeeklyReset(5, "18:00") → {day: 5, time: "18:00", suffix: "5_1800"} RateLimit->>Redis: GET provider:123:cost_weekly_5_1800 Redis-->>RateLimit: current cost (e.g., 150.00) alt Cost within limit RateLimit-->>Proxy: {allowed: true} Proxy->>Proxy: Process request Proxy->>RateLimit: trackCost(providerId, cost, options) RateLimit->>TimeUtils: getTTLForPeriod("weekly", "00:00", 5, "18:00") TimeUtils->>TimeUtils: Calculate next Friday 18:00 TimeUtils-->>RateLimit: TTL in seconds RateLimit->>Redis: INCRBYFLOAT provider:123:cost_weekly_5_1800 RateLimit->>Redis: EXPIRE provider:123:cost_weekly_5_1800 {TTL} Redis-->>RateLimit: Success RateLimit-->>Proxy: Cost tracked Proxy-->>User: Response else Cost exceeds limit RateLimit-->>Proxy: {allowed: false, reason: "Provider weekly spending limit exceeded"} Proxy-->>User: 429 Rate Limit Error end Note over User,TimeUtils: Usage Display User->>UI: View provider usage stats UI->>Action: getProviderLimitUsage(providerId) Action->>DB: Fetch provider config DB-->>Action: Provider (weeklyResetDay: 5, weeklyResetTime: "18:00") Action->>TimeUtils: getTimeRangeForPeriod("weekly", "00:00", 5, "18:00") TimeUtils->>TimeUtils: Calculate current week's Friday 18:00 window TimeUtils-->>Action: {startTime, endTime} Action->>RateLimit: getCurrentCost(providerId, "weekly", ...) RateLimit->>Redis: GET provider:123:cost_weekly_5_1800 Redis-->>RateLimit: 150.00 RateLimit-->>Action: 150.00 Action->>TimeUtils: getResetInfo("weekly", "00:00", 5, "18:00") TimeUtils->>TimeUtils: Calculate next Friday 18:00 TimeUtils-->>Action: {resetAt: Date, type: "natural"} Action-->>UI: {current: 150, limit: 200, resetAt: "2026-02-05T18:00:00Z"} UI-->>User: Display usage bar with reset countdown