fix: resolve 4 bugs in termui - #3445
Conversation
github-actions
Bot
added
type:bug
📝 WalkthroughWalkthroughThe changes correct floating-point percentage rounding, enforce decimal parsing for widget tab keys, and apply numeric sorting to ChangesNumeric correctness
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 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)
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 calculations in the renderers at
examples/pomodoro-timer/src/index.tsx:185-185 and
examples/todo-app/src/index.ts:107-107 so the epsilon is applied before
multiplying by 100 or is scaled by 100, ensuring 0.145 rounds to 15; add a
regression test covering 0.145 in both locations.
In `@packages/ui/src/TreeSelect.ts`:
- Around line 185-186: Update _valuesEqual to define one typed comparator for
string values and pass it to both sortedA and sortedB sort calls, ensuring
strict TypeScript compatibility and identical ordering for duplicate-array
comparisons. Preserve string comparison semantics; only convert with Number if
TreeSelect values are explicitly guaranteed to be numeric strings.
🪄 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: 90e74872-9b66-498c-ba68-ff95bf7428ec
📒 Files selected for processing (4)
examples/pomodoro-timer/src/index.tsxexamples/todo-app/src/index.tsexamples/widget-gallery/src/index.tspackages/ui/src/TreeSelect.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
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- files ---\n'
git ls-files | rg 'examples/(pomodoro-timer/src/index\.tsx|todo-app/src/index\.ts)$|^examples/|^packages/|^src/' || true
printf '\n--- target snippets with line numbers ---\n'
sed -n '175,195p' examples/pomodoro-timer/src/index.tsx 2>/dev/null || true
printf '\n'
sed -n '95,115p' examples/todo-app/src/index.ts 2>/dev/null || true
printf '\n--- focused eps/round usage search ---\n'
rg -n "Number\.EPSILON|Math\.round\\(.*\\* 100|percentage|percent|pct" examples todo-app pomodoro-timer 2>/dev/null || true
printf '\n--- JavaScript rounding probe ---\n'
node - <<'JS'
const values = [0.145, 0.15, 0.245, 0.25];
for (const value of values) {
const current = Math.round(value * 100 + Number.EPSILON);
const correctedPre = Math.round((value + Number.EPSILON) * 100);
const correctedScaled = Math.round(value * 100 + Number.EPSILON * 100);
console.log(`${value} -> current=${current}, pre=${correctedPre}, scaled=${correctedScaled}`);
}
console.log('Number.EPSILON =', Number.EPSILON);
JSRepository: Karanjot786/TermUI
Length of output: 247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- files ---'
git ls-files | rg 'examples/(pomodoro-timer/src/index\.tsx|todo-app/src/index\.ts)$|^examples/|^packages/|^src/' || true
echo
echo '--- target snippets with line numbers ---'
sed -n '175,195p' examples/pomodoro-timer/src/index.tsx 2>/dev/null || true
echo
sed -n '95,115p' examples/todo-app/src/index.ts 2>/dev/null || true
echo
echo '--- focused eps/round usage search ---'
rg -n "Number\.EPSILON|Math\.round\\(.*\\* 100|percentage|percent|pct" examples todo-app pomodoro-timer 2>/dev/null || true
echo
echo '--- JavaScript rounding probe ---'
node - <<'JS'
const values = [0.145, 0.15, 0.245, 0.25];
for (const value of values) {
const current = Math.round(value * 100 + Number.EPSILON);
const correctedPre = Math.round((value + Number.EPSILON) * 100);
const correctedScaled = Math.round(value * 100 + Number.EPSILON * 100);
console.log(`${value} -> current=${current}, pre=${correctedPre}, scaled=${correctedScaled}`);
}
console.log('Number.EPSILON =', Number.EPSILON);
JSRepository: Karanjot786/TermUI
Length of output: 48938
Scale the epsilon before percentage conversion in both renderers.
Adding Number.EPSILON after multiplying by 100 does not reliably fix half-percent boundaries. For example, 0.145 * 100 + Number.EPSILON still rounds to 14. Move the epsilon before * 100, or scale it by 100, and add a 0.145 regression test in both locations.
📍 Affects 2 files
examples/pomodoro-timer/src/index.tsx#L185-L185(this comment)examples/todo-app/src/index.ts#L107-L107
🤖 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 calculations in the renderers at
examples/pomodoro-timer/src/index.tsx:185-185 and
examples/todo-app/src/index.ts:107-107 so the epsilon is applied before
multiplying by 100 or is scaled by 100, ensuring 0.145 rounds to 15; add a
regression test covering 0.145 in both locations.
| const sortedA = [...a].sort((a, b) => a - b); | ||
| const sortedB = [...b].sort(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
npx --no-install tsc --noEmitRepository: Karanjot786/TermUI
Length of output: 5708
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Files:"
git ls-files packages/ui/src | rg 'TreeSelect|package\.json|tsconfig' || true
echo
echo "TreeSelect outline:"
if [ -f packages/ui/src/TreeSelect.ts ]; then
ast-grep outline packages/ui/src/TreeSelect.ts --view compact || true
wc -l packages/ui/src/TreeSelect.ts
sed -n '1,240p' packages/ui/src/TreeSelect.ts
fi
echo
echo "Relevant compiler/config references:"
fd -a 'tsconfig.*|package.json' . | sed 's#^\./##' | rg '(^|/)(tsconfig.*|package\.json)$' | head -c 8000
for f in $(git ls-files | rg '(^|/)tsconfig.*\.json$|(^|/)package\.json$'); do
if rg -n '"strict"|"tsconfigPaths"|"compilerOptions"|"compilerOptions":{"strict"' "$f" >/dev/null; then
echo "--- $f ---"
sed -n '1,160p' "$f"
fi
doneRepository: Karanjot786/TermUI
Length of output: 35956
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Files:"
git ls-files packages/ui/src | rg 'TreeSelect|package\.json|tsconfig' || true
echo
echo "TreeSelect outline:"
if [ -f packages/ui/src/TreeSelect.ts ]; then
ast-grep outline packages/ui/src/TreeSelect.ts --view compact || true
wc -l packages/ui/src/TreeSelect.ts
sed -n '1,260p' packages/ui/src/TreeSelect.ts
fi
echo
echo "Relevant compiler/config references:"
for f in $(git ls-files | rg '(^|/)tsconfig[^/]*\.json$|(^|/)package\.json$'); do
if rg -n '"strict"|"compilerOptions"|"extends"|"include"|"files"' "$f" >/dev/null; then
echo "--- $f ---"
sed -n '1,220p' "$f"
fi
done
echo
echo "Package scripts:"
for f in package.json packages/*/package.json; do
[ -f "$f" ] || continue
if rg -n '"scripts"|"typecheck"|"tsc"' "$f" >/dev/null; then
echo "--- $f ---"
jq -r '.scripts // empty | to_entries[]? | "\(.key)=\(.value)"' "$f"
fi
done
echo
echo "TypeScript availability/version:"
command -v tsc || true
if command -v npx >/dev/null; then
npx --yes --package=typescript@latest tsc --version
fiRepository: Karanjot786/TermUI
Length of output: 44939
🌐 Web query:
TypeScript arithmetic operator string operands strict mode subtract strings error
💡 Result:
In TypeScript, using arithmetic operators (such as -, *, /) with strings will result in a compilation error [1][2][3]. The specific error you will encounter is: "The left-hand side of an arithmetic operation must be of type 'any', 'number', 'bigint' or an enum type" (or a similar message regarding the right-hand side) [1][2][4]. Unlike the addition operator (+), which TypeScript allows with strings because it performs string concatenation [5][6][7], other arithmetic operators are reserved for numeric types (including number, bigint, any, and numeric enums) [1][8][9]. This restriction is in place to prevent unintended implicit type coercion that can lead to runtime bugs [1][9]. To resolve this error, you must explicitly convert your string operands to numbers before performing the arithmetic operation [2][4][3]. Common methods include: * Using Number(value) or parseFloat(value)/parseInt(value) [10]. * Using the unary plus (+) operator, such as +stringVar1 - +stringVar2 [10]. If you are attempting to sort arrays of strings, you should use localeCompare instead of subtraction [10]. ### Summary of Differences * + Operator: Allowed with strings because it supports concatenation [5][6]. * -, *, / Operators: Disallowed with strings; attempting to use them will trigger a TypeScript compilation error [1][8][9].
Citations:
- 1: https://docs.syntblaze.com/typescript/arithmetic-operators/subtraction
- 2: https://bobbyhadz.com/blog/typescript-left-hand-side-of-arithmetic-operation-must-be-type
- 3: https://ngoclb.com/2023/03/typescript-the-left-hand-side-of-an-arithmetic-operation-must-be-type-any-number-bigint-or-an-enum-type/
- 4: https://thewebdev.info/2022/03/19/how-to-fix-the-the-left-hand-and-right-hand-side-of-an-arithmetic-operation-must-be-of-type-any-number-or-an-enum-type-error-with-typescript/
- 5: no error when using
+operator on incompatible types microsoft/TypeScript#48962 - 6: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Addition
- 7: https://stackoverflow.com/questions/52101977/why-typescript-lets-me-add-a-string-and-a-number-can-i-prevent-it
- 8: https://reflectoring.io/typescript-operators/
- 9: https://www.tektutorialshub.com/typescript/arithmetic-operators-in-typescript/
- 10: https://stackoverflow.com/questions/69228914/how-to-sort-strings-in-typescript-if-subtraction-cannot-be-performed-on-two-stri
🌐 Web query:
site:github.com/microsoft/TypeScript String.prototype.sort comparator undefined undefined localeCompare sort behavior source
💡 Result:
There is no String.prototype.sort method in JavaScript [1][2]. The sort method is a property of Array.prototype (and TypedArray.prototype) [3][4][5]. Regarding the behavior of Array.prototype.sort: 1. Default Sorting: If no comparator function is provided, the sort method converts all array elements into strings and compares their sequences of UTF-16 code unit values [3][4][5]. This often leads to unexpected results when sorting numbers, as "10" will be sorted before "2" [3][6][7]. 2. Handling of undefined values: When sorting an array that contains undefined values, the ECMAScript specification dictates that all undefined elements are moved to the end of the array [8]. The compare function, if provided, is never called with undefined values [8]. 3. localeCompare: The String.prototype.localeCompare method is commonly used within a custom comparator function to handle language-sensitive string sorting [4]. For example: array.sort((a, b) => a.localeCompare(b)); This approach ensures that strings are sorted according to specific locale rules rather than raw UTF-16 code unit values [4].
Citations:
- 1: Can not extend String interface microsoft/TypeScript#55931
- 2: Unsupported wrapper object microsoft/TypeScript#54059
- 3: Array<number>.sort() uses lexicographic comparator by default microsoft/TypeScript#32593
- 4: Incorrect JSDoc description for
Array#sortdefault algorithm "ASCII character order" (actually UTF-16 code unit order) microsoft/TypeScript#60978 - 5: Array of numbers wrong sort microsoft/TypeScript#58160
- 6: Warn when Array.sort is used without a callback on non-string arrays microsoft/TypeScript#35551
- 7: Better Array.sort compareFn docs microsoft/TypeScript#33372
- 8: Suggestion: Array.prototype.sort - remove the need for compareFn to handle undefined values microsoft/TypeScript#41708
Use one typed comparator for both arrays.
_valuesEqual receives string[], so a - b is invalid under TypeScript strict mode. The second sort() also uses a different string sort order than localCompare, so duplicate value arrays can compare as unequal.
Define one comparator with a typed parameter, and pass it to both sort calls. Use Number(...) only if TreeSelect values are guaranteed numeric strings.
Proposed fix
- const sortedA = [...a].sort((a, b) => a - b);
- const sortedB = [...b].sort();
+ const compareValues = (left: string, right: string): number =>
+ Number(left) - Number(right);
+ const sortedA = [...a].sort(compareValues);
+ const sortedB = [...b].sort(compareValues);🧰 Tools
🪛 GitHub Actions: CI / 0_build-and-test.txt
[error] 185-185: TypeScript DTS build failed: TS2362 and TS2363 indicate that the left- and right-hand operands of an arithmetic operation are not typed as any, number, bigint, or an enum. The '@termuijs/ui' 'tsup' build exited with code 1.
🪛 GitHub Actions: CI / build-and-test
[error] 185-185: TypeScript errors TS2362 and TS2363: arithmetic operands at columns 43 and 47 are not typed as any, number, bigint, or an enum. The tsup DTS build failed, causing 'bun run build' to exit with code 1.
🤖 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` around lines 185 - 186, Update _valuesEqual to
define one typed comparator for string values and pass it to both sortedA and
sortedB sort calls, ensuring strict TypeScript compatibility and identical
ordering for duplicate-array comparisons. Preserve string comparison semantics;
only convert with Number if TreeSelect values are explicitly guaranteed to be
numeric strings.
Source: Coding guidelines
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)..sort()coerces elements to strings, so[10, 9, 2]sorts as[10, 2, 9]; numeric comparator sorts correctly.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: #3444
Summary by CodeRabbit