fix(stats): unify the device unknown bucket across group-by and filters

The pre-existing device strategy defaulted missing docs to "Unknown"
while the classifier stores "unknown", splitting the bucket and making
neither half filterable. Aggregation now defaults to the classifier's
value, device joins the null-sentinel filter map, and the sentinel
branch keeps stored values in its $in since "unknown" is also real.

Author: Zingzy

schemas/dto/requests/_descriptions.py

@@ -145,7 +145,8 @@
     "**Method 2: Individual Filter Parameter**\n\n"
     "Comma-separated device types. Alternative to using the `filters` JSON "
     "parameter.\n\n"
-    "**Values:** `mobile`, `tablet`, `desktop`, `unknown`.\n\n"
+    "**Values:** `mobile`, `tablet`, `desktop`, `unknown`. `unknown` also "
+    "matches clicks recorded before device tracking existed.\n\n"
     "**Note:** Both `filters` JSON and individual parameters can be combined."
 )
 

services/stats_service.py

@@ -63,10 +63,14 @@
 _KNOWN_TIMEZONES: frozenset[str] = frozenset(available_timezones())
 
 # Dimensions whose aggregation output maps null/missing to a sentinel value
-# ("Direct" referrers, "(none)" for untagged utm clicks). Filtering by the
-# sentinel must match those null/missing documents too.
+# ("Direct" referrers, "(none)" for untagged utm clicks, "unknown" for
+# clicks that predate device tracking). Filtering by the sentinel must
+# match those null/missing documents too. Note "unknown" is ALSO a real
+# stored value (the classifier's fallback) — the filter branch keeps the
+# sentinel in its stored-value $in for exactly that reason.
 _NULL_SENTINEL_FILTERS: dict[StatsDimension, str] = {
     StatsDimension.REFERRER: "Direct",
+    StatsDimension.DEVICE: "unknown",
     StatsDimension.UTM_SOURCE: "(none)",
     StatsDimension.UTM_MEDIUM: "(none)",
     StatsDimension.UTM_CAMPAIGN: "(none)",
@@ -182,18 +186,21 @@ def _build_click_query(
                     )
                     continue
                 query["meta.short_code"] = {"$in": values}
-            elif dimension in _NULL_SENTINEL_FILTERS:
-                sentinel = _NULL_SENTINEL_FILTERS[dimension]
-                if sentinel in values:
-                    non_null = [v for v in values if v != sentinel]
-                    clauses: list[dict[str, Any]] = []
-                    if non_null:
-                        clauses.append({dimension: {"$in": non_null}})
-                    clauses.append({dimension: {"$in": [None, ""]}})
-                    clauses.append({dimension: {"$exists": False}})
-                    or_groups.append(clauses)
-                else:
-                    query[dimension] = {"$in": values}
+            elif (
+                dimension in _NULL_SENTINEL_FILTERS
+                and _NULL_SENTINEL_FILTERS[dimension] in values
+            ):
+                # The stored-value $in keeps the sentinel: for device,
+                # "unknown" is also a real stored value; for referrer/utm
+                # the extra literal matches nothing (and if a visitor ever
+                # sends the literal, group-by merges it with null anyway).
+                or_groups.append(
+                    [
+                        {dimension: {"$in": values}},
+                        {dimension: {"$in": [None, ""]}},
+                        {dimension: {"$exists": False}},
+                    ]
+                )
             else:
                 query[dimension] = {"$in": values}
 

shared/aggregation_strategies.py

@@ -258,7 +258,12 @@ class AggregationStrategyFactory:
     _FIELD_STRATEGIES: ClassVar[dict[str, Callable[[], AggregationStrategy]]] = {
         "browser": lambda: FieldAggregationStrategy("$browser", "browser", 20),
         "os": lambda: FieldAggregationStrategy("$os", "os", 20),
-        "device": lambda: FieldAggregationStrategy("$device", "device", 20),
+        # default matches the classifier's own fallback value so clicks
+        # recorded before device tracking existed merge into the same
+        # "unknown" bucket instead of a separate synthetic "Unknown".
+        "device": lambda: FieldAggregationStrategy(
+            "$device", "device", 20, default="unknown"
+        ),
         "country": lambda: FieldAggregationStrategy(
             "$country", "country", 50, transform_fn=convert_country_name
         ),

tests/unit/services/test_stats_service.py

@@ -530,6 +530,7 @@ def test_utm_none_sentinel_matches_missing_field(self):
             "all", OWNER_ID, None, START, NOW, {"utm_source": ["(none)"]}
         )
         assert q["$or"] == [
+            {"utm_source": {"$in": ["(none)"]}},
             {"utm_source": {"$in": [None, ""]}},
             {"utm_source": {"$exists": False}},
         ]
@@ -541,7 +542,7 @@ def test_utm_sentinel_mixed_with_values(self):
             "all", OWNER_ID, None, START, NOW, {"utm_medium": ["(none)", "email"]}
         )
         assert q["$or"] == [
-            {"utm_medium": {"$in": ["email"]}},
+            {"utm_medium": {"$in": ["(none)", "email"]}},
             {"utm_medium": {"$in": [None, ""]}},
             {"utm_medium": {"$exists": False}},
         ]
