Skip to content

Conversation

@flovilmart
Copy link
Contributor

@flovilmart flovilmart commented Nov 8, 2025

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:

Output data:
{"object":"error","message":"Unexpected role 'user' after role 'tool'","type":"invalid_request_message_order","param":null,"code":"3230"}
[ERROR] 2025-11-07 19:08:30
Error: {"object":"error","message":"Unexpected role 'user' after role 'tool'","type":"invalid_request_message_order","param":null,"code":"3230"}

When this happens, the session is fully broken

Checklist

  • I've read the contributing guidelines and have adhered to them in this PR
  • I've added test coverage for this fix/feature
  • I've run make all to ensure docs are generated, tests pass and my formatting is applied
  • (optional) I've updated CodeCompanion.has in the init.lua file for my new feature
  • (optional) I've updated the README and/or relevant docs pages

@olimorris olimorris added the P4 Negligible impact and urgency label Nov 8, 2025
@flovilmart
Copy link
Contributor Author

flovilmart commented Nov 8, 2025

@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.
Happy to revisit implementation with your input 😉

@olimorris
Copy link
Owner

olimorris commented Nov 8, 2025

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 form_messages to receive the chat buffer number or the ID. That way you could store "pending" messages in the adapter itself and clear them out when they are consumed. Doubt many people would have multiple Mistral chats going on at the same time, using tools, but just in case...

@flovilmart
Copy link
Contributor Author

flovilmart commented Nov 8, 2025

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.

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

Or, store it and insert it into the message stack at an appropriate time in the future?

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.
Personally I think this workaround is plenty enough.

@flovilmart
Copy link
Contributor Author

Another example after using an edit tool and accepting the change:

    {
      "role": "assistant",
      "content": "I'll edit the `tests/adapters/http/test_mistral.lua` file to add the new test case. Here's the code to add:\n\n````lua\n// filepath: tests/adapters/http/test_mistral.lua\n// ...existing code...\n{ role = \"tool\", content = \"ok\" },\n{ role = \"assistant\", content = \"that works\" },\n// ...existing code...\nT[\"Mistral adapter\"][\"form_messages\"][\"it skips messages when user follows tools\"] = function()\n// ...existing code...\n{ role = \"tool\", content = \"ok\" },\n// ...existing code...\nT[\"Mistral adapter\"][\"form_messages\"][\"it can form messages with assistant after tool\"] = function()\n  local input = {\n    { role = \"user\", content = \"User1\" },\n    {\n      role = \"llm\",\n      tools = {\n        calls = {\n          {\n            [\"function\"] = {\n              arguments = '{\"location\":\"London, UK\",\"units\":\"fahrenheit\"}',\n              name = \"weather\",\n            },\n            id = \"call_1_a460d461-60a7-468c-a699-ef9e2dced125\",\n            type = \"function\",\n          },\n        },\n      },\n    },\n    { role = \"tool\", content = \"ok\" },\n    { role = \"assistant\", content = \"Assistant response\" },\n  }\n\n  local expected = {\n    messages = {\n      {\n        content = \"User1\",\n        role = \"user\",\n      },\n      {\n        role = \"llm\",\n        tool_calls = {\n          {\n            [\"function\"] = {\n              arguments = '{\"location\":\"London, UK\",\"units\":\"fahrenheit\"}',\n              name = \"weather\",\n            },\n            id = \"call_1_a460d461-60a7-468c-a699-ef9e2dced125\",\n            type = \"function\",\n          },\n        },\n      },\n      { role = \"tool\", content = \"ok\" },\n      { role = \"assistant\", content = \"Assistant response\" },\n    },\n  }\n\n  h.eq(expected, adapter.handlers.form_messages(adapter, input))\nend\n// ...existing code...\n````\n\nThis test case verifies that the mistral adapter can properly form messages when there's an assistant message following a tool message. The test checks that the assistant message is included in the output without any modifications."
    },
    { "role": "user", "content": "EDIT THE FILE" },
    {
      "role": "assistant",
      "tool_calls": [
        {
          "id": "5xJ4yNuGq",
          "function": {
            "arguments": "{\"filepath\": \"tests/adapters/http/test_mistral.lua\", \"mode\": \"append\", \"dryRun\": false, \"explanation\": \"Adds test for the mistral adapter for a case where there is an assistant message after the tool. it forms messages properly\", \"edits\": [{\"newText\": \"T[\\\"Mistral adapter\\\"][\\\"form_messages\\\"][\\\"it can form messages with assistant after tool\\\"] = function()\\n  local input = {\\n    { role = \\\"user\\\", content = \\\"User1\\\" },\\n    {\\n      role = \\\"llm\\\",\\n      tools = {\\n        calls = {\\n          {\\n            [\\\"function\\\"] = {\\n              arguments = '{\\\"location\\\":\\\"London, UK\\\",\\\"units\\\":\\\"fahrenheit\\\"}',\\n              name = \\\"weather\\\",\\n            },\\n            id = \\\"call_1_a460d461-60a7-468c-a699-ef9e2dced125\\\",\\n            type = \\\"function\\\",\\n          },\\n        },\\n      },\\n    },\\n    { role = \\\"tool\\\", content = \\\"ok\\\" },\\n    { role = \\\"assistant\\\", content = \\\"Assistant response\\\" },\\n  }\\n\\n  local expected = {\\n    messages = {\\n      {\\n        content = \\\"User1\\\",\\n        role = \\\"user\\\",\\n      },\\n      {\\n        role = \\\"llm\\\",\\n        tool_calls = {\\n          {\\n            [\\\"function\\\"] = {\\n              arguments = '{\\\"location\\\":\\\"London, UK\\\",\\\"units\\\":\\\"fahrenheit\\\"}',\\n              name = \\\"weather\\\",\\n            },\\n            id = \\\"call_1_a460d461-60a7-468c-a699-ef9e2dced125\\\",\\n            type = \\\"function\\\",\\n          },\\n        },\\n      },\\n      { role = \\\"tool\\\", content = \\\"ok\\\" },\\n      { role = \\\"assistant\\\", content = \\\"Assistant response\\\" },\\n    },\\n  }\\n\\n  h.eq(expected, adapter.handlers.form_messages(adapter, input))\\nend\\n\", \"replaceAll\": false, \"oldText\": \"{ role = \\\"tool\\\", content = \\\"ok\\\" },\\n    { role = \\\"assistant\\\", content = \\\"that works\\\" },\"}]}",
            "name": "insert_edit_into_file"
          }
        }
      ]
    },
    {
      "role": "tool",
      "tool_call_id": "5xJ4yNuGq",
      "content": "Edited `tests/adapters/http/test_mistral.lua` buffer\nAdds test for the mistral adapter for a case where there is an assistant message after the tool. it forms messages properly"
    },
    {
      "role": "user",
      "content": "<attachment filepath=\"tests/adapters/http/test_mistral.lua\" buffer_number=\"56\">The file `tests/adapters/http/test_mistral.lua`, has been modified. Here are the changes:\n```diff\n@@ -50,8 +50,55 @@\n         },\n       },\n     },\n+T[\"Mistral adapter\"][\"form_messages\"][\"it can form messages with assistant after tool\"] = function()\n+  local input = {\n+    { role = \"user\", content = \"User1\" },\n+    {\n+      role = \"llm\",\n+      tools = {\n+        calls = {\n+          {\n+            [\"function\"] = {\n+              arguments = '{\"location\":\"London, UK\",\"units\":\"fahrenheit\"}',\n+              name = \"weather\",\n+            },\n+            id = \"call_1_a460d461-60a7-468c-a699-ef9e2dced125\",\n+            type = \"function\",\n+          },\n+        },\n+      },\n+    },\n     { role = \"tool\", content = \"ok\" },\n-    { role = \"assistant\", content = \"that works\" },\n+    { role = \"assistant\", content = \"Assistant response\" },\n+  }\n+\n+  local expected = {\n+    messages = {\n+      {\n+        content = \"User1\",\n+        role = \"user\",\n+      },\n+      {\n+        role = \"llm\",\n+        tool_calls = {\n+          {\n+            [\"function\"] = {\n+              arguments = '{\"location\":\"London, UK\",\"units\":\"fahrenheit\"}',\n+              name = \"weather\",\n+            },\n+            id = \"call_1_a460d461-60a7-468c-a699-ef9e2dced125\",\n+            type = \"function\",\n+          },\n+        },\n+      },\n+      { role = \"tool\", content = \"ok\" },\n+      { role = \"assistant\", content = \"Assistant response\" },\n+    },\n+  }\n+\n+  h.eq(expected, adapter.handlers.form_messages(adapter, input))\n+end\n+\n   }\n \n   local expected = {\n```</attachment>"
    }

@flovilmart
Copy link
Contributor Author

flovilmart commented Nov 8, 2025

actually it comes from here, when the buffer gets updated after accepting a change 🤔

@flovilmart
Copy link
Contributor Author

@olimorris any idea? Can we disable the file watcher for mistral otherwise if the file is in the buffer?

@olimorris
Copy link
Owner

olimorris commented Nov 13, 2025

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.

Can we disable the file watcher for mistral otherwise if the file is in the buffer?

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P4 Negligible impact and urgency

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants