ios(widgets+live activity): polish notifs, lock screen, and live activity surfaces#207
Merged
Merged
Conversation
…vity surfaces Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@copilot review but do not make fixes |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughSession state management is refactored across the iOS ADE system to segregate chat sessions into running, awaiting-input, and idle states. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Copilot stopped work on behalf of
arul28 due to an error
April 26, 2026 21:51
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
| /// user-dismiss gesture is treated uniformly — within the cooldown window, | ||
| /// no flavor of LA (ambient running roster, attention banner, count | ||
| /// summary) is allowed to come back. Silence is silence. | ||
| private func shouldStartFreshActivity( |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
The dismissedCooldown property doc (line 59-62) says "Attention signals (awaiting-input / failed / CI failing / etc.) override the cooldown because the user actually needs to see those." However, shouldStartFreshActivity accepts state as a parameter but never checks state.attention — it only checks elapsed time. The function's own doc at line 203-206 explicitly contradicts this: "no flavor of LA... is allowed to come back. Silence is silence."
If the Configuration doc is correct (attention should override), urgent signals like failed CI, awaiting user input, or review requests will be silently suppressed for 10 minutes after a user dismisses the Live Activity. If the function doc is correct, the Configuration property doc is misleading and should be updated.
// Configuration doc says attention overrides, but this never checks state.attention:
private func shouldStartFreshActivity(
for state: ADESessionAttributes.ContentState
) -> Bool {
guard let dismissedAt = lastUserDismissalAt else { return true }
if Date().timeIntervalSince(dismissedAt) >= configuration.dismissedCooldown {
lastUserDismissalAt = nil
return true
}
return false // attention signals are silently suppressed
}Either add if state.attention != nil { return true } before the cooldown check, or update the Configuration.dismissedCooldown doc to remove the override promise.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ios/ADEWidgets/ADEWidgetPreviewData.swift (1)
63-68:⚠️ Potential issue | 🟡 Minor
populatedSnapshotno longer covers the waiting-state surfaces.
sampleAgentsstill contains an awaiting-input session, but this snapshot leaves the new aggregate counts at their default0values. Most widget previews that usepopulatedSnapshotwill therefore miss the waiting/standby UI path entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADEWidgets/ADEWidgetPreviewData.swift` around lines 63 - 68, populatedSnapshot currently uses sampleAgents (which includes an awaiting-input session) but leaves WorkspaceSnapshot aggregate counts at their defaults, so waiting/standby UI paths are not exercised; update the populatedSnapshot construction to set the snapshot's aggregate/waiting counts to reflect sampleAgents' awaiting-input session (either by passing the correct aggregate parameters to the WorkspaceSnapshot initializer or by computing and assigning the counts from sampleAgents before creating populatedSnapshot) so previews using populatedSnapshot will render the waiting/standby surfaces.apps/ios/ADEWidgets/ADELiveActivityViews.swift (1)
307-336:⚠️ Potential issue | 🟡 MinorHonor
accessibilityReduceMotionfor the expanded-leading pulse.
WorkspaceCompactLeading,LockRosterRow,ExpandedRosterStrip, andFocusedCardBottomall read@Environment(\.accessibilityReduceMotion)and disable the pulse when reduce-motion is on.WorkspaceExpandedLeadingis the odd one out — line 322 hardcodespulse: true, so users with Reduce Motion enabled still see the phased halo animation here.♿ Proposed fix
struct WorkspaceExpandedLeading: View { let state: ADESessionAttributes.ContentState + `@Environment`(\.accessibilityReduceMotion) private var reduceMotion var body: some View { Group { if let attention = state.attention { … } else if !state.sessions.isEmpty { - ActiveDotMini(color: ADESharedTheme.statusSuccess, pulse: true) + ActiveDotMini(color: ADESharedTheme.statusSuccess, pulse: !reduceMotion) .scaleEffect(1.4) .frame(width: 28, height: 28)As per coding guidelines, this falls under "proper SwiftUI patterns" for
apps/ios/**/*.swift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADEWidgets/ADELiveActivityViews.swift` around lines 307 - 336, WorkspaceExpandedLeading currently hardcodes the pulsing halo by passing pulse: true to ActiveDotMini, which ignores the user's Reduce Motion setting; add an `@Environment`(\.accessibilityReduceMotion) private var accessibilityReduceMotion to the WorkspaceExpandedLeading view and change the ActiveDotMini initializer to pulse: !accessibilityReduceMotion (i.e., disable the pulse when accessibilityReduceMotion is true) so behavior matches other components like WorkspaceCompactLeading and LockRosterRow.
🧹 Nitpick comments (1)
apps/ios/ADEWidgets/ADEWorkspaceWidgetViews.swift (1)
332-334:WorkspaceIdleStatecarries dead branches under its only call site.
WorkspaceIdleStateis rendered only inside the gate at lines 332-334, which already requiresawaitingInputCount == 0 && idleCount == 0and an emptyopenPrs. Inside the view:
idleTitle(lines 446-452): theawaitingInputCount > 0 || idleCount > 0branch is unreachable, so it always returns"ADE · idle".idleSummary(lines 454-468): every part is gated on counts that are guaranteed0plusopenPrs > 0which is alsofalse, so it always returnsniland theif let summary = idleSummary { … }block at lines 433-440 never renders.var snapshot: WorkspaceSnapshot? = nil(line 418): the default is never used since the call site always passes a snapshot.Either drop the dead "standby"/summary code (and the optional snapshot), or loosen the parent gate to
runningAgents.isEmpty && openPrs.isEmptyso this view actually gets to render its richer states.♻️ Option A — trim to current behavior
private struct WorkspaceIdleState: View { let accented: Bool - var snapshot: WorkspaceSnapshot? = nil var body: some View { - VStack(alignment: .leading, spacing: 6) { + VStack(alignment: .leading, spacing: 6) { Spacer(minLength: 0) HStack(spacing: 8) { Circle() .fill(accented ? Color.primary.opacity(0.5) : WorkspaceWidgetPalette.textQuaternary) .frame(width: 8, height: 8) - Text(idleTitle) + Text("ADE · idle") .font(.system(size: 13.5, weight: .semibold)) .kerning(-0.1) .lineLimit(1) .foregroundStyle(accented ? Color.primary : WorkspaceWidgetPalette.textPrimary) } - if let summary = idleSummary { … } Spacer(minLength: 0) } .frame(maxWidth: .infinity, alignment: .leading) } - - private var idleTitle: String { … } - private var idleSummary: String? { … } }♻️ Option B — keep the rich view, expand the gate
- if snapshot.runningAgents.isEmpty && openPrs.isEmpty - && snapshot.awaitingInputCount == 0 && snapshot.idleCount == 0 { + if snapshot.runningAgents.isEmpty && openPrs.isEmpty { WorkspaceIdleState(accented: accented, snapshot: snapshot) } else {Also applies to: 416-468
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADEWidgets/ADEWorkspaceWidgetViews.swift` around lines 332 - 334, WorkspaceIdleState contains unreachable branches because its only call site already guarantees awaitingInputCount == 0, idleCount == 0 and openPrs.isEmpty; to fix, trim the dead standby/summary code and the unnecessary optional snapshot: update WorkspaceIdleState to accept a non-optional snapshot (change var snapshot: WorkspaceSnapshot? = nil to snapshot: WorkspaceSnapshot), simplify idleTitle to the single `"ADE · idle"` branch (remove the awaitingInputCount/idleCount conditional), remove idleSummary logic (or make it always nil) and delete the if-let summary rendering block, and update any initializers/uses accordingly since callers already pass a snapshot; alternatively, if you prefer to keep the richer UI instead, loosen the parent gate to only require runningAgents.isEmpty && openPrs.isEmpty so the existing branches can become reachable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ios/ADE/Services/LiveActivityCoordinator.swift`:
- Around line 196-216: The cooldown gate in shouldStartFreshActivity currently
only checks lastUserDismissalAt and dismissedCooldown, ignoring urgent attention
signals; update shouldStartFreshActivity(for:) to return true immediately if the
incoming ADESessionAttributes.ContentState contains a non-nil/priority attention
(e.g., check state.attention or the same predicate used by selectAttention), so
attention overrides the cooldown, and clear lastUserDismissalAt when bypassing;
also revise the surrounding comment to reflect that attention-level events can
resurrect the Live Activity despite a recent user dismissal.
In `@apps/ios/ADE/Shared/ADESharedModels.swift`:
- Around line 130-138: The decoder currently defaults awaitingInputCount and
idleCount to 0 when keys are missing; change init(from decoder: Decoder) so that
if decodeIfPresent returns nil you derive those counts from the decoded agents
array instead of zeroing them — e.g. compute awaitingInputCount by counting
AgentSnapshot entries in agents that indicate an awaiting-input state and
idleCount by counting entries indicating idle (use the AgentSnapshot
status/flags your model exposes), then assign those derived values; keep
existing behavior when the keys are present. Ensure this preserves existing
fields generatedAt, agents, prs, connection and interacts correctly with
runningAgents logic.
In `@apps/ios/ADEWidgets/ADEWidgetPreviewData.swift`:
- Around line 72-76: The checked-in preview fixtures in ADEWidgetPreviewData
(the real ADE data block and the larger preview dataset around the MARK comment
and lines 78–216) contain developer-specific paths, PR titles, branch names and
real UUIDs; replace that hard-coded real workspace data with sanitized/generated
sample data: remove the local filesystem path and any real strings, replace
UUIDs with freshly generated placeholders (e.g., UUID()), replace
PR/branch/title strings with generic examples, or construct the preview data via
a small factory method (e.g., makeSampleADEPreviewData()) used by the previews
so fixtures are deterministic and non-sensitive while preserving shape and types
used by the widget preview code in ADEWidgetPreviewData.
In `@apps/ios/ADEWidgets/ADEWorkspaceWidgetViews.swift`:
- Line 117: The UI currently checks agent.status == "failed" (e.g.,
SmallStatusDot(failed: focus?.status == "failed"), smallStatusLabel, rowDotColor
and the inline "failed" Text) but WorkspaceSnapshot.runningAgents filters out
failed agents, making those branches dead; choose one of two fixes: (A) keep
roster-level failed rendering — modify WorkspaceSnapshot.runningAgents to stop
filtering out status.lowercased() == "failed" (only filter out
idle/ended/completed) so focusAgent(), WorkspaceMediumView and
WorkspaceLargeView can surface recently-failed agents, or (B) remove the dead UI
— delete/stop-setting the failed flag in SmallStatusDot, remove the failed
branch in smallStatusLabel, the inline failed Text, and the failed branch in
rowDotColor so the roster matches the current runningAgents behavior; update
corresponding uses in focusAgent(), SmallStatusDot, smallStatusLabel,
WorkspaceMediumView and WorkspaceLargeView accordingly.
---
Outside diff comments:
In `@apps/ios/ADEWidgets/ADELiveActivityViews.swift`:
- Around line 307-336: WorkspaceExpandedLeading currently hardcodes the pulsing
halo by passing pulse: true to ActiveDotMini, which ignores the user's Reduce
Motion setting; add an `@Environment`(\.accessibilityReduceMotion) private var
accessibilityReduceMotion to the WorkspaceExpandedLeading view and change the
ActiveDotMini initializer to pulse: !accessibilityReduceMotion (i.e., disable
the pulse when accessibilityReduceMotion is true) so behavior matches other
components like WorkspaceCompactLeading and LockRosterRow.
In `@apps/ios/ADEWidgets/ADEWidgetPreviewData.swift`:
- Around line 63-68: populatedSnapshot currently uses sampleAgents (which
includes an awaiting-input session) but leaves WorkspaceSnapshot aggregate
counts at their defaults, so waiting/standby UI paths are not exercised; update
the populatedSnapshot construction to set the snapshot's aggregate/waiting
counts to reflect sampleAgents' awaiting-input session (either by passing the
correct aggregate parameters to the WorkspaceSnapshot initializer or by
computing and assigning the counts from sampleAgents before creating
populatedSnapshot) so previews using populatedSnapshot will render the
waiting/standby surfaces.
---
Nitpick comments:
In `@apps/ios/ADEWidgets/ADEWorkspaceWidgetViews.swift`:
- Around line 332-334: WorkspaceIdleState contains unreachable branches because
its only call site already guarantees awaitingInputCount == 0, idleCount == 0
and openPrs.isEmpty; to fix, trim the dead standby/summary code and the
unnecessary optional snapshot: update WorkspaceIdleState to accept a
non-optional snapshot (change var snapshot: WorkspaceSnapshot? = nil to
snapshot: WorkspaceSnapshot), simplify idleTitle to the single `"ADE · idle"`
branch (remove the awaitingInputCount/idleCount conditional), remove idleSummary
logic (or make it always nil) and delete the if-let summary rendering block, and
update any initializers/uses accordingly since callers already pass a snapshot;
alternatively, if you prefer to keep the richer UI instead, loosen the parent
gate to only require runningAgents.isEmpty && openPrs.isEmpty so the existing
branches can become reachable.
🪄 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: 00b77a1b-7340-4754-a15e-dc40902294a0
📒 Files selected for processing (11)
apps/ios/ADE/Services/LiveActivityCoordinator.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Shared/ADESharedContainer.swiftapps/ios/ADE/Shared/ADESharedModels.swiftapps/ios/ADEWidgets/ADEControlWidget.swiftapps/ios/ADEWidgets/ADELiveActivity.swiftapps/ios/ADEWidgets/ADELiveActivityPreviews.swiftapps/ios/ADEWidgets/ADELiveActivityViews.swiftapps/ios/ADEWidgets/ADELockScreenWidget.swiftapps/ios/ADEWidgets/ADEWidgetPreviewData.swiftapps/ios/ADEWidgets/ADEWorkspaceWidgetViews.swift
…shot decode, scrub fixtures, drop dead failed UI) - LiveActivityCoordinator: attention signals override the dismissal cooldown - ADESharedModels: derive awaiting/idle counts from agents when keys absent - ADEWidgetPreviewData: scrub real workspace data from preview fixtures - ADEWorkspaceWidgetViews: remove unreachable failed-agent UI branches Addresses: 3144225338, 3144229701, 3144229739, 3144229740, 3144229741, 3144229742 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
April 26, 2026 22:19
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Style
New Features
Greptile Summary
This PR refactors the iOS widget and Live Activity surfaces to support a three-state session model (running / awaiting-input / idle), replacing per-session awaiting-input roster rows with aggregate count chips, and polishing the visual language across Dynamic Island, Lock Screen, and home widget views.
activeSessionssemantic change inSyncService: the field now stores all live chat sessions (running + awaiting-input + idle), but the newly added doc-comment still reads "Roster only — sessions whose runtime is actively producing output", which directly contradicts its actual contents and could silently break existing consumers that relied on the old "running/awaiting only" contract.Confidence Score: 4/5
Safe to merge with one doc-comment fix; the semantic change to
activeSessionsis the main risk.One P1 finding: the
activeSessionsdoc-comment contradicts its new contents and could mislead future consumers. The rest of the changes are well-structured visual/layout work with correct backward-compatible decoders and no security concerns.apps/ios/ADE/Services/SyncService.swift— verify all existing consumers ofactiveSessionshandle the expanded set (running + awaiting + idle) correctly after this change.Important Files Changed
activeSessionssemantics changed to include all live sessions (not just running) with a misleading "Roster only" doc-comment that could break existing consumers.awaitingInputCount/idleCountfields with backward-compatible custom decoder and arunningAgentscomputed filter; filter is a negative exclusion list that doesn't explicitly guard againststatus == "awaiting_input"whenawaitingInput == false.activityStateTask) and passes the newawaitingInputCount/idleCountthrough reconcile; correctly guards attention signals from the cooldown window.ActiveDot, addsCountsStripfor chat/PR counts, and unifies lock-screen + expanded glance surfaces;ExpandedGlanceStripnow re-usesCountsStripwhich can produce redundant attention+count duplication.ATTN_STATESsubscripts will crash the canvas if any attention key is missing.runningAgentsand new count fields; consistent use of helper methods and cleaner visual treatment throughout.bell.slash.fill; cosmetic only.Sequence Diagram
sequenceDiagram participant SS as SyncService participant LAC as LiveActivityCoordinator participant CS as ContentState participant LA as Live Activity (iOS) SS->>SS: refreshActiveSessionsAndSnapshot() Note over SS: Partition sessions into<br/>runningAgents / awaitingInputCount / idleCount SS->>SS: activeSessions = allAgents SS->>LAC: reconcile(with: runningAgents, awaitingInputCount:, idleCount:) LAC->>LAC: shouldStartFreshActivity(for:)? Note over LAC: Cooldown suppressed if<br/>attention != nil LAC->>CS: makeContentState(...) CS->>CS: selectAttention() → .awaitingInput LAC->>LA: update / start activity LA-->>LAC: activityStateUpdates (.dismissed) LAC->>LAC: lastUserDismissalAt = Date()Comments Outside Diff (4)
apps/ios/ADEWidgets/ADEWidgetPreviewData.swift, line 1879-1923 (link)The preview fixtures embed real session and PR UUIDs sampled from the developer's actual on-disk database (
/Users/arul/ADE/.ade/ade.db), and the file path itself is preserved in a comment. Although this is#if DEBUG-only, it still commits live internal IDs and a machine-specific absolute path to the repository history. Consider replacing the UUIDs with clearly synthetic stand-ins (e.g."real-session-1") and removing the path from the comment.Prompt To Fix With AI
apps/ios/ADEWidgets/ADELiveActivityPreviews.swift, line 678-683 (link)Every
ATTN_STATES[.someKey]!call will crash the Xcode canvas ifATTN_STATESever omits that key. Since this is#if DEBUGit won't ship, but a missing attention-kind entry will silently break all canvas previews without a helpful error. Consider usingguard letor providing a fallback state.Prompt To Fix With AI
apps/ios/ADEWidgets/ADELockScreenWidget.swift, line 1797-1803 (link)active > 0Gauge(value: Double(active), in: 0...Double(max(active, 1)))sets the upper bound toactiveitself, so the ring is permanently at full fill regardless of total chat count. The current range gives the ring no contextual meaning beyond "something is running." If the intent is a purely boolean indicator, a simpleCirclestroke might read more clearly than a gauge stuck at 100%.Prompt To Fix With AI
apps/ios/ADE/Services/SyncService.swift, line 281-283 (link)activeSessionsdoc-comment contradicts new semanticsThe newly added line says "Roster only — sessions whose runtime is actively producing output", but the assignment
activeSessions = allAgentsstores every live chat — including idle and awaiting-input sessions. Any existing observer ofactiveSessionsthat relied on the old "running/awaiting only" contract will now silently receive a larger set.The mismatch between the doc-comment and the actual contents makes the property dangerous to consume: a caller reading the comment would filter assuming the list is already trimmed, and either under-count or expose stale-idle rows that were specifically excluded before this change.
Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "ship: iter 1 — address PR #207 review (c..." | Re-trigger Greptile