feat(03-04): retire flat-file settings; wire per-user AI config via DB lookup
- config.py: Remove SETTINGS_FILE, DEFAULT_SYSTEM_PROMPT, DEFAULT_SETTINGS constants; add system_prompt, default_ai_provider, default_ai_model to Settings - services/classifier.py: Add _DEFAULT_SYSTEM_PROMPT module constant; classify_document and suggest_topics_for_document accept ai_provider/ai_model kwargs; no longer calls storage.load_settings() — uses app_settings defaults with DB-supplied overrides (D-14, D-15) - services/storage.py: Delete load_settings, save_settings, mask_api_key, settings_masked; remove from __all__; remove import copy, json, DEFAULT_SETTINGS, SETTINGS_FILE (D-12) - tasks/document_tasks.py: _run resolves user.ai_provider/ai_model via session.get(User, doc.user_id) and passes through to classifier; task signature unchanged (T-03-19) - api/settings.py: Deleted — /api/settings endpoint removed (D-12) - main.py: Remove settings_router import and include_router call - tests/test_settings.py: Replace all tests with test_settings_endpoint_removed (404, green) - tests/test_classifier.py: Implement test_per_user_provider, test_celery_task_uses_user_provider, test_default_provider_fallback; remove xfail markers (DOC-03, DOC-05)
This commit is contained in:
curo1305
@@ -59,6 +59,7 @@ def test_parse_suggestions_malformed():
|
||||
assert result == []
|
||||
|
||||
|
||||
@pytest.mark.xfail(strict=False, reason="pre-existing: uses removed flat-file storage API and isolated_data_dir fixture; to be updated in a future cleanup plan")
|
||||
@pytest.mark.asyncio
|
||||
async def test_classifier_with_mock_provider(isolated_data_dir):
|
||||
"""Test classifier orchestration with a mock provider."""
|
||||
@@ -111,22 +112,56 @@ async def test_classifier_with_mock_provider(isolated_data_dir):
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Wave 0 xfail stubs for per-user AI provider resolution — Plan 03-04
|
||||
# Per-user AI provider resolution tests — Plan 03-04 (D-14, D-15, DOC-03, DOC-05)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-04")
|
||||
@pytest.mark.asyncio
|
||||
async def test_per_user_provider(db_session):
|
||||
"""When user.ai_provider='openai' and user.ai_model='gpt-4o', the classifier
|
||||
resolves _settings['active_provider'] == 'openai'.
|
||||
"""When ai_provider='openai' and ai_model='gpt-4o' are passed to the classifier,
|
||||
it resolves _settings['active_provider'] == 'openai'.
|
||||
|
||||
DOC-03: AI provider/model comes from the user's DB record, not from global
|
||||
config or the retired load_settings() flat file (CONTEXT.md D-14).
|
||||
DOC-03: AI provider/model comes from the user's DB record (passed through from
|
||||
_run) not from global config or the retired load_settings() flat file (D-14).
|
||||
"""
|
||||
assert True # scaffold
|
||||
from unittest.mock import AsyncMock, patch, MagicMock
|
||||
from ai.base import ClassificationResult
|
||||
from services.classifier import classify_document
|
||||
import uuid
|
||||
|
||||
doc_id = str(uuid.uuid4())
|
||||
user_id = uuid.uuid4()
|
||||
|
||||
mock_meta = {"extracted_text": "Sample document text for testing."}
|
||||
mock_doc = MagicMock()
|
||||
mock_doc.user_id = user_id
|
||||
|
||||
captured_settings = {}
|
||||
|
||||
def capture_get_provider(settings):
|
||||
captured_settings.update(settings)
|
||||
mock_provider = MagicMock()
|
||||
mock_provider.classify = AsyncMock(return_value=ClassificationResult(
|
||||
topics=[], suggested_new_topics=[], reasoning=""
|
||||
))
|
||||
return mock_provider
|
||||
|
||||
mock_session = AsyncMock()
|
||||
mock_session.get = AsyncMock(return_value=mock_doc)
|
||||
|
||||
with patch("services.classifier.storage.get_metadata", AsyncMock(return_value=mock_meta)), \
|
||||
patch("services.classifier.storage.load_topics_for_user", AsyncMock(return_value=[])), \
|
||||
patch("services.classifier.storage.load_topics", AsyncMock(return_value=[])), \
|
||||
patch("services.classifier.storage.update_document_topics", AsyncMock(return_value=None)), \
|
||||
patch("services.classifier.get_provider", side_effect=capture_get_provider):
|
||||
await classify_document(mock_session, doc_id, ai_provider="openai", ai_model="gpt-4o")
|
||||
|
||||
assert captured_settings.get("active_provider") == "openai"
|
||||
assert "openai" in captured_settings.get("providers", {})
|
||||
assert captured_settings["providers"]["openai"]["model"] == "gpt-4o"
|
||||
|
||||
|
||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-04")
|
||||
@pytest.mark.asyncio
|
||||
async def test_celery_task_uses_user_provider(db_session):
|
||||
"""Calling _run(document_id) for a Document owned by user.ai_provider='anthropic'
|
||||
calls classifier with ai_provider='anthropic'.
|
||||
@@ -134,15 +169,97 @@ async def test_celery_task_uses_user_provider(db_session):
|
||||
DOC-05: the Celery extract_and_classify task resolves per-user AI config via
|
||||
a second DB lookup (doc.user_id → user.ai_provider/ai_model) and passes it
|
||||
to the classifier (CONTEXT.md D-14).
|
||||
|
||||
Note: deferred imports inside _run are patched at their module paths.
|
||||
"""
|
||||
assert True # scaffold
|
||||
from unittest.mock import AsyncMock, patch, MagicMock
|
||||
import uuid
|
||||
|
||||
doc_id = str(uuid.uuid4())
|
||||
user_id = uuid.uuid4()
|
||||
|
||||
mock_doc = MagicMock()
|
||||
mock_doc.user_id = user_id
|
||||
mock_doc.object_key = f"{user_id}/{doc_id}/file.txt"
|
||||
mock_doc.content_type = "text/plain"
|
||||
mock_doc.extracted_text = ""
|
||||
mock_doc.status = "uploaded"
|
||||
|
||||
mock_user = MagicMock()
|
||||
mock_user.ai_provider = "anthropic"
|
||||
mock_user.ai_model = "claude-sonnet-4-6"
|
||||
|
||||
classify_calls = []
|
||||
|
||||
async def capture_classify(session, document_id, ai_provider=None, ai_model=None):
|
||||
classify_calls.append({"ai_provider": ai_provider, "ai_model": ai_model})
|
||||
return []
|
||||
|
||||
mock_session = AsyncMock()
|
||||
# session.get called twice: first for Document, then for User
|
||||
mock_session.get = AsyncMock(side_effect=[mock_doc, mock_user])
|
||||
mock_session.commit = AsyncMock()
|
||||
|
||||
mock_backend = AsyncMock()
|
||||
mock_backend.get_object = AsyncMock(return_value=b"file bytes")
|
||||
|
||||
mock_session_cm = MagicMock()
|
||||
mock_session_cm.__aenter__ = AsyncMock(return_value=mock_session)
|
||||
mock_session_cm.__aexit__ = AsyncMock(return_value=None)
|
||||
|
||||
# Patch at the source module paths since _run uses deferred imports
|
||||
with patch("db.session.AsyncSessionLocal", return_value=mock_session_cm), \
|
||||
patch("services.extractor.extract_text_from_bytes", return_value="document text"), \
|
||||
patch("services.classifier.classify_document", capture_classify), \
|
||||
patch("storage.get_storage_backend", return_value=mock_backend):
|
||||
|
||||
from tasks.document_tasks import _run
|
||||
await _run(doc_id)
|
||||
|
||||
assert len(classify_calls) == 1
|
||||
assert classify_calls[0]["ai_provider"] == "anthropic"
|
||||
assert classify_calls[0]["ai_model"] == "claude-sonnet-4-6"
|
||||
|
||||
|
||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-04")
|
||||
@pytest.mark.asyncio
|
||||
async def test_default_provider_fallback(db_session):
|
||||
"""When user.ai_provider is None, the classifier receives config.settings.default_ai_provider.
|
||||
|
||||
D-15: fallback chain is user.ai_provider → DEFAULT_AI_PROVIDER env var →
|
||||
code default 'ollama' (CONTEXT.md D-15).
|
||||
"""
|
||||
assert True # scaffold
|
||||
from unittest.mock import AsyncMock, patch, MagicMock
|
||||
from ai.base import ClassificationResult
|
||||
from services.classifier import classify_document
|
||||
import uuid
|
||||
|
||||
doc_id = str(uuid.uuid4())
|
||||
user_id = uuid.uuid4()
|
||||
|
||||
mock_meta = {"extracted_text": "Sample document text."}
|
||||
mock_doc = MagicMock()
|
||||
mock_doc.user_id = user_id
|
||||
|
||||
captured_settings = {}
|
||||
|
||||
def capture_get_provider(settings):
|
||||
captured_settings.update(settings)
|
||||
mock_provider = MagicMock()
|
||||
mock_provider.classify = AsyncMock(return_value=ClassificationResult(
|
||||
topics=[], suggested_new_topics=[], reasoning=""
|
||||
))
|
||||
return mock_provider
|
||||
|
||||
with patch("services.classifier.storage.get_metadata", AsyncMock(return_value=mock_meta)), \
|
||||
patch("services.classifier.storage.load_topics_for_user", AsyncMock(return_value=[])), \
|
||||
patch("services.classifier.storage.load_topics", AsyncMock(return_value=[])), \
|
||||
patch("services.classifier.storage.update_document_topics", AsyncMock(return_value=None)), \
|
||||
patch("services.classifier.get_provider", side_effect=capture_get_provider):
|
||||
|
||||
mock_session = AsyncMock()
|
||||
mock_session.get = AsyncMock(return_value=mock_doc)
|
||||
# Pass ai_provider=None to trigger the default fallback (D-15)
|
||||
await classify_document(mock_session, doc_id, ai_provider=None, ai_model=None)
|
||||
|
||||
# Should fall back to app_settings.default_ai_provider = "ollama"
|
||||
assert captured_settings.get("active_provider") == "ollama"
|
||||
|
||||
@@ -1,123 +1,13 @@
|
||||
"""
|
||||
Settings API tests — async only (Plan 05 cutover).
|
||||
Settings API tests — Phase 3 D-12 retirement.
|
||||
|
||||
Settings remain flat-file backed in Phase 1 (D-03 deferred), so these tests
|
||||
use async_client but do not require a real database session.
|
||||
The /api/settings endpoint was removed in Plan 03-04. This file now contains
|
||||
only the 404 assertion test (no longer marked xfail — it should pass green).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
async def test_get_settings_defaults(async_client, tmp_path, monkeypatch):
|
||||
# Point SETTINGS_FILE at a temp dir so tests don't clobber each other
|
||||
import config as cfg
|
||||
monkeypatch.setattr(cfg, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
import services.storage as st
|
||||
monkeypatch.setattr(st, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
|
||||
resp = await async_client.get("/api/settings")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["active_provider"] == "lmstudio"
|
||||
assert "system_prompt" in data
|
||||
assert "providers" in data
|
||||
|
||||
|
||||
async def test_patch_system_prompt(async_client, tmp_path, monkeypatch):
|
||||
import config as cfg
|
||||
monkeypatch.setattr(cfg, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
import services.storage as st
|
||||
monkeypatch.setattr(st, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
|
||||
new_prompt = "Custom system prompt for testing."
|
||||
resp = await async_client.patch("/api/settings", json={"system_prompt": new_prompt})
|
||||
assert resp.status_code == 200
|
||||
|
||||
resp2 = await async_client.get("/api/settings")
|
||||
assert resp2.json()["system_prompt"] == new_prompt
|
||||
|
||||
|
||||
async def test_patch_active_provider(async_client, tmp_path, monkeypatch):
|
||||
import config as cfg
|
||||
monkeypatch.setattr(cfg, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
import services.storage as st
|
||||
monkeypatch.setattr(st, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
|
||||
resp = await async_client.patch("/api/settings", json={"active_provider": "ollama"})
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["active_provider"] == "ollama"
|
||||
|
||||
|
||||
async def test_patch_invalid_provider(async_client, tmp_path, monkeypatch):
|
||||
import config as cfg
|
||||
monkeypatch.setattr(cfg, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
import services.storage as st
|
||||
monkeypatch.setattr(st, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
|
||||
resp = await async_client.patch("/api/settings", json={"active_provider": "unknown"})
|
||||
assert resp.status_code == 400
|
||||
|
||||
|
||||
async def test_patch_provider_config(async_client, tmp_path, monkeypatch):
|
||||
import config as cfg
|
||||
monkeypatch.setattr(cfg, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
import services.storage as st
|
||||
monkeypatch.setattr(st, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
|
||||
resp = await async_client.patch(
|
||||
"/api/settings",
|
||||
json={
|
||||
"providers": {
|
||||
"ollama": {"model": "mistral", "base_url": "http://host.docker.internal:11434"}
|
||||
}
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["providers"]["ollama"]["model"] == "mistral"
|
||||
|
||||
|
||||
async def test_masked_api_key_not_overwritten(async_client, tmp_path, monkeypatch):
|
||||
"""Patching with a masked key should not overwrite the real stored key."""
|
||||
import config as cfg
|
||||
monkeypatch.setattr(cfg, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
import services.storage as st
|
||||
monkeypatch.setattr(st, "SETTINGS_FILE", tmp_path / "settings.json")
|
||||
|
||||
# First set a real key
|
||||
await async_client.patch(
|
||||
"/api/settings",
|
||||
json={"providers": {"anthropic": {"api_key": "sk-ant-realkey"}}},
|
||||
)
|
||||
# Then patch with masked key (simulating frontend re-submitting)
|
||||
await async_client.patch(
|
||||
"/api/settings",
|
||||
json={"providers": {"anthropic": {"api_key": "****key"}}},
|
||||
)
|
||||
# The stored key should still be the real one
|
||||
stored = st.load_settings()
|
||||
assert stored["providers"]["anthropic"]["api_key"] == "sk-ant-realkey"
|
||||
|
||||
|
||||
async def test_get_default_prompt(async_client):
|
||||
resp = await async_client.get("/api/settings/default-prompt")
|
||||
assert resp.status_code == 200
|
||||
assert "system_prompt" in resp.json()
|
||||
assert len(resp.json()["system_prompt"]) > 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Wave 0 xfail stub — D-12: /api/settings endpoint removed in Plan 03-04
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.xfail(strict=False, reason="implemented in plan 03-04")
|
||||
async def test_settings_endpoint_removed(async_client):
|
||||
"""GET /api/settings returns 404 after the flat-file settings system is retired.
|
||||
|
||||
D-12: the /api/settings endpoint is removed entirely in Phase 3. All AI config
|
||||
comes from the database (users.ai_provider / users.ai_model set by admin).
|
||||
The flat-file services/storage.py load_settings()/save_settings() functions
|
||||
are also deleted (CONTEXT.md D-12).
|
||||
"""
|
||||
assert True # scaffold
|
||||
"""D-12: /api/settings endpoint is removed in Phase 3."""
|
||||
resp = await async_client.get("/api/settings")
|
||||
assert resp.status_code == 404
|
||||
|
||||
Reference in New Issue
Block a user