Skip to content

Commit 41aca69

Browse files
author
Test User
committed
fix(#247): address additional code review feedback
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)
1 parent 121dc59 commit 41aca69

File tree

4 files changed

+144
-30
lines changed

4 files changed

+144
-30
lines changed

codeframe/persistence/database.py

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -720,9 +720,15 @@ def delete_discovery_answers(self, *args, **kwargs):
720720
return self.activities.delete_discovery_answers(*args, **kwargs)
721721

722722
def delete_project_tasks_and_issues(self, project_id: int) -> dict:
723-
"""Delete all tasks and issues for a project.
723+
"""Delete all tasks and issues for a project atomically.
724724
725-
Performs cascading delete: tasks first (due to FK constraint), then issues.
725+
Performs cascading delete in a single transaction:
726+
1. Deletes task dependencies, test results, correction attempts
727+
2. Deletes tasks (code_reviews and task_evidence cascade automatically)
728+
3. Deletes issues
729+
730+
This method delegates to TaskRepository and IssueRepository for proper
731+
separation of concerns and handles all FK constraints correctly.
726732
727733
Args:
728734
project_id: Project ID
@@ -733,34 +739,34 @@ def delete_project_tasks_and_issues(self, project_id: int) -> dict:
733739
with self._sync_lock:
734740
cursor = self.conn.cursor()
735741

736-
# Count tasks before deletion
737-
cursor.execute(
738-
"SELECT COUNT(*) FROM tasks WHERE project_id = ?",
739-
(project_id,),
740-
)
741-
task_count = cursor.fetchone()[0]
742-
743-
# Count issues before deletion
744-
cursor.execute(
745-
"SELECT COUNT(*) FROM issues WHERE project_id = ?",
746-
(project_id,),
747-
)
748-
issue_count = cursor.fetchone()[0]
749-
750-
# Delete tasks first (due to FK constraint on issue_id)
751-
cursor.execute(
752-
"DELETE FROM tasks WHERE project_id = ?",
753-
(project_id,),
754-
)
755-
756-
# Then delete issues
757-
cursor.execute(
758-
"DELETE FROM issues WHERE project_id = ?",
759-
(project_id,),
760-
)
761-
762-
self.conn.commit()
763-
return {"tasks": task_count, "issues": issue_count}
742+
try:
743+
# Count before deletion for return value
744+
cursor.execute(
745+
"SELECT COUNT(*) FROM tasks WHERE project_id = ?",
746+
(project_id,),
747+
)
748+
task_count = cursor.fetchone()[0]
749+
750+
cursor.execute(
751+
"SELECT COUNT(*) FROM issues WHERE project_id = ?",
752+
(project_id,),
753+
)
754+
issue_count = cursor.fetchone()[0]
755+
756+
# Delete tasks first with all FK dependencies (single transaction)
757+
# Pass cursor to avoid intermediate commits
758+
self.tasks.delete_all_project_tasks(project_id, cursor=cursor)
759+
760+
# Then delete issues
761+
self.issues.delete_all_project_issues(project_id, cursor=cursor)
762+
763+
# Commit the entire operation atomically
764+
self.conn.commit()
765+
return {"tasks": task_count, "issues": issue_count}
766+
767+
except Exception:
768+
self.conn.rollback()
769+
raise
764770

765771
def create_audit_log(self, *args, **kwargs):
766772
"""Delegate to audit_logs.create_audit_log()."""

codeframe/persistence/repositories/issue_repository.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,3 +465,38 @@ def get_issue_completion_status(self, issue_id: int) -> Dict[str, Any]:
465465
"completion_percentage": completion_percentage,
466466
}
467467

