From 3945d54e3babc7fe046ad26f0252baa645aebb9d Mon Sep 17 00:00:00 2001 From: Light1YT Date: Thu, 4 Jun 2026 11:49:41 +0500 Subject: [PATCH] fix(#945): indicator-aware default region in macro reader + document debt data-quality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FOLLOW-UP (Leha: not a bug, follow-up). get_macro_series / get_latest_macro defaulted region='rf', but CBR mortgage_* series live under 'sverdl' in macro_indicator (key_rate under 'rf'). Only caller passes region explicitly so no active leak, but a future caller omitting region would silently get None — latent footgun. region now str|None; when None → _canonical_region (mortgage set sourced from _CBR_SERIES_MAP, single source of truth). Explicit region always wins → existing callers unchanged. Region-binding tests added. 159 tests pass. Documented (NOT code-fixed — upstream data-quality): mortgage_debt / mortgage_overdue stay empty in macro_indicator because cbr_mortgage_series.period holds value-like garbage (e.g. '10054588.0') for the Debt series — corruption from the upstream CBR-XLSX scraper that built domrf.db (imported verbatim by 44_import_anton_db.py), NOT an ingest bug. 123_macro_indicator.sql correctly skips unparseable periods. Needs a separate scraper re-ingest (idempotent backfill once period is valid). Refs #945 --- backend/app/services/site_finder/macro.py | 46 ++++++++- .../tests/services/site_finder/test_macro.py | 94 +++++++++++++++++-- 2 files changed, 127 insertions(+), 13 deletions(-) diff --git a/backend/app/services/site_finder/macro.py b/backend/app/services/site_finder/macro.py index dde3ab37..96b96ec0 100644 --- a/backend/app/services/site_finder/macro.py +++ b/backend/app/services/site_finder/macro.py @@ -59,25 +59,56 @@ def classify_cbr_series(series_id: str | None) -> tuple[str, str] | None: return _CBR_SERIES_MAP.get(series_id) +# ── Canonical region per indicator_type ─────────────────────────────────────────── +# Реальная раскладка данных ЦБ в macro_indicator (проверено на prod): +# • семейство CBR-ипотеки (формы 02, ИЖК) лежит под region='sverdl' — оно +# бэкфиллится из cbr_mortgage_series, где единственный регион — 'sverdl' +# (см. 123_macro_indicator.sql §2: region берётся из источника КАК ЕСТЬ); +# • key_rate (и прочие национальные ряды) — под region='rf'. +# Это НЕ опечатка в данных, а фактический layout. Поэтому дефолтный регион ридера +# должен быть indicator-aware: иначе чтение mortgage_*-ряда без явного region молча +# вернёт пусто (footgun, #945 follow-up). Множество ипотечных типов берём как +# values() из _CBR_SERIES_MAP — единый источник правды, синхронный с миграцией. +_MORTGAGE_INDICATOR_TYPES: frozenset[str] = frozenset( + itype for itype, _unit in _CBR_SERIES_MAP.values() +) + + +def _canonical_region(indicator_type: str) -> str: + """Канонический region для indicator_type, когда вызыватель не задал явный. + + Детерминированно, без БД. 6 CBR-ипотечных типов (mortgage_*) → 'sverdl' + (так они лежат в macro_indicator на prod); всё остальное (key_rate и пр.) → 'rf'. + Источник истины по ипотечному множеству — _CBR_SERIES_MAP (зеркало миграции). + """ + return "sverdl" if indicator_type in _MORTGAGE_INDICATOR_TYPES else "rf" + + # ── Reader ────────────────────────────────────────────────────────────────────── + def get_macro_series( db: Session, indicator_type: str, - region: str = "rf", + region: str | None = None, since: date | None = None, ) -> list[tuple[date, float]]: """Весь ряд (obs_date, value) для (indicator_type, region), ASC по obs_date. Args: db: SQLAlchemy sync Session. - indicator_type: напр. 'mortgage_rate_weighted', 'key_rate' (key_rate заполнит PR B). - region: 'rf' | 'sverdl' | ... (по умолчанию 'rf'). + indicator_type: напр. 'mortgage_rate_weighted', 'key_rate'. + region: 'rf' | 'sverdl' | ... Если None — резолвится indicator-aware дефолт + через _canonical_region (CBR-ипотека → 'sverdl', остальное → 'rf'), + т.к. на prod эти семейства лежат в РАЗНЫХ регионах. Явный region + ВСЕГДА выигрывает (вызыватель может переопределить дефолт). since: если задано — только obs_date >= since. Returns: Список (obs_date, value) по возрастанию даты; строки с NULL value отброшены. """ + if region is None: + region = _canonical_region(indicator_type) rows = db.execute( text( """ @@ -98,18 +129,23 @@ def get_macro_series( def get_latest_macro( db: Session, indicator_type: str, - region: str = "rf", + region: str | None = None, ) -> tuple[date, float] | None: """Последняя (по obs_date) непустая точка ряда или None, если ряда нет. Args: db: SQLAlchemy sync Session. indicator_type: тип показателя. - region: регион (по умолчанию 'rf'). + region: 'rf' | 'sverdl' | ... Если None — резолвится indicator-aware дефолт + через _canonical_region (CBR-ипотека → 'sverdl', остальное → 'rf'): + на prod эти семейства лежат в РАЗНЫХ регионах. Явный region ВСЕГДА + выигрывает (вызыватель может переопределить дефолт). Returns: (obs_date, value) самой свежей точки, либо None. """ + if region is None: + region = _canonical_region(indicator_type) row = db.execute( text( """ diff --git a/backend/tests/services/site_finder/test_macro.py b/backend/tests/services/site_finder/test_macro.py index 1c3801e3..26156d95 100644 --- a/backend/tests/services/site_finder/test_macro.py +++ b/backend/tests/services/site_finder/test_macro.py @@ -18,6 +18,7 @@ from unittest.mock import MagicMock from app.services.site_finder.macro import ( _CBR_SERIES_MAP, + _canonical_region, classify_cbr_series, get_latest_macro, get_macro_series, @@ -25,10 +26,7 @@ from app.services.site_finder.macro import ( # Путь к миграции — для теста-консистентности зеркала. _MIGRATION = ( - pathlib.Path(__file__).resolve().parents[4] - / "data" - / "sql" - / "123_macro_indicator.sql" + pathlib.Path(__file__).resolve().parents[4] / "data" / "sql" / "123_macro_indicator.sql" ) @@ -94,6 +92,81 @@ class TestClassifyCbrSeries: assert classify_cbr_series("02_99_unknown_series") is None +# ── canonical region resolution (#945 footgun) ──────────────────────────────── + + +class TestCanonicalRegion: + """Indicator-aware дефолт региона: prod-layout = CBR-ипотека под 'sverdl', + key_rate / прочее под 'rf' (см. 123_macro_indicator.sql §2).""" + + def test_all_six_mortgage_indicators_resolve_sverdl(self) -> None: + # Источник истины — _CBR_SERIES_MAP (зеркало миграции): все 6 mortgage_*. + mortgage_types = {itype for itype, _unit in _CBR_SERIES_MAP.values()} + assert len(mortgage_types) == 6 # guard: набор не «усох» + for itype in mortgage_types: + assert _canonical_region(itype) == "sverdl", itype + + def test_key_rate_resolves_rf(self) -> None: + assert _canonical_region("key_rate") == "rf" + + def test_unknown_indicator_defaults_rf(self) -> None: + # Любой не-ипотечный / неизвестный тип → 'rf' (национальный дефолт). + assert _canonical_region("cpi") == "rf" + assert _canonical_region("mortgage_rate_primary_domrf") == "rf" # это region='rf' + assert _canonical_region("totally_unknown") == "rf" + + +# ── reader region resolution end-to-end (mock Session) ───────────────────────── + + +class TestReaderRegionResolution: + """Какой region реально биндится в SQL ридером при/без явного region. + + Покрывает контракт #945: дефолт indicator-aware, явный region — выигрывает. + """ + + def _bound_region(self, db: MagicMock) -> str: + _args, _kwargs = db.execute.call_args + return _args[1]["region"] + + def test_series_mortgage_no_region_binds_sverdl(self) -> None: + db = _mock_db([]) + get_macro_series(db, "mortgage_rate_weighted") + assert self._bound_region(db) == "sverdl" + + def test_series_key_rate_no_region_binds_rf(self) -> None: + db = _mock_db([]) + get_macro_series(db, "key_rate") + assert self._bound_region(db) == "rf" + + def test_series_explicit_region_overrides_default(self) -> None: + # Явный 'rf' для ипотечного типа — caller override выигрывает над 'sverdl'. + db = _mock_db([]) + get_macro_series(db, "mortgage_debt", region="rf") + assert self._bound_region(db) == "rf" + + def test_series_existing_sverdl_caller_unchanged(self) -> None: + # Регресс на текущего вызывателя (macro_series.py:372 region="sverdl"). + db = _mock_db([]) + get_macro_series(db, "mortgage_issued_count", region="sverdl") + assert self._bound_region(db) == "sverdl" + + def test_latest_mortgage_no_region_binds_sverdl(self) -> None: + db = _mock_db([]) + get_latest_macro(db, "mortgage_overdue") + assert self._bound_region(db) == "sverdl" + + def test_latest_key_rate_no_region_binds_rf(self) -> None: + db = _mock_db([]) + get_latest_macro(db, "key_rate") + assert self._bound_region(db) == "rf" + + def test_latest_explicit_region_overrides_default(self) -> None: + db = _mock_db([]) + get_latest_macro(db, "mortgage_term", region="rf") + assert self._bound_region(db) == "rf" + + # ── консистентность Python-зеркала с SQL-миграцией ──────────────────────────── @@ -162,13 +235,18 @@ class TestGetMacroSeries: db = _mock_db([]) assert get_macro_series(db, "key_rate") == [] - def test_default_region_and_params(self) -> None: + def test_default_region_is_indicator_aware(self) -> None: + """No region → mortgage_* резолвится в 'sverdl' (его prod-layout), не 'rf'. + + Регресс на #945-footgun: раньше дефолт был жёстко 'rf' → mortgage_*-ряд + молча возвращал пусто. Проверяем РЕАЛЬНО забинженный в запрос region. + """ db = _mock_db([]) get_macro_series(db, "mortgage_debt") - _args, kwargs = db.execute.call_args - params = _args[1] if len(_args) > 1 else kwargs + _args, _kwargs = db.execute.call_args + params = _args[1] assert params["itype"] == "mortgage_debt" - assert params["region"] == "rf" + assert params["region"] == "sverdl" assert params["since"] is None def test_since_passed_through(self) -> None: