mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 13:18:58 +00:00
docs(reviews): independent code review of pipeline snapshot (#765)
Manual review requested by the owner after the automated reviewer failed. Material af09d36d: green, no High/Medium findings, one non-blocking Low. Issue: #765 User-Visible: no
This commit is contained in:
@@ -0,0 +1,101 @@
|
||||
# CODE-REVIEW-765-r1
|
||||
|
||||
Вердикт: зелёный · заход r1 · блокирующих циклов 0/2 · High: 0 · Medium: 0 · Low: 1
|
||||
|
||||
## Материал и скоуп
|
||||
|
||||
- Issue: [#765](https://github.com/Matysh/houseplan-card/issues/765), `infra`, `track:show`.
|
||||
- Материал: `af09d36dad666e09c8ae5fe59f4be5d982683a6f`, ветка `issue/765-pipeline-tools-snapshot`.
|
||||
- База: `d0c13bc5555aedb7a19221398c2ba8d14081b60b`; полный разбор одного коммита и всех 10 изменённых файлов, не повторный раунд по дельте.
|
||||
- Ревью выполнено Codex независимо от автора реализации, по прямому поручению владельца заменить недоступную модель. [Неудавшийся Process](https://github.com/Matysh/houseplan-card/actions/runs/36975993233) не опубликовал полноценного результата и цикла ревью не образовал.
|
||||
- Контракт — тело issue и [аналитика](https://github.com/Matysh/houseplan-card/issues/765#issuecomment-5934579146), с сопоставлением [хендоффа](https://github.com/Matysh/houseplan-card/issues/765#issuecomment-5946980600) с кодом. Инфраструктура не требует отдельного продуктового ТЗ.
|
||||
|
||||
Проверены `_process.yml`, `review-doc-guard.mjs`, новые и изменённые тесты, три записи mutation registry и изменение PROCESS.md. Задача не меняет работу пользователя карточки: она защищает достоверность допуска материала к ревью и учёт расхода модели. Классы B/C, `Issue: #765` и `User-Visible: no` в терминальном блоке коммита корректны; changelog не требуется.
|
||||
|
||||
Перед разбором просмотрены строки подсистемы в `docs/reviews/INDEX.md` и предыдущие документы #749/#737/#751. Их зелёные вердикты не использованы вместо проверки этого диффа.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Независимый локальный прогон: свежий изолированный клон в ext4 WSL Ubuntu, Node 22.23.2, git 2.53.0, точный SHA материала. После установки зависимостей все перечисленные ниже тесты исполнены, включая bash/git-фикстуры, которые нельзя зачесть по пропускам Windows.
|
||||
|
||||
| Проверка | Результат |
|
||||
|---|---|
|
||||
| Typecheck, build, unit, bundle verification, private writes/any, unused code | Приняты по [Validate 36975770091](https://github.com/Matysh/houseplan-card/actions/runs/36975770091) на точном `af09d36d`: соответствующие шаги frontend job реально `success`, не `skipped` |
|
||||
| `node --test test/process-prepare-tools.test.mjs test/model-usage.test.mjs test/process-resume.test.mjs test/process-track.test.mjs test/rebase-generated.test.mjs test/review-doc-guard.test.mjs test/process-integrate-tools.test.mjs test/process-digests.test.mjs` | 161/161, пропусков 0 |
|
||||
| `node --test test/validate-gate.test.mjs test/ci-proof.test.mjs test/workflow-jobs.test.mjs test/publish-push-refusal.test.mjs test/review-result-gate.test.mjs` | 94/94 |
|
||||
| Независимый архив `HEAD scripts .github/workflows/validate.yml` вне репозитория; импорт шести скриптов из пустого cwd без `node_modules` | `process-track`, `rebase-generated`, `merge-candidate`, `review-doc-guard`, `validate-gate`, `model-usage` успешно загружаются со всеми транзитивными импортами |
|
||||
| `node scripts/mutation-gate.mjs --check` | Exit 0; все три новых якоря сходятся. Общие предупреждения реестра: 4; browser guards 216 при ориентире 200. Мутанты НЕ применялись |
|
||||
| `node scripts/smoke-select.mjs --base d0c13bc5555aedb7a19221398c2ba8d14081b60b --head af09d36dad666e09c8ae5fe59f4be5d982683a6f` | 10 файлов; исполняемого frontend-диффа нет, browser-smoke не выбираются. Решение: браузер не запускать |
|
||||
|
||||
Первый локальный запуск без установленных зависимостей не загрузил `process-track.test.mjs` из-за отсутствующего `typescript`. После `npm ci --ignore-scripts --no-audit --no-fund` весь набор перезапущен успешно. Это ошибка подготовки свежего клона, не дефект ветки. Продакшен-снимку `node_modules` не нужен — это проверено отдельно, а не выведено из наличия зависимостей у тестов.
|
||||
|
||||
## Контракт и отрицательные свидетели
|
||||
|
||||
Обозначения AC1–AC3 ниже соответствуют группам тестов автора; четвертая строка — отдельный косметический пункт тела issue.
|
||||
|
||||
| AC / требование | Чем доказано | Чем краснеет |
|
||||
|---|---|---|
|
||||
| AC1: единый снимок prepare, до перехода на материал, закреплённый SHA; все repo-скрипты и их импорты из снимка | Два теста `#765 AC1`, bash/git execution AC2; дополнительный импорт из автономного архива. Workflow прочитан построчно: один `git rev-parse origin/dev`, `set -euo pipefail`, `TOOLS` из outputs, отсутствие смены cwd на tools | Контрактные проверки отвергают вызов из `scripts/` рабочей копии, отдельное извлечение в шаге, отсутствие validate.yml или передачу другого источника tools; `prepare-reuse-runs-material-script` — зарегистрированный отрицательный свидетель для исполняемого сценария |
|
||||
| AC2: материал не может своим `review-doc-guard` подменить хеш тела, reuse или признак изменения ТЗ | `#765 AC2`: настоящие шаги bash на ветке с подменённым модулем; ожидаются правильный digest, SHA материала, `reuse=false`, `changed=false` | Фикстура подмены печатает `reuse=true` и неверный digest; мутант `prepare-reuse-runs-material-script` переводит вызов на неё |
|
||||
| AC3: расход из того же закреплённого SHA, работает на старой ветке без файла; ошибка снимка не выдаётся за успешные данные | `#765 AC3`: dev сдвинут и его новая версия печатает другое, в материале файла нет, результат совпадает с реальным usage; пустой `TOOLS_SHA` даёт ненулевой exit и объяснение. `model-usage.test` исполняет реальный шаг и проверяет отсутствие секретов | `usage-script-from-moving-dev`; отрицательный вход `TOOLS_SHA=''` непосредственно в тесте; отсутствие result/usage и мусор проверяются соседними тестами |
|
||||
| Повторная запись блока якорей не копит разделители | Тест `#765` в `review-doc-guard.test`: обычный документ стабилен после трёх записей; внутренний авторский разделитель сохраняется | `material-anchors-pile-separators`; независимый дополнительный вход с завершающим авторским `---` выявил L1 ниже |
|
||||
|
||||
Применение мутантов не проводилось согласно PROCESS.md §2.7/#709. Реестр и соответствие именам тестов проверены статически; поимку мутантов проверяет ночь. Защита Validate подтверждена чтением вызова через `TOOLS`, автономной загрузкой его импортов и 94 смежными тестами, но не выдаётся за запуск нового workflow на GitHub.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
1. `prepare` берёт SHA до переключения на ветку задачи. Последующие `git fetch` трека и ребейза не меняют архив. `tools_sha` передаётся в `model_review` через job output; usage не разрешает заново подвижный `origin/dev`.
|
||||
2. Полный каталог `scripts/` переносит и локальные импорты, включая `model-usage`/`spawn-portable`; `validate.yml` лежит по пути, который ожидает `workflow-jobs.mjs`. Скрипты разбирают git и документы в cwd материала, а не в каталоге инструментов. Новый код не переносит вычисление SHA/дерева материала в снимок.
|
||||
3. Ошибка `git archive` в prepare останавливает подготовку (`pipefail`). Для usage отсутствующий SHA явно диагностируется; `continue-on-error` ограничен отчётным шагом. Неполученные данные не превращаются в нулевой расход.
|
||||
4. Контракт безопасности #556 не расширен: модель работает на своём раннере, и снимок не объявлен защитой от её действий на этом же раннере. Строка usage остаётся недоверенной и строго разбирается при публикации. Это явно записано в PROCESS.md.
|
||||
5. `integrate` продолжает использовать свой свежий dev-снимок по #749 — его не переводят на старый SHA подготовки. `_ship-review.yml` не затронут: там checkout — кандидат линии dev. Само изменение конвейера будет активно после слияния, а не при собственном ревью.
|
||||
6. `track:show` остаётся обоснованным: изменения реализуют существующий контракт PROCESS.md §10.4/#749. Нет новых UX-решений, миграции, продуктовой геометрии, touch/device/performance-поверхностей. Маршрут `fix`, оснований для `reclassify` нет.
|
||||
7. «Одно число — один источник»: новых пользовательских чисел нет. SHA инструментов имеет один источник `steps.tools.outputs.sha`; счётчики usage остаются в `model-usage.mjs`. Числа в фикстурах — независимые ожидаемые результаты, не дублирующие конфигурацию продукта.
|
||||
|
||||
## Находки
|
||||
|
||||
High: 0. Medium: 0. Low: 1.
|
||||
|
||||
### L1 — завершающий авторский разделитель теряется при повторной записи якорей
|
||||
|
||||
`scripts/review-doc-guard.mjs:400`: регулярное выражение `/(?:\s*\n---)*\s*$/` снимает всю последовательность горизонтальных разделителей перед маркером, а не только разделитель, добавленный helper-ом.
|
||||
|
||||
Воспроизведение на материале, без изменения исходников:
|
||||
|
||||
```js
|
||||
const text = '# Review\n\nAuthor text\n\n---\n';
|
||||
const anchors = { sha: 'a'.repeat(40), tree: 'b'.repeat(40), branch: 'test', specs: [] };
|
||||
const once = withMaterialAnchors(text, anchors);
|
||||
const twice = withMaterialAnchors(once, anchors);
|
||||
// once: два разделителя; twice: один; once !== twice.
|
||||
```
|
||||
|
||||
Тест автора сохраняет внутренний разделитель, после которого есть текст, но не проверяет разделитель в самом конце авторского документа. Поэтому заявление «разделитель автора остаётся» нуждается в этом уточнении. Возможное улучшение: снимать один служебный разделитель перед известным маркером и добавить этот граничный пример.
|
||||
|
||||
Снимаю L1 с блокировки с записью: это косметический пункт, уже обозначенный Low в исходной задаче; текст отчёта, хеши, verdict/reuse и безопасность исполнения не повреждаются. По треку show он не образует Medium и не расходует цикл. В рамках ревью код не исправлялся.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Новый `_process.yml` в живом GitHub Actions после слияния: текущий зелёный Validate проверяет код ветки, а предыдущий Process использовал тело из dev. Оба критических сценария исполнены локально на извлечённых настоящих bash-шагах, не только regex-проверками.
|
||||
- Полный typecheck/unit/build повторно локально: приняты по точному SHA зелёного Validate, см. таблицу; 255 целевых и смежных тестов запущены независимо.
|
||||
- Golden, браузер, HA harness, инварианты геометрии и performance: нет соответствующего продуктового диффа, `ci:golden`/`ci:full` и таких AC. Смоки отдельно отобраны выше.
|
||||
- Поимка мутантов ночью: только статическая проверка якорей, без применения мутаций.
|
||||
- Слияние в dev и перевод в S8: поручение — ручное код-ревью. Документ публикуется в ветке задачи; продуктовый код, dev и main не изменяются этим ревью.
|
||||
|
||||
## Итог
|
||||
|
||||
Зелёный, `route: fix`. Блокирующих дефектов не найдено. Основные сценарии изоляции скриптов, закрепления версии и сохранения cwd материала проверены; есть одно неблокирующее косметическое замечание L1. Перед итогом `git rev-parse HEAD` и `git ls-remote` подтвердили неизменный `af09d36d`; собственный последующий docs-only коммит не считается новым материалом реализации. Сдвиг `dev` в ходе ревью не подмешивался в материал.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/765-pipeline-tools-snapshot`, коммит `af09d36dad66` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `33644f842ed8d13f799280dea157a495b00257ad`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 33644f842ed8
|
||||
```
|
||||
- Тело issue: `88afd58d385bb60c666387604f60c7f631daa4602143a73d4cfdea1916ac97cf`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
@@ -9,6 +9,7 @@
|
||||
| #780 | [SPEC-REVIEW-780-r1.md](SPEC-REVIEW-780-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 5 | · Medium — в редакторе устройств нет «существующего контекстного лотка» и модели выделения; · Medium — поведение бэкенда на висящую ссылку не определено и ломает сохранение «старо…; · Medium — запрет поднимать бюджеты исполним только ленивой загрузкой, а ТЗ её не требует; · Medium — смещение от грани не определено для стен нулевой толщины и смешанных лент; · Medium — AC17 не проверяем: нет порогов; · Low — D назван «диаметром устройства пространства», а такой величины нет | `validation.py` `scripts/bundle-budget.mjs` `types.ts` `houseplan-card.ts` |
|
||||
| #775 | [CODE-REVIEW-775-r1.md](CODE-REVIEW-775-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #772 | [CODE-REVIEW-772-r1.md](CODE-REVIEW-772-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #765 | [CODE-REVIEW-765-r1.md](CODE-REVIEW-765-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | завершающий авторский разделитель теряется при повторной записи якорей | `scripts/review-doc-guard.mjs` |
|
||||
| #762 | [SPEC-REVIEW-762-r1.md](SPEC-REVIEW-762-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #762 | [CODE-REVIEW-762-r1.md](CODE-REVIEW-762-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #761 | [CODE-REVIEW-761-r1.md](CODE-REVIEW-761-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | заявленная «красная» мутация не воспроизводится в этой среде | `scripts/process-metrics.mjs` |
|
||||
|
||||
Reference in New Issue
Block a user