All checks were successful
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 55s
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 8s
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
Two display-layer bugs surfaced by pre-launch smoke (golden-path address "Екатеринбург, улица Малышева, 51"): 1. n_analogs (same-building anchor path) was computed from the post-MAD-clip comp population (_compute_same_building_anchor: n = len(surviving ppm2)), but the displayed analogs cards were built from the ORIGINAL, un-clipped anchor_comps list. A MAD-clip-excluded price outlier (e.g. a cross-source duplicate priced ~4.5x above the rest) never affected the median/n_analogs, yet still showed up as a card. Fix: _compute_same_building_anchor now returns the final surviving "comps" list (len == n), and the caller sets anchor_comps_used = anchor["comps"] instead of the raw pre-clip anchor_comps. 2. The statistical cross-source dedup (_dedup_cross_source) requires the same price_bucket (rounded price_rub) to consider two listings duplicates. Real cross-posts of the same physical unit almost always have a small asking- price drift between platforms, which can straddle a bucket boundary and let the duplicate survive into both the price population and the UI. Fix: a new display-only dedup pass (_dedup_display_lots / _union_find_phys_dedup) uses the same physical key (cadnum-or-street + floor + area_bucket) but WITHOUT price_bucket, applied only when building the displayed analogs_lots — never touches n_analogs/median/cv. To avoid over-merging genuinely distinct same-building units that happen to share floor/area, it requires the two lots to come from DIFFERENT sources (require_diff_source guard) — same-source pairs are left untouched. The radius-path (non-anchor) branch had a related count-consistency gap: n_analogs counted only PRICED listings_clean entries, while the displayed cards were sliced from the full (unfiltered) listings_clean, occasionally letting price-less entries inflate the card count beyond n_analogs. Cards are now built from the same priced subset. Frozen backtest regression gate (tests/test_backtest_regression_gate.py, hermetic replay of committed fixture) is byte-identical after this change — zero delta on median/expected_sold/coverage/calibration, confirming this is display-only and does not touch pricing. Added tests/test_estimator_analogs_display_consistency.py: full-pipeline regression for both root causes (outlier duplicate pair fully excluded from both count and cards; non-outlier duplicate pair collapses in cards only, honestly documenting len(analogs) <= n_analogs when the display-only dedup catches a residual cross-source dup the stat dedup missed) + unit tests on _dedup_display_lots. Updated test_estimator_quarter_index.py's hand-rolled _compute_same_building_anchor test double to match the new "comps" contract.
261 lines
15 KiB
Python
261 lines
15 KiB
Python
"""Display-consistency fix — показанные карточки аналогов согласованы с n_analogs
|
||
и не содержат ни MAD-clip-отсечённых ценовых выбросов, ни кросс-source дублей
|
||
одного физлота (live QA прод-смоук: «Екатеринбург, ул. Малышева, 51»).
|
||
|
||
Два независимых root cause (см. PR):
|
||
|
||
1. `n_analogs` считался ПОСЛЕ MAD-clip (_compute_same_building_anchor: n = len
|
||
выживших ppm2), но отображаемые карточки строились из ИСХОДНОГО (пред-clip)
|
||
anchor_comps — клип-отсечённый ценовой выброс оставался видимым в UI, даже
|
||
когда он не участвовал в headline/n_analogs. Фикс: anchor_comps_used теперь =
|
||
anchor["comps"] (тот же пост-clip пул, что дал n).
|
||
|
||
2. Статистический кросс-source дедуп (_dedup_cross_source) требует совпадения
|
||
ЦЕНОВОГО бакета (price_bucket) — реальный кросс-пост одного физлота на разных
|
||
площадках почти всегда имеет небольшой ценовой дрейф (перевыставили дешевле/
|
||
дороже), который может увести пару в разные бакеты → дубль выживает в
|
||
популяции. Фикс: отдельный display-only дедуп (_dedup_display_lots) — тот же
|
||
физический ключ (building/floor/area), но БЕЗ price_bucket — ловит именно эти
|
||
остаточные дубли ТОЛЬКО на уровне отображаемых карточек, не трогая
|
||
n_analogs/median/cv (frozen backtest regression gate не задет — см.
|
||
tests/test_backtest_regression_gate.py, который эту ветку кода не вызывает).
|
||
|
||
Test A воспроизводит буквальный прод-паттерн (кросс-source дубль-пара, которая
|
||
ЦЕЛИКОМ является ценовым выбросом, — «Гоголя 18» из QA-смоука): MAD-clip убирает
|
||
ОБЕ копии из anchor["comps"] → они пропадают из карточек естественным образом
|
||
(fix #1 самодостаточен) → len(analogs) == n_analogs И нет дубль-адресов.
|
||
|
||
Test B покрывает менее тривиальный случай — дубль-пара НЕ является ценовым
|
||
выбросом (обычная цена, просто с небольшим дрейфом между площадками) и
|
||
переживает MAD-clip как 2 отдельные записи (её и ловит fix #2, display-only
|
||
дедуп). Здесь len(analogs) < n_analogs ЧЕСТНО (статистика не трогается) — тест
|
||
документирует именно это, а не искусственное равенство.
|
||
|
||
Полный estimate-путь со всеми I/O застаблен через harness _run_estimate из
|
||
test_same_building_anchor.py (anchor_tier="A" → same-building anchor ветка, где
|
||
живёт MAD-clip).
|
||
"""
|
||
|
||
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")
|
||
|
||
from app.services import estimator
|
||
|
||
_ANCHOR_TEST = Path(__file__).parent / "test_same_building_anchor.py"
|
||
_spec = importlib.util.spec_from_file_location("_anchor_harness_display_consistency", _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 _comp(
|
||
*,
|
||
source: str,
|
||
address: str,
|
||
ppm2: float,
|
||
area_m2: float = 146.2,
|
||
rooms: int = 4,
|
||
scraped_at: datetime | None = None,
|
||
) -> dict[str, Any]:
|
||
"""Минимальный same-building comp-dict (без floor/total_floors → нейтральный
|
||
floor-вес; area_m2 по умолчанию = target площадь _make_payload(), area-вес ~1.0).
|
||
"""
|
||
return {
|
||
"source": source,
|
||
"source_url": f"https://{source}.example/offer/{address}/{ppm2}",
|
||
"address": address,
|
||
"area_m2": area_m2,
|
||
"rooms": rooms,
|
||
"price_per_m2": ppm2,
|
||
"price_rub": ppm2 * area_m2,
|
||
"scraped_at": scraped_at or datetime(2026, 6, 1, tzinfo=UTC),
|
||
"photo_urls": [],
|
||
}
|
||
|
||
|
||
# --------------------------------------------------------------------------- #
|
||
# Test A — outlier дубль-пара («Гоголя 18»-паттерн): MAD-clip убирает ОБЕ копии
|
||
# --------------------------------------------------------------------------- #
|
||
|
||
|
||
def test_outlier_duplicate_pair_excluded_from_both_count_and_cards() -> None:
|
||
"""5 обычных комплов (разные дома, MAD-clip не трогает) + кросс-source
|
||
дубль-пара одного физлота, которая ЦЕЛИКОМ ценовой выброс (~4.4× медианы,
|
||
как «Гоголя 18» в QA-смоуке) → MAD-clip убирает ОБЕ копии из
|
||
anchor["comps"]. Т.к. карточки теперь строятся из anchor["comps"] (не из
|
||
исходного anchor_comps), выброс не просачивается в UI — количество карточек
|
||
равно n_analogs, и обеих копий дубля нет ни в счётчике, ни в карточках.
|
||
"""
|
||
clean = [
|
||
_comp(source="cian", address="Екатеринбург, ул. Хохрякова, 48", ppm2=195_000.0),
|
||
_comp(source="avito", address="Екатеринбург, ул. Хохрякова, 50", ppm2=200_000.0),
|
||
_comp(source="domklik", address="Екатеринбург, ул. Хохрякова, 52", ppm2=205_000.0),
|
||
_comp(source="yandex", address="Екатеринбург, ул. Хохрякова, 54", ppm2=210_000.0),
|
||
_comp(source="cian", address="Екатеринбург, ул. Хохрякова, 58", ppm2=215_000.0),
|
||
]
|
||
# Кросс-source дубль одного физлота — ~4.4-4.5× дороже остальных (целиком
|
||
# ценовой выброс, как реальный прод-кейс «Гоголя 18»).
|
||
outlier_dup = [
|
||
_comp(source="avito", address="Екатеринбург, ул. Гоголя, 18", ppm2=900_000.0),
|
||
_comp(source="domklik", address="Екатеринбург, ул. Гоголя, 18", ppm2=920_000.0),
|
||
]
|
||
|
||
est = _h._run_estimate(anchor_comps=clean + outlier_dup, anchor_tier="A")
|
||
|
||
# Выброс не участвовал в headline/n_analogs (MAD-clip) — фикс #1 не менял эту
|
||
# часть: сверяем, что n_analogs действительно = 5 (7 - 2 отсечённых).
|
||
assert est.n_analogs == 5
|
||
|
||
# Главный инвариант display-consistency: показанные карточки == n_analogs.
|
||
assert len(est.analogs) == est.n_analogs == 5
|
||
|
||
# Ни одна из копий выброса не попала в карточки.
|
||
shown_addresses = {a.address for a in est.analogs}
|
||
assert "Екатеринбург, ул. Гоголя, 18" not in shown_addresses
|
||
shown_ppm2 = {a.price_per_m2 for a in est.analogs}
|
||
assert 900_000 not in shown_ppm2
|
||
assert 920_000 not in shown_ppm2
|
||
|
||
# Нет дублей адресов среди карточек.
|
||
assert len(shown_addresses) == len(est.analogs)
|
||
|
||
|
||
# --------------------------------------------------------------------------- #
|
||
# Test B — non-outlier дубль-пара («Пушкина 9»-паттерн): переживает MAD-clip,
|
||
# ловится ТОЛЬКО display-only дедупом (price_bucket-строгий стат-дедуп её
|
||
# пропускает из-за ценового дрейфа, бакет-straddle).
|
||
# --------------------------------------------------------------------------- #
|
||
|
||
|
||
def test_non_outlier_cross_source_duplicate_collapses_in_cards_only() -> None:
|
||
"""4 обычных компла + кросс-source дубль-пара ОБЫЧНОЙ цены (небольшой дрейф
|
||
2-25к, «Пушкина 9»-паттерн из QA-смоука) — НЕ выброс, переживает MAD-clip как
|
||
2 отдельные записи (n_analogs их считает по отдельности, статистика не
|
||
тронута). display-only дедуп (без price_bucket) схлопывает пару в ОДНУ
|
||
карточку — карточек становится МЕНЬШЕ n_analogs (честно: это НЕ баг, а
|
||
следствие того, что стат-пайплайн и display-дедуп используют разные ключи
|
||
по дизайну — см. docstring _dedup_display_lots). Инвариант, который держит
|
||
этот тест: адрес дубля встречается в карточках РОВНО один раз.
|
||
"""
|
||
clean = [
|
||
_comp(source="cian", address="Екатеринбург, ул. Хохрякова, 48", ppm2=195_000.0),
|
||
_comp(source="avito", address="Екатеринбург, ул. Хохрякова, 50", ppm2=200_000.0),
|
||
_comp(source="domklik", address="Екатеринбург, ул. Хохрякова, 52", ppm2=205_000.0),
|
||
_comp(source="yandex", address="Екатеринбург, ул. Хохрякова, 54", ppm2=210_000.0),
|
||
]
|
||
# Один физлот, кросс-source (yandex/domklik), лёгкий ценовой дрейф ~1.6% —
|
||
# достаточно, чтобы price_bucket (_DEDUP_PRICE_BUCKET_RUB=100_000 руб.)
|
||
# развёл пару по РАЗНЫМ бакетам (311 vs 316), но НЕ выброс (в пределах
|
||
# MAD-clip диапазона остальных 4).
|
||
dup_a = _comp(
|
||
source="yandex",
|
||
address="Екатеринбург, ул. Пушкина, 9",
|
||
ppm2=219_000.0,
|
||
area_m2=142.0,
|
||
scraped_at=datetime(2026, 6, 1, tzinfo=UTC),
|
||
)
|
||
dup_b = _comp(
|
||
source="domklik",
|
||
address="Екатеринбург, ул. Пушкина, 9",
|
||
ppm2=222_500.0,
|
||
area_m2=142.0,
|
||
scraped_at=datetime(2026, 6, 3, tzinfo=UTC), # свежее → представитель
|
||
)
|
||
|
||
# sanity: стат-дедуп (price_bucket-строгий) ДЕЙСТВИТЕЛЬНО пропускает пару —
|
||
# иначе тест проверял бы не то, что заявлено в docstring.
|
||
assert estimator.settings.estimate_dedup_analogs_enabled is True
|
||
deduped_upstream = estimator._dedup_cross_source([dup_a, dup_b])
|
||
assert len(deduped_upstream) == 2, "price_bucket dedup ошибочно поймал дрейф — тест не то мерит"
|
||
|
||
est = _h._run_estimate(anchor_comps=[*clean, dup_a, dup_b], anchor_tier="A")
|
||
|
||
# Статистика ЧЕСТНО считает обе копии (стат-дедуп их не поймал, MAD-clip не
|
||
# выброс) — n_analogs = 6.
|
||
assert est.n_analogs == 6
|
||
|
||
# display-only дедуп схлопнул пару → карточек МЕНЬШЕ n_analogs.
|
||
assert len(est.analogs) == 5
|
||
assert len(est.analogs) < est.n_analogs
|
||
|
||
# Инвариант: адрес дубля встречается РОВНО один раз среди карточек.
|
||
shown_addresses = [a.address for a in est.analogs]
|
||
assert shown_addresses.count("Екатеринбург, ул. Пушкина, 9") == 1
|
||
# Представитель — свежайший scraped_at (domklik, 222_500).
|
||
pushkina_card = next(a for a in est.analogs if a.address == "Екатеринбург, ул. Пушкина, 9")
|
||
assert pushkina_card.price_per_m2 == 222_500
|
||
|
||
|
||
# --------------------------------------------------------------------------- #
|
||
# Unit-level: _dedup_display_lots — прямые проверки поведения helper'а
|
||
# --------------------------------------------------------------------------- #
|
||
|
||
|
||
def _lot(
|
||
*,
|
||
source: str,
|
||
address: str = "ул. Ленина, 5",
|
||
area: float = 60.0,
|
||
price: float = 12_000_000.0,
|
||
floor: int | None = 5,
|
||
scraped_at: datetime | None = None,
|
||
) -> dict[str, Any]:
|
||
return {
|
||
"source": source,
|
||
"address": address,
|
||
"area_m2": area,
|
||
"price_rub": price,
|
||
"price_per_m2": price / area if area else 0,
|
||
"floor": floor,
|
||
"scraped_at": scraped_at or datetime(2026, 6, 1, tzinfo=UTC),
|
||
"photo_urls": [],
|
||
}
|
||
|
||
|
||
def test_dedup_display_lots_collapses_price_drifted_cross_source_pair() -> None:
|
||
# Тот же building/floor/area, РАЗНЫЙ source, цена отличается на 20% — стат-
|
||
# дедуп (price_bucket) не поймал бы такой дрейф, display-дедуп игнорирует
|
||
# цену вовсе.
|
||
lots = [
|
||
_lot(source="avito", price=12_000_000.0, scraped_at=datetime(2026, 6, 1, tzinfo=UTC)),
|
||
_lot(source="cian", price=14_400_000.0, scraped_at=datetime(2026, 6, 5, tzinfo=UTC)),
|
||
]
|
||
out = estimator._dedup_display_lots(lots)
|
||
assert len(out) == 1
|
||
assert out[0]["source"] == "cian" # свежайший scraped_at
|
||
|
||
|
||
def test_dedup_display_lots_same_source_pair_not_merged() -> None:
|
||
# Тот же building/floor/area, ОДИНАКОВЫЙ source, разная цена — НЕ кросс-пост
|
||
# (либо два разных реальных юнита, случайно совпавших по округлённой
|
||
# площади/этажу), display-дедуп НЕ должен их схлопывать (require_diff_source
|
||
# guard).
|
||
lots = [
|
||
_lot(source="cian", price=12_000_000.0),
|
||
_lot(source="cian", price=13_000_000.0),
|
||
]
|
||
out = estimator._dedup_display_lots(lots)
|
||
assert len(out) == 2
|
||
|
||
|
||
def test_dedup_display_lots_different_addresses_stay_distinct() -> None:
|
||
lots = [
|
||
_lot(source="avito", address="ул. Ленина, 5", price=12_000_000.0),
|
||
_lot(source="cian", address="ул. Мира, 3", price=12_000_000.0),
|
||
]
|
||
out = estimator._dedup_display_lots(lots)
|
||
assert len(out) == 2
|
||
|
||
|
||
def test_dedup_display_lots_noop_under_two_lots() -> None:
|
||
lots = [_lot(source="avito")]
|
||
out = estimator._dedup_display_lots(lots)
|
||
assert out is lots
|