fix(tradein/cian): management_companies dedup — SELECT-before-INSERT, not ON CONFLICT

ext_id=NULL rows never conflict under Postgres default NULLS DISTINCT, so the
ON CONFLICT (ext_source, name, ext_id) DO UPDATE in _upsert_cian_valuation_management_company
was dead code — every valuation cache-miss for the same building inserted a fresh
duplicate management_companies row instead of deduping by name. Replaced with an
explicit SELECT-then-branch (UPDATE existing / INSERT new); docstring corrected
to stop claiming the constraint dedups it. Found by deep-code-reviewer on #2435.

Refs #2435

tradein-mvp/backend/tests/test_cian_bti_house_persist.py

@@ -225,8 +225,16 @@ def _cian_result(
     )
 
 
-def _mock_db_valuation(mc_insert_id: int | None = 900) -> MagicMock:
-    """Mock db для _save_to_cache: INSERT management_companies RETURNING id + UPDATE houses."""
+def _mock_db_valuation(
+    mc_insert_id: int | None = 900, mc_existing_id: int | None = None
+) -> MagicMock:
+    """Mock db для _save_to_cache: SELECT (dedup) → UPDATE-or-INSERT management_companies,
+    затем UPDATE houses.
+
+    `mc_existing_id` simulates the dedup SELECT finding a prior row (existing.fetchone()
+    returns that id) — the UPDATE branch is taken instead of INSERT. Default None means
+    "no existing row", exercising the INSERT branch (the original test-suite behavior).
+    """
     db = MagicMock()
 
     @contextmanager
@@ -238,7 +246,11 @@ def _mock_db_valuation(mc_insert_id: int | None = 900) -> MagicMock:
     def _exec(sql, params=None):
         sql_str = str(sql)
         mock_result = MagicMock()
-        if "INSERT INTO management_companies" in sql_str:
+        if "SELECT id FROM management_companies" in sql_str:
+            mock_result.fetchone.return_value = (
+                (mc_existing_id,) if mc_existing_id is not None else None
+            )
+        elif "INSERT INTO management_companies" in sql_str:
             mock_result.fetchone.return_value = (
                 (mc_insert_id,) if mc_insert_id is not None else None
             )
@@ -250,6 +262,22 @@ def _mock_db_valuation(mc_insert_id: int | None = 900) -> MagicMock:
     return db
 
 
+def _mc_calls(db: MagicMock) -> list[str]:
+    """SQL statement kinds touching management_companies, in call order."""
+    kinds = []
+    for call_args in db.execute.call_args_list:
+        if not call_args.args:
+            continue
+        sql = str(call_args.args[0])
+        if "SELECT id FROM management_companies" in sql:
+            kinds.append("select")
+        elif "INSERT INTO management_companies" in sql:
+            kinds.append("insert")
+        elif "UPDATE management_companies" in sql:
+            kinds.append("update")
+    return kinds
+
+
 def _houses_update_call(db: MagicMock) -> tuple[str, dict] | None:
     for call_args in db.execute.call_args_list:
         if not call_args.args:
