Skip to content

Commit d5e59d9

Browse files
wolfibDevtools-frontend LUCI CQ
authored andcommitted
[Conversation history] Fix serialization of conversations
Currently, `Conversation.serialize()` leaves the `confirm()` function of responses of type side-effect unchanged. Consequently, in `AiHistoryStorage.upsertHistoryEntry()` the `confirm()` function is persisted in a DevTools setting (only in the setting's in-memory copy, the copy persisted to storage is stringified which removes the `confirm()` function). When the setting is later accessed, `structuredClone()` is executed on it, which throws when encountering a function. This bug does not surface in the current implementation of the `AiAssistancePanel` because the setting is only accessed once, in the panel's constructor. Current refactoring efforts will change this, and the conversation serialization needs to be fixed first. Bug: 427407133 Change-Id: Ib620878245530a5086668a414c85bd70b8be31cf Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6827303 Commit-Queue: Alex Rudenko <[email protected]> Auto-Submit: Wolfgang Beyer <[email protected]> Reviewed-by: Alex Rudenko <[email protected]>
1 parent 298735c commit d5e59d9

File tree

4 files changed

+25
-8
lines changed

4 files changed

+25
-8
lines changed

front_end/models/ai_assistance/AiHistoryStorage.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,8 @@ describe('AiHistoryStorage', () => {
510510
assert.lengthOf(imageHistory, 1);
511511
const historyWithoutImages = storage.getHistory();
512512
assert.lengthOf(historyWithoutImages, 2);
513-
const conversationFromHistory = historyWithoutImages.map(item => {
514-
return new AiAssistance.Conversation(item.type, item.history, item.id, true);
515-
});
513+
const conversationFromHistory =
514+
historyWithoutImages.map(item => AiAssistance.Conversation.fromSerializedConversation(item));
516515
assert.lengthOf(conversationFromHistory, 2);
517516
assert.deepEqual(conversationFromHistory[0].history, [{
518517
type: AiAssistance.ResponseType.USER_QUERY,

front_end/models/ai_assistance/AiHistoryStorage.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import * as Common from '../../core/common/common.js';
77

8-
import {type ResponseData, ResponseType} from './agents/AiAgent.js';
8+
import {type ResponseData, ResponseType, type SerializedResponseData} from './agents/AiAgent.js';
99

1010
const MAX_TITLE_LENGTH = 80;
1111

@@ -22,7 +22,7 @@ export const NOT_FOUND_IMAGE_DATA = '';
2222
export interface SerializedConversation {
2323
id: string;
2424
type: ConversationType;
25-
history: ResponseData[];
25+
history: SerializedResponseData[];
2626
isExternal: boolean;
2727
}
2828

@@ -117,12 +117,27 @@ export class Conversation {
117117
if (item.type === ResponseType.USER_QUERY) {
118118
return {...item, imageInput: undefined};
119119
}
120+
// Remove the `confirm()`-function because `structuredClone()` throws on functions
121+
if (item.type === ResponseType.SIDE_EFFECT) {
122+
return {...item, confirm: undefined};
123+
}
120124
return item;
121125
}),
122126
type: this.type,
123127
isExternal: this.#isExternal,
124128
};
125129
}
130+
131+
static fromSerializedConversation(serializedConversation: SerializedConversation): Conversation {
132+
const history = serializedConversation.history.map(entry => {
133+
if (entry.type === ResponseType.SIDE_EFFECT) {
134+
return {...entry, confirm: () => {}};
135+
}
136+
return entry;
137+
});
138+
return new Conversation(
139+
serializedConversation.type, history, serializedConversation.id, true, serializedConversation.isExternal);
140+
}
126141
}
127142

128143
let instance: AiHistoryStorage|null = null;

front_end/models/ai_assistance/agents/AiAgent.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ export interface SideEffectResponse {
8585
code?: string;
8686
confirm: (confirm: boolean) => void;
8787
}
88+
interface SerializedSideEffectResponse extends Omit<SideEffectResponse, 'confirm'> {}
8889

8990
export interface ActionResponse {
9091
type: ResponseType.ACTION;
@@ -107,6 +108,9 @@ export interface UserQuery {
107108
export type ResponseData = AnswerResponse|SuggestionsResponse|ErrorResponse|ActionResponse|SideEffectResponse|
108109
ThoughtResponse|TitleResponse|QueryingResponse|ContextResponse|UserQuery;
109110

111+
export type SerializedResponseData = AnswerResponse|SuggestionsResponse|ErrorResponse|ActionResponse|
112+
SerializedSideEffectResponse|ThoughtResponse|TitleResponse|QueryingResponse|ContextResponse|UserQuery;
113+
110114
export type FunctionCallResponseData =
111115
TitleResponse|ThoughtResponse|ActionResponse|SideEffectResponse|SuggestionsResponse;
112116

front_end/panels/ai_assistance/AiAssistancePanel.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -557,9 +557,8 @@ export class AiAssistancePanel extends UI.Panel.Panel {
557557
accountFullName: syncInfo.accountFullName,
558558
};
559559

560-
this.#historicalConversations = AiAssistanceModel.AiHistoryStorage.instance().getHistory().map(item => {
561-
return new AiAssistanceModel.Conversation(item.type, item.history, item.id, true, item.isExternal);
562-
});
560+
this.#historicalConversations = AiAssistanceModel.AiHistoryStorage.instance().getHistory().map(
561+
serializedConversation => AiAssistanceModel.Conversation.fromSerializedConversation(serializedConversation));
563562

564563
if (UI.ActionRegistry.ActionRegistry.instance().hasAction('elements.toggle-element-search')) {
565564
this.#toggleSearchElementAction =

0 commit comments

Comments
 (0)