feat(obj-3): SF backend migrate to objective_lots ground truth (price 0.3% → 81%) #324

Merged
lekss361 merged 1 commit from feat/obj-3-sf-migrate-objective into main 2026-05-17 20:08:18 +00:00
Owner

Summary

Issue #307 sub-task OBJ-3 — Site Finder competitors блок мигрирован на objective_lots ground truth (81% price coverage vs 0.3% в domrf_kn_flats).

Backbone JOIN

domrf_kn_objects.obj_idobjective_complex_mapping.domrf_obj_idobjective_complex_mapping.objective_complex_name = objective_lots.project_name

После OBJ-2 PR #323 (mapping backfill 142 → 200+) этот JOIN покрывает большинство ЕКБ ЖК.

Files

  • backend/app/api/v1/parcels.py — competitor query (section 5, analyze_parcel) расширен obj_pricing CTE с LEFT JOIN на mapping → objective_lots
  • backend/tests/test_velocity.py — fix pre-existing breakage от OBJ-2 migration

New competitor fields

  • avg_price_per_m2_rub — средняя ₽/м² (81% coverage)
  • avg_area_pd — средняя площадь лота
  • units_sold / units_available — динамика продаж
  • lots_with_price — UI coverage indicator

LEFT JOIN сохраняет ЖК без mapping.

Tests

Все 33 теста проходят. VelocityResult обновлён с velocity_data_available/velocity_source semantics.

Acceptance

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

  • avg_price_per_m2_rub ∈ [110000, 220000]
  • avg_area_pd ∈ [28, 80]
  • units_sold > 0 для активных ЖК

Part of issue #307 (Wave 2). Depends on OBJ-2 #323 (merged).

## Summary Issue #307 sub-task **OBJ-3** — Site Finder competitors блок мигрирован на `objective_lots` ground truth (81% price coverage vs 0.3% в `domrf_kn_flats`). ## Backbone JOIN `domrf_kn_objects.obj_id` → `objective_complex_mapping.domrf_obj_id` → `objective_complex_mapping.objective_complex_name = objective_lots.project_name` После OBJ-2 PR #323 (mapping backfill 142 → 200+) этот JOIN покрывает большинство ЕКБ ЖК. ## Files - `backend/app/api/v1/parcels.py` — competitor query (section 5, `analyze_parcel`) расширен `obj_pricing` CTE с LEFT JOIN на mapping → objective_lots - `backend/tests/test_velocity.py` — fix pre-existing breakage от OBJ-2 migration ## New competitor fields - `avg_price_per_m2_rub` — средняя ₽/м² (81% coverage) - `avg_area_pd` — средняя площадь лота - `units_sold` / `units_available` — динамика продаж - `lots_with_price` — UI coverage indicator LEFT JOIN сохраняет ЖК без mapping. ## Tests Все **33 теста проходят**. `VelocityResult` обновлён с `velocity_data_available`/`velocity_source` semantics. ## Acceptance После deploy на участке `66:41:0204016:10`: - `avg_price_per_m2_rub` ∈ [110000, 220000] - `avg_area_pd` ∈ [28, 80] - `units_sold` > 0 для активных ЖК Part of issue #307 (Wave 2). Depends on OBJ-2 #323 (merged).
lekss361 added 1 commit 2026-05-17 20:04:02 +00:00
Enrich competitor_rows query with pricing from objective_lots via
objective_complex_mapping JOIN backbone (domrf_obj_id → project_name).

New fields in competitors[]: avg_price_per_m2_rub (81% coverage vs 0.3%
from domrf_kn_flats), avg_area_pd, units_sold, units_available,
lots_with_price. LEFT JOIN preserves competitors without mapping (fields
= NULL), no regression on existing competitor count/order.

Also fix test_velocity.py: update _sales_row mock with has_mapping field
and VelocityResult constructor with velocity_data_available/velocity_source
fields introduced in OBJ-2 (velocity.py had diverged from test mocks).
All 33 velocity + gate_verdict tests pass.

Closes #307 (OBJ-3 sub-task)
Author
Owner

