feat(cli): Add number formatting and fix statistics calculation#177
feat(cli): Add number formatting and fix statistics calculation#177
Conversation
|
|
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve modifications to two files: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Packager
User->>CLI: Request output summary
CLI->>CLI: Call printSummary()
CLI->>CLI: Format counts using toLocaleString()
CLI->>User: Display formatted summary
User->>Packager: Trigger pack function
Packager->>Packager: Calculate totalCharacters from output
Packager->>Packager: Calculate totalTokens using tokenCounter
Packager->>User: Return pack results
Possibly related PRs
Warning Rate limit exceeded@yamadashy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 51 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (1)tests/core/packager.test.ts (1)
The new assertions for total characters (11) and total tokens (10) appear to be derived from different sources than the individual file metrics:
This suggests a change in how totals are calculated, moving from sum of file metrics to output-based calculation. Let's verify the implementation changes in the packager: ✅ Verification successfulTest assertions correctly reflect the implementation's output-based calculation The test assertions are consistent with the implementation in
This explains why:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the changes in total calculation logic
# Expected: Find changes that move from file-based to output-based calculation
# Search for totalCharacters calculation
rg -A 5 'totalCharacters.*=' src/core/packager.ts
# Search for totalTokens calculation
rg -A 5 'totalTokens.*=' src/core/packager.ts
Length of output: 462 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
yamadashy
force-pushed
the
feature/print-human-readable
branch
from
81ed6e0 to
56c13de
Compare
![standard-input[bot]](https://avatars.githubusercontent.com/in/1011034?s=60&v=4){kind=small}
![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
🧹 Outside diff range and nitpick comments (3)
src/cli/cliPrint.ts (2)
30-32: Consider adding thousands separators for better readability of large numbers.The addition of
toLocaleString()and units improves the clarity of statistics. For even better readability with very large numbers, consider adding a helper function to format numbers with appropriate unit suffixes (K, M, B).Example implementation:
function formatNumber(num: number): string { const units = ['', 'K', 'M', 'B']; const order = Math.floor(Math.log10(Math.abs(num)) / 3); if (order < 0 || order >= units.length) return num.toLocaleString(); const scaled = num / Math.pow(10, order * 3); return `${scaled.toFixed(1)}${units[order]}`; }
68-68: Consider a more descriptive variable name.While the dynamic separator line alignment is a good improvement, the variable name
topFilesLengthStrLencould be more descriptive.Consider renaming to something more clear:
- const topFilesLengthStrLen = topFilesLength.toString().length; + const digitCountInTopFilesNumber = topFilesLength.toString().length;Also applies to: 70-70
src/core/packager.ts (1)
108-110: Consider optimizing token counting performance.The current implementation counts tokens twice: once per file for individual metrics and once for the total. Consider storing the individual token counts and summing them if the output generation doesn't modify the tokens.
- const totalTokens = tokenCounter.countTokens(output); + // If output generation preserves tokens, use the sum of individual counts + const totalTokens = Object.values(fileTokenCounts).reduce((sum, count) => sum + count, 0);Note: Only apply this optimization if you can verify that the output generation process doesn't modify the tokens. If you're unsure, the current implementation is safer as it guarantees accuracy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/cli/cliPrint.ts(4 hunks)src/core/packager.ts(1 hunks)
🔇 Additional comments (3)
src/cli/cliPrint.ts (2)
18-20: LGTM! Improved number formatting for better readability.
The addition of toLocaleString() enhances the readability of large numbers in the security check message.
80-80: LGTM! Consistent formatting of statistics.
The formatting of character and token counts is consistent with other changes and improves readability.
src/core/packager.ts (1)
107-108: LGTM! Improved accuracy of character count calculation.
The change to calculate totalCharacters directly from the output string length is more accurate than summing individual file metrics, as it correctly reflects any transformations applied during output generation.
yamadashy
force-pushed
the
feature/print-human-readable
branch
from
56c13de to
875ff5c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #177 +/- ##
==========================================
+ Coverage 92.05% 92.07% +0.01%
==========================================
Files 35 35
Lines 1889 1892 +3
Branches 431 429 -2
==========================================
+ Hits 1739 1742 +3
Misses 150 150 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
1 participant
This PR improves the readability of statistics output and fixes how total character and token counts are calculated. Previously, the totals were showing the sum of individual files rather than the metrics of the final output file.