feat(tradein-estimator): tiered house-match S→H→W #507
No reviewers
Labels
No labels
admin
analytics
auth
automation
bug
business
chore
ci
compliance
data
data-moat
docs
duplicate
dx
enhancement
Fable 5 ревью
feedback/max
generative
GG-форсайт
needs-discussion
needs-human
observability
pause-bots
performance
priority/p0
priority/p1
priority/p2
priority/p3
scope/backend
scope/db
scope/devops
scope/frontend
scope/qa
scrapers
security
site-finder
stage/1
stage/2
status/blocked
status/done
status/needs-analysis
status/needs-fix
status/qa
status/ready
status/review
status/wip
tech-debt
tradein
ux
week ревью 1
wontfix
вторичка
ИРД
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: lekss361/gendesign#507
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "feat/tradein-house-match-tiered"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
Estimate
a0a0b820-e8a8-4eee-aa73-0ab3b98ac233(17-эт. 1975) returned 10 analogs from 5-9 эт. брежневок because year/house_type were only inrelevance_score(sort), not WHERE.Fix
Three tiers in
_fetch_analogs():address ILIKE prefix, ≥3 → return onlyyear ±15, total_floors ±30%, ≥5 → returnconfidence_explanationcarries the tier label so user sees the trade-off.Roadmap
PR 3 of 8 —
[[Decision_TradeIn_DataQuality_8PR_Roadmap]].Deep Code Review — PR #507 (tiered house-match S→H→W)
Summary
tradein-mvp/backend/app/services/estimator.py)_fetch_analogs[[Decision_TradeIn_DataQuality_8PR_Roadmap]](PR 3 of 8)_fetch_dealsне вызывается изestimate_quality()🟠 High —
_extract_short_addrmishandles 4 из 6 типичных address-форматов (включая target UUID)Воспроизведено против адреса target UUID
Свердловская область, г. Екатеринбург, Склад, ул. Заводская, д. 44-а(из roadmap):Root causes в
estimator.py:825:admin_re = r'^(г\.?|обл\.?|р-н\.?|пгт\.?|с\.?|д\.?|мкр\.?)\s'требует trailing space после аббревиатуры.'Свердловская обл'(без точки в конце) и'Россия'(без аббр.) никогда не стрипаются → попадают вstreet.'д.'интерпретируется как «деревня», но также collides с'д.'= «дом». Номер домад. 44-апоэтому дропается admin-фильтром вместо того чтобы попасть вhouse_raw. Для target UUID токен дома потерян.'кв. N'/'корп. N'НЕ вadmin_re— для адресов, где они идут перед street, протекают какmeaningful.Demo против 6 типичных входов:
Impact: для таких inputs Tier S делает
address ILIKE 'Свердловская область, Склад%'→ ~0 results → fallthrough в Tier H. Net behavior не хуже базы (Tier H+W фильтруют поyear_built ±15иtotal_floors ±30%в WHERE), так что заявленная цель PR (исключить 5-9 эт. брежневки из выборки 17-эт.) всё равно достигается через Tier H. Но Tier S — dead code для половины реальных geocode outputs.Рекомендации:
r'^(г|обл(?:асть)?|р-н|пгт|с|мкр)\.?\s*$|^(г|обл|р-н|пгт|мкр)\.?\s'+ явный handling для'Россия'/'РФ'.'обл' in s,'Россия' in s) вместо строгой regex.houses.idиспользуяlistings.house_id_fk(real FK column, см.data/sql/011_listings_alter.sql:25). Tier S становитсяWHERE house_id_fk = :target_house_fk— без regex.pytestдля_extract_short_addrпокрывающий 6 форм выше + empty + None.🟡 Medium — Tier S
relevance_scoreхардкод0.0ломаетORDER BYи_stratify_candidatesre-sortВ Tier S CTE (
estimator.py:962-985):Outer
ORDER BY relevance_score— tie на0.0→ undefined intra-tier order._stratify_candidatesвызываетresult.sort(key=lambda r: r.get('relevance_score') or 0.0)— тоже no-op для Tier S. Результат недетерминирован между запросами для того же target/listings set.Fix: для Tier S сортировать по
scraped_at DESC(freshness) в outer SELECT, илиrelevance_score = -EXTRACT(EPOCH FROM scraped_at)так чтобы newer выигрывал.🟡 Medium — Zero tests для 312 строк парсинга + SQL
Никаких updates в
tradein-mvp/backend/tests/._extract_short_addr— pure-function string parsing — тривиально юнит-тестировать. Roadmap acceptance ("для target UUID — analogs исключить 5-9 эт. дома") — только manual curl. Минимум:tests/services/test_extract_short_addr.pyс 6 формами вышеtier='H'возвращается при year/total_floors present и≥5совпадений🟢 Low —
ext_house_idпараметр ссылается на колонку, которой нет наlistingsНовый kwarg
ext_house_id: str | None = Noneв_fetch_analogs:901. SQL clause:"ext_house_id IS NOT NULL AND ext_house_id = :ext_house_id". Ноext_house_idсуществует только на таблицеhouses(data/sql/009_houses.sql:17), никогда наlistings. Grep подтверждает.Dead code сегодня (
estimate_qualityне передаёт kwarg →has_ext_idвсегда False → SQL branch не запускается). Но это footgun: будущий caller добавляющийext_house_id=...поймаетUndefinedColumnв runtime, не в PR time. Варианты:short_addr), либоhouse_id_fkJOIN кhouses(ext_house_id)(preferred — заодно закрывает parser-баг выше).🟢 Low — Worst-case 9 SQL запросов на estimate (было 3)
Outer loop вызывает
_fetch_analogsдо 3× (radius escalation × area_tolerance escalation). Каждый call теперь до 3 SQL queries (Tier S → H → W). Tier S short-circuit'ит при ≥3 same-house hits, так что realistic average 1-3 queries. PostGIS GIST +listings(rooms, scraped_at)индексы должны держать, но стоит мониторить p95 латентности/estimateпосле deploy.🟢 Low —
fallback_usedsemantics ambiguous для Tier Sreturn _stratify_candidates(tier_s), radius_m > DEFAULT_RADIUS_M, 'S'—fallback_used=Trueкогда Tier S engagе'ится на 2-м outer call (2km radius), даже хотя Tier S — самый точный результат. UI message может показать «расширили поиск» рядом с «аналоги из того же дома» — противоречиво. Косметика, но user-facing.Cross-file impact
tradein-mvp/backend/app/schemas/trade_in.py—AggregatedEstimate.confidence_explanation— plain str, нет schema change. ✅tradein-mvp/backend/app/api/v1/trade_in.py— pure callerestimate_quality, сигнатура не изменена. ✅frontend/src/types/trade-in.ts— нет contract change (tier_note дописывается к existingconfidence_explanation). ✅data/sql/**— нет миграции; полагается на existinglistings.address,year_built,total_floors,geom. ✅Vault cross-check
[[Decision_TradeIn_DataQuality_8PR_Roadmap]]PR 3 plan: ✅ совпадает по scope (S → H → W тиры,≥3/≥5пороги, fallthrough). Tier-note вconfidence_explanationсоответствует «расширили поиск» intent.total_floors BETWEEN 12 AND 22(17 × 0.7 / 1.3) исключает 5-9 эт. напрямую в WHERE.Positive
_fetch_dealscall site).analog_tierpropagation черезif len < 5/if len < 3outer-loop branches корректна (всегда апдейтится к wider-tier result).tier_noteuser-facing message соответствует PR description.CAST(:x AS type)PostgreSQL conventions соблюдены (Tier H SQL имеет explicit CAST для:tf_min,:tf_max,:year_min,:year_max,:target_year,:target_house_type).text()+ named params — no SQL injection vectors.MAX_ANALOGS_PER_ADDRESSper-tier cap сохранён.Recommended next steps
_extract_short_addr(или заменить наhouse_id_fkJOIN) + pytest coverage. Без этого Tier S engagе'ится для <50% реальных geocode outputs.ORDER BYна scraped_at DESC для детерминированной сортировки.ext_house_idkwarg (dead code ссылающийся на несуществующую колонку).a0a0b820-e8a8-4eee-aa73-0ab3b98ac233per Roadmap regression-сверка, иgreplogs наanalogs tier=distribution за первые 100 estimates.Complexity / blast radius
7da58ed4a7toa03fad9b68Re-review PR #507 — fixup a03fad9 (delta from 7da58ed/d4194da)
Delta scope
a03fad9= pure ruff lint fixes (RUF046int(round(...))→round(...),E501line-length wraps на_compute_confidenceи_listing_to_analog). Затрагивает строки 553, 1335-1368 — полностью вне нового Tier S/H/W кода.Statuses of 4 prior issues
_extract_short_addradmin_re требует trailing space, дропает 'д. NN', не ловит 'Свердловская область'/'Россия'estimator.py:875-880relevance_score = 0.0hardcoded → недетерминированный sortestimator.py:1016ext_house_idkwarg ссылается на несуществующую колонкуlistings.ext_house_idestimator.py:951, 996, 1001, 1007estimator.pyв filesVerdict: COMMENT (unchanged)
Prior review был non-blocking COMMENT — primary roadmap goal (исключить 5-9 эт. брежневки из 17-эт. estimate) достигается через Tier H WHERE-clause даже при сломанном Tier S parser. Tier S фактически dead для ~50% real address formats, но не регрессит base behavior.
Lint fixup корректный (no functional change в production paths), но ни один из 4 deferred items не адресован. Поскольку issues были изначально non-blocking, статус остаётся COMMENT, не BLOCK. Автор может мержить и пофиксить в follow-up, либо amend'ить с parser fix + tests до merge.
Recommended follow-up PR (если merge сейчас)
admin_reна token-end + явный handling для 'Россия'/'РФ' (или replace наhouse_id_fkJOIN — eliminates regex entirely).ORDER BY scraped_at DESC(детерминированная сортировка).ext_house_idkwarg + SQL clause (dead code referencing non-existent column).tests/services/test_extract_short_addr.pyс 6 типичными форматами из prior review.Bot action
Verdict=comment → no auto-merge. Author decides: merge as-is (deferred fixes via follow-up PR) или push amend с item 1+4.
Re-review PR #507 — fixup e9d9db3 (delta from a03fad9)
Delta scope
e9d9db3= pure merge commit offorgejo/maininto branch. Brought in PRs #508 (feat(tradein-scrapers): parse listing_date from Avito + Yandex) и #509 (fix(tradein-estimator): atomic IMV link, batched yandex history, drop redundant CAST). Zero changes к Tier S/H/W parser code, к_extract_short_addr, к_fetch_analogs.New lines в
estimator.py(+10 net) — это batched yandex history insert (PR #509) на ~line 397-450, полностью unrelated к tiered house-match scope этого PR.Statuses of 4 prior issues (unchanged since 7da58ed/a03fad9)
_extract_short_addradmin_re требует trailing space, дропает 'д. NN', не ловит 'Свердловская область'/'Россия'estimator.py:885-888relevance_score = 0.0hardcoded → недетерминированныйORDER BY relevance_scoreestimator.py:1026, 1040ext_house_idkwarg ссылается на несуществующую колонкуlistings.ext_house_idestimator.py:961, 998, 1006, 1011, 1017estimator.pytouchedVerdict: COMMENT (unchanged)
Merge commit clean (no conflicts, mergeable=true). PR не регрессирует — все prior issues были изначально non-blocking (primary roadmap goal — исключить 5-9 эт. брежневки из 17-эт. estimate — достигается через Tier H WHERE-clause даже при сломанном Tier S parser). Tier S остаётся dead code для ~50% real address formats, но fallthrough в Tier H работает корректно.
Bot не merge'ит автоматически (verdict=comment). Автор decides: merge as-is с deferred follow-up PR на parser fix + tests, либо push amend с item 1+4 до merge.
Recommended follow-up PR (если merge сейчас)
admin_reна token-end + явный handling для 'Россия'/'РФ' (или replace наhouse_id_fkJOIN — eliminates regex entirely).ORDER BY scraped_at DESC(детерминированная сортировка вместо ORDER BY константы).ext_house_idkwarg + SQL clause (dead code — column not inlistings).tests/services/test_extract_short_addr.pyс 6 типичными форматами из prior review.e9d9db3a1btofa43ea8dc8Re-review PR #507 — fixup
fa43ea8(delta from e9d9db3)Delta scope
fa43ea8= тот же contents чтоa03fad9(ruff lint fixes RUF046 + E501 wraps в_compute_confidenceи_listing_to_analog). Branch был rebased — merge commite9d9db3исчез из history (parent offa43ea8=c4f1978e, single parent). Comparee9d9db3...fa43ea8показывает 11 commits forward, но ни один из них не модифицирует Tier S parser /_extract_short_addr/_fetch_analogsSQL.Statuses of 4 prior issues (unchanged since 7da58ed / a03fad9 / e9d9db3)
fa43ea8_extract_short_addradmin_reтребует trailing space, дропает «д. NN», не ловит «Свердловская область» / «Россия»estimator.py:886-889relevance_score = 0.0hardcoded → undefined intra-tier order вORDER BY relevance_scoreestimator.py:1027, 1041ext_house_idkwarg ссылается на несуществующую колонкуlistings.ext_house_id(есть только наhouses)estimator.py:962, 1007, 1012, 1018estimator.pyв files (+313 / −55)Verdict: COMMENT (unchanged, non-blocking)
Prior reviews были изначально non-blocking COMMENT — primary roadmap goal (исключить 5-9 эт. брежневки из 17-эт. estimate) достигается через Tier H WHERE-clause (
year_built BETWEEN :year_min AND :year_max+total_floors BETWEEN :tf_min AND :tf_max) даже при сломанном Tier S parser. Tier S остаётся dead code для ~50% реальных address formats (включая target UUID «Свердловская область, г. Екатеринбург, Склад, ул. Заводская, д. 44-а»), но fallthrough в Tier H работает корректно — net behavior не хуже базы.Lint fixup корректный (no functional change в production paths), но ни один из 4 deferred items не адресован за три фиксапа (7da58ed → a03fad9 → e9d9db3 →
fa43ea8). Bot не merge'ит автоматически (verdict=comment).Recommended follow-up PR (если merge сейчас)
ORDER BY scraped_at DESC(детерминированная сортировка вместо ORDER BY константы).ext_house_idkwarg + SQL clause (dead code referencing non-existentlistings.ext_house_id).tests/services/test_extract_short_addr.pyс 6 типичными форматами из prior review (7da58ed).Re-review PR #507 — fixup
8e0e80c(delta fromfa43ea8)Delta scope
8e0e80c= substantive fixup адресующий 3 из 4 prior issues + новый тест-файл (49 lines).estimator.py+313 / −55,tests/test_extract_short_addr.py+49 (new).Statuses of 4 prior issues
8e0e80c_extract_short_addradmin_re требует trailing space, дропает «д. NN», не ловит «Свердловская область»/«Россия»admin_reудалён, заменён на 3 module-level compiled regex'а (_TRAILING_NOISE_RE,_STREET_START_RE,_ADMIN_PREFIX_RE). Алгоритм: 1) strip trailing кв./корп./оф., 2) найти первый street-keyword, вернуть с него, 3) fallback — strip admin prefix. Я прогнал regex против 9 кейсов offline (включая target UUID «Свердловская область, г. Екатеринбург, Склад, ул. Заводская, д. 44-а» →ул. Заводская, д. 44-а✓): 5 docstring-кейсов PASS, edge case без street-keyword отдаёт fallback (acceptable). Target UUID работает корректно.estimator.py:864-921relevance_score = 0.0hardcoded → недетерминированныйORDER BY relevance_scoreORDER BY scraped_at DESC(детерминированный), а Pythonresult.sort(...)стабилен → порядок из SQL сохраняется.estimator.py:1032-1046ext_house_idkwarg ссылается на несуществующую колонкуlistings.ext_house_idext_house_idудалены из SQL/kwargs. Остался только TODO-comment в docstring на line 980 («когда listings получит колонку house_id_fk — добавить JOIN»). Никакого dead-code SQL clause.tests/test_extract_short_addr.py(+49 lines, 7 тестов): target UUID full chain, Россия prefix, apt stripped, проспект keyword, korp stripped, fallback на не-street, None passthrough. SQL paths (Tier S/H/W) остались без unit-test'ов, но это уже за scope «312 строк парсинга».Sanity checks
:nullable_param IS NOT NULLобёрнуты вCAST(:target_house_type AS text) IS NOT NULL. Соответствует prior-incident rule из памяти (PR #509→#518, 2026-05-24). Не дропать.year_built BETWEEN ... AND total_floors BETWEEN ...исключает 5-9 эт. брежневки из 17-эт. estimate (primary roadmap goal)._stratify_candidatesextracted в standalone func — DRY, reusable между tiers._listing_to_analogубралсяisinstance(..., list)guard уphoto_url— semantics эквивалентны при правильном upstream typing, но менее defensive. Non-blocker.'36'вместо'Ленина, 36'— admin fallback strips слишком агрессивно. Real-world impact низкий (Avito/Yandex обычно дают полную форму «ул. Ленина»). Non-blocker; можно добавить keyword «дом N» в_STREET_START_REfollow-up'ом.Verdict: APPROVE
Все 4 prior issues адресованы; target UUID parser работает; CAST trap правильно сохранён; tests добавлены; нет регрессий. PR ready to merge.
Vault cross-check
[[Decision_TradeIn_DataQuality_8PR_Roadmap]]PR 3 of 8 — completed end-to-end.[[feedback_cast_in_is_not_null_predicates]]rule applied correctly (CAST kept in Tier H predicates).