keymap: extract pending session module and unify replay dispatch

[rgoulter]

Summary

Extracts pending-key session logic into src/keymap/pending.rs and unifies replay dispatch for the two replay paths.

Context

Continues the pending key-event resolution cleanup in Keymap. Prior steps:

• rust: keymap: extract InputEventQueue from keymap input handling #571 — extracted InputEventQueue from Keymap (delay-line pacing + prepend API)
• keymap: prepend on pending resolve + extract resolution filter #575 — prepend on resolve instead of drain/append; extracted pending_resolution_events filter helper
• rust: keymap: fix scheduled input double-processing during pending #578 — fixed scheduled-input double-processing during pending (record_pending_input, was_pending in handle_event)
• keymap: document delay-line vs pending session log #581 — documented delay-line (input_queue) vs session log (queued_events) at implementation sites

This PR is the next step: pull pending session concerns out of keymap.rs and route both replay paths through one dispatcher. Bundled as one PR because extract and unify touch the same call sites (resolve_pending_key_state, nested-pending branch in update_pending_state).

Not in scope: pending-local ingest pacing (medium-risk follow-up; naive one-buffer bypass still fails rust-integration tap-hold/chorded tests).

Changes

• PendingKeySession — wraps active pending state; record_input, start, take, etc. (replaces ad-hoc record_pending_input + Option<PendingState> field)
• pending_resolution_events — moved from keymap.rs with unit tests (originally added in keymap: prepend on pending resolve + extract resolution filter #575)
• ReplayPolicy + dispatch_replayed_events — shared routing for:
  • OnResolve — filter to last self-event (keymap: prepend on pending resolve + extract resolution filter #575 policy); prepend inputs; stagger key events
  • OnNestedPending — LIFO re-queue of all session-log inputs (prepend_pending_input_events)
• pressed_key_result_outcome — maps PressedKeyResult for the pending update loop
• Keymap keeps thin glue (resolve_pending_key_state, update_pending_state, process_input)

Testing

• cargo test -p smart-keymap --lib
• cargo clippy -p smart-keymap -- -D warnings

[Copilot]

Pull request overview

This PR continues the Keymap pending-key refactor by extracting pending-session state into a dedicated pending module and routing both “resolve replay” and “nested pending replay” through a single dispatcher, reducing duplicated replay logic in keymap.rs.

Changes:

• Extract pending-session types and helpers into src/keymap/pending.rs (PendingState, PendingKeySession, pending_resolution_events + tests).
• Unify replay routing via ReplayPolicy + dispatch_replayed_events, used by both resolve and nested-pending replay paths.
• Simplify pending update loop matching via pressed_key_result_outcome / PendingPressedKeyOutcome.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/keymap/pending.rs	New module containing pending-session state, replay dispatch, and the pending-resolution filter (with unit tests).
src/keymap.rs	Integrates PendingKeySession and replaces duplicated replay logic with dispatch_replayed_events for both replay paths.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rgoulter]

Eh.

Previous iteration of PR was under-explained. This is over-explained. :|

[rgoulter]

So, this is called once with OnNestedPending...

[rgoulter]

...And this is called once with OnResolve...

[rgoulter]

Hmm. Reading through this. I might have to come back to this later.

It might be clearer if many cases had unit tests to illustrate the mechanism (or at least encode what happens now). -- This seems (imo) the best next task to get an LLM to help with. -- It's apparent there are several subtle cases, but these are only caught later in the Rust integration test suite.

I do like the LLM describing the two current queues in terms of 'input queue is delay lane' (so e.g. chorded keys can accurately resolve), and 'session log' for replay-on-resolve. -- That was covered in #581.

I think previous PRs #571 and #575 (and to an extend the CRUCIAL bugfix #578!) already give me more confidence in the pending key resolution.

• rust: keymap: extract InputEventQueue from keymap input handling #571 used a better abstraction for the queue (which is what my intuition was nagging me about.
• keymap: prepend on pending resolve + extract resolution filter #575 documented some of the subtle points
• rust: keymap: fix scheduled input double-processing during pending #578 was fantastic at fixing the subtle incorrect implementation.

This PR? It's 'just' shifting code around. Which is nice in terms of being a small change. -- But, although the ReplayCase does emphasise this "it's handled differently during key resolution vs continuing resolution with a nested pending state".. it's not clear to me that replay_dispatched_events (taking in a two-valued enum) is a big win.

• In favour of "unify replay dispatch into one function" is then the mutation of the session log, paced inputs, and queued events is all done in strictly one place.
• Against it, mechanically: using a two-valued enum and the whole implementation of replay_dispatched_events practically being two disjoint functions just smells. This looks really stupid.

Yeah. Having an enum only used to call a function with disjointed body is just a silly LLMism.

I think moving the code stuff to "pending key resolution" module is nice. I think having dispatch_events_ functions is nice.

---

Snippets from LLM's analysis (LLM makes a good point as to what would be worth trying to separate out, but it's not clear if it'll work):

Problem summary

resolve_pending_key_state is thorny because it merges two buffers and inserts replayed events at the front of the global input queue.

While a key is pending, input can accumulate in:

1. PendingState.queued_events — the session log (what happened while deciding).
2. input_queue — the delay line (events waiting on tick delay after handle_input pushed but pop_front_if_ready returned None).

On resolve, replayed events must run before whatever was already sitting in the global input_queue tail. The still-pending path is already simpler: prepend_pending_input_events (no drain of the global tail).

---

Three jobs tangled in resolve_pending_key_state

Job	What it means
Filter	For the resolving key: keep only the last event targeting that keymap_index; replay all other events in order.
Route	key::Event::Input → input queue; other key::Event → event_scheduler with staggered tick delays (so press/release can affect HID report one tick apart).
Priority	Replay batch must run before whatever was already in the global input_queue.

take_all / append_all existed only for priority (fixed in #575). The partition + .last() logic is a separate policy concern (extracted as pending_resolution_events in #575).

. . .

LLM reckons:

Delay only on replay (optional)

Apply tick spacing when flushing on resolve, not when ingesting during pending. Combines with pending-local ingest pacing for a single logical buffer.

...

What's still gnarly

handle_input → input_queue (pacing) → process_input → queued_events (log)
                                              ↓
                                    update_pending_state
                                              ↓
                         resolve → filter log → prepend / scheduler → handle_event ↺

Three concerns still share plumbing:

1. Pacing — input_queue spaces inputs one-per-tick
2. Logging — queued_events records the pending session
3. Replay — filter, route, cascade via handle_pending_events

#578 fixed a specific handle_event/process_input hazard; the overall flow is still dense.

Subtle point: events still in the input_queue delay line at resolve time are intentionally not in queued_events — they process post-resolve in normal mode.

PendingState owns ingest_delay; global input_queue only for non-pending traffic. Gate on rust-integration tap-hold/chorded tests.

[rgoulter]

Similarly in terms of LLM-isms, this enum provides very little value.

This codebase is full of abstractions that are 'cute'. But this is a bit too excessive.

[rgoulter]

Ok, if the LLMisms were cleaned up, I think this'd be good.

[rgoulter]

Hmm.

Working on the code:

1. Yeah, pressed_key_result_outcome really provided no value at this time.
2. Even ReplayCase isn't very clear. Just a 'simplified' KeyResolution is worth having..
3. ..But dispatch_replayed_events.. as lame as it is to have the enum and then disjoint impl, it's also 'lame' to have _resolved and _pending rather than an enum.

pending.rs has no unit tests. This is not-worse than the codebase currently is. There's good integration test coverage, and the current implementation is somewhat obscure (so may need to be changed anyway).