diff --git a/apps/mobile/src/lib/composerContext.test.ts b/apps/mobile/src/lib/composerContext.test.ts index d28b58d055b0..a17941c04933 100644 --- a/apps/mobile/src/lib/composerContext.test.ts +++ b/apps/mobile/src/lib/composerContext.test.ts @@ -159,10 +159,25 @@ describe("mobile composer context", () => { it("keeps exact source positions and repeated references alongside existing native tokens", () => { const reference = formatComposerContextReference(terminal); const text = `Use $playwright and [app.ts](src/app.ts) with ${reference} then ${reference}`; - const tokens = composerContextEditorTokens(text, collectComposerInlineTokens(text)); + const tokens = composerContextEditorTokens( + text, + collectComposerInlineTokens(text), + new Set(["playwright"]), + ); expect(tokens.map((token) => token.type)).toEqual(["skill", "mention", "context", "context"]); for (const token of tokens) expect(text.slice(token.start, token.end)).toBe(token.source); }); + it("leaves a $name that is not one of the provider's skills as plain text", () => { + const text = "Run $playwright then echo $HOME and $notaskill "; + const tokensFor = (skills: ReadonlyArray) => + composerContextEditorTokens(text, collectComposerInlineTokens(text), new Set(skills)).map( + (token) => token.source, + ); + expect(tokensFor(["playwright"])).toEqual(["$playwright"]); + // The skill list loads after the editor opens, and differs per provider. + expect(tokensFor([])).toEqual([]); + expect(tokensFor(["playwright", "notaskill"])).toEqual(["$playwright", "$notaskill"]); + }); it("removes deleted payloads but keeps the screenshot linked to a remaining annotation", () => { const context: OrchestrationMessageContext = { diff --git a/apps/mobile/src/lib/composerContext.ts b/apps/mobile/src/lib/composerContext.ts index c5422405c966..af6263bb8e79 100644 --- a/apps/mobile/src/lib/composerContext.ts +++ b/apps/mobile/src/lib/composerContext.ts @@ -144,11 +144,18 @@ export function pullRequestComposerContext( } /** Native editors collapse the canonical source range to a single atomic attachment. */ -export function composerContextEditorTokens(text: string, tokens: readonly ComposerInlineToken[]) { +export function composerContextEditorTokens( + text: string, + tokens: readonly ComposerInlineToken[], + knownSkills: { has(name: string): boolean }, +) { const references = collectComposerContextReferences(text); return [ ...tokens.filter( - (token) => !references.some((ref) => token.start < ref.end && token.end > ref.start), + (token) => + // `$HOME` and other `$name` text that is not one of the provider's skills stays prose. + (token.type !== "skill" || knownSkills.has(token.value)) && + !references.some((ref) => token.start < ref.end && token.end > ref.start), ), ...references.map((ref) => ({ type: "context" as const, diff --git a/apps/mobile/src/native/T3ComposerEditor.ios.tsx b/apps/mobile/src/native/T3ComposerEditor.ios.tsx index 68c340769868..fab4603b8ca5 100644 --- a/apps/mobile/src/native/T3ComposerEditor.ios.tsx +++ b/apps/mobile/src/native/T3ComposerEditor.ios.tsx @@ -168,7 +168,7 @@ export function ComposerEditor({ }); confirmedTokensRef.current = tokens; return JSON.stringify( - composerContextEditorTokens(props.value, tokens).map((token) => { + composerContextEditorTokens(props.value, tokens, skillLabels).map((token) => { const record = token.type === "context" ? props.context?.records.find((record) => record.contextId === token.contextId) diff --git a/apps/mobile/src/native/T3ComposerEditor.native.tsx b/apps/mobile/src/native/T3ComposerEditor.native.tsx index 9391a3814c39..b99d29095ced 100644 --- a/apps/mobile/src/native/T3ComposerEditor.native.tsx +++ b/apps/mobile/src/native/T3ComposerEditor.native.tsx @@ -166,7 +166,7 @@ export function ComposerEditor({ }); confirmedTokensRef.current = tokens; return JSON.stringify( - composerContextEditorTokens(props.value, tokens).map((token) => { + composerContextEditorTokens(props.value, tokens, skillLabels).map((token) => { const record = token.type === "context" ? props.context?.records.find((record) => record.contextId === token.contextId) diff --git a/apps/web/src/components/ComposerPromptEditorTiptap.tsx b/apps/web/src/components/ComposerPromptEditorTiptap.tsx index 5d5c87f53881..290a28fe5829 100644 --- a/apps/web/src/components/ComposerPromptEditorTiptap.tsx +++ b/apps/web/src/components/ComposerPromptEditorTiptap.tsx @@ -57,18 +57,18 @@ import { ComposerCodeBlockExtension, ComposerListExtensions, buildTiptapContent, - collapsedToFlat, caretTakesMarksBefore, convertBulletItemToTask, ComposerCodeExtension, ComposerTaskItemExtension, ComposerTaskListExtension, - flatToCollapsed, flatToMarkdown, flatToPm, + markdownToFlat, pmToFlat, serializeEditorDoc, serializeSelection, + skillChipReplacements, splitOrLiftListItem, stepCaretAcrossStyledEdge, type SkillMeta, @@ -312,13 +312,30 @@ const ComposerSkillExtension = Node.create({ }, }); +/** Chip presentation for `$name`, or null when the provider has no such skill. */ +function composerSkillMeta( + skills: ReadonlyArray, + name: string, +): SkillMeta | null { + const skill = skills.find((candidate) => candidate.name === name); + if (!skill) return null; + const shortDescription = skill.shortDescription?.trim(); + return { + label: formatProviderSkillDisplayName(skill), + description: shortDescription || skill.description?.trim() || null, + }; +} + function ComposerSkillNodeView({ node }: NodeViewProps) { const actions = use(ComposerContextActionsContext); const skills = use(RichComposerSkillsContext); const skillName = (node.attrs.skillName as string) ?? ""; - const skillLabel = (node.attrs.skillLabel as string) || skillName; const skillDescription = (node.attrs.skillDescription as string | null) ?? null; const skill = skills.find((candidate) => candidate.name === skillName); + // The node outlives a provider switch, so its stored label can be another provider's. + const skillLabel = skill + ? formatProviderSkillDisplayName(skill) + : (node.attrs.skillLabel as string) || skillName; return (

{skillLabel}

- {skill?.description ?? - skillDescription ?? + {(skill ? skill.description : skillDescription) ?? "No description is available for this skill."}

{skill?.path ? ( @@ -778,12 +794,32 @@ const ComposerMarkersExtension = Extension.create({ type TiptapEditor = NonNullable>; export function ComposerPromptEditorTiptap(props: ComposerPromptEditorProps) { + const { onChange: onPromptChange } = props; + const [selection, setSelection] = useState<{ + value: string; + cursor: number; + expandedCursor: number; + } | null>(null); + const onChange = useCallback( + (value, cursor, expandedCursor, ...rest) => { + setSelection({ value, cursor, expandedCursor }); + onPromptChange(value, cursor, expandedCursor, ...rest); + }, + [onPromptChange], + ); // Extensions are creation-time, so changing text mode remounts the editor. - // Every mode initializes from the controlled value, preserving the draft. + // Keep the exact Markdown caret across that remount: the collapsed cursor + // cannot represent positions inside an unknown `$name` rendered as text. return ( ); } @@ -808,7 +844,9 @@ const ComposerUndoGroupingExtension = Extension.create< }, }); -function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { +function ComposerPromptEditorTiptapInner( + props: ComposerPromptEditorProps & { initialExpandedCursor: number }, +) { const { value, cursor, @@ -886,28 +924,17 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { useEffect(() => { importFragmentRef.current = importContextFragment; }, [importContextFragment]); - useEffect(() => { - skillsRef.current = skills; - }, [skills]); useLayoutEffect(() => { latestValueRef.current = value; }, [value]); - const skillLabelFor = useCallback((name: string): SkillMeta => { - const normalized = name.startsWith("$") ? name.slice(1) : name; - const skill = skillsRef.current.find((candidate) => candidate.name === normalized); - if (!skill) { - return { label: formatProviderSkillDisplayName({ name: normalized }), description: null }; - } - const shortDescription = skill.shortDescription?.trim(); - return { - label: formatProviderSkillDisplayName(skill), - description: shortDescription || skill.description?.trim() || null, - }; - }, []); + const skillLabelFor = useCallback( + (name: string) => composerSkillMeta(skillsRef.current, name), + [], + ); const initialCursor = clampEditorCursor(value, cursor); - const initialExpandedCursor = expandEditorCursor(value, initialCursor); + const initialExpandedCursor = Math.max(0, Math.min(value.length, props.initialExpandedCursor)); const snapshotRef = useRef({ value, cursor: initialCursor, @@ -947,11 +974,11 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { const fromFlat = pmToFlat(map, from); const toFlat = pmToFlat(map, to); const nextValue = map.value; - const nextCursor = clampEditorCursor(map.value, flatToCollapsed(map, fromFlat)); const nextExpandedCursor = Math.max( 0, Math.min(map.value.length, flatToMarkdown(map, fromFlat)), ); + const nextCursor = collapseExpandedComposerCursor(map.value, nextExpandedCursor, literalText); const nextSelectionRange = { start: Math.min(nextExpandedCursor, flatToMarkdown(map, toFlat)), end: Math.max(nextExpandedCursor, flatToMarkdown(map, toFlat)), @@ -998,7 +1025,7 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { isCollapsedCursorAdjacentToInlineToken(nextValue, nextCursor, "right"); onChangeRef.current(nextValue, nextCursor, nextExpandedCursor, suppressTrigger, contextIds); }, - [clampEditorCursor, literalText], + [literalText], ); const editorAttributes = useMemo( @@ -1139,25 +1166,10 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { ] : []), ], - content: buildDocJson( - value, - (name) => { - const normalized = name.startsWith("$") ? name.slice(1) : name; - const found = skills.find((candidate) => candidate.name === normalized); - if (!found) { - return { - label: formatProviderSkillDisplayName({ name: normalized }), - description: null, - }; - } - const shortDescription = found.shortDescription?.trim(); - return { - label: formatProviderSkillDisplayName(found), - description: shortDescription || found.description?.trim() || null, - }; - }, - { styling: richText, literalText }, - ), + content: buildDocJson(value, (name) => composerSkillMeta(skills, name), { + styling: richText, + literalText, + }), editable: !disabled, editorProps: { attributes: editorAttributes, @@ -1405,17 +1417,14 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { : pastedText; // Complete chips at paste boundaries just as autocomplete does. const tokens = literalText ? [] : collectComposerPromptInlineTokens(`${text}\n`); + const isChip = (token: (typeof tokens)[number] | undefined) => + token?.type === "mention" || + (token?.type === "skill" && skillLabelFor(token.value) !== null); const lastToken = tokens.at(-1); - if ( - (lastToken?.type === "mention" || lastToken?.type === "skill") && - lastToken.end === text.length - ) { + if (isChip(lastToken) && lastToken?.end === text.length) { text += " "; } - if ( - (tokens[0]?.type === "mention" || tokens[0]?.type === "skill") && - tokens[0].start === 0 - ) { + if (isChip(tokens[0]) && tokens[0]?.start === 0) { const map = serializeEditorDoc(view.state.doc); const offset = flatToMarkdown(map, pmToFlat(map, view.state.selection.from)); if (offset > 0 && !/\s/.test(map.value[offset - 1]!)) text = ` ${text}`; @@ -1505,10 +1514,11 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { const map = serializeEditorDoc(editor.state.doc); const { from, to } = editor.state.selection; const fromFlat = pmToFlat(map, from); + const expandedCursor = Math.max(0, Math.min(map.value.length, flatToMarkdown(map, fromFlat))); const next: typeof snapshot = { value: map.value, - cursor: clampEditorCursor(map.value, flatToCollapsed(map, fromFlat)), - expandedCursor: Math.max(0, Math.min(map.value.length, flatToMarkdown(map, fromFlat))), + cursor: collapseExpandedComposerCursor(map.value, expandedCursor, literalText), + expandedCursor, contextIds: literalText ? [] : map.contextIds, }; const toFlat = pmToFlat(map, to); @@ -1518,7 +1528,7 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { }; snapshotRef.current = next; return next; - }, [clampEditorCursor, editor, literalText]); + }, [editor, literalText]); // Controlled value/cursor from the store (history recall, chip insertion…). useLayoutEffect(() => { @@ -1534,7 +1544,9 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { ) { return; } - const normalizedExpandedCursor = expandEditorCursor(value, normalizedCursor); + const normalizedExpandedCursor = initialSelection + ? initialExpandedCursor + : expandEditorCursor(value, normalizedCursor); snapshotRef.current = { value, cursor: normalizedCursor, @@ -1562,7 +1574,7 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { ); } const map = serializeEditorDoc(editor.state.doc); - const flat = collapsedToFlat(map, normalizedCursor); + const flat = markdownToFlat(map, normalizedExpandedCursor); editor.commands.setTextSelection(flatToPm(map, flat)); if (isFocused) scrollTiptapCaretIntoView(editor); if (pendingCitation) { @@ -1593,14 +1605,32 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { cursor, editor, expandEditorCursor, + initialExpandedCursor, literalText, richText, skillLabelFor, value, ]); + // Skill chips follow the provider's skill list. Only the affected nodes are + // replaced, outside undo history, so undo cannot bring back a stale chip and + // citation chips keep the keys their open comment editors are bound to. + useEffect(() => { + const previousSkills = skillsRef.current; + skillsRef.current = skills; + if (!editor || literalText || previousSkills === skills) return; + if (previousSkills.length === 0 && skills.length === 0) return; + const replacements = skillChipReplacements(editor.state.doc, (name) => + composerSkillMeta(skills, name), + ); + if (replacements.length === 0) return; + const transaction = editor.state.tr.setMeta("addToHistory", false); + for (const { from, to, node } of replacements) transaction.replaceWith(from, to, node); + editor.view.dispatch(transaction); + }, [editor, literalText, skills]); + const focusAt = useCallback( - (nextCursor: number) => { + (nextCursor: number, exactExpandedCursor?: number) => { if (!editor) return; // view.focus() writes the state selection to the DOM as it focuses. A // bare DOM focus leaves the native caret at the start until @@ -1612,15 +1642,16 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { // overwrite that prompt; the pending rewrite places the caret instead. if (snapshotRef.current.value !== latestValueRef.current) return; const boundedCursor = clampEditorCursor(snapshotRef.current.value, nextCursor); + const expandedCursor = + exactExpandedCursor ?? expandEditorCursor(snapshotRef.current.value, boundedCursor); const map = serializeEditorDoc(editor.state.doc); - const flat = collapsedToFlat(map, boundedCursor); - editor.commands.setTextSelection(flatToPm(map, flat)); + editor.commands.setTextSelection(flatToPm(map, markdownToFlat(map, expandedCursor))); scrollTiptapCaretIntoView(editor); if (boundedCursor === snapshotRef.current.cursor) return; snapshotRef.current = { value: snapshotRef.current.value, cursor: boundedCursor, - expandedCursor: expandEditorCursor(snapshotRef.current.value, boundedCursor), + expandedCursor, contextIds: snapshotRef.current.contextIds, }; selectionRangeRef.current = { @@ -1642,9 +1673,11 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { editorRef, () => ({ focus: () => { - focusAt(snapshotRef.current.cursor); + // The collapsed cursor cannot say where inside `$name` text a caret + // sits, so restoring the caret passes along its exact offset. + focusAt(snapshotRef.current.cursor, snapshotRef.current.expandedCursor); }, - focusAt, + focusAt: (cursor) => focusAt(cursor), focusAtEnd: () => { focusAt( literalText @@ -1827,7 +1860,7 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { */ function insertMarkdownParagraphs( value: string, - skillLabelFor: (name: string) => SkillMeta, + skillLabelFor: (name: string) => SkillMeta | null, options: { styling: boolean; blocks?: boolean; literalText?: boolean; literalLength?: number }, insertContent: (content: JSONContent[] | JSONContent) => void, ): void { diff --git a/apps/web/src/composer-rich-text-doc.test.ts b/apps/web/src/composer-rich-text-doc.test.ts index 19c6e0299f2e..34bfbef6d405 100644 --- a/apps/web/src/composer-rich-text-doc.test.ts +++ b/apps/web/src/composer-rich-text-doc.test.ts @@ -4,9 +4,14 @@ import { TaskList } from "@tiptap/extension-task-list"; import { Node as ProseMirrorNode } from "@tiptap/pm/model"; import { EditorState, TextSelection } from "@tiptap/pm/state"; import { splitBlockKeepMarks } from "@tiptap/pm/commands"; +import { Transform } from "@tiptap/pm/transform"; import { describe, expect, it } from "vite-plus/test"; -import { collapseExpandedComposerCursor, pastedPathQueryLength } from "./composer-logic"; +import { + collapseExpandedComposerCursor, + expandCollapsedComposerCursor, + pastedPathQueryLength, +} from "./composer-logic"; import { buildDocJson, @@ -15,15 +20,15 @@ import { ComposerBlockExtensions, ComposerCodeBlockExtension, ComposerListExtensions, - collapsedToFlat, ComposerCodeExtension, ComposerTaskItemExtension, - flatToCollapsed, flatToMarkdown, flatToPm, + markdownToFlat, pmToFlat, serializeEditorDoc, serializeSelection, + skillChipReplacements, stepCaretAcrossStyledEdge, } from "./composer-rich-text-doc"; @@ -159,7 +164,9 @@ describe("literal editor answers", () => { expect(serialized.contextIds).toEqual([]); expect(serializeSelection(doc, 1, doc.content.size - 1)).toBe(value); for (let offset = 0; offset <= value.length; offset += 1) { - expect(flatToCollapsed(serialized, offset)).toBe(offset); + expect(collapseExpandedComposerCursor(value, flatToMarkdown(serialized, offset), true)).toBe( + offset, + ); } }); @@ -255,7 +262,7 @@ describe("composer rich text document model", () => { const position = flatToPm(map, flat); expect(doc.resolve(position).parent.isTextblock).toBe(true); expect(pmToFlat(map, position)).toBe(flat); - expect(collapsedToFlat(map, flatToCollapsed(map, flat))).toBe(flat); + expect(markdownToFlat(map, flatToMarkdown(map, flat))).toBe(flat); } }); @@ -366,7 +373,7 @@ describe("composer rich text document model", () => { expect(serializeEditorDoc(rebuilt).value).toBe(map.value); for (let flat = 0; flat <= map.docLength; flat += 1) { expect(pmToFlat(map, flatToPm(map, flat))).toBe(flat); - expect(collapsedToFlat(map, flatToCollapsed(map, flat))).toBe(flat); + expect(markdownToFlat(map, flatToMarkdown(map, flat))).toBe(flat); if (flat < map.docLength && !/\s/.test(doc.textContent[flat]!)) { expect(rebuilt.resolve(flat + 1).nodeAfter!.marks.map((mark) => mark.type.name)).toEqual( doc.resolve(flat + 1).nodeAfter!.marks.map((mark) => mark.type.name), @@ -444,12 +451,12 @@ describe("composer rich text document model", () => { }); it.each(["- one\n- two", "1. a\n - b\n2. c", "- **bold** @README.md tail", "-"])( - "maps every document offset of the list %s through collapsed coordinates and back", + "maps every document offset of the list %s through markdown coordinates and back", (value) => { const map = roundTrip(value); expect(map.value).toBe(value); for (let flat = 0; flat <= map.docLength; flat += 1) { - expect(collapsedToFlat(map, flatToCollapsed(map, flat))).toBe(flat); + expect(markdownToFlat(map, flatToMarkdown(map, flat))).toBe(flat); } }, ); @@ -458,11 +465,11 @@ describe("composer rich text document model", () => { const value = "- item"; const map = roundTrip(value); // The marker owns no document characters, like a checkbox. - for (let collapsed = 0; collapsed <= "- ".length; collapsed += 1) { - expect(collapsedToFlat(map, collapsed)).toBe(0); + for (let markdown = 0; markdown <= "- ".length; markdown += 1) { + expect(markdownToFlat(map, markdown)).toBe(0); } - expect(collapsedToFlat(map, "- it".length)).toBe(2); - expect(flatToCollapsed(map, 0)).toBe("- ".length); + expect(markdownToFlat(map, "- it".length)).toBe(2); + expect(flatToMarkdown(map, 0)).toBe("- ".length); }); it.each([ @@ -625,12 +632,12 @@ describe("composer rich text document model", () => { "# Heading text", "## **b** @README.md", ])( - "maps every document offset of the block %s through collapsed coordinates and back", + "maps every document offset of the block %s through markdown coordinates and back", (value) => { const map = roundTrip(value); expect(map.value).toBe(value); for (let flat = 0; flat <= map.docLength; flat += 1) { - expect(collapsedToFlat(map, flatToCollapsed(map, flat))).toBe(flat); + expect(markdownToFlat(map, flatToMarkdown(map, flat))).toBe(flat); } }, ); @@ -641,10 +648,10 @@ describe("composer rich text document model", () => { ["# Heading", "# "], ] as const) { const map = roundTrip(value); - for (let collapsed = 0; collapsed <= prefix.length; collapsed += 1) { - expect(collapsedToFlat(map, collapsed)).toBe(0); + for (let markdown = 0; markdown <= prefix.length; markdown += 1) { + expect(markdownToFlat(map, markdown)).toBe(0); } - expect(flatToCollapsed(map, 0)).toBe(prefix.length); + expect(flatToMarkdown(map, 0)).toBe(prefix.length); } }); @@ -670,12 +677,12 @@ describe("composer rich text document model", () => { }); it.each(["```ts\nconst a = 1;\n```", "```\n```", "before\n```ts\ncode\n```\nafter"])( - "maps every document offset of %s through collapsed coordinates and back", + "maps every document offset of %s through markdown coordinates and back", (value) => { const map = roundTrip(value); expect(map.value).toBe(value); for (let flat = 0; flat <= map.docLength; flat += 1) { - expect(collapsedToFlat(map, flatToCollapsed(map, flat))).toBe(flat); + expect(markdownToFlat(map, flatToMarkdown(map, flat))).toBe(flat); } }, ); @@ -698,17 +705,16 @@ describe("composer rich text document model", () => { const map = roundTrip(value); const endOfCode = "func();".length; // Not the end of the string: the closing fence is a line of its own. - expect(flatToCollapsed(map, endOfCode)).toBe("```ts\nfunc();".length); expect(flatToMarkdown(map, endOfCode)).toBe("```ts\nfunc();".length); - expect(collapsedToFlat(map, flatToCollapsed(map, endOfCode))).toBe(endOfCode); + expect(markdownToFlat(map, flatToMarkdown(map, endOfCode))).toBe(endOfCode); }); it("keeps the caret inside an empty fence", () => { const value = "before\n```\n```"; const map = roundTrip(value); const inside = "before\n".length; - expect(flatToCollapsed(map, inside)).toBe("before\n```".length); - expect(collapsedToFlat(map, flatToCollapsed(map, inside))).toBe(inside); + expect(flatToMarkdown(map, inside)).toBe("before\n```".length); + expect(markdownToFlat(map, flatToMarkdown(map, inside))).toBe(inside); }); it("still places the end of an inline mark after its markers", () => { @@ -724,13 +730,13 @@ describe("composer rich text document model", () => { expect(map.value).toBe(value); // The opening fence owns no document characters, so every offset in it // lands on the first character of the code. - for (let collapsed = 0; collapsed <= "```ts\n".length; collapsed += 1) { - expect(collapsedToFlat(map, collapsed)).toBe(0); + for (let markdown = 0; markdown <= "```ts\n".length; markdown += 1) { + expect(markdownToFlat(map, markdown)).toBe(0); } - expect(collapsedToFlat(map, "```ts\na".length)).toBe(1); + expect(markdownToFlat(map, "```ts\na".length)).toBe(1); // Everything from the closing newline onwards clamps to the code's end. - for (let collapsed = "```ts\nab".length; collapsed <= value.length; collapsed += 1) { - expect(collapsedToFlat(map, collapsed)).toBe(2); + for (let markdown = "```ts\nab".length; markdown <= value.length; markdown += 1) { + expect(markdownToFlat(map, markdown)).toBe(2); } }); @@ -813,19 +819,18 @@ describe("composer rich text document model", () => { expect(roundTrip("```\ncode").value).toBe("```\ncode"); }); - it("counts a chip's source in a fence as one cursor position, as the draft store does", () => { - const value = "```\nsee @b.md now\n```\nafter"; - const map = roundTrip(value); + it("keeps a chip's source in a fence as literal text", () => { + const value = "```\nsee @b.md and $babysit now\n```\nafter"; + const known = () => ({ label: "Babysit", description: null }); + const doc = ProseMirrorNode.fromJSON(schema, buildDocJson(value, known)); + const map = serializeEditorDoc(doc); expect(map.value).toBe(value); + expect(map.runs.filter((run) => run.kind === "token")).toEqual([]); for (let flat = 0; flat <= map.docLength; flat += 1) { - expect(flatToCollapsed(map, flat)).toBe( - collapseExpandedComposerCursor(value, flatToMarkdown(map, flat)), - ); + expect(markdownToFlat(map, flatToMarkdown(map, flat))).toBe(flat); } - const afterChip = "see @b.md".length; - expect(collapsedToFlat(map, flatToCollapsed(map, afterChip))).toBe(afterChip); - const end = map.docLength; - expect(collapsedToFlat(map, flatToCollapsed(map, end))).toBe(end); + // Reconciling against the provider's skills leaves the code alone too. + expect(skillChipReplacements(doc, known)).toEqual([]); }); it("keeps fences literal in plain mode", () => { @@ -833,12 +838,12 @@ describe("composer rich text document model", () => { expect(roundTripPlain(value).value).toBe(value); }); - it("maps every document offset through collapsed coordinates and back", () => { + it("maps every document offset through markdown coordinates and back", () => { const value = "hi **bold** @README.md bye"; const map = roundTrip(value); expect(map.value).toBe(value); for (let flat = 0; flat <= map.docLength; flat += 1) { - expect(collapsedToFlat(map, flatToCollapsed(map, flat))).toBe(flat); + expect(markdownToFlat(map, flatToMarkdown(map, flat))).toBe(flat); } }); @@ -856,8 +861,9 @@ describe("composer rich text document model", () => { expect(rich.value).toBe(value); expect(plain.value).toBe(value); for (let collapsed = 0; collapsed <= value.length; collapsed += 1) { - expect(flatToMarkdown(rich, collapsedToFlat(rich, collapsed))).toBe( - flatToMarkdown(plain, collapsedToFlat(plain, collapsed)), + const markdown = expandCollapsedComposerCursor(value, collapsed); + expect(flatToMarkdown(rich, markdownToFlat(rich, markdown))).toBe( + flatToMarkdown(plain, markdownToFlat(plain, markdown)), ); } }); @@ -869,8 +875,8 @@ describe("composer rich text document model", () => { // Between the two asterisks: a real caret position in plain mode, and no // position at all in rich mode, where it lands on the first styled // character instead. The flip moves the caret by a marker's width at most. - expect(flatToMarkdown(plain, collapsedToFlat(plain, 3))).toBe(3); - expect(flatToMarkdown(rich, collapsedToFlat(rich, 3))).toBe(4); + expect(flatToMarkdown(plain, markdownToFlat(plain, 3))).toBe(3); + expect(flatToMarkdown(rich, markdownToFlat(rich, 3))).toBe(4); }); it("maps markdown offsets at styled edges onto document text", () => { @@ -880,8 +886,67 @@ describe("composer rich text document model", () => { // document text is "a bold c" (flat), markdown has the markers. expect(flatToMarkdown(map, 2)).toBe(4); expect(flatToMarkdown(map, 6)).toBe(10); - expect(collapsedToFlat(map, 3)).toBe(2); - expect(collapsedToFlat(map, 9)).toBe(6); + expect(markdownToFlat(map, 3)).toBe(2); + expect(markdownToFlat(map, 9)).toBe(6); + }); + + it("chips only the skills the provider has and leaves other $names as text", () => { + const value = "run $babysit then echo $HOME and $notaskill done"; + const known = (name: string) => + name === "babysit" ? { label: "Babysit", description: null } : null; + const doc = ProseMirrorNode.fromJSON(schema, buildDocJson(value, known)); + doc.check(); + const map = serializeEditorDoc(doc); + expect(map.value).toBe(value); + expect(map.runs.filter((run) => run.kind === "token").map((run) => run.nodeName)).toEqual([ + "composer-skill", + ]); + expect(doc.textContent).toBe("run then echo $HOME and $notaskill done"); + // A caret after `$HOME` sits after the same five characters in the document. + const afterHome = value.indexOf("$HOME") + "$HOME".length; + const flat = markdownToFlat(map, afterHome); + expect(doc.textBetween(flatToPm(map, flat) - 5, flatToPm(map, flat))).toBe("$HOME"); + // Any offset inside the chip source lands after the chip. + expect(markdownToFlat(map, value.indexOf("$babysit") + 3)).toBe("run ".length + 1); + }); + + it("re-chips for a new skill list by replacing only the affected nodes", () => { + const value = "run $babysit then **echo $HOME now** and $deploy done"; + const metaFor = (known: ReadonlyArray) => (name: string) => + known.includes(name) ? { label: name, description: null } : null; + const chipNames = (doc: ProseMirrorNode) => { + const names: string[] = []; + doc.descendants((node) => { + if (node.type.name === "composer-skill") names.push(node.attrs.skillName); + }); + return names; + }; + const reconcile = (doc: ProseMirrorNode, known: ReadonlyArray) => { + const transform = new Transform(doc); + for (const { from, to, node } of skillChipReplacements(doc, metaFor(known))) { + transform.replaceWith(from, to, node); + } + transform.doc.check(); + return transform.doc; + }; + + const codex = ProseMirrorNode.fromJSON(schema, buildDocJson(value, metaFor(["babysit"]))); + expect(chipNames(codex)).toEqual(["babysit"]); + expect(skillChipReplacements(codex, metaFor(["babysit"]))).toEqual([]); + + // The next provider has $deploy and $HOME but not $babysit. + const claude = reconcile(codex, ["deploy", "HOME"]); + expect(chipNames(claude)).toEqual(["HOME", "deploy"]); + expect(serializeEditorDoc(claude).value).toBe(value); + // The new chip keeps the formatting of the text it replaced. + const home = claude.nodeAt( + serializeEditorDoc(claude).runs.find((run) => run.kind === "token")!.pmPos, + )!; + expect(home.marks.map((mark) => mark.type.name)).toEqual(["bold"]); + + const none = reconcile(claude, []); + expect(chipNames(none)).toEqual([]); + expect(serializeEditorDoc(none).value).toBe(value); }); }); diff --git a/apps/web/src/composer-rich-text-doc.ts b/apps/web/src/composer-rich-text-doc.ts index 8c376fb5d93f..98bfe0e0f3c3 100644 --- a/apps/web/src/composer-rich-text-doc.ts +++ b/apps/web/src/composer-rich-text-doc.ts @@ -27,14 +27,16 @@ import { collectInlineContextIds } from "~/lib/composerContextReferences"; * * The stored prompt stays markdown (`**bold**`, `@file` chips as canonical * links). The Tiptap document holds styled text plus inline atom chips, so - * this module translates both ways and maps cursor offsets between the three - * coordinate spaces the composer speaks: + * this module translates both ways and maps cursor offsets between the two + * coordinate spaces of the document: * * - flat document offsets (styled markers excluded, chips count 1), - * - collapsed cursor offsets (markers literal, chips count 1 — the coordinate - * the draft store and mention detection use), * - markdown offsets (markers literal, chips expand to their source). * + * The collapsed cursor the draft store uses is derived from the markdown text + * alone (see composer-logic), so it never depends on which tokens the + * document happens to render as chips. + * * DOM-free on purpose: unit tests build a real ProseMirror document from the * JSON this produces and assert the round trip without a browser. */ @@ -438,10 +440,11 @@ interface DocLine { inline: InlineJson[]; } +/** Null for a `$name` that is not one of the provider's skills: it stays editable text. */ function atomJsonForSegment( segment: Exclude[number], { type: "text" }>, - skillLabelFor: (name: string) => SkillMeta, -): InlineJson { + skillLabelFor: (name: string) => SkillMeta | null, +): InlineJson | null { if (segment.type === "mention") { return { type: "composer-mention", @@ -450,6 +453,7 @@ function atomJsonForSegment( } if (segment.type === "skill") { const meta = skillLabelFor(segment.name); + if (!meta) return null; return { type: "composer-skill", attrs: { @@ -541,7 +545,7 @@ function textJsonForSpan(text: string, marks: RichTextMark[]): Record SkillMeta, + skillLabelFor: (name: string) => SkillMeta | null, options?: { styling?: boolean; blocks?: boolean; literalText?: boolean; literalLength?: number }, ): Record[] { // Editor answers are verbatim text, including Markdown and context-token sources. @@ -569,7 +573,9 @@ export function buildTiptapContent( const text = splitPromptIntoComposerSegments(value.slice(literal.length)) .map((segment) => { if (segment.type === "text") return segment.text; - atoms.push(atomJsonForSegment(segment, skillLabelFor)); + const atom = atomJsonForSegment(segment, skillLabelFor); + if (!atom) return segment.source; + atoms.push(atom); atomSources.push(segment.source); return sentinel; }) @@ -779,7 +785,7 @@ export function buildTiptapContent( export function buildDocJson( value: string, - skillLabelFor: (name: string) => SkillMeta, + skillLabelFor: (name: string) => SkillMeta | null, options?: { styling?: boolean; literalText?: boolean }, ) { const content = buildTiptapContent(value, skillLabelFor, options); @@ -793,8 +799,6 @@ export interface RichRun { /** Flat document offset (atoms count 1, markers excluded). */ flatStart: number; docLen: number; - /** Collapsed cursor length (markers literal, tokens count 1). */ - collapsedLen: number; /** Markdown length (tokens expand to their source). */ mdLen: number; /** Marker layout inside text runs. */ @@ -803,7 +807,6 @@ export interface RichRun { /** ProseMirror position of the run start. */ pmPos: number; mdStart: number; - collapsedStart: number; nodeName?: string; } @@ -834,7 +837,6 @@ interface RichAccumulator { runs: RichRun[]; value: string; flat: number; - collapsed: number; md: number; } @@ -846,17 +848,14 @@ function pushBreakRun(acc: RichAccumulator, position?: number): void { kind: "break", flatStart: acc.flat, docLen: 1, - collapsedLen: 1, mdLen: 1, openLen: 0, closeLen: 0, pmPos, mdStart: acc.md, - collapsedStart: acc.collapsed, }); acc.value += "\n"; acc.flat += 1; - acc.collapsed += 1; acc.md += 1; } @@ -969,23 +968,19 @@ function appendInlineRuns( const source = child.isText ? child.text! : readAtomSource(child); const docLen = child.isText ? source.length : 1; const mdText = open + source + close; - const collapsedLen = open.length + docLen + close.length; acc.runs.push({ kind: child.isText ? "text" : "token", flatStart: acc.flat, docLen, - collapsedLen, mdLen: mdText.length, openLen: open.length, closeLen: close.length, pmPos, mdStart: acc.md, - collapsedStart: acc.collapsed, ...(child.isText ? {} : { nodeName: child.type.name }), }); acc.value += mdText; acc.flat += docLen; - acc.collapsed += collapsedLen; acc.md += mdText.length; }); // Empty paragraphs have an editable position even though they emit no text. @@ -994,13 +989,11 @@ function appendInlineRuns( kind: "text", flatStart: acc.flat, docLen: 0, - collapsedLen: 0, mdLen: 0, openLen: 0, closeLen: 0, pmPos: contentStart, mdStart: acc.md, - collapsedStart: acc.collapsed, }); } } @@ -1225,16 +1218,13 @@ function walkList(list: ProseMirrorNode, listStart: number, acc: RichAccumulator kind: "prefix", flatStart: acc.flat, docLen: 0, - collapsedLen: prefix.length, mdLen: prefix.length, openLen: 0, closeLen: 0, pmPos: itemContentStart + 1, mdStart: acc.md, - collapsedStart: acc.collapsed, }); acc.value += prefix; - acc.collapsed += prefix.length; acc.md += prefix.length; let childPos = itemContentStart; let firstBlock = true; @@ -1257,11 +1247,6 @@ function walkList(list: ProseMirrorNode, listStart: number, acc: RichAccumulator * every cursor rule that already clamps out of a style marker clamps out of a * fence too. `nodeName` keeps the marker decorations off it: a code block is * drawn as a block, not revealed a character at a time. - * - * The code is literal text in the editor, but the draft store still counts a - * chip's source in it as one cursor position, as it does everywhere. So the - * run splits at each chip source, which spans its characters in the document - * and one position in the collapsed space. */ function appendCodeBlockRun( block: ProseMirrorNode, @@ -1270,37 +1255,20 @@ function appendCodeBlockRun( acc: RichAccumulator, ): void { const { open, content, close } = codeBlockSource(block, followed); - const pieces = content - ? splitPromptIntoComposerSegments(content).map((segment) => - segment.type === "text" - ? { length: segment.text.length, collapsedLen: segment.text.length } - : { length: segment.source.length, collapsedLen: 1 }, - ) - : [{ length: 0, collapsedLen: 0 }]; - let offset = 0; - pieces.forEach((piece, index) => { - const openLen = index === 0 ? open.length : 0; - const closeLen = index === pieces.length - 1 ? close.length : 0; - const collapsedLen = openLen + piece.collapsedLen + closeLen; - const mdLen = openLen + piece.length + closeLen; - acc.runs.push({ - kind: "text", - flatStart: acc.flat, - docLen: piece.length, - collapsedLen, - mdLen, - openLen, - closeLen, - pmPos: pmPos + offset, - mdStart: acc.md, - collapsedStart: acc.collapsed, - nodeName: "codeBlock", - }); - offset += piece.length; - acc.flat += piece.length; - acc.collapsed += collapsedLen; - acc.md += mdLen; + const mdLen = open.length + content.length + close.length; + acc.runs.push({ + kind: "text", + flatStart: acc.flat, + docLen: content.length, + mdLen, + openLen: open.length, + closeLen: close.length, + pmPos, + mdStart: acc.md, + nodeName: "codeBlock", }); + acc.flat += content.length; + acc.md += mdLen; acc.value += open + content + close; } @@ -1317,16 +1285,13 @@ function walkBlockquote(quote: ProseMirrorNode, quoteStart: number, acc: RichAcc kind: "prefix", flatStart: acc.flat, docLen: 0, - collapsedLen: prefix.length, mdLen: prefix.length, openLen: 0, closeLen: 0, pmPos: childPos + 1, mdStart: acc.md, - collapsedStart: acc.collapsed, }); acc.value += prefix; - acc.collapsed += prefix.length; acc.md += prefix.length; if (child.type.name === "paragraph") appendInlineRuns(child, childPos + 1, acc); childPos += child.nodeSize; @@ -1358,7 +1323,7 @@ export function serializeSelection(doc: ProseMirrorNode, from: number, to: numbe } export function serializeEditorDoc(doc: ProseMirrorNode): RichDocMap { - const acc: RichAccumulator = { runs: [], value: "", flat: 0, collapsed: 0, md: 0 }; + const acc: RichAccumulator = { runs: [], value: "", flat: 0, md: 0 }; const blocks: ProseMirrorNode[] = []; doc.content.forEach((node) => { blocks.push(node); @@ -1382,16 +1347,13 @@ export function serializeEditorDoc(doc: ProseMirrorNode): RichDocMap { kind: "prefix", flatStart: acc.flat, docLen: 0, - collapsedLen: prefix.length, mdLen: prefix.length, openLen: 0, closeLen: 0, pmPos: pmBlockStart + 1, mdStart: acc.md, - collapsedStart: acc.collapsed, }); acc.value += prefix; - acc.collapsed += prefix.length; acc.md += prefix.length; appendInlineRuns(block, pmBlockStart + 1, acc); } else if (block.type.name === "horizontalRule") { @@ -1401,16 +1363,13 @@ export function serializeEditorDoc(doc: ProseMirrorNode): RichDocMap { kind: "prefix", flatStart: acc.flat, docLen: 0, - collapsedLen: source.length, mdLen: source.length, openLen: 0, closeLen: 0, pmPos: pmBlockStart + block.nodeSize, mdStart: acc.md, - collapsedStart: acc.collapsed, }); acc.value += source; - acc.collapsed += source.length; acc.md += source.length; } else if (block.type.name === "paragraph") { appendInlineRuns(block, pmBlockStart + 1, acc); @@ -1426,12 +1385,68 @@ export function serializeEditorDoc(doc: ProseMirrorNode): RichDocMap { }; } -function lastRunEnd(map: RichDocMap, space: "collapsed" | "md"): number { - const last = map.runs[map.runs.length - 1]; - if (!last) return 0; - return space === "collapsed" - ? last.collapsedStart + last.collapsedLen - : last.mdStart + last.mdLen; +export interface SkillChipReplacement { + from: number; + to: number; + node: ProseMirrorNode; +} + +/** + * The edits that bring a document's skill chips in line with the provider's + * skills: a chip for a skill the provider lacks becomes its `$name` text, and + * `$name` text for a skill it has becomes a chip. The skill list loads after + * the editor opens and differs per provider, so the same prompt can need + * different chips without its text changing. Ordered last-first, so applying + * them in order keeps every position valid. + */ +export function skillChipReplacements( + doc: ProseMirrorNode, + skillLabelFor: (name: string) => SkillMeta | null, +): SkillChipReplacement[] { + const schema = doc.type.schema; + const skillType = schema.nodes["composer-skill"]; + if (!skillType) return []; + const replacements: SkillChipReplacement[] = []; + doc.descendants((node, pos) => { + if (node.type !== skillType) return; + const name = typeof node.attrs.skillName === "string" ? node.attrs.skillName : ""; + if (name && !skillLabelFor(name)) { + replacements.push({ + from: pos, + to: pos + node.nodeSize, + node: schema.text(readAtomSource(node), node.marks), + }); + } + }); + const map = serializeEditorDoc(doc); + let end = 0; + for (const segment of splitPromptIntoComposerSegments(map.value)) { + const start = end; + end += segment.type === "text" ? segment.text.length : segment.source.length; + if (segment.type !== "skill") continue; + const meta = skillLabelFor(segment.name); + if (!meta) continue; + // Only text needs converting; an existing chip is a token run. + const run = map.runs.find( + (candidate) => + candidate.kind === "text" && + candidate.nodeName !== "codeBlock" && + start >= candidate.mdStart + candidate.openLen && + end <= candidate.mdStart + candidate.openLen + candidate.docLen, + ); + if (!run) continue; + const from = run.pmPos + (start - run.mdStart - run.openLen); + replacements.push({ + from, + to: from + segment.source.length, + node: skillType.create( + { skillName: segment.name, skillLabel: meta.label, skillDescription: meta.description }, + null, + doc.nodeAt(from)?.marks, + ), + }); + } + return replacements.sort((left, right) => right.from - left.from); } /** @@ -1445,24 +1460,6 @@ function runOwnsOffset(run: RichRun, flatOffset: number): boolean { return flatOffset < end || (flatOffset === end && run.nodeName === "codeBlock"); } -export function flatToCollapsed(map: RichDocMap, flatOffset: number): number { - const bounded = Math.max(0, Math.min(flatOffset, map.docLength)); - for (const run of map.runs) { - if (runOwnsOffset(run, bounded)) { - if (run.kind === "text" || run.kind === "token") { - // A chip's source in a fence spans its characters but one position. - const within = Math.min( - bounded - run.flatStart, - run.collapsedLen - run.openLen - run.closeLen, - ); - return run.collapsedStart + run.openLen + within; - } - return run.collapsedStart + (bounded - run.flatStart); - } - } - return lastRunEnd(map, "collapsed"); -} - export function flatToMarkdown(map: RichDocMap, flatOffset: number): number { const bounded = Math.max(0, Math.min(flatOffset, map.docLength)); for (const run of map.runs) { @@ -1473,24 +1470,27 @@ export function flatToMarkdown(map: RichDocMap, flatOffset: number): number { return run.mdStart + (bounded - run.flatStart); } } - return lastRunEnd(map, "md"); + const last = map.runs[map.runs.length - 1]; + return last ? last.mdStart + last.mdLen : 0; } -export function collapsedToFlat(map: RichDocMap, collapsedOffset: number): number { +export function markdownToFlat(map: RichDocMap, markdownOffset: number): number { for (const run of map.runs) { - if (collapsedOffset < run.collapsedStart + run.collapsedLen) { + if (markdownOffset < run.mdStart + run.mdLen) { // Checkbox prefixes and style markers are shown, never edited: every // offset inside them clamps to the adjacent document position. if (run.kind === "prefix") return run.flatStart; if (run.kind === "text" || run.kind === "token") { - const within = collapsedOffset - run.collapsedStart; - const contentLen = run.collapsedLen - run.openLen - run.closeLen; + const within = markdownOffset - run.mdStart; // Marker characters clamp to the styled edge: they are shown, never edited. if (within <= run.openLen) return run.flatStart; - if (within >= run.openLen + contentLen) return run.flatStart + run.docLen; + // A chip is one document position however long its source is. + if (run.kind === "token" || within >= run.openLen + run.docLen) { + return run.flatStart + run.docLen; + } return run.flatStart + (within - run.openLen); } - return run.flatStart + (collapsedOffset - run.collapsedStart); + return run.flatStart + (markdownOffset - run.mdStart); } } return map.docLength;