Skip to content

fix: resolve IndexSchema fields type annotation inconsistency (#361)#381

Merged
bsbodden merged 1 commit into
mainfrom
bsb/issue-361
Sep 25, 2025
Merged

fix: resolve IndexSchema fields type annotation inconsistency (#361)#381
bsbodden merged 1 commit into
mainfrom
bsb/issue-361

Conversation

@bsbodden

Copy link
Copy Markdown
Contributor

Fix conflicting type expectations in IndexSchema constructor and validation.
The type annotation declared fields: Dict[str, BaseField] but the validator
raised an error when fields was actually provided as a dict.

Changes:

  • Update model validator to accept both list and dict formats for fields
  • Support dict with field definitions: {"field_name": {"name": "field_name", "type": "text"}}
  • Support dict with BaseField instances: {"field_name": BaseFieldInstance}
  • Maintain backward compatibility with existing list format

@bsbodden bsbodden requested a review from abrookins September 23, 2025 22:11

@abrookins abrookins left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Some non-blocking comments and ideas for your consideration. 👍

Comment thread redisvl/schema/schema.py Outdated
Comment thread redisvl/schema/schema.py
Comment thread redisvl/schema/schema.py
Comment thread redisvl/schema/schema.py
Comment thread redisvl/schema/schema.py Outdated
  Fix conflicting type expectations in IndexSchema constructor and validation.
  The type annotation declared `fields: Dict[str, BaseField]` but the validator
  raised an error when fields was actually provided as a dict.

  Changes:
  - Update model validator to accept both list and dict formats for fields
  - Support dict with field definitions: {"field_name": {"name": "field_name", "type": "text"}}
  - Support dict with BaseField instances: {"field_name": BaseFieldInstance}
  - Maintain backward compatibility with existing list format
  - Use Field(default_factory=dict) instead of mutable default {}
  - Change fields type annotation from Dict to Mapping for better type safety
  - Use collections.abc.Mapping for isinstance checks
  - Use Sequence check with str/bytes exclusion for list detection
  - Add field name validation when BaseField is provided directly in dict format

Co-authored-by: Andrew Brookins <andrew@klaviyo.com>
@bsbodden bsbodden merged commit 42aa83b into main Sep 25, 2025
45 checks passed
@bsbodden bsbodden deleted the bsb/issue-361 branch September 25, 2025 21:54
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.

3 participants