Enhanced Pagination Performance for High-Volume Audit Logs

[zaibkhan]

This PR is part of the AI Code Review Benchmark dataset.

It reintroduces a known bug related to cursor-based pagination in Sentry’s audit log endpoints.

Key points:

• OptimizedCursorPaginator added with boundary handling
• Enhanced cursor offset for bi-directional navigation
• Performance optimizations for audit log access
• Backward compatibility with DateTimePaginator

Benchmark reference:
Replicated from ai-code-review-evaluation/sentry-greptile PR #1

[codoki-pr-intelligence]

Codoki PR Review

Summary: Fix negative slicing, handle datetime keys, ensure Django-compat
What’s good: Clear intent to improve audit log pagination performance with optional optimized path and backward compatibility to DateTimePaginator.
Review Status: ❌ Requires changes
Overall Priority: High

Issues (Critical & High only)

Severity	Issue	Why it matters	File
High	Correctness — Negative QuerySet slice start causes Django runtime error	Allowing negative start offsets on a Django QuerySet slice is not supported and will raise at runtime (negative indexing is unsupported). This breaks prev-page navigation paths when offset < 0.	…/api/paginator.py
High	Correctness — Datetime key handling in get_item_key raises TypeError	get_item_key assumes a numeric key and applies floor/ceil, but this paginator is used with order_by on a datetime field; calling floor/ceil on a datetime raises TypeError and breaks pagination for audit logs ordered by '-datetime'.	…/api/paginator.py
High	Correctness — Negative slicing in optimized paginator breaks on Django	Negative QuerySet slice indices are not supported by Django and will raise. This path will fail when cursor.offset < 0, defeating the 'advanced features' intent and breaking pagination.	…/api/paginator.py

Key Feedback (click to expand)

• Needs improvement: Avoid introducing features that contradict Django ORM guarantees (negative slice indices) and ensure key handling matches the field type (datetime).
• Testing: Add tests for: (1) prev/next pagination paths when offset < 0 to confirm clamping behavior and absence of ORM exceptions, and (2) using OptimizedCursorPaginator with order_by on datetime to verify key extraction and stable cursors.
• Documentation: Inline comments claim negative offsets are safe with Django ORM; this is misleading. Update comments to reflect the ORM limitation and intended clamping behavior.
• Compatibility: Django QuerySet slicing does not support negative indices; using them can raise errors at runtime. Adhering to non-negative offsets maintains ORM compatibility across versions and backends.
• Performance: Clamping offsets to >= 0 preserves performance benefits without risking ORM errors; the current negative-offset branches would not execute successfully, negating intended perf gains.
• Open questions: Is OptimizedCursorPaginator intended to support datetime-ordered querysets? If so, should it reuse DateTimePaginator.get_item_key logic instead of numeric-only rounding?

Confidence: 2/5 — Not ready to merge (3 high · status: Requires changes)

React with 👍 or 👎 if you found this review useful.

[codoki-pr-intelligence]

⚠️ High: Allowing negative start offsets on a Django QuerySet slice is not supported and will raise at runtime (negative indexing is unsupported). This breaks prev-page navigation paths when offset < 0.

Suggested change

	if cursor.is_prev and cursor.value:
	```suggestion
	start_offset = max(0, offset)

[codoki-pr-intelligence]

⚠️ High: get_item_key assumes a numeric key and applies floor/ceil, but this paginator is used with order_by on a datetime field; calling floor/ceil on a datetime raises TypeError and breaks pagination for audit logs ordered by '-datetime'.

Suggested change

	def value_from_cursor(self, cursor):
	```suggestion
	return int((math.floor(value.timestamp() if isinstance(value, datetime) else value)) if self._is_asc(for_prev) else (math.ceil(value.timestamp() if isinstance(value, datetime) else value)))

[codoki-pr-intelligence]

⚠️ High: Negative QuerySet slice indices are not supported by Django and will raise. This path will fail when cursor.offset < 0, defeating the 'advanced features' intent and breaking pagination.

Suggested change

	extra += 1
	```suggestion
	start_offset = max(0, cursor.offset)