-
Notifications
You must be signed in to change notification settings - Fork 22
Add user submission management API endpoints #412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add get_user_submissions() DB method to query user's submissions
- Add GET /user/submissions endpoint to list user's submissions
- Add GET /user/submissions/{id} endpoint to view submission with code
- Add DELETE /user/submissions/{id} endpoint to delete own submissions
- All endpoints verify ownership before allowing access
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds user submission management capabilities to the API, allowing authenticated users to view and manage their own submissions through new endpoints.
Changes:
- Added
get_user_submissions()database method to query submissions for a specific user with optional filtering and pagination - Added three new authenticated API endpoints: GET /user/submissions (list), GET /user/submissions/{id} (view details), and DELETE /user/submissions/{id} (delete)
- All endpoints include ownership verification to ensure users can only access their own submissions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/libkernelbot/leaderboard_db.py | Adds get_user_submissions() method to query user's submissions with optional leaderboard filtering and pagination support |
| src/kernelbot/api/main.py | Implements three new authenticated API endpoints for listing, viewing, and deleting user submissions with ownership verification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| limit: int = 50, | ||
| offset: int = 0, |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The limit and offset parameters should be validated to prevent negative values or excessively large limits that could impact database performance. Consider adding validation such as:
- Ensure limit is positive and has a reasonable maximum (e.g., limit <= 100)
- Ensure offset is non-negative
While the default limit of 50 is reasonable, users can override it with arbitrarily large values via query parameters.
| def get_user_submissions( | ||
| self, | ||
| user_id: str, | ||
| leaderboard_name: Optional[str] = None, | ||
| limit: int = 50, | ||
| offset: int = 0, | ||
| ) -> list[dict]: | ||
| """ | ||
| Get submissions for a specific user. | ||
|
|
||
| Args: | ||
| user_id: The user's ID | ||
| leaderboard_name: Optional filter by leaderboard name | ||
| limit: Maximum number of submissions to return | ||
| offset: Offset for pagination | ||
|
|
||
| Returns: | ||
| List of submission dictionaries with summary info | ||
| """ | ||
| try: | ||
| if leaderboard_name: | ||
| query = """ | ||
| SELECT | ||
| s.id, | ||
| lb.name as leaderboard_name, | ||
| s.file_name, | ||
| s.submission_time, | ||
| s.done, | ||
| r.runner as gpu_type, | ||
| r.score | ||
| FROM leaderboard.submission s | ||
| JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id | ||
| LEFT JOIN leaderboard.runs r ON r.submission_id = s.id | ||
| AND NOT r.secret AND r.passed | ||
| WHERE s.user_id = %s AND lb.name = %s | ||
| ORDER BY s.submission_time DESC | ||
| LIMIT %s OFFSET %s | ||
| """ | ||
| self.cursor.execute(query, (user_id, leaderboard_name, limit, offset)) | ||
| else: | ||
| query = """ | ||
| SELECT | ||
| s.id, | ||
| lb.name as leaderboard_name, | ||
| s.file_name, | ||
| s.submission_time, | ||
| s.done, | ||
| r.runner as gpu_type, | ||
| r.score | ||
| FROM leaderboard.submission s | ||
| JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id | ||
| LEFT JOIN leaderboard.runs r ON r.submission_id = s.id | ||
| AND NOT r.secret AND r.passed | ||
| WHERE s.user_id = %s | ||
| ORDER BY s.submission_time DESC | ||
| LIMIT %s OFFSET %s | ||
| """ | ||
| self.cursor.execute(query, (user_id, limit, offset)) | ||
|
|
||
| results = [] | ||
| for row in self.cursor.fetchall(): | ||
| results.append({ | ||
| "id": row[0], | ||
| "leaderboard_name": row[1], | ||
| "file_name": row[2], | ||
| "submission_time": row[3], | ||
| "done": row[4], | ||
| "gpu_type": row[5], | ||
| "score": row[6], | ||
| }) | ||
| return results | ||
| except psycopg2.Error as e: | ||
| self.connection.rollback() | ||
| logger.exception("Error fetching user submissions for user %s", user_id, exc_info=e) | ||
| raise KernelBotError("Error fetching user submissions") from e | ||
|
|
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new database method lacks test coverage. Based on the existing test structure in tests/test_leaderboard_db.py, you should add tests that verify:
- Submissions are returned only for the specified user_id
- The leaderboard_name filter works correctly when provided
- Pagination with limit and offset works as expected
- The method handles users with no submissions
- Once the duplication bug is fixed, verify that submissions with multiple runs are not duplicated
| @app.get("/user/submissions") | ||
| async def get_user_submissions( | ||
| user_info: Annotated[dict, Depends(validate_user_header)], | ||
| leaderboard: Optional[str] = None, | ||
| limit: int = 50, | ||
| offset: int = 0, | ||
| db_context=Depends(get_db), | ||
| ) -> list[dict]: | ||
| """Get the authenticated user's submissions. | ||
|
|
||
| Args: | ||
| leaderboard: Optional filter by leaderboard name | ||
| limit: Maximum number of submissions to return (default 50) | ||
| offset: Offset for pagination | ||
|
|
||
| Returns: | ||
| List of user's submissions with summary info | ||
| """ | ||
| await simple_rate_limit() | ||
| try: | ||
| with db_context as db: | ||
| return db.get_user_submissions( | ||
| user_id=user_info["user_id"], | ||
| leaderboard_name=leaderboard, | ||
| limit=limit, | ||
| offset=offset, | ||
| ) | ||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=f"Error fetching user submissions: {e}") from e | ||
|
|
||
|
|
||
| @app.get("/user/submissions/{submission_id}") | ||
| async def get_user_submission( | ||
| submission_id: int, | ||
| user_info: Annotated[dict, Depends(validate_user_header)], | ||
| db_context=Depends(get_db), | ||
| ) -> dict: | ||
| """Get a specific submission by ID. Only the owner can view their submission. | ||
|
|
||
| Args: | ||
| submission_id: The submission ID to retrieve | ||
|
|
||
| Returns: | ||
| Full submission details including code | ||
| """ | ||
| await simple_rate_limit() | ||
| try: | ||
| with db_context as db: | ||
| submission = db.get_submission_by_id(submission_id) | ||
|
|
||
| if submission is None: | ||
| raise HTTPException(status_code=404, detail="Submission not found") | ||
|
|
||
| # Verify ownership | ||
| if str(submission.user_id) != str(user_info["user_id"]): | ||
| raise HTTPException(status_code=403, detail="Not authorized to view this submission") | ||
|
|
||
| return { | ||
| "id": submission.submission_id, | ||
| "leaderboard_id": submission.leaderboard_id, | ||
| "leaderboard_name": submission.leaderboard_name, | ||
| "file_name": submission.file_name, | ||
| "user_id": submission.user_id, | ||
| "submission_time": submission.submission_time, | ||
| "done": submission.done, | ||
| "code": submission.code, | ||
| "runs": [ | ||
| { | ||
| "start_time": r.start_time, | ||
| "end_time": r.end_time, | ||
| "mode": r.mode, | ||
| "secret": r.secret, | ||
| "runner": r.runner, | ||
| "score": r.score, | ||
| "passed": r.passed, | ||
| } | ||
| for r in submission.runs | ||
| ], | ||
| } | ||
| except HTTPException: | ||
| raise | ||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=f"Error fetching submission: {e}") from e | ||
|
|
||
|
|
||
| @app.delete("/user/submissions/{submission_id}") | ||
| async def delete_user_submission( | ||
| submission_id: int, | ||
| user_info: Annotated[dict, Depends(validate_user_header)], | ||
| db_context=Depends(get_db), | ||
| ) -> dict: | ||
| """Delete a submission by ID. Only the owner can delete their submission. | ||
|
|
||
| Args: | ||
| submission_id: The submission ID to delete | ||
|
|
||
| Returns: | ||
| Success message | ||
| """ | ||
| await simple_rate_limit() | ||
| try: | ||
| with db_context as db: | ||
| submission = db.get_submission_by_id(submission_id) | ||
|
|
||
| if submission is None: | ||
| raise HTTPException(status_code=404, detail="Submission not found") | ||
|
|
||
| # Verify ownership | ||
| if str(submission.user_id) != str(user_info["user_id"]): | ||
| raise HTTPException(status_code=403, detail="Not authorized to delete this submission") | ||
|
|
||
| db.delete_submission(submission_id) | ||
|
|
||
| return {"message": f"Submission {submission_id} deleted successfully"} | ||
| except HTTPException: | ||
| raise | ||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=f"Error deleting submission: {e}") from e |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These new API endpoints lack test coverage. Based on the existing test structure in tests/test_admin_api.py, you should add tests that verify:
- Endpoints require authentication (missing or invalid auth headers return 401)
- GET /user/submissions returns submissions for the authenticated user
- GET /user/submissions respects the leaderboard filter
- GET /user/submissions/{id} returns submission details for owned submissions
- GET /user/submissions/{id} returns 403 when attempting to view someone else's submission
- GET /user/submissions/{id} returns 404 for non-existent submissions
- DELETE /user/submissions/{id} successfully deletes owned submissions
- DELETE /user/submissions/{id} returns 403 when attempting to delete someone else's submission
- Rate limiting is applied (via simple_rate_limit)
|
|
||
| def get_user_submissions( | ||
| self, | ||
| user_id: str, |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type annotation for user_id parameter is inconsistent with the actual data type. The user_id is returned as an integer from validate_identity (line 263 in leaderboard_db.py returns row[0] from the database id column), but this method signature declares it as str.
This should be changed to user_id: int to match the actual type and be consistent with other methods in this class like create_submission (line 274) which also uses user_id: int.
| user_id: str, | |
| user_id: int, |
src/kernelbot/api/main.py
Outdated
|
|
||
| db.delete_submission(submission_id) | ||
|
|
||
| return {"message": f"Submission {submission_id} deleted successfully"} |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency with admin endpoints in this codebase, consider using the response format: {"status": "ok", "submission_id": submission_id} instead of {"message": "..."}.
See the admin_delete_submission endpoint (line 581) which returns {"status": "ok", "submission_id": submission_id} for a similar delete operation.
src/libkernelbot/leaderboard_db.py
Outdated
| s.id, | ||
| lb.name as leaderboard_name, | ||
| s.file_name, | ||
| s.submission_time, | ||
| s.done, | ||
| r.runner as gpu_type, | ||
| r.score | ||
| FROM leaderboard.submission s | ||
| JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id | ||
| LEFT JOIN leaderboard.runs r ON r.submission_id = s.id | ||
| AND NOT r.secret AND r.passed | ||
| WHERE s.user_id = %s AND lb.name = %s | ||
| ORDER BY s.submission_time DESC | ||
| LIMIT %s OFFSET %s | ||
| """ | ||
| self.cursor.execute(query, (user_id, leaderboard_name, limit, offset)) | ||
| else: | ||
| query = """ | ||
| SELECT | ||
| s.id, | ||
| lb.name as leaderboard_name, | ||
| s.file_name, | ||
| s.submission_time, | ||
| s.done, | ||
| r.runner as gpu_type, | ||
| r.score | ||
| FROM leaderboard.submission s | ||
| JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id | ||
| LEFT JOIN leaderboard.runs r ON r.submission_id = s.id | ||
| AND NOT r.secret AND r.passed | ||
| WHERE s.user_id = %s | ||
| ORDER BY s.submission_time DESC |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LEFT JOIN with the runs table without any DISTINCT or aggregation will produce duplicate rows when a submission has multiple runs. For example, if a submission was run on multiple GPU types or had multiple runs with different statuses, each run will create a separate row in the result set, causing the same submission to appear multiple times in the returned list.
To fix this, you should either:
- Use DISTINCT ON (s.id) to get one row per submission with an ORDER BY clause to determine which run to select
- Use aggregation (e.g., MAX or MIN) to select specific run values per submission
- Consider if you want to return all runs per submission and structure the result differently
Similar queries in this codebase use DISTINCT ON to handle this (see line 655 for an example).
| s.id, | |
| lb.name as leaderboard_name, | |
| s.file_name, | |
| s.submission_time, | |
| s.done, | |
| r.runner as gpu_type, | |
| r.score | |
| FROM leaderboard.submission s | |
| JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id | |
| LEFT JOIN leaderboard.runs r ON r.submission_id = s.id | |
| AND NOT r.secret AND r.passed | |
| WHERE s.user_id = %s AND lb.name = %s | |
| ORDER BY s.submission_time DESC | |
| LIMIT %s OFFSET %s | |
| """ | |
| self.cursor.execute(query, (user_id, leaderboard_name, limit, offset)) | |
| else: | |
| query = """ | |
| SELECT | |
| s.id, | |
| lb.name as leaderboard_name, | |
| s.file_name, | |
| s.submission_time, | |
| s.done, | |
| r.runner as gpu_type, | |
| r.score | |
| FROM leaderboard.submission s | |
| JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id | |
| LEFT JOIN leaderboard.runs r ON r.submission_id = s.id | |
| AND NOT r.secret AND r.passed | |
| WHERE s.user_id = %s | |
| ORDER BY s.submission_time DESC | |
| * | |
| FROM ( | |
| SELECT DISTINCT ON (s.id) | |
| s.id, | |
| lb.name as leaderboard_name, | |
| s.file_name, | |
| s.submission_time, | |
| s.done, | |
| r.runner as gpu_type, | |
| r.score | |
| FROM leaderboard.submission s | |
| JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id | |
| LEFT JOIN leaderboard.runs r ON r.submission_id = s.id | |
| AND NOT r.secret AND r.passed | |
| WHERE s.user_id = %s AND lb.name = %s | |
| ORDER BY s.id, r.score DESC NULLS LAST | |
| ) sub | |
| ORDER BY submission_time DESC | |
| LIMIT %s OFFSET %s | |
| """ | |
| self.cursor.execute(query, (user_id, leaderboard_name, limit, offset)) | |
| else: | |
| query = """ | |
| SELECT | |
| * | |
| FROM ( | |
| SELECT DISTINCT ON (s.id) | |
| s.id, | |
| lb.name as leaderboard_name, | |
| s.file_name, | |
| s.submission_time, | |
| s.done, | |
| r.runner as gpu_type, | |
| r.score | |
| FROM leaderboard.submission s | |
| JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id | |
| LEFT JOIN leaderboard.runs r ON r.submission_id = s.id | |
| AND NOT r.secret AND r.passed | |
| WHERE s.user_id = %s | |
| ORDER BY s.id, r.score DESC NULLS LAST | |
| ) sub | |
| ORDER BY submission_time DESC |
src/libkernelbot/leaderboard_db.py
Outdated
| query = """ | ||
| SELECT | ||
| s.id, | ||
| lb.name as leaderboard_name, | ||
| s.file_name, | ||
| s.submission_time, | ||
| s.done, | ||
| r.runner as gpu_type, | ||
| r.score | ||
| FROM leaderboard.submission s | ||
| JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id | ||
| LEFT JOIN leaderboard.runs r ON r.submission_id = s.id | ||
| AND NOT r.secret AND r.passed | ||
| WHERE s.user_id = %s | ||
| ORDER BY s.submission_time DESC | ||
| LIMIT %s OFFSET %s | ||
| """ |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LEFT JOIN with the runs table without any DISTINCT or aggregation will produce duplicate rows when a submission has multiple runs. For example, if a submission was run on multiple GPU types or had multiple runs with different statuses, each run will create a separate row in the result set, causing the same submission to appear multiple times in the returned list.
To fix this, you should either:
- Use DISTINCT ON (s.id) to get one row per submission with an ORDER BY clause to determine which run to select
- Use aggregation (e.g., MAX or MIN) to select specific run values per submission
- Consider if you want to return all runs per submission and structure the result differently
Similar queries in this codebase use DISTINCT ON to handle this (see line 655 for an example).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is wrong, we shouldn't just fetch one run of a given submission, that would be unexpected. Instead, we probably want the code below to restructure result to be a dict, with an item "runs" that is a list of (gpu_type, score) tuples?
- Fix SQL duplication bug with DISTINCT ON for get_user_submissions
- Add input validation for limit (1-100) and offset (>=0)
- Use consistent response format {"status": "ok", "submission_id": ...}
- Add unit tests for get_user_submissions DB method
- Update CLAUDE.md with comprehensive testing guidance
- Use dict-style access (submission["key"]) instead of attribute access
(submission.key) since SubmissionItem is a TypedDict
- Change wget to curl in problem_sync.py for better cross-platform support
Fixes bug where GET/DELETE /user/submissions/{id} endpoints returned
500 error with "'dict' object has no attribute 'user_id'"
Document the full workflow for testing CLI against local API server: - PostgreSQL setup and migrations - Creating test users in database - Starting API server with --api-only - Syncing leaderboards from reference-kernels - Configuring CLI for local testing - Running CLI commands and curl examples - Troubleshooting common errors
src/kernelbot/api/main.py
Outdated
| "passed": r["passed"], | ||
| } | ||
| for r in submission["runs"] | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to_dict utility function?
src/libkernelbot/leaderboard_db.py
Outdated
| JOIN leaderboard.leaderboard lb ON s.leaderboard_id = lb.id | ||
| LEFT JOIN leaderboard.runs r ON r.submission_id = s.id | ||
| AND NOT r.secret AND r.passed | ||
| WHERE s.user_id = %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unify with the string above, the only difference is the WHERE part?
- Unified SQL queries in get_user_submissions() by building WHERE clause conditionally instead of duplicating the entire query - Changed response structure to return all runs per submission as a list with (gpu_type, score) tuples instead of using DISTINCT ON for one run - Simplified runs serialization in get_user_submission endpoint using dict comprehension with explicit field selection - Added test for multiple runs per submission
Should be reviewed in conjunction with gpu-mode/popcorn-cli#32
Fixes #364