fix(tradein/pricing): скоупить сторону объявлений по городу, как сторону сделок (#2583 H2) #2617

Merged
lekss361 merged 1 commit from fix/tradein-asking-sold-city-scope into main 2026-08-02 09:10:36 +00:00
Owner

Summary

  • Bug (audit #2583 H2): asking_to_sold_ratios refresh (tradein-mvp/backend/app/tasks/asking_to_sold_ratio.py) scopes the SOLD side (deal_side/deal_global) to Ekaterinburg (city ILIKE :asking_city), but the ASKING side (ask_side/ask_global) had NO city filter at all. Oblast sweeps went live 2026-07-12 — cheap oblast listings entered the ask-median denominator unscoped while the sold-median numerator stayed EKB-only. Result: ratio = sold_median / ask_median systemically inflated, buyout prices overstated 2.5-5.3% across nearly all room buckets (including EKB estimates, since the ratio itself is shared).
  • Fix: added AND (city IS NULL OR city ILIKE :asking_city) to both ask_side and ask_global, symmetric to the deal-side predicate. city IS NULL is deliberately kept (not excluded) — listings.city is populated only for Avito so far (#2598/#2606: Cian/Domclick/Yandex rows are still NULL); a naive symmetric filter without the NULL-tolerance would drop ~71% of the ask sample instead of the ~19% of genuinely-oblast rows. As column coverage grows, the predicate tightens itself with no further code changes; once coverage is complete it can be hardened to the strictly symmetric city ILIKE :asking_city.
  • Grepped the file for every FROM listings occurrence — confirmed exactly these two CTEs reference the ask-side, no other unscoped spot.

Measurements (prod, read-only, 2026-08-02)

Listings passing the existing WHERE predicates (is_active, rooms IS NOT NULL, ppm² band, novostroyki guard), grouped by city:

city n median ₽/m²
(city unknown / NULL) 8218 138 461
Екатеринбург 2132 150 803
Каменск-Уральский 285 69 750
Нижний Тагил 267 77 702
Верхняя Пышма 245 148 861
Первоуральск 211 79 032
Серов 150 63 958
total 11 508

After the fix, the ask-side sample is NULL + Екатеринбург = 8218 + 2132 = 10 350 rows (out of 11 508) — not the 2 132 EKB-only rows a naive fix would collapse to, and not the full 11 508 unscoped rows of the current bug.

Per-room-bucket ask_median and resulting ratio (sold_median unchanged, EKB-only):

bucket n before ask_median before n after ask_median after ratio before ratio after ratio inflation
0 (студия) 823 183 333 657 182 825 0.7322 0.7342 ~0%
1 3063 145 552 2637 153 015 0.8525 0.8110 +5.1%
2 3840 129 448 3492 133 600 0.9027 0.8747 +3.2%
3 2962 125 000 2782 128 205 0.9834 0.9589 +2.6%
4+ 820 127 733 782 130 567 1.0500 1.0272 +2.2%
global (-1 fallback) 11 508 136 547 10 350 140 794 0.9089 0.8815 +3.1%

Matches the audit's expected +2.5% to +5.3% range.

Heads-up: once asking_to_sold_ratio_refresh runs on its next schedule (06:00-07:00 UTC), buyout prices will drop ~2.5-5.3% system-wide (studio bucket ~unaffected). This is the fix taking effect — the previous ratio was inflated by oblast listings improperly diluting the ask-median — not a regression. Be ready for a "why did prices drop" question.

Scope / what was NOT touched

  • estimator.py — reads the ratio unchanged.
  • deal_side/deal_global — already correctly scoped, untouched.
  • n_deals >= 30 / n_listings >= 30 thresholds and bucket -1 fallback logic — untouched.
  • asking_city config / _ASKING_CITY_PATTERN value — untouched (still %Екатеринбург%, now referenced by both sides).
  • No SQL migration — the fix lives entirely in the Python-side refresh query; the table repopulates on its normal schedule.

Test plan

  • Two new falsifying tests added: test_ask_side_and_ask_global_scoped_to_asking_city (asserts the new predicate is present in both CTEs) and test_ask_side_keeps_city_is_null_rows_not_naive_filter (guards against the naive/wrong fix that would silently drop city-IS-NULL rows). Verified via git stash on the implementation-only lines: both FAIL against unpatched code (AssertionError: ask_side AS: missing IS NULL tolerance etc.), then PASS after git stash pop.
  • Updated test_rederivation_scopes_sold_side_to_asking_city (removed now-false assertion that ask-side is unfiltered) and test_migration_080_derivation_is_subset_of_refresh_sql's _drop_city_guard helper to also normalise-away the new ask-side predicate so refresh-vs-080-seed subset comparison still holds.
  • Full tests/test_asking_to_sold_ratio.py: 21/21 passed.
  • Full pytest in tradein-mvp/backend: 3058 passed, 9 skipped, 1 pre-existing failure (tests/test_search_api.py::test_search_cache_hit, 401 RBAC — unrelated, not touched by this PR).
  • ruff check clean on both changed files.

Refs #2583

## Summary - **Bug (audit #2583 H2):** `asking_to_sold_ratios` refresh (`tradein-mvp/backend/app/tasks/asking_to_sold_ratio.py`) scopes the SOLD side (`deal_side`/`deal_global`) to Ekaterinburg (`city ILIKE :asking_city`), but the ASKING side (`ask_side`/`ask_global`) had NO city filter at all. Oblast sweeps went live 2026-07-12 — cheap oblast listings entered the ask-median denominator unscoped while the sold-median numerator stayed EKB-only. Result: `ratio = sold_median / ask_median` systemically inflated, buyout prices overstated 2.5-5.3% across nearly all room buckets (including EKB estimates, since the ratio itself is shared). - **Fix:** added `AND (city IS NULL OR city ILIKE :asking_city)` to both `ask_side` and `ask_global`, symmetric to the deal-side predicate. `city IS NULL` is deliberately kept (not excluded) — `listings.city` is populated only for Avito so far (#2598/#2606: Cian/Domclick/Yandex rows are still NULL); a naive symmetric filter without the NULL-tolerance would drop ~71% of the ask sample instead of the ~19% of genuinely-oblast rows. As column coverage grows, the predicate tightens itself with no further code changes; once coverage is complete it can be hardened to the strictly symmetric `city ILIKE :asking_city`. - Grepped the file for every `FROM listings` occurrence — confirmed exactly these two CTEs reference the ask-side, no other unscoped spot. ## Measurements (prod, read-only, 2026-08-02) Listings passing the existing WHERE predicates (`is_active`, `rooms IS NOT NULL`, ppm² band, novostroyki guard), grouped by city: | city | n | median ₽/m² | |---|---|---| | (city unknown / NULL) | 8218 | 138 461 | | Екатеринбург | 2132 | 150 803 | | Каменск-Уральский | 285 | 69 750 | | Нижний Тагил | 267 | 77 702 | | Верхняя Пышма | 245 | 148 861 | | Первоуральск | 211 | 79 032 | | Серов | 150 | 63 958 | | **total** | **11 508** | | After the fix, the ask-side sample is `NULL + Екатеринбург = 8218 + 2132 = 10 350` rows (out of 11 508) — **not** the 2 132 EKB-only rows a naive fix would collapse to, and not the full 11 508 unscoped rows of the current bug. Per-room-bucket `ask_median` and resulting `ratio` (sold_median unchanged, EKB-only): | bucket | n before | ask_median before | n after | ask_median after | ratio before | ratio after | ratio inflation | |---|---|---|---|---|---|---|---| | 0 (студия) | 823 | 183 333 | 657 | 182 825 | 0.7322 | 0.7342 | ~0% | | 1 | 3063 | 145 552 | 2637 | 153 015 | 0.8525 | 0.8110 | **+5.1%** | | 2 | 3840 | 129 448 | 3492 | 133 600 | 0.9027 | 0.8747 | +3.2% | | 3 | 2962 | 125 000 | 2782 | 128 205 | 0.9834 | 0.9589 | +2.6% | | 4+ | 820 | 127 733 | 782 | 130 567 | 1.0500 | 1.0272 | +2.2% | | global (-1 fallback) | 11 508 | 136 547 | 10 350 | 140 794 | 0.9089 | 0.8815 | +3.1% | Matches the audit's expected +2.5% to +5.3% range. **Heads-up:** once `asking_to_sold_ratio_refresh` runs on its next schedule (06:00-07:00 UTC), buyout prices will drop ~2.5-5.3% system-wide (studio bucket ~unaffected). This is the fix taking effect — the previous ratio was inflated by oblast listings improperly diluting the ask-median — not a regression. Be ready for a "why did prices drop" question. ## Scope / what was NOT touched - `estimator.py` — reads the ratio unchanged. - `deal_side`/`deal_global` — already correctly scoped, untouched. - `n_deals >= 30` / `n_listings >= 30` thresholds and bucket `-1` fallback logic — untouched. - `asking_city` config / `_ASKING_CITY_PATTERN` value — untouched (still `%Екатеринбург%`, now referenced by both sides). - No SQL migration — the fix lives entirely in the Python-side refresh query; the table repopulates on its normal schedule. ## Test plan - [x] Two new falsifying tests added: `test_ask_side_and_ask_global_scoped_to_asking_city` (asserts the new predicate is present in both CTEs) and `test_ask_side_keeps_city_is_null_rows_not_naive_filter` (guards against the naive/wrong fix that would silently drop city-IS-NULL rows). Verified via `git stash` on the implementation-only lines: both FAIL against unpatched code (`AssertionError: ask_side AS: missing IS NULL tolerance` etc.), then PASS after `git stash pop`. - [x] Updated `test_rederivation_scopes_sold_side_to_asking_city` (removed now-false assertion that ask-side is unfiltered) and `test_migration_080_derivation_is_subset_of_refresh_sql`'s `_drop_city_guard` helper to also normalise-away the new ask-side predicate so refresh-vs-080-seed subset comparison still holds. - [x] Full `tests/test_asking_to_sold_ratio.py`: 21/21 passed. - [x] Full `pytest` in `tradein-mvp/backend`: 3058 passed, 9 skipped, 1 pre-existing failure (`tests/test_search_api.py::test_search_cache_hit`, 401 RBAC — unrelated, not touched by this PR). - [x] `ruff check` clean on both changed files. Refs #2583
lekss361 added 1 commit 2026-08-02 08:54:45 +00:00
fix(tradein/pricing): скоупить сторону объявлений по городу, как сторону сделок (#2583 H2)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 9s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 2m37s
661f19853b
Author
Owner

Deep review — APPROVE (независимая перепроверка на проде)

Все замеры автора воспроизведены байт-в-байт на проде (read-only, 2026-08-02): срез по городам 8218/2132/285/267/245/211/150 = 11 508; после фикса 10 350; global ask 136 547 → 140 794; ratio 0.9089 → 0.8815. Наивный вариант (AND city ILIKE :asking_city без IS NULL) даёт 2 132 строки и ratio 0.8230 (−9.4%) — подтверждено, NULL-толерантность выбрана правильно.

Фальсификация тестов: реализация откачена на main-версию файла (git checkout <base> -- app/tasks/asking_to_sold_ratio.py), тесты оставлены → ровно 2 падения (test_ask_side_and_ask_global_scoped_to_asking_city, test_ask_side_keeps_city_is_null_rows_not_naive_filter). Отдельная мутация «наивный фикс» → 3 падения. Заявление автора подтверждено.

Остаточный перекос — цифры для протокола (H2 закрыт НЕ полностью)

Фикс убирает 1 158 областных строк, но часть областных объявлений остаётся внутри ветки city IS NULL. Замер по координатам (расстояние от центра ЕКБ 56.8389, 60.6057) среди 8 218 NULL-city строк:

полоса n median ₽/м²
≤20 км 8 004 139 810
20–40 км 5 109 285
40–100 км 10 81 920
>100 км 197 83 488
без гео 2 143 182

То есть ~210 заведомо неЕКБ-строк (в основном Нижний Тагил, ~197) остаются в знаменателе. «Идеальный» фикс (гео-фильтр ≤30 км поверх городского) дал бы:

bucket n после фикса ask после n «идеал» ask «идеал» остаточный сдвиг ratio
0 657 182 825 653 183 333 −0.28%
1 2 637 153 015 2 553 154 850 −1.20%
2 3 492 133 600 3 371 135 081 −1.11%
3 2 782 128 205 2 782 128 205 0.00%
4+ 782 130 567 781 130 641 −0.06%
global 10 350 140 794 10 140 141 715 −0.65%

Вывод: фикс снимает ~80–85% ошибки H2, остаточное завышение коэффициента ≈0.6–1.2%. Это приемлемо (сильно лучше, чем ничего), но H2 нельзя считать полностью закрытым — оставить follow-up в #2583 до полного покрытия listings.city. Хорошая новость: колонка уже проставляется на новых прогонах (за 3 дня: cian 939/953, yandex 3002/3704, avito 1722/1722), так что остаток будет сам сходиться к нулю — «предикат сам ужесточается» из комментария подтверждается фактами.

Дополнения к описанию эффекта на цены

estimator.py:2911-2919 клампит effective_ratio > 1.0 до 1.0 (ESTIMATE_EXPECTED_SOLD_LE_ASKING, default True, на проде не переопределён). Поэтому фактический эффект на выкупную цену отличается от таблицы ratio:

  • bucket 0 (студии): 0.7322 → 0.7342 — цена вырастет на +0.28%, а не «~0%»;
  • bucket 1: −4.88%, bucket 2: −3.11%, bucket 3: −2.50%;
  • bucket 4+: 1.0500 и 1.0272 оба клампятся до 1.0 — изменения цены НЕТ;
  • global -1 fallback: −3.01%.

Итого клиентам: −2.5…−4.9% на 1/2/3-комнатных, ~+0.3% на студиях, 0% на 4+.

Прочее (не блокирует)

  • deals.city не имеет NULL вообще (0 из 96 974) — асимметрия предикатов (deal-сторона строгая, ask-сторона NULL-толерантная) доказуемо безвредна.
  • %Екатеринбург% корректно матчит обе таблицы: в deals формы Екатеринбург / город Екатеринбург / городской округ город Екатеринбург, в listings ровно Екатеринбург; ложных срабатываний по остальным 5 значениям колонки нет.
  • Пороги 30/30: после фикса минимум n_listings = 657 (bucket 0), минимум n_deals = 1 476 — запас 20×+, ни один бакет не уезжает на global fallback.
  • ratio > 1 у 4+ комнат — не шум (n_deals=1 476, n_listings=782), а методическая асимметрия: у ask-стороны есть novostroyki-гард (#1186), у deal-стороны его нет и в deals вообще нет segment-маркера; плюс deals.rooms обрезан на 4 (max=4, >4 нет), а в listings в bucket 4 сваливаются 5–10-комнатные. Предсуществующее, в этом PR чинить не нужно, но стоит завести отдельно.
  • Единый ЕКБ-коэффициент по-прежнему применяется ко всем городам (#647). Реальные per-city ratio на проде: ЕКБ 0.823, Н. Тагил 0.822, Первоуральск 0.847, В. Пышма 0.692, Каменск-Уральский 0.921, Серов 0.920. Для Каменска и Серова фикс уводит применяемый коэффициент чуть дальше от истины, для остальных — ближе. Нетто выигрыш очевиден.
  • Этот PR превращает listings.city из write-only колонки (#2598) в money-critical чтение — любой будущий mis-stamp двигает цены напрямую. Гард на месте (test_city_sweep.py::test_resolve_city_name_known_oblast_slugs, set-equality по CITY_LOCATIONS), но это стоит помнить.

Полный pytest воспроизведён локально: 1 failed, 3058 passed, 9 skipped — единственный фейл tests/test_search_api.py::test_search_cache_hit (401), pre-existing. ruff check чист. Диff строго в границах: только tasks/asking_to_sold_ratio.py + его тесты; estimator.py, миграции, пороги и deal-сторона не тронуты.

## Deep review — APPROVE (независимая перепроверка на проде) Все замеры автора воспроизведены байт-в-байт на проде (read-only, 2026-08-02): срез по городам 8218/2132/285/267/245/211/150 = 11 508; после фикса 10 350; global ask 136 547 → 140 794; ratio 0.9089 → 0.8815. Наивный вариант (`AND city ILIKE :asking_city` без `IS NULL`) даёт 2 132 строки и ratio 0.8230 (−9.4%) — подтверждено, NULL-толерантность выбрана правильно. Фальсификация тестов: реализация откачена на `main`-версию файла (`git checkout <base> -- app/tasks/asking_to_sold_ratio.py`), тесты оставлены → ровно 2 падения (`test_ask_side_and_ask_global_scoped_to_asking_city`, `test_ask_side_keeps_city_is_null_rows_not_naive_filter`). Отдельная мутация «наивный фикс» → 3 падения. Заявление автора подтверждено. ### Остаточный перекос — цифры для протокола (H2 закрыт НЕ полностью) Фикс убирает 1 158 областных строк, но часть областных объявлений остаётся внутри ветки `city IS NULL`. Замер по координатам (расстояние от центра ЕКБ 56.8389, 60.6057) среди 8 218 NULL-city строк: | полоса | n | median ₽/м² | |---|---|---| | ≤20 км | 8 004 | 139 810 | | 20–40 км | 5 | 109 285 | | 40–100 км | 10 | 81 920 | | >100 км | 197 | 83 488 | | без гео | 2 | 143 182 | То есть ~210 заведомо неЕКБ-строк (в основном Нижний Тагил, ~197) остаются в знаменателе. «Идеальный» фикс (гео-фильтр ≤30 км поверх городского) дал бы: | bucket | n после фикса | ask после | n «идеал» | ask «идеал» | остаточный сдвиг ratio | |---|---|---|---|---|---| | 0 | 657 | 182 825 | 653 | 183 333 | −0.28% | | 1 | 2 637 | 153 015 | 2 553 | 154 850 | −1.20% | | 2 | 3 492 | 133 600 | 3 371 | 135 081 | −1.11% | | 3 | 2 782 | 128 205 | 2 782 | 128 205 | 0.00% | | 4+ | 782 | 130 567 | 781 | 130 641 | −0.06% | | global | 10 350 | 140 794 | 10 140 | 141 715 | −0.65% | Вывод: фикс снимает ~80–85% ошибки H2, остаточное завышение коэффициента ≈0.6–1.2%. Это приемлемо (сильно лучше, чем ничего), но **H2 нельзя считать полностью закрытым** — оставить follow-up в #2583 до полного покрытия `listings.city`. Хорошая новость: колонка уже проставляется на новых прогонах (за 3 дня: cian 939/953, yandex 3002/3704, avito 1722/1722), так что остаток будет сам сходиться к нулю — «предикат сам ужесточается» из комментария подтверждается фактами. ### Дополнения к описанию эффекта на цены `estimator.py:2911-2919` клампит `effective_ratio > 1.0` до 1.0 (`ESTIMATE_EXPECTED_SOLD_LE_ASKING`, default True, на проде не переопределён). Поэтому фактический эффект на выкупную цену отличается от таблицы ratio: - bucket 0 (студии): 0.7322 → 0.7342 — цена вырастет на +0.28%, а не «~0%»; - bucket 1: −4.88%, bucket 2: −3.11%, bucket 3: −2.50%; - bucket 4+: 1.0500 и 1.0272 **оба клампятся до 1.0** — изменения цены НЕТ; - global -1 fallback: −3.01%. Итого клиентам: −2.5…−4.9% на 1/2/3-комнатных, ~+0.3% на студиях, 0% на 4+. ### Прочее (не блокирует) - `deals.city` не имеет NULL вообще (0 из 96 974) — асимметрия предикатов (deal-сторона строгая, ask-сторона NULL-толерантная) доказуемо безвредна. - `%Екатеринбург%` корректно матчит обе таблицы: в `deals` формы `Екатеринбург` / `город Екатеринбург` / `городской округ город Екатеринбург`, в `listings` ровно `Екатеринбург`; ложных срабатываний по остальным 5 значениям колонки нет. - Пороги 30/30: после фикса минимум n_listings = 657 (bucket 0), минимум n_deals = 1 476 — запас 20×+, ни один бакет не уезжает на global fallback. - ratio > 1 у 4+ комнат — не шум (n_deals=1 476, n_listings=782), а методическая асимметрия: у ask-стороны есть novostroyki-гард (#1186), у deal-стороны его нет и в `deals` вообще нет segment-маркера; плюс `deals.rooms` обрезан на 4 (max=4, >4 нет), а в listings в bucket 4 сваливаются 5–10-комнатные. Предсуществующее, в этом PR чинить не нужно, но стоит завести отдельно. - Единый ЕКБ-коэффициент по-прежнему применяется ко всем городам (#647). Реальные per-city ratio на проде: ЕКБ 0.823, Н. Тагил 0.822, Первоуральск 0.847, В. Пышма 0.692, Каменск-Уральский 0.921, Серов 0.920. Для Каменска и Серова фикс уводит применяемый коэффициент чуть дальше от истины, для остальных — ближе. Нетто выигрыш очевиден. - Этот PR превращает `listings.city` из write-only колонки (#2598) в money-critical чтение — любой будущий mis-stamp двигает цены напрямую. Гард на месте (`test_city_sweep.py::test_resolve_city_name_known_oblast_slugs`, set-equality по `CITY_LOCATIONS`), но это стоит помнить. Полный pytest воспроизведён локально: `1 failed, 3058 passed, 9 skipped` — единственный фейл `tests/test_search_api.py::test_search_cache_hit` (401), pre-existing. `ruff check` чист. Диff строго в границах: только `tasks/asking_to_sold_ratio.py` + его тесты; `estimator.py`, миграции, пороги и deal-сторона не тронуты.
lekss361 merged commit e484b5cce9 into main 2026-08-02 09:10:36 +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#2617
No description provided.