From e594a289bc82951361e50b84de0c220070964af2 Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Thu, 15 Oct 2026 11:50:35 +0530 Subject: [PATCH 1/3] fix(pii_remover): keep on_fail/stage as enums in the config model use_enum_values=True on PIIRemoverSafetyValidatorConfig coerced the inherited on_fail and stage fields to plain strings. resolve_on_fail() compares against GuardrailOnFail members, so every explicit on_fail raised "Invalid on_fail value", and config.stage.value in the log writer raised AttributeError. Drop use_enum_values and pass entity_types to Presidio as .value strings from build() instead. Add a regression test covering all three on_fail values. Closes #167 Co-Authored-By: Claude Fable 5.1 --- .../config/pii_remover_safety_validator_config.py | 8 +++----- backend/app/tests/validators/test_pii_remover.py | 11 +++++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/backend/app/core/validators/config/pii_remover_safety_validator_config.py b/backend/app/core/validators/config/pii_remover_safety_validator_config.py index fbc7e7a..de3b0fb 100644 --- a/backend/app/core/validators/config/pii_remover_safety_validator_config.py +++ b/backend/app/core/validators/config/pii_remover_safety_validator_config.py @@ -1,21 +1,19 @@ from __future__ import annotations from typing import List, Literal -from pydantic import ConfigDict - from app.core.enum import PIIEntityType from app.core.validators.pii_remover import DEFAULT_ENTITY_TYPES, PIIRemover from app.core.validators.config.base_validator_config import BaseValidatorConfig class PIIRemoverSafetyValidatorConfig(BaseValidatorConfig): - model_config = ConfigDict(use_enum_values=True) - + # No use_enum_values here: it would also coerce the inherited on_fail to a + # plain string, which resolve_on_fail() compares against GuardrailOnFail. type: Literal["pii_remover"] entity_types: List[PIIEntityType] = DEFAULT_ENTITY_TYPES def build(self): return PIIRemover( - entity_types=self.entity_types, + entity_types=[PIIEntityType(e).value for e in self.entity_types], on_fail=self.resolve_on_fail(), ) diff --git a/backend/app/tests/validators/test_pii_remover.py b/backend/app/tests/validators/test_pii_remover.py index 4dd83a9..33cd30b 100644 --- a/backend/app/tests/validators/test_pii_remover.py +++ b/backend/app/tests/validators/test_pii_remover.py @@ -90,6 +90,17 @@ def test_config_default_entity_types_passed_as_strings(mock_presidio): assert v.entity_types == DEFAULT_ENTITY_TYPES +@pytest.mark.parametrize("on_fail", ["fix", "exception", "rephrase"]) +def test_config_explicit_on_fail_resolves(mock_presidio, on_fail): + # Regression: use_enum_values on this config coerced on_fail to a str and + # resolve_on_fail() raised "Invalid on_fail value" for every stored config. + cfg = PIIRemoverSafetyValidatorConfig(type="pii_remover", on_fail=on_fail) + assert cfg.resolve_on_fail() is not None + v = cfg.build() + assert v.entity_types == DEFAULT_ENTITY_TYPES + assert all(type(e) is str for e in v.entity_types) + + def test_config_rejects_unknown_entity_type(): with pytest.raises(ValidationError): PIIRemoverSafetyValidatorConfig(type="pii_remover", entity_types=["AADHAR"]) From 76eb8cc955bbaf350f0a8db5cbceeaea13318ac2 Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Thu, 15 Oct 2026 11:54:51 +0530 Subject: [PATCH 2/3] chore(pii_remover): trim comments, clearer test variable names Co-Authored-By: Claude Fable 5.1 --- .../config/pii_remover_safety_validator_config.py | 3 +-- backend/app/tests/validators/test_pii_remover.py | 12 +++++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/backend/app/core/validators/config/pii_remover_safety_validator_config.py b/backend/app/core/validators/config/pii_remover_safety_validator_config.py index de3b0fb..6e70dd1 100644 --- a/backend/app/core/validators/config/pii_remover_safety_validator_config.py +++ b/backend/app/core/validators/config/pii_remover_safety_validator_config.py @@ -7,8 +7,7 @@ class PIIRemoverSafetyValidatorConfig(BaseValidatorConfig): - # No use_enum_values here: it would also coerce the inherited on_fail to a - # plain string, which resolve_on_fail() compares against GuardrailOnFail. + # use_enum_values would also stringify the inherited on_fail/stage enums. type: Literal["pii_remover"] entity_types: List[PIIEntityType] = DEFAULT_ENTITY_TYPES diff --git a/backend/app/tests/validators/test_pii_remover.py b/backend/app/tests/validators/test_pii_remover.py index 33cd30b..58f8413 100644 --- a/backend/app/tests/validators/test_pii_remover.py +++ b/backend/app/tests/validators/test_pii_remover.py @@ -92,13 +92,11 @@ def test_config_default_entity_types_passed_as_strings(mock_presidio): @pytest.mark.parametrize("on_fail", ["fix", "exception", "rephrase"]) def test_config_explicit_on_fail_resolves(mock_presidio, on_fail): - # Regression: use_enum_values on this config coerced on_fail to a str and - # resolve_on_fail() raised "Invalid on_fail value" for every stored config. - cfg = PIIRemoverSafetyValidatorConfig(type="pii_remover", on_fail=on_fail) - assert cfg.resolve_on_fail() is not None - v = cfg.build() - assert v.entity_types == DEFAULT_ENTITY_TYPES - assert all(type(e) is str for e in v.entity_types) + config = PIIRemoverSafetyValidatorConfig(type="pii_remover", on_fail=on_fail) + assert config.resolve_on_fail() is not None + validator = config.build() + assert validator.entity_types == DEFAULT_ENTITY_TYPES + assert all(type(entity) is str for entity in validator.entity_types) def test_config_rejects_unknown_entity_type(): From 653ee049208b83076ae3c257ce25cd4a29964ccc Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Thu, 15 Oct 2026 11:55:22 +0530 Subject: [PATCH 3/3] chore(pii_remover): name the entity_types loop variable Co-Authored-By: Claude Fable 5.1 --- .../validators/config/pii_remover_safety_validator_config.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/app/core/validators/config/pii_remover_safety_validator_config.py b/backend/app/core/validators/config/pii_remover_safety_validator_config.py index 6e70dd1..27a8c6a 100644 --- a/backend/app/core/validators/config/pii_remover_safety_validator_config.py +++ b/backend/app/core/validators/config/pii_remover_safety_validator_config.py @@ -13,6 +13,8 @@ class PIIRemoverSafetyValidatorConfig(BaseValidatorConfig): def build(self): return PIIRemover( - entity_types=[PIIEntityType(e).value for e in self.entity_types], + entity_types=[ + PIIEntityType(entity_type).value for entity_type in self.entity_types + ], on_fail=self.resolve_on_fail(), )