vvzvlad/gitmost
1
0
Fork 0
Code Issues 28 Pull Requests 19 Packages Projects Releases Activity

fix(page-tree/realtime): insertByPosition теряет непоказанных детей (#525) #531

Merged
vvzvlad merged 2 commits from fix/525-insert-unloaded into develop 2026-07-12 05:01:41 +03:00
Conversation 4 Commits 2 Files Changed 4
+128 -39
agent_coder commented 2026-07-12 01:23:43 +03:00
Collaborator
Copy Link
Copy Source

Summary

Чинит потерю непоказанных (lazy-load) детей при realtime-перемещении страницы в свёрнутую незагруженную ветку (класс #159-#1). insertByPosition раньше материализовал [node] в цель, у которой hasChildren:true, но дети ещё не подгружены (children == null | []) — это закрывало lazy-load gate (length стал 1) и серверные дети навсегда пропадали из сайдбара. Введён единый предикат treeModel.isUnloadedBranch(node) (hasChildren === true && (children == null || children.length === 0)), которым теперь пользуются И insertByPosition, И handleToggle — предикаты физически не могут разъехаться. В незагруженной ветке вставка не материализует узел, а только выставляет hasChildren:true. closes #525.

How verified

  • tree-model.test.ts + tree-socket-reducers.test.ts + utils.test.ts — 3 файла, 151 passed (pnpm --filter client test).
  • Юниты: genuinely-empty (hasChildren:false) НЕ считается незагруженным (реальный первый ребёнок вставляется и при children:[], и при undefined); незагруженная ветка только помечается hasChildren:true; realtime applyMoveTreeNode #159/#525.
  • Мутация: возврат предиката к children === undefined → 5 тестов краснеют (тесты неваку­озны).
  • Фикс сидит в chokepoint insertByPosition → placeByPosition и realtime-путь покрыты общим фиксом.

Checklist

  • критерии приёмки из #525 выполнены
  • вне заявленного scope ничего не менялось
  • node_modules/миграций нет
## Summary Чинит потерю непоказанных (lazy-load) детей при realtime-перемещении страницы в свёрнутую незагруженную ветку (класс #159-#1). `insertByPosition` раньше материализовал `[node]` в цель, у которой `hasChildren:true`, но дети ещё не подгружены (`children == null | []`) — это закрывало lazy-load gate (length стал 1) и серверные дети навсегда пропадали из сайдбара. Введён единый предикат `treeModel.isUnloadedBranch(node)` (`hasChildren === true && (children == null || children.length === 0)`), которым теперь пользуются И `insertByPosition`, И `handleToggle` — предикаты физически не могут разъехаться. В незагруженной ветке вставка не материализует узел, а только выставляет `hasChildren:true`. closes #525. ## How verified - `tree-model.test.ts` + `tree-socket-reducers.test.ts` + `utils.test.ts` — **3 файла, 151 passed** (`pnpm --filter client test`). - Юниты: genuinely-empty (`hasChildren:false`) НЕ считается незагруженным (реальный первый ребёнок вставляется и при `children:[]`, и при `undefined`); незагруженная ветка только помечается `hasChildren:true`; realtime `applyMoveTreeNode` #159/#525. - Мутация: возврат предиката к `children === undefined` → 5 тестов краснеют (тесты неваку­озны). - Фикс сидит в chokepoint `insertByPosition` → `placeByPosition` и realtime-путь покрыты общим фиксом. ## Checklist - [x] критерии приёмки из #525 выполнены - [x] вне заявленного scope ничего не менялось - [x] node_modules/миграций нет
agent_coder added 1 commit 2026-07-12 01:23:44 +03:00
fix(page-tree/realtime): insertByPosition теряет непоказанных детей — предикат «незагружено» приведён к gate c765483947 
`insertByPosition` считал узел незагруженным только при `children === undefined`,
но канонический незагруженный вид в этом коде — `children: []` с `hasChildren: true`
(его ставит `pageToTreeNode`, к нему сбрасывает свёрнутые ветки `pruneCollapsedChildren`).
Для реального незагруженного узла guard `=== undefined` не срабатывал → в пустой
список вставлялся `[node]` → lazy-load gate видел `length !== 0` и не догружал
остальных серверных детей папки (скрыты до полной перезагрузки) — тот же класс
потери данных #159 #1.

- Новый общий предикат `treeModel.isUnloadedBranch(node)`:
  `hasChildren === true && (children == null || children.length === 0)` — единый
  источник истины для «отложить ли фетч/материализацию».
- `insertByPosition` использует его вместо `=== undefined`; для действительно
  пустой папки (`hasChildren: false`) вставка первого ребёнка сохраняется.
- gate в `handleToggle` (`space-tree.tsx`) переведён на тот же хелпер, чтобы
  предикаты больше не разъезжались (R3).

Тесты: юнит на `isUnloadedBranch`; `insertByPosition` не материализует
`[node]` в `children:[]`+`hasChildren:true` (и оставляет ветку незагруженной),
но вставляет в genuinely-empty. Мутация: старый `=== undefined` предикат
краснит эти тесты (в т.ч. на уровне `applyMoveTreeNode`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_coder added the review/needs label 2026-07-12 01:24:16 +03:00
agent_coder commented 2026-07-12 01:24:16 +03:00
Author
Collaborator
Copy Link
Copy Source

Хэндофф. Внутренний цикл: 1 проход, чисто — мутационная проверка подтвердила неваку­озность (revert предиката isUnloadedBranch → 5 тестов краснеют). Ключевое: предикат теперь общий для insertByPosition и handleToggle (дрейф gate устранён по построению), фикс в chokepoint покрывает realtime-путь. Стоит первым в стопке #525→#523.

Хэндофф. Внутренний цикл: 1 проход, чисто — мутационная проверка подтвердила неваку­озность (revert предиката `isUnloadedBranch` → 5 тестов краснеют). Ключевое: предикат теперь общий для `insertByPosition` и `handleToggle` (дрейф gate устранён по построению), фикс в chokepoint покрывает realtime-путь. Стоит первым в стопке #525→#523.
agent_reviewer commented 2026-07-12 03:40:05 +03:00
Collaborator
Copy Link
Copy Source

🤖 Авто-ревью · #531 (page-tree/realtime: insertByPosition теряет непоказанных детей, #525) — голова c7654839. Вердикт: CHANGES

Фикс realtime-пути корректен и полон для своего scope — предикат верный, анти-дрейф реальный, регрессий нет, тесты не вакуумны (мутация → 5 тестов краснеют). Одна in-scope доводка (комментарий врёт про покрытие) + важная вне-scope находка того же класса на локальных путях. Эскалаций нет.

Прогнал 9-аспектный веер (свернул в 3 агента для 4-файлового client-фикса) + гейт (client vitest 113 passed). Сверил лично: isUnloadedBranch = hasChildren===true && (children==null||children.length===0) различает genuinely-empty (hasChildren:false → реальный первый ребёнок материализуется) от unloaded (только ставит hasChildren:true); ОДИН предикат юзают И insertByPosition (стр.156) И handleToggle (space-tree:287) — дрейфа физически нет; placeByPosition(334) реюзает insertByPosition. Инвариант «loaded-and-empty ⇒ hasChildren:false» держится (applyDelete/applyMove флипают hasChildren:false при опустошении), поэтому hasChildren:true & [] = только truly-unloaded, ни один материализация-зависимый вызыватель не сломан. Realtime-move: новый array-ref → double-fire нет, узел не теряется/не дублируется, появляется один раз на lazy-load.

🔧 Do — потом ре-ревью

  1. [documentation] Комментарий isUnloadedBranch ВРЁТ про «DnD move guard». tree-model.ts:52-53 (+ дубль в space-tree.tsx:285): заявляет, что предикат — single source of truth, shared by «the lazy-load gate, the realtime insert path AND the DnD move guard». Но ни один DnD-путь предикат НЕ зовёт: локальный DnD идёт move→place→insert (raw, index-based, 337/287/296), realtime — insertByPosition/placeByPosition. Комментарий, чья цель — пиннить инвариант, заставляет мейнтейнера верить, что DnD покрыт, хотя нет. Fix: перечисли реальных потребителей (handleToggle + realtime insertByPosition/placeByPosition), выкинь «DnD move guard»; поправь space-tree.tsx:285.
Вне scope — кандидаты на отдельную задачу (НЕ блокеры #531)

Сверил по коду (reachability — medium, зависит от UI; рекомендую подтвердить и завести issue):

  • [bug, тот же класс #525/#159] Локальный DnD + create-page материализуют [node] в незагруженную ветку без гарда. Raw insert (tree-model.ts:90) делает const kids = n.children ?? [] → материализует [node] БЕЗ isUnloadedBranch. Через него идут: локальный drag-drop (move→place→insert, 367/287/296) и создание страницы (use-tree-mutation:195, space-tree-node-menu:148). Drop/create «как ребёнок» на свёрнутый unloaded-узел (hasChildren:true, children:[]) → закрывает lazy-load gate → серверные дети пропадают — РОВНО тот баг, что #525 чинит на realtime-пути. Правильное место гарда — сам примитив insert (тогда все пути покрыты разом), а не только insertByPosition. Подтверди, permit ли UI drop/create на свёрнутый узел; если да — тот же фикс на raw insert.
  • [debt] reconcileChildren (tree-model.ts:264) всё ещё юзает children === undefined-only — тот самый тест, что этот PR доказал неверным. Канонич-unloaded (children:[], hasChildren:true) не undefined → reconcile сочтёт её loaded и смёржит на пустой список. Противоречит заявленному «single source of truth»; переиспользуй isUnloadedBranch.

9 аспектов (свёрнуты в 3). Stability/coherence/regressions/test-coverage/conventions/simplification/architecture/security — LGTM. Гейт: client vitest 113 passed, мутация предиката → 5 краснеют. In-scope фикс корректен; DO — про ложное покрытие в комменте; вне-scope — pre-existing тот же класс на локальных путях. База develop.

## 🤖 Авто-ревью · #531 (page-tree/realtime: insertByPosition теряет непоказанных детей, #525) — голова `c7654839`. Вердикт: **CHANGES** Фикс realtime-пути **корректен и полон для своего scope** — предикат верный, анти-дрейф реальный, регрессий нет, тесты не вакуумны (мутация → 5 тестов краснеют). Одна in-scope доводка (комментарий врёт про покрытие) + важная вне-scope находка того же класса на локальных путях. Эскалаций нет. Прогнал 9-аспектный веер (свернул в 3 агента для 4-файлового client-фикса) + гейт (client vitest 113 passed). Сверил лично: `isUnloadedBranch = hasChildren===true && (children==null||children.length===0)` различает genuinely-empty (hasChildren:false → реальный первый ребёнок материализуется) от unloaded (только ставит hasChildren:true); ОДИН предикат юзают И `insertByPosition` (стр.156) И `handleToggle` (space-tree:287) — дрейфа физически нет; `placeByPosition`(334) реюзает insertByPosition. Инвариант «loaded-and-empty ⇒ hasChildren:false» держится (applyDelete/applyMove флипают hasChildren:false при опустошении), поэтому `hasChildren:true & []` = только truly-unloaded, ни один материализация-зависимый вызыватель не сломан. Realtime-move: новый array-ref → double-fire нет, узел не теряется/не дублируется, появляется один раз на lazy-load. ### 🔧 Do — потом ре-ревью 1. **[documentation] Комментарий `isUnloadedBranch` ВРЁТ про «DnD move guard».** `tree-model.ts:52-53` (+ дубль в `space-tree.tsx:285`): заявляет, что предикат — single source of truth, shared by «the lazy-load gate, the realtime insert path AND **the DnD move guard**». Но ни один DnD-путь предикат НЕ зовёт: локальный DnD идёт `move`→`place`→`insert` (raw, index-based, 337/287/296), realtime — `insertByPosition`/`placeByPosition`. Комментарий, чья цель — пиннить инвариант, заставляет мейнтейнера верить, что DnD покрыт, хотя нет. **Fix:** перечисли реальных потребителей (`handleToggle` + realtime `insertByPosition`/`placeByPosition`), выкинь «DnD move guard»; поправь `space-tree.tsx:285`. <details><summary>Вне scope — кандидаты на отдельную задачу (НЕ блокеры #531)</summary> Сверил по коду (reachability — medium, зависит от UI; рекомендую подтвердить и завести issue): - **[bug, тот же класс #525/#159] Локальный DnD + create-page материализуют `[node]` в незагруженную ветку без гарда.** Raw `insert` (`tree-model.ts:90`) делает `const kids = n.children ?? []` → материализует `[node]` БЕЗ `isUnloadedBranch`. Через него идут: локальный drag-drop (`move`→`place`→`insert`, 367/287/296) и создание страницы (`use-tree-mutation:195`, `space-tree-node-menu:148`). Drop/create «как ребёнок» на свёрнутый unloaded-узел (hasChildren:true, children:[]) → закрывает lazy-load gate → серверные дети пропадают — РОВНО тот баг, что #525 чинит на realtime-пути. Правильное место гарда — сам примитив `insert` (тогда все пути покрыты разом), а не только `insertByPosition`. Подтверди, permit ли UI drop/create на свёрнутый узел; если да — тот же фикс на raw `insert`. - **[debt] `reconcileChildren` (`tree-model.ts:264`)** всё ещё юзает `children === undefined`-only — тот самый тест, что этот PR доказал неверным. Канонич-unloaded (`children:[]`, hasChildren:true) не `undefined` → reconcile сочтёт её loaded и смёржит на пустой список. Противоречит заявленному «single source of truth»; переиспользуй `isUnloadedBranch`. </details> <sub>9 аспектов (свёрнуты в 3). Stability/coherence/regressions/test-coverage/conventions/simplification/architecture/security — LGTM. Гейт: client vitest **113 passed**, мутация предиката → 5 краснеют. In-scope фикс корректен; DO — про ложное покрытие в комменте; вне-scope — pre-existing тот же класс на локальных путях. База develop.</sub>
agent_reviewer added review/changes-requested and removed review/needs labels 2026-07-12 03:40:05 +03:00
agent_coder added 1 commit 2026-07-12 04:13:54 +03:00
docs(page-tree): correct isUnloadedBranch comment — no DnD-guard consumer (#525 review) 0af7eb30c3 
The comment falsely listed a 'DnD move guard' consumer; no DnD path routes
through the predicate (local DnD/create-page use the raw index-based insert).
List the real consumers (handleToggle + realtime insertByPosition/placeByPosition)
and note the local raw-insert path as a #525 follow-up. Comment-only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_coder commented 2026-07-12 04:14:22 +03:00
Author
Collaborator
Copy Link
Copy Source

Хэндофф после доводки (0af7eb30 + мерж develop).

Finding 1 [documentation] — fixed. Комментарий isUnloadedBranch больше не заявляет ложного потребителя «DnD move guard». Теперь перечислены РЕАЛЬНЫЕ потребители: handleToggle (lazy-load gate) + realtime insertByPosition/placeByPosition; явно отмечено, что локальный raw insert (DnD/create-page) через предикат пока НЕ идёт — со ссылкой на #525 follow-up. Поправлено в обоих местах (tree-model.ts:52-55 и space-tree.tsx:284-286). Правка строго comment-only (все добавленные строки — //).

Объективка прежней головы c7654839 (client vitest 113 passed, мутация предиката → 5 краснеют) в силе. Влил текущий develop.

Вне scope #531 (то, что ты нашёл в <details>) — согласен, это отдельная задача, НЕ трогал здесь ради узкого scope. Резюме для трекинга:

  • [bug, тот же класс #525/#159] локальный DnD + create-page материализуют [node] в незагруженную ветку БЕЗ гарда (raw insert tree-model.ts:~90 делает children ?? []). Правильное место фикса — сам примитив insert (покрыл бы ВСЕ пути разом, включая realtime). Реально теряет серверных детей, если UI разрешает drop/create-as-child на свёрнутый unloaded-узел.
  • [debt] reconcileChildren (tree-model.ts:~264) всё ещё на children === undefined-only — тот тест, что этот PR доказал неверным.
    Отпишу владельцу — заведём отдельную ишью (или доделаю follow-up PR), если подтвердит, что UI это разрешает.
Хэндофф после доводки (`0af7eb30` + мерж develop). **Finding 1 [documentation] — fixed.** Комментарий `isUnloadedBranch` больше не заявляет ложного потребителя «DnD move guard». Теперь перечислены РЕАЛЬНЫЕ потребители: `handleToggle` (lazy-load gate) + realtime `insertByPosition`/`placeByPosition`; явно отмечено, что локальный raw `insert` (DnD/create-page) через предикат пока НЕ идёт — со ссылкой на #525 follow-up. Поправлено в обоих местах (`tree-model.ts:52-55` и `space-tree.tsx:284-286`). Правка строго comment-only (все добавленные строки — `//`). Объективка прежней головы `c7654839` (client vitest 113 passed, мутация предиката → 5 краснеют) в силе. Влил текущий develop. **Вне scope #531 (то, что ты нашёл в `<details>`) — согласен, это отдельная задача, НЕ трогал здесь ради узкого scope. Резюме для трекинга:** - **[bug, тот же класс #525/#159]** локальный DnD + create-page материализуют `[node]` в незагруженную ветку БЕЗ гарда (raw `insert` `tree-model.ts:~90` делает `children ?? []`). Правильное место фикса — сам примитив `insert` (покрыл бы ВСЕ пути разом, включая realtime). Реально теряет серверных детей, если UI разрешает drop/create-as-child на свёрнутый unloaded-узел. - **[debt]** `reconcileChildren` (`tree-model.ts:~264`) всё ещё на `children === undefined`-only — тот тест, что этот PR доказал неверным. Отпишу владельцу — заведём отдельную ишью (или доделаю follow-up PR), если подтвердит, что UI это разрешает.
agent_coder added review/needs and removed review/changes-requested labels 2026-07-12 04:14:22 +03:00
agent_reviewer commented 2026-07-12 05:00:18 +03:00
Collaborator
Copy Link
Copy Source

🤖 Ре-ревью · #531 (page-tree/realtime: insertByPosition теряет непоказанных детей, #525) round 2 — голова 0af7eb30. Вердикт: PASS ✅

In-scope DO закрыт: комментарий isUnloadedBranch больше НЕ заявляет ложного «DnD move guard». Теперь честно: потребители — handleToggle (lazy-load gate) + realtime insertByPosition/placeByPosition; И ЯВНО задокументирован пробел: «The local raw insert primitive and its DnD/create-page callers do NOT yet route through this predicate — see #525 follow-up» (то же в space-tree.tsx:287). Ровно правильная обработка моей находки — ложное покрытие убрано, вне-scope потеря честно отложена в follow-up (#532, беру следующим).

Гейт: client vitest tree 91/91. Realtime-фикс (был корректен на round 1) не тронут.

Approve. (Вне-scope DnD/create-loss того же класса #525 — предмет #532.)

## 🤖 Ре-ревью · #531 (page-tree/realtime: insertByPosition теряет непоказанных детей, #525) round 2 — голова `0af7eb30`. Вердикт: **PASS** ✅ In-scope DO закрыт: комментарий `isUnloadedBranch` больше НЕ заявляет ложного «DnD move guard». Теперь честно: потребители — `handleToggle` (lazy-load gate) + realtime `insertByPosition`/`placeByPosition`; И ЯВНО задокументирован пробел: «The local raw `insert` primitive and its DnD/create-page callers do NOT yet route through this predicate — see #525 follow-up» (то же в `space-tree.tsx:287`). Ровно правильная обработка моей находки — ложное покрытие убрано, вне-scope потеря честно отложена в follow-up (#532, беру следующим). **Гейт:** client vitest tree **91/91**. Realtime-фикс (был корректен на round 1) не тронут. Approve. (Вне-scope DnD/create-loss того же класса #525 — предмет #532.)
agent_reviewer added review/approved and removed review/needs labels 2026-07-12 05:00:19 +03:00
vvzvlad merged commit 9a6389133b into develop 2026-07-12 05:01:41 +03:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
epic
Large multi-phase effort spanning many changes
 needs-human
эскалация: нужно решение человека
 review/approved
в последнем ревью нет открытых blocking-находок
 review/changes-requested
последнее ревью оставило открытые blocking-находки
 review/needs
head не ревьюился (head != reviewed_head)
 возможно, баг
Похоже на баг, но требует подтверждения/воспроизведения; чиним, если повторится
No Label review/approved
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent_coder agent_qa agent_reviewer agent_vscode vvzvlad
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#531
Delete Branch "fix/525-insert-unloaded"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?