-
Notifications
You must be signed in to change notification settings - Fork 121
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
[front] Edit messages and conversations branching #10771
base: main
Are you sure you want to change the base?
Conversation
|
front/components/assistant/conversation/ConversationContainer.tsx
Outdated
Show resolved
Hide resolved
front/components/assistant/conversation/input_bar/InputBarContainer.tsx
Outdated
Show resolved
Hide resolved
front/components/assistant/conversation/ConversationsNavigationProvider.tsx
Outdated
Show resolved
Hide resolved
hooks: false, | ||
silent: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we need those two here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Message has hooks and an updatedAt field, we don't want to call them here ?
ALTER TABLE "public"."messages" ADD COLUMN "nextThreadVersion" INTEGER; | ||
ALTER TABLE "public"."messages" ADD COLUMN "previousThreadVersion" INTEGER; | ||
ALTER TABLE "public"."messages" ADD COLUMN "threadVersions" INTEGER[] DEFAULT ARRAY[0]::INTEGER[]; | ||
CREATE UNIQUE INDEX CONCURRENTLY "messages_conversation_id_rank_version_parent_id" ON "messages" ("conversationId", "rank", "version", "parentId"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, CREATE UNIQUE INDEX CONCURRENTLY
will resolve before the index is ready, we might want to avoid deleting the existing one until this one is not fully ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This index won't be fully leverage (at least on in the legacy edit message path to find the "newest" message since version
is not in the query anymore.
front/pages/api/w/[wId]/assistant/conversations/[cId]/content_fragment/index.ts
Outdated
Show resolved
Hide resolved
where: { | ||
rank: messageRow.rank, | ||
conversationId: conversation.id, | ||
version: messageRow.version + 1, | ||
parentId: messageRow.parentId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we keep using the version here? This would avoid the need to sort afterward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
message is not necessarily the last version - we need to find the last version with this query to get the new message version
ALTER TABLE "public"."messages" ADD COLUMN "nextThreadVersion" INTEGER; | ||
ALTER TABLE "public"."messages" ADD COLUMN "previousThreadVersion" INTEGER; | ||
ALTER TABLE "public"."messages" ADD COLUMN "threadVersions" INTEGER[] DEFAULT ARRAY[0]::INTEGER[]; | ||
CREATE UNIQUE INDEX CONCURRENTLY "messages_conversation_id_rank_version_parent_id" ON "messages" ("conversationId", "rank", "version", "parentId"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This index won't be fully leverage (at least on in the legacy edit message path to find the "newest" message since version
is not in the query anymore.
conversationId: conversation.id, | ||
const lastMessage = await Message.findOne({ | ||
attributes: ["id", "rank"], | ||
where: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do this query pretty often, any chance we can leverage GIN
index (doc) to use a composite index and keep good performance here?
Co-authored-by: Flavien David <[email protected]>
Co-authored-by: Flavien David <[email protected]>
Co-authored-by: Flavien David <[email protected]>
Co-authored-by: Flavien David <[email protected]>
Description
This PR introduces message threading and edit message functionality to conversations, allowing users to edit messages, and navigate through different versions.
Design here : https://www.notion.so/dust-tt/Design-Doc-Edit-message-19928599d94180ccbf83e75f838b4796
Tests
Tested message editing functionality
Verified thread version navigation
Validated UI updates and button interactions
Risk
Deploy Plan
run migration
deploy front