fix(#348 review round-2 F5-F6): index page_access(workspace_id) + test the workspace-cache bust

Both are direct consequences of the round-1 F1 fix (uncaching hasRestrictedPagesInWorkspace): - F5: that EXISTS(SELECT 1 FROM page_access WHERE workspace_id=?) now runs per-request on every whole-workspace list endpoint (global search + suggest, favorites, notifications, recent, created-by), and page_access only had a space_id index → a seq scan in the common zero-restriction case. Added idx_page_access_workspace_id to the perf migration (up + down) so it's an index-only existence probe. - F6: the DomainMiddleware workspace cache invalidation was untested — the int-spec passed `{}` for cacheManager, so bustWorkspaceCache's `del` threw into its own try/catch and never ran. Added a Map-backed cache double with a working del and two tests: updateSetting busts WORKSPACE_SELF_HOSTED; updateSharingSettings busts WORKSPACE_SELF_HOSTED + WORKSPACE_BY_HOST(hostname). A missed/mismatched bust key now fails the suite instead of letting a stale security-relevant workspace row (enforceSso/status) outlive the mutation. Gate: server tsc 0; workspace-repo-update-setting + page-permission-workspace-filter int-specs pass on real Postgres (the new index applies via global-setup). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(#348 review F1-F4): uncache the workspace-restriction gate + int-spec + docs
2026-07-06 04:10:12 +03:00 · 2026-07-06 04:10:12 +03:00 · 2026-07-06 04:10:12 +03:00
28 changed files with 726 additions and 886 deletions
@@ -1,3 +1,10 @@
 export const HISTORY_INTERVAL = 5 * 60 * 1000;
 export const HISTORY_FAST_INTERVAL = 60 * 1000;
 export const HISTORY_FAST_THRESHOLD = 5 * 60 * 1000;
+
+// #348 — debounce window for the per-page RAG re-embed job. Repeated saves
+// within this window collapse to a single delayed job (coalesced by a stable
+// jobId), so active editing does not pile up expensive re-embeds (external API
+// + page_embeddings rewrite, concurrency 1). The worker reads the CURRENT page
+// state at run time, so the last content within the window wins.
+export const EMBED_DEBOUNCE_MS = 30 * 1000;
@@ -431,7 +431,17 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
  it('uses the canonical page.id (not the slugId doc name) for post-store side effects (#260)', async () => {
    const SLUG = 'slug-1'; // persistedHumanPage.slugId; findById resolves it
    const document = ydocFor(doc('NEW AGENT CONTENT'));
-    pageRepo.findById.mockResolvedValue(persistedHumanPage('NEW AGENT CONTENT'));
+    // #348 — the transclusion sync now runs only when the new OR the previously
+    // persisted content carries a transclusion-family node. Give the persisted
+    // (old) content a pageEmbed so the sync path is exercised and the #260
+    // UUID-vs-slugId contract asserted below is still verified.
+    pageRepo.findById.mockResolvedValue({
+      ...persistedHumanPage('NEW AGENT CONTENT'),
+      content: {
+        type: 'doc',
+        content: [{ type: 'pageEmbed', attrs: { sourcePageId: 'src-1' } }],
+      },
+    });
    pageHistoryRepo.findPageLastHistory.mockResolvedValue(null);

    // A `page.<slugId>` document name (the bug's smoking gun), agent store over
@@ -36,11 +36,13 @@ import {
 import { Page } from '@docmost/db/types/entity.types';
 import { CollabHistoryService } from '../services/collab-history.service';
 import {
+  EMBED_DEBOUNCE_MS,
  HISTORY_FAST_INTERVAL,
  HISTORY_FAST_THRESHOLD,
  HISTORY_INTERVAL,
 } from '../constants';
 import { TransclusionService } from '../../core/page/transclusion/transclusion.service';
+import { hasTransclusionFamilyNodes } from '../../core/page/transclusion/utils/transclusion-prosemirror.util';
 import { observeCollabStore } from '../../integrations/metrics/metrics.registry';

 /**
@@ -415,7 +417,18 @@ export class PersistenceExtension implements Extension {
      // Use the canonical page UUID (page.id), not the doc-name id, which may be
      // a slugId for a `page.<slugId>` doc (#260). The transclusion/reference
      // syncs write uuid-typed columns, so a slugId here threw Postgres 22P02.
-      await this.syncTransclusion(page.id, page.workspaceId, tiptapJson);
+      //
+      // #348 — skip the three sync SELECTs when neither the new content nor the
+      // previously-persisted content has any transclusion/reference/pageEmbed
+      // node: nothing to insert, and (the DB mirrors the old content) nothing to
+      // delete. Whenever either side has one, run the idempotent sync exactly as
+      // before so removals are still reconciled.
+      if (
+        hasTransclusionFamilyNodes(tiptapJson) ||
+        hasTransclusionFamilyNodes(page.content)
+      ) {
+        await this.syncTransclusion(page.id, page.workspaceId, tiptapJson);
+      }
    }

    if (page) {
@@ -431,7 +444,17 @@ export class PersistenceExtension implements Extension {
        (m) => m.entityId,
      );

-      if (userMentions.length > 0) {
+      // #348 — only enqueue when the mentioned-user set actually GAINED a member.
+      // The processor (processPageMention) already no-ops when every current
+      // mention was present before (newMentions.length === 0), so skipping the
+      // enqueue in that case is behavior-identical and avoids piling up no-op jobs
+      // on every save of a page that merely CONTAINS (unchanged) mentions.
+      const oldMentionedUserIdSet = new Set(oldMentionedUserIds);
+      const hasNewMentionedUser = userMentions.some(
+        (m) => !oldMentionedUserIdSet.has(m.entityId),
+      );
+
+      if (hasNewMentionedUser) {
        await this.notificationQueue.add(QueueJob.PAGE_MENTION_NOTIFICATION, {
          userMentions: userMentions.map((m) => ({
            userId: m.entityId,
@@ -446,12 +469,23 @@ export class PersistenceExtension implements Extension {
        } as IPageMentionNotificationJob);
      }

-      await this.aiQueue.add(QueueJob.PAGE_CONTENT_UPDATED, {
-        // Canonical UUID: the embedding reindex resolves pages by uuid, so a
-        // slugId here threw Postgres 22P02 invalid-uuid (#260).
-        pageIds: [page.id],
-        workspaceId: page.workspaceId,
-      });
+      await this.aiQueue.add(
+        QueueJob.PAGE_CONTENT_UPDATED,
+        {
+          // Canonical UUID: the embedding reindex resolves pages by uuid, so a
+          // slugId here threw Postgres 22P02 invalid-uuid (#260).
+          pageIds: [page.id],
+          workspaceId: page.workspaceId,
+        },
+        // #348 — coalesce re-embeds during active editing. A stable per-page
+        // jobId + delay means repeated saves within EMBED_DEBOUNCE_MS collapse
+        // to one delayed job instead of one expensive re-embed per save. The
+        // worker reads the current page state at run time, so last content wins.
+        // BullMQ forbids ':' in custom job ids (Redis key separator), so '-' is
+        // used; page.id is a UUID, so the id is unique per page. removeOnComplete
+        // (queue.module) frees the id after each run so the next window re-arms.
+        { jobId: `embed-${page.id}`, delay: EMBED_DEBOUNCE_MS },
+      );

      await this.enqueuePageHistory(page, lastUpdatedSource);
    }
@@ -220,6 +220,13 @@ export class RedisSyncExtension<TCE extends CustomEvents> implements Extension {
  };

  async maintainLock(documentName: string) {
+    // #348 — clear any existing timer for this document before installing a new
+    // one. Without this, a second maintainLock for the same document (a
+    // reload-without-unload) overwrites this.locks[documentName] and leaks the
+    // previous interval, which keeps firing SET forever with no way to clear it.
+    if (this.locks[documentName]) {
+      clearInterval(this.locks[documentName]);
+    }
    this.locks[documentName] = setInterval(() => {
      this.pub.set(
        this.getKey(documentName),
@@ -4,8 +4,21 @@ export const CacheKey = {
    `perm:space-roles:${userId}:${spaceId}`,
  PAGE_CAN_EDIT: (userId: string, pageId: string) =>
    `perm:can-edit:${userId}:${pageId}`,
+  // #348 — DomainMiddleware workspace resolution. Self-hosted resolves the single
+  // workspace (constant key); cloud resolves by the request subdomain (lowercased
+  // to match the case-insensitive `LOWER(hostname)` lookup). Every WorkspaceRepo
+  // mutator busts these, so staleness is bounded by both explicit invalidation and
+  // the short TTL below.
+  WORKSPACE_SELF_HOSTED: 'workspace:self-hosted',
+  WORKSPACE_BY_HOST: (subdomain: string) =>
+    `workspace:byhost:${subdomain.toLowerCase()}`,
 };

 // Permission caches dedupe repeated checks within and across short request bursts.
 // 5s keeps staleness on revocations bounded.
 export const PERMISSION_CACHE_TTL_MS = 5_000;
+
+// #348 — workspace row changes rarely; a short TTL bounds staleness of
+// security-relevant fields (enforceSso/enforceMfa/status) even if an explicit
+// bust is ever missed, while still removing the per-request workspace query.
+export const WORKSPACE_CACHE_TTL_MS = 15_000;
@@ -1,13 +1,42 @@
-import { Injectable, NestMiddleware, NotFoundException } from '@nestjs/common';
+import { Inject, Injectable, NestMiddleware } from '@nestjs/common';
 import { FastifyRequest, FastifyReply } from 'fastify';
+import { CACHE_MANAGER } from '@nestjs/cache-manager';
+import { Cache } from 'cache-manager';
 import { EnvironmentService } from '../../integrations/environment/environment.service';
 import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo';
+import { Workspace } from '@docmost/db/types/entity.types';
+import { withCache } from '../helpers/with-cache';
+import { CacheKey, WORKSPACE_CACHE_TTL_MS } from '../helpers/cache-keys';
+
+// #348 — timestamptz columns on the workspace row. The cache store (Keyv/Redis)
+// JSON-serializes values, so a cached workspace comes back with these fields as
+// ISO strings. Reviving them to Date keeps the cached path byte-identical to the
+// direct DB path (postgres.js returns Date), so nothing downstream can observe a
+// cache hit vs miss. Idempotent: `new Date(date)` on an already-Date value is a
+// no-op-equivalent. Keep in sync with the workspace timestamptz columns.
+const WORKSPACE_DATE_FIELDS: Array<keyof Workspace> = [
+  'createdAt',
+  'updatedAt',
+  'deletedAt',
+  'trialEndAt',
+];
+
+function reviveWorkspaceDates(workspace: Workspace): Workspace {
+  for (const field of WORKSPACE_DATE_FIELDS) {
+    const value = workspace[field];
+    if (value != null) {
+      (workspace as any)[field] = new Date(value as any);
+    }
+  }
+  return workspace;
+}

@Injectable()
 export class DomainMiddleware implements NestMiddleware {
  constructor(
    private workspaceRepo: WorkspaceRepo,
    private environmentService: EnvironmentService,
+    @Inject(CACHE_MANAGER) private readonly cacheManager: Cache,
  ) {}
  async use(
    req: FastifyRequest['raw'],
@@ -15,13 +44,21 @@ export class DomainMiddleware implements NestMiddleware {
    next: () => void,
  ) {
    if (this.environmentService.isSelfHosted()) {
-      const workspace = await this.workspaceRepo.findFirst();
+      // #348 — cache the single-workspace lookup that runs on every request.
+      // Invalidated by every WorkspaceRepo mutator (see bustWorkspaceCache).
+      const workspace = await withCache(
+        this.cacheManager,
+        CacheKey.WORKSPACE_SELF_HOSTED,
+        WORKSPACE_CACHE_TTL_MS,
+        () => this.workspaceRepo.findFirst(),
+      );
      if (!workspace) {
        //throw new NotFoundException('Workspace not found');
        (req as any).workspaceId = null;
        return next();
      }

+      reviveWorkspaceDates(workspace);
      // TODO: unify
      (req as any).workspaceId = workspace.id;
      (req as any).workspace = workspace;
@@ -29,13 +66,21 @@ export class DomainMiddleware implements NestMiddleware {
      const header = req.headers.host;
      const subdomain = header.split('.')[0];

-      const workspace = await this.workspaceRepo.findByHostname(subdomain);
+      // #348 — cache per-subdomain workspace resolution. Keyed by subdomain (the
+      // hostname column); busted per hostname by every WorkspaceRepo mutator.
+      const workspace = await withCache(
+        this.cacheManager,
+        CacheKey.WORKSPACE_BY_HOST(subdomain),
+        WORKSPACE_CACHE_TTL_MS,
+        () => this.workspaceRepo.findByHostname(subdomain),
+      );

      if (!workspace) {
        (req as any).workspaceId = null;
        return next();
      }

+      reviveWorkspaceDates(workspace);
      (req as any).workspaceId = workspace.id;
      (req as any).workspace = workspace;
    }
@@ -51,7 +51,21 @@ export class JwtStrategy extends PassportStrategy(Strategy, 'jwt') {
      throw new UnauthorizedException();
    }

-    const workspace = await this.workspaceRepo.findById(payload.workspaceId);
+    // #348 — reuse the workspace DomainMiddleware already loaded for this request
+    // instead of re-querying it. `validate()` above has confirmed
+    // `req.raw.workspaceId === payload.workspaceId` (or that it is unset), and the
+    // middleware sets `req.raw.workspace` alongside `req.raw.workspaceId` from the
+    // SAME workspace row, so when the ids match this is that row. NOTE it is the
+    // middleware's `selectAll` object (a superset of the fallback `findById` base
+    // fields — it also carries licenseKey/auditRetentionDays); that is harmless
+    // here because every consumer reads this workspace via the AuthWorkspace
+    // decorator, which already preferred `req.raw.workspace` (the selectAll object)
+    // over `req.user.workspace` before this change. Fall back to the query if the
+    // middleware did not populate it (a path that bypasses DomainMiddleware).
+    const workspace =
+      req.raw.workspace && req.raw.workspaceId === payload.workspaceId
+        ? req.raw.workspace
+        : await this.workspaceRepo.findById(payload.workspaceId);

    if (!workspace) {
      throw new UnauthorizedException();
@@ -38,6 +38,8 @@ export class FavoriteService {
        await this.pagePermissionRepo.filterAccessiblePageIds({
          pageIds: result.items,
          userId,
+          // #348 — favorites load at app-start; enable the workspace short-circuit.
+          workspaceId,
        });
      const accessibleSet = new Set(accessibleIds);
      result.items = result.items.filter((id) => accessibleSet.has(id));
@@ -125,6 +127,8 @@ export class FavoriteService {
        await this.pagePermissionRepo.filterAccessiblePageIds({
          pageIds,
          userId,
+          // #348 — workspace-level short-circuit for the favorites list.
+          workspaceId,
        });
      accessiblePageSet = new Set(accessibleIds);
    }
@@ -23,7 +23,12 @@ export class NotificationController {
    @Body() dto: ListNotificationsDto,
    @AuthUser() user: User,
  ) {
-    return this.notificationService.findByUserId(user.id, dto, dto.type);
+    return this.notificationService.findByUserId(
+      user.id,
+      dto,
+      dto.type,
+      user.workspaceId,
+    );
  }

  @HttpCode(HttpStatus.OK)
@@ -45,6 +45,7 @@ export class NotificationService {
    userId: string,
    pagination: PaginationOptions,
    type: NotificationTab = 'all',
+    workspaceId?: string | null,
  ) {
    const result = await this.notificationRepo.findByUserId(
      userId,
@@ -61,6 +62,8 @@ export class NotificationService {
        await this.pagePermissionRepo.filterAccessiblePageIds({
          pageIds,
          userId,
+          // #348 — notifications list; enable the workspace short-circuit.
+          workspaceId,
        });
      const accessibleSet = new Set(accessiblePageIds);

@@ -446,7 +446,11 @@ export class PageController {
      );
    }

-    return this.pageService.getRecentPages(user.id, pagination);
+    return this.pageService.getRecentPages(
+      user.id,
+      pagination,
+      user.workspaceId,
+    );
  }

  @HttpCode(HttpStatus.OK)
@@ -469,7 +473,13 @@ export class PageController {
      }
    }

-    return this.pageService.getCreatedByPages(targetUserId, user.id, pagination, dto.spaceId);
+    return this.pageService.getCreatedByPages(
+      targetUserId,
+      user.id,
+      pagination,
+      dto.spaceId,
+      user.workspaceId,
+    );
  }

  @HttpCode(HttpStatus.OK)
@@ -1165,6 +1165,7 @@ export class PageService {
  async getRecentPages(
    userId: string,
    pagination: PaginationOptions,
+    workspaceId?: string | null,
  ): Promise<CursorPaginationResult<Page>> {
    const result = await this.pageRepo.getRecentPages(userId, pagination);

@@ -1174,6 +1175,8 @@ export class PageService {
        await this.pagePermissionRepo.filterAccessiblePageIds({
          pageIds,
          userId,
+          // #348 — cross-space "recent"; enable the workspace short-circuit.
+          workspaceId,
        });
      const accessibleSet = new Set(accessibleIds);
      result.items = result.items.filter((p) => accessibleSet.has(p.id));
@@ -1187,6 +1190,7 @@ export class PageService {
    requestingUserId: string,
    pagination: PaginationOptions,
    spaceId?: string,
+    workspaceId?: string | null,
  ): Promise<CursorPaginationResult<Page>> {
    const result = await this.pageRepo.getCreatedByPages(
      creatorId,
@@ -1201,6 +1205,9 @@ export class PageService {
        await this.pagePermissionRepo.filterAccessiblePageIds({
          pageIds,
          userId: requestingUserId,
+          spaceId,
+          // #348 — enable the workspace short-circuit when not space-scoped.
+          workspaceId,
        });
      const accessibleSet = new Set(accessibleIds);
      result.items = result.items.filter((p) => accessibleSet.has(p.id));
@@ -93,6 +93,41 @@ function collectNodes<T>(
  return Array.from(byKey.values());
 }

+/**
+ * #348 — cheap early-exit probe: does this doc contain ANY node the transclusion
+ * syncs care about (`transclusionSource` / `transclusionReference` / `pageEmbed`)?
+ * Lets the collab store skip the three sync SELECTs when neither the previous nor
+ * the new content has any such node — there is nothing to insert, and (since the
+ * DB mirrors the previously-persisted content) nothing to delete. Walks once and
+ * short-circuits on the first match; uses the same depth ceiling as the
+ * collectors. Deliberately does NOT skip `transclusionSource` subtrees: it only
+ * answers "any node present?", so descending everywhere is strictly conservative
+ * (it can never wrongly report "none").
+ */
+export function hasTransclusionFamilyNodes(doc: unknown): boolean {
+  const visit = (node: any, depth: number): boolean => {
+    if (!node || typeof node !== 'object') return false;
+    if (depth > MAX_PM_WALK_DEPTH) return false;
+
+    if (
+      node.type === TRANSCLUSION_TYPE ||
+      node.type === REFERENCE_TYPE ||
+      node.type === PAGE_EMBED_TYPE
+    ) {
+      return true;
+    }
+
+    if (Array.isArray(node.content)) {
+      for (const child of node.content) {
+        if (visit(child, depth + 1)) return true;
+      }
+    }
+    return false;
+  };
+
+  return visit(doc, 0);
+}
+
 /**
 * Walks a ProseMirror JSON document and returns one snapshot per top-level
 * `transclusion` node. Does not recurse into transclusions (schema disallows
@@ -155,6 +155,8 @@ export class SearchService {
          pageIds,
          userId: opts.userId,
          spaceId: searchParams.spaceId,
+          // #348 — enables the workspace-level short-circuit when not space-scoped.
+          workspaceId: opts.workspaceId,
        });
      const accessibleSet = new Set(accessibleIds);
      results = results.filter((r: any) => accessibleSet.has(r.id));
@@ -266,6 +268,8 @@ export class SearchService {
          await this.pagePermissionRepo.filterAccessiblePageIds({
            pageIds,
            userId,
+            // #348 — workspace-level short-circuit for the suggest path.
+            workspaceId,
          });
        const accessibleSet = new Set(accessibleIds);
        pages = pages.filter((p) => accessibleSet.has(p.id));
@@ -0,0 +1,118 @@
+import { type Kysely, sql } from 'kysely';
+
+/**
+ * #348 — targeted hot-path indexes.
+ *
+ * 1. GIN trigram indexes for `/search/suggest`. That endpoint runs a
+ *    leading-wildcard `LOWER(f_unaccent(col)) LIKE '%q%'` per keystroke, which
+ *    is a sequential scan without a trigram index. The index EXPRESSIONS below
+ *    are `LOWER(f_unaccent(title|name))`, matching the predicates in
+ *    search.service.ts exactly so the planner uses them (verified with EXPLAIN:
+ *    the suggest predicate resolves to a Bitmap Index Scan on these indexes).
+ *
+ *    IMMUTABLE-wrapper fix (required for the index to build): `f_unaccent` was
+ *    defined as `SELECT unaccent('unaccent', $1)` (the two-arg, dictionary-named
+ *    unaccent). That body CANNOT be used in an index expression: when Postgres
+ *    inlines the IMMUTABLE SQL wrapper while building the index it fails to
+ *    resolve the two-arg call (`function unaccent(unknown, text) does not exist`,
+ *    the `'unaccent'` literal loses its regdictionary coercion). The single-arg
+ *    `unaccent($1)` is the same operation (the default text-search dictionary IS
+ *    `unaccent`; verified byte-equal on accented samples), and — crucially —
+ *    SCHEMA-QUALIFIED as `public.unaccent($1)` it inlines cleanly, so the index
+ *    builds. We therefore `CREATE OR REPLACE` `f_unaccent` to the qualified
+ *    single-arg body. This is output-identical for every existing caller (the
+ *    tsvector trigger, the main `tsv @@` search, and the suggest LIKE), so no
+ *    reindex/backfill is needed; `down()` restores the original two-arg body.
+ *    (The `unaccent` extension is installed in `public` in this codebase, which
+ *    is why `public.unaccent` is the correct qualification.)
+ *
+ * 2. Composite indexes for two ORDER-BY-only-on-id queries that currently sort
+ *    on top of a created_at index:
+ *    - page_history: `findPageHistoryByPageId` does WHERE page_id ORDER BY id
+ *      DESC, but only `(page_id, created_at DESC)` exists → extra sort.
+ *    - comments: `findPageComments` does WHERE page_id ORDER BY id ASC, but only
+ *      `(page_id)` exists → extra sort.
+ *
+ * DEPLOY-TIME LOCK WARNING: these are plain (non-CONCURRENT) CREATE INDEX
+ * statements — CONCURRENTLY is impossible because Kysely runs each migration in a
+ * transaction. They take a SHARE lock that BLOCKS writes (INSERT/UPDATE/DELETE) on
+ * pages/users/groups/comments/page_history for the duration of the build. The two
+ * GIN trigram builds on pages.title / users.name are the slow ones and can take
+ * minutes on a large tenant → a write-outage window during the deploy migration.
+ * For large installations, run this migration in a maintenance window, or build
+ * the trigram indexes out-of-band with CREATE INDEX CONCURRENTLY before deploying
+ * (then this migration's `IF NOT EXISTS` is a no-op). Small/typical tenants are
+ * unaffected.
+ */
+export async function up(db: Kysely<any>): Promise<void> {
+  // Index-compatible, output-identical redefinition of f_unaccent (see header).
+  await sql`
+    CREATE OR REPLACE FUNCTION f_unaccent(text)
+      RETURNS text
+      LANGUAGE sql
+      IMMUTABLE PARALLEL SAFE STRICT
+    AS $func$
+      SELECT public.unaccent($1);
+    $func$
+  `.execute(db);
+
+  // Search-suggest trigram indexes. Expressions match search.service.ts.
+  await sql`
+    CREATE INDEX IF NOT EXISTS idx_pages_title_trgm
+      ON pages USING gin ((LOWER(f_unaccent(title))) gin_trgm_ops)
+  `.execute(db);
+
+  await sql`
+    CREATE INDEX IF NOT EXISTS idx_users_name_trgm
+      ON users USING gin ((LOWER(f_unaccent(name))) gin_trgm_ops)
+  `.execute(db);
+
+  await sql`
+    CREATE INDEX IF NOT EXISTS idx_groups_name_trgm
+      ON groups USING gin ((LOWER(f_unaccent(name))) gin_trgm_ops)
+  `.execute(db);
+
+  // page_history: WHERE page_id ORDER BY id DESC (findPageHistoryByPageId).
+  await sql`
+    CREATE INDEX IF NOT EXISTS idx_page_history_page_id
+      ON page_history (page_id, id DESC)
+  `.execute(db);
+
+  // comments: WHERE page_id ORDER BY id ASC (findPageComments).
+  await sql`
+    CREATE INDEX IF NOT EXISTS idx_comments_page_id_id
+      ON comments (page_id, id)
+  `.execute(db);
+
+  // page_access(workspace_id): #348 made hasRestrictedPagesInWorkspace uncached
+  // (F1 fix), so `EXISTS(SELECT 1 FROM page_access WHERE workspace_id=?)` now runs
+  // per-request on every whole-workspace list endpoint (global search + suggest,
+  // favorites, notifications, recent, created-by). page_access only had a
+  // space_id index → that EXISTS was a seq scan in the common zero-restriction
+  // case. This index makes it an index-only existence probe.
+  await sql`
+    CREATE INDEX IF NOT EXISTS idx_page_access_workspace_id
+      ON page_access (workspace_id)
+  `.execute(db);
+}
+
+export async function down(db: Kysely<any>): Promise<void> {
+  // Drop the expression indexes before restoring the function body.
+  await sql`DROP INDEX IF EXISTS idx_pages_title_trgm`.execute(db);
+  await sql`DROP INDEX IF EXISTS idx_users_name_trgm`.execute(db);
+  await sql`DROP INDEX IF EXISTS idx_groups_name_trgm`.execute(db);
+  await sql`DROP INDEX IF EXISTS idx_page_history_page_id`.execute(db);
+  await sql`DROP INDEX IF EXISTS idx_comments_page_id_id`.execute(db);
+  await sql`DROP INDEX IF EXISTS idx_page_access_workspace_id`.execute(db);
+
+  // Restore the original two-arg (dictionary-named) f_unaccent body.
+  await sql`
+    CREATE OR REPLACE FUNCTION f_unaccent(text)
+      RETURNS text
+      LANGUAGE sql
+      IMMUTABLE PARALLEL SAFE STRICT
+    AS $func$
+      SELECT unaccent('unaccent', $1);
+    $func$
+  `.execute(db);
+}
@@ -657,8 +657,9 @@ export class PagePermissionRepo {
    pageIds: string[];
    userId: string;
    spaceId?: string;
+    workspaceId?: string | null;
  }): Promise<string[]> {
-    const { pageIds, userId, spaceId } = opts;
+    const { pageIds, userId, spaceId, workspaceId } = opts;
    if (pageIds.length === 0) return [];

    if (spaceId) {
@@ -666,6 +667,17 @@ export class PagePermissionRepo {
      if (!hasRestrictions) {
        return pageIds;
      }
+    } else if (workspaceId) {
+      // #348 — whole-workspace callers (no spaceId: favorites, notifications,
+      // recent, created-by, global search) skip the recursive-ancestor CTE + anti
+      // -join entirely when the workspace has ZERO restricted pages. When any
+      // restriction DOES exist, fall through to the identical CTE below, so
+      // behavior is unchanged whenever restrictions are present.
+      const hasRestrictions =
+        await this.hasRestrictedPagesInWorkspace(workspaceId);
+      if (!hasRestrictions) {
+        return pageIds;
+      }
    }

    const results = await this.db
@@ -903,6 +915,39 @@ export class PagePermissionRepo {
    return Boolean(result?.exists);
  }

+  /**
+   * Workspace-level analogue of hasRestrictedPagesInSpace: does ANY page in the
+   * whole workspace carry a restriction? Lets whole-workspace access filters
+   * short-circuit the recursive-ancestor CTE when nothing is restricted at all.
+   *
+   * UNCACHED (like the sibling hasRestrictedPagesInSpace) — a single cheap
+   * `EXISTS(pageAccess WHERE workspaceId=?)` per call. This is an ACCESS-CONTROL
+   * gate on whole-workspace list endpoints, so it must never go stale: caching it
+   * (even 5s) reintroduced a leak the space-path never had — a concurrent
+   * whole-workspace read in the insert->commit window of the FIRST restricted page
+   * could re-populate `false` under withCache (read-then-set, no del-during-read
+   * guard) and override the insert bust, leaking that page to unauthorized users
+   * for up to the TTL (#348 review F1). An uncached EXISTS removes both the
+   * cache/DB asymmetry with hasRestrictedPagesInSpace and that race; the space
+   * path already accepts this exact per-call cost.
+   */
+  async hasRestrictedPagesInWorkspace(workspaceId: string): Promise<boolean> {
+    const result = await this.db
+      .selectNoFrom((eb) =>
+        eb
+          .exists(
+            eb
+              .selectFrom('pageAccess')
+              .select(sql`1`.as('one'))
+              .where('pageAccess.workspaceId', '=', workspaceId),
+          )
+          .as('exists'),
+      )
+      .executeTakeFirst();
+
+    return Boolean(result?.exists);
+  }
+
  /**
   * Given a list of parent page IDs, return which ones have at least one accessible child.
   * Efficient batch query for sidebar hasChildren calculation.
@@ -581,6 +581,9 @@ export class PageRepo {
    const query = this.db
      .selectFrom('pages')
      .select(this.baseFields)
+      // NOTE: `content` IS needed here — the trash UI reads page.content to render
+      // the deleted-page preview modal (trash.tsx handlePageClick ->
+      // TrashPageContentModal pageContent). Do NOT drop it (see #348 review F3).
      .select('content')
      .select((eb) => this.withSpace(eb))
      .select((eb) => this.withDeletedBy(eb))
@@ -1,4 +1,6 @@
-import { Injectable } from '@nestjs/common';
+import { Inject, Injectable } from '@nestjs/common';
+import { CACHE_MANAGER } from '@nestjs/cache-manager';
+import { Cache } from 'cache-manager';
 import { InjectKysely } from 'nestjs-kysely';
 import { KyselyDB, KyselyTransaction } from '../../types/kysely.types';
 import { dbOrTx } from '../../utils';
@@ -9,6 +11,7 @@ import {
 } from '@docmost/db/types/entity.types';
 import { ExpressionBuilder, sql } from 'kysely';
 import { DB, Workspaces } from '@docmost/db/types/db';
+import { CacheKey } from '../../../common/helpers/cache-keys';

 /**
 * Writable `settings.ai.provider` keys, enforced at this generic SQL layer. This
@@ -61,7 +64,34 @@ export class WorkspaceRepo {
    'temporaryNoteHours',
    'isScimEnabled',
  ];
-  constructor(@InjectKysely() private readonly db: KyselyDB) {}
+  constructor(
+    @InjectKysely() private readonly db: KyselyDB,
+    @Inject(CACHE_MANAGER) private readonly cacheManager: Cache,
+  ) {}
+
+  /**
+   * #348 — bust the DomainMiddleware workspace caches after any workspace write.
+   * Deletes BOTH the self-hosted (constant) key and the cloud per-hostname key so
+   * a single implementation covers either deployment mode (the irrelevant key is a
+   * harmless no-op). Best-effort: a cache error must never fail the write, and a
+   * missed bust is bounded by WORKSPACE_CACHE_TTL_MS. Note: a hostname RENAME only
+   * busts the NEW hostname's key (the row returned here carries the new hostname);
+   * the old key expires via TTL.
+   */
+  private async bustWorkspaceCache(
+    workspace?: Pick<Workspace, 'hostname'> | undefined,
+  ): Promise<void> {
+    try {
+      await this.cacheManager.del(CacheKey.WORKSPACE_SELF_HOSTED);
+      if (workspace?.hostname) {
+        await this.cacheManager.del(
+          CacheKey.WORKSPACE_BY_HOST(workspace.hostname),
+        );
+      }
+    } catch {
+      // cache is best-effort; TTL is the backstop
+    }
+  }

  async findById(
    workspaceId: string,
@@ -144,12 +174,14 @@ export class WorkspaceRepo {
    trx?: KyselyTransaction,
  ): Promise<Workspace> {
    const db = dbOrTx(this.db, trx);
-    return db
+    const workspace = await db
      .updateTable('workspaces')
      .set({ ...updatableWorkspace, updatedAt: new Date() })
      .where('id', '=', workspaceId)
      .returning(this.baseFields)
      .executeTakeFirst();
+    await this.bustWorkspaceCache(workspace);
+    return workspace;
  }

  async insertWorkspace(
@@ -157,11 +189,14 @@ export class WorkspaceRepo {
    trx?: KyselyTransaction,
  ): Promise<Workspace> {
    const db = dbOrTx(this.db, trx);
-    return db
+    const workspace = await db
      .insertInto('workspaces')
      .values(insertableWorkspace)
      .returning(this.baseFields)
      .executeTakeFirst();
+    // Bust the cached "not found" so a fresh install / new tenant is seen at once.
+    await this.bustWorkspaceCache(workspace);
+    return workspace;
  }

  async count(): Promise<number> {
@@ -203,7 +238,7 @@ export class WorkspaceRepo {
    trx?: KyselyTransaction,
  ) {
    const db = dbOrTx(this.db, trx);
-    return db
+    const workspace = await db
      .updateTable('workspaces')
      .set({
        settings: sql`COALESCE(settings, '{}'::jsonb)
@@ -214,6 +249,8 @@ export class WorkspaceRepo {
      .where('id', '=', workspaceId)
      .returning(this.baseFields)
      .executeTakeFirst();
+    await this.bustWorkspaceCache(workspace);
+    return workspace;
  }

  async updateAiSettings(
@@ -223,7 +260,7 @@ export class WorkspaceRepo {
    trx?: KyselyTransaction,
  ) {
    const db = dbOrTx(this.db, trx);
-    return db
+    const workspace = await db
      .updateTable('workspaces')
      .set({
        settings: sql`COALESCE(settings, '{}'::jsonb)
@@ -234,6 +271,8 @@ export class WorkspaceRepo {
      .where('id', '=', workspaceId)
      .returning(this.baseFields)
      .executeTakeFirst();
+    await this.bustWorkspaceCache(workspace);
+    return workspace;
  }

  /**
@@ -272,7 +311,7 @@ export class WorkspaceRepo {
          entries.flatMap(([k, v]) => [sql.lit(k), sql`${v}::text`]),
        )})`
      : sql`'{}'::jsonb`;
-    return db
+    const workspace = await db
      .updateTable('workspaces')
      .set({
        settings: sql`COALESCE(settings, '{}'::jsonb) || jsonb_build_object(
@@ -287,6 +326,8 @@ export class WorkspaceRepo {
      .where('id', '=', workspaceId)
      .returning(this.baseFields)
      .executeTakeFirst();
+    await this.bustWorkspaceCache(workspace);
+    return workspace;
  }

  /**
@@ -303,7 +344,7 @@ export class WorkspaceRepo {
    trx?: KyselyTransaction,
  ) {
    const db = dbOrTx(this.db, trx);
-    return db
+    const workspace = await db
      .updateTable('workspaces')
      .set({
        settings: sql`COALESCE(settings, '{}'::jsonb)
@@ -313,6 +354,8 @@ export class WorkspaceRepo {
      .where('id', '=', workspaceId)
      .returning(this.baseFields)
      .executeTakeFirst();
+    await this.bustWorkspaceCache(workspace);
+    return workspace;
  }

  async updateSharingSettings(
@@ -322,7 +365,7 @@ export class WorkspaceRepo {
    trx?: KyselyTransaction,
  ) {
    const db = dbOrTx(this.db, trx);
-    return db
+    const workspace = await db
      .updateTable('workspaces')
      .set({
        settings: sql`COALESCE(settings, '{}'::jsonb)
@@ -333,6 +376,8 @@ export class WorkspaceRepo {
      .where('id', '=', workspaceId)
      .returning(this.baseFields)
      .executeTakeFirst();
+    await this.bustWorkspaceCache(workspace);
+    return workspace;
  }

  async updateTemplateSettings(
@@ -342,7 +387,7 @@ export class WorkspaceRepo {
    trx?: KyselyTransaction,
  ) {
    const db = dbOrTx(this.db, trx);
-    return db
+    const workspace = await db
      .updateTable('workspaces')
      .set({
        settings: sql`COALESCE(settings, '{}'::jsonb)
@@ -353,6 +398,8 @@ export class WorkspaceRepo {
      .where('id', '=', workspaceId)
      .returning(this.baseFields)
      .executeTakeFirst();
+    await this.bustWorkspaceCache(workspace);
+    return workspace;
  }

 }
@@ -0,0 +1,113 @@
+import { Kysely } from 'kysely';
+import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo';
+import { GroupRepo } from '@docmost/db/repos/group/group.repo';
+import {
+  getTestDb,
+  destroyTestDb,
+  createWorkspace,
+  createSpace,
+  createUser,
+  createPage,
+} from './db';
+
+/**
+ * #348 — the whole-workspace access-filter short-circuit is an ACCESS-CONTROL
+ * path, so it must produce the SAME result as the full recursive-ancestor CTE.
+ *
+ * filterAccessiblePageIds({ workspaceId }) (no spaceId — the favorites /
+ * notifications / recent / created-by / global-search callers) skips the CTE only
+ * when the workspace has ZERO restricted pages. A page is "restricted &
+ * inaccessible" when it (or an ancestor) has a `pageAccess` row and the user has
+ * no matching `pagePermissions`. Driven against real Postgres, asserts:
+ *   1. zero restrictions -> short-circuit returns the full input set;
+ *   2. a restriction present -> the CTE runs and drops the page the user can't
+ *      reach while keeping the reachable ones (behavior unchanged);
+ *   3. inserting the FIRST pageAccess flips hasRestrictedPagesInWorkspace
+ *      false -> true immediately (the 0->1 transition — now uncached, no stale
+ *      window, review F1); it is scoped per workspace.
+ */
+describe('#348 filterAccessiblePageIds workspace short-circuit (real PG)', () => {
+  let db: Kysely<any>;
+  let repo: PagePermissionRepo;
+  let workspaceId: string;
+  let otherWorkspaceId: string;
+  let userId: string;
+  let spaceId: string;
+
+  beforeAll(async () => {
+    db = getTestDb();
+    // hasRestrictedPagesInWorkspace is now uncached, and no other cached
+    // permission path is exercised here, so a no-op cache stub suffices.
+    const cacheStub = {
+      get: async () => undefined,
+      set: async () => undefined,
+      del: async () => undefined,
+    } as never;
+    repo = new PagePermissionRepo(db, new GroupRepo(db), cacheStub);
+
+    const ws = await createWorkspace(db);
+    workspaceId = ws.id;
+    const other = await createWorkspace(db);
+    otherWorkspaceId = other.id;
+    const user = await createUser(db, workspaceId);
+    userId = user.id;
+    const space = await createSpace(db, workspaceId);
+    spaceId = space.id;
+  });
+
+  afterAll(async () => {
+    await destroyTestDb();
+  });
+
+  it('zero restrictions: short-circuit returns the full input set', async () => {
+    const p1 = await createPage(db, { workspaceId, spaceId });
+    const p2 = await createPage(db, { workspaceId, spaceId });
+
+    expect(await repo.hasRestrictedPagesInWorkspace(workspaceId)).toBe(false);
+
+    const ids = [p1.id, p2.id];
+    const filtered = await repo.filterAccessiblePageIds({
+      pageIds: ids,
+      userId,
+      workspaceId,
+    });
+    expect(new Set(filtered)).toEqual(new Set(ids));
+  });
+
+  it('a restriction present: filters out the page the user cannot reach', async () => {
+    const openPage = await createPage(db, { workspaceId, spaceId });
+    const restrictedPage = await createPage(db, { workspaceId, spaceId });
+
+    // Add a pageAccess row on restrictedPage with NO matching pagePermissions for
+    // `userId` → the CTE anti-join marks it inaccessible for this user.
+    await db
+      .insertInto('pageAccess')
+      .values({
+        pageId: restrictedPage.id,
+        workspaceId,
+        spaceId,
+        accessLevel: 'read',
+        creatorId: userId,
+      })
+      .execute();
+
+    // 0->1 transition is reflected immediately (uncached).
+    expect(await repo.hasRestrictedPagesInWorkspace(workspaceId)).toBe(true);
+
+    const filtered = await repo.filterAccessiblePageIds({
+      pageIds: [openPage.id, restrictedPage.id],
+      userId,
+      workspaceId,
+    });
+    expect(filtered).toContain(openPage.id);
+    expect(filtered).not.toContain(restrictedPage.id);
+  });
+
+  it('hasRestrictedPagesInWorkspace is scoped per workspace', async () => {
+    // The other workspace has no pageAccess rows → still false, unaffected by the
+    // restriction added above in `workspaceId`.
+    expect(await repo.hasRestrictedPagesInWorkspace(otherWorkspaceId)).toBe(
+      false,
+    );
+  });
+});
@@ -1,7 +1,25 @@
 import { Kysely } from 'kysely';
 import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo';
+import { CacheKey } from 'src/common/helpers/cache-keys';
 import { getTestDb, destroyTestDb, createWorkspace } from './db';

+// A minimal Map-backed cache double with a working `del` (the previous `{}` stub
+// made bustWorkspaceCache's `del` throw into its own try/catch, so the #348
+// invalidation was never actually exercised — review F6).
+function makeCacheDouble() {
+  const store = new Map<string, unknown>();
+  return {
+    store,
+    get: async (k: string) => store.get(k),
+    set: async (k: string, v: unknown) => {
+      store.set(k, v);
+    },
+    del: async (k: string) => {
+      store.delete(k);
+    },
+  };
+}
+
 /**
 * A — WorkspaceRepo.updateSetting jsonb-MERGE (the html-embed kill-switch
 * write-half). Setting a single top-level key must NOT clobber sibling
@@ -15,7 +33,9 @@ describe('WorkspaceRepo.updateSetting (jsonb merge) [integration]', () => {
  beforeAll(() => {
    db = getTestDb();
    // Repos are plain classes taking @InjectKysely() db — instantiate directly.
-    repo = new WorkspaceRepo(db as any);
+    // 2nd arg is CACHE_MANAGER (used only to bust the #348 workspace cache); a
+    // stub is fine here since bustWorkspaceCache is best-effort (try/catch).
+    repo = new WorkspaceRepo(db as any, {} as any);
  });

  afterAll(async () => {
@@ -58,3 +78,62 @@ describe('WorkspaceRepo.updateSetting (jsonb merge) [integration]', () => {
    expect(updated.settings).toEqual({ htmlEmbed: false });
  });
 });
+
+/**
+ * #348 F6 — the DomainMiddleware workspace cache (WORKSPACE_SELF_HOSTED /
+ * WORKSPACE_BY_HOST, 15s TTL) caches security-relevant fields (enforceSso/
+ * enforceMfa/status). Its correctness rests entirely on bustWorkspaceCache being
+ * called from every mutator. This exercises the real invalidation with a working
+ * cache double (not the {} stub, whose del throws-and-swallows): warm the cache
+ * like DomainMiddleware, mutate, and assert the busted key is gone so a stale
+ * workspace row can't outlive the mutation.
+ */
+describe('WorkspaceRepo bustWorkspaceCache invalidation [integration]', () => {
+  let db: Kysely<any>;
+
+  beforeAll(() => {
+    db = getTestDb();
+  });
+  afterAll(async () => {
+    await destroyTestDb();
+  });
+
+  it('updateSetting busts the self-hosted workspace cache key', async () => {
+    const cache = makeCacheDouble();
+    const repo = new WorkspaceRepo(db as any, cache as any);
+    const ws = await createWorkspace(db, { settings: {} });
+
+    // Warm the cache as DomainMiddleware would (self-hosted key).
+    cache.store.set(CacheKey.WORKSPACE_SELF_HOSTED, ws);
+    expect(cache.store.has(CacheKey.WORKSPACE_SELF_HOSTED)).toBe(true);
+
+    await repo.updateSetting(ws.id, 'htmlEmbed', true);
+
+    // The mutation must have invalidated the cached row.
+    expect(cache.store.has(CacheKey.WORKSPACE_SELF_HOSTED)).toBe(false);
+  });
+
+  it('updateSharingSettings busts the by-host workspace cache key too', async () => {
+    const cache = makeCacheDouble();
+    const repo = new WorkspaceRepo(db as any, cache as any);
+    const ws = await createWorkspace(db, { settings: {} });
+    // createWorkspace assigns a unique hostname; read it back for the by-host key.
+    const { hostname } = await db
+      .selectFrom('workspaces')
+      .select(['hostname'])
+      .where('id', '=', ws.id)
+      .executeTakeFirstOrThrow();
+
+    // Warm BOTH keys (self-hosted + by-host); the by-host bust needs the row's
+    // hostname, which the mutator returns from the DB.
+    cache.store.set(CacheKey.WORKSPACE_SELF_HOSTED, ws);
+    cache.store.set(CacheKey.WORKSPACE_BY_HOST(hostname as string), ws);
+
+    await repo.updateSharingSettings(ws.id, 'allowInvite', true);
+
+    expect(cache.store.has(CacheKey.WORKSPACE_SELF_HOSTED)).toBe(false);
+    expect(cache.store.has(CacheKey.WORKSPACE_BY_HOST(hostname as string))).toBe(
+      false,
+    );
+  });
+});
@@ -55,15 +55,10 @@ describe('stabilizePageFile — normalize-on-write fixpoint (SPEC §11)', () =>
    const file2 = await stabilizePageFile(doc2, meta);
    expect(file2).toBe(file1);

-    // 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"');
+    // 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"');
  });

  it('already-stable content is unchanged by the pass (idempotent)', async () => {
@@ -48,52 +48,6 @@ 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
@@ -393,15 +347,9 @@ 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)) {
@@ -651,34 +599,20 @@ export function convertProseMirrorToMarkdown(
        return processTaskItem(node);

      case "listItem":
-        // 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");
+        return nodeContent.map(processNode).join("\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. 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(
+        // quotes round-trip correctly.
+        return nodeContent
+          .map((n: any) =>
            processNode(n)
              .split("\n")
              .map((line: string) => (line.length ? `> ${line}` : ">"))
              .join("\n"),
-          );
-          prevBqType = n?.type;
-        }
-        return bqParts.join("\n>\n");
-      }
+          )
+          .join("\n>\n");

      case "horizontalRule":
        return "---";
@@ -853,12 +787,9 @@ 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 = 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")
+        const calloutBody = nodeContent
+          .map(processNode)
+          .join("\n")
          .split("\n")
          .map((l: string) => (l.length ? `> ${l}` : ">"))
          .join("\n");
@@ -878,10 +809,7 @@ export function convertProseMirrorToMarkdown(
        return `<summary>${renderInlineChildren(nodeContent)}</summary>\n\n`;

      case "detailsContent":
-        // 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`;
+        return `${nodeContent.map(processNode).join("\n")}\n`;

      case "mathInline": {
        // #293 canon #6: inline math serializes as Obsidian-native `$LaTeX$`
@@ -1410,19 +1338,11 @@ 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((child: any) =>
-              typeof child?.text === "string" ? child.text : "",
-            )
-            .join(""),
+            .map(processNode)
+            .join("")
+            .replace(/\n+$/, ""),
        );
        const cls = lang ? ` class="language-${escapeAttr(lang)}"` : "";
        return `<pre><code${cls}>${code}</code></pre>`;
@@ -1564,13 +1484,6 @@ 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.
@@ -1587,9 +1500,7 @@ export function convertProseMirrorToMarkdown(

  const processListItem = (item: any, prefix: string): string => {
    const itemContent = item.content || [];
-    // 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);
+    const childStrings = itemContent.map(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
@@ -1603,9 +1514,7 @@ export function convertProseMirrorToMarkdown(
    const checkbox = checked ? "[x]" : "[ ]";
    const prefix = `- ${checkbox}`;
    const itemContent = item.content || [];
-    // 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);
+    const childStrings = itemContent.map(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;
@@ -270,10 +270,9 @@ 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.
 */
-// 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+)\]/;
+const CALLOUT_BQ_OPEN_RE = /^>\s*\[!(\w+)\]/;
+/** Matches any blockquote continuation line (`>` … ). */
+const BLOCKQUOTE_LINE_RE = /^>/;
 /** Matches the start/end of a code fence (``` or ~~~), capturing the marker. */
 const CODE_FENCE_RE = /^(\s*)(`{3,}|~{3,})/;

@@ -403,47 +402,20 @@ 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 prefix = bqOpen[1];
-        const type = bqOpen[2].toLowerCase();
-        const cont = prefix + ">"; // a body line = prefix + the callout's own `>`
+        const type = bqOpen[1].toLowerCase();
        const bodyLines: string[] = [];
        let j = i + 1;
        for (; j < lines.length; j++) {
-          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?/, ""));
+          if (!BLOCKQUOTE_LINE_RE.test(lines[j])) break;
+          bodyLines.push(lines[j].replace(/^>\s?/, ""));
        }
        const inner = await transform(bodyLines);
        const renderedInner = await markedInstance.parse(inner);
-        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`,
-          );
-        }
+        out.push(
+          `\n<div data-type="callout" data-callout-type="${type}">${renderedInner}</div>\n`,
+        );
        i = j;
        continue;
      }
@@ -625,40 +597,6 @@ 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;
@@ -726,13 +664,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 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
+      // 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
      const div = document.createElement("div");
      if (parsed.name === "pagebreak") {
        div.setAttribute("data-type", "pageBreak");
@@ -742,18 +680,26 @@ function applyCommentDirectives(html: string): string {
          div.setAttribute("data-recursive", "true");
        }
      }
-      placeStandalone(comment, div, tag, leadingDivs);
+      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);
+      }
      continue;
    }

    if (parsed.name === "pageembed" || parsed.name === "transclusion") {
      // #293 canon #8 STANDALONE media. Like subpages/pagebreak: a lone comment
-      // 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
+      // 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
      const el = buildElement(
        parsed.name === "pageembed"
          ? pageEmbedToHtml({ sourcePageId: parsed.attrs.sourcePageId })
@@ -763,7 +709,12 @@ function applyCommentDirectives(html: string): string {
            }),
      );
      if (!el) continue; // defensive: builder always yields an element
-      placeStandalone(comment, el, tag, leadingDivs);
+      if (tag === "body") {
+        comment.replaceWith(el);
+      } else {
+        comment.remove();
+        leadingDivs.push(el);
+      }
      continue;
    }

@@ -855,36 +806,18 @@ function applyCommentDirectives(html: string): string {

    if (!parent) continue; // attrs comment must have an element parent
    if (parsed.name !== "attrs") continue; // unknown name -> inert
-    const align = parsed.attrs.textAlign;
    // #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();
+    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;
    }
+    // 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.
@@ -46,11 +46,7 @@ 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)}"`);
-  // 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.align) 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>`;
@@ -66,9 +62,7 @@ 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)}"`);
-  // "center" is the schema default -> omit (see videoToHtml rationale).
-  if (attrs.align && attrs.align !== "center")
-    parts.push(`data-align="${escapeAttr(attrs.align)}"`);
+  if (attrs.align) parts.push(`data-align="${escapeAttr(attrs.align)}"`);
  return `<div ${parts.join(" ")}></div>`;
 }

@@ -103,9 +97,7 @@ 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)}"`);
-  // "center" is the schema default -> omit (see videoToHtml rationale).
-  if (attrs.align && attrs.align !== "center")
-    parts.push(`data-align="${escapeAttr(attrs.align)}"`);
+  if (attrs.align) parts.push(`data-align="${escapeAttr(attrs.align)}"`);
  if (attrs.attachmentId)
    parts.push(`data-attachment-id="${escapeAttr(attrs.attachmentId)}"`);
  return `<div ${parts.join(" ")}></div>`;
@@ -118,18 +110,10 @@ export function embedToHtml(attrs: Record<string, any>): string {
    `data-src="${escapeAttr(attrs.src ?? "")}"`,
    `data-provider="${escapeAttr(attrs.provider ?? "")}"`,
  ];
-  // "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)
+  if (attrs.align) parts.push(`data-align="${escapeAttr(attrs.align)}"`);
+  if (attrs.width != null)
    parts.push(`data-width="${escapeAttr(attrs.width)}"`);
-  if (attrs.height != null && attrs.height !== 600)
+  if (attrs.height != null)
    parts.push(`data-height="${escapeAttr(attrs.height)}"`);
  return `<div ${parts.join(" ")}></div>`;
 }
@@ -1,392 +0,0 @@
-/**
- * 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 };
@@ -1,185 +0,0 @@
-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 },
-    );
-  });
-});
@@ -374,9 +374,7 @@ describe('converter gap coverage — emission branches (specs 1–11)', () => {
        ],
      }),
    );
-    // 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');
+    expect(out).toBe('- [ ] top\n  - child');
  });

  // 10. A bulletList inside a blockquote: each list line independently prefixed.
@@ -198,11 +198,7 @@ describe('convertProseMirrorToMarkdown', () => {
        }),
      );
      // First line carries the marker; the nested list is indented 2 columns.
-      // 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');
+      expect(out).toBe('- parent\n  - child');
    });

    it('nested ordered list indents by the wider 3-col marker width', () => {
@@ -223,9 +219,8 @@ describe('convertProseMirrorToMarkdown', () => {
          ],
        }),
      );
-      // "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');
+      // "1. " is 3 columns wide, so the continuation indent is 3 spaces.
+      expect(out).toBe('1. parent\n   1. child');
    });
  });

@@ -544,12 +539,11 @@ 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. Block children are
-      // blank-line separated (loose list) per the #351 fix.
-      expect(out).toContain('10. j\n\n    1. x');
+      // indented exactly 4 spaces (prefix.length 3 + 1), NOT 3.
+      expect(out).toContain('10. j\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\n   1. x');
+      expect(out).not.toContain('10. j\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);
@@ -586,18 +580,17 @@ describe('convertProseMirrorToMarkdown', () => {
          content: [para(text('line1')), para(text('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');
+      // 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 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, blank-`>`
-      // separated so they stay distinct paragraphs on re-parse.
-      expect(out).toContain('> line1\n>\n> line2');
+      // Both paragraph children are present, each blockquote-prefixed.
+      expect(out).toContain('> line1\n> line2');
    });

    // Spec 4 — blockquote per-line prefixer over a multi-line nested callout.
@@ -614,11 +607,13 @@ describe('convertProseMirrorToMarkdown', () => {
          ],
        }),
      );
-      // 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');
+      // 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');
      // Every produced line carries the '> ' prefix (no line escapes to col 0).
      for (const line of out.split('\n')) {
        expect(line.startsWith('>')).toBe(true);