Skip to content

fix(recording): render the touch overlay at most 30 fps and inside the record request - #3219

Merged
thymikee merged 3 commits into
callstack:mainfrom
harrisrobin:fix/touch-overlay-frame-cap
Oct 5, 2026
Merged

thymikee merged 3 commits into
callstack:mainfrom
harrisrobin:fix/touch-overlay-frame-cap

Conversation

@harrisrobin

@harrisrobin harrisrobin commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

record stop with touches took about 0.4 s per second of recording, outlasting record's 90 s request past about 3.5 minutes. Closes #3218. Five files:

  • Swift: the composition renders at most 30 fps. It used the capture's minFrameDuration, one timescale tick (1/600 s from simctl): 75 fps on the simulator, three times the frames captured. A capture reporting none renders at 30. The export wait takes a timeout.
  • overlay.ts: the overlay gets 70 s for compile, export and exit; the export gets what is left less 5 s, as --timeout-ms (both caps were 120 s). Past it the stop returns the raw video with overlayWarning, inside the request.

The export keeps the capture's resolution (#2707).

Validation

At ea2a631: pnpm check:affected --run passed (format, lint, typecheck, layering, fallow, build, 1,733 related tests, packaged runner Swift). Two new overlay.test.ts cases fail without the change; the root envelope check, which keeps 20 s of record's envelope outside the budget, fails with a 71 s one.

iPhone 16 simulator, record stop in seconds:

Recording Before After
30 s 13.8 6.3
2 min 48.6 19.9
5 min 90.2, timed out 47.8
10 min 66.5: export cut at its budget; raw video with overlayWarning, no helper or temp file left

Emulator, 2 min: 48.3 → 18.8. The 10-minute row ran on b50f71a06; the others ran this change on npm 0.21.20.

Not run locally: Swift runner builds and device lanes.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/capture-kit/src/recording/overlay.ts">

<violation number="1" location="packages/capture-kit/src/recording/overlay.ts:73">
P3: The process-kill timer (`runCmd` `timeoutMs: helperMs`) and the export timer (`--timeout-ms = helperMs - 5s`) start at different moments, but the difference between them is the only budget for everything else that can keep the helper alive past a completed export: helper startup, the post-export `verifyCompositedOverlay`, and exit. Because `runCmd`'s kill fires only when the process is still running at `helperMs` (measured from spawn), a helper whose export ran close to its `--timeout-ms` can be SIGKILLed before it finishes `verifyCompositedOverlay`/exits, discarding a good overlay down the raw-video fallback path. Give the post-export steps their own margin instead of deriving both from a single shared 5s grace.</violation>
</file>

Reply to a comment to ask cubic a question or push back. It learns from your replies.

Re-trigger cubic

Comment thread packages/capture-kit/src/recording/overlay.ts Outdated
env: buildSwiftToolEnv(),
});
const helperMs = deadline - Date.now();
const exportMs = helperMs - HELPER_EXIT_GRACE_MS;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The process-kill timer (runCmd timeoutMs: helperMs) and the export timer (--timeout-ms = helperMs - 5s) start at different moments, but the difference between them is the only budget for everything else that can keep the helper alive past a completed export: helper startup, the post-export verifyCompositedOverlay, and exit. Because runCmd's kill fires only when the process is still running at helperMs (measured from spawn), a helper whose export ran close to its --timeout-ms can be SIGKILLed before it finishes verifyCompositedOverlay/exits, discarding a good overlay down the raw-video fallback path. Give the post-export steps their own margin instead of deriving both from a single shared 5s grace.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/capture-kit/src/recording/overlay.ts, line 73:

<comment>The process-kill timer (`runCmd` `timeoutMs: helperMs`) and the export timer (`--timeout-ms = helperMs - 5s`) start at different moments, but the difference between them is the only budget for everything else that can keep the helper alive past a completed export: helper startup, the post-export `verifyCompositedOverlay`, and exit. Because `runCmd`'s kill fires only when the process is still running at `helperMs` (measured from spawn), a helper whose export ran close to its `--timeout-ms` can be SIGKILLed before it finishes `verifyCompositedOverlay`/exits, discarding a good overlay down the raw-video fallback path. Give the post-export steps their own margin instead of deriving both from a single shared 5s grace.</comment>

