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

Ensure the 'inputs' field in /chat-messages takes effect every time #7955

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

Conversation

Wu-Jiayang
Copy link

@Wu-Jiayang Wu-Jiayang commented Sep 4, 2024

Checklist:

Important

Please review the checklist below before submitting your pull request.

  • Please open an issue before creating a PR or link to an existing issue
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

Description

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue. Close issue syntax: Fixes #<issue number>, see documentation for more details.

Close #7952
Close #7846
Close #8063
Close #8839
Close #8546
Close #10522
Close #10055
Close #10643

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

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. 🐞 bug Something isn't working labels Sep 4, 2024
@cubxxw
Copy link

cubxxw commented Sep 9, 2024

This PR does not resolve, using the conversation id to link to previous sessions but the input works

@Wu-Jiayang
Copy link
Author

This PR does not resolve, using the conversation id to link to previous sessions but the input works

I don't understand "using the conversation id to link to previous sessions but the input works". As for "This PR does not resolve", I have tried and I think it works.

@Wu-Jiayang
Copy link
Author

This PR does not resolve, using the conversation id to link to previous sessions but the input works

This PR is to make sure the 'inputs' field in the API "/chat-messages" takes effect every time, is that what you mean "using the conversation id to link to previous sessions but the input works"?

@cubxxw
Copy link

cubxxw commented Sep 10, 2024

Yes, you mean that the input field of the conversation can be updated under the same conversation, because the input is only effective when the conversation link is first established

@Wu-Jiayang
Copy link
Author

Yes, you mean that the input field of the conversation can be updated under the same conversation, because the input is only effective when the conversation link is first established

@cubxxw

I noticed that both table 'messages' and table 'conversations' have an 'inputs' field. I think the 'inputs' field should be aligned with table 'messages'. The 'inputs' field in table 'conversations' may not be very useful.

In this PR, the 'inputs' field in table 'conversations' means the 'inputs' in the first message, and the 'inputs' field in table 'messages' means the true inputs in this message.

@Wu-Jiayang
Copy link
Author

This PR does not resolve, using the conversation id to link to previous sessions but the input works

What's the mean of 'This PR does not resolve'? Is that any problems?

@cubxxw
Copy link

cubxxw commented Sep 10, 2024

#8063

This is the problem I encountered,

maybe I understand wrong, this PR can't solve the problem of the video in issue?

@cubxxw
Copy link

cubxxw commented Sep 10, 2024

I understand that the conversation table input is valid for the first time. The message table is used for every subsequent interaction, right?

@Wu-Jiayang
Copy link
Author

#8063

This is the problem I encountered,

maybe I understand wrong, this PR can't solve the problem of the video in issue?

This PR can solve the problem of the video in issue. @cubxxw

2024-09-10.16-19-57.mp4

@Wu-Jiayang
Copy link
Author

I understand that the conversation table input is valid for the first time. The message table is used for every subsequent interaction, right?

right

Copy link
Contributor

@Hisir0909 Hisir0909 left a comment

Choose a reason for hiding this comment

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

Excellent changes, making the workflow much more flexible.

@cubxxw
Copy link

cubxxw commented Oct 17, 2024

I think it's great, too

Hope to support this feature

@liuzixiao666
Copy link

Has this feature not been merged?

@Wu-Jiayang
Copy link
Author

Has this feature not been merged?

I don't know why they don't merge it, maybe we need more people to push them.

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Nov 11, 2024
@liuzixiao666
Copy link

liuzixiao666 commented Nov 11, 2024

Author

I highly appreciate your functionality and have submitted a new issue. If they do not merge, I will download your branch
https://github.com/langgenius/dify/issues/10522
#10522

@crazywoola crazywoola mentioned this pull request Nov 13, 2024
12 tasks
@crazywoola
Copy link
Member

crazywoola commented Nov 13, 2024

I have already talked to our team about this pr. :)
We will evaluate this later.

@crazywoola
Copy link
Member

I have opened a discussion here, please forward this to other community groups, we would like to gather some information regarding this feature.

#10652

@cubxxw
Copy link

cubxxw commented Nov 13, 2024

Hope to merge

@liuzixiao666
Copy link

I tried to merge your code in my project, but it didn't take effect. I modified the three lines of code in those three files and changed them to inputs=self_ Prepare_user_inputs (user_inputs=inputs, app_ config=app_ config), right?

@Wu-Jiayang
Copy link
Author

I tried to merge your code in my project, but it didn't take effect. I modified the three lines of code in those three files and changed them to inputs=self_ Prepare_user_inputs (user_inputs=inputs, app_ config=app_ config), right?

right

@liuzixiao666
Copy link

liuzixiao666 commented Nov 14, 2024

