Skip to content

fix(AIC-3506): strip tools from LangChain model constructor kwargs - #110

Closed
andrewklatzke wants to merge 1 commit into
mainfrom
aklatzke/AIC-3506/strip-lc-openai-tools-from-params
Closed

andrewklatzke wants to merge 1 commit into
mainfrom
aklatzke/AIC-3506/strip-lc-openai-tools-from-params

Conversation

@andrewklatzke

@andrewklatzke andrewklatzke commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Human explanation:

Strips model.parameter.tools off of Langchain handlers so they don't get splatted into the kwargs. model.parameters.tools is a copy of the existing tools left in for backwards compatibility.

AI explanation:

Tools in model.parameters are forwarded raw and rejected by the provider API. They are already bound from the top-level tools field.


Note

Overview
Stops tools in model.parameters from being passed into LangChain chat model constructors (ChatOpenAI, ChatAnthropic, ChatBedrockConverse), which was causing provider APIs to reject requests. Tooling stays on the existing config.tools path (bindTools, agent createAgent, graph ToolNode).

modelConstructorArgs in both langchain-agents and langchain-messages handlers now always strips parameters.tools (previously only Bedrock). The default ChatOpenAI factory in native-graph applies the same rule when spreading node model.parameters. Docs/comments note that model.parameters.tools may still exist for backwards compatibility but must not be forwarded as constructor kwargs.

Tests assert OpenAI/Bedrock constructors receive temperature and model name only—not a tools key—even when model.parameters.tools is set.

Reviewed by Cursor Bugbot for commit 220b40b. Bugbot is set up for automated code reviews on this repo. Configure here.

Tools in model.parameters are forwarded raw and rejected by the provider API. They are already bound from the top-level tools field.

@jeffdupont jeffdupont left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be closed. js #73 already fixed it on main.

At 993225f, both LangChain handlers and the native graph build their constructor options with modelConstructorParameters (model-parameters.ts:129-145). That keeps only the per-model-class *_FORWARDED_KEYS, and tools is not on any of them. I probed it on main. With { temperature: 0.2, tools: [...] }, tools is dropped for openai, anthropic and bedrock in both langchain-messages and langchain-agents, and temperature comes through. Flipping the assertion fails all six cases, so the probe is testing the real behaviour.

main also has a regression test for this: model-parameters.test.ts:15 in both packages puts tools in the smuggled bag. This PR now conflicts with main in those same files, and the delete parameters.tools lines have nothing left to change.

The Python twin, py #133, is still needed. Python main still forwards tools to ChatOpenAI and ChatAnthropic, and I've approved it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants