fix(week-review): backend-аудит v2 — 82 фиксов #1660

Merged

lekss361 merged 3 commits from fix/week-review-backend-v2 into main 2026-06-17 17:13:39 +00:00

Conversation 0 Commits 3 Files changed 60 +1345 -397

bot-backend commented 2026-06-16 20:32:06 +00:00

Collaborator

Copy link

Фиксы 2-го аудита backend/app/** (label week ревью 1), после merge #1543.

Итог

• 82 issue исправлено (Closes ниже)
• Открытыми оставлены 13: partial/needs-cross-file/needs-leha — #1569, #1590, #1593, #1606, #1609, #1617, #1633, #1635, #1637, #1638, #1640, #1642, #1650
• Верификация: py_compile 58/58 .py. Юнит-тесты — в CI.
• ⚠️ Возможны конфликты с незамёрженными ветками Лехи по backend/app (perf/, feat/obj, feat/poi-saturation) — резолв при merge.

Closes #1560
Closes #1561
Closes #1562
Closes #1563
Closes #1564
Closes #1565
Closes #1566
Closes #1567
Closes #1570
Closes #1571
Closes #1572
Closes #1573
Closes #1574
Closes #1576
Closes #1577
Closes #1578
Closes #1579
Closes #1580
Closes #1581
Closes #1582
Closes #1583
Closes #1584
Closes #1585
Closes #1586
Closes #1587
Closes #1588
Closes #1589
Closes #1591
Closes #1592
Closes #1594
Closes #1595
Closes #1596
Closes #1597
Closes #1598
Closes #1599
Closes #1600
Closes #1601
Closes #1602
Closes #1603
Closes #1604
Closes #1605
Closes #1607
Closes #1608
Closes #1610
Closes #1611
Closes #1612
Closes #1613
Closes #1614
Closes #1615
Closes #1616
Closes #1618
Closes #1619
Closes #1620
Closes #1621
Closes #1622
Closes #1623
Closes #1624
Closes #1625
Closes #1626
Closes #1627
Closes #1628
Closes #1629
Closes #1630
Closes #1631
Closes #1632
Closes #1634
Closes #1636
Closes #1639
Closes #1641
Closes #1643
Closes #1644
Closes #1645
Closes #1646
Closes #1647
Closes #1648
Closes #1649
Closes #1651
Closes #1652
Closes #1653
Closes #1654
Closes #1655
Closes #1656

Фиксы 2-го аудита backend/app/** (label week ревью 1), после merge #1543. ## Итог - **82 issue исправлено** (Closes ниже) - Открытыми оставлены 13: partial/needs-cross-file/needs-leha — #1569, #1590, #1593, #1606, #1609, #1617, #1633, #1635, #1637, #1638, #1640, #1642, #1650 - Верификация: py_compile 58/58 .py. Юнит-тесты — в CI. - ⚠️ Возможны конфликты с незамёрженными ветками Лехи по backend/app (perf/*, feat/obj*, feat/poi-saturation) — резолв при merge. Closes #1560 Closes #1561 Closes #1562 Closes #1563 Closes #1564 Closes #1565 Closes #1566 Closes #1567 Closes #1570 Closes #1571 Closes #1572 Closes #1573 Closes #1574 Closes #1576 Closes #1577 Closes #1578 Closes #1579 Closes #1580 Closes #1581 Closes #1582 Closes #1583 Closes #1584 Closes #1585 Closes #1586 Closes #1587 Closes #1588 Closes #1589 Closes #1591 Closes #1592 Closes #1594 Closes #1595 Closes #1596 Closes #1597 Closes #1598 Closes #1599 Closes #1600 Closes #1601 Closes #1602 Closes #1603 Closes #1604 Closes #1605 Closes #1607 Closes #1608 Closes #1610 Closes #1611 Closes #1612 Closes #1613 Closes #1614 Closes #1615 Closes #1616 Closes #1618 Closes #1619 Closes #1620 Closes #1621 Closes #1622 Closes #1623 Closes #1624 Closes #1625 Closes #1626 Closes #1627 Closes #1628 Closes #1629 Closes #1630 Closes #1631 Closes #1632 Closes #1634 Closes #1636 Closes #1639 Closes #1641 Closes #1643 Closes #1644 Closes #1645 Closes #1646 Closes #1647 Closes #1648 Closes #1649 Closes #1651 Closes #1652 Closes #1653 Closes #1654 Closes #1655 Closes #1656

bot-backend added 1 commit 2026-06-16 20:32:07 +00:00

fix(week-review): backend-аудит v2 — 82 issue (#1560-1656) ... e0fff9cfb0

Имплементация фиксов 2-го аудита backend/app/** (после merge #1543).
Воркер на файл, точечные правки. Верификация: py_compile 58/58 .py.

Полностью исправлено (82).
Оставлены открытыми (13): partial/needs-cross-file/needs-leha — #1569, #1590, #1593, #1606, #1609, #1617, #1633, #1635, #1637, #1638, #1640, #1642, #1650.

Closes #1560
Closes #1561
Closes #1562
Closes #1563
Closes #1564
Closes #1565
Closes #1566
Closes #1567
Closes #1570
Closes #1571
Closes #1572
Closes #1573
Closes #1574
Closes #1576
Closes #1577
Closes #1578
Closes #1579
Closes #1580
Closes #1581
Closes #1582
Closes #1583
Closes #1584
Closes #1585
Closes #1586
Closes #1587
Closes #1588
Closes #1589
Closes #1591
Closes #1592
Closes #1594
Closes #1595
Closes #1596
Closes #1597
Closes #1598
Closes #1599
Closes #1600
Closes #1601
Closes #1602
Closes #1603
Closes #1604
Closes #1605
Closes #1607
Closes #1608
Closes #1610
Closes #1611
Closes #1612
Closes #1613
Closes #1614
Closes #1615
Closes #1616
Closes #1618
Closes #1619
Closes #1620
Closes #1621
Closes #1622
Closes #1623
Closes #1624
Closes #1625
Closes #1626
Closes #1627
Closes #1628
Closes #1629
Closes #1630
Closes #1631
Closes #1632
Closes #1634
Closes #1636
Closes #1639
Closes #1641
Closes #1643
Closes #1644
Closes #1645
Closes #1646
Closes #1647
Closes #1648
Closes #1649
Closes #1651
Closes #1652
Closes #1653
Closes #1654
Closes #1655
Closes #1656

bot-reviewer requested changes 2026-06-17 07:19:36 +00:00

bot-reviewer left a comment

Collaborator

Copy link

Deep Code Review — REQUEST_CHANGES (verdict: 🟠 blocker — CI red)

Bulk audit-fix PR (82 fixes, 50 files, +1302/-377). Security tripwire clean, ~12 hot/high-risk files audited individually, all production logic looks sound. But CI backend-tests is RED on head e0fff9c, and I reproduced 3 deterministic failures locally (worktree at the PR head, uv run pytest on touched modules). All three are deliberate behavior changes where a pre-existing test was NOT updated — exactly the class py_compile can't catch.

🟠 Blockers (3 failing tests — must fix before merge)

1. tests/services/forecasting/test_affordability.py:272 TestPaymentAtScenario::test_scenario_payment_per_horizon

   • Fix #1639 in affordability.py:351 now adds _KEY_RATE_MARKET_SPREAD_PP (4.5pp) to the scenario rate before the annuity. The test still asserts the bare-rate annuity → assert 68168.43 == 50186.40.
   • The sibling test_default_market_path (line ~240) WAS updated for the spread; this scenario test was missed. Production code is arguably correct; the test must be updated to expect _annuity(principal, 8.0 + _KEY_RATE_MARKET_SPREAD_PP, _ANNUITY_TERM_MONTHS) (and 20.0 + spread).

2. tests/services/test_nspd_denorm.py:307 test_denorm_dump_empty_features

   • nspd_denorm.denorm_dump now returns a 4th key "skipped". Test asserts the old 3-key dict {parcels,buildings,errors} → Left contains 1 more item: {'skipped': 0}.
   • Update expected dict to include "skipped": 0.

3. tests/services/test_nspd_denorm.py:322 test_denorm_dump_no_cad_num_counted_as_error

   • The fix re-categorizes a feature with no cad_num from errors → skipped. Test still asserts counts["errors"] == 1 → got 0 (it's now in skipped).
   • Update to assert counts["skipped"] == 1 and counts["errors"] == 0.

✅ What passed (no changes needed)

• Security tripwire: redaction.py STRENGTHENS redaction (adds RU local-phone format #1641 + INN checksum gate #1640 — the checksum only suppresses false-positive large-number redaction, valid INNs still redacted). llm/client.py/provider.py add finish_reason propagation + tolerant token coercion. main.py adds before_send=scrub_sensitive_query to Sentry (improvement). No literal secret/key/token anywhere in the diff.
• models/parcel.py: removed Parcel ORM model — verified zero importers of app.models.parcel.Parcel repo-wide; only kills a phantom CREATE TABLE parcels in alembic autogen. Safe.
• api/v1/parcels.py: #1561 persist-after-forecast reordering is correct (jsonb snapshot now matches live response; forecast enqueue reads the run at execution time ~30-180s later, no race). run_in_threadpool for sync DB-IO in async handlers is the right asyncio fix.
• placement.py: place_strategy -> ConceptVariant | None + place_all_strategies raising ParcelGeometryError (mapped to 422 in concepts.py:30) — sole caller is internal; existing tests use normal parcels, still pass (421 site_finder + placement tests green).
• regression.py #1636 confounded-flag, competitors.py #1615 price gap-fill SQL (no psycopg ::type trap — casts glued to ), the allowed form; CAST(:radius AS float) correct), dxf.py interior-ring support + TextEntityAlignment enum (ezdxf 1.4.3 has it), domrf_kn.py pagination + _to_float, ekburg_permits.py workbook try/finally close, thumbs.py atomic write — all sound.
• ruff check: clean. site_finder 421✓, exporters 95✓, forecasting 1042✓/1F, generative placement✓.

Cross-file impact

• No API/schema break: schemas/chat.py swaps max_length (422) for truncating validator (looser — no FE break); schemas/concept.py adds ge=0 to optional land_cost_rub (negative now 422 — nonsensical value, low risk).

Recommended next steps

1. Update the 3 stale test assertions above (production code is fine — tests just lag the deliberate behavior change).
2. Push fixup → CI backend-tests green → I re-review and merge (squash, delete branch).

Complexity / blast radius

• Risk: medium (broad surface, but each fix small & graceful). Reversibility: high (squash, isolated). Merge window: blocked on CI green.

(Note: PR #1661 dependency satisfied — it merged. Holding #1660 merge per gate: CI red.)

## Deep Code Review — REQUEST_CHANGES (verdict: 🟠 blocker — CI red) Bulk audit-fix PR (82 fixes, 50 files, +1302/-377). Security tripwire clean, ~12 hot/high-risk files audited individually, all production logic looks sound. **But CI `backend-tests` is RED on head `e0fff9c`, and I reproduced 3 deterministic failures locally** (worktree at the PR head, `uv run pytest` on touched modules). All three are deliberate behavior changes where a pre-existing test was NOT updated — exactly the class py_compile can't catch. ### 🟠 Blockers (3 failing tests — must fix before merge) 1. `tests/services/forecasting/test_affordability.py:272` `TestPaymentAtScenario::test_scenario_payment_per_horizon` - Fix #1639 in `affordability.py:351` now adds `_KEY_RATE_MARKET_SPREAD_PP` (4.5pp) to the scenario rate before the annuity. The test still asserts the bare-rate annuity → `assert 68168.43 == 50186.40`. - The sibling `test_default_market_path` (line ~240) WAS updated for the spread; this scenario test was missed. Production code is arguably correct; the test must be updated to expect `_annuity(principal, 8.0 + _KEY_RATE_MARKET_SPREAD_PP, _ANNUITY_TERM_MONTHS)` (and 20.0 + spread). 2. `tests/services/test_nspd_denorm.py:307` `test_denorm_dump_empty_features` - `nspd_denorm.denorm_dump` now returns a 4th key `"skipped"`. Test asserts the old 3-key dict `{parcels,buildings,errors}` → `Left contains 1 more item: {'skipped': 0}`. - Update expected dict to include `"skipped": 0`. 3. `tests/services/test_nspd_denorm.py:322` `test_denorm_dump_no_cad_num_counted_as_error` - The fix re-categorizes a feature with no `cad_num` from `errors` → `skipped`. Test still asserts `counts["errors"] == 1` → got `0` (it's now in `skipped`). - Update to assert `counts["skipped"] == 1` and `counts["errors"] == 0`. ### ✅ What passed (no changes needed) - **Security tripwire**: `redaction.py` STRENGTHENS redaction (adds RU local-phone format #1641 + INN checksum gate #1640 — the checksum only suppresses false-positive large-number redaction, valid INNs still redacted). `llm/client.py`/`provider.py` add `finish_reason` propagation + tolerant token coercion. `main.py` adds `before_send=scrub_sensitive_query` to Sentry (improvement). No literal secret/key/token anywhere in the diff. - **`models/parcel.py`**: removed `Parcel` ORM model — verified zero importers of `app.models.parcel.Parcel` repo-wide; only kills a phantom `CREATE TABLE parcels` in alembic autogen. Safe. - **`api/v1/parcels.py`**: #1561 persist-after-forecast reordering is correct (jsonb snapshot now matches live response; forecast enqueue reads the run at execution time ~30-180s later, no race). `run_in_threadpool` for sync DB-IO in async handlers is the right asyncio fix. - **`placement.py`**: `place_strategy -> ConceptVariant | None` + `place_all_strategies` raising `ParcelGeometryError` (mapped to 422 in concepts.py:30) — sole caller is internal; existing tests use normal parcels, still pass (421 site_finder + placement tests green). - **`regression.py`** #1636 confounded-flag, **`competitors.py`** #1615 price gap-fill SQL (no psycopg `::type` trap — casts glued to `)`, the allowed form; `CAST(:radius AS float)` correct), **`dxf.py`** interior-ring support + `TextEntityAlignment` enum (ezdxf 1.4.3 has it), **`domrf_kn.py`** pagination + `_to_float`, **`ekburg_permits.py`** workbook `try/finally close`, **`thumbs.py`** atomic write — all sound. - ruff check: clean. site_finder 421✓, exporters 95✓, forecasting 1042✓/1F, generative placement✓. ### Cross-file impact - No API/schema break: `schemas/chat.py` swaps `max_length` (422) for truncating validator (looser — no FE break); `schemas/concept.py` adds `ge=0` to optional `land_cost_rub` (negative now 422 — nonsensical value, low risk). ### Recommended next steps 1. Update the 3 stale test assertions above (production code is fine — tests just lag the deliberate behavior change). 2. Push fixup → CI `backend-tests` green → I re-review and merge (squash, delete branch). ### Complexity / blast radius - Risk: medium (broad surface, but each fix small & graceful). Reversibility: high (squash, isolated). Merge window: blocked on CI green. _(Note: PR #1661 dependency satisfied — it merged. Holding #1660 merge per gate: CI red.)_

Light1YT referenced this pull request from a commit 2026-06-17 09:33:47 +00:00

fix(backend): исправить 4 регрессии code-review (PR #1660 follow-up)

Light1YT added 1 commit 2026-06-17 09:33:47 +00:00

fix(backend): исправить 4 регрессии code-review (PR #1660 follow-up) ... dd1443045e

- parcels: forecast_site_finder_report.delay() ПОСЛЕ persist_analysis_run
  (был commit-before-enqueue race → воркер видел None/stale ран, ретраев нет)
- test_affordability: ожидания сценарного платежа с _KEY_RATE_MARKET_SPREAD_PP
  (правка спреда ломала test_scenario_payment_per_horizon → красный gate)
- scrape_cadastre: targets_total += уже выполненное при resume (skipped_fresh
  кварталы не должны занижать total → преждевременный 'done')
- trade_in_pdf: бейдж достоверности = n_analogs (полная статбаза), не len(analogs)

bot-backend referenced this pull request from a commit 2026-06-17 17:03:17 +00:00

test(nspd): align denorm_dump tests with skipped-count return shape (#1660)

bot-backend added 1 commit 2026-06-17 17:03:18 +00:00

test(nspd): align denorm_dump tests with skipped-count return shape (#1660) ead08549e4

lekss361 merged commit 14f3ef2019 into main 2026-06-17 17:13:39 +00:00

lekss361 referenced this pull request from a commit 2026-06-17 17:13:41 +00:00

fix(week-review): backend-аудит v2 — 82 фиксов (#1660)

bot-backend referenced this pull request from a commit 2026-06-17 17:21:18 +00:00

merge: resolve conflicts with main (#1660) into week-review-finish

bot-backend referenced this pull request from a commit 2026-06-17 19:38:59 +00:00

test(etl): fix stale assert in objective_backfill dry_run test (#1709)

lekss361 referenced this pull request 2026-06-17 19:39:12 +00:00

test(etl): fix stale assert in objective_backfill dry_run test (#1709 systemic CI red) #1721

Sign in to join this conversation.

Reviewers

No reviewers

bot-reviewer

Labels

Clear labels   

admin

Admin UI / internal tools / pipeline tracker

  

analytics

Аналитика данных / matching

  

auth

Authentication / authorization / multi-tenancy

  

automation

Автоматизация / cron / scheduler

  

bug

Something isn't working

  

business

Коммерческая воронка / пилоты / pipeline / реестр ПО

  

chore

Operational housekeeping

  

ci

Forgejo Actions / build pipeline

  

compliance

152-ФЗ / реестр ПО / legal / TOS

  

data

Data sources, parsers, ETL

  

data-moat

Уникальные данные — sales tracker, CRM aggregation

  

docs

README / runbooks / onboarding

  

duplicate

This issue or pull request already exists

  

dx

Developer experience / build speed

  

enhancement

New feature or request

  

Fable 5 ревью

Находки полного мульти-агентного ревью (Claude Fable 5, 2026-06-09)

  

feedback/max

Feedback from Max (Senior partner)

  

generative

Generative Design — concept generation engine

  

GG-форсайт

Новое ТЗ от Ген Геныча (июнь 2026): Site Finder v2 — ИИ-прогноз продуктовой стратегии девелопера. Актуальный приоритет.

  

needs-discussion

Pending team sync / decision

  

needs-human

Human review/decision required

  

observability

Monitoring / metrics / logs / Sentry / Uptime

  

pause-bots

Kill-switch — all autonomous bots pause

  

performance

Latency / throughput / DB query tuning

  

priority/p0

Critical / production-blocker

  

priority/p1

High priority

  

priority/p2

Normal priority

  

priority/p3

Низкий приоритет (follow-up, tech-debt)

  

scope/backend

Backend Python — FastAPI, Celery, scrapers

  

scope/db

DB migrations, indexes, SQL

  

scope/devops

docker-compose, Caddy, workflows

  

scope/frontend

Frontend — Next.js, React, TypeScript

  

scope/qa

Tests, smoke, Playwright

  

scrapers

Скрапперы (Avito/Cian/Yandex/Rosreestr)

  

security

Security audit / secrets / vulnerabilities

  

site-finder

Site Finder scoring quality & data layer

  

stage/1

Stage 1: Generative MVP (нед 2-7)

  

stage/2

Stage 2: Site Finder MVP (нед 8-12)

  

status/blocked

Blocked — needs human intervention

  

status/done

Closed, completed

  

status/needs-analysis

Raw/fuzzy task filed by human — auto-analyst подхватывает: code-archeology + декомпозиция в actionable scope/* + status/ready (или split на под-issues)

  

status/needs-fix

Reviewer/QA вернул worker на доработку — worker сам чинит свой PR (fixup), НЕ требует human (в отличие от status/blocked)

  

status/qa

PR merged, awaiting smoke test

  

status/ready

Ready for pickup by worker bot

  

status/review

PR open, awaiting code review

  

status/wip

Work in progress (worker claimed)

  

tech-debt

Технический долг

  

tradein

Trade-In MVP проект

  

ux

User experience / responsive / conversion

  

week ревью 1

Week review 1 — находки multi-agent аудита проекта (2026-06-14)

  

wontfix

This will not be worked on

  

вторичка

Trade-In Estimator — проект оценки вторичного жилья

  

ИРД

Исходно-разрешительная документация — слой ограничений участка для analyze-таблицы

No labels

admin

analytics

auth

automation

bug

business

chore

ci

compliance

data

data-moat

docs

duplicate

dx

enhancement

Fable 5 ревью

feedback/max

generative

GG-форсайт

needs-discussion

needs-human

observability

pause-bots

performance

priority/p0

priority/p1

priority/p2

priority/p3

scope/backend

scope/db

scope/devops

scope/frontend

scope/qa

scrapers

security

site-finder

stage/1

stage/2

status/blocked

status/done

status/needs-analysis

status/needs-fix

status/qa

status/ready

status/review

status/wip

tech-debt

tradein

ux

week ревью 1

wontfix

вторичка

ИРД

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#1660

No description provided.