-
-
Notifications
You must be signed in to change notification settings - Fork 317
fix: Include cache tokens in live usage token count calculations #193
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
Conversation
Previously, the live usage monitor and blocks command only counted input and output tokens, excluding cache_creation_input_tokens and cache_read_input_tokens from the total. This caused inaccurate token usage reporting, especially when cache creation tokens (which cost more than input tokens) were significant. Changes: - Add getTotalTokens() helper function to sum all four token types - Update live rendering display to use getTotalTokens() for accurate counts - Fix burn rate calculation to include cache tokens in tokens/minute metric - Fix usage projection to include cache tokens in projected totals - Update blocks command to count cache tokens in all calculations - Add unit tests verifying cache tokens are included in calculations - Fix UI alignment issues in live display (SESSION and Models lines) This ensures users see accurate total token usage that matches their actual costs. Fixes ryoppippi#179
|
Caution Review failedThe pull request is closed. """ WalkthroughA helper function Changes
Sequence Diagram(s)sequenceDiagram
participant renderLiveDisplay
participant getTotalTokens
renderLiveDisplay->>getTotalTokens: Call with tokenCounts
getTotalTokens-->>renderLiveDisplay: Return sum of all token types
renderLiveDisplay->>renderLiveDisplay: Use totalTokens for display
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Summary of Changes
Hello @tifoji, 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 a critical issue in the live usage monitor by ensuring that all token types, including cache-related tokens, are correctly accounted for in total token calculations. By introducing a dedicated helper function and integrating it into the live display, the PR significantly improves the accuracy of burn rate and projection metrics, providing users with a more complete understanding of their token usage. Additionally, minor UI refinements enhance the display's readability.
Highlights
- Token Calculation Accuracy: Introduced a new helper function,
getTotalTokens, to accurately sum all four token types: input, output, cache creation input, and cache read input. This ensures that the live usage monitor provides a comprehensive view of token consumption. - Live Display Integration: Updated the
renderLiveDisplayfunction to utilize the newgetTotalTokenshelper. This change directly impacts the live burn rate and projection calculations, making them more precise by including previously omitted cache tokens. - UI Formatting Adjustments: Made minor adjustments to padding calculations for the session bar and models line within the live display, improving the visual alignment and presentation of information.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request includes cache tokens in live usage token count calculations, updating the live usage monitor and blocks command to include all four token types (input, output, cache_creation_input, cache_read_input). A getTotalTokens() helper function is added to sum all token types, and burn rate and projection calculations are updated to include cache tokens.
src/_live-rendering.ts
Outdated
| function getTotalTokens(tokenCounts: SessionBlock['tokenCounts']): number { | ||
| return ( | ||
| tokenCounts.inputTokens | ||
| + tokenCounts.outputTokens | ||
| + tokenCounts.cacheCreationInputTokens | ||
| + tokenCounts.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.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/_live-rendering.ts (1)
38-44: Consider potential code duplication with existing getTotalTokens function.A similar function exists in
src/calculate-cost.tswith slightly different property names (cacheCreationTokensvscacheCreationInputTokens,cacheReadTokensvscacheReadInputTokens). While the different property names suggest they handle different data structures, consider if there's an opportunity to consolidate or ensure naming consistency.Let me check if these functions could be unified or if the naming differences are intentional:
#!/bin/bash # Compare the two getTotalTokens functions and their usage patterns echo "=== Function in calculate-cost.ts ===" ast-grep --pattern 'function getTotalTokens($$$) { $$$ }' echo "=== TokenCounts type definition ===" ast-grep --pattern 'type TokenCounts = $$$' echo "=== TokenData type definition ===" ast-grep --pattern 'type TokenData = $$$'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/_live-rendering.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.ts`: Use .ts extensions for local file imports (e.g., import { foo } from './utils.ts'). No console.log allowed except where explicitly disabled with eslint-disable; use logg...
**/*.ts: Use .ts extensions for local file imports (e.g., import { foo } from './utils.ts').
No console.log allowed except where explicitly disabled with eslint-disable; use logger.ts instead.
Tests are written directly in the same files as the source code, not in separate test files. Use in-source testing pattern with if (import.meta.vitest != null) blocks.
Dynamic imports using await import() should only be used within test blocks to avoid tree-shaking issues.
Only export constants, functions, and types that are actually used by other modules. Internal/private constants that are only used within the same file should NOT be exported.
Variables should start with lowercase (camelCase), types with uppercase (PascalCase), and constants can use UPPER_SNAKE_CASE.
After making any code changes, always run bun run format, bun typecheck, and bun run test to ensure code quality.
File paths must always use Node.js path utilities for cross-platform compatibility.
Dependencies should always be added as devDependencies unless explicitly requested otherwise.
src/_live-rendering.ts
`**/_*.ts`: Internal files should use underscore prefix (e.g., _types.ts, _utils.ts, _consts.ts).
**/_*.ts: Internal files should use underscore prefix (e.g., _types.ts, _utils.ts, _consts.ts).
src/_live-rendering.ts
🧬 Code Graph Analysis (1)
src/_live-rendering.ts (2)
src/calculate-cost.ts (1)
getTotalTokens(76-83)src/_session-blocks.ts (1)
SessionBlock(49-60)
🪛 ESLint
src/_live-rendering.ts
[error] 161-161: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 347-347: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
🔇 Additional comments (3)
src/_live-rendering.ts (3)
33-45: LGTM! Helper function correctly implements cache token inclusion.The function properly sums all four token types as specified in the PR objectives, ensuring cache tokens are included in live usage calculations.
110-110: Excellent update to use the new helper function.This change ensures the live display now includes cache tokens in the total count, addressing the core issue described in PR #179.
161-161: Static analysis errors appear to be false positives.The ESLint errors about "unsafe call of error type" seem incorrect here, as both
sessionBarStrandmodelsLineare constructed from string concatenations and should be string types. The padding adjustments from offset 3 to 2 appear to be legitimate layout improvements.Let me verify the string types to confirm these are false positives:
#!/bin/bash # Check the type definitions and usage patterns for the variables flagged by ESLint ast-grep --pattern 'const sessionBarStr = $$$' ast-grep --pattern 'const modelsLine = $$$'Also applies to: 347-347
…e duplication - Create _token-utils.ts with shared getTotalTokens function - Support both raw usage data format (cacheCreationInputTokens) and aggregated format (cacheCreationTokens) - Remove duplicate implementations from 4 files - Maintain backward compatibility with re-export in calculate-cost.ts - Add comprehensive tests for both token formats Addresses code review feedback about duplication.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/_token-utils.ts (2)
40-56: Improve type safety in the function implementation.The function logic is correct, but the type casting on lines 44 and 48 can be improved for better type safety.
Apply this diff to improve type safety:
- const cacheCreation = 'cacheCreationInputTokens' in tokenCounts - ? tokenCounts.cacheCreationInputTokens - : (tokenCounts).cacheCreationTokens; + const cacheCreation = 'cacheCreationInputTokens' in tokenCounts + ? tokenCounts.cacheCreationInputTokens + : (tokenCounts as AggregatedTokenCounts).cacheCreationTokens; - const cacheRead = 'cacheReadInputTokens' in tokenCounts - ? tokenCounts.cacheReadInputTokens - : (tokenCounts).cacheReadTokens; + const cacheRead = 'cacheReadInputTokens' in tokenCounts + ? tokenCounts.cacheReadInputTokens + : (tokenCounts as AggregatedTokenCounts).cacheReadTokens;
58-121: Comprehensive test coverage with minor styling issue.The test suite provides excellent coverage for both token formats and edge cases. The vitest typing errors in static analysis are likely false positives due to type configuration.
Add a newline at the end of the file to satisfy the ESLint rule:
}); } +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/_token-utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.ts`: Use .ts extensions for local file imports (e.g., import { foo } from './utils.ts'). No console.log allowed except where explicitly disabled with eslint-disable; use logg...
**/*.ts: Use .ts extensions for local file imports (e.g., import { foo } from './utils.ts').
No console.log allowed except where explicitly disabled with eslint-disable; use logger.ts instead.
Tests are written directly in the same files as the source code, not in separate test files. Use in-source testing pattern with if (import.meta.vitest != null) blocks.
Dynamic imports using await import() should only be used within test blocks to avoid tree-shaking issues.
Only export constants, functions, and types that are actually used by other modules. Internal/private constants that are only used within the same file should NOT be exported.
Variables should start with lowercase (camelCase), types with uppercase (PascalCase), and constants can use UPPER_SNAKE_CASE.
After making any code changes, always run bun run format, bun typecheck, and bun run test to ensure code quality.
File paths must always use Node.js path utilities for cross-platform compatibility.
Dependencies should always be added as devDependencies unless explicitly requested otherwise.
src/_token-utils.ts
`**/_*.ts`: Internal files should use underscore prefix (e.g., _types.ts, _utils.ts, _consts.ts).
**/_*.ts: Internal files should use underscore prefix (e.g., _types.ts, _utils.ts, _consts.ts).
src/_token-utils.ts
🪛 ESLint
src/_token-utils.ts
[error] 60-60: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 61-61: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 68-68: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 68-68: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 68-68: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 71-71: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 78-78: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 78-78: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 78-78: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 81-81: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 88-88: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 88-88: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 88-88: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 91-91: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 98-98: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 98-98: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 98-98: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 101-101: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 108-108: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 108-108: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 108-108: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 111-111: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 118-118: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 118-118: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 118-118: Unsafe member access .toBe on an error typed value.
(ts/no-unsafe-member-access)
[error] 121-121: Newline required at end of file but not found.
(style/eol-last)
🔇 Additional comments (2)
src/_token-utils.ts (2)
1-7: Excellent documentation and file structure.The file header provides clear context about the module's purpose and usage throughout the application. Good adherence to internal file naming convention with underscore prefix.
9-32: Well-designed type system for token counting.The type definitions effectively handle both naming conventions used across the application. The union type
AnyTokenCountsprovides flexibility while maintaining type safety.
- Import getTotalTokens from shared _token-utils.ts - Remove duplicate helper function - Maintain consistent token counting across commands
- Import getTotalTokens from shared _token-utils.ts - Add re-export for backward compatibility - Update TokenData type alias to use AggregatedTokenCounts - Maintain existing public API while eliminating duplication
- Import getTotalTokens from shared _token-utils.ts - Remove duplicate helper function - Fix alignment issues by using consistent token counting - Maintain consistent live display behavior
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.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/_live-rendering.ts(12 hunks)src/calculate-cost.ts(4 hunks)src/commands/blocks.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.ts`: Use .ts extensions for local file imports (e.g., import { foo } from './utils.ts'). No console.log allowed except where explicitly disabled with eslint-disable; use logg...
**/*.ts: Use .ts extensions for local file imports (e.g., import { foo } from './utils.ts').
No console.log allowed except where explicitly disabled with eslint-disable; use logger.ts instead.
Tests are written directly in the same files as the source code, not in separate test files. Use in-source testing pattern with if (import.meta.vitest != null) blocks.
Dynamic imports using await import() should only be used within test blocks to avoid tree-shaking issues.
Only export constants, functions, and types that are actually used by other modules. Internal/private constants that are only used within the same file should NOT be exported.
Variables should start with lowercase (camelCase), types with uppercase (PascalCase), and constants can use UPPER_SNAKE_CASE.
After making any code changes, always run bun run format, bun typecheck, and bun run test to ensure code quality.
File paths must always use Node.js path utilities for cross-platform compatibility.
Dependencies should always be added as devDependencies unless explicitly requested otherwise.
src/calculate-cost.tssrc/_live-rendering.tssrc/commands/blocks.ts
`**/_*.ts`: Internal files should use underscore prefix (e.g., _types.ts, _utils.ts, _consts.ts).
**/_*.ts: Internal files should use underscore prefix (e.g., _types.ts, _utils.ts, _consts.ts).
src/_live-rendering.ts
🧬 Code Graph Analysis (2)
src/_live-rendering.ts (1)
src/_utils.ts (2)
formatNumber(293-295)formatCurrency(302-304)
src/commands/blocks.ts (2)
src/calculate-cost.ts (1)
getTotalTokens(70-70)src/_consts.ts (1)
BLOCKS_WARNING_THRESHOLD(20-20)
🪛 ESLint
src/calculate-cost.ts
[error] 211-211: Newline required at end of file but not found.
(style/eol-last)
src/_live-rendering.ts
[error] 148-148: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 162-162: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 185-185: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 186-186: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 187-187: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 188-188: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 189-189: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 190-190: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 190-190: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 190-190: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 191-191: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 191-191: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 191-191: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 191-191: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 192-192: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 193-193: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 194-194: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 195-195: Expected indentation of 3 tabs but found 2.
(style/indent)
[error] 216-216: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 217-217: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 217-217: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 217-217: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 218-218: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 218-218: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 218-218: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 219-219: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 219-219: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 219-219: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 220-220: Expected indentation of 3 tabs but found 2.
(style/indent)
[error] 222-222: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 223-223: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 223-223: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 223-223: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 224-224: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 225-225: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 225-225: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 225-225: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 226-226: Expected indentation of 3 tabs but found 2.
(style/indent)
[error] 229-229: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 238-238: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 264-264: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 265-265: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 266-266: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 267-267: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 268-268: Expected indentation of 6 tabs but found 5.
(style/indent)
[error] 269-269: Expected indentation of 6 tabs but found 5.
(style/indent)
[error] 269-269: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 269-269: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 270-270: Expected indentation of 6 tabs but found 5.
(style/indent)
[error] 270-270: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 270-270: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 270-270: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 271-271: Expected indentation of 6 tabs but found 5.
(style/indent)
[error] 272-272: Expected indentation of 6 tabs but found 5.
(style/indent)
[error] 273-273: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 274-274: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 275-275: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 275-275: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 275-275: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 275-275: Unsafe member access .gray on an error typed value.
(ts/no-unsafe-member-access)
[error] 277-283: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 279-279: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 279-279: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 279-279: Unsafe member access .red on an error typed value.
(ts/no-unsafe-member-access)
[error] 280-280: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 281-281: Expected indentation of 6 tabs but found 5.
(style/indent)
[error] 281-281: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 281-281: Unsafe member access .yellow on an error typed value.
(ts/no-unsafe-member-access)
[error] 282-282: Expected indentation of 6 tabs but found 5.
(style/indent)
[error] 282-282: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 282-282: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 283-283: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 283-283: Unsafe member access .green on an error typed value.
(ts/no-unsafe-member-access)
[error] 286-286: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 286-286: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 286-286: Unsafe member access .bold on an error typed value.
(ts/no-unsafe-member-access)
[error] 287-287: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 287-287: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 290-290: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 304-304: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 309-309: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 323-323: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 334-334: Unsafe call of a(n) error type typed value.
(ts/no-unsafe-call)
[error] 423-423: Newline required at end of file but not found.
(style/eol-last)
src/commands/blocks.ts
[error] 29-29: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 30-30: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 31-31: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 32-32: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 33-33: Expected indentation of 3 tabs but found 2.
(style/indent)
[error] 39-39: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 40-40: Expected indentation of 5 tabs but found 4.
(style/indent)
[error] 41-41: Expected indentation of 4 tabs but found 3.
(style/indent)
[error] 278-278: Expected indentation of 9 tabs but found 8.
(style/indent)
[error] 278-278: Unsafe argument of type any assigned to a parameter of type string | undefined.
(ts/no-unsafe-argument)
[error] 278-278: Unsafe member access .values on an any value.
(ts/no-unsafe-member-access)
[error] 279-279: Expected indentation of 9 tabs but found 8.
(style/indent)
[error] 280-280: Expected indentation of 10 tabs but found 9.
(style/indent)
[error] 281-281: Expected indentation of 12 tabs but found 10.
(style/indent)
[error] 282-282: Expected indentation of 12 tabs but found 10.
(style/indent)
[error] 283-283: Expected indentation of 12 tabs but found 10.
(style/indent)
[error] 284-284: Expected indentation of 12 tabs but found 10.
(style/indent)
[error] 285-285: Expected indentation of 13 tabs but found 11.
(style/indent)
[error] 286-286: Expected indentation of 13 tabs but found 11.
(style/indent)
[error] 287-287: Expected indentation of 11 tabs but found 9.
(style/indent)
[error] 288-288: Expected indentation of 10 tabs but found 9.
(style/indent)
[error] 289-289: Expected indentation of 8 tabs but found 7.
(style/indent)
[error] 109-486: Unsafe assignment of an error typed value.
(ts/no-unsafe-assignment)
[error] 486-486: Newline required at end of file but not found.
(style/eol-last)
🔇 Additional comments (8)
src/commands/blocks.ts (2)
14-14: LGTM: Proper import of shared token utility.The import correctly brings in the centralized
getTotalTokensfunction, aligning with the PR objective to standardize token counting across the codebase.
184-184: Excellent refactoring to use shared token utility.Replacing manual token summation with
getTotalTokens(block.tokenCounts)ensures that all four token types (input, output, cache_creation_input, cache_read_input) are properly included in calculations, which directly addresses the PR objective.Also applies to: 342-342, 406-406, 430-430
src/_live-rendering.ts (3)
19-19: LGTM: Proper import resolves previous duplication concern.The import correctly uses the centralized
getTotalTokensfunction from the shared utility module, addressing the previous review comment about code duplication.
97-97: Excellent adoption of shared token utility.Replacing the manual summation
block.tokenCounts.inputTokens + block.tokenCounts.outputTokenswithgetTotalTokens(block.tokenCounts)ensures cache tokens are properly included in the live monitoring display.
148-148: Layout padding adjustments improve alignment.The changes from subtracting 3 to subtracting 2 in the padding calculations appear to correct alignment issues in the live display box layout.
Also applies to: 162-162, 229-229, 238-238, 290-290, 304-304, 309-309, 323-323, 334-334
src/calculate-cost.ts (3)
11-11: LGTM: Clean import of shared token utilities.The imports correctly bring in the centralized types and functions from the shared utility module, supporting the PR's goal to standardize token counting logic.
Also applies to: 13-13
24-27: Good backward compatibility approach with deprecation notice.The type alias maintains compatibility while encouraging migration to the shared type. The deprecation comment provides clear guidance for future updates.
69-70: Excellent backward compatibility strategy.Re-exporting
getTotalTokensensures existing code continues to work while centralizing the implementation in the shared utility module.
|
Created a shared The refactoring is complete and all tests are passing (except for some timezone-related date formatting tests which are unrelated to this PR). |
|
Yah I think we need to use Too bad I cannot share my screenshot but a bit disappointed that the right side border is still not aligned. Thought it looks perfect on mine. |
|
Thank you for your contribution |
|
Also @a-c-m could you review this? |
Yah would be great if others can test and confirm but the calcs were updated and UI slightly tweaked to get the aligned border. |
commit: |
a-c-m
left a comment
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.
Looks reasonable, from the code review. Moves some stuff around, but the main thing is:
export function getTotalTokens(tokens: TokenData): number {
return (
tokens.inputTokens
+ tokens.outputTokens
+ tokens.cacheCreationTokens
+ tokens.cacheReadTokens
);
}
into
export function getTotalTokens(tokenCounts: AnyTokenCounts): number {
// Support both property naming conventions
const cacheCreation = 'cacheCreationInputTokens' in tokenCounts
? tokenCounts.cacheCreationInputTokens
: (tokenCounts).cacheCreationTokens;
const cacheRead = 'cacheReadInputTokens' in tokenCounts
? tokenCounts.cacheReadInputTokens
: (tokenCounts).cacheReadTokens;
return (
tokenCounts.inputTokens
+ tokenCounts.outputTokens
+ cacheCreation
+ cacheRead
);
}
No idea if this is a good idea or not. But, swapping to use the getTotalTokens in the blocks.ts is a good idea for sure.
|
Thank you @a-c-m |
ryoppippi
left a comment
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.
Sorry for late, but LGTM
Guys good job!!!



Summary
Test plan
bun run build./dist/index.js blocks --liveto see live monitoring with accurate token countsbun run testto verify calculations include cache tokensFixes #179
Summary by CodeRabbit