diff --git a/tradein-mvp/backend/app/services/estimator.py b/tradein-mvp/backend/app/services/estimator.py index d0074c7f..76e58cc0 100644 --- a/tradein-mvp/backend/app/services/estimator.py +++ b/tradein-mvp/backend/app/services/estimator.py @@ -2862,6 +2862,7 @@ def _fetch_analogs( ) AS rn_addr FROM listings WHERE ST_DWithin(geom::geography, ST_MakePoint(:lon, :lat)::geography, :radius) + AND (geo_precision IS DISTINCT FROM 'city') {_COMMON_WHERE} AND total_floors BETWEEN CAST(:tf_min AS integer) AND CAST(:tf_max AS integer) @@ -2972,6 +2973,7 @@ def _fetch_analogs( ) AS rn_addr FROM listings WHERE ST_DWithin(geom::geography, ST_MakePoint(:lon, :lat)::geography, :radius) + AND (geo_precision IS DISTINCT FROM 'city') AND rooms = :rooms AND area_m2 BETWEEN :area_min AND :area_max AND is_active = true @@ -2991,6 +2993,10 @@ def _fetch_analogs( -- C-5 audit). Listings с NULL coords отфильтруются через ST_DWithin -- (geom IS NULL → не matches). geocode-missing-listings backfill -- подтягивает координаты для address-only Avito листингов. + -- #769 Part E: geo_precision='city' исключает city-centroid листинги + -- из radius-аналогов (centroid загрязнял comp set при ST_DWithin). + -- IS DISTINCT FROM 'city' пропускает NULL (неизвестная точность — + -- консервативно: листинг участвует в аналогах, не удаляем без причины). ) SELECT source, source_url, address, lat, lon, diff --git a/tradein-mvp/backend/app/services/scrapers/base.py b/tradein-mvp/backend/app/services/scrapers/base.py index 2c478ba6..dbf623f7 100644 --- a/tradein-mvp/backend/app/services/scrapers/base.py +++ b/tradein-mvp/backend/app/services/scrapers/base.py @@ -93,6 +93,12 @@ class ScrapedLot(BaseModel): price_rub: int = Field(gt=0) price_per_m2: int | None = None + # Геокодинг + # None → точность неизвестна (координаты пришли из скрейпера напрямую). + # 'city' → геокодер вернул city-centroid (нет номера дома в адресе) — + # листинг исключается из radius-аналогов estimator'а. + geo_precision: str | None = None + # Метаданные listing_date: date | None = None days_on_market: int | None = None @@ -251,6 +257,7 @@ def save_listings( description_minhash, cadastral_number, building_cadastral_number, phones, is_homeowner, is_pro_seller, bargain_allowed, sale_type, metro_stations, + geo_precision, scraped_at, last_seen_at ) VALUES ( :source, :source_url, :source_id, :dedup, @@ -266,6 +273,7 @@ def save_listings( :description_minhash, :cadastral_number, :building_cadastral_number, CAST(:phones AS jsonb), :is_homeowner, :is_pro_seller, :bargain_allowed, :sale_type, CAST(:metro_stations AS jsonb), + :geo_precision, NOW(), NOW() ) ON CONFLICT (dedup_hash) DO UPDATE @@ -332,6 +340,7 @@ def save_listings( "bargain_allowed": lot.bargain_allowed, "sale_type": lot.sale_type, "metro_stations": _to_json(lot.metro_stations) if lot.metro_stations else None, + "geo_precision": lot.geo_precision, }, ).fetchone() diff --git a/tradein-mvp/backend/app/tasks/geocode_missing.py b/tradein-mvp/backend/app/tasks/geocode_missing.py index 05845c83..d14af37d 100644 --- a/tradein-mvp/backend/app/tasks/geocode_missing.py +++ b/tradein-mvp/backend/app/tasks/geocode_missing.py @@ -12,6 +12,7 @@ Rate limit: Nominatim 1 req/sec. Yandex 25K/day если YANDEX_GEOCODER_API_KEY - Поддерживает all sources включая Avito (после PR #487 убрали jitter). - Возвращает GeocodeBackfillResult с детальными counters. """ + from __future__ import annotations import logging @@ -21,6 +22,7 @@ from dataclasses import dataclass, field from sqlalchemy import text from sqlalchemy.orm import Session +from app.services.estimator import _geocode_is_coarse from app.services.geocoder import geocode logger = logging.getLogger(__name__) @@ -28,13 +30,13 @@ logger = logging.getLogger(__name__) @dataclass class GeocodeBackfillResult: - addresses_total: int = 0 # unique addresses pending geocode в этом batch - addresses_processed: int = 0 # фактически обработано - addresses_geocoded: int = 0 # успешно получили coords - addresses_failed: int = 0 # geocoder вернул None - listings_updated: int = 0 # total listings затронуто (1 address → N listings) - cache_hits: int = 0 # из geocode_cache (instant) - cache_misses: int = 0 # реальные geocoder calls + addresses_total: int = 0 # unique addresses pending geocode в этом batch + addresses_processed: int = 0 # фактически обработано + addresses_geocoded: int = 0 # успешно получили coords + addresses_failed: int = 0 # geocoder вернул None + listings_updated: int = 0 # total listings затронуто (1 address → N listings) + cache_hits: int = 0 # из geocode_cache (instant) + cache_misses: int = 0 # реальные geocoder calls duration_sec: float = field(default=0.0) @@ -69,9 +71,10 @@ async def geocode_missing_listings( result = GeocodeBackfillResult() # 1. Найти top-N адресов с NULL coords (DESC by occurrence count) - rows = db.execute( - text( - """ + rows = ( + db.execute( + text( + """ SELECT address, COUNT(*) AS listings_count FROM listings WHERE lat IS NULL @@ -81,9 +84,12 @@ async def geocode_missing_listings( ORDER BY listings_count DESC, address ASC LIMIT :limit """ - ), - {"limit": batch_size}, - ).mappings().all() + ), + {"limit": batch_size}, + ) + .mappings() + .all() + ) result.addresses_total = len(rows) @@ -106,9 +112,7 @@ async def geocode_missing_listings( try: geo = await geocode(address, db) except Exception as exc: - logger.warning( - "geocode_missing: geocode raised for '%s': %s", address[:60], exc - ) + logger.warning("geocode_missing: geocode raised for '%s': %s", address[:60], exc) result.addresses_failed += 1 continue @@ -139,16 +143,23 @@ async def geocode_missing_listings( ) continue - # UPDATE listings — PostGIS trigger (listings_set_geom_trg) обновит geom автоматически + # Определяем точность геокода: city-centroid (нет номера дома) → 'city'. + # _geocode_is_coarse() проверяет confidence='locality' ИЛИ отсутствие + # house-number токена (1-3 цифры) в full_address — оба случая означают + # что геокодер не дошёл до дома и вернул центр НП/города (#769 Part E). + precision: str | None = "city" if _geocode_is_coarse(geo) else None + + # UPDATE listings — PostGIS trigger (listings_set_geom_trg) обновит geom автоматически. + # geo_precision проставляется одновременно с координатами. update_result = db.execute( text( """ UPDATE listings - SET lat = :lat, lon = :lon + SET lat = :lat, lon = :lon, geo_precision = :precision WHERE address = :addr AND lat IS NULL """ ), - {"lat": geo.lat, "lon": geo.lon, "addr": address}, + {"lat": geo.lat, "lon": geo.lon, "precision": precision, "addr": address}, ) db.commit() result.listings_updated += update_result.rowcount diff --git a/tradein-mvp/backend/data/sql/089_listings_geo_precision.sql b/tradein-mvp/backend/data/sql/089_listings_geo_precision.sql new file mode 100644 index 00000000..5a841444 --- /dev/null +++ b/tradein-mvp/backend/data/sql/089_listings_geo_precision.sql @@ -0,0 +1,44 @@ +-- 089_listings_geo_precision.sql +-- Issue #769 Part E (finding #17) — geo_precision flag for city-centroid geocodes. +-- +-- Context: +-- Listings scraped from Cian/N1 with bare city addresses ("Екатеринбург (Cian)", +-- "Екатеринбург (N1)") have no house number. When geocode_missing_listings() runs +-- the geocoder against such an address, it receives a city-centroid coordinate +-- (56.838, 60.605 area) and saves it as the listing's precise lat/lon. These +-- city-centroid listings then pollute radius (ST_DWithin) analog matching in the +-- estimator — they match every address in a wide radius around the city center. +-- +-- Fix: +-- Add geo_precision TEXT column. Values: +-- NULL — not yet populated (geocoded before this migration, or coords came +-- directly from scraper without going through geocode_missing path) +-- 'city' — geocoder returned a city-level/coarse result (no house-number in +-- full_address); listing is a city-centroid fallback — EXCLUDE from +-- radius analog queries. +-- +-- Idempotency: +-- ALTER TABLE ... ADD COLUMN IF NOT EXISTS — safe on re-run. +-- BEGIN/COMMIT block. +-- +-- Backfill note: +-- NULL rows are treated conservatively as "unknown" (not excluded from analogs) +-- to avoid removing valid listings geocoded before this migration. +-- Targeted backfill of known city-centroid addresses is a follow-up task: +-- UPDATE listings SET geo_precision = 'city' +-- WHERE address IN ('Екатеринбург (Cian)', 'Екатеринбург (N1)') +-- AND lat IS NOT NULL; +-- This can be applied by QA/ops once this migration is deployed. +-- +-- Dependencies: +-- 002_core_tables.sql (listings table). +-- +-- Deploy order: +-- Apply after 088_scrape_schedules_seed_search_matview_refresh.sql. +-- No data loss — column is nullable, existing rows unaffected. + +BEGIN; + +ALTER TABLE listings ADD COLUMN IF NOT EXISTS geo_precision text; + +COMMIT; diff --git a/tradein-mvp/backend/tests/test_geo_precision.py b/tradein-mvp/backend/tests/test_geo_precision.py new file mode 100644 index 00000000..ae5cc040 --- /dev/null +++ b/tradein-mvp/backend/tests/test_geo_precision.py @@ -0,0 +1,201 @@ +"""Tests for geo_precision city-fallback flag (#769 Part E). + +Two assertions: +(a) A city-fallback geocode (no house number in full_address) causes + geocode_missing_listings() to set geo_precision='city'. +(b) The radius-analog SQL in estimator._fetch_analogs contains the + geo_precision exclusion filter (AND (geo_precision IS DISTINCT FROM 'city')). +""" + +from __future__ import annotations + +import inspect +import os +import sys +from unittest.mock import AsyncMock, MagicMock, patch + +# DATABASE_URL required by config before any app import. +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +# WeasyPrint stub — not installed in CI without GTK. +_wp_mock = MagicMock() +sys.modules.setdefault("weasyprint", _wp_mock) + +import pytest # noqa: E402 + +from app.services.geocoder import GeocodeResult # noqa: E402 + +# ── helpers ────────────────────────────────────────────────────────────────── + + +def _make_geo(full_address: str, confidence: str = "approximate") -> GeocodeResult: + return GeocodeResult( + lat=56.838, + lon=60.605, + full_address=full_address, + provider="nominatim", # type: ignore[arg-type] + confidence=confidence, # type: ignore[arg-type] + ) + + +# ── (a) geocode_missing sets geo_precision='city' for coarse geocode ───────── + + +@pytest.mark.asyncio +async def test_geocode_missing_sets_city_precision_for_bare_city_address() -> None: + """Bare city address geocoded to centroid → geo_precision='city' in UPDATE. + + Simulates "Екатеринбург (Cian)" address: geocoder returns a result whose + full_address has no house-number token → _geocode_is_coarse() → True. + The UPDATE should include geo_precision='city'. + """ + from app.tasks.geocode_missing import geocode_missing_listings + + # City-centroid result: full_address has no 1-3-digit house number. + city_geo = _make_geo("Екатеринбург", confidence="approximate") + + rows = [{"address": "Екатеринбург (Cian)", "listings_count": 3}] + + db = MagicMock() + select_result = MagicMock() + select_result.mappings.return_value.all.return_value = rows + update_result = MagicMock() + update_result.rowcount = 3 + db.execute.side_effect = [select_result, update_result] + + with patch( + "app.tasks.geocode_missing.geocode", + new_callable=AsyncMock, + return_value=city_geo, + ): + result = await geocode_missing_listings(db, batch_size=50) + + assert result.addresses_geocoded == 1 + assert result.listings_updated == 3 + + # Inspect the UPDATE call parameters: geo_precision must be 'city'. + update_call = db.execute.call_args_list[1] + update_params = update_call[0][1] # positional arg[1] = params dict + assert ( + update_params.get("precision") == "city" + ), f"Expected precision='city' for bare city address, got {update_params.get('precision')!r}" + + +@pytest.mark.asyncio +async def test_geocode_missing_sets_none_precision_for_precise_address() -> None: + """Address with house number → _geocode_is_coarse() → False → geo_precision=None.""" + from app.tasks.geocode_missing import geocode_missing_listings + + # Precise result: full_address contains house number "30". + precise_geo = _make_geo("Екатеринбург, улица Малышева, 30", confidence="exact") + + rows = [{"address": "ул. Малышева, 30", "listings_count": 2}] + + db = MagicMock() + select_result = MagicMock() + select_result.mappings.return_value.all.return_value = rows + update_result = MagicMock() + update_result.rowcount = 2 + db.execute.side_effect = [select_result, update_result] + + with patch( + "app.tasks.geocode_missing.geocode", + new_callable=AsyncMock, + return_value=precise_geo, + ): + await geocode_missing_listings(db, batch_size=50) + + update_call = db.execute.call_args_list[1] + update_params = update_call[0][1] + assert ( + update_params.get("precision") is None + ), f"Expected precision=None for precise address, got {update_params.get('precision')!r}" + + +@pytest.mark.asyncio +async def test_geocode_missing_sets_city_precision_for_locality_confidence() -> None: + """confidence='locality' → _geocode_is_coarse() → True → geo_precision='city'.""" + from app.tasks.geocode_missing import geocode_missing_listings + + # confidence='locality' is an explicit centroid marker (even if address has digits). + locality_geo = _make_geo("Екатеринбург, Свердловская область", confidence="locality") + + rows = [{"address": "Екатеринбург (N1)", "listings_count": 1}] + + db = MagicMock() + select_result = MagicMock() + select_result.mappings.return_value.all.return_value = rows + update_result = MagicMock() + update_result.rowcount = 1 + db.execute.side_effect = [select_result, update_result] + + with patch( + "app.tasks.geocode_missing.geocode", + new_callable=AsyncMock, + return_value=locality_geo, + ): + await geocode_missing_listings(db, batch_size=50) + + update_call = db.execute.call_args_list[1] + update_params = update_call[0][1] + assert ( + update_params.get("precision") == "city" + ), f"Expected precision='city' for locality confidence, got {update_params.get('precision')!r}" + + +# ── (b) estimator radius-analog SQL contains geo_precision exclusion ────────── + + +def test_fetch_analogs_tier_w_excludes_city_precision() -> None: + """Tier W (wide radius) query must filter out geo_precision='city' listings. + + Asserts the SQL fragment is present in _fetch_analogs source code so that + city-centroid listings are excluded from radius analog matching. + """ + from app.services import estimator + + source = inspect.getsource(estimator._fetch_analogs) + assert "geo_precision IS DISTINCT FROM 'city'" in source, ( + "Tier W radius-analog query must contain " + "AND (geo_precision IS DISTINCT FROM 'city') to exclude city-centroid listings" + ) + + +def test_fetch_analogs_tier_h_excludes_city_precision() -> None: + """Tier H (same class, radius) query must also filter out geo_precision='city'.""" + from app.services import estimator + + source = inspect.getsource(estimator._fetch_analogs) + # The filter appears in both Tier H and Tier W sub-queries. + # Count occurrences to verify both are patched. + occurrences = source.count("geo_precision IS DISTINCT FROM 'city'") + assert occurrences >= 2, ( + f"Expected geo_precision filter in both Tier H and Tier W queries, " + f"found {occurrences} occurrence(s)" + ) + + +def test_scraped_lot_has_geo_precision_field() -> None: + """ScrapedLot has geo_precision field with default None.""" + from app.services.scrapers.base import ScrapedLot + + lot = ScrapedLot( + source="cian", + source_url="https://cian.ru/sale/flat/123/", + price_rub=5_000_000, + ) + assert hasattr(lot, "geo_precision") + assert lot.geo_precision is None + + +def test_scraped_lot_accepts_city_geo_precision() -> None: + """ScrapedLot accepts geo_precision='city'.""" + from app.services.scrapers.base import ScrapedLot + + lot = ScrapedLot( + source="n1", + source_url="https://n1.ru/123/", + price_rub=3_000_000, + geo_precision="city", + ) + assert lot.geo_precision == "city"