Fix IndexError when collating chat history #195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes a bug which causes the exception:
```
ERROR:root:Exception in /api/conversation/custom | list index out of range
Traceback (most recent call last):
File "/workspaces/chat-with-your-data-solution-accelerator/code/app/app.py", line 283, in conversation_custom
chat_history.append((user_assistant_messages[i]['content'],user_assistant_messages[i+1]['content']))
```
This is caused when there has been an error providing a response, and
the latest message in the history is from a user, rather than the
assitant. Our code assumes a user message is always followed by an
assistant message.
This change removes that assumption and explitely retreives the role for
each message when collating the chat history.
Required by Azure-Samples#114
|
I hate asking this - but is there a test that we can put around this? |
- Extract some elements to dedicated function to allow mocking
|
@ross-p-smith test added! |
eduardogch
pushed a commit
to devopsdale/chat-with-your-data-solution-accelerator
that referenced
this pull request
Apr 30, 2024
* Fix IndexError when collating chat history
This fixes a bug which causes the exception:
```
ERROR:root:Exception in /api/conversation/custom | list index out of range
Traceback (most recent call last):
File "/workspaces/chat-with-your-data-solution-accelerator/code/app/app.py", line 283, in conversation_custom
chat_history.append((user_assistant_messages[i]['content'],user_assistant_messages[i+1]['content']))
```
This is caused when there has been an error providing a response, and
the latest message in the history is from a user, rather than the
assitant. Our code assumes a user message is always followed by an
assistant message.
This change removes that assumption and explitely retreives the role for
each message when collating the chat history.
Required by Azure-Samples#114
* Add python formatter to dev container
* Add tests for conversation custom
- Extract some elements to dedicated function to allow mocking
* Switch to black formatter to align with precommit hook
* Add test to cover error scenario when message index is out of range
* Add dependencies required for running app tests
---------
Co-authored-by: Ross Smith <ross-p-smith@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
This fixes a bug which causes the exception:
This is caused when there has been an error providing a response, and the latest message in the history is from a user, rather than the assitant. Our code assumes a user message is always followed by an assistant message.
This change removes that assumption and explitely retreives the role for each message when collating the chat history.
Required by #114
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?