Skip to content

fix(agent): let a thumb-wheel swipe fire Volume Up/Down more than once - #1455

Open
4ni1ak wants to merge 2 commits into
AprilNEA:masterfrom
4ni1ak:fix/thumbwheel-volume-sensitivity-887
Open

4ni1ak wants to merge 2 commits into
AprilNEA:masterfrom
4ni1ak:fix/thumbwheel-volume-sensitivity-887

Conversation

@4ni1ak

@4ni1ak 4ni1ak commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Thumb-wheel VolumeUp/VolumeDown bindings went through the same
    discrete-action state machine as any other one-shot binding
    (MissionControl, NextTab, …): one threshold crossing fires the action
    once, then a 200ms cooldown discards every further rotation increment
    until it expires. Sensitivity only changed how much rotation was needed to
    cross the threshold the first time — it never let a single swipe fire
    more than once — so no matter how high the sensitivity, one swipe could
    never move the system volume by more than one native OS step (reported as
    "the volume only goes up or down by 2").
  • Volume is meant to behave like a physical volume wheel, not a one-shot
    action: a longer or faster swipe should be able to fire several times,
    scaled by sensitivity. The cooldown exists specifically to stop a one-shot
    action from repeating on a fast spin (firing MissionControl five times
    from one flick would be wrong) — that protection doesn't apply to Volume.

Changes

  • crates/openlogi-agent-core/src/watchers/gesture/dispatch/wheel.rs:
    WheelOutput::FireAction now carries a repeat count (u32).
    advance_action classifies VolumeUp/VolumeDown as repeatable
    (is_repeatable) and, for those, computes how many complete sensitivity
    thresholds the accumulated swipe distance spans and fires that many times
    in one update, with no cooldown gate between them; every other discrete
    action keeps the exact previous cooldown-gated, fire-once behavior.
  • crates/openlogi-agent-core/src/watchers/gesture/dispatch.rs: the
    FireAction dispatch site now issues the action repeats times.

Testing

  • cargo test -p openlogi-agent-core -p openlogi-agent (256 tests) —
    updated existing FireAction assertions to the new FireAction(1) shape,
    moved the cooldown/decay/binding-change tests that used VolumeUp purely
    as a stand-in discrete action onto a genuinely non-repeatable action
    (NextTab/PrevTab) so they keep testing cooldown semantics correctly,
    and added a_repeatable_action_fires_once_per_threshold_crossed_in_one_swipe
    and repeatable_action_progress_below_threshold_carries_over proving the
    fix: a swipe spanning three thresholds fires three times in one update,
    and a repeatable action is never cooldown-gated the way an ordinary one is.
  • cargo fmt --all -- --check
  • RUSTFLAGS="-D warnings" cargo clippy -p openlogi-agent-core -p openlogi-agent --all-targets -- -D warnings
    (affected-package tier: this change is confined to openlogi-agent-core,
    whose only reverse dependency is openlogi-agent; no Cargo.toml/lock,
    wire-format, or other workspace-wide input changed)
  • Not runtime-tested on hardware — the fix is proven by the unit tests above
    against the exact accumulator/threshold model the real capture session
    drives; a maintainer with a thumb-wheel device can confirm a firm swipe on
    a Volume binding now changes the system volume by more than one step.

Fixes #887

VolumeUp/VolumeDown were dispatched through the same discrete-action state
machine as any other thumb-wheel binding: one threshold crossing fires the
action once, then a 200ms cooldown discards every further increment until
it expires. Sensitivity only changed how much rotation was needed to cross
the threshold the first time, so a single swipe — however far or fast, and
regardless of sensitivity — could never move the system volume by more
than one native step, since the cooldown threw away the rest of the swipe's
distance instead of queuing it.

Volume behaves like a physical volume wheel, not a one-shot action like
MissionControl (where firing more than once per flick would be wrong): let
it fire once per threshold crossed within a single swipe, scaled by
sensitivity, with no cooldown gate between repeats.

Fixes AprilNEA#887
@4ni1ak
4ni1ak requested a review from AprilNEA as a code owner September 17, 2026 04:02
@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with a non-blocking concern that pathological excess wheel distance can still drain across later callbacks.

Fix All in CodexFindings

  1. P2 Capped distance persists
  2. P2 Unbounded synchronous dispatch burst

Summary

This PR makes thumb-wheel volume actions repeat according to accumulated swipe distance while preserving cooldown behavior for one-shot actions.

  • Adds repeat counts to wheel action outputs and dispatches each requested volume action.
  • Bounds immediate dispatch to 20 repetitions per captured event.
  • Updates existing tests and adds coverage for repeatable actions, carry-over, and the immediate cap.

Reviews (2) · Last reviewed commit: "fix(agent): cap repeats a single thumbwh..."

Comment on lines +233 to +235
for _ in 0..repeats {
self.outputs.actions.dispatch(action, Some(key));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Unbounded synchronous dispatch burst

At maximum sensitivity, the threshold is 1, while one captured i16 wheel increment can have a magnitude of 32,768. This can produce FireAction(32768), causing this loop to synchronously send 32,768 native volume actions. An unusually large device event could therefore tie up the input callback and flood the OS with volume events. Please cap or otherwise bound the repeats accepted from one event.

Fix in Codex Fix in Claude Code

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.

Good catch, valid — fixed in ab12fc8. Added a MAX_REPEATS_PER_EVENT cap (20) so one event's repeats are bounded regardless of magnitude; a real swipe never legitimately needs more than a handful of fires, and anything beyond the cap just carries the leftover progress into the next event instead of dropping it. Added a test proving a single event at i16::MAX magnitude and max sensitivity (threshold 1) still only fires the capped count.

A captured rotation increment is a signed i16; at max sensitivity
(threshold == 1) an unusually large or corrupt report could otherwise
claim a magnitude of thousands, synchronously dispatching that many
native volume presses from one input callback. Bound it to a repeat
count no legitimate swipe ever needs.
Comment on lines +185 to +186
let repeats = (increments / threshold).min(MAX_REPEATS_PER_EVENT);
increments -= repeats * threshold;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Capped distance persists

The per-event cap retains every excess threshold in increments. A large report at threshold 1 fires 20 actions but leaves the remaining distance in persistent state, so each following wheel event within the 300 ms decay window can fire another 20 actions from that stale distance. This spreads the volume flood across callbacks instead of discarding the pathological excess; clamp or discard the excess accumulator when the cap is reached.

Fix in Codex Fix in Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Thumbwheel set to control volume ignores sensitivity

1 participant