feat(tradein-estimator): unique-address-based confidence threshold #513

Merged

lekss361 merged 1 commit from feat/tradein-confidence-unique-addresses into main

2026-05-24 11:52:00 +00:00

lekss361 commented

2026-05-24 11:46:34 +00:00

Owner

Problem

In _compute_confidence thresholds were on raw n_analogs. With MIN_ANALOGS_PER_SOURCE=5 quota + same-building lots, n=10 could come from just 2-3 unique buildings → confidence=high was overstated.

Fix

high: unique_addr ≥ 7 AND iqr_pct < 15%
medium: unique_addr ≥ 4 OR (unique_addr ≥ 2 AND iqr_pct < 25%)
downgrade one step if avg_lots_per_addr > 2.5 (anti-concentration bias)

scripts/probe-confidence-distribution.py to tune p75 threshold from real data.

Roadmap

PR 4 of 8 — [[Decision_TradeIn_DataQuality_8PR_Roadmap]]. Lands independently of PR #507; if order differs at merge time, one will need rebase (own-branch, --force-with-lease).

Verify

cd /opt/gendesign/tradein-mvp/backend
python scripts/probe-confidence-distribution.py 100

## Problem In `_compute_confidence` thresholds were on raw `n_analogs`. With `MIN_ANALOGS_PER_SOURCE=5` quota + same-building lots, `n=10` could come from just 2-3 unique buildings → `confidence=high` was overstated. ## Fix - **high**: `unique_addr ≥ 7 AND iqr_pct < 15%` - **medium**: `unique_addr ≥ 4` OR (`unique_addr ≥ 2 AND iqr_pct < 25%`) - **downgrade one step** if `avg_lots_per_addr > 2.5` (anti-concentration bias) + `scripts/probe-confidence-distribution.py` to tune p75 threshold from real data. ## Roadmap PR 4 of 8 — `[[Decision_TradeIn_DataQuality_8PR_Roadmap]]`. Lands independently of PR #507; if order differs at merge time, one will need rebase (own-branch, --force-with-lease). ## Verify ```bash cd /opt/gendesign/tradein-mvp/backend python scripts/probe-confidence-distribution.py 100 ```

lekss361 added 1 commit 2026-05-24 11:46:35 +00:00

feat(tradein-estimator): unique-address-based confidence threshold 0eac769db2

Previous confidence used raw n_analogs which inflated to 'high' when
MIN_ANALOGS_PER_SOURCE quota + same-building lots concentrated count on
few unique addresses.

New logic:
- high: unique_addr >= 7 AND iqr_pct < 15%
- medium: unique_addr >= 4 OR (unique_addr >= 2 AND iqr_pct < 25%)
- downgrade one step if avg_lots_per_addr > 2.5 (anti-concentration bias)

probe-confidence-distribution.py added for tuning p75 threshold on real data.

lekss361 commented

2026-05-24 11:51:53 +00:00

Author

Owner

Deep Code Review — PR #513

Status: APPROVE
Files: 2 (estimator.py P0 logic · probe script P3 ops tool) · +122/-16
Roadmap: PR 4/8 of Decision_TradeIn_DataQuality_8PR_Roadmap — aligned.

Correctness — sound

unique_addrs = {(lot.get('address') or '').strip().lower() for lot in listings if lot.get('address')} — корректно нормализует case + whitespace, отбрасывает пустые/None addresses.
avg_lots_per_addr = n_analogs / max(unique_addr_count, 1) — нет division by zero.
Тиры consistent с roadmap: high=unique≥7 ∧ iqr<15%, medium=unique≥4 ∨ (unique≥2 ∧ iqr<25%), low — иначе. Downgrade при avg_lots_per_addr > 2.5 маппится high→medium, medium→low, low не трогается.
listings=None/[] fallback (unique_addr_count = n_analogs, avg=1.0) сохраняет обратную совместимость для любых не-estimate_quality callers.
median_ppm2==0 shortcut в начале функции остаётся — пустой listings_clean отрабатывает прежним путём, не доходя до новой логики.
Edge cases:
- n=10, 1 уникальный адрес → base=low, downgrade не применяется (уже low) — correct.
- n=12, 3 адреса по 4 лота, iqr<25% → base=medium → downgrade=low — correct.
- n=14, 7 адресов по 2, iqr<15% → base=high, avg=2.0≤2.5 → high — correct.

Cross-PR compatibility

PR #507 (tiered house-match): оба PR трогают call к _compute_confidence(...) (строки 565-568 base) — будет реальный merge conflict, автор это явно оговорил в body (own-branch rebase). Функционально совместимы: tier_note из #507 присоединяется поверх explanation из #513 (explanation + tier_note + repair_note).
PR #504 (rosreestr exclusion): не пересекается — deals path не трогается.
PR #509 (atomic IMV link): independent code path, no overlap.

Performance

O(n) set construction over listings_clean (≤50 cap из _fetch_analogs). Negligible.

Security

Нет SQL injection / PII leak / auth изменений. Address content идёт в Python set — безопасно.

Tests

