From e926ea453e2cf4d10776fd72ac7bb6548194e933 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:20:46 +0000 Subject: [PATCH] docs: review document for #405 Issue: #405 User-Visible: no --- docs/reviews/SPEC-REVIEW-405-r2.md | 157 +++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-405-r2.md diff --git a/docs/reviews/SPEC-REVIEW-405-r2.md b/docs/reviews/SPEC-REVIEW-405-r2.md new file mode 100644 index 00000000..c453bbc0 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-405-r2.md @@ -0,0 +1,157 @@ +# SPEC-REVIEW-405-r2 + +- Issue: https://github.com/Matysh/houseplan-card/issues/405 +- ТЗ: `docs/specs/405-dropped-promise-and-witness-floor.md` +- SHA ТЗ: `d5c93d4e28e977b05f22ca293da8945d60234b20` +- Заход: r2 · блокирующих циклов израсходовано 1 из 4 (после r1) +- Трек: полный (заявлен автором — правка пересекает три модуля, + `editor-secondary.ts`, `houseplan-editor-runtime.ts`, `houseplan-card.ts`; + критерий `small` «один модуль» не выполняется — согласен, довод предметный, + не декларативный) +- Вердикт: **зелёный** + +## Почему разбор полный, а не только по дельте + +Формально это второй заход (§2.10), но объём правки ревизии 2 не локален: +`git diff 2c6d937d..HEAD -- docs/specs/405-dropped-promise-and-witness-floor.md` +даёт 150 вставок / 206 удалений в файле, который после правки насчитывает 182 +строки, — переписан не абзац по находке, а документ целиком: сменилась +структура разделов («Проблема и контракты по пунктам» → «Проблема» + +«Техническая карта» + «Контракт поведения» отдельно), сузился скоуп (M4 и +Low ушли в #409, golden-двойник остаётся в #408), полностью переписаны AC1–AC8 +→ AC1–AC6 и план автотестов. Это «объём дельты сопоставим с исходной задачей» +из §2.10 — разбор веду по §7.1 целиком, как в r1, а не только по находкам +прошлого раунда. + +## Скоуп ревью + +ТЗ описывает единственную оставшуюся часть исходного аудита (M2 — оброненный +промис в `_deletePhysicalSelection`/`_deleteDraftWhole`); M4 (порог +свидетелей `docs-acceptance.mjs`) и сопутствующий Low (`acceptance.declared`) +исключены из этого issue и закрыты отдельно в #409 — проверено, что это не +формальная отписка, а фактически смердженный код (см. ниже). + +Продуктовая рамка не изменилась относительно r1: задача не про J1–J7, а про +доказательность существующего поведения (async-цепочка удаления черновика уже +работает для реального пользователя — клик не блокируется, промис просто +роняется на уровне API, поэтому `User-Visible: no` обоснован); класс файлов — +A (`src/**`), что делает трек полным по §1, но не требует отдельного +продуктового обоснования по `docs/SCOPE.md` — это тот же вывод, что и в r1, и +дельта его не задевает. + +## Как проверялось + +Читкой кода на HEAD (`d5c93d4e`), без исполнения — spec review, кода задачи +ещё нет. Каждая строка «Технической карты» сверена с файлом построчно: + +- `src/editor-secondary.ts:221` — `runContext(capturedContext, currentContext, action: () => void): void { if (capturedContext === currentContext) action(); }` — сигнатура и поведение совпадают с ТЗ дословно; +- `src/houseplan-editor-runtime.ts:5290` — `_runEditorContext(contextId, action: () => void): void` — совпадает; +- `src/houseplan-card.ts:8929-8930` — фасад `_runEditorContext` типа `void`, пробрасывает вызов в runtime — совпадает; +- `src/houseplan-editor-runtime.ts:3073-3100` — `_deletePhysicalSelection = (): void =>`, ветка `sel.kind === 'draft'` вызывает `this._deleteDraftWhole(); return;` без ожидания — совпадает, включая точное описание синхронных веток partition/column; +- `src/houseplan-card.ts:7897-7899` — `_deletePhysicalSelection = (): void => { return this._editorRuntimeOrThrow()._deletePhysicalSelection(); }` — это ровно тот пробел, который r1 нашёл как M2-a; в ревизии 2 он явно назван в карте и отдельным абзацем («Фасад … обязателен в правке») — закрыт; +- `src/houseplan-card.ts:7905,7909` — соседние фасады `_deleteDraftWhole`/`_deleteDraftSegment` уже `(): Promise` — подтверждает довод ТЗ о существующей конвенции файла; +- `src/houseplan-editor-runtime.ts:3119-3143` — `_deleteDraftWhole = async (): Promise =>`, ранний выход при `!accepted` без коммита транзакции (`:3133`) — подтверждает AC3; +- `grep runContext( src/*.ts` — единственный вызов `this.host._editorSecondary.runContext(...)` на `houseplan-editor-runtime.ts:5291`; единственный потребитель, генерализация не задевает скрытых вызывающих; +- `_runEditorContext` в редакторе используется в шести местах (`:5340,5344,5375,5411,5415,5465`), один из них — `() => this._deletePhysicalSelection())` (`:5344`) — именно этот вызов после правки станет возвращать `Promise› из замыкания, что обосновывает необходимость генерика, а не `void`; +- `demo/smoke_free_walls.mjs:11,16,194,199` — `window.__card`, `_confirmDanger = async () => true` (мгновенный стаб), `await c._deletePhysicalSelection()`, `o.deleteRemovesPartition`, `o.deleteOnDraftRemovesWholeOutline` — имена утверждений и структура совпадают с тем, что ТЗ использует в AC1/AC2; +- `src/houseplan-card.ts:2153` — `_confirmDanger = (request): Promise =>` — подтверждает, что подтверждение в реальном UI асинхронно (раздел «Сценарий»), не декларация; +- `scripts/mutation-gate.mjs` — конвенция `id: 'kebab-case'` подтверждена (20+ примеров), `draft-delete-drops-the-promise` соответствует. + +Отдельно проверено закрытие вынесенных находок r1 вне этого ТЗ: + +- `git show --stat 8119c523` — коммит существует, меняет `scripts/docs-acceptance.mjs` + (сообщение коммита прямо описывает переход порога с `withCommitted.length` + на размер набора сценариев и попутное исправление `acceptance.declared` — + это ровно M4 + Low из r1); +- `gh issue view 409` — закрыт, содержит сообщение о починке, ссылается на + #405 и #408 симметрично; +- `gh issue view 408` — закрыт, комментарий закрытия описывает тот же класс + дефекта для golden и прямо называет `docsWitnessFloor` как «сестринский, + пока открытый» на момент своего закрытия — согласуется с тем, что #409 + тогда ещё не был сделан. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| M2-a (Medium, в скоупе): таблица «Место» не называет фасад `src/houseplan-card.ts:7856` (сейчас `:7897`, номер строки съехал вместе с остальным файлом) | Строка добавлена в «Техническую карту» ревизии 2 с явным обоснованием (`TS2322`, конвенция соседних фасадов) | `docs/specs/405-dropped-promise-and-witness-floor.md:50` (строка таблицы `src/houseplan-card.ts:7897`) и абзац `:53-57` | +| Low: ссылка «#406 «д»» на несуществующий пункт | Ссылка удалена вместе со всем разделом M4/Low — тема перенесена в #409, ссылка на #406 в файле отсутствует | `grep -n '#406' docs/specs/405-*.md` — пусто; раздел «Не-скоуп» ссылается на `#409, коммит 8119c523` вместо `#406` | +| M4 (Medium вне скоупа #405, но упомянутая в исходном ТЗ r1 как часть контракта) | Исключена из #405 решением владельца, сделана отдельно | Комментарий владельца в issue от 2026-09-01T14:55, коммит `8119c523`, issue #409 закрыт | + +## Унаследовано из r1 + +Формально ничего — раздел ведён для соблюдения формата §2.10, но по причине, +изложенной выше («Почему разбор полный»), в этом раунде заново проверены все +технические утверждения ТЗ, а не только находки r1. Единственное, что +перенесено без повторной проверки, — общая продуктовая рамка «задача не +требует обоснования по `docs/SCOPE.md`, потому что чинит доказательность +существующего инварианта, а не добавляет функциональность»: вывод сделан в +`docs/reviews/SPEC-REVIEW-405-r1.md` (SHA ревью `bf40e26e`, ТЗ на тот момент +`cd0c85b9`/эквивалентно `2c6d937d`) и дельта его не касается — скоуп сузился +(M4 ушла), но не сменил категорию. + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +## Проверено и корректно + +- Обязательные разделы §7.1 присутствуют все: сценарий, что человек увидит, + проблема, техническая карта, контракт поведения, скоуп/не-скоуп, UX, модель + данных/миграция, i18n, производительность, AC1–AC6 с доказательством, план + автотестов, риски, откат, release-артефакты, плюс блок «Принятые технические + предположения». +- Каждая строка «Технической карты» сверена с кодом на HEAD построчно (список + выше) — расхождений нет, номера строк точны. +- AC1–AC6 однозначны, у каждого назван способ доказательства + (`demo/smoke_free_walls.mjs`, `scripts/mutation-gate.mjs`, `tsc --noEmit`, + unit-тест `runContext`, ревью кода `@click`-обработчиков) — ни один не + сформулирован как «работает корректно» без критерия. +- AC2 явно требует отрицательный прогон (мутант `draft-delete-drops-the-promise`) + — соблюдено правило «тест должен уметь падать» на уровне постановки. +- Риски называют реальные последствия генерализации (`runContext` вызывается + в единственном месте — при желании можно было бы не обобщать, но обобщение + оправдано тем, что тот же вызывающий `_runEditorContext` используется для + других действий с `void`-контрактом, и явно назван компромисс с + `void | undefined`); синхронные исключения `_deletePhysicalSelection`, + ставшего асинхронным, корректно отнесены к #404, а не к этой задаче. +- «Не-скоуп» точен: `_deleteDraftSegment` действительно имеет отдельный, + уже типизированный `Promise` путь (`houseplan-card.ts:7909`), + вынесение M4/Low в #409 подтверждено смердженным кодом, golden-аналог в + #408 подтверждён закрытым issue с описанием фикса. +- Технические предположения в конце (генерик `T` вместо `void | Promise`, + единый `Promise` вместо union, номера строк ориентировочны) — + действительно технические решения по границе §7.1, не продуктовые вопросы, + и явно помечены как предположения, а не факты. +- Продуктовых вопросов владельцу нет, откат и release-артефакты соразмерны + заявленному `User-Visible: no`. + +## Чего не проверял + +- Гейты (`tsc`, `npm test`, `npm run build`, смоки, `golden:verify`, + `model-invariants`) не прогонял — кода задачи ещё нет, прогонять нечего; + единственная эмпирика уровня r1 (изолированный `tsc` на форме + `houseplan-card.ts:7897`, доказавшая `TS2322`) не переповторялась — вывод + тот же, а строка в ревизии 2 теперь прямо в карте. +- Не проверял, действительно ли `scripts/mutation-gate.mjs` технически + заведёт мутант `draft-delete-drops-the-promise` без ошибок раннера — вопрос + реализации, не ТЗ. +- Не проверял `test/docs-acceptance.test.mjs` и код `8119c523` целиком на + предмет собственных дефектов — это закрытый issue #409 вне скоупа этого + ревью; проверено только то, что коммит существует и по сообщению делает + заявленное. +- Не оценивал возможные дефекты вне названных в ТЗ AC1–AC6 — полнота + «Технической карты» и корректность привязки к коду проверены, дальше это + вопрос код-ревью. + +## Итог + +Ревизия 2 закрывает единственную блокирующую находку r1 (M2-a) точным +добавлением строки в карту с верным техническим обоснованием, и Low-находку — +удалением ложной ссылки вместе с переносом темы в #409. Скоуп сузился +корректно: M4 и сопутствующий Low закрыты отдельно (#409, `8119c523`, +смерджено), golden-двойник остаётся заведённым (#408, тоже уже закрыт). +Оставшийся контракт (M2) описан однозначно, каждый AC доказуем, техническая +карта совпадает с кодом построчно, продуктовых вопросов нет. + +**Вердикт: зелёный.**