Repository navigation
fix: resolve 2 bugs - #3372
fix: resolve 2 bugs#3372saurabhhhcodes wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 35 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 add bounded numeric entity parsing, explicit decimal radix arguments for keyboard parsing, and explicit comparators for string array and dependency sorting. ChangesNumeric parsing
Explicit ordering
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 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 numeric-entity handling in the entity-decoding
function to match at most eight digits, parse the complete suffix rather than a
truncated slice, and reject values outside the valid Unicode range, including
values above 0x10FFFF, before calling String.fromCodePoint. Apply the same
validation to both decimal entities and hexadecimal entities while preserving
the existing fallback to match for invalid input.
In `@packages/ui/src/TreeSelect.ts`:
- Around line 185-186: Update both sorted selection comparisons in
packages/ui/src/TreeSelect.ts (lines 185-186) and scripts/build-registry.ts
(line 47) to use string comparators via localeCompare instead of numeric
subtraction. Preserve the existing _valuesEqual comparison behavior while
correctly sorting non-numeric string values.
🪄 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: 2dde5597-85e3-464f-a132-ede607350a34
📒 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
echo "Files:"
git ls-files | rg '(^|/)index\.tsx$|package.json|tsconfig' | head -200
echo
echo "Target file excerpt:"
if [ -f examples/rss-reader/src/index.tsx ]; then
nl -ba examples/rss-reader/src/index.tsx | sed -n '1,80p'
fi
echo
echo "Search numeric entity decode logic:"
rg -n "fromCodePoint|codePoint|parseInt|slashEncode|replace.*entities|entities" examples/rss-reader/src examples 2>/dev/null | head -200
echo
echo "Behavioral probe for current slicing and fromCodePoint behavior:"
node - <<'JS'
const candidates = [
'22fffff',
'999999999',
'10ffff',
'110000',
'0xfffffffffffffffffffffffffff',
'abc',
];
for (const text of candidates) {
for (const [radix, sliceStart, sliceEnd] of [
['16', 2, 10],
['10', 1, 10]
]) {
const entity = '#' + text;
const s = entity.slice(sliceStart, sliceEnd);
let codePoint;
let thrown;
try {
codePoint = Number.parseInt(s, radix);
const ok = Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : null;
if (ok !== null) console.log(`${text} radix${radix} first${sliceEnd-1} digits as utf16 ok`);
else console.log(`${text} radix${radix} first${sliceEnd-1} digits NaN`);
} catch (err) {
thrown = err && err.toString ? err.toString().split('\n')[0] : String(err);
console.log(`${text} radix${radix} first${sliceEnd-1} digits parsed ${codePoint} throws ${thrown}`);
}
}
let codePoint = Number.parseInt(text, 16);
try { String.fromCodePoint(codePoint); console.log(`${text} parsed ${codePoint} fromCodePoint ok`); }
catch (err) { console.log(`${text} parsed ${codePoint} fromCodePoint throws ${err.toString().split('\n')[0]}`); }
}
JSRepository: Karanjot786/TermUI
Length of output: 5143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Target file excerpt:"
awk '{printf("%6d: %s\n", NR, $0)}' examples/rss-reader/src/index.tsx | sed -n '1,90p'
echo
echo "Search numeric entity decode logic:"
rg -n "fromCodePoint|codePoint|parseInt|entities|slice\\(.*10|replace.*&#|`#x`?\\[" examples/rss-reader/src examples 2>/dev/null | head -200
echo
echo "Behavioral probe for current slicing and fromCodePoint behavior:"
node - <<'JS'
const regex = /&(`#x`?[0-9a-fA-F]+|[a-zA-Z]+);/g;
const sliceFn = (entity) => {
if (entity.startsWith('`#x`')) {
const codePoint = Number.parseInt(entity.slice(2, 10), 16);
return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : null;
}
if (entity.startsWith('#')) {
const codePoint = Number.parseInt(entity.slice(1, 10), 10);
return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : null;
}
return null;
};
const candidates = [
'&`#22fffff`;',
'&`#999999999`;',
'&`#10ffff`;',
'&`#110000`;',
'&`#x0xfffffffffffffffffffffffffff`;',
'&`#xabcdefg`;',
];
for (const text of candidates) {
const entity = text.replace(/^&#|;$/, '');
console.log(text + ' entity=' + entity);
console.log(' regexMatches=', !!entity.match(/^`#x`?[0-9a-fA-F]+$/));
let thrown;
try {
console.log(' decoded=' + sliceFn(entity));
} catch (err) {
thrown = err && err.toString ? err.toString().split('\n')[0] : String(err);
console.log(' throws=' + thrown);
}
}
JSRepository: Karanjot786/TermUI
Length of output: 5595
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Behavioral probe with correct entity suffix slicing:"
node - <<'JS'
function currentDecode(entity) {
if (entity.startsWith('`#x`')) {
const codePoint = Number.parseInt(entity.slice(2, 10), 16);
return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : null;
}
if (entity.startsWith('#')) {
const codePoint = Number.parseInt(entity.slice(1, 10), 10);
return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : null;
}
return null;
}
function wholeEntityDecode(entity) {
if (entity.startsWith('`#x`')) {
const codePoint = Number.parseInt(entity.slice(2), 16);
return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : null;
}
if (entity.startsWith('#')) {
const codePoint = Number.parseInt(entity.slice(1), 10);
return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : null;
}
return null;
}
for (const entity of ['22fffff', '999999999', '10ffff', '110000', '0xfffffffffffffffffffffffffff']) {
console.log(`##${entity}: regexMatch=${/^`#x`?[0-9a-fA-F]+$/.test(entity)} code=${currentDecode(entity)} whole=${wholeEntityDecode(entity)}`);
}
for (const entity of ['22fffff', '999999999', '10ffff', '110000', '0xfffffffffffffffffffffffffff']) {
console.log(`x0#${entity}: hexRegexMatch=${/^[0-9a-fA-F]+$/.test(entity)} code=${currentDecode(entity)} whole=${wholeEntityDecode(entity)}`);
}
JSRepository: Karanjot786/TermUI
Length of output: 759
Reject overlong and out-of-range numeric entities before decoding.
slice(..., 10) takes nine characters after the #; decimal entities like � can decode from a prefix. Hex decoding slices after #x, so malformed hex-only input or excessively long hex values can also pass the predicate. Limit the regex to at most eight digits, parse the complete entity suffix, and reject values above 0x10FFFF before calling String.fromCodePoint.
🤖 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
numeric-entity handling in the entity-decoding function to match at most eight
digits, parse the complete suffix rather than a truncated slice, and reject
values outside the valid Unicode range, including values above 0x10FFFF, before
calling String.fromCodePoint. Apply the same validation to both decimal entities
and hexadecimal entities while preserving the existing fallback to match for
invalid input.
| 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 | rg '(^|/)(TreeSelect|build-registry)\.(ts|tsx)$|packages/tsconfig|scripts/.*ts$' || true
echo "== TreeSelect relevant sections =="
if [ -f packages/ui/src/TreeSelect.ts ]; then
wc -l packages/ui/src/TreeSelect.ts
sed -n '150,210p' packages/ui/src/TreeSelect.ts
fi
echo "== build-registry relevant sections =="
if [ -f scripts/build-registry.ts ]; then
wc -l scripts/build-registry.ts
sed -n '30,65p' scripts/build-registry.ts
fi
echo "== TypeScript strict config snippets =="
for f in packages/ui/tsconfig.json tsconfig.json packages/tsconfig.json; do
if [ -f "$f" ]; then
echo "--- $f"
cat "$f"
fi
done
echo "== package scripts / dependency type context =="
if [ -f scripts/build-registry.ts ]; then
rg -n "deps|dependenc|sorted|sort\\(" scripts/build-registry.ts packages --glob '*.ts' --glob '*.tsx' -C 2 || true
fi
echo "== standalone JS subtraction comparator behavior =="
node - <<'JS'
const values = ["apples", "bananas", "Oranges"];
const sortedSub = [...values].sort((a, b) => a - b);
const sortedLex = [...values].sort((a, b) => a.localeCompare(b));
console.log(JSON.stringify({ sortedSub, sortedLex }));
for (const pair of [["a", "b"], ["1", "2"], ["Oranges", "apples"]]) {
const [a, b] = pair;
console.log(`${a} - ${b} = ${a - b}`);
}
JSRepository: Karanjot786/TermUI
Length of output: 50374
Use a string comparator for the sorted selection comparisons.
_valuesEqual compares string arrays, but a - b converts the elements to numbers and returns NaN when either value is non-numeric. Replace both sort comparators with a.localeCompare(b).
📍 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, Update both sorted
selection comparisons in packages/ui/src/TreeSelect.ts (lines 185-186) and
scripts/build-registry.ts (line 47) to use string comparators via localeCompare
instead of numeric subtraction. Preserve the existing _valuesEqual comparison
behavior while correctly sorting non-numeric string values.
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