fix(tools): align serper, firecrawl, langsmith and qdrant declared shapes with the APIs - #7267
fix(tools): align serper, firecrawl, langsmith and qdrant declared shapes with the APIs#7267waleedlatif1 wants to merge 4 commits into
Conversation
…apes with the APIs Serper places mapped `link`, `snippet` and `reviews`, none of which /places returns, and never emitted the declared `ratingCount`. News dropped the `source` it always returns. /search's peopleAlsoAsk and relatedSearches were undeclared. Firecrawl search declared `data` as an array; /v2/search answers with a source-keyed envelope. /v2/map returns link objects, not URL strings, and its `timeout` param was being consumed as the outbound fetch deadline. LangSmith feedback score coercion lived only in the block, so the LLM and direct-tool paths posted uncoerced values. Qdrant's block declared `matches` and `upsertedCount`, which no tool emits.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR aligns several integration declarations and transformations with their provider response and request contracts.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/tools/firecrawl/map.ts | Renames the provider map deadline, normalizes map links into declared objects, and preserves valid edge-case timeout values. |
| apps/sim/tools/firecrawl/search.ts | Exposes source selection and transforms Firecrawl v2 search responses into the source-keyed output envelope. |
| apps/sim/tools/firecrawl/types.ts | Defines source-specific Firecrawl result shapes, nullable scraped fields, metadata, and object-based map links. |
| apps/sim/tools/serper/search.ts | Corrects place and news mappings and conditionally surfaces the web-search question and related-query panels. |
| apps/sim/tools/serper/types.ts | Aligns unified Serper result and panel declarations with the mapped provider payloads. |
| apps/sim/tools/langsmith/create_feedback.ts | Applies finite numeric score coercion consistently at the tool request boundary. |
| apps/sim/blocks/blocks/qdrant.ts | Removes block outputs that no Qdrant tool emits and documents the actual operation-specific payload. |
| apps/sim/tools/firecrawl/search.test.ts | Covers Firecrawl v2 envelopes, source forwarding, map timeout behavior, link normalization, and declared output details. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Inputs[Block or direct tool inputs] --> Normalize[Tool request normalization]
Normalize --> Providers[Firecrawl / Serper / LangSmith / Qdrant]
Providers --> Transform[Provider-specific response transformation]
Transform --> Shapes[Declared tool output shapes]
Shapes --> Metadata[Generated metadata and integration docs]
Reviews (2): Last reviewed commit: "fix(tools): reject non-numeric langsmith..." | Re-trigger Greptile
There was a problem hiding this comment.
7 issues found across 18 files
Confidence score: 3/5
apps/docs/content/docs/integrations/firecrawl.mdxdocuments Search requests fornewsandimages, but the tool schema and Firecrawl block do not exposesources, so those documented requests cannot work; exposesourcesbefore relying on the documentation.apps/sim/tools/langsmith/create_feedback.tscan serialize infinite scores asnulland converts whitespace-only scores to0, producing incorrect LangSmith feedback; reject non-finite values and treat trimmed-empty input as undefined.apps/sim/tools/firecrawl/types.tsdoes not fully match Firecrawl’s nullable or missing response fields, so valid search results can fail validation or misrepresent optional content; make metadata and nullable scraped fields optional in the advertised schema.apps/sim/tools/firecrawl/map.tsdrops an explicitmapTimeoutof0, andapps/sim/blocks/blocks/qdrant.tsleaves the registered skill describing an upsert count that the output no longer exposes; preserve zero-valued timeout configuration and align the skill with the returned data.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/tools/firecrawl/map.ts">
<violation number="1" location="apps/sim/tools/firecrawl/map.ts:93">
P2: When `mapTimeout` is explicitly `0`, this guard omits the Firecrawl body timeout and silently applies the provider default. Preserve zero while still omitting empty values, for example by checking `params.mapTimeout || params.mapTimeout === 0`.
(Based on your team's feedback about preserving explicit zero numeric parameters.) .</violation>
</file>
<file name="apps/sim/blocks/blocks/qdrant.ts">
<violation number="1" location="apps/sim/blocks/blocks/qdrant.ts:263">
P2: After this change, the upsert output exposes only `data` containing Qdrant’s operation result, but the registered `upsert-points` skill still tells agents to report an upserted count. Update that skill to describe and report the operation ID and status instead.</violation>
</file>
<file name="apps/docs/content/docs/integrations/firecrawl.mdx">
<violation number="1" location="apps/docs/content/docs/integrations/firecrawl.mdx:166">
P2: When users use the documented Search action, `news` and `images` cannot be requested because neither the tool schema nor the Firecrawl block exposes `sources`. Expose `sources` in the tool and block before documenting these verticals, or remove the unreachable result arrays here.</violation>
</file>
<file name="apps/sim/tools/langsmith/create_feedback.ts">
<violation number="1" location="apps/sim/tools/langsmith/create_feedback.ts:16">
P2: When the optional score contains only whitespace, `Number(value)` converts it to `0`, so the tool records a score instead of omitting the blank input. Treat trimmed-empty strings as undefined before coercion.</violation>
<violation number="2" location="apps/sim/tools/langsmith/create_feedback.ts:18">
P2: When a caller supplies `Infinity` or `-Infinity` as the score, `Number.isNaN(parsed)` allows it through and JSON serialization sends `null` to LangSmith. Reject non-finite parsed scores with `Number.isFinite`.</violation>
</file>
<file name="apps/sim/tools/firecrawl/types.ts">
<violation number="1" location="apps/sim/tools/firecrawl/types.ts:214">
P2: When Firecrawl returns a search item without scraped metadata, the output declaration still requires `metadata`. Mark metadata optional in both web and news item definitions, because `transformResponse` forwards items unchanged.</violation>
<violation number="2" location="apps/sim/tools/firecrawl/types.ts:458">
P2: When Firecrawl returns `null` for optional scraped content, the new TypeScript shape allows it but the advertised output schema says only `string`. Mark `markdown`, `html`, `rawHtml`, `screenshot`, and metadata `error` nullable, or normalize nulls before returning.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
…e search sources - langsmith: reject non-finite scores and treat a whitespace-only score as omitted (both bugs were inherited verbatim from the block). - firecrawl: mark scraped content nullable and search metadata optional per the v2 schema and the official SDK's unscraped result types. - firecrawl map: preserve an explicit mapTimeout of 0, and widen bare string links to the declared object shape the way the SDK does. - firecrawl search: expose `sources` on the tool and block so the documented news and images verticals are actually reachable. - qdrant: the upsert-points skill no longer tells agents to report an upserted count Qdrant has never returned.
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
4 issues found across 18 files
Confidence score: 3/5
apps/docs/content/docs/integrations/firecrawl.mdx:mapTimeoutis documented but cannot be configured or forwarded by themapoperation, so user timeout settings have no effect; expose and pass the setting through the integration block.apps/sim/tools/firecrawl/types.ts: Search web and news results can containaudioandvideo, but the declared interfaces and output properties omit them, potentially dropping fields for consumers; add nullable fields to the shared interface and both output property sets.apps/sim/tools/firecrawl/types.ts: Crawled pages may return a null screenshot while the response interfaces reject null, creating a type mismatch for valid crawl responses; usestring | nullthroughout the shared screenshot shape.apps/sim/tools/firecrawl/types.ts:metadata.urlcontains the final redirected URL but is absent fromSEARCH_METADATA_OUTPUT_PROPERTIES, so generated metadata consumers cannot access it; add theurlproperty.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/tools/firecrawl/types.ts">
<violation number="1" location="apps/sim/tools/firecrawl/types.ts:95">
P2: The crawl output now allows null screenshots, but the corresponding response interfaces reject null. Update the crawled-page screenshot types to `string | null` wherever this shared shape is used.</violation>
<violation number="2" location="apps/sim/tools/firecrawl/types.ts:240">
P2: Search scraping can return `audio` and `video`, but the new web and news result shapes do not declare them. Add nullable `audio` and `video` fields to the shared interface and both output property sets.</violation>
<violation number="3" location="apps/sim/tools/firecrawl/types.ts:482">
P2: Search results preserve `metadata.url`, but the declared output omits it, hiding the final redirected URL from generated tool metadata and consumers. Add `url` to `SEARCH_METADATA_OUTPUT_PROPERTIES`.</violation>
</file>
<file name="apps/docs/content/docs/integrations/firecrawl.mdx">
<violation number="1" location="apps/docs/content/docs/integrations/firecrawl.mdx:314">
P2: When users configure the Firecrawl integration block, `mapTimeout` has no input path and is never forwarded by the block's `map` operation, so this documented setting cannot affect the request. Expose `mapTimeout` through the block mapping or remove it from this integration documentation.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…crawl edit, expose mapTimeout - audio and video are reachable through the hidden scrapeOptions passthrough, so declare them nullable on web and news items rather than assuming the tool never requests those formats. - metadata.url (the final URL after redirects) was added to the TS interface but never to the declared output. - Revert nullable on the crawled-page screenshot: a regex in the previous commit leaked into crawl properties, which this PR never verified. - Expose mapTimeout on the block so the documented setting reaches the request.
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 18 files
Confidence score: 4/5
- In
apps/sim/tools/langsmith/create_feedback.ts, malformed boolean input such asfalsecan be coerced into score0, polluting evaluation data; restrict coercion to number and string values before callingNumber. - In
apps/sim/tools/firecrawl/types.ts, categorized web or news results cannot expose the provider’scategoryfield through generated metadata and exported interfaces; add an optionalcategoryproperty to the relevant declarations.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/tools/langsmith/create_feedback.ts">
<violation number="1" location="apps/sim/tools/langsmith/create_feedback.ts:18">
P2: Malformed direct or LLM input such as `false` is silently recorded as score `0` instead of rejected, producing incorrect evaluation data. Restrict coercion to `number` and `string` values before calling `Number`.</violation>
</file>
<file name="apps/sim/tools/firecrawl/types.ts">
<violation number="1" location="apps/sim/tools/firecrawl/types.ts:163">
P3: When Firecrawl returns a categorized web or news result, these declarations omit `category`, so generated tool metadata and exported result interfaces cannot describe that provider field. Add optional `category` to both result property maps and both TypeScript result interfaces.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
… result category - langsmith: restrict score coercion to number and string, so `false`/`[]` are rejected instead of silently recorded as 0. - firecrawl: declare the optional `category` the official SDK types on web and news search results.
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
|
@greptile review |
|
Closing for now — not because of a defect. This batch grew to 17 PRs across ~700 changed call sites, and we would rather revisit it as smaller, independently testable pieces than merge this much at once. Nothing here is lost: the branch |
Declared tool outputs that do not match what the provider actually returns. Every shape change below is backed by a primary doc, cited inline. Each fix has a test that was verified to fail against the old code first.
1. Serper
placesmapped fields/placesdoes not returnplaces.toResultemittedlink,snippetandreviews. A/placesitem has none of those keys, so all three resolved toundefinedon every result, while the fields Serper does return (ratingCount,category,phoneNumber,website,latitude,longitude,cid) were dropped.ratingCountwas a declared output that no code path could ever populate — the review count arrives under that name, not asreviews.Documented
/placesitem (serper.dev, via the published response examples):{ "position": 1, "title": "Whole Foods Market", "address": "2001 Market St, San Francisco, CA 94114", "latitude": 37.7687616, "longitude": -122.42701559999999, "rating": 4.2, "ratingCount": 1500, "category": "Grocery store", "phoneNumber": "(415) 626-1430", "website": "https://…", "cid": "6353588238324409422" }linkandsnippetstay declared but are nowoptional— they are real on every other vertical, so they are documented as absent on places rather than removed.reviewswas never a declared output (only a TS field commented "not an advertised output"), so dropping it removes no advertised surface.Also:
newsitems always carrysource(declared on the unified result as "Source name (news/videos/shopping)") and the mapper dropped it. Now mapped.Docs: https://serper.dev (places + news response examples). Cross-checked against the
Newstype in the community client at https://github.com/tkdkid1000/serper/blob/main/docs/types/News.html.2. Serper
/searchpanels were undeclaredpeopleAlsoAskandrelatedSearchesare top-level keys on the/searchpayload and were never surfaced. Both are now declared and populated, and stay absent (not empty arrays) on verticals that do not return them.Docs: https://serper.dev (search response example showing both blocks verbatim).
knowledgeGraphis not included — see "Unverifiable" below.3. Firecrawl search declared the wrong container
SearchResponse.output.datawasArray<…>.POST /v2/searchanswers with a source-keyed envelope:data.web,data.news,data.images, present only for the requested sources. News items carrysnippet(notdescription); image items put the containing page inurland the image inimageUrl. Declared outputs now model all three item shapes, and an absent body resolves to{}rather thanundefined.Docs: https://docs.firecrawl.dev/api-reference/v2-openapi.json (
paths./search.post.responses.200). Cross-checked againstSearchData/SearchResultWeb/SearchResultNews/SearchResultImagesin the official JS SDK: https://github.com/firecrawl/firecrawl/blob/main/apps/js-sdk/firecrawl/src/v2/types.ts.Note the drift's origin: the repo's
apps/api/openapi.jsonandv1-openapi.jsonboth still describe v1, wheredatagenuinely was an array. The tool calls/v2.4. Firecrawl
map—timeoutparam renamed, andlinksare objectsrequest-transport.ts:191readsparams.timeoutas the outbound fetch deadline for every tool.firecrawl_mapdeclared a param of that name intended for Firecrawl's own map deadline, so setting it also armed the local abort at the same instant — the client abort wins the race and the caller gets an opaqueAbortErrorinstead of Firecrawl's response. Renamed tomapTimeoutand mapped to the body'stimeout.No subBlock id changed. The block's
timeoutsubBlock is conditioned on['scrape', 'search', 'parse']and themapbranch oftools.config.paramsnever forwarded it, so no saved workflow state referenced this param — the rename is confined to the tool's direct-call surface. Nothing needed adding totools.config.params.firecrawl/parse.ts'stimeoutis left alone as instructed, and is unchanged in this PR.Separately,
/v2/mapreturnslinksasSearchResultWeb[]— objects with a requiredurlplus optionaltitle/description— while the tool declareditems: { type: 'string' }. Corrected.Docs: https://docs.firecrawl.dev/api-reference/v2-openapi.json (
components.schemas.MapResponse, andpaths./map.post.requestBodyfortimeout: "Timeout in milliseconds. There is no timeout by default."). Cross-checked againstMapData.links: SearchResultWeb[]in the official JS SDK.5. LangSmith feedback score coercion only ran in the block
blocks/blocks/langsmith.tsdid theNumber(value)+ NaN check insidetools.config.params, so only the block path got it — LLM tool calls and direct tool calls posted the raw value. Moved intocreate_feedback.ts'sbody, so every caller is covered; the block now passes the value straight through. Behavior is preserved exactly, including theInvalid score: "…" is not a numbermessage and treating''/null/undefinedas omitted.6. Qdrant block declared outputs no tool emits
QdrantBlock.outputsdeclaredmatchesandupsertedCount. All three Qdrant tools return exactly{ status, data }— no code path has ever produced either key, so every reference resolved toundefined. Both removed;data's description now says what it holds per operation.This one needed no API doc — it is provable from the repo. For completeness the point shapes were checked against Qdrant's OpenAPI (
Record,ScoredPoint,UpdateResultat https://github.com/qdrant/qdrant/blob/master/docs/redoc/master/openapi.json) and every declared per-point field (version,score,shard_key,order_value,operation_id) is real — no phantom fields found in the qdrant tool layer.Fields removed, and the evidence
reviewsSearchResult/placesitems report the review count asratingCount.reviewsappears in no serper.dev example. Was not a declared output.matchesQdrantBlock.outputstransformResponsewrites it; all three return{ status, data }.upsertedCountQdrantBlock.outputsUpdateResulthas onlyoperation_idandstatus.linkandsnippeton places results are narrowed to optional, not removed — they remain populated on every other vertical.Unverifiable
knowledgeGraph.SearchResponsehas carried a deadknowledgeGraph?TS field and aKNOWLEDGE_GRAPH_OUTPUT_PROPERTIESconstant, neither declared as an output nor populated. I could not retrieve a primary serper.dev example of the object, so I did not add it. To finish it I need a captured/searchpayload for an entity query (e.g.q: "OpenAI") or a live Serper key. Left exactly as-is.answerBoxandtopStories: dead TS-only fields, no advertised output, untouched.Tests
Every assertion was run against the unmodified source first and confirmed red (13 failures), then green after the fix, then the source changes were stashed and the same 13 confirmed red again.
tools/serper/search.test.ts— places payload copied from serper.dev; asserts nolink/snippet/reviews; newssource; the two/searchpanels present onsearchand absent onnews.tools/firecrawl/search.test.ts(new) — source-keyed envelope, newssnippetnotdescription, imageurlvsimageUrl, empty-envelope fallback, declared-output shape,mapTimeoutrename and body mapping, map link objects.tools/langsmith/langsmith.test.ts— string coercion,0preserved, blank omitted, non-numeric rejected, all at the tool layer.tools/qdrant/search_vector.test.ts— block declares onlydataandstatus; no tool emitsmatches/upsertedCount.Gates
tool-metadata:generate,generate-docs.ts(generated diff committed),lint,check:audits(39/39, includesdocs:check),check-block-registry.tsall pass. Type-check is clean for these files; the two@aws-sdk/client-lambdaerrors are pre-existing and unrelated.