From 2047446546251abe4caed657e5e3ffc9da414c96 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 24 Apr 2026 22:52:04 -0700 Subject: [PATCH] Scope NULLS LAST to ttft_ms only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous version appended NULLS LAST to every ORDER BY, which would silently change DESC semantics for any nullable sort column added to the whitelist later. Today the existing sort columns (spend, total_tokens, startTime, endTime, request_duration_ms, model) are all non-null in the result set, so the clause is a no-op for them — but the broader form is misleading. Apply NULLS LAST only when sorting by ttft_ms (the only column whose computed expression actually produces NULLs). Update the model test to assert the clause is absent for non-nullable columns. --- .../spend_tracking/spend_management_endpoints.py | 16 ++++++++++------ .../test_spend_management_endpoints.py | 5 ++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/litellm/proxy/spend_tracking/spend_management_endpoints.py b/litellm/proxy/spend_tracking/spend_management_endpoints.py index b1d3ee8e358..4b8b341a4bf 100644 --- a/litellm/proxy/spend_tracking/spend_management_endpoints.py +++ b/litellm/proxy/spend_tracking/spend_management_endpoints.py @@ -2047,18 +2047,22 @@ async def ui_view_spend_logs( # noqa: PLR0915 sql_params.append(f"%{error_message}%") p += 1 - # Build the ORDER BY expression. NULLS LAST keeps rows without a - # meaningful value for the sorted column at the bottom regardless of - # direction. ttft_ms is computed from completionStartTime - startTime; - # non-streaming rows (where completionStartTime is null or equals - # endTime) yield NULL so they sort last. + # Build the ORDER BY expression. ttft_ms is computed from + # completionStartTime - startTime; non-streaming rows (where + # completionStartTime is null or equals endTime) yield NULL, so we + # append NULLS LAST in that case to keep them at the bottom regardless + # of direction. The other sort columns are non-null in the result set, + # so we leave the NULLS clause off and preserve their existing DESC + # semantics. _sql_dir = "ASC" if order_direction == "asc" else "DESC" + _nulls_clause = "" if order_column == "ttft_ms": _order_expr = ( 'CASE WHEN "completionStartTime" IS NULL ' 'OR "completionStartTime" = "endTime" THEN NULL ' 'ELSE (EXTRACT(EPOCH FROM ("completionStartTime" - "startTime")) * 1000) END' ) + _nulls_clause = " NULLS LAST" elif order_column in ("startTime", "endTime"): _order_expr = f'"{order_column}"' else: @@ -2076,7 +2080,7 @@ async def ui_view_spend_logs( # noqa: PLR0915 COALESCE(request_duration_ms, (EXTRACT(EPOCH FROM ("endTime" - "startTime")) * 1000)::INTEGER) AS request_duration_ms FROM "LiteLLM_SpendLogs" WHERE {" AND ".join(sql_conditions)} - ORDER BY {_order_expr} {_sql_dir} NULLS LAST + ORDER BY {_order_expr} {_sql_dir}{_nulls_clause} LIMIT ${p} OFFSET ${p + 1} """ sql_params.extend([page_size, skip]) diff --git a/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py b/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py index ab477151fa9..4bcabfe853a 100644 --- a/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py +++ b/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py @@ -833,7 +833,10 @@ async def test_ui_view_spend_logs_sort_by_model( async def mock_query_raw(sql_query, *params): assert "model" in sql_query - assert "NULLS LAST" in sql_query + # model is non-nullable in the schema, so NULLS LAST should NOT be + # appended — only ttft_ms gets that clause. This guards against + # accidentally widening the change to all sort columns. + assert "NULLS LAST" not in sql_query reverse = "DESC" in sql_query sorted_logs = sorted( base_logs, key=lambda x: x.get("model", ""), reverse=reverse