Skip to content

Commit 77a3704

Browse files
FIX: Resolve CI failures from upstream merge with PR #1881 (Pydantic identifiers)
Four related fixes needed to unblock CI after the upstream merge that brought in PR #1881 (Refactoring Identifiers to be Pydantic classes): 1. `pyrit/models/identifiers/evaluation_identifier.py`: The merge hoisted `from pyrit.executor.attack.core.attack_strategy import AttackStrategy` to module level, forming a cycle through `pyrit.executor.attack` -> `pyrit.message_normalizer` -> `pyrit.common.data_url_converter` -> `pyrit.models`. Move it back inside `if TYPE_CHECKING:` (`from __future__ import annotations` is already enabled, so the string annotation `attack: AttackStrategy` still resolves at type-check time). 2. `tests/unit/models/identifiers/test_evaluation_identifier.py`: Add missing blank lines between top-level classes (ruff format) and replace four `from pyrit.identifiers import ...` lines with `from pyrit.models.identifiers import ...` (the former is now a deprecation shim that the static-scan deprecation test forbids internal callers from using). 3. `pyrit/scenario/scenarios/adaptive/adaptive_scenario.py`: Mark the three classmethod stubs as `@abstractmethod` so `inspect.isabstract(AdaptiveScenario)` returns `True` and the scenario registry's auto-discovery skips it (otherwise the registry tries to instantiate the abstract base and raises `NotImplementedError`, breaking `test_load_default_datasets`). 4. `pyrit/scenario/scenarios/adaptive/adaptive_scenario.py` and `pyrit/scenario/scenarios/adaptive/text_adaptive.py`: Move `from pyrit.setup.initializers.components.scenario_techniques import build_scenario_technique_factories` from module level back into the function body. `scenario_techniques` imports `pyrit.scenario.core`, which transitively re-imports the adaptive package during `pyrit.scenario` initialization, so a top-level import forms a cycle. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 115d1f7 commit 77a3704

4 files changed

Lines changed: 30 additions & 12 deletions

File tree

pyrit/models/identifiers/evaluation_identifier.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@
2323
from __future__ import annotations
2424

2525
from abc import ABC
26-
from typing import Any, ClassVar, Optional
26+
from typing import TYPE_CHECKING, Any, ClassVar, Optional
2727

2828
from pydantic import BaseModel, ConfigDict, Field
2929

30-
from pyrit.executor.attack.core.attack_strategy import AttackStrategy
3130
from pyrit.models.identifiers.component_identifier import ComponentIdentifier, config_hash
3231

32+
if TYPE_CHECKING:
33+
from pyrit.executor.attack.core.attack_strategy import AttackStrategy
34+
3335
# Behavioral params that define model output quality for scoring.
3436
TARGET_EVAL_PARAMS: frozenset[str] = frozenset({"underlying_model_name", "temperature", "top_p"})
3537
TARGET_EVAL_PARAM_FALLBACKS: dict[str, str] = {"underlying_model_name": "model_name"}

pyrit/scenario/scenarios/adaptive/adaptive_scenario.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from __future__ import annotations
1818

1919
import logging
20+
from abc import abstractmethod
2021
from typing import TYPE_CHECKING, ClassVar
2122

2223
from pyrit.executor.attack import AttackScoringConfig
@@ -32,9 +33,6 @@
3233
EpsilonGreedyTechniqueSelector,
3334
TechniqueSelector,
3435
)
35-
from pyrit.setup.initializers.components.scenario_techniques import (
36-
build_scenario_technique_factories,
37-
)
3836

3937
if TYPE_CHECKING:
4038
from pyrit.models import SeedAttackGroup
@@ -63,16 +61,19 @@ class AdaptiveScenario(Scenario):
6361
_atomic_attack_prefix: ClassVar[str] = "adaptive"
6462

6563
@classmethod
64+
@abstractmethod
6665
def get_strategy_class(cls) -> type[ScenarioStrategy]:
6766
"""Return the scenario's strategy enum (subclasses must override)."""
6867
raise NotImplementedError
6968

7069
@classmethod
70+
@abstractmethod
7171
def get_default_strategy(cls) -> ScenarioStrategy:
7272
"""Return the scenario's default strategy aggregate (subclasses must override)."""
7373
raise NotImplementedError
7474

