From 4750b47b04a483fcbfe9c5538629c641d42d2669 Mon Sep 17 00:00:00 2001 From: SEPURI-SAI-KRISHNA Date: Tue, 4 Aug 2026 09:53:19 +0530 Subject: [PATCH] fix(profiles): delete a profile's generations instead of orphaning them --- app/src/lib/hooks/useProfiles.ts | 4 + backend/services/history.py | 34 +++- backend/services/profiles.py | 26 ++- backend/tests/test_profile_delete_cascade.py | 190 +++++++++++++++++++ 4 files changed, 243 insertions(+), 11 deletions(-) create mode 100644 backend/tests/test_profile_delete_cascade.py diff --git a/app/src/lib/hooks/useProfiles.ts b/app/src/lib/hooks/useProfiles.ts index f05fd999..bd5a28da 100644 --- a/app/src/lib/hooks/useProfiles.ts +++ b/app/src/lib/hooks/useProfiles.ts @@ -51,6 +51,10 @@ export function useDeleteProfile() { mutationFn: (profileId: string) => apiClient.deleteProfile(profileId), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ['profiles'] }); + // Deleting a profile also deletes its generations, which story items + // reference — both caches still hold rows that no longer exist. + queryClient.invalidateQueries({ queryKey: ['history'] }); + queryClient.invalidateQueries({ queryKey: ['stories'] }); }, }); } diff --git a/backend/services/history.py b/backend/services/history.py index 6df36396..3d41d0ca 100644 --- a/backend/services/history.py +++ b/backend/services/history.py @@ -11,10 +11,23 @@ from sqlalchemy.orm import Session from sqlalchemy import or_ from ..models import GenerationRequest, GenerationResponse, HistoryQuery, HistoryResponse, HistoryListResponse, GenerationVersionResponse, EffectConfig -from ..database import Generation as DBGeneration, GenerationVersion as DBGenerationVersion, VoiceProfile as DBVoiceProfile +from ..database import Generation as DBGeneration, GenerationVersion as DBGenerationVersion, StoryItem as DBStoryItem, VoiceProfile as DBVoiceProfile from .. import config +def _delete_generation_children(generation_id: str, db: Session) -> None: + """Remove the rows that reference a generation, plus any version audio files. + + Story items and versions both point at the generation by a non-null FK. + The story detail query inner-joins generations, so a leftover story item + vanishes from the timeline while staying in the table forever. + """ + from . import versions as versions_mod + + db.query(DBStoryItem).filter_by(generation_id=generation_id).delete() + versions_mod.delete_versions_for_generation(generation_id, db) + + def _get_versions_for_generations(generation_ids: list[str], db: Session) -> dict: """Fetch versions for many generations in a single query. @@ -280,8 +293,7 @@ async def delete_generation( return False # Delete all version files and records - from . import versions as versions_mod - versions_mod.delete_versions_for_generation(generation_id, db) + _delete_generation_children(generation_id, db) # Delete main audio file (if not already removed by version cleanup) if generation.audio_path: @@ -307,13 +319,11 @@ async def delete_failed_generations(db: Session) -> int: Returns: Number of generations deleted. """ - from . import versions as versions_mod - failed = db.query(DBGeneration).filter(DBGeneration.status == "failed").all() count = 0 for generation in failed: # Clean up version files/rows first. - versions_mod.delete_versions_for_generation(generation.id, db) + _delete_generation_children(generation.id, db) # Remove the main audio file if it somehow made it to disk. if generation.audio_path: @@ -352,14 +362,18 @@ async def delete_generations_by_profile( count = 0 for generation in generations: # Delete associated version files and rows first - from . import versions as versions_mod - versions_mod.delete_versions_for_generation(generation.id, db) + _delete_generation_children(generation.id, db) # Delete audio file audio_path = config.resolve_storage_path(generation.audio_path) if audio_path is not None and audio_path.exists(): - audio_path.unlink() - + try: + audio_path.unlink() + except OSError: + # A file locked by playback shouldn't abort the whole sweep + # and leave the profile half-deleted. + pass + # Delete from database db.delete(generation) count += 1 diff --git a/backend/services/profiles.py b/backend/services/profiles.py index f9872995..e7762f32 100644 --- a/backend/services/profiles.py +++ b/backend/services/profiles.py @@ -11,7 +11,14 @@ from sqlalchemy import func from sqlalchemy.orm import Session from .. import config -from ..database import Generation as DBGeneration, ProfileSample as DBProfileSample, VoiceProfile as DBVoiceProfile +from ..database import ( + CaptureSettings as DBCaptureSettings, + Generation as DBGeneration, + MCPClientBinding as DBMCPClientBinding, + ProfileChannelMapping as DBProfileChannelMapping, + ProfileSample as DBProfileSample, + VoiceProfile as DBVoiceProfile, +) from ..models import ( EffectConfig, ProfileSampleResponse, @@ -21,6 +28,7 @@ from ..models import ( from ..utils.audio import save_audio, validate_and_load_reference_audio from ..utils.cache import _get_cache_dir, clear_profile_cache from ..utils.images import process_avatar, validate_image +from . import history logger = logging.getLogger(__name__) @@ -429,7 +437,23 @@ async def delete_profile( if not profile: return False + # Generations carry a non-null FK to the profile and the history query + # inner-joins profiles, so anything left behind here becomes a row the UI + # can never show and a .wav in data/generations the user can never reclaim. + deleted_generations = await history.delete_generations_by_profile(profile_id, db) + if deleted_generations: + logger.info("Deleted %d generations belonging to profile %s", deleted_generations, profile_id) + db.query(DBProfileSample).filter_by(profile_id=profile_id).delete() + db.query(DBProfileChannelMapping).filter_by(profile_id=profile_id).delete() + + # Nullable pointers at the profile — resolve_profile() already tolerates a + # dangling id, but leaving one behind makes the UI show an empty selection + # that the user can't clear. + db.query(DBMCPClientBinding).filter_by(profile_id=profile_id).update({"profile_id": None}) + db.query(DBCaptureSettings).filter_by(default_playback_voice_id=profile_id).update( + {"default_playback_voice_id": None} + ) db.delete(profile) db.commit() diff --git a/backend/tests/test_profile_delete_cascade.py b/backend/tests/test_profile_delete_cascade.py new file mode 100644 index 00000000..aed2e84f --- /dev/null +++ b/backend/tests/test_profile_delete_cascade.py @@ -0,0 +1,190 @@ +""" +Regression tests for DELETE /profiles/{id} cleaning up everything it owns. + +``delete_profile`` used to remove only the profile row, its samples, and the +profile directory. Generations carry a non-null FK to the profile and +``list_generations`` inner-joins profiles, so every generation made with a +deleted profile turned into a row the UI could never show plus a ``.wav`` in +``data/generations`` the user could never reclaim — unbounded disk growth with +no way out. Version rows, story items, and channel mappings leaked the same way. + +Usage: + python -m pytest backend/tests/test_profile_delete_cascade.py -v +""" + +import sys +from pathlib import Path + +import pytest +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +# Repo root on sys.path so ``backend`` imports as a package (the services use +# package-relative imports). +sys.path.insert(0, str(Path(__file__).parent.parent.parent)) + +from backend import config +from backend.database import ( + Base, + CaptureSettings, + Generation, + GenerationVersion, + MCPClientBinding, + ProfileChannelMapping, + ProfileSample, + Story, + StoryItem, + VoiceProfile, +) +from backend.models import HistoryQuery +from backend.services import history, profiles + + +@pytest.fixture +def db(tmp_path, monkeypatch): + """A session backed by a temp SQLite file, with the data dir redirected.""" + monkeypatch.setattr(config, "_data_dir", tmp_path) + + engine = create_engine(f"sqlite:///{tmp_path / 'test.db'}") + Base.metadata.create_all(bind=engine) + session = sessionmaker(autocommit=False, autoflush=False, bind=engine)() + + yield session + + session.close() + + +def _write_wav(name: str) -> tuple[Path, str]: + """Create a placeholder audio file and return (abs path, stored path).""" + path = config.get_generations_dir() / name + path.write_bytes(b"RIFF placeholder") + return path, f"generations/{name}" + + +def _make_profile(db, profile_id: str = "profile-1") -> VoiceProfile: + profile = VoiceProfile(id=profile_id, name=f"Voice {profile_id}", language="en") + db.add(profile) + db.commit() + return profile + + +@pytest.mark.asyncio +async def test_delete_profile_removes_generations_and_audio(db): + """Generations, versions, and their files go with the profile.""" + _make_profile(db) + gen_file, gen_stored = _write_wav("gen.wav") + version_file, version_stored = _write_wav("gen-reverb.wav") + + db.add(Generation(id="gen-1", profile_id="profile-1", text="hello", audio_path=gen_stored)) + db.add( + GenerationVersion( + id="version-1", + generation_id="gen-1", + label="Reverb", + audio_path=version_stored, + ) + ) + db.commit() + + assert await profiles.delete_profile("profile-1", db) is True + + assert db.query(Generation).count() == 0 + assert db.query(GenerationVersion).count() == 0 + assert not gen_file.exists() + assert not version_file.exists() + + +@pytest.mark.asyncio +async def test_orphaned_generations_are_invisible_in_history(db): + """The history join hides orphans, so leaving them behind strands them.""" + _make_profile(db) + _, gen_stored = _write_wav("gen.wav") + db.add(Generation(id="gen-1", profile_id="profile-1", text="hello", audio_path=gen_stored)) + db.commit() + + assert (await history.list_generations(HistoryQuery(limit=50, offset=0), db)).total == 1 + + await profiles.delete_profile("profile-1", db) + + listed = await history.list_generations(HistoryQuery(limit=50, offset=0), db) + assert listed.total == 0 + assert db.query(Generation).count() == 0, "generation is unreachable but still on disk" + + +@pytest.mark.asyncio +async def test_delete_profile_removes_story_items(db): + """Story items reference generations; the story query would hide leftovers.""" + _make_profile(db) + _, gen_stored = _write_wav("gen.wav") + db.add(Generation(id="gen-1", profile_id="profile-1", text="hello", audio_path=gen_stored)) + db.add(Story(id="story-1", name="Chapter one")) + db.add(StoryItem(id="item-1", story_id="story-1", generation_id="gen-1")) + db.commit() + + await profiles.delete_profile("profile-1", db) + + assert db.query(StoryItem).count() == 0 + assert db.query(Story).count() == 1, "the story itself must survive" + + +@pytest.mark.asyncio +async def test_delete_profile_clears_references_to_it(db): + """Samples, channel mappings, and default-voice pointers are cleaned up.""" + _make_profile(db) + db.add(ProfileSample(id="sample-1", profile_id="profile-1", audio_path="p.wav", reference_text="hi")) + db.add(ProfileChannelMapping(profile_id="profile-1", channel_id="channel-1")) + db.add(MCPClientBinding(client_id="claude-code", profile_id="profile-1")) + db.add(CaptureSettings(id=1, default_playback_voice_id="profile-1")) + db.commit() + + await profiles.delete_profile("profile-1", db) + + assert db.query(ProfileSample).count() == 0 + assert db.query(ProfileChannelMapping).count() == 0 + assert db.query(MCPClientBinding).filter_by(client_id="claude-code").one().profile_id is None + assert db.query(CaptureSettings).filter_by(id=1).one().default_playback_voice_id is None + + +@pytest.mark.asyncio +async def test_delete_profile_survives_unremovable_audio(db, monkeypatch): + """A file locked by playback must not abort the delete half-way through.""" + _make_profile(db) + gen_file, gen_stored = _write_wav("gen.wav") + db.add(Generation(id="gen-1", profile_id="profile-1", text="hello", audio_path=gen_stored)) + db.commit() + + real_unlink = Path.unlink + + def refuse_locked_file(self, *args, **kwargs): + if self == gen_file: + raise OSError("file is in use by another process") + return real_unlink(self, *args, **kwargs) + + monkeypatch.setattr(Path, "unlink", refuse_locked_file) + + assert await profiles.delete_profile("profile-1", db) is True + assert db.query(VoiceProfile).count() == 0 + assert db.query(Generation).count() == 0 + + +@pytest.mark.asyncio +async def test_delete_profile_leaves_other_profiles_alone(db): + """Cleanup is scoped to the deleted profile.""" + _make_profile(db, "profile-1") + _make_profile(db, "profile-2") + _, keep_stored = _write_wav("keep.wav") + _, drop_stored = _write_wav("drop.wav") + db.add(Generation(id="gen-keep", profile_id="profile-2", text="keep", audio_path=keep_stored)) + db.add(Generation(id="gen-drop", profile_id="profile-1", text="drop", audio_path=drop_stored)) + db.add(ProfileChannelMapping(profile_id="profile-2", channel_id="channel-1")) + db.commit() + + await profiles.delete_profile("profile-1", db) + + assert [g.id for g in db.query(Generation).all()] == ["gen-keep"] + assert db.query(ProfileChannelMapping).count() == 1 + + +@pytest.mark.asyncio +async def test_delete_missing_profile_returns_false(db): + assert await profiles.delete_profile("nope", db) is False