Skip to content

Commit ddf73fd

Browse files
thymikeethymikee
committed
fix(web): put on-screen text in the snapshot tree instead of losing it
The web capture asked agent-browser for `--compact`, and that flag removes a ref-less element TOGETHER WITH the text under it: on one page the role census goes ListMarker 200 -> 0 and status 1 -> 0, so a `status` region that just announced something never reaches the projection at all. Measured on the page this fix was developed against: `wait text "Hello Zed"` times out on text the user can read off the screen, and `find text`/`get text` cannot answer for it either. The flag's pruning is owned here instead, by a projection that can tell an empty container from the text inside it. What survives the projection is now decided per line. Text the backend issued no ref for becomes a node carrying that text on `value`, in the ref-less shape the layer above already models (see `SnapshotRefMinting`). A string rides on `value` rather than `label` because the backend spells a control's visible string twice — as the control's name and as its own text line — and a second `label` claiming it would refuse every `label="Name"` read as ambiguous. Suppression of a text line is a structural relation read off the tree, not a page-global denylist: a line is a duplicate when it renders inside a `<label>` element whose control reports the string as its name, or sits inside the control that reports it as its name or value. Anything else keeps its own node, which is what preserves the content this capture exists to publish — an announcement that happens to repeat a button's label, or a table cell echoing a field's contents, is content. `extractQuotedText` parses and unescapes an escaped quoted payload; the trailing `: value` form stays raw, because that is what the backend prints and unescaping it would publish a value the page does not hold. Closes #3509
1 parent ad579e0 commit ddf73fd

5 files changed

Lines changed: 618 additions & 38 deletions

File tree

‎packages/platform-web/src/__tests__/agent-browser-snapshot-refless.test.ts‎

