From 7775d9ba0814c8c3469882ce3ca58c61f86c7e90 Mon Sep 17 00:00:00 2001 From: otomatty Date: Sun, 26 Apr 2026 21:21:20 +0900 Subject: [PATCH] fix: address PR #757 review comments Made-with: Cursor --- AGENTS.md | 2 +- server/api/src/lib/extractPlainTextFromYXml.ts | 3 ++- .../src/extractPlainTextFromYXml.test.ts | 2 +- server/hocuspocus/src/extractPlainTextFromYXml.ts | 3 ++- .../TiptapEditor/useWikiLinkStatusSync.test.tsx | 8 ++++---- .../editor/extensions/TagExtension.test.ts | 15 ++++++++++----- src/components/editor/extensions/TagExtension.ts | 7 ++++++- .../editor/extensions/WikiLinkExtension.test.ts | 15 ++++++++++----- .../editor/extensions/WikiLinkExtension.ts | 7 ++++++- .../extensions/slashSuggestionPlugin.test.ts | 3 +++ src/hooks/useMermaidGenerator.test.ts | 3 +++ src/hooks/useWorkflowDraft.test.ts | 1 + src/hooks/useWorkflowRunSession.test.ts | 5 +++++ src/lib/aiServiceServer.test.ts | 2 ++ src/stores/aiChatStore.test.ts | 3 +++ 15 files changed, 59 insertions(+), 20 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7cb25c3b..53825d46 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,7 +58,7 @@ bun run test:run # Vitest 単体テスト ## DB スキーマ変更（必読） / Database schema changes (must read) - **TS スキーマと SQL マイグレーションは常に対で更新する**。`server/api/src/schema/**/*.ts` を編集したら、必ず `server/api/drizzle/NNNN_*.sql` を新規追加し、`server/api/drizzle/meta/_journal.json` にエントリを追記する。 - _Always pair TS schema edits with a SQL migration: add a new `server/api/drizzle/NNNN_\*.sql`and append an entry to`server/api/drizzle/meta/_journal.json`. Skipping this caused PR #728 → production 500s on `/api/onboarding/status`and`/api/pages`._ + Always pair TS schema edits with a SQL migration: add a new `server/api/drizzle/NNNN_*.sql` and append an entry to `server/api/drizzle/meta/_journal.json`. Skipping this caused production 500s in PR #728 on `/api/onboarding/status` and `/api/pages`. - **正本のマイグレーション置き場は `server/api/drizzle/` のみ**。CI (`deploy-{dev,prod}.yml`) は `bunx drizzle-kit migrate` だけを実行するため、ここ以外に SQL を置いても本番には適用されない。 _Source of truth is `server/api/drizzle/`. CI runs only `bunx drizzle-kit migrate`; SQL placed elsewhere is dead code._ - **マイグレーションの書き方**: diff --git a/server/api/src/lib/extractPlainTextFromYXml.ts b/server/api/src/lib/extractPlainTextFromYXml.ts index 23541c12..15eaad9b 100644 --- a/server/api/src/lib/extractPlainTextFromYXml.ts +++ b/server/api/src/lib/extractPlainTextFromYXml.ts @@ -73,5 +73,6 @@ export const CONTENT_PREVIEW_MAX_LENGTH = 120; export function buildContentPreview(text: string): string { const trimmed = text.trim().replace(/\s+/g, " "); if (trimmed.length <= CONTENT_PREVIEW_MAX_LENGTH) return trimmed; - return trimmed.slice(0, CONTENT_PREVIEW_MAX_LENGTH).trim() + "..."; + const headLength = Math.max(0, CONTENT_PREVIEW_MAX_LENGTH - 3); + return `${trimmed.slice(0, headLength).trim()}...`; } diff --git a/server/hocuspocus/src/extractPlainTextFromYXml.test.ts b/server/hocuspocus/src/extractPlainTextFromYXml.test.ts index 80dbf313..e875f427 100644 --- a/server/hocuspocus/src/extractPlainTextFromYXml.test.ts +++ b/server/hocuspocus/src/extractPlainTextFromYXml.test.ts @@ -140,6 +140,6 @@ describe("buildContentPreview", () => { const long = "x".repeat(200); const prev = buildContentPreview(long); expect(prev.endsWith("...")).toBe(true); - expect(prev.length).toBeLessThanOrEqual(124); + expect(prev.length).toBeLessThanOrEqual(120); }); }); diff --git a/server/hocuspocus/src/extractPlainTextFromYXml.ts b/server/hocuspocus/src/extractPlainTextFromYXml.ts index 8ac6fd31..053416f5 100644 --- a/server/hocuspocus/src/extractPlainTextFromYXml.ts +++ b/server/hocuspocus/src/extractPlainTextFromYXml.ts @@ -72,5 +72,6 @@ export const CONTENT_PREVIEW_MAX_LENGTH = 120; export function buildContentPreview(text: string): string { const trimmed = text.trim().replace(/\s+/g, " "); if (trimmed.length <= CONTENT_PREVIEW_MAX_LENGTH) return trimmed; - return trimmed.slice(0, CONTENT_PREVIEW_MAX_LENGTH).trim() + "..."; + const headLength = Math.max(0, CONTENT_PREVIEW_MAX_LENGTH - 3); + return `${trimmed.slice(0, headLength).trim()}...`; } diff --git a/src/components/editor/TiptapEditor/useWikiLinkStatusSync.test.tsx b/src/components/editor/TiptapEditor/useWikiLinkStatusSync.test.tsx index aed41954..ca3fddb6 100644 --- a/src/components/editor/TiptapEditor/useWikiLinkStatusSync.test.tsx +++ b/src/components/editor/TiptapEditor/useWikiLinkStatusSync.test.tsx @@ -19,10 +19,10 @@ vi.mock("@/hooks/usePageQueries", () => ({ checkExistence: vi.fn(async (titles: string[]) => { const inScope = options?.pageNoteId ? (options.notePages ?? []) : []; const pageTitles = new Set(inScope.map((page) => page.title.toLowerCase().trim())); - // issue #737: `pageTitleToId` を返すモック契約。テストは「resolution は - // するが id 解決は不要」のシナリオを想定しているため空 Map を返しても十分。 - // Mock contract for issue #737. The current scenarios only assert - // exists/referenced changes, so an empty map is fine. + // issue #737: `pageTitleToId` を返すモック契約。`targetId` 解決を伴う + // シナリオを検証できるよう、note スコープ内ページから title→id を構築する。 + // Mock contract for issue #737. Build a title→id map from in-scope + // pages so `targetId` resolution paths are testable. const pageTitleToId = new Map( inScope.map((page) => [page.title.toLowerCase().trim(), page.id]), ); diff --git a/src/components/editor/extensions/TagExtension.test.ts b/src/components/editor/extensions/TagExtension.test.ts index b7f99e46..c65a0392 100644 --- a/src/components/editor/extensions/TagExtension.test.ts +++ b/src/components/editor/extensions/TagExtension.test.ts @@ -219,12 +219,14 @@ describe("Tag extension configuration", () => { if (typeof addAttributes !== "function") { throw new Error("addAttributes must be a function"); } - const attrs = addAttributes.call({ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...(extension as any), + type AddAttributesContext = Record & { + parent?: (() => Record) | undefined; + }; + const context: AddAttributesContext = { + ...extension, parent: undefined, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any) as Record; + }; + const attrs = addAttributes.call(context) as Record; const targetId = attrs.targetId as ReturnType; if (!targetId) throw new Error("targetId attribute missing"); return targetId; @@ -243,6 +245,9 @@ describe("Tag extension configuration", () => { const empty = document.createElement("span"); expect(spec.parseHTML(empty)).toBeNull(); + + empty.setAttribute("data-target-id", ""); + expect(spec.parseHTML(empty)).toBeNull(); }); it("omits data-target-id when targetId is null or empty", () => { diff --git a/src/components/editor/extensions/TagExtension.ts b/src/components/editor/extensions/TagExtension.ts index d5451191..a6a60118 100644 --- a/src/components/editor/extensions/TagExtension.ts +++ b/src/components/editor/extensions/TagExtension.ts @@ -167,7 +167,12 @@ export const Tag = Mark.create({ */ targetId: { default: null, - parseHTML: (element) => element.getAttribute("data-target-id"), + parseHTML: (element) => { + const raw = element.getAttribute("data-target-id"); + if (typeof raw !== "string") return null; + const normalized = raw.trim(); + return normalized.length > 0 ? normalized : null; + }, renderHTML: (attributes) => { const value = attributes.targetId; if (typeof value !== "string" || value.length === 0) { diff --git a/src/components/editor/extensions/WikiLinkExtension.test.ts b/src/components/editor/extensions/WikiLinkExtension.test.ts index b5a8841f..96a9e15c 100644 --- a/src/components/editor/extensions/WikiLinkExtension.test.ts +++ b/src/components/editor/extensions/WikiLinkExtension.test.ts @@ -107,12 +107,14 @@ describe("WikiLinkExtension paste rule", () => { if (typeof addAttributes !== "function") { throw new Error("addAttributes must be a function"); } - const attrs = addAttributes.call({ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...(extension as any), + type AddAttributesContext = Record & { + parent?: (() => Record) | undefined; + }; + const context: AddAttributesContext = { + ...extension, parent: undefined, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any) as Record; + }; + const attrs = addAttributes.call(context) as Record; const targetId = attrs.targetId as ReturnType; if (!targetId) throw new Error("targetId attribute missing"); return targetId; @@ -131,6 +133,9 @@ describe("WikiLinkExtension paste rule", () => { const empty = document.createElement("span"); expect(spec.parseHTML(empty)).toBeNull(); + + empty.setAttribute("data-target-id", ""); + expect(spec.parseHTML(empty)).toBeNull(); }); it("omits data-target-id when targetId is null or empty", () => { diff --git a/src/components/editor/extensions/WikiLinkExtension.ts b/src/components/editor/extensions/WikiLinkExtension.ts index cd0f62b4..784f476b 100644 --- a/src/components/editor/extensions/WikiLinkExtension.ts +++ b/src/components/editor/extensions/WikiLinkExtension.ts @@ -118,7 +118,12 @@ export const WikiLink = Mark.create({ */ targetId: { default: null, - parseHTML: (element) => element.getAttribute("data-target-id"), + parseHTML: (element) => { + const raw = element.getAttribute("data-target-id"); + if (typeof raw !== "string") return null; + const normalized = raw.trim(); + return normalized.length > 0 ? normalized : null; + }, renderHTML: (attributes) => { const value = attributes.targetId; if (typeof value !== "string" || value.length === 0) { diff --git a/src/components/editor/extensions/slashSuggestionPlugin.test.ts b/src/components/editor/extensions/slashSuggestionPlugin.test.ts index 0a964a0e..68c65b18 100644 --- a/src/components/editor/extensions/slashSuggestionPlugin.test.ts +++ b/src/components/editor/extensions/slashSuggestionPlugin.test.ts @@ -148,11 +148,13 @@ describe("slashSuggestionPlugin — deactivation", () => { const plugin = getPlugin(onStateChange); // 1) Activate with `/foo`. + // 1) `/foo` でアクティブ化する。 let state = makeState("/foo", plugin); expect(slashSuggestionPluginKey.getState(state)?.active).toBe(true); onStateChange.mockClear(); // 2) Expand the selection to a range; the plugin must turn off. + // 2) 選択範囲をレンジに広げると、プラグインは非アクティブになる。 const tr = state.tr.setSelection(TextSelection.create(state.doc, 1, 5)); state = state.apply(tr); const pluginState = slashSuggestionPluginKey.getState(state); @@ -182,6 +184,7 @@ describe("slashSuggestionPlugin — deactivation", () => { const plugin = getPlugin(); // Active first. + // まずアクティブ状態にする。 let state = makeState("/foo", plugin); expect(slashSuggestionPluginKey.getState(state)?.active).toBe(true); diff --git a/src/hooks/useMermaidGenerator.test.ts b/src/hooks/useMermaidGenerator.test.ts index 0889f787..9d2d34d8 100644 --- a/src/hooks/useMermaidGenerator.test.ts +++ b/src/hooks/useMermaidGenerator.test.ts @@ -40,6 +40,7 @@ import { useMermaidGenerator } from "./useMermaidGenerator"; // Safety net to keep spies (e.g. console.error if added in future tests) and // any module-level setup from leaking between tests on assertion failures. +// assertion 失敗時でも spy やモジュールレベルの設定が次のテストへ漏れないようにする。 afterEach(() => { vi.restoreAllMocks(); }); @@ -225,6 +226,7 @@ describe("useMermaidGenerator", () => { const { result } = renderHook(() => useMermaidGenerator()); // Trigger an initial error first. + // まず初期エラーを発生させる。 await act(async () => { await result.current.generate("", ["flowchart"]); }); @@ -232,6 +234,7 @@ describe("useMermaidGenerator", () => { // Stub a long-running generation that resolves later so we can observe the // transitional state. + // 遷移中の状態を観測できるよう、あとで resolve する長時間生成を stub する。 let resolveCb: (() => void) | null = null; mockGenerateMermaidDiagram.mockImplementation( ( diff --git a/src/hooks/useWorkflowDraft.test.ts b/src/hooks/useWorkflowDraft.test.ts index 47ece40b..c4d17b9a 100644 --- a/src/hooks/useWorkflowDraft.test.ts +++ b/src/hooks/useWorkflowDraft.test.ts @@ -225,6 +225,7 @@ describe("useWorkflowDraft - import / export", () => { expect(createObjectURLSpy).toHaveBeenCalledTimes(1); expect(clickSpy).toHaveBeenCalledTimes(1); + // キューされた setTimeout(..., 0) を実行して revokeObjectURL を発火させる。 // Flush the queued setTimeout(..., 0) that triggers revokeObjectURL. vi.runAllTimers(); expect(revokeObjectURLSpy).toHaveBeenCalledWith("blob:mock-url"); diff --git a/src/hooks/useWorkflowRunSession.test.ts b/src/hooks/useWorkflowRunSession.test.ts index 9b97accb..1001ed83 100644 --- a/src/hooks/useWorkflowRunSession.test.ts +++ b/src/hooks/useWorkflowRunSession.test.ts @@ -284,6 +284,7 @@ describe("useWorkflowRunSession - resume", () => { it("resume passes startStepIndex/stepOutputs/resumePartial back to runWorkflowExecution", async () => { // First, drive a paused outcome so the hook captures pausedState. + // まず paused outcome を発生させ、hook が pausedState を保持する状態にする。 mockRunWorkflowExecution.mockResolvedValueOnce({ outcome: "paused", pausedAtStepIndex: 1, @@ -345,6 +346,7 @@ describe("useWorkflowRunSession - resume", () => { await waitFor(() => expect(result.current.pausedState).not.toBeNull()); // User edited the draft and removed `s2`. + // ユーザーが draft を編集し、`s2` を削除したケース。 const editedDraft = makeDraft({ steps: [{ id: "s1", title: "Step One", instruction: "do" }], }); @@ -359,6 +361,7 @@ describe("useWorkflowRunSession - resume", () => { variant: "destructive", }); // After aborting, pausedState is reset to null. + // abort 後、pausedState が null にリセットされる。 expect(result.current.pausedState).toBeNull(); }); }); @@ -401,6 +404,7 @@ describe("useWorkflowRunSession - cleanup and signals", () => { expect(capturedStepAbort?.signal.aborted).toBe(true); // Drain the promise so the test does not leak. + // テストリークを防ぐために Promise を解放する。 await act(async () => { resolveExecution?.({ outcome: "completed" }); await pending; @@ -482,6 +486,7 @@ describe("useWorkflowRunSession - cleanup and signals", () => { expect(capturedStepAbort?.signal.aborted).toBe(true); // Resolve to avoid leaking the pending promise into the next test. + // 次のテストへ pending Promise が漏れないように resolve する。 resolveExecution?.({ outcome: "stopped" }); await pending; }); diff --git a/src/lib/aiServiceServer.test.ts b/src/lib/aiServiceServer.test.ts index 199a594e..aaadf918 100644 --- a/src/lib/aiServiceServer.test.ts +++ b/src/lib/aiServiceServer.test.ts @@ -358,10 +358,12 @@ describe("aiServiceServer / callAIWithServer", () => { pulledOnce = true; controller.enqueue(encoder.encode('data: {"content":"a"}\n')); // 次の pull の前に abort + // Abort before the next pull. abortController.abort(); return; } controller.enqueue(encoder.encode(":\n")); + controller.close(); }, }); fetchSpy.mockResolvedValue(new Response(stream, { status: 200 })); diff --git a/src/stores/aiChatStore.test.ts b/src/stores/aiChatStore.test.ts index 3a20f2bc..6700d056 100644 --- a/src/stores/aiChatStore.test.ts +++ b/src/stores/aiChatStore.test.ts @@ -176,6 +176,8 @@ describe("aiChatStore", () => { // partialize されるので rehydrate 後に取り込まれない…はずが、zustand // の persist はそのまま反映してしまう。partialize は書き込み側のみ。 // ここでは契約として「書き込み時に partialize される」ことを担保する。 + // Even if volatile fields exist in storage, persist rehydrate can reflect them as-is. + // `partialize` only applies on write, so this test guards the write-side contract. }, }), ); @@ -188,6 +190,7 @@ describe("aiChatStore", () => { expect(state.selectedModel).toEqual(model); // CodeRabbit のレビュー対応: 揮発フィールドが rehydrate で蘇らないことを明示確認。 // Pin volatile fields explicitly so the test name matches its assertions. + // テスト名と検証内容の整合を保つため、揮発フィールドを明示的に検証する。 expect(state.activeConversationId).toBeNull(); expect(state.isStreaming).toBe(false); expect(state.showConversationList).toBe(false);