refactor: introduce FunctionCallbackResolver interface #1804
+292
−206
Conversation
This file contains 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
| * @param functionCallbackContext the function callback context used to store the | ||
| * state of the function calls. | ||
| * @param functionCallbackResolver the function callback resolver used to resolve the | ||
| * function by bean name.s |
| @@ -128,28 +128,29 @@ public MoonshotChatModel(MoonshotApi moonshotApi, MoonshotChatOptions options) { | |||
| * @param moonshotApi The Moonshot instance to be used for interacting with the | |||
| * Moonshot Chat API. | |||
| * @param options The MoonshotChatOptions to configure the chat client. | |||
| * @param functionCallbackContext The function callback context. | |||
| * @param functionCallbackResolver The function callback resolver to | |||
There was a problem hiding this comment.
javadoc sentence incomplete.
| @@ -155,27 +155,27 @@ public OpenAiChatModel(OpenAiApi openAiApi, OpenAiChatOptions options) { | |||
| * @param openAiApi The OpenAiApi instance to be used for interacting with the OpenAI | |||
| * Chat API. | |||
| * @param options The OpenAiChatOptions to configure the chat model. | |||
| * @param functionCallbackContext The function callback context. | |||
| * @param functionCallbackResolver The function callback context. | |||
There was a problem hiding this comment.
javadoc still refers to callback context.
| @@ -43,7 +43,7 @@ public class SpringAiCoreRuntimeHints implements RuntimeHintsRegistrar { | |||
| public void registerHints(@NonNull RuntimeHints hints, @Nullable ClassLoader classLoader) { | |||
|
|
|||
| var chatTypes = Set.of(AbstractMessage.class, AssistantMessage.class, ToolResponseMessage.class, Message.class, | |||
| MessageType.class, UserMessage.class, SystemMessage.class, FunctionCallbackContext.class, | |||
| MessageType.class, UserMessage.class, SystemMessage.class, DefaultFunctionCallbackResolver.class, | |||
There was a problem hiding this comment.
How does this work when we have other implementations of "FunctionCallbackResolver"
There was a problem hiding this comment.
If there are other implementation they have to be added here.
| null); | ||
| if (functionCallback != null) { | ||
| this.functionCallbackRegister.put(functionName, functionCallback); | ||
| } | ||
| else { | ||
| throw new IllegalStateException( | ||
| "No function callback [" + functionName + "] fund in tht FunctionCallbackContext"); | ||
| "No function callback [" + functionName + "] fund in tht FunctionCallbackRegister"); |
There was a problem hiding this comment.
typo in the exception message.
| import org.springframework.lang.Nullable; | ||
|
|
||
| /** | ||
| * Strategy interface for resolving {@link FunctionCallback} instances as bean from the |
There was a problem hiding this comment.
I believe there could be an implementation for the FunctionCallbackResolver that doesn't depend on the application context, right?
There was a problem hiding this comment.
I can not think of any not app context case?
- Introduces a new FunctionCallbackResolver interface to define the strategy for
resolving FunctionCallback instances from the application context.
- Renames FunctionCallbackContext to DefaultFunctionCallbackResolver to better reflect its
implementation role. Updates all related components to use the new interface.
- Update the affected AI model implementations
- Replaces FunctionCallbackContext parameter with FunctionCallbackResolver in all
model constructors
- Updates builder patterns to use functionCallbackResolver() method instead of
withFunctionCallbackContext()
- Deprecates old withFunctionCallbackContext() methods in builders to guide
migration
- Updates integration tests to use DefaultFunctionCallbackResolver
- Improves documentation to clarify the resolver's role in function callbacks
- Moves SchemaType enum from FunctionCallbackContext to FunctionCallback (Braking change)git add .
Resolves spring-projects#758
tzolov
force-pushed
the
issue-758-function-callback-resolver
branch
from
33c25ee
to
30af292
Compare
markpollack
reviewed
Nov 25, 2024
| * @param beanName the name of the bean to resolve | ||
| * @return the {@link FunctionCallback} instance | ||
| */ | ||
| FunctionCallback getFunctionCallback(@NonNull String beanName, @Nullable String defaultDescription); |
There was a problem hiding this comment.
what about
FunctionCallback resolve(@NonNull String name)
The description should be provided by the implementation.
| * @param functionCallbackContext the function callback context used to store the | ||
| * state of the function calls. | ||
| * @param functionCallbackResolver the function callback resolver used to resolve the | ||
| * function by bean name |
There was a problem hiding this comment.
Javadoc doesn't need to refer "bean" and just "by name" is enough. will fix when merging.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Resolves #758