Skip to content

Scorer composition (PR B): TrueFalse pure-composition API + CallableResponseHandler#2146

Open
romanlutz wants to merge 15 commits into
microsoft:mainfrom
romanlutz:romanlutz-scorer-composition-truefalse
Open

Scorer composition (PR B): TrueFalse pure-composition API + CallableResponseHandler#2146
romanlutz wants to merge 15 commits into
microsoft:mainfrom
romanlutz:romanlutz-scorer-composition-truefalse

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

PR B of 3 — Scorer-owned composition ("Option 5")

This is the middle layer of a 3-PR scorer-architecture refactor. It builds on PR A (#2125, merged) and is the precedent PR C follows when rolling the pattern out to the remaining LLM scorers. This is a clean v1.0 breaking change — no deprecation shims, no legacy kwargs.

What changed

1. CallableResponseHandler (concern-D escape hatch). A ResponseHandler that wraps a user-supplied Callable[[str], dict] mapping raw target text → the score dict. This is the clean replacement for the old response_parser hook and unblocks plain-text scoring targets (e.g. LlamaGuard). Its response_format is None, so plain-text targets are not forced into JSON.

2. TrueFalseQuestion value type (Pydantic). Owns category, true_description, false_description, metadata="", with from_yaml(path) and to_render_dict(). Templates and params are decoupled: a templated system-prompt SeedPrompt can be rendered with a TrueFalseQuestion loaded from its own separate YAML. TrueFalseQuestionPaths and render_true_false_system_prompt(question=...) are exported alongside it.

3. SelfAskTrueFalseScorer — pure composition constructor. No legacy kwargs, no deprecation:

def __init__(
    self,
    *,
    chat_target: PromptTarget | None = None,
    system_prompt: SeedPrompt | str | None = None,
    response_handler: ResponseHandler | None = None,
    score_category: Sequence[str] | str | None = None,
    validator: ScorerPromptValidator | None = None,
    score_aggregator: TrueFalseAggregatorFunc = TrueFalseScoreAggregator.OR,
) -> None: ...

Two construction modes work: (a) a static system prompt (str or SeedPrompt, no params — e.g. LlamaGuard), and (b) a templated SeedPrompt + TrueFalseQuestion params. The scorer stores self._response_handler (the caller's, or a default JsonSchemaResponseHandler) and reads the schema identifier off it.

4. from_question factory (sanctioned "preset drives more than the prompt" pattern). Because a TrueFalseQuestion drives both the system prompt and the category, a non-deprecated classmethod renders the prompt via render_true_false_system_prompt(question=...) and sets score_category = question.category:

@classmethod
def from_question(cls, *, chat_target, question, response_handler=None,
                  validator=None, score_aggregator=TrueFalseScoreAggregator.OR): ...

5. LlamaGuard pilot. An end-to-end unit test (mocked target) wires SelfAskTrueFalseScorer with a static LlamaGuard system prompt (str) + a CallableResponseHandler that parses LlamaGuard's safe / unsafe\nS1,S2,... output into the score dict — demonstrating the escape hatch replacing the response_parser hook.

Consumers migrated to the new API

tree_of_attacks.py, scenario.py, setup/initializers/scorers.py, and the ai_recruiter integration test now use the composition API. SelfAskQuestionAnswerScorer (subclass) is made consistent with the slim API.

Response contract (from PR A, preserved)

The round-trip helper reads the schema/format off the handler: JsonSchemaResponseHandler.response_format → "json", CallableResponseHandler.response_format → None. Existing JSON scorers are unaffected.

Verification

  • tests/unit/score: 1311 passed / 16 skipped
  • ruff check, ruff format --check, ty check, full pre-commit on changed files: clean

Notes for reviewers

  • This branch was merged (not rebased) with latest main after PR A landed, so the diff is scoped to B's deltas only.
  • The remaining LLM scorers + consumers are finalized in PR C, which stacks on this branch.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Copilot AI added 15 commits July 1, 2026 23:40
…_async

PR A of the scorer-architecture refactor (Option 5: scorer-owned composition).
Moves the LLM evaluation mechanism and JSON response parsing off the base Scorer
class so the base no longer carries LLM-only machinery (retry, system-prompt
setting, JSON parsing).

- Add pyrit/score/response_handler.py: ResponseHandler ABC + JsonSchemaResponseHandler
  (response parsing) reproducing the existing JSON parsing exactly.
- Add pyrit/score/llm_scoring.py: stateless module-level run_llm_scoring_async
  (evaluation mechanism) wrapping an inner @pyrit_json_retry round-trip; the optional
  numeric-value float check runs outside the retry, preserving the old
  FloatScaleScorer behavior.
- Convert Scorer._score_value_with_llm_async into a deprecated thin shim that forwards
  to run_llm_scoring_async (removed_in 0.17.0). Signature unchanged, so the eight
  not-yet-migrated scorers keep working (migrated in PR C).
- Remove the FloatScaleScorer._score_value_with_llm_async override; replace it with a
  _score_value_is_numeric class flag the shim reads to apply the float check.
- Wire SelfAskTrueFalseScorer to call run_llm_scoring_async directly. Public
  constructor and behavior unchanged.
- Export ResponseHandler, JsonSchemaResponseHandler, run_llm_scoring_async from
  pyrit.score (additive).
- Update test_scorer_remove_markdown_json_called patch target to the new module.

No public API change. All existing score tests pass; lints clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Introduce CallableResponseHandler, an escape hatch that maps raw LLM text to a score dict, plus a response_format property on ResponseHandler so non-JSON classifiers are not forced into JSON response metadata. This replaces PR microsoft#1867's response_parser hook.

Make TrueFalseQuestion a Pydantic value type owning category/true_description/false_description/metadata with from_yaml and render_params, decoupling question params from the system-prompt template so they can live in separate YAML files. Add render_true_false_system_prompt to render a templated SeedPrompt from a question while preserving the embedded response_json_schema.

Give SelfAskTrueFalseScorer a composition constructor (target + system_prompt[str|SeedPrompt|None] + response_handler) supporting both static and templated prompts. Keep the legacy keyword arguments working behind a deprecation warning and add a from_question_yaml shim (removed_in 0.17.0). Migrate SelfAskQuestionAnswerScorer to the new API.

Pilot a static LlamaGuard system prompt scored through CallableResponseHandler, demonstrating the escape hatch end-to-end.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
For the upcoming v1.0.0 clean breaking change, PR A keeps `chat_target`
(renaming it to `target` was an unnecessary break) and ships no deprecation
warnings.

- run_llm_scoring_async / _send_and_parse_async: rename the `target` parameter
  back to `chat_target`; update both call sites (the base Scorer forwarder and
  SelfAskTrueFalseScorer).
- Scorer._score_value_with_llm_async: remove the print_deprecation_message call
  and its import. It is now a plain, warning-free internal forwarder kept only
  as a transitional shim until PR C migrates the remaining scorers and deletes
  it entirely.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rework PR B into a clean breaking change (targeting v1.0.0): drop all
legacy kwargs and deprecation shims and expose a pure-composition
constructor plus a from_question factory.

- __init__ takes chat_target/system_prompt/response_handler/score_category/
  validator/score_aggregator only; chat_target is the sole target param and
  raises ValueError when missing.
- Add from_question(chat_target, question, ...) which renders the system
  prompt via render_true_false_system_prompt and sets score_category from
  the question's category.
- Remove legacy true_false_question[_path]/true_false_system_prompt_path
  kwargs, the from_question_yaml deprecation shim,
  _build_system_prompt_from_question, TrueFalseQuestion dict-compat shims,
  and all print_deprecation_message usage.
- Fix SelfAskQuestionAnswerScorer to call super().__init__(chat_target=...).
- Migrate in-repo consumers (scenario, tree_of_attacks, setup scorers) to
  from_question and update score + integration tests to the new API.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR A is an internal refactor with no public API change, so the shared
round-trip helper should not look like public API. Rename
run_llm_scoring_async -> _run_llm_scoring_async (matching the already-private
_send_and_parse_async) and drop it from pyrit.score's __init__ import and
__all__. Both internal callers import it directly from pyrit.score.llm_scoring.

ResponseHandler / JsonSchemaResponseHandler stay exported as the new
composition abstraction.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…utz-romanlutz-scorer-composition-truefalse

# Conflicts:
#	pyrit/score/__init__.py
#	pyrit/score/true_false/self_ask_true_false_scorer.py
Address review feedback on the scorer round-trip refactor (PR A):

- ResponseHandler now owns the full response contract (schema + validation).
  The JSON schema lives on the handler via a `response_schema` property, and
  the numeric float() validation is folded into JsonSchemaResponseHandler.parse.
  llm_scoring is now a pure round-trip that reads the schema off the handler.
- Drop the "magic" getattr(self, "_score_value_is_numeric", False) flag. Float
  scorers now pass numeric_value=True explicitly through the base shim, which
  forwards it to the handler. Removed the _score_value_is_numeric ClassVar.
- Document that _run_llm_scoring_async is intentionally module-internal.

Behavior note: the numeric check now runs inside the JSON retry (via the
handler) rather than after it, so a well-formed-but-non-numeric response is
retried before failing instead of failing immediately. The end result is
unchanged (raises InvalidJsonException); no test pinned the old semantics.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…utz-romanlutz-scorer-composition-truefalse

Absorb PR A f372fc2: the ResponseHandler now owns the full response contract (JSON schema via a response_schema property + numeric float() validation folded into JsonSchemaResponseHandler.parse), and _run_llm_scoring_async no longer takes response_json_schema/numeric_value params.

Conflict resolutions:

- response_handler.py: base ResponseHandler keeps BOTH properties -- my response_format (so plain-text handlers like CallableResponseHandler are not forced into JSON) and A's response_schema. JsonSchemaResponseHandler gains response_schema/numeric_value and folds the numeric check into parse via _build_unvalidated_score.

- llm_scoring.py: keep the conditional response_format hint from the handler AND forward response_handler.response_schema (drop the removed response_json_schema param).

- self_ask_true_false_scorer.py: the system-prompt-derived schema is now injected into the default JsonSchemaResponseHandler(response_schema=...); a caller-supplied handler owns its own contract. _score_piece_async passes response_handler=self._response_handler; _build_identifier reads self._response_handler.response_schema. Removed self._response_json_schema.

Tests updated to read the schema off _response_handler.response_schema for the migrated true/false scorer (shared schema test uses a polymorphic accessor since the other scorers are not migrated until PR C).
…onse-handler-refactor

# Conflicts:
#	pyrit/score/__init__.py
…Score

Merging current main (via PR A 4b5e46c) surfaced 9 ty errors against
tightened shared models:

- MessagePiece.id is uuid.UUID, but Scorer._score_value_with_llm_async
  declared scored_prompt_id: str. Widen to str | uuid.UUID to match the
  _run_llm_scoring_async helper it forwards to (clears the 8 non-migrated
  scorer call sites).
- UnvalidatedScore.score_value_description is a required str; default a
  missing description to empty string in _build_unvalidated_score.

tests/unit/score green (1310 passed / 16 skipped); ruff, ruff-format, ty,
pre-commit all clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Main's tightened model typing surfaced 8 `ty` errors in the transitional
`Scorer._score_value_with_llm_async` shim: it declared `scored_prompt_id: str`,
but all non-migrated scorers call it with `message_piece.id`, which is a
`uuid.UUID`. The helper it forwards to (`_run_llm_scoring_async`) already accepts
`str | uuid.UUID`, so the shim signature was just too narrow.

Widen the shim param to `str | uuid.UUID` (annotation-only `uuid` import under
TYPE_CHECKING) so the whole `pyrit/score` package is `ty`-clean again. No runtime
behavior change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR A merged to main as squash commit a28ab44. Merging latest origin/main
into the PR B branch so the PR diff is scoped to B's deltas only.

Conflict resolutions:
- pyrit/score/response_handler.py, llm_scoring.py (add/add from A squash):
  kept B's superset (CallableResponseHandler, _build_unvalidated_score helper,
  conditional response_format from handler). B's normalization logic is
  identical to main's inline version plus the score_value_description='' ty fix.
- pyrit/score/__init__.py: unioned B's ResponseHandler/CallableResponseHandler/
  render_true_false_system_prompt exports with main's new OWASP output scorers.
- self_ask_true_false_scorer.py: kept B's stored self._response_handler
  composition over A's inline JsonSchemaResponseHandler(response_schema=...).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
romanlutz pushed a commit to romanlutz/PyRIT that referenced this pull request Jul 9, 2026
B opened PR microsoft#2146 and advanced to 38891a7 (666b4b6 + origin/main).
Not a no-op: origin/main moved well beyond A's squash, bringing a large
models/messages/scenario/executor refactor onto the stack. Auto-merge was
conflict-free; only fix needed was updating main's new likert test
(test_likert_scorer_accepts_float_string_score_value) from the removed
legacy constructor to the v1.0 SelfAskLikertScorer.from_likert_scale(...)
factory.

Verified green: tests/unit/score 1311 passed/16 skipped; consumers
(setup/scenario/multi_turn/promptgen) 1515 passed; ruff/ruff-format/
ty check pyrit/pre-commit --all-files all clean. Base forwarder stays
deleted; zero public run_llm_scoring_async refs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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