test(ai-chat): simplify msg factory and lock signature↔render coupling

Address non-blocking review items on the AI-chat stream-perf PR:

- Drop the unused `metadata` param from the `msg` test factory in
  message-item.test.ts; no caller passed it.
- Add a per-part-kind coupling guard to message-signature.test.ts that, for
  each part kind rendered today (text, reasoning, tool-*) plus the metadata
  banners, asserts that mutating a field the MessageItem render body DRAWS
  flips messageSignature — an executable lock for the load-bearing memo
  invariant documented in message-signature.ts.

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

CHANGELOG.md

@@ -52,14 +52,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Footnote multi-backlinks.** A footnote referenced more than once now shows a
   back-link per reference (↩ a b c …), each scrolling to its own occurrence, like
   Pandoc/Wikipedia; a single-reference footnote keeps the plain ↩. (#168)
-- **Model-friendly AI-chat tool-input errors.** When the model calls an in-app
-  AI tool with bad arguments, the validation failure is now a concise,
-  human-readable message that NAMES each offending parameter (by its dotted
-  path) and appends a fixed retry hint ("include every REQUIRED parameter…, do
-  not drop ids like `pageId`"), instead of the raw zod text. This nudges the
-  model to re-issue the call correctly — particularly in parallel tool-call
-  batches where it tends to drop a repeated id. The required/optional contract
-  and unknown-key stripping are unchanged. (#190)
 
 ### Changed
 
@@ -86,6 +78,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **AI chat: the desktop app no longer freezes at 100% CPU on long agent runs.**
+  `useChat` re-rendered on every streamed token and `MessageItem`/`ReasoningBlock`
+  re-parsed the whole transcript markdown (marked + DOMPurify) on every delta, so
+  per-turn work grew quadratically and saturated the main thread. The stream is now
+  throttled (`experimental_throttle`) to ~20 Hz and each finalized message row /
+  markdown part / reasoning block is memoized, so a long turn no longer re-parses
+  already-finished content. (#182)
 - **Editor: caret/selection landed on the wrong line when clicking inside code
   blocks and footnotes.** The affected NodeViews rendered their non-editable
   chrome (language menu, footnotes heading, footnote number marker) before the
@@ -100,14 +99,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   no longer froze on the previous step's authoritative usage; the current step's
   estimate is combined per-component with `max`, so the count rises smoothly and
   never jumps backwards. (#163)
-- **Concurrent page moves can no longer lose a subtree to a cycle.** Two
-  opposing re-parents racing each other (A: X under Y, B: Y under X) could each
-  pass a cycle check built from a stale snapshot and commit a cycle, orphaning a
-  subtree. A genuine re-parent under a concrete parent now serializes: it locks
-  the moved page and the destination parent `FOR UPDATE` in a canonical
-  (UUID-sorted) order — so opposing moves can't deadlock — and re-runs the cycle
-  check INSIDE the transaction against the now-committed state. Same-parent
-  reorders and moves to root keep the lock-free path. (#159)
 
 ## [0.93.0] - 2026-06-21
 

apps/client/src/features/ai-chat/components/chat-thread.tsx

@@ -29,6 +29,14 @@ import {
 } from "@/features/ai-chat/utils/queue-helpers.ts";
 import classes from "@/features/ai-chat/components/ai-chat.module.css";
 
+// Throttle how often the streamed `messages` state triggers a re-render. Without
+// it, useChat updates state on EVERY token, so the whole transcript's markdown
+// (marked + DOMPurify) is re-parsed per token — on a long agent run that grows
+// into a quadratic CPU storm that pins the main thread and freezes the UI.
+// ~50ms (20 Hz) keeps streaming visually smooth while decoupling re-render cost
+// from the token rate.
+const STREAM_THROTTLE_MS = 50;
+
 /** The page the user is currently viewing, sent as chat context. */
 export interface OpenPageContext {
   id: string;
@@ -246,6 +254,8 @@ export default function ChatThread({
     id: chatStoreId,
     messages: initialMessages,
     transport,
+    // See STREAM_THROTTLE_MS — bounds re-render/markdown-reparse frequency.
+    experimental_throttle: STREAM_THROTTLE_MS,
     // `onFinish` (ai@6 useChat) fires from a `finally` on EVERY terminal outcome
     // — success, user Stop/abort (`isAbort`), network drop (`isDisconnect`), and
     // stream error (`isError`). Keep calling `onTurnFinished()` on all of them

apps/client/src/features/ai-chat/components/message-item-memo.test.tsx

@@ -0,0 +1,81 @@
+import { describe, expect, it, vi } from "vitest";
+import { render } from "@testing-library/react";
+import { MantineProvider } from "@mantine/core";
+import type { UIMessage } from "@ai-sdk/react";
+
+// Stub react-i18next (the component reads `useTranslation`). Mirrors the stub in
+// reasoning-block.test.tsx.
+vi.mock("react-i18next", () => ({
+  useTranslation: () => ({ t: (key: string) => key }),
+}));
+
+// Spy on `renderChatMarkdown` so we can count parse calls per text. We keep every
+// OTHER named export of markdown.ts intact via `importActual`, and override only
+// `renderChatMarkdown` with a `vi.fn()` that returns simple HTML so the component
+// still renders. This is the seam that proves the MarkdownPart memo works: a
+// finalized text part must NOT be re-parsed on a later streamed delta.
+// `vi.hoisted` so the spy exists when the hoisted `vi.mock` factory runs.
+const { renderChatMarkdownSpy } = vi.hoisted(() => ({
+  renderChatMarkdownSpy: vi.fn((text: string) => `<p>${text}</p>`),
+}));
+vi.mock("@/features/ai-chat/utils/markdown.ts", async () => {
+  const actual = await vi.importActual<
+    typeof import("@/features/ai-chat/utils/markdown.ts")
+  >("@/features/ai-chat/utils/markdown.ts");
+  return { ...actual, renderChatMarkdown: renderChatMarkdownSpy };
+});
+
+import MessageItem from "./message-item";
+
+// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
+
+const msg = (parts: UIMessage["parts"]): UIMessage =>
+  ({ id: "m1", role: "assistant", parts }) as UIMessage;
+
+const renderRow = (message: UIMessage) =>
+  render(
+    <MantineProvider>
+      <MessageItem message={message} />
+    </MantineProvider>,
+  );
+
+/** Count how many spy calls parsed exactly `text` (filtering by the first arg). */
+const callsFor = (text: string) =>
+  renderChatMarkdownSpy.mock.calls.filter((c) => c[0] === text).length;
+
+describe("MessageItem markdown memoization", () => {
+  it("does not re-parse finalized text parts when only a tail part grows", () => {
+    renderChatMarkdownSpy.mockClear();
+
+    // Two finalized text parts.
+    const first = msg([
+      { type: "text", text: "alpha" },
+      { type: "text", text: "beta" },
+    ]);
+    const { rerender } = renderRow(first);
+
+    // Both finalized parts parsed exactly once on the initial render.
+    expect(callsFor("alpha")).toBe(1);
+    expect(callsFor("beta")).toBe(1);
+
+    // A streamed delta: a NEW message object where only a third tail part grows;
+    // the first two parts' text is byte-identical.
+    const next = msg([
+      { type: "text", text: "alpha" },
+      { type: "text", text: "beta" },
+      { type: "text", text: "gamm" },
+    ]);
+    rerender(
+      <MantineProvider>
+        <MessageItem message={next} />
+      </MantineProvider>,
+    );
+
+    // The finalized parts hit the MarkdownPart memo: still parsed at most once
+    // each across BOTH renders (the resilient invariant). The only new parse is
+    // for the changed/added tail part.
+    expect(callsFor("alpha")).toBe(1);
+    expect(callsFor("beta")).toBe(1);
+    expect(callsFor("gamm")).toBe(1);
+  });
+});

apps/client/src/features/ai-chat/components/message-item.test.ts

@@ -0,0 +1,73 @@
+import { describe, expect, it, vi } from "vitest";
+import type { UIMessage } from "@ai-sdk/react";
+
+// Stub react-i18next: importing the component module pulls in `useTranslation`,
+// and we only exercise the pure `arePropsEqual` comparator (no rendering), so a
+// minimal `t` that echoes the key is enough. Mirrors the stub in
+// reasoning-block.test.tsx.
+vi.mock("react-i18next", () => ({
+  useTranslation: () => ({ t: (key: string) => key }),
+}));
+
+import { arePropsEqual } from "./message-item";
+
+/**
+ * Tests for `arePropsEqual`, the `React.memo` comparator for MessageItem. It must
+ * return false on any visible prop/content change (so the row re-renders) and
+ * true when nothing visible changed (so a finalized row is skipped). A FIXED
+ * message id is used so a content-identical clone yields an equal signature.
+ */
+const msg = (parts: UIMessage["parts"]): UIMessage =>
+  ({ id: "m1", role: "assistant", parts }) as UIMessage;
+
+const props = (
+  message: UIMessage,
+  over: Record<string, unknown> = {},
+) => ({
+  message,
+  showCitations: true,
+  neutralizeInternalLinks: false,
+  assistantName: "AI",
+  ...over,
+});
+
+describe("arePropsEqual", () => {
+  it("returns false when showCitations differs", () => {
+    const m = msg([{ type: "text", text: "answer" }]);
+    expect(
+      arePropsEqual(props(m), props(m, { showCitations: false })),
+    ).toBe(false);
+  });
+
+  it("returns false when neutralizeInternalLinks differs", () => {
+    const m = msg([{ type: "text", text: "answer" }]);
+    expect(
+      arePropsEqual(props(m), props(m, { neutralizeInternalLinks: true })),
+    ).toBe(false);
+  });
+
+  it("returns false when assistantName differs", () => {
+    const m = msg([{ type: "text", text: "answer" }]);
+    expect(
+      arePropsEqual(props(m), props(m, { assistantName: "Other" })),
+    ).toBe(false);
+  });
+
+  it("returns true on the identity fast path (same message object, equal props)", () => {
+    const m = msg([{ type: "text", text: "answer" }]);
+    expect(arePropsEqual(props(m), props(m))).toBe(true);
+  });
+
+  it("returns true for the same content in a different message object", () => {
+    const a = msg([{ type: "text", text: "answer" }]);
+    const b = msg([{ type: "text", text: "answer" }]);
+    expect(a).not.toBe(b);
+    expect(arePropsEqual(props(a), props(b))).toBe(true);
+  });
+
+  it("returns false when content changed in a different message object", () => {
+    const a = msg([{ type: "text", text: "answer" }]);
+    const b = msg([{ type: "text", text: "answer grown" }]);
+    expect(arePropsEqual(props(a), props(b))).toBe(false);
+  });
+});

apps/client/src/features/ai-chat/components/message-item.tsx

@@ -1,3 +1,4 @@
+import { memo } from "react";
 import { Box, Text } from "@mantine/core";
 import { useTranslation } from "react-i18next";
 import type { UIMessage } from "@ai-sdk/react";
@@ -10,6 +11,7 @@ import { assistantMessageHasVisibleContent } from "@/features/ai-chat/utils/mess
 import { renderChatMarkdown } from "@/features/ai-chat/utils/markdown.ts";
 import { resolveAssistantName } from "@/features/ai-chat/utils/assistant-name.ts";
 import { reasoningTokensForPart } from "@/features/ai-chat/utils/reasoning-tokens.ts";
+import { messageSignature } from "@/features/ai-chat/utils/message-signature.ts";
 import { describeChatError } from "@/features/ai-chat/utils/error-message.ts";
 import classes from "@/features/ai-chat/components/ai-chat.module.css";
 
@@ -34,6 +36,39 @@ interface MessageItemProps {
   assistantName?: string;
 }
 
+/**
+ * One assistant text part rendered as sanitized markdown. Memoized on its inputs
+ * so a finalized text part is NOT re-parsed on every streamed delta: during a
+ * turn only the actively-growing tail part changes its `text`, so every earlier
+ * part hits the memo and skips the expensive marked + DOMPurify pass. Props are
+ * primitives, so React.memo's default shallow compare is exactly right (the
+ * `text` string is compared by value).
+ */
+const MarkdownPart = memo(function MarkdownPart({
+  text,
+  neutralizeInternalLinks,
+}: {
+  text: string;
+  neutralizeInternalLinks: boolean;
+}) {
+  const html = renderChatMarkdown(text, { neutralizeInternalLinks });
+  if (html) {
+    return (
+      <div
+        className={classes.markdown}
+        // Sanitized by renderChatMarkdown (DOMPurify) before insertion.
+        dangerouslySetInnerHTML={{ __html: html }}
+      />
+    );
+  }
+  // Fallback when markdown could not render synchronously: raw text.
+  return (
+    <Text className={classes.markdown} style={{ whiteSpace: "pre-wrap" }}>
+      {text}
+    </Text>
+  );
+});
+
 /**
  * Render a single UIMessage by iterating its `parts`:
  *  - `text` parts -> sanitized markdown.
@@ -41,12 +76,13 @@ interface MessageItemProps {
  * Other part kinds (reasoning, sources, files, step-start) are ignored for v1.
  * User messages render their text as a right-aligned plain bubble.
  *
- * This component is intentionally NOT memoized: `useChat` replaces the streaming
- * assistant message with a freshly cloned object on every streamed delta, so the
- * `message` prop identity (and its `parts`) changes each tick. Re-rendering the
- * text parts on each delta is what makes the answer stream in progressively.
+ * This component is memoized (see `arePropsEqual` at the bottom) on a cheap
+ * per-message content signature: the streaming TAIL message's signature changes
+ * on each delta so it still re-renders and streams in, while finalized rows are
+ * skipped. Each text part's markdown is itself memoized via `MarkdownPart`, so a
+ * long turn no longer re-parses the whole transcript on every token.
  */
-export default function MessageItem({
+function MessageItem({
   message,
   showCitations = true,
   neutralizeInternalLinks = false,
@@ -109,24 +145,12 @@ export default function MessageItem({
           // starts with an empty text part before the first token arrives); the
           // typing indicator covers that gap until real content streams in.
           if (!part.text.trim()) return null;
-          const html = renderChatMarkdown(part.text, {
-            neutralizeInternalLinks,
-          });
-          if (html) {
-            return (
-              <div
-                key={index}
-                className={classes.markdown}
-                // Sanitized by renderChatMarkdown (DOMPurify) before insertion.
-                dangerouslySetInnerHTML={{ __html: html }}
-              />
-            );
-          }
-          // Fallback when markdown could not render synchronously: raw text.
           return (
-            <Text key={index} className={classes.markdown} style={{ whiteSpace: "pre-wrap" }}>
-              {part.text}
-            </Text>
+            <MarkdownPart
+              key={index}
+              text={part.text}
+              neutralizeInternalLinks={neutralizeInternalLinks}
+            />
           );
         }
 
@@ -177,3 +201,26 @@ export default function MessageItem({
     </Box>
   );
 }
+
+/** Skip re-rendering a message whose visible content is unchanged. The streaming
+ *  TAIL message gets a fresh object whose signature changes each delta, so it
+ *  still re-renders and streams in; every FINALIZED message is skipped, turning a
+ *  per-token whole-transcript re-render into a tail-only one. */
+export function arePropsEqual(
+  prev: MessageItemProps,
+  next: MessageItemProps,
+): boolean {
+  if (
+    prev.showCitations !== next.showCitations ||
+    prev.neutralizeInternalLinks !== next.neutralizeInternalLinks ||
+    prev.assistantName !== next.assistantName
+  ) {
+    return false;
+  }
+  // Fast path: identical message object (finalized rows keep their identity
+  // across deltas) — skip without building signatures.
+  if (prev.message === next.message) return true;
+  return messageSignature(prev.message) === messageSignature(next.message);
+}
+
+export default memo(MessageItem, arePropsEqual);

apps/client/src/features/ai-chat/components/reasoning-block.tsx

@@ -1,4 +1,4 @@
-import { useState } from "react";
+import { memo, useMemo, useState } from "react";
 import { Box, Collapse, Group, Text, UnstyledButton } from "@mantine/core";
 import { IconChevronDown } from "@tabler/icons-react";
 import { useTranslation } from "react-i18next";
@@ -27,19 +27,23 @@ interface ReasoningBlockProps {
  * Providers that don't stream reasoning TEXT still render this block from the
  * authoritative count alone (header only, empty body) so the cost is visible.
  */
-export default function ReasoningBlock({ text, tokens }: ReasoningBlockProps) {
+function ReasoningBlock({ text, tokens }: ReasoningBlockProps) {
   const { t } = useTranslation();
   const [open, setOpen] = useState(false);
 
   // Authoritative count wins; otherwise estimate live from the streamed text.
   const count = tokens && tokens > 0 ? tokens : estimateTokens(text);
   const trimmed = text.trim();
-  // Collapse the blank-line gaps the model emits between every list item /
-  // paragraph so the reasoning renders compactly (tight lists, joined
-  // paragraphs) — see collapseBlankLines. ONLY here, not in the normal answer.
-  const html = trimmed
-    ? renderChatMarkdown(collapseBlankLines(trimmed), {})
-    : "";
+  // Memoize the markdown render so toggling `open` (or a parent re-render caused
+  // by an unrelated streamed delta) does not re-parse the reasoning text; it
+  // recomputes only when the reasoning text itself changes (while it streams in).
+  // collapseBlankLines collapses the blank-line gaps the model emits between every
+  // list item / paragraph so the reasoning renders compactly (tight lists, joined
+  // paragraphs) — ONLY here, not in the normal answer.
+  const html = useMemo(
+    () => (trimmed ? renderChatMarkdown(collapseBlankLines(trimmed), {}) : ""),
+    [trimmed],
+  );
 
   return (
     <Box className={classes.reasoningBlock} mb={6}>
@@ -87,3 +91,8 @@ export default function ReasoningBlock({ text, tokens }: ReasoningBlockProps) {
     </Box>
   );
 }
+
+// Memoized: re-renders only when `text`/`tokens` change (primitive props, default
+// shallow compare), so a parent re-render during streaming of OTHER content does
+// not re-run the markdown parse for an already-finalized reasoning block.
+export default memo(ReasoningBlock);

apps/client/src/features/ai-chat/utils/message-signature.test.ts

@@ -0,0 +1,241 @@
+import { describe, expect, it } from "vitest";
+import type { UIMessage } from "@ai-sdk/react";
+import { messageSignature } from "@/features/ai-chat/utils/message-signature.ts";
+
+/**
+ * Pure-helper tests for `messageSignature`, the cheap per-message content
+ * signature that drives MessageItem's memo (a streaming row's signature must
+ * change on every delta so it re-renders, while a finalized row's stays stable
+ * so it is skipped). Each test exercises ONE change signal and asserts it flips
+ * the signature; a content-identical clone must keep an EQUAL signature.
+ *
+ * The signature embeds `message.id` and `message.role`, so the `msg` factory
+ * uses a FIXED id/role here (not `Math.random()`): otherwise two messages with
+ * identical content would get different signatures and the negative case would
+ * be impossible to express.
+ */
+const msg = (
+  parts: UIMessage["parts"],
+  metadata?: unknown,
+): UIMessage =>
+  ({
+    id: "m1",
+    role: "assistant",
+    parts,
+    metadata,
+  }) as UIMessage;
+
+describe("messageSignature", () => {
+  it("changes when a text part grows", () => {
+    const before = msg([{ type: "text", text: "alpha" }]);
+    const after = msg([{ type: "text", text: "alpha beta" }]);
+    expect(messageSignature(before)).not.toBe(messageSignature(after));
+  });
+
+  it("changes when a new part is appended", () => {
+    const before = msg([{ type: "text", text: "alpha" }]);
+    const after = msg([
+      { type: "text", text: "alpha" },
+      { type: "text", text: "beta" },
+    ]);
+    expect(messageSignature(before)).not.toBe(messageSignature(after));
+  });
+
+  it("changes when a part's state flips", () => {
+    const before = msg([
+      { type: "tool-getPage", state: "input-streaming" } as never,
+    ]);
+    const after = msg([
+      { type: "tool-getPage", state: "output-available" } as never,
+    ]);
+    expect(messageSignature(before)).not.toBe(messageSignature(after));
+  });
+
+  it("changes when a tool part gains an output", () => {
+    const before = msg([
+      { type: "tool-getPage", state: "output-available" } as never,
+    ]);
+    const after = msg([
+      {
+        type: "tool-getPage",
+        state: "output-available",
+        output: { ok: true },
+      } as never,
+    ]);
+    expect(messageSignature(before)).not.toBe(messageSignature(after));
+  });
+
+  it("changes when a part gains an errorText", () => {
+    const before = msg([
+      { type: "tool-getPage", state: "output-error" } as never,
+    ]);
+    const after = msg([
+      {
+        type: "tool-getPage",
+        state: "output-error",
+        errorText: "boom",
+      } as never,
+    ]);
+    expect(messageSignature(before)).not.toBe(messageSignature(after));
+  });
+
+  it("changes when usage.reasoningTokens arrives on finish-step (text/state already frozen)", () => {
+    // The specifically-commented edge case: the authoritative turn total lands on
+    // the final finish-step AFTER the reasoning text length and state are frozen.
+    // Only the token count appears between these two snapshots, so the signature
+    // MUST still flip — otherwise the "Thinking · N tokens" header would never
+    // snap from the live estimate to the exact figure.
+    const before = msg([
+      { type: "reasoning", text: "thinking", state: "done" } as never,
+    ]);
+    const after = msg(
+      [{ type: "reasoning", text: "thinking", state: "done" } as never],
+      { usage: { reasoningTokens: 42 } },
+    );
+    expect(messageSignature(before)).not.toBe(messageSignature(after));
+  });
+
+  it("changes when metadata.error appears", () => {
+    const before = msg([{ type: "text", text: "answer" }]);
+    const after = msg([{ type: "text", text: "answer" }], { error: "boom" });
+    expect(messageSignature(before)).not.toBe(messageSignature(after));
+  });
+
+  it("changes when metadata.finishReason changes (e.g. to 'aborted')", () => {
+    const before = msg([{ type: "text", text: "answer" }], {
+      finishReason: "stop",
+    });
+    const after = msg([{ type: "text", text: "answer" }], {
+      finishReason: "aborted",
+    });
+    expect(messageSignature(before)).not.toBe(messageSignature(after));
+  });
+
+  it("is UNCHANGED for a content-identical clone (different object, same values)", () => {
+    // A finalized row that is re-created as a fresh object (different parts array
+    // by reference, same parts by value) must keep an EQUAL signature, so the
+    // memo skips re-rendering it.
+    const a = msg([
+      { type: "text", text: "alpha" },
+      { type: "tool-getPage", state: "output-available", output: { ok: true } } as never,
+    ]);
+    const b = msg([
+      { type: "text", text: "alpha" },
+      { type: "tool-getPage", state: "output-available", output: { ok: true } } as never,
+    ]);
+    expect(a).not.toBe(b);
+    expect(messageSignature(a)).toBe(messageSignature(b));
+  });
+});
+
+/**
+ * Per-part-kind coupling guard for the load-bearing invariant documented at the
+ * top of message-signature.ts: the signature MUST sample every VISIBLE field the
+ * MessageItem render body draws, or the memo freezes a stale row. This is an
+ * executable lock for the part kinds rendered TODAY — read alongside
+ * `MessageItem` (message-item.tsx) and the `assistantMessageHasVisibleContent`
+ * helper (message-content.ts), which "mirrors MessageItem's render decisions
+ * EXACTLY". For each kind, mutating a field the render body DRAWS must flip the
+ * signature. If a new visible field is rendered without being added here AND to
+ * the signature, the corresponding assertion below should fail — that is the
+ * guard. (This intentionally stops short of the render-descriptor refactor:
+ * adding a part kind or a visible field still requires a human to extend both
+ * the signature and this block.)
+ */
+describe("messageSignature ↔ render coupling (per visible part kind)", () => {
+  describe("text part — render draws part.text (MarkdownPart text={part.text})", () => {
+    it("flips when the visible text changes", () => {
+      // Streaming is append-only, so the visible text only grows; the signature
+      // samples its length, so the growth is the change signal.
+      const before = msg([{ type: "text", text: "answer" }]);
+      const after = msg([{ type: "text", text: "answer extended" }]);
+      expect(messageSignature(before)).not.toBe(messageSignature(after));
+    });
+  });
+
+  describe("reasoning part — render draws text + tokens (ReasoningBlock)", () => {
+    it("flips when the visible reasoning text changes", () => {
+      const before = msg([
+        { type: "reasoning", text: "think", state: "streaming" } as never,
+      ]);
+      const after = msg([
+        { type: "reasoning", text: "think harder", state: "streaming" } as never,
+      ]);
+      expect(messageSignature(before)).not.toBe(messageSignature(after));
+    });
+
+    it("flips when the visible token count (metadata.usage.reasoningTokens) lands", () => {
+      // The header's "Thinking · N tokens" reads reasoningTokensForPart, fed by
+      // metadata.usage.reasoningTokens — a VISIBLE field that arrives on the final
+      // finish-step after text length and state are frozen.
+      const before = msg([
+        { type: "reasoning", text: "think", state: "done" } as never,
+      ]);
+      const after = msg(
+        [{ type: "reasoning", text: "think", state: "done" } as never],
+        { usage: { reasoningTokens: 99 } },
+      );
+      expect(messageSignature(before)).not.toBe(messageSignature(after));
+    });
+  });
+
+  describe("tool-* part — render draws state/errorText/citations (ToolCallCard)", () => {
+    it("flips when the run state changes (running ↔ done icon + label)", () => {
+      // toolRunState(part.state) selects the spinner/check/error icon.
+      const before = msg([
+        { type: "tool-getPage", state: "input-available" } as never,
+      ]);
+      const after = msg([
+        { type: "tool-getPage", state: "output-available" } as never,
+      ]);
+      expect(messageSignature(before)).not.toBe(messageSignature(after));
+    });
+
+    it("flips when output arrives (drives the rendered citation links)", () => {
+      // toolCitations reads part.output to render the "/p/{id}" anchors.
+      const before = msg([
+        { type: "tool-getPage", state: "output-available" } as never,
+      ]);
+      const after = msg([
+        {
+          type: "tool-getPage",
+          state: "output-available",
+          output: { id: "page-1", title: "Doc" },
+        } as never,
+      ]);
+      expect(messageSignature(before)).not.toBe(messageSignature(after));
+    });
+
+    it("flips when errorText appears (the visible red error detail line)", () => {
+      const before = msg([
+        { type: "tool-getPage", state: "output-error" } as never,
+      ]);
+      const after = msg([
+        {
+          type: "tool-getPage",
+          state: "output-error",
+          errorText: "permission denied",
+        } as never,
+      ]);
+      expect(messageSignature(before)).not.toBe(messageSignature(after));
+    });
+  });
+
+  describe("metadata banners — render draws error / aborted notices", () => {
+    it("flips when metadata.error appears (ChatErrorAlert banner)", () => {
+      const before = msg([{ type: "text", text: "answer" }]);
+      const after = msg([{ type: "text", text: "answer" }], { error: "boom" });
+      expect(messageSignature(before)).not.toBe(messageSignature(after));
+    });
+
+    it("flips when metadata.finishReason becomes 'aborted' (ChatStoppedNotice)", () => {
+      const before = msg([{ type: "text", text: "answer" }], {
+        finishReason: "stop",
+      });
+      const after = msg([{ type: "text", text: "answer" }], {
+        finishReason: "aborted",
+      });
+      expect(messageSignature(before)).not.toBe(messageSignature(after));
+    });
+  });
+});

apps/client/src/features/ai-chat/utils/message-signature.ts

@@ -0,0 +1,44 @@
+import type { UIMessage } from "@ai-sdk/react";
+
+/** Cheap content signature for one message: changes iff something VISIBLE in the
+ *  row changed. Streaming is APPEND-ONLY (text parts only grow, parts are only
+ *  appended, a tool/text part flips state once), so a per-part [type, text
+ *  length, state, error/output presence] tuple + the persisted metadata
+ *  (error/finishReason) is a sufficient change signal without comparing full
+ *  strings on every delta. WARNING — load-bearing for the MessageItem memo:
+ *  if a future part kind's VISIBLE content can change WITHOUT changing [type,
+ *  text length, state, error/output presence] (e.g. a tool that streams
+ *  `preliminary` output, or a client-side regenerate that edits a finalized
+ *  row in place), extend this signature or the memo will freeze a stale row. */
+export function messageSignature(message: UIMessage): string {
+  const parts = message.parts
+    .map((p) => {
+      const any = p as {
+        type: string;
+        text?: string;
+        state?: string;
+        errorText?: string;
+        output?: unknown;
+      };
+      return [
+        any.type,
+        any.text?.length ?? 0,
+        any.state ?? "",
+        any.errorText ? 1 : 0,
+        any.output !== undefined ? 1 : 0,
+      ].join(":");
+    })
+    .join("|");
+  const meta = message.metadata as
+    | { error?: string; finishReason?: string; usage?: { reasoningTokens?: number } }
+    | undefined;
+  // `usage.reasoningTokens` is neither append-only nor part-bound: the authoritative
+  // turn total arrives on the final `finish-step` AFTER the reasoning text length and
+  // state are already frozen. Without it in the signature the row's signature would be
+  // unchanged at that point and the re-render skipped, so the "Thinking · N tokens"
+  // header (reasoningTokensForPart) would keep the live estimate instead of snapping
+  // to the exact figure.
+  return `${message.id}#${message.role}#${parts}#${meta?.error ?? ""}#${
+    meta?.finishReason ?? ""
+  }#${meta?.usage?.reasoningTokens ?? ""}`;
+}

apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts

@@ -120,26 +120,21 @@ describe('AiChatToolsService deletePage guardrail (H4)', () => {
     const tools = await buildTools();
     const deletePage = tools.deletePage;
 
-    // inputSchema is now an AI SDK `Schema` (not a raw zod object). Its
-    // `validate` runs the same zod safeParse and forwards the STRIPPED data, so
-    // a permanent/force flag is never part of the validated input the SDK then
-    // hands to execute.
+    // The Zod input schema only allows `pageId`; parsing strips/ignores extra
+    // keys, so a permanent/force flag is never part of the validated input.
     const schema = (deletePage as unknown as { inputSchema: unknown })
       .inputSchema as {
-      validate: (
-        v: unknown,
-      ) => Promise<{ success: boolean; value?: Record<string, unknown> }>;
+      parse: (v: unknown) => Record<string, unknown>;
     };
-    const result = await schema.validate({
+    const parsed = schema.parse({
       pageId: 'page-789',
       permanentlyDelete: true,
       forceDelete: true,
     });
 
-    expect(result.success).toBe(true);
-    expect(result.value).toHaveProperty('pageId', 'page-789');
-    expect(result.value).not.toHaveProperty('permanentlyDelete');
-    expect(result.value).not.toHaveProperty('forceDelete');
+    expect(parsed).toHaveProperty('pageId', 'page-789');
+    expect(parsed).not.toHaveProperty('permanentlyDelete');
+    expect(parsed).not.toHaveProperty('forceDelete');
   });
 });
 
@@ -212,25 +207,21 @@ describe('AiChatToolsService expanded toolset guardrails', () => {
     const tools = await buildTools();
     const transformPage = tools.transformPage;
 
-    // inputSchema is now an AI SDK `Schema`; its `validate` runs the same zod
-    // safeParse, which only allows pageId/transformJs/dryRun and strips unknown
-    // keys — so deleteComments can never reach the client.
+    // The Zod input schema only allows pageId/transformJs/dryRun; parsing
+    // strips unknown keys, so deleteComments can never reach the client.
     const schema = (transformPage as unknown as { inputSchema: unknown })
       .inputSchema as {
-      validate: (
-        v: unknown,
-      ) => Promise<{ success: boolean; value?: Record<string, unknown> }>;
+      parse: (v: unknown) => Record<string, unknown>;
     };
-    const result = await schema.validate({
+    const parsed = schema.parse({
       pageId: 'p',
       transformJs: '(d)=>d',
       dryRun: true,
       deleteComments: true,
     });
 
-    expect(result.success).toBe(true);
-    expect(result.value).toHaveProperty('pageId', 'p');
-    expect(result.value).not.toHaveProperty('deleteComments');
+    expect(parsed).toHaveProperty('pageId', 'p');
+    expect(parsed).not.toHaveProperty('deleteComments');
   });
 });
 

apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts

@@ -15,7 +15,6 @@ import {
 } from './docmost-client.loader';
 import { resolveCurrentPageResult } from './current-page.util';
 import { parseNodeArg } from './parse-node-arg';
-import { modelFriendlyInput } from './model-friendly-input';
 
 /**
  * Per-user, per-request adapter that exposes Docmost READ operations to the
@@ -103,9 +102,9 @@ export class AiChatToolsService {
     ): Tool =>
       tool({
         description: spec.description,
-        inputSchema: modelFriendlyInput(
-          spec.buildShape ? (spec.buildShape(z) as z.ZodRawShape) : {},
-        ),
+        inputSchema: spec.buildShape
+          ? z.object(spec.buildShape(z) as z.ZodRawShape)
+          : z.object({}),
         execute,
       });
 
@@ -119,7 +118,7 @@ export class AiChatToolsService {
           'and entities), not a full sentence. If the first results look weak ' +
           'or incomplete, search again with different wording or synonyms ' +
           'before answering.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           query: z.string().describe('The search query.'),
           limit: z
             .number()
@@ -228,7 +227,7 @@ export class AiChatToolsService {
           '"the current page", or "here" refers to. Returns the page id and title, ' +
           'or null if the user is not currently on a page. Call this first whenever ' +
           'the user refers to the current page without giving an explicit id.',
-        inputSchema: modelFriendlyInput({}),
+        inputSchema: z.object({}),
         execute: async () => resolveCurrentPageResult(openedPage),
       }),
 
@@ -236,7 +235,7 @@ export class AiChatToolsService {
         description:
           'Fetch a single page as Markdown by its page id. Returns the page ' +
           'title and its Markdown content.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id (or slugId) of the page.'),
         }),
         execute: async ({ pageId }) => {
@@ -260,7 +259,7 @@ export class AiChatToolsService {
           'Create a new page with a Markdown body in a space, optionally under ' +
           'a parent page. Returns the new page id and title. Reversible: a page ' +
           'can be moved to trash later.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           title: z.string().describe('The title of the new page.'),
           content: z
             .string()
@@ -295,7 +294,7 @@ export class AiChatToolsService {
         description:
           "Replace a page's body with new Markdown content (and optionally its " +
           'title). Reversible: the previous version is kept in page history.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page to update.'),
           content: z.string().describe('The new page body as Markdown.'),
           title: z
@@ -317,7 +316,7 @@ export class AiChatToolsService {
         description:
           "Rename a page (change its title only; the body is untouched). " +
           'Reversible: rename back at any time.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page to rename.'),
           title: z.string().describe('The new title.'),
         }),
@@ -332,7 +331,7 @@ export class AiChatToolsService {
         description:
           'Move a page under a new parent page, or to the space root when no ' +
           'parent is given. Reversible: move it back at any time.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page to move.'),
           parentPageId: z
             .string()
@@ -354,7 +353,7 @@ export class AiChatToolsService {
         description:
           'Move a page to the trash (SOFT delete only — fully reversible; the ' +
           'page can be restored from trash). This NEVER permanently deletes.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page to move to trash.'),
         }),
         // GUARDRAIL (§14 H4): the only field ever passed to the client is
@@ -380,7 +379,7 @@ export class AiChatToolsService {
           '"selection not found" error, retry with a corrected EXACT selection ' +
           'copied verbatim from a single paragraph/block. Reversible via the ' +
           'comment UI.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page to comment on.'),
           content: z.string().describe('The comment body as Markdown.'),
           selection: z
@@ -429,7 +428,7 @@ export class AiChatToolsService {
         description:
           'Resolve or reopen a top-level comment thread (reversible — toggle ' +
           'the resolved flag). Only top-level comments can be resolved.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           commentId: z
             .string()
             .describe('The id of the top-level comment to resolve/reopen.'),
@@ -461,7 +460,7 @@ export class AiChatToolsService {
           'List the most recent pages, optionally scoped to a single space. ' +
           'Returns a bounded list (default 50, max 100). Pass tree:true (with ' +
           "spaceId) to instead get the space's full page hierarchy as a nested tree.",
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           spaceId: z
             .string()
             .optional()
@@ -489,7 +488,7 @@ export class AiChatToolsService {
           'List sidebar pages for a space. With no pageId, returns the ' +
           "space's ROOT pages; with a pageId, returns that page's direct " +
           'CHILDREN.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           spaceId: z.string().describe('The id of the space.'),
           pageId: z
             .string()
@@ -521,7 +520,7 @@ export class AiChatToolsService {
         description:
           'Read a table as a matrix of cell texts (plus a parallel cellIds ' +
           'matrix so cells can be addressed for rich edits).',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page.'),
           tableRef: z
             .string()
@@ -537,7 +536,7 @@ export class AiChatToolsService {
       listComments: tool({
         description:
           'List all comments on a page (content as Markdown).',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page.'),
         }),
         execute: async ({ pageId }) => await client.listComments(pageId),
@@ -545,7 +544,7 @@ export class AiChatToolsService {
 
       getComment: tool({
         description: 'Fetch a single comment by id (content as Markdown).',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           commentId: z.string().describe('The id of the comment.'),
         }),
         execute: async ({ commentId }) => await client.getComment(commentId),
@@ -555,7 +554,7 @@ export class AiChatToolsService {
         description:
           'Find new comments across a space (optionally scoped to a subtree) ' +
           'created after a given timestamp.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           spaceId: z.string().describe('The id of the space to scan.'),
           since: z
             .string()
@@ -587,7 +586,7 @@ export class AiChatToolsService {
         description:
           'Fetch a single page-history version including its lossless ' +
           'ProseMirror content.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           historyId: z.string().describe('The id of the history version.'),
         }),
         execute: async ({ historyId }) =>
@@ -605,7 +604,7 @@ export class AiChatToolsService {
           'Export a page to a single self-contained Docmost-flavoured ' +
           'Markdown file (meta + body + comment threads). Lossless round-trip ' +
           'with importPageMarkdown.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page to export.'),
         }),
         execute: async ({ pageId }) => {
@@ -631,7 +630,7 @@ export class AiChatToolsService {
           '{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node arg ' +
           'may be a JSON object or a JSON string (both accepted). Reversible: ' +
           'the previous version is kept in page history.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page.'),
           nodeId: z
             .string()
@@ -664,7 +663,7 @@ export class AiChatToolsService {
           '{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node arg ' +
           'may be a JSON object or a JSON string (both accepted). Reversible ' +
           'via page history.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page.'),
           node: z
             .any()
@@ -723,7 +722,7 @@ export class AiChatToolsService {
           'object or a JSON string (both accepted). Omit content for a ' +
           'title-only update. Reversible: the previous version is kept in page ' +
           'history.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page to update.'),
           content: z
             .any()
@@ -754,7 +753,7 @@ export class AiChatToolsService {
         description:
           'Insert a row of plain-text cells into a table. Reversible via ' +
           'page history.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page.'),
           tableRef: z
             .string()
@@ -773,7 +772,7 @@ export class AiChatToolsService {
       tableDeleteRow: tool({
         description:
           'Delete a table row at a 0-based index. Reversible via page history.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page.'),
           tableRef: z
             .string()
@@ -788,7 +787,7 @@ export class AiChatToolsService {
         description:
           'Set the plain-text content of a table cell at [row, col] (0-based). ' +
           'Reversible via page history.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page.'),
           tableRef: z
             .string()
@@ -818,7 +817,7 @@ export class AiChatToolsService {
           'Make a page PUBLICLY accessible and return its public URL. ' +
           'Reversible via unsharePage. Only share when the user explicitly ' +
           'asked, since this exposes the page to anyone with the link.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page to share.'),
           searchIndexing: z
             .boolean()
@@ -845,7 +844,7 @@ export class AiChatToolsService {
           "page's ProseMirror document for complex/scripted rewrites. dryRun " +
           '(default true) previews a diff WITHOUT writing; set dryRun:false to ' +
           'apply. Reversible: applying creates a new page-history snapshot.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z.string().describe('The id of the page to transform.'),
           transformJs: z
             .string()

apps/server/src/core/ai-chat/tools/model-friendly-input.spec.ts

@@ -1,112 +0,0 @@
-import { z } from 'zod';
-import { modelFriendlyInput } from './model-friendly-input';
-
-/**
- * Unit tests for the model-friendly input wrapper (issue #190): validation
- * failures must report a human-readable, parameter-naming message (not the raw
- * zod text), successful validation must strip unknown keys (preserving the
- * strip guardrails), and the JSON schema handed to the model must keep the
- * required/optional contract and field descriptions intact.
- */
-describe('modelFriendlyInput', () => {
-  // A representative shape: a required id + description, plus an optional field.
-  const shape = {
-    pageId: z.string().describe('The id of the page to comment on.'),
-    content: z.string(),
-    limit: z.number().int().optional(),
-  };
-
-  // The AI SDK `Schema` exposes a `validate` callback and a `jsonSchema` field;
-  // type them loosely for the test.
-  type SchemaLike = {
-    validate?: (
-      v: unknown,
-    ) =>
-      | { success: boolean; value?: Record<string, unknown>; error?: Error }
-      | PromiseLike<{
-          success: boolean;
-          value?: Record<string, unknown>;
-          error?: Error;
-        }>;
-    jsonSchema: unknown;
-  };
-
-  it('reports a model-friendly error naming the missing REQUIRED param + retry hint', async () => {
-    const schema = modelFriendlyInput(shape) as unknown as SchemaLike;
-    // Drop the required `pageId` (the parallel-batch failure mode).
-    const result = await schema.validate!({ content: 'hi' });
-
-    expect(result.success).toBe(false);
-    const message = result.error?.message ?? '';
-    // Names the offending parameter by name.
-    expect(message).toContain('pageId');
-    // Carries the fixed actionable retry hint.
-    expect(message).toContain('Include every REQUIRED parameter and retry');
-    expect(message).toContain('do not drop ids like "pageId"');
-    // It must NOT be the bare raw zod text alone — our wrapper prefix is present.
-    expect(message).toContain('Invalid tool input');
-  });
-
-  it('accepts valid input and STRIPS unknown keys (keeps declared ones)', async () => {
-    const schema = modelFriendlyInput(shape) as unknown as SchemaLike;
-    const result = await schema.validate!({
-      pageId: 'p-1',
-      content: 'hello',
-      // An extra unknown key a (compromised) model might emit.
-      permanentlyDelete: true,
-    });
-
-    expect(result.success).toBe(true);
-    expect(result.value).toEqual({ pageId: 'p-1', content: 'hello' });
-    expect(result.value).not.toHaveProperty('permanentlyDelete');
-  });
-
-  it('produces a draft-07 JSON schema that preserves required + descriptions', async () => {
-    const schema = modelFriendlyInput(shape) as unknown as SchemaLike;
-    // jsonSchema may be a value or a promise; await either way.
-    const json = (await Promise.resolve(schema.jsonSchema)) as {
-      required?: string[];
-      properties?: Record<string, { description?: string }>;
-    };
-
-    // Required contract preserved: pageId + content required, limit optional.
-    expect(json.required).toEqual(expect.arrayContaining(['pageId', 'content']));
-    expect(json.required).not.toContain('limit');
-    // Field description preserved.
-    expect(json.properties?.pageId?.description).toBe(
-      'The id of the page to comment on.',
-    );
-  });
-
-  it('de-duplicates a parameter that produces MULTIPLE issues on the same path', async () => {
-    // A single field can fail several zod checks at once (here min-length AND a
-    // regex), yielding two issues with the SAME path. The friendly message must
-    // name that parameter only once (the `seen` dedup branch).
-    const multiIssueShape = {
-      code: z
-        .string()
-        .min(5)
-        .regex(/^[0-9]+$/),
-    };
-    const schema = modelFriendlyInput(
-      multiIssueShape,
-    ) as unknown as SchemaLike;
-    // "ab" violates BOTH the min(5) and the digit-only regex.
-    const result = await schema.validate!({ code: 'ab' });
-
-    expect(result.success).toBe(false);
-    const message = result.error?.message ?? '';
-    // The parameter name appears exactly once despite two underlying issues.
-    const occurrences = message.split('parameter "code"').length - 1;
-    expect(occurrences).toBe(1);
-  });
-
-  it('handles a root-level type error with a "(root)" parameter name', async () => {
-    const schema = modelFriendlyInput(shape) as unknown as SchemaLike;
-    // Passing a non-object yields an issue with an empty path.
-    const result = await schema.validate!('not an object');
-
-    expect(result.success).toBe(false);
-    expect(result.error?.message).toContain('(root)');
-  });
-});

apps/server/src/core/ai-chat/tools/model-friendly-input.ts

@@ -1,72 +0,0 @@
-import { jsonSchema, type JSONSchema7, type Schema } from 'ai';
-import { z } from 'zod';
-
-// Centralized input-schema wrapper for in-app AI tools. The JSON schema handed
-// to the model is derived from the same zod shape (so `required`/`description`/
-// constraints are unchanged), but validation failures are reported with a
-// human-readable message that NAMES the offending parameter(s) and asks the
-// model to retry with every required field — instead of the raw zod text. This
-// matters for parallel tool-call batches where the model tends to drop a
-// repeated id like `pageId`.
-
-// Fixed, actionable hint appended to every validation error. Kept as a constant
-// so the message stays deterministic and the spec can assert on it verbatim.
-const RETRY_HINT =
-  'Include every REQUIRED parameter and retry; when issuing parallel tool ' +
-  'calls, do not drop ids like "pageId".';
-
-/**
- * Turn a zod validation error into a concise, model-friendly message that names
- * each offending parameter (by its dotted path; the root object is "(root)"),
- * gives a short reason, and ends with the fixed retry hint. Repeated parameter
- * names are de-duplicated and the output is deterministic.
- */
-export function formatIssues(error: z.ZodError): string {
-  const seen = new Set<string>();
-  const parts: string[] = [];
-  for (const issue of error.issues) {
-    const name =
-      Array.isArray(issue.path) && issue.path.length > 0
-        ? issue.path.join('.')
-        : '(root)';
-    if (seen.has(name)) continue;
-    seen.add(name);
-    // Prefer zod's own message (e.g. "Invalid input: expected string, received
-    // undefined"); fall back to a generic reason when it is missing.
-    const reason = issue.message ? issue.message : 'missing or invalid';
-    parts.push(`parameter "${name}": ${reason}`);
-  }
-  const summary = parts.length > 0 ? parts.join('; ') : 'invalid tool input';
-  return `Invalid tool input — ${summary}. ${RETRY_HINT}`;
-}
-
-/**
- * Build an AI SDK `Schema` from a zod raw shape. The JSON schema exposed to the
- * model is derived from the zod object (preserving `required`, `description`,
- * and field constraints), so the required/optional contract is UNCHANGED. On a
- * validation failure we return a model-friendly error (see `formatIssues`); on
- * success we return the PARSED data, which has unknown keys stripped by zod —
- * this preserves the existing strip guardrails (e.g. deletePage never forwards
- * permanentlyDelete/forceDelete; transformPage never forwards deleteComments).
- */
-export function modelFriendlyInput<Shape extends z.ZodRawShape>(
-  shape: Shape,
-): Schema<z.infer<z.ZodObject<Shape>>> {
-  const object = z.object(shape);
-  // draft-07 JSON schema for the model (keeps required/description/constraints).
-  const schema = z.toJSONSchema(object, { target: 'draft-7' }) as JSONSchema7;
-  return jsonSchema<z.infer<typeof object>>(schema, {
-    validate: (value: unknown) => {
-      const result = object.safeParse(value);
-      if (result.success) {
-        // Return the PARSED (unknown-key-stripped) data so the SDK forwards a
-        // clean object to execute — preserves the existing strip guardrails.
-        return { success: true as const, value: result.data };
-      }
-      return {
-        success: false as const,
-        error: new Error(formatIssues(result.error)),
-      };
-    },
-  });
-}

apps/server/src/core/ai-chat/tools/public-share-chat-tools.service.ts

@@ -5,7 +5,6 @@ import { ShareService } from '../../share/share.service';
 import { SearchService } from '../../search/search.service';
 import { PageRepo } from '@docmost/db/repos/page/page.repo';
 import { jsonToMarkdown } from '../../../collaboration/collaboration.util';
-import { modelFriendlyInput } from './model-friendly-input';
 
 /**
  * Isolated, READ-ONLY toolset for the ANONYMOUS public-share assistant.
@@ -53,7 +52,7 @@ export class PublicShareChatToolsService {
           '(key terms and entities), not a full sentence. If the first ' +
           'results look weak, search again with different wording before ' +
           'answering. Only pages inside this share are ever returned.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           query: z.string().describe('The search query.'),
           limit: z
             .number()
@@ -88,7 +87,7 @@ export class PublicShareChatToolsService {
           'Markdown, by its page id. Returns the page title and its Markdown ' +
           'content. Only pages inside this share can be read; reading any ' +
           'other page fails.',
-        inputSchema: modelFriendlyInput({
+        inputSchema: z.object({
           pageId: z
             .string()
             .describe('The id (or slugId) of a page within this share.'),
@@ -143,7 +142,7 @@ export class PublicShareChatToolsService {
           'List the pages (titles + ids) that make up THIS published ' +
           'documentation share, so you can orient yourself before reading or ' +
           'searching. Only pages inside this share are listed.',
-        inputSchema: modelFriendlyInput({}),
+        inputSchema: z.object({}),
         execute: async () => {
           // Reuse the same share-tree logic the public /shares/tree route uses:
           // it validates the share + workspace, excludes restricted subtrees,

apps/server/src/core/page/services/page.service.spec.ts

@@ -3,22 +3,6 @@ import { PageService } from './page.service';
 import { MovePageDto } from '../dto/move-page.dto';
 import { Page } from '@docmost/db/types/entity.types';
 
-// A permissive chainable Proxy stands in for the locked Kysely trx so the
-// FOR-UPDATE lock query chains inside executeTx(this.db, ...) resolve. Shared by
-// every spec that drives a transactional write (movePage cycle guard, movePage
-// provenance, movePageToSpace).
-const makeChain = () => {
-  const c: any = new Proxy(function () {}, {
-    get: (_t, p) =>
-      p === 'then'
-        ? undefined
-        : p === 'execute' || p === 'executeTakeFirst'
-          ? () => Promise.resolve([])
-          : () => c,
-  });
-  return c;
-};
-
 // Direct instantiation with stub deps. The Test.createTestingModule form failed
 // to resolve the @InjectKysely()/@InjectQueue() tokens at compile(), and this
 // smoke test only needs the service to construct.
@@ -55,14 +39,12 @@ describe('PageService', () => {
     // Build a PageService whose pageRepo (findById/updatePage) and own
     // getPageBreadCrumbs are mockable, while every other collaborator stays a
     // bare stub. We only need to drive the three cycle-guard branches, so we
-    // mock minimally rather than standing up the whole DI graph. The trx stub
-    // comes from the shared module-level `makeChain` helper.
+    // mock minimally rather than standing up the whole DI graph.
     const makeService = (overrides?: {
       breadcrumbs?: Array<{ id: string }>;
     }) => {
       const pageRepo = {
-        // Destination parent lookup: a valid, non-deleted, same-space page. Also
-        // serves the FOR-UPDATE lock reads inside the transaction.
+        // Destination parent lookup: a valid, non-deleted, same-space page.
         findById: jest.fn().mockResolvedValue({
           id: 'dest-parent',
           deletedAt: null,
@@ -75,19 +57,11 @@ describe('PageService', () => {
 
       const eventEmitter = { emit: jest.fn() };
 
-      // Re-parenting under a concrete parent now runs through
-      // executeTx(this.db, ...), which calls db.transaction().execute(fn). The
-      // trxStub is the value handed to the callback (the locked transaction).
-      const trxStub = makeChain();
-      const db = {
-        transaction: jest.fn(() => ({ execute: (fn: any) => fn(trxStub) })),
-      };
-
       const svc = new PageService(
         pageRepo as any, // pageRepo
         {} as any, // pagePermissionRepo
         {} as any, // attachmentRepo
-        db as any, // db
+        {} as any, // db
         {} as any, // storageService
         {} as any, // attachmentQueue
         {} as any, // aiQueue
@@ -105,7 +79,7 @@ describe('PageService', () => {
         .spyOn(svc, 'getPageBreadCrumbs')
         .mockResolvedValue((overrides?.breadcrumbs ?? []) as any);
 
-      return { svc, pageRepo, eventEmitter, trxStub, db };
+      return { svc, pageRepo, eventEmitter };
     };
 
     // movePage takes `movedPage` as a param. Keep its parentPageId distinct from
@@ -172,65 +146,6 @@ describe('PageService', () => {
       await expect(svc.movePage(dto, makeMovedPage())).resolves.not.toThrow();
       expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
     });
-
-    it('serializes a legitimate re-parent under FOR UPDATE in canonical lock order (#159 #9)', async () => {
-      // Destination's ancestor chain does NOT contain the moved page -> no cycle.
-      const { svc, pageRepo, trxStub } = makeService({
-        breadcrumbs: [{ id: 'dest-parent' }, { id: 'root' }],
-      });
-      const getBreadcrumbsSpy = jest.spyOn(svc, 'getPageBreadCrumbs');
-      const dto: MovePageDto = {
-        pageId: 'page-1',
-        position: VALID_POSITION,
-        parentPageId: 'dest-parent',
-      };
-
-      await expect(svc.movePage(dto, makeMovedPage())).resolves.not.toThrow();
-
-      // Both rows are locked FOR UPDATE inside the transaction: findById is
-      // called with { withLock: true, trx: <the locked tx> } for the moved page
-      // and the destination parent.
-      const lockCalls = pageRepo.findById.mock.calls.filter(
-        (c: any[]) => c[1]?.withLock === true,
-      );
-      expect(lockCalls).toHaveLength(2);
-      for (const call of lockCalls) {
-        expect(call[1].withLock).toBe(true);
-        expect(call[1].trx).toBe(trxStub);
-      }
-
-      // Locks are acquired in a canonical (id-sorted) order so the two opposing
-      // moves serialize without deadlocking.
-      const lockedIds = lockCalls.map((c: any[]) => c[0]);
-      expect(lockedIds).toEqual(['page-1', 'dest-parent'].sort());
-
-      // The cycle re-check runs inside the locked transaction (trx passed).
-      expect(getBreadcrumbsSpy).toHaveBeenCalledWith('dest-parent', trxStub);
-
-      // The update is written inside the same transaction (trx is the 3rd arg).
-      expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
-      expect(pageRepo.updatePage.mock.calls[0][2]).toBe(trxStub);
-    });
-
-    it('moves a page to root WITHOUT a transaction (no cycle possible)', async () => {
-      // A move-to-root (parentPageId === null) can never create a cycle, so it
-      // takes the unlocked else-branch: updatePage runs with NO trx and the
-      // db.transaction() serialization path is skipped entirely.
-      const { svc, pageRepo, db } = makeService();
-      const dto: MovePageDto = {
-        pageId: 'page-1',
-        position: VALID_POSITION,
-        parentPageId: null,
-      };
-
-      await expect(svc.movePage(dto, makeMovedPage())).resolves.not.toThrow();
-
-      // No FOR-UPDATE serialization: the transaction was never opened.
-      expect(db.transaction).not.toHaveBeenCalled();
-      // The update is written outside any transaction (3rd arg is undefined).
-      expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
-      expect(pageRepo.updatePage.mock.calls[0][2]).toBeUndefined();
-    });
   });
 
   describe('agent provenance stamping (#143)', () => {
@@ -344,8 +259,6 @@ describe('PageService', () => {
 
     describe('movePage() → updatePage', () => {
       const VALID_POSITION = 'a0';
-      // Re-parenting under a concrete parent runs through executeTx(this.db, ...);
-      // the shared `makeChain` helper stands in for the locked Kysely trx.
       const run = async (provenance: any) => {
         const pageRepo = {
           findById: jest.fn().mockResolvedValue({
@@ -355,12 +268,9 @@ describe('PageService', () => {
           }),
           updatePage: jest.fn().mockResolvedValue({ numUpdatedRows: 1n }),
         };
-        const trxStub = makeChain();
         const svc = makeSvc({
           pageRepo,
-          db: {
-            transaction: () => ({ execute: (fn: any) => fn(trxStub) }),
-          } as any,
+          db: {} as any,
         });
         // Legitimate move: destination ancestors do NOT include the moved page.
         jest
@@ -406,9 +316,20 @@ describe('PageService', () => {
 
     describe('movePageToSpace() → root-page updatePage', () => {
       // movePageToSpace runs its writes inside executeTx(this.db, cb), which
-      // calls this.db.transaction().execute(fn => fn(trx)). The shared
-      // `makeChain` helper stands in for the Kysely trx so arbitrary chains
-      // resolve.
+      // calls this.db.transaction().execute(fn => fn(trx)). A permissive
+      // chainable Proxy stands in for the Kysely trx so arbitrary chains resolve.
+      const makeChain = () => {
+        const c: any = new Proxy(function () {}, {
+          get: (_t, p) =>
+            p === 'then'
+              ? undefined
+              : p === 'execute' || p === 'executeTakeFirst'
+                ? () => Promise.resolve([])
+                : () => c,
+        });
+        return c;
+      };
+
       const run = async (provenance: any) => {
         const trxStub = makeChain();
         const db = {

apps/server/src/core/page/services/page.service.ts

@@ -15,13 +15,13 @@ import {
   executeWithCursorPagination,
 } from '@docmost/db/pagination/cursor-pagination';
 import { InjectKysely } from 'nestjs-kysely';
-import { KyselyDB, KyselyTransaction } from '@docmost/db/types/kysely.types';
+import { KyselyDB } from '@docmost/db/types/kysely.types';
 import { generateJitteredKeyBetween } from 'fractional-indexing-jittered';
 import { MovePageDto } from '../dto/move-page.dto';
 import { shapeSidebarPagesTree } from './sidebar-pages-tree.util';
 import { generateSlugId } from '../../../common/helpers';
 import { getPageTitle } from '../../../common/helpers';
-import { executeTx, dbOrTx } from '@docmost/db/utils';
+import { executeTx } from '@docmost/db/utils';
 import { AttachmentRepo } from '@docmost/db/repos/attachment/attachment.repo';
 import { v7 as uuid7 } from 'uuid';
 import {
@@ -915,53 +915,34 @@ export class PageService {
       }
     }
 
-    // Cheap self-move guard (no DB) — keep before the transaction.
-    if (dto.parentPageId && dto.parentPageId === dto.pageId) {
-      throw new BadRequestException('Cannot move a page into its own subtree');
+    // Server-side cycle guard: a page may not be moved into itself or into any
+    // page within its own subtree. Without this, an MCP/REST/agent caller (or a
+    // fast drag racing the client check) could persist a cycle and broadcast it.
+    // Only relevant when re-parenting under a concrete parent; moving to root
+    // (parentPageId null/undefined) can never create a cycle.
+    if (dto.parentPageId) {
+      if (dto.parentPageId === dto.pageId) {
+        throw new BadRequestException('Cannot move a page into its own subtree');
+      }
+      // Walk the destination parent's ancestor chain (reusing the breadcrumb
+      // ancestor CTE). If the page being moved appears among those ancestors,
+      // the destination lives inside the moved page's subtree -> cycle.
+      const destAncestors = await this.getPageBreadCrumbs(dto.parentPageId);
+      if (destAncestors.some((ancestor) => ancestor.id === dto.pageId)) {
+        throw new BadRequestException('Cannot move a page into its own subtree');
+      }
     }
 
-    const updateValues = {
-      position: dto.position,
-      parentPageId: parentPageId,
-      // Agent-edit provenance: annotate the source on an agent move. A normal
-      // user request leaves the existing source value unchanged.
-      ...agentSourceFields(provenance, 'lastUpdatedSource', 'lastUpdatedAiChatId'),
-    };
-
-    let updateResult;
-    if (typeof parentPageId === 'string') {
-      // Genuine re-parent under a concrete parent: the ONLY path that can create
-      // a cycle. Two opposing moves (A: X under Y, B: Y under X) racing each
-      // other could each pass a cycle check built from a stale snapshot and
-      // persist a cycle (#159 finding #9). Serialize them: lock the moved page
-      // and the destination parent FOR UPDATE in a canonical (id-sorted) order
-      // so they cannot deadlock, then run the cycle check INSIDE the transaction
-      // against the now-committed state.
-      updateResult = await executeTx(this.db, async (trx) => {
-        // Both opposing moves touch the same two rows {pageId, parentPageId};
-        // a fixed lock order forces one to wait for the other to commit. Lock by
-        // canonical UUIDs — `dto.pageId` can be a slugId (MovePageDto.pageId is a
-        // bare @IsString), so two opposing moves passing slugIds could sort into
-        // different lock orders and deadlock (AB-BA). `movedPage.id` is the
-        // resolved row UUID, matching `parentPageId`.
-        const lockIds = [movedPage.id, parentPageId].sort();
-        for (const id of lockIds) {
-          await this.pageRepo.findById(id, { withLock: true, trx });
-        }
-        // Re-read the destination's ancestor chain within the locked tx: it now
-        // reflects any concurrent re-parent that committed before we got the lock.
-        const destAncestors = await this.getPageBreadCrumbs(parentPageId, trx);
-        if (destAncestors.some((ancestor) => ancestor.id === dto.pageId)) {
-          throw new BadRequestException(
-            'Cannot move a page into its own subtree',
-          );
-        }
-        return this.pageRepo.updatePage(updateValues, dto.pageId, trx);
-      });
-    } else {
-      // Same-parent reorder or move-to-root: no cycle possible, no lock needed.
-      updateResult = await this.pageRepo.updatePage(updateValues, dto.pageId);
-    }
+    const updateResult = await this.pageRepo.updatePage(
+      {
+        position: dto.position,
+        parentPageId: parentPageId,
+        // Agent-edit provenance: annotate the source on an agent move. A normal
+        // user request leaves the existing source value unchanged.
+        ...agentSourceFields(provenance, 'lastUpdatedSource', 'lastUpdatedAiChatId'),
+      },
+      dto.pageId,
+    );
 
     // Guard against a phantom broadcast: if the row was concurrently deleted or
     // otherwise not updated, skip the PAGE_MOVED event so we don't replay a move
@@ -1000,8 +981,8 @@ export class PageService {
     });
   }
 
-  async getPageBreadCrumbs(childPageId: string, trx?: KyselyTransaction) {
-    const ancestors = await dbOrTx(this.db, trx)
+  async getPageBreadCrumbs(childPageId: string) {
+    const ancestors = await this.db
       .withRecursive('page_ancestors', (db) =>
         db
           .selectFrom('pages')

apps/server/test/integration/page-move-cycle-concurrency.int-spec.ts

@@ -1,122 +0,0 @@
-import { Kysely } from 'kysely';
-import { BadRequestException } from '@nestjs/common';
-import { PageRepo } from '@docmost/db/repos/page/page.repo';
-import { PageService } from '../../src/core/page/services/page.service';
-import {
-  getTestDb,
-  destroyTestDb,
-  createWorkspace,
-  createSpace,
-  createPage,
-} from './db';
-
-/**
- * #159 finding #9 — concurrent opposing page moves must not create a cycle.
- *
- * User A drags X under Y while user B simultaneously drags Y under X. Before the
- * fix, the cycle guard (a breadcrumb/ancestor read) and the parent-update write
- * were NOT in a transaction, so both moves ran against a stale snapshot, both
- * passed their cycle check, and both committed -> X.parent=Y AND Y.parent=X: a
- * cycle with no path to root, which breaks the recursive ancestor CTEs and makes
- * both subtrees vanish from the sidebar.
- *
- * The fix wraps the cycle check + update in one READ COMMITTED transaction and
- * locks the two involved rows FOR UPDATE in a canonical (id-sorted) order, then
- * re-runs the cycle check inside the lock. One move wins; the loser sees the
- * committed re-parent and trips the "own subtree" guard.
- *
- * NOTE: this runs against real Postgres in CI (the integration suite). There is
- * no local Postgres in dev, so it is not expected to run locally.
- */
-describe('movePage concurrent opposing moves do not create a cycle [integration]', () => {
-  let db: Kysely<any>;
-  let workspaceId: string;
-  let spaceId: string;
-
-  beforeAll(async () => {
-    db = getTestDb();
-    workspaceId = (await createWorkspace(db)).id;
-    spaceId = (await createSpace(db, workspaceId)).id;
-  });
-
-  afterAll(async () => {
-    await destroyTestDb();
-  });
-
-  it('lets exactly one opposing move win and never persists a cycle', async () => {
-    // Real collaborators against the test DB. movePage only touches db-backed
-    // methods on pageRepo plus the db itself and the event emitter (stubbed).
-    const pageRepo = new PageRepo(
-      db as any,
-      {} as any,
-      { emit: () => undefined } as any,
-    );
-    const svc = new PageService(
-      pageRepo as any, // pageRepo
-      {} as any, // pagePermissionRepo
-      {} as any, // attachmentRepo
-      db as any, // db
-      {} as any, // storageService
-      {} as any, // attachmentQueue
-      {} as any, // aiQueue
-      {} as any, // generalQueue
-      { emit: () => undefined } as any, // eventEmitter
-      {} as any, // collaborationGateway
-      {} as any, // watcherService
-      {} as any, // transclusionService
-    );
-
-    // Seed R (root), X and Y as children of R.
-    const root = await createPage(db, { workspaceId, spaceId, title: 'R' });
-    const pageX = await createPage(db, { workspaceId, spaceId, title: 'X' });
-    const pageY = await createPage(db, { workspaceId, spaceId, title: 'Y' });
-
-    // createPage does not set parentPageId; wire X.parent=R and Y.parent=R and
-    // give each a valid fractional-index position.
-    await db
-      .updateTable('pages')
-      .set({ parentPageId: root.id, position: 'a0' })
-      .where('id', 'in', [pageX.id, pageY.id])
-      .execute();
-
-    // The Page snapshots movePage receives (parentPageId must be R so each move
-    // is a genuine re-parent rather than a same-parent reorder).
-    const movedX = await pageRepo.findById(pageX.id);
-    const movedY = await pageRepo.findById(pageY.id);
-
-    // Two opposing moves racing: A moves X under Y, B moves Y under X.
-    const results = await Promise.allSettled([
-      svc.movePage(
-        { pageId: pageX.id, parentPageId: pageY.id, position: 'a1' },
-        movedX,
-      ),
-      svc.movePage(
-        { pageId: pageY.id, parentPageId: pageX.id, position: 'a1' },
-        movedY,
-      ),
-    ]);
-
-    // Exactly one fulfilled and one rejected; the rejection is the cycle guard.
-    const fulfilled = results.filter((r) => r.status === 'fulfilled');
-    const rejected = results.filter((r) => r.status === 'rejected');
-    expect(fulfilled).toHaveLength(1);
-    expect(rejected).toHaveLength(1);
-
-    const reason = (rejected[0] as PromiseRejectedResult).reason;
-    expect(reason).toBeInstanceOf(BadRequestException);
-    expect(String(reason?.message)).toContain('own subtree');
-
-    // No cycle persisted: re-fetch X and Y and assert NOT (X->Y AND Y->X).
-    const xNow = await pageRepo.findById(pageX.id);
-    const yNow = await pageRepo.findById(pageY.id);
-    const isCycle =
-      xNow.parentPageId === pageY.id && yNow.parentPageId === pageX.id;
-    expect(isCycle).toBe(false);
-
-    // Exactly one re-parent took effect: one of X/Y still points at R, the other
-    // points at its sibling.
-    const xWon = xNow.parentPageId === pageY.id && yNow.parentPageId === root.id;
-    const yWon = yNow.parentPageId === pageX.id && xNow.parentPageId === root.id;
-    expect(xWon || yWon).toBe(true);
-  });
-});