FIX Include the template parameters in the LLMGenericTextConverter identifier

pyrit/converter/llm_generic_text_converter.py

@@ -4,6 +4,10 @@
 import hashlib
 import logging
 import uuid
+from collections import defaultdict
+from dataclasses import fields, is_dataclass
+from inspect import isroutine
+from pathlib import PurePath
 from typing import TYPE_CHECKING, Any
 
 from tenacity import AsyncRetrying, retry_if_exception_type, stop_after_attempt, wait_exponential, wait_none
@@ -28,6 +32,66 @@
 logger = logging.getLogger(__name__)
 
 
+def _describe_template_value(value: Any, *, enclosing: frozenset[int] = frozenset()) -> str:
+    """
+    Describe a template parameter value so that values which render differently are told apart.
+
+    The description keeps container types, iteration order, path flavor, dataclass fields and
+    instance attributes. These can all affect Jinja rendering even when values compare equal
+    or have the same ``repr``. Opaque values without structural state use their ``repr``.
+
+    Args:
+        value (Any): The template parameter value.
+        enclosing (frozenset[int]): Ids of the containers being described, to stop at self-references.
+
+    Returns:
+        str: A description of the value's rendering-relevant state, tagged with its type.
+    """
+    type_name = f"{type(value).__module__}.{type(value).__qualname__}"
+    if id(value) in enclosing:
+        return f"{type_name}(<recursive>)"
+    nested = enclosing | {id(value)}
+
+    def describe(item: Any) -> str:
+        return _describe_template_value(item, enclosing=nested)
+
+    attributes = getattr(value, "__dict__", None)
+    state = f"(attributes={describe(attributes)})" if isinstance(attributes, dict) else ""
+    if is_dataclass(value) and not isinstance(value, type):
+        entries = ", ".join(f"{item.name!r}: {describe(getattr(value, item.name))}" for item in fields(value))
+        return f"{type_name}{state}{{{entries}}}"
+    if isinstance(value, dict):
+        entries = ", ".join(f"{describe(key)}: {describe(item)}" for key, item in value.items())
+        factory = f"(default_factory={describe(value.default_factory)})" if isinstance(value, defaultdict) else ""
+        return f"{type_name}{state}{factory}{{{entries}}}"
+    if isinstance(value, (list, tuple, set, frozenset)):
+        return f"{type_name}{state}[{', '.join(describe(item) for item in value)}]"
+    if isinstance(value, PurePath):
+        return f"{type_name}({str(value)!r})"
+    if state and not isinstance(value, (str, bytes, int, float, complex)) and not isroutine(value):
+        return f"{type_name}{state}"
+    return f"{type_name}{state}({value!r})"
+
+
+def _hash_template_parameters(parameters: dict[str, Any]) -> str | None:
+    """
+    Hash template parameters for a converter identifier.
+
+    Args:
+        parameters (dict[str, Any]): Template parameter names and values.
+
+    Returns:
+        str | None: The first 16 hex characters of the SHA-256 digest, or ``None`` if there are no parameters.
+    """
+    if not parameters:
+        return None
+    # Keyword argument order does not change the rendered prompt, so sort by parameter name.
+    description = ", ".join(
+        f"{name!r}: {_describe_template_value(value)}" for name, value in sorted(parameters.items())
+    )
+    return hashlib.sha256(description.encode("utf-8", errors="backslashreplace")).hexdigest()[:16]
+
+
 class LLMGenericTextConverter(Converter):
     """
     Represents a generic LLM-backed converter for text-in/text-out transformations.
@@ -81,7 +145,9 @@ def __init__(
                 between retry attempts. If ``None``, no wait is applied between attempts (matches
                 ``pyrit_json_retry``).
             kwargs: Additional parameters forwarded to both the system prompt and user prompt templates
-                during rendering.
+                during rendering. Their constructor-time state contributes to the identifier, including
+                container iteration order, path flavor and object fields. Parameters should remain static
+                after construction.
 
         Raises:
             ValueError: If converter_target is not provided and no default has been configured.
@@ -92,6 +158,9 @@ def __init__(
         self._converter_target = converter_target
         self._system_prompt_template = system_prompt_template
         self._prompt_kwargs = kwargs
+        # Some subclasses add per-call values (such as the current prompt) to ``_prompt_kwargs``
+        # before converting, so the identifier is based on the parameters given here only.
+        self._template_parameters_hash = _hash_template_parameters(kwargs)
         self._retry_exceptions = retry_exceptions if retry_exceptions is not None else self.RETRY_EXCEPTIONS
         self._max_retry_attempts = max_retry_attempts
         self._retry_wait_max_seconds = retry_wait_max_seconds
@@ -128,6 +197,8 @@ def _build_identifier(self) -> ComponentIdentifier:
             params={
                 "system_prompt_template_hash": system_prompt_hash,
                 "user_prompt_template_hash": user_prompt_hash,
+                # Template parameters are rendered into both prompts, so they change what is sent.
+                "template_parameters_hash": self._template_parameters_hash,
             },
             converter_target=self._converter_target.get_identifier(),
         )

tests/unit/converter/test_generic_llm_converter.py

@@ -1,24 +1,33 @@
 # Copyright (c) Microsoft Corporation.
 # Licensed under the MIT license.
 
+from collections import defaultdict
+from dataclasses import dataclass, field
+from pathlib import Path, PurePosixPath, PureWindowsPath
+from typing import Any
 from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
-from unit.mocks import get_mock_target_identifier
+from unit.mocks import MockPromptTarget, get_mock_target_identifier
 
 from pyrit.converter import (
     LLMGenericTextConverter,
     MaliciousQuestionGeneratorConverter,
+    MathPromptConverter,
     NoiseConverter,
     ScientificTranslationConverter,
     TenseConverter,
     ToneConverter,
+    ToxicSentenceGeneratorConverter,
     TranslationConverter,
     VariationConverter,
 )
 from pyrit.exceptions import InvalidJsonException
-from pyrit.models import Message, MessagePiece, SeedPrompt
+from pyrit.memory import SQLiteMemory
+from pyrit.models import ConverterIdentifier, Message, MessagePiece, SeedPrompt
+from pyrit.prompt_normalizer import ConverterConfiguration, PromptNormalizer
 from pyrit.prompt_target.common.prompt_target import PromptTarget
+from pyrit.registry import ConverterRegistry
 
 
 @pytest.fixture
@@ -338,3 +347,251 @@ def test_llm_converter_identifier_includes_custom_prompt_template(
     assert custom.get_identifier().hash == same_custom.get_identifier().hash
     for key, value in kwargs.items():
         assert custom_params[key] == value
+
+
+def test_generic_llm_converter_identifier_includes_template_parameters(mock_target) -> None:
+    template = _custom_template("Translate the text into {{ language }}.", "language")
+
+    french = LLMGenericTextConverter(converter_target=mock_target, system_prompt_template=template, language="French")
+    spanish = LLMGenericTextConverter(converter_target=mock_target, system_prompt_template=template, language="Spanish")
+    same_french = LLMGenericTextConverter(
+        converter_target=mock_target, system_prompt_template=template, language="French"
+    )
+    no_parameters = LLMGenericTextConverter(converter_target=mock_target, system_prompt_template=template)
+
+    assert len(french.get_identifier().params["template_parameters_hash"]) == 16
+    assert french.get_identifier().hash != spanish.get_identifier().hash
+    assert french.get_identifier().hash == same_french.get_identifier().hash
+    assert no_parameters.get_identifier().params.get("template_parameters_hash") is None
+
+
+async def _sent_prompt_async(converter: LLMGenericTextConverter, target: MagicMock) -> str:
+    await converter.convert_async(prompt="hello")
+    value = target.send_prompt_async.call_args.kwargs["message"].message_pieces[0].converted_value
+    assert isinstance(value, str)
+    return value
+
+
+@dataclass
+class _TemplateConfig:
+    language: str = field(repr=False)
+
+
+@dataclass(slots=True)
+class _SlottedTemplateConfig:
+    language: str = field(repr=False)
+
+
+class _StatefulTemplateValue:
+    def __init__(self, language: str) -> None:
+        self.language = language
+
+    def __repr__(self) -> str:
+        return "template configuration"
+
+
+class _TemplateText(str):
+    __slots__ = ("__dict__",)
+
+
+def _empty_list() -> list[str]:
+    return []
+
+
+def _empty_dict() -> dict[str, str]:
+    return {}
+
+
+def _colliding_sets() -> tuple[set[int], set[int]]:
+    first: set[int] = set()
+    second: set[int] = set()
+    for value in (0, 8):
+        first.add(value)
+    for value in (8, 0):
+        second.add(value)
+    assert list(first) != list(second)
+    return first, second
+
+
+@pytest.mark.parametrize(
+    "template_value, first_words, second_words",
+    [
+        ("{{ objective }} {{ words }}", ("alpha", "beta"), ["alpha", "beta"]),
+        ("{{ objective }}{% for word in words %} {{ word }}{% endfor %}", {"a": 1, "b": 2}, {"b": 2, "a": 1}),
+        ("{{ objective }} {{ words }}", 1, 1.0),
+        ("{{ objective }} {{ words }}", {1: "one"}, {"1": "one"}),
+        (
+            "{{ objective }} {{ words }}",
+            PureWindowsPath("styles", "noir.yaml"),
+            PurePosixPath("styles", "noir.yaml"),
+        ),
+        ("{{ objective }}{% for word in words %} {{ word }}{% endfor %}", *_colliding_sets()),
+        ("{{ objective }} {{ words.missing }}", defaultdict(list), defaultdict(dict)),
+        ("{{ objective }} {{ words.missing }}", defaultdict(_empty_list), defaultdict(_empty_dict)),
+        ("{{ objective }} {{ words }}", _TemplateText("French"), _TemplateText("Spanish")),
+        ("{{ objective }} {{ words.language }}", _TemplateConfig("French"), _TemplateConfig("Spanish")),
+        (
+            "{{ objective }} {{ words.language }}",
+            _SlottedTemplateConfig("French"),
+            _SlottedTemplateConfig("Spanish"),
+        ),
+        (
+            "{{ objective }} {{ words.language }}",
+            _StatefulTemplateValue("French"),
+            _StatefulTemplateValue("Spanish"),
+        ),
+    ],
+)
+async def test_generic_llm_converter_identifier_distinguishes_values_that_render_differently(
+    mock_target, template_value, first_words, second_words
+) -> None:
+    template = _custom_template(template_value, "objective", "words")
+    first = LLMGenericTextConverter(
+        converter_target=mock_target, user_prompt_template_with_objective=template, words=first_words
+    )
+    second = LLMGenericTextConverter(
+        converter_target=mock_target, user_prompt_template_with_objective=template, words=second_words
+    )
+
+    assert await _sent_prompt_async(first, mock_target) != await _sent_prompt_async(second, mock_target)
+    assert first.get_identifier().hash != second.get_identifier().hash
+
+    # Both configurations get their own default registry name instead of colliding.
+    instances = ConverterRegistry().instances
+    instances.register(first)
+    instances.register(second)
+    assert instances.get(first.get_identifier().unique_name) is first
+    assert instances.get(second.get_identifier().unique_name) is second
+
+
+def test_generic_llm_converter_identifier_is_stable_for_equal_template_parameters(mock_target) -> None:
+    template = _custom_template("{{ objective }} {{ words }} {{ path }} {{ tags }}", "objective")
+    tags = {"x", "y", "z"}
+
+    first = LLMGenericTextConverter(
+        converter_target=mock_target,
+        user_prompt_template_with_objective=template,
+        words={"alpha": ["a", ("b", 2)]},
+        path=Path("styles") / "noir.yaml",
+        tags=tags,
+    )
+    # Same values, keyword arguments given in a different order.
+    second = LLMGenericTextConverter(
+        converter_target=mock_target,
+        user_prompt_template_with_objective=template,
+        tags=tags,
+        path=Path("styles") / "noir.yaml",
+        words={"alpha": ["a", ("b", 2)]},
+    )
+
+    assert first.get_identifier().hash == second.get_identifier().hash
+
+
+@pytest.mark.parametrize("config_type", [_TemplateConfig, _SlottedTemplateConfig, _StatefulTemplateValue])
+def test_generic_llm_converter_identifier_is_stable_for_equal_object_state(
+    mock_target: MagicMock, config_type: type
+) -> None:
+    template = _custom_template("{{ objective }} {{ words.language }}", "objective", "words")
+    first = LLMGenericTextConverter(
+        converter_target=mock_target, user_prompt_template_with_objective=template, words=config_type("French")
+    )
+    second = LLMGenericTextConverter(
+        converter_target=mock_target, user_prompt_template_with_objective=template, words=config_type("French")
+    )
+    assert first.get_identifier().hash == second.get_identifier().hash
+
+
+def test_generic_llm_converter_without_parameters_preserves_identifier_hash(mock_target: MagicMock) -> None:
+    converter = LLMGenericTextConverter(converter_target=mock_target)
+    previous_identifier = converter._create_identifier(
+        params={"system_prompt_template_hash": None, "user_prompt_template_hash": None},
+        converter_target=mock_target.get_identifier(),
+    )
+    assert converter.get_identifier().hash == previous_identifier.hash
+
+
+class _UnrepresentableValue:
+    def __repr__(self) -> str:
+        raise RuntimeError("no repr")
+
+    def __str__(self) -> str:
+        return "custom"
+
+
+@pytest.mark.parametrize(
+    "words",
+    [
+        {1: "first", "two": "second"},
+        {("a", "b"): "pair", None: "none"},
+        {frozenset({1, "x"}), 2.5, b"bytes"},
+        _UnrepresentableValue(),
+    ],
+)
+async def test_generic_llm_converter_converts_with_any_renderable_template_parameter(mock_target, words) -> None:
+    self_referencing: list[Any] = ["loop"]
+    self_referencing.append(self_referencing)
+    converter = LLMGenericTextConverter(
+        converter_target=mock_target,
+        user_prompt_template_with_objective=_custom_template("{{ objective }} {{ words }} {{ loop }}", "objective"),
+        words=words,
+        loop=self_referencing,
+    )
+
+    result = await converter.convert_async(prompt="hello")
+
+    assert result.output_text == "prompt value"
+    request = mock_target.send_prompt_async.call_args.kwargs["message"]
+    assert request.message_pieces[0].converter_identifiers[0].hash == converter.get_identifier().hash
+    assert len(converter.get_identifier().params["template_parameters_hash"]) == 16
+
+
+@pytest.mark.usefixtures("patch_central_database")
+async def test_generic_llm_converter_distinct_path_identifiers_persist_async(sqlite_instance: SQLiteMemory) -> None:
+    converter_target = MockPromptTarget()
+    objective_target = MockPromptTarget()
+    normalizer = PromptNormalizer()
+    template = _custom_template("{{ objective }} {{ path }}", "objective", "path")
+    converters = [
+        LLMGenericTextConverter(
+            converter_target=converter_target, user_prompt_template_with_objective=template, path=path
+        )
+        for path in (PureWindowsPath("styles", "noir.yaml"), PurePosixPath("styles", "noir.yaml"))
+    ]
+
+    for converter in converters:
+        await normalizer.send_prompt_async(
+            message=MessagePiece(role="user", original_value="hello").to_message(),
+            target=objective_target,
+            request_converter_configurations=[ConverterConfiguration(converters=[converter])],
+        )
+
+    assert converter_target.prompt_sent == ["hello styles\\noir.yaml", "hello styles/noir.yaml"]
+    identifiers = await sqlite_instance.get_converter_identifiers_async(class_name="LLMGenericTextConverter")
+    assert {identifier.hash for identifier in identifiers} == {
+        converter.get_identifier().hash for converter in converters
+    }
+    assert len(identifiers) == 2
+    for converter in converters:
+        identifier = converter.get_identifier()
+        assert ConverterIdentifier.model_validate_json(identifier.model_dump_json()).hash == identifier.hash
+
+
+@pytest.mark.parametrize(
+    "converter_class",
+    [MathPromptConverter, MaliciousQuestionGeneratorConverter, ToxicSentenceGeneratorConverter],
+)
+async def test_per_call_prompt_does_not_change_converter_identifier(mock_target, converter_class) -> None:
+    read_before = converter_class(converter_target=mock_target)
+    read_after = converter_class(converter_target=mock_target)
+    identifier_before_conversion = read_before.get_identifier()
+
+    sent_identifiers = []
+    for converter in (read_before, read_after):
+        for prompt in ("hello", "goodbye"):
+            await converter.convert_async(prompt=prompt)
+            request = mock_target.send_prompt_async.call_args.kwargs["message"]
+            sent_identifiers.append(request.message_pieces[0].converter_identifiers[0])
+
+    assert read_after.get_identifier().hash == identifier_before_conversion.hash
+    assert {identifier.hash for identifier in sent_identifiers} == {identifier_before_conversion.hash}
+    assert identifier_before_conversion.params.get("template_parameters_hash") is None