From ea8e8a9df7a2f106d8a99dcd0d9f5ddb775eead9 Mon Sep 17 00:00:00 2001
From: Yaro Kasear <git@kasear.net>
Date: Thu, 25 Sep 2025 13:38:48 -0500
Subject: [PATCH] Another tricky bug. Now we should be good.

---
 crudkit/core/service.py | 67 +++++++++++++++++++++++++++++++++++------
 1 file changed, 58 insertions(+), 9 deletions(-)

diff --git a/crudkit/core/service.py b/crudkit/core/service.py
index 889cbcd..5732802 100644
--- a/crudkit/core/service.py
+++ b/crudkit/core/service.py
@@ -41,6 +41,45 @@ class _CRUDModelProto(_HasID, _HasTable, _HasADict, Protocol):
 
 T = TypeVar("T", bound=_CRUDModelProto)
 
+def _unwrap_ob(ob):
+    """Return (col, is_desc) from an ORDER BY element (handles .asc()/.desc())."""
+    col = getattr(ob, "element", None)
+    if col is None:
+        col = ob
+    is_desc = False
+    dir_attr = getattr(ob, "_direction", None)
+    if dir_attr is not None:
+        is_desc = (dir_attr is operators.desc_op) or (getattr(op, "name", "").upper() == "DESC")
+    elif isinstance(ob, UnaryExpression):
+        op = getattr(ob, "operator", None)
+        is_desc = (op is operators.desc_op) or (getattr(op, "name", "").upper() == "DESC")
+    return col, bool(is_desc)
+
+def _order_identity(col: ColumnElement):
+    """
+    Build a stable identity for a column suitable for deduping.
+    We ignore direction here. Duplicates are duplicates regardless of ASC/DESC.
+    """
+    table = getattr(col, "table", None)
+    table_key = getattr(table, "key", None) or id(table)
+    col_key = getattr(col, "key", None) or getattr(col, "name", None)
+    return (table_key, col_key)
+
+def _dedupe_order_by(order_by):
+    """Remove duplicate ORDER BY entries (by column identity, ignoring direction)."""
+    if not order_by:
+        return []
+    seen = set()
+    out = []
+    for ob in order_by:
+        col, _ = _unwrap_ob(ob)
+        ident = _order_identity(col)
+        if ident in seen:
+            continue
+        seen.add(ident)
+        out.append(ob)
+    return out
+
 def _hops_from_sort(params: dict | None) -> set[str]:
     """Extract first-hop relationship names from a sort spec like 'owner.first_name,-brand.name'."""
     if not params:
@@ -406,20 +445,27 @@ class CRUDService(Generic[T]):
         """
         Ensure deterministic ordering by appending PK columns as tiebreakers.
         If no order is provided, fall back to default primary-key order.
+        Never duplicates columns in ORDER BY (SQL Server requires uniqueness).
         """
         order_by = list(given_order_by or [])
         if not order_by:
-            return self._default_order_by(root_alias)
+            return _dedupe_order_by(self._default_order_by(root_alias))
+
+        order_by = _dedupe_order_by(order_by)
 
         mapper: Mapper[Any] = cast(Mapper[Any], inspect(self.model))
-        pk_cols = []
-        for col in mapper.primary_key:
-            try:
-                pk_cols.append(getattr(root_alias, col.key))
-            except AttributeError:
-                pk_cols.append(col)
+        present = {_order_identity(_unwrap_ob(ob)[0]) for ob in order_by}
 
-        return [*order_by, *pk_cols]
+        for pk in mapper.primary_key:
+            try:
+                pk_col = getattr(root_alias, pk.key)
+            except AttributeError:
+                pk_col = pk
+            if _order_identity(pk_col) not in present:
+                order_by.append(pk_col.asc())
+                present.add(_order_identity(pk_col))
+
+        return order_by
 
     def get(self, id: int, params=None) -> T | None:
         """
@@ -551,8 +597,11 @@ class CRUDService(Generic[T]):
             paginating = (limit is not None) or (offset is not None and offset != 0)
             if paginating and not order_by and self.backend.requires_order_by_for_offset:
                 order_by = self._default_order_by(root_alias)
+
+            order_by = _dedupe_order_by(order_by)
+
             if order_by:
-                query = query.order_by(*self._stable_order_by(root_alias, order_by))
+                query = query.order_by(*order_by)
 
             # Offset/limit
             if offset is not None and offset != 0: