fix: make runtime cleanup and transitions consistent

docs/runtime.md

@@ -0,0 +1,63 @@
+# Execution contract
+
+A loaded behavior initially contains only its root instance. `update()` first
+checks reevaluating decisions from the bottom of the active stack upward, then
+executes the current frame. Decision outcomes descend immediately through further
+decisions to an action. An action runs at most once per update.
+
+Actions retain their instance state between updates. `return self.pop()` completes
+an action. If the action has a chain successor, its cleanup runs before the
+successor is initialized, and that successor runs on the next update. Without a
+successor, the previous decision becomes current and decides on the next update.
+The only remaining frame cannot pop itself.
+
+A reevaluated decision whose selected branch changes cancels every frame above
+it, then initializes and executes the newly selected path in the same update.
+Different outcomes handled by the same `ELSE` branch do not rebuild the path.
+An invalid outcome is reported before the existing path is canceled.
+
+`interrupt()` cancels all active frames, including the old root, and initializes
+a fresh instance of the declared start element. It runs on the next update.
+A callback that pops, interrupts, or otherwise changes the stack cancels the
+remainder of the current traversal; its returned decision outcome is discarded.
+Return immediately after `pop()` or `interrupt()` to avoid continuing work on an
+instance that has left the active stack.
+
+`on_pop()` runs once for each discarded instance, in order from the top down.
+Frames are detached before cleanup. Cleanup exceptions are logged, and cleanup
+continues for the remaining detached frames. If cleanup itself changes the stack,
+the pending transition does not overwrite that change. Constructors initialize
+instance state; they must not change the DSD stack. Each activation receives an
+independent copy of its parameters.
+
+Behavior loading parses and binds the replacement before canceling the old
+behavior. Parse and registration errors therefore leave the active behavior
+intact. Once cleanup starts, replacement construction can still fail; callers
+must handle load errors. Do not rely on rollback of already canceled hardware
+operations.
+
+Ordinary update failures retain the existing log-and-continue policy. The first
+error of an update is available as `dsd.last_error`; a successful subsequent
+update clears it. An empty or non-string decision outcome is a normal library
+error and does not terminate the host process. Update errors do not imply that
+an action succeeded, was retried safely, or was rolled back. Error recovery and
+explicit action outcomes remain under discussion in
+[issue #115](https://github.com/bit-bots/dynamic_stack_decider/issues/115).
+
+`do_not_reevaluate()` suppresses the next reevaluation. Actions with `r:false` or
+`reevaluate:false` renew suppression while running. Completing a normal frame
+clears suppression; advancing a chain retains it. Interruption and behavior
+replacement clear it. Root-chain completion and suppression scope remain under
+discussion in [issue #121](https://github.com/bit-bots/dynamic_stack_decider/issues/121).
+
+Debug snapshots describe the state after the update. Debug failures are logged
+and recorded in `last_error` without preventing behavior execution. The tree and
+current-action topics use transient-local durability, so matching late
+subscribers receive their latest values. An empty current-action name means that
+no action is active. Debugging requires both a topic and a ROS node; otherwise it
+is disabled.
+
+DSD is synchronous. Call `update()` from one owner, avoid blocking work in
+`perform()`, and marshal external changes through that owner. Recursive updates
+are rejected. The stack returned by `get_stack()` is for inspection and must not
+be mutated by callers.

dynamic_stack_decider/dynamic_stack_decider/abstract_action_element.py

@@ -1,6 +1,8 @@
+"""Base class for persistent actions."""
+
 from __future__ import annotations
 
-from abc import ABCMeta, abstractmethod
+from abc import abstractmethod
 from typing import TYPE_CHECKING
 
 from dynamic_stack_decider.abstract_stack_element import AbstractStackElement
@@ -10,46 +12,21 @@
     from dynamic_stack_decider.dsd import DSD
 
 
-class AbstractActionElement(AbstractStackElement[BlackboardT], metaclass=ABCMeta):
-    """
-    One action is similar to a state of an FSM.
-    As in this case, the system stays in this state in contrast to the decision elements which are called only for determining the active action.
-    It defines the actions which the robot does, for example performing a kick.
-    Another example is an action which takes care of going to the ball, the action remains on top of the stack until the ball is reached.
-    The action only makes decisions which are necessary for its purpose, like some adjustments to the kicking movement.
-    Actions do not push further elements on the stack but command actions on lower-level modules like new movement goals.
-    If the action is complete, it can remove itself from the stack by performing a pop command.
-    """
+class AbstractActionElement(AbstractStackElement[BlackboardT]):
+    """Perform work until completion or preemption; implement cleanup in ``on_pop``."""
 
     def __init__(self, blackboard: BlackboardT, dsd: DSD[BlackboardT], parameters: Parameters) -> None:
-        """
-        Constructor of the action element
-        :param blackboard: Shared blackboard for data exchange between elements
-        :param dsd: The stack decider which has this element on its stack.
-        :param parameters: Parameters which serve as arguments to this element
-        """
         super().__init__(blackboard, dsd, parameters)
-        # Reevaluation can be disabled by setting 'r' or 'reevaluate' to False
         self.never_reevaluate = not parameters.get("r", True) or not parameters.get("reevaluate", True)
 
     def do_not_reevaluate(self) -> None:
-        """
-        Prohibits the next reevaluate.
-        This is useful if you have an action which has to be completed without interruption, e.g. a kick animation.
-        """
+        """Suppress the next reevaluation, for example while finishing an atomic motion."""
         self._dsd.set_do_not_reevaluate()
 
     def repr_dict(self) -> ElementData:
-        """
-        Represent this stack element as dictionary which is JSON encodable
-        """
-        return {
-            "type": "action",
-            "name": self.name,
-            "debug_data": self._debug_data,
-        }
+        return {"type": "action", "name": self.name, "debug_data": self._debug_data}
 
     @abstractmethod
     def perform(self, reevaluate: bool = False) -> None:
-        """Execute the element; decisions return an outcome and actions return nothing."""
+        """Perform one tick of work. Complete with ``return self.pop()``."""
         raise NotImplementedError(f"Implement perform() in {self.name}")

dynamic_stack_decider/dynamic_stack_decider/abstract_decision_element.py

@@ -1,55 +1,26 @@
+"""Base class for outcome-producing decisions."""
+
 from __future__ import annotations
 
-from abc import ABCMeta, abstractmethod
+from abc import abstractmethod
 
 from dynamic_stack_decider.abstract_stack_element import AbstractStackElement
 from dynamic_stack_decider.types import BlackboardT, ElementData
 
 
-class AbstractDecisionElement(AbstractStackElement[BlackboardT], metaclass=ABCMeta):
-    """
-    The logic is encapsulated in two types of elements.
-    The decision elements define the logical path similar to a behavior tree.
-    Corresponding decisions can be as complex as wished, using in the most simple case
-    if-else clauses or in a more complicated situation, for example, neural networks.
-    Either way, one decision-element should capsule one logical decision and determine
-    the following module, without giving any calls to the hardware level.
-    One example is the decision if the robot has sufficient knowledge about the
-    current ball position, using data from the vision components and the team-communication.
-    By using a push method they add further elements to the stack.
-    Thereby, each decision element can have other decision or actions as following active elements.
-    All decisions can, therefore, be displayed as a decision tree.
-    """
+class AbstractDecisionElement(AbstractStackElement[BlackboardT]):
+    """Select a branch from the blackboard; reevaluation should avoid hardware side effects."""
 
     _reevaluate = False
 
-    def repr_dict(self) -> ElementData:
-        """
-        Represent this stack element as dictionary which is JSON encodable
-        """
-        return {
-            "type": "decision",
-            "name": self.name,
-            "debug_data": self._debug_data,
-        }
-
     def get_reevaluate(self) -> bool:
-        """
-        Each decision element may define a \textit{reevaluate} criteria.
-        If the corresponding method returns true, the element will be executed even if it is in the middle of the stack.
-        This way a precondition can be checked, for example, every tenth iteration.
-        As an example, every iteration it could be checked if the ball position is known to the robot with a given certainty.
-        If the now pushed element is different from the element which is currently above in the stack, the whole
-        stack above will be dropped and the newly selected element executed.
-
-        This method returns whether the element should be reevaluated. This means that the element is recomputed
-        even if it is not on top of the stack. If the decision pushes a different element than in the original perform,
-        the stack above this decision is cleared.
-        This can be used to regularly check preconditions, e.g. if the robot knows where the ball is.
-        """
+        """Request reevaluation while this decision is below the current frame."""
         return self._reevaluate
 
+    def repr_dict(self) -> ElementData:
+        return {"type": "decision", "name": self.name, "debug_data": self._debug_data}
+
     @abstractmethod
     def perform(self, reevaluate: bool = False) -> str:
-        """Execute the element; decisions return an outcome and actions return nothing."""
+        """Return a non-empty outcome matching a branch or handled by ``ELSE``."""
         raise NotImplementedError(f"Implement perform() in {self.name}")

dynamic_stack_decider/dynamic_stack_decider/abstract_stack_element.py

@@ -1,6 +1,9 @@
+"""Shared lifecycle and debugging helpers for actions and decisions."""
+
 from __future__ import annotations
 
-from abc import ABCMeta, abstractmethod
+import json
+from abc import ABC, abstractmethod
 from typing import TYPE_CHECKING, Generic
 
 from dynamic_stack_decider.logger import get_logger
@@ -10,108 +13,44 @@
     from dynamic_stack_decider.dsd import DSD
 
 
-class AbstractStackElement(Generic[BlackboardT], metaclass=ABCMeta):
-    """
-    The AbstractStackElement is the basis of all elements on the stack.
-    It provides some help functions which should not be overloaded.
-    The work of an element is done in the :func:`perform`.
-    Each element which inherits from the AbstractStackElement can be used as a root element on the stack.
-    """
-
-    _dsd: DSD[BlackboardT]
-    parameters: Parameters
-
+class AbstractStackElement(Generic[BlackboardT], ABC):
     def __init__(self, blackboard: BlackboardT, dsd: DSD[BlackboardT], parameters: Parameters) -> None:
-        """
-        :param blackboard: Shared blackboard for data exchange between elements
-        :param dsd: The stack decider which has this element on its stack.
-        :param parameters: Parameters which serve as arguments to this element
-        """
-        self._debug_data: dict[str, object] = {}
-        """
-        This is a dict in which data can be saved that should get represented on a __repr__ call.
-        It can be set using publish_debug_data()
-        """
-
         self._dsd = dsd
-        self.parameters = parameters
         self.blackboard = blackboard
+        self.parameters = parameters
+        self._debug_data: dict[str, object] = {}
 
     @property
     def name(self) -> str:
-        """
-        Returns the name of the action
-        """
-        return self.__class__.__name__
+        return type(self).__name__
 
     def pop(self) -> None:
-        """
-        Help method which pops the element of the stack.
-
-        This method should always be called with a return:
-            return self.pop()
-
-        If no return is used, further code is executed after the pop, which leads to difficult to debug behavior.
-        """
+        """Complete this frame. Return immediately afterward to avoid using a discarded instance."""
         self._dsd.pop()
 
     def on_pop(self) -> None:
-        """
-        This method is called when the element is popped from the stack.
-        It can be used to clean up resources, cancel actions or similar tasks.
-        Overload this method if you need to do something when the element is popped.
-        """
-        pass
+        """Release resources when completed, preempted, interrupted, or replaced by another behavior."""
 
     @abstractmethod
     def perform(self, reevaluate: bool = False) -> str | None:
-        """
-        This method is called when the element is on top of the stack.
-        This method has to be overloaded by the implementation!
-
-        :param reevaluate: True if the current method call is a reevaluate of the state. Meaning the module is not on top of the stack.
-        """
-        msg = f"You should override perform() in {self.__class__.__name__}"
-        raise NotImplementedError(msg)
+        """Execute the element, or check a decision during reevaluation."""
+        raise NotImplementedError(f"Implement perform() in {self.name}")
 
     def interrupt(self) -> None:
-        """
-        An interrupt leads to a complete clearing of the stack.
-        """
+        """Restart the behavior. Return immediately afterward."""
         self._dsd.interrupt()
 
     def publish_debug_data(self, label: str, data: object) -> None:
-        """
-        Publish debug data. Can be viewed using the DSD visualization
-
-        This method is safe to call without wrapping it in a try-catch block although invalid values will
-        be wrapped in a `str()` call
-
-        :param label: A label that describes the given data
-        :param data: data that should be displayed for debugging purposes
-        """
-        if type(data) not in (dict, list, int, float, str, bool):
-            get_logger().debug(
-                f"The supplied debug data of type {type(data)} is not JSON serializable and will be wrapped in str()",
-                throttle_duration_sec=1,
-            )
+        """Store JSON-compatible debug data, representing unsupported values as strings."""
+        try:
+            data = json.loads(json.dumps(data, default=str, allow_nan=False))
+        except (TypeError, ValueError):
             data = str(data)
-
         get_logger().debug(f"{label}: {data}")
         self._debug_data[label] = data
 
     def clear_debug_data(self) -> None:
-        """
-        Clear existing debug data
-
-        This is needed when old values are no longer supposed to be visible
-        """
-        self._debug_data = {}
+        self._debug_data.clear()
 
     def repr_dict(self) -> ElementData:
-        """Represent this stack element as dictionary which is JSON encodable"""
-        return {
-            "type": "abstract",
-            "name": self.name,
-            "debug_data": self._debug_data,
-        }
+        return {"type": "abstract", "name": self.name, "debug_data": self._debug_data}