fix: save cache creation 5m/1h tokens and TTL to database, fix React …#310
fix: save cache creation 5m/1h tokens and TTL to database, fix React …#310
Conversation
…rendering bug - Add missing cacheCreation5mInputTokens, cacheCreation1hInputTokens, cacheTtlApplied fields to updateMessageRequestDetails calls in handleNonStream and finalizeStream - Fix React short-circuit rendering bug where '0 && expr' renders literal '0' in error-details-dialog.tsx and usage-logs-table.tsx billing tooltips - Change conditional renders from 'x && x > 0 &&' to '(x ?? 0) > 0 &&'
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 addresses two distinct but important areas: enhancing data persistence for cache-related usage metrics and rectifying a user interface rendering anomaly. It ensures that comprehensive cache creation and TTL information is saved to the database, providing a more complete picture of system usage. Concurrently, it fixes a subtle React rendering issue that caused '0' to appear in billing tooltips when a numerical value was zero, improving the clarity and correctness of the displayed information. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces two main improvements. First, it correctly persists cache creation metrics (cacheCreation5mInputTokens, cacheCreation1hInputTokens, cacheTtlApplied) to the database by updating the updateMessageRequestDetails calls for both streaming and non-streaming responses in response-handler.ts. Second, it resolves a React rendering issue in error-details-dialog.tsx and usage-logs-table.tsx where a token count of zero would cause the number '0' to be displayed in the UI. The conditional rendering logic has been updated from x && x > 0 to (x ?? 0) > 0, which is a robust fix. The changes are well-executed and address the described issues effectively. The code quality is high, and I have no further suggestions.
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The changes correctly fix two distinct bugs:
-
React short-circuit rendering bug: Changed
x && x > 0 &&to(x ?? 0) > 0 &&in billing tooltips to prevent React from rendering the literal0when token counts are zero. -
Missing cache token fields: Added
cacheCreation5mInputTokens,cacheCreation1hInputTokens, andcacheTtlAppliedfields to database updates inhandleNonStreamandfinalizeStreamfunctions.
PR Size: XS
- Lines changed: 50
- Files changed: 3
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - N/A (UI rendering fix)
- Code clarity - Good
Automated review by Claude AI
Summary
Fix missing cache token fields in database updates and React rendering bugs in billing tooltips.
Problem
After the differentiated cache billing feature was implemented (#277/#278), two bugs were discovered:
Missing database fields: The
updateMessageRequestDetailscalls inhandleNonStreamandfinalizeStreamwere not saving the new cache fields (cacheCreation5mInputTokens,cacheCreation1hInputTokens,cacheTtlApplied), causing cache token data to be lost.React short-circuit rendering bug: Conditional renders using
x && x > 0 && <Component />would render a literal0whenx = 0due to JavaScript's short-circuit evaluation behavior. This affected billing tooltips in the logs UI.Related Issues:
Solution
Database Fix
Added the missing fields to
updateMessageRequestDetailscalls in bothhandleNonStreamandfinalizeStreamfunctions:cacheCreation5mInputTokenscacheCreation1hInputTokenscacheTtlAppliedReact Rendering Fix
Changed conditional rendering pattern from:
to:
This ensures
0is never passed to the JSX expression, preventing the literal0from being rendered.Changes
src/app/v1/_lib/proxy/response-handler.tssrc/app/[locale]/dashboard/logs/_components/error-details-dialog.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.tsxChecklist
Description enhanced by Claude AI