7575
@classmethod
76+
@abstractmethod
7677
def default_dataset_config(cls) -> DatasetConfiguration:
7778
"""Return the scenario's default ``DatasetConfiguration`` (subclasses must override)."""
7879
raise NotImplementedError
@@ -120,6 +121,13 @@ def _get_attack_technique_factories(self) -> dict[str, AttackTechniqueFactory]:
120121
Returns:
121122
dict[str, AttackTechniqueFactory]: Mapping of technique name to factory.
122123
"""
124+
# Local import: ``scenario_techniques`` imports ``pyrit.scenario.core``,
125+
# which transitively re-imports this module, so a top-level import
126+
# would form a cycle during ``pyrit.scenario`` package initialization.
127+
from pyrit.setup.initializers.components.scenario_techniques import (
128+
build_scenario_technique_factories,
129+
)
130+
123131
return {factory.name: factory for factory in build_scenario_technique_factories()}
124132

125133
async def _get_atomic_attacks_async(self) -> list[AtomicAttack]:

pyrit/scenario/scenarios/adaptive/text_adaptive.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@
2424
from pyrit.registry.tag_query import TagQuery
2525
from pyrit.scenario.core.dataset_configuration import DatasetConfiguration
2626
from pyrit.scenario.scenarios.adaptive.adaptive_scenario import AdaptiveScenario
27-
from pyrit.setup.initializers.components.scenario_techniques import (
28-
build_scenario_technique_factories,
29-
)
3027

3128
if TYPE_CHECKING:
3229
from pyrit.scenario.core.scenario_strategy import ScenarioStrategy
@@ -48,6 +45,13 @@ def _build_text_adaptive_strategy() -> type[ScenarioStrategy]:
4845
Returns:
4946
type[ScenarioStrategy]: The dynamically-built strategy enum class.
5047
"""
48+
# Local import: ``scenario_techniques`` imports ``pyrit.scenario.core``,
49+
# which transitively re-imports this module, so a top-level import would
50+
# form a cycle during ``pyrit.scenario`` package initialization.
51+
from pyrit.setup.initializers.components.scenario_techniques import (
52+
build_scenario_technique_factories,
53+
)
54+
5155
factories = [
5256
factory for factory in build_scenario_technique_factories() if factory.name not in _EXCLUDED_TECHNIQUES
5357
]

tests/unit/models/identifiers/test_evaluation_identifier.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,7 @@ def test_scorer_eval_hash_matches_with_and_without_round_robin(self):
585585

586586
assert eval_direct == eval_rr
587587

588+
588589
class TestComputeInnerAttackEvalHash:
589590
"""``compute_inner_attack_eval_hash`` should match what the executor stamps."""
590591

@@ -597,7 +598,7 @@ def _attack_with_identifier(self, identifier: ComponentIdentifier):
597598

598599
def test_matches_manual_two_step_composition(self):
599600
"""Helper equals the executor recipe (build_atomic_attack_identifier + AtomicAttackEvaluationIdentifier)."""
600-
from pyrit.identifiers import (
601+
from pyrit.models.identifiers import (
601602
AtomicAttackEvaluationIdentifier,
602603
build_atomic_attack_identifier,
603604
compute_inner_attack_eval_hash,
@@ -615,7 +616,7 @@ def test_matches_manual_two_step_composition(self):
615616
assert compute_inner_attack_eval_hash(attack=attack) == expected
616617

617618
def test_differs_when_attack_class_differs(self):
618-
from pyrit.identifiers import compute_inner_attack_eval_hash
619+
from pyrit.models.identifiers import compute_inner_attack_eval_hash
619620

620621
a = self._attack_with_identifier(
621622
ComponentIdentifier(class_name="A", class_module="m"),
@@ -626,7 +627,7 @@ def test_differs_when_attack_class_differs(self):
626627
assert compute_inner_attack_eval_hash(attack=a) != compute_inner_attack_eval_hash(attack=b)
627628

628629
def test_stable_across_calls_for_same_attack(self):
629-
from pyrit.identifiers import compute_inner_attack_eval_hash
630+
from pyrit.models.identifiers import compute_inner_attack_eval_hash
630631

631632
attack = self._attack_with_identifier(
632633
ComponentIdentifier(class_name="Same", class_module="m"),
@@ -636,9 +637,9 @@ def test_stable_across_calls_for_same_attack(self):
636637
def test_matches_persisted_row_eval_hash(self):
637638
"""Whatever the helper returns, persisting an attack result with the same
638639
identifier must yield an entry with the same eval_hash."""
639-
from pyrit.identifiers import build_atomic_attack_identifier, compute_inner_attack_eval_hash
640640
from pyrit.memory.memory_models import AttackResultEntry
641641
from pyrit.models import AttackResult
642+
from pyrit.models.identifiers import build_atomic_attack_identifier, compute_inner_attack_eval_hash
642643

643644
inner_id = ComponentIdentifier(
644645
class_name="MyAttack",
@@ -654,6 +655,8 @@ def test_matches_persisted_row_eval_hash(self):
654655
)
655656
entry = AttackResultEntry(entry=result)
656657
assert entry.atomic_attack_identifier["eval_hash"] == predicted
658+
659+
657660
# ---------------------------------------------------------------------------
658661
# OWN_RULE / leaf-entity eval-hash tests
659662
# ---------------------------------------------------------------------------
@@ -864,6 +867,7 @@ def test_stored_eval_hash_takes_precedence(self):
864867

865868
assert ObjectiveTargetEvaluationIdentifier(cid).eval_hash == stored
866869

870+
867871
class TestComputeInnerAttackEvalHash:
868872
"""``compute_inner_attack_eval_hash`` should match what the executor stamps."""
869873

0 commit comments

Comments
 (0)