Deep code review — verdict: APPROVE

Files: 2 (P1: parcels.py SQL, P3: test_velocity.py mocks). +60/-16. Mergeable, no conflicts, no prior reviews.

SQL JOIN correctness — verified

latest_obj (DISTINCT ON obj_id) LEFT JOIN obj_pricing CTE (cm.domrf_obj_id, group-by aggregate of objective_complex_mapping ⋈ objective_lots ON cm.objective_complex_name = ol.project_name).

  • LEFT JOIN correctly preserves ЖК without mapping → NULL price fields (matches PR body claim and frontend Optional contract).
  • obj_pricing CTE uses INNER JOIN (mapping ⋈ lots) — non-mapped names are simply absent; LEFT JOIN on outside handles that.
  • Same JOIN pattern as proven 100_fix_mv_layout_velocity_weighted_avg.sql and the pipeline_rows block lines 1392-1399.

Aggregate weighting — NOT a regression of SF Bug #21

The #21 bug (PR #290/#291) was specific to objective_corpus_room_month: per-month aggregate rows with deals_total_count where months with zero sales contributed area=0, price=0 rows, dragging AVG down 2-14×. Different data shape here:

  • objective_lots = per-flat snapshot (UNIQUE objective_lot_id, current state UPSERT per lot)
  • Each row = 1 real flat with a real price_per_m2_rub or NULL
  • Postgres AVG() ignores NULLs by default → only the ~81% of lots with a price contribute
  • Every lot counts equally (no area weighting). For a "avg ₽/m²" market metric this is the standard interpretation (price_per_m2_rub is already normalized per м²); not a regression.

Acceptance band in PR body (avg_price_per_m2_rub ∈ [110000, 220000]) is consistent with current EKB market.

psycopg v3 style — clean

ROUND(AVG(...)::numeric, N)::numeric is attached to the AVG(...) expression (right of )), not to a :bind param. Matches the documented exception in .claude/rules/backend.md. No :wkt::geography-style bugs. All params parametrized via :wkt.

Backward compat

  • Old query AUGMENTED, not removed → comm_name, obj_class, distance_m etc. still present.
  • dict(c) over mappings() picks up new columns automatically.
  • Frontend ParcelAnalysisCompetitor (frontend/src/types/site-finder.ts:34) does NOT yet have avg_price_per_m2_rub / units_sold / etc. — additive backend, no TS break (existing fields untouched). UI consumption + npm run codegen is a fair separate PR (precedent: line 42-45 comment for site_status).

Tests

test_velocity.py adds has_mapping field + new velocity_data_available/velocity_source assertions, consistent with OBJ-2 #323. test_score_zero_when_no_sales_sqm semantics flipped (None → VelocityResult with velocity_data_available=False) is intentional and documented. Mock-only, no live DB needed.

