diff --git a/.github/workflows/diff-guard.yml b/.github/workflows/diff-guard.yml index 92d0d3d..30f6511 100644 --- a/.github/workflows/diff-guard.yml +++ b/.github/workflows/diff-guard.yml @@ -5,8 +5,28 @@ on: types: [opened, synchronize, reopened] jobs: + run-tests: + runs-on: ubuntu-latest + steps: + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Install Dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + + - name: Run Tests + run: pytest tests/ -v + analyze-risk: runs-on: ubuntu-latest + needs: run-tests permissions: pull-requests: write contents: read @@ -20,7 +40,7 @@ jobs: - name: Set up Python uses: actions/setup-python@v5 with: - python-version: "3.10" + python-version: "3.11" - name: Install Dependencies run: | diff --git a/app.py b/app.py index ebadf65..7eee305 100644 --- a/app.py +++ b/app.py @@ -1,12 +1,16 @@ +from __future__ import annotations + import os import asyncio import difflib import concurrent.futures +from functools import lru_cache from fastapi import FastAPI, Request, BackgroundTasks, HTTPException from fastapi.responses import JSONResponse, RedirectResponse from fastapi.staticfiles import StaticFiles from pydantic import BaseModel import httpx +import networkx as nx from streaming_client import fetch_repository_archive from graph_engine import build_dependency_graph, get_impacted_files @@ -17,6 +21,39 @@ # Fetch token from environment variable GITHUB_TOKEN = os.getenv("GITHUB_TOKEN") +# --------------------------------------------------------------------------- +# Custom Exceptions +# --------------------------------------------------------------------------- + +class RepoBuildError(Exception): + """Raised when the dependency graph cannot be built from the fetched archive.""" + pass + +class RateLimitError(Exception): + """Raised when GitHub returns 403/429 indicating API rate limiting.""" + pass + +class UnsupportedRepoError(Exception): + """Raised when the repository URL is invalid or unsupported.""" + pass + +# --------------------------------------------------------------------------- +# SHA-keyed LRU Cache for Repository Fetching +# --------------------------------------------------------------------------- + +@lru_cache(maxsize=50) +def _cached_fetch(owner: str, repo: str, sha: str, token: str | None) -> dict: + """ + Thin LRU-cached wrapper around fetch_repository_archive. + Since commit SHAs are immutable, caching by (owner, repo, sha, token) + is safe and avoids redundant downloads of the same snapshot. + """ + return fetch_repository_archive(owner, repo, sha, token) + +# --------------------------------------------------------------------------- +# Utility Functions +# --------------------------------------------------------------------------- + def get_changed_line_numbers(base_code: bytes, head_code: bytes) -> tuple[set[int], set[int]]: """ Returns (base_changed_lines, head_changed_lines) as 1-indexed line numbers @@ -39,14 +76,337 @@ def get_changed_line_numbers(base_code: bytes, head_code: bytes) -> tuple[set[in return base_changed, head_changed +# --------------------------------------------------------------------------- +# Multi-Factor Risk Score +# --------------------------------------------------------------------------- + +def compute_risk_score( + graph: nx.DiGraph, + modified_files: list, + modified_functions_by_file: dict, + all_impacted_files: set, + impacted_apis: list +) -> int: + """ + Computes an architectural risk score (0-100) across four weighted factors: + 1. Betweenness centrality of modified files → 0–40 pts + 2. Blast radius breadth → 0–30 pts + 3. Public API endpoints hit → 0–20 pts + 4. Deleted functions count → 0–10 pts + + Returns a dict with 'total' and individual factor breakdowns. + """ + # 1. Betweenness centrality (how central the modified files are in the graph) + centrality = nx.betweenness_centrality(graph) if len(graph) > 0 else {} + centrality_sum = sum(centrality.get(f, 0) for f in modified_files) + centrality_pts = min(int(centrality_sum * 200), 40) + + # 2. Blast radius breadth + blast_pts = min(len(all_impacted_files) * 3, 30) + + # 3. Public API endpoints hit + api_pts = min(len(impacted_apis) * 10, 20) + + # 4. Deleted functions + deletion_count = 0 + for funcs in modified_functions_by_file.values(): + for _, change_type in funcs: + if change_type in ("deleted", "file_deleted"): + deletion_count += 1 + deletion_pts = min(deletion_count * 5, 10) + + total = min(centrality_pts + blast_pts + api_pts + deletion_pts, 100) + + return { + "total": total, + "centrality_pts": centrality_pts, + "blast_pts": blast_pts, + "api_pts": api_pts, + "deletion_pts": deletion_pts, + "deletion_count": deletion_count, + } + + +def _build_summary_text( + risk_score: int, + breakdown: dict, + modified_files: list, + modified_functions_by_file: dict, + all_impacted_files: set, + at_risk_functions: dict, + impacted_apis: list, +) -> str: + """ + Builds a detailed, human-readable summary paragraph explaining the risk + score, the contributing factors, and actionable next steps. + """ + if risk_score == 0: + return ( + "No architectural risk detected. The modified files are isolated leaf " + "nodes with zero downstream consumers in the dependency graph. No " + "functions, modules, or public API endpoints are affected by this change. " + "This diff is safe to merge with minimal review." + ) + + # Count total modified and at-risk functions + total_modified_funcs = sum(len(v) for v in modified_functions_by_file.values()) + total_at_risk_funcs = sum(len(v) for v in at_risk_functions.values()) + + # Opening sentence based on severity tier + if risk_score >= 80: + opening = ( + f"Critical architectural risk detected (score: {risk_score}/100). " + f"This change touches core infrastructure that is deeply interconnected " + f"within the codebase." + ) + elif risk_score >= 50: + opening = ( + f"High architectural risk detected (score: {risk_score}/100). " + f"The modified code is centrally positioned in the dependency graph " + f"and has a wide downstream impact." + ) + elif risk_score >= 20: + opening = ( + f"Moderate architectural risk detected (score: {risk_score}/100). " + f"While the change is not to core infrastructure, it ripples into " + f"several downstream consumers that should be verified." + ) + else: + opening = ( + f"Low architectural risk detected (score: {risk_score}/100). " + f"The modified files have limited connectivity in the dependency graph, " + f"but a small number of downstream modules are still affected." + ) + + # Impact details + impact_detail = ( + f" {len(modified_files)} file(s) containing {total_modified_funcs} " + f"semantic entities (functions/methods) were modified, impacting " + f"{len(all_impacted_files)} downstream module(s) through transitive " + f"dependency chains." + ) + + # At-risk functions detail + risk_detail = "" + if total_at_risk_funcs > 0: + risk_detail = ( + f" {total_at_risk_funcs} function(s) in direct consumer files " + f"actively reference the modified symbols and are at risk of " + f"behavioral regression." + ) + + # API impact detail + api_detail = "" + if impacted_apis: + api_detail = ( + f" {len(impacted_apis)} public API endpoint(s) are in the blast " + f"radius — these are user-facing routes that may exhibit changed " + f"behavior or break client contracts." + ) + + # Score breakdown sentence + factor_parts = [] + if breakdown["centrality_pts"] > 0: + factor_parts.append(f"graph centrality (+{breakdown['centrality_pts']})") + if breakdown["blast_pts"] > 0: + factor_parts.append(f"blast radius breadth (+{breakdown['blast_pts']})") + if breakdown["api_pts"] > 0: + factor_parts.append(f"API surface exposure (+{breakdown['api_pts']})") + if breakdown["deletion_pts"] > 0: + factor_parts.append( + f"deleted functions ({breakdown['deletion_count']} removed, " + f"+{breakdown['deletion_pts']})" + ) + breakdown_sentence = "" + if factor_parts: + breakdown_sentence = ( + f" Score contributors: {', '.join(factor_parts)}." + ) + + # Recommendation + if risk_score >= 50: + recommendation = ( + " A senior engineer review is strongly recommended. Run the full " + "regression test suite and validate all impacted API contracts " + "before merging." + ) + elif risk_score >= 20: + recommendation = ( + " Ensure integration tests cover the affected downstream modules. " + "Review the dependency paths to confirm no unintended side effects." + ) + else: + recommendation = ( + " Standard code review is sufficient. Spot-check the listed " + "downstream consumers for any unexpected behavioral changes." + ) + + return ( + opening + impact_detail + risk_detail + api_detail + + breakdown_sentence + recommendation + ) + +# --------------------------------------------------------------------------- +# Unified Analysis Pipeline +# --------------------------------------------------------------------------- + +def run_analysis_pipeline(base_files: dict, head_files: dict) -> dict: + """ + Core analysis pipeline shared by both the webhook handler and the API endpoint. + + Takes two file snapshots (base and head) and returns a dict containing: + graph, modified_files, modified_functions_by_file, all_impacted_files, + impact_paths, at_risk_functions, impacted_apis, risk_score, status, summary_text + """ + # Build dependency graph from the base state + graph = build_dependency_graph(base_files) + + # Compare base and head files to locate structural changes + modified_files = [] + modified_functions_by_file = {} + + for file_path, base_code in base_files.items(): + if file_path not in head_files: + # File deleted completely + modified_files.append(file_path) + modified_functions_by_file[file_path] = [("All", "file_deleted")] + else: + head_code = head_files[file_path] + if base_code != head_code: + # Ingest code and extract function boundary definitions + base_funcs = extract_functions(base_code, file_path=file_path) + head_funcs = extract_functions(head_code, file_path=file_path) + + # Calculate exact line changes between base and head + base_changed, head_changed = get_changed_line_numbers(base_code, head_code) + + changed_funcs = [] + + # Check for modified or deleted functions based on coordinate intersections + for f_name, (start_b, end_b) in base_funcs.items(): + if f_name not in head_funcs: + changed_funcs.append((f_name, "deleted")) + else: + # Intersection: Did any of the changed base lines fall within this function? + # We check both base and head coordinates. + start_h, end_h = head_funcs[f_name] + + b_intersect = any(line in base_changed for line in range(start_b, end_b + 1)) + h_intersect = any(line in head_changed for line in range(start_h, end_h + 1)) + + if b_intersect or h_intersect: + changed_funcs.append((f_name, "modified")) + + # Check for newly added functions + for f_name, (start_h, end_h) in head_funcs.items(): + if f_name not in base_funcs: + changed_funcs.append((f_name, "added")) + + if changed_funcs: + modified_files.append(file_path) + modified_functions_by_file[file_path] = changed_funcs + + # Trace impacted modules upward via DiGraph BFS + all_impacted_files = set() + impact_paths = {} + at_risk_functions = {} + + for m_file in modified_files: + impacted = get_impacted_files(graph, m_file) + all_impacted_files.update(impacted) + + paths_dict = {} + for imp_file in impacted: + try: + path = nx.shortest_path(graph, source=m_file, target=imp_file) + paths_dict[imp_file] = path + except nx.NetworkXNoPath: + paths_dict[imp_file] = [m_file, imp_file] + impact_paths[m_file] = paths_dict + + # Identify AT RISK functions in direct consumers + for m_file in modified_files: + changed_entities = modified_functions_by_file.get(m_file, []) + for imp_file, path in impact_paths.get(m_file, {}).items(): + if len(path) == 2: # Direct consumer + # Scan the consumer for usage of the modified symbols + code_bytes = head_files.get(imp_file) + if code_bytes: + for c_func_name, c_type in changed_entities: + if c_type in ("modified", "deleted"): + found_funcs = find_functions_using_symbol(code_bytes, c_func_name, file_path=imp_file) + if found_funcs: + if imp_file not in at_risk_functions: + at_risk_functions[imp_file] = [] + at_risk_functions[imp_file].extend(found_funcs) + + # Dedup at_risk_functions + for k in at_risk_functions: + at_risk_functions[k] = list(dict.fromkeys(at_risk_functions[k])) + + # Scan for impacted API routes + all_api_routes = {} + for f_path, h_code in head_files.items(): + routes = extract_api_routes(h_code, file_path=f_path) + for f_name, r_info in routes.items(): + all_api_routes[(f_path, f_name)] = r_info + + impacted_apis = [] + for imp_file, funcs in at_risk_functions.items(): + for f in funcs: + if (imp_file, f) in all_api_routes: + route_info = all_api_routes[(imp_file, f)] + impacted_apis.append({ + "file": imp_file, + "function": f, + "method": route_info["method"], + "path": route_info["path"] + }) + + # Compute multi-factor risk score + score_breakdown = compute_risk_score( + graph, modified_files, modified_functions_by_file, + all_impacted_files, impacted_apis + ) + risk_score = score_breakdown["total"] + + if risk_score >= 50: + status = "HIGH RISK" + elif risk_score > 0: + status = "MEDIUM RISK" + else: + status = "LOW RISK" + + # Generate detailed human-readable summary + summary_text = _build_summary_text( + risk_score, score_breakdown, modified_files, + modified_functions_by_file, all_impacted_files, + at_risk_functions, impacted_apis + ) + + return { + "graph": graph, + "modified_files": modified_files, + "modified_functions_by_file": modified_functions_by_file, + "all_impacted_files": all_impacted_files, + "impact_paths": impact_paths, + "at_risk_functions": at_risk_functions, + "impacted_apis": impacted_apis, + "risk_score": risk_score, + "status": status, + "summary_text": summary_text, + } + +# --------------------------------------------------------------------------- +# Webhook Background Worker +# --------------------------------------------------------------------------- + def analyze_diff_and_report(payload: dict): """ Background worker that runs the full Diff-Guard pipeline: 1. Downloads base & head commit snapshots. - 2. Builds a codebase dependency graph. - 3. Resolves modified functions/entities. - 4. Computes upstream blast radius. - 5. Formats and dispatches a markdown report to the PR thread. + 2. Runs the unified analysis pipeline. + 3. Formats and dispatches a markdown report to the PR thread. """ try: pull_number = payload["pull_request"]["number"] @@ -59,125 +419,27 @@ def analyze_diff_and_report(payload: dict): print(f"\n[Diff-Guard] Analyzing PR #{pull_number} ({base_sha} -> {head_sha})") - # Step 4 logic: Fetch archive states concurrently using ThreadPoolExecutor + # Fetch archive states concurrently using ThreadPoolExecutor with concurrent.futures.ThreadPoolExecutor() as executor: - future_base = executor.submit(fetch_repository_archive, owner, repo_name, base_sha, GITHUB_TOKEN) - future_head = executor.submit(fetch_repository_archive, owner, repo_name, head_sha, GITHUB_TOKEN) + future_base = executor.submit(_cached_fetch, owner, repo_name, base_sha, GITHUB_TOKEN) + future_head = executor.submit(_cached_fetch, owner, repo_name, head_sha, GITHUB_TOKEN) base_files = future_base.result() head_files = future_head.result() - # Step 3 logic: Build dependency graph from base state - graph = build_dependency_graph(base_files) - - # Step 2 logic: Compare base and head files to locate structural changes - modified_files = [] - modified_functions_by_file = {} + # Run unified analysis pipeline + result = run_analysis_pipeline(base_files, head_files) - for file_path, base_code in base_files.items(): - if file_path not in head_files: - # File deleted completely - modified_files.append(file_path) - modified_functions_by_file[file_path] = [("All", "file_deleted")] - else: - head_code = head_files[file_path] - if base_code != head_code: - # Ingest code and extract function boundary definitions - base_funcs = extract_functions(base_code, file_path=file_path) - head_funcs = extract_functions(head_code, file_path=file_path) - - # Calculate exact line changes between base and head - base_changed, head_changed = get_changed_line_numbers(base_code, head_code) - - changed_funcs = [] - - # Check for modified or deleted functions based on coordinate intersections - for f_name, (start_b, end_b) in base_funcs.items(): - if f_name not in head_funcs: - changed_funcs.append((f_name, "deleted")) - else: - # Intersection: Did any of the changed base lines fall within this function? - # We check both base and head coordinates. - start_h, end_h = head_funcs[f_name] - - b_intersect = any(line in base_changed for line in range(start_b, end_b + 1)) - h_intersect = any(line in head_changed for line in range(start_h, end_h + 1)) - - if b_intersect or h_intersect: - changed_funcs.append((f_name, "modified")) - - # Check for newly added functions - for f_name, (start_h, end_h) in head_funcs.items(): - if f_name not in base_funcs: - changed_funcs.append((f_name, "added")) - - if changed_funcs: - modified_files.append(file_path) - modified_functions_by_file[file_path] = changed_funcs - - # Step 3/7 logic: Trace impacted modules upward via DiGraph BFS - import networkx as nx - from parser import find_functions_using_symbol - - all_impacted_files = set() - impact_paths = {} - at_risk_functions = {} - - for m_file in modified_files: - impacted = get_impacted_files(graph, m_file) - all_impacted_files.update(impacted) - - paths_dict = {} - for imp_file in impacted: - try: - path = nx.shortest_path(graph, source=m_file, target=imp_file) - paths_dict[imp_file] = path - except nx.NetworkXNoPath: - paths_dict[imp_file] = [m_file, imp_file] - impact_paths[m_file] = paths_dict - - # Identify AT RISK functions in direct consumers - for m_file in modified_files: - changed_entities = modified_functions_by_file.get(m_file, []) - for imp_file, path in impact_paths.get(m_file, {}).items(): - if len(path) == 2: # Direct consumer - # Scan the consumer for usage of the modified symbols - code_bytes = head_files.get(imp_file) - if code_bytes: - for c_func_name, c_type in changed_entities: - if c_type in ("modified", "deleted"): - found_funcs = find_functions_using_symbol(code_bytes, c_func_name, file_path=imp_file) - if found_funcs: - if imp_file not in at_risk_functions: - at_risk_functions[imp_file] = [] - at_risk_functions[imp_file].extend(found_funcs) - - # Dedup at_risk_functions - for k in at_risk_functions: - at_risk_functions[k] = list(dict.fromkeys(at_risk_functions[k])) - - all_api_routes = {} - for f_path, h_code in head_files.items(): - routes = extract_api_routes(h_code, file_path=f_path) - for f_name, r_info in routes.items(): - all_api_routes[(f_path, f_name)] = r_info - - impacted_apis = [] - for imp_file, funcs in at_risk_functions.items(): - for f in funcs: - if (imp_file, f) in all_api_routes: - route_info = all_api_routes[(imp_file, f)] - impacted_apis.append({ - "file": imp_file, - "function": f, - "method": route_info["method"], - "path": route_info["path"] - }) - - # Step 8 logic: Format and Dispatch feedback + # Format and Dispatch feedback report_md = format_markdown_report( - pull_number, modified_files, modified_functions_by_file, - all_impacted_files, impact_paths, at_risk_functions, impacted_apis + pull_number, + result["modified_files"], + result["modified_functions_by_file"], + result["all_impacted_files"], + result["impact_paths"], + result["at_risk_functions"], + result["impacted_apis"], + result["risk_score"], ) print("\n--- Generated Architectural Risk Report ---") @@ -192,6 +454,10 @@ def analyze_diff_and_report(payload: dict): except Exception as e: print(f"[Diff-Guard] Error during PR analysis: {e}") +# --------------------------------------------------------------------------- +# Markdown Report Formatter +# --------------------------------------------------------------------------- + def format_markdown_report( pull_number: int, modified_files: list, @@ -199,13 +465,18 @@ def format_markdown_report( all_impacted_files: set, impact_paths: dict, at_risk_functions: dict, - impacted_apis: list + impacted_apis: list, + risk_score: int = None, + graph: nx.DiGraph = None, ) -> str: """ Constructs a structured GitHub Markdown report with a Risk Score. + Accepts a pre-computed risk_score. If not provided, falls back to the + legacy formula for backward compatibility with cli.py. """ - # 10 points of risk per impacted downstream file, capped at 100 - risk_score = min(len(all_impacted_files) * 10, 100) + if risk_score is None: + # Legacy fallback for callers that don't pass risk_score + risk_score = min(len(all_impacted_files) * 10, 100) if risk_score >= 50: status = "🔴 HIGH RISK" @@ -262,6 +533,10 @@ def format_markdown_report( return md +# --------------------------------------------------------------------------- +# GitHub Comment Dispatcher +# --------------------------------------------------------------------------- + def post_comment_to_github(owner: str, repo_name: str, pull_number: int, body: str): """ Dispatches the generated markdown comment to the GitHub PR thread. @@ -278,6 +553,15 @@ def post_comment_to_github(owner: str, repo_name: str, pull_number: int, body: s else: print(f"[Diff-Guard] Failed to post comment: {response.status_code} - {response.text}") +# --------------------------------------------------------------------------- +# API Routes +# --------------------------------------------------------------------------- + +@app.get("/api/health") +async def api_health(): + """Health check endpoint.""" + return {"status": "ok", "service": "diff-guard"} + @app.post("/webhook") async def github_webhook(request: Request, background_tasks: BackgroundTasks): event_type = request.headers.get("X-GitHub-Event") @@ -308,7 +592,7 @@ def parse_github_url(url: str) -> tuple[str, str]: parts = url.split("/") if len(parts) == 2: return parts[0], parts[1] - raise ValueError("Invalid GitHub URL. Must be like 'https://github.com/owner/repo' or 'owner/repo'") + raise UnsupportedRepoError("Invalid GitHub URL. Must be like 'https://github.com/owner/repo' or 'owner/repo'") class AnalyzeRequest(BaseModel): repo_url: str @@ -329,12 +613,18 @@ async def api_get_branches(repo_url: str): async with httpx.AsyncClient() as client: resp = await client.get(url, headers=headers, params={"per_page": 100}) + if resp.status_code in (403, 429): + raise RateLimitError(f"GitHub API rate limit exceeded: {resp.text}") if resp.status_code != 200: raise HTTPException(status_code=resp.status_code, detail=f"Failed to fetch branches: {resp.text}") branches_data = resp.json() formatted_branches = [b["name"] for b in branches_data] return {"branches": formatted_branches} + except (RateLimitError, UnsupportedRepoError, RepoBuildError): + raise + except HTTPException: + raise except Exception as e: print(f"[Diff-Guard API Error] {e}") raise HTTPException(status_code=400, detail=str(e)) @@ -357,6 +647,8 @@ async def api_get_commits(repo_url: str, branch: str = None): async with httpx.AsyncClient() as client: resp = await client.get(url, headers=headers, params=params) + if resp.status_code in (403, 429): + raise RateLimitError(f"GitHub API rate limit exceeded: {resp.text}") if resp.status_code != 200: raise HTTPException(status_code=resp.status_code, detail=f"Failed to fetch commits: {resp.text}") @@ -370,6 +662,10 @@ async def api_get_commits(repo_url: str, branch: str = None): "date": c["commit"]["author"]["date"] }) return {"commits": formatted_commits} + except (RateLimitError, UnsupportedRepoError, RepoBuildError): + raise + except HTTPException: + raise except Exception as e: print(f"[Diff-Guard API Error] {e}") raise HTTPException(status_code=400, detail=str(e)) @@ -380,104 +676,23 @@ async def api_analyze(req: AnalyzeRequest): owner, repo_name = parse_github_url(req.repo_url) loop = asyncio.get_event_loop() with concurrent.futures.ThreadPoolExecutor() as executor: - future_base = loop.run_in_executor(executor, fetch_repository_archive, owner, repo_name, req.base, GITHUB_TOKEN) - future_head = loop.run_in_executor(executor, fetch_repository_archive, owner, repo_name, req.head, GITHUB_TOKEN) + future_base = loop.run_in_executor(executor, _cached_fetch, owner, repo_name, req.base, GITHUB_TOKEN) + future_head = loop.run_in_executor(executor, _cached_fetch, owner, repo_name, req.head, GITHUB_TOKEN) base_files = await future_base head_files = await future_head - graph = build_dependency_graph(base_files) - modified_files = [] - modified_functions_by_file = {} + # Run unified analysis pipeline + result = run_analysis_pipeline(base_files, head_files) - for file_path, base_code in base_files.items(): - if file_path not in head_files: - modified_files.append(file_path) - modified_functions_by_file[file_path] = [("All", "file_deleted")] - else: - head_code = head_files[file_path] - if base_code != head_code: - base_funcs = extract_functions(base_code, file_path=file_path) - head_funcs = extract_functions(head_code, file_path=file_path) - base_changed, head_changed = get_changed_line_numbers(base_code, head_code) - changed_funcs = [] - - for f_name, (start_b, end_b) in base_funcs.items(): - if f_name not in head_funcs: - changed_funcs.append((f_name, "deleted")) - else: - start_h, end_h = head_funcs[f_name] - b_intersect = any(line in base_changed for line in range(start_b, end_b + 1)) - h_intersect = any(line in head_changed for line in range(start_h, end_h + 1)) - if b_intersect or h_intersect: - changed_funcs.append((f_name, "modified")) - - for f_name, (start_h, end_h) in head_funcs.items(): - if f_name not in base_funcs: - changed_funcs.append((f_name, "added")) - - if changed_funcs: - modified_files.append(file_path) - modified_functions_by_file[file_path] = changed_funcs - - import networkx as nx - from parser import find_functions_using_symbol - all_impacted_files = set() - impact_paths = {} - at_risk_functions = {} - - for m_file in modified_files: - impacted = get_impacted_files(graph, m_file) - all_impacted_files.update(impacted) - paths_dict = {} - for imp_file in impacted: - try: - path = nx.shortest_path(graph, source=m_file, target=imp_file) - paths_dict[imp_file] = path - except nx.NetworkXNoPath: - paths_dict[imp_file] = [m_file, imp_file] - impact_paths[m_file] = paths_dict - - for m_file in modified_files: - changed_entities = modified_functions_by_file.get(m_file, []) - for imp_file, path in impact_paths.get(m_file, {}).items(): - if len(path) == 2: - code_bytes = head_files.get(imp_file) - if code_bytes: - for c_func_name, c_type in changed_entities: - if c_type in ("modified", "deleted"): - found_funcs = find_functions_using_symbol(code_bytes, c_func_name, file_path=imp_file) - if found_funcs: - if imp_file not in at_risk_functions: - at_risk_functions[imp_file] = [] - at_risk_functions[imp_file].extend(found_funcs) - - for k in at_risk_functions: - at_risk_functions[k] = list(dict.fromkeys(at_risk_functions[k])) - - all_api_routes = {} - for f_path, h_code in head_files.items(): - routes = extract_api_routes(h_code, file_path=f_path) - for f_name, r_info in routes.items(): - all_api_routes[(f_path, f_name)] = r_info - - impacted_apis = [] - for imp_file, funcs in at_risk_functions.items(): - for f in funcs: - if (imp_file, f) in all_api_routes: - route_info = all_api_routes[(imp_file, f)] - impacted_apis.append({ - "file": imp_file, - "function": f, - "method": route_info["method"], - "path": route_info["path"] - }) - - risk_score = min(len(all_impacted_files) * 10, 100) - status = "LOW RISK" - if risk_score >= 50: - status = "HIGH RISK" - elif risk_score > 0: - status = "MEDIUM RISK" + graph = result["graph"] + modified_files = result["modified_files"] + modified_functions_by_file = result["modified_functions_by_file"] + all_impacted_files = result["all_impacted_files"] + at_risk_functions = result["at_risk_functions"] + impacted_apis = result["impacted_apis"] + risk_score = result["risk_score"] + status = result["status"] + summary_text = result["summary_text"] nodes = [] for n in graph.nodes(): @@ -508,16 +723,6 @@ async def api_analyze(req: AnalyzeRequest): "changed_entities": changed_entities }) - summary_text = "" - if risk_score == 0: - summary_text = "No downstream dependencies are impacted by the modified files. The blast radius is contained and there is no significant architectural risk." - else: - api_impact_text = f" This includes {len(impacted_apis)} public API entrypoint(s) that may experience side effects." if impacted_apis else "" - if risk_score >= 50: - summary_text = f"Major architectural risk detected. {len(modified_files)} modified file(s) are impacting {len(all_impacted_files)} downstream modules.{api_impact_text} Extensive regression testing is highly recommended." - else: - summary_text = f"Moderate risk detected. {len(modified_files)} modified file(s) are impacting {len(all_impacted_files)} downstream modules.{api_impact_text} Proceed with caution and ensure affected modules are tested." - return { "status": status, "risk_score": risk_score, @@ -531,9 +736,24 @@ async def api_analyze(req: AnalyzeRequest): "edges": edges } } - except Exception as e: + except UnsupportedRepoError as e: print(f"[Diff-Guard API Error] {e}") raise HTTPException(status_code=400, detail=str(e)) + except RateLimitError as e: + print(f"[Diff-Guard API Error] {e}") + raise HTTPException(status_code=422, detail=str(e)) + except RepoBuildError as e: + print(f"[Diff-Guard API Error] {e}") + raise HTTPException(status_code=503, detail=str(e)) + except HTTPException: + raise + except Exception as e: + print(f"[Diff-Guard API Error] {e}") + raise HTTPException(status_code=500, detail=str(e)) + +# --------------------------------------------------------------------------- +# Static Files & Root Redirect +# --------------------------------------------------------------------------- @app.get("/") async def root_redirect(): diff --git a/requirements.txt b/requirements.txt index 306314c..763caeb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ tree-sitter==0.21.3 -tree-sitter-languages>=1.10.0 -networkx>=3.0 -fastapi>=0.110.0 -uvicorn>=0.28.0 -httpx>=0.27.0 -pytest>=8.0.0 +tree-sitter-languages==1.10.2 +networkx==3.2.1 +fastapi==0.128.8 +uvicorn==0.39.0 +httpx==0.28.1 +pytest==8.4.2 # dev diff --git a/tests/test_api.py b/tests/test_api.py index b88fe76..1949565 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -4,6 +4,13 @@ client = TestClient(app) +def test_health_endpoint(): + response = client.get("/api/health") + assert response.status_code == 200 + data = response.json() + assert data["status"] == "ok" + assert data["service"] == "diff-guard" + def test_webhook_missing_header(): response = client.post("/webhook", json={}) assert response.status_code == 400 diff --git a/tests/test_graph_engine.py b/tests/test_graph_engine.py index 7c943ba..cc3700f 100644 --- a/tests/test_graph_engine.py +++ b/tests/test_graph_engine.py @@ -1,6 +1,11 @@ import pytest import networkx as nx from graph_engine import resolve_relative_import, build_dependency_graph, get_impacted_files +from parser import get_ast_parser, parse_code, extract_functions, find_functions_using_symbol + +# --------------------------------------------------------------------------- +# Existing unit tests +# --------------------------------------------------------------------------- def test_resolve_relative_import(): assert resolve_relative_import("src/flask/app.py", ".testing") == "src.flask.testing" @@ -35,3 +40,23 @@ def test_get_impacted_files(): impacted_a = get_impacted_files(graph, "file_a") assert "file_b" not in impacted_a assert "file_c" in impacted_a + +# --------------------------------------------------------------------------- +# Tests migrated from scratch_test.py +# --------------------------------------------------------------------------- + +def test_shortest_path_finding(): + """Verifies nx.shortest_path works for our graph structure.""" + graph = nx.DiGraph() + graph.add_edge("file_b.py", "file_a.py") + + path = nx.shortest_path(graph, "file_b.py", "file_a.py") + assert path == ["file_b.py", "file_a.py"] + + +def test_find_functions_using_symbol_via_graph(): + """Verifies that functions using a specific symbol in consumer files are detected.""" + consumer_code = b"from file_b import compute_value\n\ndef run_calculation():\n return compute_value(10)\n" + + at_risk = find_functions_using_symbol(consumer_code, "compute_value") + assert "run_calculation" in at_risk diff --git a/tests/test_parser.py b/tests/test_parser.py index 129c97d..c931076 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -1,5 +1,11 @@ import pytest +import difflib from parser import extract_functions, find_functions_using_symbol +from graph_engine import build_dependency_graph, get_impacted_files + +# --------------------------------------------------------------------------- +# Existing unit tests +# --------------------------------------------------------------------------- def test_extract_functions(): code = b""" @@ -34,3 +40,156 @@ def safe_func(): at_risk = find_functions_using_symbol(code, "modified_target", "python") assert "outer" in at_risk assert "safe_func" not in at_risk + +# --------------------------------------------------------------------------- +# Integration tests (migrated from sandbox_test.py) +# --------------------------------------------------------------------------- + +# Mock base and head repository snapshots +_base_repo = { + "file_b.py": b""" +def compute_value(x): + return x * 2 +""", + "file_a.py": b""" +from file_b import compute_value + +def run_calculation(): + return compute_value(10) +""" +} + +_head_repo = { + "file_b.py": b""" +def compute_value(x): + # Modified comment and behavior inside function + print("Computing value...") + return x * 3 +""", + "file_a.py": b""" +from file_b import compute_value + +def run_calculation(): + return compute_value(10) +""" +} + + +def _get_changed_line_numbers(base_code: bytes, head_code: bytes) -> tuple[set[int], set[int]]: + """Local helper replicating the diff utility for tests.""" + base_lines = base_code.decode("utf-8", errors="replace").splitlines() + head_lines = head_code.decode("utf-8", errors="replace").splitlines() + + sm = difflib.SequenceMatcher(None, base_lines, head_lines) + base_changed = set() + head_changed = set() + + for tag, i1, i2, j1, j2 in sm.get_opcodes(): + if tag in ("replace", "delete"): + for idx in range(i1, i2): + base_changed.add(idx + 1) + if tag in ("replace", "insert"): + for idx in range(j1, j2): + head_changed.add(idx + 1) + + return base_changed, head_changed + + +def _run_sandbox_analysis(): + """Helper that runs the sandbox pipeline and returns all computed data.""" + import networkx as nx + + graph = build_dependency_graph(_base_repo) + + modified_files = [] + modified_functions = {} + + for file_path, base_code in _base_repo.items(): + if file_path not in _head_repo: + modified_files.append(file_path) + modified_functions[file_path] = [("All", "deleted")] + continue + + head_code = _head_repo[file_path] + if base_code != head_code: + base_funcs = extract_functions(base_code, "python") + head_funcs = extract_functions(head_code, "python") + base_changed, head_changed = _get_changed_line_numbers(base_code, head_code) + + changed_funcs = [] + for f_name, (start_b, end_b) in base_funcs.items(): + if f_name not in head_funcs: + changed_funcs.append((f_name, "deleted")) + else: + start_h, end_h = head_funcs[f_name] + b_intersect = any(line in base_changed for line in range(start_b, end_b + 1)) + h_intersect = any(line in head_changed for line in range(start_h, end_h + 1)) + if b_intersect or h_intersect: + changed_funcs.append((f_name, "modified")) + + for f_name, (start_h, end_h) in head_funcs.items(): + if f_name not in base_funcs: + changed_funcs.append((f_name, "added")) + + if changed_funcs: + modified_files.append(file_path) + modified_functions[file_path] = changed_funcs + + all_impacted_files = set() + impact_paths = {} + at_risk_functions = {} + + for m_file in modified_files: + impacted = get_impacted_files(graph, m_file) + all_impacted_files.update(impacted) + paths_dict = {} + for imp_file in impacted: + paths_dict[imp_file] = nx.shortest_path(graph, m_file, imp_file) + impact_paths[m_file] = paths_dict + + for m_file in modified_files: + changed_entities = modified_functions[m_file] + for imp_file, path in impact_paths.get(m_file, {}).items(): + if len(path) == 2: + code_bytes = _head_repo.get(imp_file) + if code_bytes: + for c_func_name, c_type in changed_entities: + if c_type in ("modified", "deleted"): + found_funcs = find_functions_using_symbol(code_bytes, c_func_name) + if found_funcs: + if imp_file not in at_risk_functions: + at_risk_functions[imp_file] = [] + at_risk_functions[imp_file].extend(found_funcs) + + return modified_files, modified_functions, all_impacted_files, impact_paths, at_risk_functions + + +def test_sandbox_modified_file_detection(): + """file_b.py should be detected as modified.""" + modified_files, _, _, _, _ = _run_sandbox_analysis() + assert "file_b.py" in modified_files + + +def test_sandbox_compute_value_modified(): + """compute_value should be detected as a modified function in file_b.py.""" + _, modified_functions, _, _, _ = _run_sandbox_analysis() + file_b_changes = [f[0] for f in modified_functions["file_b.py"]] + assert "compute_value" in file_b_changes + + +def test_sandbox_blast_radius(): + """file_a.py should be in the blast radius of file_b.py.""" + _, _, all_impacted_files, _, _ = _run_sandbox_analysis() + assert "file_a.py" in all_impacted_files + + +def test_sandbox_impact_path(): + """The correct dependency path should be detected.""" + _, _, _, impact_paths, _ = _run_sandbox_analysis() + assert impact_paths["file_b.py"]["file_a.py"] == ["file_b.py", "file_a.py"] + + +def test_sandbox_at_risk_function(): + """run_calculation should be marked at risk inside file_a.py.""" + _, _, _, _, at_risk_functions = _run_sandbox_analysis() + assert "run_calculation" in at_risk_functions["file_a.py"]