Skip to content

total quota fix#1124

Closed
ilnli wants to merge 3 commits into
ding113:devfrom
ilnli:dev
Closed

total quota fix#1124
ilnli wants to merge 3 commits into
ding113:devfrom
ilnli:dev

Conversation

@ilnli
Copy link
Copy Markdown
Contributor

@ilnli ilnli commented Apr 27, 2026

Summary

Adds total cost limit (limitTotalUsd) to provider and key quotas, enabling administrators to set absolute spending caps that never auto-reset. This complements the existing time-based quotas (5h, daily, weekly, monthly) with a permanent lifetime limit.

Related PRs

Problem

Previously, quota management only supported time-windowed limits (5h, daily, weekly, monthly). For long-term budget control and hard spending caps, administrators needed a total/lifetime cost limit that accumulates indefinitely until manually reset.

Solution

1. Total Cost Limit Implementation

Added new limitTotalUsd field support across the quota system:

Component Changes
Repository Layer Added sumProviderTotalCost() in src/repository/statistics.ts - aggregates all historical costs for a provider with optional reset date
Provider Actions Updated getProviderLimitUsage() and getProviderLimitUsageBatch() in src/actions/providers.ts to include total cost data
Key Quota UI Added total quota display and edit controls in key quota management dialogs
Provider Quota UI Added total quota visualization in provider quota list and client components

2. Key Features

  • Non-resetting accumulation: Total cost is calculated from resetAt (if set) or from the earliest ledger record
  • No 365-day truncation: Unlike other windows, total quota intentionally never auto-expires to enforce hard caps
  • Cross-provider consistency: Same limit field works for both provider-level and key-level quotas
  • Full i18n support: Added translations in all 5 languages (en, ja, ru, zh-CN, zh-TW)

Changes

Core Changes

  • src/repository/statistics.ts (+27 lines) - New sumProviderTotalCost() function
  • src/actions/providers.ts (+19/-4 lines) - Include limitTotalUsd in provider limit usage queries

