tap-hold pending_output (speculative Hold) by rgoulter · Pull Request #708 · rgoulter/smart-keymap

rgoulter · 2026-08-25T01:20:05Z

Implements speculative eager Hold for tap-hold (FAK eager_decision='hold' / ZMK hold-while-undecided / QMK Speculative Hold) behind a generic System::pending_output seam.

Stack

core: add System::pending_output hook — adds defaulted System::pending_output(&PendingKeyState) -> Option<KeyOutput> and includes it in Keymap's pressed_keys / aggregate_pressed_modifiers fold while a pending session is live. Keeps keymap.rs generic (no tap-hold type).
ncl: tap-hold pending_output authoring and codegen — adds pending_output (None|Hold) to Profile/Config, lowers flat config.tap_hold.pending_output to default_profile, generates composite System::pending_output that decodes the speculative hold ref (keyboard only; GUI LEFT_GUI/RIGHT_GUI refused via has_modifiers). Updates families.ncl to PendingKeyState<Ref>.
core: tap-hold pending_output profile and speculative state — PendingOutput enum, PendingKeyState<R> { speculative: Option<R> }, new_pending_key stores hold ref when pending_output=Hold.
test: refresh expected.rs snapshots — ncl/scripts/save-test-snapshots.sh.
test: add pending_output integration tests — 5 AAA tests in tests/rust/pending_output.rs (default silent, Hold+timeout, Hold+quick-release cancel, Hold+HoldOnKeyPress interrupt, GUI refusal); AAA prose is inline // Assemble --/// Act --/// Assert -- inside bodies.
test: add pending_output cucumber feature — tap_hold-config-pending_output.feature (4 scenarios).

Verification

cargo test --test rust-integration pending_output 5 passed
just test-fast 218 passed
just check-quick green (fmt, clippy, doc, nickel format)

Linger / same-mod ZMK tap deferred to follow-on.

Add a defaulted trait method System::pending_output(&PKS) -> Option<KeyOutput> so Keymap's HID fold can include speculative output without special-casing tap-hold in keymap.rs. The fold now includes pending_output in pressed_keys and aggregate_pressed_modifiers while a pending session is live. This is the generic seam for tap-hold Hold-while-undecided; other families keep the default None. Model: muse-spark-1.2 Agent: Muse Code Co-Authored-By: Muse Code powered by Meta Muse Spark <muse-code@meta.com> Co-Authored-By: muse-spark-1.2 <muse-spark@meta.com>

Add pending_output (None|Hold) to tap-hold Profile/Config, lower flat config.tap_hold.pending_output to default_profile, and generate composite System::pending_output that decodes the speculative hold ref (keyboard only, GUI refused). Update families to use PendingKeyState<Ref>. Model: muse-spark-1.2 Agent: Muse Code Co-Authored-By: Muse Code powered by Meta Muse Spark <muse-code@meta.com> Co-Authored-By: muse-spark-1.2 <muse-spark@meta.com>

Add PendingOutput (None|Hold) to Profile with serde default None, extend PendingKeyState<R> with speculative: Option<R>, and wire new_pending_key to store the hold ref when pending_output is Hold. GUI filtering is deferred to the composite pending_output decoder. Model: muse-spark-1.2 Agent: Muse Code Co-Authored-By: Muse Code powered by Meta Muse Spark <muse-code@meta.com> Co-Authored-By: muse-spark-1.2 <muse-spark@meta.com>

Generated via ncl/scripts/save-test-snapshots.sh after making PendingKeyState generic and adding System::pending_output. Model: muse-spark-1.2 Agent: Muse Code Co-Authored-By: Muse Code powered by Meta Muse Spark <muse-code@meta.com> Co-Authored-By: muse-spark-1.2 <muse-spark@meta.com>

Add tests/rust/pending_output.rs with 5 AAA tests: default silent, Hold+timeout, Hold+quick-release cancel, Hold+HoldOnKeyPress interrupt, and GUI refusal. AAA prose is inside each test body (Assemble/Act/Assert with -- description). Register the module in tests/rust/keymap.rs. Model: muse-spark-1.2 Agent: Muse Code Co-Authored-By: Muse Code powered by Meta Muse Spark <muse-code@meta.com> Co-Authored-By: muse-spark-1.2 <muse-spark@meta.com>

Add features/keymap/key/tap_hold-config-pending_output.feature with 4 scenarios (default silent, Hold+timeout, Hold+quick-release, Hold+HoldOnKeyPress) cross-linked to FAK eager_decision, ZMK hold-while-undecided, and QMK Speculative Hold. Model: muse-spark-1.2 Agent: Muse Code Co-Authored-By: Muse Code powered by Meta Muse Spark <muse-code@meta.com> Co-Authored-By: muse-spark-1.2 <muse-spark@meta.com>

rgoulter · 2026-08-25T01:22:18Z

+  The `pending_output` config for tap-hold keys controls speculative HID
+   while the tap-vs-hold decision is still pending.
+
+  Default `None` is today's silence (no HID until timeout / interrupt / release).


"today's silence" is an awkward phrase here.

rgoulter · 2026-08-25T01:23:12Z

+  `Hold` emits the hold binding's HID while still pending, then keeps it
+   or retracts it when tap vs hold settles. Decision logic is unchanged —
+   only when hold appears.
+
+  This is the same behaviour as FAK `eager_decision = 'hold`, ZMK
+   `hold-while-undecided`, and QMK Speculative Hold.


