refactor: tighten Settings copy to match macOS System Settings density#322
Merged
Conversation
Settings — especially Plugins — accumulated layered explanations: section
detail repeating row label, "Stored locally" / "Saved automatically." status
flicker after every keystroke, "Optional X…" captions where placeholders
suffice, and a Firecrawl guidance card spending 80 lines on a single R2
prerequisite. This sweeps the noise so the panes feel closer to System
Settings: section header carries the noun, row label carries the value, and
captions only appear when load-bearing.
- Drop "Saved automatically." status across all auto-save plugin views
- Strip "Stored locally / Changes save automatically" boilerplate from
PluginAPIKey, MinerU, TTS, STT, Firecrawl
- Web Search: fold Clear into the API Key row, delete the standalone
Status and Configured rows, rename Provider Settings to "{Provider}
Options", drop the single-toggle "Web Search" wrapper, replace Brave
country/language captions with placeholder hints, and trim Tavily/Exa/
Jina/Firecrawl/Brave advanced-row captions
- Replace "Optional X…" captions with TextField placeholders across STT,
TTS, Cloudflare R2, MinerU
- Merge ChatNaming "Behavior" + "Naming Model" into one section; drop
Cloudflare "Actions" wrapper; rename OpenRouter OCR "Connection" and
MinerU "API Token" sections to "API Key"
- Collapse Firecrawl 3-row guidance into one info callout + a Configure
R2 Upload… button (new .settingsNavigateToPlugin event the SettingsView
observes for cross-plugin navigation)
- Add per-provider Get-an-API-key Links for Web Search providers, plus
Cloudflare R2 dashboard and MinerU sign-up Links
- Trim Agent Mode Details disclosures, Updates section detail, and
Appearance overlay-scrollbars caption; consolidate AddMCPServer
duplicate help/supportingText pairs
swift build, swift test (2180 tests, 0 failures) and ./Packaging/package.sh
all pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a notification-based plugin navigation system to the settings UI and simplifies help/supporting text and auto-save status messaging across many plugin and application settings views. ChangesPlugin Navigation & Settings UI Simplification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/UI/TextToSpeechPluginSettingsView+Providers.swift (1)
234-235: 💤 Low valueConsider using "click" instead of "tap" for macOS platform.
The instruction text uses "tap Test Connection" which is mobile/touch-interface language. On macOS, "click" is more conventional. However, if "tap" is used consistently throughout the codebase for cross-platform messaging, this can be deferred.
📝 Suggested wording adjustment
- Text("Enter your API key and tap Test Connection to load voices.") + Text("Enter your API key and click Test Connection to load voices.")🤖 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/UI/TextToSpeechPluginSettingsView`+Providers.swift around lines 234 - 235, The message string "Enter your API key and tap Test Connection to load voices." uses touch terminology; update the Text(...) initializer to use platform-appropriate wording by switching "tap" to "click" on macOS (e.g. wrap the string in a conditional compilation branch using `#if` os(macOS) / `#else` so macOS shows "click" and other platforms keep "tap"), leaving the jinInfoCallout() usage unchanged.
🤖 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/Domain/WebSearchAndToolControls.swift`:
- Around line 111-120: The signupURL computed property contains potentially
incorrect endpoints for the .exa and .perplexity cases; verify the canonical
dashboard/signup URLs and replace "https://dashboard.exa.ai/" and
"https://www.perplexity.ai/settings/api" with the correct targets (or a
documented help/support URL) inside the signupURL switch branch for .exa and
.perplexity in WebSearchAndToolControls so the links no longer return 429/403;
if no direct dashboard URL exists, point to the provider’s public signup or
account/settings landing page and add a short comment above the .exa/.perplexity
cases noting the reason for the chosen fallback.
In `@Sources/UI/SettingsView.swift`:
- Around line 176-185: The NotificationCenter handler for the
.settingsNavigateToPlugin publisher mutates `@State` (selectedSection,
selectedPluginID) off the main thread; ensure delivery on the main thread by
adding .receive(on: RunLoop.main) to the publisher chain (i.e., change
.onReceive(NotificationCenter.default.publisher(for: .settingsNavigateToPlugin))
to .onReceive(NotificationCenter.default.publisher(for:
.settingsNavigateToPlugin).receive(on: RunLoop.main))) or alternatively wrap the
mutation block in Task { await MainActor.run { ... } } so the
withAnimation(settingsMotionAnimation) and assignments to
selectedSection/selectedPluginID always run on the main thread.
---
Nitpick comments:
In `@Sources/UI/TextToSpeechPluginSettingsView`+Providers.swift:
- Around line 234-235: The message string "Enter your API key and tap Test
Connection to load voices." uses touch terminology; update the Text(...)
initializer to use platform-appropriate wording by switching "tap" to "click" on
macOS (e.g. wrap the string in a conditional compilation branch using `#if`
os(macOS) / `#else` so macOS shows "click" and other platforms keep "tap"),
leaving the jinInfoCallout() usage unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48e36164-4db6-4fb6-b261-bd5e47ef26c3
📒 Files selected for processing (25)
Sources/Domain/AppNotifications.swiftSources/Domain/WebSearchAndToolControls.swiftSources/UI/AddMCPServerSections.swiftSources/UI/AddProviderView.swiftSources/UI/AgentModeSettingsView+CommandPrefixes.swiftSources/UI/AgentModeSettingsView+ToolsAndSafety.swiftSources/UI/AppearanceSettingsView.swiftSources/UI/ChatNamingPluginSettingsView.swiftSources/UI/CloudflareR2UploadPluginSettingsView.swiftSources/UI/DeepSeekOCRPluginSettingsView.swiftSources/UI/FirecrawlOCRPluginSettingsView.swiftSources/UI/MCPServerConfigFormSections.swiftSources/UI/MinerUOCRPluginSettingsView.swiftSources/UI/OpenRouterOCRPluginSettingsView.swiftSources/UI/PluginAPIKeySettingsView.swiftSources/UI/SettingsView.swiftSources/UI/SpeechToTextPluginSettingsView+Actions.swiftSources/UI/SpeechToTextPluginSettingsView+Providers.swiftSources/UI/SpeechToTextPluginSettingsView.swiftSources/UI/TextToSpeechPluginSettingsView+Credentials.swiftSources/UI/TextToSpeechPluginSettingsView+Providers.swiftSources/UI/TextToSpeechPluginSettingsView.swiftSources/UI/UpdateSettingsView.swiftSources/UI/WebSearchPluginSettingsChromeViews.swiftSources/UI/WebSearchPluginSettingsView.swift
Comment on lines
+111
to
+120
| var signupURL: URL? { | ||
| switch self { | ||
| case .exa: return URL(string: "https://dashboard.exa.ai/") | ||
| case .brave: return URL(string: "https://api-dashboard.search.brave.com/") | ||
| case .jina: return URL(string: "https://jina.ai/api-dashboard/") | ||
| case .firecrawl: return URL(string: "https://www.firecrawl.dev/") | ||
| case .tavily: return URL(string: "https://app.tavily.com/") | ||
| case .perplexity: return URL(string: "https://www.perplexity.ai/settings/api") | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check HTTP status of all signup URLs
urls=(
"https://dashboard.exa.ai/"
"https://api-dashboard.search.brave.com/"
"https://jina.ai/api-dashboard/"
"https://www.firecrawl.dev/"
"https://app.tavily.com/"
"https://www.perplexity.ai/settings/api"
)
for url in "${urls[@]}"; do
status=$(curl -o /dev/null -s -w "%{http_code}" -L "$url")
echo "$url -> HTTP $status"
doneRepository: hrayleung/Jin
Length of output: 316
Verify signup URLs for Exa and Perplexity; most other URLs are accessible.
Accessibility check reveals most provider signup URLs are reachable, but two need attention:
- Exa dashboard (
https://dashboard.exa.ai/) returns HTTP 429 (Rate Limited) - Perplexity settings (
https://www.perplexity.ai/settings/api) returns HTTP 403 (Forbidden)
Confirm these are the correct signup/dashboard URLs, or update them if they should point elsewhere. The remaining URLs (Brave, Jina, Firecrawl, Tavily) are confirmed accessible.
🤖 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/Domain/WebSearchAndToolControls.swift` around lines 111 - 120, The
signupURL computed property contains potentially incorrect endpoints for the
.exa and .perplexity cases; verify the canonical dashboard/signup URLs and
replace "https://dashboard.exa.ai/" and "https://www.perplexity.ai/settings/api"
with the correct targets (or a documented help/support URL) inside the signupURL
switch branch for .exa and .perplexity in WebSearchAndToolControls so the links
no longer return 429/403; if no direct dashboard URL exists, point to the
provider’s public signup or account/settings landing page and add a short
comment above the .exa/.perplexity cases noting the reason for the chosen
fallback.
Resolve conflict in WebSearchPluginSettingsChromeViews.swift between master's expanded provider settings (#321 — Exa Category, User location, moderation; Jina Country/Locale; Firecrawl Country/Language and source toggles; Tavily Country and auto-tune; Perplexity Country/Language) and this branch's Apple-density cleanup. Resolution keeps every new control while applying the same cleanup rules: self-explanatory toggles ship without supportingText, and Optional X… captions on country/language/locale fields are replaced with `e.g. US` / `e.g. en` placeholders. Tavily's "applies on General topic only" gating note and the auto-tune warning are kept as trimmed supportingText since the behavior is non-obvious. swift build and swift test (2258 tests, 0 failures) pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Deliver .settingsNavigateToPlugin on RunLoop.main so the SettingsView handler always mutates @State on the main thread (mirrors the existing pattern in TTSMiniPlayerView+Coordinator.swift). NotificationCenter publishers fire on the posting thread, so a future poster off-main would otherwise touch SwiftUI state on a background queue. - Repoint the Perplexity sign-up Link from www.perplexity.ai/settings/api (auth-walled, returns 403 to unauthenticated visitors) to docs.perplexity.ai, which is reliably public-facing for users who do not yet have an account. Skipped: Exa dashboard URL — `dashboard.exa.ai` is the canonical Exa dashboard subdomain (referenced by Exa's own docs); the 429 the bot saw is request rate limiting on its scraper, not a user-facing issue. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/UI/WebSearchPluginSettingsChromeViews.swift (1)
204-211: 💤 Low valueReorder Tavily search-depth options along a single axis.
The current order
Basic → Fast → Advanced → Ultra-fastdoesn't follow speed nor quality monotonically, which forces the user to scan all four to compare. Reordering by speed (or by latency-vs-relevance per Tavily's own framing) makes the choice more legible. Tavily/langchain-tavily docs confirm all four tags are valid, so only the picker order is at issue here.♻️ Suggested ordering (slowest/most-relevant → fastest)
private var tavilySearchDepthRow: some View { JinSettingsPickerRow("Search depth", selection: $tavilySearchDepth) { + Text("Advanced").tag("advanced") Text("Basic").tag("basic") - Text("Fast").tag("fast") - Text("Advanced").tag("advanced") - Text("Ultra-fast").tag("ultra-fast") + Text("Fast").tag("fast") + Text("Ultra-fast").tag("ultra-fast") } }Worth a quick check that Tavily's current
search_depthenum still includesfast/ultra-fast(the public REST reference page primarily documentsbasic/advanced; the SDK references list all four). If the REST endpoint hasn't graduatedfast/ultra-fastout of beta, you may want to gate them behind a comment or omit them.🤖 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/UI/WebSearchPluginSettingsChromeViews.swift` around lines 204 - 211, Reorder the picker items in tavilySearchDepthRow (the JinSettingsPickerRow bound to $tavilySearchDepth) so options run slowest/most-relevant → fastest (e.g., "Basic".tag("basic"), "Advanced".tag("advanced"), "Fast".tag("fast"), "Ultra-fast".tag("ultra-fast")); update the sequence in that closure and optionally add a short comment above tavilySearchDepthRow noting to verify Tavily's search_depth enum still supports "fast" and "ultra-fast" before shipping.
🤖 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.
Nitpick comments:
In `@Sources/UI/WebSearchPluginSettingsChromeViews.swift`:
- Around line 204-211: Reorder the picker items in tavilySearchDepthRow (the
JinSettingsPickerRow bound to $tavilySearchDepth) so options run
slowest/most-relevant → fastest (e.g., "Basic".tag("basic"),
"Advanced".tag("advanced"), "Fast".tag("fast"), "Ultra-fast".tag("ultra-fast"));
update the sequence in that closure and optionally add a short comment above
tavilySearchDepthRow noting to verify Tavily's search_depth enum still supports
"fast" and "ultra-fast" before shipping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f06c5d2f-039b-42aa-a673-f8704e978969
📒 Files selected for processing (3)
Sources/Domain/WebSearchAndToolControls.swiftSources/UI/WebSearchPluginSettingsChromeViews.swiftSources/UI/WebSearchPluginSettingsView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/Domain/WebSearchAndToolControls.swift
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
1 participant
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
Settings — especially Plugins — accumulated layered explanations that made the panes feel cluttered and un-Apple: section detail paraphrasing row labels,
Stored locally/Saved automatically.flicker after every keystroke,Optional X…captions where placeholders would do, and a Firecrawl guidance card spending 80 lines on a single R2 prerequisite. This pass strips that noise so each pane sits closer to System Settings density (section header = noun, row label = value, caption only when load-bearing).Net diff: +146 / −282 across 25 files. No behavior change beyond UI copy and a new cross-plugin navigation notification used by the Firecrawl callout.
Highlights
Saved automatically.after every keystroke; keepCleared.and errors. Standardize the missing period in OpenRouter OCR.Stored locally on this Mac. Changes save automatically.from PluginAPIKey, MinerU, TTS, STT, Firecrawl.StatusandConfiguredrows, fold Clear into the API Key row, renameProvider Settings→{Provider} Options, drop the single-toggle wrapping section, replace Brave country/language captions with placeholders, and trim the Exa/Brave/Jina/Firecrawl/Tavily advanced-row captions.Behavior+Naming Modelinto one section, drop CloudflareActionswrapper, rename OpenRouter OCRConnectionand MinerUAPI Tokensections toAPI Key.Configure R2 Upload…button. Adds a new.settingsNavigateToPluginnotification (AppNotifications.swift) thatSettingsViewobserves for cross-plugin navigation.Get an API key on …Links for the 6 Web Search providers (signupURLonSearchPluginProvider), plus Cloudflare R2 dashboard and MinerU sign-up Links..help()that mirroredsupportingText; trim the import disclosure.Test plan
swift build— cleanswift test— 2180 tests, 0 failures./Packaging/package.sh— releaseJin.appbundle built and ad-hoc signedNotes
.settingsNavigateToPluginnotification is internal-only; userInfo carriespluginIDkeyed bySettingsNavigationUserInfoKey.pluginID.SearchPluginProvider.signupURLURLs point at well-known stable provider sign-up landing pages; if any provider relocates we just update the URL.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
UI/UX Updates