Skip to content

Add multi-engine search settings, onboarding, and custom search support#278

Open
JaggedGem wants to merge 9 commits into
MultiboxLabs:mainfrom
JaggedGem:feature/multiple-search-engines
Open

Add multi-engine search settings, onboarding, and custom search support#278
JaggedGem wants to merge 9 commits into
MultiboxLabs:mainfrom
JaggedGem:feature/multiple-search-engines

Conversation

@JaggedGem

@JaggedGem JaggedGem commented Jun 8, 2026

Copy link
Copy Markdown

Summary

This PR expands Flow's search engine support from a Google-only setup to a fully configurable search system.

Users can now choose between Google, DuckDuckGo, Yandex, or a custom search engine URL. The new search configuration is available both during onboarding and later in settings, and the selected engine is respected across search URL generation, autocomplete suggestions, and related browser UI flows.

What Changed

  • added built-in search engine options for:
    • Google
    • DuckDuckGo
    • Yandex
  • added a new onboarding step for choosing the default search provider
  • added settings UI for changing the default search provider after onboarding
  • added support for custom search engine templates using {{query}}
  • added support for choosing a suggestion source for custom engines:
    • none
    • Google
    • DuckDuckGo
    • Yandex

Implementation Details

  • introduced shared typed search settings/config utilities for:
    • provider IDs and labels
    • normalized search settings snapshots
    • custom URL template validation
    • search URL construction
  • refactored renderer search plumbing to use the selected provider instead of hard-coded Google URLs
  • updated older omnibox/search codepaths to use the shared search URL builder for consistency
  • exposed a normalized search settings snapshot through the settings API so renderer startup logic can read the current provider synchronously
  • updated the tab context menu search action to use the selected search engine instead of always targeting Google

Validation and Safety

  • added validation around active custom search configurations
  • prevented invalid custom configurations from becoming the active search engine state
  • repaired previously invalid saved custom search states by falling back to a safe built-in provider
  • improved typing around search provider completions and settings handling

Supporting Changes

  • added dev scripts for opening omnibox devtools during development
  • added small copy/UX cleanups to remove Google-specific wording where the active engine is now dynamic

Validation

  • bun run lint
  • bun run typecheck

Summary by CodeRabbit

  • New Features

    • Choose Google, DuckDuckGo, Yandex or a validated custom search URL (with suggestions provider) and set DuckDuckGo AI toggle.
    • New onboarding step to pick and configure search provider; Settings UI updated with a Search Engine card and custom template editor.
    • Omnibox now uses provider-driven suggestions and builds searches from the selected engine.
  • Bug Fixes

    • Context-menu and omnibox wording now respect the chosen search engine.
  • Chores

    • Added dev scripts to enable devtools conditionally.

JaggedGem added 7 commits June 8, 2026 14:05
- add node scripts to start the browser with omnibox devtools
- improve typing to better handle multiple search engines
…th a variable that is replaced at runtime with the user's query

- add support to choose either none or any of the available search providers for completion (when custom mode is enabled)
- add the custom search engine configuration options to both settings and the onboarding screen
- Introduced a new search settings snapshot mechanism to manage search engine preferences and custom search URL templates.
- Updated context menu to utilize dynamic search engine settings for search actions.
- Added IPC handlers for retrieving search settings snapshots asynchronously and synchronously.
- Refactored basic settings to include search engine options and custom search URL templates.
- Implemented validation for custom search URL templates to ensure they meet required formats.
- Enhanced onboarding and settings components to support new search engine configurations.
- Updated omnibox providers to build search URLs based on selected search engine settings.
- Improved overall search experience by integrating custom search capabilities with existing providers.
@JaggedGem JaggedGem requested a review from iamEvanYT as a code owner June 8, 2026 14:11
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Refactors search from Google-only to a settings-driven multi-provider system: adds provider abstractions (Google/DuckDuckGo/Yandex/Custom), custom-template validation/builders, provider registry, settings snapshot/IPC/preload accessors, onboarding and settings UI, and context-menu/omnibox integration.

