reviewer(ET): auto-commit from reviewer run_id=84
All checks were successful
All checks were successful
This commit is contained in:
@@ -1,8 +1,8 @@
|
||||
---
|
||||
type: review
|
||||
work_item_id: ET-013
|
||||
verdict: REQUEST_CHANGES
|
||||
version: 1
|
||||
verdict: APPROVED
|
||||
version: 2
|
||||
created_at: 2026-06-04
|
||||
updated_at: 2026-06-04
|
||||
authors:
|
||||
@@ -12,24 +12,28 @@ related:
|
||||
- "ET-013:adr-017"
|
||||
---
|
||||
|
||||
# Review ET-013 — Перепады высот на z9-z11
|
||||
# Review ET-013 — Перепады высот на z9-z11 (re-run #2)
|
||||
|
||||
## TL;DR
|
||||
|
||||
- **Branch:** `feature/ET-013-z9-z11-z8`
|
||||
- **Scope:** калибровка клиентского paint для hillshade/TRI на z9-z11
|
||||
+ понижение UI-минзума hillshade с z10 до z9
|
||||
(ADR-017, TRZ REQ-F-01..F-21).
|
||||
- **Реализация:** соответствует TRZ дословно — 79 строк правок в
|
||||
`src/web/app.js`, 1 строка в `src/web/index.html`, 300 строк
|
||||
unit-тестов, 129 строк integration-теста. `make test`
|
||||
(Python-unit) — **17/17 PASS** локально.
|
||||
- **Verdict: REQUEST_CHANGES** — найден **P1** (потенциально P0)
|
||||
пробел: backend `terrain_tile` whitelist **не пропускает `tri`**,
|
||||
поэтому весь TRI-функционал (центральный для ET-013) либо ушёл
|
||||
в 404, либо обслуживается каким-то внешним прокси, не описанным
|
||||
ни в репо, ни в TRZ. Перед merge нужно либо доработать backend,
|
||||
либо явно зафиксировать факт прокси в TRZ/infra.
|
||||
+ понижение UI-минзума hillshade с z10 до z9 + расширение whitelist
|
||||
backend-endpoint'а на `tri` (фикс по результатам review v1, F-1).
|
||||
- **HEAD:** `099669d fix(terrain): расширить whitelist endpoint'а на 'tri' (ET-013 review F-1)`
|
||||
- **Что изменилось со времени review v1:**
|
||||
- `src/api/main.py:1252` whitelist расширен:
|
||||
`("hypso", "hillshade") → ("hypso", "hillshade", "tri")` + docstring
|
||||
с пояснением (см. F-1 v1).
|
||||
- `tests/integration/test_terrain_z9_tiles.py` параметризован по
|
||||
`layer = ["hillshade", "tri"]` для z9/z10/z11; добавлен явный
|
||||
регрессионный тест `test_known_terrain_layer_accepted_by_whitelist`
|
||||
по всем трём слоям (см. F-2 v1).
|
||||
- **Тесты:** `pytest tests/unit/test_terrain_paint.py` — **17/17 PASS**,
|
||||
`pytest tests/integration/test_terrain_z9_tiles.py` — **6 passed, 7 skipped**
|
||||
(skip — отсутствие PNG-данных в sandbox, ожидаемо).
|
||||
- **Verdict: APPROVED.** P0/P1 не найдено. Остались два опциональных
|
||||
P3 из v1, оба косметика — не блокеры.
|
||||
|
||||
## Что прочитано
|
||||
|
||||
@@ -38,16 +42,16 @@ related:
|
||||
- `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/07-infra-requirements.md` (по ссылкам)
|
||||
- `docs/work-items/ET-013/08-data-requirements.md` (по ссылкам)
|
||||
- `docs/work-items/ET-013/10-tech-risks.md` (по ссылкам)
|
||||
- `docs/work-items/ET-013/07-infra-requirements.md`
|
||||
- `docs/work-items/ET-013/12-review.md` v1 (предыдущий вердикт)
|
||||
- `CLAUDE.md`
|
||||
- `git diff main...HEAD` (15 файлов, +3564/-12)
|
||||
- `src/web/app.js` (диапазоны 2715-2835 и 3350-3440)
|
||||
- `src/web/index.html:57-60`
|
||||
- `src/api/main.py:1233-1256` (`terrain_tile`)
|
||||
- `docker-compose.yml`, `Dockerfile` (на предмет прокси)
|
||||
- `tests/unit/test_terrain_paint.py`, `tests/integration/test_terrain_z9_tiles.py`
|
||||
- `git diff main...HEAD --stat` (18 файлов, +3911/-14)
|
||||
- `git diff main...HEAD -- src/api/main.py src/web/app.js src/web/index.html`
|
||||
- `src/api/main.py:1235-1264` (`terrain_tile` после фикса)
|
||||
- `src/web/app.js` (диапазоны 2725-2835 и 3356-3430)
|
||||
- `src/web/index.html:57-65`
|
||||
- `tests/unit/test_terrain_paint.py`
|
||||
- `tests/integration/test_terrain_z9_tiles.py`
|
||||
|
||||
## Соответствие ТЗ
|
||||
|
||||
@@ -56,7 +60,7 @@ related:
|
||||
| REQ-F-01 — `updateHillshadeAvailability`: порог `zoom < 9` | `if (zoom < 9)` с комментарием ET-013 | `src/web/app.js:3425` | ✅ |
|
||||
| REQ-F-02 — `source.minzoom = 9` для hillshade | `applyTerrainLayer('terrain-hillshade', …, HILLSHADE_PAINT, 9, 15)` | `src/web/app.js:2825` | ✅ |
|
||||
| REQ-F-03 — TRI minzoom = 5 без изменений | `applyTerrainLayer('terrain-tri', …, TRI_PAINT, 5, 15)` | `src/web/app.js:2826` | ✅ |
|
||||
| REQ-F-04 — обратно-совместимое расширение `applyTerrainLayer(opacityOrPaint)` | нормализация `(typeof opacityOrPaint === 'number') ? legacyPaint : opacityOrPaint` | `src/web/app.js:3371-3380` | ✅ |
|
||||
| REQ-F-04 — обратно-совместимое расширение `applyTerrainLayer(opacityOrPaint)` | нормализация `(typeof opacityOrPaint === 'number') ? legacyPaint : opacityOrPaint` | `src/web/app.js:3376-3380` | ✅ |
|
||||
| REQ-F-05 — HILLSHADE_PAINT `raster-opacity` interpolate по zoom (stops 9/10/11/12/14 → 0.65/0.60/0.55/0.50/0.40) | константа `HILLSHADE_PAINT`, точные stops | `src/web/app.js:2734-2742` | ✅ |
|
||||
| REQ-F-06 — HILLSHADE_PAINT `raster-contrast` interpolate (stops 9/10/11/12/14 → 0.40/0.35/0.30/0.15/0.00) | присутствует | `src/web/app.js:2743-2750` | ✅ |
|
||||
| REQ-F-07 — HILLSHADE_PAINT `raster-resampling: 'nearest'` | присутствует | `src/web/app.js:2751` | ✅ |
|
||||
@@ -65,223 +69,146 @@ related:
|
||||
| REQ-F-10 — hint «Зум 9+» | `<span … id="terrain-hillshade-hint" …>Зум 9+</span>` | `src/web/index.html:60` | ✅ |
|
||||
| REQ-F-11 — единый порог в `updateHillshadeAvailability` | тот же `< 9` | — | ✅ |
|
||||
| REQ-F-12 — контракт `onTerrainCheckbox` (localStorage `terrain-hillshade`, `terrain-tri`, `#terrain-toggle.active`) | без изменений | `src/web/app.js:2816-2821` | ✅ |
|
||||
| REQ-F-13 — unit-тесты paint (Вариант B: Python-парсер) | 17 тестов, все PASS локально | `tests/unit/test_terrain_paint.py` | ✅ |
|
||||
| REQ-F-13 — unit-тесты paint (Вариант B: Python-парсер) | 17 тестов, все PASS | `tests/unit/test_terrain_paint.py` | ✅ |
|
||||
| REQ-F-14 — регрессионные тесты (порог 9, hint, callers) | `test_minzoom_threshold_lowered_to_9`, `test_hint_text_updated_to_z9`, `test_apply_terrain_layer_caller_count` | `tests/unit/test_terrain_paint.py` | ✅ |
|
||||
| REQ-F-15 — integration smoke: `/terrain/hillshade/9/.../….png` → 200 + 404 на невалидный layer + Cache-Control immutable | `test_hillshade_tile_available_z9_z10_z11` (skip без данных), `test_unknown_terrain_layer_returns_404`, `test_terrain_tile_cache_control_immutable` | `tests/integration/test_terrain_z9_tiles.py` | ⚠ (см. F-1) |
|
||||
| REQ-F-15 — integration smoke: `/terrain/{layer}/9/.../….png` → 200 + 404 на невалидный layer + Cache-Control immutable | параметризован по `["hillshade", "tri"]` × `[9, 10, 11]`, регрессии 404, whitelist-тест по 3 слоям | `tests/integration/test_terrain_z9_tiles.py` | ✅ |
|
||||
| REQ-F-16 — Playwright UI-тесты | в test-плане, исполняет Тестер | — | n/a (review) |
|
||||
| REQ-F-17 — localStorage без миграции | не тронуто | — | ✅ |
|
||||
| REQ-F-18 — API-контракт без изменений | `src/api/main.py` не тронут | — | ✅ |
|
||||
| REQ-F-18 — API-контракт без изменений | сигнатура `GET /terrain/{layer}/{z}/{x}/{y}.png` сохранена; whitelist расширен (см. §«Изменения после v1») | `src/api/main.py:1240-1264` | ✅ |
|
||||
| REQ-F-19 — конфиги/стили не тронуты | `style.json`, `style-dark.json`, `app.css`, `config/*.yaml` — без правок (`git diff --stat` подтверждает) | — | ✅ |
|
||||
| REQ-F-20 — pre-deploy curl + smoke | задача deployer'а | — | n/a (review) |
|
||||
| REQ-F-21 — документация | `00-..-10-` + `06-adr/ADR-017-…` присутствуют | — | ✅ |
|
||||
|
||||
**Acceptance Criteria.** AC-01, AC-02, AC-04, AC-05 (структура paint),
|
||||
AC-22 (back-compat) — проверяемы unit-тестами и проходят.
|
||||
AC-03, AC-06..AC-13, AC-19, AC-21 требуют test-среды и не относятся
|
||||
к review-стадии (передаются Тестеру). AC-15, AC-17, AC-18 — зелёные
|
||||
(см. §«Тесты»).
|
||||
**Acceptance Criteria.**
|
||||
- AC-01, AC-02, AC-04, AC-05 (структура paint), AC-15, AC-17, AC-22
|
||||
(back-compat) — покрыты unit-тестами, **зелёные**.
|
||||
- AC-16 — integration-тесты структурно корректны, в sandbox skip
|
||||
из-за отсутствия PNG; whitelist-регрессия по `tri/hillshade/hypso`
|
||||
работает без данных и зелёная.
|
||||
- AC-03, AC-06..AC-13, AC-19, AC-21 — требуют test-среды и Playwright,
|
||||
относятся к этапу Тестирования.
|
||||
|
||||
## Соответствие ADR
|
||||
|
||||
ADR-017 («Zoom-aware terrain paint») реализован по всем пунктам:
|
||||
|
||||
- **P-A** (frontend-only): backend / тайлы / стили / nginx не тронуты — подтверждено `git diff --stat`.
|
||||
- **P-A** (frontend-only): backend-фикс whitelist'а `tri` — это
|
||||
**корректная инфра-уточнение**, не выход за P-A. ADR-017 §«Контекст»
|
||||
утверждал, что эндпоинт уже отдаёт `/terrain/{layer}/…` для TRI;
|
||||
фактически до этого PR `tri` не был в whitelist'е в dev-режиме, и
|
||||
фикс восстанавливает заявленное состояние (а не вводит новый
|
||||
endpoint/source/слой). Документировано в docstring `terrain_tile`.
|
||||
- **U-A** (UI-минзум 10→9): подтверждено `app.js:3425` и `index.html:60`.
|
||||
- **A-A** (обратно-совместимое расширение `applyTerrainLayer`): нормализация числа в legacy-paint реализована (`app.js:3377-3380`), unit-test `test_apply_terrain_layer_normalizes_number_to_legacy_paint` зелёный.
|
||||
- **O-B + C-A + R-A** для HILLSHADE_PAINT: stops, contrast, `nearest`-resampling — точно по ADR.
|
||||
- **O-B + R-A** для TRI_PAINT: stops с явной точкой `8→0.70` для регрессии z8 — точно по ADR.
|
||||
- **T-A** (один paint на все темы): theme-specific paint не добавлен — соответствует MVP-решению ADR.
|
||||
- **M-A** (константы живут в `app.js` рядом с `TERRAIN_BASE_URL`): подтверждено, расстояние 1 строка.
|
||||
- **A-A** (обратно-совместимое расширение `applyTerrainLayer`):
|
||||
нормализация числа в legacy-paint реализована (`app.js:3376-3380`),
|
||||
unit-test `test_apply_terrain_layer_normalizes_number_to_legacy_paint`
|
||||
зелёный.
|
||||
- **O-B + C-A + R-A** для HILLSHADE_PAINT: stops, contrast,
|
||||
`nearest`-resampling — точно по ADR.
|
||||
- **O-B + R-A** для TRI_PAINT: stops с явной точкой `8→0.70` для
|
||||
регрессии z8 — точно по ADR.
|
||||
- **T-A** (один paint на все темы): theme-specific paint не добавлен —
|
||||
соответствует MVP-решению ADR.
|
||||
- **M-A** (константы живут в `app.js` рядом с `TERRAIN_BASE_URL`):
|
||||
подтверждено, расстояние 1 строка.
|
||||
|
||||
Нарушений ADR-017 не найдено.
|
||||
|
||||
## Изменения после review v1 (что было исправлено)
|
||||
|
||||
| v1 finding | Severity | Статус | Что сделано |
|
||||
|---|---|---|---|
|
||||
| F-1 — backend whitelist не пропускает `tri` | P1 | **RESOLVED** | `src/api/main.py:1252`: `("hypso", "hillshade") → ("hypso", "hillshade", "tri")` + docstring с обоснованием (nginx на prod/test перехватывает, но dev-режим должен поддерживать нативно) |
|
||||
| F-2 — integration-тест не параметризован по layer | P2 | **RESOLVED** | `test_terrain_tile_available_z9_z10_z11` параметризован по `["hillshade", "tri"]` × `[9, 10, 11]`; добавлен явный `test_known_terrain_layer_accepted_by_whitelist[hypso/hillshade/tri]` |
|
||||
| F-3 — комментарий в `HILLSHADE_PAINT` не упоминает MapLibre clamping ниже z9 | P3 | OPEN (косметика) | Не блокер; см. ниже |
|
||||
| F-4 — `from __future__ import annotations` неиспользован | P3 | N/A | В текущем integration-тесте `from __future__` отсутствует; в unit-тесте остался, но это микро-косметика |
|
||||
|
||||
Все P0/P1 v1 закрыты.
|
||||
|
||||
## Тесты
|
||||
|
||||
- **Unit (`tests/unit/test_terrain_paint.py`).** 17 тестов, **17 PASS**
|
||||
локально (Python 3.12.13, pytest 8.3.3, время 0.03s). Покрывают:
|
||||
объявление констант, форму `interpolate`-выражений, ключевые
|
||||
stops (z9/11/14 для hillshade, z8/10/11 для TRI), монотонность,
|
||||
локально (Python 3.12.13, pytest 8.3.3, время 0.04s). Покрывают:
|
||||
объявление констант, форму `interpolate`-выражений, ключевые stops
|
||||
(z9/11/14 для hillshade, z8/10/11 для TRI), монотонность,
|
||||
`nearest`-resampling, регрессию порога `< 9` и текста «Зум 9+»,
|
||||
обратную совместимость `applyTerrainLayer`, корректное использование
|
||||
констант в вызовах.
|
||||
|
||||
Парсер `_extract_block` использует brace-counting (не regex по `}`),
|
||||
безопасен для вложенных объектов. `_parse_zoom_stops` тоже
|
||||
устойчив к форматированию.
|
||||
|
||||
- **Integration (`tests/integration/test_terrain_z9_tiles.py`).** В
|
||||
данной песочнице collection падает из-за отсутствия `shapely`
|
||||
в окружении (импорт `src/api/main.py` тянет всё подряд) — это
|
||||
не дефект ET-013, в репо `pyproject.toml` декларирует зависимости,
|
||||
CI их установит. Структурно тест корректен:
|
||||
- параметризован по z9/z10/z11,
|
||||
- skip при отсутствии локальных PNG (через `_maybe_skip`),
|
||||
- регрессии 404 (unknown layer, missing tile, invalid zoom) — работают всегда,
|
||||
- дополнительный `test_terrain_tile_cache_control_immutable` проверяет `Cache-Control: public, max-age=31536000, immutable`.
|
||||
|
||||
Замечание (F-1): тест `test_unknown_terrain_layer_returns_404`
|
||||
использует `unknown_layer`, но это совпадает с реальным
|
||||
поведением для `tri` — см. P1 ниже.
|
||||
- **Integration (`tests/integration/test_terrain_z9_tiles.py`).**
|
||||
13 тестов: **6 passed, 7 skipped** в sandbox.
|
||||
- Skipped: тесты, требующие реальных PNG-тайлов
|
||||
(`test_terrain_tile_available_z9_z10_z11[*]`,
|
||||
`test_terrain_tile_cache_control_immutable`) — корректное поведение
|
||||
через `_maybe_skip`.
|
||||
- Passed: whitelist-регрессия для всех трёх слоёв
|
||||
(`hypso/hillshade/tri`), 404 на `unknown_layer`, 404 на
|
||||
missing tile, 404 на невалидный zoom. Эти тесты доказывают,
|
||||
что фикс F-1 работает (для `tri` теперь возвращается
|
||||
`"Tile not found"`, а не `"Unknown layer"`).
|
||||
|
||||
## Качество кода
|
||||
|
||||
- Стиль соответствует существующему `app.js` (vanilla JS, JSDoc,
|
||||
комментарии-маркеры `// ET-NNN:`).
|
||||
- Изменение функции `applyTerrainLayer` минимально-инвазивное:
|
||||
новая нормализация в 4 строки + переменная `paint`, остальное
|
||||
новая нормализация в 4 строки + переменная `paint`, остальное —
|
||||
переименование параметра. Никаких ломок других call-sites
|
||||
(их всего 2, оба в `onTerrainCheckbox`).
|
||||
- Backend-фикс whitelist'а — 1 строка кода + docstring; не меняет
|
||||
сигнатуру endpoint'а и не вводит новых query/headers/code-path'ов
|
||||
(REQ-F-18 формально сохранён).
|
||||
- Все новые константы (`HILLSHADE_PAINT`, `TRI_PAINT`) UPPER_SNAKE_CASE,
|
||||
как принято в `app.js`.
|
||||
- Комментарии содержат ссылки на ADR-017, RTM-аргументы по stops.
|
||||
- Комментарии содержат ссылки на ADR-017, RTM-аргументы по stops,
|
||||
ссылку на review F-1 в backend-docstring.
|
||||
- Нет дублирования, нет dead code, нет `console.log`, нет
|
||||
закомментированного старого кода.
|
||||
|
||||
## Findings
|
||||
## Findings (текущая ревизия)
|
||||
|
||||
### P1 — Backend `terrain_tile` whitelist не пропускает `tri`
|
||||
### P3 — Комментарий в `HILLSHADE_PAINT` не упоминает MapLibre clamping ниже z9
|
||||
|
||||
**Severity:** P1 (must-fix перед merge) — может быть P0, если TRI на
|
||||
test-стенде действительно отдаётся через FastAPI.
|
||||
**Где.** `src/web/app.js:2728-2733`.
|
||||
|
||||
**Где.** `src/api/main.py:1243` (mainline, не тронут ET-013):
|
||||
**Замечание.** Stops opacity начинаются с `9, 0.65` — MapLibre сделает
|
||||
clamping на нижнем стопе, поэтому при z<9 (если когда-нибудь UI-gate
|
||||
уберут) opacity всё равно будет 0.65, что попадёт в render.
|
||||
В текущем scope не проблема (UI-gate отрубает чекбокс при z<9), но
|
||||
если в будущем порог понизят — нужно будет добавить нижний stop
|
||||
`8, 0.00`.
|
||||
|
||||
```python
|
||||
@app.get("/terrain/{layer}/{z}/{x}/{y}.png")
|
||||
async def terrain_tile(layer: str, z: int, x: int, y: int):
|
||||
"""Отдаёт растровые тайлы рельефа (hypso/hillshade)"""
|
||||
if layer not in ("hypso", "hillshade"):
|
||||
raise HTTPException(404, "Unknown layer")
|
||||
…
|
||||
```
|
||||
**Действие.** Опционально. **Не блокер.**
|
||||
|
||||
**Проблема.** Frontend клиент шлёт запросы на
|
||||
`/terrain/tri/{z}/{x}/{y}.png` (BRD §2.1, TRZ §1, `src/web/app.js:2826`),
|
||||
но backend whitelist возвращает `404 "Unknown layer"` для `layer == "tri"`.
|
||||
Это **пре-существующее** состояние, унаследованное из коммита
|
||||
`eda66ee` («feat: migrate prototype to canonical structure»), а не
|
||||
регрессия ET-013.
|
||||
### P3 — `from __future__ import annotations` в unit-тесте
|
||||
|
||||
Однако:
|
||||
**Где.** `tests/unit/test_terrain_paint.py:15`.
|
||||
|
||||
1. **TRZ §2 утверждает**: «`src/api/main.py:1240` (`terrain_tile`) —
|
||||
**без изменений**». Это формально соблюдено, но скрывает факт,
|
||||
что endpoint **никогда не поддерживал** `tri`.
|
||||
2. **ADR-017** в §«Контекст» пишет: «Раздача — `GET /terrain/{layer}/{z}/{x}/{y}.png`
|
||||
через FastAPI (`src/api/main.py:1240`), `Cache-Control: immutable`» —
|
||||
подразумевая, что TRI обслуживается тем же endpoint. По коду — нет.
|
||||
3. **AC-05/AC-06/AC-07/AC-08/AC-09/AC-12/AC-13/AC-21** требуют, чтобы
|
||||
слой `terrain-tri` рендерился на test-стенде. Если backend
|
||||
действительно 404-ит, то ET-013 не доставляет основной заявленной
|
||||
ценности («перепады высот на z9-z11»).
|
||||
**Замечание.** Не используется (нет forward-ref в аннотациях). Не вредит.
|
||||
|
||||
**Что должно быть проверено перед merge:**
|
||||
|
||||
- (a) **Если** на test-стенде есть внешний прокси (nginx/Caddy/Traefik
|
||||
на mva154), который перехватывает `/enduro/terrain/tri/…` и отдаёт
|
||||
PNG напрямую с диска `data/terrain/tri/…` — это нужно явно
|
||||
задокументировать в `07-infra-requirements.md` (сейчас этого нет)
|
||||
+ в ADR-017 (§«Контекст»). Тогда вердикт по этому пункту снимается.
|
||||
- (b) **Если** прокси нет — TRZ/ADR не годятся, **возвращать в Анализ**
|
||||
с правкой: добавить REQ на расширение whitelist
|
||||
(`if layer not in ("hypso", "hillshade", "tri")`) и integration-тест
|
||||
`test_tri_tile_available_z9_z10_z11`. Это +2 строки в `main.py` и
|
||||
один параметризованный тест.
|
||||
|
||||
В обоих случаях нужно отразить решение в `12-review.md` re-run / в
|
||||
follow-up commit перед approve.
|
||||
|
||||
**Доказательства, что это не «мнение reviewer'а»:**
|
||||
- `git grep "tri" -- src/api/` возвращает только matches в `LineString`
|
||||
(shapely), нет ни одного match по слою TRI.
|
||||
- `docker-compose.yml` показывает единственный сервис `app`
|
||||
(FastAPI на 5556) — внутри контейнера нет nginx.
|
||||
- `Dockerfile` устанавливает только `pip install` + `uvicorn` — без
|
||||
reverse-прокси.
|
||||
- Internal FastAPI mount-порядок: route `/terrain/{layer}/{z}/{x}/{y}.png`
|
||||
объявлен **до** `StaticFiles("/")`, поэтому даже если бы TRI лежал
|
||||
под `STATIC_DIR`, его сначала ловит API-route с whitelist.
|
||||
- Существующий integration-тест `test_unknown_terrain_layer_returns_404`
|
||||
проверяет, что `unknown_layer` возвращает 404 — но `tri` тоже
|
||||
попадает в эту ветку.
|
||||
|
||||
### P2 — Integration-тест не покрывает `tri`
|
||||
|
||||
**Где.** `tests/integration/test_terrain_z9_tiles.py:79-92`.
|
||||
|
||||
**Проблема.** `test_hillshade_tile_available_z9_z10_z11` параметризован
|
||||
по zoom, но не по layer. Даже если backend whitelist расширят (см. P1),
|
||||
останется нулевое покрытие endpoint'а для TRI на integration-уровне.
|
||||
|
||||
**Рекомендация.** После исправления P1 — параметризовать тест
|
||||
по `("hillshade", "tri")` и оставить `parametrize(zoom=[9, 10, 11])`.
|
||||
Без P1 fix этот тест всё равно будет упасть на `tri`, поэтому
|
||||
вынесен в P2, а не P1.
|
||||
|
||||
### P3 — Комментарий в `HILLSHADE_PAINT` чуть рассинхронизирован со stops
|
||||
|
||||
**Где.** `src/web/app.js:2730-2733`:
|
||||
|
||||
```js
|
||||
// Pre-z9 — hillshade не показывается (UI-минзум). На z9-z11 — максимальный
|
||||
// контраст и opacity, чтобы тени читались как на z8. К z12-z14 — возврат
|
||||
// к исходным значениям …
|
||||
```
|
||||
|
||||
**Замечание.** Формально верно (UI-gate отрубает чекбокс при z<9),
|
||||
но opacity-stops начинаются с `9, 0.65` — clamping MapLibre на
|
||||
z<9 даст opacity 0.65, что попадёт в render, если когда-нибудь
|
||||
UI-gate уберут. Не страшно и в scope не входит, но если в будущем
|
||||
кто-то понизит UI-минзум ещё ниже (z8), он не должен забыть
|
||||
добавить нижний stop. Опционально — добавить явный stop `8, 0.00`
|
||||
для safety. **Не блокер.**
|
||||
|
||||
### P3 — Минор: `from __future__ import annotations` в integration-тесте
|
||||
|
||||
**Где.** `tests/integration/test_terrain_z9_tiles.py:18`.
|
||||
|
||||
Не вредит, но не используется (нет forward-ref в аннотациях). Можно
|
||||
убрать. **Не блокер.**
|
||||
**Действие.** Опционально. **Не блокер.**
|
||||
|
||||
## Вердикт
|
||||
|
||||
**REQUEST_CHANGES.**
|
||||
**APPROVED.**
|
||||
|
||||
Причина: P1 finding по backend whitelist `tri`. Реализация ET-013
|
||||
сама по себе аккуратная и точно соответствует TRZ; но без подтверждения,
|
||||
что `/terrain/tri/{z}/{x}/{y}.png` реально возвращает 200 на test-стенде,
|
||||
ключевая часть acceptance criteria (AC-05..AC-09, AC-12, AC-13, AC-21)
|
||||
не может пройти, и merge приведёт к деплою фичи, которая не работает.
|
||||
- P0/P1 не найдено.
|
||||
- P1 из review v1 (backend whitelist) и P2 (integration coverage) —
|
||||
закрыты.
|
||||
- Оставшиеся два P3 — косметика, не влияют на функциональность.
|
||||
|
||||
**Дальнейшие шаги для разработчика/owner:**
|
||||
|
||||
1. Воспроизвести в test-среде:
|
||||
```bash
|
||||
curl -sI https://openclaw.mva154.duckdns.org/enduro/terrain/tri/8/156/79.png | head -1
|
||||
curl -sI https://openclaw.mva154.duckdns.org/enduro/terrain/tri/10/623/318.png | head -1
|
||||
```
|
||||
- **Если 200** → есть внешний прокси, задокументировать его в
|
||||
`07-infra-requirements.md` и в ADR-017 §«Контекст», добавить
|
||||
integration-тест с `requests` (а не TestClient) — отдельный
|
||||
mark `@pytest.mark.requires_test_env`. После этого re-review.
|
||||
- **Если 404** → вернуть в Анализ или добавить в текущий PR
|
||||
минимальный fix: в `src/api/main.py:1243` заменить whitelist на
|
||||
`("hypso", "hillshade", "tri")`, параметризовать integration-тест
|
||||
по `("hillshade", "tri")`, обновить `09-validation.md`/`AC-19`
|
||||
curl-проверкой на TRI z9-z11. После этого re-review.
|
||||
|
||||
2. После решения P1 — устранить P2 (параметризация по layer) и
|
||||
опционально P3.
|
||||
|
||||
3. После re-review при отсутствии P0/P1 — `verdict: APPROVED`.
|
||||
Реализация ET-013 точно соответствует TRZ REQ-F-01..F-21 и ADR-017.
|
||||
Тестовое покрытие достаточное:
|
||||
- AC-01/02/04/05/15/17/22 — закрыты unit-тестами (зелёные).
|
||||
- AC-16 — закрыт integration-тестами (структурно корректно, skip без данных, whitelist-регрессия зелёная).
|
||||
- Поведенческие AC (AC-03, AC-06..AC-13, AC-19, AC-21) — корректно
|
||||
переданы Тестеру для исполнения в test-среде.
|
||||
|
||||
## Сводная таблица findings
|
||||
|
||||
| ID | Severity | Где | Кратко | Действие |
|
||||
|---|---|---|---|---|
|
||||
| F-1 | P1 | `src/api/main.py:1243` | backend whitelist не пропускает `tri` → AC-05..AC-09, AC-12, AC-13, AC-21 не выполнимы | проверить прокси на test, либо расширить whitelist + integration-тест |
|
||||
| F-2 | P2 | `tests/integration/test_terrain_z9_tiles.py:79-92` | integration-тест не параметризован по layer | после F-1 — добавить `("hillshade", "tri")` в `parametrize` |
|
||||
| F-3 | P3 | `src/web/app.js:2730-2733` | комментарий не учитывает MapLibre clamping ниже z9 | опциональный явный stop `8, 0.00` |
|
||||
| F-4 | P3 | `tests/integration/test_terrain_z9_tiles.py:18` | `from __future__ import annotations` не используется | косметика |
|
||||
| F-3 | P3 | `src/web/app.js:2728-2733` | комментарий не учитывает MapLibre clamping ниже z9 | опционально добавить явный stop `8, 0.00` |
|
||||
| F-5 | P3 | `tests/unit/test_terrain_paint.py:15` | `from __future__ import annotations` неиспользован | косметика |
|
||||
|
||||
P0/P1 присутствуют → **REQUEST_CHANGES**.
|
||||
P0/P1 отсутствуют → **APPROVED**.
|
||||
|
||||
Reference in New Issue
Block a user