Dashboard UI Changes

  • src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx (+30 lines) - Total quota input field
  • src/app/[locale]/dashboard/quotas/keys/_components/keys-quota-client.tsx (+50 lines) - Total quota column with progress bar
  • src/app/[locale]/dashboard/quotas/keys/_components/keys-quota-manager.tsx (+1 line) - Type definition update
  • src/app/[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsx (+9 lines) - Total quota display
  • src/app/[locale]/dashboard/quotas/providers/_components/providers-quota-client.tsx (+9/-1 lines) - Total limit check in quota calculation
  • src/app/[locale]/dashboard/quotas/providers/page.tsx (+1 line) - Type update

i18n Changes (5 Languages)

  • messages/en/quota.json (+9 lines) - English translations
  • messages/ja/quota.json (+9 lines) - Japanese translations
  • messages/ru/quota.json (+9 lines) - Russian translations
  • messages/zh-CN/quota.json (+9 lines) - Simplified Chinese translations
  • messages/zh-TW/quota.json (+9 lines) - Traditional Chinese translations

Breaking Changes

None. All changes are additive:

  • New quota field in existing data structures - consumers without total limit will see null (unlimited)
  • New UI elements only appear when total limit is configured

Testing

Verification Steps

  1. Navigate to Provider Quotas page - verify total quota displays correctly
  2. Edit a provider's key quotas - verify total quota field appears and saves
  3. Set a total limit on a key - verify progress bar shows usage percentage
  4. Create usage records - verify total cost accumulates correctly without auto-reset

Database Query Verification

The new sumProviderTotalCost() function uses the existing usageLedger table with proper billing condition filtering.

Checklist

  • Code follows project conventions (named exports, Biome formatting)
  • i18n strings added for all 5 languages
  • Type definitions updated consistently
  • No breaking changes introduced
  • UI components tested manually

Description enhanced by Claude AI

Greptile Summary

Adds a permanent limitTotalUsd spending cap to both provider and key quotas — complementing the existing time-windowed limits (5h, daily, weekly, monthly) with a lifetime accumulator that never auto-resets. The implementation is additive: null means unlimited, and all UI elements are guarded accordingly. The shared KeyQuota helper type in quota-helpers.ts is correctly updated with an optional costTotal field, and hasKeyQuotaSet / getMaxUsageRate (and transitively isWarning / isExceeded) now include it.

Confidence Score: 5/5

Safe to merge — all changes are additive, no breaking changes, and the shared helper types are updated correctly.

Only P2 findings: missing test assertions on the new limitTotalUsd output field, and a minor redundant condition in the provider list item. No P0 or P1 issues found; the previously flagged P1 (costTotal absent from quota-helpers) is addressed in this PR.

tests/unit/actions/providers-usage.test.ts — consider adding assertions to verify limitTotalUsd is present and correct in both getProviderLimitUsage and getProviderLimitUsageBatch output.

Important Files Changed

Filename Overview
src/repository/statistics.ts Adds sumProviderTotalCost() — queries usageLedger with no time truncation, correctly respecting an optional resetAt. Implementation is consistent with the analogous sumUserTotalCost / sumKeyTotalCost patterns.
src/actions/providers.ts Adds totalCost to both getProviderLimitUsage and getProviderLimitUsageBatch Promise.all arrays and exposes it as limitTotalUsd; resetAt is not passed to sumProviderTotalCost because providers have no such field yet — intentional per PR design.
src/lib/utils/quota-helpers.ts Adds optional costTotal field to KeyQuota type and updates hasKeyQuotaSet and getMaxUsageRateisWarning/isExceeded are covered transitively through getMaxUsageRate.
src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx Adds total-limit input and current-usage tooltip; the costTotal.limit truthiness guard (falsy at 0) for the tooltip was flagged in a previous review thread.
src/app/[locale]/dashboard/quotas/providers/_components/providers-quota-client.tsx Includes limitTotalUsd in both hasQuotaLimit (explicit !== null && > 0) and calculateMaxUsage; consistent with existing quota field treatment.
tests/unit/actions/providers-usage.test.ts Wires sumProviderTotalCostMock into existing setup but adds no assertions on the limitTotalUsd output field in either test suite.

Sequence Diagram

sequenceDiagram
    participant UI as Dashboard UI
    participant PA as providers.ts (action)
    participant ST as statistics.ts (repo)
    participant DB as usageLedger (DB)

    UI->>PA: getProviderLimitUsage(providerId)
    PA->>ST: sumProviderCostInTimeRange() x4
    PA->>ST: sumProviderTotalCost(providerId)
    ST->>DB: SELECT SUM(cost_usd) WHERE finalProviderId = ? [no time cutoff]
    DB-->>ST: totalCost
    ST-->>PA: totalCost
    PA-->>UI: { ..., limitTotalUsd: { current, limit } }

    UI->>PA: getProviderLimitUsageBatch(providers[])
    loop per provider
        PA->>ST: sumProviderTotalCost(provider.id)
        ST->>DB: SELECT SUM(cost_usd) WHERE finalProviderId = ?
        DB-->>ST: totalCost
        ST-->>PA: totalCost
    end
    PA-->>UI: Map<providerId, ProviderLimitUsageData>
Loading

Comments Outside Diff (1)

  1. src/lib/utils/quota-helpers.ts, line 8-14 (link)

    P1 costTotal missing from shared KeyQuota type and helper functions

    The PR adds costTotal to every component-level KeyQuota interface, but the canonical KeyQuota type in quota-helpers.ts is not updated. As a result, hasKeyQuotaSet, getMaxUsageRate, isWarning, and isExceeded all silently ignore costTotal. A key whose only limit is limitTotalUsd will be treated as having no quota at all — filter tabs ("key-quota", "warning", "exceeded") in keys-quota-manager.tsx will misclassify it, and the per-key status badge will show "正常" even after the total budget is exhausted.

    Add costTotal to the type and include it in the helper functions:

    // quota-helpers.ts
    export type KeyQuota = {
      cost5h: { current: number; limit: number | null };
      costDaily: { current: number; limit: number | null };
      costWeekly: { current: number; limit: number | null };
      costMonthly: { current: number; limit: number | null };
      costTotal: { current: number; limit: number | null };   // add
      concurrentSessions: { current: number; limit: number };
    } | null;

    And in hasKeyQuotaSet:

    quota.costTotal?.limit ||

    And in getMaxUsageRate:

    if (quota.costTotal?.limit) {
      rates.push(getUsageRate(quota.costTotal.current, quota.costTotal.limit));
    }
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/lib/utils/quota-helpers.ts
    Line: 8-14
    
    Comment:
    **`costTotal` missing from shared `KeyQuota` type and helper functions**
    
    The PR adds `costTotal` to every component-level `KeyQuota` interface, but the canonical `KeyQuota` type in `quota-helpers.ts` is not updated. As a result, `hasKeyQuotaSet`, `getMaxUsageRate`, `isWarning`, and `isExceeded` all silently ignore `costTotal`. A key whose only limit is `limitTotalUsd` will be treated as having no quota at all — filter tabs ("key-quota", "warning", "exceeded") in `keys-quota-manager.tsx` will misclassify it, and the per-key status badge will show "正常" even after the total budget is exhausted.
    
    Add `costTotal` to the type and include it in the helper functions:
    
    ```ts
    // quota-helpers.ts
    export type KeyQuota = {
      cost5h: { current: number; limit: number | null };
      costDaily: { current: number; limit: number | null };
      costWeekly: { current: number; limit: number | null };
      costMonthly: { current: number; limit: number | null };
      costTotal: { current: number; limit: number | null };   // add
      concurrentSessions: { current: number; limit: number };
    } | null;
    ```
    
    And in `hasKeyQuotaSet`:
    ```ts
    quota.costTotal?.limit ||
    ```
    
    And in `getMaxUsageRate`:
    ```ts
    if (quota.costTotal?.limit) {
      rates.push(getUsageRate(quota.costTotal.current, quota.costTotal.limit));
    }
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: tests/unit/actions/providers-usage.test.ts
Line: 168-169

Comment:
**No assertions on `limitTotalUsd` output shape**

The mock is wired up and returns `0`, but no test case checks that `getProviderLimitUsage` or `getProviderLimitUsageBatch` actually include `limitTotalUsd: { current: 0, limit: <value> }` in the returned data. A future refactor that accidentally drops the field would go undetected.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/app/[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsx
Line: 221-223

Comment:
**Redundant double-check on `limit`**

`limit && limit > 0` is redundant: if `limit` is truthy (non-null, non-zero) it is already `> 0` for positive values. More importantly, this differs from the `!== null && > 0` pattern used in `hasQuotaLimit` in `providers-quota-client.tsx`. Prefer the explicit pattern for consistency.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (2): Last reviewed commit: "fix(quota): repair total quota compile i..." | Re-trigger Greptile

Copilot AI review requested due to automatic review settings April 27, 2026 14:18
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

📝 Walkthrough

Walkthrough

为配额功能新增“总成本/总配额”维度:添加多语言本地化条目、后端汇总总成本的数据字段,并在提供商与密钥的配额 UI 与类型声明中引入可读/可编辑的总 USD 限额(limitTotalUsd / costTotal)。

Changes

Cohort / File(s) Summary
本地化字符串
messages/en/quota.json, messages/ja/quota.json, messages/ru/quota.json, messages/zh-CN/quota.json, messages/zh-TW/quota.json
新增多语言键以支持“总成本 / 总限额”显示:提供商标签、密钥表列和编辑对话框字段(包括占位与 current/limit 格式)。
提供商配额后端
src/actions/providers.ts
在单个与批量提供商限额使用查询中并行调用 sumProviderTotalCost,返回并穿透 limitTotalUsd.currentlimitTotalUsd.limit,并扩展 ProviderLimitUsageData 类型。
提供商配额 UI
src/app/[locale]/dashboard/quotas/providers/page.tsx, .../provider-quota-list-item.tsx, .../providers-quota-client.tsx, .../providers-quota-manager.tsx
为 ProviderQuota 添加 limitTotalUsd 字段,条件渲染“总成本”项,更新 hasQuotaLimit 与使用率(calculateMaxUsage)计算以纳入总 USD 配额。
密钥配额 UI
src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx, .../keys-quota-client.tsx, .../keys-quota-manager.tsx
为 KeyQuota 添加 costTotal 字段;在编辑对话框和表格中新增/保存 limitTotalUsd(表单输入、清除逻辑、快速编辑保存与进度可视化)。
公用工具
src/lib/utils/quota-helpers.ts
扩展 KeyQuota 类型以支持可选 costTotal,并在 hasKeyQuotaSetgetMaxUsageRate 中纳入 costTotal 使用率计算。
测试
tests/unit/actions/providers-usage.test.ts
新增对 sumProviderTotalCost 的 mock,默认返回 0,以覆盖后端新增的总成本聚合调用。

Estimated code review effort

🎯 3 (中等) | ⏱️ ~25 分钟

Possibly related PRs

Suggested reviewers

  • ding113

漫游指南

此PR为总成本/总配额功能添加了多语言本地化字符串、扩展了提供商配额数据类型以包含limitTotalUsd字段、并更新了配额UI组件以展示和编辑总成本限制。

更改内容

分组 / 文件 摘要
本地化字符串
messages/en/quota.json, messages/ja/quota.json, messages/ru/quota.json, messages/zh-CN/quota.json, messages/zh-TW/quota.json
添加了5种语言的新本地化键,支持总成本/总配额UI展示,包括提供商级别标签、密钥表格列和编辑对话框字段。
提供商配额后端
src/actions/providers.ts
扩展ProviderLimitUsageData类型以包含limitTotalUsd字段,在单个和批量限制使用端点中并行调用sumProviderTotalCost()以获取总成本数据。
提供商配额UI
src/app/[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsx, src/app/[locale]/dashboard/quotas/providers/_components/providers-quota-client.tsx, src/app/[locale]/dashboard/quotas/providers/page.tsx, src/app/[locale]/dashboard/quotas/providers/_components/providers-quota-manager.tsx
更新ProviderQuota接口以添加limitTotalUsd字段,在配额列表中有条件地渲染新的总配额项,并在配额检测和使用率计算逻辑中包含总USD配额。
密钥配额UI
src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx, src/app/[locale]/dashboard/quotas/keys/_components/keys-quota-client.tsx, src/app/[locale]/dashboard/quotas/keys/_components/keys-quota-manager.tsx
扩展KeyQuota接口以支持costTotal字段,添加编辑对话框和表格UI来展示/编辑总成本限制,包括表单输入和进度可视化。
公用工具
src/lib/utils/quota-helpers.ts
加入对 costTotal 的可选支持,并将其纳入 hasKeyQuotaSetgetMaxUsageRate 的判断与计算中。
测试
tests/unit/actions/providers-usage.test.ts
为新增的总成本聚合调用添加 mock(sumProviderTotalCost),确保测试环境确定性。

代码审查工作量评估

🎯 3 (中等) | ⏱️ ~25 分钟

可能相关的PR

建议审查者

  • ding113
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题"total quota fix"与变更集的主要目的相关,但表述不够具体。该PR的主要内容是为供应商和密钥配额添加永久性总成本限制(limitTotalUsd),但标题未能清晰传达这一核心功能。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed PR描述清晰地说明了功能目的(添加总成本限额)、实现方式、相关组件变更及测试步骤,与代码变更内容完全相关。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment on lines +351 to +364
min="0"
placeholder={t("limitTotalUsd.placeholder")}
value={limitTotal}
onChange={(e) => setLimitTotal(e.target.value)}
className="h-9"
/>
{currentQuota?.costTotal.limit && (
<p className="text-xs text-muted-foreground">
{t("limitTotalUsd.current", {
currency: currencySymbol,
current: Number(currentQuota.costTotal.current).toFixed(4),
limit: Number(currentQuota.costTotal.limit).toFixed(2),
})}
</p>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Current-usage tooltip hidden when limit is exactly 0

The condition currentQuota?.costTotal.limit && … is falsy when limit is 0. Because an admin can legitimately set a limitTotalUsd of 0 to hard-block a key, the tooltip won't appear in that case even though there is an active limit.

Suggested change
min="0"
placeholder={t("limitTotalUsd.placeholder")}
value={limitTotal}
onChange={(e) => setLimitTotal(e.target.value)}
className="h-9"
/>
{currentQuota?.costTotal.limit && (
<p className="text-xs text-muted-foreground">
{t("limitTotalUsd.current", {
currency: currencySymbol,
current: Number(currentQuota.costTotal.current).toFixed(4),
limit: Number(currentQuota.costTotal.limit).toFixed(2),
})}
</p>
{currentQuota?.costTotal.limit !== null && currentQuota?.costTotal.limit !== undefined && (
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx
Line: 351-364

Comment:
**Current-usage tooltip hidden when limit is exactly `0`**

The condition `currentQuota?.costTotal.limit && …` is falsy when `limit` is `0`. Because an admin can legitimately set a `limitTotalUsd` of `0` to hard-block a key, the tooltip won't appear in that case even though there is an active limit.

```suggestion
                {currentQuota?.costTotal.limit !== null && currentQuota?.costTotal.limit !== undefined && (
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e6a0151b4c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/actions/providers.ts Outdated
sumProviderCostInTimeRange(providerId, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(providerId, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(providerId, rangeMonthly.startTime, rangeMonthly.endTime),
sumProviderTotalCost(providerId),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Respect provider resetAt when summing total quota usage

getProviderLimitUsage now computes total usage with sumProviderTotalCost(providerId) but does not pass provider.totalCostResetAt. After an admin uses resetProviderTotalUsage, enforcement paths use that reset marker (RateLimitService.checkTotalCostLimit(..., { resetAt })), while this UI/API path keeps counting historical ledger rows, so the displayed total quota usage is wrong and can show providers as still over limit immediately after a reset.

Useful? React with 👍 / 👎.


{/* 总限额 */}
<TableCell>
{hasKeyQuota && key.quota && key.quota.costTotal.limit !== null ? (
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Render total key quota without requiring other quota types

The new total-quota column is gated by hasKeyQuota, but hasKeyQuotaSet does not include costTotal, so keys that only configure limitTotalUsd are treated as “inherited” and this cell renders - instead of the configured total limit and quick-edit control. This makes the new total quota effectively invisible for total-only configurations.

Useful? React with 👍 / 👎.

@github-actions github-actions Bot added the size/S Small PR (< 200 lines) label Apr 27, 2026
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] [ERROR-NO-USER-FEEDBACK] Current-usage tooltip hidden when total limit is exactly 0

Why this is a problem: The condition currentQuota?.costTotal.limit && ... is falsy when limit is 0. Because an admin can legitimately set a limitTotalUsd of 0 to hard-block a key, the tooltip won't appear in that case even though there is an active limit.

Suggested fix:

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR adds total cost limit (limitTotalUsd) support to provider and key quotas. While the implementation is mostly solid, I've identified one critical issue and one high priority bug that must be addressed before merging.

PR Size: S

  • Lines changed: 173
  • Files changed: 12

Issues Found

Category Critical High Medium Low
Logic/Bugs 1 1 0 0
Security 0 0 0 0
Error Handling 0 1 0 0
Types 1 0 0 0
Comments/Docs 0 0 0 0
Tests 0 0 0 0
Simplification 0 0 0 0

Critical Issues (Must Fix)

1. [TYPE-WEAK-INVARIANT] costTotal missing from shared KeyQuota type in src/lib/utils/quota-helpers.ts

Why this is a problem: The PR adds costTotal to every component-level KeyQuota interface, but the canonical KeyQuota type in quota-helpers.ts is not updated. As a result:

  • hasKeyQuotaSet() returns false for keys with only total quota set
  • getMaxUsageRate() ignores total cost - warning/exceeded states are never triggered
  • isWarning() and isExceeded() don't consider total quota
  • Filter tabs ("key-quota", "warning", "exceeded") in keys-quota-manager.tsx misclassify keys
  • A key whose only limit is limitTotalUsd will be treated as having no quota at all, and the per-key status badge will show "正常" even after the total budget is exhausted

Suggested fix:

// Update KeyQuota type
export type KeyQuota = {
  cost5h: { current: number; limit: number | null };
  costDaily: { current: number; limit: number | null };
  costWeekly: { current: number; limit: number | null };
  costMonthly: { current: number; limit: number | null };
  costTotal: { current: number; limit: number | null };  // ADD THIS
  concurrentSessions: { current: number; limit: number };
} | null;

// Update hasKeyQuotaSet()
return \!\!(
  quota.cost5h.limit ||
  quota.costDaily.limit ||
  quota.costWeekly.limit ||
  quota.costMonthly.limit ||
  quota.costTotal?.limit ||  // ADD THIS
  (quota.concurrentSessions.limit && quota.concurrentSessions.limit > 0)
);

// Update getMaxUsageRate()
if (quota.costTotal?.limit) {  // ADD THIS
  rates.push(getUsageRate(quota.costTotal.current, quota.costTotal.limit));
}

High Priority Issues (Should Fix)

2. [ERROR-NO-USER-FEEDBACK] Current-usage tooltip hidden when limit is exactly 0 in edit-key-quota-dialog.tsx:356-366

Why this is a problem: The condition currentQuota?.costTotal.limit && ... is falsy when limit is 0. Because an admin can legitimately set a limitTotalUsd of 0 to hard-block a key, the tooltip won't appear in that case even though there is an active limit.

Suggested fix:

// Change from:
{currentQuota?.costTotal.limit && (
  <p className="text-xs text-muted-foreground">...</p>
)}

// To:
{currentQuota?.costTotal.limit \!== null && currentQuota?.costTotal.limit \!== undefined && (
  <p className="text-xs text-muted-foreground">...</p>
)}

3. [LOGIC-BUG] handleClearQuota does not clear limitTotalUsd in edit-key-quota-dialog.tsx:118-145

Why this is a problem: The "Clear All Quotas" button resets all quotas except limitTotalUsd. This leaves the total quota active when the user intended to clear all limits.

Suggested fix: Add limitTotalUsd: null, to the handleClearQuota call at line 127-130.

Medium Priority (Consider Fixing)

4. [STANDARD-VIOLATION] Missing test coverage for new quota functionality

Per CLAUDE.md: "All new features must have unit test coverage of at least 80%". The new total quota functionality in sumProviderTotalCost() and the quota helpers should have corresponding unit tests.

Review Coverage

  • Logic and correctness - 2 issues found
  • Security (OWASP Top 10) - Clean
  • Error handling - 1 issue found
  • Type safety - 1 critical issue found
  • Documentation accuracy - Clean
  • Test coverage - Gap identified
  • Code clarity - Clean

Automated review by Claude AI

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a 'Total Quota' (USD) feature across the provider and key quota management systems, including translation updates, backend logic to calculate total costs, and UI components for viewing and editing these limits. Feedback focuses on ensuring that manual resets of total usage are respected by passing the reset timestamp to the cost calculation functions in the provider actions. Additionally, a naming inconsistency between 'costTotal' and 'limitTotalUsd' was identified across different components, which should be standardized for better maintainability.

Comment thread src/actions/providers.ts Outdated
sumProviderCostInTimeRange(providerId, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(providerId, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(providerId, rangeMonthly.startTime, rangeMonthly.endTime),
sumProviderTotalCost(providerId),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The call to sumProviderTotalCost is missing the resetAt parameter. Without passing provider.totalCostResetAt, the total cost calculation will ignore manual resets performed via resetProviderTotalUsage, leading to incorrect usage display in the UI.

Suggested change
sumProviderTotalCost(providerId),
sumProviderTotalCost(providerId, provider.totalCostResetAt),

Comment thread src/actions/providers.ts
Comment on lines +2833 to 2834
limitTotalUsd?: number | null;
limitConcurrentSessions?: number | null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The providers parameter in getProviderLimitUsageBatch should include totalCostResetAt to allow correct calculation of the total cost after a reset. Note that getProviders (not in this diff) also needs to be updated to include this field in its return mapping.

    limitTotalUsd?: number | null;
    totalCostResetAt?: Date | null;
    limitConcurrentSessions?: number | null;

Comment thread src/actions/providers.ts
sumProviderCostInTimeRange(provider.id, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(provider.id, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(provider.id, rangeMonthly.startTime, rangeMonthly.endTime),
sumProviderTotalCost(provider.id),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Similar to getProviderLimitUsage, this call to sumProviderTotalCost must pass provider.totalCostResetAt to ensure the batch calculation respects manual resets.

Suggested change
sumProviderTotalCost(provider.id),
sumProviderTotalCost(provider.id, provider.totalCostResetAt),

Comment on lines +28 to 29
limitTotalUsd: p.limitTotalUsd,
limitConcurrentSessions: p.limitConcurrentSessions,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The mapping logic for getProviderLimitUsageBatch is missing the totalCostResetAt field. This is necessary for the server action to correctly calculate the total cost relative to the last reset time. Ensure that getProviders() is also updated to return this field and that it is converted to a Date object if necessary.

      limitTotalUsd: p.limitTotalUsd,
      totalCostResetAt: p.totalCostResetAt ? new Date(p.totalCostResetAt) : null,
      limitConcurrentSessions: p.limitConcurrentSessions,

costDaily: { current: number; limit: number | null; resetAt?: Date };
costWeekly: { current: number; limit: number | null; resetAt?: Date };
costMonthly: { current: number; limit: number | null; resetAt?: Date };
costTotal: { current: number; limit: number | null };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is a naming inconsistency for the total quota field. Here it is named costTotal, while in the provider-related components and actions (e.g., ProviderLimitUsageData in src/actions/providers.ts), it is named limitTotalUsd. Standardizing the naming across the project would improve maintainability.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx (1)

395-398: ⚠️ Potential issue | 🟡 Minor

清空按钮条件遗漏总限额,导致“仅总限额”场景无法一键清空。

Line [395]-[398] 的条件未包含 currentQuota?.costTotal.limit。如果用户只配置了总限额,clearAll 不会显示。

建议修改
             {(currentQuota?.cost5h.limit ||
+              currentQuota?.costTotal.limit ||
               currentQuota?.costWeekly.limit ||
               currentQuota?.costMonthly.limit ||
               (currentQuota?.concurrentSessions.limit ?? 0) > 0) && (
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/`[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx
around lines 395 - 398, The clear-all button's render condition omits
currentQuota?.costTotal.limit, so when only a total cost limit is set the clear
(clearAll) button won't appear; update the JSX conditional in
edit-key-quota-dialog.tsx where the expression uses currentQuota (the line with
currentQuota?.cost5h.limit || currentQuota?.costWeekly.limit ||
currentQuota?.costMonthly.limit || (currentQuota?.concurrentSessions.limit ?? 0)
> 0) to include currentQuota?.costTotal.limit in the OR chain (and ensure
parentheses around the concurrentSessions check remain correct) so the clearAll
button is shown when only costTotal is configured.
🧹 Nitpick comments (1)
src/app/[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsx (1)

19-19: 字段命名与同级不一致

新增的 limitTotalUsd 与同接口中 cost5h / costDaily / costWeekly / costMonthlycost* 命名风格不一致,且其语义与它们一样是 { current; limit }(含已使用与上限),并非"纯限额"。i18n 也使用 costTotal.label。建议统一为 costTotal 以避免混淆,否则未来维护时容易和数据库字段 limitTotalUsd(裸数值)混为一谈。

♻️ 建议改名(同步 page.tsx 中 quotaMap 与 actions/providers.ts 的返回字段)
 interface ProviderQuota {
   cost5h: { current: number; limit: number | null; resetInfo: string };
   costDaily: { current: number; limit: number | null; resetAt?: Date };
   costWeekly: { current: number; limit: number | null; resetAt: Date };
   costMonthly: { current: number; limit: number | null; resetAt: Date };
   concurrentSessions: { current: number; limit: number };
-  limitTotalUsd: { current: number; limit: number | null };
+  costTotal: { current: number; limit: number | null };
 }

并相应调整 222–227 行的引用。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsx
at line 19, Rename the new prop/field limitTotalUsd to costTotal to match the
naming convention of cost5h/costDaily/costWeekly/costMonthly and i18n key
costTotal.label; update all usages and shapes that expect limitTotalUsd
(including the component provider-quota-list-item, the quotaMap in page.tsx, and
the returned fields in actions/providers.ts) and adjust the references that
currently access limitTotalUsd (the block referenced around the previous 222–227
lines) to use costTotal.current and costTotal.limit instead of the old name so
the shape remains { current; limit } across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/actions/providers.ts`:
- Around line 2738-2747: The total cost calls are missing the reset boundary so
resetProviderTotalUsage semantics break—pass the provider's reset boundary into
sumProviderTotalCost (the resetAt param) wherever total cost is computed (e.g.,
the call to sumProviderTotalCost in the Promise.all and the other occurrences
around the later ranges); locate usages of sumProviderTotalCost and change them
to supply the same total_cost_reset_at (or the computed reset boundary used for
aggregation) so the aggregation lower bound is respected, and ensure callers
like resetProviderTotalUsage and RateLimitService.getCurrentCost continue to
align with that resetAt value.

In `@src/app/`[locale]/dashboard/quotas/keys/_components/keys-quota-client.tsx:
- Line 395: hasKeyQuotaSet currently omits checking costTotal.limit which causes
keys that only have costTotal.limit set to be treated as having no quota; update
the hasKeyQuotaSet function in quota-helpers.ts to include a truthy/non-null
check for quota.costTotal?.limit (or equivalent) alongside the existing checks
for cost5h, costDaily, costWeekly, costMonthly and concurrentSessions so that
the component relying on hasKeyQuota (line with hasKeyQuota && key.quota &&
key.quota.costTotal.limit !== null) correctly renders when only total cost limit
is set.

---

Outside diff comments:
In
`@src/app/`[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx:
- Around line 395-398: The clear-all button's render condition omits
currentQuota?.costTotal.limit, so when only a total cost limit is set the clear
(clearAll) button won't appear; update the JSX conditional in
edit-key-quota-dialog.tsx where the expression uses currentQuota (the line with
currentQuota?.cost5h.limit || currentQuota?.costWeekly.limit ||
currentQuota?.costMonthly.limit || (currentQuota?.concurrentSessions.limit ?? 0)
> 0) to include currentQuota?.costTotal.limit in the OR chain (and ensure
parentheses around the concurrentSessions check remain correct) so the clearAll
button is shown when only costTotal is configured.

---

Nitpick comments:
In
`@src/app/`[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsx:
- Line 19: Rename the new prop/field limitTotalUsd to costTotal to match the
naming convention of cost5h/costDaily/costWeekly/costMonthly and i18n key
costTotal.label; update all usages and shapes that expect limitTotalUsd
(including the component provider-quota-list-item, the quotaMap in page.tsx, and
the returned fields in actions/providers.ts) and adjust the references that
currently access limitTotalUsd (the block referenced around the previous 222–227
lines) to use costTotal.current and costTotal.limit instead of the old name so
the shape remains { current; limit } across the codebase.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e4481f80-7c8e-48c5-8183-a03d7707b1d8

📥 Commits

Reviewing files that changed from the base of the PR and between 4e7ff96 and e6a0151.

📒 Files selected for processing (12)
  • messages/en/quota.json
  • messages/ja/quota.json
  • messages/ru/quota.json
  • messages/zh-CN/quota.json
  • messages/zh-TW/quota.json
  • src/actions/providers.ts
  • src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx
  • src/app/[locale]/dashboard/quotas/keys/_components/keys-quota-client.tsx
  • src/app/[locale]/dashboard/quotas/keys/_components/keys-quota-manager.tsx
  • src/app/[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsx
  • src/app/[locale]/dashboard/quotas/providers/_components/providers-quota-client.tsx
  • src/app/[locale]/dashboard/quotas/providers/page.tsx

Comment thread src/actions/providers.ts Outdated
Comment on lines 2738 to 2747
const [cost5h, costDaily, costWeekly, costMonthly, totalCost, concurrentSessions] =
await Promise.all([
limit5hResetMode === "fixed"
? RateLimitService.getCurrentCost(providerId, "provider", "5h", undefined, limit5hResetMode)
: sumProviderCostInTimeRange(providerId, range5h.startTime, range5h.endTime),
sumProviderCostInTimeRange(providerId, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(providerId, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(providerId, rangeMonthly.startTime, rangeMonthly.endTime),
sumProviderTotalCost(providerId),
SessionTracker.getProviderSessionCount(providerId),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

总额使用量未传入 reset 边界,resetProviderTotalUsage 语义会失效。

sumProviderTotalCost 支持 resetAt,但这里两处调用都未传入。结合 Line [1250] 的“通过 total_cost_reset_at 作为聚合下限”设计,当前会在重置后继续累计历史总额。

建议修改
-      sumProviderTotalCost(providerId),
+      sumProviderTotalCost(providerId, provider.totalCostResetAt ?? null),
   providers: Array<{
     id: number;
+    totalCostResetAt?: Date | null;
     dailyResetTime?: string | null;
     dailyResetMode?: string | null;
     limit5hResetMode?: string | null;
     limit5hUsd?: number | null;
-        sumProviderTotalCost(provider.id),
+        sumProviderTotalCost(provider.id, provider.totalCostResetAt ?? null),

Also applies to: 2824-2834, 2877-2905

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/actions/providers.ts` around lines 2738 - 2747, The total cost calls are
missing the reset boundary so resetProviderTotalUsage semantics break—pass the
provider's reset boundary into sumProviderTotalCost (the resetAt param) wherever
total cost is computed (e.g., the call to sumProviderTotalCost in the
Promise.all and the other occurrences around the later ranges); locate usages of
sumProviderTotalCost and change them to supply the same total_cost_reset_at (or
the computed reset boundary used for aggregation) so the aggregation lower bound
is respected, and ensure callers like resetProviderTotalUsage and
RateLimitService.getCurrentCost continue to align with that resetAt value.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses missing/incorrect “total quota” (lifetime spending limit) visibility by plumbing limitTotalUsd through provider/key quota data models, UI rendering, and translations.

Changes:

  • Add provider-side limitTotalUsd to quota usage payloads and render “Total Cost/Total Quota” in the provider quotas UI.
  • Add key-side total quota (costTotal) to the keys quota table and key quota edit dialog.
  • Add i18n strings for “total” quota/cost labels across multiple locales.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/app/[locale]/dashboard/quotas/providers/page.tsx Passes limitTotalUsd into the provider batch quota loader.
src/app/[locale]/dashboard/quotas/providers/_components/providers-quota-client.tsx Includes total quota in “has quota” detection and max-usage calculation.
src/app/[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsx Renders provider “Total Cost” quota indicator.
src/app/[locale]/dashboard/quotas/keys/_components/keys-quota-manager.tsx Extends key quota shape with costTotal.
src/app/[locale]/dashboard/quotas/keys/_components/keys-quota-client.tsx Adds a “Total Quota” column and rendering for key quotas.
src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx Adds total quota input and saves it as limitTotalUsd.
src/actions/providers.ts Adds limitTotalUsd usage aggregation via sumProviderTotalCost for single/batch provider quota usage.
messages/en/quota.json Adds provider/key “total” labels and key edit-dialog strings.
messages/zh-CN/quota.json Adds provider/key “total” labels and key edit-dialog strings.
messages/zh-TW/quota.json Adds provider/key “total” labels and key edit-dialog strings.
messages/ru/quota.json Adds provider/key “total” labels and key edit-dialog strings.
messages/ja/quota.json Adds provider/key “total” labels and key edit-dialog strings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/actions/providers.ts
Comment on lines 2901 to +2904
sumProviderCostInTimeRange(provider.id, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(provider.id, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(provider.id, rangeMonthly.startTime, rangeMonthly.endTime),
sumProviderTotalCost(provider.id),
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sumProviderTotalCost(...) runs for every provider even when limitTotalUsd is null/0 (i.e., total quota disabled). This adds an extra DB aggregate per provider for no user-visible benefit. Consider only querying total cost when limitTotalUsd is a positive number (otherwise skip and set current to 0 or omit the field).

Copilot uses AI. Check for mistakes.
Comment thread src/actions/providers.ts Outdated
Comment on lines 2738 to 2748
const [cost5h, costDaily, costWeekly, costMonthly, totalCost, concurrentSessions] =
await Promise.all([
limit5hResetMode === "fixed"
? RateLimitService.getCurrentCost(providerId, "provider", "5h", undefined, limit5hResetMode)
: sumProviderCostInTimeRange(providerId, range5h.startTime, range5h.endTime),
sumProviderCostInTimeRange(providerId, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(providerId, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(providerId, rangeMonthly.startTime, rangeMonthly.endTime),
sumProviderTotalCost(providerId),
SessionTracker.getProviderSessionCount(providerId),
]);
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sumProviderTotalCost(providerId) is executed unconditionally, even if provider.limitTotalUsd is null/0 (total quota disabled). Skipping this query when the limit isn’t enabled avoids an unnecessary full-history SUM and reduces load on the quota page.

Suggested change
const [cost5h, costDaily, costWeekly, costMonthly, totalCost, concurrentSessions] =
await Promise.all([
limit5hResetMode === "fixed"
? RateLimitService.getCurrentCost(providerId, "provider", "5h", undefined, limit5hResetMode)
: sumProviderCostInTimeRange(providerId, range5h.startTime, range5h.endTime),
sumProviderCostInTimeRange(providerId, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(providerId, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(providerId, rangeMonthly.startTime, rangeMonthly.endTime),
sumProviderTotalCost(providerId),
SessionTracker.getProviderSessionCount(providerId),
]);
const totalCostPromise =
provider.limitTotalUsd != null && provider.limitTotalUsd > 0
? sumProviderTotalCost(providerId)
: Promise.resolve(0);
const [cost5h, costDaily, costWeekly, costMonthly, totalCost, concurrentSessions] =
await Promise.all([
limit5hResetMode === "fixed"
? RateLimitService.getCurrentCost(providerId, "provider", "5h", undefined, limit5hResetMode)
: sumProviderCostInTimeRange(providerId, range5h.startTime, range5h.endTime),
sumProviderCostInTimeRange(providerId, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(providerId, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(providerId, rangeMonthly.startTime, rangeMonthly.endTime),
totalCostPromise,
SessionTracker.getProviderSessionCount(providerId),
]);

Copilot uses AI. Check for mistakes.
Comment thread src/actions/providers.ts Outdated
sumProviderCostInTimeRange(providerId, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(providerId, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(providerId, rangeMonthly.startTime, rangeMonthly.endTime),
sumProviderTotalCost(providerId),
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sumProviderTotalCost(providerId) is called without the provider’s totalCostResetAt. That means the “total quota” usage shown here will be all-time, and can disagree with the actual enforcement logic (which resets cumulative total after a manual reset). Pass provider.totalCostResetAt into the total-cost aggregation (or use the same helper used for enforcement) so UI and rate limiting semantics match.

Suggested change
sumProviderTotalCost(providerId),
sumProviderTotalCost(providerId, provider.totalCostResetAt),

Copilot uses AI. Check for mistakes.
Comment thread src/actions/providers.ts Outdated
Comment on lines +2887 to +2904
@@ -2887,6 +2901,7 @@ export async function getProviderLimitUsageBatch(
sumProviderCostInTimeRange(provider.id, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(provider.id, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(provider.id, rangeMonthly.startTime, rangeMonthly.endTime),
sumProviderTotalCost(provider.id),
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the batch path, sumProviderTotalCost(provider.id) also ignores the provider’s totalCostResetAt, so the displayed total usage can stay “exceeded” even after an admin resets total-cost usage. Extend the providers input to include totalCostResetAt (from getProviders()), and pass it into the total-cost sum so the batch view matches enforcement behavior.

Copilot uses AI. Check for mistakes.
limitDailyUsd: p.limitDailyUsd,
limitWeeklyUsd: p.limitWeeklyUsd,
limitMonthlyUsd: p.limitMonthlyUsd,
limitTotalUsd: p.limitTotalUsd,
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getProviderLimitUsageBatch will need the provider’s totalCostResetAt to compute “total quota” usage correctly after a reset. Right now only limitTotalUsd is passed, so the batch action can’t apply the reset boundary and will show an all-time total.

Suggested change
limitTotalUsd: p.limitTotalUsd,
limitTotalUsd: p.limitTotalUsd,
totalCostResetAt: p.totalCostResetAt,

Copilot uses AI. Check for mistakes.
Comment on lines 18 to 25
interface KeyQuota {
cost5h: { current: number; limit: number | null };
costDaily: { current: number; limit: number | null };
costWeekly: { current: number; limit: number | null };
costMonthly: { current: number; limit: number | null };
costTotal: { current: number; limit: number | null };
concurrentSessions: { current: number; limit: number };
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KeyQuota now includes costTotal, but the logic in this component relies on hasKeyQuotaSet(...) for filtering and “independent vs inherited” labeling. hasKeyQuotaSet (and its KeyQuota type) currently doesn’t consider total quota, so keys with only a total limit will be misclassified and may be filtered out. Update the helper/type to include costTotal as a quota dimension.

Copilot uses AI. Check for mistakes.
Comment on lines +393 to +399
{/* 总限额 */}
<TableCell>
{hasKeyQuota && key.quota && key.quota.costTotal.limit !== null ? (
<div className="space-y-1">
<div className="flex items-center justify-between mb-1">
<span className="text-xs text-muted-foreground">
{t("table.costTotal")}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new total-quota column is gated on hasKeyQuota, but hasKeyQuota comes from hasKeyQuotaSet(key.quota), which doesn’t include total quota. As a result, keys that only have a total spending limit won’t display this column and will be labeled as “inherited”. Either include costTotal in hasKeyQuotaSet (and its KeyQuota type), or adjust the gating here to consider costTotal directly.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/quotas/keys/_components/edit-key-quota-dialog.tsx (1)

396-409: ⚠️ Potential issue | 🟡 Minor

“清除全部”按钮可见条件遗漏了 costTotal(以及 costDaily

本次 PR 在 handleClearQuota 中新增了 limitTotalUsd: null 的清空逻辑(Line 135),但该按钮的可见条件未包含 currentQuota?.costTotal.limit。后果:当某 Key 配置了总额限额(且其他维度为空、并发为 0)时,hasKeyQuotaSet 视为已配置,但用户在编辑对话框中看不到“清除全部”按钮,无法一键清空。同时此条件原本也未涵盖 costDaily.limit,建议一并补齐:

🐛 建议改动
-            {(currentQuota?.cost5h.limit ||
-              currentQuota?.costWeekly.limit ||
-              currentQuota?.costMonthly.limit ||
-              (currentQuota?.concurrentSessions.limit ?? 0) > 0) && (
+            {(currentQuota?.cost5h.limit ||
+              currentQuota?.costDaily.limit ||
+              currentQuota?.costWeekly.limit ||
+              currentQuota?.costMonthly.limit ||
+              currentQuota?.costTotal.limit ||
+              (currentQuota?.concurrentSessions.limit ?? 0) > 0) && (

注:同样存在 0 作为有效限额时被判为假值的隐患,若希望严格匹配 hasKeyQuotaSet 的语义,可改为 != null 比较。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/`[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx
around lines 396 - 409, The visibility condition for the "清除全部" button in the
DialogFooter uses currentQuota checks but omits costTotal and costDaily and
treats 0 as falsy; update the conditional that currently references
currentQuota?.cost5h.limit, costWeekly, costMonthly and concurrentSessions to
also include currentQuota?.costTotal.limit and currentQuota?.costDaily.limit,
and use explicit null/undefined checks (e.g., != null) for each limit comparison
so zero is considered a valid set limit; ensure this aligns with the
handleClearQuota logic that clears limitTotalUsd and other limits.
♻️ Duplicate comments (1)
src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx (1)

359-367: ⚠️ Potential issue | 🟡 Minor

当前用量提示在 limitTotalUsd === 0 时被隐藏

currentQuota?.costTotal.limit && (...)limit0 时为假值,因此即便管理员显式将总额限额设为 0(用于硬封禁该 Key),也不会显示已用/限额提示,与其他维度上已有的同类问题一致。建议显式判断非 null/undefined

🐛 建议改动
-                {currentQuota?.costTotal.limit && (
+                {currentQuota?.costTotal.limit != null && (
                   <p className="text-xs text-muted-foreground">
                     {t("limitTotalUsd.current", {
                       currency: currencySymbol,
                       current: Number(currentQuota.costTotal.current).toFixed(4),
                       limit: Number(currentQuota.costTotal.limit).toFixed(2),
                     })}
                   </p>
                 )}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/`[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx
around lines 359 - 367, The JSX currently hides the "current/limit" line when
costTotal.limit is 0 because it uses a truthy check; change the condition in the
render for currentQuota?.costTotal.limit to an explicit null/undefined check
(e.g., currentQuota?.costTotal.limit != null) so that a limit of 0 still
renders; update the component where currentQuota and costTotal.limit are
referenced (edit-key-quota-dialog, render block using
currentQuota?.costTotal.limit) to use this explicit check and keep the same
formatting logic for current and limit values.
🧹 Nitpick comments (2)
src/lib/utils/quota-helpers.ts (1)

8-15: costTotal 类型应与上游契约保持一致(必需而非可选)

上游数据源对 costTotal 均为必需字段:

  • src/repository/statistics.tssumKeyQuotaCostsById() 返回值始终包含 costTotal: number
  • src/actions/keys.tsgetKeyLimitUsage() 的返回类型显式声明 costTotal: { current: number; limit: number | null }(必需)
  • 同 PR 中 keys-quota-client.tsxedit-key-quota-dialog.tsx 的本地 KeyQuota 接口也都把 costTotal 定义为必需

仅在此处声明为 costTotal?: ... 会让类型系统弱化对上游契约的约束,且其他文件的 KeyQuota 与本文件不再结构兼容(在严格类型检查下可能导致赋值不匹配)。建议改为必需,与其他所有定义对齐:

♻️ 建议改动
 export type KeyQuota = {
   cost5h: { current: number; limit: number | null };
   costDaily: { current: number; limit: number | null };
   costWeekly: { current: number; limit: number | null };
   costMonthly: { current: number; limit: number | null };
-  costTotal?: { current: number; limit: number | null };
+  costTotal: { current: number; limit: number | null };
   concurrentSessions: { current: number; limit: number };
 } | null;

如改为必需,下方 quota.costTotal?.limit 的可选链可同步去掉。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/quota-helpers.ts` around lines 8 - 15, The KeyQuota type
declares costTotal as optional which diverges from the upstream contract; update
the KeyQuota type in this file to make costTotal required (change costTotal?:
... to costTotal: { current: number; limit: number | null }) so it matches
sumKeyQuotaCostsById and getKeyLimitUsage and the local interfaces in
keys-quota-client.tsx and edit-key-quota-dialog.tsx; after making costTotal
required, remove any unnecessary optional chaining (e.g.,
quota.costTotal?.limit) in code that accesses costTotal.
src/app/[locale]/dashboard/quotas/providers/_components/providers-quota-client.tsx (1)

39-81: null 检查模式不一致(可选优化)。

hasQuotaLimit 中对 limitTotalUsd.limit 使用显式 !== null && > 0(Line [44]),而 calculateMaxUsage 中对同一字段使用隐式真值判断 limit && limit > 0(Line [76])。两者对 number | null 类型行为等价,但在同一文件内风格不一致;本次新增的 total 字段沿用了不同模式,建议统一以提升可读性。

建议修改:与 hasQuotaLimit 保持一致
-  if (provider.quota.limitTotalUsd.limit && provider.quota.limitTotalUsd.limit > 0) {
+  if (provider.quota.limitTotalUsd.limit !== null && provider.quota.limitTotalUsd.limit > 0) {
     usages.push((provider.quota.limitTotalUsd.current / provider.quota.limitTotalUsd.limit) * 100);
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/dashboard/quotas/providers/_components/providers-quota-client.tsx
around lines 39 - 81, The null-check style is inconsistent: hasQuotaLimit uses
explicit `!== null && > 0` while calculateMaxUsage uses an implicit truthy
check; update calculateMaxUsage to use the same explicit null checks as
hasQuotaLimit for all quota fields (e.g. `provider.quota.limitTotalUsd.limit !==
null && provider.quota.limitTotalUsd.limit > 0`, and similarly for cost5h,
costDaily, costWeekly, costMonthly, concurrentSessions) so the logic and style
match between hasQuotaLimit and calculateMaxUsage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@src/app/`[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx:
- Around line 396-409: The visibility condition for the "清除全部" button in the
DialogFooter uses currentQuota checks but omits costTotal and costDaily and
treats 0 as falsy; update the conditional that currently references
currentQuota?.cost5h.limit, costWeekly, costMonthly and concurrentSessions to
also include currentQuota?.costTotal.limit and currentQuota?.costDaily.limit,
and use explicit null/undefined checks (e.g., != null) for each limit comparison
so zero is considered a valid set limit; ensure this aligns with the
handleClearQuota logic that clears limitTotalUsd and other limits.

---

Duplicate comments:
In
`@src/app/`[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx:
- Around line 359-367: The JSX currently hides the "current/limit" line when
costTotal.limit is 0 because it uses a truthy check; change the condition in the
render for currentQuota?.costTotal.limit to an explicit null/undefined check
(e.g., currentQuota?.costTotal.limit != null) so that a limit of 0 still
renders; update the component where currentQuota and costTotal.limit are
referenced (edit-key-quota-dialog, render block using
currentQuota?.costTotal.limit) to use this explicit check and keep the same
formatting logic for current and limit values.

---

Nitpick comments:
In
`@src/app/`[locale]/dashboard/quotas/providers/_components/providers-quota-client.tsx:
- Around line 39-81: The null-check style is inconsistent: hasQuotaLimit uses
explicit `!== null && > 0` while calculateMaxUsage uses an implicit truthy
check; update calculateMaxUsage to use the same explicit null checks as
hasQuotaLimit for all quota fields (e.g. `provider.quota.limitTotalUsd.limit !==
null && provider.quota.limitTotalUsd.limit > 0`, and similarly for cost5h,
costDaily, costWeekly, costMonthly, concurrentSessions) so the logic and style
match between hasQuotaLimit and calculateMaxUsage.

In `@src/lib/utils/quota-helpers.ts`:
- Around line 8-15: The KeyQuota type declares costTotal as optional which
diverges from the upstream contract; update the KeyQuota type in this file to
make costTotal required (change costTotal?: ... to costTotal: { current: number;
limit: number | null }) so it matches sumKeyQuotaCostsById and getKeyLimitUsage
and the local interfaces in keys-quota-client.tsx and edit-key-quota-dialog.tsx;
after making costTotal required, remove any unnecessary optional chaining (e.g.,
quota.costTotal?.limit) in code that accesses costTotal.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2bc4d910-1b68-4996-81ca-c9f18571cf68

📥 Commits

Reviewing files that changed from the base of the PR and between e6a0151 and 0d64074.

📒 Files selected for processing (7)
  • src/actions/providers.ts
  • src/app/[locale]/dashboard/quotas/keys/_components/edit-key-quota-dialog.tsx
  • src/app/[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsx
  • src/app/[locale]/dashboard/quotas/providers/_components/providers-quota-client.tsx
  • src/app/[locale]/dashboard/quotas/providers/_components/providers-quota-manager.tsx
  • src/lib/utils/quota-helpers.ts
  • tests/unit/actions/providers-usage.test.ts
✅ Files skipped from review due to trivial changes (1)
  • src/app/[locale]/dashboard/quotas/providers/_components/providers-quota-manager.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/app/[locale]/dashboard/quotas/providers/_components/provider-quota-list-item.tsx

@ilnli ilnli closed this Apr 27, 2026
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Claude Code Hub Roadmap Apr 27, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d64074a6d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/actions/providers.ts
sumProviderCostInTimeRange(providerId, rangeDaily.startTime, rangeDaily.endTime),
sumProviderCostInTimeRange(providerId, rangeWeekly.startTime, rangeWeekly.endTime),
sumProviderCostInTimeRange(providerId, rangeMonthly.startTime, rangeMonthly.endTime),
sumProviderTotalCost(providerId),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Apply totalCostResetAt when computing provider total usage

sumProviderTotalCost is invoked without the provider’s reset marker, so this path always sums lifetime ledger rows even after resetProviderTotalUsage updates providers.total_cost_reset_at. That makes quota usage shown by this action diverge from enforcement (RateLimitService.checkTotalCostLimit(..., { resetAt })), and providers can appear over the total cap immediately after an admin reset. Pass provider.totalCostResetAt to the total-cost aggregation here (and in the batch variant) to keep displayed usage consistent with runtime limit checks.

Useful? React with 👍 / 👎.

@github-actions github-actions Bot mentioned this pull request Apr 27, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:i18n area:provider area:UI area:UX bug Something isn't working size/S Small PR (< 200 lines)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants