Files
enduro-trails/docs/work-items/ET-013/12-review.md
claude-bot 7866174d8a
All checks were successful
CI / lint (push) Successful in 4s
CI / test (push) Successful in 10s
CI / build (push) Successful in 2s
reviewer(ET): auto-commit from reviewer run_id=224
2026-06-06 22:43:36 +00:00

14 KiB
Raw Blame History

type, work_item_id, verdict, version, created_at, updated_at, authors, related
type work_item_id verdict version created_at updated_at authors related
review ET-013 APPROVED 9 2026-06-04 2026-06-06
agent:reviewer
ET-013:trz
ET-013:adr-017
ET-013:test-report

Review ET-013 — Перепады высот на z9-z11 (re-run #9)

Re-run #9 (independent). Перечитаны TRZ, AC, ADR-017, CLAUDE.md. Заново снят feature-diff git diff 8da09e6..HEAD (merge-base с main теперь 316bb0d, т.к. в main влит ET-015). src/api/main.py, src/web/app.js, src/web/index.html, оба тест-файла прочитаны построчно. Unit-тесты перезапущены локально — 17/17 PASS (0.03 s). Integration (test_terrain_z9_tiles.py) структурно корректен, но в этой среде не исполняется из-за отсутствия пакета shapely (env-проблема импорта src/api/main.py, не дефект PR; в CI с зависимостями — отработает со skipif). grep applyTerrainLayer src/web/app.js → 1 декларация + 2 call-site. С момента v8 ни src/, ни tests/ не менялись — содержательные коммиты прежние (5be81f9 feat, 099669d fix-whitelist). Вердикт кода подтверждён повторно: APPROVED.

TL;DR

  • Branch: feature/ET-013-z9-z11-z8
  • HEAD: fb90ad9 tester(ET): auto-commit from tester run_id=223
  • Scope: zoom-aware paint (hillshade/TRI) на z9-z11, понижение UI-минзума hillshade z10→z9, обратно-совместимое расширение applyTerrainLayer, расширение backend-whitelist на tri (фикс F-1 из v1).
  • Что изменилось со времени review v7: в коде и тестах — ничего. Содержательные коммиты фичи — 5be81f9 (feat) и 099669d (fix whitelist); всё после них — docs-коммиты ревью/тестера. src/ и tests/ бит-в-бит совпадают с одобренным в v2..v7.
  • Перепроверено в re-run #8 независимо: перечитаны TRZ (REQ-F-01..F-21), AC (AC-01..AC-22), ADR-017, feature-diff по src/web/app.js, src/web/index.html, src/api/main.py и обоим тест-файлам. Unit-тесты перезапущены локально — 17/17 PASS (0.03 s). grep applyTerrainLayer → 1 декларация + ровно 2 call-site (onTerrainCheckbox).
  • Verdict кода: APPROVED. P0/P1 в коде/ADR/тестах не найдено. Код соответствует TRZ REQ-F-01..F-19, F-21 и всем семи решениям ADR-017.
  • ⚠️ Деплой не разблокирован ревью. Тестер (run_id=221, test-report v5) держит verdict: BLOCKED из-за AC-19 (P1): на test-среде /terrain/hillshade/9/{x}/{y}.png → 404 (тайлы z9 не нарезаны). Это предусловие данных PH-6, не дефект PR (см. §AC-19). Шиппинг держит тест-гейт Тестера, а не этот code-review.

Что прочитано

  • 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 (v7), 13-test-report.md (v5, BLOCKED)
  • CLAUDE.md
  • git diff 8da09e6..HEAD (полный diff фичи), --stat по src/+tests/
  • src/web/app.js (2725-2826, 3356-3434)
  • src/web/index.html:60
  • src/api/main.py:1239-1256 (terrain_tile)
  • tests/unit/test_terrain_paint.py, tests/integration/test_terrain_z9_tiles.py
  • grep applyTerrainLayer src/web/app.js → 1 декларация + ровно 2 вызова

Соответствие ТЗ

Требование Реализация Файл OK
REQ-F-01 / F-11 — порог zoom < 9 if (zoom < 9) + комментарий; zoom < 10 удалён 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 — back-compat opacityOrPaint typeof === 'number' ? legacy : obj, paint: paint app.js:3371-3408
REQ-F-05/06/07 — HILLSHADE_PAINT opacity 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 z8=0.70, пик z10/z11=0.85, спад до 0.70; nearest 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:2808-2826
REQ-F-13 — unit-тесты paint (Вариант B) 17 тестов, 17/17 PASS (перезапущено) test_terrain_paint.py
REQ-F-14 — регрессии (порог 9, hint, callers) покрыты там же
REQ-F-15 — integration smoke z9-z11 + 404 + cache параметризация [hillshade,tri]×[9,10,11], whitelist-регрессия, 404, immutable test_terrain_z9_tiles.py
REQ-F-16 — Playwright UI этап Тестирования n/a (review)
REQ-F-17 — localStorage без миграции не тронуто
REQ-F-18 — API-контракт без изменений сигнатура/коды/Cache-Control те же; whitelist расширен на tri (см. §ADR) main.py:1239-1256
REQ-F-19 — конфиги/стили не тронуты style.json, style-dark.json, app.css, config/* — нет правок git diff --stat
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/16/17/22 — закрыты зелёными авто-тестами. Поведенческие AC-03/06..13 — за этапом Тестирования. AC-19 — FAIL (тайлы z9 на сервере 404) и, как следствие, AC-03/07/13 — недостижимы в рантайме; это предусловие данных, не дефект PR.

Соответствие ADR

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). Подтверждено независимым чтением исходников; расхождений stops со spec нет.

Замечание по фразе ADR «Не меняется: src/api/main.py». Фактически main.py:1252 изменён (whitelist tri + docstring). Это не нарушение ни одного из решений 17: фикс не добавляет endpoint/source/query/header, не меняет коды ответа и Cache-Control, а лишь делает уже существующий слой tri маршрутизируемым в dev-режиме (FastAPI без nginx). Без него make dev отдаёт 404 на /terrain/tri/..., который фронтенд запрашивает с PH-6. ADR в «Контексте» сам исходил из того, что endpoint отдаёт /terrain/{layer}/… в т.ч. для TRI. Объём = 1 строка + docstring со ссылкой на review F-1, риск низкий, path-traversal не появляется (layer сверяется с фиксированным кортежем). Это расхождение документации, не кода — на P0/P1 не тянет (P3).

AC-19 / деплой-блокер (анализ от ревью)

Тестер (run_id=221, v5) воспроизвёл: на выкаченной test-среде GET …/terrain/hillshade/9/{x}/{y}.png404 (grid 5×5 вокруг (309,348) + 6 точек ЦФО → 0/25, 0/6 → 200; hillshade-стек начинается с z10). Тайлы hillshade/z10, z11, tri/z5..z12 присутствуют. Следствие: AC-03/07/13 не достижимы → тест-вердикт BLOCKED.

Почему это не code-review P0/P1.

  • Ни одна строка PR не управляет наличием PNG на сервере.
  • TRZ это предусмотрел — REQ-F-20 §1: «Если 404 — задача останавливается, тайлы z9 нужно догенерировать в рамках PH-6 follow-up».
  • REQUEST_CHANGES на разработчика бессмысленно: правкой кода тайлы не появятся.

Гейт. Это инфра/данные PH-6. Шиппинг держит тест-гейт Тестера (BLOCKED), а не code-review. Рекомендация: открыть/довести до конца PH-6 follow-up «нарезать hillshade z9-z11 для региона test-среды», повторить AC-19 и поведенческие AC-03/07/13. До этого к деплою не выпускать — по линии Тестера.

Качество кода

  • Стиль совпадает с app.js (vanilla JS, JSDoc, маркеры // ET-013:).
  • applyTerrainLayer расширён минимально (нормализация paint + переменная paint); ровно 2 call-site (grep подтверждает), back-compat сохранён.
  • HILLSHADE_PAINT / TRI_PAINT — UPPER_SNAKE_CASE, рядом с TERRAIN_BASE_URL, комментарии ссылаются на ADR-017.
  • Нет дублирования, dead code, console.log, закомментированного кода.
  • Backend-фикс: whitelist — фиксированный кортеж ("hypso","hillshade","tri"), z/x/y типизированы как int, path-traversal-рисков нет.
  • Тесты: парсер с балансировкой скобок (_extract_block), проверка монотонности stops, back-compat и whitelist-регрессии. Integration — корректный skipif при отсутствии PNG-данных (AC-16).

Изменения после review v7

Только документация (по src//tests/ пусто): 12-review.md (этот файл, v8), 13-test-report.md (v5 Тестера, run 221). Код и тесты с момента APPROVED-вердикта v2..v7 не менялись (содержательные коммиты — 5be81f9 feat, 099669d fix-whitelist). Findings v1 (F-1 P1 backend whitelist, F-2 P2 integration coverage) — закрыты.

Findings

F-3 — P3 (косметика) — комментарий HILLSHADE_PAINT не упоминает clamping ниже z9

src/web/app.js. 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. Не вредит. Опционально.

Вердикт

APPROVED (ось code-review).

  • P0/P1 в коде/ADR/тестах не найдено.
  • Код соответствует TRZ REQ-F-01..F-19, F-21 и всем решениям ADR-017.
  • Unit-тесты зелёные (17/17, перезапущено независимо), 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 app.js HILLSHADE_PAINT комментарий не учитывает clamping ниже z9 опц. добавить stop 8, 0.00 при будущем понижении порога
F-5 P3 test_terrain_paint.py неиспользуемый from __future__ import annotations косметика
(вне кода) test-среда AC-19: hillshade z9 = 404, тайлы не нарезаны PH-6 follow-up; гейт держит Тестер (BLOCKED), не ревью

P0/P1 по коду отсутствуют → APPROVED. Деплой-готовность — за тест-гейтом.