fix: resolve 4 bugs in termui - #3521
Conversation
github-actions
Bot
added
type:bug
📝 WalkthroughWalkthroughThis change applies four targeted correctness fixes: interval cleanup in AI streaming, whitespace-only paragraph detection, explicit decimal key parsing, and numeric sorting in ChangesExample correctness fixes
TreeSelect value comparison
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)
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: 1
🧹 Nitpick comments (1)
examples/ai-streaming/src/index.tsx (1)
43-43: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueKeep the streaming interval per
AIStreamingAppinstance.This file only constructs a single
AIStreamingApp, so the globalwindow.__intervaldoes not cancel another mounted instance here. Store the handle on the instance instead for self-contained lifecycle management.🤖 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/ai-streaming/src/index.tsx` at line 43, Update the interval setup in AIStreamingApp to store and clear the timer handle on that component instance rather than using the global window.__interval property. Preserve the existing clear-and-restart behavior while ensuring each AIStreamingApp manages only its own streaming interval.
🤖 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/ui/src/TreeSelect.ts`:
- Around line 185-186: Update the sorting logic used by _valuesEqual so sortedA
and sortedB both use the same comparator compatible with the arrays’ value type.
Ensure equivalent string inputs such as ["2", "10"] receive identical ordering
and avoid numeric subtraction that violates TypeScript strict typing.
---
Nitpick comments:
In `@examples/ai-streaming/src/index.tsx`:
- Line 43: Update the interval setup in AIStreamingApp to store and clear the
timer handle on that component instance rather than using the global
window.__interval property. Preserve the existing clear-and-restart behavior
while ensuring each AIStreamingApp manages only its own streaming interval.
🪄 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: 73fe7ae1-9c1d-45cc-a97b-ddf8aef6fc18
📒 Files selected for processing (4)
examples/ai-streaming/src/index.tsxexamples/chat-app/src/index.tsxexamples/widget-gallery/src/index.tspackages/ui/src/TreeSelect.ts
| const sortedA = [...a].sort((a, b) => a - b); | ||
| const sortedB = [...b].sort(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate TreeSelect.ts =="
fd -a 'TreeSelect\.ts$' . || true
echo "== inspect file outline =="
TREE="$(fd 'TreeSelect\.ts$' . | head -n1 || true)"
if [ -n "${TREE:-}" ]; then
echo "TREE=$TREE"
wc -l "$TREE"
ast-grep outline "$TREE" --match '_valuesEqual' --view expanded || true
echo "== relevant lines =="
sed -n '160,200p' "$TREE" | nl -ba -v160
fi
echo "== package type script / tsconfig =="
if [ -f tsconfig.json ]; then echo "root tsconfig"; cat tsconfig.json; fi
fd -a 'tsconfig.*json$' . | while read -r f; do
echo "-- $f --"
sed -n '1,160p' "$f"
done
echo "== package deps mentioning typescript =="
rg -n '"typescript"|"type": "module"|strict|noImplicitAny|compilerOptions' package.json pnpm-lock.yaml package-lock.json yarn.lock . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | head -n 200 || trueRepository: Karanjot786/TermUI
Length of output: 490
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== behavioral probe: comparator effects for ["2","10"] =="
node - <<'JS'
const cases = {
naiveA: ["2","10"].sort((a,b) => a - b),
naiveB: ["2","10"].sort(),
numeric: ["2","10"].sort((a,b) => Number(a)-Number(b)),
};
for (const [k,v] of Object.entries(cases)) console.log(`${k}: ${JSON.stringify(v)}`);
function sortedEqual(a, b) {
const sortedA = [...a].sort((a, b) => a - b);
const sortedB = [...b].sort();
return sortedA.toString() === sortedB.toString();
}
console.log('sortedEqual(["2","10"],["2","10"]):', sortedEqual(["2","10"],["2","10"]));
JSRepository: Karanjot786/TermUI
Length of output: 310
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read-only type compatibility probe for Node 18-only without installing deps.
node - <<'JS'
// This JavaScript probe cannot exercise TypeScript strict mode.
// We document the expression only because it was requested in the original comment.
console.log('Node cannot demonstrate TypeScript strict-mode typing for string subtraction without the tsc toolchain');
JSRepository: Karanjot786/TermUI
Length of output: 259
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TREE=$(fd 'TreeSelect\.ts$' . | head -n1 || true)
echo "TREE=$TREE"
echo "== relevant lines =="
awk 'NR>=160 && NR<=191 { printf "%4d\t%s\n", NR, $0 }' "$TREE"
echo "== tsconfig files =="
fd 'tsconfig.*json$' . | sort | while IFS= read -r f; do
echo "-- $f --"
sed -n '1,180p' "$f"
done
echo "== typecheck availability =="
command -v tsc || true
command -v npx || true
if [ -x "$(command -v npx || true 2>/dev/null)" ]; then
npx --no-install tsc --version || true
fiRepository: Karanjot786/TermUI
Length of output: 17646
Use one typed comparator for both arrays.
a - b rejects string operands in TypeScript strict mode, and mixing numeric and lexicographic sorting makes _valuesEqual return false for equivalent input values like ["2", "10"]. Apply the same typed comparator to both sortedA and sortedB.
Proposed fix
- const sortedA = [...a].sort((a, b) => a - b);
- const sortedB = [...b].sort();
+ const compareValues = (left: string, right: string) =>
+ 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: the left-hand side of an arithmetic operation must be any, number, bigint, or an enum type (TS2362), and the right-hand side must meet the same requirement (TS2363). Failed command: bun run build (tsup).
🪛 GitHub Actions: CI / build-and-test
[error] 185-185: TypeScript DTS build failed: TS2362 and TS2363 indicate that the operands of an arithmetic operation at columns 43 and 47 are not typed as any, number, bigint, or an enum. The @termuijs/ui build command (tsup) exited 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 the sorting
logic used by _valuesEqual so sortedA and sortedB both use the same comparator
compatible with the arrays’ value type. Ensure equivalent string inputs such as
["2", "10"] receive identical ordering and avoid numeric subtraction that
violates TypeScript strict typing.
Source: Coding guidelines
1 participant
Description
This PR fixes real bugs found in the codebase:
trim()to''misses whitespace-only input;.trim().length === 0is explicit.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.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3520
Summary by CodeRabbit