fix: defer code review editor construction for collapsed files (APP-4518)#11281
Draft
warp-dev-github-integration[bot] wants to merge 1 commit into
Draft
fix: defer code review editor construction for collapsed files (APP-4518)#11281warp-dev-github-integration[bot] wants to merge 1 commit into
warp-dev-github-integration[bot] wants to merge 1 commit into
Conversation
) Fix memory spike in code review diff rendering where build_view_state_for_file_diffs eagerly constructs editor state (LocalCodeEditorView, CodeEditorView, CommentEditor, GlobalBufferModel) for every file diff — including collapsed files that are never rendered. Root cause: Editor and global-buffer construction happened unconditionally for all non-binary files before the expansion state was checked. For large PRs with many files, the majority are collapsed (binary, autogenerated, or large), yet each still materialized ~50-100MB of styled buffer data, pushing total in-use heap to ~9 GiB. Fix: Check the expansion state *before* creating the editor. Collapsed files now store only the lightweight content_at_head string in a new deferred_content_at_head field on FileState. The full editor is created lazily via ensure_editor_for_file() when the user expands the file via ToggleFileExpanded or FileSelected. Expected impact from heap profile: - CodeEditorView constructors: ~4.37 GiB → 0 for collapsed files - CommentEditor::new: ~3.25 GiB → 0 for collapsed files - GlobalBufferModel allocation: significant reduction for collapsed files Linked issue: APP-4518 Sentry: https://warpdotdev.sentry.io/issues/7259255054/ Co-Authored-By: Oz <oz-agent@warp.dev>
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
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
Fix a memory spike in code review diff rendering (Sentry issue #7259255054) where
CodeReviewView::build_view_state_for_file_diffseagerly constructs editor state (LocalCodeEditorView, CodeEditorView, CommentEditor, GlobalBufferModel) for every file diff — including collapsed files that are never rendered.Root cause: Editor and global-buffer construction happened unconditionally for all non-binary files. Only afterward was the expansion state checked. For large PRs with many files, the majority are collapsed (binary, autogenerated, or large), yet each still materialized ~50-100MB of styled buffer data, pushing total in-use heap to ~9 GiB.
Fix: Check the expansion state before creating the editor. Collapsed files now store only the lightweight
content_at_headstring in a newdeferred_content_at_headfield onFileState. The full editor is created lazily viaensure_editor_for_file()when the user expands the file viaToggleFileExpandedorFileSelected.Expected impact from heap profile:
CodeEditorViewconstructors: ~4.37 GiB → 0 for collapsed filesCommentEditor::new: ~3.25 GiB → 0 for collapsed filesGlobalBufferModelallocation: significant reduction for collapsed filesNote: This supersedes the approach in PRs #11275 and #11250, which attempted the same fix but remain unmerged.
Linked Issue
APP-4518
Sentry #7259255054
The linked issue is labeled
ready-to-specorready-to-implement.Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes).
Testing
Verified the change compiles cleanly with
cargo clippy -p warp --all-targets -- -D warnings✅Ran
cargo fmtwith no formatting issues ✅Existing unit tests in
code_review_view_tests.rspass (updated test to include newdeferred_content_at_headfield)The change is behavioral: collapsed files now start without editors, and editors are created on-demand when expanded. All existing expansion/selection/comment code paths that reference
editor_statealready handleNonegracefully.I have manually tested my changes locally with
./script/runAgent Mode
Conversation: https://staging.warp.dev/conversation/029ffaf3-29b0-4f69-9672-d489b97ecf9d
Run: https://oz.staging.warp.dev/runs/019e4032-1d47-7c52-bea5-60704339f9e7
This PR was generated with Oz.