[HIGH] site-finder: weights_profile.source = "profile" ставится по факту параметра, а не по факту найденного профиля #2811

Closed
opened 2026-08-10 08:50:27 +00:00 by bot-backend · 2 comments
Collaborator

Найдено при разборе #2790, подтверждено чтением origin/main. Фронт после PR #2810 под это больше не подставляется, но источник лжи жив и доступен любому вызывающему API.

Метка ставится по факту передачи параметра, а не по факту нахождения профиля

backend/app/api/v1/parcels.py:2192-2196:

_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")
)

Условие — profile_id is not None, то есть «параметр пришёл». Нашёлся ли профиль, никого не спрашивают.

А resolve_weights (backend/app/services/site_finder/weight_profiles.py:349-372) при ненайденном профиле молча уходит вниз по лестнице приоритетов и в пределе возвращает _SYSTEM_POI_WEIGHTS:

if profile_id is not None and user_id is not None:
    profile = get_profile(db, user_id, profile_id)
    if profile is not None and profile.weights:
        return dict(profile.weights)          # ← нашёлся
    # не нашёлся — проваливаемся дальше, БЕЗ следа
if user_id is not None:
    profile = get_default_profile(db, user_id)
    ...
return dict(_SYSTEM_POI_WEIGHTS)

Единственный след — logger.debug, который на проде не пишется.

Что видит потребитель

Ответ /analyze отдаёт weights_profile.source = "profile" (parcels.py:4089, дублируется в 4205) — то есть утверждает, что оценка посчитана по профилю, при том что она посчитана по системным весам.

Три способа получить это без единой ошибки в ответе:

  1. profile_id есть, profile_user_id нет → первая ветка не выполняется вовсе (условие требует оба), падаем на системные, метка profile;
  2. profile_id указывает на чужой профиль → get_profile scoped к владельцу, не найдёт, метка profile;
  3. профиль удалён между показом списка и запросом → то же самое.

Случай 1 — не гипотетический: ровно так и было на проде до PR #2788, запрос анализа слал profile_id без владельца, и вес трамвайной остановки приходил −0.5 вместо −0.4 из профиля. Тогда починили вызывающего. Сам механизм остался.

Почему это стоит починить, а не оставить

Это ровно тема эпика #2674: метка утверждает то, чего не было, и её нельзя опровергнуть по ответу — веса в ответе выглядят законно, просто они не те. Пользователь видит ползунки одного профиля и оценку по другим; ошибку заметить нечем.

Отдельно неприятно, что resolve_weights устроен как лестница «попробуй это, иначе то, иначе дефолт». Такая форма удобна, но по построению стирает разницу между «взял, что просили» и «не нашёл, взял что было» — а для метки нужна именно эта разница.

Что предлагается

  1. resolve_weights возвращает не только веса, но и что фактически применилось (найден ли запрошенный профиль, или сработал запасной вариант). Тогда метка выводится из результата, а не из входа.
  2. Ненайденный запрошенный профиль — это не «молча вернём дефолты». Как минимум logger.warning с идентификаторами; возможно 404/422, но это меняет контракт — решать отдельно.
  3. Тест обязан краснеть на случае «profile_id задан, профиль не найден, метка profile» — сейчас такого теста нет, иначе дефект не дожил бы.

Чего НЕ делать

Не «чинить» подстановкой source = "system" во всех сомнительных случаях — потеряется различие между «запрошен профиль и применён» и «профиль не запрашивали». Нужны обе величины: что просили и что получилось.

Связано: #2790 (где нашлось), #2788 (где чинили вызывающего), #2674 (эпик).

