Repository navigation
fix(terminal): increase minimum width for numeric columns to prevent truncation - #701
Conversation
WalkthroughIncreases the minimum width used for right-aligned (numeric) columns in the table renderer from 10 to 14 characters during responsive width adjustment to avoid truncating large numeric values. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Renderer / CLI
participant Table as Table.toString()
participant Layout as WidthCalculator
Note over Caller,Table: Request table rendering
Caller->>Table: toString()
Table->>Layout: compute column widths
Layout-->>Table: widths with minWidth applied
Note right of Layout: For numeric columns\nminWidth = 14 (was 10)
Table->>Caller: rendered table string (no truncation for large numbers)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/terminal/src/table.ts (1)
64-64: Consider using logger from logger.ts instead of console.warn.The coding guidelines specify using
logger.tsinstead of console methods for logging. Whileconsole.warnis used here as a default fallback, consider importing and using the proper logger fromlogger.tsas the default value.As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/terminal/src/table.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use tab indentation and double quotes (ESLint formatting)
Do not use console.log; only allow where explicitly disabled via eslint-disable
Always use Node.js path utilities for file paths for cross-platform compatibility
Use .ts extensions for local file imports (e.g., import { foo } from './utils.ts')
Prefer @praha/byethrow Result type over traditional try-catch for functional error handling
Use Result.try() to wrap operations that may throw (e.g., JSON parsing)
Use Result.isFailure() for checking errors instead of negating isSuccess()
Use early return on failures (e.g., if (Result.isFailure(r)) continue) instead of ternary patterns
For async operations, create a wrapper using Result.try() and call it
Keep traditional try-catch only for complex file I/O or legacy code that’s hard to refactor
Always use Result.isFailure() and Result.isSuccess() type guards for clarity
Variables use camelCase naming
Types use PascalCase naming
Constants can use UPPER_SNAKE_CASE
Only export constants, functions, and types that are actually used by other modules
Do not export internal/private constants that are only used within the same file
Before exporting a constant, verify it is referenced by other modules
Use Vitest globals (describe, it, expect) without imports in test blocks
Never use await import() dynamic imports anywhere in the codebase
Never use dynamic imports inside Vitest test blocks
Use fs-fixture createFixture() for mock Claude data directories in tests
All tests must use current Claude 4 models (not Claude 3)
Test coverage should include both Sonnet and Opus models
Model names in tests must exactly match LiteLLM pricing database entries
Use logger.ts instead of console.log for logging
Files:
packages/terminal/src/table.ts
🔇 Additional comments (2)
packages/terminal/src/table.ts (2)
193-194: Fix is correct; test coverage for numeric truncation in narrow terminals would improve robustness.The minimum width increase from 10 to 14 at line 194 correctly aligns with the non-responsive path's calculation at line 173 (Math.max(width + 3, 11), which yields 14 for 11-character numbers). This prevents truncation of comma-formatted token numbers like "536,073,421" (11 characters) when the table scales down in narrow terminals.
While unit tests for
formatNumberexist (lines 886-914), there's no integration test coverage for verifying numeric values don't truncate when rendering tables in narrow terminal widths. Consider adding a test case that callsrenderStatisticsor similar with a mocked terminal width to validate this scenario and prevent regression.
185-207: No changes needed—compact mode already mitigates the concern about minimum width constraints in narrow terminals.The code correctly handles the interaction between the increased minimum width (14) for numeric columns and compact mode. Compact mode is activated at line 135 when terminal width drops below the compactThreshold (default 100), which triggers before the width calculation logic. At line 137, the active configuration switches to compactHead with fewer columns, and rows are filtered accordingly at lines 145-146. This means the minimum width constraints at lines 194-206 are applied to an already-reduced column set, preventing overflow in narrow terminals. For example, in typical configurations, compact mode reduces from 5 columns to 3, eliminating extra numeric columns before the 14-character minimum is enforced.
…truncation Fixes ccusage#700 by increasing the minimum column width for right-aligned numeric columns from 10 to 14 characters in responsive mode. This ensures large token numbers (e.g., 536,073,421) are displayed in full without ellipsis truncation when the table is resized to fit narrow terminals. The fix maintains consistency with the normal mode's generous padding while providing enough space for comma-formatted large numbers.
85cd0da to
a70e497
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/terminal/src/table.ts (1)
783-832: Consider adding a test case for large numeric values in narrow terminals.While the existing tests cover responsive behavior well, consider adding a test that specifically verifies large token numbers (e.g., 536,073,421 or larger) are not truncated when the terminal width is narrow. This would prevent regression of the fix and document the expected behavior.
Example test structure:
it('should not truncate large numeric values in narrow terminals', () => { const table = new ResponsiveTable({ head: ['Date', 'Tokens', 'Cost'], colAligns: ['left', 'right', 'right'], }); const originalColumns = process.env.COLUMNS; process.env.COLUMNS = '60'; // Very narrow terminal table.push(['2024-01-01', '536,073,421', '$1.50']); const output = table.toString(); // Verify the large number is not truncated with ellipsis expect(output).toContain('536,073,421'); expect(output).not.toContain('…'); process.env.COLUMNS = originalColumns; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/terminal/src/table.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use tab indentation and double quotes (ESLint formatting)
Do not use console.log; only allow where explicitly disabled via eslint-disable
Always use Node.js path utilities for file paths for cross-platform compatibility
Use .ts extensions for local file imports (e.g., import { foo } from './utils.ts')
Prefer @praha/byethrow Result type over traditional try-catch for functional error handling
Use Result.try() to wrap operations that may throw (e.g., JSON parsing)
Use Result.isFailure() for checking errors instead of negating isSuccess()
Use early return on failures (e.g., if (Result.isFailure(r)) continue) instead of ternary patterns
For async operations, create a wrapper using Result.try() and call it
Keep traditional try-catch only for complex file I/O or legacy code that’s hard to refactor
Always use Result.isFailure() and Result.isSuccess() type guards for clarity
Variables use camelCase naming
Types use PascalCase naming
Constants can use UPPER_SNAKE_CASE
Only export constants, functions, and types that are actually used by other modules
Do not export internal/private constants that are only used within the same file
Before exporting a constant, verify it is referenced by other modules
Use Vitest globals (describe, it, expect) without imports in test blocks
Never use await import() dynamic imports anywhere in the codebase
Never use dynamic imports inside Vitest test blocks
Use fs-fixture createFixture() for mock Claude data directories in tests
All tests must use current Claude 4 models (not Claude 3)
Test coverage should include both Sonnet and Opus models
Model names in tests must exactly match LiteLLM pricing database entries
Use logger.ts instead of console.log for logging
Files:
packages/terminal/src/table.ts
🔇 Additional comments (1)
packages/terminal/src/table.ts (1)
193-195: LGTM! Appropriate fix for numeric truncation.The increase from 10 to 14 characters directly addresses the truncation issue for large token numbers. The comment clearly explains the rationale, and the minimum of 14 accommodates numbers like 536,073,421 (11 characters) with adequate padding. The responsive path having a higher minimum (14) than the non-responsive path (11 at line 173) makes sense since scaled-down columns need extra cushion to prevent truncation.
commit: |
2 participants
Summary
Fixes #700 by increasing the minimum column width for right-aligned numeric columns in responsive table mode.
Problem:
When running
ccusage monthly, large token numbers (e.g., 536,073,421) were being truncated with ellipsis (536,073,…) in narrow terminal windows due to insufficient minimum column width in responsive mode.Solution:
Increased the minimum width for right-aligned numeric columns from 10 to 14 characters in the responsive resizing logic (
packages/terminal/src/table.tsline 194). This ensures large comma-formatted numbers display in full without truncation while maintaining table responsiveness.Changes:
packages/terminal/src/table.tsline 194Math.max(adjustedWidth, 10)toMath.max(adjustedWidth, 14)Testing:
✅ All format, typecheck, and test checks passed
✅ Tested with wide terminal (120+ columns) - numbers display correctly
✅ Tested with narrow terminal (60-80 columns) - no truncation, responsive mode works
✅ Large numbers (238,430,570+) display in full without ellipsis
Test plan
pnpm run formatin packages/terminalpnpm typecheckin packages/terminalpnpm run testin packages/terminal (all 32 tests passed)pnpm run buildccusage monthlywith various terminal widths (60, 80, 120 columns)Summary by CodeRabbit