@@ -571,3 +572,40 @@ def test_device_filter_added(self):
         )
         assert q["device"] == {"$in": ["mobile", "tablet"]}
         assert "$in" not in str(q.get("meta.short_code", ""))
+
+    def test_device_unknown_matches_stored_and_missing(self):
+        """ "unknown" is BOTH a stored value (classifier fallback) and the
+        sentinel for pre-device-tracking clicks — the filter must match
+        both, or it disagrees with what group-by shows."""
+        from services.stats_service import StatsService
+
+        q = StatsService._build_click_query(
+            "all", OWNER_ID, None, START, NOW, {"device": ["unknown"]}
+        )
+        assert q["$or"] == [
+            {"device": {"$in": ["unknown"]}},
+            {"device": {"$in": [None, ""]}},
+            {"device": {"$exists": False}},
+        ]
+
+    def test_device_groupby_and_filter_agree_on_missing_docs(self):
+        """The invariant: a click doc with no device field lands in the
+        same "unknown" bucket for group-by (aggregation $ifNull default),
+        for filtering (null-sentinel map), and for new writes (classifier
+        fallback). If any of the three drifts, widget counts and filter
+        counts stop agreeing."""
+        from services.click.handlers import classify_device
+        from services.stats_service import _NULL_SENTINEL_FILTERS
+        from shared.aggregation_strategies import AggregationStrategyFactory
+
+        pipeline = AggregationStrategyFactory.get("device").build_pipeline({})
+        group_expr = pipeline[1]["$group"]["_id"]
+        assert group_expr == {"$ifNull": ["$device", "unknown"]}
+
+        from ua_parser import parse as ua_parse
+
+        classifier_fallback = classify_device(
+            ua_parse("SomeExoticClient/1.0"), "SomeExoticClient/1.0"
+        )
+        assert classifier_fallback == "unknown"
+        assert _NULL_SENTINEL_FILTERS["device"] == "unknown"

tests/unit/shared/test_aggregation_strategies.py

@@ -143,9 +143,14 @@ def test_referrer_pipeline_uses_direct_as_null_fallback():
     [
         ("browser", "Unknown"),
         ("os", "Unknown"),
-        ("device", "Unknown"),
+        # lowercase: must equal the classifier's own fallback so missing
+        # docs and classified-unknown docs share one bucket
+        ("device", "unknown"),
         ("country", "Unknown"),
         ("city", "Unknown"),
+        ("utm_source", "(none)"),
+        ("utm_medium", "(none)"),
+        ("utm_campaign", "(none)"),
     ],
 )
 def test_pipeline_uses_unknown_as_null_fallback(strategy_name, expected_null_fallback):