Repository navigation
test: measure pointer gestures on the isolated visible display - #23678
Conversation
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Frame-aware pointer budget — the spec now derives its reference from
Math.max(latency.evalControl, 1000 / 60), so a sub-framebrowser.evalcontrol can no longer set an impossible budget, while the 3x/3x/10x multipliers and every gesture-fidelity assertion stay as they were.
The change is sound and the tradeoff is understood. On the fast macOS host where evalControl measured ~0.28–0.38 ms, the previous relative budget was below a single display frame, so the test rejected the correct in-process implementation. The floor keeps host-relative scaling on slow hosts and still catches the gross regressions the earlier Linux run showed (562 ms wheel, 2033 ms click). The deterministic no-subprocess guarantee lives in src/main/browser/agent-browser-bridge-pointer-input.test.ts:104, so the E2E spec remains a coarse latency budget rather than the correctness gate. No action needed.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 2 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe pointer latency assertions now use the greater of the measured evaluation-control latency and one 60 Hz frame as their baseline. The move and wheel limits remain three times the baseline. The click limit remains ten times the baseline. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This test-only change accounts for frame pacing while preserving its measurements and latency limits. No actionable merge risk is evident; it is ready to merge subject to normal checks. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
nwparker
changed the title
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
|
|
||
| test('dispatches coordinate pointer input fast enough for real gestures', async ({ orcaPage }) => { | ||
| test('dispatches coordinate pointer input fast enough for real gestures @headful', async ({ |
There was a problem hiding this comment.
Adding @headful excludes this test from the headless project used by the full E2E CI shards and the default local E2E command. CI runs headful tests only when their spec is in the changed-spec list, so a full-suite run no longer checks pointer gestures or dispatch latency. Later regressions can go unnoticed unless someone explicitly runs the headful test.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const dispatchControlMs = Math.max(latency.evalControl, 1000 / 60) | ||
| expect(latency.mouseMove, latencyReport).toBeLessThan(dispatchControlMs * 3) | ||
| expect(latency.mouseWheel, latencyReport).toBeLessThan(dispatchControlMs * 3) | ||
| expect(latency.click, latencyReport).toBeLessThan(dispatchControlMs * 10) |
There was a problem hiding this comment.
When the no-op control takes less than one frame, this floor raises the allowed pointer latency substantially. With a 5 ms control, a 30 ms move and a 103 ms click pass the new 50 ms and 167 ms limits, even though those times are roughly the slower helper-path multiples documented by the previous test. This makes the check less able to catch a return to that slower path on fast hosts.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Retagged the pointer-latency spec
@headful— the test title now ends in@headful, moving it out of theelectron-headlessproject (grepInvert: /@headful/) and intoelectron-headful(grep: /@headful/), where the fixture launches withORCA_E2E_HEADFUL=1and presents the window without focus. The frame-aware budget and all gesture-fidelity assertions are unchanged.
ℹ️ The latency budget now runs only in the changed-spec lane
Tagging @headful removes the spec from the scheduled --project=electron-headless shards (the only full-suite run), and the only scheduled headful invocation is the explicit worktree-switch-first-paint.spec.ts step at .github/workflows/e2e.yml:226. After merge this spec runs only when changed-e2e selects it, so the pointer-dispatch latency guard will no longer run on the daily reference suite. That may be the intended trade — the deterministic no-subprocess guarantee already lives in src/main/browser/agent-browser-bridge-pointer-input.test.ts:114 — but if the coarse gesture-latency budget should keep running routinely, it needs its own headful step alongside the first-paint benchmark.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
1 participant
ELI5
The pointer test now measures gestures with a visible browser on CI's isolated display, where Chromium delivers input at the normal frame rate.
What Changed
Run the test in the existing
electron-headfullane. Compare latency against the greater of the no-op RPC baseline and one 60 Hz frame, keeping the same 3/3/10 multipliers and all gesture checks.Why
Hidden Linux windows delay button events by about one second and wheel events by about half a second. A no-op RPC also finishes faster than a frame, so it cannot alone represent the cost of rendered input. The isolated visible run measures real gestures without changing application behavior or weakening their timing requirements.
Linked Issue
CI failure: https://github.com/stablyai/orca/actions/runs/36476013862
Visual Proof
N/A — test configuration and timing baseline only.
Testing
Review
No runtime, protocol, SSH, or product UI changes. Visible runs use CI's isolated display.
Agent skill upstream boundary
Checklist