fix(loader): handle BaseMetadataConfig in serialize_value#10
fix(loader): handle BaseMetadataConfig in serialize_value#10MicaPerdomo wants to merge 2 commits intocogsol:mainfrom
Conversation
The serialize_value function handles BaseRetrieval, BaseReferenceFormatter, BaseTopic, BaseIngestionConfig, and BaseRetrievalTool subclasses, but does not handle BaseMetadataConfig. When a retrieval defines filters as a list of metadata config classes, they fall through to repr(value), producing strings like "<class 'data.movies.metadata.GenreMetadata'>" instead of the config name. Add an explicit issubclass check for BaseMetadataConfig alongside the existing content-layer type checks.
|
Related to #38 — this PR fixes the serialization step (filters stored correctly in migrations), but the full fix also requires changes in |
|
Also fixes #39. |
…ig upsert
- Consolidate serialize_value type checks into _SERIALIZABLE_TYPE_BASES tuple
- Move metadata config upsert before retrievals so filter IDs exist at resolve time
- Replace _set_if_defined("filters") with proper ID resolution from new_remote
- Add type validation for filters field consistent with formatters pattern
- Use name attr as collect_classes key for retrieval tools (fixes cogsol#39)
- Add tests for collect_classes keying behavior
|
Updated this PR to fully fix #38 (previously it only fixed the serialization step). The problem: when a user defines filters on a retrieval ( 1. Serialization ( 2. Execution order ( 3. ID resolution ( This update fixes all three layers: serialization recognizes |
Summary
Fixes #38 — filters not assigned to retrieval tools during migrations.
Fixes #39 —
collect_classeskeying retrieval tools by__name__instead ofnameattr.Problem
When a user defines filters on a retrieval (
filters = [GenreMetadata, LanguageMetadata]), those filters never reach the platform. The issue had three layers:serialize_valuedidn't recognizeBaseMetadataConfigsubclasses, falling back torepr()and producing"<class 'data.movies.metadata.GenreMetadata'>"instead of"genre"._sync_content_with_apicreated retrievals before metadata configs, so filter IDs didn't exist at resolve time._set_if_defined("filters")copied raw string values to the API payload without resolving them to remote metadata config IDs.Changes
cogsol/core/loader.py: AddBaseMetadataConfigto type handling. Consolidate 6 identicalissubclassbranches into a single_SERIALIZABLE_TYPE_BASEStuple check. Fixcollect_classesto key retrieval tools bynameattr instead of__name__.cogsol/management/commands/migrate.py: Move metadata config upsert before retrievals. Replace_set_if_defined("filters")with proper ID resolution fromnew_remote["metadata_configs"], with type validation and descriptive error messages.tests/test_loader.py: Add tests forBaseMetadataConfigserialization andcollect_classeskeying behavior.Test plan
pytest— all 98 tests pass