feat(tinyfish): make search provider permissive, attribute errors#31997
Merged
tin-berri merged 7 commits intoJul 3, 2026
Merged
Conversation
Reshapes the TinyFish search provider so LiteLLM mirrors the TinyFish
Search API surface instead of maintaining a parallel cherry-pick.
Request side:
- Drop misleading request TypedDict
- Stop sending max_results on wire (TinyFish ignores it); clamp to [1,10]
client-side via self-threaded state
- Guard non-numeric max_results from bare ValueError
- Auto-JSON-encode dict params; lowercase bool serialization for ux-labs
Response side:
- Drop both Pydantic response models; parse directly into SearchResponse
so per-result extras flow through via extra="allow"
- Default missing title/url/snippet to "" instead of failing the call
- Read top-level parameter_warnings and re-fire as verbose_logger.warning
(pre-wired for upcoming TinyFish-side rollout; no-op today)
Error handling:
- Attributed _wrap_error helper at 3 call sites in transform_search_response
("TinyFish Search: <msg>. See https://docs.tinyfish.ai/search-api for
details.")
- Dispatch non-2xx responses through _wrap_error (fixes pre-existing bug
where 4xx/5xx silently returned empty SearchResponse)
- Wrap json.JSONDecodeError on 200 bodies
- Wrap pydantic.ValidationError for envelope-shape mismatches
Bug fix worth flagging: 4xx/5xx responses now raise an attributed
BaseLLMException instead of silently returning SearchResponse(results=[]).
Follow-up to #30634.
…clamp - Run ruff format on the touched files (CI lint job rejected the prior commit's formatting). - Add OverflowError to the except clause in the max_results clamp so callers passing math.inf (or other non-finite floats) get the same warn-and-ignore behavior as other malformed values. Greptile spotted this in the first-pass review. - Add test_max_results_infinity_float_warns_and_skips covering the inf case.
CI uses 'ruff format --check --line-length 88'; my prior format pass used the default line length, leaving several lines unwrapped. No behavior change — purely whitespace.
CI's ruff strict-rule budget rejected the prior commit with: - C901: transform_search_response complexity 16 > 10 (cap exceeded by 1) - I001: import sort violation (cap exceeded by 1) Extract two module-level helpers from transform_search_response to drop its cyclomatic complexity: - _default_missing_result_fields: in-place title/url/snippet defaulting - _emit_parameter_warnings: defensive parameter_warnings reader Auto-fix the import sort via ruff --fix. No behavior change; the 59 existing tests still pass.
…ields Codecov flagged 97.61% patch coverage (2 lines missing). The uncovered lines were the non-dict raw_json and non-dict per-result item early-exits in _default_missing_result_fields. Add two unit tests on the helper directly to bring patch coverage to 100%.
Contributor
Greptile SummaryThis PR reshapes the TinyFish search provider to mirror the TinyFish API surface more faithfully: it drops the custom
Confidence Score: 4/5Safe to merge; the core logic changes are well-tested and fix a real bug where 4xx/5xx responses silently returned empty results. The change is well-scoped and thoroughly tested with ~60 unit tests covering error paths, truncation, bool/dict param serialization, and parameter_warnings. The passthrough loop JSON-encodes dicts but not lists (contrary to the docstring), which would cause a confusing ValidationError from get_complete_url if a caller ever passes a list-valued param. The mutable _caller_max_results design is safe today because ProviderConfigManager instantiates a fresh config per call, but it introduces a silent coupling that would break max_results for concurrent calls if that ever changes. litellm/llms/tinyfish/search/transformation.py — the passthrough serialization loop and the instance-state threading between the two transform methods.
|
| Filename | Overview |
|---|---|
| litellm/llms/tinyfish/search/transformation.py | Major refactor: drops custom Pydantic response models, routes non-2xx/JSON-decode/schema errors through BaseLLMException, and stores max_results as instance state for client-side truncation. A docstring claims list passthrough params are JSON-encoded but the code only serializes dicts. |
| tests/test_litellm/llms/tinyfish/test_tinyfish_search.py | Tests updated to reflect new behavior: max_results stored on instance state, new error-handling cases, parameter_warnings, and extra per-result field pass-through. All tests use mock responses; no real network calls. |
| tests/search_tests/test_tinyfish_search.py | Integration-level test adds fetch param round-trip test and updates max_results test; the new test correctly patches AsyncHTTPHandler so no real network calls are made. |
Reviews (1): Last reviewed commit: "chore(tinyfish): apply ruff format to fi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+147
to
+156
| # `fetch` expects a JSON-encoded object on the wire; accept the | ||
| # natural Python dict form and serialize here so callers don't | ||
| # have to pre-stringify. | ||
| if isinstance(value, dict): | ||
| value = json.dumps(value, separators=(",", ":")) | ||
| # `urlencode` would render Python bool as "True"/"False" | ||
| # (capitalized). ux-labs validators require lowercase | ||
| # "true"/"false" (e.g. `include_thumbnail`); normalize here. | ||
| elif isinstance(value, bool): | ||
| value = "true" if value else "false" |
Contributor
There was a problem hiding this comment.
urlencode", but the passthrough loop only handles dict; a list value falls through unchanged. get_complete_url then runs _UrlEncodableParams.validate_python (typed dict[str, str | int | bool]), which raises a Pydantic ValidationError on any list value, surfacing as a confusing internal error instead of a clean caller-facing one. Either serialize lists or remove the claim from the docstring.
Suggested change
| # `fetch` expects a JSON-encoded object on the wire; accept the | |
| # natural Python dict form and serialize here so callers don't | |
| # have to pre-stringify. | |
| if isinstance(value, dict): | |
| value = json.dumps(value, separators=(",", ":")) | |
| # `urlencode` would render Python bool as "True"/"False" | |
| # (capitalized). ux-labs validators require lowercase | |
| # "true"/"false" (e.g. `include_thumbnail`); normalize here. | |
| elif isinstance(value, bool): | |
| value = "true" if value else "false" | |
| # `fetch` expects a JSON-encoded object on the wire; accept the | |
| # natural Python dict/list form and serialize here so callers don't | |
| # have to pre-stringify. | |
| if isinstance(value, (dict, list)): | |
| value = json.dumps(value, separators=(",", ":")) | |
| # `urlencode` would render Python bool as "True"/"False" | |
| # (capitalized). ux-labs validators require lowercase | |
| # "true"/"false" (e.g. `include_thumbnail`); normalize here. | |
| elif isinstance(value, bool): | |
| value = "true" if value else "false" |
Comment on lines
+37
to
+43
| def __init__(self) -> None: | ||
| super().__init__() | ||
| # Threaded from transform_search_request → transform_search_response so the | ||
| # response slice honors the caller's max_results without re-sending it on | ||
| # the wire (TinyFish doesn't honor it server-side). Safe because the | ||
| # config is instantiated per-call via ProviderConfigManager. | ||
| self._caller_max_results: int | None = None |
Contributor
There was a problem hiding this comment.
_caller_max_results is set in transform_search_request and read in transform_search_response, relying on the guarantee that ProviderConfigManager.get_search_provider_config always calls config_class() fresh (confirmed at utils.py:8822). The design is safe today, but it creates an invisible coupling: if the instantiation site is ever cached for performance, concurrent searches through the same config instance would silently return wrong result counts with no error. Consider passing the value explicitly (e.g., in the kwargs threading already available on both methods) to make the data flow self-evident and immune to changes in how the config is managed.
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!
Contributor
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2af27aa. Configure here.
yuneng-berri
approved these changes
Jul 3, 2026
6 tasks
ChenluJi
added a commit
to ChenluJi/litellm
that referenced
this pull request
Jul 7, 2026
Follow-up to BerriAI#31411 (superseded and merged as BerriAI#31997). Two related fixes so LiteLLM callers see what TinyFish actually returns. ## Top-level response extras (query, total_results, page, future fields) transform_search_response was constructing a fresh SearchResponse from just `results`, silently dropping every top-level extra that SearchResponse.model_validate(raw_json) had captured via extra='allow'. Reproduced twice against production — resp.query, resp.total_results, resp.page all absent, model_extra empty. Fix: spread parsed.model_extra at construction time. return SearchResponse( results=list(parsed.results[:max_results]), **(parsed.model_extra or {}), ) Future-proof: any new top-level field TinyFish adds rides through with no LiteLLM code change (SearchResponse's extra='allow' captures every non-declared field into parsed.model_extra automatically). ## Response headers surfaced on _hidden_params TinyFish sets useful response headers (X-Request-ID on every response; Retry-After and X-RateLimit-Limit on 429s). These were only accessible via BaseLLMException.headers on error paths; on the success path they were dropped entirely. Fix: stash headers on both LiteLLM-conventional channels, matching the convention used by Gemini/Volcengine/Manus/ChatGPT/OpenAI responses providers. response._hidden_params['headers'] = dict(raw_response.headers) response._hidden_params['additional_headers'] = ( process_response_headers(dict(raw_response.headers)) ) - 'headers' — raw dict for debugging. - 'additional_headers' — passed through process_response_headers so downstream LiteLLM consumers get sanitized keys (strips any x-litellm-* a misbehaving provider might set). Future-proof: no allowlist, no filtering. Any future response header flows through automatically. ## Tests Adds 5 new mock-only unit tests: - test_top_level_extras_flow_through - test_top_level_future_extras_flow_through - test_response_headers_stashed_on_hidden_params - test_response_headers_future_headers_flow_through - test_response_headers_strips_x_litellm_spoof 66 unit + integration tests pass locally. Live-tested against production TinyFish (23/23 regression checks). No request-side changes. No behavior change for per-result extras (already correct). Error paths unchanged.
ChenluJi
added a commit
to ChenluJi/litellm
that referenced
this pull request
Jul 7, 2026
Follow-up to BerriAI#31411 (superseded and merged as BerriAI#31997). Two related fixes so LiteLLM callers see what TinyFish actually returns. ## Top-level response extras (query, total_results, page, future fields) transform_search_response was constructing a fresh SearchResponse from just `results`, silently dropping every top-level extra that SearchResponse.model_validate(raw_json) had captured via extra='allow'. Reproduced twice against production — resp.query, resp.total_results, resp.page all absent, model_extra empty. Fix: spread parsed.model_extra at construction time. return SearchResponse( results=list(parsed.results[:max_results]), **(parsed.model_extra or {}), ) Future-proof: any new top-level field TinyFish adds rides through with no LiteLLM code change (SearchResponse's extra='allow' captures every non-declared field into parsed.model_extra automatically). ## Response headers surfaced on _hidden_params TinyFish sets useful response headers (X-Request-ID on every response; Retry-After and X-RateLimit-Limit on 429s). These were only accessible via BaseLLMException.headers on error paths; on the success path they were dropped entirely. Fix: stash headers on both LiteLLM-conventional channels, matching the convention used by Gemini/Volcengine/Manus/ChatGPT/OpenAI responses providers. response._hidden_params['headers'] = dict(raw_response.headers) response._hidden_params['additional_headers'] = ( process_response_headers(dict(raw_response.headers)) ) - 'headers' — raw dict for debugging. - 'additional_headers' — passed through process_response_headers so downstream LiteLLM consumers get sanitized keys (strips any x-litellm-* a misbehaving provider might set). Future-proof: no allowlist, no filtering. Any future response header flows through automatically. ## Tests Adds 5 new mock-only unit tests: - test_top_level_extras_flow_through - test_top_level_future_extras_flow_through - test_response_headers_stashed_on_hidden_params - test_response_headers_future_headers_flow_through - test_response_headers_strips_x_litellm_spoof 66 unit + integration tests pass locally. Live-tested against production TinyFish (23/23 regression checks). No request-side changes. No behavior change for per-result extras (already correct). Error paths unchanged.
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
3 participants
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.
Relevant issues
Supersedes #31411. Same change set, moved from the fork branch
ChenluJi:feat/tinyfish-search-mirror-api-surfaceonto an internallitellm_branch so it follows the internal-contributor convention. Original work by @ChenluJi; the only thing added on top is a formatting-only commit that runsruff formaton the touched file to satisfy the lint job after the latestlitellm_internal_stagingmergeLinear ticket
n/a (the code references ML-2084 and ML-2085 for planned TinyFish-side follow-ups)
Pre-Submission checklist
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewGreptile already reviewed the identical change on #31411; re-requesting here on the moved branch
Screenshots / Proof of Fix
This PR carries the reviewed change from #31411 with no behavior difference apart from the formatting-only commit, so the original test plan on that PR stands. See #31411 for the 58 unit and integration tests and the live TinyFish endpoint runs
Type
🧹 Refactoring
Changes
Reshapes the TinyFish search provider so LiteLLM mirrors the TinyFish Search API surface instead of maintaining a parallel cherry-pick. On the request side it drops the misleading request TypedDict, stops sending
max_resultson the wire (TinyFish ignores it server-side) and instead clamps to the natural SERP ceiling and truncates client-side, guards non-numericmax_resultsfrom raising, and auto-encodes dict and bool passthrough params so they survive urlencode. On the response side it drops both Pydantic response models and parses directly intoSearchResponseso per-result extras ride through, defaults missing title/url/snippet to empty strings so a single degraded result no longer fails the whole call, and reads TinyFish'sparameter_warningswhen present. On the error side it routes non-2xx, JSON-decode, and schema-mismatch failures through an attributedBaseLLMException, which also fixes a pre-existing bug where 4xx and 5xx responses silently returned an empty result setNote
Medium Risk
Behavior change for TinyFish search callers: HTTP and parse failures now raise instead of returning empty results, and default result cap shifts from 20 to 10 when max_results is unset.
Overview
TinyFish search is reshaped so LiteLLM tracks the TinyFish API surface more closely instead of maintaining parallel request/response models.
Requests:
max_resultsis no longer sent on the query string (TinyFish ignores it); the value is clamped to 1–10, stored on the per-call config, and applied when slicing results. Unknown optional params (e.g.fetch) pass through, with dict values JSON-encoded and bools normalized to lowercasetrue/falsefor urlencode. Domain filters still fold into the query viasite:clauses.Responses: Parsing goes directly into unified
SearchResponseso per-result extras (fetch,position, etc.) ride through. Missing/null title/url/snippet are defaulted to""so one bad hit does not fail the call. Top-levelparameter_warningsare re-logged when present.Errors: Non-2xx responses, non-JSON bodies, and schema mismatches now raise attributed
BaseLLMExceptionmessages (with ux-labs error unwrapping and docs link)—fixing the prior behavior where HTTP errors could look like empty result sets.Tests cover passthrough, truncation, warnings, error paths, and an integration
fetchround-trip.Reviewed by Cursor Bugbot for commit 2af27aa. Bugbot is set up for automated code reviews on this repo. Configure here.