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

Update Chat Model Serialization #1226

Merged
merged 5 commits into from May 12, 2023
Merged

Conversation

vowelparrot
Copy link
Contributor

Unify format with the python lib, which was defined of yore as

def _message_to_dict(message: BaseMessage) -> dict:
    return {"type": message.type, "data": message.dict()}


def messages_to_dict(messages: List[BaseMessage]) -> List[dict]:
    return [_message_to_dict(m) for m in messages]


def _message_from_dict(message: dict) -> BaseMessage:
    _type = message["type"]
    if _type == "human":
        return HumanMessage(**message["data"])
    elif _type == "ai":
        return AIMessage(**message["data"])
    elif _type == "system":
        return SystemMessage(**message["data"])
    elif _type == "chat":
        return ChatMessage(**message["data"])
    else:
        raise ValueError(f"Got unexpected type: {_type}")


def messages_from_dict(messages: List[dict]) -> List[BaseMessage]:
    return [_message_from_dict(m) for m in messages]

@vercel
Copy link

vercel bot commented May 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
langchainjs-docs ✅ Ready (Inspect) Visit Preview May 12, 2023 10:41pm

@vowelparrot vowelparrot force-pushed the vwp/chat_message_serialization branch from bffc5d4 to dbcfa2d Compare May 11, 2023 19:18
@vowelparrot vowelparrot force-pushed the vwp/chat_message_serialization branch from dbcfa2d to 5c7efd3 Compare May 11, 2023 19:24
@vowelparrot vowelparrot changed the title Update Chat Model Serialization [Breaking] Update Chat Model Serialization May 11, 2023
export interface StoredMessage {
type: string;
role: string | undefined;
text: string;
data: StoredMessageData;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do feel like I'd rather shim this, mark it, then remove the shim when 1.0.0 comes out as a breaking change - I know all this is recent but have seen a few people using it already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jacoblee93 thoughts on the mapV1MessageToStoredMessage ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm for it! Just would add a comment saying to remove when ready to make a breaking change so we don't forget about it

@vowelparrot vowelparrot requested a review from nfcampos May 12, 2023 06:55
@vowelparrot vowelparrot force-pushed the vwp/chat_message_serialization branch from c5deea2 to a787a3a Compare May 12, 2023 21:07
@vowelparrot vowelparrot changed the title [Breaking] Update Chat Model Serialization Update Chat Model Serialization May 12, 2023
export function mapStoredMessagesToChatMessages(
messages: StoredMessage[]
): BaseChatMessage[] {
return messages.map((message) => {
switch (message.type) {
const stored_msg = mapV1MessageToStoredMessage(message);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Camel case to match JS variable name style

export function mapV1MessageToStoredMessage(
message: StoredMessage | StoredMessageV1
): StoredMessage {
if ("data" in message) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be some weird edge case where message has the data property set but is undefined - the more idiomatic way to do this in TS is (weirdly):

if (message.data !== undefined) {
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool thank you!

@vowelparrot vowelparrot force-pushed the vwp/chat_message_serialization branch from 7069413 to 86b9091 Compare May 12, 2023 22:35
@vowelparrot vowelparrot merged commit f14fe19 into main May 12, 2023
10 checks passed
@vowelparrot vowelparrot deleted the vwp/chat_message_serialization branch May 12, 2023 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants