Repository navigation
Fix pane tab width regression after #4290 - #4438
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds a pixel-measurement test that renders TabBarView to verify the selected pane tab indicator is drawn and remains ≤80 pixels; it includes bitmap-sampling helpers and advances the vendored bonsplit submodule. ChangesTab Indicator Width Test and Bonsplit
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 16 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (16 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 |
![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 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bee7f46. Configure here.
Greptile SummaryThis PR fixes a visual regression from #4290 by bumping the
Confidence Score: 4/5Safe to merge once the sampleRect sampling edge is confirmed correct for the flipped NSHostingView coordinate system. No production Swift is modified — only a test file and a submodule pointer. The one open question is whether sampleRect at y=0, height=4 samples the correct edge of the flipped NSHostingView: if TabBarView places the indicator at the bottom of the view, the current sampleRect will always miss it and the test will fail spuriously rather than catching real regressions. Everything else — the nil-guard, the polling loop, the two-commit structure — looks correct. cmuxTests/PortalTabDragRoutingTests.swift — verify the sampleRect y-origin matches the actual rendering position of the selected-tab indicator inside TabBarView. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[testCompactPaneTabChromeStaysBelowDragHitMinimum] --> B[renderedSelectedPaneTabIndicatorWidth]
B --> C[Create BonsplitController + pane + TabItem]
C --> D[NSHostingView wrapping TabBarView]
D --> E[NSWindow.makeKeyAndOrderFront]
E --> F[waitForHighSaturationWidth loop\nup to 1s, 10ms ticks]
F --> G{highSaturationWidth\nfinds saturated pixels?}
G -- yes --> H[Return CGFloat width]
G -- no, timeout --> I[Return nil]
H --> J[XCTAssertGreaterThan width > 40\nproves indicator rendered]
J --> K[XCTAssertLessThanOrEqual width <= 80\npins compact chrome]
I --> L[XCTUnwrap throws - test fails]
Reviews (2): Last reviewed commit: "fix: restore compact pane tab width" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
bee7f46 to
f3b1a78
Compare
f3b1a78 to
f037070
Compare
![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 `@cmuxTests/PortalTabDragRoutingTests.swift`:
- Around line 32-44: The test passes a false negative because
highSaturationWidth returns 0 when it finds no pixels; update
highSaturationWidth to return an optional (or nil) when no qualifying pixels are
found and propagate that nil through renderedSelectedPaneTabIndicatorWidth so it
returns nil instead of 0, causing XCTUnwrap(measuredWidth) in the test to fail
when the pixel probe found nothing; modify the implementations of
highSaturationWidth and renderedSelectedPaneTabIndicatorWidth accordingly and
keep measuredWidth usage in the test unchanged.
🪄 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: 35898848-9e10-42d3-a13f-65c68658d898
📒 Files selected for processing (2)
cmuxTests/PortalTabDragRoutingTests.swiftvendor/bonsplit
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Stale bot review from an earlier head. The referenced pixel-measurement false-pass issue is fixed on f037070: highSaturationWidth now returns nil when no indicator pixels are detected, the test unwraps that optional, and it also requires the measured width to be > 40 before enforcing the compact upper bound.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cmuxTests/PortalTabDragRoutingTests.swift (1)
102-116:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWait for the width sample to settle before returning.
The helper returns the first non-
nilwidth it sees. If SwiftUI/AppKit paints the tab chrome over multiple passes, a narrow early frame can satisfy the<= 80check and let this regression test false-pass. Track the widest sample during the settle window, or require a couple of stable samples before returning.Possible fix
private func waitForHighSaturationWidth( in view: NSView, sampleRect: NSRect, timeout: TimeInterval = 1.0 ) -> CGFloat? { let deadline = Date().addingTimeInterval(timeout) + var maxWidth: CGFloat? repeat { view.layoutSubtreeIfNeeded() view.displayIfNeeded() if let width = highSaturationWidth(in: view, sampleRect: sampleRect) { - return width + maxWidth = max(maxWidth ?? 0, width) } _ = RunLoop.current.run(mode: .default, before: Date().addingTimeInterval(0.01)) } while Date() < deadline - return highSaturationWidth(in: view, sampleRect: sampleRect) + if let width = highSaturationWidth(in: view, sampleRect: sampleRect) { + maxWidth = max(maxWidth ?? 0, width) + } + return maxWidth }🤖 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 `@cmuxTests/PortalTabDragRoutingTests.swift` around lines 102 - 116, The helper waitForHighSaturationWidth currently returns the first non-nil sample and can early-exit on a narrow paint; change it to sample repeatedly until the deadline and return the widest observed width (or require N consecutive identical samples) to avoid transient narrow frames. Update waitForHighSaturationWidth to call highSaturationWidth(in: sampleRect:) on each loop iteration, track the maximum non-nil width seen (or count stable samples) while still performing view.layoutSubtreeIfNeeded()/view.displayIfNeeded(), and after the timeout return the tracked widest value (or nil if none observed).
🤖 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.
Duplicate comments:
In `@cmuxTests/PortalTabDragRoutingTests.swift`:
- Around line 102-116: The helper waitForHighSaturationWidth currently returns
the first non-nil sample and can early-exit on a narrow paint; change it to
sample repeatedly until the deadline and return the widest observed width (or
require N consecutive identical samples) to avoid transient narrow frames.
Update waitForHighSaturationWidth to call highSaturationWidth(in: sampleRect:)
on each loop iteration, track the maximum non-nil width seen (or count stable
samples) while still performing
view.layoutSubtreeIfNeeded()/view.displayIfNeeded(), and after the timeout
return the tracked widest value (or nil if none observed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b8be9163-39ef-4090-8767-5fbf67e47ad7
📒 Files selected for processing (2)
cmuxTests/PortalTabDragRoutingTests.swiftvendor/bonsplit
This branch was successfully deployed
1 participant
Fixes #4433.\n\n## Summary\n- Adds a cmux regression test that renders a short pane tab and asserts compact visible chrome stays below the drag-hit minimum.\n- Updates Bonsplit to manaflow-ai/bonsplit#127, restoring compact visual pane-tab width while preserving #4290's minimal-mode drag routing through expanded hit-test rects.\n- Keeps empty tab-strip chrome available for explicit window dragging outside the expanded tab hit area.\n\n## Regression timing\n- #4290 merged at 2026-05-20 00:36 UTC and introduced the sizing side effect while fixing minimal-mode pane-tab drag routing.\n- This PR preserves the drag routing fix and moves the affordance back into hit testing instead of visual chrome width.\n\n## Testing\n- Not run locally: per repo/user instructions, I did not run reload.sh, bare xcodebuild, or local tests.\n- Bonsplit PR #127 remote CI: tests passed.\n- cmux CI is the validation gate for this branch.\n\n## Build handoff\n- HQ should run the tagged dev build after CI is green; I will report the exact command then.
Need help on this PR? Tag
@codesmithwith what you need.Note
Low Risk
Adds a rendering-based XCTest that relies on SwiftUI/AppKit snapshot-style pixel scanning, which could be mildly flaky in CI but does not affect production code paths.
Overview
Adds a new regression test in
PortalTabDragRoutingTeststhat renders a minimal/short pane tab viaTabBarViewinside anNSHostingViewand asserts the selected-tab indicator’s visible width stays compact (<= 80pt) to prevent tab chrome from expanding to satisfy drag hit targets.The test measures width by caching the rendered view into a bitmap and scanning for high-saturation pixels, waiting briefly for SwiftUI layout/rendering to settle before asserting bounds.
Reviewed by Cursor Bugbot for commit f037070. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Restores compact pane tab width while keeping minimal‑mode drag routing intact. Adds a render-and-measure regression test to ensure the tab’s visible chrome stays narrow and drag comes from hit‑testing. Fixes #4433.
bonsplitto restore compact tab chrome and preserve drag routing via expanded hit‑test rects; keeps empty tab‑strip area for window dragging.TabBarViewand measures the selected‑tab indicator (<= 80pt) via bitmap sampling to pin compact chrome.Written for commit f037070. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Tests
Chores