Skip to content

Disable Apple Intelligence in engine pickers when it is unavailable - #837

Merged
FuJacob merged 1 commit into
mainfrom
fix/apple-intelligence-availability-gate
Sep 29, 2026
Merged

FuJacob merged 1 commit into
mainfrom
fix/apple-intelligence-availability-gate

Conversation

@FuJacob

@FuJacob FuJacob commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Summary

The Settings and menu bar engine pickers listed Apple Intelligence as selectable even on Macs where FoundationModelAvailabilityService reports it unavailable; only onboarding and the power-profile picker gated it. A new pure SuggestionEngineSelectionPolicy now drives both pickers: an unavailable Apple Intelligence stays listed (so a stored selection still renders) but is greyed out with an "(Unavailable)" suffix via selectionDisabled, and the binding setters refuse to persist an unselectable engine as a backstop.

Validation

  • xcodebuild test (prepared workspace, CODE_SIGNING_ALLOWED=NO) with -only-testing SuggestionEngineSelectionPolicyTests + SettingsAttentionEvaluatorTests: 11 tests, 0 failures, ** TEST SUCCEEDED **.
  • Not yet eyeballed in the running app on a Mac without Apple Intelligence.

Linked issues

None.

Risk / rollout notes

  • Gates new selections only. An existing persisted Apple Intelligence selection is still honored with the existing callout and comes back to life if the model becomes available; no auto-migration was added.
  • pbxproj regenerated via xcodegen generate for the two new files.

Summary by CodeRabbit

  • Improvements
    • Engine pickers now show when Apple Intelligence is unavailable and prevent selecting it when Foundation Models aren’t available.
    • Open Source and OpenAI-compatible engines remain selectable regardless of Foundation Models availability.

RetriggerConfidence Score: 4/5

The PR appears safe to merge; the remaining concern is non-blocking test coverage for the picker bindings.

Fix All in CodexFindings

  1. P2 Picker paths lack test coverage ▶

Summary

The PR adds a shared policy that keeps Apple Intelligence visible but unselectable when unavailable, then applies it to the Settings and menu-bar engine pickers.

  • Both picker bindings reject an unavailable selection before saving it.
  • Unit tests cover the policy, but not the picker selection paths.

Reviews (1) · Last reviewed commit: "Disable Apple Intelligence in engine pic..."

Onboarding and the power-profile pickers already refuse Apple Intelligence
when FoundationModelAvailabilityService reports it unavailable, but the
Settings Engine picker and the menu bar Engine picker listed every engine
unconditionally, so a user could switch to an engine the Mac cannot run.

Add SuggestionEngineSelectionPolicy (pure, Support/Settings) as the single
rule for which engines are selectable and how each is labelled. Both
pickers now show an unavailable engine greyed out with an "(Unavailable)"
suffix via selectionDisabled, and their bindings refuse to persist it.
A previously stored selection is still honored as before; the existing
warning callout and engine error continue to explain it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 29, 2026 00:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 033ca589-3826-4d7b-afc4-cfe13ac92a78

📥 Commits

Reviewing files that changed from the base of the PR and between d0465e5 and e91dab3.

📒 Files selected for processing (6)
  • Cotabby.xcodeproj/project.pbxproj
  • Cotabby/Support/Settings/SuggestionEngineSelectionPolicy.swift
  • Cotabby/UI/MenuBar/MenuBarView.swift
  • Cotabby/UI/Settings/Panes/Engine/EngineAndModelPaneView+Actions.swift
  • Cotabby/UI/Settings/Panes/Engine/EngineAndModelPaneView.swift
  • CotabbyTests/Support/Settings/SuggestionEngineSelectionPolicyTests.swift

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Engine pickers now show availability-aware labels and disable unavailable engines. Selection bindings reject unavailable engines before changing the selected engine or power-source profile.

Changes

Engine Selection

