Repository navigation
Conversation
…homas#142) validate_sequence() tries each schema alternative for a list item and early-exits (re-raising immediately) whenever an alternative's error path is deeper than the item's own path -- on the assumption that a deeper path means the alternative's top-level shape matched and only a nested value is wrong, so surfacing that error immediately beats silently trying the remaining alternatives. A dict-schema alternative breaks that assumption: validate_mapping always reports 'extra keys not allowed' and 'required key not provided' errors one level deeper than the path it was given (it names the specific offending key), even when the alternative's keys have nothing to do with the data's keys at all. So for any dict alternative in a list, the heuristic fired on the very first non-matching alternative and aborted the try-next-alternative loop. Add _is_key_shape_mismatch() to recognize exactly those two error kinds at that one-level-deeper depth as shape mismatches, so the search keeps going to the next alternative instead of giving up. Any other error at that depth (e.g. a wrong-typed value for a key that genuinely matched) still aborts the search immediately, preserving the heuristic's original intent.
…tches too The list-of-dict-alternatives early-exit heuristic in validate_sequence previously only recognized plain 'extra keys not allowed' Invalid and RequiredFieldInvalid as dict-shape mismatches, via message/class inspection. This missed the case where a dict alternative's key is matched via a key validator/schema (In(...), Match(...), etc.) instead of a literal key -- such rejections surface as the key validator's own Invalid subclass at the same depth, and the old message/class-based check couldn't recognize them, so the early-exit fired incorrectly and aborted the search over remaining alternatives. Now validate_mapping tags every error that represents 'this candidate key didn't match the data's key at all' (a key validator rejecting the key, 'extra keys not allowed' for keys with no candidates, or a missing required key) with a _key_shape_mismatch marker at the point it's constructed, and _is_key_shape_mismatch simply checks for that marker. This covers all key-shape-mismatch sources generically instead of special-casing specific error messages/classes.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 119a238b4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| errors.append(er.RequiredFieldInvalid(msg, path + [key])) | ||
| missing_key_error = er.RequiredFieldInvalid(msg, path + [key]) | ||
| missing_key_error._key_shape_mismatch = True |
There was a problem hiding this comment.
When a dict alternative uses the supported Required(Any(...)) complex-key form and none of its candidate keys are present, the earlier complex-required check appends an untagged RequiredFieldInvalid. Because _is_key_shape_mismatch() requires every error to carry this tag, validation stops at that alternative instead of trying later ones; for example, Schema([{Required(Any('a', 'b')): str}, {'c': str}])([{'c': 'x'}]) is incorrectly rejected. The complex-required error needs the same shape-mismatch marking applied here to ordinary missing keys.
Useful? React with 👍 / 👎.
This branch has not been deployed
2 participants
Summary
Instead of trying to reverse-engineer 'is this a key-shape-mismatch error' from an error's class/message downstream, validate_mapping now tags every error it constructs that genuinely represents 'this candidate key didn't match the data's key at all' with a
_key_shape_mismatch = Truemarker attribute, at each of the three points such errors are created: (1) when a key candidate's ckey() call rejects the key (covers key validators like In/Match generically, not just literal keys), (2) when no candidate matched and 'extra keys not allowed' is raised, (3) when a required key is missing entirely. _is_key_shape_mismatch in schema_builder.py now just checks for that marker plus depth, so it generically covers every source of 'key didn't match' rather than special-casing specific error text/classes. Value-validation errors (a key that WAS matched but whose value is wrong) are never tagged, so the heuristic still correctly aborts the alternative search for genuine value-type errors like Case 1 in the issue.Problem
alecthomas/voluptuous issue reference: #142
Root Cause
In _compile_sequence's validate_sequence (schema_builder.py), the loop over schema alternatives for a list item aborts early via
if len(e.path) > len(index_path): raisewhenever an alternative's error path is one level deeper than the item's own path -- intended to mean 'top-level shape matched, nested detail is wrong, surface it immediately'. But validate_mapping (the dict validator) always reports key-related failures ('extra keys not allowed', missing required key, or a key validator/schema like In()/Match() rejecting the key) exactly one level deeper than the path it was given, even when the alternative's shape doesn't match the data at all. The prior attempt patched this by recognizing two specific error shapes (plain 'extra keys not allowed' Invalid and RequiredFieldInvalid) via message/class inspection in a new _is_key_shape_mismatch helper, but this missed the case where a dict alternative uses a key validator (In, Match, etc.) instead of a literal key: that rejection surfaces as the key validator's own differently-classed, differently-worded Invalid (e.g. InInvalid) at the same depth, which the message/class-based check couldn't recognize as a shape mismatch.Testing
PASS - full suite: 183 passed (including tests.md doctests); test_fix_142_list_of_dict_alternatives extended to cover: Case 0 (issue's reported bug) success, Case 1 (genuine value-type error) still correctly raised, 3-alternative case, Required-key shape mismatch, and the new key-validator cases (In and Match, both success and a real value-error-not-swallowed case). The 2 pre-existing --doctest-modules failures (Schema._compile_dict, Remove) were confirmed identical on both the base commit and this change via git stash comparison, so they are unrelated pre-existing issues, not a regression.
Related Issue
#142