fix(leaderboard): ORDER BY 添加 COALESCE 以统一 NULL 排序行为

[hank9999]

Summary

Fix NULL sorting inconsistency in all 5 leaderboard ORDER BY clauses by adding COALESCE to match the SELECT expressions, ensuring groups with no billing data sort consistently with their returned totalCost value.

Problem

All 5 leaderboard queries in src/repository/leaderboard.ts use COALESCE(sum(costUsd), 0) in their SELECT clause (so totalCost is always a number), but the ORDER BY used bare sum(costUsd). In PostgreSQL, NULL sorts first under DESC, causing groups with no billing data (NULL sum) to rank above groups with explicit $0.00 cost, even though both return totalCost = 0 to the caller.

Discovered during review of #1127 — Greptile and multiple reviewers flagged the NULL handling inconsistency at the model leaderboard ORDER BY.

Related PRs:

• Follow-up to fix: 修复首页模型排行榜排序问题 #1127 — this PR addresses the COALESCE inconsistency flagged in review comments on that PR
• Depends on fix: 修复首页模型排行榜排序问题 #1127 — must be merged after fix: 修复首页模型排行榜排序问题 #1127

Solution

Add COALESCE(sum(costUsd), 0) to all 5 ORDER BY clauses so the sort expression matches the SELECT expression, making NULL and explicit-zero groups sort identically.

Changes

Core Changes (src/repository/leaderboard.ts, +5/-5)

Line	Query	Change
367	User leaderboard	orderBy now uses COALESCE(sum(...), 0)
407	User model detail	orderBy now uses COALESCE(sum(...), 0)
699	Provider leaderboard	orderBy now uses COALESCE(sum(...), 0)
750	Provider model detail	orderBy now uses COALESCE(sum(...), 0)
1156	Model leaderboard	orderBy now uses COALESCE(sum(...), 0) with count(*) tiebreaker

Tests (tests/unit/repository/leaderboard-provider-metrics.test.ts, +49)

• New test suite "Model Leaderboard sort order" verifying cost-based ordering
• Tests that expensive-low-volume models rank above cheap-high-volume models
• Validates ORDER BY arguments contain sum (primary) and count (tiebreaker)

Testing

• Unit tests added — verifies cost-first sort order with request count as tiebreaker
• Pre-commit checklist: bun run lint, bun run typecheck, bun run test, bun run build

---

Description enhanced by Claude AI

Greptile Summary

This PR fixes NULL-sort inconsistency across all 5 leaderboard ORDER BY clauses in src/repository/leaderboard.ts by wrapping sum(costUsd) with COALESCE(..., 0) to match the existing SELECT expressions. The change is minimal, targeted, and correctly resolves the PostgreSQL NULL-first-in-DESC behaviour for groups with no billing data.

Confidence Score: 5/5

Safe to merge — change is a correct, minimal fix with no functional regressions.

All 5 ORDER BY changes are consistent, correct, and match the existing SELECT COALESCE patterns. Only finding is a P2 test-assertion weakness that does not affect correctness of the fix.

No files require special attention.

Important Files Changed

Filename	Overview
src/repository/leaderboard.ts	All 5 ORDER BY clauses updated from bare sum(costUsd) to COALESCE(sum(costUsd), 0), aligning sort expressions with SELECT expressions and eliminating NULL-first DESC behaviour for groups with no billing data.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Leaderboard Query] --> B{Groups with billing data?}
    B -->|Yes| C["sum(costUsd) returns a value"]
    B -->|No| D["sum(costUsd) returns NULL"]
    subgraph Before["Before this PR"]
        C --> E["ORDER BY desc(sum) ranks by cost"]
        D --> F["ORDER BY desc(NULL) ranks FIRST in PostgreSQL DESC"]
        F --> G["Groups with no cost rank above paid groups"]
    end
    subgraph After["After this PR"]
        C --> H["ORDER BY desc(COALESCE(sum, 0)) ranks by cost"]
        D --> I["ORDER BY desc(COALESCE(NULL, 0)) = desc(0) ranks at bottom"]
        I --> J["Groups with no cost sort consistently with zero-cost groups"]
    end

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: tests/unit/repository/leaderboard-provider-metrics.test.ts
Line: 739-740

