From 1f8b8c9f477a62ed60f5f24ad57d42587ff7cbdc Mon Sep 17 00:00:00 2001
From: busya <uglypod@gmail.com>
Date: Wed, 13 May 2026 20:17:14 +0300
Subject: [PATCH] =?UTF-8?q?fix(qa):=20address=20qa-tester=20findings=20?=
 =?UTF-8?q?=E2=80=94=20mock=20target=5Fcolumn,=20FetchModelsRequest=20sche?=
 =?UTF-8?q?ma,=20error=20handling?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Fix MagicMock poisoning: add target_column=None to _make_mock_job/mock_job fixtures (3 files)
- Add FetchModelsRequest Pydantic model for type-safe fetch-models endpoint
- logger.warning→logger.error for fetch_models 502 path
- Early validation: require api_key or provider_id in fetch-models
- 50 tests pass, 0 failures (test_clickhouse_insert_integration, test_orchestrator, test_preview)
---
 backend/src/api/routes/llm.py                 | 32 +++++++++++++++----
 .../test_clickhouse_insert_integration.py     |  1 +
 .../translate/__tests__/test_orchestrator.py  |  1 +
 .../translate/__tests__/test_preview.py       |  1 +
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/backend/src/api/routes/llm.py b/backend/src/api/routes/llm.py
index 46ec517c..44c70d97 100644
--- a/backend/src/api/routes/llm.py
+++ b/backend/src/api/routes/llm.py
@@ -8,6 +8,7 @@
 
 from fastapi import APIRouter, Depends, HTTPException, status
 from typing import List, Optional
+from pydantic import BaseModel, Field
 from ...core.logger import logger
 from ...schemas.auth import User
 from ...dependencies import get_current_user as get_current_active_user
@@ -16,6 +17,18 @@ from ...services.llm_provider import LLMProviderService
 from ...core.database import get_db
 from sqlalchemy.orm import Session
 
+
+# #region FetchModelsRequest [C:1] [TYPE Class]
+# @BRIEF Pydantic request model for the fetch-models endpoint.
+class FetchModelsRequest(BaseModel):
+    base_url: Optional[str] = Field(None, description="LLM provider base URL")
+    provider_type: Optional[str] = Field(None, description="Provider type (openai, anthropic, etc)")
+    api_key: Optional[str] = Field(None, description="Direct API key (takes precedence over provider_id)")
+    provider_id: Optional[str] = Field(None, description="Saved provider ID to look up stored API key")
+
+
+# #endregion FetchModelsRequest
+
 # #region router [TYPE Global]
 # @BRIEF APIRouter instance for LLM routes.
 router = APIRouter(tags=["LLM"])
@@ -83,17 +96,24 @@ async def get_providers(
 # @RELATION CALLS -> [LLMClient]
 @router.post("/providers/fetch-models")
 async def fetch_models(
-    payload: dict,
+    payload: FetchModelsRequest,
     current_user: User = Depends(get_current_active_user),
     db: Session = Depends(get_db),
 ):
     from ...plugins.llm_analysis.service import LLMClient
     from ...plugins.llm_analysis.models import LLMProviderType
 
-    base_url = (payload.get("base_url") or "").strip()
-    provider_type_str = (payload.get("provider_type") or "").strip()
-    api_key = payload.get("api_key") or ""
-    provider_id = payload.get("provider_id") or ""
+    base_url = (payload.base_url or "").strip()
+    provider_type_str = (payload.provider_type or "").strip()
+    api_key = payload.api_key or ""
+    provider_id = payload.provider_id or ""
+
+    # Early validation: at least one of api_key or provider_id is required
+    if not api_key and not provider_id:
+        raise HTTPException(
+            status_code=400,
+            detail="api_key or provider_id is required",
+        )
 
     # Resolve provider_id to stored credentials if no direct api_key given
     if not api_key and provider_id:
@@ -128,7 +148,7 @@ async def fetch_models(
         models = await client.fetch_models()
         return {"models": models}
     except Exception as e:
-        logger.warning(
+        logger.error(
             f"[llm_routes.fetch_models] Failed to fetch models: {e}",
             extra={"src": "llm_routes.fetch_models"},
         )
diff --git a/backend/src/plugins/translate/__tests__/test_clickhouse_insert_integration.py b/backend/src/plugins/translate/__tests__/test_clickhouse_insert_integration.py
index d174dc50..9b980d2a 100644
--- a/backend/src/plugins/translate/__tests__/test_clickhouse_insert_integration.py
+++ b/backend/src/plugins/translate/__tests__/test_clickhouse_insert_integration.py
@@ -35,6 +35,7 @@ def _make_mock_job(**overrides):
     job.target_database_id = "1"
     job.environment_id = "test-env"
     job.context_columns = []
+    job.target_column = None
     for k, v in overrides.items():
         setattr(job, k, v)
     return job
diff --git a/backend/src/plugins/translate/__tests__/test_orchestrator.py b/backend/src/plugins/translate/__tests__/test_orchestrator.py
index e87ca96e..6d57ba25 100644
--- a/backend/src/plugins/translate/__tests__/test_orchestrator.py
+++ b/backend/src/plugins/translate/__tests__/test_orchestrator.py
@@ -48,6 +48,7 @@ def mock_job() -> MagicMock:
     job.provider_id = "provider-1"
     job.batch_size = 50
     job.upsert_strategy = "MERGE"
+    job.target_column = None
     return job
 
 
diff --git a/backend/src/plugins/translate/__tests__/test_preview.py b/backend/src/plugins/translate/__tests__/test_preview.py
index c7b0e778..e48b1542 100644
--- a/backend/src/plugins/translate/__tests__/test_preview.py
+++ b/backend/src/plugins/translate/__tests__/test_preview.py
@@ -35,6 +35,7 @@ def _make_mock_job(**overrides):
     job.batch_size = overrides.get("batch_size", 50)
     job.upsert_strategy = overrides.get("upsert_strategy", "MERGE")
     job.status = overrides.get("status", "DRAFT")
+    job.target_column = None
     return job
 # endregion _make_mock_job