fix: preserve previous settings when saving fails

.github/workflows/request-validation.yml

@@ -1,4 +1,4 @@
-name: Request validation
+name: Unit tests and Compose
 
 on:
   pull_request:
@@ -25,10 +25,10 @@ jobs:
       - uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0
         with:
           python-version: ${{ matrix.python-version }}
-      - name: Install request-test dependencies
+      - name: Install unit-test dependencies
         run: python -m pip install --disable-pip-version-check --no-cache-dir -r requirements-test.txt
-      - name: Test request validation
-        run: python -m pytest tests/test_search_request.py -q
+      - name: Test lightweight units
+        run: python -m pytest tests -q
       - name: Validate Compose configuration
         run: |
           cp .env.example .env

README.md

@@ -142,11 +142,11 @@ This is an example request, not a recorded result. Search responses include medi
 - Speech alignment may fall back to English when a language-specific model cannot load; this can affect timestamps and transcript quality.
 - Dependencies, model caches, GPU compatibility, and the separately hosted LLM must be validated together. These setup instructions are derived from source inspection; they are not a claim that a clean build or full inference run has passed.
 
-The request-validation tests run without model assets or a running service. In an isolated environment, run:
+The request-validation and settings-persistence tests run without model assets or a running service. In an isolated environment, run:
 
 ```bash
 python -m pip install -r requirements-test.txt
-python -m pytest tests/test_search_request.py
+python -m pytest tests
 ```
 
-GitHub Actions runs these tests and validates the Compose configuration on Python 3.11 and 3.13 for pull requests and pushes to `main`. It does not build the inference image or start services.
+GitHub Actions runs these tests and validates the Compose configuration on Python 3.11 and 3.13 for pull requests and pushes to `main`. The settings tests cover successful saves, serialization and write failures, replacement failures, and Unicode round trips. The workflow does not build the inference image or start services.

app/core/config.py

@@ -1,5 +1,7 @@
 import os
 import json
+import tempfile
+from pathlib import Path
 from pydantic import BaseModel
 from dotenv import load_dotenv
 
@@ -39,7 +41,23 @@ def load_settings() -> LLMSettings:
     )
 
 def save_settings(settings: LLMSettings):
-    with open(SETTINGS_FILE, "w", encoding="utf-8") as f:
-        f.write(settings.model_dump_json(indent=4))
+    payload = settings.model_dump_json(indent=4)
+    temporary_path = None
+    try:
+        with tempfile.NamedTemporaryFile(
+            mode="w",
+            encoding="utf-8",
+            dir=os.path.dirname(os.path.abspath(SETTINGS_FILE)),
+            prefix=".settings-",
+            suffix=".tmp",
+            delete=False,
+        ) as temporary:
+            temporary_path = temporary.name
+            temporary.write(payload)
+        os.replace(temporary_path, SETTINGS_FILE)
+        temporary_path = None
+    finally:
+        if temporary_path is not None:
+            Path(temporary_path).unlink(missing_ok=True)
 
 llm_settings = load_settings()

requirements-test.txt

@@ -1,2 +1,3 @@
 pydantic>=2,<3
 pytest>=8,<10
+python-dotenv>=1,<2

tests/test_settings_persistence.py

@@ -0,0 +1,101 @@
+import json
+from contextlib import contextmanager
+from tempfile import NamedTemporaryFile
+from unittest.mock import patch
+
+import pytest
+
+from app.core import config
+
+
+@pytest.fixture
+def settings_path(tmp_path, monkeypatch):
+    path = tmp_path / "settings.json"
+    monkeypatch.setattr(config, "SETTINGS_FILE", str(path))
+    return path
+
+
+def test_saved_settings_replace_the_previous_document(settings_path):
+    settings_path.write_text('{"provider": "local", "api_key": "previous-value"}')
+    settings = config.LLMSettings(
+        provider="custom", model="local-model", api_key="new-value"
+    )
+
+    config.save_settings(settings)
+
+    assert config.load_settings() == settings
+    assert json.loads(settings_path.read_text())["api_key"] == "new-value"
+    assert list(settings_path.parent.glob(".settings-*.tmp")) == []
+
+
+def test_serialization_failure_preserves_previous_settings(settings_path):
+    previous = '{"provider": "local", "api_key": "previous-value"}'
+    settings_path.write_text(previous)
+
+    with patch.object(
+        config.LLMSettings, "model_dump_json", side_effect=ValueError("serialize")
+    ):
+        with pytest.raises(ValueError, match="serialize"):
+            config.save_settings(config.LLMSettings())
+
+    assert settings_path.read_text() == previous
+    assert list(settings_path.parent.glob(".settings-*.tmp")) == []
+
+
+def test_replace_failure_preserves_previous_settings(settings_path):
+    previous = '{"provider": "local", "api_key": "previous-value"}'
+    settings_path.write_text(previous)
+
+    with patch("os.replace", side_effect=OSError("replace failed")):
+        with pytest.raises(OSError, match="replace failed"):
+            config.save_settings(config.LLMSettings(api_key="new-value"))
+
+    assert settings_path.read_text() == previous
+    assert list(settings_path.parent.glob(".settings-*.tmp")) == []
+
+
+def test_partial_write_failure_preserves_previous_settings(settings_path):
+    previous = '{"provider": "local", "api_key": "previous-value"}'
+    settings_path.write_text(previous)
+
+    @contextmanager
+    def failing_temporary_file(*args, **kwargs):
+        with NamedTemporaryFile(*args, **kwargs) as temporary:
+
+            class PartialWriter:
+                name = temporary.name
+
+                def write(self, payload):
+                    temporary.write(payload[:5])
+                    temporary.flush()
+                    raise OSError("write failed")
+
+            yield PartialWriter()
+
+    with patch("tempfile.NamedTemporaryFile", failing_temporary_file):
+        with pytest.raises(OSError, match="write failed"):
+            config.save_settings(config.LLMSettings(api_key="new-value"))
+
+    assert settings_path.read_text() == previous
+    assert list(settings_path.parent.glob(".settings-*.tmp")) == []
+
+
+def test_first_save_creates_a_complete_document(settings_path):
+    settings = config.LLMSettings(model="local-model")
+    config.save_settings(settings)
+    assert config.load_settings() == settings
+
+
+def test_failed_first_serialization_does_not_create_an_empty_file(settings_path):
+    with patch.object(
+        config.LLMSettings, "model_dump_json", side_effect=ValueError("serialize")
+    ):
+        with pytest.raises(ValueError, match="serialize"):
+            config.save_settings(config.LLMSettings())
+    assert not settings_path.exists()
+
+
+def test_unicode_settings_round_trip(settings_path):
+    settings = config.LLMSettings(model="نموذج محلي")
+    config.save_settings(settings)
+    assert config.load_settings() == settings