-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: handle empty/greenfield projects in spec creation (#1426) #1841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f497416
251cef9
5ba13c8
e221371
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,18 +6,54 @@ | |
| """ | ||
|
|
||
| import json | ||
| from typing import TYPE_CHECKING | ||
| from pathlib import Path | ||
|
|
||
| from .. import validator, writer | ||
| from ..discovery import get_project_index_stats | ||
| from .models import MAX_RETRIES, PhaseResult | ||
|
|
||
| if TYPE_CHECKING: | ||
| pass | ||
|
|
||
| def _is_greenfield_project(spec_dir: Path) -> bool: | ||
| """Check if the project is empty/greenfield (0 discovered files).""" | ||
| stats = get_project_index_stats(spec_dir) | ||
| if not stats: | ||
| return False # Can't determine - don't assume greenfield | ||
| return stats.get("file_count", 0) == 0 | ||
|
|
||
|
|
||
| def _greenfield_context() -> str: | ||
| """Return additional context for greenfield/empty projects.""" | ||
| return """ | ||
| **GREENFIELD PROJECT**: This is an empty or new project with no existing code. | ||
| There are no existing files to reference or modify. You are creating everything from scratch. | ||
| Adapt your approach: | ||
| - Do NOT reference existing files, patterns, or code structures | ||
| - Focus on what needs to be CREATED, not modified | ||
| - Define the initial project structure, files, and directories | ||
| - Specify the tech stack, frameworks, and dependencies to install | ||
| - Provide setup instructions for the new project | ||
| - For "Files to Modify" and "Files to Reference" sections, list files to CREATE instead | ||
| - For "Patterns to Follow", describe industry best practices rather than existing code | ||
| """ | ||
|
|
||
|
|
||
| class SpecPhaseMixin: | ||
| """Mixin for spec writing and critique phase methods.""" | ||
|
|
||
| def _check_and_log_greenfield(self) -> bool: | ||
| """Check if the project is greenfield and log if so. | ||
| Returns: | ||
| True if the project is greenfield (no existing files). | ||
| """ | ||
| is_greenfield = _is_greenfield_project(self.spec_dir) | ||
| if is_greenfield: | ||
| self.ui.print_status( | ||
| "Greenfield project detected - adapting spec for new project", "info" | ||
| ) | ||
| return is_greenfield | ||
|
Comment on lines
+44
to
+55
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Greenfield detection not captured in the persistent task log.
♻️ Suggested addition def _check_and_log_greenfield(self) -> bool:
is_greenfield = _is_greenfield_project(self.spec_dir)
if is_greenfield:
self.ui.print_status(
"Greenfield project detected - adapting spec for new project", "info"
)
+ if hasattr(self, "task_logger") and self.task_logger is not None:
+ self.task_logger.log_info("Greenfield project detected: no existing files found")
return is_greenfield🤖 Prompt for AI Agents |
||
|
|
||
| async def phase_quick_spec(self) -> PhaseResult: | ||
| """Quick spec for simple tasks - combines context and spec in one step.""" | ||
| spec_file = self.spec_dir / "spec.md" | ||
|
|
@@ -29,6 +65,8 @@ async def phase_quick_spec(self) -> PhaseResult: | |
| "quick_spec", True, [str(spec_file), str(plan_file)], [], 0 | ||
| ) | ||
|
|
||
| is_greenfield = self._check_and_log_greenfield() | ||
|
|
||
| errors = [] | ||
| for attempt in range(MAX_RETRIES): | ||
| self.ui.print_status( | ||
|
|
@@ -42,7 +80,7 @@ async def phase_quick_spec(self) -> PhaseResult: | |
| This is a SIMPLE task. Create a minimal spec and implementation plan directly. | ||
| No research or extensive analysis needed. | ||
| {_greenfield_context() if is_greenfield else ""} | ||
| Create: | ||
| 1. A concise spec.md with just the essential sections | ||
| 2. A simple implementation_plan.json with 1-2 subtasks | ||
|
|
@@ -80,6 +118,9 @@ async def phase_spec_writing(self) -> PhaseResult: | |
| "spec.md exists but has issues, regenerating...", "warning" | ||
| ) | ||
|
|
||
| is_greenfield = self._check_and_log_greenfield() | ||
| greenfield_ctx = _greenfield_context() if is_greenfield else "" | ||
|
|
||
| errors = [] | ||
| for attempt in range(MAX_RETRIES): | ||
| self.ui.print_status( | ||
|
|
@@ -88,6 +129,7 @@ async def phase_spec_writing(self) -> PhaseResult: | |
|
|
||
| success, output = await self.run_agent_fn( | ||
| "spec_writer.md", | ||
| additional_context=greenfield_ctx, | ||
| phase_name="spec_writing", | ||
| ) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| """ | ||
|
|
||
| import json | ||
| import types | ||
| from collections.abc import Callable | ||
| from pathlib import Path | ||
|
|
||
|
|
@@ -18,6 +19,7 @@ | |
| from task_logger import ( | ||
| LogEntryType, | ||
| LogPhase, | ||
| TaskLogger, | ||
| get_task_logger, | ||
| ) | ||
| from ui import ( | ||
|
|
@@ -238,6 +240,9 @@ async def run(self, interactive: bool = True, auto_approve: bool = False) -> boo | |
| task_logger.start_phase(LogPhase.PLANNING, "Starting spec creation process") | ||
| TaskEventEmitter.from_spec_dir(self.spec_dir).emit("PLANNING_STARTED") | ||
|
|
||
| # Track whether we've already ended the planning phase (to avoid double-end) | ||
| self._planning_phase_ended = False | ||
|
|
||
| try: | ||
| return await self._run_phases(interactive, auto_approve, task_logger, ui) | ||
| except Exception as e: | ||
|
|
@@ -251,22 +256,24 @@ async def run(self, interactive: bool = True, auto_approve: bool = False) -> boo | |
| ) | ||
| except Exception: | ||
| pass # Don't mask the original error | ||
| try: | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, | ||
| success=False, | ||
| message=f"Spec creation crashed: {e}", | ||
| ) | ||
| except Exception: | ||
| pass # Best effort - don't mask the original error when logging fails | ||
| if not self._planning_phase_ended: | ||
| self._planning_phase_ended = True | ||
| try: | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, | ||
| success=False, | ||
| message=f"Spec creation crashed: {e}", | ||
| ) | ||
| except Exception: | ||
| pass # Best effort - don't mask the original error when logging fails | ||
| raise | ||
|
Comment on lines
243
to
269
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The To fix this,
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| async def _run_phases( | ||
| self, | ||
| interactive: bool, | ||
| auto_approve: bool, | ||
| task_logger, | ||
| ui, | ||
| task_logger: TaskLogger, | ||
| ui: types.ModuleType, | ||
| ) -> bool: | ||
| """Internal method that runs all spec creation phases. | ||
|
|
@@ -327,6 +334,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: | |
| results.append(result) | ||
| if not result.success: | ||
| print_status("Discovery failed", "error") | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, success=False, message="Discovery failed" | ||
| ) | ||
|
|
@@ -342,6 +350,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: | |
| results.append(result) | ||
| if not result.success: | ||
| print_status("Requirements gathering failed", "error") | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, | ||
| success=False, | ||
|
|
@@ -380,6 +389,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: | |
| results.append(result) | ||
| if not result.success: | ||
| print_status("Complexity assessment failed", "error") | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, success=False, message="Complexity assessment failed" | ||
| ) | ||
|
|
@@ -442,6 +452,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: | |
| f"Phase '{phase_name}' failed: {'; '.join(result.errors)}", | ||
| LogEntryType.ERROR, | ||
| ) | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, | ||
| success=False, | ||
|
|
@@ -456,6 +467,7 @@ def run_phase(name: str, phase_fn: Callable) -> phases.PhaseResult: | |
| self._print_completion_summary(results, phases_executed) | ||
|
|
||
| # End planning phase successfully | ||
| self._planning_phase_ended = True | ||
| task_logger.end_phase( | ||
| LogPhase.PLANNING, success=True, message="Spec creation complete" | ||
| ) | ||
|
|
@@ -729,9 +741,8 @@ def _run_review_checkpoint(self, auto_approve: bool) -> bool: | |
| print_status("Build will not proceed without approval.", "warning") | ||
| return False | ||
|
|
||
| except SystemExit as e: | ||
| if e.code != 0: | ||
| return False | ||
| except SystemExit: | ||
| # Review checkpoint may call sys.exit(); treat any exit as unapproved | ||
| return False | ||
|
Comment on lines
+744
to
746
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find the file and examine the context around lines 744-746
cat -n apps/backend/spec/pipeline/orchestrator.py | sed -n '730,760p'Repository: AndyMik90/Auto-Claude Length of output: 1362 🏁 Script executed: #!/bin/bash
# Search for run_review_checkpoint function definition and usage
rg -n 'def run_review_checkpoint' --type py -A 15Repository: AndyMik90/Auto-Claude Length of output: 1136 🏁 Script executed: #!/bin/bash
# Search for sys.exit patterns in the codebase
rg -n 'sys\.exit' --type py -B 2 -A 2Repository: AndyMik90/Auto-Claude Length of output: 40968 Fix The bare 🤖 Prompt for AI Agents |
||
| except KeyboardInterrupt: | ||
| print() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Good fix for the false-positive greenfield detection on missing/corrupt index.
The
if not stats: return Falseguard correctly prevents misclassifying projects whenget_project_index_statsreturns{}due to a missing or unparsable index file.One remaining edge case: if
project_index.jsonexists and parses as valid JSON but uses an unrecognized format (no"files"or"services"key),get_project_index_statsreturns{"file_count": 0, ...}— a non-empty dict that passes the guard and yieldsfile_count == 0 → True. This would be a false positive on a valid-but-unrecognized index format. Consider additionally checking that"file_count"is actually present instatsas a key (rather than relying on the default):Proposed hardening
def _is_greenfield_project(spec_dir: Path) -> bool: """Check if the project is empty/greenfield (0 discovered files).""" stats = get_project_index_stats(spec_dir) if not stats: return False # Can't determine - don't assume greenfield - return stats.get("file_count", 0) == 0 + if "file_count" not in stats: + return False # Unrecognized format - don't assume greenfield + return stats["file_count"] == 0🤖 Prompt for AI Agents