Fix dev server hang by reducing log spam and event frequency#828
Fix dev server hang by reducing log spam and event frequency#828gsxdsm merged 4 commits intoAutoMaker-Org:v1.0.0rcfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses performance and stability issues in the development server and UI by implementing a series of optimizations. The primary goal is to prevent the dev server from hanging and the browser from slowing down due to excessive logging, high event frequency, and duplicate React instances. Changes include reducing server-side log spam, throttling WebSocket output, batching client-side log updates, extending polling intervals, and refining Vite's configuration for dependency resolution. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughOptimizes WebSocket logging and dev-server output frequency, adds an allocatedPort field to DevServerInfo, introduces RAF-based batching for UI dev-server logs, records WebSocket health to suppress polling, and tightens Vite React resolution and per-worktree caching. Changes
Sequence Diagram(s)sequenceDiagram
participant Server as Server (DevServer Service)
participant WS as WebSocket
participant Client as UI (http-api-client)
participant Buffer as UI Buffer (use-dev-server-logs)
participant Store as UI State
rect rgba(200,200,255,0.5)
Server->>WS: emit dev-server:output (high-frequency)
end
rect rgba(200,255,200,0.5)
WS->>Client: onmessage (raw event)
Client->>Client: filter high-frequency events -> log at debug or info
Client->>Buffer: deliver event (appendLogs)
end
rect rgba(255,200,200,0.5)
Buffer->>Buffer: batch via requestAnimationFrame / fallback timer
Buffer->>Store: flushPendingOutput -> commit logs to state
end
rect rgba(255,255,200,0.5)
Note over Client,Store: For non-output events Client calls recordGlobalEvent() to mark WS healthy, suppressing reconciliation polling
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts`:
- Around line 197-205: The current useEffect only cleans rafIdRef on unmount,
allowing stale pendingOutputRef to carry across worktree/session changes; modify
the effect in use-dev-server-logs.ts (the useEffect that currently returns the
raf cancellation) to run on relevant context changes (e.g., worktree id /
session id / dev-server started flag) instead of an empty dependency array and
inside its cleanup reset pendingOutputRef.current = "" (or null/[]) and
cancelAnimationFrame(rafIdRef.current) then set rafIdRef.current = null so any
buffered output is cleared whenever the worktree or dev-server session changes;
ensure you reference pendingOutputRef and rafIdRef in the dependency closure
correctly.
In `@apps/ui/src/lib/http-api-client.ts`:
- Around line 930-936: The high-frequency filter (variable isHighFrequency)
currently only checks data.type === 'auto_mode_progress' and misses wrapped
auto-mode messages like data.type === 'auto-mode:event'; update isHighFrequency
to also mark messages as high-frequency when data.type === 'auto-mode:event' and
the event payload indicates progress (e.g., data.payload?.subtype ===
'auto_mode_progress' or similar), while keeping the existing checks for
'dev-server:output' and 'test-runner:output' so logger.info('WebSocket
message:', data.type) is skipped for these high-frequency auto-mode progress
events.
In `@apps/ui/vite.config.mts`:
- Around line 256-268: The regex alias find: /^react(\/|$)/ is shadowing the
explicit aliases for 'react/jsx-runtime' and 'react/jsx-dev-runtime' so those
entries never match; fix by reordering the alias array in vite.config.mts to
place the explicit aliases for 'react/jsx-runtime' and 'react/jsx-dev-runtime'
(replacement paths pointing to ../../node_modules/react/...js) before the
broader /^react(\/|$)/ entry so Vite applies the specific matches first and
enables the deduplication fix.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/server/src/index.tsapps/server/src/services/dev-server-service.tsapps/server/tests/unit/services/dev-server-event-types.test.tsapps/ui/src/components/views/board-view/hooks/use-board-features.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-servers.tsapps/ui/src/lib/http-api-client.tsapps/ui/vite.config.mts
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts
Outdated
Show resolved
Hide resolved
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a critical performance issue in the Automaker development server by reducing log spam, throttling output, batching events, and optimizing Vite configuration. The changes aim to prevent memory growth, browser slowdown, and duplicate React instances. The review focuses on the server-side logging optimization and dev server output throttling, ensuring the changes effectively mitigate the identified problems and maintain code clarity.
Note: Security Review did not run due to the size of the PR.
| const OUTPUT_THROTTLE_MS = 100; // ~10fps max update rate | ||
| const OUTPUT_BATCH_SIZE = 8192; // Larger batches to compensate for lower frequency |
There was a problem hiding this comment.
The OUTPUT_THROTTLE_MS and OUTPUT_BATCH_SIZE constants are updated to 100ms and 8192 bytes, respectively. While these values are chosen to balance log streaming readability and WebSocket traffic, it's important to consider making these configurable via environment variables. This would allow administrators to fine-tune the throttling behavior without requiring code changes.
Making these values configurable would provide flexibility to adapt to different network conditions and browser capabilities.
| const OUTPUT_THROTTLE_MS = 100; // ~10fps max update rate | |
| const OUTPUT_BATCH_SIZE = 8192; // Larger batches to compensate for lower frequency | |
| const OUTPUT_THROTTLE_MS = parseInt(process.env.OUTPUT_THROTTLE_MS || '100', 10); // ~10fps max update rate | |
| const OUTPUT_BATCH_SIZE = parseInt(process.env.OUTPUT_BATCH_SIZE || '8192', 10); // Larger batches to compensate for lower frequency |
| } else { | ||
| logger.info('WARNING: Cannot send event, WebSocket not open. ReadyState:', ws.readyState); | ||
| logger.warn('Cannot send event, WebSocket not open. ReadyState:', ws.readyState); | ||
| } |
There was a problem hiding this comment.
The log level for the WebSocket not-open warning is changed from info to warn. While this change aligns with the severity of the message, it's important to ensure that such warnings are appropriately handled and monitored in a production environment. Consider adding metrics or alerts to track the frequency of these warnings to proactively address potential WebSocket connectivity issues.
If the WebSocket is not opened, it might be a critical issue that needs to be addressed soon.
| // Throttle output to prevent overwhelming WebSocket under heavy load. | ||
| // 100ms (~10fps) is sufficient for readable log streaming while keeping | ||
| // WebSocket traffic manageable. The previous 4ms rate (~250fps) generated | ||
| // up to 250 events/sec which caused progressive browser slowdown from | ||
| // accumulated console logs, JSON serialization overhead, and React re-renders. |
There was a problem hiding this comment.
The comments added to explain the throttling mechanism are very helpful for understanding the rationale behind the changes. However, the comment could be improved by explicitly mentioning the specific metrics or monitoring tools that can be used to observe the impact of the throttling on browser performance and server load. This would provide additional guidance for future maintenance and optimization.
It's important to monitor the impact of this change on the user experience. If the throttling is too aggressive, it could lead to a noticeable degradation in the responsiveness of the dev server logs.
| // Throttled output needs a bit of time (OUTPUT_THROTTLE_MS is 100ms) | ||
| await new Promise((resolve) => setTimeout(resolve, 250)); |
There was a problem hiding this comment.
The timeout in the test is increased from 100ms to 250ms to accommodate the new throttle rate. While this addresses the immediate issue, it's important to ensure that the test remains reliable and doesn't become flaky due to timing variations. Consider adding additional assertions or retries to make the test more robust.
It's important to ensure that the test consistently passes and doesn't flake due to timing issues.
- Reset RAF buffer on context changes (worktree switch, dev-server restart) to prevent stale output from flushing into new sessions - Fix high-frequency WebSocket filter to catch auto-mode:event wrapping (auto_mode_progress is wrapped in auto-mode:event) and add feature:progress - Reorder Vite aliases so explicit jsx-runtime entries aren't shadowed by the broad /^react(\/|$)/ regex (Vite uses first-match-wins) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts (1)
206-211:⚠️ Potential issue | 🟠 MajorReset the pending RAF buffer on worktree/session context change, not only unmount.
At Line 206, cleanup only runs on unmount. If
worktreePathchanges whilependingOutputRef(Line 198) still has content or a RAF is queued, stale output can flush into the new context.💡 Proposed fix
+ // Reset buffered output whenever the monitored context changes + useEffect(() => { + resetPendingOutput(); + }, [worktreePath, resetPendingOutput]); + // Clean up pending RAF on unmount to prevent state updates after unmount useEffect(() => { return () => { resetPendingOutput(); }; }, [resetPendingOutput]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts` around lines 206 - 211, The cleanup currently only runs on unmount so pendingOutputRef and any queued RAF can flush into a new context when the worktree/session changes; update the hook so resetPendingOutput() runs whenever the worktree/session context changes (e.g. include worktreePath and/or session identifiers in the useEffect dependency array or add a separate effect that calls resetPendingOutput on change) so any pending RAF buffer is cleared before switching to the new worktree; reference resetPendingOutput, pendingOutputRef, and the worktreePath/session context in your change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts`:
- Around line 196-201: The appendLogs path can let pendingOutputRef grow
unbounded when requestAnimationFrame callbacks are throttled; modify appendLogs
and the RAF scheduling so it also sets a fallback timer (e.g., setTimeout(...,
250)) when scheduling flushPendingOutput and stores its id (e.g., timerIdRef),
clear the timer when flushPendingOutput runs (and clear rafIdRef as already
done), and in appendLogs check pendingOutputRef.current.length (or byte/char
count) against MAX_LOG_BUFFER_SIZE and call flushPendingOutput immediately if it
reaches the limit; ensure both rafIdRef and timerIdRef are cleared/managed in
flushPendingOutput and when cancelling/rescheduling to avoid duplicate flushes.
---
Duplicate comments:
In
`@apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts`:
- Around line 206-211: The cleanup currently only runs on unmount so
pendingOutputRef and any queued RAF can flush into a new context when the
worktree/session changes; update the hook so resetPendingOutput() runs whenever
the worktree/session context changes (e.g. include worktreePath and/or session
identifiers in the useEffect dependency array or add a separate effect that
calls resetPendingOutput on change) so any pending RAF buffer is cleared before
switching to the new worktree; reference resetPendingOutput, pendingOutputRef,
and the worktreePath/session context in your change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.tsapps/ui/src/lib/http-api-client.tsapps/ui/vite.config.mts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/ui/src/lib/http-api-client.ts
- apps/ui/vite.config.mts
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts (1)
236-241:⚠️ Potential issue | 🟠 MajorReset pending output on worktree context changes, not only unmount/restart.
pendingOutputRefcan survive a switch from one non-nullworktreePathto another and flush old output into the new session. This is because cleanup at Line 236 currently runs only on unmount, while Line 243-251 does not reset pending buffers before loading the next worktree.Proposed fix
@@ - useEffect(() => { - if (worktreePath && autoSubscribe) { + useEffect(() => { + // Prevent buffered output from a previous worktree/session + // from flushing into the newly selected context. + resetPendingOutput(); + if (worktreePath && autoSubscribe) { hasFetchedInitialLogs.current = false; fetchLogs(); } else { clearLogs(); } - }, [worktreePath, autoSubscribe, fetchLogs, clearLogs]); + }, [worktreePath, autoSubscribe, fetchLogs, clearLogs, resetPendingOutput]);Also applies to: 243-251
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts` around lines 236 - 241, pendingOutputRef can leak output across sessions because resetPendingOutput is only called in the unmount cleanup of the useEffect; update the logic so pending buffers are cleared whenever the active worktree changes, not just on unmount. Specifically, ensure resetPendingOutput is invoked before switching/initializing a new worktree (e.g., call resetPendingOutput right before invoking loadWorktree or add a useEffect that depends on worktreePath and calls resetPendingOutput), referencing pendingOutputRef, resetPendingOutput, loadWorktree and the existing useEffect cleanup to prevent old output from flushing into a new session.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts`:
- Around line 236-241: pendingOutputRef can leak output across sessions because
resetPendingOutput is only called in the unmount cleanup of the useEffect;
update the logic so pending buffers are cleared whenever the active worktree
changes, not just on unmount. Specifically, ensure resetPendingOutput is invoked
before switching/initializing a new worktree (e.g., call resetPendingOutput
right before invoking loadWorktree or add a useEffect that depends on
worktreePath and calls resetPendingOutput), referencing pendingOutputRef,
resetPendingOutput, loadWorktree and the existing useEffect cleanup to prevent
old output from flushing into a new session.
…er-Org#828) * Changes from fix/dev-server-hang * fix: Address PR AutoMaker-Org#828 review feedback - Reset RAF buffer on context changes (worktree switch, dev-server restart) to prevent stale output from flushing into new sessions - Fix high-frequency WebSocket filter to catch auto-mode:event wrapping (auto_mode_progress is wrapped in auto-mode:event) and add feature:progress - Reorder Vite aliases so explicit jsx-runtime entries aren't shadowed by the broad /^react(\/|$)/ regex (Vite uses first-match-wins) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: Batch dev server logs and fix React module resolution order * feat: Add fallback timer for flushing dev server logs in background tabs --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
Changes
Server-side logging optimization (
apps/server/src/index.ts)Dev server output throttling (
apps/server/src/services/dev-server-service.ts)OUTPUT_THROTTLE_MSfrom 4ms (~250fps) to 100ms (~10fps) for more manageable WebSocket trafficOUTPUT_BATCH_SIZEfrom 4096 to 8192 bytes to compensate for lower update frequencyClient-side event batching (
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-server-logs.ts)appendLogsto batch rapid output events using requestAnimationFrame, ensuring at most one React re-render per animation frame (~60fps max)flushPendingOutputfunction that accumulates content and flushes it in RAF callbacksClient-side polling optimization (
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-servers.ts)STATE_RECONCILE_INTERVAL_MSfrom 5s to 30s since WebSocket events handle real-time updates; reconciliation is only a fallback for missed eventsuseEventRecencyStoreto signal WebSocket health to the polling systemClient-side WebSocket message logging (
apps/ui/src/lib/http-api-client.ts)Vite configuration improvements (
apps/ui/vite.config.mts)cacheDirconfiguration to keep dep-optimization cache local to apps/ui, preventing shared state across worktrees that can produce duplicate React instancesTest update (
apps/server/tests/unit/services/dev-server-event-types.test.ts)Summary by CodeRabbit
Bug Fixes
Performance
Chores
New Features