fix: merge-last-assistant deletes tool/system rows from the DB (history desync) (#1929)
This commit is contained in:
@@ -15,6 +15,26 @@ from routes.session_routes import _verify_session_owner
|
|||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
def _merge_continue_rows_to_delete(db_messages, db1, db2):
|
||||||
|
"""DB rows to delete when merging the last two assistant messages.
|
||||||
|
|
||||||
|
Always the second assistant message (db2), plus ONLY the single
|
||||||
|
intervening "continue" user message (the one carrying "previous response
|
||||||
|
was interrupted") — matching the in-memory merge. The previous code
|
||||||
|
deleted the whole index range between the two assistant rows, destroying
|
||||||
|
any tool/system/user messages in between and desyncing the DB from the
|
||||||
|
in-memory history.
|
||||||
|
"""
|
||||||
|
to_delete = [db2]
|
||||||
|
i1 = next((i for i, m in enumerate(db_messages) if m is db1), None)
|
||||||
|
i2 = next((i for i, m in enumerate(db_messages) if m is db2), None)
|
||||||
|
if i1 is not None and i2 is not None and i2 - 1 > i1:
|
||||||
|
between = db_messages[i2 - 1]
|
||||||
|
if getattr(between, "role", "") == "user" and "previous response was interrupted" in (getattr(between, "content", "") or ""):
|
||||||
|
to_delete.append(between)
|
||||||
|
return to_delete
|
||||||
|
|
||||||
|
|
||||||
def setup_history_routes(session_manager) -> APIRouter:
|
def setup_history_routes(session_manager) -> APIRouter:
|
||||||
router = APIRouter(tags=["history"])
|
router = APIRouter(tags=["history"])
|
||||||
|
|
||||||
@@ -418,11 +438,13 @@ def setup_history_routes(session_manager) -> APIRouter:
|
|||||||
db1.content = merged_content
|
db1.content = merged_content
|
||||||
db1.meta_data = _json.dumps(merged_meta)
|
db1.meta_data = _json.dumps(merged_meta)
|
||||||
|
|
||||||
# Remove the continue user message if between them
|
# Mirror the in-memory deletion: remove the second assistant
|
||||||
db_idx2 = db_messages.index(db2)
|
# message and ONLY the "continue" user message between them
|
||||||
db_idx1 = db_messages.index(db1)
|
# (not arbitrary tool/system/user rows). The old
|
||||||
for di in range(db_idx2, db_idx1, -1):
|
# range-delete destroyed every row between the two assistant
|
||||||
db.delete(db_messages[di])
|
# messages, desyncing the DB from the in-memory history.
|
||||||
|
for _row in _merge_continue_rows_to_delete(db_messages, db1, db2):
|
||||||
|
db.delete(_row)
|
||||||
|
|
||||||
db.commit()
|
db.commit()
|
||||||
finally:
|
finally:
|
||||||
|
|||||||
41
tests/test_merge_last_assistant_rows.py
Normal file
41
tests/test_merge_last_assistant_rows.py
Normal file
@@ -0,0 +1,41 @@
|
|||||||
|
"""merge-last-assistant must not delete tool/system rows between the messages.
|
||||||
|
|
||||||
|
The in-memory merge removes the second assistant message plus only the
|
||||||
|
"continue" user message between the last two assistant replies. The DB path
|
||||||
|
deleted the ENTIRE index range between them, destroying any tool/system/user
|
||||||
|
rows in between — so on reload the DB lost messages the in-memory history
|
||||||
|
kept (data loss + count desync). _merge_continue_rows_to_delete makes the DB
|
||||||
|
deletion mirror the in-memory rule.
|
||||||
|
"""
|
||||||
|
from types import SimpleNamespace
|
||||||
|
|
||||||
|
from routes.history_routes import _merge_continue_rows_to_delete
|
||||||
|
|
||||||
|
|
||||||
|
def _m(role, content=""):
|
||||||
|
return SimpleNamespace(role=role, content=content)
|
||||||
|
|
||||||
|
|
||||||
|
def test_tool_message_between_is_not_deleted():
|
||||||
|
u, a1, tool, a2 = _m("user", "q"), _m("assistant", "a1"), _m("tool", "RESULT"), _m("assistant", "a2")
|
||||||
|
rows = _merge_continue_rows_to_delete([u, a1, tool, a2], a1, a2)
|
||||||
|
assert rows == [a2] # only the 2nd assistant
|
||||||
|
assert tool not in rows # the tool result survives
|
||||||
|
|
||||||
|
|
||||||
|
def test_continue_user_message_is_deleted():
|
||||||
|
u, a1, cont, a2 = (_m("user", "q"), _m("assistant", "a1"),
|
||||||
|
_m("user", "(the previous response was interrupted)"), _m("assistant", "a2"))
|
||||||
|
rows = _merge_continue_rows_to_delete([u, a1, cont, a2], a1, a2)
|
||||||
|
assert a2 in rows and cont in rows and len(rows) == 2
|
||||||
|
|
||||||
|
|
||||||
|
def test_adjacent_assistants_delete_only_second():
|
||||||
|
a1, a2 = _m("assistant", "a1"), _m("assistant", "a2")
|
||||||
|
assert _merge_continue_rows_to_delete([a1, a2], a1, a2) == [a2]
|
||||||
|
|
||||||
|
|
||||||
|
def test_plain_user_between_not_deleted():
|
||||||
|
a1, usr, a2 = _m("assistant", "a1"), _m("user", "a real follow-up question"), _m("assistant", "a2")
|
||||||
|
rows = _merge_continue_rows_to_delete([a1, usr, a2], a1, a2)
|
||||||
|
assert rows == [a2] and usr not in rows
|
||||||
Reference in New Issue
Block a user