Skip to content

Commit 8ba29ab

Browse files
committed
fix(typescript): guard ambiguous application match; mark 2.0.0 e2e-verify assumptions (#268)
Review fixes on the graph-schema-2.0.0 rewrite: - `_resolve_application_id` resolves `application_name` to exactly one :Application `can://` id on connect; 0 or >1 suffix matches raise CodeanalyzerUsageException naming the candidate ids, so a shared database with two apps ending in the same path segment can no longer silently merge module scopes. `_load_module_keys` anchors on the resolved id. The schema check still runs first (DB-level) so pre-2.0.0 graphs keep the actionable mismatch error; rationale commented in __init__. - Every unverified 2.0.0 assumption now carries a VERIFY(2.0.0-e2e) marker at its query site (`_module` scoping, array-prop class hierarchy, method_name fallback, reconstruct's module_name->name fallback); the module docstring states the `_module` claim as to-verify, and the two leftover v1 "Symbol-keyed" prose comments are reworded. - Query-construction unit tests capture the built Cypher through the `_run` seam for the two riskiest rewrites: the TS_HAS_BODY_NODE / TSBodyNode {kind:"call"} / TS_RESOLVES_TO call-site path (reading `callee`) and TSModule `_module` scoping, plus the app-id guard (0-match and 2-match). - CldkSchemaMismatchException exported from cldk.utils.exceptions __all__.
1 parent 4a43e75 commit 8ba29ab

4 files changed

Lines changed: 164 additions & 15 deletions

File tree

cldk/analysis/typescript/neo4j/neo4j_backend.py

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,12 @@
4545
* call *sites* are body nodes — ``(:TSBodyNode {kind: "call"})`` reached via
4646
``TS_HAS_BODY_NODE`` and resolved through ``TS_RESOLVES_TO`` — not standalone
4747
call-site nodes;
48-
* every project-owned node carries a ``_module`` provenance property (the
49-
project-relative path), so a single database may hold several applications —
50-
all queries here are scoped to this backend's application by the set of its
51-
module ``_module`` keys.
48+
* module nodes carry a ``_module`` property (the project-relative path); this
49+
backend further *assumes* — to-verify against a live 1.0.0 graph, see the
50+
``VERIFY(2.0.0-e2e)`` markers — that every project-owned node carries the same
51+
``_module`` provenance property, so a single database may hold several
52+
applications and all queries here can be scoped to this backend's application
53+
by the set of its module ``_module`` keys.
5254
5355
Parity caveats (inherent to what schema 2.0.0 projects, not bugs):
5456
@@ -91,7 +93,7 @@
9193
TSTypeAlias,
9294
TSVariableDeclaration,
9395
)
94-
from cldk.utils.exceptions.exceptions import CldkSchemaMismatchException, CodeanalyzerExecutionException
96+
from cldk.utils.exceptions.exceptions import CldkSchemaMismatchException, CodeanalyzerExecutionException, CodeanalyzerUsageException
9597

9698
logger = logging.getLogger(__name__)
9799

@@ -139,9 +141,16 @@ def __init__(
139141
self._database = neo4j_database
140142
self._driver = GraphDatabase.driver(neo4j_uri, auth=(neo4j_username, neo4j_password))
141143

142-
# Fail fast if the persisted graph isn't the schema this SDK speaks.
144+
# Fail fast if the persisted graph isn't the schema this SDK speaks. This runs *before*
145+
# the application-id resolution on purpose: a pre-2.0.0 graph has no `can://` ids, so
146+
# resolving first would report "no application found" instead of the (more actionable)
147+
# schema mismatch.
143148
self._check_schema_version()
144149

150+
# Resolve `application_name` to exactly one Application `id` (guards against a shared
151+
# database where several apps' ids share the same trailing path segment).
152+
self._app_id: str = self._resolve_application_id()
153+
145154
# The application's module `_module` keys, used to scope every query to this app.
146155
self._modules: List[str] = self._load_module_keys()
147156

@@ -177,12 +186,39 @@ def _check_schema_version(self, expected: str | None = None, found: str | None =
177186
f"graph schema {found!r} in database, this SDK speaks {expected!r} — re-analyze with codeanalyzer-typescript>=1.0.0"
178187
)
179188

180-
def _load_module_keys(self) -> List[str]:
189+
def _resolve_application_id(self) -> str:
190+
"""Resolve ``application_name`` to exactly one ``:Application`` node's ``can://`` id.
191+
192+
The suffix match (``a.id ENDS WITH "/" + $app``) can bind multiple applications in a
193+
shared database — e.g. two repos whose ids both end in ``/frontend`` — which would
194+
silently merge their module scopes. Raise :class:`CodeanalyzerUsageException` unless the
195+
match is unique; the caller disambiguates by passing a longer trailing path (any suffix
196+
of the ``can://`` id starting at a ``/`` boundary works as ``application_name``).
197+
"""
181198
rows = self._run(
182-
'MATCH (a:Application) WHERE a.id ENDS WITH "/" + $app '
183-
"MATCH (a)-[:TS_HAS_MODULE]->(m:TSModule) RETURN m._module AS k",
199+
'MATCH (a:Application) WHERE a.id ENDS WITH "/" + $app RETURN a.id AS id ORDER BY id',
184200
app=self.application_name,
185201
)
202+
if not rows:
203+
raise CodeanalyzerUsageException(
204+
f"no :Application found whose id ends with '/{self.application_name}' — check application_name (the --app-name the graph was loaded with)."
205+
)
206+
if len(rows) > 1:
207+
candidates = ", ".join(r["id"] for r in rows)
208+
raise CodeanalyzerUsageException(
209+
f"application_name '{self.application_name}' is ambiguous: it matches {len(rows)} applications in this database ({candidates}). "
210+
"Pass a longer trailing path of the intended application's id to disambiguate."
211+
)
212+
return rows[0]["id"]
213+
214+
def _load_module_keys(self) -> List[str]:
215+
# VERIFY(2.0.0-e2e): every project-owned node is assumed to carry a `_module` provenance
216+
# property (as in the pre-2.0.0 projection); all the `x._module IN $mods` scoping below
217+
# depends on it — validate against a live 1.0.0 graph (Task 9).
218+
rows = self._run(
219+
"MATCH (a:Application {id: $app_id})-[:TS_HAS_MODULE]->(m:TSModule) RETURN m._module AS k",
220+
app_id=self._app_id,
221+
)
186222
return [r["k"] for r in rows]
187223

188224
# -----[ child-fetch helpers (reconstruction) ]-----
@@ -195,7 +231,7 @@ def _callsites_of(self, signature: str) -> List[TSCallsite]:
195231

196232
def _callable_full(self, props: Dict[str, Any]) -> TSCallable:
197233
sig = props["signature"]
198-
# Symbol-keyed containers are keyed by signature (matching the analyzer's dict keys).
234+
# Nested-declaration containers are keyed by signature (matching the analyzer's dict keys).
199235
inner_callables = {p["signature"]: self._callable_full(p) for p in self._children(sig, "TS_DECLARES", "TSCallable")}
200236
inner_classes = {p["signature"]: self._class_full(p) for p in self._children(sig, "TS_DECLARES", "TSClass")}
201237
return R.callable_(
@@ -309,7 +345,7 @@ def get_typescript_module(self, file_path: str) -> TSModule | None:
309345
if not rows:
310346
return None
311347
props = rows[0]["p"]
312-
# All symbol containers are keyed by signature (matching the analyzer's dict keys).
348+
# All declaration containers are keyed by signature (matching the analyzer's dict keys).
313349
classes = {p["signature"]: self._class_full(p) for p in self._module_decls(file_path, "TSClass")}
314350
interfaces = {p["signature"]: self._interface_full(p) for p in self._module_decls(file_path, "TSInterface")}
315351
enums = {p["signature"]: R.enum(p) for p in self._module_decls(file_path, "TSEnum")}
@@ -488,6 +524,10 @@ def get_class_call_graph(self, qualified_class_name: str, method_signature: str
488524

489525
def get_class_hierarchy(self) -> nx.DiGraph:
490526
"""Inheritance/implementation graph: an edge child → base for every base_class."""
527+
# VERIFY(2.0.0-e2e): hierarchy is read from the `base_classes` / `implements_types` array
528+
# properties (as pre-2.0.0), not the TS_EXTENDS / TS_IMPLEMENTS edges — validate against a
529+
# live 1.0.0 graph (Task 9); this also covers get_extended_classes,
530+
# get_implemented_interfaces and get_all_sub_classes.
491531
graph = nx.DiGraph()
492532
rows = self._run(
493533
"MATCH (n:CanNode) WHERE (n:TSClass OR n:TSInterface) AND n._module IN $mods " "RETURN n.signature AS sig, n.base_classes AS bases",
@@ -513,6 +553,9 @@ def get_calling_lines(self, target_signature: str) -> List[int]:
513553
return [r["line"] for r in rows]
514554

515555
def get_call_targets(self, source_signature: str) -> Set[str]:
556+
# VERIFY(2.0.0-e2e): falls back to `cs.method_name` when a call body node has no resolved
557+
# `callee`; whether TSBodyNode carries `method_name` is unconfirmed — validate against a
558+
# live 1.0.0 graph (Task 9).
516559
rows = self._run(
517560
"MATCH (c:TSCallable {signature: $sig})-[:TS_HAS_BODY_NODE]->(cs:TSBodyNode {kind: 'call'}) " "RETURN cs.callee AS cosig, cs.method_name AS mn",
518561
sig=source_signature,

cldk/analysis/typescript/neo4j/reconstruct.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,8 @@ def module(props: Props, **children: Any) -> TSModule:
373373
return TSModule(
374374
# schema 2.0.0 modules carry the project-relative path as ``_module``.
375375
file_path=props.get("_module", props.get("file_path", "")),
376+
# VERIFY(2.0.0-e2e): assumes the 2.0.0 module node names the module via `name` (falling
377+
# back from the pre-2.0.0 `module_name`) — validate against a live 1.0.0 graph (Task 9).
376378
module_name=props.get("module_name", props.get("name", "")),
377379
is_tsx=props.get("is_tsx", False),
378380
is_declaration_file=props.get("is_declaration_file", False),

cldk/utils/exceptions/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@
2020

2121
from .exceptions import (
2222
CldkInitializationException,
23+
CldkSchemaMismatchException,
2324
CodeanalyzerExecutionException,
2425
)
2526

2627
__all__ = [
2728
"CodeanalyzerExecutionException",
2829
"CldkInitializationException",
30+
"CldkSchemaMismatchException",
2931
]

tests/analysis/typescript/test_typescript_neo4j_schema.py

Lines changed: 106 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,25 @@
1616

1717
"""Unit tests for the TS Neo4j backend's graph-schema contract (no live Neo4j required).
1818
19-
Two things are exercised here without ever opening a Bolt connection:
19+
Four things are exercised here without ever opening a Bolt connection:
2020
2121
* the fail-fast ``schema_version`` gate (``_check_schema_version``) the backend runs on first use;
22+
* the application-id resolution guard (``_resolve_application_id``) — the suffix match must bind
23+
exactly one ``:Application`` or raise, never silently merge two apps' module scopes;
2224
* the accessors whose vocabulary is *not projected* into graph schema 2.0.0 (decorators,
2325
attributes/fields, module imports/exports, variables) — these must raise a clear
24-
``NotImplementedError`` rather than silently returning wrong data.
26+
``NotImplementedError`` rather than silently returning wrong data;
27+
* the constructed Cypher for the two riskiest 2.0.0 rewrites (the call-site body-node path and
28+
module `_module` scoping) — behavioral verification against a live graph is Task 9's e2e run.
2529
26-
Both are tested against a bare ``__new__`` instance (no ``__init__``, so no driver), because the
30+
All are tested against a bare ``__new__`` instance (no ``__init__``, so no driver), because the
2731
logic under test never needs a real session.
2832
"""
2933

3034
import pytest
3135

3236
from cldk.analysis.typescript.neo4j import TSNeo4jBackend
33-
from cldk.utils.exceptions.exceptions import CldkSchemaMismatchException
37+
from cldk.utils.exceptions.exceptions import CldkSchemaMismatchException, CodeanalyzerUsageException
3438

3539

3640
def _bare_backend() -> TSNeo4jBackend:
@@ -70,6 +74,104 @@ def test_schema_version_absent_fails_fast(monkeypatch):
7074
backend._check_schema_version(expected="2.0.0")
7175

7276

77+
# -----[ application-id resolution guard ]-----
78+
def test_resolve_application_id_unique_match(monkeypatch):
79+
backend = _bare_backend()
80+
backend.application_name = "frontend"
81+
monkeypatch.setattr(backend, "_run", lambda *a, **k: [{"id": "can://repo-a/frontend"}])
82+
assert backend._resolve_application_id() == "can://repo-a/frontend"
83+
84+
85+
def test_resolve_application_id_no_match_raises(monkeypatch):
86+
backend = _bare_backend()
87+
backend.application_name = "frontend"
88+
monkeypatch.setattr(backend, "_run", lambda *a, **k: [])
89+
with pytest.raises(CodeanalyzerUsageException, match="no :Application found"):
90+
backend._resolve_application_id()
91+
92+
93+
def test_resolve_application_id_ambiguous_raises_naming_candidates(monkeypatch):
94+
backend = _bare_backend()
95+
backend.application_name = "frontend"
96+
monkeypatch.setattr(
97+
backend,
98+
"_run",
99+
lambda *a, **k: [{"id": "can://repo-a/frontend"}, {"id": "can://repo-b/frontend"}],
100+
)
101+
with pytest.raises(CodeanalyzerUsageException) as exc_info:
102+
backend._resolve_application_id()
103+
message = str(exc_info.value)
104+
assert "ambiguous" in message
105+
assert "can://repo-a/frontend" in message
106+
assert "can://repo-b/frontend" in message
107+
108+
109+
# -----[ constructed-Cypher shape for the riskiest 2.0.0 rewrites ]-----
110+
def _recording_backend(monkeypatch):
111+
"""A bare backend whose ``_run`` records every (query, params) call and returns no rows."""
112+
backend = _bare_backend()
113+
backend._modules = ["src/mod.ts"]
114+
calls = []
115+
116+
def _run(query, **params):
117+
calls.append((query, params))
118+
return []
119+
120+
monkeypatch.setattr(backend, "_run", _run)
121+
return backend, calls
122+
123+
124+
def test_call_site_query_uses_body_node_path(monkeypatch):
125+
backend, calls = _recording_backend(monkeypatch)
126+
backend.get_call_sites("src/mod.Foo.bar")
127+
(query, params), = calls
128+
assert "-[:TS_HAS_BODY_NODE]->" in query
129+
assert "TSBodyNode {kind: 'call'}" in query
130+
assert params == {"sig": "src/mod.Foo.bar"}
131+
132+
133+
def test_calling_lines_query_reads_callee_property(monkeypatch):
134+
backend, calls = _recording_backend(monkeypatch)
135+
backend.get_calling_lines("src/mod.Foo.bar")
136+
(query, _), = calls
137+
assert "TSBodyNode {kind: 'call'}" in query
138+
assert "cs.callee = $sig" in query
139+
140+
141+
def test_call_targets_query_reads_callee_property(monkeypatch):
142+
backend, calls = _recording_backend(monkeypatch)
143+
backend.get_call_targets("src/mod.Foo.bar")
144+
(query, _), = calls
145+
assert "-[:TS_HAS_BODY_NODE]->" in query
146+
assert "cs.callee AS" in query
147+
148+
149+
def test_external_symbols_query_resolves_via_body_nodes(monkeypatch):
150+
backend, calls = _recording_backend(monkeypatch)
151+
backend.get_external_symbols()
152+
(query, _), = calls
153+
assert "-[:TS_CALLS]->(e:TSExternal)" in query
154+
assert "(cs:TSBodyNode {kind: 'call'})-[:TS_RESOLVES_TO]->(e:TSExternal)" in query
155+
156+
157+
def test_module_lookup_query_keys_on_module_property(monkeypatch):
158+
backend, calls = _recording_backend(monkeypatch)
159+
assert backend.get_typescript_module("src/mod.ts") is None # no rows stubbed
160+
(query, params), = calls
161+
assert "TSModule {_module: $key}" in query
162+
assert params == {"key": "src/mod.ts"}
163+
164+
165+
def test_module_keys_query_scoped_to_resolved_application_id(monkeypatch):
166+
backend, calls = _recording_backend(monkeypatch)
167+
backend._app_id = "can://repo-a/frontend"
168+
backend._load_module_keys()
169+
(query, params), = calls
170+
assert "Application {id: $app_id}" in query
171+
assert "-[:TS_HAS_MODULE]->" in query
172+
assert params == {"app_id": "can://repo-a/frontend"}
173+
174+
73175
# -----[ accessors with no graph support in schema 2.0.0 ]-----
74176
_FALLBACK_CALLS = [
75177
("get_decorators", ("src/x.f",)),

0 commit comments

Comments
 (0)