Repository navigation
Put the Cloud sidebar on the right sidebar's chrome columns - #15998
Conversation
The Cloud surfaces sit inside the right sidebar but cannot import the app target that owns `RightSidebarChromeMetrics`, so each of them carries its own copy of the numbers. The copies have drifted: the Cloud banners use a 12pt outer inset against the sidebar's 8 and a 5pt vertical against its 4, and the Cloud tree reserves a 12pt trailing column against the sidebar's 6. `CloudSidebarChromeMetrics` states the sidebar's numbers once for the package. These tests run in the app target, where both types are visible, so the copy cannot drift from the original without failing. They are red until the Cloud tree is moved onto the shared trailing column. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Cloud banners sat on a 12pt outer inset while the mode bar, the Vault grouping pills and the Vault search row sit on 8, so the Cloud surfaces stepped in from the sidebar they are part of. The Cloud tree reserved a 12pt trailing column against the sidebar's 6, so machine rows stopped short of the header's controls and titles truncated 6pt early. All of these now read `CloudSidebarChromeMetrics` instead of repeating a literal. `CloudTreeLayoutMetrics.referenceInset` and the spacing lab's `referenceInset` follow `CloudTreeRowGrid`'s trailing padding rather than restating it, which is what would otherwise have let the outline document and the hosted row content drift apart on this change. The operation activity row keeps its 8pt vertical padding: it carries a progress indicator rather than a line of text, and shortening it is a look rather than a mismatch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e sidebar rows Review of #15307 caught a suite left red by the previous commit: `titleWidthReservesControls` asserted a literal 240, which is `420 - 92 - 76 - 12`. Moving `referenceInset` to the row grid's 6 makes that 246. The assertion now derives from `referenceInset` so the sidebar's chrome can move again without rewriting arithmetic here. The doc comments on `CloudTreeLayoutMetrics` and in that test claimed the outline document reserves this inset. It does not: `documentWidth` is `max(0, viewportWidth)`, and `titleWidth` has no caller outside the test. Both now say what is true, which is that the default keeps a dormant helper from being wired up at a stale number. Two more rows in the same vertical stack were still on a 10pt outer inset while the banners beside them moved to 8: the team picker's fleet status row and the "machines unavailable" notice. Aligned both. The three Cloud views that live in the app target now read `RightSidebarChromeMetrics` directly instead of the package's copy. The copy exists because CmuxCloud cannot import the app target; app-target code has no such problem, and pointing it at the copy invents drift where there was none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d inset `titleWidth(rowWidth: 180, …)` came out at exactly zero only because the reference inset was 12, so the assertion read as a clamp test and was really arithmetic. Moving the inset to the sidebar's 6 made it 6 and the test failed with no clamping behaviour changed. Now the narrow case is written against `referenceInset` like the wide one, and the clamp gets its own case at a width that actually underflows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…space scripts/lint_swift_namespaces.py rejects an all-static public surface. The metrics are now an Equatable, Sendable struct with a .sidebar instance for the numbers the app ships. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Cloud header row went on `.rightSidebarChromeBar()`, which insets it by `RightSidebarChromeMetrics.barHorizontalPadding` (8). The team-change error row sits directly under it and kept a hardcoded 10, so the message and its Close button stood 2pt inboard of the header they belong to. Read both paddings from the metric instead, which is what the rest of this branch does. The vertical value is unchanged at 4; it now names the metric rather than repeating the number. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of this branch found the tree moved to the wrong column. The commit that introduced CloudSidebarChromeMetrics took the tree's trailing column from headerTrailingPadding (6) on the theory that the header above it sits there. It does not. CloudTeamPickerHeader calls a plain rightSidebarChromeBar(), whose trailingPadding defaults to barHorizontalPadding (8). The two call sites that opt into 6 are the mode bar and the Vault grouping bar, neither of which is above the Cloud tree. So the row hover buttons went from 4pt inboard of the header's refresh and + buttons to 2pt outboard of them: still misaligned, and the doc comment claimed they lined up. The branch also contradicted itself, since the team-change error row was aligned to 8 two commits later for exactly the reason the tree was aligned to 6. The tree now follows barHorizontalPadding, so the header, the error row and the row accessories all stop on the same column. headerTrailingPadding had no reader left and is removed rather than kept as a copy documenting a relationship that does not hold. Also in the same stack, MachinesCloudStatus was still on a literal 10. It is the status slot rendered directly under the error row, so the header read 8 / 8 / 10 after the previous commit. It now names the metrics. CloudTreeMachineBand takes trailingPadding - 2, whose headroom fell from 10 to 4 when the column moved off 12. The DEBUG spacing lab's slider starts at 0, so that expression can go negative; clamped at 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
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 |
|
All contributors have signed the CLA ✍️ ✅ |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Caught up to main at e709b69 (the newest main commit with green fast guards; one newer main commit was still pending when the catch-up ran) in merge commit 8d1afbb. There were no file conflicts. The catch-up also brings in dogfood/scenarios/cloud-sidebar-audit-tour.json, so this PR can produce the before/after tour frames. GitHub now reports the PR mergeable, and CI has completed successfully. |
…metrics-v2 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…metrics-v2 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Merge receipt for
Labeled |
This is #15881 re-landed on an origin branch so it can produce the evidence the call needs. On a
fork,
pr-media.ymlreturns at once for anything but a same-repository pull request andci.yml'sui-testsjob refuses a fork on its first step, so #15881 could never attach the tour frames that adesign call is judged on. The commits are unchanged: head
5f480d560ee90ab1a42131759b0ad7dd2f5538fd, the same tree #15881 carries.Close #15881 in favor of this one.
Tour frames follow once the Cloud-sidebar scenario lands (#15923).
This re-lands #15307 on a clean branch because one commit there was authored by a bot address that CLA Assistant cannot accept. The code is unchanged apart from catching up to main.
Part of the Cloud right sidebar audit (manaflow-ai/cmuxterm-hq#853).
The Cloud surfaces live inside the right sidebar but cannot import the app target that owns
RightSidebarChromeMetrics, so each of them carries its own copy of the sidebar's numbers. The copies drifted:CloudSidebarChromeMetricsstates those numbers once for the package.CloudTreeLayoutMetricsTestsruns in the app target, where both types are visible, and fails if the copy ever disagrees with the original, so this cannot drift again silently.It also removes the drift that this change would otherwise have introduced:
CloudTreeLayoutMetrics.referenceInsetand the spacing lab'sreferenceInseteach restated12independently ofCloudTreeRowGrid.trailingPadding. The outline document and the hosted row content compute their trailing reservation separately, so had only one moved, titles would truncate before the space they were given ran out. Both now follow the row grid.Not in this PR. The operation activity row keeps its 8pt vertical padding: it carries a progress indicator rather than a line of text, so shortening it is a look, not a mismatch. The bigger Cloud alignment questions are taste calls and went to cmux#13742 with before/after shots instead: the tree reserves a disclosure-caret column before its icon (level 0 icon at x=26, text at x=46) so it structurally cannot sit on the shared content column of 12/30 without giving that column up, and section headers render with no icon slot so they sit on a different text column from their children.
Changelog
Fixed: Cloud sidebar banners and machine rows now line up with the rest of the right sidebar's columns instead of stepping in from them.
Verification
Red and green from the same focused command,
cmuxTests/CloudTreeLayoutMetricsTests:9764d00afbf(the pin, before the surfaces move onto it)8fb16647b0c, run also coveringCloudTreeCompactLayoutTests,CloudSidebarAttentionLayoutTests,CloudSidebarPinGeometryTests,CloudSidebarScaleTestsThose two SHAs are on #15307, where the runs happened. They are not on this branch. The replayed equivalents here are
c6c6a800ba6(the red pin) and13c8d3b1d67(the fix), whose contents are byte-identical to the originals.python3 scripts/verify-local.pypasses 5/5 selected checks on this branch. An earlier version of this description said it was blocked in this session; that was a missingTMPDIRdirectory, not a sandbox limit.Before/after frames to follow.
A review subagent then found two bugs in the replayed work: the tree had been put on the wrong trailing column (6 rather than the header's 8), and the fleet status row was described as aligned but never was. Both are fixed in
5f480d560ee, with the details in a comment on this PR. What no test covers: nothing measures a rendered view's inset, so the banner, notice, error row and status row changes are pinned only by the metric they read.🤖 Generated with Claude Code
Summary by cubic
Aligns the Cloud sidebar surfaces with the right sidebar's chrome columns instead of drifting from them. The Cloud package cannot import the app target that owns
RightSidebarChromeMetrics, so its surfaces each carried hardcoded copies (12pt outer insets, 5pt verticals, a 12pt tree trailing column) that had drifted from the sidebar's 8pt/4pt chrome, making Cloud banners and rows visibly step in from the sidebar they sit under.CloudSidebarChromeMetricsto state the sidebar's numbers once for the package, and points the Cloud banners, notices, status rows, and tree trailing column at it instead of literals.CloudTreeLayoutMetrics.referenceInsetand the spacing lab'sreferenceInsettoCloudTreeRowGrid.trailingPaddingso the outline document and hosted rows cannot drift apart.RightSidebarChromeMetricsdirectly rather than the package's copy.CloudTreeLayoutMetricsTests, which runs where both types are visible, so the copy cannot drift silently again.Written for commit c5628c0. Summary will update on new commits.