Skip to content

fix(loader): handle BaseMetadataConfig in serialize_value#10

Open
MicaPerdomo wants to merge 2 commits intocogsol:mainfrom
MicaPerdomo:fix/serialize-value-metadata-config
Open

fix(loader): handle BaseMetadataConfig in serialize_value#10
MicaPerdomo wants to merge 2 commits intocogsol:mainfrom
MicaPerdomo:fix/serialize-value-metadata-config

Conversation

@MicaPerdomo
Copy link

@MicaPerdomo MicaPerdomo commented Feb 22, 2026

Summary

Fixes #38 — filters not assigned to retrieval tools during migrations.
Fixes #39collect_classes keying retrieval tools by __name__ instead of name attr.

Problem

When a user defines filters on a retrieval (filters = [GenreMetadata, LanguageMetadata]), those filters never reach the platform. The issue had three layers:

  1. Serialization: serialize_value didn't recognize BaseMetadataConfig subclasses, falling back to repr() and producing "<class 'data.movies.metadata.GenreMetadata'>" instead of "genre".
  2. Ordering: _sync_content_with_api created retrievals before metadata configs, so filter IDs didn't exist at resolve time.
  3. Resolution: _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: Add BaseMetadataConfig to type handling. Consolidate 6 identical issubclass branches into a single _SERIALIZABLE_TYPE_BASES tuple check. Fix collect_classes to key retrieval tools by name attr instead of __name__.
  • cogsol/management/commands/migrate.py: Move metadata config upsert before retrievals. Replace _set_if_defined("filters") with proper ID resolution from new_remote["metadata_configs"], with type validation and descriptive error messages.
  • tests/test_loader.py: Add tests for BaseMetadataConfig serialization and collect_classes keying behavior.

Test plan

  • pytest — all 98 tests pass
  • Verified metadata config upsert order is correct (before retrievals)
  • Verified filter resolution follows same pattern as formatter resolution

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.
@MicaPerdomo
Copy link
Author

Related to #38 — this PR fixes the serialization step (filters stored correctly in migrations), but the full fix also requires changes in migrate.py to send the resolved filter IDs to the API.

@MicaPerdomo
Copy link
Author

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
@MicaPerdomo
Copy link
Author

Updated this PR to fully fix #38 (previously it only fixed the serialization step).

The problem: when a user defines filters on a retrieval (filters = [GenreMetadata, LanguageMetadata]), those filters never reach the platform. Three things were broken:

1. Serialization (loader.py)
serialize_value didn't have a branch for BaseMetadataConfig. When makemigrations ran, it fell back to repr() and produced ["<class 'data.movies.metadata.GenreMetadata'>"] instead of ["genre"].

2. Execution order (migrate.py)
During migrate, the code created retrievals first and metadata configs after. So even with correct filter names, the metadata config IDs wouldn't exist yet when the retrieval tried to reference them.

3. ID resolution (migrate.py)
_set_if_defined("filters") just copied the raw string values into the API payload. It never resolved filter names to their remote metadata config IDs — unlike formatters, which already had proper ID resolution.

This update fixes all three layers: serialization recognizes BaseMetadataConfig, metadata configs are created before retrievals, and filter names are resolved to remote IDs (following the same pattern as formatters).

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.

collect_classes uses inconsistent key for retrieval tools Migrations: filters not assigned to retrieval tool

1 participant