From 97807961c4de3364737a631dbb6c5c726256d122 Mon Sep 17 00:00:00 2001
From: David Ormsbee <dave@axim.org>
Date: Wed, 14 Oct 2026 18:01:40 -0400
Subject: [PATCH 1/3] fix: reject a child row that keeps its renamed parent's
 stale external_id

Closes #858. Alternative to #867 with a much smaller footprint.

UpdateParentTag.applies_for compared the row's parent_id with the parent's
pre-import external_id, so a row that kept the old id was "no change" and
_validate_parent never ran for it. When the same file renamed that parent
away (tag_1 -> tag_50), the stale reference was accepted silently.

Now such a row applies as a parent change whenever its parent is renamed
away (`_vacated_pks`), and _validate_parent rejects it with the existing
"renamed to X; use X" error. _validate_parent also rejects a row whose
parent_id is its own parent's pre-import id when another row takes that id
(a swap): that reference is ambiguous, while a child naming the end-state
id of a swapped parent is still accepted. A case-only rename no longer
counts as vacating the id, since external_id is case-insensitive.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---
 src/openedx_tagging/import_export/actions.py  | 101 ++++++++++--------
 .../import_export/import_plan.py              |   8 +-
 .../openedx_tagging/import_export/test_api.py |  38 +++++++
 .../import_export/test_import_plan.py         |  62 +++++++++++
 4 files changed, 161 insertions(+), 48 deletions(-)

