Skip to content

REFACTOR: Make Translation/Variation/Persuasion inherit from LLMGenericTextConverter#1714

Open
romanlutz wants to merge 1 commit intomicrosoft:mainfrom
romanlutz:llm-converter-inheritance
Open

REFACTOR: Make Translation/Variation/Persuasion inherit from LLMGenericTextConverter#1714
romanlutz wants to merge 1 commit intomicrosoft:mainfrom
romanlutz:llm-converter-inheritance

Conversation

@romanlutz
Copy link
Copy Markdown
Contributor

What

Refactors TranslationConverter, VariationConverter, and PersuasionConverter to inherit from LLMGenericTextConverter instead of duplicating the build-conversation/send/return loop. Adds three small extensibility points to the base class so the three subclasses can be ~25-line thin wrappers.

Why

Three converters (~500 lines combined) each reimplemented:

  • Conversation construction + system-prompt setup + send to target
  • A bespoke retry policy (Translation: tenacity with max_retries/max_wait_time_in_seconds; Variation/Persuasion: @pyrit_json_retry)
  • Response post-processing (Translation: .strip(); Variation: parse JSON list, take [0]; Persuasion: parse JSON, extract mutated_text)

The base class gains a _process_response hook and a unified retry helper that can produce both retry policies, so the duplication collapses cleanly.

Base-class additions (LLMGenericTextConverter)

  • _process_response(response_text) -> str hook (default: identity)
  • RETRY_EXCEPTIONS: tuple[type[BaseException], ...] class attribute (default () = no retry) + per-instance retry_exceptions constructor override
  • max_retry_attempts: int | None and retry_wait_max_seconds: int | None constructor params for fixed-attempt + exponential-backoff policy; defaults give the same env-controlled, no-wait behavior as pyrit_json_retry
  • **kwargs now also forwarded to user_prompt_template_with_objective rendering (in addition to system prompt)
  • MessagePiece now built with original_value=raw_input, converted_value=templated_text (previously the raw input was lost when a user template was set)
  • Input-type validation moved before set_system_prompt

Subclass changes

  • TranslationConverter: inherits; passes both max_retry_attempts (=max_retries, default 3) and retry_wait_max_seconds (=max_wait_time_in_seconds, default 60) → behavior identical to before. F-string user prompt extracted into translation_user_prompt.yaml (byte-for-byte equivalent, regression-tested).
  • VariationConverter: inherits; RETRY_EXCEPTIONS = (InvalidJsonException,); new variation_user_prompt.yaml for the per-call wrapper.
  • PersuasionConverter: inherits; RETRY_EXCEPTIONS = (InvalidJsonException,); no user template (sends raw user prompt as before).

Compatibility

Deprecations (one release):

  • VariationConverter.send_variation_prompt_async → public shim emitting DeprecationWarning; delegates to _send_with_retries_async
  • PersuasionConverter.send_persuasion_prompt_async → same treatment

Observable bug-fix-style changes:

  • VariationConverter: MessagePiece.original_value now stores the raw user input instead of the templated === begin === ... === end === wrapper. Matches Translation/Persuasion convention.
  • TranslationConverter: set_system_prompt is no longer called on the target when input type is unsupported (validation moved before side effects).

Preserved:

  • Translation max_retries=3, max_wait_time_in_seconds=60 defaults — fully functional, not deprecated
  • Variation/Persuasion JSON retry semantics match pyrit_json_retry exactly (same exception type, same env-controlled attempt count via _DynamicStopAfterAttempt/RETRY_MAX_NUM_ATTEMPTS, same reraise=True, no wait between attempts)
  • All persisted converter identifiers
  • Translation user-prompt rendered text (byte-for-byte verified by regression test)
  • All 8 existing LLMGenericTextConverter children (Tone, Tense, Noise, Denylist, Math, MaliciousQuestion, ScientificTranslation, ToxicSentenceGenerator) and RandomTranslationConverter's multiple inheritance

Tests

  • 922 prompt_converter unit tests pass (added 15 new for base class hooks + 6 for refactored subclasses)
  • 6,564 other unit tests pass — no cross-module regression
  • Pre-commit clean (ruff, ty, yaml)

…icTextConverter

Adds three small extensibility points to LLMGenericTextConverter so the three

remaining LLM-backed text converters (Translation, Variation, Persuasion) can

inherit from it instead of duplicating the build-conversation/send/return loop:

  - `_process_response` hook for response post-processing (.strip, JSON parse)

  - `RETRY_EXCEPTIONS` class attribute (with per-instance `retry_exceptions`

    override) opting subclasses into a unified retry helper that uses the same

    env-controlled `RETRY_MAX_NUM_ATTEMPTS` knob as `pyrit_json_retry`,

    plus `wait_exponential`, `reraise=True` and `log_exception`.

  - `**kwargs` forwarded to `user_prompt_template_with_objective` rendering

    so subclasses can pass static template parameters (e.g., language).

Other changes:

  - Base `MessagePiece` now preserves the raw input as `original_value` and

    the templated text as `converted_value` (previously the raw was lost when

    a user template was set).

  - Input-type validation moved before `set_system_prompt` (matches the

    behaviour Variation/Persuasion already had).

  - Translation's f-string user prompt is now a YAML SeedPrompt that renders

    byte-for-byte equivalent text (regression test included).

  - Translation's `max_retries` and `max_wait_time_in_seconds` ctor args are

    deprecated for one release; retry attempts are env-controlled now.

  - Three send-method shims (`_send_translation_prompt_async`,

    `send_variation_prompt_async`, `send_persuasion_prompt_async`) emit

    `DeprecationWarning` but still delegate to the unified helper.

  - Persisted converter identifiers are unchanged for the three subclasses.

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.

1 participant