mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(editor): retry the whole-text patch and refuse escaped dollars as math closers
Second-round review findings that survived the mark rewrite: - A source whose last line is whitespace only (`Last.\n \n`) has a trailing run of one newline, so the body strip ran, landed the end hunk after the spaces, failed the branch-6 proof, and canonicalized the whole file where the base branch had patched it correctly. The body strip is now the first attempt and the whole-text patch the second, so no file does worse than before; the extra round trip only runs when the first attempt fails. - `costs $5 to \$x here` parsed as inline math with latex `5 to \`, because the tokenizer accepted an escaped `\$` as the closing delimiter. Escaped characters are now consumed inside the formula and cannot close it. - The escaped-dollar test passed on the base branch for the wrong reason (the `\$` was deleted before the math tokenizer ran); it now also asserts the bytes round-trip, and a new test pins escapes as ordinary searchable text.
This commit is contained in:
@@ -673,7 +673,25 @@ describe('rich markdown round trip', () => {
|
||||
})
|
||||
|
||||
it('does not turn escaped dollars into inline math', () => {
|
||||
expect(countInlineMathNodes('shell \\$HOME\\$ var and \\$x\\$ too')).toBe(0)
|
||||
const content = 'shell \\$HOME\\$ var, \\$x\\$ too, and costs $5 to \\$x here'
|
||||
expect(countInlineMathNodes(content)).toBe(0)
|
||||
expect(roundTripMarkdown(`${content}\n`)).toBe(content)
|
||||
})
|
||||
|
||||
it('keeps escaped characters as searchable text', () => {
|
||||
const codec = createRichMarkdownEditorCodec()
|
||||
const editor = new Editor({
|
||||
element: null,
|
||||
extensions: createRichMarkdownExtensions({ codec }),
|
||||
content: encodeRawMarkdownHtmlForRichEditor('cost \\$1,200 and file\\_name\n', codec),
|
||||
contentType: 'markdown'
|
||||
})
|
||||
try {
|
||||
// Why: find/replace treats atoms as read-only, so escapes must stay ordinary text.
|
||||
expect(editor.state.doc.textContent).toBe('cost $1,200 and file_name')
|
||||
} finally {
|
||||
editor.destroy()
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps dollar amounts as text instead of inline math', () => {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { InlineMath } from '@tiptap/extension-mathematics'
|
||||
|
||||
// Keep money as text while allowing valid multiline and escaped LaTeX content.
|
||||
const INLINE_MATH = /^\$(?![\s$])([^$]*?[^\s$])\$(?![\d$])/
|
||||
const INLINE_MATH = /^\$(?![\s$])((?:\\[\s\S]|[^$\\])*?)(?<!\s)\$(?![\d$])/
|
||||
|
||||
const baseTokenizer = InlineMath.config.markdownTokenizer
|
||||
if (!baseTokenizer) {
|
||||
|
||||
@@ -440,6 +440,23 @@ describe('serializeRichMarkdownForReconcile (real editor pipeline)', () => {
|
||||
expect(reconciled).toBe('Cost \\$1 for _em_.\n\ntext\n\nAdded')
|
||||
})
|
||||
|
||||
it('keeps source style when appending to a source whose last line is whitespace only', () => {
|
||||
// Why: the trailing run is one newline, so the body strip is tried first; it lands after the
|
||||
// spaces and fails the proof, and the whole-text patch must then take over instead of canonical.
|
||||
const originalSource = 'Cost \\$1 & co.\n\nLast.\n \n'
|
||||
const baseCanonical = serialize(originalSource)!
|
||||
const edited = `${baseCanonical} Added.`
|
||||
|
||||
const reconciled = reconcileSerializedMarkdown({
|
||||
originalSource,
|
||||
baseCanonical,
|
||||
edited,
|
||||
roundTrip: (md) => serialize(md)
|
||||
})
|
||||
|
||||
expect(reconciled).toBe('Cost \\$1 & co.\n\nLast. Added.\n \n')
|
||||
})
|
||||
|
||||
it('keeps source style when deleting the trailing empty paragraph', () => {
|
||||
const originalSource = 'Cost \\$1 for _em_.\n\ntext\n\n'
|
||||
const baseCanonical = serialize(originalSource)!
|
||||
|
||||
@@ -30,6 +30,8 @@ export type ReconcileSerializedMarkdownParams = {
|
||||
roundTrip: (markdown: string) => string | null
|
||||
}
|
||||
|
||||
type PatchAttempt = { source: string; base: string; edited: string; trailing: string }
|
||||
|
||||
export function restoreMarkdownSourceEol(markdown: string, source: string): string {
|
||||
return restoreEol(toLf(markdown), detectDominantEol(source))
|
||||
}
|
||||
@@ -92,24 +94,47 @@ export function reconcileSerializedMarkdown({
|
||||
return canonicalFallback()
|
||||
}
|
||||
|
||||
// Why: dmp's half-match accelerator ignores the diff deadline (100ms+ on repeated seeds), so bail to canonical for highly repetitive replacements.
|
||||
if (hasRepeatedHalfMatchSeed(baseLf, editedLf)) {
|
||||
return canonicalFallback()
|
||||
}
|
||||
|
||||
// Branch 4: run the divergent-base patch entirely in LF space.
|
||||
// Why: when the source ends in one newline that canonical lacks, an end-of-document hunk (whose
|
||||
// trailing context is dmp's end-of-text padding) lands after that newline and fails branch 6, so
|
||||
// patch the bodies and re-attach the run. Any other shape (a trailing empty paragraph is `\n\n`
|
||||
// in canonical too) patches correctly whole.
|
||||
const editedTrailingNewlines = editedLf.match(/\n+$/)?.[0] ?? ''
|
||||
const stripEnd = !baseLf.endsWith('\n') && originalTrailingNewlines.length === 1
|
||||
const sourceBody = stripEnd ? stripTrailingNewlines(originalSourceLf) : originalSourceLf
|
||||
const baseBody = baseLf
|
||||
const editedBody = stripEnd ? stripTrailingNewlines(editedLf) : editedLf
|
||||
const reconciledTrailingNewlines = stripEnd
|
||||
? editedTrailingNewlines + originalTrailingNewlines
|
||||
: ''
|
||||
// Why: dmp's half-match accelerator ignores the diff deadline (100ms+ on repeated seeds), so bail to canonical for highly repetitive replacements.
|
||||
if (hasRepeatedHalfMatchSeed(baseBody, editedBody)) {
|
||||
return canonicalFallback()
|
||||
// try the bodies first and re-attach the run; the whole-text patch (right for every other shape)
|
||||
// stays as the second attempt so no file does worse than before.
|
||||
const attempts: PatchAttempt[] = []
|
||||
if (!baseLf.endsWith('\n') && originalTrailingNewlines.length === 1) {
|
||||
attempts.push({
|
||||
source: stripTrailingNewlines(originalSourceLf),
|
||||
base: baseLf,
|
||||
edited: stripTrailingNewlines(editedLf),
|
||||
trailing: (editedLf.match(/\n+$/)?.[0] ?? '') + originalTrailingNewlines
|
||||
})
|
||||
}
|
||||
let diffs = makeDiff(baseBody, editedBody, {
|
||||
attempts.push({ source: originalSourceLf, base: baseLf, edited: editedLf, trailing: '' })
|
||||
|
||||
for (const attempt of attempts) {
|
||||
// Branch 5: a hunk failed to locate in the non-canonical source → unreliable fuzzy match.
|
||||
const patched = patchDivergentSource(attempt.source, attempt.base, attempt.edited)
|
||||
if (patched === null) {
|
||||
continue
|
||||
}
|
||||
const reconciledLf = patched + attempt.trailing
|
||||
// Branch 6: prove reconciled bytes render-equal the editor's document — any fuzzy misplacement changes canonical output and is caught here.
|
||||
const reparsed = roundTrip(reconciledLf)
|
||||
if (reparsed !== null && normalizeForSafety(reparsed) === normalizeForSafety(editedLf)) {
|
||||
// Restore the detected EOL as the final step so reconciled CRLF stays CRLF.
|
||||
return restoreEol(reconciledLf, eol)
|
||||
}
|
||||
}
|
||||
return canonicalFallback()
|
||||
}
|
||||
|
||||
/** Applies the base→edited diff onto the divergent source; null when a hunk could not be located. */
|
||||
function patchDivergentSource(source: string, base: string, edited: string): string | null {
|
||||
let diffs = makeDiff(base, edited, {
|
||||
checkLines: true,
|
||||
timeout: RECONCILE_DIFF_TIMEOUT_SECONDS
|
||||
})
|
||||
@@ -118,32 +143,18 @@ export function reconcileSerializedMarkdown({
|
||||
diffs = cleanupSemantic(diffs)
|
||||
diffs = cleanupEfficiency(diffs)
|
||||
}
|
||||
const patches = makePatches(baseBody, diffs)
|
||||
const patches = makePatches(base, diffs)
|
||||
// Why: applyPatches decodes starts as UTF-8 offsets even though makePatches returns UTF-16 indices; encode against the divergent text being patched so decoding preserves the fuzzy-match seed.
|
||||
const utf8Offsets = getUtf8OffsetsAtCodeUnitIndices(
|
||||
sourceBody,
|
||||
source,
|
||||
patches.flatMap((patch) => [patch.start1, patch.start2])
|
||||
)
|
||||
for (const patch of patches) {
|
||||
patch.start1 = utf8Offsets.get(patch.start1) ?? 0
|
||||
patch.start2 = utf8Offsets.get(patch.start2) ?? 0
|
||||
}
|
||||
const [reconciledBody, results] = applyPatches(patches, sourceBody)
|
||||
const reconciledLf = reconciledBody + reconciledTrailingNewlines
|
||||
|
||||
// Branch 5: a hunk failed to locate in the non-canonical source → unreliable fuzzy match, fall back to canonical.
|
||||
if (results.some((applied) => !applied)) {
|
||||
return canonicalFallback()
|
||||
}
|
||||
|
||||
// Branch 6: prove reconciled bytes render-equal the editor's document — any fuzzy misplacement changes canonical output and is caught here → canonical fallback.
|
||||
const reparsed = roundTrip(reconciledLf)
|
||||
if (reparsed === null || normalizeForSafety(reparsed) !== normalizeForSafety(editedLf)) {
|
||||
return canonicalFallback()
|
||||
}
|
||||
|
||||
// Restore the detected EOL as the final step so reconciled CRLF stays CRLF.
|
||||
return restoreEol(reconciledLf, eol)
|
||||
const [patched, results] = applyPatches(patches, source)
|
||||
return results.some((applied) => !applied) ? null : patched
|
||||
}
|
||||
|
||||
function stripTrailingNewlines(lfText: string): string {
|
||||
|
||||
Reference in New Issue
Block a user