lekss361/gendesign

Fork 0

fix(tradein): filter ДКП-only in rosreestr importer + re-enable deals #549

Merged

lekss361 merged 1 commit from feat/tradein-rosreestr-dkp-only into main

2026-05-24 19:42:29 +00:00

lekss361 commented

2026-05-24 19:34:34 +00:00

Owner

Problem

rosreestr_deals (open dataset Росреестра в gendesign-БД) содержит ОБА типа договоров:

ДКП (вторичка, ~110-120 К ₽/м²) — то что нужно для оценки
ДДУ (первичка от застройщика, ~177-200 К ₽/м²) — скёюит median вторички

Текущий import-rosreestr.sh льёт ОБА в tradein.deals без фильтра по doc_type. Из-за этого PR #504 сознательно отключил deals в estimator — компромисс на нехватку чистого источника. См. Decision_TradeIn_DataQuality_8PR_Roadmap.

Fix

Filter источник до ДКП → возвращаем deals в estimator.

Changes

tradein-mvp/deploy/import-rosreestr.sh
- AND doc_type = 'ДКП' в WHERE
- dedup_hash префикс изменён на 'ros:dkp:' — чтобы при будущем re-import ДДУ как отдельного источника не было коллизий
- Обновлён header-comment
tradein-mvp/backend/app/services/estimator.py:837-841
- deals: list[dict[str, Any]] = [] заменён на вызов уже существующего _fetch_deals() (line 1501)
tradein-mvp/backend/tests/test_estimator_cian_integration.py (bonus fix)
- _fetch_analogs mock возвращал 2-tuple ([], False) вместо 3-tuple ([], False, "W") — broken после PR #507 (Tier S/H/W). Worker нашёл при pytest run, исправил во всех 5 вхождений.

Post-merge ops (NOT in PR)

После merge — на prod manual через SSH:

DELETE FROM deals WHERE source='rosreestr' AND dedup_hash NOT LIKE 'ros:dkp:%';

Затем ./deploy/import-rosreestr.sh re-runs с правильным фильтром.

Verification

ruff check — pass
pytest test_estimator_cian_integration.py — 7/7 pass

Reverses PR #504 на правильной стороне (источник, не estimator).

## Problem `rosreestr_deals` (open dataset Росреестра в gendesign-БД) содержит ОБА типа договоров: - **ДКП** (вторичка, ~110-120 К ₽/м²) — то что нужно для оценки - **ДДУ** (первичка от застройщика, ~177-200 К ₽/м²) — скёюит median вторички Текущий `import-rosreestr.sh` льёт ОБА в `tradein.deals` без фильтра по `doc_type`. Из-за этого PR #504 сознательно отключил deals в estimator — компромисс на нехватку чистого источника. См. `Decision_TradeIn_DataQuality_8PR_Roadmap`. ## Fix Filter источник до ДКП → возвращаем deals в estimator. ### Changes 1. **`tradein-mvp/deploy/import-rosreestr.sh`** - `AND doc_type = 'ДКП'` в WHERE - `dedup_hash` префикс изменён на `'ros:dkp:'` — чтобы при будущем re-import ДДУ как отдельного источника не было коллизий - Обновлён header-comment 2. **`tradein-mvp/backend/app/services/estimator.py:837-841`** - `deals: list[dict[str, Any]] = []` заменён на вызов уже существующего `_fetch_deals()` (line 1501) 3. **`tradein-mvp/backend/tests/test_estimator_cian_integration.py`** (bonus fix) - `_fetch_analogs` mock возвращал 2-tuple `([], False)` вместо 3-tuple `([], False, "W")` — broken после PR #507 (Tier S/H/W). Worker нашёл при pytest run, исправил во всех 5 вхождений. ## Post-merge ops (NOT in PR) После merge — на prod manual через SSH: ```sql DELETE FROM deals WHERE source='rosreestr' AND dedup_hash NOT LIKE 'ros:dkp:%'; ``` Затем `./deploy/import-rosreestr.sh` re-runs с правильным фильтром. ## Verification - `ruff check` — pass - `pytest test_estimator_cian_integration.py` — 7/7 pass Reverses PR #504 на правильной стороне (источник, не estimator).

