fix: resolve 4 bugs in termui - #3487
Conversation
github-actions
Bot
added
type:bug
📝 WalkthroughWalkthroughThe changes refine numeric handling in the Pomodoro timer, RSS reader, and UI prompt selection. They adjust rounding precision, bound hexadecimal entity parsing to eight characters, and replace global ChangesNumeric correctness fixes
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ 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: 2
🤖 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 percentage label calculation in the timer’s
label-rendering method to add an epsilon scaled to the percentage value after
multiplying _value by 100, so values such as 0.285 round to 29%. Add a boundary
test covering this near-half percentage case and preserve the existing
conditional label behavior.
In `@examples/rss-reader/src/index.tsx`:
- Around line 30-31: Update the entity-decoding logic around codePoint to
validate the hexadecimal digit portion before parsing: reject entities with more
than eight digits, and reject parsed values above 0x10FFFF, returning match
unchanged in both cases. Only call String.fromCodePoint for valid Unicode scalar
values while preserving the existing finite-value check.
🪄 Autofix
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: b95e24f8-700f-4d0e-bf3a-11fa183d948b
📒 Files selected for processing (3)
examples/pomodoro-timer/src/index.tsxexamples/rss-reader/src/index.tsxpackages/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
Scale the epsilon to the percentage value.
At Line 185, the calculation adds Number.EPSILON after multiplying by 100. That epsilon can be too small to change values near 28.5. For example, 0.285 * 100 can produce 28.499999999999996, so the label can still display 28% instead of 29%.
Use an epsilon scaled for the percentage range and add a boundary test.
Proposed fix
- const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : '';
+ const label = this._showLabel ? ` ${Math.round(this._value * 100 + 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 * 100 + 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 percentage
label calculation in the timer’s label-rendering method to add an epsilon scaled
to the percentage value after multiplying _value by 100, so values such as 0.285
round to 29%. Add a boundary test covering this near-half percentage case and
preserve the existing conditional label behavior.
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | ||
| return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject overlong and out-of-range entities instead of truncating them.
entity contains the complete match, but slice(2, 10) discards digits after the first eight. An input such as A is therefore decoded as 0x4 instead of remaining unchanged. Check the digit length before parsing. Also reject values above 0x10FFFF before calling String.fromCodePoint.
Proposed fix
- const codePoint = Number.parseInt(entity.slice(2, 10), 16);
- return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match;
+ const digits = entity.slice(2);
+ if (digits.length > 8) return match;
+ const codePoint = Number.parseInt(digits, 16);
+ return Number.isFinite(codePoint) && codePoint <= 0x10FFFF
+ ? String.fromCodePoint(codePoint)
+ : match;📝 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 codePoint = Number.parseInt(entity.slice(2, 10), 16); | |
| return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; | |
| const digits = entity.slice(2); | |
| if (digits.length > 8) return match; | |
| const codePoint = Number.parseInt(digits, 16); | |
| return Number.isFinite(codePoint) && codePoint <= 0x10FFFF | |
| ? String.fromCodePoint(codePoint) | |
| : match; |
🤖 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/rss-reader/src/index.tsx` around lines 30 - 31, Update the
entity-decoding logic around codePoint to validate the hexadecimal digit portion
before parsing: reject entities with more than eight digits, and reject parsed
values above 0x10FFFF, returning match unchanged in both cases. Only call
String.fromCodePoint for valid Unicode scalar values while preserving the
existing finite-value check.
1 participant
Description
This PR fixes real bugs found in the codebase:
parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.Number.EPSILONtoMath.round: prevents floating-point drift (e.g.1.005 * 100rounding to 100 instead of 101).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: #3486
Summary by CodeRabbit