Repository navigation
Include ordered parameter bindings in TemplateSegmentConverter identity - #3972
Closed
Santosh Todkar (todkarsant) wants to merge 1 commit into
Closed
Santosh Todkar (todkarsant) wants to merge 1 commit into
Santosh Todkar (todkarsant) wants to merge 1 commit into
Conversation
TemplateSegmentConverter uses the ordered parameters list to assign consecutive prompt segments via dict(zip(parameters, segments)). Reversing that list changes the rendered text, but the identifier previously included only the template text hash, parameter count, and seed. Both configurations got the same registry name, preventing registration of both. Add ordered_parameters to the identifier params dict so that different orderings produce distinct identities. Fixes microsoft#3902 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the change to include ordered parameter bindings in converter identity.
No open issues 🎉
What changed in this PR
Addresses #3902 by including ordered parameter bindings in TemplateSegmentConverter identity, allowing converters with different bindings to receive distinct hashes and default registry names without changing prompt rendering.
Changes:
- Adds
ordered_parametersto identifier parameters. - Adds three tests covering different orderings, identical configurations, and the stored parameter list.
| File | Description |
|---|---|
tests/unit/converter/test_template_segment_converter.py |
Adds regression tests for order-sensitive converter identity. |
pyrit/converter/template_segment_converter.py |
Includes ordered parameter names in the identifier and explains their significance. |
Review effort: Balanced
Author
|
@microsoft-github-policy-service agree |
Contributor
|
Duplicate of #3905 |
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
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
TemplateSegmentConverteruses the orderedparameterslist to map consecutive prompt segments to template placeholders viadict(zip(parameters, segments)). Reversing the parameter list changes the rendered output, but the component identifier previously included only the template text hash, parameter count, and seed -- so two converters with different parameter orderings produced identical identities, preventing both from being registered.ordered_parametersto the identifier'sparamsdict so that different orderings yield distinct identities.ordered_parameterskey is present in the identifier params.Test plan
test_template_segment_converter.pycontinue to passordered_parameterskey is present in identifier params with correct value🤖 Generated with Claude Code