Skip to content

Conversation

mxsl-gr
Copy link
Contributor

@mxsl-gr mxsl-gr commented Jul 25, 2024

PR content:

  1. Moonshot supported function call
  2. function call unit tests

related issue: #1058

@tzolov
Copy link
Contributor

tzolov commented Jul 31, 2024

HI @mxsl-gr and thank you for the update.
As we are deprecating AbstractFunctionCallSupport in favor of AbstractToolCallSupport would you be interested to re-work your PR to support AbstractToolCallSupport?

For examples, you can check the OpenAI/AzureOpenAI/Anthropic/Mistral/Ollama/VertexAIGemini function calling implementations, or ask me for support ;)
Would appreciate your help as you own those models.

@mxsl-gr mxsl-gr force-pushed the feature/moonshot-function-call branch from d5302ae to fab3951 Compare August 5, 2024 01:05
@mxsl-gr
Copy link
Contributor Author

mxsl-gr commented Aug 5, 2024

hi, @tzolov
it worked for now, and this PR has been squash and force pushed

@markpollack
Copy link
Member

@mxsl-gr thanks, sorry I haven't gotten to merging this yet

@mxsl-gr
Copy link
Contributor Author

mxsl-gr commented Aug 15, 2024

@mxsl-gr thanks, sorry I haven't gotten to merging this yet

hi @markpollack , is ok, i went on a trip and just back now. please let me know if there's anything else to adjust.

other some PRs related to function calls: #1116, #1163

@@ -58,11 +59,12 @@ public class FunctionCallbackWrapperIT {

@Test
void functionCallTest() {
Copy link
Member

Choose a reason for hiding this comment

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

This test fails for me. It looks like it wants to make the calls sequentially or something

java.lang.AssertionError: 
Expecting actual:
  "The current weather in San Francisco is 30.0°C. Now, I will check the weather in Tokyo."
to contain:
  ["30", "10", "15"]
but could not find:
  ["10", "15"]
 

@markpollack
Copy link
Member

I'm merging this despite test failure as this PR has improved the support so significantly. We can circle back to figure out what is going on with the function calling failure in the test. The streaming function calling test did pass in my testing.

@markpollack markpollack self-assigned this Aug 23, 2024
@markpollack markpollack added this to the 1.0.0-M2 milestone Aug 23, 2024
@markpollack
Copy link
Member

merged in 611c949

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.

3 participants