diff --git a/src/openedx_tagging/import_export/actions.py b/src/openedx_tagging/import_export/actions.py
index 96aced3f1..4b662cf68 100644
--- a/src/openedx_tagging/import_export/actions.py
+++ b/src/openedx_tagging/import_export/actions.py
@@ -105,51 +105,54 @@ def _search_action(
 
     def _validate_parent(self, indexed_actions) -> ImportActionError | None:
         """
-        Validate that `parent_id` names a tag that will exist once this import finishes.
+        Validate that `parent_id` names the tag that will hold that id once this import finishes.
 
-        A match against a tag being renamed away in this same import (see
-        `_vacated_pks`) doesn't count as the real parent: it falls through to the
-        same "created or renamed-in this import" check as a brand-new parent. If
-        nothing lands on that vacated id either, the error names the id it was
-        renamed to, instead of the generic "unknown parent" message for a parent
-        never mentioned at all.
+        That is the tag currently holding `parent_id`, unless this import renames it
+        away (see `_vacated_pks`). A renamed-away id, like a brand-new one, must be
+        created or renamed-in by a previous action. Even then it must not be this
+        row's own current parent: a row that names its parent's pre-import id while
+        another row takes that id (a swap) could mean "stay with my parent" or "move
+        under the tag now called `parent_id`", so it is rejected. The error for a
+        renamed-away id names the id it was renamed to, instead of the generic
+        "unknown parent" message for an id never mentioned at all.
         """
         vacated_pks: dict[int, str] = indexed_actions.get("_vacated_pks", {})
-        parent_tag = None
-        is_vacated = False
-        try:
-            # Validates that the parent exists on the taxonomy
-            parent_tag = self.taxonomy.tag_set.get(external_id=self.tag.parent_id)
-            if parent_tag.pk in vacated_pks:
-                is_vacated = True
-                raise Tag.DoesNotExist
-        except Tag.DoesNotExist:
-            # Or if the parent is created or renamed-in on previous actions
-            found = self._search_action(
-                indexed_actions, CreateTag.name, "id", self.tag.parent_id, casefold=True
-            ) or self._search_action(
-                indexed_actions, RenameTagExternalId.name, "id", self.tag.parent_id, casefold=True
+        parent_tag = self.taxonomy.tag_set.filter(external_id=self.tag.parent_id).first()
+        new_id = vacated_pks.get(parent_tag.pk) if parent_tag is not None else None
+        if parent_tag is not None and new_id is None:
+            return None  # The parent exists and keeps its id.
+
+        # Otherwise the id must be created or renamed-in by a previous action.
+        found = self._search_action(
+            indexed_actions, CreateTag.name, "id", self.tag.parent_id, casefold=True
+        ) or self._search_action(
+            indexed_actions, RenameTagExternalId.name, "id", self.tag.parent_id, casefold=True
+        )
+        if found and not (parent_tag is not None and self._is_current_parent(parent_tag)):
+            return None
+        if new_id is not None:
+            return ImportActionError(
+                action=self,
+                message=_(
+                    "Parent tag {parent_id} is renamed to {new_id} in this file; "
+                    "use {new_id} as the parent_id."
+                ).format(parent_id=self.tag.parent_id, new_id=new_id),
             )
-            if not found:
-                if is_vacated:
-                    # is_vacated is only set after parent_tag was assigned above.
-                    assert parent_tag is not None
-                    new_id = vacated_pks[parent_tag.pk]
-                    return ImportActionError(
-                        action=self,
-                        message=_(
-                            "Parent tag {parent_id} is renamed to {new_id} in this file; "
-                            "use {new_id} as the parent_id."
-                        ).format(parent_id=self.tag.parent_id, new_id=new_id),
-                    )
-                return ImportActionError(
-                    action=self,
-                    message=_(
-                        "Unknown parent tag ({parent_id}). "
-                        "You need to add parent before the child in your file."
-                    ).format(parent_id=self.tag.parent_id),
-                )
-        return None
+        return ImportActionError(
+            action=self,
+            message=_(
+                "Unknown parent tag ({parent_id}). "
+                "You need to add parent before the child in your file."
+            ).format(parent_id=self.tag.parent_id),
+        )
+
+    def _is_current_parent(self, parent_tag: Tag) -> bool:
+        """
+        Whether `parent_tag` is the parent that this row's tag has before the import.
+        """
+        return self.taxonomy.tag_set.filter(
+            external_id=self.tag.previous_id or self.tag.id, parent=parent_tag,
+        ).exists()
 
     def _validate_value(self, indexed_actions) -> ImportActionError | None:
         """
@@ -343,16 +346,24 @@ def applies_for(cls, taxonomy: Taxonomy, tag: TagItem, indexed_actions=None) ->
         """
         This action applies whenever there is a change on the parent.
 
+        A row that keeps its parent's pre-import id while this import renames the
+        parent away from it also counts: the row no longer names its parent, so it
+        must be validated like any other parent change (`_validate_parent` rejects
+        it). Without this, such a row was "no change" and the stale id got through.
+
         See ImportAction._resolve_update_target for when this doesn't apply
         regardless of the parent change (a rename_external_id row).
         """
         taxonomy_tag = cls._resolve_update_target(taxonomy, tag)
         if taxonomy_tag is None:
             return False
-        return (
-            taxonomy_tag.parent is not None
-            and taxonomy_tag.parent.external_id != tag.parent_id
-        ) or (taxonomy_tag.parent is None and tag.parent_id is not None)
+        parent = taxonomy_tag.parent
+        if parent is None:
+            return tag.parent_id is not None
+        if parent.external_id != tag.parent_id:
+            return True
+        vacated_pks = indexed_actions.get("_vacated_pks", {}) if indexed_actions else {}
+        return parent.pk in vacated_pks
 
     def validate(self, indexed_actions) -> list[ImportActionError]:
         """
diff --git a/src/openedx_tagging/import_export/import_plan.py b/src/openedx_tagging/import_export/import_plan.py
index b39cbddda..41f16fdcb 100644
--- a/src/openedx_tagging/import_export/import_plan.py
+++ b/src/openedx_tagging/import_export/import_plan.py
@@ -207,7 +207,8 @@ def _build_staging_actions(self, tags: list[TagItem]) -> None:
         or not: once a tag is renamed away from an external_id, that id is stale
         for anyone still referencing it via the live database, even if nothing
         in this import reuses it (see _validate_parent, which uses the recorded
-        new id to name it).
+        new id to name it). A case-only rename is neither staged nor recorded:
+        external_id is case-insensitive, so the tag keeps its id.
         """
         target_ids = {
             tag.id.casefold() for tag in tags
@@ -222,9 +223,10 @@ def _build_staging_actions(self, tags: list[TagItem]) -> None:
             target_pk = self._resolve_rename_target_pk(tag)
             if target_pk is None:
                 continue
+            if tag.previous_id.casefold() == tag.id.casefold():
+                continue  # A case-only rename keeps the id: nothing is vacated or contended.
             vacated_pks[target_pk] = tag.id
-            is_case_only_rename = tag.previous_id.casefold() == tag.id.casefold()
-            if not is_case_only_rename and tag.previous_id.casefold() in target_ids:
+            if tag.previous_id.casefold() in target_ids:
                 self._build_action(StageTagExternalIdForSwap, tag, target_pk=target_pk)
         self.indexed_actions["_vacated_pks"] = vacated_pks
 
diff --git a/tests/openedx_tagging/import_export/test_api.py b/tests/openedx_tagging/import_export/test_api.py
index 65517f8e7..67523e820 100644
--- a/tests/openedx_tagging/import_export/test_api.py
+++ b/tests/openedx_tagging/import_export/test_api.py
@@ -808,6 +808,44 @@ def test_import_swap_external_ids(self) -> None:
         assert exported_by_value["Tag 1"] == "tag_3"
         assert exported_by_value["Tag 3"] == "tag_1"
 
+    def test_import_swap_external_ids_with_children(self) -> None:
+        """
+        tag_1 and tag_3 swap ids, and each of their children names the id its
+        parent will hold after the import: tag_2 (under tag_1, which becomes
+        tag_3) says parent_id=tag_3, and tag_4 (under tag_3, which becomes tag_1)
+        says parent_id=tag_1. The full-replace import succeeds and each child
+        stays on the same physical parent. See
+        test_import_plan.TestTagImportPlan
+        .test_generate_actions_swap_child_naming_own_parent_stale_id_rejected
+        for the children naming the pre-import ids instead.
+        """
+        old_pk_1 = self.taxonomy.tag_set.get(external_id="tag_1").pk
+        old_pk_3 = self.taxonomy.tag_set.get(external_id="tag_3").pk
+
+        importFile = BytesIO(json.dumps({"tags": [
+            {"id": "tag_3", "value": "Tag 1", "previous_id": "tag_1"},
+            {"id": "tag_1", "value": "Tag 3", "previous_id": "tag_3"},
+            {"id": "tag_2", "value": "Tag 2", "parent_id": "tag_3"},
+            {"id": "tag_4", "value": "Tag 4", "parent_id": "tag_1"},
+        ]}).encode())
+        result, task, _plan = import_export_api.import_tags(
+            self.taxonomy,
+            importFile,
+            self.parser_format,
+            replace=True,
+        )
+        assert "Traceback" not in task.log
+        assert result, task.log
+
+        tag_2 = self.taxonomy.tag_set.get(external_id="tag_2")
+        tag_4 = self.taxonomy.tag_set.get(external_id="tag_4")
+        assert tag_2.parent_id == old_pk_1
+        assert tag_4.parent_id == old_pk_3
+
+        output = import_export_api.export_tags(self.taxonomy, self.parser_format)
+        exported_parents = {tag["id"]: tag.get("parent_id") for tag in json.loads(output)["tags"]}
+        assert exported_parents == {"tag_3": None, "tag_2": "tag_3", "tag_1": None, "tag_4": "tag_1"}
+
     def test_import_three_cycle_external_ids(self) -> None:
         """
         End-to-end 3-cycle: tag_1 -> tag_2 -> tag_3 -> tag_1. Same staging
diff --git a/tests/openedx_tagging/import_export/test_import_plan.py b/tests/openedx_tagging/import_export/test_import_plan.py
index 9fd40b074..75d9d7e74 100644
--- a/tests/openedx_tagging/import_export/test_import_plan.py
+++ b/tests/openedx_tagging/import_export/test_import_plan.py
@@ -619,6 +619,68 @@ def test_generate_actions_parent_id_new_id_after_earlier_rename_accepted(self) -
         self.import_plan.generate_actions(tags=tags, replace=False)
         self.assertEqual(self.import_plan.errors, [])
 
+    def test_generate_actions_existing_child_keeps_stale_parent_id_rejected(self) -> None:
+        """
+        tag_2 is already under tag_1. The file renames tag_1 to tag_50 while the
+        tag_2 row still says parent_id=tag_1. That row matches the parent's
+        pre-import id, so it used to count as "no change" and slipped past
+        _validate_parent, although tag_1 names nothing after the import. It must
+        be rejected, and the error must name the parent's new id.
+        """
+        tags = [
+            TagItem(id='tag_50', value='Tag 1', previous_id='tag_1'),
+            TagItem(id='tag_2', value='Tag 2', parent_id='tag_1'),
+        ]
+        self.import_plan.generate_actions(tags=tags, replace=False)
+        self.assertEqual(
+            [str(error) for error in self.import_plan.errors],
+            [
+                "Action error in 'update_parent' (#2): "
+                "Parent tag tag_1 is renamed to tag_50 in this file; use tag_50 as the parent_id."
+            ],
+        )
+
+    @ddt.data(
+        ('tag_2', None, 'tag_1'),  # exact pre-import id
+        ('tag_2', None, 'TAG_1'),  # case variant of it
+        ('tag_20', 'tag_2', 'tag_1'),  # the child is renamed too
+    )
+    @ddt.unpack
+    def test_generate_actions_swap_child_naming_own_parent_stale_id_rejected(
+        self, child_id: str, child_previous_id: str | None, child_parent_id: str,
+    ) -> None:
+        """
+        tag_2 is under tag_1, and the file swaps the ids of tag_1 and tag_3. After
+        the import "tag_1" names the old tag_3, so a tag_2 row that still says
+        parent_id=tag_1 could mean "stay with my parent" or "move under the tag
+        now called tag_1". It is rejected, once, and told to use tag_3.
+        """
+        tags = [
+            TagItem(id='tag_3', value='Tag 1', previous_id='tag_1'),
+            TagItem(id='tag_1', value='Tag 3', previous_id='tag_3'),
+            TagItem(id=child_id, value='Tag 2', previous_id=child_previous_id, parent_id=child_parent_id),
+        ]
+        self.import_plan.generate_actions(tags=tags, replace=False)
+        self.assertEqual(len(self.import_plan.errors), 1)
+        self.assertIn(
+            f"Parent tag {child_parent_id} is renamed to tag_3 in this file; use tag_3 as the parent_id.",
+            str(self.import_plan.errors[0]),
+        )
+
+    @ddt.data('tag_1', 'TAG_1')
+    def test_generate_actions_case_only_parent_rename_keeps_child(self, child_parent_id: str) -> None:
+        """
+        Renaming tag_1 to TAG_1 only changes its capitalization. external_id is
+        case-insensitive, so the id still names the same tag after the import,
+        and a tag_2 row naming it in either spelling is not a stale reference.
+        """
+        tags = [
+            TagItem(id='TAG_1', value='Tag 1', previous_id='tag_1'),
+            TagItem(id='tag_2', value='Tag 2', parent_id=child_parent_id),
+        ]
+        self.import_plan.generate_actions(tags=tags, replace=False)
+        self.assertEqual(self.import_plan.errors, [])
+
     def test_generate_actions_genuine_collision_not_staged(self) -> None:
         """
         A rename targeting an id held by an unrelated tag that is not

From 66d5fb1b1ed1a22e6525bdf698c34690daf31292 Mon Sep 17 00:00:00 2001
From: David Ormsbee <dave@axim.org>
Date: Wed, 14 Oct 2026 18:32:32 -0400
Subject: [PATCH 2/3] fix: make tag import parent changes order-independent and
 reject bad trees

Covers the remaining bugs #867 found around parent_id handling, with one
mechanism instead of a dry-run simulator and a conditional rebuild:

- The plan computes the tree as it will be after the import (_end_state),
  rejects a parent cycle or a tree deeper than the limit there (Tag.save
  only validates the saved tag's depth, and silently corrupts lineage on a
  cycle), and records which existing tags change parent.
- Rows are built after the row that creates or renames-in their parent, so
  a child may precede its new parent in the file. _validate_parent
  recognises a renamed-in parent from _vacated_pks, whatever the row order.
- execute() detaches every moved tag before running the actions. After
  that each tag is at or above its final depth and none is an ancestor of
  its new parent, so no action can overflow or form a cycle part-way.
- Kept children of deleted tags are promoted before the first delete, so
  the cascade cannot remove a kept grandchild.
- A child whose deleted parent's id is reclaimed by a rename follows the
  renamed tag instead of being left at the root.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---
 src/openedx_tagging/import_export/actions.py  |  64 +++--
 .../import_export/exceptions.py               |  27 ++
 .../import_export/import_plan.py              | 215 +++++++++++++-
 .../import_export/test_actions.py             |  16 +-
 .../openedx_tagging/import_export/test_api.py |  19 +-
 .../import_export/test_import_plan.py         | 272 +++++++++++++++++-
 6 files changed, 559 insertions(+), 54 deletions(-)

diff --git a/src/openedx_tagging/import_export/actions.py b/src/openedx_tagging/import_export/actions.py
index 4b662cf68..3cc228d68 100644
--- a/src/openedx_tagging/import_export/actions.py
+++ b/src/openedx_tagging/import_export/actions.py
@@ -109,12 +109,15 @@ def _validate_parent(self, indexed_actions) -> ImportActionError | None:
 
         That is the tag currently holding `parent_id`, unless this import renames it
         away (see `_vacated_pks`). A renamed-away id, like a brand-new one, must be
-        created or renamed-in by a previous action. Even then it must not be this
-        row's own current parent: a row that names its parent's pre-import id while
-        another row takes that id (a swap) could mean "stay with my parent" or "move
-        under the tag now called `parent_id`", so it is rejected. The error for a
-        renamed-away id names the id it was renamed to, instead of the generic
-        "unknown parent" message for an id never mentioned at all.
+        given to a tag by a row of this file: a rename onto it (`_vacated_pks` maps
+        every renamed tag to its new id, whatever the row order) or a create action
+        built before this one (TagImportPlan builds a new parent before its
+        children). Even then it must not be this row's own current parent: a row
+        that names its parent's pre-import id while another row takes that id (a
+        swap) could mean "stay with my parent" or "move under the tag now called
+        `parent_id`", so it is rejected. The error for a renamed-away id names the
+        id it was renamed to, instead of the generic "unknown parent" message for
+        an id never mentioned at all.
         """
         vacated_pks: dict[int, str] = indexed_actions.get("_vacated_pks", {})
         parent_tag = self.taxonomy.tag_set.filter(external_id=self.tag.parent_id).first()
@@ -122,12 +125,9 @@ def _validate_parent(self, indexed_actions) -> ImportActionError | None:
         if parent_tag is not None and new_id is None:
             return None  # The parent exists and keeps its id.
 
-        # Otherwise the id must be created or renamed-in by a previous action.
-        found = self._search_action(
-            indexed_actions, CreateTag.name, "id", self.tag.parent_id, casefold=True
-        ) or self._search_action(
-            indexed_actions, RenameTagExternalId.name, "id", self.tag.parent_id, casefold=True
-        )
+        found = (self.tag.parent_id or "").casefold() in {
+            renamed_to.casefold() for renamed_to in vacated_pks.values()
+        } or self._search_action(indexed_actions, CreateTag.name, "id", self.tag.parent_id, casefold=True)
         if found and not (parent_tag is not None and self._is_current_parent(parent_tag)):
             return None
         if new_id is not None:
@@ -142,7 +142,7 @@ def _validate_parent(self, indexed_actions) -> ImportActionError | None:
             action=self,
             message=_(
                 "Unknown parent tag ({parent_id}). "
-                "You need to add parent before the child in your file."
+                "Add a row for the parent to your file."
             ).format(parent_id=self.tag.parent_id),
         )
 
