Address code review

Author: lionel-

crates/amalthea/src/fixtures/dummy_frontend.rs

@@ -260,20 +260,20 @@ impl DummyFrontend {
         options: ExecuteRequestOptions,
         metadata: serde_json::Value,
     ) -> String {
-        let content = ExecuteRequest {
-            code: String::from(code),
-            silent: false,
-            store_history: true,
-            user_expressions: serde_json::Value::Null,
-            allow_stdin: options.allow_stdin,
-            stop_on_error: false,
-            positron: options.positron,
-        };
-        let mut message = JupyterMessage::create(content, None, &self.session);
-        message.metadata = metadata;
-        let id = message.header.msg_id.clone();
-        message.send(&self.shell_socket).unwrap();
-        id
+        Self::send_with_metadata(
+            &self.shell_socket,
+            &self.session,
+            ExecuteRequest {
+                code: String::from(code),
+                silent: false,
+                store_history: true,
+                user_expressions: serde_json::Value::Null,
+                allow_stdin: options.allow_stdin,
+                stop_on_error: false,
+                positron: options.positron,
+            },
+            metadata,
+        )
     }
 
     /// Send a DAP request wrapped in a Jupyter `debug_request` on the control channel.
@@ -315,6 +315,19 @@ impl DummyFrontend {
         id
     }
 
+    fn send_with_metadata<T: ProtocolMessage>(
+        socket: &Socket,
+        session: &Session,
+        msg: T,
+        metadata: serde_json::Value,
+    ) -> String {
+        let mut message = JupyterMessage::create(msg, None, session);
+        message.metadata = metadata;
+        let id = message.header.msg_id.clone();
+        message.send(socket).unwrap();
+        id
+    }
+
     #[track_caller]
     pub fn recv(socket: &Socket) -> Message {
         // It's important to wait with a timeout because the kernel thread might have

crates/ark/src/console/console_repl.rs

@@ -888,9 +888,9 @@ impl Console {
                 return result;
             }
 
-            // Similarly, if we have pending inputs, we're about to immediately
-            // continue with the next expression. Don't emit debug notifications
-            // for these intermediate browser prompts.
+            // As with auto-stepping above, if we have pending inputs we're
+            // about to immediately continue with the next expression. Don't
+            // emit debug notifications for these intermediate browser prompts.
             let has_pending = self.pending_inputs.as_ref().is_some_and(|p| !p.is_empty());
 
             // Only now that we know we're stopping for real, set state and

crates/ark/src/dap/dap_state.rs

@@ -227,7 +227,7 @@ pub struct Dap {
     /// Whether R is stopped at an unexpected `browser()` prompt in notebook
     /// mode (no active debug session). Mutually exclusive with `is_debugging`.
     /// Used by the interrupt handler to decide whether to send a "Q" command.
-    /// `id_debugging` and `is_stopped_at_browser` could be folded into a single
+    /// `is_debugging` and `is_debugging_stdin` could be folded into a single
     /// enum in the future.
     pub is_debugging_stdin: bool,
 
@@ -285,7 +285,7 @@ pub struct Dap {
 
     /// IOPub sender for emitting DAP events as `debug_event` messages
     /// in notebook debugging mode (Jupyter Debug Protocol path).
-    iopub_tx: Option<Sender<IOPubMessage>>,
+    notebook_iopub_tx: Option<Sender<IOPubMessage>>,
 
     /// Sequence counter for IOPub DAP event messages.
     iopub_seq: i64,
@@ -314,7 +314,7 @@ impl Dap {
             current_variables_reference: 1,
             current_breakpoint_id: 1,
             comm_tx: None,
-            iopub_tx: None,
+            notebook_iopub_tx: None,
             iopub_seq: 0,
             r_request_tx,
             shared_self: None,
@@ -388,7 +388,7 @@ impl Dap {
         let was_debugging = self.is_debugging;
         self.is_debugging = false;
 
-        if was_debugging && (self.is_connected || self.iopub_tx.is_some()) {
+        if was_debugging && (self.is_connected || self.notebook_iopub_tx.is_some()) {
             log::trace!("DAP: Sending `stop_debug` events");
 
             if let Some(comm_tx) = &self.comm_tx {
@@ -406,15 +406,15 @@ impl Dap {
     }
 
     pub fn set_iopub_tx(&mut self, tx: Sender<IOPubMessage>) {
-        self.iopub_tx = Some(tx);
+        self.notebook_iopub_tx = Some(tx);
     }
 
     fn send_backend_event(&mut self, event: DapBackendEvent) {
         if let Some(tx) = &self.backend_events_tx {
             tx.send(event.clone()).log_err();
         }
 
-        if let Some(tx) = &self.iopub_tx {
+        if let Some(tx) = &self.notebook_iopub_tx {
             let dap_event = event.into_dap_event();
             self.iopub_seq += 1;
 
@@ -611,6 +611,8 @@ impl Dap {
             return;
         };
 
+        // Collect events first: `bp_list` borrows from `self.breakpoints`,
+        // which prevents calling `&mut self` methods like `send_backend_event()`.
         let mut events = Vec::new();
 
         for bp in bp_list.iter_mut() {
@@ -871,7 +873,7 @@ mod tests {
             current_breakpoint_id: 1,
             is_interrupting_for_debugger: false,
             comm_tx: None,
-            iopub_tx: None,
+            notebook_iopub_tx: None,
             iopub_seq: 0,
             is_debugging_stdin: false,
             r_request_tx,
@@ -985,7 +987,7 @@ mod tests {
             current_breakpoint_id: 1,
             is_interrupting_for_debugger: false,
             comm_tx: None,
-            iopub_tx: None,
+            notebook_iopub_tx: None,
             iopub_seq: 0,
             is_debugging_stdin: false,
             r_request_tx,