lekss361/gendesign
1
0
Fork 0
Code Issues 57 Pull requests Projects Releases Packages Wiki Activity Actions

feat(estimator): exclude city-centroid listings from radius analogs (Refs #769 Part E)

Browse source 
Закрывает последнюю часть #769 (A1/A2/B/C/D в #798/#804). Finding #17:
bare-city адреса геокодились в city-центроид и сохранялись как точные lat/lon →
mislocated листинги участвовали в radius ST_DWithin-аналогах.

- 089: ALTER TABLE listings ADD COLUMN IF NOT EXISTS geo_precision text (idempot).
- ScrapedLot.geo_precision + проброс в save_listings.
- geocode_missing.py: geo_precision='city' через существующий _geocode_is_coarse.
- estimator _fetch_analogs Tier H+W: + AND (geo_precision IS DISTINCT FROM 'city').
  NULL проходит (консервативно, сдвига до backfill нет). Pricing не тронут.

7 тестов; 270 passed, ruff clean.
This commit is contained in:
bot-backend 2026-05-30 21:22:21 +03:00
parent 950d0f60de
commit b736d7b7e0
5 changed files with 290 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
tradein-mvp/backend/app/services/estimator.py
View file

@ -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,

9
tradein-mvp/backend/app/services/scrapers/base.py
View file

@ -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()

49
tradein-mvp/backend/app/tasks/geocode_missing.py
View file

@ -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

44
tradein-mvp/backend/data/sql/089_listings_geo_precision.sql Normal file
View 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
View 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"