Add optional leaderboard_name filter to admin show-stats #445

msaroufim · 2026-02-11T07:36:00Z

Support filtering stats by a specific leaderboard in both the Discord command and the API endpoint. Defaults to all leaderboards when omitted.

github-actions · 2026-02-11T07:36:59Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
src/libkernelbot
leaderboard_db.py					719-721, 735, 824-848
utils.py
Project Total

_{This report was generated by python-coverage-comment-action}

Copilot

Pull request overview

Adds an optional leaderboard_name filter to admin stats retrieval so that both the Discord admin command and the /admin/stats API endpoint can return either global stats or stats scoped to a specific leaderboard.

Changes:

Extended LeaderboardDB.generate_stats(...) and internal SQL to support filtering by leaderboard_name.
Updated Discord admin command show_bot_stats and API endpoint admin_stats to accept/pass through the filter.
Added/updated tests for filtered/unfiltered behavior in DB stats and admin API.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tests/test_leaderboard_db.py	Adds assertions for filtered stats and “nonexistent leaderboard” behavior.
tests/test_admin_api.py	Updates admin stats API tests to include the new optional arg and adds a leaderboard filter test.
src/libkernelbot/leaderboard_db.py	Implements leaderboard-scoped stats by adding conditional JOIN/WHERE clauses and parameter binding.
src/kernelbot/cogs/admin_cog.py	Adds `leaderboard_name` option + autocomplete and passes it to `generate_stats`.
src/kernelbot/api/main.py	Adds `leaderboard_name` query parameter and forwards it to `generate_stats`.

Comments suppressed due to low confidence (1)

src/libkernelbot/leaderboard_db.py:1

These queries always pass params to cursor.execute(...), even when params is empty. Depending on the DB-API driver/wrappers used, it can be more idiomatic/robust to call execute(sql) when there are no parameters, and execute(sql, tuple(params)) when there are. Consider applying a small helper/wrapper here (and in the other stats queries) to avoid repeating/depending on empty-params behavior.

import dataclasses

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T07:38:55Z

src/libkernelbot/leaderboard_db.py