Unit-тесты для _compute_confidence не добавлены. В baseline их тоже не было, поэтому это not a regression. Roadmap acceptance (для тестовых estimate'ов с 1-2 уникальными адресами но n=10 — confidence снижается до medium/low) можно покрыть в follow-up — функция pure, легко тестируется.
probe-confidence-distribution.py — ops tool, не unit test. quantiles(n=4)[2]==p75 корректно. Redundant inline p75_idx calc — minor.

Conventions

ruff line-100 — нарушений нет.
Russian docstring consistent с file style.
psycopg v3: probe-script использует SQLAlchemy text() + named params — OK.

Nitpick (Low — не блокирует)

При n_analogs=1: новый текст "Найдено 1 аналогов из 1 разных адресов" — грамматически некорректно (раньше было "Только 1 аналог" с правильным согласованием). Стоит вернуть склонение либо использовать форму с числительным: f"Найдено {n_analogs} {_plural_analogs(n_analogs)} из {unique_addr_count} {_plural_addr(unique_addr_count)}".

Positive

Чёткая проблема + чёткое решение, root cause анализ в docstring.
Probe-script для эмпирической калибровки threshold — правильный подход вместо магических чисел.
Backward compat сохранена через listings: ... | None = None default.
Concentration bias guard (downgrade при >2.5 lots/addr) — продуманная защита.

Verdict

Logic корректна, edge cases закрыты, backward-compatible, roadmap-aligned. Грамматическая мелочь не блокирует. Approving and merging.

## Deep Code Review — PR #513 **Status:** APPROVE **Files:** 2 (estimator.py P0 logic · probe script P3 ops tool) · +122/-16 **Roadmap:** PR 4/8 of [[Decision_TradeIn_DataQuality_8PR_Roadmap]] — aligned. ### Correctness — sound - `unique_addrs = {(lot.get('address') or '').strip().lower() for lot in listings if lot.get('address')}` — корректно нормализует case + whitespace, отбрасывает пустые/None addresses. - `avg_lots_per_addr = n_analogs / max(unique_addr_count, 1)` — нет division by zero. - Тиры consistent с roadmap: high=`unique≥7 ∧ iqr<15%`, medium=`unique≥4 ∨ (unique≥2 ∧ iqr<25%)`, low — иначе. Downgrade при `avg_lots_per_addr > 2.5` маппится `high→medium`, `medium→low`, low не трогается. - `listings=None`/`[]` fallback (`unique_addr_count = n_analogs, avg=1.0`) сохраняет обратную совместимость для любых не-`estimate_quality` callers. - `median_ppm2==0` shortcut в начале функции остаётся — пустой `listings_clean` отрабатывает прежним путём, не доходя до новой логики. - Edge cases: - `n=10, 1 уникальный адрес` → base=low, downgrade не применяется (уже low) — correct. - `n=12, 3 адреса по 4 лота, iqr<25%` → base=medium → downgrade=low — correct. - `n=14, 7 адресов по 2, iqr<15%` → base=high, avg=2.0≤2.5 → high — correct. ### Cross-PR compatibility - **PR #507 (tiered house-match):** оба PR трогают call к `_compute_confidence(...)` (строки 565-568 base) — будет реальный merge conflict, автор это явно оговорил в body (own-branch rebase). Функционально совместимы: tier_note из #507 присоединяется поверх explanation из #513 (`explanation + tier_note + repair_note`). - **PR #504 (rosreestr exclusion):** не пересекается — deals path не трогается. - **PR #509 (atomic IMV link):** independent code path, no overlap. ### Performance - O(n) set construction over `listings_clean` (≤50 cap из `_fetch_analogs`). Negligible. ### Security - Нет SQL injection / PII leak / auth изменений. Address content идёт в Python set — безопасно. ### Tests - Unit-тесты для `_compute_confidence` не добавлены. В baseline их тоже не было, поэтому это not a regression. Roadmap acceptance (`для тестовых estimate'ов с 1-2 уникальными адресами но n=10 — confidence снижается до medium/low`) можно покрыть в follow-up — функция pure, легко тестируется. - `probe-confidence-distribution.py` — ops tool, не unit test. `quantiles(n=4)[2]==p75` корректно. Redundant inline `p75_idx` calc — minor. ### Conventions - ruff line-100 — нарушений нет. - Russian docstring consistent с file style. - psycopg v3: probe-script использует SQLAlchemy `text()` + named params — OK. ### Nitpick (Low — не блокирует) - При `n_analogs=1`: новый текст `"Найдено 1 аналогов из 1 разных адресов"` — грамматически некорректно (раньше было `"Только 1 аналог"` с правильным согласованием). Стоит вернуть склонение либо использовать форму с числительным: `f"Найдено {n_analogs} {_plural_analogs(n_analogs)} из {unique_addr_count} {_plural_addr(unique_addr_count)}"`. ### Positive - Чёткая проблема + чёткое решение, root cause анализ в docstring. - Probe-script для эмпирической калибровки threshold — правильный подход вместо магических чисел. - Backward compat сохранена через `listings: ... | None = None` default. - Concentration bias guard (downgrade при >2.5 lots/addr) — продуманная защита. ### Verdict Logic корректна, edge cases закрыты, backward-compatible, roadmap-aligned. Грамматическая мелочь не блокирует. **Approving and merging.**