Repository navigation
Custom sidebars: project PR context from per-panel state, not the focused-panel mirror - #5823
Conversation
…d on the focused-panel mirror The interpreter context for custom sidebars projects workspaces[i].pr from Workspace.pullRequest, a focused-panel mirror that only refreshes while its panel is focused. Live sessions routinely hold open PRs in panelPullRequests with a nil mirror, so status templates miss workspaces that are in review. This commit extracts the projection seam (still mirror-based, behavior unchanged) and adds the regression test, which fails until the next commit switches the seam to sidebarPullRequestsInDisplayOrder(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 (2)
📝 WalkthroughWalkthroughAdds Workspace.customSidebarPullRequestValues(), uses it in ContentView and TerminalController to populate Custom Sidebar Pull Request Values
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (18 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 |
Greptile SummaryThis PR fixes custom-sidebar templates reading stale workspace PR data. The focused-panel
Confidence Score: 5/5Safe to merge — the fix correctly routes all three consumer paths through the per-panel authoritative store, the seam is unit-tested against the exact nil-mirror regression, and no actor isolation or behavioral regressions were found. The change is narrowly scoped: a new 24-line extension, two targeted call-site swaps, and matching doc/validator updates. The two previously flagged concerns (missing second commit and dead-code seam) are both resolved in the final state. Actor isolation is sound — all call sites are on @mainactor. The template interpreter handles absent keys gracefully (returns [] for for-loop sequences when the key is missing), so the absent-prs-when-no-PRs design is safe. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph PerPanel["Per-panel store (always authoritative)"]
PP[panelPullRequests\n UUID → SidebarPullRequestState]
PG[panelGitBranches\n UUID → SidebarGitBranchState]
end
subgraph Mirror["Focused-panel mirror (stale for background workspaces)"]
WPR[workspace.pullRequest\n refreshes only while panel is focused]
WGB[workspace.gitBranch\n refreshes only while panel is focused]
end
subgraph Seam["New seam — Workspace+CustomSidebarPullRequests"]
SPR[sidebarPullRequestsInDisplayOrder\n branch-validated, ordered]
SGB[sidebarGitBranchesInDisplayOrder\n ordered, falls back to mirror]
CSV[customSidebarPullRequestValues\n → SwiftValue objects]
end
subgraph Consumers["Consumers"]
CV[ContentView\ncustomSidebarWorkspaceValue\npr + prs fields]
EW[ContentView\nextensionWorkspaceSnapshot\nbranchSummary]
TC[TerminalController\ncmux sidebar-state\nbranch_summary]
end
PP --> SPR
PG --> SPR
PG --> SGB
WGB -.->|fallback only| SGB
SPR --> CSV
CSV --> CV
SGB --> CV
SGB --> EW
SGB --> TC
WPR -.->|OLD — removed| CV
style WPR fill:#f99,stroke:#c00
style WGB fill:#f99,stroke:#c00
style CSV fill:#9f9,stroke:#090
style SPR fill:#9f9,stroke:#090
style SGB fill:#9f9,stroke:#090
Reviews (3): Last reviewed commit: "Fix remaining gitBranch mirror reads in ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func customSidebarPullRequestValues() -> [SwiftValue] { | ||
| guard let pullRequest else { return [] } | ||
| return [Self.customSidebarPullRequestValue(pullRequest)] | ||
| } |
There was a problem hiding this comment.
customSidebarPullRequestValues() guards on pullRequest (the focused-panel mirror) and returns [] when it is nil. The PR description explicitly states that commit 1 leaves this in the "old mirror-based" state and that commit 2 switches it to sidebarPullRequestsInDisplayOrder() — but git log for this branch shows only one commit (b21f7cebb). The regression test (testValuesIncludePanelPullRequestWhenFocusedPanelMirrorIsNil) deliberately sets workspace.pullRequest = nil after populating a panel PR, and expects values.count == 1. With the current implementation that test will always return [] and fail. The second commit that wires the seam to sidebarPullRequestsInDisplayOrder() is absent.
| func customSidebarPullRequestValues() -> [SwiftValue] { | ||
| guard let pullRequest else { return [] } | ||
| return [Self.customSidebarPullRequestValue(pullRequest)] | ||
| } |
There was a problem hiding this comment.
ContentView.customSidebarWorkspaceValue() (line 10740 in ContentView.swift) still builds the "pr" field inline with if let pr = workspace.pullRequest { ... }. customSidebarPullRequestValues() is defined but never called from any production code path, so even once the seam is fixed to read sidebarPullRequestsInDisplayOrder(), the interpreter context will continue to reflect the stale mirror. The call site in ContentView.swift needs to be updated to use the new seam, and the current inline projection needs to be removed.
![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 `@Sources/Workspace`+CustomSidebarPullRequests.swift:
- Around line 7-10: customSidebarPullRequestValues currently returns [] whenever
Workspace.pullRequest is nil; instead, when pullRequest is nil read the
authoritative per-panel, display-ordered PR source and map all entries to
SwiftValue using Self.customSidebarPullRequestValue. Concretely, update
customSidebarPullRequestValues() to: if let pullRequest { return
[Self.customSidebarPullRequestValue(pullRequest)] } else fetch the panel's
display-ordered PR collection (the per-panel PR source), iterate that collection
in display order and return map { Self.customSidebarPullRequestValue($0) } so
background/cold workspaces still emit PR context rather than an empty array.
Ensure you reference the same display ordering used by the UI.
🪄 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: 2133febc-15d8-46f6-aa07-6f46d1164204
📒 Files selected for processing (3)
Sources/Workspace+CustomSidebarPullRequests.swiftcmux.xcodeproj/project.pbxprojcmuxTests/WorkspaceCustomSidebarPullRequestContextTests.swift
Switch Workspace.customSidebarPullRequestValues() from the focused-panel pullRequest mirror to sidebarPullRequestsInDisplayOrder(), the same per-panel, branch-validated list the native sidebar pills use. ContentView now feeds workspaces[i].pr from its first entry and exposes the full list as workspaces[i].prs; the validator sample context and custom-sidebar docs document the new field. Fixes the green half of the regression test added in the previous commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 `@Sources/ContentView.swift`:
- Around line 10741-10744: The current block only sets fields["prs"] when
pullRequestValues.first exists, which omits an empty prs array for workspaces
with no PRs; modify the logic in the block around
pullRequestValues/firstPullRequest so that fields["pr"] remains conditional (set
only if firstPullRequest exists) but fields["prs"] is always assigned (set
fields["prs"] = .array(pullRequestValues) unconditionally regardless of
firstPullRequest). Ensure you update the code that references pullRequestValues
and firstPullRequest to reflect this change.
🪄 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: 1010324e-ba5b-48f6-8c03-4d8513c103dc
📒 Files selected for processing (4)
Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarValidator.swiftSources/ContentView.swiftSources/Workspace+CustomSidebarPullRequests.swiftdocs/custom-sidebars.md
| if let firstPullRequest = pullRequestValues.first { | ||
| fields["pr"] = firstPullRequest | ||
| fields["prs"] = .array(pullRequestValues) | ||
| } |
There was a problem hiding this comment.
Always emit prs; keep only pr conditional.
On Line 10743, fields["prs"] is currently gated by the first check. That drops workspaces[i].prs entirely for no-PR workspaces, which breaks the new context shape for empty state. Keep backward compatibility by leaving pr conditional, but set prs unconditionally.
Proposed fix
let pullRequestValues = workspace.customSidebarPullRequestValues()
+fields["prs"] = .array(pullRequestValues)
if let firstPullRequest = pullRequestValues.first {
fields["pr"] = firstPullRequest
- fields["prs"] = .array(pullRequestValues)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let firstPullRequest = pullRequestValues.first { | |
| fields["pr"] = firstPullRequest | |
| fields["prs"] = .array(pullRequestValues) | |
| } | |
| fields["prs"] = .array(pullRequestValues) | |
| if let firstPullRequest = pullRequestValues.first { | |
| fields["pr"] = firstPullRequest | |
| } |
🤖 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 `@Sources/ContentView.swift` around lines 10741 - 10744, The current block only
sets fields["prs"] when pullRequestValues.first exists, which omits an empty prs
array for workspaces with no PRs; modify the logic in the block around
pullRequestValues/firstPullRequest so that fields["pr"] remains conditional (set
only if firstPullRequest exists) but fields["prs"] is always assigned (set
fields["prs"] = .array(pullRequestValues) unconditionally regardless of
firstPullRequest). Ensure you update the code that references pullRequestValues
and firstPullRequest to reflect this change.
Audit of all Workspace.pullRequest / Workspace.gitBranch mirror readers found three more sites projecting whole-workspace state from the focused-panel gitBranch mirror: the custom-sidebar context's branch/dirty fields, the provider snapshot's branchSummary, and the v2 extension sidebar payload's branch_summary. All three now read sidebarGitBranchesInDisplayOrder().first, the same per-panel display-order source the native sidebar uses. Focused-panel reads that are intentional (command palette search metadata, probe fallback, restore fingerprint) were audited and left as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Custom-sidebar templates read
workspaces[i].prto know a workspace's pull-request state, but the interpreter context projected it fromWorkspace.pullRequest, a focused-panel mirror that only refreshes while its panel is focused. In live sessions the mirror is routinely nil for background workspaces whosepanelPullRequestshold a known open PR, so status-style sidebars (e.g. a review-lane board) silently miss workspaces that are in review. The native sidebar pills andcmux sidebar-statealready read the per-panel list and disagree with the interpreter context on the same workspace.Fix: project the context from
sidebarPullRequestsInDisplayOrder()(the same per-panel, display-ordered, branch-validated list the native pills use).prstays the first entry for backward compatibility, and a newprsarray exposes every PR cmux knows for the workspace. The projection lives in a newWorkspace.customSidebarPullRequestValues()seam so it is unit-testable; the validator's representative context anddocs/custom-sidebars.mdare updated forprs.Two-commit structure: the first commit adds the extraction seam (behavior unchanged, still mirror-based) plus the regression test, which fails red on CI; the second commit switches the seam to the per-panel list and goes green.
Localization audit: no user-facing strings changed (interpreter data fields, validator sample context, and an engineering doc only).
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes custom sidebar PR and branch context to use per-panel state in display order, matching the native sidebar and preventing missed PRs or stale branch info in background workspaces. Adds
prsto the interpreter context while keepingpras the first entry for compatibility.sidebarPullRequestsInDisplayOrder()instead of the focused-panel mirror.sidebarGitBranchesInDisplayOrder().firstfor custom-sidebarbranch/dirty, providerbranchSummary, and v2branch_summary.Workspace.customSidebarPullRequestValues()seam with tests; update validator sample context anddocs/custom-sidebars.md.Written for commit 0c264c6. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Documentation
Chores
Bug Fixes
Verification note: the hosted
testsjob cannot prove the red/green structure: its full-suite step hits the 900sCMUX_UNIT_TEST_TIMEOUT_SECONDSaround the alphabetically-early suites and passes anyway, socmuxTests/Workspace*classes (including this PR's regression test) never execute there. The red/green proof forWorkspaceCustomSidebarPullRequestContextTestswas run as targetedxcodebuild test -only-testing:invocations on the AWS M4 Pro builder against both commits; results posted in a PR comment. The suite-truncation gap itself is being filed separately.