From 88afb3c347c474367bc1f0f2d14df51e5b522a05 Mon Sep 17 00:00:00 2001 From: Steve Pelech Date: Thu, 12 Nov 2026 15:39:33 -0600 Subject: [PATCH] fix(grep-code): give each context line its own line number Co-Authored-By: Claude Opus 5.5 --- REQUIREMENTS.md | 11 +- app/mcp/handlers/grep_handlers.py | 27 ++++- app/services/code_grep_chunks.py | 149 ++++++++++++++++++++++++ app/services/code_grep_scanner.py | 52 +++------ app/services/indexing/processor.py | 23 ++++ tests/test_grep_code_context_lines.py | 159 ++++++++++++++++++++++++++ 6 files changed, 378 insertions(+), 43 deletions(-) create mode 100644 app/services/code_grep_chunks.py create mode 100644 tests/test_grep_code_context_lines.py diff --git a/REQUIREMENTS.md b/REQUIREMENTS.md index ce17591..e2dcfe3 100644 --- a/REQUIREMENTS.md +++ b/REQUIREMENTS.md @@ -2,7 +2,7 @@ > **Note:** This document is automatically generated and verified against the live test suite by `scripts/generate_requirements.py` and `tests/backend/test_requirements_sync.py`. -**Test Verification Baseline:** **1199 Automated Tests** (835 Pytest Backend + 314 Vitest Frontend + 50 Playwright E2E). +**Test Verification Baseline:** **1206 Automated Tests** (842 Pytest Backend + 314 Vitest Frontend + 50 Playwright E2E). --- @@ -1121,6 +1121,15 @@ code results are enriched with signature/kind/ast_symbol_id from SQLite._ - `test_mcp_grep_code_registration_and_execution` - `test_api_search_grep_endpoint` +#### `tests/test_grep_code_context_lines.py` (7 tests) +- `test_indexed_path_regex_context_lines_are_consecutive` +- `test_chunk_path_context_lines_have_file_line_numbers` +- `test_chunk_path_without_file_chunks_gives_consecutive_numbers` +- `test_chunk_context_continues_past_the_chunk_edge` +- `test_sanitizer_renumbers_context_lines_that_share_one_number` +- `test_sanitizer_keeps_correct_context_lines` +- `test_frontmatter_line_offset` + #### `tests/test_grep_code_filters.py` (6 tests) - `test_path_filter` - `test_looks_like_regex` diff --git a/app/mcp/handlers/grep_handlers.py b/app/mcp/handlers/grep_handlers.py index 57ebb38..61f141d 100644 --- a/app/mcp/handlers/grep_handlers.py +++ b/app/mcp/handlers/grep_handlers.py @@ -44,13 +44,34 @@ def truncate_grep_line(text: Optional[str], max_chars: int = _MAX_GREP_LINE_CHAR return text[:keep] + _LINE_TRUNCATION_SUFFIX +def _context_line_numbers(match: Dict[str, Any], raw_ctx: List[Dict[str, Any]]) -> List[Any]: + """ + Line numbers for the context block. Numbers that do not go up line by line (for example the same number on every + line) are replaced: the context line with the match text gets the match line, and the other lines count from it. + """ + numbers = [item.get("line") for item in raw_ctx] + if all(isinstance(n, int) for n in numbers) and all(b > a for a, b in zip(numbers, numbers[1:])): + return numbers + match_line = match.get("line") + if not isinstance(match_line, int): + return numbers + texts = [item.get("text") for item in raw_ctx] + anchor = next((i for i, (n, t) in enumerate(zip(numbers, texts)) if n == match_line and t == match.get("match_text")), None) + if anchor is None: + anchor = next((i for i, t in enumerate(texts) if t == match.get("match_text")), None) + if anchor is None: + return numbers + return [match_line - anchor + i for i in range(len(raw_ctx))] + + def _sanitize_match_lines(match: Dict[str, Any]) -> Dict[str, Any]: sanitized = dict(match) sanitized["match_text"] = truncate_grep_line(sanitized.get("match_text")) - raw_ctx = sanitized.get("context") or [] + raw_ctx = [item for item in (match.get("context") or []) if isinstance(item, dict)] + numbers = _context_line_numbers(match, raw_ctx) sanitized["context"] = [ - {"line": item.get("line"), "text": truncate_grep_line(item.get("text"))} - for item in raw_ctx + {"line": line_no, "text": truncate_grep_line(item.get("text"))} + for line_no, item in zip(numbers, raw_ctx) ] return sanitized diff --git a/app/services/code_grep_chunks.py b/app/services/code_grep_chunks.py new file mode 100644 index 0000000..b280a45 --- /dev/null +++ b/app/services/code_grep_chunks.py @@ -0,0 +1,149 @@ +""" +grep_code matches from vector-store chunks. + +The chunks of one file are put back together as one line map before the scan. Each context line then gets its own +file line number, and the context can continue past the edge of the chunk that holds the match. +""" +import logging +from typing import Any, Callable, Dict, Iterable, List, Optional, Pattern, Set, Tuple + +logger = logging.getLogger("contextcortex.file_reader") + +FileKey = Tuple[str, str] + + +def chunk_file_key(chunk: Dict[str, Any]) -> FileKey: + """(repo, path in the repo) for a chunk payload, with a 'repo://' or 'repo/' prefix removed.""" + ch_repo = chunk.get("repo") or "unknown" + raw_file = (chunk.get("rel_path") or chunk.get("path") or "").replace("\\", "/") + ch_file = raw_file.split("://", 1)[1] if "://" in raw_file else raw_file + if ch_file.startswith(f"{ch_repo}/"): + ch_file = ch_file[len(ch_repo) + 1:] + return ch_repo, ch_file + + +def is_summary_chunk(chunk: Dict[str, Any]) -> bool: + """File summaries are LLM text with placeholder line numbers, so grep must not report lines from them.""" + return chunk.get("doc_type") == "summary" or bool(chunk.get("is_summary")) + + +def _positive_int(value: Any) -> Optional[int]: + try: + number = int(value) + except (TypeError, ValueError): + return None + return number if number >= 1 else None + + +def _same_line(known: str, text: str) -> bool: + known_s, text_s = known.strip(), text.strip() + if not text_s: + return not known_s + # A nested symbol chunk can start or stop in the middle of a line of its parent chunk. + return known_s == text_s or known_s.endswith(text_s) or known_s.startswith(text_s) + + +def _fit_shift(line_map: Dict[int, str], blank_lines: Set[int], start: int, lines: List[str], slack: int) -> int: + """ + Chunk text is stripped when it is indexed, so blank lines at the start or end of a chunk can be missing. `slack` + is the number of missing lines. Use the smallest shift at which the chunk agrees with the known lines. + """ + for shift in range(slack + 1): + agrees = True + for idx, text in enumerate(lines): + line_no = start + shift + idx + known = line_map.get(line_no, "" if line_no in blank_lines else None) + if known is not None and not _same_line(known, text): + agrees = False + break + if agrees: + return shift + return 0 + + +def build_line_map(chunks: Iterable[Dict[str, Any]]) -> Dict[int, str]: + """Maps file line numbers to text for the chunks of one file. Large chunks go first, so full lines win.""" + ordered = sorted( + chunks, + key=lambda c: (_positive_int(c.get("start_line")) or 0, -(_positive_int(c.get("end_line")) or 0)), + ) + line_map: Dict[int, str] = {} + blank_lines: Set[int] = set() + next_line = 1 + for chunk in ordered: + lines = (chunk.get("content") or chunk.get("text") or "").splitlines() + if not lines: + continue + start = _positive_int(chunk.get("start_line")) or next_line + end = _positive_int(chunk.get("end_line")) + slack = max(0, end - start + 1 - len(lines)) if end and end >= start else 0 + shift = _fit_shift(line_map, blank_lines, start, lines, slack) + # The lines that the strip removed were blank. + blank_lines.update(range(start, start + shift)) + blank_lines.update(range(start + shift + len(lines), start + slack + len(lines))) + start += shift + for idx, text in enumerate(lines): + line_map.setdefault(start + idx, text) + next_line = max(next_line, start + len(lines)) + for line_no in blank_lines: + line_map.setdefault(line_no, "") + return line_map + + +def grep_chunk_matches( + chunks: Iterable[Dict[str, Any]], + compiled: Pattern[str], + context_lines: int, + limit: int, + skip_files: Set[FileKey], + seen_keys: Set[Tuple[str, str, int]], + load_file_chunks: Optional[Callable[[str, str], List[Dict[str, Any]]]] = None, +) -> List[Dict[str, Any]]: + """ + Returns up to `limit` matches from the files of the matched chunks. Files in `skip_files` were scanned on disk. + `load_file_chunks(repo, file)` gives all chunks of a file; without it only the matched chunks are used. + """ + grouped: Dict[FileKey, List[Dict[str, Any]]] = {} + for chunk in chunks: + if is_summary_chunk(chunk): + continue + key = chunk_file_key(chunk) + if key in skip_files: + continue + grouped.setdefault(key, []).append(chunk) + + matches: List[Dict[str, Any]] = [] + for (ch_repo, ch_file), matched_chunks in grouped.items(): + file_chunks = matched_chunks + if load_file_chunks is not None: + try: + loaded = [c for c in (load_file_chunks(ch_repo, ch_file) or []) if not is_summary_chunk(c)] + if loaded: + file_chunks = loaded + except Exception as e: + logger.debug(f"get_file_chunks failed for {ch_repo}:{ch_file}: {e}") + + line_map = build_line_map(file_chunks) + for line_no in sorted(line_map): + line_str = line_map[line_no] + if not compiled.search(line_str): + continue + key = (ch_repo, ch_file, line_no) + if key in seen_keys: + continue + seen_keys.add(key) + context = [ + {"line": c_line, "text": line_map[c_line]} + for c_line in range(line_no - context_lines, line_no + context_lines + 1) + if c_line in line_map + ] + matches.append({ + "repo": ch_repo, + "file": ch_file, + "line": line_no, + "match_text": line_str, + "context": context, + }) + if len(matches) >= limit: + return matches + return matches diff --git a/app/services/code_grep_scanner.py b/app/services/code_grep_scanner.py index 660e0f1..944c9e0 100644 --- a/app/services/code_grep_scanner.py +++ b/app/services/code_grep_scanner.py @@ -4,6 +4,7 @@ import re from typing import Any, Callable, Dict, List, Optional, Tuple +from app.services.code_grep_chunks import grep_chunk_matches from app.services.git_manager import format_git_permalink from app.services.text_decoding import read_source_text @@ -196,45 +197,18 @@ def execute_code_grep( case_sensitive=case_sensitive, limit=max(10, remaining_limit), ) - for ch in chunks: - ch_repo = ch.get("repo") or "unknown" - raw_ch_file = (ch.get("rel_path") or ch.get("path") or "").replace("\\", "/") - ch_file = raw_ch_file.split("://", 1)[1] if "://" in raw_ch_file else raw_ch_file - if ch_file.startswith(f"{ch_repo}/"): - ch_file = ch_file[len(ch_repo) + 1:] - - if (ch_repo, ch_file) in disk_scanned_keys: - continue - - text_content = ch.get("content") or ch.get("text") or "" - start_line = int(ch.get("start_line") or 1) - ch_lines = text_content.splitlines() - - for c_idx, line_str in enumerate(ch_lines): - curr_line = start_line + c_idx - if compiled.search(line_str): - key = (ch_repo, ch_file, curr_line) - if key in seen_keys: - continue - seen_keys.add(key) - - start_ctx = max(0, c_idx - clamped_context) - end_ctx = min(len(ch_lines), c_idx + clamped_context + 1) - context_items = [ - {"line": start_line + idx, "text": ch_lines[idx]} - for idx in range(start_ctx, end_ctx) - ] - matches.append({ - "repo": ch_repo, - "file": ch_file, - "line": curr_line, - "match_text": line_str, - "context": context_items, - }) - if len(matches) >= clamped_limit: - break - if len(matches) >= clamped_limit: - break + load_fn = None + if hasattr(store, "get_file_chunks"): + load_fn = lambda r_name, r_file: store.get_file_chunks(path=r_file, repo=r_name) + matches.extend(grep_chunk_matches( + chunks=chunks, + compiled=compiled, + context_lines=clamped_context, + limit=clamped_limit - len(matches), + skip_files=disk_scanned_keys, + seen_keys=seen_keys, + load_file_chunks=load_fn, + )) except Exception as e: logger.warning(f"grep_chunks fallback in grep_code encountered error: {e}") diff --git a/app/services/indexing/processor.py b/app/services/indexing/processor.py index dcd4aa5..a74aa9c 100644 --- a/app/services/indexing/processor.py +++ b/app/services/indexing/processor.py @@ -33,6 +33,21 @@ def _get_indexer_attr(name, default): from app.services.pdf_extractor import MAX_PDF_SIZE_BYTES +def frontmatter_line_offset(text: str, body: str) -> int: + """ + Number of file lines before `body`, the text that frontmatter.loads keeps. It removes the frontmatter and the + blank lines around the body, so chunk line numbers that count from the body must move down by this number. + """ + body_lines = [line.rstrip() for line in body.splitlines()[:5]] + if not body_lines: + return 0 + file_lines = [line.rstrip() for line in text.splitlines()] + for offset in range(len(file_lines) - len(body_lines) + 1): + if file_lines[offset:offset + len(body_lines)] == body_lines: + return offset + return 0 + + def compute_text_hash(text: str) -> str: """Computes SHA256 hash of utf-8 text.""" return hashlib.sha256(text.encode("utf-8")).hexdigest() @@ -240,6 +255,7 @@ def process_file_content( return points, ast_symbols, summary_tuple, ast_relationships, api_routes, api_calls if doc_type in ("doc", "pdf"): + body_line_offset = 0 if filepath.endswith((".md", ".txt")): # Check if file is in standard ADR directories or follows ADR naming norm_rel = rel_path.replace("\\", "/").lower() @@ -252,6 +268,7 @@ def process_file_content( try: post = frontmatter.loads(content) + body_line_offset = frontmatter_line_offset(content, post.content) content = post.content meta = post.metadata or {} if "category" in meta: @@ -267,6 +284,12 @@ def process_file_content( chunks_models = chunk_markdown(content, max_chars=CHUNK_SIZE, overlap=CHUNK_OVERLAP) chunks = [c.model_dump() for c in chunks_models] + if body_line_offset: + # Chunk lines count from the body; shift them so they are file line numbers. + for item in chunks + ast_relationships: + for line_key in ("start_line", "end_line", "line_number"): + if isinstance(item.get(line_key), int): + item[line_key] += body_line_offset valid_chunks = [c for c in chunks if c.get("content", "").strip()] headings = [c["heading"] for c in valid_chunks if c.get("heading") and c["heading"] != "Root"] keywords = extract_keywords_from_text(content, title, headings, tags) diff --git a/tests/test_grep_code_context_lines.py b/tests/test_grep_code_context_lines.py new file mode 100644 index 0000000..6d7f490 --- /dev/null +++ b/tests/test_grep_code_context_lines.py @@ -0,0 +1,159 @@ +"""grep_code: every context line has its own, correct line number on the disk, extended-scan, and chunk paths.""" + +import asyncio +import json +import re + +import pytest + +from app.mcp.handlers.grep_handlers import _sanitize_match_lines, handle_grep_code +from app.services.code_grep_chunks import build_line_map +from app.services.code_grep_scanner import execute_code_grep +from app.services.database.engine import get_db_engine, init_db +from app.services.file_reader import FileReaderService +from app.services.indexing.processor import frontmatter_line_offset + +FILE_LINES = [f"line {n}" + (" TARGET_A" if n == 47 else " TARGET_B" if n == 63 else "") for n in range(1, 100)] + + +def _assert_context(match, file_lines, context_lines): + numbers = [item["line"] for item in match["context"]] + first = max(1, match["line"] - context_lines) + last = min(len(file_lines), match["line"] + context_lines) + assert numbers == list(range(first, last + 1)) + for item in match["context"]: + assert item["text"] == file_lines[item["line"] - 1] + + +class _NoDb: + def __enter__(self): + raise RuntimeError("no database in this test") + + def __exit__(self, *exc): + return False + + +@pytest.fixture +def hub_reader(tmp_path, monkeypatch): + db_file = tmp_path / "grep_ctx.db" + db_url = f"sqlite:///{db_file}" + monkeypatch.setenv("DATABASE_URL", db_url) + monkeypatch.setattr("app.services.database.connection.CACHE_DB_PATH", str(db_file)) + monkeypatch.setattr("app.services.database.CACHE_DB_PATH", str(db_file), raising=False) + init_db(engine=get_db_engine(db_url, reset=True)) + + hub = tmp_path / "hub" + hub.mkdir() + (hub / "skill.md").write_text("\n".join(FILE_LINES) + "\n", encoding="utf-8") + reader = FileReaderService(storage_root=str(tmp_path / "storage")) + monkeypatch.setattr(reader, "_get_authorized_indexed_paths", lambda: [{"path": str(hub), "repo": "hub-prefix"}]) + monkeypatch.setattr(reader, "_get_authorized_persistent_repos", lambda: {}) + monkeypatch.setattr("app.mcp.handlers.grep_handlers._resolve_reader_factory", lambda: (lambda: reader)) + return reader + + +def _grep(**kwargs): + return json.loads(asyncio.run(handle_grep_code(**kwargs))) + + +@pytest.mark.parametrize("extra", [{}, {"offset": 95, "max_results": 10}]) +def test_indexed_path_regex_context_lines_are_consecutive(hub_reader, extra): + # offset + max_results > 100 sends the search through _scan_extended_matches. + result = _grep(pattern=r"TARGET_\w+", is_regex=True, repo="hub-prefix", context_lines=3, **extra) + if extra: + assert result["matches"] == [] + result = _grep(pattern=r"TARGET_\w+", is_regex=True, repo="hub-prefix", context_lines=3, max_results=100, offset=0) + assert [m["line"] for m in result["matches"]] == [47, 63] + for match in result["matches"]: + _assert_context(match, FILE_LINES, 3) + + +def _chunk(start, end, lines, **extra): + text = "\n".join(lines).strip() # the indexer strips chunk text + return {"repo": "hub-prefix", "rel_path": "docs/skill.md", "content": text, "start_line": start, "end_line": end, **extra} + + +def _file_chunks(): + # Chunk 2 starts with two blank lines that the strip removed; chunk 3 overlaps chunk 2. + lines = list(FILE_LINES) + lines[39] = lines[40] = "" + return lines, [ + _chunk(1, 41, lines[0:41]), + _chunk(40, 55, lines[39:55]), + _chunk(54, 99, lines[53:99]), + {"repo": "hub-prefix", "rel_path": "docs/skill.md", "doc_type": "summary", "is_summary": True, + "content": "Summary\nTARGET_A is used here\nTARGET_B too", "start_line": 1, "end_line": 1}, + ] + + +class _Store: + def __init__(self, chunks, with_file_chunks=True): + self.chunks = chunks + if with_file_chunks: + self.get_file_chunks = lambda path, repo=None: [c for c in chunks if c["rel_path"] == path] + + def grep_chunks(self, pattern, repo=None, path_glob=None, is_regex=False, case_sensitive=False, limit=100): + rx = re.compile(pattern if is_regex else re.escape(pattern), 0 if case_sensitive else re.IGNORECASE) + return [c for c in self.chunks if rx.search(c["content"])][:limit] + + +def _chunk_grep(monkeypatch, tmp_path, store): + monkeypatch.setattr("app.services.vector_store.get_vector_store", lambda: store) + return execute_code_grep( + pattern=r"TARGET_\w+", storage_root=str(tmp_path / "none"), indexed_paths=[], persistent_repos={}, + is_binary_fn=lambda p: False, db_conn_factory=_NoDb, repo="hub-prefix", is_regex=True, context_lines=3, + ) + + +def test_chunk_path_context_lines_have_file_line_numbers(monkeypatch, tmp_path): + file_lines, chunks = _file_chunks() + result = _chunk_grep(monkeypatch, tmp_path, _Store(chunks)) + # The summary chunk gives no matches, and line 47 keeps its real number after the stripped blank lines. + assert [(m["file"], m["line"]) for m in result["matches"]] == [("docs/skill.md", 47), ("docs/skill.md", 63)] + for match in result["matches"]: + assert match["match_text"] == file_lines[match["line"] - 1] + _assert_context(match, file_lines, 3) + + +def test_chunk_path_without_file_chunks_gives_consecutive_numbers(monkeypatch, tmp_path): + # Only the matched chunks are known, so the stripped start of chunk 2 cannot be placed; the numbers still go up by one. + _, chunks = _file_chunks() + result = _chunk_grep(monkeypatch, tmp_path, _Store(chunks, with_file_chunks=False)) + assert len(result["matches"]) == 2 + for match in result["matches"]: + numbers = [item["line"] for item in match["context"]] + assert numbers == list(range(match["line"] - 3, match["line"] + 4)) + + +def test_chunk_context_continues_past_the_chunk_edge(): + lines = [f"row {n}" for n in range(1, 21)] + line_map = build_line_map([_chunk(1, 10, lines[0:10]), _chunk(11, 20, lines[10:20])]) + assert [line_map[n] for n in range(1, 21)] == lines + + +def test_sanitizer_renumbers_context_lines_that_share_one_number(): + match = { + "repo": "hub-prefix", "file": "skill.md", "line": 47, "match_text": "line 47 TARGET_A", + "context": [{"line": 47, "text": FILE_LINES[n - 1]} for n in range(44, 51)], + } + sanitized = _sanitize_match_lines(match) + assert [c["line"] for c in sanitized["context"]] == list(range(44, 51)) + assert [c["text"] for c in sanitized["context"]] == FILE_LINES[43:50] + + +def test_sanitizer_keeps_correct_context_lines(): + match = {"line": 2, "match_text": "b", "context": [{"line": 1, "text": "a"}, {"line": 2, "text": "b"}]} + assert _sanitize_match_lines(match)["context"] == match["context"] + + +@pytest.mark.parametrize("text, expected", [ + ("---\ntitle: x\ntags: a\n---\n\n# Heading\nbody\n", 5), + ("\r\n# Heading\r\n\r\nbody\r\n", 1), + ("# Heading\nbody\n", 0), +]) +def test_frontmatter_line_offset(text, expected): + import frontmatter + + body = frontmatter.loads(text).content + assert frontmatter_line_offset(text, body) == expected + assert text.splitlines()[expected].strip() == body.splitlines()[0]