chore(tradein): remove dead mock-fallback code from trade_in.py #521

Merged

lekss361 merged 1 commit from chore/tradein-cleanup-dead-mock-code into main

2026-05-24 13:11:13 +00:00

lekss361 commented

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

Owner

Context

After Wave 1-4 (PRs #502 #504 #505 #508 #513 #507 #517), the real estimator in services/estimator.py is the only path used by /estimate. The legacy mock fallback was kept during early development and is now unreachable.

Removed

_EKB_STREETS, _PRICE_BANDS constants
_floor_factor, _repair_factor, _confidence, _gen_analogs, _mock_estimate helpers
_estimate_legacy_mock function
Unused imports: json, random, timedelta, uuid4 (via ruff --select F401 --fix)
Outdated module docstring TODO

Safety

Pre-flight grep confirmed zero external references to any of the removed symbols (only mention outside file is a single comment in estimator.py). All pre-commit hooks ✅, ruff clean.

Stats

net −245 lines in tradein-mvp/backend/app/api/v1/trade_in.py
1 file changed

Roadmap

PR 8 of 8 — closes [[Decision_TradeIn_DataQuality_8PR_Roadmap]]. Full chain:

#	PR	Title	Status
1	#502	Avito CSS clean address	✅ merged
2	#504	Drop rosreestr deals	✅ merged
3	#507	House-match tiered S→H→W	✅ merged
5	#505	IMV retry + categorize errors	✅ merged
6	#508	listing_date Avito+Yandex	✅ merged
4	#513	Dynamic confidence (unique addr)	✅ merged
7	#517	UI mock-disclaimer + enrichment	✅ merged
8	this	Cleanup dead mock code	🔄 this PR

## Context After Wave 1-4 (PRs #502 #504 #505 #508 #513 #507 #517), the real estimator in `services/estimator.py` is the only path used by `/estimate`. The legacy mock fallback was kept during early development and is now unreachable. ## Removed - `_EKB_STREETS`, `_PRICE_BANDS` constants - `_floor_factor`, `_repair_factor`, `_confidence`, `_gen_analogs`, `_mock_estimate` helpers - `_estimate_legacy_mock` function - Unused imports: `json`, `random`, `timedelta`, `uuid4` (via `ruff --select F401 --fix`) - Outdated module docstring TODO ## Safety Pre-flight grep confirmed **zero external references** to any of the removed symbols (only mention outside file is a single comment in `estimator.py`). All pre-commit hooks ✅, ruff clean. ## Stats - **net −245 lines** in `tradein-mvp/backend/app/api/v1/trade_in.py` - 1 file changed ## Roadmap **PR 8 of 8** — closes `[[Decision_TradeIn_DataQuality_8PR_Roadmap]]`. Full chain: | # | PR | Title | Status | |---|----|-------|--------| | 1 | #502 | Avito CSS clean address | ✅ merged | | 2 | #504 | Drop rosreestr deals | ✅ merged | | 3 | #507 | House-match tiered S→H→W | ✅ merged | | 5 | #505 | IMV retry + categorize errors | ✅ merged | | 6 | #508 | listing_date Avito+Yandex | ✅ merged | | 4 | #513 | Dynamic confidence (unique addr) | ✅ merged | | 7 | #517 | UI mock-disclaimer + enrichment | ✅ merged | | 8 | this | Cleanup dead mock code | 🔄 this PR |

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

chore(tradein): remove dead mock-fallback code from trade_in.py d97fbcd55b

After Wave 1-4 (PRs #502/#504/#505/#508/#513/#507/#517), the real estimator
in services/estimator.py is the only path used by /estimate. The legacy
_mock_estimate / _PRICE_BANDS / _gen_analogs helpers + _estimate_legacy_mock
function were kept as a fallback during early development and are now
unreachable.

Removes:
- _EKB_STREETS, _PRICE_BANDS constants
- _floor_factor, _repair_factor, _confidence, _gen_analogs, _mock_estimate
- _estimate_legacy_mock
- unused imports (random, json, timedelta)

Verified: no external references via grep.

PR 8 of 8 — closes Decision_TradeIn_DataQuality_8PR_Roadmap.

lekss361 commented

2026-05-24 13:11:06 +00:00

Author

Owner

Deep Code Review — PR #521 — APPROVE

Summary

Status: APPROVE
Files: 1 (P1: tradein-mvp/backend/app/api/v1/trade_in.py)
Lines: +4 / -249 · 1 file · head sha d97fbcd55b32b9293c2e2445e9ac418e94f60e6f
Scope: closes PR 8 of 8 in [[Decision_TradeIn_DataQuality_8PR_Roadmap]] — мёртвый mock-fallback из trade_in.py

Cross-file impact (dead-code claim verified)

Просканированы все 97 .py файлов в tradein-mvp/backend/ на head SHA для каждого удалённого символа:

_mock_estimate, _estimate_legacy_mock, _EKB_STREETS, _PRICE_BANDS, _floor_factor, _repair_factor, _gen_analogs, _confidence → 0 ссылок во всех файлах кроме самого trade_in.py.
Тесты в tradein-mvp/backend/tests/** не импортируют ни одного удалённого символа.
POST /estimate уже маршрутизирован на app.services.estimator.estimate_quality(payload, db) (line 46-47); сигнатура совпадает: async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> AggregatedEstimate.

Env / config cleanup

Pre-merge сканирование на USE_MOCK | TRADE_IN_MOCK | ESTIMATE_MOCK | MOCK_ESTIMATE | TRADEIN_FALLBACK | if.*mock во всём tradein-mvp/backend/ → 0 совпадений. Нет env-var toggle, который полагался бы на mock-путь. Никакой dev/test env не мог незаметно ходить в _estimate_legacy_mock — он не вызывался из роутера (даже в комментарии было # OLD MOCK PATH — оставлен как fallback для отладки; не вызывается из router.).

Vault cross-check

decisions/Decision_TradeIn_DataQuality_8PR_Roadmap.md явно фиксирует PR 8: удалить _estimate_legacy_mock (224-291), _PRICE_BANDS/_EKB_STREETS/_mock_estimate/_gen_analogs/_floor_factor/_repair_factor/_confidence (36-204), почистить импорты. PR делает ровно это.
audits/TradeIn_MVP_CodeReview_May23.md H-9 (Dead code) закрывается этим PR.

Findings

Low (non-blocking — docs drift)

Доки всё ещё ссылаются на удалённую функцию _mock_estimate(). Не блокер, но для чистоты можно подмести в follow-up PR:

tradein-mvp/backend/app/services/estimator.py:3 — docstring "Заменяет старый _mock_estimate() из api/v1/trade_in.py." Можно перефразировать в "Реальная оценка квартиры (заменила раннюю mock-реализацию)."
tradein-mvp/README.md:136,155,157 — секция "Что внутри _mock_estimate()" и "TODO TI-1b". Stale.
tradein-mvp/docs/ROADMAP.md:110,328 — "3.1 Замена _mock_estimate()" и "/estimate (mock)" — done, можно отметить.

(Аудит уже зафиксировал Phase 4 — decommission всей директории tradein-mvp/, так что доки скоро уйдут целиком.)

Positive

Чистый ruff --select F401 --fix для json/random/timedelta/uuid4 — соответствует backend rule.
Docstring модуля переписан корректно ("TI-2 PDF, photos, history" + "Реальная оценка через estimate_quality()") — отражает текущий scope.
Net -245 LOC за один точечный PR, без collateral изменений.
Symbol naming не коллидирует с другими _confidence*/_floor_*/_repair_* в codebase (все были module-private; parcels.py использует совсем другие функции).

Complexity / blast radius

Risk: Low — pure delete недостижимого кода; нет API/schema/DB изменений.
Reversibility: trivially revert (1 commit, 1 file) если потребуется.
Merge window: безопасно в любое время, не deploy-критично.

Recommended next steps

Merge as-is (squash).
Optional follow-up: подмести stale _mock_estimate() ссылки в tradein-mvp/README.md / docs/ROADMAP.md / services/estimator.py docstring (одна PR на 3-4 строки).

## Deep Code Review — PR #521 — APPROVE ### Summary - Status: APPROVE - Files: 1 (P1: `tradein-mvp/backend/app/api/v1/trade_in.py`) - Lines: +4 / -249 · 1 file · head sha `d97fbcd55b32b9293c2e2445e9ac418e94f60e6f` - Scope: closes PR 8 of 8 in `[[Decision_TradeIn_DataQuality_8PR_Roadmap]]` — мёртвый mock-fallback из `trade_in.py` ### Cross-file impact (dead-code claim verified) Просканированы все 97 `.py` файлов в `tradein-mvp/backend/` на head SHA для каждого удалённого символа: - `_mock_estimate`, `_estimate_legacy_mock`, `_EKB_STREETS`, `_PRICE_BANDS`, `_floor_factor`, `_repair_factor`, `_gen_analogs`, `_confidence` → **0 ссылок** во всех файлах кроме самого `trade_in.py`. - Тесты в `tradein-mvp/backend/tests/**` не импортируют ни одного удалённого символа. - `POST /estimate` уже маршрутизирован на `app.services.estimator.estimate_quality(payload, db)` (line 46-47); сигнатура совпадает: `async def estimate_quality(payload: TradeInEstimateInput, db: Session) -> AggregatedEstimate`. ### Env / config cleanup Pre-merge сканирование на `USE_MOCK | TRADE_IN_MOCK | ESTIMATE_MOCK | MOCK_ESTIMATE | TRADEIN_FALLBACK | if.*mock` во всём `tradein-mvp/backend/` → **0 совпадений**. Нет env-var toggle, который полагался бы на mock-путь. Никакой dev/test env не мог незаметно ходить в `_estimate_legacy_mock` — он не вызывался из роутера (даже в комментарии было `# OLD MOCK PATH — оставлен как fallback для отладки; не вызывается из router.`). ### Vault cross-check - `decisions/Decision_TradeIn_DataQuality_8PR_Roadmap.md` явно фиксирует PR 8: удалить `_estimate_legacy_mock` (224-291), `_PRICE_BANDS/_EKB_STREETS/_mock_estimate/_gen_analogs/_floor_factor/_repair_factor/_confidence` (36-204), почистить импорты. PR делает ровно это. - `audits/TradeIn_MVP_CodeReview_May23.md` H-9 (Dead code) закрывается этим PR. ### Findings #### Low (non-blocking — docs drift) Доки всё ещё ссылаются на удалённую функцию `_mock_estimate()`. Не блокер, но для чистоты можно подмести в follow-up PR: - `tradein-mvp/backend/app/services/estimator.py:3` — docstring "Заменяет старый `_mock_estimate()` из api/v1/trade_in.py." Можно перефразировать в "Реальная оценка квартиры (заменила раннюю mock-реализацию)." - `tradein-mvp/README.md:136,155,157` — секция "Что внутри `_mock_estimate()`" и "TODO TI-1b". Stale. - `tradein-mvp/docs/ROADMAP.md:110,328` — "3.1 Замена `_mock_estimate()`" и "/estimate (mock)" — done, можно отметить. (Аудит уже зафиксировал Phase 4 — decommission всей директории `tradein-mvp/`, так что доки скоро уйдут целиком.) ### Positive - Чистый ruff `--select F401 --fix` для `json/random/timedelta/uuid4` — соответствует backend rule. - Docstring модуля переписан корректно ("TI-2 PDF, photos, history" + "Реальная оценка через estimate_quality()") — отражает текущий scope. - Net -245 LOC за один точечный PR, без collateral изменений. - Symbol naming не коллидирует с другими `_confidence*`/`_floor_*`/`_repair_*` в codebase (все были module-private; `parcels.py` использует совсем другие функции). ### Complexity / blast radius - **Risk**: Low — pure delete недостижимого кода; нет API/schema/DB изменений. - **Reversibility**: trivially revert (1 commit, 1 file) если потребуется. - **Merge window**: безопасно в любое время, не deploy-критично. ### Recommended next steps 1. Merge as-is (squash). 2. Optional follow-up: подмести stale `_mock_estimate()` ссылки в `tradein-mvp/README.md` / `docs/ROADMAP.md` / `services/estimator.py` docstring (одна PR на 3-4 строки).