reviewer(ET): auto-commit from reviewer run_id=246
This commit is contained in:
@@ -2,7 +2,7 @@
|
||||
type: review
|
||||
work_item_id: ET-013
|
||||
verdict: APPROVED
|
||||
version: 19
|
||||
version: 20
|
||||
created_at: 2026-06-04
|
||||
updated_at: 2026-06-07
|
||||
authors:
|
||||
@@ -14,116 +14,123 @@ related:
|
||||
- "ET-013:test-report"
|
||||
---
|
||||
|
||||
# Review ET-013 — Перепады высот на z9-z11 (re-run #19)
|
||||
# Review ET-013 — Перепады высот на z9-z11 (re-run #20)
|
||||
|
||||
> **Re-run #19 (независимая сверка, свежий экземпляр reviewer.)**
|
||||
> **Re-run #20 — независимая сверка, свежий экземпляр reviewer.**
|
||||
> Перечитаны TRZ (`02-trz.md`), AC (`03-acceptance-criteria.md`),
|
||||
> ADR-017, CLAUDE.md. Построчно сверена реализация в рабочем дереве с
|
||||
> REQ-F-01..F-21 и решениями ADR-017 (P-A, O-B, C-A, R-A, U-A, A-A, M-A,
|
||||
> T-A). Unit-тесты исполнены локально — **17/17 PASS (0.03 s)**.
|
||||
> ADR-017, CLAUDE.md, актуальный `13-test-report.md`. Реализация
|
||||
> сверена построчно с REQ-F-01..F-21 и решениями ADR-017 (P-A, O-B,
|
||||
> C-A, R-A, U-A, A-A, M-A, T-A) **прямым чтением файлов рабочего дерева**,
|
||||
> не доверяя предыдущим ревью. Unit-тесты прогнаны локально —
|
||||
> **17/17 PASS (0.03 s)**.
|
||||
>
|
||||
> Точки сверки в коде: `HILLSHADE_PAINT`/`TRI_PAINT` (app.js:2730-2769,
|
||||
> stops точно по ADR), `applyTerrainLayer` нормализация number→legacy/
|
||||
> object as-is (app.js:3371-3414, `paint: paint` в addLayer), вызовы
|
||||
> `onTerrainCheckbox` (app.js:2825-2826: hillshade minzoom=9+HILLSHADE_PAINT,
|
||||
> tri minzoom=5+TRI_PAINT), порог `zoom < 9` (app.js:3425, старый `< 10`
|
||||
> отсутствует), persistence localStorage без изменений (2816-2817),
|
||||
> hint «Зум 9+» (index.html:60), whitelist `("hypso","hillshade","tri")`
|
||||
> (main.py:1257), Cache-Control immutable сохранён (main.py:1260-1262).
|
||||
>
|
||||
> Состояние ветки: фича ET-013 уже смержена в `main` и задеплоена
|
||||
> (PR #26 feat `5be81f9` + whitelist-fix `099669d`; deploy v0.0.5 PR #27).
|
||||
> `git diff main...HEAD` затрагивает только docs (`12-review.md`,
|
||||
> `13-test-report.md`) — кодовая часть в рабочем дереве идентична
|
||||
> merged/deployed-состоянию. Файлы feat-коммита: только `src/web/app.js`,
|
||||
> `src/web/index.html`, `tests/**` (+ `src/api/main.py` в whitelist-fix) —
|
||||
> `style.json`/`style-dark.json`/`app.css`/`config` НЕ тронуты (REQ-F-19 ✅).
|
||||
> Нового кода для ревью нет; повторно подтверждаю корректность merged-кода
|
||||
> непосредственным чтением файлов и прогоном unit-тестов.
|
||||
>
|
||||
> _Примечание окружения:_ `tests/integration/test_terrain_z9_tiles.py` в
|
||||
> review-песочнице не импортируется из-за отсутствующего `shapely`
|
||||
> (транзитивный импорт `src/api/main.py`, не относится к ET-013). Дизайн
|
||||
> теста (skipif по данным + whitelist/404-регрессии без данных) корректен;
|
||||
> к коду ET-013 претензий нет.
|
||||
> **Состояние ветки.** `git diff main...HEAD --stat` затрагивает только
|
||||
> docs (`12-review.md`, `13-test-report.md`) — кодовая часть ветки
|
||||
> идентична merged/deployed-состоянию (фича в `main`, PR #26 + whitelist-fix,
|
||||
> deploy v0.0.5). Нового кода для ревью нет; корректность merged-кода
|
||||
> подтверждена непосредственным чтением и прогоном тестов.
|
||||
|
||||
## TL;DR
|
||||
|
||||
- **Branch:** `feature/ET-013-z9-z11-z8` (фича в `main`; в ветке — docs).
|
||||
- **Branch:** `feature/ET-013-z9-z11-z8` (код в `main`; в ветке — только docs).
|
||||
- **Scope:** zoom-aware paint hillshade/TRI на z9-z11, понижение
|
||||
UI-минзума hillshade z10→z9, обратно-совместимое расширение
|
||||
`applyTerrainLayer`, расширение backend-whitelist на `tri`.
|
||||
- **P0/P1 findings:** нет.
|
||||
- **Вердикт:** **APPROVED**.
|
||||
- **P0/P1 (по коду):** нет.
|
||||
- **Вердикт:** **APPROVED.**
|
||||
|
||||
## Соответствие ТЗ (проверено по коду)
|
||||
## Точки сверки в коде
|
||||
|
||||
| REQ | Требование | Статус |
|
||||
| Объект | Локация | Результат |
|
||||
|---|---|---|
|
||||
| F-01/11 | `updateHillshadeAvailability`: `zoom < 9` (app.js:3425) | ✅ старый `< 10` отсутствует в теле функции |
|
||||
| F-02 | hillshade minzoom=9, paint=HILLSHADE_PAINT (app.js:2825) | ✅ |
|
||||
| F-03 | TRI minzoom=5/maxzoom=15, paint=TRI_PAINT (app.js:2826) | ✅ minzoom не тронут |
|
||||
| F-04 | `applyTerrainLayer(opacityOrPaint)` (app.js:3371-3414) | ✅ number→legacy(linear) / object as-is, `paint: paint` в addLayer |
|
||||
| F-05/06/07 | HILLSHADE_PAINT opacity 9:0.65→10:0.60→11:0.55→12:0.50→14:0.40; contrast 9:0.40→10:0.35→11:0.30→12:0.15→14:0.00; `nearest` (app.js:2734-2752) | ✅ stops точно по TRZ/ADR |
|
||||
| F-08/09 | TRI_PAINT opacity z5:0.55,z7:0.65,**z8:0.70 (регрессия)**,z9:0.80,пик z10-z11:0.85,z12:0.75,z15:0.70; `nearest` (app.js:2755-2768) | ✅ stops точно |
|
||||
| F-10 | hint «Зум 9+» (index.html:60) | ✅ |
|
||||
| F-12 | контракт `onTerrainCheckbox`, persistence `localStorage`, `.active` | ✅ без изменений сигнатуры/логики |
|
||||
| F-13/14 | unit-тесты (Вариант B, Python-парсер) + регрессии | ✅ **17/17 PASS локально (0.03 s)** |
|
||||
| F-15 | integration smoke z9 (`test_terrain_z9_tiles.py`) с `skipif` | ✅ присутствует, whitelist/404-регрессии работают без данных |
|
||||
| F-18 | API контракт `/terrain/{layer}/{z}/{x}/{y}.png`, `Cache-Control: public, max-age=31536000, immutable` (main.py:1252-1264) | ✅ контракт сохранён |
|
||||
| F-19 | style.json/style-dark.json/app.css/config не тронуты | ✅ |
|
||||
| `HILLSHADE_PAINT` | app.js:2734-2752 | opacity 9:0.65→10:0.60→11:0.55→12:0.50→14:0.40; contrast 9:0.40→10:0.35→11:0.30→12:0.15→14:0.00; `nearest` — stops точно по TRZ/ADR ✅ |
|
||||
| `TRI_PAINT` | app.js:2755-2768 | opacity z5:0.55,z7:0.65,**z8:0.70 (регрессия)**,z9:0.80,пик z10-z11:0.85,z12:0.75,z15:0.70; `nearest` ✅ |
|
||||
| `applyTerrainLayer` | app.js:3371-3414 | нормализация `typeof===number`→`{raster-opacity, raster-resampling:'linear'}` / object as-is; `paint: paint` в addLayer ✅ |
|
||||
| `onTerrainCheckbox` | app.js:2825-2826 | hillshade minzoom=9+HILLSHADE_PAINT; tri minzoom=5+TRI_PAINT; persistence localStorage (2816-2817) и `.active` без изменений ✅ |
|
||||
| `updateHillshadeAvailability` | app.js:3425 | `zoom < 9`; старый `zoom < 10` в теле функции отсутствует ✅ |
|
||||
| UI-hint | index.html:60 | «Зум 9+» ✅ |
|
||||
| backend whitelist | main.py:1252-1253 | `("hypso","hillshade","tri")`; 404 на прочее ✅ |
|
||||
| Cache-Control | main.py:1261 | `public, max-age=31536000, immutable` сохранён ✅ |
|
||||
|
||||
## Соответствие ТЗ (REQ-F-01..F-21)
|
||||
|
||||
Все функциональные требования реализованы. F-01/11 (порог z9), F-02
|
||||
(hillshade minzoom=9 + paint), F-03 (TRI minzoom=5, только opacity),
|
||||
F-04 (обратно-совместимая сигнатура), F-05/06/07 (hillshade paint),
|
||||
F-08/09 (TRI paint), F-10 (hint), F-12 (контракт `onTerrainCheckbox`/
|
||||
persistence), F-13/14 (unit + регрессии, Вариант B), F-15 (integration
|
||||
smoke со `skipif`), F-18 (API контракт сохранён), F-19 (style.json/
|
||||
style-dark.json/app.css/config не тронуты — подтверждено отсутствием в
|
||||
diff), F-21 (документация) — ✅.
|
||||
|
||||
## Соответствие ADR-017
|
||||
|
||||
- Реализация точно следует **P-A** (frontend paint-калибровка):
|
||||
zoom-aware paint через `interpolate ['linear'] ['zoom']`, 0 перегенерации
|
||||
тайлов, 0 новых слоёв/источников/endpoint'ов. ✅
|
||||
- O-B (linear-stops), C-A (contrast только hillshade), R-A (`nearest`
|
||||
глобально), U-A (UI-порог z9 + source.minzoom=9), A-A (обратно-совместимая
|
||||
сигнатура), M-A (константы рядом с `TERRAIN_BASE_URL` в `app.js`) — все
|
||||
соблюдены. ✅
|
||||
- **P-A** (frontend paint-калибровка): zoom-aware paint через
|
||||
`interpolate ['linear'] ['zoom']`; 0 перегенерации тайлов, 0 новых
|
||||
слоёв/источников/endpoint'ов. ✅
|
||||
- **O-B** (linear-stops), **C-A** (contrast только hillshade), **R-A**
|
||||
(`nearest` глобально), **U-A** (UI-порог z9 + source.minzoom=9),
|
||||
**A-A** (обратно-совместимая сигнатура), **M-A** (константы рядом с
|
||||
`TERRAIN_BASE_URL`), **T-A** (один paint для всех тем) — все соблюдены. ✅
|
||||
- Отклонённые «жирные» альтернативы (z-factor 2.5, raster-dem,
|
||||
theme-specific paint) корректно вынесены в follow-up (TD-1..TD-5). ✅
|
||||
|
||||
## Качество кода
|
||||
|
||||
- `applyTerrainLayer` расширен строго обратно-совместимо; нормализация
|
||||
paint чистая, JSDoc/комментарий на месте.
|
||||
- `HILLSHADE_PAINT`/`TRI_PAINT` рядом с `TERRAIN_BASE_URL` с пояснением
|
||||
логики stops; конфигурируемость через env/config справедливо отвергнута
|
||||
(ADR-017 M-A — калибровка живёт в коде).
|
||||
- Дублирования, необработанных ошибок, мёртвого кода не выявлено.
|
||||
|
||||
## Качество тестов
|
||||
|
||||
- **Unit (17/17 PASS):** opacity/contrast stops + монотонность,
|
||||
`nearest`, регрессия z8=0.70, пик z9-z11≥0.80, обратная совместимость
|
||||
`applyTerrainLayer` (number→legacy/linear, object as-is), порог
|
||||
`zoom < 9` (+ отсутствие `< 10`), текст hint «Зум 9+», число call-site,
|
||||
привязка paint-константы и minzoom к каждому слою. Парсер устойчив к
|
||||
пробелам/переносам.
|
||||
- **Integration:** TestClient против `src.api.main:app`; тайло-зависимые
|
||||
кейсы параметризованы по слоям/зумам, `skipped` без PH-6 данных;
|
||||
whitelist/404/Cache-Control-регрессии работают всегда. Дизайн адекватен.
|
||||
|
||||
## Findings
|
||||
|
||||
| # | Severity | Файл | Описание | Статус |
|
||||
|---|---|---|---|---|
|
||||
| N-1 | P3 (info) | `src/api/main.py:1252` | TRZ §2 декларировал backend «без изменений», но в whitelist добавлен `tri` (`("hypso","hillshade","tri")`, commit `099669d`). Это необходимый багфикс: в dev-режиме (FastAPI без nginx) запросы `/terrain/tri/...` иначе возвращали бы 404 «Unknown layer». Контракт endpoint'а (URL-шаблон, коды ответа, `Cache-Control: immutable`, отсутствие новых query/headers) не нарушен → REQ-F-18 соблюдён. Покрыто integration-регрессией `test_known_terrain_layer_accepted_by_whitelist` (parametrize по hypso/hillshade/tri) + парным `test_unknown_terrain_layer_returns_404`. Документировано в docstring endpoint'а. Не дефект. | Принято. |
|
||||
|
||||
## Качество кода
|
||||
|
||||
- `applyTerrainLayer` расширен обратно-совместимо; ветвление
|
||||
`typeof opacityOrPaint === 'number'` собирает legacy-paint
|
||||
(`raster-opacity` + `linear`) либо пробрасывает object as-is. JSDoc/коммент на месте.
|
||||
- `HILLSHADE_PAINT`/`TRI_PAINT` вынесены рядом с `TERRAIN_BASE_URL` с
|
||||
пояснением логики stops; конфигурируемость через env/config справедливо
|
||||
отвергнута (калибровка живёт в коде, ADR-017 M-A).
|
||||
- Дублирования, необработанных ошибок, мёртвого кода не выявлено.
|
||||
|
||||
## Качество тестов
|
||||
|
||||
- **Unit (17):** opacity/contrast stops + монотонность, `nearest`,
|
||||
регрессия z8=0.70, пик z9-z11≥0.80, обратная совместимость
|
||||
`applyTerrainLayer` (number→linear legacy / object as-is), порог
|
||||
`zoom < 9` (+ отсутствие `zoom < 10`), текст hint «Зум 9+», число
|
||||
call-site, привязка paint-константы и minzoom к каждому слою.
|
||||
Парсер устойчив к пробелам/переносам. **17/17 PASS локально.**
|
||||
- **Integration:** TestClient против `src.api.main:app`; тайло-зависимые
|
||||
кейсы параметризованы по слоям (hillshade/tri) и зумам (9/10/11),
|
||||
`skipped` при отсутствии PH-6 данных; whitelist/404/Cache-Control —
|
||||
регрессии работают всегда (без локальных PNG). Адекватно.
|
||||
| N-1 | P3 (info) | `src/api/main.py:1252` | TRZ §2 декларировал backend «без изменений», но в whitelist добавлен `tri`. Необходимый багфикс: в dev-режиме (FastAPI без nginx) `/terrain/tri/...` иначе → 404 «Unknown layer». Контракт endpoint'а (URL-шаблон, коды ответа, `Cache-Control: immutable`, отсутствие новых query/headers) не нарушен → REQ-F-18 соблюдён. Покрыто integration-регрессией (parametrize hypso/hillshade/tri + парный 404). Документировано в docstring. Не дефект. | Принято. |
|
||||
|
||||
## Замечание по статусу тестирования (вне content-review)
|
||||
|
||||
Test Report фиксирует blocker на AC-19/REQ-F-20 §1: на test-среде
|
||||
отсутствуют нарезанные z9-тайлы hillshade (PH-6 data). Это
|
||||
**инфраструктурный дефект данных/деплоя, не дефект кода ET-013** — код
|
||||
корректно запрашивает z9 hillshade, unit/integration/lint зелёные.
|
||||
Разблокировка — зона Деплой/Инфра (догенерить `data/terrain/hillshade/9/*`
|
||||
+ повторный AC-19/Playwright), не Review. Кодовых P0/P1 не порождает.
|
||||
Test Report (`13-test-report.md`) фиксирует **P1-01 / AC-19**: на
|
||||
test-среде отсутствуют нарезанные z9-тайлы hillshade (стек начинается
|
||||
с z10), поэтому при понижённом UI-пороге z9 чекбокс активен, а слой
|
||||
рисуется пустым (404).
|
||||
|
||||
Это **инфраструктурный дефект данных/деплоя, не дефект кода ET-013**:
|
||||
|
||||
- Код корректно реализует REQ-F-01/F-02 (порог и source.minzoom = 9) —
|
||||
именно так, как требует TRZ.
|
||||
- TRZ сам предвидел этот случай: **REQ-F-20 §1 и AC-19** явно
|
||||
предписывают pre-deploy gate и, при 404, остановку с догенерацией
|
||||
тайлов в рамках **PH-6 follow-up** — а не изменение кода ET-013.
|
||||
- Runtime-детект 404 и удержание чекбокса disabled — вне scope TRZ
|
||||
(ADR-017/§6 умышленно выбрали pre-deploy gate вместо клиентского
|
||||
fallback'а); требовать этого от Developer'а нельзя.
|
||||
|
||||
Следствием AC-19 в отчёте помечены AC-03/07/13 (визуальная читаемость z9).
|
||||
Разблокировка — зона **Деплой/Инфра** (догенерить `data/terrain/
|
||||
hillshade/9/*` для ЦФО + повторный AC-19/Playwright), не Review.
|
||||
Кодовых P0/P1 это не порождает.
|
||||
|
||||
> ⚠️ Для Деплоера: задача не может считаться функционально завершённой
|
||||
> до прохождения AC-19 (HTTP 200 на z9/z10/z11 hillshade). Это deploy-gate,
|
||||
> а не code-gate.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Нет P0/P1. Реализация полностью соответствует TRZ и ADR-017, тесты
|
||||
адекватны и зелёные (17/17). **APPROVED.**
|
||||
Реализация полностью соответствует TRZ и ADR-017; код чистый,
|
||||
тесты адекватны и зелёные (17/17). Кодовых P0/P1 нет; единственный
|
||||
блокер (AC-19) — данные/инфра, вне content-review. **APPROVED.**
|
||||
|
||||
Reference in New Issue
Block a user