Lines changed: 308 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,308 @@
1+
import { describe, expect, test } from 'vitest';
2+
3+
import { normalizeAgentBrowserSnapshot } from '../agent-browser-snapshot.ts';
4+
5+
/**
6+
* Payloads captured from agent-browser 0.27.1 against a real page (`snapshot` with no `--compact`,
7+
* which is what the capture now asks for), so the projection is tested against the wording the
8+
* backend actually produces rather than a hand-written approximation of it.
9+
*/
10+
const FORM_AFTER_SUBMIT = {
11+
snapshot: [
12+
'- heading "Bench form" [level=1, ref=e1]',
13+
'- form',
14+
' - LabelText',
15+
' - StaticText "Name"',
16+
' - textbox "Name" [ref=e2]: Zed',
17+
' - StaticText "Zed"',
18+
' - LabelText',
19+
' - StaticText "Email"',
20+
' - textbox "Email" [ref=e3]',
21+
' - checkbox "I agree" [checked=false, ref=e4]',
22+
' - combobox "Color" [expanded=false, ref=e5]: Red',
23+
' - MenuListPopup',
24+
' - option "Red" [selected, ref=e7]',
25+
' - option "Blue" [ref=e8]',
26+
' - button "Submit" [ref=e6]',
27+
'- status',
28+
' - StaticText "Hello Zed"',
29+
'- list',
30+
].join('\n'),
31+
refs: {
32+
e1: { name: 'Bench form', role: 'heading' },
33+
e2: { name: 'Name', role: 'textbox' },
34+
e3: { name: 'Email', role: 'textbox' },
35+
e4: { name: 'I agree', role: 'checkbox' },
36+
e5: { name: 'Color', role: 'combobox' },
37+
e6: { name: 'Submit', role: 'button' },
38+
e7: { name: 'Red', role: 'option' },
39+
e8: { name: 'Blue', role: 'option' },
40+
},
41+
};
42+
43+
/** The same page before the submit announced anything: the `status` region is there and silent. */
44+
const FORM_BEFORE_SUBMIT = {
45+
snapshot: [
46+
'- heading "Bench form" [level=1, ref=e1]',
47+
'- form',
48+
' - LabelText',
49+
' - StaticText "Name"',
50+
' - textbox "Name" [ref=e2]',
51+
' - button "Submit" [ref=e6]',
52+
'- status',
53+
'- list',
54+
].join('\n'),
55+
refs: {
56+
e1: { name: 'Bench form', role: 'heading' },
57+
e2: { name: 'Name', role: 'textbox' },
58+
e6: { name: 'Submit', role: 'button' },
59+
},
60+
};
61+
62+
const ROLE_VOCABULARY = {
63+
snapshot: [
64+
'- heading "Roles probe" [level=1, ref=e1]',
65+
'- paragraph',
66+
' - StaticText "Paragraph text"',
67+
'- StaticText "Span text"',
68+
'- status',
69+
' - StaticText "Status text"',
70+
'- alert',
71+
' - StaticText "Alert text"',
72+
'- note',
73+
' - StaticText "Note text"',
74+
'- LabelText',
75+
' - StaticText "Label text"',
76+
'- textbox "Label text" [ref=e2]',
77+
'- button "Go" [ref=e3]',
78+
].join('\n'),
79+
refs: {
80+
e1: { name: 'Roles probe', role: 'heading' },
81+
e2: { name: 'Label text', role: 'textbox' },
82+
e3: { name: 'Go', role: 'button' },
83+
},
84+
};
85+
86+
/** One list item per row, each carrying the bullet the renderer draws. */
87+
const LIST_WITH_MARKERS = {
88+
snapshot: [
89+
'- heading "Items" [level=1, ref=e1]',
90+
'- list',
91+
' - listitem',
92+
' - ListMarker "• "',
93+
' - StaticText "First"',
94+
' - listitem',
95+
' - ListMarker "• "',
96+
' - StaticText "Second"',
97+
].join('\n'),
98+
refs: { e1: { name: 'Items', role: 'heading' } },
99+
};
100+
101+
function refLessNodes(result: Awaited<ReturnType<typeof normalizeAgentBrowserSnapshot>>) {
102+
return result.nodes.filter((node) => node.ref === undefined);
103+
}
104+
105+
describe('normalizeAgentBrowserSnapshot ref-less content', () => {
106+
test('publishes the text of a region the backend issued no ref for', async () => {
107+
const result = await normalizeAgentBrowserSnapshot(FORM_AFTER_SUBMIT);
108+
const announcement = refLessNodes(result).filter((node) => node.value === 'Hello Zed');
109+
110+
// The announcement is the reason the capture is taken, and the backend has no ref for it: before,
111+
// it reached no reader at all, because the backend's `--compact` removed the region with it.
112+
expect(announcement.length).toBe(1);
113+
expect(announcement[0]?.type).toBe('StaticText');
114+
expect(announcement[0]?.ref).toBeUndefined();
115+
expect(announcement[0]?.label).toBeUndefined();
116+
});
117+
118+
test('leaves the ref each control carries untouched', async () => {
119+
const result = await normalizeAgentBrowserSnapshot(FORM_AFTER_SUBMIT);
120+
121+
expect(
122+
result.nodes
123+
.filter((node) => node.ref !== undefined)
124+
.map((node) => `${node.type} ${node.ref} ${node.label}`),
125+
).toEqual([
126+
'heading e1 Bench form',
127+
'textbox e2 Name',
128+
'textbox e3 Email',
129+
'checkbox e4 I agree',
130+
'combobox e5 Color',
131+
'option e7 Red',
132+
'option e8 Blue',
133+
'button e6 Submit',
134+
]);
135+
});
136+
137+
test('subsumes a label element text under the control that carries it as its name', async () => {
138+
// The backend prints the same visible string twice: as the `<label>` element's text and as the
139+
// accessible name of the control it labels. Two nodes reporting `Name` would make the ordinary
140+
// `text="Name"` on a form refuse as ambiguous, so the string stays on the node an action can
141+
// address — which is also the answer to "where is this text".
142+
const result = await normalizeAgentBrowserSnapshot(FORM_AFTER_SUBMIT);
143+
const refLess = refLessNodes(result).map((node) => node.value);
144+
145+
expect(refLess).toEqual(['Hello Zed']);
146+
expect(result.nodes.find((node) => node.ref === 'e2')).toMatchObject({
147+
label: 'Name',
148+
value: 'Zed',
149+
});
150+
});
151+
152+
test('carries the text on value, never label', async () => {
153+
const result = await normalizeAgentBrowserSnapshot(ROLE_VOCABULARY);
154+
155+
// `label` is the accessible name a selector resolves to an addressable control; a second node
156+
// claiming it would refuse every `label="Go"` read as ambiguous.
157+
expect(refLessNodes(result).every((node) => node.label === undefined)).toBe(true);
158+
expect(
159+
result.nodes.filter((node) => node.label === 'Label text').map((node) => node.ref),
160+
).toEqual(['e2']);
161+
});
162+
163+
test('keeps the text of every non-empty region the tree reports', async () => {
164+
const result = await normalizeAgentBrowserSnapshot(ROLE_VOCABULARY);
165+
166+
expect(refLessNodes(result).map((node) => `${node.type}=${node.value}`)).toEqual([
167+
'StaticText=Paragraph text',
168+
'StaticText=Span text',
169+
'StaticText=Status text',
170+
'StaticText=Alert text',
171+
'StaticText=Note text',
172+
]);
173+
});
174+
175+
test('drops the empty structural containers compact used to hide', async () => {
176+
const result = await normalizeAgentBrowserSnapshot(FORM_BEFORE_SUBMIT);
177+
const types = result.nodes.map((node) => node.type);
178+
179+
// `form`, `list` and the silent `status` region say nothing the rest of the tree does not say.
180+
expect(types).not.toContain('form');
181+
expect(types).not.toContain('list');
182+
expect(types).not.toContain('status');
183+
expect(refLessNodes(result)).toEqual([]);
184+
expect(result.nodes.map((node) => node.ref)).toEqual(['e1', 'e2', 'e6']);
185+
});
186+
187+
test('drops the list marker and keeps the text of the item it marks', async () => {
188+
const result = await normalizeAgentBrowserSnapshot(LIST_WITH_MARKERS);
189+
190+
expect(result.nodes.map((node) => `${node.type}=${node.value ?? node.label}`)).toEqual([
191+
'heading=Items',
192+
'StaticText=First',
193+
'StaticText=Second',
194+
]);
195+
});
196+
197+
test('re-parents the children of a dropped container to its parent', async () => {
198+
// `list` is dropped and `listitem` with it, so each item's text attaches under the root rather
199+
// than under whichever node happened to be kept last before it.
200+
const result = await normalizeAgentBrowserSnapshot(LIST_WITH_MARKERS);
201+
const byValue = new Map(result.nodes.map((node) => [node.value ?? node.label, node]));
202+
203+
expect(byValue.get('First')?.parentIndex).toBeUndefined();
204+
expect(byValue.get('Second')?.parentIndex).toBeUndefined();
205+
expect(byValue.get('Items')?.parentIndex).toBeUndefined();
206+
});
207+
208+
test('keeps page text that only coincidentally repeats a control name elsewhere', async () => {
209+
// Suppression is a structural relation (see `isTextAlreadyReported`), not a page-global
210+
// denylist: this announcement repeats the SUBMIT button's label and is still the reason a reader
211+
// opened the page. A table cell echoing a field's contents survives for the same reason. A
212+
// page-global "drop what any control reports" rule is the mutation this test fails on.
213+
const result = await normalizeAgentBrowserSnapshot({
214+
snapshot: [
215+
'- form',
216+
' - button "Submitting" [ref=e1]',
217+
'- status',
218+
' - StaticText "Submitting"',
219+
'- table',
220+
' - row',
221+
' - cell',
222+
' - StaticText "Zed"',
223+
].join('\n'),
224+
refs: { e1: { name: 'Submitting', role: 'button' }, e2: { name: 'Zed', role: 'textbox' } },
225+
});
226+
227+
expect(refLessNodes(result).map((node) => node.value)).toEqual(['Submitting', 'Zed']);
228+
});
229+
230+
test('subsumes a control name printed as its own text line inside the control', async () => {
231+
// The backend spells a control's own string twice — as its name and as a text line under it —
232+
// and only the descendant shape is a duplicate of that control.
233+
const result = await normalizeAgentBrowserSnapshot({
234+
snapshot: ['- button "Go" [ref=e1]', ' - StaticText "Go"', '- StaticText "Go"'].join('\n'),
235+
refs: { e1: { name: 'Go', role: 'button' } },
236+
});
237+
238+
expect(refLessNodes(result).map((node) => node.value)).toEqual(['Go']);
239+
});
240+
241+
test('unescapes the quoted name a line carries', async () => {
242+
const result = await normalizeAgentBrowserSnapshot({
243+
snapshot: [
244+
String.raw`- button "He said \"hello\"" [ref=e1]`,
245+
String.raw` - StaticText "He said \"hello\""`,
246+
].join('\n'),
247+
refs: { e1: { name: 'He said "hello"', role: 'button' } },
248+
});
249+
250+
// Stopping at the escaped quote would report `He said \` as the button's name and leave its own
251+
// text line looking like different content.
252+
expect(result.nodes.find((node) => node.ref === 'e1')?.label).toBe('He said "hello"');
253+
expect(refLessNodes(result)).toEqual([]);
254+
});
255+
256+
test('matches a value across the two spellings the backend prints for one string', async () => {
257+
// agent-browser prints a control's value two ways (both captured live): the trailing `: value`
258+
// form carries it RAW and a quoted payload carries it ESCAPED. Those are the same string, and
259+
// the projection must read them that way — unescaping the trailing form as well would turn a
260+
// value that legitimately contains a backslash into a different string.
261+
const result = await normalizeAgentBrowserSnapshot({
262+
snapshot: [
263+
'- textbox "Name" [ref=e2]: He said "hi"',
264+
String.raw` - StaticText "He said \"hi\""`,
265+
].join('\n'),
266+
refs: { e2: { name: 'Name', role: 'textbox' } },
267+
});
268+
269+
expect(result.nodes.find((node) => node.ref === 'e2')?.value).toBe('He said "hi"');
270+
expect(refLessNodes(result)).toEqual([]);
271+
});
272+
273+
test('preserves a backslash in a value the backend printed raw', async () => {
274+
const result = await normalizeAgentBrowserSnapshot({
275+
snapshot: [String.raw`- textbox "Name" [ref=e2]: a\"b`].join('\n'),
276+
refs: { e2: { name: 'Name', role: 'textbox' } },
277+
});
278+
279+
// The page really holds `a\"b` (verified through the backend against a live input), and the
280+
// trailing form is not an escaped payload, so the two characters stay.
281+
expect(result.nodes.find((node) => node.ref === 'e2')?.value).toBe(String.raw`a\"b`);
282+
});
283+
284+
test('attaches geometry only to the nodes the backend can measure', async () => {
285+
const result = await normalizeAgentBrowserSnapshot(FORM_AFTER_SUBMIT, async (ref) =>
286+
ref === 'e2' ? { x: 1, y: 2, width: 10, height: 20 } : undefined,
287+
);
288+
289+
expect(result.nodes.find((node) => node.ref === 'e2')?.rect).toEqual({
290+
x: 1,
291+
y: 2,
292+
width: 10,
293+
height: 20,
294+
});
295+
expect(refLessNodes(result).every((node) => !node.rect)).toBe(true);
296+
});
297+
298+
test('reads no ref from text that merely mentions a ref-shaped word', async () => {
299+
const result = await normalizeAgentBrowserSnapshot({
300+
snapshot: ['- status', ' - StaticText "retry in e3 seconds"'].join('\n'),
301+
refs: {},
302+
});
303+
304+
expect(result.nodes).toHaveLength(1);
305+
expect(result.nodes[0]).toMatchObject({ type: 'StaticText', value: 'retry in e3 seconds' });
306+
expect(result.nodes[0]?.ref).toBeUndefined();
307+
});
308+
});

