fix(tradein/matching): P1 extended — never create a house from a numberless address (no cadastral)
This commit is contained in:
parent
3b9dfdb44e
commit
0eb033d77d
5 changed files with 150 additions and 26 deletions
|
|
@ -38,7 +38,7 @@ def match_or_create_house(
|
|||
cadastral_number: str | None = None,
|
||||
house_fias_id: str | None = None,
|
||||
source_url: str | None = None,
|
||||
) -> tuple[int, float, str]:
|
||||
) -> tuple[int | None, float, str]:
|
||||
"""Match existing house or create new canonical record.
|
||||
|
||||
Concurrency-safe: serializes concurrent calls for the same address fingerprint
|
||||
|
|
@ -55,16 +55,26 @@ def match_or_create_house(
|
|||
Returns:
|
||||
(house_id, confidence ∈ [0.0, 1.0], method ∈ {
|
||||
'cadastr_exact', 'fias_exact', 'source_exact', 'fingerprint',
|
||||
'geo_proximity', 'new'
|
||||
'geo_proximity', 'new', 'no_house_number'
|
||||
})
|
||||
house_id is None only for the 'no_house_number' terminal case below.
|
||||
|
||||
Method values:
|
||||
'cadastr_exact' — matched by cadastral number (confidence 1.0)
|
||||
'fias_exact' — matched by house_fias_id (ГАР OBJECTGUID) (confidence 0.95)
|
||||
'source_exact' — already in house_sources for this source+ext_id (confidence 1.0)
|
||||
'fingerprint' — matched by address fingerprint (confidence 0.9)
|
||||
'geo_proximity' — matched by geo within 30 m (confidence 0.7)
|
||||
'new' — new house created (confidence 1.0)
|
||||
'cadastr_exact' — matched by cadastral number (confidence 1.0)
|
||||
'fias_exact' — matched by house_fias_id (ГАР OBJECTGUID) (confidence 0.95)
|
||||
'source_exact' — already in house_sources for this source+ext_id (confidence 1.0)
|
||||
'fingerprint' — matched by address fingerprint (confidence 0.9)
|
||||
'geo_proximity' — matched by geo within 30 m (confidence 0.7)
|
||||
'new' — new house created (confidence 1.0)
|
||||
'no_house_number' — REFUSED: numberless address with no cadastral number →
|
||||
house_id=None, confidence 0.0. No house/house_source/alias is written.
|
||||
P1 philosophy: a bare-street building-key (no house number) mass-mis-buckets
|
||||
active listings into one "house". The alias/Tier-2b guards already refuse to
|
||||
MATCH such a key (house 131237 collapsed 8 numbers); this closes the last hole
|
||||
where the New-house INSERT would still MINT one. On prod the missing gate spawned
|
||||
3 384 numberless mega-buckets ('Москва' 664 listings, 'Екатеринбург (Avito)' 587,
|
||||
'р-н Чкаловский, мкр. Вторчермет' 480). A cadastral number is a precise building
|
||||
identity, so cad-carrying rows stay exempt (Tier 0 owns them).
|
||||
"""
|
||||
cad = building_cadastral_number or cadastral_number
|
||||
|
||||
|
|
@ -265,6 +275,16 @@ def match_or_create_house(
|
|||
)
|
||||
return (house_id, 0.7, "geo_proximity")
|
||||
|
||||
# P1 (extended to INSERT-new): never CREATE a house from a numberless address that
|
||||
# carries no cadastral number. `has_num` is False for a bare-street normalized_address
|
||||
# (e.g. 'екатеринбург улица мамина сибиряка') AND for address=None. Such a building-key
|
||||
# is too ambiguous to mint a canonical row — it becomes a mega-bucket that every later
|
||||
# numberless listing geo-collapses into. Tier 0/aliases already refuse to MATCH it; here
|
||||
# we also refuse to CREATE it. A cadastral number (`cad`) is a precise identity → exempt.
|
||||
if not has_num and not cad:
|
||||
logger.info("house create skipped: numberless address %r src=%s", address, ext_source)
|
||||
return (None, 0.0, "no_house_number")
|
||||
|
||||
# New house — INSERT canonical record.
|
||||
# geom column is auto-populated by houses_set_geom_trg BEFORE INSERT trigger from lat/lon.
|
||||
# Do NOT include geom in the INSERT column list — trigger handles it.
|
||||
|
|
|
|||
|
|
@ -62,7 +62,9 @@ class RealMatcherAdapter:
|
|||
building_cadastral_number: str | None = None,
|
||||
cadastral_number: str | None = None,
|
||||
source_url: str | None = None,
|
||||
) -> tuple[int, float, str]:
|
||||
) -> tuple[int | None, float, str]:
|
||||
# house_id is None when the matcher refuses a numberless address without a
|
||||
# cadastral number (method 'no_house_number', P1). Callers must tolerate None.
|
||||
return _match_or_create_house(
|
||||
db,
|
||||
ext_source,
|
||||
|
|
|
|||
|
|
@ -919,10 +919,11 @@ async def fetch_house_catalog(
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def upsert_house(db: Session, h: HouseInfo) -> int:
|
||||
def upsert_house(db: Session, h: HouseInfo) -> int | None:
|
||||
"""Match-or-create канонический дом через match_or_create_house, затем обновить enrichment.
|
||||
|
||||
Source='avito'. Возвращает houses.id.
|
||||
Source='avito'. Возвращает houses.id, либо None если matcher отказал (P1:
|
||||
безномерный адрес без кадастра, method='no_house_number').
|
||||
Делегирует в _persist_house (без url — url пуст как раньше для этого пути).
|
||||
|
||||
Маппинг полей HouseInfo → DDL колонки (009 + 010):
|
||||
|
|
@ -937,15 +938,16 @@ def upsert_house(db: Session, h: HouseInfo) -> int:
|
|||
return _persist_house(db, h, "")
|
||||
|
||||
|
||||
def _upsert_house_with_url(db: Session, h: HouseInfo, house_url: str) -> int:
|
||||
def _upsert_house_with_url(db: Session, h: HouseInfo, house_url: str) -> int | None:
|
||||
"""Внутренний вариант upsert_house с правильным URL.
|
||||
|
||||
Делегирует в _persist_house — используй его вместо прямого вызова.
|
||||
Возвращает None если matcher отказал (P1 numberless без кадастра).
|
||||
"""
|
||||
return _persist_house(db, h, house_url)
|
||||
|
||||
|
||||
def _persist_house(db: Session, h: HouseInfo, house_url: str) -> int:
|
||||
def _persist_house(db: Session, h: HouseInfo, house_url: str) -> int | None:
|
||||
"""Канонический путь сохранения дома-avito через match_or_create_house.
|
||||
|
||||
Шаги:
|
||||
|
|
@ -974,6 +976,17 @@ def _persist_house(db: Session, h: HouseInfo, house_url: str) -> int:
|
|||
source_url=house_url or None,
|
||||
)
|
||||
|
||||
# P1: matcher refuses a numberless address without a cadastral number
|
||||
# (method 'no_house_number' → house_id None). No canonical row exists to enrich,
|
||||
# so skip the UPDATE and signal the graceful skip up to save_house_catalog_enrichment.
|
||||
if house_id is None:
|
||||
logger.info(
|
||||
"_persist_house: skip numberless avito house ext_id=%s addr=%r (no cadastral)",
|
||||
h.ext_id,
|
||||
h.short_address or h.full_address,
|
||||
)
|
||||
return None
|
||||
|
||||
db.execute(
|
||||
text("""
|
||||
UPDATE houses SET
|
||||
|
|
@ -1275,6 +1288,25 @@ def save_house_catalog_enrichment(
|
|||
}
|
||||
house_id = _upsert_house_with_url(db, e.house, e.house_url)
|
||||
|
||||
# P1: matcher refused to create a canonical house for a numberless address without a
|
||||
# cadastral number (house_id None). Downstream saves (reviews / placement history) are
|
||||
# house-scoped and meaningless with a NULL house_id, so graceful-skip the whole
|
||||
# enrichment — same zero-counter contract as the id-less guard above.
|
||||
if house_id is None:
|
||||
logger.warning(
|
||||
"save_house_catalog_enrichment: пропуск дома с безномерным адресом без кадастра"
|
||||
" (ext_id=%s addr=%r) — matcher вернул no_house_number",
|
||||
e.house.ext_id,
|
||||
e.house.short_address or e.house.full_address,
|
||||
)
|
||||
return {
|
||||
"house_id": 0,
|
||||
"reviews": 0,
|
||||
"sellers": 0,
|
||||
"listings_linked": 0,
|
||||
"placement_history": 0,
|
||||
}
|
||||
|
||||
# 2. Сохранить отзывы
|
||||
reviews_count = save_house_reviews(db, house_id, e.reviews)
|
||||
|
||||
|
|
|
|||
|
|
@ -494,12 +494,14 @@ def test_geo_match_rejected_when_house_number_differs():
|
|||
assert house_id == 500
|
||||
|
||||
|
||||
def test_bare_street_listing_skips_tier2b_and_does_not_alias():
|
||||
"""P1: a numberless ('bare street') address must not match or register an alias.
|
||||
def test_bare_street_numberless_no_house_created():
|
||||
"""P1 (extended to INSERT-new): a numberless address with no cadastral → NO house.
|
||||
|
||||
'екатеринбург улица мамина сибиряка' has no house number → Tier 2b is skipped
|
||||
and _insert_alias is a no-op, so it cannot mass-collapse into a wrong house.
|
||||
Execute order: lock, source miss, fp miss, INSERT, upsert (no Tier 2b, no alias).
|
||||
'екатеринбург улица мамина сибиряка' has no house number and no cadastral number,
|
||||
so match_or_create_house refuses to MINT a canonical row (which becomes a mega-bucket
|
||||
that later numberless listings collapse into). Returns (None, 0.0, 'no_house_number')
|
||||
and writes nothing: no Tier 2b lookup, no INSERT INTO houses, no alias, no house_sources.
|
||||
Execute order: lock, source miss, fp miss — then the gate returns.
|
||||
"""
|
||||
from app.services.matching.houses import match_or_create_house
|
||||
|
||||
|
|
@ -508,22 +510,86 @@ def test_bare_street_listing_skips_tier2b_and_does_not_alias():
|
|||
None, # pg_advisory_xact_lock
|
||||
None, # house_sources miss
|
||||
None, # fingerprint miss (Tier 2a)
|
||||
{"id": 700}, # INSERT RETURNING id (New house — Tier 2b skipped, no lat/lon)
|
||||
None, # _upsert_house_source
|
||||
]
|
||||
)
|
||||
house_id, _conf, method = match_or_create_house(
|
||||
house_id, conf, method = match_or_create_house(
|
||||
db, "yandex", "ext-bare", address="екатеринбург улица мамина сибиряка"
|
||||
)
|
||||
assert method == "new"
|
||||
assert house_id == 700
|
||||
assert (house_id, conf, method) == (None, 0.0, "no_house_number")
|
||||
sqls = _executed_sqls(db)
|
||||
assert not any(
|
||||
"normalized_address = :na" in s for s in sqls
|
||||
), "bare-street address must not run Tier 2b normalized_address lookup (P1)"
|
||||
assert not any(
|
||||
"INSERT INTO houses" in s for s in sqls
|
||||
), "numberless address must not create a house (P1 extended)"
|
||||
assert not any(
|
||||
"INSERT INTO house_address_aliases" in s for s in sqls
|
||||
), "bare-street address must not register an alias (P1)"
|
||||
), "numberless address must not register an alias (P1)"
|
||||
assert not any(
|
||||
"INSERT INTO house_sources" in s for s in sqls
|
||||
), "numberless refusal must not upsert house_sources"
|
||||
|
||||
|
||||
def test_numberless_none_address_with_coords_no_house_created():
|
||||
"""P1 edge: address=None but lat/lon present → still gated (no address-NULL house).
|
||||
|
||||
Previously the New-house INSERT would mint a house with address NULL keyed only by
|
||||
coords — exactly the mis-bucket the gate refuses. has_num is False for an empty
|
||||
normalized address, so with no cadastral the flow returns 'no_house_number'.
|
||||
Execute order: lock, source miss, fp miss, geo miss — then the gate returns.
|
||||
"""
|
||||
from app.services.matching.houses import match_or_create_house
|
||||
|
||||
db = _make_db(
|
||||
[
|
||||
None, # pg_advisory_xact_lock
|
||||
None, # house_sources miss
|
||||
None, # fingerprint miss (Tier 2a); Tier 2b skipped (empty norm_addr)
|
||||
None, # geo miss (Tier 3 — lat/lon present)
|
||||
]
|
||||
)
|
||||
house_id, conf, method = match_or_create_house(
|
||||
db, "yandex", "ext-none-addr", address=None, lat=56.83, lon=60.59
|
||||
)
|
||||
assert (house_id, conf, method) == (None, 0.0, "no_house_number")
|
||||
assert not any(
|
||||
"INSERT INTO houses" in s for s in _executed_sqls(db)
|
||||
), "address-NULL coords-only listing must not create a house (P1 extended)"
|
||||
|
||||
|
||||
def test_numberless_address_with_cadastral_creates_house():
|
||||
"""P1 exemption: a numberless address WITH a cadastral number still creates a house.
|
||||
|
||||
The cadastral number is a precise building identity, so the numberless gate does not
|
||||
apply. Tier 0 cadastr lookup misses (new cad), remaining tiers miss, and the New-house
|
||||
INSERT proceeds because `cad` is truthy.
|
||||
Execute order: lock, cadastr miss, source miss, fp miss, INSERT, upsert
|
||||
(_insert_alias is a no-op — bare-street normalized_address has no number).
|
||||
"""
|
||||
from app.services.matching.houses import match_or_create_house
|
||||
|
||||
db = _make_db(
|
||||
[
|
||||
None, # pg_advisory_xact_lock
|
||||
None, # Tier 0 cadastr miss
|
||||
None, # house_sources miss
|
||||
None, # fingerprint miss (Tier 2a; no lat/lon → no Tier 2b/geo)
|
||||
{"id": 321}, # INSERT RETURNING id (New house — cad exempts the gate)
|
||||
None, # _upsert_house_source
|
||||
]
|
||||
)
|
||||
house_id, conf, method = match_or_create_house(
|
||||
db,
|
||||
"cian",
|
||||
"ext-cad",
|
||||
address="екатеринбург улица мамина сибиряка",
|
||||
cadastral_number="66:41:0000000:12345",
|
||||
)
|
||||
assert (house_id, conf, method) == (321, 1.0, "new")
|
||||
assert any(
|
||||
"INSERT INTO houses" in s for s in _executed_sqls(db)
|
||||
), "numberless + cadastral must still create a house (cadastral = identity)"
|
||||
|
||||
|
||||
def test_insert_alias_noop_for_bare_street():
|
||||
|
|
|
|||
|
|
@ -50,13 +50,17 @@ class HouseMatcher(Protocol):
|
|||
building_cadastral_number: str | None = ...,
|
||||
cadastral_number: str | None = ...,
|
||||
source_url: str | None = ...,
|
||||
) -> tuple[int, float, str]:
|
||||
) -> tuple[int | None, float, str]:
|
||||
"""Найти или создать канонический дом.
|
||||
|
||||
Returns:
|
||||
(house_id, confidence ∈ [0.0, 1.0], method), где method ∈ {
|
||||
'cadastr_exact', 'source_exact', 'fingerprint', 'geo_proximity', 'new'
|
||||
'cadastr_exact', 'source_exact', 'fingerprint', 'geo_proximity', 'new',
|
||||
'no_house_number'
|
||||
}.
|
||||
house_id=None только при method='no_house_number' — matcher отказался
|
||||
создавать дом из безномерного адреса без кадастра (P1). Вызывающий обязан
|
||||
обработать None.
|
||||
"""
|
||||
...
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue