mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 21:01:21 +00:00
@@ -1,10 +1,11 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1106, issue: 396. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1107, issue: 396. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
| #676 | [SPEC-REVIEW-676-r1.md](SPEC-REVIEW-676-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 2 | AC8 не называет проверку двух из трёх ветвей подавления подсказки; риск «рамка перехватывает события под другим инструментом» не привязан ни к одному AC; не задан tie-break для равного протяжки по обеим осям (снимаю сам) | `src/stairs-editor-model.ts` `stairs-view.ts` `demo/smoke_stairs.mjs` `src/furniture.ts` |
|
||||
| #676 | [SPEC-REVIEW-676-r2.md](SPEC-REVIEW-676-r2.md) | spec · r2 | 🟡 жёлтый | 0 | 1 | AC8/К8 перечисляют четыре ветви stairTargetState, а не пять; состояние self выпадает из… | `src/stairs-editor-model.ts` `docs/STAIRS.md` `src/stairs.ts` `stairs-view.ts` |
|
||||
| #675 | [CODE-REVIEW-675-r1.md](CODE-REVIEW-675-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #673 | [CODE-REVIEW-673-r1.md](CODE-REVIEW-673-r1.md) | code · r1 | 🟡 жёлтый | 0 | 0 | docs/images/screenshots.json: закоммиченный sourceFingerprint не совпадает с тем, что р…; Комментарий автора в issue #673 называет SHA реализации 557d4e38e26b647437f2c931454737f… | `docs/images/screenshots.json` `demo/golden/matrix.mjs` `scripts/source-fingerprint.mjs` `accept.mjs` `policy.mjs` `AGENTS.md` `baselines-index.json` |
|
||||
| #673 | [CODE-REVIEW-673-r2.md](CODE-REVIEW-673-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
|
||||
@@ -0,0 +1,211 @@
|
||||
# SPEC-REVIEW-676-r2
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/676
|
||||
- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
|
||||
- **Трек:** полный (метки `bug`, `P1`, `S4-spec-review`; лёгкий трек — нет,
|
||||
подтверждено аналитиком в r1: несколько поверхностей, новый UX-контракт
|
||||
размещения протяжкой и подсказки, сложность > 3).
|
||||
- **Материал:** тело issue #676, раздел `## ТЗ` (редакция 2, 27.09) + 6
|
||||
комментариев (без изменений добавился только комментарий «редакция 2» и
|
||||
вердикт r1).
|
||||
- **Заход:** r2 · блокирующих циклов израсходовано 1 из 4 (эта находка сделает
|
||||
2 из 4)
|
||||
- **Роль:** ревьюер ТЗ (не автор)
|
||||
|
||||
## Скоуп ревью (по дельте, PROCESS.md §2.10)
|
||||
|
||||
Предмет r2 — ровно три точечных правки редакции 2, объявленные автором в
|
||||
комментарии `IC_kwDOTOcLQM8AAAABXQNDXw` и в шапке ТЗ («по ревью r1: AC8
|
||||
покрывает все три ветви подавления, AC12 — инертность рамки, tie-break
|
||||
диагонали в К1»):
|
||||
|
||||
1. AC8 расширен до перечисления всех ветвей подавления подсказки Просмотра
|
||||
(К8).
|
||||
2. Новый AC12 — инертность рамки узлов лестницы под инструментами, отличными
|
||||
от «Выбор»/«Лестница».
|
||||
3. К1 получил детерминированный tie-break для протяжки ровно по диагонали
|
||||
45°.
|
||||
|
||||
Код задачи не существует (ветка `issue/676-*` не создана; см. «Как
|
||||
проверялось», п. 1) — ТЗ по-прежнему живёт только в теле issue, `docs/specs/`
|
||||
не создаётся (issue открыт после 2026-09-10). Остальные разделы ТЗ (Сценарий,
|
||||
Контракт К2–К7, i18n, модель данных, риски, откат, release-артефакты)
|
||||
дельтой r1→r2 не задеты — они наследуются из r1 без повторной проверки (см.
|
||||
раздел «Унаследовано из r1» ниже), кроме одного пересечения: расширенная
|
||||
верификация К8/AC8 в этом раунде вскрыла состояние, не покрытое ни в r1, ни
|
||||
в правке r2 (см. «Находки»).
|
||||
|
||||
**SCOPE-проверка (`docs/SCOPE.md`):** не меняется относительно r1 — сценарий
|
||||
закрывает J6 (редактирование объектов плана без регресса) и J1/J2 через
|
||||
корректную навигацию/подсказку лестницы в Просмотре.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Подтверждено, что кода задачи всё ещё нет: `git branch -a | grep -i 676`
|
||||
не находит рабочую ветку (только `d03a68b8 feat: добавить лестницы между
|
||||
этажами (#663)` — это #663, не #676); `git diff f1a87fba..HEAD --stat`
|
||||
между материалом r1 и текущим `HEAD` (`9fe7f88c`) показывает только два
|
||||
файла — `docs/reviews/INDEX.md` и публикацию `SPEC-REVIEW-676-r1.md`.
|
||||
Дифф кода для этой задачи не существует; гейты `typecheck`/`npm test`/
|
||||
`npm run build`/`check-docs.mjs` неприменимы на этом этапе — см. «Чего не
|
||||
проверял».
|
||||
2. Прочитано тело issue #676 целиком (`gh issue view 676 --json body`,
|
||||
sha256 `0b58e92a…0bdd`) и все 6 комментариев (`gh issue view 676 --json
|
||||
comments`), включая новый (редакция 2) и вердикт r1.
|
||||
3. Сверены ровно три правки редакции 2 с текстом находок r1 построчно:
|
||||
- **Medium-1 (AC8, r1):** требовал отдельного смоук-шага на каждую из
|
||||
веток `missing`/`deleted`/`fixed`. Новый AC8 перечисляет **четыре**
|
||||
шага с отдельными флагами: `garden` (active) → тултип и снятие;
|
||||
`target_space_id: null` (missing) → `null`; `'no-such-space'`
|
||||
(deleted) → `null`; карточка с `floor: 'f1'` (fixed) → `null`. Текстуально
|
||||
закрывает ровно то, что просил r1.
|
||||
- **Medium-2 (инертность рамки, r1):** рекомендовал новый AC12 со
|
||||
смоук-шагом «под инструментом не-«Выбор»/не-«Лестница» клик по точке
|
||||
узла воздействует на инструмент, а не перехватывается рамкой».
|
||||
Новый AC12 в точности содержит эту проверку (`.hp-stair-frame`
|
||||
отсутствует в DOM, `_path.length` растёт под «Стенами»). К2 также
|
||||
получил явное «рамка не рендерится вовсе» — согласовано с AC12.
|
||||
- **Low (tie-break, r1):** К1 получил «при равных |dx| и |dy| доминирует
|
||||
x», с прямой ссылкой на прецедент `resizeFurnitureTransform` — снято.
|
||||
4. Проверено, что AC12 ссылается на реально существующий, а не
|
||||
выдуманный контракт: `src/houseplan-editor-runtime.ts:2213-2229`
|
||||
(`_markupClick`) подтверждает, что клик под инструментом `draw` («Стены»)
|
||||
продвигает `this.host._path` (`_path.length` растёт при добавлении точки,
|
||||
см. `_canAppendWallPoint`, `:2200-2210`) — заявление AC12 о «клик ставит
|
||||
точку стены» не расходится с существующим кодом инструмента.
|
||||
5. Так как AC8 — именно та зона, которую r1 уже признал недостаточно
|
||||
покрытой веткой, для r2 она перепроверена не только текстуально, а по
|
||||
исходной функции, которую К8/AC8 описывают:
|
||||
`stairTargetState()` (`src/stairs-editor-model.ts:97-104`) возвращает
|
||||
**пять** состояний — `active | missing | self | deleted | fixed`, — не
|
||||
четыре. `self` (`target_space_id === currentSpaceId`) — не выдуманное
|
||||
состояние: оно документировано в каноне
|
||||
`docs/STAIRS.md:21-22` («A missing, self or deleted target leaves the
|
||||
stair visible and editable but shows a repair warning...») в одной группе
|
||||
с `missing`/`deleted`, и данные, порождающие его, не отвергаются моделью
|
||||
(`src/stairs.ts:84` проверяет только тип строки `target_space_id`, не
|
||||
запрещает равенство текущему пространству). Диалог свойств лишь
|
||||
исключает текущее пространство из выпадающего списка на **вкладке, где
|
||||
лестница редактируется сейчас** (`src/stairs-editor.ts:505`,
|
||||
`.filter((item) => item.id !== this.owner._space)`) — это не мешает
|
||||
`self` возникнуть после переноса/слияния помещений или ручной правки
|
||||
конфигурации, поэтому уже существующий код `stairs-view.ts:64-73`
|
||||
отдельно проверяет ровно пять веток через ту же функцию и намеренно не
|
||||
считает `self` навигируемым (`active = interactive && targetState ===
|
||||
'active'`). К8/AC8 этой задачи вводят новый вызов той же функции для
|
||||
тултипа, но описывают только **четыре** из пяти её исходов; вывод — в
|
||||
«Находки».
|
||||
6. Разделы, не тронутые дельтой (К2–К7 кроме упомянутого в п. 3 уточнения,
|
||||
i18n, модель данных, откат, release-артефакты, план автотестов кроме
|
||||
строки AC12), не перечитывались заново построчно — они приняты из r1 без
|
||||
повторной проверки, см. «Унаследовано из r1».
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе) — AC8/К8 перечисляют четыре ветви `stairTargetState`, а не пять; состояние `self` выпадает из контракта тултипа
|
||||
|
||||
**Что не так.** К8: «Лестница без цели, с битой целью или в карточке с
|
||||
закреплённым этажом подсказки не показывает» — это `missing`, `deleted`,
|
||||
`fixed`. AC8 после правки r2 прямо называет себя «все четыре ветви» и
|
||||
перечисляет `active`/`missing`/`deleted`/`fixed`. Но функция, которую К8
|
||||
описывает, `stairTargetState()` (`src/stairs-editor-model.ts:97-104`),
|
||||
возвращает **пять** значений: `active | missing | self | deleted | fixed`.
|
||||
Ветвь `self` — лестница с валидным, непустым `target_space_id`, который
|
||||
совпадает с ID текущего пространства, — не упомянута ни в К8, ни в AC8, ни
|
||||
в плане смоук-шагов.
|
||||
|
||||
**Почему это находка, а не придирка.** `self` — не гипотетическое, а
|
||||
документированное состояние: `docs/STAIRS.md:21-22` группирует его вместе
|
||||
с `missing`/`deleted` («shows a repair warning... makes View activation a
|
||||
no-op»), и модель (`src/stairs.ts:84`) не отвергает данные, где
|
||||
`target_space_id` равен текущему пространству — такое может возникнуть при
|
||||
переносе/слиянии помещений (J6, не защищено на этом уровне валидацией) или
|
||||
ручной правке YAML. Существующий, не-скоуповый код Просмотра
|
||||
(`stairs-view.ts:64-73`) уже трактует `self` как ненавигируемое наравне с
|
||||
`missing`/`deleted`/`fixed` через ту же функцию. Реализация тултипа,
|
||||
написанная буквально по К8 («не показывать при missing, deleted или
|
||||
fixed»), — то есть перечислением трёх суппрессирующих веток, а не
|
||||
инверсией `targetState !== 'active'`, — пропустит `self` через «иначе» и
|
||||
покажет пользователю подсказку «Переход на этаж {название текущего же
|
||||
этажа}» на лестнице, которая никуда не ведёт (клик по ней не
|
||||
сработает, потому что навигация уже верно проверяет `=== 'active'`). Это
|
||||
ровно тот класс дефекта, который r1 уже указал для этой же строки ТЗ:
|
||||
неполная ветка гварда, которая пройдёт AC1–AC12 буквально и оставит
|
||||
конкретное поведение и непроверенным, и вероятно неверным. Формулировка
|
||||
«все четыре ветви» (в отличие от r1, где пропуск был очевиден по счёту)
|
||||
создаёт ложное чувство полноты — именно поэтому дельта r1→r2 не отловила
|
||||
её сама.
|
||||
|
||||
**Чем закрывается в скоупе задачи.** Один из двух путей, оба технические
|
||||
(не продуктовый вопрос владельцу): (а) переформулировать К8/AC8 как
|
||||
инверсию — «подсказка показывается только при `targetState === 'active'`,
|
||||
во всех остальных случаях, включая уже перечисленные и `self`, не
|
||||
показывается» и добавить пятый смоук-шаг (лестница с
|
||||
`target_space_id`, равным ID текущего пространства, — `_tip` остаётся
|
||||
`null`); либо (б) явно решить, что `self` в контексте тултипа — не
|
||||
предусмотренная владельцем ситуация и обосновать, почему её можно
|
||||
не тестировать (например, если правка К8 формулируется как «показывать
|
||||
только при active», для чего пятая ветка тривиально следует из первых
|
||||
четырёх и не требует отдельного теста, а лишь явного упоминания в тексте
|
||||
контракта). Любой вариант не меняет скоуп и не требует эскалации
|
||||
владельцу — решение техническое, не продуктовое.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Обе Medium-находки r1 закрыты именно так, как рекомендовал r1: AC8 —
|
||||
четырьмя отдельными флагами вместо одного; AC12 — новым смоук-шагом,
|
||||
подтверждённым существующим кодом инструмента «Стены»
|
||||
(`_markupClick`/`_canAppendWallPoint`).
|
||||
- Low-находка r1 (tie-break диагонали) закрыта явным правилом «доминирует
|
||||
x» со ссылкой на прецедент.
|
||||
- AC12 корректно вставлен в «План автотестов» (`demo/smoke_stairs.mjs`:
|
||||
«расширить … шагами … AC12»), а не оставлен только в таблице АС без
|
||||
соответствующей строки плана.
|
||||
- К2 согласован с AC12 текстуально: оба говорят «рамка не рендерится вовсе»
|
||||
под другими инструментами, а не «рендерится, но игнорирует события» —
|
||||
единственное число, один источник истины для этого поведения.
|
||||
- Правки редакции 2 не меняют ничего вне заявленных трёх пунктов: секции
|
||||
Сценарий, К1 (кроме tie-break), К2 (кроме одной фразы), К3–К7, модель
|
||||
данных, i18n, риски, откат и release-артефакты текстуально идентичны
|
||||
тому, что уже проверено в r1 (сверено построчным чтением текущего тела
|
||||
issue против содержания, процитированного в SPEC-REVIEW-676-r1.md).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `npm run typecheck`/`npm test`/`npm run build`,
|
||||
`node scripts/check-docs.mjs`, смоки, `golden:verify`, инварианты модели —
|
||||
как и в r1, кода задачи не существует (`git branch -a` не находит ветку
|
||||
`issue/676-*`; `git diff f1a87fba..HEAD` содержит только публикацию
|
||||
документа r1). Дифф для этих гейтов не существует, они относятся к этапу
|
||||
код-ревью.
|
||||
- Не проверял заново по коду весь фактический фундамент контракта
|
||||
(константы, файлы, существующие функции, 4 golden-сцены, i18n-паритет,
|
||||
math сектора курсора К2) — это сделано в r1 и не задето дельтой r2, кроме
|
||||
точки пересечения с `stairTargetState`, которую я перепроверил заново
|
||||
специально из-за характера правки AC8 (см. «Как проверялось», п. 5).
|
||||
- Не проверял, действительно ли `self` практически достижим через
|
||||
существующий UI (перенос/слияние помещений, J6) в разбивке по шагам —
|
||||
для находки достаточно, что модель (`stairs.ts`) не отвергает такие
|
||||
данные и канон (`STAIRS.md`) явно документирует состояние как
|
||||
поддерживаемое; глубже, чем чтение этих двух источников, не уходил.
|
||||
- Не проверял #663/#669 по существу — не в скоупе этой задачи, как и в r1.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Вердикт: жёлтый · заход r2 · блокирующих циклов 2/4 · High: 0 · Medium: 1 → в задаче
|
||||
|
||||
---
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `9fe7f88cb3ec` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `85bef1cf4119abfcaf8c9ff7275010213306fb9a`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 85bef1cf4119
|
||||
```
|
||||
- Тело issue: `e28517c301f2de5676ccf6e30f0997db7f3a5a36d10f7a1d66070909bb1b1d9d`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user