#464 fix(ui): brighten the Intelligence picker and gate switching behind Save

[Anonymous]

Originally created by: Akarsh-Hegde

What does this PR do?

Two things the Settings -> Intelligence panel was getting wrong: it read as pale and greyed out, and switching the AI provider applied on a single click with no confirmation.

1. Readability

The provider cards had their surface layering inverted. The card was painted with --t-box - on the ink palette that is rgba(255,255,255,.055), all but invisible against the desk - behind a 0.5px border at 10% white, while the elements inside it used the solid --t-card.

Swapped to the correct order: card on --t-card, inner elements (glyph, Test button) on --t-box, with a full-weight 1px --t-ctrl-border, badges up from 8.5px to 9.5px, and the dim --t-faint body notes lifted to --t-muted.

Deliberately avoided --t-ctrl for inner fills: it is #FFFFFF on the light palettes, identical to the --t-card those elements now sit on, so they would have disappeared. Everything stays token-based, so it holds across lilac / blush / ink.

2. Switching now requires Save

A pick stages into local pending state and is written only by an explicit Save - matching every other Settings section (Advanced, Capture and Notifications all use <SaveButton>). Intelligence was the outlier.

• The dirty check compares the (provider, custom_id) pair, so switching between two custom endpoints registers as a change.
• Re-picking the saved provider clears the stage, so Save can never offer a non-change.
• Only the selection is gated. Test connection, Rescan and adding/removing custom endpoints stay immediate - those are actions, not this setting.

3. Fixes a real bug along the way

The old code applied the pick optimistically via patch() before saving, with a comment claiming save "rolls the store back on failure". It does not - useRuntimeSettings.save only calls setSettings on success (it swallows the error), so a failed write left the panel claiming a provider the daemon had rejected. With nothing written until Save there is nothing to roll back, and a failed save now keeps the staged pick so the user can retry without re-picking.

4. Layout

• The Save bar is sticky, pinned to the bottom of the Settings scroll area, so it cannot fall below the fold however many providers, endpoints or warnings are on screen. (Widening alone would have fixed one screen and regressed the moment another endpoint was added.)
• Section widens 640px -> 880px (it sat inside a 980px modal, wasting ~280px) and the grid becomes auto-fill / minmax(232px, 1fr), giving Settings a third column while the narrower setup wizard naturally keeps two.
• "+ Add a custom endpoint" now shares the card wrap and occupies a normal grid cell instead of a full row, keeping only its dashed border to read as an affordance. Its form still spans the row - a span now owned by AddCustomProvider, since only it knows whether the form is open.

Also fixes a missing space in "<name> isn't in use yet" and drops a dead missing local.

How was it tested?

• [x] npm run build (UI) passes
• [x] bun test - 235 pass, 0 fail
• [x] Full pre-push suite green (fmt, clippy, cargo test, ui build, ui tests, security audit)
• [ ] Not visually verified across all three themes - see below

Notes for the reviewer

• LlmProviderPicker is shared between this panel and the setup wizard's Intelligence step, so the restyle intentionally lands on both. The Save gating does not - it lives in IntelligenceSection only, because the wizard's own Next/Back is its commit and each pick there writes through by design.
• IntelligenceSection no longer takes patch; the prop was dropped at the SettingsModal call site. The other three sections still use it, so useRuntimeSettings is unchanged.
• Please eyeball this in a running build. The colour work was reasoned from the token tables rather than seen rendered, so the magnitude of the brightening is the thing most worth a second opinion - particularly the sticky bar's --t-panel background as content scrolls under it. If the cards feel too narrow at three columns, the 232px minimum in the grid is the single number to raise.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Amk5pH47wJj76HBm5oDu9X

[Anonymous]

Originally posted by: coderabbitai[bot]

[!IMPORTANT]

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: edd9c50b-b220-4e7e-b0ff-baeacea24053

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:
- [ ] 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

- [ ] Create PR with unit tests - [ ] Commit unit tests in branch `fix/intelligence-settings-ui`

---

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

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

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

[Anonymous]

Originally posted by: adityaharishch

Review

Overview: Two independent fixes bundled together: (1) the Intelligence provider picker's surface layering was inverted (card painted with the near-invisible --t-box token while inner elements used the solid --t-card), and (2) switching the AI provider applied on a single click instead of being gated behind Save like every other Settings section. Also widens the section (640px → 880px) for a third grid column and fixes a real bug found along the way.

Correctness

