diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx index 2e0feb3db3..b55f67b149 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx @@ -57,6 +57,7 @@ import type { PolicyChatUIMessage } from '../types'; import { PolicyAiAssistant } from './ai/policy-ai-assistant'; import { useSuggestions } from '../hooks/use-suggestions'; import { buildPositionMap } from '../lib/build-position-map'; +import { sanitizeMarkdown } from '../lib/policy-markdown'; import { resolveInitialPolicyContent } from '../lib/resolve-initial-content'; import { InlineEditBubble } from './ai/inline-edit-bubble'; import { markdownToTipTapJSON } from './ai/markdown-utils'; @@ -111,7 +112,9 @@ function getLatestCompletedProposal(messages: PolicyChatUIMessage[]): LatestProp latest = { key: `${msg.id}:${index}`, - content: input.content, + // Strip control-char noise (e.g. stray "013" glyphs) before the content + // feeds the diff + apply pipeline, so both see identical clean text. + content: sanitizeMarkdown(input.content), summary: input.summary ?? 'Proposing policy changes', title: input.title ?? input.summary ?? 'Policy updates ready for your review', detail: @@ -471,6 +474,7 @@ export function PolicyContentManager({ messages, status, sendMessage: baseSendMessage, + regenerate, stop: stopChat, } = useChat({ transport: new DefaultChatTransport({ @@ -482,16 +486,28 @@ export function PolicyContentManager({ }, }); + // The AI needs the live editor content (it may hold unsaved accepted changes), + // so every request — initial and retry — re-derives it from the current doc. + const buildChatBody = useCallback( + () => ({ + currentContent: editorInstance + ? buildPositionMap(editorInstance.state.doc).markdown + : '', + }), + [editorInstance], + ); + const sendMessage = (payload: { text: string }) => { setChatErrorMessage(null); - // Send current editor content so the AI sees the latest state, - // not stale DB content (e.g. after accepting changes) - const currentContent = editorInstance - ? buildPositionMap(editorInstance.state.doc).markdown - : ''; - baseSendMessage(payload, { body: { currentContent } }); + baseSendMessage(payload, { body: buildChatBody() }); }; + // Recover from an interrupted/incomplete run by re-running the last request. + const handleRetryChat = useCallback(() => { + setChatErrorMessage(null); + regenerate({ body: buildChatBody() }); + }, [regenerate, buildChatBody]); + // ── Proposal state management ────────────────────────────────────── // Scan ALL assistant messages for the latest completed proposePolicy tool call. // Unlike before, this doesn't only check the last assistant message — it finds @@ -902,6 +918,7 @@ export function PolicyContentManager({ errorMessage={chatErrorMessage} sendMessage={sendMessage} stop={stopChat} + retry={handleRetryChat} close={() => setShowAiAssistant(false)} /> @@ -980,6 +997,7 @@ export function PolicyContentManager({ errorMessage={chatErrorMessage} sendMessage={sendMessage} stop={stopChat} + retry={handleRetryChat} close={() => setShowAiAssistant(false)} /> diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/ai/markdown-utils.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/ai/markdown-utils.ts index 6beb7eb160..e7ef04c9a9 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/ai/markdown-utils.ts +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/ai/markdown-utils.ts @@ -1,7 +1,10 @@ import type { JSONContent } from '@tiptap/react'; +import { parseInline, sanitizeMarkdown } from '../../lib/policy-markdown'; export function markdownToTipTapJSON(markdown: string): Array { - const lines = markdown.split('\n'); + // Strip control-char noise (e.g. stray "013" glyphs) before parsing so it + // never reaches a ProseMirror text node. + const lines = sanitizeMarkdown(markdown).split('\n'); const content: Array = []; let currentList: JSONContent | null = null; let listType: 'bulletList' | 'orderedList' | null = null; @@ -28,7 +31,7 @@ export function markdownToTipTapJSON(markdown: string): Array { content.push({ type: 'heading', attrs: { level: headingMatch[1].length }, - content: [{ type: 'text', text: headingMatch[2] }], + content: parseInline(headingMatch[2]), }); continue; } @@ -45,7 +48,7 @@ export function markdownToTipTapJSON(markdown: string): Array { content: [ { type: 'paragraph', - content: [{ type: 'text', text: bulletMatch[1] }], + content: parseInline(bulletMatch[1]), }, ], }); @@ -64,7 +67,7 @@ export function markdownToTipTapJSON(markdown: string): Array { content: [ { type: 'paragraph', - content: [{ type: 'text', text: orderedMatch[1] }], + content: parseInline(orderedMatch[1]), }, ], }); @@ -78,7 +81,7 @@ export function markdownToTipTapJSON(markdown: string): Array { } content.push({ type: 'paragraph', - content: [{ type: 'text', text: trimmed }], + content: parseInline(trimmed), }); } diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/ai/policy-ai-assistant.tsx b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/ai/policy-ai-assistant.tsx index b53b6b74f1..f2b9f57ed7 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/ai/policy-ai-assistant.tsx +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/ai/policy-ai-assistant.tsx @@ -18,8 +18,9 @@ import { PromptInputSubmit, } from '@/components/ai-elements/prompt-input'; import { Button } from '@trycompai/design-system'; -import { Close, MagicWand } from '@trycompai/design-system/icons'; +import { Close, MagicWand, Renew } from '@trycompai/design-system/icons'; import type { ChatStatus } from 'ai'; +import { getProposalCardState } from '../../lib/proposal-card-state'; import type { PolicyChatUIMessage } from '../../types'; interface PolicyAiAssistantProps { @@ -28,6 +29,8 @@ interface PolicyAiAssistantProps { errorMessage?: string | null; sendMessage: (payload: { text: string }) => void; stop?: () => void; + /** Re-run the last request (used to recover from interrupted/incomplete runs). */ + retry?: () => void; close?: () => void; } @@ -37,6 +40,7 @@ export function PolicyAiAssistant({ errorMessage, sendMessage, stop, + retry, close, }: PolicyAiAssistantProps) { const isBusy = status === 'streaming' || status === 'submitted'; @@ -99,11 +103,21 @@ export function PolicyAiAssistant({ } if (part.type === 'tool-proposePolicy') { + // The proposed markdown lives in the tool INPUT. A + // truncated run can complete with empty content while + // the prose claims success (CS-256) — detect that here. + const proposed = + typeof part.input?.content === 'string' + ? part.input.content + : ''; + const hasContent = proposed.trim().length > 0; return ( ); } @@ -175,25 +189,28 @@ export function PolicyAiAssistant({ function PolicyToolCard({ state, stopped, + hasContent, + onRetry, }: { state: string; stopped: boolean; + hasContent: boolean; + onRetry?: () => void; }) { - const isCompleted = state === 'output-available'; - const isError = state === 'output-error'; - const isWorking = !isCompleted && !isError; + const cardState = getProposalCardState(state, stopped, hasContent); // Interrupted — streaming stopped while tool was in progress - if (isWorking && stopped) { + if (cardState === 'interrupted') { return ( -
- Interrupted -
+ ); } // Working state: same compact style as data tool cards - if (isWorking) { + if (cardState === 'working') { return (
- Something went wrong while generating updates. Please try again. -

