Skip to content

fix: resolve 4 bugs in termui #3496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
saurabhhhcodes wants to merge 1 commit into from
Closed

fix: resolve 4 bugs in termui #3496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/todo-app/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class CustomMultiProgress extends (MultiProgressClass as any) {
const value = Math.max(0, Math.min(1, item.value));
const filled = Math.round(barWidth * value);

const pct = Math.round(value * 100);
const pct = Math.round(value * 100 + Number.EPSILON);

@coderabbitai coderabbitai Bot Aug 4, 2026

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 | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

node <<'EOF'
const value = 0.575;
const current = Math.round(value * 100 + Number.EPSILON);
const proposed = Math.round((value + Number.EPSILON) * 100);

console.log({ current, proposed });
if (current !== 57 || proposed !== 58) process.exit(1);
EOF

Repository: Karanjot786/TermUI

Length of output: 186

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
git ls-files | rg '(^|/)examples/todo-app/src/index\.ts$' || true

echo "== relevant context =="
if [ -f examples/todo-app/src/index.ts ]; then
  nl -ba examples/todo-app/src/index.ts | sed -n '80,120p'
fi

echo "== runtime boundary probe =="
node - <<'JS'
const values = [0.575, 0.015, 0.025, 0.035, 0.045, 0.055, 0.065, 0.075];
const expected = [58, 2, 2, 4, 4, 6, 6, 8];
const currentErrors = [];
const proposedErrors = [];
for (let i=0;i<values.length;i++) {
  const value = values[i];
  const current = Math.round(value * 100 + Number.EPSILON);
  const proposed = Math.round((value + Number.EPSILON) * 100);
  if (current !== expected[i]) currentErrors.push(`${value}: ${current}`);
  if (proposed !== expected[i]) proposedErrors.push(`${value}: ${proposed}`);
  console.log(`${value}: current=${current} proposed=${proposed} expected=${expected[i]}`);
}
console.log(`current_errors=${currentErrors.length} proposed_errors=${proposedErrors.length}`);
JS

Repository: Karanjot786/TermUI

Length of output: 268

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== relevant context =="
awk '{printf "%6d\t%s\n", NR, $0}' examples/todo-app/src/index.ts | sed -n '80,120p'

echo "== runtime boundary probe =="
node - <<'JS'
const values = [0.575, 0.015, 0.025, 0.035, 0.045, 0.055, 0.065, 0.075];
const expected = [58, 2, 2, 4, 4, 6, 6, 8];
const currentErrors = [];
const proposedErrors = [];
for (let i=0;i<values.length;i++) {
  const value = values[i];
  const current = Math.round(value * 100 + Number.EPSILON);
  const proposed = Math.round((value + Number.EPSILON) * 100);
  if (current !== expected[i]) currentErrors.push(`${value}: ${current}`);
  if (proposed !== expected[i]) proposedErrors.push(`${value}: ${proposed}`);
  console.log(`${value}: current=${current} proposed=${proposed} expected=${expected[i]}`);
}
console.log(`current_errors=${currentErrors.length} proposed_errors=${proposedErrors.length}`);
JS

Repository: Karanjot786/TermUI

Length of output: 2624

Scale the epsilon before rounding.

Adding Number.EPSILON after value * 100 does not correct some floating-point rounding boundaries. For example, 0.575 * 100 can be 57.49999999999999, which still rounds to 57 instead of 58.

Add the epsilon before scaling and add a regression test for this boundary value.

Proposed fix 
-            const pct = Math.round(value * 100 + Number.EPSILON);
+            const pct = Math.round((value + Number.EPSILON) * 100);
🤖 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/todo-app/src/index.ts` at line 107, Update the percentage
calculation at pct to add Number.EPSILON to value before multiplying by 100,
then round the scaled result. Add a regression test covering value 0.575 and
assert it produces 58.

const percentStr = ` ${pct}% `;
const showPct = barWidth >= percentStr.length;
const labelStart = showPct ? Math.floor((barWidth - percentStr.length) / 2) : -1;
Expand Down
2 changes: 2 additions & 0 deletions packages/ui/src/Form.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,5 @@ export class Form extends Widget {
}
}
}

.catch(err => console.error("Promise.all failed:", err));

@coderabbitai coderabbitai Bot Aug 4, 2026

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

Attach the rejection handler correctly and preserve submit cleanup.

Line 142 is a standalone .catch(...) expression, so TypeScript cannot parse this file. Attach the handler to the Promise.all(...) expression or wrap the await in try/catch.

When validation rejects, execution also bypasses Lines 92 and 97. Move _isValidating = false and markDirty() into finally; keep submit callbacks on the successful validation path. Add a test in packages/ui/src/Form.test.ts for a rejected validator.

🧰 Tools
🪛 Biome (2.5.5)

[error] 142-142: Expected a statement but instead found '.catch(err => console.error("Promise.all failed:", err))'.

(parse)

🪛 GitHub Actions: CI / 0_build-and-test.txt

[error] 142-142: The @termuijs/ui build (tsup/esbuild) failed due to an unexpected '.' at the start of '.catch(err => console.error("Promise.all failed:", err));'. TypeScript also reported declaration and syntax errors on this line.

🪛 GitHub Actions: CI / build-and-test

[error] 142-142: The @termuijs/ui build command (tsup) failed because of an unexpected '.' in .catch(err => console.error("Promise.all failed:", err));. TypeScript also reported syntax errors including missing try, ), and ;.

🤖 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/Form.ts` at line 142, Fix the validation flow in the Form
submit logic so the rejection handler is syntactically attached to the
Promise.all expression or handled with try/catch. Move _isValidating = false and
markDirty() into a finally block so cleanup runs for both resolved and rejected
validators, while keeping submit callbacks only on successful validation; add a
Form.test.ts case covering a rejected validator.

