From 159a02a80097ca783b52bf8106edde8c1cc18384 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 6 Jun 2026 22:16:15 +0300 Subject: [PATCH] test(tradein): quality-gate tests for 5 P0 audit fixes (Refs #781) Anti-regression guards for PRs #877/#761/#784/#779/#743 (issues #755/#753/#773/#754/#740). All 5 defects had no dedicated lock test -- any refactor could silently re-introduce them. New file test_781_quality_gate.py (13 tests): #755: anchor n=2 < min_comps=4 does not fire; pure-unit confidence cap (n<5 != high) #753: compute_dedup_hash stable across reprice + ?context= query strip; source_id priority #773: expected_sold_price_rub > 0 on anchor-only path (radius=[]) when ratio present #754: page=1 HTTP-200 block raises AvitoContentBlockedError; page>1 graceful return #740: AggregatedEstimate.insufficient_data=True when median=0; survives model_dump() Updated test_estimator_pure_units.py: test_filter_outliers_passthrough_below_5_lots -- added docstring clarifying this tests the radius-path _filter_outliers (n<5 bypass Tukey is unchanged by #755); #755 added MAD-clip only to _compute_same_building_anchor, not to the radius-path filter. 325 tests pass under -k anchor|dedup|expected_sold|avito|insufficient|filter_outliers. ruff clean. --- .../backend/tests/test_781_quality_gate.py | 524 ++++++++++++++++++ .../tests/test_estimator_pure_units.py | 10 + 2 files changed, 534 insertions(+) create mode 100644 tradein-mvp/backend/tests/test_781_quality_gate.py diff --git a/tradein-mvp/backend/tests/test_781_quality_gate.py b/tradein-mvp/backend/tests/test_781_quality_gate.py new file mode 100644 index 00000000..feffc348 --- /dev/null +++ b/tradein-mvp/backend/tests/test_781_quality_gate.py @@ -0,0 +1,524 @@ +"""Quality-gate tests locking the 5 P0 audit fixes (issue #781). + +One test per merged defect -- anti-regression guards so that future refactors +cannot silently re-introduce the same bugs. + +PRs verified merged into main before writing any assert: + #877 / commit d1034bb -- anchor safe-guard (#755) + #761 / commit 79a7900 -- stable dedup_hash (#753) + #784 / commit ec84637 -- expected_sold guard (#773) + #779 / commit 489666c -- avito block-page HTTP-200 (#754) + #743 / commit 798fb12 -- insufficient_data flag (#740) +""" + +from __future__ import annotations + +import os +from datetime import UTC, datetime +from pathlib import Path +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +import anyio +import pytest + +# Settings requires DATABASE_URL at init time. +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db") + + +# --------------------------------------------------------------------------- +# Helpers shared across tests +# --------------------------------------------------------------------------- + + +def _make_listing_qa(*, price_per_m2: float, area_m2: float = 60.0) -> dict[str, Any]: + return { + "source": "cian", + "source_url": "https://cian.ru/offer/1", + "address": "EKB, ul. Khokryakova, 48", + "lat": 56.830, + "lon": 60.592, + "rooms": 2, + "area_m2": area_m2, + "floor": 5, + "total_floors": 14, + "price_rub": int(price_per_m2 * area_m2), + "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", + } + + +_RADIUS_ANALOGS_QA: list[dict[str, Any]] = [ + _make_listing_qa(price_per_m2=200_000.0), + _make_listing_qa(price_per_m2=210_000.0), + _make_listing_qa(price_per_m2=220_000.0), +] + +# 2 comps -- below min_comps=4 threshold introduced by #755. +_TWO_COMPS: list[dict[str, Any]] = [ + {"price_per_m2": 399_478, "area_m2": 153.2, "rooms": 3}, + {"price_per_m2": 472_298, "area_m2": 110.1, "rooms": 3}, +] + +# 4 comps satisfying min_comps=4 (#755). +_SB_COMPS_QG: list[dict[str, Any]] = [ + {"price_per_m2": 399_478, "area_m2": 153.2, "rooms": 3}, + {"price_per_m2": 472_298, "area_m2": 110.1, "rooms": 3}, + {"price_per_m2": 683_995, "area_m2": 146.2, "rooms": 4}, + {"price_per_m2": 510_000, "area_m2": 125.0, "rooms": 3}, +] + + +def _run_qa_estimate( + *, + anchor_comps: list[dict[str, Any]], + anchor_tier: str | None, + ratio_tuple: tuple[float | None, str | None] = (0.92, "per_rooms"), + radius_analogs: list[dict[str, Any]] | None = None, +) -> Any: + from app.core.config import settings + from app.schemas.trade_in import TradeInEstimateInput + from app.services.estimator import estimate_quality + from app.services.geocoder import GeocodeResult + + db = MagicMock() + payload = TradeInEstimateInput( + address="Ekaterinburg, ul. Khokryakova, 48", + area_m2=60.0, + rooms=2, + floor=5, + total_floors=14, + ) + geo = GeocodeResult( + lat=56.830, + lon=60.592, + full_address="Sverdlovskaya obl., Ekaterinburg, ul. Khokryakova, 48", + provider="nominatim", + ) + radius = list(_RADIUS_ANALOGS_QA if radius_analogs is None else radius_analogs) + + async def _run() -> Any: + with ( + patch.object(settings, "estimate_same_building_anchor_enabled", True), + patch("app.services.estimator.geocode", new=AsyncMock(return_value=geo)), + patch("app.services.estimator.dadata_clean_address", new=AsyncMock(return_value=None)), + patch("app.services.estimator.match_house_readonly", return_value=None), + patch("app.services.estimator.get_house_metadata", new=AsyncMock(return_value=None)), + patch("app.services.estimator._fetch_analogs", return_value=(radius, False, "W")), + patch("app.services.estimator._fetch_deals", return_value=[]), + patch("app.services.estimator._fetch_dkp_corridor", return_value=None), + patch( + "app.services.estimator._fetch_anchor_comps", + return_value=(list(anchor_comps), anchor_tier), + ), + patch("app.services.estimator._fetch_house_imv_anchor", return_value=None), + patch( + "app.services.estimator._get_or_fetch_imv_cached", + new=AsyncMock(return_value=None), + ), + patch( + "app.services.estimator._get_or_fetch_yandex_valuation_cached", + new=AsyncMock(return_value=None), + ), + patch( + "app.services.estimator.estimate_via_cian_valuation", + new=AsyncMock(return_value=None), + ), + patch("app.services.estimator._get_asking_sold_ratio", return_value=ratio_tuple), + ): + return await estimate_quality(payload, db) + + return anyio.run(_run) + + +# --------------------------------------------------------------------------- +# Test 1 -- Anchor confidence n=2 (#755) +# +# Same-building anchor with n_comps < min_comps threshold (2 < 4) must NOT +# fire at all in the full estimate path: headline stays at the radius median +# (210 000 rub/m2), and confidence is never "high" from an under-populated +# anchor. The DoD of #755 is: min_comps 2->4; confidence cap n<5->medium; +# MAD-clip. +# --------------------------------------------------------------------------- + + +def test_755_anchor_n2_does_not_fire_headline_stays_radius() -> None: + """#755 quality-gate: 2 same-building comps < min_comps=4 -> anchor does NOT fire. + + Headline stays at the radius median (210 000 rub/m2) and is NOT inflated by + an under-populated uplift. Before #755, min_comps was 2, so this anchor + would have fired and replaced the headline with the premium anchor value. + Refs: PR #877 / commit d1034bb. + """ + est = _run_qa_estimate(anchor_comps=_TWO_COMPS, anchor_tier="A") + # Anchor did not fire -> headline is the radius-path median (210 000). + assert est.median_price_per_m2 == 210_000, ( + f"Expected radius median 210_000, got {est.median_price_per_m2} -- " + "anchor with n=2 comps must NOT fire (min_comps=4 post-#755)" + ) + # Confidence from 3 radius analogs must not be "high" (n_analogs=3 < threshold). + assert ( + est.confidence != "high" + ), f"Confidence should not be 'high' with 3 radius analogs, got {est.confidence!r}" + + +def test_755_anchor_n2_pure_unit_confidence_never_high() -> None: + """#755 quality-gate (pure unit): _compute_same_building_anchor with n=2 comps, + min_comps=1 override, verifies confidence is capped below 'high' (n < 5). + + Even if a caller bypasses the min_comps threshold, the confidence for n<5 + must remain 'medium' or 'low' -- the cap introduced by #755. + Refs: PR #877 / commit d1034bb. + """ + from app.services.estimator import _compute_same_building_anchor + + comps = [ + {"price_per_m2": 399_478, "area_m2": 153.2, "rooms": 3}, + {"price_per_m2": 472_298, "area_m2": 110.1, "rooms": 3}, + ] + res = _compute_same_building_anchor( + comps, + area_target=110.0, + rooms_target=3, + tier="A", + sigma=0.18, + rooms_boost=1.6, + min_comps=1, # bypass threshold to test confidence cap independently + ) + assert res is not None + assert res["n"] == 2 + # n < 5 -> confidence cap: must NOT be 'high'. + assert res["confidence"] != "high", ( + f"confidence must not be 'high' for n=2 anchor comps (post-#755 cap), " + f"got {res['confidence']!r}" + ) + assert res["confidence"] in ("medium", "low") + + +# --------------------------------------------------------------------------- +# Test 2 -- dedup_hash stable across price change and ?context= query (#753) +# +# Before #753 the hash included price_rub, so a reprice of the same Avito +# listing produced a duplicate active listing. After #753 the hash is based +# on source_id (if present) or source_url stripped of query-string -- so +# same-listing reprices and re-scrapes with different ?context= tokens give +# the SAME hash. +# --------------------------------------------------------------------------- + + +def _make_scraped_lot( + *, + source: str = "avito", + source_url: str, + source_id: str | None = None, + price_rub: int = 5_000_000, +) -> Any: + from app.services.scrapers.base import ScrapedLot + + return ScrapedLot( + source=source, + source_url=source_url, + source_id=source_id, + address="EKB, ul. Tkachei, 13", + rooms=2, + area_m2=52.0, + price_rub=price_rub, + ) + + +def test_753_dedup_hash_stable_across_reprice_with_source_id() -> None: + """#753 quality-gate: same Avito item (same source_id) with different price_rub + produces identical compute_dedup_hash. + + Before #753, price_rub was part of the hash key, so a seller repricing + from 5M to 4.9M created a second active listing -- poisoning + asking_to_sold_ratio and inventory counts. + Refs: PR #761 / commit 79a7900. + """ + url = "https://www.avito.ru/ekaterinburg/kvartiry/2k_52m_13_1234567890" + sid = "1234567890" + + lot_original = _make_scraped_lot(source_url=url, source_id=sid, price_rub=5_000_000) + lot_repriced = _make_scraped_lot(source_url=url, source_id=sid, price_rub=4_900_000) + + assert ( + lot_original.compute_dedup_hash() == lot_repriced.compute_dedup_hash() + ), "dedup_hash must be stable across reprice when source_id is present (#753)" + + +def test_753_dedup_hash_stable_across_context_query_no_source_id() -> None: + """#753 quality-gate: same Avito URL with different ?context= tokens -> same hash. + + Avito appends a volatile ?context=... tracking token to every listing URL. + When source_id is absent, dedup_hash falls back to source_url stripped of + the query-string. Two re-scrapes with different tokens must yield the same + hash (no phantom duplicates). + Refs: PR #761 / commit 79a7900. + """ + base_url = "https://www.avito.ru/ekaterinburg/kvartiry/2k_52m_13_1234567890" + lot_v1 = _make_scraped_lot( + source_url=f"{base_url}?context=H4sIAAAAA", + source_id=None, + price_rub=5_000_000, + ) + lot_v2 = _make_scraped_lot( + source_url=f"{base_url}?context=X9zKBQAA", + source_id=None, + price_rub=5_000_000, + ) + assert ( + lot_v1.compute_dedup_hash() == lot_v2.compute_dedup_hash() + ), "dedup_hash must strip ?context= query and match for same listing URL (#753)" + + +def test_753_dedup_hash_differs_for_distinct_listings() -> None: + """#753 sanity: two genuinely different Avito listings produce different hashes. + + Guards against the hash function becoming trivially constant. + Refs: PR #761 / commit 79a7900. + """ + lot_a = _make_scraped_lot( + source_url="https://www.avito.ru/ekaterinburg/kvartiry/aaa", + source_id="111", + ) + lot_b = _make_scraped_lot( + source_url="https://www.avito.ru/ekaterinburg/kvartiry/bbb", + source_id="222", + ) + assert lot_a.compute_dedup_hash() != lot_b.compute_dedup_hash() + + +def test_753_dedup_hash_source_id_takes_priority_over_url() -> None: + """#753: when source_id is set, URL is ignored for the hash key. + + Two lots with the same source_id but different URLs produce the same + hash -- only source_id matters. + Refs: PR #761 / commit 79a7900. + """ + lot_canonical = _make_scraped_lot( + source_url="https://www.avito.ru/ekaterinburg/kvartiry/listing_9999", + source_id="9999", + ) + lot_redirect = _make_scraped_lot( + source_url="https://www.avito.ru/ru/offer/9999?some=other", + source_id="9999", + ) + assert ( + lot_canonical.compute_dedup_hash() == lot_redirect.compute_dedup_hash() + ), "source_id takes priority: same source_id -> same hash regardless of URL (#753)" + + +# --------------------------------------------------------------------------- +# Test 3 -- expected_sold > 0 on anchor-only path with no radius comps (#773) +# +# Before #773, the expected_sold guard was `and listings_clean` (truthy check +# on the radius listing list). When radius_analogs=[] the list is empty -> +# falsy -> expected_sold was skipped even when the anchor gave median_price>0. +# After #773 the guard is `and median_price > 0`, so anchor-only estimates +# with a valid ratio also populate expected_sold. +# --------------------------------------------------------------------------- + + +def test_773_expected_sold_positive_on_anchor_only_path() -> None: + """#773 quality-gate: anchor-only path (radius_analogs=[]) with ratio present + -> expected_sold_price_rub > 0 (not NULL). + + Before #773 fix, the guard was `and listings_clean` (empty list = falsy), + so expected_sold was skipped on the anchor-only path even though the anchor + produced a valid median_price. The fix changed the guard to `and median_price > 0`. + Refs: PR #784 / commit ec84637. + """ + est = _run_qa_estimate( + anchor_comps=_SB_COMPS_QG, + anchor_tier="A", + radius_analogs=[], # no radius comps -- anchor-only path + ratio_tuple=(0.92, "per_rooms"), + ) + # Anchor gave a valid headline. + assert est.median_price_rub > 0, "anchor must produce non-zero headline" + # expected_sold must also be computed (not skipped by empty listings_clean). + assert ( + est.expected_sold_price_rub is not None + ), "expected_sold_price_rub must not be None on anchor-only path when ratio present (#773)" + assert ( + est.expected_sold_price_rub > 0 + ), f"expected_sold_price_rub must be > 0, got {est.expected_sold_price_rub} (#773)" + # Consistency: sold = asking * ratio. + assert est.expected_sold_price_rub == round(est.median_price_rub * 0.92) + + +def test_773_expected_sold_null_when_no_ratio_anchor_only() -> None: + """#773 control: no ratio (None) -> expected_sold stays None even with anchor median>0. + + The fix must not fabricate an expected_sold when no ratio row exists. + Refs: PR #784 / commit ec84637. + """ + est = _run_qa_estimate( + anchor_comps=_SB_COMPS_QG, + anchor_tier="A", + radius_analogs=[], + ratio_tuple=(None, None), + ) + assert est.median_price_rub > 0 # anchor headline is valid + assert ( + est.expected_sold_price_rub is None + ), "expected_sold must be None when ratio is None, even on anchor-only path (#773)" + + +# --------------------------------------------------------------------------- +# Test 4 -- Avito block-page HTTP-200 behavior (#754) +# +# Before #754, an HTTP-200 response with 0 parsed cards was treated as a +# silent end-of-pagination. After #754: +# - page=1 with 0 cards -> raises AvitoContentBlockedError (no silent empty) +# - page>1 with 0 cards -> graceful break (end of pagination), no exception +# --------------------------------------------------------------------------- + +_FIXTURES = Path(__file__).parent / "fixtures" +_BLOCKPAGE_HTML = (_FIXTURES / "avito_blockpage_sample.html").read_text(encoding="utf-8") +_SERP_HTML = (_FIXTURES / "avito_serp_sample.html").read_text(encoding="utf-8") + + +@pytest.mark.asyncio +async def test_754_block_page_http200_page1_raises() -> None: + """#754 quality-gate: HTTP-200 with 0 parsed cards on page=1 -> AvitoContentBlockedError. + + Before #754 this scenario caused a silent empty return ([]) masking a content block. + Refs: PR #779 / commit 489666c. + """ + from app.services.scrapers.avito import AvitoScraper + from app.services.scrapers.avito_exceptions import AvitoContentBlockedError + + scraper = AvitoScraper() + with patch.object(scraper, "_fetch_serp_html", new=AsyncMock(return_value=_BLOCKPAGE_HTML)): + with pytest.raises(AvitoContentBlockedError): + await scraper.fetch_around(56.838, 60.605, radius_m=1000, pages=1) + + +@pytest.mark.asyncio +async def test_754_block_page_http200_page_gt1_graceful_return() -> None: + """#754 quality-gate: HTTP-200 with 0 parsed cards on page>1 -> graceful return. + + page=1 succeeds (real SERP HTML), page=2 returns block-page (0 cards -> + end-of-pagination break). The call must NOT raise. + Refs: PR #779 / commit 489666c. + """ + from app.services.scrapers.avito import AvitoScraper + + scraper = AvitoScraper() + + async def _serp_html_side_effect(url: str, page: int) -> str: + return _SERP_HTML if page == 1 else _BLOCKPAGE_HTML + + with patch.object( + scraper, "_fetch_serp_html", new=AsyncMock(side_effect=_serp_html_side_effect) + ): + with patch.object(scraper, "sleep_between_requests", new=AsyncMock(return_value=None)): + result = await scraper.fetch_around(56.838, 60.605, radius_m=1000, pages=2) + + # The important invariant: no exception raised, result is a list. + assert isinstance( + result, list + ), "fetch_around must return a list, not raise, when page>1 returns 0 cards (#754)" + + +# --------------------------------------------------------------------------- +# Test 5 -- median=0 -> insufficient_data=True (#740 / #697) +# +# When the estimate has no analogs (radius empty, no anchor, no IMV) the +# median is 0. AggregatedEstimate.insufficient_data must be True so the +# frontend shows the empty-state UI instead of "0 rub". +# --------------------------------------------------------------------------- + + +def test_740_median_zero_gives_insufficient_data_true() -> None: + """#740 quality-gate: estimate with no analogs -> insufficient_data=True, median=0. + + Before #740/#697, the flag did not exist; the frontend showed "0,00 mln rub". + The fix added AggregatedEstimate.insufficient_data as a computed property + that is True when median_price_rub <= 0. + Refs: PR #743 / commit 798fb12. + """ + from uuid import uuid4 + + from app.schemas.trade_in import AggregatedEstimate + + est = AggregatedEstimate( + estimate_id=uuid4(), + median_price_rub=0, + range_low_rub=0, + range_high_rub=0, + median_price_per_m2=0, + confidence="low", + n_analogs=0, + period_months=12, + analogs=[], + actual_deals=[], + expires_at=datetime(2026, 6, 1, tzinfo=UTC), + ) + assert ( + est.insufficient_data is True + ), "insufficient_data must be True when median_price_rub=0 (#740)" + assert est.median_price_rub == 0 + + +def test_740_positive_median_gives_insufficient_data_false() -> None: + """#740 control: estimate with analogs (median>0) -> insufficient_data=False. + + Guards against the flag becoming trivially True. + Refs: PR #743 / commit 798fb12. + """ + from uuid import uuid4 + + from app.schemas.trade_in import AggregatedEstimate + + est = AggregatedEstimate( + estimate_id=uuid4(), + median_price_rub=6_900_000, + range_low_rub=6_000_000, + range_high_rub=7_800_000, + median_price_per_m2=132_000, + confidence="medium", + n_analogs=8, + period_months=12, + analogs=[], + actual_deals=[], + expires_at=datetime(2026, 6, 1, tzinfo=UTC), + ) + assert ( + est.insufficient_data is False + ), "insufficient_data must be False when median_price_rub>0 (#740)" + + +def test_740_insufficient_data_serialized_in_model_dump() -> None: + """#740: insufficient_data appears in model_dump() output (API response shape). + + The frontend reads this field via JSON; it must survive Pydantic serialization. + Refs: PR #743 / commit 798fb12. + """ + from uuid import uuid4 + + from app.schemas.trade_in import AggregatedEstimate + + zero_est = AggregatedEstimate( + estimate_id=uuid4(), + median_price_rub=0, + range_low_rub=0, + range_high_rub=0, + median_price_per_m2=0, + confidence="low", + n_analogs=0, + period_months=12, + analogs=[], + actual_deals=[], + expires_at=datetime(2026, 6, 1, tzinfo=UTC), + ) + dump = zero_est.model_dump() + assert "insufficient_data" in dump, "insufficient_data must be present in model_dump()" + assert dump["insufficient_data"] is True diff --git a/tradein-mvp/backend/tests/test_estimator_pure_units.py b/tradein-mvp/backend/tests/test_estimator_pure_units.py index 3fbbce4e..f264305f 100644 --- a/tradein-mvp/backend/tests/test_estimator_pure_units.py +++ b/tradein-mvp/backend/tests/test_estimator_pure_units.py @@ -11,6 +11,7 @@ NOTE: importing app.services.estimator pulls app.core.config.Settings, which requires DATABASE_URL. Set it BEFORE importing app modules (same pattern as tests/test_scheduler.py). """ + import os os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") @@ -76,6 +77,15 @@ def _lot(ppm2: float | None) -> dict: def test_filter_outliers_passthrough_below_5_lots() -> None: + """Radius-path outlier filter: n<5 lots -> returned unchanged (bypass Tukey). + + This tests _filter_outliers (radius-path listings), NOT the anchor-path + MAD-clip introduced by #755. The n<5 bypass in _filter_outliers is + intentional and was NOT changed by #755 -- #755 added MAD-clip only to + _compute_same_building_anchor (anchor comps), leaving the radius-path + filter unchanged. Post-#755 contract: 4 radius listings still bypass + Tukey (IQR requires >= 5 lots with >= 4 priced). + """ lots = [_lot(100), _lot(200), _lot(300), _lot(400)] assert estimator._filter_outliers(lots) is lots # returned unchanged -- 2.45.3