fix: resolve 2 bugs - #3373
Conversation
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes bound numeric HTML entity parsing, add explicit decimal radix arguments to tab-key parsing, and replace default sorting with numeric comparators in tree selection and dependency collection. ChangesParsing and ordering behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
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-35: Update the entity-decoding logic around the codePoint
parsing and replacement branches to validate the full numeric entity before
calling String.fromCodePoint: require 1–8 digits, ensure the value is finite and
within 0..0x10FFFF, and return the original match for invalid or oversized
entities. Apply the same validation to both hexadecimal and decimal entities,
preventing truncated prefixes from being decoded.
In `@packages/ui/src/TreeSelect.ts`:
- Around line 185-186: Replace the subtraction comparators in TreeSelect’s
sortedA and sortedB sorting with String.prototype.localeCompare to preserve
string ordering for all values. Also update the sorting expression at
scripts/build-registry.ts line 47 to use the default string sort, as no numeric
comparator is appropriate there.
🪄 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: 4277ad23-1519-40e9-a669-941011a774b2
📒 Files selected for processing (5)
examples/rss-reader/src/index.tsxexamples/showcase/src/index.tsxexamples/widget-gallery/src/index.tspackages/ui/src/TreeSelect.tsscripts/build-registry.ts
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | ||
| return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; | ||
| } | ||
|
|
||
| if (entity.startsWith('#')) { | ||
| const codePoint = Number.parseInt(entity.slice(1), 10); | ||
| const codePoint = Number.parseInt(entity.slice(1, 10), 10); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node -e 'for (const value of [0xFFFFFFFF, 99999999]) { try { String.fromCodePoint(value); process.exit(1); } catch {} }'Repository: Karanjot786/TermUI
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching index.tsx:\n'
fd -a 'index\.tsx$' . | sed 's#^\./##'
printf '\nContext around examples/rss-reader/src/index.tsx lines 1-80:\n'
if [ -f examples/rss-reader/src/index.tsx ]; then
nl -ba examples/rss-reader/src/index.tsx | sed -n '1,80p'
else
echo 'examples/rss-reader/src/index.tsx not found'
fi
printf '\nSearch for entity decoding functions/usages:\n'
rg -n "fromCodePoint|parseEntity|entity|Number\.parseInt|slice\(1, 10\)|slice\(2, 10\)" examples/rss-reader/src/index.tsx .Repository: Karanjot786/TermUI
Length of output: 2185
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'File size:\n'
wc -l examples/rss-reader/src/index.tsx 2>/dev/null || true
printf '\nContext around examples/rss-reader/src/index.tsx lines 1-80:\n'
sed -n '1,80p' examples/rss-reader/src/index.tsx
printf '\nSearch for entity decoding terms in repository:\n'
rg -n "fromCodePoint|Number\.parseInt|slice\(1, 10\)|slice\(2, 10\)|&#" examples/rss-reader/src/index.tsx || true
printf '\nBehavioral probes for String.fromCodePoint:\n'
node - <<'JS'
const inputs = [0xFFFFFFFF, 99999999, 0x10FFFF, 0x110000, 0];
for (const value of inputs) {
try {
const s = String.fromCodePoint(value);
console.log(`${value} => finite=${Number.isFinite(value)} => "${s}" (code=${s.codePointAt(0)?.toString(16).toUpperCase()})`);
} catch (e) {
console.log(`${value} => finite=${Number.isFinite(value)} => throws ${e.name}: ${e.message}`);
}
}
JSRepository: Karanjot786/TermUI
Length of output: 3268
Validate the Unicode range and entity length before replacing the entity.
Number.isFinite does not validate a Unicode code point. Inputs such as � or � can pass this check and make String.fromCodePoint throw RangeError. Reject values outside 0..0x10FFFF and reject entities longer than eight digits so the decoder does not decode a truncated prefix as a different code point.
🤖 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 - 35, Update the
entity-decoding logic around the codePoint parsing and replacement branches to
validate the full numeric entity before calling String.fromCodePoint: require
1–8 digits, ensure the value is finite and within 0..0x10FFFF, and return the
original match for invalid or oversized entities. Apply the same validation to
both hexadecimal and decimal entities, preventing truncated prefixes from being
decoded.
| const sortedA = [...a].sort((a, b) => a - b); | ||
| const sortedB = [...b].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
echo "== files =="
git ls-files 'packages/ui/src/TreeSelect.ts' 'scripts/build-registry.ts' 'tsconfig*.json' 2>/dev/null || true
echo
echo "== package type settings =="
for f in $(git ls-files 'tsconfig*.json' 'packages/**/tsconfig*.json' 'scripts/**/tsconfig*.json' 2>/dev/null | head -50); do
echo "--- $f"
sed -n '1,220p' "$f" | rg -n '"strict"|"moduleResolution"|"noImplicitAny"|"target"|"lib"|compilerOptions' || true
done
echo
echo "== TreeSelect relevant lines =="
sed -n '1,240p' packages/ui/src/TreeSelect.ts 2>/dev/null || true
echo
echo "== build-registry relevant lines =="
sed -n '1,120p' scripts/build-registry.ts 2>/dev/null || true
echo
echo "== occurrences =="
rg -n "sort\\(\\(a, b\\) => a - b\\)|sort\\(\\(a, b\\) =>" -S packages/ui/src scripts/build-registry.ts packages 2>/dev/null || true
echo
echo "== TypeScript availability/parser probe =="
node -e "const fs=require('node:fs'); for (const p of ['packages/ui/src/TreeSelect.ts','scripts/build-registry.ts']) console.log(p, 'exists', fs.existsSync(p));"
if command -v npx >/dev/null 2>&1; then
echo "npx available"
npx tsc --version 2>/dev/null || true
else
echo "npx unavailable"
fi
python3 - <<'PY'
import subprocess, textwrap, json, re, os
files = ['packages/ui/src/TreeSelect.ts','scripts/build-registry.ts']
for f in files:
print(f"--- {f}")
data = open(f,'r').read()
for i,line in enumerate(data.splitlines(),1):
if 'sort((a, b) => a - b)' in line:
print(f"{i}:{line}")
PYRepository: Karanjot786/TermUI
Length of output: 15092
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const values = {
zero: ['0', '0'],
numerics: ['1', '10', '2'],
packages: ['`@termuijs/core`', '`@termuijs/widgets`'],
mixed_nonnumeric: ['abc', '1'],
};
for (const [name, arr] of Object.entries(values)) {
const withSubtract = arr.slice().sort((a, b) => a - b);
const withLocaleCompare = arr.slice().sort((a, b) => a.localeCompare(b));
console.log(name, 'input', JSON.stringify(arr), 'subtract', JSON.stringify(withSubtract), 'localeCompare', JSON.stringify(withLocaleCompare));
}
JS
TS_VERSION=$(npx tsc --version 2>/dev/null | awk '{print $2}' || true)
tmpdir=${PWD}/tmp-ts-string-subtract-probe
rm -rf "$tmpdir"
mkdir -p "$tmpdir"
cat > "$tmpdir/comparators.ts" <<'TS'
function subtract(a: string, b: string): number { return a - b; }
subtract('0', '0');
subtract('1', '10');
function localeCompare(a: string, b: string): number { return a.localeCompare(b); }
localeCompare('0', '0');
localeCompare('1', '10');
TS
cat > "$tmpdir/tsconfig.json" <<'JSON'
{
"compilerOptions": {
"target": "ES2022",
"moduleResolution": "bundler",
"lib": ["ES2022"],
"strict": true,
"noEmit": true
},
"files": ["comparators.ts"]
}
JSON
cd "$tmpdir"
timeout 60 npx tsc --showConfig 2>/dev/null | sed -n '1,120p' || true
echo "--- tsc output -----------------------"
timeout 60 npx tsc 2>&1 | sed -n '1,200p' || true
rm -rf "$tmpdir"Repository: Karanjot786/TermUI
Length of output: 621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const values = {
zero: ['0', '0'],
numerics: ['1', '10', '2'],
packages: ['`@termuijs/core`', '`@termuijs/widgets`'],
mixed_nonnumeric: ['abc', '1'],
};
for (const [name, arr] of Object.entries(values)) {
const withSubtract = arr.slice().sort((a, b) => a - b);
const withLocaleCompare = arr.slice().sort((a, b) => a.localeCompare(b));
console.log(name, 'input', JSON.stringify(arr), 'subtract', JSON.stringify(withSubtract), 'localeCompare', JSON.stringify(withLocaleCompare));
}
JS
TS_VERSION=$(npx tsc --version 2>/dev/null | awk '{print $2}' || true)
tmpdir=${PWD}/tmp-ts-string-subtract-probe
rm -rf "$tmpdir"
mkdir -p "$tmpdir"
cat >("$tmpdir/comparators.ts") <<'TS'
function subtract(a: string, b: string): number { return a - b; }
subtract('0', '0');
subtract('1', '10');
function localeCompare(a: string, b: string): number { return a.localeCompare(b); }
localeCompare('0', '0');
localeCompare('1', '10');
TS
cat >"$tmpdir/tsconfig.json" <<'JSON'
{
"compilerOptions": {
"target": "ES2022",
"moduleResolution": "bundler",
"lib": ["ES2022"],
"strict": true,
"noEmit": true
},
"files": ["comparators.ts"]
}
JSON
cd "$tmpdir"
timeout 60 npx tsc --showConfig 2>/dev/null | sed -n '1,120p' || true
echo "--- tsc output -----------------------"
timeout 60 npx tsc 2>&1 | sed -n '1,200p' || true
rm -rf "$tmpdir"Repository: Karanjot786/TermUI
Length of output: 688
Use string comparators instead of subtraction.
Both comparators operate on string values, and subtraction produces numeric ordering instead of the intended string order for nonnumeric values such as ['abc', '1']. Use String.prototype.localeCompare; for scripts/build-registry.ts, the default string sort is also sufficient.
🧰 Tools
🪛 GitHub Actions: CI / 0_build-and-test.txt
[error] 185-186: TypeScript declaration build failed: arithmetic operands on lines 185-186 are not typed as any, number, bigint, or an enum (TS2362, TS2363). The '@termuijs/ui' build command 'bun run build' / 'tsup' exited with code 1.
🪛 GitHub Actions: CI / build-and-test
[error] 185-186: TypeScript declaration build failed: arithmetic operands on lines 185-186 are not typed as any, number, bigint, or an enum type (TS2362/TS2363). The @termuijs/ui build command exited with code 1.
📍 Affects 2 files
packages/ui/src/TreeSelect.ts#L185-L186(this comment)scripts/build-registry.ts#L47-L47
🤖 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, Replace the
subtraction comparators in TreeSelect’s sortedA and sortedB sorting with
String.prototype.localeCompare to preserve string ordering for all values. Also
update the sorting expression at scripts/build-registry.ts line 47 to use the
default string sort, as no numeric comparator is appropriate there.
Source: Coding guidelines
Description
This PR fixes real bugs found in the codebase:
.map()on an undefined collection threwTypeError; now falls back to[]..map()on an undefined collection threwTypeError; now falls back to[].Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3383