Skip to content

MAINT Extract scenario-history page queries from MemoryInterface #2766

Description

@romanlutz

Is your feature request related to a problem? Please describe.

MemoryInterface.get_scenario_run_history_page() combines filter normalization, cursor pagination, SQL column selection, row conversion, and aggregate lookup inside the same large class that handles messages, scores, seeds, and other persistence operations. The history-page portion is a cohesive unit that can be understood separately without changing the public API.

This is M1, the first of three small scenario-history extraction steps. It is ready for implementation. Later steps cover attempt-to-work-unit matching and aggregate counters; do not include those extractions here.

Describe the solution you'd like

Extract the history-page query and row-to-record conversion into a private, typed component in the memory package. Keep MemoryInterface.get_scenario_run_history_page() as the public entry point, with its existing signature and return shape. It should delegate page retrieval and continue using the existing aggregate method for now.

  • Move only page-specific filtering, validation, cursor handling, compact row selection, and record construction, including the directly related start-time parsing helper where appropriate.
  • Reuse the existing backend-specific expressions through a narrow internal boundary. Do not copy SQL implementations or expose a new public API.
  • Keep database access in memory. Do not move queries into the backend service or move scenario execution/presentation policy into memory.
  • Wire the extracted implementation immediately. This is not a scaffolding-only change.

Acceptance criteria:

  • Existing callers receive the same records, aggregate mapping, and has_more value.
  • Scenario/registry-name, status, and label filters preserve current normalization and validation.
  • Limits and invalid cursor IDs behave as before; timestamp ties use the existing ID tiebreaker with no skipped or duplicated rows across pages.
  • The query still selects compact columns and uses limit + 1, without hydrating full result objects or introducing per-row queries.
  • Empty pages and legacy/malformed start-time values preserve their current behavior.
  • Existing SQLite and SQL Server expression hooks remain usable; focused memory and service coverage protects the delegation boundary.

Describe alternatives you've considered, if relevant

Moving the whole scenario-history subsystem at once makes a much larger review. Extracting every conditional into a separate helper adds navigation without clarifying ownership. Prefer one cohesive internal query module that the later matching and aggregation steps can extend, not a separate service per step.

Additional context

Starting points: pyrit/memory/memory_interface.py, get_scenario_run_history_page, _parse_scenario_started_at, and the scenario-history backend expression hooks. Consumers and coverage: pyrit/backend/services/scenario_run_service.py, tests/unit/memory/memory_interface/test_interface_scenario_history.py, and tests/unit/backend/test_scenario_run_service.py.

Follow doc/code/framework.md and the applicable database, Python, and test instructions. This is a behavior-preserving extraction, not a database migration, history API redesign, or counter-policy change. The separate observation-aware LLM scoring proposal is deferred and out of scope.

Series: #2766 (history pages), #2768 (attempt-to-work-unit matching), then #2769 (aggregate queries and records).

Activity

  1. AmruthVamshi commented on Sep 22, 2026

    @AmruthVamshi
    Contributor

    I would like to take this one. Here is what I am planning to do:

    Add a small private module in pyrit/memory (something like _scenario_history_page.py) that owns the page-specific pieces currently inside get_scenario_run_history_page():

    • the limit check and the name / status / label normalization and validation
    • the keyset cursor condition (timestamp first, id as the tiebreaker)
    • the compact column select with limit + 1, including the started_at projection
    • building ScenarioHistoryRunRecord from each row, with _parse_scenario_started_at moving alongside it since that is its only caller

    MemoryInterface.get_scenario_run_history_page() keeps its signature and return shape. It builds the helper, passes it the session and the four backend hooks it already uses (_get_scenario_registry_name_condition, _get_scenario_result_labels_condition, _get_scenario_history_plan_expressions, _get_scenario_started_at_expression), and then calls get_scenario_history_aggregates exactly as it does today. No SQL is copied out of the SQLite or Azure SQL classes, and nothing new becomes public.

    For tests, the existing scenario-history and scenario_run_service tests stay green as-is. I will add focused ones on the helper for filter normalization, invalid label keys and cursor ids, has_more at the limit boundary, tie-breaking on equal timestamps across pages, and legacy / malformed started_at values. The unit-matching and aggregate pieces stay put for #2768 and #2769.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

help wantedExtra attention is needed

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions