fix(edit): prevented fuzzy replace-all self-matching
Matched fuzzy candidates against immutable source text while excluding ranges already selected for replacement. Inserted replacement content can no longer become a later exact or fuzzy candidate. Added regression coverage for multiple fuzzy source matches when the replacement contains the original search text. Fixes #7432
This commit is contained in:
@@ -24,6 +24,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed fuzzy replace-all edits re-matching replacement text indefinitely, which could freeze the TUI and exhaust memory ([#7432](https://github.com/can1357/oh-my-pi/issues/7432)).
|
||||
- Fixed template argument substitution (`substituteArgs`) executing recursive placeholder expansion when positional argument values contain literal `$@` or `$ARGUMENTS` tokens.
|
||||
- Fixed focused-agent status bar dimming darkening Powerline end caps.
|
||||
- Fixed the browser relay creating duplicate "omp" tab groups: the bridge now keeps at most one group RPC in flight (a queued drain replaces fire-and-forget per-tab requests), so concurrent requests can no longer race the extension's non-atomic query→create→set-title sequence in the same window. Also fixed an extension reconnect (relay daemon restart, service-worker recycle) being misread as the user dragging every tab out of the omp group — grouping state is reset when the extension socket closes, so tabs regroup on the next hello instead of being permanently opted out.
|
||||
|
||||
@@ -855,7 +855,6 @@ export function replaceText(content: string, oldText: string, newText: string, o
|
||||
let normalizedContent = normalizeToLF(content);
|
||||
const normalizedOldText = normalizeToLF(oldText);
|
||||
const normalizedNewText = normalizeToLF(newText);
|
||||
let count = 0;
|
||||
|
||||
if (options.all) {
|
||||
// Check for exact matches first
|
||||
@@ -867,11 +866,13 @@ export function replaceText(content: string, oldText: string, newText: string, o
|
||||
};
|
||||
}
|
||||
|
||||
// No exact matches - try fuzzy matching iteratively
|
||||
// Match against the immutable source so inserted replacement text cannot become a later candidate.
|
||||
const replacements: Array<{ startIndex: number; endIndex: number; text: string }> = [];
|
||||
while (true) {
|
||||
const matchOutcome = findMatch(normalizedContent, normalizedOldText, {
|
||||
allowFuzzy: options.fuzzy,
|
||||
threshold,
|
||||
excludedRanges: replacements,
|
||||
});
|
||||
|
||||
const shouldUseClosest =
|
||||
@@ -888,14 +889,22 @@ export function replaceText(content: string, oldText: string, newText: string, o
|
||||
if (adjustedNewText === match.actualText) {
|
||||
break;
|
||||
}
|
||||
normalizedContent =
|
||||
normalizedContent.substring(0, match.startIndex) +
|
||||
adjustedNewText +
|
||||
normalizedContent.substring(match.startIndex + match.actualText.length);
|
||||
count++;
|
||||
replacements.push({
|
||||
startIndex: match.startIndex,
|
||||
endIndex: match.startIndex + Math.max(match.actualText.length, 1),
|
||||
text: adjustedNewText,
|
||||
});
|
||||
}
|
||||
|
||||
return { content: normalizedContent, count };
|
||||
replacements.sort((a, b) => a.startIndex - b.startIndex);
|
||||
const parts: string[] = [];
|
||||
let sourceIndex = 0;
|
||||
for (const replacement of replacements) {
|
||||
parts.push(normalizedContent.substring(sourceIndex, replacement.startIndex), replacement.text);
|
||||
sourceIndex = replacement.endIndex;
|
||||
}
|
||||
parts.push(normalizedContent.substring(sourceIndex));
|
||||
return { content: parts.join(""), count: replacements.length };
|
||||
}
|
||||
|
||||
// Single replacement mode
|
||||
|
||||
@@ -258,23 +258,41 @@ function formatPreviewWindow(lines: string[], centerIndex: number, options: Prev
|
||||
.join("\n");
|
||||
}
|
||||
|
||||
function findExactMatchOutcome(content: string, target: string): MatchOutcome | undefined {
|
||||
const exactIndex = content.indexOf(target);
|
||||
if (exactIndex === -1) {
|
||||
function findExactMatchOutcome(
|
||||
content: string,
|
||||
target: string,
|
||||
excludedRanges: readonly { startIndex: number; endIndex: number }[],
|
||||
): MatchOutcome | undefined {
|
||||
let firstIndex: number | undefined;
|
||||
let occurrences = 0;
|
||||
const recordedIndices: number[] = [];
|
||||
let searchStart = 0;
|
||||
|
||||
while (searchStart <= content.length - target.length) {
|
||||
const index = content.indexOf(target, searchStart);
|
||||
if (index === -1) break;
|
||||
|
||||
const endIndex = index + target.length;
|
||||
if (!excludedRanges.some(range => index < range.endIndex && endIndex > range.startIndex)) {
|
||||
firstIndex ??= index;
|
||||
occurrences++;
|
||||
if (recordedIndices.length < MAX_RECORDED_MATCHES) {
|
||||
recordedIndices.push(index);
|
||||
}
|
||||
}
|
||||
searchStart = endIndex;
|
||||
}
|
||||
|
||||
if (firstIndex === undefined) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const occurrences = content.split(target).length - 1;
|
||||
if (occurrences > 1) {
|
||||
const contentLines = content.split("\n");
|
||||
const occurrenceLines: number[] = [];
|
||||
const occurrencePreviews: string[] = [];
|
||||
let searchStart = 0;
|
||||
|
||||
for (let i = 0; i < MAX_RECORDED_MATCHES; i++) {
|
||||
const idx = content.indexOf(target, searchStart);
|
||||
if (idx === -1) break;
|
||||
const lineNumber = content.slice(0, idx).split("\n").length;
|
||||
for (const index of recordedIndices) {
|
||||
const lineNumber = content.slice(0, index).split("\n").length;
|
||||
occurrenceLines.push(lineNumber);
|
||||
occurrencePreviews.push(
|
||||
formatPreviewWindow(contentLines, lineNumber - 1, {
|
||||
@@ -282,17 +300,16 @@ function findExactMatchOutcome(content: string, target: string): MatchOutcome |
|
||||
maxLen: OCCURRENCE_PREVIEW_MAX_LEN,
|
||||
}),
|
||||
);
|
||||
searchStart = idx + 1;
|
||||
}
|
||||
|
||||
return { occurrences, occurrenceLines, occurrencePreviews };
|
||||
}
|
||||
|
||||
const startLine = content.slice(0, exactIndex).split("\n").length;
|
||||
const startLine = content.slice(0, firstIndex).split("\n").length;
|
||||
return {
|
||||
match: {
|
||||
actualText: target,
|
||||
startIndex: exactIndex,
|
||||
startIndex: firstIndex,
|
||||
startLine,
|
||||
confidence: 1,
|
||||
},
|
||||
@@ -408,6 +425,7 @@ function findBestFuzzyMatchCore(
|
||||
offsets: number[],
|
||||
threshold: number,
|
||||
includeDepth: boolean,
|
||||
excludedRanges: readonly { startIndex: number; endIndex: number }[],
|
||||
): BestFuzzyMatchResult {
|
||||
const targetNormalized = normalizeLines(targetLines, includeDepth);
|
||||
|
||||
@@ -417,6 +435,12 @@ function findBestFuzzyMatchCore(
|
||||
let aboveThresholdCount = 0;
|
||||
|
||||
for (let start = 0; start <= contentLines.length - targetLines.length; start++) {
|
||||
const startIndex = offsets[start];
|
||||
const endLine = start + targetLines.length - 1;
|
||||
const endIndex = Math.max(startIndex + 1, offsets[endLine] + contentLines[endLine].length);
|
||||
if (excludedRanges.some(range => startIndex < range.endIndex && endIndex > range.startIndex)) {
|
||||
continue;
|
||||
}
|
||||
const windowLines = contentLines.slice(start, start + targetLines.length);
|
||||
const windowNormalized = normalizeLines(windowLines, includeDepth);
|
||||
let score = 0;
|
||||
@@ -434,7 +458,7 @@ function findBestFuzzyMatchCore(
|
||||
bestScore = score;
|
||||
best = {
|
||||
actualText: windowLines.join("\n"),
|
||||
startIndex: offsets[start],
|
||||
startIndex,
|
||||
startLine: start + 1,
|
||||
confidence: score,
|
||||
};
|
||||
@@ -446,7 +470,12 @@ function findBestFuzzyMatchCore(
|
||||
return { best, aboveThresholdCount, secondBestScore };
|
||||
}
|
||||
|
||||
function findBestFuzzyMatch(content: string, target: string, threshold: number): BestFuzzyMatchResult {
|
||||
function findBestFuzzyMatch(
|
||||
content: string,
|
||||
target: string,
|
||||
threshold: number,
|
||||
excludedRanges: readonly { startIndex: number; endIndex: number }[],
|
||||
): BestFuzzyMatchResult {
|
||||
const contentLines = content.split("\n");
|
||||
const targetLines = target.split("\n");
|
||||
|
||||
@@ -458,11 +487,18 @@ function findBestFuzzyMatch(content: string, target: string, threshold: number):
|
||||
}
|
||||
|
||||
const offsets = computeLineOffsets(contentLines);
|
||||
let result = findBestFuzzyMatchCore(contentLines, targetLines, offsets, threshold, true);
|
||||
let result = findBestFuzzyMatchCore(contentLines, targetLines, offsets, threshold, true, excludedRanges);
|
||||
|
||||
// Retry without indent depth if match is close but below threshold
|
||||
if (result.best && result.best.confidence < threshold && result.best.confidence >= FALLBACK_THRESHOLD) {
|
||||
const noDepthResult = findBestFuzzyMatchCore(contentLines, targetLines, offsets, threshold, false);
|
||||
const noDepthResult = findBestFuzzyMatchCore(
|
||||
contentLines,
|
||||
targetLines,
|
||||
offsets,
|
||||
threshold,
|
||||
false,
|
||||
excludedRanges,
|
||||
);
|
||||
if (noDepthResult.best && noDepthResult.best.confidence > result.best.confidence) {
|
||||
result = noDepthResult;
|
||||
}
|
||||
@@ -473,25 +509,35 @@ function findBestFuzzyMatch(content: string, target: string, threshold: number):
|
||||
|
||||
/**
|
||||
* Find a match for target text within content.
|
||||
* Used primarily for replace-mode edits.
|
||||
* Used primarily for replace-mode edits; excluded ranges remain invisible to exact and fuzzy matching.
|
||||
*/
|
||||
export function findMatch(
|
||||
content: string,
|
||||
target: string,
|
||||
options: { allowFuzzy: boolean; threshold?: number },
|
||||
options: {
|
||||
allowFuzzy: boolean;
|
||||
threshold?: number;
|
||||
excludedRanges?: readonly { startIndex: number; endIndex: number }[];
|
||||
},
|
||||
): MatchOutcome {
|
||||
if (target.length === 0) {
|
||||
return {};
|
||||
}
|
||||
const excludedRanges = options.excludedRanges ?? [];
|
||||
|
||||
const exactMatch = findExactMatchOutcome(content, target);
|
||||
const exactMatch = findExactMatchOutcome(content, target, excludedRanges);
|
||||
if (exactMatch) {
|
||||
return exactMatch;
|
||||
}
|
||||
|
||||
// Try fuzzy match
|
||||
const threshold = options.threshold ?? DEFAULT_FUZZY_THRESHOLD;
|
||||
const { best, aboveThresholdCount, secondBestScore } = findBestFuzzyMatch(content, target, threshold);
|
||||
const { best, aboveThresholdCount, secondBestScore } = findBestFuzzyMatch(
|
||||
content,
|
||||
target,
|
||||
threshold,
|
||||
excludedRanges,
|
||||
);
|
||||
|
||||
if (!best) {
|
||||
return {};
|
||||
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
computeHashlineDiff,
|
||||
DEFAULT_FUZZY_THRESHOLD,
|
||||
findMatch,
|
||||
replaceText,
|
||||
} from "@oh-my-pi/pi-coding-agent/edit";
|
||||
import { removeWithRetries } from "@oh-my-pi/pi-utils";
|
||||
|
||||
@@ -224,6 +225,26 @@ describe("adjustIndentation", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("replaceText", () => {
|
||||
test("does not re-match inserted text while replacing all fuzzy source matches", () => {
|
||||
const oldText = "a".repeat(50);
|
||||
const firstActual = `${"a".repeat(49)}b`;
|
||||
const secondActual = `${"a".repeat(44)}cccccc`;
|
||||
const newText = `${oldText}\nexpanded`;
|
||||
|
||||
expect(
|
||||
replaceText(`${firstActual}\n${secondActual}`, oldText, newText, {
|
||||
all: true,
|
||||
fuzzy: true,
|
||||
threshold: 0.8,
|
||||
}),
|
||||
).toEqual({
|
||||
content: `${newText}\n${newText}`,
|
||||
count: 2,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeHashlineDiff", () => {
|
||||
let tempDir = "";
|
||||
|
||||
|
||||
Reference in New Issue
Block a user