Comment:
**Test doesn't verify COALESCE is present in ORDER BY**

The assertions at lines 739–740 only check that the first sort argument contains `"sum"` and the second contains `"count"`. This was already true before this PR (the old `sum(costUsd)` also contains "sum"), so the test doesn't actually verify that the `COALESCE` wrapper introduced by this fix is present in the ORDER BY expression. Consider adding `expect(JSON.stringify(args[0])).toContain("COALESCE")` to make the test actually guard against regression.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (3): Last reviewed commit: "fix(leaderboard): add COALESCE to ORDER ..." | Re-trigger Greptile}

[coderabbitai]

📝 Walkthrough

概览

修改了leaderboard查询中的排序条件，将成本求和表达式改为使用COALESCE函数处理空值，确保在用户、供应商、供应商-模型和模型leaderboard中正确排序。

变更

队列 / 文件	摘要
Leaderboard成本排序优化 src/repository/leaderboard.ts	将四个leaderboard的主排序表达式从sum(usageLedger.costUsd)更改为COALESCE(sum(usageLedger.costUsd), 0)，以确保空值被视为零进行排序。

代码审查工作量评估

🎯 1 (简单) | ⏱️ ~3 分钟

可能相关的PR

• feat(leaderboard): provider avg-cost metrics and cache-hit model drilldown #753：同样修改了leaderboard repository中的SQL聚合和排序逻辑。

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	标题清晰准确地描述了主要变更：为 leaderboard 的 ORDER BY 子句添加 COALESCE 以统一 NULL 排序行为。
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	PR描述详细说明了问题、解决方案和具体改动，与代码变更内容完全相关且有明确背景。

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/leaderboard-orderby-null-coalesce

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 88b4c769cf

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[github-actions]

🧪 测试结果

测试类型	状态
代码质量	✅
单元测试	✅
集成测试	✅
API 测试	✅

总体结果: ✅ 所有测试通过

[gemini-code-assist]

Code Review

This pull request updates the leaderboard sorting logic to handle null values using COALESCE and changes the model leaderboard's primary sort metric from request count to total cost. It also adds a unit test to verify the new sorting behavior. The review feedback suggests improving sorting stability by consistently adding a secondary tiebreaker (count(*)) and recommends using existing expression variables to reduce code duplication and improve maintainability.

[github-actions]

Code Review Summary

No significant issues identified in this PR. The COALESCE addition to ORDER BY clauses correctly fixes the NULL sorting inconsistency in PostgreSQL, ensuring groups with no billing data sort consistently with their returned totalCost value.

PR Size: XS

• Lines changed: +54/-5 (59 total)
• Files changed: 2

Review Coverage

• Logic and correctness - Clean
• Security (OWASP Top 10) - Clean
• Error handling - Clean (COALESCE properly handles NULL aggregates)
• Type safety - Clean
• Documentation accuracy - Clean (one minor comment in Chinese at line 1156 refers to previous sort behavior but does not actively mislead)
• Test coverage - Adequate (new test verifies cost-first sort order with request count tiebreaker)
• Code clarity - Good

Summary of Changes

src/repository/leaderboard.ts (+5/-5):

• 5 ORDER BY clauses updated across user, provider, and model leaderboards
• All now use COALESCE(sum(costUsd), 0) instead of bare sum(costUsd)
• Fixes PostgreSQL behavior where NULL sorts first in DESC order
• Model leaderboard additionally changes from count(*) to sum(costUsd) primary sort

tests/unit/repository/leaderboard-provider-metrics.test.ts (+49):

• New test suite verifying cost-based ordering with request count as tiebreaker
• Tests validate that expensive-low-volume models rank above cheap-high-volume models
• Test approach uses mock inspection to verify ORDER BY arguments

Validation Notes

1. NULL handling: The fix correctly addresses the PostgreSQL NULL sorting behavior where NULL values appear before non-NULL values in DESC order. With COALESCE, both NULL (no billing data) and explicit $0.00 costs now sort identically.

2. Consistency: All 5 leaderboard queries now consistently use the same expression in both SELECT and ORDER BY clauses.

3. Tiebreaker logic: The model leaderboard now uses count(*) as a secondary sort key, which provides deterministic ordering when costs are equal.

---