Найдено при разборе #2790, подтверждено чтением `origin/main`. Фронт после PR #2810 под это больше не подставляется, но **источник лжи жив** и доступен любому вызывающему API. ## Метка ставится по факту передачи параметра, а не по факту нахождения профиля `backend/app/api/v1/parcels.py:2192-2196`: ```python _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") ) ``` Условие — `profile_id is not None`, то есть «параметр пришёл». Нашёлся ли профиль, никого не спрашивают. А `resolve_weights` (`backend/app/services/site_finder/weight_profiles.py:349-372`) при ненайденном профиле **молча уходит вниз по лестнице приоритетов** и в пределе возвращает `_SYSTEM_POI_WEIGHTS`: ```python if profile_id is not None and user_id is not None: profile = get_profile(db, user_id, profile_id) if profile is not None and profile.weights: return dict(profile.weights) # ← нашёлся # не нашёлся — проваливаемся дальше, БЕЗ следа if user_id is not None: profile = get_default_profile(db, user_id) ... return dict(_SYSTEM_POI_WEIGHTS) ``` Единственный след — `logger.debug`, который на проде не пишется. ## Что видит потребитель Ответ `/analyze` отдаёт `weights_profile.source = "profile"` (`parcels.py:4089`, дублируется в `4205`) — то есть **утверждает, что оценка посчитана по профилю**, при том что она посчитана по системным весам. Три способа получить это без единой ошибки в ответе: 1. `profile_id` есть, `profile_user_id` нет → первая ветка не выполняется вовсе (условие требует оба), падаем на системные, метка `profile`; 2. `profile_id` указывает на чужой профиль → `get_profile` scoped к владельцу, не найдёт, метка `profile`; 3. профиль удалён между показом списка и запросом → то же самое. Случай 1 — не гипотетический: **ровно так и было на проде до PR #2788**, запрос анализа слал `profile_id` без владельца, и вес трамвайной остановки приходил `−0.5` вместо `−0.4` из профиля. Тогда починили вызывающего. Сам механизм остался. ## Почему это стоит починить, а не оставить Это ровно тема эпика #2674: метка утверждает то, чего не было, и **её нельзя опровергнуть по ответу** — веса в ответе выглядят законно, просто они не те. Пользователь видит ползунки одного профиля и оценку по другим; ошибку заметить нечем. Отдельно неприятно, что `resolve_weights` устроен как лестница «попробуй это, иначе то, иначе дефолт». Такая форма удобна, но по построению **стирает разницу между «взял, что просили» и «не нашёл, взял что было»** — а для метки нужна именно эта разница. ## Что предлагается 1. `resolve_weights` возвращает не только веса, но и **что фактически применилось** (найден ли запрошенный профиль, или сработал запасной вариант). Тогда метка выводится из результата, а не из входа. 2. Ненайденный запрошенный профиль — это **не «молча вернём дефолты»**. Как минимум `logger.warning` с идентификаторами; возможно `404`/`422`, но это меняет контракт — решать отдельно. 3. Тест обязан **краснеть** на случае «`profile_id` задан, профиль не найден, метка `profile`» — сейчас такого теста нет, иначе дефект не дожил бы. ## Чего НЕ делать Не «чинить» подстановкой `source = "system"` во всех сомнительных случаях — потеряется различие между «запрошен профиль и применён» и «профиль не запрашивали». Нужны обе величины: что просили и что получилось. Связано: #2790 (где нашлось), #2788 (где чинили вызывающего), #2674 (эпик).
Author
Collaborator

Починено в PR #2817 (смержен в main 2026-08-10 10:34 UTC).

Постановка подтверждена целиком — живым запросом изнутри прод-контейнера на участке 66:41:0204016:10, все три сценария:

запрос source в ответе tram_stop применённый что применилось на деле
?profile_id=1 (owner не передан) profile -0.5 системные веса
?profile_id=1&profile_user_id=__system__ (профиль 1 принадлежит admin) profile -0.5 системные веса
?profile_id=999999&profile_user_id=admin profile -0.4 default-профиль admin (id=1)

У профиля 1 tram_stop = -0.4, у системных -0.5; остальные 10 категорий совпадают — ровно тот разрыв, что видели в #2788.

