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

feat: regenerate in Chat, agent and Chatflow app #7661

Open
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

xuzuodong
Copy link
Contributor

@xuzuodong xuzuodong commented Aug 26, 2024

Description

Fixes #7161

Support mode: Chatbot, Agent and Chatflow mode

  1. The regenerate button can be accessed via

    • ✅ In explore page, i.e. installed App
    • ✅ Single debug view
    • ✅ Web app
    • ✅ Embedded app
  2. The regenerate button won't be displayed in

    • ❌ Multiple debug view
    • ❌ Log view
    • ❌ Chatflow's chat record view
  3. Completion and Workflow mode should not be affected by this PR.

  4. All sent messages in Chatbot, Agent and Chatflow have parent_message_id field now for future considerations (important for implementing branched chat history tree).

Demonstration

  1. Agent mode, accessed via Explore page:
2024-08-26.18.10.32.mov
  1. Chatflow mode, accessed via embedding app:
2024-08-30.16.49.09.mov

Known issue

The Chatflow run history and log detail view are designed to display all historical messages. However, for now, it is flattened and displays all messages simultaneously, rather than offering an interactive UI to toggle between different threads.

I've already implemented this fix in a separated PR xuzuodong#1, so we can look into this later.

Database migration caveat

A new field, parent_message_id, will be added to the messages table. All existing messages will be set to a special value for future compatibility: 00000000-0000-0000-0000-000000000000.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update, included: Dify Document
  • Improvement, including but not limited to code refactoring, performance optimization, and UI/UX improvement
  • Dependency upgrade

Testing Instructions

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

@xuzuodong xuzuodong changed the title feat: regenerate in chat and agent app feat: regenerate in Chat, agent and Chatflow app Aug 30, 2024
@xuzuodong xuzuodong marked this pull request as ready for review September 2, 2024 01:25
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. 💪 enhancement New feature or request 🤖 feat:agent Agent related topics labels Sep 2, 2024
@xuzuodong xuzuodong marked this pull request as draft September 5, 2024 08:41
@takatost
Copy link
Collaborator

1726292255495
1726292304657

I tried regenerating both conversation histories 1 and 2. They display correctly in the WebApp. However, after regenerating, the regenerated records are also shown in the App Log, and they are sorted by time.

I think the App Log should, by default, only display the regenerated records. If possible, it would be great to add < > toggle buttons to allow viewing of the historical generation records.

@takatost
Copy link
Collaborator

1726292688243

The parent_message_id in the messages table has both UUID_NIL and None in the form of a null value, which is expected to be ?

@xuzuodong
Copy link
Contributor Author

I think the App Log should, by default, only display the regenerated records. If possible, it would be great to add < > toggle buttons to allow viewing of the historical generation records.