468+
def delete_all_project_issues(self, project_id: int, cursor: sqlite3.Cursor = None) -> int:
469+
"""Delete all issues for a project.
470+
471+
Note: Tasks must be deleted before issues due to FK constraints.
472+
Use Database.delete_project_tasks_and_issues() for the complete
473+
cascading delete operation.
474+
475+
Args:
476+
project_id: Project ID to delete issues for
477+
cursor: Optional cursor for transaction support. If provided,
478+
the caller is responsible for commit/rollback.
479+
480+
Returns:
481+
Number of issues deleted
482+
"""
483+
own_cursor = cursor is None
484+
if own_cursor:
485+
cursor = self.conn.cursor()
486+
487+
try:
488+
cursor.execute(
489+
"DELETE FROM issues WHERE project_id = ?",
490+
(project_id,),
491+
)
492+
issue_count = cursor.rowcount
493+
494+
if own_cursor:
495+
self.conn.commit()
496+
497+
return issue_count
498+
499+
except Exception:
500+
if own_cursor:
501+
self.conn.rollback()
502+
raise

codeframe/persistence/repositories/task_repository.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -918,5 +918,77 @@ def _row_to_evidence(self, row: sqlite3.Row) -> "Evidence":
918918
verification_errors=verification_errors if verification_errors else None,
919919
)
920920

921+
def delete_all_project_tasks(self, project_id: int, cursor: sqlite3.Cursor = None) -> int:
922+
"""Delete all tasks for a project, handling FK constraints.
923+
924+
This method deletes all dependent records before deleting tasks:
925+
- task_dependencies (both task_id and depends_on_task_id)
926+
- test_results
927+
- correction_attempts
928+
929+
Note: code_reviews and task_evidence have ON DELETE CASCADE and are handled
930+
automatically by the database.
931+
932+
Args:
933+
project_id: Project ID to delete tasks for
934+
cursor: Optional cursor for transaction support. If provided,
935+
the caller is responsible for commit/rollback.
936+
937+
Returns:
938+
Number of tasks deleted
939+
"""
940+
own_cursor = cursor is None
941+
if own_cursor:
942+
cursor = self.conn.cursor()
943+
944+
try:
945+
# Get all task IDs for this project
946+
cursor.execute(
947+
"SELECT id FROM tasks WHERE project_id = ?",
948+
(project_id,),
949+
)
950+
task_ids = [row[0] for row in cursor.fetchall()]
951+
952+
if not task_ids:
953+
return 0
954+
955+
# Create placeholders for IN clause
956+
placeholders = ",".join("?" * len(task_ids))
957+
958+
# Delete from task_dependencies (both FK columns)
959+
cursor.execute(
960+
f"DELETE FROM task_dependencies WHERE task_id IN ({placeholders}) OR depends_on_task_id IN ({placeholders})",
961+
task_ids + task_ids,
962+
)
963+
964+
# Delete from test_results
965+
cursor.execute(
966+
f"DELETE FROM test_results WHERE task_id IN ({placeholders})",
967+
task_ids,
968+
)
969+
970+
# Delete from correction_attempts
971+
cursor.execute(
972+
f"DELETE FROM correction_attempts WHERE task_id IN ({placeholders})",
973+
task_ids,
974+
)
975+
976+
# Now delete the tasks (code_reviews and task_evidence cascade automatically)
977+
cursor.execute(
978+
"DELETE FROM tasks WHERE project_id = ?",
979+
(project_id,),
980+
)
981+
task_count = cursor.rowcount
982+
983+
if own_cursor:
984+
self.conn.commit()
985+
986+
return task_count
987+
988+
except Exception:
989+
if own_cursor:
990+
self.conn.rollback()
991+
raise
992+
921993
# Code Review CRUD operations (Sprint 10: 015-review-polish)
922994

web-ui/src/components/DiscoveryProgress.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ const DiscoveryProgress = memo(function DiscoveryProgress({ projectId, onViewPRD
288288
setShowRestartConfirmation(false);
289289
setConfirmationData(null);
290290
setWaitingForQuestionStart(null);
291+
setIsRestarting(false);
291292
fetchProgress();
292293
} catch (err) {
293294
console.error('Failed to restart discovery:', err);

0 commit comments

Comments
 (0)