fix: resolve 4 bugs in termui - #3462
Conversation
📝 WalkthroughWalkthroughThe changes improve numeric handling in the Pomodoro progress label, widget gallery tab selection, and UI prompt selection validation. ChangesNumeric correctness corrections
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/pomodoro-timer/src/index.tsx`:
- Line 185: Update the label calculation in the relevant timer rendering method
so the epsilon is added to this._value before multiplying by 100, ensuring
boundary values round correctly; retain the existing conditional label behavior
and add a regression test covering a value such as 0.145 that must display 15%.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: deee3ef8-9498-4d84-859c-32903b46f127
📒 Files selected for processing (3)
examples/pomodoro-timer/src/index.tsxexamples/widget-gallery/src/index.tspackages/ui/src/prompts.ts
| const attrs = styleToCellAttrs(this._style); | ||
|
|
||
| const label = this._showLabel ? ` ${Math.round(this._value * 100)}%` : ''; | ||
| const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : ''; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Apply the epsilon before scaling.
Number.EPSILON is too small after this._value * 100. For example, 0.145 * 100 can produce 14.499999999999998, and adding Number.EPSILON does not reliably move it to 14.5. The label can still display 14% instead of 15%.
Use a scale-aware expression and add a boundary regression test.
Proposed fix
- const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : '';
+ const label = this._showLabel ? ` ${Math.round((this._value + Number.EPSILON) * 100)}%` : '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : ''; | |
| const label = this._showLabel ? ` ${Math.round((this._value + Number.EPSILON) * 100)}%` : ''; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/pomodoro-timer/src/index.tsx` at line 185, Update the label
calculation in the relevant timer rendering method so the epsilon is added to
this._value before multiplying by 100, ensuring boundary values round correctly;
retain the existing conditional label behavior and add a regression test
covering a value such as 0.145 that must display 15%.
Description
This PR fixes real bugs found in the codebase:
Number.EPSILONtoMath.round: prevents floating-point drift (e.g.1.005 * 100rounding to 100 instead of 101).parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.x === trueis equivalent tox(andx === falseto!x), and shorter to read.isNaNwithNumber.isNaN: the global version coerces its argument, soisNaN('1')returns false whileNumber.isNaNis strict.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3461
Summary by CodeRabbit