<file context>
@@ -54,11 +67,29 @@ async function exportProcessedVideo(params: {
-      env: buildSwiftToolEnv(),
-    });
+    const helperMs = deadline - Date.now();
+    const exportMs = helperMs - HELPER_EXIT_GRACE_MS;
+    if (exportMs <= 0) {
+      throw new AppError(
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Measured on a 600 s simulator recording with this helper:

  • 0.02 s from launch to the export;
  • after a completed export, 0.06 s in verifyCompositedOverlay and 0.06 s to exit;
  • after a timed-out export, 0.24 s from cancel to exit.

The shared 5 s covers each about 20 times, so 71650bb keeps one margin and makes HELPER_EXIT_GRACE_MS's comment name start-up, verify and exit.

@thymikee

thymikee commented Oct 5, 2026

Copy link
Copy Markdown
Member

The 30 fps cap looks right, but the new timeout route is not shown to work, so I need one live run before merge. At b50f71a, an export that runs past --timeout-ms makes the helper exit non-zero, and the finalizer returns the raw video with overlayWarning (overlay.ts#L32). Issue #3218 lists this route as required behavior, but every live row is a successful export. The 70 s budget starts in exportProcessedVideo, after the recorder stop, the pull, and the finalizer's waitForStableFile and waitForPlayableVideo. So it is not shown that stop plus 70 s plus the post-export checks stays under the 90 s envelope. If the time before the overlay is over about 15-20 s on a long recording, record stop can still time out and reset the daemon. Please run one record stop on the iPhone 16 simulator with touches and --quality high, on a recording long enough that the export needs more than 70 s (about 8-10 minutes at the measured 47.8 s per 5 min). Please report: (a) record stop wall time under 90 s with no "Daemon request timed out"; (b) the response carries overlayWarning and the raw video plays; (c) no recording-overlay helper process is left and no .agent-device-* partial file is left beside the video.

Not blocking, take or leave: overlay.ts hand-rolls Date.now() + budgetMs, and Deadline.fromTimeoutMs(...).remainingMs() in @agent-device/host-kit/retry already does this; and expect(OVERLAY_BUDGET_MS).toBeLessThan(90_000) in overlay.test.ts restates the envelope as a literal, so comparing against DEFAULT_TIMEOUT_POLICY.envelopeMs where layering allows, or dropping the line, would be better.

On the open review threads, the Swift export timer and 5 s exit grace thread still applies as a lower-priority item. These two do not apply, so you can resolve them: the deadline and pre-export waits thread is covered because the deadline is taken before both waits, and a validator timeout rejects on the first try; and the infinite timeout thread is covered because the only producer passes a finite positive value, and .isFinite would be optional hardening.

The one reported check passes, but no CI job runs the simulator or emulator overlay route, so green CI does not cover the run above. I did not run the Swift helper or any device, and the 30 fps speedup is your measurement only. I did not measure the recorder stop and finalizer waits on long recordings, or run the unit tests locally. The claim that the new tests would fail on the old code comes from reading the pre-change code. There are no conflicts. Before merge, we need that live simulator stop where the export runs out of its 70 s budget.

…ts budget against record's envelope

- `exportProcessedVideo` uses `Deadline.fromTimeoutMs(...).remainingMs()` instead of a hand-rolled
  `Date.now() + budgetMs`.
- The budget's comment no longer says the client resets the daemon: since callstack#3199 `record` keeps it
  on timeout, and the caller gets "Daemon request timed out" while the daemon finishes the stop.
- `HELPER_EXIT_GRACE_MS` names what it covers: start-up, then verifying the output or cancelling the
  export, and exiting.
- The check that the budget fits `record stop`'s envelope moves from a 90_000 literal in
  `overlay.test.ts` to the root timeout-policy test, against `record`'s resolved envelope.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/__tests__/command-descriptor-timeout-policy.test.ts">

<violation number="1" location="src/__tests__/command-descriptor-timeout-policy.test.ts:345">
P2: This check allows the overlay to consume nearly the entire request envelope, leaving no practical time for the stop, copies, and playability checks named above. Require a minimum reserve (the current configuration leaves 20 seconds).</violation>
</file>

Comment thread src/__tests__/command-descriptor-timeout-policy.test.ts Outdated
@harrisrobin

harrisrobin commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor Author

Ran it on this branch's head built from source (b50f71a06): iPhone 16 simulator, iOS 26.5, Xcode 27.0, Apple M5 Pro. Settings app, a tap, back or scroll every 4 s, record start --quality high with touches, 600 s, then record stop --json --debug.

  • (a) record stop returned in 66.5 s with success: true, and no "Daemon request timed out".
  • (b) The response has "overlayWarning": "failed to overlay recording touches: Failed to add touch overlays to the iOS recording". outPath is the raw capture: HEVC 1178x2556, 598.3 s, 11,379 frames. ffmpeg -xerror decodes all of it, and the frame 2 s from the end is the Settings screen with no ring. The gesture telemetry (103 events) is beside it.
  • (c) The helper exited by itself 64.9 s into the stop: its export timer cancelled the export (Touch overlay export timed out., exit 1), before the 69.7 s kill. ps at return and 5 s later shows no helper. The folder holds only video.mp4 and video.gesture-telemetry.json, with no .agent-device-* file. Run alone on the same video, a cancelled helper leaves nothing at --output either.

The video below is that outPath file, downscaled to 392 px wide at 15 fps so it fits as an attachment (the original is 187 MB).

record-stop-600s-raw-fallback.mp4

The run did take the route in question: with no timer, the same helper needs 93.1 s for this recording.

Where the time went, from the request's --debug log:

Step Seconds
Recorder stop, both copies, the finalizer's stable and playable checks 0 – 1.06
The overlay's budget starts; its own input checks 1.06 – 1.40
Helper (compile cached): export cancelled at its --timeout-ms, then exit 1.40 – 66.31
Raw-video fallback, response 66.33 (client: 66.47)

So about 1 s of a 10-minute stop comes before the budget, not 15-20 s.

A 5-minute stop that overlays, same setup, took 47.8 s:

  • 0.73 s before the helper;
  • 46.5 s in the helper;
  • 0.06 s for the two checks after it.

On 600 s files, waitForPlayableVideo on the output and the finalizer's isPlayableVideo take 0.03–0.04 s each.

From the non-blocking items, pushed as 71650bb:

  • Deadline.fromTimeoutMs(...).remainingMs() replaces the hand-rolled Date.now() + budgetMs.
  • The 90_000 literal is gone from overlay.test.ts. The root timeout-policy test now checks OVERLAY_BUDGET_MS against record's resolved envelope, and that check fails with a 90 s budget.
  • The budget's comment said the client resets the daemon, which fix(record): keep the daemon alive when a record request times out #3199 changed. It now says the daemon finishes the stop.

On r4179925495, measured on the same 600 s recording:

  • the helper spends 0.02 s before its export;
  • after a completed export, 0.06 s in verifyCompositedOverlay and 0.06 s to exit;
  • after a timed-out export, 0.24 s from cancel to exit.

The shared 5 s covers each about 20 times, so I kept one margin and made its comment name all three steps.

pnpm check:affected --base upstream/main --run passed on 71650bb: format, lint, typecheck, layering, fallow, build, 1,733 related tests and the packaged runner Swift. The Swift runner builds and device lanes are GitHub's.

… inside its envelope

The check let the budget take all but a millisecond of the envelope; the recorder stop, the copies
and the playability checks around the overlay need room beyond it.
@thymikee

thymikee commented Oct 5, 2026

Copy link
Copy Markdown
Member

The fixes from the earlier review (#3219 (comment)) are in, and I found no code problems at ea2a631. The timeout-policy test now requires the overlay budget plus 20 s to fit inside the record envelope, so the budget can no longer take the whole envelope. The one reported check is green, and no CI job runs the simulator overlay route. The live 600 s run covers that route, but it ran at b50f71a. This update does not touch the device path, so I treated that run as covering ea2a631. I ran no tests, Swift helper or device myself. The unit-test result and the pnpm check:affected pass are the author's claims. The 1 s pre-budget cost was measured on one simulator on one host. The 20 s reserve is a margin, not a measured bound for slower hosts or physical devices. There are no conflicts, and nothing else is needed before human review.

On the bot threads, the P2 on the timeout envelope is fixed at this head: #3219 (comment). The P1 on the pre-export waits is resolved with no code change, because the deadline is taken before both waits in overlay.ts, so they sit inside the budget: #3219 (comment). The P2 on an infinite --timeout-ms is resolved as benign, because the only producer passes a finite positive value. A non-finite check would be optional hardening: #3219 (comment). The P3 on the kill timer and export timer does not apply. The two timers start about 0.3 s apart, and the 5 s HELPER_EXIT_GRACE_MS covers the measured 0.06-0.24 s exit steps by roughly 20x. The live run showed the helper exiting by itself before the kill: #3219 (comment). Please resolve these four threads. The bot has not reviewed this head.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 5, 2026
@thymikee
thymikee merged commit 14e1bd7 into callstack:main Oct 5, 2026
13 of 14 checks passed
thymikee added a commit to okwasniewski/agent-device that referenced this pull request Oct 6, 2026
* origin/main: (77 commits)
  fix(apple-runner): fence prep spawns behind a start-owned admission (callstack#3239)
  0.21.22
  test(apple): own the simctl settings plan tests in simctl-settings.test.ts (callstack#3244)
  fix(limrun): report the session device id in iOS settings refusals (callstack#3243)
  0.21.21
  feat(remote): add a host-allocated macos-app lease backend (callstack#3236)
  test(android): bound the screenshot write wait by wall time, not event-loop turns (callstack#3250)
  feat(recording): cap the touch overlay frame rate at the caller's --fps (callstack#3241)
  fix(ad-script): let .ad scripts carry scroll --until and wait capture flags (callstack#3197) (callstack#3234)
  feat(provider-webdriver): keyboard enter, dismiss, and status over WebDriver (callstack#3233)
  feat(selectors): match role= against snapshot kind with a node-scoped alias window (callstack#3232)
  fix(provider-webdriver): read field values, placeholders, secure fields, and checked state from page source (callstack#3231)
  feat(replay): accept --test-ime on test and replay so flow-owned Android opens opt into the test IME (callstack#3235)
  refactor(daemon): route daemon-level diagnostics through one scope helper (callstack#3242)
  docs(adr): correct ADR 0031 pointer event delivery evidence (callstack#3245)
  fix(ios): stop a tap's post-gesture lookup from recording an XCTest failure (callstack#3060) (callstack#3237)
  fix(recording): render the touch overlay at most 30 fps and inside the record request (callstack#3219)
  fix(daemon): keep an idle daemon alive only for retained leases (callstack#3227)
  fix(provider-webdriver): send an empty JSON object on bodyless POSTs (callstack#3230)
  Feat/maestro repeat while (callstack#3214)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Touch overlay renders at the encoder's maximum frame rate, so record stop outlasts its 90 s request past ~3 minutes

2 participants