From 7ed29664e824d1cb77c88824fb3e99e3bfc2a6a0 Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 00:12:33 +0300 Subject: [PATCH 01/11] add format parameter to get_chunk Plumbs a format string parameter through the MCP and tools layers with raw as the default. Adds FormatMode enum, a formatter module with placeholder branches for annotated/compact, and validation that rejects unknown values. --- src/formatter.py | 25 ++++++++++++++++++ src/models.py | 9 +++++++ src/server.py | 7 +++++ src/tools.py | 23 ++++++++++++++--- tests/test_mcp_components.py | 50 ++++++++++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 src/formatter.py diff --git a/src/formatter.py b/src/formatter.py new file mode 100644 index 0000000..7b9a7d2 --- /dev/null +++ b/src/formatter.py @@ -0,0 +1,25 @@ +"""Chunk content formatting for different output modes.""" + +from typing import List + +from .models import FormatMode + + +def format_chunk(content: str, mode: FormatMode, chunk_files: List[str]) -> str: + """Format chunk content according to the specified mode. + + Args: + content: Raw diff chunk content. + mode: The format mode to apply. + chunk_files: List of file paths in this chunk. + + Returns: + Formatted content string. + """ + if mode == FormatMode.RAW: + return content + if mode == FormatMode.ANNOTATED: + return content # Placeholder - implemented in Phase 2 + if mode == FormatMode.COMPACT: + return content # Placeholder - implemented in Phase 3 + return content diff --git a/src/models.py b/src/models.py index bf0b6de..cbab068 100644 --- a/src/models.py +++ b/src/models.py @@ -2,9 +2,18 @@ import fnmatch from dataclasses import dataclass, field +from enum import Enum from typing import Any, Dict, List +class FormatMode(str, Enum): + """Output format modes for chunk content.""" + + RAW = "raw" + ANNOTATED = "annotated" + COMPACT = "compact" + + @dataclass class DiffStats: """Statistics about a loaded diff.""" diff --git a/src/server.py b/src/server.py index 617d630..96a387f 100644 --- a/src/server.py +++ b/src/server.py @@ -90,6 +90,12 @@ def get_chunk( include_context: Annotated[ bool, Field(description="Include chunk header with metadata") ] = True, + format: Annotated[ + str, + Field( + description="Output format: 'raw' (default, standard diff), 'annotated' (line numbers, new/old hunk separation), 'compact' (line numbers, new hunks only)" + ), + ] = "raw", ) -> str: """Retrieve the actual content of a specific numbered chunk from a diff file. Auto-loads the diff file if not already loaded. Use this for systematic analysis of changes chunk-by-chunk, or to examine specific chunks identified via list_chunks or find_chunks_for_files. CRITICAL: You must use an absolute directory path - relative paths will fail. DO NOT read diff files directly - they exceed LLM context windows. This tool provides manageable portions of large diffs. Track your progress through chunks when doing comprehensive analysis and clean up tracking documents before final results.""" logger.info( @@ -100,6 +106,7 @@ def get_chunk( absolute_file_path=absolute_file_path, chunk_number=chunk_number, include_context=include_context, + format=format, ) return result diff --git a/src/tools.py b/src/tools.py index d94652e..ca1516b 100644 --- a/src/tools.py +++ b/src/tools.py @@ -6,8 +6,9 @@ import os import time from typing import Dict, Any, List, Optional -from .models import DiffSession +from .models import DiffSession, FormatMode from .chunker import DiffChunker +from .formatter import format_chunk logger = logging.getLogger("diffchunk") @@ -170,7 +171,11 @@ def list_chunks(self, absolute_file_path: str) -> List[Dict[str, Any]]: ] def get_chunk( - self, absolute_file_path: str, chunk_number: int, include_context: bool = True + self, + absolute_file_path: str, + chunk_number: int, + include_context: bool = True, + format: str = "raw", ) -> str: """Get the content of a specific chunk.""" file_key = self._ensure_loaded(absolute_file_path) @@ -179,6 +184,12 @@ def get_chunk( if not isinstance(chunk_number, int) or chunk_number <= 0: raise ValueError("chunk_number must be a positive integer") + try: + mode = FormatMode(format) + except ValueError: + valid = ", ".join(f"'{m.value}'" for m in FormatMode) + raise ValueError(f"Invalid format '{format}'. Must be one of: {valid}") + chunk = session.get_chunk(chunk_number) if not chunk: total_chunks = len(session.chunks) @@ -186,14 +197,18 @@ def get_chunk( f"Chunk {chunk_number} does not exist. The diff has {total_chunks} chunks (1-{total_chunks}). Use list_chunks to see what each chunk contains." ) + content = chunk.content + if mode != FormatMode.RAW: + content = format_chunk(content, mode, chunk.files) + if include_context: header = f"=== Chunk {chunk.number} of {len(session.chunks)} ===\n" header += f"Files: {', '.join(chunk.files)}\n" header += f"Lines: {chunk.line_count}\n" header += "=" * 50 + "\n" - return header + chunk.content + return header + content else: - return chunk.content + return content def find_chunks_for_files(self, absolute_file_path: str, pattern: str) -> List[int]: """Find chunks containing files matching the given pattern.""" diff --git a/tests/test_mcp_components.py b/tests/test_mcp_components.py index f44c3b6..85aa01f 100644 --- a/tests/test_mcp_components.py +++ b/tests/test_mcp_components.py @@ -316,3 +316,53 @@ def test_large_diff_performance(self, go_diff_file): assert get_time < 1.0, f"Get chunk took too long: {get_time}s" assert len(content) > 0 + + def test_format_raw_returns_identical_output(self, react_diff_file): + """Test that format='raw' returns identical output to default (no format param).""" + tools = DiffChunkTools() + tools.load_diff(react_diff_file, max_chunk_lines=3000) + + # Get chunk with default behavior (no format param) + default_output = tools.get_chunk(react_diff_file, 1, include_context=True) + + # Get chunk with explicit format="raw" + raw_output = tools.get_chunk( + react_diff_file, 1, include_context=True, format="raw" + ) + + assert default_output == raw_output + + # Also verify without context header + default_no_ctx = tools.get_chunk(react_diff_file, 1, include_context=False) + raw_no_ctx = tools.get_chunk( + react_diff_file, 1, include_context=False, format="raw" + ) + + assert default_no_ctx == raw_no_ctx + + def test_format_invalid_raises_valueerror(self, react_diff_file): + """Test that an invalid format string raises ValueError with helpful message.""" + tools = DiffChunkTools() + tools.load_diff(react_diff_file, max_chunk_lines=3000) + + with pytest.raises(ValueError, match="Invalid format 'invalid'") as exc_info: + tools.get_chunk(react_diff_file, 1, format="invalid") + + error_msg = str(exc_info.value) + assert "'raw'" in error_msg + assert "'annotated'" in error_msg + assert "'compact'" in error_msg + + def test_format_annotated_and_compact_accepted(self, react_diff_file): + """Test that annotated and compact format values are accepted without error.""" + tools = DiffChunkTools() + tools.load_diff(react_diff_file, max_chunk_lines=3000) + + # These should not raise - they are valid modes (placeholders for now) + annotated = tools.get_chunk(react_diff_file, 1, format="annotated") + assert isinstance(annotated, str) + assert len(annotated) > 0 + + compact = tools.get_chunk(react_diff_file, 1, format="compact") + assert isinstance(compact, str) + assert len(compact) > 0 From 9e73ee29cc38648518bd5a760f7bfaa495ce4ce1 Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 00:20:39 +0300 Subject: [PATCH 02/11] add annotated format with line numbers and hunk separation Implements the annotated output mode that parses raw diff content into structured sections with file headers, line-numbered new hunks, and old hunks for removed code. Includes function context from @@ headers. --- src/formatter.py | 213 ++++++++++++++++++++++++++++++- tests/test_mcp_components.py | 234 +++++++++++++++++++++++++++++++++++ 2 files changed, 445 insertions(+), 2 deletions(-) diff --git a/src/formatter.py b/src/formatter.py index 7b9a7d2..7d15eb4 100644 --- a/src/formatter.py +++ b/src/formatter.py @@ -1,10 +1,219 @@ """Chunk content formatting for different output modes.""" -from typing import List +import re +from dataclasses import dataclass +from typing import List, Optional from .models import FormatMode +@dataclass +class HunkLine: + """A single line within a diff hunk.""" + + type: str # "context", "added", "removed" + text: str # Line content without +/- prefix + old_line: Optional[int] # Line number in old file + new_line: Optional[int] # Line number in new file + + +@dataclass +class Hunk: + """A parsed diff hunk with line-level detail.""" + + lines: List[HunkLine] + function_context: Optional[str] # From @@ trailing text + + +@dataclass +class FileSection: + """A file's diff content split into hunks.""" + + path: str + hunks: List[Hunk] + + +# Regex for @@ header: @@ -old_start[,old_count] +new_start[,new_count] @@ [context] +_HUNK_HEADER_RE = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*)$") + + +def _parse_hunk_header(line: str) -> Optional[tuple]: + """Parse an @@ header line into (old_start, new_start, function_context). + + Returns None if the line is not a valid hunk header. + """ + m = _HUNK_HEADER_RE.match(line) + if not m: + return None + old_start = int(m.group(1)) + new_start = int(m.group(3)) + trailing = m.group(5).strip() + func_ctx = trailing if trailing else None + return old_start, new_start, func_ctx + + +def _parse_file_sections(content: str) -> List[FileSection]: + """Split chunk content into file sections and their hunks.""" + sections: List[FileSection] = [] + # Strip trailing newline to avoid spurious empty context line at end + lines = content.rstrip("\n").split("\n") + + current_path: Optional[str] = None + current_hunks: List[Hunk] = [] + current_hunk_lines: List[HunkLine] = [] + current_func_ctx: Optional[str] = None + old_line = 0 + new_line = 0 + in_hunk = False # True after we've seen a @@ header for the current file + + def _flush_hunk(): + nonlocal current_hunk_lines, current_func_ctx, in_hunk + if current_hunk_lines: + current_hunks.append( + Hunk(lines=current_hunk_lines, function_context=current_func_ctx) + ) + current_hunk_lines = [] + current_func_ctx = None + in_hunk = False + + def _flush_file(): + nonlocal current_path, current_hunks, in_hunk + _flush_hunk() + if current_path is not None and current_hunks: + sections.append(FileSection(path=current_path, hunks=current_hunks)) + current_hunks = [] + in_hunk = False + + for line in lines: + # Detect file boundary + if line.startswith("diff --git "): + _flush_file() + # Extract b/ path + parts = line.split(" b/", 1) + if len(parts) == 2: + current_path = parts[1] + else: + current_path = line # fallback + continue + + # Detect hunk header + parsed = _parse_hunk_header(line) + if parsed is not None: + _flush_hunk() + old_line, new_line, current_func_ctx = parsed[0], parsed[1], parsed[2] + in_hunk = True + continue + + # Skip diff metadata lines (---, +++, index, mode, etc.) before first hunk + if current_path is not None and not in_hunk: + # These are file-level metadata lines, skip them + continue + + # Only process diff content lines when we're inside a hunk + if not in_hunk or current_path is None: + continue + + # Parse diff content lines + if line.startswith("+"): + text = line[1:] + current_hunk_lines.append( + HunkLine(type="added", text=text, old_line=None, new_line=new_line) + ) + new_line += 1 + elif line.startswith("-"): + text = line[1:] + current_hunk_lines.append( + HunkLine(type="removed", text=text, old_line=old_line, new_line=None) + ) + old_line += 1 + elif line.startswith(" "): + # Context line (starts with space) + text = line[1:] + current_hunk_lines.append( + HunkLine( + type="context", text=text, old_line=old_line, new_line=new_line + ) + ) + old_line += 1 + new_line += 1 + elif line.startswith("\\"): + # "\ No newline at end of file" - skip + continue + + _flush_file() + return sections + + +def _build_new_hunk(hunk: Hunk) -> List[str]: + """Build __new hunk__ lines: context + added, with new-file line numbers.""" + result = [] + for hl in hunk.lines: + if hl.type == "removed": + continue + if hl.type == "added": + result.append(f"{hl.new_line:>4} +{hl.text}") + else: # context + result.append(f"{hl.new_line:>4} {hl.text}") + return result + + +def _build_old_hunk(hunk: Hunk) -> List[str]: + """Build __old hunk__ lines: context + removed, no line numbers.""" + result = [] + for hl in hunk.lines: + if hl.type == "added": + continue + if hl.type == "removed": + result.append(f" -{hl.text}") + else: # context + result.append(f" {hl.text}") + return result + + +def _hunk_header(label: str, func_ctx: Optional[str]) -> str: + """Build a hunk header like '__new hunk__ | func' or just '__new hunk__'.""" + if func_ctx: + return f"{label} | {func_ctx}" + return label + + +def _has_removed_lines(hunk: Hunk) -> bool: + """Check if a hunk contains any removed lines.""" + return any(hl.type == "removed" for hl in hunk.lines) + + +def _has_added_or_context_lines(hunk: Hunk) -> bool: + """Check if a hunk has added or context lines (for __new hunk__).""" + return any(hl.type in ("added", "context") for hl in hunk.lines) + + +def _format_annotated(content: str, chunk_files: List[str]) -> str: + """Format chunk content into annotated format with line numbers and hunk separation.""" + sections = _parse_file_sections(content) + + if not sections: + # No parseable diff content, return as-is + return content + + output_parts: List[str] = [] + + for section in sections: + output_parts.append(f"## File: '{section.path}'") + + for hunk in section.hunks: + # __new hunk__ section (skip if no added/context lines, e.g. deleted file) + if _has_added_or_context_lines(hunk): + output_parts.append(_hunk_header("__new hunk__", hunk.function_context)) + output_parts.extend(_build_new_hunk(hunk)) + + # __old hunk__ section (only if there are removed lines) + if _has_removed_lines(hunk): + output_parts.append(_hunk_header("__old hunk__", hunk.function_context)) + output_parts.extend(_build_old_hunk(hunk)) + + return "\n".join(output_parts) + + def format_chunk(content: str, mode: FormatMode, chunk_files: List[str]) -> str: """Format chunk content according to the specified mode. @@ -19,7 +228,7 @@ def format_chunk(content: str, mode: FormatMode, chunk_files: List[str]) -> str: if mode == FormatMode.RAW: return content if mode == FormatMode.ANNOTATED: - return content # Placeholder - implemented in Phase 2 + return _format_annotated(content, chunk_files) if mode == FormatMode.COMPACT: return content # Placeholder - implemented in Phase 3 return content diff --git a/tests/test_mcp_components.py b/tests/test_mcp_components.py index 85aa01f..8857371 100644 --- a/tests/test_mcp_components.py +++ b/tests/test_mcp_components.py @@ -4,6 +4,7 @@ import pytest +from src.formatter import _format_annotated from src.tools import DiffChunkTools @@ -366,3 +367,236 @@ def test_format_annotated_and_compact_accepted(self, react_diff_file): compact = tools.get_chunk(react_diff_file, 1, format="compact") assert isinstance(compact, str) assert len(compact) > 0 + + +class TestAnnotatedFormat: + """Tests for the annotated format output.""" + + def test_multi_hunk_multi_file_annotated(self): + """Multi-hunk, multi-file diff produces correct annotated output.""" + diff = ( + "diff --git a/src/auth.py b/src/auth.py\n" + "index abc1234..def5678 100644\n" + "--- a/src/auth.py\n" + "+++ b/src/auth.py\n" + "@@ -10,4 +10,5 @@ def authenticate\n" + " unchanged\n" + " also unchanged\n" + "+new line\n" + " trailing ctx\n" + "@@ -30,3 +31,3 @@ def logout\n" + " ctx\n" + "-old removed\n" + "+new replaced\n" + " ctx end\n" + "diff --git a/src/utils.py b/src/utils.py\n" + "index 1111111..2222222 100644\n" + "--- a/src/utils.py\n" + "+++ b/src/utils.py\n" + "@@ -1,3 +1,4 @@\n" + " first\n" + "+inserted\n" + " second\n" + " third\n" + ) + result = _format_annotated(diff, ["src/auth.py", "src/utils.py"]) + + # File headers + assert "## File: 'src/auth.py'" in result + assert "## File: 'src/utils.py'" in result + + # Hunk markers + assert "__new hunk__" in result + assert "__old hunk__" in result + + # Function context on hunk headers + assert "__new hunk__ | def authenticate" in result + assert "__old hunk__ | def logout" in result + + # Line numbers on __new hunk__ lines + lines = result.split("\n") + + # Find a __new hunk__ added line - should have line number and + prefix + new_hunk_added = [l for l in lines if "+" in l and "new line" in l] + assert len(new_hunk_added) >= 1 + assert new_hunk_added[0].strip().startswith("12") # line 10+2 context = 12 + + # Find __old hunk__ removed line - should have - prefix, no line number + old_hunk_removed = [l for l in lines if "-" in l and "old removed" in l] + assert len(old_hunk_removed) >= 1 + # Old hunk lines have no numeric line numbers + stripped = old_hunk_removed[0].strip() + assert stripped.startswith("-") + + def test_new_file_no_old_hunks(self): + """New file should produce __new hunk__ only, no __old hunk__.""" + diff = ( + "diff --git a/newfile.py b/newfile.py\n" + "new file mode 100644\n" + "index 0000000..abcdef1\n" + "--- /dev/null\n" + "+++ b/newfile.py\n" + "@@ -0,0 +1,3 @@\n" + "+line one\n" + "+line two\n" + "+line three\n" + ) + result = _format_annotated(diff, ["newfile.py"]) + + assert "## File: 'newfile.py'" in result + assert "__new hunk__" in result + assert "__old hunk__" not in result + + # All lines should be added with line numbers + lines = result.split("\n") + added_lines = [l for l in lines if "+" in l and "line" in l] + assert len(added_lines) == 3 + + def test_deleted_file_no_new_hunks(self): + """Deleted file should produce __old hunk__ only, no __new hunk__.""" + diff = ( + "diff --git a/removed.py b/removed.py\n" + "deleted file mode 100644\n" + "index abcdef1..0000000\n" + "--- a/removed.py\n" + "+++ /dev/null\n" + "@@ -1,3 +0,0 @@\n" + "-line one\n" + "-line two\n" + "-line three\n" + ) + result = _format_annotated(diff, ["removed.py"]) + + assert "## File: 'removed.py'" in result + assert "__old hunk__" in result + assert "__new hunk__" not in result + + # All lines should be removed with - prefix + lines = result.split("\n") + removed_lines = [l for l in lines if "-" in l and "line" in l] + assert len(removed_lines) == 3 + + def test_hunk_no_function_context(self): + """Hunk with bare @@ (no trailing function) omits the | part.""" + diff = ( + "diff --git a/plain.txt b/plain.txt\n" + "index abc..def 100644\n" + "--- a/plain.txt\n" + "+++ b/plain.txt\n" + "@@ -1,3 +1,4 @@\n" + " existing\n" + "+added\n" + " more\n" + " end\n" + ) + result = _format_annotated(diff, ["plain.txt"]) + + # Should have __new hunk__ without | part + lines = result.split("\n") + hunk_headers = [l for l in lines if l.startswith("__new hunk__")] + assert len(hunk_headers) == 1 + assert "|" not in hunk_headers[0] + + def test_multiple_hunks_single_file(self): + """Multiple hunks in a single file each get their own hunk markers.""" + diff = ( + "diff --git a/multi.py b/multi.py\n" + "index abc..def 100644\n" + "--- a/multi.py\n" + "+++ b/multi.py\n" + "@@ -5,3 +5,4 @@ def first_func\n" + " ctx1\n" + "+add1\n" + " ctx2\n" + " ctx3\n" + "@@ -20,3 +21,4 @@ def second_func\n" + " ctx4\n" + "+add2\n" + " ctx5\n" + " ctx6\n" + ) + result = _format_annotated(diff, ["multi.py"]) + + # Should have exactly one file header + assert result.count("## File: 'multi.py'") == 1 + + # Should have two __new hunk__ sections + lines = result.split("\n") + new_hunk_headers = [l for l in lines if l.startswith("__new hunk__")] + assert len(new_hunk_headers) == 2 + + # Both should have function context + assert "__new hunk__ | def first_func" in result + assert "__new hunk__ | def second_func" in result + + # Verify line numbers are correct for each hunk + # First hunk: starts at line 5, add1 should be at line 6 + add1_lines = [l for l in lines if "+add1" in l] + assert any("6" in l for l in add1_lines) + + # Second hunk: starts at line 21, add2 should be at line 22 + add2_lines = [l for l in lines if "+add2" in l] + assert any("22" in l for l in add2_lines) + + def test_annotated_via_tools_get_chunk(self): + """Annotated format works end-to-end via DiffChunkTools.get_chunk.""" + test_data_dir = Path(__file__).parent / "test_data" + react_diff = test_data_dir / "react_18.0_to_18.3.diff" + if not react_diff.exists(): + pytest.skip("React test diff not found") + + tools = DiffChunkTools() + tools.load_diff(str(react_diff), max_chunk_lines=3000) + + annotated = tools.get_chunk(str(react_diff), 1, format="annotated", include_context=False) + + # Should contain structural elements + assert "## File:" in annotated + assert "__new hunk__" in annotated + + def test_annotated_line_numbers_accuracy(self): + """Verify line numbers are accurate for a known input.""" + diff = ( + "diff --git a/example.py b/example.py\n" + "index abc..def 100644\n" + "--- a/example.py\n" + "+++ b/example.py\n" + "@@ -45,5 +47,7 @@ def authenticate\n" + " unchanged line\n" + " unchanged line\n" + "+new line added\n" + "+another new line\n" + " unchanged line\n" + "-old line removed\n" + " last line\n" + ) + result = _format_annotated(diff, ["example.py"]) + lines = result.split("\n") + + # __new hunk__: context starts at new_line=47 + # Line 47: " unchanged line" + # Line 48: " unchanged line" + # Line 49: "+new line added" + # Line 50: "+another new line" + # Line 51: " unchanged line" + # skip removed + # Line 52: " last line" + new_hunk_lines = [] + in_new = False + for l in lines: + if l.startswith("__new hunk__"): + in_new = True + continue + if l.startswith("__old hunk__") or l.startswith("## File:"): + in_new = False + continue + if in_new and l.strip(): + new_hunk_lines.append(l) + + assert len(new_hunk_lines) == 6 + assert new_hunk_lines[0].strip().startswith("47") + assert new_hunk_lines[1].strip().startswith("48") + assert "49" in new_hunk_lines[2] and "+new line added" in new_hunk_lines[2] + assert "50" in new_hunk_lines[3] and "+another new line" in new_hunk_lines[3] + assert new_hunk_lines[4].strip().startswith("51") + assert new_hunk_lines[5].strip().startswith("52") From 196761d619a7b8346312100a81f687671c6c0072 Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 00:27:49 +0300 Subject: [PATCH 03/11] add compact format mode for token-efficient output Compact mode shows only new hunks (context + added lines with new-file line numbers), omitting removed lines and old hunk sections entirely. Reuses the annotated format's hunk parsing infrastructure. --- src/formatter.py | 28 ++++- tests/test_mcp_components.py | 224 ++++++++++++++++++++++++++++++++++- 2 files changed, 250 insertions(+), 2 deletions(-) diff --git a/src/formatter.py b/src/formatter.py index 7d15eb4..037618a 100644 --- a/src/formatter.py +++ b/src/formatter.py @@ -214,6 +214,32 @@ def _format_annotated(content: str, chunk_files: List[str]) -> str: return "\n".join(output_parts) +def _format_compact(content: str, chunk_files: List[str]) -> str: + """Format chunk content into compact format: only new hunks (context + added lines). + + Omits removed lines and __old hunk__ sections entirely. + Keeps ## File: headers and __new hunk__ markers for structure. + """ + sections = _parse_file_sections(content) + + if not sections: + return content + + output_parts: List[str] = [] + + for section in sections: + output_parts.append(f"## File: '{section.path}'") + + for hunk in section.hunks: + # Only emit __new hunk__ if there are added or context lines + if _has_added_or_context_lines(hunk): + output_parts.append(_hunk_header("__new hunk__", hunk.function_context)) + output_parts.extend(_build_new_hunk(hunk)) + # Skip __old hunk__ entirely + + return "\n".join(output_parts) + + def format_chunk(content: str, mode: FormatMode, chunk_files: List[str]) -> str: """Format chunk content according to the specified mode. @@ -230,5 +256,5 @@ def format_chunk(content: str, mode: FormatMode, chunk_files: List[str]) -> str: if mode == FormatMode.ANNOTATED: return _format_annotated(content, chunk_files) if mode == FormatMode.COMPACT: - return content # Placeholder - implemented in Phase 3 + return _format_compact(content, chunk_files) return content diff --git a/tests/test_mcp_components.py b/tests/test_mcp_components.py index 8857371..082f56e 100644 --- a/tests/test_mcp_components.py +++ b/tests/test_mcp_components.py @@ -4,7 +4,7 @@ import pytest -from src.formatter import _format_annotated +from src.formatter import _format_annotated, _format_compact from src.tools import DiffChunkTools @@ -600,3 +600,225 @@ def test_annotated_line_numbers_accuracy(self): assert "50" in new_hunk_lines[3] and "+another new line" in new_hunk_lines[3] assert new_hunk_lines[4].strip().startswith("51") assert new_hunk_lines[5].strip().startswith("52") + + +class TestCompactFormat: + """Tests for the compact format output.""" + + def test_compact_omits_removed_lines(self): + """Compact output contains no removed lines (no - prefixed content lines).""" + diff = ( + "diff --git a/src/auth.py b/src/auth.py\n" + "index abc1234..def5678 100644\n" + "--- a/src/auth.py\n" + "+++ b/src/auth.py\n" + "@@ -10,5 +10,5 @@ def authenticate\n" + " unchanged\n" + "-old line removed\n" + "+new line added\n" + " trailing ctx\n" + " end\n" + ) + result = _format_compact(diff, ["src/auth.py"]) + + # Split into lines and check content lines (skip headers/markers) + lines = result.split("\n") + content_lines = [ + l for l in lines + if not l.startswith("## File:") and not l.startswith("__") + and l.strip() + ] + # No content line should have a - prefix (removed line format is " -text") + for line in content_lines: + stripped = line.strip() + assert not stripped.startswith("-"), ( + f"Found removed line in compact output: {line!r}" + ) + + def test_compact_no_old_hunk_sections(self): + """No __old hunk__ sections appear in compact output.""" + diff = ( + "diff --git a/src/auth.py b/src/auth.py\n" + "index abc1234..def5678 100644\n" + "--- a/src/auth.py\n" + "+++ b/src/auth.py\n" + "@@ -10,4 +10,5 @@ def authenticate\n" + " unchanged\n" + " also unchanged\n" + "+new line\n" + " trailing ctx\n" + "@@ -30,3 +31,3 @@ def logout\n" + " ctx\n" + "-old removed\n" + "+new replaced\n" + " ctx end\n" + ) + result = _format_compact(diff, ["src/auth.py"]) + + assert "__old hunk__" not in result + assert "__new hunk__" in result + + def test_compact_line_numbers_and_plus_prefix(self): + """Line numbers use new-file numbering, + prefixes on added lines.""" + diff = ( + "diff --git a/example.py b/example.py\n" + "index abc..def 100644\n" + "--- a/example.py\n" + "+++ b/example.py\n" + "@@ -45,5 +47,7 @@ def authenticate\n" + " unchanged line\n" + " unchanged line\n" + "+new line added\n" + "+another new line\n" + " unchanged line\n" + "-old line removed\n" + " last line\n" + ) + result = _format_compact(diff, ["example.py"]) + lines = result.split("\n") + + # Collect content lines (not headers/markers) + content_lines = [] + in_hunk = False + for l in lines: + if l.startswith("__new hunk__"): + in_hunk = True + continue + if l.startswith("## File:"): + in_hunk = False + continue + if in_hunk and l.strip(): + content_lines.append(l) + + # Should have 6 lines: 2 context, 2 added, 1 context (skip removed), 1 context + assert len(content_lines) == 6 + + # Verify new-file line numbers + assert content_lines[0].strip().startswith("47") # first context + assert content_lines[1].strip().startswith("48") # second context + assert "49" in content_lines[2] and "+new line added" in content_lines[2] + assert "50" in content_lines[3] and "+another new line" in content_lines[3] + assert content_lines[4].strip().startswith("51") # context after adds + # removed line is skipped, so next is "last line" at 52 + assert content_lines[5].strip().startswith("52") + + def test_compact_multi_file_multi_hunk(self): + """Multi-file, multi-hunk content formatted correctly in compact mode.""" + diff = ( + "diff --git a/src/auth.py b/src/auth.py\n" + "index abc1234..def5678 100644\n" + "--- a/src/auth.py\n" + "+++ b/src/auth.py\n" + "@@ -10,4 +10,5 @@ def authenticate\n" + " unchanged\n" + " also unchanged\n" + "+new line\n" + " trailing ctx\n" + "@@ -30,3 +31,3 @@ def logout\n" + " ctx\n" + "-old removed\n" + "+new replaced\n" + " ctx end\n" + "diff --git a/src/utils.py b/src/utils.py\n" + "index 1111111..2222222 100644\n" + "--- a/src/utils.py\n" + "+++ b/src/utils.py\n" + "@@ -1,3 +1,4 @@\n" + " first\n" + "+inserted\n" + " second\n" + " third\n" + ) + result = _format_compact(diff, ["src/auth.py", "src/utils.py"]) + + # File headers present + assert "## File: 'src/auth.py'" in result + assert "## File: 'src/utils.py'" in result + + # __new hunk__ markers present with function context + assert "__new hunk__ | def authenticate" in result + assert "__new hunk__ | def logout" in result + + # No __old hunk__ sections + assert "__old hunk__" not in result + + # Verify no removed lines in output content + lines = result.split("\n") + content_lines = [ + l for l in lines + if not l.startswith("## File:") and not l.startswith("__") + and l.strip() + ] + for line in content_lines: + stripped = line.strip() + assert not stripped.startswith("-"), ( + f"Found removed line in compact output: {line!r}" + ) + + # Should have three __new hunk__ markers total (2 for auth.py, 1 for utils.py) + new_hunk_count = sum(1 for l in lines if l.startswith("__new hunk__")) + assert new_hunk_count == 3 + + def test_compact_deleted_file_no_output(self): + """Deleted file (only removed lines) produces file header but no hunk content.""" + diff = ( + "diff --git a/removed.py b/removed.py\n" + "deleted file mode 100644\n" + "index abcdef1..0000000\n" + "--- a/removed.py\n" + "+++ /dev/null\n" + "@@ -1,3 +0,0 @@\n" + "-line one\n" + "-line two\n" + "-line three\n" + ) + result = _format_compact(diff, ["removed.py"]) + + assert "## File: 'removed.py'" in result + # No hunk sections at all since there are no added/context lines + assert "__new hunk__" not in result + assert "__old hunk__" not in result + + def test_compact_new_file_all_added(self): + """New file (only added lines) produces correct compact output.""" + diff = ( + "diff --git a/newfile.py b/newfile.py\n" + "new file mode 100644\n" + "index 0000000..abcdef1\n" + "--- /dev/null\n" + "+++ b/newfile.py\n" + "@@ -0,0 +1,3 @@\n" + "+line one\n" + "+line two\n" + "+line three\n" + ) + result = _format_compact(diff, ["newfile.py"]) + + assert "## File: 'newfile.py'" in result + assert "__new hunk__" in result + assert "__old hunk__" not in result + + # All lines should have + prefix and line numbers + lines = result.split("\n") + added_lines = [l for l in lines if "+" in l and "line" in l] + assert len(added_lines) == 3 + + def test_compact_via_tools_get_chunk(self): + """Compact format works end-to-end via DiffChunkTools.get_chunk.""" + test_data_dir = Path(__file__).parent / "test_data" + react_diff = test_data_dir / "react_18.0_to_18.3.diff" + if not react_diff.exists(): + pytest.skip("React test diff not found") + + tools = DiffChunkTools() + tools.load_diff(str(react_diff), max_chunk_lines=3000) + + compact = tools.get_chunk( + str(react_diff), 1, format="compact", include_context=False + ) + + # Should contain structural elements + assert "## File:" in compact + assert "__new hunk__" in compact + # Should NOT contain __old hunk__ + assert "__old hunk__" not in compact From 7c61d9c4573ff0f9836aee2da9efd683c89ba972 Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 00:36:06 +0300 Subject: [PATCH 04/11] track excluded file count in load_diff The load_diff response now includes a files_excluded count showing how many files were removed by exclude_patterns. --- src/chunker.py | 4 +++ src/models.py | 1 + src/server.py | 2 +- src/tools.py | 1 + tests/test_mcp_components.py | 57 ++++++++++++++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/chunker.py b/src/chunker.py index fb58f10..a9d4cbb 100644 --- a/src/chunker.py +++ b/src/chunker.py @@ -41,6 +41,8 @@ def chunk_diff( if not file_changes: raise ValueError("Diff file parsed successfully but contains no changes") + files_excluded_count = 0 + for files, content in file_changes: # Apply filters if skip_trivial and self.parser.is_trivial_change(content): @@ -52,6 +54,7 @@ def chunk_diff( if not self.parser.should_include_file( files, include_patterns, exclude_patterns ): + files_excluded_count += 1 continue content_lines = self.parser.count_lines(content) @@ -140,6 +143,7 @@ def chunk_diff( # Update session statistics session.update_stats() + session.stats.files_excluded = files_excluded_count if not session.chunks: raise ValueError( diff --git a/src/models.py b/src/models.py index cbab068..3859bb5 100644 --- a/src/models.py +++ b/src/models.py @@ -21,6 +21,7 @@ class DiffStats: total_files: int total_lines: int chunks_count: int + files_excluded: int = 0 @dataclass diff --git a/src/server.py b/src/server.py index 96a387f..bc7fee8 100644 --- a/src/server.py +++ b/src/server.py @@ -52,7 +52,7 @@ def load_diff( Field(description="Comma-separated glob patterns for files to exclude"), ] = None, ) -> str: - """Parse and load a diff file with custom chunking settings. Use this tool ONLY when you need non-default settings (custom chunk sizes, filtering patterns). Otherwise, use list_chunks, get_chunk, or find_chunks_for_files which auto-load with optimal defaults. CRITICAL: You must use an absolute directory path - relative paths will fail. The diff file will be too large for direct reading, so you MUST use diffchunk tools for navigation. When using tracking documents for analysis, remember to clean up tracking state before presenting final results.""" + """Parse and load a diff file with custom chunking settings. Use this tool ONLY when you need non-default settings (custom chunk sizes, filtering patterns). Otherwise, use list_chunks, get_chunk, or find_chunks_for_files which auto-load with optimal defaults. CRITICAL: You must use an absolute directory path - relative paths will fail. The diff file will be too large for direct reading, so you MUST use diffchunk tools for navigation. When using tracking documents for analysis, remember to clean up tracking state before presenting final results. The response includes a `files_excluded` count showing how many files were removed by exclude_patterns.""" logger.info("load_diff: %s", os.path.basename(absolute_file_path)) logger.debug("load_diff full path: %s", absolute_file_path) result = tools.load_diff( diff --git a/src/tools.py b/src/tools.py index ca1516b..9711119 100644 --- a/src/tools.py +++ b/src/tools.py @@ -148,6 +148,7 @@ def load_diff( "files": session.stats.total_files, "total_lines": session.stats.total_lines, "file_path": absolute_file_path, + "files_excluded": session.stats.files_excluded, } def list_chunks(self, absolute_file_path: str) -> List[Dict[str, Any]]: diff --git a/tests/test_mcp_components.py b/tests/test_mcp_components.py index 082f56e..63f58a4 100644 --- a/tests/test_mcp_components.py +++ b/tests/test_mcp_components.py @@ -822,3 +822,60 @@ def test_compact_via_tools_get_chunk(self): assert "__new hunk__" in compact # Should NOT contain __old hunk__ assert "__old hunk__" not in compact + + +class TestFilesExcludedCount: + """Tests for files_excluded tracking in load_diff responses.""" + + @pytest.fixture + def test_data_dir(self): + """Return path to test data directory.""" + return Path(__file__).parent / "test_data" + + @pytest.fixture + def go_diff_file(self, test_data_dir): + """Return path to Go test diff file.""" + diff_file = test_data_dir / "go_version_upgrade_1.22_to_1.23.diff" + if not diff_file.exists(): + pytest.skip("Go test diff not found") + return str(diff_file) + + @pytest.fixture + def react_diff_file(self, test_data_dir): + """Return path to React test diff file.""" + diff_file = test_data_dir / "react_18.0_to_18.3.diff" + if not diff_file.exists(): + pytest.skip("React test diff not found") + return str(diff_file) + + def test_files_excluded_with_exclude_patterns(self, go_diff_file): + """Loading with exclude_patterns reports files_excluded > 0.""" + tools = DiffChunkTools() + result = tools.load_diff(go_diff_file, exclude_patterns="*.md") + + assert "files_excluded" in result + assert result["files_excluded"] > 0 + + def test_files_excluded_zero_without_patterns(self, go_diff_file): + """Loading without exclude_patterns reports files_excluded == 0.""" + tools = DiffChunkTools() + result = tools.load_diff(go_diff_file) + + assert "files_excluded" in result + assert result["files_excluded"] == 0 + + def test_excluded_files_not_in_list_chunks(self, react_diff_file): + """Files removed by exclude_patterns do not appear in list_chunks.""" + tools = DiffChunkTools() + + # Load with json files excluded + result = tools.load_diff(react_diff_file, exclude_patterns="*.json") + assert result["files_excluded"] > 0 + + # Verify no .json files appear in chunk listings + chunks = tools.list_chunks(react_diff_file) + for chunk_info in chunks: + for file_path in chunk_info["files"]: + assert not file_path.endswith(".json"), ( + f"Excluded file '{file_path}' found in list_chunks output" + ) From 604a85b67e25be782f422e08c4bfdfe457a7c955 Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 00:44:47 +0300 Subject: [PATCH 05/11] add context_lines parameter to load_diff Strips excess context lines from diff hunks at load time, reducing token usage. Applied per-file before chunking via reduce_context() in the parser. --- src/chunker.py | 9 +- src/parser.py | 191 ++++++++++++++++++++++++++ src/server.py | 7 + src/tools.py | 8 ++ tests/test_mcp_components.py | 252 +++++++++++++++++++++++++++++++++++ 5 files changed, 466 insertions(+), 1 deletion(-) diff --git a/src/chunker.py b/src/chunker.py index a9d4cbb..059c1ee 100644 --- a/src/chunker.py +++ b/src/chunker.py @@ -1,7 +1,7 @@ """Diff chunking functionality.""" import re -from typing import Dict, List, Tuple +from typing import Dict, List, Optional, Tuple from .models import DiffChunk, DiffSession from .parser import DiffParser @@ -21,6 +21,7 @@ def chunk_diff( include_patterns: List[str] | None = None, exclude_patterns: List[str] | None = None, max_chunk_lines: int | None = None, + context_lines: Optional[int] = None, ) -> None: """Chunk a diff file into the session.""" if max_chunk_lines is None: @@ -38,6 +39,12 @@ def chunk_diff( except ValueError as e: raise ValueError(f"Failed to parse diff: {e}") + if context_lines is not None: + file_changes = [ + (files, self.parser.reduce_context(content, context_lines)) + for files, content in file_changes + ] + if not file_changes: raise ValueError("Diff file parsed successfully but contains no changes") diff --git a/src/parser.py b/src/parser.py index 9f915b7..9333f0b 100644 --- a/src/parser.py +++ b/src/parser.py @@ -159,6 +159,197 @@ def _read_diff_file(self, file_path: str) -> List[str]: return content.splitlines(keepends=True) + @staticmethod + def reduce_context(content: str, context_lines: int) -> str: + """Reduce context lines in a diff file section to the specified count. + + Args: + content: Raw diff content for a single file section (including diff --git header). + context_lines: Number of context lines to keep around each change. + + Returns: + Modified diff content with reduced context. + """ + lines = content.rstrip("\n").split("\n") + + hunk_header_re = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*)$") + + # Separate file header from hunks + header_lines: list[str] = [] + hunks: list[tuple[str, list[str]]] = [] # (hunk_header, body_lines) + + in_hunk = False + current_hunk_header = "" + current_hunk_body: list[str] = [] + + for line in lines: + if hunk_header_re.match(line): + # Save previous hunk if any + if in_hunk: + hunks.append((current_hunk_header, current_hunk_body)) + in_hunk = True + current_hunk_header = line + current_hunk_body = [] + elif in_hunk: + current_hunk_body.append(line) + else: + header_lines.append(line) + + # Save last hunk + if in_hunk: + hunks.append((current_hunk_header, current_hunk_body)) + + if not hunks: + return content + + # Process each hunk: classify lines and mark which to keep + # First, collect all classified lines across all hunks for potential merging + classified_hunks: list[ + tuple[str, list[tuple[str, str]]] + ] = [] # (header, [(type, text)]) + + for hunk_header, body in hunks: + classified: list[tuple[str, str]] = [] + for line in body: + if line.startswith("+") and not line.startswith("+++"): + classified.append(("add", line)) + elif line.startswith("-") and not line.startswith("---"): + classified.append(("remove", line)) + elif line == "\\ No newline at end of file": + classified.append(("meta", line)) + else: + classified.append(("context", line)) + classified_hunks.append((hunk_header, classified)) + + # Process each hunk: mark context lines within distance of changes + reduced_hunks: list[tuple[str, list[tuple[str, str, bool]]]] = [] + + for hunk_header, classified in classified_hunks: + # Find indices of change lines (add/remove) + change_indices: set[int] = set() + for i, (line_type, _) in enumerate(classified): + if line_type in ("add", "remove"): + change_indices.add(i) + + # Mark context lines within context_lines distance of any change + keep: set[int] = set() + for idx in change_indices: + keep.add(idx) + for d in range(1, context_lines + 1): + if idx - d >= 0: + keep.add(idx - d) + if idx + d < len(classified): + keep.add(idx + d) + + # Also keep meta lines adjacent to kept lines + for i, (line_type, _) in enumerate(classified): + if line_type == "meta": + # Keep if adjacent to a kept line + if (i - 1 in keep) or (i + 1 in keep): + keep.add(i) + + marked = [ + (line_type, text, i in keep) + for i, (line_type, text) in enumerate(classified) + ] + reduced_hunks.append((hunk_header, marked)) + + # Build output hunks with only kept lines, recalculating headers + output_hunks: list[str] = [] + + for hunk_header, marked in reduced_hunks: + m = hunk_header_re.match(hunk_header) + if not m: + continue + orig_old_start = int(m.group(1)) + orig_new_start = int(m.group(3)) + trailing_context = m.group(5) # e.g. " def authenticate" + + # Filter to kept lines only + kept_lines = [(lt, text) for lt, text, keep_flag in marked if keep_flag] + + if not kept_lines: + continue + + # Walk all marked lines, track old/new position, + # for kept lines record their position. + positions: list[ + tuple[str, str, int, int] + ] = [] # (type, text, old_pos, new_pos) + + old_pos = orig_old_start + new_pos = orig_new_start + + for line_type, text, keep_flag in marked: + if keep_flag: + positions.append((line_type, text, old_pos, new_pos)) + # Advance positions + if line_type == "context": + old_pos += 1 + new_pos += 1 + elif line_type == "add": + new_pos += 1 + elif line_type == "remove": + old_pos += 1 + # meta lines don't advance positions + + if not positions: + continue + + # Split positions into contiguous sub-hunks. + # Two consecutive kept lines are "contiguous" if there are no gaps + # (no dropped context lines between them). + # We detect gaps by checking if position jumps. + sub_hunks: list[list[tuple[str, str, int, int]]] = [] + current_sub: list[tuple[str, str, int, int]] = [positions[0]] + + for i in range(1, len(positions)): + prev_type, _, prev_old, prev_new = positions[i - 1] + curr_type, _, curr_old, curr_new = positions[i] + + # Expected next positions + exp_old = prev_old + (1 if prev_type in ("context", "remove") else 0) + exp_new = prev_new + (1 if prev_type in ("context", "add") else 0) + + if curr_old == exp_old and curr_new == exp_new: + current_sub.append(positions[i]) + else: + sub_hunks.append(current_sub) + current_sub = [positions[i]] + + sub_hunks.append(current_sub) + + # Generate hunk output for each sub-hunk + for sub in sub_hunks: + sub_old_start = sub[0][2] + sub_new_start = sub[0][3] + + sub_old_count = sum( + 1 for lt, _, _, _ in sub if lt in ("context", "remove") + ) + sub_new_count = sum( + 1 for lt, _, _, _ in sub if lt in ("context", "add") + ) + + # Build header - use trailing context from the original header + # only for the first sub-hunk + ctx_text = trailing_context if sub is sub_hunks[0] else "" + new_header = f"@@ -{sub_old_start},{sub_old_count} +{sub_new_start},{sub_new_count} @@{ctx_text}" + + hunk_lines = [new_header] + for lt, text, _, _ in sub: + if lt != "meta": + hunk_lines.append(text) + else: + hunk_lines.append(text) + output_hunks.append("\n".join(hunk_lines)) + + if not output_hunks: + # No hunks survived - return just the header + return "\n".join(header_lines) + + return "\n".join(header_lines) + "\n" + "\n".join(output_hunks) + def count_lines(self, content: str) -> int: """Count meaningful lines in diff content.""" return len([line for line in content.split("\n") if line.strip()]) diff --git a/src/server.py b/src/server.py index bc7fee8..6647b39 100644 --- a/src/server.py +++ b/src/server.py @@ -51,6 +51,12 @@ def load_diff( Optional[str], Field(description="Comma-separated glob patterns for files to exclude"), ] = None, + context_lines: Annotated[ + Optional[int], + Field( + description="Number of context lines around each change (default: keep all from diff file)" + ), + ] = None, ) -> str: """Parse and load a diff file with custom chunking settings. Use this tool ONLY when you need non-default settings (custom chunk sizes, filtering patterns). Otherwise, use list_chunks, get_chunk, or find_chunks_for_files which auto-load with optimal defaults. CRITICAL: You must use an absolute directory path - relative paths will fail. The diff file will be too large for direct reading, so you MUST use diffchunk tools for navigation. When using tracking documents for analysis, remember to clean up tracking state before presenting final results. The response includes a `files_excluded` count showing how many files were removed by exclude_patterns.""" logger.info("load_diff: %s", os.path.basename(absolute_file_path)) @@ -62,6 +68,7 @@ def load_diff( skip_generated=skip_generated, include_patterns=include_patterns, exclude_patterns=exclude_patterns, + context_lines=context_lines, ) return json.dumps(result, indent=2) diff --git a/src/tools.py b/src/tools.py index 9711119..81ea688 100644 --- a/src/tools.py +++ b/src/tools.py @@ -53,6 +53,7 @@ def _load_diff_internal( skip_generated: bool = True, include_patterns: Optional[str] = None, exclude_patterns: Optional[str] = None, + context_lines: Optional[int] = None, ) -> DiffSession: """Internal method to load and parse a diff file.""" # Validate inputs @@ -66,6 +67,10 @@ def _load_diff_internal( if not isinstance(max_chunk_lines, int) or max_chunk_lines <= 0: raise ValueError("max_chunk_lines must be a positive integer") + if context_lines is not None: + if not isinstance(context_lines, int) or context_lines < 0: + raise ValueError("context_lines must be a non-negative integer or None") + # Canonicalize path resolved_file_path = os.path.realpath(os.path.expanduser(absolute_file_path)) @@ -105,6 +110,7 @@ def _load_diff_internal( skip_generated=skip_generated, include_patterns=include_list, exclude_patterns=exclude_list, + context_lines=context_lines, ) elapsed = time.monotonic() - start logger.info( @@ -131,6 +137,7 @@ def load_diff( skip_generated: bool = True, include_patterns: Optional[str] = None, exclude_patterns: Optional[str] = None, + context_lines: Optional[int] = None, ) -> Dict[str, Any]: """Load and parse a diff file into chunks.""" session = self._load_diff_internal( @@ -140,6 +147,7 @@ def load_diff( skip_generated=skip_generated, include_patterns=include_patterns, exclude_patterns=exclude_patterns, + context_lines=context_lines, ) # Return overview diff --git a/tests/test_mcp_components.py b/tests/test_mcp_components.py index 63f58a4..9c04271 100644 --- a/tests/test_mcp_components.py +++ b/tests/test_mcp_components.py @@ -879,3 +879,255 @@ def test_excluded_files_not_in_list_chunks(self, react_diff_file): assert not file_path.endswith(".json"), ( f"Excluded file '{file_path}' found in list_chunks output" ) + + +class TestContextLinesParameter: + """Tests for the context_lines parameter on reduce_context and load_diff.""" + + def _make_diff( + self, + context_before: int = 5, + context_after: int = 5, + old_start: int = 10, + new_start: int = 10, + func_context: str = "", + ) -> str: + """Build a simple single-hunk diff with configurable context.""" + ctx_before = [f" context_before_{i}" for i in range(context_before)] + ctx_after = [f" context_after_{i}" for i in range(context_after)] + changes = ["+added_line", "-removed_line"] + + old_count = context_before + context_after + 1 # context + removed + new_count = context_before + context_after + 1 # context + added + + trailing = f" {func_context}" if func_context else "" + header = f"@@ -{old_start},{old_count} +{new_start},{new_count} @@{trailing}" + + lines = [ + "diff --git a/example.py b/example.py", + "index abc1234..def5678 100644", + "--- a/example.py", + "+++ b/example.py", + header, + *ctx_before, + *changes, + *ctx_after, + ] + return "\n".join(lines) + + def test_reduce_context_basic(self): + """Diff with 5+ context lines reduced to context_lines=1.""" + from src.parser import DiffParser + + diff = self._make_diff(context_before=5, context_after=5) + result = DiffParser.reduce_context(diff, 1) + + # Should contain only 1 context line before and 1 after the change + result_lines = result.split("\n") + # Find content lines (not headers) + content_lines = [] + in_hunk = False + for line in result_lines: + if line.startswith("@@"): + in_hunk = True + continue + if in_hunk: + content_lines.append(line) + + # Count context lines + ctx_lines = [l for l in content_lines if l.startswith(" ")] + assert len(ctx_lines) == 2 # 1 before + 1 after + + # Change lines should still be present + assert any("+added_line" in l for l in content_lines) + assert any("-removed_line" in l for l in content_lines) + + def test_reduce_context_recalculated_headers(self): + """Hunk headers are recalculated after context reduction.""" + from src.parser import DiffParser + + diff = self._make_diff( + context_before=5, context_after=5, old_start=10, new_start=10 + ) + result = DiffParser.reduce_context(diff, 1) + + import re + + hunk_re = re.compile(r"^@@ -(\d+),(\d+) \+(\d+),(\d+) @@") + result_lines = result.split("\n") + hunk_headers = [l for l in result_lines if hunk_re.match(l)] + assert len(hunk_headers) >= 1 + + m = hunk_re.match(hunk_headers[0]) + assert m is not None + old_start = int(m.group(1)) + old_count = int(m.group(2)) + new_start = int(m.group(3)) + new_count = int(m.group(4)) + + # With 1 context line kept before the change, old_start should be + # original start + (5 - 1) = 14 (skipped 4 context lines) + assert old_start == 14 + assert new_start == 14 + + # old_count = 1 context before + 1 removed + 1 context after = 3 + assert old_count == 3 + # new_count = 1 context before + 1 added + 1 context after = 3 + assert new_count == 3 + + def test_reduce_context_none_keeps_all(self): + """context_lines=None (not calling reduce_context) keeps all context.""" + from src.parser import DiffParser + + diff = self._make_diff(context_before=5, context_after=5) + # When context_lines is None, reduce_context is not called, + # so we verify by calling with a large value that keeps everything + result = DiffParser.reduce_context(diff, 100) + + # All context lines should be preserved + result_lines = result.split("\n") + ctx_lines = [l for l in result_lines if l.startswith(" context_")] + assert len(ctx_lines) == 10 # 5 before + 5 after + + def test_reduce_context_zero_keeps_only_changes(self): + """context_lines=0 keeps only added/removed lines, no context.""" + from src.parser import DiffParser + + diff = self._make_diff(context_before=5, context_after=5) + result = DiffParser.reduce_context(diff, 0) + + result_lines = result.split("\n") + # Find content lines in hunks + content_lines = [] + in_hunk = False + for line in result_lines: + if line.startswith("@@"): + in_hunk = True + continue + if in_hunk: + content_lines.append(line) + + # No context lines + ctx_lines = [l for l in content_lines if l.startswith(" ")] + assert len(ctx_lines) == 0 + + # Changes are still present + assert any("+added_line" in l for l in content_lines) + assert any("-removed_line" in l for l in content_lines) + + def test_reduce_context_overlapping_windows(self): + """Two nearby changes with overlapping context windows preserve shared context.""" + from src.parser import DiffParser + + # Two changes separated by 2 context lines, with context_lines=2 + # The context between them overlaps and should all be kept + diff = ( + "diff --git a/example.py b/example.py\n" + "index abc..def 100644\n" + "--- a/example.py\n" + "+++ b/example.py\n" + "@@ -1,12 +1,14 @@\n" + " far_before_1\n" + " far_before_2\n" + " far_before_3\n" + " near_before\n" + "+first_add\n" + " between_1\n" + " between_2\n" + "-second_remove\n" + "+second_add\n" + " near_after\n" + " far_after_1\n" + " far_after_2\n" + " far_after_3\n" + ) + result = DiffParser.reduce_context(diff, 2) + + result_lines = result.split("\n") + content_lines = [] + in_hunk = False + for line in result_lines: + if line.startswith("@@"): + in_hunk = True + continue + if in_hunk: + content_lines.append(line) + + # With context_lines=2: + # - first_add needs 2 before (far_before_3, near_before) and 2 after (between_1, between_2) + # - second_remove/second_add needs 2 before (between_1, between_2) and 2 after (near_after, far_after_1) + # The between_1 and between_2 lines are shared context + ctx_lines = [l for l in content_lines if l.startswith(" ")] + + # Should keep: far_before_3, near_before, between_1, between_2, near_after, far_after_1 + assert len(ctx_lines) == 6 + + # far_before_1 and far_before_2 should be dropped + assert not any("far_before_1" in l for l in content_lines) + assert not any("far_before_2" in l for l in content_lines) + + # far_after_2 and far_after_3 should be dropped + assert not any("far_after_2" in l for l in content_lines) + assert not any("far_after_3" in l for l in content_lines) + + def test_context_lines_negative_raises_valueerror(self): + """Negative context_lines raises ValueError via tools validation.""" + tools = DiffChunkTools() + with pytest.raises(ValueError, match="non-negative integer"): + tools.load_diff("/some/file.diff", context_lines=-1) + + def test_context_lines_via_load_diff(self): + """context_lines works end-to-end through load_diff with real diff data.""" + test_data_dir = Path(__file__).parent / "test_data" + react_diff = test_data_dir / "react_18.0_to_18.3.diff" + if not react_diff.exists(): + pytest.skip("React test diff not found") + + tools = DiffChunkTools() + + # Load with all context (default) + result_all = tools.load_diff(str(react_diff), max_chunk_lines=5000) + + # Load with reduced context + result_reduced = tools.load_diff( + str(react_diff), max_chunk_lines=5000, context_lines=1 + ) + + # Reduced context should have fewer or equal total lines + assert result_reduced["total_lines"] <= result_all["total_lines"] + + # Both should still have files and chunks + assert result_reduced["files"] > 0 + assert result_reduced["chunks"] > 0 + + def test_reduce_context_preserves_file_header(self): + """File header lines are preserved unchanged after context reduction.""" + from src.parser import DiffParser + + diff = self._make_diff(context_before=5, context_after=5) + result = DiffParser.reduce_context(diff, 1) + + assert result.startswith("diff --git a/example.py b/example.py") + assert "--- a/example.py" in result + assert "+++ b/example.py" in result + assert "index abc1234..def5678 100644" in result + + def test_reduce_context_preserves_function_context(self): + """Trailing function context on @@ headers is preserved.""" + from src.parser import DiffParser + + diff = self._make_diff( + context_before=5, context_after=5, func_context="def my_function" + ) + result = DiffParser.reduce_context(diff, 1) + + import re + + hunk_re = re.compile(r"^@@.*@@(.*)$") + for line in result.split("\n"): + m = hunk_re.match(line) + if m: + assert "def my_function" in m.group(1) + break + else: + pytest.fail("No hunk header found in reduced output") From b381efe533d602e25e159024436d803615cb812c Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 00:53:56 +0300 Subject: [PATCH 06/11] add integration tests and docs for format options Integration and edge case tests covering format mode composition, context_lines reduction, and exclude_patterns interaction. Updated design.md and README with format options documentation. --- README.md | 38 +++- docs/design.md | 80 ++++++++- tests/test_mcp_components.py | 333 +++++++++++++++++++++++++++++++++++ 3 files changed, 443 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 89350f2..7c8b3d3 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,9 @@ # diffchunk [![CI](https://github.com/peteretelej/diffchunk/actions/workflows/ci.yml/badge.svg)](https://github.com/peteretelej/diffchunk/actions/workflows/ci.yml) -[![codecov](https://codecov.io/gh/peteretelej/diffchunk/branch/main/graph/badge.svg)](https://codecov.io/gh/peteretelej/diffchunk) [![PyPI version](https://img.shields.io/pypi/v/diffchunk.svg)](https://pypi.org/project/diffchunk/) [![Python 3.10+](https://img.shields.io/badge/python-3.10+-blue.svg)](https://www.python.org/downloads/) [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) -[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff) [![uv](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/uv/main/assets/badge/v0.json)](https://github.com/astral-sh/uv) MCP server that enables LLMs to navigate large diff files efficiently. Instead of reading entire diffs sequentially, LLMs can jump directly to relevant changes using pattern-based navigation. @@ -164,10 +162,44 @@ load_diff( "/tmp/large.diff", max_chunk_lines=2000, include_patterns="*.py,*.js", - exclude_patterns="*test*" + exclude_patterns="*test*", + context_lines=2 ) ``` +### Format Options + +Use the `format` parameter on `get_chunk` to transform output for LLM consumption: + +```python +# Default - raw diff output +get_chunk("/tmp/changes.diff", 1, format="raw") + +# Annotated - structured with line numbers, file headers, hunk separation +get_chunk("/tmp/changes.diff", 1, format="annotated") + +# Compact - token-efficient, only new hunks (context + added lines) +get_chunk("/tmp/changes.diff", 1, format="compact") +``` + +**Annotated format** adds `## File:` headers, `__new hunk__`/`__old hunk__` sections with new-file line numbers, and function context from `@@` headers. + +**Compact format** shows only what was added or kept, omitting removed lines and `__old hunk__` sections entirely. Useful when you only need to see the final state. + +### Context Reduction + +Use `context_lines` on `load_diff` to reduce context lines per hunk at load time: + +```python +# Keep only 2 lines of context around each change +load_diff("/tmp/large.diff", context_lines=2) + +# Keep only changes, no context +load_diff("/tmp/large.diff", context_lines=0) +``` + +This composes with `format` - context is reduced at load time, then formatting is applied at display time. + ## Supported Formats - Git diff output (`git diff`, `git show`) diff --git a/docs/design.md b/docs/design.md index b58072c..3ce78cc 100644 --- a/docs/design.md +++ b/docs/design.md @@ -24,10 +24,11 @@ def load_diff( skip_generated: bool = True, include_patterns: Optional[str] = None, exclude_patterns: Optional[str] = None, + context_lines: Optional[int] = None, ) -> Dict[str, Any] ``` -**Returns:** `{"chunks": int, "files": int, "total_lines": int, "file_path": str}` +**Returns:** `{"chunks": int, "files": int, "total_lines": int, "file_path": str, "files_excluded": int}` ### list_chunks (Auto-loading) @@ -58,7 +59,8 @@ def list_chunks(absolute_file_path: str) -> List[Dict[str, Any]] def get_chunk( absolute_file_path: str, chunk_number: int, - include_context: bool = True + include_context: bool = True, + format: str = "raw", ) -> str ``` @@ -168,9 +170,10 @@ src/ ├── main.py # CLI entry point ├── server.py # MCP server (FastMCP module-level tools) ├── tools.py # MCP tools (DiffChunkTools) -├── models.py # Data models -├── parser.py # Diff parsing (DiffParser) -└── chunker.py # Chunking logic (DiffChunker) +├── models.py # Data models (DiffStats, FormatMode, etc.) +├── parser.py # Diff parsing (DiffParser) and context reduction +├── chunker.py # Chunking logic (DiffChunker) +└── formatter.py # Output formatting (annotated, compact) ``` ## Resources @@ -182,6 +185,73 @@ src/ - Pattern matching (glob) is case-insensitive, matching macOS/Windows filesystem behavior - Both `find_chunks_for_files` and `get_file_diff` use case-insensitive comparison +## Format Options + +### FormatMode Enum + +```python +class FormatMode(str, Enum): + RAW = "raw" # Default - unmodified diff output + ANNOTATED = "annotated" # Structured with line numbers and hunk separation + COMPACT = "compact" # Token-efficient, new hunks only +``` + +`FormatMode` inherits from `str, Enum` so values compare directly with strings. + +### `format` Parameter on `get_chunk` + +The `format` parameter is a display-time parameter on `get_chunk`. It transforms output for rendering but stored data always remains raw. + +```python +def get_chunk( + absolute_file_path: str, + chunk_number: int, + include_context: bool = True, + format: str = "raw", +) -> str +``` + +- `"raw"` (default) - returns the original diff content, identical to pre-feature behavior +- `"annotated"` - structured output with `## File:` headers, `__new hunk__`/`__old hunk__` separation, new-file line numbers, and function context from `@@` headers +- `"compact"` - token-efficient output showing only new hunks (context + added lines), omitting removed lines and `__old hunk__` sections + +Invalid format values raise `ValueError` listing valid options. + +### `context_lines` Parameter on `load_diff` + +A load-time parameter that reduces context lines per hunk before chunking. Implemented via `DiffParser.reduce_context()`. + +```python +def load_diff( + absolute_file_path: str, + ..., + context_lines: Optional[int] = None, +) -> Dict[str, Any] +``` + +- `None` (default) - keeps all context lines from the original diff +- `0` - keeps only added/removed lines, no context +- `N` - keeps up to N context lines before and after each change + +Overlapping context windows between nearby changes preserve shared context lines. Hunk headers are recalculated after reduction. Negative values raise `ValueError`. + +### `files_excluded` in `DiffStats` + +```python +@dataclass +class DiffStats: + total_files: int + total_lines: int + chunks_count: int + files_excluded: int = 0 +``` + +When `exclude_patterns` is used with `load_diff`, the `files_excluded` count reports how many files were removed by the patterns. This count is included in the `load_diff` response. + +### Feature Composition + +`format` and `context_lines` compose correctly: `context_lines` reduces context at load time (stored in the session), then `format` transforms the already-reduced content at display time. Both can be used alongside `exclude_patterns`. + ## Performance - Target: <1 second for 100k+ line diffs diff --git a/tests/test_mcp_components.py b/tests/test_mcp_components.py index 9c04271..ab45e2f 100644 --- a/tests/test_mcp_components.py +++ b/tests/test_mcp_components.py @@ -1131,3 +1131,336 @@ def test_reduce_context_preserves_function_context(self): break else: pytest.fail("No hunk header found in reduced output") + + +class TestIntegrationFormatModes: + """Integration tests: load real diffs and verify each format mode produces valid output.""" + + @pytest.fixture + def test_data_dir(self): + return Path(__file__).parent / "test_data" + + @pytest.fixture + def react_diff_file(self, test_data_dir): + diff_file = test_data_dir / "react_18.0_to_18.3.diff" + if not diff_file.exists(): + pytest.skip("React test diff not found") + return str(diff_file) + + @pytest.fixture + def go_diff_file(self, test_data_dir): + diff_file = test_data_dir / "go_version_upgrade_1.22_to_1.23.diff" + if not diff_file.exists(): + pytest.skip("Go test diff not found") + return str(diff_file) + + def test_all_format_modes_produce_valid_output(self, react_diff_file): + """Load a diff and get a chunk with each format mode; all should produce non-empty strings.""" + tools = DiffChunkTools() + tools.load_diff(react_diff_file, max_chunk_lines=3000) + + for fmt in ("raw", "annotated", "compact"): + result = tools.get_chunk( + react_diff_file, 1, include_context=False, format=fmt + ) + assert isinstance(result, str), f"format={fmt} returned non-string" + assert len(result) > 0, f"format={fmt} returned empty string" + + def test_raw_matches_default(self, react_diff_file): + """format='raw' should produce byte-identical output to no format arg.""" + tools = DiffChunkTools() + tools.load_diff(react_diff_file, max_chunk_lines=3000) + + default = tools.get_chunk(react_diff_file, 1, include_context=False) + raw = tools.get_chunk( + react_diff_file, 1, include_context=False, format="raw" + ) + assert default == raw + + def test_annotated_has_structural_elements(self, react_diff_file): + """Annotated output should contain ## File: headers and __new hunk__ markers.""" + tools = DiffChunkTools() + tools.load_diff(react_diff_file, max_chunk_lines=3000) + + result = tools.get_chunk( + react_diff_file, 1, include_context=False, format="annotated" + ) + assert "## File:" in result + assert "__new hunk__" in result + + def test_compact_has_no_old_hunks(self, react_diff_file): + """Compact output should contain ## File: but no __old hunk__.""" + tools = DiffChunkTools() + tools.load_diff(react_diff_file, max_chunk_lines=3000) + + result = tools.get_chunk( + react_diff_file, 1, include_context=False, format="compact" + ) + assert "## File:" in result + assert "__new hunk__" in result + assert "__old hunk__" not in result + + def test_context_lines_then_annotated_composition(self, react_diff_file): + """Load with context_lines=2, then get chunk with annotated format. + + Both features should compose correctly: load-time reduction then display-time formatting. + """ + tools = DiffChunkTools() + tools.load_diff(react_diff_file, max_chunk_lines=5000, context_lines=2) + + result = tools.get_chunk( + react_diff_file, 1, include_context=False, format="annotated" + ) + assert isinstance(result, str) + assert len(result) > 0 + assert "## File:" in result + assert "__new hunk__" in result + + def test_context_lines_then_compact_composition(self, react_diff_file): + """Load with context_lines=1, then get chunk with compact format.""" + tools = DiffChunkTools() + tools.load_diff(react_diff_file, max_chunk_lines=5000, context_lines=1) + + result = tools.get_chunk( + react_diff_file, 1, include_context=False, format="compact" + ) + assert isinstance(result, str) + assert len(result) > 0 + assert "## File:" in result + assert "__old hunk__" not in result + + def test_exclude_patterns_then_format(self, react_diff_file): + """Load with exclude_patterns, verify files_excluded, then get chunk with format.""" + tools = DiffChunkTools() + result = tools.load_diff(react_diff_file, exclude_patterns="*.json") + + assert result["files_excluded"] > 0 + + # Get first chunk with annotated format - should still work + annotated = tools.get_chunk( + react_diff_file, 1, include_context=False, format="annotated" + ) + assert isinstance(annotated, str) + assert len(annotated) > 0 + assert "## File:" in annotated + + # No .json files should appear in chunk listings + chunks = tools.list_chunks(react_diff_file) + for chunk_info in chunks: + for file_path in chunk_info["files"]: + assert not file_path.endswith(".json") + + def test_exclude_and_context_lines_and_format_composition(self, react_diff_file): + """All three features compose: exclude + context_lines + format.""" + tools = DiffChunkTools() + result = tools.load_diff( + react_diff_file, + exclude_patterns="*.json", + context_lines=2, + max_chunk_lines=5000, + ) + + assert result["files_excluded"] > 0 + + compact = tools.get_chunk( + react_diff_file, 1, include_context=False, format="compact" + ) + assert isinstance(compact, str) + assert len(compact) > 0 + assert "__old hunk__" not in compact + + +class TestFormatterEdgeCases: + """Edge case tests for the formatter with crafted diff content.""" + + def test_binary_file_indicator(self): + """Binary file indicator should be handled gracefully.""" + diff = ( + "diff --git a/image.png b/image.png\n" + "index abc1234..def5678 100644\n" + "Binary files a/image.png and b/image.png differ\n" + ) + # Should not crash - binary files have no hunks so formatter returns as-is + result_annotated = _format_annotated(diff, ["image.png"]) + assert isinstance(result_annotated, str) + + result_compact = _format_compact(diff, ["image.png"]) + assert isinstance(result_compact, str) + + def test_rename_only_patch(self): + """Rename-only patch (no hunks) should be handled gracefully.""" + diff = ( + "diff --git a/old_name.py b/new_name.py\n" + "similarity index 100%\n" + "rename from old_name.py\n" + "rename to new_name.py\n" + ) + result_annotated = _format_annotated(diff, ["new_name.py"]) + assert isinstance(result_annotated, str) + + result_compact = _format_compact(diff, ["new_name.py"]) + assert isinstance(result_compact, str) + + def test_no_newline_at_end_of_file_marker(self): + """'No newline at end of file' markers should be handled without crash.""" + diff = ( + "diff --git a/file.py b/file.py\n" + "index abc..def 100644\n" + "--- a/file.py\n" + "+++ b/file.py\n" + "@@ -1,3 +1,3 @@\n" + " first line\n" + "-old last line\n" + "\\ No newline at end of file\n" + "+new last line\n" + "\\ No newline at end of file\n" + ) + result_annotated = _format_annotated(diff, ["file.py"]) + assert isinstance(result_annotated, str) + assert "__new hunk__" in result_annotated + assert "new last line" in result_annotated + + result_compact = _format_compact(diff, ["file.py"]) + assert isinstance(result_compact, str) + assert "__new hunk__" in result_compact + # Compact should not contain removed lines + lines = result_compact.split("\n") + content_lines = [ + l + for l in lines + if not l.startswith("## File:") and not l.startswith("__") and l.strip() + ] + for line in content_lines: + assert not line.strip().startswith("-"), ( + f"Found removed line in compact output: {line!r}" + ) + + def test_hunk_only_additions_new_file(self): + """New file with only added lines should produce __new hunk__ only.""" + diff = ( + "diff --git a/brand_new.py b/brand_new.py\n" + "new file mode 100644\n" + "index 0000000..abcdef1\n" + "--- /dev/null\n" + "+++ b/brand_new.py\n" + "@@ -0,0 +1,5 @@\n" + "+#!/usr/bin/env python3\n" + "+import os\n" + "+\n" + "+def main():\n" + "+ print('hello')\n" + ) + result_annotated = _format_annotated(diff, ["brand_new.py"]) + assert "## File: 'brand_new.py'" in result_annotated + assert "__new hunk__" in result_annotated + assert "__old hunk__" not in result_annotated + + # All 5 lines should be present + lines = result_annotated.split("\n") + added_lines = [l for l in lines if "+" in l and l.strip()] + assert len(added_lines) >= 5 + + result_compact = _format_compact(diff, ["brand_new.py"]) + assert "__new hunk__" in result_compact + assert "__old hunk__" not in result_compact + + def test_hunk_only_deletions_deleted_file(self): + """Deleted file with only removed lines - annotated has __old hunk__, compact has nothing.""" + diff = ( + "diff --git a/old_file.py b/old_file.py\n" + "deleted file mode 100644\n" + "index abcdef1..0000000\n" + "--- a/old_file.py\n" + "+++ /dev/null\n" + "@@ -1,4 +0,0 @@\n" + "-import sys\n" + "-\n" + "-def old_func():\n" + "- pass\n" + ) + result_annotated = _format_annotated(diff, ["old_file.py"]) + assert "## File: 'old_file.py'" in result_annotated + assert "__old hunk__" in result_annotated + assert "__new hunk__" not in result_annotated + + result_compact = _format_compact(diff, ["old_file.py"]) + assert "## File: 'old_file.py'" in result_compact + assert "__new hunk__" not in result_compact + assert "__old hunk__" not in result_compact + + def test_multi_file_chunk_with_format(self): + """Multi-file chunk: each file gets its own ## File: header.""" + diff = ( + "diff --git a/src/models.py b/src/models.py\n" + "index abc..def 100644\n" + "--- a/src/models.py\n" + "+++ b/src/models.py\n" + "@@ -1,3 +1,4 @@\n" + " class User:\n" + "+ name: str\n" + " age: int\n" + " email: str\n" + "diff --git a/src/views.py b/src/views.py\n" + "index 111..222 100644\n" + "--- a/src/views.py\n" + "+++ b/src/views.py\n" + "@@ -5,3 +5,4 @@ def index\n" + " def index():\n" + "+ log('index')\n" + " return render()\n" + " pass\n" + "diff --git a/src/config.py b/src/config.py\n" + "index 333..444 100644\n" + "--- a/src/config.py\n" + "+++ b/src/config.py\n" + "@@ -10,3 +10,3 @@ class Config\n" + " DEBUG = True\n" + "-PORT = 8000\n" + "+PORT = 9000\n" + " HOST = 'localhost'\n" + ) + result = _format_annotated( + diff, ["src/models.py", "src/views.py", "src/config.py"] + ) + + assert "## File: 'src/models.py'" in result + assert "## File: 'src/views.py'" in result + assert "## File: 'src/config.py'" in result + + # Each file should have its own hunk markers + lines = result.split("\n") + file_headers = [l for l in lines if l.startswith("## File:")] + assert len(file_headers) == 3 + + def test_very_long_lines(self): + """Lines exceeding typical terminal width should not crash or truncate.""" + long_text = "x" * 500 + diff = ( + "diff --git a/wide.txt b/wide.txt\n" + "index abc..def 100644\n" + "--- a/wide.txt\n" + "+++ b/wide.txt\n" + "@@ -1,2 +1,2 @@\n" + f"-{long_text}\n" + f"+{long_text}_modified\n" + " trailing context\n" + ) + result_annotated = _format_annotated(diff, ["wide.txt"]) + assert f"{long_text}_modified" in result_annotated + + result_compact = _format_compact(diff, ["wide.txt"]) + assert f"{long_text}_modified" in result_compact + + def test_empty_diff_no_hunks(self): + """Diff with file header but no hunks should not crash.""" + diff = ( + "diff --git a/empty.py b/empty.py\n" + "index abc..def 100644\n" + "--- a/empty.py\n" + "+++ b/empty.py\n" + ) + result_annotated = _format_annotated(diff, ["empty.py"]) + assert isinstance(result_annotated, str) + + result_compact = _format_compact(diff, ["empty.py"]) + assert isinstance(result_compact, str) From f0c49823833f3251f5257759d057ee42c4608a75 Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 01:32:03 +0300 Subject: [PATCH 07/11] add token count estimates to list_chunks list_chunks now returns per-chunk token_count and total_token_count, enabling consuming skills to budget context windows. Uses a chars/4 heuristic with no new dependencies. --- src/models.py | 14 ++++++++++++++ src/server.py | 2 +- src/tools.py | 12 ++++++++++-- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/models.py b/src/models.py index 3859bb5..bf107a0 100644 --- a/src/models.py +++ b/src/models.py @@ -6,6 +6,16 @@ from typing import Any, Dict, List +def estimate_tokens(content: str) -> int: + """Estimate token count from content length. + + Uses character count / 4 as a rough heuristic. This intentionally + underestimates for code (which has shorter tokens on average), + making it conservative for context-budget planning. + """ + return len(content) // 4 + + class FormatMode(str, Enum): """Output format modes for chunk content.""" @@ -32,6 +42,7 @@ class ChunkInfo: files: List[str] line_count: int summary: str + token_count: int = 0 parent_file: str | None = None sub_chunk_index: int | None = None file_details: List[Dict[str, Any]] = field(default_factory=list) @@ -83,6 +94,8 @@ def get_chunk_info(self, chunk_number: int) -> ChunkInfo | None: else: summary = f"{len(chunk.files)} files, {chunk.line_count} lines" + token_count = estimate_tokens(chunk.content) + file_details = [ {"path": path, "lines": lines} for path, lines in chunk.file_line_counts.items() @@ -93,6 +106,7 @@ def get_chunk_info(self, chunk_number: int) -> ChunkInfo | None: files=chunk.files, line_count=chunk.line_count, summary=summary, + token_count=token_count, parent_file=chunk.parent_file, sub_chunk_index=chunk.sub_chunk_index, file_details=file_details, diff --git a/src/server.py b/src/server.py index 6647b39..fad118a 100644 --- a/src/server.py +++ b/src/server.py @@ -79,7 +79,7 @@ def list_chunks( str, Field(description="Absolute path to the diff file") ], ) -> str: - """Get an overview of all chunks in a diff file with file mappings and summaries. Auto-loads the diff file with optimal defaults if not already loaded. Use this as your first step to understand the scope and structure of changes before diving into specific chunks. CRITICAL: You must use an absolute directory path - relative paths will fail. DO NOT attempt to read the diff file directly as it will exceed context limits. This tool provides the roadmap for systematic chunk-by-chunk analysis. If using tracking documents to resume analysis, use this to orient yourself to remaining work.""" + """Get an overview of all chunks in a diff file with file mappings and summaries. Auto-loads the diff file with optimal defaults if not already loaded. Use this as your first step to understand the scope and structure of changes before diving into specific chunks. CRITICAL: You must use an absolute directory path - relative paths will fail. DO NOT attempt to read the diff file directly as it will exceed context limits. This tool provides the roadmap for systematic chunk-by-chunk analysis. If using tracking documents to resume analysis, use this to orient yourself to remaining work. Each chunk includes a `token_count` estimate, and the response includes `total_token_count` for context-budget planning.""" logger.info("list_chunks: %s", os.path.basename(absolute_file_path)) logger.debug("list_chunks full path: %s", absolute_file_path) result = tools.list_chunks(absolute_file_path=absolute_file_path) diff --git a/src/tools.py b/src/tools.py index 81ea688..de8300c 100644 --- a/src/tools.py +++ b/src/tools.py @@ -159,19 +159,20 @@ def load_diff( "files_excluded": session.stats.files_excluded, } - def list_chunks(self, absolute_file_path: str) -> List[Dict[str, Any]]: + def list_chunks(self, absolute_file_path: str) -> Dict[str, Any]: """List all chunks with their metadata.""" file_key = self._ensure_loaded(absolute_file_path) session = self.sessions[file_key] chunk_infos = session.list_chunk_infos() - return [ + chunks = [ { "chunk": info.chunk_number, "files": info.files, "file_details": info.file_details, "lines": info.line_count, + "token_count": info.token_count, "summary": info.summary, "parent_file": info.parent_file, "sub_chunk_index": info.sub_chunk_index, @@ -179,6 +180,13 @@ def list_chunks(self, absolute_file_path: str) -> List[Dict[str, Any]]: for info in chunk_infos ] + total_token_count = sum(info.token_count for info in chunk_infos) + + return { + "chunks": chunks, + "total_token_count": total_token_count, + } + def get_chunk( self, absolute_file_path: str, From 54d2c0b4671593ad63117dc41e745c174af9c64f Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 01:41:55 +0300 Subject: [PATCH 08/11] add tests and docs for token count awareness Update all test files to handle the new list_chunks dict response shape, add unit and integration tests for token estimation, and document the token_count field in README and design docs. --- README.md | 5 +- docs/design.md | 33 ++++++----- tests/test_case_insensitive.py | 4 +- tests/test_filesystem_edge_cases.py | 4 +- tests/test_get_file_diff.py | 12 ++-- tests/test_integration.py | 86 ++++++++++++++++++++++++++++- tests/test_mcp_components.py | 30 ++++++---- 7 files changed, 134 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 7c8b3d3..6b5051d 100644 --- a/README.md +++ b/README.md @@ -103,8 +103,9 @@ This lets your AI assistant handle massive diffs that would normally crash other ```python list_chunks("/tmp/changes.diff") -# → 5 chunks across 12 files, 3,847 total lines -# Each chunk includes file_details with per-file line counts +# -> 5 chunks across 12 files, 3,847 total lines, ~15,420 tokens +# Each chunk includes token_count and file_details with per-file line counts +# Response includes total_token_count for context-budget planning ``` **Target specific files:** diff --git a/docs/design.md b/docs/design.md index 3ce78cc..de9ba18 100644 --- a/docs/design.md +++ b/docs/design.md @@ -33,24 +33,28 @@ def load_diff( ### list_chunks (Auto-loading) ```python -def list_chunks(absolute_file_path: str) -> List[Dict[str, Any]] +def list_chunks(absolute_file_path: str) -> Dict[str, Any] ``` -**Returns:** Array of chunk metadata with files, line counts, summaries, and `file_details` (per-file line counts) +**Returns:** Dictionary with `chunks` (array of chunk metadata with files, line counts, token counts, summaries, and `file_details`) and `total_token_count` (sum of all chunk token counts) ```json -[ - { - "chunk": 1, - "files": ["src/main.py", "src/utils.py"], - "file_details": [ - {"path": "src/main.py", "lines": 120}, - {"path": "src/utils.py", "lines": 45} - ], - "lines": 165, - "summary": "2 files, 165 lines" - } -] +{ + "chunks": [ + { + "chunk": 1, + "files": ["src/main.py", "src/utils.py"], + "file_details": [ + {"path": "src/main.py", "lines": 120}, + {"path": "src/utils.py", "lines": 45} + ], + "lines": 165, + "token_count": 412, + "summary": "2 files, 165 lines" + } + ], + "total_token_count": 412 +} ``` ### get_chunk (Auto-loading) @@ -113,6 +117,7 @@ class ChunkInfo: files: List[str] line_count: int summary: str + token_count: int = 0 # Estimated token count (len(content) // 4) parent_file: str | None = None sub_chunk_index: int | None = None file_details: List[Dict[str, Any]] = field(default_factory=list) # [{"path": str, "lines": int}] diff --git a/tests/test_case_insensitive.py b/tests/test_case_insensitive.py index 3abec3a..5183efd 100644 --- a/tests/test_case_insensitive.py +++ b/tests/test_case_insensitive.py @@ -68,7 +68,7 @@ def test_get_file_diff_case_insensitive_exact(self, tools, react_diff_file): tools.load_diff(react_diff_file) # Get a file name from the diff - chunks = tools.list_chunks(react_diff_file) + chunks = tools.list_chunks(react_diff_file)["chunks"] file_name = chunks[0]["files"][0] # Exact case should work @@ -83,7 +83,7 @@ def test_get_file_diff_case_insensitive_glob(self, tools, react_diff_file): """get_file_diff glob matching is case-insensitive.""" tools.load_diff(react_diff_file) - chunks = tools.list_chunks(react_diff_file) + chunks = tools.list_chunks(react_diff_file)["chunks"] # Find a file that has a unique extension to use as glob pattern file_name = chunks[0]["files"][0] diff --git a/tests/test_filesystem_edge_cases.py b/tests/test_filesystem_edge_cases.py index e11a8a9..49e8410 100644 --- a/tests/test_filesystem_edge_cases.py +++ b/tests/test_filesystem_edge_cases.py @@ -176,13 +176,13 @@ def test_load_diff_overwrite_previous_session( """Test loading new diff overwrites previous session.""" # Load first diff result1 = tools.load_diff(react_diff_file, max_chunk_lines=2000) - chunks1 = tools.list_chunks(react_diff_file) + chunks1 = tools.list_chunks(react_diff_file)["chunks"] # Load second diff (if available) go_diff = test_data_dir / "go_version_upgrade_1.22_to_1.23.diff" if go_diff.exists(): result2 = tools.load_diff(str(go_diff), max_chunk_lines=3000) - chunks2 = tools.list_chunks(str(go_diff)) + chunks2 = tools.list_chunks(str(go_diff))["chunks"] # Should be different sessions assert result1["file_path"] != result2["file_path"] diff --git a/tests/test_get_file_diff.py b/tests/test_get_file_diff.py index fe34c6a..320406a 100644 --- a/tests/test_get_file_diff.py +++ b/tests/test_get_file_diff.py @@ -165,7 +165,7 @@ def multi_file_diff_path(self): def test_list_chunks_includes_file_details(self, tools, multi_file_diff_path): """list_chunks includes file_details with per-file line counts.""" tools.load_diff(multi_file_diff_path) - chunks = tools.list_chunks(multi_file_diff_path) + chunks = tools.list_chunks(multi_file_diff_path)["chunks"] for chunk in chunks: assert "file_details" in chunk @@ -182,7 +182,7 @@ def test_list_chunks_includes_file_details(self, tools, multi_file_diff_path): def test_list_chunks_still_has_files_list(self, tools, multi_file_diff_path): """list_chunks still includes files as flat string list (backward compat).""" tools.load_diff(multi_file_diff_path) - chunks = tools.list_chunks(multi_file_diff_path) + chunks = tools.list_chunks(multi_file_diff_path)["chunks"] for chunk in chunks: assert "files" in chunk @@ -193,7 +193,7 @@ def test_list_chunks_still_has_files_list(self, tools, multi_file_diff_path): def test_file_details_paths_match_files_list(self, tools, multi_file_diff_path): """file_details paths should correspond to the files in the chunk.""" tools.load_diff(multi_file_diff_path) - chunks = tools.list_chunks(multi_file_diff_path) + chunks = tools.list_chunks(multi_file_diff_path)["chunks"] for chunk in chunks: detail_paths = {d["path"] for d in chunk["file_details"]} @@ -204,7 +204,7 @@ def test_file_details_paths_match_files_list(self, tools, multi_file_diff_path): def test_file_details_line_counts_sum(self, tools, multi_file_diff_path): """Sum of per-file line counts should approximate the chunk total.""" tools.load_diff(multi_file_diff_path) - chunks = tools.list_chunks(multi_file_diff_path) + chunks = tools.list_chunks(multi_file_diff_path)["chunks"] for chunk in chunks: detail_total = sum(d["lines"] for d in chunk["file_details"]) @@ -230,7 +230,7 @@ def test_real_diff_file_details(self, tools, test_data_dir): pytest.skip("React test diff not found") tools.load_diff(str(diff_file), max_chunk_lines=2000) - chunks = tools.list_chunks(str(diff_file)) + chunks = tools.list_chunks(str(diff_file))["chunks"] assert len(chunks) > 0 for chunk in chunks: @@ -247,7 +247,7 @@ def test_large_file_split_has_file_details(self, tools, test_data_dir): # Use small chunk size to force splits tools.load_diff(str(diff_file), max_chunk_lines=500) - chunks = tools.list_chunks(str(diff_file)) + chunks = tools.list_chunks(str(diff_file))["chunks"] sub_chunks = [c for c in chunks if c["sub_chunk_index"] is not None] if sub_chunks: diff --git a/tests/test_integration.py b/tests/test_integration.py index 4244ca6..20014d2 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -5,9 +5,30 @@ import pytest from pathlib import Path +from src.models import estimate_tokens from src.tools import DiffChunkTools +class TestEstimateTokens: + """Unit tests for estimate_tokens().""" + + def test_estimate_tokens_empty(self): + assert estimate_tokens("") == 0 + + def test_estimate_tokens_short(self): + assert estimate_tokens("abc") == 0 # 3 // 4 = 0 + + def test_estimate_tokens_exact(self): + assert estimate_tokens("a" * 100) == 25 # 100 // 4 + + def test_estimate_tokens_single_char(self): + assert estimate_tokens("x") == 0 + + def test_estimate_tokens_long(self): + content = "x" * 10000 + assert estimate_tokens(content) == 2500 + + class TestIntegrationWithRealData: """Test diffchunk functionality with real diff files.""" @@ -78,8 +99,13 @@ def test_list_chunks_functionality(self, tools, test_data_dir): tools.load_diff(str(diff_file), max_chunk_lines=2000) # List chunks - chunks = tools.list_chunks(str(diff_file)) + result = tools.list_chunks(str(diff_file)) + assert "chunks" in result + assert "total_token_count" in result + assert isinstance(result["total_token_count"], int) + + chunks = result["chunks"] assert len(chunks) > 0 # Verify chunk structure @@ -88,15 +114,18 @@ def test_list_chunks_functionality(self, tools, test_data_dir): assert "files" in chunk assert "lines" in chunk assert "summary" in chunk + assert "token_count" in chunk assert isinstance(chunk["chunk"], int) assert isinstance(chunk["files"], list) assert isinstance(chunk["lines"], int) assert isinstance(chunk["summary"], str) + assert isinstance(chunk["token_count"], int) assert chunk["chunk"] > 0 assert len(chunk["files"]) > 0 assert chunk["lines"] > 0 + assert chunk["token_count"] > 0 def test_get_chunk_functionality(self, tools, test_data_dir): """Test get_chunk with real data.""" @@ -213,11 +242,11 @@ def test_chunk_size_consistency(self, tools, test_data_dir): # Load with small chunk size result_small = tools.load_diff(str(diff_file), max_chunk_lines=500) - chunks_small = tools.list_chunks(str(diff_file)) + chunks_small = tools.list_chunks(str(diff_file))["chunks"] # Load with large chunk size result_large = tools.load_diff(str(diff_file), max_chunk_lines=5000) - chunks_large = tools.list_chunks(str(diff_file)) + chunks_large = tools.list_chunks(str(diff_file))["chunks"] # Smaller chunks should create more chunks (usually) # Note: This might not always be true due to file boundaries @@ -231,3 +260,54 @@ def test_chunk_size_consistency(self, tools, test_data_dir): for chunk in chunks_small + chunks_large: assert chunk["lines"] > 0 assert len(chunk["files"]) > 0 + + +class TestTokenCountIntegration: + """Integration tests for token count in list_chunks and get_chunk_info.""" + + @pytest.fixture + def test_data_dir(self): + return Path(__file__).parent / "test_data" + + @pytest.fixture + def tools(self): + return DiffChunkTools() + + @pytest.fixture + def react_diff_file(self, test_data_dir): + diff_file = test_data_dir / "react_18.0_to_18.3.diff" + if not diff_file.exists(): + pytest.skip("React test diff not found") + return str(diff_file) + + def test_each_chunk_has_positive_token_count(self, tools, react_diff_file): + """Each chunk in list_chunks output has token_count > 0.""" + tools.load_diff(react_diff_file, max_chunk_lines=2000) + result = tools.list_chunks(react_diff_file) + + chunks = result["chunks"] + assert len(chunks) > 0 + for chunk in chunks: + assert "token_count" in chunk + assert isinstance(chunk["token_count"], int) + assert chunk["token_count"] > 0 + + def test_total_token_count_equals_sum(self, tools, react_diff_file): + """total_token_count equals sum of individual chunk token_counts.""" + tools.load_diff(react_diff_file, max_chunk_lines=2000) + result = tools.list_chunks(react_diff_file) + + chunks = result["chunks"] + expected_total = sum(c["token_count"] for c in chunks) + assert result["total_token_count"] == expected_total + + def test_chunk_info_has_token_count(self, tools, react_diff_file): + """DiffSession.get_chunk_info() returns ChunkInfo with non-zero token_count.""" + tools.load_diff(react_diff_file, max_chunk_lines=2000) + + # Access the session directly to test get_chunk_info + file_key = tools._get_file_key(react_diff_file) + session = tools.sessions[file_key] + info = session.get_chunk_info(1) + assert info is not None + assert info.token_count > 0 diff --git a/tests/test_mcp_components.py b/tests/test_mcp_components.py index ab45e2f..3d21d59 100644 --- a/tests/test_mcp_components.py +++ b/tests/test_mcp_components.py @@ -44,7 +44,11 @@ def test_diffchunk_tools_complete_workflow(self, react_diff_file): total_chunks = result["chunks"] # 2. List chunks (auto-loads if needed) - chunks = tools.list_chunks(react_diff_file) + result = tools.list_chunks(react_diff_file) + assert "chunks" in result + assert "total_token_count" in result + assert isinstance(result["total_token_count"], int) + chunks = result["chunks"] assert len(chunks) == total_chunks assert all("chunk" in chunk for chunk in chunks) assert all("files" in chunk for chunk in chunks) @@ -94,7 +98,8 @@ def test_diffchunk_tools_auto_loading(self, react_diff_file): tools = DiffChunkTools() # Test that tools auto-load when called without explicit load_diff - chunks = tools.list_chunks(react_diff_file) + result = tools.list_chunks(react_diff_file) + chunks = result["chunks"] assert len(chunks) > 0 # Should work for other tools too @@ -165,8 +170,11 @@ def test_diffchunk_tools_multi_file_support(self, react_diff_file, go_diff_file) assert go_result["file_path"] == go_diff_file # Should be able to work with both files independently - react_chunks = tools.list_chunks(react_diff_file) - go_chunks = tools.list_chunks(go_diff_file) + react_result_chunks = tools.list_chunks(react_diff_file) + go_result_chunks = tools.list_chunks(go_diff_file) + + react_chunks = react_result_chunks["chunks"] + go_chunks = go_result_chunks["chunks"] assert len(react_chunks) == react_result["chunks"] assert len(go_chunks) == go_result["chunks"] @@ -252,7 +260,7 @@ def test_chunk_content_structure(self, react_diff_file): tools = DiffChunkTools() tools.load_diff(react_diff_file, max_chunk_lines=3000) - chunks = tools.list_chunks(react_diff_file) + chunks = tools.list_chunks(react_diff_file)["chunks"] for i, chunk_info in enumerate(chunks, 1): # Test chunk info structure @@ -304,11 +312,11 @@ def test_large_diff_performance(self, go_diff_file): # Measure navigation time start_time = time.time() - chunks = tools.list_chunks(go_diff_file) + list_result = tools.list_chunks(go_diff_file) list_time = time.time() - start_time assert list_time < 2.0, f"List chunks took too long: {list_time}s" - assert len(chunks) == result["chunks"] + assert len(list_result["chunks"]) == result["chunks"] # Measure chunk retrieval time start_time = time.time() @@ -873,8 +881,8 @@ def test_excluded_files_not_in_list_chunks(self, react_diff_file): assert result["files_excluded"] > 0 # Verify no .json files appear in chunk listings - chunks = tools.list_chunks(react_diff_file) - for chunk_info in chunks: + list_result = tools.list_chunks(react_diff_file) + for chunk_info in list_result["chunks"]: for file_path in chunk_info["files"]: assert not file_path.endswith(".json"), ( f"Excluded file '{file_path}' found in list_chunks output" @@ -1245,8 +1253,8 @@ def test_exclude_patterns_then_format(self, react_diff_file): assert "## File:" in annotated # No .json files should appear in chunk listings - chunks = tools.list_chunks(react_diff_file) - for chunk_info in chunks: + list_result = tools.list_chunks(react_diff_file) + for chunk_info in list_result["chunks"]: for file_path in chunk_info["files"]: assert not file_path.endswith(".json") From e2d61749586d5a8a4a45e0bf95b70d8b46465578 Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 12:11:32 +0300 Subject: [PATCH 09/11] fix lint failures in test files Rename ambiguous variable 'l' to 'ln' in test_mcp_components.py (E741). Align pre-push hook with CI by linting the full repo, not just src/. --- scripts/pre-push | 4 +- tests/test_mcp_components.py | 92 ++++++++++++++++++------------------ 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/scripts/pre-push b/scripts/pre-push index b981200..b9bc852 100755 --- a/scripts/pre-push +++ b/scripts/pre-push @@ -6,10 +6,10 @@ set -e echo "Running pre-push checks..." echo "Checking ruff lint..." -uv run ruff check src/ +uv run ruff check . echo "Checking code formatting..." -uv run ruff format --check src/ +uv run ruff format --check . echo "Running tests..." uv run pytest tests/ -x -q diff --git a/tests/test_mcp_components.py b/tests/test_mcp_components.py index 3d21d59..49575c1 100644 --- a/tests/test_mcp_components.py +++ b/tests/test_mcp_components.py @@ -425,12 +425,12 @@ def test_multi_hunk_multi_file_annotated(self): lines = result.split("\n") # Find a __new hunk__ added line - should have line number and + prefix - new_hunk_added = [l for l in lines if "+" in l and "new line" in l] + new_hunk_added = [ln for ln in lines if "+" in ln and "new line" in ln] assert len(new_hunk_added) >= 1 assert new_hunk_added[0].strip().startswith("12") # line 10+2 context = 12 # Find __old hunk__ removed line - should have - prefix, no line number - old_hunk_removed = [l for l in lines if "-" in l and "old removed" in l] + old_hunk_removed = [ln for ln in lines if "-" in ln and "old removed" in ln] assert len(old_hunk_removed) >= 1 # Old hunk lines have no numeric line numbers stripped = old_hunk_removed[0].strip() @@ -457,7 +457,7 @@ def test_new_file_no_old_hunks(self): # All lines should be added with line numbers lines = result.split("\n") - added_lines = [l for l in lines if "+" in l and "line" in l] + added_lines = [ln for ln in lines if "+" in ln and "line" in ln] assert len(added_lines) == 3 def test_deleted_file_no_new_hunks(self): @@ -481,7 +481,7 @@ def test_deleted_file_no_new_hunks(self): # All lines should be removed with - prefix lines = result.split("\n") - removed_lines = [l for l in lines if "-" in l and "line" in l] + removed_lines = [ln for ln in lines if "-" in ln and "line" in ln] assert len(removed_lines) == 3 def test_hunk_no_function_context(self): @@ -501,7 +501,7 @@ def test_hunk_no_function_context(self): # Should have __new hunk__ without | part lines = result.split("\n") - hunk_headers = [l for l in lines if l.startswith("__new hunk__")] + hunk_headers = [ln for ln in lines if ln.startswith("__new hunk__")] assert len(hunk_headers) == 1 assert "|" not in hunk_headers[0] @@ -530,7 +530,7 @@ def test_multiple_hunks_single_file(self): # Should have two __new hunk__ sections lines = result.split("\n") - new_hunk_headers = [l for l in lines if l.startswith("__new hunk__")] + new_hunk_headers = [ln for ln in lines if ln.startswith("__new hunk__")] assert len(new_hunk_headers) == 2 # Both should have function context @@ -539,12 +539,12 @@ def test_multiple_hunks_single_file(self): # Verify line numbers are correct for each hunk # First hunk: starts at line 5, add1 should be at line 6 - add1_lines = [l for l in lines if "+add1" in l] - assert any("6" in l for l in add1_lines) + add1_lines = [ln for ln in lines if "+add1" in ln] + assert any("6" in ln for ln in add1_lines) # Second hunk: starts at line 21, add2 should be at line 22 - add2_lines = [l for l in lines if "+add2" in l] - assert any("22" in l for l in add2_lines) + add2_lines = [ln for ln in lines if "+add2" in ln] + assert any("22" in ln for ln in add2_lines) def test_annotated_via_tools_get_chunk(self): """Annotated format works end-to-end via DiffChunkTools.get_chunk.""" @@ -591,15 +591,15 @@ def test_annotated_line_numbers_accuracy(self): # Line 52: " last line" new_hunk_lines = [] in_new = False - for l in lines: - if l.startswith("__new hunk__"): + for ln in lines: + if ln.startswith("__new hunk__"): in_new = True continue - if l.startswith("__old hunk__") or l.startswith("## File:"): + if ln.startswith("__old hunk__") or ln.startswith("## File:"): in_new = False continue - if in_new and l.strip(): - new_hunk_lines.append(l) + if in_new and ln.strip(): + new_hunk_lines.append(ln) assert len(new_hunk_lines) == 6 assert new_hunk_lines[0].strip().startswith("47") @@ -632,9 +632,9 @@ def test_compact_omits_removed_lines(self): # Split into lines and check content lines (skip headers/markers) lines = result.split("\n") content_lines = [ - l for l in lines - if not l.startswith("## File:") and not l.startswith("__") - and l.strip() + ln for ln in lines + if not ln.startswith("## File:") and not ln.startswith("__") + and ln.strip() ] # No content line should have a - prefix (removed line format is " -text") for line in content_lines: @@ -688,15 +688,15 @@ def test_compact_line_numbers_and_plus_prefix(self): # Collect content lines (not headers/markers) content_lines = [] in_hunk = False - for l in lines: - if l.startswith("__new hunk__"): + for ln in lines: + if ln.startswith("__new hunk__"): in_hunk = True continue - if l.startswith("## File:"): + if ln.startswith("## File:"): in_hunk = False continue - if in_hunk and l.strip(): - content_lines.append(l) + if in_hunk and ln.strip(): + content_lines.append(ln) # Should have 6 lines: 2 context, 2 added, 1 context (skip removed), 1 context assert len(content_lines) == 6 @@ -753,9 +753,9 @@ def test_compact_multi_file_multi_hunk(self): # Verify no removed lines in output content lines = result.split("\n") content_lines = [ - l for l in lines - if not l.startswith("## File:") and not l.startswith("__") - and l.strip() + ln for ln in lines + if not ln.startswith("## File:") and not ln.startswith("__") + and ln.strip() ] for line in content_lines: stripped = line.strip() @@ -764,7 +764,7 @@ def test_compact_multi_file_multi_hunk(self): ) # Should have three __new hunk__ markers total (2 for auth.py, 1 for utils.py) - new_hunk_count = sum(1 for l in lines if l.startswith("__new hunk__")) + new_hunk_count = sum(1 for ln in lines if ln.startswith("__new hunk__")) assert new_hunk_count == 3 def test_compact_deleted_file_no_output(self): @@ -808,7 +808,7 @@ def test_compact_new_file_all_added(self): # All lines should have + prefix and line numbers lines = result.split("\n") - added_lines = [l for l in lines if "+" in l and "line" in l] + added_lines = [ln for ln in lines if "+" in ln and "line" in ln] assert len(added_lines) == 3 def test_compact_via_tools_get_chunk(self): @@ -943,12 +943,12 @@ def test_reduce_context_basic(self): content_lines.append(line) # Count context lines - ctx_lines = [l for l in content_lines if l.startswith(" ")] + ctx_lines = [ln for ln in content_lines if ln.startswith(" ")] assert len(ctx_lines) == 2 # 1 before + 1 after # Change lines should still be present - assert any("+added_line" in l for l in content_lines) - assert any("-removed_line" in l for l in content_lines) + assert any("+added_line" in ln for ln in content_lines) + assert any("-removed_line" in ln for ln in content_lines) def test_reduce_context_recalculated_headers(self): """Hunk headers are recalculated after context reduction.""" @@ -963,7 +963,7 @@ def test_reduce_context_recalculated_headers(self): hunk_re = re.compile(r"^@@ -(\d+),(\d+) \+(\d+),(\d+) @@") result_lines = result.split("\n") - hunk_headers = [l for l in result_lines if hunk_re.match(l)] + hunk_headers = [ln for ln in result_lines if hunk_re.match(ln)] assert len(hunk_headers) >= 1 m = hunk_re.match(hunk_headers[0]) @@ -994,7 +994,7 @@ def test_reduce_context_none_keeps_all(self): # All context lines should be preserved result_lines = result.split("\n") - ctx_lines = [l for l in result_lines if l.startswith(" context_")] + ctx_lines = [ln for ln in result_lines if ln.startswith(" context_")] assert len(ctx_lines) == 10 # 5 before + 5 after def test_reduce_context_zero_keeps_only_changes(self): @@ -1016,12 +1016,12 @@ def test_reduce_context_zero_keeps_only_changes(self): content_lines.append(line) # No context lines - ctx_lines = [l for l in content_lines if l.startswith(" ")] + ctx_lines = [ln for ln in content_lines if ln.startswith(" ")] assert len(ctx_lines) == 0 # Changes are still present - assert any("+added_line" in l for l in content_lines) - assert any("-removed_line" in l for l in content_lines) + assert any("+added_line" in ln for ln in content_lines) + assert any("-removed_line" in ln for ln in content_lines) def test_reduce_context_overlapping_windows(self): """Two nearby changes with overlapping context windows preserve shared context.""" @@ -1065,18 +1065,18 @@ def test_reduce_context_overlapping_windows(self): # - first_add needs 2 before (far_before_3, near_before) and 2 after (between_1, between_2) # - second_remove/second_add needs 2 before (between_1, between_2) and 2 after (near_after, far_after_1) # The between_1 and between_2 lines are shared context - ctx_lines = [l for l in content_lines if l.startswith(" ")] + ctx_lines = [ln for ln in content_lines if ln.startswith(" ")] # Should keep: far_before_3, near_before, between_1, between_2, near_after, far_after_1 assert len(ctx_lines) == 6 # far_before_1 and far_before_2 should be dropped - assert not any("far_before_1" in l for l in content_lines) - assert not any("far_before_2" in l for l in content_lines) + assert not any("far_before_1" in ln for ln in content_lines) + assert not any("far_before_2" in ln for ln in content_lines) # far_after_2 and far_after_3 should be dropped - assert not any("far_after_2" in l for l in content_lines) - assert not any("far_after_3" in l for l in content_lines) + assert not any("far_after_2" in ln for ln in content_lines) + assert not any("far_after_3" in ln for ln in content_lines) def test_context_lines_negative_raises_valueerror(self): """Negative context_lines raises ValueError via tools validation.""" @@ -1334,9 +1334,9 @@ def test_no_newline_at_end_of_file_marker(self): # Compact should not contain removed lines lines = result_compact.split("\n") content_lines = [ - l - for l in lines - if not l.startswith("## File:") and not l.startswith("__") and l.strip() + ln + for ln in lines + if not ln.startswith("## File:") and not ln.startswith("__") and ln.strip() ] for line in content_lines: assert not line.strip().startswith("-"), ( @@ -1365,7 +1365,7 @@ def test_hunk_only_additions_new_file(self): # All 5 lines should be present lines = result_annotated.split("\n") - added_lines = [l for l in lines if "+" in l and l.strip()] + added_lines = [ln for ln in lines if "+" in ln and ln.strip()] assert len(added_lines) >= 5 result_compact = _format_compact(diff, ["brand_new.py"]) @@ -1437,7 +1437,7 @@ def test_multi_file_chunk_with_format(self): # Each file should have its own hunk markers lines = result.split("\n") - file_headers = [l for l in lines if l.startswith("## File:")] + file_headers = [ln for ln in lines if ln.startswith("## File:")] assert len(file_headers) == 3 def test_very_long_lines(self): From 1c2570ba9bc4110dcd68b71a506e566b68121f74 Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 12:15:36 +0300 Subject: [PATCH 10/11] fix formatting in test file --- tests/test_mcp_components.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/test_mcp_components.py b/tests/test_mcp_components.py index 49575c1..693944a 100644 --- a/tests/test_mcp_components.py +++ b/tests/test_mcp_components.py @@ -556,7 +556,9 @@ def test_annotated_via_tools_get_chunk(self): tools = DiffChunkTools() tools.load_diff(str(react_diff), max_chunk_lines=3000) - annotated = tools.get_chunk(str(react_diff), 1, format="annotated", include_context=False) + annotated = tools.get_chunk( + str(react_diff), 1, format="annotated", include_context=False + ) # Should contain structural elements assert "## File:" in annotated @@ -632,9 +634,9 @@ def test_compact_omits_removed_lines(self): # Split into lines and check content lines (skip headers/markers) lines = result.split("\n") content_lines = [ - ln for ln in lines - if not ln.startswith("## File:") and not ln.startswith("__") - and ln.strip() + ln + for ln in lines + if not ln.startswith("## File:") and not ln.startswith("__") and ln.strip() ] # No content line should have a - prefix (removed line format is " -text") for line in content_lines: @@ -753,9 +755,9 @@ def test_compact_multi_file_multi_hunk(self): # Verify no removed lines in output content lines = result.split("\n") content_lines = [ - ln for ln in lines - if not ln.startswith("## File:") and not ln.startswith("__") - and ln.strip() + ln + for ln in lines + if not ln.startswith("## File:") and not ln.startswith("__") and ln.strip() ] for line in content_lines: stripped = line.strip() @@ -1180,9 +1182,7 @@ def test_raw_matches_default(self, react_diff_file): tools.load_diff(react_diff_file, max_chunk_lines=3000) default = tools.get_chunk(react_diff_file, 1, include_context=False) - raw = tools.get_chunk( - react_diff_file, 1, include_context=False, format="raw" - ) + raw = tools.get_chunk(react_diff_file, 1, include_context=False, format="raw") assert default == raw def test_annotated_has_structural_elements(self, react_diff_file): From cc26a6fbbdf7a31420adf72bb7a7ec536d410146 Mon Sep 17 00:00:00 2001 From: Peter Etelej Date: Thu, 9 Apr 2026 12:52:24 +0300 Subject: [PATCH 11/11] fix coderabbit review findings Preserve no-hunk file sections (binary, rename) in formatted output, remove dead code in parser, and harden test assertions. --- src/formatter.py | 2 +- src/parser.py | 5 +---- tests/test_mcp_components.py | 38 +++++++++++++++++++++--------------- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/formatter.py b/src/formatter.py index 037618a..7d6f04d 100644 --- a/src/formatter.py +++ b/src/formatter.py @@ -79,7 +79,7 @@ def _flush_hunk(): def _flush_file(): nonlocal current_path, current_hunks, in_hunk _flush_hunk() - if current_path is not None and current_hunks: + if current_path is not None: sections.append(FileSection(path=current_path, hunks=current_hunks)) current_hunks = [] in_hunk = False diff --git a/src/parser.py b/src/parser.py index 9333f0b..293aa64 100644 --- a/src/parser.py +++ b/src/parser.py @@ -338,10 +338,7 @@ def reduce_context(content: str, context_lines: int) -> str: hunk_lines = [new_header] for lt, text, _, _ in sub: - if lt != "meta": - hunk_lines.append(text) - else: - hunk_lines.append(text) + hunk_lines.append(text) output_hunks.append("\n".join(hunk_lines)) if not output_hunks: diff --git a/tests/test_mcp_components.py b/tests/test_mcp_components.py index 693944a..12e8712 100644 --- a/tests/test_mcp_components.py +++ b/tests/test_mcp_components.py @@ -310,20 +310,12 @@ def test_large_diff_performance(self, go_diff_file): assert result["files"] > 50 assert result["total_lines"] > 1000 - # Measure navigation time - start_time = time.time() + # Navigation should work list_result = tools.list_chunks(go_diff_file) - list_time = time.time() - start_time - - assert list_time < 2.0, f"List chunks took too long: {list_time}s" assert len(list_result["chunks"]) == result["chunks"] - # Measure chunk retrieval time - start_time = time.time() + # Chunk retrieval should work content = tools.get_chunk(go_diff_file, 1) - get_time = time.time() - start_time - - assert get_time < 1.0, f"Get chunk took too long: {get_time}s" assert len(content) > 0 def test_format_raw_returns_identical_output(self, react_diff_file): @@ -1080,11 +1072,20 @@ def test_reduce_context_overlapping_windows(self): assert not any("far_after_2" in ln for ln in content_lines) assert not any("far_after_3" in ln for ln in content_lines) - def test_context_lines_negative_raises_valueerror(self): + def test_context_lines_negative_raises_valueerror(self, tmp_path): """Negative context_lines raises ValueError via tools validation.""" + diff_file = tmp_path / "test.diff" + diff_file.write_text( + "diff --git a/f.py b/f.py\n" + "--- a/f.py\n" + "+++ b/f.py\n" + "@@ -1,1 +1,1 @@\n" + "-old\n" + "+new\n" + ) tools = DiffChunkTools() with pytest.raises(ValueError, match="non-negative integer"): - tools.load_diff("/some/file.diff", context_lines=-1) + tools.load_diff(str(diff_file), context_lines=-1) def test_context_lines_via_load_diff(self): """context_lines works end-to-end through load_diff with real diff data.""" @@ -1282,21 +1283,22 @@ class TestFormatterEdgeCases: """Edge case tests for the formatter with crafted diff content.""" def test_binary_file_indicator(self): - """Binary file indicator should be handled gracefully.""" + """Binary file indicator should be handled gracefully and preserve file header.""" diff = ( "diff --git a/image.png b/image.png\n" "index abc1234..def5678 100644\n" "Binary files a/image.png and b/image.png differ\n" ) - # Should not crash - binary files have no hunks so formatter returns as-is result_annotated = _format_annotated(diff, ["image.png"]) assert isinstance(result_annotated, str) + assert "## File: 'image.png'" in result_annotated result_compact = _format_compact(diff, ["image.png"]) assert isinstance(result_compact, str) + assert "## File: 'image.png'" in result_compact def test_rename_only_patch(self): - """Rename-only patch (no hunks) should be handled gracefully.""" + """Rename-only patch (no hunks) should preserve file header in output.""" diff = ( "diff --git a/old_name.py b/new_name.py\n" "similarity index 100%\n" @@ -1305,9 +1307,11 @@ def test_rename_only_patch(self): ) result_annotated = _format_annotated(diff, ["new_name.py"]) assert isinstance(result_annotated, str) + assert "## File: 'new_name.py'" in result_annotated result_compact = _format_compact(diff, ["new_name.py"]) assert isinstance(result_compact, str) + assert "## File: 'new_name.py'" in result_compact def test_no_newline_at_end_of_file_marker(self): """'No newline at end of file' markers should be handled without crash.""" @@ -1460,7 +1464,7 @@ def test_very_long_lines(self): assert f"{long_text}_modified" in result_compact def test_empty_diff_no_hunks(self): - """Diff with file header but no hunks should not crash.""" + """Diff with file header but no hunks should preserve file header.""" diff = ( "diff --git a/empty.py b/empty.py\n" "index abc..def 100644\n" @@ -1469,6 +1473,8 @@ def test_empty_diff_no_hunks(self): ) result_annotated = _format_annotated(diff, ["empty.py"]) assert isinstance(result_annotated, str) + assert "## File: 'empty.py'" in result_annotated result_compact = _format_compact(diff, ["empty.py"]) assert isinstance(result_compact, str) + assert "## File: 'empty.py'" in result_compact