openai[minor],core[minor]: Add support for passing strict in openai tools #7
Conversation
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
|
Clone of the PR langchain-ai/langchainjs#6418 |
|
My review is in progress 📖 - I will have feedback for you in a few minutes! |
WalkthroughThe recent changes enhance the LangChain integration with OpenAI services by introducing a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatOpenAI
participant Tool
User->>ChatOpenAI: Create instance with fields including strict
ChatOpenAI->>Tool: Bind tools with strict settings
Tool-->>ChatOpenAI: Confirm tool binding
ChatOpenAI->>User: Return structured output with strict adherence
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
| } | ||
|
|
||
| return tool as ToolDefinition; |
There was a problem hiding this comment.
There's an issue with the return statement in the convertToOpenAITool function. Currently, it's returning the original tool parameter, which hasn't been modified with the strict field. Instead, it should return the toolDef variable, which contains the updated tool definition. Please update the return statement to return toolDef; to ensure the function returns the correctly modified tool definition.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| @@ -3,6 +3,7 @@ import { zodToJsonSchema } from "zod-to-json-schema"; | |||
| import { ChatPromptTemplate } from "@langchain/core/prompts"; | |||
| import { AIMessage } from "@langchain/core/messages"; | |||
| import { ChatOpenAI } from "../chat_models.js"; | |||
| import { test, expect } from "@jest/globals"; | |||
There was a problem hiding this comment.
It appears that you've added an import for 'test' and 'expect' from '@jest/globals'. However, these are typically globally available in Jest test files without explicit imports. To improve code cleanliness and avoid potential conflicts, you can remove this redundant import line.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| /** | ||
| * Whether the model supports the 'strict' argument when passing in tools. | ||
| * Defaults to `true` if `modelName`/`model` starts with 'gpt-' otherwise | ||
| * defaults to `false`. | ||
| */ | ||
| supportsStrictToolCalling?: boolean; |
There was a problem hiding this comment.
The new 'supportsStrictToolCalling' field has been added to the OpenAIChatInput interface, which is a good addition. However, it would be helpful to add a comment explaining how this field is used in the actual implementation. For example, does setting this to true enforce strict adherence to the tool's schema? How does it affect the model's behavior when calling tools? Adding this information would make the code more self-documenting and easier for other developers to understand and use correctly.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
|
/review |
PR Reviewer Guide 🔍(Review updated until commit 005ef36)
|
|
@coderabbitai full review |
Actions performedFull review triggered. |
|
Persistent review updated to latest commit 005ef36 |
PR Code Suggestions ✨
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (2)
libs/langchain-openai/src/types.ts (1)
159-164: Add Documentation forsupportsStrictToolCallingThe
supportsStrictToolCallingproperty is a valuable addition. Consider adding documentation to explain its purpose and impact on model behavior when calling tools.+ /** + * Indicates whether the model supports the 'strict' argument for tool usage. + * This affects how strictly the model adheres to the tool's schema. + */libs/langchain-openai/src/tests/chat_models_structured_output.int.test.ts (1)
6-6: Remove Redundant Import oftestandexpectThe import statement for
testandexpectis redundant as these are globally available in Jest test files. Consider removing it for cleaner code.-import { test, expect } from "@jest/globals";
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (7)
- langchain-core/src/language_models/base.ts (1 hunks)
- langchain-core/src/utils/function_calling.ts (1 hunks)
- libs/langchain-openai/package.json (1 hunks)
- libs/langchain-openai/src/chat_models.ts (7 hunks)
- libs/langchain-openai/src/tests/chat_models.test.ts (1 hunks)
- libs/langchain-openai/src/tests/chat_models_structured_output.int.test.ts (1 hunks)
- libs/langchain-openai/src/types.ts (1 hunks)
Additional context used
Biome
libs/langchain-openai/src/tests/chat_models.test.ts
[error] 31-31: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 64-64: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 110-110: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 154-154: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 199-199: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (8)
libs/langchain-openai/package.json (1)
40-40: Dependency Update: Verify Compatibility with OpenAI v4.55.0The
openaipackage has been updated to version^4.55.0. Ensure that the codebase is compatible with this version and take advantage of any new features or improvements.Run the following script to verify compatibility with the new version:
langchain-core/src/utils/function_calling.ts (1)
37-44: LGTM! Addition ofstrictParameterThe addition of the
strictparameter to theconvertToOpenAIToolfunction enhances flexibility and control over tool output formats.langchain-core/src/language_models/base.ts (1)
236-244: Well-documented addition of thestrictproperty.The
strictproperty is a valuable addition to theFunctionDefinitioninterface, providing enhanced control over schema adherence. The accompanying documentation is clear and informative.libs/langchain-openai/src/chat_models.ts (5)
302-308: Well-documented addition of thestrictproperty.The
strictproperty in theChatOpenAICallOptionsinterface is a valuable addition, providing enhanced control over model responses. The documentation is clear and informative.
310-311: Introduction ofChatOpenAIFieldsinterface enhances clarity.The
ChatOpenAIFieldsinterface consolidates configuration options, improving clarity and maintainability. This change is well-integrated into the constructor.
454-459: Well-documented addition ofsupportsStrictToolCallingproperty.The
supportsStrictToolCallingproperty enhances the flexibility of theChatOpenAIclass by indicating support for thestrictargument. The documentation is clear and informative.
Line range hint
460-562: Constructor modifications streamline configuration.The constructor modifications in the
ChatOpenAIclass streamline the configuration process and ensure proper initialization of thesupportsStrictToolCallingproperty. These changes are well-implemented.
Line range hint
585-639: Consistent integration of thestrictparameter in methods.The integration of the
strictparameter in methods likebindToolsandinvocationParamsensures consistent application, enhancing control over tool interactions.
| if (fields?.strict !== undefined) { | ||
| toolDef.function.strict = fields.strict; | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix the Return Statement in convertToOpenAITool
The return statement currently returns the original tool parameter. It should return the toolDef variable, which contains the updated tool definition with the strict field.
Fix the return statement as follows:
- return tool as ToolDefinition;
+ return toolDef;Committable suggestion was skipped due to low confidence.
| expect(mockFetch).toHaveBeenCalled(); | ||
| const [_url, options] = mockFetch.mock.calls[0]; | ||
|
|
||
| if (options && options.body) { |
There was a problem hiding this comment.
Use Optional Chaining for Safety
Consider using optional chaining to safely access properties and avoid potential runtime errors.
- if (options && options.body) {
+ if (options?.body) {Also applies to: 110-110, 154-154, 199-199
Tools
Biome
[error] 64-64: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| process.env.LANGCHAIN_TRACING_V2 = oldLangChainTracingValue; | ||
| } else { | ||
| // If it was undefined, remove the environment variable | ||
| delete process.env.LANGCHAIN_TRACING_V2; |
There was a problem hiding this comment.
Avoid Using the delete Operator
Using the delete operator can impact performance. Consider setting the variable to undefined instead.
- delete process.env.LANGCHAIN_TRACING_V2;
+ process.env.LANGCHAIN_TRACING_V2 = undefined;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| delete process.env.LANGCHAIN_TRACING_V2; | |
| process.env.LANGCHAIN_TRACING_V2 = undefined; |
Tools
Biome
[error] 31-31: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
User description
Will require bumping the min core verison on openai
Description by Korbit AI
Note
This feature is in early access. You can enable or disable it in the Korbit Console.
What change is being made?
Add support for passing a
strictparameter in OpenAI tools to enforce strict schema adherence when generating function calls.Why are these changes being made?
This change allows users to ensure that the model output strictly follows the defined JSON Schema, which is particularly useful for structured outputs. The
strictparameter is enabled by default for models starting with "gpt-". This enhancement improves the reliability and predictability of function calls in OpenAI tools.PR Type
enhancement, tests
Description
strictparameter in OpenAI tools to enforce strict schema adherence.ChatOpenAIto handle thestrictparameter, with default behavior based on model name.convertToOpenAIToolfunction to includestrictparameter in tool definitions.strictparameter.openaidependency version inpackage.json.Changes walkthrough 📝
base.ts
Add `strict` parameter to FunctionDefinition interfacelangchain-core/src/language_models/base.ts
strictparameter toFunctionDefinitioninterface.strictparameter.function_calling.ts
Update convertToOpenAITool to handle strict parameterlangchain-core/src/utils/function_calling.ts
convertToOpenAIToolfunction to acceptstrictparameter.strictin tool definition.chat_models.ts
Implement strict parameter handling in ChatOpenAIlibs/langchain-openai/src/chat_models.ts
strictparameter toChatOpenAICallOptions.strictvalue based on modelname.
strictparameter when binding tools.types.ts
Add supportsStrictToolCalling to OpenAIChatInputlibs/langchain-openai/src/types.ts
supportsStrictToolCallingtoOpenAIChatInput.chat_models.test.ts
Add tests for strict parameter in ChatOpenAIlibs/langchain-openai/src/tests/chat_models.test.ts
strictparameter in tool binding.chat_models_structured_output.int.test.ts
Update imports in structured output integration testlibs/langchain-openai/src/tests/chat_models_structured_output.int.test.ts
testandexpectfrom@jest/globals.package.json
Update openai dependency versionlibs/langchain-openai/package.json
openaidependency version to^4.55.0.Summary by CodeRabbit
New Features
strictoption to enhance control over function call generation and schema adherence.ChatOpenAIclass.Bug Fixes
Tests
ChatOpenAIclass, ensuring correct handling of thestrictparameter across various configurations.