feat(coding-agent/core): added nested @@ context anchors and hierarchical matching
- Added support for nested @@ context anchors and hierarchical context matching. - Enhanced patch parser to handle @@ line N syntax and multiple context accumulation. - Improved diff rendering and error messages with hierarchical context display. - Added comprehensive regression tests for model-generated patch formats.
This commit is contained in:
can1357
@@ -18,7 +18,7 @@ import {
|
||||
stripBom,
|
||||
} from "./normalize";
|
||||
import { normalizeCreateContent, parseHunks } from "./parser";
|
||||
import type { ApplyPatchOptions, ApplyPatchResult, DiffHunk, FileSystem, PatchInput } from "./types";
|
||||
import type { ApplyPatchOptions, ApplyPatchResult, ContextLineResult, DiffHunk, FileSystem, PatchInput } from "./types";
|
||||
import { ApplyPatchError } from "./types";
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
@@ -117,6 +117,123 @@ function getHunkHintIndex(hunk: DiffHunk, currentIndex: number): number | undefi
|
||||
return hintIndex >= currentIndex ? hintIndex : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Find hierarchical context in file lines.
|
||||
*
|
||||
* Handles three formats:
|
||||
* 1. Simple context: "function foo" - find this line
|
||||
* 2. Hierarchical (newline): "class Foo\nmethod" - find class, then method after it
|
||||
* 3. Hierarchical (space): "class Foo method" - try as literal first, then split and search
|
||||
*
|
||||
* @returns The result from finding the final (innermost) context, or undefined if not found
|
||||
*/
|
||||
function findHierarchicalContext(
|
||||
lines: string[],
|
||||
context: string,
|
||||
startFrom: number,
|
||||
lineHint: number | undefined,
|
||||
): ContextLineResult {
|
||||
// Check for newline-separated hierarchical contexts (from nested @@ anchors)
|
||||
if (context.includes("\n")) {
|
||||
const parts = context
|
||||
.split("\n")
|
||||
.map((p) => p.trim())
|
||||
.filter((p) => p.length > 0);
|
||||
let currentStart = startFrom;
|
||||
|
||||
for (let i = 0; i < parts.length; i++) {
|
||||
const part = parts[i];
|
||||
const isLast = i === parts.length - 1;
|
||||
|
||||
// For intermediate contexts, we just need to find them and continue from there
|
||||
// For the final context, we return the full result with match count
|
||||
const result = findContextLine(lines, part, currentStart);
|
||||
|
||||
if (result.index === undefined) {
|
||||
// Try from beginning if not found from current position
|
||||
const fromStartResult = findContextLine(lines, part, 0);
|
||||
if (fromStartResult.index === undefined) {
|
||||
return { index: undefined, confidence: 0 };
|
||||
}
|
||||
currentStart = fromStartResult.index + 1;
|
||||
if (isLast) {
|
||||
// Hierarchical matching narrows scope - treat as single match
|
||||
return { ...fromStartResult, matchCount: 1 };
|
||||
}
|
||||
} else {
|
||||
currentStart = result.index + 1;
|
||||
if (isLast) {
|
||||
// Hierarchical matching narrows scope - treat as single match
|
||||
return { ...result, matchCount: 1 };
|
||||
}
|
||||
}
|
||||
}
|
||||
return { index: undefined, confidence: 0 };
|
||||
}
|
||||
|
||||
// Try literal context first
|
||||
let result = findContextLine(lines, context, startFrom);
|
||||
|
||||
// If line hint exists and result is ambiguous or missing, try from hint
|
||||
if ((result.index === undefined || (result.matchCount ?? 0) > 1) && lineHint !== undefined) {
|
||||
const hintStart = Math.max(0, lineHint - 1);
|
||||
const hintedResult = findContextLine(lines, context, hintStart);
|
||||
if (hintedResult.index !== undefined) {
|
||||
// Line hint successfully found a match - treat as disambiguated
|
||||
return { ...hintedResult, matchCount: 1 };
|
||||
}
|
||||
}
|
||||
|
||||
// If found uniquely, return it
|
||||
if (result.index !== undefined && (result.matchCount ?? 0) <= 1) {
|
||||
return result;
|
||||
}
|
||||
|
||||
// Try from beginning if not found
|
||||
if (result.index === undefined && startFrom !== 0) {
|
||||
result = findContextLine(lines, context, 0);
|
||||
if (result.index !== undefined && (result.matchCount ?? 0) <= 1) {
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback: try space-separated hierarchical matching
|
||||
// e.g., "class PatchTool constructor" -> find "class PatchTool", then "constructor" after it
|
||||
const spaceParts = context.split(/\s+/).filter((p) => p.length > 0);
|
||||
if (spaceParts.length > 1) {
|
||||
let currentStart = startFrom;
|
||||
|
||||
for (let i = 0; i < spaceParts.length; i++) {
|
||||
const part = spaceParts[i];
|
||||
const isLast = i === spaceParts.length - 1;
|
||||
|
||||
const partResult = findContextLine(lines, part, currentStart);
|
||||
|
||||
if (partResult.index === undefined) {
|
||||
// Try from beginning
|
||||
const fromStartResult = findContextLine(lines, part, 0);
|
||||
if (fromStartResult.index === undefined) {
|
||||
// Space-separated fallback failed, return original result
|
||||
return result;
|
||||
}
|
||||
currentStart = fromStartResult.index + 1;
|
||||
if (isLast) {
|
||||
// Hierarchical matching narrows scope - treat as single match
|
||||
return { ...fromStartResult, matchCount: 1 };
|
||||
}
|
||||
} else {
|
||||
currentStart = partResult.index + 1;
|
||||
if (isLast) {
|
||||
// Hierarchical matching narrows scope - treat as single match
|
||||
return { ...partResult, matchCount: 1 };
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/** Find sequence with optional hint position, returning full search result */
|
||||
function findSequenceWithHint(
|
||||
lines: string[],
|
||||
@@ -224,49 +341,34 @@ function computeReplacements(originalLines: string[], path: string, hunks: DiffH
|
||||
|
||||
// If hunk has a changeContext, find it and adjust lineIndex
|
||||
if (hunk.changeContext !== undefined) {
|
||||
// Use findContextLine for robust matching with substring/fuzzy fallback
|
||||
let result = findContextLine(originalLines, hunk.changeContext, lineIndex);
|
||||
let usedLineHint = false;
|
||||
// Use hierarchical context matching for nested @@ anchors and space-separated contexts
|
||||
const result = findHierarchicalContext(originalLines, hunk.changeContext, lineIndex, lineHint);
|
||||
|
||||
// If ambiguous or missing and a line hint exists, bias using the hint
|
||||
if ((result.index === undefined || (result.matchCount ?? 0) > 1) && lineHint !== undefined) {
|
||||
const hintStart = Math.max(0, lineHint - 1);
|
||||
const hintedResult = findContextLine(originalLines, hunk.changeContext, hintStart);
|
||||
if (hintedResult.index !== undefined) {
|
||||
result = hintedResult;
|
||||
usedLineHint = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (!usedLineHint && result.matchCount !== undefined && result.matchCount > 1) {
|
||||
if (result.matchCount !== undefined && result.matchCount > 1) {
|
||||
const displayContext = hunk.changeContext.includes("\n")
|
||||
? hunk.changeContext.split("\n").pop()
|
||||
: hunk.changeContext;
|
||||
throw new ApplyPatchError(
|
||||
`Found ${result.matchCount} matches for context '${hunk.changeContext}' in ${path}. ` +
|
||||
`Found ${result.matchCount} matches for context '${displayContext}' in ${path}. ` +
|
||||
`Add more surrounding context or additional @@ anchors to make it unique.`,
|
||||
);
|
||||
}
|
||||
|
||||
// If search failed, try fallback strategies
|
||||
let idx = result.index;
|
||||
if (idx === undefined && lineIndex !== 0) {
|
||||
// Last resort: search from beginning (handles out-of-order hunks)
|
||||
const fromStartResult = findContextLine(originalLines, hunk.changeContext, 0);
|
||||
if (fromStartResult.matchCount !== undefined && fromStartResult.matchCount > 1) {
|
||||
throw new ApplyPatchError(
|
||||
`Found ${fromStartResult.matchCount} matches for context '${hunk.changeContext}' in ${path}. ` +
|
||||
`Add more surrounding context or additional @@ anchors to make it unique.`,
|
||||
);
|
||||
}
|
||||
idx = fromStartResult.index;
|
||||
}
|
||||
|
||||
const idx = result.index;
|
||||
if (idx === undefined) {
|
||||
throw new ApplyPatchError(`Failed to find context '${hunk.changeContext}' in ${path}`);
|
||||
const displayContext = hunk.changeContext.includes("\n")
|
||||
? hunk.changeContext.split("\n").join(" > ")
|
||||
: hunk.changeContext;
|
||||
throw new ApplyPatchError(`Failed to find context '${displayContext}' in ${path}`);
|
||||
}
|
||||
|
||||
// If oldLines[0] matches changeContext, start search at idx (not idx+1)
|
||||
// If oldLines[0] matches the final context, start search at idx (not idx+1)
|
||||
// This handles the common case where @@ scope and first context line are identical
|
||||
const firstOldLine = hunk.oldLines[0];
|
||||
if (firstOldLine !== undefined && firstOldLine.trim() === hunk.changeContext.trim()) {
|
||||
const finalContext = hunk.changeContext.includes("\n")
|
||||
? hunk.changeContext.split("\n").pop()?.trim()
|
||||
: hunk.changeContext.trim();
|
||||
if (firstOldLine !== undefined && firstOldLine.trim() === finalContext) {
|
||||
lineIndex = idx;
|
||||
} else {
|
||||
lineIndex = idx + 1;
|
||||
|
||||
@@ -21,6 +21,9 @@ const EMPTY_CHANGE_CONTEXT_MARKER = "@@";
|
||||
/** Regex to match unified diff hunk headers: @@ -OLD,COUNT +NEW,COUNT @@ optional-context */
|
||||
const UNIFIED_HUNK_HEADER_REGEX = /^@@\s*-(\d+)(?:,(\d+))?\s+\+(\d+)(?:,(\d+))?\s*@@(?:\s*(.*))?$/;
|
||||
|
||||
/** Regex to match @@ line N pattern (model-generated line hints) */
|
||||
const LINE_HINT_REGEX = /^line\s+(\d+)$/i;
|
||||
|
||||
/**
|
||||
* Check if a line is a diff content line (context, addition, or removal).
|
||||
* These should never be treated as metadata even if their content looks like it.
|
||||
@@ -193,13 +196,20 @@ interface ParseHunkResult {
|
||||
|
||||
/**
|
||||
* Parse a single hunk from lines starting at the current position.
|
||||
*
|
||||
* Handles several context formats:
|
||||
* - Empty: `@@` (no context, match from current position)
|
||||
* - Unified: `@@ -10,3 +10,3 @@` (line numbers as hints)
|
||||
* - Context: `@@ function foo` (search for context line)
|
||||
* - Line hint: `@@ line 125` (use line 125 as starting position)
|
||||
* - Nested: `@@ class Foo\n@@ method` (hierarchical context search)
|
||||
*/
|
||||
function parseOneHunk(lines: string[], lineNumber: number, allowMissingContext: boolean): ParseHunkResult {
|
||||
if (lines.length === 0) {
|
||||
throw new ParseError("Diff does not contain any lines", lineNumber);
|
||||
}
|
||||
|
||||
let changeContext: string | undefined;
|
||||
const changeContexts: string[] = [];
|
||||
let oldStartLine: number | undefined;
|
||||
let newStartLine: number | undefined;
|
||||
let startIndex: number;
|
||||
@@ -209,28 +219,61 @@ function parseOneHunk(lines: string[], lineNumber: number, allowMissingContext:
|
||||
|
||||
// Check for context marker
|
||||
if (headerLine === EMPTY_CHANGE_CONTEXT_MARKER) {
|
||||
changeContext = undefined;
|
||||
startIndex = 1;
|
||||
} else if (unifiedHeader) {
|
||||
changeContext = unifiedHeader.changeContext;
|
||||
if (unifiedHeader.changeContext) {
|
||||
changeContexts.push(unifiedHeader.changeContext);
|
||||
}
|
||||
oldStartLine = unifiedHeader.oldStartLine;
|
||||
newStartLine = unifiedHeader.newStartLine;
|
||||
startIndex = 1;
|
||||
} else if (headerLine.startsWith(CHANGE_CONTEXT_MARKER)) {
|
||||
changeContext = headerLine.slice(CHANGE_CONTEXT_MARKER.length);
|
||||
const contextValue = headerLine.slice(CHANGE_CONTEXT_MARKER.length);
|
||||
|
||||
// Check for @@ line N pattern (model-generated line hints)
|
||||
const lineHintMatch = contextValue.match(LINE_HINT_REGEX);
|
||||
if (lineHintMatch) {
|
||||
oldStartLine = Number(lineHintMatch[1]);
|
||||
newStartLine = oldStartLine;
|
||||
} else {
|
||||
changeContexts.push(contextValue);
|
||||
}
|
||||
startIndex = 1;
|
||||
} else {
|
||||
if (!allowMissingContext) {
|
||||
throw new ParseError(`Expected hunk to start with @@ context marker, got: '${lines[0]}'`, lineNumber);
|
||||
}
|
||||
changeContext = undefined;
|
||||
startIndex = 0;
|
||||
}
|
||||
|
||||
// Check for nested @@ anchors on subsequent lines
|
||||
// Format: @@ class Foo
|
||||
// @@ method
|
||||
while (startIndex < lines.length) {
|
||||
const nextLine = lines[startIndex];
|
||||
const trimmed = nextLine.trim();
|
||||
|
||||
// Check if it's another @@ line (nested anchor)
|
||||
if (trimmed.startsWith(CHANGE_CONTEXT_MARKER)) {
|
||||
const nestedContext = trimmed.slice(CHANGE_CONTEXT_MARKER.length);
|
||||
changeContexts.push(nestedContext);
|
||||
startIndex++;
|
||||
} else if (trimmed === EMPTY_CHANGE_CONTEXT_MARKER) {
|
||||
// Empty @@ as separator - skip it
|
||||
startIndex++;
|
||||
} else {
|
||||
// Not an @@ line, stop accumulating
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (startIndex >= lines.length) {
|
||||
throw new ParseError("Hunk does not contain any lines", lineNumber + 1);
|
||||
}
|
||||
|
||||
// Combine contexts: if multiple, join with newline for hierarchical matching
|
||||
const changeContext = changeContexts.length > 0 ? changeContexts.join("\n") : undefined;
|
||||
|
||||
const hunk: DiffHunk = {
|
||||
changeContext,
|
||||
oldStartLine,
|
||||
|
||||
@@ -6,9 +6,9 @@ import { theme } from "../theme/theme";
|
||||
* Format: "+123 content" or "-123 content" or " 123 content" or " ..."
|
||||
*/
|
||||
function parseDiffLine(line: string): { prefix: string; lineNum: string; content: string } | null {
|
||||
const match = line.match(/^([+-\s])(\s*\d*)\s(.*)$/);
|
||||
const match = line.match(/^([+-\s])(?:(\s*\d+)\s)?(.*)$/);
|
||||
if (!match) return null;
|
||||
return { prefix: match[1], lineNum: match[2], content: match[3] };
|
||||
return { prefix: match[1], lineNum: match[2] ?? "", content: match[3] };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -80,6 +80,13 @@ export function renderDiff(diffText: string, _options: RenderDiffOptions = {}):
|
||||
const lines = diffText.split("\n");
|
||||
const result: string[] = [];
|
||||
|
||||
const formatLine = (prefix: string, lineNum: string, content: string): string => {
|
||||
if (lineNum.trim().length === 0) {
|
||||
return `${prefix}${content}`;
|
||||
}
|
||||
return `${prefix}${lineNum} ${content}`;
|
||||
};
|
||||
|
||||
let i = 0;
|
||||
while (i < lines.length) {
|
||||
const line = lines[i];
|
||||
@@ -121,24 +128,26 @@ export function renderDiff(diffText: string, _options: RenderDiffOptions = {}):
|
||||
replaceTabs(added.content),
|
||||
);
|
||||
|
||||
result.push(theme.fg("toolDiffRemoved", `-${removed.lineNum} ${removedLine}`));
|
||||
result.push(theme.fg("toolDiffAdded", `+${added.lineNum} ${addedLine}`));
|
||||
result.push(theme.fg("toolDiffRemoved", formatLine("-", removed.lineNum, removedLine)));
|
||||
result.push(theme.fg("toolDiffAdded", formatLine("+", added.lineNum, addedLine)));
|
||||
} else {
|
||||
// Show all removed lines first, then all added lines
|
||||
for (const removed of removedLines) {
|
||||
result.push(theme.fg("toolDiffRemoved", `-${removed.lineNum} ${replaceTabs(removed.content)}`));
|
||||
result.push(
|
||||
theme.fg("toolDiffRemoved", formatLine("-", removed.lineNum, replaceTabs(removed.content))),
|
||||
);
|
||||
}
|
||||
for (const added of addedLines) {
|
||||
result.push(theme.fg("toolDiffAdded", `+${added.lineNum} ${replaceTabs(added.content)}`));
|
||||
result.push(theme.fg("toolDiffAdded", formatLine("+", added.lineNum, replaceTabs(added.content))));
|
||||
}
|
||||
}
|
||||
} else if (parsed.prefix === "+") {
|
||||
// Standalone added line
|
||||
result.push(theme.fg("toolDiffAdded", `+${parsed.lineNum} ${replaceTabs(parsed.content)}`));
|
||||
result.push(theme.fg("toolDiffAdded", formatLine("+", parsed.lineNum, replaceTabs(parsed.content))));
|
||||
i++;
|
||||
} else {
|
||||
// Context line
|
||||
result.push(theme.fg("toolDiffContext", ` ${parsed.lineNum} ${replaceTabs(parsed.content)}`));
|
||||
result.push(theme.fg("toolDiffContext", formatLine(" ", parsed.lineNum, replaceTabs(parsed.content))));
|
||||
i++;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user