Skip to content

fix: correct role assignment in chat_history and typo in get_embedding - #166

Open
dajiaohuang wants to merge 1 commit into
OpenBMB:mainfrom
dajiaohuang:fix/chat-history-role-assignment
Open

dajiaohuang wants to merge 1 commit into
OpenBMB:mainfrom
dajiaohuang:fix/chat-history-role-assignment

Conversation

@dajiaohuang

Copy link
Copy Markdown

What

Two fixes:

  1. chat_history.py: In to_messages(), messages from other agents (i.e., not my_name and not "function") were incorrectly assigned "assistant" role. These should be "user" role to maintain proper conversation alternation required by models like LLAMA that enforce user/assistant/user alternation.

  2. openai.py: Fixed NameError in get_embedding() exception handler where attempt was used instead of attempts (the function parameter).

Why

  1. Issue why the to_message function in chat_history.py is trying to add an assistent message?聽#133: When using LLAMA models via vLLM, the API returns BadRequestError: Conversation roles must alternate user/assistant/user/assistant/... because messages from other agents in multi-agent conversations were being labeled as "assistant" role.

  2. The get_embedding function has a bug where if an exception occurs, it tries to increment an undefined variable attempt instead of the parameter attempts.

Testing

  • The chat_history fix follows the same pattern already used in the codebase where my_name messages are labeled "assistant" and "function" messages are labeled "function" - messages from other agents should logically be "user".
  • The get_embedding fix is a simple typo correction.

- chat_history.py: messages from other agents should be "user" role
  not "assistant" to maintain proper conversation alternation for
  models like LLAMA that require user/assistant/user alternation
- openai.py: fix NameError in get_embedding exception handler
  where 'attempt' was used instead of 'attempts' parameter
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.

1 participant