fix(sf-21): mv_layout_velocity weighted AVG (P0 data corruption) #290

Merged
lekss361 merged 1 commit from fix/sf-21-mv-weighted-avg into main 2026-05-17 14:02:46 +00:00
Owner

🔴 P0 — CRITICAL data corruption

mv_layout_velocity использует unweighted AVG() на objective_corpus_room_month.deals_total_avg_area_m2/price. Источник имеет per-month rows включая месяцы с 0 сделок (count=0, area=0, price=0) — нули тянут среднее в 2-14× вниз.

Все UI значения площади и цены в Top layouts + Recommendation TZ ИСКАЖЕНЫ (например studio = 11.5 м² когда реально 28 м², 3-к = 21.8 м² когда реально 81.7 м²).

Полный context: vault fixes/Bug_MvLayoutVelocity_UnweightedAvg_May17.md.

Verified bug examples (prod DB pre-migration)

obj_id room zero/total months broken AVG fixed weighted AVG
65307 4+ 106/114 (93%) 7.42 m² 105.76 m²
65903 4+ 33/36 (92%) 9.08 m² 108.97 m²
64677 4+ 12/13 (92%) 9.23 m² 119.97 m²
57866 3 52/60 (87%) 14.0 m² 107.6 m²

Fix

data/sql/100_fix_mv_layout_velocity_weighted_avg.sql — DROP + CREATE MV с:

SUM(deals_total_avg_area_m2 * deals_total_count) / NULLIF(SUM(deals_total_count), 0)

Plus UNIQUE INDEX (obj_id, room_bucket) для future REFRESH CONCURRENTLY.

Dependency check: нет дочерних views/MV. DROP без CASCADE issues.

Deploy: auto-applied через _schema_migrations mechanism. REFRESH включён в migration — manual step не нужен.

⚠️ Phase 2 follow-up (НЕ в этом PR)

backend/app/services/site_finder/best_layouts.py:121-122 (_INLINE_VELOCITY_SQL) использует тот же broken pattern напрямую на objective_corpus_room_monthтот же bug, тот же source table. Будет fix отдельным PR. Без него inline-path всё равно покажет broken numbers.

Verify after deploy

SELECT obj_id, room_bucket, total_deals_24mo, avg_area_m2, avg_price_thousand_rub_per_m2
FROM mv_layout_velocity
WHERE obj_id IN (65307, 65903, 64677) AND room_bucket = '4+';
-- Ожидание: avg_area ~105-120, не 7-9

UI: https://gendsgn.ru/site-finder?cad=66:41:0204016:10 → "Рассчитать" в BestLayouts → studio ~28 м² (не 12), 3-к ~81 м² (не 22).

Related: NEW P0 item #21 в epic #271 (вне Wave 1-3 списка, вставлен в начало по severity)

