feat(tradein): source-quota in estimator analogs (fix Cian/Yandex starvation) #491

Merged

lekss361 merged 1 commit from feat/tradein-estimator-source-quota into main

2026-05-23 20:59:59 +00:00

lekss361 commented

2026-05-23 20:53:59 +00:00

Owner

Bug

/api/v1/trade-in/estimate/{id} для адресов с крупным ЖК (напр. Монтёрская 8/2) возвращал 100% Avito аналогов. Причина: 91 Avito листинг с distance=0м (тот же дом) + ORDER BY distance LIMIT 50 → Cian/Yandex/N1 (ближайший Cian в 175м) полностью вытеснены. UI корректно показывал "Циан — нет данных", но медиана считалась на однобокой выборке.

Changes

tradein-mvp/backend/app/services/estimator.py
- Добавлены константы MAX_ANALOGS_PER_ADDRESS = 5 и MIN_ANALOGS_PER_SOURCE = 5 рядом с AREA_TOLERANCE
- _fetch_analogs переработана (Approach B — fetch wider, filter in Python):
  - SQL-запрос расширен до LIMIT 300 с CTE, вычисляющим relevance_score и rn_addr (per-address row_number); в WHERE добавлен фильтр rn_addr <= :max_per_addr
  - Python: guaranteed pool — первые MIN_ANALOGS_PER_SOURCE строк по relevance из каждого source; оставшиеся слоты (50 − len(guaranteed)) заполняются из remainder по relevance; финальная сортировка + LIMIT 50
- Сигнатура функции, relevance-формула, fallback-логика — без изменений
tradein-mvp/backend/tests/test_estimator_source_quota.py — новый файл, 4 test cases:
1. test_address_cap_limits_per_address_listings — per-address cap не превышается
2. test_source_quota_prevents_cian_starvation — Cian гарантированно >= min(supply, MIN_ANALOGS_PER_SOURCE) (regression test for bug)
3. test_source_quota_includes_all_when_supply_below_min — все 3 Cian попадают, если их меньше квоты
4. test_fallback_signal_reflects_radius — fallback_used boolean сохраняет существующую семантику

Acceptance criteria

_fetch_analogs signature unchanged
2 константы добавлены и используются
4 новых test cases pass (26 passed total incl. pre-existing estimator tests)
Existing tests не сломаны
Ruff clean (0 new errors introduced; 5 pre-existing E501/RUF046 в других строках)
PR base = forgejo/main

## Bug `/api/v1/trade-in/estimate/{id}` для адресов с крупным ЖК (напр. Монтёрская 8/2) возвращал 100% Avito аналогов. Причина: 91 Avito листинг с `distance=0м` (тот же дом) + `ORDER BY distance LIMIT 50` → Cian/Yandex/N1 (ближайший Cian в 175м) полностью вытеснены. UI корректно показывал "Циан — нет данных", но медиана считалась на однобокой выборке. ## Changes - **`tradein-mvp/backend/app/services/estimator.py`** - Добавлены константы `MAX_ANALOGS_PER_ADDRESS = 5` и `MIN_ANALOGS_PER_SOURCE = 5` рядом с `AREA_TOLERANCE` - `_fetch_analogs` переработана (Approach B — fetch wider, filter in Python): - SQL-запрос расширен до LIMIT 300 с CTE, вычисляющим `relevance_score` и `rn_addr` (per-address row_number); в WHERE добавлен фильтр `rn_addr <= :max_per_addr` - Python: guaranteed pool — первые `MIN_ANALOGS_PER_SOURCE` строк по relevance из каждого source; оставшиеся слоты (50 − len(guaranteed)) заполняются из remainder по relevance; финальная сортировка + LIMIT 50 - Сигнатура функции, relevance-формула, fallback-логика — без изменений - **`tradein-mvp/backend/tests/test_estimator_source_quota.py`** — новый файл, 4 test cases: 1. `test_address_cap_limits_per_address_listings` — per-address cap не превышается 2. `test_source_quota_prevents_cian_starvation` — Cian гарантированно >= min(supply, MIN_ANALOGS_PER_SOURCE) (regression test for bug) 3. `test_source_quota_includes_all_when_supply_below_min` — все 3 Cian попадают, если их меньше квоты 4. `test_fallback_signal_reflects_radius` — `fallback_used` boolean сохраняет существующую семантику ## Acceptance criteria - [x] `_fetch_analogs` signature unchanged - [x] 2 константы добавлены и используются - [x] 4 новых test cases pass (26 passed total incl. pre-existing estimator tests) - [x] Existing tests не сломаны - [x] Ruff clean (0 new errors introduced; 5 pre-existing E501/RUF046 в других строках) - [x] PR base = forgejo/main

lekss361 added 1 commit 2026-05-23 20:54:00 +00:00

feat(tradein): source-quota in estimator analogs (fix Cian/Yandex starvation) 27bda58261

Prevents dominant-address Avito listings from starving Cian/Yandex out of
the top-50 analogs via per-address cap (SQL window) + per-source guaranteed
minimum (Python stratified selection).

lekss361 merged commit 7874a5c97b into main

2026-05-23 20:59:59 +00:00

lekss361 referenced this pull request from a commit

2026-05-23 21:00:00 +00:00

feat(tradein): source-quota in estimator analogs (fix Cian/Yandex starvation) (#491)

lekss361 commented

2026-05-23 21:00:27 +00:00

Author

Owner

Merged via deep-code-reviewer — verdict APPROVE.

Algorithm verified end-to-end:

SQL CTE relevance_score formula = identical to previous inline ORDER BY (no scoring regression)
row_number() OVER (PARTITION BY address ORDER BY relevance_score) caps per-address contribution; WHERE rn_addr <= 5 correctly limits
Python stratified selection: by_source preserves SQL relevance order → src_rows[:5] = top-5 per source; id(row) dedup safe (each row is freshly materialized dict); remaining_slots = 50 − len(guaranteed) correctly bounded
Final result.sort(key=lambda r: r.get("relevance_score") or 0.0) — relevance always computed in SQL (CASE returns 0, never NULL), so or 0.0 fallback is defensive but never triggers
4 sources × 5 = 20 guaranteed max + 30 filler = 50; edge case "1 source only" → 5 guaranteed + 45 filler = 50 from same source (preserves lower bound)

Edge cases verified:

Source supply < MIN_ANALOGS_PER_SOURCE → min(len, 5) takes all (Test 3)
Source with 0 listings → not in by_source, silently skipped
Total < 50 → result[:50] no-op, returns all
PARTITION BY address with NULL → all NULLs in one partition (minor concern, addresses rarely NULL post-geocode backfill)

Compliance:

psycopg v3: CAST(:x AS type) used throughout, no :x::type
Signature unchanged: callers in estimate() lines 433/441/453 work unchanged
Return tuple shape unchanged: downstream _filter_outliers/_percentile/price_per_m2 consume same keys
All 27 estimator-related tests pass locally (4 new + 23 existing)

Minor (non-blocking):

SQL CTE duplicates relevance formula (PostgreSQL window functions can't reference SELECT aliases — unavoidable, comment helps)
No tie-breaker on ORDER BY relevance_score — for identical scores order is non-deterministic; acceptable
Performance: LIMIT 300 vs 50 = 6x rows, but GIST ST_DWithin is primary filter; CTE evaluated once; acceptable for interactive /estimate

Cross-PR context:

Builds on #487 (Avito anti-bot/real coords) + #490 (geocode backfill removed Avito exclusion) — quota fix is now meaningful since Avito coords are truthful
Median calculation downstream not affected: shape preserved, only distribution shifts toward diversity

Merged via deep-code-reviewer — verdict APPROVE. **Algorithm verified end-to-end:** - SQL CTE `relevance_score` formula = identical to previous inline ORDER BY (no scoring regression) - `row_number() OVER (PARTITION BY address ORDER BY relevance_score)` caps per-address contribution; `WHERE rn_addr <= 5` correctly limits - Python stratified selection: `by_source` preserves SQL relevance order → `src_rows[:5]` = top-5 per source; `id(row)` dedup safe (each row is freshly materialized dict); `remaining_slots = 50 − len(guaranteed)` correctly bounded - Final `result.sort(key=lambda r: r.get("relevance_score") or 0.0)` — relevance always computed in SQL (CASE returns 0, never NULL), so `or 0.0` fallback is defensive but never triggers - `4 sources × 5 = 20` guaranteed max + `30` filler = `50`; edge case "1 source only" → 5 guaranteed + 45 filler = 50 from same source (preserves lower bound) **Edge cases verified:** - Source supply < MIN_ANALOGS_PER_SOURCE → `min(len, 5)` takes all (Test 3) - Source with 0 listings → not in `by_source`, silently skipped - Total < 50 → `result[:50]` no-op, returns all - `PARTITION BY address` with NULL → all NULLs in one partition (minor concern, addresses rarely NULL post-geocode backfill) **Compliance:** - psycopg v3: `CAST(:x AS type)` used throughout, no `:x::type` - Signature unchanged: callers in `estimate()` lines 433/441/453 work unchanged - Return tuple shape unchanged: downstream `_filter_outliers`/`_percentile`/`price_per_m2` consume same keys - All 27 estimator-related tests pass locally (4 new + 23 existing) **Minor (non-blocking):** - SQL CTE duplicates relevance formula (PostgreSQL window functions can't reference SELECT aliases — unavoidable, comment helps) - No tie-breaker on `ORDER BY relevance_score` — for identical scores order is non-deterministic; acceptable - Performance: LIMIT 300 vs 50 = 6x rows, but GIST `ST_DWithin` is primary filter; CTE evaluated once; acceptable for interactive /estimate **Cross-PR context:** - Builds on #487 (Avito anti-bot/real coords) + #490 (geocode backfill removed Avito exclusion) — quota fix is now meaningful since Avito coords are truthful - Median calculation downstream not affected: shape preserved, only distribution shifts toward diversity