fix(site-finder): метка источника весов выводится из результата резолва, а не из входа (#2811) (#2817)
This commit is contained in:
parent
405d2f2eec
commit
1307d55da6
4 changed files with 200 additions and 25 deletions
|
|
@ -2189,12 +2189,21 @@ def analyze_parcel(
|
||||||
_effective_weights = {**_POI_WEIGHTS, **_inline_weights}
|
_effective_weights = {**_POI_WEIGHTS, **_inline_weights}
|
||||||
_weights_source = "inline"
|
_weights_source = "inline"
|
||||||
else:
|
else:
|
||||||
_effective_weights = _resolve_weights(db, user_id=profile_user_id, profile_id=profile_id)
|
# Метка — из РЕЗУЛЬТАТА резолва, не из того, что клиент прислал (#2811):
|
||||||
_weights_source = (
|
# profile_id мог не найтись (нет owner'а в запросе / чужой / удалён), и
|
||||||
"profile"
|
# тогда веса системные или дефолтные, а не профильные.
|
||||||
if profile_id is not None
|
_resolved = _resolve_weights(db, user_id=profile_user_id, profile_id=profile_id)
|
||||||
else ("user_default" if profile_user_id is not None else "system")
|
_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
|
# 4) Scoring: weighted sum с distance decay
|
||||||
score = 0.0
|
score = 0.0
|
||||||
|
|
@ -4085,9 +4094,12 @@ def analyze_parcel(
|
||||||
# (None когда вердикт позитивный / нет площади / считать нечего). caveat внутри.
|
# (None когда вердикт позитивный / нет площади / считать нечего). caveat внутри.
|
||||||
"program_alternatives": program_alternatives,
|
"program_alternatives": program_alternatives,
|
||||||
# #114/#201: кастомные веса POI — source + applied dict для прозрачности.
|
# #114/#201: кастомные веса POI — source + applied dict для прозрачности.
|
||||||
|
# source — что ФАКТИЧЕСКИ применилось; requested_profile_applied — был ли
|
||||||
|
# удовлетворён запрошенный profile_id (#2811). None = профиль не запрашивали.
|
||||||
"weights_profile": {
|
"weights_profile": {
|
||||||
"source": _weights_source,
|
"source": _weights_source,
|
||||||
"profile_id": profile_id,
|
"profile_id": profile_id,
|
||||||
|
"requested_profile_applied": _requested_profile_applied,
|
||||||
"user_id": profile_user_id,
|
"user_id": profile_user_id,
|
||||||
"weights_applied": _effective_weights,
|
"weights_applied": _effective_weights,
|
||||||
"inline_weights": _inline_weights,
|
"inline_weights": _inline_weights,
|
||||||
|
|
@ -4203,6 +4215,7 @@ def analyze_parcel(
|
||||||
"profile_user_id": profile_user_id,
|
"profile_user_id": profile_user_id,
|
||||||
"inline_weights": _inline_weights,
|
"inline_weights": _inline_weights,
|
||||||
"weights_source": _weights_source,
|
"weights_source": _weights_source,
|
||||||
|
"requested_profile_applied": _requested_profile_applied,
|
||||||
"x_session_id": _session_id,
|
"x_session_id": _session_id,
|
||||||
},
|
},
|
||||||
district=_district_name,
|
district=_district_name,
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,7 @@ API surface:
|
||||||
- create_profile(db, payload) → WeightProfile
|
- create_profile(db, payload) → WeightProfile
|
||||||
- update_profile(db, user_id, profile_id, payload) → WeightProfile | None
|
- update_profile(db, user_id, profile_id, payload) → WeightProfile | None
|
||||||
- delete_profile(db, user_id, profile_id) → bool
|
- 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
|
from __future__ import annotations
|
||||||
|
|
@ -19,7 +19,7 @@ import json
|
||||||
import logging
|
import logging
|
||||||
import math
|
import math
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from typing import Any
|
from typing import Any, NamedTuple
|
||||||
|
|
||||||
from pydantic import BaseModel, Field, field_validator
|
from pydantic import BaseModel, Field, field_validator
|
||||||
from sqlalchemy import text
|
from sqlalchemy import text
|
||||||
|
|
@ -346,13 +346,34 @@ def delete_profile(db: Any, user_id: str, profile_id: int) -> bool:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
|
||||||
def resolve_weights(db: Any, user_id: str | None, profile_id: int | None) -> dict[str, float]:
|
class ResolvedWeights(NamedTuple):
|
||||||
"""Вернуть эффективные веса для analyze_parcel.
|
"""Веса + КАКОЙ источник фактически применился (#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 задан → загрузить именно этот профиль
|
1. profile_id задан → загрузить именно этот профиль → source="profile"
|
||||||
2. user_id задан → загрузить default-профиль пользователя
|
2. user_id задан → загрузить default-профиль пользователя → source="user_default"
|
||||||
3. Иначе → вернуть системные значения _SYSTEM_POI_WEIGHTS
|
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:
|
if profile_id is not None and user_id is not None:
|
||||||
profile = get_profile(db, user_id, profile_id)
|
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(
|
logger.debug(
|
||||||
"resolve_weights: user=%s profile_id=%s → custom weights", user_id, profile_id
|
"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:
|
if user_id is not None:
|
||||||
profile = get_default_profile(db, user_id)
|
profile = get_default_profile(db, user_id)
|
||||||
if profile is not None and profile.weights:
|
if profile is not None and profile.weights:
|
||||||
logger.debug("resolve_weights: user=%s → default profile weights", user_id)
|
resolved = ResolvedWeights(dict(profile.weights), "user_default")
|
||||||
return dict(profile.weights)
|
|
||||||
|
|
||||||
logger.debug("resolve_weights: returning system defaults")
|
if profile_id is not None:
|
||||||
return dict(_SYSTEM_POI_WEIGHTS)
|
# Сюда попадаем, если запрошенный профиль не применился: 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:
|
finally:
|
||||||
app.dependency_overrides.clear()
|
app.dependency_overrides.clear()
|
||||||
_stop_patches()
|
_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 и profile_id → системные дефолты
|
||||||
- resolve_weights: user_id задан, default-профиль есть → его веса
|
- resolve_weights: user_id задан, default-профиль есть → его веса
|
||||||
- resolve_weights: profile_id задан → его веса
|
- resolve_weights: profile_id задан → его веса
|
||||||
- resolve_weights: профиль не найден → системные дефолты (fallback)
|
- resolve_weights: профиль не найден → системные дефолты (fallback) + source != profile
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import logging
|
||||||
from unittest.mock import MagicMock
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
@ -110,7 +111,8 @@ def test_resolve_weights_system_default() -> None:
|
||||||
"""Оба аргумента None → возвращаются системные веса."""
|
"""Оба аргумента None → возвращаются системные веса."""
|
||||||
db = MagicMock()
|
db = MagicMock()
|
||||||
result = resolve_weights(db, user_id=None, profile_id=None)
|
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 не должен вызываться вообще
|
||||||
db.execute.assert_not_called()
|
db.execute.assert_not_called()
|
||||||
|
|
||||||
|
|
@ -119,7 +121,7 @@ def test_resolve_weights_system_default_returns_copy() -> None:
|
||||||
"""Возвращается копия словаря, не ссылка на _SYSTEM_POI_WEIGHTS."""
|
"""Возвращается копия словаря, не ссылка на _SYSTEM_POI_WEIGHTS."""
|
||||||
db = MagicMock()
|
db = MagicMock()
|
||||||
result = resolve_weights(db, user_id=None, profile_id=None)
|
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
|
assert _SYSTEM_POI_WEIGHTS["school"] == 1.5
|
||||||
|
|
||||||
|
|
@ -156,7 +158,8 @@ def test_resolve_weights_uses_default_profile() -> None:
|
||||||
finally:
|
finally:
|
||||||
wp_module.get_default_profile = original
|
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:
|
def test_resolve_weights_uses_specific_profile() -> None:
|
||||||
|
|
@ -175,7 +178,8 @@ def test_resolve_weights_uses_specific_profile() -> None:
|
||||||
finally:
|
finally:
|
||||||
wp_module.get_profile = original
|
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:
|
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_profile = original_get
|
||||||
wp_module.get_default_profile = original_default
|
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:
|
def test_resolve_weights_empty_profile_weights_fallback() -> None:
|
||||||
|
|
@ -212,4 +218,52 @@ def test_resolve_weights_empty_profile_weights_fallback() -> None:
|
||||||
finally:
|
finally:
|
||||||
wp_module.get_default_profile = original_default
|
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