diff --git a/src/loadpath/architecture/rules.py b/src/loadpath/architecture/rules.py index 91e927e..c3f68ce 100644 --- a/src/loadpath/architecture/rules.py +++ b/src/loadpath/architecture/rules.py @@ -296,19 +296,40 @@ def _task_idempotency(store: GraphStore, changed_ids: set[str] | None) -> list[F return out +REL_FIELD_TYPES = { + "ForeignKey", + "OneToOneField", + "ManyToManyField", + "GenericForeignKey", + "GenericRelation", +} + + def _nplusone(store: GraphStore) -> list[Finding]: out: list[Finding] = [] + fields = list(store.nodes([NodeType.FIELD])) + fields_by_name: dict[str, list[dict]] = {} + for field in fields: + fields_by_name.setdefault(field["name"], []).append(field) for node in store.nodes(): hits = (node.get("extra") or {}).get("nplusone") or [] + owner_app = (node.get("extra") or {}).get("app") for hit in hits: - accessed = ", ".join(hit.get("accessed") or []) or "related fields" + accessed = list(hit.get("accessed") or []) + related, conf = _related_accesses(accessed, fields_by_name, owner_app) + if not related: + continue + hit = dict(hit) + hit["accessed"] = related + hit["confidence"] = conf + accessed_s = ", ".join(related) fix = hit.get("suggested_fix") or ".select_related()" out.append( Finding( rule="queryset_nplusone", severity=RuleSeverity.WARNING, message=( - f"{node['name']} loops `{hit.get('loop_var')}` over a queryset and touches {accessed} " + f"{node['name']} loops `{hit.get('loop_var')}` over a queryset and touches {accessed_s} " f"without {fix} ({node.get('file_path')}:{hit.get('line')})" ), node_id=node["id"], @@ -319,6 +340,33 @@ def _nplusone(store: GraphStore) -> list[Finding]: return out +def _related_accesses( + accessed: list[str], fields_by_name: dict[str, list[dict]], owner_app: str | None +) -> tuple[list[str], str]: + related: list[str] = [] + unknown = False + for name in accessed: + matches = fields_by_name.get(name) or [] + if owner_app: + scoped = [f for f in matches if (f.get("extra") or {}).get("app") == owner_app] + if scoped: + matches = scoped + if not matches: + related.append(name) + unknown = True + continue + if any(_is_relation(f) for f in matches): + related.append(name) + return related, ("medium" if unknown else "high") + + +def _is_relation(field: dict) -> bool: + extra = field.get("extra") or {} + if extra.get("relation"): + return True + return extra.get("field_type") in REL_FIELD_TYPES + + def _missing_index(store: GraphStore) -> list[Finding]: out: list[Finding] = [] fields_by_name: dict[str, list[dict]] = {} diff --git a/src/loadpath/extractors/django_boot.py b/src/loadpath/extractors/django_boot.py index a969356..a241d7e 100644 --- a/src/loadpath/extractors/django_boot.py +++ b/src/loadpath/extractors/django_boot.py @@ -2,15 +2,136 @@ from __future__ import annotations +import json import os +import subprocess import sys from pathlib import Path from loadpath.config import LoadpathConfig from loadpath.types import Edge, EdgeType, ExtractedGraph, Node, NodeType, node_id +BOOT_JSON_MARKER = "__LOADPATH_BOOT_JSON__" + def try_boot_models(repo_root: Path, config: LoadpathConfig) -> ExtractedGraph: + """Boot Django in a subprocess so django.setup() is not process-global.""" + if os.environ.get("LOADPATH_BOOT_INPROCESS") == "1": + return _boot_inprocess(repo_root, config) + return _boot_subprocess(repo_root, config) + + +def _boot_subprocess(repo_root: Path, config: LoadpathConfig) -> ExtractedGraph: + src_root = Path(__file__).resolve().parents[2] + env = os.environ.copy() + env["LOADPATH_BOOT_INPROCESS"] = "1" + env["PYTHONPATH"] = str(src_root) + os.pathsep + env.get("PYTHONPATH", "") + payload = json.dumps( + { + "repo_root": str(repo_root.resolve()), + "django_root": config.django_root, + } + ) + code = ( + "import io,json,sys\n" + "from contextlib import redirect_stdout\n" + "from pathlib import Path\n" + "from loadpath.config import load_config\n" + "from loadpath.extractors.django_boot import _boot_inprocess\n" + "meta=json.loads(sys.argv[1])\n" + "root=Path(meta['repo_root'])\n" + "cfg=load_config(root)\n" + "cfg.django_root=meta['django_root']\n" + "cfg.boot_django=True\n" + "buf=io.StringIO()\n" + "with redirect_stdout(buf):\n" + " g=_boot_inprocess(root,cfg)\n" + "print(" + repr(BOOT_JSON_MARKER) + " + json.dumps(" + "{'nodes':[n.to_row() for n in g.nodes]," + "'edges':[e.to_row() for e in g.edges],'residuals':g.residuals}))\n" + ) + try: + proc = subprocess.run( + [sys.executable, "-c", code, payload], + capture_output=True, + text=True, + timeout=45, + env=env, + cwd=str(repo_root), + ) + except subprocess.TimeoutExpired: + graph = ExtractedGraph() + graph.residuals.append("django.setup() skipped: boot subprocess timed out") + return graph + if proc.returncode != 0: + graph = ExtractedGraph() + err = (proc.stderr or proc.stdout or "unknown error").strip().splitlines() + tail = err[-1] if err else "unknown error" + graph.residuals.append(f"django.setup() skipped: {tail}") + return graph + data = _parse_boot_payload(proc.stdout) + if data is None: + graph = ExtractedGraph() + graph.residuals.append("django.setup() skipped: boot subprocess returned invalid JSON") + return graph + return _graph_from_boot_data(data) + + +def _graph_from_boot_data(data: dict) -> ExtractedGraph: + graph = ExtractedGraph() + graph.residuals.extend(data.get("residuals") or []) + try: + for row in data.get("nodes") or []: + extra = row.get("extra") or {} + if isinstance(extra, str): + extra = json.loads(extra) + graph.nodes.append( + Node( + id=row["id"], + type=NodeType(row["type"]), + name=row["name"], + qualified_name=row["qualified_name"], + file_path=row.get("file_path"), + start_line=row.get("start_line"), + end_line=row.get("end_line"), + context=row.get("context"), + extra=extra if isinstance(extra, dict) else {}, + ) + ) + for row in data.get("edges") or []: + extra = row.get("extra") or {} + if isinstance(extra, str): + extra = json.loads(extra) + graph.edges.append( + Edge( + src=row["src"], + dst=row["dst"], + type=EdgeType(row["type"]), + confidence=float(row.get("confidence") or 1), + extra=extra if isinstance(extra, dict) else {}, + ) + ) + except (KeyError, TypeError, ValueError, json.JSONDecodeError) as exc: + graph = ExtractedGraph() + graph.residuals.append(f"django.setup() skipped: boot payload malformed ({exc})") + return graph + + +def _parse_boot_payload(stdout: str | None) -> dict | None: + text = stdout or "" + idx = text.rfind(BOOT_JSON_MARKER) + blob = text[idx + len(BOOT_JSON_MARKER) :] if idx >= 0 else text + blob = blob.strip().splitlines()[0] if blob.strip() else "" + if not blob: + return None + try: + data = json.loads(blob) + except json.JSONDecodeError: + return None + return data if isinstance(data, dict) else None + + +def _boot_inprocess(repo_root: Path, config: LoadpathConfig) -> ExtractedGraph: graph = ExtractedGraph() settings_mod = _discover_settings_module(repo_root, config.django_root) if not settings_mod: diff --git a/src/loadpath/extractors/react.py b/src/loadpath/extractors/react.py index 4066bc9..9e42d7e 100644 --- a/src/loadpath/extractors/react.py +++ b/src/loadpath/extractors/react.py @@ -236,12 +236,22 @@ def edge(src: str, dst: str, etype: EdgeType, confidence: float = 1.0, extra: di continue line = source[: m.start()].count("\n") + 1 norm = normalize_url_template(url) + generated_file = "/generated/" in f"/{rel}/" or "openapi" in Path(rel).stem.lower() + qname = f"client:{rel}:{norm}" + if any(n.qualified_name == qname for n in graph.nodes): + continue client = add( NodeType.API_CLIENT, norm, - f"client:{norm}", + qname, line, - {"raw": url, "inferred": True, "feature": feature, "file": rel}, + { + "raw": url, + "inferred": not generated_file, + "generated": generated_file, + "feature": feature, + "file": rel, + }, ) for owner in hooks or components: edge(owner.id, client.id, EdgeType.CALLS) @@ -253,14 +263,22 @@ def edge(src: str, dst: str, etype: EdgeType, confidence: float = 1.0, extra: di url = m.group(1) line = source[: m.start()].count("\n") + 1 norm = normalize_url_template(url) - if any(n.qualified_name == f"client:{norm}" for n in graph.nodes): + generated_file = "/generated/" in f"/{rel}/" or "openapi" in Path(rel).stem.lower() + qname = f"client:{rel}:{norm}" + if any(n.qualified_name == qname for n in graph.nodes): continue add( NodeType.API_CLIENT, norm, - f"client:{norm}", + qname, line, - {"raw": url, "inferred": True, "feature": feature, "file": rel}, + { + "raw": url, + "inferred": not generated_file, + "generated": generated_file, + "feature": feature, + "file": rel, + }, ) for m in ROUTE_JSX_RE.finditer(source): diff --git a/src/loadpath/index.py b/src/loadpath/index.py index ea0a501..4a80eb7 100644 --- a/src/loadpath/index.py +++ b/src/loadpath/index.py @@ -14,6 +14,8 @@ PY_SKIP = {"migrations"} # still extract migrations, just not skip INDEX_EXTENSIONS = {".py", ".ts", ".tsx", ".js", ".jsx"} +# Bump when extractor/stitch node identity changes so incremental indexes rebuild. +INDEX_REVISION = "3" def default_db_path(repo_root: Path) -> Path: @@ -80,6 +82,7 @@ def _sidecar_digest(repo_root: Path, config: LoadpathConfig) -> str: digest.update(rel.encode()) digest.update(path.read_bytes()) digest.update(_config_digest(repo_root).encode()) + digest.update(INDEX_REVISION.encode()) return digest.hexdigest() diff --git a/src/loadpath/review/confidence.py b/src/loadpath/review/confidence.py index faa3005..6ae15de 100644 --- a/src/loadpath/review/confidence.py +++ b/src/loadpath/review/confidence.py @@ -40,14 +40,17 @@ def score_confidence( tested_ids: set[str] = set() impact_ids = {n["id"] for n in impact_nodes} - all_edges = list(store.edges()) - for e in list(impact_edges) + all_edges: - if e["type"] == EdgeType.TESTED_BY.value: + for e in impact_edges: + if e["type"] != EdgeType.TESTED_BY.value: + continue + if e["src"] in impact_ids and e["dst"] in impact_ids: tested_ids.add(e["src"]) - # A sink is covered if it, or a producer within two hops (view/serializer/hook/page), is tested. + # A sink is covered if it, or a producer within two hops on THIS path, is tested. inbound: dict[str, list[str]] = {} - for e in all_edges: + for e in impact_edges: + if e["src"] not in impact_ids or e["dst"] not in impact_ids: + continue inbound.setdefault(e["dst"], []).append(e["src"]) inbound.setdefault(e["src"], []).append(e["dst"]) diff --git a/src/loadpath/review/engine.py b/src/loadpath/review/engine.py index 85b3faa..f3fc704 100644 --- a/src/loadpath/review/engine.py +++ b/src/loadpath/review/engine.py @@ -5,7 +5,7 @@ from pathlib import Path from uuid import uuid4 -from loadpath.architecture.rules import evaluate +from loadpath.architecture.rules import _related_accesses, evaluate from loadpath.config import LoadpathConfig, load_config from loadpath.graph.store import GraphStore from loadpath.index import default_db_path, index_drift, index_repo @@ -243,6 +243,9 @@ def collect_residuals(store: GraphStore, impact_nodes: list[dict], diff: DiffSet for line in stored.splitlines(): if any(f and f in line for f in impact_files) or any(n and str(n) in line for n in impact_names): residuals.append(line) + fields_by_name: dict[str, list[dict]] = {} + for field in store.nodes([NodeType.FIELD]): + fields_by_name.setdefault(field["name"], []).append(field) for n in impact_nodes: extra = n.get("extra") or {} if extra.get("get_serializer_class"): @@ -254,12 +257,28 @@ def collect_residuals(store: GraphStore, impact_nodes: list[dict], diff: DiffSet if extra.get("queryset_in_serializer"): residuals.append(f"Queryset inside serializer {n['qualified_name']}") for hit in extra.get("nplusone") or []: - accessed = ", ".join(hit.get("accessed") or []) or "related fields" + accessed = list(hit.get("accessed") or []) + related, _ = _related_accesses(accessed, fields_by_name, extra.get("app")) + if not related: + continue residuals.append( - f"N+1 {accessed} in {n.get('file_path')}:{hit.get('line')} — {hit.get('suggested_fix')}" + f"N+1 {', '.join(related)} in {n.get('file_path')}:{hit.get('line')} — {hit.get('suggested_fix')}" ) residuals.extend(_test_field_residuals(impact_nodes, diff)) residuals.extend(_react_path_residuals(impact_nodes, diff)) + ids = {n["id"] for n in impact_nodes} + for e in store.edges(): + if e["src"] not in ids or e["dst"] not in ids: + continue + extra = e.get("extra") or {} + if extra.get("overlap"): + residuals.append( + f"Inferred serializer/Zod overlap fields={extra['overlap']}" + ) + if extra.get("superseded_by_generated"): + residuals.append( + f"String URL stitch {extra.get('react')} superseded by a generated OpenAPI client" + ) seen = set() out = [] for r in residuals: @@ -269,6 +288,11 @@ def collect_residuals(store: GraphStore, impact_nodes: list[dict], diff: DiffSet return out +def _serious_evolution_notes(notes: list[str]) -> list[str]: + tokens = ("hotspot", "silo", "crosses a bounded", "cross-context", "temporal coupling") + return [n for n in notes if any(tok in n.lower() for tok in tokens)] + + def suggested_reviewers(config: LoadpathConfig, impact_nodes: list[dict]) -> list[str]: owners: list[str] = [] for n in impact_nodes: @@ -358,10 +382,11 @@ def run_review( residuals = collect_residuals(store, impact_nodes, diff) evolution = analyze_evolution(repo_root, diff, impact_nodes, config) confidence = score_confidence(store, impact_nodes, impact_edges, scoped, residuals) - if evolution.get("notes") and confidence["level"] == "high": + serious = _serious_evolution_notes(evolution.get("notes") or []) + if serious and confidence["level"] == "high": confidence["level"] = "medium" reasons = list(confidence.get("reasons") or []) - reasons = [evolution["notes"][0], *reasons][:3] + reasons = [serious[0], *reasons][:3] confidence["reasons"] = reasons boot = store.get_meta("django_boot") or "off" if boot == "failed" and confidence["level"] == "high": diff --git a/src/loadpath/stitch/openapi.py b/src/loadpath/stitch/openapi.py index 8b8f20c..b544c43 100644 --- a/src/loadpath/stitch/openapi.py +++ b/src/loadpath/stitch/openapi.py @@ -149,41 +149,19 @@ def stitch(store: GraphStore, config: LoadpathConfig, repo_root: Path) -> list[s ser_fields = [n for n in store.nodes([NodeType.SERIALIZER_FIELD])] schemas = [n for n in store.nodes([NodeType.FORM_SCHEMA])] generated_files = _generated_client_files(repo_root, config) - - # Route → OpenAPI - for route in routes: - extra = route.get("extra") or {} - if extra.get("include"): - continue - raw = extra.get("mounted_at") or extra.get("full_path") or extra.get("route") or route["name"] - tmpl = django_route_to_template(str(raw)) - ops = openapi_by_path.get(tmpl, []) - if not ops: - ops = [item for p, items in openapi_by_path.items() if _paths_match(tmpl, p) for item in items] - for op in ops: - store.upsert_edge( - Edge( - src=route["id"], - dst=node_id(NodeType.OPENAPI_PATH, f"{op['method']} {op['path']}"), - type=EdgeType.PUBLISHES_ROUTE, - confidence=1.0, - extra={"via": "openapi"}, - ) - ) + generated_templates: set[str] = set() + for client in clients: + raw = (client.get("extra") or {}).get("raw") or client["name"] + tmpl = normalize_url_template(str(raw)) + if _client_is_generated(client, generated_files): + generated_templates.add(tmpl) # Clients consumed_by matching routes / openapi for client in clients: raw = (client.get("extra") or {}).get("raw") or client["name"] tmpl = normalize_url_template(str(raw)) matched = False - generated = any( - client.get("file_path") and str(client["file_path"]).replace("\\", "/").endswith(g.replace("\\", "/")) - or (client.get("file_path") and g.replace("\\", "/") in str(client["file_path"]).replace("\\", "/")) - for g in generated_files - ) - if client.get("file_path"): - fp = str(client["file_path"]).replace("\\", "/") - generated = generated or "/generated/" in f"/{fp}/" or "openapi" in Path(fp).name.lower() + generated = _client_is_generated(client, generated_files) for route in routes: extra = route.get("extra") or {} @@ -192,29 +170,42 @@ def stitch(store: GraphStore, config: LoadpathConfig, repo_root: Path) -> list[s rraw = extra.get("mounted_at") or extra.get("full_path") or extra.get("route") or route["name"] rtmpl = django_route_to_template(str(rraw)) if _paths_match(tmpl, rtmpl): - conf = 0.95 if generated else 0.55 + if generated: + conf = 0.95 + elif tmpl in generated_templates: + conf = 0.4 + else: + conf = 0.55 store.upsert_edge( Edge( src=route["id"], dst=client["id"], type=EdgeType.CONSUMED_BY_CLIENT, confidence=conf, - extra={"match": "url_template", "generated_client": generated, "django": rtmpl, "react": tmpl}, + extra={ + "match": "url_template", + "generated_client": generated, + "django": rtmpl, + "react": tmpl, + "superseded_by_generated": bool(not generated and tmpl in generated_templates), + }, ) ) matched = True if not generated: - residuals.append( - f"Inferred client stitch {tmpl} ↔ {rtmpl} from string URL in {client.get('file_path')} " - "(not a generated OpenAPI client)" - ) + note = f"Inferred client stitch {tmpl} ↔ {rtmpl} from string URL in {client.get('file_path')}" + if tmpl in generated_templates: + note += " (generated OpenAPI client already covers this URL)" + else: + note += " (not a generated OpenAPI client)" + residuals.append(note) for op in openapi_by_path.get(tmpl, []): store.upsert_edge( Edge( src=node_id(NodeType.OPENAPI_PATH, f"{op['method']} {op['path']}"), dst=client["id"], type=EdgeType.CONSUMED_BY_CLIENT, - confidence=1.0 if generated else 0.7, + confidence=1.0 if generated else (0.45 if tmpl in generated_templates else 0.7), extra={"via": "openapi", "generated_client": generated}, ) ) @@ -222,6 +213,26 @@ def stitch(store: GraphStore, config: LoadpathConfig, repo_root: Path) -> list[s if not matched and tmpl.startswith("/api/"): residuals.append(f"React client {tmpl} has no matching Django route ({client.get('file_path')})") + for route in routes: + extra = route.get("extra") or {} + if extra.get("include"): + continue + raw = extra.get("mounted_at") or extra.get("full_path") or extra.get("route") or route["name"] + tmpl = django_route_to_template(str(raw)) + ops = openapi_by_path.get(tmpl, []) + if not ops: + ops = [item for p, items in openapi_by_path.items() if _paths_match(tmpl, p) for item in items] + for op in ops: + store.upsert_edge( + Edge( + src=route["id"], + dst=node_id(NodeType.OPENAPI_PATH, f"{op['method']} {op['path']}"), + type=EdgeType.PUBLISHES_ROUTE, + confidence=1.0, + extra={"via": "openapi"}, + ) + ) + # Serializer field ↔ Zod field overlap fields_by_serializer: dict[str, list[dict]] = {} for f in ser_fields: @@ -309,6 +320,19 @@ def _paths_match(a: str, b: str) -> bool: return "/".join(a_tail) == "/".join(b_tail) and len(a_tail[-1]) > 2 +def _client_is_generated(client: dict, generated_files: list[str]) -> bool: + extra = client.get("extra") or {} + if extra.get("generated"): + return True + fp = str(client.get("file_path") or extra.get("file") or "").replace("\\", "/") + if not fp: + return False + generated = any( + fp.endswith(g.replace("\\", "/")) or g.replace("\\", "/") in fp for g in generated_files + ) + return generated or "/generated/" in f"/{fp}/" or "openapi" in Path(fp).name.lower() + + def _generated_client_files(repo_root: Path, config: LoadpathConfig) -> list[str]: found: list[str] = [] for pattern in config.generated_client_globs: diff --git a/tests/e2e/test_brokers_and_django.py b/tests/e2e/test_brokers_and_django.py index 529d602..598a6a0 100644 --- a/tests/e2e/test_brokers_and_django.py +++ b/tests/e2e/test_brokers_and_django.py @@ -89,6 +89,27 @@ def test_django_boot_overlay_reports_skip_or_models(tmp_path): assert any(n.name == "Invoice" for n in graph.nodes) +def test_boot_payload_ignores_stdout_noise(): + from loadpath.extractors.django_boot import BOOT_JSON_MARKER, _parse_boot_payload + + raw = ( + "Watching for file changes with StatReloader\n" + + BOOT_JSON_MARKER + + '{"nodes":[],"edges":[],"residuals":["django.setup() skipped: boom"]}\n' + ) + data = _parse_boot_payload(raw) + assert data is not None + assert data["residuals"][0].startswith("django.setup() skipped:") + + +def test_boot_payload_malformed_nodes_become_residual(): + from loadpath.extractors.django_boot import _graph_from_boot_data + + graph = _graph_from_boot_data({"nodes": [{"id": 1}], "edges": [], "residuals": []}) + assert not graph.nodes + assert any("django.setup() skipped: boot payload malformed" in r for r in graph.residuals) + + def test_index_counts_grow_with_new_django_files(tmp_path): store = index_repo(FIXTURE_ROOT, db_path=tmp_path / "g.sqlite3", incremental=False) types = {n["type"] for n in store.nodes()} diff --git a/tests/unit/test_architecture_rules.py b/tests/unit/test_architecture_rules.py index b22ccb1..cce461f 100644 --- a/tests/unit/test_architecture_rules.py +++ b/tests/unit/test_architecture_rules.py @@ -2,10 +2,11 @@ from pathlib import Path -from loadpath.architecture.rules import evaluate +from loadpath.architecture.rules import _nplusone, _related_accesses, evaluate from loadpath.config import load_config +from loadpath.graph.store import GraphStore from loadpath.index import index_repo -from loadpath.types import NodeType +from loadpath.types import EdgeType, Node, NodeType, node_id from tests.conftest import FIXTURE_ROOT as FIXTURE @@ -65,6 +66,109 @@ def test_nplusone_rule_on_fixture_service(tmp_path: Path): store.close() +def _field(name: str, field_type: str, app: str = "billing") -> Node: + qname = f"{app}.Invoice.{name}" + return Node( + id=node_id(NodeType.FIELD, qname), + type=NodeType.FIELD, + name=name, + qualified_name=qname, + extra={ + "app": app, + "field_type": field_type, + "relation": field_type in {"ForeignKey", "OneToOneField", "ManyToManyField"}, + }, + ) + + +def test_related_accesses_drops_charfield_keeps_fk(): + fields = { + "status": [_field("status", "CharField").to_row()], + "account": [_field("account", "ForeignKey").to_row()], + } + related, conf = _related_accesses(["status", "account"], fields, "billing") + assert related == ["account"] + assert conf == "high" + related, conf = _related_accesses(["status"], fields, "billing") + assert related == [] + related, conf = _related_accesses(["ghost"], fields, "billing") + assert related == ["ghost"] + assert conf == "medium" + + +def test_nplusone_schema_aware_does_not_add_edges(tmp_path: Path): + store = GraphStore(tmp_path / "g.sqlite3") + status = _field("status", "CharField") + account = _field("account", "ForeignKey") + service = Node( + id=node_id(NodeType.SERVICE, "billing.overdue"), + type=NodeType.SERVICE, + name="overdue", + qualified_name="billing.overdue", + extra={ + "app": "billing", + "nplusone": [ + { + "accessed": ["status"], + "loop_var": "invoice", + "line": 4, + "suggested_fix": ".select_related()", + }, + { + "accessed": ["account"], + "loop_var": "invoice", + "line": 8, + "suggested_fix": ".select_related('account')", + }, + ], + }, + ) + store.upsert_node(status) + store.upsert_node(account) + store.upsert_node(service) + store.conn.commit() + before = store.edges() + hits = _nplusone(store) + assert not any("status" in (f.extra.get("accessed") or []) for f in hits) + assert any(f.extra.get("accessed") == ["account"] and f.extra.get("confidence") == "high" for f in hits) + assert store.edges() == before + assert not any(e["type"] == EdgeType.RELATES_TO.value for e in store.edges()) + store.close() + + +def test_nplusone_charfield_is_not_a_residual(tmp_path: Path): + from loadpath.review.engine import collect_residuals + + store = GraphStore(tmp_path / "g.sqlite3") + status = _field("status", "CharField") + account = _field("account", "ForeignKey") + service = Node( + id=node_id(NodeType.SERVICE, "billing.overdue"), + type=NodeType.SERVICE, + name="overdue", + qualified_name="billing.overdue", + extra={ + "app": "billing", + "nplusone": [ + { + "accessed": ["status"], + "loop_var": "invoice", + "line": 4, + "suggested_fix": ".select_related()", + } + ], + }, + ) + store.upsert_node(status) + store.upsert_node(account) + store.upsert_node(service) + store.conn.commit() + blob = " ".join(collect_residuals(store, [service.to_row()])) + assert "N+1" not in blob + assert "status" not in blob + store.close() + + def test_cascade_across_contexts_is_warning(tmp_path: Path): import shutil diff --git a/tests/unit/test_confidence.py b/tests/unit/test_confidence.py new file mode 100644 index 0000000..0aa9b79 --- /dev/null +++ b/tests/unit/test_confidence.py @@ -0,0 +1,92 @@ +from __future__ import annotations + +from pathlib import Path + +from loadpath.graph.store import GraphStore +from loadpath.review.confidence import score_confidence +from loadpath.review.engine import _serious_evolution_notes +from loadpath.types import Edge, EdgeType, Node, NodeType, node_id + + +def _store(tmp_path: Path) -> GraphStore: + return GraphStore(tmp_path / "g.sqlite3") + + +def _node(ntype: NodeType, name: str, **extra) -> Node: + return Node( + id=node_id(ntype, name), + type=ntype, + name=name, + qualified_name=name, + extra=extra, + ) + + +def test_off_path_tested_by_does_not_cover_sink(tmp_path: Path): + store = _store(tmp_path) + sink = _node(NodeType.ROUTE, "/api/invoices/{id}") + other = _node(NodeType.SERVICE, "unrelated") + test = _node(NodeType.TEST, "test_unrelated") + store.upsert_node(sink) + store.upsert_node(other) + store.upsert_node(test) + store.upsert_edge(Edge(src=other.id, dst=test.id, type=EdgeType.TESTED_BY)) + store.upsert_edge(Edge(src=sink.id, dst=test.id, type=EdgeType.TESTED_BY)) + store.conn.commit() + + impact_nodes = [sink.to_row()] + confidence = score_confidence(store, impact_nodes, impact_edges=[], findings=[], residuals=[]) + assert confidence["covered_sinks"] == 0 + assert sink.id in {s["id"] for s in confidence["untested_sinks"]} + store.close() + + +def test_on_path_tested_by_covers_sink(tmp_path: Path): + store = _store(tmp_path) + sink = _node(NodeType.ROUTE, "/api/invoices/{id}") + test = _node(NodeType.TEST, "test_invoice_route") + edge = Edge(src=sink.id, dst=test.id, type=EdgeType.TESTED_BY, confidence=1.0) + store.upsert_node(sink) + store.upsert_node(test) + store.upsert_edge(edge) + store.conn.commit() + + impact_nodes = [sink.to_row(), test.to_row()] + impact_edges = [edge.to_row()] + confidence = score_confidence(store, impact_nodes, impact_edges, findings=[], residuals=[]) + assert confidence["covered_sinks"] == 1 + assert confidence["untested_sinks"] == [] + store.close() + + +def test_two_hop_tested_producer_on_path_covers_sink(tmp_path: Path): + store = _store(tmp_path) + sink = _node(NodeType.PAGE, "InvoicePage") + hook = _node(NodeType.HOOK, "useInvoice") + test = _node(NodeType.REACT_TEST, "InvoicePage.test") + calls = Edge(src=sink.id, dst=hook.id, type=EdgeType.CALLS, confidence=1.0) + tested = Edge(src=hook.id, dst=test.id, type=EdgeType.TESTED_BY, confidence=1.0) + for node in (sink, hook, test): + store.upsert_node(node) + store.upsert_edge(calls) + store.upsert_edge(tested) + store.conn.commit() + + impact_nodes = [sink.to_row(), hook.to_row(), test.to_row()] + impact_edges = [calls.to_row(), tested.to_row()] + confidence = score_confidence(store, impact_nodes, impact_edges, findings=[], residuals=[]) + assert confidence["covered_sinks"] == 1 + store.close() + + +def test_weak_evolution_notes_are_not_serious(): + weak = [ + "serializers.py changed with cyclomatic complexity 4 in a historically active file", + "billing/views.py::create changed with cyclomatic complexity 3", + ] + assert _serious_evolution_notes(weak) == [] + serious = [ + "billing/views.py is a hotspot (12 commits, knowledge silo: ada)", + "Temporal coupling a.py ↔ b.py (5 co-changes, degree 0.5) crosses a bounded context", + ] + assert _serious_evolution_notes(serious) == serious diff --git a/tests/unit/test_index_and_stitch.py b/tests/unit/test_index_and_stitch.py index 5a94d42..23b8802 100644 --- a/tests/unit/test_index_and_stitch.py +++ b/tests/unit/test_index_and_stitch.py @@ -17,6 +17,13 @@ def test_index_stitches_django_route_to_react_client(tmp_path: Path): assert consumed, "expected URL stitch between Django routes and React fetch" inferred = [e for e in consumed if e["confidence"] < 0.9] assert inferred, "string-matched fetch must be marked inferred (lower confidence)" + generated = [ + e + for e in consumed + if (e.get("extra") or {}).get("generated_client") and e["confidence"] >= 0.9 + ] + assert generated, "generated OpenAPI client should stitch at high confidence" + assert any((e.get("extra") or {}).get("superseded_by_generated") for e in inferred) schema_edges = [e for e in edges if e["type"] == "matches_schema"] assert schema_edges, "serializer fields should overlap invoiceSchema" store.close() diff --git a/tests/unit/test_react_extractors.py b/tests/unit/test_react_extractors.py index 0035d63..e5885ae 100644 --- a/tests/unit/test_react_extractors.py +++ b/tests/unit/test_react_extractors.py @@ -41,6 +41,18 @@ def test_extracts_hook_query_key_and_fetch_url(): assert any(c.name == "/api/invoices/{id}" for c in clients) +def test_generated_client_is_not_inferred(): + g = extract_react_file( + "frontend/src/generated/invoices.ts", + (FIXTURE / "frontend/src/generated/invoices.ts").read_text(), + _cfg(), + ) + clients = [n for n in g.nodes if n.type is NodeType.API_CLIENT] + assert clients + assert all(c.extra.get("generated") for c in clients) + assert all(not c.extra.get("inferred") for c in clients) + + def test_extracts_zod_schema_fields(): g = extract_react_file( "frontend/src/features/billing/invoiceSchema.ts",