fix(sf): exclude NULL competitors from aggregate calculations (velocity, market_avg, top_sellers) #328

Merged

lekss361 merged 1 commit from fix/sf-aggregate-exclude-nulls into main

2026-05-17 20:46:08 +00:00

lekss361 commented

2026-05-17 20:43:09 +00:00

Owner

Summary

User design policy: показывать все ЖК на карте/в списке, но aggregates (velocity, средневзвешенная цена, top sellers, coverage) — только non-null (ЖК без mapping в Objective не искажают metrics).

Symptoms before

На участке 66:41:0204016:10:

competitors = 20 ЖК (правильно)
3/20 имеют avg_price_per_m2_rub != null (после OBJ-3 PR #324)
UI показывал «velocity 0.0», «срок продажи 1092 мес» (overflow), «top sellers 0 продано м²» — aggregates включали 17 NULL

Fix

backend/app/api/v1/parcels.py (+57 lines):

После compute_velocity добавлен market_pulse computation block, который фильтрует competitor_rows где avg_price_per_m2_rub IS NOT NULL:

market_avg_price_per_m2 — plain average по priced competitors only (Python-side, без SQL change)
top_sellers — top-5 by units_sold из priced competitors only
coverage_pct = priced_count / total * 100
avg_velocity_m2 — из velocity_data["monthly_velocity_sqm"] (уже non-null by construction в compute_velocity — только ЖК с objective_corpus_room_month data вносят вклад)

3 новых top-level ключа в /analyze response:

market_pulse (dict с velocity / avg price / coverage / top sellers)
market_avg_price_per_m2 (top-level alias)
market_data_coverage_pct (top-level alias)

competitors array (20 ЖК) не меняется — UI продолжает показывать все.

Acceptance

После deploy на участке 66:41:0204016:10:

curl -X POST https://gendsgn.ru/api/v1/parcels/66:41:0204016:10/analyze \
  -H "Content-Type: application/json" -d '{}' \
  | jq '{
    competitors_total: (.competitors | length),
    competitors_with_price: .market_pulse.competitors_with_price,
    avg_velocity: .market_pulse.avg_velocity_m2,
    market_avg_price: .market_avg_price_per_m2,
    coverage_pct: .market_data_coverage_pct
  }'

Ожидается:

