ci: add graphify structural impact analysis to PR review and structure audit (#567)

* ci: add graphify structural impact analysis to PR review and structure audit

Add a graphify-based AST analysis tool that builds a directed graph of the
codebase (~2s, no LLM calls) to detect architectural impact. Integrates
into both the PR review workflow (pre-computed before claude runs) and the
Wednesday structure audit (with week-over-week diff).

PR review: extracts changed files against the full codebase graph, reports
risk level (LOW/MEDIUM/HIGH), god nodes affected, import direction
violations, and cross-package dependencies. Output saved to /tmp and read
by the review agent.

Structure audit: produces god node rankings, cross-package edge summary
table, import violation detection, and graph diff against previous week's
cached graph. Baselines saved for runner memory trend tracking.

* fix: harden graphify integration - security, correctness, and CI weight

- Fix KeyError: god_nodes() returns 'degree' not 'edges' (3 call sites)
- Fix deduped vs raw violation count inconsistency in baselines.json
- Security: run structural_impact.py from base-branch checkout so fork
  PRs cannot inject code that executes with GH_TOKEN in scope
- Add --repo-root flag so the tool resolves package paths correctly when
  invoked from a different checkout directory
- Replace make install-dev + .venv with lightweight /tmp/graphify-venv
  (only graphifyy needed, saves ~2min CI per PR review)
- Add graphify-out/ to .gitignore (9MB graph cache is CI-only)

* fix: pin graphifyy version and fix dedup truncation

Pin graphifyy==0.4.23 in both CI workflows to prevent
breaking changes from unpinned installs. Fix _dedup()
label truncation at 30 chars that could merge distinct
entities sharing a common prefix.

* fix(ci): use array expansion for changed-files arg to handle special filenames

Replace unquoted $CHANGED_PY word-split with mapfile + array
expansion to prevent glob expansion and correctly handle
filenames containing spaces or special characters.

* fix: derive changed nodes from graph and improve MEDIUM risk reason

Derive changed_node_ids from the already-built graph by matching
source_file paths instead of running a separate extraction pass.
Removes implicit dependency on graphify ID stability across
independent extractions.

Fix MEDIUM risk reason to reflect the actual trigger (cluster
spread vs high-connectivity entity) instead of always reporting
cluster count.

* fix: address Codex review findings - security, edge coverage, dedup, stale artifacts

Split the workflow step to isolate GH_TOKEN from graphifyy execution,
preventing a compromised package release from exfiltrating write-scoped
tokens.

Scan both edge directions in _cross_package_edges so inbound dependents
and violations where the changed node is the target are visible. Detect
deleted files and report them as a risk signal.

Include relation type in dedup key so distinct edge types between the
same labels are not collapsed.

Clean stale /tmp artifacts before running analysis to prevent reruns
from reading old reports.

* fix: address review feedback - type annotations, hoist imports, narrow except, isolate daily graphify

- structural_impact.py:
  - replace bare _build_graph dict return with frozen _Analysis dataclass
  - add G: Any annotation on _cross_package_edges (STYLEGUIDE: all params typed)
  - hoist `from graphify.export import to_json` and
    `from networkx.readwrite import json_graph` to module top
    (no perf justification for deferred import)
  - narrow `except Exception` in graph-diff fallback to
    (JSONDecodeError, KeyError, TypeError, OSError)
- agentic-ci-daily.yml: install graphifyy into /tmp/graphify-venv instead of
  the project .venv, matching agentic-ci-pr-review.yml. Keeps graphify's
  transitives (networkx) out of the project venv permanently.
- structure/recipe.md: invoke the tool via /tmp/graphify-venv/bin/python
  to match the workflow change.

* feat(ci): warn when changed files touch unknown packages

A new package under packages/ that isn't in _PACKAGE_SUBDIRS is silently
absent from the graph - the analyzer would falsely report LOW risk with
0 entities. Add a _Note line in the changed-files report when any changed
or deleted file lives under packages/<unknown>/, so the failure mode the
analyzer is supposed to surface isn't itself silent.

_KNOWN_PACKAGE_DIRS is derived from _PACKAGE_SUBDIRS so future additions
stay in sync without a second source of truth.

Author: andreatgretel

.agents/recipes/pr-review/recipe.md

@@ -17,10 +17,17 @@ Review pull request #{{pr_number}} using the `review-code` skill.
 
 ## Instructions
 
-1. Run `/review-code {{pr_number}}`
-2. The skill writes the review to `/tmp/review-{{pr_number}}.md`. If it does
+1. If `/tmp/structural-impact-{{pr_number}}.md` exists, read it. This contains
+   a pre-computed structural impact analysis (risk level, god nodes affected,
+   import direction violations, cross-package dependencies). Use it to inform
+   your review - high-risk PRs warrant extra scrutiny on blast radius and
+   backward compatibility.
+2. Run `/review-code {{pr_number}}`
+3. The skill writes the review to `/tmp/review-{{pr_number}}.md`. If it does
    not, save the review output there yourself.
-3. Before finishing, read `/tmp/review-{{pr_number}}.md` and verify it contains
+4. If the structural impact analysis exists, append its content to the review
+   file (before the Verdict section) under a `### Structural Impact` heading.
+5. Before finishing, read `/tmp/review-{{pr_number}}.md` and verify it contains
    a valid review (Summary, Findings, Verdict sections). If the file is empty
    or malformed, write a brief "Review could not be completed" note to the file
    instead.

.agents/recipes/structure/recipe.md

@@ -111,7 +111,38 @@ done
 This enables modern type syntax (`list[str]` instead of `List[str]`,
 `str | None` instead of `Optional[str]`) and defers annotation evaluation.
 
-### 4. Dead exports
+### 4. Graphify structural analysis
+
+Run the graphify-based structural analysis tool. This builds a directed AST
+graph of the entire codebase (~2s) and produces god node rankings,
+cross-package edge counts, and import direction violations that grep-based
+checks miss (e.g., inferred cross-file dependencies from class-level import
+resolution).
+
+```bash
+PREV_GRAPH=""
+if [ -f "graphify-out/graph.json" ]; then
+  PREV_GRAPH="--previous-graph graphify-out/graph.json"
+fi
+/tmp/graphify-venv/bin/python .agents/tools/structural_impact.py --full $PREV_GRAPH \
+  --output /tmp/graphify-structure.md
+```
+
+Read `/tmp/graphify-structure.md` and incorporate relevant findings:
+
+- **God nodes**: track the top 10 most-connected entities. Compare against
+  the previous baseline in runner memory. A god node gaining 10+ connections
+  since last week signals growing coupling.
+- **Cross-package edge summary**: the table shows edge counts per direction.
+  All edges should flow interface -> engine -> config. Any VIOLATION row is a
+  finding for the import boundary section.
+- **Changes since last run**: new/removed entities and relationships since
+  the previous audit. Flag any unexpected removals of god nodes.
+
+After the audit, update `baselines` in runner memory with the current god
+node list and cross-package edge counts from `graphify-out/baselines.json`.
+
+### 5. Dead exports
 
 Find symbols in `__all__` that nothing outside their module references:
 
@@ -161,12 +192,18 @@ Write the report to `/tmp/audit-{{suite}}.md`:
 |---------|--------|--------|----------|
 | ... | ... | ... | No external imports found |
 
+### Structural graph analysis (graphify)
+
+(Paste the content of /tmp/graphify-structure.md here)
+
 ### Summary
 
 - N import boundary violations (M new since last run)
 - N lazy import violations (M new)
 - N files missing future annotations (M new)
 - N potentially dead exports (M new)
+- God nodes: top entity has N connections (delta: +/-M from last run)
+- Cross-package edges: N total (M violations)
 ```
 
 If no findings in any category, write `NO_FINDINGS` on the first line instead.

.agents/skills/review-code/SKILL.md

@@ -112,6 +112,33 @@ Before diving into details, build a mental model:
 5. **Note cross-cutting concerns** (e.g., a rename that touches many files vs. substantive logic changes)
 6. **Check existing feedback** (PR mode): inspect both inline comments (Step 1, item 5) and PR-level review bodies (Step 1, item 5b) so you don't duplicate feedback already given
 
+## Step 3.5: Structural Impact (if available)
+
+Check for a pre-computed structural impact analysis at
+`/tmp/structural-impact-<pr-or-branch>.md`. This file is produced by
+`graphify` AST extraction and contains:
+
+- **Risk level** (LOW/MEDIUM/HIGH) based on god nodes touched, import
+  violations, and cluster spread
+- **Core abstractions modified** - the most-connected entities in the
+  codebase (high blast radius if changed)
+- **Import direction violations** - cross-package edges that violate the
+  layering rule (interface -> engine -> config)
+- **High-connectivity changes** - entities with many dependents
+- **Cross-package dependencies** - edges crossing package boundaries
+
+If the file exists, read it and use it to calibrate your review:
+
+- **HIGH risk**: apply extra scrutiny in Pass 2 (Design & Architecture).
+  Verify backward compatibility for god nodes. Check that cross-package
+  changes don't break existing callers.
+- **Import violations**: flag them as Warnings in the review if they
+  represent real dependency direction issues (not just inferred edges).
+- **LOW risk**: the structural analysis confirms a localized change. You
+  can focus more on correctness (Pass 1) and less on architecture.
+
+If the file does not exist (e.g. local branch review), skip this step.
+
 ## Step 4: Review Each Changed File (Multi-Pass)
 
 Perform **at least 2-3 passes** over the changed files. Each pass has a different focus — this catches issues that a single read-through would miss.