Skip to content

Commit afe99bd

Browse files
thymikeethymikee
committed
fix(ios): scope the placeholder refusal to an empty baseline
Review [P1]: the guard took only the placeholder and the expected text, so it refused any append whose result happened to equal the placeholder. Value "0" + `type ".00"` against placeholder "0.00" was refused before a single read, even though the non-empty pre-dispatch value proves the placeholder is not what is rendering and a later "0.00" is genuine commit evidence. The baseline is what decides it, so it is now an input. placeholderCommitEvidence returns three states rather than a boolean: normalRead expected differs from the placeholder; the placeholder never enters into the observation indistinguishable expected IS the placeholder and the field was empty, so the placeholder was what rendered and no read can resolve it rawValueIsEvidence expected IS the placeholder but the field held content, so a raw match is real Only .indistinguishable refuses, and it still refuses before the wait, since no read resolves it. .rawValueIsEvidence reaches the observation and settles on the raw match, which the normalized read would otherwise hide. commitObservation returns for that third state, now scoped by evidence rather than applied unconditionally as in the revision that made raw equality a false success. Both readings stay closures, so normalRead — the ordinary case — never pays for the raw read. Refs #1874
1 parent 7cedfc5 commit afe99bd

3 files changed

Lines changed: 178 additions & 46 deletions

File tree

‎apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift‎

Lines changed: 62 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -222,25 +222,56 @@ extension RunnerTests {
222222
}
223223
}
224224

