Compare commits

...

2 Commits

Author SHA1 Message Date
agent_coder c765483947 fix(page-tree/realtime): insertByPosition теряет непоказанных детей — предикат «незагружено» приведён к gate
`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>
2026-07-12 01:06:42 +03:00
vvzvlad bfb6a52eea Merge pull request 'epic #497 волна 1 — интеграция четырёх approved-итераций (#486/#487/#488/#489) → develop' (#511) from integ/497-wave1 into develop
Reviewed-on: #511
2026-07-11 19:29:27 +03:00
4 changed files with 124 additions and 39 deletions
@@ -281,10 +281,10 @@ const SpaceTree = forwardRef<SpaceTreeApi, SpaceTreeProps>(function SpaceTree(
setOpenTreeNodes((prev) => ({ ...prev, [id]: isOpen }));
if (isOpen) {
const node = treeModel.find(data, id) as SpaceTreeNode | null;
if (
node?.hasChildren &&
(!node.children || node.children.length === 0)
) {
// Same "unloaded branch" predicate the insert paths use (`isUnloadedBranch`)
// so the lazy-load gate and the realtime/DnD inserts can never disagree
// about what counts as unloaded (#525).
if (treeModel.isUnloadedBranch(node)) {
const fetched = await fetchAllAncestorChildren({
pageId: id,
spaceId: node.spaceId,
@@ -74,6 +74,48 @@ describe("treeModel.isDescendant", () => {
});
});
// #525: the single "is this branch unloaded?" predicate shared by the lazy-load
// gate and the insert paths. Unloaded == server says hasChildren but none are
// present locally (canonical form `children: []`, also `undefined`). A parent
// without hasChildren is genuinely empty, not unloaded.
describe("treeModel.isUnloadedBranch", () => {
type PH = TreeNode<{ name: string; hasChildren?: boolean }>;
it("true for hasChildren + empty array (canonical unloaded form)", () => {
const n: PH = { id: "p", name: "P", hasChildren: true, children: [] };
expect(treeModel.isUnloadedBranch(n)).toBe(true);
});
it("true for hasChildren + undefined children", () => {
const n: PH = { id: "p", name: "P", hasChildren: true };
expect(treeModel.isUnloadedBranch(n)).toBe(true);
});
it("false for hasChildren + already-loaded children", () => {
const n: PH = {
id: "p",
name: "P",
hasChildren: true,
children: [{ id: "c", name: "C" }],
};
expect(treeModel.isUnloadedBranch(n)).toBe(false);
});
it("false for a genuinely-empty parent (no hasChildren)", () => {
expect(
treeModel.isUnloadedBranch({
id: "p",
name: "P",
hasChildren: false,
children: [],
} as PH),
).toBe(false);
expect(
treeModel.isUnloadedBranch({ id: "p", name: "P" } as PH),
).toBe(false);
});
it("false for null/undefined", () => {
expect(treeModel.isUnloadedBranch(null)).toBe(false);
expect(treeModel.isUnloadedBranch(undefined)).toBe(false);
});
});
describe("treeModel.visible", () => {
it("returns only root nodes when no openIds", () => {
const v = treeModel.visible(fixture, new Set());
@@ -197,43 +239,64 @@ describe("treeModel.insertByPosition", () => {
]);
});
// #159 #1: inserting/moving a node under a parent whose children are NOT
// loaded (`children === undefined`, e.g. a collapsed page) must NOT materialize
// a partial `[node]` list — that would defeat the lazy-load gate and hide the
// parent's other real children. The node is left to be lazy-loaded; only
// `hasChildren` is flagged so the chevron appears.
it("does NOT materialize a child under an UNLOADED parent (children undefined)", () => {
type PH = TreeNode<{
name: string;
position?: string;
hasChildren?: boolean;
}>;
type PH = TreeNode<{
name: string;
position?: string;
hasChildren?: boolean;
}>;
// #159 #1 / #525: inserting/moving a node under an UNLOADED parent must NOT
// materialize a partial `[node]` list — that would defeat the lazy-load gate and
// hide the parent's other real children. The canonical unloaded form here is
// `children: []` + `hasChildren: true` (from `pageToTreeNode` /
// `pruneCollapsedChildren`), which the pre-#525 `=== undefined` guard MISSED.
// The node is left to be lazy-loaded; the chevron stays enabled.
it("does NOT materialize a child under an UNLOADED parent (children: [], hasChildren: true)", () => {
const tree: PH[] = [
{ id: "p", name: "P", position: "a0", hasChildren: false }, // children: undefined
{ id: "p", name: "P", position: "a0", hasChildren: true, children: [] },
];
const node: PH = { id: "x", name: "X", position: "a1" };
const t = treeModel.insertByPosition(tree, "p", node);
const parent = treeModel.find(t, "p");
// The node was NOT inserted (children stay unloaded -> lazy-load fetches the
// full set, including this node, on expand).
expect(parent?.children).toBeUndefined();
// full set, including this node, on expand). MUTATION: the pre-#525 predicate
// `children === undefined` does not fire for `[]`, so it would insert `[x]`
// here and reredden this expectation.
expect(parent?.children).toEqual([]);
expect(treeModel.find(t, "x")).toBeNull();
// ...but the chevron is enabled so the user can expand to load it.
// ...and the chevron stays enabled so the user can expand to load it.
expect((parent as PH).hasChildren).toBe(true);
});
it("DOES insert under a LOADED-but-empty parent (children: [])", () => {
type PH = TreeNode<{
name: string;
position?: string;
hasChildren?: boolean;
}>;
it("does NOT materialize a child under an UNLOADED parent (children undefined, hasChildren: true)", () => {
const tree: PH[] = [
{ id: "p", name: "P", position: "a0", hasChildren: true }, // children: undefined
];
const node: PH = { id: "x", name: "X", position: "a1" };
const t = treeModel.insertByPosition(tree, "p", node);
const parent = treeModel.find(t, "p");
expect(parent?.children).toBeUndefined();
expect(treeModel.find(t, "x")).toBeNull();
expect((parent as PH).hasChildren).toBe(true);
});
it("DOES insert under a genuinely-empty parent (children: [], hasChildren: false)", () => {
const tree: PH[] = [
{ id: "p", name: "P", position: "a0", hasChildren: false, children: [] },
];
const node: PH = { id: "x", name: "X", position: "a1" };
const t = treeModel.insertByPosition(tree, "p", node);
// A loaded (empty) child list is complete, so the node IS inserted.
// No server children (`hasChildren: false`), so materializing the first child
// is correct — nothing is hidden.
expect(treeModel.find(t, "p")?.children?.map((n) => n.id)).toEqual(["x"]);
});
it("DOES insert under a genuinely-empty parent (children undefined, hasChildren: false)", () => {
const tree: PH[] = [
{ id: "p", name: "P", position: "a0", hasChildren: false }, // children: undefined
];
const node: PH = { id: "x", name: "X", position: "a1" };
const t = treeModel.insertByPosition(tree, "p", node);
expect(treeModel.find(t, "p")?.children?.map((n) => n.id)).toEqual(["x"]);
});
@@ -43,6 +43,24 @@ export const treeModel = {
};
},
// A branch is "unloaded" when the server says it HAS children (`hasChildren`)
// but none are present locally. The canonical unloaded form in this codebase
// is `children: []` (produced by `pageToTreeNode` and by `pruneCollapsedChildren`
// resetting collapsed branches), NOT `children: undefined` — so a predicate that
// only checks `=== undefined` misses the real case and materializes a misleading
// partial list (#525). This is the SINGLE source of truth for "should a
// fetch/materialize be deferred?", shared by the lazy-load gate (`handleToggle`),
// the realtime insert path (`insertByPosition`) and the DnD move guard, so they
// can never drift apart again. A parent WITHOUT `hasChildren` is genuinely empty
// (no server children) — inserting its first child is correct, not deferred.
isUnloadedBranch<T extends object>(
node: TreeNode<T> | null | undefined,
): boolean {
if (!node) return false;
const hasChildren = (node as { hasChildren?: boolean }).hasChildren === true;
return hasChildren && (node.children == null || node.children.length === 0);
},
isDescendant<T extends object>(
tree: TreeNode<T>[],
ancestorId: string,
@@ -127,14 +145,15 @@ export const treeModel = {
}
const parent = treeModel.find(tree, parentId);
// The parent is in the tree but its children have NOT been lazy-loaded yet
// (`children === undefined`, distinct from a loaded-but-empty `[]`). Inserting
// (`hasChildren` set + children absent/empty — see `isUnloadedBranch`; the
// canonical unloaded form is `children: []`, NOT just `undefined`). Inserting
// here would MATERIALIZE a misleading partial child list (`[node]`) that
// defeats the lazy-load gate — which fetches only when children are
// absent/empty — so the parent's OTHER real children would never load and the
// moved/added node would be the only one shown (a silent data loss, #159 #1).
// Instead, leave the children unloaded and just flag `hasChildren` so the
// chevron appears; expanding fetches the FULL set (including this node).
if (parent && parent.children === undefined) {
if (parent && treeModel.isUnloadedBranch(parent)) {
return treeModel.update(
tree,
parentId,
@@ -82,17 +82,19 @@ describe("applyMoveTreeNode", () => {
]);
});
it("does NOT create a partial child list when the destination is loaded-but-collapsed (children unloaded) — keeps it lazy-loadable (#159)", () => {
// `dstCollapsed` is in the tree but its children were never lazy-loaded
// (children === undefined). The OLD behavior inserted `src` as the ONLY
// child ([src]), which defeated the lazy-load gate and HID the parent's
// other real children. Now the move leaves children unloaded (so expanding
// fetches the FULL set, including src) and just flags hasChildren.
it("does NOT create a partial child list when the destination is loaded-but-collapsed (children unloaded) — keeps it lazy-loadable (#159 #525)", () => {
// `dstCollapsed` is in the tree but its children were never lazy-loaded. The
// CANONICAL unloaded form here is `hasChildren: true` + `children: []` (from
// `pageToTreeNode` / `pruneCollapsedChildren`), NOT `children: undefined`.
// The pre-#525 predicate (`children === undefined`) missed this form and
// inserted `src` as the ONLY child ([src]), defeating the lazy-load gate and
// HIDING the parent's other real children. Now the move leaves children
// unloaded (so expanding fetches the FULL set, including src).
const tree: SpaceTreeNode[] = [
node("dstCollapsed", {
position: "a0",
hasChildren: false,
children: undefined as unknown as SpaceTreeNode[],
hasChildren: true,
children: [],
}),
node("src", { position: "a9" }),
];
@@ -105,9 +107,10 @@ describe("applyMoveTreeNode", () => {
pageData: {},
});
const dst = treeModel.find(next, "dstCollapsed");
// Children stay unloaded -> the lazy-load gate fetches the FULL set (incl.
// src) on expand, rather than showing a misleading partial [src] list.
expect(dst?.children).toBeUndefined();
// Children stay unloaded ([] not materialized to [src]) -> the lazy-load gate
// fetches the FULL set (incl. src) on expand. MUTATION: the pre-#525
// `=== undefined` predicate would insert [src] here and redden this.
expect(dst?.children).toEqual([]);
expect(dst?.hasChildren).toBe(true);
// src moved away from its old root slot (it lives under dstCollapsed
// server-side and reappears when the parent is expanded/loaded).