From d1b4034ef3a85f3a31bec0753b38015abea6aa36 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Thu, 20 Aug 2026 15:35:56 -0400 Subject: [PATCH 1/3] feat(schema)!: converge call_sites[] into the body{} call node (#120) Every call site was emitted twice. `l1_body.py` derives each `body{}` call node from `PyCallable.call_sites`, so the same fact shipped under two unrelated identity schemes -- `body{}` keyed by the ordinal id `@line:col`, and `call_sites[]` carrying no id at all. The redundancy was not v1 debris: the list is the analyzer's internal working record, and it leaked into the wire. `call_sites` is now `Field(..., exclude=True)`. It stays in memory, because `semantic_analysis/call_graph.py`, `schema/l2_callees.py` and `dataflow/builder.py` are all built on it -- it is the input to L2 resolution and to L3/L4 dataflow, not a leftover. It simply stops being serialized, and `body{}` becomes the single emitted representation of a call site. The detail the list carried moves onto the call node: `method_name`, `receiver_expr`, `receiver_type`, `return_type`, `is_constructor_call` and the structured `arguments`. `method_name` and `is_constructor_call` are carried rather than derived from `callee`, reversing an earlier draft. Measured: 27.8% of call sites in the `requests` fixture and 19.8% in `flask` never resolve a callee, every one of them because Jedi could not infer the receiver type of an attribute call. The call graph cannot close that gap either -- a `call_graph` edge is `{src, dst, prov, weight}`, callable-to-callable with no call-site position, so it can never refine a specific body node. Deriving those two fields would lose them on one call in four. `accessed_symbols[]` and `local_variables[]` are deliberately untouched, though #120 groups all three and codeanalyzer-typescript drops all three. Only `call_sites[]` is redundant: `BodyNode` carries kind/span/callee/of/parent and has no representation of accessed symbols or local variables at any level, so dropping those would delete data rather than deduplicate it. Converging them requires first deciding what represents them. BREAKING CHANGE: `PyCallable.call_sites` no longer appears in `analysis.json`. Consumers read `body{}` entries with `kind == "call"`, which carry the same detail plus a resolved `callee` id. --- codeanalyzer/schema/l1_body.py | 12 +- codeanalyzer/schema/py_schema.py | 20 +- .../specs/call-site-body-convergence.md | 172 ++++++++++++++++++ test/test_call_site_body_convergence.py | 70 +++++++ 4 files changed, 271 insertions(+), 3 deletions(-) create mode 100644 docs/design/specs/call-site-body-convergence.md create mode 100644 test/test_call_site_body_convergence.py diff --git a/codeanalyzer/schema/l1_body.py b/codeanalyzer/schema/l1_body.py index 3c6f781..04153e8 100644 --- a/codeanalyzer/schema/l1_body.py +++ b/codeanalyzer/schema/l1_body.py @@ -9,7 +9,17 @@ def _do_callable(source: str, c: PyCallable) -> None: span = Span(start=(cs.start_line, cs.start_column), end=(cs.end_line, cs.end_column), bytes=byte_offsets(source, cs.start_line, cs.start_column, cs.end_line, cs.end_column)) if source else None - c.body[key] = BodyNode(kind="call", span=span, callee=None) + c.body[key] = BodyNode( + kind="call", + span=span, + callee=None, + method_name=cs.method_name, + receiver_expr=cs.receiver_expr, + receiver_type=cs.receiver_type, + return_type=cs.return_type, + is_constructor_call=cs.is_constructor_call, + arguments=list(cs.arguments or []), + ) for ic in (c.callables or {}).values(): _do_callable(source, ic) for icl in (c.types or {}).values(): diff --git a/codeanalyzer/schema/py_schema.py b/codeanalyzer/schema/py_schema.py index 90e5a56..2661c3b 100644 --- a/codeanalyzer/schema/py_schema.py +++ b/codeanalyzer/schema/py_schema.py @@ -22,7 +22,7 @@ from __future__ import annotations from pathlib import Path from typing import Any, Dict, List, Optional, Tuple -from pydantic import BaseModel +from pydantic import BaseModel, Field from typing_extensions import Literal @@ -132,6 +132,18 @@ class BodyNode(BaseModel): callee: Optional[str] = None # only on `call` nodes; the sanctioned null→id slot of: Optional[str] = None # param vertices: the variable/return they carry parent: Optional[str] = None # actuals: owning callsite ordinal id + # Call-site detail (#120). Previously reachable only through the parallel + # `PyCallable.call_sites` list, which emitted the same fact a second time under + # an unrelated id scheme. `method_name` and `is_constructor_call` are carried + # rather than derived from `callee`: measured across `requests` and `flask`, + # 20-28% of call sites never resolve a callee, so deriving them would lose them + # on one call in four. + method_name: Optional[str] = None + receiver_expr: Optional[str] = None + receiver_type: Optional[str] = None + return_type: Optional[str] = None + is_constructor_call: Optional[bool] = None + arguments: List["PyCallArgument"] = [] @builder @@ -334,7 +346,11 @@ class PyCallable(BaseModel): end_line: int = -1 code_start_line: int = -1 accessed_symbols: List[PySymbol] = [] - call_sites: List[PyCallsite] = [] + # Internal only (#120): the Jedi-produced record that `l1_body` derives `body{}` + # call nodes from, and that `call_graph.py`, `l2_callees.py` and the dataflow + # builder all read. Kept in memory, excluded from the wire -- `body{}` is the + # single emitted representation of a call site. + call_sites: List[PyCallsite] = Field(default_factory=list, exclude=True) callables: Dict[str, "PyCallable"] = {} # nested callables (closures) types: Dict[str, "PyClass"] = {} # nested (local) classes local_variables: List[PyVariableDeclaration] = [] diff --git a/docs/design/specs/call-site-body-convergence.md b/docs/design/specs/call-site-body-convergence.md new file mode 100644 index 0000000..aaad7a5 --- /dev/null +++ b/docs/design/specs/call-site-body-convergence.md @@ -0,0 +1,172 @@ +# Spec: converge `call_sites[]` into `body{}` — separating the IR from the wire format + +Status: draft for review +Date: 2026-08-19 +Scope: `codeanalyzer-python` schema v2, `analysis.json` + Neo4j projection +Related: #120 (converge `call_sites[]`/`accessed_symbols[]`/`local_variables[]` with `body{}`) + +--- + +## 1. The finding + +Every call site is emitted **twice**, under two unrelated identity schemes. +Verified on `main` (`6f02581`), fixture `return cls()` at line 34: + +| representation | id | Neo4j | +| --- | --- | --- | +| `PyCallable.call_sites[]` | `app.py#34:15-34:22` | `:PyCallSite` ← `PY_HAS_CALLSITE` | +| `PyCallable.body{"34:15"}`, `kind:"call"` | `@34:15` | `:PyCFGNode` ← `PY_HAS_CFG_NODE` | + +One call in source, two graph nodes, no edge between them. `PyCallSite`'s id +(`file#line:col-line:col`) belongs to neither identity tier the schema defines — +it is neither a durable `can://` id nor a `@` ordinal id. + +**They cannot disagree in content.** `schema/l1_body.py` derives one from the other +in the same pass: + +```python +for cs in c.call_sites or []: + key = f"{cs.start_line}:{cs.start_column}" + c.body[key] = BodyNode(kind="call", span=span, callee=None) +``` + +So this is not a correctness bug. It is redundancy by construction. + +## 2. Why it exists + +`call_sites[]` is **not** v1 debris left lying around. It is the analyzer's internal +working record, and it is load-bearing for every level above L1: + +| reader | uses | +| --- | --- | +| `semantic_analysis/call_graph.py:163-215` | `callee_signature`, `method_name`, `is_constructor_call` — builds the L2 call graph | +| `schema/l2_callees.py` | `callee_signature`, `start_line`, `start_column` — backfills `BodyNode.callee` | +| `dataflow/builder.py:368-420` | `callee_signature`, `is_constructor_call`, position — builds SDG call sites | +| `dataflow/summaries.py`, `dataflow/sdg.py` | consume the above | +| `neo4j/project.py:400` | projects `:PyCallSite` | + +`body{}` is the v2 wire view *derived from* that record. The duplication is an +**internal IR leaking into the wire format** — not two competing encodings of equal +standing. That reframing is what makes the fix tractable: the wire format can lose +`call_sites[]` without the internal passes losing anything, provided the record +survives as an internal structure. + +## 3. Design + +**`body{}` is the single emitted representation of a call site.** Consumers obtain the +call-site set by filtering `body` on `kind == "call"`, which works from **L1** — verified: + +``` +L1 body{"34:15"} kind=call callee=null +L2 body{"34:15"} kind=call callee="can://…/@external/app.Account/__init__" +``` + +The Jedi-produced call record stays **internal**: it is the input to L2 resolution and +L3/L4 dataflow, and is not part of the contract. `PyCallsite` leaves the emitted schema. + +### Field disposition + +| field | disposition | why | +| --- | --- | --- | +| `start_line` / `start_column` / `end_*` | **becomes the node key + `span`** | already how `body{}` is keyed | +| `callee_signature` | **internal only** | it is the *input* to resolution; `callee` (a resolved `can://` id) is what the wire carries | +| `is_constructor_call` | **moves onto the call `BodyNode`** | see § 4a — not recoverable often enough to drop | +| `method_name` | **moves onto the call `BodyNode`** | see § 4a — not recoverable often enough to drop | +| `argument_types` | **deleted** | deprecated in 0.3.1 (#86) with "will be removed in schema v2"; no internal reader; removal overdue | +| `arguments` | **moves onto the call `BodyNode`** | no internal reader; output-only detail worth keeping. Encoding is OPEN — see § 5 | +| `receiver_expr` | **moves onto the call `BodyNode`** | no internal reader; Jedi inference with no other home | +| `receiver_type` | **moves onto the call `BodyNode`** | as above | +| `return_type` | **moves onto the call `BodyNode`** | as above | + +### 4a. Why `method_name` and `is_constructor_call` stay on the wire + +An earlier draft had both as "derivable from `callee`". Measurement killed that. Callee +resolution fails far too often for it to be a reliable base: + +| corpus | call body nodes | `callee` resolved | `callee` null | +| --- | --- | --- | --- | +| `requests` | 842 | 72.2% | **27.8%** | +| `flask` | 1129 | 80.2% | **19.8%** | + +Every one of those unresolved sites has `callee_signature is None` — Jedi genuinely failed on an +attribute call whose receiver type it cannot infer (`jar.extract_cookies`, `new_jar.clear`). The +gap is not a backfill bug: `l2_callees.py` propagates every resolution it is given, and zero sites +had a resolved signature that failed to reach its body node. + +Nor can the call graph close it. PyCG supplies 1337 of 1862 call-graph edges in `requests` (71.8%), +but a `call_graph` edge is `{src, dst, prov, weight}` — callable-to-callable, with no call-site +position — so it can never refine a specific `body{}` node. `body.callee` is bounded by Jedi's +call-site-level resolution. + +So dropping `method_name` and `is_constructor_call` would lose them on one call in four. +`is_constructor_call` was `True` for **zero** unresolved sites in both corpora, so it is the +cheaper of the two to reconsider later — but on this evidence both stay. + +### Resulting `BodyNode` + +```python +class BodyNode(BaseModel): + kind: str # statement | call | entry | exit | formal_* | actual_* + span: Optional[Span] = None + callee: Optional[str] = None # call nodes; the sanctioned null→id slot at L2 + of: Optional[str] = None + parent: Optional[str] = None + # call-specific, Jedi inference, absent on every other kind + receiver_expr: Optional[str] = None + receiver_type: Optional[str] = None + return_type: Optional[str] = None +``` + +### Neo4j consequences + +- `:PyCallSite`, `PY_HAS_CALLSITE`, and the `file#line:col-line:col` id scheme are **removed**. +- One body-node label, keyed on the global ordinal id, carries every `body{}` entry. +- `PY_RESOLVES_TO` is re-sourced from `BodyNode.callee` instead of `callee_signature`. +- Merge groups drop from 9 to 8. +- **Rename the body label.** `PyCFGNode` names an L3 concept, but a `call` node exists from + L1 and is deliberately **never** on the CFG spine — verified: at L3 the call `34:15` carries + `parent="34:8"` and appears in no `cfg` edge, while `@entry`/`34:8`/`@exit` do. The current + label asserts CFG membership for a node that has none. A level-neutral name (`PyBodyNode`, + matching what `body{}` is called in the JSON) states what is true at every level. +- If `MATCH (:PyCallSite)` ergonomics are wanted, the writer already supports label layering + (`rows.py:98` merges on `labels[0]` and unions the rest; `cypher.py:95-97` renders it), so a + marker label costs no second node and no second id. Note `MARKER_LABELS` in `neo4j/schema.py` + is declaration-only today — the writer never reads it, and no call site passes >1 label. + +## 4. What this does not do + +- **Does not touch `accessed_symbols[]` or `local_variables[]`**, and that is a decision, not an + omission. #120 groups all three, and `codeanalyzer-typescript` drops all three from its wire. + But only `call_sites[]` is *redundant*: `l1_body.py` derives `body{}` call nodes from it, so the + same fact is emitted twice. `BodyNode` carries `kind`/`span`/`callee`/`of`/`parent` and nothing + else — there is no representation of accessed symbols or local variables in `body{}` at any + level. Dropping those two would delete data rather than deduplicate it. Converging them means + first deciding what represents them, which is a separate design question. +- Does not change the call graph, Jedi resolution, or any dataflow analysis — only which + representation is serialized. +- Does not settle the Neo4j merge-label strategy, the `can://` callable-signature grammar, or + the decorator shape (#128). Those are separate decisions. + +## 5. Open questions + +- **`arguments` encoding.** Inline objects (`{ast_kind, inferred_type}`, what Python does now + and what works at L1) or local-ids referencing argument body nodes. The latter requires + materializing argument nodes at L1, which is a much larger change to the body model and the + id space. Recommendation: keep inline. +- **Body label name.** `PyBodyNode` is the obvious candidate; anything level-neutral works. +- **Whether the internal record stays a Pydantic model** excluded from serialization, or becomes + a plain dataclass in the analysis passes. Purely internal; no contract impact. + +## 6. Caveats and risks + +- **Breaking for anyone reading `call_sites[]`.** That is the point of the change, but it is the + most visible field in the callable model and its removal should lead the release notes. +- **Three fields become wire-recoverable rather than wire-present** (`method_name`, + `is_constructor_call`, `callee_signature`). Recovering `method_name` from a `span` slice is + string work a consumer may not want to do. If that proves unpopular the honest fix is to put + `method_name` back on the node, not to restore `call_sites[]`. +- **`callee` must actually resolve** for `is_constructor_call` and `method_name` to be + recoverable. Where resolution fails, `callee` is null and both are lost. The size of that + set is unmeasured and should be measured before the fields are dropped. +- **Test surface.** Every test asserting on `call_sites[]` changes. They should be rewritten + against `body{}`, not deleted. diff --git a/test/test_call_site_body_convergence.py b/test/test_call_site_body_convergence.py new file mode 100644 index 0000000..a2db435 --- /dev/null +++ b/test/test_call_site_body_convergence.py @@ -0,0 +1,70 @@ +"""Call-site facts live on the body{} call node, not in a parallel list (#120). + +`l1_body.py` derived `body{}` call nodes from `call_sites[]`, so the analyzer emitted +the same fact twice under two unrelated id schemes. The list stays in memory — it is +the producer that L2 resolution and L3/L4 dataflow read — but it leaves the wire, and +the detail it carried moves onto the call node. +""" +import json + +from codeanalyzer.schema.l1_body import populate_l1_body +from codeanalyzer.schema.py_schema import ( + PyApplication, PyCallable, PyCallArgument, PyCallsite, PyModule, +) + +SRC = "def f():\n return g(1, x=2)\n" + + +def _app() -> tuple[PyApplication, PyCallable]: + fn = PyCallable(name="f", path="a.py", signature="a.f") + fn.call_sites.append( + PyCallsite( + method_name="g", + receiver_expr="obj", + receiver_type="Obj", + return_type="int", + is_constructor_call=False, + arguments=[PyCallArgument(ast_kind="Constant", inferred_type="int")], + start_line=2, start_column=11, end_line=2, end_column=22, + ) + ) + mod = PyModule(file_path="a.py", module_name="a", source=SRC, functions={"f": fn}) + return PyApplication(symbol_table={"a.py": mod}), fn + + +def test_call_detail_lands_on_the_body_node(): + app, fn = _app() + populate_l1_body(app) + (node,) = [n for n in fn.body.values() if n.kind == "call"] + assert node.method_name == "g" + assert node.receiver_expr == "obj" and node.receiver_type == "Obj" + assert node.return_type == "int" + assert node.is_constructor_call is False + assert [a.ast_kind for a in node.arguments] == ["Constant"] + + +def test_call_sites_stays_in_memory_for_the_internal_passes(): + """L2 resolution and L3/L4 dataflow read this list; it must not be emptied.""" + app, fn = _app() + populate_l1_body(app) + assert len(fn.call_sites) == 1 + assert fn.call_sites[0].method_name == "g" + + +def test_call_sites_does_not_reach_the_wire(): + app, fn = _app() + populate_l1_body(app) + emitted = json.loads(app.model_dump_json()) + callable_json = emitted["symbol_table"]["a.py"]["functions"]["f"] + assert "call_sites" not in callable_json + (node,) = [n for n in callable_json["body"].values() if n["kind"] == "call"] + assert node["method_name"] == "g" + + +def test_accessed_symbols_and_local_variables_are_untouched(): + """Only call_sites is redundant; these have no body{} representation to converge into.""" + app, fn = _app() + populate_l1_body(app) + callable_json = json.loads(app.model_dump_json())["symbol_table"]["a.py"]["functions"]["f"] + assert "accessed_symbols" in callable_json + assert "local_variables" in callable_json From 8494f67bb720347faa007b97174d014613796b0c Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Thu, 20 Aug 2026 15:45:55 -0400 Subject: [PATCH 2/3] feat(neo4j)!: collapse :PyCallSite onto the body node (#120) The JSON and graph projections disagreed about what a call site is. After the wire-format convergence, `analysis.json` emits one node per call site -- a `body{}` entry keyed by its ordinal id -- while Neo4j still emitted two: a `:PyCFGNode` keyed on the global ordinal id, and a separate `:PyCallSite` keyed on `app.py#6:11-6:22`, a third identity scheme that is neither a `can://` id nor an ordinal id and that the keystone does not define. The graph now matches the JSON. Call-site detail (`method_name`, `receiver_expr`, `receiver_type`, `return_type`, `is_constructor_call`, `arguments_json`) rides on the `:PyCFGNode` for the call, whose id is the JSON body key prefixed with the callable id. `PY_RESOLVES_TO` re-sources from the body node's resolved `callee` rather than the old `callee_signature` string, so its endpoint is an id rather than a name. `:PyCallSite` and `PY_HAS_CALLSITE` are removed, along with the now-dead `_call_site_props`. Merge groups drop from 9 to 8, which narrows what .github#36 has to freeze when it fixes the merge-label strategy. `_project_program_graphs` now takes `externals` and `sig_to_id`: resolving a callee to a symbol reference needs them, and it previously had neither. That omission raised `NameError` at emit time and was caught by running `--emit neo4j`, not by the suite -- the Bolt tests are deselected in local runs, so nothing covered it. BREAKING CHANGE: the `:PyCallSite` label and `PY_HAS_CALLSITE` relationship no longer exist. Queries traverse `PY_HAS_CFG_NODE` to a node with `kind: "call"`. `PY_RESOLVES_TO` now starts at `:PyCFGNode`. --- codeanalyzer/neo4j/project.py | 55 ++++++++++++----------------------- codeanalyzer/neo4j/schema.py | 32 ++++++-------------- schema.neo4j.json | 40 +++++-------------------- 3 files changed, 35 insertions(+), 92 deletions(-) diff --git a/codeanalyzer/neo4j/project.py b/codeanalyzer/neo4j/project.py index b838b8a..b6c8920 100644 --- a/codeanalyzer/neo4j/project.py +++ b/codeanalyzer/neo4j/project.py @@ -47,7 +47,7 @@ PyModule, PyVariableDeclaration, ) -from codeanalyzer.schema.py_schema import PyCallsite, PyDecorator +from codeanalyzer.schema.py_schema import PyDecorator def project(app: PyApplication, app_name: str, sig_to_id: dict, @@ -98,7 +98,7 @@ def project(app: PyApplication, app_name: str, sig_to_id: dict, # Level-3 CPG overlay: each callable's v2 body/cfg/cdg/ddg. Idempotent under # MERGE — a no-op when no callable carries L3 fields (levels 1/2). - _project_program_graphs(b, app) + _project_program_graphs(b, app, externals, sig_to_id) return b.finish() @@ -128,7 +128,9 @@ def _cfg_ref(callable_id: str, local_key: str) -> NodeRef: return NodeRef("PyCFGNode", "id", _global_ordinal(callable_id, local_key)) -def _project_program_graphs(b: RowBuilder, app: PyApplication) -> None: +def _project_program_graphs( + b: RowBuilder, app: PyApplication, externals: dict, sig_to_id: dict +) -> None: """Level-3 CPG overlay, projected off each callable's v2 ``body``/``cfg``/ ``cdg``/``ddg`` (populated by ``emit_l3_body`` at ``-a 3``; empty otherwise). @@ -173,11 +175,25 @@ def _project_program_graphs(b: RowBuilder, app: PyApplication) -> None: "end_line": span.end[0] if span else None, "var": node.of, "call_node": node.parent, + # Call-site detail (#120). The JSON emits one node per + # call site; the graph now does too, instead of a + # separate :PyCallSite under a third id scheme. + "method_name": node.method_name, + "receiver_expr": node.receiver_expr, + "receiver_type": node.receiver_type, + "return_type": node.return_type, + "is_constructor_call": node.is_constructor_call, + "arguments_json": _stringify_if(node.arguments), "_module": file_key, } ), ) b.edge("PY_HAS_CFG_NODE", owner, ref) + if node.kind == "call" and node.callee: + b.edge_to_symbol( + "PY_RESOLVES_TO", ref, + _symbol_ref(node.callee, externals, sig_to_id), + ) for e in c.cfg or []: # kind-discriminated: a conditional's true/false pair between one # endpoint pair must stay two relationships, not one MERGE. @@ -400,19 +416,6 @@ def _project_callable( for d in c.decorators or []: _project_decorator(b, ref, d) - for s in c.call_sites or []: - # Key off the relative file (a call site lives in its callable's file) so ids stay portable. - cs_id = ( - f"{file_key}#{s.start_line}:{s.start_column}-{s.end_line}:{s.end_column}" - ) - cs = b.node(["PyCallSite"], "id", cs_id, _call_site_props(s, file_key)) - b.edge("PY_HAS_CALLSITE", ref, cs) - if s.callee_signature: - b.edge_to_symbol( - "PY_RESOLVES_TO", cs, - _symbol_ref(s.callee_signature, externals, sig_to_id), - ) - for v in c.local_variables or []: _project_variable(b, file_key, ref, c.signature, v) for ic in (c.callables or {}).values(): @@ -577,26 +580,6 @@ def _variable_props(v: PyVariableDeclaration, var_id: str, file_key: str) -> Pro ) -def _call_site_props(s: PyCallsite, file_key: str) -> Props: - cs_id = f"{file_key}#{s.start_line}:{s.start_column}-{s.end_line}:{s.end_column}" - return prune( - { - "id": cs_id, - "method_name": s.method_name, - "receiver_expr": s.receiver_expr, - "receiver_type": s.receiver_type, - "argument_types": list(s.argument_types or []), - "arguments_json": _stringify_if(s.arguments), - "return_type": s.return_type, - "callee_signature": s.callee_signature, - "is_constructor_call": s.is_constructor_call, - "start_line": s.start_line, - "start_column": s.start_column, - "end_line": s.end_line, - "end_column": s.end_column, - "_module": file_key, - } - ) def _call_edge_props(weight: int, prov: List[str]) -> Props: diff --git a/codeanalyzer/neo4j/schema.py b/codeanalyzer/neo4j/schema.py index a1fd2aa..272c311 100644 --- a/codeanalyzer/neo4j/schema.py +++ b/codeanalyzer/neo4j/schema.py @@ -145,27 +145,6 @@ class RelType: "name", {"name": "string", "qualified_name": "string"}, ), - NodeLabel( - "PyCallSite", - "PyCallSite", - "id", - { - "id": "string", - "method_name": "string", - "receiver_expr": "string", - "receiver_type": "string", - "argument_types": "string[]", - "arguments_json": "string", - "return_type": "string", - "callee_signature": "string", - "is_constructor_call": "boolean", - "start_line": "integer", - "start_column": "integer", - "end_line": "integer", - "end_column": "integer", - "_module": "string", - }, - ), NodeLabel( "PyAttribute", "PyAttribute", @@ -209,6 +188,14 @@ class RelType: "kind": "string", "var": "string", "call_node": "string", + # Call-site detail (#120): the graph emits one node per call site, + # matching analysis.json, instead of a separate :PyCallSite. + "method_name": "string", + "receiver_expr": "string", + "receiver_type": "string", + "return_type": "string", + "is_constructor_call": "boolean", + "arguments_json": "string", **_SPAN, "_module": "string", }, @@ -224,8 +211,7 @@ class RelType: RelType("PY_HAS_METHOD", ["PyClass"], ["PyCallable"]), RelType("PY_HAS_ATTRIBUTE", ["PyClass"], ["PyAttribute"]), RelType("PY_DECLARES_VAR", ["PyModule", "PyCallable"], ["PyVariable"]), - RelType("PY_HAS_CALLSITE", ["PyCallable"], ["PyCallSite"]), - RelType("PY_RESOLVES_TO", ["PyCallSite"], ["PyCallable", "PyExternal"]), + RelType("PY_RESOLVES_TO", ["PyCFGNode"], ["PyCallable", "PyExternal"]), RelType( "PY_CALLS", ["PyCallable", "PyExternal"], diff --git a/schema.neo4j.json b/schema.neo4j.json index aaf97d8..17893b2 100644 --- a/schema.neo4j.json +++ b/schema.neo4j.json @@ -101,27 +101,6 @@ "qualified_name": "string" } }, - { - "label": "PyCallSite", - "merge_label": "PyCallSite", - "key": "id", - "properties": { - "id": "string", - "method_name": "string", - "receiver_expr": "string", - "receiver_type": "string", - "argument_types": "string[]", - "arguments_json": "string", - "return_type": "string", - "callee_signature": "string", - "is_constructor_call": "boolean", - "start_line": "integer", - "start_column": "integer", - "end_line": "integer", - "end_column": "integer", - "_module": "string" - } - }, { "label": "PyAttribute", "merge_label": "PyAttribute", @@ -161,6 +140,12 @@ "kind": "string", "var": "string", "call_node": "string", + "method_name": "string", + "receiver_expr": "string", + "receiver_type": "string", + "return_type": "string", + "is_constructor_call": "boolean", + "arguments_json": "string", "start_line": "integer", "end_line": "integer", "_module": "string" @@ -222,20 +207,10 @@ ], "properties": {} }, - { - "type": "PY_HAS_CALLSITE", - "from": [ - "PyCallable" - ], - "to": [ - "PyCallSite" - ], - "properties": {} - }, { "type": "PY_RESOLVES_TO", "from": [ - "PyCallSite" + "PyCFGNode" ], "to": [ "PyCallable", @@ -386,7 +361,6 @@ "CREATE CONSTRAINT pysymbol_id IF NOT EXISTS FOR (x:PySymbol) REQUIRE x.id IS UNIQUE", "CREATE CONSTRAINT pypackage_name IF NOT EXISTS FOR (x:PyPackage) REQUIRE x.name IS UNIQUE", "CREATE CONSTRAINT pydecorator_name IF NOT EXISTS FOR (x:PyDecorator) REQUIRE x.name IS UNIQUE", - "CREATE CONSTRAINT pycallsite_id IF NOT EXISTS FOR (x:PyCallSite) REQUIRE x.id IS UNIQUE", "CREATE CONSTRAINT pyattribute_id IF NOT EXISTS FOR (x:PyAttribute) REQUIRE x.id IS UNIQUE", "CREATE CONSTRAINT pyvariable_id IF NOT EXISTS FOR (x:PyVariable) REQUIRE x.id IS UNIQUE", "CREATE CONSTRAINT pycfgnode_id IF NOT EXISTS FOR (x:PyCFGNode) REQUIRE x.id IS UNIQUE" From d0084cb3d2993f225b048e8a7e66896448703364 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Thu, 20 Aug 2026 16:18:50 -0400 Subject: [PATCH 3/3] fix(schema): strip call_sites at emit, not with a field exclude (#120) The suite caught four failures on the convergence, two of them real defects in it. `Field(exclude=True)` was the wrong mechanism. `_save_analysis_cache` writes `model_dump_json(analysis)` -- the same call the exclude suppresses -- so `call_sites` vanished from the cache as well as the wire. On a warm cache it came back empty, and since it is the producer that `l1_body`, `l2_callees`, `call_graph.py` and the dataflow builder all read, a second run silently rebuilt with no call nodes, no callee backfill and a thinner graph. Caught by test_v2_l4's byte-identical-across-cache-reuse guard, which exists because of an earlier append-emission bug. Replaced with `strip_internal_only`, applied in `__main__`'s emit path only. The cache keeps the field; `analysis.json` does not. Verified: cold and warm runs are byte-identical and the warm run keeps its call nodes. `PY_RESOLVES_TO` was emitting dangling edges. `_symbol_ref` takes a dotted signature, looks it up in `sig_to_id` and otherwise falls back to matching a `signature` property; it was being handed `BodyNode.callee`, which is already a resolved `can://` id, so the lookup missed and the projection emitted `MATCH (b:PySymbol {signature: })` -- matching nothing at load. Now routed through `_call_endpoint`, which handles an already-resolved endpoint and classifies externals: `MATCH (b:PySymbol {id: row.t})`. Two test fixtures were reaching for the field that left the wire, and both are stand-ins for real consumers: - test_v2_l2's #80 acceptance now reads `body{}` call nodes and compares `method_name` against the callee id's trailing `name(args)` segment. The assertions are unchanged -- only the accessor moved. - test_neo4j_schema's coverage guard failed because `make_sample_app` never ran `backfill_callees`, so body nodes carried no `callee` and PY_RESOLVES_TO never fired. The fixture now mirrors the pipeline, which runs backfill at -a 2+. test_call_site_body_convergence's own wire assertion was rewritten: it pinned the abandoned mechanism. It now pins both halves -- the serialized form keeps `call_sites` (what the cache persists), the emitted form drops it (what analysis.json carries). --- codeanalyzer/__main__.py | 17 ++++++++++++--- codeanalyzer/neo4j/project.py | 12 ++++++++--- codeanalyzer/schema/__init__.py | 27 ++++++++++++++++++++++++ codeanalyzer/schema/py_schema.py | 12 ++++++----- test/sample_graph_app.py | 5 +++++ test/test_call_site_body_convergence.py | 22 ++++++++++++++++--- test/test_v2_l2.py | 23 ++++++++++++++------ test/test_v2_two_projection_agreement.py | 6 ++++++ 8 files changed, 104 insertions(+), 20 deletions(-) diff --git a/codeanalyzer/__main__.py b/codeanalyzer/__main__.py index 388edb4..41e82b6 100644 --- a/codeanalyzer/__main__.py +++ b/codeanalyzer/__main__.py @@ -1,3 +1,4 @@ +import json import os import sys from importlib.metadata import version as _pkg_version, PackageNotFoundError @@ -38,7 +39,7 @@ def _pin_hash_seed() -> None: from codeanalyzer.core import Codeanalyzer from codeanalyzer.utils import _set_log_level, logger from codeanalyzer.config import OutputFormat -from codeanalyzer.schema import model_dump_json +from codeanalyzer.schema import model_dump_json, strip_internal_only from codeanalyzer.options import AnalysisOptions, EmitTarget, ShardStrategy @@ -451,7 +452,13 @@ def main( emit_neo4j(artifacts, options) elif options.output is None: - print(model_dump_json(artifacts, exclude_none=True)) + print( + json.dumps( + strip_internal_only( + artifacts.model_dump(mode="json", exclude_none=True) + ) + ) + ) else: options.output.mkdir(parents=True, exist_ok=True) _write_output(artifacts, options.output, options.format) @@ -462,7 +469,11 @@ def _write_output(artifacts, output_dir: Path, format: OutputFormat): if format == OutputFormat.JSON: output_file = output_dir / "analysis.json" # Use Pydantic's model_dump_json() for compact output - json_str = model_dump_json(artifacts, indent=None, exclude_none=True) + # Strip internal-only fields here rather than with a field-level Pydantic + # `exclude`: the analysis cache shares the serializer and must keep them. + json_str = json.dumps( + strip_internal_only(artifacts.model_dump(mode="json", exclude_none=True)) + ) with output_file.open("w") as f: f.write(json_str) logger.info(f"Analysis saved to {output_file}") diff --git a/codeanalyzer/neo4j/project.py b/codeanalyzer/neo4j/project.py index b6c8920..33589cc 100644 --- a/codeanalyzer/neo4j/project.py +++ b/codeanalyzer/neo4j/project.py @@ -190,9 +190,15 @@ def _project_program_graphs( ) b.edge("PY_HAS_CFG_NODE", owner, ref) if node.kind == "call" and node.callee: - b.edge_to_symbol( - "PY_RESOLVES_TO", ref, - _symbol_ref(node.callee, externals, sig_to_id), + # `callee` is ALREADY a resolved can:// id (a declared callable + # or an @external home), so it must not go through + # `_symbol_ref`, which expects a dotted signature and would + # fall back to matching a `signature` property against an id -- + # emitting an edge that matches nothing at load time. + b.edge( + "PY_RESOLVES_TO", + ref, + _call_endpoint(b, node.callee, externals, sig_to_id), ) for e in c.cfg or []: # kind-discriminated: a conditional's true/false pair between one diff --git a/codeanalyzer/schema/__init__.py b/codeanalyzer/schema/__init__.py index 648a10e..9d13f32 100644 --- a/codeanalyzer/schema/__init__.py +++ b/codeanalyzer/schema/__init__.py @@ -62,6 +62,31 @@ ) Analysis.update_forward_refs(PyApplication=PyApplication) +# Fields the analyzer keeps in memory (and in the cache) but never emits. +# `call_sites` is the internal record `body{}` call nodes are derived from (#120); +# emitting both shipped the same fact twice under two identity schemes. +INTERNAL_ONLY_FIELDS = frozenset({"call_sites"}) + + +def strip_internal_only(data): + """Recursively drop `INTERNAL_ONLY_FIELDS` from a dumped payload. + + Applied at emit time rather than as a field-level Pydantic `exclude`, because + the analysis cache uses the same serializer -- excluding at the field would + drop these from the cache as well, and the next warm-cache run would rebuild + from a payload with no call sites. + """ + if isinstance(data, dict): + return { + k: strip_internal_only(v) + for k, v in data.items() + if k not in INTERNAL_ONLY_FIELDS + } + if isinstance(data, list): + return [strip_internal_only(v) for v in data] + return data + + # Compatibility helpers for Pydantic v1/v2 def model_dump_json(model, **kwargs): """Compatibility helper for JSON serialization.""" @@ -89,5 +114,7 @@ def model_validate_json(model_class, json_data): __all__.extend([ "PYDANTIC_V2", "model_dump_json", + "strip_internal_only", + "INTERNAL_ONLY_FIELDS", "model_validate_json" ]) \ No newline at end of file diff --git a/codeanalyzer/schema/py_schema.py b/codeanalyzer/schema/py_schema.py index 2661c3b..d1b61b4 100644 --- a/codeanalyzer/schema/py_schema.py +++ b/codeanalyzer/schema/py_schema.py @@ -22,7 +22,7 @@ from __future__ import annotations from pathlib import Path from typing import Any, Dict, List, Optional, Tuple -from pydantic import BaseModel, Field +from pydantic import BaseModel from typing_extensions import Literal @@ -346,11 +346,13 @@ class PyCallable(BaseModel): end_line: int = -1 code_start_line: int = -1 accessed_symbols: List[PySymbol] = [] - # Internal only (#120): the Jedi-produced record that `l1_body` derives `body{}` + # Internal (#120): the Jedi-produced record that `l1_body` derives `body{}` # call nodes from, and that `call_graph.py`, `l2_callees.py` and the dataflow - # builder all read. Kept in memory, excluded from the wire -- `body{}` is the - # single emitted representation of a call site. - call_sites: List[PyCallsite] = Field(default_factory=list, exclude=True) + # builder all read. It is stripped at EMIT time (see `wire_json`), not with a + # field-level `exclude`: the analysis cache round-trips through the same + # serializer, so excluding it would drop it from the cache too and a warm-cache + # run would rebuild with no call sites at all. + call_sites: List[PyCallsite] = [] callables: Dict[str, "PyCallable"] = {} # nested callables (closures) types: Dict[str, "PyClass"] = {} # nested (local) classes local_variables: List[PyVariableDeclaration] = [] diff --git a/test/sample_graph_app.py b/test/sample_graph_app.py index 99e39c1..f3c600a 100644 --- a/test/sample_graph_app.py +++ b/test/sample_graph_app.py @@ -38,6 +38,7 @@ from codeanalyzer.schema import PyApplication, PyExternalSymbol from codeanalyzer.schema.assign_ids import assign_ids from codeanalyzer.schema.l1_body import populate_l1_body +from codeanalyzer.schema.l2_callees import backfill_callees from codeanalyzer.schema.py_schema import PyCallEdge from codeanalyzer.semantic_analysis.call_graph import ( iter_callables_in_symbol_table, @@ -150,6 +151,10 @@ def make_sample_app() -> Tuple[PyApplication, Dict[str, str]]: } sig_to_id["os.getcwd"] = ext_id populate_l1_body(app) + # Mirror the real pipeline (core.py runs this at -a 2+): body `call` nodes get + # their resolved `callee`. Without it PY_RESOLVES_TO never fires, since #120 + # sources that edge from the body node rather than from `call_sites[]`. + backfill_callees(app, sig_to_id) syntactic_infos, _func_asts = build_function_pdgs( app, k=3, oracle_factory=lambda c, fast: SyntacticOracle() ) diff --git a/test/test_call_site_body_convergence.py b/test/test_call_site_body_convergence.py index a2db435..fd7c3be 100644 --- a/test/test_call_site_body_convergence.py +++ b/test/test_call_site_body_convergence.py @@ -7,6 +7,7 @@ """ import json +from codeanalyzer.schema import strip_internal_only from codeanalyzer.schema.l1_body import populate_l1_body from codeanalyzer.schema.py_schema import ( PyApplication, PyCallable, PyCallArgument, PyCallsite, PyModule, @@ -51,10 +52,25 @@ def test_call_sites_stays_in_memory_for_the_internal_passes(): assert fn.call_sites[0].method_name == "g" -def test_call_sites_does_not_reach_the_wire(): +def test_call_sites_is_stripped_at_emit_but_kept_when_serialized(): + """Both halves of the mechanism. + + Stripping happens at emit time, NOT via a field-level Pydantic `exclude`, + because the analysis cache round-trips through the same serializer. Excluding + at the field would drop `call_sites` from the cache too, and the next + warm-cache run would rebuild from a payload with no call sites at all — + silently losing the producer that `l1_body`, `l2_callees`, the call graph and + the dataflow builder all read. + """ app, fn = _app() populate_l1_body(app) - emitted = json.loads(app.model_dump_json()) + dumped = app.model_dump(mode="json") + + # Serialized form keeps it — this is what the cache persists. + assert "call_sites" in dumped["symbol_table"]["a.py"]["functions"]["f"] + + # Emitted form drops it — this is what analysis.json carries. + emitted = strip_internal_only(dumped) callable_json = emitted["symbol_table"]["a.py"]["functions"]["f"] assert "call_sites" not in callable_json (node,) = [n for n in callable_json["body"].values() if n["kind"] == "call"] @@ -65,6 +81,6 @@ def test_accessed_symbols_and_local_variables_are_untouched(): """Only call_sites is redundant; these have no body{} representation to converge into.""" app, fn = _app() populate_l1_body(app) - callable_json = json.loads(app.model_dump_json())["symbol_table"]["a.py"]["functions"]["f"] + callable_json = strip_internal_only(app.model_dump(mode="json"))["symbol_table"]["a.py"]["functions"]["f"] assert "accessed_symbols" in callable_json assert "local_variables" in callable_json diff --git a/test/test_v2_l2.py b/test/test_v2_l2.py index cf7e206..eaa6154 100644 --- a/test/test_v2_l2.py +++ b/test/test_v2_l2.py @@ -77,8 +77,11 @@ def walk_type(t): for t in (mod.get("types") or {}).values(): walk_type(t) def walk_callable(c): - for cs in c.get("call_sites") or []: - sites.append(cs) + # #120: call sites are `body{}` entries with kind == "call"; the + # parallel `call_sites[]` list no longer reaches the wire. + for node in (c.get("body") or {}).values(): + if node.get("kind") == "call": + sites.append(node) for ic in (c.get("callables") or {}).values(): walk_callable(ic) for fn in (mod.get("functions") or {}).values(): @@ -87,26 +90,34 @@ def walk_callable(c): for m in (t.get("callables") or {}).values(): walk_callable(m) + def _callee_name(callee: str) -> str: + """The invoked name from a resolved callee id. + + `can://…/Type/name(args)` -> `name`; an `@external` home ends in a bare + name with no parameter list, which this handles too. + """ + return callee.rsplit("/", 1)[-1].split("(", 1)[0] + resolved = [ cs for cs in sites - if cs.get("callee_signature") and not cs.get("is_constructor_call") + if cs.get("callee") and not cs.get("is_constructor_call") ] assert resolved, "fixture must produce resolved non-constructor callsites" agree = sum( 1 for cs in resolved - if cs["callee_signature"].rsplit(".", 1)[-1] == cs["method_name"] + if _callee_name(cs["callee"]) == cs["method_name"] ) ratio = agree / len(resolved) assert ratio >= 0.95, ( f"only {agree}/{len(resolved)} resolved callsites bind to the invoked " - f"name: {[(cs['method_name'], cs['callee_signature']) for cs in resolved if cs['callee_signature'].rsplit('.', 1)[-1] != cs['method_name']]}" + f"name: {[(cs['method_name'], cs['callee']) for cs in resolved if _callee_name(cs['callee']) != cs['method_name']]}" ) # no class fallback when the exact method exists: a callee binding that # names a class which declares the invoked method is the defect's signature for cs in resolved: for cls_id, methods in class_methods.items(): cls_sig_name = cls_id.rsplit("/", 1)[-1] - if cs["callee_signature"].rsplit(".", 1)[-1] == cls_sig_name and cs["method_name"] in methods: + if _callee_name(cs["callee"]) == cls_sig_name.split("(", 1)[0] and cs["method_name"] in methods: raise AssertionError( f"callsite {cs['method_name']!r} fell back to class {cls_sig_name!r} " f"which declares the exact method" diff --git a/test/test_v2_two_projection_agreement.py b/test/test_v2_two_projection_agreement.py index 332e35c..e660906 100644 --- a/test/test_v2_two_projection_agreement.py +++ b/test/test_v2_two_projection_agreement.py @@ -6,6 +6,7 @@ from codeanalyzer.dataflow.builder import build_function_pdgs, emit_l3_body from codeanalyzer.dataflow.syntactic import SyntacticOracle from codeanalyzer.schema.l1_body import populate_l1_body +from codeanalyzer.schema.l2_callees import backfill_callees from codeanalyzer.syntactic_analysis.symbol_table_builder import SymbolTableBuilder @@ -31,6 +32,11 @@ def test_py_resolves_to_edge_targets_declared_callee_by_can_id(): functions={"f": caller, "g": callee}) app = PyApplication(symbol_table={"m.py": mod}) sig_to_id = assign_ids(app, "myapp") + # #120: PY_RESOLVES_TO now sources from the `body{}` call node's resolved + # `callee`, not from `call_sites[].callee_signature`. Drive the same pipeline + # the analyzer runs so the fixture has a body node to project from. + populate_l1_body(app) + backfill_callees(app, sig_to_id) rows = project(app, "myapp", sig_to_id) resolves = [e for e in rows.edges if e.type == "PY_RESOLVES_TO"] # the callsite must resolve to g's can:// id — edge kept, not dropped