-
-
Notifications
You must be signed in to change notification settings - Fork 332
feat: Adds tools for mistral #2278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for the PR. There's a couple of things this needs before I can merge:
I don't recall tool use ever being "disabled", we just never had anyone to take the time to add support for them so appreciate your help here. |
|
Sounds good @olimorris ! I’ll have a look at this, bear with me this is my first contribution to any lua/vim plugins 😅 |
|
@flovilmart I'm here to help. Fire any questions my way. I just really appreciate your taking the time to contribute. Btw, the CONTRIBUTING.md file has a section about using memory. It's a much faster way of getting an LLM up to speed on how things work in CodeCompanion. |
|
@olimorris thank you! There are some shenanigan behaviours on failed tool calls that don't always get back to the adapter and mistral really don't like not having an assistant response between tool and user calls, but I'll dig later. My current question is more around to create the stubs for the HTTP calls - is there a way to make the spec generate the stubs with the currently implemented adapter? Or to add the |
|
You can add the weather adapter to your config to see the output. Add this to the tools section in the CodeCompanion repo config your working on: ["weather"] = {
callback = vim.fn.getcwd() .. "/tests/strategies/chat/tools/catalog/stubs/weather.lua",
description = "Get the latest weather",
},Also, make sure you have logs set to You can then do "What's the @{weather} like in Paris?" and you should get a response. I expect the handling of the streamed response is where the difference to OpenAI may lie. |
|
Thank you! I removed
|
|
Killed it, @flovilmart 👏🏼. Great job. That works brilliantly in my testing. I've updated the docs to show Mistral compatibility. Regarding your comment above, if we don't need that functionality, I suggest we remove it. |
Let me remove then! |
|
@olimorris I have another question, mistral require all tool errors being forwarded to it, otherwise it may cause this: I've countered locally by setting: But it would be better if that would be on by default using the adapter. |
|
That's strange because by default, CodeCompanion adds all error messages to the message stack. Even if there's an error in how the tool handles errors, we capture it and let the LLM know there was an internal error (source). Simply put, this can't happen from inside CodeCompanion's tool orchestration layer. All auto_submit_errors does is save the user from pressing |
|
@olimorris thanks for the pointer in the orchestrator! I'll try to reproduce and see how that goes! |
|
ex: Maybe I should open another issue on that? |
This suggests something is wrong with your config rather than with CodeCompanion. Also, are we conflating other issues in this PR? I was happy that Mistral and tool calling was working fine, so thought we could merge this PR? |
|
@olimorris, yes mistral tool calling is working, let's merge! |
|
@olimorris actually this behavior is specific to mistral. All errors have to be reported to the agent upstream. But with Mistral, you always need to report all tool call success and errors to the agent - otherwise it breaks with the following: |
|
Can you share the steps you take to get this error? I can try and recreate it |
|
Sure!
|
|
For full transparency, I've got the message stack of both Copilot and Mistral, with some messages removed for brevity: Copilot: local messages =
{ {
_meta = {
cycle = 1,
id = 841883897,
index = 3,
sent = true
},
content = "Can you use the search_web tool and tell me the weather in London?",
opts = {
visible = true
},
role = "user"
}, {
_meta = {
cycle = 1,
id = 1789836948,
index = 5
},
opts = {
visible = false
},
role = "llm",
tools = {
calls = { {
_index = 0,
["function"] = {
arguments = '{"domains":[],"query":"current weather in London"}',
name = "search_web"
},
id = "call_83qmnBnz3baNyrQjxU8mlCKG",
type = "function"
} }
}
}, {
_meta = {
cycle = 1,
id = 12606342
},
content = "Error searching for `current weather in London`",
opts = {
visible = true
},
role = "tool",
tools = {
call_id = "call_83qmnBnz3baNyrQjxU8mlCKG"
}
}, {
_meta = {
cycle = 2,
id = 405742781,
index = 7,
sent = true
},
content = "What went wrong?",
opts = {
visible = true
},
role = "user"
}, {
_meta = {
cycle = 2,
id = 663314293,
index = 8
},
content = "The search tool encountered an error while trying to retrieve information about the current weather in London. This could be due to a temporary connectivity issue, a problem with the search service, or a restriction on accessing real-time weather data.\n\nWould you like me to try again or assist with something else?",
opts = {
visible = true
},
role = "llm"
} }Mistral: local messages =
{ {
_meta = {
cycle = 1,
id = 841883897,
index = 3,
sent = true
},
content = "Can you use the search_web tool and tell me the weather in London?",
opts = {
visible = true
},
role = "user"
}, {
_meta = {
cycle = 1,
id = 1188603807,
index = 5
},
opts = {
visible = false
},
role = "llm",
tools = {
calls = { {
["function"] = {
arguments = '{"query": "weather in London", "domains": []}',
name = "search_web"
},
id = "K2gTwq1e7"
} }
}
}, {
_meta = {
cycle = 1,
id = 364323211
},
content = "Error searching for `weather in London`",
opts = {
visible = true
},
role = "tool",
tools = {
call_id = "K2gTwq1e7"
}
}, {
_meta = {
cycle = 2,
id = 442009274,
index = 7,
sent = true
},
content = "Thanks. What went wrong?",
opts = {
visible = true
},
role = "user"
} }and I get this error with Mistral: Both stacks are the same and it looks like Mistral goes against the OpenAI standard by mandating So the options I can think of, are:
|
|
@olimorris I'm glad you managed to reproduce it! |
1e37423 to
d4c5656
Compare
d4c5656 to
f0f404e
Compare
|
@olimorris this approach could work too: |
|
Hi @flovilmart I was working on a similar merge request #2246 that also enabled tool calling. I used I see you add _index instead of using the id like openai. about
I only got this error. when tool calling was interrupted somewhere.
Hope that helps you |
if you look at the code I removed the |
|
I did not run into that issue. I am now comparing you branch with mine: one change i see: i think that are the wrong args. don`t know if that was the issue. can you try again with |
|
@kwibus it's great you didn't run into that issue. This is ready to merge as far as I can tell. we can probably revisit later but I'd rather not change the implementation that I got now - given I had edge cases that required to remove the |
|
@flovilmart, no problem. I can try around a bit myself. if I can reproduce that issue, I will let you know. |
|
This may be dependent on the implementation of |
402c707 to
dacbce7
Compare
|
Thanks for all your hard work on this @flovilmart. |
|
Thanks for putting up with me @olimorris !
|
Description
Not sure why, but tools are disabled for mistral adapter.
As an avid Mistral and Codestral user, I wanted to remediate this!
I'm not sure what led to the tools being disabled, they were flaky in some old versions, but my surface testing seems to be working as needed.
Checklist
make allto ensure docs are generated, tests pass and my formatting is appliedCodeCompanion.hasin the init.lua file for my new feature