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 regeneration of freshly generated chat messages #188

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

julien-nc
Copy link
Member

Short version

This fixes regeneration of messages that were freshly generated. The task ID is now stored in the message entry so we can get a message from a task ID. That is helpful for the /chat/check_generation?taskId=123 endpoint.

Long version

When a chat task has finished, a listener stores a message in the oc_assistant_chat_msgs table.
In the UI, when a message is currently being generated, the /chat/check_generation?taskId=123 assistant endpoint is polled.
When the related controller sees that the task has finished and is successful:

  • it used to generate and return a fake message object on the fly, based on the data in the finished task (because it could not get the message in the DB based on the task ID)
  • it now gets the real message that was stored by the listener

The problem was that the message obtained via /chat/check_generation?taskId=123 had no ID so it could not be regenerated because we do it with a request like /chat/regenerate?messageId=619&sessionId=48 but we didn't have an ID for the freshly generated messages.

The issue was not so visible because when opening the assistant, the message list was correctly loaded (from DB entries), with all message having an ID.

@julien-nc julien-nc added bug Something isn't working 3. to review labels Feb 3, 2025
@julien-nc julien-nc requested a review from kyteinsky February 3, 2025 12:17
@julien-nc julien-nc force-pushed the fix/noid/regeneration branch from 62213ec to c3b02fa Compare February 3, 2025 15:01
@julien-nc julien-nc requested a review from kyteinsky February 3, 2025 15:02
Copy link
Contributor

@kyteinsky kyteinsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@julien-nc julien-nc merged commit 2945dcf into main Feb 3, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants