Pair-Bug in remember_messages functionality

[SawPsyder]

I noticed an increase in reports and messages with this error.
Invalid parameter: messages with role 'tool' must be a response to a preceeding message with 'tool_calls'.

So I decided to dive a bit deeper in the code. It seems like the origin is the remember_messages functionality.

It "thinks" in pairs - but this is not correct.
So this is what it suspects the data to look like:

{'role': 'user', 'content': '...'}
ChatCompletionMessage(content="...", role='assistant', function_call=None, tool_calls=None)

But if a request resulted in one or multiple functions beeing triggered, it looks like this:

{'role': 'user', 'content': '...'}
ChatCompletionMessage(content=None, role='assistant', function_call=None, tool_calls=[ChatCompletionMessageToolCall(id='call_pvoBrd5vbS5RgS4cMIkHZsKI', function=Function(arguments='{}', name='PowerShields'), type='function'), ChatCompletionMessageToolCall(id='call_HXZ2ka96gOohkPjN9JSqcGPM', function=Function(arguments='{}', name='PowerEngines'), type='function'), ChatCompletionMessageToolCall(id='call_Xu2l7sz7DX9stIZLpyumtKSH', function=Function(arguments='{}', name='PowerShip'), type='function'), ChatCompletionMessageToolCall(id='call_M1kGfaHCkMF951PeVUdDRoif', function=Function(arguments='{}', name='InitiateStartSequence'), type='function')])
{'role': 'tool', 'content': '', 'tool_call_id': 'call_pvoBrd5vbS5RgS4cMIkHZsKI', 'name': 'PowerShields'}
{'role': 'tool', 'content': '', 'tool_call_id': 'call_HXZ2ka96gOohkPjN9JSqcGPM', 'name': 'PowerEngines'}
{'role': 'tool', 'content': '', 'tool_call_id': 'call_Xu2l7sz7DX9stIZLpyumtKSH', 'name': 'PowerShip'}
{'role': 'tool', 'content': '', 'tool_call_id': 'call_M1kGfaHCkMF951PeVUdDRoif', 'name': 'InitiateStartSequence'}

And when in this example a 'role': 'user' element is deleted, a 'role': 'tool' element will be at the start at the history. And thats where openai says invlaid_request_error.

So it should rather count elements with 'role': 'user', than think in pairs.

So "_cleanup_conversation_history" method in open_ai_wingman.py needs a rework. And probably at every point in code, where the "pairs" are printed out, as its currently not correct.

Linked to Discord Bug report: https://discord.com/channels/1173573578604687360/1183085077022908456

[Shackless]

Thank you, that makes perfect sense. I’ll take a look at this!

[Shackless]

@SawPsyder This seems better. Would you mind to review and/or test it on the linked branch?

[SawPsyder]

I reviewed the change, but its not yet working correctly.
The deletion works in the way, that OpenAI will not receive an inkonsistent history.

But these two things need a fix:

• Currently it always deletes messages. A check is missing, determining if user messages are more than the limit.
• And if the configured limit is 3, it will delete the oldest 3 messages, instead of keeping the 3 newest messages.

@Shackless I fixed both while testing. Feel free to take this as an inspiration or copy it or whatever :)

def _cleanup_conversation_history(self):
    """Cleans up the conversation history by removing messages that are too old."""
    remember_messages = self.config.get("features", {}).get(
        "remember_messages", None
    )

    if remember_messages is None or len(self.messages) == 0:
        return 0  # Configuration not set, nothing to delete.

    # The system message aka `context` does not count
    context_offset = (
        1 if self.messages and self.messages[0]["role"] == "system" else 0
    )

    # Find the cutoff index where to end deletion, making sure to only count 'user' messages towards the limit starting with newest messages.
    cutoff_index = len(self.messages) - 1
    user_message_count = 0
    for message in reversed(self.messages):
        if self.__get_message_role(message) == "user":
            user_message_count += 1
            if user_message_count == remember_messages:
                break  # Found the cutoff point.
        cutoff_index -= 1

    # If messages below the keep limit, don't delete anything.
    if user_message_count < remember_messages:
        return 0

    total_deleted_messages = cutoff_index - context_offset  # Messages to delete.

    # Remove the messages before the cutoff index, exclusive of the system message.
    del self.messages[context_offset:cutoff_index]

    # Optional debugging printout.
    if self.debug and total_deleted_messages > 0:
        printr.print(
            f"Deleted {total_deleted_messages} messages from the conversation history.",
            tags="warn",
        )

    return total_deleted_messages