mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 21:01:21 +00:00
@@ -0,0 +1,186 @@
|
||||
# SPEC-REVIEW-265-r2
|
||||
|
||||
- Issue: [#265](https://github.com/Matysh/houseplan-card/issues/265) — «Рефакторинг 2/5: один контракт шва импорта — ремап ссылок за собой и идемпотентная уборка»
|
||||
- Этап: `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
|
||||
- Заход: r2 · блокирующих циклов израсходовано 1/4 до этого вердикта
|
||||
- ТЗ: `docs/specs/265-import-reference-seam.md`, ветка `issue/265-import-seam-contract`, SHA на момент ревью — HEAD (`67bcce8e`, коммит «docs: address import seam spec review»)
|
||||
- Ревьюер: Claude (свежая сессия, без переписки с автором)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Второй заход, разбор по дельте (PROCESS.md §2.10, issue #214). Продукт кода в
|
||||
этой задаче ещё нет — единственный изменённый файл класса C —
|
||||
`docs/specs/265-import-reference-seam.md`; диапазон дельты
|
||||
`git diff 2cf47fc1..HEAD -- docs/specs/265-import-reference-seam.md`
|
||||
(2cf47fc1 — SHA, на котором получен вердикт r1, назван явно в шапке
|
||||
`SPEC-REVIEW-265-r1.md`, повторно искать его не пришлось).
|
||||
|
||||
Дельта локальна: автор правил ровно те четыре раздела, на которые указали
|
||||
находки r1 (сценарий/«до-после», i18n-обязательства AC10, способ доказательства
|
||||
по каждому AC, раздел рисков), плюс сквозную перенумерацию заголовков,
|
||||
вызванную вставкой двух новых секций. Контракт поведения (lineage-алгоритм,
|
||||
матрица ссылок, immutable candidate, порядок remap, edge cases) не менялся ни
|
||||
по одной строке — рёбейза на ушедший вперёд `dev` не было, новая подсистема не
|
||||
затронута, объём правки заметно меньше исходной задачи. Условия «разбор
|
||||
остаётся полным» (§2.10) не выполнены — сокращение объёма законно.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M1** — «что человек увидит» слито со сценарием, написано языком реализации; поверхность/touch не названы | Раздел разделён на «§1 Сценарий и персона» и «§2 Что человек увидит до и после»; «До/После» переписаны бытовым языком без «lineage»/«canonical root»/«candidate»; отдельно назван desktop-only статус и touch/kiosk-поведение | `docs/specs/265-import-reference-seam.md:19-50` |
|
||||
| **M2** — AC10 хеджирует «если отчёт станет виден», хотя §9(старое)/§10(новое) уже безусловно описывает новый текст; i18n-ключи не перечислены | AC10 переписан без условности; конкретные RU/EN ключи перечислены поимённо | AC10: `265-import-reference-seam.md:435-439`; список ключей: `265-import-reference-seam.md:292-305` |
|
||||
| **M3** — AC1–AC9 не называют способ доказательства | Добавлена таблица «§15.1 Матрица доказательств» — по одной строке на каждый AC1–AC10 | `265-import-reference-seam.md:441-454` |
|
||||
| **M4** — нет отдельного обязательного раздела «Риски» | Добавлен «§13. Риски» — таблица из 8 строк (риск / последствие / снижение) | `265-import-reference-seam.md:347-358` |
|
||||
|
||||
Все четыре находки r1 закрыты предметно, не декларативно: в каждом случае в
|
||||
дельте есть конкретный новый текст, а не переформулировка старого абзаца под
|
||||
тем же смыслом.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Без повторной проверки принято (документ `docs/reviews/SPEC-REVIEW-265-r1.md`,
|
||||
SHA `2cf47fc1`, полный разбор первого захода):
|
||||
|
||||
- §3 «Подтверждённое состояние кода» (было §2) — построчная сверка с
|
||||
`import_export.py` (`_fresh`, `_repair_target_space_refs`,
|
||||
preview/apply/revalidate) и с пробелами `scripts/model-invariants.mjs`;
|
||||
текст не менялся, изменился только номер раздела;
|
||||
- §4 «Унаследованные продуктовые решения» (было §3) — сверка со статусом и
|
||||
текстом закрытых #244/#248/#252/#258/#262; содержимое не менялось;
|
||||
- §5 «Цели и границы» (было §4), §6 «Канонический lineage id» (было §5,
|
||||
включая проверку edge case с ложным совпадением root), §7 «Неизменяемый
|
||||
кандидат preview/apply» (было §6), §9 «Порядок remap и конфликты» (было §8),
|
||||
§11 «Инварианты и отказоустойчивость» (было §10), §12 «Совместимость и
|
||||
миграция» (было §11), §14 «Edge cases» (было §12), §16–§19 (план реализации,
|
||||
проверки, rollback, допущения) — содержимое не менялось, изменилась только
|
||||
нумерация;
|
||||
- терминология («Оптимизировать планы», «carrier») — сверена с
|
||||
`docs/USER-GUIDE.ru.md`/`docs/WALL-THICKNESS.md` в r1, дельта эти фразы не
|
||||
трогает;
|
||||
- три «унаследованных решения владельца» и отсутствие продуктовых вопросов
|
||||
владельцу — не пересматривались, дельта их не касается.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Найден вердикт r1 и SHA: `docs/reviews/SPEC-REVIEW-265-r1.md` называет SHA
|
||||
`2cf47fc1` в шапке документа явно — искать по логу не пришлось.
|
||||
2. Дельта: `git diff 2cf47fc1..HEAD --stat` — изменены только
|
||||
`docs/specs/265-import-reference-seam.md` (+168/-63 строк) и сам файл
|
||||
`docs/reviews/SPEC-REVIEW-265-r1.md` (добавлен публикующим шагом, не
|
||||
автором). Продуктового кода в дельте нет.
|
||||
3. По каждой из находок M1–M4 найдена конкретная замена в тексте (таблица
|
||||
выше), а не общее заявление «поправлено» — сверено построчно через
|
||||
`git diff 2cf47fc1..HEAD -- docs/specs/265-import-reference-seam.md`.
|
||||
4. Проверены новые фактические утверждения, добавленные именно в этой дельте
|
||||
(§2.10 требует разбирать AC/утверждения, которых касается дельта, а
|
||||
новый текст i18n-ключей и таблицы доказательств — целиком продукт этой
|
||||
дельты):
|
||||
- `backup.repaired_target_refs` и `backup.dropped_marker_links` —
|
||||
существуют в `src/i18n/en.json:995-996` и `src/i18n/ru.json:995-996`,
|
||||
утверждение «существующие … сохраняются» подтверждено, а не
|
||||
угадано;
|
||||
- `docs/USER-GUIDE.md`/`docs/USER-GUIDE.ru.md` существуют, термины
|
||||
«Оптимизировать планы»/«Общие настройки» в них есть;
|
||||
- `check-docs` (упомянут в матрице доказательств AC10) —
|
||||
`scripts/check-docs.mjs` существует;
|
||||
- `check-i18n` (упомянут в той же строке AC10) — **не существует**: нет
|
||||
такого имени ни в `package.json` (`python3 -c "..."` дамп секции
|
||||
`scripts`), ни файла `scripts/check-i18n.mjs`, ни упоминания в
|
||||
`.github/workflows/*.yml`. Единственная реальная механическая проверка
|
||||
en/ru паритета ключей — тест `test/i18n.test.mjs` («i18n: en and ru
|
||||
dictionaries carry the same key set»), часть обычного `npm test`, а не
|
||||
отдельная команда с таким именем. См. находку M5 ниже.
|
||||
5. Проверена внутренняя согласованность перенумерации: `grep -n '§[0-9]'` по
|
||||
всему файлу — все перекрёстные ссылки (`§8` в 6.2, AC3, 15.1; `§10` в AC10)
|
||||
указывают на корректный новый номер соответствующего раздела; провисших
|
||||
ссылок на старые номера не найдено.
|
||||
6. Гейты этапа: `npx tsc --noEmit`/`npm test`/`npm run build` не запускались —
|
||||
диапазон дельты не содержит кода (только `docs/specs/**`), на этапе
|
||||
`S4-spec-review` продуктового кода ещё нет, гейты по своему условию
|
||||
неприменимы — то же основание, что в r1. `node scripts/check-docs.mjs` не
|
||||
запускался: дельта не касается `src/**`.
|
||||
|
||||
## Находки
|
||||
|
||||
### M5 — AC10 называет несуществующий гейт `check-i18n` как обязательное доказательство (Medium, в скоупе)
|
||||
|
||||
**Файл:** `docs/specs/265-import-reference-seam.md`, §15.1 «Матрица
|
||||
доказательств», строка AC10 (443-454, конкретно строка со списком
|
||||
`` `check-i18n`, `check-docs`, RU/EN guide/changelog diff, targeted browser
|
||||
smoke и reviewed canonical golden import preview ``).
|
||||
|
||||
Это прямое следствие правки M3 (появление самой таблицы) — то есть новая
|
||||
находка внутри дельты этого раунда, а не наследие r1.
|
||||
|
||||
`check-i18n` не существует нигде в репозитории: ни как npm-скрипт в
|
||||
`package.json` (полный список ключей `scripts` не содержит такого имени;
|
||||
единственный i18n-related артефакт — `test/i18n.test.mjs`), ни как файл
|
||||
`scripts/check-i18n.mjs`, ни как шаг в `.github/workflows/*.yml`. Реальная
|
||||
механическая проверка паритета en/ru-ключей — тест
|
||||
`test/i18n: en and ru dictionaries carry the same key set` внутри обычного
|
||||
`npm test`; `check-docs.mjs` в этой же строке — существующий скрипт
|
||||
(`scripts/check-docs.mjs`), это единственная корректная часть строки.
|
||||
|
||||
Это ровно тот класс дефекта, о котором предупреждает регламент ревью:
|
||||
утверждение об инструменте, которого нет ни в одном документе и ни в одном
|
||||
скрипте, вписано в раздел, чья единственная функция — назвать проверяемый
|
||||
способ доказательства AC (DoR §2.5: «у каждого указано, чем он
|
||||
доказывается»). Разработчик на S7 либо потратит время на поиск
|
||||
несуществующей команды, либо спишет её как опечатку без исправления текста
|
||||
ТЗ — оба исхода хуже правильной строки с самого начала.
|
||||
|
||||
**Почему Medium, а не High:** не меняет дизайн, не блокирует понимание
|
||||
контракта — соседние 9 строк таблицы верны и однозначны, ошибка локальна
|
||||
к одному имени в одной ячейке.
|
||||
|
||||
**Как чинится:** заменить `check-i18n` на существующий механизм — например,
|
||||
«`npm test` (`test/i18n.test.mjs`, паритет ключей en/ru) плюс ручная сверка
|
||||
переводов ревьюером» — без изменения остального содержания строки AC10 и
|
||||
без изменения самого AC10.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все четыре находки r1 (M1–M4) закрыты предметно: в каждом случае есть новый
|
||||
фрагмент текста, отвечающий именно на то, что просила находка, а не
|
||||
косметическая переформулировка (детали — таблица «Закрытие раунда r1»).
|
||||
- Новые фактические утверждения, добавленные этой дельтой (существующие
|
||||
i18n-ключи, `check-docs.mjs`, наличие `USER-GUIDE.md`/`USER-GUIDE.ru.md`),
|
||||
проверены по факту, а не приняты на слово — кроме `check-i18n` (см. M5),
|
||||
все подтвердились.
|
||||
- Раздел рисков (§13) содержит по одному пункту на каждый архитектурно
|
||||
значимый риск, который upstream-документ и r1-ревью уже обсуждали
|
||||
(ложное совпадение lineage, расхождение Python/TS, память preview,
|
||||
неполнота матрицы, duplicate-policy vs light-link, fail-closed на legacy,
|
||||
revalidate race, повреждение геометрии) — пусто не осталось.
|
||||
- Перенумерация разделов (1→19) внутренне согласована, ни одной "битой"
|
||||
перекрёстной ссылки на старый номер не найдено.
|
||||
- Раздел «Что человек увидит» (§2) теперь отвечает на вопрос PROCESS.md §7.1
|
||||
без терминов реализации, а §1 явно называет desktop/View/kiosk/touch
|
||||
поверхности — ровно то, чего не хватало в r1.
|
||||
- `docs/specs/README.md` не тронут в этой дельте (правка не требовалась,
|
||||
строка таблицы уже добавлена в r1 и остаётся верной).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `typecheck`/`test`/`build`/смоки/инварианты — продуктового кода
|
||||
в дельте нет, гейты неприменимы на этапе `S4-spec-review` (то же основание,
|
||||
что в r1).
|
||||
- Не проверял `node scripts/check-docs.mjs` — дельта не касается `src/**`.
|
||||
- Не пересматривал технические решения §3–§9, §11–§12, §14, §16–§19 —
|
||||
дельта их не касается, унаследованы из r1 (раздел выше).
|
||||
- Не проверял конкретные будущие формулировки переводов en/ru строк отчёта
|
||||
(`backup.preserved_unresolved_hint` и др.) на стилистическую корректность —
|
||||
тексты появятся в реализации, это предмет код-ревью.
|
||||
- Не оценивал эффективность/производительность заново — алгоритмическая
|
||||
часть (§6, §8, §9, §11) дельтой не изменена, оценка r1 остаётся в силе.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Все четыре находки r1 закрыты предметно и проверяемо. Дельта этого раунда,
|
||||
однако, сама вносит одну новую Medium-находку в скоупе — несуществующий
|
||||
гейт `check-i18n`, названный обязательным доказательством AC10. High-находок
|
||||
нет, находка вне скоупа не заводится: правка ограничена той же ячейкой той же
|
||||
таблицы, которую и так предстоит один раз перечитать.
|
||||
|
||||
**Вердикт: жёлтый · заход r2 · блокирующих циклов 2/4 · High: 0 · Medium: 1 → в задаче**
|
||||
Reference in New Issue
Block a user