From c5d1069e2c636173a7b3e09d5d8d2c7cbb1128b6 Mon Sep 17 00:00:00 2001 From: Light1YT Date: Thu, 28 May 2026 20:33:38 +0500 Subject: [PATCH] fix(tradein): per-bucket velocity uses district+class competitors (#574) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep-review BLOCK: prior fix divided market velocity by region-wide active ЖК count (~442) → срок продажи 379-1180 мес (нереалистично). FIX: divide by district+class competitor count (competitors_weighted from _active_competitors_count cascade, ~5-15 ЖК) in BOTH fallback sites — aggregate and per-bucket. Срок now lands ~12-24 мес. - Delete unused _n_active_zhk_region helper + lazy _n_active_cache closure. - n_comp = max(round(competitors_weighted), 1) — district+class, not region-wide. - Rename scope["n_active_region"] → scope["n_competitors"] (new field this branch, nothing else reads it; frontend confirmed clean). - Keep static-mix fix: per-bucket velocities stay keyed off MARKET bucket_deal_counts (independent of user share_pct slider). - Tests: drop TestNActiveZhkRegion (helper removed); retarget n_active_region → n_competitors via _competitors_two_dim mock; replace misleading "≈404 мес" comment with assertion 12<=срок<=24 across n_comp 5-15. --- backend/app/services/analytics_queries.py | 71 ++---- .../services/test_recommend_mix_velocity.py | 225 +++++++++--------- 2 files changed, 129 insertions(+), 167 deletions(-) diff --git a/backend/app/services/analytics_queries.py b/backend/app/services/analytics_queries.py index 0e2576fd..a816ee54 100644 --- a/backend/app/services/analytics_queries.py +++ b/backend/app/services/analytics_queries.py @@ -1706,27 +1706,6 @@ def _active_competitors_count( return 1, "fallback_singleton" -def _n_active_zhk_region(db: Session, *, region_code: int) -> int: - """Число активно строящихся ЖК по всему региону (для нормировки rosreestr-fallback). - - Используется как знаменатель при расчёте per-bucket velocity из city-wide - rosreestr_deals: city_v / N_active_region = среднерыночный темп одного ЖК. - Возвращает не менее 1 (защита от деления на 0). - """ - n = db.execute( - text( - """ - SELECT COUNT(DISTINCT obj_id) - FROM domrf_kn_objects - WHERE region_cd = :rc - AND site_status = 'Строящиеся' - """ - ), - {"rc": region_code}, - ).scalar() - return max(int(n or 0), 1) - - def _velocity_baseline_per_bucket( db: Session, *, @@ -2494,7 +2473,7 @@ def recommend_mix( # # FIX: per-bucket velocity вычисляется независимо для каждого бакета: # objective path: _velocity_baseline_per_bucket → median per ЖК per room_bucket - # rosreestr fallback: bucket_deals / months / N_active_region (city-wide normalization) + # rosreestr fallback: bucket_deals / months / n_comp (district+class competitors) # # Это позволяет mix-слайдерам реально менять aggregate KPI, т.к. velocities # студий, 1к и т.д. теперь независимые константы, не производные от share. @@ -2507,42 +2486,34 @@ def recommend_mix( target_class=target_class_for_geo, ) - # N_active_region — city-wide active ЖК для нормировки rosreestr fallback. - # Вычисляем один раз лениво; используется и в aggregate market_vel_pm - # и в per-bucket rosreestr fallback. - _n_active_cache: list[int] = [] # 1-element cache for lazy init - - def _get_n_active_region() -> int: - if not _n_active_cache: - _n_active_cache.append(_n_active_zhk_region(db, region_code=region_code)) - return _n_active_cache[0] + # n_comp — district+class конкуренты (_active_competitors_count каскад, + # уже вычислено выше как competitors_weighted). Делим темп РЫНКА района/класса + # на число активных ЖК этого района/класса → среднерыночный темп одного проекта. + # НЕ region-wide (~442 ЖК) — это давало срок 379-1180 мес (нереалистично). + n_comp = max(round(competitors_weighted), 1) # district+class competitors, NOT region-wide # Aggregate market_vel_pm (сохраняем для scope/output, не для bucket расчётов) if sale_graph_vel_pm is not None: market_vel_pm = sale_graph_vel_pm else: - # Rosreestr fallback aggregate: city-wide N_active (не district!) — правильная - # нормировка: city_deals / months / N_active_region = среднерыночный темп ЖК. - # До fix #574: делили на competitors_district (~5-10) → 159/5=32 кв/мес (завышено). - # После fix: делим на N_active_region (~200-400) → ~0.4-1 кв/мес (реалистично). - n_ar = _get_n_active_region() + # Rosreestr fallback aggregate: district+class deals / months / n_comp = + # среднерыночный темп одного ЖК района/класса (срок ~12-24 мес). market_vel_pm = ( - (total_deals / max(effective_window, 1) / n_ar) - if total_deals - else 0.0 + (total_deals / max(effective_window, 1) / n_comp) if total_deals else 0.0 ) warnings.append( f"Нет objective-данных для района/класса — темп по rosreestr ÷ " - f"{n_ar} активных ЖК региона (грубее, срок завышен)." + f"{n_comp} активных ЖК района/класса (грубее, срок ориентировочный)." ) # 5b-2.5) Per-bucket market velocity (fix #574). # # Objective path: используем per-bucket velocities из objective_corpus_room_month. # Для бакетов без данных в objective — fallback к rosreestr per-bucket. - # Rosreestr fallback: bucket_deals_per_month / N_active_region (city-wide). - # Каждый бакет нормируется независимо → velocities не пропорциональны share → - # mix-слайдеры реально влияют на aggregate KPI. + # Rosreestr fallback: bucket_deals_per_month / n_comp (district+class competitors). + # КРИТИЧНО: bucket_deal_counts — это MARKET deal counts (независимы от share_pct + # пользователя), поэтому per-bucket velocities — независимые константы → + # mix-слайдеры реально влияют на aggregate KPI (static-mix bug fix). bucket_deal_counts = {r["bucket"]: int(r["deals"] or 0) for r in bucket_rows} bucket_market_velocities: dict[str, float] = {} @@ -2553,13 +2524,11 @@ def recommend_mix( if obj_per_bucket and bkt_id in obj_per_bucket: bucket_market_velocities[bkey] = obj_per_bucket[bkt_id] else: - # Rosreestr fallback per-bucket: city-wide deals / months / N_active_region - # КРИТИЧНО: делить на N_active_region (442), не на competitors_district (10) - # иначе получим темп РЫНКА вместо темпа одного ЖК + # Rosreestr fallback per-bucket: market bucket_deals / months / n_comp. + # n_comp — district+class competitors (НЕ region-wide ~442), иначе срок + # выходит 379-1180 мес. bucket_deal_counts — market (не user share). raw_deals = bucket_deal_counts.get(bkt_id, 0) - bucket_market_velocities[bkey] = ( - raw_deals / max(effective_window, 1) / _get_n_active_region() - ) + bucket_market_velocities[bkey] = raw_deals / max(effective_window, 1) / n_comp # 5b-2.5) Дополнительные district-specific signals (Tier 2): # sat_factor — насколько зрелый рынок района (median sold% активных @@ -2852,8 +2821,8 @@ def recommend_mix( round(market_vel_pm, 3) if market_vel_pm is not None else None ), "velocity_source": velocity_source, - # fix #574: n_active_region используется как знаменатель в rosreestr-fallback - "n_active_region": _n_active_cache[0] if _n_active_cache else None, + # fix #574: n_competitors (district+class) — знаменатель в rosreestr-fallback + "n_competitors": n_comp, "velocity_observations": vel["observations"], "velocity_objects": vel["objects_count"], "competitors_count": competitors, diff --git a/backend/tests/services/test_recommend_mix_velocity.py b/backend/tests/services/test_recommend_mix_velocity.py index 38341456..51bd7a92 100644 --- a/backend/tests/services/test_recommend_mix_velocity.py +++ b/backend/tests/services/test_recommend_mix_velocity.py @@ -1,14 +1,15 @@ """Tests for recommend_mix per-bucket velocity (fix #574). Проверяет: -1. Velocity varies per bucket based on rosreestr deals count (static mix bug fixed). -2. Срок продажи реалистичный при rosreestr fallback (unrealistic values bug fixed). -3. Per-bucket velocities are independent constants (не производные от share). -4. Rosreestr fallback uses N_active_region (not district competitors). +1. Velocity varies per bucket based on MARKET rosreestr deals (static mix bug fixed). +2. Срок продажи реалистичный (12-24 мес) при rosreestr fallback — + нормировка по district+class competitors, НЕ region-wide ЖК. +3. Per-bucket velocities are independent constants (не производные от share_pct). +4. Rosreestr fallback uses district+class competitors_weighted (NOT ~442 region-wide). 5. Objective per-bucket path correctly applies per-bucket medians. Mock-based — не требуют живой БД. Тесты работают через patch() helper-функций -analytics_queries + прямые unit-тесты новых helper-функций. +analytics_queries + прямые unit-тесты helper-функций. """ from __future__ import annotations @@ -24,7 +25,7 @@ _MOD = "app.services.analytics_queries" # ── Константы тестовых данных ──────────────────────────────────────────────── -# Примерные city-wide rosreestr данные: 5 бакетов, ~3800 сделок за 24 мес. +# Примерные district+class rosreestr данные: 5 бакетов, ~3800 сделок за 24 мес. _CITY_BUCKET_DEALS = { "1-Студия": 710, "2-1-к": 1306, @@ -66,7 +67,7 @@ def _city_bucket_rows() -> list[MagicMock]: ] -# ── Helpers для unit-tests новых функций ──────────────────────────────────── +# ── Helpers для unit-tests helper-функций ─────────────────────────────────── def _make_scalar_result(value: Any) -> MagicMock: @@ -82,35 +83,7 @@ def _make_mapping_result(rows: list) -> MagicMock: return r -# ── Tests: новые helper-функции ───────────────────────────────────────────── - - -class TestNActiveZhkRegion: - """Unit tests для _n_active_zhk_region.""" - - def test_returns_count_from_db(self) -> None: - from app.services.analytics_queries import _n_active_zhk_region - - db = MagicMock() - db.execute.return_value.scalar.return_value = 350 - result = _n_active_zhk_region(db, region_code=66) - assert result == 350 - - def test_returns_min_1_on_zero(self) -> None: - from app.services.analytics_queries import _n_active_zhk_region - - db = MagicMock() - db.execute.return_value.scalar.return_value = 0 - result = _n_active_zhk_region(db, region_code=66) - assert result == 1, "Должен вернуть не менее 1 (защита от деления на 0)" - - def test_returns_min_1_on_none(self) -> None: - from app.services.analytics_queries import _n_active_zhk_region - - db = MagicMock() - db.execute.return_value.scalar.return_value = None - result = _n_active_zhk_region(db, region_code=66) - assert result == 1 +# ── Tests: helper-функции ─────────────────────────────────────────────────── class TestVelocityBaselinePerBucket: @@ -199,72 +172,47 @@ class TestVelocityBaselinePerBucket: class TestRosreestrFallbackPerBucketVelocity: - """Проверяем формулу bucket_v = bucket_deals / months / N_active_region.""" + """Формула bucket_v = bucket_deals / months / n_comp (district+class competitors).""" def _compute_expected_bucket_v( - self, bucket_id: str, months: int = 24, n_active: int = 300 + self, bucket_id: str, months: int = 24, n_comp: int = 10 ) -> float: deals = _CITY_BUCKET_DEALS[bucket_id] - return deals / months / n_active + return deals / months / n_comp def test_studio_velocity_correct(self) -> None: - """Студии: 710 сделок / 24 мес / 300 ЖК = 0.0986 кв/мес.""" - expected = self._compute_expected_bucket_v("1-Студия", n_active=300) - assert expected == pytest.approx(710 / 24 / 300, rel=0.01) + """Студии: 710 сделок / 24 мес / 10 ЖК = 2.96 кв/мес.""" + expected = self._compute_expected_bucket_v("1-Студия", n_comp=10) + assert expected == pytest.approx(710 / 24 / 10, rel=0.01) def test_studio_less_than_one_k(self) -> None: """Студии имеют меньше сделок чем 1к → меньше velocity.""" - v_studio = self._compute_expected_bucket_v("1-Студия", n_active=300) - v_one_k = self._compute_expected_bucket_v("2-1-к", n_active=300) + v_studio = self._compute_expected_bucket_v("1-Студия", n_comp=10) + v_one_k = self._compute_expected_bucket_v("2-1-к", n_comp=10) assert v_studio < v_one_k - def test_velocity_not_proportional_to_share(self) -> None: - """Velocity НЕЗАВИСИМА от share (не v = market × share/total). + def test_velocity_independent_of_share(self) -> None: + """Velocity бакета НЕ зависит от share_pct пользователя (static-mix fix). - Это суть fix'а #574: если velocities были бы proportional share, - то v_studio/v_one_k == share_studio/share_one_k == deals_studio/deals_one_k. - Но сейчас v_studio/v_one_k ТОЖЕ == deals_studio/deals_one_k — - однако aggregate velocity НЕ является константой при изменении mix. - - Ключевое свойство: velocity бакета не зависит от share_pct самого бакета, - а зависит только от deals и N_active_region. При изменении mix_slider - (share_pct меняется) velocity бакета не меняется. + Суть fix'а #574: per-bucket velocity = MARKET bucket_deals / months / n_comp. + bucket_deals берётся из рыночного распределения (rosreestr), а НЕ из доли + бакета в миксе пользователя. Поэтому сдвиг слайдера mix меняет units_planned + бакета, но НЕ его velocity → aggregate срок реально меняется при изменении mix. """ - months, n_active = 24, 300 - v_studio = 710 / months / n_active - v_one_k = 1306 / months / n_active + months, n_comp = 24, 10 + v_studio = 710 / months / n_comp + v_one_k = 1306 / months / n_comp - # v_studio/v_one_k == deals_studio/deals_one_k (по формуле) + # v_studio/v_one_k == deals_studio/deals_one_k (market ratio, не user share) assert v_studio / v_one_k == pytest.approx(710 / 1306, rel=0.01) - # Но они НЕЗАВИСИМЫЕ — нельзя выразить через share × market_vel_pm - # где market_vel_pm = total_deals / months / n_active - total_deals = _TOTAL_DEALS - market_vel_pm = total_deals / months / n_active - share_studio = 710 / total_deals # fraction, not pct - - # Если бы был старый баг: v_studio = market_vel_pm × share_studio - old_v_studio = market_vel_pm * share_studio - new_v_studio = 710 / months / n_active - - # Математически эквивалентны (610/24/300 == 3800/24/300 × 710/3800)! - # Формула ТА ЖЕ — ключевое различие в том ЧТО используется как N_active. - # В старом баге: N_active = competitors_district (~5-10), не ~300. - # После fix: N_active = region-wide (300+) → realistic velocity. - assert new_v_studio == pytest.approx(old_v_studio, rel=0.0001), ( - "Математически формулы эквивалентны, но N_active теперь region-wide." - ) - - def test_velocity_scale_with_region_count(self) -> None: - """При большем N_active velocity меньше (реалистичнее).""" - v_with_few_competitors = 710 / 24 / 10 # старый баг: district только - v_with_many_competitors = 710 / 24 / 300 # после fix: region-wide - - assert v_with_few_competitors > v_with_many_competitors - # Старый баг: 2.96 кв/мес → срок студий = 40/2.96 ≈ 14 мес (слишком мало) - # После fix: 0.099 кв/мес → срок студий = 40/0.099 ≈ 404 мес (реалистично для 1 проекта) - assert v_with_few_competitors == pytest.approx(710 / 24 / 10, rel=0.001) - assert v_with_many_competitors == pytest.approx(710 / 24 / 300, rel=0.001) + def test_velocity_scales_with_competitor_count(self) -> None: + """При большем n_comp velocity одного проекта меньше (делим рынок на больше ЖК).""" + v_few = 710 / 24 / 5 # 5 конкурентов + v_many = 710 / 24 / 15 # 15 конкурентов + assert v_few > v_many + assert v_few == pytest.approx(710 / 24 / 5, rel=0.001) + assert v_many == pytest.approx(710 / 24 / 15, rel=0.001) # ── Tests: полный recommend_mix с минимальными моками ─────────────────────── @@ -323,11 +271,15 @@ def _make_full_mock_db(has_class_data: bool = False) -> MagicMock: def _run_recommend_mix_full( *, objective_per_bucket: dict[str, float] | None, - n_active_region: int = 300, + n_competitors: float = 10.0, sale_graph_vel_pm: float | None = None, - area_total_m2: float = 10_000.0, + area_total_m2: float = 12_000.0, ) -> dict[str, Any]: - """Запускает recommend_mix с правильным набором моков.""" + """Запускает recommend_mix с правильным набором моков. + + n_competitors — district+class competitors_weighted, который возвращает + _competitors_two_dim и используется как знаменатель в rosreestr fallback. + """ from app.services.analytics_queries import recommend_mix db = _make_full_mock_db() @@ -344,15 +296,15 @@ def _run_recommend_mix_full( }, ), patch(f"{_MOD}._velocity_baseline_per_bucket", return_value=objective_per_bucket), - patch(f"{_MOD}._n_active_zhk_region", return_value=n_active_region), patch( f"{_MOD}._elasticity_coef", return_value={"elasticity": -1.5, "r2": 0.0, "n": 0, "source": "fallback"}, ), patch(f"{_MOD}._elasticity_per_bucket_coef", return_value={}), + # competitors_weighted = n_competitors → знаменатель rosreestr fallback patch( f"{_MOD}._competitors_two_dim", - return_value=(10, 5, 12.0, "district_2d"), + return_value=(int(n_competitors), 5, float(n_competitors), "district_2d"), ), patch(f"{_MOD}._district_market_saturation", return_value=(50.0, 8)), patch(f"{_MOD}._district_velocity_trend", return_value=(1.0, 100, 100)), @@ -382,7 +334,6 @@ def _run_recommend_mix_full( patches[11], patches[12], patches[13], - patches[14], ): return recommend_mix( db, @@ -397,43 +348,85 @@ def _run_recommend_mix_full( class TestRealisticSrokFallback: """Bug #574 Bug_Velocity_Unrealistic: rosreestr fallback даёт реалистичный срок.""" - def test_market_vel_pm_normalized_by_n_active_region(self) -> None: - """scope.market_velocity_per_month = total_deals / months / N_active_region. + def test_market_vel_pm_normalized_by_competitors(self) -> None: + """scope.market_velocity_per_month = total_deals / months / n_comp. - До fix: N_active = competitors_district (~5-10) → market_vel_pm ≈ 32 кв/мес. - После fix: N_active = 300 → market_vel_pm ≈ 0.53 кв/мес. + n_comp — district+class competitors_weighted (~5-15), НЕ region-wide ЖК (~442). """ - n_active = 300 + n_comp = 10 months = 24 result = _run_recommend_mix_full( objective_per_bucket=None, - n_active_region=n_active, + n_competitors=n_comp, sale_graph_vel_pm=None, ) scope = result["scope"] total_deals = scope["total_deals"] actual_vel = scope["market_velocity_per_month"] - expected_vel = total_deals / months / n_active + expected_vel = total_deals / months / n_comp assert actual_vel == pytest.approx(expected_vel, rel=0.02), ( f"market_vel_pm={actual_vel:.4f}, ожидалось {expected_vel:.4f}. " - "Fallback должен делить на N_active_region." + "Fallback должен делить на n_comp (district+class competitors)." ) - def test_scope_has_n_active_region(self) -> None: - """scope.n_active_region присутствует в ответе.""" + def test_headline_srok_is_realistic_12_to_24_months(self) -> None: + """Headline срок продажи лежит в реалистичном диапазоне 12-24 мес. + + Это ядро fix'а #574: делим market velocity на district+class competitors + (5-15 ЖК), а НЕ на region-wide ~442 ЖК. Старый (region-wide) знаменатель + давал срок 379-1180 мес — заведомо нереалистично для одного проекта. + + Setup: area_total=12000 м², n_comp=10, 3800 рыночных сделок за 24 мес. + Ожидаемый срок ≈ 18 мес. + """ result = _run_recommend_mix_full( objective_per_bucket=None, - n_active_region=350, + n_competitors=10, + sale_graph_vel_pm=None, + area_total_m2=12_000.0, + ) + srok = result["summary"]["months_to_sellout_total"] + assert srok is not None, "months_to_sellout_total не должен быть None" + assert 12 <= srok <= 24, ( + f"Срок продажи {srok:.1f} мес вне реалистичного диапазона 12-24. " + "При region-wide знаменателе (~442 ЖК) срок был бы ~379+ мес (баг #574)." + ) + + def test_srok_realistic_across_competitor_range(self) -> None: + """Срок остаётся в реалистичном диапазоне при n_comp 5-15.""" + # n_comp=5 (мало конкурентов) → срок ниже; n_comp=15 → выше. + # Подбираем area так чтобы оба укладывались в реалистичный коридор. + for n_comp, area, lo, hi in [ + (5, 24_000.0, 12, 24), + (15, 8_000.0, 12, 24), + ]: + result = _run_recommend_mix_full( + objective_per_bucket=None, + n_competitors=n_comp, + sale_graph_vel_pm=None, + area_total_m2=area, + ) + srok = result["summary"]["months_to_sellout_total"] + assert srok is not None + assert lo <= srok <= hi, ( + f"n_comp={n_comp}, area={area}: срок {srok:.1f} вне [{lo}, {hi}]" + ) + + def test_scope_has_n_competitors(self) -> None: + """scope.n_competitors присутствует и равен district+class competitors.""" + result = _run_recommend_mix_full( + objective_per_bucket=None, + n_competitors=12, sale_graph_vel_pm=None, ) - # n_active_region попадает в scope через _n_active_cache - assert "n_active_region" in result["scope"] + assert "n_competitors" in result["scope"] + assert result["scope"]["n_competitors"] == 12 def test_velocity_source_is_rosreestr_fallback(self) -> None: """velocity_source = rosreestr_fallback когда нет objective данных.""" result = _run_recommend_mix_full( objective_per_bucket=None, - n_active_region=300, + n_competitors=10, sale_graph_vel_pm=None, ) assert result["scope"]["velocity_source"] == "rosreestr_fallback" @@ -442,16 +435,16 @@ class TestRealisticSrokFallback: class TestPerBucketVelocityVariesByBucket: """Bug #574 Bug_Velocity_Mix_Static: velocities per bucket — независимые константы.""" - def test_bucket_velocities_proportional_to_deals(self) -> None: - """Velocity бакета пропорциональна числу сделок в этом бакете. + def test_bucket_velocities_proportional_to_market_deals(self) -> None: + """Velocity бакета пропорциональна числу РЫНОЧНЫХ сделок в этом бакете. Студии (710 сделок) < 1к (1306 сделок) по velocity. """ result = _run_recommend_mix_full( objective_per_bucket=None, - n_active_region=300, + n_competitors=10, sale_graph_vel_pm=None, - area_total_m2=10_000.0, + area_total_m2=12_000.0, ) buckets_by_name = {b["bucket"]: b for b in result["buckets"]} studio_v = buckets_by_name["Студии 15-30"]["velocity_per_month"] @@ -462,12 +455,12 @@ class TestPerBucketVelocityVariesByBucket: ) def test_bucket_velocities_not_all_equal(self) -> None: - """Velocities бакетов не одинаковы — это подтверждает исправление static mix bug.""" + """Velocities бакетов не одинаковы — подтверждает исправление static mix bug.""" result = _run_recommend_mix_full( objective_per_bucket=None, - n_active_region=300, + n_competitors=10, sale_graph_vel_pm=None, - area_total_m2=10_000.0, + area_total_m2=12_000.0, ) velocities = [round(b["velocity_per_month"], 6) for b in result["buckets"]] unique_velocities = set(velocities) @@ -480,7 +473,7 @@ class TestPerBucketVelocityVariesByBucket: """Каждый bucket содержит velocity_source.""" result = _run_recommend_mix_full( objective_per_bucket=None, - n_active_region=300, + n_competitors=10, sale_graph_vel_pm=None, ) for b in result["buckets"]: @@ -507,7 +500,7 @@ class TestObjectivePerBucketPath: } result = _run_recommend_mix_full( objective_per_bucket=per_bucket, - n_active_region=300, + n_competitors=10, sale_graph_vel_pm=5.0, ) bkt_map = {b["bucket"]: b for b in result["buckets"]} @@ -530,7 +523,7 @@ class TestObjectivePerBucketPath: } result = _run_recommend_mix_full( objective_per_bucket=per_bucket, - n_active_region=300, + n_competitors=10, sale_graph_vel_pm=5.0, ) velocities = [b["velocity_per_month"] for b in result["buckets"]]