fix: resolve 4 bugs in termui - #3684
Conversation
github-actions
Bot
added
type:bug
📝 WalkthroughWalkthroughThe changes apply four numeric correctness fixes: bounded hexadecimal entity parsing, stable percentage rounding, explicit decimal key parsing, and numeric sorting of selected option indices. ChangesNumeric correctness fixes
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 |
![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/rss-reader/src/index.tsx`:
- Around line 30-31: Update the entity-decoding logic around codePoint to parse
the captured hexadecimal substring rather than a fixed slice, reject empty
values before calling Number.parseInt, and continue returning the original match
for invalid or non-finite values while correctly decoding 1–8 hex digits such as
&`#x10FFFF`;.
In `@examples/todo-app/src/index.ts`:
- Line 107: Update the percentage calculation in the progress-rendering logic
around pct so Number.EPSILON is multiplied by the same 100 scale as value before
Math.round. Preserve the existing conversion to an integer percentage while
ensuring boundary values such as 0.145 round to 15%.
🪄 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: 1bdb8051-8291-44e7-ac7c-baadd2da6f34
📒 Files selected for processing (4)
examples/rss-reader/src/index.tsxexamples/todo-app/src/index.tsexamples/widget-gallery/src/index.tspackages/ui/src/MultiSelect.ts
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | ||
| return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node <<'NODE'
const codePoint = Number.parseInt('FFFFFFFF'.slice(0, 8), 16);
try {
String.fromCodePoint(codePoint);
process.exit(1);
} catch (error) {
if (!(error instanceof RangeError)) throw error;
}
NODERepository: Karanjot786/TermUI
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target file and nearby entity-decoding logic.
if [ -f examples/rss-reader/src/index.tsx ]; then
wc -l examples/rss-reader/src/index.tsx
sed -n '1,80p' examples/rss-reader/src/index.tsx
else
echo "missing examples/rss-reader/src/index.tsx"
git ls-files | rg 'examples/rss-reader/src/index\.tsx|rss-reader' || true
fi
# Behavioral probe for Number.parseInt/Number.isFinite/String.fromCodePoint without executing repo code.
node <<'NODE'
const inputs = ['&`#xFFFFFFFF`;', '&`#x10FFFF00`;', '&`#x10FFFF`;', '&`#x110000`;', '&`#x012345678ABC`;'];
for (const entity of inputs) {
const matched = entity.match(/^&`#x`([0-9A-Fa-f]+);$/);
const codePoint = Number.parseInt(entity.slice(2, 10), 16);
let s;
let errClass;
try {
s = String.fromCodePoint(codePoint);
} catch (error) {
errClass = error.constructor.name;
}
console.log(JSON.stringify({
entity,
matched: matched && matched[1],
parsedSlice8: codePoint,
finite: Number.isFinite(codePoint),
exceedsMaxPlane: matched && matched[1] ? Number.parseInt(matched[1], 16) > 0x10FFFF : null,
parsedFull: matched ? Number.parseInt(matched[1], 16) : null,
finiteFull: matched ? Number.isFinite(Number.parseInt(matched[1], 16)) : null,
string: s,
errorClass: errClass,
}), null, 2);
}
NODERepository: Karanjot786/TermUI
Length of output: 3382
Reject invalid hexadecimal entities before decoding.
Hexadecimal entities that do not match exactly 1–8 hex digits cause Number.isFinite(...) to return false, so valid inputs such as  fall through to the replacement text. Use the captured hex substring as the parsed value and reject empty values before calling Number.parseInt.
🤖 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 parse the captured hexadecimal
substring rather than a fixed slice, reject empty values before calling
Number.parseInt, and continue returning the original match for invalid or
non-finite values while correctly decoding 1–8 hex digits such as &`#x10FFFF`;.
| const filled = Math.round(barWidth * value); | ||
|
|
||
| const pct = Math.round(value * 100); | ||
| const pct = Math.round(value * 100 + Number.EPSILON); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node <<'NODE'
const value = 0.145;
const current = Math.round(value * 100 + Number.EPSILON);
if (current !== 15) {
process.exit(1);
}
NODERepository: Karanjot786/TermUI
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n examples/todo-app/src/index.ts | sed -n '90,115p'
node --version
node - <<'NODE'
console.log({
value: 0.145,
product: 0.145 * 100,
current: Math.round(0.145 * 100 + Number.EPSILON),
beforeScale: Math.round((0.145 + Number.EPSILON) * 100),
scaledEpsilon: Math.round(0.145 * 100 + Number.EPSILON * 100),
});
NODERepository: Karanjot786/TermUI
Length of output: 1605
Scale the epsilon before rounding the percentage.
Adding Number.EPSILON after value * 100 leaves boundary values like 0.145 * 100 at 14.499999999999998, and Math.round(... + Number.EPSILON) rounds them to 14% instead of 15%. Scale the epsilon with the percentage instead.
Proposed fix
- const pct = Math.round(value * 100 + Number.EPSILON);
+ const pct = Math.round(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 pct = Math.round(value * 100 + Number.EPSILON); | |
| const pct = Math.round(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/todo-app/src/index.ts` at line 107, Update the percentage
calculation in the progress-rendering logic around pct so Number.EPSILON is
multiplied by the same 100 scale as value before Math.round. Preserve the
existing conversion to an integer percentage while ensuring boundary values such
as 0.145 round to 15%.
1 participant
Description
This PR fixes real bugs found in the codebase:
parseInt: without10, strings like'0x1F'or'08'parse in unintended bases..sort()coerces elements to strings, so[10, 9, 2]sorts as[10, 2, 9]; numeric comparator sorts correctly.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).Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3683
Summary by CodeRabbit