Changes

Configurable Multi-Provider Search

Layer / File(s) Summary
Core search types and validators
src/shared/search/custom-search.ts, src/shared/search/search-settings.ts, src/shared/types/settings.ts, src/shared/flow/interfaces/settings/settings.ts
Add custom-template validation/builders, SearchSettingsSnapshot and related types/guards, string-typed setting support and description field, and extend SettingsChangedEvent to optionally include a snapshot.
Backend settings registration, snapshot, repair, IPC, and preload
src/main/saving/settings.ts, src/main/modules/basic-settings.ts, src/main/ipc/window/settings.ts, src/preload/index.ts
Register search settings, derive/validate/repair active search snapshot, add IPC endpoints (async + sync) and preload snapshot accessors, emit typed settings-change events.
Provider typing and Google refactor
src/renderer/src/lib/omnibox-new/search-providers/types.ts, src/renderer/src/lib/omnibox-new/search-providers/google.ts, src/renderer/src/lib/omnibox-new/search-providers/custom-utils.ts, src/renderer/src/lib/omnibox-new/search-providers/suggestion-utils.ts
Make SearchProvider generic over completion kind, introduce query/navigation completion variants, add suggestion-relevance helper and custom-utils barrel, and update Google provider to delegate URL building to shared builder and return typed completions.
DuckDuckGo and Yandex provider implementations and URL utilities
src/renderer/src/lib/omnibox-new/search-providers/duckduckgo.ts, src/renderer/src/lib/omnibox-new/search-providers/yandex.ts, src/renderer/src/lib/omnibox-new/search-providers/url-utils.ts
Add DuckDuckGo and Yandex suggestion fetch/parse implementations that produce typed completions with relevance and URL normalization, plus URL normalization/validation helpers.
Provider registry and search library
src/renderer/src/lib/omnibox-new/search-providers/index.ts, src/renderer/src/lib/search.ts
Add provider registry (google/duckduckgo/yandex/custom), settings-driven factory/wrappers, initialize settings subscription, and refactor createSearchUrl/getSearchSuggestions to delegate to current provider.
Omnibox wording and centralized URL building
src/renderer/src/lib/omnibox-v1.ts, src/renderer/src/lib/omnibox-v2.ts
Change omnibox descriptions from "Search Google for" → "Search for" and use centralized createSearchUrl in omnibox-v2.
Onboarding search provider stage
src/renderer/src/components/onboarding/stages/search-provider.tsx, src/renderer/src/components/onboarding/main.tsx
Add onboarding stage to pick Google/DuckDuckGo/Yandex/Custom; normalize/persist custom template when chosen; conditionally show custom fields and DuckDuckGo AI toggle.
Settings UI: custom template fields & integration
src/renderer/src/components/search/custom-search-engine-fields.tsx, src/renderer/src/components/settings/sections/general/basic-settings-cards.tsx, src/renderer/src/components/search/duckduckgo-ai-toggle.tsx, src/renderer/src/components/providers/settings-provider.tsx
New CustomSearchEngineFields component with debounced commits and validation; settings card conditionally renders custom fields only when custom engine selected; add DuckDuckGo AI toggle component and string-type input handling; consolidate settings-provider init/subscription effect.
Context menu and omnibox devtools wiring
src/main/controllers/tabs-controller/context-menu.ts, src/main/controllers/windows-controller/utils/browser/omnibox.ts, package.json
Context menu "Search for" now uses buildSearchUrlFromSearchSettings and display name from snapshot; omnibox devtools toggle depends on app.isPackaged and OMNIBOX_DEVTOOLS env var; package.json dev scripts added.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant OnboardingUI as Onboarding UI
  participant FlowSettings as Flow Settings (preload/API)
  participant SettingsStore as Settings Store
  participant Omnibox
  participant ProviderFactory as Search Provider Factory
  participant CurrentProvider

  User->>OnboardingUI: select provider / edit template
  OnboardingUI->>FlowSettings: flow.settings.setSetting(searchEngine, ...)
  FlowSettings->>SettingsStore: persist change
  User->>Omnibox: type query
  Omnibox->>ProviderFactory: getSearchProvider()
  ProviderFactory->>FlowSettings: getSearchSettingsSnapshotSync()
  FlowSettings-->>ProviderFactory: SearchSettingsSnapshot
  ProviderFactory->>CurrentProvider: return matching provider
  Omnibox->>CurrentProvider: buildSearchUrl(query) / getSuggestions({ input, signal })
  CurrentProvider-->>Omnibox: URL or completions
  Omnibox-->>User: show suggestions / navigate
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

