fix: Tape.__getitem__ ignores slice step; last_actions() breaks on n_added_steps=0

tapeagents/core.py

@@ -306,7 +306,11 @@ def __len__(self) -> int:
 
     def __getitem__(self, key: int | slice) -> StepType | Self:
         if isinstance(key, slice):  # cut and erase metadata
-            return self.model_copy(update=dict(steps=self.steps[key.start : key.stop], metadata=TapeMetadata()))
+            # Pass the slice object through directly rather than reconstructing
+            # it from start/stop -- that discarded key.step, silently ignoring
+            # a step component (tape[::2], tape[::-1], tape[1:5:2], etc.)
+            # instead of applying or rejecting it.
+            return self.model_copy(update=dict(steps=self.steps[key], metadata=TapeMetadata()))
         return self.steps[key]
 
     def __add__(self, tape: Self | Iterable[Step]) -> Self:
@@ -470,4 +474,12 @@ def llm_dict(self) -> dict[str, Any]:
 
 
 def last_actions(tape: Tape) -> list[Action]:
+    # n_added_steps == 0 is a real, legitimate value (default, and produced by
+    # __add__ when zero new steps were added) -- Python's `-0 == 0`, so
+    # `tape.steps[-0:]` returns the WHOLE list rather than an empty one.
+    # Environment.react()/areact() use this to decide which actions to
+    # execute against tools; without this guard, a zero-new-steps iteration
+    # would silently re-execute every action in the tape's entire history.
+    if tape.metadata.n_added_steps <= 0:
+        return []
     return [step for step in tape.steps[-tape.metadata.n_added_steps :] if isinstance(step, Action)]

tests/test_core.py

@@ -1,7 +1,7 @@
 import json
 from typing import Literal
 
-from tapeagents.core import Action, Tape, TapeMetadata
+from tapeagents.core import Action, Tape, TapeMetadata, last_actions
 
 
 class DummyStep(Action):
@@ -86,6 +86,44 @@ def test_getitem_slice_empty():
     assert sliced_tape.metadata.author is None
 
 
+def test_getitem_slice_with_step_reversed():
+    steps = [DummyStep(), DummyStep(), DummyStep()]
+    tape = TestTape(steps=steps)
+
+    sliced_tape = tape[::-1]
+
+    assert [step.metadata.id for step in sliced_tape.steps] == [step.metadata.id for step in reversed(steps)]
+
+
+def test_getitem_slice_with_step_stride():
+    steps = [DummyStep(), DummyStep(), DummyStep(), DummyStep(), DummyStep()]
+    tape = TestTape(steps=steps)
+
+    sliced_tape = tape[::2]
+
+    assert [step.metadata.id for step in sliced_tape.steps] == [steps[0].metadata.id, steps[2].metadata.id, steps[4].metadata.id]
+
+
+def test_last_actions_zero_added_steps_returns_empty():
+    """n_added_steps == 0 is the field's own default AND a value __add__
+    legitimately produces when zero new steps were added -- Python's
+    `-0 == 0` previously made `tape.steps[-0:]` return the WHOLE list
+    instead of an empty one, which would make Environment.react() silently
+    re-execute every action in the tape's entire history."""
+    tape = TestTape(steps=[DummyStep(), DummyStep()], metadata=TapeMetadata(n_added_steps=0))
+
+    assert last_actions(tape) == []
+
+
+def test_last_actions_positive_n_added_steps_unchanged():
+    steps = [DummyStep(), DummyStep(), DummyStep()]
+    tape = TestTape(steps=steps, metadata=TapeMetadata(n_added_steps=2))
+
+    result = last_actions(tape)
+
+    assert [step.metadata.id for step in result] == [steps[1].metadata.id, steps[2].metadata.id]
+
+
 def test_append_single_step():
     tape = TestTape(steps=[DummyStep()])
     new_step = DummyStep()