Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/stacked-markdown-comments.md
Original file line number Diff line number Diff line change
@@ -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.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
30 changes: 20 additions & 10 deletions packages/app/src/components/DocPanel.dom.test.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/**
* Behavioral tests for DocPanel's single-file tab gating.
*
* Single-file `ok <file>` 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 <file>` 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';
Expand Down Expand Up @@ -48,13 +48,16 @@ mock.module('@/components/OutlinePanel', () => ({
mock.module('@/components/LinksPanel', () => ({
LinksPanel: () => <div data-testid="links-panel" />,
}));
mock.module('@/components/DocumentCommentsPanel', () => ({
DocumentCommentsPanel: () => <div data-testid="comments-panel" />,
}));
mock.module('@/components/TimelinePanel', () => ({
TimelineContent: () => <div data-testid="timeline-panel" />,
}));

const { DocPanel } = await import('./DocPanel');

function renderPanel(activeTab: 'outline' | 'links' | 'graph' | 'timeline') {
function renderPanel(activeTab: 'outline' | 'comments' | 'links' | 'graph' | 'timeline') {
return render(
<TooltipProvider>
<DocPanel
Expand All @@ -74,19 +77,26 @@ afterEach(() => {
});

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();
});
});
21 changes: 13 additions & 8 deletions packages/app/src/components/DocPanel.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
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';
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 },
Expand All @@ -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`;
Expand Down Expand Up @@ -72,13 +75,14 @@ export function DocPanel({
// TimelineContent unmounts when activeTab leaves 'timeline'.
const { t } = useLingui();
const [diffLayout, setDiffLayout] = useState<DiffLayout>('unified');
// Single-file `ok <file>` 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 <file>` 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 (
Expand Down Expand Up @@ -139,6 +143,7 @@ export function DocPanel({
{effectiveTab === 'outline' && (
<OutlinePanel docName={docName} isSourceMode={isSourceMode} />
)}
{effectiveTab === 'comments' && <DocumentCommentsPanel docName={docName} />}
{effectiveTab === 'links' && <LinksPanel docName={docName} />}
{effectiveTab === 'graph' && (
<Suspense
Expand Down
185 changes: 185 additions & 0 deletions packages/app/src/components/DocumentCommentsPanel.dom.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test';
import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
import type { ReactNode } from 'react';
import type { CommentAnchor } from '@/editor/comments/comment-store';

mock.module('@lingui/react/macro', () => ({
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 };
}) => (
<button
type="button"
data-testid={testIds.primary}
disabled={primaryDisabled}
onClick={onPrimary}
>
{primary}
</button>
),
}));

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> = {}): 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(<DocumentCommentsPanel docName="notes" />);

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([]);
});
});
Loading
Loading