fix(curve_settings): keep custom curves and DAP settings on apply

bec_widgets/widgets/plots/waveform/settings/curve_settings/curve_tree.py

@@ -486,7 +486,20 @@ class CurveRow(QTreeWidgetItem):
                 new_dap = (
                     [selected_model, *composite_models] if composite_models else selected_model
                 )
-            self.config.signal = DeviceSignal(device=device, signal=signal, dap=new_dap)
+            # Keep the DAP settings the tree does not edit. Fit parameter overrides belong to
+            # the selected model, so they only carry over while the model is unchanged.
+            old_signal = self.config.signal
+            dap_oversample = old_signal.dap_oversample if old_signal is not None else 1
+            dap_parameters = None
+            if old_signal is not None and old_signal.dap == new_dap:
+                dap_parameters = old_signal.dap_parameters
+            self.config.signal = DeviceSignal(
+                device=device,
+                signal=signal,
+                dap=new_dap,
+                dap_oversample=dap_oversample,
+                dap_parameters=dap_parameters,
+            )
             self.config.source = "dap"
             self.config.parent_label = parent_conf.label
             label_suffix = "+".join(new_dap) if isinstance(new_dap, list) else new_dap
@@ -699,10 +712,22 @@ class CurveTree(BECWidget, QWidget):
         return new_row
 
     def send_curve_json(self):
-        """Send the current tree's config as JSON to the waveform, updating wavefrom.color_palette as well."""
-        if self.waveform is not None:
+        """Send the tree's config as JSON to the waveform, applying a changed color palette too.
+
+        Custom curves are not listed in the tree; the waveform keeps them when it loads the JSON,
+        and the DAP curves fitted to them are passed through unchanged.
+        """
+        if self.waveform is not None and self.waveform.color_palette != self.color_palette:
+            # The palette setter recolors every curve, so only apply an actual change
             self.waveform.color_palette = self.color_palette
         data = self.export_all_curves()
+        if self.waveform is not None:
+            custom_labels = {c.name() for c in self.waveform.curves if c.config.source == "custom"}
+            data.extend(
+                c.config.model_dump()
+                for c in self.waveform.curves
+                if c.config.source == "dap" and c.config.parent_label in custom_labels
+            )
         json_data = json.dumps(data, indent=2)
         if self.waveform is not None:
             self.waveform.curve_json = json_data

bec_widgets/widgets/plots/waveform/waveform.py

@@ -796,15 +796,46 @@ class Waveform(PlotBase):
     def curve_json(self, json_data: str):
         """
         Load curves from a JSON string and add them to the plot, omitting custom source curves.
+
+        The serialized curves replace all device, history and DAP curves. Custom curves hold
+        data that cannot be serialized, so the existing ones are kept unchanged (configuration
+        and data) and custom entries in the JSON are not loaded. The JSON is rejected before
+        any curve is removed if one of its curves is invalid, refers to a scan that is not in
+        the history, or would get a label that a kept custom curve or another entry already uses.
         """
         try:
             curve_configs = json.loads(json_data)
-            self.clear_all()
+            custom_labels = {c.name() for c in self.curves if c.config.source == "custom"}
+            # Build and check every serialized curve before anything is removed, so a curve
+            # that cannot be added does not leave the plot half-loaded
+            configs = []
             for cfg_dict in curve_configs:
                 if cfg_dict.get("source") == "custom":
-                    logger.warning(f"Custom source curve '{cfg_dict['label']}' not loaded.")
+                    if cfg_dict.get("label") not in custom_labels:
+                        logger.warning(f"Custom source curve '{cfg_dict.get('label')}' not loaded.")
                     continue
                 config = CurveConfig(**cfg_dict)
+                if config.source == "history":
+                    self._resolve_history_config(config)
+                configs.append(config)
+            labels = [config.label for config in configs if config.label]
+            taken = custom_labels & set(labels)
+            if taken:
+                raise ValueError(f"Labels {sorted(taken)} are already used by custom curves.")
+            duplicates = {label for label in labels if labels.count(label) > 1}
+            if duplicates:
+                raise ValueError(f"Labels {sorted(duplicates)} are used by more than one curve.")
+            # Like clear_all, but keep custom curves and their colors (remove_curve recolors)
+            self._dap_curves = []
+            self._history_curves = []
+            self._sync_curves = []
+            self._async_curves = []
+            for curve in self.curves:
+                if curve.name() not in custom_labels:
+                    self._remove_curve_by_name(curve.name())
+            if self.crosshair is not None:
+                self.crosshair.clear_markers()
+            for config in configs:
                 self._add_curve(config=config)
             self._refresh_alignment_state(force_readback=self._alignment_panel_visible)
             self._refresh_dap_signals()
@@ -1124,21 +1155,9 @@ class Waveform(PlotBase):
         """
         scan_item: ScanDataContainer | None = None
         if config.source == "history":
-            scan_item = self.get_history_scan_item(
-                scan_id=config.scan_id, scan_index=config.scan_number
-            )
-            if scan_item is None:
-                raise ValueError(
-                    f"Could not find scan item for history curve '{config.label}' with scan_id='{config.scan_id}' and scan_number='{config.scan_number}'."
-                )
-
-            config.scan_id = scan_item.metadata["bec"]["scan_id"]
-            config.scan_number = scan_item.metadata["bec"]["scan_number"]
+            scan_item = self._resolve_history_config(config)
 
         label = config.label
-        if config.source == "history":
-            label = f"{config.signal.device}-{config.signal.signal}-scan-{config.scan_number}"
-            config.label = label
         if not label:
             # Fallback label
             label = WidgetContainerUtils.generate_unique_name(
@@ -1186,6 +1205,33 @@ class Waveform(PlotBase):
 
         return curve
 
+    def _resolve_history_config(self, config: CurveConfig) -> ScanDataContainer:
+        """
+        Resolve the scan of a history curve config and set its scan_id, scan_number and label.
+
+        History curves are always labelled '<device>-<signal>-scan-<scan_number>'.
+
+        Args:
+            config (CurveConfig): Config of a history curve; updated in place.
+
+        Returns:
+            ScanDataContainer: The scan item the curve reads its data from.
+
+        Raises:
+            ValueError: If the scan is not found in the scan history.
+        """
+        scan_item = self.get_history_scan_item(
+            scan_id=config.scan_id, scan_index=config.scan_number
+        )
+        if scan_item is None:
+            raise ValueError(
+                f"Could not find scan item for history curve '{config.label}' with scan_id='{config.scan_id}' and scan_number='{config.scan_number}'."
+            )
+        config.scan_id = scan_item.metadata["bec"]["scan_id"]
+        config.scan_number = scan_item.metadata["bec"]["scan_number"]
+        config.label = f"{config.signal.device}-{config.signal.signal}-scan-{config.scan_number}"
+        return scan_item
+
     def _add_curve_object(
         self, name: str, config: CurveConfig, scan_item: ScanDataContainer | None = None
     ) -> Curve | None:

tests/unit_tests/test_curve_settings.py

@@ -542,3 +542,91 @@ def test_curve_settings_edits_do_not_touch_live_curve_config(history_curve_setti
     curve_manager.export_all_curves()
 
     assert history_curve.config.model_dump() == original_config
+
+
+##################################################
+# Apply keeps custom curves and DAP settings (issues #1302, #1303)
+##################################################
+
+
+def test_apply_keeps_custom_curves(qtbot, mocked_client_with_dap):
+    """
+    Regression for #1302: custom curves are not listed in the curve tree, but applying the
+    curve settings must keep them with their configuration, their data and their DAP fit.
+    """
+    wf = create_widget(qtbot, Waveform, client=mocked_client_with_dap)
+    wf.plot(arg1="bpm4i")
+    custom = wf.plot(
+        x=[1, 2, 3],
+        y=[4, 5, 6],
+        label="custom",
+        color="#ff0000",
+        pen_width=2,
+        pen_style="dash",
+        dap="GaussianModel",
+    )
+    custom_config = custom.config.model_dump()
+    curve_setting = create_widget(qtbot, CurveSetting, parent=None, target_widget=wf)
+
+    curve_setting.accept_changes()
+
+    assert custom in wf.curves
+    assert custom.config.model_dump() == custom_config
+    x_data, y_data = custom.get_data()
+    assert list(x_data) == [1, 2, 3]
+    assert list(y_data) == [4, 5, 6]
+    assert sorted(c.name() for c in wf.curves) == ["bpm4i-bpm4i", "custom", "custom-GaussianModel"]
+    custom_dap = wf.get_curve("custom-GaussianModel")
+    assert custom_dap.config.parent_label == "custom"
+    assert custom_dap in wf._dap_curves
+
+
+def test_apply_keeps_dap_oversample_and_parameters(qtbot, mocked_client_with_dap):
+    """
+    Regression for #1303: applying an unrelated change in the curve settings must keep the
+    oversampling factor and the fit parameter overrides of a DAP curve.
+    """
+    wf = create_widget(qtbot, Waveform, client=mocked_client_with_dap)
+    wf.plot(arg1="bpm4i")
+    wf.add_dap_curve(
+        device_label="bpm4i-bpm4i",
+        dap_name="GaussianModel",
+        dap_oversample=3,
+        dap_parameters={"amplitude": 1.0},
+    )
+    dap_parameters = wf.get_curve("bpm4i-bpm4i-GaussianModel").config.signal.dap_parameters
+    assert dap_parameters
+    curve_setting = create_widget(qtbot, CurveSetting, parent=None, target_widget=wf)
+
+    dap_row = curve_setting.curve_manager.tree.topLevelItem(0).child(0)
+    dap_row.color_button.set_color("#123456")  # an unrelated change
+    curve_setting.accept_changes()
+
+    dap_curve = wf.get_curve("bpm4i-bpm4i-GaussianModel")
+    assert dap_curve.config.color == "#123456"
+    assert dap_curve.dap_oversample == 3
+    assert dap_curve.config.signal.dap_parameters == dap_parameters
+
+
+def test_dap_model_change_drops_parameters_but_keeps_oversample(qtbot, mocked_client_with_dap):
+    """
+    Fit parameter overrides belong to the selected model, so choosing another model drops them;
+    the oversampling factor does not depend on the model and is kept.
+    """
+    wf = create_widget(qtbot, Waveform, client=mocked_client_with_dap)
+    wf.plot(arg1="bpm4i")
+    wf.add_dap_curve(
+        device_label="bpm4i-bpm4i",
+        dap_name="GaussianModel",
+        dap_oversample=3,
+        dap_parameters={"amplitude": 1.0},
+    )
+    curve_tree = create_widget(qtbot, CurveTree, parent=None, waveform=wf)
+    dap_row = curve_tree.tree.topLevelItem(0).child(0)
+
+    dap_row.dap_combo.fit_model_combobox.setCurrentText("LorentzModel")
+    exported = dap_row.export_data()
+
+    assert exported["signal"]["dap"] == "LorentzModel"
+    assert exported["signal"]["dap_oversample"] == 3
+    assert exported["signal"]["dap_parameters"] is None

tests/unit_tests/test_waveform.py

@@ -342,6 +342,112 @@ def test_curve_json_setter_ignores_custom(qtbot, mocked_client):
     assert curves[0].name() == "device_curve"
 
 
+def test_curve_json_round_trip_keeps_custom_curves(qtbot, mocked_client_with_dap):
+    """
+    Regression for #1302: custom curves cannot be serialized, so setting curve_json keeps the
+    existing ones (same curve, configuration and data) while it replaces the serialized curves,
+    including the DAP curves fitted to custom curves.
+    """
+    wf = create_widget(qtbot, Waveform, client=mocked_client_with_dap)
+    wf.plot(arg1="bpm4i")
+    custom = wf.plot(x=[1, 2, 3], y=[4, 5, 6], label="custom", color="#ff0000", pen_width=2)
+    wf.add_dap_curve(device_label="custom", dap_name="GaussianModel", dap_oversample=2)
+    custom_config = custom.config.model_dump()
+    json_str = wf.curve_json
+    assert [cfg["label"] for cfg in json.loads(json_str)] == ["bpm4i-bpm4i", "custom-GaussianModel"]
+
+    wf.curve_json = json_str
+
+    assert custom in wf.curves
+    assert custom.config.model_dump() == custom_config
+    x_data, y_data = custom.get_data()
+    assert list(x_data) == [1, 2, 3]
+    assert list(y_data) == [4, 5, 6]
+    assert sorted(c.name() for c in wf.curves) == ["bpm4i-bpm4i", "custom", "custom-GaussianModel"]
+    assert wf.get_curve("custom-GaussianModel").dap_oversample == 2
+
+    # A JSON without the device curve replaces it, the custom curve stays.
+    wf.curve_json = "[]"
+    assert wf.curves == [custom]
+    assert wf._dap_curves == []
+
+
+def test_curve_json_rejects_label_of_kept_custom_curve(qtbot, mocked_client):
+    """
+    A serialized curve that would reuse the label of a kept custom curve is rejected before
+    any curve is removed, so the plot is not left half-loaded.
+    """
+    wf = create_widget(qtbot, Waveform, client=mocked_client)
+    device_curve = wf.plot(arg1="bpm3a")
+    custom = wf.plot(x=[1, 2, 3], y=[4, 5, 6], label="bpm4i-bpm4i")
+    clashing = {
+        "widget_class": "Curve",
+        "label": "bpm4i-bpm4i",
+        "source": "device",
+        "signal": {"device": "bpm4i", "signal": "bpm4i"},
+    }
+
+    wf.curve_json = json.dumps([clashing])  # SafeProperty logs the error
+
+    assert wf.curves == [device_curve, custom]
+
+
+@pytest.mark.parametrize("label", ["history_alias", None], ids=["aliased", "omitted"])
+def test_curve_json_rejects_generated_history_label_of_kept_custom_curve(
+    qtbot, mocked_client, scan_history_factory, label
+):
+    """
+    History curves are relabelled '<device>-<signal>-scan-<N>' when they are added, so the
+    clash check must use that label, not the one in the JSON, and still fail before any curve
+    is removed.
+    """
+    wf = create_widget(qtbot, Waveform, client=mocked_client)
+    inject_scan_history(wf, scan_history_factory, ("hist1", 1))
+    device_curve = wf.plot(arg1="bpm3a")
+    custom = wf.plot(x=[1, 2, 3], y=[4, 5, 6], label="bpm4i-bpm4i-scan-1")
+    history = {
+        "widget_class": "Curve",
+        "label": label,
+        "source": "history",
+        "scan_id": "hist1",
+        "signal": {"device": "bpm4i", "signal": "bpm4i"},
+    }
+
+    wf.curve_json = json.dumps([history])  # SafeProperty logs the error
+
+    assert wf.curves == [device_curve, custom]
+
+
+def test_curve_json_rejects_payload_before_removing_curves(
+    qtbot, mocked_client, scan_history_factory
+):
+    """
+    Entries that cannot be added (a history scan that is not in the history, or two entries
+    with the same label) are rejected before any existing curve is removed.
+    """
+    wf = create_widget(qtbot, Waveform, client=mocked_client)
+    inject_scan_history(wf, scan_history_factory, ("hist1", 1))
+    device_curve = wf.plot(arg1="bpm3a")
+    missing_scan = {
+        "widget_class": "Curve",
+        "source": "history",
+        "scan_id": "not-in-history",
+        "signal": {"device": "bpm4i", "signal": "bpm4i"},
+    }
+    device = {
+        "widget_class": "Curve",
+        "label": "bpm4i-bpm4i",
+        "source": "device",
+        "signal": {"device": "bpm4i", "signal": "bpm4i"},
+    }
+
+    wf.curve_json = json.dumps([device, missing_scan])
+    assert wf.curves == [device_curve]
+
+    wf.curve_json = json.dumps([device, device])
+    assert wf.curves == [device_curve]
+
+
 ##################################################
 # Waveform widget scan logic tests
 ##################################################