From 5a50aa53083e920fa83f503a26f239aceddc957e Mon Sep 17 00:00:00 2001
From: Mikhail Petrov <azalio@azalio.net>
Date: Tue, 21 Oct 2025 08:19:03 +0300
Subject: [PATCH] Handle missing recitation subtask IDs

---
 src/mapify_cli/recitation_manager.py | 28 ++++++++++++++++++----------
 tests/test_recitation_manager.py     |  8 +++-----
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/src/mapify_cli/recitation_manager.py b/src/mapify_cli/recitation_manager.py
index bf90a623..fa7f8c4d 100644
--- a/src/mapify_cli/recitation_manager.py
+++ b/src/mapify_cli/recitation_manager.py
@@ -151,15 +151,23 @@ def update_subtask_status(
                 "'python -m mapify_cli.recitation_manager create <task_id> <goal> <subtasks_json>'"
             )
 
-        for subtask in plan.subtasks:
-            if subtask.id == subtask_id:
-                subtask.status = status
-                if status == 'in_progress':
-                    plan.current_subtask_id = subtask_id
-                    subtask.iterations += 1
-                if error:
-                    subtask.errors.append(error)
-                break
+        # Find the target subtask up front so we can validate and log reliably
+        target_subtask = next(
+            (subtask for subtask in plan.subtasks if subtask.id == subtask_id),
+            None
+        )
+
+        if target_subtask is None:
+            raise ValueError(
+                f"Subtask with id {subtask_id} was not found in the current plan"
+            )
+
+        target_subtask.status = status
+        if status == 'in_progress':
+            plan.current_subtask_id = subtask_id
+            target_subtask.iterations += 1
+        if error:
+            target_subtask.errors.append(error)
 
         plan.updated_at = datetime.now().isoformat()
 
@@ -175,7 +183,7 @@ def update_subtask_status(
                     "subtask_id": subtask_id,
                     "status": status,
                     "error": error,
-                    "iterations": subtask.iterations if subtask else None
+                    "iterations": target_subtask.iterations
                 }
             )
 
diff --git a/tests/test_recitation_manager.py b/tests/test_recitation_manager.py
index 5644679d..8b7ce9b8 100644
--- a/tests/test_recitation_manager.py
+++ b/tests/test_recitation_manager.py
@@ -442,11 +442,9 @@ def test_update_nonexistent_subtask(self, manager, sample_subtasks):
         """Test updating a subtask that doesn't exist"""
         manager.create_plan('test', 'Test', sample_subtasks)
 
-        # Should not raise error, but also shouldn't update anything
-        plan = manager.update_subtask_status(999, 'completed')
-
-        # No subtask should be completed
-        assert all(st.status == 'pending' for st in plan.subtasks)
+        # Should raise an error when subtask ID is not found
+        with pytest.raises(ValueError, match="Subtask with id 999"):
+            manager.update_subtask_status(999, 'completed')
 
     def test_long_error_message(self, manager, sample_subtasks):
         """Test that long error messages are truncated in markdown"""