fix(sf-09): MAX_BUCKET_SHARE 35% cap + frontend warning banner #282
5 changed files with 269 additions and 1 deletions
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<div
|
||||
style={{
|
||||
|
|
@ -365,6 +368,15 @@ function RecommendationCard({
|
|||
gap: 16,
|
||||
}}
|
||||
>
|
||||
{/* SF-09: предупреждение о перекосе рынка — показываем только если cap не смог применить */}
|
||||
{rec.cap_skipped && (
|
||||
<div className="bg-amber-50 border border-amber-200 text-amber-800 rounded p-3 text-sm">
|
||||
Рекомендация имеет сильный перекос ({maxPct}% в одном формате) —
|
||||
рынок настолько асимметричен, что cap не применён. Проверьте
|
||||
competitive density и district pipeline.
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Rationale text — plain text only, no dangerouslySetInnerHTML */}
|
||||
<p
|
||||
style={{ fontSize: 13, color: "#374151", margin: 0, lineHeight: 1.6 }}
|
||||
|
|
|
|||
|
|
@ -48,6 +48,8 @@ export interface LayoutTzRecommendation {
|
|||
based_on_total_deals: number;
|
||||
data_window_start: string;
|
||||
data_window_end: string;
|
||||
/** Fix SF-09: true если pathological case — все bucket'ы > cap, redistribute невозможен. */
|
||||
cap_skipped: boolean;
|
||||
}
|
||||
|
||||
export interface LayoutDataQuality {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue