Repository navigation
fix(cli): pick the duration unit from the rounded value - #29651
artemkulyk wants to merge 1 commit into
Conversation
Summary of ChangesHello, 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 an issue where duration formatting incorrectly rounded values near unit boundaries, leading to misleading output like '1000ms' or '60.0s'. By ensuring rounding occurs before unit selection, the utility now correctly handles fractional values and provides more accurate time representations. 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 the 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 counterproductive. 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. Footnotes
|
|
📊 PR Size: size/S
|
There was a problem hiding this comment.
Code Review
This pull request updates the formatDuration utility in packages/cli/src/ui/utils/formatters.ts to round duration values to their displayed precision before selecting the appropriate unit. This prevents values just below a unit boundary from incorrectly rounding up into the next unit's range. Additionally, corresponding unit tests have been added in formatters.test.ts to verify rounding behavior for milliseconds, seconds, and hours. There are no review comments, and I have no feedback to provide.
Summary
formatDuration()picked the unit from the raw value and rounded afterwards, sovalues just under a boundary printed in the wrong unit:
999.5mscame out as1000msand59950msas60.0s. Both are visible in/stats, where theaverage latency is usually fractional.
Details
Round to each unit's display precision first (whole ms, tenths of a second,
whole seconds), then pick the unit from the rounded value. Values away from a
boundary are unchanged.
Related Issues
Fixes #29600
How to Validate
npx vitest run src/ui/utils/formatters.test.tsinpackages/clipasses (35tests).
npm run preflightis green here apart from failures that reproduce ona clean checkout too:
gemini.test.tsxtrust errors in headless runs (see#29634) and two fs/PTY tests.
Pre-Merge Checklist