[BUG] Settings update via PUT /api/models/{id} overwrites entire settings dict instead of merging #17

New issue

Closed

opened 2026-05-08 23:46:33 +00:00 by Mark · 7 comments

Mark commented

2026-05-08 23:46:33 +00:00

Owner

Summary

When updating model settings via PUT /api/models/{id}, the ModelRepository.update() method directly sets record.settings = value with the raw incoming dict, which completely replaces the existing settings rather than merging with the current values. This means if the frontend sends only a partial settings update, all un-sent fields are lost.

Affected File

backend/app/repositories/model_repository.py lines 104-115

Root Cause

update_data = payload.model_dump(exclude_unset=True, by_alias=True)
for key, value in update_data.items():
    if key == "settings" and value:
        setattr(record, "settings", value)  # Full replacement!
    ...

When exclude_unset=True is used, only provided keys are included. But the settings field is replaced wholesale rather than deep-merged with existing settings.

Impact

Any partial update to a model (e.g., changing just the name) that includes the settings field will wipe all existing settings. If the frontend sends partial settings (which it does via vllmSettingsToApi() which drops many fields), the dropped fields are permanently removed from the database.

Expected Behavior

Settings should be deep-merged with existing values, not replaced:

if key == "settings" and value and isinstance(value, dict):
    existing = record.settings or {}
    existing.update(value)
    setattr(record, "settings", existing)

## Summary When updating model settings via `PUT /api/models/{id}`, the `ModelRepository.update()` method directly sets `record.settings = value` with the raw incoming dict, which **completely replaces** the existing settings rather than merging with the current values. This means if the frontend sends only a partial settings update, all un-sent fields are lost. ## Affected File `backend/app/repositories/model_repository.py` lines 104-115 ## Root Cause ```python update_data = payload.model_dump(exclude_unset=True, by_alias=True) for key, value in update_data.items(): if key == "settings" and value: setattr(record, "settings", value) # Full replacement! ... ``` When `exclude_unset=True` is used, only provided keys are included. But the settings field is replaced wholesale rather than deep-merged with existing settings. ## Impact Any partial update to a model (e.g., changing just the name) that includes the settings field will wipe all existing settings. If the frontend sends partial settings (which it does via `vllmSettingsToApi()` which drops many fields), the dropped fields are permanently removed from the database. ## Expected Behavior Settings should be deep-merged with existing values, not replaced: ```python if key == "settings" and value and isinstance(value, dict): existing = record.settings or {} existing.update(value) setattr(record, "settings", existing) ```