fix: resolve 4 bugs in termui - #3529
Conversation
📝 WalkthroughWalkthroughThe changes update streaming timer cleanup, whitespace-only paragraph parsing, tab-key radix handling, and reload exit error logging. The reload handler change contains a syntactically invalid catch expression. ChangesStreaming timer update
Example parsing updates
Reload error handling
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 '=>'. 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
🧹 Nitpick comments (1)
examples/ai-streaming/src/index.tsx (1)
43-43: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winKeep the streaming interval on the
AIStreamingAppinstance.
window.__intervalis the only shared owner, so current usage does not show duplicate timers. However,clearInterval(window.__interval)still cancels that global slot on remounts if this app can mount again. Use instance-owned timer state with teardown cleanup, followingexamples/widget-gallery/src/tabs/ai-tab.ts.🤖 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/ai-streaming/src/index.tsx` at line 43, Update AIStreamingApp to store the streaming interval on the component instance rather than window.__interval, and clear that instance-owned timer during teardown or unmount. Follow the timer ownership and cleanup pattern used by ai-tab.ts while preserving the existing streaming behavior.
🤖 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/server.ts`:
- Line 383: Fix the malformed catch callback on exitedPromise by providing a
valid callback parameter and logging the caught error through console.error.
Preserve the existing await and promise-rejection handling behavior.
---
Nitpick comments:
In `@examples/ai-streaming/src/index.tsx`:
- Line 43: Update AIStreamingApp to store the streaming interval on the
component instance rather than window.__interval, and clear that instance-owned
timer during teardown or unmount. Follow the timer ownership and cleanup pattern
used by ai-tab.ts while preserving the existing streaming 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: 1275b341-403c-4a94-9ab0-377f20c53e0f
📒 Files selected for processing (4)
examples/ai-streaming/src/index.tsxexamples/chat-app/src/index.tsxexamples/showcase/src/index.tsxpackages/dev-server/src/server.ts
| this._killChild(); | ||
|
|
||
| await exitedPromise.catch(() => {}); | ||
| await exitedPromise.catch( => console.error()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="packages/dev-server/src/server.ts"
if rg -nP '\.catch\(\s*=>' "$file"; then
echo "Invalid catch callback remains in $file" >&2
exit 1
fiRepository: Karanjot786/TermUI
Length of output: 288
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="packages/dev-server/src/server.ts"
echo "Relevant line:"
sed -n '370,390p' "$file" | nl -ba -v370
echo
echo "Parse/compile check with TypeScript parser if available:"
if command -v tsc >/dev/null 2>&1; then
tsc --noEmit --strict --checkJs false "$file" 2>&1 | sed -n '1,80p' || true
else
echo "tsc not available"
fi
echo
echo "Behavioral probe for the callback expression alone:"
node --input-type=module - <<'JS'
try {
new Function('await new Promise(resolve => setTimeout(resolve, 1)).catch( => console.error());');
console.log('parsed');
} catch (error) {
console.log(error.name + ': ' + error.message);
}
JSRepository: Karanjot786/TermUI
Length of output: 211
Fix the malformed catch callback.
await exitedPromise.catch( => console.error()); uses => without a callback parameter, so packages/dev-server/src/server.ts still fails to parse. Use an explicit callback parameter or a callback body, such as (error) => console.error(error).
🧰 Tools
🪛 Biome (2.5.6)
[error] 383-383: Expected a parenthesis '(' but instead found '=>'.
(parse)
🤖 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/server.ts` at line 383, Fix the malformed catch
callback on exitedPromise by providing a valid callback parameter and logging
the caught error through console.error. Preserve the existing await and
promise-rejection handling behavior.
Source: Linters/SAST tools
Description
This PR fixes real bugs found in the codebase:
parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.trim()to''misses whitespace-only input;.trim().length === 0is explicit.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3528
Summary by CodeRabbit