From c0b0c886263abe3f32abfbca26b585b97081ff59 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:20:40 -0700 Subject: [PATCH] fix(fuzzy-match): stop context_aware from silently replacing wrong content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/tools/test_fuzzy_match.py | 53 +++++++++++++++++++++ tools/fuzzy_match.py | 84 ++++++++++++++++++++++++++------- 2 files changed, 120 insertions(+), 17 deletions(-) diff --git a/tests/tools/test_fuzzy_match.py b/tests/tools/test_fuzzy_match.py index d0b7bc7379c9e..e8d0f2a4808ae 100644 --- a/tests/tools/test_fuzzy_match.py +++ b/tests/tools/test_fuzzy_match.py @@ -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" + diff --git a/tools/fuzzy_match.py b/tools/fuzzy_match.py index b5c07aef72ce9..3f599e7b2ac05 100644 --- a/tools/fuzzy_match.py +++ b/tools/fuzzy_match.py @@ -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