## 🔴 P0 — CRITICAL data corruption `mv_layout_velocity` использует unweighted `AVG()` на `objective_corpus_room_month.deals_total_avg_area_m2/price`. Источник имеет per-month rows **включая месяцы с 0 сделок** (count=0, area=0, price=0) — нули тянут среднее в 2-14× вниз. **Все UI значения площади и цены в Top layouts + Recommendation TZ ИСКАЖЕНЫ** (например studio = 11.5 м² когда реально 28 м², 3-к = 21.8 м² когда реально 81.7 м²). Полный context: vault `fixes/Bug_MvLayoutVelocity_UnweightedAvg_May17.md`. ## Verified bug examples (prod DB pre-migration) | obj_id | room | zero/total months | broken AVG | fixed weighted AVG | |---|---|---|---|---| | 65307 | 4+ | 106/114 (93%) | **7.42 m²** | 105.76 m² | | 65903 | 4+ | 33/36 (92%) | **9.08 m²** | 108.97 m² | | 64677 | 4+ | 12/13 (92%) | **9.23 m²** | 119.97 m² | | 57866 | 3 | 52/60 (87%) | 14.0 m² | 107.6 m² | ## Fix `data/sql/100_fix_mv_layout_velocity_weighted_avg.sql` — DROP + CREATE MV с: ```sql SUM(deals_total_avg_area_m2 * deals_total_count) / NULLIF(SUM(deals_total_count), 0) ``` Plus UNIQUE INDEX `(obj_id, room_bucket)` для future `REFRESH CONCURRENTLY`. **Dependency check**: нет дочерних views/MV. DROP без CASCADE issues. **Deploy**: auto-applied через `_schema_migrations` mechanism. REFRESH включён в migration — manual step не нужен. ## ⚠️ Phase 2 follow-up (НЕ в этом PR) `backend/app/services/site_finder/best_layouts.py:121-122` (`_INLINE_VELOCITY_SQL`) использует тот же broken pattern напрямую на `objective_corpus_room_month` — **тот же bug, тот же source table**. Будет fix отдельным PR. Без него inline-path всё равно покажет broken numbers. ## Verify after deploy ```sql SELECT obj_id, room_bucket, total_deals_24mo, avg_area_m2, avg_price_thousand_rub_per_m2 FROM mv_layout_velocity WHERE obj_id IN (65307, 65903, 64677) AND room_bucket = '4+'; -- Ожидание: avg_area ~105-120, не 7-9 ``` UI: `https://gendsgn.ru/site-finder?cad=66:41:0204016:10` → "Рассчитать" в BestLayouts → studio ~28 м² (не 12), 3-к ~81 м² (не 22). Related: NEW P0 item #21 в epic #271 (вне Wave 1-3 списка, вставлен в начало по severity)
lekss361 added 1 commit 2026-05-17 13:55:02 +00:00
AVG(deals_total_avg_area_m2) averaged pre-aggregated monthly values
including months with 0 deals (area=0, price=0), pulling results down
2-14x. Replace with SUM(x * count) / NULLIF(SUM(count), 0).

Verified on prod: obj 65307 room 4+ area 7.4->105.8 m2, price 7.9->112 k/m2.
Migration 100_ includes REFRESH — no manual step needed post-deploy.
Author
Owner

Deep Code Review — verdict APPROVE (P0 critical fix)

Severity: P0 — data corruption fix, мержим.

Math correctness

SUM(avg_x * count) / NULLIF(SUM(count), 0) математически эквивалентно SUM(total_x) / SUM(count) (т.к. per-month avg_x = total_x / count, произведение восстанавливает total_x). Месяцы с count=0 дают 0 в numerator и 0 в denominator sum — корректно не contributes. Edge case (все месяцы пустые) → NULL через NULLIF. Verified.

SQL structure

  • BEGIN; ... COMMIT; обёртка — соответствует sql.md
  • DROP MV IF EXISTS ... CASCADE — idempotent, безопасно (нет зависимых VIEW/MV — confirmed)
  • WITH NO DATACREATE UNIQUE INDEXREFRESH MATERIALIZED VIEW (non-concurrent) — правильный порядок для initial populate
  • UNIQUE (obj_id, room_bucket) + (obj_id) индексы — schema идентична 94_mv_layout_velocity.sql, downstream consumers (admin_etl.py, parcels.py, layout_velocity_refresh.py, test_phantom_columns.py) не сломаются

Cross-file impact

  • Consumers MV не используют formula напрямую — только column names (avg_area_m2, avg_price_thousand_rub_per_m2), которые сохранены
  • _schema_migrations tracks by full filename → DROP+CREATE применится один раз
  • REFRESH CONCURRENTLY в layout_velocity_refresh.py будет работать (UNIQUE index сохранён)
  • Phase 2 (best_layouts.py:121-122 _INLINE_VELOCITY_SQL) — отдельный PR #291, корректно вынесено за scope

🟡 MINOR: migration numbering conflict

100_fix_mv_layout_velocity_weighted_avg.sql коллизит по prefix с уже applied 100_user_weight_profiles_default_seed.sql. Deploy НЕ упадёт — _schema_migrations tracks по full filename, не по NN-prefix; sort поставит 100_fix_* перед 100_user_* (lex: 'f' < 'u'), оба применятся. Но sql.md правило: NN — следующий sequential номер (ls data/sql/ | sort | tail -5 → следовало бы 106_*).

