Compare commits

..

10 Commits

Author SHA1 Message Date
agent_coder e9e4c1028d perf(editor): cut per-keystroke work on the typing hot path (#343)
The editor lagged while typing (worse with doc size, and under collaboration the
same cost is paid for every REMOTE keystroke). ProseMirror itself was fine — the
overhead was the surrounding work done on every transaction. Behavior is 1:1;
only WHEN work runs changed.

- getJSON() off the keystroke path: `onUpdate` no longer serializes the whole doc
  synchronously — the serialization now runs inside a 3s debounce (new hook
  use-page-content-cache.ts), flushed on unmount so the last snapshot isn't lost.
- footnote numbering: merged 3 per-docChanged O(n) doc walks into one, and
  short-circuit the whole-doc renumber when the doc has no footnotes and the
  transaction didn't insert one (step-slice scan — covers typing/paste/collab).
- toolbar: replaced per-keystroke `editor.can().undo()/.redo()` dry-runs with
  cheap history-depth reads (Yjs undoManager stack length / pm-history depth).
- render side-effect bug: `remote.attach()` moved out of the render body into a
  useEffect.
- debounced the TOC all-headings rescan and memoized the slash-command suggestion
  build (was rebuilt twice per keystroke).
- node menus (image/video/audio/pdf/callout/subpages): the per-transaction
  selectors early-return a cheap isActive check instead of running getAttributes +
  multiple alignment probes while their node type is inactive (shouldShow still
  controls display — appears exactly when it did).
- code blocks: the global selectionUpdate listener is now added only for mermaid
  blocks (the only consumer of the selected state), eliminating N listeners +
  N setStates per caret move for normal code blocks.

Deferred (documented, collab hot-path risk): full conditional menu MOUNTING
(menu-less-frame risk on same-tx context switch) and code-block re-tokenization
debounce / language-persist (self-dispatching meta tx + node-attr writes interact
with collab/undo). The route split from #342 already keeps lowlight off startup.

Gate: editor-ext build + 252/252 tests, client editor tests pass, tsc --noEmit 0,
client build ok. New tests: footnote no-footnote-doc → 0 traversals + numbering
unchanged; page-content-cache onUpdate-no-sync-getJSON + flush-on-unmount.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 22:49:48 +03:00
agent_vscode 382e5196da Merge pull request 'fix(docker): toolchain python3/make/g++ для нативной сборки re2' (#353) from fix/docker-re2-toolchain into develop 2026-07-04 22:11:49 +03:00
agent_vscode 76e0c08cec fix(docker): install python3/make/g++ toolchain for re2 native build
The develop image build broke at `pnpm install --frozen-lockfile`: the new
native dependency re2@1.25.0 (packages/mcp, search_in_page #330) always
compiles from source under pnpm — its prebuilt-binary downloader
(install-artifact-from-github) cannot identify the GitHub repo because pnpm
does not populate npm_package_repository_*/npm_package_json env vars ("No
github repository was identified. Building locally ..."), and node:22-slim
ships no python3/make/g++ for the node-gyp fallback.

- builder stage: add a cache-friendly apt layer with python3 make g++
  before COPY; the stage is discarded so the toolchain may stay.
- installer stage: install the toolchain, run the prod install as the node
  user via `su node -c`, and purge the toolchain — all in one RUN layer so
  the final image stays slim and node_modules ownership needs no extra
  chown layer; USER node is restored right after.

Fixes the failed run 28715009124 (develop docker build); release.yml uses
the same Dockerfile and is covered too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 22:09:40 +03:00
vvzvlad 8978d69f3e Merge pull request 'fix(converter): стабильность round-trip image/медиа — «» ≡ absent (класс defaults-instability)' (#350) from fix/media-roundtrip-stability into develop
Reviewed-on: #350
2026-07-04 21:30:12 +03:00
agent_coder c192f2a2e1 test(prosemirror-markdown): pin the third state — explicit "" converges once, then idempotent
Reviewer addition to the round-trip stability matrix: besides "attr absent" and
"attr has a real value", a string attr in the empty-string class has a third,
degenerate state — a LITERAL "" (a user types alt/title/name in the editor then
deletes it, and Tiptap persists `attr: ""`, distinct from never-set). The fix's
`getAttribute(...) || null` coercion normalizes such a stored "" to the default
on the FIRST round-trip (a one-time "" -> null diff) and is byte-stable from the
SECOND round-trip on.

Adds a convergence contract to the reusable matrix helper (emptyStringClass flag
+ runConvergenceCase): pass 1 must converge the attr to its schema default (NOT
asserted byte-stable vs the "" input — that is the intended one-time
normalization); pass 2 must deep-equal pass 1 (idempotent thereafter). Driven for
every empty-string-class attr across image + the media family (image/drawio
alt+title, video alt via aria-label, pdf/attachment name, attachment mime).
Documents the one-time normalization so a future sync/QA diff does not flag the
single "" -> null change as converter corruption.

Gate: package suite 33 files / 682 tests passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 21:17:17 +03:00
vvzvlad d78b985062 Merge pull request 'perf(comment): статический рендер + ленивые редакторы + мемоизация панели (#340)' (#349) from fix/340-comment-panel-perf into develop
Reviewed-on: #349
2026-07-04 20:55:11 +03:00
agent_coder 2ce672709a fix(prosemirror-markdown): stabilize image round-trip — "" ≡ absent on parse (empty-string class)
A stored image authored without `alt` gained a phantom `alt: ""` on every
round-trip (`markdownToProseMirror(convertProseMirrorToMarkdown(doc))`): `marked`
renders `![](src)` as `<img alt="">`, and the stock tiptap Image `alt` parseHTML
(`getAttribute("alt")`) materialized the empty string where the original had no
attribute. That false diff is a real GS-EDIT-REVERT churn source — an agent /
git-sync touch of a page with an image mutates the stored JSON (`absent -> ""`),
producing phantom diffs that can overwrite live edits.

Fix is PARSE-SIDE ("" ≡ absent), so the RAW round-trip is idempotent — not only
the canonical form (history / stored JSON diff on the raw shape; masking it only
in canonicalize would leave that noise). `image.alt`/`title` parseHTML now coerce
`getAttribute(...) || null`, plus defense-in-depth `|| null` across the at-risk
empty-string class (video aria-label, drawio/excalidraw title+alt, pdf name,
attachment name+mime) matching the existing `image.caption || null` precedent.

NOTE — image `align` is NOT changed: it round-trips correctly (center via the
schema default "center", left/right via the `<!--img {...}-->` comment). Its
`toBeUndefined()` in the git-sync gate is canonical-form normalization, not a loss.

Intentional divergence from editor-ext: editor-ext's literal `alt` parseHTML
returns "" verbatim, but this coercion CONVERGES on editor-ext's real STORED
shape (an image inserted without alt has no `alt` attribute -> re-parses absent,
never ""), so the round-trip is idempotent and matches real documents.

Adds a reusable, node-agnostic round-trip-stability matrix helper
(test/roundtrip-stability.helper.ts) — given a node + attr spec it enumerates
default/non-default combos and asserts byte-stability of BOTH the raw and the
canonical round-trip (the documented numeric width/height→string coercion encoded
as an explicit allowed normalization) — driven over image + the whole media
family (video/audio/pdf/attachment/embed/drawio/excalidraw). The only raw
empty-string instability it found was image.alt; the family was already stable.

Gate: package suite 33 files / 672 tests passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 20:51:34 +03:00
vvzvlad c252068672 Merge pull request 'feat(ai-chat): отложенная загрузка инструментов (deferred tools + loadTools) (#332)' (#341) from fix/332-deferred-tools into develop
Reviewed-on: #341
2026-07-04 20:47:45 +03:00
agent_coder 68caf8157a test(ai-chat): document AI_CHAT_DEFERRED_TOOLS + pin ON-path & catalog completeness (#341 review F1-F3)
- F1: document AI_CHAT_DEFERRED_TOOLS in .env.example (AI_* section) — default
  ON = deferred loading (compact catalog + loadTools), =false restores the old
  "all tools always active" behavior.
- F2: integration test of the ON path in ai-chat-stream.int-spec.ts — a deferred
  tool activated via loadTools is active on the SAME turn's next step but a fresh
  turn starts cold (CORE + loadTools only), proving the per-turn activatedTools
  Set does not leak across turns/chats. Drives the real streamText loop with a
  MockLanguageModelV3 and inspects recorded per-step activeTools-filtered tools.
- F3: replace the magic toHaveLength(28) in tool-tiers.spec.ts with a two-way
  partition against the LIVE in-app toolset (AiChatToolsService.forUser keys):
  every non-core tool must appear in buildInAppDeferredCatalog and every catalog
  entry must map to a real non-core tool — so a future tool forgotten in
  INLINE_TOOL_TIERS fails the suite instead of silently vanishing from the agent.

No production logic change (mechanism was already reviewed correct).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 20:34:42 +03:00
claude code agent 227 e431b33bb1 feat(ai-chat): deferred tool loading (tiers + loadTools meta-tool) (#332)
The in-app AI agent shipped all ~41 tool schemas on every model step. This
adds a two-tier catalog: core tools (frequent or one-line) stay always-active;
the rest are advertised as a compact catalog and their full schema is fetched
on demand via the loadTools meta-tool, wired through ai@6 prepareStep's
per-step activeTools.

- tools/tool-tiers.ts: CORE_TOOL_KEYS, INLINE_TOOL_TIERS, applyLoadTools,
  catalog builders (+ tool-tiers.spec.ts, 13 cases).
- ai-chat.service.ts prepareAgentStep: returns activeTools =
  [...CORE_TOOL_KEYS, loadTools, ...activatedTools]; per-turn activated Set.
- ai-chat.prompt.ts: buildToolCatalogBlock renders the deferred catalog.
- mcp/tool-specs.ts: tier + catalogLine metadata (external snake_case /mcp
  transport unchanged).
- EnvironmentService.isAiChatDeferredToolsEnabled(): AI_CHAT_DEFERRED_TOOLS,
  default ON per issue intent (kill-switch =false restores old behavior).

Gate: server ai-chat 631/631, tool-tiers 13/13, mcp 472/472, tsc clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 19:57:11 +03:00
33 changed files with 2314 additions and 88 deletions
+7
View File
@@ -202,6 +202,13 @@ MCP_DOCMOST_PASSWORD=
# Default 900000 (15 min).
# AI_MCP_CALL_TIMEOUT_MS=900000
# Deferred tool loading for the in-app AI chat (#332). Default ON: the agent sees
# a compact <tool_catalog> and only CORE tools + a loadTools meta-tool are active
# each step; deferred tools (the fat/rare ones + all external MCP tools) load on
# demand. Set AI_CHAT_DEFERRED_TOOLS=false to restore the old "all tools always
# active" behavior.
# AI_CHAT_DEFERRED_TOOLS=true
# --- Anonymous public-share AI assistant ---
# Opt-in per workspace (AI settings -> "public share assistant"; off by default).
# When enabled, anonymous visitors of a published share can ask an AI about that
+16 -2
View File
@@ -5,6 +5,13 @@ RUN npm install -g pnpm@10.4.0
FROM base AS builder
# re2 (packages/mcp) always compiles from source under pnpm (the prebuilt-binary
# download cannot identify the GitHub repo), so node-gyp needs python3/make/g++.
# This stage is discarded, so the toolchain can stay installed.
RUN apt-get update \
&& apt-get install -y --no-install-recommends python3 make g++ \
&& rm -rf /var/lib/apt/lists/*
WORKDIR /app
COPY . .
@@ -57,9 +64,16 @@ COPY --from=builder /app/patches /app/patches
RUN chown -R node:node /app
USER node
# Toolchain is needed transiently to compile re2 during the prod install; install
# and purge it in one layer to keep the final image slim. The install itself runs
# as the node user via su to keep node_modules ownership without a costly chown layer.
RUN apt-get update \
&& apt-get install -y --no-install-recommends python3 make g++ \
&& su node -c "pnpm install --frozen-lockfile --prod" \
&& apt-get purge -y --auto-remove python3 make g++ \
&& rm -rf /var/lib/apt/lists/*
RUN pnpm install --frozen-lockfile --prod
USER node
RUN mkdir -p /app/data/storage
@@ -46,6 +46,13 @@ export function AudioMenu({ editor }: EditorMenuProps) {
return null;
}
// #343 PART 1: skip getAttributes unless an audio node is active. The menu
// only shows for an active audio node (shouldShow), so the null state while
// inactive is never rendered — behavior unchanged.
if (!ctx.editor.isActive("audio")) {
return null;
}
const audioAttrs = ctx.editor.getAttributes("audio");
return {
@@ -43,8 +43,15 @@ export function CalloutMenu({ editor }: EditorMenuProps) {
return null;
}
// #343 PART 1: skip the per-type isActive() probes unless a callout is
// active. The menu only shows for an active callout (shouldShow), so the
// null state while inactive is never rendered — behavior unchanged.
if (!ctx.editor.isActive("callout")) {
return null;
}
return {
isCallout: ctx.editor.isActive("callout"),
isCallout: true,
isInfo: ctx.editor.isActive("callout", { type: "info" }),
isNote: ctx.editor.isActive("callout", { type: "note" }),
isSuccess: ctx.editor.isActive("callout", { type: "success" }),
@@ -22,6 +22,12 @@ export default function CodeBlockView(props: NodeViewProps) {
const [isSelected, setIsSelected] = useState(false);
useEffect(() => {
// #343 PART 6: `isSelected` only drives the mermaid source's visibility (the
// `hidden` prop below). For every non-mermaid code block it is never read,
// so skip the per-block `selectionUpdate` listener entirely — otherwise N
// code blocks each add a global listener + a setState on every caret move.
if (language !== "mermaid") return;
const updateSelection = () => {
const { state } = editor;
const { from, to } = state.selection;
@@ -32,11 +38,14 @@ export default function CodeBlockView(props: NodeViewProps) {
setIsSelected(isNodeSelected);
};
// Initialize on attach so switching a block's language to "mermaid" reflects
// the current selection immediately (the listener was not running before).
updateSelection();
editor.on("selectionUpdate", updateSelection);
return () => {
editor.off("selectionUpdate", updateSelection);
};
}, [editor, getPos(), node.nodeSize]);
}, [editor, getPos(), node.nodeSize, language]);
function changeLanguage(language: string) {
setLanguageValue(language);
@@ -1,5 +1,7 @@
import type { Editor } from "@tiptap/react";
import { useEditorState } from "@tiptap/react";
import { undoDepth, redoDepth } from "@tiptap/pm/history";
import { yUndoPluginKey } from "@tiptap/y-tiptap";
export interface ToolbarState {
isBold: boolean;
@@ -16,14 +18,45 @@ export interface ToolbarState {
canRedo: boolean;
}
// Undo/redo come from either StarterKit's history or the Yjs collaboration
// history extension. During the brief moment a page is rendered with the
// static editor (mainExtensions only, undoRedo disabled), neither is loaded
// and editor.can().undo/redo is undefined.
function safeCan(editor: Editor, command: "undo" | "redo"): boolean {
const can = editor.can() as Record<string, unknown>;
const fn = can[command];
return typeof fn === "function" ? (fn as () => boolean)() : false;
// Undo/redo availability, computed WITHOUT `editor.can().undo()/.redo()`.
//
// `editor.can()` runs the command as a dry-run (building a throwaway state +
// transaction) — the most expensive work in this selector, and it ran on every
// keystroke (and every REMOTE keystroke under collaboration). Instead we read
// the history stack depth directly, which is a cheap plugin-state lookup and
// mirrors exactly what the undo/redo commands themselves check:
//
// - Collaboration (Yjs): the yjs UndoManager's undo/redo stack lengths — the
// same `undoStack.length === 0` / `redoStack.length === 0` guard the
// Collaboration extension's undo/redo commands use.
// - Plain history (templates / non-collab): prosemirror-history's undoDepth /
// redoDepth, which back the UndoRedo extension.
//
// When neither history backend is installed (the pre-sync static editor —
// mainExtensions only, undoRedo disabled), both fall through to 0 -> false,
// matching the previous `safeCan` behavior.
function historyAvailability(editor: Editor): {
canUndo: boolean;
canRedo: boolean;
} {
const state = editor.state;
// Collaboration history (Yjs) takes precedence when present.
const yState = yUndoPluginKey.getState(state) as
| { undoManager?: { undoStack: unknown[]; redoStack: unknown[] } }
| undefined;
if (yState?.undoManager) {
return {
canUndo: yState.undoManager.undoStack.length > 0,
canRedo: yState.undoManager.redoStack.length > 0,
};
}
// Plain prosemirror-history (returns 0 when the history plugin is absent).
return {
canUndo: undoDepth(state) > 0,
canRedo: redoDepth(state) > 0,
};
}
export function useToolbarState(editor: Editor | null): ToolbarState | null {
@@ -31,6 +64,7 @@ export function useToolbarState(editor: Editor | null): ToolbarState | null {
editor,
selector: (ctx) => {
if (!ctx.editor) return null;
const { canUndo, canRedo } = historyAvailability(ctx.editor);
return {
isBold: ctx.editor.isActive("bold"),
isItalic: ctx.editor.isActive("italic"),
@@ -42,8 +76,8 @@ export function useToolbarState(editor: Editor | null): ToolbarState | null {
isBulletList: ctx.editor.isActive("bulletList"),
isOrderedList: ctx.editor.isActive("orderedList"),
isTaskList: ctx.editor.isActive("taskList"),
canUndo: safeCan(ctx.editor, "undo"),
canRedo: safeCan(ctx.editor, "redo"),
canUndo,
canRedo,
};
},
});
@@ -38,6 +38,14 @@ export function ImageMenu({ editor }: EditorMenuProps) {
return null;
}
// #343 PART 1: skip the expensive per-keystroke work (getAttributes + the
// alignment isActive() probes) unless an image is actually active. The
// menu is only shown when an image is active (see shouldShow), so a null
// state while inactive is never rendered — behavior is unchanged.
if (!ctx.editor.isActive("image")) {
return null;
}
const imageAttrs = ctx.editor.getAttributes("image");
return {
@@ -25,6 +25,13 @@ export function PdfMenu({ editor }: EditorMenuProps) {
return null;
}
// #343 PART 1: skip getAttributes unless a pdf node is active. The menu
// only shows for an active pdf node (shouldShow), so the null state while
// inactive is never rendered — behavior unchanged.
if (!ctx.editor.isActive("pdf")) {
return null;
}
const pdfAttrs = ctx.editor.getAttributes("pdf");
return {
@@ -70,7 +70,14 @@ export const SubpagesMenu = React.memo(
// toggle without re-rendering on every keystroke.
const isRecursive = useEditorState({
editor,
selector: (ctx) => ctx.editor?.getAttributes("subpages")?.recursive ?? false,
// #343 PART 1: skip getAttributes unless a subpages node is active. The
// menu only shows for an active subpages node (shouldShow), so the value
// is only read then; getAttributes on an inactive node returns the default
// (recursive === false) anyway, so this is behavior-preserving.
selector: (ctx) =>
ctx.editor?.isActive("subpages")
? (ctx.editor.getAttributes("subpages")?.recursive ?? false)
: false,
});
return (
@@ -4,6 +4,7 @@ import React, { FC, useEffect, useRef, useState } from "react";
import classes from "./table-of-contents.module.css";
import clsx from "clsx";
import { Box, Text, Title } from "@mantine/core";
import { useDebouncedCallback } from "@mantine/hooks";
import { useTranslation } from "react-i18next";
type TableOfContentsProps = {
@@ -79,13 +80,21 @@ export const TableOfContents: FC<TableOfContentsProps> = (props) => {
setHeadingDOMNodes(result.nodes);
};
// Debounce the update-driven rescan: `$nodes("heading")` scans every heading
// in the document, and it previously ran on EVERY keystroke while the TOC
// panel was open. The panel is derived UI, so recomputing ~300ms after typing
// settles keeps it correct without doing an all-headings scan per keystroke
// (#343, PART 7). `useDebouncedCallback` returns a stable reference and always
// invokes the latest `handleUpdate`.
const debouncedHandleUpdate = useDebouncedCallback(handleUpdate, 300);
useEffect(() => {
props.editor?.on("update", handleUpdate);
props.editor?.on("update", debouncedHandleUpdate);
return () => {
props.editor?.off("update", handleUpdate);
props.editor?.off("update", debouncedHandleUpdate);
};
}, [props.editor]);
}, [props.editor, debouncedHandleUpdate]);
useEffect(
() => {
@@ -31,6 +31,13 @@ export function VideoMenu({ editor }: EditorMenuProps) {
return null;
}
// #343 PART 1: skip getAttributes + alignment isActive() probes unless a
// video is active. The menu only shows for an active video (shouldShow),
// so the null state while inactive is never rendered — behavior unchanged.
if (!ctx.editor.isActive("video")) {
return null;
}
const videoAttrs = ctx.editor.getAttributes("video");
return {
@@ -6,6 +6,23 @@ import getSuggestionItems from '@/features/editor/components/slash-menu/menu-ite
export const slashMenuPluginKey = new PluginKey('slash-command');
// getSuggestionItems fuzzy-matches EVERY command against the query (plus its
// wrong-keyboard-layout remaps) and, while the slash menu is open, is invoked
// TWICE per keystroke: once by the synchronous `allow` gate below and once by
// the popup's `items` builder. A synchronous gating predicate can't be
// debounced without breaking the suggestion decoration/activation, so instead we
// memoize the LAST query's result: the two same-query calls in one keystroke
// build the list only once, and the cache invalidates the moment the query
// changes — so there is no stale-state risk (#343, PART 7).
let lastQuery: string | null = null;
let lastResult: ReturnType<typeof getSuggestionItems> | null = null;
function suggestionItemsForQuery(query: string) {
if (query === lastQuery && lastResult) return lastResult;
lastQuery = query;
lastResult = getSuggestionItems({ query });
return lastResult;
}
// @ts-ignore
const Command = Extension.create({
name: 'slash-command',
@@ -38,7 +55,7 @@ const Command = Extension.create({
// non-matching queries while keeping multi-word matches (e.g.
// "/Heading 1") working.
const query = state.doc.textBetween(range.from + 1, range.to);
const groups = getSuggestionItems({ query });
const groups = suggestionItemsForQuery(query);
const hasMatches = Object.values(groups).some(
(items) => items.length > 0,
);
@@ -61,7 +78,9 @@ const Command = Extension.create({
const SlashCommand = Command.configure({
suggestion: {
items: getSuggestionItems,
// Share the per-query memo with `allow` so the pair of same-query calls in a
// single keystroke rebuilds the list once (#343, PART 7).
items: ({ query }: { query: string }) => suggestionItemsForQuery(query),
render: renderItems,
},
});
@@ -0,0 +1,100 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { renderHook, act } from "@testing-library/react";
import type { MutableRefObject } from "react";
import type { Editor } from "@tiptap/react";
// Mock the app entry so importing the hook doesn't boot the whole app; the hook
// only needs queryClient's cache read/write, which we stub here. Declared via
// vi.hoisted so the spies exist before the hoisted vi.mock factory runs.
const { getQueryData, setQueryData } = vi.hoisted(() => ({
getQueryData: vi.fn(() => undefined as unknown),
setQueryData: vi.fn(),
}));
vi.mock("@/main.tsx", () => ({
queryClient: { getQueryData, setQueryData },
}));
import { usePageContentCache } from "./use-page-content-cache";
const SNAPSHOT = { type: "doc", content: [] };
function makeFakeEditor(overrides: Partial<Editor> = {}): Editor {
return {
isEmpty: false,
isDestroyed: false,
getJSON: vi.fn(() => SNAPSHOT),
...overrides,
} as unknown as Editor;
}
describe("usePageContentCache (#343 PART 3) — getJSON off the keystroke path", () => {
beforeEach(() => {
vi.useFakeTimers();
vi.clearAllMocks();
// A cached page exists so the write path runs.
getQueryData.mockReturnValue({ id: "p1", content: {} });
});
afterEach(() => {
vi.useRealTimers();
});
it("onUpdate (calling the debounced fn) does NOT call getJSON synchronously", () => {
const editor = makeFakeEditor();
const editorRef = { current: editor } as MutableRefObject<Editor | null>;
const { result } = renderHook(() =>
usePageContentCache(editorRef, "slug-1", 3000),
);
// Simulate a keystroke's onUpdate -> only schedules the debounce.
act(() => {
result.current();
result.current();
result.current();
});
// The whole-doc serialization must NOT have happened yet.
expect(editor.getJSON).not.toHaveBeenCalled();
expect(setQueryData).not.toHaveBeenCalled();
// Once the debounce window elapses, getJSON runs exactly once (not per call).
act(() => vi.advanceTimersByTime(3000));
expect(editor.getJSON).toHaveBeenCalledTimes(1);
expect(setQueryData).toHaveBeenCalledWith(["pages", "slug-1"], {
id: "p1",
content: SNAPSHOT,
});
});
it("flushes the pending snapshot on unmount so the last edit isn't lost", () => {
const editor = makeFakeEditor();
const editorRef = { current: editor } as MutableRefObject<Editor | null>;
const { result, unmount } = renderHook(() =>
usePageContentCache(editorRef, "slug-1", 3000),
);
act(() => result.current());
expect(editor.getJSON).not.toHaveBeenCalled();
// Navigation/unmount must flush (not drop) the pending write.
act(() => unmount());
expect(editor.getJSON).toHaveBeenCalledTimes(1);
expect(setQueryData).toHaveBeenCalledTimes(1);
});
it("skips the write when the editor is destroyed (flush racing teardown)", () => {
const editor = makeFakeEditor({ isDestroyed: true });
const editorRef = { current: editor } as MutableRefObject<Editor | null>;
const { result } = renderHook(() =>
usePageContentCache(editorRef, "slug-1", 3000),
);
act(() => result.current());
act(() => vi.advanceTimersByTime(3000));
expect(editor.getJSON).not.toHaveBeenCalled();
expect(setQueryData).not.toHaveBeenCalled();
});
});
@@ -0,0 +1,50 @@
import type { MutableRefObject } from "react";
import { useDebouncedCallback } from "@mantine/hooks";
import type { Editor } from "@tiptap/react";
import { queryClient } from "@/main.tsx";
import { IPage } from "@/features/page/types/page.types.ts";
/**
* Off-keystroke local page-cache updater (issue #343, PART 3).
*
* The editor's `onUpdate` fires on every keystroke — and, under collaboration,
* on every REMOTE keystroke too. Serializing the WHOLE document with
* `editor.getJSON()` on that hot path is expensive, and the previous 3s debounce
* only guarded the cache WRITE, not the serialization: `getJSON()` still ran per
* keystroke.
*
* This hook moves the serialization INSIDE the debounced callback, so the
* full-doc traversal happens at most once per `delay`, not per keystroke. Call
* the returned function from `onUpdate` (it only schedules the debounce); the
* `getJSON()` snapshot is taken when the debounce fires.
*
* On unmount/navigation the pending snapshot is FLUSHED (via `flushOnUnmount`)
* so the last edits within the debounce window aren't lost from the local cache.
* The source of truth is collab/Yjs, but the cache must not go stale.
*
* IMPORTANT: call this hook BEFORE `useEditor`. React runs effect cleanups in
* declaration order on unmount, so the debounce's flush cleanup must be declared
* before `useEditor`'s teardown to run while the editor is still alive; the
* `isDestroyed` guard keeps a flush that still races teardown safe (it skips).
*/
export function usePageContentCache(
editorRef: MutableRefObject<Editor | null>,
slugId: string | undefined,
delay = 3000,
) {
return useDebouncedCallback(
() => {
const e = editorRef.current;
if (!e || e.isDestroyed || e.isEmpty) return;
const pageData = queryClient.getQueryData<IPage>(["pages", slugId]);
if (pageData) {
// getJSON() (full-doc serialization) runs HERE, off the keystroke path.
queryClient.setQueryData(["pages", slugId], {
...pageData,
content: e.getJSON(),
});
}
},
{ delay, flushOnUnmount: true },
);
}
+20 -19
View File
@@ -62,7 +62,7 @@ import ExcalidrawMenu from "./components/excalidraw/excalidraw-menu-lazy";
import DrawioMenu from "./components/drawio/drawio-menu";
import { useCollabToken } from "@/features/auth/queries/auth-query.tsx";
import SearchAndReplaceDialog from "@/features/editor/components/search-and-replace/search-and-replace-dialog.tsx";
import { useDebouncedCallback, useDocumentVisibility } from "@mantine/hooks";
import { useDocumentVisibility } from "@mantine/hooks";
import { useIdle } from "@/hooks/use-idle.ts";
import { queryClient } from "@/main.tsx";
import { IPage } from "@/features/page/types/page.types.ts";
@@ -79,6 +79,7 @@ import { PageEditMode } from "@/features/user/types/user.types.ts";
import { jwtDecode } from "jwt-decode";
import { searchSpotlight } from "@/features/search/constants.ts";
import { useEditorScroll } from "./hooks/use-editor-scroll";
import { usePageContentCache } from "./hooks/use-page-content-cache";
import { useScrollRestoreOnSwap } from "./hooks/use-scroll-position";
import { useSwapHeightReservation } from "./hooks/use-swap-height-reservation";
import { EditorLinkMenu } from "@/features/editor/components/link/link-menu";
@@ -267,8 +268,13 @@ export default function PageEditor({
}
}, [isIdle, documentState, providersReady, resetIdle]);
// Attach here, to make sure the connection gets properly established
providersRef.current?.remote.attach();
// Attach the remote provider once it's ready (and again after a pageId swap
// recreates it) to make sure the connection gets properly established. This
// used to run in the render body — a side effect during render (#343, PART 7).
// `attach()` is idempotent, so re-running it on these deps is safe.
useEffect(() => {
providersRef.current?.remote.attach();
}, [providersReady, pageId]);
const extensions = useMemo(() => {
if (!providersReady || !providersRef.current || !currentUser?.user) {
@@ -283,6 +289,12 @@ export default function PageEditor({
];
}, [providersReady, currentUser?.user]);
// getJSON() serialization + cache write live in the hook, off the keystroke
// path, and flush on unmount so the last snapshot survives navigation (#343).
// MUST be declared before useEditor: React runs effect cleanups in declaration
// order on unmount, so the flush must run before the editor is torn down.
const debouncedUpdateContent = usePageContentCache(editorRef, slugId);
const editor = useEditor(
{
extensions,
@@ -353,11 +365,11 @@ export default function PageEditor({
editorRef.current = editor;
}
},
onUpdate({ editor }) {
if (editor.isEmpty) return;
const editorJson = editor.getJSON();
//update local page cache to reduce flickers
debouncedUpdateContent(editorJson);
onUpdate() {
// Only schedule the debounce here — the whole-doc getJSON() serialization
// happens INSIDE the debounced callback (see usePageContentCache), so it
// no longer runs synchronously on every (local or remote) keystroke.
debouncedUpdateContent();
},
},
[pageId, editable, extensions],
@@ -403,17 +415,6 @@ export default function PageEditor({
};
}, [editor, pageId, editorIsEditable]);
const debouncedUpdateContent = useDebouncedCallback((newContent: any) => {
const pageData = queryClient.getQueryData<IPage>(["pages", slugId]);
if (pageData) {
queryClient.setQueryData(["pages", slugId], {
...pageData,
content: newContent,
});
}
}, 3000);
const handleActiveCommentEvent = (event) => {
const { commentId, resolved } = event.detail;
@@ -1,4 +1,8 @@
import { buildSystemPrompt, buildMcpToolingBlock } from './ai-chat.prompt';
import {
buildSystemPrompt,
buildMcpToolingBlock,
buildToolCatalogBlock,
} from './ai-chat.prompt';
import { Workspace } from '@docmost/db/types/entity.types';
/**
@@ -396,3 +400,62 @@ describe('buildSystemPrompt page-changed note (#274)', () => {
expect(opens).toBe(1);
});
});
/**
* #332 deferred tool loading — the <tool_catalog> block builder and its
* gating inside buildSystemPrompt.
*/
describe('buildToolCatalogBlock (#332)', () => {
const catalog = [
{ name: 'createPage', catalogLine: 'createPage — create a new page.' },
{ name: 'transformPage', catalogLine: 'transformPage — run a JS transform.' },
];
it('renders nothing when the feature is disabled', () => {
expect(buildToolCatalogBlock(catalog, false)).toBe('');
});
it('renders nothing when the catalog is empty', () => {
expect(buildToolCatalogBlock([], true)).toBe('');
expect(buildToolCatalogBlock(undefined, true)).toBe('');
});
it('renders the verbatim header + each deferred catalogLine when enabled', () => {
const block = buildToolCatalogBlock(catalog, true);
expect(block).toContain('<tool_catalog note="deferred tools;');
expect(block).toContain('NEVER tell the user you lack a capability');
expect(block).toContain('Deferred tools (name — purpose):');
expect(block).toContain('- createPage — create a new page.');
expect(block).toContain('- transformPage — run a JS transform.');
expect(block).toContain('</tool_catalog>');
});
});
describe('buildSystemPrompt <tool_catalog> gating (#332)', () => {
const workspace = { name: 'Acme' } as unknown as Workspace;
const catalog = [
{ name: 'createPage', catalogLine: 'createPage — create a new page.' },
];
it('omits the catalog when the toggle is off (unchanged behavior)', () => {
const prompt = buildSystemPrompt({
workspace,
deferredToolsEnabled: false,
toolCatalog: catalog,
});
expect(prompt).not.toContain('<tool_catalog');
expect(prompt).not.toContain('createPage — create a new page.');
});
it('includes the catalog (deferred lines only) when enabled', () => {
const prompt = buildSystemPrompt({
workspace,
deferredToolsEnabled: true,
toolCatalog: catalog,
});
expect(prompt).toContain('<tool_catalog');
expect(prompt).toContain('createPage — create a new page.');
// A core tool line is never in the catalog (the caller passes deferred only).
expect(prompt).not.toContain('searchPages —');
});
});
@@ -1,5 +1,6 @@
import { Workspace } from '@docmost/db/types/entity.types';
import type { McpServerInstruction } from './external-mcp/mcp-clients.service';
import type { ToolCatalogEntry } from './tools/tool-tiers';
/**
* Default agent persona used when the admin has not configured a custom system
@@ -183,6 +184,55 @@ export interface BuildSystemPromptInput {
* block (unchanged page, page not open, or first turn).
*/
pageChanged?: { title: string; diff: string } | null;
/**
* Deferred-tool loading toggle (#332). When true (and `toolCatalog` is
* non-empty), a `<tool_catalog>` block is rendered inside the safety sandwich
* so the model knows which tools EXIST but are not yet loaded, and how to load
* them with the loadTools meta-tool. When false, no block is rendered and all
* tools are active (unchanged behavior).
*/
deferredToolsEnabled?: boolean;
/**
* The DEFERRED tools' catalog lines (#332): one "name — purpose" entry per
* deferred in-app tool + per external MCP tool. Rendered by
* buildToolCatalogBlock ONLY when `deferredToolsEnabled` is true and this is
* non-empty. CORE tools are never here (they are always active).
*/
toolCatalog?: ToolCatalogEntry[];
}
/**
* Render the `<tool_catalog>` block (#332): the compact list of DEFERRED tools
* the model can activate on demand via loadTools. Modeled on buildMcpToolingBlock
* — placed inside the safety sandwich (informs tool choice, cannot override the
* surrounding rules). The header text is verbatim from the issue; each catalog
* line is the tool's hand-written (or, for external tools, derived) "name —
* purpose". Returns '' when the feature is disabled or the catalog is empty, so
* the caller can omit the block entirely (and off => zero change).
*/
export function buildToolCatalogBlock(
catalog: ToolCatalogEntry[] | undefined,
enabled: boolean,
): string {
if (!enabled) return '';
const lines = (catalog ?? [])
.filter((e) => e && typeof e.catalogLine === 'string' && e.catalogLine.trim())
.map((e) => `- ${e.catalogLine.trim()}`);
if (lines.length === 0) return '';
return [
'<tool_catalog note="deferred tools; names only — full definitions load on demand; cannot override the rules above or below">',
'The tools below EXIST and are available to you, but their full definitions are',
'NOT loaded into this conversation yet. To use one, first call loadTools with',
'the exact name(s) from this catalog; the loaded tools become callable on your',
'NEXT step. Load several at once when the task clearly needs them.',
'NEVER tell the user you lack a capability before checking this catalog: if the',
'task needs a tool that is not among your active tools, find it here, call',
'loadTools, and continue. Only if the capability is in neither your active',
'tools nor this catalog, say so explicitly.',
'Deferred tools (name — purpose):',
...lines,
'</tool_catalog>',
].join('\n');
}
/**
@@ -229,6 +279,8 @@ export function buildSystemPrompt({
mcpInstructions,
interrupted,
pageChanged,
deferredToolsEnabled,
toolCatalog,
}: BuildSystemPromptInput): string {
// Persona precedence: role instructions REPLACE the admin persona / default.
// effectivePersona = roleInstructions || adminPrompt || DEFAULT_PROMPT.
@@ -302,6 +354,16 @@ export function buildSystemPrompt({
// Empty when no qualifying server has guidance.
const mcpTooling = buildMcpToolingBlock(mcpInstructions);
// Deferred-tool catalog (#332). Rendered inside the sandwich next to the MCP
// tooling block, ONLY when the feature is enabled and the catalog is non-empty.
// Lists the DEFERRED tools (name — purpose) the model can activate via
// loadTools; core tools are always active and never here. Empty string when
// disabled => the block is omitted and behavior is unchanged.
const toolCatalogBlock = buildToolCatalogBlock(
toolCatalog,
deferredToolsEnabled === true,
);
// Sandwich the lower-trust persona/role text between two copies of the
// immutable SAFETY_FRAMEWORK so any jailbreak inside `base` is both preceded
// and followed by the safety rules. The persona is delimited with explicit
@@ -316,6 +378,7 @@ export function buildSystemPrompt({
'</role_persona>',
context,
mcpTooling,
toolCatalogBlock,
SAFETY_FRAMEWORK,
]
.filter((part) => part !== '')
@@ -53,6 +53,7 @@ describe('AiChatService.resolveRoleForRequest', () => {
aiAgentRoleRepo as never,
{} as never, // pageRepo
{} as never, // pageAccess
{} as never, // environment
);
return { service, aiChatRepo, aiAgentRoleRepo };
}
@@ -22,6 +22,7 @@ describe('AiChatService.onModuleInit (startup sweep)', () => {
{} as never, // aiAgentRoleRepo
{} as never, // pageRepo
{} as never, // pageAccess
{} as never, // environment
);
return { service, aiChatMessageRepo };
}
@@ -217,23 +217,78 @@ describe('rowToUiMessage', () => {
* a text-only synthesis answer (toolChoice 'none') with the FINAL_STEP_INSTRUCTION
* appended onto — not replacing — the original system prompt.
*/
// Narrowing helpers for the prepareAgentStep union return type.
const asLockdown = (r: ReturnType<typeof prepareAgentStep>) =>
r as { toolChoice: 'none'; system: string };
const asActive = (r: ReturnType<typeof prepareAgentStep>) =>
r as { activeTools: string[] };
describe('prepareAgentStep', () => {
it('returns undefined for the first step', () => {
// --- toggle OFF (default): unchanged behavior ---
it('returns undefined for the first step (toggle off)', () => {
expect(prepareAgentStep(0, 'SYS')).toBeUndefined();
});
it('returns undefined for a non-final step (just before the last)', () => {
it('returns undefined for a non-final step (toggle off)', () => {
expect(prepareAgentStep(MAX_AGENT_STEPS - 2, 'SYS')).toBeUndefined();
});
it('forces a text-only synthesis on the final allowed step', () => {
const result = prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS');
it('forces a text-only synthesis on the final allowed step (toggle off)', () => {
const result = asLockdown(prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS'));
expect(result).toBeDefined();
expect(result?.toolChoice).toBe('none');
expect(result.toolChoice).toBe('none');
// The original persona is preserved (prefix), not replaced.
expect(result?.system.startsWith('SYS')).toBe(true);
expect(result.system.startsWith('SYS')).toBe(true);
// The synthesis instruction is appended.
expect(result?.system).toContain(FINAL_STEP_INSTRUCTION);
expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
});
it('does NOT narrow activeTools when the toggle is off', () => {
const result = prepareAgentStep(0, 'SYS', new Set(['createPage']), false);
expect(result).toBeUndefined();
});
// --- toggle ON (#332): deferred tool visibility ---
it('a non-final step exposes CORE + loadTools + activatedTools', () => {
const activated = new Set<string>();
const result = asActive(prepareAgentStep(0, 'SYS', activated, true));
expect(result.activeTools).toContain('searchPages'); // core
expect(result.activeTools).toContain('searchInPage'); // #330, core
expect(result.activeTools).toContain('editPageText'); // core
expect(result.activeTools).toContain('loadTools'); // meta-tool
// No deferred tool is active before it is loaded.
expect(result.activeTools).not.toContain('createPage');
expect(result.activeTools).not.toContain('transformPage');
});
it('adding a name to activatedTools makes it appear on the next step', () => {
const activated = new Set<string>();
// Before loading: createPage is not active.
expect(
asActive(prepareAgentStep(1, 'SYS', activated, true)).activeTools,
).not.toContain('createPage');
// loadTools grows the SAME set…
activated.add('createPage');
// …so the next step sees it.
const next = asActive(prepareAgentStep(2, 'SYS', activated, true));
expect(next.activeTools).toContain('createPage');
expect(next.activeTools).toContain('loadTools');
});
it('accepts an array for activatedTools too', () => {
const result = asActive(prepareAgentStep(0, 'SYS', ['transformPage'], true));
expect(result.activeTools).toContain('transformPage');
expect(result.activeTools).toContain('loadTools');
});
it('final-step lockdown WINS even when the toggle is on', () => {
const result = asLockdown(
prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS', new Set(['createPage']), true),
);
// The lockdown shape (toolChoice none + synthesis) — not the activeTools shape.
expect(result.toolChoice).toBe('none');
expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
expect((result as unknown as { activeTools?: string[] }).activeTools).toBeUndefined();
});
});
@@ -30,7 +30,15 @@ import {
} from '@docmost/db/types/entity.types';
import { AiChatToolsService } from './tools/ai-chat-tools.service';
import { McpClientsService } from './external-mcp/mcp-clients.service';
import { EnvironmentService } from '../../integrations/environment/environment.service';
import { buildSystemPrompt } from './ai-chat.prompt';
import {
CORE_TOOL_KEYS,
CORE_TOOL_SET,
LOAD_TOOLS_NAME,
makeLoadToolsTool,
buildExternalToolCatalog,
} from './tools/tool-tiers';
import { computePageChange } from './page-change/page-change.util';
import { roleModelOverride } from './roles/role-model-config';
import {
@@ -54,24 +62,52 @@ const FINAL_STEP_INSTRUCTION =
'language. If the information is incomplete, say so explicitly: summarize ' +
'what you found, what is still missing, and give your best partial conclusion.';
// Pure, unit-testable: decide per-step overrides. Returns undefined for normal
// steps; on the final allowed step forces a text-only synthesis answer.
// Pure, unit-testable: decide per-step overrides. Two responsibilities:
// 1. Final-step lockdown (always): on the final allowed step force a text-only
// synthesis answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION). This WINS —
// it takes precedence over the deferred-tool narrowing below.
// 2. Deferred tool visibility (#332): when `deferredEnabled` and NOT the final
// step, expose only the CORE tools + loadTools + whatever loadTools has
// activated so far this turn (`activatedTools`), via `activeTools`. Deferred
// tools stay in the <tool_catalog> until the model loads them.
// When `deferredEnabled` is false the behavior is unchanged: undefined on normal
// steps (all tools active), lockdown on the final step.
//
// `system` is the in-scope system prompt; we CONCATENATE so the original
// persona/context is preserved — a bare `system` override would REPLACE the
// whole system prompt for the step.
// whole system prompt for the step. `activatedTools` is PER-TURN mutable state
// owned by the streaming loop (a closure Set grown by loadTools); it is passed
// in (not module-global, not persisted) so this stays a pure function of its
// arguments.
//
// NOTE: at AI SDK v7 the per-step `system` field is renamed to `instructions`.
// On v6 (`^6.0.134`) `system` is the correct field — adjust when bumping.
export function prepareAgentStep(
stepNumber: number,
system: string,
): { toolChoice: 'none'; system: string } | undefined {
activatedTools: ReadonlySet<string> | readonly string[] = [],
deferredEnabled = false,
):
| { toolChoice: 'none'; system: string }
| { activeTools: string[] }
| undefined {
// Final-step lockdown WINS (applies regardless of the deferred toggle).
if (stepNumber >= MAX_AGENT_STEPS - 1) {
return {
toolChoice: 'none',
system: `${system}\n\n${FINAL_STEP_INSTRUCTION}`,
};
}
// Deferred tool loading: narrow this step's visible tools to CORE + loadTools
// + the tools already activated this turn.
if (deferredEnabled) {
const activated = Array.isArray(activatedTools)
? activatedTools
: [...activatedTools];
return {
activeTools: [...CORE_TOOL_KEYS, LOAD_TOOLS_NAME, ...activated],
};
}
return undefined;
}
@@ -206,6 +242,9 @@ export class AiChatService implements OnModuleInit {
private readonly aiAgentRoleRepo: AiAgentRoleRepo,
private readonly pageRepo: PageRepo,
private readonly pageAccess: PageAccessService,
// Reads the AI_CHAT_DEFERRED_TOOLS toggle (#332). Injected last so existing
// positional constructor callers (tests) only append one stub.
private readonly environment: EnvironmentService,
) {}
/**
@@ -625,9 +664,25 @@ export class AiChatService implements OnModuleInit {
// Build the system prompt + Docmost toolset. If either throws after the
// external MCP lease was taken above, release the lease before rethrowing so
// the leased transports are not leaked (#185 review).
// Deferred tool loading toggle (#332). When ON, the model sees a compact
// <tool_catalog> and only CORE tools + loadTools are active each step; other
// tools (fat/rare in-app tools + ALL external MCP tools) load on demand. When
// OFF, every tool is active and nothing below changes.
const deferredEnabled = this.environment.isAiChatDeferredToolsEnabled();
let system: string;
let docmostTools: Awaited<ReturnType<AiChatToolsService['forUser']>>;
try {
// Assemble the deferred catalog for the system prompt: hand-written lines
// for the in-app deferred tools + a derived line for each external MCP tool
// (also deferred by default). Only built when the feature is enabled.
const toolCatalog = deferredEnabled
? [
...(await this.tools.getInAppDeferredCatalog()),
...buildExternalToolCatalog(external.tools),
]
: [];
system = buildSystemPrompt({
workspace,
adminPrompt: resolved?.systemPrompt,
@@ -644,6 +699,10 @@ export class AiChatService implements OnModuleInit {
// Detected between-turns human edit to the open page (#274): adds the
// page_changed note + unified diff so the agent doesn't overwrite it.
pageChanged,
// Deferred tool loading (#332): renders the <tool_catalog> block (only
// when enabled + non-empty) so the model can activate deferred tools.
deferredToolsEnabled: deferredEnabled,
toolCatalog,
});
// Pass the resolved chatId so the write tools can mint provenance tokens
@@ -664,7 +723,31 @@ export class AiChatService implements OnModuleInit {
throw err;
}
const tools = { ...external.tools, ...docmostTools };
// Base toolset: external MCP tools + Docmost in-app tools (Docmost wins on a
// name clash — external are namespaced, so no clash is expected).
const baseTools = { ...external.tools, ...docmostTools };
// Deferred tool loading state (#332), scoped to THIS streaming loop:
// - `activatedTools` is per-TURN mutable state — a fresh closure Set created
// per streamText call, NOT module-global and NOT persisted, so a new turn
// starts cold. loadTools.execute adds to it; prepareAgentStep reads it to
// widen `activeTools` on the NEXT step.
// - `validDeferredNames` = every tool that is NOT core (the in-app deferred
// tools + ALL external MCP tools), computed from the ACTUAL toolset so an
// external tool is loadable by its namespaced name. loadTools rejects any
// name outside this set.
const activatedTools = new Set<string>();
const validDeferredNames = new Set<string>(
Object.keys(baseTools).filter((k) => !CORE_TOOL_SET.has(k)),
);
// Add the loadTools meta-tool ONLY when the feature is enabled; when off the
// toolset and behavior are exactly as before.
const tools = deferredEnabled
? {
...baseTools,
[LOAD_TOOLS_NAME]: makeLoadToolsTool(activatedTools, validDeferredNames),
}
: baseTools;
// Accumulate the turn's streamed output so a provider error / disconnect can
// persist the PARTIAL answer the user already saw — the SDK's onError/onAbort
@@ -799,7 +882,8 @@ export class AiChatService implements OnModuleInit {
// ends with no assistant text (an empty turn). prepareAgentStep forbids
// further tool calls and appends a synthesis instruction on that step,
// concatenated onto the original `system` so the persona is preserved.
prepareStep: ({ stepNumber }) => prepareAgentStep(stepNumber, system),
prepareStep: ({ stepNumber }) =>
prepareAgentStep(stepNumber, system, activatedTools, deferredEnabled),
abortSignal: signal,
onChunk: ({ chunk }) => {
// DIAGNOSTIC (Safari stream-drop investigation) — temporary. Any model
@@ -17,6 +17,10 @@ import { resolveCurrentPageResult } from './current-page.util';
import { parseNodeArg } from './parse-node-arg';
import { modelFriendlyInput } from './model-friendly-input';
import { SandboxStore } from '../../../integrations/sandbox/sandbox.store';
import {
buildInAppDeferredCatalog,
type ToolCatalogEntry,
} from './tool-tiers';
/**
* Per-user, per-request adapter that exposes Docmost READ operations to the
@@ -123,6 +127,18 @@ export class AiChatToolsService {
return client.exportPageMarkdown(pageId);
}
/**
* Build the IN-APP deferred <tool_catalog> entries (#332): one "name — purpose"
* line per DEFERRED tool, merging the per-layer INLINE_TOOL_TIERS with the
* shared registry's own catalogLine. Loads @docmost/mcp for the shared specs
* (memoized). Core tools are always active and are NOT listed here. External
* MCP tools are catalogued separately by the caller (they are runtime-scoped).
*/
async getInAppDeferredCatalog(): Promise<ToolCatalogEntry[]> {
const { sharedToolSpecs } = await loadDocmostMcp();
return buildInAppDeferredCatalog(sharedToolSpecs);
}
async forUser(
user: User,
sessionId: string,
@@ -241,6 +241,11 @@ export interface SharedToolSpec {
mcpName: string;
inAppKey: string;
description: string;
// Deferred-tool metadata (#332). Optional in this mirror so an older/stale
// @docmost/mcp build (pre-#332) still type-checks; the in-app catalog builder
// reads them defensively. The external /mcp server ignores both fields.
tier?: 'core' | 'deferred';
catalogLine?: string;
// Loose `z` on purpose: the registry is zod-agnostic so the server can pass
// its own zod (v4) and the MCP package its own (v3) into the same builder.
buildShape?: (z: any) => Record<string, unknown>;
@@ -0,0 +1,244 @@
import {
CORE_TOOL_KEYS,
CORE_TOOL_SET,
LOAD_TOOLS_NAME,
LOAD_TOOLS_DESCRIPTION,
INLINE_TOOL_TIERS,
buildInAppDeferredCatalog,
buildExternalToolCatalog,
shortenForCatalog,
applyLoadTools,
} from './tool-tiers';
// The real shared registry, imported from source (same approach as the
// SHARED_TOOL_SPECS contract spec) so the tier metadata is checked against
// exactly what @docmost/mcp ships.
import { SHARED_TOOL_SPECS } from '../../../../../../packages/mcp/src/tool-specs';
// For the live-toolset partition test (F3): the REAL adapter, so the catalog is
// checked against the tools AiChatToolsService.forUser() actually builds — not a
// static list that could drift from it.
import { AiChatToolsService } from './ai-chat-tools.service';
import * as loader from './docmost-client.loader';
import type { DocmostClientLike } from './docmost-client.loader';
/**
* #332 deferred tool loading — tier metadata, catalog assembly, and the
* loadTools meta-tool. Pure units; no Nest graph, no @docmost/mcp build (the
* registry is imported from TS source).
*/
describe('tool tier metadata (#332)', () => {
it('core set is the documented 13 + searchInPage (14)', () => {
expect(CORE_TOOL_KEYS).toHaveLength(14);
expect(CORE_TOOL_SET.has('searchInPage')).toBe(true); // #330, promoted to core
// loadTools is a meta-tool, not a normal core key.
expect(CORE_TOOL_SET.has(LOAD_TOOLS_NAME)).toBe(false);
});
it('SHARED_TOOL_SPECS tier agrees with CORE_TOOL_SET for every shared tool', () => {
for (const [key, spec] of Object.entries(SHARED_TOOL_SPECS)) {
const isCoreByTier = spec.tier === 'core';
const isCoreByList = CORE_TOOL_SET.has(key);
expect(isCoreByTier).toBe(isCoreByList);
// Every spec carries a non-empty catalogLine (core tools too).
expect(typeof spec.catalogLine).toBe('string');
expect(spec.catalogLine.trim().length).toBeGreaterThan(0);
}
});
it('every INLINE tool tier agrees with CORE_TOOL_SET and has a catalogLine', () => {
for (const [key, meta] of Object.entries(INLINE_TOOL_TIERS)) {
expect(meta.tier === 'core').toBe(CORE_TOOL_SET.has(key));
expect(meta.catalogLine.trim().length).toBeGreaterThan(0);
}
});
});
describe('buildInAppDeferredCatalog (#332)', () => {
const catalog = buildInAppDeferredCatalog(SHARED_TOOL_SPECS as never);
const names = catalog.map((e) => e.name);
it('includes deferred tools from BOTH the inline map and the shared registry', () => {
expect(names).toContain('transformPage'); // inline deferred
expect(names).toContain('getPageJson'); // shared deferred
expect(names).toContain('patchNode'); // shared deferred
expect(names).toContain('createPage'); // inline deferred
});
it('NEVER lists a core tool', () => {
for (const core of CORE_TOOL_KEYS) {
expect(names).not.toContain(core);
}
// spot-check a couple that are core in each source.
expect(names).not.toContain('searchInPage'); // shared core
expect(names).not.toContain('searchPages'); // inline core
expect(names).not.toContain('editPageText'); // shared core
});
it('renders every entry as a "name — purpose" line', () => {
// Non-empty catalog (the length is pinned structurally by the live-toolset
// partition test below, not by a magic constant that rots on every new tool).
expect(catalog.length).toBeGreaterThan(0);
for (const entry of catalog) {
expect(entry.catalogLine).toMatch(/ — /);
}
});
});
/**
* F3 — the deferred <tool_catalog> is built from STATIC metadata (INLINE_TOOL_TIERS
* + SHARED_TOOL_SPECS), but the loadable-by-name set is derived at RUNTIME from the
* actual toolset (`Object.keys(baseTools)` in ai-chat.service.ts). Those two must
* agree or a tool becomes loadable-but-invisible (agent thinks it doesn't exist) or
* catalogued-but-phantom. INLINE_TOOL_TIERS is a plain hand-maintained Record with
* no compile-time link to the tools AiChatToolsService.forUser() builds, so nothing
* else catches that drift. This test uses forUser()'s LIVE keys as the source of
* truth (mirroring ai-chat-tools.service.spec.ts's loader mock) and asserts a
* two-way partition against buildInAppDeferredCatalog — replacing the old magic
* toHaveLength(28), so a tool added to forUser() without a catalog line (or a
* catalog line without a real tool) fails the suite instead of silently vanishing.
*/
describe('deferred catalog ↔ live forUser() toolset partition (#332, F3)', () => {
let toolKeys: string[];
const catalogNames = buildInAppDeferredCatalog(SHARED_TOOL_SPECS as never).map(
(e) => e.name,
);
beforeAll(async () => {
// Intercept the ESM loader so forUser() builds against the TS-source shared
// specs (no @docmost/mcp build) and never touches the network.
jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({
DocmostClient: function () {
return {} as DocmostClientLike;
} as unknown as loader.DocmostClientCtor,
sharedToolSpecs: SHARED_TOOL_SPECS as Record<string, loader.SharedToolSpec>,
});
const service = new AiChatToolsService(
{
generateAccessToken: jest.fn().mockResolvedValue('access-token'),
generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
} as never,
{} as never, // aiService — not exercised while merely BUILDING the tools
{} as never, // pageEmbeddingRepo
{} as never, // spaceMemberRepo
{} as never, // pagePermissionRepo
// sandboxStore: forUser() eagerly calls asSink() to wire the stash tool.
{
asSink: () => ({ put: jest.fn(), has: jest.fn(), evict: jest.fn() }),
} as never,
);
const tools = await service.forUser(
{ id: 'user-1', email: 'u@example.com', workspaceId: 'ws-1' } as never,
'session-1',
'ws-1',
'chat-1',
);
toolKeys = Object.keys(tools);
});
afterAll(() => {
jest.restoreAllMocks();
});
it('exposes a non-trivial toolset (sanity: the mock actually built tools)', () => {
expect(toolKeys.length).toBeGreaterThan(20);
});
it('every non-core live tool is present in the catalog (no capability silently hidden)', () => {
// forUser() does not itself add loadTools (ai-chat.service does), but guard
// anyway. Every remaining non-core key MUST have a catalog line.
const catalogSet = new Set(catalogNames);
const missing = toolKeys.filter(
(k) => !CORE_TOOL_SET.has(k) && k !== LOAD_TOOLS_NAME && !catalogSet.has(k),
);
expect(missing).toEqual([]);
});
it('every catalog entry corresponds to a real, non-core live tool (no phantom)', () => {
const liveSet = new Set(toolKeys);
const phantom = catalogNames.filter(
(n) => !liveSet.has(n) || CORE_TOOL_SET.has(n),
);
expect(phantom).toEqual([]);
});
});
describe('buildExternalToolCatalog + shortenForCatalog (#332)', () => {
it('derives a short "name — purpose" line from each external tool description', () => {
const catalog = buildExternalToolCatalog({
tavily_search: { description: 'Search the web for fresh results. More detail here.' },
tavily_extract: { description: '' },
});
expect(catalog).toEqual([
{ name: 'tavily_search', catalogLine: 'tavily_search — Search the web for fresh results.' },
{ name: 'tavily_extract', catalogLine: 'tavily_extract — external tool' },
]);
});
it('caps a very long description', () => {
const long = 'x'.repeat(500);
expect(shortenForCatalog(long).length).toBeLessThanOrEqual(140);
expect(shortenForCatalog(long).endsWith('…')).toBe(true);
});
});
describe('applyLoadTools (#332)', () => {
const valid = new Set(['createPage', 'transformPage', 'tavily_search']);
it('adds valid names to the activated set and returns { loaded }', () => {
const activated = new Set<string>();
const result = applyLoadTools(['createPage', 'tavily_search'], activated, valid);
expect(result).toEqual({ loaded: ['createPage', 'tavily_search'] });
expect(activated.has('createPage')).toBe(true);
expect(activated.has('tavily_search')).toBe(true);
});
it('rejects an unknown name with an error listing the valid deferred names', () => {
const activated = new Set<string>();
expect(() => applyLoadTools(['nope'], activated, valid)).toThrow(/unknown tool name/i);
try {
applyLoadTools(['nope'], activated, valid);
} catch (e) {
const msg = (e as Error).message;
// Lists every valid name (sorted).
expect(msg).toContain('createPage');
expect(msg).toContain('transformPage');
expect(msg).toContain('tavily_search');
}
// Nothing is activated on a rejected call.
expect(activated.size).toBe(0);
});
it('tolerates a non-array / empty input (loads nothing)', () => {
const activated = new Set<string>();
expect(applyLoadTools(undefined, activated, valid)).toEqual({ loaded: [] });
expect(applyLoadTools([], activated, valid)).toEqual({ loaded: [] });
expect(activated.size).toBe(0);
});
it('loadTools description is the verbatim issue text', () => {
expect(LOAD_TOOLS_DESCRIPTION).toContain('only ACTIVATES them');
expect(LOAD_TOOLS_DESCRIPTION).toContain('callable on your NEXT step');
});
});
describe('editorial "Corrector" scenario is fully served by CORE (#332)', () => {
it('read + comment + edit + search need no loadTools', () => {
// A Corrector role reads a page, searches within it, edits text, and leaves
// inline comments — every tool it needs is core, so it never has to load a
// deferred tool.
const needed = [
'getCurrentPage',
'getPage',
'searchPages',
'searchInPage',
'editPageText',
'createComment',
'listComments',
'getComment',
'resolveComment',
];
for (const t of needed) {
expect(CORE_TOOL_SET.has(t)).toBe(true);
}
});
});
@@ -0,0 +1,309 @@
import { tool, type Tool } from 'ai';
import { z } from 'zod';
import type { SharedToolSpec } from './docmost-client.loader';
/**
* Deferred tool loading for the in-app AI chat (#332).
*
* The agent otherwise sends ALL ~41 tool definitions on EVERY model call every
* step, bloating context. Instead we split the in-app tools into two tiers:
*
* - CORE (hot, always active): frequent OR tiny tools whose full schema is
* always visible, plus the `loadTools` meta-tool. Deferring a one-line tool is
* pure loss, so tiny tools stay core even if rare.
* - DEFERRED (loaded on demand): the fat/rare tools + ALL external MCP tools by
* default. The model sees only a compact <tool_catalog> (name — purpose) and
* calls `loadTools(names)` to ACTIVATE a tool's full schema for the NEXT step
* (one extra round-trip on first use).
*
* This module is the single source of truth for the IN-APP tiering:
* - CORE_TOOL_KEYS / CORE_TOOL_SET — the authoritative core list (used by
* prepareAgentStep to build per-step `activeTools`).
* - INLINE_TOOL_TIERS — tier + catalogLine for the per-layer INLINE tools (the
* ones NOT in @docmost/mcp's SHARED_TOOL_SPECS, which carry their own).
* - buildInAppDeferredCatalog / buildExternalToolCatalog — assemble the
* <tool_catalog> deferred lines.
* - applyLoadTools / makeLoadToolsTool — the loadTools meta-tool.
*
* The tier/catalogLine fields on SHARED_TOOL_SPECS are IN-APP metadata only; the
* external /mcp server ignores them and exposes every tool normally.
*/
/** A single rendered <tool_catalog> line: the tool name + its "name — purpose". */
export interface ToolCatalogEntry {
/** Exact tool name the model must pass to loadTools. */
name: string;
/** Hand-written (in-app) or derived (external) "name — purpose" line. */
catalogLine: string;
}
/**
* CORE (always-active) in-app tool keys — 13 frequent/tiny tools. `searchInPage`
* (#330) is added to core on top of the issue's original tier list: it is
* frequent for the editorial roles this feature targets. `loadTools` is active
* too but is not a normal tool key (it is added to activeTools separately).
*/
export const CORE_TOOL_KEYS = [
'searchPages',
'listPages',
'listSpaces',
'getWorkspace',
'getCurrentPage',
'getPage',
'getOutline',
'getNode',
'createComment',
'getComment',
'listComments',
'resolveComment',
'editPageText',
// #330 search_in_page — frequent for editorial sweeps; core despite predating
// the issue's tier list.
'searchInPage',
] as const;
/** O(1) membership test for the core tier. */
export const CORE_TOOL_SET: ReadonlySet<string> = new Set(CORE_TOOL_KEYS);
/** The meta-tool name (always active alongside the core tools when enabled). */
export const LOAD_TOOLS_NAME = 'loadTools';
/**
* loadTools description — VERBATIM from issue #332. Tells the model that the
* catalog names EXIST, that loadTools only ACTIVATES them (callable next step),
* and to load several at once.
*/
export const LOAD_TOOLS_DESCRIPTION =
'loadTools — Load the full definitions of deferred tools from the <tool_catalog>\n' +
'block in your instructions. Pass the EXACT tool names from the catalog; this\n' +
'call only ACTIVATES them and returns { loaded: [...] } — the tools become\n' +
'callable on your NEXT step. Load several names in one call when the task clearly\n' +
'needs them. Unknown names are rejected with the list of valid ones.';
/**
* Tier + catalogLine for the INLINE ai-chat tools — those defined per-layer in
* ai-chat-tools.service.ts and NOT present in @docmost/mcp's SHARED_TOOL_SPECS
* (which carries its own tier/catalogLine). Together with the shared registry
* this describes every in-app tool. catalogLine is present for core tools too
* (uniformity), but only DEFERRED tools are rendered into the catalog.
*/
export const INLINE_TOOL_TIERS: Record<
string,
{ tier: 'core' | 'deferred'; catalogLine: string }
> = {
// --- core inline ---
searchPages: {
tier: 'core',
catalogLine: 'searchPages — hybrid semantic + keyword search across the wiki.',
},
getCurrentPage: {
tier: 'core',
catalogLine: 'getCurrentPage — the page the user is currently viewing.',
},
getPage: {
tier: 'core',
catalogLine: 'getPage — fetch a page as Markdown by its id.',
},
listPages: {
tier: 'core',
catalogLine: "listPages — list recent pages, or a space's full page tree.",
},
listComments: {
tier: 'core',
catalogLine: 'listComments — list all comments on a page (including resolved).',
},
getComment: {
tier: 'core',
catalogLine: 'getComment — fetch a single comment by id.',
},
createComment: {
tier: 'core',
catalogLine:
'createComment — add an inline comment (optionally with a suggested edit).',
},
resolveComment: {
tier: 'core',
catalogLine: 'resolveComment — resolve or reopen a comment thread.',
},
// --- deferred inline ---
createPage: {
tier: 'deferred',
catalogLine: 'createPage — create a new page with a Markdown body in a space.',
},
updatePageContent: {
tier: 'deferred',
catalogLine:
"updatePageContent — replace a page's body (and optionally title) with new Markdown.",
},
renamePage: {
tier: 'deferred',
catalogLine: "renamePage — change a page's title only (body untouched).",
},
movePage: {
tier: 'deferred',
catalogLine: 'movePage — move a page under a new parent or to the space root.',
},
deletePage: {
tier: 'deferred',
catalogLine: 'deletePage — move a page to trash (soft delete, reversible).',
},
listSidebarPages: {
tier: 'deferred',
catalogLine:
"listSidebarPages — list a space's root pages or a page's direct children.",
},
getTable: {
tier: 'deferred',
catalogLine: 'getTable — read a table as a matrix of cell texts and cell ids.',
},
checkNewComments: {
tier: 'deferred',
catalogLine:
'checkNewComments — find comments in a space created after a timestamp.',
},
getPageHistory: {
tier: 'deferred',
catalogLine:
'getPageHistory — fetch one page-history version with its ProseMirror content.',
},
exportPageMarkdown: {
tier: 'deferred',
catalogLine:
'exportPageMarkdown — export a page to self-contained Markdown (body + comments).',
},
updatePageJson: {
tier: 'deferred',
catalogLine:
"updatePageJson — overwrite a page's body with a full ProseMirror document.",
},
tableInsertRow: {
tier: 'deferred',
catalogLine: 'tableInsertRow — insert a row of plain-text cells into a table.',
},
tableDeleteRow: {
tier: 'deferred',
catalogLine: 'tableDeleteRow — delete a table row at a 0-based index.',
},
tableUpdateCell: {
tier: 'deferred',
catalogLine: 'tableUpdateCell — set the text of a table cell at [row, col].',
},
sharePage: {
tier: 'deferred',
catalogLine: 'sharePage — make a page publicly accessible and return its URL.',
},
transformPage: {
tier: 'deferred',
catalogLine: "transformPage — run a sandboxed JS transform over a page's document.",
},
};
/**
* Build the <tool_catalog> deferred lines for the IN-APP tools by merging the
* two metadata sources: the per-layer INLINE_TOOL_TIERS and the shared registry
* (SHARED_TOOL_SPECS, loaded at runtime). Only DEFERRED tools are included; core
* tools are always active and never appear in the catalog. Pure — the caller
* passes the loaded specs so this stays unit-testable.
*/
export function buildInAppDeferredCatalog(
sharedToolSpecs: Record<string, SharedToolSpec>,
): ToolCatalogEntry[] {
const entries: ToolCatalogEntry[] = [];
// Inline deferred tools (hand-written lines).
for (const [name, meta] of Object.entries(INLINE_TOOL_TIERS)) {
if (meta.tier === 'deferred') {
entries.push({ name, catalogLine: meta.catalogLine });
}
}
// Shared deferred tools (line comes from the registry's own catalogLine).
for (const [name, spec] of Object.entries(sharedToolSpecs)) {
if (spec.tier === 'deferred' && spec.catalogLine) {
entries.push({ name, catalogLine: spec.catalogLine });
}
}
return entries;
}
/**
* Cap an external tool's (untrusted) description into a short catalog purpose.
* External MCP tools have no hand-written catalogLine, so we derive one from the
* first sentence of the description, hard-capped. Whitespace is collapsed.
*/
export function shortenForCatalog(description: string, max = 140): string {
const flat = description.replace(/\s+/g, ' ').trim();
if (!flat) return 'external tool';
// Prefer the first sentence if it is reasonably short.
const firstSentence = flat.split(/(?<=[.!?])\s/)[0];
const base =
firstSentence.length > 0 && firstSentence.length <= max
? firstSentence
: flat;
return base.length > max ? `${base.slice(0, max - 1).trimEnd()}` : base;
}
/**
* Build catalog lines for the EXTERNAL MCP tools (all deferred by default,
* #332). Their names are the namespaced tool keys; the purpose is derived from
* each tool's own description (no hand-written line exists). Pure.
*/
export function buildExternalToolCatalog(
externalTools: Record<string, { description?: string } | undefined>,
): ToolCatalogEntry[] {
return Object.entries(externalTools).map(([name, t]) => ({
name,
catalogLine: `${name}${shortenForCatalog(t?.description ?? '')}`,
}));
}
/**
* Pure core of the loadTools meta-tool. Validates the requested names against
* the per-turn set of valid deferred names, ADDS the valid ones to the caller's
* mutable `activatedTools` set (so they become callable next step), and returns
* `{ loaded }`. An unknown name throws a clear error listing the valid deferred
* names — surfaced to the model as a tool error so it can retry.
*/
export function applyLoadTools(
names: unknown,
activatedTools: Set<string>,
validDeferredNames: ReadonlySet<string>,
): { loaded: string[] } {
const requested = Array.isArray(names)
? names.filter((n): n is string => typeof n === 'string')
: [];
const unknown = requested.filter((n) => !validDeferredNames.has(n));
if (unknown.length > 0) {
const valid = [...validDeferredNames].sort().join(', ');
throw new Error(
`loadTools: unknown tool name(s): ${unknown.join(', ')}. ` +
`Valid deferred tools are: ${valid || '(none)'}.`,
);
}
for (const n of requested) activatedTools.add(n);
return { loaded: requested };
}
/**
* Build the loadTools AI-SDK tool bound to THIS turn's mutable state: the
* `activatedTools` set (grown by execute, read by prepareAgentStep next step)
* and the `validDeferredNames` set (every non-core tool in this turn's toolset,
* incl. external MCP). Created per streamText call — never module-global.
*/
export function makeLoadToolsTool(
activatedTools: Set<string>,
validDeferredNames: ReadonlySet<string>,
): Tool {
return tool({
description: LOAD_TOOLS_DESCRIPTION,
inputSchema: z.object({
names: z
.array(z.string())
.describe(
'EXACT deferred tool names from the <tool_catalog> to activate for ' +
'your next step.',
),
}),
execute: async ({ names }) =>
applyLoadTools(names, activatedTools, validDeferredNames),
});
}
@@ -261,6 +261,21 @@ export class EnvironmentService {
return disable === 'true';
}
/**
* Deferred tool loading for the in-app AI chat (#332). When enabled, the agent
* sees a compact <tool_catalog> and only CORE tools + the loadTools meta-tool
* are active each step; deferred tools (the fat/rare ones + all external MCP
* tools) load on demand. Defaults to ENABLED — the issue treats deferred
* loading as the new behavior; set AI_CHAT_DEFERRED_TOOLS=false to restore the
* old "all tools always active" behavior.
*/
isAiChatDeferredToolsEnabled(): boolean {
const enabled = this.configService
.get<string>('AI_CHAT_DEFERRED_TOOLS', 'true')
.toLowerCase();
return enabled === 'true';
}
getPostHogHost(): string {
return this.configService.get<string>('POSTHOG_HOST');
}
@@ -1,5 +1,7 @@
import * as http from 'node:http';
import { Kysely } from 'kysely';
import { tool } from 'ai';
import { z } from 'zod';
import { MockLanguageModelV3, convertArrayToReadableStream } from 'ai/test';
import { AiChatRepo } from '@docmost/db/repos/ai-chat/ai-chat.repo';
import { AiChatMessageRepo } from '@docmost/db/repos/ai-chat/ai-chat-message.repo';
@@ -146,6 +148,9 @@ describe('AiChatService.stream [integration]', () => {
{} as any, // aiAgentRoleRepo (role is pre-resolved + passed in)
{} as any, // pageRepo (only used when body.openPage is set)
{} as any, // pageAccess (idem)
// environment (#332): keep deferred tool loading OFF for this lifecycle
// harness so the toolset/behavior is exactly as before.
{ isAiChatDeferredToolsEnabled: () => false } as any,
);
}
@@ -315,4 +320,174 @@ describe('AiChatService.stream [integration]', () => {
true,
);
});
/**
* #332 deferred tool loading, the ON path. The riskiest property is that the
* per-turn `activatedTools` Set is created FRESH inside each stream() call, so a
* tool a previous turn activated via loadTools is NOT still active when the next
* turn starts — the new turn begins "cold" (CORE + loadTools only). The unit
* tests only exercise pure prepareAgentStep with hand-fed Sets; this pins the
* real wiring end-to-end (loadTools.execute -> activatedTools -> prepareStep ->
* per-step activeTools) against the real streamText loop, and proves there is no
* cross-turn leak. We drive a MockLanguageModelV3 whose step 1 calls
* loadTools(['createPage']) and assert, via the model's recorded per-step
* CallOptions.tools (the AI SDK filters the provider tool list by activeTools),
* that the deferred tool becomes active on the SAME turn's next step but NOT on a
* fresh turn's first step.
*/
describe('deferred tool loading ON — per-turn activation, no leak (#332)', () => {
// A stub deferred (non-core) tool the agent can activate. Its execute is never
// called — the model only needs to SEE it become active — but it must be a
// valid AI-SDK tool so the SDK includes it in a step's tool list once active.
const createPageStub = tool({
description: 'create a new page',
inputSchema: z.object({ title: z.string() }),
execute: async () => ({ id: 'p-stub' }),
});
// A CORE tool in the toolset, so a cold step shows CORE tools ARE active while
// the deferred createPage is not. `searchPages` is in CORE_TOOL_SET.
const searchPagesStub = tool({
description: 'search the wiki',
inputSchema: z.object({ query: z.string() }),
execute: async () => [],
});
// Same lifecycle harness as buildService() above, but with deferred loading ON
// and a toolset that exposes exactly one deferred tool (createPage) so it is
// catalogued + loadable-by-name. Kept separate so the OFF scenarios are
// untouched.
function buildDeferredService(): AiChatService {
return new AiChatService(
{ getChatModel: async () => null } as any,
aiChatRepo,
msgRepo,
{} as any,
{ resolve: async () => null } as any,
{
forUser: async () => ({
searchPages: searchPagesStub,
createPage: createPageStub,
}),
getInAppDeferredCatalog: async () => [
{ name: 'createPage', catalogLine: 'createPage — create a new page.' },
],
} as any,
mcpClients as any,
{} as any,
{} as any,
{} as any,
// #332: deferred tool loading ON — the property under test.
{ isAiChatDeferredToolsEnabled: () => true } as any,
);
}
// Drive ONE stream() turn against `model` and wait for the assistant row to
// settle (mirrors runStream, but builds the deferred-ON service).
async function runDeferredTurn(
model: MockLanguageModelV3,
chatId: string,
body: any,
): Promise<void> {
closeCalls = 0;
const service = buildDeferredService();
const { res, cleanup } = await makeRealResponse();
try {
await service.stream({
user: { id: userId, workspaceId } as any,
workspace: { id: workspaceId, name: 'WS' } as any,
sessionId: 'sess-1',
body,
res: { raw: res } as any,
signal: new AbortController().signal,
model: model as any,
role: null,
} as any);
await waitFor(async () => {
const rows = await msgRepo.findAllByChat(chatId, workspaceId);
return rows.some(
(r) =>
r.role === 'assistant' &&
['completed', 'error', 'aborted'].includes(r.status as string),
);
});
await waitFor(() => closeCalls > 0, { timeoutMs: 5_000 });
} finally {
await cleanup();
}
}
// Tool names the provider actually received for a recorded step (activeTools
// filters this list, so it reflects what was active that step).
const toolNames = (call: any): string[] =>
((call?.tools ?? []) as any[]).map((t) => t?.name).filter(Boolean);
// A model that, on step 1, calls loadTools(['createPage']); on step 2, answers.
function loadThenAnswerModel(): MockLanguageModelV3 {
let step = 0;
return new MockLanguageModelV3({
doStream: async () => {
const n = step++;
if (n === 0) {
return {
stream: convertArrayToReadableStream([
{ type: 'stream-start', warnings: [] },
{
type: 'tool-call',
toolCallId: 'lt1',
toolName: 'loadTools',
input: JSON.stringify({ names: ['createPage'] }),
},
{
type: 'finish',
finishReason: 'tool-calls',
usage: { inputTokens: 5, outputTokens: 3, totalTokens: 8 },
},
] as any),
};
}
return { stream: successStream() };
},
} as any);
}
it('activates a deferred tool for the SAME turn, and a NEW turn starts cold (no leak)', async () => {
const chatId = (await createChat(db, { workspaceId, creatorId: userId })).id;
// --- Turn 1: loadTools(createPage) on step 1, then answer on step 2. ---
const model1 = loadThenAnswerModel();
await runDeferredTurn(model1, chatId, {
chatId,
messages: [userUiMessage('Make me a page')],
});
// The turn ran at least two steps (the load round-trip + the answer).
expect(model1.doStreamCalls.length).toBeGreaterThanOrEqual(2);
const step1Tools = toolNames(model1.doStreamCalls[0]);
const step2Tools = toolNames(model1.doStreamCalls[1]);
// Step 1 starts cold: CORE tools + the loadTools meta-tool are active, but
// the deferred createPage is NOT yet.
expect(step1Tools).toContain('loadTools');
expect(step1Tools).toContain('searchPages'); // a CORE tool, always active
expect(step1Tools).not.toContain('createPage');
// Step 2 of the SAME turn sees the just-activated deferred tool.
expect(step2Tools).toContain('createPage');
// --- Turn 2 on the SAME chat: must start cold again. ---
const model2 = new MockLanguageModelV3({
doStream: async () => ({ stream: successStream() }),
} as any);
await runDeferredTurn(model2, chatId, {
chatId,
messages: [userUiMessage('And another thing')],
});
const nextTurnFirstStep = toolNames(model2.doStreamCalls[0]);
expect(nextTurnFirstStep).toContain('loadTools');
// The activated set is per-turn: the prior turn's createPage did NOT leak,
// so the fresh turn's first step sees it deferred again.
expect(nextTurnFirstStep).not.toContain('createPage');
});
});
});
@@ -0,0 +1,134 @@
import { describe, it, expect, vi, afterEach } from 'vitest';
import { getSchema } from '@tiptap/core';
import { Document } from '@tiptap/extension-document';
import { Paragraph } from '@tiptap/extension-paragraph';
import { Text } from '@tiptap/extension-text';
import { EditorState } from '@tiptap/pm/state';
import { Node as PMNode } from '@tiptap/pm/model';
import { FootnoteReference } from './footnote-reference';
import { FootnotesList } from './footnotes-list';
import { FootnoteDefinition } from './footnote-definition';
import {
footnoteNumberingPlugin,
footnoteNumberingPluginKey,
getFootnoteNumber,
} from './footnote-numbering';
import {
FOOTNOTE_REFERENCE_NAME,
FOOTNOTES_LIST_NAME,
FOOTNOTE_DEFINITION_NAME,
} from './footnote-util';
const extensions = [
Document,
Paragraph,
Text,
FootnoteReference,
FootnotesList,
FootnoteDefinition,
];
const schema = getSchema(extensions);
function makeState(docJson: any): EditorState {
return EditorState.create({
doc: PMNode.fromJSON(schema, docJson),
plugins: [footnoteNumberingPlugin()],
});
}
const withTwoFootnotes = {
type: 'doc',
content: [
{
type: 'paragraph',
content: [
{ type: 'text', text: 'a' },
{ type: FOOTNOTE_REFERENCE_NAME, attrs: { id: 'x' } },
{ type: 'text', text: 'b' },
{ type: FOOTNOTE_REFERENCE_NAME, attrs: { id: 'y' } },
],
},
{
type: FOOTNOTES_LIST_NAME,
content: [
{
type: FOOTNOTE_DEFINITION_NAME,
attrs: { id: 'x' },
content: [{ type: 'paragraph' }],
},
{
type: FOOTNOTE_DEFINITION_NAME,
attrs: { id: 'y' },
content: [{ type: 'paragraph' }],
},
],
},
],
};
describe('footnote numbering plugin — short-circuit (#343 PART 5)', () => {
afterEach(() => vi.restoreAllMocks());
it('does ZERO document traversals on a docChanged transaction when the doc has no footnotes', () => {
const state = makeState({
type: 'doc',
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'hi' }] }],
});
// Only count traversals caused by the transaction, not the initial build.
const descendantsSpy = vi.spyOn(PMNode.prototype, 'descendants');
const before = footnoteNumberingPluginKey.getState(state);
// A real content edit (docChanged) that introduces no footnote node.
const next = state.apply(state.tr.insertText('!', 3));
const after = footnoteNumberingPluginKey.getState(next);
// The plugin never walked the document...
expect(descendantsSpy).not.toHaveBeenCalled();
// ...and reused the exact same (empty) state object — proof it short-circuited.
expect(after).toBe(before);
expect(after?.hasFootnotes).toBe(false);
});
it('rebuilds (numbering appears) the first time a footnote is inserted into a footnote-free doc', () => {
const state = makeState({
type: 'doc',
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'hi' }] }],
});
expect(footnoteNumberingPluginKey.getState(state)?.hasFootnotes).toBe(false);
const ref = schema.nodes[FOOTNOTE_REFERENCE_NAME].create({ id: 'x' });
const next = state.apply(state.tr.insert(3, ref));
const after = footnoteNumberingPluginKey.getState(next);
expect(after?.hasFootnotes).toBe(true);
expect(getFootnoteNumber(next, 'x')).toBe(1);
});
});
describe('footnote numbering plugin — numbering unchanged with footnotes (#343 PART 5)', () => {
it('numbers references in document order via the single merged walk', () => {
const state = makeState(withTwoFootnotes);
expect(getFootnoteNumber(state, 'x')).toBe(1);
expect(getFootnoteNumber(state, 'y')).toBe(2);
});
it('produces a decoration for every reference and matching definition', () => {
const state = makeState(withTwoFootnotes);
const decos = footnoteNumberingPluginKey.getState(state)?.decorations;
// 2 references + 2 definitions = 4 number decorations.
expect(decos?.find().length).toBe(4);
});
it('keeps numbering current after an edit while footnotes exist', () => {
const state = makeState(withTwoFootnotes);
// Insert a NEW reference (id "z") before the others: it must become #1 and
// shift x -> #2, y -> #3 (deterministic document-order numbering).
const ref = schema.nodes[FOOTNOTE_REFERENCE_NAME].create({ id: 'z' });
const next = state.apply(state.tr.insert(1, ref));
expect(getFootnoteNumber(next, 'z')).toBe(1);
expect(getFootnoteNumber(next, 'x')).toBe(2);
expect(getFootnoteNumber(next, 'y')).toBe(3);
});
});
@@ -1,11 +1,9 @@
import { EditorState, Plugin, PluginKey } from '@tiptap/pm/state';
import { EditorState, Plugin, PluginKey, Transaction } from '@tiptap/pm/state';
import { Decoration, DecorationSet } from '@tiptap/pm/view';
import { Node as ProseMirrorNode } from '@tiptap/pm/model';
import { Node as ProseMirrorNode, Slice } from '@tiptap/pm/model';
import {
FOOTNOTE_DEFINITION_NAME,
FOOTNOTE_REFERENCE_NAME,
computeFootnoteNumbers,
computeFootnoteRefCounts,
} from './footnote-util';
export const footnoteNumberingPluginKey = new PluginKey<FootnoteNumberingState>(
@@ -27,8 +25,22 @@ interface FootnoteNumberingState {
refCounts: Map<string, number>;
/** Decorations rendering those numbers (refs + definitions). */
decorations: DecorationSet;
/** Whether the document contains ANY footnote reference/definition node.
* Cached so `apply` can skip the whole-doc walk on every keystroke in the
* common case (documents with no footnotes), recomputing only once a
* transaction actually inserts a footnote node (#343, PART 5). */
hasFootnotes: boolean;
}
/** Reusable empty state for footnote-free documents — avoids reallocating an
* empty map/decoration set on every keystroke while there are no footnotes. */
const EMPTY_STATE: FootnoteNumberingState = {
numbers: new Map(),
refCounts: new Map(),
decorations: DecorationSet.empty,
hasFootnotes: false,
};
/**
* Build the decoration set for footnote numbers. Pure function of the document:
* walk references in document order, assign 1-based numbers, then attach a
@@ -41,50 +53,101 @@ export function buildFootnoteDecorations(doc: ProseMirrorNode): DecorationSet {
return buildFootnoteNumberingState(doc).decorations;
}
function numberDecoration(pos: number, nodeSize: number, num: number): Decoration {
return Decoration.node(pos, pos + nodeSize, {
'data-footnote-number': String(num),
style: `--footnote-number: "${num}";`,
});
}
/**
* Compute both the number map AND the decorations for `doc` in a single walk.
* The plugin caches the result so NodeViews can read numbers without
* recomputing.
* Compute the number map, reference counts AND the decorations for `doc` in a
* SINGLE document walk (previously three separate O(n) traversals per
* docChanged — computeFootnoteNumbers + computeFootnoteRefCounts + a decoration
* pass, #343 PART 5). The plugin caches the result so NodeViews can read numbers
* without recomputing.
*
* References are numbered and decorated as they are encountered (document
* order). Definition positions are collected during the same walk and decorated
* afterwards from the completed number map — so a definition that appears before
* its reference in document order still resolves to the correct number, and the
* output is identical to the previous three-pass implementation. (Decoration
* insertion order does not matter: DecorationSet.create indexes by position.)
*/
function buildFootnoteNumberingState(
doc: ProseMirrorNode,
): FootnoteNumberingState {
const numbers = computeFootnoteNumbers(doc);
const refCounts = computeFootnoteRefCounts(doc);
const numbers = new Map<string, number>();
const refCounts = new Map<string, number>();
const decorations: Decoration[] = [];
const definitions: { id: string; pos: number; nodeSize: number }[] = [];
let n = 0;
let hasFootnotes = false;
doc.descendants((node, pos) => {
if (node.type.name === FOOTNOTE_REFERENCE_NAME) {
const num = numbers.get(node.attrs.id);
if (num != null) {
decorations.push(
Decoration.node(pos, pos + node.nodeSize, {
'data-footnote-number': String(num),
style: `--footnote-number: "${num}";`,
}),
);
}
}
if (node.type.name === FOOTNOTE_DEFINITION_NAME) {
const num = numbers.get(node.attrs.id);
if (num != null) {
decorations.push(
Decoration.node(pos, pos + node.nodeSize, {
'data-footnote-number': String(num),
style: `--footnote-number: "${num}";`,
}),
);
const typeName = node.type.name;
if (typeName === FOOTNOTE_REFERENCE_NAME) {
hasFootnotes = true;
const id = node.attrs.id;
if (id) {
if (!numbers.has(id)) numbers.set(id, ++n);
refCounts.set(id, (refCounts.get(id) ?? 0) + 1);
decorations.push(numberDecoration(pos, node.nodeSize, numbers.get(id)!));
}
} else if (typeName === FOOTNOTE_DEFINITION_NAME) {
hasFootnotes = true;
const id = node.attrs.id;
if (id != null) definitions.push({ id, pos, nodeSize: node.nodeSize });
}
});
if (!hasFootnotes) return EMPTY_STATE;
for (const def of definitions) {
const num = numbers.get(def.id);
if (num != null) {
decorations.push(numberDecoration(def.pos, def.nodeSize, num));
}
}
return {
numbers,
refCounts,
decorations: DecorationSet.create(doc, decorations),
hasFootnotes: true,
};
}
/**
* Cheap check: does any of a transaction's inserted content contain a footnote
* reference/definition node? Footnote nodes can only ENTER the document through
* replace steps (ReplaceStep / ReplaceAroundStep both expose a `.slice`), so
* scanning only the inserted slices — O(change size), not O(doc) — is sufficient
* to detect a newly-added footnote. Mark/attr steps never introduce nodes.
* Lets `apply` keep skipping the whole-doc walk until a footnote first appears.
*/
function transactionInsertsFootnote(tr: Transaction): boolean {
for (const step of tr.steps) {
const slice = (step as unknown as { slice?: Slice }).slice;
if (!slice || slice.content.size === 0) continue;
let found = false;
slice.content.descendants((node) => {
if (found) return false;
const typeName = node.type.name;
if (
typeName === FOOTNOTE_REFERENCE_NAME ||
typeName === FOOTNOTE_DEFINITION_NAME
) {
found = true;
return false;
}
return true;
});
if (found) return true;
}
return false;
}
/**
* Read the cached footnote number for `id` from the numbering plugin's state.
* This is the source NodeViews should use instead of calling
@@ -126,6 +189,13 @@ export function footnoteNumberingPlugin(): Plugin {
// the number map NodeViews read stays current on every edit while
// non-doc transactions (selection, etc.) reuse the cache for free.
if (!tr.docChanged) return old;
// Short-circuit the whole-doc walk while the document has no footnotes:
// if there were none and this transaction did not INSERT one, there is
// still nothing to number, so reuse the empty state (#343, PART 5). Once
// a footnote exists we always rebuild (covers renumbering/deletion).
if (!old.hasFootnotes && !transactionInsertsFootnote(tr)) {
return old;
}
return buildFootnoteNumberingState(tr.doc);
},
},
+65
View File
@@ -31,6 +31,22 @@ export interface SharedToolSpec {
inAppKey: string;
/** Single canonical model-facing description used by both layers. */
description: string;
/**
* Deferred-tool tier for the IN-APP agent (#332). 'core' tools are always
* active; 'deferred' tools are hidden behind the <tool_catalog> and loaded on
* demand via the loadTools meta-tool. This is an IN-APP concern only: the
* standalone /mcp server ignores this field and registers every tool normally
* (registerShared in index.ts reads mcpName/description/buildShape only).
*/
tier: 'core' | 'deferred';
/**
* Hand-written one-liner "name — purpose" shown in the in-app agent's
* <tool_catalog> for a DEFERRED tool (#332). Deliberately NOT derived from the
* description's first sentence — a concise, accurate purpose line. Present on
* every spec (core tools too) for uniformity; only deferred ones are rendered.
* Inert for the external /mcp server.
*/
catalogLine: string;
/**
* Builds the tool's input schema as a plain object of zod fields (a
* ZodRawShape). Called with the consumer's own zod namespace. Omitted for
@@ -47,6 +63,8 @@ export const SHARED_TOOL_SPECS = {
mcpName: 'get_workspace',
inAppKey: 'getWorkspace',
description: 'Fetch metadata about the current workspace (name, settings).',
tier: 'core',
catalogLine: 'getWorkspace — fetch current workspace metadata (name, settings).',
},
listSpaces: {
@@ -55,6 +73,8 @@ export const SHARED_TOOL_SPECS = {
description:
'List the spaces the current user can access. Returns the array of ' +
'spaces (id, name, slug, ...).',
tier: 'core',
catalogLine: 'listSpaces — list the spaces the user can access (id, name, slug).',
},
listShares: {
@@ -62,6 +82,8 @@ export const SHARED_TOOL_SPECS = {
inAppKey: 'listShares',
description:
'List all public shares in the workspace with page titles and public URLs.',
tier: 'deferred',
catalogLine: 'listShares — list all public shares in the workspace with their URLs.',
},
// --- single-pageId read tools ---
@@ -74,6 +96,9 @@ export const SHARED_TOOL_SPECS = {
'includes block ids, callouts, tables, link/image attributes) plus the ' +
'slugId used in URLs. Use the block ids it returns to make precise ' +
'structural edits or surgical text edits without resending the page.',
tier: 'deferred',
catalogLine:
"getPageJson — get a page's raw ProseMirror JSON (lossless, with block ids).",
buildShape: (z) => ({
pageId: z.string().min(1),
}),
@@ -88,6 +113,9 @@ export const SHARED_TOOL_SPECS = {
'count) WITHOUT the full document body. Use it to locate sections/tables ' +
'and grab block ids cheaply before fetching, patching or inserting ' +
'individual blocks.',
tier: 'core',
catalogLine:
"getOutline — compact outline of a page's top-level blocks with their ids.",
buildShape: (z) => ({
pageId: z.string().min(1),
}),
@@ -104,6 +132,9 @@ export const SHARED_TOOL_SPECS = {
'outline or page-JSON view (works for headings/paragraphs/callouts/images), OR ' +
'`#<index>` to fetch a top-level block by its outline index — use the ' +
'`#<index>` form for tables/rows/cells, which carry no id.',
tier: 'core',
catalogLine:
"getNode — fetch one block's ProseMirror subtree by block id or #index.",
buildShape: (z) => ({
pageId: z.string().min(1),
nodeId: z.string().min(1),
@@ -137,6 +168,9 @@ export const SHARED_TOOL_SPECS = {
'caseSensitive:true to match case. Ideal for systematic ' +
'editorial sweeps (unquoted "ё", straight quotes, "т.е.", stray units). An ' +
'invalid regex or an empty query returns a clear error to fix.',
tier: 'core',
catalogLine:
'searchInPage — find every occurrence of a string/regex inside one page, with locations.',
buildShape: (z) => ({
pageId: z.string().min(1).describe('ID of the page to search'),
query: z
@@ -172,6 +206,8 @@ export const SHARED_TOOL_SPECS = {
description:
'Remove a single block by its attrs.id (from the page outline or ' +
'page-JSON view) WITHOUT resending the whole document.',
tier: 'deferred',
catalogLine: 'deleteNode — remove a single content block by its block id.',
buildShape: (z) => ({
pageId: z.string().min(1),
nodeId: z.string().min(1),
@@ -203,6 +239,9 @@ export const SHARED_TOOL_SPECS = {
'JSON object or a JSON string (both accepted). Cheaper and safer than ' +
'replacing the whole document for one-block structural edits. Reversible: ' +
'the previous version is kept in page history.',
tier: 'deferred',
catalogLine:
'patchNode — replace one block with a new ProseMirror node, keeping its id.',
buildShape: (z) => ({
pageId: z.string().min(1).describe('ID of the page containing the block'),
nodeId: z
@@ -245,6 +284,9 @@ export const SHARED_TOOL_SPECS = {
'[{"type":"text","text":"Title"}]}. Bold is a mark: ' +
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' +
'JSON object or a JSON string (both accepted). Reversible via page history.',
tier: 'deferred',
catalogLine:
'insertNode — insert a block before/after an anchor, or append at the end.',
buildShape: (z) => ({
pageId: z.string().min(1),
node: z
@@ -278,6 +320,8 @@ export const SHARED_TOOL_SPECS = {
mcpName: 'unshare_page',
inAppKey: 'unsharePage',
description: 'Remove the public share of a page (revokes the public URL).',
tier: 'deferred',
catalogLine: "unsharePage — revoke a page's public share (removes the public URL).",
buildShape: (z) => ({
pageId: z.string().min(1).describe('ID of the page to unshare'),
}),
@@ -295,6 +339,9 @@ export const SHARED_TOOL_SPECS = {
"`from`/`to` each accept a historyId, or null/'current' for the page's " +
'current content (defaults: from=current, to=current — pass a historyId ' +
'from the page-history list to compare against the live page).',
tier: 'deferred',
catalogLine:
'diffPageVersions — diff two page versions and return the change set + summary.',
buildShape: (z) => ({
pageId: z.string().min(1),
from: z
@@ -315,6 +362,9 @@ export const SHARED_TOOL_SPECS = {
"List a page's saved versions (Docmost auto-snapshots on every save), " +
'newest first, cursor-paginated. Returns { items, nextCursor }; each ' +
"item's id is the historyId to pass to the page diff or restore tools.",
tier: 'deferred',
catalogLine:
"listPageHistory — list a page's saved versions (newest first, paginated).",
buildShape: (z) => ({
pageId: z.string().min(1),
cursor: z
@@ -332,6 +382,9 @@ export const SHARED_TOOL_SPECS = {
'as the page\'s current content (Docmost has no restore endpoint, so ' +
'this creates a NEW history snapshot — the restore is itself revertible). ' +
'Get the historyId from the page-history list.',
tier: 'deferred',
catalogLine:
'restorePageVersion — restore a page to a saved history version (revertible).',
buildShape: (z) => ({
historyId: z.string().min(1),
}),
@@ -349,6 +402,9 @@ export const SHARED_TOOL_SPECS = {
'thread records are NOT created/updated/deleted on the server by this ' +
'tool — only the page body + inline comment marks are written; manage ' +
'comment threads via the comment tools/UI.',
tier: 'deferred',
catalogLine:
"importPageMarkdown — replace a page's content from exported Docmost Markdown.",
buildShape: (z) => ({
pageId: z.string().min(1),
markdown: z.string().min(1),
@@ -365,6 +421,9 @@ export const SHARED_TOOL_SPECS = {
'entirely server-side — the document is NOT sent through the model. The ' +
'target keeps its own title and slug; only its body is replaced. Ideal ' +
"for 'make page A's content equal to B' or 'replace A with B but keep A's URL'.",
tier: 'deferred',
catalogLine:
"copyPageContent — replace one page's body with a copy of another page's body.",
buildShape: (z) => ({
sourcePageId: z.string().min(1).describe('Page to copy content FROM'),
targetPageId: z
@@ -402,6 +461,9 @@ export const SHARED_TOOL_SPECS = {
'page JSON and use a structural node patch/update to set its marks. ' +
'Examples: edits:[{find:"teh",replace:"the"}]; edits:[{find:"Hello ' +
'world",replace:"Hello there"}] (crosses a bold boundary).',
tier: 'core',
catalogLine:
"editPageText — surgical find/replace of plain text in a page, preserving ids/marks.",
buildShape: (z) => ({
pageId: z.string().describe('ID of the page to edit'),
edits: z
@@ -440,6 +502,9 @@ export const SHARED_TOOL_SPECS = {
'server instance that created it: in a multi-replica deployment without ' +
'sticky sessions a blob stored on one instance is not retrievable via the ' +
'sandbox URL on another (it 404s like an expired one).',
tier: 'deferred',
catalogLine:
'stashPage — serialize a whole page to a short anonymous URL without loading its body.',
buildShape: (z) => ({
pageId: z.string().min(1),
}),
@@ -635,13 +635,17 @@ const Attachment = Node.create({
},
name: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("data-attachment-name"),
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
parseHTML: (el: HTMLElement) =>
el.getAttribute("data-attachment-name") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.name ? { "data-attachment-name": attrs.name } : {},
},
mime: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("data-attachment-mime"),
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
parseHTML: (el: HTMLElement) =>
el.getAttribute("data-attachment-mime") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.mime ? { "data-attachment-mime": attrs.mime } : {},
},
@@ -689,7 +693,10 @@ const Video = Node.create({
},
alt: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("aria-label"),
// Empty-string-vs-absent idempotency: coerce "" back to the default so a
// stray empty `aria-label` never materializes `alt: ""` on a video stored
// with no alt (same GS-EDIT-REVERT class as the image `alt` fix).
parseHTML: (el: HTMLElement) => el.getAttribute("aria-label") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.alt ? { "aria-label": attrs.alt } : {},
},
@@ -864,13 +871,15 @@ const diagramAttributes = () => ({
},
title: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("data-title"),
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
parseHTML: (el: HTMLElement) => el.getAttribute("data-title") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.title ? { "data-title": attrs.title } : {},
},
alt: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("data-alt"),
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
parseHTML: (el: HTMLElement) => el.getAttribute("data-alt") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.alt ? { "data-alt": attrs.alt } : {},
},
@@ -1106,7 +1115,8 @@ const Pdf = Node.create({
},
name: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("data-name"),
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
parseHTML: (el: HTMLElement) => el.getAttribute("data-name") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.name ? { "data-name": attrs.name } : {},
},
@@ -1491,6 +1501,29 @@ export const docmostExtensions = [
...parent.height,
parseHTML: (el: HTMLElement) => el.getAttribute("height"),
},
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class). `marked`
// renders `![](src)` as `<img alt="">`, so the stock Image `alt`
// parseHTML (`getAttribute("alt")`) materializes `alt: ""` on an image
// that was stored with NO alt (attr absent). That is a false diff against
// the editor-stored form (a no-alt image has alt ABSENT, not ""), so a
// git-sync / ai-chat touch of a page with a plain image produced phantom
// churn. Coerce an empty string back to the attr's default (null) so the
// import is idempotent. A real alt survives verbatim (`|| undefined` keeps
// the truthy value; the default fills the empty case). `title` is coerced
// the same way for the whole class, even though `marked` does not
// currently emit `title=""` — defence in depth against any path that does.
// NOTE: this DIVERGES from editor-ext's literal image `alt` parseHTML
// (`getAttribute("alt")`, which returns "" verbatim), but CONVERGES on
// editor-ext's real STORED shape: an editor image inserted without alt
// renders with no `alt` attribute and re-parses as absent, never "".
alt: {
...parent.alt,
parseHTML: (el: HTMLElement) => el.getAttribute("alt") || null,
},
title: {
...parent.title,
parseHTML: (el: HTMLElement) => el.getAttribute("title") || null,
},
};
},
}).configure({ inline: false }),
@@ -0,0 +1,443 @@
/**
* Reusable round-trip-STABILITY matrix helper (fixtures-first).
*
* A single stored node authored WITHOUT a given string attribute (attr
* absent / undefined) must not gain a phantom EMPTY-STRING value after a
* markdown round-trip — the "empty-string-vs-absent" churn class. This helper,
* given a node spec, drives a matrix of attribute combinations through the REAL
* converter (`convertProseMirrorToMarkdown` -> `markdownToProseMirror`) and
* asserts byte-stability on two contours:
*
* 1. RAW round-trip: for the node under test, every attribute the round-trip
* materializes must equal what the INPUT authored — an authored attr keeps
* its value, an ABSENT attr may only reappear at its SCHEMA DEFAULT. If an
* absent attr comes back as a NON-default value (e.g. `alt: ""` where the
* default is `null`), that is an instability and is reported precisely as
* `type.attr: absent -> "<got>"`. This is the contour git-sync / stored
* JSON diffs on, so masking it only in `canonicalize` would leave the noise.
*
* 2. CANONICAL round-trip: `canonicalizeContent(original)` must deep-equal
* `canonicalizeContent(roundtrip)` (a second, semantic contour).
*
* The ONLY normalization the helper treats as allowed (not an instability) is
* the DOCUMENTED numeric width/height/size/aspectRatio -> string coercion the
* converter performs on purpose (a stored numeric `640` re-parses via
* `getAttribute` as the string `"640"`). It is encoded here as an explicit
* per-spec `numericStringAttrs` set applied to BOTH contours, NOT a silent skip.
*
* The helper is node-type agnostic: image and the whole media family share the
* `align !== "center"` predicate + `<!--name {…}-->` comment machinery, so one
* matrix guards the shared class.
*/
import { getSchema } from "@tiptap/core";
import {
convertProseMirrorToMarkdown,
markdownToProseMirror,
canonicalizeContent,
docmostExtensions,
} from "../src/lib/index.js";
import { firstDivergence } from "./roundtrip-helpers.js";
/** One attribute's two probe values. */
export interface AttrMatrixEntry {
/** Attribute name on the node. */
attr: string;
/**
* The "default" pick. `undefined` means the attribute is OMITTED entirely
* (the absent case — the one that can materialize an empty string on import).
* A concrete value is authored verbatim.
*/
default: unknown;
/** A representative NON-default value to exercise (must survive verbatim). */
nonDefault: unknown;
/**
* Marks the attr as a member of the EMPTY-STRING class the fix targets: a
* string attr whose schema default is `null`/absent and whose parseHTML
* coerces `"" -> default` (image/drawio `alt`+`title`, video `alt` via
* aria-label, pdf/attachment `name`, attachment `mime`). Set true to also
* drive the THIRD-STATE convergence case (see runConvergenceCase) for this
* attr. Attrs whose default is NOT null (e.g. embed `provider`, default "")
* or that are not `""`-coerced (control attrs) are left unset.
*/
emptyStringClass?: boolean;
}
/** A node type + the attribute matrix to sweep for it. */
export interface NodeStabilitySpec {
/** Node type (e.g. "image", "video"). */
type: string;
/** Attributes always present on the node (e.g. `{ src: "/i.png" }`). */
baseAttrs?: Record<string, unknown>;
/** Attributes to sweep at default and non-default. */
attrMatrix: AttrMatrixEntry[];
/**
* Attributes whose numeric -> string coercion on round-trip is DOCUMENTED and
* intentional; compared modulo `String(x)` on both sides. Defaults to the
* converter's known sizing set.
*/
numericStringAttrs?: string[];
}
/** A single unstable finding, legible enough to tie a gate-lock to. */
export interface Instability {
type: string;
attr: string;
/** What the input authored: the literal value, or the ABSENT sentinel. */
authored: unknown | typeof ABSENT;
/** What the round-trip produced. */
got: unknown;
/** What a stable round-trip should have produced (authored value or default). */
expected: unknown;
}
/** One matrix cell's result. */
export interface ComboResult {
label: string;
authored: Record<string, unknown>;
/** RAW-contour instabilities on the node under test. */
raw: Instability[];
/** CANONICAL-contour divergence (path + values) or null when equal. */
canonical: { path: string; a: unknown; b: unknown } | null;
/** True when the node type failed to round-trip at all (structural loss). */
missing: boolean;
md: string;
}
/** Whole-matrix report for one node spec. */
export interface MatrixReport {
type: string;
combos: ComboResult[];
}
/** Sentinel marking an attribute the input did NOT author. */
export const ABSENT = Symbol("ABSENT");
const DEFAULT_NUMERIC_STRING_ATTRS = [
"width",
"height",
"size",
"aspectRatio",
];
// The ProseMirror schema the converter targets — its attribute `default`s are
// the authoritative "what an absent attr should re-materialize as" oracle.
const schema = getSchema(docmostExtensions);
/** Read the schema default for every attribute of a node type. */
function schemaDefaults(type: string): Record<string, unknown> {
const specAttrs = (schema.nodes[type]?.spec?.attrs ?? {}) as Record<
string,
{ default: unknown }
>;
const out: Record<string, unknown> = {};
for (const [k, v] of Object.entries(specAttrs)) out[k] = v.default;
return out;
}
/** Find the first node of a given type anywhere in a PM doc tree. */
function findFirst(node: any, type: string): any {
if (node && node.type === type) return node;
for (const child of node?.content ?? []) {
const hit = findFirst(child, type);
if (hit) return hit;
}
return null;
}
/** Coerce a scalar for the documented numeric->string comparison. */
const numStr = (x: unknown): unknown => (x == null ? x : String(x));
/**
* Enumerate the cartesian product of the matrix: every attribute independently
* at its default (index 0) or non-default (index 1) pick. The all-default
* corner is included (the baseline). Small by construction (2^N over a handful
* of at-risk string attrs).
*/
function enumerateCombos(matrix: AttrMatrixEntry[]): number[][] {
let combos: number[][] = [[]];
for (let i = 0; i < matrix.length; i++) {
const next: number[][] = [];
for (const c of combos) {
next.push([...c, 0]);
next.push([...c, 1]);
}
combos = next;
}
return combos;
}
/** Build the authored attrs for one combo pick vector. */
function authoredAttrs(
spec: NodeStabilitySpec,
picks: number[],
): Record<string, unknown> {
const attrs: Record<string, unknown> = { ...(spec.baseAttrs ?? {}) };
spec.attrMatrix.forEach((entry, i) => {
if (picks[i] === 1) {
attrs[entry.attr] = entry.nonDefault;
} else if (entry.default !== undefined) {
attrs[entry.attr] = entry.default;
}
// default === undefined -> OMIT the attr entirely (the absent case).
});
return attrs;
}
/** Human-readable label for a combo (which attrs are at non-default). */
function comboLabel(spec: NodeStabilitySpec, picks: number[]): string {
const on = spec.attrMatrix
.filter((_, i) => picks[i] === 1)
.map((e) => e.attr);
return on.length === 0 ? "<all-default>" : on.join("+");
}
/**
* Run the full stability matrix for one node spec and return a structured
* report (does NOT throw — the caller asserts, so a failure can print the whole
* report). Every combo runs the real export->import pipeline once.
*/
export async function runStabilityMatrix(
spec: NodeStabilitySpec,
): Promise<MatrixReport> {
const numericStringAttrs = new Set(
spec.numericStringAttrs ?? DEFAULT_NUMERIC_STRING_ATTRS,
);
const defaults = schemaDefaults(spec.type);
const combos: ComboResult[] = [];
for (const picks of enumerateCombos(spec.attrMatrix)) {
const authored = authoredAttrs(spec, picks);
const doc = { type: "doc", content: [{ type: spec.type, attrs: authored }] };
const md = convertProseMirrorToMarkdown(doc);
const rt = await markdownToProseMirror(md);
const node = findFirst(rt, spec.type);
const result: ComboResult = {
label: comboLabel(spec, picks),
authored,
raw: [],
canonical: null,
missing: node == null,
md,
};
if (node != null) {
// RAW contour: every materialized attr must equal the authored value, or
// (for an absent attr) the schema default — modulo the documented numeric
// string coercion.
const rtAttrs = (node.attrs ?? {}) as Record<string, unknown>;
for (const key of Object.keys(rtAttrs)) {
const authoredHas = Object.prototype.hasOwnProperty.call(authored, key);
const expected = authoredHas ? authored[key] : defaults[key];
let got = rtAttrs[key];
let exp = expected;
if (numericStringAttrs.has(key)) {
got = numStr(got);
exp = numStr(exp);
}
if (firstDivergence(got, exp) !== null) {
result.raw.push({
type: spec.type,
attr: key,
authored: authoredHas ? authored[key] : ABSENT,
got: rtAttrs[key],
expected,
});
}
}
// CANONICAL contour: canonical forms deep-equal, modulo the same numeric
// string coercion (applied to both trees so a documented coercion is not
// counted as a divergence).
const ca = normalizeNumeric(canonicalizeContent(doc), numericStringAttrs);
const cb = normalizeNumeric(canonicalizeContent(rt), numericStringAttrs);
result.canonical = firstDivergence(ca, cb);
}
combos.push(result);
}
return { type: spec.type, combos };
}
/**
* Deep-copy a canonical tree, coercing the documented numeric->string attrs to
* their string form so an intentional `640 -> "640"` coercion is not reported
* as a canonical divergence. Only touches the listed attribute keys.
*/
function normalizeNumeric(node: any, attrs: Set<string>): any {
if (Array.isArray(node)) return node.map((n) => normalizeNumeric(n, attrs));
if (node === null || typeof node !== "object") return node;
const out: Record<string, unknown> = {};
for (const key of Object.keys(node)) {
if (key === "attrs" && node.attrs && typeof node.attrs === "object") {
const a: Record<string, unknown> = {};
for (const [k, v] of Object.entries(node.attrs)) {
a[k] = attrs.has(k) ? numStr(v) : v;
}
out.attrs = a;
} else {
out[key] = normalizeNumeric(node[key], attrs);
}
}
return out;
}
/** Flatten a report to just its unstable combos (for a terse assertion). */
export function unstableCombos(report: MatrixReport): ComboResult[] {
return report.combos.filter(
(c) => c.missing || c.raw.length > 0 || c.canonical !== null,
);
}
// ---------------------------------------------------------------------------
// THIRD STATE: an EXPLICITLY-STORED empty string on a string attr.
//
// The matrix above sweeps TWO states per string attr: absent/default and a
// non-default value — and asserts FIRST-pass byte-stability for both. There is
// a third, degenerate state the matrix does NOT cover: the attr stored as a
// LITERAL `""`. This is DISTINCT from "the node never had the attr": a user
// types an alt in the editor, then deletes it, and Tiptap's
// `updateAttributes({ alt: "" })` persists a literal `alt: ""` in the stored
// JSON. There is no absent-vs-"" distinction in the DOM once serialized, so the
// fix's `getAttribute("alt") || null` coercion canonicalizes BOTH to the
// default (`null`).
//
// Consequence — and this is CORRECT, not a bug: a doc carrying an explicit `""`
// converges to the default on the FIRST round-trip (a ONE-TIME diff: `"" ->
// null`), then is byte-stable from the SECOND round-trip on (idempotent). So
// this state must be pinned with a DIFFERENT contract than the matrix's:
// - do NOT assert first-pass byte-stability (the first pass legitimately
// changes `""` -> default), and
// - DO assert the first pass converges to the default AND the second pass is
// idempotent (rt2 deep-equals rt1).
//
// A future sync/QA pass diffing stored pages will see this one-time `"" -> null`
// normalization exactly once per affected node; it is the converter canon, not
// corruption, and must not be flagged as data loss.
// ---------------------------------------------------------------------------
/** Result of the third-state ("explicit empty string") convergence probe. */
export interface ConvergenceResult {
type: string;
attr: string;
/** The schema default the attr must converge to on pass 1 (null / absent). */
expectedDefault: unknown;
/** rt1's materialized value for the attr — must equal `expectedDefault`. */
firstPassValue: unknown;
/** True when the node round-tripped AND rt1 converged the attr to default. */
convergedToDefault: boolean;
/** rt1-vs-rt2 divergence; MUST be null (idempotent from pass 2 on). */
secondPassDivergence: { path: string; a: unknown; b: unknown } | null;
/** True when the node type failed to round-trip at all (structural loss). */
missing: boolean;
}
/** Round-trip a full PM doc through the real converter once. */
async function roundtripDoc(doc: any): Promise<any> {
return markdownToProseMirror(convertProseMirrorToMarkdown(doc));
}
/**
* Third-state convergence probe for one string attr of the empty-string class.
*
* (a) builds a doc with the attr EXPLICITLY set to `""` (baseAttrs + `""`),
* (b) rt1 = roundtrip(doc); asserts rt1's attr equals the schema default — the
* documented ONE-TIME `"" -> default` normalization (NOT byte-stable vs the
* `""` input, so first-pass stability is deliberately NOT asserted here),
* (c) rt2 = roundtrip(rt1); asserts rt2 deep-equals rt1 — idempotent from the
* second round-trip on.
*
* Returns a structured result (does NOT throw) so the caller can assert and
* print. Reusable across the whole node family: drive it for every attr flagged
* `emptyStringClass` on every spec (see convergenceCasesFor / the test driver).
*/
export async function runConvergenceCase(
spec: NodeStabilitySpec,
attr: string,
): Promise<ConvergenceResult> {
const expectedDefault = schemaDefaults(spec.type)[attr];
// (a) The degenerate third state: attr persisted as a LITERAL "".
const authored = { ...(spec.baseAttrs ?? {}), [attr]: "" };
const doc = { type: "doc", content: [{ type: spec.type, attrs: authored }] };
// (b) First round-trip: "" must normalize to the default (a one-time diff).
const rt1 = await roundtripDoc(doc);
const node1 = findFirst(rt1, spec.type);
const firstPassValue = node1?.attrs?.[attr];
const convergedToDefault =
node1 != null && firstDivergence(firstPassValue, expectedDefault) === null;
// (c) Second round-trip: must be byte-stable (rt2 deep-equals rt1). We compare
// the WHOLE docs — both are converter OUTPUTS already in the same materialized
// form (numeric attrs are strings on both sides), so no numeric normalization
// is needed here, unlike the raw/canonical contours above.
const rt2 = node1 != null ? await roundtripDoc(rt1) : rt1;
const secondPassDivergence =
node1 != null ? firstDivergence(rt1, rt2) : null;
return {
type: spec.type,
attr,
expectedDefault,
firstPassValue,
convergedToDefault,
secondPassDivergence,
missing: node1 == null,
};
}
/** The attrs of a spec flagged as members of the empty-string class. */
export function convergenceCasesFor(spec: NodeStabilitySpec): string[] {
return spec.attrMatrix
.filter((e) => e.emptyStringClass)
.map((e) => e.attr);
}
/** True when a convergence result honours the "converges once, then stable" contract. */
export function convergenceOk(r: ConvergenceResult): boolean {
return !r.missing && r.convergedToDefault && r.secondPassDivergence === null;
}
/** Render a convergence result as a legible one-liner for a failed assertion. */
export function formatConvergence(r: ConvergenceResult): string {
if (r.missing) return `${r.type}.${r.attr}: DID-NOT-ROUND-TRIP`;
const parts: string[] = [];
if (!r.convergedToDefault) {
parts.push(
`pass1 did NOT converge: got ${JSON.stringify(r.firstPassValue)} (expected default ${JSON.stringify(r.expectedDefault)})`,
);
}
if (r.secondPassDivergence) {
parts.push(
`pass2 NOT idempotent @ ${r.secondPassDivergence.path}: ${JSON.stringify(r.secondPassDivergence.a)} vs ${JSON.stringify(r.secondPassDivergence.b)}`,
);
}
const status = parts.length === 0 ? "converges-once-then-stable" : parts.join("; ");
return `${r.type}.${r.attr}: ${status}`;
}
/** Render a report as a legible multi-line string for a failed assertion. */
export function formatReport(report: MatrixReport): string {
const lines: string[] = [`node "${report.type}":`];
for (const c of report.combos) {
const flags: string[] = [];
if (c.missing) flags.push("DID-NOT-ROUND-TRIP");
for (const i of c.raw) {
const authored =
i.authored === ABSENT ? "absent" : JSON.stringify(i.authored);
flags.push(
`RAW ${i.type}.${i.attr}: ${authored} -> ${JSON.stringify(i.got)} (expected ${JSON.stringify(i.expected)})`,
);
}
if (c.canonical) {
flags.push(
`CANON @ ${c.canonical.path}: ${JSON.stringify(c.canonical.a)} vs ${JSON.stringify(c.canonical.b)}`,
);
}
const status = flags.length === 0 ? "stable" : flags.join("; ");
lines.push(` [${c.label}] ${status}`);
}
return lines.join("\n");
}
@@ -0,0 +1,164 @@
import { describe, expect, it } from "vitest";
import {
runStabilityMatrix,
unstableCombos,
formatReport,
runConvergenceCase,
convergenceCasesFor,
convergenceOk,
formatConvergence,
type NodeStabilitySpec,
} from "./roundtrip-stability.helper.js";
// ---------------------------------------------------------------------------
// Round-trip STABILITY matrix for image + the media family.
//
// Guards the "empty-string-vs-absent" churn class (GS-EDIT-REVERT family): a
// stored node authored WITHOUT a string attr (alt/title/caption/aria-label/...)
// must not gain a phantom `attr: ""` after `markdownToProseMirror(convert…)`.
// Each spec sweeps the at-risk string attrs at DEFAULT (absent) and at a real
// NON-default value; the helper asserts both the RAW round-trip (attrs equal the
// input's, modulo the documented numeric width/height/size/aspectRatio -> string
// coercion) and the CANONICAL round-trip (canonical forms deep-equal).
//
// The image + media family share the `align !== "center"` predicate and the
// `<!--name {…}-->` comment machinery, so one matrix guards the shared class.
// align is NOT part of this class (it round-trips correctly) and is not swept.
// ---------------------------------------------------------------------------
const SPECS: NodeStabilitySpec[] = [
{
// Image carries the most at-risk string attrs. `alt` is the one marked
// materializes as `<img alt="">` on `![](src)` import (the real bug); title
// and caption are covered as the same class. attachmentId is a string attr
// that must stay absent when unset (control).
type: "image",
baseAttrs: { src: "/i.png" },
attrMatrix: [
{ attr: "alt", default: undefined, nonDefault: "a real alt text", emptyStringClass: true },
{ attr: "title", default: undefined, nonDefault: "a real title", emptyStringClass: true },
{ attr: "caption", default: undefined, nonDefault: "a real caption" },
{ attr: "attachmentId", default: undefined, nonDefault: "att-42" },
],
},
{
// Video's `alt` rides the `aria-label` attribute (media aria-label at risk).
type: "video",
baseAttrs: { src: "/v.mp4" },
attrMatrix: [
{ attr: "alt", default: undefined, nonDefault: "a clip", emptyStringClass: true },
{ attr: "attachmentId", default: undefined, nonDefault: "att-1" },
],
},
{
// Audio carries no alt/title; attachmentId is its only optional string attr.
type: "audio",
baseAttrs: { src: "/a.mp3" },
attrMatrix: [
{ attr: "attachmentId", default: undefined, nonDefault: "att-2" },
],
},
{
// pdf: link-form media. `name` (filename) is its at-risk string attr.
type: "pdf",
baseAttrs: { src: "/d.pdf" },
attrMatrix: [
{ attr: "name", default: undefined, nonDefault: "report.pdf", emptyStringClass: true },
{ attr: "attachmentId", default: undefined, nonDefault: "att-3" },
],
},
{
// attachment: link-form media (file card). `name` + `mime` string attrs.
type: "attachment",
baseAttrs: { url: "/f.zip" },
attrMatrix: [
{ attr: "name", default: undefined, nonDefault: "bundle.zip", emptyStringClass: true },
{ attr: "mime", default: undefined, nonDefault: "application/zip", emptyStringClass: true },
{ attr: "attachmentId", default: undefined, nonDefault: "att-4" },
],
},
{
// embed: link-form media. `provider` is its at-risk string attr (schema
// default ""). embed's numeric width/height defaults (800/600) are a SEPARATE,
// documented limitation OUTSIDE the empty-string class: they are not in
// canonicalize's KNOWN_DEFAULTS, so an ABSENT width/height re-imports as the
// 800/600 default and diverges canonically (see the note in canonicalize.ts).
// That is canonicalize-owned and out of scope here, so we author the
// dimensions at their defaults (as real editor embeds carry them) to keep this
// guard focused on the empty-string/provider class.
// provider's schema default is "" (NOT null), so a re-imported "" is the
// correct value, not a phantom — it is outside the null-default empty-string
// class. We author it at its "" default (the default pick) so the sweep still
// asserts a non-default provider ("youtube") round-trips, without tripping the
// canonicalize KNOWN_DEFAULTS gap for embed's non-null defaults.
type: "embed",
baseAttrs: { src: "https://example.com/x", width: 800, height: 600 },
attrMatrix: [
{ attr: "provider", default: "", nonDefault: "youtube" },
],
},
{
// drawio: image-form diagram. `title` + `alt` string attrs (data-title/-alt).
type: "drawio",
baseAttrs: { src: "blob:drawio" },
attrMatrix: [
{ attr: "title", default: undefined, nonDefault: "flow chart", emptyStringClass: true },
{ attr: "alt", default: undefined, nonDefault: "an alt", emptyStringClass: true },
{ attr: "attachmentId", default: undefined, nonDefault: "att-5" },
],
},
{
// excalidraw: image-form diagram, same shared diagramAttributes set.
type: "excalidraw",
baseAttrs: { src: "blob:excalidraw" },
attrMatrix: [
{ attr: "title", default: undefined, nonDefault: "sketch", emptyStringClass: true },
{ attr: "alt", default: undefined, nonDefault: "an alt", emptyStringClass: true },
{ attr: "attachmentId", default: undefined, nonDefault: "att-6" },
],
},
];
describe("round-trip stability matrix (image + media family)", () => {
for (const spec of SPECS) {
it(`${spec.type}: no attr materializes an empty-string / phantom value`, async () => {
const report = await runStabilityMatrix(spec);
const unstable = unstableCombos(report);
// On failure, print the WHOLE matrix so which (attr, value) combos are
// unstable is legible.
expect(unstable, `\n${formatReport(report)}\n`).toEqual([]);
});
}
});
// ---------------------------------------------------------------------------
// THIRD STATE: an attr EXPLICITLY stored as a literal "" (GS-EDIT-REVERT: a user
// typed alt/title/name/... then deleted it, so Tiptap persisted `attr: ""` — a
// value DISTINCT from "attr was never set"). Unlike the absent case above, this
// state is NOT first-pass byte-stable: the fix's `"" -> default` coercion is a
// deliberate ONE-TIME normalization on the FIRST sync round-trip, stable
// thereafter. We therefore assert a DIFFERENT contract — "converges to default
// on pass 1, then idempotent from pass 2 on" — for every empty-string-class attr
// across the whole node family (image/video/pdf/attachment/drawio/excalidraw).
//
// IMPORTANT for a future sync/QA pass: the pass-1 `"" -> null` diff is the
// converter canon, not corruption. It appears at most once per affected node and
// must NOT be flagged as "the converter is losing/corrupting page data".
// ---------------------------------------------------------------------------
describe("round-trip third state: explicit empty string converges once, then idempotent", () => {
for (const spec of SPECS) {
for (const attr of convergenceCasesFor(spec)) {
it(`${spec.type}.${attr}: "" normalizes to default on pass 1, byte-stable from pass 2`, async () => {
const r = await runConvergenceCase(spec, attr);
// Pass 1 must converge "" -> the schema default (the one-time diff) and
// pass 2 (roundtrip of pass-1 output) must be byte-stable. formatConvergence
// prints exactly which half failed.
expect(convergenceOk(r), `\n${formatConvergence(r)}\n`).toBe(true);
// Spell the contract out explicitly so the intent is legible in the test:
expect(r.convergedToDefault, `\n${formatConvergence(r)}\n`).toBe(true);
expect(r.firstPassValue).toEqual(r.expectedDefault);
expect(r.secondPassDivergence, `\n${formatConvergence(r)}\n`).toBeNull();
});
}
}
});