fix(fuzzy-match): stop context_aware from silently replacing wrong content
Strategy 9 (context_aware, the last-resort fuzzy strategy used by patch_replace, V4A UPDATE hunks, and skill_manage) had two serious flaws, both reproduced live against current main: 1. CORRECTNESS: it accepted a block when >=50% of its lines were >=0.80 similar. A 2-line pattern with one real line and one garbage line matched, silently deleting the non-matching line and persisting a wrong edit as success. Now requires the first AND last lines to anchor-match and EVERY non-blank pattern line to be >=0.80 similar — one garbage line disqualifies the block. 2. PERFORMANCE: it scored every content window with per-line SequenceMatcher, so every failed match paid O(file_lines x pattern_lines) — measured ~5.5s for a single 40-line no-match on a 10k-line file, per hunk. The first/last line anchor pre-filter skips non-candidate windows: same case now ~160ms (34x faster). Also gate replace_all: a similarity-based strategy (block_anchor, context_aware) with multiple matches under replace_all would overwrite every approximate block, not just exact ones. Now refused with a clear error directing the caller to precise text. All verified with sabotage-checked regression tests (fail against the old 50% logic). 158 file/patch/fuzzy tests pass; legit fuzzy edits (indent drift, unique near-match) unaffected.
This commit is contained in:
parent
021a076880
commit
c0b0c88626
|
|
@ -541,3 +541,56 @@ class TestEscapeNormalizedNewString:
|
|||
assert count == 1
|
||||
assert "return 2" in new
|
||||
|
||||
|
||||
class TestContextAwareCorrectness:
|
||||
"""Strategy 9 must not silently replace half-matching (wrong) blocks."""
|
||||
|
||||
def test_half_garbage_block_does_not_match(self):
|
||||
"""A pattern where one line is unrelated must NOT match/corrupt.
|
||||
|
||||
Old behavior: context_aware accepted a block when >=50% of lines were
|
||||
similar, so this 2-line pattern (one real line, one garbage line)
|
||||
matched and silently deleted the real second line.
|
||||
"""
|
||||
content = "config_value = 100\nthreshold = 200\n"
|
||||
old = "config_value = 999\ntotally_unrelated_line_here"
|
||||
new = "config_value = 42\ntotally_unrelated_line_here"
|
||||
result, count, strategy, err = fuzzy_find_and_replace(content, old, new)
|
||||
assert count == 0, f"should not match, got strategy={strategy}"
|
||||
assert err is not None
|
||||
assert "threshold = 200" in result # not destroyed
|
||||
|
||||
def test_replace_all_refuses_similarity_strategy(self):
|
||||
"""replace_all must not mass-overwrite approximate (non-exact) blocks."""
|
||||
content = "aX\nbY\naX\nbY\naX\nbY\n"
|
||||
# 'aX\nbZ' never appears exactly; only approximately (bY != bZ).
|
||||
result, count, strategy, err = fuzzy_find_and_replace(
|
||||
content, "aX\nbZ", "QQ\nRR", replace_all=True
|
||||
)
|
||||
assert count == 0, f"should refuse, got strategy={strategy}"
|
||||
assert err is not None
|
||||
assert result == content # untouched
|
||||
|
||||
def test_all_lines_matching_still_replaces(self):
|
||||
"""A block where every line is a close match still applies (unique)."""
|
||||
content = "alpha one\nbeta two\ngamma three\n"
|
||||
old = "alpha one\nbeta 2\ngamma three" # close on every line
|
||||
new = "alpha one\nbeta TWO\ngamma three"
|
||||
result, count, strategy, err = fuzzy_find_and_replace(content, old, new)
|
||||
assert count == 1, f"err={err}"
|
||||
assert "beta TWO" in result
|
||||
|
||||
def test_no_match_on_large_file_is_fast(self):
|
||||
"""The anchor pre-filter keeps a no-match scan from being O(file×pattern)."""
|
||||
import time
|
||||
from tools.fuzzy_match import _strategy_context_aware
|
||||
|
||||
big = "\n".join(f"line {i} content here" for i in range(10000))
|
||||
patt = "\n".join(f"nomatch xyzzy {i}" for i in range(40))
|
||||
start = time.perf_counter()
|
||||
matches = _strategy_context_aware(big, patt)
|
||||
elapsed = time.perf_counter() - start
|
||||
assert matches == []
|
||||
# Was ~5.5s before anchoring; generous ceiling to avoid CI flake.
|
||||
assert elapsed < 2.0, f"context_aware no-match took {elapsed:.2f}s"
|
||||
|
||||
|
|
|
|||
|
|
@ -99,6 +99,13 @@ def fuzzy_find_and_replace(content: str, old_string: str, new_string: str,
|
|||
("context_aware", _strategy_context_aware),
|
||||
]
|
||||
|
||||
# Strategies whose matches are similarity-based rather than exact-content:
|
||||
# they can accept a region that only *approximately* resembles old_string.
|
||||
# Safe for a single unique replacement (the caller asked to change that one
|
||||
# spot), but NEVER safe under replace_all — "replace every approximate
|
||||
# match" silently rewrites regions that don't actually contain old_string.
|
||||
_SIMILARITY_STRATEGIES = {"block_anchor", "context_aware"}
|
||||
|
||||
for strategy_name, strategy_fn in strategies:
|
||||
matches = strategy_fn(content, old_string)
|
||||
|
||||
|
|
@ -110,6 +117,18 @@ def fuzzy_find_and_replace(content: str, old_string: str, new_string: str,
|
|||
f"Provide more context to make it unique, or use replace_all=True."
|
||||
)
|
||||
|
||||
# replace_all with a similarity-based strategy would overwrite
|
||||
# every approximately-matching block, not just exact ones — refuse
|
||||
# and make the caller narrow old_string to something a precise
|
||||
# strategy can match exactly.
|
||||
if replace_all and len(matches) > 1 and strategy_name in _SIMILARITY_STRATEGIES:
|
||||
return content, 0, None, (
|
||||
f"Found {len(matches)} approximate matches via the "
|
||||
f"'{strategy_name}' strategy; replace_all only applies to exact "
|
||||
f"matches. Provide the precise text (whitespace included) so an "
|
||||
f"exact/line-trimmed match can be made."
|
||||
)
|
||||
|
||||
# Escape-drift guard: when the matched strategy is NOT `exact`,
|
||||
# we matched via some form of normalization. If new_string
|
||||
# contains shell/JSON-style escape sequences (\' or \") that
|
||||
|
|
@ -712,36 +731,67 @@ def _strategy_block_anchor(content: str, pattern: str) -> List[Tuple[int, int]]:
|
|||
|
||||
def _strategy_context_aware(content: str, pattern: str) -> List[Tuple[int, int]]:
|
||||
"""
|
||||
Strategy 9: Line-by-line similarity with 50% threshold.
|
||||
|
||||
Finds blocks where at least 50% of lines have high similarity.
|
||||
Strategy 9 (last resort): anchored line-by-line similarity.
|
||||
|
||||
Only considers blocks whose first AND last lines closely match the
|
||||
pattern's first/last lines (an anchor pre-filter), then requires EVERY
|
||||
non-blank pattern line to be highly similar (>=0.80) to the aligned
|
||||
content line. The anchor filter keeps this from being an O(file x pattern)
|
||||
scan on every miss, and the all-lines requirement stops a single
|
||||
coincidental line-match from silently replacing an unrelated block
|
||||
(the old 50%-of-lines threshold accepted half-garbage patterns and
|
||||
destroyed the non-matching lines).
|
||||
"""
|
||||
pattern_lines = pattern.split('\n')
|
||||
content_lines = content.split('\n')
|
||||
|
||||
|
||||
if not pattern_lines:
|
||||
return []
|
||||
|
||||
matches = []
|
||||
|
||||
pattern_line_count = len(pattern_lines)
|
||||
|
||||
if pattern_line_count > len(content_lines):
|
||||
return []
|
||||
|
||||
# Anchor pre-filter: a block is only a candidate when its first and last
|
||||
# lines are strong matches for the pattern's first/last lines. This is the
|
||||
# cheap gate that avoids scoring every window in the file.
|
||||
first_pat = pattern_lines[0].strip()
|
||||
last_pat = pattern_lines[-1].strip()
|
||||
ANCHOR_THRESHOLD = 0.80
|
||||
|
||||
def _sim(a: str, b: str) -> float:
|
||||
if a == b:
|
||||
return 1.0
|
||||
return SequenceMatcher(None, a, b).ratio()
|
||||
|
||||
matches = []
|
||||
for i in range(len(content_lines) - pattern_line_count + 1):
|
||||
block_lines = content_lines[i:i + pattern_line_count]
|
||||
|
||||
# Calculate line-by-line similarity
|
||||
high_similarity_count = 0
|
||||
|
||||
# Cheap anchor check first — skip non-candidate windows without
|
||||
# scoring their interior.
|
||||
if _sim(first_pat, block_lines[0].strip()) < ANCHOR_THRESHOLD:
|
||||
continue
|
||||
if _sim(last_pat, block_lines[-1].strip()) < ANCHOR_THRESHOLD:
|
||||
continue
|
||||
|
||||
# Candidate: require EVERY non-blank pattern line to match its aligned
|
||||
# content line closely. One garbage line disqualifies the block.
|
||||
all_match = True
|
||||
for p_line, c_line in zip(pattern_lines, block_lines):
|
||||
sim = SequenceMatcher(None, p_line.strip(), c_line.strip()).ratio()
|
||||
if sim >= 0.80:
|
||||
high_similarity_count += 1
|
||||
|
||||
# Need at least 50% of lines to have high similarity
|
||||
if high_similarity_count >= len(pattern_lines) * 0.5:
|
||||
p_stripped = p_line.strip()
|
||||
if not p_stripped:
|
||||
continue # blank pattern lines don't constrain the match
|
||||
if _sim(p_stripped, c_line.strip()) < 0.80:
|
||||
all_match = False
|
||||
break
|
||||
|
||||
if all_match:
|
||||
start_pos, end_pos = _calculate_line_positions(
|
||||
content_lines, i, i + pattern_line_count, len(content)
|
||||
)
|
||||
matches.append((start_pos, end_pos))
|
||||
|
||||
|
||||
return matches
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue