diff --git a/CHANGELOG.md b/CHANGELOG.md index 808a15c..ee887a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [v2.0.0-rc.8] - 2026-09-15 +### Fixed + +- **`locate` places a position on a decorator line on the decorated callable, on both Python + backends** (`python-sdk#408`). `PyCallable.start_line` is the `def` line (`ast.FunctionDef.lineno`) + and Python's AST puts decorators above it, so `@http.route(...)` sat inside no callable span and + came back as `module_scope`. Both backends now read where the decorator is *applied*: + `_find_innermost` admits the callable whose `decorators[].span` starts at or before the line and + above its `def`; `_LOCATE_QUERY` gains one `EXISTS` disjunct over `PY_DECORATED_BY.start_line`, + which codeanalyzer-python 1.5.2 emits. Ranking is unchanged (the def-based width), so a decorator + on a nested callable resolves to the nested one. Three things deliberately stay as they were: a + position between two callables with no decorator over it is still `module_scope` (this is not a + nearest-callable fallback); a **class** decorator is still `module_scope` (a result with `type` + set and `callable` unset is a new shape and needs its own decision); and an analysis or graph from + codeanalyzer-python 1.5.1 or earlier, which records no decorator span, answers exactly as before. + Measured on odoo-slim-19 re-emitted by 1.5.3, 40 positions, median of 5, same graph both ways: + 71.0 ms placing 20/40 → 70.2 ms placing 40/40; the plan still seeks `pysymbol_id`. + ### Changed - `codeanalyzer-python` pin `1.5.2` → **`1.5.3`** (`dependencies` and `[tool.backend-versions]`, diff --git a/cldk/analysis/python/codeanalyzer/codeanalyzer.py b/cldk/analysis/python/codeanalyzer/codeanalyzer.py index 984d415..776f3c5 100644 --- a/cldk/analysis/python/codeanalyzer/codeanalyzer.py +++ b/cldk/analysis/python/codeanalyzer/codeanalyzer.py @@ -301,12 +301,29 @@ def _resolve_callee( return cs if resolved == cs.callee_signature else cs.model_copy(update={"callee_signature": resolved}) +def _decorator_covers(c: PyCallable, line: int) -> bool: + """Whether ``line`` sits on (or between) the decorators applied to ``c`` -- at or after the first + recorded decorator's line and above ``c.start_line``, the ``def`` line. The same predicate the + Neo4j backend's ``PY_DECORATED_BY`` disjunct evaluates, so the two backends admit the same + callable for a decorator position (#408). + + A decorator whose ``span`` was not recorded (every analysis from codeanalyzer-python 1.5.1 or + earlier) is skipped: nothing is inferred from the decorator's mere presence, so such a position + stays module scope exactly as before. Line-level only, deliberately -- the span starts at the + decorator *expression*, one column past the ``@``, so a column test would miss the ``@`` itself. + """ + return any(d.span is not None and d.span.start[0] <= line < c.start_line for d in c.decorators) + + def _find_innermost(module: PyModule, line: int) -> Tuple[PyCallable, "PyClass | None"] | None: """The callable (and its immediate owning class, if any) whose span most tightly contains ``line`` — innermost first, so a closure nested inside a method wins over the method itself. Only real callable spans count: a blank line, a comment, or a gap between two callables' spans - contains no callable and must never snap to the nearest one (see :meth:`locate`). + contains no callable and must never snap to the nearest one (see :meth:`locate`). The one + widening is a decorator line: ``start_line`` is the ``def`` line and the AST puts decorators + above it, so :func:`_decorator_covers` admits the callable a decorator at that position applies + to (#408). The rank stays the def-based width, which is what the Neo4j backend ranks on too. Equal line widths tie — ``def one(self): return lambda: 2`` nests two callables on one line — and lines are all the Neo4j projection carries, so the tie breaks on the *longer signature*: a nested @@ -322,7 +339,7 @@ def consider(c: PyCallable, owner: "PyClass | None") -> None: nonlocal best, best_rank if c.start_line < 0 or c.end_line < 0: return - if not (c.start_line <= line <= c.end_line): + if not (c.start_line <= line <= c.end_line or _decorator_covers(c, line)): return rank = (c.end_line - c.start_line, -len(c.signature), c.signature) if best_rank is None or rank < best_rank: diff --git a/cldk/analysis/python/neo4j/neo4j_backend.py b/cldk/analysis/python/neo4j/neo4j_backend.py index acfe427..75ac0fd 100644 --- a/cldk/analysis/python/neo4j/neo4j_backend.py +++ b/cldk/analysis/python/neo4j/neo4j_backend.py @@ -2037,7 +2037,15 @@ def get_config_readers(self, key: str) -> List[PyCallableOverview]: # the position is the innermost callable, which naturally treats a gap between two callables # (or a module top-level line) the same way: no callable matches, so it falls through to # module_scope rather than snapping to a neighbour. PY_HAS_METHOD is walked reversed for the - # owning class (``type``); + # owning class (``type``). ``start_line`` is the ``def`` line (``ast.FunctionDef.lineno``), and + # Python's AST puts decorators *above* it, so a position on ``@http.route(...)`` sits in no + # callable span; the ``EXISTS`` disjunct admits the callable whose ``PY_DECORATED_BY`` edge is + # applied at or before the position and above its ``def`` (#408). The edge's ``start_line`` + # arrived with codeanalyzer-python 1.5.2; on an older graph the comparison is null, the + # disjunct is false, and the position is module scope exactly as before. A decorated *class* + # is not a callable and is deliberately not admitted. Ranking is unchanged — the def-based + # width — so a decorator on a nested callable, which also lies inside the enclosing + # callable's span, still resolves to the nested (narrower) one; # * the **body node** comes from PY_HAS_BODY_NODE off that same candidate callable, again by # line containment, innermost first. Synthetic vertices (@entry / @exit / @formal_in:N) carry # no span, so the emitter prunes their start_line/end_line away entirely — the @@ -2056,6 +2064,13 @@ def get_config_readers(self, key: str) -> List[PyCallableOverview]: # 1.4.1's own ``:PyCanNode(id)`` range index was measured and rejected: it spans all 955,961 # application nodes, so seeking it walked a 40x larger range (locate 54 ms, but # ``_RESOLVE_CALLABLE_QUERY`` 19 -> 210 ms), and 1.4.0 graphs have no such label at all. + # + # The ``PY_DECORATED_BY`` disjunct (#408) adds one relationship expansion per candidate + # callable. Measured on odoo-slim-19 re-emitted by codeanalyzer-python 1.5.3 at level 1 (1,626 + # modules, 15,549 callables, 5,615 decorator edges), 40 positions (20 decorator lines, 20 body + # lines), median of 5, same graph both ways: 71.0 ms without the disjunct placing 20/40, 70.2 ms + # with it placing 40/40. The plan still seeks ``pysymbol_id`` (``NodeUniqueIndexSeekByRange``); + # the disjunct runs as a ``SelectOrSemiApply`` over the seek's rows, never as a scan. _LOCATE_QUERY = ( "UNWIND $positions AS pos " "OPTIONAL MATCH (:PyApplication {id: $app_id})-[:PY_HAS_MODULE]->(m:PyModule {file_key: pos.path}) " @@ -2063,7 +2078,8 @@ def get_config_readers(self, key: str) -> List[PyCallableOverview]: "OPTIONAL MATCH (c:PyCallable:PySymbol) " "WHERE c.id STARTS WITH pos.module_prefix " "AND c.start_line IS NOT NULL AND c.end_line IS NOT NULL " - "AND c.start_line <= pos.line AND pos.line <= c.end_line " + "AND ((c.start_line <= pos.line AND pos.line <= c.end_line) " + "OR EXISTS { (c)-[r:PY_DECORATED_BY]->() WHERE r.start_line <= pos.line AND pos.line < c.start_line }) " "WITH pos, m, c " "OPTIONAL MATCH (cls:PyClass)-[:PY_HAS_METHOD]->(c) " "WITH pos, m, c, cls " diff --git a/docs/agent-api-reference.md b/docs/agent-api-reference.md index e10d132..966d70b 100644 --- a/docs/agent-api-reference.md +++ b/docs/agent-api-reference.md @@ -525,6 +525,7 @@ class LocateResult: | inside a callable | `callable` is set, no diagnostic | | module top level | `callable is None`, diagnostic `module_scope` — a **real position**, not an absence | | between two callables | same as module scope; it never snaps to the nearest callable | +| on a decorator line (Python) | `callable` is the decorated callable, no diagnostic. `start_line` is the `def` line, so the decorators sit above every span; both backends read where the decorator is *applied* (`decorators[].span` locally, `PY_DECORATED_BY.start_line` on a graph emitted by codeanalyzer-python ≥ 1.5.2; older graphs keep reporting `module_scope`). A **class** decorator is still module scope | | file not analysed | diagnostic `file_not_in_graph` — distinct from a file that doesn't exist | **Gotcha:** on **Python and TypeScript** over Neo4j, module-scope `source` is empty and carries diff --git a/tests/analysis/python/conftest.py b/tests/analysis/python/conftest.py index c14b390..7cdfc75 100644 --- a/tests/analysis/python/conftest.py +++ b/tests/analysis/python/conftest.py @@ -34,7 +34,7 @@ from cldk.analysis.python.codeanalyzer.codeanalyzer import PyCodeanalyzer from cldk.analysis.python.neo4j import PyNeo4jBackend -from cldk.models.python import BodyNode, PyApplication, PyCallable, PyClass, PyModule, Span +from cldk.models.python import BodyNode, PyApplication, PyCallable, PyClass, PyDecorator, PyModule, Span # The full vocabulary codeanalyzer-python 1.4.0 emits (see cldk/analysis/python/neo4j/neo4j_backend # .py's module docstring / the leg-1 brief) — a reasonable "healthy v2 graph" default so fixtures @@ -256,6 +256,26 @@ def query_counter(fake_driver: FakeDriver) -> QueryCounter: "", # 27 " def two(self, x):", # 28 " if x: return x", # 29 — two body nodes tie on line width + "", # 30 + " @property", # 31 — first decorator: ast puts it *above* FunctionDef.lineno (#408) + " @functools.lru_cache(maxsize=8)", # 32 — second decorator + " def cached(self):", # 33 — start_line + " return 3", # 34 + "", # 35 + " def outer(self):", # 36 + " @staticmethod", # 37 — a decorator on a *nested* callable, inside outer's span + " def helper():", # 38 + " return 4", # 39 + " return helper", # 40 + "", # 41 + " @legacy", # 42 — a decorator whose span the analyzer did not record (pre-1.5.2 shape) + " def old(self):", # 43 + " return 5", # 44 + "", # 45 + "", # 46 + "@dataclass", # 47 — a *class* decorator: no callable applies, so still module scope + "class Config:", # 48 + " x: int = 0", # 49 ] _LOCATE_MODULE_SOURCE = "".join(line + "\n" for line in _LOCATE_SOURCE_LINES) @@ -294,6 +314,9 @@ def _locate_code(start_line: int, end_line: int) -> str: # analysis vertices (@entry/@exit/@formal_in:N) that carry no span and can never contain a position. # ``has_span`` False is a callable the analyzer emitted with no span at all — an abstract method or a # protocol stub — whose source is unrecoverable, and which must degrade rather than raise. +# ``decorators`` lists (name, start_line, end_line) where each decorator is *applied*; ``None`` lines +# are a decorator the analyzer recorded without a span (every graph from 1.5.1 or earlier, and the +# local ``PyDecorator.span`` default), which must leave ``locate`` exactly where it was (#408). _LOCATE_CALLABLE_SPECS = [ { "signature": "src.app.Store.Meta.tag", @@ -374,6 +397,45 @@ def _locate_code(start_line: int, end_line: int) -> str: "has_span": False, "body": {}, }, + { + "signature": "src.app.Store.cached", + "name": "cached", + "start_line": 33, # the ``def`` line; the decorators sit on 31-32, above it + "end_line": 34, + "class_signature": "src.app.Store", + "has_span": True, + "body": {"34:8": ("return", 34, 34)}, + "decorators": [("property", 31, 31), ("functools.lru_cache", 32, 32)], + }, + { + "signature": "src.app.Store.outer", + "name": "outer", + "start_line": 36, + "end_line": 40, + "class_signature": "src.app.Store", + "has_span": True, + "body": {"40:8": ("return", 40, 40)}, + }, + { + "signature": "src.app.Store.outer..helper", + "name": "helper", + "start_line": 38, + "end_line": 39, + "class_signature": None, + "has_span": True, + "body": {"39:12": ("return", 39, 39)}, + "decorators": [("staticmethod", 37, 37)], # line 37 is also inside outer's 36-40 + }, + { + "signature": "src.app.Store.old", + "name": "old", + "start_line": 43, + "end_line": 44, + "class_signature": "src.app.Store", + "has_span": True, + "body": {"44:8": ("return", 44, 44)}, + "decorators": [("legacy", None, None)], # recorded, but with no span + }, ] _LOCATE_SPEC = {c["signature"]: c for c in _LOCATE_CALLABLE_SPECS} @@ -396,6 +458,7 @@ def _locate_pycallable(spec: dict, **children: Any) -> PyCallable: start_line=spec["start_line"], end_line=spec["end_line"], body={key: BodyNode(kind=kind, span=_locate_span(s, e) if s is not None else None) for key, (kind, s, e) in spec["body"].items()}, + decorators=[PyDecorator(name=name, expression=f"@{name}", span=_locate_span(s, e) if s is not None else None) for name, s, e in spec.get("decorators", [])], **children, ) @@ -404,6 +467,7 @@ def _locate_application() -> PyApplication: """The fixture module as the in-process analyzer would hand it over.""" inner = _locate_pycallable(_LOCATE_SPEC["src.app.Store.wrap..inner"]) lam = _locate_pycallable(_LOCATE_SPEC["src.app.Store.one.."]) + helper = _locate_pycallable(_LOCATE_SPEC["src.app.Store.outer..helper"]) meta = PyClass( name="Meta", signature="src.app.Store.Meta", @@ -418,13 +482,19 @@ def _locate_application() -> PyApplication: "one": _locate_pycallable(_LOCATE_SPEC["src.app.Store.one"], callables={"": lam}), "two": _locate_pycallable(_LOCATE_SPEC["src.app.Store.two"]), "stub": _locate_pycallable(_LOCATE_SPEC["src.app.Store.stub"]), + "cached": _locate_pycallable(_LOCATE_SPEC["src.app.Store.cached"]), + "outer": _locate_pycallable(_LOCATE_SPEC["src.app.Store.outer"], callables={"helper": helper}), + "old": _locate_pycallable(_LOCATE_SPEC["src.app.Store.old"]), }, types={"src.app.Store.Meta": meta}, ) + # A decorated *class*: the decorator applies to no callable, so a position on it stays module + # scope on both backends (#408 leaves the class case for its own decision). + config = PyClass(name="Config", signature="src.app.Config", decorators=[PyDecorator(name="dataclass", expression="@dataclass", span=_locate_span(47, 47))]) module = PyModule( file_path=_LOCATE_MODULE_PATH, module_name="src.app", - types={"src.app.Store": store}, + types={"src.app.Store": store, "src.app.Config": config}, source=_LOCATE_MODULE_SOURCE, ) return PyApplication(symbol_table={_LOCATE_MODULE_PATH: module}) @@ -482,6 +552,21 @@ def _locate_body_props(spec: dict) -> list[dict]: ] +def _locate_contains(spec: dict, line: int, query: str) -> bool: + """The callable WHERE clause, evaluated the way Cypher would. + + Plain span containment always; the ``PY_DECORATED_BY`` disjunct (#408) only when the query + actually names the relationship, and then only over decorator edges that *carry* a + ``start_line`` -- ``r.start_line <= pos.line`` is null, hence false, on an edge with no span, + exactly as on a graph emitted before codeanalyzer-python 1.5.2. + """ + if spec["start_line"] <= line <= spec["end_line"]: + return True + if "PY_DECORATED_BY" not in query: + return False + return any(s is not None and s <= line < spec["start_line"] for _, s, _ in spec.get("decorators", [])) + + def _locate_row(idx, module_props=None, callable_props=None, class_props=None, body_props=None) -> dict: return {"idx": idx, "module_props": module_props, "callable_props": callable_props, "class_props": class_props, "body_props": body_props} @@ -520,7 +605,7 @@ def _locate_responder(query: str, params: dict) -> list[dict]: rows.append(_locate_row(pos["idx"])) # no :PyModule for this file_key continue # ``OPTIONAL MATCH (c:PyCallable) WHERE c.id STARTS WITH pos.module_prefix AND ...`` - matches = [c for c in _LOCATE_CALLABLE_SPECS if _locate_callable_id(c["signature"]).startswith(pos["module_prefix"]) and c["start_line"] <= pos["line"] <= c["end_line"]] + matches = [c for c in _LOCATE_CALLABLE_SPECS if _locate_callable_id(c["signature"]).startswith(pos["module_prefix"]) and _locate_contains(c, pos["line"], query)] if not matches: rows.append(_locate_row(pos["idx"], _LOCATE_MODULE_PROPS)) continue diff --git a/tests/analysis/python/test_locate.py b/tests/analysis/python/test_locate.py index 607e0fa..ab1fa24 100644 --- a/tests/analysis/python/test_locate.py +++ b/tests/analysis/python/test_locate.py @@ -355,3 +355,98 @@ def test_locate_body_key_column_is_parsed_not_string_compared(): assert body_key_column("@entry") == -1 # synthetic vertices carry no column assert "29:10" < "29:4" # ...which is why the string comparison had to go assert body_key_column("29:10") > body_key_column("29:4") + + +# ================================================================================================ +# Decorator lines (#408). ``PyCallable.start_line`` is the ``def`` line -- codeanalyzer-python sets +# it from ``ast.FunctionDef.lineno``, and Python's AST puts decorators *above* that line -- so a +# position on a decorator sits inside no callable span and used to fall through to ``module_scope``. +# A routed controller's ``@http.route(...)`` line is not module scope; it is the method's. Both +# backends read the applied position the analyzer already records: ``PyCallable.decorators[].span`` +# locally, ``PY_DECORATED_BY.start_line`` on the graph (codeanalyzer-python 1.5.2). +# ================================================================================================ +def test_locate_decorator_line_resolves_to_the_decorated_method(py_either): + """Line 31 is ``@property``, two lines above ``def cached``.""" + r = py_either.locate("src/app.py", 31) + assert r.callable.signature == "src.app.Store.cached" + assert r.type.signature == "src.app.Store" + assert r.callable.class_signature == "src.app.Store" + assert r.diagnostics == [] + assert r.body is None # no body node starts above the def line + + +def test_locate_every_line_from_the_first_decorator_to_end_line_is_the_method(py_either): + """A position between two decorators of one callable (line 32) resolves to it as well.""" + for line in (31, 32, 33, 34): + r = py_either.locate("src/app.py", line) + assert r.callable is not None, line + assert r.callable.signature == "src.app.Store.cached", line + assert r.diagnostics == [], line + + +def test_locate_decorator_on_a_nested_callable_resolves_to_the_nested_one(py_either): + """Line 37 (``@staticmethod``) lies inside ``outer``'s span *and* above ``helper``'s def. + ``helper`` wins on width, as the existing innermost rule says -- asserted, not assumed.""" + r = py_either.locate("src/app.py", 37) + assert r.callable.signature == "src.app.Store.outer..helper" + assert r.callable.class_signature is None + assert r.type is None + assert r.diagnostics == [] + # ...and one line up, the enclosing method still owns the position. + assert py_either.locate("src/app.py", 36).callable.signature == "src.app.Store.outer" + + +def test_locate_blank_line_between_callables_is_still_module_scope(py_either): + """The widening must not turn into a nearest-callable fallback: a blank line above a decorated + callable (30) and one below it (35) contain no callable and no decorator.""" + for line in (30, 35, 41): + r = py_either.locate("src/app.py", line) + assert r.callable is None, line + assert r.type is None, line + assert "module_scope" in [d.code for d in r.diagnostics], line + + +def test_locate_class_decorator_is_still_module_scope(py_either): + """``@dataclass`` on line 47 applies to a *class*, not a callable. A result with ``type`` set and + ``callable`` unset would be a new shape; until that is decided, the position keeps today's + behaviour -- pinned so the out-of-scope decision is visible rather than assumed.""" + r = py_either.locate("src/app.py", 47) + assert r.callable is None + assert r.type is None + assert "module_scope" in [d.code for d in r.diagnostics] + + +def test_locate_decorator_without_a_recorded_span_keeps_todays_behaviour(py_either): + """``@legacy`` on line 42 is recorded with no span -- the shape of every analysis and graph from + codeanalyzer-python 1.5.1 or earlier. Nothing raises, and the position is module scope exactly + as before; the ``def`` line itself (43) still resolves.""" + r = py_either.locate("src/app.py", 42) + assert r.callable is None + assert "module_scope" in [d.code for d in r.diagnostics] + assert py_either.locate("src/app.py", 43).callable.signature == "src.app.Store.old" + + +def test_locate_parity_decorator_positions_agree(py, py_local): + """Both backends, position by position, over every decorator-adjacent line the fixture has.""" + + def probe(backend, line): + r = backend.locate("src/app.py", line) + return ( + r.callable.signature if r.callable else None, + r.type.signature if r.type else None, + r.body is None, + [d.code for d in r.diagnostics if d.code != "module_source_unavailable"], + ) + + lines = [30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 47] + assert [probe(py, line) for line in lines] == [probe(py_local, line) for line in lines] + + +def test_locate_query_reads_the_decorator_edge_and_keeps_the_pysymbol_seek(py, fake_driver): + """The widening is one ``EXISTS`` disjunct over ``PY_DECORATED_BY``, and it must not cost the + per-module index seek ``test_locate_and_resolve_seek_the_pysymbol_index`` pins.""" + py.locate("src/app.py", 31) + statement = next(s for s in fake_driver.statements if "UNWIND $positions AS pos" in s) + assert "OPTIONAL MATCH (c:PyCallable:PySymbol) " in statement + assert "[r:PY_DECORATED_BY]" in statement + assert "r.start_line <= pos.line AND pos.line < c.start_line" in statement