-
-
Notifications
You must be signed in to change notification settings - Fork 330
fix: Ensure we drop user after tool messages #2365
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
base: main
Are you sure you want to change the base?
Conversation
|
@olimorris I went with a very naive implementation with the filter and a boolean keeping the side effect from the previous message, not my best code, but that works. |
82584b1 to
cb260b4
Compare
|
Should we discard the user's message? Or, store it and insert it into the message stack at an appropriate time in the future? My concern would be if I'm a user, my tool fails, I respond via the chat buffer and the LLM responds without ever seeing what I've typed. That would be unexpected and depending on what I've responded with, a potential waste of my time. EDIT: To make the latter possible, I can make a PR that would allow |
This happens when we got a tool call response that hasn’t been answered by the LLM yet and the user has typed something into the buffer and sent it already. Not because the tool call failed, simply that it hasn’t been processed by the assistant yet
That a good suggestion, but probably overkill. . I saw it happen multiple times with and empty string, I’d have to check more failures on my side and see the content of that buffer. Now with the debug log I’ll be able not to crash my sessions and continue. Worst, case this is a simple copy paste from the previous chat message. |
|
Another example after using an edit tool and accepting the change: |
|
actually it comes from here, when the buffer gets updated after accepting a change 🤔 |
|
@olimorris any idea? Can we disable the file watcher for mistral otherwise if the file is in the buffer? |
|
I can't accept the PR in its current form as I think it could negatively impact the UX for a Mistral user, as I outlined above. Dropping a user's message fixes the issue but introduces another one further down the line.
No. I appreciate this fixes this problem. But again, we're impacting the user experience. I.e. what a user expects versus what is happening. If I've shared a watched buffer with the LLM, then I expect CodeCompanion to hold up its end of the bargain, and share that buffer with the LLM. This should be worked around in the Mistral adapter, with test coverage. |
cb260b4 to
4318517
Compare
Description
Mistral strictly requires that tool are followed by an assistant message.
In certain scenarios, a user message may get interleaved in the conversation breaking the complete flow:
When this happens, the session is fully broken
Checklist
make allto ensure docs are generated, tests pass and my formatting is appliedCodeCompanion.hasin the init.lua file for my new feature