diff --git a/.changeset/stacked-markdown-comments.md b/.changeset/stacked-markdown-comments.md new file mode 100644 index 00000000..ab07c099 --- /dev/null +++ b/.changeset/stacked-markdown-comments.md @@ -0,0 +1,5 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Add review comments to selected text in the WYSIWYG markdown editor. Selecting a passage now offers a Comment action, stacks saved comments in the right document panel, highlights each anchored passage in the editor, and can send the collected feedback to the chosen AI target when you are ready. diff --git a/artifacts/stacked-markdown-comments-screenshot.png b/artifacts/stacked-markdown-comments-screenshot.png new file mode 100644 index 00000000..3617418e Binary files /dev/null and b/artifacts/stacked-markdown-comments-screenshot.png differ diff --git a/packages/app/src/components/DocPanel.dom.test.tsx b/packages/app/src/components/DocPanel.dom.test.tsx index a9c3577e..91c52bd8 100644 --- a/packages/app/src/components/DocPanel.dom.test.tsx +++ b/packages/app/src/components/DocPanel.dom.test.tsx @@ -1,11 +1,11 @@ /** * Behavioral tests for DocPanel's single-file tab gating. * - * Single-file `ok ` keeps only the Outline tab — Links/Graph need a - * multi-doc knowledge base and Timeline is git history, all empty/inert for a - * lone git-off file. Asserts the rendered tab set (by `role="tab"` count, so the - * test doesn't depend on localized label text) and that a persisted - * links/graph/timeline selection coerces back to Outline. + * Single-file `ok ` keeps only document-local tabs — Outline and Comments. + * Links/Graph need a multi-doc knowledge base and Timeline is git history, all + * empty/inert for a lone git-off file. Asserts the rendered tab set (by + * `role="tab"` count, so the test doesn't depend on localized label text) and + * that a persisted links/graph/timeline selection coerces back to Outline. */ import { afterEach, describe, expect, mock, test } from 'bun:test'; @@ -48,13 +48,16 @@ mock.module('@/components/OutlinePanel', () => ({ mock.module('@/components/LinksPanel', () => ({ LinksPanel: () =>
, })); +mock.module('@/components/DocumentCommentsPanel', () => ({ + DocumentCommentsPanel: () =>
, +})); mock.module('@/components/TimelinePanel', () => ({ TimelineContent: () =>
, })); const { DocPanel } = await import('./DocPanel'); -function renderPanel(activeTab: 'outline' | 'links' | 'graph' | 'timeline') { +function renderPanel(activeTab: 'outline' | 'comments' | 'links' | 'graph' | 'timeline') { return render( { }); describe('DocPanel — single-file tab gating', () => { - test('project mode renders the full tab strip (outline + links + graph + timeline)', () => { + test('project mode renders the full tab strip (outline + comments + links + graph + timeline)', () => { singleFileValue = false; renderPanel('outline'); - expect(screen.getAllByRole('tab')).toHaveLength(4); + expect(screen.getAllByRole('tab')).toHaveLength(5); expect(screen.getByTestId('outline-panel')).toBeTruthy(); }); - test('single-file mode drops the tab strip and shows only the Outline', () => { + test('single-file mode keeps Outline and Comments and coerces hidden tabs back to Outline', () => { singleFileValue = true; // Persisted selection is 'graph' — it must coerce back to Outline rather // than render a now-hidden panel. renderPanel('graph'); - expect(screen.queryAllByRole('tab')).toHaveLength(0); + expect(screen.getAllByRole('tab')).toHaveLength(2); expect(screen.getByTestId('outline-panel')).toBeTruthy(); }); + + test('single-file mode can render the Comments panel', () => { + singleFileValue = true; + renderPanel('comments'); + expect(screen.getAllByRole('tab')).toHaveLength(2); + expect(screen.getByTestId('comments-panel')).toBeTruthy(); + }); }); diff --git a/packages/app/src/components/DocPanel.tsx b/packages/app/src/components/DocPanel.tsx index 89945596..38598150 100644 --- a/packages/app/src/components/DocPanel.tsx +++ b/packages/app/src/components/DocPanel.tsx @@ -1,8 +1,9 @@ import { t } from '@lingui/core/macro'; import { Trans, useLingui } from '@lingui/react/macro'; -import { Clock, Link2, ListTree, Network } from 'lucide-react'; +import { Clock, Link2, ListTree, MessageSquareText, Network } from 'lucide-react'; import { lazy, Suspense, useState } from 'react'; import type { DiffLayout } from '@/components/DiffView'; +import { DocumentCommentsPanel } from '@/components/DocumentCommentsPanel'; import { LinksPanel } from '@/components/LinksPanel'; import { OutlinePanel } from '@/components/OutlinePanel'; import { TimelineContent } from '@/components/TimelinePanel'; @@ -10,10 +11,11 @@ import { ToggleGroup, ToggleGroupItem } from '@/components/ui/toggle-group'; import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'; import { useSingleFileMode } from '@/lib/single-file-mode'; -export type PanelTab = 'outline' | 'links' | 'graph' | 'timeline'; +export type PanelTab = 'outline' | 'comments' | 'links' | 'graph' | 'timeline'; export const TABS: { id: PanelTab; icon: typeof ListTree }[] = [ { id: 'outline', icon: ListTree }, + { id: 'comments', icon: MessageSquareText }, { id: 'links', icon: Link2 }, { id: 'graph', icon: Network }, { id: 'timeline', icon: Clock }, @@ -22,6 +24,7 @@ export const TABS: { id: PanelTab; icon: typeof ListTree }[] = [ /** Localized display label for a doc-panel tab. */ function tabLabel(id: PanelTab): string { if (id === 'outline') return t`Outline`; + if (id === 'comments') return t`Comments`; if (id === 'links') return t`Links`; if (id === 'graph') return t`Graph`; return t`Timeline`; @@ -72,13 +75,14 @@ export function DocPanel({ // TimelineContent unmounts when activeTab leaves 'timeline'. const { t } = useLingui(); const [diffLayout, setDiffLayout] = useState('unified'); - // Single-file `ok ` keeps only the Outline tab. Links/Graph need a - // multi-doc knowledge base, and Timeline is git history — all empty or inert - // for a lone git-off file. Coerce a persisted links/graph/timeline selection - // back to outline so the rail never renders a now-hidden panel, and drop the - // one-item tab strip entirely. + // Single-file `ok ` keeps document-local panels only. Links/Graph need + // a multi-doc knowledge base, and Timeline is git history — all empty or + // inert for a lone git-off file. Coerce a persisted links/graph/timeline + // selection back to outline so the rail never renders a now-hidden panel. const singleFile = useSingleFileMode(); - const tabs = singleFile ? TABS.filter((tab) => tab.id === 'outline') : TABS; + const tabs = singleFile + ? TABS.filter((tab) => tab.id === 'outline' || tab.id === 'comments') + : TABS; const effectiveTab: PanelTab = tabs.some((tab) => tab.id === activeTab) ? activeTab : 'outline'; const showTabStrip = mode === 'doc' && tabs.length > 1; return ( @@ -139,6 +143,7 @@ export function DocPanel({ {effectiveTab === 'outline' && ( )} + {effectiveTab === 'comments' && } {effectiveTab === 'links' && } {effectiveTab === 'graph' && ( ({ + Trans: ({ children }: { children: ReactNode }) => <>{children}, + useLingui: () => ({ + t: (strings: TemplateStringsArray, ...values: unknown[]) => + strings.reduce((acc, part, index) => `${acc}${part}${values[index] ?? ''}`, ''), + }), +})); + +mock.module('@/components/handoff/AgentSplitButton', () => ({ + AgentSplitButton: ({ + primary, + onPrimary, + primaryDisabled, + testIds, + }: { + primary: ReactNode; + onPrimary: () => void; + primaryDisabled?: boolean; + testIds: { primary: string }; + }) => ( + + ), +})); + +mock.module('@/components/handoff/TerminalLaunchContext', () => ({ + useTerminalLaunch: () => null, +})); + +mock.module('@/hooks/use-installed-clis', () => ({ + useInstalledClis: () => ({}), +})); + +mock.module('@/components/handoff/useInstalledAgents', () => ({ + useInstalledAgents: () => ({ + states: { codex: { installed: true } }, + refresh: () => Promise.resolve(), + }), +})); + +mock.module('@/lib/use-workspace', () => ({ + useWorkspace: () => ({ contentDir: '/tmp/project', pathSeparator: '/' }), +})); + +const recordOnboardingAskedAi = mock(() => {}); +mock.module('@/lib/onboarding-signals', () => ({ recordOnboardingAskedAi })); + +const dispatchCalls: Array<{ target: string; input: unknown }> = []; +const buildCalls: Array<{ + docName: string | null; + workspace: unknown; + instruction: string; + mentions: readonly string[]; +}> = []; + +mock.module('@/components/handoff/useHandoffDispatch', () => ({ + useHandoffDispatch: () => ({ + dispatch: (target: string, input: unknown) => { + dispatchCalls.push({ target, input }); + return Promise.resolve({ ok: true as const }); + }, + }), + buildComposerHandoffInput: (args: { + docName: string | null; + workspace: unknown; + instruction: string; + mentions: readonly string[]; + }) => { + buildCalls.push(args); + if (!args.workspace) return null; + return { + compose: { + instruction: args.instruction, + mentions: args.mentions, + }, + }; + }, +})); + +const toastErrors: string[] = []; +mock.module('sonner', () => ({ + toast: { + error: (message: string) => toastErrors.push(message), + }, +})); + +const { DocumentCommentsPanel } = await import('./DocumentCommentsPanel'); +const { getDocumentCommentSnapshot, resetDocumentCommentsForTests, setPendingDocumentComment } = + await import('@/editor/comments/comment-store'); + +function anchor(overrides: Partial = {}): CommentAnchor { + const anchorText = overrides.anchorText ?? 'selected text'; + const markdown = overrides.markdown ?? anchorText; + return { + docName: 'notes', + textStart: 0, + textEnd: anchorText.length, + anchorText, + markdown, + charLen: markdown.length, + lineCount: (markdown.match(/\n/g)?.length ?? 0) + 1, + ...overrides, + }; +} + +beforeEach(() => { + dispatchCalls.length = 0; + buildCalls.length = 0; + toastErrors.length = 0; + recordOnboardingAskedAi.mockClear(); + resetDocumentCommentsForTests(); +}); + +afterEach(() => { + cleanup(); + resetDocumentCommentsForTests(); +}); + +describe('DocumentCommentsPanel', () => { + test('stacks selected comments and submits one formatted instruction to the resolved agent', async () => { + setPendingDocumentComment( + anchor({ + anchorText: 'first selected passage', + markdown: '**first selected passage**', + }), + ); + + render(); + + fireEvent.change(screen.getByPlaceholderText('Add a comment'), { + target: { value: 'Tighten the first claim.' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Add comment' })); + + act(() => { + setPendingDocumentComment( + anchor({ + textStart: 40, + textEnd: 62, + anchorText: 'second selected passage', + markdown: 'second selected passage', + }), + ); + }); + fireEvent.change(screen.getByPlaceholderText('Add a comment'), { + target: { value: 'Add source detail here.' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Add comment' })); + + expect(screen.getByText('Tighten the first claim.')).toBeTruthy(); + expect(screen.getByText('Add source detail here.')).toBeTruthy(); + expect(getDocumentCommentSnapshot('notes').comments).toHaveLength(2); + + fireEvent.click(screen.getByTestId('comments-send')); + + await waitFor(() => expect(dispatchCalls).toHaveLength(1)); + + expect(dispatchCalls[0]?.target).toBe('codex'); + expect(buildCalls).toHaveLength(1); + expect(buildCalls[0]?.docName).toBe('notes'); + expect(buildCalls[0]?.mentions).toEqual([]); + expect(buildCalls[0]?.instruction).toContain('Comment 1'); + expect(buildCalls[0]?.instruction).toContain('**first selected passage**'); + expect(buildCalls[0]?.instruction).toContain('> Tighten the first claim.'); + expect(buildCalls[0]?.instruction).toContain('Comment 2'); + expect(buildCalls[0]?.instruction).toContain('second selected passage'); + expect(buildCalls[0]?.instruction).toContain('> Add source detail here.'); + + await waitFor(() => expect(getDocumentCommentSnapshot('notes').comments).toHaveLength(0)); + expect(screen.getByText('No comments')).toBeTruthy(); + expect(recordOnboardingAskedAi).toHaveBeenCalledTimes(1); + expect(toastErrors).toEqual([]); + }); +}); diff --git a/packages/app/src/components/DocumentCommentsPanel.tsx b/packages/app/src/components/DocumentCommentsPanel.tsx new file mode 100644 index 00000000..ce2ccbdd --- /dev/null +++ b/packages/app/src/components/DocumentCommentsPanel.tsx @@ -0,0 +1,387 @@ +import { + type TargetData, + TERMINAL_CLI_IDS, + TERMINAL_CLIS, + type TerminalCli, +} from '@inkeep/open-knowledge-core'; +import { Trans, useLingui } from '@lingui/react/macro'; +import { Loader2, MessageSquareText, Send, Trash2 } from 'lucide-react'; +import { useEffect, useRef, useState } from 'react'; +import { toast } from 'sonner'; +import { AgentSplitButton } from '@/components/handoff/AgentSplitButton'; +import { useTerminalLaunch } from '@/components/handoff/TerminalLaunchContext'; +import { + buildComposerHandoffInput, + useHandoffDispatch, +} from '@/components/handoff/useHandoffDispatch'; +import { Button } from '@/components/ui/button'; +import { Textarea } from '@/components/ui/textarea'; +import { + addPendingDocumentComment, + clearDocumentComments, + clearPendingDocumentComment, + type DocumentComment, + deleteDocumentComment, + formatCommentsForAgent, + setActiveDocumentComment, + updateDocumentCommentBody, + useDocumentComments, +} from '@/editor/comments/comment-store'; +import { useInstalledClis } from '@/hooks/use-installed-clis'; +import { resolveDefaultCli } from '@/lib/default-cli-resolver'; +import { VISIBLE_TARGETS } from '@/lib/handoff/targets'; +import { recordOnboardingAskedAi } from '@/lib/onboarding-signals'; +import { + loadStickyAgent, + parseStickyCliId, + resolveStickyAgent, + saveStickyAgent, + terminalCliId, +} from '@/lib/unified-agent-store'; +import { useWorkspace } from '@/lib/use-workspace'; +import { cn } from '@/lib/utils'; +import { useInstalledAgents } from './handoff/useInstalledAgents'; + +export function DocumentCommentsPanel({ docName }: { readonly docName: string }) { + const { t } = useLingui(); + const workspace = useWorkspace(); + const { states } = useInstalledAgents(); + const { dispatch } = useHandoffDispatch(); + const terminalLaunch = useTerminalLaunch(); + const installedClis = useInstalledClis(); + const snapshot = useDocumentComments(docName); + const [body, setBody] = useState(''); + const [pending, setPending] = useState(false); + const [stickyId] = useState(() => loadStickyAgent()); + const [selectedId, setSelectedId] = useState(null); + const textareaRef = useRef(null); + + useEffect(() => { + setBody(''); + if (snapshot.pending) { + const id = requestAnimationFrame(() => textareaRef.current?.focus()); + return () => cancelAnimationFrame(id); + } + return undefined; + }, [snapshot.pending]); + + const effectiveId = selectedId ?? stickyId; + const explicitCli: TerminalCli | null = + terminalLaunch !== null ? parseStickyCliId(effectiveId) : null; + const defaultCli: TerminalCli | null = + terminalLaunch !== null && effectiveId === null ? resolveDefaultCli(null, installedClis) : null; + const selectedCli = explicitCli ?? defaultCli; + const resolvedTarget = selectedCli !== null ? null : resolveStickyAgent(states, effectiveId); + const installedTargets = VISIBLE_TARGETS.filter( + (target) => states[target.id]?.installed === true, + ); + const agentProbePending = VISIBLE_TARGETS.some((target) => states[target.id]?.installed == null); + const canSend = + !pending && snapshot.comments.length > 0 && (selectedCli !== null || resolvedTarget !== null); + + const handleSelectAgent = (target: TargetData) => { + setSelectedId(target.id); + saveStickyAgent(target.id); + }; + + const handleSelectCli = (cli: TerminalCli) => { + const id = terminalCliId(cli); + setSelectedId(id); + saveStickyAgent(id); + }; + + const cliRows = + terminalLaunch !== null + ? TERMINAL_CLI_IDS.map((cli) => { + const { displayName } = TERMINAL_CLIS[cli]; + return { + cli, + label: displayName, + ariaLabel: t`${displayName} CLI`, + selected: selectedCli === cli, + onSelect: () => handleSelectCli(cli), + }; + }) + : undefined; + + function addComment() { + if (!body.trim()) return; + addPendingDocumentComment(docName, body); + setBody(''); + } + + function sendComments() { + if (!canSend) return; + const instruction = formatCommentsForAgent(snapshot.comments); + const input = buildComposerHandoffInput({ + docName, + workspace, + instruction, + mentions: [], + }); + if (input === null) { + toast.error(t`Couldn't send comments. Please try again.`); + return; + } + + const sentIds = snapshot.comments.map((comment) => comment.id); + if (selectedCli !== null && terminalLaunch !== null) { + try { + terminalLaunch.launchInTerminal(input, selectedCli); + } catch { + toast.error(t`Couldn't open the terminal. Please try again.`); + return; + } + recordOnboardingAskedAi(); + clearDocumentComments(docName, sentIds); + return; + } + + if (resolvedTarget === null) return; + setPending(true); + void dispatch(resolvedTarget.id, input) + .then((outcome) => { + if (!outcome.ok) return; + recordOnboardingAskedAi(); + clearDocumentComments(docName, sentIds); + }) + .finally(() => setPending(false)); + } + + return ( +
+
+
+ + + Comments + + {snapshot.comments.length > 0 ? ( + + {snapshot.comments.length} + + ) : null} +
+ + {pending ? ( + + ) : ( + + )} + + Send + + + } + onPrimary={sendComments} + primaryDisabled={!canSend} + installedTargets={installedTargets} + selectedTargetId={selectedCli === null ? (resolvedTarget?.id ?? null) : null} + onSelectTarget={handleSelectAgent} + terminals={cliRows} + menuEmptyState={ +
+ {agentProbePending ? Checking agents : No agents found} +
+ } + triggerAriaLabel={t`Choose comment target`} + testIds={{ + primary: 'comments-send', + trigger: 'comments-agent-menu-trigger', + menu: 'comments-agent-menu', + option: (id) => `comments-agent-option-${id}`, + terminal: (cli) => `comments-terminal-option-${cli}`, + }} + /> +
+ +
+ {snapshot.pending ? ( +
+
+ + {snapshot.pending.markdown || snapshot.pending.anchorText} + +
+