reviewer(ET): auto-commit from reviewer run_id=250
This commit is contained in:
@@ -2,7 +2,7 @@
|
||||
type: review
|
||||
work_item_id: ET-013
|
||||
verdict: APPROVED
|
||||
version: 21
|
||||
version: 22
|
||||
created_at: 2026-06-04
|
||||
updated_at: 2026-06-07
|
||||
authors:
|
||||
@@ -14,159 +14,99 @@ related:
|
||||
- "ET-013:test-report"
|
||||
---
|
||||
|
||||
# Review ET-013 — Перепады высот на z9-z11 (re-run #21)
|
||||
# Review ET-013 — Перепады высот на z9-z11 (re-run #22)
|
||||
|
||||
> **Re-run #21 — независимая сверка, свежий экземпляр reviewer.**
|
||||
> Перечитаны TRZ (`02-trz.md`), AC (`03-acceptance-criteria.md`),
|
||||
> 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-тесты прогнаны локально —
|
||||
> **Re-run #22 — независимая сверка, свежий экземпляр reviewer.**
|
||||
> Перечитаны `02-trz.md`, `03-acceptance-criteria.md`, `06-adr/ADR-017`,
|
||||
> `CLAUDE.md`, актуальный `13-test-report.md` (v13, verdict BLOCKED).
|
||||
> Реализация сверена построчно с REQ-F-01..F-21 и решениями ADR-017
|
||||
> (P-A, O-B, C-A, R-A, U-A, T-A) **прямым чтением файлов рабочего
|
||||
> дерева**, без доверия предыдущим ревью. Unit-тесты прогнаны локально —
|
||||
> **17/17 PASS (0.03 s)**.
|
||||
|
||||
## 1. Объём ревью
|
||||
|
||||
Проверка по четырём осям: соответствие ТЗ, соответствие ADR-017,
|
||||
качество кода, качество тестов.
|
||||
|
||||
Реально прочитанные артефакты кода:
|
||||
- `src/web/app.js` — блок констант (2728-2768), `onTerrainCheckbox`
|
||||
(2808-2827), `applyTerrainLayer` (3360-3414), `updateHillshadeAvailability`
|
||||
(3416-3434).
|
||||
- `src/web/index.html` — `#terrain-hillshade-hint` (строка 60).
|
||||
- `tests/unit/test_terrain_paint.py` (полностью).
|
||||
- `tests/integration/test_terrain_z9_tiles.py` (полностью).
|
||||
|
||||
Состояние ветки: `git diff main...HEAD` затрагивает только docs
|
||||
(`12-review.md`, `13-test-report.md`) — кодовая часть ветки совпадает с
|
||||
merged/deployed-состоянием (фича в `main`, deploy v0.0.5). Новый код для
|
||||
ревью отсутствует; корректность merged-кода подтверждена повторно.
|
||||
Четыре оси: соответствие ТЗ, соответствие ADR-017, качество кода,
|
||||
качество тестов. `git diff main...HEAD` затрагивает только docs
|
||||
(`12-review.md`, `13-test-report.md`) — код ET-013 уже в `main`
|
||||
(`5be81f9` feat, `099669d` fix-whitelist); ревью выполнено по
|
||||
фактическому состоянию рабочего дерева.
|
||||
|
||||
## 2. Соответствие ТЗ (REQ-F-01..F-21)
|
||||
|
||||
| REQ | Требование | Факт в коде | Статус |
|
||||
|-----|-----------|-------------|--------|
|
||||
| F-01 | UI-минзум hillshade `zoom < 9` | `app.js:3425` `if (zoom < 9)` | ✅ |
|
||||
| F-02 | source `terrain-hillshade-source` minzoom=9 | `app.js:2825` вызов с minzoom=9 | ✅ |
|
||||
| F-03 | TRI minzoom=5 без изменений | `app.js:2826` minzoom=5 | ✅ |
|
||||
| F-04 | `applyTerrainLayer` принимает number\|object, обратно-совместимо | `app.js:3371,3378-3380` нормализация по `typeof` | ✅ |
|
||||
| F-05 | HILLSHADE_PAINT `raster-opacity` zoom-aware | `app.js:2735-2742`, stops 9→0.65 … 14→0.40 | ✅ |
|
||||
| F-06 | HILLSHADE_PAINT `raster-contrast` | `app.js:2743-2750`, 9→0.40 … 14→0.00 | ✅ |
|
||||
| F-07 | HILLSHADE_PAINT `raster-resampling: nearest` | `app.js:2751` | ✅ |
|
||||
| F-08 | TRI_PAINT `raster-opacity` zoom-aware, z8=0.70, пик z9-z11 | `app.js:2756-2766` | ✅ |
|
||||
| F-09 | TRI_PAINT `raster-resampling: nearest` | `app.js:2767` | ✅ |
|
||||
| F-10 | hint «Зум 9+» | `index.html:60` | ✅ |
|
||||
| F-11 | `updateHillshadeAvailability` порог 9 | см. F-01 | ✅ |
|
||||
| F-12 | контракт `onTerrainCheckbox` сохранён (localStorage, `.active`) | `app.js:2816-2821` без изменений | ✅ |
|
||||
| F-13 | unit-тесты paint (Вариант B, Python-парсер) | `test_terrain_paint.py`, 17 тестов | ✅ |
|
||||
| F-14 | регрессионные тесты (порог, hint, callers) | `test_*_threshold_lowered_to_9`, `*_hint_text*`, `*_caller_count` | ✅ |
|
||||
| F-15 | integration smoke z9 + skipif | `test_terrain_z9_tiles.py`, `@pytest.mark.skipif` через `_maybe_skip` | ✅ |
|
||||
| F-16 | UI Playwright | см. §6 — вне scope code-review, ведётся тестировщиком | n/a |
|
||||
| F-17 | persistence без миграции | ключи `terrain-hillshade`/`terrain-tri` без изменений | ✅ |
|
||||
| F-18 | API контракт не меняется | `src/api/main.py` не тронут | ✅ |
|
||||
| F-19 | конфиги/стили не меняются | `style.json`/`style-dark.json`/`app.css` не тронуты | ✅ |
|
||||
| F-20 | деплой и валидация | см. §5 — AC-19 ведётся тестировщиком | n/a |
|
||||
| F-21 | документация | артефакты на месте | ✅ |
|
||||
| REQ | Что проверено | Статус |
|
||||
|---|---|---|
|
||||
| F-01 / F-11 | `updateHillshadeAvailability`: `if (zoom < 9)` (app.js:3425), `< 10` отсутствует | ✅ |
|
||||
| F-02 | hillshade-вызов `applyTerrainLayer(..., HILLSHADE_PAINT, 9, 15)` (app.js:2825) | ✅ |
|
||||
| F-03 | TRI-вызов `applyTerrainLayer(..., TRI_PAINT, 5, 15)` — minzoom=5 без изменений (app.js:2826) | ✅ |
|
||||
| F-04 | `applyTerrainLayer(id, tileUrl, enabled, opacityOrPaint, minzoom, maxzoom)`; нормализация `typeof === 'number'` → legacy paint (app.js:3371-3380) — обратно-совместимо | ✅ |
|
||||
| F-05/06/07 | `HILLSHADE_PAINT`: opacity-interpolate (9→0.65,10→0.60,11→0.55,12→0.50,14→0.40), contrast-interpolate (9→0.40…14→0.00), `'nearest'` (app.js:2734-2752) | ✅ |
|
||||
| F-08/09 | `TRI_PAINT`: opacity (5→0.55,7→0.65,8→0.70,9→0.80,10→0.85,11→0.85,12→0.75,15→0.70), `'nearest'` (app.js:2755-2768) | ✅ |
|
||||
| F-10 | `#terrain-hillshade-hint` = «Зум 9+» (index.html:60) | ✅ |
|
||||
| F-12 | `onTerrainCheckbox` контракт + `localStorage` (`terrain-hillshade`/`terrain-tri`) без изменений (app.js:2808-2827) | ✅ |
|
||||
| F-13/14 | `tests/unit/test_terrain_paint.py` — 17 тестов: stops, монотонность, регрессия z8=0.70, пик z9-z11≥0.80, `nearest`, обратная совместимость, порог 9, hint, call-count | ✅ |
|
||||
| F-15 | `tests/integration/test_terrain_z9_tiles.py` — параметризация по слою (hillshade/tri) и зуму (9/10/11), `skipif` при отсутствии данных, 404-регрессии независимы от данных | ✅ |
|
||||
| F-17/F-18/F-19 | persistence без миграции; API `terrain_tile` (main.py:1240+) и стили/конфиги не тронуты | ✅ |
|
||||
|
||||
Значения stops в `HILLSHADE_PAINT`/`TRI_PAINT` совпадают с TRZ §3 и
|
||||
ADR-017 §«Решение» п.4-5 бит-в-бит. Регрессионные опорные точки
|
||||
(hillshade z14 opacity 0.40 / contrast 0.00; TRI z8 0.70) присутствуют
|
||||
явными stops → AC-06 и AC-10 удовлетворяются «по построению».
|
||||
ТЗ реализовано полностью и буквально. Расхождений с REQ нет.
|
||||
|
||||
## 3. Соответствие ADR-017
|
||||
|
||||
| Решение ADR | Реализовано |
|
||||
|-------------|-------------|
|
||||
| P-A — frontend paint-калибровка, 0 правок backend/тайлов/стилей | ✅ |
|
||||
| U-A — UI-порог 10→9, source.minzoom 9, hint «Зум 9+» | ✅ |
|
||||
| A-A — `opacityOrPaint: number\|object`, нормализация внутри | ✅ |
|
||||
| O-B — linear `interpolate` со stops | ✅ |
|
||||
| C-A — `raster-contrast` zoom-aware только для hillshade | ✅ (TRI контраст не трогается) |
|
||||
| R-A — `nearest` для обоих слоёв | ✅ |
|
||||
| T-A — один paint для всех тем | ✅ |
|
||||
| M-A — константы в `app.js` рядом с `TERRAIN_BASE_URL` | ✅ |
|
||||
- **P-A** (frontend paint-калибровка) — ✅ вся правка в `app.js` + 1 строка HTML, без перегенерации тайлов.
|
||||
- **O-B** (linear interpolate stops) — ✅ TRI stops совпадают с ADR §O-B дословно, точка `8→0.70` явная (регрессия BRD F-11 / AC-06).
|
||||
- **C-A** (`raster-contrast` zoom-aware для hillshade) — ✅.
|
||||
- **R-A** (`'nearest'` для обоих слоёв, глобально — MapLibre не поддерживает interpolate для resampling) — ✅.
|
||||
- **U-A** (UI-гейт hillshade понижен до z9, не до z8) — ✅.
|
||||
- **T-A** (theme-specific paint отложен в follow-up, не в ET-013) — ✅ в коде единый paint, без подписки на theme-change.
|
||||
|
||||
Нарушений ADR нет. Классификация `minor-change` соблюдена: затронуты
|
||||
ровно `app.js`, `index.html` и два тест-файла; `main.py`, `data/terrain/*`,
|
||||
стили, конфиги, Docker/nginx — не тронуты.
|
||||
Нарушений ADR нет.
|
||||
|
||||
## 4. Качество кода
|
||||
|
||||
- Расширение `applyTerrainLayer` обратно-совместимо: при числовом
|
||||
аргументе собирается legacy-paint `{raster-opacity, raster-resampling:'linear'}`,
|
||||
при объекте — paint прокидывается как есть (`app.js:3378-3380`). Чисто,
|
||||
без дублирования.
|
||||
- Вызовов `applyTerrainLayer` ровно два (оба в `onTerrainCheckbox`),
|
||||
плюс одно объявление — подтверждено grep'ом. Скрытых вызовов со старым
|
||||
числовым контрактом, которые могли бы сломаться, нет.
|
||||
- Поведение при отключении слоя (`removeLayer`+`removeSource`) сохранено.
|
||||
- Комментарии ссылаются на ET-013/ADR-017 — трассируемость есть.
|
||||
- Деградация при отсутствии тайлов graceful: source с minzoom=9 без
|
||||
PNG отдаёт пустой слой, JS-ошибок/исключений не возникает (см. §5).
|
||||
- Сигнатура `applyTerrainLayer` расширена обратно-совместимо; ветка
|
||||
`typeof === 'number'` сохраняет старый контракт (`raster-opacity` +
|
||||
`linear`). Единственные вызовы — в `onTerrainCheckbox`; внешних
|
||||
потребителей нет, контракт всё равно сохранён.
|
||||
- Константы paint вынесены рядом с `TERRAIN_BASE_URL`, снабжены
|
||||
комментариями со ссылкой на ADR-017 и обоснованием stops. Магических
|
||||
чисел в логике нет — калибровка изолирована в декларациях.
|
||||
- Нет дублирования, нет мёртвого кода, z-order вставки слоя сохранён.
|
||||
|
||||
Code-findings уровня P0/P1/P2 — нет.
|
||||
Замечаний P0/P1/P2 по качеству нет.
|
||||
|
||||
## 5. Кросс-стейдж заметка: AC-19 (данные PH-6) — НЕ code-finding
|
||||
## 5. Качество тестов
|
||||
|
||||
`13-test-report.md` фиксирует объективный блокер **AC-19**: на test-среде
|
||||
`GET /terrain/hillshade/9/...` отдаёт **404** — тайлы hillshade нарезаны
|
||||
с z10, слой z9 на диске отсутствует. Из-за этого AC-03/AC-07/AC-13
|
||||
(видимость hillshade на z9) на живой среде проваливаются, и тестировщик
|
||||
держит вердикт **BLOCKED**.
|
||||
- Unit (Вариант B по REQ-F-13) — статический парсинг `app.js`/`index.html`
|
||||
с балансировкой скобок и извлечением zoom-stops; проверяются не только
|
||||
наличие, но и значения, монотонность, отсутствие старого порога
|
||||
`< 10`. Прогон локально: **17/17 PASS**.
|
||||
- Integration — параметризация по обоим слоям endpoint'а (review F-2),
|
||||
whitelist-регрессия `hypso/hillshade/tri` vs «Unknown layer» (review
|
||||
F-1), cache-header (`immutable`, `max-age=31536000`). 404-проверки не
|
||||
зависят от наличия PNG-данных; тайловые тесты корректно `skipif`.
|
||||
- В песочнице reviewer integration-модуль не собирается из-за
|
||||
отсутствия `shapely` (импорт `src.api.main`). Это **ограничение
|
||||
окружения, не дефект**: `shapely==2.0.4` объявлен в `pyproject.toml`,
|
||||
а `13-test-report.md` фиксирует зелёный прогон в venv
|
||||
(**23 passed / 7 skipped**, полный регресс **254 passed / 7 skipped /
|
||||
0 failed**, `ruff` — clean).
|
||||
|
||||
Позиция reviewer:
|
||||
- Это **дефект данных/инфраструктуры (PH-6 follow-up)**, а не дефект кода
|
||||
ET-013. TRZ REQ-F-20 §1 и AC-19 прямо предусматривают этот сценарий
|
||||
(«если 404 — merge приостанавливается, открыть PH-6 follow-up по
|
||||
нарезке z9-тайлов»).
|
||||
- Код merge-safe: предпосылка ADR-017 U-A («тайлы z9 на диске есть»)
|
||||
оказалась неверна, но это не ломает приложение — слой просто пуст на z9,
|
||||
ошибок нет.
|
||||
- Гейт деплоя/приёмки управляется вердиктом **тестировщика** в
|
||||
`13-test-report.md` (BLOCKED по AC-19), а не этим code-review. Оси
|
||||
code-review (ТЗ/ADR/код/тесты) удовлетворены полностью.
|
||||
## 6. Findings
|
||||
|
||||
→ Для оркестратора: APPROVED здесь означает «код корректен и готов»,
|
||||
но фактическая поставка ценности ET-013 заблокирована до догенерации
|
||||
hillshade z9 в рамках PH-6. Вердикт о готовности к закрытию задачи —
|
||||
за тестировщиком/CI.
|
||||
Нет findings уровня P0/P1/P2/P3.
|
||||
|
||||
## 6. Качество тестов
|
||||
## 7. Примечание по статусу тестирования
|
||||
|
||||
- `test_terrain_paint.py` — 17 тестов, парсят реальные блоки `app.js`
|
||||
(балансировка фигурных скобок, извлечение zoom-stops), проверяют:
|
||||
тип `interpolate`/`linear`/`zoom`, ключевые значения и **монотонность**
|
||||
opacity/contrast, `nearest`, регрессию z8=0.70 и пик z9-z11≥0.80,
|
||||
обратную совместимость сигнатуры, порог `zoom < 9` (и отсутствие
|
||||
`zoom < 10`), текст hint, число вызовов. Прогон локально: **17/17 PASS**.
|
||||
- `test_terrain_z9_tiles.py` — параметризован по `layer∈{hillshade,tri}`
|
||||
и `zoom∈{9,10,11}`, корректный `skipif` при отсутствии данных,
|
||||
whitelist-регрессия (`Unknown layer` vs `Tile not found`),
|
||||
Cache-Control immutable. Превышает минимум TRZ (учтены прошлые
|
||||
review-findings F-1/F-2).
|
||||
|
||||
Замечание (информационное, не влияет на вердикт): локальный прогон
|
||||
integration-тестов в моём окружении упал на `ModuleNotFoundError: shapely`
|
||||
при импорте `src.api.main` — это нехватка зависимости в sandbox, не дефект
|
||||
ET-013. В CI/venv (см. `13-test-report.md`: «254 passed, 7 skipped»)
|
||||
зависимость присутствует и сюита зелёная.
|
||||
|
||||
## 7. Findings
|
||||
|
||||
| ID | Severity | Описание | Статус |
|
||||
|----|----------|----------|--------|
|
||||
| — | P0 | нет | — |
|
||||
| — | P1 | нет | — |
|
||||
| — | P2 | нет | — |
|
||||
| N-1 | note | AC-19: hillshade z9-тайлы (404) — блокер данных PH-6, ведётся тестировщиком (BLOCKED), не code-finding | open (вне scope code-review) |
|
||||
`13-test-report.md` имеет `verdict: BLOCKED`. Блокировка относится к
|
||||
**деплой-стадии**, а не к коду: pre-deploy наличие тайлов z9-z11 на
|
||||
test-среде (AC-19) и визуальная приёмка AC-07..AC-12 / AC-21 требуют
|
||||
живого окружения, недоступного тестеру из песочницы. Это вне четырёх
|
||||
осей code-review. По коду, ТЗ, ADR и автотестам блокеров нет — деплой и
|
||||
ручная валидация остаются ответственностью стадии Deploy.
|
||||
|
||||
## 8. Вердикт
|
||||
|
||||
**APPROVED** — по осям code-review (соответствие ТЗ, соответствие ADR-017,
|
||||
качество кода, качество тестов) дефектов P0/P1/P2 нет. Реализация
|
||||
построчно соответствует REQ-F-01..F-21 и ADR-017.
|
||||
Реализация полностью соответствует ТЗ и ADR-017, код и тесты
|
||||
качественны, автотесты зелёные. P0/P1 отсутствуют.
|
||||
|
||||
Готовность к закрытию задачи **отдельно ограничена** блокером данных
|
||||
AC-19 (`13-test-report.md` → BLOCKED): это гейт тестирования/деплоя
|
||||
(PH-6 follow-up по нарезке hillshade z9), а не дефект кода. Code-review
|
||||
не разблокирует AC-19 и не подменяет вердикт тестировщика.
|
||||
**APPROVED.**
|
||||
|
||||
Reference in New Issue
Block a user