• The real bug fix is the important part here and looks right: the old code called patch() optimistically before save(), with a comment claiming save "rolls the store back on failure." Looking at the described useRuntimeSettings.save behavior (only calls setSettings on success, swallows errors), that comment was simply wrong — a failed write left the UI claiming a provider the daemon had rejected. Staging the pick in local pending state and only committing on an explicit, successful Save is the correct fix, and it now keeps the staged pick on failure so retry doesn't require re-picking.
• The dirty-check comparing (provider, custom_id) as a pair (not just provider) is correct — otherwise switching between two custom endpoints wouldn't register as a change.
• onChange's backToSaved logic correctly clears the stage when the user re-picks whatever is already saved, so Save never offers a no-op write.
• Scoping the gate to only the provider selection (Test connection / Rescan / add-remove custom endpoint stay immediate) is the right call — those are actions, not the setting being saved.
• The IntelligenceSection signature change (dropping patch) has exactly one call site (SettingsModal.tsx), updated correctly; the other three sections are untouched.

Style / conventions

• Color/token choices are heavily commented with the why (e.g., --t-ctrl vs --t-box collapsing to the same value on light palettes), which is good practice for a change that's reasoned-not-rendered per the author's own caveat.
• Consistent with the project's token-based theming approach across all three palettes.

Risks / things to verify

• The author explicitly flags this was not visually verified across all three themes — this is the biggest real risk in the PR (magnitude of brightening, and the sticky Save bar's --t-panel background as content scrolls under it were called out specifically). Recommend an actual visual pass in a running build before merging, particularly the sticky-bar + scroll interaction, before treating this as done.
• The inline "Couldn't save that choice" error paragraph was removed from IntelligenceSection in favor of <SaveButton>'s own status display — this is consistent with how Advanced/Capture/Notifications already work, but worth a quick check that SaveButton's error state reads as clearly as the removed custom copy did.
• No new unit/component tests were added for the staged-pending logic (onChange/onSave) despite this being a genuine bug fix (previously-silent broken rollback). Given bun test passed at 235/0, existing coverage evidently didn't catch the original bug either — a small test asserting "failed save keeps the pending pick" would directly guard the regression this PR fixes.

Verdict: Solid fix for a real, previously-silent bug, with the right architecture (stage → explicit Save, matching sibling sections). Hold on merging until the "not visually verified" caveat is closed out with an actual look in a running build.

[Anonymous]

Originally posted by: Akarsh-Hegde

Thanks for the review. Two of the three items are addressed in 203c2df4; the third is genuinely yours to close, and I've kept it open rather than papering over it.

Addressed

Test coverage for the staged-pending logic - added ui/__tests__/intelligence-provider-save.test.ts (16 cases). Followed the house idiom for a repo with no React render harness (plan-store / oauth-setup-lifecycle): model the transitions, then scan the source for the shape that keeps them true. It pins your explicit ask - a failed save keeps the staged pick - plus the (provider, custom_id) pair comparison and the clear-on-'saved'-only rule.

Two cases are deliberate counter-examples so the guards can't be vacuous:

• one models the OLD optimistic-patch() path and asserts it retains a provider the daemon rejected (the bug this PR fixes),
• the other shows an id-only dirty check misses a switch between two custom endpoints.

The source scans run against a comment-stripped copy - the module header describes the removed patch()-on-click behaviour, so scanning raw text would conflate documenting the old bug with still doing it. (That assertion genuinely fired on the first run, which is a decent sign the scan isn't inert.)

Suite is now 251 pass / 0 fail.

SaveButton error copy - you were right that "Failed to save" drops something the removed line carried. I left <SaveButton> alone, since it's shared with Advanced/Capture/Notifications and changing it would ripple into all three. Instead the line directly above it now adapts on failure:

Couldn't switch to Cursor - it's still selected here, so you can press Save to try again.

That carries the two things generic status can't: which provider failed, and that the pick survived - which is the whole point of keeping the stage on error.

Not addressed - needs a running build (this is the merge blocker)

The visual pass is still open, and I can't close it. I have no way to see the rendered webview. I considered a standalone token-accurate preview page, but rejected it: it would show colour magnitude on static markup and not the thing you specifically named - the sticky --t-panel bar as real content scrolls under it inside the real modal. Shipping that as "visually verified" would claim a gate was closed when it wasn't.

So this needs about two minutes from someone with the app running:

git checkout fix/intelligence-settings-ui
cd tray && npm run tauri dev

Then Settings → Intelligence, and click a provider you're not currently on (the sticky Save bar only appears while a pick is staged). Worth checking in each of lilac / blush / ink (Settings → Appearance):

1. Sticky Save bar over scrolling content - the --t-panel background as cards scroll beneath it. The named risk.
2. Brightness magnitude - the cards should read as solid lit objects, not washed out. Reasoned from the token tables, not seen.
3. Three-column layout - if the cards feel cramped, the 232px minimum in the picker's auto-fill grid is the single number to raise.

Verdict: agreed, not ready to merge until [#1] is looked at. Everything else is closed.

[Anonymous]

Ticket changed by: Akarsh-Hegde

• status: open --> closed