Skip to content

Fix rewind tab: remove intro video, fix delayed screenshots#5802

Merged
kodjima33 merged 4 commits intomainfrom
fix/rewind-immediate-screenshots
Mar 18, 2026
Merged

Fix rewind tab: remove intro video, fix delayed screenshots#5802
kodjima33 merged 4 commits intomainfrom
fix/rewind-immediate-screenshots

Conversation

@kodjima33
Copy link
Copy Markdown
Collaborator

Summary

  • Remove the rewind intro video overlay — users see their screenshots directly on the Rewind tab
  • Fix 5-minute delay before screenshots appear after onboarding: notification auth callback was blocking continueStartMonitoring, and first capture waited for the 3s timer interval instead of firing immediately

Test plan

  • Fresh install: complete onboarding, navigate to Rewind tab — no video overlay shown
  • Screenshots appear within seconds of monitoring start, not minutes
  • Notification permission still requested (in parallel) without blocking capture

🤖 Generated with Claude Code

kodjima33 and others added 4 commits March 18, 2026 19:31
Users now see their screenshots directly when navigating to the
Rewind tab instead of a full-screen video overlay with Get Started/Skip.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two fixes:
- Don't block monitoring startup on notification auth callback.
  The comment said "don't block" but continueStartMonitoring was
  inside the requestAuthorization callback, delaying capture start
  until the user dismissed the macOS notification dialog.
- Capture first frame immediately after timer starts instead of
  waiting for the first 3-second interval to elapse.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kodjima33 kodjima33 merged commit 3a82f0b into main Mar 18, 2026
2 checks passed
@kodjima33 kodjima33 deleted the fix/rewind-immediate-screenshots branch March 18, 2026 23:32
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 18, 2026

Greptile Summary

This PR addresses two user-facing regressions in the Rewind tab: it removes the first-time intro video overlay (replacing it with immediate screenshot display) and fixes a multi-minute delay before screenshots appeared after onboarding.

Key changes:

  • RewindPage.swift — The RewindIntroVideoView NSViewRepresentable, rewindIntroOverlay, @AppStorage("hasSeenRewindIntro"), and the AVKit import are all cleanly removed. Users land directly on their screenshot timeline.
  • ProactiveAssistantsPlugin.swiftcontinueStartMonitoring(completion:) is moved out of the requestAuthorization callback, so monitoring starts immediately and is no longer gated by the notification permission dialog. An immediate one-shot Task { captureFrame() } is inserted right after isMonitoring = true, eliminating the wait for the first timer tick.
  • CHANGELOG.json — Two unreleased entries added to accurately describe both fixes.
  • CLAUDE.md — Computer-control tool documentation expanded (unrelated to the runtime fix).

Note on the removed video asset: The RewindIntroVideoView referenced rewind-demo.mp4 via Bundle.resourceBundle. The Swift code that loads it is removed, but if the .mp4 file itself is still in the app bundle it will add dead weight to every download. It's worth confirming the asset is also removed from the Xcode project / resource bundle.

Confidence Score: 4/5

  • Safe to merge — the logic is sound, the threading model is correct, and the fixes address the root causes described.
  • Both fixes are well-reasoned: moving continueStartMonitoring out of the notification callback correctly eliminates the blocking delay, the captureFrame() guard on isMonitoring makes the immediate first-capture safe even if stop is called before the Task runs, and the intro-video removal is clean. One point deducted for the minor strong-self inconsistency in the one-shot Task and the unconfirmed status of the bundled rewind-demo.mp4 asset, which should be verified and removed.
  • desktop/Desktop/Sources/ProactiveAssistants/ProactiveAssistantsPlugin.swift — verify the rewind-demo.mp4 asset is removed from the Xcode bundle alongside the code that referenced it.

Important Files Changed

Filename Overview
desktop/Desktop/Sources/ProactiveAssistants/ProactiveAssistantsPlugin.swift Decouples notification auth callback from monitoring startup; adds immediate first-frame capture. Minor inconsistency: the one-shot Task uses a strong self while the recurring timer closure above uses [weak self].
desktop/Desktop/Sources/Rewind/UI/RewindPage.swift Cleanly removes the intro video overlay, AVKit import, AppStorage key, and the RewindIntroVideoView NSViewRepresentable. No functional regressions visible.
desktop/CHANGELOG.json Adds two accurate unreleased changelog entries describing the intro video removal and the screenshot delay fix.
CLAUDE.md Adds documentation for computer-control MCP tools (cliclick, codriver, playwright). No code changes.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant startMonitoring
    participant UNCenter as UNUserNotificationCenter
    participant continueStart as continueStartMonitoring
    participant captureTimer
    participant captureFrame

    Caller->>startMonitoring: startMonitoring()
    startMonitoring->>UNCenter: requestAuthorization() async fire-and-forget
    Note over UNCenter: Runs in background, no longer blocks monitoring
    startMonitoring->>continueStart: continueStartMonitoring() immediate
    continueStart->>captureTimer: scheduleTimer repeating captureInterval
    continueStart->>captureFrame: Task captureFrame() first frame immediate
    Note over captureFrame: First screenshot taken right away
    captureTimer-->>captureFrame: fires every captureInterval
    continueStart->>Caller: completion true nil
    UNCenter-->>startMonitoring: granted or error callback fires later
    Note over startMonitoring: Logs result, repairs LaunchServices if needed
Loading

Last reviewed commit: "Update CLAUDE.md wit..."

Comment on lines +398 to +400
Task { @MainActor in
await self.captureFrame()
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Inconsistent self capture vs. timer closure

The recurring timer closure directly above this block (line 388–392) captures self weakly with [weak self], which is the established pattern in this file. The new one-shot Task uses a strong self instead. Although this won't produce a permanent retain cycle (the Task completes quickly and ProactiveAssistantsPlugin is a singleton), the inconsistency is worth aligning for readability and to match the style of every other async capture in this class.

Suggested change
Task { @MainActor in
await self.captureFrame()
}
Task { @MainActor [weak self] in
await self?.captureFrame()
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@@ -132,10 +129,6 @@ struct RewindPage: View {
.frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Orphaned blank line after removed intro-video overlay

The blank line left here after removing the if !hasSeenRewindIntro { rewindIntroOverlay } block is harmless but leaves unnecessary whitespace inside the ZStack. Consider removing it for a clean diff.

Suggested change
.frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
…dware#5802)

## Summary
- Remove the rewind intro video overlay — users see their screenshots
directly on the Rewind tab
- Fix 5-minute delay before screenshots appear after onboarding:
notification auth callback was blocking `continueStartMonitoring`, and
first capture waited for the 3s timer interval instead of firing
immediately

## Test plan
- [ ] Fresh install: complete onboarding, navigate to Rewind tab — no
video overlay shown
- [ ] Screenshots appear within seconds of monitoring start, not minutes
- [ ] Notification permission still requested (in parallel) without
blocking capture

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant