Fix regression preventing new predictions from being previewed in subtle mode (#51887) (cherry-pick to preview) (#52224)

zed-zippy[bot] and Ben Kunkle created

Cherry-pick of #51887 to preview

----
## Context

<!-- What does this PR do, and why? How is it expected to impact users?
     Not just what changed, but what motivated it and why this approach.

Link to Linear issue (e.g., ENG-123) or GitHub issue (e.g., Closes #456)
     if one exists β€” helps with traceability. -->
Fixes some issues with https://github.com/zed-industries/zed/pull/51842
Namely that the tests were scattered and not well organized (this PR
also makes them more thorough), and a regression where holding the
modifiers for the accept prediction keybind would not cause an incoming
prediction to be immediately previewed.

## How to Review

<!-- Help reviewers focus their attention:
- For small PRs: note what to focus on (e.g., "error handling in
foo.rs")
- For large PRs (>400 LOC): provide a guided tour β€” numbered list of
files/commits to read in order. (The `large-pr` label is applied
automatically.)
     - See the review process guidelines for comment conventions -->

## Self-Review Checklist

<!-- Check before requesting review: -->
- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX

checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- (Preview v0.229.x only) Fixed a regression where holding the modifiers
for the accept edit prediction keybind would not immediately preview
predictions as they arrived

Co-authored-by: Ben Kunkle <ben@zed.dev>

Change summary

assets/keymaps/default-linux.json          |   7 
assets/keymaps/default-macos.json          |   7 
assets/keymaps/default-windows.json        |   8 
assets/keymaps/vim.json                    |   2 
crates/editor/src/edit_prediction_tests.rs | 888 ++++++++++++++---------
crates/editor/src/editor.rs                |  57 +
6 files changed, 587 insertions(+), 382 deletions(-)

Detailed changes

assets/keymaps/default-linux.json πŸ”—

@@ -770,11 +770,16 @@
     "bindings": {
       "alt-tab": "editor::AcceptEditPrediction",
       "alt-l": "editor::AcceptEditPrediction",
-      "tab": "editor::AcceptEditPrediction",
       "alt-k": "editor::AcceptNextWordEditPrediction",
       "alt-j": "editor::AcceptNextLineEditPrediction",
     },
   },
+  {
+    "context": "Editor && edit_prediction && edit_prediction_mode == eager",
+    "bindings": {
+      "tab": "editor::AcceptEditPrediction",
+    },
+  },
   {
     "context": "Editor && showing_code_actions",
     "bindings": {

assets/keymaps/default-macos.json πŸ”—

@@ -834,11 +834,16 @@
     "context": "Editor && edit_prediction",
     "bindings": {
       "alt-tab": "editor::AcceptEditPrediction",
-      "tab": "editor::AcceptEditPrediction",
       "ctrl-cmd-right": "editor::AcceptNextWordEditPrediction",
       "ctrl-cmd-down": "editor::AcceptNextLineEditPrediction",
     },
   },
+  {
+    "context": "Editor && edit_prediction && edit_prediction_mode == eager",
+    "bindings": {
+      "tab": "editor::AcceptEditPrediction",
+    },
+  },
   {
     "context": "Editor && showing_code_actions",
     "use_key_equivalents": true,

assets/keymaps/default-windows.json πŸ”—

@@ -766,11 +766,17 @@
     "bindings": {
       "alt-tab": "editor::AcceptEditPrediction",
       "alt-l": "editor::AcceptEditPrediction",
-      "tab": "editor::AcceptEditPrediction",
       "alt-k": "editor::AcceptNextWordEditPrediction",
       "alt-j": "editor::AcceptNextLineEditPrediction",
     },
   },
+  {
+    "context": "Editor && edit_prediction && edit_prediction_mode == eager",
+    "use_key_equivalents": true,
+    "bindings": {
+      "tab": "editor::AcceptEditPrediction",
+    },
+  },
   {
     "context": "Editor && showing_code_actions",
     "use_key_equivalents": true,

assets/keymaps/vim.json πŸ”—

@@ -1060,7 +1060,7 @@
     },
   },
   {
-    "context": "Editor && edit_prediction",
+    "context": "Editor && edit_prediction && edit_prediction_mode == eager",
     "bindings": {
       // This is identical to the binding in the base keymap, but the vim bindings above to
       // "vim::Tab" shadow it, so it needs to be bound again.

crates/editor/src/edit_prediction_tests.rs πŸ”—

@@ -1,13 +1,17 @@
 use edit_prediction_types::{
     EditPredictionDelegate, EditPredictionIconSet, PredictedCursorPosition,
 };
-use gpui::{Entity, KeyBinding, Modifiers, prelude::*};
+use gpui::{
+    Entity, KeyBinding, KeybindingKeystroke, Keystroke, Modifiers, NoAction, Task, prelude::*,
+};
 use indoc::indoc;
-use language::Buffer;
 use language::EditPredictionsMode;
-use multi_buffer::{Anchor, MultiBufferSnapshot, ToPoint};
+use language::{Buffer, CodeLabel};
+use multi_buffer::{Anchor, ExcerptId, MultiBufferSnapshot, ToPoint};
+use project::{Completion, CompletionResponse, CompletionSource};
 use std::{
     ops::Range,
+    rc::Rc,
     sync::{
         Arc,
         atomic::{self, AtomicUsize},
@@ -17,8 +21,9 @@ use text::{Point, ToOffset};
 use ui::prelude::*;
 
 use crate::{
-    AcceptEditPrediction, EditPrediction, EditPredictionKeybindAction,
-    EditPredictionKeybindSurface, MenuEditPredictionsPolicy,
+    AcceptEditPrediction, CompletionContext, CompletionProvider, EditPrediction,
+    EditPredictionKeybindAction, EditPredictionKeybindSurface, MenuEditPredictionsPolicy,
+    ShowCompletions,
     editor_tests::{init_test, update_test_language_settings},
     test::editor_test_context::EditorTestContext,
 };
@@ -482,57 +487,10 @@ async fn test_edit_prediction_preview_cleanup_on_toggle_off(cx: &mut gpui::TestA
     });
 }
 
-fn load_default_keymap(cx: &mut gpui::TestAppContext) {
-    cx.update(|cx| {
-        cx.bind_keys(
-            settings::KeymapFile::load_asset_allow_partial_failure(
-                settings::DEFAULT_KEYMAP_PATH,
-                cx,
-            )
-            .expect("failed to load default keymap"),
-        );
-    });
-}
-
 #[gpui::test]
-async fn test_tab_is_preferred_accept_binding_over_alt_tab(cx: &mut gpui::TestAppContext) {
-    init_test(cx, |_| {});
-    load_default_keymap(cx);
-
-    let mut cx = EditorTestContext::new(cx).await;
-    let provider = cx.new(|_| FakeEditPredictionDelegate::default());
-    assign_editor_completion_provider(provider.clone(), &mut cx);
-    cx.set_state("let x = Λ‡;");
-
-    propose_edits(&provider, vec![(8..8, "42")], &mut cx);
-    cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
-
-    cx.update_editor(|editor, window, cx| {
-        assert!(editor.has_active_edit_prediction());
-        let keybind_display = editor.edit_prediction_keybind_display(
-            EditPredictionKeybindSurface::Inline,
-            window,
-            cx,
-        );
-        let keystroke = keybind_display
-            .accept_keystroke
-            .as_ref()
-            .expect("should have an accept binding");
-        assert!(
-            !keystroke.modifiers().modified(),
-            "preferred accept binding should be unmodified (tab), got modifiers: {:?}",
-            keystroke.modifiers()
-        );
-        assert_eq!(
-            keystroke.key(),
-            "tab",
-            "preferred accept binding should be tab"
-        );
-    });
-}
-
-#[gpui::test]
-async fn test_subtle_in_code_indicator_prefers_preview_binding(cx: &mut gpui::TestAppContext) {
+async fn test_edit_prediction_preview_activates_when_prediction_arrives_with_modifier_held(
+    cx: &mut gpui::TestAppContext,
+) {
     init_test(cx, |_| {});
     load_default_keymap(cx);
     update_test_language_settings(cx, &|settings| {
@@ -544,227 +502,324 @@ async fn test_subtle_in_code_indicator_prefers_preview_binding(cx: &mut gpui::Te
     assign_editor_completion_provider(provider.clone(), &mut cx);
     cx.set_state("let x = Λ‡;");
 
-    propose_edits(&provider, vec![(8..8, "42")], &mut cx);
-    cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
-
-    cx.update_editor(|editor, window, cx| {
-        assert!(editor.has_active_edit_prediction());
-        assert!(
-            editor.edit_prediction_requires_modifier(),
-            "subtle mode should require a modifier"
-        );
-
-        let inline_keybind_display = editor.edit_prediction_keybind_display(
-            EditPredictionKeybindSurface::Inline,
-            window,
-            cx,
-        );
-        let compact_keybind_display = editor.edit_prediction_keybind_display(
-            EditPredictionKeybindSurface::CursorPopoverCompact,
-            window,
-            cx,
-        );
-
-        let accept_keystroke = inline_keybind_display
-            .accept_keystroke
-            .as_ref()
-            .expect("should have an accept binding");
-        let preview_keystroke = inline_keybind_display
-            .preview_keystroke
-            .as_ref()
-            .expect("should have a preview binding");
-        let in_code_keystroke = inline_keybind_display
-            .displayed_keystroke
-            .as_ref()
-            .expect("should have an in-code binding");
-        let compact_cursor_popover_keystroke = compact_keybind_display
-            .displayed_keystroke
-            .as_ref()
-            .expect("should have a compact cursor popover binding");
-
-        assert_eq!(accept_keystroke.key(), "tab");
-        assert!(
-            !editor.has_visible_completions_menu(),
-            "compact cursor-popover branch should be used without a completions menu"
-        );
-        assert!(
-            preview_keystroke.modifiers().modified(),
-            "preview binding should use modifiers in subtle mode"
-        );
-        assert_eq!(
-            compact_cursor_popover_keystroke.key(),
-            preview_keystroke.key(),
-            "subtle compact cursor popover should prefer the preview binding"
-        );
-        assert_eq!(
-            compact_cursor_popover_keystroke.modifiers(),
-            preview_keystroke.modifiers(),
-            "subtle compact cursor popover should use the preview binding modifiers"
-        );
-        assert_eq!(
-            in_code_keystroke.key(),
-            preview_keystroke.key(),
-            "subtle in-code indicator should prefer the preview binding"
-        );
-        assert_eq!(
-            in_code_keystroke.modifiers(),
-            preview_keystroke.modifiers(),
-            "subtle in-code indicator should use the preview binding modifiers"
-        );
+    cx.editor(|editor, _, _| {
+        assert!(!editor.has_active_edit_prediction());
+        assert!(!editor.edit_prediction_preview_is_active());
     });
-}
 
-#[gpui::test]
-async fn test_tab_accepts_edit_prediction_over_completion(cx: &mut gpui::TestAppContext) {
-    init_test(cx, |_| {});
-    load_default_keymap(cx);
-
-    let mut cx = EditorTestContext::new(cx).await;
-    let provider = cx.new(|_| FakeEditPredictionDelegate::default());
-    assign_editor_completion_provider(provider.clone(), &mut cx);
-    cx.set_state("let x = Λ‡;");
-
-    propose_edits(&provider, vec![(8..8, "42")], &mut cx);
-    cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
-
-    assert_editor_active_edit_completion(&mut cx, |_, edits| {
-        assert_eq!(edits.len(), 1);
-        assert_eq!(edits[0].1.as_ref(), "42");
+    let preview_modifiers = cx.update_editor(|editor, window, cx| {
+        *editor
+            .preview_edit_prediction_keystroke(window, cx)
+            .unwrap()
+            .modifiers()
     });
 
-    cx.simulate_keystroke("tab");
+    cx.simulate_modifiers_change(preview_modifiers);
     cx.run_until_parked();
 
-    cx.assert_editor_state("let x = 42Λ‡;");
-}
-
-#[gpui::test]
-async fn test_single_line_prediction_uses_accept_cursor_popover_action(
-    cx: &mut gpui::TestAppContext,
-) {
-    init_test(cx, |_| {});
-    load_default_keymap(cx);
-
-    let mut cx = EditorTestContext::new(cx).await;
-    let provider = cx.new(|_| FakeEditPredictionDelegate::default());
-    assign_editor_completion_provider(provider.clone(), &mut cx);
-    cx.set_state("let x = Λ‡;");
+    cx.editor(|editor, _, _| {
+        assert!(!editor.has_active_edit_prediction());
+        assert!(editor.edit_prediction_preview_is_active());
+    });
 
     propose_edits(&provider, vec![(8..8, "42")], &mut cx);
-    cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
-
     cx.update_editor(|editor, window, cx| {
-        assert!(editor.has_active_edit_prediction());
+        editor.set_menu_edit_predictions_policy(MenuEditPredictionsPolicy::ByProvider);
+        editor.update_visible_edit_prediction(window, cx)
+    });
 
-        let keybind_display = editor.edit_prediction_keybind_display(
-            EditPredictionKeybindSurface::CursorPopoverExpanded,
-            window,
-            cx,
+    cx.editor(|editor, _, _| {
+        assert!(editor.has_active_edit_prediction());
+        assert!(
+            editor.edit_prediction_preview_is_active(),
+            "prediction preview should activate immediately when the prediction arrives while the preview modifier is still held",
         );
+    });
+}
 
-        let accept_keystroke = keybind_display
-            .accept_keystroke
-            .as_ref()
-            .expect("should have an accept binding");
-        let preview_keystroke = keybind_display
-            .preview_keystroke
-            .as_ref()
-            .expect("should have a preview binding");
-
-        assert_eq!(
-            keybind_display.action,
-            EditPredictionKeybindAction::Accept,
-            "single-line prediction should show the accept action"
+fn load_default_keymap(cx: &mut gpui::TestAppContext) {
+    cx.update(|cx| {
+        cx.bind_keys(
+            settings::KeymapFile::load_asset_allow_partial_failure(
+                settings::DEFAULT_KEYMAP_PATH,
+                cx,
+            )
+            .expect("failed to load default keymap"),
         );
-        assert_eq!(accept_keystroke.key(), "tab");
-        assert!(preview_keystroke.modifiers().modified());
     });
 }
 
 #[gpui::test]
-async fn test_multi_line_prediction_uses_preview_cursor_popover_action(
-    cx: &mut gpui::TestAppContext,
-) {
-    init_test(cx, |_| {});
-    load_default_keymap(cx);
-
-    let mut cx = EditorTestContext::new(cx).await;
-    let provider = cx.new(|_| FakeEditPredictionDelegate::default());
-    assign_editor_completion_provider(provider.clone(), &mut cx);
-    cx.set_state("let x = Λ‡;");
-
-    propose_edits(&provider, vec![(8..8, "42\n43")], &mut cx);
-    cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
-
-    cx.update_editor(|editor, window, cx| {
-        assert!(editor.has_active_edit_prediction());
+async fn test_inline_edit_prediction_keybind_selection_cases(cx: &mut gpui::TestAppContext) {
+    enum InlineKeybindState {
+        Normal,
+        ShowingCompletions,
+        InLeadingWhitespace,
+        ShowingCompletionsAndLeadingWhitespace,
+    }
 
-        let keybind_display = editor.edit_prediction_keybind_display(
-            EditPredictionKeybindSurface::CursorPopoverExpanded,
-            window,
-            cx,
-        );
-        let preview_keystroke = keybind_display
-            .preview_keystroke
-            .as_ref()
-            .expect("should have a preview binding");
+    enum ExpectedKeystroke {
+        DefaultAccept,
+        DefaultPreview,
+        Literal(&'static str),
+    }
 
-        assert_eq!(
-            keybind_display.action,
-            EditPredictionKeybindAction::Preview,
-            "multi-line prediction should show the preview action"
-        );
-        assert!(preview_keystroke.modifiers().modified());
-    });
-}
+    struct InlineKeybindCase {
+        name: &'static str,
+        use_default_keymap: bool,
+        mode: EditPredictionsMode,
+        extra_bindings: Vec<KeyBinding>,
+        state: InlineKeybindState,
+        expected_accept_keystroke: ExpectedKeystroke,
+        expected_preview_keystroke: ExpectedKeystroke,
+        expected_displayed_keystroke: ExpectedKeystroke,
+    }
 
-#[gpui::test]
-async fn test_single_line_prediction_with_preview_uses_accept_cursor_popover_action(
-    cx: &mut gpui::TestAppContext,
-) {
     init_test(cx, |_| {});
     load_default_keymap(cx);
+    let mut default_cx = EditorTestContext::new(cx).await;
+    let provider = default_cx.new(|_| FakeEditPredictionDelegate::default());
+    assign_editor_completion_provider(provider.clone(), &mut default_cx);
+    default_cx.set_state("let x = Λ‡;");
+    propose_edits(&provider, vec![(8..8, "42")], &mut default_cx);
+    default_cx
+        .update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
+
+    let (default_accept_keystroke, default_preview_keystroke) =
+        default_cx.update_editor(|editor, window, cx| {
+            let keybind_display = editor.edit_prediction_keybind_display(
+                EditPredictionKeybindSurface::Inline,
+                window,
+                cx,
+            );
+            let accept_keystroke = keybind_display
+                .accept_keystroke
+                .as_ref()
+                .expect("default inline edit prediction should have an accept binding")
+                .clone();
+            let preview_keystroke = keybind_display
+                .preview_keystroke
+                .as_ref()
+                .expect("default inline edit prediction should have a preview binding")
+                .clone();
+            (accept_keystroke, preview_keystroke)
+        });
+
+    let cases = [
+        InlineKeybindCase {
+            name: "default setup prefers tab over alt-tab for accept",
+            use_default_keymap: true,
+            mode: EditPredictionsMode::Eager,
+            extra_bindings: Vec::new(),
+            state: InlineKeybindState::Normal,
+            expected_accept_keystroke: ExpectedKeystroke::DefaultAccept,
+            expected_preview_keystroke: ExpectedKeystroke::DefaultPreview,
+            expected_displayed_keystroke: ExpectedKeystroke::DefaultAccept,
+        },
+        InlineKeybindCase {
+            name: "subtle mode displays preview binding inline",
+            use_default_keymap: true,
+            mode: EditPredictionsMode::Subtle,
+            extra_bindings: Vec::new(),
+            state: InlineKeybindState::Normal,
+            expected_accept_keystroke: ExpectedKeystroke::DefaultPreview,
+            expected_preview_keystroke: ExpectedKeystroke::DefaultPreview,
+            expected_displayed_keystroke: ExpectedKeystroke::DefaultPreview,
+        },
+        InlineKeybindCase {
+            name: "removing default tab binding still displays tab",
+            use_default_keymap: true,
+            mode: EditPredictionsMode::Eager,
+            extra_bindings: vec![KeyBinding::new(
+                "tab",
+                NoAction,
+                Some("Editor && edit_prediction && edit_prediction_mode == eager"),
+            )],
+            state: InlineKeybindState::Normal,
+            expected_accept_keystroke: ExpectedKeystroke::DefaultPreview,
+            expected_preview_keystroke: ExpectedKeystroke::DefaultPreview,
+            expected_displayed_keystroke: ExpectedKeystroke::DefaultPreview,
+        },
+        InlineKeybindCase {
+            name: "custom-only rebound accept key uses replacement key",
+            use_default_keymap: true,
+            mode: EditPredictionsMode::Eager,
+            extra_bindings: vec![KeyBinding::new(
+                "ctrl-enter",
+                AcceptEditPrediction,
+                Some("Editor && edit_prediction"),
+            )],
+            state: InlineKeybindState::Normal,
+            expected_accept_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+            expected_preview_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+            expected_displayed_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+        },
+        InlineKeybindCase {
+            name: "showing completions restores conflict-context binding",
+            use_default_keymap: true,
+            mode: EditPredictionsMode::Eager,
+            extra_bindings: vec![KeyBinding::new(
+                "ctrl-enter",
+                AcceptEditPrediction,
+                Some("Editor && edit_prediction && showing_completions"),
+            )],
+            state: InlineKeybindState::ShowingCompletions,
+            expected_accept_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+            expected_preview_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+            expected_displayed_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+        },
+        InlineKeybindCase {
+            name: "leading whitespace restores conflict-context binding",
+            use_default_keymap: false,
+            mode: EditPredictionsMode::Eager,
+            extra_bindings: vec![KeyBinding::new(
+                "ctrl-enter",
+                AcceptEditPrediction,
+                Some("Editor && edit_prediction && in_leading_whitespace"),
+            )],
+            state: InlineKeybindState::InLeadingWhitespace,
+            expected_accept_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+            expected_preview_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+            expected_displayed_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+        },
+        InlineKeybindCase {
+            name: "showing completions and leading whitespace restore combined conflict binding",
+            use_default_keymap: false,
+            mode: EditPredictionsMode::Eager,
+            extra_bindings: vec![KeyBinding::new(
+                "ctrl-enter",
+                AcceptEditPrediction,
+                Some("Editor && edit_prediction && showing_completions && in_leading_whitespace"),
+            )],
+            state: InlineKeybindState::ShowingCompletionsAndLeadingWhitespace,
+            expected_accept_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+            expected_preview_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+            expected_displayed_keystroke: ExpectedKeystroke::Literal("ctrl-enter"),
+        },
+    ];
+
+    for case in cases {
+        init_test(cx, |_| {});
+        if case.use_default_keymap {
+            load_default_keymap(cx);
+        }
+        update_test_language_settings(cx, &|settings| {
+            settings.edit_predictions.get_or_insert_default().mode = Some(case.mode);
+        });
 
-    let mut cx = EditorTestContext::new(cx).await;
-    let provider = cx.new(|_| FakeEditPredictionDelegate::default());
-    assign_editor_completion_provider(provider.clone(), &mut cx);
-    cx.set_state("let x = Λ‡;");
+        if !case.extra_bindings.is_empty() {
+            cx.update(|cx| cx.bind_keys(case.extra_bindings.clone()));
+        }
 
-    propose_edits_with_preview(&provider, vec![(8..8, "42")], &mut cx).await;
-    cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
+        let mut cx = EditorTestContext::new(cx).await;
+        let provider = cx.new(|_| FakeEditPredictionDelegate::default());
+        assign_editor_completion_provider(provider.clone(), &mut cx);
 
-    cx.update_editor(|editor, window, cx| {
-        assert!(editor.has_active_edit_prediction());
+        match case.state {
+            InlineKeybindState::Normal | InlineKeybindState::ShowingCompletions => {
+                cx.set_state("let x = Λ‡;");
+            }
+            InlineKeybindState::InLeadingWhitespace
+            | InlineKeybindState::ShowingCompletionsAndLeadingWhitespace => {
+                cx.set_state(indoc! {"
+                    fn main() {
+                        Λ‡
+                    }
+                "});
+            }
+        }
 
-        let keybind_display = editor.edit_prediction_keybind_display(
-            EditPredictionKeybindSurface::CursorPopoverExpanded,
-            window,
-            cx,
-        );
+        propose_edits(&provider, vec![(8..8, "42")], &mut cx);
+        cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
+
+        if matches!(
+            case.state,
+            InlineKeybindState::ShowingCompletions
+                | InlineKeybindState::ShowingCompletionsAndLeadingWhitespace
+        ) {
+            assign_editor_completion_menu_provider(&mut cx);
+            cx.update_editor(|editor, window, cx| {
+                editor.show_completions(&ShowCompletions, window, cx);
+            });
+            cx.run_until_parked();
+        }
 
-        let accept_keystroke = keybind_display
-            .accept_keystroke
-            .as_ref()
-            .expect("should have an accept binding");
-        let preview_keystroke = keybind_display
-            .preview_keystroke
-            .as_ref()
-            .expect("should have a preview binding");
+        cx.update_editor(|editor, window, cx| {
+            assert!(
+                editor.has_active_edit_prediction(),
+                "case '{}' should have an active edit prediction",
+                case.name
+            );
 
-        assert_eq!(
-            keybind_display.action,
-            EditPredictionKeybindAction::Accept,
-            "single-line prediction should show the accept action even with edit_preview"
-        );
-        assert_eq!(accept_keystroke.key(), "tab");
-        assert!(preview_keystroke.modifiers().modified());
-    });
+            let keybind_display = editor.edit_prediction_keybind_display(
+                EditPredictionKeybindSurface::Inline,
+                window,
+                cx,
+            );
+            let accept_keystroke = keybind_display
+                .accept_keystroke
+                .as_ref()
+                .unwrap_or_else(|| panic!("case '{}' should have an accept binding", case.name));
+            let preview_keystroke = keybind_display
+                .preview_keystroke
+                .as_ref()
+                .unwrap_or_else(|| panic!("case '{}' should have a preview binding", case.name));
+            let displayed_keystroke = keybind_display
+                .displayed_keystroke
+                .as_ref()
+                .unwrap_or_else(|| panic!("case '{}' should have a displayed binding", case.name));
+
+            let expected_accept_keystroke = match case.expected_accept_keystroke {
+                ExpectedKeystroke::DefaultAccept => default_accept_keystroke.clone(),
+                ExpectedKeystroke::DefaultPreview => default_preview_keystroke.clone(),
+                ExpectedKeystroke::Literal(keystroke) => KeybindingKeystroke::from_keystroke(
+                    Keystroke::parse(keystroke).expect("expected test keystroke to parse"),
+                ),
+            };
+            let expected_preview_keystroke = match case.expected_preview_keystroke {
+                ExpectedKeystroke::DefaultAccept => default_accept_keystroke.clone(),
+                ExpectedKeystroke::DefaultPreview => default_preview_keystroke.clone(),
+                ExpectedKeystroke::Literal(keystroke) => KeybindingKeystroke::from_keystroke(
+                    Keystroke::parse(keystroke).expect("expected test keystroke to parse"),
+                ),
+            };
+            let expected_displayed_keystroke = match case.expected_displayed_keystroke {
+                ExpectedKeystroke::DefaultAccept => default_accept_keystroke.clone(),
+                ExpectedKeystroke::DefaultPreview => default_preview_keystroke.clone(),
+                ExpectedKeystroke::Literal(keystroke) => KeybindingKeystroke::from_keystroke(
+                    Keystroke::parse(keystroke).expect("expected test keystroke to parse"),
+                ),
+            };
+
+            assert_eq!(
+                accept_keystroke, &expected_accept_keystroke,
+                "case '{}' selected the wrong accept binding",
+                case.name
+            );
+            assert_eq!(
+                preview_keystroke, &expected_preview_keystroke,
+                "case '{}' selected the wrong preview binding",
+                case.name
+            );
+            assert_eq!(
+                displayed_keystroke, &expected_displayed_keystroke,
+                "case '{}' selected the wrong displayed binding",
+                case.name
+            );
+
+            if matches!(case.mode, EditPredictionsMode::Subtle) {
+                assert!(
+                    editor.edit_prediction_requires_modifier(),
+                    "case '{}' should require a modifier",
+                    case.name
+                );
+            }
+        });
+    }
 }
 
 #[gpui::test]
-async fn test_multi_line_prediction_with_preview_uses_preview_cursor_popover_action(
-    cx: &mut gpui::TestAppContext,
-) {
+async fn test_tab_accepts_edit_prediction_over_completion(cx: &mut gpui::TestAppContext) {
     init_test(cx, |_| {});
     load_default_keymap(cx);
 
@@ -773,131 +828,194 @@ async fn test_multi_line_prediction_with_preview_uses_preview_cursor_popover_act
     assign_editor_completion_provider(provider.clone(), &mut cx);
     cx.set_state("let x = Λ‡;");
 
-    propose_edits_with_preview(&provider, vec![(8..8, "42\n43")], &mut cx).await;
+    propose_edits(&provider, vec![(8..8, "42")], &mut cx);
     cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
-    cx.update_editor(|editor, window, cx| {
-        assert!(editor.has_active_edit_prediction());
 
-        let keybind_display = editor.edit_prediction_keybind_display(
-            EditPredictionKeybindSurface::CursorPopoverExpanded,
-            window,
-            cx,
-        );
-        let preview_keystroke = keybind_display
-            .preview_keystroke
-            .as_ref()
-            .expect("should have a preview binding");
-
-        assert_eq!(
-            keybind_display.action,
-            EditPredictionKeybindAction::Preview,
-            "multi-line prediction should show the preview action with edit_preview"
-        );
-        assert!(preview_keystroke.modifiers().modified());
+    assert_editor_active_edit_completion(&mut cx, |_, edits| {
+        assert_eq!(edits.len(), 1);
+        assert_eq!(edits[0].1.as_ref(), "42");
     });
-}
-
-#[gpui::test]
-async fn test_single_line_deletion_of_newline_uses_accept_cursor_popover_action(
-    cx: &mut gpui::TestAppContext,
-) {
-    init_test(cx, |_| {});
-    load_default_keymap(cx);
-
-    let mut cx = EditorTestContext::new(cx).await;
-    let provider = cx.new(|_| FakeEditPredictionDelegate::default());
-    assign_editor_completion_provider(provider.clone(), &mut cx);
-    cx.set_state(indoc! {"
-        fn main() {
-            let value = 1;
-            Λ‡println!(\"done\");
-        }
-    "});
-
-    propose_edits(
-        &provider,
-        vec![(Point::new(1, 18)..Point::new(2, 17), "")],
-        &mut cx,
-    );
-    cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
-
-    cx.update_editor(|editor, window, cx| {
-        assert!(editor.has_active_edit_prediction());
 
-        let keybind_display = editor.edit_prediction_keybind_display(
-            EditPredictionKeybindSurface::CursorPopoverExpanded,
-            window,
-            cx,
-        );
-
-        let accept_keystroke = keybind_display
-            .accept_keystroke
-            .as_ref()
-            .expect("should have an accept binding");
-        let preview_keystroke = keybind_display
-            .preview_keystroke
-            .as_ref()
-            .expect("should have a preview binding");
+    cx.simulate_keystroke("tab");
+    cx.run_until_parked();
 
-        assert_eq!(
-            keybind_display.action,
-            EditPredictionKeybindAction::Accept,
-            "deleting one newline plus adjacent text should show the accept action"
-        );
-        assert_eq!(accept_keystroke.key(), "tab");
-        assert!(preview_keystroke.modifiers().modified());
-    });
+    cx.assert_editor_state("let x = 42Λ‡;");
 }
 
 #[gpui::test]
-async fn test_stale_single_line_prediction_does_not_force_preview_cursor_popover_action(
-    cx: &mut gpui::TestAppContext,
-) {
-    init_test(cx, |_| {});
-    load_default_keymap(cx);
-
-    let mut cx = EditorTestContext::new(cx).await;
-    let provider = cx.new(|_| FakeEditPredictionDelegate::default());
-    assign_editor_completion_provider(provider.clone(), &mut cx);
-    cx.set_state("let x = Λ‡;");
-
-    propose_edits(&provider, vec![(8..8, "42\n43")], &mut cx);
-    cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
-    cx.update_editor(|editor, _window, cx| {
-        assert!(editor.active_edit_prediction.is_some());
-        assert!(editor.stale_edit_prediction_in_menu.is_none());
-        editor.take_active_edit_prediction(cx);
-        assert!(editor.active_edit_prediction.is_none());
-        assert!(editor.stale_edit_prediction_in_menu.is_some());
-    });
+async fn test_cursor_popover_edit_prediction_keybind_cases(cx: &mut gpui::TestAppContext) {
+    enum CursorPopoverPredictionKind {
+        SingleLine,
+        MultiLine,
+        SingleLineWithPreview,
+        MultiLineWithPreview,
+        DeleteSingleNewline,
+        StaleSingleLineAfterMultiLine,
+    }
 
-    propose_edits(&provider, vec![(8..8, "42")], &mut cx);
-    cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx));
+    struct CursorPopoverCase {
+        name: &'static str,
+        prediction_kind: CursorPopoverPredictionKind,
+        expected_action: EditPredictionKeybindAction,
+    }
 
-    cx.update_editor(|editor, window, cx| {
-        assert!(editor.has_active_edit_prediction());
+    let cases = [
+        CursorPopoverCase {
+            name: "single line prediction uses accept action",
+            prediction_kind: CursorPopoverPredictionKind::SingleLine,
+            expected_action: EditPredictionKeybindAction::Accept,
+        },
+        CursorPopoverCase {
+            name: "multi line prediction uses preview action",
+            prediction_kind: CursorPopoverPredictionKind::MultiLine,
+            expected_action: EditPredictionKeybindAction::Preview,
+        },
+        CursorPopoverCase {
+            name: "single line prediction with preview still uses accept action",
+            prediction_kind: CursorPopoverPredictionKind::SingleLineWithPreview,
+            expected_action: EditPredictionKeybindAction::Accept,
+        },
+        CursorPopoverCase {
+            name: "multi line prediction with preview uses preview action",
+            prediction_kind: CursorPopoverPredictionKind::MultiLineWithPreview,
+            expected_action: EditPredictionKeybindAction::Preview,
+        },
+        CursorPopoverCase {
+            name: "single line newline deletion uses accept action",
+            prediction_kind: CursorPopoverPredictionKind::DeleteSingleNewline,
+            expected_action: EditPredictionKeybindAction::Accept,
+        },
+        CursorPopoverCase {
+            name: "stale multi line prediction does not force preview action",
+            prediction_kind: CursorPopoverPredictionKind::StaleSingleLineAfterMultiLine,
+            expected_action: EditPredictionKeybindAction::Accept,
+        },
+    ];
+
+    for case in cases {
+        init_test(cx, |_| {});
+        load_default_keymap(cx);
+
+        let mut cx = EditorTestContext::new(cx).await;
+        let provider = cx.new(|_| FakeEditPredictionDelegate::default());
+        assign_editor_completion_provider(provider.clone(), &mut cx);
+
+        match case.prediction_kind {
+            CursorPopoverPredictionKind::SingleLine => {
+                cx.set_state("let x = Λ‡;");
+                propose_edits(&provider, vec![(8..8, "42")], &mut cx);
+                cx.update_editor(|editor, window, cx| {
+                    editor.update_visible_edit_prediction(window, cx)
+                });
+            }
+            CursorPopoverPredictionKind::MultiLine => {
+                cx.set_state("let x = Λ‡;");
+                propose_edits(&provider, vec![(8..8, "42\n43")], &mut cx);
+                cx.update_editor(|editor, window, cx| {
+                    editor.update_visible_edit_prediction(window, cx)
+                });
+            }
+            CursorPopoverPredictionKind::SingleLineWithPreview => {
+                cx.set_state("let x = Λ‡;");
+                propose_edits_with_preview(&provider, vec![(8..8, "42")], &mut cx).await;
+                cx.update_editor(|editor, window, cx| {
+                    editor.update_visible_edit_prediction(window, cx)
+                });
+            }
+            CursorPopoverPredictionKind::MultiLineWithPreview => {
+                cx.set_state("let x = Λ‡;");
+                propose_edits_with_preview(&provider, vec![(8..8, "42\n43")], &mut cx).await;
+                cx.update_editor(|editor, window, cx| {
+                    editor.update_visible_edit_prediction(window, cx)
+                });
+            }
+            CursorPopoverPredictionKind::DeleteSingleNewline => {
+                cx.set_state(indoc! {"
+                    fn main() {
+                        let value = 1;
+                        Λ‡println!(\"done\");
+                    }
+                "});
+                propose_edits(
+                    &provider,
+                    vec![(Point::new(1, 18)..Point::new(2, 17), "")],
+                    &mut cx,
+                );
+                cx.update_editor(|editor, window, cx| {
+                    editor.update_visible_edit_prediction(window, cx)
+                });
+            }
+            CursorPopoverPredictionKind::StaleSingleLineAfterMultiLine => {
+                cx.set_state("let x = Λ‡;");
+                propose_edits(&provider, vec![(8..8, "42\n43")], &mut cx);
+                cx.update_editor(|editor, window, cx| {
+                    editor.update_visible_edit_prediction(window, cx)
+                });
+                cx.update_editor(|editor, _window, cx| {
+                    assert!(editor.active_edit_prediction.is_some());
+                    assert!(editor.stale_edit_prediction_in_menu.is_none());
+                    editor.take_active_edit_prediction(cx);
+                    assert!(editor.active_edit_prediction.is_none());
+                    assert!(editor.stale_edit_prediction_in_menu.is_some());
+                });
+
+                propose_edits(&provider, vec![(8..8, "42")], &mut cx);
+                cx.update_editor(|editor, window, cx| {
+                    editor.update_visible_edit_prediction(window, cx)
+                });
+            }
+        }
 
-        let keybind_display = editor.edit_prediction_keybind_display(
-            EditPredictionKeybindSurface::CursorPopoverExpanded,
-            window,
-            cx,
-        );
-        let accept_keystroke = keybind_display
-            .accept_keystroke
-            .as_ref()
-            .expect("should have an accept binding");
+        cx.update_editor(|editor, window, cx| {
+            assert!(
+                editor.has_active_edit_prediction(),
+                "case '{}' should have an active edit prediction",
+                case.name
+            );
 
-        assert_eq!(
-            keybind_display.action,
-            EditPredictionKeybindAction::Accept,
-            "single-line active prediction should show the accept action"
-        );
-        assert!(
-            editor.stale_edit_prediction_in_menu.is_none(),
-            "refreshing the visible prediction should clear stale menu state"
-        );
-        assert_eq!(accept_keystroke.key(), "tab");
-    });
+            let keybind_display = editor.edit_prediction_keybind_display(
+                EditPredictionKeybindSurface::CursorPopoverExpanded,
+                window,
+                cx,
+            );
+            let accept_keystroke = keybind_display
+                .accept_keystroke
+                .as_ref()
+                .unwrap_or_else(|| panic!("case '{}' should have an accept binding", case.name));
+            let preview_keystroke = keybind_display
+                .preview_keystroke
+                .as_ref()
+                .unwrap_or_else(|| panic!("case '{}' should have a preview binding", case.name));
+
+            assert_eq!(
+                keybind_display.action, case.expected_action,
+                "case '{}' selected the wrong cursor popover action",
+                case.name
+            );
+            assert_eq!(
+                accept_keystroke.key(),
+                "tab",
+                "case '{}' selected the wrong accept binding",
+                case.name
+            );
+            assert!(
+                preview_keystroke.modifiers().modified(),
+                "case '{}' should use a modified preview binding",
+                case.name
+            );
+
+            if matches!(
+                case.prediction_kind,
+                CursorPopoverPredictionKind::StaleSingleLineAfterMultiLine
+            ) {
+                assert!(
+                    editor.stale_edit_prediction_in_menu.is_none(),
+                    "case '{}' should clear stale menu state",
+                    case.name
+                );
+            }
+        });
+    }
 }
 
 fn assert_editor_active_edit_completion(
@@ -1054,6 +1172,12 @@ fn assign_editor_completion_provider(
     })
 }
 
+fn assign_editor_completion_menu_provider(cx: &mut EditorTestContext) {
+    cx.update_editor(|editor, _, _| {
+        editor.set_completion_provider(Some(Rc::new(FakeCompletionMenuProvider)));
+    });
+}
+
 fn propose_edits_non_zed<T: ToOffset>(
     provider: &Entity<FakeNonZedEditPredictionDelegate>,
     edits: Vec<(Range<T>, &str)>,
@@ -1086,6 +1210,54 @@ fn assign_editor_completion_provider_non_zed(
     })
 }
 
+struct FakeCompletionMenuProvider;
+
+impl CompletionProvider for FakeCompletionMenuProvider {
+    fn completions(
+        &self,
+        _excerpt_id: ExcerptId,
+        _buffer: &Entity<Buffer>,
+        _buffer_position: text::Anchor,
+        _trigger: CompletionContext,
+        _window: &mut Window,
+        _cx: &mut Context<crate::Editor>,
+    ) -> Task<anyhow::Result<Vec<CompletionResponse>>> {
+        let completion = Completion {
+            replace_range: text::Anchor::MIN..text::Anchor::MAX,
+            new_text: "fake_completion".to_string(),
+            label: CodeLabel::plain("fake_completion".to_string(), None),
+            documentation: None,
+            source: CompletionSource::Custom,
+            icon_path: None,
+            match_start: None,
+            snippet_deduplication_key: None,
+            insert_text_mode: None,
+            confirm: None,
+        };
+
+        Task::ready(Ok(vec![CompletionResponse {
+            completions: vec![completion],
+            display_options: Default::default(),
+            is_incomplete: false,
+        }]))
+    }
+
+    fn is_completion_trigger(
+        &self,
+        _buffer: &Entity<Buffer>,
+        _position: language::Anchor,
+        _text: &str,
+        _trigger_in_words: bool,
+        _cx: &mut Context<crate::Editor>,
+    ) -> bool {
+        false
+    }
+
+    fn filter_completions(&self) -> bool {
+        false
+    }
+}
+
 #[derive(Default, Clone)]
 pub struct FakeEditPredictionDelegate {
     pub completion: Option<edit_prediction_types::EditPrediction>,

crates/editor/src/editor.rs πŸ”—

@@ -2884,6 +2884,11 @@ impl Editor {
         if self.in_leading_whitespace {
             key_context.add("in_leading_whitespace");
         }
+        if self.edit_prediction_requires_modifier() {
+            key_context.set("edit_prediction_mode", "subtle")
+        } else {
+            key_context.set("edit_prediction_mode", "eager");
+        }
 
         if self.selection_mark_mode {
             key_context.add("selection_mode");
@@ -2942,7 +2947,7 @@ impl Editor {
         window: &mut Window,
         cx: &mut App,
     ) -> Option<gpui::KeybindingKeystroke> {
-        let key_context = self.key_context_internal(self.has_active_edit_prediction(), window, cx);
+        let key_context = self.key_context_internal(true, window, cx);
 
         let bindings =
             match granularity {
@@ -2969,7 +2974,7 @@ impl Editor {
         window: &mut Window,
         cx: &mut App,
     ) -> Option<gpui::KeybindingKeystroke> {
-        let key_context = self.key_context_internal(self.has_active_edit_prediction(), window, cx);
+        let key_context = self.key_context_internal(true, window, cx);
         let bindings = window.bindings_for_action_in_context(&AcceptEditPrediction, key_context);
         bindings
             .into_iter()
@@ -2980,6 +2985,32 @@ impl Editor {
             })
     }
 
+    fn edit_prediction_preview_modifiers_held(
+        &self,
+        modifiers: &Modifiers,
+        window: &mut Window,
+        cx: &mut App,
+    ) -> bool {
+        let key_context = self.key_context_internal(true, window, cx);
+        let actions: [&dyn Action; 3] = [
+            &AcceptEditPrediction,
+            &AcceptNextWordEditPrediction,
+            &AcceptNextLineEditPrediction,
+        ];
+
+        actions.into_iter().any(|action| {
+            window
+                .bindings_for_action_in_context(action, key_context.clone())
+                .into_iter()
+                .rev()
+                .any(|binding| {
+                    binding.keystrokes().first().is_some_and(|keystroke| {
+                        keystroke.modifiers().modified() && keystroke.modifiers() == modifiers
+                    })
+                })
+        })
+    }
+
     fn edit_prediction_cursor_popover_prefers_preview(
         &self,
         completion: &EditPredictionState,
@@ -8486,9 +8517,12 @@ impl Editor {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
+        self.update_edit_prediction_settings(cx);
+
         // Ensure that the edit prediction preview is updated, even when not
         // enabled, if there's an active edit prediction preview.
         if self.show_edit_predictions_in_menu()
+            || self.edit_prediction_requires_modifier()
             || matches!(
                 self.edit_prediction_preview,
                 EditPredictionPreview::Active { .. }
@@ -8581,24 +8615,7 @@ impl Editor {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        let mut modifiers_held = false;
-
-        let key_context = self.key_context_internal(self.has_active_edit_prediction(), window, cx);
-        let actions: [&dyn Action; 3] = [
-            &AcceptEditPrediction,
-            &AcceptNextWordEditPrediction,
-            &AcceptNextLineEditPrediction,
-        ];
-
-        for action in actions {
-            let bindings = window.bindings_for_action_in_context(action, key_context.clone());
-            for binding in bindings {
-                if let Some(keystroke) = binding.keystrokes().first() {
-                    modifiers_held = modifiers_held
-                        || (keystroke.modifiers() == modifiers && keystroke.modifiers().modified());
-                }
-            }
-        }
+        let modifiers_held = self.edit_prediction_preview_modifiers_held(modifiers, window, cx);
 
         if modifiers_held {
             if matches!(