Skip to content

FEAT: Output-layer Refactor and CLI Results Printing Rewire - #2548

Draft
Justin Song (jsong468) wants to merge 1 commit into
microsoft:mainfrom
jsong468:output_refactor
Draft

FEAT: Output-layer Refactor and CLI Results Printing Rewire#2548
Justin Song (jsong468) wants to merge 1 commit into
microsoft:mainfrom
jsong468:output_refactor

Conversation

@jsong468

@jsong468 Justin Song (jsong468) commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Description

This branch makes the pyrit_scan / pyrit_shell scenario-results views render through the
framework's pyrit.output printers instead of a parallel CLI-owned rendering stack. It introduces a Source seam so the same printers can render from memory (notebook/library) or REST (the thin CLI client), and moves the last CLI-only renderer (the attacks table) into pyrit.output.
No user-visible output changes — pretty output is byte-for-byte the same.

  • Added a ConversationSource Protocol seam (pyrit/output/conversation/source.py) with a MemoryConversationSource implementation.
    • Rationale: Formats (pretty/markdown, later json/html) and data sources (memory/REST) are now orthogonal — a printer takes an injected source rather than a source-specific subclass, so we get F + S wiring instead of an F × S class explosion. A Protocol (structural typing) means the REST source needs no import of pyrit.output, and pyrit.output never imports pyrit.cli, so the dependency direction (and the no-cycle rule) is preserved.
  • Routed the conversation printers' score lookups through the injected source. Removed the abstract _get_scores_async hook from the conversation base; the *MemoryPrinter leaves inject MemoryConversationSource internally so their public signatures are unchanged (no notebook/helper breakage).
  • CLI conversations (and the transcript half of full) now reuse PrettyConversationPrinter via a RestApiConversationSource (pyrit/cli/_sources.py) that hydrates pyrit.models objects from the /messages view JSON and serves the inline objective score (no new endpoint).
    • Rationale: Deletes the CLI's hand-mirrored transcript renderer and its duplicated color palette — eliminating the drift risk of two code paths formatting the same data. The CLI keeps only what is genuinely CLI-specific: the per-attack header and the per-attack REST fetch loop (which also lets --limit gate network calls).
  • Moved the attacks table into pyrit.output and consolidated it into render_async on PrettyScenarioResultPrinter via a typed view parameter (ScenarioView = Literal["overview", "attacks"]), plus a notebook-friendly output_scenario_attacks_async helper.
    • Rationale: Every printer exposes a single public render entry point (render_async). The view literal mirrors the existing OutputFormat = Literal["pretty", "markdown"] precedent and matches how the CLI already models these as views of one result. Living in pyrit.output means notebooks get the same compact per-attack view, and the CLI becomes one consumer of the shared renderer.
  • Retired the CLI attacks payload stack. Removed AttackRow / AttacksTablePayload / build_attacks_table_payload from pyrit/cli/_results.py and print_attacks_table from pyrit/cli/_output.py; _results.py now holds only CLI-side flag policy (resolve_view, apply_view_limit_policy, _select_attacks).
  • Rewired the benchmark export script (build_scripts/export_adversarial_benchmark_result.py) off the deleted CLI payload — it now dumps attacks.json from a small local helper and renders attacks.txt through the framework printer with colors off.
  • Fixed a latent dead default: the attack-result pretty/markdown printers defaulted their conversation printer to the now-source-requiring base; switched them to the memory printer.

Tests and Documentation

  • New tests: tests/unit/cli/test_sources.py (REST source hydration + inline-score capture); render_async(view="attacks") cases in tests/unit/output/scenario_result/test_pretty.py (list, --limit truncation, id filtering, empty).
  • Reworked tests: CLI conversation/attacks tests point at the new print_conversations_async / output_scenario_attacks_async paths; removed the deleted-payload builder/drift tests; tests/unit/output/test_blur_images.py injects a null source instead of overriding the removed hook.
  • Suites green: tests/unit/cli + tests/unit/output/scenario_result (527 passed) and tests/unit/output (all passing); ruff and the check_async_suffix.py gate are clean.

Next Steps

  • --output json as a format-printer. Add Json*Printer siblings that call the same source hooks, assemble a structure, and serialize (model_dump_json()), starting with scenario_result (already a value object) then conversation/attack-result — retiring any remaining CLI-side JSON assembly so console and json can't diverge.
  • --output html + output-file destination. Add Html*Printer siblings (cheapest first cut: a template around the same structure the json printer assembles) that render to a file via FileSink; make --output html require --output-file (never raw terminal) and inline media via the piece media URLs.
  • Move images off the printer hook onto a source read_image_bytes_async so each format keeps its own presentation (IPython display vs. blurred file + link vs. inline <img>) — the last seam needed before html can inline media.

@jsong468
Justin Song (jsong468) marked this pull request as draft September 2, 2026 19:13
@rlundeen2

Copy link
Copy Markdown
Contributor

(GHCP Generated): Cross-linking something from #2374 (live scenario progress dashboard) that overlaps with this refactor — no action needed here, just worth considering while the output layer is already open.

The overlap

pyrit/output/scorer/pretty.py decides what of a scorer to render via hand-maintained allowlists:

_SCORER_DISPLAY_PARAMS = frozenset({"scorer_type", "score_aggregator"})
_TARGET_DISPLAY_PARAMS = frozenset({"model_name", "temperature"})

plus hardcoded child names ("prompt_target", "sub_scorers") in _render_scorer_info.

#2374 originally had the same pattern in ScenarioRunService for attack-technique display. Review feedback was that it duplicates knowledge the typed identifiers already declare, so it's now driven by the Evaluate.* markers via a new helper:

from pyrit.models import project_behavioral_identity  # pyrit/models/identifiers/identifier_projection.py

projected = project_behavioral_identity(identifier, identifier_type=ScorerIdentifier)

It honors each component's own markers (Evaluate.Exclude drops operational params, Include(fallback=...) applies declared fallbacks, Evaluate.Unwrap collapses wrapper targets like RoundRobinTarget), and deliberately ignores parent-side only_params narrowing, since that's a hashing decision rather than a claim that the child's remaining config is uninteresting.

Why it might matter for this PR

The copies have already drifted. TargetIdentifier marks model_name as Evaluate.Exclude() and declares underlying_model_name with fallback="model_name", so for the same scorer today:

CLI (pretty.py) GUI (REST, #2374)
Target name model_name → deployment name underlying_model_name → model name
top_p omitted shown
RoundRobinTarget renders nothing unwrapped to inner target

There's also a responsibility angle: output.instructions.md says the output module "does not own deciding what to render or when … format classes only turn data into strings." _TARGET_DISPLAY_PARAMS is arguably that decision living in the formatter.

If _render_scorer_info projected first and then rendered whatever came back, both constants could go away and the CLI and dashboard would stay consistent by construction as identifier types evolve.

Caveat

This would change user-visible CLI output (deployment name → model name, top_p appearing), so it's a real behavior change and may deserve its own PR rather than being folded in here. Entirely fine to punt — mainly flagging it so the duplication is a deliberate choice rather than an accident.

cc Roman Lutz (@romanlutz)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants