diff --git a/CHANGELOG.md b/CHANGELOG.md index 10688d6c4..6b51e243c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ Status of the `main` branch. Changes prior to the next official version change w the actual semantics (configurations are automatically migrated) - Fix: glob matching bare `*` and `?` in non-`**` patterns matched across `/`, contradicting documented behaviour #1732 +* Tools: + - `search_for_pattern`: support additive exclusion globs and ordered priority globs, and preserve + representative prioritized matches with omitted-result counts when responses exceed the output limit. + * Language Servers: - Allow language server priorities to be configured in `serena_config.yml` (for auto-detection during project creation) diff --git a/src/serena/project.py b/src/serena/project.py index a66bb3528..35bc9d840 100644 --- a/src/serena/project.py +++ b/src/serena/project.py @@ -403,6 +403,7 @@ def search_project_files_for_pattern( context_lines_after: int = 0, paths_include_glob: str | None = None, paths_exclude_glob: str | None = None, + paths_exclude_globs: list[str] | None = None, multiline: bool = True, code_files_only: bool = True, ) -> list[MatchedConsecutiveLines]: @@ -415,6 +416,7 @@ def search_project_files_for_pattern( :param context_lines_after: Number of lines of context to include after each match :param paths_include_glob: Glob pattern to filter which files to include in the search :param paths_exclude_glob: Glob pattern to filter which files to exclude from the search. Takes precedence over paths_include_glob. + :param paths_exclude_globs: Additional glob patterns to exclude. Additive with paths_exclude_glob. :param multiline: Whether to compile the regex with the DOTALL flag (``.`` matches newlines). :return: List of matched consecutive lines with context """ @@ -426,6 +428,7 @@ def search_project_files_for_pattern( context_lines_after=context_lines_after, paths_include_glob=paths_include_glob, paths_exclude_glob=paths_exclude_glob, + paths_exclude_globs=paths_exclude_globs, multiline=multiline, ) diff --git a/src/serena/tools/file_tools.py b/src/serena/tools/file_tools.py index bd52f5db5..fc75247b5 100644 --- a/src/serena/tools/file_tools.py +++ b/src/serena/tools/file_tools.py @@ -6,6 +6,7 @@ * editing at the file level """ +import json import os from collections import defaultdict from fnmatch import fnmatch @@ -17,6 +18,7 @@ from serena.util.text_utils import ( ContentReplacer, GlobMatcher, + MatchedConsecutiveLines, MultiFileContentReplacer, ReplacementOccurrence, ) @@ -566,6 +568,8 @@ def apply( context_lines_after: int = 0, paths_include_glob: str = "", paths_exclude_glob: str = "", + paths_exclude_globs: list[str] | None = None, + paths_priority_globs: list[str] | None = None, relative_path: str = "", restrict_search_to_code_files: bool = False, multiline: bool = True, @@ -580,6 +584,9 @@ def apply( :param context_lines_after: number of context lines to include after each match. :param paths_include_glob: optional glob (relative to project root, e.g. ``"src/**/*.ts"``) restricting which files are searched. :param paths_exclude_glob: optional glob to exclude files; takes precedence over `paths_include_glob`. + :param paths_exclude_globs: additional optional globs to exclude files; additive with `paths_exclude_glob`. + :param paths_priority_globs: optional ordered globs that rank matching paths before unmatched paths. + The first matching glob determines a path's priority. :param relative_path: restricts the search to this file or subdirectory of the project root :param restrict_search_to_code_files: whether to search only files containing analyzable code symbols (useful when looking for class/method definitions); otherwise also search non-code files. @@ -599,10 +606,31 @@ def apply( context_lines_after=context_lines_after, paths_include_glob=paths_include_glob.strip(), paths_exclude_glob=paths_exclude_glob.strip(), + paths_exclude_globs=[glob.strip() for glob in paths_exclude_globs or [] if glob.strip()], multiline=multiline, code_files_only=restrict_search_to_code_files, ) + # normalize the public path contract before ranking and serialization + for match in matches: + assert match.source_file_path is not None + match.source_file_path = GlobMatcher.normalize_path(match.source_file_path) + + # rank prioritized matches deterministically while preserving source order when no priorities are supplied + priority_matchers = [GlobMatcher(glob.strip()) for glob in paths_priority_globs or [] if glob.strip()] + if priority_matchers: + + def priority(indexed_match: tuple[int, MatchedConsecutiveLines]) -> tuple[int, str, int, int]: + original_index, match = indexed_match + assert match.source_file_path is not None + priority_index = next( + (index for index, matcher in enumerate(priority_matchers) if matcher.matches(match.source_file_path)), + len(priority_matchers), + ) + return priority_index, match.source_file_path, match.matched_lines[0].line_number, original_index + + matches = [match for _, match in sorted(enumerate(matches), key=priority)] + # group matches by file file_to_matches: dict[str, list[str]] = defaultdict(list) for match in matches: @@ -616,9 +644,79 @@ def apply( first = match.matched_lines[0] match_lines_by_file[match.source_file_path].append({"line": first.line_number, "text": first.line_content.strip()}) + result = self._to_json(file_to_matches) + # shortened result closures, from least to most aggressive shortening _TEXT_TRUNCATE = 60 + def make_ranked_excerpt() -> str: + """Largest ranked prefix of whole match entries that fits the configured answer budget.""" + effective_max_answer_chars = ( + self.agent.serena_config.default_max_tool_answer_chars if max_answer_chars == -1 else max_answer_chars + ) + too_long_message = ( + f"The answer is too long ({len(result)} characters). You can adjust your query or raise the max_answer_chars parameter." + ) + excerpt_budget = effective_max_answer_chars - len(too_long_message) - 1 + excerpt_header = "Ranked match excerpt:\n" + + # precompute exact serialized prefix sizes and file counts in one pass + serialized_mapping_lengths = [2] # opening and closing braces + shown_file_counts = [0] + shown_paths: set[str] = set() + for match in matches: + assert match.source_file_path is not None + path = match.source_file_path + serialized_match_length = len(json.dumps(match.to_display_string(), ensure_ascii=False)) + serialized_mapping_length = serialized_mapping_lengths[-1] + if path in shown_paths: + serialized_mapping_length += 2 + serialized_match_length + else: + if shown_paths: + serialized_mapping_length += 2 + serialized_mapping_length += len(json.dumps(path, ensure_ascii=False)) + 3 + serialized_match_length + 1 + shown_paths.add(path) + serialized_mapping_lengths.append(serialized_mapping_length) + shown_file_counts.append(len(shown_paths)) + + # precompute counts of files represented by each omitted suffix + omitted_file_counts = [0] * (len(matches) + 1) + omitted_paths: set[str] = set() + for index in range(len(matches) - 1, -1, -1): + path = matches[index].source_file_path + assert path is not None + omitted_paths.add(path) + omitted_file_counts[index] = len(omitted_paths) + + def footer(shown_match_count: int) -> str: + return ( + f"Showing {shown_match_count} of {len(matches)} matches across " + f"{shown_file_counts[shown_match_count]} of {len(file_to_matches)} files; " + f"omitted {len(matches) - shown_match_count} matches across " + f"{omitted_file_counts[shown_match_count]} files; refine the query for omitted results." + ) + + # select the largest fitting prefix without serializing candidate mappings + shown_match_count = 0 + for shown_match_count in range(1, len(matches) + 1): + excerpt_length = len(excerpt_header) + serialized_mapping_lengths[shown_match_count] + 1 + len(footer(shown_match_count)) + if excerpt_length <= excerpt_budget: + continue + shown_match_count -= 1 + break + else: + shown_match_count = len(matches) + + if shown_match_count == 0: + return result + + # render the selected prefix once + excerpt: dict[str, list[str]] = defaultdict(list) + for match in matches[:shown_match_count]: + assert match.source_file_path is not None + excerpt[match.source_file_path].append(match.to_display_string()) + return f"{excerpt_header}{self._to_json(excerpt)}\n{footer(shown_match_count)}" + def render_first_lines(truncate: bool) -> str: """Render each match's first line, either in full or truncated to a fixed length.""" @@ -660,11 +758,11 @@ def make_per_file_counts() -> str: def make_summary() -> str: return f"Found {len(matches)} matches in {len(match_lines_by_file)} files." - result = self._to_json(file_to_matches) return self._limit_length( result, max_answer_chars, shortened_result_factories=[ + make_ranked_excerpt, make_first_lines_full, make_first_lines_truncated, make_line_numbers_only, diff --git a/src/serena/util/file_proxy.py b/src/serena/util/file_proxy.py index 1362aa163..03482ff82 100644 --- a/src/serena/util/file_proxy.py +++ b/src/serena/util/file_proxy.py @@ -97,22 +97,29 @@ def __iter__(self) -> Iterator[FileProxy]: def from_local_project_paths(cls, relative_paths: list[str], project: "Project") -> Self: return cls([LocalProjectFileProxy(path, project) for path in relative_paths]) - def filter_glob(self, paths_include_glob: str | None = None, paths_exclude_glob: str | None = None) -> "FileCollection": + def filter_glob( + self, + paths_include_glob: str | None = None, + paths_exclude_glob: str | None = None, + paths_exclude_globs: list[str] | None = None, + ) -> "FileCollection": """ Filters the collection based on the given patterns. Note: Filtering is applied only to local project files. Other files are always retained. :param paths_include_glob: optional glob pattern to include files from the list :param paths_exclude_glob: optional glob pattern to exclude files from the list + :param paths_exclude_globs: additional optional glob patterns to exclude; additive with paths_exclude_glob :return: the filtered collection """ from serena.util.text_utils import GlobMatcher - if paths_include_glob is None and paths_exclude_glob is None: + if paths_include_glob is None and paths_exclude_glob is None and not paths_exclude_globs: return self include_glob_matcher = GlobMatcher(paths_include_glob) if paths_include_glob else None - exclude_glob_matcher = GlobMatcher(paths_exclude_glob) if paths_exclude_glob else None + exclude_globs = [glob for glob in [paths_exclude_glob, *(paths_exclude_globs or [])] if glob] + exclude_glob_matchers = [(glob, GlobMatcher(glob)) for glob in exclude_globs] filtered_files = [] for f in self._file_proxies: @@ -122,10 +129,10 @@ def filter_glob(self, paths_include_glob: str | None = None, paths_exclude_glob: if not include_glob_matcher.matches(path): log.debug(f"Skipping {path}: does not match include pattern {paths_include_glob}") continue - if exclude_glob_matcher: - if exclude_glob_matcher.matches(path): - log.debug(f"Skipping {path}: matches exclude pattern {paths_exclude_glob}") - continue + matching_exclude_glob = next((glob for glob, matcher in exclude_glob_matchers if matcher.matches(path)), None) + if matching_exclude_glob: + log.debug(f"Skipping {path}: matches exclude pattern {matching_exclude_glob}") + continue filtered_files.append(f) return FileCollection(filtered_files) diff --git a/src/serena/util/text_utils.py b/src/serena/util/text_utils.py index 8ab6f9439..8b82696a8 100644 --- a/src/serena/util/text_utils.py +++ b/src/serena/util/text_utils.py @@ -203,7 +203,7 @@ def __init__(self, expr: str): """ :param expr: a glob pattern which may make use of brace expansion (e.g., "src/**/*.{js,jsx,ts,tsx}") """ - expr = expr.replace("\\", "/") # normalise backslashes to forward slashes + expr = self.normalize_path(expr) self._glob_expr = expr self._glob_patterns = self._expand_braces(expr) self._regex_patterns = [re.compile(self._translate_glob_to_regex(p)) for p in self._glob_patterns] @@ -211,6 +211,11 @@ def __init__(self, expr: str): def _tostring_includes(self) -> list[str]: return ["_glob_expr"] + @staticmethod + def normalize_path(path: str) -> str: + """Normalize a project-relative path for glob matching and public search output.""" + return path.replace("\\", "/") + @staticmethod def _expand_braces(pattern: str) -> list[str]: """ @@ -289,7 +294,7 @@ def _translate_glob_to_regex(pattern: str) -> str: return f"(?s:{inner})\\Z" def matches(self, path: str) -> bool: - path = path.replace("\\", "/") # normalise backslashes to forward slashes + path = self.normalize_path(path) return any(regex.match(path) for regex in self._regex_patterns) @@ -300,6 +305,7 @@ def search_files( context_lines_after: int = 0, paths_include_glob: str | None = None, paths_exclude_glob: str | None = None, + paths_exclude_globs: list[str] | None = None, multiline: bool = True, ) -> list[MatchedConsecutiveLines]: """ @@ -311,22 +317,28 @@ def search_files( :param context_lines_after: number of context lines to include after matches :param paths_include_glob: optional glob pattern to include files from the list :param paths_exclude_glob: optional glob pattern to exclude files from the list + :param paths_exclude_globs: additional optional glob patterns to exclude; additive with paths_exclude_glob :param multiline: whether to apply multi-line matching, enabling the flags re.DOTALL and re.MULTILINE (default: True) :return: list of MatchedConsecutiveLines objects """ # apply glob filter - file_collection = file_collection.filter_glob(paths_include_glob=paths_include_glob, paths_exclude_glob=paths_exclude_glob) + file_collection = file_collection.filter_glob( + paths_include_glob=paths_include_glob, + paths_exclude_glob=paths_exclude_glob, + paths_exclude_globs=paths_exclude_globs, + ) log.info(f"Processing {len(file_collection)} files.") def process_single_file(file_proxy: FileProxy) -> dict[str, Any]: """Process a single file - this function will be parallelized.""" relative_path = file_proxy.get_relative_path() + normalized_relative_path = GlobMatcher.normalize_path(relative_path) try: file_content = file_proxy.get_contents() search_results = search_text( pattern, content=file_content, - source_file_path=relative_path, + source_file_path=normalized_relative_path, context_lines_before=context_lines_before, context_lines_after=context_lines_after, multiline=multiline, diff --git a/test/serena/test_search_for_pattern.py b/test/serena/test_search_for_pattern.py index c00107d30..d08933d14 100644 --- a/test/serena/test_search_for_pattern.py +++ b/test/serena/test_search_for_pattern.py @@ -1,17 +1,61 @@ -"""Tests for the ``SearchForPatternTool`` overflow shortening chain. - -The snippet stage and, in particular, its position in the shortening chain were -previously untested. Test contributed by @AmirF194 in review of PR #1667. -""" +"""Tests for the ``SearchForPatternTool`` overflow shortening chain.""" +import json from unittest.mock import MagicMock +import pytest + from serena.config.serena_config import SerenaConfig from serena.project import Project from serena.tools.file_tools import SearchForPatternTool +from serena.util.text_utils import LineType, MatchedConsecutiveLines, TextLine + + +def make_search_tool(tmp_path) -> SearchForPatternTool: + serena_config = SerenaConfig(gui_log_window=False, web_dashboard=False) + project = Project.load(str(tmp_path), serena_config=serena_config) + agent = MagicMock() + agent.serena_config = serena_config + agent.get_active_project_or_raise.return_value = project + return SearchForPatternTool(agent) + + +def test_search_for_pattern_multiple_exclusion_globs_are_additive(tmp_path): + paths = [ + "plugins/feature-pipeline/SKILL.md", + "docs/superpowers/plans/archive.md", + "docs/superpowers/specs/archive.md", + "src/generated/cache.py", + ] + for path in paths: + file = tmp_path / path + file.parent.mkdir(parents=True, exist_ok=True) + file.write_text("feature-pipeline\n", encoding="utf-8") + + tool = make_search_tool(tmp_path) + singular_result = tool.apply( + substring_pattern="feature-pipeline", + paths_exclude_glob="**/generated/**", + ) + assert "plugins/feature-pipeline/SKILL.md" in singular_result + assert "docs/superpowers/plans/archive.md" in singular_result + assert "src/generated/cache.py" not in singular_result -def test_search_for_pattern_snippet_stage(tmp_path): + additive_result = tool.apply( + substring_pattern="feature-pipeline", + paths_exclude_glob="**/generated/**", + paths_exclude_globs=[ + "docs/superpowers/plans/**", + "docs/superpowers/specs/**", + ], + ) + assert "plugins/feature-pipeline/SKILL.md" in additive_result + assert "docs/superpowers" not in additive_result + assert "src/generated/cache.py" not in additive_result + + +def test_search_for_pattern_ranked_excerpt_precedes_smaller_fallbacks(tmp_path): lines: list[str] = [] for i in range(60): lines += [ @@ -23,10 +67,7 @@ def test_search_for_pattern_snippet_stage(tmp_path): ] (tmp_path / "data.txt").write_text("\n".join(lines) + "\n", encoding="utf-8") - project = Project.load(str(tmp_path), serena_config=SerenaConfig(gui_log_window=False, web_dashboard=False)) - agent = MagicMock() - agent.get_active_project_or_raise.return_value = project - tool = SearchForPatternTool(agent) + tool = make_search_tool(tmp_path) def run(cap: int) -> str: return tool.apply( @@ -37,12 +78,135 @@ def run(cap: int) -> str: max_answer_chars=cap, ) - # wide but overflowing cap: the snippet stage (line + matched text) is returned - snippet = run(7000) - assert "The answer is too long" in snippet - assert '"text":' in snippet and "MATCHME item number 0000" in snippet - assert "Match lines per file" not in snippet # not the bare-line-numbers stage + # wide but overflowing cap: whole ranked match entries retain their context + ranked = run(7000) + assert "The answer is too long" in ranked + assert "Ranked match excerpt:" in ranked + assert "MATCHME item number 0000" in ranked + assert "filler above" in ranked + assert "Showing " in ranked and "omitted " in ranked + + # exceptionally tight cap: the chain degrades past whole entries to a compact existing fallback + compact = run(400) + assert "Ranked match excerpt:" not in compact + assert any(marker in compact for marker in ("Match lines per file:", "Match counts per file:", "Found 60 matches")) + + +def test_search_for_pattern_prioritizes_live_results_in_ranked_overflow_excerpt(tmp_path): + for i in range(240): + file = tmp_path / f"docs/superpowers/plans/archive-{i:03d}.md" + file.parent.mkdir(parents=True, exist_ok=True) + file.write_text(f"feature-pipeline archived plan {i:03d} " + "payload " * 4 + "\n", encoding="utf-8") + + live_paths = [ + ".claude/skills/warden/SKILL.md", + "config/warden/gaia.yaml", + "scripts/warden-check.sh", + "plugins/feature-pipeline/z-last.md", + "plugins/feature-pipeline/a-first.md", + ] + for path in live_paths: + file = tmp_path / path + file.parent.mkdir(parents=True, exist_ok=True) + term = "feature-pipeline" if path.startswith("plugins/") else "Warden" + content = ( + f"{term} first live candidate\n{term} second live candidate\n" if path.endswith("a-first.md") else f"{term} live candidate\n" + ) + file.write_text(content, encoding="utf-8") + + tool = make_search_tool(tmp_path) + discovered_matches = tool.project.search_project_files_for_pattern( + pattern="feature-pipeline|Warden", + paths_exclude_glob=".serena/**", + code_files_only=False, + ) + z_matches = [match for match in discovered_matches if match.source_file_path == "plugins/feature-pipeline/z-last.md"] + a_matches = [match for match in discovered_matches if match.source_file_path == "plugins/feature-pipeline/a-first.md"] + controlled_paths = {"plugins/feature-pipeline/z-last.md", "plugins/feature-pipeline/a-first.md"} + other_matches = [match for match in discovered_matches if match.source_file_path not in controlled_paths] + controlled_source_order = z_matches + list(reversed(a_matches)) + other_matches + for match in controlled_source_order: + assert match.source_file_path is not None + match.source_file_path = match.source_file_path.replace("/", "\\") + tool.project.search_project_files_for_pattern = MagicMock(return_value=controlled_source_order) + expected_path_order = list( + dict.fromkeys(match.source_file_path.replace("\\", "/") for match in controlled_source_order if match.source_file_path is not None) + ) + + prioritized = tool.apply( + substring_pattern="feature-pipeline|Warden", + paths_exclude_glob=".serena/**", + paths_priority_globs=[ + "plugins/**", + ".claude/skills/**", + "config/**", + "scripts/**", + ], + restrict_search_to_code_files=False, + max_answer_chars=4_000, + ) + + assert "Ranked match excerpt:" in prioritized + assert all(path in prioritized for path in live_paths) + first_archive_index = prioritized.index("docs/superpowers/plans/archive-") + assert prioritized.index("plugins/feature-pipeline/a-first.md") < prioritized.index("plugins/feature-pipeline/z-last.md") + assert prioritized.index("0:feature-pipeline first live candidate") < prioritized.index("1:feature-pipeline second live candidate") + assert prioritized.index("plugins/feature-pipeline/z-last.md") < prioritized.index(".claude/skills/warden/SKILL.md") + assert prioritized.index(".claude/skills/warden/SKILL.md") < prioritized.index("config/warden/gaia.yaml") + assert prioritized.index("config/warden/gaia.yaml") < prioritized.index("scripts/warden-check.sh") + assert prioritized.index("scripts/warden-check.sh") < first_archive_index + assert "Showing " in prioritized + assert " of 246 matches across " in prioritized + assert "omitted " in prioritized + assert "refine the query for omitted results." in prioritized + + unprioritized = tool.apply( + substring_pattern="feature-pipeline|Warden", + paths_exclude_glob=".serena/**", + restrict_search_to_code_files=False, + max_answer_chars=100_000, + ) + unprioritized_mapping = json.loads(unprioritized) + assert list(unprioritized_mapping) == expected_path_order + assert "1:feature-pipeline second live candidate" in unprioritized_mapping["plugins/feature-pipeline/a-first.md"][0] + + +def test_ranked_excerpt_serialization_count_does_not_grow_with_match_count(monkeypatch): + matches = [ + MatchedConsecutiveLines( + lines=[TextLine(line_number=0, line_content="x", match_type=LineType.MATCH)], + source_file_path=f"archive/{i:05d}.md", + ) + for i in range(20_000) + ] + serena_config = SerenaConfig( + gui_log_window=False, + web_dashboard=False, + default_max_tool_answer_chars=150_000, + ) + project = MagicMock() + project.search_project_files_for_pattern.return_value = matches + agent = MagicMock() + agent.serena_config = serena_config + agent.get_active_project_or_raise.return_value = project + tool = SearchForPatternTool(agent) + + serialization_calls = 0 + serialize = tool._to_json + + def count_serialization_calls(value) -> str: + nonlocal serialization_calls + serialization_calls += 1 + if serialization_calls > 3: + pytest.fail("ranked excerpt sizing must not repeatedly serialize growing prefixes") + return serialize(value) + + monkeypatch.setattr(tool, "_to_json", count_serialization_calls) + + result = tool.apply( + substring_pattern="x", + paths_priority_globs=["plugins/**"], + ) - # tighter cap: the chain degrades past the snippet stage to bare line numbers - bare = run(1000) - assert "Match lines per file" in bare and '"text":' not in bare + assert "Ranked match excerpt:" in result + assert serialization_calls == 2 diff --git a/test/serena/test_text_utils.py b/test/serena/test_text_utils.py index 865b71b4f..4654008df 100644 --- a/test/serena/test_text_utils.py +++ b/test/serena/test_text_utils.py @@ -218,6 +218,46 @@ def __init__(self, file_paths, mock_reader: Callable[[str], str] = mock_reader_a class TestSearchFiles: + def test_multiple_exclusion_globs_are_additive(self): + results = search_files( + MockFileCollection( + [ + "plugins/feature-pipeline/SKILL.md", + "docs/superpowers/plans/archive.md", + "docs/superpowers/specs/archive.md", + "src/generated/cache.py", + ] + ), + pattern="match", + paths_exclude_glob="**/generated/**", + paths_exclude_globs=[ + "docs/superpowers/plans/**", + "docs/superpowers/specs/**", + ], + ) + + assert [result.source_file_path for result in results] == ["plugins/feature-pipeline/SKILL.md"] + + def test_windows_paths_are_filtered_and_returned_with_forward_slashes(self): + results = search_files( + MockFileCollection( + [ + r"plugins\feature-pipeline\SKILL.md", + r"docs\superpowers\plans\archive.md", + r"docs\superpowers\specs\archive.md", + r"src\generated\cache.py", + ] + ), + pattern="match", + paths_exclude_glob="**/generated/**", + paths_exclude_globs=[ + "docs/superpowers/plans/**", + "docs/superpowers/specs/**", + ], + ) + + assert [result.source_file_path for result in results] == ["plugins/feature-pipeline/SKILL.md"] + @pytest.mark.parametrize( "file_paths, pattern, paths_include_glob, paths_exclude_glob, expected_matched_files, description", [