Skip to content

Commit cdd1b9d

Browse files
Hotragnclaudedavidmckayv
authored
Cut a long page's text between characters too, not only a control's name (#578)
#539 fixed this for a control's accessible name and value in a page snapshot and left the readable page text a navigation returns, which is the same bug at thirty times the length. `slice` counts UTF-16 code units, an emoji is two, and a limit landing between the halves leaves a lone high surrogate as the last thing the Bot is handed. Measured at the real limit on main: main : last unit d83d JSON tail "aaa\ud83d" UTF-8 tail efbfbd length 6000 branch : last unit 61 JSON tail "aaaa" UTF-8 tail 61 length 5999 `efbfbd` is U+FFFD, so the Bot reads a broken character that is not on the page, in text it is about to answer a question from. More likely to bite here than in a control's name, for the reason the limit is bigger. A 200-unit name rarely reaches an emoji; 6000 units of somebody's page usually passes through several, and whether the cut lands mid-character is decided by whatever happened to be above it. The rule was already written and already correct — it just was not being called. `cutAtCodeUnits` is exported rather than duplicated, and `index.ts` already imported from `aria-snapshot.ts`, so this is one word and one call. It stays declared in the parser module because that module imports no Playwright: `index.ts` does, at load, so a helper declared beside its caller could not be tested at all, which is why the caller was left out of #539 in the first place. `truncated` is unchanged and needs no thought: dropping one more code unit cannot make an over-limit string fit. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: David McKay <david@copilotkit.ai>
1 parent ab9c493 commit cdd1b9d

4 files changed

Lines changed: 75 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
88

99
## Unreleased
1010

11+
### A long page's text is cut between characters, not through one
12+
13+
A navigation hands the Bot the first 6000 UTF-16 code units of the page's readable text. When that
14+
limit fell between the two halves of an emoji, the Bot was handed text ending on half a character,
15+
which reads as U+FFFD: a broken character that is not on the page. It now stops one code unit short
16+
in that case, which is what a control's name and value in a page snapshot already did.
17+
1118
### Tool selection reads a skill choice the model wrapped in a code fence
1219

1320
Before a run, the deployment's model picks which of a Bot's skills the message needs, so a Bot holding

agent-computer/src/aria-snapshot.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,12 @@ export function parseDescriptor(text: string): Descriptor | null {
163163
* high surrogate last: JSON carries it as a bare `\ud83d` and UTF-8 as U+FFFD, and the Bot reads a
164164
* broken character that is not on the page. The server's `cutAtCodeUnits` is the same rule; this
165165
* process shares no code with the server, so it is repeated here rather than imported.
166+
*
167+
* Exported for `index.ts`, which cuts the readable page text the same way, and so that the rule has
168+
* its own tests. It lives here rather than beside that caller because this module imports no
169+
* Playwright: `index.ts` does, at load, so a helper declared there could not be tested at all.
166170
*/
167-
function cutAtCodeUnits(text: string, limit: number): string {
171+
export function cutAtCodeUnits(text: string, limit: number): string {
168172
const sliced = text.slice(0, limit);
169173
const last = sliced.charCodeAt(sliced.length - 1);
170174
return last >= 0xd800 && last <= 0xdbff ? sliced.slice(0, -1) : sliced;

agent-computer/src/index.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
import { serve } from "bun";
22
import type { Page } from "playwright";
3-
import { parseAriaSnapshot, type SnapshotElement } from "./aria-snapshot";
4-
import { browserModeFromEnv } from "./browser-mode";
3+
import {
4+
cutAtCodeUnits,
5+
parseAriaSnapshot,
6+
type SnapshotElement,
7+
} from "./aria-snapshot";
58
import {
69
actsOnTheComputer,
710
isOpenPath,
811
matchesToken,
912
offeredToken,
1013
} from "./authorisation";
1114
import { isPlainBotId } from "./bot-id";
15+
import { browserModeFromEnv } from "./browser-mode";
1216
import {
1317
type Control,
1418
ControlError,
@@ -289,7 +293,18 @@ async function readablePageText(
289293

290294
const collapsed = raw.replace(/\n{3,}/g, "\n\n").trim();
291295
return {
292-
text: collapsed.slice(0, TEXT_EXTRACT_LIMIT),
296+
/*
297+
* Cut between characters, not through one. #539 fixed this for a control's name and value in the
298+
* snapshot and left the page text, which is the same bug at thirty times the length: `slice`
299+
* counts UTF-16 code units, an emoji is two, and a limit landing between the halves hands the Bot
300+
* a lone high surrogate that reads as U+FFFD — a character that is not on the page.
301+
*
302+
* More likely to bite here than there, for the reason the limit is bigger. A 200-unit control
303+
* name rarely reaches an emoji; 6000 units of somebody's page usually passes through several, and
304+
* whether the cut lands mid-character is decided by whatever was above it.
305+
*/
306+
text: cutAtCodeUnits(collapsed, TEXT_EXTRACT_LIMIT),
307+
// Unaffected by the line above: dropping one more unit cannot make an over-limit string fit.
293308
truncated: collapsed.length > TEXT_EXTRACT_LIMIT,
294309
};
295310
}

agent-computer/tests/aria-snapshot.test.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, expect, test } from "bun:test";
2-
import { parseAriaSnapshot, parseDescriptor } from "../src/aria-snapshot";
2+
import {
3+
cutAtCodeUnits,
4+
parseAriaSnapshot,
5+
parseDescriptor,
6+
} from "../src/aria-snapshot";
37

48
/**
59
* The parser, tested against captured Playwright output.
@@ -200,6 +204,46 @@ describe("a name or value too long to keep whole", () => {
200204
});
201205
});
202206

207+
/**
208+
* The cut itself, at any limit.
209+
*
210+
* Tested directly as well as through the parser because `index.ts` cuts the readable page text with
211+
* it at 6000 rather than 200, and that caller imports Playwright at load, so there is no test that
212+
* can reach it. The rule is the thing worth pinning, so it is pinned where it is declared.
213+
*/
214+
describe("cutting at code units", () => {
215+
const EMOJI = "\u{1F600}";
216+
217+
test("a limit landing between the halves of a character drops the character", () => {
218+
const text = `${"a".repeat(5999)}${EMOJI}tail`;
219+
const cut = cutAtCodeUnits(text, 6000);
220+
expect(cut).toBe("a".repeat(5999));
221+
// Not a lone high surrogate, which is what a bare `slice` leaves and what reads as U+FFFD.
222+
expect(cut.charCodeAt(cut.length - 1)).toBeLessThan(0xd800);
223+
});
224+
225+
test("a character that ends exactly at the limit is kept whole", () => {
226+
const text = `${"a".repeat(5998)}${EMOJI}tail`;
227+
expect(cutAtCodeUnits(text, 6000)).toBe(`${"a".repeat(5998)}${EMOJI}`);
228+
});
229+
230+
test("text that fits is returned unchanged, emoji and all", () => {
231+
const text = `hello ${EMOJI} world`;
232+
expect(cutAtCodeUnits(text, 6000)).toBe(text);
233+
});
234+
235+
test("empty text is empty rather than a thrown index", () => {
236+
// `charCodeAt(-1)` is NaN and every comparison against it is false, so this returns "".
237+
expect(cutAtCodeUnits("", 6000)).toBe("");
238+
});
239+
240+
test("a low surrogate last is a whole character and is kept", () => {
241+
// The guard must look only for an UNPAIRED high surrogate. A complete pair ends on its low half,
242+
// and dropping that would cost a character the limit had room for.
243+
expect(cutAtCodeUnits(EMOJI, 2)).toBe(EMOJI);
244+
});
245+
});
246+
203247
describe("values a real parser handles and a pattern got wrong", () => {
204248
test("a quoted numeric value is not left with its quotes", () => {
205249
// Numeric-looking text remains a string, so one-time codes are not coerced.

0 commit comments

Comments
 (0)