lekss361 added 1 commit 2026-05-24 19:34:35 +00:00

fix(tradein): filter ДКП-only in rosreestr importer + re-enable deals d3ad896efe

rosreestr_deals open dataset contains both ДКП (resale, ~110-120K ₽/м²) and
ДДУ (developer pre-sale, ~177-200K ₽/м²). Mixed import previously skewed
secondary-market median, forcing PR #504 to disable deals output entirely.

This PR fixes the root cause: filter doc_type='ДКП' in import-rosreestr.sh
(WHERE clause) and re-enable _fetch_deals() call in estimator.py (was stubbed
to deals=[] since PR #504). dedup_hash prefix changed to 'ros:dkp:' to allow
future ДДУ import as separate source without collision.

Also fix pre-existing broken mock in test_estimator_cian_integration.py:
_fetch_analogs returns 3-tuple (list, bool, str) but mocks passed ([], False)
— updated to ([], False, 'W') matching actual signature.

Post-merge ops (NOT in this PR):
  DELETE FROM deals WHERE source='rosreestr' AND dedup_hash NOT LIKE 'ros:dkp:%';
  ./deploy/import-rosreestr.sh   # на prod

Tests: _fetch_deals already mocked in test_estimator_cian_integration.py.

Refs: Decision_TradeIn_DataQuality_8PR_Roadmap (PR #504 reversed properly).

lekss361 referenced this pull request

2026-05-24 19:37:41 +00:00

feat(tradein): «Росреестр» button — copy address + open EGRN request form #550

lekss361 commented

2026-05-24 19:42:21 +00:00

Author

Owner

Deep Code Review — verdict

Summary

Status: APPROVE
Files: 3 (P0: 1 importer + 1 estimator service · P3: 1 test)
Lines: +20 / -11
Risk: Low · Reversibility: easy (revert + manual DELETE rollback) · Merge window: any

Correctness

ДКП filter at the importer is the right architectural call — reverses PR #504's compromise by fixing the root cause (data source) instead of the consumer (estimator).
DB cross-check: rosreestr_deals_dealtype_idx is a partial btree ON (doc_type) WHERE doc_type IN ('ДКП','ДДУ'). EXPLAIN ANALYZE on the importer's WHERE-clause (region 66 + ЕКБ + apt code + 2024+) shows only 2 distinct doc_type values. No leakage from дарение/наследование/мена — those transaction types simply do not appear in this slice of the dataset, so the new doc_type='ДКП' filter is sufficient (no risk of unfiltered non-market types).
_fetch_deals(db, *, lat, lon, rooms, area, radius_m) — all required kwargs supplied. geo is non-None at line 840 (earlier _empty_estimate early-return at line 698). payload.rooms/area_m2 are Pydantic-required, so safe to pass.
Downstream uses of deals: (1) saved to trade_in_estimates.actual_deals JSONB, (2) _estimate_days_on_market, (3) UI/PDF display. Not used for median/range — that's purely from listings_clean. So re-enabling does not perturb the median; it restores the supporting display data + days-on-market signal.
Frontend (DealsCard.tsx, HeroSummary.tsx, SourcesProgress.tsx) already gracefully handles both empty and populated actual_deals (uses .length > 0 / ?? 0). No frontend regression.
PDF exporter has fallback to listing range when deals are empty — and now naturally uses deals when present.

dedup_hash prefix change — good catch

md5('ros:' || id::text) → md5('ros:dkp:' || id::text) is the right move:

Forces a full re-import (since hashes change) — this is the intended behavior given the documented manual DELETE WHERE source='rosreestr' AND dedup_hash NOT LIKE 'ros:dkp:%'.
Future-proofs against a separate ДДУ importer (would use ros:ddu: prefix) — no hash collisions across data types.

Test coverage

Bonus fix: 5 _fetch_analogs mocks updated from 2-tuple ([], False) to 3-tuple ([], False, "W") — correct shape per _fetch_analogs signature at lines 1284/1383/1504 (returns (candidates, fallback_radius_used, tier)). Restores pytest after PR #507 (Tier S/H/W).
Note: existing tests still patch _fetch_deals → [], so the deals-reintegration code path is not directly asserted in unit tests. Acceptable scope — the path is trivial (call _fetch_deals and pass through), and verifying the ДКП SQL filter would require a fixture against the real rosreestr_deals partition (out of unit-test reach). Post-merge smoke (/estimate for any ЕКБ address with new data) is the appropriate verification.

Minor — stale doc comment (LOW, non-blocking)

estimator.py:578 still carries the stale docstring NOTE:

NOTE 2026-05-24: rosreestr_deals temporarily NOT included in actual_deals
output (they contain ДДУ first-market data that skews secondary-market
median). See Decision_TradeIn_DataQuality_8PR_Roadmap.

This is now obsolete — the PR re-enables deals. Suggest a follow-up edit (or fixup if you want to bundle): replace with a one-line NOTE: rosreestr_deals filtered to doc_type='ДКП' at importer (PR #549).

Post-merge ops verified

The PR body's manual ops are correct and necessary:

DELETE FROM deals WHERE source='rosreestr' AND dedup_hash NOT LIKE 'ros:dkp:%';

This is required because old rows with ros:<id> hashes would otherwise persist alongside new ros:dkp:<id> rows after re-import. The LIKE pattern is correct (NOT LIKE matches all old-prefix rows + any anomalies). The re-run ./deploy/import-rosreestr.sh then populates clean ДКП-only data; geocode-deals cron will fill lat/lon.

Cross-PR consistency with #504

Logically sound: PR #504 stubbed out _fetch_deals with deals = [] as a workaround for the ДДУ contamination. This PR addresses the upstream cause at the importer and reverses the workaround. The decision doc Decision_TradeIn_DataQuality_8PR_Roadmap PR-2 row explicitly anticipated this ("Возвращаемся к этому когда отделим вторичные сделки от ДДУ в источнике").

Security

No secrets. SRC_USER/SRC_DB use env defaults. Cyrillic literal 'ДКП' correctly single-quoted inside the double-quoted -c arg.

Verdict: approve. Merging.

## Deep Code Review — verdict ### Summary - **Status:** APPROVE - **Files:** 3 (P0: 1 importer + 1 estimator service · P3: 1 test) - **Lines:** +20 / -11 - **Risk:** Low · Reversibility: easy (revert + manual DELETE rollback) · Merge window: any  ### Correctness - ДКП filter at the importer is the right architectural call — reverses PR #504's compromise by fixing the root cause (data source) instead of the consumer (estimator). - DB cross-check: `rosreestr_deals_dealtype_idx` is a partial btree ON `(doc_type)` WHERE `doc_type IN ('ДКП','ДДУ')`. EXPLAIN ANALYZE on the importer's WHERE-clause (region 66 + ЕКБ + apt code + 2024+) shows only 2 distinct doc_type values. No leakage from дарение/наследование/мена — those transaction types simply do not appear in this slice of the dataset, so the new `doc_type='ДКП'` filter is sufficient (no risk of unfiltered non-market types). - `_fetch_deals(db, *, lat, lon, rooms, area, radius_m)` — all required kwargs supplied. `geo` is non-None at line 840 (earlier `_empty_estimate` early-return at line 698). `payload.rooms`/`area_m2` are Pydantic-required, so safe to pass. - Downstream uses of `deals`: (1) saved to `trade_in_estimates.actual_deals` JSONB, (2) `_estimate_days_on_market`, (3) UI/PDF display. **Not** used for median/range — that's purely from `listings_clean`. So re-enabling does not perturb the median; it restores the supporting display data + days-on-market signal. - Frontend (`DealsCard.tsx`, `HeroSummary.tsx`, `SourcesProgress.tsx`) already gracefully handles both empty and populated `actual_deals` (uses `.length > 0` / `?? 0`). No frontend regression. - PDF exporter has fallback to listing range when deals are empty — and now naturally uses deals when present. ### dedup_hash prefix change — good catch `md5('ros:' || id::text)` → `md5('ros:dkp:' || id::text)` is the right move: - Forces a full re-import (since hashes change) — this is the intended behavior given the documented manual `DELETE WHERE source='rosreestr' AND dedup_hash NOT LIKE 'ros:dkp:%'`. - Future-proofs against a separate ДДУ importer (would use `ros:ddu:` prefix) — no hash collisions across data types. ### Test coverage - Bonus fix: 5 `_fetch_analogs` mocks updated from 2-tuple `([], False)` to 3-tuple `([], False, "W")` — correct shape per `_fetch_analogs` signature at lines 1284/1383/1504 (returns `(candidates, fallback_radius_used, tier)`). Restores pytest after PR #507 (Tier S/H/W). - Note: existing tests still patch `_fetch_deals → []`, so the deals-reintegration code path is not directly asserted in unit tests. Acceptable scope — the path is trivial (call `_fetch_deals` and pass through), and verifying the ДКП SQL filter would require a fixture against the real `rosreestr_deals` partition (out of unit-test reach). Post-merge smoke (`/estimate` for any ЕКБ address with new data) is the appropriate verification. ### Minor — stale doc comment (LOW, non-blocking) `estimator.py:578` still carries the stale docstring NOTE: ``` NOTE 2026-05-24: rosreestr_deals temporarily NOT included in actual_deals output (they contain ДДУ first-market data that skews secondary-market median). See Decision_TradeIn_DataQuality_8PR_Roadmap. ``` This is now obsolete — the PR re-enables deals. Suggest a follow-up edit (or fixup if you want to bundle): replace with a one-line `NOTE: rosreestr_deals filtered to doc_type='ДКП' at importer (PR #549).` ### Post-merge ops verified The PR body's manual ops are correct and necessary: ```sql DELETE FROM deals WHERE source='rosreestr' AND dedup_hash NOT LIKE 'ros:dkp:%'; ``` This is required because old rows with `ros:<id>` hashes would otherwise persist alongside new `ros:dkp:<id>` rows after re-import. The `LIKE` pattern is correct (`NOT LIKE` matches all old-prefix rows + any anomalies). The re-run `./deploy/import-rosreestr.sh` then populates clean ДКП-only data; `geocode-deals` cron will fill lat/lon. ### Cross-PR consistency with #504 Logically sound: PR #504 stubbed out `_fetch_deals` with `deals = []` as a workaround for the ДДУ contamination. This PR addresses the upstream cause at the importer and reverses the workaround. The decision doc `Decision_TradeIn_DataQuality_8PR_Roadmap` PR-2 row explicitly anticipated this ("Возвращаемся к этому когда отделим вторичные сделки от ДДУ в источнике"). ### Security - No secrets. SRC_USER/SRC_DB use env defaults. Cyrillic literal `'ДКП'` correctly single-quoted inside the double-quoted `-c` arg. ### Verdict: approve. Merging.

lekss361 merged commit 596842df8d into main

2026-05-24 19:42:29 +00:00

lekss361 deleted branch feat/tradein-rosreestr-dkp-only

2026-05-24 19:42:29 +00:00

lekss361 referenced this pull request from a commit

2026-05-24 19:42:30 +00:00

fix(tradein): filter ДКП-only in rosreestr importer + re-enable deals (#549)

lekss361 referenced this pull request from a commit

2026-05-24 19:56:25 +00:00

feat(tradein): GET /street-deals — ДКП-сделки по улице target адреса

lekss361 referenced this pull request

2026-05-24 19:57:37 +00:00