Agent replies without thinking aloud or tool names; chart-links E2E names its failure (#199)
A run of the real prompt against the local model showed reasoning written as chat text, internal tool names in a capabilities answer, and only a data fetch when asked to build a tool. The working principles now say that everything written is shown, to describe abilities in plain words, and that a request for a tool means building one. The chart-links spec now waits for the chart or its error and reports browser errors, so the recurring 90-second timeout names its cause. The ZIP upload spec matches the file chip exactly, since the agent reply can quote the name.
This commit is contained in:
parent
f3b47cb7cd
commit
42eb420545
@ -1,15 +1,47 @@
|
|||||||
import { expect, test } from "@playwright/test";
|
import { expect, type Locator, type Page, test } from "@playwright/test";
|
||||||
import { captureScreenshot } from "./helpers";
|
import { captureScreenshot } from "./helpers";
|
||||||
|
|
||||||
const fixture = "/e2e/fixtures/chart-links.html";
|
const fixture = "/e2e/fixtures/chart-links.html";
|
||||||
|
|
||||||
|
/** Browser errors per page, so a chart that never renders fails with its cause. */
|
||||||
|
const browserErrors = new WeakMap<Page, string[]>();
|
||||||
|
|
||||||
|
test.beforeEach(({ page }) => {
|
||||||
|
const errors: string[] = [];
|
||||||
|
browserErrors.set(page, errors);
|
||||||
|
page.on("pageerror", (error) => errors.push(error.message));
|
||||||
|
page.on("console", (message) => {
|
||||||
|
if (message.type() === "error") errors.push(message.text());
|
||||||
|
});
|
||||||
|
page.on("response", (response) => {
|
||||||
|
if (response.status() >= 400) errors.push(`${response.status()} ${response.url()}`);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
/** Waits for the plotted dot, failing fast with the chart error and browser errors instead. */
|
||||||
|
async function expectChart(page: Page, scope: Locator | Page = page) {
|
||||||
|
const circle = scope.locator("svg circle").first();
|
||||||
|
const failure = scope.getByText("Chart failed to render", { exact: false }).first();
|
||||||
|
try {
|
||||||
|
// Every navigation renders Plot again, which takes seconds on a loaded CI runner.
|
||||||
|
await expect(circle.or(failure).first()).toBeVisible({ timeout: 90_000 });
|
||||||
|
if (await failure.isVisible()) throw new Error(await failure.innerText());
|
||||||
|
} catch (error) {
|
||||||
|
const errors = browserErrors.get(page) ?? [];
|
||||||
|
throw new Error(
|
||||||
|
`${error instanceof Error ? error.message : String(error)}\nBrowser errors:\n${errors.join("\n") || "none"}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return circle;
|
||||||
|
}
|
||||||
|
|
||||||
test.describe
|
test.describe
|
||||||
.serial("chart links", () => {
|
.serial("chart links", () => {
|
||||||
test("cannot execute script when clicked or expanded", async ({ page }, testInfo) => {
|
test("cannot execute script when clicked or expanded", async ({ page }, testInfo) => {
|
||||||
test.setTimeout(120_000);
|
test.setTimeout(120_000);
|
||||||
// ChartCanvas lazy-loads Observable Plot; pay the import cost once before the matrix below.
|
// ChartCanvas lazy-loads Observable Plot; pay the import cost once before the matrix below.
|
||||||
await page.goto(`${fixture}?${new URLSearchParams({ href: "#details", inspect: "0" })}`);
|
await page.goto(`${fixture}?${new URLSearchParams({ href: "#details", inspect: "0" })}`);
|
||||||
await expect(page.locator("svg circle").first()).toBeVisible({ timeout: 90_000 });
|
await expectChart(page);
|
||||||
for (const inspect of ["0", "1"]) {
|
for (const inspect of ["0", "1"]) {
|
||||||
for (const href of [
|
for (const href of [
|
||||||
'javascript:void(document.body.dataset.plotExecuted="yes")',
|
'javascript:void(document.body.dataset.plotExecuted="yes")',
|
||||||
@ -19,9 +51,7 @@ test.describe
|
|||||||
"file:///example.txt",
|
"file:///example.txt",
|
||||||
]) {
|
]) {
|
||||||
await page.goto(`${fixture}?${new URLSearchParams({ href, inspect })}`);
|
await page.goto(`${fixture}?${new URLSearchParams({ href, inspect })}`);
|
||||||
const circle = page.locator("svg circle").first();
|
const circle = await expectChart(page);
|
||||||
// Every navigation renders Plot again, which takes seconds on a loaded CI runner.
|
|
||||||
await expect(circle).toBeVisible({ timeout: 90_000 });
|
|
||||||
const assertInert = async () => {
|
const assertInert = async () => {
|
||||||
const links = page.locator("svg a");
|
const links = page.locator("svg a");
|
||||||
expect(await links.count()).toBeGreaterThan(0);
|
expect(await links.count()).toBeGreaterThan(0);
|
||||||
@ -43,7 +73,7 @@ test.describe
|
|||||||
await assertInert();
|
await assertInert();
|
||||||
await page.getByRole("button", { name: "Expand", exact: true }).click();
|
await page.getByRole("button", { name: "Expand", exact: true }).click();
|
||||||
const dialog = page.getByRole("dialog");
|
const dialog = page.getByRole("dialog");
|
||||||
await expect(dialog.locator("svg circle")).toBeVisible({ timeout: 90_000 });
|
await expectChart(page, dialog);
|
||||||
await dialog.locator("svg circle").click();
|
await dialog.locator("svg circle").click();
|
||||||
await assertInert();
|
await assertInert();
|
||||||
await expect(page).toHaveURL(`${fixture}?${new URLSearchParams({ href, inspect })}`);
|
await expect(page).toHaveURL(`${fixture}?${new URLSearchParams({ href, inspect })}`);
|
||||||
@ -62,7 +92,7 @@ test.describe
|
|||||||
route.fulfill({ body: "Chart destination" }),
|
route.fulfill({ body: "Chart destination" }),
|
||||||
);
|
);
|
||||||
await page.goto(`${fixture}?${new URLSearchParams({ href: "#details" })}`);
|
await page.goto(`${fixture}?${new URLSearchParams({ href: "#details" })}`);
|
||||||
await expect(page.locator("svg circle").first()).toBeVisible({ timeout: 90_000 });
|
await expectChart(page);
|
||||||
for (const href of [
|
for (const href of [
|
||||||
"https://example.test/chart?a=1&b=2",
|
"https://example.test/chart?a=1&b=2",
|
||||||
"mailto:chart@example.test",
|
"mailto:chart@example.test",
|
||||||
@ -73,7 +103,7 @@ test.describe
|
|||||||
"java%73cript:void(0)",
|
"java%73cript:void(0)",
|
||||||
]) {
|
]) {
|
||||||
await page.goto(`${fixture}?${new URLSearchParams({ href })}`);
|
await page.goto(`${fixture}?${new URLSearchParams({ href })}`);
|
||||||
await expect(page.locator("svg circle").first()).toBeVisible({ timeout: 90_000 });
|
await expectChart(page);
|
||||||
const link = page.locator("svg a").first();
|
const link = page.locator("svg a").first();
|
||||||
await expect(link).toBeAttached();
|
await expect(link).toBeAttached();
|
||||||
expect(
|
expect(
|
||||||
|
|||||||
@ -104,7 +104,8 @@ test("a repository ZIP above the inline cap streams up as a file", async ({ page
|
|||||||
const response = await upload;
|
const response = await upload;
|
||||||
expect(response.status()).toBe(201);
|
expect(response.status()).toBe(201);
|
||||||
expect(response.request().headers()["content-type"]).toBe("application/zip");
|
expect(response.request().headers()["content-type"]).toBe("application/zip");
|
||||||
await expect(page.getByTestId("transcript").getByText("repo.zip")).toBeVisible({
|
// Exact: the agent's reply can quote the file name too.
|
||||||
|
await expect(page.getByTestId("transcript").getByText("repo.zip", { exact: true })).toBeVisible({
|
||||||
timeout: 30_000,
|
timeout: 30_000,
|
||||||
});
|
});
|
||||||
await captureScreenshot(page, testInfo, "zip-upload");
|
await captureScreenshot(page, testInfo, "zip-upload");
|
||||||
|
|||||||
@ -359,6 +359,8 @@ describe("run instructions", () => {
|
|||||||
);
|
);
|
||||||
expect(instructions).toContain("Say something is done only after a tool confirmed it");
|
expect(instructions).toContain("Say something is done only after a tool confirmed it");
|
||||||
expect(instructions).toContain("Reply in the language the user writes in.");
|
expect(instructions).toContain("Reply in the language the user writes in.");
|
||||||
|
expect(instructions).toContain("do not think aloud");
|
||||||
|
expect(instructions).toContain("never by tool names or ids");
|
||||||
expect(instructions).toContain("No apps are connected yet.");
|
expect(instructions).toContain("No apps are connected yet.");
|
||||||
expect(instructions).not.toContain("A schedule started this run");
|
expect(instructions).not.toContain("A schedule started this run");
|
||||||
});
|
});
|
||||||
|
|||||||
@ -55,10 +55,11 @@ export function formatWorkingPrinciples(toolNames: ReadonlySet<string>): string
|
|||||||
: undefined,
|
: undefined,
|
||||||
"- Before saying you cannot do something, check your tools, the connected apps and the skills listed here.",
|
"- Before saying you cannot do something, check your tools, the connected apps and the skills listed here.",
|
||||||
canBuildTools
|
canBuildTools
|
||||||
? "- When no tool does what the task needs, build one: write a script in your workspace, test it, and save how to run it as a skill with skill_create so you and the user can reuse it."
|
? "- When the user asks for a tool, or none of yours does what the task needs, build one: write a script in your workspace, test it, and save how to run it as a skill with skill_create so you and the user can reuse it."
|
||||||
: undefined,
|
: undefined,
|
||||||
"- Ask only when the answer changes the outcome and there is no sensible default; otherwise pick the default and say which.",
|
"- Ask only when the answer changes the outcome and there is no sensible default; otherwise pick the default and say which.",
|
||||||
"- Reply in the language the user writes in. Lead with the result and keep it short.",
|
"- Reply in the language the user writes in. Lead with the result and keep it short.",
|
||||||
|
"- Everything you write is shown to the user: do not think aloud, and before a tool call write at most one short sentence. Describe what you can do in plain words, never by tool names or ids.",
|
||||||
]
|
]
|
||||||
.filter(Boolean)
|
.filter(Boolean)
|
||||||
.join("\n");
|
.join("\n");
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user