@@ -278,7 +306,8 @@ def _call_save_to_cache(db: MagicMock, result: CianValuationResult, house_id: in
 
 
 def test_valuation_house_info_and_management_company_land_in_houses_update():
-    """house_id задан → UPDATE houses с management_company_id + house_info-полями."""
+    """house_id задан, УК ранее не встречалась → SELECT (не найдено) → INSERT management_companies,
+    UPDATE houses с management_company_id + house_info-полями."""
     db = _mock_db_valuation(mc_insert_id=900)
     result = _cian_result(
         house_info=_HOUSE_INFO_SAMPLE,
@@ -293,6 +322,7 @@ def test_valuation_house_info_and_management_company_land_in_houses_update():
 
     _call_save_to_cache(db, result, house_id=42)
 
+    assert _mc_calls(db) == ["select", "insert"]
     call = _houses_update_call(db)
     assert call is not None
     sql, params = call
@@ -315,6 +345,29 @@ def test_valuation_house_info_and_management_company_land_in_houses_update():
     db.commit.assert_called_once()
 
 
+def test_valuation_management_company_dedups_on_repeat_call_same_name():
+    """Regression for the NULLS-DISTINCT dedup bug (#2435 review): a repeat valuation
+    call for a management company already persisted (same ext_source/name, ext_id=NULL)
+    must find it via the explicit SELECT and UPDATE it — not INSERT a duplicate row.
+    `ON CONFLICT (ext_source, name, ext_id)` alone can't dedup ext_id=NULL rows under
+    Postgres's default NULLS DISTINCT (two NULLs never compare equal), which is why
+    dedup is done via SELECT-before-INSERT rather than relying on the constraint."""
+    db = _mock_db_valuation(mc_existing_id=900)
+    result = _cian_result(
+        house_info=[],
+        management_company={"name": 'ТСЖ "Учителей, 18"', "phones": ["+7 (343) 216-59-12"]},
+    )
+
+    _call_save_to_cache(db, result, house_id=42)
+
+    assert _mc_calls(db) == ["select", "update"]
+    call = _houses_update_call(db)
+    assert call is not None
+    _, params = call
+    assert params["mcid"] == 900
+    db.commit.assert_called_once()
+
+
 def test_valuation_house_id_none_skips_house_update():
     """house_id=None → нет UPDATE houses (best-effort no-op, тот же no-crash контракт,
     что test_extval_house_id_write_path.py::test_cian_save_null_house_id_no_exception)."""

tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/cian/valuation.py

@@ -656,11 +656,14 @@ def _upsert_cian_valuation_management_company(
 ) -> int | None:
     """UPSERT management_companies из Cian Valuation Calculator's managementCompany.data.
 
-    Тот же UPSERT-паттерн, что providers/cian/newbuilding.py::save_newbuilding_enrichment
-    (независимая копия — newbuilding.py вне scope #2435, не рефакторим/не переиспользуем).
-    ext_source='cian_valuation' (не 'cian') — валюация не даёт числового id УК (в отличие
-    от newbuilding.managementCompany.id), поэтому dedup-ключ (ext_source, name, ext_id=NULL)
-    сходится по имени УК среди повторных valuation-запросов.
+    Тот же намерение, что providers/cian/newbuilding.py::save_newbuilding_enrichment
+    (независимая копия — newbuilding.py вне scope #2435, не рефакторим/не переиспользуем),
+    НО другой механизм dedup. ext_source='cian_valuation' (не 'cian') — валюация не даёт
+    числового id УК (в отличие от newbuilding.managementCompany.id), поэтому пишем
+    ext_id=NULL. `UNIQUE (ext_source, name, ext_id)` под Postgres-дефолтом NULLS DISTINCT
+    не считает два NULL равными — `ON CONFLICT` с ext_id=NULL никогда бы не сработал (каждый
+    вызов создавал бы новую дублирующую строку). Поэтому dedup сделан явно: SELECT по
+    (ext_source, name) WHERE ext_id IS NULL перед INSERT, а не через constraint.
 
     opening_hours — Cian отдаёт список словарей ([{"Пн–Пт": ["10:00-20:00"]}, ...]), не
     строку; json.dumps в text-колонку (в отличие от newbuilding.py, которая пишет объект
@@ -673,6 +676,32 @@ def _upsert_cian_valuation_management_company(
         return None
 
     opening_hours = mc.get("openingHours")
+    phones = mc.get("phones") or []
+    email = mc.get("email")
+    oh_json = json.dumps(opening_hours, ensure_ascii=False) if opening_hours else None
+    chief = mc.get("chiefName")
+
+    existing = db.execute(
+        text("""
+            SELECT id FROM management_companies
+            WHERE ext_source = 'cian_valuation' AND name = :name AND ext_id IS NULL
+            LIMIT 1
+        """),
+        {"name": name},
+    ).fetchone()
+    if existing:
+        mc_id = int(existing[0])
+        db.execute(
+            text("""
+                UPDATE management_companies SET
+                    phones = COALESCE(CAST(:phones AS text[]), phones),
+                    email  = COALESCE(:email, email)
+                WHERE id = CAST(:mc_id AS bigint)
+            """),
+            {"phones": phones, "email": email, "mc_id": mc_id},
+        )
+        return mc_id
+
     row = db.execute(
         text("""
             INSERT INTO management_companies (
@@ -680,17 +709,14 @@ def _upsert_cian_valuation_management_company(
             ) VALUES (
                 :name, CAST(:phones AS text[]), :email, :oh, :chief, 'cian_valuation', NULL
             )
-            ON CONFLICT (ext_source, name, ext_id) DO UPDATE SET
-                phones = COALESCE(EXCLUDED.phones, management_companies.phones),
-                email  = COALESCE(EXCLUDED.email, management_companies.email)
             RETURNING id
         """),
         {
             "name": name,
-            "phones": mc.get("phones") or [],
-            "email": mc.get("email"),
-            "oh": json.dumps(opening_hours, ensure_ascii=False) if opening_hours else None,
-            "chief": mc.get("chiefName"),
+            "phones": phones,
+            "email": email,
+            "oh": oh_json,
+            "chief": chief,
         },
     ).fetchone()
     return int(row[0]) if row else None