Compare commits
13 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| be8ed579a9 | |||
| d542fc58ae | |||
| 85ffdea06b | |||
| 9aa7620461 | |||
| 710a690c78 | |||
| 50f8ed717a | |||
| 1fc9c25681 | |||
| e074b101c7 | |||
| 4496afb481 | |||
| 1644de87f7 | |||
| 498a87f3c4 | |||
| 67c94de6ef | |||
| 27d51303ba |
@@ -198,6 +198,42 @@ MCP_DOCMOST_PASSWORD=
|
||||
# A slow/hung embeddings endpoint fails after this and the batch continues.
|
||||
# AI_EMBEDDING_TIMEOUT_MS=120000
|
||||
|
||||
# ─── #530 Semantic search (Phase B) ──────────────────────────────────────────
|
||||
# The GLOBAL embedding provider used by search (query embed) AND the indexer
|
||||
# (document embed) when a workspace has NO embedding provider of its own. It is
|
||||
# an OpenAI-compatible endpoint — the docker-compose `embeddings` (TEI) sidecar.
|
||||
# A workspace that configures its own embedding provider OVERRIDES all of this.
|
||||
# When neither resolves, search runs lexical-only (semantic.reason=no-provider).
|
||||
EMBEDDING_ENDPOINT=http://embeddings:80/v1
|
||||
EMBEDDING_MODEL=intfloat/multilingual-e5-small
|
||||
# Dummy — the self-hosted TEI sidecar is keyless.
|
||||
EMBEDDING_API_KEY=unused
|
||||
EMBEDDING_DIMENSIONS=384
|
||||
# PLACEHOLDER — set to a PINNED intfloat/multilingual-e5-small commit sha (used
|
||||
# both as the TEI --revision and as part of the embedding fingerprint). Keep this
|
||||
# in lockstep with docker-compose; a bump changes the fingerprint (PR-2 handles
|
||||
# the generational swap/GC — PR-1 uses a single active fingerprint).
|
||||
EMBEDDING_REVISION=REPLACE_WITH_PINNED_E5_SMALL_COMMIT_SHA
|
||||
# e5 models require these input prefixes. Empty them for a non-e5 model.
|
||||
EMBEDDING_QUERY_PREFIX="query: "
|
||||
EMBEDDING_DOC_PREFIX="passage: "
|
||||
# Per-request timeout (ms) for the interactive SEARCH query embed — much shorter
|
||||
# than the batch indexer timeout: a slow/hung sidecar degrades search fast to the
|
||||
# lexical-only path instead of blocking the request. Default 800.
|
||||
# SEARCH_EMBED_TIMEOUT_MS=800
|
||||
# RRF weight of the vector leg relative to each lexical leg (1.0 = equal). Default 1.0.
|
||||
# SEARCH_VECTOR_WEIGHT=1.0
|
||||
# Vector top-N pulled into the fused candidate union per request. Default 50.
|
||||
# SEARCH_VECTOR_CANDIDATES=50
|
||||
# Per-statement timeout (ms) bounding ONLY the fused vector query (the brute-force
|
||||
# KNN seq scan — there is no ANN index). If the vector scan exceeds this it is
|
||||
# cancelled and search degrades to lexical-only (never hangs the request). Scoped
|
||||
# per-query (SET LOCAL), so it never affects other queries. Default 2000.
|
||||
# SEARCH_VECTOR_STATEMENT_TIMEOUT_MS=2000
|
||||
# Kill-switch: set to `off` to disable the semantic layer entirely (search then
|
||||
# runs the byte-identical Phase-A lexical path). Default on.
|
||||
# SEARCH_SEMANTIC=on
|
||||
|
||||
# Silence timeout (ms) for streaming chat/agent AI calls AND external-MCP traffic.
|
||||
# Bounds time-to-first-byte and the gap BETWEEN chunks (NOT the total turn length),
|
||||
# so an arbitrarily long turn that keeps streaming is never cut. Finite so a hung
|
||||
|
||||
@@ -1,460 +0,0 @@
|
||||
/**
|
||||
* CommentsPanel — редизайн панели комментариев с фокусом на агентские правки.
|
||||
* Mantine v7. Узкая колонка ~340–400px, светлая/тёмная темы.
|
||||
*
|
||||
* Реализованные решения (см. README.md):
|
||||
* - Дифф-первая карточка: цитата = старая строка диффа (без тройного дублирования)
|
||||
* - Группировка серии прогона под одной шапкой ("Корректор · 28 правок")
|
||||
* - Пакетное "Accept all" на фронте (по одной под капотом) с полоской прогресса
|
||||
* - Метки важности/категории парсятся клиентом из тегов "[Корректура][Существенно]"
|
||||
* - Фильтры по важности/категории
|
||||
* - Безопасный Dismiss через undo-тост (удаление откладывается на клиенте)
|
||||
* - Состояния: pending / applied / dismissed / conflict (потерян якорь)
|
||||
* - Крайние диффы: вставка (␣), удаление, одна буква, дефис→тире, длинный абзац
|
||||
* - Человеческий тред не деградирует (та же система, другое наполнение)
|
||||
* - Вкладки Open/Resolved сохранены
|
||||
*/
|
||||
import { useMemo, useState, useCallback, useRef } from 'react';
|
||||
import {
|
||||
Box, Group, Stack, Text, Badge, Button, ActionIcon, Tabs, Progress,
|
||||
Avatar, Tooltip, ScrollArea, useMantineColorScheme, useMantineTheme,
|
||||
} from '@mantine/core';
|
||||
import { notifications } from '@mantine/notifications';
|
||||
|
||||
/* ─────────────────────────── Типы данных ─────────────────────────── */
|
||||
|
||||
export type Severity = 'critical' | 'major' | 'minor';
|
||||
export type Category = string; // "Корректура" | "Факт" | "Стиль" | …
|
||||
|
||||
/** Один сегмент интра-диффа: изменённый фрагмент подсвечивается. */
|
||||
export interface DiffSegment {
|
||||
text: string;
|
||||
changed: boolean;
|
||||
}
|
||||
|
||||
/** Правка: сервер уже отдаёт посегментную разметку обеих строк. */
|
||||
export interface SuggestedEdit {
|
||||
before: DiffSegment[]; // "было" (пустой массив ⇒ чистая вставка)
|
||||
after: DiffSegment[]; // "стало" (пустой массив ⇒ чистое удаление)
|
||||
}
|
||||
|
||||
export type CommentStatus = 'pending' | 'applied' | 'dismissed' | 'conflict';
|
||||
|
||||
export interface Comment {
|
||||
id: string;
|
||||
runId?: string; // id прогона агента — для группировки серии
|
||||
authorName: string; // "Корректор" | "Нарратор" | имя человека
|
||||
authorKind: 'agent' | 'human';
|
||||
triggeredBy?: string; // кто запустил агента ("vvzvlad")
|
||||
createdAtLabel: string; // "15 ч", "2 дн" — форматируется вызывающим кодом
|
||||
/** Сырой текст комментария от агента, метки в квадратных скобках. */
|
||||
bodyRaw: string;
|
||||
edit?: SuggestedEdit; // есть ⇒ агентская правка; нет ⇒ обычный тред
|
||||
status: CommentStatus;
|
||||
replyCount?: number;
|
||||
anchorLost?: boolean; // сервер ответил конфликтом якоря
|
||||
}
|
||||
|
||||
export interface CommentsPanelProps {
|
||||
comments: Comment[];
|
||||
/** Применить одну правку. Резолвит тред на сервере. */
|
||||
onApply: (id: string) => Promise<void>;
|
||||
/** Жёсткое удаление на сервере (для треда без ответов). Вызывается ПОСЛЕ окна undo. */
|
||||
onDismiss: (id: string) => Promise<void>;
|
||||
/** Резолв обычного треда. */
|
||||
onResolve?: (id: string) => Promise<void>;
|
||||
/** Клик по карточке/цитате — скролл документа к якорю и подсветка. */
|
||||
onNavigateToAnchor: (id: string) => void;
|
||||
onClose?: () => void;
|
||||
/** Мс до фактического удаления после Dismiss (окно undo). По умолчанию 5000. */
|
||||
dismissUndoMs?: number;
|
||||
}
|
||||
|
||||
/* ───────────────────── Клиентский парсинг меток ───────────────────── */
|
||||
|
||||
const SEVERITY_WORDS: Record<string, Severity> = {
|
||||
'существенно': 'major', 'критично': 'critical', 'критическая': 'critical',
|
||||
'незначительно': 'minor', 'мелко': 'minor', 'major': 'major',
|
||||
'minor': 'minor', 'critical': 'critical',
|
||||
};
|
||||
|
||||
interface ParsedBody {
|
||||
category?: Category;
|
||||
severity: Severity;
|
||||
text: string; // тело без скобочных тегов
|
||||
}
|
||||
|
||||
/** "[Корректура] [Существенно] Пробел…" → {category, severity, text}. */
|
||||
export function parseBody(raw: string): ParsedBody {
|
||||
const tags: string[] = [];
|
||||
const text = raw.replace(/\[([^\]]+)\]/g, (_, t) => { tags.push(t.trim()); return ''; }).trim();
|
||||
let severity: Severity = 'minor';
|
||||
let category: Category | undefined;
|
||||
for (const t of tags) {
|
||||
const sv = SEVERITY_WORDS[t.toLowerCase()];
|
||||
if (sv) severity = sv; else if (!category) category = t;
|
||||
}
|
||||
return { category, severity, text };
|
||||
}
|
||||
|
||||
const SEV_COLOR: Record<Severity, string> = {
|
||||
critical: 'red', major: 'orange', minor: 'gray',
|
||||
};
|
||||
|
||||
/* ──────────────────────────── Дифф ──────────────────────────── */
|
||||
|
||||
/** Делает невидимые символы видимыми в подсветке (пробел, таб). */
|
||||
function visibleWhitespace(s: string): string {
|
||||
return s.replace(/ /g, '␣').replace(/\t/g, '⇥');
|
||||
}
|
||||
|
||||
function DiffLine({ segments, kind }: { segments: DiffSegment[]; kind: 'del' | 'ins' }) {
|
||||
const theme = useMantineTheme();
|
||||
const { colorScheme } = useMantineColorScheme();
|
||||
const dark = colorScheme === 'dark';
|
||||
const isDel = kind === 'del';
|
||||
const sign = isDel ? '−' : '+';
|
||||
const signColor = isDel ? theme.colors.red[dark ? 5 : 6] : theme.colors.green[dark ? 5 : 6];
|
||||
const baseColor = isDel
|
||||
? (dark ? theme.colors.gray[5] : theme.colors.gray[6])
|
||||
: (dark ? theme.colors.gray[1] : theme.colors.dark[9]);
|
||||
const markBg = isDel
|
||||
? (dark ? 'rgba(224,49,49,.22)' : '#ffe3e3')
|
||||
: (dark ? 'rgba(47,158,68,.22)' : '#d3f9d8');
|
||||
const markFg = isDel
|
||||
? (dark ? theme.colors.red[3] : theme.colors.red[8])
|
||||
: (dark ? theme.colors.green[3] : theme.colors.green[9]);
|
||||
|
||||
return (
|
||||
<Group gap={7} wrap="nowrap" align="flex-start">
|
||||
<Text ff="monospace" fw={600} fz={12} c={signColor} w={11} ta="center" style={{ flex: 'none', lineHeight: 1.5 }}>{sign}</Text>
|
||||
<Text fz={13.5} c={baseColor} style={{ lineHeight: 1.45 }}>
|
||||
{segments.map((seg, i) =>
|
||||
seg.changed ? (
|
||||
<Box key={i} component="mark" px={3} fw={600}
|
||||
style={{ background: markBg, color: markFg, borderRadius: 3,
|
||||
textDecoration: isDel ? 'line-through' : 'none' }}>
|
||||
{visibleWhitespace(seg.text)}
|
||||
</Box>
|
||||
) : (
|
||||
<Box key={i} component="span" style={{ textDecoration: isDel ? 'line-through' : 'none' }}>{seg.text}</Box>
|
||||
)
|
||||
)}
|
||||
</Text>
|
||||
</Group>
|
||||
);
|
||||
}
|
||||
|
||||
function DiffBlock({ edit }: { edit: SuggestedEdit }) {
|
||||
const pureInsert = edit.before.length === 0;
|
||||
const pureDelete = edit.after.length === 0;
|
||||
return (
|
||||
<Stack gap={1}>
|
||||
{!pureInsert && <DiffLine segments={edit.before} kind="del" />}
|
||||
{!pureDelete && <DiffLine segments={edit.after} kind="ins" />}
|
||||
</Stack>
|
||||
);
|
||||
}
|
||||
|
||||
/* ──────────────────────── Карточка правки ──────────────────────── */
|
||||
|
||||
function EditCard({ c, onApply, onDismiss, onNavigateToAnchor, dismissUndoMs = 5000 }: {
|
||||
c: Comment;
|
||||
onApply: CommentsPanelProps['onApply'];
|
||||
onDismiss: CommentsPanelProps['onDismiss'];
|
||||
onNavigateToAnchor: CommentsPanelProps['onNavigateToAnchor'];
|
||||
dismissUndoMs?: number;
|
||||
}) {
|
||||
const parsed = useMemo(() => parseBody(c.bodyRaw), [c.bodyRaw]);
|
||||
const [busy, setBusy] = useState(false);
|
||||
const timer = useRef<number>();
|
||||
|
||||
const apply = useCallback(async () => {
|
||||
setBusy(true);
|
||||
try { await onApply(c.id); } finally { setBusy(false); }
|
||||
}, [c.id, onApply]);
|
||||
|
||||
// Безопасный Dismiss: прячем сразу, удаляем на сервере после окна undo.
|
||||
const dismiss = useCallback(() => {
|
||||
let undone = false;
|
||||
const nid = notifications.show({
|
||||
message: 'Edit dismissed',
|
||||
color: 'gray',
|
||||
autoClose: dismissUndoMs,
|
||||
withCloseButton: false,
|
||||
// Кнопка "Вернуть" — см. README про кастомный рендер экшена.
|
||||
});
|
||||
timer.current = window.setTimeout(() => {
|
||||
if (!undone) onDismiss(c.id);
|
||||
}, dismissUndoMs);
|
||||
// undo вызывается из UI: clearTimeout(timer.current); undone = true;
|
||||
return { nid, cancel: () => { undone = true; clearTimeout(timer.current); } };
|
||||
}, [c.id, onDismiss, dismissUndoMs]);
|
||||
|
||||
const conflict = c.status === 'conflict' || c.anchorLost;
|
||||
|
||||
return (
|
||||
<Box
|
||||
p="10px 12px"
|
||||
onClick={() => onNavigateToAnchor(c.id)}
|
||||
style={(t) => ({
|
||||
cursor: 'pointer',
|
||||
borderBottom: `1px solid ${t.colorScheme === 'dark' ? t.colors.dark[5] : t.colors.gray[1]}`,
|
||||
})}
|
||||
>
|
||||
<Stack gap={8}>
|
||||
{c.edit && <DiffBlock edit={c.edit} />}
|
||||
|
||||
{conflict && (
|
||||
<Group gap={7} wrap="nowrap" p="6px 8px"
|
||||
style={(t) => ({ background: t.colorScheme === 'dark' ? 'rgba(230,180,20,.12)' : '#fff9db', borderRadius: 7 })}>
|
||||
<Text fz={12}>⚠</Text>
|
||||
<Text fz={12} c="yellow.8" style={{ lineHeight: 1.4 }}>Text changed — this edit can’t be applied</Text>
|
||||
</Group>
|
||||
)}
|
||||
|
||||
{parsed.text && !conflict && (
|
||||
<Text fz={12.5} c="dimmed" style={{ lineHeight: 1.45 }}>{parsed.text}</Text>
|
||||
)}
|
||||
|
||||
<Group gap={8} wrap="nowrap" onClick={(e) => e.stopPropagation()}>
|
||||
<Box w={8} h={8} style={(t) => ({ flex: 'none', borderRadius: '50%', background: t.colors[SEV_COLOR[parsed.severity]][6] })} />
|
||||
<Text fz={10} fw={600} tt="uppercase" c="dimmed" style={{ letterSpacing: '.06em', flex: 1, whiteSpace: 'nowrap', overflow: 'hidden', textOverflow: 'ellipsis' }}>
|
||||
{[parsed.category, parsed.severity === 'major' ? 'Major' : parsed.severity === 'critical' ? 'Critical' : null].filter(Boolean).join(' · ')}
|
||||
</Text>
|
||||
|
||||
{c.status === 'applied' ? (
|
||||
<Badge color="green" variant="light" radius="xl" size="sm">✓ Applied</Badge>
|
||||
) : c.status === 'dismissed' ? (
|
||||
<Badge color="gray" variant="light" radius="xl" size="sm">Dismissed</Badge>
|
||||
) : conflict ? (
|
||||
<Button size="compact-sm" variant="default" onClick={() => onNavigateToAnchor(c.id)}>Go to text</Button>
|
||||
) : (
|
||||
<>
|
||||
<Button size="compact-sm" variant="default" color="gray" onClick={dismiss}>Dismiss</Button>
|
||||
<Button size="compact-sm" color="green" loading={busy} onClick={apply}>Apply</Button>
|
||||
</>
|
||||
)}
|
||||
</Group>
|
||||
</Stack>
|
||||
</Box>
|
||||
);
|
||||
}
|
||||
|
||||
/* ──────────────────────── Человеческий тред ──────────────────────── */
|
||||
|
||||
function HumanThread({ c, onResolve, onNavigateToAnchor }: {
|
||||
c: Comment; onResolve?: CommentsPanelProps['onResolve']; onNavigateToAnchor: CommentsPanelProps['onNavigateToAnchor'];
|
||||
}) {
|
||||
const parsed = useMemo(() => parseBody(c.bodyRaw), [c.bodyRaw]);
|
||||
return (
|
||||
<Box p="12px 14px" style={(t) => ({ borderBottom: `1px solid ${t.colorScheme === 'dark' ? t.colors.dark[5] : t.colors.gray[1]}` })}>
|
||||
<Stack gap={9}>
|
||||
<Group gap={8} wrap="nowrap">
|
||||
<Avatar size={26} radius="xl" color="orange">{c.authorName[0]}</Avatar>
|
||||
<Text fz={12.5} fw={600}>{c.authorName}
|
||||
<Text span c="dimmed" fw={400}> · {c.triggeredBy ? `${c.triggeredBy} · ` : ''}{c.createdAtLabel}</Text>
|
||||
</Text>
|
||||
<Box style={{ flex: 1 }} />
|
||||
<Tooltip label="Resolve"><ActionIcon variant="default" size="md" onClick={() => onResolve?.(c.id)}>✓</ActionIcon></Tooltip>
|
||||
<ActionIcon variant="default" size="md">⋯</ActionIcon>
|
||||
</Group>
|
||||
<Text fz={13.5} style={{ lineHeight: 1.5 }}>{parsed.text}</Text>
|
||||
<Group gap={8}>
|
||||
{c.replyCount ? <Text fz={12} c="dimmed">{c.replyCount} replies</Text> : null}
|
||||
<Box style={{ flex: 1 }} />
|
||||
<Button variant="subtle" size="compact-sm">Reply</Button>
|
||||
</Group>
|
||||
</Stack>
|
||||
</Box>
|
||||
);
|
||||
}
|
||||
|
||||
/* ──────────────────── Шапка серии + прогресс ──────────────────── */
|
||||
|
||||
function RunHeader({ runComments, onAcceptAll, progress }: {
|
||||
runComments: Comment[];
|
||||
onAcceptAll: () => void;
|
||||
progress?: { done: number; total: number } | null;
|
||||
}) {
|
||||
const head = runComments[0];
|
||||
const majors = runComments.filter((c) => parseBody(c.bodyRaw).severity !== 'minor').length;
|
||||
const applied = runComments.filter((c) => c.status === 'applied').length;
|
||||
return (
|
||||
<Box>
|
||||
<Group gap={10} wrap="nowrap" p="11px 13px" style={(t) => ({ borderBottom: `1px solid ${t.colorScheme === 'dark' ? t.colors.dark[5] : t.colors.gray[1]}` })}>
|
||||
<Box style={{ position: 'relative', width: 30, height: 30, flex: 'none' }}>
|
||||
<Avatar size={30} radius={8} color="teal">{head.authorName[0]}</Avatar>
|
||||
{head.triggeredBy && (
|
||||
<Avatar size={14} radius="xl" color="gray"
|
||||
style={{ position: 'absolute', right: -4, bottom: -4, border: '2px solid var(--mantine-color-body)' }} />
|
||||
)}
|
||||
</Box>
|
||||
<Stack gap={1} style={{ minWidth: 0 }}>
|
||||
<Text fz={13} fw={600}>{head.authorName}
|
||||
<Text span c="dimmed" fw={400}> · {head.triggeredBy} · {head.createdAtLabel}</Text>
|
||||
</Text>
|
||||
<Text fz={11.5} c="dimmed">
|
||||
{runComments.length} edits · <Text span c="orange.7" fw={600}>{majors} major</Text> · {applied} applied
|
||||
</Text>
|
||||
</Stack>
|
||||
<Box style={{ flex: 1 }} />
|
||||
{!progress && <Button size="compact-sm" color="green" onClick={onAcceptAll}>Accept all</Button>}
|
||||
</Group>
|
||||
{progress && (
|
||||
<Box p="9px 13px 11px" style={(t) => ({ background: t.colorScheme === 'dark' ? 'rgba(47,158,68,.08)' : '#f8fbf9', borderBottom: `1px solid ${t.colorScheme === 'dark' ? t.colors.dark[5] : t.colors.gray[2]}` })}>
|
||||
<Group gap={8} mb={6}>
|
||||
<Text fz={12} fw={600} c="green.7">Applying {progress.done} of {progress.total}…</Text>
|
||||
<Box style={{ flex: 1 }} />
|
||||
<Button variant="subtle" color="gray" size="compact-xs">Stop</Button>
|
||||
</Group>
|
||||
<Progress value={(progress.done / progress.total) * 100} color="green" size="sm" radius="xl" />
|
||||
</Box>
|
||||
)}
|
||||
</Box>
|
||||
);
|
||||
}
|
||||
|
||||
/* ──────────────────────────── Панель ──────────────────────────── */
|
||||
|
||||
/** Роль-автор для фильтра: у агентов — имя роли, у людей — «Пользователь». */
|
||||
function roleOf(c: Comment): string {
|
||||
return c.authorKind === 'human' ? 'Пользователь' : c.authorName;
|
||||
}
|
||||
|
||||
export function CommentsPanel(props: CommentsPanelProps) {
|
||||
const { comments, onApply, onDismiss, onResolve, onNavigateToAnchor, onClose, dismissUndoMs } = props;
|
||||
const [tab, setTab] = useState<'open' | 'resolved'>('open');
|
||||
const [roleFilter, setRoleFilter] = useState<string | null>(null);
|
||||
const [progress, setProgress] = useState<Record<string, { done: number; total: number }>>({});
|
||||
|
||||
const open = comments.filter((c) => c.status === 'pending' || c.status === 'conflict');
|
||||
const resolved = comments.filter((c) => c.status === 'applied' || c.status === 'dismissed');
|
||||
const list = tab === 'open' ? open : resolved;
|
||||
|
||||
const filtered = list.filter((c) => !roleFilter || roleOf(c) === roleFilter);
|
||||
|
||||
// Роли и счётчики для чипов-фильтров (только роли: Корректор / Фактчекер / Пользователь …).
|
||||
const roles = useMemo(() => {
|
||||
const order: string[] = [];
|
||||
const count: Record<string, number> = {};
|
||||
for (const c of list) {
|
||||
const r = roleOf(c);
|
||||
if (!(r in count)) { count[r] = 0; order.push(r); }
|
||||
count[r]++;
|
||||
}
|
||||
return order.map((r) => ({ role: r, count: count[r] }));
|
||||
}, [list]);
|
||||
|
||||
// Группировка агентских правок по runId; человеческие треды — по одному.
|
||||
const groups = useMemo(() => {
|
||||
const map = new Map<string, Comment[]>();
|
||||
const singles: Comment[] = [];
|
||||
for (const c of filtered) {
|
||||
if (c.authorKind === 'agent' && c.runId && c.edit) {
|
||||
if (!map.has(c.runId)) map.set(c.runId, []);
|
||||
map.get(c.runId)!.push(c);
|
||||
} else singles.push(c);
|
||||
}
|
||||
return { runs: [...map.entries()], singles };
|
||||
}, [filtered]);
|
||||
|
||||
// Пакетное "Accept all" — по одной на фронте, обновляем прогресс.
|
||||
const acceptAll = useCallback(async (runId: string, items: Comment[]) => {
|
||||
const minor = items.filter((c) => parseBody(c.bodyRaw).severity === 'minor' && c.status === 'pending');
|
||||
for (let i = 0; i < minor.length; i++) {
|
||||
setProgress((p) => ({ ...p, [runId]: { done: i, total: minor.length } }));
|
||||
try { await onApply(minor[i].id); } catch { /* конфликт — пропускаем, копим для сводки */ }
|
||||
}
|
||||
setProgress((p) => { const n = { ...p }; delete n[runId]; return n; });
|
||||
notifications.show({ color: 'green', message: `${minor.length} applied` });
|
||||
}, [onApply]);
|
||||
|
||||
return (
|
||||
<Stack gap={0} h="100%" style={{ width: 380, maxWidth: '100%', borderLeft: '1px solid var(--mantine-color-default-border)' }}>
|
||||
{/* Вкладки — статусная ось. Open/Resolved сохранены. */}
|
||||
<Group gap={4} p="10px 14px" style={(t) => ({ borderBottom: `1px solid ${t.colorScheme === 'dark' ? t.colors.dark[5] : t.colors.gray[2]}` })}>
|
||||
<Tabs value={tab} onChange={(v) => setTab(v as 'open' | 'resolved')} variant="pills">
|
||||
<Tabs.List>
|
||||
<Tabs.Tab value="open" rightSection={<Badge size="sm" variant="light" color="blue">{open.length}</Badge>}>Open</Tabs.Tab>
|
||||
<Tabs.Tab value="resolved" rightSection={<Badge size="sm" variant="light" color="gray">{resolved.length}</Badge>}>Resolved</Tabs.Tab>
|
||||
</Tabs.List>
|
||||
</Tabs>
|
||||
<Box style={{ flex: 1 }} />
|
||||
{onClose && <ActionIcon variant="subtle" color="gray" onClick={onClose}>✕</ActionIcon>}
|
||||
</Group>
|
||||
|
||||
{/* Фильтры-чипы: только по ролям (Корректор / Фактчекер / Пользователь). */}
|
||||
{tab === 'open' && roles.length > 1 && (
|
||||
<ScrollArea type="never" p="8px 14px" style={(t) => ({ borderBottom: `1px solid ${t.colorScheme === 'dark' ? t.colors.dark[5] : t.colors.gray[1]}` })}>
|
||||
<Group gap={6} wrap="nowrap">
|
||||
<FilterChip active={!roleFilter} onClick={() => setRoleFilter(null)} label={`All ${list.length}`} solid />
|
||||
{roles.map(({ role, count }) => (
|
||||
<FilterChip key={role} active={roleFilter === role} onClick={() => setRoleFilter(roleFilter === role ? null : role)} label={`${role} ${count}`} />
|
||||
))}
|
||||
</Group>
|
||||
</ScrollArea>
|
||||
)}
|
||||
|
||||
{/* Лента */}
|
||||
<ScrollArea style={{ flex: 1 }} bg="var(--mantine-color-default-hover)">
|
||||
{filtered.length === 0 ? (
|
||||
<EmptyState tab={tab} />
|
||||
) : (
|
||||
<>
|
||||
{groups.runs.map(([runId, items]) => (
|
||||
<Box key={runId} m="6px 10px" bg="var(--mantine-color-body)" style={{ border: '1px solid var(--mantine-color-default-border)', borderRadius: 10, overflow: 'hidden' }}>
|
||||
<RunHeader runComments={items} progress={progress[runId] ?? null} onAcceptAll={() => acceptAll(runId, items)} />
|
||||
{items.map((c) => (
|
||||
<EditCard key={c.id} c={c} onApply={onApply} onDismiss={onDismiss} onNavigateToAnchor={onNavigateToAnchor} dismissUndoMs={dismissUndoMs} />
|
||||
))}
|
||||
</Box>
|
||||
))}
|
||||
{groups.singles.map((c) => (
|
||||
<Box key={c.id} m="6px 10px" bg="var(--mantine-color-body)" style={{ border: '1px solid var(--mantine-color-default-border)', borderRadius: 10, overflow: 'hidden' }}>
|
||||
{c.edit
|
||||
? <EditCard c={c} onApply={onApply} onDismiss={onDismiss} onNavigateToAnchor={onNavigateToAnchor} dismissUndoMs={dismissUndoMs} />
|
||||
: <HumanThread c={c} onResolve={onResolve} onNavigateToAnchor={onNavigateToAnchor} />}
|
||||
</Box>
|
||||
))}
|
||||
</>
|
||||
)}
|
||||
</ScrollArea>
|
||||
</Stack>
|
||||
);
|
||||
}
|
||||
|
||||
/* ─────────────────────── Мелкие компоненты ─────────────────────── */
|
||||
|
||||
function FilterChip({ label, active, onClick, dot, solid }: {
|
||||
label: string; active: boolean; onClick: () => void; dot?: string; solid?: boolean;
|
||||
}) {
|
||||
return (
|
||||
<Button
|
||||
onClick={onClick}
|
||||
size="compact-sm"
|
||||
radius="xl"
|
||||
variant={active ? 'filled' : 'default'}
|
||||
color={active ? (solid ? 'dark' : 'blue') : 'gray'}
|
||||
leftSection={dot ? <Box w={7} h={7} style={(t) => ({ borderRadius: '50%', background: t.colors[dot][6] })} /> : undefined}
|
||||
styles={{ root: { flex: 'none' }, label: { fontWeight: 600, fontSize: 12 } }}
|
||||
>
|
||||
{label}
|
||||
</Button>
|
||||
);
|
||||
}
|
||||
|
||||
function EmptyState({ tab }: { tab: 'open' | 'resolved' }) {
|
||||
const isOpen = tab === 'open';
|
||||
return (
|
||||
<Stack align="center" gap={6} p="40px 20px">
|
||||
<Avatar size={40} radius="xl" color={isOpen ? 'green' : 'gray'}>{isOpen ? '✓' : '◌'}</Avatar>
|
||||
<Text fw={600} fz={13}>{isOpen ? 'All caught up' : 'Nothing here yet'}</Text>
|
||||
<Text fz={12} c="dimmed" ta="center" style={{ lineHeight: 1.45 }}>
|
||||
{isOpen ? 'No edits waiting on you.' : 'Applied and closed items will appear here.'}
|
||||
</Text>
|
||||
</Stack>
|
||||
);
|
||||
}
|
||||
|
||||
export default CommentsPanel;
|
||||
@@ -346,6 +346,9 @@ roles:
|
||||
□ Working sections ("Log", "Open Questions", "Revision") are moved to an
|
||||
appendix at the end of the document or clearly separated from the report
|
||||
body.
|
||||
□ Once the report is complete, call save_page_version to pin the finished
|
||||
document as a restorable named version (a checkpoint the reader can
|
||||
return to). A repeat call after no further edits is a harmless no-op.
|
||||
Be honest about gaps. If you couldn't find something, say so — don't disguise
|
||||
a guess as a fact.
|
||||
autoStart: false
|
||||
@@ -442,6 +445,7 @@ roles:
|
||||
- **The language of the notes = the main language of the call.** Technical terms — in their canonical spelling (usually Latin).
|
||||
- **Don't evaluate the participants** and don't comment on the quality of the discussion.
|
||||
- The output is the notes only, with no preambles or meta-comments, apart from targeted uncertainty marks.
|
||||
- **Pin the result.** Once the notes are complete on the page, call save_page_version to save them as a restorable named version (a checkpoint). Calling it again after no further edits is a harmless no-op.
|
||||
|
||||
## Style example (excerpt)
|
||||
|
||||
|
||||
@@ -345,6 +345,9 @@ roles:
|
||||
□ Working sections («Журнал», «Открытые вопросы», «Ревизия») are moved to
|
||||
an appendix at the end of the document or clearly separated from the
|
||||
report body.
|
||||
□ Once the report is complete, call save_page_version to pin the finished
|
||||
document as a restorable named version (a checkpoint the reader can
|
||||
return to). A repeat call after no further edits is a harmless no-op.
|
||||
Be honest about gaps. If you couldn't find something, say so — don't disguise
|
||||
a guess as a fact.
|
||||
autoStart: false
|
||||
@@ -441,6 +444,7 @@ roles:
|
||||
- **Язык конспекта = основной язык созвона.** Технические термины — в каноническом написании (обычно латиницей).
|
||||
- **Не оценивай участников** и не комментируй качество обсуждения.
|
||||
- На выходе — только конспект, без преамбул и мета-комментариев, кроме точечных пометок неуверенности.
|
||||
- **Зафиксируй результат.** Когда конспект готов на странице, вызови save_page_version, чтобы сохранить его как именованную версию (точку восстановления). Повторный вызов без новых правок — безвредный no-op.
|
||||
|
||||
## Пример стиля (фрагмент)
|
||||
|
||||
|
||||
@@ -33,6 +33,6 @@ bundles:
|
||||
- en
|
||||
roles:
|
||||
- slug: researcher
|
||||
version: 9
|
||||
version: 10
|
||||
- slug: call-summarizer
|
||||
version: 1
|
||||
version: 2
|
||||
|
||||
@@ -48,6 +48,7 @@
|
||||
"Create page": "Create page",
|
||||
"Create space": "Create space",
|
||||
"Create workspace": "Create workspace",
|
||||
"Critical": "Critical",
|
||||
"Current password": "Current password",
|
||||
"Dark": "Dark",
|
||||
"Date": "Date",
|
||||
@@ -125,6 +126,7 @@
|
||||
"Link copied": "Link copied",
|
||||
"Login": "Login",
|
||||
"Logout": "Logout",
|
||||
"Major": "Major",
|
||||
"Manage Group": "Manage Group",
|
||||
"Manage members": "Manage members",
|
||||
"member": "member",
|
||||
@@ -453,6 +455,8 @@
|
||||
"{{count}} Columns": "{{count}} Columns",
|
||||
"{{count}} command available_one": "1 command available",
|
||||
"{{count}} command available_other": "{{count}} commands available",
|
||||
"{{count}} edits": "{{count}} edits",
|
||||
"{{count}} major": "{{count}} major",
|
||||
"{{count}} result available_one": "1 result available",
|
||||
"{{count}} result available_other": "{{count}} results available",
|
||||
"{{count}} result found_one": "{{count}} result found",
|
||||
|
||||
@@ -48,6 +48,7 @@
|
||||
"Create page": "Создать страницу",
|
||||
"Create space": "Создать пространство",
|
||||
"Create workspace": "Создать рабочую область",
|
||||
"Critical": "Критично",
|
||||
"Current password": "Текущий пароль",
|
||||
"Dark": "Темная",
|
||||
"Date": "Дата",
|
||||
@@ -127,6 +128,7 @@
|
||||
"Link copied": "Ссылка скопирована",
|
||||
"Login": "Войти",
|
||||
"Logout": "Выйти",
|
||||
"Major": "Существенно",
|
||||
"Manage Group": "Управление группой",
|
||||
"Manage members": "Управление участниками",
|
||||
"member": "участник",
|
||||
@@ -461,6 +463,8 @@
|
||||
"{{count}} Columns": "{count, plural, one{# столбец} few{# столбца} many{# столбцов} other{# столбца}}",
|
||||
"{{count}} command available_one": "Доступна 1 команда",
|
||||
"{{count}} command available_other": "Доступно {{count}} команд",
|
||||
"{{count}} edits": "{count, plural, one{# правка} few{# правки} many{# правок} other{# правки}}",
|
||||
"{{count}} major": "{count, plural, one{# существенная} few{# существенных} many{# существенных} other{# существенных}}",
|
||||
"{{count}} result available_one": "Доступен 1 результат",
|
||||
"{{count}} result available_other": "Доступно {{count}} результатов",
|
||||
"{{count}} result found_one": "Найден {{count}} результат",
|
||||
|
||||
@@ -0,0 +1,284 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render, screen, fireEvent } from "@testing-library/react";
|
||||
import { MantineProvider } from "@mantine/core";
|
||||
import { createInstance } from "i18next";
|
||||
import { initReactI18next, I18nextProvider } from "react-i18next";
|
||||
import { IComment } from "@/features/comment/types/comment.types";
|
||||
|
||||
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
|
||||
|
||||
// The suggestion mutations reach react-query/network — stub them so the card
|
||||
// renders in isolation. We assert the Apply/Dismiss gating and that the click
|
||||
// hands the {commentId, pageId} pair to the existing mutation unchanged.
|
||||
const applyMutateAsync = vi.fn();
|
||||
const dismissMutateAsync = vi.fn();
|
||||
vi.mock("@/features/comment/queries/comment-query", () => ({
|
||||
useApplySuggestionMutation: () => ({
|
||||
mutateAsync: applyMutateAsync,
|
||||
isPending: false,
|
||||
}),
|
||||
useDismissSuggestionMutation: () => ({
|
||||
mutateAsync: dismissMutateAsync,
|
||||
isPending: false,
|
||||
}),
|
||||
}));
|
||||
|
||||
// CommentContentView -> mention-view -> page-query/share-query pull in the app
|
||||
// entry (createRoot) as a side effect; stub the queries so the card renders in
|
||||
// isolation.
|
||||
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
usePageQuery: () => ({ data: undefined, isLoading: false, isError: false }),
|
||||
}));
|
||||
vi.mock("@/features/share/queries/share-query.ts", () => ({
|
||||
useSharePageQuery: () => ({ data: undefined }),
|
||||
}));
|
||||
// space-query.ts -> main.tsx (createRoot) is a module side effect reached via the
|
||||
// mention view; stub it so importing the card is side-effect free.
|
||||
vi.mock("@/features/space/queries/space-query.ts", () => ({
|
||||
useSpaceQuery: () => ({ data: undefined }),
|
||||
useGetSpaceBySlugQuery: () => ({ data: undefined }),
|
||||
}));
|
||||
|
||||
import AgentEditCard, { RunHeader } from "./agent-edit-card";
|
||||
|
||||
const body = (text: string) =>
|
||||
JSON.stringify({
|
||||
type: "doc",
|
||||
content: [{ type: "paragraph", content: [{ type: "text", text }] }],
|
||||
});
|
||||
|
||||
const edit = (over?: Partial<IComment>): IComment =>
|
||||
({
|
||||
id: "c-1",
|
||||
content: body("[Существенно] tighten the wording"),
|
||||
creatorId: "user-1",
|
||||
pageId: "page-1",
|
||||
workspaceId: "ws-1",
|
||||
createdAt: new Date(),
|
||||
createdSource: "agent",
|
||||
aiChatId: "chat-1",
|
||||
agent: { name: "Corrector", emoji: "✏️", avatarUrl: null },
|
||||
launcher: { name: "Alice", avatarUrl: null },
|
||||
creator: { id: "user-1", name: "Corrector", avatarUrl: null } as any,
|
||||
selection: "old wording here",
|
||||
suggestedText: "new wording here",
|
||||
...over,
|
||||
}) as IComment;
|
||||
|
||||
function renderCard(
|
||||
comment: IComment,
|
||||
canEdit = true,
|
||||
canComment = true,
|
||||
userSpaceRole?: string,
|
||||
) {
|
||||
return render(
|
||||
<MantineProvider>
|
||||
<AgentEditCard
|
||||
comment={comment}
|
||||
canComment={canComment}
|
||||
canEdit={canEdit}
|
||||
userSpaceRole={userSpaceRole}
|
||||
/>
|
||||
</MantineProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("AgentEditCard — suggested edit diff + Apply (#315)", () => {
|
||||
it("renders the было→стало diff and an Apply button when canEdit, not applied/resolved", () => {
|
||||
const { container } = renderCard(edit(), true);
|
||||
// Both diff lines are present (old struck-through, new added).
|
||||
expect(container.textContent).toContain("old wording here");
|
||||
expect(container.textContent).toContain("new wording here");
|
||||
// Diff line signs (aria-hidden) present for a replacement.
|
||||
expect(container.textContent).toContain("−");
|
||||
expect(container.textContent).toContain("+");
|
||||
expect(screen.getByRole("button", { name: "Apply" })).toBeDefined();
|
||||
});
|
||||
|
||||
it("hides Apply when canEdit is false (still shows the diff)", () => {
|
||||
const { container } = renderCard(edit(), false);
|
||||
expect(container.textContent).toContain("new wording here");
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
});
|
||||
|
||||
it("hides Apply once the thread is resolved", () => {
|
||||
renderCard(edit({ resolvedAt: new Date() }), true);
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
});
|
||||
|
||||
it("hides Apply once suggestionAppliedAt is set", () => {
|
||||
renderCard(edit({ suggestionAppliedAt: new Date() }), true);
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
});
|
||||
|
||||
it("shows the Applied badge for an applied suggestion, not a pending one (F1)", () => {
|
||||
// Pending: no Applied badge.
|
||||
const { unmount } = renderCard(edit(), true);
|
||||
expect(screen.queryByText("Applied")).toBeNull();
|
||||
unmount();
|
||||
// Applied (kept alive by replies -> resolved, #329): the badge is restored.
|
||||
renderCard(edit({ suggestionAppliedAt: new Date() }), true);
|
||||
expect(screen.getByText("Applied")).toBeDefined();
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
expect(screen.queryByRole("button", { name: "Dismiss" })).toBeNull();
|
||||
});
|
||||
|
||||
it("calls the apply mutation with {commentId, pageId} on click", () => {
|
||||
applyMutateAsync.mockClear();
|
||||
renderCard(edit(), true);
|
||||
fireEvent.click(screen.getByRole("button", { name: "Apply" }));
|
||||
expect(applyMutateAsync).toHaveBeenCalledWith({
|
||||
commentId: "c-1",
|
||||
pageId: "page-1",
|
||||
});
|
||||
});
|
||||
|
||||
it("a pure insertion (empty selection) hides the removed line", () => {
|
||||
const { container } = renderCard(
|
||||
edit({ selection: "", suggestedText: "added text" }),
|
||||
true,
|
||||
);
|
||||
// No "−" del sign — nothing was removed.
|
||||
expect(container.textContent).not.toContain("−");
|
||||
expect(container.textContent).toContain("+");
|
||||
});
|
||||
|
||||
it("a pure deletion (empty suggestedText) hides the added line", () => {
|
||||
const { container } = renderCard(
|
||||
edit({ selection: "removed text", suggestedText: "" }),
|
||||
true,
|
||||
);
|
||||
expect(container.textContent).not.toContain("+");
|
||||
expect(container.textContent).toContain("−");
|
||||
});
|
||||
});
|
||||
|
||||
describe("AgentEditCard — Dismiss gate (#329/#338)", () => {
|
||||
it("shows Dismiss alongside Apply for an admin who can edit/comment", () => {
|
||||
renderCard(edit(), true, true, "admin");
|
||||
expect(screen.getByRole("button", { name: "Apply" })).toBeDefined();
|
||||
expect(screen.getByRole("button", { name: "Dismiss" })).toBeDefined();
|
||||
});
|
||||
|
||||
it("shows Dismiss but NOT Apply for an admin commenter who cannot edit", () => {
|
||||
renderCard(edit(), false, true, "admin");
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
expect(screen.getByRole("button", { name: "Dismiss" })).toBeDefined();
|
||||
});
|
||||
|
||||
it("hides Dismiss for a non-owner non-admin (mirrors server 403, #338 F5)", () => {
|
||||
renderCard(edit(), false, true, "member");
|
||||
expect(screen.queryByRole("button", { name: "Dismiss" })).toBeNull();
|
||||
});
|
||||
|
||||
it("hides Dismiss when the viewer cannot comment", () => {
|
||||
renderCard(edit(), false, false, "admin");
|
||||
expect(screen.queryByRole("button", { name: "Dismiss" })).toBeNull();
|
||||
});
|
||||
|
||||
it("calls the dismiss mutation with {commentId, pageId} on click", () => {
|
||||
dismissMutateAsync.mockClear();
|
||||
renderCard(edit(), true, true, "admin");
|
||||
fireEvent.click(screen.getByRole("button", { name: "Dismiss" }));
|
||||
expect(dismissMutateAsync).toHaveBeenCalledWith({
|
||||
commentId: "c-1",
|
||||
pageId: "page-1",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("AgentEditCard — provenance", () => {
|
||||
it("renders the agent avatar stack (provenance) for the edit author (#300)", () => {
|
||||
renderCard(edit(), true);
|
||||
// The agent role name is shown by the provenance stack.
|
||||
expect(screen.getAllByText("Corrector").length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
// The owner-or-admin gate uses the currentUser atom; clear localStorage so a
|
||||
// previous test's seed never leaks into the non-owner assertions above.
|
||||
beforeEach(() => localStorage.clear());
|
||||
afterEach(() => localStorage.clear());
|
||||
});
|
||||
|
||||
// The RunHeader's "N edits · M major" is built with interpolated t() keys; the
|
||||
// default (uninitialized) react-i18next t returns the key verbatim WITHOUT
|
||||
// interpolating, so a numeric assertion needs a real, initialised i18n instance.
|
||||
// This isolated instance carries just the two count keys with escapeValue off so
|
||||
// "{{count}}" is substituted and the rendered numbers are assertable.
|
||||
const headerI18n = createInstance();
|
||||
headerI18n.use(initReactI18next).init({
|
||||
lng: "en",
|
||||
fallbackLng: "en",
|
||||
resources: {
|
||||
en: {
|
||||
translation: {
|
||||
"{{count}} edits": "{{count}} edits",
|
||||
"{{count}} major": "{{count}} major",
|
||||
},
|
||||
},
|
||||
},
|
||||
interpolation: { escapeValue: false },
|
||||
});
|
||||
|
||||
const runComment = (id: string, tag: string): IComment =>
|
||||
edit({
|
||||
id,
|
||||
content: body(`${tag} some rationale`),
|
||||
});
|
||||
|
||||
function renderRunHeader(comments: IComment[]) {
|
||||
return render(
|
||||
<I18nextProvider i18n={headerI18n}>
|
||||
<MantineProvider>
|
||||
<RunHeader comments={comments} />
|
||||
</MantineProvider>
|
||||
</I18nextProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("RunHeader — agent-run series header (F3)", () => {
|
||||
// 5 edits: 2 critical + 1 major + 1 minor + 1 unknown(verdict) => 3 "major".
|
||||
const series = () => [
|
||||
runComment("e1", "[Критично]"),
|
||||
runComment("e2", "[Критично]"),
|
||||
runComment("e3", "[Существенно]"),
|
||||
runComment("e4", "[Незначительно]"),
|
||||
runComment("e5", "[Неверно]"),
|
||||
];
|
||||
|
||||
it("shows the total edit count", () => {
|
||||
renderRunHeader(series());
|
||||
expect(screen.getByText(/5 edits/)).toBeDefined();
|
||||
});
|
||||
|
||||
it("counts ONLY major+critical as major (not minor/unknown)", () => {
|
||||
renderRunHeader(series());
|
||||
// 2 critical + 1 major = 3; minor and the [Неверно] verdict are excluded.
|
||||
expect(screen.getByText(/3 major/)).toBeDefined();
|
||||
// Non-vacuous: the wrong tally (counting all 5, or 4) must NOT appear.
|
||||
expect(screen.queryByText(/5 major/)).toBeNull();
|
||||
expect(screen.queryByText(/4 major/)).toBeNull();
|
||||
});
|
||||
|
||||
it("omits the major segment entirely when there are no significant edits", () => {
|
||||
renderRunHeader([
|
||||
runComment("e1", "[Незначительно]"),
|
||||
runComment("e2", "[Неверно]"),
|
||||
]);
|
||||
expect(screen.getByText(/2 edits/)).toBeDefined();
|
||||
expect(screen.queryByText(/major/)).toBeNull();
|
||||
});
|
||||
|
||||
it("renders the provenance line (agent role name)", () => {
|
||||
renderRunHeader(series());
|
||||
expect(screen.getAllByText("Corrector").length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("renders NO 'Accept all' control (rejected by product)", () => {
|
||||
renderRunHeader(series());
|
||||
expect(screen.queryByText(/accept all/i)).toBeNull();
|
||||
expect(
|
||||
screen.queryByRole("button", { name: /accept all/i }),
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,418 @@
|
||||
import React, { useMemo } from "react";
|
||||
import {
|
||||
Badge,
|
||||
Box,
|
||||
Button,
|
||||
Group,
|
||||
Stack,
|
||||
Text,
|
||||
useMantineColorScheme,
|
||||
useMantineTheme,
|
||||
} from "@mantine/core";
|
||||
import { useAtom } from "jotai";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { AgentAvatarStack } from "@/components/ui/agent-avatar-stack.tsx";
|
||||
import CommentContentView from "@/features/comment/components/comment-content-view";
|
||||
import { IComment } from "@/features/comment/types/comment.types";
|
||||
import {
|
||||
canShowApply,
|
||||
canShowDismiss,
|
||||
computeSuggestionDiff,
|
||||
Segment,
|
||||
} from "@/features/comment/utils/suggestion";
|
||||
import { commentContentToText } from "@/features/comment/utils/comment-content-to-text";
|
||||
import {
|
||||
isSignificant,
|
||||
parseSeverity,
|
||||
Severity,
|
||||
} from "@/features/comment/utils/severity";
|
||||
import {
|
||||
useApplySuggestionMutation,
|
||||
useDismissSuggestionMutation,
|
||||
} from "@/features/comment/queries/comment-query";
|
||||
import { currentUserAtom } from "@/features/user/atoms/current-user-atom.ts";
|
||||
import { useTimeAgo } from "@/hooks/use-time-ago";
|
||||
|
||||
// Scroll the document to this comment's inline mark and flash it — the same
|
||||
// anchor navigation the old panel used (handleCommentClick). Used both by a card
|
||||
// click and by an explicit "Go to text" affordance.
|
||||
function scrollToCommentMark(commentId: string) {
|
||||
const el = document.querySelector(
|
||||
`.comment-mark[data-comment-id="${commentId}"]`,
|
||||
);
|
||||
if (el) {
|
||||
el.scrollIntoView({ behavior: "smooth", block: "center" });
|
||||
el.classList.add("comment-highlight");
|
||||
setTimeout(() => el.classList.remove("comment-highlight"), 3000);
|
||||
}
|
||||
}
|
||||
|
||||
// Make otherwise-invisible edited whitespace legible inside a highlighted diff
|
||||
// fragment (a pure-space insertion/deletion would otherwise look like nothing
|
||||
// changed).
|
||||
function visibleWhitespace(s: string): string {
|
||||
return s.replace(/ /g, "␣").replace(/\t/g, "⇥");
|
||||
}
|
||||
|
||||
// One line of the intraline diff. `kind==="del"` is the old ("было") line drawn
|
||||
// red with a strike-through; `kind==="ins"` is the new ("стало") line drawn
|
||||
// green. Only the `changed` segments carry the emphasised mark background — the
|
||||
// common segments read as plain context (git/GitHub-style, #331).
|
||||
function DiffLine({
|
||||
segments,
|
||||
kind,
|
||||
}: {
|
||||
segments: Segment[];
|
||||
kind: "del" | "ins";
|
||||
}) {
|
||||
const theme = useMantineTheme();
|
||||
const { colorScheme } = useMantineColorScheme();
|
||||
const dark = colorScheme === "dark";
|
||||
const isDel = kind === "del";
|
||||
const sign = isDel ? "−" : "+";
|
||||
const signColor = isDel
|
||||
? theme.colors.red[dark ? 5 : 6]
|
||||
: theme.colors.green[dark ? 5 : 6];
|
||||
const baseColor = isDel
|
||||
? dark
|
||||
? theme.colors.gray[5]
|
||||
: theme.colors.gray[6]
|
||||
: dark
|
||||
? theme.colors.gray[1]
|
||||
: theme.colors.dark[9];
|
||||
const markBg = isDel
|
||||
? dark
|
||||
? "rgba(224,49,49,.22)"
|
||||
: "#ffe3e3"
|
||||
: dark
|
||||
? "rgba(47,158,68,.22)"
|
||||
: "#d3f9d8";
|
||||
const markFg = isDel
|
||||
? dark
|
||||
? theme.colors.red[3]
|
||||
: theme.colors.red[8]
|
||||
: dark
|
||||
? theme.colors.green[3]
|
||||
: theme.colors.green[9];
|
||||
|
||||
return (
|
||||
<Group gap={7} wrap="nowrap" align="flex-start">
|
||||
<Text
|
||||
ff="monospace"
|
||||
fw={600}
|
||||
fz={12}
|
||||
c={signColor}
|
||||
w={11}
|
||||
ta="center"
|
||||
aria-hidden
|
||||
style={{ flex: "none", lineHeight: 1.5 }}
|
||||
>
|
||||
{sign}
|
||||
</Text>
|
||||
<Text fz={13.5} c={baseColor} style={{ lineHeight: 1.45 }}>
|
||||
{segments.map((seg, i) =>
|
||||
seg.changed ? (
|
||||
<Box
|
||||
key={i}
|
||||
component="mark"
|
||||
px={3}
|
||||
fw={600}
|
||||
style={{
|
||||
background: markBg,
|
||||
color: markFg,
|
||||
borderRadius: 3,
|
||||
textDecoration: isDel ? "line-through" : "none",
|
||||
}}
|
||||
>
|
||||
{visibleWhitespace(seg.text)}
|
||||
</Box>
|
||||
) : (
|
||||
<Box
|
||||
key={i}
|
||||
component="span"
|
||||
style={{ textDecoration: isDel ? "line-through" : "none" }}
|
||||
>
|
||||
{seg.text}
|
||||
</Box>
|
||||
),
|
||||
)}
|
||||
</Text>
|
||||
</Group>
|
||||
);
|
||||
}
|
||||
|
||||
// The "было → стало" block. A pure insertion (empty `before`) hides the old
|
||||
// line; a pure deletion (empty `after`) hides the new line.
|
||||
function DiffBlock({ before, after }: { before: Segment[]; after: Segment[] }) {
|
||||
const pureInsert = before.length === 0;
|
||||
const pureDelete = after.length === 0;
|
||||
return (
|
||||
<Stack gap={1}>
|
||||
{!pureInsert && <DiffLine segments={before} kind="del" />}
|
||||
{!pureDelete && <DiffLine segments={after} kind="ins" />}
|
||||
</Stack>
|
||||
);
|
||||
}
|
||||
|
||||
const SEV_DOT: Record<Severity, { color: string; shade: number }> = {
|
||||
critical: { color: "red", shade: 6 },
|
||||
major: { color: "orange", shade: 6 },
|
||||
minor: { color: "gray", shade: 5 },
|
||||
// A neutral, deliberately faint dot — NOT the minor grey — so an untagged or
|
||||
// verdict-only edit never reads as a graded severity.
|
||||
unknown: { color: "gray", shade: 3 },
|
||||
};
|
||||
|
||||
function SeverityDot({ severity }: { severity: Severity }) {
|
||||
return (
|
||||
<Box
|
||||
w={8}
|
||||
h={8}
|
||||
aria-hidden
|
||||
style={(t) => ({
|
||||
flex: "none",
|
||||
borderRadius: "50%",
|
||||
background: t.colors[SEV_DOT[severity].color][SEV_DOT[severity].shade],
|
||||
})}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
interface AgentEditCardProps {
|
||||
comment: IComment;
|
||||
canComment: boolean;
|
||||
canEdit?: boolean;
|
||||
userSpaceRole?: string;
|
||||
// Whether to render the per-card agent avatar + timestamp. A card inside a
|
||||
// collapsed run omits it (the RunHeader carries the one provenance line);
|
||||
// a standalone card shows it (#300 provenance must be visible on the card).
|
||||
showProvenance?: boolean;
|
||||
}
|
||||
|
||||
// Type A — the diff-first agent suggested-edit card. It carries NO TipTap editor
|
||||
// (a perf win vs. the old row, #340); the body renders through the static
|
||||
// CommentContentView. Apply/Dismiss reuse the existing mutations and gates
|
||||
// verbatim — the reskin changes presentation only, not the 409/404/400 toast
|
||||
// semantics or the authz gating.
|
||||
function AgentEditCard({
|
||||
comment,
|
||||
canComment,
|
||||
canEdit,
|
||||
userSpaceRole,
|
||||
showProvenance = true,
|
||||
}: AgentEditCardProps) {
|
||||
const { t } = useTranslation();
|
||||
const [currentUser] = useAtom(currentUserAtom);
|
||||
const applySuggestionMutation = useApplySuggestionMutation();
|
||||
const dismissSuggestionMutation = useDismissSuggestionMutation();
|
||||
const createdAtAgo = useTimeAgo(comment.createdAt);
|
||||
|
||||
const diff = useMemo(
|
||||
() =>
|
||||
computeSuggestionDiff(comment.selection ?? "", comment.suggestedText ?? ""),
|
||||
[comment.selection, comment.suggestedText],
|
||||
);
|
||||
|
||||
const severity = useMemo(
|
||||
() => parseSeverity(commentContentToText(comment.content)),
|
||||
[comment.content],
|
||||
);
|
||||
|
||||
// Owner-or-space-admin gate (#338), mirrored from the old row so we never
|
||||
// render a Dismiss the server would 403.
|
||||
const isOwnerOrAdmin =
|
||||
currentUser?.user?.id === comment.creatorId || userSpaceRole === "admin";
|
||||
|
||||
const isApplied = comment.suggestionAppliedAt != null;
|
||||
const showApply = canShowApply(comment, canEdit);
|
||||
const showDismiss = canShowDismiss(comment, canComment, isOwnerOrAdmin);
|
||||
const pending =
|
||||
applySuggestionMutation.isPending || dismissSuggestionMutation.isPending;
|
||||
|
||||
const handleApply = async () => {
|
||||
try {
|
||||
await applySuggestionMutation.mutateAsync({
|
||||
commentId: comment.id,
|
||||
pageId: comment.pageId,
|
||||
});
|
||||
} catch (error) {
|
||||
// Errors (incl. 409 "text changed") surface via the mutation's onError.
|
||||
console.error("Failed to apply suggestion:", error);
|
||||
}
|
||||
};
|
||||
|
||||
const handleDismiss = async () => {
|
||||
try {
|
||||
await dismissSuggestionMutation.mutateAsync({
|
||||
commentId: comment.id,
|
||||
pageId: comment.pageId,
|
||||
});
|
||||
} catch (error) {
|
||||
// Idempotent 404 is reconciled to success in the mutation's onError.
|
||||
console.error("Failed to dismiss suggestion:", error);
|
||||
}
|
||||
};
|
||||
|
||||
const sevLabel =
|
||||
severity === "critical"
|
||||
? t("Critical")
|
||||
: severity === "major"
|
||||
? t("Major")
|
||||
: null;
|
||||
|
||||
return (
|
||||
<Box
|
||||
p="10px 12px"
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
aria-label={t("Jump to comment selection")}
|
||||
onClick={() => scrollToCommentMark(comment.id)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter" || e.key === " ") {
|
||||
e.preventDefault();
|
||||
scrollToCommentMark(comment.id);
|
||||
}
|
||||
}}
|
||||
style={{ cursor: "pointer" }}
|
||||
>
|
||||
<Stack gap={8}>
|
||||
{showProvenance && comment.agent && (
|
||||
<Group
|
||||
gap={8}
|
||||
wrap="nowrap"
|
||||
justify="space-between"
|
||||
onClick={(e) => e.stopPropagation()}
|
||||
>
|
||||
<AgentAvatarStack
|
||||
agent={comment.agent}
|
||||
launcher={comment.launcher}
|
||||
aiChatId={comment.aiChatId}
|
||||
/>
|
||||
<Text size="xs" c="dimmed" style={{ flex: "none" }}>
|
||||
{createdAtAgo}
|
||||
</Text>
|
||||
</Group>
|
||||
)}
|
||||
|
||||
<DiffBlock before={diff.old} after={diff.new} />
|
||||
|
||||
{/* Agent rationale — rendered through the static ProseMirror view, not
|
||||
restructured into a flat string. */}
|
||||
<Box fz="xs" c="dimmed">
|
||||
<CommentContentView content={comment.content} />
|
||||
</Box>
|
||||
|
||||
<Group gap={8} wrap="nowrap" onClick={(e) => e.stopPropagation()}>
|
||||
<SeverityDot severity={severity} />
|
||||
{sevLabel && (
|
||||
<Text
|
||||
fz={10}
|
||||
fw={600}
|
||||
tt="uppercase"
|
||||
c="dimmed"
|
||||
style={{ letterSpacing: ".06em", flex: 1 }}
|
||||
>
|
||||
{sevLabel}
|
||||
</Text>
|
||||
)}
|
||||
<Box style={{ flex: 1 }} />
|
||||
{/* Applied state (#315): a suggestion that was applied but kept alive by
|
||||
its replies (so #329 resolved instead of hard-deleting it) still
|
||||
shows it was applied — the badge the pre-redesign card carried. A
|
||||
childless applied suggestion is gone from the list entirely, so this
|
||||
only renders in the Resolved tab. */}
|
||||
{isApplied && (
|
||||
<Badge
|
||||
size="sm"
|
||||
color="green"
|
||||
variant="light"
|
||||
aria-label={t("Applied")}
|
||||
>
|
||||
{t("Applied")}
|
||||
</Badge>
|
||||
)}
|
||||
{showDismiss && (
|
||||
<Button
|
||||
size="compact-sm"
|
||||
variant="default"
|
||||
color="gray"
|
||||
loading={dismissSuggestionMutation.isPending}
|
||||
disabled={pending}
|
||||
onClick={handleDismiss}
|
||||
>
|
||||
{t("Dismiss")}
|
||||
</Button>
|
||||
)}
|
||||
{showApply && (
|
||||
<Button
|
||||
size="compact-sm"
|
||||
color="green"
|
||||
loading={applySuggestionMutation.isPending}
|
||||
disabled={pending}
|
||||
onClick={handleApply}
|
||||
>
|
||||
{t("Apply")}
|
||||
</Button>
|
||||
)}
|
||||
</Group>
|
||||
</Stack>
|
||||
</Box>
|
||||
);
|
||||
}
|
||||
|
||||
// Series header for a collapsed agent run: ONE provenance line for N edits,
|
||||
// with a "N edits · M major" tally. There is intentionally NO "Accept all"
|
||||
// button / progress / Stop (rejected by product) and NO "K applied" counter
|
||||
// (uncomputable after the #329 hard-delete). Purely visual.
|
||||
export function RunHeader({ comments }: { comments: IComment[] }) {
|
||||
const { t } = useTranslation();
|
||||
const head = comments[0];
|
||||
const createdAtAgo = useTimeAgo(head?.createdAt);
|
||||
|
||||
const majors = useMemo(
|
||||
() =>
|
||||
comments.filter((c) =>
|
||||
isSignificant(parseSeverity(commentContentToText(c.content))),
|
||||
).length,
|
||||
[comments],
|
||||
);
|
||||
|
||||
if (!head) return null;
|
||||
|
||||
return (
|
||||
<Box
|
||||
p="11px 13px"
|
||||
style={{ borderBottom: "1px solid var(--mantine-color-default-border)" }}
|
||||
>
|
||||
<Group gap={10} wrap="nowrap" justify="space-between">
|
||||
{head.agent ? (
|
||||
<AgentAvatarStack
|
||||
agent={head.agent}
|
||||
launcher={head.launcher}
|
||||
aiChatId={head.aiChatId}
|
||||
/>
|
||||
) : (
|
||||
<Text size="sm" fw={600}>
|
||||
{head.creator?.name}
|
||||
</Text>
|
||||
)}
|
||||
<Text size="xs" c="dimmed" style={{ flex: "none" }}>
|
||||
{createdAtAgo}
|
||||
</Text>
|
||||
</Group>
|
||||
<Text fz={11.5} c="dimmed" mt={4}>
|
||||
{t("{{count}} edits", { count: comments.length })}
|
||||
{majors > 0 && (
|
||||
<>
|
||||
{" · "}
|
||||
<Text span c="orange.7" fw={600} inherit>
|
||||
{t("{{count}} major", { count: majors })}
|
||||
</Text>
|
||||
</>
|
||||
)}
|
||||
</Text>
|
||||
</Box>
|
||||
);
|
||||
}
|
||||
|
||||
export default React.memo(AgentEditCard);
|
||||
@@ -156,125 +156,6 @@ describe("CommentListItem — agent avatar stack", () => {
|
||||
// only guards the insertion gate (agent → stack, user → no stack).
|
||||
});
|
||||
|
||||
describe("CommentListItem — suggested edit (#315)", () => {
|
||||
const suggestion = (over?: Partial<IComment>): IComment =>
|
||||
baseComment({
|
||||
selection: "old wording here",
|
||||
suggestedText: "new wording here",
|
||||
...over,
|
||||
});
|
||||
|
||||
it("renders the было→стало diff and an Apply button when canEdit and not applied/resolved", () => {
|
||||
const { container } = renderItem(suggestion(), true);
|
||||
// Old text appears as the selection quote (a single unsplit Text node).
|
||||
expect(screen.getAllByText("old wording here").length).toBeGreaterThan(0);
|
||||
// The new line is now rendered as per-fragment spans (intraline diff, #331),
|
||||
// so it is no longer a single text node — assert the concatenated content.
|
||||
expect(container.textContent).toContain("new wording here");
|
||||
// Apply button is present.
|
||||
expect(screen.getByRole("button", { name: "Apply" })).toBeDefined();
|
||||
// No Applied badge yet.
|
||||
expect(screen.queryByText("Applied")).toBeNull();
|
||||
});
|
||||
|
||||
it("hides the Apply button when canEdit is false", () => {
|
||||
const { container } = renderItem(suggestion(), false);
|
||||
// Diff still renders (as per-fragment spans, #331)...
|
||||
expect(container.textContent).toContain("new wording here");
|
||||
// ...but no Apply button.
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
});
|
||||
|
||||
it("shows an Applied badge (no Apply button) once suggestionAppliedAt is set", () => {
|
||||
renderItem(suggestion({ suggestionAppliedAt: new Date() }), true);
|
||||
expect(screen.getByText("Applied")).toBeDefined();
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
});
|
||||
|
||||
it("hides the Apply button once the thread is resolved", () => {
|
||||
renderItem(suggestion({ resolvedAt: new Date() }), true);
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
});
|
||||
|
||||
it("calls the apply mutation when the Apply button is clicked", () => {
|
||||
applyMutateAsync.mockClear();
|
||||
renderItem(suggestion(), true);
|
||||
fireEvent.click(screen.getByRole("button", { name: "Apply" }));
|
||||
expect(applyMutateAsync).toHaveBeenCalledWith({
|
||||
commentId: "c-1",
|
||||
pageId: "page-1",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not render the diff block for a reply (child) comment", () => {
|
||||
renderItem(
|
||||
suggestion({ parentCommentId: "c-0" }),
|
||||
true,
|
||||
);
|
||||
expect(screen.queryByText("new wording here")).toBeNull();
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("CommentListItem — dismiss suggestion (#329)", () => {
|
||||
const suggestion = (over?: Partial<IComment>): IComment =>
|
||||
baseComment({
|
||||
selection: "old wording here",
|
||||
suggestedText: "new wording here",
|
||||
...over,
|
||||
});
|
||||
|
||||
// A space admin (userSpaceRole="admin") satisfies the owner-or-admin gate
|
||||
// regardless of who authored the comment; the tests below use it as the lever
|
||||
// since the currentUser atom is unseeded (null) in this harness.
|
||||
it("renders a Dismiss button alongside Apply when canEdit and canComment (owner/admin)", () => {
|
||||
renderItem(suggestion(), true, true, "admin");
|
||||
expect(screen.getByRole("button", { name: "Apply" })).toBeDefined();
|
||||
expect(screen.getByRole("button", { name: "Dismiss" })).toBeDefined();
|
||||
});
|
||||
|
||||
it("shows Dismiss but NOT Apply for an admin commenter who cannot edit", () => {
|
||||
renderItem(suggestion(), false, true, "admin");
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
expect(screen.getByRole("button", { name: "Dismiss" })).toBeDefined();
|
||||
});
|
||||
|
||||
it("hides Dismiss when the viewer cannot comment", () => {
|
||||
renderItem(suggestion(), false, false, "admin");
|
||||
expect(screen.queryByRole("button", { name: "Dismiss" })).toBeNull();
|
||||
expect(screen.queryByRole("button", { name: "Apply" })).toBeNull();
|
||||
});
|
||||
|
||||
it("hides Dismiss for a non-owner non-admin even with canComment (#338 F5: mirrors server 403)", () => {
|
||||
// canComment=true but NOT a space admin and NOT the comment owner (the
|
||||
// currentUser atom is null while the comment is authored by user-1), so the
|
||||
// server would 403 a dismiss — the button must not be shown at all.
|
||||
renderItem(suggestion(), false, true, "member");
|
||||
expect(screen.queryByRole("button", { name: "Dismiss" })).toBeNull();
|
||||
});
|
||||
|
||||
it("hides Dismiss once the thread is resolved", () => {
|
||||
renderItem(suggestion({ resolvedAt: new Date() }), true, true, "admin");
|
||||
expect(screen.queryByRole("button", { name: "Dismiss" })).toBeNull();
|
||||
});
|
||||
|
||||
it("hides Dismiss (shows the Applied badge) once applied", () => {
|
||||
renderItem(suggestion({ suggestionAppliedAt: new Date() }), true, true, "admin");
|
||||
expect(screen.queryByRole("button", { name: "Dismiss" })).toBeNull();
|
||||
expect(screen.getByText("Applied")).toBeDefined();
|
||||
});
|
||||
|
||||
it("calls the dismiss mutation when the Dismiss button is clicked", () => {
|
||||
dismissMutateAsync.mockClear();
|
||||
renderItem(suggestion(), true, true, "admin");
|
||||
fireEvent.click(screen.getByRole("button", { name: "Dismiss" }));
|
||||
expect(dismissMutateAsync).toHaveBeenCalledWith({
|
||||
commentId: "c-1",
|
||||
pageId: "page-1",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("canShowApply predicate", () => {
|
||||
const c = (over?: Partial<IComment>): IComment =>
|
||||
({ suggestedText: "x", ...over }) as IComment;
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { Group, Text, Box, Badge, Button } from "@mantine/core";
|
||||
import { Group, Text, Box } from "@mantine/core";
|
||||
import { AgentAvatarStack } from "@/components/ui/agent-avatar-stack.tsx";
|
||||
import React, { useMemo, useRef, useState } from "react";
|
||||
import React, { useRef, useState } from "react";
|
||||
import classes from "./comment.module.css";
|
||||
import { useAtom, useAtomValue } from "jotai";
|
||||
import { useTimeAgo } from "@/hooks/use-time-ago";
|
||||
@@ -12,18 +12,11 @@ import CommentMenu from "@/features/comment/components/comment-menu";
|
||||
import ResolveComment from "@/features/comment/components/resolve-comment";
|
||||
import { useHover } from "@mantine/hooks";
|
||||
import {
|
||||
useApplySuggestionMutation,
|
||||
useDeleteCommentMutation,
|
||||
useDismissSuggestionMutation,
|
||||
useResolveCommentMutation,
|
||||
useUpdateCommentMutation,
|
||||
} from "@/features/comment/queries/comment-query";
|
||||
import { IComment } from "@/features/comment/types/comment.types";
|
||||
import {
|
||||
canShowApply,
|
||||
canShowDismiss,
|
||||
computeSuggestionDiff,
|
||||
} from "@/features/comment/utils/suggestion";
|
||||
import { CustomAvatar } from "@/components/ui/custom-avatar.tsx";
|
||||
import { currentUserAtom } from "@/features/user/atoms/current-user-atom.ts";
|
||||
import { useTranslation } from "react-i18next";
|
||||
@@ -32,13 +25,21 @@ interface CommentListItemProps {
|
||||
comment: IComment;
|
||||
pageId: string;
|
||||
canComment: boolean;
|
||||
// Real page-edit permission (page.permissions.canEdit) — gates the suggestion
|
||||
// "Apply" button. Distinct from `canComment`, which may be looser (viewers
|
||||
// allowed to comment cannot apply edits).
|
||||
// Real page-edit permission (page.permissions.canEdit). Kept on the props for
|
||||
// parity with the container's wiring even though the thread row itself no
|
||||
// longer renders the suggestion Apply button (that moved to AgentEditCard).
|
||||
canEdit?: boolean;
|
||||
userSpaceRole?: string;
|
||||
}
|
||||
|
||||
// Type B — the thread ROW. Renders a single human OR agent-without-edit comment
|
||||
// in the redesigned visual: provenance avatar, author + timeago, hover-revealed
|
||||
// resolve + edit/delete menu, the anchored selection quote, and the body through
|
||||
// the static CommentContentView (or the inline TipTap editor while editing). It
|
||||
// is used for both a top-level thread comment and, recursively, each reply row.
|
||||
// ALL wiring (update/delete/resolve mutations, owner/admin gate, anchor nav) is
|
||||
// the same logic the old row carried — only the presentation changed, and the
|
||||
// agent suggested-edit block was lifted out into AgentEditCard.
|
||||
function CommentListItem({
|
||||
comment,
|
||||
pageId,
|
||||
@@ -55,29 +56,20 @@ function CommentListItem({
|
||||
const updateCommentMutation = useUpdateCommentMutation();
|
||||
const deleteCommentMutation = useDeleteCommentMutation(comment.pageId);
|
||||
const resolveCommentMutation = useResolveCommentMutation();
|
||||
const applySuggestionMutation = useApplySuggestionMutation();
|
||||
const dismissSuggestionMutation = useDismissSuggestionMutation();
|
||||
const [currentUser] = useAtom(currentUserAtom);
|
||||
const createdAtAgo = useTimeAgo(comment.createdAt);
|
||||
|
||||
// Intraline "before -> after" diff (#331) for a suggested edit: only the
|
||||
// fragments that actually changed get emphasised inside the red/green block,
|
||||
// instead of striking through / greening the whole line. Memoised on the
|
||||
// (selection, suggestedText) pair so it recomputes only when they change.
|
||||
const suggestionDiff = useMemo(
|
||||
() =>
|
||||
comment.suggestedText != null
|
||||
? computeSuggestionDiff(comment.selection ?? "", comment.suggestedText)
|
||||
: null,
|
||||
[comment.selection, comment.suggestedText],
|
||||
);
|
||||
// `canEdit`/`pageId` are threaded through for wiring parity with the container;
|
||||
// the thread row does not itself gate on them (Apply lives on AgentEditCard).
|
||||
void canEdit;
|
||||
void pageId;
|
||||
|
||||
// Owner-or-space-admin gate (#338): mirrors the server authz for both the
|
||||
// comment menu (edit/delete) and the suggestion Dismiss button, so we never
|
||||
// render an action the server will 403.
|
||||
// Owner-or-space-admin gate (#338): mirrors the server authz for the comment
|
||||
// menu (edit/delete), so we never render an action the server will 403.
|
||||
const isOwnerOrAdmin =
|
||||
currentUser?.user?.id === comment.creatorId || userSpaceRole === "admin";
|
||||
|
||||
const isAgent = comment.createdSource === "agent" && !!comment.agent;
|
||||
|
||||
async function handleUpdateComment() {
|
||||
try {
|
||||
@@ -121,34 +113,9 @@ function CommentListItem({
|
||||
}
|
||||
}
|
||||
|
||||
async function handleApplySuggestion() {
|
||||
try {
|
||||
await applySuggestionMutation.mutateAsync({
|
||||
commentId: comment.id,
|
||||
pageId: comment.pageId,
|
||||
});
|
||||
} catch (error) {
|
||||
// Errors surface via the mutation's onError notification (incl. 409).
|
||||
console.error("Failed to apply suggestion:", error);
|
||||
}
|
||||
}
|
||||
|
||||
async function handleDismissSuggestion() {
|
||||
try {
|
||||
await dismissSuggestionMutation.mutateAsync({
|
||||
commentId: comment.id,
|
||||
pageId: comment.pageId,
|
||||
});
|
||||
} catch (error) {
|
||||
// Idempotent races are reconciled to success in the mutation's onError;
|
||||
// anything else surfaces there as a notification.
|
||||
console.error("Failed to dismiss suggestion:", error);
|
||||
}
|
||||
}
|
||||
|
||||
function handleCommentClick(comment: IComment) {
|
||||
function handleCommentClick(target: IComment) {
|
||||
const el = document.querySelector(
|
||||
`.comment-mark[data-comment-id="${comment.id}"]`,
|
||||
`.comment-mark[data-comment-id="${target.id}"]`,
|
||||
);
|
||||
if (el) {
|
||||
el.scrollIntoView({ behavior: "smooth", block: "center" });
|
||||
@@ -169,10 +136,10 @@ function CommentListItem({
|
||||
|
||||
return (
|
||||
<Box ref={ref} pb={6}>
|
||||
<Group gap="xs">
|
||||
{comment.createdSource === "agent" && comment.agent ? (
|
||||
<Group gap="xs" wrap="nowrap" align="flex-start">
|
||||
{isAgent ? (
|
||||
<AgentAvatarStack
|
||||
agent={comment.agent}
|
||||
agent={comment.agent!}
|
||||
launcher={comment.launcher}
|
||||
aiChatId={comment.aiChatId}
|
||||
showName={false}
|
||||
@@ -185,13 +152,13 @@ function CommentListItem({
|
||||
/>
|
||||
)}
|
||||
|
||||
<div style={{ flex: 1 }}>
|
||||
<div style={{ flex: 1, minWidth: 0 }}>
|
||||
<Group justify="space-between" wrap="nowrap">
|
||||
<Group gap={6} wrap="nowrap" style={{ minWidth: 0 }}>
|
||||
{comment.createdSource === "agent" && comment.agent ? (
|
||||
{isAgent ? (
|
||||
<>
|
||||
<Text size="xs" fw={600} lineClamp={1} lh={1.2}>
|
||||
{comment.agent.name}
|
||||
{comment.agent!.name}
|
||||
</Text>
|
||||
{comment.launcher && (
|
||||
<>
|
||||
@@ -262,87 +229,6 @@ function CommentListItem({
|
||||
</Box>
|
||||
)}
|
||||
|
||||
{/* Suggested-edit (#315): "было → стало" diff for a top-level comment
|
||||
carrying a suggestion. Old text struck-through/red, new text green. */}
|
||||
{!comment.parentCommentId && comment.suggestedText && (
|
||||
<Box className={classes.suggestionBlock}>
|
||||
{comment.selection && (
|
||||
// Old line: read as removed as a whole (line-through/red); only the
|
||||
// changed fragments carry the extra intraline emphasis.
|
||||
<Text size="xs" className={classes.suggestionOld}>
|
||||
{suggestionDiff?.old.map((segment, index) => (
|
||||
<span
|
||||
key={index}
|
||||
className={segment.changed ? classes.suggestionChanged : undefined}
|
||||
>
|
||||
{segment.text}
|
||||
</span>
|
||||
))}
|
||||
</Text>
|
||||
)}
|
||||
<Text size="xs" className={classes.suggestionNew}>
|
||||
{suggestionDiff?.new.map((segment, index) => (
|
||||
<span
|
||||
key={index}
|
||||
className={segment.changed ? classes.suggestionChanged : undefined}
|
||||
>
|
||||
{segment.text}
|
||||
</span>
|
||||
))}
|
||||
</Text>
|
||||
|
||||
{comment.suggestionAppliedAt ? (
|
||||
<Badge
|
||||
size="sm"
|
||||
color="green"
|
||||
variant="light"
|
||||
mt={6}
|
||||
aria-label={t("Applied")}
|
||||
>
|
||||
{t("Applied")}
|
||||
</Badge>
|
||||
) : (
|
||||
(canShowApply(comment, canEdit) ||
|
||||
canShowDismiss(comment, canComment, isOwnerOrAdmin)) && (
|
||||
<Group gap="xs" mt={6}>
|
||||
{canShowApply(comment, canEdit) && (
|
||||
<Button
|
||||
size="compact-xs"
|
||||
variant="light"
|
||||
color="green"
|
||||
onClick={handleApplySuggestion}
|
||||
loading={applySuggestionMutation.isPending}
|
||||
disabled={
|
||||
applySuggestionMutation.isPending ||
|
||||
dismissSuggestionMutation.isPending
|
||||
}
|
||||
>
|
||||
{t("Apply")}
|
||||
</Button>
|
||||
)}
|
||||
{/* Dismiss ("Не применять", #329): removes the suggestion
|
||||
without changing the page text. Gated on canComment. */}
|
||||
{canShowDismiss(comment, canComment, isOwnerOrAdmin) && (
|
||||
<Button
|
||||
size="compact-xs"
|
||||
variant="subtle"
|
||||
color="gray"
|
||||
onClick={handleDismissSuggestion}
|
||||
loading={dismissSuggestionMutation.isPending}
|
||||
disabled={
|
||||
applySuggestionMutation.isPending ||
|
||||
dismissSuggestionMutation.isPending
|
||||
}
|
||||
>
|
||||
{t("Dismiss")}
|
||||
</Button>
|
||||
)}
|
||||
</Group>
|
||||
)
|
||||
)}
|
||||
</Box>
|
||||
)}
|
||||
|
||||
{!isEditing ? (
|
||||
<CommentContentView content={comment.content} />
|
||||
) : (
|
||||
@@ -350,7 +236,9 @@ function CommentListItem({
|
||||
<CommentEditor
|
||||
defaultContent={comment.content}
|
||||
editable={true}
|
||||
onUpdate={(newContent: any) => { editContentRef.current = newContent; }}
|
||||
onUpdate={(newContent: any) => {
|
||||
editContentRef.current = newContent;
|
||||
}}
|
||||
onSave={handleUpdateComment}
|
||||
autofocus={true}
|
||||
/>
|
||||
|
||||
@@ -5,7 +5,7 @@ import {
|
||||
Center,
|
||||
Divider,
|
||||
Group,
|
||||
Paper,
|
||||
Box,
|
||||
Stack,
|
||||
Tabs,
|
||||
Badge,
|
||||
@@ -14,6 +14,9 @@ import {
|
||||
Tooltip,
|
||||
} from "@mantine/core";
|
||||
import CommentListItem from "@/features/comment/components/comment-list-item";
|
||||
import AgentEditCard, {
|
||||
RunHeader,
|
||||
} from "@/features/comment/components/agent-edit-card";
|
||||
import {
|
||||
useCommentsQuery,
|
||||
useCreateCommentMutation,
|
||||
@@ -22,6 +25,10 @@ import CommentEditor from "@/features/comment/components/comment-editor";
|
||||
import CommentActions from "@/features/comment/components/comment-actions";
|
||||
import { useFocusWithin } from "@mantine/hooks";
|
||||
import { IComment } from "@/features/comment/types/comment.types.ts";
|
||||
import {
|
||||
groupAgentRuns,
|
||||
CommentRenderUnit,
|
||||
} from "@/features/comment/utils/group-agent-runs";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
import { useTranslation } from "react-i18next";
|
||||
@@ -69,6 +76,32 @@ export function sortResolvedByResolvedAt(resolved: IComment[]): IComment[] {
|
||||
);
|
||||
}
|
||||
|
||||
// The redesigned card shell: a rounded, bordered surface on the panel body. Both
|
||||
// a thread card and an agent-run group live inside one of these.
|
||||
function PanelCard({
|
||||
children,
|
||||
...rest
|
||||
}: {
|
||||
children: React.ReactNode;
|
||||
[key: string]: unknown;
|
||||
}) {
|
||||
return (
|
||||
<Box
|
||||
m="8px 10px"
|
||||
p={0}
|
||||
style={{
|
||||
background: "var(--mantine-color-body)",
|
||||
border: "1px solid var(--mantine-color-default-border)",
|
||||
borderRadius: 10,
|
||||
overflow: "hidden",
|
||||
}}
|
||||
{...rest}
|
||||
>
|
||||
{children}
|
||||
</Box>
|
||||
);
|
||||
}
|
||||
|
||||
function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
const { t } = useTranslation();
|
||||
const { pageSlug } = useParams();
|
||||
@@ -90,6 +123,8 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
canEdit ||
|
||||
(space?.settings?.comments?.allowViewerComments === true);
|
||||
|
||||
const userSpaceRole = space?.membership?.role;
|
||||
|
||||
// Separate active and resolved comments
|
||||
const { activeComments, resolvedComments } = useMemo(() => {
|
||||
if (!comments?.items) {
|
||||
@@ -113,6 +148,17 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
};
|
||||
}, [comments]);
|
||||
|
||||
// Collapse each tab's top-level list into render units (a lone comment or a
|
||||
// collapsed agent run). Purely visual — the underlying data is untouched.
|
||||
const activeUnits = useMemo(
|
||||
() => groupAgentRuns(activeComments),
|
||||
[activeComments],
|
||||
);
|
||||
const resolvedUnits = useMemo(
|
||||
() => groupAgentRuns(resolvedComments),
|
||||
[resolvedComments],
|
||||
);
|
||||
|
||||
// Index replies by their parent once, instead of an O(n^2) filter per thread.
|
||||
// The map ref changes on any comments update, so MemoizedChildComments re-runs
|
||||
// (cheap) and re-looks-up, while memoized CommentListItems skip unchanged items.
|
||||
@@ -168,56 +214,104 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
[createCommentAsync, page?.id],
|
||||
);
|
||||
|
||||
const renderComments = useCallback(
|
||||
(comment: IComment) => (
|
||||
<Paper
|
||||
shadow="sm"
|
||||
radius="md"
|
||||
p="xs"
|
||||
mb="xs"
|
||||
withBorder
|
||||
key={comment.id}
|
||||
data-comment-id={comment.id}
|
||||
>
|
||||
<div>
|
||||
<CommentListItem
|
||||
// The full subtree for ONE top-level comment: its head card (thread row or
|
||||
// agent edit card), its nested replies, and a lazily-mounted reply editor.
|
||||
// Shared by a standalone card and a card inside an agent-run group so the
|
||||
// reply threading / lazy editor (#340) is wired identically in both.
|
||||
const renderCommentSubtree = useCallback(
|
||||
(comment: IComment, isEdit: boolean, showProvenance: boolean) => (
|
||||
<>
|
||||
{isEdit ? (
|
||||
<AgentEditCard
|
||||
comment={comment}
|
||||
pageId={page?.id}
|
||||
canComment={canComment}
|
||||
canEdit={canEdit}
|
||||
userSpaceRole={space?.membership?.role}
|
||||
userSpaceRole={userSpaceRole}
|
||||
showProvenance={showProvenance}
|
||||
/>
|
||||
) : (
|
||||
<Box p="xs">
|
||||
<CommentListItem
|
||||
comment={comment}
|
||||
pageId={page?.id}
|
||||
canComment={canComment}
|
||||
canEdit={canEdit}
|
||||
userSpaceRole={userSpaceRole}
|
||||
/>
|
||||
</Box>
|
||||
)}
|
||||
|
||||
<Box px="xs">
|
||||
<MemoizedChildComments
|
||||
childrenByParent={childrenByParent}
|
||||
parentId={comment.id}
|
||||
pageId={page?.id}
|
||||
canComment={canComment}
|
||||
canEdit={canEdit}
|
||||
userSpaceRole={space?.membership?.role}
|
||||
userSpaceRole={userSpaceRole}
|
||||
/>
|
||||
</div>
|
||||
</Box>
|
||||
|
||||
{!comment.resolvedAt && canComment && (
|
||||
<>
|
||||
<Box px="xs" pb="xs">
|
||||
<Divider my={2} />
|
||||
<CommentEditorWithActions
|
||||
commentId={comment.id}
|
||||
onSave={handleAddReply}
|
||||
/>
|
||||
</>
|
||||
</Box>
|
||||
)}
|
||||
</Paper>
|
||||
</>
|
||||
),
|
||||
[
|
||||
childrenByParent,
|
||||
handleAddReply,
|
||||
page?.id,
|
||||
space?.membership?.role,
|
||||
userSpaceRole,
|
||||
canComment,
|
||||
canEdit,
|
||||
],
|
||||
);
|
||||
|
||||
// Render one collapsed unit: a standalone card (thread or lone edit) or an
|
||||
// agent-run group (one RunHeader over N stacked edit cards).
|
||||
const renderUnit = useCallback(
|
||||
(unit: CommentRenderUnit) => {
|
||||
if (unit.kind === "single") {
|
||||
const c = unit.comment;
|
||||
const isEdit =
|
||||
c.createdSource === "agent" &&
|
||||
c.suggestedText != null &&
|
||||
!c.parentCommentId;
|
||||
return (
|
||||
<PanelCard key={c.id} data-comment-id={c.id}>
|
||||
{renderCommentSubtree(c, isEdit, true)}
|
||||
</PanelCard>
|
||||
);
|
||||
}
|
||||
|
||||
// A collapsed agent run: one header, then each edit card (provenance
|
||||
// suppressed on the cards — the header carries the single provenance line).
|
||||
return (
|
||||
<PanelCard key={unit.key}>
|
||||
<RunHeader comments={unit.comments} />
|
||||
{unit.comments.map((c) => (
|
||||
<Box
|
||||
key={c.id}
|
||||
data-comment-id={c.id}
|
||||
style={{
|
||||
borderTop: "1px solid var(--mantine-color-default-border)",
|
||||
}}
|
||||
>
|
||||
{renderCommentSubtree(c, true, false)}
|
||||
</Box>
|
||||
))}
|
||||
</PanelCard>
|
||||
);
|
||||
},
|
||||
[renderCommentSubtree],
|
||||
);
|
||||
|
||||
if (isCommentsLoading) {
|
||||
return <></>;
|
||||
}
|
||||
@@ -226,8 +320,6 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
return <div>{t("Error loading comments.")}</div>;
|
||||
}
|
||||
|
||||
const totalComments = activeComments.length + resolvedComments.length;
|
||||
|
||||
const pageCommentInput = canComment ? (
|
||||
<PageCommentInput
|
||||
onSave={handleAddPageComment}
|
||||
@@ -328,7 +420,7 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
</Stack>
|
||||
</Center>
|
||||
) : (
|
||||
activeComments.map(renderComments)
|
||||
activeUnits.map(renderUnit)
|
||||
)}
|
||||
</Tabs.Panel>
|
||||
|
||||
@@ -347,7 +439,7 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
</Stack>
|
||||
</Center>
|
||||
) : (
|
||||
resolvedComments.map(renderComments)
|
||||
resolvedUnits.map(renderUnit)
|
||||
)}
|
||||
</Tabs.Panel>
|
||||
</div>
|
||||
|
||||
@@ -21,53 +21,6 @@
|
||||
box-sizing: border-box;
|
||||
}
|
||||
|
||||
/* Suggested-edit (#315) "было → стало" diff block. */
|
||||
.suggestionBlock {
|
||||
margin-top: 8px;
|
||||
margin-left: 6px;
|
||||
padding: 6px;
|
||||
border-radius: var(--mantine-radius-sm);
|
||||
border: 1px solid var(--mantine-color-default-border);
|
||||
overflow-wrap: break-word;
|
||||
word-break: break-word;
|
||||
max-width: 100%;
|
||||
box-sizing: border-box;
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
align-items: flex-start;
|
||||
}
|
||||
|
||||
.suggestionOld {
|
||||
text-decoration: line-through;
|
||||
color: var(--mantine-color-red-7);
|
||||
background: var(--mantine-color-red-light);
|
||||
border-radius: 2px;
|
||||
padding: 1px 3px;
|
||||
}
|
||||
|
||||
.suggestionNew {
|
||||
color: var(--mantine-color-green-9);
|
||||
background: var(--mantine-color-green-light);
|
||||
border-radius: 2px;
|
||||
padding: 1px 3px;
|
||||
margin-top: 4px;
|
||||
}
|
||||
|
||||
/* Intraline diff (#331): the fragment that actually changed within the
|
||||
red "before" / green "after" block. It inherits the surrounding red/green
|
||||
framing and adds a stronger tint plus bold weight so the eye lands on the
|
||||
changed letters/words (git/GitHub-style) rather than the whole line. The
|
||||
container's line-through (old) / green (new) still marks the full line. */
|
||||
.suggestionChanged {
|
||||
/* Stronger tint of the surrounding red/green so the changed fragment pops
|
||||
within the block. `currentColor` follows the parent's red (old) or green
|
||||
(new) text colour. No `text-decoration` here on purpose: the old block's
|
||||
inherited line-through must survive on the changed letters too. */
|
||||
background: color-mix(in srgb, currentColor 22%, transparent);
|
||||
border-radius: 2px;
|
||||
font-weight: 700;
|
||||
}
|
||||
|
||||
.commentEditor {
|
||||
|
||||
&[data-editable][data-surface="muted"] .ProseMirror:not(.focused) {
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { groupAgentRuns, runKey, GROUP_MIN } from "./group-agent-runs";
|
||||
import { IComment } from "@/features/comment/types/comment.types";
|
||||
|
||||
const editComment = (id: string, over?: Partial<IComment>): IComment =>
|
||||
({
|
||||
id,
|
||||
createdSource: "agent",
|
||||
aiChatId: "chat-1",
|
||||
agent: { name: "Corrector" },
|
||||
suggestedText: "new text",
|
||||
parentCommentId: null,
|
||||
...over,
|
||||
}) as unknown as IComment;
|
||||
|
||||
const human = (id: string): IComment =>
|
||||
({ id, createdSource: "user", parentCommentId: null }) as unknown as IComment;
|
||||
|
||||
describe("runKey", () => {
|
||||
it("keys a groupable agent edit on aiChatId + agent.name", () => {
|
||||
expect(runKey(editComment("a"))).toBe("chat-1:Corrector");
|
||||
});
|
||||
|
||||
it("is null for an external MCP agent (aiChatId null)", () => {
|
||||
expect(runKey(editComment("a", { aiChatId: null }))).toBeNull();
|
||||
});
|
||||
|
||||
it("is null for a non-edit agent comment (no suggestedText)", () => {
|
||||
expect(runKey(editComment("a", { suggestedText: null }))).toBeNull();
|
||||
});
|
||||
|
||||
it("is null for a reply (has parentCommentId)", () => {
|
||||
expect(runKey(editComment("a", { parentCommentId: "p" }))).toBeNull();
|
||||
});
|
||||
|
||||
it("is null for a human comment", () => {
|
||||
expect(runKey(human("a"))).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("groupAgentRuns", () => {
|
||||
it("collapses >= GROUP_MIN same chat+role edits into one run at the first position", () => {
|
||||
const units = groupAgentRuns([
|
||||
editComment("e1"),
|
||||
editComment("e2"),
|
||||
editComment("e3"),
|
||||
]);
|
||||
expect(units).toHaveLength(1);
|
||||
expect(units[0].kind).toBe("run");
|
||||
if (units[0].kind === "run") {
|
||||
expect(units[0].key).toBe("chat-1:Corrector");
|
||||
expect(units[0].comments.map((c) => c.id)).toEqual(["e1", "e2", "e3"]);
|
||||
}
|
||||
expect(GROUP_MIN).toBe(2);
|
||||
});
|
||||
|
||||
it("renders a lone edit as a single (below the threshold)", () => {
|
||||
const units = groupAgentRuns([editComment("e1")]);
|
||||
expect(units).toHaveLength(1);
|
||||
expect(units[0].kind).toBe("single");
|
||||
});
|
||||
|
||||
it("never groups external MCP edits (aiChatId null) — each is a single", () => {
|
||||
const units = groupAgentRuns([
|
||||
editComment("m1", { aiChatId: null }),
|
||||
editComment("m2", { aiChatId: null }),
|
||||
]);
|
||||
expect(units).toHaveLength(2);
|
||||
expect(units.every((u) => u.kind === "single")).toBe(true);
|
||||
});
|
||||
|
||||
it("does not collapse two different roles sharing one chat", () => {
|
||||
const units = groupAgentRuns([
|
||||
editComment("a", { agent: { name: "Corrector" } as any }),
|
||||
editComment("b", { agent: { name: "FactChecker" } as any }),
|
||||
]);
|
||||
// Each key has count 1 -> both remain singles.
|
||||
expect(units).toHaveLength(2);
|
||||
expect(units.every((u) => u.kind === "single")).toBe(true);
|
||||
});
|
||||
|
||||
it("preserves order and keeps human threads as singles interleaved with a run", () => {
|
||||
const units = groupAgentRuns([
|
||||
human("h1"),
|
||||
editComment("e1"),
|
||||
editComment("e2"),
|
||||
human("h2"),
|
||||
]);
|
||||
// h1 single, then the run (emitted at e1's position, e2 absorbed), then h2.
|
||||
expect(units.map((u) => u.kind)).toEqual(["single", "run", "single"]);
|
||||
if (units[1].kind === "run") {
|
||||
expect(units[1].comments.map((c) => c.id)).toEqual(["e1", "e2"]);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,76 @@
|
||||
import { IComment } from "@/features/comment/types/comment.types";
|
||||
|
||||
// A visual-only series threshold: this many agent suggested-edits sharing one
|
||||
// run key collapse under a single RunHeader. A group of one renders as a plain
|
||||
// standalone card (no header). Policy constant — no env override.
|
||||
export const GROUP_MIN = 2;
|
||||
|
||||
// One rendered unit of the top-level comment list: either a collapsed agent-run
|
||||
// group (>= GROUP_MIN suggested-edits from the same chat+role) or a single
|
||||
// comment (a human thread, an agent thread without an edit, or a lone edit).
|
||||
export interface AgentRunGroup {
|
||||
kind: "run";
|
||||
key: string;
|
||||
comments: IComment[];
|
||||
}
|
||||
export interface SingleUnit {
|
||||
kind: "single";
|
||||
comment: IComment;
|
||||
}
|
||||
export type CommentRenderUnit = AgentRunGroup | SingleUnit;
|
||||
|
||||
// The grouping key of a top-level agent suggested-edit, or null when the comment
|
||||
// is not a groupable edit. The key pins BOTH the AI chat and the acting role
|
||||
// (`aiChatId + ":" + agent.name`) so two roles running in the same chat do not
|
||||
// collapse under one header. A comment with `aiChatId == null` (an external MCP
|
||||
// agent) has no chat to group by — it is deliberately never groupable and always
|
||||
// renders as a single card (a time-bucketed synthetic run would split/merge runs
|
||||
// arbitrarily and choke on ISO `createdAt`). PURE.
|
||||
export function runKey(c: IComment): string | null {
|
||||
if (
|
||||
c.createdSource === "agent" &&
|
||||
c.suggestedText != null &&
|
||||
!c.parentCommentId &&
|
||||
c.aiChatId != null &&
|
||||
c.agent?.name
|
||||
) {
|
||||
return `${c.aiChatId}:${c.agent.name}`;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// Collapse an ORDERED list of top-level comments into render units, preserving
|
||||
// list order: a run is emitted in place of its FIRST member (later members are
|
||||
// absorbed), everything else stays a single unit. Grouping is computed over the
|
||||
// snapshot handed in; a streamed page / WS update may later promote a single
|
||||
// edit into a run as more members arrive — that is acceptable (purely visual).
|
||||
// PURE — no React/DOM, no mutation of the input.
|
||||
export function groupAgentRuns(comments: IComment[]): CommentRenderUnit[] {
|
||||
// First pass: tally groupable edits per key so we know which keys clear
|
||||
// GROUP_MIN before we start emitting.
|
||||
const counts = new Map<string, number>();
|
||||
for (const c of comments) {
|
||||
const key = runKey(c);
|
||||
if (key) counts.set(key, (counts.get(key) ?? 0) + 1);
|
||||
}
|
||||
|
||||
const emitted = new Set<string>();
|
||||
const units: CommentRenderUnit[] = [];
|
||||
for (const c of comments) {
|
||||
const key = runKey(c);
|
||||
if (key && (counts.get(key) ?? 0) >= GROUP_MIN) {
|
||||
// Emit the whole run once, at the position of its first member; skip the
|
||||
// absorbed later members.
|
||||
if (emitted.has(key)) continue;
|
||||
emitted.add(key);
|
||||
units.push({
|
||||
kind: "run",
|
||||
key,
|
||||
comments: comments.filter((x) => runKey(x) === key),
|
||||
});
|
||||
} else {
|
||||
units.push({ kind: "single", comment: c });
|
||||
}
|
||||
}
|
||||
return units;
|
||||
}
|
||||
@@ -0,0 +1,47 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { parseSeverity, isSignificant } from "./severity";
|
||||
|
||||
describe("parseSeverity — exact importance dictionary", () => {
|
||||
it("maps the Russian importance tags", () => {
|
||||
expect(parseSeverity("[Критично] fix now")).toBe("critical");
|
||||
expect(parseSeverity("[Существенно] tighten")).toBe("major");
|
||||
expect(parseSeverity("[Незначительно] nit")).toBe("minor");
|
||||
});
|
||||
|
||||
it("maps the English equivalents and is case-insensitive", () => {
|
||||
expect(parseSeverity("[Critical] x")).toBe("critical");
|
||||
expect(parseSeverity("[MAJOR] x")).toBe("major");
|
||||
expect(parseSeverity("[minor] x")).toBe("minor");
|
||||
expect(parseSeverity("[критично] x")).toBe("critical");
|
||||
});
|
||||
|
||||
it("treats fact-checker verdicts as unknown, NOT a severity", () => {
|
||||
expect(parseSeverity("[Неверно] this is wrong")).toBe("unknown");
|
||||
expect(parseSeverity("[Не проверено] can't verify")).toBe("unknown");
|
||||
expect(parseSeverity("[Непроверяемо] x")).toBe("unknown");
|
||||
expect(parseSeverity("[Это мнение] x")).toBe("unknown");
|
||||
});
|
||||
|
||||
it("treats a typo'd / unrecognised tag as unknown (never falls back to minor)", () => {
|
||||
expect(parseSeverity("[Существенное] typo ending")).toBe("unknown");
|
||||
expect(parseSeverity("[whatever] x")).toBe("unknown");
|
||||
});
|
||||
|
||||
it("returns unknown for no tag or empty input", () => {
|
||||
expect(parseSeverity("plain body, no tag")).toBe("unknown");
|
||||
expect(parseSeverity("")).toBe("unknown");
|
||||
});
|
||||
|
||||
it("finds the recognised importance tag even after a leading verdict tag", () => {
|
||||
expect(parseSeverity("[Неверно] [Существенно] body")).toBe("major");
|
||||
});
|
||||
});
|
||||
|
||||
describe("isSignificant", () => {
|
||||
it("counts only major and critical", () => {
|
||||
expect(isSignificant("critical")).toBe(true);
|
||||
expect(isSignificant("major")).toBe(true);
|
||||
expect(isSignificant("minor")).toBe(false);
|
||||
expect(isSignificant("unknown")).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,44 @@
|
||||
// Severity of an agent suggested-edit, parsed CLIENT-SIDE from the importance
|
||||
// tag the editorial role prompts emit at the head of the comment body
|
||||
// (`[Критично]` / `[Существенно]` / `[Незначительно]`, or their English
|
||||
// equivalents). It is a DISPLAY-ONLY signal: it drives the coloured dot on the
|
||||
// edit card and the "M major" counter in a run header. It never gates an action.
|
||||
//
|
||||
// `unknown` is a first-class value, NOT a synonym for `minor`: any bracketed
|
||||
// text that is not in the exact dictionary — a Fact-checker verdict
|
||||
// (`[Неверно]`, `[Не проверено]`, `[Непроверяемо]`, `[Это мнение]`), a typo
|
||||
// (`[Существенное]`), or the absence of a tag — resolves to `unknown` and draws
|
||||
// a neutral dot. Passing an unrecognised tag off as `minor` would mis-label it.
|
||||
export type Severity = "critical" | "major" | "minor" | "unknown";
|
||||
|
||||
// EXACT (case-insensitive) dictionary. Only these tokens map to a real severity;
|
||||
// everything else falls through to `unknown` on purpose.
|
||||
const SEVERITY_TAGS: Record<string, Severity> = {
|
||||
критично: "critical",
|
||||
существенно: "major",
|
||||
незначительно: "minor",
|
||||
critical: "critical",
|
||||
major: "major",
|
||||
minor: "minor",
|
||||
};
|
||||
|
||||
// Parse the first RECOGNISED importance tag out of the flattened comment text
|
||||
// (use `commentContentToText` to obtain it). Scans every `[...]` token so a tag
|
||||
// that trails a verdict still counts; the first exact-dictionary hit wins. When
|
||||
// no token matches the dictionary the result is `unknown`. PURE — no React/DOM.
|
||||
export function parseSeverity(text: string): Severity {
|
||||
if (!text) return "unknown";
|
||||
const matches = text.matchAll(/\[([^\]]+)\]/g);
|
||||
for (const m of matches) {
|
||||
const tag = m[1].trim().toLowerCase();
|
||||
const sev = SEVERITY_TAGS[tag];
|
||||
if (sev) return sev;
|
||||
}
|
||||
return "unknown";
|
||||
}
|
||||
|
||||
// Whether a severity counts toward the "M major" tally in a run header: only the
|
||||
// genuinely significant ones (major + critical). `minor` and `unknown` do not.
|
||||
export function isSignificant(severity: Severity): boolean {
|
||||
return severity === "major" || severity === "critical";
|
||||
}
|
||||
@@ -92,13 +92,23 @@
|
||||
wrapper, never in the document model) and is rendered inline at the start of
|
||||
the first content line via ::before. This keeps text and wrapped lines flush
|
||||
to the left margin — no hanging indent — while the editable contentDOM stays
|
||||
the FIRST DOM child (#146). */
|
||||
the FIRST DOM child (#146).
|
||||
|
||||
The rules below target `p:first-child`, NOT `> :first-child`: tiptap-react
|
||||
wraps the NodeViewContent children in an extra block-level div
|
||||
(`[data-node-view-content-react]`, style="white-space: inherit"), so
|
||||
`.definitionContent`'s first child is that wrapper, not the paragraph. An
|
||||
inline ::before on the block wrapper (whose child is a block <p>) drops onto
|
||||
its own line above the text — the "+1 line" regression. `p:first-child`
|
||||
reaches the real first paragraph so the number stays inline, and it works
|
||||
whether or not the wrapper is present (definition content is `paragraph+`,
|
||||
so there are no nested paragraphs to mis-match). */
|
||||
.definitionContent {
|
||||
flex: 1 1 auto;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.definitionContent > :first-child::before {
|
||||
.definitionContent p:first-child::before {
|
||||
content: var(--footnote-number, "?") ". ";
|
||||
color: var(--mantine-color-dimmed);
|
||||
font-variant-numeric: tabular-nums;
|
||||
@@ -108,12 +118,13 @@
|
||||
/* The inner editable paragraph inherits `.ProseMirror p { margin: 0.5em 0 }`.
|
||||
Drop the outer margins so the definition sits tight to the heading above and
|
||||
the ::before number aligns with the top of the row — same approach used for
|
||||
callouts in core.css. */
|
||||
.definitionContent > :first-child {
|
||||
callouts in core.css. Target the paragraphs directly (through the
|
||||
tiptap-react content wrapper), same reasoning as the ::before rule above. */
|
||||
.definitionContent p:first-child {
|
||||
margin-top: 0;
|
||||
}
|
||||
|
||||
.definitionContent > :last-child {
|
||||
.definitionContent p:last-child {
|
||||
margin-bottom: 0;
|
||||
}
|
||||
|
||||
|
||||
@@ -665,6 +665,13 @@ export function updateCacheOnMovePage(
|
||||
pageData: Partial<IPage>,
|
||||
) {
|
||||
invalidatePageTree();
|
||||
// Invalidate the moved page's breadcrumbs (#523). The tree-side child-loss
|
||||
// guard removes the moved node from the local tree when its new parent is an
|
||||
// unloaded branch, so `findBreadcrumbPath` misses it and the breadcrumb bar
|
||||
// falls back to the server `["breadcrumbs", pageId]` query — which this move
|
||||
// must invalidate, otherwise the crumbs keep showing the OLD parent until a
|
||||
// refocus/navigation.
|
||||
queryClient.invalidateQueries({ queryKey: ["breadcrumbs", pageId] });
|
||||
// Remove page from old parent's cache
|
||||
const oldQueryKey =
|
||||
oldParentId === null
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import type { IPage } from "@/features/page/types/page.types";
|
||||
|
||||
// A fresh QueryClient stands in for the app singleton (importing the real
|
||||
// @/main.tsx would run ReactDOM.createRoot, which has no DOM root in jsdom).
|
||||
vi.mock("@/main.tsx", async () => {
|
||||
const { QueryClient } = await import("@tanstack/react-query");
|
||||
return { queryClient: new QueryClient() };
|
||||
});
|
||||
|
||||
import { queryClient } from "@/main.tsx";
|
||||
import { updateCacheOnMovePage } from "./page-query";
|
||||
|
||||
// #523: the tree-side child-loss guard removes the moved node from the local
|
||||
// tree when its new parent is an unloaded branch, so `findBreadcrumbPath` misses
|
||||
// it and the breadcrumb bar falls back to the server `["breadcrumbs", pageId]`
|
||||
// query. That query MUST be invalidated by a move, or the crumbs keep showing
|
||||
// the OLD parent until a refocus/navigation.
|
||||
describe("updateCacheOnMovePage — breadcrumbs invalidation (#523)", () => {
|
||||
beforeEach(() => {
|
||||
queryClient.clear();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("invalidates the moved page's ['breadcrumbs', pageId] query", () => {
|
||||
const spy = vi.spyOn(queryClient, "invalidateQueries");
|
||||
|
||||
updateCacheOnMovePage("s1", "moved-page", "old-parent", "new-parent", {
|
||||
id: "moved-page",
|
||||
} as Partial<IPage>);
|
||||
|
||||
const invalidatedBreadcrumbs = spy.mock.calls.some(
|
||||
([arg]) =>
|
||||
Array.isArray((arg as { queryKey?: unknown[] })?.queryKey) &&
|
||||
(arg as { queryKey: unknown[] }).queryKey[0] === "breadcrumbs" &&
|
||||
(arg as { queryKey: unknown[] }).queryKey[1] === "moved-page",
|
||||
);
|
||||
expect(invalidatedBreadcrumbs).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -55,7 +55,14 @@ type Props<T extends object> = {
|
||||
};
|
||||
|
||||
const DRAG_TYPE = 'doc-tree-item';
|
||||
const AUTO_EXPAND_MS = 500;
|
||||
// Hover-hold before a collapsed row auto-expands during a drag. 2s (not ~0.5s)
|
||||
// so merely dragging the cursor THROUGH the tree never expands rows — only a
|
||||
// deliberate hold does (#523).
|
||||
const AUTO_EXPAND_MS = 2000;
|
||||
// How long the "a page just moved in here" cue stays on a collapsed target after
|
||||
// a make-child drop. Long enough to notice at a glance during frequent
|
||||
// collapsed-drops, short enough not to linger. Code-only UX constant (#523).
|
||||
const DROP_LANDED_HIGHLIGHT_MS = 1800;
|
||||
|
||||
function DocTreeRowInner<T extends object>(props: Props<T>) {
|
||||
const {
|
||||
@@ -93,7 +100,11 @@ function DocTreeRowInner<T extends object>(props: Props<T>) {
|
||||
const rowRef = useRef<HTMLElement>(null);
|
||||
const [isDragging, setIsDragging] = useState(false);
|
||||
const [instruction, setInstruction] = useState<Instruction | null>(null);
|
||||
// Transient "just received a child" cue: a make-child drop no longer expands
|
||||
// the (collapsed) target, so flash the row instead so the move isn't invisible.
|
||||
const [landedChild, setLandedChild] = useState(false);
|
||||
const autoExpandTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
const landedChildTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
|
||||
const cancelAutoExpand = useCallback(() => {
|
||||
if (autoExpandTimerRef.current) {
|
||||
@@ -249,11 +260,24 @@ function DocTreeRowInner<T extends object>(props: Props<T>) {
|
||||
? getDragLabel(sourceNode)
|
||||
: 'item';
|
||||
liveRegion.announce(`Moved ${sourceLabel} under ${parentName}.`);
|
||||
// After a make-child drop, expand this row so the user sees the
|
||||
// just-dropped child — especially important when the row had no
|
||||
// children before (chevron just appeared) so the drop would
|
||||
// otherwise be invisible.
|
||||
if (op.kind === 'make-child') onToggle(node.id, true);
|
||||
// Do NOT auto-expand the target on drop: a drop must leave the node
|
||||
// collapsed. Intentional expansion is handled solely by the
|
||||
// hover-hold timer (AUTO_EXPAND_MS). Feedback that the drop landed is
|
||||
// given by the post-move flash + landed-cue highlight + live-region
|
||||
// announce above. When the make-child target is collapsed, flash a
|
||||
// distinct "child moved in here" cue on the row (it stays collapsed).
|
||||
if (op.kind === 'make-child' && !isOpen) {
|
||||
if (landedChildTimerRef.current) {
|
||||
clearTimeout(landedChildTimerRef.current);
|
||||
}
|
||||
setLandedChild(true);
|
||||
landedChildTimerRef.current = setTimeout(() => {
|
||||
setLandedChild(false);
|
||||
landedChildTimerRef.current = null;
|
||||
}, DROP_LANDED_HIGHLIGHT_MS);
|
||||
}
|
||||
// Restore the openness of the MOVED page itself (source) — untouched
|
||||
// by the above; the target is never expanded here.
|
||||
if (source.data.isOpenOnDragStart) onToggle(sourceId, true);
|
||||
},
|
||||
}),
|
||||
@@ -281,6 +305,17 @@ function DocTreeRowInner<T extends object>(props: Props<T>) {
|
||||
|
||||
useEffect(() => () => cancelAutoExpand(), [cancelAutoExpand]);
|
||||
|
||||
// Clear the landed-child cue timer on unmount (mirrors autoExpandTimerRef).
|
||||
useEffect(
|
||||
() => () => {
|
||||
if (landedChildTimerRef.current) {
|
||||
clearTimeout(landedChildTimerRef.current);
|
||||
landedChildTimerRef.current = null;
|
||||
}
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
const effectiveInst =
|
||||
instruction?.type === 'instruction-blocked'
|
||||
? instruction.desired
|
||||
@@ -317,6 +352,7 @@ function DocTreeRowInner<T extends object>(props: Props<T>) {
|
||||
className={styles.node}
|
||||
data-dragging={isDragging || undefined}
|
||||
data-selected={isSelected || undefined}
|
||||
data-landed-child={landedChild || undefined}
|
||||
data-receiving-drop={
|
||||
receivingDrop === 'make-child'
|
||||
? blocked
|
||||
|
||||
@@ -0,0 +1,149 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { renderHook, act } from "@testing-library/react";
|
||||
import { Provider, createStore } from "jotai";
|
||||
import type { ReactNode } from "react";
|
||||
|
||||
import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
|
||||
import { treeModel } from "@/features/page/tree/model/tree-model";
|
||||
import type { SpaceTreeNode } from "@/features/page/tree/types";
|
||||
|
||||
// --- Boundary mocks: only the network/query + router/i18n surfaces the hook
|
||||
// touches. The tree math (treeModel, dropOpToMovePayload) runs for real so the
|
||||
// child-loss guard is exercised end-to-end.
|
||||
const moveMutate = vi.fn().mockResolvedValue({});
|
||||
const updateCacheOnMovePageMock = vi.fn();
|
||||
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
useCreatePageMutation: () => ({ mutateAsync: vi.fn() }),
|
||||
useUpdatePageMutation: () => ({ mutateAsync: vi.fn() }),
|
||||
useRemovePageMutation: () => ({ mutateAsync: vi.fn() }),
|
||||
useMovePageMutation: () => ({ mutateAsync: moveMutate }),
|
||||
updateCacheOnMovePage: (...args: unknown[]) =>
|
||||
updateCacheOnMovePageMock(...args),
|
||||
}));
|
||||
vi.mock("react-router-dom", () => ({
|
||||
useNavigate: () => vi.fn(),
|
||||
useParams: () => ({ spaceSlug: "space", pageSlug: undefined }),
|
||||
}));
|
||||
vi.mock("react-i18next", () => ({
|
||||
useTranslation: () => ({ t: (s: string) => s }),
|
||||
}));
|
||||
vi.mock("@mantine/notifications", () => ({
|
||||
notifications: { show: vi.fn() },
|
||||
}));
|
||||
|
||||
// Import AFTER mocks so the hook binds to them.
|
||||
import { useTreeMutation } from "./use-tree-mutation";
|
||||
|
||||
function node(
|
||||
id: string,
|
||||
over: Partial<SpaceTreeNode> = {},
|
||||
): SpaceTreeNode {
|
||||
return {
|
||||
id,
|
||||
slugId: `slug-${id}`,
|
||||
name: id.toUpperCase(),
|
||||
position: "a0",
|
||||
spaceId: "space-1",
|
||||
parentPageId: null as unknown as string,
|
||||
hasChildren: false,
|
||||
children: [],
|
||||
...over,
|
||||
};
|
||||
}
|
||||
|
||||
function setup(before: SpaceTreeNode[]) {
|
||||
const store = createStore();
|
||||
store.set(treeDataAtom, before);
|
||||
const wrapper = ({ children }: { children: ReactNode }) => (
|
||||
<Provider store={store}>{children}</Provider>
|
||||
);
|
||||
const { result } = renderHook(() => useTreeMutation("space-1"), { wrapper });
|
||||
return { store, result };
|
||||
}
|
||||
|
||||
describe("useTreeMutation.handleMove — child-loss guard (#523)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
moveMutate.mockResolvedValue({});
|
||||
});
|
||||
|
||||
it("make-child into an UNLOADED folder does NOT materialize [source] — keeps it lazy-loadable", async () => {
|
||||
// F has children on the server but none are loaded here (canonical unloaded
|
||||
// form: hasChildren + children:[]). X sits at root.
|
||||
const before = [
|
||||
node("F", { position: "a0", hasChildren: true, children: [] }),
|
||||
node("X", { position: "a5" }),
|
||||
];
|
||||
const { store, result } = setup(before);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleMove("X", {
|
||||
kind: "make-child",
|
||||
targetId: "F",
|
||||
});
|
||||
});
|
||||
|
||||
const tree = store.get(treeDataAtom);
|
||||
const f = treeModel.find(tree, "F");
|
||||
// The guard leaves F unloaded (children stay []), so a later expand fetches
|
||||
// the FULL server set (incl. X) instead of showing a misleading partial [X].
|
||||
// MUTATION: dropping the guard (using the `move` result) would put children
|
||||
// === [X] here and redden this.
|
||||
expect(f?.children).toEqual([]);
|
||||
expect(f?.hasChildren).toBe(true);
|
||||
// X is removed from its old (root) slot; it reappears on expand/load of F.
|
||||
expect(treeModel.find(tree, "X")).toBeNull();
|
||||
// The server move is still persisted.
|
||||
expect(moveMutate).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("make-child into a LOADED folder appends source and KEEPS the existing children", async () => {
|
||||
// F is loaded with one child c1; moving X in must preserve c1 (no loss) and
|
||||
// append X — this path does NOT hit the guard.
|
||||
const before = [
|
||||
node("F", {
|
||||
position: "a0",
|
||||
hasChildren: true,
|
||||
children: [node("c1", { position: "a1", parentPageId: "F" })],
|
||||
}),
|
||||
node("X", { position: "a5" }),
|
||||
];
|
||||
const { store, result } = setup(before);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleMove("X", {
|
||||
kind: "make-child",
|
||||
targetId: "F",
|
||||
});
|
||||
});
|
||||
|
||||
const tree = store.get(treeDataAtom);
|
||||
const f = treeModel.find(tree, "F");
|
||||
expect(f?.children?.map((n) => n.id)).toEqual(["c1", "X"]);
|
||||
expect(f?.hasChildren).toBe(true);
|
||||
// X now lives under F.
|
||||
expect(treeModel.find(tree, "X")?.parentPageId).toBe("F");
|
||||
});
|
||||
|
||||
it("make-child into a genuinely-empty leaf materializes the child (nothing to lose)", async () => {
|
||||
// Leaf L has no server children (hasChildren:false). Dropping X in should
|
||||
// show X immediately — the guard must NOT fire here.
|
||||
const before = [
|
||||
node("L", { position: "a0", hasChildren: false, children: [] }),
|
||||
node("X", { position: "a5" }),
|
||||
];
|
||||
const { store, result } = setup(before);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleMove("X", {
|
||||
kind: "make-child",
|
||||
targetId: "L",
|
||||
});
|
||||
});
|
||||
|
||||
const tree = store.get(treeDataAtom);
|
||||
const l = treeModel.find(tree, "L");
|
||||
expect(l?.children?.map((n) => n.id)).toEqual(["X"]);
|
||||
expect(l?.hasChildren).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -61,11 +61,48 @@ export function useTreeMutation(spaceId: string): UseTreeMutation {
|
||||
if (!source) return;
|
||||
const oldParentId = source.parentPageId ?? null;
|
||||
|
||||
// optimistic apply with the new position from the payload
|
||||
let optimistic = treeModel.update(after, sourceId, {
|
||||
position: payload.position,
|
||||
parentPageId: payload.parentPageId,
|
||||
} as Partial<SpaceTreeNode>);
|
||||
// Child-loss guard (#523, twin of #525's realtime `insertByPosition` fix).
|
||||
// We no longer auto-expand the make-child target on drop, so the old
|
||||
// `onToggle(target, true)` — which was ALSO the only trigger of the
|
||||
// corrective lazy-load — is gone. `treeModel.move` materialized
|
||||
// `target.children = [source]` (only the moved node); if the target is an
|
||||
// UNLOADED branch (server has children but none are loaded here), keeping
|
||||
// that partial `[source]` list would defeat the lazy-load gate and hide the
|
||||
// target's OTHER server children (the #159 #1 data-loss class). So for an
|
||||
// unloaded make-child target, build the optimistic tree WITHOUT
|
||||
// materializing source under it: just remove source from its old parent and
|
||||
// flag the target `hasChildren`. The gate stays armed and a later manual
|
||||
// expand fetches the FULL set (incl. the moved page, which the awaited
|
||||
// server move persists). Predicate is the gate's (`isUnloadedBranch`), NOT
|
||||
// `insertByPosition`'s old `=== undefined` (canonical unloaded is `[]`).
|
||||
const target =
|
||||
op.kind === "make-child"
|
||||
? (treeModel.find(before, op.targetId) as SpaceTreeNode | null)
|
||||
: null;
|
||||
const unloadedMakeChild =
|
||||
op.kind === "make-child" && treeModel.isUnloadedBranch(target);
|
||||
|
||||
let optimistic: SpaceTreeNode[];
|
||||
if (unloadedMakeChild) {
|
||||
// Do NOT materialize [source] into the unloaded target.
|
||||
optimistic = treeModel.remove(before, sourceId);
|
||||
optimistic = treeModel.update(optimistic, op.targetId, {
|
||||
hasChildren: true,
|
||||
} as Partial<SpaceTreeNode>);
|
||||
} else {
|
||||
// optimistic apply with the new position from the payload
|
||||
optimistic = treeModel.update(after, sourceId, {
|
||||
position: payload.position,
|
||||
parentPageId: payload.parentPageId,
|
||||
} as Partial<SpaceTreeNode>);
|
||||
// For make-child onto a previously-childless (loaded) target: flip
|
||||
// hasChildren on so the new parent shows its chevron.
|
||||
if (op.kind === "make-child") {
|
||||
optimistic = treeModel.update(optimistic, op.targetId, {
|
||||
hasChildren: true,
|
||||
} as Partial<SpaceTreeNode>);
|
||||
}
|
||||
}
|
||||
|
||||
// If the old parent has no children left, mark hasChildren: false so the
|
||||
// chevron disappears. Without this, the empty parent keeps rendering an
|
||||
@@ -79,14 +116,6 @@ export function useTreeMutation(spaceId: string): UseTreeMutation {
|
||||
}
|
||||
}
|
||||
|
||||
// For make-child onto a previously-childless target: flip hasChildren on
|
||||
// so the new parent shows its chevron.
|
||||
if (op.kind === "make-child") {
|
||||
optimistic = treeModel.update(optimistic, op.targetId, {
|
||||
hasChildren: true,
|
||||
} as Partial<SpaceTreeNode>);
|
||||
}
|
||||
|
||||
setData(optimistic);
|
||||
|
||||
try {
|
||||
|
||||
@@ -150,6 +150,45 @@
|
||||
);
|
||||
}
|
||||
|
||||
/* "A page just moved in here" cue (#523). A make-child drop no longer expands
|
||||
the collapsed target, so the moved page is momentarily invisible; pulse the
|
||||
target row in a distinct teal (NOT the blue make-child highlight, NOT the
|
||||
neutral post-move flash) so the landing is noticeable while the node stays
|
||||
collapsed. Two short pulses fit inside DROP_LANDED_HIGHLIGHT_MS (1.8s), after
|
||||
which the row clears the attribute and the animation stops. */
|
||||
@keyframes landedChildPulse {
|
||||
0% {
|
||||
background-color: light-dark(
|
||||
var(--mantine-color-teal-2),
|
||||
rgba(45, 212, 191, 0.30)
|
||||
);
|
||||
outline-color: light-dark(
|
||||
var(--mantine-color-teal-6),
|
||||
var(--mantine-color-teal-5)
|
||||
);
|
||||
}
|
||||
100% {
|
||||
background-color: transparent;
|
||||
outline-color: transparent;
|
||||
}
|
||||
}
|
||||
|
||||
.node[data-landed-child="true"] {
|
||||
outline: 2px solid transparent;
|
||||
outline-offset: -1px;
|
||||
animation: landedChildPulse 0.9s ease-out 2;
|
||||
}
|
||||
|
||||
@media (prefers-reduced-motion: reduce) {
|
||||
.node[data-landed-child="true"] {
|
||||
animation: none;
|
||||
background-color: light-dark(
|
||||
var(--mantine-color-teal-1),
|
||||
rgba(45, 212, 191, 0.18)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
.dropLine {
|
||||
position: absolute;
|
||||
left: var(--drop-line-indent, 0);
|
||||
|
||||
@@ -736,6 +736,53 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
|
||||
expect(historyQueue.remove).toHaveBeenCalledWith(PAGE_ID);
|
||||
expect(historyQueue.remove).not.toHaveBeenCalledWith(SLUG);
|
||||
});
|
||||
|
||||
// #370 F2 — an effectively-empty page is a REACHABLE no-op (agent calls
|
||||
// save_page_version on a blank page): the version tx short-circuits with
|
||||
// nothing to pin. The handler MUST still broadcast a TERMINAL reply
|
||||
// (version.skipped, reason:'empty') so the client resolves at once instead of
|
||||
// waiting out its 20s ack timeout and misreporting a healthy server as
|
||||
// unreachable. MUTATION: drop the `else if (skipped)` broadcast → no terminal
|
||||
// reply is sent → this reddens.
|
||||
it('empty page → no version written, broadcasts a terminal version.skipped(empty)', async () => {
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: emptyDoc,
|
||||
});
|
||||
|
||||
await emitSave(document, 'agent');
|
||||
|
||||
// Nothing pinned.
|
||||
expect(pageHistoryRepo.saveHistory).not.toHaveBeenCalled();
|
||||
expect(pageHistoryRepo.updateHistoryKind).not.toHaveBeenCalled();
|
||||
// But a terminal reply WAS sent so the client never times out. The flush
|
||||
// (onStoreDocument) emits its own `page.updated`; the version.skipped is the
|
||||
// LAST broadcast (dropping the skip branch leaves page.updated last → reds).
|
||||
const calls = (document as any).broadcastStateless.mock.calls;
|
||||
const msg = JSON.parse(calls[calls.length - 1][0]);
|
||||
expect(msg).toEqual({ type: 'version.skipped', reason: 'empty' });
|
||||
});
|
||||
|
||||
// #370 F2 — the page row is gone (deleted / never persisted). Same rule: a
|
||||
// terminal reply MUST be sent (version.skipped, reason:'page-not-found') so the
|
||||
// client surfaces a truthful "not found" immediately rather than a health
|
||||
// timeout. onStoreDocument's own `!page` guard returns early without throwing,
|
||||
// so the handler reaches the version tx and its `!page` skip branch.
|
||||
it('page not found → broadcasts a terminal version.skipped(page-not-found)', async () => {
|
||||
const document = ydocFor(doc('GONE'));
|
||||
pageRepo.findById.mockResolvedValue(null);
|
||||
|
||||
await emitSave(document, 'agent');
|
||||
|
||||
expect(pageHistoryRepo.saveHistory).not.toHaveBeenCalled();
|
||||
expect((document as any).broadcastStateless).toHaveBeenCalledTimes(1);
|
||||
const msg = JSON.parse(
|
||||
(document as any).broadcastStateless.mock.calls[0][0],
|
||||
);
|
||||
expect(msg).toEqual({ type: 'version.skipped', reason: 'page-not-found' });
|
||||
});
|
||||
});
|
||||
|
||||
// #370 — the in-memory idle-burst marker must be dropped on doc unload (like
|
||||
|
||||
@@ -68,6 +68,19 @@ export const INTENTIONAL_CLEAR_MESSAGE_TYPE = 'intentional-clear';
|
||||
*/
|
||||
export const SAVE_VERSION_MESSAGE_TYPE = 'save-version';
|
||||
|
||||
/**
|
||||
* #370 F2 — wire format of the server→client REPLY to a save-version signal, sent
|
||||
* over the same stateless channel. `version.saved` means a version was created or
|
||||
* promoted; `version.skipped` is a TERMINAL "nothing was pinned" reply for the two
|
||||
* reachable no-op cases (an effectively-empty page, or the page row is gone) so the
|
||||
* client resolves at once instead of waiting out its ack timeout and misreporting a
|
||||
* healthy server as unreachable. EXACTLY ONE of these is broadcast per handled
|
||||
* save. The MCP client duplicates these literals (it cannot import server code) —
|
||||
* keep the two in sync (see packages/mcp/src/lib/collaboration.ts).
|
||||
*/
|
||||
export const VERSION_SAVED_MESSAGE_TYPE = 'version.saved';
|
||||
export const VERSION_SKIPPED_MESSAGE_TYPE = 'version.skipped';
|
||||
|
||||
/**
|
||||
* #251 — how long an intentional-clear signal stays "pending" before it is
|
||||
* ignored. The signal is set on the clearing keystroke but consumed by the
|
||||
@@ -643,6 +656,10 @@ export class PersistenceExtension implements Extension {
|
||||
let result:
|
||||
| { historyId: string; kind: PageHistoryKind; alreadySaved: boolean }
|
||||
| undefined;
|
||||
// #370 F2 — set when there is nothing to version (empty page / page gone), so
|
||||
// the tail broadcasts a terminal `version.skipped` instead of staying silent
|
||||
// and forcing the client to time out. Mutually exclusive with `result`.
|
||||
let skipped: 'empty' | 'page-not-found' | undefined;
|
||||
|
||||
// #370 F8-twin — the contributor set popped from Redis (destructive SPOP)
|
||||
// must be restored if the version row does not durably land. The inner
|
||||
@@ -668,11 +685,20 @@ export class PersistenceExtension implements Extension {
|
||||
includeContent: true,
|
||||
trx,
|
||||
});
|
||||
if (!page) return;
|
||||
if (!page) {
|
||||
// The page row is gone (deleted/never persisted). Nothing to version —
|
||||
// record it so the tail sends a terminal skip reply (#370 F2).
|
||||
skipped = 'page-not-found';
|
||||
return;
|
||||
}
|
||||
versionedPageId = page.id;
|
||||
// Never version an effectively-empty page (mirrors the processor's
|
||||
// first-history guard); there is nothing intentional to pin.
|
||||
if (isEmptyParagraphDoc(page.content as any)) return;
|
||||
// first-history guard); there is nothing intentional to pin. Record the
|
||||
// skip so the client gets a terminal reply rather than a timeout (#370 F2).
|
||||
if (isEmptyParagraphDoc(page.content as any)) {
|
||||
skipped = 'empty';
|
||||
return;
|
||||
}
|
||||
|
||||
const lastHistory = await this.pageHistoryRepo.findPageLastHistory(
|
||||
page.id,
|
||||
@@ -747,15 +773,26 @@ export class PersistenceExtension implements Extension {
|
||||
}
|
||||
this.idleBurstStart.delete(documentName);
|
||||
|
||||
// EXACTLY ONE terminal reply per handled save (#370 F2): a real save/promote
|
||||
// broadcasts `version.saved`; the two no-op early returns (empty page / page
|
||||
// gone) broadcast `version.skipped` so the client never waits out its ack
|
||||
// timeout. A genuine failure threw above and rejected before reaching here.
|
||||
if (result) {
|
||||
document.broadcastStateless(
|
||||
JSON.stringify({
|
||||
type: 'version.saved',
|
||||
type: VERSION_SAVED_MESSAGE_TYPE,
|
||||
historyId: result.historyId,
|
||||
kind: result.kind,
|
||||
alreadySaved: result.alreadySaved,
|
||||
}),
|
||||
);
|
||||
} else if (skipped) {
|
||||
document.broadcastStateless(
|
||||
JSON.stringify({
|
||||
type: VERSION_SKIPPED_MESSAGE_TYPE,
|
||||
reason: skipped,
|
||||
}),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -32,6 +32,14 @@ describe('EmbeddingIndexerService.reindexWorkspace fail-fast', () => {
|
||||
const pageEmbeddingRepo = {};
|
||||
const aiService = {
|
||||
getEmbeddingModel: jest.fn().mockResolvedValue('some-model'),
|
||||
// #530: reindexWorkspace's pre-check now resolves the provider (workspace
|
||||
// or global). Resolve it so the batch control flow under test proceeds.
|
||||
resolveEmbeddingProvider: jest.fn().mockResolvedValue({
|
||||
model: 'some-model',
|
||||
queryPrefix: '',
|
||||
docPrefix: '',
|
||||
fingerprint: 'fp-test',
|
||||
}),
|
||||
};
|
||||
// Progress is a best-effort cosmetic store; mock its async methods so the
|
||||
// batch control flow can be tested without Redis.
|
||||
@@ -108,6 +116,14 @@ describe('EmbeddingIndexerService.reindexWorkspace progress', () => {
|
||||
const pageEmbeddingRepo = {};
|
||||
const aiService = {
|
||||
getEmbeddingModel: jest.fn().mockResolvedValue('some-model'),
|
||||
// #530: reindexWorkspace's pre-check now resolves the provider (workspace
|
||||
// or global). Resolve it so the batch control flow under test proceeds.
|
||||
resolveEmbeddingProvider: jest.fn().mockResolvedValue({
|
||||
model: 'some-model',
|
||||
queryPrefix: '',
|
||||
docPrefix: '',
|
||||
fingerprint: 'fp-test',
|
||||
}),
|
||||
};
|
||||
const reindexProgress = {
|
||||
start: jest.fn().mockResolvedValue(undefined),
|
||||
@@ -174,7 +190,7 @@ describe('EmbeddingIndexerService.reindexWorkspace progress', () => {
|
||||
const { service, aiService, reindexProgress } = makeService();
|
||||
// Embeddings not configured: reindexWorkspace returns early WITHOUT starting
|
||||
// a fresh record, but the finally must still clear the enqueue-time seed.
|
||||
aiService.getEmbeddingModel = jest
|
||||
aiService.resolveEmbeddingProvider = jest
|
||||
.fn()
|
||||
.mockRejectedValue(new AiEmbeddingNotConfiguredException());
|
||||
|
||||
@@ -187,3 +203,87 @@ describe('EmbeddingIndexerService.reindexWorkspace progress', () => {
|
||||
expect(reindexProgress.clear).toHaveBeenCalledWith(WORKSPACE_ID);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* #530 PR-1: reindexPage must (a) prepend the provider's DOC prefix to each chunk
|
||||
* BEFORE embedding (so stored vectors live in the same prefixed space as a
|
||||
* prefixed query), and (b) stamp the ACTIVE fingerprint on every inserted row (so
|
||||
* search only fuses same-generation vectors). Uses lightweight mocks; the tx is
|
||||
* stubbed to run its callback inline.
|
||||
*/
|
||||
describe('EmbeddingIndexerService.reindexPage doc-prefix + fingerprint (#530)', () => {
|
||||
const WORKSPACE_ID = 'ws-1';
|
||||
const SPACE_ID = 'space-1';
|
||||
const PAGE_ID = 'page-1';
|
||||
|
||||
function makeService(docPrefix: string) {
|
||||
const pageRepo = {
|
||||
findById: jest.fn().mockResolvedValue({
|
||||
id: PAGE_ID,
|
||||
workspaceId: WORKSPACE_ID,
|
||||
spaceId: SPACE_ID,
|
||||
title: 'Заголовок',
|
||||
// No ProseMirror content -> the plain-text fallback path (single chunk).
|
||||
content: null,
|
||||
textContent: 'простой текст страницы',
|
||||
deletedAt: null,
|
||||
}),
|
||||
};
|
||||
const insertChunks = jest.fn().mockResolvedValue(undefined);
|
||||
const pageEmbeddingRepo = {
|
||||
deleteByPage: jest.fn().mockResolvedValue(undefined),
|
||||
insertChunks,
|
||||
};
|
||||
const embedWithModel = jest.fn().mockResolvedValue([[0.1, 0.2, 0.3]]);
|
||||
const aiService = {
|
||||
resolveEmbeddingProvider: jest.fn().mockResolvedValue({
|
||||
model: { modelId: 'e5-small' },
|
||||
queryPrefix: 'query: ',
|
||||
docPrefix,
|
||||
fingerprint: 'fp-gen-1',
|
||||
}),
|
||||
embedWithModel,
|
||||
};
|
||||
const reindexProgress = {};
|
||||
// Stub the tx so executeTx runs its callback inline against a fake trx.
|
||||
const db = {
|
||||
transaction: () => ({ execute: (cb: any) => cb({}) }),
|
||||
};
|
||||
const service = new EmbeddingIndexerService(
|
||||
pageRepo as unknown as PageRepo,
|
||||
pageEmbeddingRepo as unknown as PageEmbeddingRepo,
|
||||
aiService as unknown as AiService,
|
||||
reindexProgress as unknown as EmbeddingReindexProgressService,
|
||||
db as unknown as KyselyDB,
|
||||
);
|
||||
return { service, embedWithModel, insertChunks };
|
||||
}
|
||||
|
||||
it('prepends the doc prefix to each chunk and stamps the fingerprint on rows', async () => {
|
||||
const { service, embedWithModel, insertChunks } = makeService('passage: ');
|
||||
await service.reindexPage(PAGE_ID);
|
||||
|
||||
// Embedded values are DOC-prefixed; the model is the resolved provider model.
|
||||
expect(embedWithModel).toHaveBeenCalledTimes(1);
|
||||
const [modelArg, wsArg, valuesArg] = embedWithModel.mock.calls[0];
|
||||
expect(modelArg).toEqual({ modelId: 'e5-small' });
|
||||
expect(wsArg).toBe(WORKSPACE_ID);
|
||||
expect(valuesArg).toEqual(['passage: простой текст страницы']);
|
||||
|
||||
// Inserted rows carry the active fingerprint and the ORIGINAL (un-prefixed)
|
||||
// content (the prefix is an embedding-space artifact, not stored text).
|
||||
expect(insertChunks).toHaveBeenCalledTimes(1);
|
||||
const rows = insertChunks.mock.calls[0][0];
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(rows[0].fingerprint).toBe('fp-gen-1');
|
||||
expect(rows[0].content).toBe('простой текст страницы');
|
||||
expect(rows[0].modelName).toBe('e5-small');
|
||||
});
|
||||
|
||||
it('does not prefix when the provider has an empty doc prefix', async () => {
|
||||
const { service, embedWithModel } = makeService('');
|
||||
await service.reindexPage(PAGE_ID);
|
||||
const [, , valuesArg] = embedWithModel.mock.calls[0];
|
||||
expect(valuesArg).toEqual(['простой текст страницы']);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -108,15 +108,24 @@ export class EmbeddingIndexerService {
|
||||
return;
|
||||
}
|
||||
|
||||
// Resolve embeddings config WITHOUT crashing the queue when unconfigured.
|
||||
// Resolve the embeddings provider WITHOUT crashing the queue when
|
||||
// unconfigured. #530: resolveEmbeddingProvider prefers the workspace provider
|
||||
// and falls back to the GLOBAL env provider (TEI sidecar), and yields the
|
||||
// doc-prefix + the active fingerprint stored per row so search filters by the
|
||||
// active generation.
|
||||
let modelName = 'unknown';
|
||||
let provider: Awaited<
|
||||
ReturnType<AiService['resolveEmbeddingProvider']>
|
||||
>;
|
||||
try {
|
||||
const model = await this.aiService.getEmbeddingModel(workspaceId);
|
||||
provider = await this.aiService.resolveEmbeddingProvider(workspaceId);
|
||||
// Record the model id per row so a future migration can detect + re-index
|
||||
// rows produced by a different model (see the migration header). The SDK
|
||||
// type is `string | EmbeddingModel{V2,V3}`; model objects carry `modelId`.
|
||||
modelName =
|
||||
typeof model === 'string' ? model : (model.modelId ?? 'unknown');
|
||||
typeof provider.model === 'string'
|
||||
? provider.model
|
||||
: (provider.model.modelId ?? 'unknown');
|
||||
} catch (err) {
|
||||
if (err instanceof AiEmbeddingNotConfiguredException) {
|
||||
// No embeddings provider for this workspace: NO-OP (§6.7). The page can
|
||||
@@ -145,8 +154,18 @@ export class EmbeddingIndexerService {
|
||||
return;
|
||||
}
|
||||
|
||||
// Embed all chunks in one batch.
|
||||
const vectors = await this.aiService.embedTexts(workspaceId, chunks);
|
||||
// #530: prepend the provider's DOC prefix to each chunk (e5-style
|
||||
// "passage: "; empty for a non-e5 provider) so stored vectors live in the
|
||||
// same prefixed space as a prefixed query, then embed with the RESOLVED
|
||||
// provider model (which may be the global TEI sidecar, not a workspace one).
|
||||
const prefixedChunks = provider.docPrefix
|
||||
? chunks.map((c) => provider.docPrefix + c)
|
||||
: chunks;
|
||||
const vectors = await this.aiService.embedWithModel(
|
||||
provider.model,
|
||||
workspaceId,
|
||||
prefixedChunks,
|
||||
);
|
||||
|
||||
// The column is dimension-agnostic, so ANY model dimension is stored as-is.
|
||||
// Defensive sanity check only: all chunks of ONE page come from the SAME
|
||||
@@ -170,6 +189,7 @@ export class EmbeddingIndexerService {
|
||||
vectors,
|
||||
{ pageId, workspaceId, spaceId },
|
||||
modelName,
|
||||
provider.fingerprint,
|
||||
);
|
||||
|
||||
// HARD replace in one transaction: delete then insert so search never
|
||||
@@ -216,7 +236,9 @@ export class EmbeddingIndexerService {
|
||||
// (seeded at enqueue time); the finally cleans that too.
|
||||
try {
|
||||
try {
|
||||
await this.aiService.getEmbeddingModel(workspaceId);
|
||||
// #530: resolve via the same path reindexPage uses (workspace provider,
|
||||
// else the global TEI sidecar) so a global-only deployment is NOT skipped.
|
||||
await this.aiService.resolveEmbeddingProvider(workspaceId);
|
||||
} catch (err) {
|
||||
if (err instanceof AiEmbeddingNotConfiguredException) {
|
||||
this.logger.log(
|
||||
@@ -348,6 +370,7 @@ export class EmbeddingIndexerService {
|
||||
vectors: number[][],
|
||||
ids: { pageId: string; workspaceId: string; spaceId: string },
|
||||
modelName: string,
|
||||
fingerprint: string | null,
|
||||
): PageEmbeddingChunkRow[] {
|
||||
const rows: PageEmbeddingChunkRow[] = [];
|
||||
let cursor = 0;
|
||||
@@ -370,6 +393,8 @@ export class EmbeddingIndexerService {
|
||||
// Provenance for a future re-index sweep on model change.
|
||||
modelName,
|
||||
modelDimensions: embedding.length,
|
||||
// #530: the active generation fingerprint this row belongs to.
|
||||
fingerprint,
|
||||
embedding,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -106,6 +106,7 @@ function __assertClientCallContract(client: DocmostClientLike): void {
|
||||
void client.sharePage(s, true);
|
||||
void client.unsharePage(s);
|
||||
void client.restorePageVersion(s);
|
||||
void client.savePageVersion(s);
|
||||
void client.transformPage(s, s, { dryRun: true });
|
||||
void client.stashPage(s);
|
||||
// --- write (image / footnote), in-app since #410 ---
|
||||
|
||||
@@ -58,6 +58,7 @@ type DocmostClientMethod =
|
||||
| 'sharePage'
|
||||
| 'unsharePage'
|
||||
| 'restorePageVersion'
|
||||
| 'savePageVersion'
|
||||
| 'transformPage'
|
||||
| 'stashPage'
|
||||
// --- write (image / footnote), in-app since #410 ---
|
||||
|
||||
@@ -32,10 +32,27 @@ export class SearchResultDto {
|
||||
matchedTerms: string[];
|
||||
}
|
||||
|
||||
// The paginated envelope (A5). `total` is the EXACT permission-filtered count of
|
||||
// pages matching the positive lexical query (fail-closed). `hasMore` is true when
|
||||
// more results exist WITHIN the fusion window; `truncatedAtCap` signals the match
|
||||
// set exceeded CANDIDATE_CAP and the tail is unreachable by pagination.
|
||||
// #530 Phase B — the semantic (vector) layer's per-request status.
|
||||
// - `state`: 'full' when a provider resolved and the vector arm ran this
|
||||
// request; 'off' otherwise (kill-switch, no provider, or a degrade).
|
||||
// - `available`: whether the vector arm actually contributed this request.
|
||||
// - `reason`: why the arm did NOT run — 'no-provider' (no embedding provider
|
||||
// resolved) or 'degraded' (sidecar down / query embed timed out).
|
||||
// - `indexed`/`total`: reserved for PR-2 coverage reporting (unused in PR-1).
|
||||
export class SearchSemanticDto {
|
||||
state: 'full' | 'off';
|
||||
available: boolean;
|
||||
reason?: 'no-provider' | 'degraded';
|
||||
indexed?: number;
|
||||
total?: number;
|
||||
}
|
||||
|
||||
// The paginated envelope (A5). `total` is the permission-filtered count of the
|
||||
// candidate UNION (lexical ∪ vector top-N, fail-closed) — a deliberate, #530
|
||||
// documented change from Phase A's exact-lexical count (on a semantic degrade it
|
||||
// falls back to exactly that lexical count). `hasMore` is true when more results
|
||||
// exist WITHIN the fusion window; `truncatedAtCap` signals the match set exceeded
|
||||
// CANDIDATE_CAP and the tail is unreachable by pagination.
|
||||
export class SearchResponseDto {
|
||||
items: SearchResultDto[];
|
||||
total: number;
|
||||
@@ -53,4 +70,7 @@ export class SearchResponseDto {
|
||||
mode: 'or' | 'and';
|
||||
match: string;
|
||||
};
|
||||
// Absent on the early-exit paths (empty/garbage query); present once search
|
||||
// actually runs. Optional so those short-circuit responses stay valid.
|
||||
semantic?: SearchSemanticDto;
|
||||
}
|
||||
|
||||
@@ -1,8 +1,15 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { SearchController } from './search.controller';
|
||||
import { SearchService } from './search.service';
|
||||
import { AiModule } from '../../integrations/ai/ai.module';
|
||||
|
||||
/**
|
||||
* #530: AiModule supplies AiService (embedQuery / resolveEmbeddingProvider).
|
||||
* PageEmbeddingRepo is provided by the @Global DatabaseModule, so it is injected
|
||||
* into SearchService without an explicit import here.
|
||||
*/
|
||||
@Module({
|
||||
imports: [AiModule],
|
||||
controllers: [SearchController],
|
||||
providers: [SearchService],
|
||||
exports: [SearchService],
|
||||
|
||||
@@ -48,6 +48,10 @@ describe('SearchService.searchPage — scope-security early returns', () => {
|
||||
shareRepo as any,
|
||||
spaceMemberRepo as any,
|
||||
pagePermissionRepo as any,
|
||||
// #530 semantic path off for these query-mode unit tests: a stubbed
|
||||
// embedQuery that throws makes the service degrade to the lexical path.
|
||||
{ embedQuery: jest.fn().mockRejectedValue(new Error('no embed')) } as any,
|
||||
{ vectorCandidateArm: jest.fn() } as any,
|
||||
);
|
||||
return { service, pageRepo, shareRepo, spaceMemberRepo, pagePermissionRepo };
|
||||
}
|
||||
|
||||
@@ -11,6 +11,8 @@ describe('SearchService', () => {
|
||||
{} as any, // shareRepo
|
||||
{} as any, // spaceMemberRepo
|
||||
{} as any, // pagePermissionRepo
|
||||
{} as any, // aiService
|
||||
{} as any, // pageEmbeddingRepo
|
||||
);
|
||||
expect(service).toBeDefined();
|
||||
});
|
||||
@@ -61,6 +63,8 @@ describe('SearchService.searchSuggestions — onlyTemplates filter', () => {
|
||||
shareRepo as any,
|
||||
spaceMemberRepo as any,
|
||||
pagePermissionRepo as any,
|
||||
{} as any, // aiService (searchSuggestions never embeds)
|
||||
{} as any, // pageEmbeddingRepo
|
||||
);
|
||||
|
||||
return { service, db, pageBuilder };
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { Injectable, Logger } from '@nestjs/common';
|
||||
import { SearchDTO, SearchSuggestionDTO } from './dto/search.dto';
|
||||
import {
|
||||
SearchResponseDto,
|
||||
SearchResultDto,
|
||||
SearchSemanticDto,
|
||||
} from './dto/search-response.dto';
|
||||
import { InjectKysely } from 'nestjs-kysely';
|
||||
import { KyselyDB } from '@docmost/db/types/kysely.types';
|
||||
@@ -11,6 +12,10 @@ import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
||||
import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo';
|
||||
import { ShareRepo } from '@docmost/db/repos/share/share.repo';
|
||||
import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo';
|
||||
import { PageEmbeddingRepo } from '@docmost/db/repos/ai-chat/page-embedding.repo';
|
||||
import { AiService } from '../../integrations/ai/ai.service';
|
||||
import { AiEmbeddingNotConfiguredException } from '../../integrations/ai/ai-embedding-not-configured.exception';
|
||||
import { isStatementTimeout } from '@docmost/db/utils';
|
||||
import {
|
||||
ParsedQuery,
|
||||
ParsedTerm,
|
||||
@@ -59,14 +64,42 @@ function defaultBooleanMode(): SearchBooleanMode {
|
||||
return process.env.SEARCH_MODE === 'and' ? 'and' : 'or';
|
||||
}
|
||||
|
||||
// #530 semantic env knobs. SEARCH_SEMANTIC=off is the kill-switch (hybrid is
|
||||
// otherwise transparent — no per-request mode flag). SEARCH_VECTOR_WEIGHT tunes
|
||||
// the vector RRF leg's contribution (default 1.0, i.e. equal to each lexical
|
||||
// leg). SEARCH_VECTOR_CANDIDATES caps the vector top-N pulled into the union.
|
||||
function semanticKillSwitchOff(): boolean {
|
||||
return process.env.SEARCH_SEMANTIC === 'off';
|
||||
}
|
||||
function getVectorWeight(): number {
|
||||
const raw = Number(process.env.SEARCH_VECTOR_WEIGHT);
|
||||
return Number.isFinite(raw) && raw >= 0 ? raw : 1.0;
|
||||
}
|
||||
function getVectorCandidates(): number {
|
||||
const raw = Number(process.env.SEARCH_VECTOR_CANDIDATES);
|
||||
return Number.isFinite(raw) && raw > 0 ? Math.floor(raw) : 50;
|
||||
}
|
||||
// Safety net for the brute-force vector scan (no ANN index — see
|
||||
// vectorCandidateArm): a per-statement timeout bounding ONLY the fused vector
|
||||
// query, so a pathological seq scan is cancelled (SQLSTATE 57014) and search
|
||||
// degrades to lexical instead of hanging the request. Default 2000ms.
|
||||
function getVectorStatementTimeoutMs(): number {
|
||||
const raw = Number(process.env.SEARCH_VECTOR_STATEMENT_TIMEOUT_MS);
|
||||
return Number.isFinite(raw) && raw > 0 ? Math.floor(raw) : 2000;
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
export class SearchService {
|
||||
private readonly logger = new Logger(SearchService.name);
|
||||
|
||||
constructor(
|
||||
@InjectKysely() private readonly db: KyselyDB,
|
||||
private pageRepo: PageRepo,
|
||||
private shareRepo: ShareRepo,
|
||||
private spaceMemberRepo: SpaceMemberRepo,
|
||||
private pagePermissionRepo: PagePermissionRepo,
|
||||
private aiService: AiService,
|
||||
private pageEmbeddingRepo: PageEmbeddingRepo,
|
||||
) {}
|
||||
|
||||
// === #529 SQL fragment builders (parameterized AST, never string-concat) =====
|
||||
@@ -292,33 +325,102 @@ export class SearchService {
|
||||
const scopeSql = sql`(${sql.join(scopePreds, sql` AND `)})`;
|
||||
const candidateSql = sql`(${scopeSql} AND ${this.candidatePredicate(parsed, titleOnly)})`;
|
||||
|
||||
// --- Semantic (vector) arm: embed the query, degrade gracefully. (#530) ----
|
||||
// The try/catch wraps ONLY embedQuery + vector-arm construction. On ANY throw
|
||||
// (TEI down / 800ms timeout / no provider) we omit the vector arm entirely and
|
||||
// run the byte-identical Phase-A lexical path — never a 500 from the sidecar.
|
||||
// The permission filter below stays OUTSIDE this try (a permission error must
|
||||
// 500, never fail-open). Hybrid is transparent; SEARCH_SEMANTIC=off disables it.
|
||||
let vectorArm: RawBuilder<unknown> | null = null;
|
||||
let semanticAvailable = false;
|
||||
let semanticReason: 'no-provider' | 'degraded' | undefined;
|
||||
if (!semanticKillSwitchOff()) {
|
||||
try {
|
||||
const { vector, fingerprint } = await this.aiService.embedQuery(
|
||||
opts.workspaceId,
|
||||
rawQuery,
|
||||
);
|
||||
vectorArm = this.pageEmbeddingRepo.vectorCandidateArm({
|
||||
queryEmbedding: vector,
|
||||
dimensions: vector.length,
|
||||
fingerprint,
|
||||
scope: scopeSql,
|
||||
limit: getVectorCandidates(),
|
||||
// Filter page_embeddings by (immutable) workspace_id directly, so the
|
||||
// composite index bites on its leading column and candidates are
|
||||
// workspace-scoped at the embedding level (#530 review WARNING 2). Space
|
||||
// scoping stays on the pages join only — page_embeddings.space_id can be
|
||||
// STALE after a cross-space move, so filtering it here would drop a moved
|
||||
// page's vector hit (re-review regression fix).
|
||||
workspaceId: scope.workspaceId,
|
||||
});
|
||||
semanticAvailable = true;
|
||||
} catch (err) {
|
||||
if (err instanceof AiEmbeddingNotConfiguredException) {
|
||||
// No embedding provider is the DEFAULT state of any deployment without
|
||||
// the TEI sidecar — normal, not a problem. Log at DEBUG so it never
|
||||
// floods WARN once per search request.
|
||||
semanticReason = 'no-provider';
|
||||
this.logger.debug(
|
||||
`search.semantic.no-provider workspace=${opts.workspaceId}`,
|
||||
);
|
||||
} else {
|
||||
// A provider IS configured but the embed call failed/timed out — a real
|
||||
// sidecar problem worth a WARN. Structured, greppable event (fixed code)
|
||||
// — no query text/secrets.
|
||||
semanticReason = 'degraded';
|
||||
this.logger.warn(
|
||||
`search.semantic.degraded reason=degraded workspace=${opts.workspaceId}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
// --- Ranked candidate ids (ALL matches, RRF order). -----------------------
|
||||
// Two independent branches ranked SEPARATELY (FTS by ts_rank_cd, substring by
|
||||
// tier) then fused with RRF: score = Σ 1/(k + rank_branch). Deterministic
|
||||
// ORDER BY rrf DESC, id so pagination never dupes/skips (A4, acceptance #8).
|
||||
const rankedRows = await sql<{ id: string }>`
|
||||
WITH candidates AS (
|
||||
SELECT pages.id AS id,
|
||||
${this.ftsScoreExpr(parsed)} AS fts_score,
|
||||
${this.subTierExpr(parsed, titleOnly)} AS sub_tier
|
||||
FROM pages
|
||||
WHERE ${candidateSql}
|
||||
),
|
||||
ranked AS (
|
||||
SELECT id, fts_score, sub_tier,
|
||||
row_number() OVER (ORDER BY fts_score DESC NULLS LAST, id) AS rn_fts,
|
||||
row_number() OVER (ORDER BY sub_tier DESC NULLS LAST, id) AS rn_sub
|
||||
FROM candidates
|
||||
)
|
||||
SELECT id
|
||||
FROM ranked
|
||||
ORDER BY
|
||||
(CASE WHEN fts_score IS NOT NULL THEN 1.0/(${RRF_K} + rn_fts) ELSE 0 END)
|
||||
+ (CASE WHEN sub_tier IS NOT NULL THEN 1.0/(${RRF_K} + rn_sub) ELSE 0 END) DESC,
|
||||
id ASC
|
||||
`.execute(this.db);
|
||||
// Lexical branches ranked SEPARATELY (FTS by ts_rank_cd, substring by tier);
|
||||
// when a query vector is available a third VECTOR branch is fused over the
|
||||
// UNION of lexical + vector candidates. RRF: score = Σ w/(k + rank_branch).
|
||||
// Deterministic ORDER BY rrf DESC, id so pagination never dupes/skips (A4,
|
||||
// acceptance #8). `total` is thus the union size (lexical ∪ vector top-N),
|
||||
// post-permission-filter — a documented change from Phase A's exact lexical
|
||||
// count (#530). On degrade the query is byte-identical to Phase A.
|
||||
//
|
||||
// The fused query is bounded by a per-statement timeout (see runRankedQuery):
|
||||
// if the brute-force vector scan is cancelled (SQLSTATE 57014) we degrade to
|
||||
// the byte-identical lexical path — search returns lexical results, never
|
||||
// hangs, never 500s. Only the timeout is caught here; any OTHER SQL error
|
||||
// propagates. The permission filter below stays outside, unchanged.
|
||||
let orderedIds: string[];
|
||||
try {
|
||||
orderedIds = await this.runRankedQuery(
|
||||
candidateSql,
|
||||
parsed,
|
||||
titleOnly,
|
||||
vectorArm,
|
||||
);
|
||||
} catch (err) {
|
||||
if (vectorArm && isStatementTimeout(err)) {
|
||||
semanticAvailable = false;
|
||||
semanticReason = 'degraded';
|
||||
this.logger.warn(
|
||||
`search.semantic.degraded reason=degraded workspace=${opts.workspaceId} (vector statement timeout)`,
|
||||
);
|
||||
// Re-run the exact Phase-A lexical path (no vector arm, no timeout).
|
||||
orderedIds = await this.runRankedQuery(
|
||||
candidateSql,
|
||||
parsed,
|
||||
titleOnly,
|
||||
null,
|
||||
);
|
||||
} else {
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
let orderedIds = rankedRows.rows.map((r) => r.id);
|
||||
const semantic: SearchSemanticDto = {
|
||||
state: semanticAvailable ? 'full' : 'off',
|
||||
available: semanticAvailable,
|
||||
...(semanticReason ? { reason: semanticReason } : {}),
|
||||
};
|
||||
|
||||
// --- Permission filter (fail-closed, exact total). ------------------------
|
||||
// filterAccessiblePageIds runs the #348 hasRestricted pre-check internally:
|
||||
@@ -355,14 +457,131 @@ export class SearchService {
|
||||
const hasMore = offset + pageIds.length < window.length;
|
||||
|
||||
if (pageIds.length === 0) {
|
||||
return { items: [], total, hasMore, truncatedAtCap, offset, query: queryMeta };
|
||||
return {
|
||||
items: [],
|
||||
total,
|
||||
hasMore,
|
||||
truncatedAtCap,
|
||||
offset,
|
||||
query: queryMeta,
|
||||
semantic,
|
||||
};
|
||||
}
|
||||
|
||||
// --- Detail fetch for the page slice only (ts_headline/snippet are costly, so
|
||||
// compute them ONLY for the returned rows), preserving RRF order. -------
|
||||
const items = await this.fetchDetails(pageIds, parsed, titleOnly);
|
||||
|
||||
return { items, total, hasMore, truncatedAtCap, offset, query: queryMeta };
|
||||
return {
|
||||
items,
|
||||
total,
|
||||
hasMore,
|
||||
truncatedAtCap,
|
||||
offset,
|
||||
query: queryMeta,
|
||||
semantic,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Run the ranked-candidate-ids query and return the ids in RRF order (#530).
|
||||
*
|
||||
* When `vectorArm` is null (degrade / semantic off / no query vector) this runs
|
||||
* the BYTE-IDENTICAL Phase-A 2-branch lexical query — the semantic layer must
|
||||
* be a pure superset that never changes lexical behaviour on degrade.
|
||||
*
|
||||
* When a vector arm is supplied it is UNION ALL'd with the lexical arm into one
|
||||
* `candidates` set; `agg` collapses each page to its best per-branch signal
|
||||
* (MAX fts_score / MAX sub_tier / MIN vec_distance); `ranked` assigns a
|
||||
* per-branch row_number; the final ORDER BY fuses the three legs with RRF,
|
||||
* adding the vector leg ONLY for rows that have a vec_distance (a lexical-only
|
||||
* hit contributes 0 to the vector leg, and vice-versa). W_VEC weights the
|
||||
* vector leg.
|
||||
*/
|
||||
private async runRankedQuery(
|
||||
candidateSql: RawBuilder<unknown>,
|
||||
parsed: ParsedQuery,
|
||||
titleOnly: boolean,
|
||||
vectorArm: RawBuilder<unknown> | null,
|
||||
): Promise<string[]> {
|
||||
if (!vectorArm) {
|
||||
// Phase-A lexical path — DO NOT change (byte-identical on degrade, #529).
|
||||
const rankedRows = await sql<{ id: string }>`
|
||||
WITH candidates AS (
|
||||
SELECT pages.id AS id,
|
||||
${this.ftsScoreExpr(parsed)} AS fts_score,
|
||||
${this.subTierExpr(parsed, titleOnly)} AS sub_tier
|
||||
FROM pages
|
||||
WHERE ${candidateSql}
|
||||
),
|
||||
ranked AS (
|
||||
SELECT id, fts_score, sub_tier,
|
||||
row_number() OVER (ORDER BY fts_score DESC NULLS LAST, id) AS rn_fts,
|
||||
row_number() OVER (ORDER BY sub_tier DESC NULLS LAST, id) AS rn_sub
|
||||
FROM candidates
|
||||
)
|
||||
SELECT id
|
||||
FROM ranked
|
||||
ORDER BY
|
||||
(CASE WHEN fts_score IS NOT NULL THEN 1.0/(${RRF_K} + rn_fts) ELSE 0 END)
|
||||
+ (CASE WHEN sub_tier IS NOT NULL THEN 1.0/(${RRF_K} + rn_sub) ELSE 0 END) DESC,
|
||||
id ASC
|
||||
`.execute(this.db);
|
||||
return rankedRows.rows.map((r) => r.id);
|
||||
}
|
||||
|
||||
// #530 fused 3-branch RRF over lexical ∪ vector candidates. The lexical arm
|
||||
// adds NULL::float vec_distance to stay UNION-compatible with the vector arm.
|
||||
const wVec = getVectorWeight();
|
||||
|
||||
// Bound the brute-force vector scan with a per-statement timeout, scoped to
|
||||
// THIS query only. `set_config(..., is_local := true)` = SET LOCAL semantics:
|
||||
// it applies within this transaction and auto-resets at COMMIT/ROLLBACK, so
|
||||
// it can NEVER leak onto the next query that reuses this pooled connection.
|
||||
// On a cancellation the driver raises SQLSTATE 57014, which searchPage catches
|
||||
// to degrade to lexical-only. Running inside a transaction also guarantees the
|
||||
// SET and the ranked query share one connection.
|
||||
const timeoutMs = getVectorStatementTimeoutMs();
|
||||
return this.db.transaction().execute(async (trx) => {
|
||||
await sql`SELECT set_config('statement_timeout', ${String(timeoutMs)}, true)`.execute(
|
||||
trx,
|
||||
);
|
||||
const rankedRows = await sql<{ id: string }>`
|
||||
WITH candidates AS (
|
||||
(SELECT pages.id AS id,
|
||||
${this.ftsScoreExpr(parsed)} AS fts_score,
|
||||
${this.subTierExpr(parsed, titleOnly)} AS sub_tier,
|
||||
NULL::float AS vec_distance
|
||||
FROM pages
|
||||
WHERE ${candidateSql})
|
||||
UNION ALL
|
||||
(${vectorArm})
|
||||
),
|
||||
agg AS (
|
||||
SELECT id,
|
||||
MAX(fts_score) AS fts_score,
|
||||
MAX(sub_tier) AS sub_tier,
|
||||
MIN(vec_distance) AS vec_distance
|
||||
FROM candidates
|
||||
GROUP BY id
|
||||
),
|
||||
ranked AS (
|
||||
SELECT id, fts_score, sub_tier, vec_distance,
|
||||
row_number() OVER (ORDER BY fts_score DESC NULLS LAST, id) AS rn_fts,
|
||||
row_number() OVER (ORDER BY sub_tier DESC NULLS LAST, id) AS rn_sub,
|
||||
row_number() OVER (ORDER BY vec_distance ASC NULLS LAST, id) AS rn_vec
|
||||
FROM agg
|
||||
)
|
||||
SELECT id
|
||||
FROM ranked
|
||||
ORDER BY
|
||||
(CASE WHEN fts_score IS NOT NULL THEN 1.0/(${RRF_K} + rn_fts) ELSE 0 END)
|
||||
+ (CASE WHEN sub_tier IS NOT NULL THEN 1.0/(${RRF_K} + rn_sub) ELSE 0 END)
|
||||
+ (CASE WHEN vec_distance IS NOT NULL THEN ${wVec}::float/(${RRF_K} + rn_vec) ELSE 0 END) DESC,
|
||||
id ASC
|
||||
`.execute(trx);
|
||||
return rankedRows.rows.map((r) => r.id);
|
||||
});
|
||||
}
|
||||
|
||||
// Resolve the search scope: explicit space, the authed user's member spaces, or
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
import { type Kysely, sql } from 'kysely';
|
||||
|
||||
/**
|
||||
* #530 Search Phase B (PR-1): add the embedding FINGERPRINT column.
|
||||
*
|
||||
* The fingerprint is a deterministic id of the embedding configuration that
|
||||
* produced a row — model id + revision + query/doc prefix scheme + dimensions
|
||||
* (see AiService.computeEmbeddingFingerprint). Search filters vector candidates
|
||||
* by the workspace's ACTIVE fingerprint so a revision bump or a prefix-scheme
|
||||
* change never fuses incompatible vectors into results.
|
||||
*
|
||||
* PR-1 scope: this migration only ADDS the column (nullable — existing rows stay
|
||||
* NULL = legacy) and the composite index the vector-candidate scan uses. The full
|
||||
* generational swap / GC lifecycle (target-fingerprint reindex, atomic flip,
|
||||
* old-generation GC) is deliberately deferred to PR-2.
|
||||
*
|
||||
* Independent migration (per the #363 crash-loop net): it creates ONLY its own
|
||||
* objects and never touches another migration's tables/indexes, so a partial
|
||||
* failure cannot leave a shared object half-built.
|
||||
*/
|
||||
export async function up(db: Kysely<any>): Promise<void> {
|
||||
// Nullable text column. Existing rows keep NULL (legacy generation) and are
|
||||
// simply not matched by the active-fingerprint filter until re-indexed.
|
||||
await sql`
|
||||
ALTER TABLE page_embeddings
|
||||
ADD COLUMN IF NOT EXISTS fingerprint text
|
||||
`.execute(db);
|
||||
|
||||
// Composite btree supporting the scoped, dimension- + fingerprint-filtered
|
||||
// vector-candidate scan (workspace_id + space_id + fingerprint + model_dimensions).
|
||||
await db.schema
|
||||
.createIndex('idx_page_embeddings_ws_space_fp_dim')
|
||||
.ifNotExists()
|
||||
.on('page_embeddings')
|
||||
.columns(['workspace_id', 'space_id', 'fingerprint', 'model_dimensions'])
|
||||
.execute();
|
||||
}
|
||||
|
||||
export async function down(db: Kysely<any>): Promise<void> {
|
||||
await db.schema
|
||||
.dropIndex('idx_page_embeddings_ws_space_fp_dim')
|
||||
.ifExists()
|
||||
.execute();
|
||||
|
||||
await sql`
|
||||
ALTER TABLE page_embeddings
|
||||
DROP COLUMN IF EXISTS fingerprint
|
||||
`.execute(db);
|
||||
}
|
||||
@@ -1,6 +1,6 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { InjectKysely } from 'nestjs-kysely';
|
||||
import { sql } from 'kysely';
|
||||
import { RawBuilder, sql } from 'kysely';
|
||||
import * as pgvector from 'pgvector';
|
||||
import { KyselyDB, KyselyTransaction } from '../../types/kysely.types';
|
||||
import { dbOrTx } from '../../utils';
|
||||
@@ -35,6 +35,9 @@ export interface PageEmbeddingChunkRow {
|
||||
content: string;
|
||||
modelName: string;
|
||||
modelDimensions: number;
|
||||
// #530 PR-1: the active embedding fingerprint (see computeEmbeddingFingerprint).
|
||||
// null only when no provider resolves (the indexer no-ops in that case).
|
||||
fingerprint: string | null;
|
||||
embedding: number[];
|
||||
}
|
||||
|
||||
@@ -120,6 +123,7 @@ export class PageEmbeddingRepo {
|
||||
content: row.content,
|
||||
modelName: row.modelName,
|
||||
modelDimensions: row.modelDimensions,
|
||||
fingerprint: row.fingerprint,
|
||||
// pgvector.toSql -> '[1,2,3]'; cast the bound literal to vector.
|
||||
embedding: sql`${pgvector.toSql(row.embedding)}::vector`,
|
||||
})),
|
||||
@@ -127,6 +131,82 @@ export class PageEmbeddingRepo {
|
||||
.execute();
|
||||
}
|
||||
|
||||
/**
|
||||
* #530: build the VECTOR candidate arm for SearchService's fused RRF union — a
|
||||
* page-level nearest-neighbour sub-select. Returns a `sql` fragment (not an
|
||||
* executed query) so the caller can UNION ALL it with the lexical arm inside a
|
||||
* single ranked-ids query.
|
||||
*
|
||||
* SCALING BOUNDARY: the column is dimension-agnostic, so it carries NO ANN
|
||||
* index — this is a brute-force O(N) KNN seq scan with `<=>`. Accepted for the
|
||||
* small-tenant / homelab fork target (ANN + a pinned-dimension column are
|
||||
* deferred). It is NOT unbounded, though: the caller (SearchService) runs this
|
||||
* fused query under a per-statement timeout (SEARCH_VECTOR_STATEMENT_TIMEOUT_MS)
|
||||
* and DEGRADES to lexical-only on a 57014 cancellation, so a pathological scan
|
||||
* can never hang the interactive search request.
|
||||
*
|
||||
* The arm collapses a page's chunks to its best (MIN) cosine distance:
|
||||
* SELECT pages.id, NULL fts_score, NULL sub_tier,
|
||||
* MIN(pe.embedding <=> $qvec) AS vec_distance
|
||||
* FROM page_embeddings pe JOIN pages ON pages.id = pe.page_id
|
||||
* WHERE <scope> AND pe.model_dimensions = $dim AND pe.fingerprint = $fp
|
||||
* GROUP BY pages.id ORDER BY vec_distance LIMIT $limit
|
||||
*
|
||||
* `scope` is SearchService's shared scope predicate (workspace + space/id set +
|
||||
* creator + descendants + deleted_at), referencing the `pages` table — it must
|
||||
* be spliced verbatim so the vector candidate set mirrors the lexical scope
|
||||
* EXACTLY, minus the lexical text predicate (vector candidates need not match
|
||||
* text). The vector is bound via pgvector's `toSql(...)::vector`, and both the
|
||||
* dimension and the ACTIVE fingerprint are filtered so `<=>` only ever compares
|
||||
* compatible, same-generation vectors (pgvector errors on a dimension
|
||||
* mismatch; a fingerprint mismatch would fuse incomparable vectors).
|
||||
*
|
||||
* `workspaceId` is ALSO filtered directly on `page_embeddings` — not only via
|
||||
* the pages join. It is IMMUTABLE (there are no cross-workspace page moves), so
|
||||
* an embedding row's workspace_id always matches its page's, and this drops NO
|
||||
* legitimate hit; it lets the composite index idx_page_embeddings_ws_space_fp_dim
|
||||
* (workspace_id, space_id, fingerprint, model_dimensions) bite from its LEADING
|
||||
* column, and workspace-scopes candidates at the embedding level (defense in
|
||||
* depth).
|
||||
*
|
||||
* SPACE is deliberately NOT filtered on page_embeddings: `page_embeddings.space_id`
|
||||
* is stamped at index time and is NOT updated when a page is MOVED between spaces
|
||||
* (movePageToSpace does not reindex, and PAGE_MOVED_TO_SPACE has no reindex
|
||||
* consumer), so a moved page's rows carry the OLD space until the next reindex. A
|
||||
* `page_embeddings.space_id = ANY(scope)` predicate would then wrongly drop a
|
||||
* legitimate vector hit for a page moved INTO the searched space. Space scoping
|
||||
* is therefore enforced ONLY by the join to `pages` (pages.space_id ∈ scope,
|
||||
* always current) — the same way the lexical arm scopes, so no space leak.
|
||||
*
|
||||
* The NULL fts_score / sub_tier columns keep the arm UNION-compatible with the
|
||||
* lexical arm's column list.
|
||||
*/
|
||||
vectorCandidateArm(params: {
|
||||
queryEmbedding: number[];
|
||||
dimensions: number;
|
||||
fingerprint: string;
|
||||
scope: RawBuilder<unknown>;
|
||||
limit: number;
|
||||
workspaceId: string;
|
||||
}): RawBuilder<unknown> {
|
||||
const qvec = sql`${pgvector.toSql(params.queryEmbedding)}::vector`;
|
||||
return sql`
|
||||
SELECT pages.id AS id,
|
||||
NULL::float AS fts_score,
|
||||
NULL::int AS sub_tier,
|
||||
MIN(page_embeddings.embedding <=> ${qvec}) AS vec_distance
|
||||
FROM page_embeddings
|
||||
JOIN pages ON pages.id = page_embeddings.page_id
|
||||
WHERE ${params.scope}
|
||||
AND page_embeddings.workspace_id = ${params.workspaceId}
|
||||
AND page_embeddings.model_dimensions = ${params.dimensions}
|
||||
AND page_embeddings.fingerprint = ${params.fingerprint}
|
||||
GROUP BY pages.id
|
||||
ORDER BY vec_distance ASC
|
||||
LIMIT ${params.limit}
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Cosine search over the embeddings, scoped to a workspace AND a set of
|
||||
* spaces the caller may read (see semanticSearch access-scoping). Orders by
|
||||
|
||||
@@ -7,6 +7,11 @@ export interface PageEmbeddings {
|
||||
spaceId: string;
|
||||
modelName: string;
|
||||
modelDimensions: number;
|
||||
// #530 PR-1: the active embedding fingerprint (model id + revision + prefix
|
||||
// scheme + dimensions). NULL on legacy rows written before this column existed.
|
||||
// Search filters candidates by the ACTIVE fingerprint so a revision/prefix
|
||||
// change never mixes incompatible vectors.
|
||||
fingerprint: string | null;
|
||||
workspaceId: string;
|
||||
// Nullable: page-body embeddings have no attachment (only attachment chunks set it).
|
||||
attachmentId: string | null;
|
||||
|
||||
@@ -108,6 +108,23 @@ export function isUniqueViolation(err: unknown): boolean {
|
||||
return (err as { code?: unknown } | null | undefined)?.code === PG_UNIQUE_VIOLATION;
|
||||
}
|
||||
|
||||
/** Postgres `query_canceled` SQLSTATE — raised when statement_timeout fires. */
|
||||
const PG_QUERY_CANCELED = '57014';
|
||||
|
||||
/**
|
||||
* Whether `err` is a Postgres statement-timeout cancellation (SQLSTATE `57014`).
|
||||
* Used by #530 semantic search to degrade a slow vector scan to lexical-only
|
||||
* instead of hanging/500-ing. Matches on the SQLSTATE (the driver surfaces it as
|
||||
* `.code`), with a message fallback for any wrapper that drops the code.
|
||||
*/
|
||||
export function isStatementTimeout(err: unknown): boolean {
|
||||
if ((err as { code?: unknown } | null | undefined)?.code === PG_QUERY_CANCELED) {
|
||||
return true;
|
||||
}
|
||||
const msg = err instanceof Error ? err.message : '';
|
||||
return /canceling statement due to statement timeout/i.test(msg);
|
||||
}
|
||||
|
||||
/**
|
||||
* The name of the UNIQUE index/constraint a `23505` error violated, or
|
||||
* undefined. The `kysely-postgres-js` / `postgres@3.x` driver surfaces it as
|
||||
|
||||
@@ -0,0 +1,143 @@
|
||||
import { AiService, computeEmbeddingFingerprint } from './ai.service';
|
||||
|
||||
/**
|
||||
* #530 Search Phase B (PR-1) unit coverage for the embedding layer:
|
||||
* - computeEmbeddingFingerprint: a revision bump / prefix toggle MUST change the
|
||||
* fingerprint even when the bare model name + dimension are unchanged (it is
|
||||
* deliberately NOT the bare model name);
|
||||
* - embedQuery: applies the QUERY prefix and embeds under the short SEARCH embed
|
||||
* timeout (not the long batch timeout);
|
||||
* - embedWithModel: a hung model + short timeout degrades with a clear timeout
|
||||
* error (the search caller then falls back to the lexical path).
|
||||
*/
|
||||
describe('computeEmbeddingFingerprint (#530)', () => {
|
||||
const base = {
|
||||
modelId: 'intfloat/multilingual-e5-small',
|
||||
revision: 'sha-aaa',
|
||||
queryPrefix: 'query: ',
|
||||
docPrefix: 'passage: ',
|
||||
dimensions: 384,
|
||||
};
|
||||
|
||||
it('is deterministic for identical inputs', () => {
|
||||
expect(computeEmbeddingFingerprint(base)).toBe(
|
||||
computeEmbeddingFingerprint({ ...base }),
|
||||
);
|
||||
});
|
||||
|
||||
it('CHANGES on a revision bump (same model name + dimension)', () => {
|
||||
const a = computeEmbeddingFingerprint(base);
|
||||
const b = computeEmbeddingFingerprint({ ...base, revision: 'sha-bbb' });
|
||||
expect(b).not.toBe(a);
|
||||
});
|
||||
|
||||
it('CHANGES on a query-prefix toggle (same model name + dimension + revision)', () => {
|
||||
const a = computeEmbeddingFingerprint(base);
|
||||
const b = computeEmbeddingFingerprint({ ...base, queryPrefix: '' });
|
||||
expect(b).not.toBe(a);
|
||||
});
|
||||
|
||||
it('CHANGES on a doc-prefix toggle', () => {
|
||||
const a = computeEmbeddingFingerprint(base);
|
||||
const b = computeEmbeddingFingerprint({ ...base, docPrefix: '' });
|
||||
expect(b).not.toBe(a);
|
||||
});
|
||||
|
||||
it('CHANGES on a dimension change', () => {
|
||||
const a = computeEmbeddingFingerprint(base);
|
||||
const b = computeEmbeddingFingerprint({ ...base, dimensions: 768 });
|
||||
expect(b).not.toBe(a);
|
||||
});
|
||||
|
||||
it('is NOT the bare model name (guards the mutation)', () => {
|
||||
// If the fingerprint were the bare model name, a revision bump would not
|
||||
// change it — the tests above would red. Assert it directly too.
|
||||
expect(computeEmbeddingFingerprint(base)).not.toBe(base.modelId);
|
||||
});
|
||||
|
||||
it('distinguishes swapped prefixes — ("q","") != ("","q")', () => {
|
||||
const a = computeEmbeddingFingerprint({
|
||||
...base,
|
||||
queryPrefix: 'q',
|
||||
docPrefix: '',
|
||||
});
|
||||
const b = computeEmbeddingFingerprint({
|
||||
...base,
|
||||
queryPrefix: '',
|
||||
docPrefix: 'q',
|
||||
});
|
||||
expect(a).not.toBe(b);
|
||||
});
|
||||
});
|
||||
|
||||
describe('AiService.embedQuery (#530)', () => {
|
||||
function makeService(): AiService {
|
||||
// The constructor body only stores its deps; embedQuery is fully driven by
|
||||
// the spied resolveEmbeddingProvider + embedWithModel below.
|
||||
return new AiService({} as any, {} as any, {} as any);
|
||||
}
|
||||
|
||||
it('prepends the query prefix and returns the provider fingerprint', async () => {
|
||||
const svc = makeService();
|
||||
jest.spyOn(svc, 'resolveEmbeddingProvider').mockResolvedValue({
|
||||
model: { modelId: 'm' } as any,
|
||||
queryPrefix: 'query: ',
|
||||
docPrefix: 'passage: ',
|
||||
fingerprint: 'fp-active',
|
||||
});
|
||||
const embedSpy = jest
|
||||
.spyOn(svc, 'embedWithModel')
|
||||
.mockResolvedValue([[0.1, 0.2, 0.3]]);
|
||||
|
||||
const res = await svc.embedQuery('ws-1', 'кофе');
|
||||
|
||||
// The single embedded value carries the query prefix.
|
||||
expect(embedSpy).toHaveBeenCalledTimes(1);
|
||||
const [, wsArg, valuesArg, timeoutArg] = embedSpy.mock.calls[0];
|
||||
expect(wsArg).toBe('ws-1');
|
||||
expect(valuesArg).toEqual(['query: кофе']);
|
||||
// Bound by the SHORT search-embed timeout (default 800ms), not the 120000ms
|
||||
// batch timeout.
|
||||
expect(timeoutArg).toBe(800);
|
||||
expect(res).toEqual({ vector: [0.1, 0.2, 0.3], fingerprint: 'fp-active' });
|
||||
});
|
||||
|
||||
it('propagates a no-provider error (drives the no-provider degrade)', async () => {
|
||||
const svc = makeService();
|
||||
const { AiEmbeddingNotConfiguredException } = await import(
|
||||
'./ai-embedding-not-configured.exception'
|
||||
);
|
||||
jest
|
||||
.spyOn(svc, 'resolveEmbeddingProvider')
|
||||
.mockRejectedValue(new AiEmbeddingNotConfiguredException());
|
||||
await expect(svc.embedQuery('ws-1', 'x')).rejects.toBeInstanceOf(
|
||||
AiEmbeddingNotConfiguredException,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('AiService.embedWithModel timeout (#530)', () => {
|
||||
it('degrades a hung model with a timeout error under the short window', async () => {
|
||||
const svc = new AiService({} as any, {} as any, {} as any);
|
||||
// A model whose doEmbed never resolves but honours the abort signal, so the
|
||||
// AbortSignal.timeout fires and embedWithModel raises a clear timeout error.
|
||||
const hangingModel: any = {
|
||||
specificationVersion: 'v2',
|
||||
provider: 'fake',
|
||||
modelId: 'fake',
|
||||
maxEmbeddingsPerCall: 100,
|
||||
supportsParallelCalls: true,
|
||||
doEmbed: ({ abortSignal }: { abortSignal?: AbortSignal }) =>
|
||||
new Promise((_resolve, reject) => {
|
||||
abortSignal?.addEventListener('abort', () => {
|
||||
const err = new Error('The operation was aborted');
|
||||
err.name = 'AbortError';
|
||||
reject(err);
|
||||
});
|
||||
}),
|
||||
};
|
||||
await expect(
|
||||
svc.embedWithModel(hangingModel, 'ws-1', ['x'], 30),
|
||||
).rejects.toThrow(/timed out/i);
|
||||
});
|
||||
});
|
||||
@@ -23,6 +23,50 @@ import {
|
||||
import { AiProviderCredentialsRepo } from '@docmost/db/repos/ai-chat/ai-provider-credentials.repo';
|
||||
import { SecretBoxService } from '../crypto/secret-box';
|
||||
import { AiDriver } from './ai.types';
|
||||
import { createHash } from 'node:crypto';
|
||||
|
||||
/**
|
||||
* A resolved embedding provider for #530 semantic search. `model` is the AI SDK
|
||||
* embedding model; `queryPrefix`/`docPrefix` are prepended to a query / a stored
|
||||
* chunk respectively (e5-style `"query: "` / `"passage: "`, empty for a non-e5
|
||||
* provider); `fingerprint` is the deterministic id of the whole configuration.
|
||||
*/
|
||||
export interface ResolvedEmbeddingProvider {
|
||||
model: EmbeddingModel;
|
||||
queryPrefix: string;
|
||||
docPrefix: string;
|
||||
fingerprint: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Deterministic embedding FINGERPRINT (#530). Encodes model id + revision +
|
||||
* prefix scheme + dimensions so that ANY of them changing (a revision bump, a
|
||||
* prefix toggle) yields a DIFFERENT fingerprint even when the bare model name and
|
||||
* dimension are unchanged. Deliberately NOT the bare model name: two rows from
|
||||
* the same model but a different revision/prefix must not be fused together.
|
||||
*
|
||||
* Exported as a pure function so it can be unit-tested in isolation and reused by
|
||||
* the indexer without a service instance.
|
||||
*/
|
||||
export function computeEmbeddingFingerprint(parts: {
|
||||
modelId: string;
|
||||
revision: string;
|
||||
queryPrefix: string;
|
||||
docPrefix: string;
|
||||
dimensions: number | null;
|
||||
}): string {
|
||||
// The two prefixes are SEPARATE keys (not a concatenated string): JSON.stringify
|
||||
// escapes each independently, so the prefix scheme ("a","") is distinct from
|
||||
// ("","a") with no separator/collision hazard even if a prefix contains spaces.
|
||||
const canonical = JSON.stringify({
|
||||
m: parts.modelId,
|
||||
r: parts.revision,
|
||||
q: parts.queryPrefix,
|
||||
d: parts.docPrefix,
|
||||
dim: parts.dimensions ?? 0,
|
||||
});
|
||||
return createHash('sha256').update(canonical).digest('hex').slice(0, 32);
|
||||
}
|
||||
|
||||
/**
|
||||
* Optional chat-model override carried by an agent role (`ai_agent_roles.
|
||||
@@ -362,11 +406,112 @@ export class AiService {
|
||||
async embedTexts(workspaceId: string, texts: string[]): Promise<number[][]> {
|
||||
if (texts.length === 0) return [];
|
||||
const model = await this.getEmbeddingModel(workspaceId);
|
||||
// Bound the embedding call: a slow/hung embeddings endpoint must fail loudly
|
||||
// (and let the caller move on to the next page) instead of blocking forever.
|
||||
// The single signal caps the WHOLE call, including the SDK's internal
|
||||
// retries/backoff (embedMany defaults to maxRetries: 2).
|
||||
const timeoutMs = AiService.embeddingTimeoutMs();
|
||||
return this.embedWithModel(model, workspaceId, texts);
|
||||
}
|
||||
|
||||
/**
|
||||
* #530: resolve the embedding provider for a workspace. Prefers the workspace's
|
||||
* own configured embedding provider; falls back to the GLOBAL env provider (a
|
||||
* TEI sidecar via the OpenAI-compatible path) when the workspace has none.
|
||||
* Returns the model + the query/doc prefixes + the config fingerprint. Throws
|
||||
* AiEmbeddingNotConfiguredException when NEITHER resolves, so callers can drive
|
||||
* a `no-provider` degrade path.
|
||||
*/
|
||||
async resolveEmbeddingProvider(
|
||||
workspaceId: string,
|
||||
): Promise<ResolvedEmbeddingProvider> {
|
||||
// 1. Per-workspace provider (uses the workspace's own creds/endpoint). When
|
||||
// it is not configured getEmbeddingModel throws the not-configured
|
||||
// exception; we swallow ONLY that and fall through to the global provider.
|
||||
try {
|
||||
const model = await this.getEmbeddingModel(workspaceId);
|
||||
const modelId =
|
||||
typeof model === 'string' ? model : (model.modelId ?? 'unknown');
|
||||
// A per-workspace (typically non-e5) provider gets no e5-style prefixes.
|
||||
return {
|
||||
model,
|
||||
queryPrefix: '',
|
||||
docPrefix: '',
|
||||
fingerprint: computeEmbeddingFingerprint({
|
||||
modelId,
|
||||
revision: 'workspace',
|
||||
queryPrefix: '',
|
||||
docPrefix: '',
|
||||
dimensions: null,
|
||||
}),
|
||||
};
|
||||
} catch (err) {
|
||||
if (!(err instanceof AiEmbeddingNotConfiguredException)) throw err;
|
||||
}
|
||||
|
||||
// 2. Global env provider (TEI sidecar). TEI is OpenAI-compatible, so reuse
|
||||
// the existing openai path — no new SDK. A dummy key is fine for a
|
||||
// keyless self-hosted sidecar.
|
||||
const endpoint = process.env.EMBEDDING_ENDPOINT?.trim();
|
||||
const globalModel = process.env.EMBEDDING_MODEL?.trim();
|
||||
if (endpoint && globalModel) {
|
||||
const model = createOpenAI({
|
||||
baseURL: endpoint,
|
||||
apiKey: process.env.EMBEDDING_API_KEY || 'unused',
|
||||
}).textEmbeddingModel(globalModel);
|
||||
const queryPrefix = process.env.EMBEDDING_QUERY_PREFIX ?? '';
|
||||
const docPrefix = process.env.EMBEDDING_DOC_PREFIX ?? '';
|
||||
const dimRaw = Number(process.env.EMBEDDING_DIMENSIONS);
|
||||
const dimensions = Number.isFinite(dimRaw) && dimRaw > 0 ? dimRaw : null;
|
||||
return {
|
||||
model,
|
||||
queryPrefix,
|
||||
docPrefix,
|
||||
fingerprint: computeEmbeddingFingerprint({
|
||||
modelId: globalModel,
|
||||
revision: process.env.EMBEDDING_REVISION ?? '',
|
||||
queryPrefix,
|
||||
docPrefix,
|
||||
dimensions,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
// Neither resolved -> drives semantic.reason=no-provider.
|
||||
throw new AiEmbeddingNotConfiguredException();
|
||||
}
|
||||
|
||||
/**
|
||||
* #530: embed a SEARCH QUERY. Resolves the provider, prepends its query prefix,
|
||||
* and embeds the single value under its OWN short timeout
|
||||
* (SEARCH_EMBED_TIMEOUT_MS, default 800ms) — NOT the long batch-indexing
|
||||
* timeout — so a slow/hung sidecar degrades search fast. Throws on
|
||||
* timeout/error (the caller degrades to the lexical-only path). Returns the
|
||||
* vector plus the active fingerprint used to filter candidate rows.
|
||||
*/
|
||||
async embedQuery(
|
||||
workspaceId: string,
|
||||
text: string,
|
||||
): Promise<{ vector: number[]; fingerprint: string }> {
|
||||
const provider = await this.resolveEmbeddingProvider(workspaceId);
|
||||
const [vector] = await this.embedWithModel(
|
||||
provider.model,
|
||||
workspaceId,
|
||||
[provider.queryPrefix + text],
|
||||
AiService.searchEmbedTimeoutMs(),
|
||||
);
|
||||
return { vector, fingerprint: provider.fingerprint };
|
||||
}
|
||||
|
||||
/**
|
||||
* Embed values with an EXPLICIT model, bounded by `timeoutMs` (default: the
|
||||
* batch-indexing timeout). Shared core of embedTexts / embedQuery: a slow/hung
|
||||
* embeddings endpoint must fail loudly instead of blocking forever. The single
|
||||
* signal caps the WHOLE call, including the SDK's internal retries/backoff
|
||||
* (embedMany defaults to maxRetries: 2).
|
||||
*/
|
||||
async embedWithModel(
|
||||
model: EmbeddingModel,
|
||||
workspaceId: string,
|
||||
texts: string[],
|
||||
timeoutMs: number = AiService.embeddingTimeoutMs(),
|
||||
): Promise<number[][]> {
|
||||
if (texts.length === 0) return [];
|
||||
const signal = AbortSignal.timeout(timeoutMs);
|
||||
try {
|
||||
const { embeddings } = await embedMany({
|
||||
@@ -391,8 +536,8 @@ export class AiService {
|
||||
if (signal.aborted && abortLike) {
|
||||
throw new Error(
|
||||
`Embedding request timed out after ${timeoutMs}ms ` +
|
||||
`(workspace ${workspaceId}, ${texts.length} chunk(s)). ` +
|
||||
`Increase AI_EMBEDDING_TIMEOUT_MS or check the embeddings endpoint.`,
|
||||
`(workspace ${workspaceId}, ${texts.length} value(s)). ` +
|
||||
`Increase the embedding timeout or check the embeddings endpoint.`,
|
||||
);
|
||||
}
|
||||
throw err;
|
||||
@@ -408,6 +553,16 @@ export class AiService {
|
||||
return Number.isFinite(raw) && raw > 0 ? raw : 120_000;
|
||||
}
|
||||
|
||||
/**
|
||||
* #530: per-call timeout for the interactive SEARCH query embed. Much shorter
|
||||
* than the batch-indexing timeout (a search request cannot wait 2 minutes on
|
||||
* the sidecar). Configurable via SEARCH_EMBED_TIMEOUT_MS; default 800ms.
|
||||
*/
|
||||
private static searchEmbedTimeoutMs(): number {
|
||||
const raw = Number(process.env.SEARCH_EMBED_TIMEOUT_MS);
|
||||
return Number.isFinite(raw) && raw > 0 ? raw : 800;
|
||||
}
|
||||
|
||||
// Build a tiny valid WAV (mono, 16-bit PCM, 16 kHz, ~1s of silence), used only
|
||||
// as a connectivity probe for the STT endpoint in testConnection.
|
||||
private static silentWavProbe(): Uint8Array {
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import { Kysely, sql } from 'kysely';
|
||||
import { Logger } from '@nestjs/common';
|
||||
import { SearchService } from 'src/core/search/search.service';
|
||||
import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
||||
import { PageEmbeddingRepo } from '@docmost/db/repos/ai-chat/page-embedding.repo';
|
||||
import { AiEmbeddingNotConfiguredException } from 'src/integrations/ai/ai-embedding-not-configured.exception';
|
||||
import {
|
||||
getTestDb,
|
||||
destroyTestDb,
|
||||
@@ -53,10 +56,19 @@ describe('SearchService #529 lexical overhaul [integration]', () => {
|
||||
// Service wired to the real DB + real PageRepo (recursive descendants) with
|
||||
// stubbed space-membership + permission repos so a test controls scope and the
|
||||
// permission filter explicitly. `accessibleIds` (when set) is the KEEP list.
|
||||
// #530: a FAKE AiService whose embedQuery is fully controlled per test. By
|
||||
// DEFAULT it throws AiEmbeddingNotConfiguredException, so the semantic arm is
|
||||
// omitted and every Phase-A case runs the BYTE-IDENTICAL lexical path — the
|
||||
// semantic layer must never change lexical behaviour on degrade. Semantic cases
|
||||
// pass `embedVector` (a deterministic 384-dim query vector) or `embedThrows`.
|
||||
function buildService(opts?: {
|
||||
userSpaceIds?: string[];
|
||||
accessibleIds?: string[] | null;
|
||||
filterThrows?: boolean;
|
||||
embedVector?: number[];
|
||||
embedFingerprint?: string;
|
||||
embedThrows?: 'no-provider' | 'degraded';
|
||||
slowVectorArm?: boolean;
|
||||
}): SearchService {
|
||||
const pageRepo = new PageRepo(db as any, null as any, null as any);
|
||||
const spaceMemberRepo = {
|
||||
@@ -71,15 +83,86 @@ describe('SearchService #529 lexical overhaul [integration]', () => {
|
||||
: pageIds;
|
||||
},
|
||||
};
|
||||
const aiService = {
|
||||
embedQuery: async () => {
|
||||
if (opts?.embedVector) {
|
||||
return {
|
||||
vector: opts.embedVector,
|
||||
fingerprint: opts.embedFingerprint ?? ACTIVE_FP,
|
||||
};
|
||||
}
|
||||
if (opts?.embedThrows === 'degraded') {
|
||||
const err = new Error('embedding request timed out after 1ms');
|
||||
err.name = 'TimeoutError';
|
||||
throw err;
|
||||
}
|
||||
// Default (and explicit 'no-provider'): no embedding provider resolved.
|
||||
throw new AiEmbeddingNotConfiguredException();
|
||||
},
|
||||
};
|
||||
// Real repo on the migrated test DB — exercises the actual vector-candidate
|
||||
// SQL (pgvector <=>, fingerprint + dimension filters). When slowVectorArm is
|
||||
// set, wrap the arm so it sleeps well past the (tiny, test-set) statement
|
||||
// timeout, forcing a 57014 cancellation to exercise the degrade-to-lexical net.
|
||||
const realRepo = new PageEmbeddingRepo(db as any);
|
||||
const pageEmbeddingRepo = opts?.slowVectorArm
|
||||
? {
|
||||
vectorCandidateArm: (p: any) => {
|
||||
const inner = realRepo.vectorCandidateArm(p);
|
||||
return sql`SELECT * FROM (${inner}) AS slowarm WHERE (SELECT true FROM pg_sleep(0.5))`;
|
||||
},
|
||||
}
|
||||
: realRepo;
|
||||
return new SearchService(
|
||||
db as any,
|
||||
pageRepo as any,
|
||||
{} as any,
|
||||
spaceMemberRepo as any,
|
||||
pagePermissionRepo as any,
|
||||
aiService as any,
|
||||
pageEmbeddingRepo as any,
|
||||
);
|
||||
}
|
||||
|
||||
// Active embedding fingerprint used by planted rows + the fake query embed.
|
||||
const ACTIVE_FP = 'fp-active-530';
|
||||
|
||||
// A deterministic unit-ish 384-dim vector with a single 1 at `concept`. Two
|
||||
// vectors of the same concept have cosine distance 0; different concepts are
|
||||
// orthogonal (distance 1), so nearest-neighbour ordering is fully controlled.
|
||||
function conceptVec(concept: number): number[] {
|
||||
const v = new Array(384).fill(0);
|
||||
v[concept % 384] = 1;
|
||||
return v;
|
||||
}
|
||||
|
||||
// Plant one chunk embedding for a page via the REAL repo (exercises the #530
|
||||
// fingerprint insert path). Dimension is fixed at 384 to match conceptVec.
|
||||
async function plantEmbedding(
|
||||
pageId: string,
|
||||
vector: number[],
|
||||
fingerprint: string = ACTIVE_FP,
|
||||
planSpaceId: string = spaceId,
|
||||
): Promise<void> {
|
||||
const repo = new PageEmbeddingRepo(db as any);
|
||||
await repo.insertChunks([
|
||||
{
|
||||
pageId,
|
||||
workspaceId,
|
||||
spaceId: planSpaceId,
|
||||
attachmentId: null,
|
||||
chunkIndex: 0,
|
||||
chunkStart: 0,
|
||||
chunkLength: 1,
|
||||
content: 'planted chunk',
|
||||
modelName: 'test-model',
|
||||
modelDimensions: 384,
|
||||
fingerprint,
|
||||
embedding: vector,
|
||||
},
|
||||
]);
|
||||
}
|
||||
|
||||
const search = (service: SearchService, params: any) =>
|
||||
service.searchPage(params, { userId: 'u-1', workspaceId }) as any;
|
||||
|
||||
@@ -609,4 +692,324 @@ describe('SearchService #529 lexical overhaul [integration]', () => {
|
||||
expect(hit2).toBeDefined();
|
||||
expect(hit2.path).toEqual([]);
|
||||
});
|
||||
|
||||
// === #530 Phase B (PR-1): semantic fusion ==================================
|
||||
// Deterministic fake embedder (no live model) + real PageEmbeddingRepo on the
|
||||
// migrated DB with planted vectors + the active fingerprint, so cosine ordering
|
||||
// is fully controlled. Each case runs in its OWN space so planted rows never
|
||||
// leak across tests.
|
||||
describe('#530 semantic fusion', () => {
|
||||
// 1. Vector-only hit: nearest to a page that has NO lexical match for the
|
||||
// query term — it appears via the vector arm, and a pure-lexical run does
|
||||
// not return it. Mutation (a): drop the vector arm -> this reddens.
|
||||
it('#530-1 a vector-only hit appears; a pure-lexical run would not return it', async () => {
|
||||
const s = (await createSpace(db, workspaceId)).id;
|
||||
const page = await insertPage({
|
||||
title: 'нейтральный вект заголовок',
|
||||
textContent: 'содержимое без искомого термина',
|
||||
spaceId: s,
|
||||
});
|
||||
await plantEmbedding(page, conceptVec(1), ACTIVE_FP, s);
|
||||
|
||||
const svc = buildService({ embedVector: conceptVec(1), userSpaceIds: [s] });
|
||||
const res = await search(svc, { query: 'квазонимикс', spaceId: s });
|
||||
expect(res.items.map((i: any) => i.id)).toContain(page);
|
||||
expect(res.semantic.available).toBe(true);
|
||||
expect(res.semantic.state).toBe('full');
|
||||
|
||||
// Pure-lexical (default degrade): the vector-only page is absent.
|
||||
const lex = buildService({ userSpaceIds: [s] });
|
||||
const res2 = await search(lex, { query: 'квазонимикс', spaceId: s });
|
||||
expect(res2.items.map((i: any) => i.id)).not.toContain(page);
|
||||
expect(res2.semantic.available).toBe(false);
|
||||
});
|
||||
|
||||
// 2. Sidecar down: embedQuery throws -> results = the lexical set; semantic
|
||||
// unavailable; the fixed 'search.semantic.degraded' event is logged.
|
||||
it('#530-2 sidecar-down degrades to lexical and logs search.semantic.degraded', async () => {
|
||||
const s = (await createSpace(db, workspaceId)).id;
|
||||
const lexHit = await insertPage({
|
||||
title: 'семдвамаркер лексический',
|
||||
textContent: 'обычный текст',
|
||||
spaceId: s,
|
||||
});
|
||||
const warnSpy = jest
|
||||
.spyOn(Logger.prototype, 'warn')
|
||||
.mockImplementation(() => undefined as any);
|
||||
try {
|
||||
const svc = buildService({ embedThrows: 'degraded', userSpaceIds: [s] });
|
||||
const res = await search(svc, { query: 'семдвамаркер', spaceId: s });
|
||||
expect(res.items.map((i: any) => i.id)).toContain(lexHit);
|
||||
expect(res.semantic.available).toBe(false);
|
||||
expect(res.semantic.reason).toBe('degraded');
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('search.semantic.degraded'),
|
||||
);
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
// 3. No provider: resolveEmbeddingProvider yields none -> lexical results,
|
||||
// reason no-provider, no 500.
|
||||
it('#530-3 no-provider yields lexical results with reason no-provider', async () => {
|
||||
const s = (await createSpace(db, workspaceId)).id;
|
||||
const hit = await insertPage({
|
||||
title: 'семтримаркер тема',
|
||||
textContent: 'текст',
|
||||
spaceId: s,
|
||||
});
|
||||
const svc = buildService({ embedThrows: 'no-provider', userSpaceIds: [s] });
|
||||
const res = await search(svc, { query: 'семтримаркер', spaceId: s });
|
||||
expect(res.items.map((i: any) => i.id)).toContain(hit);
|
||||
expect(res.semantic.available).toBe(false);
|
||||
expect(res.semantic.reason).toBe('no-provider');
|
||||
});
|
||||
|
||||
// 4. Permission over the union: a restricted vector-only hit is dropped from
|
||||
// items AND total; with the permission filter throwing the call rejects
|
||||
// (fail-closed, never fail-open). Mutation (b): move filterAccessiblePageIds
|
||||
// inside the semantic try/catch -> the reject assertion reddens.
|
||||
it('#530-4 permission filters a vector-only hit from items AND total (fail-closed)', async () => {
|
||||
const s = (await createSpace(db, workspaceId)).id;
|
||||
const visibleLex = await insertPage({
|
||||
title: 'семчетмаркер видимый',
|
||||
textContent: 'доступный текст',
|
||||
spaceId: s,
|
||||
});
|
||||
const hiddenVec = await insertPage({
|
||||
title: 'скрытая вект страница',
|
||||
textContent: 'содержимое без искомого термина',
|
||||
spaceId: s,
|
||||
});
|
||||
await plantEmbedding(hiddenVec, conceptVec(4), ACTIVE_FP, s);
|
||||
|
||||
// Query hits visibleLex lexically; hiddenVec only via the vector arm.
|
||||
const svc = buildService({
|
||||
embedVector: conceptVec(4),
|
||||
accessibleIds: [visibleLex],
|
||||
userSpaceIds: [s],
|
||||
});
|
||||
const res = await search(svc, { query: 'семчетмаркер', spaceId: s });
|
||||
const ids = res.items.map((i: any) => i.id);
|
||||
expect(ids).toContain(visibleLex);
|
||||
expect(ids).not.toContain(hiddenVec);
|
||||
// The hidden vector hit does not leak into total either.
|
||||
expect(res.total).toBe(1);
|
||||
expect(ids).toHaveLength(res.total);
|
||||
|
||||
// A permission-query error PROPAGATES (never a fail-open empty result).
|
||||
const boom = buildService({
|
||||
embedVector: conceptVec(4),
|
||||
filterThrows: true,
|
||||
userSpaceIds: [s],
|
||||
});
|
||||
await expect(
|
||||
search(boom, { query: 'семчетмаркер', spaceId: s }),
|
||||
).rejects.toThrow(/permission query failed/);
|
||||
});
|
||||
|
||||
// 5. Hung sidecar under a short embed timeout: graceful degrade (never a 500).
|
||||
it('#530-5 a hung sidecar under a short timeout degrades gracefully', async () => {
|
||||
const s = (await createSpace(db, workspaceId)).id;
|
||||
const hit = await insertPage({
|
||||
title: 'семпятьмаркер тема',
|
||||
textContent: 'текст',
|
||||
spaceId: s,
|
||||
});
|
||||
process.env.SEARCH_EMBED_TIMEOUT_MS = '1';
|
||||
try {
|
||||
const svc = buildService({ embedThrows: 'degraded', userSpaceIds: [s] });
|
||||
const res = await search(svc, { query: 'семпятьмаркер', spaceId: s });
|
||||
expect(res.items.map((i: any) => i.id)).toContain(hit);
|
||||
expect(res.semantic.available).toBe(false);
|
||||
expect(res.semantic.reason).toBe('degraded');
|
||||
} finally {
|
||||
delete process.env.SEARCH_EMBED_TIMEOUT_MS;
|
||||
}
|
||||
});
|
||||
|
||||
// 6. A page matched BOTH lexically and by vector is fused (de-duped), not
|
||||
// returned twice — the agg CTE collapses it to one row.
|
||||
it('#530-6 a page matched lexically AND by vector appears once', async () => {
|
||||
const s = (await createSpace(db, workspaceId)).id;
|
||||
const both = await insertPage({
|
||||
title: 'семшестьмаркер общий',
|
||||
textContent: 'и лексика и вектор',
|
||||
spaceId: s,
|
||||
});
|
||||
await plantEmbedding(both, conceptVec(6), ACTIVE_FP, s);
|
||||
const svc = buildService({ embedVector: conceptVec(6), userSpaceIds: [s] });
|
||||
const res = await search(svc, { query: 'семшестьмаркер', spaceId: s });
|
||||
const ids = res.items.map((i: any) => i.id);
|
||||
expect(ids.filter((id: string) => id === both)).toHaveLength(1);
|
||||
expect(res.total).toBe(1);
|
||||
});
|
||||
|
||||
// 7. Fingerprint isolation: a planted row under a DIFFERENT fingerprint is not
|
||||
// a vector candidate for the active-fingerprint query (guards mixing
|
||||
// generations). It only appears if it also matches lexically (it does not).
|
||||
it('#530-7 a stale-fingerprint vector row is not fused into results', async () => {
|
||||
const s = (await createSpace(db, workspaceId)).id;
|
||||
const stale = await insertPage({
|
||||
title: 'нейтральный семь заголовок',
|
||||
textContent: 'без искомого термина совсем',
|
||||
spaceId: s,
|
||||
});
|
||||
// Same concept vector as the query, but an OLD generation fingerprint.
|
||||
await plantEmbedding(stale, conceptVec(7), 'fp-stale-old', s);
|
||||
const svc = buildService({ embedVector: conceptVec(7), userSpaceIds: [s] });
|
||||
const res = await search(svc, { query: 'квазонимиксseven', spaceId: s });
|
||||
expect(res.items.map((i: any) => i.id)).not.toContain(stale);
|
||||
});
|
||||
|
||||
// 8. Stale embedding space_id (moved page): a page currently in space B whose
|
||||
// embedding rows still carry space A (movePageToSpace does NOT reindex) is
|
||||
// STILL a vector hit when searching B — space scoping is enforced only by
|
||||
// the pages join (pages.space_id, always current), NOT by a stale
|
||||
// page_embeddings.space_id. Guards against re-adding that predicate (which
|
||||
// would wrongly drop the moved page's vector-only hit).
|
||||
it('#530-8 a vector hit with a STALE embedding space_id (moved page) is still returned', async () => {
|
||||
const newSpace = (await createSpace(db, workspaceId)).id;
|
||||
const oldSpace = (await createSpace(db, workspaceId)).id;
|
||||
// The page currently lives in newSpace (as after a move into newSpace).
|
||||
const page = await insertPage({
|
||||
title: 'перемещённая вект страница',
|
||||
textContent: 'содержимое без искомого термина',
|
||||
spaceId: newSpace,
|
||||
});
|
||||
// Its embedding was stamped with the OLD space and never reindexed.
|
||||
await plantEmbedding(page, conceptVec(8), ACTIVE_FP, oldSpace);
|
||||
const svc = buildService({
|
||||
embedVector: conceptVec(8),
|
||||
userSpaceIds: [newSpace],
|
||||
});
|
||||
const res = await search(svc, {
|
||||
query: 'квазонимиксeight',
|
||||
spaceId: newSpace,
|
||||
});
|
||||
// Survives despite page_embeddings.space_id (oldSpace) != pages.space_id.
|
||||
expect(res.items.map((i: any) => i.id)).toContain(page);
|
||||
expect(res.semantic.available).toBe(true);
|
||||
});
|
||||
|
||||
// 9. Statement-timeout safety net: a pathologically slow vector scan is
|
||||
// cancelled (SQLSTATE 57014) and search degrades to lexical-only — the
|
||||
// lexical hit survives, the vector-only hit vanishes, no 500/hang, and the
|
||||
// degraded event is logged. Mutation: make the fallback re-throw instead of
|
||||
// degrading -> this test reds (the call rejects with 57014).
|
||||
it('#530-9 a vector-query timeout degrades to lexical (57014), never 500', async () => {
|
||||
const s = (await createSpace(db, workspaceId)).id;
|
||||
const lexHit = await insertPage({
|
||||
title: 'семдевятьмаркер лексический',
|
||||
textContent: 'обычный текст',
|
||||
spaceId: s,
|
||||
});
|
||||
const vecOnly = await insertPage({
|
||||
title: 'нейтральный девять заголовок',
|
||||
textContent: 'без искомого термина',
|
||||
spaceId: s,
|
||||
});
|
||||
await plantEmbedding(vecOnly, conceptVec(9), ACTIVE_FP, s);
|
||||
|
||||
const warnSpy = jest
|
||||
.spyOn(Logger.prototype, 'warn')
|
||||
.mockImplementation(() => undefined as any);
|
||||
// Tiny per-statement timeout so the 0.5s sleeping arm is cancelled.
|
||||
process.env.SEARCH_VECTOR_STATEMENT_TIMEOUT_MS = '50';
|
||||
try {
|
||||
const svc = buildService({
|
||||
embedVector: conceptVec(9),
|
||||
slowVectorArm: true,
|
||||
userSpaceIds: [s],
|
||||
});
|
||||
const res = await search(svc, { query: 'семдевятьмаркер', spaceId: s });
|
||||
// Lexical result survives; the vector-only page does NOT (arm cancelled).
|
||||
const ids = res.items.map((i: any) => i.id);
|
||||
expect(ids).toContain(lexHit);
|
||||
expect(ids).not.toContain(vecOnly);
|
||||
expect(res.semantic.available).toBe(false);
|
||||
expect(res.semantic.reason).toBe('degraded');
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('search.semantic.degraded'),
|
||||
);
|
||||
} finally {
|
||||
delete process.env.SEARCH_VECTOR_STATEMENT_TIMEOUT_MS;
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
// 10. SET LOCAL scoping: the search's statement timeout must NOT leak onto a
|
||||
// later query on the same pooled connection. After a search that set a
|
||||
// tiny timeout, a deliberately-slow standalone query still completes.
|
||||
it('#530-10 the per-query statement timeout does not leak to later queries', async () => {
|
||||
const s = (await createSpace(db, workspaceId)).id;
|
||||
await insertPage({
|
||||
title: 'семдесятьмаркер тема',
|
||||
textContent: 'текст',
|
||||
spaceId: s,
|
||||
});
|
||||
process.env.SEARCH_VECTOR_STATEMENT_TIMEOUT_MS = '50';
|
||||
try {
|
||||
const svc = buildService({
|
||||
embedVector: conceptVec(10),
|
||||
slowVectorArm: true,
|
||||
userSpaceIds: [s],
|
||||
});
|
||||
// This search trips + resets the local timeout (via SET LOCAL semantics).
|
||||
await search(svc, { query: 'семдесятьмаркер', spaceId: s });
|
||||
// A subsequent query that sleeps 200ms must NOT be cancelled — proving the
|
||||
// 50ms search timeout did not persist on the connection (it would raise
|
||||
// 57014 if it had leaked). SET LOCAL auto-resets at the search tx end.
|
||||
await expect(sql`SELECT pg_sleep(0.2)`.execute(db)).resolves.toBeDefined();
|
||||
} finally {
|
||||
delete process.env.SEARCH_VECTOR_STATEMENT_TIMEOUT_MS;
|
||||
}
|
||||
});
|
||||
|
||||
// 11. COMMIT-path no-leak (locks is_local=true). #530-10 only covers the
|
||||
// ROLLBACK path (a timed-out search rolls back — a plain session SET would
|
||||
// ALSO be undone on rollback, so it would NOT catch is_local true->false).
|
||||
// Here a NORMAL, FAST semantic search COMMITs its transaction; a leaked
|
||||
// session-level statement_timeout would then persist on the pooled
|
||||
// connection and cancel later queries. We prove it does NOT: after the
|
||||
// committing search (bounded at a small 100ms), a 300ms query succeeds.
|
||||
// Because postgres.js pools, we run several sequential slow queries so at
|
||||
// least one reuses the connection the committed search ran on (which is
|
||||
// where a leaked timeout would live); ALL must succeed. Mutation: flip
|
||||
// is_local true->false -> the committed 100ms timeout leaks and the
|
||||
// follow-up pg_sleep is cancelled (57014), reddening this test.
|
||||
it('#530-11 a COMMITTED search does not leak its statement timeout (is_local)', async () => {
|
||||
const s = (await createSpace(db, workspaceId)).id;
|
||||
const page = await insertPage({
|
||||
title: 'семодиннадцать вект страница',
|
||||
textContent: 'содержимое без искомого термина',
|
||||
spaceId: s,
|
||||
});
|
||||
await plantEmbedding(page, conceptVec(11), ACTIVE_FP, s);
|
||||
// Small bound: the fast vector arm still completes (<100ms) so the search
|
||||
// COMMITs, but small enough that a LEAKED timeout would cancel the 300ms
|
||||
// follow-ups below.
|
||||
process.env.SEARCH_VECTOR_STATEMENT_TIMEOUT_MS = '100';
|
||||
try {
|
||||
const svc = buildService({
|
||||
embedVector: conceptVec(11),
|
||||
userSpaceIds: [s],
|
||||
});
|
||||
const res = await search(svc, { query: 'семодиннадцать', spaceId: s });
|
||||
// The (committing) vector path actually ran.
|
||||
expect(res.semantic.available).toBe(true);
|
||||
// Each 300ms query must SUCCEED — a leaked 100ms timeout would cancel the
|
||||
// one that reuses the committed search's connection. 8 sweeps cover the
|
||||
// whole pool (max 5 conns), so a leak on any connection is caught.
|
||||
for (let i = 0; i < 8; i++) {
|
||||
await expect(
|
||||
sql`SELECT pg_sleep(0.3)`.execute(db),
|
||||
).resolves.toBeDefined();
|
||||
}
|
||||
} finally {
|
||||
delete process.env.SEARCH_VECTOR_STATEMENT_TIMEOUT_MS;
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,11 +4,26 @@ services:
|
||||
depends_on:
|
||||
- db
|
||||
- redis
|
||||
- embeddings
|
||||
environment:
|
||||
APP_URL: 'http://localhost:3000'
|
||||
APP_SECRET: 'REPLACE_WITH_LONG_SECRET'
|
||||
DATABASE_URL: 'postgresql://docmost:STRONG_DB_PASSWORD@db:5432/docmost'
|
||||
REDIS_URL: 'redis://redis:6379'
|
||||
# #530 semantic search: the GLOBAL embedding provider (the `embeddings` TEI
|
||||
# sidecar below). A workspace that configures its own embedding provider
|
||||
# overrides this. TEI is OpenAI-compatible, so the endpoint is /v1.
|
||||
EMBEDDING_ENDPOINT: 'http://embeddings:80/v1'
|
||||
EMBEDDING_MODEL: 'intfloat/multilingual-e5-small'
|
||||
EMBEDDING_API_KEY: 'unused'
|
||||
EMBEDDING_DIMENSIONS: '384'
|
||||
# MUST match the --revision the sidecar pins (see EMBEDDING_REVISION in
|
||||
# .env). A revision bump changes the embedding fingerprint (PR-2 swaps
|
||||
# generations); keep the two in lockstep.
|
||||
EMBEDDING_REVISION: '${EMBEDDING_REVISION}'
|
||||
# e5 models require these input prefixes; empty them for a non-e5 model.
|
||||
EMBEDDING_QUERY_PREFIX: 'query: '
|
||||
EMBEDDING_DOC_PREFIX: 'passage: '
|
||||
ports:
|
||||
- "3000:3000"
|
||||
restart: unless-stopped
|
||||
@@ -39,7 +54,33 @@ services:
|
||||
volumes:
|
||||
- redis_data:/data
|
||||
|
||||
# #530 Text Embeddings Inference (TEI) sidecar — the GLOBAL embedding provider
|
||||
# for semantic search. OpenAI-compatible, reached only on the internal network
|
||||
# (no published port). The model + revision are PINNED so the embedding
|
||||
# fingerprint is stable; set EMBEDDING_REVISION to a real commit sha in .env.
|
||||
embeddings:
|
||||
image: ghcr.io/huggingface/text-embeddings-inference:cpu-1.9
|
||||
command:
|
||||
- "--model-id"
|
||||
- "intfloat/multilingual-e5-small"
|
||||
- "--revision"
|
||||
- "${EMBEDDING_REVISION}"
|
||||
restart: unless-stopped
|
||||
# Cache downloaded model weights so a restart does not re-download them.
|
||||
volumes:
|
||||
- tei-models:/data
|
||||
# GET /health is TEI's readiness probe. (Drop this block if your TEI image
|
||||
# variant ships without curl.)
|
||||
healthcheck:
|
||||
test: ["CMD-SHELL", "curl -fsS http://localhost:80/health || exit 1"]
|
||||
interval: 30s
|
||||
timeout: 5s
|
||||
retries: 5
|
||||
# Model download on first boot can be slow; don't flap as unhealthy meanwhile.
|
||||
start_period: 120s
|
||||
|
||||
volumes:
|
||||
docmost:
|
||||
db_data:
|
||||
redis_data:
|
||||
tei-models:
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
markdownToProseMirror,
|
||||
markdownToProseMirrorCanonical,
|
||||
mutatePageContent,
|
||||
savePageVersionRealtime,
|
||||
assertYjsEncodable,
|
||||
MutationResult,
|
||||
} from "../lib/collaboration.js";
|
||||
@@ -55,6 +56,7 @@ export interface IPagesMixin {
|
||||
listPageHistory(pageId: string, cursor?: string): any;
|
||||
getPageHistory(historyId: string): any;
|
||||
restorePageVersion(historyId: string): any;
|
||||
savePageVersion(pageId: string): any;
|
||||
diffPageVersions(pageId: string, from?: string, to?: string): any;
|
||||
}
|
||||
|
||||
@@ -587,6 +589,24 @@ export function PagesMixin<TBase extends GConstructor<DocmostClientContext>>(Bas
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Save an intentional NAMED version of a page's CURRENT live content (#370).
|
||||
* The write goes over the same agent-authenticated collab session content edits
|
||||
* use, so the server derives kind='agent' from the signed actor. Resolves a
|
||||
* SaveVersionResult: `{ saved:true, historyId, kind, alreadySaved }` on success,
|
||||
* or `{ saved:false, skipped:true, reason }` when the server had nothing to pin
|
||||
* (e.g. an empty page). A stale/missing pageId throws (not a benign skip).
|
||||
*/
|
||||
async savePageVersion(pageId: string) {
|
||||
const collabToken = await this.getCollabTokenWithReauth();
|
||||
const pageUuid = await this.resolvePageId(pageId);
|
||||
// Self-heal a rejected WS handshake once (#486): re-mint the collab token and
|
||||
// retry, symmetric to the content-write path.
|
||||
return this.writeWithCollabAuthRetry(collabToken, (token) =>
|
||||
savePageVersionRealtime(pageUuid, token, this.apiUrl),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Diff two versions of a page and return a Docmost-equivalent change set.
|
||||
* `from`/`to` each resolve to a ProseMirror doc:
|
||||
|
||||
@@ -139,7 +139,13 @@ export interface CollabProviderLike {
|
||||
unsyncedChanges: number;
|
||||
destroy(): void;
|
||||
on(event: "unsyncedChanges", handler: (data: { number: number }) => void): void;
|
||||
on(event: "stateless", handler: (data: { payload: string }) => void): void;
|
||||
off(event: "unsyncedChanges", handler: (data: { number: number }) => void): void;
|
||||
off(event: "stateless", handler: (data: { payload: string }) => void): void;
|
||||
// Send a stateless (out-of-band, non-doc) message to the server. Used by the
|
||||
// #370 explicit save-version path (payload `{type:'save-version'}`); the server
|
||||
// replies with a broadcast `{type:'version.saved', …}` on the same channel.
|
||||
sendStateless(payload: string): void;
|
||||
}
|
||||
|
||||
/** The configuration object passed to the provider factory. */
|
||||
@@ -564,6 +570,125 @@ export class CollabSession {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Send a stateless message over the live collaboration connection and resolve
|
||||
* with the FIRST reply whose parsed payload satisfies `predicate`, rejecting on
|
||||
* a bounded `timeoutMs`.
|
||||
*
|
||||
* Used by the #370 explicit save-version path: the client sends
|
||||
* `{type:'save-version'}` and awaits the server's broadcast
|
||||
* `{type:'version.saved', historyId, kind, alreadySaved}`. The stateless channel
|
||||
* (not a REST read) is deliberate — the server versions the LIVE in-memory ydoc,
|
||||
* which the up-to-10s-stale page row would not yet reflect.
|
||||
*
|
||||
* `predicate` receives each incoming stateless message (already JSON-parsed) and
|
||||
* returns the resolved value for a match or `undefined` to keep waiting; a
|
||||
* malformed / unrelated payload is ignored. The listener is registered BEFORE
|
||||
* the send so a fast reply is never missed.
|
||||
*
|
||||
* Lifecycle mirrors mutate(): it registers through `inflightReject` so a
|
||||
* disconnect/close/auth-failure/teardown rejects THIS wait with the same
|
||||
* connection-loss error, refuses to run on a non-ready session, fails fast on a
|
||||
* concurrent in-flight op (the caller MUST hold the per-page lock), and re-arms
|
||||
* the idle TTL (or self-destroys an ephemeral session) on completion.
|
||||
*
|
||||
* NOTE: the server broadcast carries no request-correlation id, so under a
|
||||
* genuinely concurrent save on the SAME page (e.g. a human Cmd+S racing the
|
||||
* agent) this resolves on whichever `version.saved` arrives first. The per-page
|
||||
* lock serializes THIS process's saves; a cross-client race is inherent to the
|
||||
* broadcast design and is benign (the result still describes a real save of this
|
||||
* page's current content, and the server's save is promote-not-duplicate).
|
||||
*/
|
||||
sendStatelessAndAwait<T>(
|
||||
payload: string,
|
||||
predicate: (message: any) => T | undefined,
|
||||
timeoutMs: number,
|
||||
): Promise<T> {
|
||||
// Belt-and-suspenders (acquire already validated): refuse to send on a
|
||||
// session that is not in a live, synced, ready state.
|
||||
if (
|
||||
this.dead ||
|
||||
this.state !== "ready" ||
|
||||
this.connectionLost ||
|
||||
!this.provider ||
|
||||
this.provider.synced !== true
|
||||
) {
|
||||
return Promise.reject(
|
||||
new Error("Collaboration session is not in a ready state"),
|
||||
);
|
||||
}
|
||||
|
||||
// Fail-fast on concurrent use: a second overlapping op would overwrite the
|
||||
// first's inflightReject and hang it on disconnect (same guard as mutate).
|
||||
if (this.inflightReject) {
|
||||
return Promise.reject(
|
||||
new Error(
|
||||
"stateless op already in-flight; caller must serialize (hold the page lock)",
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
return new Promise<T>((resolve, reject) => {
|
||||
let settled = false;
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
let statelessHandler: ((data: { payload: string }) => void) | undefined;
|
||||
|
||||
const localFinish = (err: Error | null, value?: T) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
if (timer) clearTimeout(timer);
|
||||
if (statelessHandler && this.provider) {
|
||||
try {
|
||||
this.provider.off("stateless", statelessHandler);
|
||||
} catch (e) {}
|
||||
}
|
||||
this.inflightReject = undefined;
|
||||
if (err) reject(err);
|
||||
else resolve(value as T);
|
||||
// Post-settle lifecycle, identical to mutate's localFinish.
|
||||
if (this.ephemeral) {
|
||||
this.destroy("ephemeral op complete");
|
||||
} else if (!this.dead) {
|
||||
this.armIdle();
|
||||
}
|
||||
};
|
||||
|
||||
// Register so a disconnect/close/auth-failure/teardown rejects THIS op with
|
||||
// the connection-loss error text (the `settled` guard makes a racing
|
||||
// teardown + normal resolve safe — first one wins).
|
||||
this.inflightReject = (e: Error) => localFinish(e);
|
||||
|
||||
// Register the reply listener BEFORE sending so a fast server broadcast is
|
||||
// never missed.
|
||||
statelessHandler = (data: { payload: string }) => {
|
||||
if (settled) return;
|
||||
let message: any;
|
||||
try {
|
||||
message = JSON.parse(data.payload);
|
||||
} catch {
|
||||
return; // unrelated / malformed stateless message — keep waiting
|
||||
}
|
||||
const matched = predicate(message);
|
||||
if (matched !== undefined) localFinish(null, matched);
|
||||
};
|
||||
this.provider!.on("stateless", statelessHandler);
|
||||
|
||||
timer = setTimeout(() => {
|
||||
localFinish(
|
||||
new Error(
|
||||
`Timeout waiting for a stateless reply from the collaboration server ${this.hint()}`,
|
||||
),
|
||||
);
|
||||
}, timeoutMs);
|
||||
|
||||
try {
|
||||
this.provider!.sendStateless(payload);
|
||||
} catch (e) {
|
||||
localFinish(e instanceof Error ? e : new Error(String(e)));
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/** (Re)arm the idle TTL so the clock starts from the most recent activity. */
|
||||
armIdle(): void {
|
||||
if (this.dead || this.ephemeral) return;
|
||||
|
||||
@@ -324,6 +324,146 @@ export async function mutatePageContent(
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Stateless-channel message types for the #370 explicit save-version handshake.
|
||||
* These MUST match the server constants in
|
||||
* apps/server/src/collaboration/extensions/persistence.extension.ts
|
||||
* (SAVE_VERSION_MESSAGE_TYPE and the VERSION_SAVED / VERSION_SKIPPED replies) —
|
||||
* the mcp package cannot import server code, so the literals are duplicated here.
|
||||
* KEEP THE TWO SIDES IN SYNC: the server broadcasts exactly ONE terminal reply per
|
||||
* handled save (`version.saved` for a real save/promote, `version.skipped` for a
|
||||
* reachable no-op — an empty page or a missing page row); the client must match
|
||||
* BOTH or it waits out the ack timeout on the skip case and misreports a healthy
|
||||
* server as unreachable (#370 F2).
|
||||
*/
|
||||
const SAVE_VERSION_MESSAGE_TYPE = "save-version";
|
||||
const VERSION_SAVED_MESSAGE_TYPE = "version.saved";
|
||||
const VERSION_SKIPPED_MESSAGE_TYPE = "version.skipped";
|
||||
|
||||
/**
|
||||
* Bounded wait for the server's terminal reply after we ask it to save a version.
|
||||
* The server flushes the live ydoc through its store path and writes a history row
|
||||
* inside a DB transaction before broadcasting, so allow the same headroom as a
|
||||
* persistence ack; reject (do NOT hang) past it. A timeout here means NO reply at
|
||||
* all arrived — the genuine "collab server unreachable/overloaded" case — as
|
||||
* distinct from a `version.skipped` reply, which resolves immediately.
|
||||
*/
|
||||
const SAVE_VERSION_ACK_TIMEOUT_MS = 20000;
|
||||
|
||||
/**
|
||||
* The resolved outcome of an explicit save-version, surfaced to the tool caller.
|
||||
* `saved:true` → a version was created or promoted (historyId/kind/alreadySaved
|
||||
* are present). `saved:false` → the server had nothing to pin (a reachable no-op,
|
||||
* e.g. an empty page); `skipped` + `reason` explain why. A page-not-found reply is
|
||||
* NOT represented here — it is surfaced as a thrown error (a bad/stale pageId).
|
||||
*/
|
||||
export interface SaveVersionResult {
|
||||
saved: boolean;
|
||||
historyId?: string;
|
||||
kind?: string;
|
||||
alreadySaved?: boolean;
|
||||
skipped?: boolean;
|
||||
reason?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parsed terminal reply the predicate hands back to savePageVersionRealtime. Kept
|
||||
* internal: it carries the page-not-found case (mapped to a thrown error, never a
|
||||
* returned result) that SaveVersionResult deliberately does not model.
|
||||
*/
|
||||
type SaveVersionReply =
|
||||
| { outcome: "saved"; historyId: string; kind: string; alreadySaved: boolean }
|
||||
| { outcome: "skipped"; reason: string };
|
||||
|
||||
/** Match the server's terminal reply (version.saved / version.skipped), ignoring
|
||||
* any unrelated / cross-page stateless message so the wait is not resolved by
|
||||
* noise. Returns `undefined` to keep waiting (#370 F1 predicate coverage). */
|
||||
function matchSaveVersionReply(message: any): SaveVersionReply | undefined {
|
||||
if (!message || typeof message !== "object") return undefined;
|
||||
if (message.type === VERSION_SAVED_MESSAGE_TYPE) {
|
||||
return {
|
||||
outcome: "saved",
|
||||
historyId: String(message.historyId),
|
||||
kind: String(message.kind),
|
||||
alreadySaved: !!message.alreadySaved,
|
||||
};
|
||||
}
|
||||
if (message.type === VERSION_SKIPPED_MESSAGE_TYPE) {
|
||||
return { outcome: "skipped", reason: String(message.reason ?? "unknown") };
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Save an intentional version of a page's CURRENT live collaboration content
|
||||
* (#370). Runs under the per-page lock, acquires the SAME cached CollabSession the
|
||||
* content writes use (#400) — so it authenticates with the caller's agent collab
|
||||
* token, and the server derives kind='agent' from that signed actor — then sends a
|
||||
* `{type:'save-version'}` stateless message and awaits the server's terminal reply
|
||||
* (`version.saved` or `version.skipped`) on the same channel.
|
||||
*
|
||||
* Deliberately does NOT read `pages.content` over REST: the versioned content is
|
||||
* the live in-memory ydoc, which the debounced (up-to-10s-stale) page row would
|
||||
* not yet reflect. The stateless round-trip is what makes the save exact.
|
||||
*
|
||||
* Outcomes:
|
||||
* - a real save/promote → `{ saved:true, historyId, kind, alreadySaved }`;
|
||||
* - the server had nothing to pin (empty page) → `{ saved:false, skipped:true,
|
||||
* reason:'empty' }` — a clean, immediate no-op, NOT a stall;
|
||||
* - the page row is gone (a stale/bad pageId) → a thrown error, immediate and
|
||||
* truthful (not the health-timeout path);
|
||||
* - no reply within the timeout → a thrown timeout error (the genuine "server
|
||||
* unreachable/overloaded" case).
|
||||
*
|
||||
* On a TRANSPORT failure (timeout / disconnect) the session is destroyed so the
|
||||
* next call reconnects fresh; a page-not-found is a healthy-connection terminal
|
||||
* reply, so the session is left cached. A save is safe to retry — the server
|
||||
* promotes-not-duplicates an identical latest version — so the caller (and its
|
||||
* agent) may re-issue it without risking a duplicate heavy history row.
|
||||
*/
|
||||
export async function savePageVersionRealtime(
|
||||
pageId: PageId,
|
||||
collabToken: string,
|
||||
baseUrl: string,
|
||||
): Promise<SaveVersionResult> {
|
||||
return withPageLock(pageId, async () => {
|
||||
const session = await acquireCollabSession(pageId, collabToken, baseUrl);
|
||||
let reply: SaveVersionReply;
|
||||
try {
|
||||
reply = await session.sendStatelessAndAwait<SaveVersionReply>(
|
||||
JSON.stringify({ type: SAVE_VERSION_MESSAGE_TYPE }),
|
||||
matchSaveVersionReply,
|
||||
SAVE_VERSION_ACK_TIMEOUT_MS,
|
||||
);
|
||||
} catch (e) {
|
||||
// TRANSPORT failure (no reply within the timeout, or a disconnect): drop the
|
||||
// session so the next call reconnects fresh.
|
||||
session.destroy("save-version failed");
|
||||
throw e;
|
||||
}
|
||||
// A terminal reply arrived over a healthy connection — do NOT destroy the
|
||||
// session; interpret the outcome.
|
||||
if (reply.outcome === "skipped") {
|
||||
if (reply.reason === "page-not-found") {
|
||||
// A resolved pageId that the collab server no longer holds (deleted, or a
|
||||
// stale id). Surface it immediately and truthfully, not as a health error.
|
||||
throw new Error(
|
||||
`savePageVersion: page ${pageId} was not found on the collaboration ` +
|
||||
`server (it may have been deleted) — nothing was saved.`,
|
||||
);
|
||||
}
|
||||
// Reachable benign no-op (e.g. an empty page): a clean result, not a throw.
|
||||
return { saved: false, skipped: true, reason: reply.reason };
|
||||
}
|
||||
return {
|
||||
saved: true,
|
||||
historyId: reply.historyId,
|
||||
kind: reply.kind,
|
||||
alreadySaved: reply.alreadySaved,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Replace the live content of a page over the collaboration websocket.
|
||||
* Accepts a ready ProseMirror JSON document; the caller controls whether
|
||||
|
||||
@@ -44,7 +44,7 @@ export const ROUTING_PROSE =
|
||||
"EDIT: fix wording/typos/numbers -> editPageText (find/replace inside blocks, no node id needed). Edit a block -> getNode(markdown) -> edit the markdown -> patchNode(markdown) (by attrs.id from getOutline; the markdown fragment may be several blocks — a 1->N section rewrite in one call, the first block keeps the id). Reach for patchNode's `node`-JSON only for fine attr/mark work; a table cell with spans/colors/fixed width -> the table tools (patchNode markdown refuses it). Add a block -> insertNode (markdown, before/after a block by attrs.id or by anchor text, or append; `node` for raw JSON or bare table structure). Remove a block -> deleteNode (by attrs.id). Tables -> tableGet / tableUpdateCell / tableInsertRow / tableDeleteRow (address by \"#<index>\" from getOutline; table nodes have no attrs.id). Images -> insertImage (add from a web URL) / replaceImage (swap an existing image). Draw.io diagrams -> PREFER the high-level semantic tools that hide coordinates/styles: drawioFromGraph (architecture/cloud/network diagrams — describe nodes/groups/edges by kind+icon, the server picks layout, colors and verified icons; hints layer/sameLayerAs/pinned and layout:full|incremental|none) and drawioFromMermaid (standard flowcharts — write Mermaid, get an editable diagram). For targeted tweaks of an existing diagram use drawioEditCells (id-based add/update/delete with cascade delete + baseHash lock). Raw mxGraph XML via drawioCreate/drawioUpdate is the escape-hatch for exotic/wireframe diagrams; drawioGet reads a diagram as mxGraph XML + a hash (pass it as baseHash to drawioUpdate/drawioEditCells for optimistic locking). Before authoring raw XML, drawioShapes (look up verified stencil style-strings so a shape name never renders as an empty box) and drawioGuide (on-demand authoring reference: skeleton/layout/containers/icons-aws/icons-azure), and pass layout:\"elk\" to drawioCreate/drawioUpdate to auto-place nodes. Footnotes -> insertFootnote. Bulk/structural rewrite -> updatePageJson (full ProseMirror replace) or updatePageMarkdown (full plain-Markdown body replace, re-imported — block ids regenerate); prefer the granular tools above to avoid resending the whole ~100KB+ document. Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmostTransform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" +
|
||||
"PAGES: new -> createPage (Markdown). Rename (title only) -> renamePage. Move -> movePage. Delete -> deletePage (SOFT delete — the page goes to trash and is restorable; nothing is permanent). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copyPageContent. Sharing -> sharePage / unsharePage / listShares; sharePage makes the page PUBLICLY accessible — do it only when explicitly asked.\n" +
|
||||
"COMMENTS: createComment is always inline and requires an EXACT selection — contiguous text from a single block, <=250 chars (fails rather than leaving an unanchored comment); reply to a thread via parentCommentId. Propose a concrete text fix for one-click human approval -> createComment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Manage -> listComments, updateComment, resolveComment (resolve/reopen, reversible — prefer over delete to close), deleteComment, checkNewComments.\n" +
|
||||
"HISTORY: review what changed -> diffPageVersions (a historyId vs current, or two versions). List saved versions -> listPageHistory. Undo a bad edit -> restorePageVersion (writes a past version back as current; itself revertible). Export a page to self-contained Docmost Markdown (with comment anchors) -> exportPageMarkdown.";
|
||||
"HISTORY: review what changed -> diffPageVersions (a historyId vs current, or two versions). List saved versions -> listPageHistory. Undo a bad edit -> restorePageVersion (writes a past version back as current; itself revertible). Pin the page's CURRENT content as a restorable named checkpoint when you finish a coherent editing pass -> savePageVersion (kind derived server-side as an agent version; an identical-to-last save is promoted/no-op'd, so a redundant call is harmless). Export a page to self-contained Docmost Markdown (with comment anchors) -> exportPageMarkdown.";
|
||||
|
||||
/**
|
||||
* Non-tool camelCase identifiers that legitimately appear in ROUTING_PROSE:
|
||||
@@ -218,6 +218,7 @@ const TOOL_FAMILY: Record<string, Family> = {
|
||||
diffPageVersions: "HISTORY",
|
||||
listPageHistory: "HISTORY",
|
||||
restorePageVersion: "HISTORY",
|
||||
savePageVersion: "HISTORY",
|
||||
exportPageMarkdown: "HISTORY",
|
||||
// importPageMarkdown is now inAppOnly (#411) — it is not registered on the
|
||||
// external MCP host, so it no longer appears in the generated inventory.
|
||||
|
||||
@@ -93,6 +93,7 @@ export type DocmostClientLike = Pick<
|
||||
| 'sharePage'
|
||||
| 'unsharePage'
|
||||
| 'restorePageVersion'
|
||||
| 'savePageVersion'
|
||||
| 'stashPage'
|
||||
| 'insertFootnote'
|
||||
| 'insertImage'
|
||||
@@ -743,6 +744,30 @@ export const SHARED_TOOL_SPECS = {
|
||||
client.restorePageVersion(historyId as string),
|
||||
},
|
||||
|
||||
savePageVersion: {
|
||||
mcpName: 'savePageVersion',
|
||||
inAppKey: 'savePageVersion',
|
||||
writeClass: 'write',
|
||||
description:
|
||||
'Save an intentional, NAMED version (kind=agent) of the page\'s CURRENT ' +
|
||||
'live content — a restorable checkpoint pinned into its history. Call it ' +
|
||||
'when you have FINISHED a coherent editing pass (not after every small ' +
|
||||
'edit), so the reader can see and roll back to the state you left. The ' +
|
||||
'version type is derived SERVER-SIDE from your signed agent identity (you ' +
|
||||
'cannot mislabel it); a save whose content is IDENTICAL to the last saved ' +
|
||||
'version is promoted/no-op\'d server-side, so a redundant call is harmless ' +
|
||||
'and never duplicates a version. Returns { saved:true, historyId, kind, ' +
|
||||
'alreadySaved } on success, or { saved:false, skipped:true, reason:\'empty\' } ' +
|
||||
'when the page had nothing to pin (e.g. it was empty).',
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
'savePageVersion — pin the page\'s current content as a named agent version (restorable checkpoint).',
|
||||
buildShape: (z) => ({
|
||||
pageId: z.string().min(1),
|
||||
}),
|
||||
execute: (client, { pageId }) => client.savePageVersion(pageId as string),
|
||||
},
|
||||
|
||||
// --- markdown round-trip ---
|
||||
|
||||
importPageMarkdown: {
|
||||
|
||||
@@ -0,0 +1,300 @@
|
||||
import { test, beforeEach, afterEach, mock } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { EventEmitter } from "node:events";
|
||||
|
||||
import {
|
||||
acquireCollabSession,
|
||||
destroyAllSessions,
|
||||
__setCollabProviderFactory,
|
||||
} from "../../build/lib/collab-session.js";
|
||||
import { savePageVersionRealtime } from "../../build/lib/collaboration.js";
|
||||
|
||||
// #370 — unit coverage for the MCP client transport of the explicit save-version.
|
||||
// There is no collaboration server in the test env, so a fake HocuspocusProvider
|
||||
// drives the stateless round-trip: it auto-completes the connect/sync handshake on
|
||||
// a microtask (like the real provider), records sends, and either auto-broadcasts a
|
||||
// configured reply (`opts.reply`) or lets the test emit stateless messages /
|
||||
// disconnects by hand (`_emitStateless` / `_disconnect`) to exercise the predicate
|
||||
// filter, the teardown-mid-wait path, and the timeout.
|
||||
class FakeProvider extends EventEmitter {
|
||||
static instances = [];
|
||||
|
||||
static reset() {
|
||||
FakeProvider.instances = [];
|
||||
}
|
||||
|
||||
static last() {
|
||||
return FakeProvider.instances[FakeProvider.instances.length - 1];
|
||||
}
|
||||
|
||||
constructor(config, opts = {}) {
|
||||
super();
|
||||
this.config = config;
|
||||
this.synced = false;
|
||||
this.unsyncedChanges = 0;
|
||||
this.destroyed = false;
|
||||
this.sent = [];
|
||||
this.opts = opts;
|
||||
FakeProvider.instances.push(this);
|
||||
// Real HocuspocusProvider fires onSynced asynchronously after the handshake;
|
||||
// a microtask reproduces that without depending on timers (so mock.timers on
|
||||
// setTimeout does not stall the connect).
|
||||
queueMicrotask(() => {
|
||||
if (this.destroyed) return;
|
||||
this.config.onConnect?.();
|
||||
this.synced = true;
|
||||
this.config.onSynced?.();
|
||||
});
|
||||
}
|
||||
|
||||
destroy() {
|
||||
this.destroyed = true;
|
||||
}
|
||||
|
||||
sendStateless(payload) {
|
||||
this.sent.push(payload);
|
||||
const reply = this.opts.reply;
|
||||
if (!reply) return; // silent → the caller times out, or the test drives it
|
||||
// Broadcast the configured server reply on a microtask, mirroring
|
||||
// document.broadcastStateless landing back on this provider's `stateless` event.
|
||||
queueMicrotask(() => {
|
||||
if (this.destroyed) return;
|
||||
this.emit("stateless", { payload: JSON.stringify(reply) });
|
||||
});
|
||||
}
|
||||
|
||||
// --- test drivers ---
|
||||
_emitStateless(obj) {
|
||||
this.emit("stateless", { payload: JSON.stringify(obj) });
|
||||
}
|
||||
_disconnect() {
|
||||
this.config.onDisconnect?.();
|
||||
}
|
||||
}
|
||||
|
||||
const ENV_KEYS = [
|
||||
"MCP_COLLAB_SESSION_IDLE_MS",
|
||||
"MCP_COLLAB_SESSION_MAX_AGE_MS",
|
||||
"MCP_COLLAB_SESSION_MAX_ENTRIES",
|
||||
"MCP_COLLAB_TOKEN_TTL_MS",
|
||||
];
|
||||
let savedEnv;
|
||||
|
||||
beforeEach(() => {
|
||||
savedEnv = {};
|
||||
for (const k of ENV_KEYS) savedEnv[k] = process.env[k];
|
||||
FakeProvider.reset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
destroyAllSessions();
|
||||
__setCollabProviderFactory(null);
|
||||
mock.timers.reset();
|
||||
for (const k of ENV_KEYS) {
|
||||
if (savedEnv[k] === undefined) delete process.env[k];
|
||||
else process.env[k] = savedEnv[k];
|
||||
}
|
||||
});
|
||||
|
||||
const PAGE_A = "11111111-1111-4111-8111-111111111111";
|
||||
const PAGE_B = "22222222-2222-4222-8222-222222222222";
|
||||
const PAGE_C = "33333333-3333-4333-8333-333333333333";
|
||||
const PAGE_D = "44444444-4444-4444-8444-444444444444";
|
||||
const PAGE_E = "55555555-5555-4555-8555-555555555555";
|
||||
const PAGE_F = "66666666-6666-4666-8666-666666666666";
|
||||
|
||||
// --- happy paths (resolve with the server's terminal reply) -----------------
|
||||
|
||||
test("saved: resolves { saved:true, historyId, kind, alreadySaved }", async () => {
|
||||
__setCollabProviderFactory(
|
||||
(config) =>
|
||||
new FakeProvider(config, {
|
||||
reply: {
|
||||
type: "version.saved",
|
||||
historyId: "hist-42",
|
||||
kind: "agent",
|
||||
alreadySaved: false,
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await savePageVersionRealtime(PAGE_A, "collab-tok", "http://host/api");
|
||||
|
||||
assert.deepEqual(result, {
|
||||
saved: true,
|
||||
historyId: "hist-42",
|
||||
kind: "agent",
|
||||
alreadySaved: false,
|
||||
});
|
||||
// The client actually asked the server to save a version (the stateless send).
|
||||
assert.deepEqual(
|
||||
FakeProvider.last().sent.map((p) => JSON.parse(p)),
|
||||
[{ type: "save-version" }],
|
||||
);
|
||||
});
|
||||
|
||||
test("saved: surfaces alreadySaved=true (promote-not-dup / no-op save)", async () => {
|
||||
__setCollabProviderFactory(
|
||||
(config) =>
|
||||
new FakeProvider(config, {
|
||||
reply: {
|
||||
type: "version.saved",
|
||||
historyId: "hist-7",
|
||||
kind: "manual",
|
||||
alreadySaved: true,
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await savePageVersionRealtime(PAGE_B, "collab-tok", "http://host/api");
|
||||
|
||||
assert.deepEqual(result, {
|
||||
saved: true,
|
||||
historyId: "hist-7",
|
||||
kind: "manual",
|
||||
alreadySaved: true,
|
||||
});
|
||||
});
|
||||
|
||||
// --- F2: terminal SKIP replies (no timeout, no false "unreachable") ---------
|
||||
|
||||
test("F2 skip: an empty-page reply resolves to a clean { saved:false, skipped:true, reason:'empty' } — no timeout", async () => {
|
||||
__setCollabProviderFactory(
|
||||
(config) =>
|
||||
new FakeProvider(config, {
|
||||
reply: { type: "version.skipped", reason: "empty" },
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await savePageVersionRealtime(PAGE_C, "collab-tok", "http://host/api");
|
||||
|
||||
assert.deepEqual(result, { saved: false, skipped: true, reason: "empty" });
|
||||
});
|
||||
|
||||
test("F2 skip: a page-not-found reply throws an immediate, truthful error (not the health-timeout path)", async () => {
|
||||
__setCollabProviderFactory(
|
||||
(config) =>
|
||||
new FakeProvider(config, {
|
||||
reply: { type: "version.skipped", reason: "page-not-found" },
|
||||
}),
|
||||
);
|
||||
|
||||
await assert.rejects(
|
||||
savePageVersionRealtime(PAGE_D, "collab-tok", "http://host/api"),
|
||||
/was not found on the collaboration server/,
|
||||
);
|
||||
});
|
||||
|
||||
// --- F1 (a): predicate filters unrelated messages, resolves on the real reply ---
|
||||
|
||||
test("F1(a) unrelated-then-real: an unrelated stateless message does NOT resolve; the later version.saved does", async () => {
|
||||
__setCollabProviderFactory((config) => new FakeProvider(config, {})); // manual drive
|
||||
|
||||
const p = savePageVersionRealtime(PAGE_E, "collab-tok", "http://host/api");
|
||||
// Let acquire's handshake + the stateless send complete so the listener is armed.
|
||||
await new Promise((r) => setImmediate(r));
|
||||
const provider = FakeProvider.last();
|
||||
|
||||
// Noise first: an unrelated stateless type (e.g. another feature's broadcast).
|
||||
provider._emitStateless({ type: "something-else", historyId: "NOPE" });
|
||||
// The wait must still be pending — a microtask turn would have resolved it if the
|
||||
// predicate wrongly matched.
|
||||
let settledEarly = false;
|
||||
p.then(() => (settledEarly = true)).catch(() => (settledEarly = true));
|
||||
await new Promise((r) => setImmediate(r));
|
||||
assert.equal(settledEarly, false, "resolved on an unrelated stateless message");
|
||||
|
||||
// Now the real reply arrives → it resolves with that payload.
|
||||
provider._emitStateless({
|
||||
type: "version.saved",
|
||||
historyId: "h-real",
|
||||
kind: "agent",
|
||||
alreadySaved: false,
|
||||
});
|
||||
const result = await p;
|
||||
assert.deepEqual(result, {
|
||||
saved: true,
|
||||
historyId: "h-real",
|
||||
kind: "agent",
|
||||
alreadySaved: false,
|
||||
});
|
||||
});
|
||||
|
||||
// --- F1 (b): teardown mid-wait rejects AND removes the stateless listener -----
|
||||
|
||||
test("F1(b) teardown mid-wait: a disconnect rejects the wait with the connection-loss error AND removes the stateless listener (no leak)", async () => {
|
||||
__setCollabProviderFactory((config) => new FakeProvider(config, {})); // silent
|
||||
|
||||
const session = await acquireCollabSession(PAGE_F, "collab-tok", "http://host/api");
|
||||
const provider = FakeProvider.last();
|
||||
|
||||
const p = session.sendStatelessAndAwait(
|
||||
JSON.stringify({ type: "save-version" }),
|
||||
(m) => (m && m.type === "version.saved" ? m : undefined),
|
||||
20000,
|
||||
);
|
||||
// Swallow the eventual rejection so it does not race the assertions below.
|
||||
p.catch(() => {});
|
||||
await new Promise((r) => setImmediate(r));
|
||||
|
||||
// The wait armed exactly one stateless listener.
|
||||
assert.equal(
|
||||
provider.listenerCount("stateless"),
|
||||
1,
|
||||
"the wait must register one stateless listener",
|
||||
);
|
||||
|
||||
// A disconnect tears the session down mid-wait.
|
||||
provider._disconnect();
|
||||
|
||||
await assert.rejects(p, /connection closed before the update was persisted/);
|
||||
// The listener was removed on teardown — dropping provider.off() reds this.
|
||||
assert.equal(
|
||||
provider.listenerCount("stateless"),
|
||||
0,
|
||||
"the stateless listener leaked after teardown",
|
||||
);
|
||||
});
|
||||
|
||||
// --- F1 (c): concurrent-in-flight guard fails fast, no cross-wiring -----------
|
||||
|
||||
test("F1(c) concurrent guard: a second sendStatelessAndAwait while one is in flight fails fast", async () => {
|
||||
__setCollabProviderFactory((config) => new FakeProvider(config, {})); // silent
|
||||
|
||||
const session = await acquireCollabSession(PAGE_A, "collab-tok", "http://host/api");
|
||||
|
||||
const first = session.sendStatelessAndAwait(
|
||||
JSON.stringify({ type: "save-version" }),
|
||||
(m) => (m && m.type === "version.saved" ? m : undefined),
|
||||
20000,
|
||||
);
|
||||
first.catch(() => {}); // it stays pending until afterEach destroys the session
|
||||
|
||||
await assert.rejects(
|
||||
session.sendStatelessAndAwait(
|
||||
JSON.stringify({ type: "save-version" }),
|
||||
(m) => (m && m.type === "version.saved" ? m : undefined),
|
||||
20000,
|
||||
),
|
||||
/already in-flight/,
|
||||
);
|
||||
});
|
||||
|
||||
// --- timeout: NO reply at all is the genuine "server unreachable" path --------
|
||||
|
||||
test("timeout: rejects on a bounded timeout when the server never replies", async () => {
|
||||
__setCollabProviderFactory((config) => new FakeProvider(config, {})); // silent
|
||||
|
||||
mock.timers.enable({ apis: ["setTimeout"] });
|
||||
const p = savePageVersionRealtime(PAGE_B, "collab-tok", "http://host/api");
|
||||
// Swallow the eventual rejection so an unhandled-rejection does not race the tick.
|
||||
p.catch(() => {});
|
||||
|
||||
// Flush the (microtask-driven) connect handshake + the stateless send so the
|
||||
// ack timer is armed before we advance the clock.
|
||||
await new Promise((r) => setImmediate(r));
|
||||
// Advance well past the 20s ack window; the connect timer (25s) does not fire.
|
||||
mock.timers.tick(20000 + 100);
|
||||
|
||||
await assert.rejects(p, /Timeout waiting for a stateless reply/);
|
||||
});
|
||||
Reference in New Issue
Block a user