Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.

## Unreleased

### A half-ticked box is no longer described to a Bot as ticked

The snapshot a Bot reads before it acts on a page says whether each box is ticked, and Playwright
writes that as `[checked]` for one that is and `[checked=mixed]` for one that is neither — which is
what the "select all" above a partly-ticked list carries. The parser treated any value other than
the string `false` as ticked, and `mixed` is one, so a half-ticked box was reported as done. A Bot
asked to select everything read it as already selected, clicked nothing, and said the rows were
chosen when most of them were not. `mixed` is now reported as not ticked, which is both the true
half of a yes-or-no answer and the one that gets the right action: clicking a half-ticked box ticks
it. An ordinary tick and an ordinary empty box are unchanged.

### A Bot cannot end its turn by asking a person nothing

`ask_person` is how a Bot stops and puts something to a person instead of guessing, and a call with
Expand Down
9 changes: 8 additions & 1 deletion agent-computer/src/aria-snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,15 @@ function toElement(
// Playwright emits `[checked]` only when something is checked, so absence is ambiguous on its own: a
// Bot cannot tell an unticked box from a control that does not tick. Reported as false for the roles
// that can be checked, and left off entirely for the ones that cannot.
//
// `[checked=mixed]` is the third state and the one spelling Playwright gives the flag a value for:
// `aria-checked="mixed"`, which is what the box above a partly-ticked list carries. It is not
// ticked, and a Bot told it was left the rows underneath unselected and reported the job done. The
// contract this fills in is `checked?: boolean`, so "neither" has to be said as false — which is
// also the answer that gets the right action, since clicking a mixed box ticks it.
if (descriptor.flags.has("checked")) {
element.checked = descriptor.flags.get("checked") !== "false";
const state = descriptor.flags.get("checked");
element.checked = state !== "mixed" && state !== "false";
} else if (CHECKABLE_ROLES.has(descriptor.role)) {
element.checked = false;
}
Expand Down
45 changes: 45 additions & 0 deletions agent-computer/tests/aria-snapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,51 @@ describe("parseAriaSnapshot, against captured output", () => {
expect(byName.get("Submit order")).not.toHaveProperty("checked");
});

/**
* A half-ticked "select all", captured the same way the fixture above was.
*
* `ariaSnapshot({ mode: "ai" })` against a fieldset of three boxes in Chromium 151, with the first
* one's `indeterminate` set — which is what a box above a partly-ticked list carries. Playwright
* writes that as `[checked=mixed]`: a value, where an ordinary tick is the bare `[checked]`. It is
* the only flag in this output that carries one, which is the whole of the bug.
*
* Captured rather than written, for the reason the note at the top of this file gives. Guessed at,
* the entry loses the `generic` wrapper each box sits inside and the `text` node beside it, and the
* guess would pass while the shipped parser walked something else.
*/
const MIXED = `- group "Toppings" [ref=e2]:
- generic [ref=e4]:
- checkbox "Select all" [checked=mixed] [ref=e5]
- text: Select all
- generic [ref=e6]:
- checkbox "Bacon" [checked] [ref=e7]
- text: Bacon
- generic [ref=e8]:
- checkbox "Extra Cheese" [ref=e9]
- text: Extra Cheese`;

test("the mixed fixture is genuinely valid YAML", () => {
expect(() => Bun.YAML.parse(MIXED)).not.toThrow();
});

test("a half-ticked box is not reported as ticked", () => {
const byName = new Map(
parseAriaSnapshot(MIXED).elements.map((e) => [e.name, e]),
);
// Not checked, so a Bot asked to tick it clicks it. Told it was already checked, it left the
// rows underneath unselected and said they were done.
expect(byName.get("Select all")?.checked).toBe(false);
// And the two beside it are unchanged, so this is not a swap.
expect(byName.get("Bacon")?.checked).toBe(true);
expect(byName.get("Extra Cheese")?.checked).toBe(false);
});

test("a half-ticked box is still a control a Bot can act on", () => {
const [first] = parseAriaSnapshot(MIXED).elements;
// The ref is what a click needs, and it sits after the flag Playwright gave a value to.
expect(first).toMatchObject({ ref: "e5", role: "checkbox" });
});

test("empty and unparseable input produce no elements rather than throwing", () => {
expect(parseAriaSnapshot("").elements).toEqual([]);
expect(parseAriaSnapshot("\t- [[[ not yaml").elements).toEqual([]);
Expand Down