Open
Conversation
…ases Adds the Refactoring Generator Agent prompt covering four workflow phases: Phase 0 (context loading), Phase 1 (smell analysis), Phase 2 (refactoring planning), Phase 3 (apply refactorings), and Phase 4 (quality validation). Follows the documentation_generator.md pattern with code examples for all major refactoring techniques (Extract Method, Guard Clauses, Parameter Object, Extract Class, Replace Bare Exception). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ion functions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_.py Added explicit import from .refactoring_generator and __all__ entry for run_refactoring_generator_session to satisfy CodeQL static analysis. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR introduces a new "Refactoring Generator" agent module to the backend system. It includes the agent implementation with validation logic, default model configuration, prompt specifications, public API exports, and comprehensive test coverage to support automated code refactoring capabilities. Changes
Sequence DiagramsequenceDiagram
actor Caller
participant run_refactoring_generator_session
participant run_generator_session
participant FileSystem
participant validate_generated_refactorings
participant log_generator_result
Caller->>run_refactoring_generator_session: Call with project_dir, analysis_results
run_refactoring_generator_session->>run_refactoring_generator_session: Build starting_message from analysis_results
run_refactoring_generator_session->>run_generator_session: Delegate execution with message & metadata
alt Generator Success
run_generator_session-->>run_refactoring_generator_session: Return generator output
run_refactoring_generator_session->>FileSystem: Scan apps/backend/ for .py files from analysis_results
FileSystem-->>run_refactoring_generator_session: Return matching files
alt Files Found
run_refactoring_generator_session->>validate_generated_refactorings: Validate files (existence, syntax, non-empty)
validate_generated_refactorings->>FileSystem: Check file paths & parse Python
FileSystem-->>validate_generated_refactorings: File validation results
validate_generated_refactorings-->>run_refactoring_generator_session: Return validation result
run_refactoring_generator_session->>log_generator_result: Log validation outcome
log_generator_result-->>run_refactoring_generator_session: Logged
run_refactoring_generator_session-->>Caller: Return result dict with generated_files & success
else No Files Found
run_refactoring_generator_session-->>Caller: Return failure payload
end
else Generator Failure
run_generator_session-->>run_refactoring_generator_session: Return error
run_refactoring_generator_session-->>Caller: Return failure payload with error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/agents/__init__.py`:
- Around line 32-35: The import block is unsorted; reorder the two imports so
planner comes before refactoring_generator and remove redundant aliasing: import
run_followup_planner from .planner first, then import
run_refactoring_generator_session from .refactoring_generator, ensuring the
symbol names run_followup_planner and run_refactoring_generator_session are
preserved exactly to satisfy the linter.
In `@apps/backend/agents/refactoring_generator.py`:
- Around line 215-220: The call to log_generator_result uses the semantically
misleading parameter name test_files for refactoring files; update the
_generator_base.log_generator_result function signature to use a neutral name
like output_files (and accept either test_files for backward compatibility or
remove/comment deprecated alias), then change this call in
refactoring_generator.py to pass output_files=refactoring_files (and update any
other callers to use output_files) so the parameter meaning is clear across
generators; ensure function docs and any internal references
(log_generator_result, _generator_base.log_generator_result) are updated
accordingly.
- Around line 179-186: The code currently hardcodes backend_src = project_dir /
"apps" / "backend" and returns an error if that path doesn't exist, which
prevents processing valid files in analysis_results["files"]; remove or
conditionalize this existence check in refactoring_generator (the backend_src /
project_dir logic) so that instead the generator iterates
analysis_results["files"] and skips non-existent paths (the existing per-file
file_path.exists() logic) rather than failing early—either delete the
backend_src existence check and its early return, or gate it behind a condition
that only requires backend_src when no files are present in
analysis_results["files"].
- Around line 68-78: Move the local "import ast" out of the Basic Python syntax
validation block into the module-level imports at the top of
refactoring_generator.py (add a single "import ast" alongside the other
imports), then delete the inline "import ast" statement inside the block that
checks file_path.suffix == '.py'; keep the existing use of ast.parse(content)
and print_status with refactoring_file.name unchanged.
- Around line 89-166: Add GraphitiMemory-based cross-session memory integration
to the refactoring generator by initializing GraphitiMemory from
integrations.graphiti.memory inside run_refactoring_generator_session and wiring
it into the generator flow before calling run_generator_session; specifically,
create or retrieve a memory instance (e.g., GraphitiMemory(...)) and call
memory_manager.learn_patterns(...) with recent analysis_results and/or previous
session outputs, then pass the memory reference or its extracted context
(pattern suggestions from memory_manager.get_pattern_suggestions(...)) into the
starting_message or into the run_generator_session call (use the session
parameters like starting_message or a new memory/context param) so the agent can
incorporate past refactoring patterns without changing run_generator_session
internals. Ensure imports for GraphitiMemory and memory_manager are added and
handle failures gracefully (fallback to no-memory) to preserve existing
behavior.
In `@apps/backend/phase_config.py`:
- Around line 127-131: Add the new refactoring_generator to the provider
defaults and typed config: update AGENT_DEFAULT_PROVIDERS to include
"refactoring_generator": "claude" (mirror other agents' pattern) and add
refactoring_generator: str to the AgentModelConfig TypedDict so type-checkers
recognize it; this ensures get_provider_for_agent("refactoring_generator") is
consistent with other agents and avoids implicit fallbacks—search for
AGENT_DEFAULT_PROVIDERS, AgentModelConfig, and get_provider_for_agent to locate
the exact insertion points.
In `@apps/backend/prompts/refactoring_generator.md`:
- Around line 76-106: The MD031 lint errors come from fenced code blocks placed
immediately after bold headings; add a single blank line between each bold
heading and the following fenced code block throughout the file (apply to the
"Extract Method" example and all other sections flagged), e.g., ensure there is
a blank line before the triple-backtick block that contains the before/after
code for functions like _validate_order, _calculate_total, and process_order so
each heading is separated from its code fence.
- Around line 304-321: Add the missing language identifier to the fenced code
block that starts with "## Refactoring Summary": change the opening triple
backticks to "```markdown" so the block becomes ```markdown followed by the
existing content; update the one occurrence of the plain fenced block in the
file (the block containing the table and deferred notes) to include the markdown
language tag for proper syntax highlighting.
In `@apps/backend/tests/test_refactoring_generator.py`:
- Around line 56-61: importlib.util.spec_from_file_location can return None;
before accessing spec.loader or calling spec.loader.exec_module(module) add a
null check for the returned spec (from spec =
importlib.util.spec_from_file_location(...)). If spec is None, raise a clear
exception or return a meaningful error value (e.g., FileNotFoundError or
ImportError) with context about module_path; otherwise continue with module =
importlib.util.module_from_spec(spec) and spec.loader.exec_module(module).
Ensure you reference spec, spec.loader, and module_path in the error message so
the failure is diagnosable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b0db18cb-20f7-4037-a9a4-bc413bcce393
📒 Files selected for processing (5)
apps/backend/agents/__init__.pyapps/backend/agents/refactoring_generator.pyapps/backend/phase_config.pyapps/backend/prompts/refactoring_generator.mdapps/backend/tests/test_refactoring_generator.py
Comment on lines
+32
to
35
| from .refactoring_generator import ( | ||
| run_refactoring_generator_session as run_refactoring_generator_session, | ||
| ) | ||
| from .planner import run_followup_planner as run_followup_planner |
Contributor
There was a problem hiding this comment.
Fix import order to resolve lint failure.
The pipeline is failing with ruff check: I001 Import block is un-sorted or un-formatted. The refactoring_generator import should come after planner alphabetically (p < r).
Proposed fix
from .performance_profiler import run_performance_profiler as run_performance_profiler
+from .planner import run_followup_planner as run_followup_planner
from .refactoring_generator import (
run_refactoring_generator_session as run_refactoring_generator_session,
)
-from .planner import run_followup_planner as run_followup_planner
from .session import post_session_processing as post_session_processing📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| from .refactoring_generator import ( | |
| run_refactoring_generator_session as run_refactoring_generator_session, | |
| ) | |
| from .planner import run_followup_planner as run_followup_planner | |
| from .planner import run_followup_planner as run_followup_planner | |
| from .refactoring_generator import ( | |
| run_refactoring_generator_session as run_refactoring_generator_session, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/agents/__init__.py` around lines 32 - 35, The import block is
unsorted; reorder the two imports so planner comes before refactoring_generator
and remove redundant aliasing: import run_followup_planner from .planner first,
then import run_refactoring_generator_session from .refactoring_generator,
ensuring the symbol names run_followup_planner and
run_refactoring_generator_session are preserved exactly to satisfy the linter.
Comment on lines
+68
to
+78
| # Basic Python syntax validation | ||
| if file_path.suffix == ".py": | ||
| import ast | ||
|
|
||
| try: | ||
| ast.parse(content) | ||
| except SyntaxError as e: | ||
| print_status( | ||
| f"Syntax error in {refactoring_file.name}: {e}", "error" | ||
| ) | ||
| return False |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider moving ast import to module level.
The ast module is part of Python's standard library and is lightweight. Moving it to module level improves readability and follows the convention in similar files.
Proposed change
Add at top of file (after line 13):
import astThen remove line 70:
# Basic Python syntax validation
if file_path.suffix == ".py":
- import ast
-
try:
ast.parse(content)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/agents/refactoring_generator.py` around lines 68 - 78, Move the
local "import ast" out of the Basic Python syntax validation block into the
module-level imports at the top of refactoring_generator.py (add a single
"import ast" alongside the other imports), then delete the inline "import ast"
statement inside the block that checks file_path.suffix == '.py'; keep the
existing use of ast.parse(content) and print_status with refactoring_file.name
unchanged.
Comment on lines
+89
to
+166
| async def run_refactoring_generator_session( | ||
| project_dir: Path, | ||
| spec_dir: Path, | ||
| analysis_results: dict[str, Any], | ||
| model: str | None = None, | ||
| max_thinking_tokens: int | None = None, | ||
| verbose: bool = False, | ||
| ) -> dict[str, Any]: | ||
| """ | ||
| Run Refactoring Generator Agent session to suggest and apply code refactoring. | ||
|
|
||
| Args: | ||
| project_dir: Root directory for the project | ||
| spec_dir: Directory containing the spec | ||
| analysis_results: Code analysis results from detectors (error_pattern_detector, | ||
| naming_detector, organization_detector, etc.) | ||
| model: Claude model to use (defaults to phase config) | ||
| max_thinking_tokens: Extended thinking token budget (optional) | ||
| verbose: Whether to show detailed output | ||
|
|
||
| Returns: | ||
| Dictionary with: | ||
| - generated_files: List of refactored/generated file paths (relative to project_dir) | ||
| - success: Whether generation succeeded | ||
| - error: Error message if failed | ||
| """ | ||
| starting_message = f"""You are the Refactoring Generator Agent. Your task is to analyze code quality issues and apply targeted refactoring based on the analysis results below. | ||
|
|
||
| ## Code Analysis Results | ||
|
|
||
| {json.dumps(analysis_results, indent=2)} | ||
|
|
||
| ## Your Task | ||
|
|
||
| 1. Read the spec.md to understand what was implemented | ||
| 2. Review the implementation_plan.json to see what features were built | ||
| 3. Study existing code patterns in apps/backend/ to understand conventions | ||
| 4. Apply refactoring to address the detected code smells and anti-patterns: | ||
| - Fix naming issues (variables, functions, classes) | ||
| - Eliminate error pattern anti-patterns | ||
| - Improve code organization and module structure | ||
| - Extract duplicated logic into reusable helpers | ||
| - Simplify complex conditional logic | ||
| 5. Follow the project's coding conventions | ||
| 6. Ensure all refactored files are syntactically valid | ||
| 7. Do not change external interfaces or break existing functionality | ||
|
|
||
| ## Refactoring Priorities | ||
|
|
||
| - High: Syntax errors, undefined variables, incorrect exception handling | ||
| - Medium: Naming violations, duplicated code blocks, overly long functions | ||
| - Low: Style improvements, minor reorganization | ||
|
|
||
| Apply refactoring directly to the source files. Focus on correctness and maintainability. | ||
|
|
||
| Begin by loading context (Phase 0 in your prompt). | ||
| """ | ||
|
|
||
| # Run the shared generator session boilerplate | ||
| session_result = await run_generator_session( | ||
| project_dir=project_dir, | ||
| spec_dir=spec_dir, | ||
| analysis_results=analysis_results, | ||
| session_title="REFACTORING GENERATOR SESSION", | ||
| session_description="Analyzing code and applying automated refactoring...", | ||
| prompt_name="refactoring_generator", | ||
| agent_type="refactoring_generator", | ||
| session_name="refactoring-generator-session", | ||
| starting_message=starting_message, | ||
| log_phase=LogPhase.CODING, | ||
| log_summary=( | ||
| f"Analyzing {len(analysis_results.get('issues', []))} issues across " | ||
| f"{len(analysis_results.get('files', []))} files" | ||
| ), | ||
| model=model, | ||
| max_thinking_tokens=max_thinking_tokens, | ||
| verbose=verbose, | ||
| ) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if other generator agents use GraphitiMemory
rg -l "GraphitiMemory" apps/backend/agents/Repository: OBenner/Auto-Coding
Length of output: 185
🏁 Script executed:
# Check other generator agents and how they use run_generator_session
fd "generator\.py$" apps/backend/agents/ --exec cat -n {} +Repository: OBenner/Auto-Coding
Length of output: 50562
🏁 Script executed:
# Examine the run_generator_session implementation to see if it handles memory
fd "run_generator_session" apps/backend/ -t f --exec rg -l "run_generator_session" {} +Repository: OBenner/Auto-Coding
Length of output: 45
🏁 Script executed:
# Check memory_manager.py to understand how memory is integrated
cat -n apps/backend/agents/memory_manager.pyRepository: OBenner/Auto-Coding
Length of output: 50375
🏁 Script executed:
# Check session_context.py for GraphitiMemory usage
head -100 apps/backend/agents/session_context.pyRepository: OBenner/Auto-Coding
Length of output: 3253
🏁 Script executed:
# Find and examine _generator_base.py
fd "_generator_base" apps/backend/agents/ -type fRepository: OBenner/Auto-Coding
Length of output: 233
🏁 Script executed:
# Search for run_generator_session definition
rg "def run_generator_session" apps/backend/ -A 30Repository: OBenner/Auto-Coding
Length of output: 2336
🏁 Script executed:
# Get the full run_generator_session implementation
rg "def run_generator_session" apps/backend/agents/_generator_base.py -A 200 | head -250Repository: OBenner/Auto-Coding
Length of output: 4724
🏁 Script executed:
# Check if _generator_base.py imports or uses GraphitiMemory
rg "GraphitiMemory|graphiti|memory_manager" apps/backend/agents/_generator_base.pyRepository: OBenner/Auto-Coding
Length of output: 45
Consider adding GraphitiMemory integration for cross-session learning.
Per coding guidelines, agents in apps/backend/agents/ should use GraphitiMemory from integrations.graphiti.memory for cross-session context and semantic search. While the shared run_generator_session() doesn't currently integrate memory, the refactoring generator could benefit from:
- Remembering past refactoring patterns and decisions across sessions
- Learning from previously applied refactorings to provide context-aware suggestions
- Building a knowledge base of project-specific refactoring strategies
This aligns with memory utilities available in memory_manager.py (e.g., learn_patterns(), get_pattern_suggestions()). Optional refactor to improve agent effectiveness over time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/agents/refactoring_generator.py` around lines 89 - 166, Add
GraphitiMemory-based cross-session memory integration to the refactoring
generator by initializing GraphitiMemory from integrations.graphiti.memory
inside run_refactoring_generator_session and wiring it into the generator flow
before calling run_generator_session; specifically, create or retrieve a memory
instance (e.g., GraphitiMemory(...)) and call memory_manager.learn_patterns(...)
with recent analysis_results and/or previous session outputs, then pass the
memory reference or its extracted context (pattern suggestions from
memory_manager.get_pattern_suggestions(...)) into the starting_message or into
the run_generator_session call (use the session parameters like starting_message
or a new memory/context param) so the agent can incorporate past refactoring
patterns without changing run_generator_session internals. Ensure imports for
GraphitiMemory and memory_manager are added and handle failures gracefully
(fallback to no-memory) to preserve existing behavior.
Comment on lines
+179
to
+186
| backend_src = project_dir / "apps" / "backend" | ||
| if not backend_src.exists(): | ||
| logger.warning("Backend source directory not found") | ||
| return { | ||
| "generated_files": [], | ||
| "success": False, | ||
| "error": "Backend source directory not found", | ||
| } |
Contributor
There was a problem hiding this comment.
Hardcoded path apps/backend may not exist in all projects.
The code assumes a specific project structure. If the refactoring generator is run on a project without apps/backend/, it will fail even if there are valid files to refactor in analysis_results["files"].
Consider removing this check or making it conditional based on whether analysis_results["files"] contains valid paths.
Proposed fix - remove the hardcoded path check
- backend_src = project_dir / "apps" / "backend"
- if not backend_src.exists():
- logger.warning("Backend source directory not found")
- return {
- "generated_files": [],
- "success": False,
- "error": "Backend source directory not found",
- }
-
# Collect files listed in analysis_results as having been processed
refactoring_files: list[Path] = []
for file_entry in analysis_results.get("files", []):The subsequent loop already handles non-existent files by checking file_path.exists().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/agents/refactoring_generator.py` around lines 179 - 186, The
code currently hardcodes backend_src = project_dir / "apps" / "backend" and
returns an error if that path doesn't exist, which prevents processing valid
files in analysis_results["files"]; remove or conditionalize this existence
check in refactoring_generator (the backend_src / project_dir logic) so that
instead the generator iterates analysis_results["files"] and skips non-existent
paths (the existing per-file file_path.exists() logic) rather than failing
early—either delete the backend_src existence check and its early return, or
gate it behind a condition that only requires backend_src when no files are
present in analysis_results["files"].
Comment on lines
+215
to
+220
| log_generator_result( | ||
| spec_dir=spec_dir, | ||
| test_files=refactoring_files, | ||
| validation_success=validation_success, | ||
| framework="Refactoring", | ||
| ) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Parameter name test_files is semantically misleading for refactoring files.
The log_generator_result function uses test_files as the parameter name (inherited from test generator use case), but here it's logging refactoring files. This works correctly but could cause confusion during maintenance.
Consider whether _generator_base.log_generator_result should be generalized with a more neutral parameter name like output_files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/agents/refactoring_generator.py` around lines 215 - 220, The
call to log_generator_result uses the semantically misleading parameter name
test_files for refactoring files; update the
_generator_base.log_generator_result function signature to use a neutral name
like output_files (and accept either test_files for backward compatibility or
remove/comment deprecated alias), then change this call in
refactoring_generator.py to pass output_files=refactoring_files (and update any
other callers to use output_files) so the parameter meaning is clear across
generators; ensure function docs and any internal references
(log_generator_result, _generator_base.log_generator_result) are updated
accordingly.
Comment on lines
+127
to
131
| # ═══════════════════════════════════════════════════════════════════════ | ||
| # REFACTORING AGENTS (Use sonnet for code refactoring) | ||
| # ═══════════════════════════════════════════════════════════════════════ | ||
| "refactoring_generator": "sonnet", | ||
| } |
Contributor
There was a problem hiding this comment.
Missing entries for refactoring_generator in AGENT_DEFAULT_PROVIDERS and AgentModelConfig.
The new refactoring_generator agent is added to AGENT_DEFAULT_MODELS, but for consistency with other agents, it should also be added to:
AGENT_DEFAULT_PROVIDERS(around line 182) — defaults to"claude"like other agentsAgentModelConfigTypedDict (around line 234) — for type checking support
Without the provider entry, get_provider_for_agent("refactoring_generator") will fall back to "claude" (line 807), which works but is inconsistent with how other agents are configured.
Proposed additions
Add to AGENT_DEFAULT_PROVIDERS after line 181:
# ═══════════════════════════════════════════════════════════════════════
# REFACTORING AGENTS (Use claude as default provider)
# ═══════════════════════════════════════════════════════════════════════
"refactoring_generator": "claude",Add to AgentModelConfig TypedDict after line 234:
# Refactoring agents
refactoring_generator: str🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/phase_config.py` around lines 127 - 131, Add the new
refactoring_generator to the provider defaults and typed config: update
AGENT_DEFAULT_PROVIDERS to include "refactoring_generator": "claude" (mirror
other agents' pattern) and add refactoring_generator: str to the
AgentModelConfig TypedDict so type-checkers recognize it; this ensures
get_provider_for_agent("refactoring_generator") is consistent with other agents
and avoids implicit fallbacks—search for AGENT_DEFAULT_PROVIDERS,
AgentModelConfig, and get_provider_for_agent to locate the exact insertion
points.
Comment on lines
+76
to
+106
| **Extract Method** | ||
| ```python | ||
| # Before - long function doing too many things | ||
| def process_order(order): | ||
| # validate | ||
| if not order.get("id"): | ||
| raise ValueError("Missing order id") | ||
| if not order.get("items"): | ||
| raise ValueError("Empty order") | ||
| # calculate total | ||
| total = sum(item["price"] * item["qty"] for item in order["items"]) | ||
| tax = total * 0.1 | ||
| # persist | ||
| db.save({"order": order, "total": total + tax}) | ||
|
|
||
| # After - each responsibility is its own function | ||
| def _validate_order(order: dict) -> None: | ||
| if not order.get("id"): | ||
| raise ValueError("Missing order id") | ||
| if not order.get("items"): | ||
| raise ValueError("Empty order") | ||
|
|
||
| def _calculate_total(items: list[dict]) -> float: | ||
| subtotal = sum(item["price"] * item["qty"] for item in items) | ||
| return subtotal * 1.1 | ||
|
|
||
| def process_order(order: dict) -> None: | ||
| _validate_order(order) | ||
| total = _calculate_total(order["items"]) | ||
| db.save({"order": order, "total": total}) | ||
| ``` |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add blank lines before fenced code blocks.
Static analysis flagged multiple instances where fenced code blocks immediately follow headings without a blank line (MD031). This affects lines 77, 109, 130, 152, 177, 210, 267, and 279.
Example fix for Extract Method section
**Extract Method**
+
```python
# Before - long function doing too many thingsApply the same pattern to other code blocks after bold headings.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 77-77: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/prompts/refactoring_generator.md` around lines 76 - 106, The
MD031 lint errors come from fenced code blocks placed immediately after bold
headings; add a single blank line between each bold heading and the following
fenced code block throughout the file (apply to the "Extract Method" example and
all other sections flagged), e.g., ensure there is a blank line before the
triple-backtick block that contains the before/after code for functions like
_validate_order, _calculate_total, and process_order so each heading is
separated from its code fence.
Comment on lines
+304
to
+321
| ``` | ||
| ## Refactoring Summary | ||
|
|
||
| ### Changes Applied | ||
|
|
||
| | File | Lines Changed | Smell Addressed | Technique | | ||
| |------|--------------|-----------------|-----------| | ||
| | apps/backend/agents/foo.py | 45-120 | Long function | Extract Method | | ||
| | apps/backend/core/bar.py | 12-18 | Bare exception | Replace Bare Exception | | ||
|
|
||
| ### Tests | ||
|
|
||
| All X existing tests pass. Y new tests added for extracted helpers. | ||
|
|
||
| ### Deferred | ||
|
|
||
| - `apps/backend/agents/baz.py` - God class (350 lines): deferred due to missing test coverage; recommend adding tests before refactoring. | ||
| ``` |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add language identifier to the summary format code block.
The fenced code block at line 304 has no language specified (MD040). Adding markdown as the language will improve syntax highlighting.
Proposed fix
-```
+```markdown
## Refactoring Summary📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` | |
| ## Refactoring Summary | |
| ### Changes Applied | |
| | File | Lines Changed | Smell Addressed | Technique | | |
| |------|--------------|-----------------|-----------| | |
| | apps/backend/agents/foo.py | 45-120 | Long function | Extract Method | | |
| | apps/backend/core/bar.py | 12-18 | Bare exception | Replace Bare Exception | | |
| ### Tests | |
| All X existing tests pass. Y new tests added for extracted helpers. | |
| ### Deferred | |
| - `apps/backend/agents/baz.py` - God class (350 lines): deferred due to missing test coverage; recommend adding tests before refactoring. | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 304-304: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/prompts/refactoring_generator.md` around lines 304 - 321, Add
the missing language identifier to the fenced code block that starts with "##
Refactoring Summary": change the opening triple backticks to "```markdown" so
the block becomes ```markdown followed by the existing content; update the one
occurrence of the plain fenced block in the file (the block containing the table
and deferred notes) to include the markdown language tag for proper syntax
highlighting.
Comment on lines
+56
to
+61
| spec = importlib.util.spec_from_file_location( | ||
| "agents.refactoring_generator", module_path | ||
| ) | ||
| module = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(module) | ||
| return module |
Contributor
There was a problem hiding this comment.
Add null check for spec before accessing spec.loader.
importlib.util.spec_from_file_location can return None if the file doesn't exist or isn't a valid module. Dereferencing spec.loader without checking could raise AttributeError.
Proposed fix
spec = importlib.util.spec_from_file_location(
"agents.refactoring_generator", module_path
)
+ if spec is None or spec.loader is None:
+ raise ImportError(f"Could not load module from {module_path}")
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return module🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/tests/test_refactoring_generator.py` around lines 56 - 61,
importlib.util.spec_from_file_location can return None; before accessing
spec.loader or calling spec.loader.exec_module(module) add a null check for the
returned spec (from spec = importlib.util.spec_from_file_location(...)). If spec
is None, raise a clear exception or return a meaningful error value (e.g.,
FileNotFoundError or ImportError) with context about module_path; otherwise
continue with module = importlib.util.module_from_spec(spec) and
spec.loader.exec_module(module). Ensure you reference spec, spec.loader, and
module_path in the error message so the failure is diagnosable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Create a new refactoring generator agent following the same pattern as test_generator.py and documentation_generator.py. This agent would analyze code and suggest or apply automated refactoring based on detected code smells and anti-patterns.
Summary by CodeRabbit
New Features
Tests