Minor / non-blocking notes (defer to follow-up)

  1. No objective_group = 'Екатеринбург' filter in obj_pricing CTE. Currently EKB-scoped because the mapping table itself is EKB-scoped (per 103_seed_prinzip_mappings.sql, 104_seed_bulk_mappings.sql, 116_obj2_multi_feature_matcher.sql), so practically a no-op. If objective_lots ever ingests multi-city data alongside MSK/SPb mappings, the agg would silently mix cities. Add WHERE cm.objective_group = 'Екатеринбург' defensively in a follow-up.
  2. Sold-vs-available price mixing: AVG(price_per_m2_rub) mixes sold lots (registered at older prices) with currently-available lots. If sold history skews lower, this can drag the avg ~3-8% below current ask. Consider AVG(price_per_m2_rub) FILTER (WHERE NOT is_sold) for an "ask price" variant in a future iteration; current behavior is acceptable as a blended "realised market" metric.
  3. Frontend types regen + UI for the 5 new columns — separate frontend PR (issue #307 follow-up).

Verdict

APPROVE — merging now via squash.

## Deep code review — verdict: APPROVE **Files**: 2 (P1: parcels.py SQL, P3: test_velocity.py mocks). +60/-16. Mergeable, no conflicts, no prior reviews. ### SQL JOIN correctness — verified `latest_obj` (DISTINCT ON obj_id) **LEFT JOIN** `obj_pricing` CTE (`cm.domrf_obj_id`, group-by aggregate of `objective_complex_mapping ⋈ objective_lots ON cm.objective_complex_name = ol.project_name`). - LEFT JOIN correctly preserves ЖК without mapping → NULL price fields (matches PR body claim and frontend Optional contract). - `obj_pricing` CTE uses INNER JOIN (mapping ⋈ lots) — non-mapped names are simply absent; LEFT JOIN on outside handles that. - Same JOIN pattern as proven `100_fix_mv_layout_velocity_weighted_avg.sql` and the `pipeline_rows` block lines 1392-1399. ### Aggregate weighting — NOT a regression of SF Bug #21 The #21 bug (PR #290/#291) was specific to `objective_corpus_room_month`: per-month aggregate rows with `deals_total_count` where months with zero sales contributed `area=0, price=0` rows, dragging AVG down 2-14×. Different data shape here: - `objective_lots` = **per-flat snapshot** (`UNIQUE objective_lot_id`, current state UPSERT per lot) - Each row = 1 real flat with a real `price_per_m2_rub` or NULL - Postgres `AVG()` **ignores NULLs by default** → only the ~81% of lots with a price contribute - Every lot counts equally (no area weighting). For a "avg ₽/m²" market metric this is **the standard interpretation** (price_per_m2_rub is already normalized per м²); not a regression. Acceptance band in PR body (`avg_price_per_m2_rub ∈ [110000, 220000]`) is consistent with current EKB market. ### psycopg v3 style — clean `ROUND(AVG(...)::numeric, N)` — `::numeric` is attached to the `AVG(...)` expression (right of `)`), not to a `:bind` param. Matches the documented exception in `.claude/rules/backend.md`. No `:wkt::geography`-style bugs. All params parametrized via `:wkt`. ### Backward compat - Old query AUGMENTED, not removed → `comm_name`, `obj_class`, `distance_m` etc. still present. - `dict(c)` over `mappings()` picks up new columns automatically. - Frontend `ParcelAnalysisCompetitor` (`frontend/src/types/site-finder.ts:34`) does NOT yet have `avg_price_per_m2_rub` / `units_sold` / etc. — additive backend, no TS break (existing fields untouched). UI consumption + `npm run codegen` is a fair separate PR (precedent: line 42-45 comment for `site_status`). ### Tests `test_velocity.py` adds `has_mapping` field + new `velocity_data_available`/`velocity_source` assertions, consistent with OBJ-2 #323. `test_score_zero_when_no_sales_sqm` semantics flipped (None → VelocityResult with `velocity_data_available=False`) is intentional and documented. Mock-only, no live DB needed. ### Minor / non-blocking notes (defer to follow-up) 1. **No `objective_group = 'Екатеринбург'` filter in `obj_pricing` CTE.** Currently EKB-scoped because the mapping table itself is EKB-scoped (per `103_seed_prinzip_mappings.sql`, `104_seed_bulk_mappings.sql`, `116_obj2_multi_feature_matcher.sql`), so practically a no-op. If `objective_lots` ever ingests multi-city data alongside MSK/SPb mappings, the agg would silently mix cities. Add `WHERE cm.objective_group = 'Екатеринбург'` defensively in a follow-up. 2. **Sold-vs-available price mixing**: `AVG(price_per_m2_rub)` mixes sold lots (registered at older prices) with currently-available lots. If sold history skews lower, this can drag the avg ~3-8% below current ask. Consider `AVG(price_per_m2_rub) FILTER (WHERE NOT is_sold)` for an "ask price" variant in a future iteration; current behavior is acceptable as a blended "realised market" metric. 3. Frontend types regen + UI for the 5 new columns — separate frontend PR (issue #307 follow-up). ### Verdict APPROVE — merging now via squash.
lekss361 merged commit 7a8d59beb4 into main 2026-05-17 20:08:18 +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#324
No description provided.