From 2c7f647204640df84d923b2d92b51430d82621bf Mon Sep 17 00:00:00 2001 From: Gang Tao Date: Mon, 21 Sep 2026 10:58:31 +0800 Subject: [PATCH] feat(ingest): derive code kinds and class-qualified names from graph structure (#17) graphify has no class/method kinds, and tpk mapped every non-callable code node to kind=file. On proton-enterprise that is 102k 'file' nodes of which 13k are files: an out-of-class C++ definition (BlockIO Foo::execute() {...}, the dominant form) comes out of graphify non-callable with a bare label, merged into its header declaration -- so InterpreterInsertQuery::execute was stored as ('execute', kind=file), with 73 other 'execute's. Its calls edges were intact. parse_graph_json now reads the edges first and classifies code nodes as function / class / member / file / symbol: _callable_class -> class; a node a class 'defines' with a body in the graph (a file 'contains' it, or it calls) -> function; defined without a body -> member; label == file basename -> file; either end of 'inherits' -> class; otherwise symbol (String, ContextPtr, ...). Methods and members are named Class::name from the owning class. Two functions sharing file::name (overloads) no longer collide on one id. On src/Interpreters (14,202 nodes): function 4523 / symbol 4517 / member 2911 / class 1517 / file 734; every node that makes a call is now a function; 0 id collisions. Agent prompt, Explorer kind filter and docs list the new vocabulary. Takes effect on re-ingest (stale rows are deleted per entry). Co-Authored-By: Claude Fable 5.1 --- docs/FEATURES.md | 2 +- src/tpk/agent.py | 8 +- src/tpk/graphify_runner.py | 160 ++++++++++++++++++++++++---------- tests/test_graphify_runner.py | 121 ++++++++++++++++++++++++- web/src/Explorer.tsx | 10 ++- 5 files changed, 244 insertions(+), 57 deletions(-) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 77e22c7..1a39372 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -40,7 +40,7 @@ Builds the graph from a pinned, versioned corpus. Stored entirely in Timeplus streams — no separate graph database. -- **Entities** (`kg_nodes`): functions, classes, files, documents, concepts — each with kind, name, qualified name, source location, and a summary. +- **Entities** (`kg_nodes`): each with kind, name, qualified name, source location, and a summary. Code kinds are `function` (functions and methods, named `Class::method`), `class`, `member` (a field, or a method that is only declared), `file`, and `symbol` (a bare type/alias reference); doc kinds are `document`, `concept`, `rationale`, `paper`, `image`. graphify has no class/method notion of its own, so tpk derives the code kinds from the graph structure — e.g. an out-of-class C++ definition (`BlockIO Foo::execute() {…}`) is recognised as the method `Foo::execute` via the class's `defines` edge. - **Relationships** (`kg_edges`): calls, imports, containment, and semantic links, each tagged with a confidence bucket (extracted vs. inferred). - **Communities**: clusters of densely connected entities, found by graphify's community detection at ingest — roughly a subsystem. Ids are opaque numbers unique only within one `repo@ref`; `list_communities` labels each with its dominant directories and files. - Node IDs hash a versioned entry key so multiple releases of the same repo coexist without collision. diff --git a/src/tpk/agent.py b/src/tpk/agent.py index ad466ac..8fd4060 100644 --- a/src/tpk/agent.py +++ b/src/tpk/agent.py @@ -106,9 +106,11 @@ def system_prompt(repos) -> str: the code exists — treat a thin result as "the graph doesn't record this," fall back to read_source, and report the partial connections you did find rather than flatly answering "not found". -2. search_entities' `kinds` filter only accepts these exact values: file, - function, document, concept, rationale. There is no "doc", "code", or - "repo" kind — omit `kinds` if unsure rather than guessing a value, and +2. search_entities' `kinds` filter only accepts these exact values: function + (functions AND methods, named `Class::method`), class, member (a class + field or a method that is only declared), symbol (a bare type / alias + reference -- rarely what you want), file, document, concept, rationale. + There is no "doc", "code", "method", or "repo" kind — omit `kinds` if unsure rather than guessing a value, and use `repos` (repo names from the corpus list above) to narrow scope. search_entities requires EVERY word in `query` to match — multi-word queries fail fast if you guess the wrong phrasing. Prefer short, one- diff --git a/src/tpk/graphify_runner.py b/src/tpk/graphify_runner.py index 3b6cc33..0048299 100644 --- a/src/tpk/graphify_runner.py +++ b/src/tpk/graphify_runner.py @@ -28,26 +28,36 @@ Notable differences from a naive "kind"/"name"/"edges" guess: - Edges live under "links", not "edges" (bare networkx node_link_data shape). -- There is no "kind"/"type" field on nodes. Node category is derived from - "file_type" plus the "_callable" flag: a callable code node is a - `function`, a non-callable "code" node is mapped to `file`, and any other - file_type passes through as-is (see the documented vocabulary below). - NOTE: despite the name, `file` is *not* one node per source file in real - graphify output -- it also catches top-level structs/types/vars and - functions the `_callable` heuristic misses (observed on timeplus-cli: - e.g. a shell function labeled "handle_signal()" with `_callable` unset). - Do not assume `file`-kind qualified_name/id is a stable per-file handle. +- There is no "kind"/"type" field on nodes. tpk derives one (CODE_KINDS) from + the node's flags AND its edges -- flags alone misfile a third of a C++ + codebase (#17, measured on proton-enterprise: 102k "file" nodes, 13k real): + * `class` -- `_callable_class`; or a node known only as one end of an + `inherits` edge. + * `function` -- `_callable` ("name()" free function, ".name()" method + written inside its class); OR a non-callable node a class + `defines` whose body is in the graph (a source file + `contains` it, or it makes calls). That second shape is an + OUT-OF-CLASS definition (`BlockIO Foo::execute() {...}`, the + dominant C++ form): graphify merges it into the header + declaration, leaving a bare `execute` label and no + `_callable` flag -- but its `calls` edges are intact. + * `member` -- a class `defines` it and no body is known: a field, or a + method that is only declared (pure virtual / defined in a + file outside the extraction). + * `file` -- the one node whose label is its source file's basename. + * `symbol` -- anything else: bare type / alias / variable references + (`String`, `ContextPtr`, ...). Often very high degree. + Methods and members are named `Class::name` from the owning class's label. - There is no "name" field; the human name is derived from "label" (stripping a trailing "()" for callables). - There is no "qualified_name"/"fqn" field; one is synthesized so it is - stable *and unique* across the graph: `source_file::name` for callables - (`function` kind); `source_file::` for everything - else (`file` kind and all DOC_KINDS). The node's own "id" is used -- - rather than, say, its label or line number -- because it is the one field + stable *and unique* across the graph: `source_file::name` for `function` + nodes (falling back to `...#` when two share it, e.g. + overloads); `source_file::` for everything else. + The node's own "id" is used -- rather than, say, its label or line number -- because it is the one field graphify guarantees is unique per node (it is the dict key in the - node-link graph); label and line number are not (a "file" node can share - a source_file and line with an unrelated node -- see the `file` kind note - below). + node-link graph); label and line number are not (two nodes can share a + source_file, label and line -- see the kind note above). - File location is "source_file" (not "file"/"file_path"), and there is a single "source_location" like "L6" rather than separate line_start/ line_end fields. @@ -241,20 +251,81 @@ def _first(d: dict, keys: list[str], default=""): DOC_KINDS = {"document", "paper", "image", "rationale", "concept"} -def _node_kind_and_name(rn: dict) -> tuple[str, str]: - """Derive (kind, name) from graphify's file_type/_callable/label fields.""" - label = str(_first(rn, ["label", "name", "title"], _first(rn, ["id"], ""))) +# Kinds for source-derived ("code") nodes. graphify itself has no such field: +# they are derived from the node's flags AND its edges (see _classify_code_node). +CODE_KINDS = ("file", "class", "function", "member", "symbol") + + +def _label(rn: dict) -> str: + return str(_first(rn, ["label", "name", "title"], _first(rn, ["id"], ""))) + + +class _Structure: + """What the EDGES say about each raw node -- needed because graphify's node + flags alone misfile a third of a C++ codebase (#17): an out-of-class method + definition (`BlockIO Foo::execute() {...}`, the dominant C++ form) comes out + NON-callable with a bare label, merged into its header declaration.""" + + def __init__(self, raw_nodes: list[dict], raw_edges: list[dict]): + self.owner: dict[str, str] = {} # member id -> id of the class that defines it + self.contained: set[str] = set() # a source file `contains` it (it has a body there) + self.calls_out: set[str] = set() + self.in_hierarchy: set[str] = set() # either end of an `inherits` edge + self.label = {str(_first(rn, ["id", "name"])): _label(rn) for rn in raw_nodes} + for re_ in raw_edges: + src = str(_first(re_, ["source", "src", "from"])) + dst = str(_first(re_, ["target", "dst", "to"])) + rel = str(_first(re_, ["relation", "rel", "type", "label"], "")).lower() + if rel in ("defines", "method"): + self.owner.setdefault(dst, src) + elif rel == "contains": + self.contained.add(dst) + elif rel in ("calls", "indirect_call"): + self.calls_out.add(src) + elif rel in ("inherits", "extends"): + self.in_hierarchy.update((src, dst)) + + def qualify(self, raw_id: str, name: str) -> str: + """`execute` -> `InterpreterInsertQuery::execute` when a class owns it.""" + owner = self.label.get(self.owner.get(raw_id, ""), "") + if owner and "::" not in name: + return f"{owner}::{name}" + return name + + +def _node_kind_and_name(rn: dict, structure: "_Structure | None" = None) -> tuple[str, str]: + """Derive (kind, name) from graphify's flags plus the graph structure.""" + label = _label(rn) file_type = str(_first(rn, ["file_type", "kind", "type"], "code")).lower() - callable_ = bool(rn.get("_callable", False)) - - if callable_: + if file_type != "code" and not rn.get("_callable"): + # document/paper/image/rationale/concept (or any other non-"code" value): + # pass the documented file_type through as the node kind unchanged. + return file_type or "entity", label + return _classify_code_node(rn, label, structure or _Structure([], [])) + + +def _classify_code_node(rn: dict, label: str, st: "_Structure") -> tuple[str, str]: + raw_id = str(_first(rn, ["id", "name"])) + if rn.get("_callable_class"): + return "class", label + if rn.get("_callable"): + # "name()" free function, or ".name()" for a method written inside its class. name = label[:-2] if label.endswith("()") else label - return "function", name - if file_type == "code": - return "file", label - # document/paper/image/rationale/concept (or any other non-"code" value): - # pass the documented file_type through as the node kind unchanged. - return file_type or "entity", label + return "function", st.qualify(raw_id, name.lstrip(".")) + source_file = str(_first(rn, ["source_file", "file_path", "file", "path"], "")) + if source_file and label == source_file.rsplit("/", 1)[-1]: + return "file", label # the node that IS the file + if raw_id in st.owner: + # A class member. It is a method when its body is in the graph (a .cpp + # `contains` it, or it makes calls); otherwise a field -- or a method + # that is only declared here (pure virtual / defined elsewhere). + has_body = raw_id in st.contained or raw_id in st.calls_out + return ("function" if has_body else "member"), st.qualify(raw_id, label) + if raw_id in st.in_hierarchy: + return "class", label # known only as a base/derived class + if raw_id in st.calls_out: + return "function", label + return "symbol", label # bare type / alias / variable reference def _parse_line(rn: dict) -> int: @@ -277,33 +348,30 @@ def parse_graph_json( raw_nodes = data.get("nodes", []) raw_edges = data.get("edges") or data.get("links") or [] + structure = _Structure(raw_nodes, raw_edges) + seen_function_names: set[str] = set() nodes: list[Node] = [] id_map: dict[str, str] = {} # graphify id -> stable tpk id for rn in raw_nodes: raw_id = str(_first(rn, ["id", "name"])) - kind, name = _node_kind_and_name(rn) + kind, name = _node_kind_and_name(rn, structure) file_path = str(_first(rn, ["source_file", "file_path", "file", "path"])) - if kind == "file": - # "file" is not actually one node per file: graphify emits many - # non-callable "code" nodes per file (structs, top-level vars, - # shell functions missed by the `_callable` heuristic, ...), all - # mapped to kind="file" by _node_kind_and_name above. A bare - # `file_path` qualified_name collapsed all of them onto one node - # id (mutable-stream upsert silently dropped the rest -- verified - # against real timeplus-cli output: 473 parsed nodes down to 99 - # stored rows). Disambiguate with the node's own graphify id, - # which is guaranteed unique within one graph.json (it is the - # node-link graph's own dict key) -- same pattern as the - # DOC_KINDS branch below, which never collided. - base = f"{file_path}::{raw_id}" if file_path else raw_id - qualified = str(_first(rn, ["qualified_name", "qualifiedName", "fqn"], base)) - elif kind == "function": + if kind == "function": + # `file::Class::method` -- stable across runs and human-meaningful. + # Collisions (overloads, or a graphify node split across decl/def) + # fall back to the raw graphify id, which is unique per graph.json. base = f"{file_path}::{name}" if file_path else name - qualified = str(_first(rn, ["qualified_name", "qualifiedName", "fqn"], base)) + if base in seen_function_names: + base = f"{base}#{raw_id}" + seen_function_names.add(base) else: + # Everything else is keyed by graphify's own node id: many such + # nodes share a file AND a label (fields, symbols, doc fragments), + # and a bare `file::label` collapsed them onto one row (verified on + # real timeplus-cli output: 473 parsed nodes -> 99 stored rows). base = f"{file_path}::{raw_id}" if file_path else raw_id - qualified = str(_first(rn, ["qualified_name", "qualifiedName", "fqn"], base)) + qualified = str(_first(rn, ["qualified_name", "qualifiedName", "fqn"], base)) line = _parse_line(rn) stable = node_id(repo, kind, qualified) diff --git a/tests/test_graphify_runner.py b/tests/test_graphify_runner.py index 1265841..b46eba3 100644 --- a/tests/test_graphify_runner.py +++ b/tests/test_graphify_runner.py @@ -112,8 +112,8 @@ def test_inferred_confidence_stays_inferred(tmp_path: Path): def test_file_kind_nodes_in_same_file_get_distinct_ids(tmp_path: Path): - # Real graphify output maps every non-callable "code" node to kind="file" - # -- not just the one node representing the file itself. Two such nodes + # Real graphify output emits many non-callable "code" nodes per file, not + # just the one node representing the file itself. Two such nodes # in the same source_file (e.g. two top-level structs, or a shell # function the `_callable` heuristic missed) must not collapse onto the # same node id, or the mutable-stream upsert silently drops one of them. @@ -136,13 +136,128 @@ def test_file_kind_nodes_in_same_file_get_distinct_ids(tmp_path: Path): ) nodes, _ = parse_graph_json(g, repo="r", default_visibility="internal") assert len(nodes) == 2 - assert all(n.kind == "file" for n in nodes) + # only the node that IS the file is a "file"; the struct is a "symbol" (#17) + assert {n.name: n.kind for n in nodes} == {"common.go": "file", "Topology": "symbol"} ids = {n.id for n in nodes} assert len(ids) == 2, "two distinct file-kind nodes in one file must not share an id" qualified_names = {n.qualified_name for n in nodes} assert len(qualified_names) == 2 +# -- kinds and names from graph structure (#17) ------------------------------- +# graphify has no class/method kinds: a class is `_callable` + `_callable_class`, +# an in-class inline method is callable with a ".name()" label, and an +# OUT-OF-CLASS definition (`BlockIO Foo::execute() {...}` -- the dominant C++ +# form) comes out NON-callable with a BARE label, merged into the header +# declaration. The shapes below are copied from real proton-enterprise output +# (src/Interpreters/InterpreterInsertQuery.{h,cpp}). + +def _cpp_graph(tmp_path: Path) -> Path: + H, CPP = "src/I/Insert.h", "src/I/Insert.cpp" + code = {"file_type": "code"} + g = tmp_path / "graph.json" + g.write_text(json.dumps({ + "nodes": [ + {"id": "h", "label": "Insert.h", "source_file": H, "source_location": "L1", **code}, + {"id": "cpp", "label": "Insert.cpp", "source_file": CPP, "source_location": "L1", **code}, + {"id": "cls", "label": "InterpreterInsertQuery", "source_file": H, "source_location": "L20", + "_callable": True, "_callable_class": True, **code}, + {"id": "base", "label": "IInterpreter", "source_file": H, "source_location": "L20", **code}, + {"id": "ctor", "label": "InterpreterInsertQuery::InterpreterInsertQuery()", "source_file": CPP, + "source_location": "L46", "_callable": True, **code}, + {"id": "inline", "label": ".supportsTransactions()", "source_file": H, "source_location": "L66", + "_callable": True, **code}, + {"id": "exec", "label": "execute", "source_file": H, "source_location": "L36", **code}, + {"id": "decl_only", "label": "getTable", "source_file": H, "source_location": "L63", **code}, + {"id": "field", "label": "query_ptr", "source_file": H, "source_location": "L71", **code}, + {"id": "free", "label": "isTrivialSelect()", "source_file": CPP, "source_location": "L183", + "_callable": True, **code}, + {"id": "alias", "label": "String", "source_file": H, "source_location": "L5", **code}, + ], + "links": [ + {"source": "h", "target": "cls", "relation": "contains"}, + {"source": "cls", "target": "base", "relation": "inherits"}, + {"source": "cls", "target": "inline", "relation": "method"}, + {"source": "cls", "target": "exec", "relation": "defines"}, + {"source": "cpp", "target": "exec", "relation": "contains"}, + {"source": "exec", "target": "free", "relation": "calls"}, + {"source": "cls", "target": "decl_only", "relation": "defines"}, + {"source": "cls", "target": "field", "relation": "defines"}, + {"source": "cpp", "target": "ctor", "relation": "contains"}, + {"source": "cpp", "target": "free", "relation": "contains"}, + {"source": "exec", "target": "alias", "relation": "references"}, + ], + })) + return g + + +def test_kinds_are_derived_from_graph_structure(tmp_path: Path): + nodes, _ = parse_graph_json(_cpp_graph(tmp_path), repo="r", default_visibility="internal") + assert {n.name: n.kind for n in nodes} == { + "Insert.h": "file", + "Insert.cpp": "file", + "InterpreterInsertQuery": "class", + "IInterpreter": "class", # only known as a base class + "InterpreterInsertQuery::InterpreterInsertQuery": "function", + "InterpreterInsertQuery::supportsTransactions": "function", # ".name()" inline method + "InterpreterInsertQuery::execute": "function", # out-of-class definition + "InterpreterInsertQuery::getTable": "member", # declared, body not in the graph + "InterpreterInsertQuery::query_ptr": "member", # field + "isTrivialSelect": "function", + "String": "symbol", # bare type reference + } + + +def test_out_of_class_method_keeps_its_calls_and_is_searchable_by_class(tmp_path: Path): + nodes, edges = parse_graph_json(_cpp_graph(tmp_path), repo="r", default_visibility="internal") + by_name = {n.name: n for n in nodes} + execute = by_name["InterpreterInsertQuery::execute"] + assert "InterpreterInsertQuery::execute" in execute.qualified_name + calls = {(e.src, e.dst) for e in edges if e.rel == "calls"} + assert (execute.id, by_name["isTrivialSelect"].id) in calls + + +def test_reclassified_nodes_keep_distinct_ids(tmp_path: Path): + nodes, _ = parse_graph_json(_cpp_graph(tmp_path), repo="r", default_visibility="internal") + assert len({n.id for n in nodes}) == len(nodes) == 11 + + +def test_same_method_name_in_two_classes_does_not_collide(tmp_path: Path): + g = tmp_path / "graph.json" + code = {"file_type": "code", "source_file": "a.h", "source_location": "L1"} + g.write_text(json.dumps({ + "nodes": [ + {"id": "A", "label": "A", "_callable": True, "_callable_class": True, **code}, + {"id": "B", "label": "B", "_callable": True, "_callable_class": True, **code}, + {"id": "a_exec", "label": "execute", **code}, + {"id": "b_exec", "label": "execute", **code}, + ], + "links": [ + {"source": "A", "target": "a_exec", "relation": "defines"}, + {"source": "B", "target": "b_exec", "relation": "defines"}, + {"source": "a_exec", "target": "b_exec", "relation": "calls"}, + ], + })) + nodes, edges = parse_graph_json(g, repo="r", default_visibility="internal") + assert sorted(n.name for n in nodes if n.kind == "function") == ["A::execute"] + assert sorted(n.name for n in nodes if n.kind == "member") == ["B::execute"] + assert len({n.id for n in nodes}) == 4 and len(edges) == 3 + + +def test_overloaded_functions_in_one_file_keep_distinct_ids(tmp_path: Path): + # Two overloads share `file::name`; the mutable-stream upsert would silently + # keep only one (~3% of proton's functions). The second falls back to the + # graphify id, so both survive and the first keeps its readable name. + g = tmp_path / "graph.json" + code = {"file_type": "code", "source_file": "a.cpp", "source_location": "L1", "_callable": True} + g.write_text(json.dumps({"nodes": [{"id": "f_int", "label": "f()", **code}, + {"id": "f_str", "label": "f()", **code}], "links": []})) + nodes, _ = parse_graph_json(g, repo="r", default_visibility="internal") + assert [n.name for n in nodes] == ["f", "f"] + assert sorted(n.qualified_name for n in nodes) == ["a.cpp::f", "a.cpp::f#f_str"] + assert len({n.id for n in nodes}) == 2 + + class _FakePopen: """Stand-in for subprocess.Popen: yields canned stdout lines, fabricates graph.json under --out, and reports returncode.""" diff --git a/web/src/Explorer.tsx b/web/src/Explorer.tsx index 01c8a6f..5004050 100644 --- a/web/src/Explorer.tsx +++ b/web/src/Explorer.tsx @@ -11,10 +11,12 @@ import { type SourceResponse, } from "./graph"; -// Real kind vocabulary emitted by the graphify pipeline (src/tpk/ -// graphify_runner.py's _node_kind_and_name / DOC_KINDS) -- "class" etc in -// the mockup are illustrative, not an actual value this backend produces. -const KIND_OPTIONS = ["function", "file", "document", "paper", "image", "rationale", "concept"]; +// Real kind vocabulary emitted by the ingest pipeline (src/tpk/ +// graphify_runner.py: CODE_KINDS + DOC_KINDS). Code kinds are derived from the +// graph structure (#17): "member" = a class field or declared-only method, +// "symbol" = a bare type / alias / variable reference. +const KIND_OPTIONS = ["function", "class", "member", "symbol", "file", + "document", "paper", "image", "rationale", "concept"]; // Radial subgraph layout caps at this many neighbor nodes before folding // the rest into a single dashed "...N more" node (mockup screen 1d).