+    def _generate_submission_stats(self, last_day: bool = False, leaderboard_name: Optional[str] = None):
+        joins = ""
+        conditions = []
+        params = []
+
+        if leaderboard_name:
+            joins = "JOIN leaderboard.leaderboard lb ON leaderboard.submission.leaderboard_id = lb.id"
+            conditions.append("lb.name = %s")
+            params.append(leaderboard_name)
+
+        if last_day:
+            conditions.append("NOW() - submission_time <= interval '24 hours'")
+
+        where_clause = f"WHERE {' AND '.join(conditions)}" if conditions else ""
+
        self.cursor.execute(
            f"""
            SELECT
                COUNT(*),
                COUNT(*) FILTER (WHERE NOT done),
                COUNT(DISTINCT user_id)
            FROM leaderboard.submission
-            {select_expr}
+            {joins}
+            {where_clause}
            ;
-            """
+            """,
+            params,
        )


In _generate_submission_stats, several columns in the WHERE/SELECT are unqualified (submission_time, done, user_id), while the join condition references leaderboard.submission explicitly. For clarity and to avoid future ambiguity if similarly named columns are added via joins, consider aliasing leaderboard.submission (e.g., FROM leaderboard.submission s) and consistently qualifying references (s.submission_time, s.done, s.user_id, and s.leaderboard_id).

Copilot · 2026-02-11T07:38:56Z

src/libkernelbot/leaderboard_db.py

+    def _generate_runner_stats(self, last_day: bool = False, leaderboard_name: Optional[str] = None):
+        joins = ""
+        conditions = []
+        params = []
+
+        if leaderboard_name:
+            joins = "JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id"
+            conditions.append("lb.name = %s")
+            params.append(leaderboard_name)
+
+        if last_day:
+            conditions.append("NOW() - s.submission_time <= interval '24 hours'")
+
+        where_clause = f"WHERE {' AND '.join(conditions)}" if conditions else ""
+


The JOIN/conditions/params/where-clause construction is duplicated across _generate_runner_stats, _generate_submission_stats, and the heavy-hitters query in _generate_stats. To reduce drift and make future filters easier to add, consider extracting a small helper that returns (joins_sql, where_sql, params) given last_day + leaderboard_name (and any table alias differences), and reuse it in all three places.

Copilot · 2026-02-11T07:38:56Z

src/libkernelbot/leaderboard_db.py

+    def _generate_submission_stats(self, last_day: bool = False, leaderboard_name: Optional[str] = None):
+        joins = ""
+        conditions = []
+        params = []
+
+        if leaderboard_name:
+            joins = "JOIN leaderboard.leaderboard lb ON leaderboard.submission.leaderboard_id = lb.id"
+            conditions.append("lb.name = %s")
+            params.append(leaderboard_name)
+
+        if last_day:
+            conditions.append("NOW() - submission_time <= interval '24 hours'")
+
+        where_clause = f"WHERE {' AND '.join(conditions)}" if conditions else ""
+


The JOIN/conditions/params/where-clause construction is duplicated across _generate_runner_stats, _generate_submission_stats, and the heavy-hitters query in _generate_stats. To reduce drift and make future filters easier to add, consider extracting a small helper that returns (joins_sql, where_sql, params) given last_day + leaderboard_name (and any table alias differences), and reuse it in all three places.

Copilot · 2026-02-11T07:38:56Z

src/libkernelbot/leaderboard_db.py

+            joins = ""
+            conditions = ["NOW() - s.submission_time <= interval '24 hours'"]
+            params = []
+
+            if leaderboard_name:
+                joins = "JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id"
+                conditions.append("lb.name = %s")
+                params.append(leaderboard_name)
+
+            where_clause = f"WHERE {' AND '.join(conditions)}"
+


The JOIN/conditions/params/where-clause construction is duplicated across _generate_runner_stats, _generate_submission_stats, and the heavy-hitters query in _generate_stats. To reduce drift and make future filters easier to add, consider extracting a small helper that returns (joins_sql, where_sql, params) given last_day + leaderboard_name (and any table alias differences), and reuse it in all three places.

Copilot · 2026-02-11T07:38:56Z

src/kernelbot/api/main.py

    _: Annotated[None, Depends(require_admin)],
    db_context=Depends(get_db),
    last_day_only: bool = False,
+    leaderboard_name: Optional[str] = None,
 ) -> dict:


The new leaderboard_name query parameter will show up in OpenAPI, but without any description/constraints. Consider using FastAPI's Query(...) metadata (e.g., description like 'Filter stats to a specific leaderboard name') so API consumers can discover the feature from docs and generated clients.

Copilot · 2026-02-11T07:38:56Z

tests/test_admin_api.py

        )
        assert response.status_code == 200
-        mock_backend.db.generate_stats.assert_called_once_with(True)
+        mock_backend.db.generate_stats.assert_called_once_with(True, None)


These assertions couple the tests to positional argument passing (including an explicit None). If the implementation later chooses to omit the optional argument when it’s None (behavior unchanged), the first assertion would fail. Consider asserting assert_called_once() and then checking call_args more flexibly (or asserting with keyword arguments if you standardize on keyword calls).

Suggested change

mock_backend.db.generate_stats.assert_called_once_with(True, None)

mock_backend.db.generate_stats.assert_called_once()

args, kwargs = mock_backend.db.generate_stats.call_args

assert args and args[0] is True

assert len(args) == 1 or args[1] is None

Copilot · 2026-02-11T07:38:57Z

tests/test_admin_api.py

+            headers={"Authorization": "Bearer test_token"}
+        )
+        assert response.status_code == 200
+        mock_backend.db.generate_stats.assert_called_once_with(False, "my-leaderboard")


These assertions couple the tests to positional argument passing (including an explicit None). If the implementation later chooses to omit the optional argument when it’s None (behavior unchanged), the first assertion would fail. Consider asserting assert_called_once() and then checking call_args more flexibly (or asserting with keyword arguments if you standardize on keyword calls).

Deduplicate JOIN/WHERE/params construction across stats queries into a shared _stats_filter staticmethod. Alias leaderboard.submission as s in _generate_submission_stats and qualify all column references.

Add FastAPI Query metadata so leaderboard_name shows a description in OpenAPI docs. Use call_args-based assertions in tests instead of positional assert_called_once_with to decouple from argument passing style.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T07:46:15Z

tests/test_admin_api.py

-        mock_backend.db.generate_stats.assert_called_once_with(True)
+        mock_backend.db.generate_stats.assert_called_once()
+        args, kwargs = mock_backend.db.generate_stats.call_args
+        assert args[0] is True  # last_day_only


This test verifies the last_day_only parameter but doesn't check the leaderboard_name parameter. Add an assertion to verify that leaderboard_name is None or not passed when not specified in the request.

Copilot · 2026-02-11T07:46:15Z

tests/test_admin_api.py

+        assert response.status_code == 200
+        mock_backend.db.generate_stats.assert_called_once()
+        args, kwargs = mock_backend.db.generate_stats.call_args
+        assert args[1] == "my-leaderboard"  # leaderboard_name


This test should verify the last_day_only parameter value in addition to leaderboard_name. Add an assertion to ensure last_day_only defaults to False when not specified.

Copilot · 2026-02-11T07:46:15Z

src/libkernelbot/leaderboard_db.py

+
            self.cursor.execute(
-                """
+                f"""


The query string has been changed from a regular string to an f-string, but the parameterized values are still passed separately via the params list. This mixing of f-string interpolation for structure (joins, where_clause) with parameterized queries for values is correct, but ensure all user-supplied values (like leaderboard_name) continue to use parameterized queries to prevent SQL injection.

msaroufim · 2026-02-11T08:06:20Z

For you @ngc92

Add optional leaderboard_name filter to admin show-stats

ad771dc

Support filtering stats by a specific leaderboard in both the Discord command and the API endpoint. Defaults to all leaderboards when omitted.

Copilot AI review requested due to automatic review settings February 11, 2026 07:36

Copilot AI reviewed Feb 11, 2026

View reviewed changes

msaroufim added 2 commits February 10, 2026 23:40

Extract _stats_filter helper and qualify column references

1408550

Deduplicate JOIN/WHERE/params construction across stats queries into a shared _stats_filter staticmethod. Alias leaderboard.submission as s in _generate_submission_stats and qualify all column references.

Add Query description for leaderboard_name and use flexible assertions

b6d7934

Add FastAPI Query metadata so leaderboard_name shows a description in OpenAPI docs. Use call_args-based assertions in tests instead of positional assert_called_once_with to decouple from argument passing style.

msaroufim requested a review from Copilot February 11, 2026 07:45

Copilot AI reviewed Feb 11, 2026

View reviewed changes

msaroufim merged commit 2abd398 into main Feb 11, 2026
5 checks passed

Add optional leaderboard_name filter to admin show-stats #445

Add optional leaderboard_name filter to admin show-stats #445

Conversation

msaroufim commented Feb 11, 2026

Uh oh!

github-actions bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

msaroufim commented Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions bot commented Feb 11, 2026 •

edited

Loading