Emdash remains a sign of cringe. (LLMs still lack taste about what a good comment is).

Should help try rearrange to more 'semantic' linebreaks.

rgoulter · 2026-08-25T01:23:59Z

+  Example: Hold with quick release shows mod then cancels to tap without Ctrl+A flash
+
+    Given a keymap.ncl:
+      """
+      let K = import "keys.ncl" in
+      {
+        config.tap_hold.pending_output = "Hold",
+        keys = [ K.A & K.hold K.LeftCtrl ]
+      }
+      """
+    When the keymap registers the following input
+      """
+      [
+        press (K.A & K.hold K.LeftCtrl),
+        wait 1,
+        release (K.A & K.hold K.LeftCtrl),
+      ]
+      """
+    Then the HID keyboard report should equal
+      """
+      { key_codes = [K.A] }
+      """


That's not what this test demonstrates. (The hid report is about the last state).

rgoulter · 2026-08-25T01:25:16Z

+  Background:
+
+    Given a keymap.ncl:
+      """
+      let K = import "keys.ncl" in
+      {
+        keys = [
+          K.A & K.hold K.LeftCtrl,
+          K.B
+        ]
+      }
+      """
+
+  Example: default None stays silent while pending (regression)
+
+    When the keymap registers the following input
+      """
+      [
+        press (K.A & K.hold K.LeftCtrl),
+        wait 1,
+      ]
+      """
+    Then the HID keyboard report should equal
+      """
+      { modifiers = {} }
+      """


Background should have the Some config option. Can check that there's no output in a rust integration test, and otherwise assume it.

rgoulter · 2026-08-25T01:26:47Z

+                    if let Some(spec) = pks.speculative {
+                        match spec {
+                            Ref::Keyboard(kb_ref) => {
+                                let ko = self.keyboard.key_output(&kb_ref, &smart_keymap::key::keyboard::KeyState);
+                                if let Some(ko) = ko {
+                                    let mods = ko.key_modifiers();
+                                    if mods.has_modifiers(&smart_keymap::key::KeyboardModifiers::LEFT_GUI)
+                                        || mods.has_modifiers(&smart_keymap::key::KeyboardModifiers::RIGHT_GUI)
+                                    {
+                                        None
+                                    } else {
+                                        Some(ko)
+                                    }
+                                } else {
+                                    None
+                                }
+                            },
+                            _ => None,
+                        }
+                    } else {
+                        None
+                    }


Really would prefer this be in smart-keymap-core rather than Rust code.

rgoulter · 2026-08-25T01:27:44Z

+                return base.union(&ko.key_modifiers());
+            }
+        }
+        base


Prefer to try and keep this in the expression, rather than use return.

rgoulter · 2026-08-25T01:29:11Z

+    /// No speculative output (default).
+    None,


NoOutput would be better, because None otherwise strongly pairs with Some.

rgoulter · 2026-08-25T01:30:19Z

+        { keys = [ K.A & K.hold K.LeftCtrl ] }
+    "#
+    ));
+    // Act -- press TH


For readability / structure, there should be a blank line before the // Act and // Assert comments.

rgoulter · 2026-08-25T01:31:19Z

+    // Act -- timeout
+    for _ in 0..210 {
+        keymap.tick();
+    }
+    let reports2 = keymap.distinct_reports().reports().to_vec();
+    // Assert -- still hold
+    assert!(reports2.iter().any(|r| r[0] == 0x01));


Multiple // Act, // Assert is smelly. May be acceptable to have a saga of tests, but I'd want it clearer.

rgoulter · 2026-08-25T01:32:23Z

+    }
+    let reports2 = keymap.distinct_reports().reports().to_vec();
+    assert!(
+        reports2.iter().any(|r| r[0] == 0x01),


Use of 0x01 is unclear.

Perhaps out of scope for this PR, but this .any(|r| ...) is a common pattern across tests which could be factored out.

rgoulter · 2026-08-25T01:32:44Z

+    );
+    let has_flash = reports
+        .windows(2)
+        .any(|w| w[0][0] == 0x01 && w[1][0] == 0x01 && w[1][2] == KC_A);


This is unreadable.

rgoulter · 2026-08-25T01:34:03Z

        let suppress = self.context.suppressed_modifiers();
-        let pressed_key_codes = self.pressed_inputs.iter().filter_map(|pi| {
+        let mut outputs: heapless::Vec<key::KeyOutput, { MAX_PRESSED_KEYS }> = heapless::Vec::new();
+        for pi in self.pressed_inputs.iter() {


Prefer expression-based construction, rather than constructing with a for loop.

rgoulter · 2026-08-25T01:35:11Z

+    ///
+    /// Default: `None` (today's silence).
+    /// Only tap-hold implements it in v1.


"today's silence" is an awkward phrase.

It's awkward to mention which implementations there are. "v1" is also awkward.

rgoulter and others added 6 commits August 24, 2026 09:15

rgoulter commented Aug 25, 2026

View reviewed changes

Navigation Menu

tap-hold pending_output (speculative Hold) - #708

tap-hold pending_output (speculative Hold)#708
rgoulter wants to merge 6 commits into
masterfrom
pending-output

rgoulter commented Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

rgoulter Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

rgoulter commented Aug 25, 2026

Stack

Verification

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant