MAINT: Detangle scorer LLM round-trip — ResponseHandler + internal helper (1/3)#2125
Open
romanlutz wants to merge 3 commits into
Open
MAINT: Detangle scorer LLM round-trip — ResponseHandler + internal helper (1/3)#2125romanlutz wants to merge 3 commits into
romanlutz wants to merge 3 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>
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>
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>
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.
This is step 1 of 3 in a scorer-architecture refactor — "Option 5: Scorer-owned composition". Full design notes: design gist.
Follow-up to #1867, which bolted a
response_parserhook ontoSelfAskTrueFalseScorerfor LlamaGuard and surfaced that the baseScorerclass conflates concerns that should be separate (evaluation mechanism, system-prompt construction, and response parsing all live on the base, so every non-LLM scorer inherits machinery it can never use).Scope of this PR — internal refactor, no public API change
No behavior change and no constructor changes. This purely rehomes internals so the base
Scorerstops carrying LLM-only machinery.ResponseHandlerabstraction (pyrit/score/response_handler.py) — aResponseHandlerABC plus a defaultJsonSchemaResponseHandlerthat owns turning the target's raw text into the parsedUnvalidatedScoreshape (score_value,rationale/description,metadata,category).JsonSchemaResponseHandlerreproduces today's parsing exactly —json.loads(remove_markdown_json(...))plus the same key lookups, category/metadata normalization, andInvalidJsonException/ValueErrorbehavior.pyrit/score/llm_scoring.py) — a module-internal_run_llm_scoring_async(*, chat_target, system_prompt, response_handler, value, ...)that performs the LLM round-trip previously living inScorer._score_value_with_llm_async: sets the system prompt on the target, sends the prompt, applies the existing@pyrit_json_retrybehavior, and delegates parsing to the handler.Scorerslimmed — the retry decorator, JSON parsing, and system-prompt handling move off the base.Scorer._score_value_with_llm_asyncis now a thin internal forwarder to the helper, kept only so the not-yet-migrated scorers keep working. It is deleted in step 3.FloatScaleScoreroverride removed — the old override only added afloat()post-check on the parsed value; that check is folded into the helper via a_score_value_is_numericflag, preserving behavior.SelfAskTrueFalseScorerwired to call the helper directly. Its public constructor and behavior are unchanged.The 3-step plan (per the gist)
ResponseHandler(JSON-schema default), rehome the_score_value_with_llm_asynclogic off the baseScorer, and wireSelfAskTrueFalseScorer. No public-API change.SelfAskTrueFalseScorer.__init__(chat_target + system_prompt + response_handler) plus aTrueFalseQuestionvalue type; add theCallableResponseHandlerescape hatch and pilot LlamaGuard.from_question_yamlbecomes a deprecation shim.Composites/wrappers (
TrueFalseCompositeScorer,Audio/VideoTrueFalseScorer,ConversationScorer, …) and all non-LLM scorers are unchanged.Testing
uv run pytest tests/unit/score→ 1232 passed, 16 skipped.ruff,ty, andpre-commitall clean.