Repository navigation
FIX: Include ordered parameter names in TemplateSegmentConverter identifier (#3902) - #3907
Closed
Hrushikesh Ramilla (Hrushikesh-ramilla) wants to merge 2 commits into
Conversation
…tifier (microsoft#3902) TemplateSegmentConverter assigns prompt segments based on the ordered parameters list in SeedPrompt. Include the ordered parameter names in the converter's identifier params so that differing parameter orderings produce distinct identifier hashes and default registry names, avoiding registration collisions. Signed-off-by: Hrushikesh Ramilla <ramillahrushikesh@gmail.com>
Copilot started reviewing on behalf of
Hrushikesh Ramilla (Hrushikesh-ramilla)
November 4, 2026 05:31
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new coroutine lacks the _async suffix required by the repository’s async-function naming convention.
1 open issue
What changed in this PR
Fixes #3902 by including ordered template parameter names in TemplateSegmentConverter identity, preventing registry-name collisions between converters with different segment bindings.
Changes:
- Adds ordered parameter names to identifier generation without changing rendering behavior.
- Documents how parameter ordering affects identifiers and registry names.
- Adds regression tests for identifier contents, distinct hashes, and registry coexistence.
| File | Description |
|---|---|
tests/unit/converter/test_template_segment_converter.py |
Adds identifier and registry regression tests. |
pyrit/converter/template_segment_converter.py |
Includes ordered parameters in identity and documents the behavior. |
Review effort: Balanced
| assert second.get_identifier().params["parameters"] == ["right", "left"] | ||
|
|
||
|
|
||
| async def test_template_segment_converter_reordered_parameters_coexist_in_registry(): |
…routine Signed-off-by: Hrushikesh Ramilla <ramillahrushikesh@gmail.com>
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.
Description
Fixes #3902.
TemplateSegmentConverterbinds prompt segments to template parameters using the orderedSeedPrompt.parameterslist. When parameters are reordered, the rendered output changes, but_build_identifier()previously only included the template hash, parameter count, and seed. As a result, configurations with different parameter orderings shared the same identifier hash and default registry name, causing automatic registration collisions.This change:
"parameters": list(self.prompt_template.parameters or [])in_build_identifier()so differing parameter orderings produce distinct identifier hashes and registry names.TemplateSegmentConverter.ConverterRegistry.Tests
tests/unit/converter/test_template_segment_converter.py.test_template_segment_converter.pypass cleanly.