+ + ); + } + + // Completed, but no content actually materialized — the run was truncated + // (timeout / output-token cap). Don't claim success on a blank policy (CS-256). + if (cardState === 'incomplete') { + return ( + ); } @@ -232,6 +261,30 @@ function PolicyToolCard({ ); } +/** + * Compact failure row for the proposePolicy tool — interrupted, errored, or + * truncated-with-no-content. Offers a Retry affordance so a dead run isn't a + * permanent dead-end (the previous UI showed a static "Interrupted" forever). + */ +function ToolFailureRow({ + message, + onRetry, +}: { + message: string; + onRetry?: () => void; +}) { + return ( +
+ {message} + {onRetry && ( + + )} +
+ ); +} + const TOOL_LABELS: Record = { 'tool-listVendors': { loading: 'Fetching vendors', done: 'Fetched vendors' }, 'tool-getVendor': { loading: 'Looking up vendor details', done: 'Looked up vendor details' }, diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/hooks/use-suggestions.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/hooks/use-suggestions.ts index 1e6f22effd..de2cddb75d 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/hooks/use-suggestions.ts +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/hooks/use-suggestions.ts @@ -1,7 +1,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import type { Editor } from '@tiptap/react'; import { suggestionsPluginKey } from '@trycompai/ui/editor'; -import { markdownToTipTapJSON } from '../components/ai/markdown-utils'; +import { buildReplacementNodes, extendDeleteRangesToSections } from '../lib/apply-suggestion'; import { buildPositionMap } from '../lib/build-position-map'; import { computeSuggestionRanges } from '../lib/compute-suggestion-ranges'; import type { SuggestionRange } from '../lib/suggestion-types'; @@ -167,44 +167,9 @@ export function useSuggestions({ } const positionMap = buildPositionMap(editor.state.doc); const initial = computeSuggestionRanges(positionMap, proposedMarkdown); - // For delete ranges that start at a heading, extend to the next heading - // of the same or higher level. This ensures full section deletions - // include all content between headings, even if the AI left some. - const doc = editor.state.doc; - const extended = initial.map((range) => { - if (range.type !== 'delete') return range; - - // Find the first block node in the range to check if it's a heading. - // range.from may point inside a node, so resolve to find the parent. - let headingLevel: number | null = null; - doc.nodesBetween(range.from, Math.min(range.from + 5, range.to), (node) => { - if (node.type.name === 'heading' && headingLevel === null) { - headingLevel = (node.attrs as { level?: number }).level ?? 1; - } - }); - if (headingLevel === null) return range; - - // Walk forward from the end of the range to find the next heading - // of the same or higher level. - let nextHeadingPos: number | null = null; - doc.nodesBetween(range.to, doc.content.size, (node, pos) => { - if (nextHeadingPos !== null) return false; - if (node.type.name === 'heading') { - const level = (node.attrs as { level?: number }).level ?? 1; - if (headingLevel !== null && level <= headingLevel) { - nextHeadingPos = pos; - return false; - } - } - return true; - }); - - const extendTo = nextHeadingPos ?? doc.content.size; - if (extendTo > range.to) { - return { ...range, to: extendTo }; - } - return range; - }); + // Extend heading-led delete ranges to cover the whole section so full + // section deletions include all content between headings. + const extended = extendDeleteRangesToSections(editor.state.doc, initial); setRanges(extended); setCurrentIndex(0); rangesHistoryRef.current = []; @@ -262,19 +227,13 @@ export function useSuggestions({ tr.delete(range.from, range.to); } else if (range.type === 'insert') { // Insert at the end of the anchor position, not replacing it - const jsonNodes = markdownToTipTapJSON(range.proposedText); - const pmNodes = jsonNodes.map((json) => - editor.state.schema.nodeFromJSON(json), - ); + const pmNodes = buildReplacementNodes(editor.state, range.proposedText, range.to); if (pmNodes.length > 0) { tr.insert(range.to, pmNodes); } } else { // Modify: replace old content with new - const jsonNodes = markdownToTipTapJSON(range.proposedText); - const pmNodes = jsonNodes.map((json) => - editor.state.schema.nodeFromJSON(json), - ); + const pmNodes = buildReplacementNodes(editor.state, range.proposedText, range.from); if (pmNodes.length > 0) { tr.replaceWith(range.from, range.to, pmNodes); } @@ -358,18 +317,12 @@ export function useSuggestions({ if (range.type === 'delete') { tr.delete(range.from, range.to); } else if (range.type === 'insert') { - const jsonNodes = markdownToTipTapJSON(range.proposedText); - const pmNodes = jsonNodes.map((json) => - editor.state.schema.nodeFromJSON(json), - ); + const pmNodes = buildReplacementNodes(editor.state, range.proposedText, range.to); if (pmNodes.length > 0) { tr.insert(range.to, pmNodes); } } else { - const jsonNodes = markdownToTipTapJSON(range.proposedText); - const pmNodes = jsonNodes.map((json) => - editor.state.schema.nodeFromJSON(json), - ); + const pmNodes = buildReplacementNodes(editor.state, range.proposedText, range.from); if (pmNodes.length > 0) { tr.replaceWith(range.from, range.to, pmNodes); } @@ -415,6 +368,11 @@ export function useSuggestions({ ), ); + // Abort a stuck request so the range can't stay 'loading' forever. The + // edit-section route caps at 30s (maxDuration), so 35s is a safe ceiling. + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 35_000); + try { const policyId = window.location.pathname.match(/policies\/([^/]+)/)?.[1]; const res = await fetch(`/api/policies/${policyId}/edit-section`, { @@ -425,6 +383,7 @@ export function useSuggestions({ sectionText: range.proposedText, feedback, }), + signal: controller.signal, }); if (!res.ok) throw new Error('Failed to edit section'); @@ -444,6 +403,8 @@ export function useSuggestions({ r.id === id ? { ...r, decision: 'pending' as const } : r, ), ); + } finally { + clearTimeout(timeout); } }, [], diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/build-position-map.test.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/build-position-map.test.ts index 1c9e6075aa..0399952a8f 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/build-position-map.test.ts +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/build-position-map.test.ts @@ -68,10 +68,11 @@ describe('buildPositionMap', () => { const result = buildPositionMap(doc(h(2, 'Purpose'))); expect(result.markdown).toBe('## Purpose'); - // The heading content line should be mapped + // The heading line maps to the node's outer boundary (before the node), + // which is position 0 for the first block. const pos = result.lineToPos.get(1); expect(pos).toBeDefined(); - expect(pos!.from).toBe(1); + expect(pos!.from).toBe(0); }); it('uses the correct number of # for each level', () => { @@ -127,9 +128,11 @@ describe('buildPositionMap', () => { const result = buildPositionMap(doc(p('Hello world'))); expect(result.markdown).toBe('Hello world'); + // Maps to the paragraph's outer boundary (before the node) = 0 for the + // first block. const pos = result.lineToPos.get(1); expect(pos).toBeDefined(); - expect(pos!.from).toBe(1); + expect(pos!.from).toBe(0); }); it('separates consecutive paragraphs with a blank line', () => { diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/compute-suggestion-ranges.test.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/compute-suggestion-ranges.test.ts index 520c24d5a5..1529cce8e1 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/compute-suggestion-ranges.test.ts +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/compute-suggestion-ranges.test.ts @@ -262,8 +262,8 @@ describe('computeSuggestionRanges', () => { expect(allOriginalText).toContain('Section B'); }); - it('does not merge distant ranges', () => { - // Manually create a position map with widely separated positions + it('never merges separate modifies across a gap (no content loss)', () => { + // Four separate single-line changes, each separated by unchanged context. const lineToPos = new Map(); lineToPos.set(1, { from: 1, to: 10 }); lineToPos.set(3, { from: 12, to: 25 }); @@ -279,19 +279,14 @@ describe('computeSuggestionRanges', () => { const ranges = computeSuggestionRanges(posMap, proposed); - // The first two changes (positions 1-25) are close and may merge. - // The last two changes (positions 200-230) are close and may merge. - // But the two groups (25 vs 200) are >20 apart so should NOT merge. - if (ranges.length > 1) { - // Verify there's a gap between groups - const sorted = [...ranges].sort((a, b) => a.from - b.from); - for (let i = 1; i < sorted.length; i++) { - const gap = sorted[i]!.from - sorted[i - 1]!.to; - // If they didn't merge, the gap must be > 20 - if (gap > 0) { - expect(gap).toBeGreaterThan(20); - } - } + // Modifies are no longer merged across a gap (even a small one), so each + // change stays its own suggestion and no unchanged content between them + // can be swallowed/deleted on apply. + expect(ranges.length).toBe(4); + const sorted = [...ranges].sort((a, b) => a.from - b.from); + for (let i = 1; i < sorted.length; i++) { + // Non-overlapping, ascending — distinct ranges. + expect(sorted[i]!.from).toBeGreaterThanOrEqual(sorted[i - 1]!.to); } }); @@ -378,4 +373,57 @@ describe('computeSuggestionRanges', () => { expect(modRange!.proposedText).toContain('must'); }); }); + + describe('inline formatting-only changes still surface (CS-265 fmt)', () => { + it('produces a suggestion when only inline bold is added', () => { + const current = + 'Provide authentication that resists brute-force and credential-stuffing attacks.'; + const proposed = + 'Provide authentication that resists **brute-force** and **credential-stuffing** attacks.'; + const posMap = makePositionMap(current); + + const ranges = computeSuggestionRanges(posMap, proposed); + + // A user explicitly asking to bold text must get an accept-able suggestion + // — the formatting must NOT be silently swallowed by normalization. + expect(ranges.length).toBeGreaterThan(0); + expect(ranges[0]!.proposedText).toContain('**brute-force**'); + }); + + it('inserts BEFORE the nearest mapped line when it is after the anchor', () => { + // Only line 3 is mapped; an insert anchored at the unmapped top (line 1) + // must land before that block (.from = 50), not after it (cubic P1). + const lineToPos = new Map(); + lineToPos.set(3, { from: 50, to: 60 }); + const posMap: PositionMap = { lineToPos, markdown: 'aaa\nbbb\nTarget' }; + + const ranges = computeSuggestionRanges(posMap, 'NEW\naaa\nbbb\nTarget'); + const insert = ranges.find((r) => r.type === 'insert'); + expect(insert).toBeDefined(); + expect(insert!.from).toBe(50); + }); + + it('inserts AFTER the nearest mapped line when it is before the anchor', () => { + const lineToPos = new Map(); + lineToPos.set(1, { from: 5, to: 10 }); + const posMap: PositionMap = { lineToPos, markdown: 'Target\naaa\nbbb' }; + + const ranges = computeSuggestionRanges(posMap, 'Target\naaa\nbbb\nNEW'); + const insert = ranges.find((r) => r.type === 'insert'); + expect(insert).toBeDefined(); + expect(insert!.from).toBe(10); + }); + + it('produces a suggestion when only a link is added', () => { + const current = 'Store passwords in an approved password manager.'; + const proposed = + 'Store passwords in an approved [password manager](https://1password.com).'; + const posMap = makePositionMap(current); + + const ranges = computeSuggestionRanges(posMap, proposed); + + expect(ranges.length).toBeGreaterThan(0); + expect(ranges[0]!.proposedText).toContain('](https://1password.com)'); + }); + }); }); diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/list-edit-apply.test.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/list-edit-apply.test.ts new file mode 100644 index 0000000000..18b26641e4 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/list-edit-apply.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it } from 'vitest'; +import { EditorState } from '@tiptap/pm/state'; +import type { Node as ProseMirrorNode } from '@tiptap/pm/model'; +import { buildPositionMap } from '../build-position-map'; +import { buildReplacementNodes } from '../apply-suggestion'; +import { schema } from '../test-helpers/editor-schema'; + +function listItem(text: string) { + return schema.node('listItem', null, [ + schema.node('paragraph', null, [schema.text(text)]), + ]); +} + +function bulletDoc(items: string[]) { + return schema.node('doc', null, [ + schema.node('bulletList', null, items.map(listItem)), + ]); +} + +// Real ProseMirror [before, after) boundaries for every listItem in the doc. +function realListItemBoundaries(doc: ProseMirrorNode): Array<{ from: number; to: number }> { + const out: Array<{ from: number; to: number }> = []; + doc.descendants((node, pos) => { + if (node.type.name === 'listItem') { + out.push({ from: pos, to: pos + node.nodeSize }); + } + }); + return out; +} + +// The list-item ranges the position map produced (lines beginning with "- "). +function mappedListItemRanges(doc: ProseMirrorNode): Array<{ from: number; to: number }> { + const map = buildPositionMap(doc); + const lines = map.markdown.split('\n'); + const out: Array<{ from: number; to: number }> = []; + lines.forEach((line, idx) => { + if (line.startsWith('- ')) { + const pos = map.lineToPos.get(idx + 1); + if (pos) out.push(pos); + } + }); + return out.sort((a, b) => a.from - b.from); +} + +describe('buildPositionMap list-item boundaries (CS-265)', () => { + it('maps list items to their exact ProseMirror node boundaries', () => { + const doc = bulletDoc(['First item', 'Second item', 'Third item']); + expect(mappedListItemRanges(doc)).toEqual(realListItemBoundaries(doc)); + }); + + it('is correct when a paragraph precedes the list', () => { + const doc = schema.node('doc', null, [ + schema.node('paragraph', null, [schema.text('Intro paragraph.')]), + schema.node('bulletList', null, [listItem('Alpha'), listItem('Beta')]), + ]); + expect(mappedListItemRanges(doc)).toEqual(realListItemBoundaries(doc)); + }); +}); + +describe('single-bullet edit does not bleed into neighbors (CS-265)', () => { + it('replaces only the targeted item, leaving siblings intact', () => { + const doc = bulletDoc(['Alpha', 'Bravo', 'Charlie']); + const ranges = mappedListItemRanges(doc); + const middle = ranges[1]!; // "Bravo" + + // Replicate applyRangeToDoc's "modify" path exactly. + const state = EditorState.create({ doc }); + const pmNodes = buildReplacementNodes(state, '- Bravo replaced', middle.from); + const tr = state.tr.replaceWith(middle.from, middle.to, pmNodes); + const newDoc = tr.doc; + + // The document must remain a single bulletList with exactly three sibling + // list items — no nesting, no fragments bleeding between bullets. + expect(newDoc.childCount).toBe(1); + const list = newDoc.child(0); + expect(list.type.name).toBe('bulletList'); + expect(list.childCount).toBe(3); + + const texts: string[] = []; + list.forEach((item) => { + expect(item.type.name).toBe('listItem'); + texts.push(item.textContent); + }); + expect(texts).toEqual(['Alpha', 'Bravo replaced', 'Charlie']); + }); + + it('replaces a multi-bullet range with the right number of clean items', () => { + const doc = bulletDoc(['Alpha', 'Bravo', 'Charlie']); + const ranges = mappedListItemRanges(doc); + const from = ranges[0]!.from; // start of Alpha + const to = ranges[1]!.to; // end of Bravo (covers two items) + + const state = EditorState.create({ doc }); + const pmNodes = buildReplacementNodes(state, '- One\n- Two', from); + const newDoc = state.tr.replaceWith(from, to, pmNodes).doc; + + expect(newDoc.childCount).toBe(1); + const list = newDoc.child(0); + expect(list.type.name).toBe('bulletList'); + const texts: string[] = []; + list.forEach((item) => texts.push(item.textContent)); + expect(texts).toEqual(['One', 'Two', 'Charlie']); + }); + + it('inserts a new bullet without splitting or nesting the list', () => { + const doc = bulletDoc(['Alpha', 'Bravo']); + const ranges = mappedListItemRanges(doc); + const afterFirst = ranges[0]!.to; // boundary between Alpha and Bravo + + const state = EditorState.create({ doc }); + const pmNodes = buildReplacementNodes(state, '- Inserted', afterFirst); + const newDoc = state.tr.insert(afterFirst, pmNodes).doc; + + expect(newDoc.childCount).toBe(1); + const list = newDoc.child(0); + expect(list.type.name).toBe('bulletList'); + const texts: string[] = []; + list.forEach((item) => { + expect(item.type.name).toBe('listItem'); + texts.push(item.textContent); + }); + expect(texts).toEqual(['Alpha', 'Inserted', 'Bravo']); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/pipeline-e2e.test.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/pipeline-e2e.test.ts new file mode 100644 index 0000000000..1c92c54409 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/pipeline-e2e.test.ts @@ -0,0 +1,539 @@ +import { describe, expect, it } from 'vitest'; +import { EditorState } from '@tiptap/pm/state'; +import type { Node as ProseMirrorNode } from '@tiptap/pm/model'; +import { buildPositionMap } from '../build-position-map'; +import { computeSuggestionRanges } from '../compute-suggestion-ranges'; +import { buildReplacementNodes, extendDeleteRangesToSections } from '../apply-suggestion'; +import { sanitizeMarkdown } from '../policy-markdown'; +import { schema } from '../test-helpers/editor-schema'; + +/** + * Faithfully reproduces the real client flow for "accept all suggestions": + * proposed (sanitized) -> buildPositionMap -> computeSuggestionRanges + * -> buildReplacementNodes -> apply (reverse doc order, one transaction). + * + * This is the deterministic part of the AI edit pipeline (everything except the + * model call + React state), so it catches end-to-end logic regressions without + * a deploy. + */ +function acceptAll(startDoc: ProseMirrorNode, rawProposed: string) { + const proposed = sanitizeMarkdown(rawProposed); + const positionMap = buildPositionMap(startDoc); + const ranges = extendDeleteRangesToSections( + startDoc, + computeSuggestionRanges(positionMap, proposed), + ); + + const state = EditorState.create({ doc: startDoc }); + const tr = state.tr; + // Reverse doc order so earlier edits don't shift later positions (matches the + // real acceptAll which resolves positions against the original doc). + for (const range of [...ranges].sort((a, b) => b.from - a.from)) { + if (range.type === 'delete') { + tr.delete(range.from, range.to); + } else if (range.type === 'insert') { + const nodes = buildReplacementNodes(state, range.proposedText, range.to); + if (nodes.length) tr.insert(range.to, nodes); + } else { + const nodes = buildReplacementNodes(state, range.proposedText, range.from); + if (nodes.length) tr.replaceWith(range.from, range.to, nodes); + } + } + return { ranges, doc: tr.doc }; +} + +// ── doc builders ── +function p(text: string) { + return schema.node('paragraph', null, text ? [schema.text(text)] : []); +} +function h(level: number, text: string) { + return schema.node('heading', { level }, [schema.text(text)]); +} +function li(text: string) { + return schema.node('listItem', null, [p(text)]); +} +function ul(...items: string[]) { + return schema.node('bulletList', null, items.map(li)); +} +function ol(...items: string[]) { + return schema.node('orderedList', null, items.map(li)); +} +function doc(...blocks: ProseMirrorNode[]) { + return schema.node('doc', null, blocks); +} + +// ── inspectors ── +function listTexts(node: ProseMirrorNode, listType = 'bulletList'): string[] { + const out: string[] = []; + node.descendants((n) => { + if (n.type.name === listType) { + n.forEach((item) => out.push(item.textContent)); + return false; + } + return true; + }); + return out; +} + +function marksOnPhrase(node: ProseMirrorNode, phrase: string): string[] { + let found: string[] | null = null; + node.descendants((n) => { + if (found) return false; + if (n.isText && n.text === phrase) { + found = n.marks.map((m) => m.type.name).sort(); + } + return true; + }); + return found ?? []; +} + +function topLevelTypes(d: ProseMirrorNode): string[] { + const out: string[] = []; + d.forEach((n) => out.push(n.type.name)); + return out; +} + +describe('E2E pipeline: single-bullet edit (CS-265)', () => { + it('changes only the targeted bullet, keeps one list', () => { + const start = doc( + h(2, 'Password Requirements'), + ul( + 'Minimum 12 characters or a passphrase; prohibit commonly breached strings.', + 'No forced periodic rotation unless compromise is suspected.', + 'Unique password per system; store passwords in an approved password manager.', + 'Do not reuse passwords across work and personal accounts.', + ), + p('Enforce length and reuse rules in IdP/directory policies and document settings.'), + ); + const proposed = [ + '## Password Requirements', + '', + '- Minimum 12 characters or a passphrase; prohibit commonly breached strings.', + '- No forced periodic rotation unless compromise is suspected.', + '- Use a unique password for every system and store it in an approved password manager.', + '- Do not reuse passwords across work and personal accounts.', + '', + 'Enforce length and reuse rules in IdP/directory policies and document settings.', + ].join('\n'); + + const { doc: result } = acceptAll(start, proposed); + expect(listTexts(result)).toEqual([ + 'Minimum 12 characters or a passphrase; prohibit commonly breached strings.', + 'No forced periodic rotation unless compromise is suspected.', + 'Use a unique password for every system and store it in an approved password manager.', + 'Do not reuse passwords across work and personal accounts.', + ]); + // Single list, trailing paragraph preserved + expect(topLevelTypes(result)).toEqual(['heading', 'bulletList', 'paragraph']); + expect(result.textContent).toContain('Enforce length and reuse rules'); + }); +}); + +describe('E2E pipeline: inline formatting (CS-265 fmt / SALE-65)', () => { + it('applies bold as real marks, not literal asterisks', () => { + const start = doc( + h(2, 'Purpose'), + p('Provide authentication that resists brute-force and credential-stuffing attacks.'), + ); + const proposed = [ + '## Purpose', + '', + 'Provide authentication that resists **brute-force** and **credential-stuffing** attacks.', + ].join('\n'); + + const { ranges, doc: result } = acceptAll(start, proposed); + expect(ranges.length).toBeGreaterThan(0); // a suggestion must appear + expect(marksOnPhrase(result, 'brute-force')).toEqual(['bold']); + expect(marksOnPhrase(result, 'credential-stuffing')).toEqual(['bold']); + expect(result.textContent).not.toContain('**'); // no literal markers + }); + + it('applies a link as a real mark', () => { + const start = doc(p('Store passwords in an approved password manager.')); + const proposed = 'Store passwords in an approved [password manager](https://1password.com).'; + const { doc: result } = acceptAll(start, proposed); + expect(marksOnPhrase(result, 'password manager')).toEqual(['link']); + expect(result.textContent).not.toContain(']('); + }); + + it('preserves existing bold when an unrelated paragraph changes', () => { + const start = doc( + schema.node('paragraph', null, [ + schema.text('Resists '), + schema.text('brute-force', [schema.marks.bold!.create()]), + schema.text(' attacks.'), + ]), + p('Scope covers all accounts.'), + ); + const proposed = [ + 'Resists **brute-force** attacks.', + '', + 'Scope covers all human and service accounts.', + ].join('\n'); + const { doc: result } = acceptAll(start, proposed); + // bold survives, only the scope sentence changed + expect(marksOnPhrase(result, 'brute-force')).toEqual(['bold']); + expect(result.textContent).toContain('all human and service accounts'); + }); +}); + +describe('E2E pipeline: list inserts', () => { + it('adds a new bullet without splitting the list', () => { + const start = doc(ul('Alpha', 'Bravo')); + const proposed = ['- Alpha', '- Inserted', '- Bravo'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(topLevelTypes(result)).toEqual(['bulletList']); + expect(listTexts(result)).toEqual(['Alpha', 'Inserted', 'Bravo']); + }); +}); + +describe('E2E pipeline: control-char sanitization (013)', () => { + it('strips control chars from applied content', () => { + const start = doc(p('Old text.')); + const vt = String.fromCharCode(0x0b); + const proposed = `New ${vt}clean${vt} text.`; + const { doc: result } = acceptAll(start, proposed); + expect(result.textContent).toBe('New clean text.'); + }); +}); + +describe('E2E pipeline: ordered list item edit', () => { + it('keeps the list ORDERED after editing an item', () => { + const start = doc(ol('First step', 'Second step', 'Third step')); + // buildPositionMap emits "- " for ordered items too, so the AI echoes "- ". + const proposed = ['- First step', '- Second step revised', '- Third step'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(topLevelTypes(result)).toEqual(['orderedList']); + expect(listTexts(result, 'orderedList')).toEqual([ + 'First step', + 'Second step revised', + 'Third step', + ]); + }); + + it('inserts a new step into an ordered list, kept ordered', () => { + const start = doc(ol('First step', 'Second step')); + const proposed = ['- First step', '- Inserted step', '- Second step'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(topLevelTypes(result)).toEqual(['orderedList']); + expect(listTexts(result, 'orderedList')).toEqual([ + 'First step', + 'Inserted step', + 'Second step', + ]); + }); +}); + +describe('E2E pipeline: delete a single bullet', () => { + it('removes only the targeted bullet', () => { + const start = doc(ul('Keep one', 'Remove me', 'Keep two')); + const proposed = ['- Keep one', '- Keep two'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(topLevelTypes(result)).toEqual(['bulletList']); + expect(listTexts(result)).toEqual(['Keep one', 'Keep two']); + }); +}); + +describe('E2E pipeline: plain paragraph modify', () => { + it('replaces paragraph text in place', () => { + const start = doc(h(2, 'Purpose'), p('Old purpose statement.')); + const proposed = ['## Purpose', '', 'New, clearer purpose statement.'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(topLevelTypes(result)).toEqual(['heading', 'paragraph']); + expect(result.textContent).toContain('New, clearer purpose statement.'); + expect(result.textContent).not.toContain('Old purpose'); + }); +}); + +describe('E2E pipeline: multi-change in one proposal', () => { + it('applies a bold edit AND an unrelated paragraph edit together', () => { + const start = doc( + h(2, 'Purpose'), + p('Provide authentication that resists brute-force attacks.'), + h(2, 'Scope'), + p('Applies to all accounts.'), + ); + const proposed = [ + '## Purpose', + '', + 'Provide authentication that resists **brute-force** attacks.', + '', + '## Scope', + '', + 'Applies to all human and service accounts.', + ].join('\n'); + const { ranges, doc: result } = acceptAll(start, proposed); + expect(ranges.length).toBeGreaterThanOrEqual(2); + expect(marksOnPhrase(result, 'brute-force')).toEqual(['bold']); + expect(result.textContent).toContain('all human and service accounts'); + expect(topLevelTypes(result)).toEqual(['heading', 'paragraph', 'heading', 'paragraph']); + }); +}); + +describe('E2E pipeline: realistic policy with two lists', () => { + it('edits one bullet in the MFA list, leaving Password Requirements intact', () => { + const start = doc( + h(2, 'Password Requirements'), + ul('Minimum 12 characters.', 'No forced rotation.', 'Unique per system.'), + h(2, 'Multi-Factor Authentication (MFA)'), + ul('Enforce MFA for admins.', 'Prefer authenticator apps; SMS only as fallback.', 'Apply step-up MFA for sensitive actions.'), + ); + const proposed = [ + '## Password Requirements', + '', + '- Minimum 12 characters.', + '- No forced rotation.', + '- Unique per system.', + '', + '## Multi-Factor Authentication (MFA)', + '', + '- Enforce MFA for admins.', + '- Prefer hardware tokens or authenticator apps; SMS only as a last resort.', + '- Apply step-up MFA for sensitive actions.', + ].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(topLevelTypes(result)).toEqual(['heading', 'bulletList', 'heading', 'bulletList']); + + const lists: string[][] = []; + result.forEach((n) => { + if (n.type.name === 'bulletList') { + const items: string[] = []; + n.forEach((it) => items.push(it.textContent)); + lists.push(items); + } + }); + expect(lists[0]).toEqual(['Minimum 12 characters.', 'No forced rotation.', 'Unique per system.']); + expect(lists[1]).toEqual([ + 'Enforce MFA for admins.', + 'Prefer hardware tokens or authenticator apps; SMS only as a last resort.', + 'Apply step-up MFA for sensitive actions.', + ]); + }); +}); + +describe('E2E pipeline: append a new section', () => { + it('adds a new heading + paragraph at the end', () => { + const start = doc(h(2, 'Purpose'), p('Purpose text.')); + const proposed = [ + '## Purpose', + '', + 'Purpose text.', + '', + '## Enforcement', + '', + 'Violations may result in disciplinary action.', + ].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(result.textContent).toContain('Purpose text.'); + expect(result.textContent).toContain('Enforcement'); + expect(result.textContent).toContain('Violations may result in disciplinary action.'); + }); +}); + +describe('E2E pipeline: bold inside a list item', () => { + it('adds bold to one bullet, keeps the list intact', () => { + const start = doc(ul('Use MFA everywhere.', 'Rotate keys quarterly.')); + const proposed = ['- Use **MFA** everywhere.', '- Rotate keys quarterly.'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(topLevelTypes(result)).toEqual(['bulletList']); + expect(listTexts(result)).toEqual(['Use MFA everywhere.', 'Rotate keys quarterly.']); + expect(marksOnPhrase(result, 'MFA')).toEqual(['bold']); + expect(result.textContent).not.toContain('**'); + }); +}); + +describe('E2E pipeline: heading text change', () => { + it('changes heading text without duplicating the heading', () => { + const start = doc(h(2, 'Password Requirements'), p('Body.')); + const proposed = ['## Password & Credential Requirements', '', 'Body.'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(topLevelTypes(result)).toEqual(['heading', 'paragraph']); + expect(result.child(0).textContent).toBe('Password & Credential Requirements'); + }); +}); + +describe('E2E pipeline: combined bold+italic', () => { + it('applies ***text*** as both marks', () => { + const start = doc(p('This is important text.')); + const proposed = 'This is ***important*** text.'; + const { doc: result } = acceptAll(start, proposed); + expect(marksOnPhrase(result, 'important')).toEqual(['bold', 'italic']); + }); +}); + +describe('E2E pipeline: section delete', () => { + it('removes a whole section (heading + body + list) and keeps the rest', () => { + const start = doc( + h(2, 'Purpose'), + p('Purpose body.'), + h(2, 'Deprecated Section'), + p('Outdated guidance.'), + ul('Old item one.', 'Old item two.'), + h(2, 'Enforcement'), + p('Enforcement body.'), + ); + const proposed = [ + '## Purpose', + '', + 'Purpose body.', + '', + '## Enforcement', + '', + 'Enforcement body.', + ].join('\n'); + const { doc: result } = acceptAll(start, proposed); + const text = result.textContent; + expect(text).toContain('Purpose body.'); + expect(text).toContain('Enforcement body.'); + expect(text).not.toContain('Deprecated Section'); + expect(text).not.toContain('Outdated guidance.'); + expect(text).not.toContain('Old item one.'); + // Surviving headings intact + const headings: string[] = []; + result.forEach((n) => { + if (n.type.name === 'heading') headings.push(n.textContent); + }); + expect(headings).toEqual(['Purpose', 'Enforcement']); + }); +}); + +describe('E2E pipeline: no-op proposal', () => { + it('produces zero suggestions when content is unchanged', () => { + const start = doc(h(2, 'Purpose'), p('Stays the same.'), ul('One.', 'Two.')); + const proposed = ['## Purpose', '', 'Stays the same.', '', '- One.', '- Two.'].join('\n'); + const { ranges, doc: result } = acceptAll(start, proposed); + expect(ranges.length).toBe(0); + expect(result.textContent).toBe(start.textContent); + }); +}); + +describe('E2E pipeline: tolerant of stray whitespace from the model', () => { + it('normalizes extra spaces in a modified paragraph', () => { + const start = doc(p('Keep this tidy.')); + const proposed = 'Keep this much tidier.'; + const { doc: result } = acceptAll(start, proposed); + expect(result.textContent).toBe('Keep this much tidier.'); + }); +}); + +describe('E2E pipeline: heading level change', () => { + // KNOWN LIMITATION: normalizeContent strips block markers (#, -, >), so a + // change to ONLY a heading's level (or a block-type change like heading<->para + // or bullet<->para) with identical text is treated as "no change" and yields + // no suggestion. Text edits to headings work; pure structural changes do not. + // Loosening this is a deliberate behavioral choice (more sensitivity to the + // model reformatting block markers) — left out of scope pending a decision. + it.skip('changes ## to ### (whole-node replace) — known limitation', () => { + const start = doc(h(2, 'Sub Section'), p('Body.')); + const proposed = ['### Sub Section', '', 'Body.'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(result.child(0).attrs.level).toBe(3); + }); + + it('applies a heading TEXT change (whole-node replace)', () => { + const start = doc(h(2, 'Old Heading'), p('Body.')); + const proposed = ['## New Heading', '', 'Body.'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(result.child(0).type.name).toBe('heading'); + expect(result.child(0).attrs.level).toBe(2); + expect(result.child(0).textContent).toBe('New Heading'); + }); +}); + +describe('E2E pipeline: delete first / last section', () => { + it('deletes the first section cleanly', () => { + const start = doc(h(2, 'First'), p('First body.'), h(2, 'Second'), p('Second body.')); + const proposed = ['## Second', '', 'Second body.'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + const headings: string[] = []; + result.forEach((n) => { + if (n.type.name === 'heading') headings.push(n.textContent); + }); + expect(headings).toEqual(['Second']); + expect(result.textContent).not.toContain('First'); + }); + + it('deletes the last section cleanly', () => { + const start = doc(h(2, 'First'), p('First body.'), h(2, 'Second'), p('Second body.')); + const proposed = ['## First', '', 'First body.'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + const headings: string[] = []; + result.forEach((n) => { + if (n.type.name === 'heading') headings.push(n.textContent); + }); + expect(headings).toEqual(['First']); + expect(result.textContent).not.toContain('Second'); + }); +}); + +describe('E2E pipeline: two separate lists', () => { + it('edits a bullet in the second list, leaving the first untouched', () => { + const start = doc( + ul('A1', 'A2'), + p('Divider paragraph.'), + ul('B1', 'B2'), + ); + const proposed = [ + '- A1', + '- A2', + '', + 'Divider paragraph.', + '', + '- B1', + '- B2 revised', + ].join('\n'); + const { doc: result } = acceptAll(start, proposed); + const lists: string[][] = []; + result.forEach((n) => { + if (n.type.name === 'bulletList') { + const items: string[] = []; + n.forEach((it) => items.push(it.textContent)); + lists.push(items); + } + }); + expect(lists[0]).toEqual(['A1', 'A2']); + expect(lists[1]).toEqual(['B1', 'B2 revised']); + expect(result.textContent).toContain('Divider paragraph.'); + }); +}); + +describe('E2E pipeline: blockquote edit', () => { + it('edits a blockquote in place', () => { + const start = doc( + schema.node('blockquote', null, [p('Quoted guidance.')]), + p('After.'), + ); + const proposed = ['> Updated quoted guidance.', '', 'After.'].join('\n'); + const { doc: result } = acceptAll(start, proposed); + expect(result.textContent).toContain('Updated quoted guidance.'); + expect(result.textContent).toContain('After.'); + }); +}); + +describe('E2E pipeline: generate full policy from a draft stub', () => { + it('replaces a draft stub with a full multi-section policy', () => { + const start = doc(p('This policy is a draft.')); + const proposed = [ + '## Purpose', + '', + 'Provide strong authentication.', + '', + '## Password Requirements', + '', + '- Minimum 12 characters.', + '- No forced rotation.', + '', + '## Enforcement', + '', + 'Violations may result in disciplinary action.', + ].join('\n'); + const { doc: result } = acceptAll(start, proposed); + const text = result.textContent; + expect(text).toContain('Provide strong authentication.'); + expect(text).toContain('Minimum 12 characters.'); + expect(text).toContain('Violations may result in disciplinary action.'); + expect(text).not.toContain('This policy is a draft.'); + // A real list materialized, not flattened paragraphs + expect(listTexts(result)).toEqual(['Minimum 12 characters.', 'No forced rotation.']); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/policy-markdown.test.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/policy-markdown.test.ts new file mode 100644 index 0000000000..b237e20cf5 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/policy-markdown.test.ts @@ -0,0 +1,143 @@ +import { describe, expect, it } from 'vitest'; +import { parseInline, sanitizeMarkdown, serializeInline } from '../policy-markdown'; +import { schema } from '../test-helpers/editor-schema'; + +// Build a paragraph PM node from inline JSON so we can round-trip through +// serializeInline against the real ProseMirror schema. +function paragraphFrom(inline: ReturnType) { + return schema.nodeFromJSON({ type: 'paragraph', content: inline }); +} + +function roundTrip(markdown: string): string { + return serializeInline(paragraphFrom(parseInline(markdown))); +} + +describe('sanitizeMarkdown', () => { + it('strips C0/C1 control chars including the "013" vertical tab', () => { + const vt = String.fromCharCode(0x0b); // vertical tab = octal 013 + const dc3 = String.fromCharCode(0x13); + const del = String.fromCharCode(0x7f); + expect(sanitizeMarkdown(`Access${vt}Control${dc3}Policy${del}`)).toBe( + 'AccessControlPolicy', + ); + }); + + it('preserves tabs and newlines', () => { + expect(sanitizeMarkdown('line1\nline2\tindented')).toBe('line1\nline2\tindented'); + }); + + it('normalizes CR and CRLF to LF', () => { + expect(sanitizeMarkdown('a\r\nb\rc')).toBe('a\nb\nc'); + }); + + it('handles empty input', () => { + expect(sanitizeMarkdown('')).toBe(''); + }); +}); + +describe('parseInline', () => { + it('returns a plain text node for unformatted text', () => { + expect(parseInline('Just plain text.')).toEqual([ + { type: 'text', text: 'Just plain text.' }, + ]); + }); + + it('parses bold, italic, code and links', () => { + expect(parseInline('**bold**')).toEqual([ + { type: 'text', text: 'bold', marks: [{ type: 'bold' }] }, + ]); + expect(parseInline('*italic*')).toEqual([ + { type: 'text', text: 'italic', marks: [{ type: 'italic' }] }, + ]); + expect(parseInline('`code`')).toEqual([ + { type: 'text', text: 'code', marks: [{ type: 'code' }] }, + ]); + expect(parseInline('[site](https://x.com)')).toEqual([ + { type: 'text', text: 'site', marks: [{ type: 'link', attrs: { href: 'https://x.com' } }] }, + ]); + }); + + it('parses a mark in the middle of a sentence', () => { + expect(parseInline('see the **Security** section')).toEqual([ + { type: 'text', text: 'see the ' }, + { type: 'text', text: 'Security', marks: [{ type: 'bold' }] }, + { type: 'text', text: ' section' }, + ]); + }); + + it('parses bold+italic ***text***', () => { + expect(parseInline('***strong***')).toEqual([ + { type: 'text', text: 'strong', marks: [{ type: 'bold' }, { type: 'italic' }] }, + ]); + }); + + it('does NOT treat snake_case underscores as italic', () => { + expect(parseInline('use the snake_case_name variable')).toEqual([ + { type: 'text', text: 'use the snake_case_name variable' }, + ]); + }); + + it('does NOT treat a lone asterisk as a mark', () => { + expect(parseInline('5 * 3 = 15')).toEqual([{ type: 'text', text: '5 * 3 = 15' }]); + }); + + it('collapses stray whitespace runs (SALE-65)', () => { + expect(parseInline('Retain records for')).toEqual([ + { type: 'text', text: 'Retain records for' }, + ]); + }); +}); + +describe('serializeInline', () => { + const bold = schema.marks.bold!.create(); + const italic = schema.marks.italic!.create(); + const code = schema.marks.code!.create(); + const link = (href: string) => schema.marks.link!.create({ href }); + + function para(...nodes: ReturnType[]) { + return schema.node('paragraph', null, nodes); + } + + it('emits markdown markers for marks', () => { + expect(serializeInline(para(schema.text('bold', [bold])))).toBe('**bold**'); + expect(serializeInline(para(schema.text('italic', [italic])))).toBe('*italic*'); + expect(serializeInline(para(schema.text('code', [code])))).toBe('`code`'); + expect(serializeInline(para(schema.text('site', [link('https://x.com')])))).toBe( + '[site](https://x.com)', + ); + }); + + it('emits ***text*** for bold+italic', () => { + expect(serializeInline(para(schema.text('s', [bold, italic])))).toBe('***s***'); + }); + + it('serializes a mid-sentence mark', () => { + expect( + serializeInline( + para( + schema.text('see the '), + schema.text('Security', [bold]), + schema.text(' section'), + ), + ), + ).toBe('see the **Security** section'); + }); +}); + +describe('round-trip parse <-> serialize', () => { + const cases = [ + 'Just plain text.', + 'A sentence with **bold** word.', + 'A sentence with *italic* word.', + 'Use `code` inline.', + 'See [the policy](https://example.com/p) here.', + 'A **bold** and *italic* mix.', + '***both at once***', + 'snake_case_name stays literal', + 'Plain ending after a [link](https://x.com).', + ]; + + it.each(cases)('round-trips: %s', (md) => { + expect(roundTrip(md)).toBe(md); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/proposal-card-state.test.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/proposal-card-state.test.ts new file mode 100644 index 0000000000..7b96679297 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/__tests__/proposal-card-state.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; +import { getProposalCardState } from '../proposal-card-state'; + +describe('getProposalCardState', () => { + it('shows success only when completed WITH content', () => { + expect(getProposalCardState('output-available', false, true)).toBe('done'); + }); + + it('flags a completed-but-empty run as incomplete, never success (CS-256)', () => { + expect(getProposalCardState('output-available', false, false)).toBe('incomplete'); + }); + + it('flags a stopped in-progress run as interrupted', () => { + expect(getProposalCardState('input-streaming', true, false)).toBe('interrupted'); + expect(getProposalCardState('input-available', true, true)).toBe('interrupted'); + }); + + it('shows working while the tool is running and not stopped', () => { + expect(getProposalCardState('input-streaming', false, false)).toBe('working'); + }); + + it('surfaces tool errors', () => { + expect(getProposalCardState('output-error', false, true)).toBe('error'); + }); + + it('treats a completed run with content as done even if previously stopped flag set', () => { + // Completed (output-available) overrides the stopped heuristic. + expect(getProposalCardState('output-available', true, true)).toBe('done'); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/apply-suggestion.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/apply-suggestion.ts new file mode 100644 index 0000000000..cc0a299f99 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/apply-suggestion.ts @@ -0,0 +1,92 @@ +import type { Node as ProseMirrorNode } from '@tiptap/pm/model'; +import type { EditorState } from '@tiptap/pm/state'; +import { markdownToTipTapJSON } from '../components/ai/markdown-utils'; +import type { SuggestionRange } from './suggestion-types'; + +/** + * Extend delete ranges that start at a heading to cover the whole section (up to + * the next heading of the same or higher level), so "remove this section" takes + * the heading AND all its content even if the diff only flagged part of it. + */ +export function extendDeleteRangesToSections( + doc: ProseMirrorNode, + ranges: SuggestionRange[], +): SuggestionRange[] { + return ranges.map((range) => { + if (range.type !== 'delete') return range; + + // Is the start of the range a heading? + let headingLevel: number | null = null; + doc.nodesBetween(range.from, Math.min(range.from + 5, range.to), (node) => { + if (node.type.name === 'heading' && headingLevel === null) { + headingLevel = (node.attrs as { level?: number }).level ?? 1; + } + }); + if (headingLevel === null) return range; + + // Walk forward to the next heading of the same or higher level. + let nextHeadingPos: number | null = null; + doc.nodesBetween(range.to, doc.content.size, (node, pos) => { + if (nextHeadingPos !== null) return false; + if (node.type.name === 'heading') { + const level = (node.attrs as { level?: number }).level ?? 1; + if (headingLevel !== null && level <= headingLevel) { + nextHeadingPos = pos; + return false; + } + } + return true; + }); + + const extendTo = nextHeadingPos ?? doc.content.size; + return extendTo > range.to ? { ...range, to: extendTo } : range; + }); +} + +/** + * Build the ProseMirror nodes to insert/replace for an accepted suggestion. + * + * When the edit lands inside a list, unwrap the list wrappers that + * markdownToTipTapJSON emits ("- text" -> bulletList) down to their listItems, + * so we replace a listItem with listItem(s) instead of dropping a whole new + * list into the parent list — which would otherwise split the list into pieces + * or nest it (CS-265). Outside a list, the nodes are used as-is. + */ +export function buildReplacementNodes( + state: EditorState, + proposedText: string, + at: number, +): ProseMirrorNode[] { + const pmNodes = markdownToTipTapJSON(proposedText).map((json) => + state.schema.nodeFromJSON(json), + ); + + // Resolve the parent at the edit site. Guarded so a malformed/out-of-range + // position can never throw mid-apply — fall back to the nodes as-is. + let parent: ProseMirrorNode; + try { + parent = state.doc.resolve(at).parent; + } catch { + return pmNodes; + } + const parentName = parent.type.name; + + // Inside a list: unwrap the list wrappers markdownToTipTapJSON emits + // ("- text" -> bulletList) down to their listItems, so we replace a listItem + // with listItem(s) rather than dropping a whole new list into the parent list + // (which would split or nest it). CS-265. + if (parentName === 'bulletList' || parentName === 'orderedList') { + return pmNodes.flatMap((node) => { + if (node.type.name !== 'bulletList' && node.type.name !== 'orderedList') { + return [node]; + } + const items: ProseMirrorNode[] = []; + node.forEach((child) => items.push(child)); + return items; + }); + } + + // Otherwise the range spans whole block boundaries (see build-position-map), + // so the block nodes replace cleanly as-is. + return pmNodes; +} diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/build-position-map.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/build-position-map.ts index d7ff0cdf60..8986b18e57 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/build-position-map.ts +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/build-position-map.ts @@ -1,4 +1,5 @@ import type { Node as ProseMirrorNode } from '@tiptap/pm/model'; +import { serializeInline } from './policy-markdown'; import type { PositionMap } from './suggestion-types'; export function buildPositionMap(doc: ProseMirrorNode): PositionMap { @@ -17,33 +18,42 @@ export function buildPositionMap(doc: ProseMirrorNode): PositionMap { const nodeFrom = offset + 1; // +1 because doc node itself takes position 0 if (node.type.name === 'bulletList' || node.type.name === 'orderedList') { + // `nodeFrom` (offset + 1) is already the position of the list's first + // child boundary, and `childOffset` is the listItem's offset within the + // list's content. The previous `+ 1` double-counted the boundary, pushing + // every range one position forward so `to` overshot into the NEXT item — + // which made single-bullet edits bleed into adjacent bullets (CS-265). node.forEach((listItem, childOffset) => { - const itemFrom = nodeFrom + 1 + childOffset; + const itemFrom = nodeFrom + childOffset; const itemTo = itemFrom + listItem.nodeSize; - const text = extractText(listItem); + const text = serializeInline(listItem); entries.push({ type: 'list-item', markdown: '- ' + text, from: itemFrom, to: itemTo }); }); return; } - const nodeTo = nodeFrom + node.nodeSize - 2; + // Use the node's outer boundaries [before, after] so accept/delete operate + // on the whole block. Using the inner content range instead leaves an empty + // block behind on delete, and splits the block on replace (CS-265). + const nodeStart = offset; + const nodeEnd = offset + node.nodeSize; if (node.type.name === 'heading') { const level = (node.attrs.level as number) || 1; - const text = extractText(node); - entries.push({ type: 'heading', markdown: '#'.repeat(level) + ' ' + text, from: nodeFrom, to: nodeTo }); + const text = serializeInline(node); + entries.push({ type: 'heading', markdown: '#'.repeat(level) + ' ' + text, from: nodeStart, to: nodeEnd }); } else if (node.type.name === 'paragraph') { - const text = extractText(node); - entries.push({ type: 'paragraph', markdown: text, from: nodeFrom, to: nodeTo }); + const text = serializeInline(node); + entries.push({ type: 'paragraph', markdown: text, from: nodeStart, to: nodeEnd }); } else if (node.type.name === 'blockquote') { - const text = extractText(node); - entries.push({ type: 'other', markdown: '> ' + text, from: nodeFrom, to: nodeTo }); + const text = serializeInline(node); + entries.push({ type: 'other', markdown: '> ' + text, from: nodeStart, to: nodeEnd }); } else if (node.type.name === 'horizontalRule') { - entries.push({ type: 'other', markdown: '---', from: nodeFrom, to: nodeTo }); + entries.push({ type: 'other', markdown: '---', from: nodeStart, to: nodeEnd }); } else { - const text = extractText(node); + const text = serializeInline(node); if (text) { - entries.push({ type: 'other', markdown: text, from: nodeFrom, to: nodeTo }); + entries.push({ type: 'other', markdown: text, from: nodeStart, to: nodeEnd }); } } }); @@ -65,8 +75,11 @@ export function buildPositionMap(doc: ProseMirrorNode): PositionMap { currentLine++; } - // Add blank line before first list item if previous wasn't a list item - if (entry.type === 'list-item' && prevType !== 'list-item') { + // Add blank line before a list group, but never when the list is the very + // first block: a leading blank line gets removed by the trailing .trim() + // below, which would shift every lineToPos key out of sync with the emitted + // markdown and mis-map edits in list-first documents. + if (entry.type === 'list-item' && prevType !== 'list-item' && prevType !== null) { markdownLines.push(''); currentLine++; } @@ -115,15 +128,3 @@ export function buildPositionMap(doc: ProseMirrorNode): PositionMap { markdown: markdownLines.join('\n').trim(), }; } - -function extractText(node: ProseMirrorNode): string { - let text = ''; - node.forEach((child) => { - if (child.isText) { - text += child.text ?? ''; - } else { - text += extractText(child); - } - }); - return text; -} diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/compute-suggestion-ranges.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/compute-suggestion-ranges.ts index 7eecd5e4da..ae88f29e2c 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/compute-suggestion-ranges.ts +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/compute-suggestion-ranges.ts @@ -87,13 +87,15 @@ function mergeOverlappingRanges(ranges: SuggestionRange[]): SuggestionRange[] { continue; } - // Merge if overlapping or adjacent (gap of ≤20 positions, which covers - // a few empty paragraphs / blank lines between hunks). - // Always merge two adjacent deletes — they're almost certainly one section. + // Merge only when it can't drop unchanged content between the ranges: + // - overlapping ranges (any type), or + // - adjacent deletes split by blank-line context (one logical section). + // Do NOT merge two modifies across a gap — the unchanged content in the gap + // (e.g. a heading between two edited paragraphs) is absent from the merged + // proposedText and would be deleted on apply. const gap = range.from - prev.to; const shouldMerge = gap <= 0 || // overlapping - (gap <= 20 && prev.type === range.type) || // same type, close together (gap <= 20 && prev.type === 'delete' && range.type === 'delete'); // adjacent deletes if (shouldMerge) { @@ -125,9 +127,22 @@ function resolveHunkPositions( lineToPos: Map, ): { from: number; to: number } | null { if (oldLines === 0) { - const anchor = lineToPos.get(oldStart) ?? lineToPos.get(oldStart - 1); - if (!anchor) return findNearestPosition(oldStart, lineToPos); - return anchor; + // Pure insertion: collapse to a zero-width point at the right boundary. + // jsdiff's oldStart is the old line the new content is inserted BEFORE, so + // anchor to that line's start; otherwise insert after the previous line. + const before = lineToPos.get(oldStart); + if (before) return { from: before.from, to: before.from }; + const after = lineToPos.get(oldStart - 1); + if (after) return { from: after.to, to: after.to }; + // Fallback: anchor to the nearest mapped line, choosing the side by its + // position relative to the insertion point. If the nearest line is at/after + // oldStart, insert BEFORE it (.from); otherwise insert AFTER it (.to). + // Using .to unconditionally would drop content on the wrong side. + const near = findNearestEntry(oldStart, lineToPos); + if (!near) return null; + return near.line >= oldStart + ? { from: near.from, to: near.from } + : { from: near.to, to: near.to }; } let from: number | null = null; @@ -148,22 +163,30 @@ function resolveHunkPositions( return { from, to }; } -function findNearestPosition( +function findNearestEntry( targetLine: number, lineToPos: Map, -): { from: number; to: number } | null { - let closest: { from: number; to: number } | null = null; +): { line: number; from: number; to: number } | null { + let closest: { line: number; from: number; to: number } | null = null; let closestDist = Infinity; for (const [line, pos] of lineToPos) { const dist = Math.abs(line - targetLine); if (dist < closestDist) { closestDist = dist; - closest = pos; + closest = { line, from: pos.from, to: pos.to }; } } return closest; } +function findNearestPosition( + targetLine: number, + lineToPos: Map, +): { from: number; to: number } | null { + const entry = findNearestEntry(targetLine, lineToPos); + return entry ? { from: entry.from, to: entry.to } : null; +} + function computeWordDiff(oldText: string, newText: string): DiffSegment[] { const changes = diffWords(oldText, newText); return changes.map((change) => ({ @@ -173,12 +196,16 @@ function computeWordDiff(oldText: string, newText: string): DiffSegment[] { } /** - * Aggressively normalize text for comparison: - * - Strip list markers (- , * , 1. ) - * - Strip heading markers (# ## ### etc.) - * - Collapse all whitespace - * - Lowercase - * This catches formatting-only diffs the AI introduces. + * Normalize text for comparison: + * - Strip block markers (list - / * , heading #, blockquote >) so the AI + * reformatting a block marker isn't treated as a content change. + * - Collapse all whitespace + lowercase. + * + * NOTE: inline marks (bold/italic/code/link) are deliberately NOT stripped. + * The markdown encoder is mark-aware and symmetric with the model's output, so + * unchanged formatted content already compares equal — while a user explicitly + * adding/removing formatting (e.g. "make this bold") still produces an + * accept-able suggestion instead of being silently swallowed (CS-265). */ function normalizeContent(text: string): string { return text diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/policy-markdown.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/policy-markdown.ts new file mode 100644 index 0000000000..d1f569ae07 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/policy-markdown.ts @@ -0,0 +1,192 @@ +import type { Node as ProseMirrorNode } from '@tiptap/pm/model'; +import type { JSONContent } from '@tiptap/react'; + +/** + * Shared markdown helpers for the policy AI editor. + * + * Both the diff pipeline (build-position-map -> compute-suggestion-ranges) + * and the apply pipeline (markdown-utils -> ProseMirror) must agree on how + * markdown maps to document text, otherwise the diff and the applied edit + * drift apart and produce phantom ranges / misplaced edits. Keeping the + * conversion in one place is the single source of truth for that contract. + * + * Inline marks supported (encode + decode, kept symmetric): bold (**), italic + * (*), bold+italic (***), inline code (`) and links ([text](href)). Underscore + * emphasis is intentionally NOT parsed so snake_case identifiers in policy text + * are never mangled. Because encode and decode use the same conventions, + * unchanged formatted content round-trips identically and the line diff sees no + * change — while a real formatting edit (e.g. adding bold) still diffs and can + * be accepted. + */ + +/** + * Control characters that must never reach the document. We keep TAB + * (U+0009) and LF (U+000A); CR is normalized to LF separately. Everything + * else in the C0/C1 control ranges is stripped -- including U+000B (vertical + * tab, octal 013), which the model occasionally emits and which surfaces as a + * stray "013" glyph inside policy text. + */ +const CONTROL_CHARS = /[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F-\u009F]/g; + +/** + * Strip control-character noise from AI-generated markdown before it enters + * the editor. Preserves newlines and tabs; normalizes CR/CRLF to LF so the + * line-based diff stays stable. + */ +export function sanitizeMarkdown(markdown: string): string { + if (!markdown) return markdown; + return markdown.replace(/\r\n?/g, '\n').replace(CONTROL_CHARS, ''); +} + +/** Collapse runs of spaces/tabs to a single space (SALE-65 stray whitespace). */ +function collapseSpaces(text: string): string { + return text.replace(/[ \t]{2,}/g, ' '); +} + +interface InlineMark { + type: string; + attrs?: Record; +} + +function addMark(marks: InlineMark[], mark: InlineMark): InlineMark[] { + if (marks.some((m) => m.type === mark.type)) return marks; + return [...marks, mark]; +} + +function makeTextNode(text: string, marks: InlineMark[]): JSONContent { + const isCode = marks.some((m) => m.type === 'code'); + // Code spans are literal — never collapse their whitespace. + const value = isCode ? text : collapseSpaces(text); + return marks.length > 0 + ? { type: 'text', text: value, marks: marks.map((m) => ({ ...m })) } + : { type: 'text', text: value }; +} + +/** + * Parse a single line of inline markdown into TipTap text nodes with marks. + * Recurses for nestable marks (bold/italic/link); code spans are terminal. + */ +export function parseInline(text: string): JSONContent[] { + return parseInlineWithMarks(text, []); +} + +function parseInlineWithMarks(text: string, marks: InlineMark[]): JSONContent[] { + const nodes: JSONContent[] = []; + let buffer = ''; + let i = 0; + + const flush = () => { + if (buffer) { + nodes.push(makeTextNode(buffer, marks)); + buffer = ''; + } + }; + + while (i < text.length) { + const rest = text.slice(i); + + // Link: [text](href) + const link = /^\[([^\]]*)\]\(([^)]*)\)/.exec(rest); + if (link) { + flush(); + nodes.push( + ...parseInlineWithMarks( + link[1]!, + addMark(marks, { type: 'link', attrs: { href: link[2]! } }), + ), + ); + i += link[0].length; + continue; + } + + // Inline code: `text` (terminal — no nested marks, raw content) + const code = /^`([^`]+)`/.exec(rest); + if (code) { + flush(); + nodes.push(makeTextNode(code[1]!, addMark(marks, { type: 'code' }))); + i += code[0].length; + continue; + } + + // Bold + italic: ***text*** + const boldItalic = /^\*\*\*([^]+?)\*\*\*/.exec(rest); + if (boldItalic) { + flush(); + nodes.push( + ...parseInlineWithMarks( + boldItalic[1]!, + addMark(addMark(marks, { type: 'bold' }), { type: 'italic' }), + ), + ); + i += boldItalic[0].length; + continue; + } + + // Bold: **text** + const bold = /^\*\*([^]+?)\*\*/.exec(rest); + if (bold) { + flush(); + nodes.push(...parseInlineWithMarks(bold[1]!, addMark(marks, { type: 'bold' }))); + i += bold[0].length; + continue; + } + + // Italic: *text* (single line, no nested asterisk) + const italic = /^\*([^*\n]+?)\*/.exec(rest); + if (italic) { + flush(); + nodes.push(...parseInlineWithMarks(italic[1]!, addMark(marks, { type: 'italic' }))); + i += italic[0].length; + continue; + } + + buffer += text[i]; + i += 1; + } + + flush(); + return nodes; +} + +/** + * Serialize a block node's inline content back to markdown, mirroring + * parseInline so encode/decode round-trip. Recurses into block children + * (e.g. a listItem's paragraph) and concatenates text. + */ +export function serializeInline(node: ProseMirrorNode): string { + let out = ''; + node.forEach((child) => { + if (child.isText) { + out += applyMarks(child.text ?? '', child.marks); + } else if (child.type.name === 'hardBreak') { + out += ' '; + } else { + out += serializeInline(child); + } + }); + return out; +} + +function applyMarks( + text: string, + marks: readonly { type: { name: string }; attrs: Record }[], +): string { + const names = new Set(marks.map((m) => m.type.name)); + const link = marks.find((m) => m.type.name === 'link'); + + let out = names.has('code') ? '`' + text + '`' : collapseSpaces(text); + + if (names.has('bold') && names.has('italic')) { + out = '***' + out + '***'; + } else if (names.has('bold')) { + out = '**' + out + '**'; + } else if (names.has('italic')) { + out = '*' + out + '*'; + } + + if (link && typeof link.attrs.href === 'string') { + out = '[' + out + '](' + link.attrs.href + ')'; + } + + return out; +} diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/proposal-card-state.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/proposal-card-state.ts new file mode 100644 index 0000000000..27d28956a8 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/proposal-card-state.ts @@ -0,0 +1,29 @@ +/** + * Pure decision for how the proposePolicy tool card should render, derived from + * the AI-SDK tool-part state, whether streaming stopped, and whether any policy + * content actually arrived. + * + * - 'interrupted': the run stopped before the tool finished. + * - 'working': the tool is still running. + * - 'error': the tool errored. + * - 'incomplete': the tool "completed" but no content materialized — a + * truncated run that must NOT be reported as success (CS-256). + * - 'done': completed with real content. + */ +export type ProposalCardState = 'interrupted' | 'working' | 'error' | 'incomplete' | 'done'; + +export function getProposalCardState( + toolState: string, + stopped: boolean, + hasContent: boolean, +): ProposalCardState { + const isCompleted = toolState === 'output-available'; + const isError = toolState === 'output-error'; + const isWorking = !isCompleted && !isError; + + if (isWorking && stopped) return 'interrupted'; + if (isWorking) return 'working'; + if (isError) return 'error'; + if (!hasContent) return 'incomplete'; + return 'done'; +} diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/test-helpers/editor-schema.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/test-helpers/editor-schema.ts index 188811f464..2fc56ff479 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/test-helpers/editor-schema.ts +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/lib/test-helpers/editor-schema.ts @@ -59,6 +59,10 @@ export const schema = new Schema({ parseDOM: [{ tag: 'em' }], toDOM: () => ['em', 0] as const, }, + code: { + parseDOM: [{ tag: 'code' }], + toDOM: () => ['code', 0] as const, + }, link: { attrs: { href: {} }, parseDOM: [ diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/tools/policy-tools.ts b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/tools/policy-tools.ts index f77a2bfd04..cd1385f90a 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/tools/policy-tools.ts +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/tools/policy-tools.ts @@ -66,8 +66,14 @@ export function getPolicyTools({ currentPolicyId, cookieHeader }: PolicyToolsOpt 'A very short imperative phrase that tells the user to review the updated policy content in the editor below.', ), }), - execute: async ({ summary, title, detail, reviewHint }) => ({ - success: true, + // Report success based on whether content actually materialized. When a + // response is truncated (timeout / output-token cap), the tool call can + // complete with empty/partial `content`, yet the assistant's prose still + // claims success — leaving the user with a blank policy (CS-256). Tie the + // tool's own `success` flag to real content so the UI can flag it. + execute: async ({ content, summary, title, detail, reviewHint }) => ({ + success: content.trim().length > 0, + contentLength: content.length, summary, title, detail, diff --git a/apps/app/src/app/api/policies/[policyId]/chat/route.ts b/apps/app/src/app/api/policies/[policyId]/chat/route.ts index 97798e22b3..f9a6b5dd69 100644 --- a/apps/app/src/app/api/policies/[policyId]/chat/route.ts +++ b/apps/app/src/app/api/policies/[policyId]/chat/route.ts @@ -6,7 +6,10 @@ import { headers } from 'next/headers'; import { NextResponse } from 'next/server'; import { getPolicyTools } from '../../../../(app)/[orgId]/policies/[policyId]/editor/tools/policy-tools'; -export const maxDuration = 60; +// Full-policy regenerations can be large; 60s was too tight and cut streams +// mid-tool-call, leaving runs stuck "Interrupted". Hosting plans clamp this to +// their own max, so raising it is safe (no-op where unsupported). +export const maxDuration = 300; export async function POST(req: Request, { params }: { params: Promise<{ policyId: string }> }) { try { @@ -193,6 +196,10 @@ You MUST produce the policy by starting from the text above and messages: await convertToModelMessages(cleanedMessages), toolChoice: 'auto', stopWhen: stepCountIs(5), + // The model must return the COMPLETE policy in the proposePolicy tool + // input. Without an explicit cap, large policies can be truncated mid-JSON + // (blank/partial content → CS-256). Give ample headroom for a full policy. + maxOutputTokens: 16000, tools: getPolicyTools({ currentPolicyId: policyId, cookieHeader: cookieStr }), }); diff --git a/apps/app/src/app/api/policies/[policyId]/edit-section/route.ts b/apps/app/src/app/api/policies/[policyId]/edit-section/route.ts index 998f5b8c21..443ca0a900 100644 --- a/apps/app/src/app/api/policies/[policyId]/edit-section/route.ts +++ b/apps/app/src/app/api/policies/[policyId]/edit-section/route.ts @@ -3,6 +3,7 @@ import { anthropic } from '@ai-sdk/anthropic'; import { generateText } from 'ai'; import { headers } from 'next/headers'; import { NextResponse } from 'next/server'; +import { sanitizeMarkdown } from '../../../../(app)/[orgId]/policies/[policyId]/editor/lib/policy-markdown'; export const maxDuration = 30; @@ -33,6 +34,9 @@ export async function POST(req: Request) { const result = await generateText({ model: anthropic('claude-sonnet-4-6'), + // A single section is small; cap output so a runaway generation can't hang + // the 30s request, and so we get a clean stop rather than a truncated edit. + maxOutputTokens: 4000, system: `You are a GRC policy editor. You will receive text from a policy and feedback on how to change it. Return ONLY the updated text. Rules: - Do not include explanations, preamble, or commentary — just the updated text. - If the input is a plain sentence or paragraph, return a plain sentence or paragraph. Do NOT add markdown formatting (no ##, no **, no -) unless the input already uses it. @@ -41,7 +45,7 @@ export async function POST(req: Request) { prompt: `Text to edit:\n${sectionText}\n\nInstruction: ${feedback}`, }); - return NextResponse.json({ updatedText: result.text.trim() }); + return NextResponse.json({ updatedText: sanitizeMarkdown(result.text).trim() }); } catch (error) { console.error('Edit section error:', error); return NextResponse.json({ error: 'Failed to edit section' }, { status: 500 }); diff --git a/packages/ui/src/components/editor/utils/validate-content.test.ts b/packages/ui/src/components/editor/utils/validate-content.test.ts index 6c748418e3..6ec235ba11 100644 --- a/packages/ui/src/components/editor/utils/validate-content.test.ts +++ b/packages/ui/src/components/editor/utils/validate-content.test.ts @@ -337,6 +337,79 @@ describe('validateAndFixTipTapContent', () => { }); }); + describe('table header preservation (CS-418)', () => { + it('preserves a tableHeader row instead of collapsing it to an empty cell', () => { + const content = { + type: 'doc', + content: [ + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { type: 'tableHeader', attrs: { colspan: 1, rowspan: 1 }, content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Name' }] }] }, + { type: 'tableHeader', attrs: { colspan: 1, rowspan: 1 }, content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Role' }] }] }, + ], + }, + { + type: 'tableRow', + content: [ + { type: 'tableCell', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Alice' }] }] }, + { type: 'tableCell', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Admin' }] }] }, + ], + }, + ], + }, + ], + }; + + const fixed = validateAndFixTipTapContent(content); + const table = (fixed.content as any[])[0]; + expect(table.type).toBe('table'); + + const headerRow = table.content[0]; + expect(headerRow.content).toHaveLength(2); + expect(headerRow.content[0].type).toBe('tableHeader'); + expect(headerRow.content[1].type).toBe('tableHeader'); + // Header text must survive + expect(headerRow.content[0].content[0].content[0].text).toBe('Name'); + expect(headerRow.content[1].content[0].content[0].text).toBe('Role'); + // Header attrs (colspan/rowspan/colwidth) must survive + expect(headerRow.content[0].attrs).toEqual({ colspan: 1, rowspan: 1 }); + + // Body row still works + const bodyRow = table.content[1]; + expect(bodyRow.content[0].type).toBe('tableCell'); + expect(bodyRow.content[0].content[0].content[0].text).toBe('Alice'); + }); + + it('preserves mixed header + cell within a single row', () => { + const content = { + type: 'doc', + content: [ + { + type: 'table', + content: [ + { + type: 'tableRow', + content: [ + { type: 'tableHeader', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Key' }] }] }, + { type: 'tableCell', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Value' }] }] }, + ], + }, + ], + }, + ], + }; + + const fixed = validateAndFixTipTapContent(content); + const row = (fixed.content as any[])[0].content[0]; + expect(row.content[0].type).toBe('tableHeader'); + expect(row.content[1].type).toBe('tableCell'); + }); + }); + describe('empty text node handling', () => { const strip = (s: string) => s.replace(/[\u00A0\u200B\u202F]/g, '').trim(); diff --git a/packages/ui/src/components/editor/utils/validate-content.ts b/packages/ui/src/components/editor/utils/validate-content.ts index 8626e3b372..6810223029 100644 --- a/packages/ui/src/components/editor/utils/validate-content.ts +++ b/packages/ui/src/components/editor/utils/validate-content.ts @@ -193,6 +193,8 @@ function fixNode(node: any): JSONContent | null { return fixTableRow(node); case 'tableCell': return fixTableCell(node); + case 'tableHeader': + return fixTableHeader(node); case 'text': return fixTextNode(node); case 'heading': @@ -408,7 +410,14 @@ function fixTableRow(node: any): JSONContent { let cells: JSONContent[] = []; if (Array.isArray(content)) { cells = content - .map((child: any) => (child?.type === 'tableCell' ? fixTableCell(child) : null)) + .map((child: any) => { + // Preserve header cells. Previously only `tableCell` was handled, so + // pasted/AI-generated header rows (`tableHeader`, e.g. from ) were + // dropped and an all-header row collapsed to a single empty cell. + if (child?.type === 'tableHeader') return fixTableHeader(child); + if (child?.type === 'tableCell') return fixTableCell(child); + return null; + }) .filter(Boolean) as JSONContent[]; } if (cells.length === 0) { @@ -417,7 +426,7 @@ function fixTableRow(node: any): JSONContent { return { type: 'tableRow', content: cells, ...(attrs && { attrs }), ...rest }; } -function fixTableCell(node: any): JSONContent { +function fixTableCellLike(node: any, type: 'tableCell' | 'tableHeader'): JSONContent { const { content, attrs, ...rest } = node; let blocks: JSONContent[] = []; if (Array.isArray(content)) { @@ -426,7 +435,15 @@ function fixTableCell(node: any): JSONContent { if (blocks.length === 0) { blocks = [createEmptyParagraph()]; } - return { type: 'tableCell', content: blocks, ...(attrs && { attrs }), ...rest }; + return { type, content: blocks, ...(attrs && { attrs }), ...rest }; +} + +function fixTableCell(node: any): JSONContent { + return fixTableCellLike(node, 'tableCell'); +} + +function fixTableHeader(node: any): JSONContent { + return fixTableCellLike(node, 'tableHeader'); } /**