‎packages/platform-web/src/agent-browser-provider.test.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,6 @@ test('agent-browser provider normalizes snapshot refs, labels, values, and paren
238238
assert.deepEqual(calls[0]?.args, [
239239
'snapshot',
240240
'--interactive',
241-
'--compact',
242241
'--depth',
243242
'4',
244243
'--selector',
@@ -284,7 +283,7 @@ test('agent-browser provider fetches snapshot rects only when requested', async
284283
assert.deepEqual(
285284
calls.map((call) => call.args.slice(0, 3)),
286285
[
287-
['snapshot', '--compact', '--json'],
286+
['snapshot', '--json', '--session'],
288287
['get', 'box', '@e1'],
289288
['get', 'box', '@e2'],
290289
['get', 'box', '@e3'],

‎packages/platform-web/src/agent-browser-provider.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ async function movePointer(
228228
}
229229

230230
async function captureAgentBrowserSnapshot(
231-
runJson: (args: string[]) => Promise<unknown>,
231+
runJson: (args: string[], signal?: AbortSignal) => Promise<unknown>,
232232
options: WebSnapshotOptions | undefined,
233233
): Promise<WebSnapshotResult> {
234234
const data = await runJson(buildSnapshotArgs(options));
@@ -238,18 +238,23 @@ async function captureAgentBrowserSnapshot(
238238
);
239239
}
240240

241+
/**
242+
* The backend's own `--compact` is deliberately not requested: it removes an element with no ref
243+
* TOGETHER WITH the text under it, so a status region that just announced something never reaches
244+
* the projection at all. The empty structural lines it existed to hide are dropped by
245+
* `parseSnapshotDraftNodes` instead, which can tell an empty container from the text inside it.
246+
*/
241247
function buildSnapshotArgs(options: WebSnapshotOptions | undefined): string[] {
242248
return [
243249
'snapshot',
244250
...(options?.interactiveOnly ? ['--interactive'] : []),
245-
...(options?.raw ? [] : ['--compact']),
246251
...(typeof options?.depth === 'number' ? ['--depth', String(options.depth)] : []),
247252
...(options?.scope ? ['--selector', options.scope] : []),
248253
];
249254
}
250255

251256
async function fetchRefRect(
252-
runJson: (args: string[]) => Promise<unknown>,
257+
runJson: (args: string[], signal?: AbortSignal) => Promise<unknown>,
253258
ref: string,
254259
): Promise<Rect | undefined> {
255260
try {

0 commit comments

Comments
 (0)