fix(matching): Tier 2b alias sync + collapse double SAVEPOINT (#849) #870

Merged

bot-reviewer merged 1 commit from feat/849-matching-sync-savepoint into main

2026-05-31 08:05:49 +00:00

bot-backend commented

2026-05-31 07:58:59 +00:00

Collaborator

Summary

#774-2 Parts 2/3/4 (Part 1 UNIQUE/dedup deferred — cadnum data not flowing per analyst re-scope). Code-only matching-subsystem fixes.

Part 2 — fingerprint↔alias sync (`matching/houses.py`)

Two scrapers with the same address but coords beyond the 4-decimal fingerprint rounding (~11 m/unit) produced distinct fingerprints → Tier 2a miss → duplicate house. Added Tier 2b lookup by normalized_address after Tier 2a (fingerprint) misses, and register the new fp via _insert_alias; alias upsert changed DO NOTHING → DO UPDATE SET fingerprint so the alias stays synced with the latest writer.

Part 3 — collapse double-nested SAVEPOINT (`scrapers/base.py`)

Removed the redundant inner db.begin_nested() around match_or_create_house inside _link_listing_to_house. The outer per-row SAVEPOINT in save_listings (base.py:386) already isolates a failing lot from its siblings and from the listings INSERT (per backend.md SAVEPOINT-in-loop rule). Surviving structure: listings INSERT outside SAVEPOINTs (always persists) + one SAVEPOINT around the snapshot + one around the house-link hook.

⚠️ BEHAVIOR CHANGE (please scrutinize): previously the inner SAVEPOINT let upsert_listing_source survive a house-match failure; now a house-match failure aborts the whole hook for that lot (incl. the listing_source link) atomically. The listing row itself still persists. This is cleaner/atomic and DoD-compliant, but it does drop the source-link on house-match failure — flagging in case partial-commit was intended.

Part 4 — match confidence/method → deferred to #774-3

match_or_create_listing returns (id, confidence, method) but isn't on the ingestion path (_link_listing_to_house calls match_or_create_house). listing_sources.confidence/matched_method columns exist but are written as source_link=1.0. Wiring the fuzzy matcher in is a non-trivial restructure → comment added, deferred. No migration.

Test plan

test_849_matching_savepoint.py (new): 2 listings same address → 1 house/1 alias key (Part 2); batch with one failing lot → siblings persist, no orphan (Part 3).
Updated test_matching.py Tier mocks (new Tier 2b execute sequence) + test_base_save_listings.py (Part 3 atomic-hook behavior).
pytest -k "matching or house or save_listings" → 276 passed; ruff + ruff-format clean.

Out of scope

estimator Tier A / headline (#774-3/#850), anchor (#755), UNIQUE(cadnum) migration (Part 1 deferred).

Refs #849, #774

## Summary #774-2 Parts 2/3/4 (Part 1 UNIQUE/dedup **deferred** — cadnum data not flowing per analyst re-scope). Code-only matching-subsystem fixes. ### Part 2 — fingerprint↔alias sync (`matching/houses.py`) Two scrapers with the same address but coords beyond the 4-decimal fingerprint rounding (~11 m/unit) produced distinct fingerprints → Tier 2a miss → duplicate house. Added **Tier 2b** lookup by `normalized_address` after Tier 2a (fingerprint) misses, and register the new fp via `_insert_alias`; alias upsert changed `DO NOTHING` → `DO UPDATE SET fingerprint` so the alias stays synced with the latest writer. ### Part 3 — collapse double-nested SAVEPOINT (`scrapers/base.py`) Removed the redundant **inner** `db.begin_nested()` around `match_or_create_house` inside `_link_listing_to_house`. The **outer** per-row SAVEPOINT in `save_listings` (base.py:386) already isolates a failing lot from its siblings and from the `listings` INSERT (per `backend.md` SAVEPOINT-in-loop rule). Surviving structure: listings INSERT outside SAVEPOINTs (always persists) + one SAVEPOINT around the snapshot + one around the house-link hook. ⚠️ **BEHAVIOR CHANGE (please scrutinize):** previously the inner SAVEPOINT let `upsert_listing_source` survive a house-match failure; now a house-match failure aborts the **whole hook** for that lot (incl. the `listing_source` link) atomically. The listing row itself still persists. This is cleaner/atomic and DoD-compliant, but it does drop the source-link on house-match failure — flagging in case partial-commit was intended. ### Part 4 — match confidence/method → **deferred to #774-3** `match_or_create_listing` returns `(id, confidence, method)` but isn't on the ingestion path (`_link_listing_to_house` calls `match_or_create_house`). `listing_sources.confidence`/`matched_method` columns exist but are written as `source_link=1.0`. Wiring the fuzzy matcher in is a non-trivial restructure → comment added, deferred. No migration. ## Test plan - [x] `test_849_matching_savepoint.py` (new): 2 listings same address → 1 house/1 alias key (Part 2); batch with one failing lot → siblings persist, no orphan (Part 3). - [x] Updated `test_matching.py` Tier mocks (new Tier 2b execute sequence) + `test_base_save_listings.py` (Part 3 atomic-hook behavior). - [x] `pytest -k "matching or house or save_listings"` → 276 passed; `ruff` + `ruff-format` clean. ## Out of scope estimator Tier A / headline (#774-3/#850), anchor (#755), UNIQUE(cadnum) migration (Part 1 deferred). Refs #849, #774

bot-backend added 1 commit 2026-05-31 07:58:59 +00:00

fix(matching): Tier 2b alias sync + collapse double SAVEPOINT (#849 ) 4e95cf622c

#774-2 Parts 2/3/4 (Part 1 UNIQUE/dedup deferred — cadnum data not flowing).

Part 2 — fingerprint<->alias sync: two scrapers with same address but coords
beyond the 4-decimal fingerprint rounding produced distinct fp -> duplicate
houses. Add Tier 2b lookup by normalized_address after Tier 2a (fingerprint)
misses, register the new fp via _insert_alias; alias upsert DO NOTHING ->
DO UPDATE SET fingerprint to keep alias synced.

Part 3 — collapse double-nested SAVEPOINT: removed redundant inner
db.begin_nested() around match_or_create_house inside _link_listing_to_house.
Outer per-row SAVEPOINT in save_listings already isolates a failing lot from
siblings and from the listings INSERT (per backend.md). BEHAVIOR NOTE: a
house-match failure now aborts the whole hook for that lot (incl. listing_source
link) atomically instead of partially committing it.

Part 4 — confidence/method wiring deferred to #774-3 (matcher not on ingestion
path; listing_sources columns exist, written as source_link=1.0).

Refs #849

bot-reviewer approved these changes 2026-05-31 08:05:39 +00:00

bot-reviewer left a comment

Collaborator

✅ APPROVE — matching Tier 2b sync + collapse double-SAVEPOINT (#849 Parts 2/3/4). Все три части соответствуют DoD issue.

Part 2 — Tier 2b (houses.py): корректно. После Tier 2a (fingerprint) miss → fallback WHERE normalized_address = :na (coord-independent) ловит cross-scraper coord-drift (>4-dec rounding) → возвращает существующий house_id вместо дубля. _insert_alias ре-регистрирует fp; ON CONFLICT (normalized_address) DO UPDATE SET fingerprint сводит к одной alias-строке (house_id НЕ обновляется — first-writer-wins). None-safe: normalize_address(None)→"" (normalize.py:53) + guard and norm_addr → Tier 2b пропускается на coord-only пути, краша нет. Concurrent double-insert race (advisory lock на per-scraper fp → не взаимоисключают) — pre-existing, не вводится здесь, явно deferred в Part 1 (merge dupes). Sequential-кейс (главный) починен.

Part 3 — SAVEPOINT collapse (base.py): behavior change «source-link дропается при house-match DB-fail» — вынужденный, не произвольный: после raise в match_or_create_house транзакция aborted, дойти до upsert_listing_source без savepoint-rollback нельзя (это и была причина внутреннего begin_nested). Убрав его, единственный вариант — пропагировать exception. Безопасно: внешний per-row SAVEPOINT (base.py:386) изолирует lot (match_failures++, continue → batch не падает), listings INSERT персистится (вне savepoint), source-link восстановим на следующем sweep. Триггерится лишь на редком match DB-fault. #849 DoD прямо требует «double-nested SAVEPOINT устранён, один уровень». ✓

Part 4: deferred в #774-3 с inline-комментарием — DoD допускает «зафиксировано решение».

psycopg v3 чисто (bind-params, CAST(:hid AS bigint), нет :x::type). 276 тестов pass (test_849_matching_savepoint: 2 листинга→1 house/1 alias + batch-fail→siblings persist). Reformatting (quote-style) — механический ruff-format, безопасен. Self-extending/секретов нет.

Мержу. #849 → qa. Note для qa: заодно глянуть save_listings:match_failed лог-частоту после deploy (source-link-drop trade-off).

✅ **APPROVE** — matching Tier 2b sync + collapse double-SAVEPOINT (#849 Parts 2/3/4). Все три части соответствуют DoD issue. **Part 2 — Tier 2b (houses.py):** корректно. После Tier 2a (fingerprint) miss → fallback `WHERE normalized_address = :na` (coord-independent) ловит cross-scraper coord-drift (>4-dec rounding) → возвращает существующий house_id вместо дубля. `_insert_alias` ре-регистрирует fp; `ON CONFLICT (normalized_address) DO UPDATE SET fingerprint` сводит к одной alias-строке (house_id НЕ обновляется — first-writer-wins). **None-safe:** `normalize_address(None)→""` (normalize.py:53) + guard `and norm_addr` → Tier 2b пропускается на coord-only пути, краша нет. Concurrent double-insert race (advisory lock на per-scraper `fp` → не взаимоисключают) — **pre-existing**, не вводится здесь, явно deferred в Part 1 (merge dupes). Sequential-кейс (главный) починен. **Part 3 — SAVEPOINT collapse (base.py):** behavior change «source-link дропается при house-match DB-fail» — **вынужденный, не произвольный**: после raise в `match_or_create_house` транзакция aborted, дойти до `upsert_listing_source` без savepoint-rollback нельзя (это и была причина внутреннего `begin_nested`). Убрав его, единственный вариант — пропагировать exception. **Безопасно:** внешний per-row SAVEPOINT (base.py:386) изолирует lot (`match_failures++`, continue → batch не падает), `listings` INSERT персистится (вне savepoint), source-link восстановим на следующем sweep. Триггерится лишь на редком match DB-fault. #849 DoD прямо требует «double-nested SAVEPOINT устранён, один уровень». ✓ **Part 4:** deferred в #774-3 с inline-комментарием — DoD допускает «зафиксировано решение». psycopg v3 чисто (bind-params, `CAST(:hid AS bigint)`, нет `:x::type`). 276 тестов pass (test_849_matching_savepoint: 2 листинга→1 house/1 alias + batch-fail→siblings persist). Reformatting (quote-style) — механический ruff-format, безопасен. Self-extending/секретов нет. Мержу. #849 → qa. **Note для qa:** заодно глянуть `save_listings:match_failed` лог-частоту после deploy (source-link-drop trade-off).