From 16a8cb5c931d8ed1fd4ed84dfd526ad91680433e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:24:43 +0000 Subject: [PATCH 1/7] Add ADR-053: communities hold only repo nodes, data flow has per-language rules Co-Authored-By: Claude [ADR-053] --- .../adr/ADR-053-graph-tools-tell-the-truth.md | 201 ++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 docs/adr/ADR-053-graph-tools-tell-the-truth.md diff --git a/docs/adr/ADR-053-graph-tools-tell-the-truth.md b/docs/adr/ADR-053-graph-tools-tell-the-truth.md new file mode 100644 index 0000000..b109cb7 --- /dev/null +++ b/docs/adr/ADR-053-graph-tools-tell-the-truth.md @@ -0,0 +1,201 @@ +# ADR-053: Communities Hold Only Repo Nodes, and Data Flow Has Per-Language Rules + +**Status:** proposed +**Date:** 2026-10-20 +**Branch:** `feature/adr-053-graph-tool-accuracy` +**Reviewer:** @edb +**Backlog:** [B-043](../backlog.md#b-043) (communities filled with names from outside the repo) · +[B-042](../backlog.md#b-042) (`trace_data_flow` only understands Firestore and Next.js) +**Depends on:** [ADR-044](./ADR-044-python-imports-captured-and-resolved.md) — Python IMPORTS edges +carry a `resolved_target`, which is what lets the B-043 filter keep import links. +**Depended on by:** none yet. + +## Context + +Two exploratory tools give wrong answers about this repo. + +**B-043: `map_module_communities`.** `CodeDB.get_graph_edges()` returns +`COALESCE(resolved_target, target)`, so an edge that never resolved points at its raw name. Every +unresolved `append()`, `bool()`, `ArgumentParser()`, `import os` and `class X(Protocol)` became a +graph node, and then a community member and a betweenness hub. The backlog's caveat (no Python +import resolves, so a filter would drop every import link) no longer holds: ADR-044 resolves them. + +Measured on this repo at `871b55c`, graph built SQLite-only (scan → `parse_file` → `upsert_file` → +`resolve_python_imports` → `resolve_call_edges`, the path `tools/resolution_eval.py` uses; no +embedder). 172 files, tests and conformance fixtures included, as the indexer scans them. + +| kind | resolved | unresolved | of the unresolved | +|---|---|---|---| +| CALLS | 2,664 | 4,572 | none is a repo node | +| IMPORTS | 297 (to files) | 596 | 578 `external = 1`, 6 broken relative imports in fixtures, 12 C#/C++ unknown | +| EXTENDS | 0 | 27 | bare names; 9 name an in-repo type | +| IMPLEMENTS | 0 | 4 | bare names; all 4 name an in-repo type | +| OWNS | 0 | 480 | the raw target is already a symbol FQN | + +So "resolved" is the wrong test: OWNS targets never get a `resolved_target` and are all real, and +heritage edges never get one either. **615 community members were not a symbol or file of the +repo.** + +**B-042: `trace_data_flow`.** Its layers are path tests (`firebase/admin`, `functions/src`, +`page.tsx`), a definition is `export interface|type|class`, and a producer is a Firestore +`.set/.add/.update` in an admin or functions file. On this repo `trace_data_flow("chunk_summaries")` +found no definition (`CREATE TABLE chunk_summaries` in `db.py`) and no producer, and listed the +writer `cache_summaries` (`INSERT OR IGNORE INTO chunk_summaries`) as a consumer. On Apps Script it +finds nothing either: there is no `export`, and writes are `setValue(s)`, `appendRow`, +`setProperty`. + +## Decision + +### 1. A graph edge must end at a node the repo defines (B-043) + +`get_graph_edges()` returns an edge only when its effective target (`resolved_target`, else +`target`) is a symbol FQN or an indexed file path. Everything else (an unresolved call, an +external import, a broken relative import) is dropped, for every kind, not only CALLS. + +**EXTENDS/IMPLEMENTS are bound at read time**, because nothing else binds them: a bare base name +binds to the type of that name (class, interface, struct, record, enum, type alias) in the +subclass's own file, else to the repo's only type of that name. `Protocol`, `NamedTuple`, +`Exception` have no in-repo type and drop. An ambiguous name across files (`Animal` in three +fixtures, with none in the subclass's file) drops too: the prefer-unknown rule of ADR-021. On this +repo 13 of 31 heritage edges bind, including `TypeScriptAdapter → _WebAdapter`. + +The filter lives in `get_graph_edges()`, so `graph_viz`'s DSM, its other reader, gets the same +graph. + +### 2. Data-flow rules are per language and live on the adapters (B-042) + +Each adapter that has rules gets `data_flow_rules()`, returning +`{"definition": [...], "producer": [...], "file_marker": [...]}`. Each rule is a regex in which +`{name}` stands for the traced name (and its PascalCase variant, as today). `file_marker`, when +present, gates the whole set: the rules apply only to files whose text matches one marker. + +- **Python** (`python_adapter.py`): + - definition: `CREATE [TEMP] TABLE|VIEW [IF NOT EXISTS] name`, `class Name` (dataclasses + included), a module-level `NAME = …` or `NAME: T = …`; + - producer: `INSERT [OR …] INTO name`, `REPLACE INTO name`, `UPDATE name … SET`, + `DELETE FROM name`; an attribute or subscript write (`self.name = …`, `x.name[k] = …`, + `name[k] = …`, `+=` too, never `==`); a mutating call (`name.append|extend|insert|update|add| + setdefault|pop|remove|discard|clear(`). + - SQL keywords match **upper case only**, the house SQL style, so a docstring's "Update + chunk_summaries when …" is not a writer. A bare local rebinding (`name = fetch()`) is not a + producer: it is usually a read into a local. +- **TS/JS, Firestore and Next.js:** unchanged, and still in `trace_data_flow`, line for line. Its + layer labels, its file-level producer pre-pass and its `lib/ types/ models/` definition fallback + stay as they were (a regression test pins their output). +- **Apps Script** (`ts_adapter.py`, for `.js`/`.ts` — clasp pulls `.gs` files as `.js`), gated on + a file marker (`SpreadsheetApp`, `PropertiesService`, `DocumentApp`, `DriveApp`, `ScriptApp`), so + a Next.js file's `form.setValue()` changes nothing: + - definition: `function name`, `class Name`, `const|let|var name =`; + - producer: `setValue(s)`, `setFormula(s)`, `appendRow`, `insertRow…`, `setProperty`, + `setProperties`, `deleteProperty`, `deleteAllProperties`. +- **C#** (`csharp_adapter.py`) gets rules now, because they are cheap and unambiguous: + - definition: `class|record|struct|interface|enum Name`; + - producer: the SQL rules (Dapper and raw ADO.NET), and EF Core's + `.Name.Add|AddRange|Update|UpdateRange|Remove|RemoveRange|Attach(` and + `Set().Add…(`. + They are tested on fixtures only; B-063 (a real ASP.NET build) is where they get checked. +- **C++ and L5X:** no rules. L5X writers are `what_writes`' job, exactly, and are not routed + through here (ADR-013, `src/CLAUDE.md`). + +### 3. How `trace_data_flow` uses the rules + +- In the ranked loop, a chunk is a **definition** if the legacy `export` test or one of its file's + definition rules matches; else a **producer** if its file is a legacy Firestore producer or one of + its file's producer rules matches **in the chunk**; else transformer or consumer, as before. +- The rules match per chunk, not per file: a Python file usually holds both the writer and the + readers of a table, so marking the whole file (as the Firestore pre-pass does) would make every + reader a producer. +- **The answer is no longer limited by ranking for rule hits.** After the ranked loop, two sweeps + over the whole doc store add what ranking missed: a definition sweep, when nothing was found (the + legacy `lib/` fallback first, then the rules, up to 3 entries), and a producer sweep over + tier-1 chunks (up to 15). This is what finds `CREATE TABLE chunk_summaries`, which sits in a + tier-2 window the ranked loop skips once a tier-1 chunk of `db.py` has been seen. +- Neither sweep runs a rule on a file with none, so a Firestore/Next.js repo's output is unchanged. + +### 4. No index is an error + +Both tools now raise when the index has never been built (`index_meta.last_verified_at` unset, +ADR-025 §4), so a client sees `isError: true`. `map_module_communities` used to return its "No +index found" text as a successful result, and since ADR-047 binds the index at entry, the empty +`graph.db` that binding creates made even that check pass. `trace_data_flow` went on to search an +empty index. + +## Consequences + +**Better:** +- Communities and god objects describe the repo. On this repo, modularity rises from 0.576 to + 0.861 and `CodeDB` (51 in, 51 out, 10 coupling communities) is the top god object, which it is; + before, external hubs hid it (Implementation Log). +- In-repo import links survive: all 297 resolved IMPORTS edges stay. +- `trace_data_flow` names SQL tables' definitions and writers, Python classes, attribute and + collection writes, Apps Script writers and EF/SQL writers in C#. + +**Worse:** +- ADR-006's numbers on this repo change: 73 → 84 communities, nodes 2,620 → 1,930, edges + 8,614 → 3,428. Anyone comparing reports across this change compares two graphs. +- The graph no longer shows what the repo leans on outside itself (`requests`, `os`). That was + never a useful community member, but it is information, and only `analyze_blast_radius`'s import + text scan still sees it. +- Rule hits are regexes over chunk text, not resolved relations: a producer is a chunk that writes + the name, not a proven writer. The tool stays exploratory. +- `get_graph_edges()` does a little name binding. Binding heritage edges belongs in the resolver, + and would move there if a second reader needed it. + +**Neutral:** +- `what_writes` is untouched and still not routed through `trace_data_flow`. +- Python and C# outputs keep today's layer labels (`UTILITY` for most Python files). Per-language + layers are an open question below. + +## Alternatives Considered + +| Option | Why rejected | +|--------|-------------| +| Drop unresolved CALLS targets only | Leaves 118 non-repo members on this repo: `os`, `json`, `Protocol`, broken relative imports. Measured below. | +| Drop every edge with no `resolved_target` | Drops all 480 OWNS edges (their targets are FQNs, never "resolved") unless special-cased, and every in-repo heritage edge. With OWNS special-cased it is close to the decision but loses `TypeScriptAdapter → _WebAdapter` and the C# base types. | +| Keep external nodes, exclude them from membership | They still shape Louvain and betweenness: `os` is a hub between modules that share nothing else, and fan-out counts would still include every `append`. Excluding them from the printout only hides the distortion. | +| Rules in `indexer.toml` | Users could add their own, but a rule needs `{name}` templating, flags and a file gate, which is a small language in TOML; and a config default must equal the code default (`test_config_drift.py`), so the built-ins would be duplicated in two places. Adapters already own each language's idioms (`test_conventions`). A `[data_flow]` extension block can be added later on top of this. | +| Route producers through the resolved edge graph (like `what_writes`) | Only L5X has `writes` edges. For SQL, attributes and Apps Script there is no relation to read; the regex is what there is. | +| Mark whole files as producers, as the Firestore pre-pass does | Python files hold readers and writers together (`db.py`), so every reader would become a producer. | + +## Implementation Log + +- [x] Measure the base graph and the alternatives on this repo (below) +- [x] `get_graph_edges()` keeps only edges to repo nodes and binds heritage edges +- [ ] `data_flow_rules()` on the Python, TS/JS (Apps Script) and C# adapters +- [ ] `trace_data_flow` reads the rules, sweeps for definitions and producers +- [ ] Both tools raise when there is no index +- [ ] Tests, `src/CLAUDE.md` paragraphs, Inspector run + +**Notes:** + + +**2026-10-20, B-043 measurements.** Stack: commit `871b55c`, Python 3.13.16 (the venv; CI runs +3.12), networkx 3.7, tree-sitter 0.26.0 (python 0.25.0, javascript 0.25.0, typescript 0.23.2, +c-sharp 0.23.5, cpp 0.23.4), CPU, no embedder. Graph built SQLite-only from this repo's 172 +scannable files, then `graph_analytics.analyze()` over each edge set: + +| edge set | nodes | edges | communities (≥3) | modularity | non-repo members | top 5 sizes | top god objects | +|---|---|---|---|---|---|---|---| +| base (raw fallback) | 2,620 | 8,614 | 73 (47) | 0.5755 | 615 | 322, 313, 241, 213, 211 | HybridRetriever, IsolatedChunkSummarizer, DocumentStore, ScanPolicy, SearchScreen | +| drop unresolved CALLS only | 2,068 | 4,042 | 93 (66) | 0.8605 | 118 | 210, 159, 143, 132, 107 | CodeDB, HybridRetriever, IsolatedChunkSummarizer, DocumentStore, PythonAdapter | +| drop every unresolved target (OWNS kept) | 1,929 | 3,415 | 95 (67) | 0.8609 | 0 | 135, 129, 119, 116, 106 | CodeDB, HybridRetriever, IsolatedChunkSummarizer, DocumentStore, PythonAdapter | +| **ADR-053 (repo nodes + heritage)** | 1,930 | 3,428 | 84 (64) | 0.8613 | 0 | 138, 138, 124, 119, 116 | CodeDB, HybridRetriever, DocumentStore, PythonAdapter, SearchScreen | + +Kept edges under ADR-053: CALLS 2,664, OWNS 480, IMPORTS 297, EXTENDS 9, IMPLEMENTS 4. Top god +object before: `HybridRetriever` (score 0.429, span 3); after: `CodeDB` (score 0.770, fan-in 51, +fan-out 51, span 10). Base examples of non-repo members: `ArgumentParser`, `AssertionError`, `App`, +``, ``, `.store`, `..shared.config`. + +## Open questions for @edb + +1. **Per-language layers.** Python and C# entries are labelled `UTILITY` or `CORE LOGIC / LIB` by + the old path tests. Worth a `layer` rule on the adapters (tests, migrations, `db.py`-style data + modules), or is the bucket (definition/producer/consumer) enough? +2. **`.gs` files are not scanned.** Apps Script rules run on `.js`/`.ts` (clasp's local form). A + repo that keeps raw `.gs` files gets nothing until `scan_policy` and the registry learn `.gs`; + that is the scan session's file, so it is not done here. +3. **User-defined rules.** Is a `[data_flow]` block in `indexer.toml` that adds to the adapter + rules wanted? Nothing here needs it. +4. **Heritage binding in the resolver.** If another reader needs bound EXTENDS/IMPLEMENTS edges, + move the binding from `get_graph_edges()` into `call_resolver` and store it as + `resolved_target`. From 9a44794fcd986ae87ef0475c0003ea6a133a7c2c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:25:53 +0000 Subject: [PATCH 2/7] Keep only edges to repo nodes in the community graph get_graph_edges() returned an unresolved edge's raw target, so append, bool, ArgumentParser, os and Protocol became community members and betweenness hubs. It now returns an edge only when its target is a symbol FQN or an indexed file, and binds EXTENDS/IMPLEMENTS bare names to the in-repo type of that name (same file first, else the only one). Resolved import links survive (ADR-044). Co-Authored-By: Claude [ADR-053 src/db.py] --- src/db.py | 45 ++++++++- tests/test_graph_analytics.py | 6 ++ tests/test_graph_tools_communities.py | 140 ++++++++++++++++++++++++++ 3 files changed, 189 insertions(+), 2 deletions(-) create mode 100644 tests/test_graph_tools_communities.py diff --git a/src/db.py b/src/db.py index 465438e..2983768 100644 --- a/src/db.py +++ b/src/db.py @@ -1300,11 +1300,52 @@ def get_graph_edges(self) -> list[tuple[str, str, str]]: differently. ``resolved_target`` is preferred over the raw ``target`` when present, so edges point at canonical FQNs. Read-only; reflects the current committed graph on the shared connection. + + ADR-053: only edges whose target is a node the repo defines — a symbol FQN + or an indexed file path — are returned. An unresolved ``append``, ``bool`` + or ``os`` is not a module of this codebase, and as a node it became a + community member and a betweenness hub. An EXTENDS/IMPLEMENTS target is a + bare type name the resolver never binds, so it is bound here to the type of + that name in the subclass's own file, else to the repo's only type of that + name; ``Protocol`` or an ambiguous ``Animal`` is dropped. """ rows = self._conn.execute( - "SELECT source_fqn, COALESCE(resolved_target, target) AS tgt, kind FROM edges" + """ + SELECT source_fqn, COALESCE(resolved_target, target) AS tgt, kind FROM edges + WHERE COALESCE(resolved_target, target) IN (SELECT fqn FROM symbols) + OR COALESCE(resolved_target, target) IN (SELECT path FROM files) + """ + ).fetchall() + edges = [(r["source_fqn"], r["tgt"], r["kind"]) for r in rows] + + heritage = self._conn.execute( + """ + SELECT e.source_fqn, e.target, e.kind, s.file_id FROM edges e + LEFT JOIN symbols s ON s.fqn = e.source_fqn + WHERE e.kind IN ('EXTENDS', 'IMPLEMENTS') AND e.resolved_target IS NULL + AND e.target NOT IN (SELECT fqn FROM symbols) + """ ).fetchall() - return [(r["source_fqn"], r["tgt"], r["kind"]) for r in rows] + if not heritage: + return edges + type_kinds = ("class", "interface", "struct", "record", "partial_class", + "enum", "type_alias", "typedef") + names = sorted({r["target"] for r in heritage}) + types: dict[str, list[tuple[str, int]]] = {} + for i in range(0, len(names), 500): # SQLite's bound-parameter limit + batch = names[i:i + 500] + for r in self._conn.execute( + f"SELECT name, fqn, file_id FROM symbols WHERE name IN ({','.join('?' * len(batch))}) " + f"AND kind IN ({','.join('?' * len(type_kinds))})", + (*batch, *type_kinds), + ): + types.setdefault(r["name"], []).append((r["fqn"], r["file_id"])) + for r in heritage: + cands = types.get(r["target"], []) + pick = [f for f, fid in cands if fid == r["file_id"]] or [f for f, _ in cands] + if len(pick) == 1: + edges.append((r["source_fqn"], pick[0], r["kind"])) + return edges # ------------------------------------------------------------------ # Point lookups and search diff --git a/tests/test_graph_analytics.py b/tests/test_graph_analytics.py index c39106c..c6eb44a 100644 --- a/tests/test_graph_analytics.py +++ b/tests/test_graph_analytics.py @@ -363,6 +363,12 @@ def test_get_graph_edges_all_kinds_and_resolved_target(tmp_path): db = CodeDB(tmp_path / "graph.db") try: + # ADR-053: an edge must end at a repo node, so the targets exist here. + db._conn.execute("INSERT INTO files(id, path, content_hash) VALUES (1, 'canonical.target', 'h')") + for fqn in ("mod.B", "mod.Cls.method"): + db._conn.execute( + "INSERT INTO symbols(fqn, file_id, kind, name, start_line, end_line, text) " + "VALUES (?, 1, 'function', ?, 1, 1, '')", (fqn, fqn)) db._conn.execute( "INSERT INTO edges(source_fqn, target, kind, resolved_target) VALUES (?,?,?,?)", ("mod.A", "mod.B", "CALLS", None), diff --git a/tests/test_graph_tools_communities.py b/tests/test_graph_tools_communities.py new file mode 100644 index 0000000..9527c9c --- /dev/null +++ b/tests/test_graph_tools_communities.py @@ -0,0 +1,140 @@ +"""ADR-053 §1 (B-043): the community graph holds only nodes the repo defines. + +A small Python package goes through the real pipeline the indexer runs, model-free: +`parse_file` → `CodeDB.upsert_file` → `resolve_python_imports` → `resolve_call_edges` +(the path `tools/resolution_eval.py` uses). Then `get_graph_edges()`, `analyze()` and the +`map_module_communities` tool read it. Before ADR-053, every unresolved `append()`, +`bool()`, `ArgumentParser()`, `import os` and `class X(Protocol)` became a community member. +""" +import os +import sys + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "src")) + +from ast_chunker import parse_file # noqa: E402 +from call_resolver import resolve_call_edges # noqa: E402 +from db import CodeDB # noqa: E402 +from import_resolver import resolve_python_imports # noqa: E402 +import graph_analytics # noqa: E402 +import MCPServer as M # noqa: E402 + +SOURCES = { + "pkg/__init__.py": "", + "pkg/store.py": ( + "import os\n" + "from typing import Protocol\n" + "\n" + "class Reader(Protocol):\n" + " def read(self): ...\n" + "\n" + "class Base:\n" + " def save(self, rows):\n" + " rows.append(1)\n" + " return bool(rows)\n" + "\n" + "class Store(Base):\n" + " def load(self):\n" + " return os.listdir('.')\n" + "\n" + "def open_store():\n" + " return Store()\n" + ), + "pkg/cli.py": ( + "from argparse import ArgumentParser\n" + "from pkg.store import open_store\n" + "\n" + "def main():\n" + " parser = ArgumentParser()\n" + " store = open_store()\n" + " helper(store)\n" + "\n" + "def helper(store):\n" + " return len(store)\n" + ), + # Two unrelated files each define `Animal`: a `Dog(Animal)` in a third file is ambiguous. + "zoo/a.py": "class Animal:\n pass\n", + "zoo/b.py": "class Animal:\n pass\n", + "zoo/dog.py": "class Dog(Animal):\n pass\n", + "zoo/cat.py": "class Animal:\n pass\n\nclass Cat(Animal):\n pass\n", +} + +EXTERNAL = {"append", "bool", "ArgumentParser", "listdir", "len", "os", "argparse", "typing", + "Protocol", "Animal"} + + +def _build(db_path) -> CodeDB: + db = CodeDB(str(db_path)) + for rel, text in SOURCES.items(): + result = parse_file(rel, text) + db.upsert_file(rel, "h:" + rel, result.symbols, result.edges) + resolve_python_imports(db) + resolve_call_edges(db) + return db + + +@pytest.fixture +def graph_db(tmp_path): + db = _build(tmp_path / "graph.db") + yield db + db.close() + + +def test_the_raw_edges_do_hold_external_names(graph_db): + """The premise: without the filter these names would be nodes.""" + raw = {t for (t,) in graph_db._conn.execute("SELECT COALESCE(resolved_target, target) FROM edges")} + assert {"append", "bool", "ArgumentParser", "os", "Protocol"} <= raw + + +def test_every_edge_ends_at_a_symbol_or_file_of_the_repo(graph_db): + nodes = ({f for (f,) in graph_db._conn.execute("SELECT fqn FROM symbols")} + | {p for (p,) in graph_db._conn.execute("SELECT path FROM files")}) + edges = graph_db.get_graph_edges() + assert edges + assert {t for _, t, _ in edges} <= nodes + + +def test_resolved_import_links_survive(graph_db): + edges = graph_db.get_graph_edges() + assert ("pkg/cli.py", "pkg/store.py", "IMPORTS") in edges + assert not any(t in ("os", "argparse", "typing") for _, t, _ in edges) + + +def test_resolved_calls_and_ownership_survive(graph_db): + edges = graph_db.get_graph_edges() + assert ("pkg/cli.py::main", "pkg/store.py::open_store", "CALLS") in edges + assert ("pkg/cli.py::main", "pkg/cli.py::helper", "CALLS") in edges + assert ("pkg/store.py::Store", "pkg/store.py::Store.load", "OWNS") in edges + + +def test_heritage_binds_to_an_in_repo_type_and_drops_the_rest(graph_db): + edges = graph_db.get_graph_edges() + heritage = {(s, t) for s, t, k in edges if k == "EXTENDS"} + assert ("pkg/store.py::Store", "pkg/store.py::Base") in heritage + assert ("zoo/cat.py::Cat", "zoo/cat.py::Animal") in heritage # same file wins + assert not any(s == "zoo/dog.py::Dog" for s, _ in heritage) # three Animals: ambiguous + assert not any(t == "Protocol" for _, t in heritage) # not a repo type + + +def test_communities_contain_no_unresolved_external_names(graph_db): + analysis = graph_analytics.analyze(graph_db) + members = {m for c in analysis.communities for m in c.members} + assert members + assert not members & EXTERNAL + assert {"pkg/cli.py", "pkg/store.py"} <= members # the import link keeps both files + + +def test_map_module_communities_reports_no_external_name(tmp_path, monkeypatch): + import incremental_indexer + db = _build(tmp_path / "graph.db") + db.meta_set("last_verified_at", "2026-10-20T00:00:00Z") + db.close() + monkeypatch.setattr(incremental_indexer, "INDEX_DIR", str(tmp_path)) + monkeypatch.setattr(M, "_ensure_indexes", lambda: None) + + out = M.map_module_communities(min_community_size=1) + + assert "open_store" in out + for name in ("append", "ArgumentParser", "Protocol", "listdir"): + assert name not in out From 9abca9b5dc299e4842ce4b6dbacd940f1096ad68 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:31:53 +0000 Subject: [PATCH 3/7] Give trace_data_flow per-language definition and producer rules trace_data_flow knew only Firestore and Next.js: on this repo it found no definition for chunk_summaries and listed its writer, cache_summaries, as a consumer. Each adapter now supplies data_flow_rules(): Python (SQL CREATE/ INSERT/UPDATE/DELETE, classes, attribute and collection writes), Apps Script on the JS/TS adapter (setValue(s), appendRow, setProperty, gated on an Apps Script service in the file) and C# (types, SQL, EF Core DbSet writes). Rule hits are swept across the whole doc store, not only the ranked top-80. The Firestore/Next.js path is untouched; a golden captured from 871b55c pins its output. Both graph tools now raise when no index has been built, so the client sees isError instead of a successful refusal or a search of an empty index. Co-Authored-By: Claude [ADR-053 src/CLAUDE.md src/MCPServer.py src/adapters/base.py src/adapters/csharp_adapter.py src/adapters/python_adapter.py src/adapters/ts_adapter.py] --- src/CLAUDE.md | 20 +- src/MCPServer.py | 173 ++++++++++++++-- src/adapters/base.py | 15 ++ src/adapters/csharp_adapter.py | 21 +- src/adapters/python_adapter.py | 24 ++- src/adapters/ts_adapter.py | 19 ++ tests/test_graph_tools_data_flow.py | 311 ++++++++++++++++++++++++++++ 7 files changed, 555 insertions(+), 28 deletions(-) create mode 100644 tests/test_graph_tools_data_flow.py diff --git a/src/CLAUDE.md b/src/CLAUDE.md index f0862c0..0c9efa2 100644 --- a/src/CLAUDE.md +++ b/src/CLAUDE.md @@ -156,9 +156,17 @@ Fourteen AI-facing tools grouped by intent: PLC tag it returns every routine that writes it, with rung numbers, straight out of the resolved `writes` edges — no embedder, no RRF, no reranker, and the answer is *complete* rather than top-k. It is grouped under Tracing but it is not `trace_data_flow`'s sibling: -that tool detects producers by regex over ranked search results and is hardcoded to -Firebase/TypeScript idioms. Do not "unify" them — routing an exactly-resolved relation -through the ranked surface can only lose writers (ADR-019's hop-decay finding). +that tool detects producers by regex over chunk text. Do not "unify" them — routing an +exactly-resolved relation through the ranked surface can only lose writers (ADR-019's +hop-decay finding). + +`trace_data_flow` buckets a data name's chunks into definitions, producers, transformers +and consumers. Its Firestore/Next.js rules are inline in the tool and unchanged; every other +language's rules come from its adapter's `data_flow_rules()` (ADR-053): `{name}`-templated +regexes for `definition` and `producer`, gated by an optional `file_marker` (Apps Script's +rules apply only to files that use `SpreadsheetApp`/`PropertiesService`). Rule hits are swept +across the whole doc store, not just the ranked top-80. Add a language's idioms to its adapter, +not to the tool. The shared SQL rules are in `adapters/base.py`. `verify_candidate_edges` (ADR-023) reports edge-aware three-state verdicts over candidate (name-based / unresolved) call edges. `index_status` (ADR-025) reports @@ -169,7 +177,11 @@ content changed within a window — for agents and downstream context hubs. SQLite edge graph, runs Louvain community detection + betweenness centrality over it (`src/graph_analytics.py`, pure NetworkX), and renders a community map + god-object report (`src/graph_report.py`). Inspired by Graphify (MIT); reimplemented natively -over our EXTRACTED edges. Output is explicitly exploratory — its quality bar is +over our EXTRACTED edges. **Its nodes are the repo's own symbols and files** (ADR-053): +`get_graph_edges()` drops every edge whose target is neither, so an unresolved `append` or an +external `import os` is not a community member, and it binds a bare EXTENDS/IMPLEMENTS base +name to the in-repo type of that name. Both it and `trace_data_flow` raise, so the client sees +`isError`, when no index has been built. Output is explicitly exploratory — its quality bar is deferred to ADR-008. Tool docstrings are intentionally written for AI consumption — they describe *when* to call the tool, not just what it does. diff --git a/src/MCPServer.py b/src/MCPServer.py index b0aeb0b..551206b 100644 --- a/src/MCPServer.py +++ b/src/MCPServer.py @@ -820,11 +820,88 @@ def get_op_profile(text): return context +# ADR-053: trace_data_flow's per-language rules come from the adapters. Firestore and +# Next.js stay inline in the tool, as they were. +def _trace_layer(file_path: str, doc_text: str) -> str: + """trace_data_flow's layer label (path tests; unchanged by ADR-053).""" + is_client = "'use client'" in doc_text or ".tsx" in file_path.lower() + if any(x in file_path.lower() for x in ["firebase/admin", "lib/admin"]): + layer = "DATABASE (Admin SDK)" + elif "firebase/functions" in file_path.lower() or "functions/src" in file_path.lower(): + layer = "CLOUD FUNCTION" + elif is_client: + layer = "CLIENT COMPONENT (UI)" + if "app/" in file_path.lower() and "page.tsx" in file_path.lower() and "'use client'" not in doc_text: + layer = "SERVER COMPONENT" + elif any(x in file_path.lower() for x in ["lib/", "types/", "models/"]): + layer = "CORE LOGIC / LIB" + else: + layer = "UTILITY" + return layer + + +def _data_flow_rules(names: list[str], file_texts: dict[str, str]) -> dict[str, tuple[list, list]]: + """``{file: (definition_patterns, producer_patterns)}`` for every file whose adapter + has ``data_flow_rules()`` and whose text passes the rules' ``file_marker`` gate. + + ``{name}`` in a rule stands for any of ``names``. + """ + from adapters import get_adapter + alt = "(?:" + "|".join(re.escape(n) for n in dict.fromkeys(names)) + ")" + compiled: dict[int, tuple[list, list, list]] = {} + out: dict[str, tuple[list, list]] = {} + for fp, text in file_texts.items(): + adapter = get_adapter(os.path.splitext(fp)[1].lower()) + if not hasattr(adapter, "data_flow_rules"): + continue + if id(adapter) not in compiled: + r = adapter.data_flow_rules() + compiled[id(adapter)] = ( + [re.compile(m) for m in r.get("file_marker", [])], + [re.compile(p.replace("{name}", alt)) for p in r.get("definition", [])], + [re.compile(p.replace("{name}", alt)) for p in r.get("producer", [])], + ) + markers, defs, prods = compiled[id(adapter)] + if markers and not any(m.search(text) for m in markers): + continue + out[fp] = (defs, prods) + return out + + +def _rule_match(patterns: list, text: str): + """The first match of any pattern in ``text``, or None.""" + for p in patterns: + m = p.search(text) + if m: + return m + return None + + +def _rule_entry(layer: str, file_path: str, doc: dict, match) -> str: + """A trace entry whose snippet is centred on the rule's match, not the chunk's start.""" + text = doc['text'] + start = max(0, match.start() - 40) + snippet = text[start:start + 140].replace('\n', ' ').strip() + "..." + return f"- [{layer}] {file_path}\n [Scope]: {get_clean_scope(doc)}\n [Snippet]: {snippet}\n\n" + + @_tool() def trace_data_flow(target_symbol: str) -> str: + """Trace one piece of data through the code: where it is defined, who writes it, + who transforms it and who reads it. + + WHEN TO CALL: before changing a table, collection, type, sheet or stored property, + to find its schema and its writers. For a PLC tag call what_writes instead: it is + exact, this is not. + + target_symbol — the data's name as the code spells it (`chunk_summaries`, `orders`). + + Definitions and writers come from per-language rules: SQL CREATE/INSERT/UPDATE/DELETE, + Python classes and attribute/collection writes, Firestore writes, Apps Script + setValue(s)/appendRow/setProperty, EF Core. Rule hits are searched across the whole + index; the other buckets come from ranked search. Exploratory: verify before acting. """ - Traces data lifecycle. v11.0: Broad definition lookup + Dynamic Producer Tracing. - """ + _require_built_index("trace_data_flow") print(f"\n[MCP] Running v11.0 trace for '{target_symbol}'...") query_text = f"Definition, usage, and fetching of {target_symbol} get{target_symbol} fetch{target_symbol}" # Shared RTR surface (ADR-023 §1) instead of raw t1+t2 FAISS: resolved @@ -861,6 +938,11 @@ def trace_data_flow(target_symbol: str) -> str: elif "firebase/functions" in _fp_lower or "functions/src" in _fp_lower: producer_files.add(_fp) + # ADR-053: the per-language rules of every file that mentions the name. + _names = [target_symbol, pascal_symbol] + _rules = _data_flow_rules( + _names, {fp: t for fp, t in _file_texts.items() if any(n in t for n in _names)}) + seen_scopes = set() # Tracks files that received any bucket entry from a tier-1 chunk. # Tier-2/3 chunks are skipped for files already covered, preventing duplicate @@ -887,19 +969,7 @@ def trace_data_flow(target_symbol: str) -> str: doc_text = c.text # --- LAYER DETECTION --- - is_client = "'use client'" in doc_text or ".tsx" in file_path.lower() - if any(x in file_path.lower() for x in ["firebase/admin", "lib/admin"]): - layer = "DATABASE (Admin SDK)" - elif "firebase/functions" in file_path.lower() or "functions/src" in file_path.lower(): - layer = "CLOUD FUNCTION" - elif is_client: - layer = "CLIENT COMPONENT (UI)" - if "app/" in file_path.lower() and "page.tsx" in file_path.lower() and "'use client'" not in doc_text: - layer = "SERVER COMPONENT" - elif any(x in file_path.lower() for x in ["lib/", "types/", "models/"]): - layer = "CORE LOGIC / LIB" - else: - layer = "UTILITY" + layer = _trace_layer(file_path, doc_text) # --- FIX 3: DYNAMIC PRODUCER DETECTION --- # We broaden the anchors to catch 'aggRef.set' or 'customBatch.update' @@ -911,6 +981,16 @@ def trace_data_flow(target_symbol: str) -> str: is_def = bool(re.search(rf"export\s+(interface|type|class)\s+({re.escape(target_symbol)}|{re.escape(pascal_symbol)})", doc_text)) is_producer = file_path in producer_files + # ADR-053: the file's language rules, matched in this chunk. + rule_hit = None + if file_path in _rules: + _defs, _prods = _rules[file_path] + if not is_def: + rule_hit = _rule_match(_defs, doc_text) + is_def = rule_hit is not None + if not is_def and not is_producer: + rule_hit = _rule_match(_prods, doc_text) + is_producer = rule_hit is not None is_transformer = any(op in doc_text for op in [".filter(", ".map(", ".sort(", "useMemo("]) violation_tag = "" @@ -921,6 +1001,8 @@ def trace_data_flow(target_symbol: str) -> str: clean_scope = get_clean_scope({'scope': c.scope, 'text': c.text, 'file': c.file}) snippet = doc_text[:140].replace('\n', ' ').strip() + "..." entry = f"- [{layer}] {file_path}\n [Scope]: {clean_scope}\n{violation_tag} [Snippet]: {snippet}\n\n" + if rule_hit is not None: + entry = _rule_entry(layer, file_path, {'scope': c.scope, 'text': c.text, 'file': c.file}, rule_hit) seen_files_any_bucket.add(file_path) @@ -949,6 +1031,42 @@ def trace_data_flow(target_symbol: str) -> str: buckets["DEFINITIONS"].append(entry) break + # --- ADR-053: RULE SWEEPS over the whole index --- + # Ranking decides only the order of the loop above. A rule hit the search missed + # (a CREATE TABLE in a tier-2 window of a file whose tier-1 chunk was already seen, + # a writer ranked below 80) is still a definition or a producer. + if _rules: + _docs = sorted(_index().doc_store.docs.values(), key=lambda d: d['tier']) + if not buckets["DEFINITIONS"]: + for doc in _docs: + fp = doc['file'].replace('\\', '/') + if fp not in _rules or not any(n in doc['text'] for n in _names): + continue + key = f"{fp}::{doc['scope']}" + m = _rule_match(_rules[fp][0], doc['text']) + if m is None or key in seen_scopes: + continue + seen_scopes.add(key) + buckets["DEFINITIONS"].append(_rule_entry(_trace_layer(fp, doc['text']), fp, doc, m)) + if len(buckets["DEFINITIONS"]) >= 3: + break + swept = 0 + for doc in _docs: + if doc['tier'] != 'tier1_surgical' or swept >= 15: + continue + fp = doc['file'].replace('\\', '/') + if fp not in _rules or target_symbol not in doc['text']: + continue + key = f"{fp}::{doc['scope']}" + if key in seen_scopes or _rule_match(_rules[fp][0], doc['text']): + continue + m = _rule_match(_rules[fp][1], doc['text']) + if m is None: + continue + seen_scopes.add(key) + buckets["PRODUCERS"].append(_rule_entry(_trace_layer(fp, doc['text']), fp, doc, m)) + swept += 1 + context = f"--- DATA FLOW TRACE: {target_symbol} ---\n\n" for cat, items in buckets.items(): context += f"### {cat}\n" @@ -2314,6 +2432,20 @@ def find_unabstracted_collection_reads(collection_name: str, canonical_symbols_c return context +def _require_built_index(tool: str) -> str: + """The index directory, or raise so the client sees ``isError`` (ADR-053 §4). + + A built index has ``last_verified_at`` (ADR-025 §4). The empty ``graph.db`` that + binding the index at a tool's entry creates (ADR-047) does not count. + """ + from incremental_indexer import INDEX_DIR + if not _index_built(INDEX_DIR): + raise RuntimeError( + f"{tool}: no index has been built in {INDEX_DIR}. Run `reindex` first, then " + f"call this tool again.") + return INDEX_DIR + + @_tool() def map_module_communities(target_path: str = "", min_community_size: int = 3, suggest_splits: bool = False) -> str: @@ -2330,6 +2462,9 @@ def map_module_communities(target_path: str = "", min_community_size: int = 3, '[HEURISTIC — unverified]'. Also writes the DSM view to .code-index/architecture_matrix.html when the visualization layer is available. + Nodes are this repo's symbols and files only; calls and imports that resolve + outside the repo are left out. + Inputs: target_path — optional subtree to scope analysis to (e.g. 'src/'); empty = whole graph. min_community_size — communities smaller than this are omitted from the body (still counted). @@ -2341,18 +2476,12 @@ def map_module_communities(target_path: str = "", min_community_size: int = 3, print(f"\n[MCP] map_module_communities: path='{target_path}' " f"min_size={min_community_size} splits={suggest_splits}") import os - from incremental_indexer import INDEX_DIR from db import CodeDB import graph_analytics from graph_report import render_report + INDEX_DIR = _require_built_index("map_module_communities") db_path = os.path.join(INDEX_DIR, "graph.db") - if not os.path.exists(db_path): - return ( - "--- MODULE COMMUNITY MAP ---\n\n" - "No index found — `graph.db` does not exist. Run `reindex` first, then " - "call this tool again.\n" - ) with CodeDB(db_path) as db: analysis = graph_analytics.analyze( diff --git a/src/adapters/base.py b/src/adapters/base.py index cf0d4ca..863a2ce 100644 --- a/src/adapters/base.py +++ b/src/adapters/base.py @@ -135,6 +135,21 @@ class ParseResult(NamedTuple): symbol_types: list[SymbolType] +# --------------------------------------------------------------------------- +# Data-flow rules (ADR-053) — shared SQL idioms for adapters' data_flow_rules() +# --------------------------------------------------------------------------- +# `{name}` stands for the traced name. Keywords match upper case only, the usual SQL +# style in code, so a docstring's "Update chunk_summaries when ..." is not a writer. +_SQL_IDENT = r"""[\"'`\[]?{name}\b""" +SQL_DEFINITION_RULES = [ + rf"\bCREATE\s+(?:TEMP(?:ORARY)?\s+)?(?:TABLE|VIEW)\s+(?:IF\s+NOT\s+EXISTS\s+)?{_SQL_IDENT}", +] +SQL_PRODUCER_RULES = [ + rf"\b(?:INSERT(?:\s+OR\s+[A-Z]+)?\s+INTO|REPLACE\s+INTO|DELETE\s+FROM)\s+{_SQL_IDENT}", + rf"\bUPDATE(?:\s+OR\s+[A-Z]+)?\s+{_SQL_IDENT}[\]\"'`]?\s+SET\b", +] + + # --------------------------------------------------------------------------- # FQN builder — language-neutral default; C# adapter overrides for CLR convention # --------------------------------------------------------------------------- diff --git a/src/adapters/csharp_adapter.py b/src/adapters/csharp_adapter.py index ecd00de..435d92d 100644 --- a/src/adapters/csharp_adapter.py +++ b/src/adapters/csharp_adapter.py @@ -41,7 +41,9 @@ from tree_sitter import Language, Parser, Node import tree_sitter_c_sharp as tscs -from adapters.base import Edge, ParseResult, Reference, Symbol, TestConventions +from adapters.base import ( + SQL_DEFINITION_RULES, SQL_PRODUCER_RULES, Edge, ParseResult, Reference, Symbol, TestConventions, +) from adapters._treesitter import node_text, run_query from category_tagger import tag_symbol from type_resolver import infer_csharp_call_targets, csharp_field_types @@ -322,6 +324,23 @@ def test_conventions(self) -> TestConventions: def project_resolver(self) -> CsprojResolver: return _CSPROJ_RESOLVER + def data_flow_rules(self) -> dict[str, list[str]]: + """ADR-053: what defines and what writes a traced name, for trace_data_flow. + + Producers are SQL (Dapper, ADO.NET) and EF Core's DbSet writes. `{name}` + stands for the name. + """ + return { + "definition": SQL_DEFINITION_RULES + [ + r"\b(?:class|record|struct|interface|enum)\s+{name}\b", + ], + "producer": SQL_PRODUCER_RULES + [ + r"\.{name}\s*\.\s*(?:Add|AddRange|Update|UpdateRange|Remove|RemoveRange|Attach)(?:Async)?\s*\(", + r"\bSet<{name}>\(\)\s*\.\s*(?:Add|AddRange|Update|UpdateRange|Remove|RemoveRange|Attach)" + r"(?:Async)?\s*\(", + ], + } + # ------------------------------------------------------------------ # Internal — AST walk # ------------------------------------------------------------------ diff --git a/src/adapters/python_adapter.py b/src/adapters/python_adapter.py index 31e10fc..6df39e3 100644 --- a/src/adapters/python_adapter.py +++ b/src/adapters/python_adapter.py @@ -8,7 +8,10 @@ from tree_sitter import Language, Parser, Node import tree_sitter_python as tspython -from adapters.base import Edge, ParseResult, Reference, Symbol, TestConventions, build_fqn +from adapters.base import ( + SQL_DEFINITION_RULES, SQL_PRODUCER_RULES, Edge, ParseResult, Reference, Symbol, TestConventions, + build_fqn, +) from adapters._treesitter import ( merge_adjacent_same_fqn, node_text, run_query, skeleton_with_lines, ) @@ -233,6 +236,25 @@ def test_conventions(self): def project_resolver(self): return None + def data_flow_rules(self) -> dict[str, list[str]]: + """ADR-053: what defines and what writes a traced name, for trace_data_flow. + + `{name}` stands for the name. A bare rebinding (`name = fetch()`) is not a + producer: it is usually a read into a local. + """ + return { + "definition": SQL_DEFINITION_RULES + [ + r"(?m)^\s*class\s+{name}\b", + r"(?m)^{name}\s*(?::[^=\n]+)?=(?!=)", # module level: column 0 + ], + "producer": SQL_PRODUCER_RULES + [ + r"\.{name}\s*(?:\[[^\]\n]*\]\s*)*(?:[-+*/|&]?=)(?!=)", + r"(? dict[str, list[str]]: + """ADR-053: Apps Script's definitions and writers, for trace_data_flow. + + Firestore and Next.js stay in trace_data_flow itself, unchanged. These rules + apply only to a file that uses an Apps Script service (`file_marker`), so a + Next.js form's `setValue()` is not a writer. clasp pulls `.gs` files as `.js`. + """ + return { + "file_marker": [r"\b(?:SpreadsheetApp|PropertiesService|DocumentApp|DriveApp|ScriptApp)\b"], + "definition": [ + r"(?m)^\s*(?:function|class)\s+{name}\b", + r"(?m)^\s*(?:const|let|var)\s+{name}\s*=", + ], + "producer": [ + r"\.(?:setValues?|setFormulas?|appendRow|insertRows?\w*|setProperty|setProperties" + r"|deleteProperty|deleteAllProperties)\s*\(", + ], + } + # ------------------------------------------------------------------ # Internal: symbol extraction (mirrors _extract_ts_symbols exactly) # ------------------------------------------------------------------ diff --git a/tests/test_graph_tools_data_flow.py b/tests/test_graph_tools_data_flow.py new file mode 100644 index 0000000..9356a2a --- /dev/null +++ b/tests/test_graph_tools_data_flow.py @@ -0,0 +1,311 @@ +"""ADR-053 §2–§4 (B-042): trace_data_flow's per-language rules, and both graph tools' +failure when there is no index. + +`trace_data_flow` classifies ranked search results, which need the embedder. Here the +search is faked: `_search` returns the fixture's chunks in a fixed order, and the bound +index state holds the same chunks as its doc store. Everything after retrieval — the +Firestore pre-pass, the rules, the buckets, the sweeps — is the real code. +""" +import os +import sys +from types import SimpleNamespace + +import anyio +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "src")) + +from db import CodeDB # noqa: E402 +from hybrid_retriever import RetrievedChunk # noqa: E402 +import MCPServer as M # noqa: E402 + +T1, T2 = "tier1_surgical", "tier2_component" + + +def _wire(monkeypatch, tmp_path, chunks, ranked=None): + """Serve ``chunks`` ((tier, file, scope, text)) as the index; ``ranked`` (indexes into + chunks) is what the fake search returns, default all of them in order.""" + import incremental_indexer + db = CodeDB(str(tmp_path / "graph.db")) + db.meta_set("last_verified_at", "2026-10-20T00:00:00Z") + db.close() + monkeypatch.setattr(incremental_indexer, "INDEX_DIR", str(tmp_path)) + + docs = {str(i): {"tier": t, "file": f, "scope": s, "text": x, "tags": []} + for i, (t, f, s, x) in enumerate(chunks)} + state = SimpleNamespace(doc_store=SimpleNamespace(docs=docs)) + monkeypatch.setattr(M, "_ensure_indexes", lambda: state) + order = range(len(chunks)) if ranked is None else ranked + hits = [RetrievedChunk(faiss_id=i, score=1.0, file=chunks[i][1], scope=chunks[i][2], + tier=chunks[i][0], text=chunks[i][3], source="semantic") + for i in order] + monkeypatch.setattr(M, "_search", lambda query, top_n=10: hits) + + +def _buckets(out: str) -> dict[str, str]: + """Each ### section of the report, by name.""" + parts = {} + for block in out.split("### ")[1:]: + name, _, body = block.partition("\n") + parts[name.strip()] = body + return parts + + +# ------------------------------------------------------------------- Python / SQL + +PY_CHUNKS = [ + (T1, "src/db.py", "src/db.py::CodeDB.get_cached_summaries", + "def get_cached_summaries(self, hashes):\n" + " rows = self._conn.execute(\n" + " f\"SELECT text_hash, summary FROM chunk_summaries WHERE text_hash IN ({ph})\",\n" + " hashes).fetchall()\n" + " return dict(rows)\n"), + (T1, "src/db.py", "src/db.py::CodeDB.cache_summaries", + "def cache_summaries(self, items):\n" + " with self._tx() as cur:\n" + " cur.executemany(\n" + " \"INSERT OR IGNORE INTO chunk_summaries(text_hash, summary) VALUES (?, ?)\",\n" + " items)\n"), + # The schema lives in a module-level string: a tier-2 window, which the ranked loop + # skips once db.py's tier-1 chunks have been seen. + (T2, "src/db.py", "src/db.py::anonymous_part_1", + "_SCHEMA = \"\"\"\n" + "CREATE TABLE IF NOT EXISTS files (id INTEGER PRIMARY KEY);\n" + "CREATE TABLE IF NOT EXISTS chunk_summaries (\n" + " text_hash TEXT PRIMARY KEY,\n" + " summary TEXT NOT NULL\n" + ");\n\"\"\"\n"), + (T1, "src/summarizer.py", "src/summarizer.py::summarize_pending", + "def summarize_pending(db):\n" + " \"\"\"Update chunk_summaries for every chunk that has none.\"\"\"\n" + " cached = db.get_cached_summaries(keys) # reads chunk_summaries\n" + " return [k for k in keys if k not in cached]\n"), + (T1, "src/cache.py", "src/cache.py::Cache.remember", + "def remember(self, key, value):\n" + " self.chunk_summaries[key] = value\n"), + (T1, "src/cache.py", "src/cache.py::Cache.forget_all", + "def forget_all(self):\n" + " self.chunk_summaries.clear()\n"), + (T1, "src/cache.py", "src/cache.py::Cache.is_empty", + "def is_empty(self):\n" + " return self.chunk_summaries == {}\n"), +] + + +def test_chunk_summaries_finds_its_definition_and_its_producer(monkeypatch, tmp_path): + # The search ranks the writer below the reader and never returns the schema window first. + _wire(monkeypatch, tmp_path, PY_CHUNKS, ranked=[0, 2, 3, 1, 4, 5, 6]) + b = _buckets(M.trace_data_flow("chunk_summaries")) + + assert "CREATE TABLE IF NOT EXISTS chunk_summaries" in b["DEFINITIONS"] + assert "CodeDB.cache_summaries" in b["PRODUCERS"] + assert "CodeDB.cache_summaries" not in b["CONSUMERS"] + assert "CodeDB.get_cached_summaries" in b["CONSUMERS"] + + +def test_attribute_subscript_and_mutating_calls_are_producers(monkeypatch, tmp_path): + _wire(monkeypatch, tmp_path, PY_CHUNKS) + b = _buckets(M.trace_data_flow("chunk_summaries")) + + assert "Cache.remember" in b["PRODUCERS"] # self.chunk_summaries[key] = value + assert "Cache.forget_all" in b["PRODUCERS"] # self.chunk_summaries.clear() + assert "Cache.is_empty" in b["CONSUMERS"] # == is a comparison, not a write + + +def test_a_docstring_saying_update_is_not_a_writer(monkeypatch, tmp_path): + _wire(monkeypatch, tmp_path, PY_CHUNKS) + b = _buckets(M.trace_data_flow("chunk_summaries")) + assert "summarize_pending" in b["CONSUMERS"] + assert "summarize_pending" not in b["PRODUCERS"] + + +def test_the_producer_sweep_finds_a_writer_search_did_not_return(monkeypatch, tmp_path): + _wire(monkeypatch, tmp_path, PY_CHUNKS, ranked=[0]) + b = _buckets(M.trace_data_flow("chunk_summaries")) + assert "CodeDB.cache_summaries" in b["PRODUCERS"] + assert "CREATE TABLE IF NOT EXISTS chunk_summaries" in b["DEFINITIONS"] + + +def test_python_classes_and_dataclasses_are_definitions(monkeypatch, tmp_path): + chunks = [ + (T1, "src/model.py", "src/model.py::Order", + "@dataclass(frozen=True)\nclass Order:\n id: int\n"), + (T1, "src/shop.py", "src/shop.py::place", + "def place(order: Order):\n return order.id\n"), + ] + _wire(monkeypatch, tmp_path, chunks) + b = _buckets(M.trace_data_flow("Order")) + assert "src/model.py::Order" in b["DEFINITIONS"] + assert "src/shop.py::place" in b["CONSUMERS"] + + +# --------------------------------------------------------------- Firestore / Next.js + +FIRESTORE_CHUNKS = [ + (T1, "lib/types/inventory.ts", "lib/types/inventory.ts::AggregatedInventory", + "export interface AggregatedInventory {\n sku: string;\n onHand: number;\n}\n"), + (T1, "functions/src/aggregate.ts", "functions/src/aggregate.ts::rebuild", + "export async function rebuild() {\n" + " const aggregatedInventory = await compute();\n" + " await aggRef.set({ aggregatedInventory });\n}\n"), + (T1, "app/inventory/page.tsx", "app/inventory/page.tsx::Page", + "export default async function Page() {\n" + " const aggregatedInventory = await getAggregatedInventory();\n" + " return ;\n}\n"), + (T1, "components/InventoryTable.tsx", "components/InventoryTable.tsx::InventoryTable", + "'use client'\nexport function InventoryTable({ aggregatedInventory }) {\n" + " const rows = aggregatedInventory.filter(r => r.onHand > 0);\n" + " return rows;\n}\n"), + (T1, "components/EditStock.tsx", "components/EditStock.tsx::EditStock", + "'use client'\nexport function EditStock({ aggregatedInventory, form }) {\n" + " form.setValue('sku', aggregatedInventory.sku);\n" + " docRef.update({ aggregatedInventory });\n}\n"), + (T1, "lib/admin/inventory.ts", "lib/admin/inventory.ts::getAggregatedInventory", + "export async function getAggregatedInventory() {\n" + " const snap = await db.collection('aggregatedInventory').get();\n" + " return snap.data();\n}\n"), +] + +# trace_data_flow("aggregatedInventory") on the fixture above, from the code at 871b55c +# (before ADR-053), captured by running that commit's MCPServer on it. ADR-053 must not +# change a character of it. +FIRESTORE_GOLDEN = """--- DATA FLOW TRACE: aggregatedInventory --- + +### DEFINITIONS +- [CORE LOGIC / LIB] lib/types/inventory.ts + [Scope]: lib/types/inventory.ts::AggregatedInventory + [Snippet]: export interface AggregatedInventory { + sku: string; + onHand: number; +}... + + +### PRODUCERS +- [CLOUD FUNCTION] functions/src/aggregate.ts + [Scope]: functions/src/aggregate.ts::rebuild + [Snippet]: export async function rebuild() { const aggregatedInventory = await compute(); await aggRef.set({ aggregatedInventory }); }... + + +### TRANSFORMERS +- [CLIENT COMPONENT (UI)] components/InventoryTable.tsx + [Scope]: components/InventoryTable.tsx::InventoryTable + [Snippet]: 'use client' export function InventoryTable({ aggregatedInventory }) { const rows = aggregatedInventory.filter(r => r.onHand > 0); retur... + + +### CONSUMERS +- [SERVER COMPONENT] app/inventory/page.tsx + [Scope]: app/inventory/page.tsx::Page + [Snippet]: export default async function Page() { const aggregatedInventory = await getAggregatedInventory(); return
\n" + " _conn.ExecuteAsync(\"DELETE FROM Order WHERE Id = @id\", new { id });\n"), + (T1, "Data/OrderRepo.cs", "Shop.Data.OrderRepo.Get", + "public Task Get(int id) =>\n" + " _conn.QuerySingleAsync(\"SELECT * FROM Order WHERE Id = @id\", new { id });\n"), + ] + _wire(monkeypatch, tmp_path, chunks) + b = _buckets(M.trace_data_flow("Order")) + assert "Shop.Data.Order\n" in b["DEFINITIONS"] + assert "OrderService.Place" in b["PRODUCERS"] + assert "OrderRepo.Delete" in b["PRODUCERS"] + assert "OrderRepo.Get" in b["CONSUMERS"] + + +# ------------------------------------------------------------- no index: isError + +def _call(tool: 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(tool, args) + return anyio.run(main) + + +@pytest.mark.parametrize("tool,args", [ + ("trace_data_flow", {"target_symbol": "chunk_summaries"}), + ("map_module_communities", {}), +]) +def test_no_index_is_an_error_through_the_mcp_layer(tool, args, tmp_path, monkeypatch): + import incremental_indexer + monkeypatch.setattr(incremental_indexer, "INDEX_DIR", str(tmp_path)) + CodeDB(str(tmp_path / "graph.db")).close() # what binding an empty index leaves + state = SimpleNamespace(doc_store=SimpleNamespace(docs={})) + monkeypatch.setattr(M, "_ensure_indexes", lambda: state) + monkeypatch.setattr(M, "_search", lambda *a, **k: pytest.fail("searched an empty index")) + + result = _call(tool, args) + + assert result.isError is True + assert "no index has been built" in result.content[0].text From 5518eb49d558119e45360c5c79956c726d82e080 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:36:38 +0000 Subject: [PATCH 4/7] Log ADR-053's measurements, deviations and Inspector run Co-Authored-By: Claude [ADR-053] --- .../adr/ADR-053-graph-tools-tell-the-truth.md | 50 +++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/docs/adr/ADR-053-graph-tools-tell-the-truth.md b/docs/adr/ADR-053-graph-tools-tell-the-truth.md index b109cb7..3c9e499 100644 --- a/docs/adr/ADR-053-graph-tools-tell-the-truth.md +++ b/docs/adr/ADR-053-graph-tools-tell-the-truth.md @@ -161,10 +161,13 @@ empty index. - [x] Measure the base graph and the alternatives on this repo (below) - [x] `get_graph_edges()` keeps only edges to repo nodes and binds heritage edges -- [ ] `data_flow_rules()` on the Python, TS/JS (Apps Script) and C# adapters -- [ ] `trace_data_flow` reads the rules, sweeps for definitions and producers -- [ ] Both tools raise when there is no index -- [ ] Tests, `src/CLAUDE.md` paragraphs, Inspector run +- [x] `data_flow_rules()` on the Python, TS/JS (Apps Script) and C# adapters; the shared SQL + rules in `adapters/base.py` +- [x] `trace_data_flow` reads the rules, sweeps for definitions and producers +- [x] Both tools raise when there is no index (`_require_built_index`) +- [x] Tests: `tests/test_graph_tools_communities.py` (7), `tests/test_graph_tools_data_flow.py` + (12), one existing test updated +- [x] `src/CLAUDE.md` paragraphs, MCP Inspector run **Notes:** @@ -186,6 +189,45 @@ object before: `HybridRetriever` (score 0.429, span 3); after: `CodeDB` (score 0 fan-out 51, span 10). Base examples of non-repo members: `ArgumentParser`, `AssertionError`, `App`, ``, ``, `.store`, `..shared.config`. +**2026-10-20, B-042 on this repo's code.** No index can be built here (no model), so the trace ran +over a doc store made from this repo's symbols (each symbol's text as a tier-1 chunk, each file's +whole text as a tier-2 window), with the 80 first chunks naming the target standing in for the +search. Same stack as above. `trace_data_flow("chunk_summaries")`: +- before (`871b55c`): no definition, no producer; `cache_summaries` a consumer (the backlog's + report, reproduced); +- after: definition `CREATE TABLE IF NOT EXISTS chunk_summaries` in `src/db.py`; producers + `CodeDB.cache_summaries`, `CodeDB._migrate_summary_keys`, and two test/tool seeders; consumers + `get_cached_summaries`, `summary_cache_key`, `run_summarization_pass` and others. + +**2026-10-20, Firestore/Next.js regression guard.** The golden in +`test_firestore_and_nextjs_output_is_unchanged` was captured by running `871b55c`'s +`MCPServer.trace_data_flow` on the test's fixture (and on a ranked-to-one-chunk variant). The +branch's output is byte-identical on both. + +**2026-10-20, deviations and surprises.** +- **Outside the ownership list:** `src/adapters/base.py` gains `SQL_DEFINITION_RULES` and + `SQL_PRODUCER_RULES` (15 lines, additive), so the Python and C# adapters share one SQL rule set + instead of two copies. `tests/test_graph_analytics.py`'s `get_graph_edges` test now inserts the + symbols its edges point at, since an edge to a non-node is dropped by design. In `src/CLAUDE.md` + the `what_writes` paragraph's clause about `trace_data_flow` ("hardcoded to Firebase/TypeScript + idioms") was no longer true and is shortened. +- The inline layer code of `trace_data_flow` moved into `_trace_layer()`, unchanged, so the sweeps + label their entries the same way. +- **The old no-index check never fired** (Decision §4): the base server, run from an empty + directory, returned an empty community map as a success. +- Heritage binding was not in the prompt's three options; it was added after the measurement showed + 13 in-repo heritage edges the plain filter would lose. + +**2026-10-20, MCP Inspector 2.8.0**, server run from an empty `/tmp/insp`, CPU: +- `tools/list --strict`: exit 0, 14 tools. `trace_data_flow`'s description is 721 characters, + `map_module_communities`' 1,267. No tool declares `readOnlyHint` (B-045, `_tool()`, not this + ADR's). +- `trace_data_flow target_symbol=chunk_summaries` and `map_module_communities target_path=src/`: + exit 5, `isError: true`, "no index has been built in .code-index. Run `reindex` first". +- `trace_data_flow` with no argument and `map_module_communities min_community_size=notanint`: + exit 5, `isError: true`, argument validation errors. +- The success paths are covered in pytest (`test_graph_tools_*.py`), since there is no index here. + ## Open questions for @edb 1. **Per-language layers.** Python and C# entries are labelled `UTILITY` or `CORE LOGIC / LIB` by From 6d178785d3263d3763630d24d227fb1089e21c31 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:37:14 +0000 Subject: [PATCH 5/7] Keep the violation tag on rule hits and refuse an empty trace name A rule-matched entry dropped the client-side-write tag the legacy entry carries, and an empty target_symbol made every rule match every file. Co-Authored-By: Claude [ADR-053 src/MCPServer.py] --- docs/adr/ADR-053-graph-tools-tell-the-truth.md | 2 +- src/MCPServer.py | 9 ++++++--- tests/test_graph_tools_data_flow.py | 6 ++++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/adr/ADR-053-graph-tools-tell-the-truth.md b/docs/adr/ADR-053-graph-tools-tell-the-truth.md index 3c9e499..94eef41 100644 --- a/docs/adr/ADR-053-graph-tools-tell-the-truth.md +++ b/docs/adr/ADR-053-graph-tools-tell-the-truth.md @@ -166,7 +166,7 @@ empty index. - [x] `trace_data_flow` reads the rules, sweeps for definitions and producers - [x] Both tools raise when there is no index (`_require_built_index`) - [x] Tests: `tests/test_graph_tools_communities.py` (7), `tests/test_graph_tools_data_flow.py` - (12), one existing test updated + (13), one existing test updated - [x] `src/CLAUDE.md` paragraphs, MCP Inspector run **Notes:** diff --git a/src/MCPServer.py b/src/MCPServer.py index 551206b..f76b9b5 100644 --- a/src/MCPServer.py +++ b/src/MCPServer.py @@ -877,12 +877,12 @@ def _rule_match(patterns: list, text: str): return None -def _rule_entry(layer: str, file_path: str, doc: dict, match) -> str: +def _rule_entry(layer: str, file_path: str, doc: dict, match, tag: str = "") -> str: """A trace entry whose snippet is centred on the rule's match, not the chunk's start.""" text = doc['text'] start = max(0, match.start() - 40) snippet = text[start:start + 140].replace('\n', ' ').strip() + "..." - return f"- [{layer}] {file_path}\n [Scope]: {get_clean_scope(doc)}\n [Snippet]: {snippet}\n\n" + return f"- [{layer}] {file_path}\n [Scope]: {get_clean_scope(doc)}\n{tag} [Snippet]: {snippet}\n\n" @_tool() @@ -901,6 +901,8 @@ def trace_data_flow(target_symbol: str) -> str: setValue(s)/appendRow/setProperty, EF Core. Rule hits are searched across the whole index; the other buckets come from ranked search. Exploratory: verify before acting. """ + if not target_symbol.strip(): + raise ValueError("trace_data_flow: target_symbol is empty; pass the data's name.") _require_built_index("trace_data_flow") print(f"\n[MCP] Running v11.0 trace for '{target_symbol}'...") query_text = f"Definition, usage, and fetching of {target_symbol} get{target_symbol} fetch{target_symbol}" @@ -1002,7 +1004,8 @@ def trace_data_flow(target_symbol: str) -> str: snippet = doc_text[:140].replace('\n', ' ').strip() + "..." entry = f"- [{layer}] {file_path}\n [Scope]: {clean_scope}\n{violation_tag} [Snippet]: {snippet}\n\n" if rule_hit is not None: - entry = _rule_entry(layer, file_path, {'scope': c.scope, 'text': c.text, 'file': c.file}, rule_hit) + entry = _rule_entry(layer, file_path, {'scope': c.scope, 'text': c.text, 'file': c.file}, + rule_hit, violation_tag) seen_files_any_bucket.add(file_path) diff --git a/tests/test_graph_tools_data_flow.py b/tests/test_graph_tools_data_flow.py index 9356a2a..73849cb 100644 --- a/tests/test_graph_tools_data_flow.py +++ b/tests/test_graph_tools_data_flow.py @@ -282,6 +282,12 @@ def test_csharp_ef_and_sql_writers_and_type_definitions(monkeypatch, tmp_path): assert "OrderRepo.Get" in b["CONSUMERS"] +def test_an_empty_name_is_refused(monkeypatch, tmp_path): + _wire(monkeypatch, tmp_path, PY_CHUNKS) + with pytest.raises(ValueError): + M.trace_data_flow(" ") + + # ------------------------------------------------------------- no index: isError def _call(tool: str, args: dict): From 541188b2baec90327c163a30aa18f854f92f4fca Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 20:38:55 +0000 Subject: [PATCH 6/7] Record ADR-053's gate results Co-Authored-By: Claude [ADR-053] --- docs/adr/ADR-053-graph-tools-tell-the-truth.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/adr/ADR-053-graph-tools-tell-the-truth.md b/docs/adr/ADR-053-graph-tools-tell-the-truth.md index 94eef41..f6c1d35 100644 --- a/docs/adr/ADR-053-graph-tools-tell-the-truth.md +++ b/docs/adr/ADR-053-graph-tools-tell-the-truth.md @@ -228,6 +228,13 @@ branch's output is byte-identical on both. exit 5, `isError: true`, argument validation errors. - The success paths are covered in pytest (`test_graph_tools_*.py`), since there is no index here. +**2026-10-20, gates** (Python 3.12.3, CPU, no network, so `HF_HUB_OFFLINE=1`): flake8 on `src/` +clean; collection 704 → 724 tests. Full suite: `871b55c` 654 passed, 48 failed, 2 skipped; this +branch 673 passed (before the empty-name test), the **same 48 failed**, 2 skipped. All 48 also fail +on `871b55c`: 31 need the tokenizer from huggingface.co, the rest fail the same way on both +commits in this sandbox. `conformance_eval --check-baseline`, `resolution_eval --check-baseline` +and `l5x_fixture_teeth --check` exit 0. + ## Open questions for @edb 1. **Per-language layers.** Python and C# entries are labelled `UTILITY` or `CORE LOGIC / LIB` by From 685dbe265c251165facff0fbbc6a7ce33b559eb9 Mon Sep 17 00:00:00 2001 From: Ethan Blauw Date: Mon, 2 Nov 2026 11:22:44 -0600 Subject: [PATCH 7/7] ADR-053: apply @edb's answers to the open questions; status accepted Co-Authored-By: Claude Opus 5.5 [ADR-053 src/MCPServer.py] --- docs/adr/ADR-053-graph-tools-tell-the-truth.md | 11 ++++++++++- src/MCPServer.py | 18 ++++++++++++++++-- tests/test_graph_tools_data_flow.py | 9 +++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/docs/adr/ADR-053-graph-tools-tell-the-truth.md b/docs/adr/ADR-053-graph-tools-tell-the-truth.md index f6c1d35..8151a1c 100644 --- a/docs/adr/ADR-053-graph-tools-tell-the-truth.md +++ b/docs/adr/ADR-053-graph-tools-tell-the-truth.md @@ -1,6 +1,6 @@ # ADR-053: Communities Hold Only Repo Nodes, and Data Flow Has Per-Language Rules -**Status:** proposed +**Status:** accepted **Date:** 2026-10-20 **Branch:** `feature/adr-053-graph-tool-accuracy` **Reviewer:** @edb @@ -248,3 +248,12 @@ and `l5x_fixture_teeth --check` exit 0. 4. **Heritage binding in the resolver.** If another reader needs bound EXTENDS/IMPLEMENTS edges, move the binding from `get_graph_edges()` into `call_resolver` and store it as `resolved_target`. + +**Answers, 2026-11-02** (@edb: "all recommended", after a walk-through with an example for each): +1. **No layer rules.** The bucket is the answer. The Next.js path labels are now printed only for + TS/JS files (`_layer_tag`), so a Python or C# entry no longer says `[UTILITY]` or + `[CORE LOGIC / LIB]`. Bucket order is unchanged; test added. +2. **Moot.** ADR-052 merged `.gs` scanning, and the Apps Script rules cover it. +3. **Not in this ADR.** Filed as a backlog item, with GanttWebApp's `recordAudit(...)` writing + `audit_events` as the example the adapter rules can't see. +4. **Deferred** until another reader needs bound heritage edges. diff --git a/src/MCPServer.py b/src/MCPServer.py index 61ae297..09e81fa 100644 --- a/src/MCPServer.py +++ b/src/MCPServer.py @@ -864,6 +864,20 @@ def _trace_layer(file_path: str, doc_text: str) -> str: return layer +def _layer_tag(file_path: str, layer: str) -> str: + """``"[LAYER] "`` for a TS/JS file, else ``""``. + + The layer path tests are Next.js/Firestore conventions, so on a Python or C# file + they say nothing true (ADR-053 open question 1). The bucket carries the role. + """ + from adapters import get_adapter + from adapters.ts_adapter import _WebAdapter + adapter = get_adapter(os.path.splitext(file_path)[1].lower()) + if adapter is not None and not isinstance(adapter, _WebAdapter): + return "" + return f"[{layer}] " + + def _data_flow_rules(names: list[str], file_texts: dict[str, str]) -> dict[str, tuple[list, list]]: """``{file: (definition_patterns, producer_patterns)}`` for every file whose adapter has ``data_flow_rules()`` and whose text passes the rules' ``file_marker`` gate. @@ -906,7 +920,7 @@ def _rule_entry(layer: str, file_path: str, doc: dict, match, tag: str = "") -> text = doc['text'] start = max(0, match.start() - 40) snippet = text[start:start + 140].replace('\n', ' ').strip() + "..." - return f"- [{layer}] {file_path}\n [Scope]: {get_clean_scope(doc)}\n{tag} [Snippet]: {snippet}\n\n" + return f"- {_layer_tag(file_path, layer)}{file_path}\n [Scope]: {get_clean_scope(doc)}\n{tag} [Snippet]: {snippet}\n\n" @_tool() @@ -1026,7 +1040,7 @@ def trace_data_flow(target_symbol: str) -> str: # FIX 1 applied here (Scope Cleanup) clean_scope = get_clean_scope({'scope': c.scope, 'text': c.text, 'file': c.file}) snippet = doc_text[:140].replace('\n', ' ').strip() + "..." - entry = f"- [{layer}] {file_path}\n [Scope]: {clean_scope}\n{violation_tag} [Snippet]: {snippet}\n\n" + entry = f"- {_layer_tag(file_path, layer)}{file_path}\n [Scope]: {clean_scope}\n{violation_tag} [Snippet]: {snippet}\n\n" if rule_hit is not None: entry = _rule_entry(layer, file_path, {'scope': c.scope, 'text': c.text, 'file': c.file}, rule_hit, violation_tag) diff --git a/tests/test_graph_tools_data_flow.py b/tests/test_graph_tools_data_flow.py index 73849cb..289efee 100644 --- a/tests/test_graph_tools_data_flow.py +++ b/tests/test_graph_tools_data_flow.py @@ -103,6 +103,15 @@ def test_chunk_summaries_finds_its_definition_and_its_producer(monkeypatch, tmp_ assert "CodeDB.get_cached_summaries" in b["CONSUMERS"] +def test_python_entries_carry_no_nextjs_layer_label(monkeypatch, tmp_path): + # ADR-053 open question 1: the layer path tests are web conventions; a Python entry + # says only its bucket, not a made-up "[UTILITY]" or "[CORE LOGIC / LIB]". + _wire(monkeypatch, tmp_path, PY_CHUNKS) + out = M.trace_data_flow("chunk_summaries") + assert "- src/db.py\n" in out + assert "[UTILITY]" not in out and "[CORE LOGIC / LIB]" not in out + + def test_attribute_subscript_and_mutating_calls_are_producers(monkeypatch, tmp_path): _wire(monkeypatch, tmp_path, PY_CHUNKS) b = _buckets(M.trace_data_flow("chunk_summaries"))