Repository navigation
Sidebar perf: kill the LazyVStack layout livelock (combined: 6019 + 6021 + 6026) - #6033
Merged
Merged
Conversation
…ivelock) Replace the LazyVStack background GeometryReader -> SidebarWorkspaceRowsHeightPreferenceKey -> @State workspaceRowsMeasurement -> emptyAreaHeight round trip with SidebarRowsFillLayout, a custom Layout that places the rows at their natural height and stretches the empty drop/tap area to fill the remaining viewport from its own concrete bounds, in one geometry pass with no state writes. The preference write during layout fed a non-converging relayout transaction: main thread pinned 100%+ in GraphHost.flushTransactions -> LazySubviewPlacements.placeSubviews -> LazyStack.place -> ForEachList.applyNodes. A fresh 2026-06-12 capture on stable 0.64.15 (which already contains the mitigations from #5708, #5846, #5855, and #5859) shows the identical signature: 128% CPU, 400 threads, debug socket refusing connections, 3344/3715 main-thread samples inside flushTransactions. The rows-height key is the last live write-during-layout edge in the sidebar after #5325 (frame anchors) and #5708 (row IDs) removed their siblings. Same approach as #5852, re-ported on top of the #5846 pixel-alignment work (contentMinHeight flooring is kept; only the empty-area math moves into the Layout). Fixes #5764. Helps #2586, #5570, #5845. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…le enum
ForEach(renderItems, id: \.id) gathers every row's identifier on each list
diff, and the sidebar re-diffs all rows per update. The previous computed
String id ("workspace.\(uuid.uuidString)") allocated and formatted a fresh
36-char string per access; SidebarWorkspaceRenderItem.id.getter was the
hottest app-owned frame in the
#5764 livelock spindump.
SidebarWorkspaceRenderItemID is a two-case enum over UUID: identity compare
and hash with zero heap allocation, and group headers can never collide with
workspace rows on the same UUID (same guarantee the string prefixes gave).
Identity values are unchanged in meaning, so row lifetime and animations are
unaffected; nothing persisted the string form (the only consumers are the
ForEach key path and scrollTo, which targets the explicit inner .id(tab.id)
UUIDs, not the ForEach identity).
Pure per-pass cost cut for #5764,
#5845,
#2586; complements the structural
loop fix in #6019.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on, eager markdown) Two changes that stop agent activity from continuously varying sidebar row heights, which kept re-feeding the sidebar-wide layout/measurement cycle at animation frame rate (#5764, #5845): 1. Remove the three implicit .animation(value:) modifiers on agent-mutable snapshot fields (latestLog, progress, metadataBlocks.count) and reduce the four height-moving .transition(.opacity.combined(.move(edge: .top))) modifiers in TabItemView's log/progress/metadata sections to .transition(.opacity). While a row-height animation runs, every frame produces a different LazyVStack content height; with dozens of agent sessions some row is always animating. Content changes now apply in one discrete layout pass. 2. SidebarMetadataMarkdownBlockRow parsed its markdown in onAppear into @State: a guaranteed nil -> attributed swap (and height change) on every first appearance of every block scrolling in. It now renders inline via a new SidebarMetadataMarkdownRenderer with a bounded (512-entry) memo cache, so the FIRST render is already attributed and appearance performs no state write and no height change. Matches the SidebarWorkspaceDescriptionText sibling, plus memoization to keep repeat body evals cheap and growth bounded. WWDC backing: lazy rows must be height-stable after appearing; initialize row state in the initializer, not onAppear (WWDC26 "Dive into lazy stacks", 321); keep body cheap / precompute (WWDC23 10160, WWDC25 306). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…values) With [String: AttributedString?], `cache[markdown] = parsed` removes the key when parsed is nil, so unparseable blocks re-parsed on every body eval and appended phantom keys to insertionOrder, mis-evicting valid entries once at capacity. Caught by Greptile on the PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o sidebar-perf-combined
…idebar-perf-combined
…o sidebar-perf-combined
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Jun 13, 2026
Closed
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR removes preference-key–driven sidebar row-height measurement and relayout loops by adding a geometric SidebarRowsFillLayout, introduces a memoized markdown renderer, changes render-item identity to a UUID-backed enum, simplifies row transitions/animation, and updates project build/test wiring. ChangesSidebar Layout Refactor and UI Optimizations
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (17 passed)
✨ 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 combines three sidebar-performance branches to eliminate a well-documented
Confidence Score: 5/5Safe to merge — the changes eliminate a confirmed non-converging layout transaction rather than patching around it, and the dogfood result shows zero flushTransactions frames on the previously locked path. All three changes address root causes rather than symptoms: the GeometryReader→PreferenceKey→@State cycle is gone, the per-call String allocation in ForEach ids is gone, and the nil→attributed height swap on first appearance is gone. The new Layout implementation is unit-covered, the cache invariants are correct (including nil-value caching for failed parses), and SidebarEmptyArea correctly accepts the layout-proposed height with its existing minHeight: 0 default. The only finding is a mildly misleading code comment about the cache's memory bound. Sources/SidebarMetadataMarkdownRenderer.swift — minor comment inaccuracy about the memory ceiling (keys vs total including AttributedString values), no behavioral issue. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["ScrollView geometry viewportHeight"] -->|explicit input| B["SidebarRowsFillLayout"]
B -->|sizeThatFits| C["workspaceRows LazyVStack"]
B -->|emptyAreaFillHeight| D["SidebarEmptyArea"]
C -->|natural height| B
F["block.markdown"] --> G["SidebarMetadataMarkdownRenderer @MainActor cache"]
G -->|cache hit| H["AttributedString"]
G -->|cache miss parse+evict| H
I["SidebarWorkspaceRenderItem"] --> J["SidebarWorkspaceRenderItemID enum over UUID"]
J --> K["ForEach LazyVStack diff"]
Reviews (3): Last reviewed commit: "Size the sidebar empty area from an expl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+22
to
+38
| struct SidebarRowsFillLayout: Layout { | ||
| func sizeThatFits(proposal: ProposedViewSize, subviews: Subviews, cache: inout ()) -> CGSize { | ||
| let resolved = proposal.replacingUnspecifiedDimensions() | ||
| let rowsHeight = subviews.first?.sizeThatFits( | ||
| ProposedViewSize(width: resolved.width, height: nil) | ||
| ).height ?? 0 | ||
| // Fill the proposed (viewport) height when the rows are shorter; grow to | ||
| // the rows' natural height when they overflow it. The parent | ||
| // `.frame(minHeight:)` supplies the viewport floor. | ||
| return CGSize(width: resolved.width, height: max(rowsHeight, resolved.height)) | ||
| } | ||
|
|
||
| func placeSubviews(in bounds: CGRect, proposal: ProposedViewSize, subviews: Subviews, cache: inout ()) { | ||
| guard let rows = subviews.first else { return } | ||
| let rowsHeight = rows.sizeThatFits( | ||
| ProposedViewSize(width: bounds.width, height: nil) | ||
| ).height |
Contributor
There was a problem hiding this comment.
rows.sizeThatFits is called twice per layout pass — once in sizeThatFits and again in placeSubviews. The Layout protocol's Cache associated type exists precisely to thread the result between the two, avoiding a redundant measurement of the LazyVStack on every layout cycle. Given that LazyVStack measurement was the root of the original livelock, keeping a cached height here is a straightforward improvement.
Suggested change
| struct SidebarRowsFillLayout: Layout { | |
| func sizeThatFits(proposal: ProposedViewSize, subviews: Subviews, cache: inout ()) -> CGSize { | |
| let resolved = proposal.replacingUnspecifiedDimensions() | |
| let rowsHeight = subviews.first?.sizeThatFits( | |
| ProposedViewSize(width: resolved.width, height: nil) | |
| ).height ?? 0 | |
| // Fill the proposed (viewport) height when the rows are shorter; grow to | |
| // the rows' natural height when they overflow it. The parent | |
| // `.frame(minHeight:)` supplies the viewport floor. | |
| return CGSize(width: resolved.width, height: max(rowsHeight, resolved.height)) | |
| } | |
| func placeSubviews(in bounds: CGRect, proposal: ProposedViewSize, subviews: Subviews, cache: inout ()) { | |
| guard let rows = subviews.first else { return } | |
| let rowsHeight = rows.sizeThatFits( | |
| ProposedViewSize(width: bounds.width, height: nil) | |
| ).height | |
| struct SidebarRowsFillLayout: Layout { | |
| struct Cache { | |
| var rowsHeight: CGFloat = 0 | |
| } | |
| func makeCache(subviews: Subviews) -> Cache { Cache() } | |
| func sizeThatFits(proposal: ProposedViewSize, subviews: Subviews, cache: inout Cache) -> CGSize { | |
| let resolved = proposal.replacingUnspecifiedDimensions() | |
| let rowsHeight = subviews.first?.sizeThatFits( | |
| ProposedViewSize(width: resolved.width, height: nil) | |
| ).height ?? 0 | |
| cache.rowsHeight = rowsHeight | |
| // Fill the proposed (viewport) height when the rows are shorter; grow to | |
| // the rows' natural height when they overflow it. The parent | |
| // `.frame(minHeight:)` supplies the viewport floor. | |
| return CGSize(width: resolved.width, height: max(rowsHeight, resolved.height)) | |
| } | |
| func placeSubviews(in bounds: CGRect, proposal: ProposedViewSize, subviews: Subviews, cache: inout Cache) { | |
| guard let rows = subviews.first else { return } | |
| let rowsHeight = cache.rowsHeight |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+21
to
+36
| private static var cache: [String: AttributedString?] = [:] | ||
| private static var insertionOrder: [String] = [] | ||
| private static let capacity = 512 | ||
|
|
||
| static func rendered(_ markdown: String) -> AttributedString? { | ||
| if let hit = cache[markdown] { | ||
| return hit | ||
| } | ||
| let parsed = try? AttributedString( | ||
| markdown: markdown, | ||
| options: .init(interpretedSyntax: .full) | ||
| ) | ||
| if insertionOrder.count >= capacity, let oldest = insertionOrder.first { | ||
| insertionOrder.removeFirst() | ||
| cache.removeValue(forKey: oldest) | ||
| } |
Contributor
There was a problem hiding this comment.
insertionOrder.removeFirst() is O(n) on every eviction because Array shifts all remaining elements. In a high-churn session where metadata blocks are constantly updated, each eviction walks all 512 entries. A ring-buffer eviction index avoids the per-eviction copy entirely while keeping the same [String: AttributedString?] cache dictionary.
Suggested change
| private static var cache: [String: AttributedString?] = [:] | |
| private static var insertionOrder: [String] = [] | |
| private static let capacity = 512 | |
| static func rendered(_ markdown: String) -> AttributedString? { | |
| if let hit = cache[markdown] { | |
| return hit | |
| } | |
| let parsed = try? AttributedString( | |
| markdown: markdown, | |
| options: .init(interpretedSyntax: .full) | |
| ) | |
| if insertionOrder.count >= capacity, let oldest = insertionOrder.first { | |
| insertionOrder.removeFirst() | |
| cache.removeValue(forKey: oldest) | |
| } | |
| private static var cache: [String: AttributedString?] = [:] | |
| private static var insertionOrder: [String] = [] | |
| private static var evictIndex: Int = 0 | |
| private static let capacity = 512 | |
| static func rendered(_ markdown: String) -> AttributedString? { | |
| if let hit = cache[markdown] { | |
| return hit | |
| } | |
| let parsed = try? AttributedString( | |
| markdown: markdown, | |
| options: .init(interpretedSyntax: .full) | |
| ) | |
| if insertionOrder.count >= capacity { | |
| let oldest = insertionOrder[evictIndex] | |
| cache.removeValue(forKey: oldest) | |
| insertionOrder[evictIndex] = markdown | |
| evictIndex = (evictIndex + 1) % capacity | |
| } else { | |
| insertionOrder.append(markdown) | |
| } |
The 512-entry cap bounded entry count but not retained bytes. Metadata blocks are agent/control-socket supplied and uncapped at this boundary, so a key churning large unique markdown could keep hundreds of big payloads alive after the workspace metadata was overwritten or cleared (worse than the old row-local @State, which released on update). Skip caching blocks over 4096 UTF-8 bytes: they parse inline each eval (rare, still attributed from the first frame), and total retained cache bytes are now bounded by capacity * maxCacheableBytes regardless of churn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Parsing >4KB blocks inline (previous commit) removed the retention but moved the cost to CPU: TabItemView.body re-runs on snapshot changes under agent churn, so a large block reparsed each time. Return nil for oversized blocks instead, so the row falls back to the existing Text(block.markdown) plain path: no parse, no retention, and height-stable (the result never changes for a given block, so no nil->attributed swap). Small blocks still cache and render as markdown. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmux.xcodeproj/project.pbxproj`:
- Line 2821: The shellScript currently mirrors CMUX_SHELL_SRC with rsync -a
which doesn't propagate removals, leaving stale dotfiles; update the
CMUX_SHELL_SRC handling to use rsync --delete for the dotfile-aware copy
(replace rsync -a "$CMUX_SHELL_SRC/." "$CMUX_SHELL_DEST/" with rsync -a --delete
"$CMUX_SHELL_SRC/." "$CMUX_SHELL_DEST/") and handle the single-file
CMUX_GHOSTTY_ZSH_SRC so deletions are propagated (if the source file is absent,
remove "$CMUX_SHELL_DEST/ghostty-integration.zsh"; if present, ensure you still
copy with rsync -a --delete or a direct copy after mkdir -p).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c4394970-2bdc-4cb0-8f00-4d1adccc2197
📒 Files selected for processing (9)
Sources/ContentView.swiftSources/SidebarMetadataMarkdownRenderer.swiftSources/SidebarRowsFillLayout.swiftSources/SidebarWorkspaceRenderItem.swiftSources/SidebarWorkspaceRowsHeightPreferenceKey.swiftSources/SidebarWorkspaceRowsMeasurement.swiftSources/WindowChromeMetrics.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SidebarWorkspaceScrollLayoutTests.swift
💤 Files with no reviewable changes (2)
- Sources/SidebarWorkspaceRowsMeasurement.swift
- Sources/SidebarWorkspaceRowsHeightPreferenceKey.swift
| runOnlyForDeploymentPostprocessing = 0; | ||
| shellPath = /bin/sh; | ||
| shellScript = "set -euo pipefail\nDEST=\"${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}\"\nGHOSTTY_DEST=\"${DEST}/ghostty\"\nTERMINFO_DEST=\"${DEST}/terminfo\"\nCMUX_SHELL_DEST=\"${DEST}/shell-integration\"\nBIN_DEST=\"${DEST}/bin\"\nSRC_SHARE=\"${SRCROOT}/ghostty/zig-out/share\"\nGHOSTTY_SRC=\"${SRC_SHARE}/ghostty\"\nTERMINFO_SRC=\"${SRC_SHARE}/terminfo\"\nFALLBACK_GHOSTTY=\"${SRCROOT}/Resources/ghostty\"\nFALLBACK_TERMINFO=\"${SRCROOT}/Resources/ghostty/terminfo\"\nTERMINFO_OVERLAY=\"${SRCROOT}/Resources/terminfo-overlay\"\nCMUX_SHELL_SRC=\"${SRCROOT}/Resources/shell-integration\"\nCMUX_GHOSTTY_ZSH_SRC=\"${SRCROOT}/ghostty/src/shell-integration/zsh/ghostty-integration\"\nBUILD_GHOSTTY_HELPER=\"${SRCROOT}/scripts/build-ghostty-cli-helper.sh\"\nGHOSTTY_HELPER_DEST=\"${BIN_DEST}/ghostty\"\nif [ -d \"$GHOSTTY_SRC\" ]; then\n mkdir -p \"$GHOSTTY_DEST\"\n rsync -a --delete \"$GHOSTTY_SRC/\" \"$GHOSTTY_DEST/\"\nelif [ -d \"$FALLBACK_GHOSTTY\" ]; then\n mkdir -p \"$GHOSTTY_DEST\"\n rsync -a --delete \"$FALLBACK_GHOSTTY/\" \"$GHOSTTY_DEST/\"\nfi\nif [ -d \"$TERMINFO_SRC\" ]; then\n mkdir -p \"$TERMINFO_DEST\"\n rsync -a --delete \"$TERMINFO_SRC/\" \"$TERMINFO_DEST/\"\nelif [ -d \"$FALLBACK_TERMINFO\" ]; then\n mkdir -p \"$TERMINFO_DEST\"\n rsync -a --delete \"$FALLBACK_TERMINFO/\" \"$TERMINFO_DEST/\"\nfi\n# Overlay any cmux-specific terminfo adjustments.\n# This intentionally does not use --delete so we only patch specific entries.\nif [ -d \"$TERMINFO_OVERLAY\" ]; then\n mkdir -p \"$TERMINFO_DEST\"\n rsync -a \"$TERMINFO_OVERLAY/\" \"$TERMINFO_DEST/\"\nfi\nif [ -d \"$CMUX_SHELL_SRC\" ]; then\n mkdir -p \"$CMUX_SHELL_DEST\"\n # Use '/.' so dotfiles like .zshenv/.zprofile are copied too.\n rsync -a \"$CMUX_SHELL_SRC/.\" \"$CMUX_SHELL_DEST/\"\nfi\nif [ -f \"$CMUX_GHOSTTY_ZSH_SRC\" ]; then\n mkdir -p \"$CMUX_SHELL_DEST\"\n rsync -a \"$CMUX_GHOSTTY_ZSH_SRC\" \"$CMUX_SHELL_DEST/ghostty-integration.zsh\"\nfi\nif [ ! -x \"$BUILD_GHOSTTY_HELPER\" ]; then\n echo \"error: missing Ghostty CLI helper build script at $BUILD_GHOSTTY_HELPER\" >&2\n exit 1\nfi\nARCHS_LIST=\" ${ARCHS:-} \"\nHAS_ARM64=0\nHAS_X86_64=0\nGHOSTTY_HELPER_TARGET=\"\"\ncase \"$ARCHS_LIST\" in\n *\" arm64 \"*) HAS_ARM64=1 ;;\nesac\ncase \"$ARCHS_LIST\" in\n *\" x86_64 \"*) HAS_X86_64=1 ;;\nesac\nif [ \"$HAS_ARM64\" -eq 1 ] && [ \"$HAS_X86_64\" -eq 1 ]; then\n \"$BUILD_GHOSTTY_HELPER\" --universal --output \"$GHOSTTY_HELPER_DEST\"\nelif [ \"$HAS_ARM64\" -eq 1 ]; then\n GHOSTTY_HELPER_TARGET=\"aarch64-macos\"\nelif [ \"$HAS_X86_64\" -eq 1 ]; then\n GHOSTTY_HELPER_TARGET=\"x86_64-macos\"\nfi\nif [ -n \"$GHOSTTY_HELPER_TARGET\" ]; then\n \"$BUILD_GHOSTTY_HELPER\" --target \"$GHOSTTY_HELPER_TARGET\" --output \"$GHOSTTY_HELPER_DEST\"\nelif [ \"$HAS_ARM64\" -eq 0 ] || [ \"$HAS_X86_64\" -eq 0 ]; then\n \"$BUILD_GHOSTTY_HELPER\" --output \"$GHOSTTY_HELPER_DEST\"\nfi\nif [ ! -x \"$GHOSTTY_HELPER_DEST\" ]; then\n echo \"error: Ghostty CLI helper was not created at $GHOSTTY_HELPER_DEST\" >&2\n exit 1\nfi\nINFO_PLIST=\"${TARGET_BUILD_DIR}/${INFOPLIST_PATH}\"\nCOMMIT=\"$(git -C \"${SRCROOT}\" rev-parse --short=9 HEAD 2>/dev/null || true)\"\nif [ -n \"$COMMIT\" ] && [ -f \"$INFO_PLIST\" ]; then\n /usr/libexec/PlistBuddy -c \"Set :CMUXCommit $COMMIT\" \"$INFO_PLIST\" >/dev/null 2>&1 || /usr/libexec/PlistBuddy -c \"Add :CMUXCommit string $COMMIT\" \"$INFO_PLIST\" >/dev/null 2>&1 || true\nfi\n"; | ||
| shellScript = "set -euo pipefail\nDEST=\"${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}\"\nGHOSTTY_DEST=\"${DEST}/ghostty\"\nTERMINFO_DEST=\"${DEST}/terminfo\"\nCMUX_SHELL_DEST=\"${DEST}/shell-integration\"\nBIN_DEST=\"${DEST}/bin\"\nSRC_SHARE=\"${SRCROOT}/ghostty/zig-out/share\"\nGHOSTTY_SRC=\"${SRC_SHARE}/ghostty\"\nTERMINFO_SRC=\"${SRC_SHARE}/terminfo\"\nFALLBACK_GHOSTTY=\"${SRCROOT}/Resources/ghostty\"\nFALLBACK_TERMINFO=\"${SRCROOT}/Resources/ghostty/terminfo\"\nTERMINFO_OVERLAY=\"${SRCROOT}/Resources/terminfo-overlay\"\nCMUX_SHELL_SRC=\"${SRCROOT}/Resources/shell-integration\"\nCMUX_GHOSTTY_ZSH_SRC=\"${SRCROOT}/ghostty/src/shell-integration/zsh/ghostty-integration\"\nBUILD_GHOSTTY_HELPER=\"${SRCROOT}/scripts/build-ghostty-cli-helper.sh\"\nGHOSTTY_HELPER_DEST=\"${BIN_DEST}/ghostty\"\nmkdir -p \"$BIN_DEST\"\nif [ -d \"$GHOSTTY_SRC\" ]; then\n mkdir -p \"$GHOSTTY_DEST\"\n rsync -a --delete \"$GHOSTTY_SRC/\" \"$GHOSTTY_DEST/\"\nelif [ -d \"$FALLBACK_GHOSTTY\" ]; then\n mkdir -p \"$GHOSTTY_DEST\"\n rsync -a --delete \"$FALLBACK_GHOSTTY/\" \"$GHOSTTY_DEST/\"\nfi\nif [ -d \"$TERMINFO_SRC\" ]; then\n mkdir -p \"$TERMINFO_DEST\"\n rsync -a --delete \"$TERMINFO_SRC/\" \"$TERMINFO_DEST/\"\nelif [ -d \"$FALLBACK_TERMINFO\" ]; then\n mkdir -p \"$TERMINFO_DEST\"\n rsync -a --delete \"$FALLBACK_TERMINFO/\" \"$TERMINFO_DEST/\"\nfi\n# Overlay any cmux-specific terminfo adjustments.\n# This intentionally does not use --delete so we only patch specific entries.\nif [ -d \"$TERMINFO_OVERLAY\" ]; then\n mkdir -p \"$TERMINFO_DEST\"\n rsync -a \"$TERMINFO_OVERLAY/\" \"$TERMINFO_DEST/\"\nfi\nif [ -d \"$CMUX_SHELL_SRC\" ]; then\n mkdir -p \"$CMUX_SHELL_DEST\"\n # Use '/.' so dotfiles like .zshenv/.zprofile are copied too.\n rsync -a \"$CMUX_SHELL_SRC/.\" \"$CMUX_SHELL_DEST/\"\nfi\nif [ -f \"$CMUX_GHOSTTY_ZSH_SRC\" ]; then\n mkdir -p \"$CMUX_SHELL_DEST\"\n rsync -a \"$CMUX_GHOSTTY_ZSH_SRC\" \"$CMUX_SHELL_DEST/ghostty-integration.zsh\"\nfi\nif [ ! -x \"$BUILD_GHOSTTY_HELPER\" ]; then\n echo \"error: missing Ghostty CLI helper build script at $BUILD_GHOSTTY_HELPER\" >&2\n exit 1\nfi\nARCHS_LIST=\" ${ARCHS:-} \"\nHAS_ARM64=0\nHAS_X86_64=0\nGHOSTTY_HELPER_TARGET=\"\"\ncase \"$ARCHS_LIST\" in\n *\" arm64 \"*) HAS_ARM64=1 ;;\nesac\ncase \"$ARCHS_LIST\" in\n *\" x86_64 \"*) HAS_X86_64=1 ;;\nesac\nif [ \"$HAS_ARM64\" -eq 1 ] && [ \"$HAS_X86_64\" -eq 1 ]; then\n \"$BUILD_GHOSTTY_HELPER\" --universal --output \"$GHOSTTY_HELPER_DEST\"\nelif [ \"$HAS_ARM64\" -eq 1 ]; then\n GHOSTTY_HELPER_TARGET=\"aarch64-macos\"\nelif [ \"$HAS_X86_64\" -eq 1 ]; then\n GHOSTTY_HELPER_TARGET=\"x86_64-macos\"\nfi\nif [ -n \"$GHOSTTY_HELPER_TARGET\" ]; then\n \"$BUILD_GHOSTTY_HELPER\" --target \"$GHOSTTY_HELPER_TARGET\" --output \"$GHOSTTY_HELPER_DEST\"\nelif [ \"$HAS_ARM64\" -eq 0 ] || [ \"$HAS_X86_64\" -eq 0 ]; then\n \"$BUILD_GHOSTTY_HELPER\" --output \"$GHOSTTY_HELPER_DEST\"\nfi\nif [ ! -x \"$GHOSTTY_HELPER_DEST\" ]; then\n echo \"error: Ghostty CLI helper was not created at $GHOSTTY_HELPER_DEST\" >&2\n exit 1\nfi\nINFO_PLIST=\"${TARGET_BUILD_DIR}/${INFOPLIST_PATH}\"\nCOMMIT=\"$(git -C \"${SRCROOT}\" rev-parse --short=9 HEAD 2>/dev/null || true)\"\nif [ -n \"$COMMIT\" ] && [ -f \"$INFO_PLIST\" ]; then\n /usr/libexec/PlistBuddy -c \"Set :CMUXCommit $COMMIT\" \"$INFO_PLIST\" >/dev/null 2>&1 || /usr/libexec/PlistBuddy -c \"Add :CMUXCommit string $COMMIT\" \"$INFO_PLIST\" >/dev/null 2>&1 || true\nfi\n"; |
There was a problem hiding this comment.
Delete removed shell-integration assets during incremental builds.
Line 2821 mirrors Resources/shell-integration with plain rsync -a, so renames/removals never get propagated to ${CMUX_SHELL_DEST}. That leaves stale dotfiles or an old ghostty-integration.zsh in the app bundle after incremental builds, which makes the packaged shell integration diverge from the source tree.
Suggested fix
-if [ -d "$CMUX_SHELL_SRC" ]; then
- mkdir -p "$CMUX_SHELL_DEST"
- # Use '/.' so dotfiles like .zshenv/.zprofile are copied too.
- rsync -a "$CMUX_SHELL_SRC/." "$CMUX_SHELL_DEST/"
-fi
-if [ -f "$CMUX_GHOSTTY_ZSH_SRC" ]; then
+if [ -d "$CMUX_SHELL_SRC" ]; then
+ mkdir -p "$CMUX_SHELL_DEST"
+ # Use '/.' so dotfiles like .zshenv/.zprofile are copied too.
+ rsync -a --delete "$CMUX_SHELL_SRC/." "$CMUX_SHELL_DEST/"
+fi
+if [ -f "$CMUX_GHOSTTY_ZSH_SRC" ]; then
mkdir -p "$CMUX_SHELL_DEST"
rsync -a "$CMUX_GHOSTTY_ZSH_SRC" "$CMUX_SHELL_DEST/ghostty-integration.zsh"
+else
+ rm -f "$CMUX_SHELL_DEST/ghostty-integration.zsh"
fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmux.xcodeproj/project.pbxproj` at line 2821, The shellScript currently
mirrors CMUX_SHELL_SRC with rsync -a which doesn't propagate removals, leaving
stale dotfiles; update the CMUX_SHELL_SRC handling to use rsync --delete for the
dotfile-aware copy (replace rsync -a "$CMUX_SHELL_SRC/." "$CMUX_SHELL_DEST/"
with rsync -a --delete "$CMUX_SHELL_SRC/." "$CMUX_SHELL_DEST/") and handle the
single-file CMUX_GHOSTTY_ZSH_SRC so deletions are propagated (if the source file
is absent, remove "$CMUX_SHELL_DEST/ghostty-integration.zsh"; if present, ensure
you still copy with rsync -a --delete or a direct copy after mkdir -p).
… proposal (autoreview P2) SidebarRowsFillLayout derived its container height from proposal.replacingUnspecifiedDimensions(). A vertical ScrollView leaves the scroll-axis height unspecified, so that fell back to a 10pt placeholder and the empty area collapsed to 0 whenever the rows fit the viewport — dropping the blank area below the last row out of the double-click/drop target. Pass the viewport height (minHeight, the floored content height the call site already computes from the scroll geometry) into the layout explicitly and size the empty area from it. New emptyAreaFillHeight(viewportHeight:rowsHeight:) overload encodes container = max(viewport, rows). Verified at runtime via temporary instrumentation (since removed): rows fit -> viewport=628 rows=421 empty=207 and rows=370 empty=258; rows overflow -> viewport=628 rows=676 empty=0. Added unit coverage for both the fit and overflow viewport paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit
added a commit
that referenced
this pull request
Jun 13, 2026
Re-sync Stack F after origin/main advanced (af32009..0289eef): - Sidebar perf: kill the LazyVStack layout livelock (#6033) — adds SidebarRowsFillLayout/SidebarMetadataMarkdownRenderer, deletes SidebarWorkspaceRows{HeightPreferenceKey,Measurement}. - iOS sign-out: local-first, offline-safe, bounded best-effort revocation (#5776). - iOS: add workspace row actions (#6022). Resolutions: - .github/swift-file-length-budget.tsv: regenerated from the merged tree via swift_file_length_budget.py --write-budget (not hunk surgery). cmuxApp.swift stays 4896 (Stack F drain preserved over main's 5462); MobileShellComposite.swift reflects merged actual 4775; main's deleted/added sidebar files reconciled. - project.pbxproj: union auto-merged, normalized, check-pbxproj clean; deleted sidebar files dropped, new files wired. Verified: no drained settings symbol is referenced by main's new app-target files (silent cross-file break check); ghostty pointer == origin/main (5697db8); CmuxControlSocket diff vs origin/main empty (no 3c-phantom). Gates green: ensure-ghosttykit, swift_file_length_budget (exit 0), lint-ios-package-conventions (exit 0), local app compile BUILD SUCCEEDED (0 errors). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit
added a commit
that referenced
this pull request
Jun 13, 2026
Brings in iOS notification dismiss-sync + unread badge (#5916), iOS workspace row actions (#6022), and the sidebar LazyVStack layout-livelock fix (#6033). Conflicts: only .github/swift-file-length-budget.tsv (regenerated via swift_file_length_budget.py --write-budget from the merged tree). project.pbxproj auto-merged clean (normalized + check-pbxproj + xcodebuild -list parse gate all pass; new Remote* package IDs unchanged, no triple-ID inflation). ghostty pointer unchanged vs origin/main. CmuxControlSocket zero-diff vs main (no 3c phantom). Silent cross-file break avoided: main's ProcessPipeReader callers stay on the app-target enum on main, while this slice's NotificationSoundSettings already routes through CmuxFoundation's FileHandle ProcessPipe extension; verified no dangling ProcessPipeReader. call sites remain and the full app target compiles (BUILD SUCCEEDED). Preserved the RemoteSessionProcessRunner @suite(.serialized) fd-recycle fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 15, 2026
Merged
lawrencecchen
added a commit
that referenced
this pull request
Jun 16, 2026
…6188) * Sidebar: remove rows measurement that re-livelocks layout at scale SidebarRowsFillLayout split the rows vs the empty drop area by calling `subviews.first?.sizeThatFits(width:, height: nil)` in both sizeThatFits and placeSubviews. Asking a LazyVStack for its natural height realizes every row synchronously on every layout pass, so at high workspace counts one `GraphHost.flushTransactions` pegs the main thread (a fresh capture on 0.64.16 sat ~4s inside a single transaction under SidebarRowsFillLayout). This is the same livelock class as #2586 / #5764 / #5845. #6033 removed the old GeometryReader+PreferenceKey @State feedback loop but replaced it with this custom Layout, which still force-measures the whole list; it was only dogfooded at 12 workspaces where the O(N) measure is cheap. Remove the custom Layout. The `.frame(minHeight:)` that already wrapped it is the correct primitive: it clamps the content to max(rowsHeight, viewport) without forcing realization. The rows stay lazy and pin to the top; the empty drop/tap area becomes a full-size background filling the same frame; the end-of-list drop indicator anchors to the rows' own bottom edge. Fill semantics (rows fit -> fill viewport, no phantom scrollbar #3241; rows overflow -> scroll) are preserved, computed natively by SwiftUI. Deletes SidebarRowsFillLayout and the now-unused emptyAreaFillHeight helpers + their unit tests; keeps contentMinHeight and its #3241 tests. * Trim comments to keep ContentView within file-length budget * Sidebar: keep end-of-list drop target below the rows, not behind them Autoreview flagged that mounting SidebarEmptyArea as the background of the framed rows spread its end-of-list drop delegate (targetTabId: nil) behind the entire row block: workspace drops in the 2pt inter-row gaps and row padding — and the whole list when rows overflow and there is no blank area at all — could resolve to the end target instead of the hovered/adjacent row. Add a rows-sized background that claims SidebarTabDragPayload drops over the rows block and no-ops them, so only the genuine blank area below the last row routes to SidebarEmptyArea (and nothing does when rows overflow). Per-row delegates render in front and still win over their own rows. Measurement-free: the blocker is sized to the lazy rows, not the filled frame. Bumps the ContentView file-length budget by the net fix lines. * Sidebar: neutralize all empty-area interactions over the rows block Follow-up to the drop-routing fix: the rows-sized blocker now also claims the double-tap-to-create gesture and Bonsplit new-workspace drops, not just workspace-reorder drops. Otherwise a double-tap or Bonsplit drop landing in the 2pt inter-row gaps, row padding, or anywhere over an overflowing list could still reach SidebarEmptyArea behind and create/route a workspace where the old SidebarRowsFillLayout placed a zero-height empty area. Physically placing the empty area below the rows requires measuring the LazyVStack height every layout pass (the realized-all-rows livelock this PR removes), so neutralizing every empty-area interaction over the rows block is the measurement-free equivalent. --------- Co-authored-by: austinpower1258 <austinwang115@gmail.com>
This was referenced Jun 26, 2026
austinywang
pushed a commit
that referenced
this pull request
Jun 26, 2026
… name Codex autoreview P2 (#6870): the guard only banned the literal deleted type name `SidebarRowsFillLayout`, so a future regression could wrap `workspaceRows(...)` in a differently named custom `Layout` (with the `subview.sizeThatFits(ProposedViewSize(... height: nil ...))` body living outside the two scanned functions) and CI would pass -- the exact #6033 shape under a new name. Generalize the guard: discover every type conforming to SwiftUI's `Layout` protocol across the whole Sources/ tree (comment/string- neutralized, pre-filtered to files that mention `Layout`), then fail if ANY of those type names is applied within `workspaceScrollContent` / `workspaceRows`. A custom Layout wrapping the LazyVStack measures it on every pass regardless of the type's name; rows must be sized by `.frame(minHeight:)` instead. The literal-name and direct force-measure token bans are kept as belt-and-suspenders. Add self-test case (d2): a `struct RowsFillLayout: Layout` (NOT the old name) whose force-measure lives in the layout type, applied to the rows in `workspaceScrollContent`; the guard must fail it. Without the generalization this is a false negative. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
austinywang
pushed a commit
that referenced
this pull request
Jun 26, 2026
Codex autoreview P2 (#6870): custom-Layout discovery scanned only Sources/, but cmux migrates app code into Packages/. A force-measuring sidebar layout defined in a repo-owned package and applied in workspaceScrollContent would not be discovered, leaving the guard bypassable exactly where code is moving. - Replace the Sources-only glob with repo_owned_swift_files(), which walks both Sources/ and Packages/ and prunes build/VCS/vendored dirs (.build, .git, DerivedData, Vendor, Pods, Carthage, node_modules, ...). External-dependency Layouts remain out of scope by design. - Add self-test case (i): repo_owned_swift_files() covers Sources/ and Packages/, discovers their Layout types, and excludes a .build/checkouts vendored Layout. - Document the guard's scope boundary: it protects the rows layout as expressed in workspaceScrollContent/workspaceRows and does not chase a force-measure relocated into an arbitrary transitively-called helper (fragile to track in a lint; such an extraction should re-review this guard). Custom Layout types are the exception chased across files, since a renamed force-measuring layout is the concrete #6033 regression. Real-repo scan stays ~2s (Layout-substring pre-filter). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
austinywang
added a commit
that referenced
this pull request
Jun 29, 2026
…#6870) * Guard sidebar LazyVStack against re-livelock at scale (#6384) The workspace-sidebar rows render as a LazyVStack inside a vertical ScrollView. Keeping that stack lazy at *measure time* is load-bearing: the sidebar is re-diffed on every workspace/telemetry update, so any code that forces SwiftUI to realize and measure the whole row list on each layout pass turns a routine update into a multi-second GraphHost.flushTransactions() main-thread livelock once enough workspaces/surfaces are open. That is exactly the ~1s beachball reported in #6384. The root cause -- SidebarRowsFillLayout, a custom Layout that called subviews.first?.sizeThatFits(ProposedViewSize(width:, height: nil)) on the LazyVStack every pass -- was removed in #6188 (#6210), and the rows are lazy again in main. But this same class of bug has now regressed four times (#2586, #5764, #5845, #6033 -> #6210/#6384) and is defended only by inline comments, which CI cannot enforce. Add a source-scan regression guard so the contract fails CI on re-introduction: - scripts/check-sidebar-lazy-layout.py neutralizes comments/string literals (the guarded functions deliberately *name* the forbidden anti-patterns in explanatory comments), extracts the bodies of workspaceScrollContent and workspaceRows from Sources/ContentView.swift, and fails if either reintroduces a whole-list measurement signature (GeometryReader, ProposedViewSize(..., nil), .sizeThatFits(, or SidebarRowsFillLayout) or drops a lazy-fill primitive the fix relies on (LazyVStack( in workspaceRows, .frame(minHeight:) in workspaceScrollContent). A renamed/removed guarded function fails loudly rather than silently skipping. - tests/test_ci_sidebar_lazy_layout_guard.py proves the guard catches the bug: it passes the real repo and a clean fixture whose comments/strings name every forbidden token, and fails synthetic fixtures for each regression mode (force-measure, reintroduced custom Layout, GeometryReader, eager VStack, missing minHeight, renamed function). - Wire the self-test into the workflow-guard-tests CI job. The drag-only drop-target reader (rowsWithGatedDropTargetReader) is not scanned: it intentionally uses a GeometryReader to resolve per-row drop anchors and is gated behind an active drag (#5325), so it never runs during the steady-state layout this guard protects. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Guard: handle Swift multi-line string literals in neutralize_swift Greptile P2 (#6870 review): neutralize_swift treated every `"` as a regular string boundary, so a Swift multi-line string `"""..."""` parsed as two empty strings plus an unclosed string. A bare `"` inside such a literal (e.g. `"""... he said "GeometryReader" ..."""`) would close the outer string early and expose the remaining content -- including a forbidden token named in prose -- as apparent code, tripping the guard with a false positive and a spurious CI failure. Add a MULTILINE_STRING tokenizer state: `"""` opens it, only a closing `"""` ends it, and a lone `"` inside is neutralized like any other string content. Add self-test case (b2) with a multi-line string containing a bare quote plus GeometryReader / sizeThatFits(ProposedViewSize(height: nil)) / SidebarRowsFillLayout, asserting the guard still passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Guard: ban any custom Layout applied to the sidebar rows, not just by name Codex autoreview P2 (#6870): the guard only banned the literal deleted type name `SidebarRowsFillLayout`, so a future regression could wrap `workspaceRows(...)` in a differently named custom `Layout` (with the `subview.sizeThatFits(ProposedViewSize(... height: nil ...))` body living outside the two scanned functions) and CI would pass -- the exact #6033 shape under a new name. Generalize the guard: discover every type conforming to SwiftUI's `Layout` protocol across the whole Sources/ tree (comment/string- neutralized, pre-filtered to files that mention `Layout`), then fail if ANY of those type names is applied within `workspaceScrollContent` / `workspaceRows`. A custom Layout wrapping the LazyVStack measures it on every pass regardless of the type's name; rows must be sized by `.frame(minHeight:)` instead. The literal-name and direct force-measure token bans are kept as belt-and-suspenders. Add self-test case (d2): a `struct RowsFillLayout: Layout` (NOT the old name) whose force-measure lives in the layout type, applied to the rows in `workspaceScrollContent`; the guard must fail it. Without the generalization this is a false negative. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Guard: discover custom Layouts across Packages/ too, document scope Codex autoreview P2 (#6870): custom-Layout discovery scanned only Sources/, but cmux migrates app code into Packages/. A force-measuring sidebar layout defined in a repo-owned package and applied in workspaceScrollContent would not be discovered, leaving the guard bypassable exactly where code is moving. - Replace the Sources-only glob with repo_owned_swift_files(), which walks both Sources/ and Packages/ and prunes build/VCS/vendored dirs (.build, .git, DerivedData, Vendor, Pods, Carthage, node_modules, ...). External-dependency Layouts remain out of scope by design. - Add self-test case (i): repo_owned_swift_files() covers Sources/ and Packages/, discovers their Layout types, and excludes a .build/checkouts vendored Layout. - Document the guard's scope boundary: it protects the rows layout as expressed in workspaceScrollContent/workspaceRows and does not chase a force-measure relocated into an arbitrary transitively-called helper (fragile to track in a lint; such an extraction should re-review this guard). Custom Layout types are the exception chased across files, since a renamed force-measuring layout is the concrete #6033 regression. Real-repo scan stays ~2s (Layout-substring pre-filter). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: cmux <cmux@cmuxs-Mac-mini.local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closed
2 of 5 tasks
This was referenced Jul 13, 2026
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.
Combines the three sidebar-performance PRs into one for a single CI/review/dogfood pass. Supersedes and closes #6019, #6021, #6026.
Context
A fresh capture on stable 0.64.15 (which already contains #5708, #5846, #5855, #5859) still hard-livelocks the sidebar: 128% CPU, 400 threads, debug socket refusing connections, 3344/3715 main-thread samples inside one never-returning
NSHostingView.beginTransaction → GraphHost.flushTransactions → LazySubviewPlacements.placeSubviews → LazyStack.place → ForEachList.applyNodes. So the merged mitigations don't close the loop at agent-heavy scale. Full investigation:docs/sidebar-lazyvstack-perf/REPORT.md(in the hq repo).The
LazyVStackitself is unchanged and stays lazy. What this removes is three different kinds of work bolted around it.What's in here
1. Remove the whole-content rows-height measurement (was #6019).
workspaceRowswrapped theLazyVStackin a.background { GeometryReader }that wrote the rows' total height intoSidebarWorkspaceRowsHeightPreferenceKeyduring layout;onPreferenceChangewrote@State, which sizedSidebarEmptyAreainside the same content. Layout → measure → write state → layout: a non-converging feedback cycle that violates "layout must never depend on a value produced by that same layout." Replaced withSidebarRowsFillLayout, a customLayoutthat derives the empty-area fill from its own concretebounds(the viewport handed down by.frame(minHeight:)), an input rather than a measured output. #3241-safe by construction (only places into finite bounds). DeletesSidebarWorkspaceRowsHeightPreferenceKey+SidebarWorkspaceRowsMeasurement.2. Allocation-free ForEach identity (was #6021).
SidebarWorkspaceRenderItem.idwas a computedString("workspace.\(uuid.uuidString)"), a heap allocation on every getter call, and the list reads ids constantly while diffing.SidebarWorkspaceRenderItem.id.getterwas the hottest app-owned frame in the #5764 spindump. Now a two-caseHashableenum overUUID(group vs workspace), zero allocation. Doesn't touch the container.3. Height-stable rows under agent churn (was #6026). Removed three implicit
.animation(value:)modifiers on agent-mutable fields and reduced four height-moving.move(edge: .top)transitions to.opacity(so a row's height stops interpolating over animation frames every time an agent updates).SidebarMetadataMarkdownBlockRowparsed markdown inonAppearinto@State(a guaranteed nil→attributed height change on first appearance); now rendered inline viaSidebarMetadataMarkdownRendererwith a bounded 512-entry memo cache, attributed from the first frame.Also carries a one-line build-script fix (
mkdir -p "$BIN_DEST"before the Ghostty-helper install) that rode along on the #6019 branch.Verification
sbperf, this exact branch): 12 workspaces + a workspace group + status pill + 60% progress + markdown metadata block, then 12 full-list drag-reorders. Socket stayed responsive (81ms reply after the drags); a 3ssampleshows the main thread parked inmach_msg2_trapwith zeroflushTransactions/placeSubviews/applyNodesframes (vs 3344/3715 in the 0.64.15 livelock). Group header, status pill, progress bar, and bold markdown all render correctly with no visual regression.SidebarWorkspaceScrollLayoutTestscovers the fill math + the sidebar scrollbar always visible — should only appear when content overflows #3241 pixel-rounding invariant.WorkspaceGroupTestscovers renderItems semantics (unchanged).Regression-test note
A two-commit red/green isn't practical for the headline fix: the failure mode is a non-converging ViewGraph transaction under live agent churn, which no unit test reproduces. The layout math is unit-covered; the behavioral gate is the dogfood above.
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes the sidebar livelock by removing
LazyVStackmeasurement/state feedback and making rows height‑stable under churn. The empty area now sizes from the explicit viewport so the sidebar fills the window without phantom scrollbars and the bottom drop/tap area stays usable when rows fit.Bug Fixes
SidebarRowsFillLayout, which sizes the empty drop/tap area from the explicit viewport height (not a layout proposal); removedSidebarWorkspaceRowsHeightPreferenceKeyandSidebarWorkspaceRowsMeasurement, and updated fill math/tests..animation(value:), simplified transitions to.opacity, and rendered markdown inline viaSidebarMetadataMarkdownRendererwith a 512‑entry cache; blocks over 4KB fall back to plain text, and failed parses are cached.Refactors
SidebarWorkspaceRenderItem.idtoSidebarWorkspaceRenderItemID(Hashable enum overUUID) to avoid per‑accessStringallocations.mkdir -p "$BIN_DEST"before installing the Ghostty helper.Written for commit f13128a. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Improvements