bug: Fix field removal and blanking to clean up child UID references

- Ensure child field UIDs are removed from internal lookup when parent sequence is deleted or blanked
- Update tests in test_action_interaction.py to show this behavior for blanked and removed sequences
- Prevent stale references and errors in subsequent recipe actions

Before the fix:

- If you removed or blanked a parent sequence tag, the internal field lookup (self.fields) still contained UIDs for child tags.
- Subsequent recipe actions targeting child tags could cause errors (KeyError, IndexError) or operate on stale references.

After the fix:

- When you remove or blank a parent sequence tag, all child UIDs nested under that parent are also removed from self.fields.
- Subsequent actions do not find stale child references, preventing errors and ensuring consistency.

Author: Simlomb

deid/dicom/parser.py

@@ -181,22 +181,42 @@ def delete_field(self, field):
         Delete a field from the dicom.
 
         We do this by way of parsing all nested levels of a tag into actual tags,
-        and deleting the child node.
+        and deleting the child node. If the field being deleted has nested children
+        (e.g., a sequence), also remove all child field UIDs from the internal lookup.
         """
         # Returns the parent, and a DataElement (indexes into parent by tag)
         parent, desired = self.get_nested_field(field, return_parent=True)
         if parent and desired in parent:
             del parent[desired]
+            # Remove the field itself from the lookup
             self.fields.remove(field.uid)
+            # Also remove any child fields that were nested under this field
+            field_uid_prefix = field.uid + "__"
+            child_uids = [
+                uid for uid in self.fields if uid.startswith(field_uid_prefix)
+            ]
+            for child_uid in child_uids:
+                self.fields.remove(child_uid)
 
     def blank_field(self, field):
         """
-        Blank a field
+        Blank a field.
+
+        If the field being blanked has nested children (e.g., a sequence),
+        also remove all child field UIDs from the internal lookup since they
+        become inaccessible after blanking the parent.
         """
         # Returns the parent, and a DataElement (indexes into parent by tag)
         parent, desired = self.get_nested_field(field, return_parent=True)
         if parent and desired in parent:
             parent[desired].value = None
+            # Also remove any child fields that were nested under this field
+            field_uid_prefix = field.uid + "__"
+            child_uids = [
+                uid for uid in self.fields if uid.startswith(field_uid_prefix)
+            ]
+            for child_uid in child_uids:
+                self.fields.remove(child_uid)
 
     def replace_field(self, field, value):
         """

deid/tests/test_action_interaction.py

@@ -1865,6 +1865,82 @@ def test_remove_remove_should_remove(self):
         with self.assertRaises(KeyError):
             _ = outputfile[field].value
 
+    def test_remove_parent_tag_replace_child_with_no_conflict(self):
+        """RECIPE RULE
+        REMOVE (0032,1064) Requested Procedure Code Sequence
+        REPLACE (0008,0104) "Anonymized"
+        """
+        print(
+            "Test REMOVE standard sequence tag and REPLACE child tag should not cause conflict"
+        )
+        dicom_file = get_file(self.dataset)
+
+        field1 = "(0032,1064)"
+        action1 = "REMOVE"
+        action2 = "REPLACE"
+        value2 = "Anonymized"
+        field2 = "(0008,0104)"
+        field_dicom1 = "00321064"
+        field_dicom2 = "00080104"
+
+        actions = [
+            {"action": action1, "field": field1},
+            {"action": action2, "field": field2, "value": value2},
+        ]
+        recipe = create_recipe(actions)
+
+        result = replace_identifiers(
+            dicom_files=dicom_file,
+            deid=recipe,
+            save=True,
+            remove_private=False,
+            strip_sequences=False,
+        )
+        outputfile = utils.dcmread(result[0])
+
+        self.assertEqual(1, len(result))
+        with self.assertRaises(KeyError):
+            _ = outputfile[field_dicom1].value
+            _ = outputfile[field_dicom1][0][field_dicom2].value
+
+    def test_blank_parent_tag_remove_child_with_no_conflict(self):
+        """RECIPE RULE
+        BLANK (0032,1064) Requested Procedure Code Sequence
+        REPLACE (0008,0104) "Anonymized"
+        """
+        print(
+            "Test BLANK standard sequence tag and REPLACE child tag should not cause conflict"
+        )
+        dicom_file = get_file(self.dataset)
+
+        field1 = "(0032,1064)"
+        action1 = "BLANK"
+        action2 = "REPLACE"
+        value2 = "Anonymized"
+        field2 = "(0008,0104)"
+        field_dicom1 = "00321064"
+        field_dicom2 = "00080104"
+
+        actions = [
+            {"action": action1, "field": field1},
+            {"action": action2, "field": field2, "value": value2},
+        ]
+        recipe = create_recipe(actions)
+
+        result = replace_identifiers(
+            dicom_files=dicom_file,
+            deid=recipe,
+            save=True,
+            remove_private=False,
+            strip_sequences=False,
+        )
+        outputfile = utils.dcmread(result[0])
+
+        self.assertEqual(1, len(result))
+        self.assertEqual(len(outputfile[field_dicom1].value), 0)
+        with self.assertRaises(IndexError):
+            _ = outputfile[field_dicom1][0][field_dicom2].value
+
 
 if __name__ == "__main__":
     unittest.main()