feat: add gesture telemetry recording overlays#245
Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d352b5c7b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const result = await runCmd('swift', ['-', filePath], { | ||
| stdin: VIDEO_VALIDATION_SCRIPT, |
There was a problem hiding this comment.
Avoid hard-requiring Swift for Android recording validation
Android record stop now depends on isPlayableVideo, which shells out to swift/AVFoundation; on non-macOS hosts this validator is unavailable, so valid pulled MP4s are treated as unplayable and copyAndroidRecordingWithValidation eventually fails with pulled file is not a playable MP4. This breaks Android recording stop in environments like the repo’s Ubuntu Android workflow, so the validation path should be cross-platform or gracefully skipped when the validator runtime is missing.
Useful? React with 👍 / 👎.
| field: 'gestureTelemetry', | ||
| path: recording.telemetryPath, | ||
| fileName: path.basename(recording.telemetryPath), |
There was a problem hiding this comment.
Keep telemetry artifact keyed to telemetryPath
The new telemetry artifact is emitted as field: 'gestureTelemetry' and without localPath, while the response key is telemetryPath; in remote-daemon flows artifact materialization keys off artifact.field and requires a client path, so this artifact is not materialized back and telemetryPath remains a daemon-local path. As a result remote clients cannot reliably download/use the telemetry sidecar from record stop.
Useful? React with 👍 / 👎.
776d414 to
71ca739
Compare
71ca739 to
369b02c
Compare
PR callstack#248 refactored interaction.ts into a thin routing layer delegating to focused command modules. PR callstack#245 (gesture telemetry overlays) unintentionally reverted get/is/scrollintoview back to inline implementations due to a stale branch. This restores the thin router from callstack#248, adapted for callstack#245's handleTouchInteractionCommands, and removes interaction-press.ts and interaction-fill.ts which were made dead code by callstack#245's interaction-touch.ts.
PR #248 refactored interaction.ts into a thin routing layer delegating to focused command modules. PR #245 (gesture telemetry overlays) unintentionally reverted get/is/scrollintoview back to inline implementations due to a stale branch. This restores the thin router from #248, adapted for #245's handleTouchInteractionCommands, and removes interaction-press.ts and interaction-fill.ts which were made dead code by #245's interaction-touch.ts.
…ry.swift Behavior-preserving relocation: move the text-entry concern out of the (formerly 1805-line) RunnerTests+Interaction.swift — the repo's most-changed file — into a new RunnerTests+TextEntry.swift. Interaction.swift drops to 1068 lines (-737). Moved verbatim (no logic changes): - value types: TextTypingRepairMode, TextEntryTiming, TextEntryResult, TextEntryTarget - the focus -> type -> verify -> repair pipeline (typeTextReliably + focus orchestration + readiness polling + dropped-char/repair heuristics) - clearTextInput and the text-entry leaf helpers (editableTextValue, isPlaceholderValue, isGenericTextInputLabel, normalizedElementText, moveCaretToEnd, estimatedDeleteCount, keyboardBecameVisible, keyboardElementExists) The whole cluster is text-entry-exclusive (its only callers are within the moved code or the already-internal command entry points), so every symbol keeps its original visibility — no access widened. Shared helpers used by gestures/snapshot/get-text (isKeyboardVisible, visibleKeyboardFrame, textInputAt, textInputCandidatesAt, readableText, ...) stay in Interaction. The new file is auto-included via the project's file-system-synchronized group. This is the safe, cosmetic half of the "TextEntry engine" architecture candidate. A deeper extraction behind a real seam/type is intentionally deferred: it carries the #245 revert risk and wants stronger text-entry e2e coverage first. Verified: xcodebuild build-for-testing -> TEST BUILD SUCCEEDED. Pure relocation, no behavior change.
…ry.swift (#651) Behavior-preserving relocation: move the text-entry concern out of the (formerly 1805-line) RunnerTests+Interaction.swift — the repo's most-changed file — into a new RunnerTests+TextEntry.swift. Interaction.swift drops to 1068 lines (-737). Moved verbatim (no logic changes): - value types: TextTypingRepairMode, TextEntryTiming, TextEntryResult, TextEntryTarget - the focus -> type -> verify -> repair pipeline (typeTextReliably + focus orchestration + readiness polling + dropped-char/repair heuristics) - clearTextInput and the text-entry leaf helpers (editableTextValue, isPlaceholderValue, isGenericTextInputLabel, normalizedElementText, moveCaretToEnd, estimatedDeleteCount, keyboardBecameVisible, keyboardElementExists) The whole cluster is text-entry-exclusive (its only callers are within the moved code or the already-internal command entry points), so every symbol keeps its original visibility — no access widened. Shared helpers used by gestures/snapshot/get-text (isKeyboardVisible, visibleKeyboardFrame, textInputAt, textInputCandidatesAt, readableText, ...) stay in Interaction. The new file is auto-included via the project's file-system-synchronized group. This is the safe, cosmetic half of the "TextEntry engine" architecture candidate. A deeper extraction behind a real seam/type is intentionally deferred: it carries the #245 revert risk and wants stronger text-entry e2e coverage first. Verified: xcodebuild build-for-testing -> TEST BUILD SUCCEEDED. Pure relocation, no behavior change.
Summary
Replace recording-time touch visualization with a telemetry-first pipeline.
Improve recording reliability and timing.
screenrecord+ overlay rendering and remove the old screenshot-encoder pathAdd a real verification harness for this feature.
Validation
pnpm typecheckpnpm test:unitpnpm test:smokepnpm build:xcuitestAGENT_DEVICE_RECORDING_E2E=1 node --test test/integration/recording-overlay.test.tsKnown gap:
screenrecordstop path is still less stable than longer recordings.