[Freestyler] Maintain history between inputs and report errors to the model and the user.
Drive-by:
* Updated `ACTION` in preamble to be `INFORMATION_RETRIEVAL` otherwise model is trying to update the page directly when you ask "how".
* Removed `throwOnSideEffects: true` as it is causing more failures
which would make it harder to evaluate the quality.
* Remove `js` from the code's prelude if it exists.
Fixed: 347232987, 347661232, 347661233
Change-Id: Iff94cec78f4445a42becd9b4718c7605325d4c7c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5633604
Commit-Queue: Alex Rudenko <[email protected]>
Commit-Queue: Ergün Erdoğmuş <[email protected]>
Auto-Submit: Ergün Erdoğmuş <[email protected]>
Reviewed-by: Alex Rudenko <[email protected]>
diff --git a/front_end/panels/freestyler/FreestylerAgent.test.ts b/front_end/panels/freestyler/FreestylerAgent.test.ts
index eb9af75..d89411a 100644
--- a/front_end/panels/freestyler/FreestylerAgent.test.ts
+++ b/front_end/panels/freestyler/FreestylerAgent.test.ts
@@ -121,6 +121,17 @@
});
});
+ it('parses an action with 5 backticks in the code and `js` text in the prelude', async () => {
+ const payload = `const data = {
+ someKey: "value",
+}`;
+ assert.deepStrictEqual(FreestylerAgent.parseResponse(`ACTION\n\`\`\`\`\`\njs\n${payload}\n\`\`\`\`\`\nSTOP`), {
+ action: payload,
+ thought: undefined,
+ answer: undefined,
+ });
+ });
+
it('parses a thought and an action', async () => {
const actionPayload = `const data = {
someKey: "value",
diff --git a/front_end/panels/freestyler/FreestylerAgent.ts b/front_end/panels/freestyler/FreestylerAgent.ts
index f30bfe3..44e207a 100644
--- a/front_end/panels/freestyler/FreestylerAgent.ts
+++ b/front_end/panels/freestyler/FreestylerAgent.ts
@@ -7,7 +7,7 @@
import * as SDK from '../../core/sdk/sdk.js';
import * as UI from '../../ui/legacy/legacy.js';
-import {FreestylerEvaluateAction} from './FreestylerEvaluateAction.js';
+import {ExecutionError, FreestylerEvaluateAction} from './FreestylerEvaluateAction.js';
const preamble = `You are a CSS debugging agent integrated into Chrome DevTools.
You are going to receive a query about the CSS on the page and you are going to answer to this query in these steps:
@@ -51,10 +51,11 @@
THOUGHT = 'thought',
ACTION = 'action',
ANSWER = 'answer',
+ ERROR = 'error',
}
export type StepData = {
- step: Step.THOUGHT|Step.ANSWER,
+ step: Step.THOUGHT|Step.ANSWER|Step.ERROR,
text: string,
}|{
step: Step.ACTION,
@@ -83,12 +84,21 @@
throw new Error('Execution context is not found for executing code');
}
- return FreestylerEvaluateAction.execute(code, executionContext);
+ try {
+ return await FreestylerEvaluateAction.execute(code, executionContext);
+ } catch (err) {
+ if (err instanceof ExecutionError) {
+ return `Error: ${err.message}`;
+ }
+
+ throw err;
+ }
}
const MAX_STEPS = 5;
export class FreestylerAgent {
#aidaClient: Host.AidaClient.AidaClient;
+ #chatHistory: {text: string, entity: Host.AidaClient.Entity}[] = [];
constructor({aidaClient}: {aidaClient: Host.AidaClient.AidaClient}) {
this.#aidaClient = aidaClient;
@@ -133,12 +143,15 @@
const actionLines = [];
let j = i + 1;
while (j < lines.length && lines[j].trim() !== 'STOP') {
- actionLines.push(lines[j]);
+ // Sometimes the code block is in the form of "`````\njs\n{code}`````"
+ if (lines[j].trim() !== 'js') {
+ actionLines.push(lines[j]);
+ }
j++;
}
// TODO: perhaps trying to parse with a Markdown parser would
// yield more reliable results.
- action = actionLines.join('\n').replaceAll('```', '').trim();
+ action = actionLines.join('\n').replaceAll('```', '').replaceAll('``', '').trim();
i = j + 1;
} else if (trimmed.startsWith('ANSWER:') && !answer) {
const answerLines = [
@@ -171,20 +184,31 @@
return result ?? '';
}
+ resetHistory(): void {
+ this.#chatHistory = [];
+ }
+
async run(query: string, onStep: (data: StepData) => void): Promise<void> {
const structuredLog = [];
- const chatHistory: Host.AidaClient.Chunk[] = [];
query = `QUERY: ${query}`;
for (let i = 0; i < MAX_STEPS; i++) {
- const request = FreestylerAgent.buildRequest(query, preamble, chatHistory.length ? chatHistory : undefined);
- const response = await this.#aidaFetch(request);
- debugLog(`Iteration: ${i}: ${JSON.stringify(request)}\n${response}`);
+ const request =
+ FreestylerAgent.buildRequest(query, preamble, this.#chatHistory.length ? this.#chatHistory : undefined);
+ let response: string;
+ try {
+ response = await this.#aidaFetch(request);
+ } catch (err) {
+ onStep({step: Step.ERROR, text: err.message});
+ break;
+ }
+
+ debugLog(`Iteration: ${i}`, 'Request', request, 'Response', response);
structuredLog.push({
request: request,
response: response,
});
- chatHistory.push({
+ this.#chatHistory.push({
text: query,
entity: i === 0 ? Host.AidaClient.Entity.USER : Host.AidaClient.Entity.SYSTEM,
});
@@ -198,7 +222,7 @@
if (answer) {
onStep({step: Step.ANSWER, text: answer});
- chatHistory.push({
+ this.#chatHistory.push({
text: `ANSWER: ${answer}`,
entity: Host.AidaClient.Entity.SYSTEM,
});
@@ -207,22 +231,27 @@
if (thought) {
onStep({step: Step.THOUGHT, text: thought});
- chatHistory.push({
+ this.#chatHistory.push({
text: `THOUGHT: ${thought}`,
entity: Host.AidaClient.Entity.SYSTEM,
});
}
if (action) {
- chatHistory.push({
+ this.#chatHistory.push({
text: `ACTION\n${action}\nSTOP`,
entity: Host.AidaClient.Entity.SYSTEM,
});
+ debugLog(`Action to execute: ${action}`);
const observation = await executeJsCode(`{${action};data}`);
- debugLog(`Executed action: ${action}\nResult: ${observation}`);
+ debugLog(`Action result: ${observation}`);
onStep({step: Step.ACTION, code: action, output: observation});
query = `OBSERVATION: ${observation}`;
}
+
+ if (i === MAX_STEPS - 1) {
+ onStep({step: Step.ERROR, text: 'Max steps reached, please try again.'});
+ }
}
if (isDebugMode()) {
localStorage.setItem('freestylerStructuredLog', JSON.stringify(structuredLog));
@@ -235,13 +264,13 @@
return Boolean(localStorage.getItem('debugFreestylerEnabled'));
}
-function debugLog(log: string): void {
+function debugLog(...log: unknown[]): void {
if (!isDebugMode()) {
return;
}
// eslint-disable-next-line no-console
- console.log(log);
+ console.log(...log);
}
function setDebugFreestylerEnabled(enabled: boolean): void {
diff --git a/front_end/panels/freestyler/FreestylerEvaluateAction.test.ts b/front_end/panels/freestyler/FreestylerEvaluateAction.test.ts
index 6770268..670c64b 100644
--- a/front_end/panels/freestyler/FreestylerEvaluateAction.test.ts
+++ b/front_end/panels/freestyler/FreestylerEvaluateAction.test.ts
@@ -71,10 +71,8 @@
return getExecutionContext(runtimeModel!);
}
- async function executeForTest(
- code: string, {allowSideEffectForTest = false}: {allowSideEffectForTest?: boolean} = {}) {
- return Freestyler.FreestylerEvaluateAction.execute(
- code, await executionContextForTest(), {allowSideEffectForTest});
+ async function executeForTest(code: string) {
+ return Freestyler.FreestylerEvaluateAction.execute(code, await executionContextForTest());
}
it('should serialize primitive values correctly', async () => {
@@ -88,13 +86,11 @@
it('should serialize DOM nodes correctly', async () => {
assert.strictEqual(
- await executeForTest(
- `{
+ await executeForTest(`{
const div = document.createElement('div');
div.setAttribute('data-custom-attr', 'i exist');
div
- }`,
- {allowSideEffectForTest: true}),
+ }`),
'"<div data-custom-attr=\\"i exist\\"></div>"');
});
diff --git a/front_end/panels/freestyler/FreestylerEvaluateAction.ts b/front_end/panels/freestyler/FreestylerEvaluateAction.ts
index d37dcf7..4895f0f 100644
--- a/front_end/panels/freestyler/FreestylerEvaluateAction.ts
+++ b/front_end/panels/freestyler/FreestylerEvaluateAction.ts
@@ -67,9 +67,7 @@
}
export class FreestylerEvaluateAction {
- static async execute(code: string, executionContext: SDK.RuntimeModel.ExecutionContext, options: {
- allowSideEffectForTest?: boolean,
- } = {}): Promise<string> {
+ static async execute(code: string, executionContext: SDK.RuntimeModel.ExecutionContext): Promise<string> {
const response = await executionContext.evaluate(
{
expression: code,
@@ -79,7 +77,6 @@
silent: false,
generatePreview: true,
allowUnsafeEvalBlockedByCSP: false,
- throwOnSideEffect: options.allowSideEffectForTest ? false : true,
},
/* userGesture */ false, /* awaitPromise */ true);
diff --git a/front_end/panels/freestyler/FreestylerPanel.ts b/front_end/panels/freestyler/FreestylerPanel.ts
index b972bbc..e54ded8 100644
--- a/front_end/panels/freestyler/FreestylerPanel.ts
+++ b/front_end/panels/freestyler/FreestylerPanel.ts
@@ -83,7 +83,7 @@
private constructor(private view: View = defaultView) {
super(FreestylerPanel.panelName);
- createToolbar(this.contentElement, {onClearClick: this.#handleClearClick.bind(this)});
+ createToolbar(this.contentElement, {onClearClick: this.#clearMessages.bind(this)});
this.#toggleSearchElementAction =
UI.ActionRegistry.ActionRegistry.instance().getAction('elements.toggle-element-search');
this.#aidaClient = new Host.AidaClient.AidaClient();
@@ -105,7 +105,13 @@
});
UI.Context.Context.instance().addFlavorChangeListener(SDK.DOMModel.DOMNode, ev => {
+ if (this.#viewProps.selectedNode === ev.data) {
+ return;
+ }
+
this.#viewProps.selectedNode = ev.data;
+ this.#agent.resetHistory();
+ this.#clearMessages();
this.doUpdate();
});
this.doUpdate();
@@ -139,21 +145,22 @@
switch (actionId) {
case 'freestyler.element-panel-context': {
// TODO(340805362): Add UMA
- this.#handleClearClick();
+ this.#clearMessages();
break;
}
case 'freestyler.style-tab-context': {
// TODO(340805362): Add UMA
- this.#handleClearClick();
+ this.#clearMessages();
break;
}
}
}
// TODO(ergunsh): Handle cancelling agent run.
- #handleClearClick(): void {
+ #clearMessages(): void {
this.#viewProps.messages = [];
this.#viewProps.state = FreestylerChatUiState.CHAT_VIEW;
+ this.#agent.resetHistory();
this.doUpdate();
}
diff --git a/front_end/panels/freestyler/components/FreestylerChatUi.ts b/front_end/panels/freestyler/components/FreestylerChatUi.ts
index 2d8d51d..1f4bfd2 100644
--- a/front_end/panels/freestyler/components/FreestylerChatUi.ts
+++ b/front_end/panels/freestyler/components/FreestylerChatUi.ts
@@ -106,15 +106,23 @@
input.value = '';
};
- #renderActionResult = ({code, output}: {code: string, output: string}): LitHtml.TemplateResult => {
- return LitHtml.html`
- <div class="action-result">
- <${MarkdownView.CodeBlock.CodeBlock.litTagName} .code=${code.trim()} .codeLang="js" .displayToolbar=${
- false}></${MarkdownView.CodeBlock.CodeBlock.litTagName}>
- <div class="js-code-output">${output}</div>
- </div>
- `;
- };
+ #renderStep(step: StepData): LitHtml.TemplateResult {
+ if (step.step === Step.ACTION) {
+ return LitHtml.html`
+ <div class="action-result">
+ <${MarkdownView.CodeBlock.CodeBlock.litTagName} .code=${step.code.trim()} .codeLang="js" .displayToolbar=${
+ false}></${MarkdownView.CodeBlock.CodeBlock.litTagName}>
+ <div class="js-code-output">${step.output}</div>
+ </div>
+ `;
+ }
+
+ if (step.step === Step.ERROR) {
+ return LitHtml.html`<p class="error-step">${step.text}</p>`;
+ }
+
+ return LitHtml.html`<p>${step.text}</p>`;
+ }
#renderChatMessage = (message: ChatMessage): LitHtml.TemplateResult => {
if (message.entity === ChatMessageEntity.USER) {
@@ -125,7 +133,7 @@
// clang-format off
return LitHtml.html`
<div class="chat-message answer">
- ${steps.map(step => LitHtml.html`${step.step === Step.ACTION ? this.#renderActionResult(step) : LitHtml.html`<p>${step.text}</p>`}`)}
+ ${steps.map(step => this.#renderStep(step))}
</div>
`;
// clang-format on
diff --git a/front_end/panels/freestyler/components/freestylerChatUi.css b/front_end/panels/freestyler/components/freestylerChatUi.css
index 2650cdf..2e6190e 100644
--- a/front_end/panels/freestyler/components/freestylerChatUi.css
+++ b/front_end/panels/freestyler/components/freestylerChatUi.css
@@ -139,3 +139,7 @@
font-size: 10px;
font-family: var(--source-code-font-family);
}
+
+.error-step {
+ color: var(--sys-color-error);
+}