mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
@@ -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<void>` — подтверждает довод ТЗ о существующей конвенции файла;
|
||||
- `src/houseplan-editor-runtime.ts:3119-3143` — `_deleteDraftWhole = async (): Promise<void> =>`, ранний выход при `!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>› из замыкания, что обосновывает необходимость генерика, а не `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<boolean> =>` — подтверждает, что подтверждение в реальном 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<void>` путь (`houseplan-card.ts:7909`),
|
||||
вынесение M4/Low в #409 подтверждено смердженным кодом, golden-аналог в
|
||||
#408 подтверждён закрытым issue с описанием фикса.
|
||||
- Технические предположения в конце (генерик `T` вместо `void | Promise<void>`,
|
||||
единый `Promise<void>` вместо 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 доказуем, техническая
|
||||
карта совпадает с кодом построчно, продуктовых вопросов нет.
|
||||
|
||||
**Вердикт: зелёный.**
|
||||
Reference in New Issue
Block a user