Уточнение к тексту issue. «В пределе возвращает _SYSTEM_POI_WEIGHTS» верно не всегда: в сценарии 3 промах уходит не на системные, а на чужой default-профиль пользователя. Это хуже, а не мягче: веса выглядят как настроенные, по значениям подмена не видна вообще, а метка при этом называет id, которого не существует.

Замер истории (прод). Из 4071 рана analysis_runs метку profile носили 3 (все 2026-08-07). Один из них — id=4000, profile_id=1, profile_user_id=NULL, weights_applied.tram_stop = -0.5 — посчитан системными весами. Различить задним числом получилось, потому что снимок weights_applied лежит в result, а веса профилей ни разу не редактировались с момента создания (created_at = updated_at у всех 4). Если бы профиль правили после рана — или если бы его веса совпали с системными — сравнение было бы неразрешимым.

Что сделано (детали в PR): resolve_weightsResolvedWeights(weights, source); промах пишется logger.warning с идентификаторами; в ответе появился weights_profile.requested_profile_applied (True/False/None) — чтобы «что просили» и «что получилось» остались разными величинами.

Про 404/422 — решено НЕ менять код ответа. profile_id для /analyze это необязательный модификатор, а не адресуемый ресурс: 404 на /parcels/{cad}/analyze уже занят «нет геометрии», а 422 превратил бы штатную гонку «профиль удалили между показом списка и анализом» в отказ вместо честно помеченного ответа. Врала метка, а не сам fallback — чинили метку.

