diff --git a/.github/scripts/product_file_line_count_ratchet_baseline/backend-database.json b/.github/scripts/product_file_line_count_ratchet_baseline/backend-database.json index a31c6d89802..3767e64ff8d 100644 --- a/.github/scripts/product_file_line_count_ratchet_baseline/backend-database.json +++ b/.github/scripts/product_file_line_count_ratchet_baseline/backend-database.json @@ -1,10 +1,10 @@ { "files": { - "backend/database/conversations.py": 1601, + "backend/database/conversations.py": 1616, "backend/database/users.py": 1943 }, "raise_justifications": { - "backend/database/conversations.py": "Public shared-chat uses the existing conversation storage format for bounded, safe transcript decoding; extraction would duplicate crypto/storage invariants. +21 for the #10033 merge-target eligibility predicate and pure closest-match selector beside the query they filter." + "backend/database/conversations.py": "Public shared-chat uses the existing conversation storage format for bounded, safe transcript decoding; extraction would duplicate crypto/storage invariants. +15 for the shared is_soft_deleted tombstone-eligibility predicate that eligible_merge_target and the reprocess guard converge on (#10119/#10262)." }, "threshold": 1500 } diff --git a/backend/database/conversations.py b/backend/database/conversations.py index 07809f8af35..cfb797a61e7 100644 --- a/backend/database/conversations.py +++ b/backend/database/conversations.py @@ -1524,6 +1524,21 @@ def _store(transaction) -> bool: # ******************************** +def is_soft_deleted(conversation: Optional[dict]) -> bool: + """Whether a conversation is a soft-deleted tombstone. + + A tombstone is invisible to the user, so any content operation that reads it + and writes derived state — merging its segments, or reprocessing to + regenerate structured data, action items, memories and embeddings — + resurrects data the user deleted. Such operations must reject a tombstone. + + Shared predicate behind that contract (sync #10119 via `eligible_merge_target`, + merge #10262, reprocess). Deliberately distinct from `discarded`, which stays + revivable: the merge and reprocess paths intentionally revive a discarded row. + """ + return bool(conversation) and bool(conversation.get('deleted')) + + def eligible_merge_target(conversation: Optional[dict]) -> bool: """Whether synced audio may merge into this conversation (#10033). @@ -1532,7 +1547,7 @@ def eligible_merge_target(conversation: Optional[dict]) -> bool: conversation". Discarded rows stay eligible; the merge path reprocesses and revives them. """ - return bool(conversation) and not conversation.get('deleted') + return bool(conversation) and not is_soft_deleted(conversation) def select_closest_conversation(conversations, start_timestamp: int, end_timestamp: int) -> Optional[dict]: diff --git a/backend/routers/conversations.py b/backend/routers/conversations.py index a99fe3dbd17..30304b09370 100644 --- a/backend/routers/conversations.py +++ b/backend/routers/conversations.py @@ -318,6 +318,14 @@ def reprocess_conversation( :return: The updated conversation after reprocessing. """ conversation = _get_valid_conversation_by_id(uid, conversation_id) + # Reprocess force-processes a *discarded* conversation to revive it, but a + # soft-deleted tombstone is invisible to the user and must not be reprocessed: + # process_conversation would regenerate structured data, action items, memories + # and embeddings from content the user deleted, resurrecting it. Same + # tombstone-eligibility contract as sync (#10119) and merge (#10262). Checked + # on the raw doc because the Conversation model does not carry `deleted`. + if conversations_db.is_soft_deleted(conversation): + raise HTTPException(status_code=404, detail="Conversation not found") conversation = deserialize_conversation(conversation) if not language_code: language_code = conversation.language or 'en' diff --git a/backend/tests/unit/test_reprocess_tombstone_guard.py b/backend/tests/unit/test_reprocess_tombstone_guard.py new file mode 100644 index 00000000000..7e83fb7ca64 --- /dev/null +++ b/backend/tests/unit/test_reprocess_tombstone_guard.py @@ -0,0 +1,64 @@ +"""Reprocess must reject a soft-deleted conversation. + +`POST /v1/conversations/{id}/reprocess` fetches through `_get_valid_conversation_by_id`, +which does not filter soft-deleted tombstones (`get_conversation` returns them and the +helper only 404s on a missing doc). Reprocessing runs `process_conversation` with +`force_process=True`, regenerating structured data, action items, memories and +embeddings — so reprocessing a tombstone resurrects content the user deleted. + +The guard rejects a *deleted* conversation while still allowing a *discarded* one, +which reprocess intentionally revives — the same tombstone-eligibility contract as +sync (#10119) and merge (#10262), via the shared `is_soft_deleted` predicate. +""" + +from types import SimpleNamespace +from unittest.mock import patch + +import pytest +from fastapi import HTTPException + +import routers.conversations as conv_router +from database.conversations import eligible_merge_target, is_soft_deleted + + +class TestIsSoftDeleted: + def test_deleted_is_tombstoned(self): + assert is_soft_deleted({'id': 'c1', 'deleted': True}) is True + + def test_discarded_is_not_tombstoned(self): + # Discarded stays revivable — reprocess/merge intentionally revive it. + assert is_soft_deleted({'id': 'c1', 'discarded': True}) is False + + def test_plain_conversation_is_not_tombstoned(self): + assert is_soft_deleted({'id': 'c1'}) is False + + def test_none_is_not_tombstoned(self): + assert is_soft_deleted(None) is False + + def test_eligible_merge_target_still_excludes_only_deleted(self): + # The refactor onto is_soft_deleted must be behaviour-preserving. + assert eligible_merge_target({'id': 'c1', 'deleted': True}) is False + assert eligible_merge_target({'id': 'c1', 'discarded': True}) is True + assert eligible_merge_target(None) is False + + +class TestReprocessTombstoneGuard: + def test_reprocess_rejects_soft_deleted_conversation(self): + deleted = {'id': 'c1', 'deleted': True, 'status': 'completed'} + with patch.object(conv_router, '_get_valid_conversation_by_id', return_value=deleted), patch.object( + conv_router, 'process_conversation' + ) as process: + with pytest.raises(HTTPException) as exc: + conv_router.reprocess_conversation(conversation_id='c1', uid='u1') + assert exc.value.status_code == 404 + process.assert_not_called() # deleted content never re-enters the pipeline + + def test_reprocess_still_allows_a_discarded_conversation(self): + discarded = {'id': 'c1', 'discarded': True, 'status': 'completed'} + fake_conv = SimpleNamespace(language='en') + with patch.object(conv_router, '_get_valid_conversation_by_id', return_value=discarded), patch.object( + conv_router, 'deserialize_conversation', return_value=fake_conv + ), patch.object(conv_router, 'process_conversation', return_value=fake_conv) as process: + result = conv_router.reprocess_conversation(conversation_id='c1', uid='u1') + process.assert_called_once() + assert result is fake_conv