fix: unify tool toggle semantics

crates/app/AGENTS.md

@@ -11,6 +11,9 @@ show/hide/disable behavior regardless of who adds them.
   command palette, and shortcuts share one enabled/placement decision. Do not
   add ad-hoc business-action buttons that bypass the catalog. Panel-local
   widget interactions (rename fields, tree-row toggles) are exempt.
+- A command whose checked state derives from `session.tool` is a toggle and
+  must execute through `toggle_tool`; reserve `set_tool` for navigation and
+  other ensure-on flows.
 
 ## Hide vs disable
 

crates/app/src/ui/command_exec.rs

@@ -103,7 +103,7 @@ pub fn execute(
         CommandId::SpectrumArithmetic => super::arithmetic::open_spectrum_arithmetic_dialog(app),
         CommandId::AlignSpectra => super::align::open_align_spectra_dialog(app),
         CommandId::StackData => app.stack_selected_data(),
-        CommandId::SelectRange => app.set_tool(Tool::SelectRegion),
+        CommandId::SelectRange => app.toggle_tool(Tool::SelectRegion),
         CommandId::ClearRange => app.clear_analysis_selection(),
         CommandId::Regions => toggle_regions(app),
         CommandId::SeriesTable => open_active_region_table(app),
@@ -116,7 +116,7 @@ pub fn execute(
         CommandId::Statistics => open_statistics(app),
         CommandId::ChartType => open_chart_type(app),
         CommandId::FigureTypography => app.session.ui.figure_typography_open = true,
-        CommandId::Integrate => app.set_tool(Tool::Integrate),
+        CommandId::Integrate => app.toggle_tool(Tool::Integrate),
         CommandId::Multiplets => analyze_multiplets(app),
         CommandId::TidyBoard => app.tidy_board(),
         CommandId::CanvasSettings => {
@@ -144,7 +144,7 @@ pub fn execute(
                 app.apply_theme(&theme);
             }
         }
-        CommandId::Tool(tool) => app.set_tool(tool),
+        CommandId::Tool(tool) => app.toggle_tool(tool),
     }
 }
 
