Assistant v2: conversations types #1227
Conversation
| @@ -0,0 +1,119 @@ | |||
| import { ModelId } from "@app/lib/models"; | |||
|
|
|||
| import { UserProviderType, UserType } from "../user"; | |||
There was a problem hiding this comment.
fixing in another PR 👍
| id: ModelId; | ||
| sId: string; | ||
| status: "visible" | "deleted"; | ||
| parentMessageId: string; |
There was a problem hiding this comment.
Wouldn't parentId be enough here ?
There was a problem hiding this comment.
this is a Message sId so that's my motivation for having messageId visible to follow convention... will leave it as is for now
| }; | ||
|
|
||
| export type AssistantUserMention = { | ||
| provider: UserProviderType; |
There was a problem hiding this comment.
Why don't we point to a front.users.id field?
There was a problem hiding this comment.
So far ModelId is only used as a convenience on some object for us, and the invariant is that we should be able to switch data store transparently. The real business logic pointer to a user is its provider and providerId, we could add a shortId to circumvent that, but I'm quite bearish on relying entirely on the SQL ID.
There was a problem hiding this comment.
Ok - I would be curious to know why you don't want to rely on this SQL id but we can have that conversation later.
I am fine with this rational.
| sId: string; | ||
| title?: string; | ||
| participants: UserType[]; | ||
| content: (AssistantUserMessageType[] | AssistantAgentMessageType[])[]; |
There was a problem hiding this comment.
We are going to need some magic flag to know what type it is here at runtime.
Otherwise we would need a class if we want to do it without any magic flag.
Just mentioning it :)
There was a problem hiding this comment.
Yeah we'll likely iterate on that if this is too much of an issue. I presume here that we can use a isAssistantUserMessage and isAssistantAgentMessage functions similar to Result, but maybe I'm wrong?
There was a problem hiding this comment.
We can use functions like this to do type assertion, but we'll need to rely on some known properties of the type and set it for each object of these types we instantiate. Exactly the same way we do it with __is_dust_error here:
const workflowError: WorkflowError = {
type: "transient_upstream_activity_error",
message:
"Received unexpected undefined replies from Slack API in getMessagesForChannel (generally transient)",
__is_dust_error: true,
};
There was a problem hiding this comment.
Yep. I thiiiink this should be easy-ish we'll have clear invariants that separate the two (we can add a string to the type to name it and use that as a flag also)?
Sounds good to you?
Introduces conversations types for Assistant v2.
See https://www.notion.so/dust-tt/DesignDoc-Assistant-V2-Architecture-acb11f35708c49b1bba4f0294d98daf5 for context
@dust-tt/engineering-team please review thoroughly