[ufedaseyeuconsultant]

Description

Closes #858. This PR takes the simpler implementation from @ormsbee's branch ormsbee/858-stale-parent-id-simpler and addresses the review on it. Its three commits are included unchanged; the commits on top address the three reviews. It replaces #867.

What the import does now

A taxonomy import with the previous_id column (from #673) can rename a tag's external_id, swap ids between tags, and move tags between parents in one file. This PR makes the parent handling correct in those cases:

• A row whose parent_id is its parent's pre-import id, when the same file gives that id away, is rejected at the Plan step. A plain rename makes that id name nothing after the import. A swap makes it ambiguous: the id now belongs to a different tag, so the row could mean "stay with my parent" or "move under the tag that now has this id". A child that names its parent's final id is accepted.
• Parent references no longer depend on row order. A row can name a parent that a later row creates or renames.
• Every existing tag whose parent changes is first moved to the root, then attached under its new parent, with parents attached before children. This avoids intermediate states that are cyclic or too deep. There is no in-memory simulation and no second plan build.
• With replace=True, kept children of a deleted parent are promoted to root before any delete runs, so a delete no longer cascades onto a tag the file keeps.
• Parent cycles and imports deeper than the depth limit are rejected at the Plan step.

Changes made after the reviews

• The stale-parent error names the row. Before, it showed only the action's number in the plan, which an admin cannot map back to the file once delete, staging and detach actions come first.
• When another row takes the id the child names, the error also says how to make the other move: in two separate imports.
• UpdateParentTag builds its description when the action is created. A caller that runs generate_actions() and then execute() now logs the old parent instead of "None".
• The casefolded set of renamed-to ids is built once per plan instead of once per row.
• Readability changes asked for in the review: named booleans in _validate_parent, _is_current_parent renamed to _was_parent_before_import, "pre-import id" and "final id" used consistently, comments on the final-tree walk, the detach builder and the row ordering, and clearer variable names.
• Tests: TestImportAcceptanceScenarios from fix: make tag import parent_id handling correct and order-independent #867 is added, adjusted to this implementation's messages, and reduced after the second and third reviews to the scenarios that no other test covers. Each scenario imports through the public API in JSON, in the file order, the reversed order and one mixed order, and checks the export and content associations. One CSV import covers the parser. Where a removed acceptance test added row orders or an assertion to a test that remains, that part moved into the remaining test.
• Docstrings and comments after the second review: the reason for detaching first is explained once, in the TagImportPlan docstring; the _validate_parent and _end_state docstrings are shorter; the set of renamed-to ids is a plain lookup, without a fallback that only tests used.
• After the third review: two more acceptance tests that repeated existing coverage are removed, four API and plan tests became ddt cases of the plan tests they repeated, the "Unknown previous_id" message is asserted in full, and a comment that restated an assert is gone.

Who is affected

• Platform Administrators importing taxonomies with previous_id: some files that were accepted and then misbehaved are now rejected with an error that names the row, and some files that were rejected only because of their row order now import.
• Developers extending the import actions: plans contain a detach step for each moved existing tag, and the order of actions in a plan can differ from the file order.

User-visible changes

• New or changed Plan errors:
  • Action error in 'update_parent' (#K): Row N: Parent tag X is renamed to Y in this file; use Y as the parent_id., and when another row takes the id X, also To move under the tag that takes the id X in this file instead, import the id changes and the move as two separate files. The action name is the one of the row's action (update_parent, create or rename_external_id). The (#K) prefix is the action's position in the plan, which every action error has; the row is the Row N part. Showing the row without that prefix is left to Tag import: parent errors name their row and report every cycle as a cycle #892.
  • Row N's parent_id (X) creates a parent cycle.
  • This import would leave tag X more than 6 levels deep.
  • Unknown parent tag (X). Add a row for the parent to your file. (it used to say "You need to add parent before the child in your file", which no longer applies).
• The plan lists Move tag X to the root first, so that the parent changes in this file can run in any order. for each existing tag whose parent changes, before the row actions.
• "Row N" is the 1-based position among the file's data rows, like the existing duplicate-id error. In a CSV file that is spreadsheet line N + 1, because the header is line 1. In a JSON file it is the N-th entry of tags.

Supporting information

• Issue: Tag import: parent_id isn't re-validated when it matches the pre-import parent #858. Its acceptance criteria are updated to this implementation: the two criteria about "the same plan as before" and "no extra move-to-root actions" now describe the outcome an admin relies on, and the quoted messages match this branch.
• Parent work: [BE] Support editing an existing tag's external_id (Competency ID) on taxonomy re-import #673, merged in feat: support editing an existing tag's external_id on taxonomy re-im… #804. Design: ADR 0010.
• Superseded PR: fix: make tag import parent_id handling correct and order-independent #867.
• Follow-ups: Tag import: parent errors name their row and report every cycle as a cycle #892 and Tag import: reject a parent_id that names a tag the full replace import deletes #893 (see "Other information").

Testing instructions

Automated:

1. pytest tests/openedx_tagging/import_export -o addopts="" (231 tests; -o addopts="" skips the coverage flags that tox.ini adds).
2. pytest tests/openedx_tagging -o addopts="" (617 tests).
3. The quality checks as CI runs them: lint-imports, pylint src tests test_utils manage.py test_settings.py mysql_test_settings.py, pycodestyle, isort --check-only, pydocstyle src and mypy --show-traceback.

Manual, through the taxonomy import endpoint or the Studio import wizard. Create a taxonomy with tag_1 (root), tag_2 (child of tag_1), tag_3 (root) and tag_4 (child of tag_3), with values Tag 1 to Tag 4. Import each file below. The import is a full replace, so each file lists the whole taxonomy.

File (JSON tags rows)	Expected
{id: tag_50, value: "Tag 1", previous_id: tag_1}, {id: tag_2, value: "Tag 2", parent_id: tag_1}, {id: tag_3, value: "Tag 3"}, {id: tag_4, value: "Tag 4", parent_id: tag_3}	Rejected at the Plan step with Action error in 'update_parent' (#2): Row 2: Parent tag tag_1 is renamed to tag_50 in this file; use tag_50 as the parent_id. Nothing changes.
{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}	Imports. Each child stays on the same physical parent, and the export shows tag_2 under tag_3 and tag_4 under tag_1.
The same rows as the second file, with the two child rows first	Same result as the second file.
The same swap, with the tag_2 row saying parent_id: tag_1	Rejected with Action error in 'update_parent' (#6): Row 3: Parent tag tag_1 is renamed to tag_3 in this file; use tag_3 as the parent_id. To move under the tag that takes the id tag_1 in this file instead, import the id changes and the move as two separate files. The plan's staging and detach steps come first, so the action number differs from the row number.

Other information

• No migrations, no model changes, no new dependencies, nothing in openedx-platform needs to change.
• Existing tests that changed in the review commits:
  • test_generate_actions_existing_child_keeps_stale_parent_id_rejected (test_import_plan.py) expects the "Row N" prefix and has a second case: a new child row, after the deletes of a full replace import, so that its action number differs from its row.
  • The swap test in test_import_plan.py keeps only the case where the child is renamed too, as test_generate_actions_swap_renamed_child_naming_own_parent_stale_id_rejected, and expects the "Row N" prefix and the hint. Its two other cases are covered by TestImportAcceptanceScenarios.
  • test_generate_actions_parent_id_stale_after_plain_rename_rejected (test_import_plan.py) also runs the parent id in another case and with the child row first.
  • test_import_swap_external_ids_with_children (test_api.py) and test_execute_child_of_deleted_parent_follows_rename_onto_its_id (test_import_plan.py) are removed, because TestImportAcceptanceScenarios covers the same scenarios in more row orders.
  • test_execute_parent_moves_whose_steps_depend_on_each_other (test_import_plan.py) runs each case in its listed order, reversed and one mixed order, checks that moved tags keep their pks, and has two more trees: a three-level chain reversed, and a tag moved under its former grandchild.
  • test_generate_actions_parent_cycle_rejected_once (test_import_plan.py) has one more case: the two renamed tags that name each other, in the other row order.
  • test_import_swap_external_ids and test_import_three_cycle_external_ids (test_api.py) run in more row orders.
  • The _validate_parent tests in test_actions.py that build actions outside a plan set the renamed-to ids next to the renamed tags, as the plan does.
• Beyond the committed tests, a randomized end-state test (not committed, about 5 minutes per large run) imported about 27,000 random valid and invalid files (renames, swaps, rotations, reclaimed ids, new tags, re-parenting, deletes by omission, deep chains, ancestry inversions, random capitalization of ids) in 4 row orders with replace True and False, and compared the result with an expected tree. It found no mismatch on @ormsbee's branch, and the same counts after each review commit.
• Locally I could only run SQLite. The CI job on MySQL is the check for database-dependent behavior.
• Known issues left for follow-up, filed as two issues:
  • Tag import: parent errors name their row and report every cycle as a cycle #892: the "Unknown parent tag" error names the row, and every parent cycle, including a cycle made only of new tags, gets the parent cycle error instead of a misleading "Unknown parent tag".
  • Tag import: reject a parent_id that names a tag the full replace import deletes #893: a row whose parent_id names a tag that the same replace=True import deletes is rejected at the Plan step. Today it either fails when the import runs, or, when a kept tag's row names its current parent, imports and leaves that tag at the root. This also happens on main.
• AI tools: Claude Code with Claude Opus 5.5 (claude-opus-5-5) wrote the review commits on top of @ormsbee's commits.

---

🤖 Generated with Claude Code

[openedx-webhooks]

Thanks for the pull request, @ufedaseyeuconsultant!

This repository is currently maintained by @axim-engineering.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[ufedaseyeuconsultant]

@mgwozdz-unicon Three follow-up problems in the taxonomy tag import, found while working on #858. None of them was introduced by the #858 fix, and none corrupts data: a failing import runs in one transaction and is rolled back. Each one can become its own issue.

1. The "Unknown parent tag" error does not name the row

Problem. When a row's parent_id names a tag that neither exists nor is created or renamed onto by the file, the Plan step reports:

Action error in 'update_parent' (#5): Unknown parent tag (tag_100). Add a row for the parent to your file.

#5 is the action's number in the plan, not the row in the file. Delete, staging and detach actions come before the row actions, and rows are reordered so that parents come before children, so an admin cannot map #5 back to a line of the file. The stale-parent error from the same validation now starts with Row N: (#858), so the two errors from the same check are inconsistent.

Expected. The error names the row, with the same "Row N" numbering as the other import errors (the 1-based position among the file's data rows; in a CSV file that is spreadsheet line N + 1), for example Row 2: Unknown parent tag (tag_100). Add a row for the parent to your file.

Proposed fix. In ImportAction._validate_parent, add the same Row N: prefix the stale-parent error uses when the action has a row. Two existing tests assert the full message text (test_import_plan.py and test_actions.py, "Unknown parent tag (tag_100)") and need the new prefix.

2. A new parent that the same replace=True import deletes passes the Plan step and fails when the import runs

Problem. A file can name, as a row's new parent, a tag that the same full-replace import deletes because the file has no row for it. The Plan step accepts the file, and the import then fails while it runs.

To reproduce. Take a taxonomy with tag_1, tag_2 under tag_1, and tag_3. Import with replace=True:

{"tags": [
  {"id": "tag_1", "value": "Tag 1"},
  {"id": "tag_2", "value": "Tag 2", "parent_id": "tag_3"}
]}

Actual. The plan is:

#1: Delete tag <TagItem> (tag_3 / Tag 3)
#2: No changes needed for <TagItem> (tag_1 / Tag 1)
#3: Update the parent of <Tag> (tag_2 / Tag 2) from parent <Tag> (tag_1 / Tag 1) to tag_3

It reports no error. When the import runs, #1 deletes tag_3, and #3 then fails with DoesNotExist('Tag matching query does not exist.'). import_tags returns False and the transaction is rolled back. The same happens on main before #858.

Why. _validate_parent accepts the row because tag_3 exists when the plan is built. It does not check whether the same plan deletes that tag.

Expected. The Plan step rejects the row with an error that names it, for example Row 2's parent_id (tag_3) names a tag this import deletes.

Careful with. A file may delete a tag and, in the same import, rename another tag onto the freed id. A child naming that id is valid and ends up under the renamed tag; this is covered by test_child_of_deleted_holder_moves_under_tag_renamed_onto_its_id. The new check must only reject a deleted id that no row creates or renames onto.

3. A cycle of a renamed tag and a new tag can produce a second, misleading error, depending on row order

Problem. A file in which a renamed tag and a new tag name each other as parent is always rejected, but the number of errors depends on the order of the rows.

To reproduce. Take a taxonomy with tag_1. Import with replace=True the two rows below, in both orders:

{"id": "tag_50", "value": "Tag 1", "previous_id": "tag_1", "parent_id": "new_x"}
{"id": "new_x", "value": "New", "parent_id": "tag_50"}

Actual.

• With the rename row first:

  Row 1's parent_id (new_x) creates a parent cycle.
  

• With the new tag's row first:

  Row 1's parent_id (tag_50) creates a parent cycle.
  Action error in 'rename_external_id' (#1): Unknown parent tag (new_x). Add a row for the parent to your file.
  

  The second error is misleading: the file does have a row for new_x, and the real problem is the cycle that the first error already reports.

Expected. Exactly one error, the parent cycle error on the cycle's first row, in any row order.

Proposed fix. When the plan-level cycle check has rejected a row, skip the per-action "Unknown parent tag" check for the rows on that cycle, or report the cycle and stop validating those rows' parents.

[mgwozdz-unicon]

Thanks for working through the review on #871. Every inline comment I left there is addressed here. I'm requesting changes for two reasons: #858 still describes the other implementation, and the tests are now much larger than they need to be. I had Claude do a first pass on the tests and comments before my own review. The duplicate pairs below were checked against the code, and all line numbers refer to 30df7b4.

1. #858's acceptance criteria still describe the other implementation

The PR description says the acceptance criteria in #858 were updated to match this implementation, but the issue's last edit is from 2026-10-14, before #871 was opened. Three criteria still don't match this branch:

• Additional fix 1, last scenario: "the Plan lists the actions in the same order as before this fix." A file that moves an existing tag now also gets a "Move tag X to the root first" action.
• Additional fix 5, last scenario: "the Plan has no extra actions that move tags to the root." This PR adds one for every existing tag whose parent changes, by design.
• Additional fix 4 quotes the error This import would leave tags deeper than 6 levels, or build an invalid parent hierarchy, so it cannot be applied. This branch says This import would leave tag X more than 6 levels deep.

I'm fine with this branch's behavior in all three cases. Could you update the issue, so that a reviewer checking the PR against #858 sees criteria it actually meets?

2. TestImportAcceptanceScenarios is too large

On #871 I suggested bringing this class over from #867. I should have been clearer: I meant the scenarios that nothing else covers, not the whole class. As ported, it is about 1,036 lines with 25 tests that together run about 285 full imports, and it is most of why this PR is twice the size of #871. Three changes would shrink it.

Tests that repeat another test on the same input. Each pair below builds the same tree from the same rows:

• test_ancestry_inversion_in_any_row_order (test_api.py:1677), test_parent_and_child_swap_places (1710), test_deep_tag_moved_to_root_with_new_children_in_any_row_order (1799) and test_subtree_moved_under_deep_tag_before_it_is_shallowed (1866) are the four cases of test_execute_parent_moves_whose_steps_depend_on_each_other (test_import_plan.py:764).
• test_import_that_leaves_tags_too_deep_is_rejected (test_api.py:1942) runs the same two scenarios as test_generate_actions_import_too_deep_rejected (test_import_plan.py:939).
• test_rename_onto_freed_id_forming_parent_cycle_rejected (test_api.py:1516) runs the last two cases of test_generate_actions_parent_cycle_rejected_once (test_import_plan.py:868).
• test_swap_keeps_tags_and_associations (test_api.py:1342) and test_three_way_rotation_in_any_row_order (1369) repeat the existing test_import_swap_external_ids (777) and test_import_three_cycle_external_ids (819), adding more row orders.
• test_import_stale_parent_id_error_names_row (test_api.py:1063) is a third test that the stale-parent error names its row, after test_import_plan.py:669 and the swap tests in the new class.

Where the API version adds something real, such as more row orders, I'd move that into the existing test, for example as more ddt cases at the plan level, rather than keep a second copy that goes through the API.

Format and row-order loops. Most tests run every scenario in both JSON and CSV, and many in all 6 or 24 row orders. The two parsers differ only in how they read and write the file. Both hand the plan the same list of TagItem rows, as the comments at test_api.py:1701, 1822, 1902 and 2017 already note. One format, with the file order, the reversed order and one mixed order, would cover the ordering logic, and one CSV import is enough to cover the parser.

Docstrings. The 25 test docstrings add up to about 258 lines. Most draw ASCII before-and-after trees of rows that the test body already lists, quote #673 or #858 scenario titles word for word, or point at other tests: "the test above" at lines 1835, 1859, 1935 and 2096, and full test names in the class docstring at lines 1117 to 1130. A reference to another test goes stale silently when that test is renamed or moved. One or two sentences on the behavior under test would be enough for each.

Claude's rough estimate is that these three changes would remove about half of the class without losing a distinct assertion.

3. Comments in the source

• The reason for detaching first is explained three times: in the TagImportPlan docstring (import_plan.py:68), the DetachTag docstring (actions.py:709) and the comment in generate_actions (import_plan.py:521). I'd keep it in the TagImportPlan docstring and cut the other two to a line each.
• The _end_state docstring (import_plan.py:297) and the comment in its second loop (import_plan.py:336) both explain what happens to an existing tag without a row under replace=True and replace=False. The comment is enough, so the docstring can drop those sentences.
• The _validate_parent docstring (actions.py:132) is one 11-line paragraph that walks through the code. Two sentences would do, for example: "Return an error unless parent_id names a tag that holds that id once this import finishes. A row that names its own parent's pre-import id while this file renames that parent is always rejected, because that id then names nothing or, in a swap, another tag."
• _renamed_to_ids (actions.py:121) falls back to deriving the set from _vacated_pks "for an action built outside a plan". The only code that builds actions outside a plan is the tests in test_actions.py (lines 149, 174, 201 and 241), which set _vacated_pks by hand. If those tests also set _renamed_to_ids, the helper becomes a plain dict lookup, and the production code no longer carries a branch that only tests use.

4. Nit, optional

The stale-parent error reads Action error in 'update_parent' (#6): Row 2: Parent tag ..., so the plan position that an admin can't map back to the file still comes before the row number. The "User-visible changes" section of the PR description quotes the message without that prefix. Dropping the prefix for errors that have a row would change ImportActionError for every action, so I'm fine leaving it to the follow-up issue about naming the row in the "Unknown parent tag" error.

[ufedaseyeuconsultant]

1. #858's acceptance criteria

Done. AC were updated

2. TestImportAcceptanceScenarios is too large

Done. The class went from 1,036 lines and 25 tests to 588 lines and 17 tests. The import_export suite now runs 111 full imports instead of 363.

• Duplicates removed. I removed the tests you listed:

  • the four parent-move scenarios;
  • the too-deep scenarios;
  • the freed-id cycles;
  • the swap and the three-way rotation;
  • the third test that the stale-parent error names its row.

  Where a removed test added something, that part moved into the test that remains:

  • test_execute_parent_moves_whose_steps_depend_on_each_other now runs each case in its listed order, reversed and one mixed order, and checks that moved tags keep their pks.
  • test_generate_actions_parent_cycle_rejected_once has a case for the other row order of the two renamed tags.
  • test_import_swap_external_ids and test_import_three_cycle_external_ids run in more row orders.

• Formats and row orders. Each remaining scenario runs in JSON, in the file order, the reversed order and one mixed order. The class makes a single CSV import. It is in the stale-swap test, where the row numbers in the error depend on the parser.

• Docstrings. Each docstring is now one or two sentences on the behavior under test. The ASCII trees, the scenario titles and the references to other tests are gone, including the list of test names in the class docstring.

3. Comments in the source

Done.

• The reason for detaching first is now only in the TagImportPlan docstring. The DetachTag docstring and the comment in generate_actions are one line each.
• The _end_state docstring no longer covers existing tags without a row. The comment in its second loop now also says that such a tag stays under the same parent tag, recorded by that parent's final id.
• The _validate_parent docstring is two sentences, and the second one is yours. I changed the first sentence: "names a tag that holds that id once this import finishes" is slightly too strong. A parent_id that names an existing tag the import does not rename is accepted even if a full replace import deletes that tag. That case is listed in the PR description as a known issue for a follow-up. The sentence now says what the code accepts: an existing tag that the import does not rename, or the final id that a row gives to a tag by a rename or a create.
• The fallback in _renamed_to_ids is gone, and so is the helper. Both callers read indexed_actions["_renamed_to_ids"] directly, with an empty set when the key is missing, as they already do for _vacated_pks. The four _validate_parent tests in test_actions.py that build actions outside a plan now set _renamed_to_ids next to _vacated_pks.

The behavior is unchanged. The randomized end-state run gave the same counts before and after these changes, with no mismatch.

4. The plan position before the row number

I left it for the follow-up about naming the row in the "Unknown parent tag" error. The PR description now quotes the error as it appears in the log, Action error in 'update_parent' (#K): Row N: .... It also explains that #K is the action's position in the plan, and that the row is the Row N part.

[mgwozdz-unicon]

Claude Fable and I worked on this review together.

Thanks, this round addressed everything from my last review: #858 now matches the branch, TestImportAcceptanceScenarios is down to the scenarios nothing else covers, and the source comments are consolidated. I ran the import_export suite (234 tests), pylint, pycodestyle, isort, mypy and lint-imports against 299cae5 and everything passes, and CI is green. Approving, but please file the follow-up issues you mentioned. Everything else below are nits. Line numbers refer to 299cae5.

1. Please file the three follow-up issues and link them in the PR description

The "Other information" section says each known issue has its own issue, but I can't find any filed yet. Your comment above is already issue-ready, so please open the three issues from it (each as a child of #673, labeled as belonging to Epic 4, and assigned to you in Sprint 5) and replace "each with its own issue" with the issue numbers. Two scope additions while you're there:

• For the second follow-up (a new parent that the same replace=True import deletes), please also cover the sibling case I ran into: a child row that keeps the id of its current parent while the file omits that parent. The plan accepts it, the delete phase promotes the child to the root, and the import succeeds, so the result differs from what the file says (the row said parent_id: tag_1, the tag ends up at the root). This also happens on main. A check in _validate_parent alone wouldn't catch it, because UpdateParentTag.applies_for treats the row as "no change" and _validate_parent never runs for it.
• For the third follow-up (the renamed-plus-new cycle), please include the new-only cycle too. _validate_hierarchy deliberately leaves a cycle made only of new tags to _validate_parent (import_plan.py:377), so a file where new_a and new_b name each other gets Unknown parent tag (new_a). Add a row for the parent to your file., which is misleading because the file does have that row. Reporting the cycle error for every cycle and skipping the unknown-parent check for rows on a reported cycle would fix both at once.

2. Nit in the source

1. actions.py:724: the comment restates the assert below it, which is there to narrow the type for mypy. I'd delete the comment.

3. Nits in the tests

1. test_import_plan.py:837-840: the assertion that every detach comes before every other action took me a moment to read. Something like self.assertEqual(names[:names.count("detach")], ["detach"] * names.count("detach")) says it directly.
2. test_api.py:1292 (test_new_id_without_previous_id_is_created) and 1354 (test_export_has_no_previous_id_and_round_trips_without_changes) pass on main and repeat existing coverage: test_valid and test_import_with_export_output in the same file, test_export_does_not_include_previous_id in test_parsers.py, and test_reimport_after_generated_external_ids_is_a_no_op. I'd delete both.
3. test_api.py:1468 (test_three_level_chain_reversed) and 1486 (test_tag_moved_under_its_former_grandchild) also pass on main, so they don't catch a bug this PR fixes. They're still reasonable regression guards for the detach step. Keep them, or fold the two trees into the ddt list of test_execute_parent_moves_whose_steps_depend_on_each_other (test_import_plan.py:765), whichever you prefer.
4. test_api.py:1396: the two messages above it are asserted in full, while this one only checks for COMP-GHOST. The full message is Unknown previous_id (COMP-GHOST). No tag with that external_id exists in this taxonomy.
5. test_api.py:1180: _export is a one-line wrapper with three callers. I'd inline it.
6. test_import_plan.py:652: the first two ddt cases (exact id, case variant) are the same scenarios as test_swap_children_naming_stale_ids_rejected and test_swap_child_naming_parent_id_in_another_case in test_api.py. The third case, where the child is renamed too, is the only one not covered elsewhere.
7. test_import_plan.py:670 only differs from line 632 in that the child is a new tag and the deletes shift the action number, so it could be a ddt case of that test rather than its own.
8. test_api.py:1654: a plain rename with a case-variant parent id goes through the same path as the exact-case test at test_import_plan.py:600, because the case-insensitivity comes from the database field. It could be a ddt case there instead of an API test.

[ufedaseyeuconsultant]

@mgwozdz-unicon

1. Follow-up issues

I filed two issues instead of three, both as children of #673, in Epic 4 and Sprint 5:

• Tag import: parent errors name their row and report every cycle as a cycle #892: parent errors name their row and report every cycle as a cycle. This merges the first and third follow-ups from my earlier comment. Both change the same "Unknown parent tag" error in _validate_parent and the same tests, and the first one alone was a couple of hours of work. It includes your scope addition: every cycle gets the parent cycle error, including a cycle made only of new tags, and the unknown-parent check is skipped only for rows on a reported cycle, so the stale-parent error from this PR is still reported for such a row.
• Tag import: reject a parent_id that names a tag the full replace import deletes #893: reject a parent_id that names a tag the full replace import deletes. It includes your sibling case, where a kept child's row names its current parent while the file omits that parent. As you noted, _validate_parent never runs for that row, so the issue proposes one plan-level check over the rows instead. One thing worth knowing: three existing tests on main (test_generate_actions_3, test_plan_3 and test_execute_2) import exactly that file and expect it to succeed, so they change with the fix. The issue also asks whether this changed outcome needs a DEPR or only a callout in the PR description.

The PR description now links both issues in place of "each with its own issue".

2. Nit in the source

1. Done. The comment above the assert in DetachTag.execute is gone.

3. Nits in the tests

1. Done. The assertion now reads self.assertEqual(names[:names.count('detach')], ['detach'] * names.count('detach')).
2. Done. Both tests are deleted. I checked each assertion against the tests you named before deleting. One small check did not carry over: the deleted export test also compared the CSV header to exactly id,value,parent_id, while test_export_does_not_include_previous_id only checks that previous_id is absent. I judged that enough; I can add the exact-header check to the parser test if you'd like it.
3. I folded both trees into the ddt list of test_execute_parent_moves_whose_steps_depend_on_each_other. Each case lists its rows in the order that builds a cycle without the detach step, and the test runs it in that order, reversed and one mixed order, and checks that every tag keeps its pk.
4. Done. The test asserts the full Unknown previous_id (COMP-GHOST). No tag with that external_id exists in this taxonomy.
5. Done. _export is inlined at its callers.
6. Done. Only the case where the child is renamed too remains, as test_generate_actions_swap_renamed_child_naming_own_parent_stale_id_rejected, and it asserts the full error.
7. Done. The new-child case is now the second ddt case of test_generate_actions_existing_child_keeps_stale_parent_id_rejected.
8. Done. The case-variant plain rename is now ddt cases of test_generate_actions_parent_id_stale_after_plain_rename_rejected, one with the child row first, and the API test is gone.

The import_export suite now has 231 tests, and tests/openedx_tagging passes with 617.

[mgwozdz-unicon]

Thank you!