From 8488012f058c47a966fcb839ce385051df94f6a2 Mon Sep 17 00:00:00 2001 From: lekss361 Date: Sat, 16 May 2026 12:01:09 +0300 Subject: [PATCH] fix(best-layouts): address review-bot minor items (#113 PR C) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3 carryover 🟡 items из re-review verdict на a88917f: 1. Coverage % denominator now post-filter (was mixing pre-filter count с post-filter intersection → artificially low confidence when user passed exclude/filter list) 2. MAX(snapshot_date) pre-computed via db.scalar() + bind :latest_snap instead of subquery in WHERE (avoid re-execution per spatial scan) 3. LAYOUT_CONFIDENCE_HIGH_PCT / LAYOUT_CONFIDENCE_MEDIUM_PCT as module-level constants instead of hardcoded literals Integration test gap (#4 в re-review) — отдельный follow-up issue, не в scope этого PR. Tests: 10/10 best-layouts pass, 43/43 regression OK. --- .../app/services/site_finder/best_layouts.py | 52 +++++++++++++------ .../tests/api/v1/test_parcel_best_layouts.py | 15 ++++-- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/backend/app/services/site_finder/best_layouts.py b/backend/app/services/site_finder/best_layouts.py index 7be4bf6b..ac3ca21c 100644 --- a/backend/app/services/site_finder/best_layouts.py +++ b/backend/app/services/site_finder/best_layouts.py @@ -39,6 +39,11 @@ from app.services.site_finder.layout_signature import area_bin, layout_signature logger = logging.getLogger(__name__) +# Confidence thresholds (per coverage % of objects with MV velocity data) +# Tune via PR if business feedback требует. +LAYOUT_CONFIDENCE_HIGH_PCT = 50.0 +LAYOUT_CONFIDENCE_MEDIUM_PCT = 20.0 + # Делители velocity: 24 мес → масштаб на указанный window _VELOCITY_DIVISORS: dict[str, float] = { "last_month": 24.0, @@ -139,7 +144,7 @@ _SUPPLY_BATCH_SQL = text(""" )::geography, CAST(:radius_m AS float) ) - AND f.snapshot_date = (SELECT MAX(snapshot_date) FROM domrf_kn_flats) + AND f.snapshot_date = CAST(:latest_snap AS date) GROUP BY rb, ab """) @@ -279,18 +284,29 @@ def get_best_layouts( ) # ── Step 5: supply side (батч-запрос) ──────────────────────────────────── - try: - supply_rows = ( - db.execute( - _SUPPLY_BATCH_SQL, - {"center_lon": center_lon, "center_lat": center_lat, "radius_m": radius_m}, - ) - .mappings() - .all() - ) - except Exception: - logger.warning("best_layouts: supply query failed, supply=0 fallback") + # Pre-compute последний snapshot_date один раз — избегаем subquery на каждый scan. + latest_snap: dt.date | None = db.scalar(text("SELECT MAX(snapshot_date) FROM domrf_kn_flats")) + if latest_snap is None: + logger.warning("best_layouts: domrf_kn_flats пустой (нет snapshot_date), supply=0 fallback") supply_rows = [] + else: + try: + supply_rows = ( + db.execute( + _SUPPLY_BATCH_SQL, + { + "center_lon": center_lon, + "center_lat": center_lat, + "radius_m": radius_m, + "latest_snap": latest_snap, + }, + ) + .mappings() + .all() + ) + except Exception: + logger.warning("best_layouts: supply query failed, supply=0 fallback") + supply_rows = [] supply_map: dict[tuple[str, str], int] = { (str(r["rb"]), str(r["ab"])): int(r["units"]) for r in supply_rows @@ -410,22 +426,24 @@ def get_best_layouts( ) # ── Step 9: data_quality ───────────────────────────────────────────────── + # Denominator = post-filter set (effective consideration set после exclude/filter). + objects_total_after_filter = len(all_obj_ids) objects_with_data = len(obj_ids_with_data & set(all_obj_ids)) coverage_pct = ( - round(objects_with_data / objects_total_in_radius * 100.0, 1) - if objects_total_in_radius > 0 + round(objects_with_data / objects_total_after_filter * 100.0, 1) + if objects_total_after_filter > 0 else 0.0 ) - if coverage_pct >= 50.0: + if coverage_pct >= LAYOUT_CONFIDENCE_HIGH_PCT: confidence: str = "high" - elif coverage_pct >= 20.0: + elif coverage_pct >= LAYOUT_CONFIDENCE_MEDIUM_PCT: confidence = "medium" else: confidence = "low" data_quality = LayoutDataQuality( objects_with_velocity_data=objects_with_data, - objects_total_in_radius=objects_total_in_radius, + objects_total_in_radius=objects_total_after_filter, velocity_coverage_pct=coverage_pct, confidence=confidence, # type: ignore[arg-type] ) diff --git a/backend/tests/api/v1/test_parcel_best_layouts.py b/backend/tests/api/v1/test_parcel_best_layouts.py index 779a9190..8b391f90 100644 --- a/backend/tests/api/v1/test_parcel_best_layouts.py +++ b/backend/tests/api/v1/test_parcel_best_layouts.py @@ -3,11 +3,13 @@ Mock-based — не требуют живой БД. Паттерн mock DB: аналогично test_parcel_competitors.py — dependency_overrides[get_db]. -Порядок вызовов db.execute в get_best_layouts: +Порядок вызовов в get_best_layouts: + db.scalar() → MAX(snapshot_date) (только когда vel_rows non-empty) + db.execute() calls: 1. _PARCEL_CENTROID_SQL → .mappings().first() 2. _COMPETITORS_IN_RADIUS_SQL → .mappings().all() 3. _VELOCITY_BY_ROOM_SQL → .mappings().all() - 4. _SUPPLY_BATCH_SQL → .mappings().all() + 4. _SUPPLY_BATCH_SQL → .mappings().all() (пропускается если latest_snap is None) """ from __future__ import annotations @@ -83,17 +85,22 @@ def _make_db( id_rows: list[MagicMock] | None = None, vel_rows: list[MagicMock] | None = None, supply_rows: list[MagicMock] | None = None, + latest_snap: dt.date | None = None, ) -> MagicMock: """Сконструировать mock Session. - Порядок execute: + db.scalar() возвращает latest_snap (MAX snapshot_date) — вызывается перед supply. + Порядок db.execute(): 1. centroid → .mappings().first() 2. competitors-in-radius → .mappings().all() 3. velocity → .mappings().all() - 4. supply → .mappings().all() + 4. supply → .mappings().all() (только если latest_snap is not None) """ db = MagicMock() + # db.scalar — pre-computed MAX(snapshot_date) для supply query + db.scalar.return_value = latest_snap if latest_snap is not None else _TODAY + results: list[MagicMock] = [] # 1: centroid