Add opt-out for sending app and window info to the context model - #209
Draft
ojhurst wants to merge 2 commits into
Draft
Add opt-out for sending app and window info to the context model#209ojhurst wants to merge 2 commits into
ojhurst wants to merge 2 commits into
Conversation
Adds a "Use screen context for smarter dictation" toggle in Settings → Prompts → Context Prompt, plus a Skip button on the Setup wizard's Screen Recording step. Both default to on, so existing users see zero behavior change. Motivation: the context inference is genuinely useful but requires Screen Recording permission, which feels invasive to some users. Every dictation captures a JPEG of the active window and ships it to whatever inference endpoint they have configured. Today there is no way to disable just the screenshot without revoking the OS permission (which also kills the grant, so flipping it back on means re-granting). How it works: AppContextService.captureActiveWindowScreenshot gates on the new use_screenshot_context UserDefaults flag (default true). When off it returns the same (nil, nil, error) tuple shape as the permission-denied path — so the existing screenshotDataURL == nil fallback handles everything downstream. The error string is user-facing copy that points the user back at the toggle, and it surfaces in the Run Log's Capture Context step via a new no-image caption added in this PR. Test button in Settings refuses to run when the toggle is off, with a clear message pointing back at the toggle. Observability: adds an os_log subsystem com.zachlatta.freeflow category Context that emits one line per capture outcome (skipped / blocked / captured-active-window / captured-focused-title / captured-fullscreen / failed). Useful for verifying gate behavior from outside the app via log show. 79 insertions, 0 deletions. Purely additive — no upstream logic modified. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pairs with the screenshot-context toggle (zachlatta#208) so users have full control over both kinds of context the model sees — pixels and text strings — independently. Motivation: even with the screenshot off, the context inference still sends the active app name, bundle identifier, window title, and any selected text. That string set is enough to leak meaningful detail on its own — a browser window title alone often contains the email recipient, the open Gmail thread, the active file in a code editor, or the active page on a SaaS dashboard. Some users would rather the model see none of that, accepting weaker context-aware cleanup in exchange. How it works: AppContextService.collectContext still performs the AX reads internally so the screenshot path can find the active window, but when the new send_app_and_window_context UserDefaults flag is off, the appName, bundleIdentifier, windowTitle, and selectedText fields are scrubbed to nil before being passed to the LLM inference call OR stored on the returned AppContext. The downstream pipelineHistory and run-log display honor the nils naturally. When BOTH this and the screenshot toggle are off, the LLM inference call is skipped entirely (no signal, no API call), and currentActivity is the plain string "You are dictating. App and window context are disabled in settings, and no screenshot was captured." — a stable string so the run log makes the configuration explicit instead of silently inferring nothing. Observability: one new os_log line under the existing Context category, "app/window context skipped — disabled in settings (Send app and window info = off)". Pairs with the screenshot skip line from zachlatta#208 — when both fire on the same dictation, the unified log shows two skip entries on the same thread at the same millisecond, which is the verifiable signature of fully blind dictation. Default-on stays the safe choice for the same reason as zachlatta#208 — zero behavior change for existing users, opt-in for the privacy-conscious. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR adds two user-configurable feature flags to control screenshot context and app/window metadata collection. AppState persists both settings via UserDefaults. AppContextService gates context and screenshot capture based on these flags, adding detailed os.log messaging throughout. SettingsView and SetupView expose configuration controls and validation. ChangesContext availability feature flags
Sequence DiagramsequenceDiagram
participant Settings as User in Settings
participant AppState as AppState
participant Defaults as UserDefaults
participant ContextService as AppContextService
Settings->>AppState: Toggle useScreenshotContext
AppState->>Defaults: Persist via didSet
Settings->>AppState: Toggle sendAppAndWindowContext
AppState->>Defaults: Persist via didSet
ContextService->>Defaults: Read useScreenshotContext
ContextService->>ContextService: If disabled, return nil image + error log
ContextService->>Defaults: Read sendAppAndWindowContext
ContextService->>ContextService: If disabled, set app/window/text to nil + info log
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Sources/AppContextService.swift (1)
110-113: ⚡ Quick winUse shared constants for settings keys to avoid silent drift
"send_app_and_window_context"and"use_screenshot_context"are duplicated string literals across layers. A typo/rename in one location would silently break behavior. Move these to shared constants and reference them from bothAppStateandAppContextService.Also applies to: 437-439
🤖 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/AppContextService.swift` around lines 110 - 113, Replace hard-coded setting key literals with a shared constant to avoid drift: define reusable keys (e.g., SettingsKeys.sendAppAndWindowContext and SettingsKeys.useScreenshotContext) in a shared place and update AppContextService (the let sendAppAndWindowContext initialization) and the corresponding usage in AppState to reference those constants instead of the string literals so both layers use the same identifier.Sources/SetupView.swift (1)
155-173: ⚡ Quick winContinue button should explicitly enable screenshot context.
The Skip button explicitly sets
useScreenshotContext = false, but the Continue button relies on the default value remainingtrue. If a user re-runs setup after previously opting out (useScreenshotContext = false) and has permission already granted, clicking Continue will advance without re-enabling the feature. Since Continue semantically means "grant permission and use this feature," it should explicitly set the value totrue.🔧 Proposed fix to explicitly enable screenshot context
} else if currentStep == .screenRecording { HStack(spacing: 10) { Button("Skip") { appState.useScreenshotContext = false withAnimation { currentStep = nextStep(currentStep) } } .buttonStyle(.plain) .foregroundStyle(.secondary) Button("Continue") { + appState.useScreenshotContext = true withAnimation { currentStep = nextStep(currentStep) } } .keyboardShortcut(.defaultAction) .disabled(!canContinueFromCurrentStep) }🤖 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/SetupView.swift` around lines 155 - 173, The Continue button handler should explicitly enable the screenshot context before advancing steps: inside the Button("Continue") action (the closure associated with the Continue button in SetupView where currentStep is updated via nextStep(currentStep) and canContinueFromCurrentStep is checked), set appState.useScreenshotContext = true and then call withAnimation { currentStep = nextStep(currentStep) } so re-running setup after a prior opt-out correctly re-enables screenshot usage.
🤖 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/AppContextService.swift`:
- Around line 164-166: The message uses screenshot.dataURL == nil to decide the
"both disabled" status, which is wrong; change the condition that sets
currentActivity/contextPrompt to check the screenshot context toggle (the
screenshot-send setting) together with sendAppAndWindowContext instead of
relying on screenshot.dataURL being nil. In practice replace the branch that
reads "else if !sendAppAndWindowContext && screenshot.dataURL == nil" with a
check like "else if !sendAppAndWindowContext && !sendScreenshotContext" (or the
actual screenshot-toggle variable name used in the codebase) so the message only
appears when both context toggles are disabled, not when capture failed. Ensure
you only modify the condition around currentActivity and contextPrompt and leave
screenshot.dataURL handling for capture-failure paths.
---
Nitpick comments:
In `@Sources/AppContextService.swift`:
- Around line 110-113: Replace hard-coded setting key literals with a shared
constant to avoid drift: define reusable keys (e.g.,
SettingsKeys.sendAppAndWindowContext and SettingsKeys.useScreenshotContext) in a
shared place and update AppContextService (the let sendAppAndWindowContext
initialization) and the corresponding usage in AppState to reference those
constants instead of the string literals so both layers use the same identifier.
In `@Sources/SetupView.swift`:
- Around line 155-173: The Continue button handler should explicitly enable the
screenshot context before advancing steps: inside the Button("Continue") action
(the closure associated with the Continue button in SetupView where currentStep
is updated via nextStep(currentStep) and canContinueFromCurrentStep is checked),
set appState.useScreenshotContext = true and then call withAnimation {
currentStep = nextStep(currentStep) } so re-running setup after a prior opt-out
correctly re-enables screenshot usage.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 74a9b7f3-4238-46f4-b63a-182846e10073
📒 Files selected for processing (4)
Sources/AppContextService.swiftSources/AppState.swiftSources/SettingsView.swiftSources/SetupView.swift
Comment on lines
+164
to
+166
| } else if !sendAppAndWindowContext && screenshot.dataURL == nil { | ||
| currentActivity = "You are dictating. App and window context are disabled in settings, and no screenshot was captured." | ||
| contextPrompt = nil |
Contributor
There was a problem hiding this comment.
Gate the “both disabled” status on the screenshot toggle, not screenshot capture outcome
Line 164 currently uses screenshot.dataURL == nil as the condition, so this message is also emitted when screenshot capture fails for other reasons (for example permission/encoding), even if screenshot context is enabled. That makes the state message inaccurate.
💡 Proposed fix
+ let useScreenshotContext = UserDefaults.standard.object(forKey: "use_screenshot_context") == nil
+ ? true
+ : UserDefaults.standard.bool(forKey: "use_screenshot_context")
+
let currentActivity: String
let contextPrompt: String?
if !apiKey.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty
&& (sendAppAndWindowContext || screenshot.dataURL != nil) {
...
- } else if !sendAppAndWindowContext && screenshot.dataURL == nil {
+ } else if !sendAppAndWindowContext && !useScreenshotContext {
currentActivity = "You are dictating. App and window context are disabled in settings, and no screenshot was captured."
contextPrompt = nil
} else {🤖 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/AppContextService.swift` around lines 164 - 166, The message uses
screenshot.dataURL == nil to decide the "both disabled" status, which is wrong;
change the condition that sets currentActivity/contextPrompt to check the
screenshot context toggle (the screenshot-send setting) together with
sendAppAndWindowContext instead of relying on screenshot.dataURL being nil. In
practice replace the branch that reads "else if !sendAppAndWindowContext &&
screenshot.dataURL == nil" with a check like "else if !sendAppAndWindowContext
&& !sendScreenshotContext" (or the actual screenshot-toggle variable name used
in the codebase) so the message only appears when both context toggles are
disabled, not when capture failed. Ensure you only modify the condition around
currentActivity and contextPrompt and leave screenshot.dataURL handling for
capture-failure paths.
Collaborator
|
same as #208, does not build due to missing ‘SettingsSubcard’ |
marcbodea
marked this pull request as draft
June 3, 2026 17:59
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
2 participants
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
Adds a second toggle in Settings → Prompts → Context Prompt: "Send app and window info". Pairs with the screenshot-context toggle in #208 so users can independently control both kinds of context the model sees — pixels (screenshot) and text strings (app name, window title, selected text). Default on, no behavior change for existing users.
Why
Even with the screenshot toggle off, the context inference still sends the active app's name, bundle identifier, window title, and any selected text to the model. That string set alone is often enough to leak meaningful information — a browser window title contains the Gmail thread subject and the user's email address, a code editor's title bar reveals the active file, a SaaS dashboard's title gives away the account being viewed.
Some users would rather the model see none of that — accepting weaker context-aware cleanup in exchange for the strongest privacy posture. This toggle gives them that option, and pairs with #208 to make the privacy story complete:
How it works
AppContextService.collectContextstill performs the AX reads internally so the screenshot capture path can find the active window. But when the newsend_app_and_window_contextUserDefaults flag is off,appName,bundleIdentifier,windowTitle, andselectedTextare scrubbed tonilbefore being passed to the LLM inference call OR stored on the returnedAppContext. The downstreampipelineHistoryand run-log display honor thenils naturally.When BOTH toggles are off, the LLM inference call is skipped entirely (no signal, no API call), and
currentActivityis set to a stable string:"You are dictating. App and window context are disabled in settings, and no screenshot was captured."— so the run log makes the configuration explicit instead of silently producing nothing.Observability
One new
os_logline under the existingContextcategory (added in #208):Pairs with the screenshot skip line. When both fire on the same dictation, the unified log shows two skip entries on the same thread at the same millisecond — the verifiable signature of fully blind dictation.
What changed
Sources/AppState.swift— newsendAppAndWindowContext@PublishedBool, persisted at UserDefaults keysend_app_and_window_context, defaulttrue.Sources/AppContextService.swift— adds the gate incollectContext; AX reads still happen internally (the screenshot path needs them), but the values are scrubbed before going to the LLM or to the returnedAppContextwhen the toggle is off. Also short-circuits the LLM inference call when both toggles are off, since there is no signal worth spending an API call on.Sources/SettingsView.swift— adds the second toggle subcard right below the screenshot toggle.+50 / -6. The -6 lines are a small refactor in
collectContextto move the AX reads into local variables that get conditionally scrubbed.Testing
Verified locally:
app/window context skipped — disabled in settingsper dictation; screenshot still fires as expected. Inferred activity from the model becomes generic ("a coding environment", "an email application") instead of naming the specific app or thread."You are dictating. App and window context are disabled in settings, and no screenshot was captured."Screenshots
The second toggle in Prompts → Context Prompt (sits directly below the screenshot toggle from #208):
Notes
feat/optional-screenshot-context). When Add opt-out for sending screenshots to the context model #208 lands, this rebases tomaincleanly. If Add opt-out for sending screenshots to the context model #208 stalls, this stays a dependent and can be re-targeted.Summary by CodeRabbit
New Features
UI/UX