enhancement

Poem

"A rabbit hops through search and code,
New providers sprout along the road,
Templates tamed with token bright,
Settings baked to guide the flight,
🐇 Click, suggest, and land just right."

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.67% which is insufficient. The required threshold is 80.00%. 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 accurately summarizes the main change: adding multi-engine search settings, onboarding, and custom search support.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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 and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the hard-coded Google search setup with a fully configurable search system supporting Google, DuckDuckGo, Yandex, and custom URL templates, wiring the selection through onboarding, settings UI, context menus, omnibox suggestions, and the shared URL builder.

  • Introduces src/shared/search/ with typed provider metadata, URL builders, snapshot validation, and custom template helpers shared across main and renderer processes.
  • Adds new onboarding stage and settings card for provider selection; custom engine fields use a 350 ms debounce with draft/committed state separation.
  • Refactors getSearchProvider() to read a settings snapshot via a synchronous IPC call at initialization, with an onSettingsChanged listener to keep the active provider current.

Confidence Score: 3/5

Mostly safe to merge, but the startup repair path can leave the renderer's displayed engine out of sync with what the main process actually uses.

The repairInvalidActiveSearchConfiguration function silently corrects an invalid custom-engine state at startup without notifying the renderer. Any user who had saved an incomplete custom-engine config would see the settings UI still displaying 'Custom' while searches actually go to Google — the gap persists until the next user-initiated settings change fires a refresh. The rest of the feature is well-structured and the validation logic correctly guards against bad states being written in the first place.

src/main/saving/settings.ts — the repair function needs to emit a settings-changed notification; src/renderer/src/lib/omnibox-new/search-providers/index.ts — the onSettingsChanged handler issues a synchronous IPC on every settings write.

Important Files Changed

Filename Overview
src/main/saving/settings.ts Adds startup repair of invalid custom search configs and a new snapshot export, but repairInvalidActiveSearchConfiguration omits fireOnSettingsChanged(), leaving the renderer out of sync if the repair races against initialization.
src/shared/search/search-settings.ts New shared module for provider metadata, URL builders, type guards, and snapshot validation. Well-structured, though buildDuckDuckGoSearchUrl has a dead aiEnabled parameter and uses the non-canonical www.duckduckgo.com domain.
src/shared/search/custom-search.ts New utility for custom template validation and URL building. Query is properly encodeURIComponent-encoded; template is validated to HTTP/HTTPS with a hostname. Looks correct.
src/renderer/src/lib/omnibox-new/search-providers/index.ts Refactored to support multiple providers with dynamic selection. The onSettingsChanged handler calls a synchronous IPC on every (non-search-related) setting change, which can briefly block the renderer thread.
src/renderer/src/lib/omnibox-new/search-providers/duckduckgo.ts New DuckDuckGo provider using the /ac/?type=list Chrome-suggest-compatible endpoint. Parsing logic and error handling look correct.
src/renderer/src/lib/omnibox-new/search-providers/yandex.ts New Yandex provider with typed raw response parsing, navigation/query suggestion differentiation, and URL validation. Looks solid.
src/renderer/src/components/onboarding/stages/search-provider.tsx New onboarding step for search provider selection. The canContinue gate is partially driven by the draft validation state from CustomSearchEngineFields, which can lead to the Continue button enabling before the template debounce has flushed to settings.
src/renderer/src/components/search/custom-search-engine-fields.tsx New shared custom search fields component with debounced save, draft/committed state separation, and validation feedback. Implementation is clean.
src/main/ipc/window/settings.ts Adds async and synchronous IPC handlers for the search settings snapshot. Straightforward delegation to getSearchSettingsSnapshot().
src/renderer/src/lib/search.ts Replaced hard-coded Google URL construction with dynamic provider delegation. The getSearchSuggestions return now correctly filters to query-kind completions only.
src/renderer/src/components/settings/sections/general/basic-settings-cards.tsx Adds search engine card with custom engine fields surfaced inline. Correctly hides template/suggestions fields when custom is not selected.
src/main/modules/basic-settings.ts Registers three new settings (searchEngine, customSearchUrlTemplate, customSearchSuggestionsProvider) and a Search Engine settings card. Definitions look correct.

