From 0314416bfab6189e070fed3e54a4804e01768091 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 26 Jun 2026 17:25:33 +0300 Subject: [PATCH] Address PR #210 review: changelog, navigation guard, hook tests (#199) - CHANGELOG: add an [Unreleased]/Added bullet documenting the "generate title from content" byline button (reads live editor content, generates via the workspace AI provider, applies through /pages/update, gated by settings.ai.generative, throttled per user). - use-generate-page-title: guard the visible title write against page navigation during generation. The mutation awaits the model for 1-3s; its closure captures the editors from the starting render, but the global page/title atoms re-point on navigation. We now keep a live ref to the current editors and skip setContent unless the live page editor still belongs to the page the title was generated for (editor.storage.pageId === pageId, mirroring TitleEditor's activePageId guard). The DB write stays correct (keyed by the captured pageId) and the websocket broadcast is unchanged, so only the wrong-page field write is suppressed. - Add a vitest suite for the hook: empty content, empty model response, happy path, the navigation guard, and 403/503/429/other onError mapping. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 6 + .../hooks/use-generate-page-title.test.tsx | 232 ++++++++++++++++++ .../editor/hooks/use-generate-page-title.ts | 34 ++- 3 files changed, 270 insertions(+), 2 deletions(-) create mode 100644 apps/client/src/features/editor/hooks/use-generate-page-title.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 77fe9718..061cf9af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,12 @@ per-workspace rolling-day token budget. - **Footnote multi-backlinks.** A footnote referenced more than once now shows a back-link per reference (↩ a b c …), each scrolling to its own occurrence, like Pandoc/Wikipedia; a single-reference footnote keeps the plain ↩. (#168) +- **Generate a page title from its content.** A "sparkles" button in the page + byline reads the live editor content (including unsaved edits), generates a + title via the workspace AI provider (`POST /ai-chat/generate-page-title`), and + applies it through the existing `/pages/update` route — reflecting it in the + title field and broadcasting to other clients. Gated by the `settings.ai.generative` + flag and throttled per user. (#199) ### Changed diff --git a/apps/client/src/features/editor/hooks/use-generate-page-title.test.tsx b/apps/client/src/features/editor/hooks/use-generate-page-title.test.tsx new file mode 100644 index 00000000..17800312 --- /dev/null +++ b/apps/client/src/features/editor/hooks/use-generate-page-title.test.tsx @@ -0,0 +1,232 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { Provider, createStore } from "jotai"; +import type { Editor } from "@tiptap/core"; +import { + pageEditorAtom, + titleEditorAtom, +} from "@/features/editor/atoms/editor-atoms.ts"; + +// --- Mocks for the hook's collaborators --------------------------------------- + +const generatePageTitleMock = vi.fn(); +vi.mock("@/features/ai-chat/services/ai-chat-service.ts", () => ({ + generatePageTitle: (content: string) => generatePageTitleMock(content), +})); + +const updateTitleMock = vi.fn(); +const updatePageDataMock = vi.fn(); +vi.mock("@/features/page/queries/page-query.ts", () => ({ + useUpdateTitlePageMutation: () => ({ mutateAsync: updateTitleMock }), + updatePageData: (page: unknown) => updatePageDataMock(page), +})); + +const emitMock = vi.fn(); +vi.mock("@/features/websocket/use-query-emit.ts", () => ({ + useQueryEmit: () => emitMock, +})); + +const localEmitMock = vi.fn(); +vi.mock("@/lib/local-emitter.ts", () => ({ + default: { emit: (...args: unknown[]) => localEmitMock(...args) }, +})); + +// htmlToMarkdown just echoes the editor HTML so each test controls the markdown +// purely via the fake page editor's getHTML(). +vi.mock("@docmost/editor-ext", () => ({ + htmlToMarkdown: (html: string) => html, +})); + +const notificationsShowMock = vi.fn(); +vi.mock("@mantine/notifications", () => ({ + notifications: { show: (opts: unknown) => notificationsShowMock(opts) }, +})); + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +// Import after mocks are registered. +import { useGeneratePageTitle } from "./use-generate-page-title.ts"; + +// --- Test helpers ------------------------------------------------------------- + +function makePageEditor(pageId: string, html = "

