Skip to content

FIX: Reject mismatched normalizer templates in RoundRobinTarget - #3768

Merged
Roman Lutz (romanlutz) merged 8 commits into
microsoft:mainfrom
Kaap10:fix/round-robin-normalizer-templates-3752
Nov 5, 2026
Merged

Roman Lutz (romanlutz) merged 8 commits into
microsoft:mainfrom
Kaap10:fix/round-robin-normalizer-templates-3752

Conversation

@Kaap10

Copy link
Copy Markdown
Contributor

Description

Fixes #3752

When initializing RoundRobinTarget with inner targets that have different JSON-schema normalizer templates (configured via normalizer_overrides on TargetConfiguration), configuration validation did not catch the difference. Because TargetConfiguration.as_identifier_params()["normalization_pipeline"] previously only serialized the normalizer class qualname (f"{type(n).__module__}.{type(n).__qualname__}"), targets with distinct schema_instructions_template parameters appeared identical during consistency checking. As a result, the round-robin target adopted the first target's configuration and silently replaced the second target's schema instructions.

Changes:

  • MessageListNormalizer: Inherits from Identifiable and provides default as_identifier_params() -> dict[str, Any]: return {} and _build_identifier().
  • JsonSchemaNormalizer: Implements as_identifier_params() exporting {"schema_instructions_template": self._schema_instructions_template}.
  • ChatMessageNormalizer: Implements as_identifier_params() exporting {"use_developer_role": ..., "system_message_behavior": ...}.
  • HistorySquashNormalizer: Implements as_identifier_params() exporting child formatter identifier and message count.
  • TargetConfiguration: Adds _serialize_normalizer() so as_identifier_params()["normalization_pipeline"] reflects normalizer parameters deterministically ({"type": ..., "params": ...}).
  • RoundRobinTarget: Preserves existing validation logic while gaining parameter-level diffing across normalizers.

cc Roman Lutz (@romanlutz)

Tests and Documentation

  • Added test_init_rejects_mismatched_normalizer_templates and test_init_accepts_identical_custom_normalizer_templates in tests/unit/prompt_target/test_round_robin_target.py.
  • Added test_as_identifier_params_pipeline_distinguishes_normalizer_templates and test_as_identifier_params_pipeline_matches_when_normalizer_templates_equal in tests/unit/prompt_target/target/test_target_configuration.py.
  • Verified all unit test suites pass:
    • tests/unit/prompt_target/: 32 passed
    • tests/unit/prompt_target/target/: 35 passed
    • tests/unit/message_normalizer/: 90 passed
  • Verified linting and type checking:
    • ruff check pyrit tests (0 errors)
    • ruff format --check pyrit tests (1820 files formatted)
    • ty check (all checks passed)
  • JupyText: Documentation was not modified as this is an internal pipeline validation fix with no breaking changes to public APIs.

@Kaap10
Vardhman Gupta (Kaap10) force-pushed the fix/round-robin-normalizer-templates-3752 branch from 4714715 to 221aaf4 Compare November 1, 2026 05:32
@romanlutz Roman Lutz (romanlutz) self-assigned this Nov 1, 2026
@Kaap10

Copy link
Copy Markdown
Contributor Author

CI looks good now, up for review!

@romanlutz Roman Lutz (romanlutz) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found two issues in the custom normalizer contract, described below.

@Kaap10
Vardhman Gupta (Kaap10) force-pushed the fix/round-robin-normalizer-templates-3752 branch 3 times, most recently from 97708df to 0f21726 Compare November 1, 2026 07:42
@Kaap10

Copy link
Copy Markdown
Contributor Author

Both issues have been addressed in the latest commit Roman Lutz (@romanlutz)!

…osoft#3752)

When initializing RoundRobinTarget with targets that have different JSON-schema normalizer templates, configuration validation did not catch the difference because normalization pipeline serialization only compared class qualnames. As a result, the first target's configuration was adopted and silently replaced the second target's schema instructions.

- Preserve MessageListNormalizer(abc.ABC, Generic[T]) without Identifiable to prevent MRO conflicts in (Identifiable, MessageListNormalizer) subclasses
- Implement Identifiable in JsonSchemaNormalizer and HistorySquashNormalizer
- Implement _build_identifier() in ChatMessageNormalizer (inherits Identifiable)
- Update TargetConfiguration._serialize_normalizer() to prioritize Identifiable declared identifiers, honoring custom list normalizers' parameters
- Add unit tests verifying RoundRobinTarget rejects custom Identifiable normalizers with mismatched params and accepts identical custom normalizers
- Cover fallback and MRO compatibility scenarios with 100% diff coverage
@Kaap10
Vardhman Gupta (Kaap10) force-pushed the fix/round-robin-normalizer-templates-3752 branch from 0f21726 to d7425d1 Compare November 1, 2026 09:37

@romanlutz Roman Lutz (romanlutz) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One remaining issue affects custom normalizers that delegate to another normalizer.

@romanlutz Roman Lutz (romanlutz) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The child-identity regression is fixed. One CI-blocking test typing issue remains.

@Kaap10

Copy link
Copy Markdown
Contributor Author

CI Looks good now, Roman Lutz (@romanlutz)!

Copilot AI balanced review requested due to automatic review settings November 4, 2026 23:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@romanlutz Roman Lutz (romanlutz) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more thing I want to fix then it's merge-ready.

@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Nov 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 5, 2026
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Nov 5, 2026
Merged via the queue into microsoft:main with commit c188d21 Nov 5, 2026
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RoundRobinTarget accepts different JSON-schema normalizer templates and silently replaces the second target's instructions

3 participants