-
-
Notifications
You must be signed in to change notification settings - Fork 181
fix(logs): improve cache tooltip display when 5m/1h breakdown unavail… #445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,10 +252,24 @@ export function UsageLogsTable({ | |
| <TooltipContent align="end" className="text-xs space-y-1"> | ||
| <div className="font-medium">{t("logs.columns.cacheWrite")}</div> | ||
| <div className="pl-2"> | ||
| 5m: {formatTokenAmount(log.cacheCreation5mInputTokens)} | ||
| 5m:{" "} | ||
| {formatTokenAmount( | ||
| (log.cacheCreation5mInputTokens ?? 0) > 0 | ||
| ? log.cacheCreation5mInputTokens | ||
| : log.cacheTtlApplied !== "1h" | ||
| ? log.cacheCreationInputTokens | ||
| : 0 | ||
| )} | ||
| </div> | ||
| <div className="pl-2"> | ||
| 1h: {formatTokenAmount(log.cacheCreation1hInputTokens)} | ||
| 1h:{" "} | ||
| {formatTokenAmount( | ||
| (log.cacheCreation1hInputTokens ?? 0) > 0 | ||
| ? log.cacheCreation1hInputTokens | ||
| : log.cacheTtlApplied === "1h" | ||
| ? log.cacheCreationInputTokens | ||
| : 0 | ||
| )} | ||
|
Comment on lines
+256
to
+272
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logic to determine the token amount for 5m and 1h cache writes uses nested ternary operators, which are difficult to read and maintain. This exact logic is also duplicated in To improve code quality, I recommend extracting this logic into a shared helper function. For example, you could create a function in a utility file: // in a utils file
export function getDisplayCacheTokens(log, type: '5m' | '1h') {
if (type === '5m') {
if ((log.cacheCreation5mInputTokens ?? 0) > 0) {
return log.cacheCreation5mInputTokens;
}
return log.cacheTtlApplied !== '1h' ? log.cacheCreationInputTokens : 0;
}
// type === '1h'
if ((log.cacheCreation1hInputTokens ?? 0) > 0) {
return log.cacheCreation1hInputTokens;
}
return log.cacheTtlApplied === '1h' ? log.cacheCreationInputTokens : 0;
}Using this helper would make the component code much cleaner and prevent logic duplication. |
||
| </div> | ||
| <div className="font-medium mt-1">{t("logs.columns.cacheRead")}</div> | ||
| <div className="pl-2"> | ||
|
|
@@ -425,6 +439,7 @@ export function UsageLogsTable({ | |
| billingModelSource={billingModelSource} | ||
| inputTokens={log.inputTokens} | ||
| outputTokens={log.outputTokens} | ||
| cacheCreationInputTokens={log.cacheCreationInputTokens} | ||
| cacheCreation5mInputTokens={log.cacheCreation5mInputTokens} | ||
| cacheCreation1hInputTokens={log.cacheCreation1hInputTokens} | ||
| cacheReadInputTokens={log.cacheReadInputTokens} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -428,10 +428,24 @@ export function VirtualizedLogsTable({ | |
| <TooltipContent align="end" className="text-xs space-y-1"> | ||
| <div className="font-medium">{t("logs.columns.cacheWrite")}</div> | ||
| <div className="pl-2"> | ||
| 5m: {formatTokenAmount(log.cacheCreation5mInputTokens)} | ||
| 5m:{" "} | ||
| {formatTokenAmount( | ||
| (log.cacheCreation5mInputTokens ?? 0) > 0 | ||
| ? log.cacheCreation5mInputTokens | ||
| : log.cacheTtlApplied !== "1h" | ||
| ? log.cacheCreationInputTokens | ||
| : 0 | ||
| )} | ||
| </div> | ||
| <div className="pl-2"> | ||
| 1h: {formatTokenAmount(log.cacheCreation1hInputTokens)} | ||
| 1h:{" "} | ||
| {formatTokenAmount( | ||
| (log.cacheCreation1hInputTokens ?? 0) > 0 | ||
| ? log.cacheCreation1hInputTokens | ||
| : log.cacheTtlApplied === "1h" | ||
| ? log.cacheCreationInputTokens | ||
| : 0 | ||
| )} | ||
|
Comment on lines
+432
to
+448
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This component duplicates the complex logic for calculating cache token amounts from |
||
| </div> | ||
| <div className="font-medium mt-1">{t("logs.columns.cacheRead")}</div> | ||
| <div className="pl-2">{formatTokenAmount(log.cacheReadInputTokens)}</div> | ||
|
|
@@ -555,6 +569,7 @@ export function VirtualizedLogsTable({ | |
| billingModelSource={billingModelSource} | ||
| inputTokens={log.inputTokens} | ||
| outputTokens={log.outputTokens} | ||
| cacheCreationInputTokens={log.cacheCreationInputTokens} | ||
| cacheCreation5mInputTokens={log.cacheCreation5mInputTokens} | ||
| cacheCreation1hInputTokens={log.cacheCreation1hInputTokens} | ||
| cacheReadInputTokens={log.cacheReadInputTokens} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for displaying the 5m and 1h cache write tokens is duplicated and makes the component harder to read. To improve readability and maintainability, you can use a self-invoking function for each block. This encapsulates the logic, removes repetition, and makes the rendering logic cleaner.