chore(mv): regression test + sql rule for weighted AVG (#295) #626
4 changed files with 386 additions and 0 deletions
|
|
@ -61,6 +61,24 @@ Reference: vault `Pattern_CAST_AS_Type`.
|
|||
|
||||
Reference: `93_cad_parcels_geom_multipolygon.sql` (Polygon → MultiPolygon migration).
|
||||
|
||||
## Агрегация по pre-aggregated строкам (обязательно weighted AVG)
|
||||
|
||||
Если источник содержит строки вида «одна строка = один период (месяц) + уже посчитанный
|
||||
`avg_value` + `count`» (например `objective_corpus_room_month`), то наивный `AVG(avg_value)`
|
||||
**неверен**: строки с нулевыми сделками занижают результат в 2-10x.
|
||||
|
||||
Правильная формула — count-weighted AVG:
|
||||
```sql
|
||||
SUM(avg_value * cnt) / NULLIF(SUM(cnt), 0)
|
||||
```
|
||||
|
||||
- `NULLIF(..., 0)` обязателен — предотвращает `division by zero` при all-zero периодах
|
||||
и возвращает `NULL` вместо фейкового `0`.
|
||||
- Без весов: `AVG()` равноправно учитывает «пустые» месяцы → занижение.
|
||||
|
||||
Reference: fix #295 (`120_fix_mv_layout_velocity_weighted_avg.sql`),
|
||||
тест `backend/tests/sql/test_mv_layout_velocity_weighted_avg.py`.
|
||||
|
||||
## Запреты
|
||||
|
||||
- ❌ `DROP TABLE` / `TRUNCATE` без явного approval пользователя
|
||||
|
|
|
|||
0
backend/tests/sql/__init__.py
Normal file
0
backend/tests/sql/__init__.py
Normal file
276
backend/tests/sql/test_mv_layout_velocity_weighted_avg.py
Normal file
276
backend/tests/sql/test_mv_layout_velocity_weighted_avg.py
Normal file
|
|
@ -0,0 +1,276 @@
|
|||
"""
|
||||
Regression test for fix #295 — mv_layout_velocity weighted AVG.
|
||||
|
||||
Proves that the count-weighted formula
|
||||
SUM(val * cnt) / NULLIF(SUM(cnt), 0)
|
||||
produces the correct result and differs from naive AVG when zero-deal months are present.
|
||||
|
||||
Uses psycopg v3 (never psycopg2) with a temporary table containing known data.
|
||||
All bind parameters use CAST(:x AS type) — never :x::type.
|
||||
"""
|
||||
|
||||
from decimal import Decimal
|
||||
|
||||
import psycopg
|
||||
import pytest
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixture: connect to the test / local DB via env-var DATABASE_URL,
|
||||
# fall back to the tunnel URL used in local dev.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _get_dsn() -> str:
|
||||
import os
|
||||
return os.environ.get(
|
||||
"DATABASE_URL",
|
||||
"postgresql://gendesign@localhost:15432/gendesign",
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def conn():
|
||||
"""Open a psycopg v3 connection and roll back after the test."""
|
||||
with psycopg.connect(_get_dsn(), autocommit=False) as c:
|
||||
yield c
|
||||
c.rollback()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_CREATE_TEMP = """
|
||||
CREATE TEMP TABLE _test_ocm (
|
||||
room_bucket TEXT,
|
||||
deals_total_count INTEGER,
|
||||
deals_total_avg_area_m2 NUMERIC,
|
||||
deals_total_avg_price_thousand_rub_per_m2 NUMERIC
|
||||
) ON COMMIT DROP;
|
||||
"""
|
||||
|
||||
_INSERT_ROW = """
|
||||
INSERT INTO _test_ocm VALUES (
|
||||
CAST(%s AS text),
|
||||
CAST(%s AS integer),
|
||||
CAST(%s AS numeric),
|
||||
CAST(%s AS numeric)
|
||||
);
|
||||
"""
|
||||
|
||||
# Weighted aggregate query — mirrors the fixed mv_layout_velocity formula exactly.
|
||||
_WEIGHTED_QUERY = """
|
||||
SELECT
|
||||
room_bucket,
|
||||
SUM(deals_total_count) AS total_deals,
|
||||
AVG(deals_total_avg_area_m2)::numeric(10,2) AS naive_avg_area,
|
||||
(SUM(deals_total_avg_area_m2 * deals_total_count)
|
||||
/ NULLIF(SUM(deals_total_count), 0))::numeric(10,2) AS weighted_avg_area,
|
||||
AVG(deals_total_avg_price_thousand_rub_per_m2)::numeric(12,2) AS naive_avg_price,
|
||||
(SUM(deals_total_avg_price_thousand_rub_per_m2 * deals_total_count)
|
||||
/ NULLIF(SUM(deals_total_count), 0))::numeric(12,2) AS weighted_avg_price
|
||||
FROM _test_ocm
|
||||
GROUP BY room_bucket
|
||||
ORDER BY room_bucket;
|
||||
"""
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Test cases
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestWeightedAvgFormula:
|
||||
"""Unit-level verification of the count-weighted formula using a temp table."""
|
||||
|
||||
def test_zero_deal_months_skew_naive_avg(self, conn: psycopg.Connection) -> None:
|
||||
"""
|
||||
Scenario: studio, 3 months in window.
|
||||
Month 1: 10 deals, avg_area=25, avg_price=180
|
||||
Month 2: 0 deals, avg_area=0, avg_price=0 ← zero row
|
||||
Month 3: 0 deals, avg_area=0, avg_price=0 ← zero row
|
||||
|
||||
Naive AVG: area = (25+0+0)/3 = 8.33 (wrong — dragged down by zeros)
|
||||
Weighted: area = (25*10) / 10 = 25 (correct)
|
||||
|
||||
This replicates the exact bug reported in #295.
|
||||
"""
|
||||
with conn.cursor() as cur:
|
||||
cur.execute(_CREATE_TEMP)
|
||||
rows = [
|
||||
("studio", 10, 25.0, 180.0),
|
||||
("studio", 0, 0.0, 0.0),
|
||||
("studio", 0, 0.0, 0.0),
|
||||
]
|
||||
cur.executemany(_INSERT_ROW, rows)
|
||||
cur.execute(_WEIGHTED_QUERY)
|
||||
result = cur.fetchone()
|
||||
|
||||
assert result is not None
|
||||
room, total_deals, naive_area, weighted_area, naive_price, weighted_price = result
|
||||
|
||||
assert room == "studio"
|
||||
assert total_deals == 10
|
||||
|
||||
# Naive AVG is wrong — zero rows drag it down
|
||||
assert naive_area == Decimal("8.33"), (
|
||||
f"Expected naive AVG=8.33 (showing the bug), got {naive_area}"
|
||||
)
|
||||
assert naive_price == Decimal("60.00"), (
|
||||
f"Expected naive price=60.00 (showing the bug), got {naive_price}"
|
||||
)
|
||||
|
||||
# Weighted AVG is correct — ignores zero-deal months
|
||||
assert weighted_area == Decimal("25.00"), (
|
||||
f"Expected weighted area=25.00, got {weighted_area}"
|
||||
)
|
||||
assert weighted_price == Decimal("180.00"), (
|
||||
f"Expected weighted price=180.00, got {weighted_price}"
|
||||
)
|
||||
|
||||
# The whole point: weighted != naive when zeros present
|
||||
assert weighted_area != naive_area
|
||||
assert weighted_price != naive_price
|
||||
|
||||
def test_weighted_differs_from_naive_sparse_project(
|
||||
self, conn: psycopg.Connection
|
||||
) -> None:
|
||||
"""
|
||||
Scenario inspired by real data: 3-room flat, project with 5 deals
|
||||
spread across 5 active months out of 15 total months.
|
||||
|
||||
Mirrors the 'Vitamin-квартал на Титова' / 3-room case from prod
|
||||
where buggy=27.14m² vs correct=81.43m² (3x undercount).
|
||||
"""
|
||||
with conn.cursor() as cur:
|
||||
cur.execute(_CREATE_TEMP)
|
||||
# 10 zero-deal months + 5 active months with avg_area=81
|
||||
rows = [("3", 0, 0.0, 0.0)] * 10 + [("3", 1, 81.0, 136.0)] * 5
|
||||
cur.executemany(_INSERT_ROW, rows)
|
||||
cur.execute(_WEIGHTED_QUERY)
|
||||
result = cur.fetchone()
|
||||
|
||||
assert result is not None
|
||||
room, total_deals, naive_area, weighted_area, naive_price, weighted_price = result
|
||||
|
||||
assert room == "3"
|
||||
assert total_deals == 5
|
||||
|
||||
# Weighted: 5 deals * 81 / 5 deals = 81.00
|
||||
assert weighted_area == Decimal("81.00")
|
||||
assert weighted_price == Decimal("136.00")
|
||||
|
||||
# Naive: (10*0 + 5*81) / 15 = 27.00 — wrong
|
||||
assert naive_area == Decimal("27.00")
|
||||
|
||||
# Correction factor > 2x
|
||||
ratio = float(weighted_area) / float(naive_area)
|
||||
assert ratio > 2.5, (
|
||||
f"Expected correction factor > 2.5x for sparse project, got {ratio:.2f}"
|
||||
)
|
||||
|
||||
def test_no_zero_months_weighted_equals_naive(
|
||||
self, conn: psycopg.Connection
|
||||
) -> None:
|
||||
"""
|
||||
When every month has deals, weighted and naive AVG should be equal
|
||||
(within numeric(10,2) rounding) only if counts are uniform.
|
||||
When counts differ between months, weighted still differs — and is more accurate.
|
||||
But if all counts are equal, they match exactly.
|
||||
"""
|
||||
with conn.cursor() as cur:
|
||||
cur.execute(_CREATE_TEMP)
|
||||
# All months active, same deal count → weighted == naive
|
||||
rows = [
|
||||
("1", 5, 38.0, 170.0),
|
||||
("1", 5, 40.0, 175.0),
|
||||
("1", 5, 36.0, 165.0),
|
||||
]
|
||||
cur.executemany(_INSERT_ROW, rows)
|
||||
cur.execute(_WEIGHTED_QUERY)
|
||||
result = cur.fetchone()
|
||||
|
||||
assert result is not None
|
||||
_, total_deals, naive_area, weighted_area, naive_price, weighted_price = result
|
||||
|
||||
assert total_deals == 15
|
||||
# With equal weights (5 each), weighted == naive
|
||||
assert weighted_area == naive_area, (
|
||||
f"Equal-weight case: expected weighted={naive_area}, got {weighted_area}"
|
||||
)
|
||||
assert weighted_price == naive_price
|
||||
|
||||
def test_nullif_prevents_division_by_zero(
|
||||
self, conn: psycopg.Connection
|
||||
) -> None:
|
||||
"""
|
||||
When all months have 0 deals, NULLIF(SUM(cnt), 0) → NULL instead of divide-by-zero.
|
||||
The buggy AVG() also returns 0 (not NULL) for all-zero rows, which is arguably
|
||||
worse — it emits a fake value rather than NULL.
|
||||
"""
|
||||
with conn.cursor() as cur:
|
||||
cur.execute(_CREATE_TEMP)
|
||||
rows = [
|
||||
("2", 0, 0.0, 0.0),
|
||||
("2", 0, 0.0, 0.0),
|
||||
]
|
||||
cur.executemany(_INSERT_ROW, rows)
|
||||
cur.execute(_WEIGHTED_QUERY)
|
||||
result = cur.fetchone()
|
||||
|
||||
assert result is not None
|
||||
_, total_deals, naive_area, weighted_area, naive_price, weighted_price = result
|
||||
|
||||
assert total_deals == 0
|
||||
# Weighted formula returns NULL for all-zero-deal case (correct — no real data)
|
||||
assert weighted_area is None, (
|
||||
f"Expected weighted_area=NULL for all-zero-deal case, got {weighted_area}"
|
||||
)
|
||||
assert weighted_price is None, (
|
||||
f"Expected weighted_price=NULL for all-zero-deal case, got {weighted_price}"
|
||||
)
|
||||
# Naive AVG returns 0.00 — a misleading non-NULL value
|
||||
assert naive_area == Decimal("0.00")
|
||||
|
||||
def test_hand_computed_weighted_average(
|
||||
self, conn: psycopg.Connection
|
||||
) -> None:
|
||||
"""
|
||||
End-to-end hand-computed check with multiple room buckets and
|
||||
mixed deal counts to verify the formula is exactly correct.
|
||||
|
||||
Hand computation:
|
||||
studio: area = (30*8 + 28*12 + 27*3) / (8+12+3) = (240+336+81)/23 = 657/23 ≈ 28.57
|
||||
price = (160*8 + 170*12 + 155*3) / 23 = (1280+2040+465)/23 = 3785/23 ≈ 164.57
|
||||
1-room: area = (38*15 + 40*5) / 20 = (570+200)/20 = 770/20 = 38.50
|
||||
price = (175*15 + 180*5) / 20 = (2625+900)/20 = 3525/20 = 176.25
|
||||
"""
|
||||
with conn.cursor() as cur:
|
||||
cur.execute(_CREATE_TEMP)
|
||||
rows = [
|
||||
("studio", 8, 30.0, 160.0),
|
||||
("studio", 12, 28.0, 170.0),
|
||||
("studio", 3, 27.0, 155.0),
|
||||
("1", 15, 38.0, 175.0),
|
||||
("1", 5, 40.0, 180.0),
|
||||
]
|
||||
cur.executemany(_INSERT_ROW, rows)
|
||||
cur.execute(_WEIGHTED_QUERY)
|
||||
rows_out = cur.fetchall()
|
||||
|
||||
by_room = {r[0]: r for r in rows_out}
|
||||
|
||||
studio = by_room["studio"]
|
||||
assert studio[3] == Decimal("28.57"), (
|
||||
f"studio weighted_area: expected 28.57, got {studio[3]}"
|
||||
)
|
||||
assert studio[5] == Decimal("164.57"), (
|
||||
f"studio weighted_price: expected 164.57, got {studio[5]}"
|
||||
)
|
||||
|
||||
one_room = by_room["1"]
|
||||
assert one_room[3] == Decimal("38.50"), (
|
||||
f"1-room weighted_area: expected 38.50, got {one_room[3]}"
|
||||
)
|
||||
assert one_room[5] == Decimal("176.25"), (
|
||||
f"1-room weighted_price: expected 176.25, got {one_room[5]}"
|
||||
)
|
||||
92
data/sql/120_fix_mv_layout_velocity_weighted_avg.sql
Normal file
92
data/sql/120_fix_mv_layout_velocity_weighted_avg.sql
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
-- 120_fix_mv_layout_velocity_weighted_avg.sql
|
||||
-- Fix #295 — mv_layout_velocity used naive AVG() over pre-aggregated per-month rows.
|
||||
-- objective_corpus_room_month has rows even for months with 0 deals; those zero rows
|
||||
-- dragged the average down 2-10x depending on how sparse the project's deal history is.
|
||||
-- (e.g. a project with 1 deal in 1 month out of 24 reported months: AVG divided by 24,
|
||||
-- weighted AVG divides by 1 → correct value.)
|
||||
--
|
||||
-- Fix: replace AVG() with count-weighted formula
|
||||
-- SUM(val * deals_total_count) / NULLIF(SUM(deals_total_count), 0)
|
||||
-- applied to both avg_area_m2 and avg_price_thousand_rub_per_m2.
|
||||
--
|
||||
-- Prod verification (2026-05-28, Vitamin-квартал на Титова, last 24 mo):
|
||||
-- room | buggy_area | wtd_area | buggy_price | wtd_price
|
||||
-- studio| 19.11 | 26.03 | 129.83 | 175.83
|
||||
-- 1-rm | 31.72 | 37.15 | 145.68 | 174.21
|
||||
-- 2-rm | 41.44 | 55.89 | 98.52 | 134.97
|
||||
-- 3-rm | 27.14 | 81.43 | 45.40 | 136.20
|
||||
-- Weighted values match expected (studio ≈28m², 1-rm ≈38m², 2-rm ≈57m², 3-rm ≈81m²,
|
||||
-- prices 110-180k ₽/m²). Sparse 3-rm (5 deals / 15 months) had 3x undercount.
|
||||
--
|
||||
-- All indexes recreated identically to 94_mv_layout_velocity.sql:
|
||||
-- mv_layout_velocity_pk (UNIQUE on obj_id, room_bucket) — required for REFRESH CONCURRENTLY
|
||||
-- mv_layout_velocity_obj_idx (btree on obj_id)
|
||||
--
|
||||
-- CASCADE check (2026-05-28): no dependent views or tables found → no extra DDL needed.
|
||||
--
|
||||
-- Deploy: auto-applied by deploy.yml via _schema_migrations tracking.
|
||||
-- After apply: REFRESH MATERIALIZED VIEW CONCURRENTLY mv_layout_velocity is safe
|
||||
-- (MV is populated WITH DATA in this migration).
|
||||
--
|
||||
-- WARN: Re-applying this file will DROP + recreate the MV (CASCADE-safe as no dependents),
|
||||
-- but will cause a brief data gap. Normally prevented by _schema_migrations tracking.
|
||||
|
||||
BEGIN;
|
||||
|
||||
DROP MATERIALIZED VIEW IF EXISTS mv_layout_velocity CASCADE;
|
||||
|
||||
CREATE MATERIALIZED VIEW mv_layout_velocity AS
|
||||
WITH last24mo AS (
|
||||
SELECT
|
||||
ocm.project_name,
|
||||
CASE
|
||||
WHEN ocm.room_bucket = 'студия' THEN 'studio'
|
||||
ELSE ocm.room_bucket
|
||||
END AS room_bucket,
|
||||
ocm.deals_total_count,
|
||||
ocm.deals_total_avg_area_m2,
|
||||
ocm.deals_total_avg_price_thousand_rub_per_m2,
|
||||
ocm.deals_total_vol_m2,
|
||||
ocm.report_month
|
||||
FROM objective_corpus_room_month ocm
|
||||
WHERE ocm.report_month >= (NOW() - INTERVAL '24 months')::date
|
||||
)
|
||||
SELECT
|
||||
cm.domrf_obj_id AS obj_id,
|
||||
l.room_bucket,
|
||||
SUM(l.deals_total_count)::int AS total_deals_24mo,
|
||||
-- Count-weighted average area: avoids zero-deal months dragging value down
|
||||
(SUM(l.deals_total_avg_area_m2 * l.deals_total_count)
|
||||
/ NULLIF(SUM(l.deals_total_count), 0))::numeric(10, 2) AS avg_area_m2,
|
||||
-- Count-weighted average price: same reasoning
|
||||
(SUM(l.deals_total_avg_price_thousand_rub_per_m2 * l.deals_total_count)
|
||||
/ NULLIF(SUM(l.deals_total_count), 0))::numeric(12, 2) AS avg_price_thousand_rub_per_m2,
|
||||
SUM(l.deals_total_vol_m2)::numeric(12, 2) AS total_vol_m2,
|
||||
MIN(l.report_month) AS window_start,
|
||||
MAX(l.report_month) AS window_end,
|
||||
COUNT(DISTINCT l.report_month)::int AS months_with_data
|
||||
FROM last24mo l
|
||||
JOIN objective_complex_mapping cm
|
||||
ON cm.objective_complex_name = l.project_name
|
||||
WHERE l.room_bucket IS NOT NULL
|
||||
AND cm.domrf_obj_id IS NOT NULL
|
||||
AND cm.objective_group = 'Екатеринбург' -- защита от cross-region Cartesian при future multi-city
|
||||
GROUP BY cm.domrf_obj_id, l.room_bucket
|
||||
WITH DATA;
|
||||
|
||||
-- UNIQUE index required for REFRESH CONCURRENTLY (periodic refreshes via layout_velocity_refresh.py)
|
||||
CREATE UNIQUE INDEX mv_layout_velocity_pk
|
||||
ON mv_layout_velocity (obj_id, room_bucket);
|
||||
|
||||
-- Lookup index used by /best-layouts endpoint
|
||||
CREATE INDEX mv_layout_velocity_obj_idx
|
||||
ON mv_layout_velocity (obj_id);
|
||||
|
||||
COMMENT ON MATERIALIZED VIEW mv_layout_velocity IS
|
||||
'Per-(obj_id, room_bucket) deals aggregation за last 24 months. '
|
||||
'Source: objective_corpus_room_month × objective_complex_mapping (EKB only). '
|
||||
'avg_area_m2 and avg_price_thousand_rub_per_m2 are COUNT-WEIGHTED to avoid '
|
||||
'zero-deal months skewing the average (fix #295). '
|
||||
'Refresh via layout_velocity_refresh.py (concurrently=True — MV is pre-populated).';
|
||||
|
||||
COMMIT;
|
||||
Loading…
Add table
Reference in a new issue