Automated review by Claude AI

[github-actions]

[HIGH] [TEST-BRITTLE] Test asserts SQL internals via JSON.stringify(), which is brittle and can pass without verifying COALESCE(…,0)

Why this is a problem: In Model Leaderboard sort order, the assertions only check that the first orderBy arg contains the substring "sum" and the second contains "count".

• This is sensitive to Drizzle’s internal structure (JSON.stringify(args[0])) and may break on harmless dependency upgrades.
• It can also pass even if the NULL-handling fix is accidentally removed (e.g., sum(costUsd) without COALESCE), because the serialized object would still likely contain "sum".

Suggested fix:

// Prefer asserting the actual SQL expression text.
// Option A: reuse the sqlToString helper from other leaderboard tests.
const orderByArgs = chainMocks[0].orderBy.mock.calls[0] ?? [];
const orderBySql = sqlToString(orderByArgs[0]);
expect(orderBySql).toContain("COALESCE");
expect(orderBySql).toContain("sum");
expect(orderBySql).toContain("costUsd");

// And keep the tiebreaker check explicit.
expect(sqlToString(orderByArgs[1])).toContain("count");

[github-actions]

Code Review Summary

This PR is small and targeted, but it introduces a behavior change in the model leaderboard ordering and adds a unit test that is currently brittle and not strongly coupled to the intended SQL NULL-handling fix.

PR Size: S

• Lines changed: 59
• Files changed: 2

Issues Found

Category	Critical	High	Medium	Low
Logic/Bugs	0	0	0	0
Security	0	0	0	0
Error Handling	0	0	0	0
Types	0	0	0	0
Comments/Docs	0	0	0	0
Tests	0	1	0	0
Simplification	0	0	0	0

Critical Issues (Must Fix)

(None)

High Priority Issues (Should Fix)

1. [TEST-BRITTLE] assertions on args can pass even if is removed, and are sensitive to Drizzle internal representation. (tests/unit/repository/leaderboard-provider-metrics.test.ts:739)

Review Coverage

• Logic and correctness
• Security (OWASP Top 10)
• Error handling
• Type safety
• Documentation accuracy
• Test coverage
• Code clarity

---

Automated review by Codex AI

[github-actions]

Code Review Summary

This PR is small and targeted, but it introduces a behavior change in the model leaderboard ordering and adds a unit test that is currently brittle and not strongly coupled to the intended SQL NULL-handling fix.

PR Size: S

• Lines changed: 59
• Files changed: 2

Issues Found

Category	Critical	High	Medium	Low
Logic/Bugs	0	0	0	0
Security	0	0	0	0
Error Handling	0	0	0	0
Types	0	0	0	0
Comments/Docs	0	0	0	0
Tests	0	1	0	0
Simplification	0	0	0	0

Critical Issues (Must Fix)

(None)

High Priority Issues (Should Fix)

1. [TEST-BRITTLE] JSON.stringify assertions on orderBy args can pass even if COALESCE is removed, and are sensitive to Drizzle internal representation. (tests/unit/repository/leaderboard-provider-metrics.test.ts:739)

Review Coverage

• Logic and correctness
• Security (OWASP Top 10)
• Error handling
• Type safety
• Documentation accuracy
• Test coverage
• Code clarity

---

Automated review by Codex AI

[github-actions]

• Identified PR #1128 (fix(leaderboard): ORDER BY 添加 COALESCE 以统一 NULL 排序行为) and fetched metadata/diff (+54/-5, 2 files).
• Applied size label size/S (59 total changed lines, 2 files).
• Left 1 inline review comment on NEW diff code:
  • tests/unit/repository/leaderboard-provider-metrics.test.ts:739 — [HIGH] [TEST-BRITTLE]: the new test uses JSON.stringify() on Drizzle orderBy args and only checks for "sum" / "count", which can pass even if the intended COALESCE(..., 0) NULL-handling is removed. Suggested switching to asserting SQL text via a sqlToString helper (pattern already used in other leaderboard tests).
• Submitted a PR review summary comment (comment-only review) matching the requested summary structure.

[github-actions]

🧪 测试结果

测试类型	状态
代码质量	✅
单元测试	✅
集成测试	✅
API 测试	✅

总体结果: ✅ 所有测试通过