From c9da18fdd035e77ed803d1adb5790aef68cfabd9 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Sat, 6 Jun 2026 22:32:45 +0000 Subject: [PATCH] reviewer(ET): auto-commit from reviewer run_id=220 --- docs/work-items/ET-013/12-review.md | 93 +++++++++++++++-------------- 1 file changed, 49 insertions(+), 44 deletions(-) diff --git a/docs/work-items/ET-013/12-review.md b/docs/work-items/ET-013/12-review.md index 0ad4f91..25991be 100644 --- a/docs/work-items/ET-013/12-review.md +++ b/docs/work-items/ET-013/12-review.md @@ -2,7 +2,7 @@ type: review work_item_id: ET-013 verdict: APPROVED -version: 6 +version: 7 created_at: 2026-06-04 updated_at: 2026-06-06 authors: @@ -13,26 +13,27 @@ related: - "ET-013:test-report" --- -# Review ET-013 — Перепады высот на z9-z11 (re-run #6) +# Review ET-013 — Перепады высот на z9-z11 (re-run #7) ## TL;DR - **Branch:** `feature/ET-013-z9-z11-z8` -- **HEAD:** `fd0c7a4 tester(ET): auto-commit from tester run_id=215` +- **HEAD:** `23cd31d tester(ET): auto-commit from tester run_id=217` - **Scope:** zoom-aware paint (hillshade/TRI) на z9-z11, понижение UI-минзума hillshade z10→z9, обратно-совместимое расширение `applyTerrainLayer`, расширение backend-whitelist на `tri` (фикс F-1 из v1). -- **Что изменилось со времени review v5:** **в коде и тестах — ничего.** - `git diff 8da09e6..HEAD --name-only` по `src/`+`tests/` даёт ровно - 5 файлов, идентичных одобренным в v2..v5; новые коммиты (run 214/215) — - только docs (`12-review.md`, `13-test-report.md`). -- **Перепроверено в re-run #6 независимо:** перечитаны TRZ, AC, ADR-017, - полный diff фичи (`git diff 8da09e6...HEAD`), исходники и оба тест-файла. - Unit-тесты перезапущены локально — **17/17 PASS** (0.03 s). +- **Что изменилось со времени review v6:** **в коде и тестах — ничего.** + `git diff 316bb0d..HEAD --stat` даёт ровно 2 файла — оба docs + (`12-review.md`, `13-test-report.md`). Feature-diff (`8da09e6..HEAD` + по `src/`+`tests/`) идентичен одобренному в v2..v6: 5 файлов. +- **Перепроверено в re-run #7 независимо:** перечитаны TRZ, AC, ADR-017, + полный diff фичи (`git diff 8da09e6..HEAD`), исходники и оба тест-файла. + Unit-тесты перезапущены локально — **17/17 PASS** (0.03 s). `grep + applyTerrainLayer` → 1 декларация + ровно 2 call-site. - **Verdict кода: APPROVED.** P0/P1 в коде/ADR/тестах не найдено. Код соответствует TRZ REQ-F-01..F-19, F-21 и всем семи решениям ADR-017. -- **⚠️ Деплой не разблокирован ревью.** Тестер (run_id=215) держит - `verdict: BLOCKED` из-за **AC-19 (P1): на test-среде +- **⚠️ Деплой не разблокирован ревью.** Тестер (run_id=217, test-report v4) + держит `verdict: BLOCKED` из-за **AC-19 (P1): на test-среде `/terrain/hillshade/9/{x}/{y}.png` → 404** (тайлы z9 не нарезаны). Это предусловие данных PH-6, **не дефект PR** (см. §AC-19). Шиппинг держит тест-гейт Тестера, а не этот code-review. @@ -42,12 +43,12 @@ 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/12-review.md` (v5), `13-test-report.md` (v3, BLOCKED) +- `docs/work-items/ET-013/12-review.md` (v6), `13-test-report.md` (v4, BLOCKED) - `CLAUDE.md` -- `git diff 8da09e6...HEAD` (полный diff фичи), `--stat` по `src/`+`tests/` -- `src/web/app.js` (2722-2827, 3356-3434) +- `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:1240-1264` (`terrain_tile`) +- `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 вызова @@ -59,16 +60,16 @@ related: | 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:2734-2752` | ✅ | -| REQ-F-08/09 — TRI_PAINT | z8=0.70, пик z10/z11=0.85, спад до 0.70; `nearest` | `app.js:2755-2768` | ✅ | +| 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+» | `Зум 9+` | `index.html:60` | ✅ | -| REQ-F-12 — контракт `onTerrainCheckbox` | localStorage + `.active` без изменений | `app.js:2808-2827` | ✅ | +| 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:1240-1264` | ✅ | +| 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` присутствуют | — | ✅ | @@ -84,24 +85,27 @@ ADR-017 реализован по всем семи решениям: P-A (front 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 нет. +независимым чтением исходников; расхождений stops со spec нет. **Замечание по фразе ADR «Не меняется: `src/api/main.py`».** Фактически `main.py:1252` изменён (whitelist `tri` + docstring). Это **не нарушение** ни одного из решений 1–7: фикс не добавляет endpoint/source/query/header, не меняет коды ответа и `Cache-Control`, а лишь делает уже существующий -слой `tri` маршрутизируемым в dev-режиме (без nginx). ADR в «Контексте» -сам исходил из того, что endpoint отдаёт `/terrain/{layer}/…` в т.ч. для -TRI. Объём = 1 строка + docstring со ссылкой на review F-1, риск низкий. -Это расхождение **документации**, не кода — на P0/P1 не тянет (P3). +слой `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=215) объективно зафиксировал: на выкаченной test-среде +Тестер (run_id=217, v4) воспроизвёл: на выкаченной test-среде `GET …/terrain/hillshade/9/{x}/{y}.png` → **404** (grid 5×5 вокруг -`(309,348)` + 6 точек ЦФО → 0/25 → 200). Тайлы `hillshade/z10`, `z11`, -`tri/z5..z12` присутствуют. Следствие: AC-03/07/13 не достижимы → тест- -вердикт **BLOCKED**. +`(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 на сервере. @@ -111,10 +115,10 @@ TRI. Объём = 1 строка + docstring со ссылкой на review F-1 появятся. **Гейт.** Это инфра/данные PH-6. Шиппинг держит тест-гейт Тестера -(BLOCKED), а не code-review. Рекомендация: открыть PH-6 follow-up -«нарезать hillshade z9-z11 для региона test-среды», повторить AC-19 и -поведенческие AC-03/07/13. До этого к деплою не выпускать — по линии -Тестера. +(BLOCKED), а не code-review. Рекомендация: открыть/довести до конца +PH-6 follow-up «нарезать hillshade z9-z11 для региона test-среды», +повторить AC-19 и поведенческие AC-03/07/13. До этого к деплою не +выпускать — по линии Тестера. ## Качество кода @@ -130,25 +134,26 @@ TRI. Объём = 1 строка + docstring со ссылкой на review F-1 монотонности stops, back-compat и whitelist-регрессии. Integration — корректный `skipif` при отсутствии PNG-данных (AC-16). -## Изменения после review v5 +## Изменения после review v6 -Только документация (`git diff e948861..HEAD --name-only` по `src/`/`tests/` -— пусто): `12-review.md` (этот файл, v6), `13-test-report.md` (v3 Тестера). -Код и тесты с момента APPROVED-вердикта v2..v5 **не менялись**. Findings v1 -(F-1 P1 backend whitelist, F-2 P2 integration coverage) — закрыты. +Только документация (`git diff 316bb0d..HEAD --name-only` — два docs-файла, +по `src/`/`tests/` пусто): `12-review.md` (этот файл, v7), `13-test-report.md` +(v4 Тестера, run 217). Код и тесты с момента APPROVED-вердикта v2..v6 +**не менялись**. Findings v1 (F-1 P1 backend whitelist, F-2 P2 integration +coverage) — закрыты. ## Findings ### F-3 — P3 (косметика) — комментарий HILLSHADE_PAINT не упоминает clamping ниже z9 -`src/web/app.js:2734`. Stops opacity начинаются с `9, 0.65`; при z<9 +`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:15`. Не вредит. **Опционально.** +`tests/unit/test_terrain_paint.py`. Не вредит. **Опционально.** ## Вердикт @@ -156,8 +161,8 @@ MapLibre заклампит на нижнем стопе. В текущем scop - P0/P1 в коде/ADR/тестах не найдено. - Код соответствует TRZ REQ-F-01..F-19, F-21 и всем решениям ADR-017. -- Unit-тесты зелёные (17/17, перезапущено), integration структурно - корректен. +- Unit-тесты зелёные (17/17, перезапущено независимо), integration + структурно корректен. - Остаются два P3-косметических замечания — не блокеры. > ⚠️ **Это вердикт только по качеству/соответствию кода.** Готовность к @@ -170,8 +175,8 @@ MapLibre заклампит на нижнем стопе. В текущем scop | ID | Severity | Где | Кратко | Действие | |---|---|---|---|---| -| F-3 | P3 | `app.js:2734` | комментарий не учитывает clamping ниже z9 | опц. добавить stop `8, 0.00` при будущем понижении порога | -| F-5 | P3 | `test_terrain_paint.py:15` | неиспользуемый `from __future__ import annotations` | косметика | +| 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**. Деплой-готовность — за тест-гейтом.