From 746052ad8b25cbfbbba60f6fec8732f5e4eb2a99 Mon Sep 17 00:00:00 2001 From: Avinash Date: Fri, 22 Aug 2025 15:51:18 +0100 Subject: [PATCH 1/7] Final changes for explainer agent --- AntiPattern_Remediator/main.py | 31 +++++-- .../src/core/agents/__init__.py | 4 +- .../src/core/agents/explainer.py | 86 +++++++++++++++++++ .../src/core/graph/create_graph.py | 13 ++- .../src/core/prompt/prompt_manager.py | 29 ++++--- AntiPattern_Remediator/src/core/state.py | 2 + .../src/core/utils/__init__.py | 3 + .../src/core/utils/json_utils.py | 27 ++++++ .../static/prompt/explainer.yaml | 42 +++++++++ AntiPattern_Remediator/static/tinydb.json | 2 +- 10 files changed, 213 insertions(+), 26 deletions(-) create mode 100644 AntiPattern_Remediator/src/core/agents/explainer.py create mode 100644 AntiPattern_Remediator/src/core/utils/__init__.py create mode 100644 AntiPattern_Remediator/src/core/utils/json_utils.py create mode 100644 AntiPattern_Remediator/static/prompt/explainer.yaml diff --git a/AntiPattern_Remediator/main.py b/AntiPattern_Remediator/main.py index fc05fdd..cbd3b45 100644 --- a/AntiPattern_Remediator/main.py +++ b/AntiPattern_Remediator/main.py @@ -1,13 +1,12 @@ - """ Main entry point - Legacy Code Migration Tool """ from config.settings import initialize_settings -# from scripts import seed_database from dotenv import load_dotenv load_dotenv() from colorama import Fore, Style + def main(): """Main function: Run antipattern analysis""" @@ -18,9 +17,9 @@ def main(): provider_map = {"1": "ollama", "2": "ibm", "3": "vllm"} provider = provider_map.get(choice, "ollama") # default to ollama - #Let us choose which DB to interact with + # Let us choose which DB to interact with print("Choose your trove: 1) ChromaDB (VectorDB) 2) TinyDB (DocumentDB)") - db_choice = input("Choose 1 or 2: ").strip() + db_choice = input("Choose 1 or 2: ").strip() # Initialize global settings with selected provider settings = initialize_settings(provider) @@ -74,6 +73,8 @@ def main(): } } """ + + # Initial workflow state initial_state = { "code": legacy_code, "context": None, @@ -84,24 +85,29 @@ def main(): "code_review_results": None, "code_review_times": 0, "msgs": [], - "answer": None + "answer": None, + + # ExplainerAgent fields + "explanation_response_raw": None, + "explanation_json": None, } - #Setup Database + # Setup Database if db_choice == "2": print("Seeding TinyDB with AntiPattern Dataset") seed_database.main() db_manager = TinyDBManager() - print("Using TinyDB for knowledge retreival") + print("Using TinyDB for knowledge retrieval") else: vector_db = VectorDBManager() db_manager = vector_db.get_db() - print("Using ChromaDB for knowledge retreival") + print("Using ChromaDB for knowledge retrieval") retriever = db_manager.as_retriever() langgraph = CreateGraph(db_manager, prompt_manager, retriever=retriever).workflow final_state = langgraph.invoke(initial_state) + # Final results summary print(Fore.GREEN + f"\nAnalysis Complete!" + Style.RESET_ALL) print(f"Final state keys: {list(final_state.keys())}") print(f"Context retrieved: {'Yes' if final_state.get('context') else 'No'}") @@ -109,5 +115,14 @@ def main(): print(f"Refactored code: {'Yes' if final_state.get('refactored_code') else 'No'}") print(f"Code review results: {final_state.get('code_review_times')}") + # Show explanation from ExplainerAgent + if final_state.get("explanation_json"): + import json + print(Fore.CYAN + "\n=== Explanation (JSON) ===" + Style.RESET_ALL) + print(json.dumps(final_state["explanation_json"], indent=2, ensure_ascii=False)) + else: + print(Fore.RED + "\nNo explanation was generated." + Style.RESET_ALL) + + if __name__ == "__main__": main() diff --git a/AntiPattern_Remediator/src/core/agents/__init__.py b/AntiPattern_Remediator/src/core/agents/__init__.py index 79c1adc..d5eceaa 100644 --- a/AntiPattern_Remediator/src/core/agents/__init__.py +++ b/AntiPattern_Remediator/src/core/agents/__init__.py @@ -7,10 +7,12 @@ from .refactor_strategist import RefactorStrategist from .code_transformer import CodeTransformer from .code_reviewer import CodeReviewerAgent +from .explainer import ExplainerAgent __all__ = [ "AntipatternScanner", "RefactorStrategist", "CodeTransformer", - "CodeReviewerAgent" + "CodeReviewerAgent", + "ExplainerAgent" ] diff --git a/AntiPattern_Remediator/src/core/agents/explainer.py b/AntiPattern_Remediator/src/core/agents/explainer.py new file mode 100644 index 0000000..9344121 --- /dev/null +++ b/AntiPattern_Remediator/src/core/agents/explainer.py @@ -0,0 +1,86 @@ +""" +ExplainerAgent — minimal version +- Delegates state handling to create_graph.py +- Only builds messages and parses JSON response +- Keeps code minimal and focused +""" +from __future__ import annotations + +from typing import Dict, Any +import json + +from langchain_core.language_models import BaseLanguageModel +from ..prompt import PromptManager +from src.core.utils import extract_first_json + +PROMPT_KEY = "explainer" + + +class ExplainerAgent: + def __init__(self, llm: BaseLanguageModel, prompt_manager: PromptManager): + self.llm = llm + self.prompt_manager = prompt_manager + + def explain_antipattern(self, state: Dict[str, Any]) -> Dict[str, Any]: + """Generate explanation JSON for detected antipatterns and refactor.""" + kwargs = dict( + code=state.get("code", ""), + language=state.get("language", "Java"), + context=state.get("context", ""), + refactored_code=state.get("refactored_code", ""), + refactor_rationale=state.get("refactor_rationale", ""), + antipattern_name=state.get("antipattern_name", "Unknown antipattern"), + antipattern_description=state.get("antipattern_description", ""), + antipatterns_json=json.dumps(state.get("antipatterns_json", []), ensure_ascii=False), + ) + + messages = self._build_messages(**kwargs) + response = self.llm.invoke(messages) + raw = getattr(response, "content", None) or str(response) + state["explanation_response_raw"] = raw + + parsed = extract_first_json(raw) + state["explanation_json"] = parsed if isinstance(parsed, dict) else {} + return state + + def display_explanation(self, state: Dict[str, Any]) -> Dict[str, Any]: + print("\n=== Explanation (raw) ===\n", state.get("explanation_response_raw", "N/A")) + if state.get("explanation_json"): + print("\n=== Explanation (JSON) ===\n", json.dumps(state["explanation_json"], indent=2, ensure_ascii=False)) + return state + + def _build_messages(self, **kwargs) -> Any: + try: + getp = getattr(self.prompt_manager, "get_prompt", None) + if callable(getp): + prompt = getp(PROMPT_KEY) + if prompt is not None: + return prompt.format_messages(**kwargs) + except Exception: + pass + + schema = { + "items": [{ + "antipattern_name": "", + "antipattern_description": "", + "impact": "", + "why_it_is_bad": "", + "how_we_fixed_it": "", + "refactored_code": "", + "summary": "" + }], + "what_changed": [], + "why_better": [], + "principles_applied": [], + "trade_offs": [], + "closing_summary": "" + } + content = ( + "Given inputs (JSON):\n" + json.dumps(kwargs, ensure_ascii=False) + + "\nRespond with STRICT JSON using exactly this schema:\n" + + json.dumps(schema, ensure_ascii=False) + ) + return [ + {"role": "system", "content": "Return STRICT JSON only. No commentary."}, + {"role": "user", "content": content}, + ] diff --git a/AntiPattern_Remediator/src/core/graph/create_graph.py b/AntiPattern_Remediator/src/core/graph/create_graph.py index 9c4cf93..096d403 100644 --- a/AntiPattern_Remediator/src/core/graph/create_graph.py +++ b/AntiPattern_Remediator/src/core/graph/create_graph.py @@ -14,6 +14,7 @@ from ..agents import RefactorStrategist from ..agents import CodeTransformer from ..agents import CodeReviewerAgent +from ..agents import ExplainerAgent # Imports for LangSmith tracing import os @@ -62,7 +63,7 @@ def __init__(self, db_manager, prompt_manager: PromptManager, retriever=None, ll self.prompt_manager = prompt_manager self.conditional_edges = ConditionalEdges() - # ✅ assign the instance attribute before use + # assign the instance attribute before use self.retriever = retriever or self.db_manager.as_retriever() retriever_tool = create_retriever_tool( @@ -78,13 +79,13 @@ def __init__(self, db_manager, prompt_manager: PromptManager, retriever=None, ll "strategist": RefactorStrategist(self.llm, self.prompt_manager, retriever=self.retriever), "transformer": CodeTransformer(self.llm, self.prompt_manager), "reviewer": CodeReviewerAgent(self.llm, self.prompt_manager), + "explainer": ExplainerAgent(self.llm, self.prompt_manager) } # Build the LangGraph workflow self.workflow = self._build_graph() def _build_graph(self): - """Build LangGraph workflow""" graph = StateGraph(AgentState) # Scanner: retrieve + analyze @@ -104,6 +105,10 @@ def _build_graph(self): graph.add_node("review_code", self.agents["reviewer"].review_code) graph.add_node("display_code_review_results", self.agents["reviewer"].display_code_review_results) + # Explainer: final storytelling + graph.add_node("explain_antipattern", self.agents["explainer"].explain_antipattern) + graph.add_node("display_explanation", self.agents["explainer"].display_explanation) + # Topology graph.set_entry_point("retrieve_context") graph.add_edge("retrieve_context", "analyze_antipatterns") @@ -123,6 +128,8 @@ def _build_graph(self): }, ) - graph.add_edge("display_code_review_results", END) + graph.add_edge("display_code_review_results", "explain_antipattern") + graph.add_edge("explain_antipattern", "display_explanation") + graph.add_edge("display_explanation", END) return graph.compile() diff --git a/AntiPattern_Remediator/src/core/prompt/prompt_manager.py b/AntiPattern_Remediator/src/core/prompt/prompt_manager.py index 0650429..1f1abef 100644 --- a/AntiPattern_Remediator/src/core/prompt/prompt_manager.py +++ b/AntiPattern_Remediator/src/core/prompt/prompt_manager.py @@ -7,27 +7,26 @@ class PromptManager: """Manager for handling prompt templates and configurations.""" def __init__(self): - # Prompt key constants, **same as YAML filenames** + # Prompt key constants, **same as YAML filenames (without .yaml)** self.ANTIPATTERN_SCANNER = "antipattern_scanner" - self.REFACTOR_STRATEGIST = "refactor_strategist" + self.REFACTOR_STRATEGIST = "refactor_strategist" self.CODE_TRANSFORMER = "code_transformer" self.CODE_REVIEWER = "code_reviewer" + self.EXPLAINER_AGENT = "explainer" self.prompt_directory = settings.PROMPT_DIR - # Initialize storage for prompt templates self._prompt_cache = {} - # Load prompts on initialization self._load_all_prompts() def _load_all_prompts(self) -> None: """Load all prompt configurations from YAML files.""" try: - # Get all prompt constants and load corresponding files prompt_constants = [ self.ANTIPATTERN_SCANNER, self.REFACTOR_STRATEGIST, self.CODE_TRANSFORMER, self.CODE_REVIEWER, + self.EXPLAINER_AGENT, ] for prompt_key in prompt_constants: @@ -55,12 +54,17 @@ def _load_prompt_from_yaml(self, filename: str, prompt_key: str) -> None: return prompt_config = config[prompt_key] - # Create ChatPromptTemplate - self._prompt_cache[prompt_key] = ChatPromptTemplate([ - ("system", prompt_config.get('system', '')), - ("user", prompt_config.get('user', '')), - MessagesPlaceholder("msgs") - ]) + + # Build messages in (role, content) format + messages = [] + if prompt_config.get("system"): + messages.append(("system", prompt_config["system"])) + if prompt_config.get("user"): + messages.append(("user", prompt_config["user"])) + messages.append(MessagesPlaceholder("msgs")) + + # Use the correct constructor + self._prompt_cache[prompt_key] = ChatPromptTemplate.from_messages(messages) print(f"Loaded prompt '{prompt_key}' from {filename}") except Exception as e: @@ -70,5 +74,4 @@ def get_prompt(self, prompt_key: str) -> Optional[ChatPromptTemplate]: if prompt_key not in self._prompt_cache: print(f"Warning: Prompt '{prompt_key}' not found in cache") return None - - return self._prompt_cache[prompt_key] \ No newline at end of file + return self._prompt_cache[prompt_key] diff --git a/AntiPattern_Remediator/src/core/state.py b/AntiPattern_Remediator/src/core/state.py index c85077f..63a147f 100644 --- a/AntiPattern_Remediator/src/core/state.py +++ b/AntiPattern_Remediator/src/core/state.py @@ -17,3 +17,5 @@ class AgentState(TypedDict): code_review_times: int # Number of times code has been reviewed msgs: List[Dict[str, Any]] # Message history for conversation context answer: Optional[str] # Analysis result + explanation_response_raw: Optional[str] # Raw LLM output from explainer + explanation_json: Optional[Dict[str, Any]] # Parsed JSON explanation diff --git a/AntiPattern_Remediator/src/core/utils/__init__.py b/AntiPattern_Remediator/src/core/utils/__init__.py new file mode 100644 index 0000000..ccba67c --- /dev/null +++ b/AntiPattern_Remediator/src/core/utils/__init__.py @@ -0,0 +1,3 @@ +from .json_utils import extract_first_json + +__all__ = ["extract_first_json"] \ No newline at end of file diff --git a/AntiPattern_Remediator/src/core/utils/json_utils.py b/AntiPattern_Remediator/src/core/utils/json_utils.py new file mode 100644 index 0000000..d2e0b2f --- /dev/null +++ b/AntiPattern_Remediator/src/core/utils/json_utils.py @@ -0,0 +1,27 @@ +import json + +def extract_first_json(text): + """ + Try to extract the first JSON object from a string. + Works if JSON is inside ```json ... ``` fences or just plain text. + """ + if not isinstance(text, str): + return None + + # 1. If the text has fenced JSON like ```json ... ``` + if "```" in text: + parts = text.split("```") + for part in parts: + # Look for JSON-specific fences + if part.strip().lower().startswith("json"): + json_part = part[len("json"):].strip() + try: + return json.loads(json_part) + except Exception: + pass # Try next part + + # 2. If no fenced JSON worked, try to parse the whole text + try: + return json.loads(text.strip()) + except Exception: + return None \ No newline at end of file diff --git a/AntiPattern_Remediator/static/prompt/explainer.yaml b/AntiPattern_Remediator/static/prompt/explainer.yaml new file mode 100644 index 0000000..8963f95 --- /dev/null +++ b/AntiPattern_Remediator/static/prompt/explainer.yaml @@ -0,0 +1,42 @@ +explainer: + template: | + You are a senior software reviewer. + Your job is to explain detected anti-patterns and the applied refactor in a clear, structured way. + Output STRICT JSON only — no commentary outside JSON. + + === Inputs === + Language: {language} + Context: {context} + Detected Anti-patterns (JSON): {antipatterns_json} + Code: + ```{code}``` + Refactored Code: + ```{refactored_code}``` + Refactor Rationale: + {refactor_rationale} + + === Required Output Schema === + { + "items": [ + { + "antipattern_name": "", + "antipattern_description": "", + "impact": "", + "why_it_is_bad": "", + "how_we_fixed_it": "", + "refactored_code": "", + "summary": "" + } + ], + "what_changed": [], + "why_better": [], + "principles_applied": [], + "trade_offs": [], + "closing_summary": "" + } + + Notes: + - Always return valid JSON. + - Use multiple entries under "items" if more than one antipattern is relevant. + - Keep `refactored_code` short (or truncated if needed). + - Fill all fields, even if briefly. diff --git a/AntiPattern_Remediator/static/tinydb.json b/AntiPattern_Remediator/static/tinydb.json index cd87283..8f6198a 100644 --- a/AntiPattern_Remediator/static/tinydb.json +++ b/AntiPattern_Remediator/static/tinydb.json @@ -1 +1 @@ -{"_default": {"1": {"name": "Deep Nesting", "description": "Deep Nesting occurs when conditional or loop blocks are embedded within one another across multiple levels, creating code with high indentation and complex control flow. While not always increasing cyclomatic complexity linearly, deep nesting significantly raises cognitive complexity (the mental effort required to understand, modify, and debug a method).", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Low readability : The \"arrowhead\" structure makes it hard to trace logic and understand the intended flow.\nHigh cognitive load : Developers must mentally track the conditions that lead to or prevent reaching a particular line of code.\nError-prone maintenance : Adding or modifying logic inside deeply nested blocks increases the risk of missing cases or introducing bugs.\nInhibited reuse and testing : Deep nesting often combines concerns that should be split into smaller, testable methods or units.\nPoor diffs in version control : Even small changes can alter indentation across many lines, making reviews harder.", "remediation": "Guard clauses (early return) : Exit early when preconditions fail, flattening the control flow\nExtract method : Isolate deeply nested blocks into private methods with clear names to separate concerns and reduce depth.\nInvet conditionals : Invert logic to return early or skip unnecessary branches.\nReplace nested loops with streams (Java-specific): Abstract common filtering or mapping logic into declarative operations.\nUse pattern matching : Replace layered `if` chains with clearer structural or type-based matching.\nEncapsulate state checks : Group multiple conditionals into intention-revealing boolean helpers or state objects.", "limitation": "Can conflict with existing code style : Teams unfamiliar with guard clauses may resist early exits or multiple returns.\nRefactoring can obscure logic during transition : Extracted methods must be clearly named to preserve readability and avoid confusion.\nNested logic may be unavoidable in rare edge cases : Complex parsing, state machines, or embedded domain-specific languages may naturally involve deeper control structures.", "type": "antipattern", "source_file": "deep_nesting.json"}, "2": {"name": "Generic Exception handling", "description": "Generic exception handling refers to the use of broad or unspecific catch blocks (catching but ignoring the exceptions). These patterns obscure the true source of errors, suppress the useful debugging information and can unintentionally hide critical failures. This anti-pattern often stems from the desire to keep the code running, but typically leads to weak systems and increased technical debt.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Loss of context : Catching high-level exceptions removes the granularity needed to understand specific failure reasons\nRepeated boilerplate : Developers may re-implement logging, default values, or stream-closing logic instead of using safe utility methods\nViolation of fail-fast principles : Silent or overly generic handling can let critical errors go unnoticed for too long", "remediation": "", "limitation": "Catching too many specific exceptions can bloat the code and reduce readability.\nRefactoring exception handling may require thorough testing to avoid regressions.\nOver-logging exceptions can clutter logs and obscure real issues.\nSecurity-sensitive applications may need specialised exception handling strategies to avoid leaks", "type": "antipattern", "source_file": "generic_exception_handling.json"}, "3": {"name": "Magic Constants", "description": "Magic constants (or magic numbers) are hard-coded literal values (e.g., 3.14, 42, \"admin\") that appear directly in code without context or explanation. These values become problematic when their purpose is unclear, undocumented, or reused inconsistently. While some literals (like 0, 1, or -1) may be self-explanatory in some contexts, others represent thresholds, identifiers, or rules that should be named and documented for clarity.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor readability : Developers must guess the meaning of the value, increasing cognitive load.\nLow maintainability : Changing the value requires updating it everywhere, risking inconsistent updates.\nHarder debugging : Literal values lack descriptive meaning in stack traces, logs, or debuggers.\nDuplication : The same literal used in multiple places leads to repeated logic and tighter coupling.\nViolation of DRY : Embeds implicit meaning multiple times without abstraction.", "remediation": "Define Constants - Move literal values into named constants (e.g., const, final, or static readonly).\nUse Enums - Group related constants (e.g., roles, statuses) as enumerations with meaningful names.", "limitation": "Extracting trivial values like 0 or 1 may clutter code and reduce clarity if overdone.\nIn performance-critical code, indirection through constants or functions may introduce slight overhead.\nOver-abstracting unnamed constants (e.g., MAXCOUNT3) may make code harder to understand.\nIf a value is used only once and is self-explanatory, extracting it may be unnecessary overhead.", "type": "antipattern", "source_file": "magic_constants.json"}, "4": {"name": "SRP Violation", "description": "The Single Responsibility Principle (SRP) is one of the SOLID principles of object-oriented design, stating that a class (or, at a lower level, a method) should have only one reason to change (i.e., it should have only one responsibility or concern). A SRP Violation occurs when a class (method) takes on multiple unrelated responsibilities, making it harder to maintain, test, and understand. These violations often result in bloated classes or methods that mix unrelated concerns such as I/O, business logic, error handling, and configuration.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor maintainability : Changing one responsibility might inadvertently affect others, introducing bugs.\nLow cohesion : Code with unrelated responsibilities lacks a clear purpose, reducing clarity and reusability.\nDifficult testing : Unit testing becomes more complex as setup may require mocking or initializing unrelated dependencies.\nCode duplication and tight coupling : Responsibilities are harder to reuse or share, often leading to repeated logic or tight inter-class dependencies.\nHarder onboarding : New developers struggle to understand the purpose and scope of large, multi-purpose classes.", "remediation": "Extract Method \\- Split complex methods into smaller, single-purpose private methods.\nEarly Return (Guard Clauses) \\- Use early exits to reduce nested logic and clarify separate responsibilities.\nUse Local Functions/Lambdas \\- Encapsulate small inline logic into local functions for clarity.\nEncapsulate Temporary Variables \\- Move logic-heavy expressions into descriptive helper methods.\nGroup Related Logic \\- Cluster related operations into distinct helper methods within the same class.\nSeparate Concerns in Loops \\- Extract filtering, transforming, and aggregating into distinct steps.\nIsolate Logging/Error Handling \\- Move side-effect code like logging into dedicated private methods (unless the logging is a trivial single line and does not obscure business logic).", "limitation": "Fixing certain SRP violations (especially class-level violations) requires changing public method signatures, creating new classes, and/or breaking interfaces.\nMethods often depend on multiple injected services or shared state; untangling responsibilities might require broader architectural changes.\nLogging, error handling, metrics, and security checks are often scattered across responsibilities and difficult to isolate cleanly at the method level without aspect-oriented programming (AOP) or middleware/interceptor patterns.", "type": "antipattern", "source_file": "srp_violation.json"}, "5": {"name": "Unsafe or Vague Exception Handling", "description": "Reliable exception handling, type safety, and controlled flow are essential forr writing maintainable and robust software. Unsafe or Vague exception handling often results in code that is fragile, difficult to test, and challenging to debug. Instead of providing meaningful error handling or clear separation of concerns, these implementations either hide the underlying problem or use language features in a way that breaks maintainability.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Hidden failures : Catching exceptions without logging or meaningful handling hides the source of issues and makes debugging difficult\nPoor diagnosis : Broad exception handling with vague error messages hides intent and makes it harder to trace the root cause\nInconsistent runtime behaviour : using assertions for control logic can lead to unpredictable behaviour depending on the JVM configuration\nUncontrolled termination : Using system.exit() directly in application logic makes code untestable and prevents proper resource cleanup", "remediation": "Replace assertions with proper condition checks and informative exceptions to ensure consistent behaviour across environments\nAvoid silent catch blocks, log exceptions or rethrow them to preserve error context\nHandle specific exception types instead of catching broad categories like Exception or RuntimeException\nRefactor abrupt shutdown calls into controlled exits using exception handling or return code to support recovery", "limitation": "", "type": "antipattern", "source_file": "unsafe_or_vague_exception_handling.json"}}} \ No newline at end of file +{"_default": {"1": {"name": "Monolithic Method", "description": "A Monolithic Method is a single method that tries to do too much, often combining unrelated responsibilities into one block of code. This is essentially a method-level violation of the Single Responsibility Principle (SRP). Monolithic methods are hard to read, understand, maintain, and test because they mix business logic, I/O, error handling, and other concerns in one place.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor maintainability : Modifying one part of the method risks breaking unrelated functionality.\nLow readability : Long, complex methods are hard to follow and understand.\nDifficult testing : Unit tests become cumbersome because the method does too many things at once.\nCode duplication : Reusing logic is difficult; similar tasks often get reimplemented elsewhere.\nTight coupling : Internal details are intertwined, making refactoring risky.", "remediation": "", "limitation": "Refactoring may require changes to public method signatures, especially if other code depends on it.\nSome logic may rely on shared state or multiple services, making it hard to separate without broader architectural changes.\nIf the method handles cross-cutting concerns (e.g., logging, metrics, validation), isolating responsibilities may require AOP or middleware.", "type": "antipattern", "source_file": "monolithic_method.json"}, "2": {"name": "Unsafe or Vague Exception Handling", "description": "Reliable exception handling, type safety, and controlled flow are essential forr writing maintainable and robust software. Unsafe or Vague exception handling often results in code that is fragile, difficult to test, and challenging to debug. Instead of providing meaningful error handling or clear separation of concerns, these implementations either hide the underlying problem or use language features in a way that breaks maintainability.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Hidden failures : Catching exceptions without logging or meaningful handling hides the source of issues and makes debugging difficult\nPoor diagnosis : Broad exception handling with vague error messages hides intent and makes it harder to trace the root cause\nInconsistent runtime behaviour : using assertions for control logic can lead to unpredictable behaviour depending on the JVM configuration\nUncontrolled termination : Using system.exit() directly in application logic makes code untestable and prevents proper resource cleanup", "remediation": "Replace assertions with proper condition checks and informative exceptions to ensure consistent behaviour across environments\nAvoid silent catch blocks, log exceptions or rethrow them to preserve error context\nHandle specific exception types instead of catching broad categories like Exception or RuntimeException\nRefactor abrupt shutdown calls into controlled exits using exception handling or return code to support recovery", "limitation": "", "type": "antipattern", "source_file": "unsafe_or_vague_exception_handling.json"}, "3": {"name": "Deep Nesting", "description": "Deep Nesting occurs when conditional or loop blocks are embedded within one another across multiple levels, creating code with high indentation and complex control flow. While not always increasing cyclomatic complexity linearly, deep nesting significantly raises cognitive complexity (the mental effort required to understand, modify, and debug a method).", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Low readability : The \"arrowhead\" structure makes it hard to trace logic and understand the intended flow.\nHigh cognitive load : Developers must mentally track the conditions that lead to or prevent reaching a particular line of code.\nError-prone maintenance : Adding or modifying logic inside deeply nested blocks increases the risk of missing cases or introducing bugs.\nInhibited reuse and testing : Deep nesting often combines concerns that should be split into smaller, testable methods or units.\nPoor diffs in version control : Even small changes can alter indentation across many lines, making reviews harder.", "remediation": "Guard clauses (early return) : Exit early when preconditions fail, flattening the control flow\nExtract method : Isolate deeply nested blocks into private methods with clear names to separate concerns and reduce depth.\nInvet conditionals : Invert logic to return early or skip unnecessary branches.\nReplace nested loops with streams (Java-specific): Abstract common filtering or mapping logic into declarative operations.\nUse pattern matching : Replace layered `if` chains with clearer structural or type-based matching.\nEncapsulate state checks : Group multiple conditionals into intention-revealing boolean helpers or state objects.", "limitation": "Can conflict with existing code style : Teams unfamiliar with guard clauses may resist early exits or multiple returns.\nRefactoring can obscure logic during transition : Extracted methods must be clearly named to preserve readability and avoid confusion.\nNested logic may be unavoidable in rare edge cases : Complex parsing, state machines, or embedded domain-specific languages may naturally involve deeper control structures.", "type": "antipattern", "source_file": "deep_nesting.json"}, "4": {"name": "SRP Violation", "description": "The Single Responsibility Principle (SRP) is one of the SOLID principles of object-oriented design, stating that a class (or, at a lower level, a method) should have only one reason to change (i.e., it should have only one responsibility or concern). A SRP Violation occurs when a class (method) takes on multiple unrelated responsibilities, making it harder to maintain, test, and understand. These violations often result in bloated classes or methods that mix unrelated concerns such as I/O, business logic, error handling, and configuration.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor maintainability : Changing one responsibility might inadvertently affect others, introducing bugs.\nLow cohesion : Code with unrelated responsibilities lacks a clear purpose, reducing clarity and reusability.\nDifficult testing : Unit testing becomes more complex as setup may require mocking or initializing unrelated dependencies.\nCode duplication and tight coupling : Responsibilities are harder to reuse or share, often leading to repeated logic or tight inter-class dependencies.\nHarder onboarding : New developers struggle to understand the purpose and scope of large, multi-purpose classes.", "remediation": "Extract Method \\- Split complex methods into smaller, single-purpose private methods.\nEarly Return (Guard Clauses) \\- Use early exits to reduce nested logic and clarify separate responsibilities.\nUse Local Functions/Lambdas \\- Encapsulate small inline logic into local functions for clarity.\nEncapsulate Temporary Variables \\- Move logic-heavy expressions into descriptive helper methods.\nGroup Related Logic \\- Cluster related operations into distinct helper methods within the same class.\nSeparate Concerns in Loops \\- Extract filtering, transforming, and aggregating into distinct steps.\nIsolate Logging/Error Handling \\- Move side-effect code like logging into dedicated private methods (unless the logging is a trivial single line and does not obscure business logic).", "limitation": "Fixing certain SRP violations (especially class-level violations) requires changing public method signatures, creating new classes, and/or breaking interfaces.\nMethods often depend on multiple injected services or shared state; untangling responsibilities might require broader architectural changes.\nLogging, error handling, metrics, and security checks are often scattered across responsibilities and difficult to isolate cleanly at the method level without aspect-oriented programming (AOP) or middleware/interceptor patterns.", "type": "antipattern", "source_file": "srp_violation.json"}, "5": {"name": "Magic Constants", "description": "Magic constants (or magic numbers) are hard-coded literal values (e.g., 3.14, 42, \"admin\") that appear directly in code without context or explanation. These values become problematic when their purpose is unclear, undocumented, or reused inconsistently. While some literals (like 0, 1, or -1) may be self-explanatory in some contexts, others represent thresholds, identifiers, or rules that should be named and documented for clarity.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor readability : Developers must guess the meaning of the value, increasing cognitive load.\nLow maintainability : Changing the value requires updating it everywhere, risking inconsistent updates.\nHarder debugging : Literal values lack descriptive meaning in stack traces, logs, or debuggers.\nDuplication : The same literal used in multiple places leads to repeated logic and tighter coupling.\nViolation of DRY : Embeds implicit meaning multiple times without abstraction.", "remediation": "Define Constants - Move literal values into named constants (e.g., const, final, or static readonly).\nUse Enums - Group related constants (e.g., roles, statuses) as enumerations with meaningful names.", "limitation": "Extracting trivial values like 0 or 1 may clutter code and reduce clarity if overdone.\nIn performance-critical code, indirection through constants or functions may introduce slight overhead.\nOver-abstracting unnamed constants (e.g., MAXCOUNT3) may make code harder to understand.\nIf a value is used only once and is self-explanatory, extracting it may be unnecessary overhead.", "type": "antipattern", "source_file": "magic_constants.json"}, "6": {"name": "Duplicate Code", "description": "Duplicate Code occurs when identical or very similar code blocks are repeated throughout the codebase. This pattern creates maintenance overhead, increases the likelihood of bugs, and violates the DRY (Don't Repeat Yourself) principle. Common examples include repeated null checks, validation logic, and similar conditional patterns across different methods or classes.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Maintenance overhead : Changes need to be applied in multiple places, increasing the risk of inconsistencies\nBug multiplication : A bug in duplicated code affects multiple locations, making fixes more complex\nCode bloat : Repeated code increases the overall size of the codebase without adding functionality\nViolation of DRY principle : Makes the code harder to understand and reason about", "remediation": "", "limitation": "Over-abstraction can make code harder to understand if the duplication is minimal or contextually different\nPremature extraction of methods may create unnecessary coupling between unrelated parts of the system\nSome duplication might be acceptable if the code serves different business contexts", "type": "antipattern", "source_file": "duplicate_code.json"}, "7": {"name": "God Class", "description": "A God Class anti-pattern refers to a class that centralises too many responsibilities in a single location, becoming overly complex and difficult to maintain. Such classes tend to know too much, do too much, and interact with many different parts of the system. This leads to tightly coupled code, reduces modularity, and makes the system hard to test and extend.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "High coupling: God Classes tend to interact with many other classes and modules, thereby reducing system modularity and increasing the risk of changes.\nPoor maintainability : Large, complex classes are difficult to read, understand, and modify.\nLow reusability : The class becomes so specific and bloated that it is rarely useful outside of its original context.\nHidden dependencies : God Classes often hide dependencies within fields or methods, making the codebase less transparent.", "remediation": "", "limitation": "", "type": "antipattern", "source_file": "god_class.json"}, "8": {"name": "Middle Man", "description": "The Middle Man anti-pattern occurs when a class exists primarily to delegate calls to another class without adding meaningful logic of its own. Essentially, the class acts as a pass-through or proxy, forwarding method calls without adding value. While delegation is sometimes necessary for abstraction, excessive or trivial delegation leads to unnecessary indirection and increases maintenance overhead.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Extra Indirection : Code must go through one more layer, which can complicate understanding the code flow.\nIncreased maintenance : When the delegated class changes, the middle-man class often needs updates for all its pass-through methods.\nLow cohesion : The middle-man class has little real logic, making its purpose unclear\nCode bloat : Many trivial delegation methods clutter the class, making it harder to navigate.\nHarder debugging : Tracing behaviour through multiple layers of delegation adds cognitive load.", "remediation": "", "limitation": "Some delegation is unavoidable, e.g., to implement an interface or provide a stable abstraction layer\nRemoving middleman classes may break existing APIs or require refactoring client code.\nIn some cases, delegation is part of a design pattern (like a proxy), which is intentional and not an anti-pattern", "type": "antipattern", "source_file": "middle_man.json"}, "9": {"name": "Generic Exception handling", "description": "Generic exception handling refers to the use of broad or unspecific catch blocks (catching but ignoring the exceptions). These patterns obscure the true source of errors, suppress the useful debugging information and can unintentionally hide critical failures. This anti-pattern often stems from the desire to keep the code running, but typically leads to weak systems and increased technical debt.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Loss of context : Catching high-level exceptions removes the granularity needed to understand specific failure reasons\nRepeated boilerplate : Developers may re-implement logging, default values, or stream-closing logic instead of using safe utility methods\nViolation of fail-fast principles : Silent or overly generic handling can let critical errors go unnoticed for too long", "remediation": "", "limitation": "Catching too many specific exceptions can bloat the code and reduce readability.\nRefactoring exception handling may require thorough testing to avoid regressions.\nOver-logging exceptions can clutter logs and obscure real issues.\nSecurity-sensitive applications may need specialised exception handling strategies to avoid leaks", "type": "antipattern", "source_file": "generic_exception_handling.json"}}} \ No newline at end of file From e921bf9356e513ba6b8a2792da4fe7ae83416140 Mon Sep 17 00:00:00 2001 From: Avinash Date: Sat, 23 Aug 2025 10:36:01 +0100 Subject: [PATCH 2/7] Unit test updated for explainer --- .../src/core/prompt/prompt_manager.py | 31 ++++++++++--------- .../unit_test/prompt/test_prompt_manager.py | 20 ++++++++++-- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/AntiPattern_Remediator/src/core/prompt/prompt_manager.py b/AntiPattern_Remediator/src/core/prompt/prompt_manager.py index 1f1abef..d7fc7dd 100644 --- a/AntiPattern_Remediator/src/core/prompt/prompt_manager.py +++ b/AntiPattern_Remediator/src/core/prompt/prompt_manager.py @@ -41,34 +41,37 @@ def _load_all_prompts(self) -> None: def _load_prompt_from_yaml(self, filename: str, prompt_key: str) -> None: """Load a prompt configuration from a YAML file.""" yaml_path = self.prompt_directory / filename - + if not yaml_path.exists(): print(f"Warning: Prompt file {yaml_path} not found") return - + try: with open(yaml_path, 'r', encoding='utf-8') as file: config = yaml.safe_load(file) - if prompt_key not in config: + + if not config or prompt_key not in config: print(f"Warning: Section '{prompt_key}' not found in {filename}") return - - prompt_config = config[prompt_key] - # Build messages in (role, content) format - messages = [] - if prompt_config.get("system"): - messages.append(("system", prompt_config["system"])) - if prompt_config.get("user"): - messages.append(("user", prompt_config["user"])) - messages.append(MessagesPlaceholder("msgs")) + prompt_config = config.get(prompt_key) or {} + + # Always include System first (empty string if not provided) to satisfy tests + system_text = str(prompt_config.get("system", "") or "") + user_text = str(prompt_config.get("user", "") or "") + + messages = [ + ("system", system_text), # always present (possibly empty) + ("user", user_text), # always present (possibly empty) + MessagesPlaceholder("msgs"), # conversation history + ] - # Use the correct constructor self._prompt_cache[prompt_key] = ChatPromptTemplate.from_messages(messages) print(f"Loaded prompt '{prompt_key}' from {filename}") - + except Exception as e: print(f"Error loading prompt from {filename}: {e}") + def get_prompt(self, prompt_key: str) -> Optional[ChatPromptTemplate]: if prompt_key not in self._prompt_cache: diff --git a/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py b/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py index 088af2f..6e859c4 100644 --- a/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py +++ b/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py @@ -44,6 +44,7 @@ def __init__(self): self.REFACTOR_STRATEGIST = "refactor_strategist" self.CODE_TRANSFORMER = "code_transformer" self.CODE_REVIEWER = "code_reviewer" + self.EXPLAINER_AGENT = "explainer" self.prompt_directory = Path(__file__).parent self._prompt_cache = {} self._load_all_prompts() # Call this to match real behavior @@ -87,7 +88,8 @@ def _load_all_prompts(self): self.ANTIPATTERN_SCANNER, self.REFACTOR_STRATEGIST, self.CODE_TRANSFORMER, - self.CODE_REVIEWER + self.CODE_REVIEWER, + self.EXPLAINER_AGENT, ] for prompt_key in prompt_constants: @@ -155,6 +157,7 @@ def test_initialization_creates_correct_attributes(self): assert hasattr(manager, 'REFACTOR_STRATEGIST') assert hasattr(manager, 'CODE_TRANSFORMER') assert hasattr(manager, 'CODE_REVIEWER') + assert hasattr(manager, 'EXPLAINER_AGENT') assert hasattr(manager, 'prompt_directory') assert hasattr(manager, '_prompt_cache') assert isinstance(manager._prompt_cache, dict) @@ -176,6 +179,7 @@ def test_prompt_constants_have_correct_values(self): assert manager.REFACTOR_STRATEGIST == "refactor_strategist" assert manager.CODE_TRANSFORMER == "code_transformer" assert manager.CODE_REVIEWER == "code_reviewer" + assert manager.EXPLAINER_AGENT == "explainer" def test_prompt_directory_is_set_correctly(self): """Test that prompt directory is assigned from settings.""" @@ -505,6 +509,7 @@ def test_initialization_with_missing_directory(self, capsys): manager.REFACTOR_STRATEGIST = "refactor_strategist" manager.CODE_TRANSFORMER = "code_transformer" manager.CODE_REVIEWER = "code_reviewer" + manager.EXPLAINER_AGENT = "explainer" manager.prompt_directory = Path("/non/existent/path") manager._prompt_cache = {} @@ -575,6 +580,12 @@ def temp_prompt_files(self): 'system': 'You are an expert code reviewer.', 'user': 'Review this code for quality and best practices: {code}' } + }, + 'explainer.yaml': { + 'explainer': { + 'system': 'You are a senior software reviewer.', + 'user': 'Explain: {code}\nLang: {language}\nCtx: {context}' + } } } @@ -593,6 +604,7 @@ def test_load_all_prompts_loads_all_available_files(self, temp_prompt_files, cap manager.REFACTOR_STRATEGIST = "refactor_strategist" manager.CODE_TRANSFORMER = "code_transformer" manager.CODE_REVIEWER = "code_reviewer" + manager.EXPLAINER_AGENT = "explainer" manager.prompt_directory = temp_prompt_files manager._prompt_cache = {} @@ -600,15 +612,16 @@ def test_load_all_prompts_loads_all_available_files(self, temp_prompt_files, cap manager._load_all_prompts() # Assert: Verify all prompts were loaded - assert len(manager._prompt_cache) == 4 + assert len(manager._prompt_cache) == 5 assert "antipattern_scanner" in manager._prompt_cache assert "refactor_strategist" in manager._prompt_cache assert "code_transformer" in manager._prompt_cache assert "code_reviewer" in manager._prompt_cache + assert "explainer" in manager._prompt_cache # Verify success message captured = capsys.readouterr() - assert "Successfully loaded 4 prompts" in captured.out + assert "Successfully loaded 5 prompts" in captured.out def test_load_all_prompts_handles_partial_failures(self, capsys): """Test that _load_all_prompts continues loading even if some files are missing.""" @@ -633,6 +646,7 @@ def test_load_all_prompts_handles_partial_failures(self, capsys): manager.REFACTOR_STRATEGIST = "refactor_strategist" manager.CODE_TRANSFORMER = "code_transformer" manager.CODE_REVIEWER = "code_reviewer" + manager.EXPLAINER_AGENT = "explainer" manager.prompt_directory = temp_path manager._prompt_cache = {} From c61bac754d75a727e18c95af88e2c5b56ad21d88 Mon Sep 17 00:00:00 2001 From: Avinash Date: Wed, 27 Aug 2025 10:16:54 +0100 Subject: [PATCH 3/7] Final changes --- .../src/core/agents/explainer.py | 85 ++++++++++++++----- AntiPattern_Remediator/src/core/state.py | 21 +++-- .../static/prompt/explainer.yaml | 27 +++--- .../unit_test/prompt/test_prompt_manager.py | 2 +- 4 files changed, 91 insertions(+), 44 deletions(-) diff --git a/AntiPattern_Remediator/src/core/agents/explainer.py b/AntiPattern_Remediator/src/core/agents/explainer.py index 9344121..4ab86a6 100644 --- a/AntiPattern_Remediator/src/core/agents/explainer.py +++ b/AntiPattern_Remediator/src/core/agents/explainer.py @@ -1,8 +1,8 @@ """ ExplainerAgent — minimal version - Delegates state handling to create_graph.py -- Only builds messages and parses JSON response -- Keeps code minimal and focused +- Uses PromptManager if available; otherwise a tiny inline fallback prompt +- Always passes msgs; always returns a non-empty explanation_json """ from __future__ import annotations @@ -10,6 +10,7 @@ import json from langchain_core.language_models import BaseLanguageModel +from langchain.prompts import ChatPromptTemplate, MessagesPlaceholder from ..prompt import PromptManager from src.core.utils import extract_first_json @@ -32,33 +33,55 @@ def explain_antipattern(self, state: Dict[str, Any]) -> Dict[str, Any]: antipattern_name=state.get("antipattern_name", "Unknown antipattern"), antipattern_description=state.get("antipattern_description", ""), antipatterns_json=json.dumps(state.get("antipatterns_json", []), ensure_ascii=False), + msgs=state.get("msgs", []), # ensure MessagesPlaceholder is satisfied ) messages = self._build_messages(**kwargs) - response = self.llm.invoke(messages) - raw = getattr(response, "content", None) or str(response) + + try: + response = self.llm.invoke(messages) + raw = getattr(response, "content", None) or str(response) + except Exception as e: + raw = f"LLM error: {e}" + state["explanation_response_raw"] = raw - parsed = extract_first_json(raw) - state["explanation_json"] = parsed if isinstance(parsed, dict) else {} + # Robust parse: accept dict, wrap list, or emit a minimal fallback + try: + parsed = extract_first_json(raw) + except Exception: + parsed = None + + if isinstance(parsed, dict): + state["explanation_json"] = parsed + elif isinstance(parsed, list): + state["explanation_json"] = {"items": parsed} + else: + state["explanation_json"] = self._fallback_payload(state) + return state def display_explanation(self, state: Dict[str, Any]) -> Dict[str, Any]: print("\n=== Explanation (raw) ===\n", state.get("explanation_response_raw", "N/A")) if state.get("explanation_json"): - print("\n=== Explanation (JSON) ===\n", json.dumps(state["explanation_json"], indent=2, ensure_ascii=False)) + print("\n=== Explanation (JSON) ===\n", + json.dumps(state["explanation_json"], indent=2, ensure_ascii=False)) return state def _build_messages(self, **kwargs) -> Any: - try: - getp = getattr(self.prompt_manager, "get_prompt", None) - if callable(getp): - prompt = getp(PROMPT_KEY) - if prompt is not None: - return prompt.format_messages(**kwargs) - except Exception: - pass + # Always ensure msgs exists + if "msgs" not in kwargs or kwargs["msgs"] is None: + kwargs = {**kwargs, "msgs": []} + # 1) Try preloaded template from PromptManager + prompt = None + getp = getattr(self.prompt_manager, "get_prompt", None) + if callable(getp): + prompt = getp(PROMPT_KEY) + if prompt is not None: + return prompt.format_messages(**kwargs) + + # 2) Minimal inline fallback schema = { "items": [{ "antipattern_name": "", @@ -76,11 +99,33 @@ def _build_messages(self, **kwargs) -> Any: "closing_summary": "" } content = ( - "Given inputs (JSON):\n" + json.dumps(kwargs, ensure_ascii=False) + + "Given inputs (JSON):\n" + json.dumps({k: v for k, v in kwargs.items() if k != "msgs"}, ensure_ascii=False) + "\nRespond with STRICT JSON using exactly this schema:\n" + json.dumps(schema, ensure_ascii=False) ) - return [ - {"role": "system", "content": "Return STRICT JSON only. No commentary."}, - {"role": "user", "content": content}, - ] + fallback = ChatPromptTemplate.from_messages([ + ("system", "Return STRICT JSON only. No commentary."), + ("user", content), + MessagesPlaceholder("msgs"), + ]) + return fallback.format_messages(**kwargs) + + @staticmethod + def _fallback_payload(state: Dict[str, Any]) -> Dict[str, Any]: + """Tiny fallback so downstream never breaks if parsing fails.""" + return { + "items": [{ + "antipattern_name": state.get("antipattern_name", "Unknown antipattern"), + "antipattern_description": state.get("antipattern_description", ""), + "impact": "", + "why_it_is_bad": "", + "how_we_fixed_it": state.get("refactor_rationale", ""), + "refactored_code": state.get("refactored_code", ""), + "summary": "Auto-generated minimal explanation (parser fallback)." + }], + "what_changed": [], + "why_better": [], + "principles_applied": [], + "trade_offs": [], + "closing_summary": "" + } diff --git a/AntiPattern_Remediator/src/core/state.py b/AntiPattern_Remediator/src/core/state.py index 63a147f..3fa9562 100644 --- a/AntiPattern_Remediator/src/core/state.py +++ b/AntiPattern_Remediator/src/core/state.py @@ -8,14 +8,17 @@ class AgentState(TypedDict): """State definition for passing data through the workflow""" code: str # Code to be analyzed + language: Optional[str] # Language of the code (used by ExplainerAgent) context: Optional[str] # Context retrieved from knowledge base (scanner) trove_context: Optional[str] # Context retrieved from the Anti-Pattern Trove (TinyDB/Chroma) - antipatterns_scanner_results: Optional[str] - refactoring_strategy_results: Optional[str] # Refactoring strategy generated by strategist - refactored_code: Optional[str] # Code after refactoring - code_review_results: Optional[str] # Code review results - code_review_times: int # Number of times code has been reviewed - msgs: List[Dict[str, Any]] # Message history for conversation context - answer: Optional[str] # Analysis result - explanation_response_raw: Optional[str] # Raw LLM output from explainer - explanation_json: Optional[Dict[str, Any]] # Parsed JSON explanation + antipatterns_scanner_results: Optional[Dict[str, Any]] # Scanner output (structured) + antipatterns_json: Optional[List[Dict[str, Any]]] # Normalized list used by ExplainerAgent + refactoring_strategy_results: Optional[Any] # Strategy can be dict/list/str depending on agent + refactored_code: Optional[str] # Code after refactoring + code_review_results: Optional[str] # Code review results + code_review_times: int # Number of times code has been reviewed + msgs: List[Any] # Conversation history (LangChain BaseMessages) + answer: Optional[str] # Final/aggregated analysis result + + explanation_response_raw: Optional[str] # Raw LLM output from explainer + explanation_json: Optional[Dict[str, Any]] # Parsed JSON explanation diff --git a/AntiPattern_Remediator/static/prompt/explainer.yaml b/AntiPattern_Remediator/static/prompt/explainer.yaml index 8963f95..be29ab7 100644 --- a/AntiPattern_Remediator/static/prompt/explainer.yaml +++ b/AntiPattern_Remediator/static/prompt/explainer.yaml @@ -1,9 +1,8 @@ explainer: - template: | - You are a senior software reviewer. - Your job is to explain detected anti-patterns and the applied refactor in a clear, structured way. - Output STRICT JSON only — no commentary outside JSON. + system: | + You are a senior software reviewer. Output STRICT JSON only — no commentary outside the JSON object. + user: | === Inputs === Language: {language} Context: {context} @@ -15,10 +14,10 @@ explainer: Refactor Rationale: {refactor_rationale} - === Required Output Schema === - { + === Return EXACTLY this JSON structure === + {{ "items": [ - { + {{ "antipattern_name": "", "antipattern_description": "", "impact": "", @@ -26,17 +25,17 @@ explainer: "how_we_fixed_it": "", "refactored_code": "", "summary": "" - } + }} ], "what_changed": [], "why_better": [], "principles_applied": [], "trade_offs": [], "closing_summary": "" - } + }} - Notes: - - Always return valid JSON. - - Use multiple entries under "items" if more than one antipattern is relevant. - - Keep `refactored_code` short (or truncated if needed). - - Fill all fields, even if briefly. + Rules: + - Return valid JSON only (no code fences, no prose). + - If multiple antipatterns apply, include multiple entries in "items". + - Keep "refactored_code" concise; truncate if needed but keep it valid. + - Populate all fields; use brief placeholders if unsure. diff --git a/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py b/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py index 6e859c4..1fb5dec 100644 --- a/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py +++ b/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py @@ -583,7 +583,7 @@ def temp_prompt_files(self): }, 'explainer.yaml': { 'explainer': { - 'system': 'You are a senior software reviewer.', + 'system': 'You are a senior software code explainer.', 'user': 'Explain: {code}\nLang: {language}\nCtx: {context}' } } From 710d95e6aaf4ead9aa37e7ede78026125a8c293e Mon Sep 17 00:00:00 2001 From: Avinash Date: Fri, 22 Aug 2025 15:51:18 +0100 Subject: [PATCH 4/7] Final changes for explainer agent --- AntiPattern_Remediator/main.py | 113 ++++++++---------- .../src/core/agents/__init__.py | 4 +- .../src/core/agents/explainer.py | 86 +++++++++++++ .../src/core/graph/create_graph.py | 13 +- .../src/core/prompt/prompt_manager.py | 29 +++-- AntiPattern_Remediator/src/core/state.py | 2 + .../src/core/utils/__init__.py | 3 + .../src/core/utils/json_utils.py | 27 +++++ .../static/prompt/explainer.yaml | 42 +++++++ AntiPattern_Remediator/static/tinydb.json | 2 +- 10 files changed, 240 insertions(+), 81 deletions(-) create mode 100644 AntiPattern_Remediator/src/core/agents/explainer.py create mode 100644 AntiPattern_Remediator/src/core/utils/__init__.py create mode 100644 AntiPattern_Remediator/src/core/utils/json_utils.py create mode 100644 AntiPattern_Remediator/static/prompt/explainer.yaml diff --git a/AntiPattern_Remediator/main.py b/AntiPattern_Remediator/main.py index a659983..1c96cc8 100644 --- a/AntiPattern_Remediator/main.py +++ b/AntiPattern_Remediator/main.py @@ -1,9 +1,7 @@ - """ Main entry point - Legacy Code Migration Tool """ from config.settings import initialize_settings -# from scripts import seed_database from dotenv import load_dotenv load_dotenv() from colorama import Fore, Style @@ -12,11 +10,34 @@ from full_repo_workflow import run_full_repo_workflow -def run_code_snippet_workflow(settings, db_manager, prompt_manager, langgraph): - """Run the original workflow with a hardcoded Java code snippet.""" - print(Fore.BLUE + "\n=== Code Snippet Analysis Workflow ===" + Style.RESET_ALL) - print("Analyzing the provided Java code snippet...") - +def main(): + """Main function: Run antipattern analysis""" + + # Let user select provider + print("Available providers: 1) ollama 2) ibm 3) vllm") + choice = input("Select provider (1-3): ").strip() + + provider_map = {"1": "ollama", "2": "ibm", "3": "vllm"} + provider = provider_map.get(choice, "ollama") # default to ollama + + # Let us choose which DB to interact with + print("Choose your trove: 1) ChromaDB (VectorDB) 2) TinyDB (DocumentDB)") + db_choice = input("Choose 1 or 2: ").strip() + + # Initialize global settings with selected provider + settings = initialize_settings(provider) + print(Fore.GREEN + f"Using {settings.LLM_PROVIDER} with model {settings.LLM_MODEL}" + Style.RESET_ALL) + + # Temporary Lazy Imports + from src.core.graph import CreateGraph + from src.data.database import VectorDBManager, TinyDBManager + from src.core.prompt import PromptManager + from scripts import seed_database + + # Initialize PromptManager + print("Initializing PromptManager...") + prompt_manager = PromptManager() + # Example Java code legacy_code = """ public class ApplicationManager { @@ -55,7 +76,8 @@ def run_code_snippet_workflow(settings, db_manager, prompt_manager, langgraph): } } """ - + + # Initial workflow state initial_state = { "code": legacy_code, "context": None, @@ -66,58 +88,12 @@ def run_code_snippet_workflow(settings, db_manager, prompt_manager, langgraph): "code_review_results": None, "code_review_times": 0, "msgs": [], - "answer": None - } - - final_state = langgraph.invoke(initial_state) + "answer": None, - print(Fore.GREEN + f"\nAnalysis Complete!" + Style.RESET_ALL) - print(f"Final state keys: {list(final_state.keys())}") - print(f"Context retrieved: {'Yes' if final_state.get('context') else 'No'}") - print(f"Analysis completed: {'Yes' if final_state.get('antipatterns_scanner_results') else 'No'}") - print(f"Refactored code: {'Yes' if final_state.get('refactored_code') else 'No'}") - print(f"Code review results: {final_state.get('code_review_times')}") - - -def main(): - """Main function: Choose between code snippet analysis or full repository run""" - - print(Fore.BLUE + "=== AntiPattern Remediator Tool ===" + Style.RESET_ALL) - print("Choose your analysis mode:") - print("1) Code Snippet Analysis - Analyze a sample Java code snippet") - print("2) Full Repository Run - Process files with 100% test coverage from JaCoCo results") - - # Let user choose analysis mode - mode_choice = input("\nSelect mode (1-2): ").strip() - - if mode_choice not in ["1", "2"]: - print(Fore.RED + "Invalid choice. Defaulting to Code Snippet Analysis." + Style.RESET_ALL) - mode_choice = "1" - - # Let user select provider - print("\nAvailable providers: 1) ollama 2) ibm 3) vllm") - choice = input("Select provider (1-3): ").strip() - - provider_map = {"1": "ollama", "2": "ibm", "3": "vllm"} - provider = provider_map.get(choice, "ollama") # default to ollama - - # Let us choose which DB to interact with - print("Choose your trove: 1) ChromaDB (VectorDB) 2) TinyDB (DocumentDB)") - db_choice = input("Choose 1 or 2: ").strip() - - # Initialize global settings with selected provider - settings = initialize_settings(provider) - print(Fore.GREEN + f"Using {settings.LLM_PROVIDER} with model {settings.LLM_MODEL}" + Style.RESET_ALL) - - # Temporary Lazy Imports - from src.core.graph import CreateGraph - from src.data.database import VectorDBManager, TinyDBManager - from src.core.prompt import PromptManager - from scripts import seed_database - - # Initialize PromptManager - print("Initializing PromptManager...") - prompt_manager = PromptManager() + # ExplainerAgent fields + "explanation_response_raw": None, + "explanation_json": None, + } # Setup Database if db_choice == "2": @@ -133,11 +109,22 @@ def main(): retriever = db_manager.as_retriever() langgraph = CreateGraph(db_manager, prompt_manager, retriever=retriever).workflow - # Run the selected workflow - if mode_choice == "1": - run_code_snippet_workflow(settings, db_manager, prompt_manager, langgraph) + # Final results summary + print(Fore.GREEN + f"\nAnalysis Complete!" + Style.RESET_ALL) + print(f"Final state keys: {list(final_state.keys())}") + print(f"Context retrieved: {'Yes' if final_state.get('context') else 'No'}") + print(f"Analysis completed: {'Yes' if final_state.get('antipatterns_scanner_results') else 'No'}") + print(f"Refactored code: {'Yes' if final_state.get('refactored_code') else 'No'}") + print(f"Code review results: {final_state.get('code_review_times')}") + + # Show explanation from ExplainerAgent + if final_state.get("explanation_json"): + import json + print(Fore.CYAN + "\n=== Explanation (JSON) ===" + Style.RESET_ALL) + print(json.dumps(final_state["explanation_json"], indent=2, ensure_ascii=False)) else: - run_full_repo_workflow(settings, db_manager, prompt_manager, langgraph) + print(Fore.RED + "\nNo explanation was generated." + Style.RESET_ALL) + if __name__ == "__main__": main() diff --git a/AntiPattern_Remediator/src/core/agents/__init__.py b/AntiPattern_Remediator/src/core/agents/__init__.py index 79c1adc..d5eceaa 100644 --- a/AntiPattern_Remediator/src/core/agents/__init__.py +++ b/AntiPattern_Remediator/src/core/agents/__init__.py @@ -7,10 +7,12 @@ from .refactor_strategist import RefactorStrategist from .code_transformer import CodeTransformer from .code_reviewer import CodeReviewerAgent +from .explainer import ExplainerAgent __all__ = [ "AntipatternScanner", "RefactorStrategist", "CodeTransformer", - "CodeReviewerAgent" + "CodeReviewerAgent", + "ExplainerAgent" ] diff --git a/AntiPattern_Remediator/src/core/agents/explainer.py b/AntiPattern_Remediator/src/core/agents/explainer.py new file mode 100644 index 0000000..9344121 --- /dev/null +++ b/AntiPattern_Remediator/src/core/agents/explainer.py @@ -0,0 +1,86 @@ +""" +ExplainerAgent — minimal version +- Delegates state handling to create_graph.py +- Only builds messages and parses JSON response +- Keeps code minimal and focused +""" +from __future__ import annotations + +from typing import Dict, Any +import json + +from langchain_core.language_models import BaseLanguageModel +from ..prompt import PromptManager +from src.core.utils import extract_first_json + +PROMPT_KEY = "explainer" + + +class ExplainerAgent: + def __init__(self, llm: BaseLanguageModel, prompt_manager: PromptManager): + self.llm = llm + self.prompt_manager = prompt_manager + + def explain_antipattern(self, state: Dict[str, Any]) -> Dict[str, Any]: + """Generate explanation JSON for detected antipatterns and refactor.""" + kwargs = dict( + code=state.get("code", ""), + language=state.get("language", "Java"), + context=state.get("context", ""), + refactored_code=state.get("refactored_code", ""), + refactor_rationale=state.get("refactor_rationale", ""), + antipattern_name=state.get("antipattern_name", "Unknown antipattern"), + antipattern_description=state.get("antipattern_description", ""), + antipatterns_json=json.dumps(state.get("antipatterns_json", []), ensure_ascii=False), + ) + + messages = self._build_messages(**kwargs) + response = self.llm.invoke(messages) + raw = getattr(response, "content", None) or str(response) + state["explanation_response_raw"] = raw + + parsed = extract_first_json(raw) + state["explanation_json"] = parsed if isinstance(parsed, dict) else {} + return state + + def display_explanation(self, state: Dict[str, Any]) -> Dict[str, Any]: + print("\n=== Explanation (raw) ===\n", state.get("explanation_response_raw", "N/A")) + if state.get("explanation_json"): + print("\n=== Explanation (JSON) ===\n", json.dumps(state["explanation_json"], indent=2, ensure_ascii=False)) + return state + + def _build_messages(self, **kwargs) -> Any: + try: + getp = getattr(self.prompt_manager, "get_prompt", None) + if callable(getp): + prompt = getp(PROMPT_KEY) + if prompt is not None: + return prompt.format_messages(**kwargs) + except Exception: + pass + + schema = { + "items": [{ + "antipattern_name": "", + "antipattern_description": "", + "impact": "", + "why_it_is_bad": "", + "how_we_fixed_it": "", + "refactored_code": "", + "summary": "" + }], + "what_changed": [], + "why_better": [], + "principles_applied": [], + "trade_offs": [], + "closing_summary": "" + } + content = ( + "Given inputs (JSON):\n" + json.dumps(kwargs, ensure_ascii=False) + + "\nRespond with STRICT JSON using exactly this schema:\n" + + json.dumps(schema, ensure_ascii=False) + ) + return [ + {"role": "system", "content": "Return STRICT JSON only. No commentary."}, + {"role": "user", "content": content}, + ] diff --git a/AntiPattern_Remediator/src/core/graph/create_graph.py b/AntiPattern_Remediator/src/core/graph/create_graph.py index 9c4cf93..096d403 100644 --- a/AntiPattern_Remediator/src/core/graph/create_graph.py +++ b/AntiPattern_Remediator/src/core/graph/create_graph.py @@ -14,6 +14,7 @@ from ..agents import RefactorStrategist from ..agents import CodeTransformer from ..agents import CodeReviewerAgent +from ..agents import ExplainerAgent # Imports for LangSmith tracing import os @@ -62,7 +63,7 @@ def __init__(self, db_manager, prompt_manager: PromptManager, retriever=None, ll self.prompt_manager = prompt_manager self.conditional_edges = ConditionalEdges() - # ✅ assign the instance attribute before use + # assign the instance attribute before use self.retriever = retriever or self.db_manager.as_retriever() retriever_tool = create_retriever_tool( @@ -78,13 +79,13 @@ def __init__(self, db_manager, prompt_manager: PromptManager, retriever=None, ll "strategist": RefactorStrategist(self.llm, self.prompt_manager, retriever=self.retriever), "transformer": CodeTransformer(self.llm, self.prompt_manager), "reviewer": CodeReviewerAgent(self.llm, self.prompt_manager), + "explainer": ExplainerAgent(self.llm, self.prompt_manager) } # Build the LangGraph workflow self.workflow = self._build_graph() def _build_graph(self): - """Build LangGraph workflow""" graph = StateGraph(AgentState) # Scanner: retrieve + analyze @@ -104,6 +105,10 @@ def _build_graph(self): graph.add_node("review_code", self.agents["reviewer"].review_code) graph.add_node("display_code_review_results", self.agents["reviewer"].display_code_review_results) + # Explainer: final storytelling + graph.add_node("explain_antipattern", self.agents["explainer"].explain_antipattern) + graph.add_node("display_explanation", self.agents["explainer"].display_explanation) + # Topology graph.set_entry_point("retrieve_context") graph.add_edge("retrieve_context", "analyze_antipatterns") @@ -123,6 +128,8 @@ def _build_graph(self): }, ) - graph.add_edge("display_code_review_results", END) + graph.add_edge("display_code_review_results", "explain_antipattern") + graph.add_edge("explain_antipattern", "display_explanation") + graph.add_edge("display_explanation", END) return graph.compile() diff --git a/AntiPattern_Remediator/src/core/prompt/prompt_manager.py b/AntiPattern_Remediator/src/core/prompt/prompt_manager.py index 0650429..1f1abef 100644 --- a/AntiPattern_Remediator/src/core/prompt/prompt_manager.py +++ b/AntiPattern_Remediator/src/core/prompt/prompt_manager.py @@ -7,27 +7,26 @@ class PromptManager: """Manager for handling prompt templates and configurations.""" def __init__(self): - # Prompt key constants, **same as YAML filenames** + # Prompt key constants, **same as YAML filenames (without .yaml)** self.ANTIPATTERN_SCANNER = "antipattern_scanner" - self.REFACTOR_STRATEGIST = "refactor_strategist" + self.REFACTOR_STRATEGIST = "refactor_strategist" self.CODE_TRANSFORMER = "code_transformer" self.CODE_REVIEWER = "code_reviewer" + self.EXPLAINER_AGENT = "explainer" self.prompt_directory = settings.PROMPT_DIR - # Initialize storage for prompt templates self._prompt_cache = {} - # Load prompts on initialization self._load_all_prompts() def _load_all_prompts(self) -> None: """Load all prompt configurations from YAML files.""" try: - # Get all prompt constants and load corresponding files prompt_constants = [ self.ANTIPATTERN_SCANNER, self.REFACTOR_STRATEGIST, self.CODE_TRANSFORMER, self.CODE_REVIEWER, + self.EXPLAINER_AGENT, ] for prompt_key in prompt_constants: @@ -55,12 +54,17 @@ def _load_prompt_from_yaml(self, filename: str, prompt_key: str) -> None: return prompt_config = config[prompt_key] - # Create ChatPromptTemplate - self._prompt_cache[prompt_key] = ChatPromptTemplate([ - ("system", prompt_config.get('system', '')), - ("user", prompt_config.get('user', '')), - MessagesPlaceholder("msgs") - ]) + + # Build messages in (role, content) format + messages = [] + if prompt_config.get("system"): + messages.append(("system", prompt_config["system"])) + if prompt_config.get("user"): + messages.append(("user", prompt_config["user"])) + messages.append(MessagesPlaceholder("msgs")) + + # Use the correct constructor + self._prompt_cache[prompt_key] = ChatPromptTemplate.from_messages(messages) print(f"Loaded prompt '{prompt_key}' from {filename}") except Exception as e: @@ -70,5 +74,4 @@ def get_prompt(self, prompt_key: str) -> Optional[ChatPromptTemplate]: if prompt_key not in self._prompt_cache: print(f"Warning: Prompt '{prompt_key}' not found in cache") return None - - return self._prompt_cache[prompt_key] \ No newline at end of file + return self._prompt_cache[prompt_key] diff --git a/AntiPattern_Remediator/src/core/state.py b/AntiPattern_Remediator/src/core/state.py index fb42b13..a0b34f3 100644 --- a/AntiPattern_Remediator/src/core/state.py +++ b/AntiPattern_Remediator/src/core/state.py @@ -17,4 +17,6 @@ class AgentState(TypedDict): code_review_times: int # Number of times code has been reviewed msgs: List[Dict[str, Any]] # Message history for conversation context answer: Optional[str] # Analysis result + explanation_response_raw: Optional[str] # Raw LLM output from explainer + explanation_json: Optional[Dict[str, Any]] # Parsed JSON explanation current_file_path: Optional[str] # Path to the current file being processed diff --git a/AntiPattern_Remediator/src/core/utils/__init__.py b/AntiPattern_Remediator/src/core/utils/__init__.py new file mode 100644 index 0000000..ccba67c --- /dev/null +++ b/AntiPattern_Remediator/src/core/utils/__init__.py @@ -0,0 +1,3 @@ +from .json_utils import extract_first_json + +__all__ = ["extract_first_json"] \ No newline at end of file diff --git a/AntiPattern_Remediator/src/core/utils/json_utils.py b/AntiPattern_Remediator/src/core/utils/json_utils.py new file mode 100644 index 0000000..d2e0b2f --- /dev/null +++ b/AntiPattern_Remediator/src/core/utils/json_utils.py @@ -0,0 +1,27 @@ +import json + +def extract_first_json(text): + """ + Try to extract the first JSON object from a string. + Works if JSON is inside ```json ... ``` fences or just plain text. + """ + if not isinstance(text, str): + return None + + # 1. If the text has fenced JSON like ```json ... ``` + if "```" in text: + parts = text.split("```") + for part in parts: + # Look for JSON-specific fences + if part.strip().lower().startswith("json"): + json_part = part[len("json"):].strip() + try: + return json.loads(json_part) + except Exception: + pass # Try next part + + # 2. If no fenced JSON worked, try to parse the whole text + try: + return json.loads(text.strip()) + except Exception: + return None \ No newline at end of file diff --git a/AntiPattern_Remediator/static/prompt/explainer.yaml b/AntiPattern_Remediator/static/prompt/explainer.yaml new file mode 100644 index 0000000..8963f95 --- /dev/null +++ b/AntiPattern_Remediator/static/prompt/explainer.yaml @@ -0,0 +1,42 @@ +explainer: + template: | + You are a senior software reviewer. + Your job is to explain detected anti-patterns and the applied refactor in a clear, structured way. + Output STRICT JSON only — no commentary outside JSON. + + === Inputs === + Language: {language} + Context: {context} + Detected Anti-patterns (JSON): {antipatterns_json} + Code: + ```{code}``` + Refactored Code: + ```{refactored_code}``` + Refactor Rationale: + {refactor_rationale} + + === Required Output Schema === + { + "items": [ + { + "antipattern_name": "", + "antipattern_description": "", + "impact": "", + "why_it_is_bad": "", + "how_we_fixed_it": "", + "refactored_code": "", + "summary": "" + } + ], + "what_changed": [], + "why_better": [], + "principles_applied": [], + "trade_offs": [], + "closing_summary": "" + } + + Notes: + - Always return valid JSON. + - Use multiple entries under "items" if more than one antipattern is relevant. + - Keep `refactored_code` short (or truncated if needed). + - Fill all fields, even if briefly. diff --git a/AntiPattern_Remediator/static/tinydb.json b/AntiPattern_Remediator/static/tinydb.json index cd87283..8f6198a 100644 --- a/AntiPattern_Remediator/static/tinydb.json +++ b/AntiPattern_Remediator/static/tinydb.json @@ -1 +1 @@ -{"_default": {"1": {"name": "Deep Nesting", "description": "Deep Nesting occurs when conditional or loop blocks are embedded within one another across multiple levels, creating code with high indentation and complex control flow. While not always increasing cyclomatic complexity linearly, deep nesting significantly raises cognitive complexity (the mental effort required to understand, modify, and debug a method).", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Low readability : The \"arrowhead\" structure makes it hard to trace logic and understand the intended flow.\nHigh cognitive load : Developers must mentally track the conditions that lead to or prevent reaching a particular line of code.\nError-prone maintenance : Adding or modifying logic inside deeply nested blocks increases the risk of missing cases or introducing bugs.\nInhibited reuse and testing : Deep nesting often combines concerns that should be split into smaller, testable methods or units.\nPoor diffs in version control : Even small changes can alter indentation across many lines, making reviews harder.", "remediation": "Guard clauses (early return) : Exit early when preconditions fail, flattening the control flow\nExtract method : Isolate deeply nested blocks into private methods with clear names to separate concerns and reduce depth.\nInvet conditionals : Invert logic to return early or skip unnecessary branches.\nReplace nested loops with streams (Java-specific): Abstract common filtering or mapping logic into declarative operations.\nUse pattern matching : Replace layered `if` chains with clearer structural or type-based matching.\nEncapsulate state checks : Group multiple conditionals into intention-revealing boolean helpers or state objects.", "limitation": "Can conflict with existing code style : Teams unfamiliar with guard clauses may resist early exits or multiple returns.\nRefactoring can obscure logic during transition : Extracted methods must be clearly named to preserve readability and avoid confusion.\nNested logic may be unavoidable in rare edge cases : Complex parsing, state machines, or embedded domain-specific languages may naturally involve deeper control structures.", "type": "antipattern", "source_file": "deep_nesting.json"}, "2": {"name": "Generic Exception handling", "description": "Generic exception handling refers to the use of broad or unspecific catch blocks (catching but ignoring the exceptions). These patterns obscure the true source of errors, suppress the useful debugging information and can unintentionally hide critical failures. This anti-pattern often stems from the desire to keep the code running, but typically leads to weak systems and increased technical debt.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Loss of context : Catching high-level exceptions removes the granularity needed to understand specific failure reasons\nRepeated boilerplate : Developers may re-implement logging, default values, or stream-closing logic instead of using safe utility methods\nViolation of fail-fast principles : Silent or overly generic handling can let critical errors go unnoticed for too long", "remediation": "", "limitation": "Catching too many specific exceptions can bloat the code and reduce readability.\nRefactoring exception handling may require thorough testing to avoid regressions.\nOver-logging exceptions can clutter logs and obscure real issues.\nSecurity-sensitive applications may need specialised exception handling strategies to avoid leaks", "type": "antipattern", "source_file": "generic_exception_handling.json"}, "3": {"name": "Magic Constants", "description": "Magic constants (or magic numbers) are hard-coded literal values (e.g., 3.14, 42, \"admin\") that appear directly in code without context or explanation. These values become problematic when their purpose is unclear, undocumented, or reused inconsistently. While some literals (like 0, 1, or -1) may be self-explanatory in some contexts, others represent thresholds, identifiers, or rules that should be named and documented for clarity.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor readability : Developers must guess the meaning of the value, increasing cognitive load.\nLow maintainability : Changing the value requires updating it everywhere, risking inconsistent updates.\nHarder debugging : Literal values lack descriptive meaning in stack traces, logs, or debuggers.\nDuplication : The same literal used in multiple places leads to repeated logic and tighter coupling.\nViolation of DRY : Embeds implicit meaning multiple times without abstraction.", "remediation": "Define Constants - Move literal values into named constants (e.g., const, final, or static readonly).\nUse Enums - Group related constants (e.g., roles, statuses) as enumerations with meaningful names.", "limitation": "Extracting trivial values like 0 or 1 may clutter code and reduce clarity if overdone.\nIn performance-critical code, indirection through constants or functions may introduce slight overhead.\nOver-abstracting unnamed constants (e.g., MAXCOUNT3) may make code harder to understand.\nIf a value is used only once and is self-explanatory, extracting it may be unnecessary overhead.", "type": "antipattern", "source_file": "magic_constants.json"}, "4": {"name": "SRP Violation", "description": "The Single Responsibility Principle (SRP) is one of the SOLID principles of object-oriented design, stating that a class (or, at a lower level, a method) should have only one reason to change (i.e., it should have only one responsibility or concern). A SRP Violation occurs when a class (method) takes on multiple unrelated responsibilities, making it harder to maintain, test, and understand. These violations often result in bloated classes or methods that mix unrelated concerns such as I/O, business logic, error handling, and configuration.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor maintainability : Changing one responsibility might inadvertently affect others, introducing bugs.\nLow cohesion : Code with unrelated responsibilities lacks a clear purpose, reducing clarity and reusability.\nDifficult testing : Unit testing becomes more complex as setup may require mocking or initializing unrelated dependencies.\nCode duplication and tight coupling : Responsibilities are harder to reuse or share, often leading to repeated logic or tight inter-class dependencies.\nHarder onboarding : New developers struggle to understand the purpose and scope of large, multi-purpose classes.", "remediation": "Extract Method \\- Split complex methods into smaller, single-purpose private methods.\nEarly Return (Guard Clauses) \\- Use early exits to reduce nested logic and clarify separate responsibilities.\nUse Local Functions/Lambdas \\- Encapsulate small inline logic into local functions for clarity.\nEncapsulate Temporary Variables \\- Move logic-heavy expressions into descriptive helper methods.\nGroup Related Logic \\- Cluster related operations into distinct helper methods within the same class.\nSeparate Concerns in Loops \\- Extract filtering, transforming, and aggregating into distinct steps.\nIsolate Logging/Error Handling \\- Move side-effect code like logging into dedicated private methods (unless the logging is a trivial single line and does not obscure business logic).", "limitation": "Fixing certain SRP violations (especially class-level violations) requires changing public method signatures, creating new classes, and/or breaking interfaces.\nMethods often depend on multiple injected services or shared state; untangling responsibilities might require broader architectural changes.\nLogging, error handling, metrics, and security checks are often scattered across responsibilities and difficult to isolate cleanly at the method level without aspect-oriented programming (AOP) or middleware/interceptor patterns.", "type": "antipattern", "source_file": "srp_violation.json"}, "5": {"name": "Unsafe or Vague Exception Handling", "description": "Reliable exception handling, type safety, and controlled flow are essential forr writing maintainable and robust software. Unsafe or Vague exception handling often results in code that is fragile, difficult to test, and challenging to debug. Instead of providing meaningful error handling or clear separation of concerns, these implementations either hide the underlying problem or use language features in a way that breaks maintainability.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Hidden failures : Catching exceptions without logging or meaningful handling hides the source of issues and makes debugging difficult\nPoor diagnosis : Broad exception handling with vague error messages hides intent and makes it harder to trace the root cause\nInconsistent runtime behaviour : using assertions for control logic can lead to unpredictable behaviour depending on the JVM configuration\nUncontrolled termination : Using system.exit() directly in application logic makes code untestable and prevents proper resource cleanup", "remediation": "Replace assertions with proper condition checks and informative exceptions to ensure consistent behaviour across environments\nAvoid silent catch blocks, log exceptions or rethrow them to preserve error context\nHandle specific exception types instead of catching broad categories like Exception or RuntimeException\nRefactor abrupt shutdown calls into controlled exits using exception handling or return code to support recovery", "limitation": "", "type": "antipattern", "source_file": "unsafe_or_vague_exception_handling.json"}}} \ No newline at end of file +{"_default": {"1": {"name": "Monolithic Method", "description": "A Monolithic Method is a single method that tries to do too much, often combining unrelated responsibilities into one block of code. This is essentially a method-level violation of the Single Responsibility Principle (SRP). Monolithic methods are hard to read, understand, maintain, and test because they mix business logic, I/O, error handling, and other concerns in one place.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor maintainability : Modifying one part of the method risks breaking unrelated functionality.\nLow readability : Long, complex methods are hard to follow and understand.\nDifficult testing : Unit tests become cumbersome because the method does too many things at once.\nCode duplication : Reusing logic is difficult; similar tasks often get reimplemented elsewhere.\nTight coupling : Internal details are intertwined, making refactoring risky.", "remediation": "", "limitation": "Refactoring may require changes to public method signatures, especially if other code depends on it.\nSome logic may rely on shared state or multiple services, making it hard to separate without broader architectural changes.\nIf the method handles cross-cutting concerns (e.g., logging, metrics, validation), isolating responsibilities may require AOP or middleware.", "type": "antipattern", "source_file": "monolithic_method.json"}, "2": {"name": "Unsafe or Vague Exception Handling", "description": "Reliable exception handling, type safety, and controlled flow are essential forr writing maintainable and robust software. Unsafe or Vague exception handling often results in code that is fragile, difficult to test, and challenging to debug. Instead of providing meaningful error handling or clear separation of concerns, these implementations either hide the underlying problem or use language features in a way that breaks maintainability.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Hidden failures : Catching exceptions without logging or meaningful handling hides the source of issues and makes debugging difficult\nPoor diagnosis : Broad exception handling with vague error messages hides intent and makes it harder to trace the root cause\nInconsistent runtime behaviour : using assertions for control logic can lead to unpredictable behaviour depending on the JVM configuration\nUncontrolled termination : Using system.exit() directly in application logic makes code untestable and prevents proper resource cleanup", "remediation": "Replace assertions with proper condition checks and informative exceptions to ensure consistent behaviour across environments\nAvoid silent catch blocks, log exceptions or rethrow them to preserve error context\nHandle specific exception types instead of catching broad categories like Exception or RuntimeException\nRefactor abrupt shutdown calls into controlled exits using exception handling or return code to support recovery", "limitation": "", "type": "antipattern", "source_file": "unsafe_or_vague_exception_handling.json"}, "3": {"name": "Deep Nesting", "description": "Deep Nesting occurs when conditional or loop blocks are embedded within one another across multiple levels, creating code with high indentation and complex control flow. While not always increasing cyclomatic complexity linearly, deep nesting significantly raises cognitive complexity (the mental effort required to understand, modify, and debug a method).", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Low readability : The \"arrowhead\" structure makes it hard to trace logic and understand the intended flow.\nHigh cognitive load : Developers must mentally track the conditions that lead to or prevent reaching a particular line of code.\nError-prone maintenance : Adding or modifying logic inside deeply nested blocks increases the risk of missing cases or introducing bugs.\nInhibited reuse and testing : Deep nesting often combines concerns that should be split into smaller, testable methods or units.\nPoor diffs in version control : Even small changes can alter indentation across many lines, making reviews harder.", "remediation": "Guard clauses (early return) : Exit early when preconditions fail, flattening the control flow\nExtract method : Isolate deeply nested blocks into private methods with clear names to separate concerns and reduce depth.\nInvet conditionals : Invert logic to return early or skip unnecessary branches.\nReplace nested loops with streams (Java-specific): Abstract common filtering or mapping logic into declarative operations.\nUse pattern matching : Replace layered `if` chains with clearer structural or type-based matching.\nEncapsulate state checks : Group multiple conditionals into intention-revealing boolean helpers or state objects.", "limitation": "Can conflict with existing code style : Teams unfamiliar with guard clauses may resist early exits or multiple returns.\nRefactoring can obscure logic during transition : Extracted methods must be clearly named to preserve readability and avoid confusion.\nNested logic may be unavoidable in rare edge cases : Complex parsing, state machines, or embedded domain-specific languages may naturally involve deeper control structures.", "type": "antipattern", "source_file": "deep_nesting.json"}, "4": {"name": "SRP Violation", "description": "The Single Responsibility Principle (SRP) is one of the SOLID principles of object-oriented design, stating that a class (or, at a lower level, a method) should have only one reason to change (i.e., it should have only one responsibility or concern). A SRP Violation occurs when a class (method) takes on multiple unrelated responsibilities, making it harder to maintain, test, and understand. These violations often result in bloated classes or methods that mix unrelated concerns such as I/O, business logic, error handling, and configuration.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor maintainability : Changing one responsibility might inadvertently affect others, introducing bugs.\nLow cohesion : Code with unrelated responsibilities lacks a clear purpose, reducing clarity and reusability.\nDifficult testing : Unit testing becomes more complex as setup may require mocking or initializing unrelated dependencies.\nCode duplication and tight coupling : Responsibilities are harder to reuse or share, often leading to repeated logic or tight inter-class dependencies.\nHarder onboarding : New developers struggle to understand the purpose and scope of large, multi-purpose classes.", "remediation": "Extract Method \\- Split complex methods into smaller, single-purpose private methods.\nEarly Return (Guard Clauses) \\- Use early exits to reduce nested logic and clarify separate responsibilities.\nUse Local Functions/Lambdas \\- Encapsulate small inline logic into local functions for clarity.\nEncapsulate Temporary Variables \\- Move logic-heavy expressions into descriptive helper methods.\nGroup Related Logic \\- Cluster related operations into distinct helper methods within the same class.\nSeparate Concerns in Loops \\- Extract filtering, transforming, and aggregating into distinct steps.\nIsolate Logging/Error Handling \\- Move side-effect code like logging into dedicated private methods (unless the logging is a trivial single line and does not obscure business logic).", "limitation": "Fixing certain SRP violations (especially class-level violations) requires changing public method signatures, creating new classes, and/or breaking interfaces.\nMethods often depend on multiple injected services or shared state; untangling responsibilities might require broader architectural changes.\nLogging, error handling, metrics, and security checks are often scattered across responsibilities and difficult to isolate cleanly at the method level without aspect-oriented programming (AOP) or middleware/interceptor patterns.", "type": "antipattern", "source_file": "srp_violation.json"}, "5": {"name": "Magic Constants", "description": "Magic constants (or magic numbers) are hard-coded literal values (e.g., 3.14, 42, \"admin\") that appear directly in code without context or explanation. These values become problematic when their purpose is unclear, undocumented, or reused inconsistently. While some literals (like 0, 1, or -1) may be self-explanatory in some contexts, others represent thresholds, identifiers, or rules that should be named and documented for clarity.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor readability : Developers must guess the meaning of the value, increasing cognitive load.\nLow maintainability : Changing the value requires updating it everywhere, risking inconsistent updates.\nHarder debugging : Literal values lack descriptive meaning in stack traces, logs, or debuggers.\nDuplication : The same literal used in multiple places leads to repeated logic and tighter coupling.\nViolation of DRY : Embeds implicit meaning multiple times without abstraction.", "remediation": "Define Constants - Move literal values into named constants (e.g., const, final, or static readonly).\nUse Enums - Group related constants (e.g., roles, statuses) as enumerations with meaningful names.", "limitation": "Extracting trivial values like 0 or 1 may clutter code and reduce clarity if overdone.\nIn performance-critical code, indirection through constants or functions may introduce slight overhead.\nOver-abstracting unnamed constants (e.g., MAXCOUNT3) may make code harder to understand.\nIf a value is used only once and is self-explanatory, extracting it may be unnecessary overhead.", "type": "antipattern", "source_file": "magic_constants.json"}, "6": {"name": "Duplicate Code", "description": "Duplicate Code occurs when identical or very similar code blocks are repeated throughout the codebase. This pattern creates maintenance overhead, increases the likelihood of bugs, and violates the DRY (Don't Repeat Yourself) principle. Common examples include repeated null checks, validation logic, and similar conditional patterns across different methods or classes.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Maintenance overhead : Changes need to be applied in multiple places, increasing the risk of inconsistencies\nBug multiplication : A bug in duplicated code affects multiple locations, making fixes more complex\nCode bloat : Repeated code increases the overall size of the codebase without adding functionality\nViolation of DRY principle : Makes the code harder to understand and reason about", "remediation": "", "limitation": "Over-abstraction can make code harder to understand if the duplication is minimal or contextually different\nPremature extraction of methods may create unnecessary coupling between unrelated parts of the system\nSome duplication might be acceptable if the code serves different business contexts", "type": "antipattern", "source_file": "duplicate_code.json"}, "7": {"name": "God Class", "description": "A God Class anti-pattern refers to a class that centralises too many responsibilities in a single location, becoming overly complex and difficult to maintain. Such classes tend to know too much, do too much, and interact with many different parts of the system. This leads to tightly coupled code, reduces modularity, and makes the system hard to test and extend.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "High coupling: God Classes tend to interact with many other classes and modules, thereby reducing system modularity and increasing the risk of changes.\nPoor maintainability : Large, complex classes are difficult to read, understand, and modify.\nLow reusability : The class becomes so specific and bloated that it is rarely useful outside of its original context.\nHidden dependencies : God Classes often hide dependencies within fields or methods, making the codebase less transparent.", "remediation": "", "limitation": "", "type": "antipattern", "source_file": "god_class.json"}, "8": {"name": "Middle Man", "description": "The Middle Man anti-pattern occurs when a class exists primarily to delegate calls to another class without adding meaningful logic of its own. Essentially, the class acts as a pass-through or proxy, forwarding method calls without adding value. While delegation is sometimes necessary for abstraction, excessive or trivial delegation leads to unnecessary indirection and increases maintenance overhead.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Extra Indirection : Code must go through one more layer, which can complicate understanding the code flow.\nIncreased maintenance : When the delegated class changes, the middle-man class often needs updates for all its pass-through methods.\nLow cohesion : The middle-man class has little real logic, making its purpose unclear\nCode bloat : Many trivial delegation methods clutter the class, making it harder to navigate.\nHarder debugging : Tracing behaviour through multiple layers of delegation adds cognitive load.", "remediation": "", "limitation": "Some delegation is unavoidable, e.g., to implement an interface or provide a stable abstraction layer\nRemoving middleman classes may break existing APIs or require refactoring client code.\nIn some cases, delegation is part of a design pattern (like a proxy), which is intentional and not an anti-pattern", "type": "antipattern", "source_file": "middle_man.json"}, "9": {"name": "Generic Exception handling", "description": "Generic exception handling refers to the use of broad or unspecific catch blocks (catching but ignoring the exceptions). These patterns obscure the true source of errors, suppress the useful debugging information and can unintentionally hide critical failures. This anti-pattern often stems from the desire to keep the code running, but typically leads to weak systems and increased technical debt.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Loss of context : Catching high-level exceptions removes the granularity needed to understand specific failure reasons\nRepeated boilerplate : Developers may re-implement logging, default values, or stream-closing logic instead of using safe utility methods\nViolation of fail-fast principles : Silent or overly generic handling can let critical errors go unnoticed for too long", "remediation": "", "limitation": "Catching too many specific exceptions can bloat the code and reduce readability.\nRefactoring exception handling may require thorough testing to avoid regressions.\nOver-logging exceptions can clutter logs and obscure real issues.\nSecurity-sensitive applications may need specialised exception handling strategies to avoid leaks", "type": "antipattern", "source_file": "generic_exception_handling.json"}}} \ No newline at end of file From 1003a4f0e8b2e541c42d37e79c4318fc43d3b91f Mon Sep 17 00:00:00 2001 From: Avinash Date: Sat, 23 Aug 2025 10:36:01 +0100 Subject: [PATCH 5/7] Unit test updated for explainer --- .../src/core/prompt/prompt_manager.py | 31 ++++++++++--------- .../unit_test/prompt/test_prompt_manager.py | 20 ++++++++++-- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/AntiPattern_Remediator/src/core/prompt/prompt_manager.py b/AntiPattern_Remediator/src/core/prompt/prompt_manager.py index 1f1abef..d7fc7dd 100644 --- a/AntiPattern_Remediator/src/core/prompt/prompt_manager.py +++ b/AntiPattern_Remediator/src/core/prompt/prompt_manager.py @@ -41,34 +41,37 @@ def _load_all_prompts(self) -> None: def _load_prompt_from_yaml(self, filename: str, prompt_key: str) -> None: """Load a prompt configuration from a YAML file.""" yaml_path = self.prompt_directory / filename - + if not yaml_path.exists(): print(f"Warning: Prompt file {yaml_path} not found") return - + try: with open(yaml_path, 'r', encoding='utf-8') as file: config = yaml.safe_load(file) - if prompt_key not in config: + + if not config or prompt_key not in config: print(f"Warning: Section '{prompt_key}' not found in {filename}") return - - prompt_config = config[prompt_key] - # Build messages in (role, content) format - messages = [] - if prompt_config.get("system"): - messages.append(("system", prompt_config["system"])) - if prompt_config.get("user"): - messages.append(("user", prompt_config["user"])) - messages.append(MessagesPlaceholder("msgs")) + prompt_config = config.get(prompt_key) or {} + + # Always include System first (empty string if not provided) to satisfy tests + system_text = str(prompt_config.get("system", "") or "") + user_text = str(prompt_config.get("user", "") or "") + + messages = [ + ("system", system_text), # always present (possibly empty) + ("user", user_text), # always present (possibly empty) + MessagesPlaceholder("msgs"), # conversation history + ] - # Use the correct constructor self._prompt_cache[prompt_key] = ChatPromptTemplate.from_messages(messages) print(f"Loaded prompt '{prompt_key}' from {filename}") - + except Exception as e: print(f"Error loading prompt from {filename}: {e}") + def get_prompt(self, prompt_key: str) -> Optional[ChatPromptTemplate]: if prompt_key not in self._prompt_cache: diff --git a/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py b/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py index 088af2f..6e859c4 100644 --- a/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py +++ b/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py @@ -44,6 +44,7 @@ def __init__(self): self.REFACTOR_STRATEGIST = "refactor_strategist" self.CODE_TRANSFORMER = "code_transformer" self.CODE_REVIEWER = "code_reviewer" + self.EXPLAINER_AGENT = "explainer" self.prompt_directory = Path(__file__).parent self._prompt_cache = {} self._load_all_prompts() # Call this to match real behavior @@ -87,7 +88,8 @@ def _load_all_prompts(self): self.ANTIPATTERN_SCANNER, self.REFACTOR_STRATEGIST, self.CODE_TRANSFORMER, - self.CODE_REVIEWER + self.CODE_REVIEWER, + self.EXPLAINER_AGENT, ] for prompt_key in prompt_constants: @@ -155,6 +157,7 @@ def test_initialization_creates_correct_attributes(self): assert hasattr(manager, 'REFACTOR_STRATEGIST') assert hasattr(manager, 'CODE_TRANSFORMER') assert hasattr(manager, 'CODE_REVIEWER') + assert hasattr(manager, 'EXPLAINER_AGENT') assert hasattr(manager, 'prompt_directory') assert hasattr(manager, '_prompt_cache') assert isinstance(manager._prompt_cache, dict) @@ -176,6 +179,7 @@ def test_prompt_constants_have_correct_values(self): assert manager.REFACTOR_STRATEGIST == "refactor_strategist" assert manager.CODE_TRANSFORMER == "code_transformer" assert manager.CODE_REVIEWER == "code_reviewer" + assert manager.EXPLAINER_AGENT == "explainer" def test_prompt_directory_is_set_correctly(self): """Test that prompt directory is assigned from settings.""" @@ -505,6 +509,7 @@ def test_initialization_with_missing_directory(self, capsys): manager.REFACTOR_STRATEGIST = "refactor_strategist" manager.CODE_TRANSFORMER = "code_transformer" manager.CODE_REVIEWER = "code_reviewer" + manager.EXPLAINER_AGENT = "explainer" manager.prompt_directory = Path("/non/existent/path") manager._prompt_cache = {} @@ -575,6 +580,12 @@ def temp_prompt_files(self): 'system': 'You are an expert code reviewer.', 'user': 'Review this code for quality and best practices: {code}' } + }, + 'explainer.yaml': { + 'explainer': { + 'system': 'You are a senior software reviewer.', + 'user': 'Explain: {code}\nLang: {language}\nCtx: {context}' + } } } @@ -593,6 +604,7 @@ def test_load_all_prompts_loads_all_available_files(self, temp_prompt_files, cap manager.REFACTOR_STRATEGIST = "refactor_strategist" manager.CODE_TRANSFORMER = "code_transformer" manager.CODE_REVIEWER = "code_reviewer" + manager.EXPLAINER_AGENT = "explainer" manager.prompt_directory = temp_prompt_files manager._prompt_cache = {} @@ -600,15 +612,16 @@ def test_load_all_prompts_loads_all_available_files(self, temp_prompt_files, cap manager._load_all_prompts() # Assert: Verify all prompts were loaded - assert len(manager._prompt_cache) == 4 + assert len(manager._prompt_cache) == 5 assert "antipattern_scanner" in manager._prompt_cache assert "refactor_strategist" in manager._prompt_cache assert "code_transformer" in manager._prompt_cache assert "code_reviewer" in manager._prompt_cache + assert "explainer" in manager._prompt_cache # Verify success message captured = capsys.readouterr() - assert "Successfully loaded 4 prompts" in captured.out + assert "Successfully loaded 5 prompts" in captured.out def test_load_all_prompts_handles_partial_failures(self, capsys): """Test that _load_all_prompts continues loading even if some files are missing.""" @@ -633,6 +646,7 @@ def test_load_all_prompts_handles_partial_failures(self, capsys): manager.REFACTOR_STRATEGIST = "refactor_strategist" manager.CODE_TRANSFORMER = "code_transformer" manager.CODE_REVIEWER = "code_reviewer" + manager.EXPLAINER_AGENT = "explainer" manager.prompt_directory = temp_path manager._prompt_cache = {} From 69c00d0aae7f10bef8e07d8109237a41a53b0a40 Mon Sep 17 00:00:00 2001 From: Avinash Date: Wed, 27 Aug 2025 10:16:54 +0100 Subject: [PATCH 6/7] Final changes --- .../src/core/agents/explainer.py | 85 ++++++++++++++----- AntiPattern_Remediator/src/core/state.py | 4 + .../static/prompt/explainer.yaml | 27 +++--- .../unit_test/prompt/test_prompt_manager.py | 2 +- 4 files changed, 83 insertions(+), 35 deletions(-) diff --git a/AntiPattern_Remediator/src/core/agents/explainer.py b/AntiPattern_Remediator/src/core/agents/explainer.py index 9344121..4ab86a6 100644 --- a/AntiPattern_Remediator/src/core/agents/explainer.py +++ b/AntiPattern_Remediator/src/core/agents/explainer.py @@ -1,8 +1,8 @@ """ ExplainerAgent — minimal version - Delegates state handling to create_graph.py -- Only builds messages and parses JSON response -- Keeps code minimal and focused +- Uses PromptManager if available; otherwise a tiny inline fallback prompt +- Always passes msgs; always returns a non-empty explanation_json """ from __future__ import annotations @@ -10,6 +10,7 @@ import json from langchain_core.language_models import BaseLanguageModel +from langchain.prompts import ChatPromptTemplate, MessagesPlaceholder from ..prompt import PromptManager from src.core.utils import extract_first_json @@ -32,33 +33,55 @@ def explain_antipattern(self, state: Dict[str, Any]) -> Dict[str, Any]: antipattern_name=state.get("antipattern_name", "Unknown antipattern"), antipattern_description=state.get("antipattern_description", ""), antipatterns_json=json.dumps(state.get("antipatterns_json", []), ensure_ascii=False), + msgs=state.get("msgs", []), # ensure MessagesPlaceholder is satisfied ) messages = self._build_messages(**kwargs) - response = self.llm.invoke(messages) - raw = getattr(response, "content", None) or str(response) + + try: + response = self.llm.invoke(messages) + raw = getattr(response, "content", None) or str(response) + except Exception as e: + raw = f"LLM error: {e}" + state["explanation_response_raw"] = raw - parsed = extract_first_json(raw) - state["explanation_json"] = parsed if isinstance(parsed, dict) else {} + # Robust parse: accept dict, wrap list, or emit a minimal fallback + try: + parsed = extract_first_json(raw) + except Exception: + parsed = None + + if isinstance(parsed, dict): + state["explanation_json"] = parsed + elif isinstance(parsed, list): + state["explanation_json"] = {"items": parsed} + else: + state["explanation_json"] = self._fallback_payload(state) + return state def display_explanation(self, state: Dict[str, Any]) -> Dict[str, Any]: print("\n=== Explanation (raw) ===\n", state.get("explanation_response_raw", "N/A")) if state.get("explanation_json"): - print("\n=== Explanation (JSON) ===\n", json.dumps(state["explanation_json"], indent=2, ensure_ascii=False)) + print("\n=== Explanation (JSON) ===\n", + json.dumps(state["explanation_json"], indent=2, ensure_ascii=False)) return state def _build_messages(self, **kwargs) -> Any: - try: - getp = getattr(self.prompt_manager, "get_prompt", None) - if callable(getp): - prompt = getp(PROMPT_KEY) - if prompt is not None: - return prompt.format_messages(**kwargs) - except Exception: - pass + # Always ensure msgs exists + if "msgs" not in kwargs or kwargs["msgs"] is None: + kwargs = {**kwargs, "msgs": []} + # 1) Try preloaded template from PromptManager + prompt = None + getp = getattr(self.prompt_manager, "get_prompt", None) + if callable(getp): + prompt = getp(PROMPT_KEY) + if prompt is not None: + return prompt.format_messages(**kwargs) + + # 2) Minimal inline fallback schema = { "items": [{ "antipattern_name": "", @@ -76,11 +99,33 @@ def _build_messages(self, **kwargs) -> Any: "closing_summary": "" } content = ( - "Given inputs (JSON):\n" + json.dumps(kwargs, ensure_ascii=False) + + "Given inputs (JSON):\n" + json.dumps({k: v for k, v in kwargs.items() if k != "msgs"}, ensure_ascii=False) + "\nRespond with STRICT JSON using exactly this schema:\n" + json.dumps(schema, ensure_ascii=False) ) - return [ - {"role": "system", "content": "Return STRICT JSON only. No commentary."}, - {"role": "user", "content": content}, - ] + fallback = ChatPromptTemplate.from_messages([ + ("system", "Return STRICT JSON only. No commentary."), + ("user", content), + MessagesPlaceholder("msgs"), + ]) + return fallback.format_messages(**kwargs) + + @staticmethod + def _fallback_payload(state: Dict[str, Any]) -> Dict[str, Any]: + """Tiny fallback so downstream never breaks if parsing fails.""" + return { + "items": [{ + "antipattern_name": state.get("antipattern_name", "Unknown antipattern"), + "antipattern_description": state.get("antipattern_description", ""), + "impact": "", + "why_it_is_bad": "", + "how_we_fixed_it": state.get("refactor_rationale", ""), + "refactored_code": state.get("refactored_code", ""), + "summary": "Auto-generated minimal explanation (parser fallback)." + }], + "what_changed": [], + "why_better": [], + "principles_applied": [], + "trade_offs": [], + "closing_summary": "" + } diff --git a/AntiPattern_Remediator/src/core/state.py b/AntiPattern_Remediator/src/core/state.py index a0b34f3..531cfb1 100644 --- a/AntiPattern_Remediator/src/core/state.py +++ b/AntiPattern_Remediator/src/core/state.py @@ -8,6 +8,7 @@ class AgentState(TypedDict): """State definition for passing data through the workflow""" code: str # Code to be analyzed + language: Optional[str] # Language of the code (used by ExplainerAgent) context: Optional[str] # Context retrieved from knowledge base (scanner) trove_context: Optional[str] # Context retrieved from the Anti-Pattern Trove (TinyDB/Chroma) antipatterns_scanner_results: Optional[str] @@ -20,3 +21,6 @@ class AgentState(TypedDict): explanation_response_raw: Optional[str] # Raw LLM output from explainer explanation_json: Optional[Dict[str, Any]] # Parsed JSON explanation current_file_path: Optional[str] # Path to the current file being processed + + explanation_response_raw: Optional[str] # Raw LLM output from explainer + explanation_json: Optional[Dict[str, Any]] # Parsed JSON explanation diff --git a/AntiPattern_Remediator/static/prompt/explainer.yaml b/AntiPattern_Remediator/static/prompt/explainer.yaml index 8963f95..be29ab7 100644 --- a/AntiPattern_Remediator/static/prompt/explainer.yaml +++ b/AntiPattern_Remediator/static/prompt/explainer.yaml @@ -1,9 +1,8 @@ explainer: - template: | - You are a senior software reviewer. - Your job is to explain detected anti-patterns and the applied refactor in a clear, structured way. - Output STRICT JSON only — no commentary outside JSON. + system: | + You are a senior software reviewer. Output STRICT JSON only — no commentary outside the JSON object. + user: | === Inputs === Language: {language} Context: {context} @@ -15,10 +14,10 @@ explainer: Refactor Rationale: {refactor_rationale} - === Required Output Schema === - { + === Return EXACTLY this JSON structure === + {{ "items": [ - { + {{ "antipattern_name": "", "antipattern_description": "", "impact": "", @@ -26,17 +25,17 @@ explainer: "how_we_fixed_it": "", "refactored_code": "", "summary": "" - } + }} ], "what_changed": [], "why_better": [], "principles_applied": [], "trade_offs": [], "closing_summary": "" - } + }} - Notes: - - Always return valid JSON. - - Use multiple entries under "items" if more than one antipattern is relevant. - - Keep `refactored_code` short (or truncated if needed). - - Fill all fields, even if briefly. + Rules: + - Return valid JSON only (no code fences, no prose). + - If multiple antipatterns apply, include multiple entries in "items". + - Keep "refactored_code" concise; truncate if needed but keep it valid. + - Populate all fields; use brief placeholders if unsure. diff --git a/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py b/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py index 6e859c4..1fb5dec 100644 --- a/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py +++ b/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py @@ -583,7 +583,7 @@ def temp_prompt_files(self): }, 'explainer.yaml': { 'explainer': { - 'system': 'You are a senior software reviewer.', + 'system': 'You are a senior software code explainer.', 'user': 'Explain: {code}\nLang: {language}\nCtx: {context}' } } From e4c364aba3894061643f765ca3a2a18c90f31c24 Mon Sep 17 00:00:00 2001 From: Avinash Mallick Date: Wed, 27 Aug 2025 10:58:10 +0100 Subject: [PATCH 7/7] Revert "Ibm 21 explainer agent" --- AntiPattern_Remediator/main.py | 114 ++++++++------- .../src/core/agents/__init__.py | 4 +- .../src/core/agents/explainer.py | 131 ------------------ .../src/core/graph/create_graph.py | 13 +- .../src/core/prompt/prompt_manager.py | 44 +++--- AntiPattern_Remediator/src/core/state.py | 20 ++- .../src/core/utils/__init__.py | 3 - .../src/core/utils/json_utils.py | 27 ---- .../static/prompt/explainer.yaml | 41 ------ AntiPattern_Remediator/static/tinydb.json | 2 +- .../unit_test/prompt/test_prompt_manager.py | 20 +-- 11 files changed, 98 insertions(+), 321 deletions(-) delete mode 100644 AntiPattern_Remediator/src/core/agents/explainer.py delete mode 100644 AntiPattern_Remediator/src/core/utils/__init__.py delete mode 100644 AntiPattern_Remediator/src/core/utils/json_utils.py delete mode 100644 AntiPattern_Remediator/static/prompt/explainer.yaml diff --git a/AntiPattern_Remediator/main.py b/AntiPattern_Remediator/main.py index e5f52aa..a659983 100644 --- a/AntiPattern_Remediator/main.py +++ b/AntiPattern_Remediator/main.py @@ -1,7 +1,9 @@ + """ Main entry point - Legacy Code Migration Tool """ from config.settings import initialize_settings +# from scripts import seed_database from dotenv import load_dotenv load_dotenv() from colorama import Fore, Style @@ -10,35 +12,11 @@ from full_repo_workflow import run_full_repo_workflow - -def main(): - """Main function: Run antipattern analysis""" - - # Let user select provider - print("Available providers: 1) ollama 2) ibm 3) vllm") - choice = input("Select provider (1-3): ").strip() - - provider_map = {"1": "ollama", "2": "ibm", "3": "vllm"} - provider = provider_map.get(choice, "ollama") # default to ollama - - # Let us choose which DB to interact with - print("Choose your trove: 1) ChromaDB (VectorDB) 2) TinyDB (DocumentDB)") - db_choice = input("Choose 1 or 2: ").strip() - - # Initialize global settings with selected provider - settings = initialize_settings(provider) - print(Fore.GREEN + f"Using {settings.LLM_PROVIDER} with model {settings.LLM_MODEL}" + Style.RESET_ALL) - - # Temporary Lazy Imports - from src.core.graph import CreateGraph - from src.data.database import VectorDBManager, TinyDBManager - from src.core.prompt import PromptManager - from scripts import seed_database - - # Initialize PromptManager - print("Initializing PromptManager...") - prompt_manager = PromptManager() - +def run_code_snippet_workflow(settings, db_manager, prompt_manager, langgraph): + """Run the original workflow with a hardcoded Java code snippet.""" + print(Fore.BLUE + "\n=== Code Snippet Analysis Workflow ===" + Style.RESET_ALL) + print("Analyzing the provided Java code snippet...") + # Example Java code legacy_code = """ public class ApplicationManager { @@ -77,8 +55,7 @@ def main(): } } """ - - # Initial workflow state + initial_state = { "code": legacy_code, "context": None, @@ -89,13 +66,59 @@ def main(): "code_review_results": None, "code_review_times": 0, "msgs": [], - "answer": None, - - # ExplainerAgent fields - "explanation_response_raw": None, - "explanation_json": None, + "answer": None } + final_state = langgraph.invoke(initial_state) + + print(Fore.GREEN + f"\nAnalysis Complete!" + Style.RESET_ALL) + print(f"Final state keys: {list(final_state.keys())}") + print(f"Context retrieved: {'Yes' if final_state.get('context') else 'No'}") + print(f"Analysis completed: {'Yes' if final_state.get('antipatterns_scanner_results') else 'No'}") + print(f"Refactored code: {'Yes' if final_state.get('refactored_code') else 'No'}") + print(f"Code review results: {final_state.get('code_review_times')}") + + +def main(): + """Main function: Choose between code snippet analysis or full repository run""" + + print(Fore.BLUE + "=== AntiPattern Remediator Tool ===" + Style.RESET_ALL) + print("Choose your analysis mode:") + print("1) Code Snippet Analysis - Analyze a sample Java code snippet") + print("2) Full Repository Run - Process files with 100% test coverage from JaCoCo results") + + # Let user choose analysis mode + mode_choice = input("\nSelect mode (1-2): ").strip() + + if mode_choice not in ["1", "2"]: + print(Fore.RED + "Invalid choice. Defaulting to Code Snippet Analysis." + Style.RESET_ALL) + mode_choice = "1" + + # Let user select provider + print("\nAvailable providers: 1) ollama 2) ibm 3) vllm") + choice = input("Select provider (1-3): ").strip() + + provider_map = {"1": "ollama", "2": "ibm", "3": "vllm"} + provider = provider_map.get(choice, "ollama") # default to ollama + + # Let us choose which DB to interact with + print("Choose your trove: 1) ChromaDB (VectorDB) 2) TinyDB (DocumentDB)") + db_choice = input("Choose 1 or 2: ").strip() + + # Initialize global settings with selected provider + settings = initialize_settings(provider) + print(Fore.GREEN + f"Using {settings.LLM_PROVIDER} with model {settings.LLM_MODEL}" + Style.RESET_ALL) + + # Temporary Lazy Imports + from src.core.graph import CreateGraph + from src.data.database import VectorDBManager, TinyDBManager + from src.core.prompt import PromptManager + from scripts import seed_database + + # Initialize PromptManager + print("Initializing PromptManager...") + prompt_manager = PromptManager() + # Setup Database if db_choice == "2": print("Seeding TinyDB with AntiPattern Dataset") @@ -110,22 +133,11 @@ def main(): retriever = db_manager.as_retriever() langgraph = CreateGraph(db_manager, prompt_manager, retriever=retriever).workflow - # Final results summary - print(Fore.GREEN + f"\nAnalysis Complete!" + Style.RESET_ALL) - print(f"Final state keys: {list(final_state.keys())}") - print(f"Context retrieved: {'Yes' if final_state.get('context') else 'No'}") - print(f"Analysis completed: {'Yes' if final_state.get('antipatterns_scanner_results') else 'No'}") - print(f"Refactored code: {'Yes' if final_state.get('refactored_code') else 'No'}") - print(f"Code review results: {final_state.get('code_review_times')}") - - # Show explanation from ExplainerAgent - if final_state.get("explanation_json"): - import json - print(Fore.CYAN + "\n=== Explanation (JSON) ===" + Style.RESET_ALL) - print(json.dumps(final_state["explanation_json"], indent=2, ensure_ascii=False)) + # Run the selected workflow + if mode_choice == "1": + run_code_snippet_workflow(settings, db_manager, prompt_manager, langgraph) else: - print(Fore.RED + "\nNo explanation was generated." + Style.RESET_ALL) - + run_full_repo_workflow(settings, db_manager, prompt_manager, langgraph) if __name__ == "__main__": main() diff --git a/AntiPattern_Remediator/src/core/agents/__init__.py b/AntiPattern_Remediator/src/core/agents/__init__.py index d5eceaa..79c1adc 100644 --- a/AntiPattern_Remediator/src/core/agents/__init__.py +++ b/AntiPattern_Remediator/src/core/agents/__init__.py @@ -7,12 +7,10 @@ from .refactor_strategist import RefactorStrategist from .code_transformer import CodeTransformer from .code_reviewer import CodeReviewerAgent -from .explainer import ExplainerAgent __all__ = [ "AntipatternScanner", "RefactorStrategist", "CodeTransformer", - "CodeReviewerAgent", - "ExplainerAgent" + "CodeReviewerAgent" ] diff --git a/AntiPattern_Remediator/src/core/agents/explainer.py b/AntiPattern_Remediator/src/core/agents/explainer.py deleted file mode 100644 index 4ab86a6..0000000 --- a/AntiPattern_Remediator/src/core/agents/explainer.py +++ /dev/null @@ -1,131 +0,0 @@ -""" -ExplainerAgent — minimal version -- Delegates state handling to create_graph.py -- Uses PromptManager if available; otherwise a tiny inline fallback prompt -- Always passes msgs; always returns a non-empty explanation_json -""" -from __future__ import annotations - -from typing import Dict, Any -import json - -from langchain_core.language_models import BaseLanguageModel -from langchain.prompts import ChatPromptTemplate, MessagesPlaceholder -from ..prompt import PromptManager -from src.core.utils import extract_first_json - -PROMPT_KEY = "explainer" - - -class ExplainerAgent: - def __init__(self, llm: BaseLanguageModel, prompt_manager: PromptManager): - self.llm = llm - self.prompt_manager = prompt_manager - - def explain_antipattern(self, state: Dict[str, Any]) -> Dict[str, Any]: - """Generate explanation JSON for detected antipatterns and refactor.""" - kwargs = dict( - code=state.get("code", ""), - language=state.get("language", "Java"), - context=state.get("context", ""), - refactored_code=state.get("refactored_code", ""), - refactor_rationale=state.get("refactor_rationale", ""), - antipattern_name=state.get("antipattern_name", "Unknown antipattern"), - antipattern_description=state.get("antipattern_description", ""), - antipatterns_json=json.dumps(state.get("antipatterns_json", []), ensure_ascii=False), - msgs=state.get("msgs", []), # ensure MessagesPlaceholder is satisfied - ) - - messages = self._build_messages(**kwargs) - - try: - response = self.llm.invoke(messages) - raw = getattr(response, "content", None) or str(response) - except Exception as e: - raw = f"LLM error: {e}" - - state["explanation_response_raw"] = raw - - # Robust parse: accept dict, wrap list, or emit a minimal fallback - try: - parsed = extract_first_json(raw) - except Exception: - parsed = None - - if isinstance(parsed, dict): - state["explanation_json"] = parsed - elif isinstance(parsed, list): - state["explanation_json"] = {"items": parsed} - else: - state["explanation_json"] = self._fallback_payload(state) - - return state - - def display_explanation(self, state: Dict[str, Any]) -> Dict[str, Any]: - print("\n=== Explanation (raw) ===\n", state.get("explanation_response_raw", "N/A")) - if state.get("explanation_json"): - print("\n=== Explanation (JSON) ===\n", - json.dumps(state["explanation_json"], indent=2, ensure_ascii=False)) - return state - - def _build_messages(self, **kwargs) -> Any: - # Always ensure msgs exists - if "msgs" not in kwargs or kwargs["msgs"] is None: - kwargs = {**kwargs, "msgs": []} - - # 1) Try preloaded template from PromptManager - prompt = None - getp = getattr(self.prompt_manager, "get_prompt", None) - if callable(getp): - prompt = getp(PROMPT_KEY) - if prompt is not None: - return prompt.format_messages(**kwargs) - - # 2) Minimal inline fallback - schema = { - "items": [{ - "antipattern_name": "", - "antipattern_description": "", - "impact": "", - "why_it_is_bad": "", - "how_we_fixed_it": "", - "refactored_code": "", - "summary": "" - }], - "what_changed": [], - "why_better": [], - "principles_applied": [], - "trade_offs": [], - "closing_summary": "" - } - content = ( - "Given inputs (JSON):\n" + json.dumps({k: v for k, v in kwargs.items() if k != "msgs"}, ensure_ascii=False) + - "\nRespond with STRICT JSON using exactly this schema:\n" + - json.dumps(schema, ensure_ascii=False) - ) - fallback = ChatPromptTemplate.from_messages([ - ("system", "Return STRICT JSON only. No commentary."), - ("user", content), - MessagesPlaceholder("msgs"), - ]) - return fallback.format_messages(**kwargs) - - @staticmethod - def _fallback_payload(state: Dict[str, Any]) -> Dict[str, Any]: - """Tiny fallback so downstream never breaks if parsing fails.""" - return { - "items": [{ - "antipattern_name": state.get("antipattern_name", "Unknown antipattern"), - "antipattern_description": state.get("antipattern_description", ""), - "impact": "", - "why_it_is_bad": "", - "how_we_fixed_it": state.get("refactor_rationale", ""), - "refactored_code": state.get("refactored_code", ""), - "summary": "Auto-generated minimal explanation (parser fallback)." - }], - "what_changed": [], - "why_better": [], - "principles_applied": [], - "trade_offs": [], - "closing_summary": "" - } diff --git a/AntiPattern_Remediator/src/core/graph/create_graph.py b/AntiPattern_Remediator/src/core/graph/create_graph.py index 096d403..9c4cf93 100644 --- a/AntiPattern_Remediator/src/core/graph/create_graph.py +++ b/AntiPattern_Remediator/src/core/graph/create_graph.py @@ -14,7 +14,6 @@ from ..agents import RefactorStrategist from ..agents import CodeTransformer from ..agents import CodeReviewerAgent -from ..agents import ExplainerAgent # Imports for LangSmith tracing import os @@ -63,7 +62,7 @@ def __init__(self, db_manager, prompt_manager: PromptManager, retriever=None, ll self.prompt_manager = prompt_manager self.conditional_edges = ConditionalEdges() - # assign the instance attribute before use + # ✅ assign the instance attribute before use self.retriever = retriever or self.db_manager.as_retriever() retriever_tool = create_retriever_tool( @@ -79,13 +78,13 @@ def __init__(self, db_manager, prompt_manager: PromptManager, retriever=None, ll "strategist": RefactorStrategist(self.llm, self.prompt_manager, retriever=self.retriever), "transformer": CodeTransformer(self.llm, self.prompt_manager), "reviewer": CodeReviewerAgent(self.llm, self.prompt_manager), - "explainer": ExplainerAgent(self.llm, self.prompt_manager) } # Build the LangGraph workflow self.workflow = self._build_graph() def _build_graph(self): + """Build LangGraph workflow""" graph = StateGraph(AgentState) # Scanner: retrieve + analyze @@ -105,10 +104,6 @@ def _build_graph(self): graph.add_node("review_code", self.agents["reviewer"].review_code) graph.add_node("display_code_review_results", self.agents["reviewer"].display_code_review_results) - # Explainer: final storytelling - graph.add_node("explain_antipattern", self.agents["explainer"].explain_antipattern) - graph.add_node("display_explanation", self.agents["explainer"].display_explanation) - # Topology graph.set_entry_point("retrieve_context") graph.add_edge("retrieve_context", "analyze_antipatterns") @@ -128,8 +123,6 @@ def _build_graph(self): }, ) - graph.add_edge("display_code_review_results", "explain_antipattern") - graph.add_edge("explain_antipattern", "display_explanation") - graph.add_edge("display_explanation", END) + graph.add_edge("display_code_review_results", END) return graph.compile() diff --git a/AntiPattern_Remediator/src/core/prompt/prompt_manager.py b/AntiPattern_Remediator/src/core/prompt/prompt_manager.py index d7fc7dd..0650429 100644 --- a/AntiPattern_Remediator/src/core/prompt/prompt_manager.py +++ b/AntiPattern_Remediator/src/core/prompt/prompt_manager.py @@ -7,26 +7,27 @@ class PromptManager: """Manager for handling prompt templates and configurations.""" def __init__(self): - # Prompt key constants, **same as YAML filenames (without .yaml)** + # Prompt key constants, **same as YAML filenames** self.ANTIPATTERN_SCANNER = "antipattern_scanner" - self.REFACTOR_STRATEGIST = "refactor_strategist" + self.REFACTOR_STRATEGIST = "refactor_strategist" self.CODE_TRANSFORMER = "code_transformer" self.CODE_REVIEWER = "code_reviewer" - self.EXPLAINER_AGENT = "explainer" self.prompt_directory = settings.PROMPT_DIR + # Initialize storage for prompt templates self._prompt_cache = {} + # Load prompts on initialization self._load_all_prompts() def _load_all_prompts(self) -> None: """Load all prompt configurations from YAML files.""" try: + # Get all prompt constants and load corresponding files prompt_constants = [ self.ANTIPATTERN_SCANNER, self.REFACTOR_STRATEGIST, self.CODE_TRANSFORMER, self.CODE_REVIEWER, - self.EXPLAINER_AGENT, ] for prompt_key in prompt_constants: @@ -41,40 +42,33 @@ def _load_all_prompts(self) -> None: def _load_prompt_from_yaml(self, filename: str, prompt_key: str) -> None: """Load a prompt configuration from a YAML file.""" yaml_path = self.prompt_directory / filename - + if not yaml_path.exists(): print(f"Warning: Prompt file {yaml_path} not found") return - + try: with open(yaml_path, 'r', encoding='utf-8') as file: config = yaml.safe_load(file) - - if not config or prompt_key not in config: + if prompt_key not in config: print(f"Warning: Section '{prompt_key}' not found in {filename}") return - - prompt_config = config.get(prompt_key) or {} - - # Always include System first (empty string if not provided) to satisfy tests - system_text = str(prompt_config.get("system", "") or "") - user_text = str(prompt_config.get("user", "") or "") - - messages = [ - ("system", system_text), # always present (possibly empty) - ("user", user_text), # always present (possibly empty) - MessagesPlaceholder("msgs"), # conversation history - ] - - self._prompt_cache[prompt_key] = ChatPromptTemplate.from_messages(messages) + + prompt_config = config[prompt_key] + # Create ChatPromptTemplate + self._prompt_cache[prompt_key] = ChatPromptTemplate([ + ("system", prompt_config.get('system', '')), + ("user", prompt_config.get('user', '')), + MessagesPlaceholder("msgs") + ]) print(f"Loaded prompt '{prompt_key}' from {filename}") - + except Exception as e: print(f"Error loading prompt from {filename}: {e}") - def get_prompt(self, prompt_key: str) -> Optional[ChatPromptTemplate]: if prompt_key not in self._prompt_cache: print(f"Warning: Prompt '{prompt_key}' not found in cache") return None - return self._prompt_cache[prompt_key] + + return self._prompt_cache[prompt_key] \ No newline at end of file diff --git a/AntiPattern_Remediator/src/core/state.py b/AntiPattern_Remediator/src/core/state.py index 3fa9562..fb42b13 100644 --- a/AntiPattern_Remediator/src/core/state.py +++ b/AntiPattern_Remediator/src/core/state.py @@ -8,17 +8,13 @@ class AgentState(TypedDict): """State definition for passing data through the workflow""" code: str # Code to be analyzed - language: Optional[str] # Language of the code (used by ExplainerAgent) context: Optional[str] # Context retrieved from knowledge base (scanner) trove_context: Optional[str] # Context retrieved from the Anti-Pattern Trove (TinyDB/Chroma) - antipatterns_scanner_results: Optional[Dict[str, Any]] # Scanner output (structured) - antipatterns_json: Optional[List[Dict[str, Any]]] # Normalized list used by ExplainerAgent - refactoring_strategy_results: Optional[Any] # Strategy can be dict/list/str depending on agent - refactored_code: Optional[str] # Code after refactoring - code_review_results: Optional[str] # Code review results - code_review_times: int # Number of times code has been reviewed - msgs: List[Any] # Conversation history (LangChain BaseMessages) - answer: Optional[str] # Final/aggregated analysis result - - explanation_response_raw: Optional[str] # Raw LLM output from explainer - explanation_json: Optional[Dict[str, Any]] # Parsed JSON explanation + antipatterns_scanner_results: Optional[str] + refactoring_strategy_results: Optional[str] # Refactoring strategy generated by strategist + refactored_code: Optional[str] # Code after refactoring + code_review_results: Optional[str] # Code review results + code_review_times: int # Number of times code has been reviewed + msgs: List[Dict[str, Any]] # Message history for conversation context + answer: Optional[str] # Analysis result + current_file_path: Optional[str] # Path to the current file being processed diff --git a/AntiPattern_Remediator/src/core/utils/__init__.py b/AntiPattern_Remediator/src/core/utils/__init__.py deleted file mode 100644 index ccba67c..0000000 --- a/AntiPattern_Remediator/src/core/utils/__init__.py +++ /dev/null @@ -1,3 +0,0 @@ -from .json_utils import extract_first_json - -__all__ = ["extract_first_json"] \ No newline at end of file diff --git a/AntiPattern_Remediator/src/core/utils/json_utils.py b/AntiPattern_Remediator/src/core/utils/json_utils.py deleted file mode 100644 index d2e0b2f..0000000 --- a/AntiPattern_Remediator/src/core/utils/json_utils.py +++ /dev/null @@ -1,27 +0,0 @@ -import json - -def extract_first_json(text): - """ - Try to extract the first JSON object from a string. - Works if JSON is inside ```json ... ``` fences or just plain text. - """ - if not isinstance(text, str): - return None - - # 1. If the text has fenced JSON like ```json ... ``` - if "```" in text: - parts = text.split("```") - for part in parts: - # Look for JSON-specific fences - if part.strip().lower().startswith("json"): - json_part = part[len("json"):].strip() - try: - return json.loads(json_part) - except Exception: - pass # Try next part - - # 2. If no fenced JSON worked, try to parse the whole text - try: - return json.loads(text.strip()) - except Exception: - return None \ No newline at end of file diff --git a/AntiPattern_Remediator/static/prompt/explainer.yaml b/AntiPattern_Remediator/static/prompt/explainer.yaml deleted file mode 100644 index be29ab7..0000000 --- a/AntiPattern_Remediator/static/prompt/explainer.yaml +++ /dev/null @@ -1,41 +0,0 @@ -explainer: - system: | - You are a senior software reviewer. Output STRICT JSON only — no commentary outside the JSON object. - - user: | - === Inputs === - Language: {language} - Context: {context} - Detected Anti-patterns (JSON): {antipatterns_json} - Code: - ```{code}``` - Refactored Code: - ```{refactored_code}``` - Refactor Rationale: - {refactor_rationale} - - === Return EXACTLY this JSON structure === - {{ - "items": [ - {{ - "antipattern_name": "", - "antipattern_description": "", - "impact": "", - "why_it_is_bad": "", - "how_we_fixed_it": "", - "refactored_code": "", - "summary": "" - }} - ], - "what_changed": [], - "why_better": [], - "principles_applied": [], - "trade_offs": [], - "closing_summary": "" - }} - - Rules: - - Return valid JSON only (no code fences, no prose). - - If multiple antipatterns apply, include multiple entries in "items". - - Keep "refactored_code" concise; truncate if needed but keep it valid. - - Populate all fields; use brief placeholders if unsure. diff --git a/AntiPattern_Remediator/static/tinydb.json b/AntiPattern_Remediator/static/tinydb.json index 8f6198a..cd87283 100644 --- a/AntiPattern_Remediator/static/tinydb.json +++ b/AntiPattern_Remediator/static/tinydb.json @@ -1 +1 @@ -{"_default": {"1": {"name": "Monolithic Method", "description": "A Monolithic Method is a single method that tries to do too much, often combining unrelated responsibilities into one block of code. This is essentially a method-level violation of the Single Responsibility Principle (SRP). Monolithic methods are hard to read, understand, maintain, and test because they mix business logic, I/O, error handling, and other concerns in one place.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor maintainability : Modifying one part of the method risks breaking unrelated functionality.\nLow readability : Long, complex methods are hard to follow and understand.\nDifficult testing : Unit tests become cumbersome because the method does too many things at once.\nCode duplication : Reusing logic is difficult; similar tasks often get reimplemented elsewhere.\nTight coupling : Internal details are intertwined, making refactoring risky.", "remediation": "", "limitation": "Refactoring may require changes to public method signatures, especially if other code depends on it.\nSome logic may rely on shared state or multiple services, making it hard to separate without broader architectural changes.\nIf the method handles cross-cutting concerns (e.g., logging, metrics, validation), isolating responsibilities may require AOP or middleware.", "type": "antipattern", "source_file": "monolithic_method.json"}, "2": {"name": "Unsafe or Vague Exception Handling", "description": "Reliable exception handling, type safety, and controlled flow are essential forr writing maintainable and robust software. Unsafe or Vague exception handling often results in code that is fragile, difficult to test, and challenging to debug. Instead of providing meaningful error handling or clear separation of concerns, these implementations either hide the underlying problem or use language features in a way that breaks maintainability.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Hidden failures : Catching exceptions without logging or meaningful handling hides the source of issues and makes debugging difficult\nPoor diagnosis : Broad exception handling with vague error messages hides intent and makes it harder to trace the root cause\nInconsistent runtime behaviour : using assertions for control logic can lead to unpredictable behaviour depending on the JVM configuration\nUncontrolled termination : Using system.exit() directly in application logic makes code untestable and prevents proper resource cleanup", "remediation": "Replace assertions with proper condition checks and informative exceptions to ensure consistent behaviour across environments\nAvoid silent catch blocks, log exceptions or rethrow them to preserve error context\nHandle specific exception types instead of catching broad categories like Exception or RuntimeException\nRefactor abrupt shutdown calls into controlled exits using exception handling or return code to support recovery", "limitation": "", "type": "antipattern", "source_file": "unsafe_or_vague_exception_handling.json"}, "3": {"name": "Deep Nesting", "description": "Deep Nesting occurs when conditional or loop blocks are embedded within one another across multiple levels, creating code with high indentation and complex control flow. While not always increasing cyclomatic complexity linearly, deep nesting significantly raises cognitive complexity (the mental effort required to understand, modify, and debug a method).", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Low readability : The \"arrowhead\" structure makes it hard to trace logic and understand the intended flow.\nHigh cognitive load : Developers must mentally track the conditions that lead to or prevent reaching a particular line of code.\nError-prone maintenance : Adding or modifying logic inside deeply nested blocks increases the risk of missing cases or introducing bugs.\nInhibited reuse and testing : Deep nesting often combines concerns that should be split into smaller, testable methods or units.\nPoor diffs in version control : Even small changes can alter indentation across many lines, making reviews harder.", "remediation": "Guard clauses (early return) : Exit early when preconditions fail, flattening the control flow\nExtract method : Isolate deeply nested blocks into private methods with clear names to separate concerns and reduce depth.\nInvet conditionals : Invert logic to return early or skip unnecessary branches.\nReplace nested loops with streams (Java-specific): Abstract common filtering or mapping logic into declarative operations.\nUse pattern matching : Replace layered `if` chains with clearer structural or type-based matching.\nEncapsulate state checks : Group multiple conditionals into intention-revealing boolean helpers or state objects.", "limitation": "Can conflict with existing code style : Teams unfamiliar with guard clauses may resist early exits or multiple returns.\nRefactoring can obscure logic during transition : Extracted methods must be clearly named to preserve readability and avoid confusion.\nNested logic may be unavoidable in rare edge cases : Complex parsing, state machines, or embedded domain-specific languages may naturally involve deeper control structures.", "type": "antipattern", "source_file": "deep_nesting.json"}, "4": {"name": "SRP Violation", "description": "The Single Responsibility Principle (SRP) is one of the SOLID principles of object-oriented design, stating that a class (or, at a lower level, a method) should have only one reason to change (i.e., it should have only one responsibility or concern). A SRP Violation occurs when a class (method) takes on multiple unrelated responsibilities, making it harder to maintain, test, and understand. These violations often result in bloated classes or methods that mix unrelated concerns such as I/O, business logic, error handling, and configuration.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor maintainability : Changing one responsibility might inadvertently affect others, introducing bugs.\nLow cohesion : Code with unrelated responsibilities lacks a clear purpose, reducing clarity and reusability.\nDifficult testing : Unit testing becomes more complex as setup may require mocking or initializing unrelated dependencies.\nCode duplication and tight coupling : Responsibilities are harder to reuse or share, often leading to repeated logic or tight inter-class dependencies.\nHarder onboarding : New developers struggle to understand the purpose and scope of large, multi-purpose classes.", "remediation": "Extract Method \\- Split complex methods into smaller, single-purpose private methods.\nEarly Return (Guard Clauses) \\- Use early exits to reduce nested logic and clarify separate responsibilities.\nUse Local Functions/Lambdas \\- Encapsulate small inline logic into local functions for clarity.\nEncapsulate Temporary Variables \\- Move logic-heavy expressions into descriptive helper methods.\nGroup Related Logic \\- Cluster related operations into distinct helper methods within the same class.\nSeparate Concerns in Loops \\- Extract filtering, transforming, and aggregating into distinct steps.\nIsolate Logging/Error Handling \\- Move side-effect code like logging into dedicated private methods (unless the logging is a trivial single line and does not obscure business logic).", "limitation": "Fixing certain SRP violations (especially class-level violations) requires changing public method signatures, creating new classes, and/or breaking interfaces.\nMethods often depend on multiple injected services or shared state; untangling responsibilities might require broader architectural changes.\nLogging, error handling, metrics, and security checks are often scattered across responsibilities and difficult to isolate cleanly at the method level without aspect-oriented programming (AOP) or middleware/interceptor patterns.", "type": "antipattern", "source_file": "srp_violation.json"}, "5": {"name": "Magic Constants", "description": "Magic constants (or magic numbers) are hard-coded literal values (e.g., 3.14, 42, \"admin\") that appear directly in code without context or explanation. These values become problematic when their purpose is unclear, undocumented, or reused inconsistently. While some literals (like 0, 1, or -1) may be self-explanatory in some contexts, others represent thresholds, identifiers, or rules that should be named and documented for clarity.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor readability : Developers must guess the meaning of the value, increasing cognitive load.\nLow maintainability : Changing the value requires updating it everywhere, risking inconsistent updates.\nHarder debugging : Literal values lack descriptive meaning in stack traces, logs, or debuggers.\nDuplication : The same literal used in multiple places leads to repeated logic and tighter coupling.\nViolation of DRY : Embeds implicit meaning multiple times without abstraction.", "remediation": "Define Constants - Move literal values into named constants (e.g., const, final, or static readonly).\nUse Enums - Group related constants (e.g., roles, statuses) as enumerations with meaningful names.", "limitation": "Extracting trivial values like 0 or 1 may clutter code and reduce clarity if overdone.\nIn performance-critical code, indirection through constants or functions may introduce slight overhead.\nOver-abstracting unnamed constants (e.g., MAXCOUNT3) may make code harder to understand.\nIf a value is used only once and is self-explanatory, extracting it may be unnecessary overhead.", "type": "antipattern", "source_file": "magic_constants.json"}, "6": {"name": "Duplicate Code", "description": "Duplicate Code occurs when identical or very similar code blocks are repeated throughout the codebase. This pattern creates maintenance overhead, increases the likelihood of bugs, and violates the DRY (Don't Repeat Yourself) principle. Common examples include repeated null checks, validation logic, and similar conditional patterns across different methods or classes.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Maintenance overhead : Changes need to be applied in multiple places, increasing the risk of inconsistencies\nBug multiplication : A bug in duplicated code affects multiple locations, making fixes more complex\nCode bloat : Repeated code increases the overall size of the codebase without adding functionality\nViolation of DRY principle : Makes the code harder to understand and reason about", "remediation": "", "limitation": "Over-abstraction can make code harder to understand if the duplication is minimal or contextually different\nPremature extraction of methods may create unnecessary coupling between unrelated parts of the system\nSome duplication might be acceptable if the code serves different business contexts", "type": "antipattern", "source_file": "duplicate_code.json"}, "7": {"name": "God Class", "description": "A God Class anti-pattern refers to a class that centralises too many responsibilities in a single location, becoming overly complex and difficult to maintain. Such classes tend to know too much, do too much, and interact with many different parts of the system. This leads to tightly coupled code, reduces modularity, and makes the system hard to test and extend.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "High coupling: God Classes tend to interact with many other classes and modules, thereby reducing system modularity and increasing the risk of changes.\nPoor maintainability : Large, complex classes are difficult to read, understand, and modify.\nLow reusability : The class becomes so specific and bloated that it is rarely useful outside of its original context.\nHidden dependencies : God Classes often hide dependencies within fields or methods, making the codebase less transparent.", "remediation": "", "limitation": "", "type": "antipattern", "source_file": "god_class.json"}, "8": {"name": "Middle Man", "description": "The Middle Man anti-pattern occurs when a class exists primarily to delegate calls to another class without adding meaningful logic of its own. Essentially, the class acts as a pass-through or proxy, forwarding method calls without adding value. While delegation is sometimes necessary for abstraction, excessive or trivial delegation leads to unnecessary indirection and increases maintenance overhead.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Extra Indirection : Code must go through one more layer, which can complicate understanding the code flow.\nIncreased maintenance : When the delegated class changes, the middle-man class often needs updates for all its pass-through methods.\nLow cohesion : The middle-man class has little real logic, making its purpose unclear\nCode bloat : Many trivial delegation methods clutter the class, making it harder to navigate.\nHarder debugging : Tracing behaviour through multiple layers of delegation adds cognitive load.", "remediation": "", "limitation": "Some delegation is unavoidable, e.g., to implement an interface or provide a stable abstraction layer\nRemoving middleman classes may break existing APIs or require refactoring client code.\nIn some cases, delegation is part of a design pattern (like a proxy), which is intentional and not an anti-pattern", "type": "antipattern", "source_file": "middle_man.json"}, "9": {"name": "Generic Exception handling", "description": "Generic exception handling refers to the use of broad or unspecific catch blocks (catching but ignoring the exceptions). These patterns obscure the true source of errors, suppress the useful debugging information and can unintentionally hide critical failures. This anti-pattern often stems from the desire to keep the code running, but typically leads to weak systems and increased technical debt.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Loss of context : Catching high-level exceptions removes the granularity needed to understand specific failure reasons\nRepeated boilerplate : Developers may re-implement logging, default values, or stream-closing logic instead of using safe utility methods\nViolation of fail-fast principles : Silent or overly generic handling can let critical errors go unnoticed for too long", "remediation": "", "limitation": "Catching too many specific exceptions can bloat the code and reduce readability.\nRefactoring exception handling may require thorough testing to avoid regressions.\nOver-logging exceptions can clutter logs and obscure real issues.\nSecurity-sensitive applications may need specialised exception handling strategies to avoid leaks", "type": "antipattern", "source_file": "generic_exception_handling.json"}}} \ No newline at end of file +{"_default": {"1": {"name": "Deep Nesting", "description": "Deep Nesting occurs when conditional or loop blocks are embedded within one another across multiple levels, creating code with high indentation and complex control flow. While not always increasing cyclomatic complexity linearly, deep nesting significantly raises cognitive complexity (the mental effort required to understand, modify, and debug a method).", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Low readability : The \"arrowhead\" structure makes it hard to trace logic and understand the intended flow.\nHigh cognitive load : Developers must mentally track the conditions that lead to or prevent reaching a particular line of code.\nError-prone maintenance : Adding or modifying logic inside deeply nested blocks increases the risk of missing cases or introducing bugs.\nInhibited reuse and testing : Deep nesting often combines concerns that should be split into smaller, testable methods or units.\nPoor diffs in version control : Even small changes can alter indentation across many lines, making reviews harder.", "remediation": "Guard clauses (early return) : Exit early when preconditions fail, flattening the control flow\nExtract method : Isolate deeply nested blocks into private methods with clear names to separate concerns and reduce depth.\nInvet conditionals : Invert logic to return early or skip unnecessary branches.\nReplace nested loops with streams (Java-specific): Abstract common filtering or mapping logic into declarative operations.\nUse pattern matching : Replace layered `if` chains with clearer structural or type-based matching.\nEncapsulate state checks : Group multiple conditionals into intention-revealing boolean helpers or state objects.", "limitation": "Can conflict with existing code style : Teams unfamiliar with guard clauses may resist early exits or multiple returns.\nRefactoring can obscure logic during transition : Extracted methods must be clearly named to preserve readability and avoid confusion.\nNested logic may be unavoidable in rare edge cases : Complex parsing, state machines, or embedded domain-specific languages may naturally involve deeper control structures.", "type": "antipattern", "source_file": "deep_nesting.json"}, "2": {"name": "Generic Exception handling", "description": "Generic exception handling refers to the use of broad or unspecific catch blocks (catching but ignoring the exceptions). These patterns obscure the true source of errors, suppress the useful debugging information and can unintentionally hide critical failures. This anti-pattern often stems from the desire to keep the code running, but typically leads to weak systems and increased technical debt.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Loss of context : Catching high-level exceptions removes the granularity needed to understand specific failure reasons\nRepeated boilerplate : Developers may re-implement logging, default values, or stream-closing logic instead of using safe utility methods\nViolation of fail-fast principles : Silent or overly generic handling can let critical errors go unnoticed for too long", "remediation": "", "limitation": "Catching too many specific exceptions can bloat the code and reduce readability.\nRefactoring exception handling may require thorough testing to avoid regressions.\nOver-logging exceptions can clutter logs and obscure real issues.\nSecurity-sensitive applications may need specialised exception handling strategies to avoid leaks", "type": "antipattern", "source_file": "generic_exception_handling.json"}, "3": {"name": "Magic Constants", "description": "Magic constants (or magic numbers) are hard-coded literal values (e.g., 3.14, 42, \"admin\") that appear directly in code without context or explanation. These values become problematic when their purpose is unclear, undocumented, or reused inconsistently. While some literals (like 0, 1, or -1) may be self-explanatory in some contexts, others represent thresholds, identifiers, or rules that should be named and documented for clarity.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor readability : Developers must guess the meaning of the value, increasing cognitive load.\nLow maintainability : Changing the value requires updating it everywhere, risking inconsistent updates.\nHarder debugging : Literal values lack descriptive meaning in stack traces, logs, or debuggers.\nDuplication : The same literal used in multiple places leads to repeated logic and tighter coupling.\nViolation of DRY : Embeds implicit meaning multiple times without abstraction.", "remediation": "Define Constants - Move literal values into named constants (e.g., const, final, or static readonly).\nUse Enums - Group related constants (e.g., roles, statuses) as enumerations with meaningful names.", "limitation": "Extracting trivial values like 0 or 1 may clutter code and reduce clarity if overdone.\nIn performance-critical code, indirection through constants or functions may introduce slight overhead.\nOver-abstracting unnamed constants (e.g., MAXCOUNT3) may make code harder to understand.\nIf a value is used only once and is self-explanatory, extracting it may be unnecessary overhead.", "type": "antipattern", "source_file": "magic_constants.json"}, "4": {"name": "SRP Violation", "description": "The Single Responsibility Principle (SRP) is one of the SOLID principles of object-oriented design, stating that a class (or, at a lower level, a method) should have only one reason to change (i.e., it should have only one responsibility or concern). A SRP Violation occurs when a class (method) takes on multiple unrelated responsibilities, making it harder to maintain, test, and understand. These violations often result in bloated classes or methods that mix unrelated concerns such as I/O, business logic, error handling, and configuration.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Poor maintainability : Changing one responsibility might inadvertently affect others, introducing bugs.\nLow cohesion : Code with unrelated responsibilities lacks a clear purpose, reducing clarity and reusability.\nDifficult testing : Unit testing becomes more complex as setup may require mocking or initializing unrelated dependencies.\nCode duplication and tight coupling : Responsibilities are harder to reuse or share, often leading to repeated logic or tight inter-class dependencies.\nHarder onboarding : New developers struggle to understand the purpose and scope of large, multi-purpose classes.", "remediation": "Extract Method \\- Split complex methods into smaller, single-purpose private methods.\nEarly Return (Guard Clauses) \\- Use early exits to reduce nested logic and clarify separate responsibilities.\nUse Local Functions/Lambdas \\- Encapsulate small inline logic into local functions for clarity.\nEncapsulate Temporary Variables \\- Move logic-heavy expressions into descriptive helper methods.\nGroup Related Logic \\- Cluster related operations into distinct helper methods within the same class.\nSeparate Concerns in Loops \\- Extract filtering, transforming, and aggregating into distinct steps.\nIsolate Logging/Error Handling \\- Move side-effect code like logging into dedicated private methods (unless the logging is a trivial single line and does not obscure business logic).", "limitation": "Fixing certain SRP violations (especially class-level violations) requires changing public method signatures, creating new classes, and/or breaking interfaces.\nMethods often depend on multiple injected services or shared state; untangling responsibilities might require broader architectural changes.\nLogging, error handling, metrics, and security checks are often scattered across responsibilities and difficult to isolate cleanly at the method level without aspect-oriented programming (AOP) or middleware/interceptor patterns.", "type": "antipattern", "source_file": "srp_violation.json"}, "5": {"name": "Unsafe or Vague Exception Handling", "description": "Reliable exception handling, type safety, and controlled flow are essential forr writing maintainable and robust software. Unsafe or Vague exception handling often results in code that is fragile, difficult to test, and challenging to debug. Instead of providing meaningful error handling or clear separation of concerns, these implementations either hide the underlying problem or use language features in a way that breaks maintainability.", "category": "Uncategorised", "language": "Any", "severity": "MEDIUM", "problem": "Hidden failures : Catching exceptions without logging or meaningful handling hides the source of issues and makes debugging difficult\nPoor diagnosis : Broad exception handling with vague error messages hides intent and makes it harder to trace the root cause\nInconsistent runtime behaviour : using assertions for control logic can lead to unpredictable behaviour depending on the JVM configuration\nUncontrolled termination : Using system.exit() directly in application logic makes code untestable and prevents proper resource cleanup", "remediation": "Replace assertions with proper condition checks and informative exceptions to ensure consistent behaviour across environments\nAvoid silent catch blocks, log exceptions or rethrow them to preserve error context\nHandle specific exception types instead of catching broad categories like Exception or RuntimeException\nRefactor abrupt shutdown calls into controlled exits using exception handling or return code to support recovery", "limitation": "", "type": "antipattern", "source_file": "unsafe_or_vague_exception_handling.json"}}} \ No newline at end of file diff --git a/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py b/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py index 1fb5dec..088af2f 100644 --- a/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py +++ b/AntiPattern_Remediator/test/unit_test/prompt/test_prompt_manager.py @@ -44,7 +44,6 @@ def __init__(self): self.REFACTOR_STRATEGIST = "refactor_strategist" self.CODE_TRANSFORMER = "code_transformer" self.CODE_REVIEWER = "code_reviewer" - self.EXPLAINER_AGENT = "explainer" self.prompt_directory = Path(__file__).parent self._prompt_cache = {} self._load_all_prompts() # Call this to match real behavior @@ -88,8 +87,7 @@ def _load_all_prompts(self): self.ANTIPATTERN_SCANNER, self.REFACTOR_STRATEGIST, self.CODE_TRANSFORMER, - self.CODE_REVIEWER, - self.EXPLAINER_AGENT, + self.CODE_REVIEWER ] for prompt_key in prompt_constants: @@ -157,7 +155,6 @@ def test_initialization_creates_correct_attributes(self): assert hasattr(manager, 'REFACTOR_STRATEGIST') assert hasattr(manager, 'CODE_TRANSFORMER') assert hasattr(manager, 'CODE_REVIEWER') - assert hasattr(manager, 'EXPLAINER_AGENT') assert hasattr(manager, 'prompt_directory') assert hasattr(manager, '_prompt_cache') assert isinstance(manager._prompt_cache, dict) @@ -179,7 +176,6 @@ def test_prompt_constants_have_correct_values(self): assert manager.REFACTOR_STRATEGIST == "refactor_strategist" assert manager.CODE_TRANSFORMER == "code_transformer" assert manager.CODE_REVIEWER == "code_reviewer" - assert manager.EXPLAINER_AGENT == "explainer" def test_prompt_directory_is_set_correctly(self): """Test that prompt directory is assigned from settings.""" @@ -509,7 +505,6 @@ def test_initialization_with_missing_directory(self, capsys): manager.REFACTOR_STRATEGIST = "refactor_strategist" manager.CODE_TRANSFORMER = "code_transformer" manager.CODE_REVIEWER = "code_reviewer" - manager.EXPLAINER_AGENT = "explainer" manager.prompt_directory = Path("/non/existent/path") manager._prompt_cache = {} @@ -580,12 +575,6 @@ def temp_prompt_files(self): 'system': 'You are an expert code reviewer.', 'user': 'Review this code for quality and best practices: {code}' } - }, - 'explainer.yaml': { - 'explainer': { - 'system': 'You are a senior software code explainer.', - 'user': 'Explain: {code}\nLang: {language}\nCtx: {context}' - } } } @@ -604,7 +593,6 @@ def test_load_all_prompts_loads_all_available_files(self, temp_prompt_files, cap manager.REFACTOR_STRATEGIST = "refactor_strategist" manager.CODE_TRANSFORMER = "code_transformer" manager.CODE_REVIEWER = "code_reviewer" - manager.EXPLAINER_AGENT = "explainer" manager.prompt_directory = temp_prompt_files manager._prompt_cache = {} @@ -612,16 +600,15 @@ def test_load_all_prompts_loads_all_available_files(self, temp_prompt_files, cap manager._load_all_prompts() # Assert: Verify all prompts were loaded - assert len(manager._prompt_cache) == 5 + assert len(manager._prompt_cache) == 4 assert "antipattern_scanner" in manager._prompt_cache assert "refactor_strategist" in manager._prompt_cache assert "code_transformer" in manager._prompt_cache assert "code_reviewer" in manager._prompt_cache - assert "explainer" in manager._prompt_cache # Verify success message captured = capsys.readouterr() - assert "Successfully loaded 5 prompts" in captured.out + assert "Successfully loaded 4 prompts" in captured.out def test_load_all_prompts_handles_partial_failures(self, capsys): """Test that _load_all_prompts continues loading even if some files are missing.""" @@ -646,7 +633,6 @@ def test_load_all_prompts_handles_partial_failures(self, capsys): manager.REFACTOR_STRATEGIST = "refactor_strategist" manager.CODE_TRANSFORMER = "code_transformer" manager.CODE_REVIEWER = "code_reviewer" - manager.EXPLAINER_AGENT = "explainer" manager.prompt_directory = temp_path manager._prompt_cache = {}