Skip to content

Commit a8bf5ab

Browse files
committed
fix: tighten large text read follow-ups
1 parent ca1da9a commit a8bf5ab

6 files changed

Lines changed: 95 additions & 29 deletions

File tree

ios-runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,13 @@ extension RunnerTests {
132132
if leftArea != rightArea {
133133
return leftArea < rightArea
134134
}
135-
return left.debugDescription.count < right.debugDescription.count
135+
if left.frame.minY != right.frame.minY {
136+
return left.frame.minY < right.frame.minY
137+
}
138+
if left.frame.minX != right.frame.minX {
139+
return left.frame.minX < right.frame.minX
140+
}
141+
return left.elementType.rawValue < right.elementType.rawValue
136142
}
137143

138144
for element in candidates where prefersExpandedTextRead(element) {

macos-helper/Sources/AgentDeviceMacOSHelper/main.swift

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,11 @@ private func optionValue(arguments: [String], name: String) -> String? {
349349
}
350350

351351
private func readTextAtPosition(bundleId: String?, surface: String?, x: Double, y: Double) throws -> String {
352+
let targetApp: NSRunningApplication?
352353
if surface == "frontmost-app" || (surface == nil && bundleId != nil) {
353-
_ = try resolveTargetApplication(bundleId: bundleId, surface: surface)
354+
targetApp = try resolveTargetApplication(bundleId: bundleId, surface: surface)
355+
} else {
356+
targetApp = nil
354357
}
355358

356359
let systemWide = AXUIElementCreateSystemWide()
@@ -361,14 +364,32 @@ private func readTextAtPosition(bundleId: String?, surface: String?, x: Double,
361364
throw HelperError.commandFailed("read did not resolve an accessibility element")
362365
}
363366

367+
if let targetApp {
368+
var pid: pid_t = 0
369+
guard AXUIElementGetPid(hitElement, &pid) == .success else {
370+
throw HelperError.commandFailed("read could not resolve element owner")
371+
}
372+
guard pid == targetApp.processIdentifier else {
373+
throw HelperError.commandFailed(
374+
"read resolved text from a different app",
375+
details: [
376+
"expectedPid": String(targetApp.processIdentifier),
377+
"actualPid": String(pid)
378+
]
379+
)
380+
}
381+
}
382+
364383
var current: AXUIElement? = hitElement
365-
var depth = 0
366-
while let element = current, depth < 8 {
384+
while let element = current {
367385
if let text = readableText(for: element) {
368386
return text
369387
}
370-
current = elementAttribute(element, attribute: kAXParentAttribute as String)
371-
depth += 1
388+
let parent = elementAttribute(element, attribute: kAXParentAttribute as String)
389+
if let parent, CFEqual(parent, element) {
390+
break
391+
}
392+
current = parent
372393
}
373394

374395
throw HelperError.commandFailed("read did not resolve text")

src/core/dispatch.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,7 @@ export async function dispatchCommand(
605605
});
606606
return { action: 'read', text: result.text };
607607
}
608+
// macOS app sessions run through the XCUITest runner; only desktop/menubar surfaces use the helper.
608609
const result = await runIosRunnerCommand(
609610
device,
610611
{

src/daemon/__tests__/snapshot-processing.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,20 @@ test('extractNodeReadText prefers underlying value for text surfaces', () => {
6363
]);
6464
assert.equal(extractNodeReadText(nodes[0]), 'package com.example.app\nclass MainActivity {}');
6565
});
66+
67+
test('extractNodeReadText ignores generic implementation identifiers as fallback text', () => {
68+
const nodes = attachRefs([
69+
{
70+
index: 0,
71+
type: 'android.widget.TextView',
72+
identifier: 'com.example:id/content_frame',
73+
},
74+
{
75+
index: 1,
76+
type: 'AXStaticText',
77+
identifier: '_NS:248',
78+
},
79+
]);
80+
assert.equal(extractNodeReadText(nodes[0]), '');
81+
assert.equal(extractNodeReadText(nodes[1]), '');
82+
});

src/utils/snapshot-lines.ts

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
import type { SnapshotNode } from './snapshot.ts';
2-
import {
3-
buildTextPreview,
4-
extractReadableText,
5-
isLargeTextSurface,
6-
trimText,
7-
} from './text-surface.ts';
2+
import { buildTextPreview, describeTextSurface, trimText } from './text-surface.ts';
83

94
type SnapshotDisplayLine = {
105
node: SnapshotNode;
@@ -56,10 +51,11 @@ export function formatSnapshotLine(
5651
options: SnapshotLineFormatOptions = {},
5752
): string {
5853
const type = normalizedType ?? formatRole(node.type ?? 'Element');
59-
const label = resolveDisplayLabel(node, type, options);
54+
const textSurface = describeTextSurface(node, type);
55+
const label = resolveDisplayLabel(node, type, options, textSurface);
6056
const indent = ' '.repeat(depth);
6157
const ref = node.ref ? `@${node.ref}` : '';
62-
const metadata = buildLineMetadata(node, type, options);
58+
const metadata = buildLineMetadata(node, type, options, textSurface);
6359
const metadataText = metadata.map((entry) => ` [${entry}]`).join('');
6460
const textPart = label ? ` "${label}"` : '';
6561
if (hiddenGroup) {
@@ -188,22 +184,23 @@ function resolveDisplayLabel(
188184
node: SnapshotNode,
189185
type: string,
190186
options: SnapshotLineFormatOptions,
187+
textSurface: { text: string; isLargeSurface: boolean; shouldSummarize: boolean },
191188
): string {
192189
if (!options.summarizeTextSurfaces) {
193190
return displayLabel(node, type);
194191
}
195-
const text = extractReadableText(node);
196-
if (!isLargeTextSurface(node, type) || !shouldSummarizeTextSurface(text)) {
192+
if (!textSurface.shouldSummarize) {
197193
return displayLabel(node, type);
198194
}
199-
const semanticLabel = semanticSurfaceLabel(node, type, text);
195+
const semanticLabel = semanticSurfaceLabel(node, type, textSurface.text);
200196
return semanticLabel || displayLabel(node, type);
201197
}
202198

203199
function buildLineMetadata(
204200
node: SnapshotNode,
205201
type: string,
206202
options: SnapshotLineFormatOptions,
203+
textSurface: { text: string; isLargeSurface: boolean; shouldSummarize: boolean },
207204
): string[] {
208205
const metadata: string[] = [];
209206
if (node.enabled === false) metadata.push('disabled');
@@ -213,22 +210,14 @@ function buildLineMetadata(
213210
if (!options.summarizeTextSurfaces) {
214211
return metadata;
215212
}
216-
const text = extractReadableText(node);
217-
if (!isLargeTextSurface(node, type) || !shouldSummarizeTextSurface(text)) {
213+
if (!textSurface.shouldSummarize) {
218214
return metadata;
219215
}
220-
metadata.push(`preview:"${escapePreviewText(buildTextPreview(text))}"`);
216+
metadata.push(`preview:"${escapePreviewText(buildTextPreview(textSurface.text))}"`);
221217
metadata.push('truncated');
222218
return uniqueMetadata(metadata);
223219
}
224220

225-
function shouldSummarizeTextSurface(text: string): boolean {
226-
if (!text) {
227-
return false;
228-
}
229-
return text.length > 80 || /[\r\n]/.test(text);
230-
}
231-
232221
function semanticSurfaceLabel(node: SnapshotNode, type: string, text: string): string {
233222
const label = trimText(node.label);
234223
if (label && label !== text) {

src/utils/text-surface.ts

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ export function extractReadableText(node: TextSurfaceNode): string {
1111
const label = trimText(node.label);
1212
const value = trimText(node.value);
1313
const identifier = trimText(node.identifier);
14+
const fallbackIdentifier = isMeaningfulReadableIdentifier(identifier) ? identifier : '';
1415
if (prefersValueForReadableText(node.type ?? '')) {
15-
return value || label || identifier;
16+
return value || label || fallbackIdentifier;
1617
}
17-
return label || value || identifier;
18+
return label || value || fallbackIdentifier;
1819
}
1920

2021
export function isLargeTextSurface(node: TextSurfaceNode, displayType?: string): boolean {
@@ -43,6 +44,30 @@ export function buildTextPreview(text: string): string {
4344
return `${normalized.slice(0, 45)}...`;
4445
}
4546

47+
export function describeTextSurface(
48+
node: TextSurfaceNode,
49+
displayType?: string,
50+
): {
51+
text: string;
52+
isLargeSurface: boolean;
53+
shouldSummarize: boolean;
54+
} {
55+
const text = extractReadableText(node);
56+
const isLargeSurface = isLargeTextSurface(node, displayType);
57+
return {
58+
text,
59+
isLargeSurface,
60+
shouldSummarize: isLargeSurface && shouldSummarizeTextSurface(text),
61+
};
62+
}
63+
64+
export function shouldSummarizeTextSurface(text: string): boolean {
65+
if (!text) {
66+
return false;
67+
}
68+
return text.length > 80 || /[\r\n]/.test(text);
69+
}
70+
4671
export function trimText(value: unknown): string {
4772
return typeof value === 'string' ? value.trim() : '';
4873
}
@@ -71,3 +96,10 @@ function normalizeTextSurfaceType(type: string): string {
7196
}
7297
return normalized;
7398
}
99+
100+
function isMeaningfulReadableIdentifier(value: string): boolean {
101+
if (!value) {
102+
return false;
103+
}
104+
return !/^[\w.]+:id\/[\w.-]+$/i.test(value) && !/^_?NS:\d+$/i.test(value);
105+
}

0 commit comments

Comments
 (0)