Scorer composition (PR B): TrueFalse pure-composition API + CallableResponseHandler#2146
Open
romanlutz wants to merge 15 commits into
Open
Scorer composition (PR B): TrueFalse pure-composition API + CallableResponseHandler#2146romanlutz wants to merge 15 commits into
romanlutz wants to merge 15 commits into
Conversation
…_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>
…utz-romanlutz-scorer-composition-truefalse
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
…utz-romanlutz-scorer-composition-truefalse
…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>
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.
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). AResponseHandlerthat wraps a user-suppliedCallable[[str], dict]mapping raw target text → the score dict. This is the clean replacement for the oldresponse_parserhook and unblocks plain-text scoring targets (e.g. LlamaGuard). Itsresponse_formatisNone, so plain-text targets are not forced into JSON.2.
TrueFalseQuestionvalue type (Pydantic). Ownscategory,true_description,false_description,metadata="", withfrom_yaml(path)andto_render_dict(). Templates and params are decoupled: a templated system-promptSeedPromptcan be rendered with aTrueFalseQuestionloaded from its own separate YAML.TrueFalseQuestionPathsandrender_true_false_system_prompt(question=...)are exported alongside it.3.
SelfAskTrueFalseScorer— pure composition constructor. No legacy kwargs, no deprecation:Two construction modes work: (a) a static system prompt (
strorSeedPrompt, no params — e.g. LlamaGuard), and (b) a templatedSeedPrompt+TrueFalseQuestionparams. The scorer storesself._response_handler(the caller's, or a defaultJsonSchemaResponseHandler) and reads the schema identifier off it.4.
from_questionfactory (sanctioned "preset drives more than the prompt" pattern). Because aTrueFalseQuestiondrives both the system prompt and the category, a non-deprecated classmethod renders the prompt viarender_true_false_system_prompt(question=...)and setsscore_category = question.category:5. LlamaGuard pilot. An end-to-end unit test (mocked target) wires
SelfAskTrueFalseScorerwith a static LlamaGuard system prompt (str) + aCallableResponseHandlerthat parses LlamaGuard'ssafe/unsafe\nS1,S2,...output into the score dict — demonstrating the escape hatch replacing theresponse_parserhook.Consumers migrated to the new API
tree_of_attacks.py,scenario.py,setup/initializers/scorers.py, and theai_recruiterintegration 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 skippedruff check,ruff format --check,ty check, fullpre-commiton changed files: cleanNotes for reviewers
mainafter PR A landed, so the diff is scoped to B's deltas only.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com