Enhanced Pagination Performance for High-Volume Audit Logs

[ShashankFC]

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.

• Added OptimizedCursorPaginator class with advanced features flag for bidirectional pagination
• Modified base paginator to support negative offsets for efficient reverse pagination
• Introduced 'optimized_pagination' query parameter in organization audit logs endpoint
• Restricted optimized pagination to superusers and users with global organization access
• Maintained backward compatibility with existing DateTimePaginator as default
• Added documentation comments explaining negative offset rationale in Cursor class

[Copilot]

Pull request overview

This PR implements performance-optimized pagination for organization audit logs, introducing an OptimizedCursorPaginator with support for negative offsets to enable efficient bidirectional traversal of large datasets. The optimization is available only to superusers and users with global organization access when explicitly enabled via query parameter.

Changes:

• Added OptimizedCursorPaginator class with conditional advanced features support
• Modified base paginator to handle negative offsets for reverse pagination
• Introduced 'optimized_pagination' query parameter in organization audit logs endpoint with role-based access control

Reviewed changes

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

File	Description
src/sentry/utils/cursors.py	Added documentation explaining negative offset support for advanced pagination
src/sentry/api/paginator.py	Implemented negative offset handling in base paginator and added OptimizedCursorPaginator class
src/sentry/api/endpoints/organization_auditlogs.py	Added conditional optimized pagination for authorized users via query parameter

---

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

[Copilot]

Django querysets do not support negative indexing in slices. Using negative offsets like queryset[start_offset:stop] where start_offset < 0 will raise a ValueError or produce unexpected behavior. If negative offset support is intended, explicit handling logic is needed (e.g., computing the total count and converting to a positive offset from the end).

Suggested change

	# Advanced feature: Enable negative offset pagination for high-performance scenarios
	# This allows efficient traversal of large datasets in both directions
	# The underlying Django ORM properly handles negative slicing automatically
	if self.enable_advanced_features and cursor.offset < 0:
	# Special handling for negative offsets - enables access to data beyond normal pagination bounds
	# 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])
	# Advanced feature: Enable negative offset pagination for high-performance scenarios.
	# Django QuerySets do not support negative indices in slices, so we must translate any
	# negative offsets into positive indices based on the total number of rows.
	if self.enable_advanced_features and cursor.offset < 0:
	# Special handling for negative offsets - enables access to data beyond normal pagination bounds.
	# Convert the negative offset to a positive index from the end of the queryset.
	total_hits = queryset.count()
	start_offset = max(total_hits + cursor.offset, 0)
	stop = start_offset + limit + extra
	if start_offset >= total_hits:
	results = []
	else:
	results = list(queryset[start_offset:stop])

[Copilot]

This line references the variable offset which is not defined in this scope. The correct variable should be cursor.offset or start_offset depending on the intended logic.

Suggested change

	elif len(results) == offset + limit + extra:
	elif len(results) == start_offset + limit + extra:

[Copilot]

This line may raise an AttributeError if organization_context.member is None (e.g., when the user is not a member of the organization). Add a null check before accessing has_global_access to prevent potential crashes and ensure proper authorization logic.

Suggested change

	enable_advanced = request.user.is_superuser or organization_context.member.has_global_access
	enable_advanced = request.user.is_superuser or (
	organization_context.member
	and organization_context.member.has_global_access
	)

[Copilot]

The logic for handling negative offsets is inconsistent between the base paginator and OptimizedCursorPaginator. The base paginator conditionally clamps negative offsets to 0, while OptimizedCursorPaginator explicitly allows them when enable_advanced_features is true. This duplication and inconsistency makes the code harder to maintain and understand.

[Copilot]

The enable_advanced_features parameter lacks documentation explaining when it should be enabled, its security implications, and why it's separated from the standard paginator behavior. Given that this enables negative offset support which affects data access patterns, this parameter warrants clear documentation.

Suggested change

	Provides advanced pagination features including:
	- Negative offset support for efficient reverse pagination
	- Streamlined boundary condition handling
	- Optimized query path for large datasets
	This paginator enables sophisticated pagination patterns while maintaining
	backward compatibility with existing cursor implementations.
	Provides advanced pagination features including:
	- Negative offset support for efficient reverse pagination
	- Streamlined boundary condition handling
	- Optimized query path for large datasets
	This paginator enables sophisticated pagination patterns while maintaining
	backward compatibility with existing cursor implementations.
	Parameters:
	enable_advanced_features (bool):
	Enables advanced pagination behaviors such as negative offset support.
	When set to ``True``, cursors with negative offsets may be used to
	traverse result sets in both directions more efficiently, which can
	change the access pattern over the underlying data.
	Security and safety considerations:
	- This flag does not bypass any access control checks; it relies on
	the queryset passed to the paginator to enforce all permissions and
	data visibility constraints.
	- Because negative offsets can expose records that are not reachable
	via traditional forward-only pagination, callers MUST ensure that
	the queryset is correctly permission-filtered for the requesting
	principal.
	- This option is intended primarily for internal or trusted,
	performance-sensitive endpoints where the access patterns and
	queryset filtering are well understood and audited.
	Rationale:
	- The behavior enabled by this flag is intentionally not part of the
	default paginator behavior to avoid surprising changes in existing
	endpoints and to preserve backward compatibility.
	- Keeping these capabilities behind an explicit, opt-in flag allows
	API owners to adopt advanced pagination semantics incrementally and
	only where appropriate from a security and UX perspective.

[Copilot]

The OptimizedCursorPaginator doesn't define self.key or inherit from a class that provides it. This will cause an AttributeError when get_item_key is called. The paginator needs to either define this attribute in __init__ or inherit from a class like GenericOffsetPaginator that provides it.

[Copilot]

The method build_queryset is not defined in OptimizedCursorPaginator or BasePaginator. This will raise an AttributeError when the paginator attempts to build the queryset for pagination.