Merge remote-tracking branch 'gitea/develop' into feat/492-incremental-render

apps/client/src/features/editor/atoms/editor-atoms.ts

@@ -3,11 +3,20 @@ import { atom } from "jotai";
 // import would drag the whole @tiptap/core engine into the eager graph of every
 // shell component that reads one of these atoms.
 import type { Editor } from "@tiptap/core";
+import type { HocuspocusProvider } from "@hocuspocus/provider";
 import { PageEditMode } from "@/features/user/types/user.types.ts";
 import type { DictationUnavailableReason } from "@/features/dictation/dictation-status";
 
 export const pageEditorAtom = atom<Editor | null>(null);
 
+// #370 — the active page's collab provider, published by the page editor so the
+// header menu can emit the "save-version" stateless signal (Cmd+S / button).
+// Null when the page is read-only / collab isn't connected. A typed initial
+// value (rather than an explicit generic) keeps jotai's overload resolution on
+// the writable PrimitiveAtom branch.
+const initialCollabProvider: HocuspocusProvider | null = null;
+export const collabProviderAtom = atom(initialCollabProvider);
+
 export const titleEditorAtom = atom<Editor | null>(null);
 
 export const readOnlyEditorAtom = atom<Editor | null>(null);

apps/client/src/features/editor/page-editor.tsx

@@ -31,11 +31,18 @@ import { useAtom, useAtomValue, useSetAtom } from "jotai";
 import useCollaborationUrl from "@/features/editor/hooks/use-collaboration-url";
 import { currentUserAtom } from "@/features/user/atoms/current-user-atom";
 import {
+  collabProviderAtom,
   currentPageEditModeAtom,
   dictationAvailabilityAtom,
   pageEditorAtom,
   yjsConnectionStatusAtom,
 } from "@/features/editor/atoms/editor-atoms";
+import { notifications } from "@mantine/notifications";
+import {
+  VERSION_SAVED_MESSAGE_TYPE,
+  type VersionSavedMessage,
+  saveVersionPending,
+} from "@/features/page-history/version-messages";
 import { asideStateAtom } from "@/components/layouts/global/hooks/atoms/sidebar-atom";
 import {
   activeCommentIdAtom,
@@ -124,6 +131,7 @@ export default function PageEditor({
 
   const [currentUser] = useAtom(currentUserAtom);
   const [, setEditor] = useAtom(pageEditorAtom);
+  const setCollabProvider = useSetAtom(collabProviderAtom);
   const [, setAsideState] = useAtom(asideStateAtom);
   const [, setActiveCommentId] = useAtom(activeCommentIdAtom);
   const [showCommentPopup, setShowCommentPopup] = useAtom(showCommentPopupAtom);
@@ -181,6 +189,24 @@ export default function PageEditor({
       const onStatelessHandler = ({ payload }: onStatelessParameters) => {
         try {
           const message = JSON.parse(payload);
+          // #370 — a version was saved somewhere; live-refresh the history panel
+          // on every client. Only the client that pressed Save (tracked by the
+          // module-level flag) shows the confirmation toast.
+          if (message?.type === VERSION_SAVED_MESSAGE_TYPE) {
+            const versionMsg = message as VersionSavedMessage;
+            queryClient.invalidateQueries({
+              queryKey: ["page-history-list"],
+            });
+            if (saveVersionPending.current) {
+              saveVersionPending.current = false;
+              notifications.show({
+                message: versionMsg.alreadySaved
+                  ? t("Already saved as the latest version")
+                  : t("Version saved"),
+              });
+            }
+            return;
+          }
           if (message?.type !== "page.updated" || !message.updatedAt) return;
           const pageData = queryClient.getQueryData<IPage>(["pages", slugId]);
           if (pageData) {
@@ -238,12 +264,16 @@ export default function PageEditor({
 
       local.on("synced", onLocalSyncedHandler);
       providersRef.current = { socket, local, remote };
+      // #370 — publish the provider so the header menu can emit save-version.
+      setCollabProvider(remote);
       setProvidersReady(true);
     } else {
+      setCollabProvider(providersRef.current.remote);
       setProvidersReady(true);
     }
     // Only destroy on final unmount
     return () => {
+      setCollabProvider(null);
       providersRef.current?.socket.destroy();
       providersRef.current?.remote.destroy();
       providersRef.current?.local.destroy();

apps/client/src/features/page-history/components/history-item.tsx

@@ -1,4 +1,11 @@
-import { Text, Group, UnstyledButton, Avatar, Tooltip } from "@mantine/core";
+import {
+  Text,
+  Group,
+  UnstyledButton,
+  Avatar,
+  Tooltip,
+  Badge,
+} from "@mantine/core";
 import { CustomAvatar } from "@/components/ui/custom-avatar.tsx";
 import { AgentAvatarStack } from "@/components/ui/agent-avatar-stack.tsx";
 import { formattedDate } from "@/lib/time";
@@ -7,36 +14,59 @@ import clsx from "clsx";
 import { IPageHistory } from "@/features/page-history/types/page.types";
 import { memo, useCallback } from "react";
 import { useSetAtom } from "jotai";
+import { useTranslation } from "react-i18next";
 import { historyAtoms } from "@/features/page-history/atoms/history-atoms.ts";
 
 const MAX_VISIBLE_AVATARS = 5;
 
+/**
+ * #370 — map a snapshot's intentionality tier to its badge. `version: true`
+ * marks the intentional points (manual / agent); autosaves (boundary / idle /
+ * legacy null) are non-versions and get dimmed in the list.
+ */
+type HistoryKindMeta = { labelKey: string; color: string; version: boolean };
+export function historyKindMeta(kind?: string | null): HistoryKindMeta {
+  switch (kind) {
+    case "manual":
+      return { labelKey: "Saved", color: "blue", version: true };
+    case "agent":
+      return { labelKey: "Agent version", color: "violet", version: true };
+    case "boundary":
+      return { labelKey: "Boundary", color: "gray", version: false };
+    default: // "idle" | null | undefined (legacy autosave)
+      return { labelKey: "Autosave", color: "gray", version: false };
+  }
+}
+
 interface HistoryItemProps {
   historyItem: IPageHistory;
-  index: number;
-  onSelect: (id: string, index: number) => void;
-  onHover?: (id: string, index: number) => void;
+  // The previous snapshot for diff/restore is resolved by id from the FULL list
+  // in the parent (resolvePrevSnapshotId), so the item only needs to report its
+  // own id — never a list index (which would be the filtered-view index).
+  onSelect: (id: string) => void;
+  onHover?: (id: string) => void;
   onHoverEnd?: () => void;
   isActive: boolean;
 }
 
 const HistoryItem = memo(function HistoryItem({
   historyItem,
-  index,
   onSelect,
   onHover,
   onHoverEnd,
   isActive,
 }: HistoryItemProps) {
   const setHistoryModalOpen = useSetAtom(historyAtoms);
+  const { t } = useTranslation();
+  const kindMeta = historyKindMeta(historyItem.kind);
 
   const handleClick = useCallback(() => {
-    onSelect(historyItem.id, index);
-  }, [onSelect, historyItem.id, index]);
+    onSelect(historyItem.id);
+  }, [onSelect, historyItem.id]);
 
   const handleMouseEnter = useCallback(() => {
-    onHover?.(historyItem.id, index);
-  }, [onHover, historyItem.id, index]);
+    onHover?.(historyItem.id);
+  }, [onHover, historyItem.id]);
 
   const contributors = historyItem.contributors;
   const hasContributors = contributors && contributors.length > 0;
@@ -49,8 +79,20 @@ const HistoryItem = memo(function HistoryItem({
       onMouseEnter={handleMouseEnter}
       onMouseLeave={onHoverEnd}
       className={clsx(classes.history, { [classes.active]: isActive })}
+      // #370 — dim autosnapshots so intentional versions stand out.
+      style={{ opacity: kindMeta.version ? 1 : 0.55 }}
     >
-      <Text size="sm">{formattedDate(new Date(historyItem.createdAt))}</Text>
+      <Group gap={6} wrap="nowrap" justify="space-between">
+        <Text size="sm">{formattedDate(new Date(historyItem.createdAt))}</Text>
+        <Badge
+          size="xs"
+          radius="sm"
+          variant={kindMeta.version ? "filled" : "light"}
+          color={kindMeta.color}
+        >
+          {t(kindMeta.labelKey)}
+        </Badge>
+      </Group>
 
       <Group gap={6} wrap="nowrap" mt={4}>
         {hasContributors ? (

apps/client/src/features/page-history/components/history-list.tsx

@@ -2,14 +2,16 @@ import {
   usePageHistoryListQuery,
   prefetchPageHistory,
 } from "@/features/page-history/queries/page-history-query";
-import HistoryItem from "@/features/page-history/components/history-item";
+import HistoryItem, {
+  historyKindMeta,
+} from "@/features/page-history/components/history-item";
 import {
   activeHistoryIdAtom,
   activeHistoryPrevIdAtom,
   historyAtoms,
 } from "@/features/page-history/atoms/history-atoms";
 import { useAtom, useSetAtom } from "jotai";
-import { useCallback, useEffect, useMemo, useRef } from "react";
+import { useCallback, useEffect, useMemo, useRef, useState } from "react";
 import {
   Button,
   ScrollArea,
@@ -17,9 +19,12 @@ import {
   Divider,
   Loader,
   Center,
+  Switch,
+  Text,
 } from "@mantine/core";
 import { useTranslation } from "react-i18next";
 import { useHistoryRestore } from "@/features/page-history/hooks";
+import { resolvePrevSnapshotId } from "@/features/page-history/utils/resolve-prev-snapshot";
 
 const PREFETCH_DELAY_MS = 150;
 
@@ -47,6 +52,22 @@ function HistoryList({ pageId }: Props) {
     [pageHistoryData],
   );
 
+  // #370 — "only versions" filter: hide autosnapshots (idle/boundary/legacy
+  // null), keep only intentional points (manual/agent). Filtering is over the
+  // already-loaded pages; the diff/restore still targets the true previous
+  // snapshot, so items carry their index within the FULL list.
+  const [onlyVersions, setOnlyVersions] = useState(false);
+  // Reuse historyKindMeta().version — the SAME predicate the badge (HistoryItem)
+  // uses to mark intentional points — so the "Only versions" filter and the badge
+  // can never drift apart when a future intentional kind is added.
+  const visibleItems = useMemo(
+    () =>
+      onlyVersions
+        ? historyItems.filter((item) => historyKindMeta(item.kind).version)
+        : historyItems,
+    [historyItems, onlyVersions],
+  );
+
   const loadMoreRef = useRef<HTMLDivElement>(null);
   const prefetchTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
 
@@ -60,11 +81,13 @@ function HistoryList({ pageId }: Props) {
   }, []);
 
   const handleHover = useCallback(
-    (historyId: string, index: number) => {
+    (historyId: string) => {
       clearPrefetchTimeout();
       prefetchTimeoutRef.current = setTimeout(() => {
         prefetchPageHistory(historyId);
-        const prevId = historyItems[index + 1]?.id;
+        // The true previous snapshot in the FULL list (not the previous visible
+        // one under the "only versions" filter).
+        const prevId = resolvePrevSnapshotId(historyItems, historyId);
         if (prevId) {
           prefetchPageHistory(prevId);
         }
@@ -78,9 +101,11 @@ function HistoryList({ pageId }: Props) {
   }, [clearPrefetchTimeout]);
 
   const handleSelect = useCallback(
-    (id: string, index: number) => {
+    (id: string) => {
       setActiveHistoryId(id);
-      setActiveHistoryPrevId(historyItems[index + 1]?.id ?? "");
+      // Baseline = true previous snapshot in the FULL list, so the "only
+      // versions" filter never diffs/restores against the wrong item.
+      setActiveHistoryPrevId(resolvePrevSnapshotId(historyItems, id));
     },
     [historyItems, setActiveHistoryId, setActiveHistoryPrevId],
   );
@@ -128,12 +153,27 @@ function HistoryList({ pageId }: Props) {
 
   return (
     <div>
+      <Group px="xs" py={6} justify="flex-end">
+        <Switch
+          size="xs"
+          checked={onlyVersions}
+          onChange={(e) => setOnlyVersions(e.currentTarget.checked)}
+          label={t("Only versions")}
+        />
+      </Group>
+
       <ScrollArea h={620} w="100%" type="scroll" scrollbarSize={5}>
-        {historyItems.map((historyItem, index) => (
+        {onlyVersions && visibleItems.length === 0 && (
+          <Center py="md">
+            <Text size="sm" c="dimmed">
+              {t("No saved versions yet.")}
+            </Text>
+          </Center>
+        )}
+        {visibleItems.map((historyItem) => (
           <HistoryItem
             key={historyItem.id}
             historyItem={historyItem}
-            index={index}
             onSelect={handleSelect}
             onHover={handleHover}
             onHoverEnd={clearPrefetchTimeout}

apps/client/src/features/page-history/types/page.types.ts

@@ -24,6 +24,10 @@ export interface IPageHistory {
   updatedAt: string;
   lastUpdatedBy: IPageHistoryUser;
   contributors?: IPageHistoryUser[];
+  // #370 — intentionality tier: 'manual'/'agent' are versions (intentional
+  // points), 'idle'/'boundary' are autosnapshots; null/undefined = legacy
+  // autosave. Derived server-side, drives the history badge + "versions" filter.
+  kind?: "manual" | "agent" | "idle" | "boundary" | null;
   // Provenance markers copied off the page row when the snapshot was saved.
   // `'agent'` marks a version written by the AI agent; `lastUpdatedAiChatId`
   // (when present) deep-links to the chat that produced the edit.

apps/client/src/features/page-history/utils/resolve-prev-snapshot.test.ts

@@ -0,0 +1,42 @@
+import { describe, it, expect } from "vitest";
+import { resolvePrevSnapshotId } from "./resolve-prev-snapshot";
+
+// #370 F4 — the risky client path: with the "only versions" filter active, diff
+// and restore must still baseline against the TRUE previous snapshot in the FULL
+// list, never the previous VISIBLE version (which would skip the autosnapshots
+// between two versions). These pin that the resolution is by FULL-list order.
+describe("resolvePrevSnapshotId", () => {
+  // Newest-first, as the history list stores it: a version, then two autosaves,
+  // then an older version.
+  const full = [
+    { id: "v2", kind: "manual" },
+    { id: "a2", kind: "idle" },
+    { id: "a1", kind: "boundary" },
+    { id: "v1", kind: "manual" },
+    { id: "a0", kind: null },
+  ];
+
+  it("returns the immediate FULL-list successor, not the previous visible version", () => {
+    // Selecting v2 while filtered to versions-only must baseline against a2 (the
+    // real chronological predecessor), NOT v1 (the previous visible version).
+    expect(resolvePrevSnapshotId(full, "v2")).toBe("a2");
+  });
+
+  it("resolves an autosnapshot's predecessor by full-list order", () => {
+    expect(resolvePrevSnapshotId(full, "a1")).toBe("v1");
+  });
+
+  it("returns '' for the oldest item (no predecessor)", () => {
+    expect(resolvePrevSnapshotId(full, "a0")).toBe("");
+  });
+
+  it("returns '' for an id not in the list", () => {
+    expect(resolvePrevSnapshotId(full, "missing")).toBe("");
+  });
+
+  it("does not depend on a filtered subset — same result whatever is visible", () => {
+    // The helper only ever sees the full list; a filtered view cannot change the
+    // baseline it computes.
+    expect(resolvePrevSnapshotId(full, "v1")).toBe("a0");
+  });
+});

apps/client/src/features/page-history/utils/resolve-prev-snapshot.ts

@@ -0,0 +1,22 @@
+/**
+ * #370 — resolve the TRUE previous snapshot for a history item.
+ *
+ * The history panel can be filtered to "only versions" (manual/agent), but diff
+ * and restore must always compare against the immediately-preceding snapshot in
+ * the FULL, unfiltered list — NOT the previous VISIBLE item. Comparing against
+ * the previous visible version would silently skip the autosnapshots between two
+ * versions and diff/restore the wrong baseline.
+ *
+ * Given the full (newest-first) list and an item id, this returns the id of the
+ * item right after it in the full list (its chronological predecessor), or "" if
+ * it is the oldest / not found. Pure and list-order-preserving so it can be unit
+ * tested without mounting the component.
+ */
+export function resolvePrevSnapshotId(
+  fullItems: ReadonlyArray<{ id: string }>,
+  id: string,
+): string {
+  const index = fullItems.findIndex((item) => item.id === id);
+  if (index === -1) return "";
+  return fullItems[index + 1]?.id ?? "";
+}

apps/client/src/features/page-history/version-messages.ts

@@ -0,0 +1,28 @@
+/**
+ * #370 — page-version stateless wire formats. Kept in one place so the client
+ * emitter (Save hotkey / button) and the client listener (page-editor) agree
+ * with the server (PersistenceExtension) on the message shapes.
+ */
+
+/** Client → server: "save a version now". The server derives the tier
+ * (manual/agent) from the signed connection actor, never from this payload. */
+export const SAVE_VERSION_MESSAGE_TYPE = "save-version";
+
+/** Server → all clients: a version was saved (or promoted / already existed). */
+export const VERSION_SAVED_MESSAGE_TYPE = "version.saved";
+
+export interface VersionSavedMessage {
+  type: typeof VERSION_SAVED_MESSAGE_TYPE;
+  historyId: string;
+  kind: "manual" | "agent";
+  /** True when the latest snapshot was already a manual version (a no-op save). */
+  alreadySaved: boolean;
+}
+
+/**
+ * Cross-component coordination flag so only the client that pressed Save shows
+ * the confirmation toast, while every other client silently refreshes its
+ * history panel on the broadcast. A module-level ref avoids stale-closure
+ * pitfalls in the editor's long-lived stateless handler.
+ */
+export const saveVersionPending = { current: false };

apps/client/src/features/page/components/header/page-header-menu.tsx

@@ -3,6 +3,7 @@ import {
   IconArrowRight,
   IconArrowsHorizontal,
   IconClockHour4,
+  IconDeviceFloppy,
   IconDots,
   IconEye,
   IconEyeOff,
@@ -17,7 +18,7 @@ import {
   IconTrash,
   IconWifiOff,
 } from "@tabler/icons-react";
-import React, { useEffect, useRef, useState } from "react";
+import React, { useCallback, useEffect, useRef, useState } from "react";
 import { useAsideTriggerProps } from "@/hooks/use-toggle-aside.tsx";
 import { useAtom, useAtomValue } from "jotai";
 import { historyAtoms } from "@/features/page-history/atoms/history-atoms.ts";
@@ -39,9 +40,14 @@ import { Trans, useTranslation } from "react-i18next";
 import ExportModal from "@/components/common/export-modal";
 import { convertProseMirrorToMarkdown } from "@docmost/prosemirror-markdown/browser";
 import {
+  collabProviderAtom,
   pageEditorAtom,
   yjsConnectionStatusAtom,
 } from "@/features/editor/atoms/editor-atoms.ts";
+import {
+  SAVE_VERSION_MESSAGE_TYPE,
+  saveVersionPending,
+} from "@/features/page-history/version-messages.ts";
 import { formattedDate } from "@/lib/time.ts";
 import { PageEditModeToggle } from "@/features/user/components/page-state-pref.tsx";
 import MovePageModal from "@/features/page/components/move-page-modal.tsx";
@@ -72,9 +78,34 @@ export default function PageHeaderMenu({ readOnly }: PageHeaderMenuProps) {
   });
   const isDeleted = !!page?.deletedAt;
   const [workspace] = useAtom(workspaceAtom);
+  const collabProvider = useAtomValue(collabProviderAtom);
   // Community public-sharing entry point (replaces the removed EE PageShareModal)
   const workspaceSharingDisabled = workspace?.settings?.sharing?.disabled === true;
 
+  // #370 — explicit "save a version" (Cmd+S / Save button). One path for the
+  // human; the server derives the tier from the signed actor. Readers can't save
+  // (the button is hidden and the collab connection is read-only server-side).
+  const handleSaveVersion = useCallback(() => {
+    if (readOnly || !collabProvider) return;
+    // Flag this client as the initiator so only it shows the confirmation toast;
+    // a safety timeout clears it if no broadcast comes back (e.g. offline).
+    saveVersionPending.current = true;
+    window.setTimeout(() => {
+      saveVersionPending.current = false;
+    }, 5000);
+    collabProvider.sendStateless(
+      JSON.stringify({ type: SAVE_VERSION_MESSAGE_TYPE }),
+    );
+  }, [readOnly, collabProvider]);
+
+  // mod+S must also block the browser's "Save page" dialog. `triggerOnContent-
+  // Editable` + empty ignore-list so it fires while typing in the editor/title.
+  useHotkeys(
+    [["mod+S", handleSaveVersion, { preventDefault: true }]],
+    [],
+    true,
+  );
+
   useHotkeys(
     [
       [
@@ -133,15 +164,16 @@ export default function PageHeaderMenu({ readOnly }: PageHeaderMenuProps) {
         </ActionIcon>
       </Tooltip>
 
-      <PageActionMenu readOnly={readOnly} />
+      <PageActionMenu readOnly={readOnly} onSaveVersion={handleSaveVersion} />
     </>
   );
 }
 
 interface PageActionMenuProps {
   readOnly?: boolean;
+  onSaveVersion?: () => void;
 }
-function PageActionMenu({ readOnly }: PageActionMenuProps) {
+function PageActionMenu({ readOnly, onSaveVersion }: PageActionMenuProps) {
   const { t } = useTranslation();
   const [, setHistoryModalOpen] = useAtom(historyAtoms);
   const clipboard = useClipboard({ timeout: 500 });
@@ -303,6 +335,20 @@ function PageActionMenu({ readOnly }: PageActionMenuProps) {
             </Group>
           </Menu.Item>
 
+          {!readOnly && (
+            <Menu.Item
+              leftSection={<IconDeviceFloppy size={16} />}
+              onClick={onSaveVersion}
+              rightSection={
+                <Text size="xs" c="dimmed">
+                  {t("Ctrl+S")}
+                </Text>
+              }
+            >
+              {t("Save version")}
+            </Menu.Item>
+          )}
+
           <Menu.Item
             leftSection={<IconHistory size={16} />}
             onClick={openHistoryModal}

apps/client/src/features/page/tree/components/space-tree.tsx

@@ -281,10 +281,12 @@ 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 realtime insert paths use
+        // (`isUnloadedBranch`) so the lazy-load gate and the realtime inserts
+        // (`insertByPosition` / `placeByPosition`) can never disagree about what
+        // counts as unloaded (#525). Note: local raw `insert` (DnD/create-page)
+        // does not yet route through it — see #525 follow-up.
+        if (treeModel.isUnloadedBranch(node)) {
           const fetched = await fetchAllAncestorChildren({
             pageId: id,
             spaceId: node.spaceId,

apps/client/src/features/page/tree/model/tree-model.test.ts

@@ -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"]);
   });
 

apps/client/src/features/page/tree/model/tree-model.ts

@@ -43,6 +43,26 @@ 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`)
+  // and the realtime insert paths (`insertByPosition` / `placeByPosition`), so they
+  // can never drift apart again. (The local raw `insert` primitive and its DnD/
+  // create-page callers do NOT yet route through this predicate — see #525
+  // follow-up.) 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 +147,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,

apps/client/src/features/websocket/tree-socket-reducers.test.ts

@@ -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).