Repository navigation
🧭 feat: Add Tavily Search And Scraper Providers - #135
Conversation
- Replace per-provider union types with `AnyScraperResponse` named type - Add optional `scrapeUrls` batch method to `BaseScraper` interface - Rename `raw_content` to `rawContent` in `TavilyScrapeResponse` (camelCase convention) - Add `TavilySearchResult` and `TavilyExtractResult` named types - Split `TAVILY_API_URL` into separate `tavilySearchUrl` and `tavilyExtractUrl` fields - Add configurable `searchDepth` to `SearchConfig` - Flatten `TavilyConfig` into `SearchToolConfig` via `SearchConfig` extension
Major fixes: - Wrap `new URL().hostname` in try/catch to prevent single bad URL from discarding entire result set (Finding #1) - Remove Tavily from country schema — Tavily API does not support country filtering (Finding #2) - Split shared `TAVILY_API_URL` into `TAVILY_SEARCH_URL` and `TAVILY_EXTRACT_URL` to avoid env var collision (Finding #3) - Reduce default timeout from 60s to 15s, matching other providers (Finding #4) - Implement batch URL extraction via `scrapeUrls` — Tavily Extract supports up to 20 URLs per request (Finding #5) - Remove dead `topStories` computation; let `news` flow through the existing `executeParallelSearches` merge pipeline (Finding #6) - Use `scraper.extractMetadata()` abstraction instead of runtime `'metadata' in` check (Finding #8) - Read `failed_results` from Tavily Extract API for actionable error messages (Finding #12) - Add JSDoc for `'h'` → `'day'` time range approximation (Finding #13) - Make `search_depth` configurable via `searchDepth` config (Finding #14) - Hoist `TAVILY_TIME_RANGE_MAP` to module scope (Finding #15) - Use batch-aware `scrapeMany` that delegates to `scrapeUrls` when available on the scraper instance
Covers constructor defaults, env var handling, single URL scraping, batch scraping with chunking, mixed success/failure, extractContent, and extractMetadata — 18 tests total.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77318d8a6a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const getSources = async ({ | ||
| query, | ||
| date, | ||
| numResults = 8, | ||
| type, | ||
| news, | ||
| }: t.GetSourcesParams): Promise<t.SearchResult> => { |
There was a problem hiding this comment.
The new Tavily provider drops localization input because getSources does not read country from GetSourcesParams, so the payload can never include a country constraint. executeParallelSearches still passes country for all searches, so users switching from Serper/SearXNG lose expected locale targeting with Tavily and get broader/global results instead. Tavily’s Search API supports a country field, so this is a functional regression for country-scoped queries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 23f8a05. Tavily search now forwards the normalized country filter in the API payload, and the Tavily tool schema exposes the country field when Tavily is selected. Covered by the updated Tavily search unit test and verified with NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBand plus npx tsc --noEmit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23f8a05c3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (searchProvider === 'serper' || searchProvider === 'tavily') { | ||
| schemaProperties.country = countrySchema; |
There was a problem hiding this comment.
This change enables country for Tavily by reusing countrySchema, whose prompt text tells the model to send 2-letter codes (e.g. us, de). Tavily Search expects full country names (e.g. united states), so requests generated from this schema are likely to be ignored or fail to localize correctly once routed through Tavily. Please add a Tavily-specific country schema (or translate codes to Tavily country names) before enabling this field for that provider.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1d661de. I kept the shared two-letter country schema exposed for Tavily, but translate those inputs to Tavily country-name values before building the request payload, including uk -> united kingdom. The Tavily search test now expects US to be sent as united states.
| const normalizedCountry = country?.trim().toLowerCase(); | ||
| if (normalizedCountry != null && normalizedCountry !== '') { | ||
| payload.country = normalizedCountry; |
There was a problem hiding this comment.
country when topic is not general
The request builder always sets payload.country when a country is provided, but Tavily documents country as valid only for topic: "general". In this tool flow, news searches call Tavily with type: 'news' (thus topic: 'news') while still forwarding country, which can cause request validation errors or dropped news results whenever users include country filtering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1d661de. Tavily country is now added only when the resolved topic is general, so news-topic searches omit country. Covered by a new Tavily unit test that calls getSources with news: true and asserts the payload has topic: news without country. Local verification: NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBand and npx tsc --noEmit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d661dee1d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const payload: Record<string, unknown> = { | ||
| query, | ||
| search_depth: searchDepth, | ||
| topic, | ||
| max_results: Math.min(Math.max(1, maxResults), 20), | ||
| }; |
There was a problem hiding this comment.
The Tavily integration drops the safeSearch setting entirely: getSources never reads it and the request payload never sets Tavily’s safe_search field, so searches run with Tavily’s default safe_search=false even when callers configure moderation (including the tool’s default safe mode). This is a functional regression for safety-sensitive deployments using searchProvider: 'tavily', because unsafe/adult results are no longer filtered when they were expected to be.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6c4560e. Tavily getSources now reads safeSearch and sends safe_search in the request payload, defaulting to enabled unless callers explicitly pass safeSearch: 0. The Tavily search unit test covers safe_search: true for moderated search and safe_search: false for explicit off. Local verification: NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBand and npx tsc --noEmit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c4560ee85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| search_depth: searchDepth, | ||
| topic, | ||
| max_results: Math.min(Math.max(1, maxResults), 20), | ||
| safe_search: (safeSearch ?? 1) !== 0, |
There was a problem hiding this comment.
Always setting safe_search in the Tavily payload can break valid configurations: this field is enterprise-only and explicitly unsupported for fast/ultra-fast depths, but the new code sends it for every request (including when searchDepth is fast or ultra-fast). In those cases Tavily can reject the request, causing the entire search call to fail instead of returning results.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 36b1a39. Tavily safe_search is now only included for supported search depths; fast and ultra-fast omit the field so those configurations remain valid. Added a Tavily unit test for searchDepth: fast that asserts safe_search is not sent. Local verification: NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBand and npx tsc --noEmit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36b1a396bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (/^[a-z]{2}$/.test(normalizedCountry)) { | ||
| return TAVILY_REGION_NAMES.of( | ||
| normalizedCountry.toUpperCase() | ||
| )?.toLowerCase(); |
There was a problem hiding this comment.
The 2-letter country normalization currently relies on Intl.DisplayNames, which can emit names that do not match Tavily’s documented country enum (for example cz becomes czechia, while Tavily expects czech republic). In those cases, localized searches can fail with 400s or silently lose localization, so valid user inputs become unreliable depending on the country code.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ee25701. Country normalization now includes Tavily-specific enum aliases for ISO codes whose Intl display names differ from Tavily values, including cz -> czech republic, plus punctuation/diacritic normalization for display-name fallbacks. Added a Tavily request test that sends country: CZ and asserts country: czech republic.
| const effectiveTimeout = options.timeout ?? this.timeout; | ||
| payload.timeout = Math.min(Math.max(effectiveTimeout / 1000, 1), 60); |
There was a problem hiding this comment.
This always sends a timeout value derived from the client timeout, including the default 15000ms (15s). Tavily’s Extract API applies depth-aware defaults when timeout is omitted (10s for basic, 30s for advanced), so this overrides advanced requests to a shorter timeout and can cause avoidable extraction failures on slower pages.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ee25701. Tavily extract payloads now omit timeout when no timeout was explicitly configured, preserving Tavily depth-aware defaults for basic/advanced extraction while still keeping the local axios timeout. Explicit per-call/configured timeouts continue to be sent. Added a Tavily scraper test for advanced extraction that asserts payload.timeout is omitted. Local verification: NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBand and npx tsc --noEmit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee25701ca4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (searchDepth !== 'fast' && searchDepth !== 'ultra-fast') { | ||
| payload.safe_search = (safeSearch ?? 1) !== 0; | ||
| } |
There was a problem hiding this comment.
This path always injects safe_search for Tavily when search_depth is basic/advanced, and with the tool defaults that means every Tavily request sends safe_search: true unless callers explicitly override it. Tavily documents safe_search as an enterprise-only option, so standard-plan keys can be rejected for otherwise valid searches; gate this field behind an explicit Tavily option (or omit it unless the caller opts in) to keep baseline Tavily usage working.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 21802df. Tavily safe_search is now gated behind the explicit tavilySearchOptions.safeSearch flag and is no longer sent from the generic safeSearch default, so standard Tavily requests omit the enterprise-only field. The unsupported-depth guard still applies for fast/ultra-fast. Updated tests cover explicit opt-in, default omission, and fast-depth omission. Local verification: NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBand and npx tsc --noEmit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21802df699
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (options?.chunksPerSource != null) { | ||
| payload.chunks_per_source = options.chunksPerSource; |
There was a problem hiding this comment.
Only advanced Tavily searches accept chunks_per_source, but this code always forwards chunksPerSource whenever it is set in config. With the default search_depth of basic, requests that include this field can be rejected by Tavily, turning valid searches into provider errors for users who enable chunk tuning without also forcing advanced depth.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7996aec. Tavily search now only forwards chunks_per_source when searchDepth is advanced; basic/fast/ultra-fast omit it. Added a unit test that verifies basic omits chunks_per_source while advanced sends it.
| if (chunksPerSource != null) { | ||
| payload.chunks_per_source = chunksPerSource; | ||
| } |
There was a problem hiding this comment.
The Tavily scraper exposes chunksPerSource and unconditionally sends chunks_per_source, but this extract flow never sends a query field. Tavily Extract only supports chunk selection for query-reranked extraction, so enabling tavilyScraperOptions.chunksPerSource can cause every scrape in that batch to fail with API validation errors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7996aec. Tavily extract no longer exposes or sends chunks_per_source because this scraper flow does not provide a query for query-reranked extraction. Updated the scraper option test to assert chunks_per_source is absent. Local verification: NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBand and npx tsc --noEmit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7996aeca31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| options?.safeSearch != null && | ||
| searchDepth !== 'fast' && | ||
| searchDepth !== 'ultra-fast' | ||
| ) { | ||
| payload.safe_search = options.safeSearch; |
There was a problem hiding this comment.
executeParallelSearches passes safeSearch on every search request, but the Tavily path only reads options?.safeSearch from static tavilySearchOptions. As a result, createSearchTool({ safeSearch: ... }) is ignored for Tavily unless callers also duplicate that setting in tavilySearchOptions, which silently drops the expected safety filtering behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 88fef38. Explicit createSearchTool({ safeSearch }) now becomes tavilySearchOptions.safeSearch for Tavily, so configured safety filtering is honored while the default still omits the enterprise-only safe_search field. Added a high-level search tool test that asserts safe_search is sent when safeSearch: 2 is explicit.
| organic: organicResults, | ||
| images: imageResults, | ||
| topStories: [], | ||
| videos: [], |
There was a problem hiding this comment.
The Tavily mapper always emits videos: [], and this implementation never handles type === 'videos', so a videos subquery still incurs an extra Tavily request but cannot contribute video results. In the videos: true flow this adds cost and latency with no user-visible data unless a video-specific mapping is implemented or the call is skipped.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 88fef38. Tavily search tools now skip video subqueries because the Tavily mapper does not produce video results. The same high-level test passes videos: true and asserts only one Tavily API call is made. Local verification: NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBand and npx tsc --noEmit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88fef383c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| config.apiUrl ?? | ||
| process.env.TAVILY_EXTRACT_URL ?? | ||
| 'https://api.tavily.com/extract'; | ||
| this.timeout = config.timeout ?? DEFAULT_TIMEOUT; |
There was a problem hiding this comment.
The new scraper hardcodes a 15s client timeout (DEFAULT_TIMEOUT) regardless of extractDepth, but Tavily advanced extraction allows significantly longer processing when no explicit per-request timeout is set. With extractDepth: 'advanced' and default settings, requests that legitimately take longer than 15s will be aborted client-side and reported as failures even though the provider would still return successfully, which can make advanced scraping unreliable on heavier pages.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f9b66d3. Tavily extract now uses a 30s client timeout for advanced extraction defaults, including per-call advanced overrides, while still omitting the Tavily payload timeout unless the user explicitly configures one. Covered by the focused Tavily scraper tests and npx tsc --noEmit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9b66d3055
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| searchProvider === 'tavily' && config.safeSearch != null | ||
| ? { | ||
| safeSearch: config.safeSearch !== 0, | ||
| ...tavilySearchOptions, |
There was a problem hiding this comment.
createSearchTool sets safeSearch = 1 by default, but this value is never propagated to Tavily unless config.safeSearch is explicitly set. In the common case where callers rely on defaults, effectiveTavilySearchOptions stays undefined and Tavily requests omit safe_search, falling back to Tavily’s default (false). That makes Tavily return less-filtered content than other providers under the same default tool config.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I am leaving this unchanged. Tavily documents safe_search as enterprise-only and unsupported for fast/ultra-fast search depths (https://docs.tavily.com/documentation/api-reference/endpoint/search), so propagating the default tool safeSearch value would make baseline Tavily usage fail for standard-plan keys. The current behavior still honors explicit safety config: callers can set safeSearch on the tool/config or tavilySearchOptions.safeSearch, and the Tavily payload sends safe_search only for supported depths.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9b66d3055
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (/^[a-z]{2}$/.test(normalizedCountry)) { | ||
| const regionName = TAVILY_REGION_NAMES.of(normalizedCountry.toUpperCase()); | ||
| return regionName != null | ||
| ? normalizeTavilyCountryName(regionName) | ||
| : undefined; |
There was a problem hiding this comment.
The 2-letter country path currently converts any ISO code via Intl.DisplayNames and forwards it directly to Tavily, but Tavily only accepts a fixed country enum. Codes like CD map to values such as congo - kinshasa (and other non-enum names), which can make otherwise valid searches fail with a request error once payload.country is set. This should be gated by an allowlist of Tavily-supported country values (or omitted when the mapped name is not supported).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 897cf8f. Tavily country normalization now validates the mapped value against the supported Tavily country enum before adding payload.country; unsupported ISO mappings such as CD are omitted instead of being forwarded. Added a Tavily unit test for the unsupported ISO path. Local verification: NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBand and npx tsc --noEmit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 897cf8f069
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| scraperInstance = createTavilyScraper({ | ||
| ...tavilyScraperOptions, | ||
| apiKey: tavilyApiKey ?? process.env.TAVILY_API_KEY, | ||
| apiUrl: tavilyExtractUrl, | ||
| timeout: scraperTimeout ?? tavilyScraperOptions?.timeout, |
There was a problem hiding this comment.
Do not overwrite tavilyScraperOptions connection fields with possibly undefined top-level values. In this object literal, apiUrl: tavilyExtractUrl and apiKey: tavilyApiKey ?? process.env.TAVILY_API_KEY run after ...tavilyScraperOptions, so a caller-provided tavilyScraperOptions.apiUrl/apiKey is discarded whenever the top-level fields are unset. In deployments that route Extract through a custom proxy or per-scraper credential, the scraper silently falls back to the default endpoint/env key and requests fail or go to the wrong backend.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 35a50b5. Tavily scraper-level apiKey and apiUrl now take precedence over top-level Tavily values when creating the scraper, so custom extract proxies and per-scraper credentials are preserved. Added a search-tool regression test that verifies search uses the top-level key while extract uses the scraper override URL/key. Local verification: NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBand and npx tsc --noEmit.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68fcec60da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const topic = | ||
| options?.topic ?? | ||
| (news === true || type === 'news' ? 'news' : 'general'); |
There was a problem hiding this comment.
Make the request-level news intent take precedence over tavilySearchOptions.topic. Right now options?.topic is chosen first, so if a tool is configured with topic: 'finance' (or 'general'), a later type: 'news' request is silently downgraded and no news items are produced/mapped for that call. This breaks the news flag path in executeParallelSearches for those configurations and causes users to request news but receive non-news search behavior.
Useful? React with 👍 / 👎.
| safeSearch: config.safeSearch !== 0, | ||
| ...tavilySearchOptions, |
There was a problem hiding this comment.
Ensure explicit safeSearch in createSearchTool wins over tavilySearchOptions.safeSearch. The current merge order sets safeSearch and then spreads tavilySearchOptions, so any safeSearch inside provider options overrides the top-level setting. In practice, configurations that set safeSearch: 0 (or 2) can be silently inverted by inherited Tavily options, producing unexpected filtering behavior at runtime.
Useful? React with 👍 / 👎.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d1c305d46
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| searchProvider === 'tavily' && config.safeSearch != null | ||
| ? { | ||
| ...tavilySearchOptions, | ||
| safeSearch: config.safeSearch !== 0, | ||
| } |
There was a problem hiding this comment.
createSearchTool assigns safeSearch = 1 by default, but this block only forwards a Tavily safeSearch value when config.safeSearch is explicitly provided. In the common case where callers rely on defaults, Tavily requests are emitted without safe_search, so filtering behavior differs from the tool’s configured default and from other providers unless users manually set safeSearch every time.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this intentionally unchanged. Tavily safe_search is enterprise-only, and earlier review feedback required avoiding default propagation so basic/non-enterprise Tavily configs do not break. The code still honors explicit safeSearch values: Tavily-specific tavilySearchOptions.safeSearch is forwarded, and an explicitly provided top-level safeSearch now takes precedence over that provider option. The default tool value is intentionally not sent to Tavily.
3 participants
I continued #85 on a maintainer-owned branch because the contributor fork rejected maintainer pushes with HTTP 403, and rebased the Tavily work onto latest
origin/dev.includeAnswerandincludeRawContent, and map Tavily answers/images into the existing search result shape.Change Type
Testing
NODE_OPTIONS="--experimental-vm-modules" npx jest src/tools/search/tavily.test.ts --runInBandnpx tsc --noEmitTest Configuration:
origin/devChecklist