@@ -329,27 +329,33 @@ class UpdateParentTag(ImportAction):
     """
 
     name = "update_parent"
+    _description: str | None = None
 
     def __str__(self) -> str:
-        taxonomy_tag = self._get_tag()
-
-        description_str = _("Update the parent of {tag} from parent {old_parent} to {new_parent}").format(
-            tag=taxonomy_tag.display_str(),
-            old_parent=taxonomy_tag.parent.display_str() if taxonomy_tag.parent else None,
-            new_parent=self.tag.parent_id,
-        )
-
-        return str(description_str)
+        # Describe the change as of the plan: the plan is logged before it runs, and the
+        # execution detaches every moved tag first, which would then read as "from None".
+        if self._description is None:
+            taxonomy_tag = self._get_tag()
+            self._description = str(
+                _("Update the parent of {tag} from parent {old_parent} to {new_parent}").format(
+                    tag=taxonomy_tag.display_str(),
+                    old_parent=taxonomy_tag.parent.display_str() if taxonomy_tag.parent else None,
+                    new_parent=self.tag.parent_id,
+                )
+            )
+        return self._description
 
     @classmethod
     def applies_for(cls, taxonomy: Taxonomy, tag: TagItem, indexed_actions=None) -> bool:
         """
         This action applies whenever there is a change on the parent.
 
-        A row that keeps its parent's pre-import id while this import renames the
-        parent away from it also counts: the row no longer names its parent, so it
-        must be validated like any other parent change (`_validate_parent` rejects
-        it). Without this, such a row was "no change" and the stale id got through.
+        A row that names its parent's current id is still a parent change when
+        this import gives that id to another tag: by renaming the parent away
+        from it, or by renaming another tag onto it once the parent is deleted.
+        Such a row used to count as "no change", so a stale id got through
+        `_validate_parent` and a reclaimed id lost the child to the delete
+        phase's promotion.
 
         See ImportAction._resolve_update_target for when this doesn't apply
         regardless of the parent change (a rename_external_id row).
@@ -362,8 +368,10 @@ def applies_for(cls, taxonomy: Taxonomy, tag: TagItem, indexed_actions=None) ->
             return tag.parent_id is not None
         if parent.external_id != tag.parent_id:
             return True
-        vacated_pks = indexed_actions.get("_vacated_pks", {}) if indexed_actions else {}
-        return parent.pk in vacated_pks
+        vacated_pks: dict[int, str] = indexed_actions.get("_vacated_pks", {}) if indexed_actions else {}
+        return parent.pk in vacated_pks or parent.external_id.casefold() in {
+            new_id.casefold() for new_id in vacated_pks.values()
+        }
 
     def validate(self, indexed_actions) -> list[ImportActionError]:
         """
diff --git a/src/openedx_tagging/import_export/exceptions.py b/src/openedx_tagging/import_export/exceptions.py
index 1f1caf1f6..d248f0006 100644
--- a/src/openedx_tagging/import_export/exceptions.py
+++ b/src/openedx_tagging/import_export/exceptions.py
@@ -80,6 +80,33 @@ def __init__(self, tag_id: str, plain_row: int, rename_row: int, **kargs):
         ).format(tag_id=tag_id, plain_row=plain_row, rename_row=rename_row)
 
 
+class ParentCycleError(TagImportError):
+    """
+    Exception raised when a row's parent_id would make its tag its own ancestor.
+
+    ``row`` is the 1-based position of the row among the file's data rows, as
+    in DuplicateFinalIdError.
+    """
+
+    def __init__(self, row: int, parent_id: str, **kargs):
+        super().__init__(**kargs)
+        self.message = _(
+            "Row {row}'s parent_id ({parent_id}) creates a parent cycle."
+        ).format(row=row, parent_id=parent_id)
+
+
+class TagTooDeepError(TagImportError):
+    """
+    Exception raised when the import would leave a tag deeper than the taxonomy allows.
+    """
+
+    def __init__(self, tag_id: str, max_levels: int, **kargs):
+        super().__init__(**kargs)
+        self.message = _(
+            "This import would leave tag {tag_id} more than {max_levels} levels deep."
+        ).format(tag_id=tag_id, max_levels=max_levels)
+
+
 class ImportActionConflict(ImportActionError):
     """
     Exception used when exists a conflict between actions
diff --git a/src/openedx_tagging/import_export/import_plan.py b/src/openedx_tagging/import_export/import_plan.py
index 41f16fdcb..f23ea9a92 100644
--- a/src/openedx_tagging/import_export/import_plan.py
+++ b/src/openedx_tagging/import_export/import_plan.py
@@ -3,10 +3,13 @@
 """
 from __future__ import annotations
 
+from typing import NamedTuple
+
 from attrs import define
 from django.db import transaction
 
 from ..models import Tag, TagImportTask, Taxonomy
+from ..models.base import TAXONOMY_MAX_DEPTH
 from .actions import (
     DeleteTag,
     ImportAction,
@@ -16,7 +19,27 @@
     WithoutChanges,
     available_actions,
 )
-from .exceptions import DuplicateFinalIdError, StaleIdTargetedByPlainRowError, TagImportError
+from .exceptions import (
+    DuplicateFinalIdError,
+    ParentCycleError,
+    StaleIdTargetedByPlainRowError,
+    TagImportError,
+    TagTooDeepError,
+)
+
+
+class ExistingTag(NamedTuple):
+    """
+    What the plan needs to know about a tag that exists before the import.
+    """
+
+    external_id: str
+    pk: int
+    parent_pk: int | None
+
+
+# Stands in for the pk of a tag the import creates. Never equal to a real pk or to None.
+NEW_TAG = -1
 
 
 @define
@@ -43,6 +66,22 @@ def __str__(self):
 class TagImportPlan:
     """
     Class with functions to build an import plan and excute the plan
+
+    A row's ``parent_id`` names the id its parent holds *after* the import. Three steps
+    make parent changes work whatever the order of the rows in the file:
+
+    - The plan computes the tag tree as it will be after the import (``_end_state``),
+      rejects a parent cycle or a tree deeper than the limit there, and records which
+      existing tags change parent (``moved_tag_pks``).
+    - A row is built after the row that creates or renames-in its parent
+      (``_order_rows_parent_first``), so the per-action parent validation and the
+      execution both find the parent.
+    - ``execute`` first detaches every tag whose parent changes. After that each tag is
+      at or above its final depth, and none is an ancestor of its new parent, so no
+      action can exceed the depth limit or build a cycle part-way through.
+
+    "Row N" in an error is the N-th data row of the file, counting from 1: in a CSV file
+    that is spreadsheet line N + 1, because the header is line 1.
     """
 
     actions: list[ImportAction]
@@ -50,12 +89,14 @@ class TagImportPlan:
     indexed_actions: dict
     actions_dict: dict
     taxonomy: Taxonomy
+    moved_tag_pks: set[int]
 
     def __init__(self, taxonomy: Taxonomy):
         self.actions = []
         self.errors = []
         self.taxonomy = taxonomy
         self.actions_dict = {}
+        self.moved_tag_pks = set()
         self._init_indexed_actions()
 
     def _init_indexed_actions(self):
@@ -112,6 +153,9 @@ def _get_tag_id(self, tag: Tag) -> str:
     def _build_delete_actions(self, tags: dict):
         """
         Adds delete actions for `tags`
