internal: better device classification via UA parsing

Signed-off-by: NotAShelf <raf@notashelf.dev>
Change-Id: I6c78f1eebe71ef4cf037ebbda2caaeb36a6a6964

internal/api/handler.go

@@ -137,7 +137,7 @@ func (h *IngestionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 		// Device classification
 		if h.cfg.Site.Collect.Device {
-			device = h.classifyDevice(event.Width)
+			device = h.classifyDevice(event.Width, userAgent)
 		}
 
 		// Referrer classification
@@ -271,19 +271,43 @@ func (h *IngestionHandler) ipInCIDR(ip, cidr string) bool {
 	return network.Contains(testIP)
 }
 
-// Classifies screen width into device categories using configured breakpoints
+// Classifies device using both screen width and User-Agent parsing
-// FIXME: we need a more robust mechanism for classifying devices. Breakpoints
+// Uses UA hints for better detection, falls back to width breakpoints
-// are the only ones I can think of *right now* but I'm positive there are better
+func (h *IngestionHandler) classifyDevice(width int, userAgent string) string {
-// mechanisns. We'll get to this later.
+	// First try User-Agent based detection for better accuracy
-func (h *IngestionHandler) classifyDevice(width int) string {
+	ua := strings.ToLower(userAgent)
-	if width == 0 {
+
-		return "unknown"
+	// Tablet detection via UA (must come before mobile: Android tablets lack "mobile" keyword)
-	}
+	if strings.Contains(ua, "tablet") ||
-	if width < h.cfg.Limits.DeviceBreakpoints.Mobile {
+		strings.Contains(ua, "ipad") ||
-		return "mobile"
+		(strings.Contains(ua, "android") && !strings.Contains(ua, "mobile")) {
-	}
-	if width < h.cfg.Limits.DeviceBreakpoints.Tablet {
 		return "tablet"
 	}
-	return "desktop"
+
+	// Mobile detection via UA
+	if strings.Contains(ua, "mobile") ||
+		strings.Contains(ua, "iphone") ||
+		strings.Contains(ua, "ipod") ||
+		strings.Contains(ua, "windows phone") ||
+		strings.Contains(ua, "blackberry") {
+		return "mobile"
+	}
+
+	// If UA doesn't provide clear signal, use width breakpoints
+	if width > 0 {
+		if width < h.cfg.Limits.DeviceBreakpoints.Mobile {
+			return "mobile"
+		}
+		if width < h.cfg.Limits.DeviceBreakpoints.Tablet {
+			return "tablet"
+		}
+		return "desktop"
+	}
+
+	// Default to desktop if UA suggests desktop browser
+	if userAgent != "" {
+		return "desktop"
+	}
+
+	return "unknown"
 }

internal/api/handler_test.go

@@ -2,7 +2,6 @@ package api
 
 import (
 	"bytes"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -207,51 +206,113 @@ func TestIngestionHandler_InvalidJSON(t *testing.T) {
 	}
 }
 
-func TestIngestionHandler_DeviceClassification(t *testing.T) {
+func newTestHandler(cfg *config.Config) *IngestionHandler {
-	cfg := config.Config{
-		Site: config.SiteConfig{
-			Domains: []string{"example.com"},
-			Collect: config.CollectConfig{
-				Pageviews: true,
-				Device:    true,
-			},
-			Path: config.PathConfig{},
-		},
-		Limits: config.LimitsConfig{
-			MaxPaths:   100,
-			MaxSources: 50,
-		},
-	}
-
 	pathNorm := normalize.NewPathNormalizer(cfg.Site.Path)
 	pathRegistry := aggregate.NewPathRegistry(cfg.Limits.MaxPaths)
 	refRegistry := normalize.NewReferrerRegistry(cfg.Limits.MaxSources)
 	metricsAgg := aggregate.NewMetricsAggregator(
 		pathRegistry,
 		aggregate.NewCustomEventRegistry(100),
-		&cfg,
+		cfg,
 	)
+	return NewIngestionHandler(cfg, pathNorm, pathRegistry, refRegistry, metricsAgg)
+}
 
-	handler := NewIngestionHandler(&cfg, pathNorm, pathRegistry, refRegistry, metricsAgg)
+func TestClassifyDevice_UA(t *testing.T) {
+	cfg := &config.Config{
+		Limits: config.LimitsConfig{
+			DeviceBreakpoints: config.DeviceBreaks{
+				Mobile: 768,
+				Tablet: 1024,
+			},
+		},
+	}
+	h := newTestHandler(cfg)
 
 	tests := []struct {
-		name  string
+		name      string
-		width int
+		width     int
+		userAgent string
+		want      string
 	}{
-		{"mobile", 375},
+		// UA takes priority
-		{"tablet", 768},
+		{
-		{"desktop", 1920},
+			name:      "iphone via UA",
+			width:     390,
+			userAgent: "Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15",
+			want:      "mobile",
+		},
+		{
+			name:      "android phone via UA",
+			width:     0,
+			userAgent: "Mozilla/5.0 (Linux; Android 13; Pixel 7) Mobile Safari/537.36",
+			want:      "mobile",
+		},
+		{
+			name:      "windows phone via UA",
+			width:     0,
+			userAgent: "Mozilla/5.0 (compatible; MSIE 10.0; Windows Phone 8.0)",
+			want:      "mobile",
+		},
+		{
+			name:      "ipad via UA",
+			width:     1024,
+			userAgent: "Mozilla/5.0 (iPad; CPU OS 17_0 like Mac OS X) AppleWebKit/605.1.15",
+			want:      "tablet",
+		},
+		{
+			name:      "android tablet via UA (no mobile keyword)",
+			width:     0,
+			userAgent: "Mozilla/5.0 (Linux; Android 13; SM-T870) AppleWebKit/537.36",
+			want:      "tablet",
+		},
+		// Falls back to width when UA is desktop
+		{
+			name:      "desktop UA wide screen",
+			width:     1920,
+			userAgent: "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 Chrome/120.0",
+			want:      "desktop",
+		},
+		{
+			name:      "desktop UA narrow width",
+			width:     500,
+			userAgent: "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 Chrome/120.0",
+			want:      "mobile",
+		},
+		// Width-only fallback
+		{
+			name:      "no UA mobile width",
+			width:     375,
+			userAgent: "",
+			want:      "mobile",
+		},
+		{
+			name:      "no UA tablet width",
+			width:     800,
+			userAgent: "",
+			want:      "tablet",
+		},
+		{
+			name:      "no UA desktop width",
+			width:     1440,
+			userAgent: "",
+			want:      "desktop",
+		},
+		// Unknown
+		{
+			name:      "no UA no width",
+			width:     0,
+			userAgent: "",
+			want:      "unknown",
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			body := fmt.Sprintf(`{"d":"example.com","p":"/test","w":%d}`, tt.width)
+			got := h.classifyDevice(tt.width, tt.userAgent)
-			req := httptest.NewRequest("POST", "/api/event", bytes.NewBufferString(body))
+			if got != tt.want {
-			w := httptest.NewRecorder()
+				t.Errorf("classifyDevice(%d, %q) = %q, want %q",
-			handler.ServeHTTP(w, req)
+					tt.width, tt.userAgent, got, tt.want)
-
-			if w.Code != http.StatusNoContent {
-				t.Errorf("expected status %d, got %d", http.StatusNoContent, w.Code)
 			}
 		})
 	}