fix(site-finder): метка источника весов выводится из результата резолва, а не из входа (#2811) #2817
4 changed files with 200 additions and 25 deletions
|
|
@ -2189,12 +2189,21 @@ def analyze_parcel(
|
|||
_effective_weights = {**_POI_WEIGHTS, **_inline_weights}
|
||||
_weights_source = "inline"
|
||||
else:
|
||||
_effective_weights = _resolve_weights(db, user_id=profile_user_id, profile_id=profile_id)
|
||||
_weights_source = (
|
||||
"profile"
|
||||
if profile_id is not None
|
||||
else ("user_default" if profile_user_id is not None else "system")
|
||||
)
|
||||
# Метка — из РЕЗУЛЬТАТА резолва, не из того, что клиент прислал (#2811):
|
||||
# profile_id мог не найтись (нет owner'а в запросе / чужой / удалён), и
|
||||
# тогда веса системные или дефолтные, а не профильные.
|
||||
_resolved = _resolve_weights(db, user_id=profile_user_id, profile_id=profile_id)
|
||||
_effective_weights = _resolved.weights
|
||||
_weights_source = _resolved.source
|
||||
|
||||
# «Что просили» vs «что получилось»: profile_id echo'ит запрос, флаг говорит,
|
||||
# был ли запрос удовлетворён. Отдельное поле, а не подмена source на "system" —
|
||||
# иначе пропадёт разница «профиль не запрашивали» / «запрашивали, но не нашли».
|
||||
# None когда profile_id не передавали; False когда передали, но применилось
|
||||
# другое (не найден / чужой / перебит inline-весами).
|
||||
_requested_profile_applied: bool | None = (
|
||||
None if profile_id is None else _weights_source == "profile"
|
||||
)
|
||||
|
||||
# 4) Scoring: weighted sum с distance decay
|
||||
score = 0.0
|
||||
|
|
@ -4085,9 +4094,12 @@ def analyze_parcel(
|
|||
# (None когда вердикт позитивный / нет площади / считать нечего). caveat внутри.
|
||||
"program_alternatives": program_alternatives,
|
||||
# #114/#201: кастомные веса POI — source + applied dict для прозрачности.
|
||||
# source — что ФАКТИЧЕСКИ применилось; requested_profile_applied — был ли
|
||||
# удовлетворён запрошенный profile_id (#2811). None = профиль не запрашивали.
|
||||
"weights_profile": {
|
||||
"source": _weights_source,
|
||||
"profile_id": profile_id,
|
||||
"requested_profile_applied": _requested_profile_applied,
|
||||
"user_id": profile_user_id,
|
||||
"weights_applied": _effective_weights,
|
||||
"inline_weights": _inline_weights,
|
||||
|
|
@ -4203,6 +4215,7 @@ def analyze_parcel(
|
|||
"profile_user_id": profile_user_id,
|
||||
"inline_weights": _inline_weights,
|
||||
"weights_source": _weights_source,
|
||||
"requested_profile_applied": _requested_profile_applied,
|
||||
"x_session_id": _session_id,
|
||||
},
|
||||
district=_district_name,
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ API surface:
|
|||
- create_profile(db, payload) → WeightProfile
|
||||
- update_profile(db, user_id, profile_id, payload) → WeightProfile | None
|
||||
- delete_profile(db, user_id, profile_id) → bool
|
||||
- resolve_weights(db, user_id, profile_id) → dict[str, float]
|
||||
- resolve_weights(db, user_id, profile_id) → ResolvedWeights(weights, source)
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -19,7 +19,7 @@ import json
|
|||
import logging
|
||||
import math
|
||||
from datetime import datetime
|
||||
from typing import Any
|
||||
from typing import Any, NamedTuple
|
||||
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
from sqlalchemy import text
|
||||
|
|
@ -346,13 +346,34 @@ def delete_profile(db: Any, user_id: str, profile_id: int) -> bool:
|
|||
return True
|
||||
|
||||
|
||||
def resolve_weights(db: Any, user_id: str | None, profile_id: int | None) -> dict[str, float]:
|
||||
"""Вернуть эффективные веса для analyze_parcel.
|
||||
class ResolvedWeights(NamedTuple):
|
||||
"""Веса + КАКОЙ источник фактически применился (#2811).
|
||||
|
||||
Лестница приоритетов ниже по построению стирает разницу между «взял, что
|
||||
просили» и «не нашёл, взял что было» — а метка в ответе /analyze строится
|
||||
именно на этой разнице. Поэтому источник возвращается вместе с весами, а не
|
||||
выводится вызывающим из своих же входных параметров. NamedTuple, а не голый
|
||||
dict: старый вызов `w = resolve_weights(...); w["school"]` падает громко,
|
||||
молча «весами» этот объект не притворится.
|
||||
"""
|
||||
|
||||
weights: dict[str, float]
|
||||
source: str # "profile" | "user_default" | "system"
|
||||
|
||||
|
||||
def resolve_weights(db: Any, user_id: str | None, profile_id: int | None) -> ResolvedWeights:
|
||||
"""Вернуть эффективные веса для analyze_parcel + фактический их источник.
|
||||
|
||||
Порядок приоритетов:
|
||||
1. profile_id задан → загрузить именно этот профиль
|
||||
2. user_id задан → загрузить default-профиль пользователя
|
||||
3. Иначе → вернуть системные значения _SYSTEM_POI_WEIGHTS
|
||||
1. profile_id задан → загрузить именно этот профиль → source="profile"
|
||||
2. user_id задан → загрузить default-профиль пользователя → source="user_default"
|
||||
3. Иначе → системные значения _SYSTEM_POI_WEIGHTS → source="system"
|
||||
|
||||
Запрошенный, но НЕ применённый profile_id — не тишина: warning с
|
||||
идентификаторами (см. ниже). HTTP-статус на этом не меняем: profile_id для
|
||||
/analyze — необязательный модификатор, а не адресуемый ресурс; 404 превратил
|
||||
бы гонку «профиль удалили между списком и анализом» в отказ вместо честно
|
||||
помеченного ответа. Клиенту хватает source + requested_profile_not_found.
|
||||
"""
|
||||
if profile_id is not None and user_id is not None:
|
||||
profile = get_profile(db, user_id, profile_id)
|
||||
|
|
@ -360,13 +381,26 @@ def resolve_weights(db: Any, user_id: str | None, profile_id: int | None) -> dic
|
|||
logger.debug(
|
||||
"resolve_weights: user=%s profile_id=%s → custom weights", user_id, profile_id
|
||||
)
|
||||
return dict(profile.weights)
|
||||
return ResolvedWeights(dict(profile.weights), "profile")
|
||||
|
||||
resolved = ResolvedWeights(dict(_SYSTEM_POI_WEIGHTS), "system")
|
||||
if user_id is not None:
|
||||
profile = get_default_profile(db, user_id)
|
||||
if profile is not None and profile.weights:
|
||||
logger.debug("resolve_weights: user=%s → default profile weights", user_id)
|
||||
return dict(profile.weights)
|
||||
resolved = ResolvedWeights(dict(profile.weights), "user_default")
|
||||
|
||||
logger.debug("resolve_weights: returning system defaults")
|
||||
return dict(_SYSTEM_POI_WEIGHTS)
|
||||
if profile_id is not None:
|
||||
# Сюда попадаем, если запрошенный профиль не применился: owner не передан
|
||||
# (первая ветка требует ОБА аргумента), профиль чужой/удалён, либо weights
|
||||
# пустые. Раньше это был logger.debug, которого на проде нет, — и оценка
|
||||
# молча считалась не по тем весам (#2811, ранее #2788).
|
||||
logger.warning(
|
||||
"resolve_weights: запрошенный profile_id=%s (user_id=%r) НЕ применён — "
|
||||
"фактический источник весов %r",
|
||||
profile_id,
|
||||
user_id,
|
||||
resolved.source,
|
||||
)
|
||||
else:
|
||||
logger.debug("resolve_weights: источник весов %s", resolved.source)
|
||||
return resolved
|
||||
|
|
|
|||
|
|
@ -316,3 +316,77 @@ def test_analyze_inline_weights_beats_profile_id() -> None:
|
|||
finally:
|
||||
app.dependency_overrides.clear()
|
||||
_stop_patches()
|
||||
|
||||
|
||||
def test_analyze_missing_profile_is_not_labelled_profile() -> None:
|
||||
"""#2811: profile_id задан, профиль НЕ найден → метка НЕ смеет быть 'profile'.
|
||||
|
||||
Три способа промахнуться мимо профиля (все три воспроизведены живым запросом
|
||||
на проде 2026-08-10): owner не передан вовсе, чужой профиль, удалённый id.
|
||||
В mock-БД профилей нет — значит применились системные веса, и ответ обязан
|
||||
это признать, а не утверждать, что считал по профилю.
|
||||
"""
|
||||
from app.core.db import get_db
|
||||
from app.services.site_finder.weight_profiles import _SYSTEM_POI_WEIGHTS
|
||||
|
||||
for qs in ("profile_id=999999", "profile_id=999999&profile_user_id=nobody"):
|
||||
db = _make_db_for_analyze() # профилей нет → get_profile/get_default_profile → None
|
||||
app.dependency_overrides[get_db] = _override_db(db)
|
||||
_start_patches()
|
||||
try:
|
||||
client = TestClient(app)
|
||||
resp = client.post(f"/api/v1/parcels/{_CAD}/analyze?{qs}")
|
||||
assert resp.status_code == 200, resp.text
|
||||
wp = resp.json()["weights_profile"]
|
||||
# sanity: веса и правда системные, промах реальный
|
||||
assert wp["weights_applied"]["tram_stop"] == pytest.approx(
|
||||
_SYSTEM_POI_WEIGHTS["tram_stop"]
|
||||
)
|
||||
assert wp["source"] != "profile", (
|
||||
f"?{qs}: применились системные веса, а метка source='profile' — "
|
||||
"ответ утверждает то, чего не было (#2811)"
|
||||
)
|
||||
assert wp["source"] == "system"
|
||||
# «что просили» не теряется: запрошенный id + явный признак промаха
|
||||
assert wp["profile_id"] == 999999
|
||||
assert wp["requested_profile_applied"] is False
|
||||
finally:
|
||||
app.dependency_overrides.clear()
|
||||
_stop_patches()
|
||||
|
||||
|
||||
def test_analyze_found_profile_keeps_label_and_flag() -> None:
|
||||
"""Обратная сторона: профиль найден → source='profile', флаг промаха False."""
|
||||
from datetime import UTC, datetime
|
||||
|
||||
import app.services.site_finder.weight_profiles as wp_module
|
||||
from app.core.db import get_db
|
||||
from app.services.site_finder.weight_profiles import WeightProfile
|
||||
|
||||
profile = WeightProfile(
|
||||
id=7,
|
||||
user_id="user-1",
|
||||
profile_name="test",
|
||||
weights={"tram_stop": -0.4},
|
||||
is_default=False,
|
||||
description=None,
|
||||
created_at=datetime.now(UTC),
|
||||
updated_at=datetime.now(UTC),
|
||||
)
|
||||
db = _make_db_for_analyze()
|
||||
app.dependency_overrides[get_db] = _override_db(db)
|
||||
_start_patches()
|
||||
original = wp_module.get_profile
|
||||
wp_module.get_profile = lambda _db, uid, pid: profile
|
||||
try:
|
||||
client = TestClient(app)
|
||||
resp = client.post(f"/api/v1/parcels/{_CAD}/analyze?profile_id=7&profile_user_id=user-1")
|
||||
assert resp.status_code == 200, resp.text
|
||||
wp = resp.json()["weights_profile"]
|
||||
assert wp["source"] == "profile"
|
||||
assert wp["requested_profile_applied"] is True
|
||||
assert wp["weights_applied"]["tram_stop"] == pytest.approx(-0.4)
|
||||
finally:
|
||||
wp_module.get_profile = original
|
||||
app.dependency_overrides.clear()
|
||||
_stop_patches()
|
||||
|
|
|
|||
|
|
@ -8,11 +8,12 @@ Mock-based — без реальной БД. Проверяет:
|
|||
- resolve_weights: нет user_id и profile_id → системные дефолты
|
||||
- resolve_weights: user_id задан, default-профиль есть → его веса
|
||||
- resolve_weights: profile_id задан → его веса
|
||||
- resolve_weights: профиль не найден → системные дефолты (fallback)
|
||||
- resolve_weights: профиль не найден → системные дефолты (fallback) + source != profile
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
|
@ -110,7 +111,8 @@ def test_resolve_weights_system_default() -> None:
|
|||
"""Оба аргумента None → возвращаются системные веса."""
|
||||
db = MagicMock()
|
||||
result = resolve_weights(db, user_id=None, profile_id=None)
|
||||
assert result == _SYSTEM_POI_WEIGHTS
|
||||
assert result.weights == _SYSTEM_POI_WEIGHTS
|
||||
assert result.source == "system"
|
||||
# db не должен вызываться вообще
|
||||
db.execute.assert_not_called()
|
||||
|
||||
|
|
@ -119,7 +121,7 @@ def test_resolve_weights_system_default_returns_copy() -> None:
|
|||
"""Возвращается копия словаря, не ссылка на _SYSTEM_POI_WEIGHTS."""
|
||||
db = MagicMock()
|
||||
result = resolve_weights(db, user_id=None, profile_id=None)
|
||||
result["school"] = 999.0
|
||||
result.weights["school"] = 999.0
|
||||
# Оригинал не изменён
|
||||
assert _SYSTEM_POI_WEIGHTS["school"] == 1.5
|
||||
|
||||
|
|
@ -156,7 +158,8 @@ def test_resolve_weights_uses_default_profile() -> None:
|
|||
finally:
|
||||
wp_module.get_default_profile = original
|
||||
|
||||
assert result == custom_weights
|
||||
assert result.weights == custom_weights
|
||||
assert result.source == "user_default"
|
||||
|
||||
|
||||
def test_resolve_weights_uses_specific_profile() -> None:
|
||||
|
|
@ -175,7 +178,8 @@ def test_resolve_weights_uses_specific_profile() -> None:
|
|||
finally:
|
||||
wp_module.get_profile = original
|
||||
|
||||
assert result == custom_weights
|
||||
assert result.weights == custom_weights
|
||||
assert result.source == "profile"
|
||||
|
||||
|
||||
def test_resolve_weights_profile_not_found_fallback() -> None:
|
||||
|
|
@ -194,7 +198,9 @@ def test_resolve_weights_profile_not_found_fallback() -> None:
|
|||
wp_module.get_profile = original_get
|
||||
wp_module.get_default_profile = original_default
|
||||
|
||||
assert result == _SYSTEM_POI_WEIGHTS
|
||||
assert result.weights == _SYSTEM_POI_WEIGHTS
|
||||
# #2811: главное — источник НЕ выдаёт себя за профиль, которого не нашли
|
||||
assert result.source == "system"
|
||||
|
||||
|
||||
def test_resolve_weights_empty_profile_weights_fallback() -> None:
|
||||
|
|
@ -212,4 +218,52 @@ def test_resolve_weights_empty_profile_weights_fallback() -> None:
|
|||
finally:
|
||||
wp_module.get_default_profile = original_default
|
||||
|
||||
assert result == _SYSTEM_POI_WEIGHTS
|
||||
assert result.weights == _SYSTEM_POI_WEIGHTS
|
||||
assert result.source == "system"
|
||||
|
||||
|
||||
def test_resolve_weights_profile_id_without_owner_is_not_profile(
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""#2811 сценарий 1: profile_id есть, user_id нет → первая ветка не выполняется.
|
||||
|
||||
Ровно это жило на проде: ран analysis_runs #4000 от 2026-08-07 —
|
||||
source='profile', profile_id=1, а tram_stop=-0.5 (системный, у профиля 1 он
|
||||
-0.4). Метка обязана быть 'system', а промах — попасть в warning.
|
||||
"""
|
||||
db = MagicMock()
|
||||
with caplog.at_level(logging.WARNING, logger="app.services.site_finder.weight_profiles"):
|
||||
result = resolve_weights(db, user_id=None, profile_id=1)
|
||||
|
||||
assert result.source == "system"
|
||||
assert result.weights == _SYSTEM_POI_WEIGHTS
|
||||
assert "profile_id=1" in caplog.text
|
||||
db.execute.assert_not_called() # профиль даже не искали
|
||||
|
||||
|
||||
def test_resolve_weights_missing_profile_falls_to_user_default_not_profile(
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""#2811 сценарий 3: profile_id не найден, но у юзера есть default-профиль.
|
||||
|
||||
Худший вариант: веса НЕ системные, поэтому по значениям подмена вообще не
|
||||
видна. Метка должна сказать 'user_default', а не 'profile'.
|
||||
"""
|
||||
import app.services.site_finder.weight_profiles as wp_module
|
||||
|
||||
default_profile = _make_profile_mock({"school": 2.0})
|
||||
db = MagicMock()
|
||||
original_get = wp_module.get_profile
|
||||
original_default = wp_module.get_default_profile
|
||||
wp_module.get_profile = lambda _db, uid, pid: None
|
||||
wp_module.get_default_profile = lambda _db, uid: default_profile
|
||||
try:
|
||||
with caplog.at_level(logging.WARNING, logger="app.services.site_finder.weight_profiles"):
|
||||
result = resolve_weights(db, user_id="user-1", profile_id=999)
|
||||
finally:
|
||||
wp_module.get_profile = original_get
|
||||
wp_module.get_default_profile = original_default
|
||||
|
||||
assert result.source == "user_default"
|
||||
assert result.weights == {"school": 2.0}
|
||||
assert "profile_id=999" in caplog.text
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue