From 51b686b823e756e2f49718a4d39ddd6595816d14 Mon Sep 17 00:00:00 2001
From: Rudolf Weeber <weeber@icp.uni-stuttgart.de>
Date: Mon, 15 Jun 2026 10:46:58 +0200
Subject: [PATCH] script_interface: require all BreakageSpec parameters
 (bug-sweep #42)

BreakageSpec registered breakage_length and action_type via AutoParameters
but did not override do_construct, so the inherited ObjectHandle::do_construct
only set the parameters actually passed. Omitting breakage_length left the
core struct's value-initialized 0.0 in place; since the breakage check is
`distance >= spec.breakage_length`, that is always true and queues every bond
of that type for breakage on the first execute(), silently deleting them.

0.0 is a legitimate, tested breakage_length, so it cannot serve as an unset
sentinel. Following the established bonded-interaction convention
(ScriptInterface::Interactions::BondedInteraction::check_valid_parameters),
add a do_construct override that wraps construction in parallel_try_catch and
throws "Parameter '<name>' is missing" if any valid parameter is absent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 .../bond_breakage/BreakageSpec.hpp            | 19 +++++++++++++++++++
 testsuite/python/bond_breakage.py             |  8 ++++++++
 2 files changed, 27 insertions(+)

diff --git a/src/script_interface/bond_breakage/BreakageSpec.hpp b/src/script_interface/bond_breakage/BreakageSpec.hpp
index b2605af9a5..f9e1adb4d1 100644
--- a/src/script_interface/bond_breakage/BreakageSpec.hpp
+++ b/src/script_interface/bond_breakage/BreakageSpec.hpp
@@ -24,6 +24,10 @@
 #include "script_interface/ScriptInterface.hpp"
 
 #include <memory>
+#include <ranges>
+#include <set>
+#include <stdexcept>
+#include <string>
 #include <variant>
 
 namespace ScriptInterface {
@@ -52,6 +56,21 @@ class BreakageSpec : public AutoParameters<BreakageSpec> {
   }
 
 private:
+  void do_construct(VariantMap const &params) override {
+    context()->parallel_try_catch([&]() {
+      // all parameters are required: reject construction if any is omitted,
+      // because "breakage_length" has no meaningful default (0.0 is a valid
+      // value that would silently queue every bond of that type for breakage)
+      for (auto const &key : valid_parameters()) {
+        if (not params.contains(std::string{key})) {
+          throw std::runtime_error("Parameter '" + std::string{key} +
+                                   "' is missing");
+        }
+      }
+      AutoParameters<BreakageSpec>::do_construct(params);
+    });
+  }
+
   std::shared_ptr<::BondBreakage::BreakageSpec> m_breakage_spec;
   std::unordered_map<::BondBreakage::ActionType, std::string>
       m_breakage_enum_to_str = {
diff --git a/testsuite/python/bond_breakage.py b/testsuite/python/bond_breakage.py
index a071431331..ed94e2d0b9 100644
--- a/testsuite/python/bond_breakage.py
+++ b/testsuite/python/bond_breakage.py
@@ -100,6 +100,14 @@ def test_00_interface(self):
         with self.assertRaisesRegex(RuntimeError, "Inserting breakage spec without a bond type is not permitted"):
             self.system.bond_breakage.call_method("insert", object=spec2)
 
+        # all parameters are required: omitting one must raise at construction
+        # rather than silently defaulting breakage_length to 0.0 (which would
+        # queue every bond of that type for breakage on the first execute())
+        with self.assertRaisesRegex(Exception, "Parameter 'breakage_length' is missing"):
+            BreakageSpec(action_type="delete_bond")
+        with self.assertRaisesRegex(Exception, "Parameter 'action_type' is missing"):
+            BreakageSpec(breakage_length=1.0)
+
     def test_ignore(self):
         system = self.system