lekss361/gendesign

Fork 0

fix(#112): competitors avg_price_per_m2 всегда None — убран WHERE status='sold' #228

Merged

lekss361 merged 1 commit from fix/issue112-sold-filter-avg-price into main

2026-05-16 19:52:45 +00:00

lekss361 commented

2026-05-16 19:46:33 +00:00

Owner

Проблема

avg_price_per_m2 возвращал null для всех конкурентов. Баг из Issue #227.

Root cause

_AVG_PRICE_SQL фильтровал AND f.status = 'sold', но domrf_kn_flats.status заполнен только в ~0.2% строк → AVG(price_per_m2) всегда NULL.

Fix

Убран фильтр AND f.status = 'sold'. AVG теперь по всем квартирам с price_per_m2 IS NOT NULL — корректно для средней цены ЖК.

Changes

backend/app/services/site_finder/competitors.py — remove broken status filter, add comment
backend/tests/api/v1/test_parcel_competitors.py — add test_competitors_avg_price_populated regression test

Acceptance

avg_price_per_m2 не None на реальном кадастре после merge (qa-tester проверит curl)
Tests pass: uv run pytest backend/tests/api/v1/test_parcel_competitors.py

## Проблема `avg_price_per_m2` возвращал `null` для всех конкурентов. Баг из Issue #227. ## Root cause `_AVG_PRICE_SQL` фильтровал `AND f.status = 'sold'`, но `domrf_kn_flats.status` заполнен только в ~0.2% строк → `AVG(price_per_m2)` всегда NULL. ## Fix Убран фильтр `AND f.status = 'sold'`. `AVG` теперь по всем квартирам с `price_per_m2 IS NOT NULL` — корректно для средней цены ЖК. ## Changes - `backend/app/services/site_finder/competitors.py` — remove broken status filter, add comment - `backend/tests/api/v1/test_parcel_competitors.py` — add `test_competitors_avg_price_populated` regression test ## Acceptance - [ ] `avg_price_per_m2` не None на реальном кадастре после merge (qa-tester проверит curl) - [ ] Tests pass: `uv run pytest backend/tests/api/v1/test_parcel_competitors.py`

lekss361 added the

bug

feedback/max

site-finder

labels 2026-05-16 19:46:33 +00:00

lekss361 added 1 commit 2026-05-16 19:46:34 +00:00

fix(#112 ): remove broken status='sold' filter from competitors avg_price query 31581cedd2

domrf_kn_flats.status is NULL in ~99.8% of rows, so WHERE status='sold'
always returned 0 rows and avg_price_per_m2 was always None. Drop the
filter; AVG over all rows with price_per_m2 IS NOT NULL is semantically
correct for a complex-level price estimate.

Adds regression test test_competitors_avg_price_populated (Issue #227).

lekss361 commented

2026-05-16 19:52:33 +00:00

Author

Owner

Deep Code Review — verdict APPROVE

Severity: APPROVE (no critical / high / medium issues)
Files reviewed: 2 (P1: competitors.py, P3: test_parcel_competitors.py) — +33/-2
Head SHA: 31581ce (стабилен, fixup'ов не было)

Что проверено

Diff scope: убрана ровно одна строка AND f.status = 'sold'. Фильтр f.price_per_m2 IS NOT NULL сохранён, GROUP BY / запятые целы. Чисто.
Объяснительный комментарий перед _AVG_PRICE_SQL — 3 строки + ссылка на Issue #112/227. Защита от регрессии: будущий разработчик не вернёт фильтр обратно.
psycopg v3 conventions: CAST(:x AS type) соблюдён везде в _COMPETITORS_SQL; bind :obj_ids + ANY(...) без :: cast trap. grep ':[a-z_]+::[a-z]' пуст.
SQL safety: bind params, без f-string / конкатенации. SQL injection нет.
Тест test_competitors_avg_price_populated: mock-based (консистентно с docstring файла line 4), assert pytest.approx(150_000.0), explicit message про регрессию #227. Регрессионный — fails если кто-то вернёт status='sold' (запрос вернёт пустой result map → avg_price_per_m2=None).
Cross-file impact: TS frontend/src/types/site-finder.ts:305 уже number | null → contract не меняется. SuccessRecommendationBlock.tsx:171 через formatPrice — null-safe. Frontend изменений не нужно.
Vault: fixes/Fix_Competitors_Sold_Filter_May16.md создан, ссылается на forgejo_issue: 227. Соблюдён CLAUDE.md rule #6.
History: filter был в original commit f3d85f4 (Issue #112 backend), без промежуточных правок — это первая корректировка, не повтор ошибки.

Data semantic note (не блокирует, на будущее)

После фикса avg_price_per_m2 = AVG по всем domrf_kn_flats с price_per_m2 IS NOT NULL — включает активные объявления + любые статусы. Семантически это «средняя цена м² в ЖК», а не «средняя цена продажи». Текущее имя поля нейтральное (avg_price_per_m2, не avg_sold_price_per_m2) — UI tooltip / label не нужно менять. Если в будущем появится отдельная метрика «средняя цена закрытых сделок» (когда status будет надёжно заполнено) — она будет новым полем, не переименованием.

Carryover (отдельным PR, scope этого фикса не расширять)

backend/tests/integration/test_phantom_columns.py:294-307 — test_avg_price_sold_query всё ещё EXPLAIN'ит SQL с AND f.status = 'sold'. Тест проходит (EXPLAIN валиден), но имя и тело не отражают актуальный production-запрос. Документационный drift — можно обновить отдельным chore PR.

Risk / blast radius

Risk: Low — single SQL filter removal, ничего не ломается; в worst case avg_price_per_m2 для немногих ЖК с очень редкими ценами окажется чуть менее точным (что лучше чем гарантированный null).
Reversibility: easy revert — одна строка.
Merge window: anytime.

Approved. Мержу через merge_pull_request (style: merge).

## Deep Code Review — verdict APPROVE **Severity**: APPROVE (no critical / high / medium issues) **Files reviewed**: 2 (P1: competitors.py, P3: test_parcel_competitors.py) — +33/-2 **Head SHA**: 31581ce (стабилен, fixup'ов не было) ### Что проверено - **Diff scope**: убрана ровно одна строка `AND f.status = 'sold'`. Фильтр `f.price_per_m2 IS NOT NULL` сохранён, `GROUP BY` / запятые целы. Чисто. - **Объяснительный комментарий** перед `_AVG_PRICE_SQL` — 3 строки + ссылка на Issue #112/227. Защита от регрессии: будущий разработчик не вернёт фильтр обратно. - **psycopg v3 conventions**: `CAST(:x AS type)` соблюдён везде в `_COMPETITORS_SQL`; bind `:obj_ids` + `ANY(...)` без `::` cast trap. `grep ':[a-z_]+::[a-z]'` пуст. - **SQL safety**: bind params, без f-string / конкатенации. SQL injection нет. - **Тест `test_competitors_avg_price_populated`**: mock-based (консистентно с docstring файла line 4), assert `pytest.approx(150_000.0)`, explicit message про регрессию #227. Регрессионный — fails если кто-то вернёт `status='sold'` (запрос вернёт пустой result map → `avg_price_per_m2=None`). - **Cross-file impact**: TS `frontend/src/types/site-finder.ts:305` уже `number | null` → contract не меняется. `SuccessRecommendationBlock.tsx:171` через `formatPrice` — null-safe. Frontend изменений не нужно. - **Vault**: `fixes/Fix_Competitors_Sold_Filter_May16.md` создан, ссылается на `forgejo_issue: 227`. Соблюдён CLAUDE.md rule #6. - **History**: filter был в original commit `f3d85f4` (Issue #112 backend), без промежуточных правок — это первая корректировка, не повтор ошибки. ### Data semantic note (не блокирует, на будущее) После фикса `avg_price_per_m2` = AVG по всем `domrf_kn_flats` с `price_per_m2 IS NOT NULL` — включает активные объявления + любые статусы. Семантически это **«средняя цена м² в ЖК»**, а не «средняя цена продажи». Текущее имя поля нейтральное (`avg_price_per_m2`, не `avg_sold_price_per_m2`) — UI tooltip / label не нужно менять. Если в будущем появится отдельная метрика «средняя цена закрытых сделок» (когда `status` будет надёжно заполнено) — она будет новым полем, не переименованием. ### Carryover (отдельным PR, scope этого фикса не расширять) - `backend/tests/integration/test_phantom_columns.py:294-307` — `test_avg_price_sold_query` всё ещё EXPLAIN'ит SQL с `AND f.status = 'sold'`. Тест проходит (EXPLAIN валиден), но имя и тело не отражают актуальный production-запрос. Документационный drift — можно обновить отдельным chore PR. ### Risk / blast radius - **Risk**: Low — single SQL filter removal, ничего не ломается; в worst case `avg_price_per_m2` для немногих ЖК с очень редкими ценами окажется чуть менее точным (что лучше чем гарантированный `null`). - **Reversibility**: easy revert — одна строка. - **Merge window**: anytime. Approved. Мержу через `merge_pull_request` (style: `merge`).

lekss361 merged commit e7a5ba8f81 into main

2026-05-16 19:52:45 +00:00

lekss361 referenced this pull request from a commit

2026-05-16 19:52:46 +00:00

Merge pull request 'fix(#112): competitors avg_price_per_m2 всегда None — убран WHERE status='sold'' (#228) from fix/issue112-sold-filter-avg-price into main

lekss361 commented

2026-05-16 19:52:56 +00:00

Author

Owner

Merged via deep-code-reviewer — verdict APPROVE. Merge commit e7a5ba8.

QA-tester должен проверить на проде: curl POST /api/v1/parcels/{cad_num}/competitors → competitors[].avg_price_per_m2 не null для ЖК с реальными ценами в domrf_kn_flats.

Merged via deep-code-reviewer — verdict APPROVE. Merge commit `e7a5ba8`. QA-tester должен проверить на проде: `curl POST /api/v1/parcels/{cad_num}/competitors` → `competitors[].avg_price_per_m2` не null для ЖК с реальными ценами в `domrf_kn_flats`.