225-
/// Whether the field's placeholder makes this commit impossible to observe.
225+
/// How the field's placeholder bears on observing this particular commit.
226226
///
227-
/// An empty text field renders its placeholder AS its accessibility value, and
228-
/// `editableTextValue(treatingPlaceholderAsEmpty:)` classifies that value as empty for exactly
229-
/// that reason. When the expected final text IS the placeholder, both states — nothing
230-
/// committed, and everything committed — produce the identical read, before and after dispatch.
231-
/// Raw equality is therefore not evidence: accepting it would report success over a field that
232-
/// may have received nothing, which is the failure this whole wait exists to remove.
227+
/// An empty text field renders its placeholder AS its accessibility value, which is why
228+
/// `editableTextValue(treatingPlaceholderAsEmpty:)` classifies that value as empty. That
229+
/// normalization is correct for the baseline and is what makes the expected value computable at
230+
/// all — but it also hides a committed value that happens to equal the placeholder.
233231
///
234-
/// No read can resolve the ambiguity, so the wait does not spend its deadline discovering that.
235-
/// It reports the commit unobserved immediately, which is what the caller refuses on.
236-
static func placeholderMakesCommitUnobservable(
232+
/// Whether a raw read equal to the placeholder is evidence depends entirely on what the field
233+
/// held before dispatch, which is the distinction these three cases carry.
234+
enum PlaceholderCommitEvidence: Equatable {
235+
/// The expected value differs from the placeholder, so the normalized read observes the
236+
/// commit and the placeholder never enters into it. The ordinary case.
237+
case normalRead
238+
/// The expected value IS the placeholder and the field was empty beforehand, so the
239+
/// placeholder was what rendered. Every read is byte-identical whether or not anything
240+
/// committed, before and after dispatch. No observation can resolve this, so accepting a raw
241+
/// match would report success over a field that may have received nothing — the failure this
242+
/// wait exists to remove.
243+
case indistinguishable
244+
/// The expected value IS the placeholder, but the field held content beforehand, so the
245+
/// placeholder is NOT what is rendering. A raw read equal to it is therefore real commit
246+
/// evidence, and refusing here would fail a `type` that worked (value "0", `type ".00"`).
247+
case rawValueIsEvidence
248+
}
249+
250+
static func placeholderCommitEvidence(
237251
placeholder: String?,
238-
expectedText: String
239-
) -> Bool {
240-
guard let placeholder else { return false }
241-
let trimmedPlaceholder = placeholder.trimmingCharacters(in: .whitespacesAndNewlines)
242-
guard !trimmedPlaceholder.isEmpty else { return false }
243-
return trimmedPlaceholder == expectedText.trimmingCharacters(in: .whitespacesAndNewlines)
252+
expectedText: String,
253+
baselineWasEmpty: Bool
254+
) -> PlaceholderCommitEvidence {
255+
let trimmedPlaceholder = placeholder?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
256+
guard !trimmedPlaceholder.isEmpty,
257+
trimmedPlaceholder == expectedText.trimmingCharacters(in: .whitespacesAndNewlines)
258+
else {
259+
return .normalRead
260+
}
261+
return baselineWasEmpty ? .indistinguishable : .rawValueIsEvidence
262+
}
263+
264+
/// The reading the commit wait compares against, given what the placeholder can prove here.
265+
/// Both readings are closures so the ordinary case stays at one accessibility read on a path
266+
/// that polls every 20ms for up to three seconds.
267+
static func commitObservation(
268+
evidence: PlaceholderCommitEvidence,
269+
expectedText: String,
270+
rawValue: () -> String?,
271+
normalizedValue: () -> String?
272+
) -> String? {
273+
if evidence == .rawValueIsEvidence, rawValue() == expectedText { return expectedText }
274+
return normalizedValue()
244275
}
245276

246277
/// The command-level consequence of a commit wait. `.unobservable` is not a failure: there was
@@ -277,20 +308,27 @@ extension RunnerTests {
277308
return .unobservable
278309
}
279310
let expectedText = textBefore + typedText
280-
if Self.placeholderMakesCommitUnobservable(
311+
let evidence = Self.placeholderCommitEvidence(
281312
placeholder: resolveTextEntryElement(app: app, target: target)?.placeholderValue,
282-
expectedText: expectedText
283-
) {
284-
return .notObserved
285-
}
313+
expectedText: expectedText,
314+
baselineWasEmpty: textBefore.isEmpty
315+
)
316+
// Nothing to wait for: no read distinguishes committed from untouched here, so the deadline
317+
// would only add latency to a verdict already determined.
318+
if evidence == .indistinguishable { return .notObserved }
286319
let deadline = Date().addingTimeInterval(TextEntryTiming.synthesizedCommitTimeout)
287320
return Self.awaitSynthesizedCommitOutcome(
288321
expectedText: expectedText,
289322
isExpired: { Date() >= deadline },
290323
observe: {
291-
editableTextValue(
292-
for: resolveTextEntryElement(app: app, target: target),
293-
treatingPlaceholderAsEmpty: true
324+
let element = resolveTextEntryElement(app: app, target: target)
325+
return Self.commitObservation(
326+
evidence: evidence,
327+
expectedText: expectedText,
328+
rawValue: { editableTextValue(for: element) },
329+
normalizedValue: {
330+
editableTextValue(for: element, treatingPlaceholderAsEmpty: true)
331+
}
294332
)
295333
},
296334
waitForNextObservation: { sleepFor(TextEntryTiming.pollInterval) }

‎apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryPolicyTests.swift‎

Lines changed: 115 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -150,36 +150,130 @@ extension RunnerTests {
150150

151151
// Review [P1]: an empty field renders its placeholder AS its accessibility value, so when the
152152
// requested text IS the placeholder, the raw read equals the expected text BEFORE anything
153-
// commits. An earlier revision treated that equality as evidence and reported ok with zero
154-
// characters delivered — the success-misdescribes-the-device failure this PR removes. The state
155-
// is structurally indeterminate: no read distinguishes "placeholder rendering" from "committed
156-
// value", so it must not be a success.
157-
func testTextEqualToThePlaceholderIsNeverEvidenceOfACommit() {
158-
XCTAssertTrue(
159-
Self.placeholderMakesCommitUnobservable(placeholder: "0.00", expectedText: "0.00"),
160-
"the requested text equals the placeholder — a raw match proves nothing committed"
153+
// commits. Treating that equality as evidence reported ok with zero characters delivered — the
154+
// success-misdescribes-the-device failure this PR removes.
155+
func testTextEqualToThePlaceholderOverAnEmptyFieldIsNeverEvidence() {
156+
XCTAssertEqual(
157+
Self.placeholderCommitEvidence(
158+
placeholder: "0.00",
159+
expectedText: "0.00",
160+
baselineWasEmpty: true
161+
),
162+
.indistinguishable,
163+
"an empty field renders the placeholder, so a raw match proves nothing committed"
161164
)
162-
// Both sides are compared trimmed, matching how isPlaceholderValue classifies the field.
163-
XCTAssertTrue(
164-
Self.placeholderMakesCommitUnobservable(placeholder: " 0.00 ", expectedText: "0.00")
165+
// Compared trimmed on both sides, matching how isPlaceholderValue classifies the field.
166+
XCTAssertEqual(
167+
Self.placeholderCommitEvidence(
168+
placeholder: " 0.00 ",
169+
expectedText: "0.00",
170+
baselineWasEmpty: true
171+
),
172+
.indistinguishable
165173
)
166174
// And the command consequence: this state refuses rather than reporting success.
167175
XCTAssertEqual(Self.textEntryFailure(forCommitOutcome: .notObserved), .commitNotObserved)
168176
}
169177

178+
// Review [P1], the other half: a NON-empty baseline proves the placeholder is not what is
179+
// rendering, so the same raw read is real evidence. Refusing here would fail a `type` that
180+
// worked. Reverting to a baseline-blind guard makes this return .indistinguishable and go red.
181+
func testTextEqualToThePlaceholderOverExistingContentIsEvidence() {
182+
XCTAssertEqual(
183+
Self.placeholderCommitEvidence(
184+
placeholder: "0.00",
185+
expectedText: "0.00",
186+
baselineWasEmpty: false
187+
),
188+
.rawValueIsEvidence
189+
)
190+
}
191+
192+
// The end-to-end shape of that case: value "0", `type ".00"`, placeholder "0.00". It must reach
193+
// the observation path and settle, not refuse before reading. The normalized reading stays ""
194+
// throughout — it is the placeholder-equal value being normalized away — so only the
195+
// evidence-scoped raw match can settle this, which is what makes the test discriminate.
196+
func testAppendWhoseResultEqualsThePlaceholderSettlesThroughTheObservation() {
197+
let evidence = Self.placeholderCommitEvidence(
198+
placeholder: "0.00",
199+
expectedText: "0.00",
200+
baselineWasEmpty: false
201+
)
202+
XCTAssertEqual(evidence, .rawValueIsEvidence)
203+
204+
var polls = 0
205+
let outcome = Self.awaitSynthesizedCommitOutcome(
206+
expectedText: "0.00",
207+
isExpired: { polls >= 5 },
208+
observe: {
209+
Self.commitObservation(
210+
evidence: evidence,
211+
expectedText: "0.00",
212+
// The commit lands after the first poll: "0" -> "0.00".
213+
rawValue: { polls == 0 ? "0" : "0.00" },
214+
normalizedValue: { "" }
215+
)
216+
},
217+
waitForNextObservation: { polls += 1 }
218+
)
219+
XCTAssertEqual(outcome, .settled, "the append committed and must be observed as such")
220+
XCTAssertEqual(polls, 1)
221+
}
222+
170223
// The guard must stay narrow: it fires only when the WHOLE expected value is the placeholder.
171224
// Widening it would refuse ordinary typing into any placeheld field, which is most of them.
172-
func testPlaceholderGuardDoesNotFireOnOrdinaryTyping() {
173-
XCTAssertFalse(
174-
Self.placeholderMakesCommitUnobservable(placeholder: "0.00", expectedText: "0.005"),
175-
"a distinguishable value must still be observed rather than refused"
176-
)
177-
XCTAssertFalse(
178-
Self.placeholderMakesCommitUnobservable(placeholder: "Email", expectedText: "[email protected]")
225+
func testPlaceholderEvidenceDoesNotFireOnOrdinaryTyping() {
226+
for baselineWasEmpty in [true, false] {
227+
XCTAssertEqual(
228+
Self.placeholderCommitEvidence(
229+
placeholder: "0.00",
230+
expectedText: "0.005",
231+
baselineWasEmpty: baselineWasEmpty
232+
),
233+
.normalRead
234+
)
235+
XCTAssertEqual(
236+
Self.placeholderCommitEvidence(
237+
placeholder: "Email",
238+
expectedText: "[email protected]",
239+
baselineWasEmpty: baselineWasEmpty
240+
),
241+
.normalRead
242+
)
243+
XCTAssertEqual(
244+
Self.placeholderCommitEvidence(
245+
placeholder: nil,
246+
expectedText: "0.00",
247+
baselineWasEmpty: baselineWasEmpty
248+
),
249+
.normalRead
250+
)
251+
XCTAssertEqual(
252+
Self.placeholderCommitEvidence(
253+
placeholder: " ",
254+
expectedText: "",
255+
baselineWasEmpty: baselineWasEmpty
256+
),
257+
.normalRead
258+
)
259+
}
260+
}
261+
262+
// A normalRead observation never pays for the raw read, so the ordinary path stays at one
263+
// accessibility read per poll.
264+
func testOrdinaryObservationDoesNotPayForTheRawRead() {
265+
var rawReads = 0
266+
let observed = Self.commitObservation(
267+
evidence: .normalRead,
268+
expectedText: "hardware-keyboard",
269+
rawValue: {
270+
rawReads += 1
271+
return "hardware-keyboard"
272+
},
273+
normalizedValue: { "hardware-" }
179274
)
180-
XCTAssertFalse(Self.placeholderMakesCommitUnobservable(placeholder: nil, expectedText: "0.00"))
181-
XCTAssertFalse(Self.placeholderMakesCommitUnobservable(placeholder: "", expectedText: ""))
182-
XCTAssertFalse(Self.placeholderMakesCommitUnobservable(placeholder: " ", expectedText: ""))
275+
XCTAssertEqual(rawReads, 0)
276+
XCTAssertEqual(observed, "hardware-")
183277
}
184278

185279
// The mapping the command actually refuses on. `.unobservable` must stay a success: it is the

‎website/docs/docs/commands.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ agent-device gesture transform 200 420 80 -40 2 35 700 # combined pan, zoom, and
427427
`type` accepts text only. Do not pass `@ref` to `type`; use `fill @ref "text"` to target a field directly, or `press @ref` then `type "text"` to append in the focused field.
428428
If `type` reports `TEXT_INPUT_NOT_FOCUSED`, focus a visible text input and retry; when accessibility does not expose the input, use a coordinate focus command before typing.
429429
On iOS, if `type` reports `TEXT_INPUT_SYNTHESIS_UNAVAILABLE` while the software keyboard is hidden, show the software keyboard, then retry `type` or `fill`. The runner reports this error instead of risking partial input through an unreliable text-entry path.
430-
On iOS, if `type` reports `TEXT_INPUT_COMMIT_NOT_OBSERVED`, the runner could not confirm the typed text reached the field — either it did not land before the runner's deadline, or the requested text is identical to the field's placeholder, which makes a committed value indistinguishable from an untouched one. The field may hold none, part, or all of the text: read it back, then prefer `fill`, which replaces the whole value, over `type`, which appends to whatever committed. This covers the bare-type route, which observes the field after synthesizing; it is not a guarantee that every text-entry route verifies its result.
430+
On iOS, if `type` reports `TEXT_INPUT_COMMIT_NOT_OBSERVED`, the runner could not confirm the typed text reached the field — either it did not land before the runner's deadline, or the field was empty and the requested text is identical to its placeholder, which an empty field renders as its own value — leaving a committed result indistinguishable from an untouched one. The field may hold none, part, or all of the text: read it back, then prefer `fill`, which replaces the whole value, over `type`, which appends to whatever committed. This covers the bare-type route, which observes the field after synthesizing; it is not a guarantee that every text-entry route verifies its result.
431431
Use plain `fill` or `type` first for ordinary login and form fields. Use `--delay-ms` on `type` or `fill` only when a debounced search field or search-as-you-type input actually misses characters, or when the app must receive incremental updates.
432432
Delayed typing intentionally prefers paced character entry over clipboard-style fallbacks so the target field receives each incremental update.
433433
On Android, `fill` also verifies text and treats IME-owned capture as a terminal failure instead of retrying against the wrong field.

0 commit comments

Comments
 (0)