+
+        Every kept child of a deleted tag is first promoted to the root, before
+        any delete, because a delete cascades to the descendants.
         """
         for tag in tags.values():
             for child in tag.children.all():
@@ -129,6 +173,7 @@ def _build_delete_actions(self, tags: dict):
                             ),
                         )
 
+        for tag in tags.values():
             # Delete action
             self._build_action(
                 DeleteTag,
@@ -230,6 +275,153 @@ def _build_staging_actions(self, tags: list[TagItem]) -> None:
                 self._build_action(StageTagExternalIdForSwap, tag, target_pk=target_pk)
         self.indexed_actions["_vacated_pks"] = vacated_pks
 
+    def _existing_tags(self) -> dict[str, ExistingTag]:
+        """
+        Return the taxonomy's tags before the import, keyed by casefolded external_id. One query.
+        """
+        return {
+            external_id.casefold(): ExistingTag(external_id, pk, parent_pk)
+            for external_id, pk, parent_pk in self.taxonomy.tag_set.values_list("external_id", "pk", "parent_id")
+        }
+
+    def _end_state(
+        self, tags: list[TagItem], existing: dict[str, ExistingTag],
+    ) -> tuple[dict[str, str | None], dict[str, int], dict[str, int]]:
+        """
+        Compute the tag tree as it will be once the import is applied.
+
+        Returns three dicts keyed by casefolded final id: ``parent_of`` (the parent's
+        final id, or None for a root), ``pk_of`` (the pk of the tag that will hold the
+        id, or NEW_TAG) and ``row_of`` (the position in ``tags`` of the id's row, for
+        the ids that have one). Each row contributes its own id and parent_id. An
+        existing tag without a row is either deleted or keeps its parent, named by
+        that parent's final id. Must run after the delete and staging actions are
+        built.
+        """
+        vacated_pks: dict[int, str] = self.indexed_actions["_vacated_pks"]
+        deleted = {action.tag.id.casefold() for action in self.indexed_actions["delete"]}
+        key_of_pk = {info.pk: key for key, info in existing.items()}
+        parent_of: dict[str, str | None] = {}
+        pk_of: dict[str, int] = {}
+        row_of: dict[str, int] = {}
+        for position, tag in enumerate(tags):
+            key = tag.id.casefold()
+            if key in row_of:
+                continue  # Rejected by _validate_no_duplicate_final_ids.
+            current = existing.get((tag.previous_id or tag.id).casefold())
+            row_of[key] = position
+            pk_of[key] = current.pk if current is not None else NEW_TAG
+            parent_of[key] = tag.parent_id.casefold() if tag.parent_id else None
+        in_file = {key for tag in tags for key in (tag.id.casefold(), (tag.previous_id or "").casefold())}
+        for key, info in existing.items():
+            if key in in_file or key in deleted:
+                continue
+            pk_of[key] = info.pk
+            parent_of[key] = None
+            if info.parent_pk is not None:
+                parent_of[key] = vacated_pks.get(info.parent_pk, key_of_pk[info.parent_pk]).casefold()
+        return parent_of, pk_of, row_of
+
+    def _validate_hierarchy(
+        self,
+        tags: list[TagItem],
+        existing: dict[str, ExistingTag],
+        parent_of: dict[str, str | None],
+        pk_of: dict[str, int],
+        row_of: dict[str, int],
+    ) -> None:
+        """
+        Reject a parent cycle, and a tree deeper than the taxonomy allows.
+
+        A cycle is reported once, on its first row in the file. A cycle made only of
+        new tags is left to ``_validate_parent``, which reports one of them as an
+        unknown parent. A parent_id that is no tag of the end state counts as a root
+        here: it is unknown, which ``_validate_parent`` reports, or deleted without a
+        row reclaiming it, and the delete phase promotes its children.
+        """
+        depth_of: dict[str, int] = {}
+        unresolved: set[str] = set()  # on a cycle, or below one
+        for start in parent_of:
+            path: list[str] = []
+            node: str | None = start
+            while node is not None and node not in depth_of and node not in unresolved and node not in path:
+                path.append(node)
+                node = parent_of.get(node)
+                if node not in parent_of:
+                    node = None
+            if node in path:
+                assert node is not None
+                cycle = path[path.index(node):]
+                rows = [row_of[key] for key in cycle if key in row_of]
+                if rows and any(pk_of[key] != NEW_TAG for key in cycle):
+                    position = min(rows)
+                    self.errors.append(ParentCycleError(position + 1, tags[position].parent_id or ""))
+            if node is None or node in depth_of:
+                depth = depth_of[node] if node is not None else -1
+                for key in reversed(path):
+                    depth += 1
+                    depth_of[key] = depth
+            else:
+                unresolved.update(path)
+
+        too_deep = [key for key, depth in depth_of.items() if depth > TAXONOMY_MAX_DEPTH]
+        if too_deep:
+            # Name the shallowest too-deep tag: the first one past the limit.
+            key = min(too_deep, key=lambda key: (depth_of[key], row_of.get(key, len(tags)), key))
+            tag_id = tags[row_of[key]].id if key in row_of else existing[key].external_id
+            self.errors.append(TagTooDeepError(tag_id, TAXONOMY_MAX_DEPTH + 1))
+
+    @staticmethod
+    def _moved_tag_pks(
+        existing: dict[str, ExistingTag], parent_of: dict[str, str | None], pk_of: dict[str, int],
+    ) -> set[int]:
+        """
+        Return the pks of the existing tags whose parent after the import is a different tag than before.
+        """
+        current_parent_pk = {info.pk: info.parent_pk for info in existing.values()}
+        moved = set()
+        for key, pk in pk_of.items():
+            if pk == NEW_TAG:
+                continue
+            parent_key = parent_of[key]
+            final_parent_pk = pk_of.get(parent_key) if parent_key is not None else None
+            if final_parent_pk != current_parent_pk[pk]:
+                moved.add(pk)
+        return moved
+
+    @staticmethod
+    def _order_rows_parent_first(tags: list[TagItem], existing: dict[str, ExistingTag]) -> list[TagItem]:
+        """
+        Return ``tags`` with each row after the row that creates or renames-in its parent, if any.
+
+        The per-action parent validation and the execution look for a parent among the
+        existing tags and the actions built so far, so this makes both independent of
+        the row order. Rows keep their file order otherwise: a row is pulled forward only
+        when a later row needs it, and a file whose parents already come before their
+        children is left as it is.
+        """
+        defining_row: dict[str, int] = {}
+        for position, tag in enumerate(tags):
+            is_rename = bool(tag.previous_id) and tag.id != tag.previous_id
+            if is_rename or tag.id.casefold() not in existing:
+                defining_row.setdefault(tag.id.casefold(), position)
+        ordered: list[TagItem] = []
+        emitted: set[int] = set()
+        for start in range(len(tags)):
+            chain: list[int] = []
+            position = start
+            while position not in emitted and position not in chain:
+                chain.append(position)
+                parent_id = tags[position].parent_id
+                parent_row = defining_row.get(parent_id.casefold()) if parent_id else None
+                if parent_row is None:
+                    break
+                position = parent_row
+            for row in reversed(chain):
+                emitted.add(row)
+                ordered.append(tags[row])
+        return ordered
+
     def generate_actions(
         self,
         tags: list[TagItem],
@@ -248,7 +440,9 @@ def generate_actions(
         """
         self.actions.clear()
         self.errors.clear()
+        self.moved_tag_pks.clear()
         self._init_indexed_actions()
+        existing = self._existing_tags()
 
         # Reject two or more rows claiming the same final id outright,
         # before staging or per-row action-building runs (see