@@ -222,13 +222,13 @@ fn analyze_multiplets(app: &mut PlotxApp) {
 }
 
 fn reveal_tool_group(app: &mut PlotxApp, tool: Tool, group: ToolGroup) {
-    app.set_tool(tool);
+    app.toggle_tool(tool);
     reveal_group(app, group);
 }
 
 fn toggle_regions(app: &mut PlotxApp) {
     if app.session.tool == Tool::Regions {
-        app.set_tool(Tool::BrowseZoom);
+        app.toggle_tool(Tool::Regions);
         return;
     }
     let Some(dataset) = app
@@ -237,7 +237,7 @@ fn toggle_regions(app: &mut PlotxApp) {
     else {
         return;
     };
-    app.set_tool(Tool::Regions);
+    app.toggle_tool(Tool::Regions);
     super::tools::open_region_task(app, dataset);
 }
 

crates/app/src/ui/commands.rs

@@ -122,6 +122,18 @@ pub enum CommandExecutionClass {
 }
 
 impl CommandId {
+    fn tool_target(self) -> Option<Tool> {
+        match self {
+            Self::SelectRange => Some(Tool::SelectRegion),
+            Self::Regions => Some(Tool::Regions),
+            Self::PeakList => Some(Tool::Peaks),
+            Self::LineFit => Some(Tool::LineFit),
+            Self::Integrate => Some(Tool::Integrate),
+            Self::Tool(tool) => Some(tool),
+            _ => None,
+        }
+    }
+
     pub fn execution_class(self) -> CommandExecutionClass {
         match self {
             Self::RunBatchWorkflow => CommandExecutionClass::ToolEditor,
@@ -520,7 +532,12 @@ pub fn describe(app: &PlotxApp, id: CommandId) -> CommandDescriptor {
             && app.session.ui.spectrum_arithmetic_dialog.is_none()
             && app.session.ui.align_spectra_dialog.is_none()
             && !app.session.ui.interaction.is_active());
-    let enabled = gate.is_ok() && palette_available;
+    // Activation requirements must not trap an already-active tool after the
+    // dataset context changes: its command remains available for deactivation.
+    let active_tool = id
+        .tool_target()
+        .is_some_and(|tool| app.session.tool == tool);
+    let enabled = (gate.is_ok() || active_tool) && palette_available;
     let disabled_reason = if enabled { None } else { gate.err() };
     // A contextual group is dropped from the Ribbon entirely rather than shown
     // permanently dead: a dead group still consumes the width budget in

crates/app/src/ui/commands_tests.rs

@@ -373,6 +373,63 @@ fn live_checked_and_enabled_state_comes_from_one_descriptor() {
     assert_eq!(describe(&app, CommandId::CurveFit).label, "Fit Curves");
 }
 
+#[test]
+fn commands_that_activate_tools_toggle_back_to_their_rest_tool() {
+    let catalog_app = app_with_nmr();
+    let command_ids: Vec<_> = catalog(&catalog_app)
+        .into_iter()
+        // Plain actions may open native dialogs or external URLs; the checked
+        // contract is what identifies catalog toggles without enumerating tools.
+        .filter(|command| command.checked.is_some())
+        .map(|command| command.id)
+        .collect();
+
+    for id in command_ids {
+        let mut app = app_with_nmr();
+        let before = app.session.tool;
+        let ctx = egui::Context::default();
+        let mut clipboard = crate::ui::clipboard_table::ClipboardTablePaste::default();
+
+        execute(id, &mut app, &mut clipboard, &ctx);
+        let activated = app.session.tool;
+        if activated == before {
+            continue;
+        }
+
+        execute(id, &mut app, &mut clipboard, &ctx);
+        assert_eq!(
+            app.session.tool,
+            activated.rest(),
+            "command {id:?} activated {activated:?} but did not toggle to its rest tool"
+        );
+    }
+}
+
+#[test]
+fn active_tool_can_be_deactivated_after_its_requirements_become_unmet() {
+    let mut app = app_with_nmr();
+    let ctx = egui::Context::default();
+    let mut clipboard = crate::ui::clipboard_table::ClipboardTablePaste::default();
+    execute(CommandId::Integrate, &mut app, &mut clipboard, &ctx);
+    assert_eq!(app.session.tool, Tool::Integrate);
+
+    let mut table_app = app_with_table(0);
+    let table = table_app.doc.datasets.remove(0);
+    let action = Action::insert_dataset_with_default_canvas(
+        &app,
+        table,
+        "Canvas — table".to_owned(),
+        DEFAULT_CANVAS_SIZE_MM,
+    );
+    app.execute_action(action);
+    let integrate = describe(&app, CommandId::Integrate);
+    assert!(integrate.enabled);
+    assert_eq!(integrate.checked, Some(true));
+
+    execute(CommandId::Integrate, &mut app, &mut clipboard, &ctx);
+    assert_eq!(app.session.tool, Tool::BrowseZoom);
+}
+
 /// The disabled tooltip must name the requirement that actually blocked the
 /// command, not one fixed sentence per command.
 #[test]

crates/app/src/ui/primary_sidebar.rs

@@ -663,6 +663,8 @@ fn select_dataset(app: &mut PlotxApp, ui: &Ui, di: usize, extend: bool) {
 fn select_analysis(app: &mut PlotxApp, ui: &Ui, di: usize, item: &AnalysisItem, open: bool) {
     app.focus_single(di);
     app.session.ui.data_browser_selected_node = Some(item.kind.key(di));
+    // Opening a browser result must reveal its editor even when that tool is
+    // already active, so this navigation path intentionally uses ensure-on.
     match item.kind {
         AnalysisKind::Peak(id) => {
             app.session.ui.selected_peak = Some(id);

crates/app/src/ui/shortcuts.rs

@@ -267,7 +267,10 @@ pub(super) fn handle_escape_shortcut(app: &mut PlotxApp, ctx: &egui::Context) {
     if !escape {
         return;
     }
+    handle_escape(app, now);
+}
 
+fn handle_escape(app: &mut PlotxApp, now: f64) {
     if app.interaction().is_active() {
         let phase = matches!(app.interaction(), Interaction::Phase(_));
         app.cancel_interaction();
@@ -284,7 +287,8 @@ pub(super) fn handle_escape_shortcut(app: &mut PlotxApp, ctx: &egui::Context) {
         app.session.status = "Cancelled interaction.".to_owned();
         return;
     }
-    // Esc steps back one level: analysis region -> panel-letter sub-selection -> selection.
+    // Esc steps back one level: analysis region -> panel-letter sub-selection ->
+    // selection -> active tool.
     if app.session.ui.analysis_selection.is_some() {
         if let Some(ci) = app.session.active_canvas {
             canvas::clear_canvas_interaction_state(
@@ -307,6 +311,13 @@ pub(super) fn handle_escape_shortcut(app: &mut PlotxApp, ctx: &egui::Context) {
 
     if !matches!(app.session.ui.selection, Selection::None) {
         exit_to_page(app, "Selection cleared.");
+        return;
+    }
+
+    let rest = app.session.tool.rest();
+    if app.session.tool != rest {
+        app.set_tool(rest);
+        app.session.status = "Exited tool mode.".to_owned();
     }
 }
 
@@ -533,4 +544,15 @@ mod tests {
         );
         assert!(shortcut_label(commands::CommandId::About).is_none());
     }
+
+    #[test]
+    fn escape_exits_an_active_tool_after_other_fallbacks() {
+        let mut app = PlotxApp::new_with_settings(plotx_core::settings::Settings::default());
+        app.set_tool(Tool::Integrate);
+
+        handle_escape(&mut app, 0.0);
+
+        assert_eq!(app.session.tool, Tool::BrowseZoom);
+        assert_eq!(app.session.status, "Exited tool mode.");
+    }
 }

crates/app/src/ui/tools/line_fit.rs

@@ -31,11 +31,7 @@ pub(super) fn line_fit_group(app: &mut PlotxApp, di: usize, ui: &mut Ui) -> bool
         .on_hover_text("Deconvolve a region into overlapping lineshape components.")
         .clicked()
     {
-        app.set_tool(if active {
-            Tool::BrowseZoom
-        } else {
-            Tool::LineFit
-        });
+        app.toggle_tool(Tool::LineFit);
     }
     if active {
         ui.small(

crates/app/src/ui/tools/mod.rs

@@ -121,7 +121,7 @@ fn analysis_group(app: &mut PlotxApp, di: usize, ui: &mut Ui) -> bool {
     ui.horizontal(|ui| {
         let selected = app.session.tool == Tool::SelectRegion;
         if ui.selectable_label(selected, "Analysis range").clicked() {
-            app.set_tool(Tool::SelectRegion);
+            app.toggle_tool(Tool::SelectRegion);
         }
         if ui
             .add_enabled(has_selection, Button::new("Clear"))
@@ -186,11 +186,7 @@ fn peaks_group(app: &mut PlotxApp, di: usize, ui: &mut Ui) -> bool {
         .on_hover_text("Pick peaks by region, click, or one-shot detection — one set.")
         .clicked()
     {
-        app.set_tool(if active {
-            Tool::BrowseZoom
-        } else {
-            Tool::Peaks
-        });
+        app.toggle_tool(Tool::Peaks);
     }
     if active {
         ui.small(
@@ -303,11 +299,7 @@ pub(super) fn integrate_group(app: &mut PlotxApp, di: usize, ui: &mut Ui) {
         .on_hover_text("Drag across a multiplet to add an integral; drag its edges to adjust it.")
         .clicked()
     {
-        app.set_tool(if drawing {
-            Tool::BrowseZoom
-        } else {
-            Tool::Integrate
-        });
+        app.toggle_tool(Tool::Integrate);
     }
     if drawing {
         ui.small(
@@ -389,11 +381,7 @@ fn integrate_2d_group(app: &mut PlotxApp, di: usize, ui: &mut Ui) {
         )
         .clicked()
     {
-        app.set_tool(if drawing {
-            Tool::BrowseZoom
-        } else {
-            Tool::Integrate
-        });
+        app.toggle_tool(Tool::Integrate);
     }
     if drawing {
         ui.small("Drag to add · corners/edges resize · interior moves · right-click for reference or delete.");

crates/app/src/ui/tools/slice.rs

@@ -31,11 +31,7 @@ pub(super) fn slice_group(app: &mut PlotxApp, di: usize, ui: &mut Ui) {
         .on_hover_text("Turn on, then hover the plot to position the cut (S).")
         .clicked()
     {
-        app.set_tool(if active {
-            Tool::BrowseZoom
-        } else {
-            Tool::Slice
-        });
+        app.toggle_tool(Tool::Slice);
     }
 
     if is_true_2d {

crates/core/src/actions/tests/interaction.rs

@@ -163,6 +163,44 @@ fn switching_to_data_tool_resets_in_flight_interaction() {
     assert!(!app.interaction().is_active());
 }
 
+#[test]
+fn toggling_manual_phase_cancels_the_in_flight_drag() {
+    use crate::actions::DatasetProcessingState;
+    use crate::state::{Interaction, PhaseAxis, PhaseDrag, PhaseDragKind, Tool};
+
+    let mut app = sample_app();
+    app.set_tool(Tool::ManualPhase);
+    let before = DatasetProcessingState::from_dataset(&app.doc.datasets[0]);
+    let original_phase0 = app.doc.datasets[0]
+        .phase_params_mut(PhaseAxis::Direct)
+        .unwrap()
+        .phase0;
+    app.doc.datasets[0]
+        .phase_params_mut(PhaseAxis::Direct)
+        .unwrap()
+        .phase0 = original_phase0 + 0.5;
+    app.apply_dataset_edit(0);
+    app.set_interaction(Interaction::Phase(PhaseDrag {
+        kind: PhaseDragKind::Ph0,
+        dataset: 0,
+        axis: PhaseAxis::Direct,
+        preview_pivot_ppm: None,
+        gesture_before: before,
+    }));
+
+    app.toggle_tool(Tool::ManualPhase);
+
+    assert_eq!(app.session.tool, Tool::BrowseZoom);
+    assert!(!app.interaction().is_active());
+    assert_eq!(
+        app.doc.datasets[0]
+            .phase_params_mut(PhaseAxis::Direct)
+            .unwrap()
+            .phase0,
+        original_phase0
+    );
+}
+
 #[test]
 fn switching_tools_preserves_selection() {
     use crate::state::{Selection, Tool};

crates/core/src/state/app_impl.rs

@@ -425,6 +425,18 @@ impl PlotxApp {
         }
     }
 
+    pub fn toggle_tool(&mut self, tool: Tool) {
+        let next = if self.session.tool == tool {
+            tool.rest()
+        } else {
+            tool
+        };
+        if self.session.tool != next && self.interaction().is_active() {
+            self.cancel_interaction();
+        }
+        self.set_tool(next);
+    }
+
     /// Select a whole object in page space. Clicking a grouped member selects the
     /// whole group, with the clicked object primary.
     pub fn select_object(&mut self, ci: usize, id: ObjectId) {

crates/core/src/state/units.rs

@@ -185,6 +185,14 @@ impl Tool {
         )
     }
 
+    pub fn rest(self) -> Self {
+        if self.is_data_tool() {
+            Self::BrowseZoom
+        } else {
+            Self::Select
+        }
+    }
+
     pub fn is_layout_tool(self) -> bool {
         matches!(self, Tool::Select)
     }
@@ -200,6 +208,19 @@ impl Tool {
     }
 }
 
+#[cfg(test)]
+mod tool_tests {
+    use super::Tool;
+
+    #[test]
+    fn rest_uses_the_tool_family_neutral_tool() {
+        assert_eq!(Tool::Integrate.rest(), Tool::BrowseZoom);
+        assert_eq!(Tool::Text.rest(), Tool::Select);
+        assert_eq!(Tool::BrowseZoom.rest(), Tool::BrowseZoom);
+        assert_eq!(Tool::Select.rest(), Tool::Select);
+    }
+}
+
 /// A phaseable/processable frequency dimension. `Direct` is the 1D axis (and the
 /// direct axis of a pseudo-2D stack); `F2`/`F1` are the direct/indirect axes of a
 /// true-2D spectrum. Lets the phase panel and canvas drag address any dimension