Expand web search provider settings#321
Conversation
|
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 (8)
📝 WalkthroughWalkthroughThis pull request extends web-search provider configuration by adding new domain types, settings fields, request-body builders, and UI controls for Exa, Firecrawl, Tavily, Perplexity, and Jina. Refactors inline request construction into testable pure helpers and persists expanded settings via AppStorage and UserDefaults. ChangesWeb Search Provider Configuration Expansion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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: 4
🧹 Nitpick comments (1)
Sources/Domain/WebSearchAndToolControls.swift (1)
169-171: 💤 Low valueConsider typing
exaCategoryasExaCategory?for consistency withexaSearchType.
exaSearchTypeis stored as the typed enumExaSearchType?, butexaCategoryis kept asString?even though the newExaCategoryenum (and itsresolved(from:)helper) exist alongside it. Storing the raw string allows arbitrary/invalid values to roundtrip throughCodable. If the only reason for the string form is forward-compat with new Exa categories, that mirrorsExaSearchType(which has.resolved(from:)and a "keyword" →.fastfallback) and could be handled the same way.Not blocking — this is a low-effort, low-reward style alignment, deferable.
Also applies to: 193-194, 209-210
🤖 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 169 - 171, Change exaCategory from String? to ExaCategory? and mirror how exaSearchType/ExaSearchType handle forward-compatibility: use ExaCategory.resolved(from:) when decoding incoming raw strings so unknown/new category values map to a safe fallback, and encode using the enum's rawValue (or original string if you preserved it) when encoding; update any usages and Codable init/encode paths that reference exaCategory to accept ExaCategory? instead of String? so parsing/roundtripping behavior matches exaSearchType/ExaSearchType.
🤖 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/Tools/BuiltinSearchProvider`+Firecrawl.swift:
- Around line 28-33: The code appends parsed buckets from dataDict["news"] and
dataDict["images"] into raw via parseArray but applies the max-results cap
before removing duplicates, allowing duplicate URLs to crowd out unique results;
update the merge logic in BuiltinSearchProvider+Firecrawl.swift (the code around
parseArray(...), the raw array, and the result capping) to deduplicate merged
buckets first (e.g., use a Set keyed by each bucket's URL or unique identifier)
preserving the original order, then apply the existing max-results limit; ensure
you reference and dedupe the same bucket structure returned by parseArray so the
downstream code that enforces the cap sees only unique URLs.
In `@Sources/Tools/BuiltinSearchProvider`+Jina.swift:
- Around line 90-91: The domain filters built in the snippet (variable
operators) must be grouped before being appended to the main query so multiple
domains produce "(site:a.com OR site:b.com)"; change the return logic to wrap
the joined operators in parentheses (e.g., operatorsWrapped = "(\(operators))")
and use operatorsWrapped both when trimmed.isEmpty and when building "\(trimmed)
\(operatorsWrapped)"; reference the operators variable and the existing return
expression to locate and update the code.
- Around line 14-16: The code forces at least one result via resolvedMaxResults
= min(max(args.maxResults, 1), 5) which ignores an explicit args.maxResults ==
0; change the clamping to allow 0 (e.g., clamp between 0 and 5) and add an early
exit when resolvedMaxResults == 0 so the Jina provider call is not made and an
empty result/citation set is returned; touch the resolvedMaxResults computation
and the call site that uses Self.extractJinaResults(from:) to implement the
early return.
In `@Sources/UI/WebSearchPluginSettingsChromeViews.swift`:
- Around line 357-363: In firecrawlSelectedSources(), when firecrawlSourcesRaw
is empty or decoding fails the function currently returns an empty array; change
this to return [.web] so the UI matches Firecrawl's default behavior. Update the
guard/fallback logic in the firecrawlSelectedSources() function (and any use of
FirecrawlSourceKind) to return [FirecrawlSourceKind.web] (or .web) instead of []
when trimmed/decoded input is nil or empty, ensuring the UI toggles reflect the
API default.
---
Nitpick comments:
In `@Sources/Domain/WebSearchAndToolControls.swift`:
- Around line 169-171: Change exaCategory from String? to ExaCategory? and
mirror how exaSearchType/ExaSearchType handle forward-compatibility: use
ExaCategory.resolved(from:) when decoding incoming raw strings so unknown/new
category values map to a safe fallback, and encode using the enum's rawValue (or
original string if you preserved it) when encoding; update any usages and
Codable init/encode paths that reference exaCategory to accept ExaCategory?
instead of String? so parsing/roundtripping behavior matches
exaSearchType/ExaSearchType.
🪄 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: 5f9e45e2-852a-40e5-bc86-1b05ed3548c7
📒 Files selected for processing (21)
Sources/Domain/WebSearchAndToolControls.swiftSources/Tools/BuiltinSearchHelpers.swiftSources/Tools/BuiltinSearchProvider+Brave.swiftSources/Tools/BuiltinSearchProvider+Firecrawl.swiftSources/Tools/BuiltinSearchProvider+Jina.swiftSources/Tools/BuiltinSearchProvider+Perplexity.swiftSources/Tools/BuiltinSearchProvider+Tavily.swiftSources/Tools/BuiltinSearchProviders.swiftSources/UI/AppPreferenceKeys.swiftSources/UI/WebSearchPluginSettingsChromeViews.swiftSources/UI/WebSearchPluginSettingsStore.swiftSources/UI/WebSearchPluginSettingsView.swiftTests/JinTests/BraveSearchAPITests.swiftTests/JinTests/BuiltinSearchExaPayloadTests.swiftTests/JinTests/BuiltinSearchFirecrawlPayloadTests.swiftTests/JinTests/BuiltinSearchJinaRequestTests.swiftTests/JinTests/BuiltinSearchPerplexityPayloadTests.swiftTests/JinTests/BuiltinSearchTavilyPayloadTests.swiftTests/JinTests/BuiltinSearchToolHubTests.swiftTests/JinTests/ChatAuxiliaryControlSupportTests.swiftTests/JinTests/WebSearchPluginSettingsStoreTests.swift
| private func firecrawlSelectedSources() -> [FirecrawlSourceKind] { | ||
| guard let trimmed = firecrawlSourcesRaw.trimmedNonEmpty, | ||
| let data = trimmed.data(using: .utf8), | ||
| let raw = try? JSONDecoder().decode([String].self, from: data) else { | ||
| return [] | ||
| } | ||
| return raw.compactMap(FirecrawlSourceKind.init(rawValue:)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the file in question
find . -name "WebSearchPluginSettingsChromeViews.swift" -type fRepository: hrayleung/Jin
Length of output: 111
🏁 Script executed:
# Search for FirecrawlSourceKind definition
rg "enum FirecrawlSourceKind|case web" --type swift -A 10Repository: hrayleung/Jin
Length of output: 4090
🏁 Script executed:
# Check how firecrawlSelectedSources is used in the codebase
rg "firecrawlSelectedSources" --type swift -B 3 -A 3Repository: hrayleung/Jin
Length of output: 1637
🌐 Web query:
Firecrawl search API sources parameter default behavior
💡 Result:
Firecrawl Search API: the sources parameter defaults to web. If you omit sources (or don’t provide it in the request), Firecrawl uses sources: ['web'] by default, which means you’ll get standard web results (and the response will be shaped accordingly, e.g., web results available). [1][2] When sources is provided, it controls which result-type arrays are returned (e.g., web, news, images). The docs also state that web is the default source type. [2][1]
Citations:
- 1: https://docs.firecrawl.dev/api-reference/endpoint/search
- 2: https://docs.firecrawl.dev/features/search
🏁 Script executed:
# Check for the firecrawlSourcesRaw property and persistence logic
rg "firecrawlSourcesRaw" --type swift -B 5 -A 5Repository: hrayleung/Jin
Length of output: 7858
🏁 Script executed:
# Look for the complete firecrawlSourceBinding method and surrounding context
rg "firecrawlSourceBinding" --type swift -B 2 -A 15Repository: hrayleung/Jin
Length of output: 4182
🏁 Script executed:
# Check if there are any initialization or default values set elsewhere
rg "FirecrawlSourceKind|firecrawl.*[Ss]ource" --type swift | head -40Repository: hrayleung/Jin
Length of output: 3909
🏁 Script executed:
# Verify the API request handling one more time to ensure sources omission defaults to web
cat -n Sources/Tools/BuiltinSearchProvider+Firecrawl.swift | grep -A 20 "firecrawlSources"Repository: hrayleung/Jin
Length of output: 1039
🏁 Script executed:
# Check if there's any initialization or default handling in WebSearchPluginSettingsStore
rg "decodeFirecrawlSources|encodeFirecrawlSources" --type swift -B 3 -A 10Repository: hrayleung/Jin
Length of output: 6310
Return [.web] instead of [] to match Firecrawl's default behavior.
When firecrawlSourcesRaw is empty, this falls back to [], but Firecrawl treats omitted sources as web by default. The UI renders all toggles disabled while the API still returns web results, misrepresenting the actual state and control the user has over search results.
Suggested fix
private func firecrawlSelectedSources() -> [FirecrawlSourceKind] {
guard let trimmed = firecrawlSourcesRaw.trimmedNonEmpty,
let data = trimmed.data(using: .utf8),
let raw = try? JSONDecoder().decode([String].self, from: data) else {
- return []
+ return [.web]
}
- return raw.compactMap(FirecrawlSourceKind.init(rawValue:))
+ let decoded = raw.compactMap(FirecrawlSourceKind.init(rawValue:))
+ return decoded.isEmpty ? [.web] : decoded
}This ensures the UI accurately reflects the API's default source when no explicit selection is persisted, maintaining coherent interaction flow per your UI/UX guidelines.
🤖 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 357 - 363,
In firecrawlSelectedSources(), when firecrawlSourcesRaw is empty or decoding
fails the function currently returns an empty array; change this to return
[.web] so the UI matches Firecrawl's default behavior. Update the guard/fallback
logic in the firecrawlSelectedSources() function (and any use of
FirecrawlSourceKind) to return [FirecrawlSourceKind.web] (or .web) instead of []
when trimmed/decoded input is nil or empty, ensuring the UI toggles reflect the
API default.
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>
1 participant
Summary
Validation
Summary by CodeRabbit
New Features
Tests