@@ -293,7 +487,13 @@ def generate_actions(
         # execution order regardless of how the rows are ordered in the file.
         self._build_staging_actions(tags)
 
-        for tag in tags:
+        # Reject what no single action can see: a cycle or an overflow of the tree the
+        # rows describe together. Record the tags to detach before executing.
+        parent_of, pk_of, row_of = self._end_state(tags, existing)
+        self._validate_hierarchy(tags, existing, parent_of, pk_of, row_of)
+        self.moved_tag_pks = self._moved_tag_pks(existing, parent_of, pk_of)
+
+        for tag in self._order_rows_parent_first(tags, existing):
             has_action = False
 
             # Check all available actions and add which ones should be executed
@@ -337,6 +537,17 @@ def execute(self, task: TagImportTask | None = None):
         """
         if self.errors:
             return
+        # Detach every tag whose parent changes before any action runs, so that no
+        # action can exceed the depth limit or build a cycle part-way through (see
+        # the class docstring). Each action then attaches its tag to its new parent.
+        if self.moved_tag_pks:
+            if task:
+                task.add_log("Detaching the tags whose parent changes [Started]", save=False)
+            for tag in self.taxonomy.tag_set.filter(pk__in=self.moved_tag_pks, parent__isnull=False):
+                tag.parent = None
+                tag.save()
+            if task:
+                task.add_log("Success", save=False)
         for action in self.actions:
             # Avoid to save each log because it is slow and costs a lot in memory
             # It is necessary to save at the end.
diff --git a/tests/openedx_tagging/import_export/test_actions.py b/tests/openedx_tagging/import_export/test_actions.py
index cbec11b54..511ebae67 100644
--- a/tests/openedx_tagging/import_export/test_actions.py
+++ b/tests/openedx_tagging/import_export/test_actions.py
@@ -132,7 +132,7 @@ def test_validate_parent(self, parent_id: str, expected: bool):
                 (
                     "Action error in 'import_action' (#100): "
                     "Unknown parent tag (tag_100). "
-                    "You need to add parent before the child in your file."
+                    "Add a row for the parent to your file."
                 )
             )
 
@@ -140,16 +140,12 @@ def test_validate_parent_with_rename_external_id_action(self) -> None:
         """
         Regression: a parent referenced by external_id that doesn't exist in
         the DB yet, but is being renamed-in via a `RenameTagExternalId`
-        action earlier in the same import, must validate as a known parent.
+        action in the same import (recorded in `_vacated_pks` by the plan,
+        wherever that row is in the file), must validate as a known parent.
         """
-        indexed_actions = dict(self.indexed_actions)
-        indexed_actions['rename_external_id'] = [
-            RenameTagExternalId(
-                taxonomy=self.taxonomy,
-                tag=TagItem(id='tag_60', value='Tag 60', previous_id='tag_3', index=1),
-                index=1,
-            )
-        ]
+        tag_3_pk = self.taxonomy.tag_set.get(external_id='tag_3').pk
+        indexed_actions: dict[str, list[ImportAction] | dict[int, str]] = dict(self.indexed_actions)
+        indexed_actions['_vacated_pks'] = {tag_3_pk: 'tag_60'}
         action = ImportAction(
             self.taxonomy,
             TagItem(
diff --git a/tests/openedx_tagging/import_export/test_api.py b/tests/openedx_tagging/import_export/test_api.py
index 67523e820..0cb11df63 100644
--- a/tests/openedx_tagging/import_export/test_api.py
+++ b/tests/openedx_tagging/import_export/test_api.py
@@ -4,6 +4,7 @@
 import json
 from io import BytesIO
 
+import ddt  # type: ignore[import]
 from django.test.testcases import TestCase
 
 import openedx_tagging.import_export.api as import_export_api
@@ -13,6 +14,7 @@
 from .mixins import TestImportExportMixin
 
 
+@ddt.ddt
 class TestImportExportApi(TestImportExportMixin, TestCase):
     """
     Test import/export API functions
@@ -808,26 +810,31 @@ def test_import_swap_external_ids(self) -> None:
         assert exported_by_value["Tag 1"] == "tag_3"
         assert exported_by_value["Tag 3"] == "tag_1"
 
-    def test_import_swap_external_ids_with_children(self) -> None:
+    @ddt.data(True, False)
+    def test_import_swap_external_ids_with_children(self, children_first: bool) -> None:
         """
         tag_1 and tag_3 swap ids, and each of their children names the id its
         parent will hold after the import: tag_2 (under tag_1, which becomes
         tag_3) says parent_id=tag_3, and tag_4 (under tag_3, which becomes tag_1)
-        says parent_id=tag_1. The full-replace import succeeds and each child
-        stays on the same physical parent. See
-        test_import_plan.TestTagImportPlan
+        says parent_id=tag_1. The full-replace import succeeds, whether the
+        children's rows come before or after the renames, and each child stays on
+        the same physical parent. See test_import_plan.TestTagImportPlan
         .test_generate_actions_swap_child_naming_own_parent_stale_id_rejected
         for the children naming the pre-import ids instead.
         """
         old_pk_1 = self.taxonomy.tag_set.get(external_id="tag_1").pk
         old_pk_3 = self.taxonomy.tag_set.get(external_id="tag_3").pk
 
-        importFile = BytesIO(json.dumps({"tags": [
+        renames = [
             {"id": "tag_3", "value": "Tag 1", "previous_id": "tag_1"},
             {"id": "tag_1", "value": "Tag 3", "previous_id": "tag_3"},
+        ]
+        children = [
             {"id": "tag_2", "value": "Tag 2", "parent_id": "tag_3"},
             {"id": "tag_4", "value": "Tag 4", "parent_id": "tag_1"},
-        ]}).encode())
+        ]
+        rows = children + renames if children_first else renames + children
+        importFile = BytesIO(json.dumps({"tags": rows}).encode())
         result, task, _plan = import_export_api.import_tags(
             self.taxonomy,
             importFile,
diff --git a/tests/openedx_tagging/import_export/test_import_plan.py b/tests/openedx_tagging/import_export/test_import_plan.py
index 75d9d7e74..8d8e1be34 100644
--- a/tests/openedx_tagging/import_export/test_import_plan.py
+++ b/tests/openedx_tagging/import_export/test_import_plan.py
@@ -7,6 +7,7 @@
 from openedx_tagging.import_export.actions import CreateTag
 from openedx_tagging.import_export.exceptions import TagImportError
 from openedx_tagging.import_export.import_plan import TagImportPlan, TagItem
+from openedx_tagging.models import Tag, Taxonomy
 
 from .test_actions import TestImportActionMixin
 
@@ -181,6 +182,10 @@ def test_build_delete_actions(self) -> None:
             True,
             0,
             [
+                {
+                    'name': 'update_parent',
+                    'id': 'tag_4',
+                },
                 {
                     'name': 'delete',
                     'id': 'tag_1',
@@ -189,10 +194,6 @@ def test_build_delete_actions(self) -> None:
                     'name': 'delete',
                     'id': 'tag_2',
                 },
-                {
-                    'name': 'update_parent',
-                    'id': 'tag_4',
-                },
                 {
                     'name': 'delete',
                     'id': 'tag_3',
@@ -265,7 +266,7 @@ def test_generate_actions(self, tags, replace, expected_errors, expected_actions
             "Conflict with 'create' (#2) and action #1: Duplicated external_id tag.\n"
             "Action error in 'rename' (#3): Duplicated tag value with tag in database (external_id=tag_2).\n"
             "Action error in 'update_parent' (#4): Unknown parent tag (tag_100). "
-            "You need to add parent before the child in your file.\n"
+            "Add a row for the parent to your file.\n"
             "Conflict with 'create' (#5) and action #2: Duplicated tag value.\n"
             "Conflict with 'rename' (#7) and action #1: Duplicated tag value.\n"
         ),
@@ -319,10 +320,10 @@ def test_generate_actions(self, tags, replace, expected_errors, expected_actions
             True,
             "Import plan for Import Taxonomy Test\n"
             "--------------------------------\n"
-            "#1: Delete tag <TagItem> (tag_1 / Tag 1)\n"
-            "#2: Delete tag <TagItem> (tag_2 / Tag 2)\n"
-            "#3: Update the parent of <Tag> (tag_4 / Tag 4) from parent "
+            "#1: Update the parent of <Tag> (tag_4 / Tag 4) from parent "
             "<Tag> (tag_3 / Tag 3) to None\n"
+            "#2: Delete tag <TagItem> (tag_1 / Tag 1)\n"
+            "#3: Delete tag <TagItem> (tag_2 / Tag 2)\n"
             "#4: Delete tag <TagItem> (tag_3 / Tag 3)\n"
             "#5: No changes needed for <TagItem> (tag_4 / Tag 4)\n"
         ),
@@ -681,6 +682,261 @@ def test_generate_actions_case_only_parent_rename_keeps_child(self, child_parent
         self.import_plan.generate_actions(tags=tags, replace=False)
         self.assertEqual(self.import_plan.errors, [])
 
+    @ddt.data(
+        # A new child listed before its new parent.
+        (
+            [
+                TagItem(id='new_child', value='New child', parent_id='new_parent'),
+                TagItem(id='new_parent', value='New parent'),
+            ],
+            ['new_parent', 'new_child'],
+        ),
+        # A row naming a parent's new id, spelled in another case, listed before the rename.
+        (
+            [
+                TagItem(id='tag_60', value='Tag 60', parent_id='TAG_50'),
+                TagItem(id='tag_50', value='Tag 1', previous_id='tag_1'),
+            ],
+            ['tag_50', 'tag_60'],
+        ),
+        # Parents already before their children: the plan keeps the file order.
+        (
+            [
+                TagItem(id='new_parent', value='New parent'),
+                TagItem(id='tag_4', value='Tag 4', parent_id='tag_1'),
+                TagItem(id='new_child', value='New child', parent_id='new_parent'),
+            ],
+            ['new_parent', 'tag_4', 'new_child'],
+        ),
+    )
+    @ddt.unpack
+    def test_generate_actions_orders_each_row_after_the_row_defining_its_parent(
+        self, tags: list[TagItem], expected_order: list[str],
+    ) -> None:
+        """
+        A row is built after the row that creates or renames-in its parent, so a
+        file may list rows in any order. Rows are only pulled forward when needed.
+        """
+        self.import_plan.generate_actions(tags=tags, replace=False)
+        self.assertEqual(self.import_plan.errors, [])
+        self.assertEqual([action.tag.id for action in self.import_plan.actions], expected_order)
+
+    def _plan_for_tree(self, tree: dict[str, str | None]) -> TagImportPlan:
+        """
+        Create a taxonomy holding ``tree`` (id -> parent id, parents listed first) and
+        return a plan for it. The tag values are the ids.
+        """
+        taxonomy = Taxonomy.objects.create(name='Tree taxonomy')
+        tags: dict[str, Tag] = {}
+        for tag_id, parent_id in tree.items():
+            tags[tag_id] = Tag.objects.create(
+                taxonomy=taxonomy, value=tag_id, external_id=tag_id,
+                parent=tags[parent_id] if parent_id else None,
+            )
+        return TagImportPlan(taxonomy)
+
+    @staticmethod
+    def _tree(taxonomy: Taxonomy) -> dict[str, str | None]:
+        """
+        The taxonomy's tags as id -> parent id.
+        """
+        return {tag.external_id: tag.parent.external_id if tag.parent else None for tag in taxonomy.tag_set.all()}
+
+    SIX_LEVELS: dict[str, str | None] = {
+        'level_1': None, 'level_2': 'level_1', 'level_3': 'level_2',
+        'level_4': 'level_3', 'level_5': 'level_4', 'level_6': 'level_5',
+    }
+
+    @ddt.data(
+        # A parent and its child swap places.
+        (
+            {'parent': None, 'child': 'parent'},
+            [TagItem(id='parent', value='parent', parent_id='child'), TagItem(id='child', value='child')],
+            {'child': None, 'parent': 'child'},
+        ),
+        # The ancestry is inverted through a new tag: new_root must leave old_root's
+        # subtree before old_root moves under new_middle.
+        (
+            {'old_root': None, 'sibling': 'old_root', 'new_root': 'old_root'},
+            [
+                TagItem(id='old_root', value='old_root', parent_id='new_middle'),
+                TagItem(id='sibling', value='sibling', parent_id='new_middle'),
+                TagItem(id='new_middle', value='new_middle', parent_id='new_root'),
+                TagItem(id='new_root', value='new_root'),
+            ],
+            {'new_root': None, 'new_middle': 'new_root', 'old_root': 'new_middle', 'sibling': 'new_middle'},
+        ),
+        # A deep tag moves to the root while new tags are created under it: created
+        # first, the new grandchild would be at a seventh level.
+        (
+            {'level_1': None, 'level_2': 'level_1', 'level_3': 'level_2', 'level_4': 'level_3', 'deep': 'level_4'},
+            [
+                TagItem(id='new_grandchild', value='new_grandchild', parent_id='new_child'),
+                TagItem(id='new_child', value='new_child', parent_id='deep'),
+                TagItem(id='deep', value='deep'),
+            ],
+            {
+                'level_1': None, 'level_2': 'level_1', 'level_3': 'level_2', 'level_4': 'level_3',
+                'deep': None, 'new_child': 'deep', 'new_grandchild': 'new_child',
+            },
+        ),
+        # A subtree moves under a tag that itself moves up: attached first, the
+        # grandchild would be at a seventh level.
+        (
+            {
+                'chain_1': None, 'chain_2': 'chain_1', 'chain_3': 'chain_2', 'moved_up': 'chain_3',
+                'subtree': None, 'subtree_child': 'subtree', 'subtree_grandchild': 'subtree_child',
+            },
+            [
+                TagItem(id='subtree', value='subtree', parent_id='moved_up'),
+                TagItem(id='moved_up', value='moved_up'),
+            ],
+            {
+                'chain_1': None, 'chain_2': 'chain_1', 'chain_3': 'chain_2', 'moved_up': None,
+                'subtree': 'moved_up', 'subtree_child': 'subtree', 'subtree_grandchild': 'subtree_child',
+            },
+        ),
+    )
+    @ddt.unpack
+    def test_execute_parent_moves_whose_steps_depend_on_each_other(
+        self, tree: dict[str, str | None], rows: list[TagItem], expected: dict[str, str | None],
+    ) -> None:
+        """
+        Restructures that fail when their actions run in file order, because an
+        intermediate step would build a cycle or a tree too deep, succeed: every
+        moved tag is detached before the actions run. ``rows`` lists the worst
+        order for each case. Omitted tags keep their parent (replace=False).
+        """
+        plan = self._plan_for_tree(tree)
+        plan.generate_actions(tags=rows, replace=False)
+        self.assertEqual([str(error) for error in plan.errors], [])
+        plan.execute()
+        self.assertEqual(self._tree(plan.taxonomy), expected)
+
+    def test_execute_kept_tag_under_deleted_ancestors_survives(self) -> None:
+        """
+        With replace=True, a kept tag whose parent and grandparent are deleted is
+        promoted to the root before the first delete, so the cascade does not
+        remove it, and it keeps its database row.
+        """
+        plan = self._plan_for_tree({'deleted_root': None, 'deleted_child': 'deleted_root', 'kept': 'deleted_child'})
+        kept_pk = plan.taxonomy.tag_set.get(external_id='kept').pk
+        plan.generate_actions(tags=[TagItem(id='kept', value='kept')], replace=True)
+        self.assertEqual(plan.errors, [])
+        names = [action.name for action in plan.actions]
+        self.assertLess(names.index('update_parent'), names.index('delete'))
+        plan.execute()
+        self.assertEqual(self._tree(plan.taxonomy), {'kept': None})
+        self.assertEqual(plan.taxonomy.tag_set.get(external_id='kept').pk, kept_pk)
+
+    def test_execute_child_of_deleted_parent_follows_rename_onto_its_id(self) -> None:
+        """
+        With replace=True the file omits tag_1, so it is deleted, and renames tag_3
+        onto the id tag_1. The tag_2 row names tag_1, so tag_2 ends up under the
+        renamed tag_3 rather than promoted to the root.
+        """
+        tag_3_pk = self.taxonomy.tag_set.get(external_id='tag_3').pk
+        tags = [
+            TagItem(id='tag_2', value='Tag 2', parent_id='tag_1'),
+            TagItem(id='tag_1', value='Tag 3', previous_id='tag_3'),
+        ]
+        self.import_plan.generate_actions(tags=tags, replace=True)
+        self.assertEqual(self.import_plan.errors, [])
+        self.import_plan.execute()
+        self.assertEqual(self.taxonomy.tag_set.get(external_id='tag_2').parent_id, tag_3_pk)
+        self.assertEqual(self.taxonomy.tag_set.get(pk=tag_3_pk).external_id, 'tag_1')
+
+    @ddt.data(
+        # tag_1 under its own child tag_2.
+        ([TagItem(id='tag_1', value='Tag 1', parent_id='tag_2')], False, "Row 1's parent_id (tag_2)"),
+        # A tag under itself.
+        ([TagItem(id='tag_2', value='Tag 2', parent_id='tag_2')], False, "Row 1's parent_id (tag_2)"),
+        # tag_3 takes the id of the deleted tag_1 and names that id as its parent.
+        (
+            [TagItem(id='tag_1', value='Tag 3', previous_id='tag_3', parent_id='tag_1')],
+            True,
+            "Row 1's parent_id (tag_1)",
+        ),
+        # Two renamed tags, on ids the import frees, name each other.
+        (
+            [
+                TagItem(id='tag_4', value='Tag 4', parent_id='tag_2'),
+                TagItem(id='tag_2', value='Tag 1', previous_id='tag_1', parent_id='tag_3'),
+                TagItem(id='tag_3', value='Tag 2', previous_id='tag_2', parent_id='tag_2'),
+            ],
+            True,
+            "Row 2's parent_id (tag_3)",
+        ),
+    )
+    @ddt.unpack
+    def test_generate_actions_parent_cycle_rejected_once(
+        self, tags: list[TagItem], replace: bool, expected_prefix: str,
+    ) -> None:
+        """
+        A parent_id that makes a tag its own ancestor after the import is rejected
+        with one error, on the cycle's first row in the file.
+        """
+        self.import_plan.generate_actions(tags=tags, replace=replace)
+        messages = [str(error) for error in self.import_plan.errors]
+        self.assertEqual(len(messages), 1, messages)
+        self.assertEqual(messages[0], f"{expected_prefix} creates a parent cycle.")
+
+    def test_generate_actions_long_reversed_chain_terminates(self) -> None:
+        """
+        A 1100-row chain of new tags listed child first is ordered and checked
+        without recursion, so it neither hits the recursion limit nor crashes:
+        every row is planned, and the chain is rejected as too deep.
+        """
+        tags = [
+            TagItem(id=f'n{i}', value=f'N{i}', parent_id=f'n{i - 1}' if i else None)
+            for i in reversed(range(1100))
+        ]
+        self.import_plan.generate_actions(tags=tags, replace=False)
+        self.assertEqual(len(self.import_plan.actions), 1100)
+        self.assertEqual(self.import_plan.actions[0].tag.id, 'n0')
+        self.assertEqual(
+            [str(error) for error in self.import_plan.errors],
+            ["This import would leave tag n6 more than 6 levels deep."],
+        )
+
+    def test_generate_actions_cycle_of_new_tags_reports_an_unknown_parent_once(self) -> None:
+        """
+        New tags that name each other as parent get the existing "Unknown parent"
+        error for one of them, and no cycle error on top.
+        """
+        tags = [
+            TagItem(id='new_a', value='New A', parent_id='new_b'),
+            TagItem(id='new_b', value='New B', parent_id='new_a'),
+        ]
+        self.import_plan.generate_actions(tags=tags, replace=False)
+        messages = [str(error) for error in self.import_plan.errors]
+        self.assertEqual(len(messages), 1, messages)
+        self.assertIn("Unknown parent tag", messages[0])
+
+    FOUR_LEVELS: dict[str, str | None] = {
+        'other_1': None, 'other_2': 'other_1', 'other_3': 'other_2', 'other_4': 'other_3',
+    }
+
+    @ddt.data(
+        # A new tag under the sixth level.
+        (TagItem(id='level_7', value='level_7', parent_id='level_6'), 'level_7'),
+        # A four-level chain moved under the fourth level: other_3 and other_4 end up
+        # at the seventh and eighth level, although neither has a row.
+        (TagItem(id='other_1', value='other_1', parent_id='level_4'), 'other_3'),
+    )
+    @ddt.unpack
+    def test_generate_actions_import_too_deep_rejected(self, row: TagItem, named_tag: str) -> None:
+        """
+        An import whose result would leave a tag deeper than the taxonomy allows is
+        rejected at the plan step, with one error naming a tag that would be too deep.
+        """
+        plan = self._plan_for_tree({**self.SIX_LEVELS, **self.FOUR_LEVELS})
+        plan.generate_actions(tags=[row], replace=False)
+        self.assertEqual(
+            [str(error) for error in plan.errors],
+            [f"This import would leave tag {named_tag} more than 6 levels deep."],
+        )
+
     def test_generate_actions_genuine_collision_not_staged(self) -> None:
         """
         A rename targeting an id held by an unrelated tag that is not

From 8a7b691b9e158bc3c7a96c3eebec044108005860 Mon Sep 17 00:00:00 2001
From: David Ormsbee <dave@axim.org>
Date: Wed, 14 Oct 2026 19:20:07 -0400
Subject: [PATCH 3/3] refactor: list the detach step in the plan as a DetachTag
 action

The detach of every tag whose parent changes used to run as a hidden phase at
the start of execute(). It is now a synthesized DetachTag action per tag,
like StageTagExternalIdForSwap, built after staging and before the row
actions, so the plan lists everything that runs. A root tag, a kept child
of a deleted tag (already promoted) and a row with an unknown parent get no
detach action. The tag is looked up by pk, since staging may have moved it
onto a placeholder id.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---
 src/openedx_tagging/import_export/actions.py  | 51 ++++++++++++++
 .../import_export/import_plan.py              | 69 +++++++++----------
 .../import_export/test_actions.py             | 42 +++++++++++
 .../import_export/test_import_plan.py         | 60 ++++++++++------
 4 files changed, 165 insertions(+), 57 deletions(-)

diff --git a/src/openedx_tagging/import_export/actions.py b/src/openedx_tagging/import_export/actions.py
index 3cc228d68..68f76ccdb 100644
--- a/src/openedx_tagging/import_export/actions.py
+++ b/src/openedx_tagging/import_export/actions.py
@@ -660,6 +660,56 @@ def execute(self) -> None:
         self.taxonomy.tag_set.filter(pk=self.target_pk).update(external_id=placeholder)
 
 
+class DetachTag(ImportAction):
+    """
+    Action to move a tag to the root before the actions that change parents run.
+
+    Synthesized by TagImportPlan._build_detach_actions for every existing tag
+    whose parent changes, not read from a file row. Tag.save checks the depth
+    of each step, and a cycle corrupts the tree silently, so a restructure that
+    is valid as a whole can still go wrong part-way through, depending on the
+    order of the actions. Once every moved tag is at the root, each tag is at
+    or above its final depth and none is an ancestor of its new parent, so the
+    actions that follow can attach tags in any order.
+    """
+
+    name = "detach"
+
+    def __str__(self) -> str:
+        return str(
+            _(
+                "Move tag {tag} to the root first, so that the parent changes "
+                "in this file can run in any order."
+            ).format(tag=self.tag)
+        )
+
+    @classmethod
+    def applies_for(cls, taxonomy: Taxonomy, tag: TagItem, indexed_actions=None) -> bool:
+        """
+        This action is an exception: synthesized in TagImportPlan.generate_actions.
+        """
+        return False
+
+    def validate(self, indexed_actions) -> list[ImportActionError]:
+        """
+        No validations necessary
+        """
+        return []
+
+    def execute(self) -> None:
+        """
+        Moves the tag to the root, with its descendants.
+
+        Looks the tag up by pk: a StageTagExternalIdForSwap action may have
+        moved it onto a placeholder id by now.
+        """
+        # _build_detach_actions only builds this action with a target_pk.
+        assert self.target_pk is not None
+        tag = self.taxonomy.tag_set.get(pk=self.target_pk)
+        tag.parent = None
+        tag.save()
+
+
 class DeleteTag(ImportAction):
     """
     Action for delete a Tag
@@ -737,6 +787,7 @@ def execute(self) -> None:
     RenameTagExternalId,
     CreateTag,
     StageTagExternalIdForSwap,
+    DetachTag,
     DeleteTag,
     WithoutChanges,
 ]
diff --git a/src/openedx_tagging/import_export/import_plan.py b/src/openedx_tagging/import_export/import_plan.py
index f23ea9a92..2b01ab959 100644
--- a/src/openedx_tagging/import_export/import_plan.py
+++ b/src/openedx_tagging/import_export/import_plan.py
@@ -12,6 +12,7 @@
 from ..models.base import TAXONOMY_MAX_DEPTH
 from .actions import (
     DeleteTag,
+    DetachTag,
     ImportAction,
     RenameTagExternalId,
     StageTagExternalIdForSwap,
@@ -34,6 +35,7 @@ class ExistingTag(NamedTuple):
     """
 
     external_id: str
+    value: str
     pk: int
     parent_pk: int | None
 
@@ -70,15 +72,15 @@ class TagImportPlan:
     A row's ``parent_id`` names the id its parent holds *after* the import. Three steps
     make parent changes work whatever the order of the rows in the file:
 
-    - The plan computes the tag tree as it will be after the import (``_end_state``),
-      rejects a parent cycle or a tree deeper than the limit there, and records which
-      existing tags change parent (``moved_tag_pks``).
+    - The plan computes the tag tree as it will be after the import (``_end_state``)
+      and rejects a parent cycle or a tree deeper than the limit there.
+    - A DetachTag action moves every existing tag whose parent changes to the root
+      before the row actions (``_build_detach_actions``). After that each tag is at or
+      above its final depth, and none is an ancestor of its new parent, so no action can
+      exceed the depth limit or build a cycle part-way through.
     - A row is built after the row that creates or renames-in its parent
       (``_order_rows_parent_first``), so the per-action parent validation and the
       execution both find the parent.
-    - ``execute`` first detaches every tag whose parent changes. After that each tag is
-      at or above its final depth, and none is an ancestor of its new parent, so no
-      action can exceed the depth limit or build a cycle part-way through.
 
     "Row N" in an error is the N-th data row of the file, counting from 1: in a CSV file
     that is spreadsheet line N + 1, because the header is line 1.
@@ -89,14 +91,12 @@ class TagImportPlan:
     indexed_actions: dict
     actions_dict: dict
     taxonomy: Taxonomy
-    moved_tag_pks: set[int]
 
     def __init__(self, taxonomy: Taxonomy):
         self.actions = []
         self.errors = []
         self.taxonomy = taxonomy
         self.actions_dict = {}
-        self.moved_tag_pks = set()
         self._init_indexed_actions()
 
     def _init_indexed_actions(self):
@@ -280,8 +280,10 @@ def _existing_tags(self) -> dict[str, ExistingTag]:
         Return the taxonomy's tags before the import, keyed by casefolded external_id. One query.
         """
         return {
-            external_id.casefold(): ExistingTag(external_id, pk, parent_pk)
-            for external_id, pk, parent_pk in self.taxonomy.tag_set.values_list("external_id", "pk", "parent_id")
+            external_id.casefold(): ExistingTag(external_id, value, pk, parent_pk)
+            for external_id, value, pk, parent_pk in self.taxonomy.tag_set.values_list(
+                "external_id", "value", "pk", "parent_id",
+            )
         }
 
     def _end_state(
@@ -371,23 +373,31 @@ def _validate_hierarchy(
             tag_id = tags[row_of[key]].id if key in row_of else existing[key].external_id
             self.errors.append(TagTooDeepError(tag_id, TAXONOMY_MAX_DEPTH + 1))
 
-    @staticmethod
-    def _moved_tag_pks(
-        existing: dict[str, ExistingTag], parent_of: dict[str, str | None], pk_of: dict[str, int],
-    ) -> set[int]:
+    def _build_detach_actions(
+        self, existing: dict[str, ExistingTag], parent_of: dict[str, str | None], pk_of: dict[str, int],
+    ) -> None:
         """
-        Return the pks of the existing tags whose parent after the import is a different tag than before.
+        Add a DetachTag action for every existing tag whose parent after the import
+        is a different tag than before.
+
+        A root tag has nothing to detach, a kept child of a deleted tag is promoted
+        by the delete phase, and a row whose parent_id names no tag of the end state
+        is left to ``_validate_parent``.
         """
-        current_parent_pk = {info.pk: info.parent_pk for info in existing.values()}
-        moved = set()
+        by_pk = {info.pk: info for info in existing.values()}
+        deleted_pks = {existing[action.tag.id.casefold()].pk for action in self.indexed_actions["delete"]}
         for key, pk in pk_of.items():
             if pk == NEW_TAG:
                 continue
+            info = by_pk[pk]
+            if info.parent_pk is None or info.parent_pk in deleted_pks:
+                continue
             parent_key = parent_of[key]
-            final_parent_pk = pk_of.get(parent_key) if parent_key is not None else None
-            if final_parent_pk != current_parent_pk[pk]:
-                moved.add(pk)
-        return moved
+            if parent_key is not None and parent_key not in pk_of:
+                continue
+            final_parent_pk = pk_of[parent_key] if parent_key is not None else None
+            if final_parent_pk != info.parent_pk:
+                self._build_action(DetachTag, TagItem(id=info.external_id, value=info.value), target_pk=pk)
 
     @staticmethod
     def _order_rows_parent_first(tags: list[TagItem], existing: dict[str, ExistingTag]) -> list[TagItem]:
@@ -440,7 +450,6 @@ def generate_actions(
         """
         self.actions.clear()
         self.errors.clear()
-        self.moved_tag_pks.clear()
         self._init_indexed_actions()
         existing = self._existing_tags()
 
@@ -488,10 +497,11 @@ def generate_actions(
         self._build_staging_actions(tags)
 
         # Reject what no single action can see: a cycle or an overflow of the tree the
-        # rows describe together. Record the tags to detach before executing.
+        # rows describe together. Then detach every tag whose parent changes, so that
+        # the row actions can run in any order.
         parent_of, pk_of, row_of = self._end_state(tags, existing)
         self._validate_hierarchy(tags, existing, parent_of, pk_of, row_of)
-        self.moved_tag_pks = self._moved_tag_pks(existing, parent_of, pk_of)
+        self._build_detach_actions(existing, parent_of, pk_of)
 
         for tag in self._order_rows_parent_first(tags, existing):
             has_action = False
@@ -537,17 +547,6 @@ def execute(self, task: TagImportTask | None = None):
         """
         if self.errors:
             return
-        # Detach every tag whose parent changes before any action runs, so that no
-        # action can exceed the depth limit or build a cycle part-way through (see
-        # the class docstring). Each action then attaches its tag to its new parent.
-        if self.moved_tag_pks:
-            if task:
-                task.add_log("Detaching the tags whose parent changes [Started]", save=False)
-            for tag in self.taxonomy.tag_set.filter(pk__in=self.moved_tag_pks, parent__isnull=False):
-                tag.parent = None
-                tag.save()
-            if task:
-                task.add_log("Success", save=False)
         for action in self.actions:
             # Avoid to save each log because it is slow and costs a lot in memory
             # It is necessary to save at the end.
diff --git a/tests/openedx_tagging/import_export/test_actions.py b/tests/openedx_tagging/import_export/test_actions.py
index 511ebae67..fad8727de 100644
--- a/tests/openedx_tagging/import_export/test_actions.py
+++ b/tests/openedx_tagging/import_export/test_actions.py
@@ -10,6 +10,7 @@
 from openedx_tagging.import_export.actions import (
     CreateTag,
     DeleteTag,
+    DetachTag,
     ImportAction,
     RenameTag,
     RenameTagExternalId,
@@ -1006,6 +1007,47 @@ def test_execute(self) -> None:
         self.assertTrue(tag.external_id.startswith("oel-import-staging:"))
 
 
+class TestDetachTag(TestImportActionMixin, TestCase):
+    """
+    Test for 'detach' action
+    """
+
+    def _action(self) -> DetachTag:
+        return DetachTag(
+            taxonomy=self.taxonomy,
+            tag=TagItem(id='tag_2', value='Tag 2', index=100),
+            index=100,
+            target_pk=self.taxonomy.tag_set.get(external_id='tag_2').pk,
+        )
+
+    def test_str(self) -> None:
+        self.assertEqual(
+            str(self._action()),
+            "Move tag <TagItem> (tag_2 / Tag 2) to the root first, "
+            "so that the parent changes in this file can run in any order."
+        )
+
+    def test_applies_for(self) -> None:
+        self.assertFalse(DetachTag.applies_for(self.taxonomy, tag=TagItem(id='tag_2', value='Tag 2', index=100)))
+
+    def test_validate(self) -> None:
+        self.assertEqual(self._action().validate(self.indexed_actions), [])
+
+    def test_execute(self) -> None:
+        """
+        The tag moves to the root and keeps its pk; it is found by pk, so a
+        placeholder external_id from a staging action does not matter.
+        """
+        action = self._action()
+        tag = self.taxonomy.tag_set.get(external_id='tag_2')
+        self.assertIsNotNone(tag.parent)
+        self.taxonomy.tag_set.filter(pk=tag.pk).update(external_id="oel-import-staging:x")
+        action.execute()
+        tag.refresh_from_db()
+        self.assertIsNone(tag.parent)
+        self.assertEqual(tag.depth, 0)
+
+
 class TestDeleteTag(TestImportActionMixin, TestCase):
     """
     Test for 'delete' action
diff --git a/tests/openedx_tagging/import_export/test_import_plan.py b/tests/openedx_tagging/import_export/test_import_plan.py
index 8d8e1be34..32f5fef38 100644
--- a/tests/openedx_tagging/import_export/test_import_plan.py
+++ b/tests/openedx_tagging/import_export/test_import_plan.py
@@ -102,6 +102,10 @@ def test_build_delete_actions(self) -> None:
             False,
             0,
             [
+                {
+                    'name': 'detach',
+                    'id': 'tag_4'
+                },
                 {
                     'name': 'create',
                     'id': 'tag_31'
@@ -250,25 +254,27 @@ def test_generate_actions(self, tags, replace, expected_errors, expected_actions
             False,
             "Import plan for Import Taxonomy Test\n"
             "--------------------------------\n"
-            "#1: Create a new tag with values (external_id=tag_31, value=Tag 31, parent_id=None).\n"
-            "#2: Create a new tag with values (external_id=tag_31, value=Tag 32, parent_id=None).\n"
-            "#3: Rename tag value of <Tag> (tag_1 / Tag 1) to 'Tag 2'\n"
-            "#4: Update the parent of <Tag> (tag_4 / Tag 4) from parent "
+            "#1: Move tag <TagItem> (tag_2 / Tag 2) to the root first, so that the parent changes "
+            "in this file can run in any order.\n"
+            "#2: Create a new tag with values (external_id=tag_31, value=Tag 31, parent_id=None).\n"
+            "#3: Create a new tag with values (external_id=tag_31, value=Tag 32, parent_id=None).\n"
+            "#4: Rename tag value of <Tag> (tag_1 / Tag 1) to 'Tag 2'\n"
+            "#5: Update the parent of <Tag> (tag_4 / Tag 4) from parent "
             "<Tag> (tag_3 / Tag 3) to tag_100\n"
-            "#5: Create a new tag with values (external_id=tag_33, value=Tag 32, parent_id=None).\n"
-            "#6: Update the parent of <Tag> (tag_2 / Tag 2) from parent "
+            "#6: Create a new tag with values (external_id=tag_33, value=Tag 32, parent_id=None).\n"
+            "#7: Update the parent of <Tag> (tag_2 / Tag 2) from parent "
             "<Tag> (tag_1 / Tag 1) to None\n"
-            "#7: Rename tag value of <Tag> (tag_2 / Tag 2) to 'Tag 31'\n"
+            "#8: Rename tag value of <Tag> (tag_2 / Tag 2) to 'Tag 31'\n"
             "\nOutput errors\n"
             "--------------------------------\n"
             "Duplicate id (tag_31): file rows 1, 2 all claim it as their final id. "
             "Each row's id must be unique within a single import.\n"
-            "Conflict with 'create' (#2) and action #1: Duplicated external_id tag.\n"
-            "Action error in 'rename' (#3): Duplicated tag value with tag in database (external_id=tag_2).\n"
-            "Action error in 'update_parent' (#4): Unknown parent tag (tag_100). "
+            "Conflict with 'create' (#3) and action #2: Duplicated external_id tag.\n"
+            "Action error in 'rename' (#4): Duplicated tag value with tag in database (external_id=tag_2).\n"
+            "Action error in 'update_parent' (#5): Unknown parent tag (tag_100). "
             "Add a row for the parent to your file.\n"
-            "Conflict with 'create' (#5) and action #2: Duplicated tag value.\n"
-            "Conflict with 'rename' (#7) and action #1: Duplicated tag value.\n"
+            "Conflict with 'create' (#6) and action #3: Duplicated tag value.\n"
+            "Conflict with 'rename' (#8) and action #2: Duplicated tag value.\n"
         ),
         # Testing valid plan
         (
@@ -300,13 +306,15 @@ def test_generate_actions(self, tags, replace, expected_errors, expected_actions
             False,
             "Import plan for Import Taxonomy Test\n"
             "--------------------------------\n"
-            "#1: Create a new tag with values (external_id=tag_31, value=Tag 31, parent_id=None).\n"
-            "#2: Create a new tag with values (external_id=tag_32, value=Tag 32, parent_id=tag_1).\n"
-            "#3: Rename tag value of <Tag> (tag_2 / Tag 2) to 'Tag 2 v2'\n"
-            "#4: Update the parent of <Tag> (tag_4 / Tag 4) from parent "
+            "#1: Move tag <TagItem> (tag_4 / Tag 4) to the root first, so that the parent changes "
+            "in this file can run in any order.\n"
+            "#2: Create a new tag with values (external_id=tag_31, value=Tag 31, parent_id=None).\n"
+            "#3: Create a new tag with values (external_id=tag_32, value=Tag 32, parent_id=tag_1).\n"
+            "#4: Rename tag value of <Tag> (tag_2 / Tag 2) to 'Tag 2 v2'\n"
+            "#5: Update the parent of <Tag> (tag_4 / Tag 4) from parent "
             "<Tag> (tag_3 / Tag 3) to tag_1\n"
-            "#5: Rename tag value of <Tag> (tag_4 / Tag 4) to 'Tag 4 v2'\n"
-            "#6: No changes needed for <TagItem> (tag_1 / Tag 1)\n"
+            "#6: Rename tag value of <Tag> (tag_4 / Tag 4) to 'Tag 4 v2'\n"
+            "#7: No changes needed for <TagItem> (tag_1 / Tag 1)\n"
         ),
         # Testing deletes (replace=True)
         (
@@ -703,7 +711,7 @@ def test_generate_actions_case_only_parent_rename_keeps_child(self, child_parent
         (
             [
                 TagItem(id='new_parent', value='New parent'),
-                TagItem(id='tag_4', value='Tag 4', parent_id='tag_1'),
+                TagItem(id='tag_4', value='Tag 4 v2', parent_id='tag_3'),
                 TagItem(id='new_child', value='New child', parent_id='new_parent'),
             ],
             ['new_parent', 'tag_4', 'new_child'],
@@ -803,13 +811,20 @@ def test_execute_parent_moves_whose_steps_depend_on_each_other(
     ) -> None:
         """
         Restructures that fail when their actions run in file order, because an
-        intermediate step would build a cycle or a tree too deep, succeed: every
-        moved tag is detached before the actions run. ``rows`` lists the worst
-        order for each case. Omitted tags keep their parent (replace=False).
+        intermediate step would build a cycle or a tree too deep, succeed: a
+        detach action moves every tag whose parent changes to the root before the
+        row actions. ``rows`` lists the worst order for each case. Omitted tags
+        keep their parent (replace=False).
         """
         plan = self._plan_for_tree(tree)
         plan.generate_actions(tags=rows, replace=False)
         self.assertEqual([str(error) for error in plan.errors], [])
+        names = [action.name for action in plan.actions]
+        self.assertIn('detach', names)
+        self.assertLess(
+            max(index for index, name in enumerate(names) if name == 'detach'),
+            min(index for index, name in enumerate(names) if name != 'detach'),
+        )
         plan.execute()
         self.assertEqual(self._tree(plan.taxonomy), expected)
 
@@ -825,6 +840,7 @@ def test_execute_kept_tag_under_deleted_ancestors_survives(self) -> None:
         self.assertEqual(plan.errors, [])
         names = [action.name for action in plan.actions]
         self.assertLess(names.index('update_parent'), names.index('delete'))
+        self.assertNotIn('detach', names)  # The promotion already moves the tag to the root.
         plan.execute()
         self.assertEqual(self._tree(plan.taxonomy), {'kept': None})
         self.assertEqual(plan.taxonomy.tag_set.get(external_id='kept').pk, kept_pk)