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.

Sign up for GitHub

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

Jump to bottom

[fix] for issue #2447 (OpenAI-like API fails when a redundant, empty assistant message is sent over) #2453

Merged
merged 10 commits into from
May 21, 2024
Merged

[fix] for issue #2447 (OpenAI-like API fails when a redundant, empty assistant message is sent over) #2453

merged 10 commits into from
May 21, 2024

Conversation

AlyMobarak
Copy link
Contributor

Pull Request Checklist

Note to first-time contributors: Please open a discussion post in Discussions and describe your changes before submitting a pull request.

Before submitting, make sure you've checked the following:

  • [✅] Target branch: Please verify that the pull request targets the dev branch.
  • [✅] Description:
  • [✅] Changelog: Ensure a changelog entry following the format of Keep a Changelog is added at the bottom of the PR description.
  • [NO NEED] Documentation: Have you updated relevant documentation Open WebUI Docs, or other documentation sources?
  • [NO NEED] Dependencies: Are there any new dependencies? Have you updated the dependency versions in the documentation?
  • [NO NEED] Testing: Have you written and run sufficient tests for validating the changes?
  • [✅] Code review: Have you performed a self-review of your code, addressing any coding standard issues and ensuring adherence to the project's coding standards?
  • [✅] Label: fix

Changelog Entry

Description

I've noticed an issue with the UI described at issue #2447. After fiddling around for some time, I've come up with a really simple solution that doesn't require any major code refactoring or even a change in testing methods. I simply added a rule to filter messages before getting sent to OpenAI API and remove empty ones. I have no idea if the actual OpenAI API accepts this or not as I'm broke enough to not be able to buy the API and test on it, but it should work fine based on my ChatGPT free usage network monitoring.

I've tested it on my LM Studio OpenAI like server and it works like a charm now.. I'll be using my local image for the docker container (which was built successfully btw) until this change is hopefully upstream.

Added

  • N/A

Changed

  • N/A

Deprecated

  • N/A

Removed

  • N/A

Fixed

  • Filtered messages before getting sent to an OpenAI API or similar based on whether they have actual contents or not. This should not have any effects on what was previously working, but would make OpenAI APIs like LM Studio's work.

Security

  • N/A

Breaking Changes

  • N/A

Additional Information

Screenshots or Videos

BEFORE:
Before Bug Fix, OpenWebUI Interface

AFTER:
Screenshot 2024-05-21 at 1 47 20 PM

tjbck and others added 10 commits May 19, 2024 10:16
@AlyMobarak AlyMobarak changed the title Bug Fix for issue #2447 (OpenAI-like API fails when a redundant, empty assistant message is sent over) [fix] for issue #2447 (OpenAI-like API fails when a redundant, empty assistant message is sent over) May 21, 2024
@justinh-rahb
Copy link
Collaborator

justinh-rahb commented May 21, 2024 •
edited
Loading

This should already be fixed in main branch. You can also workaround it by enabling the Memory feature and adding a memory to it, but updating is the best course of action.

@AlyMobarak AlyMobarak mentioned this pull request May 21, 2024
Closed
@AlyMobarak
Copy link
Contributor Author

Oh..? But I really just checked right before writing my fix.. I don't know I'm just grateful it now works.... but can you honestly take a moment and review it? I don't want to be stuck with the current version for this small thing. Or as you said I could just workaround using the memory which I don't really want but hey it might fix it so yeah. Thank you!

@justinh-rahb
Copy link
Collaborator

I'm somewhat certain this is the same issue I'm thinking of, if enabling memory causes things to work normally again with unmodified code then that would probably be the case. It was supposed to have been fixed already but I suppose it's also possible we missed something. Let us know if you can test a re-pulled main or dev unmodified with and without memory enabled.

@AlyMobarak
Copy link
Contributor Author

AlyMobarak commented May 21, 2024 •
edited
Loading

Yup. I have tested it with a re-pulled dev and a local docker image build from it, and the images of the results are provided in the issue. I've also references the two lines of code I changed. Literally two lines of code fix this, it's simple just like you said.. so simple that you may've missed it 😊! Here they are just in case,
332342901-88747ca3-fe11-4f2c-8aeb-b6284ec5610d-2
332343357-b406c42c-ed90-4673-9ad3-84d7677ea6ff
332353907-65fae7d1-24ec-4261-98c1-3a58e3c7ecc5

@AlyMobarak
Copy link
Contributor Author

Screenshot 2024-05-21 at 2 28 24 PM

Also here's the server logs output after filtering.

@nwtgck
Copy link

nwtgck commented May 21, 2024

Hi,

I have the same issue at the current main (3355577) where memory is both ON and OFF.
image

This PR fixes the issue 👍 :
image

@tjbck
Copy link
Contributor

tjbck commented May 21, 2024

LGTM, thanks!

@tjbck tjbck merged commit b3de612 into open-webui:dev May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AlyMobarak @justinh-rahb @nwtgck @tjbck