fix(web): restore keyed Parallel v1 API - #89766
Conversation
Co-authored-by: Pavel Tajduš <6259149+PavelTajdus@users.noreply.github.com>
|
@alexferrari88 Thanks for the PR! This is very close, but the legacy mode mappings are reversed relative to Parallel’s migration guide. agentic should map to advanced, while one-shot and legacy fast should map to basic. Right now the default Hermes behavior silently changes from the old agentic quality profile to basic. The current V1 fast mode is a new mode with no Beta equivalent, so we should also decide how to expose it without silently changing existing PARALLEL_SEARCH_MODE=fast users. Migration docs: https://docs.parallel.ai/search/search-migration-guide Happy to help with the patch or add a real-SDK MockTransport test if useful. |
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head 73141f4d02c3eb5da91b15c9f6f372d1a33749cd against recorded base 292d0df04dfe040506694d4f180ccc75a935d2e6 and current main fab8479aa005e3c6f9e779a2b9da4e186a17db4a. There were no prior formal reviews on this PR. Exact-head CI, Docker, and Nix are green; the branch is now 210 main commits behind and carries 2 commits not on main, so it needs a current-main rebase after the semantic fix below.
The GA/v1 transport repair itself is directionally correct: Parallel.search() / AsyncParallel.extract() are the right non-beta SDK surfaces, advanced_settings.max_results and advanced_settings.full_content match the generated 1.3.0 SDK contract, the exact pin is propagated through pyproject.toml, lazy deps, and uv.lock, and the normalized Hermes result/error shapes are preserved. I found one blocking compatibility defect, plus two merge-topology obligations.
Blocker — the legacy mode translation reverses the provider contract
_LEGACY_SEARCH_MODES currently maps:
agentic -> basicone-shot -> advanced- legacy
fast-> current v1fastby passthrough
That is not backward compatibility. It changes the behavior of existing keyed Hermes installations on upgrade.
The strongest evidence is upstream, not our local ticket:
@georgeatparallelalready called this out on this PR: Parallel's migration mapping isagentic -> advanced, whileone-shotand legacyfastmap tobasic; current v1fastis a new mode with no Beta-equivalent semantics.- Parallel's current first-party LangChain integration documents the same compatibility mapping: legacy
fast/one-shot->basic,agentic->advanced. - Parallel's generated Python SDK at the current 1.3.0 line confirms that v1 has four distinct wire values —
turbo,fast,basic,advanced— and defaults toadvancedwhen omitted. In other words, the fact that the stringfaststill parses does not mean oldfastand new v1fastmean the same thing.
The impact is immediate: Hermes currently defaults PARALLEL_SEARCH_MODE to agentic; this branch maps that default to basic, silently moving every keyed user from the old agent-oriented quality profile onto the lower-latency profile. Existing one-shot users get the inverse problem, and existing fast users are silently reinterpreted as a new v1 mode rather than migrated.
The tests currently make this worse by canonizing the wrong local specification: test_search_mode_resolves_legacy_and_v1_values asserts agentic -> basic, one-shot -> advanced, and the request-shape test asserts one-shot emits advanced. #89764 itself contains those same mappings, so the suite proves consistency with the issue text, not compatibility with the provider contract. This is exactly the class where an external protocol/provider contract outranks our ticket prose.
Required fix: preserve existing semantics first. At minimum, agentic -> advanced and one-shot -> basic. Legacy fast cannot safely become v1 fast merely because the spelling collides; either map existing fast to basic and expose the new v1 fast through an explicitly versioned/new configuration surface, or make a deliberate migration mechanism that can distinguish legacy intent from new-v1 intent. Also make the unset/invalid fallback preserve the old default behavior — advanced, not basic. Replace the current tests with provider-contract witnesses, including unset, agentic, one-shot, legacy fast, explicit v1 basic/advanced/turbo, and whatever explicit surface owns new v1 fast.
A real-SDK serialization test would be useful here too: the current SimpleNamespace fakes prove our kwargs and normalization, but they cannot catch a drift between our assumed request model and Stainless-generated validation. This is not a second blocker because the generated 1.3.0 types do confirm the current advanced_settings shapes.
Merge topology / provenance
- #24389 /
@PavelTajdusis earlier GA-migration prior art and should retain credit. - #34971 /
@NormallyGaussian, salvaged and merged through #43798 /@teknium1, is the direct lineage for the keyed v1 implementation. #46350 correctly removed the broader keyless/default behavior but unintentionally rolled the keyed path back to beta. This PR is the narrow restoration of that keyed slice, not a greenfield implementation. - #90313 /
@teknium1is overlapping, broader keyless/tier work and currently still targets the beta/0.4.2 keyed provider. Land this corrected keyed-v1 repair first, then restack #90313 so its Free/Paid architecture composes with the GA paid path rather than resurrecting beta. The two PRs are complementary in intent but overlapping in the same provider/config/dependency files. - #72367 /
@shaneblickis complementary final-URL validation. When it lands, its provider-reported-final-URL invariant must wrap the GA extract result path introduced here as well; do not let transport migration bypass that security boundary.
Re-review gate
- Correct the legacy-mode semantic mapping against upstream truth, including the
fastname collision. - Rewrite the tests so they discriminate old Hermes behavior -> intended v1 behavior rather than restating #89764's mistaken mapping.
- Rebase onto current main.
- Re-run focused provider tests plus exact-head CI/Docker/Nix.
- Compose #90313 on top of the corrected GA keyed path and keep the prior contributor lineage explicit.
Once those are done, I don't see another code-level blocker in this narrow PR.
Summary
parallel-webpin and lock state consistently to 1.3.0Fixes #89764.
Scope
This is intentionally the narrow keyed-only repair. It keeps
PARALLEL_API_KEYrequired and does not restore keyless/default behavior, the hosted MCP path, attribution, telemetry, branding, or display-label changes.Prior work and credit
Validation
scripts/run_tests.sh tests/plugins/web/test_parallel_provider.py tests/plugins/web/test_web_search_provider_plugins.py tests/tools/test_web_tools_config.py -q— 77 passedscripts/run_tests.sh tests/plugins/web/ tests/tools/test_web_tools_config.py tests/test_project_metadata.py tests/test_packaging_metadata.py tests/tools/test_web_providers_brave_free.py tests/tools/test_web_providers_ddgs.py tests/tools/test_web_providers_searxng.py -q— 128 passedscripts/run_tests.sh tests/plugins/web/test_parallel_provider.py -q— 11 passed on the final commituv run ruff check plugins/web/parallel/provider.py tests/plugins/web/test_parallel_provider.py tools/lazy_deps.py— passeduv lock --check— passed (253 packages resolved)git diff --check HEAD^..HEAD— passedNo live Parallel API request was sent because
PARALLEL_API_KEYwas not configured. Wire compatibility was verified against installedparallel-web1.3.0 signatures/models and mocked provider contracts. GitHub CI will provide the upstream checks.