fix(core): drop auto-important heuristic and surface toolkitVersions per-call#3376
Open
jkomyno wants to merge 4 commits into
Open
fix(core): drop auto-important heuristic and surface toolkitVersions per-call#3376jkomyno wants to merge 4 commits into
jkomyno wants to merge 4 commits into
Conversation
…per-call
Resolves PLEN-2394.
`getRawComposioTools({ toolkits: [...] })` was silently appending
`important: 'true'` whenever `limit` was omitted, so the server returned
only the curated subset (17 of 23 tools for airtable). Callers now get
the full toolkit unless they explicitly ask for `important: true`.
Also adds `toolkitVersions` to the public ToolListParams (string |
Record<slug, version>), matching the existing constructor option but per
call. The schema is updated alongside the type so safeParse no longer
strips the field silently.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
The Please review and fix the vulnerabilities. You can try running: pnpm audit --fix --prodAudit output |
…eedback Address review on PR #3376: - Move the auto-`important: true` heuristic into `tools.get` (LLM-bound path) via a private `applyImportantHeuristic` helper. `getRawComposioTools` stays raw — full toolkit by default. This protects providers (OpenAI, Anthropic, ...) from runaway tool counts while keeping the raw escape hatch honest. - Drop the dead `!== undefined` guard on `toolkit_versions`; restore an unconditional spread that matches the surrounding truthy-spread pattern in this file. `this.toolkitVersions` is always initialized to `'latest'`, so the guard never fired. - Inline `effectiveImportant` — the variable was just `queryParams.data.important` after parsing. One less name to track. - JSDoc on `toolkitVersions` and `important` schema fields and on `BaseParams`, documenting precedence and the `tools.get` vs raw split. - Add two examples to `getRawComposioTools` JSDoc: how to recover the curated subset (`important: true`) and how to pin a specific toolkit version per call. - Rename two schema tests from `it('accepts ...')` to `it('should accept ...')` to match the file convention; move them out of the `Tools` model describe into a top-level `ToolListParamsSchema` describe. - Add three `tools.get` tests covering the heuristic: auto-applies for toolkits-only, respects explicit `important: false`, skipped when `limit` is provided.
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
1 participant
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 PLEN-2394.
The Linear ticket framed this as two
@composio/client(Stainless) bugs:limitsilently dropped,toolkit_versionsmissing fromToolListParams. The framing was reasonable given the symptom —composio.tools.list({ toolkit_slug: "airtable" })returned 17 tools where Python'sget_raw_composio_tools(toolkits=["airtable"], limit=100)returned 23. However, neither bug is real. The published@composio/client@0.1.0-alpha.70typeslimit?: number | nullandtoolkit_versions?: string | { [key: string]: string }, and the runtime serializes both correctly viaqs.stringifywitharrayFormat: 'comma'. I verified this directly against the npm tarball.The actual bug is in
@composio/core.getRawComposioToolscarried a heuristic that silently appendedimportant: 'true'to the request whenevertoolkitswas set withoutlimit. The server then filtered server-side to the curated "important" subset. For airtable, that subset is exactly 17 tools — the same number the reporter saw. The remaining six are the non-important tools the heuristic was hiding.Fix
I dropped the heuristic. Callers who want the curated subset can pass
important: trueexplicitly; everyone else now gets the full toolkit. This is the change the bug calls for, and the change Python parity demands.A second, smaller change rides along:
toolkitVersionsis now accepted on individualgetRawComposioToolscalls ('latest'orRecord<slug, version>), overriding the SDK-init default for that one call. Previously the only way to pin a toolkit version was at SDK init, which forced re-instantiatingComposiofor any per-call override. The Zod schema is updated alongside the type; otherwisesafeParsewould silently strip the new field, and the per-call branch would be permanently dead code.What changed
ts/packages/core/src/models/Tools.ts— removedshouldAutoApplyImportantand the auto-defaulting derived from it. AddedeffectiveToolkitVersionsresolution that prefers the per-call value over the constructor default.ts/packages/core/src/types/tool.types.ts— addedtoolkitVersionstoToolListParamsSchema(mirroring the existingToolkitVersionParam) and Picked it into every variant of the publicToolListParamsunion.ts/packages/core/test/tools/tools.test.ts— updated 7 tests that assertedimportant: 'true'was sent for toolkit-only calls; added 5 new tests covering schema acceptance for both forms, per-call override (table-driven for string + object form), fallback to SDK-init default, and a state-leak guard across calls.Behavior change & semver
Marked minor, not patch. Removing the heuristic changes observable result counts for any caller that relied on the implicit filtering, and I'd rather be honest about that in the bump than smuggle a behavior change into a patch release. Migration is one line: add
important: trueto the query if you want the old behavior. The changeset frames this as a "Behavior change" with migration guidance inline.Test plan
pnpm testin@composio/core— 902 tests pass.pnpm tsc --noEmitin@composio/core— clean.composio.tools.getRawComposioTools({ toolkits: ['airtable'] })returns the full toolkit, matching Python parity.Out of scope
tools.get(provider-tool fetch) flow usesgetRawComposioToolsand benefits from the same change. If any provider relied on the curated subset, that's a follow-up — I don't think it did, but I'd rather find out from a failing integration test than guess.