Files
2026-09-28 07:35:39 +00:00

20 KiB
Raw Permalink Blame History

SPEC-REVIEW-685-r1

  • Issue: https://github.com/Matysh/houseplan-card/issues/685
  • Этап: S4-spec-review (ревью ТЗ, PROCESS.md §2.4)
  • Трек: полный. Аналитик сам назвал критерий §5, который задача не проходит: «нарушены критерии small: решение влияет на производительность и touch» (обновление viewBox на каждом промежуточном кадре рискует вернуть регрессии #531/#579). Разбор по существу, полный трек выбран корректно.
  • Материал: тело issue #685, раздел ## ТЗ + оба комментария владельца: (1) «S2 — аналитика» с оценкой, диагностикой на dev и вопросами Q1/Q2 с предложенными default, blocked поверх S3-spec; (2) «Уточнение после ответа владельца» — Q1/Q2 закрыты, blocked снят, ТЗ дописано в тело issue.
  • Заход: r1 · блокирующих циклов израсходовано 0 из 4
  • Роль: ревьюер ТЗ (не автор)

Скоуп ревью

Плоская проекция houseplan-card (View + три редактора на общем архитектурном SVG): статическая резкость осевших стен, диагональной штриховки и проёмов (дверь/окно/ворота/проход) на дробных конечных масштабах зума — колесо, кнопки, pinch. Дефект — не потеря резкости во время самой анимации (владелец явно объявил это не-проблемой), а масштабозависимая мягкость уже неподвижной геометрии. Не-скоуп: 2.5D, статичная houseplan-space-card, браузерный zoom страницы, изменение геометрии/толщины/плотности штриховки, длительности анимации.

SCOPE-проверка (docs/SCOPE.md): задача закрывает J1 («at a glance» — архитектура должна оставаться читаемой на увеличенном плане) и косвенно J6 (план остаётся «true», в том числе визуально, пока меняется масштаб). Ничего из «никогда не строить» не задевается: геометрия, толщина и штриховка прямо объявлены неизменными (К4), 2.5D и статичная карточка явно исключены. Формулировка аналитика «J1/J6» подтверждаю без замечаний.

Как проверялось

  1. Прочитаны docs/SCOPE.md, AGENTS.md, docs/process/REVIEWER.md целиком; по ссылкам конспекта открыт PROCESS.md §2.4, §2.5, §4, §7.1, §7.2 (§2.10 не применялся — это r1).
  2. Прочитано тело issue #685 целиком (gh issue view 685 --json body) и оба комментария (gh issue view 685 --json comments) — история вопрос/ответ владельца воспроизведена в «Материал» выше.
  3. Сверены обязательные разделы §7.1: Сценарий, Что человек увидит до/после, Проблема, Скоуп/Не-скоуп, Контракт поведения К1–К5, UX, Модель данных и миграция, i18n, AC1–AC6 с указанным способом доказательства, План автотестов, Затронутые файлы и модули, Производительность и touch, Риски, Откат, Release-артефакты, «Принято предположительно» — все на месте, в правильном порядке; первые два раздела продуктовые и без терминов реализации, как требует §7.1.
  4. Оба вопроса владельцу (Q1/Q2 из первого комментария) сверены с итоговым текстом: ответ «да, убрать анимацию дискретного зума» из Q1 не попал в контракт как обязательный — вместо этого второй комментарий переопределяет рамку («размытие только во время анимации допустимо… анимацию кнопок/колеса и живой pinch сохраняем») и К5 прямо требует сохранить «короткую анимацию колеса/кнопки и живой pinch». Это не расхождение с ответом владельца, а его уточнение тем же владельцем во втором комментарии — второй комментарий специально помечен «Предыдущие Q1/Q2 закрыты новым наблюдением» и заменяет принятые default на итоговое решение после дополнительной диагностики (побайтовое совпадение кадра 12/12). Текст ТЗ (Проблема, К1, К5, Не-скоуп п.«изменение длительности или характера зум-анимации») последовательно отражает именно вторую, финальную версию. Открытых продуктовых вопросов не осталось — подтверждаю.
  5. Технические утверждения ТЗ сверены с реальным кодом, а не приняты на слово: src/live-viewport.ts:127-149 подтверждает описанный механизм (style.removeProperty('transform'), willChange = 'transform' во время жеста, снятие после терминального кадра) — диагностика «transform: none, will-change: auto» в осевшем кадре не является придуманным фактом. Существование файлов/тестов, на которые ссылаются AC и план автотестов, проверено ls/grep: src/live-viewport.ts, src/wall-thickness.ts, src/render/opening-symbol.ts, src/styles/plan.styles.ts, test/live-viewport.test.mjs, demo/smoke_wall_hatch_density.mjs, wallHatchStepUnits (src/wall-thickness.ts, test/wall-thickness.test.mjs), demo/golden/matrix.mjs, demo/golden/harness.mjs, scripts/mutation-gate.mjs, бюджет large-house-interaction-v1 (demo/performance/budgets-large-house-interaction.json) — все существуют. Ссылки на #230/#531/#579 сверены с legacy/reviews/** — все три issue действительно решали названные контракты (плотность штриховки, дорогая перерисовка viewBox при панорамировании, белые/прозрачные кадры pinch).
  6. Каноническая документация подсистемы сверена на противоречия: docs/WALL-THICKNESS.md:284-295 («Hatch density is physical (#230)… The step is NOT compensated for zoom») совпадает с К4 (геометрическая неизменность) без зазора. docs/CANVAS.md не содержит упоминаний will-change/transform/DPR/rasterisation — контракт терминального кадра действительно новый для канона, и ТЗ корректно называет его release- артефактом («зафиксировать контракт конечного кадра»), а не выдаёт за уже существующий. docs/TESTING.md:512-528 (Environments matrix) подтверждает, что Firefox — пункт предрелизного ручного прогона, а не гейта ревью; риск «Возврат лагов Firefox» в разделе «Риски» ТЗ корректно назван риском, а не пропущенным AC. demo/golden/run.mjs:718,1120-1141 подтверждает, что DPR 1/2 — уже действующая в проекте пара для golden-проверок, а не решение, придуманное для этой задачи в одиночку.
  7. git branch -a / git log --all --oneline | grep 685 — ветки issue/685-* и коммитов с трейлером Issue: #685 не существует; git diff origin/dev...HEAD пуст. Продуктового кода для #685 нет — стадия spec, гейты (tsc, test, build, смоки, golden, инварианты) неприменимы, штатное состояние, а не находка.

Находки

Не найдено High и Medium.

Один Low, снимаю без возврата автору (в том же порядке, что допускает §7.1 для деталей, не влияющих на наблюдаемое поведение):

Low-1 — AC2 не называет конкретное числовое значение «заведомо проблемного дробного масштаба». Раздел «Проблема» перечисляет диагностированные масштабы (100/115/132/140/196/250/384 %), но AC2 говорит только «на заведомо проблемном дробном масштабе», не фиксируя, какой из них войдёт в golden-сцену. Практического расхождения это не создаёт: приёмка AC2 в любом случае идёт через человеческую визуальную проверку («принимаются только после визуальной проверки… линии и штриховка одинаково чёткие»), а не через числовой порог, и выбор конкретной тестовой сцены — деталь реализации, а не наблюдаемое пользователем поведение (§7.1: «всё, чего пользователь не наблюдает, агенты решают сами»). Материала для диагностики уже достаточно (семь конкретных масштабов названы), так что реализатору не придётся исследовать вопрос с нуля. Не возвращаю на цикл; стоит зафиксировать выбранный масштаб в коде/PR при реализации, чтобы код-ревью не пришлось гадать, какая сцена что доказывает.

Что проверено и корректно

  • Все обязательные разделы §7.1 присутствуют, в правильном порядке; раздел «Сценарий» называет персону (администратор/член семьи), поверхность (плоский план, live-жесты) и момент (после завершения зума); «Что человек увидит» — одной парой фраз до/после, без терминов реализации.
  • Скоуп/Не-скоуп разделены явно (шесть пунктов скоупа, восемь пунктов не-скоупа) и совпадают с диагностикой и ответами владельца — расхождений между Q1/Q2, предложенными default и итоговым текстом не найдено (см. «Как проверялось» п.4).
  • Контракт К1–К5 внутренне согласован: К1 (терминальный кадр) → AC1; К2 (статическая резкость) → AC2; К3 (проёмы) → AC1/AC2/AC3; К4 (геометрическая неизменность) → AC3; К5 (живой жест, контракты #531/#579) → AC4/AC5. Каждый пункт контракта имеет хотя бы один AC, который его проверяет — пробелов «контракт есть, AC нет» не найдено (в отличие от прецедента #663, где §6.2 и §8 противоречили друг другу — здесь такого противоречия нет).
  • AC1–AC6 однозначны и указывают способ доказательства (smoke/golden/ smoke+существующий unit/performance/golden+отрицательная проба), что удовлетворяет DoR §2.5. AC1 задаёт точные машинно проверяемые условия (viewBox, transform: none, will-change: auto, побайтовая идемпотентность повтора масштаба). AC6 заранее требует зарегистрированный «чем краснеет» свидетель для дорогого golden-защитного AC — это ровно то, что потребует §2.7 на код-ревью, и ТЗ не оставляет это открытием для последующего раунда.
  • AC5 ссылается на существующий, а не гипотетический бюджет (large-house-interaction-v1), причём этот перф-смок и так гоняется Validate'ом автоматически при правке src/live-* (PROCESS.md §8, строки 787-791) — AC не изобретает новый гейт, а фиксирует уже действующий порог как критерий приёмки. В отличие от #663 Low-2 («AC13 не называет конкретный бюджет»), здесь бюджет назван по имени файла.
  • Модель данных и миграция, i18n: явные «нет» с обоснованием (нет новых настроек/строк/сохранённых полей) — соответствует docs/CONFIG- COMPATIBILITY.md (нет полей — нет вопроса совместимости).
  • Производительность и touch: риск назван явно («риск высокий»), touch View назван блокирующей поверхностью (docs/TOUCH-SUPPORT.md), pinch — с отдельным AC4.
  • Откат (соответствует правилу docs/SCOPE.md «никогда не удалять файл пользователя на инференс» — здесь данных нет, откат чисто ревертом продуктового коммита) и Release-артефакты (оба changelog, docs/CANVAS.md/ docs/WALL-THICKNESS.md, golden с Baseline-Reviewed, docs screenshots через check-docs) — полный список, ничего не забыто относительно затронутых канонических документов.
  • Раздел «Принято предположительно» корректно ограничивает свободу автора: выбор конкретного механизма стабилизации — техническое решение, а не скрытый продуктовый вопрос; явно запрещено решать задачу изменением сохранённых координат/округлением модели — усиливает К4, а не противоречит ему.
  • Продуктовые вопросы Q1/Q2 заданы владельцу одним комментарием, пачкой, каждый с предложенным default (форма §7.1); ответ владельца привёл к дополнительной диагностике и уточнению рамки, итог внесён в тело issue, blocked снят — открытых продуктовых вопросов не осталось.

Чего не проверял

  • Гейты (tsc --noEmit, npm test, npm run build, check-docs.mjs, смоки, golden:verify, инварианты) — не прогонял: этап spec, ветки/ коммитов для #685 нет, git diff origin/dev...HEAD пуст. Предмет код-ревью после реализации.
  • Не проверял, реализуемо ли требование К2 («не должно быть заметного скачка между соседними масштабами») без изменения геометрии технически дешёво — раздел «Риски» ТЗ сам называет это неопределённостью («golden может закрепить плохой результат»); это риск, явно принятый автором и подлежащий человеческой визуальной приёмке на код-ревью, а не пробел ТЗ.
  • Не пересчитывал вручную побайтовое совпадение кадра 12/12, заявленное автором в диагностике, — это протокол ручного эксперимента до кода, не проверяемый на этапе spec без ветки; приму как предпосылку, а не как проверяемый в этом ревью факт.
  • Не оспаривал сами решения владельца по Q1/Q2 — только сверял, что итоговый контракт им (в уточнённой версии) соответствует.

Вердикт

Обязательные разделы ТЗ полны и в правильном порядке, оба продуктовых вопроса закрыты владельцем без остатка, все шесть AC однозначны, имеют названный способ доказательства и полностью покрывают контракт К1–К5 без внутренних противоречий. Технические утверждения (диагностика, ссылки на файлы/тесты/issue) проверены против реального кода и документации и не являются догадками, выданными за факт. Единственная находка — Low, редакционная, снята без возврата.

Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 · Документ: docs/reviews/SPEC-REVIEW-685-r1.md


Материал раунда

  • Ветка: issue/685-static-zoom-sharpness, коммит 03a3dd991627 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 5e70a0cf6f4ce4d022390f0abb2d28618644cd01
    git log --all --format='%H %T' | grep 5e70a0cf6f4c
    
  • Тело issue: 0107c8af6612487d0d3e54d481d818e69ea33110507e7b572d37038613acdd60
  • Вердикт конвейера: green · High 0