-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Serialize and restore ParsedChatRequest in chat sessions #16736
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
base: master
Are you sure you want to change the base?
Conversation
620ad62 to
f69415f
Compare
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.
High-level feedback:
- The ChatModel is restored via the ChatService. At the moment we already avoid handing in registries into the chat model by restoring registry-related data later, see here and here.
- Could we follow this pattern here too? This would overall be a simpler process and continue with the established pattern, avoiding this registry dependency
|
@sdirix That is a very good point! Thanks for catching that. I refactored the code accordingly to restore the tool requests from the registry in the chat service. |
|
I will review at the latest next week. |
sdirix
left a comment
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.
Works for me and could be merged as is 👍
I have some comments, please have a look.
| if (partData.variableValue !== undefined) { | ||
| varPart.resolution = { | ||
| variable: { | ||
| id: partData.variableId ?? 'restored', |
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.
I think if there is no id we should not restore it? And during storing we should make sure they have an id?
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.
I think the problem lies earlier: variable ids are only available if the variable is resolved during serialization (see packages/ai-chat/src/common/parsed-chat-request.ts). Currently we serialize the variable in any case.
We could think about serializing unresolved variables as text parts but there could be value in knowing this is a variable even if it was not/could not be resolved at the time of serialization.
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.
For now, I moved adding a fallback value of the id to the serialization function.
Implement serialization of ParsedChatRequest data to preserve structured information (agent references, variable references, tool requests) across session persistence and restoration. Previously, only raw text was preserved after session restoration, causing loss of parsed structure data. This affected UI rendering and prevented proper display of agent names, variables, and tool references in restored chat sessions. Main code changes: - Add serialization interfaces for ParsedChatRequest components in chat-model-serialization.ts (SerializableParsedRequestPart, SerializableToolRequest, SerializableResolvedVariable, etc.) - Implement toSerializable() methods on ParsedChatRequestTextPart, ParsedChatRequestVariablePart, ParsedChatRequestFunctionPart, and ParsedChatRequestAgentPart classes - Update MutableChatRequestModel to serialize/deserialize parsedRequest data via toSerializable() and deserializeParsedRequest() methods - Serialize tool functions as id only and restore from the registry. For this, MutableChatModel and MutableChatRequestModel now require the tool registry as constructor parameter - Add comprehensive test coverage for serialization/deserialization of all part types, including edge cases and complex mixed requests
0cedbf0 to
8c3fce2
Compare
|
@sdirix I added serialization of the description. I also removed the 2 changelog commits during rebase: They cancelled each other and caused conflicts. |
What it does
Fixes #16561
Implement serialization of ParsedChatRequest data to preserve structured information (agent references, variable references, tool requests) across session persistence and restoration.
Previously, only raw text was preserved after session restoration, causing loss of parsed structure data. This affected UI rendering and prevented proper display of agent names, variables, and tool references in restored chat sessions.
Main code changes:
16561-pr.webm
How to test
NOTE: While the serialization also supports functions/tool requests, highlighting them does not work yet in general. See #16722Follow-ups
Breaking changes
Attribution
Review checklist
nlsservice (for details, please see the Internationalization/Localization section in the Coding Guidelines)Reminder for reviewers