perf(desktop): fix log ingest freeze and wrap log lines#683
Conversation
The main process froze under high-volume command output. Root causes, all on the ingest path rather than rendering: - Synchronous appendFileSync per line blocked the event loop. Switch LogStore to buffered async write streams (closeLog flushes on exit). - One IPC message per line flooded the channel. Coalesce lines into batched command-progress events (flush every 64ms or 250 lines) via a shared log sink; add an optional lines[] field to CommandProgress. - The renderer log buffer grew unbounded. Cap it (ring buffer, 5000 lines with hysteresis) in the detail page and create wizard. Display: rewrite LogTable to drop manual virtualization in favor of content-visibility windowing, so long lines soft-wrap while offscreen rows stay cheap. Cap rendered rows with a 'lines hidden' notice so a huge saved log can't create excessive DOM nodes. Verified with a 30k-line burst: UI stays responsive and lines wrap.
✅ Deploy Preview for images-devsy-sh canceled.
|
✅ Deploy Preview for devsydev canceled.
|
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughWorkspace command output is batched through IPC and buffered file writes, while renderer buffers and log table rendering receive memory and DOM limits. Progress payloads now support multiple lines with optional summary messages. ChangesWorkspace log streaming
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WorkspaceCommand
participant ProgressSink
participant LogStore
participant Renderer
WorkspaceCommand->>ProgressSink: Stream formatted output
ProgressSink->>LogStore: Append batched lines
ProgressSink->>Renderer: Send command-progress lines
WorkspaceCommand->>ProgressSink: Signal completion
ProgressSink->>LogStore: Close buffered stream
ProgressSink->>Renderer: Send completion metadata
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 4
🤖 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 `@desktop/src/main/ipc.ts`:
- Around line 90-93: Update done() and every workspace command completion path
to enqueue the final log line, await the asynchronous closeLog() flush, and only
then emit the sink completion event with done: true. Preserve the existing level
and CLIError handling while ensuring all listed completion sites use this
ordering.
In `@desktop/src/main/log-store.ts`:
- Around line 76-90: Update appendLog and the upstream streaming path to honor
the boolean returned by stream.write: when it returns false, pause the stdout
reader and resume it on the stream’s drain event. Thread this pause/resume
behavior through the relevant command-output reader while preserving existing
ENOENT handling and normal streaming.
In `@desktop/src/renderer/src/lib/components/log/LogTable.svelte`:
- Line 57: Update the aria-rowcount and per-row aria-rowindex bindings in
LogTable so ARIA positions account for the header row and any omitted earlier
data rows. Ensure the first visible data row receives its actual 1-based
position after applying the hidden-row offset, while preserving the header’s row
position and total row count semantics.
- Around line 81-102: Update the LogTable grid classes to use minmax(0,1fr) for
the message column, and add min-w-0 with wrap-anywhere to the message cell
containing line.message. Preserve the existing layout, styling, and whitespace
behavior while allowing long unbroken tokens to shrink and wrap without
horizontal overflow.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0804eb21-c697-44c3-8f15-ba8f03395b30
📒 Files selected for processing (7)
desktop/src/main/__tests__/log-store.test.tsdesktop/src/main/ipc.tsdesktop/src/main/log-store.tsdesktop/src/renderer/src/lib/components/log/LogTable.sveltedesktop/src/renderer/src/lib/components/workspace/WorkspaceWizard.sveltedesktop/src/renderer/src/lib/types/index.tsdesktop/src/renderer/src/pages/WorkspaceDetailPage.svelte
- LogTable: use minmax(0,1fr) for the message column plus min-w-0 and wrap-anywhere so long unbroken tokens shrink and wrap instead of forcing horizontal overflow. - LogTable: offset ARIA row positions for the header and hidden rows (aria-rowcount + 1; data rows start at hiddenCount + 2). - ipc/log-store: flush the log file (await closeLog) before emitting the done event, so a post-done read can't observe partial output.
appendLog now returns the write result and LogStore exposes onDrain. runStreaming pauses the child's stdout/stderr when onLine returns a promise (the consumer applying backpressure) and resumes on drain, so a command emitting logs faster than disk can't grow the write buffer without bound. Verified with a 50k-line burst: no deadlock, UI stays responsive.
1 participant
Summary
High-volume command output froze the main process. The bottleneck was the ingest path, not rendering:
appendFileSyncper line blocked the event loop.LogStorenow uses buffered async write streams;closeLogflushes and closes on command exit (and is awaitable so reads see the full log).command-progressevents (flush every 64 ms or 250 lines).CommandProgressgains an optionallines[]field;messageis kept for provider-init and success detection.Display
LogTabledrops manual fixed-height virtualization in favor ofcontent-visibilitywindowing: rows render in normal flow so long lines soft-wrap, while offscreen rows are skipped by the browser. A hard row cap with a "N earlier lines hidden" notice keeps a huge saved log from creating excessive DOM nodes.Verification
Drove a synthetic 30,000-line burst of long lines: the UI stayed responsive (tab-switching worked mid/post-flood) and lines wrapped correctly.
svelte-checkclean; 249 unit tests pass (updated the twolog-storetests that read right after appending to await the async flush).Addresses the logs performance/wrapping concern from #671.
Summary by CodeRabbit
New Features
Bug Fixes