Conversation
Add SwiftUI views for the main dashboard experience: - DashboardView: NavigationStack with Active/Completed worktree sections, pull-to-refresh, connection status indicator, empty and disconnected states - WorktreeCard: status card with name, branch, agent count, status badge - WorktreeDetailView: inspector with agent list, diff stats, merge/kill/PR actions - AgentRow: agent status row with kill/restart action buttons Includes @observable DashboardStore protocol and domain models (Worktree, Agent, WorktreeStatus, AgentStatus) aligned with the macOS app's model layer. Closes #83
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughFour new SwiftUI view files introduce a Dashboard UI module for iOS, comprising a main dashboard view with connection state management, domain models for worktrees and agents, a DashboardStore class for state coordination, and reusable view components for displaying worktree and agent details. Changes
Sequence DiagramsequenceDiagram
actor User
participant View as Dashboard View
participant Store as DashboardStore
participant Backend as Backend/API
User->>View: Tap Action (Kill/Restart Agent)
View->>Store: killAgent(_:in:) or restartAgent(_:in:)
activate Store
Store->>Backend: Perform Async Operation
activate Backend
Backend-->>Store: Response
deactivate Backend
Store->>View: Update connectionState/worktrees
deactivate Store
View->>View: Re-render with New State
View-->>User: Display Updated UI
User->>View: Pull to Refresh
View->>Store: refresh()
activate Store
Store->>Backend: Fetch Manifest
activate Backend
Backend-->>Store: Updated Worktree Data
deactivate Backend
Store->>View: Update worktrees Array
deactivate Store
View-->>User: Display Refreshed Data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (8)
ios/PPGMobile/PPGMobile/Views/Dashboard/WorktreeCard.swift (1)
53-66: Minor inconsistency in status badge opacity.The status badge here uses
opacity(0.15)(line 63), whileAgentRow.statusLabelusesopacity(0.12)(line 63 in AgentRow.swift). Consider using a consistent opacity value across the module for visual uniformity.🎨 Proposed fix for consistency
Either change this to
0.12to matchAgentRow, or define a shared constant:-.background(worktree.status.color.opacity(0.15)) +.background(worktree.status.color.opacity(0.12))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Dashboard/WorktreeCard.swift` around lines 53 - 66, status badge opacity is inconsistent: WorktreeCard.statusBadge uses opacity(0.15) while AgentRow.statusLabel uses opacity(0.12); update WorktreeCard.statusBadge to use the same opacity (0.12) or extract a shared constant (e.g., STATUS_BADGE_OPACITY) and use it in both WorktreeCard.statusBadge and AgentRow.statusLabel so the module shares a single source of truth for the opacity value.ios/PPGMobile/PPGMobile/Views/Dashboard/DashboardView.swift (5)
151-160: Consider addingEquatableandHashableconformance toWorktree.
Worktreeis used inForEachand compared viafirst(where:). AddingEquatableenables SwiftUI to diff changes more efficiently. Since all stored properties are value types (or arrays ofIdentifiableitems), synthesis is straightforward.♻️ Proposed refinement
-struct Worktree: Identifiable { +struct Worktree: Identifiable, Equatable, Hashable { let id: String // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Dashboard/DashboardView.swift` around lines 151 - 160, The Worktree struct should conform to Equatable and Hashable so SwiftUI can diff it in ForEach and comparisons like first(where:); update the declaration of Worktree (the struct named Worktree) to adopt Equatable and Hashable (e.g. "struct Worktree: Identifiable, Equatable, Hashable") and ensure its stored types (WorktreeStatus, Agent) also support these protocols (or add conformance there) so the compiler can synthesize equality and hashing.
145-149: Consider makingConnectionStateEquatableandHashable.The enum is compared using
!=on line 33. While Swift enums without associated values are implicitlyEquatable, explicitly declaring conformance improves clarity and enables future use in collections or as dictionary keys.💡 Proposed refinement
-enum ConnectionState { +enum ConnectionState: Equatable, Hashable { case disconnected case connecting case connected }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Dashboard/DashboardView.swift` around lines 145 - 149, The enum ConnectionState should explicitly conform to Equatable and Hashable to make comparisons and future collection use explicit; update the enum declaration for ConnectionState to adopt Equatable and Hashable (e.g., add : Equatable, Hashable to the enum), rebuild to ensure no other code relies on implicit conformance, and adjust any usages if needed (the != comparison that compares ConnectionState values will continue to work).
196-206: Consider addingEquatableandHashableconformance toAgent.Similar to
Worktree,Agentis used inForEachloops and would benefit fromEquatablefor efficient diffing andHashablefor potential use in sets or dictionaries.♻️ Proposed refinement
-struct Agent: Identifiable { +struct Agent: Identifiable, Equatable, Hashable { let id: String // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Dashboard/DashboardView.swift` around lines 196 - 206, The Agent struct lacks Equatable and Hashable conformance, which hurts efficient diffing in SwiftUI ForEach and prevents using Agent in sets/dictionaries; update the Agent declaration (struct Agent: Identifiable) to also conform to Equatable and Hashable (i.e., struct Agent: Identifiable, Equatable, Hashable) and ensure any stored properties (id, name, agentType, status, prompt, startedAt, completedAt, exitCode, error) either already conform or provide custom implementations of == and hash(into:) on Agent if needed (e.g., if AgentStatus is not Hashable/Equatable), mirroring the approach used for Worktree.
247-261: DashboardStore methods are empty stubs — ensure implementation follows.The store methods (
refresh(),connect(),killAgent(), etc.) have empty bodies. The comment says "Store Protocol" butDashboardStoreis a concrete class, not a protocol. If this is intentional placeholder code for the PR, consider addingTODOcomments orfatalError("Not implemented")to make the incomplete state explicit during development.Would you like me to suggest a protocol-based design if the intent is to enable dependency injection for testing?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Dashboard/DashboardView.swift` around lines 247 - 261, The DashboardStore currently contains empty stub methods (refresh, connect, killAgent, restartAgent, mergeWorktree, killWorktree) and is declared as a concrete class though the comment says "Store Protocol"; fix by creating a protocol (e.g., DashboardStoreProtocol) that declares these async methods and properties (projectName, worktrees, connectionState), then implement a concrete type (e.g., DefaultDashboardStore or have DashboardStore conform) with real implementations or, if not yet implemented, replace empty bodies with explicit placeholders such as a TODO comment and fatalError("Not implemented in DashboardStore") to make the incomplete state explicit and enable DI for testing; ensure you reference the protocol/implementation names and the exact method signatures (refresh(), connect(), killAgent(_:in:), restartAgent(_:in:), mergeWorktree(_:), killWorktree(_:)) when updating code so callers can compile.
67-71: Navigation destination may show empty view for stale worktree IDs.If
store.worktreesis refreshed and a worktree is removed while the user is viewing the list, tapping a staleNavigationLinkwill navigate to an empty view since theif letguard silently fails. Consider providing a fallback UI or handling the missing worktree case.💡 Proposed enhancement
.navigationDestination(for: String.self) { worktreeId in if let worktree = store.worktrees.first(where: { $0.id == worktreeId }) { WorktreeDetailView(worktree: worktree, store: store) + } else { + ContentUnavailableView("Worktree Not Found", systemImage: "questionmark.folder") } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Dashboard/DashboardView.swift` around lines 67 - 71, The navigationDestination(for: String.self) handler can return an empty view when a stale worktreeId no longer exists in store.worktrees; update the handler in DashboardView to handle the missing-worktree case explicitly by returning a fallback UI (e.g., a Text("Worktree not found") or a Retry/Back view) or presenting an alert instead of silently doing nothing. Locate the navigationDestination block that uses worktreeId and WorktreeDetailView, check for let worktree = store.worktrees.first(where: { $0.id == worktreeId }), and add an else branch that constructs a clear fallback UI (or triggers navigation pop/alert) so users see a meaningful message when the worktree is removed.ios/PPGMobile/PPGMobile/Views/Dashboard/WorktreeDetailView.swift (1)
124-129: PR creation button has no action — add a TODO comment or disable if not ready.The "Create Pull Request" button has an empty action closure. Consider adding a TODO comment to make this explicit, or wire it to a store method for consistency with other actions.
💡 Proposed clarification
Button { - // PR creation — will be wired to store action + // TODO: Wire to store.createPullRequest(worktree.id) } label: { Label("Create Pull Request", systemImage: "arrow.triangle.pull") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Dashboard/WorktreeDetailView.swift` around lines 124 - 129, The Button in WorktreeDetailView currently has an empty action closure; either add a clear TODO comment inside the closure (e.g., "// TODO: wire Create Pull Request to store.createPullRequest(...)") so intent is explicit, or wire it to the appropriate store action (call store.createPullRequest(worktree) or similar) and handle result/async, and if the feature isn't ready disable the button unconditionally (e.g., set .disabled(true) or expand the existing .disabled condition). Update the Button action in WorktreeDetailView and keep the existing .disabled(worktree.status != .running && worktree.status != .merged) logic consistent with the chosen approach.ios/PPGMobile/PPGMobile/Views/Dashboard/AgentRow.swift (1)
70-95: Consider adding accessibility labels to action buttons.The kill and restart buttons use only icons without text labels. For VoiceOver users, adding
.accessibilityLabel()would improve accessibility.♿ Proposed enhancement
Button { onKill?() } label: { Image(systemName: "stop.fill") .font(.caption) .foregroundStyle(.red) } .buttonStyle(.borderless) +.accessibilityLabel("Kill agent") // ... Button { onRestart?() } label: { Image(systemName: "arrow.counterclockwise") .font(.caption) .foregroundStyle(.blue) } .buttonStyle(.borderless) +.accessibilityLabel("Restart agent")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/PPGMobile/PPGMobile/Views/Dashboard/AgentRow.swift` around lines 70 - 95, Update the icon-only buttons in the actionButtons ViewBuilder to provide VoiceOver text by adding accessibility labels (and optional hints) to the Button or Image; specifically, in AgentRow.swift inside the actionButtons computed property, add .accessibilityLabel("Kill agent") (or localized string) to the stop.fill button that invokes onKill and .accessibilityLabel("Restart agent") to the arrow.counterclockwise button that invokes onRestart (you may also add .accessibilityHint(...) and use localized strings for accessibility).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ios/PPGMobile/PPGMobile/Views/Dashboard/AgentRow.swift`:
- Around line 70-95: Update the icon-only buttons in the actionButtons
ViewBuilder to provide VoiceOver text by adding accessibility labels (and
optional hints) to the Button or Image; specifically, in AgentRow.swift inside
the actionButtons computed property, add .accessibilityLabel("Kill agent") (or
localized string) to the stop.fill button that invokes onKill and
.accessibilityLabel("Restart agent") to the arrow.counterclockwise button that
invokes onRestart (you may also add .accessibilityHint(...) and use localized
strings for accessibility).
In `@ios/PPGMobile/PPGMobile/Views/Dashboard/DashboardView.swift`:
- Around line 151-160: The Worktree struct should conform to Equatable and
Hashable so SwiftUI can diff it in ForEach and comparisons like first(where:);
update the declaration of Worktree (the struct named Worktree) to adopt
Equatable and Hashable (e.g. "struct Worktree: Identifiable, Equatable,
Hashable") and ensure its stored types (WorktreeStatus, Agent) also support
these protocols (or add conformance there) so the compiler can synthesize
equality and hashing.
- Around line 145-149: The enum ConnectionState should explicitly conform to
Equatable and Hashable to make comparisons and future collection use explicit;
update the enum declaration for ConnectionState to adopt Equatable and Hashable
(e.g., add : Equatable, Hashable to the enum), rebuild to ensure no other code
relies on implicit conformance, and adjust any usages if needed (the !=
comparison that compares ConnectionState values will continue to work).
- Around line 196-206: The Agent struct lacks Equatable and Hashable
conformance, which hurts efficient diffing in SwiftUI ForEach and prevents using
Agent in sets/dictionaries; update the Agent declaration (struct Agent:
Identifiable) to also conform to Equatable and Hashable (i.e., struct Agent:
Identifiable, Equatable, Hashable) and ensure any stored properties (id, name,
agentType, status, prompt, startedAt, completedAt, exitCode, error) either
already conform or provide custom implementations of == and hash(into:) on Agent
if needed (e.g., if AgentStatus is not Hashable/Equatable), mirroring the
approach used for Worktree.
- Around line 247-261: The DashboardStore currently contains empty stub methods
(refresh, connect, killAgent, restartAgent, mergeWorktree, killWorktree) and is
declared as a concrete class though the comment says "Store Protocol"; fix by
creating a protocol (e.g., DashboardStoreProtocol) that declares these async
methods and properties (projectName, worktrees, connectionState), then implement
a concrete type (e.g., DefaultDashboardStore or have DashboardStore conform)
with real implementations or, if not yet implemented, replace empty bodies with
explicit placeholders such as a TODO comment and fatalError("Not implemented in
DashboardStore") to make the incomplete state explicit and enable DI for
testing; ensure you reference the protocol/implementation names and the exact
method signatures (refresh(), connect(), killAgent(_:in:), restartAgent(_:in:),
mergeWorktree(_:), killWorktree(_:)) when updating code so callers can compile.
- Around line 67-71: The navigationDestination(for: String.self) handler can
return an empty view when a stale worktreeId no longer exists in
store.worktrees; update the handler in DashboardView to handle the
missing-worktree case explicitly by returning a fallback UI (e.g., a
Text("Worktree not found") or a Retry/Back view) or presenting an alert instead
of silently doing nothing. Locate the navigationDestination block that uses
worktreeId and WorktreeDetailView, check for let worktree =
store.worktrees.first(where: { $0.id == worktreeId }), and add an else branch
that constructs a clear fallback UI (or triggers navigation pop/alert) so users
see a meaningful message when the worktree is removed.
In `@ios/PPGMobile/PPGMobile/Views/Dashboard/WorktreeCard.swift`:
- Around line 53-66: status badge opacity is inconsistent:
WorktreeCard.statusBadge uses opacity(0.15) while AgentRow.statusLabel uses
opacity(0.12); update WorktreeCard.statusBadge to use the same opacity (0.12) or
extract a shared constant (e.g., STATUS_BADGE_OPACITY) and use it in both
WorktreeCard.statusBadge and AgentRow.statusLabel so the module shares a single
source of truth for the opacity value.
In `@ios/PPGMobile/PPGMobile/Views/Dashboard/WorktreeDetailView.swift`:
- Around line 124-129: The Button in WorktreeDetailView currently has an empty
action closure; either add a clear TODO comment inside the closure (e.g., "//
TODO: wire Create Pull Request to store.createPullRequest(...)") so intent is
explicit, or wire it to the appropriate store action (call
store.createPullRequest(worktree) or similar) and handle result/async, and if
the feature isn't ready disable the button unconditionally (e.g., set
.disabled(true) or expand the existing .disabled condition). Update the Button
action in WorktreeDetailView and keep the existing .disabled(worktree.status !=
.running && worktree.status != .merged) logic consistent with the chosen
approach.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ios/PPGMobile/PPGMobile/Views/Dashboard/AgentRow.swiftios/PPGMobile/PPGMobile/Views/Dashboard/DashboardView.swiftios/PPGMobile/PPGMobile/Views/Dashboard/WorktreeCard.swiftios/PPGMobile/PPGMobile/Views/Dashboard/WorktreeDetailView.swift
- Extract domain models to Models/DashboardModels.swift (was embedded in DashboardView.swift) - Add DiffStats model and Changes section to WorktreeDetailView - Fix stale worktree data: WorktreeDetailView now takes worktreeId and derives worktree from store, so refreshes propagate to detail view - Add else branch to navigationDestination for missing worktree IDs - Add confirmation dialog to individual agent kill in AgentRow - Add store.createPullRequest(for:) and wire PR button with TODO - Gate all #Preview blocks and preview helpers with #if DEBUG - Add worktree(by:) lookup helper to DashboardStore
Summary
NavigationStackwith Active/Completed worktree sections, pull-to-refresh, connection status indicator in toolbar, empty state and disconnected stateAll views use
@ObservableDashboardStorepattern and domain models (Worktree,Agent,WorktreeStatus,AgentStatus) aligned with the macOS app's model layer. Includes SwiftUI#Previewblocks for all states.Closes #83
Test plan
store.refresh()Summary by CodeRabbit