fix: resolve 4 bugs in termui - #3659
Conversation
📝 WalkthroughWalkthroughThe changes update streaming interval cleanup, alter dev-server reload rejection handling, add epsilon-adjusted Switch rounding, and use numeric sorting in TreeSelect value comparison. ChangesRuntime and UI fixes
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🧪 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: 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 `@packages/dev-server/src/server.ts`:
- Line 383: Fix the invalid rejection handler in the exitedPromise await flow by
declaring the caught error parameter and passing it to console.error. Preserve
the existing behavior of logging promise rejections.
In `@packages/ui/src/TreeSelect.ts`:
- Line 185: The _valuesEqual comparison in TreeSelect.ts uses a numeric
comparator on string arrays and sorts the two arrays inconsistently. Replace the
sortedA comparator with a type-safe string comparator and apply that same
comparator to sortedB, preserving equality for reordered multi-selections; add
coverage for reordered values such as “10” and “2”.
🪄 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: b83cd279-862f-42e5-8622-fc38635a6175
📒 Files selected for processing (4)
examples/ai-streaming/src/index.tsxpackages/dev-server/src/server.tspackages/ui/src/Switch.tspackages/ui/src/TreeSelect.ts
| this._killChild(); | ||
|
|
||
| await exitedPromise.catch(() => {}); | ||
| await exitedPromise.catch( => console.error()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if rg -n '\.catch\(\s*=>' packages/dev-server/src/server.ts; then
echo "Malformed catch callback found"
exit 1
fi
rg -n 'exitedPromise\.catch\(' packages/dev-server/src/server.tsRepository: Karanjot786/TermUI
Length of output: 251
Fix the invalid catch callback before merge.
Line 383 uses catch( => ...), which is invalid TypeScript and prevents packages/dev-server/src/server.ts from parsing. Pass the rejection to console.error:
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 invalid rejection
handler in the exitedPromise await flow by declaring the caught error parameter
and passing it to console.error. Preserve the existing behavior of logging
promise rejections.
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:
#!/bin/bash
set -euo pipefail
config="$(fd -H -t f '^tsconfig.*\.json$' packages | head -n 1)"
test -n "$config"
npx tsc --noEmit --project "$config"
rg -n -C 4 '_valuesEqual|sort\(' packages/ui/src/TreeSelect.ts
rg -n -C 3 "value: 's'|value: 'a'|selectedValues" packages/ui/src/TreeSelect.test.tsRepository: Karanjot786/TermUI
Length of output: 8776
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
fd -t f 'TreeSelect\.(ts|test\.ts)$|^tsconfig.*\.json$' .
echo
echo "TreeSelect section:"
sed -n '170,195p' packages/ui/src/TreeSelect.ts
echo
echo "TreeSelect test relevant section:"
rg -n -C 4 "_valuesEqual|valuesEqual|value: '(s|a)'|selectedValues|valuesEqual|compare" packages/ui/src packages -g '*.ts' || true
echo
echo "TS configs with relevant compiler options:"
for f in $(fd -t f '^tsconfig.*\.json$' packages'); do
echo "--- $f ---"
sed -n '1,220p' "$f"
done
echo
echo "TypeScript availability and TS error for string subtraction:"
if command -v npx >/dev/null 2>&1 && command -v tsc >/dev/null 2>&1; then
(cd packages/ui && npx tsc --noEmit -p tsconfig.json 2>&1 | rg 'TreeSelect|_valuesEqual|Operator "-" cannot be applied|TS2362|TS2363|TS2769' || true)
node -e "console.log('10' - '2', 's' - 'a')" 2>&1 || true
else
echo "tsc not globally available"
node -e "console.log('10' - '2', 's' - 'a')" 2>&1 || true
fiRepository: Karanjot786/TermUI
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "UI tsconfig files:"
for f in packages/ui/tsconfig.json packages/ui/tsconfig.typecheck.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,220p' "$f"
fi
done
echo
echo "Base tsconfig files:"
for f in packages/tsconfig.base.json tsconfig.base.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,220p' "$f"
fi
done
echo
echo "Relevant tsconfig snippets with compiler options:"
python3 - <<'PY'
import json, pathlib
for path in [pathlib.Path('packages/ui/tsconfig.json'), pathlib.Path('packages/ui/tsconfig.typecheck.json'), pathlib.Path('tsconfig.base.json')]:
if path.exists():
data = json.loads(path.read_text())
print(f'--- {path} ---')
print(json.dumps({k:v for k,v in data.items() if k in {'compilerOptions', 'extends'}} or data.get('compilerOptions'), indent=2))
PY
echo
echo "Read-only static check of _valuesEqual comparator semantics and TS string subtraction shape:"
python3 - <<'PY'
from pathlib import Path
import re
p = Path('packages/ui/src/TreeSelect.ts')
s = p.read_text()
m = re.search(r'function _valuesEqual\(a: string\[\], b: string\[\]\): boolean \{(?P<body>.*?)\n\}', s, re.S)
print('has _valuesEqual:', bool(m))
if m:
body = m.group('body')
print('lines:')
print('\n'.join(f'{i:4d}: {line}' for i,line in enumerate(s.splitlines()[183:189], 183)))
print('sortedA uses subtraction:', bool(re.search(r'\.sort\(\(a, b\)\s*=>\s*a\s*-\s*b\)', body)))
print('sortedB uses comparison:', bool(re.search(r'\.sort\(\)\s*;', body)))
print('imports NodeFS (for later TS type verification context):')
print('node:fs' in s[:500])
PY
node - <<'JS'
function valuesEqualRuntime(a, b) {
if (a.length !== b.length) return false;
const sortedA = [...a].sort((a, b) => a - b);
const sortedB = [...b].sort();
return sortedA.every((v, i) => v === sortedB[i]);
}
console.log(JSON.stringify({
['10','2'] vs ['2','10']: valuesEqualRuntime(['10','2'], ['2','10']),
['a','s'] vs ['s','a']: valuesEqualRuntime(['a','s'], ['s','a']),
sortedNumericStrings: JSON.stringify([...['10','2']].sort((a,b) => a - b)),
sortedLexicographic: JSON.stringify([...['10','2']].sort())
}));
JS
echo
echo "Check whether package script config allows typecheck without full install:"
for f in package.json packages/ui/package.json; do
[ -f "$f" ] && { echo "--- $f ---"; sed -n '1,220p' "$f"; }
doneRepository: Karanjot786/TermUI
Length of output: 2463
Use one type-safe comparator for both sorted value arrays.
_valuesEqual compares string[], and a - b is a TypeScript error under packages/ui/tsconfig.typecheck.json. It also produces non-numeric comparison values, while sortedB uses lexicographic sorting, so reordered numeric-like strings such as ['10', '2'] and ['2', '10'] may be reported as changed. Sort both arrays with the same comparator and add a test for re-ordered multi selections.
🤖 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, The _valuesEqual comparison in
TreeSelect.ts uses a numeric comparator on string arrays and sorts the two
arrays inconsistently. Replace the sortedA comparator with a type-safe string
comparator and apply that same comparator to sortedB, preserving equality for
reordered multi-selections; add coverage for reordered values such as “10” and
“2”.
Source: Coding guidelines
github-actions
Bot
added
type:bug
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)..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: #3658
Summary by CodeRabbit