openai[minor],core[minor]: Add support for passing strict in openai tools #13
Conversation
![local-dev-korbit-ai-mentor[bot]](https://avatars.githubusercontent.com/in/318591?s=60&v=4){kind=small}
![[local-dev] Korbit - AI Mentor](https://avatars.githubusercontent.com/in/318591?s=40&u=4153787aed90096cc5d05c9c38a439007e524086&v=4){kind=avatar}
|
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 introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatOpenAI
participant Tool
User->>ChatOpenAI: Call with strict=true
ChatOpenAI->>Tool: Invoke tool with strict adherence
Tool-->>ChatOpenAI: Return output
ChatOpenAI-->>User: Return structured output
Poem
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 (
|
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
| } | ||
|
|
||
| return tool as ToolDefinition; |
There was a problem hiding this comment.
The convertToOpenAITool function is not returning the modified toolDef object. Instead, it's returning the original tool parameter, which means any modifications made to toolDef (including setting the strict field) are lost. To fix this, replace the last line return tool as ToolDefinition; with return toolDef;.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
| // This will fail since we're not returning a valid response in our mocked fetch function. | ||
| await expect(modelWithTools.invoke("What's the weather like?")).rejects.toThrow(); |
There was a problem hiding this comment.
The test cases are currently expecting the modelWithTools.invoke() call to throw an error, but the mock implementation is returning a successful response. This mismatch could lead to false positives in your tests. Consider either:
- Modifying the mock to return an error response:
mockFetch.mockRejectedValue(new Error('Mock API error'));- Or, if the intention is to test successful responses, update the test expectation:
await expect(modelWithTools.invoke("What's the weather like?")).resolves.not.toThrow();Additionally, add more specific error assertions to ensure you're catching the expected errors. This will make your tests more robust and informative.
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 property has been added, which is great for extending functionality. However, it would be beneficial to add a comment explaining when a user might want to override the default behavior. For example, you could add a note like: 'Set this to true for models that support strict tool calling, or false for those that don't. Only override if you're certain about the model's capabilities.'
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
|
/review |
|
@coderabbitai review |
PR Reviewer Guide 🔍
|
Actions performedReview triggered.
|
|
@coderabbitai review |
Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
libs/langchain-openai/src/types.ts (1)
159-164: Add documentation forsupportsStrictToolCalling.The new
supportsStrictToolCallingproperty should have documentation explaining its default behavior and when it should be overridden. This will help users understand its purpose and usage.+ /** + * Set this to true for models that support strict tool calling, or false for those that don't. + * Only override if you're certain about the model's capabilities. + */ supportsStrictToolCalling?: boolean;
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 (12)
libs/langchain-openai/package.json (1)
40-40: Verify compatibility with the updated OpenAI dependency.The
openaidependency has been updated from^4.49.1to^4.55.0. Ensure that the rest of the codebase is compatible with this new version, as it may introduce breaking changes or new features.libs/langchain-openai/src/tests/chat_models.test.ts (4)
58-59: Ensure mock setup matches test expectations.The existing comment highlights a mismatch in the mock setup. Consider updating the mock to return an error if the test intends to check error handling, or adjust the test to expect a successful response.
105-106: Ensure mock setup matches test expectations.The existing comment highlights a mismatch in the mock setup. Consider updating the mock to return an error if the test intends to check error handling, or adjust the test to expect a successful response.
149-150: Ensure mock setup matches test expectations.The existing comment highlights a mismatch in the mock setup. Consider updating the mock to return an error if the test intends to check error handling, or adjust the test to expect a successful response.
194-195: Ensure mock setup matches test expectations.The existing comment highlights a mismatch in the mock setup. Consider updating the mock to return an error if the test intends to check error handling, or adjust the test to expect a successful response.
libs/langchain-openai/src/tests/chat_models_structured_output.int.test.ts (2)
6-6: Import statement for Jest is appropriate.The import of
testandexpectfrom@jest/globalsis necessary and correctly implemented for the test cases.
Line range hint
8-90:
Test cases for structured output are well-structured and comprehensive.The test cases effectively verify the functionality of structured output using Zod schemas and JSON schemas. They include appropriate assertions to ensure expected keys are present in the results.
langchain-core/src/language_models/base.ts (1)
237-244: Addition ofstrictproperty toFunctionDefinitionis well-implemented.The optional
strictproperty is well-documented and enhances functionality by enforcing strict schema adherence. It integrates seamlessly with the existing interface.libs/langchain-openai/src/chat_models.ts (4)
310-311: LGTM!The introduction of the
ChatOpenAIFieldsinterface consolidates properties and enhances modularity and readability.
302-308: Verify the usage of thestrictproperty.The
strictproperty has been added to theChatOpenAICallOptionsinterface. Ensure that all instances whereChatOpenAICallOptionsis used are updated to handle this new property appropriately.Verification successful
The
strictproperty is correctly integrated and tested.The
strictproperty inChatOpenAICallOptionsis actively used and tested in the codebase, ensuring its correct functionality.
- File:
libs/langchain-openai/src/chat_models.ts- Tests:
libs/langchain-openai/src/tests/chat_models.test.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instances where `ChatOpenAICallOptions` is used to ensure the `strict` property is handled. # Test: Search for `ChatOpenAICallOptions` usage. Expect: Instances where `strict` is utilized. rg --type typescript -A 5 $'ChatOpenAICallOptions'Length of output: 84
Script:
#!/bin/bash # Description: Verify all instances where `ChatOpenAICallOptions` is used to ensure the `strict` property is handled. # Test: Search for `ChatOpenAICallOptions` usage. Expect: Instances where `strict` is utilized. rg --type ts -A 5 $'ChatOpenAICallOptions'Length of output: 9355
Script:
#!/bin/bash # Description: Search for the usage of the `strict` property in the codebase to ensure it is handled properly. # Test: Search for `strict` property usage. Expect: Instances where `strict` is assigned or used. rg --type ts 'strict' -A 5Length of output: 66796
Line range hint
585-639:
Verify the propagation of thestrictargument.The
bindToolsandinvocationParamsmethods have been updated to include thestrictargument. Ensure that this argument is correctly propagated and utilized in all relevant tool interactions.Verification successful
Strict Argument Propagation Verified
The
strictargument is correctly propagated and utilized in thebindToolsandinvocationParamsmethods. The presence of tests inlibs/langchain-openai/src/tests/chat_models.test.tsconfirms that thestrictargument is being handled appropriately, including its default behavior for specific model names.
- File:
libs/langchain-openai/src/chat_models.ts- Test Coverage:
libs/langchain-openai/src/tests/chat_models.test.tsThese findings indicate that the
strictargument is integrated and tested as intended.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the propagation of the `strict` argument in `bindTools` and `invocationParams` methods. # Test: Search for method calls and ensure `strict` is correctly utilized. rg --type typescript -A 5 $'(bindTools|invocationParams)'Length of output: 93
Script:
#!/bin/bash # Description: Verify the propagation of the `strict` argument in `bindTools` and `invocationParams` methods. # Test: Search for method calls and ensure `strict` is correctly utilized. rg 'bindTools|invocationParams' -A 5Length of output: 121694
Script:
#!/bin/bash # Description: Search for the usage of the `strict` argument in the codebase to verify its propagation. # Test: Search for the `strict` keyword in TypeScript files to ensure it is utilized correctly. rg 'strict' --type tsLength of output: 16972
Line range hint
459-562:
Verify the constructor logic forsupportsStrictToolCalling.The constructor has been updated to determine the
supportsStrictToolCallingproperty based on the model name. Ensure that this logic is correct and consistent with the intended behavior for all models.Verification successful
Constructor Logic for
supportsStrictToolCallingis CorrectThe constructor logic for determining
supportsStrictToolCallingbased on the model name is consistent with the intended behavior. It correctly sets this property for models whose names start with "gpt-", such as "gpt-3.5-turbo" and "gpt-4". No issues were found with this implementation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the logic for determining `supportsStrictToolCalling` in the `ChatOpenAI` constructor. # Test: Search for constructor calls and ensure `supportsStrictToolCalling` is correctly set. rg --type typescript -A 10 $'new ChatOpenAI'Length of output: 80
Script:
#!/bin/bash # Description: Verify the logic for determining `supportsStrictToolCalling` in the `ChatOpenAI` constructor. # Test: Search for constructor calls and ensure `supportsStrictToolCalling` is correctly set. rg --type ts -A 10 $'new ChatOpenAI'Length of output: 394577
| if (fields?.strict !== undefined) { | ||
| toolDef.function.strict = fields.strict; | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix the return value to include modifications.
The function is currently returning the original tool parameter instead of the modified toolDef. Ensure that the return statement reflects the intended changes, including the strict parameter.
- return tool as ToolDefinition;
+ return toolDef;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.
| if (fields?.strict !== undefined) { | |
| toolDef.function.strict = fields.strict; | |
| } | |
| if (fields?.strict !== undefined) { | |
| toolDef.function.strict = fields.strict; | |
| } | |
| return toolDef; |
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 default behavior is enabled for models starting with "gpt-". This enhancement improves the reliability and predictability of function calls in OpenAI tools.
Summary by CodeRabbit
New Features
strictoption for enhanced control over function call adherence to schemas.supportsStrictToolCallingproperty to improve configurability of tool interactions.ChatOpenAIclass to support strict adherence to JSON Schema during tool calling.Bug Fixes
openailibrary.Tests
ChatOpenAIclass.