fix: resolve 4 bugs in termui - #3695
Conversation
github-actions
Bot
added
type:bug
📝 WalkthroughWalkthroughThe PR updates progress percentage rounding in the Pomodoro and todo examples. It also adds promise rejection logging in ChangesBehavior updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.6)packages/dev-server/src/server.tsFile contains syntax errors that prevent linting: Line 383: Expected a parenthesis '(' but instead found '=>'. packages/ui/src/Form.tsFile contains syntax errors that prevent linting: Line 142: Expected a statement but instead found '.catch(err => console.error("Promise.all failed:", err))'. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/pomodoro-timer/src/index.tsx`:
- Line 185: Update both percentage renderers in
examples/pomodoro-timer/src/index.tsx:185-185 and
examples/todo-app/src/index.ts:107-107 to add Number.EPSILON to the value before
multiplying by 100, preserving the existing conditional label behavior. Add or
update tests covering 0.145 so it renders as 15% in both renderers.
In `@packages/ui/src/Form.ts`:
- Around line 141-142: Fix the malformed Promise rejection handlers in both
sites: in packages/ui/src/Form.ts lines 141-142, attach a valid callback to
Promise.all that logs the error and rethrows it, then reset validation state in
finally; in packages/dev-server/src/server.ts line 383, replace the invalid
handler with a valid (err: unknown) callback that logs the child-process exit
failure.
🪄 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: 8df3b6dd-802b-4cb4-83b9-ed93bf721b8a
📒 Files selected for processing (4)
examples/pomodoro-timer/src/index.tsxexamples/todo-app/src/index.tspackages/dev-server/src/server.tspackages/ui/src/Form.ts
| const attrs = styleToCellAttrs(this._style); | ||
|
|
||
| const label = this._showLabel ? ` ${Math.round(this._value * 100)}%` : ''; | ||
| const label = this._showLabel ? ` ${Math.round(this._value * 100 + Number.EPSILON)}%` : ''; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node <<'NODE'
const value = 0.145;
const current = Math.round(value * 100 + Number.EPSILON);
const corrected = Math.round((value + Number.EPSILON) * 100);
if (current !== 14 || corrected !== 15) {
throw new Error(`Unexpected result: current=${current}, corrected=${corrected}`);
}
NODERepository: Karanjot786/TermUI
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate changed/mentioned files:"
git ls-files | rg '(examples/pomodoro-timer/src/index\.tsx|examples/todo-app/src/index\.ts)$' || true
echo
echo "Relevant lines:"
sed -n '175,190p' examples/pomodoro-timer/src/index.tsx
echo "---"
sed -n '95,115p' examples/todo-app/src/index.ts
echo
echo "Behavioral probe for representative float boundaries:"
node <<'JS'
const boundaries = [
0.145,
0.00005,
0.0145,
0.00045,
];
for (const value of boundaries) {
const current = Math.round(value * 100 + Number.EPSILON);
const corrected = Math.round((value + Number.EPSILON) * 100);
console.log(JSON.stringify({ value, current, corrected }));
}
JSRepository: Karanjot786/TermUI
Length of output: 2017
Apply epsilon before scaling in both renderers.
Number.EPSILON is too small at the scaled value to fix values like 0.145, which currently rounds to 14%; Math.round((value + Number.EPSILON) * 100) rounds it to 15%. Update both percentage renders and add a test for 0.145.
📍 Affects 2 files
examples/pomodoro-timer/src/index.tsx#L185-L185(this comment)examples/todo-app/src/index.ts#L107-L107
🤖 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/pomodoro-timer/src/index.tsx` at line 185, Update both percentage
renderers in examples/pomodoro-timer/src/index.tsx:185-185 and
examples/todo-app/src/index.ts:107-107 to add Number.EPSILON to the value before
multiplying by 100, preserving the existing conditional label behavior. Add or
update tests covering 0.145 so it renders as 15% in both renderers.
|
|
||
| .catch(err => console.error("Promise.all failed:", err)); No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Correct both malformed Promise rejection handlers before merging.
Both changes contain invalid TypeScript catch syntax, so the affected files cannot compile.
packages/ui/src/Form.ts#L141-L142: attach the handler toPromise.all, rethrow after logging, and reset validation state infinally.packages/dev-server/src/server.ts#L383-L383: use a valid callback such as(err: unknown) => console.error('Child process exit failed:', err).
🧰 Tools
🪛 Biome (2.5.6)
[error] 142-142: Expected a statement but instead found '.catch(err => console.error("Promise.all failed:", err))'.
(parse)
📍 Affects 2 files
packages/ui/src/Form.ts#L141-L142(this comment)packages/dev-server/src/server.ts#L383-L383
🤖 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` around lines 141 - 142, Fix the malformed Promise
rejection handlers in both sites: in packages/ui/src/Form.ts lines 141-142,
attach a valid callback to Promise.all that logs the error and rethrows it, then
reset validation state in finally; in packages/dev-server/src/server.ts line
383, replace the invalid handler with a valid (err: unknown) callback that logs
the child-process exit failure.
Source: Linters/SAST tools
1 participant
Description
This PR fixes real bugs found in the codebase:
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).Promise.all: an unhandled rejection in any input promise previously crashed silently.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3694
Summary by CodeRabbit