Skip to content

fix: delete orphaned DB records when deleting a thread#2803

Open
farukonfly wants to merge 3 commits into
bytedance:mainfrom
farukonfly:fix/thread-delete-orphaned-db-records
Open

fix: delete orphaned DB records when deleting a thread#2803
farukonfly wants to merge 3 commits into
bytedance:mainfrom
farukonfly:fix/thread-delete-orphaned-db-records

Conversation

@farukonfly
Copy link
Copy Markdown

fix: delete orphaned DB records when deleting a thread

Problem

DELETE /api/threads/{id} cleaned up filesystem data but left orphaned rows in the database — runs, run events, and feedback records remained after the thread was deleted.

Changes

  • routers/threads.py: after filesystem/checkpoint/thread-meta cleanup, also call delete_by_thread on run_store, run_event_store, and feedback_repo (all best-effort, errors are logged and swallowed)
  • persistence/run/sql.py: add RunRepository.delete_by_thread
  • persistence/feedback/sql.py: add FeedbackRepository.delete_by_thread
  • Both use SELECT COUNT + DELETE instead of rowcount to work reliably on both SQLite and PostgreSQL

Tests

  • TestMemoryRunStoreDeleteByThread — 3 unit tests for in-memory store
  • TestRunRepositoryDeleteByThread — 4 parametrized tests (SQLite + PostgreSQL)
  • TestFeedbackRepositoryDeleteByThread — 4 parametrized tests (SQLite + PostgreSQL)
  • 5 router-level tests verifying the DELETE endpoint calls all three cleanup methods

PostgreSQL tests skip automatically when PG_TEST_URL is not set.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 8, 2026

CLA assistant check
All committers have signed the CLA.

@farukonfly farukonfly force-pushed the fix/thread-delete-orphaned-db-records branch from f500c3b to 889c7a8 Compare May 8, 2026 17:14
- Add delete_by_thread() method to RunStore base class
- Implement delete_by_thread() in MemoryRunStore (in-memory)
- Implement delete_by_thread() in RunRepository (SQLAlchemy) with SQLite/PostgreSQL compatibility
- Implement delete_by_thread() in FeedbackRepository (SQLAlchemy) with SQLite/PostgreSQL compatibility
- Update DELETE /{thread_id} endpoint to clean up run and feedback records
- Use SELECT COUNT + DELETE pattern instead of result.rowcount for cross-database compatibility
@farukonfly farukonfly force-pushed the fix/thread-delete-orphaned-db-records branch from 889c7a8 to 794442b Compare May 8, 2026 17:15
- TestMemoryRunStoreDeleteByThread: 3 tests for in-memory RunStore
- TestRunRepositoryDeleteByThread: 4 parametrized tests (SQLite + PG)
- TestFeedbackRepositoryDeleteByThread: 4 parametrized tests (SQLite + PG)
- 5 router-level tests verifying DELETE /api/threads/{id} calls
  delete_by_thread on run_store, event_store, and feedback_repo

PG tests gate on PG_TEST_URL env var; skip automatically when absent.
@farukonfly farukonfly force-pushed the fix/thread-delete-orphaned-db-records branch from 794442b to e043bea Compare May 8, 2026 17:16
@WillemJiang
Copy link
Copy Markdown
Collaborator

@farukonfly thanks for your contribution. Please click the CLA button to sign the CLA before we review your patch.

@farukonfly
Copy link
Copy Markdown
Author

@farukonfly thanks for your contribution. Please click the CLA button to sign the CLA before we review your patch.

I have already signed the CLA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants