From 3d0b753205fdb9d470b589f44ecb915975cdbfda Mon Sep 17 00:00:00 2001 From: Iisyourdad Date: Fri, 12 Jun 2026 09:02:51 -0500 Subject: [PATCH] Add a 200ms click debounce with extensive behavioral tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per request: clicks of the same button closer together than capture.clickDebounceMs (default 200ms) now collapse into a single step, so accidental fast/double clicks don't each become a step. It is a leading-edge debounce measured from the last *accepted* click, so a run of fast clicks can't push the next deliberate click out — two clicks spaced beyond the window (e.g. the reported 400-500ms apart) always register. Replaces the prior 8ms duplicate-delivery suppression (subsumed by the window). Configurable; 0 captures every click. Tests (the point of this change is that it can't silently regress): - 13 behavioral unit tests in capture.test.js that drive real onOsClick calls with controlled timestamps and assert which clicks survive — the reported 400/450/500ms cases, sub-window collapse, the 200ms boundary, per-button independence, configurability, debounce=0, last-accepted (not last-dropped) reference, session reset, and a full onOsClick -> queue -> store integration check. No keyword/comment assertions. - A fourth end-to-end self-test scenario (burst of 40ms clicks collapses to 1; three 300ms-apart clicks each register => 4 total). The marker/drain scenarios set debounce to 0 so they keep stressing the frame pipeline. 147 unit tests + all repo checks pass. Co-Authored-By: Claude Fable 5 --- app/capture.js | 61 +++++++++------ app/main.js | 40 ++++++++++ core/settings.js | 5 ++ docs/CHANGELOG.md | 12 +++ tests/unit/capture.test.js | 154 +++++++++++++++++++++++++++++-------- 5 files changed, 217 insertions(+), 55 deletions(-) diff --git a/app/capture.js b/app/capture.js index 0cbe496..7ae7224 100644 --- a/app/capture.js +++ b/app/capture.js @@ -45,12 +45,13 @@ const { physicalToDip } = require('./coords'); * failures surface as { ok: false, reason } instead of crashing. */ -// Suppress only *duplicate deliveries* of one physical press (same button, -// same coordinates, a few ms apart). This deliberately replaces the old -// time-only debounce: real humans double-click ~50-100ms apart, and any -// purely temporal cutoff eventually drops a legitimate fast click, which -// reads as "my click didn't register". One hook/watcher event = one click. -const CLICK_EVENT_DUPLICATE_MS = 8; +// Leading-edge click debounce: the first click of a button is captured, and +// further clicks of that button within this window of the last *accepted* +// click are ignored. This collapses accidental fast / double clicks into one +// step, while any two deliberate clicks spaced more than the window apart +// each register. Tunable via capture.clickDebounceMs; this is only the +// default when the setting is absent. +const DEFAULT_CLICK_DEBOUNCE_MS = 200; // How long a Linux raw button event waits for its regular twin (the // representation that carries root coordinates) before firing without them. const LINUX_CLICK_TWIN_MS = 25; @@ -122,7 +123,7 @@ class CaptureService { this.frameLoopGrabStartedAt = null; this.recentFrames = []; this.shooting = false; - this.lastClickEventByButton = new Map(); + this.lastAcceptedClickByButton = new Map(); this.streamBackend = null; this.streamBackendStarting = false; } @@ -946,7 +947,7 @@ public static class SFMouseHook { this.clickWatcherBuf = ''; this.linuxEvent = null; this.discardPendingRawClick(); - this.lastClickEventByButton.clear(); + this.lastAcceptedClickByButton.clear(); } /** @@ -1101,15 +1102,22 @@ public static class SFMouseHook { this.pendingRawClick = null; } + /** Debounce window in ms (capture.clickDebounceMs, default 200). */ + clickDebounceMs() { + const raw = this.settings.get('capture.clickDebounceMs'); + const v = Number(raw); + return raw != null && Number.isFinite(v) && v >= 0 ? v : DEFAULT_CLICK_DEBOUNCE_MS; + } + onOsClick(at = Date.now(), osPoint = null, button = 'mouse') { if (!this.session || this.session.paused) return; const clickAt = Number.isFinite(at) ? at : Date.now(); - // Source-aware dedupe, not a debounce: each hook/watcher event is one - // click however fast it follows the previous one. Only an *identical* - // event a few ms later — duplicate delivery of one physical press — is - // suppressed. - if (this.isDuplicateClickEvent(clickAt, osPoint, button)) { - clog('click@', clickAt, button, 'suppressed as duplicate delivery'); + // Leading-edge debounce: ignore a click that lands within the debounce + // window of the last accepted click of the same button. This makes fast + // / accidental repeat clicks register once, while two deliberate clicks + // spaced more than the window apart each register (one step per click). + if (this.isDebouncedClick(clickAt, button)) { + clog('click@', clickAt, button, 'debounced (within', this.clickDebounceMs(), 'ms of last accepted)'); return; } // Prefer the position the watcher sampled with the button-down event @@ -1124,18 +1132,21 @@ public static class SFMouseHook { this.enqueueClickCapture(clickPos, clickAt, button || 'mouse'); } - isDuplicateClickEvent(at, osPoint, button) { + /** + * Whether this click should be dropped by the debounce. A click is dropped + * only when it follows the last *accepted* click of the same button by + * less than the debounce window — so the window is measured from accepted + * clicks, never from dropped ones, and a run of fast clicks can't push the + * next deliberate click out indefinitely. Accepting a click records it as + * the new reference point. Different buttons debounce independently. + */ + isDebouncedClick(at, button) { const key = button || 'mouse'; - const last = this.lastClickEventByButton.get(key); - this.lastClickEventByButton.set(key, { at, osPoint }); - if (!last) return false; - if (at < last.at || at - last.at >= CLICK_EVENT_DUPLICATE_MS) return false; - // Same button within a few ms: duplicate only if it is the *same* event - // (same coordinates, or neither delivery carried coordinates). - if (osPoint && last.osPoint) { - return osPoint.x === last.osPoint.x && osPoint.y === last.osPoint.y; - } - return !osPoint && !last.osPoint; + const windowMs = this.clickDebounceMs(); + const last = this.lastAcceptedClickByButton.get(key); + if (last != null && at >= last && at - last < windowMs) return true; + this.lastAcceptedClickByButton.set(key, at); + return false; } /** diff --git a/app/main.js b/app/main.js index 6c7b88c..e833fd8 100644 --- a/app/main.js +++ b/app/main.js @@ -104,6 +104,11 @@ function createWindow() { if (process.env.STEPFORGE_CLICK_SELFTEST) { setTimeout(async () => { try { + // The marker/drain scenarios inject clicks faster than the default + // debounce to stress the frame pipeline; turn the debounce off for + // them so every injected click is captured. A dedicated scenario + // at the end re-enables it and verifies the debounce itself. + settings.set('capture.clickDebounceMs', 0); const guide = store.createGuide({ title: 'click selftest' }); capture.startSession(guide.guideId, { intervalSec: 0 }); // Isolate the test from the user's real mouse: the session starts @@ -222,6 +227,41 @@ function createWindow() { console.log('CLICK-SELFTEST arm: first click ->', armStepIds.length, 'step(s)', armPreClick ? '(see margin in [capture] log above)' : 'FAIL — first click lost'); capture.finishSession(); + + // Fourth scenario: the debounce itself, exercised end to end through + // onOsClick. A fast burst (40ms apart) must collapse to one step, + // and deliberate clicks (300ms apart) must each register. + settings.set('capture.clickDebounceMs', 200); + const dbGuide = store.createGuide({ title: 'debounce selftest' }); + mainWindow.show(); + await new Promise((res) => setTimeout(res, 200)); + capture.startSession(dbGuide.guideId, { intervalSec: 0 }); + capture.stopClickWatcher(); + capture.clickCaptureAvailable = () => true; + capture.hiddenForSession = true; + capture.togglePause(false); + await capture.startClickFrameBackend(); + await new Promise((res) => setTimeout(res, 1500)); + const dbPoint = { + x: Math.round(bounds.x + bounds.width * 0.55), + y: Math.round(bounds.y + bounds.height * 0.55), + }; + // 4 clicks 40ms apart — accidental fast clicking → expect 1 step. + for (let i = 0; i < 4; i++) { + capture.onOsClick(Date.now(), toPhysical(dbPoint), 'button-1'); + await new Promise((res) => setTimeout(res, 40)); + } + // 3 deliberate clicks 300ms apart → expect 3 more steps. + for (let i = 0; i < 3; i++) { + await new Promise((res) => setTimeout(res, 300)); + capture.onOsClick(Date.now(), toPhysical(dbPoint), 'button-1'); + } + await capture.clickQueue; + await new Promise((res) => setTimeout(res, 800)); + const dbSteps = store.getGuide(dbGuide.guideId).stepsOrder.length; + console.log('CLICK-SELFTEST debounce:', dbSteps, 'of 4 expected', + dbSteps === 4 ? 'OK — burst collapsed to 1, three deliberate clicks kept' : 'FAIL'); + capture.finishSession(); } catch (err) { console.log('CLICK-SELFTEST ERROR', err.message); } finally { diff --git a/core/settings.js b/core/settings.js index 0a4ff46..62aa672 100644 --- a/core/settings.js +++ b/core/settings.js @@ -18,6 +18,11 @@ const DEFAULT_SETTINGS = { hotkeyPauseResume: 'CommandOrControl+Shift+2', captureOutsideClicks: true, confirmSimpleCapture: false, + // Leading-edge click debounce (ms): clicks of the same button closer + // together than this collapse into one step, so accidental fast/double + // clicks don't each become a step. Clicks spaced further apart always + // register. Set to 0 to capture every click. + clickDebounceMs: 200, autoIntervalSec: 5, // session fallback when click capture is unavailable // Strict click timing: a step never uses a frame whose grab started // after the click. Turn off only if captures are too slow to keep a diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index bc63a20..29c213b 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -37,6 +37,18 @@ Keep-a-Changelog conventions; versions follow semver. buffering by the time the window tucks away, so the first click is served a pre-click frame like the rest. +### Added + +- **Click debounce (`capture.clickDebounceMs`, default 200ms).** Clicks of + the same mouse button closer together than the window collapse into one + step, so accidental fast or double clicks don't each become a step, while + any two deliberate clicks spaced further apart both register. It is a + leading-edge debounce measured from the last *accepted* click, so a run of + fast clicks can't push the next real click out. Set it to 0 to capture + every click. Backed by behavioral unit tests that drive click sequences + through real timestamps (not keyword checks) plus an end-to-end self-test + scenario. + ### Fixed - **Fast click bursts no longer lose screenshots.** Finishing or pausing a diff --git a/tests/unit/capture.test.js b/tests/unit/capture.test.js index f08ab4d..a6371ec 100644 --- a/tests/unit/capture.test.js +++ b/tests/unit/capture.test.js @@ -422,42 +422,136 @@ test('windows hook click lines carry button and event timestamp', () => { }]); }); -// ---- click dedupe (source-aware, not a debounce) ------------------------------- +// ---- click debounce (~200ms) --------------------------------------------------- +// +// These tests drive real onOsClick calls with controlled timestamps and +// record which clicks survive to enqueueClickCapture, so they exercise the +// actual debounce arithmetic rather than asserting on comments or constants. +// The behavior they lock in: clicks of one button closer together than the +// debounce window collapse to one; clicks spaced further apart all register. -test('fast same-button hook clicks are all captured — there is no time debounce', () => { - const service = makeService(); - service.session = { guideId: 'guide-burst', paused: false, count: 0, intervalSec: 0 }; - const seen = []; - service.enqueueClickCapture = (clickPos, at) => { - seen.push(at); - }; +/** Run a timestamp sequence through onOsClick; return the accepted times. */ +function runClickSequence(service, times, { button = 'left', point = { x: 10, y: 20 } } = {}) { + service.session = service.session || { guideId: 'g', paused: false, count: 0, intervalSec: 0 }; + const accepted = []; + service.enqueueClickCapture = (clickPos, at) => { accepted.push(at); }; + for (const t of times) service.onOsClick(t, point, button); + return accepted; +} - // A 5-click burst 15ms apart — faster than the old 40ms debounce allowed. - const base = 1770000000000; - for (let i = 0; i < 5; i++) { - service.onOsClick(base + i * 15, { x: 100 + i, y: 200 }, 'left'); - } - - assert.equal(seen.length, 5, 'every distinct hook event is one click'); +test('default debounce is 200ms when the setting is absent', () => { + const service = makeService(); // settings stub has no clickDebounceMs + assert.equal(service.clickDebounceMs(), 200); }); -test('duplicate delivery of one physical press is suppressed', () => { - const service = makeService(); - service.session = { guideId: 'guide-dupe', paused: false, count: 0, intervalSec: 0 }; - const seen = []; - service.enqueueClickCapture = (clickPos, at) => { - seen.push(at); +test('two deliberate clicks 400ms apart both register (the reported case)', () => { + const service = makeService({ settings: { 'capture.clickDebounceMs': 200 } }); + const accepted = runClickSequence(service, [0, 400]); + assert.deepEqual(accepted, [0, 400], 'clicks well outside the window must not be dropped'); +}); + +test('clicks 400–500ms apart all register across a longer sequence', () => { + const service = makeService({ settings: { 'capture.clickDebounceMs': 200 } }); + const times = [0, 450, 950, 1400, 1900]; // 450/500/450/500 ms gaps + assert.deepEqual(runClickSequence(service, times), times, + 'every click spaced beyond the window is captured'); +}); + +test('a click just past the window (250ms) registers; just inside (150ms) does not', () => { + const service = makeService({ settings: { 'capture.clickDebounceMs': 200 } }); + assert.deepEqual(runClickSequence(makeService({ settings: { 'capture.clickDebounceMs': 200 } }), [0, 250]), [0, 250]); + assert.deepEqual(runClickSequence(service, [0, 150]), [0], '150ms < 200ms window is debounced away'); +}); + +test('exactly 200ms apart registers (window is exclusive at the boundary)', () => { + const service = makeService({ settings: { 'capture.clickDebounceMs': 200 } }); + assert.deepEqual(runClickSequence(service, [0, 200]), [0, 200]); +}); + +test('a fast burst collapses to a single step', () => { + const service = makeService({ settings: { 'capture.clickDebounceMs': 200 } }); + // 5 clicks 30ms apart — accidental fast clicking. + assert.deepEqual(runClickSequence(service, [0, 30, 60, 90, 120]), [0], + 'rapid repeats within the window are one click'); +}); + +test('the window is measured from the last ACCEPTED click, not the last dropped one', () => { + // A run of fast clicks must not push the next real click out forever: once + // a click is accepted, only later clicks reset the reference, and dropped + // clicks never do. 0 accepted; 100/150 dropped; 250 is 250ms after 0 so + // accepted; 300 dropped; 500 is 250ms after 250 so accepted. + const service = makeService({ settings: { 'capture.clickDebounceMs': 200 } }); + assert.deepEqual(runClickSequence(service, [0, 100, 150, 250, 300, 500]), [0, 250, 500]); +}); + +test('different mouse buttons debounce independently', () => { + const service = makeService({ settings: { 'capture.clickDebounceMs': 200 } }); + service.session = { guideId: 'g', paused: false, count: 0, intervalSec: 0 }; + const accepted = []; + service.enqueueClickCapture = (clickPos, at, button) => { accepted.push(`${button}@${at}`); }; + // Left then right 50ms apart: different buttons, both register. A second + // left 50ms after the first left is debounced. + service.onOsClick(0, { x: 1, y: 1 }, 'left'); + service.onOsClick(50, { x: 1, y: 1 }, 'right'); + service.onOsClick(50, { x: 1, y: 1 }, 'left'); + assert.deepEqual(accepted, ['left@0', 'right@50']); +}); + +test('the debounce window is configurable', () => { + const service = makeService({ settings: { 'capture.clickDebounceMs': 500 } }); + // With a 500ms window, the 400ms-apart clicks now collapse. + assert.deepEqual(runClickSequence(service, [0, 400]), [0]); + // 600ms apart clears the larger window. + const service2 = makeService({ settings: { 'capture.clickDebounceMs': 500 } }); + assert.deepEqual(runClickSequence(service2, [0, 600]), [0, 600]); +}); + +test('a debounce of 0 captures every click', () => { + const service = makeService({ settings: { 'capture.clickDebounceMs': 0 } }); + assert.deepEqual(runClickSequence(service, [0, 1, 2, 3]), [0, 1, 2, 3]); +}); + +test('duplicate hook deliveries of one press collapse under the debounce', () => { + // The same physical press delivered twice a few ms apart is well inside + // any reasonable window, so it still yields exactly one step. + const service = makeService({ settings: { 'capture.clickDebounceMs': 200 } }); + assert.deepEqual(runClickSequence(service, [1770000000000, 1770000000003]), [1770000000000]); +}); + +test('debounce state resets between sessions so the first click always registers', () => { + const service = makeService({ settings: { 'capture.clickDebounceMs': 200 } }); + runClickSequence(service, [0]); + service.stopClickWatcher(); // clears per-button accepted times + const accepted = []; + service.session = { guideId: 'g2', paused: false, count: 0, intervalSec: 0 }; + service.enqueueClickCapture = (clickPos, at) => { accepted.push(at); }; + // A click 50ms later but in a fresh session must not be debounced away. + service.onOsClick(50, { x: 10, y: 20 }, 'left'); + assert.deepEqual(accepted, [50]); +}); + +test('debounced clicks never reach the capture queue (end to end through onOsClick)', async () => { + // Integration: drive the real onOsClick → enqueueClickCapture → clickQueue + // → sessionCapture → store path with a stubbed frame source, and confirm + // the stored step count matches the number of accepted clicks. + const service = makeService({ settings: { 'capture.clickDebounceMs': 200, 'capture.clickMarker': false } }); + service.session = { guideId: 'g-e2e', paused: false, count: 0, intervalSec: 0 }; + service.userIsInApp = () => false; + service.frameForClick = async () => makeFrame('frame'); + const stored = []; + service.store.addStep = (guideId, fields, png) => { + stored.push(png.toString()); + return { stepId: `s${stored.length}` }; }; + // Two deliberate clicks 450ms apart, with an accidental fast repeat 40ms + // after the first. Expect two stored steps, not three or one. + service.onOsClick(0, { x: 10, y: 20 }, 'left'); + service.onOsClick(40, { x: 10, y: 20 }, 'left'); // debounced + service.onOsClick(450, { x: 30, y: 40 }, 'left'); + await service.clickQueue; - // Same button, same coordinates, 3ms apart: the same event delivered twice. - service.onOsClick(1770000000000, { x: 50, y: 60 }, 'left'); - service.onOsClick(1770000000003, { x: 50, y: 60 }, 'left'); - // Different coordinates inside the same window: a real second click. - service.onOsClick(1770000000006, { x: 80, y: 60 }, 'left'); - // Different button inside the same window: also real. - service.onOsClick(1770000000007, { x: 80, y: 60 }, 'right'); - - assert.deepEqual(seen, [1770000000000, 1770000000006, 1770000000007]); + assert.equal(stored.length, 2, 'one step per accepted click — burst repeat dropped, real clicks kept'); + assert.equal(service.session.count, 2); }); // ---- coordinate conversion ------------------------------------------------------