docs: review document for #405

Issue: #405
User-Visible: no

docs/reviews/SPEC-REVIEW-405-r1.md

@@ -0,0 +1,250 @@
+# SPEC-REVIEW-405-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/405
+- ТЗ: `docs/specs/405-dropped-promise-and-witness-floor.md`
+- SHA ТЗ: `cd0c85b9ad5096d4772f99edc0354dea6a62f241`
+- Заход: r1 · блокирующих циклов израсходовано 0 из 4 (до этого раунда)
+- Трек: полный (заявлен автором — две несвязанные поверхности, `src/**` и
+  `scripts/**`; критерий `small` «одна поверхность» не выполняется — согласен)
+- Вердикт: **жёлтый**
+
+## Скоуп ревью
+
+Первый заход, дельты нет — разбор ТЗ целиком по §7.1: обязательные разделы,
+однозначность и доказуемость AC1…AC8, соответствие описанных технических фактов
+реальному коду (`src/houseplan-editor-runtime.ts`, `src/houseplan-card.ts`,
+`src/editor-secondary.ts`, `demo/smoke_free_walls.mjs`,
+`scripts/docs-acceptance.mjs`, `test/docs-acceptance.test.mjs`), а также
+продуктовая рамка по `docs/SCOPE.md` (задача инфраструктурная по духу — не про
+J1–J7, а про доказательность гейтов, что для класса B/тестовой инфраструктуры
+уместно и не требует отдельного продуктового обоснования; правка `src/**`
+делает задачу полным треком по §1, но не продуктовой в смысле пользовательского
+сценария — ТЗ прямо пишет «видимого поведения не меняет», что не противоречит
+скоупу, поскольку задача чинит доказательство существующего инварианта, а не
+добавляет функциональность).
+
+## Как проверялось
+
+Читкой кода, без исполнения (spec review, гейты не прогонялись — это код-ревью
+этап, здесь ему нечего проверять по §2.4). Для каждого технического утверждения
+ТЗ — сверка со строками файлов на HEAD (`cd0c85b9`):
+
+- `src/houseplan-editor-runtime.ts:3050-3150, 5282-5300` — таблица «Место» для
+  M2 (`_deletePhysicalSelection`, `_runEditorContext`, `_deleteDraftWhole`);
+- `src/editor-secondary.ts:200-224` — `runContext`;
+- `src/houseplan-card.ts:3015-3022, 7845-7874` — вызывающие и параллельные
+  обёртки `_deletePhysicalSelection`/`_deleteDraftWhole`/`_deleteDraftSegment`;
+- `demo/smoke_free_walls.mjs:1-30, 175-208` — стаб `_confirmDanger` и вызовы
+  `_deletePhysicalSelection`;
+- `scripts/docs-acceptance.mjs:1-128` — `docsWitnessFloor`,
+  `docsAcceptancePlan`, вычисление `floor`;
+- `test/docs-acceptance.test.mjs` — существующие тесты AC7/AC8;
+- `scripts/golden-acceptance.mjs:1-200`, `demo/golden/accept.mjs:1-121`,
+  `demo/golden/run.mjs:800-850` — сверка риска «изменение порога затрагивает
+  golden»;
+- `scripts/mutation-gate.mjs` — конвенция именования мутантов (`id: 'kebab-case'`),
+  сверена с предложенными `draft-delete-drops-the-promise` и
+  `witness-floor-counts-survivors` — согласуется.
+
+Один факт проверен эмпирически (не просто чтением): предположение, что
+TypeScript пропустит функцию, аннотированную `(): void`, но фактически
+возвращающую `Promise<void>` изнутри, — **проверено `tsc`** на изолированном
+файле (см. находку M2-a ниже), потому что от этого зависела оценка серьёзности
+находки, а не только её наличие.
+
+Issue #406 (куда ТЗ отправляет один Low-пункт) прочитан целиком через
+`gh issue view 406` — сверка ссылки «#406 «д»».
+
+## Находки
+
+### M2-a (Medium, в скоупе) — таблица «Место» неполна: пропущена обёртка `houseplan-card.ts:7856`, и без неё правка не пройдёт `tsc --noEmit`
+
+Файл: `docs/specs/405-dropped-promise-and-witness-floor.md`, раздел «(1) M2»,
+таблица «Место» и абзац «Практически».
+
+**Что не так.** Смок вызывает не `houseplan-editor-runtime.ts`, а
+`window.__card` — экземпляр класса `houseplan-card.ts` (`demo/srv/demo.html:194`
+создаёт `<houseplan-card>`, `demo/serve.mjs` его же отдаёт в `window.__card`,
+`smoke_free_walls.mjs:194,199` зовёт `c._deletePhysicalSelection()`). Реальная
+точка входа — обёртка `src/houseplan-card.ts:7856`:
+
+```ts
+private _deletePhysicalSelection = (): void => {
+  return this._editorRuntimeOrThrow()._deletePhysicalSelection();
+}
+```
+
+Таблица «Место» в ТЗ называет только `editor-secondary.ts:221`,
+`houseplan-editor-runtime.ts:5290` и `:3073` — эту строку не называет вовсе.
+«Практически» перечисляет три функции для правки (`_deletePhysicalSelection`
+в runtime, `_runEditorContext`, `runContext`) — тоже без неё.
+
+**Воспроизведено**: изолированный `tsc --strict --noEmit` на паре функций,
+повторяющей форму `houseplan-card.ts:7856` после того, как внутренний вызов
+меняет тип на `Promise<void>`, а внешняя обёртка остаётся `(): void`:
+
+```ts
+async function inner(): Promise<void> {}
+const wrapper = (): void => {
+  return inner();
+};
+```
+
+```
+error TS2322: Type 'Promise<void>' is not assignable to type 'void'.
+```
+
+Это не эстетика типов — это реальный отказ гейта `npx tsc --noEmit`,
+единственного среди трёх дешёвых гейтов, который правка данной задачи
+действительно затрагивает. Если исполнитель проверит только смок (JS,
+типов не видит) или отправит правку без прогона `tsc`, регресс поймает CI,
+но раунд код-ревью на это тратиться не должен.
+
+**Это не догадка о недостающем шаге, а видимый паттерн этого же файла.**
+Строками ниже, `houseplan-card.ts:7864` и `:7868`, у соседних методов
+(`_deleteDraftWhole`, `_deleteDraftSegment`) обёртки **уже** типизированы
+`Promise<void>`, зеркаля асинхронные методы runtime. Это подтверждает: в файле
+есть устоявшаяся конвенция «тип обёртки повторяет тип метода runtime», и
+`_deletePhysicalSelection`-обёртка на `:7856` — единственная, которую эта
+задача обязана привести к тому же виду, а ТЗ её не называет.
+
+**Требуется**: добавить `src/houseplan-card.ts:7856` в таблицу «Место» и
+в перечень правок «Практически» — тип обёртки меняется на `Promise<void>` с
+`return`, как у `_deleteDraftWhole`/`_deleteDraftSegment` рядом.
+
+Серьёзность — Medium, не High: контракт AC1–AC8 в остальном верен и
+достижим, дефект — не в контракте, а в полноте технической карты внутри уже
+названного в скоупе файла (`src/houseplan-card.ts` фигурирует в файлах задачи
+неявно — это тот же модуль, где объявлена обёртка `_deleteDraftWhole`,
+`_deleteDraftSegment`, уже упомянутых как соседний прецедент); дешёвый гейт
+`tsc` его поймает при обычной проверке, но ТЗ, которое ведёт прямиком к
+провалу первого же гейта, не «выполнимо без доработки» по мерке §2.4.
+
+### M4-b (Low) — ссылка «#406 «д»» на несуществующий пункт
+
+Файл: `docs/specs/405-dropped-promise-and-witness-floor.md`, раздел «Скоуп /
+не-скоуп»:
+
+> затирание `acceptance.declared` при повторной приёмке (#406 «д») — соседняя
+> строка того же файла, но другой контракт
+
+Прочитан issue #406 целиком (`gh issue view 406`): в нём ровно четыре пункта,
+(а)–(г) — мёртвые ключи `confirm.*`, роль диалога, непокрытая HA-ветка
+подтверждения, снапшоты area. Пункта «(д)» нет. Оригинальный аудит-комментарий
+в теле #405 упоминает это как отдельный Low-пункт («Отдельно, из того же
+разбора (Low, чинить попутно): повторная приёмка неизменённого набора
+затирает `acceptance.declared`… `scripts/docs-accept.mjs:142-150`»), но нигде
+не заводит его как пункт «(д)» и не кладёт в #406.
+
+Итог: Low-находка из исходного аудита сейчас **не отслеживается ни одним
+issue** — ссылка ТЗ на несуществующий пункт создаёт впечатление, что она
+где-то учтена, хотя это не так.
+
+**Решение ревьюера**: не правится сейчас (сам дефект `docs-accept.mjs:142-150`
+за пределами скоупа #405 — другая строка того же файла, другой контракт, как
+верно замечает автор) — но ссылка ложная и должна быть исправлена: либо
+пункт действительно добавляется в #406, либо ТЗ отправляет его в новый issue,
+либо ТЗ формулирует «Low, пока нигде не заведён» вместо несуществующей ссылки.
+Это правится в рамках текущего цикла возврата (Low, не отдельный лимит).
+
+## Медиум вне скоупа — заведён отдельным issue
+
+### Golden-эталоны подвержены тому же классу дефекта, что и M4 — #408
+
+`scripts/golden-acceptance.mjs:182-183` считает порог свидетелей от
+`withBaseline.length` — числа сцен со статусом **не** `missing-baseline`, то
+есть от того, что уцелело на диске, а не от размера известной матрицы
+(`GOLDEN_SCENARIOS.length`). Проверено чтением всей цепочки
+(`demo/golden/run.mjs:836-839` ставит `status: 'missing-baseline'`, когда файла
+эталона нет; `goldenAcceptanceRefusal` в `scripts/golden-acceptance.mjs:59-93`
+**требует**, чтобы каждая такая сцена была объявлена через `--expect-new` —
+и не более; `demo/golden/accept.mjs:64-72` вызывает `goldenWitnessRefusal` уже
+после этой проверки). Модельный разбор: `git rm demo/golden/baselines/*.png`
++ объявление всех сцен `--expect-new` → каждая сцена легально проходит
+`goldenAcceptanceRefusal` → `withBaseline.length === 0` →
+`goldenWitnessFloor(0) === 0` → `witnesses.length (0) < floor (0)` ложно →
+отказа `goldenWitnessRefusal` нет. `--reviewed` не является независимой
+проверкой (булев CLI-флаг), так что рассуждение ТЗ #405 о том, что M4
+недостаточно прикрыт `--reviewed` для `docs-accept.mjs`, симметрично относится
+и сюда.
+
+ТЗ #405 обсуждает golden только в разделе «Риски» и только применительно к
+формуле (`goldenWitnessFloor` та же формула, что `docsWitnessFloor`, и должна
+остаться той же) — не к тому, что вызывающий код (`withBaseline.length`)
+подвержен той же уязвимости на входе, что чинит M4. Файл другой
+(`scripts/golden-acceptance.mjs`, не `scripts/docs-acceptance.mjs`), скоуп
+ТЗ его не называет — правка не входит в #405.
+
+Issue заведён: **#408**, метки `bug`, `P2`, `S1-new`, `infra`, ссылка на #405.
+
+## Проверено и корректно
+
+- Обязательные разделы §7.1 присутствуют все: сценарий, что человек увидит,
+  проблема, скоуп/не-скоуп, контракт, UX, модель данных/миграция, i18n, AC1–AC8
+  с доказательством, план автотестов, риски, откат, release-артефакты.
+- Продуктовых вопросов владельцу нет, догадок, выданных за факт, в контракте
+  M2/M4 не найдено (кроме описанного выше пробела в таблице «Место» — это не
+  догадка, а неполнота карты).
+- Таблица «Место» для M2 (кроме отсутствующей строки `houseplan-card.ts:7856`)
+  точно соответствует коду: `runContext` (`editor-secondary.ts:221`),
+  `_runEditorContext` (`houseplan-editor-runtime.ts:5290`),
+  `_deletePhysicalSelection` (`:3073`, ветка `sel.kind === 'draft'` действительно
+  роняет промис через `return`), `_deleteDraftWhole` (`:3119`, действительно
+  `async`, `await _confirmDanger`).
+- Смок `demo/smoke_free_walls.mjs:16` — стаб `_confirmDanger = async () => true`
+  резолвится в том же микротаске, и `:194,199` действительно не проверяет
+  ожидание по-настоящему; воспроизведение из тела issue (макрозадача → два
+  `FAILED`) правдоподобно и согласуется с кодом.
+- `scripts/docs-acceptance.mjs:44-46,107-108` — формула и место вычисления
+  `floor` от `withCommitted.length` подтверждены; контраст «PNG на месте» vs
+  «PNG удалены» из тела issue воспроизводим по чтению кода без исполнения.
+- Предложенный контракт AC6 (`floor` от размера набора сценариев, не от
+  наличия файлов) корректно сохраняет существующее поведение AC7 — проверено
+  арифметически: `docsWitnessFloor(ids.length)` и текущее
+  `docsWitnessFloor(withCommitted.length)` дают одно и то же значение во всех
+  ветках существующих тестов `test/docs-acceptance.test.mjs`, включая
+  «кадр без закоммиченной пары не может быть свидетелем» (`floor` остаётся 1
+  в обоих случаях — 3 или 1 округляются к одному порогу на этом наборе).
+- «Не-скоуп» по `_deleteDraftSegment` — проверено: у него отдельный,
+  корректно типизированный путь (`houseplan-card.ts:7868` уже `Promise<void>`,
+  зовётся напрямую из своей кнопки, не через `_deletePhysicalSelection`) —
+  исключение обосновано верно, не прячет тот же дефект.
+- Риск «изменение порога затрагивает golden» правильно определяет, что *формулу*
+  трогать нельзя без зеркалирования — но не замечает независимый дефект на
+  стороне вызова (см. #408).
+- AC2 (отрицательный прогон обязателен) сформулирован по правилу «тест должен
+  уметь падать» и предлагает конкретный механизм (`scripts/mutation-gate.mjs`,
+  который в проекте уже используется по этому шаблону) — не декларация.
+- Откат и release-артефакты соразмерны факту «видимого поведения нет»
+  (`User-Visible: no` подтверждён отсутствием изменений в UX/i18n разделах).
+
+## Чего не проверял
+
+- Гейты (`tsc`, `npm test`, `npm run build`, смоки, `golden:verify`,
+  `model-invariants`) не прогонял — этап spec review, кода задачи ещё нет,
+  прогонять нечего. Эмпирическая проверка ограничена одним изолированным
+  TS-файлом вне репозитория, подтверждающим находку M2-a (см. выше), — это не
+  прогон проектных гейтов.
+- Не проверял `scripts/mutation-gate.mjs` на предмет того, заведутся ли
+  предложенные мутанты технически без ошибок раннера — это вопрос реализации,
+  а не ТЗ; на этапе спеки достаточно, что названный механизм существует и
+  используется в проекте по тому же образцу.
+- Не проверял golden `test/golden-policy.test.mjs` целиком — находка по #408
+  установлена чтением `scripts/golden-acceptance.mjs` и `demo/golden/accept.mjs`
+  напрямую; тестовое покрытие этой ветки (принимает ли текущий набор тестов
+  сценарий «все эталоны удалены») не сверял — это будет частью работы над #408,
+  не #405.
+- Не оценивал вне двух Medium/одного Low другие возможные дефекты в
+  `scripts/docs-accept.mjs` целиком (не-`docs-acceptance.mjs`) — за пределами
+  того, что уже упомянуто как «не-скоуп» самим ТЗ.
+
+## Итог
+
+Один Medium **в скоупе** (M2-a) не даёт зелёный вердикт по §2.4 — ТЗ ведёт
+прямиком к отказу гейта `tsc --noEmit`, если исполнитель буквально следует
+таблице «Место». Плюс один Low (ложная ссылка на #406) для правки в этом же
+цикле. Medium вне скоупа заведён отдельным issue (#408), в задаче #405 не
+чинится.
+
+**Вердикт: жёлтый.** Возврат автору: дополнить таблицу «Место» и раздел
+«Практически» строкой `src/houseplan-card.ts:7856`, поправить ссылку на #406.