reviewer(ET): auto-commit from reviewer run_id=211
This commit is contained in:
@@ -2,7 +2,7 @@
|
||||
type: review
|
||||
work_item_id: ET-013
|
||||
verdict: APPROVED
|
||||
version: 3
|
||||
version: 4
|
||||
created_at: 2026-06-04
|
||||
updated_at: 2026-06-06
|
||||
authors:
|
||||
@@ -10,138 +10,179 @@ authors:
|
||||
related:
|
||||
- "ET-013:trz"
|
||||
- "ET-013:adr-017"
|
||||
- "ET-013:test-report"
|
||||
---
|
||||
|
||||
# Review ET-013 — Перепады высот на z9-z11 (re-run #3)
|
||||
# Review ET-013 — Перепады высот на z9-z11 (re-run #4)
|
||||
|
||||
## TL;DR
|
||||
|
||||
- **Branch:** `feature/ET-013-z9-z11-z8`
|
||||
- **HEAD:** `316bb0d tester(ET): auto-commit from tester run_id=85`
|
||||
- **HEAD:** `c66d4fb tester(ET): auto-commit from tester run_id=209`
|
||||
- **Scope:** калибровка клиентского paint (hillshade/TRI) на z9-z11,
|
||||
понижение UI-минзума hillshade z10→z9, обратно-совместимое
|
||||
расширение `applyTerrainLayer`, расширение whitelist backend-endpoint'а
|
||||
на `tri` (фикс по review v1, F-1).
|
||||
- **Что изменилось со времени review v2:** **в коде и тестах — ничего.**
|
||||
`git diff 099669d..HEAD --name-only` → только `docs/.../12-review.md`
|
||||
(этот файл, v2) и `docs/.../13-test-report.md` (отчёт Тестера).
|
||||
Реализация (`src/`, `tests/`) идентична той, что была одобрена в v2.
|
||||
- **Что изменилось со времени review v3:** **в коде и тестах — ничего.**
|
||||
`git diff 316bb0d..HEAD --name-only` → только `12-review.md` (этот файл)
|
||||
и `13-test-report.md` (новый прогон Тестера, run_id=209). Реализация
|
||||
(`src/`, `tests/`) бит-в-бит совпадает с одобренной в v2/v3.
|
||||
- **Тесты:** `pytest tests/unit/test_terrain_paint.py` — **17/17 PASS**
|
||||
(локально, 0.03s). Integration-тесты в sandbox не собираются из-за
|
||||
отсутствия `shapely` (ограничение окружения, не код) — Тестер
|
||||
подтвердил 6 passed / 7 skipped в полном окружении (`13-test-report.md`).
|
||||
- **Verdict: APPROVED.** P0/P1 не найдено. Остаются те же два
|
||||
опциональных P3-косметических замечания из v1/v2.
|
||||
(перезапущено локально, 0.03 s). Тестер в полном venv: **254 passed,
|
||||
7 skipped, 0 failed**; ET-013 unit 17/17, integration 6 PASS / 7 SKIP
|
||||
(skip — отсутствие PNG-данных, штатно по AC-16).
|
||||
- **Verdict кода: APPROVED.** P0/P1 в коде не найдено. Код соответствует
|
||||
TRZ REQ-F-01..F-19, F-21 и всем решениям ADR-017.
|
||||
- **⚠️ ВАЖНО — деплой не разблокирован.** Тестер (run_id=209) выставил
|
||||
`verdict: BLOCKED` из-за **AC-19 (P1): на test-среде
|
||||
`/terrain/hillshade/9/{x}/{y}.png` отдаёт 404** — тайлы z9 не нарезаны
|
||||
на сервере. Это **не дефект PR** (см. §«AC-19»), а предусловие данных
|
||||
PH-6; merge/деплой держит именно тест-гейт Тестера, а не этот ревью.
|
||||
|
||||
## Что прочитано
|
||||
|
||||
- `docs/work-items/ET-013/02-trz.md`
|
||||
- `docs/work-items/ET-013/03-acceptance-criteria.md`
|
||||
- `docs/work-items/ET-013/06-adr/ADR-017-zoom-aware-terrain-paint.md`
|
||||
- `docs/work-items/ET-013/12-review.md` (v2)
|
||||
- `docs/work-items/ET-013/13-test-report.md`
|
||||
- `CLAUDE.md`
|
||||
- `git diff 8da09e6..HEAD -- src/web/app.js src/web/index.html src/api/main.py`
|
||||
(полный diff фичи)
|
||||
- `git diff 099669d..HEAD --name-only` (что изменилось со времени v2)
|
||||
- `src/api/main.py:1240-1264` (`terrain_tile`)
|
||||
- `docs/work-items/ET-013/12-review.md` (v3)
|
||||
- `docs/work-items/ET-013/13-test-report.md` (run_id=209, verdict BLOCKED)
|
||||
- `docs/architecture/README.md`, `CLAUDE.md`
|
||||
- `git diff 8da09e6..HEAD` (полный diff фичи: `src/`, `tests/`, docs)
|
||||
- `git diff 316bb0d..HEAD --name-only` (что изменилось со времени v3)
|
||||
- `src/api/main.py:1239-1264` (`terrain_tile`)
|
||||
- `src/web/app.js` (2725-2767, 2821-2826, 3356-3430)
|
||||
- `src/web/index.html:57-65`
|
||||
- `tests/unit/test_terrain_paint.py`, `tests/integration/test_terrain_z9_tiles.py`
|
||||
- `grep applyTerrainLayer src/web/` → 1 декларация + ровно 2 вызова
|
||||
|
||||
## Соответствие ТЗ
|
||||
|
||||
| Требование | Реализация | Файл | OK |
|
||||
|---|---|---|---|
|
||||
| REQ-F-01 / F-11 — порог `zoom < 9` в `updateHillshadeAvailability` | `if (zoom < 9)` + комментарий | `src/web/app.js:3425` | ✅ |
|
||||
| REQ-F-02 — hillshade `minzoom=9` | `applyTerrainLayer('terrain-hillshade', …, HILLSHADE_PAINT, 9, 15)` | `app.js:2824` | ✅ |
|
||||
| REQ-F-03 — TRI `minzoom=5` без изменений | `applyTerrainLayer('terrain-tri', …, TRI_PAINT, 5, 15)` | `app.js:2825` | ✅ |
|
||||
| REQ-F-04 — обратно-совместимое `opacityOrPaint` | нормализация `typeof === 'number' ? legacy : obj` | `app.js:3376-3380` | ✅ |
|
||||
| REQ-F-05/06/07 — HILLSHADE_PAINT (opacity/contrast interpolate + nearest) | stops 9/10/11/12/14 → 0.65/0.60/0.55/0.50/0.40; contrast 0.40→0.00; `nearest` | `app.js:2729-2751` | ✅ |
|
||||
| REQ-F-08/09 — TRI_PAINT (opacity interpolate z8=0.70, пик 0.80-0.85 + nearest) | точное совпадение со spec | `app.js:2754-2767` | ✅ |
|
||||
| REQ-F-01 / F-11 — порог `zoom < 9` в `updateHillshadeAvailability` | `if (zoom < 9)` + комментарий | `app.js:3425` | ✅ |
|
||||
| REQ-F-02 — hillshade `minzoom=9` | `applyTerrainLayer('terrain-hillshade', …, HILLSHADE_PAINT, 9, 15)` | `app.js:2825` | ✅ |
|
||||
| REQ-F-03 — TRI `minzoom=5` без изменений | `applyTerrainLayer('terrain-tri', …, TRI_PAINT, 5, 15)` | `app.js:2826` | ✅ |
|
||||
| REQ-F-04 — обратно-совместимый `opacityOrPaint` | `typeof === 'number' ? legacy : obj` | `app.js:3382-3385` | ✅ |
|
||||
| REQ-F-05/06/07 — HILLSHADE_PAINT (opacity/contrast interpolate + nearest) | stops 9/10/11/12/14 → 0.65/0.60/0.55/0.50/0.40; contrast 0.40→0.00; `nearest` | `app.js:2732-2750` | ✅ |
|
||||
| REQ-F-08/09 — TRI_PAINT (interpolate, z8=0.70, пик 0.80-0.85, nearest) | точное совпадение со spec | `app.js:2753-2766` | ✅ |
|
||||
| REQ-F-10 — hint «Зум 9+» | `<span id="terrain-hillshade-hint">Зум 9+</span>` | `index.html:60` | ✅ |
|
||||
| REQ-F-12 — контракт `onTerrainCheckbox` (localStorage, `.active`) | без изменений | `app.js:2816-2826` | ✅ |
|
||||
| REQ-F-12 — контракт `onTerrainCheckbox` (localStorage, `.active`) | без изменений | `app.js:2818-2826` | ✅ |
|
||||
| REQ-F-13 — unit-тесты paint (Вариант B) | 17 тестов, все PASS | `tests/unit/test_terrain_paint.py` | ✅ |
|
||||
| REQ-F-14 — регрессии (порог 9, hint, callers) | покрыты | `tests/unit/test_terrain_paint.py` | ✅ |
|
||||
| REQ-F-15 — integration smoke `/terrain/{layer}/9.../`→200 + 404 + cache-header | параметризация `[hillshade,tri]×[9,10,11]`, whitelist по 3 слоям, 404-регрессии | `tests/integration/test_terrain_z9_tiles.py` | ✅ |
|
||||
| REQ-F-16 — Playwright UI | исполнено Тестером (`13-test-report.md`) | — | n/a (review) |
|
||||
| REQ-F-14 — регрессии (порог 9, hint, callers) | покрыты | там же | ✅ |
|
||||
| REQ-F-15 — integration smoke `/terrain/{layer}/9..11`→200 + 404 + cache | параметризация `[hillshade,tri]×[9,10,11]`, whitelist по 3 слоям, 404-регрессии | `tests/integration/test_terrain_z9_tiles.py` | ✅ |
|
||||
| REQ-F-16 — Playwright UI | этап Тестирования (`13-test-report.md`) | — | n/a (review) |
|
||||
| REQ-F-17 — localStorage без миграции | не тронуто | — | ✅ |
|
||||
| REQ-F-18 — API-контракт без изменений | сигнатура `GET /terrain/{layer}/{z}/{x}/{y}.png` сохранена; whitelist расширен на `tri` (см. ADR) | `main.py:1240-1264` | ✅ |
|
||||
| REQ-F-18 — API-контракт без изменений | сигнатура `GET /terrain/{layer}/{z}/{x}/{y}.png` сохранена; коды/Cache-Control те же; whitelist расширен на `tri` (см. §ADR) | `main.py:1240-1264` | ✅ |
|
||||
| REQ-F-19 — конфиги/стили не тронуты | `style.json`, `style-dark.json`, `app.css`, `config/*` — без правок | `git diff --stat` | ✅ |
|
||||
| REQ-F-20 — pre-deploy curl/smoke | этап Деплоя | — | n/a (review) |
|
||||
| REQ-F-20 — pre-deploy curl/smoke | этап Деплоя; **выполнено Тестером → 404** (см. §AC-19) | — | ⚠️ (вне кода) |
|
||||
| REQ-F-21 — документация | `00`..`10` + `06-adr/ADR-017` присутствуют | — | ✅ |
|
||||
|
||||
**Acceptance Criteria.** AC-01/02/04/05/15/17/22 — покрыты unit-тестами
|
||||
(зелёные). AC-16 — integration структурно корректен (skip без PNG-данных
|
||||
ожидаем; whitelist-регрессия работает без данных). Поведенческие
|
||||
AC-03/06..13/19/21 — за этапом Тестирования (см. `13-test-report.md`).
|
||||
**Acceptance Criteria (по коду).** AC-01/02/04/05/15/16/17/22 — закрыты
|
||||
авто-тестами (зелёные). Поведенческие AC-03/06..13 — за этапом
|
||||
Тестирования. **AC-19 — FAIL (тайлы z9 на сервере 404)**, и как следствие
|
||||
AC-03/07/13 — FAIL; это предусловие данных, не дефект PR (см. ниже).
|
||||
|
||||
## Соответствие ADR
|
||||
|
||||
ADR-017 реализован по всем решениям: P-A (frontend-калибровка),
|
||||
U-A (UI-минзум 10→9), A-A (back-compat `applyTerrainLayer`),
|
||||
O-B+C-A+R-A (HILLSHADE_PAINT), O-B+R-A (TRI_PAINT), T-A (один paint
|
||||
на все темы), M-A (константы в `app.js`).
|
||||
ADR-017 реализован по всем семи решениям: P-A (frontend-калибровка),
|
||||
U-A (UI-минзум 10→9 + source.minzoom 9), A-A (back-compat
|
||||
`applyTerrainLayer`), O-B+C-A+R-A (HILLSHADE_PAINT), O-B+R-A (TRI_PAINT),
|
||||
T-A (единый paint на все темы), M-A (константы в `app.js`).
|
||||
|
||||
**Замечание по разделу «Не меняются: `src/api/main.py`».** Фактически
|
||||
`main.py:1252` изменён (whitelist `tri`). Это **не нарушение ADR**, а
|
||||
корректное инфра-уточнение из review v1 (F-1): ADR-017 в «Контексте»
|
||||
исходил из того, что endpoint уже отдаёт `/terrain/{layer}/…` для TRI;
|
||||
до фикса `tri` не проходил whitelist в dev-режиме. Фикс восстанавливает
|
||||
заявленное состояние, не вводит новый endpoint/source/слой/query/header,
|
||||
сохраняет код ответа и `Cache-Control: immutable` (REQ-F-18 по сути
|
||||
соблюдён). Документировано в docstring `terrain_tile` со ссылкой на F-1.
|
||||
Изменение = 1 строка + docstring, низкий риск.
|
||||
**Замечание по разделу ADR «Не меняются: `src/api/main.py`».** Фактически
|
||||
`main.py:1252` изменён (whitelist `tri` + расширенный docstring). Это **не
|
||||
нарушение архитектурного решения**: ни одно из решений 1–7 ADR-017 не
|
||||
затронуто. Решение P-A гласит «0 изменений в backend-логике, 0 в тайлах,
|
||||
0 новых endpoint'ов»; whitelist-фикс не добавляет endpoint/source/query/
|
||||
header, не меняет коды ответа и `Cache-Control`, а лишь делает уже
|
||||
существующий слой `tri` маршрутизируемым в dev-режиме (без nginx).
|
||||
ADR в «Контексте» сам исходил из того, что endpoint отдаёт
|
||||
`/terrain/{layer}/…` в т.ч. для TRI. Фикс восстанавливает заявленное
|
||||
состояние, документирован в docstring со ссылкой на review F-1, объём =
|
||||
1 строка + docstring, риск низкий. Расхождение с побочной фразой раздела
|
||||
«Классификация» — несоответствие документации, не кода; на P-severity
|
||||
не тяну.
|
||||
|
||||
## Изменения после review v2
|
||||
## AC-19 / деплой-блокер (анализ от ревью)
|
||||
|
||||
Только документация:
|
||||
- `12-review.md` — переписан до v2 (предыдущий вердикт APPROVED).
|
||||
- `13-test-report.md` — добавлен Тестером (run_id=85).
|
||||
Тестер (run_id=209) объективно зафиксировал: на выкаченной test-среде
|
||||
`GET …/terrain/hillshade/9/{x}/{y}.png` → **404**. Тайлы z9 для hillshade
|
||||
на сервере **отсутствуют**. Следствие: AC-03/07/13 (видимость и
|
||||
читаемость hillshade на z9) не достижимы в рантайме → тест-вердикт
|
||||
**BLOCKED**.
|
||||
|
||||
Код и тесты (`src/`, `tests/`) **не менялись** с момента APPROVED-вердикта v2.
|
||||
Ранее закрытые findings v1 (F-1 P1 backend whitelist, F-2 P2 integration
|
||||
coverage) остаются закрытыми.
|
||||
**Почему это не code-review P0/P1.** Предпосылка U-A ADR-017 («тайлы z9
|
||||
на диске есть, нарезка z8-z14 по PH-6») оказалась ложной в среде. Но:
|
||||
- ни одна строка PR не управляет наличием PNG на сервере;
|
||||
- TRZ это предусмотрел — REQ-F-20 §1: «Если 404 — задача
|
||||
останавливается, тайлы z9 нужно догенерировать в рамках PH-6
|
||||
follow-up»; ADR-017 — TD-1 / альтернатива F-1 «hillshade-rerender-z9-z14»;
|
||||
- `REQUEST_CHANGES` на разработчика бессмысленно: правкой кода тайлы не
|
||||
появятся.
|
||||
|
||||
**Владелец и гейт.** Это инфра/данные PH-6. Шиппинг держит **тест-гейт
|
||||
Тестера (BLOCKED)**, а не code-review. Рекомендация ревью: открыть
|
||||
PH-6 follow-up «нарезать hillshade z9-z11 (и проверить tri z9-z11) для
|
||||
покрытия региона test-среды», после чего повторить AC-19 и поведенческие
|
||||
AC-03/07/13. До этого задачу к деплою не выпускать — но по линии Тестера.
|
||||
|
||||
## Качество кода
|
||||
|
||||
- Стиль соответствует `app.js` (vanilla JS, JSDoc, маркеры `// ET-013:`).
|
||||
- `applyTerrainLayer` расширён минимально-инвазивно (4 строки нормализации
|
||||
+ переменная `paint`); оба call-site обновлены, других нет.
|
||||
- Константы `HILLSHADE_PAINT` / `TRI_PAINT` — UPPER_SNAKE_CASE, рядом с
|
||||
`TERRAIN_BASE_URL`; комментарии ссылаются на ADR-017.
|
||||
- Стиль совпадает с `app.js` (vanilla JS, JSDoc, маркеры `// ET-013:`).
|
||||
- `applyTerrainLayer` расширён минимально (нормализация paint + переменная
|
||||
`paint`); ровно 2 call-site (grep подтверждает), обратная совместимость
|
||||
сохранена.
|
||||
- `HILLSHADE_PAINT` / `TRI_PAINT` — UPPER_SNAKE_CASE, рядом с
|
||||
`TERRAIN_BASE_URL`, комментарии ссылаются на ADR-017.
|
||||
- Нет дублирования, dead code, `console.log`, закомментированного кода.
|
||||
- Backend-фикс не вводит новых code-path'ов.
|
||||
- Backend-фикс: whitelist — фиксированный кортеж, `z/x/y` типизированы
|
||||
как `int`, path-traversal-рисков нет.
|
||||
|
||||
## Изменения после review v3
|
||||
|
||||
Только документация (`git diff 316bb0d..HEAD --name-only`):
|
||||
- `12-review.md` — этот файл (v4).
|
||||
- `13-test-report.md` — новый прогон Тестера (run_id=209).
|
||||
|
||||
Код и тесты (`src/`, `tests/`) с момента APPROVED-вердикта v2/v3 **не
|
||||
менялись**. Findings v1 (F-1 P1 backend whitelist, F-2 P2 integration
|
||||
coverage) остаются закрытыми.
|
||||
|
||||
## Findings
|
||||
|
||||
### F-3 — P3 (косметика) — комментарий HILLSHADE_PAINT не упоминает MapLibre clamping ниже z9
|
||||
### F-3 — P3 (косметика) — комментарий HILLSHADE_PAINT не упоминает clamping ниже z9
|
||||
|
||||
**Где.** `src/web/app.js:2729-2733`. Stops opacity начинаются с `9, 0.65`;
|
||||
при z<9 MapLibre заклампит на нижнем стопе. В текущем scope не проблема
|
||||
(UI-gate отрубает чекбокс при z<9). Если порог когда-нибудь понизят —
|
||||
добавить нижний stop `8, 0.00`. **Опционально, не блокер.**
|
||||
`src/web/app.js:2732`. Stops opacity начинаются с `9, 0.65`; при z<9
|
||||
MapLibre заклампит на нижнем стопе. В текущем scope не проблема (UI-gate
|
||||
отключает чекбокс при z<9). Если порог когда-нибудь понизят — добавить
|
||||
нижний stop `8, 0.00`. **Опционально, не блокер.**
|
||||
|
||||
### F-5 — P3 (косметика) — неиспользуемый `from __future__ import annotations`
|
||||
|
||||
**Где.** `tests/unit/test_terrain_paint.py:15`. Не вредит. **Опционально.**
|
||||
`tests/unit/test_terrain_paint.py:15`. Не вредит. **Опционально.**
|
||||
|
||||
## Вердикт
|
||||
|
||||
**APPROVED.**
|
||||
**APPROVED** (ось code-review).
|
||||
|
||||
- P0/P1 не найдено.
|
||||
- Код/тесты не менялись с момента APPROVED v2; повторная проверка
|
||||
подтверждает соответствие TRZ REQ-F-01..F-21 и ADR-017.
|
||||
- Unit-тесты зелёные (17/17). Integration структурно корректен,
|
||||
подтверждён Тестером.
|
||||
- P0/P1 в коде/ADR/тестах не найдено.
|
||||
- Код соответствует TRZ REQ-F-01..F-19, F-21 и всем решениям ADR-017.
|
||||
- Unit-тесты зелёные (17/17), полный регресс зелёный (254 passed),
|
||||
integration структурно корректен.
|
||||
- Остаются два P3-косметических замечания — не блокеры.
|
||||
|
||||
> ⚠️ **Это вердикт только по качеству/соответствию кода.** Готовность к
|
||||
> деплою отдельно блокируется тест-гейтом Тестера (AC-19, z9-тайлы 404 на
|
||||
> сервере). Перед выпуском нужен PH-6 follow-up на нарезку тайлов z9-z11 и
|
||||
> повторный прогон AC-19 / AC-03 / AC-07 / AC-13. Code-review этого не
|
||||
> снимает и не может снять правкой кода.
|
||||
|
||||
## Сводная таблица findings
|
||||
|
||||
| ID | Severity | Где | Кратко | Действие |
|
||||
|---|---|---|---|---|
|
||||
| F-3 | P3 | `src/web/app.js:2729-2733` | комментарий не учитывает MapLibre clamping ниже z9 | опц. добавить stop `8, 0.00` при будущем понижении порога |
|
||||
| F-5 | P3 | `tests/unit/test_terrain_paint.py:15` | неиспользуемый `from __future__ import annotations` | косметика |
|
||||
| F-3 | P3 | `app.js:2732` | комментарий не учитывает clamping ниже z9 | опц. добавить stop `8, 0.00` при будущем понижении порога |
|
||||
| F-5 | P3 | `test_terrain_paint.py:15` | неиспользуемый `from __future__ import annotations` | косметика |
|
||||
| — | (вне кода) | test-среда | AC-19: hillshade z9 = 404, тайлы не нарезаны | PH-6 follow-up; гейт держит Тестер (BLOCKED), не ревью |
|
||||
|
||||
P0/P1 отсутствуют → **APPROVED**.
|
||||
P0/P1 по коду отсутствуют → **APPROVED**. Деплой-готовность — за тест-гейтом.
|
||||
|
||||
Reference in New Issue
Block a user