Skip to content

fix: resolve 4 bugs in termui - #3670

Closed
saurabhhhcodes wants to merge 1 commit into
Karanjot786:mainfrom
saurabhhhcodes:fix/termui-93050
Closed

fix: resolve 4 bugs in termui#3670
saurabhhhcodes wants to merge 1 commit into
Karanjot786:mainfrom
saurabhhhcodes:fix/termui-93050

Conversation

@saurabhhhcodes

@saurabhhhcodes saurabhhhcodes commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes real bugs found in the codebase:

  • Fixed default sort: .sort() coerces elements to strings, so [10, 9, 2] sorts as [10, 2, 9]; numeric comparator sorts correctly.
  • Added Number.EPSILON to Math.round: prevents floating-point drift (e.g. 1.005 * 100 rounding to 100 instead of 101).
  • Added Number.EPSILON to Math.round: prevents floating-point drift (e.g. 1.005 * 100 rounding to 100 instead of 101).
  • Prevented interval leak: repeated mounts now clear the previous interval before scheduling a new one.

Type of Change

  • Bug fix (non-breaking change fixing an issue)

How Has This Been Tested?

  • Local manual testing

Checklist

  • My code follows the style guidelines
  • I have performed a self-review

Related Issue

Ref: #3669

Summary by CodeRabbit

  • Bug Fixes
    • Prevented overlapping streaming timers when setup runs repeatedly.
    • Improved progress percentage rounding for more accurate labels and calculations.
    • Ensured dependency names are sorted consistently during registry generation.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR resets streaming intervals, improves floating-point percentage rounding in two examples, and adds an explicit comparator when sorting dependency names during registry generation.

Changes

Example runtime fixes

Layer / File(s) Summary
Timer and percentage corrections
examples/ai-streaming/src/index.tsx, examples/pomodoro-timer/src/index.tsx, examples/todo-app/src/index.ts
The streaming example clears the existing interval before creating a new one. Both progress displays add Number.EPSILON before rounding percentages.

Registry dependency sorting

Layer / File(s) Summary
Explicit dependency ordering
scripts/build-registry.ts
collectDeps now uses an explicit comparator when sorting dependency package names.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested labels: type:bug, area:examples

Suggested reviewers: karanjot786

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the fixes but omits required package information, a closing issue link, the repository checklist, and GSSoC details. Add the affected packages, replace Ref: #3669 with `Closes `#3669, complete the required checklist, and provide the GSSoC profile information.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies four bug fixes and follows the required type: short description format.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 6c7584e and 9d850a2.

📒 Files selected for processing (4)
  • examples/ai-streaming/src/index.tsx
  • examples/pomodoro-timer/src/index.tsx
  • examples/todo-app/src/index.ts
  • scripts/build-registry.ts

Comment thread scripts/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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.ts

Repository: 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 || true

Repository: 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.

Suggested change
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

@saurabhhhcodes saurabhhhcodes mentioned this pull request Aug 6, 2026
4 tasks
@coderabbitai coderabbitai Bot mentioned this pull request Aug 7, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant