FIX: Paginate attack results in the database to keep the GUI responsive on large histories#2246
Open
varunj-msft wants to merge 1 commit into
Conversation
…nsive on large histories Listing attack results hydrated every AttackResult into memory and then deduplicated and sorted them in Python, so response time grew with the total number of stored attack results. Push that work into the database so only the requested page is materialized: - memory_interface.get_attack_results() gains keyword-only limit/offset (and min_turns/max_turns) and performs filter-aware deduplication with ROW_NUMBER() OVER (PARTITION BY conversation_id ORDER BY <recency>) in SQL, returning only the requested page instead of the full history. - Per-backend recency ordering pushed to the engine: SQLite via json_extract, Azure SQL via JSON_VALUE (ORM functions, not raw text). - The backend attack service now returns an opaque, filter-fingerprinted pagination cursor so a page token cannot be replayed against a different set of filters. Backend-only: no frontend changes and no change to the public API response shape. Covered by unit tests for memory pagination/dedup/ordering and the service cursor behavior; the paginated query was also verified against live Azure SQL.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes attack history listing by pushing per-conversation deduplication, recency ordering, and (optional) pagination into the database layer, so the GUI can remain responsive on large datasets while preserving the existing response shape.
Changes:
- Added
limit/offsetandmin_turns/max_turnssupport toMemoryInterface.get_attack_results(), with SQL-side dedup viaROW_NUMBER()and backend-specific JSON recency ordering. - Updated
AttackService.list_attacks_asyncto use offset-based pagination with an opaque cursor that includes a filter fingerprint, and to over-fetch by one row to determinehas_more. - Expanded unit test coverage for paging correctness, filter-aware dedup, turns bounds behavior, cursor behavior, and hydration under pagination.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/memory/memory_interface/test_interface_attack_results.py | Adds deterministic pagination/dedup/order tests for get_attack_results() (SQLite). |
| tests/unit/backend/test_attack_service.py | Updates service-layer tests to validate forwarding of turn bounds and new opaque cursor/offset pagination behavior. |
| pyrit/memory/sqlite_memory.py | Implements SQLite recency ordering using json_extract for metadata-based sorting. |
| pyrit/memory/memory_interface.py | Adds pagination/turn-bounds plumbing, SQL paginated query path, and shared helper for turns filtering. |
| pyrit/memory/azure_sql_memory.py | Implements Azure SQL recency ordering using JSON_VALUE (via SQLAlchemy func). |
| pyrit/backend/services/attack_service.py | Switches to DB-paginated results and introduces filter-fingerprinted opaque cursor encoding/decoding. |
| pyrit/backend/routes/attacks.py | Updates API parameter description to reflect opaque cursor semantics. |
Comment on lines
+878
to
+887
| def _norm_labels( | ||
| raw: dict[str, str | Sequence[str]] | None, | ||
| ) -> dict[str, str | list[str]] | None: | ||
| if not raw: | ||
| return None | ||
| normalized: dict[str, str | list[str]] = {} | ||
| for key in sorted(raw): | ||
| value = raw[key] | ||
| normalized[key] = value if isinstance(value, str) else sorted(str(v) for v in value) | ||
| return normalized |
Comment on lines
1075
to
+1077
| limit (int | None): Maximum number of rows to return. Defaults to None (no limit). | ||
| offset (int | None): Number of leading rows to skip, applied after ``limit`` for | ||
| pagination. Defaults to None (no offset). |
| Returns: | ||
| An opaque ``"<fingerprint>.<offset>"`` cursor string. | ||
| """ | ||
| return f"{fingerprint}.{max(0, offset)}" |
Contributor
There was a problem hiding this comment.
there's a bug here because results are sorted newest-first, so a new attack finishing between two page loads pushes everything down a slot and then the cursor just says "skip N rows," so it doesn't know that happened — you can end up seeing the same attack twice or missing one entirely. I think this would potentially happen a lot since ppl are probably paging through results as they wait for an attack to finish
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Listing attack results gets slow when the database holds a lot of records. The fix pushes the paging, dedup, and sorting into the database so only the page you're viewing is loaded:
get_attack_results()takes optionallimit/offset(plusmin_turns/max_turns), so it only loads the requested page instead of the whole history.ROW_NUMBER()instead of in Python.json_extract, Azure SQL viaJSON_VALUE.This is backend-only:
limit/offset, behavior is unchanged.Tests and Documentation
Added 13 unit tests (in tests/unit/memory/... and tests/unit/backend/test_attack_service.py) covering:
min_turns/max_turnsfilters.Verification: