From 800e390108ec9f3de5b90301bf804495f0742339 Mon Sep 17 00:00:00 2001 From: Jamie Pine Date: Fri, 24 Apr 2026 20:36:48 -0700 Subject: [PATCH] fix(settings): honor explicit null on nullable fields, ignore on the rest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routes were calling model_dump(exclude_none=True), which drops every client-sent null before it reaches the service. The service then layered on its own `if value is not None` guard. Net effect: setting a nullable column back to null was a no-op — the MCPPage default-voice picker sends null when the user picks "no default" and the row was silently keeping whatever was there before. Switched the routes to exclude_unset=True so absent fields stay absent but explicit nulls survive the dump, and centralised the per-field nullability check in the service. The check inspects the SQLAlchemy column metadata so non-nullable columns (stt_model, llm_model, the chord key lists) still drop nulls instead of crashing the request, while default_playback_voice_id can finally be cleared. --- backend/routes/settings.py | 4 ++-- backend/services/settings.py | 26 ++++++++++++++++++++------ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/backend/routes/settings.py b/backend/routes/settings.py index 41f672eb..9ce6616c 100644 --- a/backend/routes/settings.py +++ b/backend/routes/settings.py @@ -20,7 +20,7 @@ async def update_capture_settings_endpoint( patch: models.CaptureSettingsUpdate, db: Session = Depends(get_db), ): - return settings_service.update_capture_settings(db, patch.model_dump(exclude_none=True)) + return settings_service.update_capture_settings(db, patch.model_dump(exclude_unset=True)) @router.get("/generation", response_model=models.GenerationSettingsResponse) @@ -33,4 +33,4 @@ async def update_generation_settings_endpoint( patch: models.GenerationSettingsUpdate, db: Session = Depends(get_db), ): - return settings_service.update_generation_settings(db, patch.model_dump(exclude_none=True)) + return settings_service.update_generation_settings(db, patch.model_dump(exclude_unset=True)) diff --git a/backend/services/settings.py b/backend/services/settings.py index 4171ac97..bf938ba2 100644 --- a/backend/services/settings.py +++ b/backend/services/settings.py @@ -38,6 +38,24 @@ def _get_or_create_generation_row(db: Session) -> DBGenerationSettings: return row +def _apply_patch(row: Any, patch: dict[str, Any]) -> None: + """Apply a partial update to a settings row. + + Values explicitly set to ``None`` are honored only for columns where the + schema allows it — clearing ``default_playback_voice_id`` works, but a + ``None`` for a non-nullable field is dropped rather than crashing the + request. Unknown keys are ignored. + """ + columns = type(row).__table__.columns + for key, value in patch.items(): + col = columns.get(key) + if col is None: + continue + if value is None and not col.nullable: + continue + setattr(row, key, value) + + def get_capture_settings(db: Session) -> DBCaptureSettings: """Return the capture settings row, creating it with defaults if missing.""" return _get_or_create_capture_row(db) @@ -45,9 +63,7 @@ def get_capture_settings(db: Session) -> DBCaptureSettings: def update_capture_settings(db: Session, patch: dict[str, Any]) -> DBCaptureSettings: row = _get_or_create_capture_row(db) - for key, value in patch.items(): - if value is not None and hasattr(row, key): - setattr(row, key, value) + _apply_patch(row, patch) db.commit() db.refresh(row) return row @@ -60,9 +76,7 @@ def get_generation_settings(db: Session) -> DBGenerationSettings: def update_generation_settings(db: Session, patch: dict[str, Any]) -> DBGenerationSettings: row = _get_or_create_generation_row(db) - for key, value in patch.items(): - if value is not None and hasattr(row, key): - setattr(row, key, value) + _apply_patch(row, patch) db.commit() db.refresh(row) return row