Disable Apple Intelligence in engine pickers when it is unavailable - #837
Conversation
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>
|
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 configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughEngine pickers now show availability-aware labels and disable unavailable engines. Selection bindings reject unavailable engines before changing the selected engine or power-source profile. ChangesEngine Selection
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to 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 ReviewSecurity architecture risk: 🔵 Low · up to 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 Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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.)
✨ Finishing Touches 💡 1📝 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 |
| /// 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 { |
There was a problem hiding this comment.
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!
…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>
Summary
The Settings and menu bar engine pickers listed Apple Intelligence as selectable even on Macs where
FoundationModelAvailabilityServicereports it unavailable; only onboarding and the power-profile picker gated it. A new pureSuggestionEngineSelectionPolicynow drives both pickers: an unavailable Apple Intelligence stays listed (so a stored selection still renders) but is greyed out with an "(Unavailable)" suffix viaselectionDisabled, and the binding setters refuse to persist an unselectable engine as a backstop.Validation
xcodebuild test(prepared workspace,CODE_SIGNING_ALLOWED=NO) with-only-testingSuggestionEngineSelectionPolicyTests+SettingsAttentionEvaluatorTests: 11 tests, 0 failures,** TEST SUCCEEDED **.Linked issues
None.
Risk / rollout notes
xcodegen generatefor the two new files.Summary by CodeRabbit
The PR appears safe to merge; the remaining concern is non-blocking test coverage for the picker bindings.
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.
Reviews (1) · Last reviewed commit: "Disable Apple Intelligence in engine pic..."