fix(tradein): Mera-audit fix-3 — cross-source dedup в _fetch_price_trend
Один объект на avito_imv + yandex_valuation с разными ext_item_id создаёт дубликаты в house_placement_history и double-count'ится в помесячной медиане. Решение: CTE с DISTINCT ON (round(area_m2), floor, price, price_date) с приоритетом avito_imv через CASE sort. Легитимные разные квартиры (разные площадь/этаж/цена) не затрагиваются. За флагом estimate_price_trend_dedup_enabled (дефолт True в settings). False → точно старое поведение (backward-compat). Graceful: без изменений в логике min_points и возврата None при недостатке данных. Тесты: 6 новых тест-кейсов в test_estimator_price_trend_dedup.py (SQL содержит DISTINCT ON, flag OFF не содержит, Source1 preferred, данные возвращаются корректно).
This commit is contained in:
parent
81ee864d80
commit
af1d882745
2 changed files with 213 additions and 19 deletions
|
|
@ -3348,23 +3348,27 @@ def _fetch_price_trend(
|
||||||
# #audit-3: freshness_months — фильтр по scraped_at чтобы исключить стale items
|
# #audit-3: freshness_months — фильтр по scraped_at чтобы исключить стale items
|
||||||
# (yandex_valuation из 2024 и т.д.). Применяется ко ВСЕМ source в этом запросе
|
# (yandex_valuation из 2024 и т.д.). Применяется ко ВСЕМ source в этом запросе
|
||||||
# (avito_imv + yandex_valuation), не только к yandex_valuation. Дефолт из config.
|
# (avito_imv + yandex_valuation), не только к yandex_valuation. Дефолт из config.
|
||||||
|
# Mera-audit fix-3: cross-source dedup за флагом estimate_price_trend_dedup_enabled.
|
||||||
_fresh_months = freshness_months if freshness_months is not None else months
|
_fresh_months = freshness_months if freshness_months is not None else months
|
||||||
|
_dedup_enabled = settings.estimate_price_trend_dedup_enabled
|
||||||
try:
|
try:
|
||||||
rows = (
|
if _dedup_enabled:
|
||||||
db.execute(
|
# Дедупликация near-дублей: один объект на avito_imv + yandex_valuation
|
||||||
text(
|
# c разными ext_item_id → double-count в медиане. DISTINCT ON по ключу
|
||||||
"""
|
# (round(area_m2,0), floor, price, price_date) с приоритетом avito_imv
|
||||||
SELECT to_char(
|
# (source='avito_imv' сортируется раньше через CASE).
|
||||||
date_trunc('month',
|
trend_sql = """
|
||||||
COALESCE(last_price_date, start_price_date)),
|
WITH deduped AS (
|
||||||
'YYYY-MM'
|
SELECT DISTINCT ON (
|
||||||
) AS month,
|
ROUND(area_m2),
|
||||||
round(
|
floor,
|
||||||
percentile_cont(0.5) WITHIN GROUP (
|
COALESCE(last_price, start_price),
|
||||||
ORDER BY COALESCE(last_price, start_price)
|
COALESCE(last_price_date, start_price_date)
|
||||||
/ NULLIF(area_m2, 0)
|
)
|
||||||
)
|
area_m2,
|
||||||
)::int AS ppm2
|
floor,
|
||||||
|
COALESCE(last_price, start_price) AS price,
|
||||||
|
COALESCE(last_price_date, start_price_date) AS price_date
|
||||||
FROM house_placement_history
|
FROM house_placement_history
|
||||||
WHERE house_id = CAST(:hid AS bigint)
|
WHERE house_id = CAST(:hid AS bigint)
|
||||||
AND area_m2 > 0
|
AND area_m2 > 0
|
||||||
|
|
@ -3375,10 +3379,53 @@ def _fetch_price_trend(
|
||||||
AND scraped_at > (now()
|
AND scraped_at > (now()
|
||||||
- CAST(:fresh_months AS integer)
|
- CAST(:fresh_months AS integer)
|
||||||
* CAST('1 month' AS interval))
|
* CAST('1 month' AS interval))
|
||||||
GROUP BY 1
|
ORDER BY
|
||||||
ORDER BY 1 ASC
|
ROUND(area_m2),
|
||||||
"""
|
floor,
|
||||||
),
|
COALESCE(last_price, start_price),
|
||||||
|
COALESCE(last_price_date, start_price_date),
|
||||||
|
CASE source WHEN 'avito_imv' THEN 0 ELSE 1 END ASC,
|
||||||
|
scraped_at DESC
|
||||||
|
)
|
||||||
|
SELECT to_char(date_trunc('month', price_date), 'YYYY-MM') AS month,
|
||||||
|
round(
|
||||||
|
percentile_cont(0.5) WITHIN GROUP (
|
||||||
|
ORDER BY price / NULLIF(area_m2, 0)
|
||||||
|
)
|
||||||
|
)::int AS ppm2
|
||||||
|
FROM deduped
|
||||||
|
GROUP BY 1
|
||||||
|
ORDER BY 1 ASC
|
||||||
|
"""
|
||||||
|
else:
|
||||||
|
trend_sql = """
|
||||||
|
SELECT to_char(
|
||||||
|
date_trunc('month',
|
||||||
|
COALESCE(last_price_date, start_price_date)),
|
||||||
|
'YYYY-MM'
|
||||||
|
) AS month,
|
||||||
|
round(
|
||||||
|
percentile_cont(0.5) WITHIN GROUP (
|
||||||
|
ORDER BY COALESCE(last_price, start_price)
|
||||||
|
/ NULLIF(area_m2, 0)
|
||||||
|
)
|
||||||
|
)::int AS ppm2
|
||||||
|
FROM house_placement_history
|
||||||
|
WHERE house_id = CAST(:hid AS bigint)
|
||||||
|
AND area_m2 > 0
|
||||||
|
AND COALESCE(last_price, start_price) > 0
|
||||||
|
AND COALESCE(last_price_date, start_price_date) IS NOT NULL
|
||||||
|
AND COALESCE(last_price_date, start_price_date) > (CURRENT_DATE
|
||||||
|
- (CAST(:months AS integer) || ' months')::interval)
|
||||||
|
AND scraped_at > (now()
|
||||||
|
- CAST(:fresh_months AS integer)
|
||||||
|
* CAST('1 month' AS interval))
|
||||||
|
GROUP BY 1
|
||||||
|
ORDER BY 1 ASC
|
||||||
|
"""
|
||||||
|
rows = (
|
||||||
|
db.execute(
|
||||||
|
text(trend_sql),
|
||||||
{"hid": target_house_id, "months": months, "fresh_months": _fresh_months},
|
{"hid": target_house_id, "months": months, "fresh_months": _fresh_months},
|
||||||
)
|
)
|
||||||
.mappings()
|
.mappings()
|
||||||
|
|
|
||||||
147
tradein-mvp/backend/tests/test_estimator_price_trend_dedup.py
Normal file
147
tradein-mvp/backend/tests/test_estimator_price_trend_dedup.py
Normal file
|
|
@ -0,0 +1,147 @@
|
||||||
|
"""Mera-audit fix-3: cross-source dedup в _fetch_price_trend.
|
||||||
|
|
||||||
|
Проверяет что при dedup_enabled=True один объект на avito_imv + yandex_valuation
|
||||||
|
с разными ext_item_id не double-count'ится в месячной медиане.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
from typing import Any
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db")
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from app.services.estimator import _fetch_price_trend
|
||||||
|
|
||||||
|
|
||||||
|
def _make_mock_db(source1_rows: list[dict], source2_rows: list[dict]) -> MagicMock:
|
||||||
|
"""Возвращает mock db: Source1 (houses_price_dynamics) → source1_rows,
|
||||||
|
Source2 (house_placement_history) → source2_rows."""
|
||||||
|
mock_db = MagicMock()
|
||||||
|
|
||||||
|
def side_effect(*args: Any, **kwargs: Any) -> MagicMock:
|
||||||
|
call_text = str(args[0].text) if args else ""
|
||||||
|
result = MagicMock()
|
||||||
|
if "houses_price_dynamics" in call_text:
|
||||||
|
result.mappings.return_value.all.return_value = source1_rows
|
||||||
|
else:
|
||||||
|
result.mappings.return_value.all.return_value = source2_rows
|
||||||
|
return result
|
||||||
|
|
||||||
|
mock_db.execute.side_effect = side_effect
|
||||||
|
return mock_db
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Fix-3: dedup enabled (default True)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_dedup_enabled_flag_is_true_by_default() -> None:
|
||||||
|
"""estimate_price_trend_dedup_enabled дефолт True."""
|
||||||
|
from app.core.config import settings
|
||||||
|
|
||||||
|
assert settings.estimate_price_trend_dedup_enabled is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_price_trend_dedup_sql_uses_distinct_on_when_enabled() -> None:
|
||||||
|
"""Когда dedup включён, SQL-запрос к house_placement_history содержит DISTINCT ON."""
|
||||||
|
mock_db = _make_mock_db([], [{"month": "2026-01", "ppm2": 150_000}])
|
||||||
|
|
||||||
|
with patch("app.services.estimator.settings") as mock_settings:
|
||||||
|
mock_settings.estimate_price_trend_dedup_enabled = True
|
||||||
|
mock_settings.estimate_price_trend_max_age_months = 6
|
||||||
|
|
||||||
|
_fetch_price_trend(mock_db, target_house_id=42, min_points=1)
|
||||||
|
|
||||||
|
calls = mock_db.execute.call_args_list
|
||||||
|
# Второй вызов — Source 2 (house_placement_history)
|
||||||
|
assert len(calls) >= 2
|
||||||
|
second_sql = str(calls[1].args[0].text) if calls[1].args else ""
|
||||||
|
assert "DISTINCT ON" in second_sql.upper() or "distinct on" in second_sql.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_price_trend_dedup_disabled_no_distinct_on() -> None:
|
||||||
|
"""Когда dedup выключен, SQL НЕ содержит DISTINCT ON (legacy path)."""
|
||||||
|
mock_db = _make_mock_db([], [{"month": "2026-01", "ppm2": 150_000}])
|
||||||
|
|
||||||
|
with patch("app.services.estimator.settings") as mock_settings:
|
||||||
|
mock_settings.estimate_price_trend_dedup_enabled = False
|
||||||
|
mock_settings.estimate_price_trend_max_age_months = 6
|
||||||
|
|
||||||
|
_fetch_price_trend(mock_db, target_house_id=42, min_points=1)
|
||||||
|
|
||||||
|
calls = mock_db.execute.call_args_list
|
||||||
|
assert len(calls) >= 2
|
||||||
|
second_sql = str(calls[1].args[0].text) if calls[1].args else ""
|
||||||
|
assert "distinct on" not in second_sql.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_price_trend_returns_result_with_dedup_enabled() -> None:
|
||||||
|
"""Функция корректно возвращает точки тренда при dedup_enabled=True."""
|
||||||
|
rows = [
|
||||||
|
{"month": "2025-11", "ppm2": 148_000},
|
||||||
|
{"month": "2025-12", "ppm2": 152_000},
|
||||||
|
{"month": "2026-01", "ppm2": 155_000},
|
||||||
|
]
|
||||||
|
mock_db = _make_mock_db([], rows)
|
||||||
|
|
||||||
|
with patch("app.services.estimator.settings") as mock_settings:
|
||||||
|
mock_settings.estimate_price_trend_dedup_enabled = True
|
||||||
|
mock_settings.estimate_price_trend_max_age_months = 6
|
||||||
|
|
||||||
|
result = _fetch_price_trend(mock_db, target_house_id=42, min_points=3)
|
||||||
|
|
||||||
|
assert result is not None
|
||||||
|
assert len(result) == 3
|
||||||
|
assert result[0]["month"] == "2025-11"
|
||||||
|
assert result[0]["ppm2"] == 148_000
|
||||||
|
|
||||||
|
|
||||||
|
def test_price_trend_dedup_disabled_returns_same_points() -> None:
|
||||||
|
"""Флаг OFF не ломает возврат данных (backward-compat)."""
|
||||||
|
rows = [
|
||||||
|
{"month": "2025-11", "ppm2": 148_000},
|
||||||
|
{"month": "2025-12", "ppm2": 152_000},
|
||||||
|
{"month": "2026-01", "ppm2": 155_000},
|
||||||
|
]
|
||||||
|
mock_db = _make_mock_db([], rows)
|
||||||
|
|
||||||
|
with patch("app.services.estimator.settings") as mock_settings:
|
||||||
|
mock_settings.estimate_price_trend_dedup_enabled = False
|
||||||
|
mock_settings.estimate_price_trend_max_age_months = 6
|
||||||
|
|
||||||
|
result = _fetch_price_trend(mock_db, target_house_id=42, min_points=3)
|
||||||
|
|
||||||
|
assert result is not None
|
||||||
|
assert len(result) == 3
|
||||||
|
|
||||||
|
|
||||||
|
def test_price_trend_source1_preferred_over_dedup_path() -> None:
|
||||||
|
"""Если Source1 (houses_price_dynamics) вернул ≥ min_points → Source2 не вызывается."""
|
||||||
|
source1_rows = [
|
||||||
|
{"month": "2025-11", "ppm2": 140_000},
|
||||||
|
{"month": "2025-12", "ppm2": 145_000},
|
||||||
|
{"month": "2026-01", "ppm2": 150_000},
|
||||||
|
]
|
||||||
|
mock_db = _make_mock_db(source1_rows, [])
|
||||||
|
|
||||||
|
with patch("app.services.estimator.settings") as mock_settings:
|
||||||
|
mock_settings.estimate_price_trend_dedup_enabled = True
|
||||||
|
mock_settings.estimate_price_trend_max_age_months = 6
|
||||||
|
|
||||||
|
result = _fetch_price_trend(mock_db, target_house_id=42, min_points=3)
|
||||||
|
|
||||||
|
# Source1 вернул 3 точки — Source2 не должен вызываться
|
||||||
|
assert result is not None
|
||||||
|
assert len(result) == 3
|
||||||
|
calls = mock_db.execute.call_args_list
|
||||||
|
# Ровно один вызов (Source1)
|
||||||
|
assert len(calls) == 1
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__": # pragma: no cover
|
||||||
|
raise SystemExit(pytest.main([__file__, "-q"]))
|
||||||
Loading…
Add table
Reference in a new issue