Не блокирующее: deploy безопасен. Rename можно отдельным cleanup PR (или оставить — historical context).

Post-deploy verify (обязательно)

SELECT obj_id, room_bucket, total_deals_24mo, avg_area_m2, avg_price_thousand_rub_per_m2
FROM mv_layout_velocity
WHERE obj_id IN (65307, 65903, 64677) AND room_bucket = '4+';
-- expected: avg_area_m2 ~105-120, НЕ 7-9

UI: https://gendsgn.ru/site-finder?cad=66:41:0204016:10 → BestLayouts → studio ~28 м² (не 12), 3-к ~81 м² (не 22).

Followup: PR #291 (inline _INLINE_VELOCITY_SQL fix) — merge сразу после этого, иначе inline-path всё ещё будет broken.

Мержу.

## Deep Code Review — verdict ✅ APPROVE (P0 critical fix) **Severity: P0 — data corruption fix, мержим.** ### Math correctness ✅ `SUM(avg_x * count) / NULLIF(SUM(count), 0)` математически эквивалентно `SUM(total_x) / SUM(count)` (т.к. per-month `avg_x = total_x / count`, произведение восстанавливает `total_x`). Месяцы с `count=0` дают `0` в numerator и `0` в denominator sum — корректно не contributes. Edge case (все месяцы пустые) → NULL через `NULLIF`. Verified. ### SQL structure ✅ - `BEGIN; ... COMMIT;` обёртка — соответствует `sql.md` - `DROP MV IF EXISTS ... CASCADE` — idempotent, безопасно (нет зависимых VIEW/MV — confirmed) - `WITH NO DATA` → `CREATE UNIQUE INDEX` → `REFRESH MATERIALIZED VIEW` (non-concurrent) — правильный порядок для initial populate - `UNIQUE (obj_id, room_bucket)` + `(obj_id)` индексы — schema идентична `94_mv_layout_velocity.sql`, downstream consumers (`admin_etl.py`, `parcels.py`, `layout_velocity_refresh.py`, `test_phantom_columns.py`) не сломаются ### Cross-file impact ✅ - Consumers MV не используют formula напрямую — только column names (`avg_area_m2`, `avg_price_thousand_rub_per_m2`), которые сохранены - `_schema_migrations` tracks by full filename → DROP+CREATE применится один раз - `REFRESH CONCURRENTLY` в `layout_velocity_refresh.py` будет работать (UNIQUE index сохранён) - Phase 2 (`best_layouts.py:121-122` `_INLINE_VELOCITY_SQL`) — отдельный PR #291, корректно вынесено за scope ### 🟡 MINOR: migration numbering conflict `100_fix_mv_layout_velocity_weighted_avg.sql` коллизит по prefix с уже applied `100_user_weight_profiles_default_seed.sql`. Deploy НЕ упадёт — `_schema_migrations` tracks по full filename, не по NN-prefix; `sort` поставит `100_fix_*` перед `100_user_*` (lex: 'f' < 'u'), оба применятся. Но `sql.md` правило: `NN — следующий sequential номер` (`ls data/sql/ | sort | tail -5` → следовало бы `106_*`). Не блокирующее: deploy безопасен. Rename можно отдельным cleanup PR (или оставить — historical context). ### Post-deploy verify (обязательно) ```sql SELECT obj_id, room_bucket, total_deals_24mo, avg_area_m2, avg_price_thousand_rub_per_m2 FROM mv_layout_velocity WHERE obj_id IN (65307, 65903, 64677) AND room_bucket = '4+'; -- expected: avg_area_m2 ~105-120, НЕ 7-9 ``` UI: `https://gendsgn.ru/site-finder?cad=66:41:0204016:10` → BestLayouts → studio ~28 м² (не 12), 3-к ~81 м² (не 22). **Followup**: PR #291 (inline `_INLINE_VELOCITY_SQL` fix) — merge сразу после этого, иначе inline-path всё ещё будет broken. Мержу.
lekss361 merged commit 9f5f887393 into main 2026-05-17 14:02:46 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#290
No description provided.