content

"): Editor { + return { + isDestroyed: false, + getHTML: () => html, + storage: { pageId }, + } as unknown as Editor; +} + +function makeTitleEditor(): Editor & { + commands: { setContent: ReturnType }; +} { + return { + isDestroyed: false, + isFocused: false, + commands: { setContent: vi.fn() }, + } as unknown as Editor & { + commands: { setContent: ReturnType }; + }; +} + +function setup(pageId: string, store = createStore()) { + const queryClient = new QueryClient({ + defaultOptions: { mutations: { retry: false } }, + }); + const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + ); + const { result } = renderHook(() => useGeneratePageTitle(pageId), { + wrapper, + }); + return { result, store }; +} + +const PAGE_A = { + id: "pageA", + title: "Generated Title", + spaceId: "space1", + slugId: "slugA", + parentPageId: null, + icon: null, +} as any; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("useGeneratePageTitle", () => { + it("shows a notice and bails when the editor content is empty", async () => { + const store = createStore(); + store.set(pageEditorAtom as never, makePageEditor("pageA", " ")); + store.set(titleEditorAtom as never, makeTitleEditor()); + const { result } = setup("pageA", store); + + await act(async () => { + await result.current.mutateAsync(); + }); + + expect(notificationsShowMock).toHaveBeenCalledWith( + expect.objectContaining({ message: "The note is empty", color: "yellow" }), + ); + expect(generatePageTitleMock).not.toHaveBeenCalled(); + expect(updateTitleMock).not.toHaveBeenCalled(); + }); + + it("leaves the title untouched when the model returns nothing usable", async () => { + const store = createStore(); + store.set(pageEditorAtom as never, makePageEditor("pageA")); + store.set(titleEditorAtom as never, makeTitleEditor()); + generatePageTitleMock.mockResolvedValue(" "); + const { result } = setup("pageA", store); + + await act(async () => { + await result.current.mutateAsync(); + }); + + expect(updateTitleMock).not.toHaveBeenCalled(); + expect(notificationsShowMock).toHaveBeenCalledWith( + expect.objectContaining({ + message: "Could not generate a title", + color: "yellow", + }), + ); + }); + + it("happy path: applies the title, refreshes cache, writes the field, broadcasts", async () => { + const store = createStore(); + const titleEditor = makeTitleEditor(); + store.set(pageEditorAtom as never, makePageEditor("pageA")); + store.set(titleEditorAtom as never, titleEditor); + generatePageTitleMock.mockResolvedValue("Generated Title"); + updateTitleMock.mockResolvedValue(PAGE_A); + const { result } = setup("pageA", store); + + await act(async () => { + await result.current.mutateAsync(); + }); + + expect(updateTitleMock).toHaveBeenCalledWith({ + pageId: "pageA", + title: "Generated Title", + }); + expect(updatePageDataMock).toHaveBeenCalledWith(PAGE_A); + expect(titleEditor.commands.setContent).toHaveBeenCalledWith( + "Generated Title", + ); + expect(localEmitMock).toHaveBeenCalled(); + expect(emitMock).toHaveBeenCalled(); + expect(notificationsShowMock).toHaveBeenCalledWith( + expect.objectContaining({ message: "Title generated" }), + ); + }); + + it("does NOT write the visible title field when the user navigated away during generation", async () => { + const store = createStore(); + const titleEditor = makeTitleEditor(); // persistent across navigation + store.set(pageEditorAtom as never, makePageEditor("pageA")); + store.set(titleEditorAtom as never, titleEditor); + + // Control when generation resolves so we can navigate mid-flight. + let resolveTitle!: (t: string) => void; + generatePageTitleMock.mockReturnValue( + new Promise((res) => { + resolveTitle = res; + }), + ); + updateTitleMock.mockResolvedValue(PAGE_A); + const { result } = setup("pageA", store); + + let pending!: Promise; + act(() => { + pending = result.current.mutateAsync(); + }); + + // User navigates to page B: the live page editor now belongs to pageB. + act(() => { + store.set(pageEditorAtom as never, makePageEditor("pageB")); + }); + + await act(async () => { + resolveTitle("Generated Title"); + await pending; + }); + + // DB write is still correct (keyed by the captured pageId)... + expect(updateTitleMock).toHaveBeenCalledWith({ + pageId: "pageA", + title: "Generated Title", + }); + // ...but we must NOT stamp page A's title into page B's visible field. + expect(titleEditor.commands.setContent).not.toHaveBeenCalled(); + // The change is still broadcast to other clients. + expect(emitMock).toHaveBeenCalled(); + }); + + it.each([ + [403, "AI title generation is disabled"], + [503, "AI is not configured"], + [429, "Too many requests, please try again later"], + [500, "Failed to generate title"], + ])("maps HTTP %s onError to a friendly message", async (status, message) => { + const store = createStore(); + store.set(pageEditorAtom as never, makePageEditor("pageA")); + store.set(titleEditorAtom as never, makeTitleEditor()); + generatePageTitleMock.mockRejectedValue({ response: { status } }); + const { result } = setup("pageA", store); + + await act(async () => { + await expect(result.current.mutateAsync()).rejects.toBeTruthy(); + }); + + expect(notificationsShowMock).toHaveBeenCalledWith( + expect.objectContaining({ message, color: "red" }), + ); + }); +}); diff --git a/apps/client/src/features/editor/hooks/use-generate-page-title.ts b/apps/client/src/features/editor/hooks/use-generate-page-title.ts index e2ae88a3..e8d9e0e2 100644 --- a/apps/client/src/features/editor/hooks/use-generate-page-title.ts +++ b/apps/client/src/features/editor/hooks/use-generate-page-title.ts @@ -1,3 +1,4 @@ +import { useRef } from "react"; import { useMutation } from "@tanstack/react-query"; import { useAtomValue } from "jotai"; import { notifications } from "@mantine/notifications"; @@ -36,6 +37,14 @@ export function useGeneratePageTitle(pageId: string) { const { mutateAsync: updateTitle } = useUpdateTitlePageMutation(); const emit = useQueryEmit(); + // The page/title editors come from GLOBAL atoms that re-point when the user + // navigates to another page. The mutation below awaits the model for 1-3s, and + // its closure captures the editors from the render that started it. Keep a live + // reference so the post-generation write targets whatever page is on screen + // *now*, not the page the generation was started from. + const editorsRef = useRef({ pageEditor, titleEditor }); + editorsRef.current = { pageEditor, titleEditor }; + return useMutation({ mutationFn: async () => { if (!pageEditor || pageEditor.isDestroyed) return; @@ -64,8 +73,29 @@ export function useGeneratePageTitle(pageId: string) { // Reflect the new title in the field immediately. The button lives in the // byline, so the title editor is not focused — setContent is safe and stays // undoable through its History extension (Ctrl/Cmd+Z reverts the change). - if (titleEditor && !titleEditor.isDestroyed && !titleEditor.isFocused) { - titleEditor.commands.setContent(page.title); + // + // Guard against navigation during generation: if the user switched pages + // while the model ran, the (persistent) title editor now shows ANOTHER + // page, so writing here would drop page A's title into page B's visible + // field. page-editor.tsx stamps the live page editor with its pageId + // (`editor.storage.pageId`), mirroring TitleEditor's `activePageId !== + // pageId` guard — bail the visible write unless that live editor still + // belongs to the page this title was generated for. The DB write above is + // already correct (keyed by the captured `pageId`), and the broadcast below + // still propagates page A's change to other clients. + const livePageEditor = editorsRef.current.pageEditor; + const liveTitleEditor = editorsRef.current.titleEditor; + // `storage.pageId` is stamped untyped in page-editor.tsx's onCreate. + const livePageId = (livePageEditor?.storage as { pageId?: string }) + ?.pageId; + const stillOnPage = livePageId === pageId; + if ( + stillOnPage && + liveTitleEditor && + !liveTitleEditor.isDestroyed && + !liveTitleEditor.isFocused + ) { + liveTitleEditor.commands.setContent(page.title); } // Broadcast to other clients, mirroring TitleEditor.saveTitle's event shape.