fix: resolve 4 bugs in termui - #3651
Conversation
github-actions
Bot
added
area:examples
📝 WalkthroughWalkthroughThe pull request updates floating-point rounding in the todo example and switch component, changes tree-selection array sorting to numeric order, and adds error logging to the development server reload flow. ChangesBehavior updates
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 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.6)packages/dev-server/src/server.tsFile contains syntax errors that prevent linting: Line 383: Expected a parenthesis '(' but instead found '=>'. 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: 3
🤖 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/todo-app/src/index.ts`:
- Line 107: Update the percentage calculation at the pct assignment to add
Number.EPSILON to value before multiplying by 100, then round the scaled result.
Preserve the existing percentage conversion while ensuring half-percent
boundaries such as 0.285 round correctly.
In `@packages/dev-server/src/server.ts`:
- Line 383: Fix the malformed catch callback in the exitedPromise handling by
declaring its rejection parameter and passing that value to console.error,
preserving visibility of reload errors.
In `@packages/ui/src/TreeSelect.ts`:
- Line 185: Update the array equality logic in TreeSelect so it no longer
subtracts string values or relies on mixed sorting behavior. In the comparison
around the sortedA/sortedB checks, switch to a string-safe approach that matches
the _selected Set<string> semantics, such as comparing Set membership directly
or applying the same typed string comparator to both arrays. Ensure both sides
are normalized the same way before comparing so identical string lists cannot
differ by index order.
🪄 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: 2b8bf8cb-fc12-48e5-9102-30d0b2b100ed
📒 Files selected for processing (4)
examples/todo-app/src/index.tspackages/dev-server/src/server.tspackages/ui/src/Switch.tspackages/ui/src/TreeSelect.ts
| 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:
#!/usr/bin/env bash
set -euo pipefail
node <<'NODE'
const current = Math.round(0.285 * 100 + Number.EPSILON);
const corrected = Math.round((0.285 + Number.EPSILON) * 100);
if (current !== 28 || corrected !== 29) {
throw new Error(`current=${current}, corrected=${corrected}`);
}
NODERepository: Karanjot786/TermUI
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node <<'NODE'
const value = 0.285;
const currentPct = Math.round(value * 100 + Number.EPSILON);
const correctedPct = Math.round((value + Number.EPSILON) * 100);
console.log('currentPct=', currentPct);
console.log('correctedPct=', correctedPct);
console.log('EPSILON=', Number.EPSILON);
console.log('value*100=', value * 100);
console.log('current expression near integer boundary=', currentPct + 0.0000000000000001 - (value *100 + Number.EPSILON));
NODERepository: Karanjot786/TermUI
Length of output: 312
Apply epsilon before scaling.
Line 107 adds Number.EPSILON after multiplying by 100. Number.EPSILON is too small at this magnitude to correct every half-percent boundary, so values like 0.285 can round to 28 instead of 29. Add epsilon before scaling.
🤖 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 at the pct assignment to add Number.EPSILON to value before
multiplying by 100, then round the scaled result. Preserve the existing
percentage conversion while ensuring half-percent boundaries such as 0.285 round
correctly.
| this._killChild(); | ||
|
|
||
| await exitedPromise.catch(() => {}); | ||
| await exitedPromise.catch( => console.error()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix the malformed catch callback before merge.
Line 383 is invalid TypeScript because the arrow function has no parameter list. Pass the rejection value to console.error so the reload error remains visible.
Proposed fix
- await exitedPromise.catch( => console.error());
+ await exitedPromise.catch((error) => console.error(error));📝 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.
| await exitedPromise.catch( => console.error()); | |
| await exitedPromise.catch((error) => console.error(error)); |
🧰 Tools
🪛 Biome (2.5.6)
[error] 383-383: Expected a parenthesis '(' but instead found '=>'.
(parse)
🤖 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 `@packages/dev-server/src/server.ts` at line 383, Fix the malformed catch
callback in the exitedPromise handling by declaring its rejection parameter and
passing that value to console.error, preserving visibility of reload errors.
Source: Linters/SAST tools
| function _valuesEqual(a: string[], b: string[]): boolean { | ||
| if (a.length !== b.length) return false; | ||
| const sortedA = [...a].sort(); | ||
| const sortedA = [...a].sort((a, b) => a - b); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
if rg -n 'sort\(\(a, b\) => a - b\)' packages/ui/src/TreeSelect.ts; then
echo "Invalid string-subtraction comparator remains." >&2
exit 1
fiRepository: Karanjot786/TermUI
Length of output: 257
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- TreeSelect relevant lines ---\n'
sed -n '160,195p' packages/ui/src/TreeSelect.ts
printf '\n--- TreeSelect test string values ---\n'
sed -n '1,120p' packages/ui/src/TreeSelect.test.ts
printf '\n--- TypeScript compiler/type presence ---\n'
if [ -f package.json ]; then cat package.json | sed -n '1,220p'; fi
node - <<'JS'
const fs = require('node:fs');
for (const name of ['package.json','tsconfig.json','packages/ui/tsconfig.json','packages/tsconfig.json']) {
if (fs.existsSync(name)) console.log(name, fs.readFileSync(name,'utf8'));
}
JSRepository: Karanjot786/TermUI
Length of output: 247
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- TreeSelect relevant lines ---'
sed -n '160,195p' packages/ui/src/TreeSelect.ts
printf '%s\n' ''
printf '%s\n' '--- TreeSelect test string values ---'
sed -n '1,120p' packages/ui/src/TreeSelect.test.ts
printf '%s\n' ''
printf '%s\n' '--- TreeSelect type/status definitions ---'
sed -n '1,120p' packages/ui/src/TreeSelect.ts
printf '%s\n' ''
printf '%s\n' '--- TypeScript compiler/type presence ---'
node - <<'JS'
const fs = require('node:fs');
const paths = ['package.json','tsconfig.json','packages/ui/tsconfig.json','packages/tsconfig.json'];
for (const name of paths) {
if (fs.existsSync(name)) {
console.log(`--- ${name} ---`);
console.log(fs.readFileSync(name, 'utf8'));
}
}
JS
printf '%s\n' '--- Search TreeSelect equality comparator context ---'
rg -n 'selectedHasAll|sortedA|sortedB|Set|selected' packages/ui/src/TreeSelect.tsRepository: Karanjot786/TermUI
Length of output: 11434
🏁 Script executed:
#!/usr/bin/env bash
set -u
# Use read-only checks and print only the relevant lines under each section.
printf '%s\n' '--- _valuesEqual implementation ---'
sed -n '174,192p' packages/ui/src/TreeSelect.ts
printf '%s\n' '--- _valuesEqual callers ---'
rg -n '_valuesEqual|selectedValues|toggleSelection' packages/ui/src/TreeSelect.ts
printf '%s\n' '--- behavioral probe for current comparator ---'
node - <<'JS'
function sortedComparison(a, b) {
const sortedA = [...a].sort((a, b) => a - b);
const sortedB = [...b].sort();
return sortedA.map((v, i) => ({ sortedA: v, sortedB: sortedB[i], equalAt: v === sortedB[i] }));
}
console.log(JSON.stringify({
subtractionValuesForStrings: ['s', 'a', 'l'].map((a, i) => ({ a, b: i < 2 ? ['s','a','l'][i + 1] : undefined, sub: a - (i < 2 ? ['s','a','l'][i + 1] : 'undefined') })),
stringArrays: ['s','a','b','l'],
result: sortedComparison(['s','a','b','l'], ['s','a','b','l'])
}, null, 2));
JSRepository: Karanjot786/TermUI
Length of output: 1733
Use a string-safe equality strategy for both arrays.
Line 185 subtracts string values, which TypeScript strict mode rejects. If the code runs under coercion, nonnumeric values such as 's' and 'a' produce NaN. sortedB also uses default lexicographic sorting, so identical value arrays can have different index order.
Use Set membership comparison because _selected is typed as Set<string>, or apply the same typed comparator to both arrays.
🤖 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 `@packages/ui/src/TreeSelect.ts` at line 185, Update the array equality logic
in TreeSelect so it no longer subtracts string values or relies on mixed sorting
behavior. In the comparison around the sortedA/sortedB checks, switch to a
string-safe approach that matches the _selected Set<string> semantics, such as
comparing Set membership directly or applying the same typed string comparator
to both arrays. Ensure both sides are normalized the same way before comparing
so identical string lists cannot differ by index order.
Source: Coding guidelines
1 participant
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).Number.EPSILONtoMath.round: prevents floating-point drift (e.g.1.005 * 100rounding to 100 instead of 101)..sort()coerces elements to strings, so[10, 9, 2]sorts as[10, 2, 9]; numeric comparator sorts correctly.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3650
Summary by CodeRabbit