Add per-user submission rate limit (1 per hour)#436
Conversation
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR adds a global per-user submission rate limit (1 submission per hour) enforced at the API layer, backed by a new database query to detect recent submissions.
Changes:
- Added
LeaderboardDB.check_user_rate_limit()to fetch a user’s most recent submission within a configurable time window. - Enforced the per-user limit in
to_submit_info()by returning HTTP 429 with guidance about using the NVIDIA runner. - Added unit tests covering the DB rate-limit query and API behavior under rate-limited vs allowed conditions.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/test_leaderboard_db.py | Adds DB-level tests for the new per-user rate limit query. |
| tests/test_admin_api.py | Adds API tests asserting 429 on repeat submissions and non-429 on first submission. |
| src/libkernelbot/leaderboard_db.py | Introduces check_user_rate_limit() query used by the API to enforce the window. |
| src/kernelbot/api/api_utils.py | Enforces the per-user rate limit and returns 429 with a helpful client-facing message. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/kernelbot/api/api_utils.py
Outdated
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=500, | ||
| detail=f"Internal server error while checking rate limit: {e}", | ||
| ) from e |
There was a problem hiding this comment.
The 500-path includes the raw exception message in the HTTP response (...: {e}). This can leak internal details to clients. Prefer logging the exception server-side and returning a generic error message (optionally with a request ID) instead.
| """ | ||
| SELECT submission_time | ||
| FROM leaderboard.submission | ||
| WHERE user_id = %s | ||
| AND submission_time > NOW() - INTERVAL '%s hours' | ||
| ORDER BY submission_time DESC | ||
| LIMIT 1 | ||
| """, |
There was a problem hiding this comment.
The new rate-limit query filters on (user_id, submission_time) but there’s no supporting index in the schema migrations (currently only an index on submission(leaderboard_id)). At scale this will devolve into scanning many rows per request. Add a DB migration to create an index such as (user_id, submission_time DESC) (or (user_id, submission_time)) to keep the check fast.
src/kernelbot/api/api_utils.py
Outdated
| with db_context as db: | ||
| last_submission_time = db.check_user_rate_limit(user_id) | ||
| if last_submission_time: | ||
| raise HTTPException( | ||
| status_code=429, | ||
| detail=( | ||
| f"Rate limit exceeded. You can submit once per hour. " | ||
| f"Last submission: {last_submission_time.isoformat()}. " | ||
| f"Consider using the NVIDIA runner instead of Modal for faster iteration." | ||
| ), | ||
| ) |
There was a problem hiding this comment.
to_submit_info opens db_context twice (once for rate limiting, again for leaderboard/GPU validation). Because LeaderboardDB.__exit__ disconnects when refcount hits 0, this will connect/disconnect twice per request. Consider using a single with db_context as db: block for both checks to avoid extra connection overhead and reduce failure points.
| with db_context as db: | |
| last_submission_time = db.check_user_rate_limit(user_id) | |
| if last_submission_time: | |
| raise HTTPException( | |
| status_code=429, | |
| detail=( | |
| f"Rate limit exceeded. You can submit once per hour. " | |
| f"Last submission: {last_submission_time.isoformat()}. " | |
| f"Consider using the NVIDIA runner instead of Modal for faster iteration." | |
| ), | |
| ) | |
| last_submission_time = db_context.check_user_rate_limit(user_id) | |
| if last_submission_time: | |
| raise HTTPException( | |
| status_code=429, | |
| detail=( | |
| f"Rate limit exceeded. You can submit once per hour. " | |
| f"Last submission: {last_submission_time.isoformat()}. " | |
| f"Consider using the NVIDIA runner instead of Modal for faster iteration." | |
| ), | |
| ) |
d3c86d1 to
8017fef
Compare
Enforce a per-user rate limit of 1 submission per hour, scoped to Modal B200 GPU submissions on leaderboard ID 730 only. The 429 response suggests using the NVIDIA runner instead of Modal for faster iteration.
8017fef to
2c3fb71
Compare
Enforce a global per-user rate limit of 1 submission per hour across all leaderboards and modes. The 429 response suggests using the NVIDIA runner instead of Modal for faster iteration.