Sequence Diagram

sequenceDiagram
    participant R as Renderer (index.ts)
    participant P as Preload
    participant M as Main (settings.ts)
    participant DS as DataStore

    M->>DS: load all settings (parallel)
    DS-->>M: raw values
    M->>M: repairInvalidActiveSearchConfiguration()
    Note over M: mutates in-memory + saves to DS<br/>but does NOT fire onSettingsChanged
    M->>M: resolve settingsCachedPromise

    R->>P: getSearchSettingsSnapshotSync (sendSync)
    P->>M: settings:get-search-settings-snapshot-sync
    M-->>P: SearchSettingsSnapshot
    P-->>R: snapshot → currentSearchSettings

    Note over R: If repair ran before this call ✅ correct<br/>If repair runs after ❌ renderer gets stale value

    R->>R: onSettingsChanged → refreshSelectedSearchProvider()
    R->>P: getSearchSettingsSnapshotSync (sendSync)
    Note over R: fires on ANY setting change, not just search
Loading

Reviews (1): Last reviewed commit: "feat: enhance search settings management..." | Re-trigger Greptile

Comment on lines +66 to +74
function repairInvalidActiveSearchConfiguration() {
const searchSettings = getSearchSettingsSnapshotFromValues(basicSettingsCurrentValues);
if (validateActiveSearchSettings(searchSettings).valid) {
return;
}

basicSettingsCurrentValues.searchEngine = DEFAULT_SEARCH_SETTINGS_SNAPSHOT.searchEngine;
void SettingsDataStore.set("searchEngine", DEFAULT_SEARCH_SETTINGS_SNAPSHOT.searchEngine).catch(() => undefined);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Missing listener notification after startup repair

repairInvalidActiveSearchConfiguration mutates basicSettingsCurrentValues and persists the corrected engine to disk, but never calls fireOnSettingsChanged() or settingsEmitter.emit("settings-changed"). The repair runs after settingsCachedPromise resolves, which can happen after the renderer has already called getSearchSettingsSnapshotSync during module initialization. In that sequence the renderer initializes currentSearchSettings to the invalid (pre-repair) value and is never told to refresh it — so the settings UI continues to show the stale engine selection while the main process has silently moved to the default.

Comment thread src/shared/search/search-settings.ts Outdated
Comment on lines +9 to +17
function buildDuckDuckGoSearchUrl(query: string, aiEnabled: boolean = true): string {
const url = new URL("https://www.duckduckgo.com");
url.searchParams.set("q", query);
if (!aiEnabled) {
url.searchParams.set("ia", "web");
url.searchParams.set("assist", "false");
}
return url.toString();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The aiEnabled parameter defaults to true and buildSearchUrlFromProviderId always calls the function without a second argument, making the if (!aiEnabled) branch permanently unreachable dead code. Either expose the parameter publicly or remove it.

Suggested change
function buildDuckDuckGoSearchUrl(query: string, aiEnabled: boolean = true): string {
const url = new URL("https://www.duckduckgo.com");
url.searchParams.set("q", query);
if (!aiEnabled) {
url.searchParams.set("ia", "web");
url.searchParams.set("assist", "false");
}
return url.toString();
}
function buildDuckDuckGoSearchUrl(query: string): string {
const url = new URL("https://duckduckgo.com");
url.searchParams.set("q", query);
return url.toString();
}

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!

Comment on lines +68 to +70
flow.settings.onSettingsChanged(() => {
refreshSelectedSearchProvider();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Synchronous IPC on every settings change

The onSettingsChanged callback calls refreshSelectedSearchProvider()flow.settings.getSearchSettingsSnapshotSync() (a blocking ipcRenderer.sendSync) on every settings mutation, regardless of whether it's search-related. Changing an unrelated setting (e.g., sidebar opacity) will block the renderer's main thread for a round-trip to the main process. Consider reading only when a search-related key changes, or switching to the async getSearchSettingsSnapshot and updating state when the promise resolves.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/renderer/src/components/onboarding/main.tsx (1)

29-32: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing return statement causes crash at onboarding completion.

When stage exceeds the bounds of the stages array, Stage becomes undefined. The code calls flow.onboarding.finish() but doesn't return early, so execution continues to line 37 where <Stage key={stage} advance={advance} /> attempts to render undefined as a component, throwing a runtime error: "Element type is invalid".

🐛 Proposed fix
 const Stage = stages[stage];
 if (!Stage) {
   flow.onboarding.finish();
+  return null;
 }
🤖 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 `@src/renderer/src/components/onboarding/main.tsx` around lines 29 - 32, When
Stage is undefined because stage is out of bounds, call flow.onboarding.finish()
and then return early to avoid rendering an invalid component; update the
component that uses Stage (the local constant Stage and JSX <Stage key={stage}
advance={advance} />) to either return null or a safe fallback immediately after
invoking flow.onboarding.finish() so execution does not continue to the JSX that
tries to render undefined.
🤖 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 `@src/renderer/src/lib/omnibox-new/search-providers/yandex.ts`:
- Around line 28-30: The mapSuggestionRelevance function in yandex.ts is
duplicate logic also used in duckduckgo.ts; extract the shared calculation into
a new exported utility (e.g., mapSuggestionRelevanceByIndex) and replace the
local mapSuggestionRelevance usage in both yandex.ts and duckduckgo.ts with an
import of that utility; ensure the new function signature matches (index:
number) and returns Math.max(100, 400 - index * 40) so callers like
mapSuggestionRelevance(...) keep the same behavior.
- Around line 32-57: Extract the duplicated URL helpers into a shared module
(e.g., create src/renderer/src/lib/omnibox-new/search-providers/url-utils.ts)
and move normalizeNavigationUrl, isAllowedProtocol and normalizeAndValidateUrl
into it; update yandex.ts and google.ts to import those functions instead of
defining them locally, and standardize the default scheme inside
normalizeNavigationUrl to use https:// (so both providers behave consistently
and securely).

In `@src/renderer/src/lib/search.ts`:
- Around line 9-22: The getSearchSuggestions function currently creates a new
AbortController when signal is undefined, which prevents callers from cancelling
requests and leaks unused controllers; update getSearchSuggestions so it passes
the incoming signal directly to searchProvider.getSuggestions (allowing
undefined) instead of creating new AbortController().signal, or if the provider
interface requires a signal, change the provider type
(SearchProviderRequest.signal) to be optional or document/require callers to
supply a signal—adjust the call site to use getSearchProvider().getSuggestions({
input: query, limit: 10, signal }) and ensure any type changes are made on the
SearchProviderRequest/getSuggestions signatures.

---

Outside diff comments:
In `@src/renderer/src/components/onboarding/main.tsx`:
- Around line 29-32: When Stage is undefined because stage is out of bounds,
call flow.onboarding.finish() and then return early to avoid rendering an
invalid component; update the component that uses Stage (the local constant
Stage and JSX <Stage key={stage} advance={advance} />) to either return null or
a safe fallback immediately after invoking flow.onboarding.finish() so execution
does not continue to the JSX that tries to render undefined.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 8d0ac2fc-73f4-4768-b43b-ed5adbdfbb20

📥 Commits

Reviewing files that changed from the base of the PR and between 55b0bb6 and 5530315.

📒 Files selected for processing (24)
  • package.json
  • src/main/controllers/tabs-controller/context-menu.ts
  • src/main/controllers/windows-controller/utils/browser/omnibox.ts
  • src/main/ipc/window/settings.ts
  • src/main/modules/basic-settings.ts
  • src/main/saving/settings.ts
  • src/preload/index.ts
  • src/renderer/src/components/onboarding/main.tsx
  • src/renderer/src/components/onboarding/stages/search-provider.tsx
  • src/renderer/src/components/search/custom-search-engine-fields.tsx
  • src/renderer/src/components/settings/sections/general/basic-settings-cards.tsx
  • src/renderer/src/lib/omnibox-new/search-providers/custom-utils.ts
  • src/renderer/src/lib/omnibox-new/search-providers/duckduckgo.ts
  • src/renderer/src/lib/omnibox-new/search-providers/google.ts
  • src/renderer/src/lib/omnibox-new/search-providers/index.ts
  • src/renderer/src/lib/omnibox-new/search-providers/types.ts
  • src/renderer/src/lib/omnibox-new/search-providers/yandex.ts
  • src/renderer/src/lib/omnibox-v1.ts
  • src/renderer/src/lib/omnibox-v2.ts
  • src/renderer/src/lib/search.ts
  • src/shared/flow/interfaces/settings/settings.ts
  • src/shared/search/custom-search.ts
  • src/shared/search/search-settings.ts
  • src/shared/types/settings.ts

Comment thread src/renderer/src/lib/omnibox-new/search-providers/yandex.ts Outdated
Comment thread src/renderer/src/lib/omnibox-new/search-providers/yandex.ts Outdated
Comment thread src/renderer/src/lib/search.ts

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/shared/search/search-settings.ts (2)

24-28: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add optional options parameter for signature consistency.

Same issue as buildGoogleSearchUrl — add the optional options parameter to match the signature expected by buildSearchUrlFromProviderId.

♻️ Suggested fix to add optional parameter
-function buildYandexSearchUrl(query: string): string {
+function buildYandexSearchUrl(query: string, options?: SearchUrlBuildOptions): string {
   const url = new URL("https://yandex.com/search/");
   url.searchParams.set("text", query);
   return url.toString();
 }
🤖 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 `@src/shared/search/search-settings.ts` around lines 24 - 28, The
buildYandexSearchUrl function lacks the optional options parameter used by
buildSearchUrlFromProviderId; update the signature of buildYandexSearchUrl to
accept a second optional parameter (e.g., options?: Record<string, any> or
matching the type used by buildGoogleSearchUrl) and ensure the implementation
ignores or accepts it without breaking behavior (keep query handling and URL
construction the same), so the function signature matches buildGoogleSearchUrl
and can be called uniformly by buildSearchUrlFromProviderId.

13-22: ⚠️ Potential issue | 🟠 Major

DuckDuckGo “classic/non-AI” disabling: use noai.duckduckgo.com instead of ia=web/assist=false.

In src/shared/search/search-settings.ts (buildDuckDuckGoSearchUrl), when duckduckgoAiEnabled is false, DuckDuckGo’s documented AI-off experience is the noai.duckduckgo.com subdomain (e.g., https://noai.duckduckgo.com/?q=...). The ia=web and assist=false URL parameters aren’t officially documented as a reliable “disable AI” switch.

🤖 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 `@src/shared/search/search-settings.ts` around lines 13 - 22, The
buildDuckDuckGoSearchUrl function currently toggles AI off by adding ia=web and
assist=false; instead, when options?.duckduckgoAiEnabled is false, switch the
URL base to use the noai.duckduckgo.com subdomain (e.g., construct the URL with
"https://noai.duckduckgo.com") and remove the ia/assist parameter logic so the
query parameter is preserved but the request targets the non-AI domain; update
buildDuckDuckGoSearchUrl and references to duckduckgoAiEnabled to use the
alternate base URL only when AI is disabled.
src/main/saving/settings.ts (1)

30-47: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff

Consider reusing normalizeSearchSettingsSnapshot to reduce duplication.

The logic in getSearchSettingsSnapshotFromValues closely mirrors normalizeSearchSettingsSnapshot from src/shared/search/search-settings.ts. While the input types differ slightly, the core normalization logic is identical. You could potentially cast or adapt the input to reuse the shared function.

🤖 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 `@src/main/saving/settings.ts` around lines 30 - 47,
getSearchSettingsSnapshotFromValues duplicates normalization logic found in
normalizeSearchSettingsSnapshot; instead adapt the incoming values into a
Partial<SearchSettingsSnapshot> (or cast the relevant fields from
SettingType["defaultValue"] to the snapshot shape), call
normalizeSearchSettingsSnapshot(...) and return its result so defaults and
validation stay centralized. Concretely: in getSearchSettingsSnapshotFromValues
build a minimal object with keys searchEngine, customSearchUrlTemplate,
customSearchSuggestionsProvider, duckduckgoAiEnabled (or cast values as
Partial<SearchSettingsSnapshot>), import normalizeSearchSettingsSnapshot, call
normalizeSearchSettingsSnapshot(adaptedValues,
getDefaultSearchSettingsSnapshot()) and return that. Ensure types compile
(narrow/cast where necessary) and remove the duplicated per-field checks.
🤖 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 `@src/shared/search/search-settings.ts`:
- Around line 7-11: The buildGoogleSearchUrl function lacks an optional options
parameter causing a signature mismatch with buildSearchUrlFromProviderId which
forwards options; update buildGoogleSearchUrl to accept a second optional
parameter (e.g., options?: Record<string, any>) and ignore or use it as needed
so its signature matches other provider builders, keeping the existing behavior
of constructing the URL from the query; reference function names
buildGoogleSearchUrl and buildSearchUrlFromProviderId when making the change.

---

Outside diff comments:
In `@src/main/saving/settings.ts`:
- Around line 30-47: getSearchSettingsSnapshotFromValues duplicates
normalization logic found in normalizeSearchSettingsSnapshot; instead adapt the
incoming values into a Partial<SearchSettingsSnapshot> (or cast the relevant
fields from SettingType["defaultValue"] to the snapshot shape), call
normalizeSearchSettingsSnapshot(...) and return its result so defaults and
validation stay centralized. Concretely: in getSearchSettingsSnapshotFromValues
build a minimal object with keys searchEngine, customSearchUrlTemplate,
customSearchSuggestionsProvider, duckduckgoAiEnabled (or cast values as
Partial<SearchSettingsSnapshot>), import normalizeSearchSettingsSnapshot, call
normalizeSearchSettingsSnapshot(adaptedValues,
getDefaultSearchSettingsSnapshot()) and return that. Ensure types compile
(narrow/cast where necessary) and remove the duplicated per-field checks.

In `@src/shared/search/search-settings.ts`:
- Around line 24-28: The buildYandexSearchUrl function lacks the optional
options parameter used by buildSearchUrlFromProviderId; update the signature of
buildYandexSearchUrl to accept a second optional parameter (e.g., options?:
Record<string, any> or matching the type used by buildGoogleSearchUrl) and
ensure the implementation ignores or accepts it without breaking behavior (keep
query handling and URL construction the same), so the function signature matches
buildGoogleSearchUrl and can be called uniformly by
buildSearchUrlFromProviderId.
- Around line 13-22: The buildDuckDuckGoSearchUrl function currently toggles AI
off by adding ia=web and assist=false; instead, when
options?.duckduckgoAiEnabled is false, switch the URL base to use the
noai.duckduckgo.com subdomain (e.g., construct the URL with
"https://noai.duckduckgo.com") and remove the ia/assist parameter logic so the
query parameter is preserved but the request targets the non-AI domain; update
buildDuckDuckGoSearchUrl and references to duckduckgoAiEnabled to use the
alternate base URL only when AI is disabled.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: e3fdfbbc-14cd-4a87-bc6d-2b2859bdd83a

📥 Commits

Reviewing files that changed from the base of the PR and between 5530315 and df0a465.

📒 Files selected for processing (11)
  • src/main/ipc/window/settings.ts
  • src/main/modules/basic-settings.ts
  • src/main/saving/settings.ts
  • src/preload/index.ts
  • src/renderer/src/components/onboarding/stages/search-provider.tsx
  • src/renderer/src/components/providers/settings-provider.tsx
  • src/renderer/src/components/search/duckduckgo-ai-toggle.tsx
  • src/renderer/src/components/settings/sections/general/basic-settings-cards.tsx
  • src/renderer/src/lib/omnibox-new/search-providers/index.ts
  • src/shared/flow/interfaces/settings/settings.ts
  • src/shared/search/search-settings.ts

Comment on lines +7 to +11
function buildGoogleSearchUrl(query: string): string {
const url = new URL("https://www.google.com/search");
url.searchParams.set("q", query);
return url.toString();
}

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.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add optional options parameter for signature consistency.

The buildGoogleSearchUrl function doesn't accept an options parameter, but buildSearchUrlFromProviderId (line 121) passes options to all provider build functions. While JavaScript ignores extra arguments, this creates a type inconsistency that may cause TypeScript errors depending on strictness settings.

♻️ Suggested fix to add optional parameter
-function buildGoogleSearchUrl(query: string): string {
+function buildGoogleSearchUrl(query: string, options?: SearchUrlBuildOptions): string {
   const url = new URL("https://www.google.com/search");
   url.searchParams.set("q", query);
   return url.toString();
 }
🤖 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 `@src/shared/search/search-settings.ts` around lines 7 - 11, The
buildGoogleSearchUrl function lacks an optional options parameter causing a
signature mismatch with buildSearchUrlFromProviderId which forwards options;
update buildGoogleSearchUrl to accept a second optional parameter (e.g.,
options?: Record<string, any>) and ignore or use it as needed so its signature
matches other provider builders, keeping the existing behavior of constructing
the URL from the query; reference function names buildGoogleSearchUrl and
buildSearchUrlFromProviderId when making the change.

@coderabbitai coderabbitai Bot left a comment

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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/renderer/src/components/onboarding/main.tsx (1)

25-27: ⚠️ Potential issue | 🟡 Minor

Use a functional state update for stage advancement.
advance currently does setStage(stage + 1), which can drop increments if advance is triggered multiple times within the same React batch/event. Use the functional updater instead (Line 25-27).

Suggested fix
   const advance = () => {
-    setStage(stage + 1);
+    setStage((prev) => prev + 1);
   };
🤖 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 `@src/renderer/src/components/onboarding/main.tsx` around lines 25 - 27, The
advance function uses setStage(stage + 1) which can lose updates when called
multiple times in one batch; change it to use the functional updater form — call
setStage(prev => prev + 1) inside advance so state increments are applied
reliably (locate the advance function and the setStage call).
🤖 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.

Outside diff comments:
In `@src/renderer/src/components/onboarding/main.tsx`:
- Around line 25-27: The advance function uses setStage(stage + 1) which can
lose updates when called multiple times in one batch; change it to use the
functional updater form — call setStage(prev => prev + 1) inside advance so
state increments are applied reliably (locate the advance function and the
setStage call).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a53f1e79-cc9f-40f7-b42b-e7149f173d3d

📥 Commits

Reviewing files that changed from the base of the PR and between df0a465 and 54a22df.

📒 Files selected for processing (8)
  • src/renderer/src/components/onboarding/main.tsx
  • src/renderer/src/lib/omnibox-new/search-providers/duckduckgo.ts
  • src/renderer/src/lib/omnibox-new/search-providers/google.ts
  • src/renderer/src/lib/omnibox-new/search-providers/suggestion-utils.ts
  • src/renderer/src/lib/omnibox-new/search-providers/types.ts
  • src/renderer/src/lib/omnibox-new/search-providers/url-utils.ts
  • src/renderer/src/lib/omnibox-new/search-providers/yandex.ts
  • src/renderer/src/lib/search.ts

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.

1 participant