fix(conversations): transactional segment-text edits to stop lost updates (#9392)

backend/database/conversations.py

@@ -1,4 +1,4 @@
 import copy
 import json
 import uuid
 import zlib
@@ -599,38 +599,50 @@
     """
     Update a single segment's text in a conversation.
 
+    The read-modify-write runs in a Firestore transaction so concurrent edits
+    (e.g. the same conversation open in two tabs) can't lose-update each other.
+    Without it, two edits that both read the pre-edit transcript_segments array
+    and each rewrite the whole array clobber one another — the later write wins
+    and silently drops the earlier edit.
+
     Returns:
         'ok' on success, 'not_found' if conversation missing, 'locked' if conversation is locked,
         'segment_not_found' if segment_id not found.
     """
     doc_ref = db.collection('users').document(uid).collection(conversations_collection).document(conversation_id)
-    doc_snapshot = doc_ref.get()
-    if not doc_snapshot.exists:
-        return 'not_found'
-
-    raw_data = doc_snapshot.to_dict()
-    if raw_data.get('is_locked', False):
-        return 'locked'
-
-    conversation_data = _prepare_conversation_for_read(raw_data, uid)
-    if not conversation_data:
-        return 'not_found'
-
-    segments = conversation_data.get('transcript_segments', [])
-    found = False
-    for segment in segments:
-        if isinstance(segment, dict) and segment.get('id') == segment_id:
-            segment['text'] = text
-            found = True
-            break
+    transaction = db.transaction()
 
-    if not found:
-        return 'segment_not_found'
+    @firestore.transactional
+    def _update_segment_text(transaction) -> str:
+        doc_snapshot = doc_ref.get(transaction=transaction)
+        if not doc_snapshot.exists:
+            return 'not_found'
+
+        raw_data = doc_snapshot.to_dict()
+        if raw_data.get('is_locked', False):
+            return 'locked'
+
+        conversation_data = _prepare_conversation_for_read(raw_data, uid)
+        if not conversation_data:
+            return 'not_found'
+
+        segments = conversation_data.get('transcript_segments', [])
+        found = False
+        for segment in segments:
+            if isinstance(segment, dict) and segment.get('id') == segment_id:
+                segment['text'] = text
+                found = True
+                break
+
+        if not found:
+            return 'segment_not_found'
+
+        doc_level = conversation_data.get('data_protection_level', 'standard')
+        prepared_payload = _prepare_conversation_for_write({'transcript_segments': segments}, uid, doc_level)
+        transaction.update(doc_ref, prepared_payload)
+        return 'ok'
 
-    doc_level = conversation_data.get('data_protection_level', 'standard')
-    prepared_payload = _prepare_conversation_for_write({'transcript_segments': segments}, uid, doc_level)
-    doc_ref.update(prepared_payload)
-    return 'ok'
+    return _update_segment_text(transaction)
 
 
 def delete_conversation_photos(uid: str, conversation_id: str) -> int:

backend/tests/unit/test_conversation_revision_contract.py

@@ -67,6 +67,9 @@ class _Transaction:
     def set(self, ref, data, **kwargs):
         ref.set(data, **kwargs)
 
+    def update(self, ref, data):
+        ref.update(data)
+
 
 def test_document_update_time_is_exposed_as_server_revision():
     revision = datetime(2026, 7, 9, 12, 0, tzinfo=timezone.utc)
@@ -261,3 +264,67 @@ def test_mutation_response_contract_carries_canonical_revision_and_state():
     assert result.conversation.structured.title == 'Renamed'
     assert result.conversation.structured.overview == 'Processing finished'
     assert result.conversation.starred is True
+
+
+def _segment_snapshot(segments, *, is_locked=False, exists=True):
+    return _Snapshot(
+        {
+            'data_protection_level': 'standard',
+            'is_locked': is_locked,
+            'transcript_segments': segments,
+        },
+        exists=exists,
+    )
+
+
+def test_segment_text_edit_reads_and_writes_inside_a_transaction(monkeypatch):
+    # Regression for #9392: the read-modify-write must be atomic so concurrent
+    # edits to different segments can't lose-update each other.
+    ref = _ConversationRef(_segment_snapshot([{'id': 's1', 'text': 'old'}, {'id': 's2', 'text': 'keep'}]))
+    monkeypatch.setattr(conversations_db, 'db', _Firestore(ref))
+    monkeypatch.setattr(conversations_db.firestore, 'transactional', lambda function: function)
+
+    result = conversations_db.update_conversation_segment_text('user-1', 'conv-1', 's1', 'new text')
+
+    assert result == 'ok'
+    # The write went through the transaction (recorded on the ref), and the edit
+    # landed while the untouched segment is preserved.
+    assert len(ref.update_calls) == 1
+    import json as _json
+    import zlib as _zlib
+
+    written = _json.loads(_zlib.decompress(ref.update_calls[0]['transcript_segments']).decode('utf-8'))
+    assert {s['id']: s['text'] for s in written} == {'s1': 'new text', 's2': 'keep'}
+
+
+def test_segment_text_edit_missing_segment_does_not_write(monkeypatch):
+    ref = _ConversationRef(_segment_snapshot([{'id': 's1', 'text': 'old'}]))
+    monkeypatch.setattr(conversations_db, 'db', _Firestore(ref))
+    monkeypatch.setattr(conversations_db.firestore, 'transactional', lambda function: function)
+
+    result = conversations_db.update_conversation_segment_text('user-1', 'conv-1', 'missing', 'x')
+
+    assert result == 'segment_not_found'
+    assert ref.update_calls == []
+
+
+def test_segment_text_edit_rejects_locked_conversation(monkeypatch):
+    ref = _ConversationRef(_segment_snapshot([{'id': 's1', 'text': 'old'}], is_locked=True))
+    monkeypatch.setattr(conversations_db, 'db', _Firestore(ref))
+    monkeypatch.setattr(conversations_db.firestore, 'transactional', lambda function: function)
+
+    result = conversations_db.update_conversation_segment_text('user-1', 'conv-1', 's1', 'x')
+
+    assert result == 'locked'
+    assert ref.update_calls == []
+
+
+def test_segment_text_edit_missing_conversation_returns_not_found(monkeypatch):
+    ref = _ConversationRef(_segment_snapshot([], exists=False))
+    monkeypatch.setattr(conversations_db, 'db', _Firestore(ref))
+    monkeypatch.setattr(conversations_db.firestore, 'transactional', lambda function: function)
+
+    result = conversations_db.update_conversation_segment_text('user-1', 'conv-1', 's1', 'x')
+
+    assert result == 'not_found'
+    assert ref.update_calls == []