From ed0a3b0135e935b5bbfd8dbe9ae9c5a6602b5cd2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:02:58 +0000 Subject: [PATCH 1/5] Add ADR-050: outline, symbol source and changed-symbol tools Proposed on its branch: answer B-017, B-016 and B-020 from the symbol table, verify served text against the source the index was built from, and diff with git against the merge base of a validated ref. Co-Authored-By: Claude [ADR-050] --- docs/adr/ADR-050-symbol-table-read-tools.md | 176 ++++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 docs/adr/ADR-050-symbol-table-read-tools.md diff --git a/docs/adr/ADR-050-symbol-table-read-tools.md b/docs/adr/ADR-050-symbol-table-read-tools.md new file mode 100644 index 0000000..6cd7ae5 --- /dev/null +++ b/docs/adr/ADR-050-symbol-table-read-tools.md @@ -0,0 +1,176 @@ +# ADR-050: Answer from the Symbol Table: File Outline, One Symbol's Source, Changed Symbols + +**Status:** proposed +**Date:** 2026-10-20 +**Branch:** `feature/adr-050-symbol-tools` +**Reviewer:** @edb +**Backlog:** [B-017](../backlog.md#b-017) (outline), [B-016](../backlog.md#b-016) (one symbol's source), [B-020](../backlog.md#b-020) (diff-scoped), all from the [jCodeMunch study](../study-jcodemunch-mcp.md) §3.1, §3.2, §3.5 +**Depends on:** none. It reads what ADR-025 (`files.content_hash`, `content_changed_at`, `index_meta`) and ADR-042 (`source.py`, `index_meta.source`) already store. +**Depended on by:** none yet. B-021 (steer Read toward the index) would point at `get_file_outline`. + +## Context + +The index already stores every symbol's kind, name, FQN and line span (`symbols`, `symbol_locations`), +each file's content hash (`files.content_hash`), and the commit a git-mode index was built from +(`index_meta.last_indexed_commit`, ADR-042). No MCP tool serves those directly, so an agent that wants +to know what is in a file, or to see one function, Reads the whole file. + +The jCodeMunch study (§3) names three tools worth having, and one trap: jCodeMunch serves its stored +copy of a symbol even when the file has changed since it was indexed, so the agent gets old code +presented as current. The study's license warning applies: everything here is built from the behavior +described in the backlog, against this repo's schema. No jCodeMunch source was read for it. + +Facts the decisions rest on, checked on `871b55c`: + +- **No signature is stored.** `symbols` holds `kind`, `name`, `class_context`, the span and the raw + `text`. `symbol_types` has return and parameter types, but only for TypeScript. +- **`files.content_hash` is whatever `Source.list()` returned**: an MD5 of the raw bytes for the + folder, the blob SHA for a git commit (ADR-042 §1). +- **`content_changed_at` says when the content last changed before it was indexed** (git committer + time), not whether it changed since. It can't detect a stale index; only the hash can. +- **A symbol's stored `text` is its node's text**: it starts at the node's column, so its first line + lacks the indentation that the file's line has. + +## Decision + +Three read tools, registered with `@_tool()` like the other read tools, in one block of `MCPServer.py`, +over new read-only `CodeDB` methods. They do no retrieval and load no model. + +### 1. `get_file_outline(file)` (B-017) + +- Each symbol in the file, as `L- `, indented by nesting (a method under + its class), straight from `symbol_locations` joined to `symbols`. `symbol_locations` rather than + `symbols.file_id` so a C# partial class lists its part in each file it spans. +- **The signature is derived, not stored**: the declaration text of the stored symbol up to its body. + Read from the start of `text`, skip decorator and attribute lines, and stop at the first `{` outside + brackets, or at a `:` that ends a line outside brackets (Python). Whitespace is collapsed to one line + and capped at 200 characters. If no body opener is found, the first line is used. A parse-time + signature column was rejected (below): it means a schema change and a full reindex for a display + string. +- **Two different empty answers**, both explicit: + - the file is indexed but has no symbols → a success that says so (and that the file is in the index); + - the file isn't in the index → an error that says it isn't, with near-miss paths when there are any. +- **Paths:** `\` or `/`, a leading `./`, or an absolute path inside the repo are all normalised to the + repo-relative POSIX path the index stores. A bare tail (`db.py`, `src/db.py`) that matches exactly one + indexed path is accepted; several matches are an error listing them. +- **Freshness note:** when the file on disk no longer matches its indexed hash (folder mode), the + outline says so, because its line numbers may be off. + +### 2. `get_symbol_source(symbol, file="")` (B-016) + +- `symbol` is a bare name (`login`), a qualified tail (`AuthService.login`) or a full FQN. `file` + narrows it like `get_file_outline`'s argument. +- **Ambiguous → list, don't guess.** Several matches return the candidates (FQN, file, lines) as a + successful answer that says how to narrow. No match is an error. +- **Text comes from the source the index was built from, through `source.py`, never from the stored + copy and never from `open()`.** + - Folder mode: `WorkingTreeSource.read`. + - Git mode (`index_meta.source` starts with `git:`): `GitCommitSource` at the indexed commit + (`index_meta.last_indexed_commit`). That is the exact text the symbol table describes. The answer + names the commit and says it is not the caller's working tree, the same caveat the server + instructions give. +- **Staleness: re-read and verify, refuse when it can't be verified.** The stored `text` is the + witness. The span read from the source is compared with it (both stripped, which absorbs the + first-line indentation): + 1. It matches → served with exact line numbers. If the file's hash differs from the indexed one, the + answer says the file changed elsewhere but this symbol's span did not. + 2. It doesn't, but the stored text occurs exactly once elsewhere in the current file → served from + its new position, and the answer says it moved from the indexed lines. + 3. Neither → **refused with an error**: the symbol changed since indexing, here are the indexed + lines, Read the file. Serving the re-read span here would show whatever now sits on those lines, + which may be a different function. That is the backlog's trap in a new form. + The hash is the cheap "file changed" signal; the text comparison decides what is served. So a change + to how `content_hash` is computed (ADR-051 is touching keys on a sibling branch) can at worst add a + spurious "file changed" note, never serve wrong text. +- **Skeleton texts.** A class whose stored `text` is an ADR-034 skeleton (stubbed bodies) can't be + matched line for line. A class span is verified by its declaration line alone, and its hash. See the + Implementation Log for what the adapters actually store. +- Spans longer than 400 lines are cut, with a note pointing at `get_file_outline`. + +### 3. `changed_symbols(ref, blast_radius=False, limit=100)` (B-020) + +- **Shell out to git; don't read `content_changed_at`.** A timestamp says when a file last changed, not + which lines, and it can't express "since ``". Only a diff gives line ranges to intersect with + symbol spans. +- **Hardening:** + - `ref` is rejected before git sees it if it is empty, starts with `-` (`--output=x` would be an + option), or holds whitespace or control characters; + - then `git rev-parse --verify --quiet --end-of-options ^{commit}` must name a commit; + - every call uses a list of arguments (never `shell=True`), `stdin=DEVNULL` and a 30 s timeout; + - outside a git repository, or with no git binary, the tool fails with an error naming that. +- **What it diffs.** The question is "what did this work change since it left ``", so the base is + `git merge-base `, falling back to `` itself when they share no history. When + `` is an ancestor (`HEAD~3`) the merge base is ``, so nothing changes for that case. + - Git mode: the indexed side is `index_meta.last_indexed_commit`. The diff is commit to commit, and its + new-side line numbers are exactly the index's. + - Folder mode: the indexed side is the working tree (`git diff `, plus untracked indexed files, + whose symbols are all new), and the merge base is taken against `HEAD`. A file whose disk hash no + longer matches the index is flagged: its ranges may be off until the watchdog catches up. +- **Intersection.** `git diff -U0` gives the new-side lines each hunk added or changed. A pure deletion is + a point between two lines. A symbol is reported when a changed line falls in its *own* lines: its span + minus its nested symbols' spans. A one-line edit inside a method reports the method, not the method + and its class. A deletion counts only when it is strictly inside a symbol (`start <= after < end`), so + removing the blank lines between two functions reports neither. +- **What it can't see:** a symbol deleted since `` isn't in the index, which describes only the new + side. Deleted files are listed by name, and so are changed files the index doesn't hold (docs, config). +- **Blast radius** (`blast_radius=True`) reuses `analyze_blast_radius`'s internals by calling them, + unmodified: `_caller_evidence` per changed symbol (verified callers, and a count of candidate ones, + under the ADR-017 §7 split) and `_import_relations` per changed file (its importers). Capped at the + first 25 symbols, which the answer says. + +### Shared + +- New `CodeDB` methods, appended at the end of the class and read-only: `get_file_record`, + `find_file_paths`, `get_file_symbol_locations`, `find_symbol_locations`. +- An index with no files at all (a server started where nothing was built) fails each tool with + "no index here", not "file not indexed". +- Every failure is raised, so the MCP client sees `isError: true`, as `reindex` already does. + +## Consequences + +**Better:** +- "What's in this file" and "show me this function" cost a few hundred tokens, not a whole Read. +- An agent can't get old code presented as current: the answer is verified against the source, moved + with a note, or refused. +- Pre-merge review gets the changed symbols, and optionally who calls them, in one call. + +**Worse:** +- Three more tool descriptions in every session's context (17 tools, from 14; B-019 is the measuring + item). +- `changed_symbols` runs git from the server process. It is bounded by timeouts and argument lists, but + it is a subprocess per call. +- The derived signature is a heuristic. It is right for the declaration shapes of the five languages + the tests cover; an exotic declaration falls back to its first line. + +**Neutral:** +- No schema change and no reindex. The tools read only what the index already holds. +- In folder mode, symbols in files the watchdog hasn't re-indexed yet are flagged, not hidden. + +## Alternatives Considered + +| Option | Why rejected | +|--------|-------------| +| Store a `signature` column at parse time | A schema migration and a full reindex for a display string. The derived one reads the same stored text. Worth revisiting if adapters start emitting a real signature. | +| Serve the stored `symbols.text` | It is exactly the stale copy the backlog warns about, and for classes it may be a skeleton. | +| On a changed file, re-read the indexed line span and warn | The lines may now hold a different symbol. Re-reading is only safe when the stored text confirms what is there; otherwise refuse. | +| Refuse whenever the file's hash changed | Throws away the common case: an edit elsewhere in the file leaves this symbol intact. The text check keeps it. | +| B-020 from `content_changed_at` | File-level timestamps: no line ranges, no notion of a ref. | +| B-020 diffing `` directly, not the merge base | With a `` that moved on (main after a merge), the answer would include everything main did since, reversed. The merge base is what a reviewer means. | +| Use GitPython or pygit2 | A new runtime dependency. The repo already shells out to git with list arguments (`source.py`, `index_location.py`). | +| Report every symbol whose span intersects | A one-line change in a method also reports its class, and every enclosing namespace in C#. | + +## Implementation Log + +> Updated during development. Record deviations from the design, surprises, and decisions made in the moment. + +- [ ] `CodeDB` read methods +- [ ] `get_file_outline` + tests +- [ ] `get_symbol_source` + tests (unique, ambiguous, changed on disk, git mode) +- [ ] `changed_symbols` + tests (temp git repo: changed vs untouched function, bad ref, option-like ref) +- [ ] Each tool's failure path is `isError` through the MCP layer (pytest, FastMCP client session) +- [ ] MCP Inspector `tools/list` and one good and one bad call per tool +- [ ] Tool lists in `src/CLAUDE.md` and `README.md` + +**Open questions for @edb:** + +**Notes:** From b0102b3dac81d0d0955d5ccc57c7f1b345003c92 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:35:26 +0000 Subject: [PATCH 2/5] Add read-only CodeDB lookups for files and symbol locations has_files, get_file_record, find_file_paths, get_file_symbol_locations and find_symbol_locations, appended to CodeDB for ADR-050's tools. Locations are read through symbol_locations so a partial class lists its part in each file. Co-Authored-By: Claude [ADR-050 src/db.py] --- src/db.py | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/src/db.py b/src/db.py index 465438e..03b42d8 100644 --- a/src/db.py +++ b/src/db.py @@ -1478,3 +1478,73 @@ def stats(self) -> dict[str, int]: t: self._conn.execute(f"SELECT COUNT(*) FROM {t}").fetchone()[0] for t in tables } + + # ------------------------------------------------------------------ + # Symbol-table reads for the outline / source / changed-symbol tools (ADR-050) + # ------------------------------------------------------------------ + + def has_files(self) -> bool: + """Whether anything has been indexed into this database.""" + return self._conn.execute("SELECT 1 FROM files LIMIT 1").fetchone() is not None + + def get_file_record(self, path: str) -> Optional[dict]: + """The `files` row for an exact repo-relative path, or None.""" + row = self._conn.execute( + "SELECT path, content_hash, indexed_at, content_changed_at FROM files WHERE path = ?", + (path,), + ).fetchone() + return dict(row) if row is not None else None + + def find_file_paths(self, tail: str, limit: int = 20) -> list[str]: + """Indexed paths equal to ``tail`` or ending in ``/tail``, compared case-insensitively. + + ``tail`` is a repo-relative POSIX path or a trailing part of one (``db.py``, + ``src/db.py``). Matched in Python rather than with LIKE, whose ``_`` wildcard + would match half the file names in a Python repo. + """ + want = tail.lower() + rows = self._conn.execute("SELECT path FROM files").fetchall() + hits = sorted(p for (p,) in rows if p.lower() == want or p.lower().endswith("/" + want)) + return hits[:limit] + + def get_file_symbol_locations(self, path: str) -> list[dict]: + """Every symbol located in ``path``, ordered by start line, outer before inner. + + Read through ``symbol_locations``, so a C# partial class lists the part declared in + this file, with this file's span, rather than only under the file that claimed its + ``symbols`` row. Dicts: fqn, kind, name, class_context, start_line, end_line, text. + """ + rows = self._conn.execute( + """ + SELECT s.fqn, s.kind, s.name, s.class_context, + l.start_line, l.end_line, l.text + FROM symbol_locations l + JOIN symbols s ON s.id = l.symbol_id + JOIN files f ON f.id = l.file_id + WHERE f.path = ? + ORDER BY l.start_line, l.end_line DESC, s.fqn + """, + (path,), + ).fetchall() + return [dict(r) for r in rows] + + def find_symbol_locations(self, name: str, limit: int = 200) -> list[dict]: + """Every location of a symbol whose ``name`` column equals ``name`` exactly. + + Dicts as get_file_symbol_locations', plus ``path``. Callers narrow by a qualified + tail or FQN themselves; ``name`` is indexed, so this is a point lookup. + """ + rows = self._conn.execute( + """ + SELECT s.fqn, s.kind, s.name, s.class_context, + l.start_line, l.end_line, l.text, f.path + FROM symbols s + JOIN symbol_locations l ON l.symbol_id = s.id + JOIN files f ON f.id = l.file_id + WHERE s.name = ? + ORDER BY f.path, l.start_line + LIMIT ? + """, + (name, limit), + ).fetchall() + return [dict(r) for r in rows] From 158c547accf8cd8c08dced8659d2e7a80a0ff636 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:35:26 +0000 Subject: [PATCH 3/5] Add get_file_outline, get_symbol_source and changed_symbols Three read tools that answer from the symbol table (B-017, B-016, B-020). Symbol source is read through source.py from what the index was built from (the indexed commit in git mode) and checked against the stored text: served in place, served from its new lines when it moved, or refused when it changed. changed_symbols validates the ref before git sees it and diffs from the merge base to the indexed side. test_index_state's registered-tool count moves from 14 to 17. Co-Authored-By: Claude [ADR-050 src/MCPServer.py] --- src/MCPServer.py | 557 +++++++++++++++++++++++++++++++++++++ tests/test_index_state.py | 2 +- tests/test_symbol_tools.py | 539 +++++++++++++++++++++++++++++++++++ 3 files changed, 1097 insertions(+), 1 deletion(-) create mode 100644 tests/test_symbol_tools.py diff --git a/src/MCPServer.py b/src/MCPServer.py index b0aeb0b..de5bd88 100644 --- a/src/MCPServer.py +++ b/src/MCPServer.py @@ -2391,6 +2391,563 @@ def map_module_communities(target_path: str = "", min_community_size: int = 3, _reindex_lock = threading.Lock() +# --------------------------------------------------------------------------- +# ADR-050 — answers straight from the symbol table: a file's outline, one +# symbol's source, the symbols a diff touched. No retrieval and no model. +# --------------------------------------------------------------------------- + +_OUTLINE_SIG_CHARS = 200 # a derived signature is cut here +_SOURCE_MAX_LINES = 400 # a longer span is cut, with a pointer to the outline +_GIT_TIMEOUT = 30 # seconds, for every git call changed_symbols makes +_BLAST_SYMBOLS = 25 # changed symbols whose callers blast_radius looks up +_SKELETON_STUB = " ...\n" # what ADR-034 puts in place of a member's body +_PY_EXTS = (".py", ".pyi") + + +def _require_index(db) -> None: + """Fail when nothing has been built here, rather than call every file 'not indexed'.""" + if not db.has_files(): + raise RuntimeError( + "No index here: graph.db holds no files. The server indexes the directory it " + f"was started in ({os.getcwd()}); build it there first.") + + +def _indexed_path(db, file: str) -> str: + """The repo-relative POSIX path the index stores for ``file``. + + Takes either separator, a leading ``./``, an absolute path inside the repo, or a + trailing part of a path (``db.py``) that matches exactly one indexed file. + """ + raw = (file or "").strip() + if not raw: + raise ValueError("file is required: a repo-relative path such as 'src/db.py'") + p = raw.replace("\\", "/") + root = os.getcwd().replace("\\", "/").rstrip("/") + if p.lower().startswith(root.lower() + "/"): + p = p[len(root) + 1:] + while p.startswith("./"): + p = p[2:] + p = p.strip("/") + if db.get_file_record(p): + return p + hits = db.find_file_paths(p) + if len(hits) == 1: + return hits[0] + if hits: + raise ValueError(f"'{file}' matches {len(hits)} indexed files; pass one of:\n" + + "".join(f" {h}\n" for h in hits)) + msg = f"'{file}' is not in the index." + if os.path.isfile(os.path.join(os.getcwd(), p)): + msg += (" The file exists, so it is excluded by [ignore], not a language the " + "indexer parses, or not indexed yet; Read it directly.") + near = db.find_file_paths(p.rsplit("/", 1)[-1]) if "/" in p else [] + if near: + msg += " Indexed files with that name:\n" + "".join(f" {h}\n" for h in near) + raise LookupError(msg) + + +def _signature(text: str, path: str) -> str: + """A symbol's declaration, from its stored text: everything up to its body. + + No signature is stored (ADR-050 §1), so it is cut from the text: decorator and + attribute lines are skipped, then it stops at the first ``{``, ``;`` or ``=>`` + outside brackets, or, in Python, at a ``:`` that ends a line. Collapsed to one line. + """ + lines = text.split("\n") + while len(lines) > 1 and lines[0].lstrip().startswith(("@", "[")): + lines.pop(0) # decorators, C# attributes + body = "\n".join(lines) + python = path.lower().endswith(_PY_EXTS) + depth, cut = 0, None + for i, ch in enumerate(body): + if ch in "([": + depth += 1 + elif ch in ")]": + depth = max(0, depth - 1) + elif depth == 0: + if ch in "{;" or body.startswith("=>", i): + cut = i + break + if python and ch == ":" and body[i + 1:].split("\n", 1)[0].split("#", 1)[0].strip() == "": + cut = i + break + head = body[:cut] if cut is not None else lines[0] + sig = re.sub(r"\s+", " ", head).strip() + sig = re.sub(r"\(\s+", "(", re.sub(r",?\s+\)", ")", sig)) + if len(sig) > _OUTLINE_SIG_CHARS: + sig = sig[:_OUTLINE_SIG_CHARS] + " …" + return sig or lines[0].strip() + + +def _nesting(locs: list[dict]) -> list[int]: + """Each symbol's depth inside the ones before it (input is outer-before-inner).""" + depths: list[int] = [] + stack: list[int] = [] # end lines of the enclosing symbols + for loc in locs: + while stack and loc["start_line"] > stack[-1]: + stack.pop() + depths.append(len(stack)) + stack.append(loc["end_line"]) + return depths + + +def _index_source(db) -> tuple[str, str | None]: + """(``index_meta.source``, the indexed commit): what the index was built from.""" + return db.meta_get("source") or "worktree", db.meta_get("last_indexed_commit") + + +def _source_label(db) -> str | None: + """For a git-mode index, which commit answers come from; None for the folder.""" + label, commit = _index_source(db) + if not label.startswith("git:"): + return None + if not commit: + raise RuntimeError(f"This index reads {label} but records no last_indexed_commit; " + "it has no finished build to read from.") + return f"commit {commit[:10]} ({label}): the index's source, not your working tree" + + +def _open_source(db): + """The ``source.py`` Source the index was built from, and its _source_label. + + Git mode (ADR-042) reads the indexed commit, so the text is exactly what the symbol + table describes; the folder is read as the indexer reads it. + """ + from source import GitCommitSource, GitError, WorkingTreeSource + where = _source_label(db) + if where is None: + return WorkingTreeSource(os.getcwd()), None + commit = _index_source(db)[1] + try: + return GitCommitSource(os.getcwd(), commit), where + except GitError as exc: + raise RuntimeError(f"Cannot read the indexed commit {commit[:10]}: {exc}") from exc + + +def _file_unchanged(db, path: str) -> bool: + """Whether the file still has the content hash it was indexed with.""" + rec = db.get_file_record(path) + if rec is None: + return False + label, commit = _index_source(db) + if label.startswith("git:"): + try: + blob = _git_run("rev-parse", "--verify", "--quiet", f"{commit}:{path}").strip() + except RuntimeError: + return False + return blob == rec["content_hash"] + from source import md5_file + try: + return md5_file(os.path.join(os.getcwd(), path)) == rec["content_hash"] + except OSError: + return False + + +def _text_blocks(stored: str) -> list[str]: + """The pieces of a stored symbol text that appear verbatim in its source. + + A class's text is an ADR-034 skeleton (member bodies replaced by `` ...``), and a + merged overload set is its parts reordered, so the text as a whole need not occur in + the file. Each run of lines between stubs and blank lines does. + """ + blocks = [] + for piece in stored.split(_SKELETON_STUB): + for block in re.split(r"\n\s*\n", piece): + block = block.strip() + if block.endswith(" ..."): + block = block[:-4].rstrip() + if block: + blocks.append(block) + return blocks + + +def _locate(stored: str, lines: list[str], start: int, end: int) -> tuple[int, int] | None: + """Where the symbol whose indexed text is ``stored`` is now, or None if it changed. + + Its indexed span still holding every block of the stored text confirms it in place. + Otherwise a span of the same length starting where its first block now occurs, if + exactly one such span holds every block, is where it moved to. + """ + blocks = _text_blocks(stored) + if not blocks: + return None + + def holds(a: int, b: int) -> bool: + span = "\n".join(lines[a - 1:b]) + return all(blk in span for blk in blocks) + + if holds(start, end): + return start, end + first = blocks[0].split("\n", 1)[0] + found = [i + 1 for i, line in enumerate(lines) if first in line] + moved = [(a, a + end - start) for a in found if holds(a, a + end - start)] + return moved[0] if len(moved) == 1 else None + + +@_tool() +def get_file_outline(file: str) -> str: + """ + List what one file defines: each symbol's kind, signature and line range, nested, + with no bodies. Call this instead of Read when you need to know what a file + contains or where something is in it, then get_symbol_source for the one symbol + you need (or Read just its lines). `file` is a repo-relative path (either slash), + an absolute path in the repo, or a unique tail such as 'db.py'. Fails if the file + is not indexed; says so if it is indexed but defines no symbols. Lines are the + indexed version's, and the outline says when the file has changed since. + """ + print(f"\n[MCP] get_file_outline: '{file}'") + db = _db() + _require_index(db) + path = _indexed_path(db, file) + locs = db.get_file_symbol_locations(path) + where = _source_label(db) + + out = [f"--- OUTLINE: {path} ({len(locs)} symbol(s)) ---"] + if where: + out.append(f"(as of {where})") + elif not _file_unchanged(db, path): + out.append("⚠️ This file has changed on disk since it was indexed; line numbers " + "may be off until the watchdog re-indexes it.") + if not locs: + out.append("The file is indexed but defines no symbols the parser extracts " + "(e.g. only top-level statements, or a config/descriptor file). " + "Read it if you need its contents.") + return "\n".join(out) + for loc, depth in zip(locs, _nesting(locs)): + sig = _signature(loc["text"], path) + if loc["name"] not in sig: + sig = f"{loc['name']} {sig}" + out.append(f"{' ' * depth}L{loc['start_line']}-{loc['end_line']} {loc['kind']} {sig}") + return "\n".join(out) + + +def _match_symbols(db, symbol: str) -> list[dict]: + """Locations of ``symbol``: a bare name, a qualified tail or an FQN.""" + def norm(fqn: str) -> str: + fqn = re.sub(r"(\(.*\)|/\d+)$", "", fqn) # C++ parameter lists, C# arity + return fqn.replace("::", ".") + + q = norm(symbol.strip()) + name = q.rsplit(".", 1)[-1] + seen, hits = set(), [] + for cand in {name, symbol.strip()}: + for loc in db.find_symbol_locations(cand): + key = (loc["fqn"], loc["path"], loc["start_line"]) + if key in seen: + continue + seen.add(key) + f = norm(loc["fqn"]) + if loc["name"] == symbol.strip() or f == q or f.endswith("." + q): + hits.append(loc) + return sorted(hits, key=lambda h: (h["path"], h["start_line"], h["fqn"])) + + +@_tool() +def get_symbol_source(symbol: str, file: str = "") -> str: + """ + Return one function's, method's or class's source instead of Reading its whole + file. `symbol` is a name ('login'), a qualified tail ('AuthService.login') or an + FQN; `file` optionally narrows it (same forms as get_file_outline). Several matches + are listed (FQN, file, lines) for you to call again with `file` or a qualified name. + The text is read from the source the index was built from and checked against what + was indexed: a symbol that moved comes with its new lines, and one that changed + since indexing fails rather than show stale code, so Read the file then. + """ + print(f"\n[MCP] get_symbol_source: symbol='{symbol}' file='{file}'") + if not symbol or not symbol.strip(): + raise ValueError("symbol is required: a name, 'Class.method' or an FQN") + db = _db() + _require_index(db) + hits = _match_symbols(db, symbol) + if file: + path = _indexed_path(db, file) + hits = [h for h in hits if h["path"] == path] + if not hits: + near = sorted({s.fqn for s in db.search_symbols(symbol.strip().rsplit(".", 1)[-1])})[:8] + msg = f"No symbol '{symbol}' in the index" + (f" in {file}" if file else "") + "." + if near: + msg += " Symbols starting with that name:\n" + "".join(f" {f}\n" for f in near) + raise LookupError(msg) + if len(hits) > 1: + out = [f"'{symbol}' is ambiguous: {len(hits)} symbols match. Call again with `file` " + "or a qualified name (e.g. 'Class.method'):"] + out += [f" {h['fqn']} [{h['kind']}] {h['path']}:{h['start_line']}-{h['end_line']}" + for h in hits[:30]] + if len(hits) > 30: + out.append(f" … {len(hits) - 30} more") + return "\n".join(out) + + hit = hits[0] + path, start, end = hit["path"], hit["start_line"], hit["end_line"] + src, where = _open_source(db) + try: + text = src.read(path) + except (OSError, RuntimeError) as exc: + raise RuntimeError(f"{path} could not be read ({exc}); it may have been deleted " + "since it was indexed.") from exc + finally: + src.close() + lines = text.split("\n") + unchanged = _file_unchanged(db, path) + notes = [] + if not unchanged: + found = _locate(hit["text"], lines, start, end) + if found is None: + raise RuntimeError( + f"{hit['fqn']} has changed since it was indexed: {path} lines {start}-{end} " + "no longer hold the indexed code, and it was not found elsewhere in the file. " + "Not showing stale text. Read the file (get_file_outline lists the indexed " + "lines), or call again once the watchdog has re-indexed it.") + if found != (start, end): + notes.append(f"Moved since indexing: indexed at lines {start}-{end}, now at " + f"{found[0]}-{found[1]}. Other line numbers in this file may have " + "shifted too.") + start, end = found + else: + notes.append(f"{path} has changed since it was indexed, but this symbol's lines " + "have not.") + + shown_end = min(end, start + _SOURCE_MAX_LINES - 1) + out = [f"--- {hit['fqn']} [{hit['kind']}] {path}:{start}-{end} ---"] + if where: + out.append(f"(from {where})") + out += notes + out += [f"{n:>5} {lines[n - 1]}" for n in range(start, shown_end + 1) if n <= len(lines)] + if shown_end < end: + out.append(f" … {end - shown_end} more line(s) cut; get_file_outline lists its members, " + f"or Read {path} from line {shown_end + 1}.") + return "\n".join(out) + + +def _git_run(*args: str) -> str: + """``git `` in the repo; its stdout. Raises RuntimeError with git's own message.""" + import subprocess + try: + done = subprocess.run( + ["git", *args], cwd=os.getcwd(), capture_output=True, text=True, + encoding="utf-8", errors="replace", stdin=subprocess.DEVNULL, timeout=_GIT_TIMEOUT, + ) + except FileNotFoundError as exc: + raise RuntimeError("git is not installed or not on PATH") from exc + except subprocess.TimeoutExpired as exc: + raise RuntimeError(f"git {args[0]} timed out after {_GIT_TIMEOUT} s") from exc + if done.returncode != 0: + raise RuntimeError(f"git {args[0]} failed: {done.stderr.strip() or f'exit {done.returncode}'}") + return done.stdout + + +_UNSAFE_REF_RE = re.compile(r"[\s\x00-\x1f\x7f]") + + +def _check_ref_shape(ref: str) -> str: + """``ref`` stripped; refuses an empty, option-like or malformed one before git sees it.""" + ref = (ref or "").strip() + if not ref: + raise ValueError("ref is required: a branch, tag or commit such as 'main'") + if ref.startswith("-") or _UNSAFE_REF_RE.search(ref) or len(ref) > 256: + raise ValueError(f"ref {ref!r} is not a git ref (refs cannot start with '-' or " + "contain whitespace or control characters)") + return ref + + +def _verify_ref(ref: str) -> str: + """The commit ``ref`` names, after _check_ref_shape.""" + ref = _check_ref_shape(ref) + try: + return _git_run("rev-parse", "--verify", "--quiet", "--end-of-options", + f"{ref}^{{commit}}").strip() + except RuntimeError as exc: + raise ValueError(f"ref {ref!r} does not name a commit in this repository " + "(fetch it first?)") from exc + + +def _diff_hunks(diff: str) -> list[dict]: + """Per file of a ``git diff -U0``: its old and new paths, status and new-side changes. + + ``ranges`` are new-side line ranges added or changed; ``points`` are new-side lines + after which old lines were only deleted. Header lines are read only before a file's + first hunk, so a deleted line that starts with ``--`` is not mistaken for one. + """ + files: list[dict] = [] + cur: dict | None = None + in_header = False + hunk_re = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@") + for line in diff.split("\n"): + if line.startswith("diff --git "): + cur = {"old": None, "new": None, "status": "modified", "ranges": [], "points": []} + files.append(cur) + in_header = True + continue + if cur is None: + continue + if in_header and not line.startswith("@@"): + if line.startswith("new file mode"): + cur["status"] = "added" + elif line.startswith("deleted file mode"): + cur["status"] = "deleted" + elif line.startswith("rename from "): + cur["status"], cur["old"] = "renamed", line[len("rename from "):] + elif line.startswith("rename to "): + cur["new"] = line[len("rename to "):] + elif line.startswith("--- "): + cur["old"] = None if line[4:] == "/dev/null" else line[4:] + elif line.startswith("+++ "): + cur["new"] = None if line[4:] == "/dev/null" else line[4:] + continue + m = hunk_re.match(line) + if m: + in_header = False + start = int(m.group(1)) + count = int(m.group(2)) if m.group(2) is not None else 1 + if count: + cur["ranges"].append((start, start + count - 1)) + else: + cur["points"].append(start) + return files + + +def _touched(locs: list[dict], ranges: list[tuple[int, int]], points: list[int]) -> list[dict]: + """The symbols a change falls in: in a symbol's own lines, not only a nested one's. + + A one-line edit in a method reports the method, not also its class. A pure deletion + counts only strictly inside a symbol (``start <= after < end``). + """ + hit = [] + for i, loc in enumerate(locs): + s, e = loc["start_line"], loc["end_line"] + if not (any(a <= e and b >= s for a, b in ranges) or any(s <= p < e for p in points)): + continue + kids = [(k["start_line"], k["end_line"]) for j, k in enumerate(locs) + if j != i and s <= k["start_line"] and k["end_line"] <= e + and (k["start_line"], k["end_line"]) != (s, e)] + + def own(n: int) -> bool: + return not any(a <= n <= b for a, b in kids) + + if any(own(n) for a, b in ranges for n in range(max(a, s), min(b, e) + 1)) or \ + any(s <= p < e and not any(a <= p < b for a, b in kids) for p in points): + hit.append(loc) + return hit + + +@_tool() +def changed_symbols(ref: str, blast_radius: bool = False, limit: int = 100) -> str: + """ + List the functions, methods and classes whose lines changed between a git ref and + the indexed code. Use it to review a branch or PR ("what did this touch?") instead + of reading the diff file by file. `ref` is a branch, tag or commit; the diff starts + at its merge base with the indexed code, so 'main' means "since this branch left + main". blast_radius=True also lists each changed symbol's resolved callers (and a + count of unverified ones) and each changed file's importers. Symbols deleted since + `ref` are not in the index; deleted files are listed by name. `limit` caps the + symbols listed (0 = all). + """ + print(f"\n[MCP] changed_symbols: ref='{ref}' blast_radius={blast_radius}") + _check_ref_shape(ref) + db = _db() + _require_index(db) + try: + _git_run("rev-parse", "--show-toplevel") + except RuntimeError as exc: + raise RuntimeError(f"changed_symbols needs a git repository: {os.getcwd()} is not " + f"inside one ({exc})") from exc + base_ref = _verify_ref(ref) + + label, commit = _index_source(db) + git_mode = label.startswith("git:") + if git_mode and not commit: + raise RuntimeError(f"This index reads {label} but records no last_indexed_commit.") + try: + tip = commit if git_mode else _git_run("rev-parse", "--verify", "HEAD").strip() + base = _git_run("merge-base", base_ref, tip).strip() or base_ref + except RuntimeError: + base = base_ref # unrelated histories, or no HEAD yet + diff_args = ["-c", "core.quotePath=false", "diff", "-U0", "--no-color", "--no-ext-diff", + "--no-textconv", "--no-prefix", "--relative", "-M", base] + if git_mode: + diff_args.append(commit) + files = _diff_hunks(_git_run(*diff_args, "--")) + if not git_mode: + # Untracked files count only when indexed: the rest (build output, the index's + # own directory when it is not gitignored) is not work under review. + for p in _git_run("ls-files", "--others", "--exclude-standard", "-z").split("\0"): + if p and db.get_file_record(p) is not None: + files.append({"old": None, "new": p, "status": "untracked", "ranges": [], "points": []}) + + changed: list[tuple[str, str, list[dict]]] = [] # (path, status, symbols) + deleted, unindexed, quiet, behind = [], [], [], [] + for f in files: + if f["status"] == "deleted" or not f["new"]: + deleted.append(f["old"]) + continue + path = f["new"] + if db.get_file_record(path) is None: + unindexed.append(path) + continue + locs = db.get_file_symbol_locations(path) + if f["status"] in ("added", "untracked"): + syms = locs + else: + syms = _touched(locs, f["ranges"], f["points"]) + if not git_mode and not _file_unchanged(db, path): + behind.append(path) + if syms: + changed.append((path, f["status"], syms)) + else: + quiet.append(path) + + side = f"indexed commit {commit[:10]}" if git_mode else "the working tree" + total = sum(len(s) for _, _, s in changed) + out = [f"--- CHANGED SYMBOLS: {ref} → {side} (merge base {base[:10]}) ---", + f"{total} symbol(s) in {len(changed)} file(s). Lines are the indexed code's."] + if behind: + out.append(f"⚠️ {len(behind)} file(s) changed on disk since they were indexed; their " + "ranges may be off until the watchdog re-indexes them: " + ", ".join(behind)) + + texts = _texts_by_file() if blast_radius else {} + changed_fqns = {loc["fqn"] for _, _, syms in changed for loc in syms} + shown = blasted = 0 + for path, status, syms in changed: + if limit and shown >= limit: + break + out.append(f"\n{path} ({status})") + if blast_radius: + importers, _ = _import_relations(path, texts) + if importers: + names = sorted(importers) + out.append(f" imported by {len(names)}: " + ", ".join(names[:10]) + + (" …" if len(names) > 10 else "")) + for loc in syms: + if limit and shown >= limit: + break + shown += 1 + out.append(f" L{loc['start_line']}-{loc['end_line']} {loc['kind']} {loc['fqn']}") + if blast_radius and blasted < _BLAST_SYMBOLS: + blasted += 1 + verified, candidate = _caller_evidence(loc["name"], path) + callers = [n.fqn + (" (also changed)" if n.fqn in changed_fqns else "") + for n in verified if n.fqn != loc["fqn"]] + line = " callers: " + (", ".join(callers[:10]) if callers else "none resolved") + if len(callers) > 10: + line += f" … {len(callers) - 10} more" + if candidate: + line += f"; {len(candidate)} unverified (verify_candidate_edges)" + out.append(line) + if shown < total: + out.append(f"\n… {total - shown} more symbol(s); pass limit=0 to list all.") + if blast_radius and blasted < total: + out.append(f"(callers looked up for the first {blasted} symbol(s) only)") + if quiet: + out.append("\nChanged, but no symbol's own lines: " + ", ".join(quiet)) + if deleted: + out.append("Deleted since the merge base (their symbols are not in the index): " + + ", ".join(p for p in deleted if p)) + if unindexed: + out.append(f"Changed but not indexed ({len(unindexed)}): " + ", ".join(unindexed[:20]) + + (" …" if len(unindexed) > 20 else "")) + return "\n".join(out) + + def _snapshot_index(index_dir: str, db_path: str) -> str: """Copy graph.db and every .faiss file in `index_dir` to a backup directory. diff --git a/tests/test_index_state.py b/tests/test_index_state.py index 0f512b6..2c0af13 100644 --- a/tests/test_index_state.py +++ b/tests/test_index_state.py @@ -80,7 +80,7 @@ def test_the_server_holds_one_copy_of_the_index(stub_loading): def test_every_registered_tool_is_async_with_the_same_arguments(): """#53: FastMCP runs a sync tool on the event loop; an async one it awaits.""" tools = M.mcp._tool_manager.list_tools() - assert len(tools) == 14 + assert len(tools) == 17 # 14, plus ADR-050's three symbol-table tools for t in tools: assert t.is_async, t.name plain = getattr(M, t.name) diff --git a/tests/test_symbol_tools.py b/tests/test_symbol_tools.py new file mode 100644 index 0000000..4b84abb --- /dev/null +++ b/tests/test_symbol_tools.py @@ -0,0 +1,539 @@ +"""ADR-050: get_file_outline, get_symbol_source and changed_symbols, model-free. + +Each fixture parses real files with the real adapters and persists them through the +real `CodeDB.upsert_file` (the path `tools/resolution_eval.py` uses), with no chunks, +so nothing is embedded. `MCPServer._db` is pointed at that database, as in +test_what_writes. The diff tests build a temporary git repository and commit inside +the test. +""" +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import anyio +import pytest + +import MCPServer as M +from ast_chunker import parse_file +from db import CodeDB +from source import md5_file + +MOD_PY = '''\ +import os + + +class Greeter: + """Says hi.""" + + def __init__(self, name: str) -> None: + self.name = name + + @staticmethod + def hello( + x: int, + y: int = 2, + ) -> str: + return "hi" + + +def top(a, b): + return a + b + + +async def run(): + pass +''' + +SVC_TS = '''\ +export class Svc extends Base implements Api { + async login(user: string, pw: string): Promise { + return true; + } +} +export const arrow = (a: number) => a + 1; +export function run(): void {} +''' + +EMPTY_PY = 'X = 1\nprint(X)\n' + + +def git(repo, *args) -> str: + done = subprocess.run( + ["git", "-c", "user.email=t@t", "-c", "user.name=t", "-c", "core.autocrlf=false", *args], + cwd=repo, capture_output=True, text=True, stdin=subprocess.DEVNULL, + ) + assert done.returncode == 0, f"git {' '.join(args)} failed: {done.stderr.strip()}" + return done.stdout.strip() + + +def _write(root: Path, rel: str, text: str) -> None: + p = root / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(text, encoding="utf-8") + + +def _index(db: CodeDB, root: Path, rels: list[str], hashes: dict[str, str] | None = None, + texts: dict[str, str] | None = None) -> None: + """Parse each file and upsert it, hashed the way the folder source hashes it.""" + for rel in rels: + text = texts[rel] if texts else (root / rel).read_text(encoding="utf-8") + result = parse_file(rel, text) + h = hashes[rel] if hashes else md5_file(str(root / rel)) + db.upsert_file(rel, h, result.symbols, result.edges, + references=result.references) + + +def _wire(monkeypatch, db: CodeDB, texts: dict[str, str] | None = None) -> None: + monkeypatch.setattr(M, "_db", lambda: db) + monkeypatch.setattr(M, "_ensure_indexes", lambda: None) + monkeypatch.setattr(M, "_texts_by_file", lambda: dict(texts or {})) + + +@pytest.fixture +def indexed(tmp_path, monkeypatch): + """A folder-mode index of three files, the server's cwd at the repo root.""" + repo = tmp_path / "repo" + _write(repo, "pkg/mod.py", MOD_PY) + _write(repo, "pkg/empty.py", EMPTY_PY) + _write(repo, "web/svc.ts", SVC_TS) + _write(repo, "notes/readme.txt", "not indexed\n") + db = CodeDB(str(tmp_path / "graph.db")) + _index(db, repo, ["pkg/mod.py", "pkg/empty.py", "web/svc.ts"]) + db.meta_set("source", "worktree") + monkeypatch.chdir(repo) + _wire(monkeypatch, db) + yield repo, db + db.close() + + +# ── get_file_outline (B-017) ───────────────────────────────────────────────── + +def test_outline_lists_classes_methods_and_signatures_without_bodies(indexed): + out = M.get_file_outline("pkg/mod.py") + + assert "OUTLINE: pkg/mod.py (5 symbol(s))" in out + lines = out.splitlines() + assert "L4-15 class class Greeter" in lines + assert " L7-8 method def __init__(self, name: str) -> None" in lines + assert " L11-15 method def hello(x: int, y: int = 2) -> str" in lines + assert "L18-19 function def top(a, b)" in lines + assert "L22-23 function async def run()" in lines + assert "return a + b" not in out and "self.name = name" not in out + assert "changed on disk" not in out + + +def test_outline_of_a_typescript_file(indexed): + out = M.get_file_outline("web/svc.ts") + assert "L1-5 class class Svc extends Base implements Api" in out + assert " L2-4 method async login(user: string, pw: string): Promise" in out + assert "L6-6 arrow_function const arrow = (a: number)" in out + + +@pytest.mark.parametrize("spelling", [ + "pkg\\mod.py", "./pkg/mod.py", "mod.py", "PKG/MOD.PY", "", +]) +def test_outline_accepts_either_separator_relative_and_absolute_paths(indexed, spelling): + repo, _ = indexed + if spelling == "": + spelling = str(repo / "pkg" / "mod.py") + assert "OUTLINE: pkg/mod.py" in M.get_file_outline(spelling) + + +def test_outline_of_an_indexed_file_with_no_symbols_says_so(indexed): + out = M.get_file_outline("pkg/empty.py") + assert "(0 symbol(s))" in out + assert "indexed but defines no symbols" in out + + +def test_outline_of_an_unknown_file_is_an_error_naming_the_case(indexed): + with pytest.raises(LookupError, match="is not in the index"): + M.get_file_outline("pkg/nope.py") + with pytest.raises(LookupError, match="The file exists"): + M.get_file_outline("notes/readme.txt") + with pytest.raises(LookupError, match="Indexed files with that name"): + M.get_file_outline("other/mod.py") + with pytest.raises(ValueError, match="file is required"): + M.get_file_outline(" ") + + +def test_outline_with_an_ambiguous_tail_lists_the_files(indexed, monkeypatch): + repo, db = indexed + _write(repo, "lib/mod.py", "def other():\n pass\n") + _index(db, repo, ["lib/mod.py"]) + with pytest.raises(ValueError, match="matches 2 indexed files") as exc: + M.get_file_outline("mod.py") + assert "lib/mod.py" in str(exc.value) and "pkg/mod.py" in str(exc.value) + + +def test_outline_flags_a_file_changed_on_disk_since_indexing(indexed): + repo, _ = indexed + (repo / "pkg" / "mod.py").write_text("# new first line\n" + MOD_PY, encoding="utf-8") + assert "changed on disk since it was indexed" in M.get_file_outline("pkg/mod.py") + + +def test_every_tool_fails_on_an_empty_index(tmp_path, monkeypatch): + db = CodeDB(str(tmp_path / "graph.db")) + monkeypatch.chdir(tmp_path) + _wire(monkeypatch, db) + for call in (lambda: M.get_file_outline("a.py"), + lambda: M.get_symbol_source("top"), + lambda: M.changed_symbols("main")): + with pytest.raises(RuntimeError, match="No index here"): + call() + db.close() + + +@pytest.mark.parametrize("text, path, sig", [ + ("[Serializable]\n public partial class Order : IOrder {\n }", "Order.cs", + "public partial class Order : IOrder"), + ("int f(int x) const;", "w.cpp", "int f(int x) const"), + ("template T id(T v) { return v; }", "w.cpp", "template T id(T v)"), + ("type Alias = string;", "a.ts", "type Alias = string"), + ("def f(x={}): # trailing\n return x", "a.py", "def f(x={})"), + ("class A(Base, metaclass=M):\n x: int = 3\n", "a.py", "class A(Base, metaclass=M)"), +]) +def test_signature_is_the_declaration_up_to_the_body(text, path, sig): + assert M._signature(text, path) == sig + + +# ── get_symbol_source (B-016) ──────────────────────────────────────────────── + +def test_symbol_source_returns_just_that_span_with_its_lines(indexed): + out = M.get_symbol_source("top") + assert out.splitlines() == [ + "--- pkg/mod.py::top [function] pkg/mod.py:18-19 ---", + " 18 def top(a, b):", + " 19 return a + b", + ] + + +def test_symbol_source_of_a_class_is_its_real_text_not_the_skeleton(indexed): + out = M.get_symbol_source("Greeter") + assert 'return "hi"' in out and "self.name = name" in out + assert " ..." not in out + + +def test_symbol_source_lists_candidates_for_an_ambiguous_name(indexed): + out = M.get_symbol_source("run") + assert "'run' is ambiguous: 2 symbols match" in out + assert "pkg/mod.py::run [function] pkg/mod.py:22-23" in out + assert "web/svc.ts::run [function] web/svc.ts:7-7" in out + assert "pass" not in out.split("\n", 1)[1] # no body was guessed + + narrowed = M.get_symbol_source("run", file="web\\svc.ts") + assert narrowed.splitlines()[1] == " 7 export function run(): void {}" + assert "Greeter.hello" not in M.get_symbol_source("Greeter.hello").split("\n", 1)[1] + assert "def hello(" in M.get_symbol_source("Greeter.hello") + assert "def hello(" in M.get_symbol_source("pkg/mod.py::Greeter.hello") + + +def test_unknown_symbol_is_an_error(indexed): + with pytest.raises(LookupError, match="No symbol 'nosuch'"): + M.get_symbol_source("nosuch") + with pytest.raises(LookupError, match="in web/svc.ts"): + M.get_symbol_source("top", file="web/svc.ts") + + +def test_an_edit_elsewhere_in_the_file_keeps_the_span_and_says_so(indexed): + repo, _ = indexed + (repo / "pkg" / "mod.py").write_text(MOD_PY.replace("pass", "return 1"), encoding="utf-8") + out = M.get_symbol_source("top") + assert "has changed since it was indexed, but this symbol's lines have not" in out + assert " 18 def top(a, b):" in out + + +def test_a_moved_symbol_is_served_from_its_new_lines(indexed): + repo, _ = indexed + (repo / "pkg" / "mod.py").write_text("# one\n# two\n" + MOD_PY, encoding="utf-8") + out = M.get_symbol_source("top") + assert "indexed at lines 18-19, now at 20-21" in out + assert " 20 def top(a, b):" in out and " 21 return a + b" in out + + +def test_a_symbol_changed_since_indexing_is_refused_not_served_stale(indexed): + repo, _ = indexed + (repo / "pkg" / "mod.py").write_text(MOD_PY.replace("return a + b", "return a - b"), + encoding="utf-8") + with pytest.raises(RuntimeError, match="has changed since it was indexed") as exc: + M.get_symbol_source("top") + assert "a + b" not in str(exc.value) + + +def test_a_deleted_file_is_an_error(indexed): + repo, _ = indexed + (repo / "pkg" / "mod.py").unlink() + with pytest.raises(RuntimeError, match="could not be read"): + M.get_symbol_source("top") + + +def test_git_mode_reads_the_indexed_commit_not_the_working_tree(tmp_path, monkeypatch): + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init", "-q", "-b", "main") + _write(repo, "pkg/mod.py", MOD_PY) + git(repo, "add", ".") + git(repo, "commit", "-q", "-m", "init") + commit = git(repo, "rev-parse", "HEAD") + db = CodeDB(str(tmp_path / "graph.db")) + _index(db, repo, ["pkg/mod.py"], hashes={"pkg/mod.py": git(repo, "rev-parse", "HEAD:pkg/mod.py")}) + db.meta_set("source", "git:main") + db.meta_set("last_indexed_commit", commit) + monkeypatch.chdir(repo) + _wire(monkeypatch, db) + # The working tree moves on; the index is the commit. + (repo / "pkg" / "mod.py").write_text(MOD_PY.replace("return a + b", "return a * b"), + encoding="utf-8") + + out = M.get_symbol_source("top") + assert f"(from commit {commit[:10]} (git:main): the index's source, not your working tree)" in out + assert " 19 return a + b" in out + assert "changed" not in out + assert f"as of commit {commit[:10]}" in M.get_file_outline("pkg/mod.py") + db.close() + + +# ── changed_symbols (B-020) ────────────────────────────────────────────────── + +LIB_V1 = '''\ +def alpha(x): + y = x + 1 + return y + + +def beta(x): + return x * 2 + + +def gamma(x): + a = 1 + b = 2 + return a + b + x + + +class K: + def m(self): + return 1 + + def n(self): + return 2 +''' + +# beta's body changed, one line deleted inside gamma, one blank line deleted between +# alpha and beta, K.m's body changed; alpha and K.n untouched. +LIB_V2 = '''\ +def alpha(x): + y = x + 1 + return y + +def beta(x): + return x * 3 + + +def gamma(x): + a = 1 + return a + x + + +class K: + def m(self): + return 10 + + def n(self): + return 2 +''' + +USE_PY = '''\ +from pkg.lib import beta + + +def user(): + return beta(1) +''' + + +@pytest.fixture +def diffed(tmp_path, monkeypatch): + """main at v1; a branch commits v2, adds pkg/new.py, deletes pkg/gone.py and edits + a README. A folder-mode index of the branch's working tree.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init", "-q", "-b", "main") + _write(repo, "pkg/lib.py", LIB_V1) + _write(repo, "pkg/use.py", USE_PY) + _write(repo, "pkg/gone.py", "def gone():\n pass\n") + _write(repo, "README.md", "v1\n") + git(repo, "add", ".") + git(repo, "commit", "-q", "-m", "v1") + git(repo, "checkout", "-q", "-b", "work") + _write(repo, "pkg/lib.py", LIB_V2) + _write(repo, "pkg/new.py", "def fresh():\n return 0\n") + (repo / "pkg" / "gone.py").unlink() + _write(repo, "README.md", "v2\n") + git(repo, "add", "-A") + git(repo, "commit", "-q", "-m", "v2") + + rels = ["pkg/lib.py", "pkg/use.py", "pkg/new.py"] + db = CodeDB(str(tmp_path / "graph.db")) + _index(db, repo, rels) + from call_resolver import resolve_call_edges + from import_resolver import resolve_python_imports + resolve_python_imports(db) + resolve_call_edges(db) + db.meta_set("source", "worktree") + monkeypatch.chdir(repo) + _wire(monkeypatch, db, {r: (repo / r).read_text(encoding="utf-8") for r in rels}) + yield repo, db + db.close() + + +def _symbols(out: str) -> set[str]: + return {line.split()[-1] for line in out.splitlines() if line.startswith(" L")} + + +def test_changed_symbols_reports_the_changed_function_not_the_untouched_one(diffed): + out = M.changed_symbols("main") + + assert _symbols(out) == {"pkg/lib.py::beta", "pkg/lib.py::gamma", "pkg/lib.py::K.m", + "pkg/new.py::fresh"} + assert "pkg/lib.py (modified)" in out and "pkg/new.py (added)" in out + assert "Deleted since the merge base (their symbols are not in the index): pkg/gone.py" in out + assert "Changed but not indexed (1): README.md" in out + assert "on disk since" not in out + + +def test_changed_symbols_diffs_from_the_merge_base(diffed): + """main moving on after the branch left it does not show up as this branch's change.""" + repo, _ = diffed + git(repo, "checkout", "-q", "main") + _write(repo, "pkg/lib.py", LIB_V1.replace("return 2", "return 22")) + git(repo, "commit", "-q", "-am", "main moves") + git(repo, "checkout", "-q", "work") + assert "pkg/lib.py::K.n" not in M.changed_symbols("main") + + +def test_changed_symbols_includes_uncommitted_and_untracked_work(diffed): + repo, db = diffed + _write(repo, "pkg/lib.py", LIB_V2.replace("y = x + 1", "y = x + 100")) + _write(repo, "pkg/extra.py", "def later():\n pass\n") + _write(repo, "build/out.txt", "not indexed, not reported\n") + _index(db, repo, ["pkg/extra.py"]) + db.delete_file("pkg/lib.py") + _index(db, repo, ["pkg/lib.py"]) + + out = M.changed_symbols("HEAD") + assert _symbols(out) == {"pkg/lib.py::alpha", "pkg/extra.py::later"} + assert "pkg/extra.py (untracked)" in out + assert "build/out.txt" not in out + + +def test_changed_symbols_flags_files_the_index_has_not_caught_up_with(diffed): + repo, _ = diffed + _write(repo, "pkg/lib.py", LIB_V2.replace("return x * 3", "return x * 4")) + out = M.changed_symbols("main") + assert "1 file(s) changed on disk since they were indexed" in out + assert "pkg/lib.py" in out.split("on disk since", 1)[1].splitlines()[0] + + +def test_changed_symbols_with_blast_radius_names_the_callers(diffed): + out = M.changed_symbols("main", blast_radius=True) + beta = out.split("pkg/lib.py::beta", 1)[1].splitlines()[1] + assert "callers:" in beta and "pkg/use.py::user" in beta + assert "imported by 1: pkg/use.py" in out + + +def test_changed_symbols_limit_caps_the_listing_not_the_count(diffed): + out = M.changed_symbols("main", limit=1) + assert "4 symbol(s) in 2 file(s)" in out + assert len(_symbols(out)) == 1 + assert "3 more symbol(s); pass limit=0" in out + + +def test_a_bad_ref_fails_clearly(diffed): + with pytest.raises(ValueError, match="does not name a commit"): + M.changed_symbols("no-such-branch") + with pytest.raises(ValueError, match="ref is required"): + M.changed_symbols("") + + +@pytest.mark.parametrize("ref", ["--output=x", "-p", "main --output=x", "ma\nin"]) +def test_an_option_like_or_malformed_ref_is_rejected_before_git_sees_it(diffed, ref): + repo, _ = diffed + with pytest.raises(ValueError, match="is not a git ref"): + M.changed_symbols(ref) + assert not (repo / "x").exists() + + +def test_the_ref_is_checked_before_the_index_or_git(tmp_path, monkeypatch): + db = CodeDB(str(tmp_path / "graph.db")) + monkeypatch.chdir(tmp_path) + _wire(monkeypatch, db) + with pytest.raises(ValueError, match="is not a git ref"): + M.changed_symbols("--output=x") + db.close() + + +def test_changed_symbols_outside_a_git_repo_fails_clearly(indexed, monkeypatch): + monkeypatch.setenv("GIT_CEILING_DIRECTORIES", str(Path.cwd().parent)) + with pytest.raises(RuntimeError, match="needs a git repository"): + M.changed_symbols("main") + + +def test_git_mode_diffs_against_the_indexed_commit(diffed): + repo, db = diffed + db.meta_set("source", "git:work") + db.meta_set("last_indexed_commit", git(repo, "rev-parse", "work")) + _write(repo, "pkg/lib.py", LIB_V2.replace("y = x + 1", "y = x + 100")) # uncommitted + out = M.changed_symbols("main") + assert "→ indexed commit" in out + assert "pkg/lib.py::alpha" not in _symbols(out) + assert "pkg/lib.py::beta" in _symbols(out) + + +def test_hunk_parsing_ignores_deleted_lines_that_look_like_headers(): + diff = ( + "diff --git a.sql a.sql\n--- a.sql\n+++ a.sql\n" + "@@ -3,2 +2,0 @@\n--- a comment\n-x\n" + "@@ -9 +8,3 @@\n-y\n+++ not a header\n+b\n+c\n" + ) + [f] = M._diff_hunks(diff) + assert f["new"] == "a.sql" and f["points"] == [2] and f["ranges"] == [(8, 10)] + + +# ── through the MCP layer ──────────────────────────────────────────────────── + +def _call(name: str, args: dict): + from mcp.shared.memory import create_connected_server_and_client_session + + async def main(): + async with create_connected_server_and_client_session(M.mcp._mcp_server) as client: + return await client.call_tool(name, args) + return anyio.run(main) + + +def test_the_tools_are_registered_as_read_tools(): + names = {t.name for t in M.mcp._tool_manager.list_tools()} + assert {"get_file_outline", "get_symbol_source", "changed_symbols"} <= names + for name in ("get_file_outline", "get_symbol_source", "changed_symbols"): + doc = M.mcp._tool_manager.get_tool(name).description + assert len(doc) < 800, name + + +@pytest.mark.parametrize("name, args", [ + ("get_file_outline", {"file": "pkg/nope.py"}), + ("get_symbol_source", {"symbol": "nosuch"}), + ("changed_symbols", {"ref": "--output=x"}), + ("get_file_outline", {}), +]) +def test_failures_reach_the_client_as_errors(indexed, name, args): + result = _call(name, args) + assert result.isError, result + + +def test_success_reaches_the_client_as_a_result(indexed): + result = _call("get_file_outline", {"file": "pkg/mod.py"}) + assert not result.isError + assert "class Greeter" in result.content[0].text + result = _call("get_symbol_source", {"symbol": "run"}) + assert not result.isError and "ambiguous" in result.content[0].text From 0f11f68817c9c5d313f2b6efa9706a2f842b53a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:38:19 +0000 Subject: [PATCH 4/5] List the lookup tools and record ADR-050's build Add get_file_outline, get_symbol_source and changed_symbols to the tool lists in src/CLAUDE.md and README.md (17 tools). The ADR's log records what the adapters store, the block-wise verification it led to, the Inspector findings, test counts and open questions. Co-Authored-By: Claude [ADR-050 src/CLAUDE.md] --- README.md | 5 +- docs/adr/ADR-050-symbol-table-read-tools.md | 111 +++++++++++++++----- src/CLAUDE.md | 11 +- 3 files changed, 97 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 2b1153f..6c4dc89 100644 --- a/README.md +++ b/README.md @@ -214,6 +214,9 @@ first `python` on PATH): a CPU-only interpreter puts the models on the CPU. |------|----------|-------------| | `semantic_code_search` | Search | FAISS + RRF search across all three tiers | | `find_similar_code` | Search | Find duplicates and callers of a code snippet | +| `get_file_outline` | Lookup | A file's symbols: kind, signature and line range, no bodies | +| `get_symbol_source` | Lookup | One symbol's source, checked against what was indexed; lists candidates when the name is ambiguous | +| `changed_symbols` | Lookup | The symbols a branch changed since a git ref, optionally with their callers and importers | | `analyze_blast_radius` | Impact | Map callers, dependents, and parallel implementations of a symbol | | `detect_pattern_violations` | Impact | Find files that should follow a pattern but don't | | `trace_data_flow` | Tracing | Full lifecycle of a data symbol (definitions → producers → consumers) | @@ -278,7 +281,7 @@ codebase-indexer/ ├── tests/ ├── tools/ # Eval + CI harnesses (coir_eval, conformance_eval, …) └── src/ - ├── MCPServer.py # MCP server entry point — all 14 tools, and the watchdog + ├── MCPServer.py # MCP server entry point — all 17 tools, and the watchdog ├── config.py # Locates and parses the per-repo indexer.toml ├── index_location.py # Where the index lives: .code-index or /code-index (ADR-042) ├── index_lock.py # One writer and one watchdog per index (ADR-038) diff --git a/docs/adr/ADR-050-symbol-table-read-tools.md b/docs/adr/ADR-050-symbol-table-read-tools.md index 6cd7ae5..156dd05 100644 --- a/docs/adr/ADR-050-symbol-table-read-tools.md +++ b/docs/adr/ADR-050-symbol-table-read-tools.md @@ -69,22 +69,24 @@ over new read-only `CodeDB` methods. They do no retrieval and load no model. (`index_meta.last_indexed_commit`). That is the exact text the symbol table describes. The answer names the commit and says it is not the caller's working tree, the same caveat the server instructions give. -- **Staleness: re-read and verify, refuse when it can't be verified.** The stored `text` is the - witness. The span read from the source is compared with it (both stripped, which absorbs the - first-line indentation): - 1. It matches → served with exact line numbers. If the file's hash differs from the indexed one, the - answer says the file changed elsewhere but this symbol's span did not. - 2. It doesn't, but the stored text occurs exactly once elsewhere in the current file → served from - its new position, and the answer says it moved from the indexed lines. - 3. Neither → **refused with an error**: the symbol changed since indexing, here are the indexed - lines, Read the file. Serving the re-read span here would show whatever now sits on those lines, - which may be a different function. That is the backlog's trap in a new form. - The hash is the cheap "file changed" signal; the text comparison decides what is served. So a change - to how `content_hash` is computed (ADR-051 is touching keys on a sibling branch) can at worst add a - spurious "file changed" note, never serve wrong text. -- **Skeleton texts.** A class whose stored `text` is an ADR-034 skeleton (stubbed bodies) can't be - matched line for line. A class span is verified by its declaration line alone, and its hash. See the - Implementation Log for what the adapters actually store. +- **Staleness: re-read and verify, refuse when it can't be verified.** + - If the file still has its indexed hash, the indexed span is exact and is served. + - Otherwise the stored `text` is the witness. It is split into blocks, at ADR-034 skeleton stubs + (` ...`, which replace a class's member bodies) and at blank lines (a merged overload set is its + parts reordered). Each block is stripped, which absorbs the first line's missing indentation and an + `export` the node doesn't include. Then: + 1. Every block is still inside the indexed span → served with its line numbers, and a note that the + file changed elsewhere but this symbol's lines did not. + 2. Exactly one same-length span starting where the first block now occurs holds every block → + served from there, with a note that it moved from the indexed lines and that other line numbers + in the file may have shifted. + 3. Neither → **refused with an error**: the symbol changed since indexing; here are the indexed + lines; Read the file. Serving the re-read span here would show whatever now sits on those lines, + which may be a different function. That is the backlog's trap in a new form. + - The hash is the cheap "file changed" signal; the text comparison decides what is served. So a change + to how `content_hash` is computed (ADR-051 is touching keys on a sibling branch) can at worst add a + spurious "file changed" note, never serve wrong text. + - Served text is always the real source. A class comes back with its member bodies, not its skeleton. - Spans longer than 400 lines are cut, with a note pointing at `get_file_outline`. ### 3. `changed_symbols(ref, blast_radius=False, limit=100)` (B-020) @@ -103,9 +105,10 @@ over new read-only `CodeDB` methods. They do no retrieval and load no model. `` is an ancestor (`HEAD~3`) the merge base is ``, so nothing changes for that case. - Git mode: the indexed side is `index_meta.last_indexed_commit`. The diff is commit to commit, and its new-side line numbers are exactly the index's. - - Folder mode: the indexed side is the working tree (`git diff `, plus untracked indexed files, - whose symbols are all new), and the merge base is taken against `HEAD`. A file whose disk hash no - longer matches the index is flagged: its ranges may be off until the watchdog catches up. + - Folder mode: the indexed side is the working tree (`git diff `), and the merge base is taken + against `HEAD`. Untracked files are added only when the index holds them; their symbols are all new. + A file whose disk hash no longer matches the index is flagged: its ranges may be off until the + watchdog catches up. - **Intersection.** `git diff -U0` gives the new-side lines each hunk added or changed. A pure deletion is a point between two lines. A symbol is reported when a changed line falls in its *own* lines: its span minus its nested symbols' spans. A one-line edit inside a method reports the method, not the method @@ -120,8 +123,8 @@ over new read-only `CodeDB` methods. They do no retrieval and load no model. ### Shared -- New `CodeDB` methods, appended at the end of the class and read-only: `get_file_record`, - `find_file_paths`, `get_file_symbol_locations`, `find_symbol_locations`. +- New `CodeDB` methods, appended at the end of the class and read-only: `has_files`, + `get_file_record`, `find_file_paths`, `get_file_symbol_locations`, `find_symbol_locations`. - An index with no files at all (a server started where nothing was built) fails each tool with "no index here", not "file not indexed". - Every failure is raised, so the MCP client sees `isError: true`, as `reindex` already does. @@ -163,14 +166,66 @@ over new read-only `CodeDB` methods. They do no retrieval and load no model. > Updated during development. Record deviations from the design, surprises, and decisions made in the moment. -- [ ] `CodeDB` read methods -- [ ] `get_file_outline` + tests -- [ ] `get_symbol_source` + tests (unique, ambiguous, changed on disk, git mode) -- [ ] `changed_symbols` + tests (temp git repo: changed vs untouched function, bad ref, option-like ref) -- [ ] Each tool's failure path is `isError` through the MCP layer (pytest, FastMCP client session) -- [ ] MCP Inspector `tools/list` and one good and one bad call per tool -- [ ] Tool lists in `src/CLAUDE.md` and `README.md` +- [x] `CodeDB` read methods (`src/db.py`, appended to the class) +- [x] `get_file_outline` + tests +- [x] `get_symbol_source` + tests (unique, ambiguous, edited elsewhere, moved, changed, deleted, git mode) +- [x] `changed_symbols` + tests (temp git repo: changed vs untouched function, merge base, uncommitted and + untracked work, index behind disk, blast radius, bad ref, option-like refs, outside a repo, git mode) +- [x] Each tool's failure path is `isError` through the MCP layer (pytest, in-memory FastMCP client session) +- [x] MCP Inspector `tools/list` and one good and one bad call per tool +- [x] Tool lists in `src/CLAUDE.md` and `README.md` +- [ ] Once ADR-049 merges: confirm the three tools list `readOnlyHint` and their output goes through its + redaction. They are registered with the plain `@_tool()`, so both should arrive with no change here. **Open questions for @edb:** +1. **A class whose method body changed is refused.** Its span grew or shrank, so no same-length span + holds its skeleton. The answer is honest, but for a class it is coarse. Accept it, or verify a class + by its skeleton blocks in order and extend its end to the next sibling? +2. **Server instructions.** Suggested sentence for ADR-049's `_server_instructions` (not edited here): + "For what a file contains, call `get_file_outline` before Reading it; for one function, call + `get_symbol_source`." +3. **Gitignored but indexed files** are invisible to `changed_symbols` in folder mode: the indexer + follows `scan_policy`, not `.gitignore`, and `git diff` follows git. Rare in practice; worth a note? +4. **No raw-diff mode.** `changed_symbols` always diffs from the merge base. A `--no-merge-base` switch + is easy if review against a moving tip is ever wanted. + **Notes:** + + **What the adapters store, checked before building §2.** Python, TS, C# and C++ symbols +store the node's text, which starts at the node's column and leaves out a TS `export` and a Python +decorator (`start_line` is the `def` line). Class symbols store an ADR-034 skeleton, and merged +overload sets join their parts in implementation-first order. So "the stored text occurs in the span" +is false for every class. The verification is block-wise (§2) rather than whole-text. + + **C++ declaration and definition share an FQN** (`ns::W::f(int)` at line 4 in the class +and line 6 out of line). `symbols` keeps one row per FQN, and `symbol_locations` keeps one location per +(symbol, file). The outline shows one of the two, the declaration. That is the existing schema, not +something this ADR changes. + + **Inspector caught noise pytest didn't.** Against a scratch repo whose `.code-index/` was +not gitignored, `changed_symbols` listed `graph.db`, `-shm` and `-wal` as "changed but not indexed". +Untracked files now count only when the index holds them (test added). It also showed that the ref was +checked after the index, so an option-like ref against an empty directory said "No index here". The +shape check now runs first. + + **Tests.** Stack: Linux container, Python 3.12.3, git 2.43.0, mcp 1.30.0, no GPU and no +model (HuggingFace offline). The suite on `871b55c` was 654 passed, 48 failed, 2 skipped; on this branch +it is 702 passed, 48 failed, 2 skipped. The 48 are the same tests on both: each loads `BAAI/bge-code-v1` +or its tokenizer, which can't be downloaded here. `tests/test_symbol_tools.py` adds 48 tests. +`tests/test_index_state.py`'s registered-tool count moves from 14 to 17, the one line changed outside +this ADR's files. flake8, `pytest --co`, `conformance_eval --check-baseline`, `resolution_eval +--check-baseline` and `l5x_fixture_teeth --check` all pass. + + **MCP Inspector 2.8.0** (`--cli`, same stack): +- `tools/list --strict`: exit 0, 17 tools. Description lengths: `get_file_outline` 587, + `get_symbol_source` 598, `changed_symbols` 637 characters. No tool on this branch declares + `readOnlyHint`: ADR-049 adds it inside `_tool`. +- From an empty directory, each tool fails with `isError: true` and "No index here". A missing required + argument and an option-like ref (`--output=x`) also fail with `isError: true`, the ref before any + index or git work. +- Against a scratch git repo with a SQLite-only index (parse and upsert, no embedder, no FAISS vectors): + `get_file_outline pkg/mod.py`, `get_symbol_source K.m` and `changed_symbols HEAD~1` succeed. An + unknown file, an unknown symbol, a missing `symbol`, `ref=no-such-branch` and `ref=--output=x` each + return `isError: true` with a message naming the case. The Inspector CLI won't send an empty-string + argument, so `symbol=""` is covered in pytest instead. diff --git a/src/CLAUDE.md b/src/CLAUDE.md index f0862c0..7065113 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -144,9 +144,10 @@ Source files ### MCP Tools (MCPServer.py) -Fourteen AI-facing tools grouped by intent: +Seventeen AI-facing tools grouped by intent: - **Search**: `semantic_code_search`, `find_similar_code` +- **Lookup**: `get_file_outline`, `get_symbol_source`, `changed_symbols` - **Impact**: `analyze_blast_radius`, `detect_pattern_violations` - **Tracing**: `trace_data_flow`, `investigate_architecture`, `what_writes` - **Discovery**: `find_test_coverage`, `find_dead_code`, `find_unabstracted_collection_reads`, `map_module_communities`, `verify_candidate_edges` @@ -165,6 +166,14 @@ candidate (name-based / unresolved) call edges. `index_status` (ADR-025) reports index freshness — `last_verified_at`, `last_indexed_commit` vs HEAD, and files whose content changed within a window — for agents and downstream context hubs. +The three **Lookup** tools (ADR-050) answer straight from the symbol table, like +`what_writes`: no retrieval and no model. `get_symbol_source` never serves the stored +copy of a symbol. It reads the file through `source.py` from what the index was built +from (the indexed commit in git mode) and checks it against the stored text: it serves +the symbol in place, serves it from its new lines when it moved, or refuses when it +changed. `changed_symbols` diffs with git from the ref's merge base; the ref is +validated before git sees it, and every git call has a timeout. + `map_module_communities` (ADR-006) is the whole-graph structural view: it reads the SQLite edge graph, runs Louvain community detection + betweenness centrality over it (`src/graph_analytics.py`, pure NetworkX), and renders a community map + god-object From 1ca7b5ec0ca48168e2737764f4f26ac958dca35d Mon Sep 17 00:00:00 2001 From: Ethan Blauw Date: Mon, 2 Nov 2026 11:22:41 -0600 Subject: [PATCH 5/5] ADR-050: apply @edb's answers to the open questions; status accepted Co-Authored-By: Claude Opus 5.5 [ADR-050 src/MCPServer.py] --- docs/adr/ADR-050-symbol-table-read-tools.md | 18 ++++++++++++++++-- src/MCPServer.py | 9 ++++++--- tests/test_safety_tool_surface.py | 3 ++- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/docs/adr/ADR-050-symbol-table-read-tools.md b/docs/adr/ADR-050-symbol-table-read-tools.md index 156dd05..eede7e5 100644 --- a/docs/adr/ADR-050-symbol-table-read-tools.md +++ b/docs/adr/ADR-050-symbol-table-read-tools.md @@ -1,6 +1,6 @@ # ADR-050: Answer from the Symbol Table: File Outline, One Symbol's Source, Changed Symbols -**Status:** proposed +**Status:** accepted **Date:** 2026-10-20 **Branch:** `feature/adr-050-symbol-tools` **Reviewer:** @edb @@ -177,7 +177,8 @@ over new read-only `CodeDB` methods. They do no retrieval and load no model. - [ ] Once ADR-049 merges: confirm the three tools list `readOnlyHint` and their output goes through its redaction. They are registered with the plain `@_tool()`, so both should arrive with no change here. -**Open questions for @edb:** +**Open questions for @edb** (answered 2026-11-02: "all recommended", after a walk-through with an +example for each): 1. **A class whose method body changed is refused.** Its span grew or shrank, so no same-length span holds its skeleton. The answer is honest, but for a class it is coarse. Accept it, or verify a class @@ -190,6 +191,19 @@ over new read-only `CodeDB` methods. They do no retrieval and load no model. 4. **No raw-diff mode.** `changed_symbols` always diffs from the merge base. A `--no-merge-base` switch is easy if review against a moving tip is ever wanted. +**Answers, 2026-11-02:** +1. **Accepted as built.** A class with a changed method is refused; ask for the method or Read the file. + Serving the class would include the changed method, which is the stale-code trap. +2. **Accepted, with a clause.** The routing line in `_server_instructions` now also says "Call + `get_file_outline` before Reading a whole file, and `get_symbol_source` for one function (indexed + version)." Shortened from the proposed line, and the routing line's "and for the files your branch + changed" dropped (both branches already say to Read those), to keep the instructions under their + 1,000-character test budget. Both live repos index a git ref, so the + version clause matters (B-066's survey). This also closes ADR-049's open item 4. +3. **A note, nothing more.** `changed_symbols`' description says files git ignores are not diffed. + Folder mode only; a git-mode index holds only tracked files. +4. **No.** The merge base is what a reviewer means; add the switch only when someone asks. + **Notes:** **What the adapters store, checked before building §2.** Python, TS, C# and C++ symbols diff --git a/src/MCPServer.py b/src/MCPServer.py index c9f6b0e..44dad8a 100644 --- a/src/MCPServer.py +++ b/src/MCPServer.py @@ -68,7 +68,10 @@ def _server_instructions(repo_path: str | None = None) -> str: routing = ("For how the rest of the codebase works, where a symbol is used, what writes a " "field, or blast radius, call `semantic_code_search`, `trace_data_flow`, " "`what_writes` or `analyze_blast_radius` before Grep; use Grep for exact " - "strings and for the files your branch changed. ") + "strings. " + # ADR-050 open question 2: answer from the symbol table before a full Read. + "Call `get_file_outline` before Reading a whole file, and `get_symbol_source` " + "for one function (indexed version). ") if ref is not None: # ADR-042 §5: the index is a commit, read from git, not any folder. from index_location import ref_display @@ -2849,8 +2852,8 @@ def changed_symbols(ref: str, blast_radius: bool = False, limit: int = 100) -> s at its merge base with the indexed code, so 'main' means "since this branch left main". blast_radius=True also lists each changed symbol's resolved callers (and a count of unverified ones) and each changed file's importers. Symbols deleted since - `ref` are not in the index; deleted files are listed by name. `limit` caps the - symbols listed (0 = all). + `ref` are not in the index; deleted files are listed by name. Files git ignores are + not diffed, even if the index holds them. `limit` caps the symbols listed (0 = all). """ print(f"\n[MCP] changed_symbols: ref='{ref}' blast_radius={blast_radius}") _check_ref_shape(ref) diff --git a/tests/test_safety_tool_surface.py b/tests/test_safety_tool_surface.py index c0c6318..8c117c3 100644 --- a/tests/test_safety_tool_surface.py +++ b/tests/test_safety_tool_surface.py @@ -104,5 +104,6 @@ def test_both_instruction_branches_route_to_the_index_before_grep(tmp_path, monk assert ("read straight from git" in text) is git_mode assert "before Grep" in text and "exact strings" in text registered = {t.name for t in _registered()} - for name in ("semantic_code_search", "trace_data_flow", "what_writes", "analyze_blast_radius"): + for name in ("semantic_code_search", "trace_data_flow", "what_writes", "analyze_blast_radius", + "get_file_outline", "get_symbol_source"): assert f"`{name}`" in text and name in registered # names only tools that exist