Source: Linters/SAST tools

2 changes: 1 addition & 1 deletion packages/ui/src/MultiSelect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class MultiSelect extends Widget {
}

get selectedOptions(): MultiSelectOption[] {
return [...this._checked].sort().map(i => this._options[i]);
return [...this._checked].sort((a, b) => a - b).map(i => this._options[i]);
}
selectNext(): void { if (this._options.length === 0) return; let n = this._cursorIndex + 1; while (n < this._options.length && this._options[n].disabled) n++; if (n < this._options.length) { this._cursorIndex = n; this.markDirty(); } }
selectPrev(): void { if (this._options.length === 0) return; let n = this._cursorIndex - 1; while (n >= 0 && this._options[n].disabled) n--; if (n >= 0) { this._cursorIndex = n; this.markDirty(); } }
Expand Down
2 changes: 1 addition & 1 deletion scripts/build-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function collectDeps(content: string): string[] {
const deps = new Set<string>();
let m: RegExpExecArray | null;
while ((m = re.exec(content)) !== null) deps.add(m[1]!);
return [...deps].sort();
return [...deps].sort((a, b) => a - b);

@coderabbitai coderabbitai Bot Aug 4, 2026

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:

#!/usr/bin/env bash
set -euo pipefail

npx tsc --noEmit --pretty false
npx vitest run scripts/build-registry.test.ts

Repository: Karanjot786/TermUI

Length of output: 5708

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '== files ==\n'
git ls-files | rg '(^|/)(build-registry|build-registry\.test)\.(ts|tsx)$|tsconfig.*\.json$|package\.json$' || true

printf '\n== build-registry outline ==\n'
ast-grep outline scripts/build-registry.ts --view expanded || true

printf '\n== relevant build-registry lines ==\n'
sed -n '1,100p' scripts/build-registry.ts

printf '\n== relevant tests around dependency order ==\n'
sed -n '100,160p' scripts/build-registry.test.ts

printf '\n== tsconfig files ==\n'
for f in $(git ls-files 'tsconfig*.json'); do
  echo "--- $f"
  cat -n "$f"
done

printf '\n== runtime comparator behavior probe ==\n'
node - <<'JS'
const deps = ['foo', 'bar', 'baz'];
console.log([...deps].sort((a, b) => a - b));
console.log('comparator returns NaN:', Number.isNaN((a, b) => a - b) ? true : ((a,b)=>a-b)('foo','bar'));
JS

Repository: Karanjot786/TermUI

Length of output: 13309

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '== package scripts/typecheck fields ==\n'
node - <<'JS'
const { readFileSync } = require('node:fs');
const pkg = JSON.parse(readFileSync('package.json', 'utf-8'));
console.log(JSON.stringify({ scripts: pkg.scripts, dependencies: pkg.dependencies }, null, 2));
JS

printf '\n== lockfile snippets around bun/tsc packages if present ==\n'
git ls-files | rg '(^|/)(bun\.lockb|bun\.lock|pnpm-lock\.yaml|package-lock\.json|yarn\.lock)$' || true

Repository: Karanjot786/TermUI

Length of output: 816

Use a string comparator for package specifiers.

collectDeps handles string specifiers, so a - b makes the compiled script compare by NaN instead of lexicographically when the bun script is run. Use a string comparator to match the registry collectDeps test expectations.

🤖 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 sorting logic in
collectDeps to compare package specifiers lexicographically as strings instead
of subtracting them numerically. Preserve the existing deduplication and return
behavior while ensuring the ordering matches the registry tests when the bun
script runs.

Source: Coding guidelines

}

export function toSlug(name: string): string {
Expand Down
Loading