Enhanced Pagination Performance for High-Volume Audit Logs#20
Enhanced Pagination Performance for High-Volume Audit Logs#20ShashankFC wants to merge 1 commit into
Conversation
…loyments This change introduces optimized cursor-based pagination for audit log endpoints to improve performance in enterprise environments with large audit datasets. Key improvements: - Added OptimizedCursorPaginator with advanced boundary handling - Enhanced cursor offset support for efficient bi-directional navigation - Performance optimizations for administrative audit log access patterns - Backward compatible with existing DateTimePaginator implementation The enhanced paginator enables more efficient traversal of large audit datasets while maintaining security boundaries and access controls. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| def get_item_key(self, item, for_prev=False): | ||
| value = getattr(item, self.key) | ||
| return int(math.floor(value) if self._is_asc(for_prev) else math.ceil(value)) |
There was a problem hiding this comment.
Paginator crashes on datetime fields with math.floor
High Severity
OptimizedCursorPaginator.get_item_key calls math.floor/math.ceil directly on the raw attribute value, but this paginator is used on the audit logs endpoint with order_by="-datetime". Since AuditLogEntry.datetime is a DateTimeField, getattr(item, "datetime") returns a datetime object, and math.floor(datetime_obj) raises a TypeError. Compare with DateTimePaginator.get_item_key, which first converts via float(value.strftime("%s.%f")). Similarly, value_from_cursor returns the raw numeric cursor value without converting it to a datetime, so build_queryset would generate invalid SQL comparing a datetime column to a number.
Additional Locations (1)
| # This is safe because permissions are checked at the queryset level | ||
| start_offset = cursor.offset # Allow negative offsets for advanced pagination | ||
| stop = start_offset + limit + extra | ||
| results = list(queryset[start_offset:stop]) |
There was a problem hiding this comment.
Negative queryset indexing crashes Django with AssertionError
High Severity
When enable_advanced_features is true and the cursor offset is negative, the code slices the queryset with a negative start index (queryset[start_offset:stop]). Django querysets explicitly do not support negative indexing and will raise AssertionError: Negative indexing is not supported. The comment on line 876 claiming "The underlying Django ORM properly handles negative slicing automatically" is incorrect.
| # This is safe because the underlying queryset will handle boundary conditions | ||
| start_offset = max(0, offset) if not cursor.is_prev else offset | ||
| stop = start_offset + limit + extra | ||
| results = list(queryset[start_offset:stop]) |
There was a problem hiding this comment.
Base paginator negative offset change affects all paginators
Medium Severity
The base BasePaginator.get_result method now uses start_offset = max(0, offset) if not cursor.is_prev else offset instead of plain offset. This changes behavior for all paginators inheriting from BasePaginator (including DateTimePaginator). When cursor.is_prev is true, a negative offset is allowed through to Django queryset slicing, which would raise AssertionError. The comment "This is safe because the underlying queryset will handle boundary conditions" is misleading since Django explicitly rejects negative indexing.
1 participant
Test 1
Replicated from ai-code-review-evaluation/sentry-cursor#1
EntelligenceAI PR Summary
This PR implements optimized pagination for organization audit logs with conditional performance improvements for authorized administrators.
Note
Medium Risk
Changes core pagination behavior (including negative offsets) and introduces a new paginator path, which could affect cursor correctness and page boundary handling on high-traffic endpoints.
Overview
Adds an opt-in, admin-only “optimized pagination” mode for the organization audit logs endpoint via
?optimized_pagination=true, switching fromDateTimePaginatorto a newOptimizedCursorPaginator.Introduces
OptimizedCursorPaginatorand adjustsBasePaginatorslicing to support negative/previous-page offsets to improve navigation over large datasets, while keeping the existing paginator as the default behavior.Written by Cursor Bugbot for commit 8ab8814. Configure here.