MAINT FIX: Moving Message metadata to AttackResult and attack score fix

[rlundeen2]

Moving metadata to AttackResult

This PR moves many fields that were part of Message into the AttackResult. Including targeted_harm_categories, converter_identifiers, attack_identifier, scorer_identifier. The old usage still exists but is deprecated. These fields make a lot more sense as part of a conversation/attack and shouldn't be attached to every single message_piece.

Deprecated get_prompt_scores in MemoryInterface

This makes less sense, because many of those fields are now AttackResult fields. So before, the usage might be "get all scores that had this attackID". But now the usage should be "Get the AttackResults with this AttackID" and the scores are attached to those.

Crescendo/TAP Scoring Bug Fix

There was also a pretty bad bug in TAP (and to a lesser extent, Crescendo). When we allowed TrueFalseScorers to be passed in, we lost the float value, so TAP was unable to actually retrieve the "best" conversation (it was either 1 or 0). Also, Crescendo couldn't get that feedback, but it relies on it less. normalize_score_to_float now fixes this by adding the float value to metadata, and TAP now requires a FloatScaleThreshholdScorer so this value is set.

Tests

• I added tests to check the score bug and that floats are actually used
• Ran integration tests prior to merging main [e7894d7]

[bashirpartovi]

I have a few concerns with this:

• request_converters: Optional[list[Any]] is too loosely typed. When you use Any, it removes IDE autocompletion/type checking and makes it really unclear what callers should pass.
• Looking at the usage in the code, request_converters only accepts list[PromptConverterConfiguration] or list[PromptConverter]. We should make sure the signature explicitly uses a union (e.g., Optional[list[PromptConverterConfiguration | PromptConverter]]).
• Using hasattr here is really not ideal because it can fail silently if you ever rename the attribute. For example, if PromptConverterConfiguration.converters were renamed to convs, this code would still type-check and run but would not behave correctly and would be a hidden bug in the code. If we use Union, it would be safer to branch using isinstance() and handle each type explicitly.

I would push back a bit on adding all the identifier fields directly to AttackResult and removing the dataclass. It bloats the class, loses the dataclass benefits (__repr__, __eq__, field defaults), and turns AttackResult into a catch-all bucket. I think a cleaner pattern is adding a single structured field that groups related identifiers:

@dataclass
class AttackLineage: # naming is my weakness
    objective_target_identifier: dict[str, str]
    request_converter_identifiers: Optional[list[dict[str, str]]] = None
    response_converter_identifiers: Optional[list[dict[str, str]]] = None
    objective_scorer_identifier: Optional[dict[str, str]] = None
    adversarial_chat_target_identifier: Optional[dict[str, str]] = None

Then AttackResult just gets one new field which is lineage: Optional[AttackLineage] = None. This keeps the dataclass, groups related tracking info logically, and is easy to extend later without ever touching AttackResult again.
Here's how the helper methods would look:

def _get_attack_lineage(
    self,
    *,
    request_converters: Optional[list[PromptConverterConfiguration | PromptConverter]] = None,
    response_converters: Optional[list[PromptConverterConfiguration | PromptConverter]] = None,
    objective_scorer_identifier: Optional[dict[str, str]] = None,
    adversarial_chat_target_identifier: Optional[dict[str, str]] = None,
) -> AttackLineage:
    return AttackLineage(
        objective_target_identifier=self.get_objective_target().get_identifier(),
        request_converter_identifiers=self._extract_converter_identifiers(request_converters),
        response_converter_identifiers=self._extract_converter_identifiers(response_converters),
        objective_scorer_identifier=objective_scorer_identifier,
        adversarial_chat_target_identifier=adversarial_chat_target_identifier,
    )


@staticmethod
def _extract_converter_identifiers(
    converters: Optional[list[PromptConverterConfiguration | PromptConverter]],
) -> Optional[list[dict[str, str]]]:

    if not converters:
        return None

    all_converters: list[PromptConverter] = []

    for item in converters:
        if isinstance(item, PromptConverterConfiguration):
            all_converters.extend(item.converters)
        elif isinstance(item, PromptConverter):
            all_converters.append(item)

    if not all_converters:
        return None

    return [converter.get_identifier() for converter in all_converters]

[bashirpartovi]

As I mentioned above, we may be able to keep this as a @dataclass and still achieve what you are trying to do here

[bashirpartovi]

Why _attack_identifier instead of attack_identifier?

[bashirpartovi]

Would it be possible to just do combined_dict(metadaya, s.score_metadata) ? getattr doesn't give you IDE support and if we ever rename the attribute, this will be a bug which is hidden