From 7facf63ae71b846d6079a69483fc95ce9aa8db75 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 16 Jul 2026 20:01:13 -0700 Subject: [PATCH] fix(title): reconcile atomic auto-title writes with collision dedup retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Combines the two salvaged fixes so they compose instead of conflict: _persist_session_title (#50575) now writes through set_auto_title_if_empty (#51483) when the store provides it — the collision-dedup retry and the manual-/title race protection apply together. Predicate failure (a manual title landed while generation was in flight) returns None: nothing written, no callback. Legacy stores without the atomic method keep the plain set_session_title path, including the vanished-session RuntimeError. Tests cover both store shapes plus the race-skip path; E2E verified against a real SQLite SessionDB (collision -> 'Weekly Report #2', manual title preserved, cron dedup, blank guard). AUTHOR_MAP entry for rasitakyol. --- scripts/release.py | 1 + tests/agent/test_title_generator.py | 42 +++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/scripts/release.py b/scripts/release.py index 256874caad62..c665e21897a6 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -368,6 +368,7 @@ AUTHOR_MAP = { "157689911+itsflownium@users.noreply.github.com": "itsflownium", "dirtyren@users.noreply.github.com": "dirtyren", "s96919@gmail.com": "s96919", + "rasitakyol@hotmail.com": "rasitakyol", "yakimenkoleksander228@gmail.com": "doxe0x", "a54983334@163.com": "Code-suphub", "78542984+Code-suphub@users.noreply.github.com": "Code-suphub", diff --git a/tests/agent/test_title_generator.py b/tests/agent/test_title_generator.py index 9d9af4362724..bb0c9627a16a 100644 --- a/tests/agent/test_title_generator.py +++ b/tests/agent/test_title_generator.py @@ -446,7 +446,8 @@ class TestAutoTitleDuplicateHandling: def test_dedupes_duplicate_title_via_lineage(self): db = MagicMock() db.get_session_title.return_value = None - db.set_session_title.side_effect = [ValueError("in use"), True] + # Atomic write path: collision raises ValueError, retry persists. + db.set_auto_title_if_empty.side_effect = [ValueError("in use"), True] db.get_next_title_in_lineage.return_value = "Debugging Import Error #2" with patch( "agent.title_generator.generate_title", @@ -455,13 +456,34 @@ class TestAutoTitleDuplicateHandling: seen = [] auto_title_session(db, "sess-1", "hi", "hello", title_callback=seen.append) db.get_next_title_in_lineage.assert_called_once_with("Debugging Import Error") - assert db.set_session_title.call_args_list[-1][0] == ( + assert db.set_auto_title_if_empty.call_args_list[-1][0] == ( "sess-1", "Debugging Import Error #2", ) # callback fires with the actually-persisted (deduped) title assert seen == ["Debugging Import Error #2"] + def test_dedupes_duplicate_title_via_lineage_legacy_store(self): + # Store without set_auto_title_if_empty: same dedup via the plain + # set_session_title fallback. + db = MagicMock( + spec=["get_session_title", "set_session_title", "get_next_title_in_lineage"] + ) + db.get_session_title.return_value = None + db.set_session_title.side_effect = [ValueError("in use"), True] + db.get_next_title_in_lineage.return_value = "Debugging Import Error #2" + with patch( + "agent.title_generator.generate_title", + return_value="Debugging Import Error", + ): + seen = [] + auto_title_session(db, "sess-1", "hi", "hello", title_callback=seen.append) + assert db.set_session_title.call_args_list[-1][0] == ( + "sess-1", + "Debugging Import Error #2", + ) + assert seen == ["Debugging Import Error #2"] + def test_swallows_value_error_without_lineage_support(self): # No get_next_title_in_lineage -> ValueError propagates out of the # persist helper but auto_title_session still swallows it (no crash). @@ -473,11 +495,21 @@ class TestAutoTitleDuplicateHandling: ): auto_title_session(db, "sess-1", "hi", "hello") # must not raise - def test_not_found_raises_runtime_error_internally(self): - # set_session_title returning False (session vanished) -> RuntimeError - # in the persist helper, swallowed by auto_title_session, no callback. + def test_manual_title_race_skips_without_callback(self): + # Atomic predicate fails (manual /title landed while generation was in + # flight) -> nothing persisted, no callback fired. from agent.title_generator import _persist_session_title db = MagicMock() + db.set_auto_title_if_empty.return_value = False + assert _persist_session_title(db, "sess-1", "Some Title") is None + db.set_session_title.assert_not_called() + + def test_not_found_raises_runtime_error_internally(self): + # Legacy store (no atomic write): set_session_title returning False + # (session vanished) -> RuntimeError in the persist helper, swallowed + # by auto_title_session, no callback. + from agent.title_generator import _persist_session_title + db = MagicMock(spec=["get_session_title", "set_session_title"]) db.set_session_title.return_value = False with pytest.raises(RuntimeError): _persist_session_title(db, "missing", "Some Title")