fix(web): keep oversized diff lines from locking the UI#2338
Conversation
- Introduced `canRenderFileDiff` function to determine if a file diff can be displayed based on line length limits. - Updated `DiffPanel` to utilize this function, providing user feedback for large files. - Added tests for the new rendering logic to ensure proper functionality.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
size:M
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved Straightforward defensive fix that prevents UI lockup from oversized diff lines by collapsing files with lines exceeding 500k characters. Self-contained change with unit tests, no business logic impact. You can customize Macroscope's approvability policy. Learn more. |
| const renderOrder = Number(!left.canRender) - Number(!right.canRender); | ||
| if (renderOrder !== 0) { | ||
| return renderOrder; | ||
| } |
There was a problem hiding this comment.
Not sure about this, but everything else looks good
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 57d9293. Configure here.
| theme: resolveDiffThemeName(resolvedTheme), | ||
| themeType: resolvedTheme as DiffThemeType, | ||
| unsafeCSS: DIFF_PANEL_UNSAFE_CSS, | ||
| collapsed: !canRender, |
There was a problem hiding this comment.
Duplicate collapsed option key
Medium Severity
The FileDiff options object sets collapsed twice; the later collapsed: !canRender overrides the user-driven collapsed flag. Renderable files always receive collapsed: false, so manual collapse/expand no longer matches the header control or what the diff body shows.
Reviewed by Cursor Bugbot for commit 57d9293. Configure here.
| numeric: true, | ||
| sensitivity: "base", | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Wrong key in visibility effect
Medium Severity
After renderableFiles became wrapper objects, renderableFiles.map(buildFileDiffRenderKey) still passes each wrapper into buildFileDiffRenderKey instead of fileDiff. Visible keys collapse to a bogus shared value, so the effect that prunes collapsedDiffFileKeys drops legitimate per-file keys whenever the list is recomputed.
Reviewed by Cursor Bugbot for commit 57d9293. Configure here.
2 participants
What Changed
Adds a render-safety guard to the web diff panel so files with pathologically long diff lines are skipped instead of being passed into the browser diff renderer.
This includes:
canRenderFileDiffhelper indiffRenderingMAX_RENDERABLE_DIFF_LINE_LENGTHlimit for oversized added/deleted linesDiffPanelsorting and rendering behaviorRenderable diffs continue to display normally. Files that exceed the line-length threshold still appear in the diff list, but their diff body is collapsed and replaced with guidance to open the file directly.
Why
A single extremely long added or deleted line can make browser-side diff rendering expensive enough to freeze or severely degrade the UI, even when the rest of the patch is small.
This keeps the diff panel reliable under pathological inputs without rejecting or hiding the entire patch. Safe files remain visible, oversized files remain represented, and users can still inspect the full change in their editor when needed.
The implementation is intentionally localized: shared diff-rendering logic owns the renderability decision, and
DiffPaneluses that result only to avoid the unsafe render path.UI Changes
For files that are too large to render safely, the diff panel now shows:
This file is too large to display. Open the file to inspect the change.There are no layout or interaction changes for normal-sized diffs.
Checklist
Note
Low Risk
Low risk: localized UI rendering guard with a deterministic size threshold; main risk is over-collapsing diffs near the limit or unexpected edge cases in
FileDiffMetadataline arrays.Overview
Adds a render-safety guard for the diff panel by introducing
MAX_RENDERABLE_DIFF_LINE_LENGTHandcanRenderFileDiff, which flags file diffs containing pathologically long added/deleted lines.DiffPanelnow sorts unrenderable files to the bottom, forces those file diffs to render in a collapsed state, and shows a short fallback message instructing users to open the file in their editor. Unit tests cover both acceptance and rejection at the line-length threshold.Reviewed by Cursor Bugbot for commit 57d9293. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Collapse oversized diff lines in
DiffPanelto prevent UI lockupcanRenderFileDiffin diffRendering.ts that checks if any line in a file diff exceedsMAX_RENDERABLE_DIFF_LINE_LENGTH(500,000 chars).Macroscope summarized 57d9293.