Skip to content

FIX: validate pattern, replacement choices, and flags in SearchReplaceConverter - #3152

Merged
Roman Lutz (romanlutz) merged 3 commits into
microsoft:mainfrom
RohithPariki:fix/validate-search-replace-converter-parameters
Oct 13, 2026
Merged

Roman Lutz (romanlutz) merged 3 commits into
microsoft:mainfrom
RohithPariki:fix/validate-search-replace-converter-parameters

Conversation

@RohithPariki

@RohithPariki RohithPariki commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Description

Validate SearchReplaceConverter configuration during construction instead of waiting for conversion to fail. Include regex flags in the converter's identifier so configurations with different matching behavior have different identities.

Changes

  • Require a string pattern and compile it with regex_flags during initialization. Invalid regex syntax raises a descriptive ValueError.
  • Preserve valid empty regex patterns. For example, replacing "" with "-" in "ab" still produces "-a-b-".
  • Require replace to be a string or a non-empty list of strings. Copy replacement lists so later caller changes do not affect the converter.
  • Require integer regex flags, reject booleans, and include the flags in the identifier.
  • Use the compiled regex for substitution and merge the latest upstream main.

Previously, an empty replacement list failed at runtime with IndexError, and invalid replacement types or regex patterns failed during substitution. These invalid configurations now fail during initialization. Empty replacement strings remain supported.

Tests and Documentation

Added tests for invalid patterns, replacement choices, flag types, flag-aware execution, identifiers, and empty replacements. Added four regression cases for valid empty patterns, covering non-empty and empty prompts, an empty replacement, and a replacement list. All four regression cases failed before the compatibility fix and pass afterward.

Updated the constructor docstring to document empty-pattern support and validation errors.

Validation on Windows with Python 3.14.4, after merging main:

Targeted tests: 606 passed, 6 skipped.

uv run --no-sync python -m pytest tests\unit\converter\test_search_replace_converter.py tests\unit\converter\test_prompt_template_converter.py tests\unit\converter\test_converter.py tests\unit\converter\test_seeded_converter_determinism.py tests\unit\registry\test_resolution.py tests\unit\setup\test_converter_initializer.py tests\unit\backend\test_converter_service.py -q

Ruff lint: passed.

uv run --no-sync python -m ruff check pyrit\converter\search_replace_converter.py tests\unit\converter\test_search_replace_converter.py

Ruff formatting: passed; both files already formatted.

uv run --no-sync python -m ruff format --check pyrit\converter\search_replace_converter.py tests\unit\converter\test_search_replace_converter.py

Type checking: passed.

uv run --no-sync python -m ty check pyrit\converter\search_replace_converter.py tests\unit\converter\test_search_replace_converter.py

The full test suite and other OS/Python combinations were not run locally.

JupyText was not run because this change does not modify notebooks or notebook examples.

Comment thread pyrit/converter/search_replace_converter.py Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a42d9bc0-29f5-4def-8991-0d02f510f1f7
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a42d9bc0-29f5-4def-8991-0d02f510f1f7
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 13, 2026
Merged via the queue into microsoft:main with commit a2398d3 Oct 13, 2026
55 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.

2 participants