feat!: deprecate "descendant_count" to improve performance

bradenmacdonald · bradenmacdonald · commit 4c4b876d987e · 2026-03-26T14:34:10.000-07:00
It can be relatively easily calculated in python using the result data if needed, now that the API otherwise supports unlimited depth.
diff --git a/src/openedx_tagging/data.py b/src/openedx_tagging/data.py
@@ -21,7 +21,7 @@ class TagData(TypedDict):
     value: str
     external_id: str | None
     child_count: int
-    descendant_count: int
+    descendant_count: int  # Deprecated; do not use.
     depth: int
     parent_value: str | None
     # Note: usage_count may or may not be present, depending on the request.
diff --git a/src/openedx_tagging/models/base.py b/src/openedx_tagging/models/base.py
@@ -524,14 +524,8 @@ def _get_filtered_tags_one_level(
             qs = self.tag_set.filter(parent=None)
             qs = qs.annotate(parent_value=Value(None, output_field=models.CharField()))
         qs = qs.annotate(child_count=models.Count("children", distinct=True))  # type: ignore[no-redef]
-        # Count all descendants at any depth using depth + lineage prefix.
-        # depth__gt correctly excludes self; lineage prefix matches all descendants.
-        descendants_sq = (
-            self.tag_set.filter(depth__gt=models.OuterRef("depth"), lineage__startswith=models.OuterRef("lineage"))
-            .order_by()
-            .annotate(count=models.Func(F("id"), function="Count"))
-        )
-        qs = qs.annotate(descendant_count=models.Subquery(descendants_sq.values("count")))  # type: ignore[no-redef]
+        # Add the deprecated "descendant_count field". For now it's just the same as child_count.
+        qs = qs.annotate(descendant_count=F("child_count"))
         # Filter by search term:
         if search_term:
             qs = qs.filter(value__icontains=search_term)
@@ -562,11 +556,6 @@ def _get_filtered_tags_deep(
         Implementation of get_filtered_tags() for closed taxonomies, where
         we're including tags from multiple levels of the hierarchy.
         """
-        # Note: we ignore a lot of "no-redef" warnings here because we use annotations to pre-load fields like
-        # `child_count`, and `descendant_count` for all tags in a single query rather than computing them later for each
-        # Tag, with additional queries. Also, we are converting the result to a values query (that returns a dict), not
-        # returning actual Tag objects at the end, but mypy doesn't know that.
-
         if parent_tag_value:
             # Get a subtree below this tag:
             main_parent_tag = self.tag_for_value(parent_tag_value)
@@ -611,28 +600,14 @@ def _get_filtered_tags_deep(
             # frontend.
 
         # Count the direct children, and annotate the result on each row as "child_count".
-        # The query below produces the same results as:
-        #   qs = initial_qs.annotate(child_count=models.Count("children"))
-        # However, this correlated subquery avoids a JOIN + GROUP BY, and is far more efficient in practice.
-        # This also lets us use the same code path whether there's a search_term or not.
         child_count_sq = (
             initial_qs.filter(parent_id=models.OuterRef("pk"))
             .order_by()
             .annotate(count=models.Func(F("id"), function="Count"))
         )
-        # Count all descendants at any depth using the lineage prefix trick.
-        descendants_sq = (
-            initial_qs.filter(
-                depth__gt=models.OuterRef("depth"),
-                lineage__startswith=models.OuterRef("lineage"),
-            )
-            .order_by()
-            .annotate(count=models.Func(F("id"), function="Count"))
-        )
-        qs = initial_qs.annotate(  # type: ignore[no-redef]
-            child_count=models.Subquery(child_count_sq.values("count")),
-            descendant_count=models.Subquery(descendants_sq.values("count")),
-        )
+        qs = initial_qs.annotate(child_count=models.Subquery(child_count_sq.values("count")))
+        # Add the deprecated "descendant_count" field. For now it just is the same as child_count.
+        qs = qs.annotate(descendant_count=F("child_count"))
 
         # Add the parent value
         qs = qs.annotate(parent_value=F("parent__value"))
diff --git a/src/openedx_tagging/rest_api/v1/serializers.py b/src/openedx_tagging/rest_api/v1/serializers.py
@@ -226,7 +226,7 @@ class TagDataSerializer(UserPermissionsSerializerMixin, serializers.Serializer):
     value = serializers.CharField()
     external_id = serializers.CharField(allow_null=True)
     child_count = serializers.IntegerField()
-    descendant_count = serializers.IntegerField()
+    descendant_count = serializers.IntegerField()  # deprecated
     depth = serializers.IntegerField()
     parent_value = serializers.CharField(allow_null=True)
     usage_count = serializers.IntegerField(required=False)
diff --git a/tests/openedx_tagging/import_export/test_template.py b/tests/openedx_tagging/import_export/test_template.py
@@ -53,25 +53,25 @@ def test_import_template(self, template_file, parser_format):
             'Electronic instruments (ELECTRIC) (None) (children: 2)',
             '  Synthesizer (SYNTH) (Electronic instruments) (children: 0)',
             '  Theramin (THERAMIN) (Electronic instruments) (children: 0)',
-            'Percussion instruments (PERCUSS) (None) (children: 3 + 6)',
+            'Percussion instruments (PERCUSS) (None) (children: 3)',
             '  Chordophone (CHORD) (Percussion instruments) (children: 1)',
             '    Piano (PIANO) (Chordophone) (children: 0)',
             '  Idiophone (BELLS) (Percussion instruments) (children: 2)',
             '    Celesta (CELESTA) (Idiophone) (children: 0)',
             '    Hi-hat (HI-HAT) (Idiophone) (children: 0)',
-            '  Membranophone (DRUMS) (Percussion instruments) (children: 2 + 1)',
+            '  Membranophone (DRUMS) (Percussion instruments) (children: 2)',
             '    Cajón (CAJÓN) (Membranophone) (children: 1)',
             '      Pyle Stringed Jam Cajón (PYLE) (Cajón) (children: 0)',
             '    Tabla (TABLA) (Membranophone) (children: 0)',
-            'String instruments (STRINGS) (None) (children: 2 + 5)',
+            'String instruments (STRINGS) (None) (children: 2)',
             '  Bowed strings (BOW) (String instruments) (children: 2)',
             '    Cello (CELLO) (Bowed strings) (children: 0)',
             '    Violin (VIOLIN) (Bowed strings) (children: 0)',
             '  Plucked strings (PLUCK) (String instruments) (children: 3)',
             '    Banjo (BANJO) (Plucked strings) (children: 0)',
             '    Harp (HARP) (Plucked strings) (children: 0)',
             '    Mandolin (MANDOLIN) (Plucked strings) (children: 0)',
-            'Wind instruments (WINDS) (None) (children: 2 + 5)',
+            'Wind instruments (WINDS) (None) (children: 2)',
             '  Brass (BRASS) (Wind instruments) (children: 2)',
             '    Trumpet (TRUMPET) (Brass) (children: 0)',
             '    Tuba (TUBA) (Brass) (children: 0)',
diff --git a/tests/openedx_tagging/test_api.py b/tests/openedx_tagging/test_api.py
@@ -145,8 +145,8 @@ def test_get_tags(self) -> None:
             "Bacteria (children: 2)",
             "  Archaebacteria (children: 0)",
             "  Eubacteria (children: 0)",
-            "Eukaryota (children: 5 + 8)",
-            "  Animalia (children: 7 + 1)",
+            "Eukaryota (children: 5)",
+            "  Animalia (children: 7)",
             "    Arthropoda (children: 0)",
             "    Chordata (children: 1)",
             "      Mammalia (children: 0)",
@@ -175,7 +175,7 @@ def test_get_root_tags(self):
         assert pretty_format_tags(root_life_on_earth_tags, parent=False) == [
             'Archaea (children: 3)',
             'Bacteria (children: 2)',
-            'Eukaryota (children: 5 + 8)',
+            'Eukaryota (children: 5)',
         ]
 
     @override_settings(LANGUAGES=test_languages)
@@ -764,7 +764,7 @@ def get_object_tags():
             "  Proteoarchaeota (used: 0, children: 0)",
             "Bacteria (used: 0, children: 1)",  # does not contain "ar" but a child does
             "  Archaebacteria (used: 1, children: 0)",
-            "Eukaryota (used: 0, children: 1 + 2)",
+            "Eukaryota (used: 0, children: 1)",
             "  Animalia (used: 1, children: 2)",  # does not contain "ar" but a child does
             "    Arthropoda (used: 1, children: 0)",
             "    Cnidaria (used: 0, children: 0)",
@@ -786,8 +786,8 @@ def get_object_tags():
             "Bacteria (used: 0, children: 2)",
             "  Archaebacteria (used: 1, children: 0)",
             "  Eubacteria (used: 0, children: 0)",
-            "Eukaryota (used: 0, children: 4 + 8)",
-            "  Animalia (used: 1, children: 7 + 1)",
+            "Eukaryota (used: 0, children: 4)",
+            "  Animalia (used: 1, children: 7)",
             "    Arthropoda (used: 1, children: 0)",
             "    Chordata (used: 0, children: 1)",
             "      Mammalia (used: 0, children: 0)",
@@ -1107,10 +1107,10 @@ def test_deep_taxonomy(self) -> None:
         assert pretty_format_tags(
             tagging_api.get_tags(taxonomy)
         ) == [
-            "Root - depth 0 (None) (children: 1 + 4)",
-            "  Child - depth 1 (Root - depth 0) (children: 1 + 3)",
-            "    Grandchild - depth 2 (Child - depth 1) (children: 1 + 2)",
-            "      Great-Grandchild - depth 3 (Grandchild - depth 2) (children: 1 + 1)",
+            "Root - depth 0 (None) (children: 1)",
+            "  Child - depth 1 (Root - depth 0) (children: 1)",
+            "    Grandchild - depth 2 (Child - depth 1) (children: 1)",
+            "      Great-Grandchild - depth 3 (Grandchild - depth 2) (children: 1)",
             "        Great-Great-Grandchild - depth 4 (Great-Grandchild - depth 3) (children: 1)",
             "          Great-Great-Great-Grandchild - depth 5 (Great-Great-Grandchild - depth 4) (children: 0)",
         ]
@@ -1128,7 +1128,7 @@ def test_deep_taxonomy(self) -> None:
         # Or even load a subtree:
         deep_result = taxonomy.get_filtered_tags(depth=None, parent_tag_value=tag_2.value)
         assert pretty_format_tags(deep_result, parent=False) == [
-            '      Great-Grandchild - depth 3 (children: 1 + 1)',
+            '      Great-Grandchild - depth 3 (children: 1)',
             '        Great-Great-Grandchild - depth 4 (children: 1)',
             '          Great-Great-Great-Grandchild - depth 5 (children: 0)',
         ]
diff --git a/tests/openedx_tagging/test_models.py b/tests/openedx_tagging/test_models.py
@@ -331,7 +331,7 @@ def test_get_root(self) -> None:
             # These are the root tags, in alphabetical order:
             {"value": "Archaea", "child_count": 3, "descendant_count": 3, **common_fields},
             {"value": "Bacteria", "child_count": 2, "descendant_count": 2, **common_fields},
-            {"value": "Eukaryota", "child_count": 5, "descendant_count": 13, **common_fields},
+            {"value": "Eukaryota", "child_count": 5, "descendant_count": 5, **common_fields},
         ]
 
     def test_get_child_tags_one_level(self) -> None:
@@ -345,7 +345,7 @@ def test_get_child_tags_one_level(self) -> None:
             del r["_id"]  # Remove the internal database IDs; they aren't interesting here and a other tests check them
         assert result == [
             # These are the Eukaryota tags, in alphabetical order:
-            {"value": "Animalia", "child_count": 7, "descendant_count": 8, **common_fields},
+            {"value": "Animalia", "child_count": 7, "descendant_count": 7, **common_fields},
             {"value": "Fungi", "child_count": 0, "descendant_count": 0, **common_fields},
             {"value": "Monera", "child_count": 0, "descendant_count": 0, **common_fields},
             {"value": "Plantae", "child_count": 0, "descendant_count": 0, **common_fields},
@@ -441,8 +441,8 @@ def test_get_all(self) -> None:
             "Bacteria (None) (children: 2)",
             "  Archaebacteria (Bacteria) (children: 0)",
             "  Eubacteria (Bacteria) (children: 0)",
-            "Eukaryota (None) (children: 5 + 8)",
-            "  Animalia (Eukaryota) (children: 7 + 1)",
+            "Eukaryota (None) (children: 5)",
+            "  Animalia (Eukaryota) (children: 7)",
             "    Arthropoda (Animalia) (children: 0)",
             "    Chordata (Animalia) (children: 1)",
             "      Mammalia (Chordata) (children: 0)",
@@ -478,7 +478,7 @@ def test_search_2(self) -> None:
         """
         result = pretty_format_tags(self.taxonomy.get_filtered_tags(search_term="chordata"))
         assert result == [
-            "Eukaryota (None) (children: 1 + 1)",  # Has one child that matches, plus one additional matching descendant
+            "Eukaryota (None) (children: 1)",  # Has one child that matches
             "  Animalia (Eukaryota) (children: 1)",
             "    Chordata (Animalia) (children: 0)",  # this is the matching tag.
         ]
@@ -492,8 +492,8 @@ def test_search_3(self) -> None:
         assert result == [
             "Archaea (None) (children: 1)",
             "  Proteoarchaeota (Archaea) (children: 0)",
-            "Eukaryota (None) (children: 2 + 2)",  # 2 direct matching children, 2 additional matching descendants
-            "  Animalia (Eukaryota) (children: 2)",
+            "Eukaryota (None) (children: 2)",  # 2 direct matching children
+            "  Animalia (Eukaryota) (children: 2)",  # also 2 matching children
             "    Arthropoda (Animalia) (children: 0)",  # match
             "    Gastrotrich (Animalia) (children: 0)",  # match
             "  Protista (Eukaryota) (children: 0)",  # match
@@ -575,7 +575,7 @@ def test_tree_sort(self) -> None:
         taxonomy = self.create_sort_test_taxonomy()
         result = pretty_format_tags(taxonomy.get_filtered_tags())
         assert result == [
-            "1 (None) (children: 4 + 1)",
+            "1 (None) (children: 4)",
             "  1 A (1) (children: 0)",
             "  11 (1) (children: 0)",
             "  11111 (1) (children: 1)",
@@ -595,43 +595,6 @@ def test_tree_sort(self) -> None:
             "  azure (ALPHABET) (children: 0)",
         ]
 
-    def test_descendant_counts(self) -> None:
-        """
-        Test getting the descendant count on a taxonomy known to cause aggregation
-        bugs unless the aggregations are correctly specified with distinct=True
-
-        https://docs.djangoproject.com/en/5.0/topics/db/aggregation/#combining-multiple-aggregations
-        """
-        taxonomy = api.create_taxonomy("ESDC Subset")
-        api.add_tag_to_taxonomy(taxonomy, "Interests")  # root tag
-        api.add_tag_to_taxonomy(taxonomy, "Holland Codes", parent_tag_value="Interests")  # child tag
-        # Create the grandchild tag:
-        g_tag = api.add_tag_to_taxonomy(taxonomy, "Interests - Holland Codes", parent_tag_value="Holland Codes")
-        # Create the 6 great-grandchild tags:
-        api.add_tag_to_taxonomy(taxonomy, "Artistic", parent_tag_value=g_tag.value)
-        api.add_tag_to_taxonomy(taxonomy, "Conventional", parent_tag_value=g_tag.value)
-        api.add_tag_to_taxonomy(taxonomy, "Enterprising", parent_tag_value=g_tag.value)
-        api.add_tag_to_taxonomy(taxonomy, "Investigative", parent_tag_value=g_tag.value)
-        api.add_tag_to_taxonomy(taxonomy, "Realistic", parent_tag_value=g_tag.value)
-        api.add_tag_to_taxonomy(taxonomy, "Social", parent_tag_value=g_tag.value)
-
-        result = pretty_format_tags(taxonomy.get_filtered_tags(depth=1, include_counts=True))
-        assert result == [
-            "Interests (None) (used: 0, children: 1 + 7)",  # 1 child + (1 grandchild and 6 great grandchild tags)
-        ]
-        result2 = pretty_format_tags(taxonomy.get_filtered_tags(depth=None, include_counts=True))
-        assert result2 == [
-            "Interests (None) (used: 0, children: 1 + 7)",
-            "  Holland Codes (Interests) (used: 0, children: 1 + 6)",
-            "    Interests - Holland Codes (Holland Codes) (used: 0, children: 6)",
-            "      Artistic (Interests - Holland Codes) (used: 0, children: 0)",
-            "      Conventional (Interests - Holland Codes) (used: 0, children: 0)",
-            "      Enterprising (Interests - Holland Codes) (used: 0, children: 0)",
-            "      Investigative (Interests - Holland Codes) (used: 0, children: 0)",
-            "      Realistic (Interests - Holland Codes) (used: 0, children: 0)",
-            "      Social (Interests - Holland Codes) (used: 0, children: 0)",
-        ]
-
 
 class TestFilteredTagsFreeTextTaxonomy(TestCase):
     """
diff --git a/tests/openedx_tagging/test_views.py b/tests/openedx_tagging/test_views.py
diff --git a/tests/openedx_tagging/utils.py b/tests/openedx_tagging/utils.py
