Skip to content
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

fix: 4165 - Refactor message builder to avoid sending empty messages #4199

Conversation

louis-jan
Copy link
Contributor

Describe Your Changes

  • I’ve resolved the issue where the app could include empty messages, which caused problems with a few remote providers. There was a case where message pushing could include an empty message in case of an error.

CleanShot 2024-12-03 at 15 20 51

Fixes Issues

Self Checklist

The code changes involve refactoring the function normalizeMessages and its integration within the system:

  1. Removal from useSendChatMessage.ts:

    • The normalizeMessages function and the Stack import were removed from useSendChatMessage.ts. This function was previously responsible for ensuring consecutive messages from the same role (e.g., User or Assistant) were separated by an empty message. The feature was meant to meet specific requirements of some models.
    • The call to normalizeMessages was also removed from the resendChatMessage function, meaning normalization is no longer performed directly there.
  2. Integration into messageRequestBuilder.ts:

    • normalizeMessages was moved into MessageRequestBuilder class in messageRequestBuilder.ts. This function maintains the same logic to avoid consecutive messages of the same role by inserting a dummy message.
    • The normalization process is now applied during the build() function call, ensuring the message list is normalized whenever a MessageRequest is built.
    • The Stack utility was imported to support this functionality.

By consolidating the normalizeMessages logic within the MessageRequestBuilder, the process of managing message roles is now contained within the request-building phase, promoting better cohesion and separability within the codebase.

@louis-jan louis-jan requested a review from a team December 3, 2024 08:21
@github-actions github-actions bot added the type: bug Something isn't working label Dec 3, 2024
Copy link
Contributor

github-actions bot commented Dec 3, 2024

Barecheck - Code coverage report

Total: 69.33%

Your code coverage diff: 0.01% ▴

Uncovered files and lines
FileLines
web/hooks/useSendChatMessage.ts89-90, 92-97, 100-101, 106, 113, 115-117, 120-121, 123-124, 126, 128-130, 132, 136, 138, 142, 145-146, 152-153, 157, 168, 171, 175, 178, 181, 189, 192, 198, 201-202, 205-209, 212, 215, 218, 223, 228-229
web/utils/messageRequestBuilder.ts38-39, 51-54, 56, 63, 68-69, 76, 81, 96-97, 102, 118-119, 123-124, 129, 132, 138-142, 144, 146, 148, 156, 159, 163

@louis-jan louis-jan merged commit 7f62e84 into dev Dec 3, 2024
11 checks passed
@louis-jan louis-jan deleted the fix/4165-refactor-message-builder-to-avoid-sending-empty-messages branch December 3, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants