Conversation
…orrect billing
When Gemini requests use the Vertex AI URL format
/v1/publishers/google/models/{model}:generateContent, the system
failed to extract the model name, falling back to a hardcoded
"gemini-2.5-flash" default and causing incorrect billing.
Add publishers path regex to extractModelFromPath() and
detectFormatByEndpoint() to handle this URL pattern.
…ndard path buildHeaders() derives Host from provider.url, but the actual fetch target (proxyUrl) may use a different host when activeEndpoint.baseUrl differs or MCP passthrough overrides the base URL. This causes undici TLS certificate validation failures. After proxyUrl is computed, re-derive Host from it.
Skip rendering UsageLogsStatsPanel and its aggregation query when all filter conditions are empty, preventing full-table scans that cause CPU overload.
…ss conversations (#793) generateDeterministicSessionId() hashes (UA, IP, API key prefix) with no time dimension, producing identical session IDs for the same user hours apart. This merges unrelated conversations into one session, polluting usage logs, session tracking, and concurrent session limits. The existing fallback in getOrCreateSessionId() (content hash -> random ID) already provides correct session continuity without collision risk. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Apply the same hasStatsFilters guard from the old view to the virtualized logs view, preventing an unconditional full-table aggregation query on page load. Also remove the unused legacy usage-logs-view.tsx which is no longer imported anywhere.
📝 WalkthroughWalkthrough本PR删除完整的 UsageLogsView 组件，调整虚拟化日志视图中统计面板的渲染条件；扩展并导出模型路径提取以支持 Vertex AI 发布者路径；在代理层同步 Host 头并移除确定性会话ID回退；相应新增/删减单元测试与多语言文案项。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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 @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily enhances the backend proxy's capabilities by integrating support for Gemini Vertex AI endpoints and addressing a critical 'Host' header issue that affected TLS validation for various providers. Concurrently, the frontend for usage logs has been streamlined, improving performance and user experience by optimizing the display of statistical data. Highlights
Changelog
Activity
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
|
![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 appears to be a release, incorporating several valuable changes. It introduces support for Gemini on Vertex AI, complete with new endpoint detection, model extraction logic, and corresponding unit tests. A critical bug fix correctly sets the Host header for forwarded requests, which is essential for TLS validation with multi-endpoint providers and is also well-tested. The codebase has been refactored for better performance and maintainability, including the removal of an old component and centralization of session ID logic. I've suggested one minor improvement for maintainability in how filters are handled in the logs view. Overall, this is a solid set of changes.
| const statsFilters = { | ||
| userId: filters.userId, | ||
| keyId: filters.keyId, | ||
| providerId: filters.providerId, | ||
| sessionId: filters.sessionId, | ||
| startTime: filters.startTime, | ||
| endTime: filters.endTime, | ||
| statusCode: filters.statusCode, | ||
| excludeStatusCode200: filters.excludeStatusCode200, | ||
| model: filters.model, | ||
| endpoint: filters.endpoint, | ||
| minRetryCount: filters.minRetryCount, | ||
| }; |
There was a problem hiding this comment.
For better maintainability, you can create statsFilters by using object destructuring to exclude the page property from filters. This avoids having to manually list every property and prevents potential bugs if new filters are added in the future.
const { page, ...statsFilters } = filters;
🧪 测试结果
总体结果: ✅ 所有测试通过 |
* style(my-usage): use Badge for provider group values
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(my-usage): use currency symbol instead of code in quota cards
Replace manual `${currency} ${num.toFixed(2)}` formatting with
`formatCurrency()` so quota values display "$3.50" instead of "USD 3.50",
consistent with all other currency displays in the app.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* style(my-usage): replace unlimited text with infinity icon in quota cards
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(my-usage): paginate model breakdown in statistics summary card
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* chore(my-usage): suppress biome exhaustive-deps for intentional stats reset
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(my-usage): address PR #794 review comments
- Fix abbreviateModel/abbreviateClient crash on empty split parts
- Fix pagination reset on auto-refresh by using dateRange deps
- Restore noData fallback in model breakdown columns
- Add i18n for pagination controls with aria-labels (5 langs)
- Fix quota label overflow for long translations (w-8 -> w-auto)
- Rename Infinity -> InfinityIcon to avoid shadowing global
- Remove redundant span wrappers in TooltipTrigger asChild
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: John Doe <johndoe@example.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-actions
bot
added
bug
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/app/`[locale]/my-usage/_components/statistics-summary-card.tsx:
- Around line 119-138: The current logic shows t("noData") for an empty page
slice even if the overall array has items; update the rendering condition for
keyPageItems and userPageItems so you only display t("noData") when the entire
source array (stats?.keyModelBreakdown or stats?.userModelBreakdown) is empty,
and for an empty slice on a non-empty source either render nothing or a
different message (e.g., t("noDataOnThisPage")); use the existing symbols
breakdownPage, keyModelBreakdown, userModelBreakdown, keyPageItems,
userPageItems and MODEL_BREAKDOWN_PAGE_SIZE to implement this conditional check.
🧹 Nitpick comments (3)
src/app/[locale]/my-usage/_components/statistics-summary-card.tsx (1)
322-322: 建议将常量MODEL_BREAKDOWN_PAGE_SIZE移至文件顶部或组件定义之前。虽然在运行时不存在 TDZ 问题（函数执行时模块已完成初始化），但将常量放在首次使用位置之前更符合阅读习惯。
建议的调整
将
MODEL_BREAKDOWN_PAGE_SIZE移到文件顶部（如 import 语句之后）：+const MODEL_BREAKDOWN_PAGE_SIZE = 5; + interface StatisticsSummaryCardProps { className?: string; autoRefreshSeconds?: number; serverTimeZone?: string; }并删除 Line 322 处的原定义。
src/app/[locale]/my-usage/_components/provider-group-info.tsx (1)
9-51:abbreviateModel逻辑较复杂，建议增加单元测试覆盖。该函数有多条正则分支（日期过滤、纯字母、版本号混合、纯数字等），覆盖的模型命名格式较多。目前作为内部函数未导出，不便直接测试。建议：
- 将
abbreviateModel和abbreviateClient提取到独立的工具模块并导出- 补充常见模型名称格式的单元测试（如
claude-3-5-sonnet-20241022、gpt-4o、o1等）此外注意 Lines 24-27 中
versionMixed可被覆写——如果某个模型名同时包含X.Y格式和数字+字母格式的部分，后者会覆盖前者。实际场景中影响不大，但值得留意。src/app/[locale]/my-usage/_components/quota-cards.tsx (1)
167-171:Number(value)转换冗余
value的类型已经是number，Number(value)不会改变其值（NaN仍为NaN）。可以直接使用value简化逻辑。建议简化
const formatValue = (value: number) => { - const num = Number(value); - if (!Number.isFinite(num)) return currency ? formatCurrency(0, currency) : "0"; - return currency ? formatCurrency(num, currency) : String(num); + if (!Number.isFinite(value)) return currency ? formatCurrency(0, currency) : "0"; + return currency ? formatCurrency(value, currency) : String(value); };
| const [breakdownPage, setBreakdownPage] = useState(1); | ||
|
|
||
| // Reset breakdown page when date range changes | ||
| // biome-ignore lint/correctness/useExhaustiveDependencies: deps used as reset trigger on date range change | ||
| useEffect(() => { | ||
| setBreakdownPage(1); | ||
| }, [dateRange.startDate, dateRange.endDate]); | ||
|
|
||
| const isLoading = loading || refreshing; | ||
| const currencyCode = stats?.currencyCode ?? "USD"; | ||
|
|
||
| const maxBreakdownLen = Math.max( | ||
| stats?.keyModelBreakdown.length ?? 0, | ||
| stats?.userModelBreakdown.length ?? 0 | ||
| ); | ||
| const breakdownTotalPages = Math.ceil(maxBreakdownLen / MODEL_BREAKDOWN_PAGE_SIZE); | ||
| const sliceStart = (breakdownPage - 1) * MODEL_BREAKDOWN_PAGE_SIZE; | ||
| const sliceEnd = breakdownPage * MODEL_BREAKDOWN_PAGE_SIZE; | ||
| const keyPageItems = stats?.keyModelBreakdown.slice(sliceStart, sliceEnd) ?? []; | ||
| const userPageItems = stats?.userModelBreakdown.slice(sliceStart, sliceEnd) ?? []; |
There was a problem hiding this comment.
分页逻辑实现正确，但 "noData" 在末页可能产生误导。
当 keyModelBreakdown 和 userModelBreakdown 长度不一致时，较短一列在末页会因 pageItems 为空而显示 t("noData")（即"所选时段无数据"），但实际上该列在其他页仍有数据。这可能让用户以为该维度完全没有数据。
可以考虑在末页空列不显示文字，或使用一条更具上下文含义的提示。
🤖 Prompt for AI Agents
In `@src/app/`[locale]/my-usage/_components/statistics-summary-card.tsx around
lines 119 - 138, The current logic shows t("noData") for an empty page slice
even if the overall array has items; update the rendering condition for
keyPageItems and userPageItems so you only display t("noData") when the entire
source array (stats?.keyModelBreakdown or stats?.userModelBreakdown) is empty,
and for an empty slice on a non-empty source either render nothing or a
different message (e.g., t("noDataOnThisPage")); use the existing symbols
breakdownPage, keyModelBreakdown, userModelBreakdown, keyPageItems,
userPageItems and MODEL_BREAKDOWN_PAGE_SIZE to implement this conditional check.
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
![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
No significant issues identified in this PR. This is a well-structured release PR containing critical proxy fixes, Gemini Vertex AI support, and UI performance improvements.
PR Size: XL
- Lines changed: 1132 (639 additions, 493 deletions)
- Files changed: 19
Split Suggestion
For future releases, consider splitting into smaller PRs:
- Proxy fixes only (Host header + Session ID removal)
- Vertex AI support (format-mapper + session + tests)
- UI improvements (logs + my-usage components)
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Validation Notes
Session ID Removal: Validated that SessionManager.getOrCreateSessionId() properly handles null clientSessionId via:
- Content hash fallback for message matching
- Random session ID generation when hash unavailable
Host Header Fix: HeaderProcessor.extractHost() includes proper error handling with logging.
Vertex AI Support: Regex patterns correctly match /v1/publishers/google/models/{model}:{action} paths.
i18n Compliance: All 5 languages (zh-CN, zh-TW, en, ja, ru) have complete translations for new UI strings.
Test Coverage: New tests added for Vertex AI model extraction and Host header correction.
Automated review by Claude AI
| stats?.userModelBreakdown.length ?? 0 | ||
| ); | ||
| const breakdownTotalPages = Math.ceil(maxBreakdownLen / MODEL_BREAKDOWN_PAGE_SIZE); | ||
| const sliceStart = (breakdownPage - 1) * MODEL_BREAKDOWN_PAGE_SIZE; |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Model breakdown pagination can get stuck on an empty page after refresh
Evidence: src/app/[locale]/my-usage/_components/statistics-summary-card.tsx:134
const breakdownTotalPages = Math.ceil(maxBreakdownLen / MODEL_BREAKDOWN_PAGE_SIZE);
const sliceStart = (breakdownPage - 1) * MODEL_BREAKDOWN_PAGE_SIZE;
const keyPageItems = stats?.keyModelBreakdown.slice(sliceStart, sliceEnd) ?? [];
...
{breakdownTotalPages > 1 && (Why this is a problem: breakdownPage is stateful, but breakdownTotalPages can shrink when stats auto-refreshes. If the user is on page 2+ and the refreshed maxBreakdownLen drops below one page, keyPageItems/userPageItems become empty and pagination controls disappear (breakdownTotalPages > 1 is false), showing noData with no way to navigate back to page 1.
Suggested fix:
// Clamp breakdownPage when the total page count changes (e.g. auto-refresh changes breakdown size)
useEffect(() => {
setBreakdownPage((p) => {
if (breakdownTotalPages <= 1) return 1;
return Math.min(p, breakdownTotalPages);
});
}, [breakdownTotalPages]);
| const proxyUrl = "https://eu-west.anthropic.com/v1/messages"; | ||
| const actualHost = HeaderProcessor.extractHost(proxyUrl); | ||
| processedHeaders.set("host", actualHost); | ||
|
|
There was a problem hiding this comment.
[MEDIUM] [TEST-INCOMPLETE] Host-header correction test doesn't assert the production behavior
Evidence: tests/unit/proxy/proxy-forwarder-host-header-fix.test.ts:100
const actualHost = HeaderProcessor.extractHost(proxyUrl);
processedHeaders.set("host", actualHost);Why this is a problem: The production fix is in src/app/v1/_lib/proxy/forwarder.ts where Host is corrected after proxyUrl is computed. This test applies the correction directly, so it would still pass even if the production line were removed, reducing regression protection. This conflicts with CLAUDE.md:
2. Test Coverage - All new features must have unit test coverage of at least 80%
Suggested fix:
// forwarder.ts (factor into a unit-testable helper)
export function applyHostFromProxyUrl(headers: Headers, proxyUrl: string): void {
headers.set("host", HeaderProcessor.extractHost(proxyUrl));
}
// forwarder.ts (use helper at the callsite)
applyHostFromProxyUrl(processedHeaders, proxyUrl);
// proxy-forwarder-host-header-fix.test.ts (assert helper behavior)
applyHostFromProxyUrl(processedHeaders, "https://eu-west.anthropic.com/v1/messages");
expect(processedHeaders.get("host")).toBe("eu-west.anthropic.com");
There was a problem hiding this comment.
Code Review Summary
This PR bundles proxy fixes (Vertex AI model extraction, Host header correction, session ID behavior), a logs performance guard, and My Usage UI/i18n changes. Two actionable issues remain: a pagination state bug in model breakdown that can hide data after refresh, and a new host-header test that doesn't actually exercise the production fix.
PR Size: XL
- Lines changed: 1132
- Files changed: 19
- Split suggestion (XL): Consider splitting into separate PRs for (1) proxy-layer fixes + tests, (2) My Usage UI + i18n message updates, (3) logs/dashboard performance changes.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 1 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None
High Priority Issues (Should Fix)
- [HIGH] [LOGIC-BUG] Model breakdown pagination can get stuck on an empty page after refresh (confidence 86) —
src/app/[locale]/my-usage/_components/statistics-summary-card.tsx:134 - [MEDIUM] [TEST-INCOMPLETE] Host-header correction test doesn't assert the production behavior (confidence 82) —
tests/unit/proxy/proxy-forwarder-host-header-fix.test.ts:100
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
Reviewed PR #795 and updated the size label to size/XL (initially hit a 422 posting inline comments using line; retried successfully using diff position).
Inline comments posted (diff lines only):
src/app/[locale]/my-usage/_components/statistics-summary-card.tsx:134— pagination state can go out-of-range after refresh and hide data; clampbreakdownPagewhenbreakdownTotalPageschanges.tests/unit/proxy/proxy-forwarder-host-header-fix.test.ts:100— test manually setshost, so it won’t fail if the production fix is removed; suggested factoring the behavior into a helper and testing that.
Submitted the required PR review summary via gh pr review --comment (includes XL split suggestions).
2 participants
Summary
Release v0.5.8 - Contains critical proxy fixes, Gemini Vertex AI support, and UI performance improvements.
Changes Included
1. fix(proxy): correct Host header for multi-endpoint providers
Problem: When providers have multiple endpoints (e.g., regional endpoints or MCP passthrough), the Host header was set based on
provider.url, which could differ from the actual request target (proxyUrl). This caused TLS certificate validation failures.Solution: Extract the actual host from
proxyUrland set the Host header to match the real request target.Related Issues:
2. fix(proxy): remove deterministic session ID to prevent collisions
Problem:
generateDeterministicSessionId()hashed static per-user attributes (User-Agent, IP, API Key prefix) without any time dimension, producing identical session IDs for the same user across separate conversations. This caused:Solution: Remove
generateDeterministicSessionId()entirely. The existing fallback ingetOrCreateSessionId()already handles null client session IDs correctly using content hash or random ID generation.Related Issues:
3. fix(proxy): support Gemini Vertex AI publishers path
Problem: The proxy did not recognize Gemini Vertex AI paths like
/v1/publishers/google/models/{model}:generateContent.Solution:
extractModelFromPath()to extract model names from publishers URLsChanges:
src/app/v1/_lib/proxy/format-mapper.ts: Added Vertex AI patternsrc/app/v1/_lib/proxy/session.ts: Updated model extraction regex4. perf(logs): hide stats panel when no filters active
Problem: The usage logs stats panel was always visible, consuming space and making unnecessary database queries even when no filters were applied.
Solution: Only render the stats panel when at least one filter is active.
Related Issues:
5. cleanup(logs): remove legacy usage-logs-view component
The old non-virtualized
usage-logs-view.tsxhas been completely replaced byusage-logs-view-virtualized.tsx. Removed the 262-line legacy file.Testing
All changes include comprehensive unit tests:
tests/unit/proxy/proxy-forwarder-host-header-fix.test.ts(166 lines) - Host header correction teststests/unit/proxy/gemini-vertex-model-extraction.test.ts(90 lines) - Vertex AI model extraction testsmetadata-injection.test.tsandsession-guard-warmup-intercept.test.tsBreaking Changes
None.
Deployment Notes
Release v0.5.8