test(git-sync): assert stable drawio markers, not the omittable center default (#351 review)

stabilize.test.ts used `data-align="center"` as proof the convergence pass
materialized the drawio node. Since this PR correctly stops emitting the
schema-default center align in the media builders, that marker is no longer a
reliable convergence proof. Assert on the stable canonical markers instead —
`data-type="drawio"` + `data-src="/d.drawio"` — which are always materialized
regardless of the align default. The fixpoint assertion (file2 === file1) is
unchanged; round-trip stays byte-stable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CHANGELOG.md

@@ -12,14 +12,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- **Save intentional page versions.** Press `Cmd/Ctrl+S` (or use the page menu)
-  to save a named version of a page. The history panel now distinguishes
-  intentional versions (a "Saved" / "Agent version" badge) from automatic
-  snapshots, dims autosaves, and offers an "Only versions" filter. Automatic
-  snapshots switched from a fixed interval to a trailing idle-flush with a
-  max-wait ceiling, and a boundary snapshot is pinned whenever the editing source
-  changes (e.g. a person's edits followed by the AI agent). (#370)
-
 - **Place several images side by side in a row.** A new "Inline (side by
   side)" alignment mode in the image bubble menu renders consecutive inline
   images as a row that wraps onto the next line on narrow screens. The row is

apps/client/public/locales/en-US/translation.json

@@ -1418,14 +1418,5 @@
   "The commented text changed since this suggestion was made; it was not applied.": "The commented text changed since this suggestion was made; it was not applied.",
   "Dismiss": "Dismiss",
   "Suggestion dismissed": "Suggestion dismissed",
-  "Failed to dismiss suggestion": "Failed to dismiss suggestion",
-  "Save version": "Save version",
-  "Ctrl+S": "Ctrl+S",
-  "Version saved": "Version saved",
-  "Already saved as the latest version": "Already saved as the latest version",
-  "Agent version": "Agent version",
-  "Boundary": "Boundary",
-  "Autosave": "Autosave",
-  "Only versions": "Only versions",
-  "No saved versions yet.": "No saved versions yet."
+  "Failed to dismiss suggestion": "Failed to dismiss suggestion"
 }

apps/client/public/locales/ru-RU/translation.json

@@ -1281,14 +1281,5 @@
   "The commented text changed since this suggestion was made; it was not applied.": "Прокомментированный текст изменился после создания предложения; оно не было применено.",
   "Dismiss": "Не применять",
   "Suggestion dismissed": "Предложение отклонено",
-  "Failed to dismiss suggestion": "Не удалось отклонить предложение",
-  "Save version": "Сохранить версию",
-  "Ctrl+S": "Ctrl+S",
-  "Version saved": "Версия сохранена",
-  "Already saved as the latest version": "Уже сохранено как последняя версия",
-  "Agent version": "Версия агента",
-  "Boundary": "Граница",
-  "Autosave": "Автосейв",
-  "Only versions": "Только версии",
-  "No saved versions yet.": "Пока нет сохранённых версий."
+  "Failed to dismiss suggestion": "Не удалось отклонить предложение"
 }

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

@@ -1,19 +1,10 @@
 import { atom } from "jotai";
 import { 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,18 +31,11 @@ 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,
@@ -130,7 +123,6 @@ 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);
@@ -188,24 +180,6 @@ 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) {
@@ -263,16 +237,12 @@ 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,11 +1,4 @@
-import {
-  Text,
-  Group,
-  UnstyledButton,
-  Avatar,
-  Tooltip,
-  Badge,
-} from "@mantine/core";
+import { Text, Group, UnstyledButton, Avatar, Tooltip } 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";
@@ -14,59 +7,36 @@ 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;
-  // 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;
+  index: number;
+  onSelect: (id: string, index: number) => void;
+  onHover?: (id: string, index: number) => 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);
-  }, [onSelect, historyItem.id]);
+    onSelect(historyItem.id, index);
+  }, [onSelect, historyItem.id, index]);
 
   const handleMouseEnter = useCallback(() => {
-    onHover?.(historyItem.id);
-  }, [onHover, historyItem.id]);
+    onHover?.(historyItem.id, index);
+  }, [onHover, historyItem.id, index]);
 
   const contributors = historyItem.contributors;
   const hasContributors = contributors && contributors.length > 0;
@@ -79,20 +49,8 @@ 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 }}
     >
