fix: review findings from initial scaffold#2
Merged
Conversation
Two real bugs caught by extended smoke + four parallel reviewers:
Bugs (would have shipped broken in 0.0.1):
- _enrich: client.task_run.execute(output=dict) raises TypeError; switched
to client.beta.task_run.create(task_spec={"output_schema": {"type":"json",
"json_schema": ...}}) + task_run.result(api_timeout=...) for the create+poll
path. The dict envelope is required by the API contract.
- extract: full_content=True was a top-level kwarg the SDK rejects. Now nested
under advanced_settings={"full_content": True}.
Code quality:
- Tool names now public (web_search, web_fetch, extract, deep_research, enrich)
instead of leading-underscore — LLMs handle clean names better.
- Submodules renamed to _search, _extract, _task to avoid attribute/submodule
shadowing in the package namespace.
- ParallelTracingPlugin keys per-call timing on ToolContext.function_call_id
(concurrency-safe vs the prior id(tool_args) approach which leaked on errors)
and adds on_tool_error_callback to record failures.
- __version__ now read from importlib.metadata (was hardcoded, drifted).
- enrich passes api_timeout to result() instead of timeout to create().
- Dropped enrich_with_model helper (was untested + unused public API).
Packaging / CI:
- pyproject: parallel-web>=0.5.1 (was >=0.3, broken on older), google-adk>=1.31.
- pyproject: Documentation URL points at GitHub README until docs page exists.
- auto-release.yml: read commit message via env var, not direct interpolation
(closes shell-injection vector via crafted commit messages).
- release.sh: refuses to run off main, refuses if release tag already exists.
- README + integration-docs: fixed StreamableHTTPServerParams (doesn't exist)
to StreamableHTTPConnectionParams.
Tests:
- New test_search.py, test_extract.py, test_task.py with monkeypatched SDK
client (string-target setattr to handle the namespace shadow).
- test_tracing.py: added concurrent-call test and on_tool_error_callback test.
- test_imports.py: parametrized tool name check; version asserted against
importlib.metadata.
- Moved smoke_live*.py to tests/integration/ with addopts ignore so default
pytest no longer needs --ignore flags.
- Live extended smoke (tests/integration/smoke_live_full.py) verifies _enrich
end-to-end against api.parallel.ai + LLM picking web_fetch and extract.
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
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.
Summary
Closes the gaps from four parallel reviewers (code, tests, docs, workflows) and the extended live smoke against api.parallel.ai. Two real bugs would have shipped broken in 0.0.1 — both caught and fixed here.
Bugs fixed
enrichwas broken end-to-end. The SDK rejects JSON-Schema dicts viatask_run.execute(output=…)(it accepts only Pydantic classes or strings). Routes viabeta.task_run.create+task_run.resultinstead, with the discriminated-union envelope the API requires:Verified live — returns
run_id+ cited basis.extract(full_content=True)was broken. Top-levelfull_contentis rejected by the SDK; it lives insideadvanced_settings. The original smoke only testedFalse(the default), so this would have hit the first user who flipped it. Fixed + regression test added.Code quality
web_search,web_fetch,extract,deep_research,enrich. The leading-underscore variants confused LLMs and didn't match the doc-published names._search.py/_extract.py/_task.pyto avoid attribute-vs-submodule shadowing in the package namespace.ToolContext.function_call_id(concurrency-safe across overlapping sessions) and addson_tool_error_callbackso timing entries don't leak when tools error out.__version__read fromimportlib.metadata— fixes the 0.0.1 vs 0.1.0 drift the reviewers flagged.enrich_with_modelhelper.Packaging / CI
parallel-web>=0.5.1(was>=0.3— broken on older),google-adk>=1.31.DocumentationURL points at the GitHub README until a real docs page exists.auto-release.ymlreadshead_commit.messagevia env var, not direct${{ }}interpolation — closes a shell-injection vector via crafted commit messages.release.shrefuses to run offmain, refuses if the release tag already exists.StreamableHTTPServerParams(doesn't exist) →StreamableHTTPConnectionParams.Tests
test_search.py,test_extract.py,test_task.pywith monkeypatched SDK clients. Covers call shapes (max_resultsinadvanced_settings,full_contentnesting,enrichJSON parse errors,_format_resultbranches).function_call_idkeys don't race) and an error-path test for the newon_tool_error_callback.tests/integration/; defaultpytestno longer needs--ignoreflags.Verification (run on the HEAD of this branch)
pytest -vruff check + format --checkapi.parallel.ai_enrichdirect returnsrun_id+ cited basis; Gemini Flash picksweb_fetchandextractwith correctcitation_counttracesSequencing
Should merge before PR #1 (the 0.0.1 release). Otherwise 0.0.1 ships broken on PyPI. Once this lands, PR #1 needs a rebase or a re-open on top of main.