competitors_total = 20 (без изменений)
competitors_with_price ≥ 3 (after migration 117 PR #327 — could be 8-10)
avg_velocity > 0 (не ноль, не overflow)
market_avg_price ∈ [110000, 220000]
coverage_pct ≈ priced_count × 5

Files

backend/app/api/v1/parcels.py (+57)

Part of issue #307 OBJ-3 follow-up + task #69 SF policy fix.

## Summary User design policy: показывать **все** ЖК на карте/в списке, но aggregates (velocity, средневзвешенная цена, top sellers, coverage) — **только non-null** (ЖК без mapping в Objective не искажают metrics). ## Symptoms before На участке `66:41:0204016:10`: - competitors = 20 ЖК (правильно) - 3/20 имеют `avg_price_per_m2_rub != null` (после OBJ-3 PR #324) - UI показывал «velocity 0.0», «срок продажи 1092 мес» (overflow), «top sellers 0 продано м²» — aggregates включали 17 NULL ## Fix `backend/app/api/v1/parcels.py` (+57 lines): После `compute_velocity` добавлен **market_pulse** computation block, который фильтрует `competitor_rows` где `avg_price_per_m2_rub IS NOT NULL`: - `market_avg_price_per_m2` — plain average по priced competitors only (Python-side, без SQL change) - `top_sellers` — top-5 by `units_sold` из priced competitors only - `coverage_pct` = `priced_count / total * 100` - `avg_velocity_m2` — из `velocity_data["monthly_velocity_sqm"]` (уже non-null by construction в `compute_velocity` — только ЖК с `objective_corpus_room_month` data вносят вклад) 3 новых top-level ключа в `/analyze` response: - `market_pulse` (dict с velocity / avg price / coverage / top sellers) - `market_avg_price_per_m2` (top-level alias) - `market_data_coverage_pct` (top-level alias) `competitors` array (20 ЖК) **не меняется** — UI продолжает показывать все. ## Acceptance После deploy на участке `66:41:0204016:10`: ```bash curl -X POST https://gendsgn.ru/api/v1/parcels/66:41:0204016:10/analyze \ -H "Content-Type: application/json" -d '{}' \ | jq '{ competitors_total: (.competitors | length), competitors_with_price: .market_pulse.competitors_with_price, avg_velocity: .market_pulse.avg_velocity_m2, market_avg_price: .market_avg_price_per_m2, coverage_pct: .market_data_coverage_pct }' ``` Ожидается: - `competitors_total` = 20 (без изменений) - `competitors_with_price` ≥ 3 (after migration 117 PR #327 — could be 8-10) - `avg_velocity > 0` (не ноль, не overflow) - `market_avg_price` ∈ [110000, 220000] - `coverage_pct` ≈ priced_count × 5 ## Files - `backend/app/api/v1/parcels.py` (+57) Part of issue #307 OBJ-3 follow-up + task #69 SF policy fix.

lekss361 added 1 commit 2026-05-17 20:43:09 +00:00

fix(sf): exclude NULL competitors from aggregate calculations (velocity, market_avg, top_sellers) 630a5bbd77

ЖК без маппинга в Objective (Желдорипотека, Эфес и пр.) остаются в
competitor list для карты, но исключаются из aggregate-метрик.

Добавлен блок market_pulse в ответ /analyze:
- market_avg_price_per_m2: AVG(avg_price_per_m2_rub) по non-null competitors
- avg_velocity_m2: monthly_velocity_sqm из compute_velocity (уже non-null by design)
- market_data_coverage_pct / coverage_pct: priced_count / total * 100
- top_sellers: топ-5 ЖК по units_sold среди priced
- competitors_total / competitors_with_price: для UI badge

competitor list (20 ЖК) не изменён.

lekss361 commented

2026-05-17 20:46:02 +00:00

Author

Owner

Deep Code Review — APPROVE

Scope: purely additive Python-side post-processing in parcels.py:2143-2199 + 3 new response keys (market_pulse, market_avg_price_per_m2, market_data_coverage_pct). No existing logic touched.

Aggregate math

market_avg_price_per_m2 = sum(prices) / len(prices) — unweighted mean of per-ЖК means (each avg_price_per_m2_rub already a ROUND(AVG(ol.price_per_m2_rub)) per domrf_obj_id in the obj_pricing CTE). This is "average of averages" — each competitor weighted equally regardless of lot count, which matches the UI intent (market pulse across ЖК, not across lots). Not the Bug #21 shape (no zero-count rows, since NULL is filtered upstream). If product later wants SUM(price*lots_with_price) / SUM(lots_with_price) — separate enhancement; flag in vault as known trade-off.

NULL / zero / divide-by-zero

avg_price_per_m2_rub is not None filter — 0 correctly preserved (0 is not None). Decimal | None from ROUND(...::numeric) → float(...) safe.
units_sold guard is not None and int(...) > 0 — handles LEFT JOIN nulls, prevents zero-sales rows in top_sellers.
coverage_pct guarded with if _competitors_total > 0 else 0.0.
market_avg_price = None when no priced competitors — frontend should handle (UI fallback already exists per task #69).

Contract / response schema

@router.post("/{cad_num}/analyze") has no response_model (line 1050) → raw dict returned, new keys pass through cleanly. No Pydantic schema update needed.
Frontend TS codegen (npm run codegen) — follow-up PR; additive optional fields won't break compilation.

Tests

No strict shape assertions (set(keys) == {...} / .json() == {...}) anywhere in tests/api/v1/ → existing analyze tests (test_analyze_competitors_status, test_analyze_parcel_meta, test_analyze_recent_permits, test_custom_pois) won't break.
No new regression test for market_pulse shape — acceptable for fast-track Python-side fix, but worth adding in follow-up alongside the frontend codegen PR.

Conventions

✓ No psycopg2 / requests / print() / hardcoded creds
✓ Any imported, lines ≤100, logger.* used for velocity exception
✓ Idempotent (no DB write)

Nits (non-blocking, ignore or address later)

int(c["units_sold"]) called twice (lambda key + filter) — micro
Underscore-prefix locals (_competitors_with_price) — convention typically signals "unused"; cosmetic

Merging via squash.

## Deep Code Review — APPROVE **Scope**: purely additive Python-side post-processing in `parcels.py:2143-2199` + 3 new response keys (`market_pulse`, `market_avg_price_per_m2`, `market_data_coverage_pct`). No existing logic touched. ### Aggregate math - `market_avg_price_per_m2 = sum(prices) / len(prices)` — unweighted mean of per-ЖК means (each `avg_price_per_m2_rub` already a `ROUND(AVG(ol.price_per_m2_rub))` per `domrf_obj_id` in the `obj_pricing` CTE). This is "average of averages" — each competitor weighted equally regardless of lot count, which matches the UI intent (market pulse across ЖК, not across lots). Not the Bug #21 shape (no zero-count rows, since NULL is filtered upstream). If product later wants `SUM(price*lots_with_price) / SUM(lots_with_price)` — separate enhancement; flag in vault as known trade-off. ### NULL / zero / divide-by-zero - `avg_price_per_m2_rub is not None` filter — `0` correctly preserved (`0 is not None`). `Decimal | None` from `ROUND(...::numeric)` → `float(...)` safe. - `units_sold` guard `is not None and int(...) > 0` — handles LEFT JOIN nulls, prevents zero-sales rows in `top_sellers`. - `coverage_pct` guarded with `if _competitors_total > 0 else 0.0`. - `market_avg_price = None` when no priced competitors — frontend should handle (UI fallback already exists per task #69). ### Contract / response schema - `@router.post("/{cad_num}/analyze")` has **no `response_model`** (line 1050) → raw dict returned, new keys pass through cleanly. No Pydantic schema update needed. - Frontend TS codegen (`npm run codegen`) — follow-up PR; additive optional fields won't break compilation. ### Tests - No strict shape assertions (`set(keys) == {...}` / `.json() == {...}`) anywhere in `tests/api/v1/` → existing analyze tests (`test_analyze_competitors_status`, `test_analyze_parcel_meta`, `test_analyze_recent_permits`, `test_custom_pois`) won't break. - No new regression test for `market_pulse` shape — acceptable for fast-track Python-side fix, but worth adding in follow-up alongside the frontend codegen PR. ### Conventions - ✓ No `psycopg2` / `requests` / `print()` / hardcoded creds - ✓ `Any` imported, lines ≤100, `logger.*` used for velocity exception - ✓ Idempotent (no DB write) ### Nits (non-blocking, ignore or address later) - `int(c["units_sold"])` called twice (lambda key + filter) — micro - Underscore-prefix locals (`_competitors_with_price`) — convention typically signals "unused"; cosmetic Merging via squash.