This is exactly the next PR I will create (currently at xuzuodong#1). I separated this to another PR so that changes of files and codes won't be that much and can be easier to review.

@takatost
Copy link
Collaborator

I think the App Log should, by default, only display the regenerated records. If possible, it would be great to add < > toggle buttons to allow viewing of the historical generation records.

This is exactly the next PR I will create (currently at xuzuodong#1). I separated this to another PR so that changes of files and codes won't be that much and can be easier to review.

Is it possible to keep only the last record this time? Showing it all in the Log can be confusing for app maintainers.

@xuzuodong
Copy link
Contributor Author

xuzuodong commented Sep 14, 2024

1726292688243

The parent_message_id in the messages table has both UUID_NIL and None in the form of a null value, which is expected to be ?

The parent_message_id value of all messages before this PR will be marked as UUID_NIL.

For the first message in a conversation and the first regenerated message, their parent_message_id will be set to NULL.

These two cases need to be distinguished to support left-right conversation switching in next implementation.

@xuzuodong
Copy link
Contributor Author

Is it possible to keep only the last record this time? Showing it all in the Log can be confusing for app maintainers.

Sure, will do it today.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 14, 2024
@takatost
Copy link
Collaborator

1726292688243 The parent_message_id in the messages table has both UUID_NIL and None in the form of a null value, which is expected to be ?

The parent_message_id value of all messages before this PR will be marked as UUID_NIL.

For the first message in a conversation and the first regenerated message, their parent_message_id will be set to NULL.

These two cases need to be distinguished to support left-right conversation switching in next implementation.

Does it mean that UUID_NIL means that it was generated only once, None means the first time, and having a specific UUID is the subsequent re-generation of the message record?

@xuzuodong
Copy link
Contributor Author

xuzuodong commented Sep 14, 2024

Does it mean that UUID_NIL means that it was generated only once,

UUID_NIL is actually just a mark that says the message is from old version, and later when implementing the message navigating feature, these messages should be processed in a different way.

None means the first time

NULL means the message is a head message.

having a specific UUID is the subsequent re-generation of the message record?

And after this PR take effect, all non-head sent messages will have a valid UUID (non NULL and non UUID_NIL).


Let me show some images to explain this more accurately

  1. Before this PR takes effect, all messages that already exist in the DB (or we can call this kind of messages old version messages, or legacy messages) will look like this:

image

  1. After this PR takes effect, newly sent messages will look like:

image

Note that the first message with id 1 doesn't have a parent_message_id, meaning that it's a head message.

  1. Now Let's say we regenerate message 3, which will introduce a new branch, and the newly generated message will also be based on message 2:

image

  1. Now what if I want to regenerate message 1?

Here's what's going to happen:

image

The newly created message will also be a head message. That said, if a message's parent_message_id is NULL, this is no doubt a head message, but it can be either the first message of the conversation or a regeneration of the first message.

  1. Plus, we can also regenerate a message that already exists before this PR takes affect:

image

@takatost
Copy link
Collaborator

Does it mean that UUID_NIL means that it was generated only once,UUID_NIL 是否意味着只生成过一次?

UUID_NIL is actually just a mark that says the message is from old version, and later when implementing the message navigating feature, these messages should be processed in a different way.UUID_NIL 实际上只是一个标记,表示该消息来自旧版本,以后在实现消息导航功能时,应以不同的方式处理这些消息。

None means the first time没有意味着第一次

NULL means the message is a head message.NULL 表示报文是标题报文。

having a specific UUID is the subsequent re-generation of the message record?如果有一个特定的 UUID,随后是否会重新生成报文记录?

And after this PR take effect, all non-head sent messages will have a valid UUID (non NULL and non UUID_NIL).此 PR 生效后,所有非头部发送的信息都将具有有效的 UUID(非 NULL 和非 UUID_NIL)。

Let me show some images to explain this more accurately让我展示一些图片来更准确地解释这一点

  1. Before this PR takes effect, all messages that already exist in the DB (or we can call this kind of messages old version messages, or legacy messages) will look like this:在此 PR 生效之前,数据库中已存在的所有报文(我们也可以称此类报文为旧版报文或遗留报文)都将是这样的:

image

  1. After this PR takes effect, newly sent messages will look like:此 PR 生效后,新发送的信息将看起来像这样:

image

Note that the first message with id 1 doesn't have a parent_message_id, meaning that it's a head message.请注意,id 为 1 的第一条报文没有 parent_message_id,这意味着它是一条头部报文。

  1. Now Let's say we regenerate message 3, which will introduce a new branch, and the newly generated message will also be based on message 2:现在,假设我们重新生成消息 3,这将引入一个新的分支,而新生成的消息也将基于消息 2

image

  1. Now what if I want to regenerate message 1?现在,如果我想重新生成消息 1 怎么办?

Here's what's going to happen:接下来会发生什么?

image

The newly created message will also be a head message. That said, if a message's parent_message_id is NULL, this is no doubt a head message, but it can be either the first message of the conversation or a regeneration of the first message.新创建的消息也将是一条头部消息。也就是说,如果一条信息的 parent_message_idNULL ,这无疑是一条头部信息,但它既可以是对话的第一条信息,也可以是第一条信息的再生信息。

  1. Plus, we can also regenerate a message that already exists before this PR takes affect:此外,我们还可以重新生成在此 PR 生效之前已经存在的信息:

image

I see what UUID_NIL means, if you don't do that, you have to batch update the parent-child ID data via script, and there will be a longer data break when updating the service, it's really better this way

Does it mean that UUID_NIL means that it was generated only once,

UUID_NIL is actually just a mark that says the message is from old version, and later when implementing the message navigating feature, these messages should be processed in a different way.

None means the first time

NULL means the message is a head message.

having a specific UUID is the subsequent re-generation of the message record?

And after this PR take effect, all non-head sent messages will have a valid UUID (non NULL and non UUID_NIL).

Let me show some images to explain this more accurately

  1. Before this PR takes effect, all messages that already exist in the DB (or we can call this kind of messages old version messages, or legacy messages) will look like this:

image

  1. After this PR takes effect, newly sent messages will look like:

image

Note that the first message with id 1 doesn't have a parent_message_id, meaning that it's a head message.

  1. Now Let's say we regenerate message 3, which will introduce a new branch, and the newly generated message will also be based on message 2:

image

  1. Now what if I want to regenerate message 1?

Here's what's going to happen:

image

The newly created message will also be a head message. That said, if a message's parent_message_id is NULL, this is no doubt a head message, but it can be either the first message of the conversation or a regeneration of the first message.

  1. Plus, we can also regenerate a message that already exists before this PR takes affect:

image

I see what UUID_NIL means, if don't do that, you have to batch update the parent-child ID data via script, and there will be a longer data break when updating the service, it's really better this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 enhancement New feature or request 🤖 feat:agent Agent related topics lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chat Enhance Edit and Retry/Regenerate button
3 participants