feat(estimator): exclude city-centroid listings from radius analogs (Refs #769 Part E) #811
5 changed files with 290 additions and 19 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
||||
|
|
|
|||
|
|
@ -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__)
|
||||
|
|
@ -69,7 +71,8 @@ async def geocode_missing_listings(
|
|||
result = GeocodeBackfillResult()
|
||||
|
||||
# 1. Найти top-N адресов с NULL coords (DESC by occurrence count)
|
||||
rows = db.execute(
|
||||
rows = (
|
||||
db.execute(
|
||||
text(
|
||||
"""
|
||||
SELECT address, COUNT(*) AS listings_count
|
||||
|
|
@ -83,7 +86,10 @@ async def geocode_missing_listings(
|
|||
"""
|
||||
),
|
||||
{"limit": batch_size},
|
||||
).mappings().all()
|
||||
)
|
||||
.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
|
||||
|
|
|
|||
44
tradein-mvp/backend/data/sql/089_listings_geo_precision.sql
Normal file
44
tradein-mvp/backend/data/sql/089_listings_geo_precision.sql
Normal file
|
|
@ -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;
|
||||
201
tradein-mvp/backend/tests/test_geo_precision.py
Normal file
201
tradein-mvp/backend/tests/test_geo_precision.py
Normal file
|
|
@ -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"
|
||||
Loading…
Add table
Reference in a new issue