Conversation
A config-model field whose schema default is not None survives the loader's exclude_none dump, so it is passed to the constructor even when the YAML entry omits the key, overriding the class default with whatever the schema happens to say. The two defaults live in different files with nothing tying them together. That is how #2254 happened: CustomRecognizerConfig.deny_list_score defaulted to 0.0 against PatternRecognizer's 1.0, so every YAML-defined deny list scored 0.0 and detected nothing. #2254 fixed the value; nothing stops the pair drifting apart again. Two tests, both in the M0 conformance suite: - test_schema_default_matches_constructor_default, parametrized over every config model field that has a non-None default and is reachable on the constructor it configures. Pairs come from CONFIG_MODEL_MAP plus CustomRecognizerConfig -> PatternRecognizer. One case today (deny_list_score); the parametrization grows as fields are added. Verified to fail, with a message naming both defaults, when the pre-#2254 value is reinstated. - test_shared_config_models_declare_no_reaching_defaults for BaseRecognizerConfig and PredefinedRecognizerConfig, which validate entries for every predefined recognizer and so have no single constructor to compare against: a non-None default there is imposed on all of them at once. Registry-only keys the loader strips before construction are exempt. Tests only, no behavior change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
omri374
force-pushed
the
omri/config-03-schema-default-drift-guard
branch
from
September 17, 2026 18:33
0fa8c67 to
7ab52a9
Compare
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.
Change Description
Adds two tests to the M0 conformance suite that pin every config-model default against the constructor default it can override. Tests only, no behavior change.
A config-model field whose schema default is not
Nonesurvives the loader'sexclude_nonedump, so it is passed to the constructor even when the YAML entry omits the key, overriding the class default with whatever the schema happens to say. The two defaults live in different files with nothing tying them together.That is exactly how the
deny_list_scorebug happened:CustomRecognizerConfig.deny_list_scoredefaulted to0.0againstPatternRecognizer's1.0, so every YAML-defined deny list scored 0.0 and detected nothing. #2254 fixed the value. Nothing stops the pair drifting apart again, and no existing test compares them.Tests added
test_schema_default_matches_constructor_default, parametrized over every config-model field that has a non-Nonedefault and is reachable on the constructor it configures. Pairs come fromCONFIG_MODEL_MAPplusCustomRecognizerConfigtoPatternRecognizer. One case today (deny_list_score); the parametrization grows on its own as fields are added. Verified to fail, with a message naming both defaults, when the pre-fix(analyzer): omitted deny_list_score in YAML registry now defaults to 1.0 #2254 value is reinstated:test_shared_config_models_declare_no_reaching_defaultsforBaseRecognizerConfigandPredefinedRecognizerConfig. These validate entries for every predefined recognizer, so there is no single constructor to compare against: a non-Nonedefault there is imposed on all of them at once.Registry-only keys the loader strips before construction (
enabled,type,class_name,score_thresholds,supported_languages,country_code) are exempt, since a default for one of them can never reach a constructor.Why this is a guard and not a fix
The invariant holds today only because the two defaults happen to hold the same literal. The structural fix is presence-aware application, where omitting a key means "keep the constructor default" regardless of what the schema says. That is module M1 in the ADR. Until then this test is what keeps the pair honest, and it is cheap.
Verification
3800 passed, 13 skipped, 1 xfailed.config_hardeningaftermainwas merged into it, so the diff is one test file.ruff checkfrom the repository root, the way CI runs it: clean.Issue reference
Part of the ADR at https://github.com/data-privacy-stack/presidio-product-core/issues/139 (module M0, the conformance suite). Follows PR #2248.
Checklist
🤖 Generated with Claude Code