Skip to content

Commit 9295dfc

Browse files
ergunshDevtools-frontend LUCI CQ
authored andcommitted
[AiAssistance] Do not save partial responses to history
Previously, we were managing the history inside the agent and was deciding what to add to history from the yielded results in there. This is changed in crrev.com/c/6239203 where it introduced `Conversation` (for history feature) separate from the agent's history and started saving all the yielded results to conversation history, including the partial ones. This CL updates AiAssistancePanel to save only complete responses to conversation history. Fixed: 396610180 Change-Id: I615156568336aac93346fcbcdec158b3c4d5ad9b Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6268783 Reviewed-by: Alex Rudenko <[email protected]> Commit-Queue: Alex Rudenko <[email protected]> Commit-Queue: Ergün Erdoğmuş <[email protected]> Auto-Submit: Ergün Erdoğmuş <[email protected]>
1 parent 453deeb commit 9295dfc

File tree

8 files changed

+44
-4
lines changed

8 files changed

+44
-4
lines changed

front_end/panels/ai_assistance/AiAssistancePanel.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,29 @@ describeWithMockConnection('AI Assistance Panel', () => {
557557
},
558558
]);
559559
});
560+
561+
it('should not save partial responses to conversation history', async () => {
562+
updateHostConfig({
563+
devToolsFreestyler: {
564+
enabled: true,
565+
},
566+
});
567+
const addHistoryItemStub = sinon.stub(AiAssistance.Conversation.prototype, 'addHistoryItem');
568+
UI.Context.Context.instance().setFlavor(
569+
ElementsPanel.ElementsPanel.ElementsPanel,
570+
sinon.createStubInstance(ElementsPanel.ElementsPanel.ElementsPanel));
571+
const {view} = await createAiAssistancePanel({
572+
aidaClient: mockAidaClient([[
573+
{explanation: 'ANSWER: partially started'}, {explanation: 'ANSWER: partially started and now it\'s finished'}
574+
]])
575+
});
576+
// Trigger running the conversation (observe that there are two answers: one partial, one complete)
577+
await view.lastCall.args[0].onTextSubmit('User question to Freestyler?');
578+
579+
sinon.assert.calledWith(
580+
addHistoryItemStub, sinon.match({type: 'answer', text: 'partially started and now it\'s finished'}));
581+
sinon.assert.neverCalledWith(addHistoryItemStub, sinon.match({type: 'answer', text: 'partially started'}));
582+
});
560583
});
561584

562585
it('should have empty state after clear chat', async () => {

front_end/panels/ai_assistance/AiAssistancePanel.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1004,7 +1004,10 @@ Output one filename per line and nothing else!
10041004
#saveResponsesToCurrentConversation(items: AsyncIterable<ResponseData, void, void>):
10051005
AsyncGenerator<ResponseData, void, void> {
10061006
for await (const data of items) {
1007-
this.#currentConversation?.addHistoryItem(data);
1007+
// We don't want to save partial responses to the conversation history.
1008+
if (data.type !== ResponseType.ANSWER || data.complete) {
1009+
this.#currentConversation?.addHistoryItem(data);
1010+
}
10081011
yield data;
10091012
}
10101013
}

front_end/panels/ai_assistance/agents/AiAgent.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,13 @@ describeWithEnvironment('AiAgent', () => {
231231
},
232232
{
233233
type: ResponseType.ANSWER,
234+
complete: false,
234235
text: 'Partial ans',
235236
},
236237
{
237238
type: ResponseType.ANSWER,
238239
text: 'Partial answer is now completed',
240+
complete: true,
239241
rpcId: undefined,
240242
suggestions: undefined,
241243
},

front_end/panels/ai_assistance/agents/AiAgent.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export const enum ErrorType {
2929
export interface AnswerResponse {
3030
type: ResponseType.ANSWER;
3131
text: string;
32+
// Whether this is the complete answer or only a part of it (for streaming reasons)
33+
complete: boolean;
3234
rpcId?: Host.AidaClient.RpcGlobalId;
3335
suggestions?: [string, ...string[]];
3436
}
@@ -398,6 +400,7 @@ export abstract class AiAgent<T> {
398400
yield {
399401
type: ResponseType.ANSWER,
400402
text: parsedResponse.answer,
403+
complete: false,
401404
};
402405
}
403406
}
@@ -429,6 +432,7 @@ export abstract class AiAgent<T> {
429432
type: ResponseType.ANSWER,
430433
text: parsedResponse.answer,
431434
suggestions: parsedResponse.suggestions,
435+
complete: true,
432436
rpcId,
433437
};
434438
break;

front_end/panels/ai_assistance/agents/FileAgent.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ content
174174
{
175175
type: ResponseType.ANSWER,
176176
text: 'This is the answer',
177+
complete: true,
177178
suggestions: undefined,
178179
rpcId: 123,
179180
},

front_end/panels/ai_assistance/agents/NetworkAgent.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ describeWithMockConnection('NetworkAgent', () => {
188188
{
189189
type: ResponseType.ANSWER,
190190
text: 'This is the answer',
191+
complete: true,
191192
suggestions: undefined,
192193
rpcId: 123,
193194
},

front_end/panels/ai_assistance/agents/PerformanceAgent.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ self: 3
174174
{
175175
type: ResponseType.ANSWER,
176176
text: 'This is the answer',
177+
complete: true,
177178
suggestions: undefined,
178179
rpcId: 123,
179180
},

front_end/panels/ai_assistance/agents/StylingAgent.test.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,7 @@ STOP`,
794794
{
795795
type: AiAssistance.ResponseType.ANSWER,
796796
text: 'this is the answer',
797+
complete: true,
797798
suggestions: undefined,
798799
rpcId: undefined,
799800
},
@@ -904,6 +905,7 @@ STOP`,
904905
{
905906
type: AiAssistance.ResponseType.ANSWER,
906907
text: 'this is the answer',
908+
complete: true,
907909
suggestions: undefined,
908910
rpcId: 123,
909911
},
@@ -914,10 +916,10 @@ STOP`,
914916
const agent = new StylingAgent({
915917
aidaClient: mockAidaClient([[
916918
{
917-
explanation: 'ANSWER: this is the answer',
919+
explanation: 'ANSWER: this is the partial answer',
918920
},
919921
{
920-
explanation: 'ANSWER: this is another answer',
922+
explanation: 'ANSWER: this is the partial answer and now it\'s complete',
921923
metadata: {
922924
attributionMetadata: {
923925
attributionAction: Host.AidaClient.RecitationAction.BLOCK,
@@ -950,8 +952,9 @@ STOP`,
950952
type: AiAssistance.ResponseType.QUERYING,
951953
},
952954
{
953-
text: 'this is the answer',
955+
text: 'this is the partial answer',
954956
type: AiAssistance.ResponseType.ANSWER,
957+
complete: false,
955958
},
956959
{
957960
type: AiAssistance.ResponseType.ERROR,
@@ -999,6 +1002,7 @@ STOP`,
9991002
{
10001003
type: AiAssistance.ResponseType.ANSWER,
10011004
text: 'this is the answer',
1005+
complete: true,
10021006
suggestions: undefined,
10031007
rpcId: 123,
10041008
},
@@ -1129,6 +1133,7 @@ STOP
11291133
{
11301134
type: AiAssistance.ResponseType.ANSWER,
11311135
text: 'this is the actual answer',
1136+
complete: true,
11321137
suggestions: undefined,
11331138
rpcId: undefined,
11341139
},

0 commit comments

Comments
 (0)