Repository navigation
Keep Swift file length budget current - #6016
Closed
austinywang wants to merge 7 commits into
Closed
austinywang wants to merge 7 commits into
austinywang wants to merge 7 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds normalization and an update-gating helper to sidebar workspace row-height measurement handling; preference-key reduction and ContentView preference-change handling now avoid storing equivalent measurements. Includes a unit test verifying idempotent behavior across equivalent and different inputs. ChangesMeasurement idempotence and preference update gating
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 19 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (19 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR fixes a SwiftUI layout livelock in the sidebar workspace list by making the
Confidence Score: 5/5Safe to merge. The idempotency fix is tightly scoped to the preference-update gate, all writes go through the same normalization helper, and the new tests cover all branches. Every changed code path has direct test coverage, the normalization logic is pure and value-typed, the No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["SwiftUI layout pass\n(LazyVStack measures rows)"] --> B["SidebarWorkspaceRowsHeightPreferenceKey.reduce"]
B --> C{"nextValue?.normalizedForStorage\nis nil?"}
C -- "yes" --> D["return — keep current"]
C -- "no" --> E{"current == nil?"}
E -- "yes" --> F["value = next"]
E -- "no" --> G{"current.isEquivalent(to: next)\n(same IDs + ≤0.5pt)"}
G -- "yes" --> H["return — keep current\n(suppress churn)"]
G -- "no" --> I["value = max-height winner"]
F & I & H --> J["onPreferenceChange(measurement)"]
J --> K{"shouldStorePreferenceUpdate\n(current, preference)"}
K -- "preference nil & current nil" --> L["skip @State write"]
K -- "preference nil & current != nil" --> M["workspaceRowsMeasurement = nil"]
K -- "equivalent / jitter" --> L
K -- "real change" --> N["workspaceRowsMeasurement =\nmeasurement?.normalizedForStorage"]
M & N --> O["empty-area sizing recomputes"]
L --> P["layout converges — no re-trigger"]
Reviews (2): Last reviewed commit: "Clarify sidebar measurement reduction" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…yout-loop # Conflicts: # .github/swift-file-length-budget.tsv # Sources/ContentView.swift # Sources/SidebarWorkspaceRowsHeightPreferenceKey.swift # Sources/SidebarWorkspaceRowsMeasurement.swift # cmuxTests/SidebarWorkspaceScrollLayoutTests.swift
austinywang
changed the title
austinywang
changed the title
Contributor
Author
|
Closing this out. |
This branch was successfully deployed
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
1 participant
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.
Fixes #5764
Related same-root-cause reports: #5845, #2586, #5570.
Summary
origin/main..github/swift-file-length-budget.tsvaligned to the current measured file sizes.Diagnosis
The captured hang was a main-thread SwiftUI layout livelock over the sidebar workspace list. The representative stack is:
The runtime sidebar convergence fix is already present on the current base branch: the default workspace sidebar no longer measures the full
LazyVStackinto@Statethrough a rows-height PreferenceKey. It usesSidebarRowsFillLayout(viewportHeight:)to size the blank drop/tap area from the explicit scroll viewport, so there is no same-value preference write available to re-trigger the layout pass.File length budget
Current guard output on this branch:
The PR diff only reduces stale allowances:
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift: 4726 -> 4706Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift: 3665 -> 3663Testing
python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv.Localization
No user-facing strings changed.