Clean up label-embedding architecture

- Move {id,label} stripping into _SetCriterion.from_json() so both
  MultiCriterion and ChoiceCriterion normalise at the parse boundary;
  the querying layer stays typed (list[int] / list[str]) and clean.
- Revert MultiCriterion to a thin _extra_q() override; _SetCriterion.to_q()
  is no longer duplicated.
- JS: readSearchSelect always emits {id, label} objects — no conditional
  mixed-type arrays. filter_bar.js stores them as-is for all fields,
  removing the fragile isIdField hardcoded list.
- Update tests to use the {id, label} filter format.

https://claude.ai/code/session_01EyAJcMoDktLrY9tSbdHViA

common/criteria.py

@@ -310,57 +310,32 @@ class _SetCriterion(_Criterion):
         """Hook for subclass-specific modifiers; ``None`` means unsupported."""
         return None
 
+    @classmethod
+    def from_json(cls, data: dict | None) -> Self | None:
+        result = super().from_json(data)
+        if result is None:
+            return None
+        # Labels embedded as {id, label} dicts are display-only; strip to bare ids
+        # so the querying layer stays clean and typed.
+        result.value = [item["id"] if isinstance(item, dict) else item for item in result.value]
+        result.excludes = [item["id"] if isinstance(item, dict) else item for item in result.excludes]
+        return result
+
 
 @dataclass
 class MultiCriterion(_SetCriterion):
-    """Filter on a many-to-many or ForeignKey relationship by ID list.
+    """Filter on a many-to-many or ForeignKey relationship by ID list."""
 
-    Each entry in ``value`` and ``excludes`` may be either a bare integer id or
-    a ``{"id": <int>, "label": <str>}`` dict (Stash-style embedded label). The
-    label is display-only; only the id is used for querying.
-    """
-
-    value: list = field(default_factory=list)
-    excludes: list = field(default_factory=list)
-
-    def _ids(self, items: list) -> list[int]:
-        """Extract integer ids from a mixed list of bare ints or {id, label} dicts."""
-        result = []
-        for item in items:
-            if isinstance(item, dict):
-                result.append(int(item["id"]))
-            else:
-                result.append(int(item))
-        return result
-
-    def to_q(self, field_name: str) -> Q:
-        modifier = self.modifier
-        value_ids = self._ids(self.value)
-        excludes_ids = self._ids(self.excludes)
-        if modifier in (Modifier.INCLUDES, Modifier.EQUALS):
-            q = Q()
-            if value_ids:
-                q &= Q(**{f"{field_name}__in": value_ids})
-            if excludes_ids:
-                q &= ~Q(**{f"{field_name}__in": excludes_ids})
-            return q
-        if modifier == Modifier.IS_NULL:
-            return Q(**{f"{field_name}__isnull": True})
-        if modifier == Modifier.NOT_NULL:
-            return Q(**{f"{field_name}__isnull": False})
-        extra = self._extra_q(field_name)
-        if extra is not None:
-            return extra
-        raise ValueError(f"Unsupported modifier {modifier} for MultiCriterion")
+    value: list[int] = field(default_factory=list)
+    excludes: list[int] = field(default_factory=list)
 
     def _extra_q(self, field_name: str) -> Q | None:
-        value_ids = self._ids(self.value)
         if self.modifier == Modifier.EXCLUDES:
-            return ~Q(**{f"{field_name}__in": value_ids})
+            return ~Q(**{f"{field_name}__in": self.value})
         if self.modifier == Modifier.INCLUDES_ALL:
             q = Q()
-            for id_val in value_ids:
-                q &= Q(**{field_name: id_val})
+            for value in self.value:
+                q &= Q(**{field_name: value})
             return q
         return None
 

games/static/js/filter_bar.js

@@ -71,38 +71,13 @@
       if (modifier === "NOT_NULL" || modifier === "IS_NULL") {
         filter[field] = { modifier: modifier };
       } else if (included.length > 0 || excluded.length > 0) {
-        var isIdField =
-          field === "platform" ||
-          field === "game" ||
-          field === "device" ||
-          field === "games";
-        if (isIdField) {
-          // Store {id, label} objects so the filter URL/preset is self-describing
-          // and pills can be rendered without a DB lookup (Stash-style).
-          filter[field] = {
-            value: included.map(function (item) {
-              return typeof item === "object"
-                ? {id: parseInt(item.id, 10), label: item.label || ""}
-                : {id: parseInt(item, 10), label: ""};
-            }),
-            excludes: excluded.map(function (item) {
-              return typeof item === "object"
-                ? {id: parseInt(item.id, 10), label: item.label || ""}
-                : {id: parseInt(item, 10), label: ""};
-            }),
-            modifier: modifier || "INCLUDES",
-          };
-        } else {
-          filter[field] = {
-            value: included.map(function (item) {
-              return typeof item === "object" ? item.id : item;
-            }),
-            excludes: excluded.map(function (item) {
-              return typeof item === "object" ? item.id : item;
-            }),
-            modifier: modifier || "INCLUDES",
-          };
-        }
+        // All filter pills carry {id, label}; store them as-is so the filter
+        // URL and saved presets are self-describing (Stash-style).
+        filter[field] = {
+          value: included.map(function (item) { return {id: item.id, label: item.label}; }),
+          excludes: excluded.map(function (item) { return {id: item.id, label: item.label}; }),
+          modifier: modifier || "INCLUDES",
+        };
       }
     });
 

games/static/js/search_select.js

@@ -436,11 +436,10 @@
             }
             var value = pill.getAttribute("data-value");
             var label = pill.getAttribute("data-label") || "";
-            var entry = label ? {id: value, label: label} : value;
             if (pill.getAttribute("data-search-select-type") === "exclude") {
-              excluded.push(entry);
+              excluded.push({id: value, label: label});
             } else {
-              included.push(entry);
+              included.push({id: value, label: label});
             }
           });
         }

tests/test_filter_bars.py

@@ -94,7 +94,7 @@ class FilterBarRenderingTest(TestCase):
 
     def test_game_filter_bar_roundtrips_selected_status(self):
         """A status in filter_json renders as an include pill in the widget."""
-        filter_json = json.dumps({"status": {"value": ["f"], "modifier": ""}})
+        filter_json = json.dumps({"status": {"value": [{"id": "f", "label": "Finished"}], "modifier": "INCLUDES"}})
         html = str(
             FilterBar(
                 filter_json=filter_json, preset_list_url="/l", preset_save_url="/s"

tests/test_filters.py

@@ -121,6 +121,15 @@ class TestMultiCriterion:
         c = MultiCriterion(value=[], modifier=Modifier.IS_NULL)
         assert c.to_q("device_id") == Q(device_id__isnull=True)
 
+    def test_from_json_strips_embedded_labels(self):
+        """from_json normalises {id, label} dicts to bare ids."""
+        c = MultiCriterion.from_json(
+            {"value": [{"id": 797, "label": "Hollow Knight"}], "excludes": [{"id": 11, "label": "Steam Deck"}]}
+        )
+        assert c.value == [797]
+        assert c.excludes == [11]
+        assert c.to_q("game_id") == Q(game_id__in=[797]) & ~Q(game_id__in=[11])
+
 
 class TestChoiceCriterionAgainstDB:
     """Verify ChoiceCriterion produces correct DB results."""