lekss361/gendesign

Fork 0

feat(tradein): estimator — Cian Valuation as on-demand evaluation source (Stage 9) #476

Merged

lekss361 merged 1 commit from feat/tradein-estimator-cian-clean into main

2026-05-23 14:29:53 +00:00

lekss361 commented

2026-05-23 14:25:12 +00:00

Owner

Summary

Stage 9 / Wave 7 of CianScraper v1. Integrate cian_valuation.estimate_via_cian_valuation() (Stage 7, PR #465) as on-demand evaluation source в estimator.py.

Files

app/services/estimator.py — +42 lines (import + Cian block mirroring existing avito_imv / yandex_valuation patterns)
tests/test_estimator_cian_integration.py (NEW) — 7 unit tests, 244 lines

Pattern reference

Mirrors yandex_valuation integration from PR #473 (commit b26e0ff):

Call async estimate_via_cian_valuation(db, ...) после aggregation
Append 'cian_valuation' к sources_used if result has sale_price_rub
Wrap в try/except for graceful degradation (no cookies / network / 401 → log + continue)

Graceful degradation

Cookies missing → estimate_via_cian_valuation returns None → log warning, estimator continues без Cian
Session expired → result.is_authenticated=False → marked invalid (Stage 4 hook) → estimator continues
Network error → caught, logged, continues

Cache

24h TTL via external_valuations cache_key (Stage 7 implementation). Cache hit → no Cian HTTP call.

Tests

7/7 pass. Coverage:

Cian result populates sources_used
Graceful when estimate_via_cian_valuation returns None (no cookies)
Graceful when raises RuntimeError (network error)
Cache hit path
(плюс edge cases)

Deviations from spec

external_benchmarks dict не реализован — AggregatedEstimate schema не has такого field. Codebase pattern (avito_imv, yandex_valuation) populates только sources_used. If external_benchmarks needed — separate schema change PR.
Branch name feat/tradein-estimator-cian-clean (suffix -clean) — added by worker, see commit fcded1a.

Push verify

	SHA
LOCAL	`fcded1a6522040742ebc45cac4101c307cfdef9e`
REMOTE	`fcded1a6522040742ebc45cac4101c307cfdef9e`

Ruff

New code clean. 5 pre-existing errors в estimator.py не touched.

Refs

decisions/CianScraper_v1_Implementation_Plan.md Stage 9
decisions/Schema_Cian_SERP_Inventory.md sec 27 (final flow)
PR #465 (Stage 7 cian_valuation.py — estimate_via_cian_valuation API)
PR #473 (yandex_valuation integration — pattern reference)

Stage 10 — QA smoke matrix after deploy.

Test plan

7 unit tests pass
No regression в estimator IMV / yandex tests
Ruff clean (new code)
Post-deploy: POST /estimate с ЕКБ address → sources_used includes cian_valuation (если cookies uploaded)
Cache hit on repeat request within 24h

## Summary **Stage 9 / Wave 7** of CianScraper v1. Integrate `cian_valuation.estimate_via_cian_valuation()` (Stage 7, PR #465) as on-demand evaluation source в `estimator.py`. ## Files - `app/services/estimator.py` — +42 lines (import + Cian block mirroring existing avito_imv / yandex_valuation patterns) - `tests/test_estimator_cian_integration.py` (NEW) — 7 unit tests, 244 lines ## Pattern reference Mirrors `yandex_valuation` integration from PR #473 (commit `b26e0ff`): - Call async `estimate_via_cian_valuation(db, ...)` после aggregation - Append `'cian_valuation'` к `sources_used` if result has `sale_price_rub` - Wrap в try/except for graceful degradation (no cookies / network / 401 → log + continue) ## Graceful degradation - Cookies missing → `estimate_via_cian_valuation` returns None → log warning, estimator continues без Cian - Session expired → result.is_authenticated=False → marked invalid (Stage 4 hook) → estimator continues - Network error → caught, logged, continues ## Cache 24h TTL via `external_valuations` cache_key (Stage 7 implementation). Cache hit → no Cian HTTP call. ## Tests 7/7 pass. Coverage: - Cian result populates `sources_used` - Graceful when `estimate_via_cian_valuation` returns None (no cookies) - Graceful when raises RuntimeError (network error) - Cache hit path - (плюс edge cases) ## Deviations from spec 1. **`external_benchmarks` dict не реализован** — `AggregatedEstimate` schema не has такого field. Codebase pattern (avito_imv, yandex_valuation) populates только `sources_used`. If `external_benchmarks` needed — separate schema change PR. 2. Branch name `feat/tradein-estimator-cian-clean` (suffix `-clean`) — added by worker, see commit `fcded1a`. ## Push verify | | SHA | |---|---| | LOCAL | `fcded1a6522040742ebc45cac4101c307cfdef9e` | | REMOTE | `fcded1a6522040742ebc45cac4101c307cfdef9e` | ## Ruff New code clean. 5 pre-existing errors в estimator.py не touched. ## Refs - `decisions/CianScraper_v1_Implementation_Plan.md` Stage 9 - `decisions/Schema_Cian_SERP_Inventory.md` sec 27 (final flow) - PR #465 (Stage 7 cian_valuation.py — `estimate_via_cian_valuation` API) - PR #473 (yandex_valuation integration — pattern reference) ## Next Stage 10 — QA smoke matrix after deploy. ## Test plan - [x] 7 unit tests pass - [x] No regression в estimator IMV / yandex tests - [x] Ruff clean (new code) - [ ] Post-deploy: POST /estimate с ЕКБ address → `sources_used` includes `cian_valuation` (если cookies uploaded) - [ ] Cache hit on repeat request within 24h

lekss361 added 1 commit 2026-05-23 14:25:12 +00:00

feat(tradein): estimator — Cian Valuation as 7th evaluation source (Stage 9) fcded1a652

lekss361 merged commit 37c4574d0a into main

2026-05-23 14:29:53 +00:00

lekss361 referenced this pull request from a commit

2026-05-23 14:29:54 +00:00

feat(tradein): estimator — Cian Valuation as on-demand 7th source (Stage 9) (#476)

lekss361 commented

2026-05-23 14:30:18 +00:00

Author

Owner

Merged via deep-code-reviewer — verdict APPROVE.

Verification summary

Pattern fidelity to PR #473 (yandex_valuation): high. Identical structure — import block, post-aggregation placement (before deals fetch), two-place sources_used append (pre + final), try/except graceful wrapper, logger conventions.

Signature alignment with PR #465 (cian_valuation module): verified. All kwargs match estimate_via_cian_valuation(db, *, address, total_area, rooms_count, floor, total_floors, repair_type, deal_type, use_cache) signature. No drift.

UNIQUE constraint re-confirmed: external_valuations.UNIQUE(source, cache_key) in data/sql/026_external_valuations.sql:34 applies for source='cian_valuation'. _save_to_cache uses ON CONFLICT DO UPDATE per Stage 7.

Graceful degradation tested: 4 modes — no cookies / session expired / network error / result without sale_price. All covered by 7 unit tests.

Architectural deviation noted (not blocking): Inline try/except + await estimate_via_cian_valuation() instead of _get_or_fetch_cian_valuation_cached() wrapper. Acceptable because the module already has internal cache lookup (use_cache=True default), making a separate wrapper redundant.

Non-blocking follow-ups

repair_type="cosmetic" and deal_type="sale" hardcoded — consider deriving from payload.repair_state in future PR
Sequential await for IMV (~5s) + Yandex (~3s) + Cian (~5s) ≈ ~13s on cache miss. Suggest asyncio.gather parallelization PR
No cross-source regression test (IMV + Yandex + Cian co-execution). Stage 10 QA smoke matrix will cover

Post-merge verification steps (for user)

deploy-tradein.yml should trigger on main
Verify CI pytest 7/7 green
Pre-condition: cookies uploaded via /admin/scrape/cian/upload-cookies (Stage 4 PR #457) if not already
Live smoke: POST /estimate with ЕКБ address → sources_used should contain cian_valuation (if auth OK)
Re-POST same address within 24h → cache HIT (no Cian HTTP fetch in logs)

Wave 7 (Stage 10 QA smoke matrix) now unblocked.

Merged via deep-code-reviewer — verdict APPROVE. ## Verification summary **Pattern fidelity to PR #473 (yandex_valuation)**: high. Identical structure — import block, post-aggregation placement (before deals fetch), two-place `sources_used` append (pre + final), try/except graceful wrapper, logger conventions. **Signature alignment with PR #465 (cian_valuation module)**: verified. All kwargs match `estimate_via_cian_valuation(db, *, address, total_area, rooms_count, floor, total_floors, repair_type, deal_type, use_cache)` signature. No drift. **UNIQUE constraint re-confirmed**: `external_valuations.UNIQUE(source, cache_key)` in `data/sql/026_external_valuations.sql:34` applies for `source='cian_valuation'`. `_save_to_cache` uses ON CONFLICT DO UPDATE per Stage 7. **Graceful degradation tested**: 4 modes — no cookies / session expired / network error / result without sale_price. All covered by 7 unit tests. **Architectural deviation noted (not blocking)**: Inline `try/except + await estimate_via_cian_valuation()` instead of `_get_or_fetch_cian_valuation_cached()` wrapper. Acceptable because the module already has internal cache lookup (`use_cache=True` default), making a separate wrapper redundant. ## Non-blocking follow-ups 1. `repair_type="cosmetic"` and `deal_type="sale"` hardcoded — consider deriving from `payload.repair_state` in future PR 2. Sequential await for IMV (~5s) + Yandex (~3s) + Cian (~5s) ≈ ~13s on cache miss. Suggest `asyncio.gather` parallelization PR 3. No cross-source regression test (IMV + Yandex + Cian co-execution). Stage 10 QA smoke matrix will cover ## Post-merge verification steps (for user) 1. `deploy-tradein.yml` should trigger on main 2. Verify CI pytest 7/7 green 3. **Pre-condition**: cookies uploaded via `/admin/scrape/cian/upload-cookies` (Stage 4 PR #457) if not already 4. Live smoke: `POST /estimate` with ЕКБ address → `sources_used` should contain `cian_valuation` (if auth OK) 5. Re-POST same address within 24h → cache HIT (no Cian HTTP fetch in logs) Wave 7 (Stage 10 QA smoke matrix) now unblocked.

lekss361 referenced this pull request

2026-05-24 15:59:54 +00:00

fix(tradein/devops): wire COOKIE_ENCRYPTION_KEY env to tradein-backend #543