fix: resolve 4 bugs in termui - #3656
Open
saurabhhhcodes wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThe PR tracks the weather polling interval and adds ChangesTimer and rendering corrections
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@packages/dev-server/src/devtools.ts`:
- Line 81: Update the FPS rounding expression in the devtools FPS calculation so
the epsilon is applied relative to the scaled value before Math.round, rather
than adding raw Number.EPSILON to the original product. Preserve the existing
one-decimal output 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: d2181fb5-0c99-418d-bb7a-e8015e93f655
📒 Files selected for processing (3)
examples/weather/src/index.tsxpackages/dev-server/src/devtools.tspackages/ui/src/Switch.ts
| widgetCount, | ||
| lastRenderAt: now, | ||
| fps: Math.round(fps * 10) / 10, | ||
| fps: Math.round(fps * 10 + Number.EPSILON) / 10, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
node <<'EOF'
for (const fps of [10, 30, 60, 120]) {
const scaled = fps * 10;
const current = scaled + Number.EPSILON;
const scaleAware = scaled + Number.EPSILON * Math.max(1, Math.abs(scaled));
console.log({
fps,
currentChangesValue: current !== scaled,
scaleAwareChangesValue: scaleAware !== scaled,
});
}
EOFRepository: Karanjot786/TermUI
Length of output: 437
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant file around the changed line.
if [ -f packages/dev-server/src/devtools.ts ]; then
echo "---- packages/dev-server/src/devtools.ts lines 60-95 ----"
sed -n '60,95p' packages/dev-server/src/devtools.ts | nl -ba -v60
else
echo "missing packages/dev-server/src/devtools.ts"
fd -a 'devtools\.ts$' .
fi
echo "---- behavioral probe for intended boundary cases ----"
node - <<'EOF'
function current(fps) {
return Math.round(fps * 10 + Number.EPSILON) / 10;
}
function scaleAware(fps) {
const scaledFps = fps * 10;
return Math.round(
scaledFps + Number.EPSILON * Math.max(1, Math.abs(scaledFps)),
) / 10;
}
const inputs = [
0.005, 0.015, 0.05,
1.005, 1.015, 1.05,
10.005, 10.015, 10.05,
30.005, 30.015, 30.05,
60.005, 60.015, 60.05,
120.05,
Number.float64FromBits?(0x3fd0000000000000n): undefined,
];
for (const fps of inputs) {
const scaled = fps * 10;
if (Number.isNaN(scaled)) continue;
console.log(JSON.stringify({
fps,
rounded: current(fps),
scaleAwareRounded: scaleAware(fps),
boundaryNear: fps === Math.round(fps * 10 + Number.EPSILON * Math.max(1, Math.abs(fps * 10))) / 10 + 0,
scaled: scaled,
currentChangesScaled: Number.NEXT_UP(scaled) !== scaled,
maxSafeIntFloor: Math.floor(Number.MAX_SAFE_INTEGER)
}));
}
EOFRepository: Karanjot786/TermUI
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("packages/dev-server/src/devtools.ts")
if p.exists():
lines = p.read_text().splitlines()
print("---- packages/dev-server/src/devtools.ts lines 60-95 ----")
for i, line in enumerate(lines[59:95], start=60):
print(f"{i}\t{line}")
else:
print("missing packages/dev-server/src/devtools.ts")
PY
echo "---- behavioral probe for intended boundary cases ----"
node - <<'EOF'
function current(fps) {
return Math.round(fps * 10 + Number.EPSILON) / 10;
}
function scaleAware(fps) {
const scaledFps = fps * 10;
return Math.round(
scaledFps + Number.EPSILON * Math.max(1, Math.abs(scaledFps)),
) / 10;
}
const inputs = [
0.005, 0.015, 0.05,
1.005, 1.015, 1.05,
10.005, 10.015, 10.05,
30.005, 30.015, 30.05,
60.005, 60.015, 60.05,
120.05,
];
for (const fps of inputs) {
const scaled = fps * 10;
console.log(JSON.stringify({
fps,
currentRounded: current(fps),
scaleAwareRounded: scaleAware(fps),
scaledFloat: scaled,
currentChangesScaled: scaled + Number.EPSILON !== scaled,
scaleAwareChangesScaled: scaled + Number.EPSILON * Math.max(1, Math.abs(scaled)) !== scaled,
}));
}
EOFRepository: Karanjot786/TermUI
Length of output: 3982
Scale the epsilon with the rounded FPS value.
At 60 FPS, fps * 10 becomes 600, and Number.EPSILON is far smaller than the spacing of values around 600. This makes the + Number.EPSILON correction ineffective for normal FPS values. Apply the epsilon to the scaled value instead.
🤖 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/dev-server/src/devtools.ts` at line 81, Update the FPS rounding
expression in the devtools FPS calculation so the epsilon is applied relative to
the scaled value before Math.round, rather than adding raw Number.EPSILON to the
original product. Preserve the existing one-decimal output behavior.
github-actions
Bot
added
area:examples
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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).trim()to''misses whitespace-only input;.trim().length === 0is explicit.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: #3655
Summary by CodeRabbit