Conversation
…r posts The delivery budget added in #2904 was a parallel projection: projectedSeconds re-derived the spaced split, the warmup peel and the warmup wait, all decided elsewhere, and two pre-merge fix commits were drift this shape caused. It also under-charged the plan it modeled: a post that replaces selects with its own synthesize record (RunnerSynthesizedTextEntry.m), which the estimate counted as one call. SynthesizedTextPlan now carries the posts a command makes (characters, whether the post selects first, the wait after it), the budget charges the sum over those steps, and both synthesized routes execute that same array — the loop sleeps the plan's own pauses instead of re-deriving delaySeconds > 0 twice more. The type route decides the budget once, outside the per-chunk closure, and its budget refusal and synthesizer .fallback arm share one application-wide typing path. The pace moved from the ObjC bridge (whose getter Swift read back) into TextEntryTiming and is passed to the bridge, so no call site can type at a pace its own budget never charged. The test-only acknowledge window moved into the tests, and the fixture's never-asserted edits= counter is gone. fill's admitted length at --delay-ms 0 moves 214 -> 212 (honesty about the select-all record, which the old hint overpromised); the recovery hint now fits the host's 400-character diagnostic bound it previously overflowed by 60, losing its last sentence on the wire.
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed 43ea03c. I found no blocking defect. Both synthesized routes now run the step array the budget charges, and the refusal shares the Could one step executor serve both routes? It would take a plan and a post closure, slice the characters, and sleep Related, not blocking: the warmup step's pause is charged from the plan, but Smoke Tests, Repo Guards and Coverage were still running at review time, with no failures. Smoke Tests exercises this change directly. Next step: answer the executor question above. For evidence, please attach the simulator log lines from the head commit for the pace test ( |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
Downstream report from a CI fleet pinning agent-device releases, before this lands. On main (#2904) Ask, since the budget model is being rewritten here anyway: make the budget a pace, never a refusal.
Happy to take this as a follow-up PR on top of this one if you prefer to keep the scope here; wanted the ask on record before the admitted length is pinned again. |
…utor Both synthesized routes now run the plan through runSynthesizedTextPlan: the executor slices the next characters per step, takes the wait the plan carries, and hands a warmup step to the route's own read-back. The plan records no shape flag; the burst/warmup/paced phase naming derives from the steps. A post that stops the plan answers with its typed failure before the target reads the last element, so a vanished input cannot turn a focused failure into a snapshot error.
|
One The warmup read-back is a per-step hook and must stay outside the plan, for the reason the charge already encodes: the plan owns that a wait is owed and its budgeted cost (one poll), while what the route waits on depends on a resource the plan cannot see. An element-backed route polls the field until it reads back The consolidation also caught a real ordering regression, now pinned: reading Cubic's three: the counter comment now distinguishes gating from asserting and names Also dropped the review-history sentence on the hint comment, keeping the 400-char bound as the constraint. Simulator log lines from this head, as asked:
|
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift">
<violation number="1" location="apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift:315">
P3: Do not emit a full-text `type-delayed` phase after the plan stops early; it misstates delivery telemetry for mid-command failures. Skip this phase when `planFailure` is set, or log the posted prefix length.</violation>
</file>
<file name="apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift">
<violation number="1" location="apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift:263">
P2: This warmup branch replaces the plan’s charged pause instead of applying it. The read-back can return immediately, allowing the remaining slice to post faster than the budgeted plan; apply `pauseAfterSeconds` after the callback or include the callback duration in admission.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| if step.warmsUpField { | ||
| waitAfterWarmupCharacter(String(characters[postedCount - step.characterCount..<postedCount])) | ||
| } else if step.pauseAfterSeconds > 0 { | ||
| sleepFor(step.pauseAfterSeconds) | ||
| } |
There was a problem hiding this comment.
P2: This warmup branch replaces the plan’s charged pause instead of applying it. The read-back can return immediately, allowing the remaining slice to post faster than the budgeted plan; apply pauseAfterSeconds after the callback or include the callback duration in admission.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift, line 263:
<comment>This warmup branch replaces the plan’s charged pause instead of applying it. The read-back can return immediately, allowing the remaining slice to post faster than the budgeted plan; apply `pauseAfterSeconds` after the callback or include the callback duration in admission.</comment>
<file context>
@@ -225,6 +219,56 @@ extension RunnerTests {
+ }
+ postedCount = nextCount
+ didPostStep(step)
+ if step.warmsUpField {
+ waitAfterWarmupCharacter(String(characters[postedCount - step.characterCount..<postedCount]))
+ } else if step.pauseAfterSeconds > 0 {
</file context>
| if step.warmsUpField { | |
| waitAfterWarmupCharacter(String(characters[postedCount - step.characterCount..<postedCount])) | |
| } else if step.pauseAfterSeconds > 0 { | |
| sleepFor(step.pauseAfterSeconds) | |
| } | |
| if step.warmsUpField { | |
| waitAfterWarmupCharacter(String(characters[postedCount - step.characterCount..<postedCount])) | |
| } | |
| if step.pauseAfterSeconds > 0 { | |
| sleepFor(step.pauseAfterSeconds) | |
| } |
| repairMode: repairMode | ||
| ) | ||
| ) | ||
| if synthesizedTypePlan.pacesEveryCharacter { |
There was a problem hiding this comment.
P3: Do not emit a full-text type-delayed phase after the plan stops early; it misstates delivery telemetry for mid-command failures. Skip this phase when planFailure is set, or log the posted prefix length.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift, line 315:
<comment>Do not emit a full-text `type-delayed` phase after the plan stops early; it misstates delivery telemetry for mid-command failures. Skip this phase when `planFailure` is set, or log the posted prefix length.</comment>
<file context>
@@ -235,138 +235,100 @@ extension RunnerTests {
- repairMode: repairMode
- )
+ )
+ if synthesizedTypePlan.pacesEveryCharacter {
+ // Paced delivery is one paced burst: the phase covers every post and names the whole text
+ // rather than its last character.
</file context>
| if synthesizedTypePlan.pacesEveryCharacter { | |
| if synthesizedTypePlan.pacesEveryCharacter && planFailure == nil { |
This branch has not been deployed
2 participants
Second commit
705ef4bc0answers the review on #2955's own terms: one step executor serves both synthesized routes.runSynthesizedTextPlantakes the plan, the text, a post closure and the route's warmup read-back, slices the next characters per step, and takes every wait the plan carries.isSpacedandwarmupSplitare gone; the phase log's burst/warmup/paced naming derives from the steps themselves.RunnerTests+TextTyping.swiftloses 38 net lines and the fill route no longer re-derives anything fromdelaySeconds.Closes #2955. The delivery budget from #2904 was a parallel projection:
projectedSecondsre-derived the spaced split, the warmup peel and the warmup wait — all decided elsewhere — and under-charged even its own model, counting a select-and-type post as one synthesize call when the bridge runs two (Cmd-A record + text record).SynthesizedTextPlannow carries the posts a command makes (character counts, whether a post selects first, the wait after it). The budget charges the sum over those steps, and both synthesized routes execute that same array: the loop sleeps the plan's own pauses instead of re-derivingdelaySeconds > 0(twice more, at that). The type route decides the budget once, outside the per-chunk closure, and its budget refusal and synthesizer.fallbackarm share one application-wide typing path. The pace moved from the ObjC bridge (whose getter Swift read back through a second source of truth) intoTextEntryTimingand is passed to the bridge, so no call site can type at a pace its own budget did not charge.synthesizedAcknowledgeWindowSecondswas production dead weight; it moved into the tests. The fixture's never-assertededits=counter is gone.Numbers:
fill's admitted length at--delay-ms 0moves 214 → 212 — honesty about the select-all record the old hint overpromised (at 80 ms both models say 57). The recovery hint now fits the host's 400-char diagnostic bound it previously overflowed by 60, silently losing its last sentence viaredactDiagnosticData.Validation
Head
705ef4bc0.pnpm check:affected --runpassed locally (xctest-selection: 0 dark tests; packaged-runner-swift ok) andpnpm check:fallow --base origin/mainreports no issues in the changed files.Local XCUITest evidence on head
705ef4bc0:RunnerTests+TextEntryPolicyTests,RunnerTests+SynthesizedTextEntryTestsandRunnerTests+TextTypingTestspassed (TEST EXECUTE SUCCEEDED), including the two new executor contract tests.TEST EXECUTE SUCCEEDED); the other 14 areos(iOS)-guarded.testSynthesizedReplacementPacesAnAppOwnedFieldAtItsAcknowledgeWindowpassed on that first line's route (16.4 s);testOverBudgetTypeWithoutResolvableElementTypesApplicationWidepassed reading the field back after the second pair (16.7 s).Regression evidence (mutation discipline, head
43ea03ca0):synthesizeCallCount → 1→testSynthesizedPlanChargesAReplacingPostTwoSynthesizeCallsred at both asserts.peelsWarmupCharacter: false(budget ignores the peel) →testOverBudgetTypeWithoutResolvableElementTypesApplicationWidered: routesynthesized-first-responder≠xctest-application-fallback— proving the peel charge is what makes the over-budget branch reachable.1e-9only absorbs float accumulation between the two subtracted sums); plan steps cover the text exactly once (lengths 2/3/11/240 × delays 0/0.08); the executor hands each step its own slice, gives a warmup step the read-back with exactly its own characters, and reports only delivered characters when a post stops the plan; append chunks admitted at fill's own advertised budget; hint ≤ 400 chars.One regression the executor consolidation found and fixed: reading
withElement's frame after a post that refused the text turnedtestBareDelayedTypeFailsWhenTappedInputDisappearsMidCommandinto a snapshot error, because the input is gone by then. A stopped plan now answers with its typed failure before touching the target.Risk: admission tightened by 2 chars for
fillat delay 0 (refuses 213–214 that previously shipped and could time out mid-command).type.noneburst admission unchanged. Docs need no change;commands.md:491describes the hint's shape, which is preserved.