Layer / File(s) Summary
Selection policy and project registration
Cotabby/Support/Settings/SuggestionEngineSelectionPolicy.swift, CotabbyTests/Support/Settings/SuggestionEngineSelectionPolicyTests.swift, Cotabby.xcodeproj/project.pbxproj
The policy gates Apple Intelligence selection on Foundation Model availability and adds an “(Unavailable)” label when an engine cannot be selected. Tests cover selectability and labels. The project registers the policy and tests in their targets.
Picker and selection integration
Cotabby/UI/MenuBar/MenuBarView.swift, Cotabby/UI/Settings/Panes/Engine/EngineAndModelPaneView.swift, Cotabby/UI/Settings/Panes/Engine/EngineAndModelPaneView+Actions.swift
The menu bar and settings pickers use policy-generated labels and disable unavailable choices. Their selection bindings ignore unavailable engine selections.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to e91da

No actionable issue is established. The change is mergeable after normal checks; displaying an existing Apple Intelligence selection on an unsupported Mac has not been confirmed in the running app.

Security Architecture Review

Security architecture risk: 🔵 Low · up to e91da

The change narrows which engines can be selected without changing how existing selections are stored or introducing a new external access path. Some runtime transition behavior remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed control governs engine selection on the local Mac. The inspected picker paths do not add a remote entrypoint, privileged operation, or direct generation call.

Trust Boundaries and Controls

  • observed — Disabled picker rows are backed by setter guards, so an attempted unavailable selection does not persist through either changed picker path. The generation boundary separately checks runtime availability.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 5 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: disabling Apple Intelligence in engine pickers when it is unavailable.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 5 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

/// Pins the engine-picker gating shared by Settings and the menu bar so Apple Intelligence can never
/// again be offered as a selectable engine on a Mac that cannot run it, while the other engines stay
/// selectable regardless of Apple Intelligence availability.
final class SuggestionEngineSelectionPolicyTests: XCTestCase {

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 Picker paths lack test coverage The new tests check the policy’s answers, but neither picker’s binding setter nor its disabled option. A guard could be removed from Settings or the menu bar and these tests would still pass, even though users could then select an unavailable engine. Please cover accepted and rejected selections through both paths, or move their selection handling into a testable helper.

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!

Fix in Codex Fix in Claude Code

@FuJacob
FuJacob merged commit e3ceb6f into main Sep 29, 2026
6 checks passed
flexi767 added a commit to flexi767/cotabby that referenced this pull request Sep 30, 2026
…anchor

Brings in FuJacob/cotabby through 7724926 (FuJacob#825, FuJacob#827, FuJacob#834, FuJacob#835, FuJacob#836, FuJacob#837,
FuJacob#838): ghost text matched to the host's own rendering plus finishing the word
being typed, Obsidian/Electron accessibility priming, rapid Tab accepts kept
owned while the overlay presents, Apple Intelligence greyed out when
unavailable, a new llama-recall eval fixture, the installer artwork and
kickstart script, and a CI test-flakiness fix.

Resolution, where both sides changed the same thing:
- Suppression reasons: upstream's `CompletionContentPolicy` rejects any
  completion without a letter or digit (`noWordContent`), which subsumes this
  fork's `periodOnly`, so that case and its helper are removed and the bare-dot
  test now asserts the broader reason. Behaviour change worth knowing: a lone
  closing bracket or question mark is now withheld too, where this fork
  deliberately let those through; its test records that.
- Prompt renderer: upstream's `personaLine(_:prefix:)` (the name only appears at
  a sign-off) wins; this fork's learned-phrases section is unchanged beside it.
- Eval case schema: upstream added `visualContextSummary` and `clipboardContext`
  for its recall fixture; this fork's `screenText` and `learnedPhrases` stay
  alongside them. The harness prefers the pre-rendered summary and still puts
  raw `screenText` through the live local passes. Both scorer helpers kept.
- Kept from this fork: the separate test host, the 2-4 word eval preset, phrase
  memory, the clock-time rule, the debug-artifact gate.

Full suite: 2,724 tests, 16 skipped, 0 failures; the installed app's defaults
byte-identical afterwards and no debug artifacts written.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

2 participants