feat(discovery): allow restart from any phase with confirmation (#247) #268
Conversation
Previously, the /restart endpoint only allowed restarting discovery when the project was in the "discovery" phase. After PRD generation, the project transitions to "planning" phase, making the "Restart Discovery" button non-functional (returning 400 error). This change enables full discovery restart from any project phase: Backend changes: - Modified /restart endpoint to support restart from any phase - Added confirmation requirement for non-discovery phases (returns requires_confirmation: true with data_to_be_deleted info) - Added full_reset_discovery() method to LeadAgent that clears: - Discovery answers - Conversation history - Category coverage data - Added delete_prd() and delete_discovery_answers() to ActivityRepository - Added delete_project_tasks_and_issues() to Database facade - Added get_memories_by_category() to MemoryRepository - Updated VALID_TRANSITIONS to allow active→discovery and review→discovery Frontend changes: - Updated restartDiscovery API to accept optional confirmed parameter - Added confirmation dialog showing what data will be deleted - Users can cancel or confirm the restart action Test coverage: - Added 16 new backend tests covering: - Restart from planning/active/review phases - Confirmation requirement flow - Data cleanup verification (PRD, tasks, issues, answers) - Phase manager transitions - Database methods - Updated existing frontend tests for new API signature Closes #247
WalkthroughAdds a full discovery reset path: backend LeadAgent gains Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as DiscoveryProgress UI
participant API as /api/projects/:id/discovery/restart
participant DB as Database
participant Agent as LeadAgent
User->>UI: Click Restart Discovery
UI->>API: POST /discovery/restart (confirmed=false)
API->>DB: GET project phase
alt phase == discovery
API->>Agent: reset_discovery()
Agent->>DB: delete discovery answers/turns/coverage
DB-->>API: OK
API-->>UI: {success, state: "idle"}
else
API->>DB: count PRD, answers, tasks, issues
DB-->>API: counts
API-->>UI: {requires_confirmation, data_to_be_deleted}
UI->>User: Show confirmation dialog
User->>UI: Confirm
UI->>API: POST /discovery/restart (confirmed=true)
API->>Agent: full_reset_discovery()
Agent->>DB: delete answers, turns, coverage
API->>DB: delete_prd(), delete_project_tasks_and_issues()
API->>DB: update project phase -> discovery
DB-->>API: cleared_items
API-->>UI: {success, cleared_items}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Add confirmation-gated restart of discovery from any phase and wire UI/API to perform full reset and phase transition when confirmedIntroduce 📍Where to StartStart with the Macroscope summarized 41aca69. |
Code Review: feat(discovery): allow restart from any phase with confirmationOverall AssessmentThis is a well-implemented feature that adds important functionality - allowing users to restart discovery from any phase with appropriate safeguards. The code is well-tested (16 new backend tests), properly documented, and follows existing patterns. ✅ Strengths
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| await performRestartDiscovery(true); | ||
| }; |
There was a problem hiding this comment.
Restart UX hides failures. Suggest keeping the confirmation dialog open until performRestartDiscovery succeeds, and surface restartError in completed views so users see restart failures.
| await performRestartDiscovery(true); | |
| }; | |
| await performRestartDiscovery(true); |
🚀 Want me to fix this? Reply ex: "fix it for me".
codeframe/persistence/database.py
Outdated
| ) | ||
| issue_count = cursor.fetchone()[0] | ||
|
|
||
| # Delete tasks first (due to FK constraint on issue_id) |
There was a problem hiding this comment.
Deletes will violate FKs. Suggest deleting dependent rows first (task_dependencies both directions, correction_attempts, test_results, git_branches), or add/document ON DELETE CASCADE on those FKs.
+ # Delete dependent rows to satisfy foreign key constraints
+ cursor.execute(
+ "DELETE FROM task_dependencies WHERE task_id IN (SELECT id FROM tasks WHERE project_id = ?)",
+ (project_id,),
+ )
+ cursor.execute(
+ "DELETE FROM task_dependencies WHERE depends_on_task_id IN (SELECT id FROM tasks WHERE project_id = ?)",
+ (project_id,),
+ )
+ cursor.execute(
+ "DELETE FROM correction_attempts WHERE task_id IN (SELECT id FROM tasks WHERE project_id = ?)",
+ (project_id,),
+ )
+ cursor.execute(
+ "DELETE FROM test_results WHERE task_id IN (SELECT id FROM tasks WHERE project_id = ?)",
+ (project_id,),
+ )🚀 Want me to fix this? Reply ex: "fix it for me".
| "updated_at": updated_at, | ||
| } | ||
|
|
||
| def delete_prd(self, project_id: int) -> bool: |
There was a problem hiding this comment.
These methods use self.conn/commit directly, bypassing BaseRepository helpers. Suggest using _execute(...) and _commit() so async-only conns fail predictably and _sync_lock applies.
🚀 Want me to fix this? Reply ex: "fix it for me".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @codeframe/agents/lead_agent.py:
- Around line 688-783: full_reset_discovery is trying to clear an AnswerCapture
instance using the wrong attribute name and a non-existent clear() method;
update the check in full_reset_discovery to use self.answer_capture (not
self._answer_capture) and implement a clear(self) method on the AnswerCapture
class that resets all its internal state (e.g., empty lists/dicts and any
counters used by capture_answer, extract_features, extract_users,
extract_constraints, get_structured_data, and _clean_feature_text) so calling
self.answer_capture.clear() fully clears captured answers and features.
🧹 Nitpick comments (6)
codeframe/core/phase_manager.py (1)
44-49: Consider updating the docstring to reflect new transitions.The class docstring still shows only the original transition rules and doesn't mention the new
active → discoveryandreview → discoverypaths added in this PR.📝 Suggested docstring update
Phase Transition Rules: discovery → planning (forward only) planning → active, discovery (forward or back) - active → review, planning (forward or back) - review → complete, active (forward or back) + active → review, planning, discovery (forward, back, or restart) + review → complete, active, discovery (forward, back, or restart) complete → (terminal state)codeframe/persistence/database.py (1)
722-763: Thread safety: Missing_sync_lockacquisition for shared connection access.The
delete_project_tasks_and_issuesmethod directly accessesself.connwithout acquiringself._sync_lock, while other direct SQL operations in this class should use the lock for thread-safe access. The class docstring and__init__explicitly set up_sync_lockfor this purpose, and repositories receive it for thread-safe operations.Additionally, consider delegating to repository methods (e.g.,
self.tasksandself.issues) for consistency with the facade pattern, rather than executing SQL directly here.♻️ Suggested fix: Add lock acquisition
def delete_project_tasks_and_issues(self, project_id: int) -> dict: """Delete all tasks and issues for a project. Performs cascading delete: tasks first (due to FK constraint), then issues. Args: project_id: Project ID Returns: Dictionary with counts: {"tasks": int, "issues": int} """ - cursor = self.conn.cursor() + with self._sync_lock: + cursor = self.conn.cursor() - # Count tasks before deletion - cursor.execute( - "SELECT COUNT(*) FROM tasks WHERE project_id = ?", - (project_id,), - ) - task_count = cursor.fetchone()[0] + # Count tasks before deletion + cursor.execute( + "SELECT COUNT(*) FROM tasks WHERE project_id = ?", + (project_id,), + ) + task_count = cursor.fetchone()[0] - # Count issues before deletion - cursor.execute( - "SELECT COUNT(*) FROM issues WHERE project_id = ?", - (project_id,), - ) - issue_count = cursor.fetchone()[0] + # Count issues before deletion + cursor.execute( + "SELECT COUNT(*) FROM issues WHERE project_id = ?", + (project_id,), + ) + issue_count = cursor.fetchone()[0] - # Delete tasks first (due to FK constraint on issue_id) - cursor.execute( - "DELETE FROM tasks WHERE project_id = ?", - (project_id,), - ) + # Delete tasks first (due to FK constraint on issue_id) + cursor.execute( + "DELETE FROM tasks WHERE project_id = ?", + (project_id,), + ) - # Then delete issues - cursor.execute( - "DELETE FROM issues WHERE project_id = ?", - (project_id,), - ) + # Then delete issues + cursor.execute( + "DELETE FROM issues WHERE project_id = ?", + (project_id,), + ) - self.conn.commit() - return {"tasks": task_count, "issues": issue_count} + self.conn.commit() + return {"tasks": task_count, "issues": issue_count}web-ui/src/components/DiscoveryProgress.tsx (1)
1106-1157: Consider handlingonOpenChangeto trigger cancel behavior.When the user closes the dialog by clicking outside or pressing Escape,
onOpenChangefires withfalse, but the current implementation only updatesshowRestartConfirmation. This leavesconfirmationDatapopulated andisRestartingpotentially in an inconsistent state. Consider usinghandleCancelRestartfor theonOpenChangehandler to ensure consistent cleanup.♻️ Suggested improvement
- <Dialog open={showRestartConfirmation} onOpenChange={setShowRestartConfirmation}> + <Dialog open={showRestartConfirmation} onOpenChange={(open) => !open && handleCancelRestart()}>tests/api/test_discovery_restart.py (2)
244-284: Consider verifying the fullcleared_itemsstructure.The test only asserts
prd_existed, but the API returns additional fields (answers,tasks,issues). Consider adding assertions for the complete structure to ensure the response contract is fully validated.Example assertions to add
assert "cleared_items" in data assert data["cleared_items"]["prd_existed"] is True + # Verify full structure + assert "answers" in data["cleared_items"] + assert "tasks" in data["cleared_items"] + assert "issues" in data["cleared_items"]
471-491: Remove unusedapi_clientfixture parameter.These tests call
PhaseManager.can_transition()which is a pure static method that doesn't require database access. Theapi_clientfixture triggers unnecessary database setup/teardown for each test.Proposed fix
class TestPhaseManagerTransitions: """Test phase manager allows transitions back to discovery from any phase.""" - def test_active_to_discovery_transition_allowed(self, api_client): + def test_active_to_discovery_transition_allowed(self): """Test transition from active to discovery is allowed.""" from codeframe.core.phase_manager import PhaseManager assert PhaseManager.can_transition("active", "discovery") is True - def test_review_to_discovery_transition_allowed(self, api_client): + def test_review_to_discovery_transition_allowed(self): """Test transition from review to discovery is allowed.""" from codeframe.core.phase_manager import PhaseManager assert PhaseManager.can_transition("review", "discovery") is True - def test_planning_to_discovery_transition_already_allowed(self, api_client): + def test_planning_to_discovery_transition_already_allowed(self): """Test transition from planning to discovery is already allowed.""" from codeframe.core.phase_manager import PhaseManager # This should already be allowed per existing VALID_TRANSITIONS assert PhaseManager.can_transition("planning", "discovery") is Truecodeframe/ui/routers/discovery.py (1)
586-641: Consolidate database commits to ensure atomicity across the reset operation sequence.The sequence of operations (lines 590-599) —
full_reset_discovery(),delete_prd(),delete_project_tasks_and_issues(), andupdate_project()— each callcommit()individually. If any operation fails after a previous commit, the system is left in an inconsistent state where some data has been cleared but not all.Defer all commits until all operations complete successfully, or wrap the entire sequence in a single transaction. Additionally, this code violates the coding guideline to use domain-specific repositories; consider refactoring to use repositories from
codeframe/persistence/repositories/instead of direct database method calls.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
codeframe/agents/lead_agent.pycodeframe/core/phase_manager.pycodeframe/persistence/database.pycodeframe/persistence/repositories/activity_repository.pycodeframe/persistence/repositories/memory_repository.pycodeframe/ui/routers/discovery.pytests/api/test_discovery_restart.pyweb-ui/__tests__/components/DiscoveryProgress.error.test.tsxweb-ui/__tests__/components/DiscoveryProgress.test.tsxweb-ui/__tests__/integration/prd-button-sync.test.tsxweb-ui/src/components/DiscoveryProgress.tsxweb-ui/src/lib/api.ts
🧰 Additional context used
📓 Path-based instructions (5)
codeframe/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/**/*.py: Use Python 3.11+ for backend development
Use FastAPI framework for backend API development
Use SQLite with async support (aiosqlite) for database operations
Use tiktoken for token counting
Run linting with ruff check .
Import and use domain-specific repositories from codeframe/persistence/repositories/ for data access instead of direct database calls
Files:
codeframe/persistence/repositories/memory_repository.pycodeframe/persistence/database.pycodeframe/persistence/repositories/activity_repository.pycodeframe/agents/lead_agent.pycodeframe/ui/routers/discovery.pycodeframe/core/phase_manager.py
codeframe/persistence/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/persistence/**/*.py: Use Repository pattern for data access with domain-specific repositories in codeframe/persistence/repositories/
Use pre-production flattened v1.0 database schema with direct table creation (no migration system)
Maintain backward compatibility with existing imports and method signatures in Database repository migration
Files:
codeframe/persistence/repositories/memory_repository.pycodeframe/persistence/database.pycodeframe/persistence/repositories/activity_repository.py
web-ui/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web-ui/src/**/*.{ts,tsx}: Use TypeScript 5.3+ for frontend development
Use React 18 and Next.js 14 for frontend development
Use Tailwind CSS for styling with Nova design system template
Use shadcn/ui components from @/components/ui/ for UI elements
Use Hugeicons (@hugeicons/react) for all icons, never mix with lucide-react
Use Nova color palette variables (bg-card, text-foreground, etc.) instead of hardcoded color values
Use cn() utility for conditional CSS classes in React components
Use process.env.NEXT_PUBLIC_API_URL with fallback to http://localhost:8080 for API endpoint configuration
Include auth token as query parameter in WebSocket connections (?token=TOKEN)
Store auth tokens in localStorage with key 'auth_token' and include token in API requests via Authorization header
Files:
web-ui/src/lib/api.tsweb-ui/src/components/DiscoveryProgress.tsx
web-ui/src/lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Implement WebSocket automatic reconnection with exponential backoff (1s → 30s) and full state resync
Files:
web-ui/src/lib/api.ts
web-ui/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web-ui/src/components/**/*.{ts,tsx}: Use React Context with useReducer for centralized state management in Dashboard
Wrap AgentStateProvider with ErrorBoundary component for graceful error handling
Use React.memo on Dashboard sub-components and useMemo for derived state to optimize performance
Files:
web-ui/src/components/DiscoveryProgress.tsx
🧠 Learnings (7)
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to codeframe/persistence/**/*.py : Maintain backward compatibility with existing imports and method signatures in Database repository migration
Applied to files:
codeframe/persistence/database.pycodeframe/persistence/repositories/activity_repository.py
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to tests/e2e/**/*.{ts,test.ts} : Use TEST_PROJECT_IDS.ACTIVE for tests requiring pre-seeded active phase with agents
Applied to files:
web-ui/__tests__/components/DiscoveryProgress.error.test.tsxweb-ui/__tests__/components/DiscoveryProgress.test.tsxtests/api/test_discovery_restart.pyweb-ui/__tests__/integration/prd-button-sync.test.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to tests/**/*.{ts,tsx,test.ts} : Never use test.skip() inside test logic; skip at describe level or use separate test projects for different states
Applied to files:
web-ui/__tests__/components/DiscoveryProgress.test.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to tests/e2e/**/*.{ts,test.ts} : Use TEST_PROJECT_IDS.PLANNING for tests requiring pre-seeded planning phase tasks
Applied to files:
web-ui/__tests__/components/DiscoveryProgress.test.tsxweb-ui/__tests__/integration/prd-button-sync.test.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/lib/**/*.{ts,tsx} : Implement WebSocket automatic reconnection with exponential backoff (1s → 30s) and full state resync
Applied to files:
web-ui/src/components/DiscoveryProgress.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Use Hugeicons (hugeicons/react) for all icons, never mix with lucide-react
Applied to files:
web-ui/src/components/DiscoveryProgress.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to codeframe/persistence/**/*.py : Use Repository pattern for data access with domain-specific repositories in codeframe/persistence/repositories/
Applied to files:
codeframe/persistence/repositories/activity_repository.py
🧬 Code graph analysis (6)
codeframe/persistence/repositories/memory_repository.py (1)
codeframe/persistence/database.py (1)
get_memories_by_category(534-536)
web-ui/__tests__/components/DiscoveryProgress.error.test.tsx (1)
web-ui/__tests__/components/DiscoveryProgress.testutils.tsx (1)
mockRestartDiscovery(22-22)
web-ui/__tests__/components/DiscoveryProgress.test.tsx (1)
web-ui/__tests__/components/DiscoveryProgress.testutils.tsx (1)
mockRestartDiscovery(22-22)
tests/api/test_discovery_restart.py (3)
tests/api/conftest.py (1)
api_client(67-177)codeframe/persistence/repositories/memory_repository.py (2)
upsert_memory(48-87)get_memories_by_category(122-138)codeframe/persistence/repositories/activity_repository.py (2)
get_prd(70-125)delete_prd(127-158)
codeframe/persistence/repositories/activity_repository.py (1)
codeframe/persistence/database.py (2)
delete_prd(714-716)delete_discovery_answers(718-720)
codeframe/agents/lead_agent.py (3)
codeframe/persistence/database.py (2)
delete_discovery_answers(718-720)upsert_memory(522-524)codeframe/persistence/repositories/activity_repository.py (1)
delete_discovery_answers(160-191)codeframe/persistence/repositories/memory_repository.py (1)
upsert_memory(48-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Frontend Unit Tests
- GitHub Check: Backend Unit Tests
- GitHub Check: Macroscope - Correctness Check
- GitHub Check: E2E Smoke Tests (Chromium)
- GitHub Check: opencode-review
🔇 Additional comments (25)
codeframe/core/phase_manager.py (1)
17-23: LGTM! Valid transitions correctly extended for discovery restart.The changes properly enable restart-to-discovery from both
activeandreviewphases, which aligns with the PR objective.web-ui/__tests__/components/DiscoveryProgress.error.test.tsx (2)
122-124: LGTM! Test expectations correctly updated for new API signature.The test now expects
restartDiscovery(1, false)which aligns with the updated API that accepts an optionalconfirmedboolean parameter.
204-206: LGTM! Consistent with the updated restart API signature.web-ui/__tests__/components/DiscoveryProgress.test.tsx (2)
2554-2556: LGTM! Test correctly updated for the new restart API signature.The expectation now matches the updated
restartDiscovery(projectId, confirmed)signature.
2636-2638: LGTM! Consistent with the two-argument restart API.codeframe/persistence/repositories/memory_repository.py (1)
122-138: LGTM! Clean implementation following existing repository patterns.The new
get_memories_by_categorymethod:
- Uses parameterized queries for SQL safety
- Returns consistent
List[Dict[str, Any]]type matching sibling methods- Orders by
created_atconsistent withget_project_memoriesThis correctly supports the discovery restart feature's need to query category-specific memory data. As per coding guidelines, this follows the Repository pattern for data access.
web-ui/__tests__/integration/prd-button-sync.test.tsx (1)
390-392: LGTM! Integration test correctly updated for new restart API signature.The expectation now matches the updated
restartDiscovery(projectId, confirmed)signature, consistent with the other test file updates in this PR.codeframe/persistence/database.py (2)
534-537: LGTM!The delegation method follows the existing pattern for facade methods, maintaining backward compatibility with the monolithic Database API.
714-721: LGTM!Both delegation methods follow the consistent facade pattern used throughout the file.
codeframe/persistence/repositories/activity_repository.py (2)
127-158: LGTM!The
delete_prdmethod follows the existing repository pattern with proper parameterized queries, existence check before deletion, transaction commit, and logging.
160-191: LGTM!The
delete_discovery_answersmethod is well-structured, follows the same pattern asdelete_prd, and correctly returns the count of deleted items for use in the confirmation response.web-ui/src/components/DiscoveryProgress.tsx (3)
16-25: LGTM!Imports follow the coding guidelines: using
@hugeicons/reactfor icons (AlertDiamondIcon) and@/components/ui/for shadcn/ui components. Based on learnings, this correctly avoids mixing with lucide-react.
69-76: LGTM!State structure for the confirmation dialog is well-typed and matches the API response shape for
data_to_be_deleted.
265-316: LGTM!The restart logic correctly handles the confirmation flow:
- First call with
confirmed=falsechecks if confirmation is needed- If
requires_confirmationis true, shows dialog and exits early- Second call with
confirmed=trueproceeds with the actual restart- State cleanup is handled properly in both success and cancel paths
web-ui/src/lib/api.ts (1)
93-118: LGTM!The API signature update correctly:
- Adds the optional
confirmedparameter- Expands the response type to cover all backend response variants (
requires_confirmation,data_to_be_deleted,cleared_items)- Uses conditional params to avoid sending undefined values
- Maintains backward compatibility (confirmed is optional)
tests/api/test_discovery_restart.py (6)
10-14: LGTM!The helper function correctly extracts the database instance from the test client's app state for direct DB verification in tests.
17-100: LGTM!The tests for planning phase restart are well-structured, covering both the confirmation request flow (without
confirmed=True) and the successful reset flow (withconfirmed=True). The assertions properly verify:
- Response structure with
requires_confirmationanddata_to_be_deleted- Project phase reset to discovery after confirmation
- PRD and discovery answers are cleared
101-150: LGTM!Good coverage for active phase restart, verifying that tasks and issues are properly deleted and the project phase is reset to discovery.
172-198: LGTM!Good tests for discovery phase (no confirmation needed) and 404 handling for nonexistent projects.
286-403: LGTM!Comprehensive tests for the new database helper methods. Each test properly verifies:
- Return values (boolean for
delete_prd, count fordelete_discovery_answers, dict fordelete_project_tasks_and_issues)- Data actually removed from database after deletion
406-468: LGTM!Good tests for
LeadAgent.full_reset_discovery(). The pattern of creating a newLeadAgentinstance after reset (line 439) correctly verifies that the reset persisted to the database, not just in-memory state.codeframe/ui/routers/discovery.py (4)
480-514: LGTM!The docstring clearly documents the two-path confirmation flow and the data that will be deleted. Good alignment with PR objectives for Issue #247.
525-553: LGTM!The discovery phase branch correctly handles the simple reset case without requiring confirmation, maintaining backward compatibility for projects already in discovery phase.
555-584: LGTM!The confirmation request path correctly gathers counts of data that would be deleted and returns a structured response for the UI to display. The
data_to_be_deletedobject provides useful context for the confirmation dialog.
644-648: LGTM!Proper exception handling with logging and 500 status code for server errors during reset.
| def full_reset_discovery(self) -> dict: | ||
| """Fully reset discovery, clearing ALL discovery data. | ||
| This method performs a comprehensive reset that clears: | ||
| - Discovery answers (from discovery_answers category) | ||
| - Conversation history (conversation_turn_N entries) | ||
| - Category coverage data | ||
| - Discovery state (reset to idle) | ||
| Unlike reset_discovery(), this method clears all previously gathered | ||
| data, effectively starting discovery completely from scratch. | ||
| Returns: | ||
| Dictionary with counts of cleared items: | ||
| {"answers": int, "conversation_turns": int} | ||
| """ | ||
| logger.info(f"Performing full discovery reset for project {self.project_id}") | ||
|
|
||
| # Clear discovery answers from database | ||
| answer_count = self.db.delete_discovery_answers(self.project_id) | ||
|
|
||
| # Clear conversation history from database | ||
| cursor = self.db.conn.cursor() | ||
|
|
||
| # Count conversation turns before deletion | ||
| cursor.execute( | ||
| """ | ||
| SELECT COUNT(*) FROM memory | ||
| WHERE project_id = ? AND category = 'discovery_state' | ||
| AND key LIKE 'conversation_turn_%' | ||
| """, | ||
| (self.project_id,), | ||
| ) | ||
| turn_count = cursor.fetchone()[0] | ||
|
|
||
| # Delete conversation turns | ||
| cursor.execute( | ||
| """ | ||
| DELETE FROM memory | ||
| WHERE project_id = ? AND category = 'discovery_state' | ||
| AND key LIKE 'conversation_turn_%' | ||
| """, | ||
| (self.project_id,), | ||
| ) | ||
|
|
||
| # Delete category coverage | ||
| cursor.execute( | ||
| """ | ||
| DELETE FROM memory | ||
| WHERE project_id = ? AND category = 'discovery_state' | ||
| AND key = 'category_coverage' | ||
| """, | ||
| (self.project_id,), | ||
| ) | ||
|
|
||
| self.db.conn.commit() | ||
|
|
||
| # Reset in-memory state completely | ||
| self._discovery_state = "idle" | ||
| self._current_question_id = None | ||
| self._current_question_text = None | ||
| self._discovery_answers = {} | ||
| self._discovery_conversation_history = [] | ||
| self._category_coverage = {} | ||
| self._coverage_turn_count = 0 | ||
|
|
||
| # Clear DiscoveryAnswerCapture state if it exists | ||
| if hasattr(self, "_answer_capture") and self._answer_capture: | ||
| self._answer_capture.clear() | ||
|
|
||
| # Reset state in database | ||
| self.db.upsert_memory( | ||
| project_id=self.project_id, | ||
| category="discovery_state", | ||
| key="state", | ||
| value="idle", | ||
| ) | ||
| self.db.upsert_memory( | ||
| project_id=self.project_id, | ||
| category="discovery_state", | ||
| key="current_question_id", | ||
| value="", | ||
| ) | ||
| self.db.upsert_memory( | ||
| project_id=self.project_id, | ||
| category="discovery_state", | ||
| key="current_question_text", | ||
| value="", | ||
| ) | ||
|
|
||
| logger.info( | ||
| f"Full discovery reset complete for project {self.project_id}: " | ||
| f"cleared {answer_count} answers, {turn_count} conversation turns" | ||
| ) | ||
|
|
||
| return {"answers": answer_count, "conversation_turns": turn_count} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the __init__ method around line 84 to confirm the attribute name
sed -n '70,100p' codeframe/agents/lead_agent.pyRepository: frankbria/codeframe
Length of output: 1143
🏁 Script executed:
# Search for AnswerCapture class definition and its clear method
find . -name "*.py" -type f -exec grep -l "class AnswerCapture" {} \;Repository: frankbria/codeframe
Length of output: 95
🏁 Script executed:
# Also search for discovery/answers.py which is likely where AnswerCapture is defined
rg -n "class AnswerCapture|def clear" codeframe/discovery/answers.py -A 5Repository: frankbria/codeframe
Length of output: 328
🏁 Script executed:
# Get the full AnswerCapture class to find the clear() method
rg -A 200 "^class AnswerCapture:" codeframe/discovery/answers.py | head -250Repository: frankbria/codeframe
Length of output: 8365
🏁 Script executed:
# Search specifically for the clear() method in AnswerCapture
rg -n "def clear" codeframe/discovery/answers.pyRepository: frankbria/codeframe
Length of output: 45
🏁 Script executed:
# Also check the total line count to understand class size
wc -l codeframe/discovery/answers.pyRepository: frankbria/codeframe
Length of output: 97
🏁 Script executed:
# Get all method definitions in the AnswerCapture class
rg -n "^\s+def " codeframe/discovery/answers.pyRepository: frankbria/codeframe
Length of output: 547
🏁 Script executed:
# Let's check what line 755-756 in lead_agent.py actually calls
sed -n '754,757p' codeframe/agents/lead_agent.pyRepository: frankbria/codeframe
Length of output: 232
Two issues prevent proper reset of AnswerCapture state:
-
Incorrect attribute name: Line 755 checks
self._answer_capturebut__init__createsself.answer_capture(no underscore prefix). -
Missing
clear()method: TheAnswerCaptureclass does not define aclear()method. It only has:__init__,capture_answer,extract_features,extract_users,extract_constraints,get_structured_data, and_clean_feature_text.
Fix by correcting the attribute name and adding a clear() method to the AnswerCapture class:
🐛 Proposed fixes
In codeframe/agents/lead_agent.py (lines 755-756):
- if hasattr(self, "_answer_capture") and self._answer_capture:
- self._answer_capture.clear()
+ if hasattr(self, "answer_capture") and self.answer_capture:
+ self.answer_capture.clear()In codeframe/discovery/answers.py, add to AnswerCapture class:
+ def clear(self) -> None:
+ """Clear all captured answers."""
+ self.answers.clear()🤖 Prompt for AI Agents
In @codeframe/agents/lead_agent.py around lines 688 - 783, full_reset_discovery
is trying to clear an AnswerCapture instance using the wrong attribute name and
a non-existent clear() method; update the check in full_reset_discovery to use
self.answer_capture (not self._answer_capture) and implement a clear(self)
method on the AnswerCapture class that resets all its internal state (e.g.,
empty lists/dicts and any counters used by capture_answer, extract_features,
extract_users, extract_constraints, get_structured_data, and
_clean_feature_text) so calling self.answer_capture.clear() fully clears
captured answers and features.
Fixes: - Fix critical bug: use self.answer_capture (not self._answer_capture) in full_reset_discovery - Add clear() method to AnswerCapture class for proper state reset - Add thread safety lock (_sync_lock) in delete_project_tasks_and_issues - Update Dialog onOpenChange handler to call handleCancelRestart when closed - Update phase_manager.py docstring to document discovery restart transitions - Remove unused api_client fixture from TestPhaseManagerTransitions tests
| try: | ||
| # Initialize LeadAgent and perform full reset | ||
| agent = LeadAgent(project_id=project_id, db=db, api_key="dummy-key-for-reset") | ||
| reset_result = agent.full_reset_discovery() |
There was a problem hiding this comment.
The reset isn’t atomic. After agent.full_reset_discovery(), if db.delete_prd(...), db.delete_project_tasks_and_issues(...), or db.update_project(...) fails, the system can end up partially reset (answers cleared but PRD/tasks/issues/phase unchanged). Consider wrapping these operations in a single transaction so the reset succeeds or fully rolls back.
🚀 Want me to fix this? Reply ex: "fix it for me".
Code Review SummaryThis PR is well-implemented with comprehensive test coverage (16 new backend tests) and follows good coding practices. The feature allows restarting discovery from any phase with proper confirmation safeguards. Strengths
Issues Requiring Fix1. CRITICAL: Foreign Key Constraint ViolationsThe current delete_project_tasks_and_issues will fail when deleting tasks due to missing cascade deletes. Several tables reference tasks without ON DELETE CASCADE:
Fix: Delete dependent rows before tasks - see detailed SQL in the review comment 2. Non-Atomic OperationsThe reset sequence performs multiple individual commits. If any operation fails after a previous commit, the system is left in an inconsistent state. Fix: Consolidate into a single transaction or defer commits until all operations complete. 3. Repository Pattern ViolationPer CLAUDE.md guidelines, data access should use domain-specific repositories. The delete_project_tasks_and_issues method executes SQL directly instead of delegating to TaskRepository and IssueRepository. Recommendations4. Frontend Error HandlingWhen performRestartDiscovery(true) fails, confirmation dialog closes before error is shown. Users may not see restartError. 5. Test Coverage GapsTest test_restart_returns_cleared_items_count only asserts prd_existed. Should verify complete cleared_items structure (answers, tasks, issues counts). After Fixing FK ViolationsOnce the foreign key constraint issue is addressed, this PR will be ready for merge. The other issues are code quality improvements rather than blockers. Overall: Approve with requested changes - Fix FK constraint issue before merging. |
|
I've reviewed PR #268 and posted my feedback as a comment. Here's my assessment: SummaryStatus: Approve with requested changes The PR is well-implemented with:
Critical Issue to FixForeign Key Constraint Violations in The
Fix: Delete dependent rows before deleting tasks. See my detailed comment on the PR for the exact SQL. Other Issues
Once the foreign key constraint issue is addressed, this PR is ready to merge. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/src/components/DiscoveryProgress.tsx (1)
265-297: Success path doesn't resetisRestartingstate.When restart succeeds (lines 287-291),
isRestartingis never explicitly set tofalse. The code relies on thediscovery_resetWebSocket message (line 544) to clear this state. If WebSocket delivery is delayed or fails, buttons remain disabled indefinitely.🔧 Suggested fix
// Success - reset completed setShowRestartConfirmation(false); setConfirmationData(null); setWaitingForQuestionStart(null); + setIsRestarting(false); fetchProgress(); } catch (err) {
🧹 Nitpick comments (1)
codeframe/persistence/database.py (1)
722-763: Thread-safe cascading delete is well-implemented.Good use of
_sync_lockfor thread safety, and correct FK ordering (tasks before issues). The method appropriately lives in the Database facade as cross-repository orchestration.Consider adding logging for observability, similar to
delete_prdanddelete_discovery_answersinActivityRepository.📝 Optional: Add logging for observability
self.conn.commit() + logger.info(f"Deleted {task_count} tasks and {issue_count} issues for project {project_id}") return {"tasks": task_count, "issues": issue_count}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
codeframe/agents/lead_agent.pycodeframe/core/phase_manager.pycodeframe/discovery/answers.pycodeframe/persistence/database.pytests/api/test_discovery_restart.pyweb-ui/src/components/DiscoveryProgress.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- codeframe/agents/lead_agent.py
- tests/api/test_discovery_restart.py
🧰 Additional context used
📓 Path-based instructions (4)
codeframe/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/**/*.py: Use Python 3.11+ for backend development
Use FastAPI framework for backend API development
Use SQLite with async support (aiosqlite) for database operations
Use tiktoken for token counting
Run linting with ruff check .
Import and use domain-specific repositories from codeframe/persistence/repositories/ for data access instead of direct database calls
Files:
codeframe/discovery/answers.pycodeframe/core/phase_manager.pycodeframe/persistence/database.py
web-ui/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web-ui/src/**/*.{ts,tsx}: Use TypeScript 5.3+ for frontend development
Use React 18 and Next.js 14 for frontend development
Use Tailwind CSS for styling with Nova design system template
Use shadcn/ui components from @/components/ui/ for UI elements
Use Hugeicons (@hugeicons/react) for all icons, never mix with lucide-react
Use Nova color palette variables (bg-card, text-foreground, etc.) instead of hardcoded color values
Use cn() utility for conditional CSS classes in React components
Use process.env.NEXT_PUBLIC_API_URL with fallback to http://localhost:8080 for API endpoint configuration
Include auth token as query parameter in WebSocket connections (?token=TOKEN)
Store auth tokens in localStorage with key 'auth_token' and include token in API requests via Authorization header
Files:
web-ui/src/components/DiscoveryProgress.tsx
web-ui/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web-ui/src/components/**/*.{ts,tsx}: Use React Context with useReducer for centralized state management in Dashboard
Wrap AgentStateProvider with ErrorBoundary component for graceful error handling
Use React.memo on Dashboard sub-components and useMemo for derived state to optimize performance
Files:
web-ui/src/components/DiscoveryProgress.tsx
codeframe/persistence/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/persistence/**/*.py: Use Repository pattern for data access with domain-specific repositories in codeframe/persistence/repositories/
Use pre-production flattened v1.0 database schema with direct table creation (no migration system)
Maintain backward compatibility with existing imports and method signatures in Database repository migration
Files:
codeframe/persistence/database.py
🧠 Learnings (3)
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Use Hugeicons (hugeicons/react) for all icons, never mix with lucide-react
Applied to files:
web-ui/src/components/DiscoveryProgress.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/lib/**/*.{ts,tsx} : Implement WebSocket automatic reconnection with exponential backoff (1s → 30s) and full state resync
Applied to files:
web-ui/src/components/DiscoveryProgress.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to codeframe/persistence/**/*.py : Maintain backward compatibility with existing imports and method signatures in Database repository migration
Applied to files:
codeframe/persistence/database.py
🧬 Code graph analysis (3)
codeframe/discovery/answers.py (2)
codeframe/agents/agent_pool_manager.py (1)
clear(531-536)codeframe/agents/dependency_resolver.py (1)
clear(371-376)
web-ui/src/components/DiscoveryProgress.tsx (2)
web-ui/src/components/ui/dialog.tsx (6)
Dialog(110-110)DialogContent(115-115)DialogHeader(116-116)DialogTitle(118-118)DialogDescription(119-119)DialogFooter(117-117)web-ui/src/components/ui/button.tsx (1)
Button(56-56)
codeframe/persistence/database.py (2)
codeframe/persistence/repositories/memory_repository.py (1)
get_memories_by_category(122-138)codeframe/persistence/repositories/activity_repository.py (2)
delete_prd(127-158)delete_discovery_answers(160-191)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Backend Unit Tests
🔇 Additional comments (10)
codeframe/discovery/answers.py (1)
25-31: LGTM!The
clear()method follows the established pattern used elsewhere in the codebase (e.g.,DependencyResolver.clear(),AgentPoolManager.clear()). The implementation is simple and correct for resetting the internal state.codeframe/core/phase_manager.py (2)
20-21: LGTM!The transition updates correctly enable the restart-to-discovery flow from
activeandreviewphases while preserving existing forward transitions.
47-48: Documentation accurately reflects the new transition rules.The docstring updates clearly explain the restart paths back to discovery from
activeandreviewphases.web-ui/src/components/DiscoveryProgress.tsx (5)
16-25: LGTM!Imports correctly use Hugeicons (
@hugeicons/react) for theAlertDiamondIconand shadcn/ui components for the dialog. Based on learnings, the project requires Hugeicons exclusively for icons.
69-76: LGTM!The confirmation state structure correctly mirrors the
data_to_be_deletedtype from the API response.
304-308: LGTM!Closing the dialog immediately before awaiting the restart provides good UX feedback.
310-315: LGTM!The cancel handler properly resets all confirmation-related state.
1106-1157: LGTM!The confirmation dialog implementation is well-structured:
- Proper use of shadcn/ui Dialog components with accessibility support
AlertDiamondIconcorrectly sourced from Hugeicons- Conditional rendering only shows items that will actually be deleted
- Appropriate button variants (
outlinefor Cancel,destructivefor Restart)- Dialog closure via
onOpenChangecorrectly triggershandleCancelRestartcodeframe/persistence/database.py (2)
534-536: LGTM!Follows the established delegation pattern and maintains backward compatibility with the Database facade API. Based on learnings, backward compatibility is required.
714-720: LGTM!Both methods follow the delegation pattern and correctly route to
ActivityRepositorymethods.
Fixes: - Fix FK constraint violations: delete from task_dependencies, test_results, correction_attempts before deleting tasks - Make delete_project_tasks_and_issues atomic with single transaction - Follow repository pattern: delegate to TaskRepository.delete_all_project_tasks and IssueRepository.delete_all_project_issues - Fix isRestarting not cleared on success in performRestartDiscovery The delete operation now: 1. Uses _sync_lock for thread safety 2. Deletes all FK-dependent rows before tasks 3. Uses a shared cursor for atomic transaction 4. Commits only after all operations succeed (or rolls back on error)
Code Review SummaryI've reviewed the changes for allowing restart from any phase. The implementation is solid with good test coverage. Here are a few additional observations: 1. Repository Pattern Violation inLocation: The method uses direct cursor access to delete conversation turns and category coverage: Per CLAUDE.md guidelines, all data access should use domain-specific repositories from . While this works, it creates inconsistency with the rest of the codebase. Recommendation: Add a method to that handles deleting conversation turns and category coverage, then delegate from . 2. Partial AtomicityLocation: The reset sequence has multiple commits:
While is now atomic (good!), the overall operation is not. If fails after all other commits, the project is left in a partially reset state. Recommendation: Consider deferring all commits until all operations complete, or wrap the entire sequence in a single database transaction. 3. Minor: Unreachable CheckLocation: The schema defines for the phase field, so should never occur. This creates an unreachable code path. Recommendation: Remove the check for clarity, as database constraints prevent it. 4. Test Coverage GapLocation: The test for only verifies the agent state is idle, but doesn't check that conversation turns and category coverage were actually deleted from the database. Recommendation: Add assertions verifying the database cleanup: Overall AssessmentThe PR is well-implemented with:
The issues above are code quality improvements rather than blockers. The implementation is production-ready after addressing the atomicity concern. Status: ✅ Approve with requested changes |
Code Review SummaryI've reviewed the changes for allowing restart from any phase. The implementation is solid with good test coverage. Here are a few additional observations: 1. Repository Pattern Violation in full_reset_discoveryLocation: codeframe/agents/lead_agent.py:710-743 The full_reset_discovery method uses direct cursor access to delete conversation turns and category coverage. Per CLAUDE.md guidelines, all data access should use domain-specific repositories. While this works, it creates inconsistency with the rest of the codebase. Recommendation: Add a delete_discovery_state_memories() method to MemoryRepository that handles deleting conversation turns and category coverage, then delegate from full_reset_discovery(). 2. Partial AtomicityLocation: codeframe/ui/routers/discovery.py:590-599 The reset sequence has multiple commits - agent.full_reset_discovery(), db.delete_prd(), db.delete_project_tasks_and_issues(), and db.update_project() each commit separately. If update_project() fails after all other commits, the project is left in a partially reset state. Recommendation: Consider deferring all commits until all operations complete, or wrap the entire sequence in a single database transaction. 3. Minor: Unreachable None CheckLocation: codeframe/ui/routers/discovery.py:528 The phase check includes None which is unreachable since the schema defines DEFAULT 'discovery'. Recommendation: Remove the None check for clarity. 4. Test Coverage GapLocation: tests/api/test_discovery_restart.py:461-468 The test for full_reset_discovery() only verifies agent state is "idle" but doesn't check that conversation turns and category coverage were deleted from the database. Recommendation: Add assertions verifying database cleanup of conversation turns and category coverage. Overall Assessment: The PR is well-implemented with comprehensive test coverage, good two-phase confirmation flow, proper thread safety, and the AnswerCapture bug has been fixed. The issues above are code quality improvements rather than blockers. The implementation is production-ready after addressing the atomicity concern. Status: Approve with requested changes |
|
I've completed my review of PR #268 and posted a comment on the pull request. Here's a summary: Code Review SummaryThe PR implements a well-structured feature for restarting discovery from any phase with confirmation. The implementation is solid with comprehensive test coverage (16 new tests), good two-phase confirmation flow, proper thread safety, and the AnswerCapture bug has been fixed. Issues Found
Overall Status✅ Approve with requested changes - The issues are code quality improvements rather than blockers. The implementation is production-ready after addressing the atomicity concern. |
| cursor = self.conn.cursor() | ||
|
|
||
| try: | ||
| # Get all task IDs for this project |
There was a problem hiding this comment.
Large IN lists can exceed SQLite’s 999 parameter limit here. Consider using subqueries on project_id (e.g., DELETE ... WHERE task_id IN (SELECT id FROM tasks WHERE project_id = ?)), which avoids oversized parameter arrays.
🚀 Want me to fix this? Reply ex: "fix it for me".
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/src/components/DiscoveryProgress.tsx (1)
293-297:restartErroris not displayed after a confirmed restart failure.When a restart is triggered from the completed state (via the "Restart" or "Restart Discovery" buttons), and the confirmed restart fails,
restartErroris set but never shown to the user. The error message is currently only rendered inside the "stuck" state block (lines 706-713).Consider adding error feedback near the confirmation dialog or in the completed state section.
💡 Suggested approach
Add an error alert inside the dialog footer or as a toast notification when
restartErroris set:<DialogFooter className="gap-2 sm:gap-0"> + {restartError && ( + <div className="w-full text-sm text-destructive mb-2" role="alert"> + {restartError} + </div> + )} <Button variant="outline" onClick={handleCancelRestart}Alternatively, display the error near the restart buttons in the completed state.
🧹 Nitpick comments (1)
web-ui/src/components/DiscoveryProgress.tsx (1)
306-309: Consider adding double-click protection to the confirm button.While
performRestartDiscoveryhas a guard against concurrent calls (if (isRestarting) return), there's a brief window between clicking the confirm button and theisRestartingstate being updated where a fast double-click could queue multiple calls before the guard takes effect.♻️ Optional: Add local ref-based guard
+ const isRestartingRef = useRef(false); + const handleConfirmedRestart = async () => { + if (isRestartingRef.current) return; + isRestartingRef.current = true; setShowRestartConfirmation(false); - await performRestartDiscovery(true); + try { + await performRestartDiscovery(true); + } finally { + isRestartingRef.current = false; + } };Alternatively, rely on the existing
disabled={isRestarting}prop and React's batching—this is likely sufficient for typical user interactions.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
codeframe/persistence/database.pycodeframe/persistence/repositories/issue_repository.pycodeframe/persistence/repositories/task_repository.pyweb-ui/src/components/DiscoveryProgress.tsx
🧰 Additional context used
📓 Path-based instructions (4)
codeframe/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/**/*.py: Use Python 3.11+ for backend development
Use FastAPI framework for backend API development
Use SQLite with async support (aiosqlite) for database operations
Use tiktoken for token counting
Run linting with ruff check .
Import and use domain-specific repositories from codeframe/persistence/repositories/ for data access instead of direct database calls
Files:
codeframe/persistence/repositories/task_repository.pycodeframe/persistence/database.pycodeframe/persistence/repositories/issue_repository.py
codeframe/persistence/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
codeframe/persistence/**/*.py: Use Repository pattern for data access with domain-specific repositories in codeframe/persistence/repositories/
Use pre-production flattened v1.0 database schema with direct table creation (no migration system)
Maintain backward compatibility with existing imports and method signatures in Database repository migration
Files:
codeframe/persistence/repositories/task_repository.pycodeframe/persistence/database.pycodeframe/persistence/repositories/issue_repository.py
web-ui/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web-ui/src/**/*.{ts,tsx}: Use TypeScript 5.3+ for frontend development
Use React 18 and Next.js 14 for frontend development
Use Tailwind CSS for styling with Nova design system template
Use shadcn/ui components from @/components/ui/ for UI elements
Use Hugeicons (@hugeicons/react) for all icons, never mix with lucide-react
Use Nova color palette variables (bg-card, text-foreground, etc.) instead of hardcoded color values
Use cn() utility for conditional CSS classes in React components
Use process.env.NEXT_PUBLIC_API_URL with fallback to http://localhost:8080 for API endpoint configuration
Include auth token as query parameter in WebSocket connections (?token=TOKEN)
Store auth tokens in localStorage with key 'auth_token' and include token in API requests via Authorization header
Files:
web-ui/src/components/DiscoveryProgress.tsx
web-ui/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web-ui/src/components/**/*.{ts,tsx}: Use React Context with useReducer for centralized state management in Dashboard
Wrap AgentStateProvider with ErrorBoundary component for graceful error handling
Use React.memo on Dashboard sub-components and useMemo for derived state to optimize performance
Files:
web-ui/src/components/DiscoveryProgress.tsx
🧠 Learnings (3)
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to codeframe/persistence/**/*.py : Maintain backward compatibility with existing imports and method signatures in Database repository migration
Applied to files:
codeframe/persistence/database.py
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/**/*.{ts,tsx} : Use Hugeicons (hugeicons/react) for all icons, never mix with lucide-react
Applied to files:
web-ui/src/components/DiscoveryProgress.tsx
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to web-ui/src/lib/**/*.{ts,tsx} : Implement WebSocket automatic reconnection with exponential backoff (1s → 30s) and full state resync
Applied to files:
web-ui/src/components/DiscoveryProgress.tsx
🧬 Code graph analysis (1)
web-ui/src/components/DiscoveryProgress.tsx (2)
web-ui/src/components/ui/dialog.tsx (6)
Dialog(110-110)DialogContent(115-115)DialogHeader(116-116)DialogTitle(118-118)DialogDescription(119-119)DialogFooter(117-117)web-ui/src/components/ui/button.tsx (1)
Button(56-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Frontend Unit Tests
- GitHub Check: Backend Unit Tests
- GitHub Check: opencode-review
- GitHub Check: E2E Smoke Tests (Chromium)
- GitHub Check: Macroscope - Correctness Check
🔇 Additional comments (8)
codeframe/persistence/repositories/issue_repository.py (1)
468-502: LGTM! Well-structured transactional delete method.The implementation correctly handles:
- Optional cursor injection for transaction composition
- Proper commit/rollback semantics when owning the cursor
- Clear docstring documenting the FK constraint ordering dependency
The pattern aligns with the companion
delete_all_project_tasksmethod and integrates cleanly with theDatabase.delete_project_tasks_and_issuesorchestrator.codeframe/persistence/database.py (3)
534-537: LGTM! Consistent delegation pattern.The new
get_memories_by_categorydelegation follows the established facade pattern.
722-769: LGTM! Proper atomic cascading delete with thread-safety.Solid implementation:
_sync_lockensures thread-safe access to the shared connection- Single transaction spans both repository delete calls via shared cursor
- Pre-deletion counts accurately reflect what will be deleted
- Rollback on failure ensures data consistency
- Follows the repository pattern by delegating actual deletions to
TaskRepositoryandIssueRepositoryThe docstring clearly documents the cascading behavior and FK constraint handling.
714-720: LGTM! Maintains backward compatibility.The
delete_prdanddelete_discovery_answersdelegation methods correctly forward to their ActivityRepository counterparts. Both methods exist in ActivityRepository with compatible signatures and the delegation pattern properly maintains the monolithic Database API interface.codeframe/persistence/repositories/task_repository.py (1)
921-991: LGTM! Comprehensive cascading delete with proper FK handling.The implementation correctly:
- Collects task IDs upfront to handle dependent tables
- Deletes FK-dependent records (task_dependencies, test_results, correction_attempts) before tasks
- Uses parameterized queries safely—the f-string interpolates only
?placeholders, not user data- Relies on
ON DELETE CASCADEforcode_reviewsandtask_evidence(verified: both tables haveREFERENCES tasks(id) ON DELETE CASCADEin schema_manager.py)The early return when no tasks exist is a good optimization that avoids unnecessary queries.
web-ui/src/components/DiscoveryProgress.tsx (3)
16-25: LGTM!The imports correctly use Hugeicons (
@hugeicons/react) exclusively and leverage shadcn/ui Dialog and Button components as per the coding guidelines.
69-76: LGTM!State structure aligns with the API's
data_to_be_deletedresponse shape and is appropriately initialized.
1107-1158: LGTM!The confirmation dialog is well-structured:
- Properly uses shadcn/ui Dialog components with correct accessibility attributes
- Pluralization logic for items is correct
- Destructive variant on the confirm button clearly indicates the action's impact
- Loading state and disabled prop prevent duplicate submissions
Summary
Problem
Previously, the
/restartendpoint only allowed restarting discovery when the project was in the "discovery" phase. After PRD generation, the project transitions to "planning" phase, making the "Restart Discovery" button non-functional (returning 400 error).Solution
Backend Changes
/restartendpoint to support restart from any phaserequires_confirmation: truewithdata_to_be_deletedinfo)full_reset_discovery()method to LeadAgent that clears:delete_prd()- removes PRD entriesdelete_discovery_answers()- removes discovery answersdelete_project_tasks_and_issues()- removes tasks and issuesget_memories_by_category()- retrieves memories by categoryVALID_TRANSITIONSto allowactive→discoveryandreview→discoveryFrontend Changes
restartDiscoveryAPI to accept optionalconfirmedparameterTest plan
ruff check .)npm run lint)npm run type-check)npm run build)Manual Testing Scenarios
Closes #247
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.