I tried to merge your code in my project, but it didn't take effect. I modified the three lines of code in those three files and changed them to inputs=self_ Prepare_user_inputs (user_inputs=inputs, app_ config=app_ config), right?

right

It seems to have failed? My DIY version is 0.11.0

@liuzixiao666
Copy link

Clipchamp.mp4

@Wu-Jiayang
Copy link
Author

I tried to merge your code in my project, but it didn't take effect. I modified the three lines of code in those three files and changed them to inputs=self_ Prepare_user_inputs (user_inputs=inputs, app_ config=app_ config), right?

right

It seems to have failed? My DIY version is 0.11.0

It works well in 0.8.0. I will try 0.11.0 if I have time

@Wu-Jiayang
Copy link
Author

Wu-Jiayang commented Nov 15, 2024

It seems to have failed? My DIY version is 0.11.0

@liuzixiao666
I merged the 'langgenius:main' branch into my branch and tested it on v0.11.1. It works well.

2024-11-15.17-36-33.mp4

@liuzixiao666
Copy link

It seems to have failed? My DIY version is 0.11.0似乎失败了?我的DIY版本是0.11.0

@liuzixiao666 I merged the 'langgenius:main' branch into my branch and tested it on v0.11.1. It works well.我将"langgenius:main“分支合并到我的分支中,并在v0.11.1上进行了测试。很好用。

2024-11-15.17-36-33.mp4

i will have a try again

@liuzixiao666
Copy link

liuzixiao666 commented Nov 16, 2024

It seems to have failed? My DIY version is 0.11.0

@liuzixiao666 I merged the 'langgenius:main' branch into my branch and tested it on v0.11.1. It works well.

2024-11-15.17-36-33.mp4

Mine still hasn't taken effect. Is it because I started Docker? Are you starting from local source code

@Wu-Jiayang
Copy link
Author

It seems to have failed? My DIY version is 0.11.0

@liuzixiao666 I merged the 'langgenius:main' branch into my branch and tested it on v0.11.1. It works well.
2024-11-15.17-36-33.mp4

Mine still hasn't taken effect. Is it because I started Docker? Are you starting from local source code

I built Docker from local source code and run docker run -d -e MODE=api --restart=unless-stopped --name "dify-api" --network host --env-file .env dify-api

@liuzixiao666
Copy link

It seems to have failed? My DIY version is 0.11.0

@liuzixiao666 I merged the 'langgenius:main' branch into my branch and tested it on v0.11.1. It works well.
2024-11-15.17-36-33.mp4

Mine still hasn't taken effect. Is it because I started Docker? Are you starting from local source code

I built Docker from local source code and run docker run -d -e MODE=api --restart=unless-stopped --name "dify-api" --network host --env-file .env dify-api

I can't quite understand your code, it seems like you have rebuilt a new Docker container. Does that mean that stopping the container and using 'docker compose up - d' will not work when I run it in the Docker directory? Do you know how to make the modified code take effect

@Wu-Jiayang
Copy link
Author

@liuzixiao666
I have read the default docker-compose.yaml and It seems not from local source code:
image

If you want to use docker, you need to make sure the docker you use is built locally, or waiting for merging this PR. ^_^

@TheFu527
Copy link
Contributor

What a great PR! This will solve a lot of problems! Looking forward to merging

@liuzixiao666
Copy link

What a great PR! This will solve a lot of problems! Looking forward to merging

Remember to give a like here
#10652

@HaiShin
Copy link

HaiShin commented Nov 25, 2024

It seems to have failed? My DIY version is 0.11.0

@liuzixiao666 I merged the 'langgenius:main' branch into my branch and tested it on v0.11.1. It works well.
2024-11-15.17-36-33.mp4

Mine still hasn't taken effect. Is it because I started Docker? Are you starting from local source code

I built Docker from local source code and run docker run -d -e MODE=api --restart=unless-stopped --name "dify-api" --network host --env-file .env dify-api

I think your three lines of code will not work, because each conversation does not pass the inputs from the front end. My version is 0.11.2.

@liuzixiao666
Copy link

It seems to have failed? My DIY version is 0.11.0

@liuzixiao666 I merged the 'langgenius:main' branch into my branch and tested it on v0.11.1. It works well.
2024-11-15.17-36-33.mp4

Mine still hasn't taken effect. Is it because I started Docker? Are you starting from local source code

I built Docker from local source code and run docker run -d -e MODE=api --restart=unless-stopped --name "dify-api" --network host --env-file .env dify-api

I think your three lines of code will not work, because each conversation does not pass the inputs from the front end. My version is 0.11.2.

docker,This is an issue with pulling Docker from the cloud

@Wu-Jiayang
Copy link
Author

Wu-Jiayang commented Nov 25, 2024

@HaiShin
I tested my code on v0.11.1. It works well.
I don't know why you said 'each conversation does not pass the inputs from the front end'?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
7 participants