Found by adversarial valuation audit (2 confirmed, bot-safe). FIX A (#5): both radius comp queries (Tier H ~3990, Tier W ~4135) ended with a bare ORDER BY relevance_score; on score ties Postgres returned rows in undefined order, so the same /analyze could pick different comps across runs. Append deterministic tiebreaker: relevance_score ASC, distance_m ASC, scraped_at DESC NULLS LAST, id ASC (id = listings PK → total order). Added id to each base CTE; outer projection unchanged (no leak downstream). FIX B (#2): _filter_outliers keeps rows with price_per_m2 IS NULL, but the median is built from prices_ppm2 (drops them) while n_analogs counted all of listings_clean — overstating contributing comps ("Найдено N аналогов" misleading; all-price-less -> median=0 but n_analogs>0). Count n_analogs from prices_ppm2 in the radius path. #698 anchor overwrite + #691 zero-analog->low guard unaffected; listings_clean itself not filtered. Adds tests/test_estimator_n_analogs_priced.py (verified to fail on old code). Audit also flagged velocity fan-out (false-positive: 0 duplicate domrf_obj_id on prod) and >/>= disclosure tweaks (cosmetic) — deliberately not changed. Refs #1871
131 lines
6.7 KiB
Python
131 lines
6.7 KiB
Python
"""#2 (adversarial audit) — n_analogs должен считать аналоги С ЦЕНОЙ, а не все.
|
||
|
||
Дефект: `_filter_outliers` СОХРАНЯЕТ листинги с `price_per_m2 IS NULL` (нечего
|
||
судить как outlier — keep). Затем агрегация считает медиану только по
|
||
`prices_ppm2` (price-less отброшены), но `n_analogs = len(listings_clean)` считал
|
||
ВСЕ листинги, включая безценовые. Итог: «Найдено N аналогов» завышен, а в пределе
|
||
(все радиусные аналоги без цены) median_ppm2=0 при n_analogs>0.
|
||
|
||
Фикс: в radius-aggregation ветке `n_analogs = len(prices_ppm2)` — число аналогов,
|
||
реально внёсших цену. listings_clean НЕ фильтруется (downstream analogs_lots /
|
||
sources_used / repair_coef сохраняют полное выживание outlier-фильтра).
|
||
|
||
Контракты, которые фикс НЕ ломает (проверены отдельными тестами в
|
||
test_same_building_anchor.py): #698 anchor-путь перезаписывает n_analogs на
|
||
anchor['n']; #691 gate + _enforce_zero_analog_low ловят n_analogs==0 → 'low'.
|
||
|
||
Полный estimate-путь со всеми I/O застаблен через harness _run_estimate из
|
||
test_same_building_anchor.py (anchor_comps=[] / anchor_tier=None → radius-путь).
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import importlib.util
|
||
import os
|
||
from datetime import UTC, datetime
|
||
from pathlib import Path
|
||
from typing import Any
|
||
|
||
# Settings требует DATABASE_URL при инициализации (fail-fast, C-3).
|
||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db")
|
||
|
||
# Переиспользуем harness _run_estimate / _make_payload из anchor-теста (полный
|
||
# estimate путь со всеми I/O застабленными). anchor_comps=[] + anchor_tier=None
|
||
# направляет estimate в radius-aggregation ветку (где живёт фикс #2).
|
||
_ANCHOR_TEST = Path(__file__).parent / "test_same_building_anchor.py"
|
||
_spec = importlib.util.spec_from_file_location("_anchor_harness_n_analogs", _ANCHOR_TEST)
|
||
assert _spec is not None and _spec.loader is not None
|
||
_h = importlib.util.module_from_spec(_spec)
|
||
_spec.loader.exec_module(_h)
|
||
|
||
|
||
def _listing(price_per_m2: float | None, area_m2: float = 60.0) -> dict[str, Any]:
|
||
"""Радиусный аналог; price_per_m2=None моделирует листинг без ₽/м².
|
||
|
||
Это и есть прод-сценарий дефекта: SQL-фильтр гарантирует price_rub>0, но
|
||
price_per_m2 — отдельно вычисляемая колонка и может быть NULL (напр. area_m2
|
||
отсутствует → ₽/м² не определён). _filter_outliers СОХРАНЯЕТ такой листинг
|
||
(нечего судить по ₽/м²), он попадает в listings_clean, но НЕ в prices_ppm2.
|
||
price_rub остаётся валидным (как в проде, WHERE price_rub > 0), поэтому
|
||
_listing_to_analog не падает — дефект именно в счётчике, не в построении.
|
||
"""
|
||
return {
|
||
"source": "cian",
|
||
"source_url": "https://cian.ru/offer/1",
|
||
"address": "ЕКБ, ул. Хохрякова, 48",
|
||
"lat": 56.830,
|
||
"lon": 60.592,
|
||
"rooms": 2,
|
||
"area_m2": area_m2,
|
||
"floor": 5,
|
||
"total_floors": 14,
|
||
# price_rub всегда задан (прод WHERE price_rub > 0); ₽/м² может быть NULL.
|
||
"price_rub": (price_per_m2 * area_m2) if price_per_m2 else 9_000_000.0,
|
||
"price_per_m2": price_per_m2,
|
||
"listing_date": datetime(2026, 5, 1),
|
||
"days_on_market": 10,
|
||
"photo_urls": [],
|
||
"scraped_at": datetime(2026, 5, 20, tzinfo=UTC),
|
||
"distance_m": 0.0,
|
||
"relevance_score": 0.0,
|
||
"listing_segment": "vtorichka",
|
||
}
|
||
|
||
|
||
def test_n_analogs_counts_only_priced_radius_analogs() -> None:
|
||
"""Radius-путь: 3 аналога с ценой + 2 без цены (price_per_m2=None) пережили
|
||
outlier-фильтр → n_analogs == 3 (число ВНЁСШИХ цену), а не 5 (всего).
|
||
|
||
До фикса n_analogs = len(listings_clean) = 5 → «Найдено 5 аналогов» вводило в
|
||
заблуждение (медиана построена лишь по 3)."""
|
||
radius = [
|
||
_listing(price_per_m2=200_000.0),
|
||
_listing(price_per_m2=210_000.0),
|
||
_listing(price_per_m2=220_000.0),
|
||
_listing(price_per_m2=None),
|
||
_listing(price_per_m2=None),
|
||
]
|
||
est = _h._run_estimate(
|
||
anchor_comps=[],
|
||
anchor_tier=None,
|
||
radius_analogs=radius,
|
||
payload=_h._make_payload(area=60.0, rooms=2),
|
||
)
|
||
# n_analogs = число PRICED аналогов (3), НЕ len(listings_clean) (5).
|
||
assert est.n_analogs == 3
|
||
# Медиана построена → headline ненулевой (sanity: фикс не обнулил оценку).
|
||
assert est.median_price_rub > 0
|
||
|
||
|
||
def test_n_analogs_all_priceless_yields_zero_and_low() -> None:
|
||
"""Radius-путь, ВСЕ аналоги без цены (price_per_m2=None) → median_ppm2=0,
|
||
n_analogs=0, и confidence='low' (zero-analog guard применяется).
|
||
|
||
Это крайний случай дефекта: раньше n_analogs>0 при median=0 («Найдено N
|
||
аналогов» при нулевой цене). Теперь n_analogs=0 → _compute_confidence
|
||
(median_ppm2==0 → 'low') + _enforce_zero_analog_low не дублирует (уже 'low')."""
|
||
radius = [_listing(price_per_m2=None) for _ in range(4)]
|
||
est = _h._run_estimate(
|
||
anchor_comps=[],
|
||
anchor_tier=None,
|
||
radius_analogs=radius,
|
||
payload=_h._make_payload(area=60.0, rooms=2),
|
||
)
|
||
assert est.n_analogs == 0
|
||
assert est.median_price_rub == 0
|
||
assert est.median_price_per_m2 == 0
|
||
assert est.confidence == "low"
|
||
|
||
|
||
def test_n_analogs_all_priced_unchanged() -> None:
|
||
"""Контроль: все аналоги С ценой → n_analogs = их число (поведение не меняется,
|
||
зеркало test_radius_path_n_analogs_unchanged для priced-only выборки)."""
|
||
radius = [_listing(price_per_m2=200_000.0 + i * 1_000) for i in range(5)]
|
||
est = _h._run_estimate(
|
||
anchor_comps=[],
|
||
anchor_tier=None,
|
||
radius_analogs=radius,
|
||
payload=_h._make_payload(area=60.0, rooms=2),
|
||
)
|
||
assert est.n_analogs == 5
|
||
assert est.median_price_rub > 0
|