fix: make API list ordering deterministic

pokemon_v2/api.py

@@ -1021,7 +1021,7 @@ def get(self, request: Request, pokemon_id: int) -> Response:
                 "encounterconditionvaluemap_set__encounter_condition_value",
                 "encounterpokemondetail_set",
             )
-            .order_by("location_area_id", "version_id", "encounter_slot_id")
+            .order_by("location_area_id", "version_id", "encounter_slot_id", "pk")
         )
 
         grouped_data: list[dict[str, Any]] = []

pokemon_v2/models.py

@@ -1,10 +1,12 @@
 # pyright: reportIncompatibleVariableOverride=false
 from __future__ import annotations
 
-from typing import Any
+from typing import Any, ClassVar, TypeVar
 
 from django.db import models
-from typing_extensions import override
+from typing_extensions import Self, override
+
+_ModelT = TypeVar("_ModelT", bound=models.Model)
 
 __all__: tuple[str, ...] = (
     "Ability",
@@ -165,6 +167,7 @@
     "PalPark",
     "PalParkArea",
     "PalParkAreaName",
+    "PokeApiManager",
     "PokeApiModel",
     "PokeathlonStat",
     "PokeathlonStatName",
@@ -230,7 +233,24 @@
 ############################
 
 
+class PokeApiManager(models.Manager[_ModelT]):
+    """Default manager ordering every queryset by primary key, so that the API
+    serializes lists deterministically. Call sites needing another order
+    override it with ``order_by()``.
+
+    A ``distinct(*fields)`` call has to spell out a matching ``order_by()``,
+    since PostgreSQL requires the leading ``ORDER BY`` expressions to match the
+    ``DISTINCT ON`` ones. A plain ``distinct()`` is unaffected.
+    """
+
+    @override
+    def get_queryset(self) -> models.QuerySet[_ModelT]:
+        return super().get_queryset().order_by("pk")
+
+
 class PokeApiModel(models.Model):
+    objects: ClassVar[PokeApiManager[Self]] = PokeApiManager()
+
     class Meta:
         abstract = True
 

pokemon_v2/serializers.py

@@ -1053,7 +1053,7 @@ def get_method_rates(self, obj: LocationAreaEncounterRate) -> ReturnList[ReturnD
         rates = (
             LocationAreaEncounterRate.objects.filter(location_area=obj, encounter_method__isnull=False)
             .select_related("encounter_method", "version")
-            .order_by("encounter_method_id")
+            .order_by("encounter_method_id", "version_id", "pk")
         )
         grouped_rates: list[dict[str, Any]] = [
             {
@@ -1081,7 +1081,7 @@ def get_encounters(self, obj: LocationArea) -> ReturnList[ReturnDict[str, Any]]:
                 "encounterconditionvaluemap_set",
                 "encounterpokemondetail_set",
             )
-            .order_by("pokemon_id", "version_id")
+            .order_by("pokemon_id", "version_id", "encounter_slot_id", "pk")
         )
 
         grouped_data: list[dict[str, Any]] = []
@@ -1597,7 +1597,7 @@ def get_held_by_pokemon(self, obj: Item) -> ReturnList[ReturnDict[str, Any]]:
         pokemon_items = (
             PokemonItem.objects.filter(item=obj)
             .select_related("pokemon", "version")
-            .order_by("pokemon_id", "version_id")
+            .order_by("pokemon_id", "version_id", "pk")
         )
         grouped_data: list[dict[str, Any]] = [
             {
@@ -1739,7 +1739,9 @@ class Meta:
     @extend_schema_field(BerryFlavorBerryMapSerializer(many=True))
     def get_berries_with_flavor(self, obj: BerryFlavor) -> ReturnList[ReturnDict[str, Any]]:
         flavor_map_objects = (
-            BerryFlavorMap.objects.filter(berry_flavor=obj, potency__gt=0).select_related("berry").order_by("potency")
+            BerryFlavorMap.objects.filter(berry_flavor=obj, potency__gt=0)
+            .select_related("berry")
+            .order_by("potency", "berry_id", "pk")
         )
         return cast(
             "ReturnList[ReturnDict[str, Any]]",
@@ -2629,7 +2631,7 @@ def get_pokemon_form_sprites(self, obj: PokemonForm) -> dict[str, Any]:
 
     @extend_schema_field(PokemonFormTypeSerializer(many=True))
     def get_pokemon_form_types(self, obj: PokemonForm) -> ReturnList[ReturnDict[str, Any]]:
-        form_types = PokemonFormType.objects.filter(pokemon_form=obj).select_related("type").order_by("slot")
+        form_types = PokemonFormType.objects.filter(pokemon_form=obj).select_related("type").order_by("slot", "pk")
 
         if form_types:
             return cast(
@@ -2638,7 +2640,7 @@ def get_pokemon_form_types(self, obj: PokemonForm) -> ReturnList[ReturnDict[str,
             )
 
         # Fall back to parent Pokemon's types if no form-specific types exist
-        pokemon_types = PokemonType.objects.filter(pokemon=obj.pokemon).select_related("type").order_by("slot")
+        pokemon_types = PokemonType.objects.filter(pokemon=obj.pokemon).select_related("type").order_by("slot", "pk")
         return cast(
             "ReturnList[ReturnDict[str, Any]]",
             PokemonTypeSerializer(pokemon_types, many=True, context=self.context).data,
@@ -2946,7 +2948,7 @@ def get_pokemon_moves(self, obj: Pokemon) -> list[dict[str, Any]]:
         pokemon_moves = (
             PokemonMove.objects.filter(pokemon=obj, move__isnull=False)
             .select_related("move", "version_group", "move_learn_method")
-            .order_by("move__id", "version_group_id")
+            .order_by("move__id", "version_group_id", "move_learn_method_id", "level", "pk")
         )
 
         vg_cache: dict[int, Any] = {}
@@ -2990,7 +2992,7 @@ def get_pokemon_held_items(self, obj: Pokemon) -> list[dict[str, Any]]:
         pokemon_items = (
             PokemonItem.objects.filter(pokemon=obj, item__isnull=False)
             .select_related("item", "version")
-            .order_by("item__id", "version_id")
+            .order_by("item__id", "version_id", "pk")
         )
 
         version_cache: dict[int, Any] = {}
@@ -3033,7 +3035,7 @@ def get_past_pokemon_abilities(self, obj: Pokemon) -> list[dict[str, Any]]:
         past_abilities = (
             PokemonAbilityPast.objects.filter(pokemon=obj, generation__isnull=False)
             .select_related("generation", "ability")
-            .order_by("generation_id")
+            .order_by("generation_id", "slot", "pk")
         )
 
         final_data: list[dict[str, Any]] = []
@@ -3061,7 +3063,7 @@ def get_past_pokemon_stats(self, obj: Pokemon) -> list[dict[str, Any]]:
         past_stats = (
             PokemonStatPast.objects.filter(pokemon=obj, generation__isnull=False)
             .select_related("generation", "stat")
-            .order_by("generation_id")
+            .order_by("generation_id", "stat_id", "pk")
         )
 
         final_data: list[dict[str, Any]] = []
@@ -3086,7 +3088,7 @@ def get_past_pokemon_stats(self, obj: Pokemon) -> list[dict[str, Any]]:
 
     @extend_schema_field(PokemonTypeSerializer(many=True))
     def get_pokemon_types(self, obj: Pokemon) -> ReturnList[ReturnDict[str, Any]]:
-        types = PokemonType.objects.filter(pokemon=obj).select_related("type").order_by("slot")
+        types = PokemonType.objects.filter(pokemon=obj).select_related("type").order_by("slot", "pk")
         return cast(
             "ReturnList[ReturnDict[str, Any]]",
             PokemonTypeSerializer(types, many=True, context=self.context).data,
@@ -3097,7 +3099,7 @@ def get_past_pokemon_types(self, obj: Pokemon) -> list[dict[str, Any]]:
         past_types = (
             PokemonTypePast.objects.filter(pokemon=obj, generation__isnull=False)
             .select_related("generation", "type")
-            .order_by("generation_id", "slot")
+            .order_by("generation_id", "slot", "pk")
         )
 
         final_data: list[dict[str, Any]] = []
@@ -3361,7 +3363,7 @@ class Meta:
 
     @extend_schema_field(EvolutionChainLinkSerializer)
     def build_chain(self, obj: EvolutionChain) -> dict[str, Any]:
-        pokemon_objects = PokemonSpecies.objects.filter(evolution_chain=obj).order_by("order")
+        pokemon_objects = PokemonSpecies.objects.filter(evolution_chain=obj).order_by("order", "pk")
         summary_data = cast(
             "ReturnList[ReturnDict[str, Any]]",
             PokemonSpeciesSummarySerializer(pokemon_objects, many=True, context=self.context).data,
@@ -3535,7 +3537,9 @@ class Meta:
     @extend_schema_field(PokemonDexNumberSerializer(many=True))
     def get_pokedex_entries(self, obj: Pokedex) -> ReturnList[ReturnDict[str, Any]]:
         entries = (
-            PokemonDexNumber.objects.filter(pokedex=obj).select_related("pokemon_species").order_by("pokedex_number")
+            PokemonDexNumber.objects.filter(pokedex=obj)
+            .select_related("pokemon_species")
+            .order_by("pokedex_number", "pokemon_species_id", "pk")
         )
         return cast(
             "ReturnList[ReturnDict[str, Any]]",

pokemon_v2/tests.py

@@ -1309,6 +1309,7 @@ def setup_pokemon_species_data(
         is_legendary=False,
         is_mythical=False,
         order=1,
+        pk=None,
     ):
         generation = generation or cls.setup_generation_data(name="gen for " + name)
 
@@ -1321,6 +1322,7 @@ def setup_pokemon_species_data(
         pokemon_habitat = pokemon_habitat or cls.setup_pokemon_habitat_data(name="pkm hbtt for " + name)
 
         pokemon_species = PokemonSpecies.objects.create(
+            pk=pk,
             name=name,
             generation=generation,
             evolves_from_species=evolves_from_species,
@@ -1573,10 +1575,11 @@ def setup_pokemon_item_data(cls, pokemon=None, item=None, version=None, rarity=5
         return pokemon_item
 
     @classmethod
-    def setup_pokemon_move_data(cls, pokemon, move, version_group, level=0, order=1):
-        move_learn_method = cls.setup_move_learn_method_data(name="mv lrn mthd for pkmn")
+    def setup_pokemon_move_data(cls, pokemon, move, version_group, level=0, order=1, move_learn_method=None, pk=None):
+        move_learn_method = move_learn_method or cls.setup_move_learn_method_data(name="mv lrn mthd for pkmn")
 
         pokemon_move = PokemonMove.objects.create(
+            pk=pk,
             pokemon=pokemon,
             version_group=version_group,
             move=move,
@@ -2686,6 +2689,29 @@ def test_growth_rate_api(self):
             "{}{}/pokemon-species/{}/".format(TEST_HOST, API_V2, pokemon_species.pk),
         )
 
+    def test_reverse_relation_lists_are_ordered_by_pk(self):
+        # Lists coming straight from a reverse relation have no order_by() of
+        # their own, so they fall back to the manager ordering by pk. The pks
+        # are inserted out of order, since an unordered query returns them in
+        # insertion order and would pass either way.
+        growth_rate = self.setup_growth_rate_data(name="grth rt for ordering")
+        species = [
+            self.setup_pokemon_species_data(
+                pk=pk,
+                growth_rate=growth_rate,
+                name="pkmn spcs for ordering {}".format(pk),
+            )
+            for pk in (30, 10, 20)
+        ]
+
+        response = self.client.get("{}/growth-rate/{}/".format(API_V2, growth_rate.pk))
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(
+            [entry["name"] for entry in response.data["pokemon_species"]],
+            [entry.name for entry in sorted(species, key=lambda entry: entry.pk)],
+        )
+
     # Location Tests
     def test_location_api(self):
         location = self.setup_location_data(name="base lctn")
@@ -4591,6 +4617,38 @@ def test_pokemon_moves_with_non_contiguous_version_group_and_method_ids(self):
             "{}{}/move-learn-method/{}/".format(TEST_HOST, API_V2, pokemon_move.move_learn_method.pk),
         )
 
+    def test_pokemon_moves_version_group_details_are_deterministically_ordered(self):
+        # A pokemon can learn the same move in the same version group more than
+        # once, so these rows tie on every other order_by() field and fall back
+        # to the pk. The pks are inserted out of order on purpose.
+        pokemon_species = self.setup_pokemon_species_data(name="pkmn spcs for mv ordering")
+        pokemon = self.setup_pokemon_data(pokemon_species=pokemon_species, name="pkmn for mv ordering")
+        self.setup_pokemon_sprites_data(pokemon=pokemon)
+        self.setup_pokemon_cries_data(pokemon, latest=True, legacy=True)
+
+        move = self.setup_move_data(name="mv for mv ordering")
+        version_group = self.setup_version_group_data(name="ver grp for mv ordering")
+        move_learn_method = self.setup_move_learn_method_data(name="mv lrn mthd for mv ordering")
+
+        for pk, order in ((30, 3), (10, 1), (20, 2)):
+            self.setup_pokemon_move_data(
+                pk=pk,
+                pokemon=pokemon,
+                move=move,
+                version_group=version_group,
+                move_learn_method=move_learn_method,
+                level=5,
+                order=order,
+            )
+
+        response = self.client.get("{}/pokemon/{}/".format(API_V2, pokemon.pk), headers={"host": "testserver"})
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(
+            [detail["order"] for detail in response.data["moves"][0]["version_group_details"]],
+            [1, 2, 3],
+        )
+
     def test_pokemon_form_api(self):
         pokemon_species = self.setup_pokemon_species_data()
         pokemon = self.setup_pokemon_data(pokemon_species=pokemon_species)