Mobile: rebuild PR detail view (perf + desktop parity) + Work-tab 'Open in PRs tab'#701
Conversation
…b Open-in-PRs-tab menu - Flat adaptive PR palette (desktop --pr-surface/thread/panel tokens, light-mode correct) replacing the fixed dark liquid-glass purple/pink gradients - prGlassCard/backdrop flattened: no materials, blend modes, blur or 24pt shadows inside scroll content; always-on pulse/blur halos removed - Overview thread split into per-row List items (real virtualization) with timeline built once per data change, ascending order, commit-group folding, slim commit/force-push dividers, inline lifecycle rows, tap-to-expand comment cards, cached inline-code AttributedStrings - Merge rail gains the desktop requirement checklist; metadata cards added (checks/commits/files/people incl. labels + linked issues/stack) - Projection-bump reloads now refetch detail sidecars once the 25s warm window lapses, so webhook-relay-driven host updates keep threads/activity live - Work tab: chat rows with any linked PR (mapped or GitHub-by-branch) get an 'Open in PRs tab' context-menu item routed through requestedPrNavigation - Preview fixtures for the rebuilt Overview thread (dark + light) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- delete dead PrGlassPalette.pink / .purpleSoft aliases - AI-summary +/- totals share the Files card's snapshot-file source - namespace PrInlineCodeText cache keys away from the full-markdown renderer - sheet drag handles use adaptive token instead of fixed white Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- 3 new ADETests pinning buildPrTimelineDisplayItems (fold, author split, pass-through) alongside the existing timeline order test — all green scoped - ios-companion.md: PR detail screen subsection (row order, derived models, adaptive tokens, 25s sidecar freshness gate), Work-tab menu item, stale browser-fallback claim fixed - pull-requests README: mobile freshness-gate note Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR updates PR detail and work-tab SwiftUI flows to use adaptive palette tokens, rebuilds activity and overview content into row-based components with structured merge checklist data, caches derived detail state, adds overview preview fixtures and timeline tests, and wires Work tab rows to open linked PRs. ChangesPR Detail UI Refresh
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
@copilot review but do not make fixes |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/ios/ADE/Views/PRs/PrMergeGateCard.swift (1)
58-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
highlightOpacityparameter.PrGlassCardStyleno longer reads it, andprGlassCard(...)only forwards it, so the public API suggests a customization that no longer changes rendering.🤖 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 `@apps/ios/ADE/Views/PRs/PrMergeGateCard.swift` around lines 58 - 94, Remove the unused highlightOpacity customization from PrGlassCardStyle and any call sites that forward it, since body(content:) no longer uses it. Update the prGlassCard(...) modifier/helper to stop accepting or passing highlightOpacity, and keep the remaining parameters aligned with the actual rendering behavior.
🤖 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 `@apps/ios/ADE/Views/PRs/PrDetailActivityTab.swift`:
- Around line 804-816: The Show more gate in PrDetailActivityTab should use the
same collapse criteria as the PrInlineCodeText lineLimit, not raw
character/newline checks. Update the button condition near the expanded/bodyText
logic to mirror the 10-line clamp used by Self.collapsedLineLimit so wrapped
bodies that visually exceed the limit still show the expansion control.
In `@apps/ios/ADE/Views/PRs/PrDetailOverviewTab.swift`:
- Around line 677-695: The reviewerEntries builder in PrDetailOverviewTab
currently adds requestedReviewers before reviews, so a matching login is kept as
pending even when a submitted review exists. Update reviewerEntries to prefer
the state from reviews when a reviewer has any review entry, and only fall back
to the pending requestedReviewer entry when no review is present; keep the
dedupe logic around seen and the prBotProvider(from:) filter intact.
---
Nitpick comments:
In `@apps/ios/ADE/Views/PRs/PrMergeGateCard.swift`:
- Around line 58-94: Remove the unused highlightOpacity customization from
PrGlassCardStyle and any call sites that forward it, since body(content:) no
longer uses it. Update the prGlassCard(...) modifier/helper to stop accepting or
passing highlightOpacity, and keep the remaining parameters aligned with the
actual rendering behavior.
🪄 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: CHILL
Plan: Pro
Run ID: 0df9e861-9797-40fb-b9c9-8bf5f5bf4de0
⛔ Files ignored due to path filters (3)
apps/ios/ADE.xcodeproj/project.pbxprojis excluded by!**/*.xcodeproj/project.pbxprojdocs/features/pull-requests/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**
📒 Files selected for processing (13)
apps/ios/ADE/Views/PRs/PrAiSummaryCard.swiftapps/ios/ADE/Views/PRs/PrCommitRailView.swiftapps/ios/ADE/Views/PRs/PrDetailActivityTab.swiftapps/ios/ADE/Views/PRs/PrDetailOverviewPreviews.swiftapps/ios/ADE/Views/PRs/PrDetailOverviewTab.swiftapps/ios/ADE/Views/PRs/PrDetailScreen.swiftapps/ios/ADE/Views/PRs/PrHelpers.swiftapps/ios/ADE/Views/PRs/PrListRowModifier.swiftapps/ios/ADE/Views/PRs/PrMergeGateCard.swiftapps/ios/ADE/Views/Work/WorkRootComponents.swiftapps/ios/ADE/Views/Work/WorkRootScreen+Actions.swiftapps/ios/ADE/Views/Work/WorkRootScreen.swiftapps/ios/ADETests/ADETests.swift
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…and gate - reviewerEntries: a requested reviewer with a submitted review now shows approved/changes/commented instead of being masked as pending (Greptile P1, CodeRabbit); latest review per reviewer wins - PrTimelineCommentCard: 'Show more' now appears for any body that could exceed the 10-line clamp (>280 chars or >=10 newlines) (CodeRabbit) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c09e7307b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .padding(.leading, 2) | ||
| } | ||
| } | ||
| statusLine(icon: "exclamationmark.octagon.fill", tint: ADEColor.danger, title: "Merging is blocked") |
There was a problem hiding this comment.
Keep non-checklist merge blockers visible
When prComputeMergeGate returns red because of reviewThreadsUnresolved > 0 or capabilities.mergeBlockedReason, the new rail drops model.gate.subline and removed the previous model.blockers list, while PrMergeChecklist.build has no row for unresolved threads or arbitrary blocked reasons. In that scenario the Overview rail shows only “Merging is blocked” plus passing checklist rows, so users cannot tell why the merge button is disabled.
Useful? React with 👍 / 👎.
| laneId: laneId.isEmpty ? nil : laneId | ||
| ) | ||
| } else { | ||
| syncService.requestedPrNavigation = PrNavigationRequest(prNumber: tag.githubPrNumber) |
There was a problem hiding this comment.
Preserve repo identity for GitHub-only PR navigation
When the row’s PR tag is GitHub-only (tag.prId == nil, the branch-matched-but-unmapped case), this request carries only the PR number. prNavigationTarget resolves number-only requests by taking the first matching PR number across the repo items, so in a multi-repo project where two repos both have, for example, PR #12, this context-menu action can open or link the wrong PR instead of the PR represented by the row’s tag.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified against the code and declining this one: the mobile GitHub snapshot is single-repo (GitHubPrSnapshot.repo + repoPullRequests, filtered to scope == "repo" for lane tags). The row's by-branch LanePrTag and prNavigationTarget's number-only match both resolve within that same repo-scoped list, so a cross-repo PR-number collision can't arise from this path. This also mirrors the pre-existing chat-header openLaneOpenPr fallback.
Unresolved review threads and host mergeBlockedReason have no checklist row, so a red rail could read 'Merging is blocked' over all-passing rows (Codex P2). The gate subline now renders under the red/amber status line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf2d0fd2bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| // Inline merge rail with the requirement checklist (desktop merge rail). | ||
| PrOverviewMergeRail(model: overviewMergeRailModel, checklist: mergeChecklistItems) |
There was a problem hiding this comment.
Gate merge action on failed checklist rows
When a protected PR reports status.reviewDecision == .reviewRequired/requiredApprovals but has no currently requested reviewers, mergeChecklistItems contains a failing review requirement while overviewMergeRailModel.canMerge can still be true because it only uses the older requested-reviewer count via mergeGateInfo. This new rail passes the failed checklist alongside that model, and PrOverviewMergeRail disables its primary merge button only from model.canMerge, so the mobile detail view can present the merge sheet even though the displayed checklist says review is still required. Derive canMerge from the same structured merge/checklist state, or disable when any checklist row is .fail.
Useful? React with 👍 / 👎.
Gate green is derived from requestedReviewers counts, which read 0 when reviewDecision is reviewRequired with nobody currently requested — the rail could enable merge under a visible failing checklist row (Codex P1). Green enablement (rail + sticky bar) now also requires zero failing rows; the blocked-attempt/bypass affordance is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
💡 Codex Review
When a session's stored laneId is missing or stale but its lane name/branch still resolves to a real lane, this direct lookup returns nil, so the new Open in PRs tab context-menu item is omitted even though goToLane/openPullRequest already handle that scenario through resolvedWorkNavigationLaneId(for:lanes:). Use the resolved lane id for this PR-tag lookup as well (and for the child-row lookup below) so legacy/branch-named sessions with linked PRs get the shortcut.
The primary merge button is disabled when model.canMerge is false or a PR action is busy, but this adjacent method button still opens the merge sheet; that sheet's footer calls onMerge unconditionally. In draft/review-required/check-failing or busy states, users can therefore dispatch prs.land through the chevron even though the rail shows merge as unavailable. Gate this button with the same model.canMerge && !model.isBusy condition or disable the sheet's merge action.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…vron - Work rows with a stale laneId now resolve their LanePrTag through the same name/branch fallback that navigation uses, so legacy sessions keep the 'Open in PRs tab' shortcut (Codex P2) - The merge-method chevron shares the primary button's canMerge/busy gate — the method sheet's footer executes the merge (Codex P2) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
What
Rebuilds the iOS PR detail view and adds a Work-tab context-menu shortcut to the PRs tab.
PR detail view (inside a mapped/unmapped PR)
Work tab
Tests / docs
/quality: dual review, 0 Blocker/High; 4 Low fixes applied. Builds verified via xcodebuild (iPhone 17 Pro sim).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Greptile Summary
This PR rebuilds the mobile PR detail experience and adds a Work-tab shortcut into PRs. The main changes are:
Confidence Score: 5/5
This PR appears safe to merge with low risk.
The changed PR detail, Work-tab navigation, helper logic, and tests were reviewed with no accepted bugs found.
No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Work as Work tab row participant Sync as SyncService navigation request participant PRs as PRs tab/detail router participant Detail as PrDetailView participant Host as Host sidecars Work->>Sync: requestedPrNavigation(prId or prNumber) Sync->>PRs: tab observes request PRs->>Detail: open mapped detail or GitHub-number route Detail->>Detail: seed warm cache Detail->>Host: fetch snapshot + sidecars when stale/live Host-->>Detail: threads, activity, runs, deployments, capabilities Detail->>Detail: recompute timeline + folded display rows Detail-->>PRs: render virtualized Overview list rows%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Work as Work tab row participant Sync as SyncService navigation request participant PRs as PRs tab/detail router participant Detail as PrDetailView participant Host as Host sidecars Work->>Sync: requestedPrNavigation(prId or prNumber) Sync->>PRs: tab observes request PRs->>Detail: open mapped detail or GitHub-number route Detail->>Detail: seed warm cache Detail->>Host: fetch snapshot + sidecars when stale/live Host-->>Detail: threads, activity, runs, deployments, capabilities Detail->>Detail: recompute timeline + folded display rows Detail-->>PRs: render virtualized Overview list rowsComments Outside Diff (1)
apps/ios/ADE/Views/PRs/PrDetailScreen.swift, line 285-289 (link)reviewsNeedednow uses onlyrequestedReviewers.count, but branch protection requirements live onsnapshot.status.requiredApprovalsandreviewDecision. When a protected PR requires one approval and has no current requested reviewers, this returns0,prComputeMergeGatecan go green, andoverviewMergeRailModel.canMergeenables the merge button before review approval.Artifacts
Repro: focused merge-gate harness
Repro: harness output showing required approval ignored
Repro: native Swift and Xcode toolchain availability check
Prompt To Fix With AI
Reviews (6): Last reviewed commit: "review: resolved-lane fallback for row P..." | Re-trigger Greptile