fix(staged): prevent UI freeze when opening ActionOutputModal - #623
Conversation
…rge output processChunksToLines reprocessed every chunk from scratch on each new event, causing O(total_chars × num_events) work. Combined with un-memoized ANSI rendering and no batching, opening the modal during a verbose error stream froze the app. - Add incremental line processor that only parses new chunks - Batch output events via requestAnimationFrame (one flush per frame) - Cache renderLine results (ANSI conversion + sanitization) by line text Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce105ef06a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (currentText.length > 0) { | ||
| return [...finalizedLines, { text: currentText, stream: currentStream }]; |
There was a problem hiding this comment.
Clear pending carriage-return text before returning lines
createIncrementalProcessor.process still emits currentText when a chunk ends with a bare \r, because pendingCR is left unresolved at return-time. For progress-style output like "in progress\r", that text should be considered overwritten (the legacy parser explicitly cleared it), but this path keeps it visible and it can be treated as real output in the modal/save flow until another chunk arrives. This is a behavioral regression from the existing processChunksToLines semantics for trailing bare carriage returns.
Useful? React with 👍 / 👎.
The key test ('preserves line text when \r\n is split across calls') proves
that clearing currentText on a trailing \r would break the \r\n-split-across-
chunks case — the incremental processor intentionally keeps the text visible
until the next character resolves the ambiguity.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
processChunksToLines(which reprocessed every chunk from scratch on each update) with an incremental line processor that only parses new chunksrequestAnimationFrameso rapid-fire error output causes at most one re-render per framerenderLineresults (ANSI-to-HTML conversion + sanitization) by line text to avoid redundant work on re-rendersTest plan
pnpm --filter staged checkpassespnpm --filter staged test— all 17 processOutput tests pass🤖 Generated with Claude Code