Кто читает метку: никто не строит на ней видимый пользователю текст. Фронт weights_profile.source не рендерит (три упоминания — комментарии-предупреждения «врёт», #2782/#2810), в §19-allowlist чата её нет, PDF/DOCX/экспортёры не читают. Единственные потребители — сырой ответ API и analysis_runs.params.weights_source. Видимых изменений в UI не будет; фронтовые комментарии после этого PR устарели — снять их отдельным follow-up.

Починено в PR #2817 (смержен в main 2026-08-10 10:34 UTC). **Постановка подтверждена целиком** — живым запросом изнутри прод-контейнера на участке `66:41:0204016:10`, все три сценария: | запрос | `source` в ответе | `tram_stop` применённый | что применилось на деле | |---|---|---|---| | `?profile_id=1` (owner не передан) | `profile` | `-0.5` | системные веса | | `?profile_id=1&profile_user_id=__system__` (профиль 1 принадлежит `admin`) | `profile` | `-0.5` | системные веса | | `?profile_id=999999&profile_user_id=admin` | `profile` | `-0.4` | **default-профиль `admin` (id=1)** | У профиля 1 `tram_stop = -0.4`, у системных `-0.5`; остальные 10 категорий совпадают — ровно тот разрыв, что видели в #2788. **Уточнение к тексту issue.** «В пределе возвращает `_SYSTEM_POI_WEIGHTS`» верно не всегда: в сценарии 3 промах уходит не на системные, а на **чужой default-профиль пользователя**. Это хуже, а не мягче: веса выглядят как настроенные, по значениям подмена не видна вообще, а метка при этом называет id, которого не существует. **Замер истории (прод).** Из 4071 рана `analysis_runs` метку `profile` носили **3** (все 2026-08-07). Один из них — `id=4000`, `profile_id=1`, `profile_user_id=NULL`, `weights_applied.tram_stop = -0.5` — посчитан **системными** весами. Различить задним числом получилось, потому что снимок `weights_applied` лежит в `result`, а веса профилей **ни разу не редактировались** с момента создания (`created_at = updated_at` у всех 4). Если бы профиль правили после рана — или если бы его веса совпали с системными — сравнение было бы неразрешимым. **Что сделано** (детали в PR): `resolve_weights` → `ResolvedWeights(weights, source)`; промах пишется `logger.warning` с идентификаторами; в ответе появился `weights_profile.requested_profile_applied` (True/False/None) — чтобы «что просили» и «что получилось» остались разными величинами. **Про `404`/`422` — решено НЕ менять код ответа.** `profile_id` для `/analyze` это необязательный модификатор, а не адресуемый ресурс: `404` на `/parcels/{cad}/analyze` уже занят «нет геометрии», а `422` превратил бы штатную гонку «профиль удалили между показом списка и анализом» в отказ вместо честно помеченного ответа. Врала метка, а не сам fallback — чинили метку. **Кто читает метку:** никто не строит на ней видимый пользователю текст. Фронт `weights_profile.source` не рендерит (три упоминания — комментарии-предупреждения «врёт», #2782/#2810), в §19-allowlist чата её нет, PDF/DOCX/экспортёры не читают. Единственные потребители — сырой ответ API и `analysis_runs.params.weights_source`. Видимых изменений в UI не будет; фронтовые комментарии после этого PR устарели — снять их отдельным follow-up.
Author
Collaborator

Поправка к моей шапке: промах падает не на системные веса, а на ЧУЖОЙ профиль — это хуже

Я написал: «resolve_weights при ненайденном профиле молча уходит вниз по лестнице и в пределе возвращает системные веса». Верно только для части случаев. Проверил лестницу по коду и живыми запросами — три сценария дают три разных исхода:

запрос метка была что применялось на деле
profile_id=1, владелец не передан profile системные веса
profile_id=1, чужой владелец profile системные веса
profile_id=999999, владелец admin profile default-профиль этого пользователя

Третий случай — тот, что я описал неверно, и он опаснее двух первых. Веса выглядят настроенными; по значениям подмена не видна вообще (это чей-то реальный профиль, а не дефолты); а метка при этом называет profile_id, которого не существует.

Отсюда прямое следствие для починки: подставлять source="system" во всех сомнительных случаях было бы вторым враньём, а не половинчатым фиксом. Хорошо, что этого не сделали.

Замер истории: различить задним числом МОЖНО, и ложь случилась один раз

Из 4071 рана метку profile носили три, все от 07.08. Один из них — id=4000, profile_id=1, владелец пуст, вес трамвайной остановки в снимке −0.5 — посчитан системными весами при метке «профиль».

Различимо это потому, что снимок применённых весов лежит в результате, а веса профилей ни разу не редактировались. Оговорка честная: правка профиля после рана — или совпадение его весов с системными — сделала бы сравнение неразрешимым.

Ещё 2006 ранов вообще не имеют этого ключа (схема до #201) — к ним вопрос неприменим.

Что сделано

resolve_weights возвращает пару «веса + фактический источник». Форма выбрана так, что вызывающий не может взять веса и не взять источник: старый вызов падает громко. Промах теперь пишется предупреждением с идентификаторами — покрывает и случай «владелец не передан», где первая ветка вообще не выполнялась.

В ответе появилось requested_profile_applied — различие сохранено двумя полями: что получилось и что просили. source при этом не схлопывается в system: третий сценарий честно отдаёт user_default.

Код ответа оставлен 200, и это обосновано: profile_id здесь необязательный модификатор, а не адресуемый ресурс; 422 превратил бы штатную гонку «профиль удалили между показом списка и анализом» в отказ вместо честно помеченного ответа. Врала метка, а не сам запасной путь.

Прод после деплоя

?profile_id=1                             → source=system        applied=False
?profile_id=1&profile_user_id=__system__  → source=system        applied=False
?profile_id=999999&profile_user_id=admin  → source=user_default  applied=False
?profile_id=1&profile_user_id=admin       → source=profile       applied=True
?profile_user_id=admin                    → source=user_default  applied=None
(без параметров)                          → source=system        applied=None

Видимых изменений не будет — и это проверено, а не предположено

Полный поиск потребителей: фронт метку не рендерит (три упоминания — комментарии-предупреждения «врёт»), в разрешённом списке чата её нет, экспортёры не читают. Единственные потребители — сырой ответ API и запись в истории прогонов.

Три фронтовых комментария теперь устарели — снять отдельно.

Ещё одна моя предпосылка опровергнута

Я написал, что «фронт после #2810 под это больше не подставляется». Верно для основного пути, но в сборке параметров анализа осталась ветка, которая шлёт пару «профиль + владелец» без прямых весов. Сегодня она безопасна — профиль реально найдётся, — но это единственное место, где интерфейс всё ещё зависит от честности резолва.

## Поправка к моей шапке: промах падает не на системные веса, а на ЧУЖОЙ профиль — это хуже Я написал: «`resolve_weights` при ненайденном профиле молча уходит вниз по лестнице и **в пределе возвращает системные веса**». Верно только для части случаев. Проверил лестницу по коду и живыми запросами — три сценария дают три разных исхода: | запрос | метка была | что применялось на деле | |---|---|---| | `profile_id=1`, владелец не передан | `profile` | системные веса | | `profile_id=1`, чужой владелец | `profile` | системные веса | | **`profile_id=999999`, владелец `admin`** | `profile` | **default-профиль этого пользователя** | Третий случай — тот, что я описал неверно, и он **опаснее двух первых**. Веса выглядят настроенными; по значениям подмена не видна вообще (это чей-то реальный профиль, а не дефолты); а метка при этом называет `profile_id`, которого не существует. Отсюда прямое следствие для починки: подставлять `source="system"` во всех сомнительных случаях было бы **вторым враньём**, а не половинчатым фиксом. Хорошо, что этого не сделали. ## Замер истории: различить задним числом МОЖНО, и ложь случилась один раз Из **4071** рана метку `profile` носили **три**, все от 07.08. Один из них — `id=4000`, `profile_id=1`, владелец пуст, вес трамвайной остановки в снимке `−0.5` — посчитан **системными** весами при метке «профиль». Различимо это потому, что снимок применённых весов лежит в результате, а веса профилей **ни разу не редактировались**. Оговорка честная: правка профиля после рана — или совпадение его весов с системными — сделала бы сравнение неразрешимым. Ещё 2006 ранов вообще не имеют этого ключа (схема до #201) — к ним вопрос неприменим. ## Что сделано `resolve_weights` возвращает пару «веса + фактический источник». Форма выбрана так, что вызывающий **не может взять веса и не взять источник**: старый вызов падает громко. Промах теперь пишется предупреждением с идентификаторами — покрывает и случай «владелец не передан», где первая ветка вообще не выполнялась. В ответе появилось `requested_profile_applied` — различие сохранено двумя полями: **что получилось** и **что просили**. `source` при этом не схлопывается в `system`: третий сценарий честно отдаёт `user_default`. Код ответа **оставлен 200**, и это обосновано: `profile_id` здесь необязательный модификатор, а не адресуемый ресурс; `422` превратил бы штатную гонку «профиль удалили между показом списка и анализом» в отказ вместо честно помеченного ответа. Врала метка, а не сам запасной путь. ## Прод после деплоя ``` ?profile_id=1 → source=system applied=False ?profile_id=1&profile_user_id=__system__ → source=system applied=False ?profile_id=999999&profile_user_id=admin → source=user_default applied=False ?profile_id=1&profile_user_id=admin → source=profile applied=True ?profile_user_id=admin → source=user_default applied=None (без параметров) → source=system applied=None ``` ## Видимых изменений не будет — и это проверено, а не предположено Полный поиск потребителей: фронт метку **не рендерит** (три упоминания — комментарии-предупреждения «врёт»), в разрешённом списке чата её нет, экспортёры не читают. Единственные потребители — сырой ответ API и запись в истории прогонов. Три фронтовых комментария теперь устарели — снять отдельно. ## Ещё одна моя предпосылка опровергнута Я написал, что «фронт после #2810 под это больше не подставляется». Верно для основного пути, но в сборке параметров анализа осталась ветка, которая шлёт пару «профиль + владелец» без прямых весов. Сегодня она безопасна — профиль реально найдётся, — но это единственное место, где интерфейс всё ещё зависит от честности резолва.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
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#2811
No description provided.