-      <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>
+      <Text size="sm">{formattedDate(new Date(historyItem.createdAt))}</Text>
 
       <Group gap={6} wrap="nowrap" mt={4}>
         {hasContributors ? (

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

@@ -9,7 +9,7 @@ import {
   historyAtoms,
 } from "@/features/page-history/atoms/history-atoms";
 import { useAtom, useSetAtom } from "jotai";
-import { useCallback, useEffect, useMemo, useRef, useState } from "react";
+import { useCallback, useEffect, useMemo, useRef } from "react";
 import {
   Button,
   ScrollArea,
@@ -17,12 +17,9 @@ 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;
 
@@ -50,23 +47,6 @@ 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);
-  const isVersion = useCallback(
-    (kind?: string | null) => kind === "manual" || kind === "agent",
-    [],
-  );
-  const visibleItems = useMemo(
-    () =>
-      onlyVersions
-        ? historyItems.filter((item) => isVersion(item.kind))
-        : historyItems,
-    [historyItems, onlyVersions, isVersion],
-  );
-
   const loadMoreRef = useRef<HTMLDivElement>(null);
   const prefetchTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
 
@@ -80,13 +60,11 @@ function HistoryList({ pageId }: Props) {
   }, []);
 
   const handleHover = useCallback(
-    (historyId: string) => {
+    (historyId: string, index: number) => {
       clearPrefetchTimeout();
       prefetchTimeoutRef.current = setTimeout(() => {
         prefetchPageHistory(historyId);
-        // The true previous snapshot in the FULL list (not the previous visible
-        // one under the "only versions" filter).
-        const prevId = resolvePrevSnapshotId(historyItems, historyId);
+        const prevId = historyItems[index + 1]?.id;
         if (prevId) {
           prefetchPageHistory(prevId);
         }
@@ -100,11 +78,9 @@ function HistoryList({ pageId }: Props) {
   }, [clearPrefetchTimeout]);
 
   const handleSelect = useCallback(
-    (id: string) => {
+    (id: string, index: number) => {
       setActiveHistoryId(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));
+      setActiveHistoryPrevId(historyItems[index + 1]?.id ?? "");
     },
     [historyItems, setActiveHistoryId, setActiveHistoryPrevId],
   );
@@ -152,27 +128,12 @@ 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}>
-        {onlyVersions && visibleItems.length === 0 && (
-          <Center py="md">
-            <Text size="sm" c="dimmed">
-              {t("No saved versions yet.")}
-            </Text>
-          </Center>
-        )}
-        {visibleItems.map((historyItem) => (
+        {historyItems.map((historyItem, index) => (
           <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,10 +24,6 @@ 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

@@ -1,42 +0,0 @@
-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

@@ -1,22 +0,0 @@
-/**
- * #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

@@ -1,28 +0,0 @@
-/**
- * #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,7 +3,6 @@ import {
   IconArrowRight,
   IconArrowsHorizontal,
   IconClockHour4,
-  IconDeviceFloppy,
   IconDots,
   IconEye,
   IconEyeOff,
@@ -18,7 +17,7 @@ import {
   IconTrash,
   IconWifiOff,
 } from "@tabler/icons-react";
-import React, { useCallback, useEffect, useRef, useState } from "react";
+import React, { 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";
@@ -40,14 +39,9 @@ import { Trans, useTranslation } from "react-i18next";
 import ExportModal from "@/components/common/export-modal";
 import { htmlToMarkdown } from "@docmost/editor-ext";
 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";
@@ -78,34 +72,9 @@ 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(
     [
       [
@@ -164,16 +133,15 @@ export default function PageHeaderMenu({ readOnly }: PageHeaderMenuProps) {
         </ActionIcon>
       </Tooltip>
 
-      <PageActionMenu readOnly={readOnly} onSaveVersion={handleSaveVersion} />
+      <PageActionMenu readOnly={readOnly} />
     </>
   );
 }
 
 interface PageActionMenuProps {
   readOnly?: boolean;
-  onSaveVersion?: () => void;
 }
-function PageActionMenu({ readOnly, onSaveVersion }: PageActionMenuProps) {
+function PageActionMenu({ readOnly }: PageActionMenuProps) {
   const { t } = useTranslation();
   const [, setHistoryModalOpen] = useAtom(historyAtoms);
   const clipboard = useClipboard({ timeout: 500 });
@@ -334,20 +302,6 @@ function PageActionMenu({ readOnly, onSaveVersion }: 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/server/src/collaboration/constants.ts

@@ -1,29 +1,3 @@
-/**
- * #370 — page-history intentionality tiers. Domain of `page_history.kind`.
- *   - 'manual' / 'agent'  → Tier 1 versions (intentional points)
- *   - 'idle' / 'boundary' → Tier 0 autosnapshots (safety net)
- * A legacy `null` kind is treated as an autosave.
- */
-export type PageHistoryKind = 'manual' | 'agent' | 'idle' | 'boundary';
-
-/**
- * #370 — trailing idle-flush windows. A page's pending idle snapshot is
- * re-armed on every store and fires this long after edits go quiet, so a burst
- * of edits collapses into a single autosnapshot instead of one-per-store. Human
- * sessions are noisier and less risky, so they flush less often than the agent.
- */
-export const IDLE_INTERVAL_USER = 60 * 60 * 1000; // 60m
-export const IDLE_INTERVAL_AGENT = 15 * 60 * 1000; // 15m
-
-/**
- * #370 — max-wait ceiling for the idle flush. Pure trailing debounce starves the
- * safety net: hocuspocus stores at least every ~45s, so a CONTINUOUS editing
- * session would re-arm the trailing timer forever and never take an idle
- * snapshot until edits finally go quiet (up to IDLE_INTERVAL_USER = 60m). This
- * ceiling bounds the actual wait from the FIRST edit of a burst, so an idle
- * snapshot fires at least this often during a long unbroken session — restoring
- * a recovery point cadence closer to the old heuristic without one-per-store
- * noise. Mirrors hocuspocus's own maxDebounce idea.
- */
-export const IDLE_MAX_WAIT_USER = 10 * 60 * 1000; // 10m
-export const IDLE_MAX_WAIT_AGENT = 5 * 60 * 1000; // 5m
+export const HISTORY_INTERVAL = 5 * 60 * 1000;
+export const HISTORY_FAST_INTERVAL = 60 * 1000;
+export const HISTORY_FAST_THRESHOLD = 5 * 60 * 1000;

apps/server/src/collaboration/extensions/compute-history-job.spec.ts

@@ -1,93 +1,84 @@
-import { computeHistoryJob, resolveSource } from './persistence.extension';
 import {
-  IDLE_INTERVAL_AGENT,
-  IDLE_INTERVAL_USER,
-  IDLE_MAX_WAIT_AGENT,
-  IDLE_MAX_WAIT_USER,
+  computeHistoryJob,
+  resolveSource,
+} from './persistence.extension';
+import {
+  HISTORY_FAST_INTERVAL,
+  HISTORY_FAST_THRESHOLD,
+  HISTORY_INTERVAL,
 } from '../constants';
 
+// A fixed clock + fixed createdAt make pageAge deterministic.
+const NOW = 1_700_000_000_000;
 const PAGE_ID = '550e8400-e29b-41d4-a716-446655440000';
 
-const page = { id: PAGE_ID };
+// Build a minimal page whose age (NOW - createdAt) is exactly `ageMs`.
+const pageAged = (ageMs: number) => ({
+  id: PAGE_ID,
+  createdAt: new Date(NOW - ageMs),
+});
 
-describe('computeHistoryJob (#370 — shared trailing idle pipeline)', () => {
-  it('human edit → user idle window, bare page.id job', () => {
-    // Humans and the agent now share ONE idle job per page (jobId = page.id).
-    // The agent's old delay=0 fast path is GONE — intentional agent points now
-    // arrive via the explicit save-version signal, not a zero-delay snapshot.
-    const { jobId, delay } = computeHistoryJob(page, 'user');
-    expect(delay).toBe(IDLE_INTERVAL_USER);
+describe('computeHistoryJob', () => {
+  it('agent edit → delay MUST be 0 and job id is source-keyed', () => {
+    // INVARIANT (§15 H2 / persistence.extension): the agent delay MUST stay 0.
+    // The worker re-reads the page row at run time, so any non-zero delay risks
+    // snapshotting content a later human edit has already overwritten. This is
+    // the load-bearing assertion of this spec — do not relax it.
+    const { jobId, delay } = computeHistoryJob(pageAged(0), 'agent', NOW);
+    expect(delay).toBe(0);
+    expect(jobId).toBe(`${PAGE_ID}-agent`);
+  });
+
+  it('agent edit on an OLD page is still delay 0 (age never applies to agents)', () => {
+    // Even when the page is far older than the fast threshold, the agent path
+    // must short-circuit to 0 — age-based debounce is a human-only concern.
+    const { jobId, delay } = computeHistoryJob(
+      pageAged(HISTORY_FAST_THRESHOLD + 60_000),
+      'agent',
+      NOW,
+    );
+    expect(delay).toBe(0);
+    expect(jobId).toBe(`${PAGE_ID}-agent`);
+  });
+
+  it('human edit on a YOUNG page (age < threshold) → fast interval, bare job id', () => {
+    const { jobId, delay } = computeHistoryJob(
+      pageAged(HISTORY_FAST_THRESHOLD - 1),
+      'user',
+      NOW,
+    );
+    expect(delay).toBe(HISTORY_FAST_INTERVAL);
     expect(jobId).toBe(PAGE_ID);
   });
 
-  it('agent edit → agent idle window (shorter), still the bare page.id job', () => {
-    const { jobId, delay } = computeHistoryJob(page, 'agent');
-    expect(delay).toBe(IDLE_INTERVAL_AGENT);
-    // No `-agent` suffix anymore: the agent joins the common idle pipeline.
+  it('human edit on an OLD page (age > threshold) → standard interval', () => {
+    const { jobId, delay } = computeHistoryJob(
+      pageAged(HISTORY_FAST_THRESHOLD + 1),
+      'user',
+      NOW,
+    );
+    expect(delay).toBe(HISTORY_INTERVAL);
     expect(jobId).toBe(PAGE_ID);
   });
 
-  it('agent flushes sooner than a human', () => {
-    expect(IDLE_INTERVAL_AGENT).toBeLessThan(IDLE_INTERVAL_USER);
+  it('boundary: pageAge EXACTLY === threshold takes the slow branch (the `<` is strict)', () => {
+    // Off-by-one guard: the condition is `pageAge < HISTORY_FAST_THRESHOLD`, so
+    // an age of exactly the threshold is NOT "fast" — it must use HISTORY_INTERVAL.
+    const { delay } = computeHistoryJob(
+      pageAged(HISTORY_FAST_THRESHOLD),
+      'user',
+      NOW,
+    );
+    expect(delay).toBe(HISTORY_INTERVAL);
   });
 
-  it('treats any non-"agent" source string as human (keys strictly on === agent)', () => {
-    const { jobId, delay } = computeHistoryJob(page, 'user');
-    expect(delay).toBe(IDLE_INTERVAL_USER);
+  it('treats any non-"agent" source string as human', () => {
+    // resolveSource only ever yields 'agent' | 'user', but guard the contract:
+    // the agent branch keys strictly on === 'agent'.
+    const { jobId, delay } = computeHistoryJob(pageAged(0), 'user', NOW);
+    expect(delay).toBe(HISTORY_FAST_INTERVAL);
     expect(jobId).toBe(PAGE_ID);
   });
-
-  // #370 review round-1 WARNING: the max-wait ceiling prevents autosnapshot
-  // starvation during a continuous editing session (the trailing timer would
-  // otherwise re-arm forever and never fire).
-  describe('max-wait ceiling', () => {
-    const T0 = 1_000_000; // arbitrary fixed epoch for deterministic tests
-
-    it('once a burst is armed, delay clamps to the remaining max-wait budget', () => {
-      // 1 minute into the burst the USER interval (60m) far exceeds the remaining
-      // max-wait budget (10m - 1m = 9m), so the delay is clamped DOWN to that
-      // remaining budget — the full interval is NOT used once a ceiling applies.
-      const { delay } = computeHistoryJob(page, 'user', T0, T0 + 60_000);
-      expect(delay).toBe(IDLE_MAX_WAIT_USER - 60_000);
-    });
-
-    it('never waits longer than the max-wait budget from the burst start', () => {
-      // A store arriving right at the ceiling → delay 0 (fire promptly).
-      const { delay } = computeHistoryJob(
-        page,
-        'user',
-        T0,
-        T0 + IDLE_MAX_WAIT_USER,
-      );
-      expect(delay).toBe(0);
-    });
-
-    it('past the ceiling never returns a negative delay', () => {
-      const { delay } = computeHistoryJob(
-        page,
-        'user',
-        T0,
-        T0 + IDLE_MAX_WAIT_USER + 5 * 60_000,
-      );
-      expect(delay).toBe(0);
-    });
-
-    it('the agent ceiling is shorter than the user ceiling', () => {
-      expect(IDLE_MAX_WAIT_AGENT).toBeLessThan(IDLE_MAX_WAIT_USER);
-      const { delay } = computeHistoryJob(
-        page,
-        'agent',
-        T0,
-        T0 + IDLE_MAX_WAIT_AGENT,
-      );
-      expect(delay).toBe(0);
-    });
-
-    it('without a burstStart there is no ceiling (backward-compatible)', () => {
-      expect(computeHistoryJob(page, 'user').delay).toBe(IDLE_INTERVAL_USER);
-      expect(computeHistoryJob(page, 'agent').delay).toBe(IDLE_INTERVAL_AGENT);
-    });
-  });
 });
 
 describe('resolveSource (truth table)', () => {

apps/server/src/collaboration/extensions/persistence-store.spec.ts

@@ -40,12 +40,11 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
   let pageHistoryRepo: {
     saveHistory: jest.Mock;
     findPageLastHistory: jest.Mock;
-    updateHistoryKind: jest.Mock;
   };
   let aiQueue: { add: jest.Mock };
-  let historyQueue: { add: jest.Mock; remove: jest.Mock };
+  let historyQueue: { add: jest.Mock };
   let notificationQueue: { add: jest.Mock };
-  let collabHistory: { addContributors: jest.Mock; popContributors: jest.Mock };
+  let collabHistory: { addContributors: jest.Mock };
   let transclusionService: {
     syncPageTransclusions: jest.Mock;
     syncPageReferences: jest.Mock;
@@ -94,22 +93,13 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
     pageHistoryRepo = {
       saveHistory: jest.fn().mockImplementation(async () => {
         callOrder.push('saveHistory');
-        return { id: 'history-1' };
       }),
       findPageLastHistory: jest.fn().mockResolvedValue(null),
-      updateHistoryKind: jest.fn().mockResolvedValue(undefined),
     };
     aiQueue = { add: jest.fn().mockResolvedValue(undefined) };
-    historyQueue = {
-      add: jest.fn().mockResolvedValue(undefined),
-      // #370 — enqueuePageHistory now removes any pending idle job before re-adding.
-      remove: jest.fn().mockResolvedValue(undefined),
-    };
+    historyQueue = { add: jest.fn().mockResolvedValue(undefined) };
     notificationQueue = { add: jest.fn().mockResolvedValue(undefined) };
-    collabHistory = {
-      addContributors: jest.fn().mockResolvedValue(undefined),
-      popContributors: jest.fn().mockResolvedValue([]),
-    };
+    collabHistory = { addContributors: jest.fn().mockResolvedValue(undefined) };
     transclusionService = {
       syncPageTransclusions: jest.fn().mockResolvedValue(undefined),
       syncPageReferences: jest.fn().mockResolvedValue(undefined),
@@ -175,50 +165,6 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
     expect(pageRepo.updatePage.mock.calls[0][0].lastUpdatedSource).toBe('user');
   });
 
-  // #370 review round-1 SUGGESTION: the boundary was GENERALIZED from a
-  // user→agent special-case to ANY lastUpdatedSource transition. These pin the
-  // generalized behaviour it was rebuilt for.
-  describe('generalized boundary — any source transition', () => {
-    // Same persisted page but with an explicit prior source.
-    const pageWithPriorSource = (prior: string | null) => ({
-      ...persistedHumanPage('NEW CONTENT'),
-      lastUpdatedSource: prior,
-    });
-
-    it('agent→user transition fires the boundary (pins the prior agent revision)', async () => {
-      const document = ydocFor(doc('NEW CONTENT'));
-      pageRepo.findById.mockResolvedValue(pageWithPriorSource('agent'));
-      pageHistoryRepo.findPageLastHistory.mockResolvedValue(null);
-
-      await ext.onStoreDocument(buildData(document, 'user') as any);
-
-      expect(pageHistoryRepo.saveHistory).toHaveBeenCalledTimes(1);
-      expect(callOrder).toEqual(['saveHistory', 'updatePage']);
-      expect(pageRepo.updatePage.mock.calls[0][0].lastUpdatedSource).toBe('user');
-    });
-
-    it('git→user transition fires the boundary (git-sync overwrite is a source change)', async () => {
-      const document = ydocFor(doc('NEW CONTENT'));
-      pageRepo.findById.mockResolvedValue(pageWithPriorSource('git'));
-      pageHistoryRepo.findPageLastHistory.mockResolvedValue(null);
-
-      await ext.onStoreDocument(buildData(document, 'user') as any);
-
-      expect(pageHistoryRepo.saveHistory).toHaveBeenCalledTimes(1);
-      expect(callOrder).toEqual(['saveHistory', 'updatePage']);
-    });
-
-    it('a null prior source (first-ever edit) does NOT fire the boundary', async () => {
-      const document = ydocFor(doc('NEW CONTENT'));
-      pageRepo.findById.mockResolvedValue(pageWithPriorSource(null));
-
-      await ext.onStoreDocument(buildData(document, 'agent') as any);
-
-      expect(pageHistoryRepo.saveHistory).not.toHaveBeenCalled();
-      expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
-    });
-  });
-
   it('idempotency: unchanged content → no updatePage, no history, no queues', async () => {
     // The Y.Doc content equals the persisted content deeply → early skip.
     // A Y.Doc round-trip normalizes attrs (e.g. paragraph indent), so derive
@@ -523,125 +469,4 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
     // Contributors keyed by the UUID so they match the PAGE_HISTORY job (page.id).
     expect(collabHistory.addContributors.mock.calls[0][0]).toBe(PAGE_ID);
   });
-
-  // #370 — explicit save-version (Cmd+S / agent save tool) over the stateless
-  // seam. The tier is derived from the SIGNED connection actor, the store path
-  // is reused, and promote-not-dup avoids duplicating heavy content rows.
-  describe('save-version (#370)', () => {
-    const emitSave = (document: any, actor: 'user' | 'agent') =>
-      ext.onStateless({
-        connection: {
-          readOnly: false,
-          context: { user: { id: USER_ID, name: 'Alice' }, actor },
-        } as any,
-        documentName: `page.${PAGE_ID}`,
-        document: document as any,
-        payload: JSON.stringify({ type: 'save-version' }),
-      } as any);
-
-    // findById returns a page whose content already equals the live doc, so the
-    // store path is a no-op and we isolate the versioning decision.
-    const pageMatchingDoc = (document: any) => ({
-      ...persistedHumanPage('IGNORED'),
-      content: TiptapTransformer.fromYdoc(document, 'default'),
-    });
-
-    it('human save with no prior snapshot → writes a manual version + broadcasts', async () => {
-      const document = ydocFor(doc('VERSION ME'));
-      pageRepo.findById.mockResolvedValue(pageMatchingDoc(document));
-      pageHistoryRepo.findPageLastHistory.mockResolvedValue(null);
-
-      await emitSave(document, 'user');
-
-      expect(pageHistoryRepo.saveHistory).toHaveBeenCalledTimes(1);
-      expect(pageHistoryRepo.saveHistory.mock.calls[0][1]).toEqual(
-        expect.objectContaining({ kind: 'manual' }),
-      );
-      // The pending idle autosnapshot is cancelled by the explicit version.
-      expect(historyQueue.remove).toHaveBeenCalledWith(PAGE_ID);
-      const msg = JSON.parse(
-        (document as any).broadcastStateless.mock.calls[(document as any).broadcastStateless.mock.calls.length - 1][0],
-      );
-      expect(msg).toMatchObject({
-        type: 'version.saved',
-        kind: 'manual',
-        alreadySaved: false,
-      });
-    });
-
-    it('agent save derives kind=agent from the signed actor', async () => {
-      const document = ydocFor(doc('AGENT VERSION'));
-      pageRepo.findById.mockResolvedValue(pageMatchingDoc(document));
-      pageHistoryRepo.findPageLastHistory.mockResolvedValue(null);
-
-      await emitSave(document, 'agent');
-
-      expect(pageHistoryRepo.saveHistory.mock.calls[pageHistoryRepo.saveHistory.mock.calls.length - 1][1]).toEqual(
-        expect.objectContaining({ kind: 'agent' }),
-      );
-    });
-
-    it('promote-not-dup: latest snapshot is an autosave with identical content → upgrades in place', async () => {
-      const document = ydocFor(doc('SAME'));
-      const page = pageMatchingDoc(document);
-      pageRepo.findById.mockResolvedValue(page);
-      pageHistoryRepo.findPageLastHistory.mockResolvedValue({
-        id: 'auto-1',
-        content: page.content,
-        kind: 'idle',
-      });
-
-      await emitSave(document, 'user');
-
-      // No heavy new content row — the existing autosave is promoted to manual.
-      expect(pageHistoryRepo.updateHistoryKind).toHaveBeenCalledWith(
-        'auto-1',
-        'manual',
-        expect.anything(),
-      );
-      expect(pageHistoryRepo.saveHistory).not.toHaveBeenCalled();
-      const msg = JSON.parse(
-        (document as any).broadcastStateless.mock.calls[(document as any).broadcastStateless.mock.calls.length - 1][0],
-      );
-      expect(msg).toMatchObject({ historyId: 'auto-1', alreadySaved: false });
-    });
-
-    it('no-op when the latest snapshot is already a manual version of this content', async () => {
-      const document = ydocFor(doc('ALREADY SAVED'));
-      const page = pageMatchingDoc(document);
-      pageRepo.findById.mockResolvedValue(page);
-      pageHistoryRepo.findPageLastHistory.mockResolvedValue({
-        id: 'ver-1',
-        content: page.content,
-        kind: 'manual',
-      });
-
-      await emitSave(document, 'user');
-
-      expect(pageHistoryRepo.updateHistoryKind).not.toHaveBeenCalled();
-      expect(pageHistoryRepo.saveHistory).not.toHaveBeenCalled();
-      const msg = JSON.parse(
-        (document as any).broadcastStateless.mock.calls[(document as any).broadcastStateless.mock.calls.length - 1][0],
-      );
-      expect(msg).toMatchObject({ alreadySaved: true, kind: 'manual' });
-    });
-
-    it('a read-only connection cannot save a version', async () => {
-      const document = ydocFor(doc('READER'));
-      pageRepo.findById.mockResolvedValue(pageMatchingDoc(document));
-
-      await ext.onStateless({
-        connection: {
-          readOnly: true,
-          context: { user: { id: USER_ID }, actor: 'user' },
-        } as any,
-        documentName: `page.${PAGE_ID}`,
-        document: document as any,
-        payload: JSON.stringify({ type: 'save-version' }),
-      } as any);
-
-      expect(pageHistoryRepo.saveHistory).not.toHaveBeenCalled();
-      expect(pageHistoryRepo.updateHistoryKind).not.toHaveBeenCalled();
-    });
-  });
 });

apps/server/src/collaboration/extensions/persistence.extension.ts

@@ -36,11 +36,9 @@ import {
 import { Page } from '@docmost/db/types/entity.types';
 import { CollabHistoryService } from '../services/collab-history.service';
 import {
-  IDLE_INTERVAL_AGENT,
-  IDLE_INTERVAL_USER,
-  IDLE_MAX_WAIT_AGENT,
-  IDLE_MAX_WAIT_USER,
-  PageHistoryKind,
+  HISTORY_FAST_INTERVAL,
+  HISTORY_FAST_THRESHOLD,
+  HISTORY_INTERVAL,
 } from '../constants';
 import { TransclusionService } from '../../core/page/transclusion/transclusion.service';
 import { observeCollabStore } from '../../integrations/metrics/metrics.registry';
@@ -53,16 +51,6 @@ import { observeCollabStore } from '../../integrations/metrics/metrics.registry'
  */
 export const INTENTIONAL_CLEAR_MESSAGE_TYPE = 'intentional-clear';
 
-/**
- * #370 — wire format of the client→server "save a version" signal. Sent by the
- * human (Cmd+S / Save button) and by the agent's explicit save tool over the
- * SAME stateless channel. The intentionality tier ('manual' vs 'agent') is
- * derived SERVER-SIDE from the signed connection actor, never from this
- * payload, so a version's type is unforgeable. The document is taken from the
- * connection (not the payload), so the signal cannot be aimed at another page.
- */
-export const SAVE_VERSION_MESSAGE_TYPE = 'save-version';
-
 /**
  * #251 — how long an intentional-clear signal stays "pending" before it is
  * ignored. The signal is set on the clearing keystroke but consumed by the
@@ -99,39 +87,35 @@ export function resolveSource(
 }
 
 /**
- * #370 — compute the BullMQ job id + delay for a page's trailing idle-flush
- * autosnapshot. Pure so the timing is unit-testable.
+ * Compute the BullMQ job id + delay for a page-history snapshot job. Pure so
+ * the data-loss-sensitive timing arithmetic is unit-testable; `now` is injected
+ * (caller passes `Date.now()`) for determinism.
  *
- * Both humans and the agent now share ONE idle pipeline (the agent's old
- * `delay=0` fast path is gone — intentional agent points arrive via the
- * explicit save-version signal instead). The job id is the bare `page.id`, so a
- * page has at most one pending idle job; the caller removes-and-re-adds it on
- * every store to keep it debounced to the trailing edge of an edit burst. The
- * window differs by source only: the agent flushes sooner than a human.
+ * - Agent edits: delay 0 and a source-keyed job id `${page.id}-agent`. The
+ *   delay MUST stay 0 — the worker re-reads the page row at run time, so any
+ *   delay risks reading content a later human edit has already overwritten
+ *   (mis-tagged snapshot). 0 minimizes that window. The `-agent` suffix keeps
+ *   the job from coalescing with the bare-page.id human job.
+ * - Human edits: age-based debounce so rapid human edits coalesce into one
+ *   snapshot; job id is the bare `page.id`.
+ *
+ * BullMQ forbids ':' in custom job ids (Redis key separator), so '-' is used;
+ * page.id is a UUID, so `${page.id}-agent` cannot collide with a human job.
  */
 export function computeHistoryJob(
-  page: Pick<Page, 'id'>,
+  page: Pick<Page, 'id' | 'createdAt'>,
   source: string,
-  // Epoch ms of the FIRST edit in the current burst (when the pending idle job
-  // was first armed). Used to enforce the max-wait ceiling so a continuous
-  // editing session cannot re-arm the trailing timer forever. `now` is injectable
-  // for tests; both default to a live clock / no ceiling when omitted.
-  burstStart?: number,
-  now: number = Date.now(),
+  now: number,
 ): { jobId: string; delay: number } {
   const isAgent = source === 'agent';
-  const interval = isAgent ? IDLE_INTERVAL_AGENT : IDLE_INTERVAL_USER;
-  const maxWait = isAgent ? IDLE_MAX_WAIT_AGENT : IDLE_MAX_WAIT_USER;
-
-  let delay = interval;
-  if (burstStart !== undefined) {
-    // Time already elapsed since the burst's first edit; the snapshot must fire
-    // no later than `maxWait` after that, so shrink the trailing delay to the
-    // remaining budget (never negative, so BullMQ fires it promptly).
-    const remaining = burstStart + maxWait - now;
-    delay = Math.max(0, Math.min(interval, remaining));
-  }
-  return { jobId: page.id, delay };
+  const pageAge = now - new Date(page.createdAt).getTime();
+  const delay = isAgent
+    ? 0
+    : pageAge < HISTORY_FAST_THRESHOLD
+      ? HISTORY_FAST_INTERVAL
+      : HISTORY_INTERVAL;
+  const jobId = isAgent ? `${page.id}-agent` : page.id;
+  return { jobId, delay };
 }
 
 @Injectable()
@@ -143,11 +127,6 @@ export class PersistenceExtension implements Extension {
   // coalescing window" per document and OR it across all edits in the window,
   // so the snapshot is marked 'agent' regardless of who wrote last.
   private agentTouched: Map<string, boolean> = new Map();
-  // #370 — epoch ms of the FIRST edit in the current idle-flush burst, per page.
-  // Set when the pending idle job is first armed (empty entry), read to enforce
-  // the max-wait ceiling in computeHistoryJob, and cleared when the idle job is
-  // consumed/cancelled so the next burst starts a fresh window.
-  private idleBurstStart: Map<string, number> = new Map();
   // #251 — per-document "intentional clear pending" flags. Keyed by
   // documentName, value = expiry timestamp (ms). Set by onStateless when the
   // client reports a deliberate clear; consumed once by the next
@@ -347,19 +326,20 @@ export class PersistenceExtension implements Extension {
             //this.logger.debug('Contributors error:' + err?.['message']);
           }
 
-          // #370 — boundary snapshot on ANY source transition. When the store
-          // flips the page's provenance (user↔agent↔git), pin the OUTGOING
-          // state as its own history version BEFORE the incoming source
-          // overwrites it. `page` still holds the OLD content/provenance here,
-          // so saveHistory(page) captures the pre-transition state tagged with
-          // its own source, kind='boundary'. The incoming content is snapshotted
-          // later by the debounced idle job. Skip if the page is effectively
-          // empty or if the latest existing snapshot already equals this state
-          // (the shared isDeepStrictEqual gate — avoids duplicates). Generalizing
-          // beyond the old user→agent special-case also covers git-sync for free.
+          // Approach A — boundary snapshot before the agent's first edit.
+          // When this store is the agent's and the page's currently persisted
+          // state was authored by a human, pin that human state as its own
+          // history version BEFORE the agent overwrites it. `page` still holds
+          // the OLD content/provenance here, so saveHistory(page) captures the
+          // pre-agent state tagged 'user'. The agent's new content is
+          // snapshotted later by the debounced PAGE_HISTORY job ('agent'). Skip
+          // if the prior state is already agent-authored (boundary already
+          // pinned on the user->agent transition), if the page is effectively
+          // empty, or if the latest existing snapshot already equals this human
+          // state (avoid duplicates).
           if (
-            page.lastUpdatedSource &&
-            page.lastUpdatedSource !== lastUpdatedSource
+            lastUpdatedSource === 'agent' &&
+            page.lastUpdatedSource !== 'agent'
           ) {
             // pageHistory.pageId is uuid-typed; use page.id (never the doc-name
             // slugId) so a `page.<slugId>` doc cannot throw 22P02 here (#260).
@@ -367,13 +347,15 @@ export class PersistenceExtension implements Extension {
               page.id,
               { includeContent: true, trx },
             );
-            const baselineMissing =
+            const humanBaselineMissing =
               !lastHistory ||
               !isDeepStrictEqual(lastHistory.content, page.content);
-            if (!isEmptyParagraphDoc(page.content as any) && baselineMissing) {
+            if (
+              !isEmptyParagraphDoc(page.content as any) &&
+              humanBaselineMissing
+            ) {
               await this.pageHistoryRepo.saveHistory(page, {
                 contributorIds: page.contributorIds ?? undefined,
-                kind: 'boundary',
                 trx,
               });
             }
@@ -498,14 +480,6 @@ export class PersistenceExtension implements Extension {
       return; // unrelated / malformed stateless message
     }
 
-    // #370 — explicit "save a version" (human Cmd+S / agent save tool). Edit
-    // rights are already enforced by the readOnly reject above (a reader can't
-    // create a version), exactly as intentional-clear requires.
-    if (message?.type === SAVE_VERSION_MESSAGE_TYPE) {
-      await this.handleSaveVersion(data);
-      return;
-    }
-
     if (message?.type !== INTENTIONAL_CLEAR_MESSAGE_TYPE) return;
 
     this.intentionalClear.set(
@@ -514,117 +488,6 @@ export class PersistenceExtension implements Extension {
     );
   }
 
-  /**
-   * #370 — persist an intentional version from the live in-memory ydoc.
-   *
-   * One stateless path serves BOTH the human and the agent; the tier is derived
-   * SERVER-SIDE from the signed connection actor ('agent' → 'agent', anything
-   * else → 'manual'), so the version type cannot be spoofed by the client. We
-   * take the fresh ydoc from the collab process memory and run it through the
-   * EXISTING store path first (so pages.content/ydoc reflect the exact content
-   * being versioned — a REST endpoint would race the up-to-10s-stale page row),
-   * then snapshot it into page_history with the intentional kind.
-   *
-   * Promote-not-dup: if the latest history row already holds this exact content
-   * and it is an autosave (idle/boundary/legacy-null), upgrade its kind in place
-   * instead of duplicating a heavy content row; if it is already 'manual', it is
-   * a no-op (the client shows an "already saved" toast). Otherwise a fresh
-   * version row is written, popping the aggregated contributors from Redis.
-   */
-  private async handleSaveVersion(data: onStatelessPayload): Promise<void> {
-    const { connection, document, documentName } = data;
-    const context = connection?.context;
-    const pageId = getPageId(documentName);
-    // Unforgeable: 'agent' only for a signed agent connection, else 'manual'.
-    const kind: PageHistoryKind =
-      context?.actor === 'agent' ? 'agent' : 'manual';
-
-    // Flush the live ydoc through the normal store path so the page row + ydoc
-    // hold exactly what we are about to version (also fires the idle enqueue we
-    // supersede below, plus any source-transition boundary). onStoreDocument
-    // only needs document/documentName/context.
-    await this.onStoreDocument({
-      document,
-      documentName,
-      context,
-    } as onStoreDocumentPayload);
-
-    let result:
-      | { historyId: string; kind: PageHistoryKind; alreadySaved: boolean }
-      | undefined;
-
-    await executeTx(this.db, async (trx) => {
-      const page = await this.pageRepo.findById(pageId, {
-        withLock: true,
-        includeContent: true,
-        trx,
-      });
-      if (!page) return;
-      // Never version an effectively-empty page (mirrors the processor's
-      // first-history guard); there is nothing intentional to pin.
-      if (isEmptyParagraphDoc(page.content as any)) return;
-
-      const lastHistory = await this.pageHistoryRepo.findPageLastHistory(
-        page.id,
-        { includeContent: true, trx },
-      );
-
-      if (
-        lastHistory &&
-        isDeepStrictEqual(lastHistory.content, page.content)
-      ) {
-        // Content is already snapshotted. Promote-not-dup.
-        if (lastHistory.kind === 'manual') {
-          result = {
-            historyId: lastHistory.id,
-            kind: 'manual',
-            alreadySaved: true,
-          };
-          return;
-        }
-        await this.pageHistoryRepo.updateHistoryKind(
-          lastHistory.id,
-          kind,
-          trx,
-        );
-        result = { historyId: lastHistory.id, kind, alreadySaved: false };
-        return;
-      }
-
-      // Fresh version row. Pop the contributors aggregated since the last
-      // snapshot (SPOP); restore them if the write fails so they aren't lost.
-      const contributorIds = await this.collabHistory.popContributors(page.id);
-      try {
-        const saved = await this.pageHistoryRepo.saveHistory(page, {
-          contributorIds,
-          kind,
-          trx,
-        });
-        result = { historyId: saved.id, kind, alreadySaved: false };
-      } catch (err) {
-        await this.collabHistory.addContributors(page.id, contributorIds);
-        throw err;
-      }
-    });
-
-    // Housekeeping: this explicit version supersedes the page's pending idle
-    // autosnapshot, so cancel it (delayed job → remove() just deletes it) and
-    // end the current idle burst so the next edit starts a fresh max-wait window.
-    await this.historyQueue.remove(pageId).catch(() => undefined);
-    this.idleBurstStart.delete(pageId);
-
-    if (result) {
-      document.broadcastStateless(
-        JSON.stringify({
-          type: 'version.saved',
-          historyId: result.historyId,
-          kind: result.kind,
-          alreadySaved: result.alreadySaved,
-        }),
-      );
-    }
-  }
-
   async onChange(data: onChangePayload) {
     const documentName = data.documentName;
     const userId = data.context?.user?.id;
@@ -682,45 +545,17 @@ export class PersistenceExtension implements Extension {
     page: Page,
     lastUpdatedSource: string,
   ): Promise<void> {
-    // #370 — trailing idle debounce with a max-wait ceiling. One pending idle
-    // job per page (jobId = page.id); on every store we remove the pending
-    // delayed job and re-add it, so the snapshot lands `delay` after edits go
-    // quiet rather than once per store (precedent: workspace.service.ts).
-    // remove() on a delayed job simply deletes it (0 if absent, no throw); if the
-    // job is already ACTIVE and the remove is a no-op, the add still de-dups and
-    // the processor's isDeepStrictEqual gate collapses the duplicate content.
-    //
-    // The FIRST arm of a burst records `burstStart`; computeHistoryJob shrinks
-    // the delay to the remaining max-wait budget from that point, so a continuous
-    // session cannot re-arm the trailing timer forever and starve the snapshot.
-    // A burst marker older than THIS TIER's max-wait means the previous idle job
-    // has already fired — start a fresh window instead of firing immediately on
-    // the next edit. Must use the SAME source-specific max-wait computeHistoryJob
-    // uses (agent 5m / user 10m): a hardcoded USER ceiling would leave an agent
-    // burst's marker stale for 5..10m, forcing delay=0 on every store in that
-    // window and writing one idle row per store — exactly the per-store bloat the
-    // debounce exists to prevent, on the continuous-agent path.
-    const maxWait =
-      lastUpdatedSource === 'agent' ? IDLE_MAX_WAIT_AGENT : IDLE_MAX_WAIT_USER;
-    const now = Date.now();
-    let burstStart = this.idleBurstStart.get(page.id);
-    if (burstStart === undefined || now - burstStart >= maxWait) {
-      burstStart = now;
-      this.idleBurstStart.set(page.id, burstStart);
-    }
-
+    // Job id + delay arithmetic lives in the pure `computeHistoryJob` (see its
+    // doc comment for the agent-delay-0 / age-based-debounce invariants).
     const { jobId, delay } = computeHistoryJob(
       page,
       lastUpdatedSource,
-      burstStart,
-      now,
+      Date.now(),
     );
 
-    await this.historyQueue.remove(jobId).catch(() => undefined);
-
     await this.historyQueue.add(
       QueueJob.PAGE_HISTORY,
-      { pageId: page.id, kind: 'idle' } as IPageHistoryJob,
+      { pageId: page.id } as IPageHistoryJob,
       { jobId, delay },
     );
   }

apps/server/src/collaboration/processors/history.processor.spec.ts

@@ -66,15 +66,6 @@ describe('HistoryProcessor.process', () => {
     notificationQueue = { add: jest.fn().mockResolvedValue(undefined) };
     generalQueue = { add: jest.fn().mockResolvedValue(undefined) };
 
-    // #370 F3 — the processor now serializes its find+save under a page-row lock
-    // via executeTx. A db whose transaction().execute(fn) runs fn with a trx stub
-    // drives the real executeTx() helper without a database.
-    const db = {
-      transaction: () => ({
-        execute: (fn: (trx: any) => Promise<any>) => fn({ __trx: true }),
-      }),
-    };
-
     // WorkerHost's constructor reads `this.worker`; passing repos positionally
     // matches the constructor and avoids the Nest DI container.
     proc = new HistoryProcessor(
@@ -82,7 +73,6 @@ describe('HistoryProcessor.process', () => {
       pageRepo as any,
       collabHistory as any,
       watcherService as any,
-      db as any,
       notificationQueue as any,
       generalQueue as any,
     );
@@ -136,26 +126,15 @@ describe('HistoryProcessor.process', () => {
     await proc.process(buildJob());
 
     expect(collabHistory.popContributors).toHaveBeenCalledWith(PAGE_ID);
-    // #370 F3/F9 — the snapshot decision runs under a page-row lock. Pin the lock
-    // structurally so a refactor that drops withLock/trx (silently reintroducing
-    // the TOCTOU double-insert) turns this red. The tx stub is { __trx: true }.
-    expect(pageRepo.findById).toHaveBeenCalledWith(
-      PAGE_ID,
-      expect.objectContaining({ withLock: true, trx: { __trx: true } }),
-    );
-    // #370 F7 — addPageWatchers MUST receive the trx, or its FK-check runs on a
-    // separate connection and self-deadlocks against our FOR UPDATE. Asserting
-    // the trx arg here is exactly what would have caught that regression.
     expect(watcherService.addPageWatchers).toHaveBeenCalledWith(
       ['u1', 'u2'],
       PAGE_ID,
       SPACE_ID,
       WORKSPACE_ID,
-      { __trx: true },
     );
     expect(pageHistoryRepo.saveHistory).toHaveBeenCalledWith(
       expect.objectContaining({ id: PAGE_ID }),
-      { contributorIds: ['u1', 'u2'], kind: 'idle', trx: { __trx: true } },
+      { contributorIds: ['u1', 'u2'] },
     );
     expect(generalQueue.add).toHaveBeenCalledWith(
       QueueJob.PAGE_BACKLINKS,
@@ -207,48 +186,6 @@ describe('HistoryProcessor.process', () => {
     ]);
   });
 
-  it('COMMIT failure (throw outside the tx callback) → contributors RESTORED', async () => {
-    // #370 F8 — a commit-time failure throws OUTSIDE the callback, so the inner
-    // try/catch does not run; the outer catch must restore the popped set (else a
-    // BullMQ retry writes an unattributed version). Use a db whose execute() runs
-    // the callback THEN throws, simulating a commit abort.
-    pageHistoryRepo.findPageLastHistory.mockResolvedValue({
-      content: { type: 'doc', content: [] },
-    });
-    const commitFail = {
-      transaction: () => ({
-        execute: async (fn: (trx: any) => Promise<any>) => {
-          await fn({ __trx: true }); // callback succeeds (saveHistory ok)
-          throw new Error('commit aborted'); // ...but the COMMIT fails
-        },
-      }),
-    };
-    const procCommitFail = new HistoryProcessor(
-      pageHistoryRepo as any,
-      pageRepo as any,
-      collabHistory as any,
-      watcherService as any,
-      commitFail as any,
-      notificationQueue as any,
-      generalQueue as any,
-    );
-    jest
-      .spyOn(procCommitFail['logger'], 'error')
-      .mockImplementation(() => undefined);
-
-    await expect(procCommitFail.process(buildJob())).rejects.toThrow(
-      'commit aborted',
-    );
-    // The inner catch did NOT run (save succeeded), so only the outer catch can
-    // restore — assert it did.
-    expect(collabHistory.addContributors).toHaveBeenCalledWith(PAGE_ID, [
-      'u1',
-      'u2',
-    ]);
-    // And the post-snapshot queue work must NOT have run (we rethrew).
-    expect(generalQueue.add).not.toHaveBeenCalled();
-  });
-
   it('backlinks + notification queue failures are swallowed (history still committed)', async () => {
     pageHistoryRepo.findPageLastHistory.mockResolvedValue({
       content: { type: 'doc', content: [] },

apps/server/src/collaboration/processors/history.processor.ts

@@ -19,9 +19,6 @@ import { isDeepStrictEqual } from 'node:util';
 import { CollabHistoryService } from '../services/collab-history.service';
 import { WatcherService } from '../../core/watcher/watcher.service';
 import { isEmptyParagraphDoc } from '../collaboration.util';
-import { InjectKysely } from 'nestjs-kysely';
-import { KyselyDB } from '@docmost/db/types/kysely.types';
-import { executeTx } from '@docmost/db/utils';
 
 @Processor(QueueName.HISTORY_QUEUE)
 export class HistoryProcessor extends WorkerHost implements OnModuleDestroy {
@@ -32,7 +29,6 @@ export class HistoryProcessor extends WorkerHost implements OnModuleDestroy {
     private readonly pageRepo: PageRepo,
     private readonly collabHistory: CollabHistoryService,
     private readonly watcherService: WatcherService,
-    @InjectKysely() private readonly db: KyselyDB,
     @InjectQueue(QueueName.NOTIFICATION_QUEUE) private notificationQueue: Queue,
     @InjectQueue(QueueName.GENERAL_QUEUE) private generalQueue: Queue,
   ) {
@@ -45,9 +41,6 @@ export class HistoryProcessor extends WorkerHost implements OnModuleDestroy {
     try {
       const { pageId } = job.data;
 
-      // Read the page WITHOUT a lock first, only to bail early on the two cheap
-      // no-write cases (page gone / empty first snapshot) without opening a
-      // transaction. The authoritative check-then-write happens locked below.
       const page = await this.pageRepo.findById(pageId, {
         includeContent: true,
       });
@@ -58,109 +51,40 @@ export class HistoryProcessor extends WorkerHost implements OnModuleDestroy {
         return;
       }
 
-      // #370 F3 — the snapshot decision (findPageLastHistory → saveHistory) must
-      // be serialized against manual-save/boundary writers, which run under a
-      // page-row lock in onStoreDocument. Without it, this processor and a
-      // concurrent manual-save each read the same lastHistory (MVCC), both see
-      // content != lastHistory, and both insert — producing two page_history rows
-      // with IDENTICAL content (one 'idle', one 'manual'), defeating
-      // promote-not-dup and the version-vs-autosave split. Taking the same
-      // page-row lock makes the second writer observe the first's committed row so
-      // the isDeepStrictEqual gate collapses the duplicate. Only the read+write
-      // is transacted; the post-snapshot queue work stays outside.
-      let contributorIds: string[] = [];
-      let snapshotWritten = false;
-      let lastHistoryContent: unknown;
-      // #370 F8 — the contributor set popped from Redis (destructive SPOP) must be
-      // restored if the snapshot does not durably land. The inner try/catch only
-      // covers a throw INSIDE the callback; a COMMIT failure (connection drop,
-      // serialization/deadlock abort on commit — the transient class the epic
-      // already retries) throws OUTSIDE it, rolling the snapshot back while the
-      // pop is already gone. We track the popped set here and restore it in the
-      // outer catch so a BullMQ retry re-attributes the version. addContributors
-      // is an idempotent Redis SADD, so a double-restore is harmless.
-      let poppedForRestore: string[] = [];
+      const lastHistory = await this.pageHistoryRepo.findPageLastHistory(
+        pageId,
+        { includeContent: true },
+      );
 
-      try {
-        await executeTx(this.db, async (trx) => {
-          const lockedPage = await this.pageRepo.findById(pageId, {
-            includeContent: true,
-            withLock: true,
-            trx,
-          });
-          if (!lockedPage) return;
-
-          const lastHistory = await this.pageHistoryRepo.findPageLastHistory(
-            pageId,
-            { includeContent: true, trx },
-          );
-          lastHistoryContent = lastHistory?.content;
-
-          if (!lastHistory && isEmptyParagraphDoc(lockedPage.content as any)) {
-            this.logger.debug(
-              `Skipping first history for page ${pageId}: empty content`,
-            );
-            return;
-          }
-
-          if (
-            lastHistory &&
-            isDeepStrictEqual(lastHistory.content, lockedPage.content)
-          ) {
-            return; // already snapshotted at this content — nothing to write
-          }
-
-          contributorIds = await this.collabHistory.popContributors(pageId);
-          poppedForRestore = contributorIds;
-          try {
-            // Pass `trx` so the watcher insert's FK check (FOR KEY SHARE on
-            // pages[pageId]) runs on the SAME connection that already holds the
-            // FOR UPDATE lock from findById — otherwise it takes the FK lock on a
-            // separate pool connection and self-deadlocks against our own tx.
-            await this.watcherService.addPageWatchers(
-              contributorIds,
-              pageId,
-              lockedPage.spaceId,
-              lockedPage.workspaceId,
-              trx,
-            );
-
-            // #370 — every job on this queue is a trailing idle-flush autosnapshot.
-            await this.pageHistoryRepo.saveHistory(lockedPage, {
-              contributorIds,
-              kind: job.data.kind ?? 'idle',
-              trx,
-            });
-            snapshotWritten = true;
-            this.logger.debug(`History created for page: ${pageId}`);
-          } catch (err) {
-            await this.collabHistory.addContributors(pageId, contributorIds);
-            poppedForRestore = [];
-            throw err;
-          }
-        });
-      } catch (err) {
-        // A throw here means the tx did NOT commit (callback threw, or the commit
-        // itself failed and rolled back). If we popped contributors and the inner
-        // catch did not already restore them, restore now so the retry keeps
-        // attribution. snapshotWritten is irrelevant: it is set before commit, so
-        // it can be true even when the commit rolled the snapshot back.
-        if (poppedForRestore.length) {
-          await this.collabHistory.addContributors(pageId, poppedForRestore);
-        }
-        throw err;
-      }
-
-      // No snapshot written (page vanished / empty-first / unchanged content) →
-      // clear the contributor set for the skip cases and stop.
-      if (!snapshotWritten) {
-        if (!lastHistoryContent && isEmptyParagraphDoc(page.content as any)) {
-          await this.collabHistory.clearContributors(pageId);
-        }
+      if (!lastHistory && isEmptyParagraphDoc(page.content as any)) {
+        this.logger.debug(
+          `Skipping first history for page ${pageId}: empty content`,
+        );
+        await this.collabHistory.clearContributors(pageId);
         return;
       }
 
-      {
+      if (
+        !lastHistory ||
+        !isDeepStrictEqual(lastHistory.content, page.content)
+      ) {
+        const contributorIds = await this.collabHistory.popContributors(pageId);
+
+        try {
+          await this.watcherService.addPageWatchers(
+            contributorIds,
+            pageId,
+            page.spaceId,
+            page.workspaceId,
+          );
+
+          await this.pageHistoryRepo.saveHistory(page, { contributorIds });
+          this.logger.debug(`History created for page: ${pageId}`);
+        } catch (err) {
+          await this.collabHistory.addContributors(pageId, contributorIds);
+          throw err;
+        }
+
         const mentions = extractMentions(page.content);
         const pageMentions = extractPageMentions(mentions);
         const internalLinkSlugIds = extractInternalLinkSlugIds(page.content);
@@ -178,7 +102,7 @@ export class HistoryProcessor extends WorkerHost implements OnModuleDestroy {
             );
           });
 
-        if (contributorIds.length > 0 && lastHistoryContent) {
+        if (contributorIds.length > 0 && lastHistory?.content) {
           await this.notificationQueue
             .add(QueueJob.PAGE_UPDATED, {
               pageId,

apps/server/src/database/migrations/20260705T120000-page-history-kind.ts

@@ -1,27 +0,0 @@
-import { type Kysely } from 'kysely';
-
-/**
- * #370 — page-versioning intentionality tier on a history snapshot.
- *
- * Adds `page_history.kind`, the three-tier "how intentional was this snapshot"
- * marker that lets versions (intentional points) be told apart from autosaves:
- *   - 'manual'   — a human explicitly saved a version (Cmd+S / Save button)
- *   - 'agent'    — the AI agent explicitly saved a version
- *   - 'idle'     — trailing idle-flush autosnapshot (safety net)
- *   - 'boundary' — autosnapshot pinned on a source transition (user↔agent↔git)
- *
- * Nullable with NO default (mirrors last_updated_source in the agent-provenance
- * migration): legacy rows predate the marker and read back as `null`, which the
- * client renders as a plain autosave. Stored as a short varchar to stay
- * forward-compatible without an enum migration.
- */
-export async function up(db: Kysely<any>): Promise<void> {
-  await db.schema
-    .alterTable('page_history')
-    .addColumn('kind', 'varchar(20)', (col) => col)
-    .execute();
-}
-
-export async function down(db: Kysely<any>): Promise<void> {
-  await db.schema.alterTable('page_history').dropColumn('kind').execute();
-}

apps/server/src/database/repos/page/page-history.repo.ts

@@ -13,7 +13,6 @@ import { jsonArrayFrom, jsonObjectFrom } from 'kysely/helpers/postgres';
 import { ExpressionBuilder, sql } from 'kysely';
 import { DB } from '@docmost/db/types/db';
 import { resolveAgentProvenance } from '../agent-provenance';
-import { PageHistoryKind } from '../../../collaboration/constants';
 
 /**
  * Role-resolution subquery for a page-history row's bound AI chat (#300). Joins
@@ -47,9 +46,6 @@ export class PageHistoryRepo {
     'lastUpdatedById',
     'lastUpdatedSource',
     'lastUpdatedAiChatId',
-    // #370 — intentionality tier ('manual' | 'agent' | 'idle' | 'boundary');
-    // null on legacy rows (= autosave). Selected so callers can read/promote it.
-    'kind',
     'contributorIds',
     'spaceId',
     'workspaceId',
@@ -89,15 +85,9 @@ export class PageHistoryRepo {
 
   async saveHistory(
     page: Page,
-    opts?: {
-      contributorIds?: string[];
-      // #370 — intentionality tier for this snapshot. Omitted → null (legacy
-      // autosave semantics). Callers derive it server-side, never from a client.
-      kind?: PageHistoryKind;
-      trx?: KyselyTransaction;
-    },
-  ): Promise<PageHistory> {
-    return await this.insertPageHistory(
+    opts?: { contributorIds?: string[]; trx?: KyselyTransaction },
+  ): Promise<void> {
+    await this.insertPageHistory(
       {
         pageId: page.id,
         slugId: page.slugId,
@@ -109,7 +99,6 @@ export class PageHistoryRepo {
         // Copy the provenance marker off the page row, as for lastUpdatedById.
         lastUpdatedSource: page.lastUpdatedSource,
         lastUpdatedAiChatId: page.lastUpdatedAiChatId,
-        kind: opts?.kind ?? null,
         contributorIds: opts?.contributorIds,
         spaceId: page.spaceId,
         workspaceId: page.workspaceId,
@@ -118,25 +107,6 @@ export class PageHistoryRepo {
     );
   }
 
-  /**
-   * #370 — promote an existing snapshot's intentionality tier in place. Used by
-   * the manual-save "promote-not-dup" path: when the latest history row already
-   * holds the exact content being versioned, we upgrade its `kind` instead of
-   * duplicating a heavy content row.
-   */
-  async updateHistoryKind(
-    pageHistoryId: string,
-    kind: PageHistoryKind,
-    trx?: KyselyTransaction,
-  ): Promise<void> {
-    const db = dbOrTx(this.db, trx);
-    await db
-      .updateTable('pageHistory')
-      .set({ kind })
-      .where('id', '=', pageHistoryId)
-      .execute();
-  }
-
   async findPageHistoryByPageId(pageId: string, pagination: PaginationOptions) {
     const query = this.db
       .selectFrom('pageHistory')

apps/server/src/database/types/db.d.ts

@@ -280,7 +280,6 @@ export interface PageHistory {
   createdAt: Generated<Timestamp>;
   icon: string | null;
   id: Generated<string>;
-  kind: string | null;
   lastUpdatedAiChatId: string | null;
   lastUpdatedById: string | null;
   lastUpdatedSource: string | null;

apps/server/src/integrations/queue/constants/queue.interface.ts

@@ -20,10 +20,6 @@ export interface IStripeSeatsSyncJob {
 
 export interface IPageHistoryJob {
   pageId: string;
-  // #370 — intentionality tier the worker stamps on the snapshot. All jobs on
-  // this queue are trailing idle-flush autosnapshots, so this is 'idle' (absent
-  // → treated as 'idle' by the processor).
-  kind?: 'idle';
 }
 
 /**

packages/git-sync/test/stabilize.test.ts

@@ -55,10 +55,15 @@ describe('stabilizePageFile — normalize-on-write fixpoint (SPEC §11)', () =>
     const file2 = await stabilizePageFile(doc2, meta);
     expect(file2).toBe(file1);
 
-    // The materialized diagram default is present in the stabilized body (proof
-    // that the convergence pass actually ran, not just that two naive exports
-    // happened to match).
-    expect(body1).toContain('data-align="center"');
+    // The drawio node was materialized to its canonical HTML form by the
+    // convergence pass — a bare `{ src }` doc node becomes the full
+    // `<div data-type="drawio" data-src=...>` — proof the pass actually ran, not
+    // just two naive exports happening to match. Assert on the stable canonical
+    // markers rather than `data-align="center"`: center is a schema default the
+    // converter may omit (see prosemirror-markdown media-html.ts), so it is not
+    // a reliable convergence proof.
+    expect(body1).toContain('data-type="drawio"');
+    expect(body1).toContain('data-src="/d.drawio"');
   });
 
   it('already-stable content is unchanged by the pass (idempotent)', async () => {

packages/prosemirror-markdown/src/lib/markdown-converter.ts

@@ -48,6 +48,52 @@ export interface ConvertProseMirrorToMarkdownOptions {
   dropResolvedCommentAnchors?: boolean;
 }
 
+/**
+ * Adjacent sibling lists that share a markdown MARKER FAMILY re-parse as ONE
+ * merged list — bulletList and taskList both emit `- ` markers (→ a single
+ * `<ul>`), and two orderedLists both emit `1.` markers (→ a single `<ol>`). The
+ * cross-type case is real data loss the editor CAN produce (e.g. a taskList
+ * followed by a bulletList: the merged `<ul>` has a mix of checkbox and plain
+ * items, so `bridgeTaskLists` refuses to convert it and every taskItem loses its
+ * checkbox). Between two such adjacent list children we emit an empty HTML comment
+ * `<!-- -->`: marked renders it as its own HTML block that interrupts the list, so
+ * the two lists stay distinct; on import the comment is inert (parseAttachedComment
+ * → null) and dropped by generateJSON, and re-export re-inserts it, so the marker
+ * is byte-stable. It fires ONLY between two adjacent same-family list nodes — no
+ * separator is emitted for any other join, so non-list output is unchanged.
+ */
+const LIST_MARKER_SEPARATOR = "<!-- -->";
+function listMarkerFamily(type: string | undefined): "ul" | "ol" | null {
+  if (type === "bulletList" || type === "taskList") return "ul";
+  if (type === "orderedList") return "ol";
+  return null;
+}
+function adjacentListsMerge(
+  prevType: string | undefined,
+  curType: string | undefined,
+): boolean {
+  const a = listMarkerFamily(prevType);
+  return a !== null && a === listMarkerFamily(curType);
+}
+/**
+ * Render each block child, inserting a `<!-- -->` separator entry between any two
+ * adjacent same-marker-family list nodes (see LIST_MARKER_SEPARATOR). Callers
+ * join the returned strings with their own context separator.
+ */
+function renderBlockChildren(
+  children: any[],
+  render: (n: any) => string,
+): string[] {
+  const out: string[] = [];
+  let prevType: string | undefined;
+  for (const child of children) {
+    if (adjacentListsMerge(prevType, child?.type)) out.push(LIST_MARKER_SEPARATOR);
+    out.push(render(child));
+    prevType = child?.type;
+  }
+  return out;
+}
+
 /**
  * Convert ProseMirror/TipTap JSON content to Markdown
  * Supports all Docmost-specific node types and extensions
@@ -347,9 +393,15 @@ export function convertProseMirrorToMarkdown(
         // lossless (the body survives) and byte-stable (it re-exports identically),
         // so it is deliberately not treated as data loss.
         const parts: string[] = [];
+        let prevDocType: string | undefined;
         for (const child of nodeContent) {
           if (child?.type === "footnotesList") continue;
+          // Keep adjacent same-family sibling lists distinct (see renderBlockChildren).
+          if (adjacentListsMerge(prevDocType, child?.type)) {
+            parts.push(LIST_MARKER_SEPARATOR);
+          }
           parts.push(processNode(child));
+          prevDocType = child?.type;
         }
         for (const [id, def] of footnoteDefs) {
           if (!referencedFootnoteIds.has(id)) {
@@ -599,20 +651,34 @@ export function convertProseMirrorToMarkdown(
         return processTaskItem(node);
 
       case "listItem":
-        return nodeContent.map(processNode).join("\n");
+        // Direct-listItem path (lists normally render via processListItem, which
+        // handles the marker + indentation). Blank line between block children so
+        // multiple paragraphs do not merge on re-parse; a `<!-- -->` entry
+        // (renderBlockChildren) separates adjacent sibling lists.
+        return renderBlockChildren(nodeContent, processNode).join("\n\n");
 
-      case "blockquote":
+      case "blockquote": {
         // Prefix EVERY line of EVERY child with "> " and separate block-level
         // children with a blank ">" line so code blocks / multi-paragraph
-        // quotes round-trip correctly.
-        return nodeContent
-          .map((n: any) =>
+        // quotes round-trip correctly. A `> <!-- -->` separator line is inserted
+        // between two adjacent same-family sibling lists (renderBlockChildren)
+        // so they stay distinct inside the quote.
+        const bqParts: string[] = [];
+        let prevBqType: string | undefined;
+        for (const n of nodeContent) {
+          if (adjacentListsMerge(prevBqType, n?.type)) {
+            bqParts.push(`> ${LIST_MARKER_SEPARATOR}`);
+          }
+          bqParts.push(
             processNode(n)
               .split("\n")
               .map((line: string) => (line.length ? `> ${line}` : ">"))
               .join("\n"),
-          )
-          .join("\n>\n");
+          );
+          prevBqType = n?.type;
+        }
+        return bqParts.join("\n>\n");
+      }
 
       case "horizontalRule":
         return "---";
@@ -787,9 +853,12 @@ export function convertProseMirrorToMarkdown(
         // blockquote-prefixed; a blank line becomes a bare `>` so the callout is
         // not split.
         const calloutType = (node.attrs?.type || "info").toLowerCase();
-        const calloutBody = nodeContent
-          .map(processNode)
-          .join("\n")
+        const calloutBody = renderBlockChildren(nodeContent, processNode)
+          // Blank line between block children (rendered as a bare `>` after the
+          // prefix pass below) so multiple paragraphs stay separate nodes instead
+          // of merging on re-parse — same rule blockquote already uses. A
+          // `<!-- -->` entry (renderBlockChildren) separates adjacent sibling lists.
+          .join("\n\n")
           .split("\n")
           .map((l: string) => (l.length ? `> ${l}` : ">"))
           .join("\n");
@@ -809,7 +878,10 @@ export function convertProseMirrorToMarkdown(
         return `<summary>${renderInlineChildren(nodeContent)}</summary>\n\n`;
 
       case "detailsContent":
-        return `${nodeContent.map(processNode).join("\n")}\n`;
+        // Blank line between block children so multiple paragraphs in a details
+        // body survive as separate nodes (a single "\n" merges them on re-parse);
+        // a `<!-- -->` entry (renderBlockChildren) separates adjacent sibling lists.
+        return `${renderBlockChildren(nodeContent, processNode).join("\n\n")}\n`;
 
       case "mathInline": {
         // #293 canon #6: inline math serializes as Obsidian-native `$LaTeX$`
@@ -1338,11 +1410,19 @@ export function convertProseMirrorToMarkdown(
         // The code itself is element TEXT content (between <code> tags), so it
         // must escape < > & — NOT the attribute escaper. The language rides in
         // a class ATTRIBUTE, so it uses escapeAttr.
+        //
+        // Read the child text RAW (as `case "codeBlock"` does) and keep it
+        // VERBATIM — do NOT strip the trailing newline. Unlike the markdown fence
+        // path (which strips then relies on marked re-adding one `\n`), the schema
+        // codeBlock parseHTML reads the `<code>` text content back byte-for-byte,
+        // so stripping here would drop a trailing newline the node legitimately
+        // carries and break the round trip inside a column/cell.
         const code = escapeHtmlText(
           children
-            .map(processNode)
-            .join("")
-            .replace(/\n+$/, ""),
+            .map((child: any) =>
+              typeof child?.text === "string" ? child.text : "",
+            )
+            .join(""),
         );
         const cls = lang ? ` class="language-${escapeAttr(lang)}"` : "";
         return `<pre><code${cls}>${code}</code></pre>`;
@@ -1484,6 +1564,13 @@ export function convertProseMirrorToMarkdown(
     const indent = " ".repeat(indentWidth);
     const lines: string[] = [];
     childStrings.forEach((child, childIndex) => {
+      // Separate consecutive block children with a BLANK line so the item is a
+      // CommonMark "loose" list item and each block stays its own node. Without
+      // it, a second paragraph (`- a\n  b`) is re-parsed as a lazy continuation
+      // of the first and the two merge into one paragraph — silent data loss.
+      // The blank line still sits INSIDE the item (the following block keeps the
+      // continuation indent), so nested lists/code blocks remain nested.
+      if (childIndex > 0) lines.push("");
       child.split("\n").forEach((line, lineIndex) => {
         if (childIndex === 0 && lineIndex === 0) {
           // First physical line of the first block gets the marker.
@@ -1500,7 +1587,9 @@ export function convertProseMirrorToMarkdown(
 
   const processListItem = (item: any, prefix: string): string => {
     const itemContent = item.content || [];
-    const childStrings = itemContent.map(processNode);
+    // A `<!-- -->` entry separates two adjacent same-family sublists inside the
+    // item so they do not merge on re-parse (see renderBlockChildren).
+    const childStrings = renderBlockChildren(itemContent, processNode);
     if (childStrings.length === 0) return prefix;
     // The rendered marker is `${prefix} ` (prefix + one space), so its width —
     // and thus the continuation indent — is prefix.length + 1. This is correct
@@ -1514,7 +1603,9 @@ export function convertProseMirrorToMarkdown(
     const checkbox = checked ? "[x]" : "[ ]";
     const prefix = `- ${checkbox}`;
     const itemContent = item.content || [];
-    const childStrings = itemContent.map(processNode);
+    // A `<!-- -->` entry separates two adjacent same-family sublists inside the
+    // item so they do not merge on re-parse (see renderBlockChildren).
+    const childStrings = renderBlockChildren(itemContent, processNode);
     // An empty task item still needs its checkbox marker; without this guard
     // the indent below produces "" and the "- [ ]"/"- [x]" row disappears.
     if (childStrings.length === 0) return prefix;

packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts

@@ -270,9 +270,10 @@ const CALLOUT_CLOSE_RE = /^:::\s*$/;
  * optional title after the type is allowed but ignored (the Docmost callout
  * schema has no title). The body is the following contiguous blockquote lines.
  */
-const CALLOUT_BQ_OPEN_RE = /^>\s*\[!(\w+)\]/;
-/** Matches any blockquote continuation line (`>` … ). */
-const BLOCKQUOTE_LINE_RE = /^>/;
+// The callout's own `>` marker may be preceded by an ENCLOSING container prefix:
+// list-item indentation (`  `) and/or blockquote markers (`> `). Group 1 captures
+// that prefix (lazily, so the LAST `>` before `[!type]` is the callout's own).
+const CALLOUT_BQ_OPEN_RE = /^([>\s]*?)>\s*\[!(\w+)\]/;
 /** Matches the start/end of a code fence (``` or ~~~), capturing the marker. */
 const CODE_FENCE_RE = /^(\s*)(`{3,}|~{3,})/;
 
@@ -402,20 +403,47 @@ async function preprocessCallouts(markdown: string): Promise<string> {
       // recurse so nested callouts (`> > [!type]`) are handled, then emit the same
       // callout div the `:::` path produces. A normal blockquote (no `[!type]` on
       // its first line) does not match and stays a blockquote.
+      //
+      // PREFIX-aware: a callout nested inside a list item and/or a blockquote is
+      // serialized with the enclosing container prefix in front of its own `>`
+      // marker — `  > [!type]` (list indent) or `> > [!type]` (blockquote). We
+      // capture that prefix, take only continuation lines carrying `prefix>`, strip
+      // it, and re-apply the prefix to the emitted HTML block so the callout div
+      // stays WITHIN its container (an unprefixed div would escape and re-parse as
+      // a top-level callout / plain blockquote — silent structure loss).
       const bqOpen = line.match(CALLOUT_BQ_OPEN_RE);
       if (bqOpen) {
-        const type = bqOpen[1].toLowerCase();
+        const prefix = bqOpen[1];
+        const type = bqOpen[2].toLowerCase();
+        const cont = prefix + ">"; // a body line = prefix + the callout's own `>`
         const bodyLines: string[] = [];
         let j = i + 1;
         for (; j < lines.length; j++) {
-          if (!BLOCKQUOTE_LINE_RE.test(lines[j])) break;
-          bodyLines.push(lines[j].replace(/^>\s?/, ""));
+          if (!lines[j].startsWith(cont)) break;
+          // Drop the prefix + `>` + one optional space, leaving the body content.
+          bodyLines.push(lines[j].slice(prefix.length).replace(/^>\s?/, ""));
         }
         const inner = await transform(bodyLines);
         const renderedInner = await markedInstance.parse(inner);
-        out.push(
-          `\n<div data-type="callout" data-callout-type="${type}">${renderedInner}</div>\n`,
-        );
+        const block = `<div data-type="callout" data-callout-type="${type}">${renderedInner}</div>`;
+        if (prefix.length === 0) {
+          // Top-level callout: blank lines isolate the HTML block.
+          out.push(`\n${block}\n`);
+        } else if (prefix.includes(">")) {
+          // Enclosing BLOCKQUOTE: prefix every line and add NO surrounding blank
+          // lines — a blank line would terminate the blockquote and split the
+          // callout out of it.
+          out.push(block.split("\n").map((l) => prefix + l).join("\n"));
+        } else {
+          // Pure LIST-ITEM indentation: re-indent and keep the blank-line
+          // separators (a loose list item), so the div sits at the marker column.
+          out.push(
+            `\n${block
+              .split("\n")
+              .map((l) => (l.length ? prefix + l : l))
+              .join("\n")}\n`,
+          );
+        }
         i = j;
         continue;
       }
@@ -597,6 +625,40 @@ function bridgeTaskLists(html: string): string {
  * (null from parseAttachedComment), an unknown name, a wrong-position comment, or
  * an unknown/empty attr value is ignored.
  */
+/**
+ * A directive comment is in ATTACHED position when it sits inside a `<p>`/`<hN>`
+ * textblock — bound to that block's text (the `attrs`/`img` conventions). Every
+ * other parent (body, document level, a block container like blockquote/details/
+ * li/column div) is STANDALONE position, where a lone-block directive
+ * (subpages/pagebreak/pageembed/transclusion) is materialized. Broadening
+ * standalone beyond body/document is what lets these nodes survive NESTED inside
+ * a blockquote/callout/details/list item (previously dropped -> silent data loss).
+ */
+function isAttachedPosition(tag: string): boolean {
+  return tag === "p" || /^h[1-6]$/.test(tag);
+}
+
+/**
+ * Place a materialized standalone-directive element in the DOM: replace the
+ * comment IN PLACE when it has a real element parent inside <body> (body itself
+ * or a nested block container), preserving document order; queue it as a leading
+ * div only when the comment is at document level (no parentElement) or directly
+ * under `<html>` (outside <body>, which `document.body.innerHTML` would drop).
+ */
+function placeStandalone(
+  comment: any,
+  el: any,
+  tag: string,
+  leadingDivs: any[],
+): void {
+  if (comment.parentElement && tag !== "html") {
+    comment.replaceWith(el);
+  } else {
+    comment.remove();
+    leadingDivs.push(el);
+  }
+}
+
 function applyCommentDirectives(html: string): string {
   // Cheap early-out: no comments at all -> nothing to intercept.
   if (!html.includes("<!--")) return html;
@@ -664,13 +726,13 @@ function applyCommentDirectives(html: string): string {
 
     if (parsed.name === "subpages" || parsed.name === "pagebreak") {
       // #293 canon #5 STANDALONE machinery. A lone comment line is rendered by
-      // marked as an HTML block; the parser places it either directly under
-      // <body> (when other content surrounds it) or at document level (when it
-      // leads the output). Both are STANDALONE position. A `subpages`/`pagebreak`
-      // comment sitting inside a `<p>`/`<hN>` (or any other element) is attached
-      // position -> INERT.
-      const standalone = tag === "" || tag === "body" || tag === "html";
-      if (!standalone) continue; // wrong position -> inert
+      // marked as its own HTML block; the parser places it under <body>, at
+      // document level (leading), or — when the directive is NESTED — inside a
+      // block CONTAINER (`<blockquote>` for blockquote/callout, `<details>`,
+      // `<li>`, a column `<div>`, …). All of those are STANDALONE position. Only a
+      // comment ATTACHED inside a `<p>`/`<hN>` (bound to that block's text) is
+      // attached position -> INERT.
+      if (isAttachedPosition(tag)) continue; // wrong position -> inert
       const div = document.createElement("div");
       if (parsed.name === "pagebreak") {
         div.setAttribute("data-type", "pageBreak");
@@ -680,26 +742,18 @@ function applyCommentDirectives(html: string): string {
           div.setAttribute("data-recursive", "true");
         }
       }
-      if (tag === "body") {
-        // In-body: replace in place so surrounding content keeps its order.
-        comment.replaceWith(div);
-      } else {
-        // Document-level (leading): drop the stray comment and queue the div to
-        // be prepended into body below.
-        comment.remove();
-        leadingDivs.push(div);
-      }
+      placeStandalone(comment, div, tag, leadingDivs);
       continue;
     }
 
     if (parsed.name === "pageembed" || parsed.name === "transclusion") {
       // #293 canon #8 STANDALONE media. Like subpages/pagebreak: a lone comment
-      // line placed under <body> or at document level (leading). An attached-
-      // position comment (inside a <p>/<hN> with a sibling) is INERT. We rebuild
-      // the schema div the raw-HTML path emits (media-html.ts) from the decoded
-      // attrs so serialize/parse stay in sync.
-      const standalone = tag === "" || tag === "body" || tag === "html";
-      if (!standalone) continue; // wrong position -> inert
+      // line placed under <body>, at document level (leading), or NESTED inside a
+      // block container (blockquote/callout/details/li/column). An ATTACHED-
+      // position comment (inside a `<p>`/`<hN>`) is INERT. We rebuild the schema
+      // div the raw-HTML path emits (media-html.ts) from the decoded attrs so
+      // serialize/parse stay in sync.
+      if (isAttachedPosition(tag)) continue; // wrong position -> inert
       const el = buildElement(
         parsed.name === "pageembed"
           ? pageEmbedToHtml({ sourcePageId: parsed.attrs.sourcePageId })
@@ -709,12 +763,7 @@ function applyCommentDirectives(html: string): string {
             }),
       );
       if (!el) continue; // defensive: builder always yields an element
-      if (tag === "body") {
-        comment.replaceWith(el);
-      } else {
-        comment.remove();
-        leadingDivs.push(el);
-      }
+      placeStandalone(comment, el, tag, leadingDivs);
       continue;
     }
 
@@ -806,18 +855,36 @@ function applyCommentDirectives(html: string): string {
 
     if (!parent) continue; // attrs comment must have an element parent
     if (parsed.name !== "attrs") continue; // unknown name -> inert
-    // #293 canon #9 ATTACHED attrs: honored only in attached position.
-    const isBlock = tag === "p" || /^h[1-6]$/.test(tag);
-    if (!isBlock) continue; // misplaced comment -> inert
     const align = parsed.attrs.textAlign;
-    if (typeof align === "string" && align) {
-      // Re-express as an inline style; the schema's textAlign parseHTML reads
-      // `el.style.textAlign` back onto the paragraph/heading node.
-      parent.style.textAlign = align;
+    // #293 canon #9 ATTACHED attrs: honored only in attached position.
+    if (tag === "p" || /^h[1-6]$/.test(tag)) {
+      // A real <p>/<hN> host (loose list item, top-level block, …): re-express as
+      // an inline style; the schema's textAlign parseHTML reads `el.style.textAlign`
+      // back onto the paragraph/heading node.
+      if (typeof align === "string" && align) parent.style.textAlign = align;
+      comment.remove();
+    } else if (tag === "li" || tag === "td" || tag === "th") {
+      // TIGHT list item / GFM table cell: marked emits the paragraph's inline
+      // content DIRECTLY inside the <li>/<td>/<th> with NO <p> wrapper, so there
+      // is no element to carry the style — generateJSON materializes the
+      // paragraph later. Wrap the host's LEADING inline content (everything up to
+      // the comment; any trailing block child such as a nested list stays put) in
+      // a <p> carrying the alignment, so the materialized paragraph re-reads it.
+      if (typeof align === "string" && align) {
+        const p = document.createElement("p");
+        p.style.textAlign = align;
+        while (parent.firstChild && parent.firstChild !== comment) {
+          p.appendChild(parent.firstChild);
+        }
+        parent.insertBefore(p, comment);
+      }
+      comment.remove();
+    } else {
+      // Misplaced `attrs` comment (not a textblock/li/cell host): inert. Consume
+      // it anyway so no attached marker ever survives into the parsed body
+      // (matches the pre-existing "consume regardless" behaviour).
+      comment.remove();
     }
-    // Consume the marker regardless (unknown keys are simply ignored) so no
-    // attached comment ever survives into the parsed body.
-    comment.remove();
   }
   // Prepend any document-level (leading) standalone divs into body, preserving
   // their document order relative to each other and ahead of existing content.

packages/prosemirror-markdown/src/lib/media-html.ts

@@ -46,7 +46,11 @@ export function videoToHtml(attrs: Record<string, any>): string {
   if (attrs.width != null) parts.push(`width="${escapeAttr(attrs.width)}"`);
   if (attrs.height != null) parts.push(`height="${escapeAttr(attrs.height)}"`);
   if (attrs.size != null) parts.push(`data-size="${escapeAttr(attrs.size)}"`);
-  if (attrs.align) parts.push(`data-align="${escapeAttr(attrs.align)}"`);
+  // align default is "center" (schema): OMIT it so a bare/center node stays
+  // clean and parse's re-materialized "center" default is not a P2 churn — only
+  // a genuinely non-default left/right emits data-align (mirrors imageToHtml).
+  if (attrs.align && attrs.align !== "center")
+    parts.push(`data-align="${escapeAttr(attrs.align)}"`);
   if (attrs.aspectRatio != null)
     parts.push(`data-aspect-ratio="${escapeAttr(attrs.aspectRatio)}"`);
   return `<div><video ${parts.join(" ")}></video></div>`;
@@ -62,7 +66,9 @@ export function youtubeToHtml(attrs: Record<string, any>): string {
     parts.push(`data-width="${escapeAttr(attrs.width)}"`);
   if (attrs.height != null)
     parts.push(`data-height="${escapeAttr(attrs.height)}"`);
-  if (attrs.align) parts.push(`data-align="${escapeAttr(attrs.align)}"`);
+  // "center" is the schema default -> omit (see videoToHtml rationale).
+  if (attrs.align && attrs.align !== "center")
+    parts.push(`data-align="${escapeAttr(attrs.align)}"`);
   return `<div ${parts.join(" ")}></div>`;
 }
 
@@ -97,7 +103,9 @@ export function diagramToHtml(
   if (attrs.size != null) parts.push(`data-size="${escapeAttr(attrs.size)}"`);
   if (attrs.aspectRatio != null)
     parts.push(`data-aspect-ratio="${escapeAttr(attrs.aspectRatio)}"`);
-  if (attrs.align) parts.push(`data-align="${escapeAttr(attrs.align)}"`);
+  // "center" is the schema default -> omit (see videoToHtml rationale).
+  if (attrs.align && attrs.align !== "center")
+    parts.push(`data-align="${escapeAttr(attrs.align)}"`);
   if (attrs.attachmentId)
     parts.push(`data-attachment-id="${escapeAttr(attrs.attachmentId)}"`);
   return `<div ${parts.join(" ")}></div>`;
@@ -110,10 +118,18 @@ export function embedToHtml(attrs: Record<string, any>): string {
     `data-src="${escapeAttr(attrs.src ?? "")}"`,
     `data-provider="${escapeAttr(attrs.provider ?? "")}"`,
   ];
-  if (attrs.align) parts.push(`data-align="${escapeAttr(attrs.align)}"`);
-  if (attrs.width != null)
+  // "center" is the schema default -> omit (see videoToHtml rationale).
+  if (attrs.align && attrs.align !== "center")
+    parts.push(`data-align="${escapeAttr(attrs.align)}"`);
+  // embed width/height default to the NUMBERS 800/600 (schema). Getting a data-
+  // attribute back always yields a STRING, so emitting the default here would
+  // round-trip 800 -> "800" (a number->string P1 divergence canonicalize does
+  // NOT normalize). OMIT the defaults so parse re-materializes the numeric
+  // default instead — mirrors the top-level embed path (markdown-converter.ts),
+  // which also emits width/height only when they differ from 800/600.
+  if (attrs.width != null && attrs.width !== 800)
     parts.push(`data-width="${escapeAttr(attrs.width)}"`);
-  if (attrs.height != null)
+  if (attrs.height != null && attrs.height !== 600)
     parts.push(`data-height="${escapeAttr(attrs.height)}"`);
   return `<div ${parts.join(" ")}></div>`;
 }

packages/prosemirror-markdown/test/generative/doc-generator.ts

@@ -0,0 +1,392 @@
+/**
+ * Nested whole-document generator (#351, PR 2) — "variant A": a random walk over
+ * the schema's ContentMatch automaton.
+ *
+ * Where the FLAT generator (node-generators.ts) emits `{doc:[ <one target> ]}`,
+ * this module produces arbitrarily DEEP, valid ProseMirror documents. Validity is
+ * NOT hand-asserted: it comes straight from the schema. To fill a container's
+ * block content we start at `nodeType.contentMatch` and walk the automaton —
+ * enumerate the legal next node types (`match.edge(i).type`), let fast-check pick
+ * one (or STOP once `match.validEnd`), generate that child RECURSIVELY, then
+ * advance the automaton via `match.matchType(childType)`. A bad walk therefore
+ * cannot emit a structurally-invalid doc (the `generator validity` test guards
+ * this with `schema.nodeFromJSON(json).check()`).
+ *
+ * ── What the walk drives, and what it delegates ──────────────────────────────
+ * The walk owns BLOCK STRUCTURE (which blocks nest inside which containers, in
+ * what order and how deep). Two things it deliberately delegates, to avoid
+ * REGRESSING the byte-stable space the flat suite already proved empirically:
+ *
+ *   - INLINE content of textblocks (paragraph/heading/codeBlock/detailsSummary)
+ *     is filled from text-arbitraries.ts — the exact hostile-but-byte-stable
+ *     inline corpus the flat suite established. Walking the inline ContentMatch
+ *     instead would re-derive (and re-fail) the text-space limitations the flat
+ *     suite already pins, which is not this generator's job.
+ *   - Node ATTRS come from nodeAttrsArb(type, 'p1') — the round-trip-safe
+ *     attribute space. Attribute-degenerate fuzzing (P2/P3 over 'fuzz') is the
+ *     flat suite's concern; the nested suite isolates STRUCTURAL round-trip, so
+ *     it stays in 'p1' and does not re-litigate frozen/pinned attributes.
+ *
+ * ── Coordination the automaton cannot express ────────────────────────────────
+ * A ContentMatch guarantees a child SEQUENCE is legal, but not cross-sibling
+ * invariants. Two nodes need coordination the walk injects by hand:
+ *   - `table`: GFM needs a RECTANGULAR grid with column-consistent alignment.
+ *     The automaton happily allows ragged rows / per-cell align, which are not a
+ *     converter bug — just a malformed table. So a table is generated ATOMICALLY
+ *     (same shape the flat suite proved) rather than walked.
+ *   - `columns`: the `layout` attr must agree with the column COUNT. We pick the
+ *     layout, derive the count, then walk each column's block body normally — so
+ *     columns still gain real nested content, only the count is coordinated.
+ *
+ * ── Excluded from the nested walk ────────────────────────────────────────────
+ * Footnote nodes (footnoteReference / footnotesList / footnoteDefinition) need a
+ * DOCUMENT-GLOBAL id match between a reference and its definition. That
+ * coordination is owned by the flat suite's `footnotes` generator; placing them
+ * independently here would fabricate id mismatches that look like converter bugs
+ * but are generator defects. They are filtered out of the walk (documented in
+ * EXCLUDED). The completeness contract lives in the FLAT suite and is unaffected.
+ *
+ * ── Termination / budgets ────────────────────────────────────────────────────
+ * Two bounds keep every doc finite and the suite fast:
+ *   - MAX_DEPTH — a hard cap on block-nesting depth. A precomputed `minDepth`
+ *     fixpoint (the minimum extra nesting a subtree of each type needs to be
+ *     valid) lets the walk pick a CONTAINER child only when there is depth
+ *     headroom to complete it — so the walk can never paint itself into a corner
+ *     where a required child cannot fit (no invalid docs, guaranteed termination).
+ *   - NODE_BUDGET — a soft cap on total nodes; as it runs low the walk biases
+ *     toward STOP (when validEnd) or toward cheap terminating children.
+ */
+import fc from 'fast-check';
+import { getSchema } from '@tiptap/core';
+import { docmostExtensions } from '../../src/lib/docmost-schema.js';
+import { nodeAttrsArb } from './attr-arbitraries.js';
+import {
+  inlineContentArb,
+  headingInlineContentArb,
+  plainInlineContentArb,
+  phraseArb,
+} from './text-arbitraries.js';
+
+/** The exact ProseMirror schema the converter targets (built per the issue). */
+export const schema = getSchema(docmostExtensions as never);
+
+/** Hard cap on block-nesting depth (doc = depth 0). Kept in the issue's 4–5 band. */
+export const MAX_DEPTH = 4;
+/**
+ * Soft cap on total node count per generated document. Kept moderate: every P1/P2
+ * run parses the emitted markdown through jsdom (heavy), so 100+ node docs across
+ * hundreds of runs exhaust the worker heap. 60 still yields deeply-nested docs
+ * (depth 4) while keeping the suite within memory.
+ */
+export const NODE_BUDGET = 60;
+
+/**
+ * Nodes kept OUT of the nested walk: footnote nodes need a doc-global id match a
+ * local walk cannot coordinate (owned by the flat suite's `footnotes` generator).
+ */
+const EXCLUDED = new Set<string>([
+  'footnoteReference',
+  'footnotesList',
+  'footnoteDefinition',
+]);
+
+/**
+ * Structural-only children that carry `group: "block"` in the schema and so leak
+ * into EVERY block container's ContentMatch, even though the editor only ever
+ * places them inside their one true parent. Choosing them freely (e.g. a bare
+ * `column` at the document root) fabricates documents no editor produces and that
+ * the converter is not designed to round-trip — a GENERATOR artifact, not a
+ * converter bug. They are admitted ONLY when the container being filled is their
+ * dedicated parent. (`column` is in fact always built inside columnsArb, so this
+ * just double-guards it.)
+ */
+const DEDICATED_PARENT: Record<string, string> = {
+  column: 'columns',
+  detailsSummary: 'details',
+  detailsContent: 'details',
+};
+
+/** Is child type `t` legal as a freely-chosen child of container `parentType`? */
+function childAllowedUnder(t: string, parentType: string): boolean {
+  const dedicated = DEDICATED_PARENT[t];
+  return dedicated === undefined || dedicated === parentType;
+}
+
+/** Textblock (inlineContent) types — filled from the proven inline corpus. */
+function isTextblock(typeName: string): boolean {
+  return !!schema.nodes[typeName]?.isTextblock;
+}
+/** Leaf/atom types — no content, only generated attrs. */
+function isLeaf(typeName: string): boolean {
+  return !!schema.nodes[typeName]?.isLeaf;
+}
+
+// ---------------------------------------------------------------------------
+// minDepth fixpoint: the minimum EXTRA block-nesting depth a valid subtree
+// rooted at each node type requires. Leaves and textblocks need 0 (a textblock
+// is satisfied by inline content, no block recursion). A container needs
+// 1 + the cheapest way to satisfy its ContentMatch. Computed as a min–max path
+// to `validEnd` over the automaton, iterated to a fixpoint over node types.
+// ---------------------------------------------------------------------------
+
+/** Cheapest (min over reachable validEnd of max child minDepth) to complete a match. */
+function minCompletion(
+  match: any,
+  md: Record<string, number>,
+  seen: Set<any>,
+  parentType: string,
+): number {
+  let best = match.validEnd ? 0 : Infinity;
+  if (seen.has(match)) return best; // a cycle never completes more cheaply
+  seen.add(match);
+  for (let i = 0; i < match.edgeCount; i++) {
+    const edge = match.edge(i);
+    const t = edge.type.name;
+    if (EXCLUDED.has(t) || t === 'text') continue;
+    if (!childAllowedUnder(t, parentType)) continue;
+    const childCost = md[t];
+    if (childCost === undefined || childCost === Infinity) continue;
+    const rest = minCompletion(edge.next, md, seen, parentType);
+    if (rest === Infinity) continue;
+    best = Math.min(best, Math.max(childCost, rest));
+  }
+  seen.delete(match);
+  return best;
+}
+
+function computeMinDepth(): Record<string, number> {
+  const md: Record<string, number> = {};
+  for (const name of Object.keys(schema.nodes)) {
+    md[name] = isLeaf(name) || isTextblock(name) ? 0 : Infinity;
+  }
+  let changed = true;
+  while (changed) {
+    changed = false;
+    for (const name of Object.keys(schema.nodes)) {
+      if (md[name] === 0) continue; // leaves/textblocks fixed at 0
+      const nt: any = schema.nodes[name];
+      const completion = minCompletion(nt.contentMatch, md, new Set(), name);
+      const next = completion === Infinity ? Infinity : 1 + completion;
+      if (next < md[name]) {
+        md[name] = next;
+        changed = true;
+      }
+    }
+  }
+  return md;
+}
+
+const MIN_DEPTH = computeMinDepth();
+
+// ---------------------------------------------------------------------------
+// Leaf / textblock builders (attrs from 'p1', inline from the proven corpus).
+// ---------------------------------------------------------------------------
+
+function attachAttrs(typeName: string, base: Record<string, unknown> = {}) {
+  return nodeAttrsArb(typeName, 'p1', base).map((attrs) => {
+    const node: any = { type: typeName };
+    if (Object.keys(attrs).length) node.attrs = attrs;
+    return node;
+  });
+}
+
+/** A leaf/atom block: attrs only, no content. */
+function leafArb(typeName: string): fc.Arbitrary<any> {
+  return attachAttrs(typeName);
+}
+
+/** A textblock, inline content taken from the byte-stable flat corpus. */
+function textblockArb(typeName: string): fc.Arbitrary<any> {
+  if (typeName === 'codeBlock') {
+    return fc
+      .tuple(
+        nodeAttrsArb('codeBlock', 'p1'),
+        // Fenced code re-imports with a TRAILING NEWLINE (flat suite finding);
+        // author it so the doc is already at the round-trip fixpoint.
+        fc.array(phraseArb, { minLength: 1, maxLength: 3 }).map((l) => l.join('\n') + '\n'),
+      )
+      .map(([attrs, code]) => ({
+        type: 'codeBlock',
+        ...(Object.keys(attrs).length ? { attrs } : {}),
+        content: [{ type: 'text', text: code }],
+      }));
+  }
+  const inline =
+    typeName === 'heading'
+      ? headingInlineContentArb
+      : typeName === 'detailsSummary'
+        ? plainInlineContentArb
+        : inlineContentArb;
+  return fc
+    .tuple(nodeAttrsArb(typeName, 'p1'), inline)
+    .map(([attrs, content]) => ({
+      type: typeName,
+      ...(Object.keys(attrs).length ? { attrs } : {}),
+      content,
+    }));
+}
+
+// ---------------------------------------------------------------------------
+// Coordinated builders: table (atomic, rectangular, column-consistent align)
+// and columns (layout coupled to count, bodies walked).
+// ---------------------------------------------------------------------------
+
+/** A rectangular GFM-safe table (mirrors the flat suite's proven shape). */
+function tableArb(): fc.Arbitrary<any> {
+  return fc.integer({ min: 1, max: 3 }).chain((cols) => {
+    // One alignment per COLUMN, identical on header + every body cell, so the
+    // second export cannot re-align and churn.
+    const alignsArb = fc.array(fc.constantFrom(undefined, 'left', 'center', 'right'), {
+      minLength: cols,
+      maxLength: cols,
+    });
+    const cell = (header: boolean, align?: string) =>
+      phraseArb.map((t) => ({
+        type: header ? 'tableHeader' : 'tableCell',
+        attrs: { colspan: 1, rowspan: 1, ...(align ? { align } : {}) },
+        content: [{ type: 'paragraph', content: [{ type: 'text', text: t }] }],
+      }));
+    return alignsArb.chain((aligns) => {
+      const headerRow = fc
+        .tuple(...aligns.map((a) => cell(true, a)))
+        .map((cells) => ({ type: 'tableRow', content: cells }));
+      const bodyRow = fc
+        .tuple(...aligns.map((a) => cell(false, a)))
+        .map((cells) => ({ type: 'tableRow', content: cells }));
+      return fc
+        .tuple(headerRow, fc.array(bodyRow, { minLength: 1, maxLength: 2 }))
+        .map(([h, body]) => ({ type: 'table', content: [h, ...body] }));
+    });
+  });
+}
+
+/** A columns block: layout ↔ count coupled, each column body walked as blocks. */
+function columnsArb(depth: number, budget: number): fc.Arbitrary<any> {
+  return fc
+    .constantFrom('two_equal', 'three_equal', 'left_sidebar', 'right_sidebar')
+    .chain((layout) => {
+      const count = layout === 'three_equal' ? 3 : 2;
+      const columnType: any = schema.nodes.column;
+      // Split the remaining budget across the fixed number of columns.
+      const per = Math.max(2, Math.floor((budget - 1) / count));
+      return nodeAttrsArb('columns', 'p1', { layout, widthMode: 'normal' }).chain((attrs) =>
+        fc
+          .tuple(
+            ...Array.from({ length: count }, () =>
+              fillMatch(columnType.contentMatch, depth + 1, per, 'column').map(({ children }) => ({
+                type: 'column',
+                content: children,
+              })),
+            ),
+          )
+          .map((cols) => ({ type: 'columns', attrs, content: cols })),
+      );
+    });
+}
+
+// ---------------------------------------------------------------------------
+// The ContentMatch walk.
+// ---------------------------------------------------------------------------
+
+/** Count every node in a subtree (block + inline), for budget accounting. */
+function countNodes(node: any): number {
+  let n = 1;
+  for (const c of node.content ?? []) n += countNodes(c);
+  return n;
+}
+
+/** Build a single child node of a given type at `depth`, within `budget`. */
+function blockNode(typeName: string, depth: number, budget: number): fc.Arbitrary<any> {
+  if (typeName === 'table') return tableArb();
+  if (typeName === 'columns') return columnsArb(depth, budget);
+  if (isTextblock(typeName)) return textblockArb(typeName);
+  if (isLeaf(typeName)) return leafArb(typeName);
+  // Generic container: attrs from 'p1', block content from the automaton walk.
+  const nt: any = schema.nodes[typeName];
+  return nodeAttrsArb(typeName, 'p1').chain((attrs) =>
+    fillMatch(nt.contentMatch, depth, budget - 1, typeName).map(({ children }) => {
+      const node: any = { type: typeName };
+      if (Object.keys(attrs).length) node.attrs = attrs;
+      if (children.length) node.content = children;
+      return node;
+    }),
+  );
+}
+
+/**
+ * Fill a container's block content by walking its ContentMatch automaton from
+ * `match`. Returns the children array plus the budget left after them.
+ */
+function fillMatch(
+  match: any,
+  depth: number,
+  budget: number,
+  parentType: string,
+): fc.Arbitrary<{ children: any[]; budget: number }> {
+  const canStop = match.validEnd;
+  // A child lives at depth+1; only pick it if its subtree can complete within
+  // MAX_DEPTH. This headroom rule is what makes the walk deadlock-free.
+  const headroom = MAX_DEPTH - (depth + 1);
+  const edges: { t: string; next: any }[] = [];
+  if (headroom >= 0) {
+    for (let i = 0; i < match.edgeCount; i++) {
+      const edge = match.edge(i);
+      const t = edge.type.name;
+      if (EXCLUDED.has(t) || t === 'text') continue;
+      if (!childAllowedUnder(t, parentType)) continue;
+      if ((MIN_DEPTH[t] ?? Infinity) > headroom) continue;
+      edges.push({ t, next: edge.next });
+    }
+  }
+
+  // Decide the next action: STOP (if allowed) or extend with one more child.
+  // Bias toward stopping when the budget is spent; force a child only when the
+  // match is not yet at a valid end.
+  const pool: { weight: number; arbitrary: fc.Arbitrary<{ t: string; next: any } | null> }[] = [];
+  const canGo = edges.length > 0 && (budget > 0 || !canStop);
+  if (canStop) {
+    // Stop is weighted higher when the budget is low so docs stay bounded.
+    pool.push({ weight: budget > 0 ? 2 : 5, arbitrary: fc.constant(null) });
+  }
+  if (canGo && !(canStop && budget <= 0)) {
+    pool.push({ weight: 3, arbitrary: fc.constantFrom(...edges) });
+  }
+  // Forced continuation: not a valid end yet and (budget exhausted) — must place
+  // a mandatory child regardless of budget.
+  if (pool.length === 0) {
+    if (edges.length > 0) {
+      pool.push({ weight: 1, arbitrary: fc.constantFrom(...edges) });
+    } else {
+      // No legal child and not required to place one: stop with what we have.
+      return fc.constant({ children: [], budget });
+    }
+  }
+
+  return fc.oneof(...pool).chain((choice) => {
+    if (choice === null) return fc.constant({ children: [], budget });
+    return blockNode(choice.t, depth + 1, budget).chain((node) => {
+      const cost = countNodes(node);
+      return fillMatch(choice.next, depth, budget - cost, parentType).map(
+        ({ children, budget: left }) => ({
+          children: [node, ...children],
+          budget: left,
+        }),
+      );
+    });
+  });
+}
+
+/**
+ * The nested-document arbitrary: a valid, arbitrarily-deep ProseMirror doc built
+ * by walking the schema from the document root. Attrs stay in the round-trip-safe
+ * 'p1' space; inline content reuses the byte-stable flat corpus.
+ */
+export const docArb: fc.Arbitrary<any> = fillMatch(
+  schema.nodes.doc.contentMatch,
+  0,
+  NODE_BUDGET,
+  'doc',
+).map(({ children }) => ({ type: 'doc', content: children }));
+
+/** The precomputed minDepth table, exported for inspection/debugging. */
+export { MIN_DEPTH };

packages/prosemirror-markdown/test/generative/nested-roundtrip.property.test.ts

@@ -0,0 +1,185 @@
+import { describe, expect, it, vi } from 'vitest';
+import fc from 'fast-check';
+// Real converters. Importing markdownToProseMirror (transitively, via index)
+// mutates the global DOM via jsdom at module load — expected, required for
+// @tiptap/html's generateJSON under Node (same as the flat sibling suite).
+import {
+  convertProseMirrorToMarkdown,
+  markdownToProseMirror,
+  docsCanonicallyEqual,
+  canonicalizeContent,
+} from '../../src/lib/index.js';
+import { firstDivergence } from '../roundtrip-helpers.js';
+import { schema, docArb } from './doc-generator.js';
+
+// Each run does a real convert + jsdom parse; give ample headroom so the suite
+// is deterministic under parallel worker load (matching the flat sibling suite).
+vi.setConfig({ testTimeout: 60000 });
+
+// ---------------------------------------------------------------------------
+// #351 PR 2 — GENERATIVE round-trip over NESTED (whole-document) docs produced
+// by the ContentMatch random walk (doc-generator.ts). The invariants mirror the
+// flat suite, plus a parser-fuzz totality property (P4):
+//
+//   P1 — semantic round-trip: docsCanonicallyEqual(mdToPm(pmToMd(d)), d)
+//   P2 — byte fixpoint (2nd pass): pmToMd(mdToPm(pmToMd(d))) === pmToMd(d)
+//        (the FIRST pass may normalize once; the SECOND pass must be a fixpoint)
+//   P3 — totality: neither converter throws; bounded.
+//   P4 — parser fuzz totality: for ANY string, markdownToProseMirror does NOT
+//        throw and returns a SCHEMA-VALID document.
+//
+// GUARDRAIL: a P1/P2/P3/P4 failure means the generator FOUND A REAL CONVERTER
+// BUG. These invariants are kept STRICT — no it.fails / skip / weakening. A
+// failure prints the shrunk minimal counterexample for triage.
+// ---------------------------------------------------------------------------
+
+const SEED = 20250705;
+// The nested walk builds far heavier docs than the flat suite (each P1/P2 run
+// parses the emitted markdown through jsdom), so keep the run count moderate to
+// hold runtime and worker memory in budget while still exercising deep
+// structures. P4 (cheap string parsing) runs at a higher count below.
+const NUM_RUNS = 100;
+
+const pmToMd = (doc: unknown): string => convertProseMirrorToMarkdown(doc);
+const mdToPm = (md: string): Promise<any> => markdownToProseMirror(md);
+
+async function roundTrip(doc: unknown): Promise<{ md1: string; md2: string; doc2: any }> {
+  const md1 = pmToMd(doc);
+  const doc2 = await mdToPm(md1);
+  const md2 = pmToMd(doc2);
+  return { md1, md2, doc2 };
+}
+
+describe('#351 nested generative round-trip — generator validity', () => {
+  it('every generated nested doc passes schema.nodeFromJSON(...).check()', () => {
+    // A nested generator that emits an invalid ProseMirror document is a
+    // GENERATOR bug — the ContentMatch walk must only produce schema-valid docs.
+    fc.assert(
+      fc.property(docArb, (doc) => {
+        schema.nodeFromJSON(doc).check(); // throws on an invalid doc
+        return true;
+      }),
+      { numRuns: NUM_RUNS * 2, seed: SEED },
+    );
+  });
+});
+
+// ── STATUS: P1/P2/P3/P4 all GREEN. The nested generator originally surfaced a
+// batch of real converter bugs; all were fixed in the serializer/parser (see the
+// #351 hand-off). For the record, the classes it found and that are now fixed:
+//   • Loose (multi-block) list items / task items / callouts / details bodies were
+//     joined with a single "\n", so every block after the first merged into the
+//     first paragraph on re-parse (silent content loss) — now blank-line separated.
+//   • Paragraph `textAlign` was dropped inside a TIGHT list item (no <p> host).
+//   • A nested codeBlock lost its trailing newline on the raw-HTML path.
+//   • Media (embed/video/youtube/drawio/excalidraw) inside `columns` churned a
+//     default `data-align` and coerced embed's numeric width/height to strings.
+//   • pageBreak / pageEmbed / subpages / transclusion were dropped when nested in
+//     blockquote / callout / details / list item (standalone-comment position).
+//   • Callouts nested in a list item or a blockquote (`  > [!type]` / `> > [!type]`)
+//     were re-parsed as plain blockquotes (prefix-unaware callout preprocessor).
+//   • Two adjacent sibling lists sharing a marker family (bulletList/taskList →
+//     `<ul>`; orderedList → `<ol>`) merged into one list on re-parse — and for the
+//     cross-type case (taskList beside bulletList) the merged `<ul>` LOST every
+//     taskItem checkbox. The serializer now emits a `<!-- -->` separator between
+//     such adjacent lists (markdown-converter.ts renderBlockChildren), so they stay
+//     distinct and round-trip; the generator therefore emits them freely again.
+describe('#351 nested generative round-trip — properties', () => {
+  it('P1 — semantic round-trip: docsCanonicallyEqual(mdToPm(pmToMd(d)), d)', async () => {
+    await fc.assert(
+      fc.asyncProperty(docArb, async (doc) => {
+        const { doc2 } = await roundTrip(doc);
+        if (!docsCanonicallyEqual(doc2, doc)) {
+          const div = firstDivergence(
+            JSON.parse(JSON.stringify(canonicalizeContent(doc2))),
+            JSON.parse(JSON.stringify(canonicalizeContent(doc))),
+          );
+          throw new Error(
+            `P1 divergence @ ${div?.path}: got=${JSON.stringify(div?.a)} want=${JSON.stringify(div?.b)}`,
+          );
+        }
+      }),
+      { numRuns: NUM_RUNS, seed: SEED },
+    );
+  });
+
+  it('P2 — byte fixpoint: pmToMd(mdToPm(pmToMd(d))) === pmToMd(d)', async () => {
+    await fc.assert(
+      fc.asyncProperty(docArb, async (doc) => {
+        const { md1, md2 } = await roundTrip(doc);
+        expect(md2).toBe(md1);
+      }),
+      { numRuns: NUM_RUNS, seed: SEED },
+    );
+  });
+
+  it('P3 — totality: neither converter throws', async () => {
+    await fc.assert(
+      fc.asyncProperty(docArb, async (doc) => {
+        await roundTrip(doc);
+      }),
+      { numRuns: NUM_RUNS, seed: SEED },
+    );
+  });
+});
+
+// ---------------------------------------------------------------------------
+// P4 — parser fuzz. Independent of the doc generator: for ANY input string the
+// PARSER (markdownToProseMirror) must be TOTAL — never throw — and must always
+// return a schema-valid document. The corpus mixes raw unicode strings with
+// strings assembled from markdown-significant fragments (headings, list bullets,
+// fences, pipes, thematic breaks, HTML-ish snippets) to probe the block/inline
+// parsers on hostile but plausible input.
+// ---------------------------------------------------------------------------
+
+const mdFragmentArb: fc.Arbitrary<string> = fc.constantFrom(
+  '# ', '## ', '### ###', '- ', '* ', '+ ', '1. ', '> ', '>> ',
+  '```', '```js', '~~~', '---', '***', '___', '| a | b |', '|---|---|',
+  '[link](http://x)', '![img](http://x)', '**', '__', '~~', '`code`',
+  '<div>', '</div>', '<b>', '<!-- c -->', '<table>', '<br>', '&amp;',
+  '\t', '\n', '  ', '\\', '^[fn]', '[^1]:', '- [ ] ', '- [x] ',
+  '$$', '$x$', ':::', '{.class}', '\u0000', '\uFEFF', '😀', 'مرحبا',
+);
+
+// Full-unicode strings (fast-check v4 replaced fullUnicodeString with the
+// `unit: 'binary'` string option, which draws over the whole code-point range).
+const fullUnicodeStringArb = (max?: number) =>
+  fc.string({ unit: 'binary', ...(max !== undefined ? { maxLength: max } : {}) });
+
+const assembledMarkdownArb: fc.Arbitrary<string> = fc
+  .array(fc.oneof(mdFragmentArb, fc.string(), fullUnicodeStringArb(8)), {
+    minLength: 1,
+    maxLength: 12,
+  })
+  .map((parts) => parts.join(''));
+
+const parserInputArb: fc.Arbitrary<string> = fc.oneof(
+  { weight: 2, arbitrary: fc.string() },
+  { weight: 2, arbitrary: fullUnicodeStringArb() },
+  { weight: 3, arbitrary: assembledMarkdownArb },
+  { weight: 1, arbitrary: fc.array(mdFragmentArb, { minLength: 1, maxLength: 8 }).map((p) => p.join('\n')) },
+);
+
+describe('#351 parser fuzz — totality on arbitrary input (P4)', () => {
+  it('P4 — markdownToProseMirror never throws and always returns a schema-valid doc', async () => {
+    await fc.assert(
+      fc.asyncProperty(parserInputArb, async (s) => {
+        let result: any;
+        try {
+          result = await mdToPm(s);
+        } catch (e: any) {
+          throw new Error(`P4 parser THREW on input ${JSON.stringify(s)}: ${e?.message ?? e}`);
+        }
+        try {
+          schema.nodeFromJSON(result).check();
+        } catch (e: any) {
+          throw new Error(
+            `P4 parser produced an INVALID doc for input ${JSON.stringify(s)}: ${e?.message ?? e}\n` +
+              `doc=${JSON.stringify(result).slice(0, 600)}`,
+          );
+        }
+      }),
+      { numRuns: NUM_RUNS * 2, seed: SEED },
+    );
+  });
+});

packages/prosemirror-markdown/test/markdown-converter-gaps.test.ts

@@ -374,7 +374,9 @@ describe('converter gap coverage — emission branches (specs 1–11)', () => {
         ],
       }),
     );
-    expect(out).toBe('- [ ] top\n  - child');
+    // Block children of a task item are blank-line separated (loose list) per the
+    // #351 fix; the sublist stays at the fixed 2-column continuation indent.
+    expect(out).toBe('- [ ] top\n\n  - child');
   });
 
   // 10. A bulletList inside a blockquote: each list line independently prefixed.

packages/prosemirror-markdown/test/markdown-converter.test.ts

@@ -198,7 +198,11 @@ describe('convertProseMirrorToMarkdown', () => {
         }),
       );
       // First line carries the marker; the nested list is indented 2 columns.
-      expect(out).toBe('- parent\n  - child');
+      // Block children of a list item are separated by a BLANK line (loose list):
+      // this is the #351 fix — a single "\n" let a following block merge into the
+      // first paragraph on re-parse (silent content loss). The blank line stays
+      // inside the item, so the sublist remains nested at the 2-col marker column.
+      expect(out).toBe('- parent\n\n  - child');
     });
 
     it('nested ordered list indents by the wider 3-col marker width', () => {
@@ -219,8 +223,9 @@ describe('convertProseMirrorToMarkdown', () => {
           ],
         }),
       );
-      // "1. " is 3 columns wide, so the continuation indent is 3 spaces.
-      expect(out).toBe('1. parent\n   1. child');
+      // "1. " is 3 columns wide, so the continuation indent is 3 spaces. Block
+      // children are blank-line separated (loose list) per the #351 fix.
+      expect(out).toBe('1. parent\n\n   1. child');
     });
   });
 
@@ -539,11 +544,12 @@ describe('convertProseMirrorToMarkdown', () => {
       );
 
       // The 10th marker is the 4-column "10. "; the nested sublist line must be
-      // indented exactly 4 spaces (prefix.length 3 + 1), NOT 3.
-      expect(out).toContain('10. j\n    1. x');
+      // indented exactly 4 spaces (prefix.length 3 + 1), NOT 3. Block children are
+      // blank-line separated (loose list) per the #351 fix.
+      expect(out).toContain('10. j\n\n    1. x');
       // Guard against the off-by-one (3-space) regression that would re-parse
       // the sublist as loose/sibling content on import.
-      expect(out).not.toContain('10. j\n   1. x');
+      expect(out).not.toContain('10. j\n\n   1. x');
       // And the single-digit items keep the narrower 3-column marker (no body
       // continuation here, but the marker itself must stay "1. ".."9. ").
       expect(out.startsWith('1. a\n2. b\n')).toBe(true);
@@ -580,17 +586,18 @@ describe('convertProseMirrorToMarkdown', () => {
           content: [para(text('line1')), para(text('line2'))],
         }),
       );
-      // NOTE(review): the spec predicted ':::warning\nline1\n\nline2\n:::' (a
-      // The converter joins the callout's rendered children with a single '\n'
-      // and emits an Obsidian-native callout: a `> [!type]` opener plus one
-      // `>`-prefixed body line per content line. We pin the lowercasing
-      // (WARNING -> warning) and the multi-child join.
-      expect(out).toBe('> [!warning]\n> line1\n> line2');
+      // The converter emits an Obsidian-native callout: a `> [!type]` opener plus
+      // one `>`-prefixed body line per content line. Block children are separated
+      // by a blank `>` line (#351 fix): a single '\n' let the two paragraphs merge
+      // into one on re-parse. We pin the lowercasing (WARNING -> warning) and the
+      // blank-line-separated multi-child join.
+      expect(out).toBe('> [!warning]\n> line1\n>\n> line2');
       // The type is lowercased (an uppercase `[!WARNING]` would not re-import).
       expect(out.startsWith('> [!warning]\n')).toBe(true);
       expect(out).not.toContain('[!WARNING]');
-      // Both paragraph children are present, each blockquote-prefixed.
-      expect(out).toContain('> line1\n> line2');
+      // Both paragraph children are present, each blockquote-prefixed, blank-`>`
+      // separated so they stay distinct paragraphs on re-parse.
+      expect(out).toContain('> line1\n>\n> line2');
     });
 
     // Spec 4 — blockquote per-line prefixer over a multi-line nested callout.
@@ -607,13 +614,11 @@ describe('convertProseMirrorToMarkdown', () => {
           ],
         }),
       );
-      // NOTE(review): the spec predicted '> :::info\n> a\n>\n> b\n> :::',
-      // assuming the nested callout body contains a blank line between 'a' and
-      // The nested callout renders as an Obsidian callout '> [!info]\n> a\n> b'
-      // (single-'\n' join, no blank line). The outer blockquote prefixer then
-      // prefixes each of those lines with '> ' again, yielding a doubly-nested
-      // blockquote — the realistic per-line-prefix loop over a multi-line child.
-      expect(out).toBe('> > [!info]\n> > a\n> > b');
+      // The nested callout renders as an Obsidian callout '> [!info]\n> a\n>\n> b'
+      // (blank-`>` separated children per the #351 fix). The outer blockquote
+      // prefixer then prefixes each of those lines with '> ' again, yielding a
+      // doubly-nested blockquote — the per-line-prefix loop over a multi-line child.
+      expect(out).toBe('> > [!info]\n> > a\n> >\n> > b');
       // Every produced line carries the '> ' prefix (no line escapes to col 0).
       for (const line of out.split('\n')) {
         expect(line.startsWith('>')).toBe(true);