mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 16:03:47 +00:00
fix(native-chat): stop parsing user prose as link reference definitions
Escaping a line-leading bracket only reached definitions at the top level. Measured against the live renderer, a definition-shaped marker line still rendered as an empty bubble inside a bullet, an ordered item or a blockquote, and whenever the label wrapped across lines — and the escape injected a visible backslash into a fenced block whose fence characters differed from an inner fence line. Turn the construct off for user rows instead, so every container is covered and the source text is never rewritten. Inline links, reference links, GFM and fenced code all render byte-identically.
This commit is contained in:
@@ -17,7 +17,6 @@ import { stripNoiseMessages } from './native-chat-noise'
|
||||
import { foldToolMessages, splitNativeChatBlocks } from './native-chat-tool-fold'
|
||||
import { isNearBottom, shouldShowJumpToLatest, type ScrollGeometry } from './native-chat-autoscroll'
|
||||
import { isNativeChatPastedImagePath } from './native-chat-image-paste'
|
||||
import { escapeNativeChatUserMarkdown } from './native-chat-user-markdown'
|
||||
import { NativeChatToolRun } from './NativeChatToolRun'
|
||||
import { NativeChatCopyButton } from './NativeChatCopyButton'
|
||||
import { NATIVE_CHAT_STREAMING_ID } from '../../../../shared/native-chat-streaming'
|
||||
@@ -175,11 +174,14 @@ function MessageRow({
|
||||
<>
|
||||
<ImageAttachmentRefs blocks={prose} />
|
||||
<CommentMarkdown
|
||||
content={escapeNativeChatUserMarkdown(markdown)}
|
||||
content={markdown}
|
||||
variant="document"
|
||||
className="text-sm"
|
||||
onLinkClick={onLinkClick}
|
||||
allowFileUriLinks={allowFileUriLinks}
|
||||
// A user turn is prose, so `[Image #1]: /tmp/a.png` must render
|
||||
// as typed rather than parse away as a link definition.
|
||||
disableLinkDefinitions
|
||||
/>
|
||||
</>
|
||||
) : (
|
||||
|
||||
@@ -1,54 +0,0 @@
|
||||
import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import CommentMarkdown from '@/components/sidebar/CommentMarkdown'
|
||||
import { escapeNativeChatUserMarkdown } from './native-chat-user-markdown'
|
||||
|
||||
function renderedText(content: string): string {
|
||||
return renderToStaticMarkup(<CommentMarkdown variant="document" content={content} />)
|
||||
.replace(/<[^>]*>/g, '')
|
||||
.trim()
|
||||
}
|
||||
|
||||
describe('escapeNativeChatUserMarkdown', () => {
|
||||
// The swallow this guards against: without the escape the marker line is a
|
||||
// CommonMark link reference definition and the whole turn renders empty.
|
||||
it('keeps a marker line the raw Markdown pipeline drops entirely', () => {
|
||||
const typed = '[Image #1]: /tmp/a.png'
|
||||
|
||||
expect(renderedText(typed)).toBe('')
|
||||
expect(renderedText(escapeNativeChatUserMarkdown(typed))).toBe('[Image #1]: /tmp/a.png')
|
||||
})
|
||||
|
||||
it('keeps a definition-shaped line that precedes other prose', () => {
|
||||
const typed = '[Image #1]: /tmp/a.png\n\nwhat do you make of it?'
|
||||
|
||||
expect(renderedText(typed)).toBe('what do you make of it?')
|
||||
expect(renderedText(escapeNativeChatUserMarkdown(typed))).toBe(
|
||||
'[Image #1]: /tmp/a.png\nwhat do you make of it?'
|
||||
)
|
||||
})
|
||||
|
||||
it.each([
|
||||
'keep [Image #1] literal',
|
||||
'[Image #1] describe this',
|
||||
'here is my note\n[Image #1]: /tmp/a.png\nend',
|
||||
'[Image #1]: /tmp/a.png and more text after',
|
||||
'```\n[Image #1]: /tmp/a.png\n```',
|
||||
'- [Image #1]: /tmp/a.png',
|
||||
'plain prose with a colon: like this'
|
||||
])('renders text the pipeline already showed identically', (typed) => {
|
||||
expect(renderedText(escapeNativeChatUserMarkdown(typed))).toBe(renderedText(typed))
|
||||
})
|
||||
|
||||
it.each(['keep [Image #1] literal', '```\n[Image #1]: /tmp/a.png\n```'])(
|
||||
'leaves lines that cannot open a definition byte-identical',
|
||||
(typed) => {
|
||||
expect(escapeNativeChatUserMarkdown(typed)).toBe(typed)
|
||||
}
|
||||
)
|
||||
|
||||
it('escapes only the line-leading bracket, once per line', () => {
|
||||
expect(escapeNativeChatUserMarkdown('[a]: x\n[b]: y')).toBe('\\[a]: x\n\\[b]: y')
|
||||
expect(escapeNativeChatUserMarkdown(' [a]: x')).toBe(' \\[a]: x')
|
||||
})
|
||||
})
|
||||
@@ -1,25 +0,0 @@
|
||||
const LINK_REFERENCE_DEFINITION_LINE = /^( {0,3})\[(?=[^\]\n]*\]:)/
|
||||
const FENCE_LINE = /^ {0,3}(?:`{3,}|~{3,})/
|
||||
|
||||
/**
|
||||
* A user turn is prose, not authored Markdown. CommonMark reads a line like
|
||||
* `[Image #1]: /tmp/a.png` as a link reference definition and renders nothing at
|
||||
* all, so a marker the transcript rules preserve would still vanish on screen.
|
||||
* Escaping the opening bracket is output-neutral for every other line.
|
||||
*/
|
||||
export function escapeNativeChatUserMarkdown(markdown: string): string {
|
||||
if (!markdown.includes(']:')) {
|
||||
return markdown
|
||||
}
|
||||
let inFence = false
|
||||
return markdown
|
||||
.split('\n')
|
||||
.map((line) => {
|
||||
if (FENCE_LINE.test(line)) {
|
||||
inFence = !inFence
|
||||
return line
|
||||
}
|
||||
return inFence ? line : line.replace(LINK_REFERENCE_DEFINITION_LINE, '$1\\[')
|
||||
})
|
||||
.join('\n')
|
||||
}
|
||||
@@ -0,0 +1,57 @@
|
||||
import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { NativeChatMessage } from '../../../../shared/native-chat-types'
|
||||
import { NativeChatMessageList } from './NativeChatMessageList'
|
||||
import type { NativeChatLiveSession } from './use-native-chat-live-session'
|
||||
|
||||
function sessionOf(messages: NativeChatMessage[]): NativeChatLiveSession {
|
||||
return {
|
||||
messages,
|
||||
status: 'ready',
|
||||
sessionId: 's1',
|
||||
agent: 'claude',
|
||||
hasMore: false,
|
||||
loadingEarlier: false,
|
||||
loadEarlier: () => {},
|
||||
readPhase: 'ready'
|
||||
}
|
||||
}
|
||||
|
||||
function userTurn(text: string): NativeChatMessage {
|
||||
return {
|
||||
id: 'u1',
|
||||
role: 'user',
|
||||
blocks: [{ type: 'text', text }],
|
||||
timestamp: 1,
|
||||
source: 'transcript'
|
||||
}
|
||||
}
|
||||
|
||||
function renderedText(text: string): string {
|
||||
return renderToStaticMarkup(
|
||||
<NativeChatMessageList
|
||||
session={sessionOf([userTurn(text)])}
|
||||
isWorking={false}
|
||||
expandSignal={false}
|
||||
fontScale={1}
|
||||
/>
|
||||
).replace(/<[^>]*>/g, '')
|
||||
}
|
||||
|
||||
describe('user turn rendering', () => {
|
||||
// Wiring guard: the row must opt out of link reference definitions, or a
|
||||
// marker line the transcript rules deliberately preserved still renders as
|
||||
// an empty bubble.
|
||||
it.each([
|
||||
'[Image #1]: /tmp/a.png',
|
||||
'- [Image #1]: /tmp/a.png',
|
||||
'> [Image #1]: /tmp/a.png',
|
||||
'[Image #1\nstill mine]: /tmp/a.png'
|
||||
])('shows a definition-shaped user line on screen', (typed) => {
|
||||
expect(renderedText(typed)).toContain('[Image #1')
|
||||
})
|
||||
|
||||
it('shows a literal marker the user typed without an attachment', () => {
|
||||
expect(renderedText('keep [Image #1] literal')).toContain('keep [Image #1] literal')
|
||||
})
|
||||
})
|
||||
@@ -5,6 +5,7 @@ import remarkBreaks from 'remark-breaks'
|
||||
import rehypeRaw from 'rehype-raw'
|
||||
import rehypeSanitize, { defaultSchema } from 'rehype-sanitize'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { remarkDisableDefinitions } from './remark-disable-definitions'
|
||||
import {
|
||||
compactCommentMarkdownComponents,
|
||||
createCompactCommentMarkdownComponents,
|
||||
@@ -186,6 +187,9 @@ type CommentMarkdownProps = React.ComponentPropsWithoutRef<'div'> & {
|
||||
onLinkClick?: CommentMarkdownLinkClickHandler
|
||||
allowFileUriLinks?: boolean
|
||||
expandImages?: boolean
|
||||
/** Set when `content` is prose the user typed, not Markdown they authored, so
|
||||
* a `[label]: target` line renders instead of silently disappearing. */
|
||||
disableLinkDefinitions?: boolean
|
||||
}
|
||||
|
||||
// Why forwardRef + rest props: Radix's HoverCardTrigger asChild merges a ref
|
||||
@@ -201,6 +205,7 @@ const CommentMarkdown = React.memo(
|
||||
onLinkClick,
|
||||
allowFileUriLinks = false,
|
||||
expandImages = false,
|
||||
disableLinkDefinitions = false,
|
||||
...rest
|
||||
},
|
||||
ref
|
||||
@@ -217,10 +222,12 @@ const CommentMarkdown = React.memo(
|
||||
? createDocumentCommentMarkdownComponents(onLinkClick)
|
||||
: createCompactCommentMarkdownComponents(onLinkClick, expandImages)
|
||||
}, [expandImages, variant, onLinkClick])
|
||||
const activeRemarkPlugins = React.useMemo(
|
||||
() => (githubRepo ? [...remarkPlugins, remarkGitHubReferences(githubRepo)] : remarkPlugins),
|
||||
[githubRepo]
|
||||
)
|
||||
const activeRemarkPlugins = React.useMemo(() => {
|
||||
const active = githubRepo
|
||||
? [...remarkPlugins, remarkGitHubReferences(githubRepo)]
|
||||
: remarkPlugins
|
||||
return disableLinkDefinitions ? [...active, remarkDisableDefinitions] : active
|
||||
}, [disableLinkDefinitions, githubRepo])
|
||||
|
||||
return (
|
||||
<div
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import CommentMarkdown from './CommentMarkdown'
|
||||
|
||||
function rendered(content: string, disableLinkDefinitions: boolean): string {
|
||||
return renderToStaticMarkup(
|
||||
<CommentMarkdown
|
||||
variant="document"
|
||||
content={content}
|
||||
disableLinkDefinitions={disableLinkDefinitions}
|
||||
/>
|
||||
)
|
||||
.replace(/<[^>]*>/g, '')
|
||||
.trim()
|
||||
}
|
||||
|
||||
describe('disableLinkDefinitions', () => {
|
||||
// Each of these renders to nothing at all without the opt-in — the user's
|
||||
// whole line disappears, which is the defect, not a formatting nicety.
|
||||
it.each([
|
||||
['at the top level', '[Image #1]: /tmp/a.png', '[Image #1]: /tmp/a.png'],
|
||||
['inside a bullet', '- [Image #1]: /tmp/a.png', '[Image #1]: /tmp/a.png'],
|
||||
['inside an ordered item', '1. [Image #1]: /tmp/a.png', '[Image #1]: /tmp/a.png'],
|
||||
['inside a blockquote', '> [Image #1]: /tmp/a.png', '[Image #1]: /tmp/a.png'],
|
||||
// A link label may span lines, so the swallow is not a single-line shape.
|
||||
[
|
||||
'with a wrapped label',
|
||||
'[Image #1\nstill mine]: /tmp/a.png',
|
||||
'[Image #1\nstill mine]: /tmp/a.png'
|
||||
]
|
||||
])('keeps a definition-shaped line %s', (_name, typed, expected) => {
|
||||
expect(rendered(typed, false)).toBe('')
|
||||
expect(rendered(typed, true)).toBe(expected)
|
||||
})
|
||||
|
||||
it('keeps the sibling content a swallowed definition took with it', () => {
|
||||
const typed = '- [Image #1]: /tmp/a.png\n- second'
|
||||
|
||||
expect(rendered(typed, false)).toBe('second')
|
||||
expect(rendered(typed, true)).toBe('[Image #1]: /tmp/a.png\nsecond')
|
||||
})
|
||||
|
||||
it.each([
|
||||
'keep [Image #1] literal',
|
||||
'plain prose with a colon: like this',
|
||||
'see [the docs](https://example.com) here',
|
||||
'see [the docs][d] here',
|
||||
'```\n[Image #1]: /tmp/a.png\n```',
|
||||
'~~~\ncode\n```\n[Image #1]: /tmp/a.png\n~~~',
|
||||
'| a | b |\n| - | - |\n| 1 | 2 |'
|
||||
])('leaves content the pipeline already rendered untouched', (typed) => {
|
||||
expect(rendered(typed, true)).toBe(rendered(typed, false))
|
||||
})
|
||||
|
||||
// The one intended difference: an authored definition stops being consumed,
|
||||
// so its reference renders literally rather than as a resolved link.
|
||||
it('renders an authored definition as the text it is', () => {
|
||||
const typed = '[d]: https://example.com\n\nsee [the docs][d] here'
|
||||
|
||||
expect(rendered(typed, false)).toBe('see the docs here')
|
||||
expect(rendered(typed, true)).toBe('[d]: https://example.com\nsee [the docs][d] here')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,19 @@
|
||||
import type { Processor } from 'unified'
|
||||
|
||||
/**
|
||||
* Stop CommonMark from reading `[label]: /some/target` as a link reference
|
||||
* definition.
|
||||
*
|
||||
* Definitions render to nothing at all, so on content that is prose rather than
|
||||
* authored Markdown — a chat turn the user typed — such a line silently deletes
|
||||
* itself. Escaping the bracket in the source text only reaches lines at the top
|
||||
* level; a definition inside a list item or blockquote, or one whose label wraps
|
||||
* across lines, is still swallowed. Turning the construct off covers every
|
||||
* container, and leaves inline links, reference *links*, and GFM intact.
|
||||
*/
|
||||
export function remarkDisableDefinitions(this: Processor): undefined {
|
||||
const data = this.data()
|
||||
const extensions = (data.micromarkExtensions ??= [])
|
||||
extensions.push({ disable: { null: ['definition'] } })
|
||||
return undefined
|
||||
}
|
||||
Reference in New Issue
Block a user