feat: modernize langchain integration connector - #35
Conversation
Three-reviewer panel: staff-review, thermonuclear, code-review. All FAIL. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XGFrUYj223hDH4uJDQfQER
Independent re-validation of all panel findings: 7 confirmed, 3 overstated, 3 false positives (gpt-5-mini, missing clamp-test field, test tautology). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XGFrUYj223hDH4uJDQfQER
|
I pushed follow-up changes on top of this PR as #36 (base: Fixes
Quality (behaviour-equivalent)
Generated by Claude Code |
jirispilka
left a comment
There was a problem hiding this comment.
Thanks. Approving. Decide whether makes sense for you or not #36
Co-authored-by: Claude <noreply@anthropic.com>
git-cliff regenerates the unreleased section between its markers on every push to main, so hand-written entries there are overwritten. The breaking changes are conveyed via the BREAKING CHANGE: footer on the merge commit instead, which git-cliff renders automatically. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
👋 Testing note on this PR — looks solid overall (tools, retriever/loader, and LangChain agent-routing all validated live). One connector item to consider:
max_results cap doesn't match the Actor's real limit
ApifyRAGWebBrowserTool / ApifySearchRetriever advertise max_results as "clamped to 1000" (the generic _MAX_ITEMS_CAP), but the apify/rag-web-browser Actor's input schema enforces maxResults <= 100. _clamp_items clamps only to 1000, and ApifySearchRetriever (retrievers.py:97 / :112) passes max_results through with no clamp at all — so any value in (100, 1000] reaches the Actor and is rejected, and the call fails instead of clamping down.
Repro:
ApifyRAGWebBrowserTool().invoke({"query": "x", "max_results": 150})
# -> 'Apify Actor call failed ... Field input.maxResults must be <= 100'
# max_results=100 succeedsApifyScrapeUrlTool is unaffected (single URL, no max_results).
Two directions (leaving the choice to you)
- Minimal, matches this PR's pattern — a dedicated
_RAG_MAX_RESULTS_CAP = 100in_constants.py, referenced by the tool (max_items), the retriever, and the interpolated field description (keepstest_clamp_descriptions.pytruthful). One line to change if the Actor's limit ever moves. - Self-correcting — derive the cap from the Actor's input schema at init (
properties.maxResults.maximum) and clamp to it. The connector already reads Actor input schemas via_get_actor_latest_build()/_prune_actor_input_schema()forApifyActorsTool, so the machinery exists. This drops the need to hardcode/monitor per-Actor limits and generalizes to the other tools — at the cost of a fetch at construction and making the advertised-cap description dynamic (currently static + drift-tested). Could ride on the plannedActorClient.default_build()follow-up.
Either way test_clamp_descriptions.py will need a matching update. Happy to open a follow-up issue if that's easier to track. (Found via live testing; details on request.)
cc: @daveomri
✅ Option A implementation posted as inline suggestions in the review just below (apply as a batch — Add to batch → Commit suggestions). Option B remains the more robust alternative.
There was a problem hiding this comment.
Option A — inline suggestions (named constant). Draft of implementation of the constant-based fix from my comment above. _RAG_MAX_RESULTS_CAP plus its imports across 3 files, so applying them one at a time can transiently break until all land.
Option B (deriving the cap from the Actor's input schema at init) is still the more robust route if you'd rather not track per-Actor caps by hand.
After applying, run make test — test_clamp_descriptions.py may need a small tweak since ApifyRAGWebBrowserTool's advertised cap changes 1000 → 100.
|
@daveomri I think we can merge this irrespective of other PRs, right? |
Summary
Umbrella PR for the
langchain-apifymodernisation. Four feature sub-branches were squash-merged into this branch (each independently reviewed, with per-tool behaviour validated in the original sub-PRs by the QA tester), plus umbrella-level cleanup, a few bug fixes caught during review, schema-level clamp documentation, and refreshed user-facing docs.What lands
Four squash-merged sub-branches:
ApifyRunActorTool,ApifyGetDatasetItemsTool,ApifyRunActorAndGetDatasetTool,ApifyScrapeUrlTool,ApifyRunTaskTool,ApifyRunTaskAndGetDatasetTool);APIFY_TOKENenv-var rename withAPIFY_API_TOKENkept as a deprecated alias;apify_tokenkwarg.ApifySearchRetriever(BaseRetrieveroverapify/rag-web-browser) andApifyCrawlLoader(BaseLoaderoverapify/website-content-crawler).ApifyGoogleSearchTool,ApifyWebCrawlerTool,ApifyRAGWebBrowserTool,ApifyGoogleMapsTool,ApifyYouTubeScraperTool,ApifyEcommerceScraperTool).Plus README improvements (#27) and convenience lists
APIFY_CORE_TOOLS/APIFY_SEARCH_TOOLS/APIFY_SOCIAL_TOOLS.Umbrella-level changes on top of the merges
_actor_tools.py+tools.pyinto atools/package (base/core/actors/search/social); public API unchanged._constants.py, sharedLiteralaliases in_types.py, centralised error strings in_error_messages.py.max_timeout_secs=600,max_memory_mbytes=32768,max_items=1000,max_crawl_depth=5) from hardcodedFielddefaults on_ApifyGenericToolinto named constants in_constants.py(_MAX_TIMEOUT_SECS_CAP, etc.).(clamped to N max), interpolated from the same constant the base class uses. Addresses the tester's "Fetch 999999 items" feedback: the LLM now sees the cap in the schema and won't promise above-cap results in its narration.tests/unit_tests/test_clamp_descriptions.py(30 parametrised tests) pins the description text and asserts no drift between the advertised cap and the live_ApifyGenericTooldefault.{run: {...}, items: [...]}envelope shape (the old asserts wouldKeyErroragainst the live API).ApifyWrapper.(a)call_actor(_task)forwardsapify_tokento theApifyDatasetLoaderit constructs; an explicit wrapper token no longer requiresAPIFY_TOKENto also be in the environment._prune_actor_input_schemapreserves Actor-schema entries withdefault: 0,default: false, ordefault: ""(22 such fields inapify/website-content-crawleralone were being silently dropped).ApifyActorsTool.__init__now fetches the Actor build once and reuses it across_create_descriptionand_build_tool_args_schema_model(was 2 fetches per init).ApifyDatasetLoader.apify_clientis nowPrivateAttrinstead ofField(default=None) # type: ignore, matching the pattern used by every other internal-client field in the package.Docs
.invoke()example per family.create_react_agent.ApifySearchRetrieverandApifyCrawlLoader.ApifyActorsTool/ApifyDatasetLoader/ApifyWrapper/ "Note for Apify Actor developers" sections preserved.DEVELOPMENT.md: light edits; canonicalisedAPIFY_TOKENreferences with a footer note about theAPIFY_API_TOKENdeprecation alias; added a pointer to the newCONTRIBUTING.md.CONTRIBUTING.md(new): issue / PR scope conventions, commit message prefixes that drivegit-cliffversion bumps, review expectations, and the auto-release workflow.Test plan
make test: 352 unit tests pass (322 from sub-PRs + 30 new clamp-description tests).make lint: clean (35 source files, including the new test module).dist/.playground.pyagainst the real Apify API: offline checks (merge surface, social input mapping, Instagram URL building, retriever source order, Google-search page count, the new clamp-offline block: 20/20 checks pass), plus live core + selected social / search / crawling probes all green. Tester-confirmed in the sub-PRs; the umbrella was re-validated against the live API after each fix.Follow-ups (separate PRs)
ActorClient.default_build()from the Apify SDK in place of the rawrequests.GETin_get_actor_latest_build. Deferred because it requires bumpingapify-client ^2.3.0 → ^2.5.0(the sync method was incorrectlyasync defuntil 2.5.0); the raw call works fine for both public and private Actors today.docs/examples/tools_example.py) to the LangGraph 1.0 API. The connector itself does not depend on LangGraph, so this is docs/examples / dev-deps only.