From 77c3cd3a628ac6f44ec4d1585048f3b30f6e33e0 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 3 Sep 2026 22:58:50 -0700 Subject: [PATCH 1/2] fix(chat): render Codex source citations --- .../src/features/threads/ThreadFeed.tsx | 12 +- apps/web/src/components/ChatMarkdown.tsx | 47 +-- .../src/components/chat/CodexCitationChip.tsx | 61 +++ .../chat/MessagesTimeline.logic.test.ts | 53 +++ .../components/chat/MessagesTimeline.logic.ts | 7 +- apps/web/src/markdown-clipboard.test.ts | 11 + apps/web/src/markdown-clipboard.ts | 3 + apps/web/src/markdown-list-indentation.ts | 144 +------- docs/internals/codex-source-citations.md | 103 ++++++ docs/user/providers-codex.md | 9 + packages/client-runtime/package.json | 9 + .../client-runtime/src/codexCitations.test.ts | 349 ++++++++++++++++++ packages/client-runtime/src/codexCitations.ts | 347 +++++++++++++++++ .../src/markdownListIndentation.test.ts | 145 ++++++++ .../src/markdownListIndentation.ts | 215 +++++++++++ pnpm-lock.yaml | 3 + 16 files changed, 1349 insertions(+), 169 deletions(-) create mode 100644 apps/web/src/components/chat/CodexCitationChip.tsx create mode 100644 docs/internals/codex-source-citations.md create mode 100644 packages/client-runtime/src/codexCitations.test.ts create mode 100644 packages/client-runtime/src/codexCitations.ts create mode 100644 packages/client-runtime/src/markdownListIndentation.test.ts create mode 100644 packages/client-runtime/src/markdownListIndentation.ts diff --git a/apps/mobile/src/features/threads/ThreadFeed.tsx b/apps/mobile/src/features/threads/ThreadFeed.tsx index ab48046fbd96..6533bb170dae 100644 --- a/apps/mobile/src/features/threads/ThreadFeed.tsx +++ b/apps/mobile/src/features/threads/ThreadFeed.tsx @@ -15,6 +15,7 @@ import { codexArtifactTemplatePresentationLabel, type CodexArtifactTemplate, } from "@t3tools/client-runtime/codex-artifact-templates"; +import { renderCodexCitationsAsMarkdown } from "@t3tools/client-runtime/codex-citations"; import { resolveAssetUrl } from "@t3tools/client-runtime/state/assets"; import { formatAttachmentSize } from "@t3tools/client-runtime/state/attachments"; import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; @@ -726,6 +727,7 @@ interface MarkdownLinkHandlers { const AssistantMarkdownContent = memo(function AssistantMarkdownContent(props: { readonly markdown: string; + readonly isStreaming: boolean; readonly markdownStyles: MarkdownStyleSet; readonly linkHandlers: MarkdownLinkHandlers; readonly onUseArtifactTemplate?: ((template: CodexArtifactTemplate) => void) | undefined; @@ -733,8 +735,11 @@ const AssistantMarkdownContent = memo(function AssistantMarkdownContent(props: { readonly skills?: ReadonlyArray | undefined; }) { const segments = useMemo( - () => splitCodexArtifactTemplateMarkdown(props.markdown), - [props.markdown], + () => + splitCodexArtifactTemplateMarkdown( + renderCodexCitationsAsMarkdown(props.markdown, { isStreaming: props.isStreaming }), + ), + [props.markdown, props.isStreaming], ); return segments.map((segment) => { @@ -1514,6 +1519,7 @@ function renderFeedEntry( {renderedText.trim().length > 0 ? ( [0]; -const CHAT_MARKDOWN_REMARK_PLUGINS = [ - remarkGfm, - remarkGithubAlerts, - remarkNormalizeListItemIndentation, - remarkCodexDirectives, - remarkPreserveCodeMeta, - remarkNormalizeLinksAndTagInlineCode, -] satisfies NonNullable; - -const CHAT_MARKDOWN_REMARK_PLUGINS_WITH_BREAKS = [ - remarkGfm, - remarkGithubAlerts, - remarkNormalizeListItemIndentation, - remarkCodexDirectives, - remarkBreaks, - remarkPreserveCodeMeta, - remarkNormalizeLinksAndTagInlineCode, -] satisfies NonNullable; - const CHAT_MARKDOWN_REHYPE_PLUGINS = [ rehypeRaw, rehypePreserveImageSourceMeta, @@ -2342,6 +2330,14 @@ function ChatMarkdown({ }; return { + span({ node, children, ...props }) { + const citation = codexCitationFromHastProperties(node?.properties); + return citation ? ( + + ) : ( + {children} + ); + }, div({ node, children, ...props }) { const artifactTemplate = artifactTemplateFromHastProperties(node?.properties); if (artifactTemplate) { @@ -2766,12 +2762,19 @@ function ChatMarkdown({ ]); /* eslint-enable react/no-unstable-nested-components */ - const remarkPlugins = useMemo( + const remarkPlugins = useMemo>( () => [ - ...(lineBreaks ? CHAT_MARKDOWN_REMARK_PLUGINS_WITH_BREAKS : CHAT_MARKDOWN_REMARK_PLUGINS), + remarkGfm, + remarkGithubAlerts, + remarkNormalizeListItemIndentation, + remarkCodexDirectives, + [remarkCodexCitations, { isStreaming }], + ...(lineBreaks ? [remarkBreaks] : []), + remarkPreserveCodeMeta, + remarkNormalizeLinksAndTagInlineCode, ...extraRemarkPlugins, ], - [extraRemarkPlugins, lineBreaks], + [extraRemarkPlugins, isStreaming, lineBreaks], ); // react-markdown converts unparsed HTML nodes to text when skipHtml is false. diff --git a/apps/web/src/components/chat/CodexCitationChip.tsx b/apps/web/src/components/chat/CodexCitationChip.tsx new file mode 100644 index 000000000000..0c864cef9226 --- /dev/null +++ b/apps/web/src/components/chat/CodexCitationChip.tsx @@ -0,0 +1,61 @@ +import { + codexCitationMarkdown, + codexCitationText, + type CodexCitation, +} from "@t3tools/client-runtime/codex-citations"; +import { cn } from "~/lib/utils"; +import { CHAT_INLINE_CHIP_CLASS_NAME } from "../composerInlineChip"; +import { + Popover, + PopoverDescription, + PopoverPopup, + PopoverTitle, + PopoverTrigger, +} from "../ui/popover"; + +/** A source reference stays inspectable even when Codex does not supply its destination. */ +export function CodexCitationChip({ citation }: { citation: CodexCitation }) { + const sourceNumbers = citation.sources.map((source) => source.number).join(", "); + const label = `${citation.sources.length === 1 ? "Source" : "Sources"} ${sourceNumbers}`; + + return ( + + + + {sourceNumbers} + + + {label} +
+ {citation.sources.map((source) => ( +
+
Source {source.number}
+
{source.id}
+
+ ))} + {citation.locator ? ( +
+
Lines
+
{citation.locator}
+
+ ) : null} +
+ + Source URL unavailable. Codex supplied source IDs without destination links. + + + + + ); +} diff --git a/apps/web/src/components/chat/MessagesTimeline.logic.test.ts b/apps/web/src/components/chat/MessagesTimeline.logic.test.ts index 67dea03e44ea..085002fefc9b 100644 --- a/apps/web/src/components/chat/MessagesTimeline.logic.test.ts +++ b/apps/web/src/components/chat/MessagesTimeline.logic.test.ts @@ -470,6 +470,59 @@ describe("resolveAssistantMessageCopyState", () => { visible: true, }); }); + + it("copies readable source references while preserving citation examples in code", () => { + const citation = "\uE200cite\uE202turn0view0\uE201"; + expect( + resolveAssistantMessageCopyState({ + showCopyButton: true, + text: `The report supports this. ${citation}\n\nExample: \`${citation}\``, + streaming: false, + }), + ).toEqual({ + text: `The report supports this. \\[Source 1: turn0view0\\]\n\nExample: \`${citation}\``, + visible: true, + }); + }); + + it("keeps artifact names literal without shifting copied source numbers", () => { + const artifactName = "\uE200cite\uE202turn0view0\uE201"; + const citation = "\uE200cite\uE202turn1search2\uE201"; + expect( + resolveAssistantMessageCopyState({ + showCopyButton: true, + text: [ + `::artifact-template{skill_name="artifact-template-hello-world" skill_directory="/Users/test/.codex/skills/artifact-template-hello-world" display_name="${artifactName}" artifact_kind="document"}`, + "", + `The report supports this. ${citation}`, + ].join("\n"), + streaming: false, + }), + ).toEqual({ + text: `${artifactName} (Document template)\n\nThe report supports this. \\[Source 1: turn1search2\\]`, + visible: true, + }); + }); + + it("does not rewrite an unfinished response for a hidden copy button", () => { + const text = "The report supports this. \uE200cite\uE202turn0view0"; + expect( + resolveAssistantMessageCopyState({ showCopyButton: true, text, streaming: true }), + ).toEqual({ text, visible: false }); + }); + + it("keeps citation numbering consistent when copying recovered list content", () => { + expect( + resolveAssistantMessageCopyState({ + showCopyButton: true, + text: "- First. \uE200cite\uE202turn0view0\uE201\n\nSecond. \uE200cite\uE202turn1view0\uE201", + streaming: false, + }), + ).toEqual({ + text: "- First. \\[Source 1: turn0view0\\]\n\nSecond. \\[Source 2: turn1view0\\]", + visible: true, + }); + }); }); describe("workEntryIsVisibleInGroup", () => { diff --git a/apps/web/src/components/chat/MessagesTimeline.logic.ts b/apps/web/src/components/chat/MessagesTimeline.logic.ts index f33e847adc5d..399b56aec025 100644 --- a/apps/web/src/components/chat/MessagesTimeline.logic.ts +++ b/apps/web/src/components/chat/MessagesTimeline.logic.ts @@ -1,5 +1,6 @@ import * as Equal from "effect/Equal"; import { renderCodexDirectivesForCopy } from "@t3tools/client-runtime/codex-markdown-directives"; +import { renderCodexCitationsAsMarkdown } from "@t3tools/client-runtime/codex-citations"; import { commandProgramName } from "@t3tools/client-runtime/work-log/command-label"; import { liveActivityToolStatus, @@ -435,7 +436,11 @@ export function resolveAssistantMessageCopyState({ const hasText = text !== null && text.trim().length > 0; const visible = showCopyButton && hasText && !streaming; return { - text: hasText ? (visible ? renderCodexDirectivesForCopy(text) : text) : null, + text: hasText + ? visible + ? renderCodexDirectivesForCopy(renderCodexCitationsAsMarkdown(text)) + : text + : null, visible, }; } diff --git a/apps/web/src/markdown-clipboard.test.ts b/apps/web/src/markdown-clipboard.test.ts index ec44fcc22292..390ab63a9f6e 100644 --- a/apps/web/src/markdown-clipboard.test.ts +++ b/apps/web/src/markdown-clipboard.test.ts @@ -154,6 +154,17 @@ describe("serializeRenderedMarkdownFragment", () => { expect(collectAssistantCitations(copied).map((entry) => entry.citation)).toEqual([citation]); }); + it("keeps source references when copying a paragraph containing numbered citation controls", () => { + const chip = new FakeElement("SPAN", [], { + "data-markdown-copy": "\\[Source 1: turn0view0\\]", + }).append(new FakeElement("BUTTON").append(new FakeText("1"))); + const paragraph = new FakeElement("P").append(new FakeText("The report supports this. "), chip); + + expect(serializeRenderedMarkdownFragment(asNode(paragraph))).toBe( + "The report supports this. \\[Source 1: turn0view0\\]", + ); + }); + it("keeps a highlighted block code selection plain when its pre wrapper is outside the range", () => { const code = new FakeElement("CODE").append( shikiCodeLine("git show-ref --verify refs/remotes/origin/opt/deploy/dev"), diff --git a/apps/web/src/markdown-clipboard.ts b/apps/web/src/markdown-clipboard.ts index 4a96c8b31d13..429e501a13c5 100644 --- a/apps/web/src/markdown-clipboard.ts +++ b/apps/web/src/markdown-clipboard.ts @@ -363,6 +363,9 @@ export function serializeTableElementToCsv(table: Element): string { } function sanitizedHtmlFrom(container: Element): string { + for (const node of container.querySelectorAll("[data-markdown-copy-text]")) { + node.textContent = node.getAttribute("data-markdown-copy-text"); + } for (const node of container.querySelectorAll(SANITIZED_HTML_SELECTOR)) { if ( node.classList.contains("chat-markdown-file-link") || diff --git a/apps/web/src/markdown-list-indentation.ts b/apps/web/src/markdown-list-indentation.ts index bd1728e4d932..f57220c29e2c 100644 --- a/apps/web/src/markdown-list-indentation.ts +++ b/apps/web/src/markdown-list-indentation.ts @@ -1,143 +1 @@ -interface MarkdownPosition { - readonly start?: { - readonly line?: number; - readonly offset?: number; - }; -} - -interface MarkdownAstNode { - readonly type: string; - readonly value?: unknown; - readonly position?: MarkdownPosition; - children?: MarkdownAstNode[]; -} - -interface MarkdownFile { - readonly value?: unknown; -} - -interface MarkdownParser { - parse(markdown: string): unknown; -} - -interface RecoveredMarkdown { - readonly blocks: MarkdownAstNode[]; - readonly source: string; -} - -const INLINE_PARSE_PREFIX = "t3-markdown-inline-prefix:"; - -function isSameLineOverIndentedCode( - node: MarkdownAstNode, - parent: MarkdownAstNode | undefined, - markdown: string, -): boolean { - if ( - node.type !== "code" || - parent?.type !== "listItem" || - typeof node.value !== "string" || - !/^[\t ]/.test(node.value) - ) { - return false; - } - - const nodeStart = node.position?.start; - const parentStart = parent.position?.start; - if ( - nodeStart?.line === undefined || - nodeStart.offset === undefined || - parentStart?.line === undefined || - nodeStart.line !== parentStart.line - ) { - return false; - } - - const sourceCharacter = markdown[nodeStart.offset]; - return sourceCharacter !== "`" && sourceCharacter !== "~"; -} - -function parseRecoveredMarkdown(value: string, parser: MarkdownParser): RecoveredMarkdown { - // A text prefix forces block-looking input into a paragraph while preserving - // the processor's configured inline extensions (for example, GFM syntax). - // Later root children are kept as blocks so blank-line-separated content is - // never discarded. - const source = `${INLINE_PARSE_PREFIX}${value}`; - const document = parser.parse(source) as MarkdownAstNode; - const blocks = document.children; - const paragraph = blocks?.[0]; - const children = paragraph?.type === "paragraph" ? paragraph.children : undefined; - const first = children?.[0]; - if ( - !blocks || - !children || - first?.type !== "text" || - typeof first.value !== "string" || - !first.value.startsWith(INLINE_PARSE_PREFIX) - ) { - return { blocks: [{ type: "text", value }], source }; - } - - const firstValue = first.value.slice(INLINE_PARSE_PREFIX.length); - return { - blocks: [ - { - ...paragraph, - type: "paragraph", - children: [...(firstValue ? [{ ...first, value: firstValue }] : []), ...children.slice(1)], - }, - ...blocks.slice(1), - ], - source, - }; -} - -function blocksFromIndentedCode(node: MarkdownAstNode, parser: MarkdownParser): RecoveredMarkdown { - const value = typeof node.value === "string" ? node.value.trim() : ""; - const recovered = parseRecoveredMarkdown(value, parser); - const first = recovered.blocks[0]; - return { - ...recovered, - blocks: - first && node.position - ? [{ ...first, position: node.position }, ...recovered.blocks.slice(1)] - : recovered.blocks, - }; -} - -/** - * CommonMark treats four or more spaces after a list marker as an indented - * code block. In chat output, excessive spacing is commonly accidental - * alignment such as `- text`, which otherwise produces a full code card - * for every bullet. Only normalize blocks that retain excess indentation and - * start on the marker's own line; explicit fences and conventional indented - * blocks remain code. - */ -function attachListItemIndentationNormalizer(this: MarkdownParser) { - return (tree: MarkdownAstNode, file: MarkdownFile) => { - if (typeof file.value !== "string") { - return; - } - const markdown = file.value; - - const visit = (node: MarkdownAstNode, source: string) => { - if (!node.children) { - return; - } - node.children = node.children.flatMap((child) => { - if (isSameLineOverIndentedCode(child, node, source)) { - const recovered = blocksFromIndentedCode(child, this); - for (const block of recovered.blocks) { - visit(block, recovered.source); - } - return recovered.blocks; - } - visit(child, source); - return [child]; - }); - }; - - visit(tree, markdown); - }; -} - -export const remarkNormalizeListItemIndentation = attachListItemIndentationNormalizer; +export { remarkNormalizeListItemIndentation } from "@t3tools/client-runtime/markdown-list-indentation"; diff --git a/docs/internals/codex-source-citations.md b/docs/internals/codex-source-citations.md new file mode 100644 index 000000000000..52c59a5d32c8 --- /dev/null +++ b/docs/internals/codex-source-citations.md @@ -0,0 +1,103 @@ +# Codex source citations + +> For maintainers. Using T3 Code? See [Source citations](../user/providers-codex.md#source-citations). + +Codex can include source references in assistant Markdown using private-use Unicode delimiters. +These are provider-generated research citations, separate from T3's +[assistant quote citations](./assistant-citations.md) and Codex's `:codex-file-citation{...}` +file directives. + +## Marker syntax + +OpenAI's [citation formatting guide][format] defines these forms: + +```text +citeturn0view0 +citeturn0view0turn1search2 +citeturn0view0L10-L20 +``` + +The opening delimiter is `U+E200`, the closing delimiter is `U+E201`, and `U+E202` separates +the marker type, source references, and optional trailing line locator. +The locator can be one line such as `L10` or a range such as `L10-L20`. One marker can cite several +sources. + +A reference such as `turn0view0` identifies a tool result in the provider's context. `turnN` +counts tool invocations, not T3 conversation turns; suffixes distinguish results. The reference +is not a URL or an app-server tool-call ID, and its spelling does not identify the source's +website. Line locators refer to the source's numbered content, not lines in the assistant reply. + +## Source metadata and transport + +The [Responses API web search guide][web-search] describes separate URL citation annotations +carrying source URLs, titles, and text offsets. The marker text alone does not carry those fields. + +T3 uses [Codex app-server][app-server], whose [generated protocol schema][schema] exposes +`agentMessage.text` and optional `webSearch.results`. The results are opaque JSON for standalone +web search; their presence does not guarantee a reference-ID-to-URL mapping. `memoryCitation` +is separate metadata, not web citation annotations. Raw response `output_text` in the pinned +schema contains only `type` and `text`. + +The report that prompted this renderer had a source marker in the saved assistant text, no +annotations, and `results: null` on the completed web search. The tool action contained a page +URL but no association between that URL and the marker's source ID. Reading the full stored +payload did not recover that association. + +The [Codex adapter][adapter] retains full tool payloads, and [runtime ingestion][ingestion] +persists them on completion. [Activity payload projection][projection] removes unneeded tool +fields before sending activities to clients, including web search result data. Neither that +projection nor a database migration can recover metadata the provider never supplied. + +Do not derive citation URLs from search order, nearby Markdown links, or a similar-looking tool +ID. Add source navigation only when the provider supplies an explicit mapping. Normalize that +metadata at the adapter boundary rather than shipping entire search outputs to every client. + +## Rendering and copy + +[`codexCitations.ts`][parser], exported as `@t3tools/client-runtime/codex-citations`, owns +parsing, source numbering, and the readable Markdown fallback. `remarkCodexCitations` transforms +valid citation markers into annotated Markdown nodes. `codexCitationFromHastProperties` reads +their citation payload for the web renderer. Source numbers follow first appearance within the +rendered document, and repeated source IDs keep their number. + +GFM parsing and [list indentation recovery][list-recovery] run before citation rendering on both +paths. Recovered nodes retain their original source positions, so copying can replace just the +marker and streaming can recognize the actual end of the message. On web, citations render +before `remarkBreaks` and other transforms that can replace text nodes. + +[`ChatMarkdown`][markdown] renders these nodes as [`CodexCitationChip`][chip] controls on web and +desktop. A chip shows the source numbers; its popover lists the original source IDs and any line +locator. It explains that the source URL is unavailable instead of offering an invented link. +`codexCitationMarkdown` supplies readable text for selection copy through `data-markdown-copy`. +Rich-text copy uses `codexCitationText` through `data-markdown-copy-text` before removing UI +controls, so the pasted reference retains its source IDs and locator. + +`renderCodexCitationsAsMarkdown` uses the same parser for [whole-message copy][copy] and +[mobile assistant responses][mobile]. For example, a single source displays as +`[Source 1: turn0view0]`; grouped citations retain every source ID and any line locator. +The generated Markdown escapes the brackets so these references do not become links. Mobile +applies the conversion before splitting artifacts and passes the result to its native Markdown +renderers. Mobile rendering and whole-message copy leave user messages unchanged. + +Only prose markers are transformed. Code examples, link and image labels or destinations, and +HTML code or anchor content stay literal. Unknown citation families and malformed complete +markers also stay literal. With `isStreaming`, an unfinished recognized citation at the end of +the document is hidden while its remaining text arrives. Once streaming ends, an incomplete +marker remains visible rather than disappearing from the answer. + +This is a rendering change. Stored message text retains the original markers, and citations do +not create new server requests, database records, or provider configuration. + +[format]: https://developers.openai.com/api/docs/guides/citation-formatting +[web-search]: https://developers.openai.com/api/docs/guides/tools-web-search +[app-server]: https://developers.openai.com/codex/app-server +[schema]: ../../packages/effect-codex-app-server/src/_generated/schema.gen.ts +[adapter]: ../../apps/server/src/provider/Layers/CodexAdapter.ts +[ingestion]: ../../apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts +[projection]: ../../apps/server/src/orchestration/ActivityPayloadProjection.ts +[parser]: ../../packages/client-runtime/src/codexCitations.ts +[list-recovery]: ../../packages/client-runtime/src/markdownListIndentation.ts +[markdown]: ../../apps/web/src/components/ChatMarkdown.tsx +[chip]: ../../apps/web/src/components/chat/CodexCitationChip.tsx +[copy]: ../../apps/web/src/components/chat/MessagesTimeline.logic.ts +[mobile]: ../../apps/mobile/src/features/threads/ThreadFeed.tsx diff --git a/docs/user/providers-codex.md b/docs/user/providers-codex.md index 5556ef18f119..b34ca0ead038 100644 --- a/docs/user/providers-codex.md +++ b/docs/user/providers-codex.md @@ -49,6 +49,15 @@ The web and desktop Agents panel shows each sub-agent's model and reasoning effo reports them. If Codex does not report either value, T3 Code leaves it out instead of using the parent agent's settings. +## Source citations + +Codex research responses can include numbered source citations. On web and desktop, select a +citation to inspect its source IDs and any cited line ranges. When Codex supplies only source +IDs, the details explain that the source URL is unavailable. There is no destination link to open. + +Mobile and copied responses show readable source references. These citations are separate from +quoting an assistant response in the composer. + ## Browser and computer activity Browser and Computer Use calls show their user-facing task title when Codex provides one. Expanded diff --git a/packages/client-runtime/package.json b/packages/client-runtime/package.json index 68f5bad51474..cde251c73b17 100644 --- a/packages/client-runtime/package.json +++ b/packages/client-runtime/package.json @@ -23,6 +23,10 @@ "types": "./src/markdownLinks.ts", "default": "./src/markdownLinks.ts" }, + "./markdown-list-indentation": { + "types": "./src/markdownListIndentation.ts", + "default": "./src/markdownListIndentation.ts" + }, "./media-reference": { "types": "./src/mediaReference.ts", "default": "./src/mediaReference.ts" @@ -39,6 +43,10 @@ "types": "./src/codexFileCitations.ts", "default": "./src/codexFileCitations.ts" }, + "./codex-citations": { + "types": "./src/codexCitations.ts", + "default": "./src/codexCitations.ts" + }, "./codex-artifact-templates": { "types": "./src/codexArtifactTemplates.ts", "default": "./src/codexArtifactTemplates.ts" @@ -223,6 +231,7 @@ "mdast-util-directive": "^3.1.0", "micromark-extension-directive": "^4.0.0", "micromark-util-character": "^2.1.1", + "remark-gfm": "^4.0.1", "remark-parse": "^11.0.0", "unified": "^11.0.5" }, diff --git a/packages/client-runtime/src/codexCitations.test.ts b/packages/client-runtime/src/codexCitations.test.ts new file mode 100644 index 000000000000..e5a8c52b7e36 --- /dev/null +++ b/packages/client-runtime/src/codexCitations.test.ts @@ -0,0 +1,349 @@ +import { describe, expect, it } from "vite-plus/test"; +import remarkGfm from "remark-gfm"; +import remarkParse from "remark-parse"; +import { unified } from "unified"; + +import { + CODEX_CITATION_HAST_PROPERTIES, + codexCitationFromHastProperties, + codexCitationMarkdown, + codexCitationText, + remarkCodexCitations, + renderCodexCitationsAsMarkdown, + type CodexCitation, +} from "./codexCitations.ts"; +import { remarkCodexDirectives } from "./codexMarkdownDirectives.ts"; +import { remarkNormalizeListItemIndentation } from "./markdownListIndentation.ts"; + +interface TestNode { + readonly type: string; + readonly value?: string; + readonly position?: { + readonly start: { readonly offset?: number }; + readonly end: { readonly offset?: number }; + }; + readonly data?: { + readonly hName?: string; + readonly hProperties?: Readonly>; + }; + readonly children?: readonly TestNode[]; +} + +function marker(...parts: string[]): string { + return `\uE200cite\uE202${parts.join("\uE202")}\uE201`; +} + +const SCREENSHOT_CITATION = marker("turn0view0"); + +function parse(markdown: string, options: { readonly isStreaming?: boolean } = {}): TestNode { + const processor = unified() + .use(remarkParse) + .use(remarkGfm) + .use(remarkNormalizeListItemIndentation) + .use(remarkCodexDirectives) + .use(remarkCodexCitations, options); + return processor.runSync(processor.parse(markdown), { value: markdown }) as TestNode; +} + +function citationNodes(tree: TestNode): TestNode[] { + return [ + ...(tree.data?.hName === "span" ? [tree] : []), + ...(tree.children?.flatMap(citationNodes) ?? []), + ]; +} + +function citations(markdown: string): (CodexCitation | null)[] { + return citationNodes(parse(markdown)).map((node) => + codexCitationFromHastProperties(node.data?.hProperties), + ); +} + +describe("remarkCodexCitations", () => { + it("recognizes the screenshot's source marker and retains its exact source position", () => { + const markdown = `Both reporters proposed refreshing after rejection. ${SCREENSHOT_CITATION}`; + const [node] = citationNodes(parse(markdown)); + expect(node).toMatchObject({ + type: "text", + value: "[Source 1: turn0view0]", + position: { + start: { offset: markdown.indexOf(SCREENSHOT_CITATION) }, + end: { offset: markdown.length }, + }, + data: { hName: "span" }, + }); + expect(codexCitationFromHastProperties(node?.data?.hProperties)).toEqual({ + raw: SCREENSHOT_CITATION, + sources: [{ id: "turn0view0", number: 1 }], + }); + expect(Object.keys(node?.data?.hProperties ?? {})).toEqual(CODEX_CITATION_HAST_PROPERTIES); + }); + + it("numbers sources by first appearance and deduplicates grouped references", () => { + const grouped = marker("alpha__id__", "turn0search0", "alpha__id__", "L8-L13"); + const repeated = marker("turn0search0", "turn2view3", "L9"); + expect(citations(`**${grouped}**\n\n- Again ${repeated}.`)).toEqual([ + { + raw: grouped, + sources: [ + { id: "alpha__id__", number: 1 }, + { id: "turn0search0", number: 2 }, + ], + locator: "L8-L13", + }, + { + raw: repeated, + sources: [ + { id: "turn0search0", number: 2 }, + { id: "turn2view3", number: 3 }, + ], + locator: "L9", + }, + ]); + }); + + it("resets numbering when a processor is reused for another document", () => { + const processor = unified().use(remarkParse).use(remarkCodexCitations).freeze(); + for (const id of ["turn0view0", "other-id"]) { + const markdown = marker(id); + const tree = processor.runSync(processor.parse(markdown), { value: markdown }) as TestNode; + const [node] = citationNodes(tree); + expect(codexCitationFromHastProperties(node?.data?.hProperties)?.sources).toEqual([ + { id, number: 1 }, + ]); + } + }); + + it("handles consecutive markers without swallowing the following text", () => { + const markdown = `Before ${SCREENSHOT_CITATION}${marker("second")}, after.`; + expect(renderCodexCitationsAsMarkdown(markdown)).toBe( + "Before \\[Source 1: turn0view0\\]\\[Source 2: second\\], after.", + ); + }); + + it.each([ + ["inline code", `\`${SCREENSHOT_CITATION}\``], + ["fenced code", `\`\`\`md\n${SCREENSHOT_CITATION}\n\`\`\``], + ["indented code", ` ${SCREENSHOT_CITATION}`], + ["quoted code", `> ${SCREENSHOT_CITATION}`], + ["link label", `[Source ${SCREENSHOT_CITATION}](https://example.com)`], + ["link destination", `[Source](https://example.com/${SCREENSHOT_CITATION})`], + ["link title", `[Source](https://example.com "${SCREENSHOT_CITATION}")`], + ["bare URL", `https://example.com/${SCREENSHOT_CITATION}`], + ["bare www URL", `www.example.com/${SCREENSHOT_CITATION}`], + ["angle-bracket URL", ``], + ["reference link", `[${SCREENSHOT_CITATION}][ref]\n\n[ref]: https://example.com`], + ["image label", `![Source ${SCREENSHOT_CITATION}](image.png)`], + ["image destination", `![Source](images/${SCREENSHOT_CITATION})`], + ["reference image", `![${SCREENSHOT_CITATION}][ref]\n\n[ref]: image.png`], + ["HTML code", `Before ${SCREENSHOT_CITATION} after.`], + ["HTML anchor", `Before ${SCREENSHOT_CITATION} after.`], + ["HTML preformatted block", `
\n${SCREENSHOT_CITATION}\n
`], + ["HTML script", ``], + ["HTML attribute", `text`], + ])("preserves citations in %s", (_name, markdown) => { + expect(citations(markdown)).toEqual([]); + expect(renderCodexCitationsAsMarkdown(markdown)).toBe(markdown); + }); + + it("excluded examples do not consume source numbers or suppress following citations", () => { + const markdown = `Before ${marker("excluded")} ${SCREENSHOT_CITATION}.`; + expect(citations(markdown)).toEqual([ + { raw: SCREENSHOT_CITATION, sources: [{ id: "turn0view0", number: 1 }] }, + ]); + }); + + it("does not mistake commented HTML for an open code element", () => { + expect(citations(`\n\n${SCREENSHOT_CITATION}`)).toHaveLength(1); + }); + + it("does not mistake a quoted HTML attribute for an open code element", () => { + const html = 'text'; + expect(citations(`Before ${html} ${SCREENSHOT_CITATION}`)).toHaveLength(1); + expect(renderCodexCitationsAsMarkdown(`${html}\n\n${SCREENSHOT_CITATION}`)).toBe( + `${html}\n\n\\[Source 1: turn0view0\\]`, + ); + }); + + it("keeps HTML inside an excluded Markdown link from changing later citations", () => { + expect(citations(`[label](https://example.com) ${SCREENSHOT_CITATION}`)).toHaveLength(1); + }); + + it("keeps URL-contained markers literal without consuming source numbers", () => { + const url = `https://example.com/${marker("excluded")}`; + expect(renderCodexCitationsAsMarkdown(`${url}\n\n${SCREENSHOT_CITATION}`)).toBe( + `${url}\n\n\\[Source 1: turn0view0\\]`, + ); + expect(citations(`${url}\n\n${SCREENSHOT_CITATION}`)).toEqual([ + { raw: SCREENSHOT_CITATION, sources: [{ id: "turn0view0", number: 1 }] }, + ]); + }); + + it("leaves markers inside Codex directives unchanged without consuming source numbers", () => { + const file = `:codex-file-citation{path="outputs/${marker("excluded-file")}.txt"}`; + const artifact = `::artifact-template{skill_name="artifact-template-report" skill_directory="/skills/report" display_name="Report ${marker("excluded-template")}" artifact_kind="document"}`; + const markdown = `${file}\n\n${artifact}\n\nResult ${SCREENSHOT_CITATION}.`; + expect(renderCodexCitationsAsMarkdown(markdown)).toBe( + `${file}\n\n${artifact}\n\nResult \\[Source 1: turn0view0\\].`, + ); + expect(citations(markdown)).toEqual([ + { raw: SCREENSHOT_CITATION, sources: [{ id: "turn0view0", number: 1 }] }, + ]); + }); + + it("numbers recovered overindented-list citations using original source positions", () => { + const first = marker("first"); + const second = marker("second"); + const markdown = `- A ${first}.\n\nOther ${second}.`; + expect(renderCodexCitationsAsMarkdown(markdown)).toBe( + "- A \\[Source 1: first\\].\n\nOther \\[Source 2: second\\].", + ); + const nodes = citationNodes(parse(markdown)); + for (const [index, raw] of [first, second].entries()) { + expect(nodes[index]?.position).toMatchObject({ + start: { offset: markdown.indexOf(raw) }, + end: { offset: markdown.indexOf(raw) + raw.length }, + }); + } + }); + + it.each([ + "\uE200image\uE202turn0view0\uE201", + "\uE200filecite\uE202turn0file0\uE201", + "\uE200CITE\uE202turn0view0\uE201", + "\uE200cite\uE201", + marker(""), + marker("L8"), + marker("L8-L13"), + marker("turn0view0", ""), + marker("two words"), + marker("source.with.dots"), + marker("https://example.com"), + marker("source", "L8–L13"), + marker("source\nnext"), + ])("preserves unsupported or malformed markers: %s", (markdown) => { + expect(citations(markdown)).toEqual([]); + expect(renderCodexCitationsAsMarkdown(markdown)).toBe(markdown); + expect(renderCodexCitationsAsMarkdown(markdown, { isStreaming: true })).toBe(markdown); + }); +}); + +describe("streaming citations", () => { + it("hides an unfinished recognized tail and restores it after streaming stops", () => { + const complete = marker("source__id", "second-id", "L8-L13"); + for (let length = 1; length < complete.length; length += 1) { + const partial = complete.slice(0, length); + expect(renderCodexCitationsAsMarkdown(`Before ${partial}`, { isStreaming: true })).toBe( + "Before ", + ); + expect(renderCodexCitationsAsMarkdown(`Before ${partial}`)).toBe(`Before ${partial}`); + } + expect(renderCodexCitationsAsMarkdown(complete, { isStreaming: true })).toBe( + "\\[Source 1: source\\_\\_id; Source 2: second-id; L8-L13\\]", + ); + }); + + it.each([ + "Before \uE200cite\uE202source\n\nAfter", + "Before \uE200cite\uE202source more text", + "Before \uE200chart", + "Before \uE200citation", + "Before \uE200cite\uE202source\uE202\uE202", + "`\uE200cite\uE202source`", + "```text\n\uE200cite\uE202source", + " \uE200cite\uE202source", + "Before \uE200cite\uE202source", + 'Before \uE200cite\uE202source', + "https://example.com/\uE200cite\uE202source", + ])("does not hide excluded or non-tail text: %s", (markdown) => { + expect(renderCodexCitationsAsMarkdown(markdown, { isStreaming: true })).toBe(markdown); + }); + + it("keeps completed citations visible while hiding only the unfinished final marker", () => { + expect( + renderCodexCitationsAsMarkdown(`${SCREENSHOT_CITATION} after \uE200cite\uE202next`, { + isStreaming: true, + }), + ).toBe("\\[Source 1: turn0view0\\] after "); + }); + + it("recognizes a streaming tail in a recovered overindented list", () => { + const markdown = "- A \uE200cite\uE202source"; + expect(renderCodexCitationsAsMarkdown(markdown, { isStreaming: true })).toBe("- A "); + expect(renderCodexCitationsAsMarkdown(markdown)).toBe(markdown); + }); +}); + +describe("portable citation Markdown", () => { + it("preserves every unrelated source byte, including escapes, entities and spacing", () => { + const before = "# Heading\n\n🦊 \\*literal\\* & **bold** \n> quote "; + const after = "\n\n- [link]()\n\n```js\nconst value = 1;\n```\n"; + expect(renderCodexCitationsAsMarkdown(`${before}${SCREENSHOT_CITATION}${after}`)).toBe( + `${before}\\[Source 1: turn0view0\\]${after}`, + ); + const ordinary = `${before}${after}`; + expect(renderCodexCitationsAsMarkdown(ordinary)).toBe(ordinary); + }); + + it("escapes labels and IDs so they cannot turn into Markdown links or emphasis", () => { + const citation = { + raw: marker("__opaque_id__"), + sources: [{ id: "__opaque_id__", number: 1 }], + }; + const markdown = codexCitationMarkdown(citation); + expect(markdown).toBe("\\[Source 1: \\_\\_opaque\\_id\\_\\_\\]"); + expect(codexCitationText(citation)).toBe("[Source 1: __opaque_id__]"); + const tree = unified().use(remarkParse).parse(markdown); + expect(tree.children).toMatchObject([ + { type: "paragraph", children: [{ type: "text", value: "[Source 1: __opaque_id__]" }] }, + ]); + }); + + it("never guesses a URL from a source ID", () => { + const rendered = renderCodexCitationsAsMarkdown(marker("example-com", "turn0view0", "L8")); + expect(rendered).toBe("\\[Source 1: example-com; Source 2: turn0view0; L8\\]"); + expect(rendered).not.toContain("]("); + }); +}); + +describe("citation HAST payload validation", () => { + const valid: CodexCitation = { + raw: marker("turn0view0", "L8-L13"), + sources: [{ id: "turn0view0", number: 3 }], + locator: "L8-L13", + }; + + it("round-trips a valid payload without requiring a link destination", () => { + expect(codexCitationFromHastProperties({ dataCodexCitation: JSON.stringify(valid) })).toEqual( + valid, + ); + }); + + it.each([undefined, null, {}, { dataCodexCitation: 1 }, { dataCodexCitation: "invalid json" }])( + "rejects missing or malformed properties", + (properties) => { + expect(codexCitationFromHastProperties(properties)).toBeNull(); + }, + ); + + it.each([ + null, + 1, + [], + { ...valid, raw: marker("https://example.com") }, + { ...valid, raw: 3 }, + { ...valid, sources: [] }, + { ...valid, sources: [null] }, + { ...valid, sources: [{ id: "different", number: 1 }] }, + { ...valid, sources: [{ id: "turn0view0", number: 0 }] }, + { ...valid, sources: [{ id: "turn0view0", number: -1 }] }, + { ...valid, sources: [{ id: "turn0view0", number: 1.5 }] }, + { ...valid, sources: [{ id: "turn0view0", number: "1" }] }, + { ...valid, sources: [{ id: "turn0view0", number: Number.MAX_SAFE_INTEGER + 1 }] }, + { ...valid, locator: "L2" }, + { ...valid, locator: null }, + ])("rejects payloads inconsistent with the source marker", (payload) => { + expect( + codexCitationFromHastProperties({ dataCodexCitation: JSON.stringify(payload) }), + ).toBeNull(); + }); +}); diff --git a/packages/client-runtime/src/codexCitations.ts b/packages/client-runtime/src/codexCitations.ts new file mode 100644 index 000000000000..3fa80f3bc399 --- /dev/null +++ b/packages/client-runtime/src/codexCitations.ts @@ -0,0 +1,347 @@ +import type { Extension, Tokenizer } from "micromark-util-types"; +import remarkGfm from "remark-gfm"; +import remarkParse from "remark-parse"; +import { unified, type Processor } from "unified"; + +import { remarkCodexDirectives } from "./codexMarkdownDirectives.ts"; +import { remarkNormalizeListItemIndentation } from "./markdownListIndentation.ts"; + +const START = "\uE200"; +const SEPARATOR = "\uE202"; +const END = "\uE201"; +const PREFIX = `${START}cite${SEPARATOR}`; +const TOKEN = "codexCitation"; +const SOURCE_ID = /^[A-Za-z0-9_-]+$/; +const LOCATOR = /^L\d+(?:-L\d+)?$/; +const RAW_HTML_CONTAINERS = new Set(["a", "code", "pre", "script", "style", "textarea"]); + +declare module "micromark-util-types" { + interface TokenTypeMap { + codexCitation: "codexCitation"; + } +} + +export interface CodexCitation { + readonly raw: string; + readonly sources: ReadonlyArray<{ readonly id: string; readonly number: number }>; + readonly locator?: string; +} + +interface CitationOptions { + readonly isStreaming?: boolean; +} + +interface MarkdownPoint { + readonly line: number; + readonly column: number; + readonly offset?: number; +} + +interface MarkdownNode { + type?: string; + value?: string; + position?: { readonly start: MarkdownPoint; readonly end: MarkdownPoint }; + data?: { + codexCitationMarkdown?: string; + hName?: string; + hProperties?: Record; + }; + children?: MarkdownNode[]; +} + +const citationTokens = new WeakMap(); + +export const CODEX_CITATION_HAST_PROPERTIES = ["dataCodexCitation"] as const; + +function parseCitation(raw: string) { + if (!raw.startsWith(PREFIX) || !raw.endsWith(END)) return null; + const ids = raw.slice(PREFIX.length, -END.length).split(SEPARATOR); + if (!ids.every((id) => SOURCE_ID.test(id))) return null; + const last = ids.at(-1); + const locator = last !== undefined && LOCATOR.test(last) ? ids.pop() : undefined; + if (ids.length === 0) return null; + return { ids: [...new Set(ids)], ...(locator === undefined ? {} : { locator }) }; +} + +function isSourceCharacter(code: number | null): code is number { + return ( + code !== null && + ((code >= 48 && code <= 57) || + (code >= 65 && code <= 90) || + (code >= 97 && code <= 122) || + code === 45 || + code === 95) + ); +} + +const tokenizeCitation: Tokenizer = function (effects, ok, nok) { + let prefixIndex = 0; + let segmentHasCharacters = false; + return prefix; + + function finish(code: number | null) { + effects.exit(TOKEN); + return ok(code); + } + + function prefix(code: number | null) { + if (code === null && prefixIndex > 0) return finish(code); + if (code !== PREFIX.charCodeAt(prefixIndex)) return nok(code); + if (prefixIndex === 0) effects.enter(TOKEN); + effects.consume(code); + prefixIndex += 1; + return prefixIndex === PREFIX.length ? source : prefix; + } + + function source(code: number | null) { + if (code === null) return finish(code); + if (isSourceCharacter(code)) { + segmentHasCharacters = true; + effects.consume(code); + return source; + } + if (!segmentHasCharacters) return nok(code); + if (code === SEPARATOR.charCodeAt(0)) { + segmentHasCharacters = false; + effects.consume(code); + return source; + } + if (code === END.charCodeAt(0)) { + effects.consume(code); + return finish; + } + return nok(code); + } +}; + +const CITATION_SYNTAX: Extension = { + text: { [START.charCodeAt(0)]: { tokenize: tokenizeCitation } }, +}; + +export function codexCitationText(citation: CodexCitation): string { + const sources = citation.sources.map(({ id, number }) => `Source ${number}: ${id}`); + if (citation.locator !== undefined) sources.push(citation.locator); + return `[${sources.join("; ")}]`; +} + +/** Keeps the opaque source IDs readable without inventing link destinations. */ +export function codexCitationMarkdown(citation: CodexCitation): string { + return codexCitationText(citation).replace(/[\\[\]*_`<&]/g, "\\$&"); +} + +function updateHtmlContext(value: string, tags: string[]): void { + if (value.startsWith("])*>/gi, + )) { + const tag = match[2]?.toLowerCase(); + if (tag === undefined || !RAW_HTML_CONTAINERS.has(tag)) continue; + if (match[1]) { + const index = tags.lastIndexOf(tag); + if (index !== -1) tags.splice(index); + } else if (!match[0].endsWith("/>")) { + tags.push(tag); + } + } +} + +function transformCitations(tree: MarkdownNode, source: string, options: CitationOptions): void { + const numbers = new Map(); + const htmlTags: string[] = []; + + function visit(node: MarkdownNode, excluded = false): MarkdownNode[] { + if (node.type === "html") { + if (!excluded) updateHtmlContext(node.value ?? "", htmlTags); + return [node]; + } + + const raw = citationTokens.get(node); + if (raw !== undefined && node.position !== undefined) { + citationTokens.delete(node); + const end = { + line: node.position.start.line, + column: node.position.start.column + raw.length, + ...(node.position.start.offset === undefined + ? {} + : { offset: node.position.start.offset + raw.length }), + }; + const parsed = parseCitation(raw); + const hidden = + parsed === null && + !raw.endsWith(END) && + options.isStreaming === true && + end.offset === source.length; + if (excluded || htmlTags.length > 0 || (parsed === null && !hidden)) return [node]; + + const citation = parsed && { + raw, + sources: parsed.ids.map((id) => { + let number = numbers.get(id); + if (number === undefined) { + number = numbers.size + 1; + numbers.set(id, number); + } + return { id, number }; + }), + ...(parsed.locator === undefined ? {} : { locator: parsed.locator }), + }; + const replacement: MarkdownNode = { + type: "text", + value: citation ? codexCitationText(citation) : "", + position: { start: node.position.start, end }, + data: { + codexCitationMarkdown: citation ? codexCitationMarkdown(citation) : "", + ...(citation + ? { + hName: "span", + hProperties: { dataCodexCitation: JSON.stringify(citation) }, + } + : {}), + }, + }; + // mdast can append following text to this token's text node. + const tail = node.value?.slice(raw.length); + return tail + ? [ + replacement, + { + type: "text", + value: tail, + position: { start: end, end: node.position.end }, + }, + ] + : [replacement]; + } + + const excludeChildren = + excluded || + node.type === "link" || + node.type === "linkReference" || + node.type === "image" || + node.type === "imageReference" || + node.type === "code" || + node.type === "inlineCode"; + if (node.children) { + node.children = node.children.flatMap((child) => visit(child, excludeChildren)); + } + return [node]; + } + + visit(tree); +} + +/** Tokenizes citations before Markdown can interpret underscores inside source IDs. */ +function attachCodexCitations(this: Processor, options: CitationOptions = {}) { + const data = this.data(); + (data.micromarkExtensions ??= []).push(CITATION_SYNTAX); + (data.fromMarkdownExtensions ??= []).push({ + enter: { + codexCitation(token) { + const raw = this.sliceSerialize(token); + const node = { type: "text" as const, value: raw }; + citationTokens.set(node, raw); + this.enter(node, token); + }, + }, + exit: { + codexCitation(token) { + this.exit(token); + }, + }, + }); + + return (tree: unknown, file: { readonly value: unknown }) => { + const source = String(file.value); + if (source.includes(START)) transformCitations(tree as MarkdownNode, source, options); + }; +} + +export const remarkCodexCitations = attachCodexCitations; + +const citationParser = unified() + .use(remarkParse) + .use(remarkGfm) + .use(remarkNormalizeListItemIndentation) + .use(remarkCodexDirectives) + .use(remarkCodexCitations) + .freeze(); +const streamingCitationParser = unified() + .use(remarkParse) + .use(remarkGfm) + .use(remarkNormalizeListItemIndentation) + .use(remarkCodexDirectives) + .use(remarkCodexCitations, { isStreaming: true }) + .freeze(); + +/** Replaces only citation source ranges; all other Markdown remains byte-for-byte intact. */ +export function renderCodexCitationsAsMarkdown( + markdown: string, + options: CitationOptions = {}, +): string { + if (!markdown.includes(START)) return markdown; + const parser = options.isStreaming ? streamingCitationParser : citationParser; + const tree = parser.runSync(parser.parse(markdown), { value: markdown }) as MarkdownNode; + const replacements: { readonly start: number; readonly end: number; readonly text: string }[] = + []; + function collect(node: MarkdownNode): void { + const text = node.data?.codexCitationMarkdown; + const start = node.position?.start.offset; + const end = node.position?.end.offset; + if (text !== undefined && start !== undefined && end !== undefined) { + replacements.push({ start, end, text }); + } + for (const child of node.children ?? []) collect(child); + } + collect(tree); + let rendered = ""; + let cursor = 0; + for (const replacement of replacements) { + rendered += markdown.slice(cursor, replacement.start) + replacement.text; + cursor = replacement.end; + } + return rendered + markdown.slice(cursor); +} + +export function codexCitationFromHastProperties( + properties: Readonly> | null | undefined, +): CodexCitation | null { + const serialized = properties?.dataCodexCitation; + if (typeof serialized !== "string") return null; + let value: unknown; + try { + value = JSON.parse(serialized); + } catch { + return null; + } + if (typeof value !== "object" || value === null || !("raw" in value)) return null; + if (typeof value.raw !== "string" || !("sources" in value) || !Array.isArray(value.sources)) { + return null; + } + const parsed = parseCitation(value.raw); + if (!parsed || parsed.ids.length !== value.sources.length) return null; + const locator = "locator" in value ? value.locator : undefined; + if (locator !== parsed.locator) return null; + const sources: { readonly id: string; readonly number: number }[] = []; + const rawSources: readonly unknown[] = value.sources; + for (const [index, source] of rawSources.entries()) { + if ( + typeof source !== "object" || + source === null || + !("id" in source) || + typeof source.id !== "string" || + source.id !== parsed.ids[index] || + !("number" in source) || + typeof source.number !== "number" || + !Number.isSafeInteger(source.number) || + source.number <= 0 + ) { + return null; + } + sources.push({ id: source.id, number: source.number }); + } + return { + raw: value.raw, + sources, + ...(parsed.locator === undefined ? {} : { locator: parsed.locator }), + }; +} diff --git a/packages/client-runtime/src/markdownListIndentation.test.ts b/packages/client-runtime/src/markdownListIndentation.test.ts new file mode 100644 index 000000000000..d407cf570078 --- /dev/null +++ b/packages/client-runtime/src/markdownListIndentation.test.ts @@ -0,0 +1,145 @@ +import { describe, expect, it } from "vite-plus/test"; +import remarkGfm from "remark-gfm"; +import remarkParse from "remark-parse"; +import { unified } from "unified"; + +import { remarkNormalizeListItemIndentation } from "./markdownListIndentation.ts"; + +interface TestNode { + readonly type: string; + readonly value?: string; + readonly position?: { + readonly start: { readonly line: number; readonly column: number; readonly offset: number }; + readonly end: { readonly line: number; readonly column: number; readonly offset: number }; + }; + readonly children?: readonly TestNode[]; +} + +const parser = unified() + .use(remarkParse) + .use(remarkGfm) + .use(remarkNormalizeListItemIndentation) + .freeze(); + +function parse(markdown: string): TestNode { + return parser.runSync(parser.parse(markdown), { value: markdown }) as TestNode; +} + +function nodesOfType(node: TestNode, type: string): TestNode[] { + return [ + ...(node.type === type ? [node] : []), + ...(node.children?.flatMap((child) => nodesOfType(child, type)) ?? []), + ]; +} + +function sourceSlice(markdown: string, node: TestNode | undefined): string { + expect(node?.position).toBeDefined(); + return markdown.slice(node?.position?.start.offset, node?.position?.end.offset); +} + +describe("remarkNormalizeListItemIndentation source positions", () => { + it("keeps inline Markdown ranges in the original source after removing the parser prefix", () => { + const markdown = "Earlier text.\n\n- lead **bold** [docs](https://example.com) `inline`"; + const tree = parse(markdown); + + expect(nodesOfType(tree, "code")).toEqual([]); + expect(sourceSlice(markdown, nodesOfType(tree, "strong")[0])).toBe("**bold**"); + expect(sourceSlice(markdown, nodesOfType(tree, "link")[0])).toBe("[docs](https://example.com)"); + expect(sourceSlice(markdown, nodesOfType(tree, "inlineCode")[0])).toBe("`inline`"); + for (const node of nodesOfType(tree, "text")) { + expect(sourceSlice(markdown, node)).toBe(node.value); + } + }); + + it.each(["\n", "\r\n", "\r"])("maps multiline recovery with %j line endings", (newline) => { + const markdown = [ + "- **first**", + " [second](https://example.com)", + "", + " **last**", + ].join(newline); + const tree = parse(markdown); + const [first, last] = nodesOfType(tree, "strong"); + const [link] = nodesOfType(tree, "link"); + + expect(sourceSlice(markdown, first)).toBe("**first**"); + expect(sourceSlice(markdown, link)).toBe("[second](https://example.com)"); + expect(sourceSlice(markdown, last)).toBe("**last**"); + expect(link?.position?.start).toEqual({ + line: 2, + column: 9, + offset: markdown.indexOf("[second]"), + }); + expect(last?.position?.end).toEqual({ line: 4, column: 17, offset: markdown.length }); + }); + + it("maps nested recovered lists repeatedly without returning synthetic coordinates", () => { + const markdown = [ + "- **outer**", + "", + " - **inner**", + "", + " - [deep](https://example.com)", + ].join("\n"); + const tree = parse(markdown); + + expect(nodesOfType(tree, "code")).toEqual([]); + expect(nodesOfType(tree, "list")).toHaveLength(3); + expect(nodesOfType(tree, "strong").map((node) => sourceSlice(markdown, node))).toEqual([ + "**outer**", + "**inner**", + ]); + const [link] = nodesOfType(tree, "link"); + expect(sourceSlice(markdown, link)).toBe("[deep](https://example.com)"); + expect(link?.position?.start).toEqual({ + line: 5, + column: 25, + offset: markdown.indexOf("[deep]"), + }); + expect(link?.position?.end.offset).toBe(markdown.length); + }); + + it.each([ + ["blockquote prefixes", "> - first\n> **last**", 11], + ["partially expanded tabs", "-\t\t first\n\t\t **last**", 4], + ["preserved tabs", "-\t\t\t first\n\t\t\t **last**", 5], + ])("maps %s using literal source characters", (_name, markdown, column) => { + const tree = parse(markdown); + const [last] = nodesOfType(tree, "strong"); + expect(nodesOfType(tree, "code")).toEqual([]); + expect(sourceSlice(markdown, last)).toBe("**last**"); + expect(last?.position?.start).toEqual({ + line: 2, + column, + offset: markdown.indexOf("**last**"), + }); + expect(last?.position?.end.offset).toBe(markdown.length); + }); + + it("keeps the final text boundary at the original streaming tail", () => { + const tail = "\uE200cite\uE202turn0view0"; + const markdown = `- first\n\n - last ${tail}`; + const texts = nodesOfType(parse(markdown), "text"); + const last = texts.at(-1); + expect(sourceSlice(markdown, last)).toBe(`last ${tail}`); + expect(last?.position?.end.offset).toBe(markdown.length); + }); + + it("keeps GFM grammar and original ranges in recovered content", () => { + const markdown = "- ~~old~~ https://example.com"; + const tree = parse(markdown); + expect(sourceSlice(markdown, nodesOfType(tree, "delete")[0])).toBe("~~old~~"); + expect(sourceSlice(markdown, nodesOfType(tree, "link")[0])).toBe("https://example.com"); + }); + + it.each([ + "- ```ts\n const value = 1;\n ```", + "-\n const value = 1;", + "- const value = 1;", + "- prose\n\n const value = 1;", + ])("preserves genuine code in %j", (markdown) => { + const code = nodesOfType(parse(markdown), "code"); + expect(code).toHaveLength(1); + expect(code[0]?.value).toContain("const value = 1;"); + }); +}); diff --git a/packages/client-runtime/src/markdownListIndentation.ts b/packages/client-runtime/src/markdownListIndentation.ts new file mode 100644 index 000000000000..73b1ab28df50 --- /dev/null +++ b/packages/client-runtime/src/markdownListIndentation.ts @@ -0,0 +1,215 @@ +interface MarkdownPoint { + readonly line?: number; + readonly column?: number; + readonly offset?: number; +} + +interface MarkdownPosition { + readonly start: MarkdownPoint; + readonly end: MarkdownPoint; +} + +interface MarkdownAstNode { + readonly type: string; + value?: unknown; + position?: MarkdownPosition; + children?: MarkdownAstNode[]; +} + +interface MarkdownFile { + readonly value?: unknown; +} + +interface MarkdownParser { + parse(markdown: string): unknown; +} + +const INLINE_PARSE_PREFIX = "t3-markdown-inline-prefix:"; + +function lineRanges(value: string) { + const lines: { start: number; end: number; next: number }[] = []; + let start = 0; + for (const match of value.matchAll(/\r\n|\r|\n/g)) { + const next = match.index + match[0].length; + lines.push({ start, end: match.index, next }); + start = next; + } + lines.push({ start, end: value.length, next: value.length }); + return lines; +} + +/** Indented code removes line prefixes but otherwise preserves each line's literal suffix. */ +function codeSourceOffsets(node: MarkdownAstNode, source: string): number[] | null { + const start = node.position?.start.offset; + const end = node.position?.end.offset; + if (typeof node.value !== "string" || start === undefined || end === undefined) return null; + + const value = node.value; + const original = source.slice(start, end); + const valueLines = lineRanges(value); + const sourceLines = lineRanges(original); + if (valueLines.length !== sourceLines.length) return null; + + const offsets: number[] = []; + for (const [index, line] of valueLines.entries()) { + const sourceLine = sourceLines[index]; + if (!sourceLine) return null; + const text = value.slice(line.start, line.end); + const indentLength = /^[\t ]*/.exec(text)?.[0].length ?? 0; + const literal = text.slice(indentLength); + let sourceCursor = sourceLine.end - literal.length; + if ( + sourceCursor < sourceLine.start || + original.slice(sourceCursor, sourceLine.end) !== literal || + (line.end === line.next) !== (sourceLine.end === sourceLine.next) + ) { + return null; + } + + for (let column = indentLength; column <= text.length; column += 1) { + offsets[line.start + column] = start + sourceCursor + column - indentLength; + } + for (let column = indentLength - 1; column >= 0; column -= 1) { + sourceCursor -= 1; + if (sourceCursor < sourceLine.start) return null; + if (original[sourceCursor] === text[column]) { + offsets[line.start + column] = start + sourceCursor; + } else if (original[sourceCursor] === "\t" && /^ *$/.test(text.slice(0, column + 1))) { + // Consuming indentation can leave part of a tab as generated spaces. + for (let remaining = 0; remaining <= column; remaining += 1) { + offsets[line.start + remaining] = start + sourceCursor; + } + break; + } else { + return null; + } + } + for (let offset = line.end; offset < line.next; offset += 1) { + offsets[offset] = + start + sourceLine.end + Math.min(offset - line.end, sourceLine.next - sourceLine.end - 1); + } + } + return offsets; +} + +function isSameLineOverIndentedCode( + node: MarkdownAstNode, + parent: MarkdownAstNode, + markdown: string, +): boolean { + if ( + node.type !== "code" || + parent.type !== "listItem" || + typeof node.value !== "string" || + !/^[\t ]/.test(node.value) + ) { + return false; + } + + const nodeStart = node.position?.start; + const parentStart = parent.position?.start; + if ( + nodeStart?.line === undefined || + nodeStart.offset === undefined || + parentStart?.line === undefined || + nodeStart.line !== parentStart.line + ) { + return false; + } + + const sourceCharacter = markdown[nodeStart.offset]; + return sourceCharacter !== "`" && sourceCharacter !== "~"; +} + +function recoverIndentedCode( + node: MarkdownAstNode, + parser: MarkdownParser, + source: string, + pointAt: (offset: number) => MarkdownPoint, +): MarkdownAstNode[] | null { + const offsets = codeSourceOffsets(node, source); + if (!offsets || typeof node.value !== "string") return null; + const value = node.value.trim(); + const trimStart = node.value.length - node.value.trimStart().length; + // The prefix keeps block-looking first-line content inline while retaining + // the processor's configured grammar, including citation tokens and GFM. + const recoveredSource = `${INLINE_PARSE_PREFIX}${value}`; + const document = parser.parse(recoveredSource) as MarkdownAstNode; + const blocks = document.children; + const paragraph = blocks?.[0]; + const children = paragraph?.type === "paragraph" ? paragraph.children : undefined; + const first = children?.[0]; + if ( + !blocks || + !children || + first?.type !== "text" || + typeof first.value !== "string" || + !first.value.startsWith(INLINE_PARSE_PREFIX) + ) { + return null; + } + + first.value = first.value.slice(INLINE_PARSE_PREFIX.length); + if (!first.value) children.shift(); + + const originalPoint = (point: MarkdownPoint) => { + if (point.offset === undefined) return null; + const valueOffset = trimStart + Math.max(0, point.offset - INLINE_PARSE_PREFIX.length); + const originalOffset = offsets[valueOffset]; + return originalOffset === undefined ? null : pointAt(originalOffset); + }; + const remap = (current: MarkdownAstNode): boolean => { + if (current.position) { + const start = originalPoint(current.position.start); + const end = originalPoint(current.position.end); + if (!start || !end) return false; + current.position = { start, end }; + } + return current.children?.every(remap) ?? true; + }; + // Preserve node identity for syntax extensions that associate metadata with nodes. + return remap(document) ? blocks : null; +} + +/** + * Recover accidental same-line list alignment without changing explicit fences + * or conventional indented code. Recovered positions still address the input, + * so later plugins can safely replace source ranges or recognize a streaming tail. + */ +function attachListItemIndentationNormalizer(this: MarkdownParser) { + return (tree: unknown, file: MarkdownFile) => { + if (typeof file.value !== "string") return; + const source = file.value; + let sourceLineStarts: number[] | undefined; + const pointAt = (offset: number) => { + sourceLineStarts ??= lineRanges(source).map((line) => line.start); + let low = 0; + let high = sourceLineStarts.length; + while (low + 1 < high) { + const middle = (low + high) >>> 1; + if ((sourceLineStarts[middle] ?? 0) <= offset) low = middle; + else high = middle; + } + return { line: low + 1, column: offset - (sourceLineStarts[low] ?? 0) + 1, offset }; + }; + + const visit = (node: MarkdownAstNode) => { + if (!node.children) return; + node.children = node.children.flatMap((child) => { + if (isSameLineOverIndentedCode(child, node, source)) { + const recovered = recoverIndentedCode(child, this, source, pointAt); + if (recovered) { + recovered.forEach(visit); + return recovered; + } + } + visit(child); + return [child]; + }); + }; + + visit(tree as MarkdownAstNode); + }; +} + +export const remarkNormalizeListItemIndentation = attachListItemIndentationNormalizer; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 032a1e05ff6d..ada9a6ea1325 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -806,6 +806,9 @@ importers: micromark-util-character: specifier: ^2.1.1 version: 2.1.1 + remark-gfm: + specifier: ^4.0.1 + version: 4.0.1 remark-parse: specifier: ^11.0.0 version: 11.0.0 From 5014ada6aaa565bc2fea1113b1daa9d3c91570d4 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 3 Sep 2026 23:18:50 -0700 Subject: [PATCH 2/2] fix(chat): preserve citation IDs in selection copy --- apps/web/src/markdown-clipboard.test.ts | 166 +++++++++++++++++- apps/web/src/markdown-clipboard.ts | 12 +- .../client-runtime/src/codexCitations.test.ts | 12 +- packages/client-runtime/src/codexCitations.ts | 4 +- 4 files changed, 186 insertions(+), 8 deletions(-) diff --git a/apps/web/src/markdown-clipboard.test.ts b/apps/web/src/markdown-clipboard.test.ts index 390ab63a9f6e..b9404365c64d 100644 --- a/apps/web/src/markdown-clipboard.test.ts +++ b/apps/web/src/markdown-clipboard.test.ts @@ -1,6 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; -import { serializeRenderedMarkdownFragment } from "./markdown-clipboard"; +import { + chatMarkdownClipboardPayload, + serializeRenderedMarkdownFragment, +} from "./markdown-clipboard"; import { EnvironmentId, MessageId, ThreadId } from "@t3tools/contracts"; import { collectAssistantCitations, @@ -13,13 +16,27 @@ const ELEMENT_NODE = 1; class FakeText { readonly nodeType = TEXT_NODE; readonly childNodes: ReadonlyArray = []; + parentElement: FakeElement | null = null; constructor(readonly textContent: string) {} } +class FakeDocumentFragment { + constructor(readonly childNodes: ReadonlyArray) {} +} + +function escapeHtml(text: string): string { + return text + .replaceAll("&", "&") + .replaceAll('"', """) + .replaceAll("<", "<") + .replaceAll(">", ">"); +} + class FakeElement { readonly nodeType = ELEMENT_NODE; readonly childNodes: Array = []; + parentElement: FakeElement | null = null; readonly classList = { contains: (name: string) => this.classNames.includes(name), }; @@ -38,15 +55,51 @@ class FakeElement { return this.childNodes.map((child) => child.textContent).join(""); } + set textContent(text: string) { + for (const child of this.childNodes) child.parentElement = null; + this.childNodes.length = 0; + this.append(new FakeText(text)); + } + + get innerHTML(): string { + return this.childNodes + .map((child) => (child instanceof FakeText ? escapeHtml(child.textContent) : child.outerHTML)) + .join(""); + } + + get outerHTML(): string { + const attributes = Object.entries(this.attributes) + .map(([name, value]) => ` ${name}="${escapeHtml(value)}"`) + .join(""); + return `<${this.localName}${attributes}>${this.innerHTML}`; + } + get children(): ReadonlyArray { return this.childNodes.filter((child): child is FakeElement => child instanceof FakeElement); } append(...children: Array): this { + for (const child of children) child.parentElement = this; this.childNodes.push(...children); return this; } + appendChild(child: FakeElement | FakeText | FakeDocumentFragment) { + if (child instanceof FakeDocumentFragment) this.append(...child.childNodes); + else this.append(child); + return child; + } + + cloneNode(): FakeElement { + return new FakeElement(this.tagName, this.classNames, this.attributes); + } + + remove() { + const siblings = this.parentElement?.childNodes; + if (siblings) siblings.splice(siblings.indexOf(this), 1); + this.parentElement = null; + } + getAttribute(name: string): string | null { return this.attributes[name] ?? null; } @@ -55,8 +108,29 @@ class FakeElement { return Object.hasOwn(this.attributes, name); } - closest(): FakeElement | null { - return null; + private matches(selector: string): boolean { + if (selector.startsWith(".")) return this.classList.contains(selector.slice(1)); + if (selector.startsWith("[")) { + return [...selector.matchAll(/\[([^\]=]+)(?:="([^"]*)")?\]/g)].every( + ([, attribute, value]) => + attribute !== undefined && + this.hasAttribute(attribute) && + (value === undefined || this.getAttribute(attribute) === value), + ); + } + return this.tagName === selector.toUpperCase(); + } + + closest(selector: string): FakeElement | null { + return this.matches(selector) ? this : (this.parentElement?.closest(selector) ?? null); + } + + querySelectorAll(selector: string): FakeElement[] { + const selectors = selector.split(",").map((part) => part.trim()); + return this.children.flatMap((child) => [ + ...(selectors.some((part) => child.matches(part)) ? [child] : []), + ...child.querySelectorAll(selector), + ]); } /** Supports only the selectors markdown-clipboard actually asks for. */ @@ -274,3 +348,89 @@ describe("serializeRenderedMarkdownFragment", () => { ); }); }); + +/** A native range clones its selected descendants, not their surrounding elements. */ +function selectionFragment( + commonAncestorContainer: FakeElement | FakeText, + ...selectedNodes: Array +): Selection { + return { + rangeCount: 1, + getRangeAt: () => ({ + collapsed: false, + commonAncestorContainer, + cloneContents: () => new FakeDocumentFragment(selectedNodes), + toString: () => selectedNodes.map((node) => node.textContent).join(""), + }), + } as unknown as Selection; +} + +describe("chatMarkdownClipboardPayload", () => { + beforeEach(() => { + vi.stubGlobal("Node", { TEXT_NODE, ELEMENT_NODE }); + vi.stubGlobal("document", { + createElement: (tagName: string) => new FakeElement(tagName.toUpperCase()), + }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it.each([ + { + name: "single source", + numbers: "1", + markdown: "\\[Source 1: turn0view0\\]", + plainText: "[Source 1: turn0view0]", + }, + { + name: "grouped sources with a line range", + numbers: "1, 2", + markdown: "\\[Source 1: turn0view0; Source 2: turn1search2; L8-L13\\]", + plainText: "[Source 1: turn0view0; Source 2: turn1search2; L8-L13]", + }, + ])("copies source details when only $name numbers are selected", (citation) => { + const numbers = new FakeText(citation.numbers); + const chip = new FakeElement("SPAN", [], { + "data-markdown-copy": citation.markdown, + "data-markdown-copy-text": citation.plainText, + }).append(new FakeElement("BUTTON").append(new FakeElement("SPAN").append(numbers))); + new FakeElement("P").append(new FakeText("Before "), chip, new FakeText(" after.")); + + const payload = chatMarkdownClipboardPayload( + selectionFragment(numbers, new FakeText(citation.numbers)), + ); + + expect(payload?.text).toBe(citation.markdown); + expect(payload?.html.replace(/<[^>]*>/g, "")).toBe(citation.plainText); + expect(payload?.html.includes(" { + const prose = new FakeText("The report supports this. "); + const chip = new FakeElement("SPAN", [], { + "data-markdown-copy": "\\[Source 1: turn0view0\\]", + "data-markdown-copy-text": "[Source 1: turn0view0]", + }).append(new FakeElement("BUTTON").append(new FakeText("1"))); + new FakeElement("P").append(prose, chip); + + const payload = chatMarkdownClipboardPayload( + selectionFragment(prose, new FakeText("supports")), + ); + + expect(payload).toEqual({ text: "supports", html: 'supports' }); + }); + + it("does not expand a title selection to an unrelated card's full copy representation", () => { + const title = new FakeText("Quarterly Report"); + new FakeElement("DIV", [], { + "data-markdown-copy": "Quarterly Report (Document template)", + }).append(new FakeElement("SPAN").append(title)); + + const payload = chatMarkdownClipboardPayload(selectionFragment(title, new FakeText("Report"))); + + expect(payload).toEqual({ text: "Report", html: 'Report' }); + }); +}); diff --git a/apps/web/src/markdown-clipboard.ts b/apps/web/src/markdown-clipboard.ts index 429e501a13c5..67db113c2c10 100644 --- a/apps/web/src/markdown-clipboard.ts +++ b/apps/web/src/markdown-clipboard.ts @@ -390,10 +390,20 @@ export function chatMarkdownClipboardPayload( const range = selection.getRangeAt(index); if (range.collapsed) continue; const container = document.createElement("div"); - container.appendChild(range.cloneContents()); const ancestor = range.commonAncestorContainer; const ancestorElement = ancestor.nodeType === Node.ELEMENT_NODE ? (ancestor as Element) : ancestor.parentElement; + const copySource = ancestorElement?.closest("[data-markdown-copy][data-markdown-copy-text]"); + const fragment = range.cloneContents(); + // A label-only range excludes the reference wrapper carrying its copy formats. + // Restore that wrapper on the fragment without changing the live selection. + if (copySource) { + const wrapper = copySource.cloneNode(false); + wrapper.appendChild(fragment); + container.appendChild(wrapper); + } else { + container.appendChild(fragment); + } if (ancestorElement?.closest("pre")) { const text = range.toString(); if (text) { diff --git a/packages/client-runtime/src/codexCitations.test.ts b/packages/client-runtime/src/codexCitations.test.ts index e5a8c52b7e36..d5a137a1e9ce 100644 --- a/packages/client-runtime/src/codexCitations.test.ts +++ b/packages/client-runtime/src/codexCitations.test.ts @@ -113,6 +113,16 @@ describe("remarkCodexCitations", () => { } }); + it.each(["L42", "L8-L13"])("keeps a locator-shaped sole source ID: %s", (id) => { + const source = marker(id); + const located = marker(id, "L5-L8"); + expect(citations(`${source} ${located}`)).toEqual([ + { raw: source, sources: [{ id, number: 1 }] }, + { raw: located, sources: [{ id, number: 1 }], locator: "L5-L8" }, + ]); + expect(renderCodexCitationsAsMarkdown(source)).toBe(`\\[Source 1: ${id}\\]`); + }); + it("handles consecutive markers without swallowing the following text", () => { const markdown = `Before ${SCREENSHOT_CITATION}${marker("second")}, after.`; expect(renderCodexCitationsAsMarkdown(markdown)).toBe( @@ -212,8 +222,6 @@ describe("remarkCodexCitations", () => { "\uE200CITE\uE202turn0view0\uE201", "\uE200cite\uE201", marker(""), - marker("L8"), - marker("L8-L13"), marker("turn0view0", ""), marker("two words"), marker("source.with.dots"), diff --git a/packages/client-runtime/src/codexCitations.ts b/packages/client-runtime/src/codexCitations.ts index 3fa80f3bc399..d45d3316cc42 100644 --- a/packages/client-runtime/src/codexCitations.ts +++ b/packages/client-runtime/src/codexCitations.ts @@ -58,8 +58,8 @@ function parseCitation(raw: string) { const ids = raw.slice(PREFIX.length, -END.length).split(SEPARATOR); if (!ids.every((id) => SOURCE_ID.test(id))) return null; const last = ids.at(-1); - const locator = last !== undefined && LOCATOR.test(last) ? ids.pop() : undefined; - if (ids.length === 0) return null; + const locator = + ids.length > 1 && last !== undefined && LOCATOR.test(last) ? ids.pop() : undefined; return { ids: [...new Set(ids)], ...(locator === undefined ? {} : { locator }) }; }