Skip to content

Commit ab99fa0

Browse files
Fix adversarial findings on N+1 residuals, index identity, and boot payload.
CharField N+1 hits were still copied into review residuals after the rule dropped them. Client node-id changes now bump INDEX_REVISION so incremental graphs rebuild. Malformed django.setup() JSON becomes a skip residual. Co-authored-by: Damon <Modsofthenation@users.noreply.github.com>
1 parent 3f5467e commit ab99fa0

5 files changed

Lines changed: 89 additions & 25 deletions

File tree

‎src/loadpath/extractors/django_boot.py‎

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -74,32 +74,46 @@ def _boot_subprocess(repo_root: Path, config: LoadpathConfig) -> ExtractedGraph:
7474
graph = ExtractedGraph()
7575
graph.residuals.append("django.setup() skipped: boot subprocess returned invalid JSON")
7676
return graph
77+
return _graph_from_boot_data(data)
78+
79+
80+
def _graph_from_boot_data(data: dict) -> ExtractedGraph:
7781
graph = ExtractedGraph()
7882
graph.residuals.extend(data.get("residuals") or [])
79-
for row in data.get("nodes") or []:
80-
graph.nodes.append(
81-
Node(
82-
id=row["id"],
83-
type=NodeType(row["type"]),
84-
name=row["name"],
85-
qualified_name=row["qualified_name"],
86-
file_path=row.get("file_path"),
87-
start_line=row.get("start_line"),
88-
end_line=row.get("end_line"),
89-
context=row.get("context"),
90-
extra=row.get("extra") or {},
83+
try:
84+
for row in data.get("nodes") or []:
85+
extra = row.get("extra") or {}
86+
if isinstance(extra, str):
87+
extra = json.loads(extra)
88+
graph.nodes.append(
89+
Node(
90+
id=row["id"],
91+
type=NodeType(row["type"]),
92+
name=row["name"],
93+
qualified_name=row["qualified_name"],
94+
file_path=row.get("file_path"),
95+
start_line=row.get("start_line"),
96+
end_line=row.get("end_line"),
97+
context=row.get("context"),
98+
extra=extra if isinstance(extra, dict) else {},
99+
)
91100
)
92-
)
93-
for row in data.get("edges") or []:
94-
graph.edges.append(
95-
Edge(
96-
src=row["src"],
97-
dst=row["dst"],
98-
type=EdgeType(row["type"]),
99-
confidence=float(row.get("confidence") or 1),
100-
extra=row.get("extra") or {},
101+
for row in data.get("edges") or []:
102+
extra = row.get("extra") or {}
103+
if isinstance(extra, str):
104+
extra = json.loads(extra)
105+
graph.edges.append(
106+
Edge(
107+
src=row["src"],
108+
dst=row["dst"],
109+
type=EdgeType(row["type"]),
110+
confidence=float(row.get("confidence") or 1),
111+
extra=extra if isinstance(extra, dict) else {},
112+
)
101113
)
102-
)
114+
except (KeyError, TypeError, ValueError, json.JSONDecodeError) as exc:
115+
graph = ExtractedGraph()
116+
graph.residuals.append(f"django.setup() skipped: boot payload malformed ({exc})")
103117
return graph
104118

105119

‎src/loadpath/index.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
PY_SKIP = {"migrations"} # still extract migrations, just not skip
1616
INDEX_EXTENSIONS = {".py", ".ts", ".tsx", ".js", ".jsx"}
17+
# Bump when extractor/stitch node identity changes so incremental indexes rebuild.
18+
INDEX_REVISION = "3"
1719

1820

1921
def default_db_path(repo_root: Path) -> Path:
@@ -80,6 +82,7 @@ def _sidecar_digest(repo_root: Path, config: LoadpathConfig) -> str:
8082
digest.update(rel.encode())
8183
digest.update(path.read_bytes())
8284
digest.update(_config_digest(repo_root).encode())
85+
digest.update(INDEX_REVISION.encode())
8386
return digest.hexdigest()
8487

8588

‎src/loadpath/review/engine.py‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from pathlib import Path
66
from uuid import uuid4
77

8-
from loadpath.architecture.rules import evaluate
8+
from loadpath.architecture.rules import _related_accesses, evaluate
99
from loadpath.config import LoadpathConfig, load_config
1010
from loadpath.graph.store import GraphStore
1111
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
243243
for line in stored.splitlines():
244244
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):
245245
residuals.append(line)
246+
fields_by_name: dict[str, list[dict]] = {}
247+
for field in store.nodes([NodeType.FIELD]):
248+
fields_by_name.setdefault(field["name"], []).append(field)
246249
for n in impact_nodes:
247250
extra = n.get("extra") or {}
248251
if extra.get("get_serializer_class"):
@@ -254,9 +257,12 @@ def collect_residuals(store: GraphStore, impact_nodes: list[dict], diff: DiffSet
254257
if extra.get("queryset_in_serializer"):
255258
residuals.append(f"Queryset inside serializer {n['qualified_name']}")
256259
for hit in extra.get("nplusone") or []:
257-
accessed = ", ".join(hit.get("accessed") or []) or "related fields"
260+
accessed = list(hit.get("accessed") or [])
261+
related, _ = _related_accesses(accessed, fields_by_name, extra.get("app"))
262+
if not related:
263+
continue
258264
residuals.append(
259-
f"N+1 {accessed} in {n.get('file_path')}:{hit.get('line')} — {hit.get('suggested_fix')}"
265+
f"N+1 {', '.join(related)} in {n.get('file_path')}:{hit.get('line')} — {hit.get('suggested_fix')}"
260266
)
261267
residuals.extend(_test_field_residuals(impact_nodes, diff))
262268
residuals.extend(_react_path_residuals(impact_nodes, diff))

‎tests/e2e/test_brokers_and_django.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@ def test_boot_payload_ignores_stdout_noise():
102102
assert data["residuals"][0].startswith("django.setup() skipped:")
103103

104104

105+
def test_boot_payload_malformed_nodes_become_residual():
106+
from loadpath.extractors.django_boot import _graph_from_boot_data
107+
108+
graph = _graph_from_boot_data({"nodes": [{"id": 1}], "edges": [], "residuals": []})
109+
assert not graph.nodes
110+
assert any("django.setup() skipped: boot payload malformed" in r for r in graph.residuals)
111+
112+
105113
def test_index_counts_grow_with_new_django_files(tmp_path):
106114
store = index_repo(FIXTURE_ROOT, db_path=tmp_path / "g.sqlite3", incremental=False)
107115
types = {n["type"] for n in store.nodes()}

‎tests/unit/test_architecture_rules.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,39 @@ def test_nplusone_schema_aware_does_not_add_edges(tmp_path: Path):
136136
store.close()
137137

138138

139+
def test_nplusone_charfield_is_not_a_residual(tmp_path: Path):
140+
from loadpath.review.engine import collect_residuals
141+
142+
store = GraphStore(tmp_path / "g.sqlite3")
143+
status = _field("status", "CharField")
144+
account = _field("account", "ForeignKey")
145+
service = Node(
146+
id=node_id(NodeType.SERVICE, "billing.overdue"),
147+
type=NodeType.SERVICE,
148+
name="overdue",
149+
qualified_name="billing.overdue",
150+
extra={
151+
"app": "billing",
152+
"nplusone": [
153+
{
154+
"accessed": ["status"],
155+
"loop_var": "invoice",
156+
"line": 4,
157+
"suggested_fix": ".select_related()",
158+
}
159+
],
160+
},
161+
)
162+
store.upsert_node(status)
163+
store.upsert_node(account)
164+
store.upsert_node(service)
165+
store.conn.commit()
166+
blob = " ".join(collect_residuals(store, [service.to_row()]))
167+
assert "N+1" not in blob
168+
assert "status" not in blob
169+
store.close()
170+
171+
139172
def test_cascade_across_contexts_is_warning(tmp_path: Path):
140173
import shutil
141174

0 commit comments

Comments
 (0)