MAINT: Updating backend to use registry to create converters#2112
Open
rlundeen2 wants to merge 9 commits into
Open
MAINT: Updating backend to use registry to create converters#2112rlundeen2 wants to merge 9 commits into
rlundeen2 wants to merge 9 commits into
Conversation
…es (Phase 4) Move TargetRegistry and ScorerRegistry off the legacy instance-only RetrievableInstanceRegistry stack onto the unified Registry + .instances shape already used by ConverterRegistry. Both registries now build instances by name (LLM scorers resolve a chat_target registry name; composite scorers and RoundRobinTarget resolve a list of names) and hold pre-configured instances under .instances. Per the no-deprecation decision, the old singleton surface (register_instance / get_instance_by_name / reset_instance) is removed entirely and every call site is updated to the .instances.* pattern in the same change. - Add Param build markers/aliases to scorer/target identifiers (chat_target, scorers, targets) - Make reference resolution list-aware; collapse the resolver getter to a flat ComponentType -> Registry map; move the resolvability gate onto the Registry base - Add TargetRegistry/ScorerRegistry under pyrit/registry/components with TargetMetadata/ScorerMetadata - Update all production consumers, tests, and docs/notebooks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… scenarios notebook Address review findings on the Phase 4 target/scorer registry migration: - microsoft#3: _resolve_registry_reference now raises a clear ValueError when a list-annotated reference receives a non-list value (and vice versa), instead of constructing the component with the wrong argument shape and failing obscurely downstream. Docstring Raises updated. - microsoft#4: add tests for pre-built instances passed inside a list (passthrough), mixed name/instance lists, and the new shape-mismatch guards, for both the target and scorer registries. - #1: refresh the stale generated output in doc/code/scenarios/0_scenarios.ipynb that still referenced the removed TargetRegistry.register_instance, matching the already-migrated source docstrings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The legacy RetrievableInstanceRegistry had zero production subclasses after targets and scorers moved onto the unified Registry + .instances stack. Its get/get_entry/get_all_instances helpers are fully provided by the new-stack DefaultInstanceRegistry. Remove the class and its exports; BaseInstanceRegistry stays for AttackTechniqueRegistry. Repoint the legacy-base tests' ConcreteTestRegistry onto BaseInstanceRegistry with inline retrieval helpers (a test double) so BaseInstanceRegistry coverage is preserved, and drop the obsolete Base/Retrievable split assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Eliminate the ~95% duplication across TargetRegistry, ScorerRegistry, and ConverterRegistry by moving the shared mechanics into the base Registry: - Default _discover() scans _discovery_package().__all__ for concrete _base_type() subclasses (skipping the base and abstract classes via inspect.isabstract) and registers each by class name. - _get_registry_name default snaps to the class name (was snake_case); registries with a different scheme override it. - New _base_type()/_discovery_package() hooks (NotImplementedError defaults) let registries with bespoke discovery override _discover() instead. The three component registries now only supply _base_type, _discovery_package, _identifier_type, _metadata_class, plus their .instances container and any projected metadata properties. Abstract-base skipping is now uniform across all three. Updated the base Registry test to the class-name default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…r service test The converter-service mock helper set `mock_target.__class__.__name__ = 'MockChatTarget'` on a `MagicMock(spec=PromptTarget)`. A spec'd mock returns the real spec class from `__class__` (so isinstance works), so this mutated the global `PromptTarget.__name__` permanently. Any later test reading that name — e.g. TargetRegistry's instance-type rejection message — then saw 'MockChatTarget', making test_register_instance_rejects_non_target fail in the full suite. The mock's identifier already carries class_name='MockChatTarget' via get_identifier(), so the line was redundant; remove it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Task 1: ConverterInstance composes ConverterIdentifier instead of duplicating its fields. Task 2: Make pyrit.models.Parameter the single JSON-serializable parameter descriptor reused across the converter, scenario, and initializer domains, the backend API, and the CLI. Collapses the near-identical ConverterParameterSchema, scenario parameter metadata, and InitializerParameterSummary onto Parameter. Serialization projects the live param_type into display fields (type_name/choices/is_list). List-valued defaults are preserved as a list of display strings instead of being flattened to a single string, so list[str] params round-trip. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… converter/Parameter unification branch # Conflicts: # pyrit/registry/components/converter_registry.py
Resolved 3 conflicts: - converter_service.py: kept origin/main's DEFAULT_MEDIA_EXTENSIONS media consolidation; both sides' extra typing imports were orphaned by the merge, so the import is reduced to the only symbol still used (Any). - pyrit_scan.py: kept origin/main's Parameter-based scenario coercer (microsoft#2092), which supersedes this branch's _SCALAR_TYPE_COERCERS approach toward the same typed-coercion goal. - test_pyrit_scan.py: migrated origin/main's new coercion tests from the pre-unification "param_type" payload field to the canonical "type_name"/ "is_list"/"choices" wire format the backend now emits. Full unit suite: 10045 passed, 121 skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… into branch Pulls origin/main, which migrated the target and scorer registries onto the unified Registry base and moved the CLI onto pyrit.models, into the converter/Parameter unification branch. Conflict resolution + finalization: - AttackTechniqueRegistry moved onto the unified Registry base; affected scenario/setup tests updated to reset_registry_singleton(). - Unify scenario/initializer catalog DTOs onto pyrit.models.Parameter: dropped ScenarioParameterSummary / InitializerParameterSummary in favor of list[Parameter] on RegisteredScenario / RegisteredInitializer, so the one reusable Parameter descriptor is used across registry, backend, and CLI. - Parameter is now round-trippable: a model_validator(mode="before") reconstructs the live param_type from the serialized display fields (type_name / choices / is_list), so REST clients (the CLI) deserialize catalog payloads straight into coercion-ready Parameter objects. - CLI build_parameters_from_api simplified to a pass-through; _output and pyrit_scan read Parameter.type_name. CLI tests build typed Parameter instances instead of the removed Summary DTOs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
romanlutz
approved these changes
Jul 1, 2026
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
2 participants
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.
Summary
Makes the backend converter service build converters entirely through the registry, deleting the custom construction/introspection glue it used to carry. This is the converter analog of the target-service work (Phase 4.5): the consumer-side payoff of the converter registry landed in Phases 1–3.
create_converter_asyncresolves the class and constructs viaregistry.create_instance(...), which coerces plain values and resolves registry references by name. The registry is now the single build path for converters.converter_service:_serialize_type,_build_parameter_schema,_resolve_converter_params, andinspect.signature-based introspection.GET /converters/catalogand data-URI persistence read their parameter shapes fromregistry.get_registered_class_metadatainstead of re-deriving them.pyrit.models.Parameter, andConverterInstanceembeds a lossless serializedConverterIdentifier(dropping the redundant backend-only DTOs).Parametershape and no longer does its ownOptional[...]unwrapping.Architecture / responsibilities
Aligns with the two-registry model documented in
doc/code/registry/0_registry.md: the class registry owns "build from name + params" and the paired.instancescontainer holds the built converters. The backend service stops owning any converter-construction responsibility and simply delegates to the registry, keeping theParametercontract as the single source of truth across registry, backend, CLI, and frontend.Breaking
Converter references now resolve by registry name (or an already-built instance) via
registry.create_instance, replacing the old nested{"converter": {"converter_id": ...}}reference payload. The current frontend never sent that shape (it blocks LLM/composite converters), so there's no UI regression, but external API clients relying on the nested reference form must switch to string names.Plan / phase
Part of the unified-registry redesign — see the design gist. This lands the newly-added Phase 3.5 — Converter service builds through the registry (the consumer-side payoff of Phases 1–3, mirroring Phase 4.5 for targets).
Testing
pytest tests/unit— 10148 passed, 121 skippednpm run type-check,npm run lint, and converter jest suites (141 passed) all green