perf(macos): eliminate redundant model rebuilds in AssistantProgressView#25256
Merged
Conversation
Cache ProgressCardPresentationModel as a let binding in body so the O(n) aggregation runs once per evaluation instead of 30+ times. Extract TimelineView content into isolated child views (ProcessingDotsLabel, ElapsedTimeLabel) so periodic ticks only re-evaluate their small subtree. Move hasStrippedToolCalls into the model's build pass to avoid a separate O(n) scan. Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Tests the three key cases: stripped complete tool call (true), normal complete tool call with populated fields (false), and incomplete tool call with empty fields (false — only completed calls qualify). Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com>
Jasonnnz
approved these changes
Apr 13, 2026
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.
Summary
Fixes app hangs during scrolling caused by
AssistantProgressViewrebuilding itsProgressCardPresentationModel(an O(n) aggregation over all tool calls) 30+ times per body evaluation. The model was a computed property accessed by every sub-view, and twoTimelineViewinstances ticking at 0.4s/1.0s amplified the cost by re-evaluating the entire view tree on each tick.Three targeted changes, all internal to the progress card:
letbinding inbody— the O(n)build()runs once; all sub-views receive the cached value as a parameter instead of re-triggering the computed property.hasStrippedToolCallsintoProgressCardPresentationModel.build()— eliminates a separate O(n) scan that previously ran on every expansion check. Now computed once perbuild()call alongside the other aggregations.TimelineViewcontent into isolated child views —ProcessingDotsLabel(0.4s ticks) andElapsedTimeLabel(1.0s ticks) are now self-contained structs, so their periodic re-evaluations only touch their own small subtree, not the entire progress card.Removed the forwarding computed properties (
phase,isActive,hasChevron,hasStrippedToolCalls) that provided no value and obscured the redundant model access pattern.Zero UI/UX changes — identical rendering output, just faster.
Test coverage added for the new
hasStrippedToolCallsmodel field:hasStrippedToolCallsDetectsStrippedContent— complete tool call with all detail fields cleared →truehasStrippedToolCallsFalseForNormalToolCall— complete tool call with populatedinputFull→falsehasStrippedToolCallsFalseForIncompleteToolCall— incomplete tool call with empty fields →false(only completed calls qualify)References:
Alternatives not taken:
ViewThatFits— originally considered removing the adaptive header layout, but with the model cached, its double-measurement only reads local values instead of triggering O(n) rebuilds. Kept as-is to preserve the adaptive permission chip layout.onChangehandlers — handlers likehandlePhaseChangestill accessself.model(the computed property), but they run infrequently on state changes. Caching there would add complexity for negligible gain.Review & Testing Checklist for Human
handlePhaseChangeline 254 — the conditionif newPhase == .processing, model.phase == .processingwas previouslyphase == .processing(using the now-removed computed property). Both resolve tomodel.phase, so behavior is unchanged, but confirm the processing-start logic still fires correctly.Notes
hasStrippedToolCallsis computed via a.containscall insidebuild(), not folded into the main for-loop. This is still O(n) perbuild()call but runs once instead of on every property access — a significant improvement from the previous pattern where it ran on every expansion check._FlexFrameLayouthangs in chat cells) — this PR addresses a different bottleneck in the same area.ToolCallDatadirectly (bypassing themakeToolCallhelper) because they need explicit control overinputFull: ""— the helper defaultsinputSummaryto a non-empty string which propagates toinputFull.Link to Devin session: https://app.devin.ai/sessions/d77235ad4bd54b82b0ef489b0d3c6b52
Requested by: @Jasonnnz