From 11c6614f64ae5e77054927e24f8c3c9f92e42e27 Mon Sep 17 00:00:00 2001 From: Tony Nguyen Date: Thu, 23 Jul 2026 09:56:02 -0500 Subject: [PATCH 1/2] feat(ci): enrich architecture drift report with actionable details MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drift report posted on PRs was too sparse to be actionable: - All baseline metric columns showed 'β€”' (metrics never persisted) - Entity movements were a bare count with no details - No component breakdown, no diagram, no smell guidance Changes: 1. Baseline now persists metrics alongside architecture (backwards compatible β€” old baselines without metrics still load fine). 2. Metric table shows real Baselineβ†’Current deltas with direction indicators (🟒 improved / οΏ½οΏ½ degraded / βšͺ unchanged). 3. Entity movements list top-15 individual moves (entity β†’ component). 4. Component breakdown table (name, entity count, responsibility). 5. Mermaid component dependency diagram for visual orientation. 6. Smell entries include actionable πŸ’‘ recommendations. Breaking change: load_architecture() now returns (Architecture, dict) tuple. All callers updated. --- src/arcade_agent/ci/arch_diff.py | 190 ++++++++++++++++++++++++------ src/arcade_agent/serialization.py | 19 ++- tests/test_arch_diff.py | 6 +- tests/test_serialization.py | 17 ++- 4 files changed, 189 insertions(+), 43 deletions(-) diff --git a/src/arcade_agent/ci/arch_diff.py b/src/arcade_agent/ci/arch_diff.py index aff58de..a005980 100755 --- a/src/arcade_agent/ci/arch_diff.py +++ b/src/arcade_agent/ci/arch_diff.py @@ -21,6 +21,51 @@ from arcade_agent.tools.parse import parse from arcade_agent.tools.recover import recover +# Metrics where higher is better (for direction indicators). +_HIGHER_IS_BETTER = { + "BalancedArchitectureScore", + "PrincipleAlignmentScore", + "RCI", + "TurboMQ", + "BasicMQ", + "IntraConnectivity", + "DependencyHealth", + "ComponentBalance", + "HubBalance", + "BoundaryClarity", + "DependencyDistribution", + "SmellDiscipline", +} + +# Metrics where lower is better. +_LOWER_IS_BETTER = { + "InterConnectivity", + "TwoWayPairRatio", +} + +# Maximum entity movements to list individually. +_MAX_ENTITY_MOVEMENTS = 15 + +# Smell β†’ actionable recommendation mapping. +_SMELL_RECOMMENDATIONS = { + "concern_overload": ( + "Split the overloaded component into smaller, focused units. " + "Consider extracting sub-packages or introducing an interface layer." + ), + "hub_like_dependency": ( + "Reduce fan-in/fan-out by introducing abstractions or splitting " + "the hub into domain-specific modules." + ), + "cyclic_dependency": ( + "Break the cycle by extracting shared abstractions or inverting " + "dependencies via interfaces/events." + ), + "unstable_interface": ( + "Stabilize the interface by reducing outgoing dependencies or " + "applying the Stable Abstractions Principle." + ), +} + def build_report( current, @@ -29,6 +74,7 @@ def build_report( smells, drift=None, baseline=None, + baseline_metrics=None, ) -> str: """Build a markdown drift report. @@ -39,6 +85,7 @@ def build_report( smells: List of SmellInstance detected. drift: Optional compare() result dict (if baseline exists). baseline: Optional baseline Architecture. + baseline_metrics: Optional dict of baseline metric nameβ†’value. Returns: Markdown string. @@ -55,14 +102,13 @@ def build_report( "", ] - # Drift table (only when baseline exists) + # ── Drift table (only when baseline exists) ────────────────────────── if drift and baseline: similarity = drift["overall_similarity"] summary = drift["summary"] baseline_components = summary["arch_a_components"] - - # Build metric lookup for current and baseline delta metric_map = {m.name: m.value for m in metrics} + bl_metrics = baseline_metrics or {} lines.append("### Drift from Baseline") lines.append("") @@ -72,9 +118,7 @@ def build_report( f"| Components | {baseline_components} | {num_components} | " f"{_delta(num_components - baseline_components)} |" ) - lines.append( - f"| Similarity | β€” | {similarity:.2f} | β€” |" - ) + lines.append(f"| Similarity | β€” | {similarity:.2f} | β€” |") preferred_metrics = ( "BalancedArchitectureScore", @@ -86,23 +130,23 @@ def build_report( for name in preferred_metrics: val = metric_map.get(name) if val is not None: - lines.append(f"| {name} | β€” | {val:.2f} | β€” |") + bl_val = bl_metrics.get(name) + lines.append(_metric_row(name, bl_val, val)) displayed_metrics.add(name) for metric in metrics: if metric.name in displayed_metrics: continue - lines.append(f"| {metric.name} | β€” | {metric.value:.2f} | β€” |") + bl_val = bl_metrics.get(metric.name) + lines.append(_metric_row(metric.name, bl_val, metric.value)) lines.append("") - # Changes summary + # ── Changes summary ────────────────────────────────────────────── lines.append("### Changes") lines.append("") if summary["components_added"]: added_names = [ - m["target"] - for m in drift["matches"] - if not m["source"] + m["target"] for m in drift["matches"] if not m["source"] ] lines.append( f"- {summary['components_added']} component(s) added: " @@ -110,23 +154,34 @@ def build_report( ) if summary["components_removed"]: removed_names = [ - m["source"] - for m in drift["matches"] - if not m["target"] + m["source"] for m in drift["matches"] if not m["target"] ] lines.append( f"- {summary['components_removed']} component(s) removed: " f"`{'`, `'.join(removed_names)}`" ) - # Count entity movements - entities_moved = sum( - len(m.get("entities_added", [])) + len(m.get("entities_removed", [])) - for m in drift["matches"] - if m["source"] and m["target"] - ) - if entities_moved: - lines.append(f"- {entities_moved} entity movement(s) between components") + # Entity movements with details + movements: list[tuple[str, str, str]] = [] + for m in drift["matches"]: + if not (m["source"] and m["target"]): + continue + for ent in m.get("entities_added", []): + movements.append((ent, "β†’", m["target"])) + for ent in m.get("entities_removed", []): + movements.append((ent, "←", m["source"])) + + if movements: + lines.append( + f"- {len(movements)} entity movement(s) between components" + ) + shown = movements[:_MAX_ENTITY_MOVEMENTS] + for ent, arrow, comp in shown: + lines.append(f" - `{ent}` {arrow} **{comp}**") + if len(movements) > _MAX_ENTITY_MOVEMENTS: + lines.append( + f" - … and {len(movements) - _MAX_ENTITY_MOVEMENTS} more" + ) if summary["possible_merges"]: lines.append( @@ -140,7 +195,7 @@ def build_report( if not any([ summary["components_added"], summary["components_removed"], - entities_moved, + movements, summary["possible_merges"], summary["possible_splits"], ]): @@ -148,21 +203,65 @@ def build_report( lines.append("") - # Smells section + # ── Component breakdown ────────────────────────────────────────────── + lines.append("### Components") + lines.append("") + lines.append("| Component | Entities | Responsibility |") + lines.append("|-----------|----------|----------------|") + for comp in sorted(current.components, key=lambda c: -len(c.entities)): + resp = (comp.responsibility or "")[:60] + lines.append(f"| {comp.name} | {len(comp.entities)} | {resp} |") + lines.append("") + + # ── Mermaid dependency diagram ─────────────────────────────────────── + lines.append("### Architecture Diagram") + lines.append("") + lines.append("```mermaid") + lines.append("graph LR") + # Build inter-component edges from the dependency graph + entity_to_comp: dict[str, str] = {} + for comp in current.components: + for ent in comp.entities: + entity_to_comp[ent] = comp.name + comp_edges: set[tuple[str, str]] = set() + for edge in graph.edges: + src_comp = entity_to_comp.get(edge.source) + tgt_comp = entity_to_comp.get(edge.target) + if src_comp and tgt_comp and src_comp != tgt_comp: + comp_edges.add((src_comp, tgt_comp)) + for comp in current.components: + safe = comp.name.replace(" ", "_") + lines.append(f" {safe}[\"{comp.name}\"]") + for src, tgt in sorted(comp_edges): + safe_src = src.replace(" ", "_") + safe_tgt = tgt.replace(" ", "_") + lines.append(f" {safe_src} --> {safe_tgt}") + lines.append("```") + lines.append("") + + # ── Smells section with recommendations ────────────────────────────── if smells: lines.append(f"### Smells ({len(smells)})") lines.append("") for smell in smells: - affected = ", ".join(smell.affected_components) if smell.affected_components else "" - lines.append(f"- {_display_value(smell.smell_type)}: {affected}") + affected = ( + ", ".join(smell.affected_components) + if smell.affected_components + else "" + ) + smell_key = _display_value(smell.smell_type).lower().replace(" ", "_") + lines.append(f"- **{_display_value(smell.smell_type)}**: {affected}") + rec = _SMELL_RECOMMENDATIONS.get(smell_key) + if rec: + lines.append(f" - πŸ’‘ {rec}") lines.append("") else: lines.append("### Smells") lines.append("") - lines.append("No architectural smells detected.") + lines.append("βœ… No architectural smells detected.") lines.append("") - # Metrics table (when no baseline β€” still useful) + # ── Metrics table (when no baseline β€” still useful) ────────────────── if not drift: lines.append("### Metrics") lines.append("") @@ -182,6 +281,27 @@ def build_report( return "\n".join(lines) +def _metric_row(name: str, baseline_val: float | None, current_val: float) -> str: + """Format a single metric row with direction indicator.""" + if baseline_val is None: + return f"| {name} | β€” | {current_val:.2f} | β€” |" + delta = current_val - baseline_val + icon = _direction_icon(name, delta) + return f"| {name} | {baseline_val:.2f} | {current_val:.2f} | {icon} {_delta(delta)} |" + + +def _direction_icon(name: str, delta: float) -> str: + """Return 🟒/πŸ”΄/βšͺ based on whether the metric moved in a good direction.""" + if abs(delta) < 0.005: + return "βšͺ" + if name in _HIGHER_IS_BETTER: + return "🟒" if delta > 0 else "πŸ”΄" + if name in _LOWER_IS_BETTER: + return "🟒" if delta < 0 else "πŸ”΄" + # Unknown metric β€” neutral + return "βšͺ" + + def _delta(val: int | float) -> str: """Format a delta value with sign.""" if isinstance(val, float): @@ -232,8 +352,9 @@ def main(argv: list[str] | None = None) -> None: # 3. Load baseline and compare (if it exists) drift = None baseline = None + baseline_metrics: dict[str, float] = {} if baseline_path.exists(): - baseline = load_architecture(baseline_path) + baseline, baseline_metrics = load_architecture(baseline_path) drift = compare(baseline, current) # 4. Metrics and smells @@ -247,13 +368,16 @@ def main(argv: list[str] | None = None) -> None: ) metrics = metrics + derived_metrics - # 5. Update baseline if requested + # 5. Update baseline if requested (persist metrics for future deltas) if args.update_baseline: - save_architecture(current, baseline_path) + metric_map = {m.name: m.value for m in metrics} + save_architecture(current, baseline_path, metrics=metric_map) print(f"Baseline updated: {baseline_path}", file=sys.stderr) # 6. Print report - report = build_report(current, graph, metrics, smells, drift, baseline) + report = build_report( + current, graph, metrics, smells, drift, baseline, baseline_metrics + ) print(report) diff --git a/src/arcade_agent/serialization.py b/src/arcade_agent/serialization.py index 9e1ea49..6ed97ad 100644 --- a/src/arcade_agent/serialization.py +++ b/src/arcade_agent/serialization.py @@ -9,7 +9,9 @@ from arcade_agent.parsers.graph import DependencyGraph, Edge, Entity -def save_architecture(arch: Architecture, path: Path) -> None: +def save_architecture( + arch: Architecture, path: Path, metrics: dict[str, float] | None = None +) -> None: """Write an Architecture to a JSON file. Creates parent directories if they don't exist. @@ -17,8 +19,10 @@ def save_architecture(arch: Architecture, path: Path) -> None: Args: arch: The architecture to serialize. path: Destination file path. + metrics: Optional metric nameβ†’value map to persist alongside the + architecture (used for baseline drift deltas). """ - data = { + data: dict[str, Any] = { "algorithm": arch.algorithm, "rationale": arch.rationale, "metadata": arch.metadata, @@ -31,18 +35,21 @@ def save_architecture(arch: Architecture, path: Path) -> None: for c in arch.components ], } + if metrics: + data["metrics"] = metrics path.parent.mkdir(parents=True, exist_ok=True) path.write_text(json.dumps(data, indent=2) + "\n") -def load_architecture(path: Path) -> Architecture: +def load_architecture(path: Path) -> tuple[Architecture, dict[str, float]]: """Read an Architecture from a JSON file. Args: path: Path to the baseline JSON file. Returns: - The deserialized Architecture. + Tuple of (Architecture, baseline_metrics). The metrics dict is empty + when the baseline was created before metric persistence was added. Raises: FileNotFoundError: If the file does not exist. @@ -56,12 +63,14 @@ def load_architecture(path: Path) -> Architecture: ) for c in data.get("components", []) ] - return Architecture( + arch = Architecture( components=components, rationale=data.get("rationale", ""), algorithm=data.get("algorithm", ""), metadata=data.get("metadata", {}), ) + baseline_metrics: dict[str, float] = data.get("metrics", {}) + return arch, baseline_metrics # --------------------------------------------------------------------------- diff --git a/tests/test_arch_diff.py b/tests/test_arch_diff.py index bd9b159..1a96edc 100644 --- a/tests/test_arch_diff.py +++ b/tests/test_arch_diff.py @@ -227,7 +227,7 @@ def test_update_baseline(sample_arch, tmp_path): baseline_path = tmp_path / ".arcade" / "baseline.json" save_architecture(sample_arch, baseline_path) - loaded = load_architecture(baseline_path) + loaded, _ = load_architecture(baseline_path) assert len(loaded.components) == 2 assert loaded.algorithm == "pkg" @@ -251,6 +251,8 @@ def test_main_update_baseline(tmp_path, monkeypatch): ]) assert baseline_path.exists() - loaded = load_architecture(baseline_path) + loaded, bl_metrics = load_architecture(baseline_path) assert loaded.algorithm == "pkg" assert len(loaded.components) >= 1 + # Metrics are persisted for future drift deltas + assert "RCI" in bl_metrics diff --git a/tests/test_serialization.py b/tests/test_serialization.py index 81647cb..77b16f1 100644 --- a/tests/test_serialization.py +++ b/tests/test_serialization.py @@ -36,7 +36,7 @@ def arch(): def test_save_load_roundtrip(arch, tmp_path): path = tmp_path / "baseline.json" save_architecture(arch, path) - loaded = load_architecture(path) + loaded, bl_metrics = load_architecture(path) assert loaded.algorithm == arch.algorithm assert loaded.rationale == arch.rationale @@ -45,6 +45,7 @@ def test_save_load_roundtrip(arch, tmp_path): assert loaded_c.name == orig.name assert loaded_c.responsibility == orig.responsibility assert loaded_c.entities == orig.entities + assert bl_metrics == {} def test_load_nonexistent(tmp_path): @@ -56,20 +57,30 @@ def test_save_creates_directory(arch, tmp_path): path = tmp_path / "nested" / "deep" / "baseline.json" save_architecture(arch, path) assert path.exists() - loaded = load_architecture(path) + loaded, _ = load_architecture(path) assert len(loaded.components) == 2 def test_roundtrip_preserves_metadata(arch, tmp_path): path = tmp_path / "baseline.json" save_architecture(arch, path) - loaded = load_architecture(path) + loaded, _ = load_architecture(path) assert loaded.metadata == {"depth": 2, "version": "1.0"} assert loaded.algorithm == "pkg" assert loaded.rationale == "Package-based grouping" +def test_save_load_with_metrics(arch, tmp_path): + path = tmp_path / "baseline.json" + metrics = {"RCI": 0.95, "TurboMQ": 0.42, "ComponentBalance": 0.7} + save_architecture(arch, path, metrics=metrics) + loaded, bl_metrics = load_architecture(path) + + assert bl_metrics == metrics + assert loaded.algorithm == arch.algorithm + + # --------------------------------------------------------------------------- # DependencyGraph serialization # --------------------------------------------------------------------------- From 3ee677c11f45c179723883fd9b0591116fb708c3 Mon Sep 17 00:00:00 2001 From: Tony Nguyen Date: Thu, 23 Jul 2026 11:22:57 -0500 Subject: [PATCH 2/2] ci: run self-dogfooding analysis on all PRs, not just main-targeted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the detailed 'πŸ€– Architecture Analysis Summary' comment (metric evolution, principle signals, score drivers, Mermaid diagram, A2A comparison) only appeared on PRs targeting main because ci.yml had 'pull_request: branches: [main]'. PRs targeting feature branches (e.g. codex/rust-parser) only got the sparse arch-drift.yml comment. Remove the branch filter so every PR receives the full analysis. The baseline artifact is still sourced from the last successful main push, so comparisons remain meaningful. --- .github/workflows/ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 52c3558..042b231 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,6 @@ on: push: branches: [main] pull_request: - branches: [main] jobs: test: