fix: resolve 4 bugs in termui - #3670
Conversation
📝 WalkthroughWalkthroughThe PR resets streaming intervals, improves floating-point percentage rounding in two examples, and adds an explicit comparator when sorting dependency names during registry generation. ChangesExample runtime fixes
Registry dependency sorting
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)
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: 1
🤖 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 `@scripts/build-registry.ts`:
- Line 47: Update the dependency sorting expression in the registry-building
logic to use a string comparator for the string-valued dependencies, ordering
names lexicographically instead of subtracting them. Preserve the existing
deduplication and returned-array behavior.
🪄 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: f9b312e5-79a6-4d27-a2b8-a4aadd9b7747
📒 Files selected for processing (4)
examples/ai-streaming/src/index.tsxexamples/pomodoro-timer/src/index.tsxexamples/todo-app/src/index.tsscripts/build-registry.ts
| let m: RegExpExecArray | null; | ||
| while ((m = re.exec(content)) !== null) deps.add(m[1]!); | ||
| return [...deps].sort(); | ||
| return [...deps].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
rg -nF 'return [...deps].sort((a, b) => a - b);' scripts/build-registry.ts
rg -n -A8 'collects unique sorted' scripts/build-registry.test.tsRepository: Karanjot786/TermUI
Length of output: 665
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,90p' scripts/build-registry.ts
sed -n '110,145p' scripts/build-registry.test.ts
sed -n '1,40p' scripts/tsconfig.json 2>/dev/null || true
rg -n '"strict"|allowUnreachableCode|noUnusedParameters|`@ts-check`|extends' scripts tsconfig.json 2>/dev/null || trueRepository: Karanjot786/TermUI
Length of output: 5617
Use a string comparator for dependency sorting.
a and b are string values, so a - b fails TypeScript strict checking. Runtime subtraction also produces NaN, which does not order the names lexicographically like the test expects.
Proposed fix
- return [...deps].sort((a, b) => a - b);
+ return [...deps].sort((a, b) => a.localeCompare(b));📝 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.
| return [...deps].sort((a, b) => a - b); | |
| return [...deps].sort((a, b) => a.localeCompare(b)); |
🤖 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 `@scripts/build-registry.ts` at line 47, Update the dependency sorting
expression in the registry-building logic to use a string comparator for the
string-valued dependencies, ordering names lexicographically instead of
subtracting them. Preserve the existing deduplication and returned-array
behavior.
Source: Coding guidelines
Description
This PR fixes real bugs found in the codebase:
.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).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: #3669
Summary by CodeRabbit