Skip to content

fix: stop storing JSON null in runtime_variants.default_model_definition#13009

Merged
fregataa merged 2 commits into
mainfrom
fix/runtime-variant-null-model-definition
Jul 22, 2026
Merged

fix: stop storing JSON null in runtime_variants.default_model_definition#13009
fregataa merged 2 commits into
mainfrom
fix/runtime-variant-null-model-definition

Conversation

@fregataa

Copy link
Copy Markdown
Member

Summary

  • Runtime variants created through the create API stored a JSON null in the NOT NULL default_model_definition column: the creator never set the attribute, and SQLAlchemy's JSON type serializes Python None as JSON null (not SQL NULL), slipping past the constraint. Such rows load back as None and fail the v2 node conversion with a 400 once the field became required in RuntimeVariantNode (feat(BA-6942): expose runtime variant model defaults #12970). The creator now seeds an empty ModelDefinitionDraft (stored as {}, consistent with the exclude_unset write path from fix(install): point PACKAGE scaling_group at the app-proxy coordinator #12872), and a new data migration (a0a28251f296) backfills already-materialized JSON null rows.
  • Fix the scheduling_history component conftest: MagicMock(spec=ActionValidators) rejects the rbac attribute (a dataclass field without a default is not a class attribute), which broke all 12 tests at fixture setup once SchedulingHistoryProcessors started wiring validators.rbac.scope (feat(BA-6891): add kernel scheduling-history v2 DTOs and service actions #12869). Construct a real ActionValidators with mocked RBACValidators instead, following the existing artifact_revision conftest pattern.

Test plan

  • tests/component/runtime_variant/test_runtime_variant_crud.py — previously 6/7 failing, now passes
  • tests/component/scheduling_history/test_scheduling_history.py — previously 12/12 setup errors, now passes
  • tests/unit/manager/api/adapters/test_runtime_variant_adapter.py passes
  • Migration verified on a local DB: inserted a 'null'::jsonb row, alembic upgrade head converts it to {} while leaving seeded variants untouched; downgrade -1 / re-upgrade round-trip OK

🤖 Generated with Claude Code

Runtime variants created through the create API stored a JSON null in the
NOT NULL default_model_definition column: the creator never set the
attribute and SQLAlchemy's JSON type serializes Python None as JSON null
rather than SQL NULL. Such rows load back as None and fail the required
node conversion with a 400 since the field became part of the v2 node.

- Seed an empty ModelDefinitionDraft ({} with exclude_unset) in
  RuntimeVariantCreatorSpec.build_row()
- Backfill already-materialized JSON null rows to {} via a new data
  migration (a0a28251f296)
- Fix scheduling_history component conftest: MagicMock(spec=ActionValidators)
  rejects the rbac attribute (dataclass field without default), which broke
  all tests once SchedulingHistoryProcessors started wiring
  validators.rbac.scope; construct a real ActionValidators with mocked
  RBACValidators instead

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@fregataa
fregataa requested a review from a team as a code owner July 22, 2026 00:23
Copilot AI review requested due to automatic review settings July 22, 2026 00:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added size:M 30~100 LoC comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated labels Jul 22, 2026
@fregataa
fregataa merged commit ed28f87 into main Jul 22, 2026
36 checks passed
@fregataa
fregataa deleted the fix/runtime-variant-null-model-definition branch July 22, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants