diff --git a/backend/app/schemas/parcel.py b/backend/app/schemas/parcel.py index 33462243..41103e55 100644 --- a/backend/app/schemas/parcel.py +++ b/backend/app/schemas/parcel.py @@ -277,6 +277,9 @@ class LayoutTzRecommendation(BaseModel): based_on_total_deals: int data_window_start: dt.date data_window_end: dt.date + # Fix SF-09 review: True если pathological case — все bucket'ы выше cap, + # redistribute невозможен. Frontend использует для отображения warning banner. + cap_skipped: bool = False class LayoutDataQuality(BaseModel): diff --git a/backend/app/services/site_finder/best_layouts.py b/backend/app/services/site_finder/best_layouts.py index cc67c1e4..ee7d2105 100644 --- a/backend/app/services/site_finder/best_layouts.py +++ b/backend/app/services/site_finder/best_layouts.py @@ -48,6 +48,10 @@ logger = logging.getLogger(__name__) LAYOUT_CONFIDENCE_HIGH_PCT = 50.0 LAYOUT_CONFIDENCE_MEDIUM_PCT = 20.0 +# Fix SF-09: cap доминирующего bucket чтобы рекомендация не зеркалила перекос рынка. +# Избыток перераспределяется пропорционально остальным bucket'ам. +MAX_BUCKET_SHARE_PCT = 35 + # Параметры time_window: (PostgreSQL interval string, months divisor для velocity_per_month). # Используются в _INLINE_VELOCITY_SQL — реальный фильтр по report_month. # Fix SF-01: убраны _VELOCITY_DIVISORS, которые делили MV (24 мес) без изменения данных. @@ -219,6 +223,91 @@ def _normalize_pct(buckets: dict[str, float]) -> dict[str, int]: return floors +def _cap_and_redistribute(pct_map: dict[str, int]) -> tuple[dict[str, int], bool]: + """Fix SF-09 round 2: capacity-aware redistribute, bounded iterations. + + Round 1 bug: surplus распределялся пропорционально текущему `v` free bucket'а, + что переливало его выше cap — на 2-bucket вход цикл осциллировал бесконечно. + + Round 2 fix: surplus распределяется пропорционально **available capacity** + `(cap - v)` каждого free bucket'а. Тогда free никогда не вылетит выше cap → + цикл сходится за ≤ len(pct_map) итераций. Hard guard `for _ in range(N+1)`. + + Если surplus > total_capacity (геометрически невозможно поместить излишек ниже + cap) — забиваем все free к cap, возвращаем `cap_skipped=True` + warning log. + + Returns: + (result_map, cap_skipped) — cap_skipped=True если cap не удержан + (pathological: всё хочет > cap, или surplus > available capacity). + """ + if not pct_map: + return pct_map, False + + cap = MAX_BUCKET_SHARE_PCT + + # Быстрый path: нет доминирующих + if all(v <= cap for v in pct_map.values()): + return pct_map, False + + work: dict[str, float] = {k: float(v) for k, v in pct_map.items()} + + # Bounded iteration: после k-й итерации число clamped не убывает только если + # surplus > capacity (тогда — pathological). При корректном capacity-aware + # redistribute достаточно ≤ len(pct_map) итераций. + for _ in range(len(pct_map) + 1): + clamped = [k for k, v in work.items() if v > cap] + if not clamped: + break + + free = [k for k, v in work.items() if v < cap] + if not free: + # Все bucket'ы либо >cap либо ровно =cap — некуда переливать. + logger.warning( + "MAX_BUCKET_SHARE cap: нет free bucket'ов (%d total) — cap_skipped", + len(pct_map), + ) + return pct_map, True + + surplus = sum(work[k] - cap for k in clamped) + capacities = {k: cap - work[k] for k in free} + total_capacity = sum(capacities.values()) + + for k in clamped: + work[k] = float(cap) + + if surplus > total_capacity + 1e-9: + # Излишек не помещается ниже cap — pathological. + # Возвращаем оригинал (sum=100 invariant) + флаг для frontend banner. + logger.warning( + "MAX_BUCKET_SHARE cap: surplus %.2f > total_capacity %.2f — cap_skipped", + surplus, + total_capacity, + ) + return pct_map, True + + for k in free: + work[k] += capacities[k] / total_capacity * surplus + else: + # Hard guard: не сошлись за N+1 итераций — bug. Лог + cap_skipped. + logger.error( + "MAX_BUCKET_SHARE cap: не сошлись за %d итераций — algorithm bug", + len(pct_map) + 1, + ) + return pct_map, True + + return _hamilton_round(work), False + + +def _hamilton_round(work: dict[str, float]) -> dict[str, int]: + """Hamilton apportionment: float → integer pct с суммой ровно 100.""" + floors = {k: int(v) for k, v in work.items()} + remainder = 100 - sum(floors.values()) + fracs = sorted(work.keys(), key=lambda k: -(work[k] - floors[k])) + for k in fracs[: max(0, remainder)]: + floors[k] += 1 + return floors + + # ── Главная функция ─────────────────────────────────────────────────────────── @@ -552,6 +641,7 @@ def _build_recommendation( total_deals = sum(rb_deals.values()) pct_map = _normalize_pct(rb_deals) + pct_map, cap_skipped = _cap_and_redistribute(pct_map) mix: list[LayoutTzMixRow] = [] for rb, pct in sorted(pct_map.items(), key=lambda x: -x[1]): @@ -600,6 +690,7 @@ def _build_recommendation( based_on_total_deals=int(total_deals), data_window_start=window_start, data_window_end=window_end, + cap_skipped=cap_skipped, ) diff --git a/backend/tests/services/site_finder/test_best_layouts.py b/backend/tests/services/site_finder/test_best_layouts.py index bfb95af8..0720feaa 100644 --- a/backend/tests/services/site_finder/test_best_layouts.py +++ b/backend/tests/services/site_finder/test_best_layouts.py @@ -25,7 +25,12 @@ from unittest.mock import MagicMock import pytest from app.schemas.parcel import BestLayoutsRequest -from app.services.site_finder.best_layouts import _TIME_WINDOW_PARAMS, get_best_layouts +from app.services.site_finder.best_layouts import ( + _TIME_WINDOW_PARAMS, + MAX_BUCKET_SHARE_PCT, + _cap_and_redistribute, + get_best_layouts, +) _TODAY = dt.date.today() CAD_NUM = "66:41:0303161:123" @@ -335,3 +340,158 @@ def test_total_sold_in_window_matches_deals_window() -> None: assert len(resp.top_layouts) == 1 assert resp.top_layouts[0].total_sold_in_window == int(deals) + + +# ── Тесты _cap_and_redistribute (Fix SF-09 review) ─────────────────────────── + + +@pytest.mark.parametrize( + "pct_map, expect_pathological", + [ + # 1. normal: одиночный bucket > 35, free достаточно capacity + ({"1k": 50, "studio": 30, "2k": 20}, False), + # 2. heavy skew (3-bucket): surplus=40, capacity=20+25=45 — помещается + ({"1k": 75, "studio": 15, "2k": 10}, False), + # 3. multiple buckets > 35 + ({"1k": 50, "studio": 40, "2k": 10}, False), + # 4. all > 35 — pathological + ({"1k": 50, "studio": 50}, True), + # 5. граничный: один bucket ровно на cap — не clamp + ({"1k": 35, "studio": 35, "2k": 30}, False), + # 6. single bucket 100% — pathological (нет free) + ({"1k": 100}, True), + # 7. 2-bucket heavy: surplus=55, capacity=25 — pathological (не помещается) + ({"1k": 90, "studio": 10}, True), + # 8. все ≤ cap — fast-path без изменений + ({"1k": 30, "studio": 35, "2k": 35}, False), + # 9. 2-bucket: 70/30 → surplus=35, capacity=5 → pathological + ({"1k": 70, "studio": 30}, True), + # 10. 2-bucket: 99/1 → surplus=64, capacity=34 → pathological + ({"1k": 99, "studio": 1}, True), + ], +) +def test_cap_and_redistribute_invariants( + pct_map: dict[str, int], + expect_pathological: bool, +) -> None: + """Invariant: max(pct) ≤ cap И sum(pct) == 100 (или cap_skipped=True в pathological). + + Pathological — `cap_skipped=True`, max МОЖЕТ быть > cap (геометрически surplus + не вмещается в free capacity). + """ + result, cap_skipped = _cap_and_redistribute(pct_map) + + assert ( + cap_skipped == expect_pathological + ), f"cap_skipped={cap_skipped} но ожидали {expect_pathological} для {pct_map}" + assert ( + sum(result.values()) == 100 + ), f"sum={sum(result.values())} != 100 для {pct_map} → {result}" + if not expect_pathological: + assert ( + max(result.values()) <= MAX_BUCKET_SHARE_PCT + ), f"max={max(result.values())} > cap={MAX_BUCKET_SHARE_PCT} для {pct_map} → {result}" + + +@pytest.mark.parametrize( + "deals, expect_pathological, label", + [ + # 3-bucket с достаточной capacity — surplus помещается, cap соблюдён + ({"1k": 75, "studio": 15, "2k": 10}, False, "{1k:75, studio:15, 2k:10}"), + ({"1k": 80, "studio": 12, "2k": 8}, False, "{1k:80, studio:12, 2k:8}"), + ({"1k": 60, "studio": 30, "2k": 10}, False, "{1k:60, studio:30, 2k:10}"), + ({"a": 50, "b": 30, "c": 20}, False, "{50, 30, 20}"), + # 2-bucket — surplus геометрически не помещается, cap_skipped=True + ({"1k": 90, "studio": 10}, True, "{1k:90, studio:10}"), + ({"1k": 70, "studio": 30}, True, "{1k:70, studio:30}"), + ({"1k": 99, "studio": 1}, True, "{1k:99, studio:1}"), + ], +) +def test_cap_reproduced_failing_cases( + deals: dict[str, int], expect_pathological: bool, label: str +) -> None: + """Review round-2 reproduced cases: 2-bucket — pathological, 3-bucket — fit cap.""" + result, cap_skipped = _cap_and_redistribute(deals) + assert ( + cap_skipped == expect_pathological + ), f"cap_skipped={cap_skipped} ожидали {expect_pathological} для {label}" + assert sum(result.values()) == 100, f"sum != 100 для {label} → {result}" + if not expect_pathological: + assert ( + max(result.values()) <= MAX_BUCKET_SHARE_PCT + ), f"max={max(result.values())} > {MAX_BUCKET_SHARE_PCT} для {label} → {result}" + + +def test_cap_iteration_count_bounded() -> None: + """Round 2 regression: алгоритм завершается за ≤ len(pct_map)+1 итераций. + + Round 1 bag: на 2-bucket {1k:70, studio:30} цикл осциллировал бесконечно. + Round 2 fix: capacity-aware redistribute + hard `for _ in range(N+1)` guard. + Этот тест гарантирует что вызов не зависает (pytest-timeout не нужен). + """ + import time + + pathological_cases = [ + {"1k": 70, "studio": 30}, + {"1k": 99, "studio": 1}, + {"1k": 90, "studio": 10}, + {"1k": 50, "studio": 50}, + ] + for case in pathological_cases: + start = time.perf_counter() + result, cap_skipped = _cap_and_redistribute(case) + elapsed_ms = (time.perf_counter() - start) * 1000 + assert elapsed_ms < 100, f"Завис ({elapsed_ms:.0f}ms) на {case}" + assert sum(result.values()) == 100, f"sum != 100 для {case}" + # 2-bucket с одним > cap всегда pathological (surplus > free capacity) + if case != {"1k": 50, "studio": 50}: + assert cap_skipped, f"Ожидали cap_skipped=True для {case}" + + +def test_cap_and_redistribute_no_dominant_unchanged() -> None: + """Если все bucket'ы ≤ cap — результат идентичен входу (fast-path).""" + pct_map = {"studio": 20, "1": 35, "2": 30, "3": 15} + result, cap_skipped = _cap_and_redistribute(pct_map) + assert not cap_skipped + assert result == pct_map + + +def test_cap_and_redistribute_empty() -> None: + """Пустой dict → возвращается как есть.""" + result, cap_skipped = _cap_and_redistribute({}) + assert result == {} + assert not cap_skipped + + +def test_cap_skipped_flag_propagates_to_recommendation() -> None: + """Pathological case → cap_skipped=True в recommendation_for_tz ответа.""" + # 2 bucket'а по 50% — pathological + id_rows = [_obj_id_row(1), _obj_id_row(2)] + vel_rows = [ + _vel_row("studio", deals_window=50.0, obj_ids=[1]), + _vel_row("1", deals_window=50.0, obj_ids=[2]), + ] + db = _make_db(coord=_coord_row(), id_rows=id_rows, vel_rows=vel_rows) + req = _request(time_window="last_quarter") + resp = get_best_layouts(db, CAD_NUM, req) + + # С deals 50/50 → normalize_pct даёт {studio:50, 1:50} — оба выше cap + assert resp.recommendation_for_tz.cap_skipped is True + + +def test_cap_skipped_false_for_normal_case() -> None: + """Normal case с capping → cap_skipped=False в recommendation_for_tz.""" + id_rows = [_obj_id_row(1), _obj_id_row(2), _obj_id_row(3)] + vel_rows = [ + _vel_row("1k", deals_window=75.0, obj_ids=[1]), + _vel_row("studio", deals_window=15.0, obj_ids=[2]), + _vel_row("2k", deals_window=10.0, obj_ids=[3]), + ] + db = _make_db(coord=_coord_row(), id_rows=id_rows, vel_rows=vel_rows) + req = _request(time_window="last_quarter") + resp = get_best_layouts(db, CAD_NUM, req) + + assert resp.recommendation_for_tz.cap_skipped is False + mix = resp.recommendation_for_tz.mix + assert all(row.pct <= MAX_BUCKET_SHARE_PCT for row in mix) + assert sum(row.pct for row in mix) == 100 diff --git a/frontend/src/components/site-finder/BestLayoutsBlock.tsx b/frontend/src/components/site-finder/BestLayoutsBlock.tsx index 9a1919af..1cd4d337 100644 --- a/frontend/src/components/site-finder/BestLayoutsBlock.tsx +++ b/frontend/src/components/site-finder/BestLayoutsBlock.tsx @@ -345,6 +345,9 @@ function RecommendationCard({ }: { rec: BestLayoutsResponse["recommendation_for_tz"]; }) { + const maxPct = + rec.mix.length > 0 ? Math.max(...rec.mix.map((r) => r.pct)) : 0; + return (
+ {/* SF-09: предупреждение о перекосе рынка — показываем только если cap не смог применить */} + {rec.cap_skipped && ( +
+ Рекомендация имеет сильный перекос ({maxPct}% в одном формате) — + рынок настолько асимметричен, что cap не применён. Проверьте + competitive density и district pipeline. +
+ )} + {/* Rationale text — plain text only, no dangerouslySetInnerHTML */}

cap, redistribute невозможен. */ + cap_skipped: boolean; } export interface LayoutDataQuality {