fix(session): reload latest imported session before title save (#3987)
This commit is contained in:
1 parent
2278867c88
commit
e1ec41b32c
2 files changed
+169
-44
No files matched your search
+91
-40
@@ -79,55 +79,106 @@ def _sync_session_title_to_insights(session) -> None:
|
||||
logger.debug("Failed to update session title in state.db", exc_info=True)
|
||||
|
||||
|
||||
def _persist_generated_session_title(session, next_title: str, *, event_reason: str) -> str:
|
||||
def _persist_generated_session_title(
|
||||
session,
|
||||
next_title: str,
|
||||
*,
|
||||
event_reason: str,
|
||||
require_default_title: bool = False,
|
||||
) -> str:
|
||||
normalized_title = str(next_title or "").strip()[:80] or "Untitled"
|
||||
with _get_session_agent_lock(session.session_id):
|
||||
session.title = normalized_title
|
||||
sid = str(getattr(session, "session_id", "") or "")
|
||||
with _get_session_agent_lock(sid):
|
||||
with LOCK:
|
||||
latest = SESSIONS.get(sid)
|
||||
if latest is not None and str(getattr(latest, "session_id", "") or "") != sid:
|
||||
SESSIONS.pop(sid, None)
|
||||
latest = None
|
||||
elif latest is not None:
|
||||
SESSIONS.move_to_end(sid)
|
||||
if latest is None:
|
||||
latest = Session.load(sid)
|
||||
if latest is None:
|
||||
raise KeyError(sid)
|
||||
latest = _ensure_full_session_before_mutation(sid, latest)
|
||||
if getattr(latest, "read_only", False):
|
||||
raise PermissionError(f"Session {sid} is read-only")
|
||||
if require_default_title:
|
||||
latest_meta = {
|
||||
"title": getattr(latest, "title", None),
|
||||
"source_tag": getattr(latest, "source_tag", None),
|
||||
"raw_source": getattr(latest, "raw_source", None),
|
||||
"session_source": getattr(latest, "session_source", None),
|
||||
"source_label": getattr(latest, "source_label", None),
|
||||
}
|
||||
if not _looks_like_default_cli_title(latest_meta):
|
||||
return latest.title
|
||||
latest.title = normalized_title
|
||||
from api.session_ops import mark_session_title_generated
|
||||
|
||||
# mark_session_title_generated sets s.llm_title_generated = True and clears manual_title.
|
||||
mark_session_title_generated(session)
|
||||
session.save(touch_updated_at=False)
|
||||
_sync_session_title_to_insights(session)
|
||||
_publish_session_list_changed(event_reason, profile=getattr(session, "profile", None))
|
||||
return session.title
|
||||
mark_session_title_generated(latest)
|
||||
latest.save(touch_updated_at=False)
|
||||
with LOCK:
|
||||
SESSIONS[sid] = latest
|
||||
SESSIONS.move_to_end(sid)
|
||||
while len(SESSIONS) > SESSIONS_MAX:
|
||||
SESSIONS.popitem(last=False)
|
||||
session.title = latest.title
|
||||
session.llm_title_generated = latest.llm_title_generated
|
||||
session.manual_title = latest.manual_title
|
||||
_sync_session_title_to_insights(latest)
|
||||
_publish_session_list_changed(event_reason, profile=getattr(latest, "profile", None))
|
||||
return latest.title
|
||||
|
||||
|
||||
def _queue_generated_title_for_imported_session(session, cli_meta: dict | None) -> None:
|
||||
cli_meta = dict(cli_meta or {})
|
||||
if not session or cli_meta.get("read_only") or not _looks_like_default_cli_title(cli_meta):
|
||||
return
|
||||
sid = str(getattr(session, "session_id", "") or "")
|
||||
if not sid:
|
||||
return
|
||||
try:
|
||||
cli_meta = dict(cli_meta or {})
|
||||
if not session or cli_meta.get("read_only") or not _looks_like_default_cli_title(cli_meta):
|
||||
return
|
||||
sid = str(getattr(session, "session_id", "") or "")
|
||||
if not sid:
|
||||
return
|
||||
|
||||
def _run() -> None:
|
||||
try:
|
||||
current = Session.load(sid)
|
||||
if not current:
|
||||
return
|
||||
current = _ensure_full_session_before_mutation(sid, current)
|
||||
if getattr(current, "read_only", False):
|
||||
return
|
||||
current_meta = {
|
||||
"title": getattr(current, "title", None),
|
||||
"source_tag": getattr(current, "source_tag", None),
|
||||
"raw_source": getattr(current, "raw_source", None),
|
||||
"session_source": getattr(current, "session_source", None),
|
||||
"source_label": getattr(current, "source_label", None),
|
||||
}
|
||||
if not _looks_like_default_cli_title(current_meta):
|
||||
return
|
||||
next_title, _reason, _raw_preview = generate_session_title_for_session(current)
|
||||
normalized_current = str(getattr(current, "title", "") or "").strip()
|
||||
normalized_next = str(next_title or "").strip()
|
||||
if not normalized_next or normalized_next == normalized_current:
|
||||
return
|
||||
_persist_generated_session_title(current, normalized_next, event_reason="session_title_regenerate")
|
||||
except Exception:
|
||||
logger.debug("Failed to generate imported session title for %s", sid, exc_info=True)
|
||||
def _run() -> None:
|
||||
try:
|
||||
current = Session.load(sid)
|
||||
if not current:
|
||||
return
|
||||
current = _ensure_full_session_before_mutation(sid, current)
|
||||
if getattr(current, "read_only", False):
|
||||
return
|
||||
current_meta = {
|
||||
"title": getattr(current, "title", None),
|
||||
"source_tag": getattr(current, "source_tag", None),
|
||||
"raw_source": getattr(current, "raw_source", None),
|
||||
"session_source": getattr(current, "session_source", None),
|
||||
"source_label": getattr(current, "source_label", None),
|
||||
}
|
||||
if not _looks_like_default_cli_title(current_meta):
|
||||
return
|
||||
next_title, _reason, _raw_preview = generate_session_title_for_session(current)
|
||||
normalized_current = str(getattr(current, "title", "") or "").strip()
|
||||
normalized_next = str(next_title or "").strip()
|
||||
if not normalized_next or normalized_next == normalized_current:
|
||||
return
|
||||
_persist_generated_session_title(
|
||||
current,
|
||||
normalized_next,
|
||||
event_reason="session_title_regenerate",
|
||||
require_default_title=True,
|
||||
)
|
||||
except Exception:
|
||||
logger.debug("Failed to generate imported session title for %s", sid, exc_info=True)
|
||||
|
||||
threading.Thread(target=_run, daemon=True, name=f"imported-title-{sid}").start()
|
||||
threading.Thread(target=_run, daemon=True, name=f"imported-title-{sid}").start()
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"Failed to queue imported session title generation for %s",
|
||||
getattr(session, "session_id", None),
|
||||
exc_info=True,
|
||||
)
|
||||
|
||||
|
||||
# ── Cron run tracking ────────────────────────────────────────────────────────
|
||||
|
||||
@@ -4,8 +4,10 @@ from __future__ import annotations
|
||||
|
||||
import io
|
||||
import json
|
||||
from collections import OrderedDict
|
||||
from pathlib import Path
|
||||
|
||||
import api.models as models
|
||||
import api.routes as routes
|
||||
|
||||
|
||||
@@ -54,7 +56,7 @@ def test_import_cli_queue_helper_is_guarded_and_runs_in_background():
|
||||
assert "not _looks_like_default_cli_title(cli_meta)" in block
|
||||
assert "if not _looks_like_default_cli_title(current_meta):" in block
|
||||
assert "generate_session_title_for_session(current)" in block
|
||||
assert "_persist_generated_session_title(current, normalized_next, event_reason=\"session_title_regenerate\")" in block
|
||||
assert "require_default_title=True" in block
|
||||
assert "threading.Thread(target=_run, daemon=True" in block
|
||||
|
||||
|
||||
@@ -85,12 +87,18 @@ def test_import_cli_queue_helper_generates_title_once_for_placeholder_session(mo
|
||||
monkeypatch.setattr(routes.Session, "load", classmethod(lambda _cls, sid: current if sid == current.session_id else None))
|
||||
monkeypatch.setattr(routes, "_ensure_full_session_before_mutation", lambda sid, session: session)
|
||||
monkeypatch.setattr(routes, "generate_session_title_for_session", lambda session: (generated.append(session.session_id) or "Better imported title", "llm", "raw"))
|
||||
monkeypatch.setattr(routes, "_persist_generated_session_title", lambda session, title, *, event_reason: persisted.append((session.session_id, title, event_reason)))
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"_persist_generated_session_title",
|
||||
lambda session, title, *, event_reason, require_default_title=False: persisted.append(
|
||||
(session.session_id, title, event_reason, require_default_title)
|
||||
),
|
||||
)
|
||||
|
||||
routes._queue_generated_title_for_imported_session(current, {"title": "CLI Session", "source_tag": "cli"})
|
||||
|
||||
assert generated == [current.session_id]
|
||||
assert persisted == [(current.session_id, "Better imported title", "session_title_regenerate")]
|
||||
assert persisted == [(current.session_id, "Better imported title", "session_title_regenerate", True)]
|
||||
|
||||
|
||||
def test_import_cli_queue_helper_skips_sessions_that_already_have_real_titles(monkeypatch):
|
||||
@@ -124,7 +132,13 @@ def test_import_cli_queue_helper_skips_sessions_that_already_have_real_titles(mo
|
||||
"generate_session_title_for_session",
|
||||
lambda session: generated.append(session.session_id) or ("Unexpected generated title", "llm", "raw"),
|
||||
)
|
||||
monkeypatch.setattr(routes, "_persist_generated_session_title", lambda session, title, *, event_reason: persisted.append((session.session_id, title, event_reason)))
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"_persist_generated_session_title",
|
||||
lambda session, title, *, event_reason, require_default_title=False: persisted.append(
|
||||
(session.session_id, title, event_reason, require_default_title)
|
||||
),
|
||||
)
|
||||
|
||||
routes._queue_generated_title_for_imported_session(
|
||||
current,
|
||||
@@ -135,6 +149,66 @@ def test_import_cli_queue_helper_skips_sessions_that_already_have_real_titles(mo
|
||||
assert persisted == []
|
||||
|
||||
|
||||
def test_generated_title_persist_reloads_latest_session_before_saving(tmp_path, monkeypatch):
|
||||
session_dir = tmp_path / "sessions"
|
||||
session_dir.mkdir()
|
||||
cache = OrderedDict()
|
||||
|
||||
monkeypatch.setattr(models, "SESSION_DIR", session_dir)
|
||||
monkeypatch.setattr(models, "SESSIONS", cache, raising=False)
|
||||
monkeypatch.setattr(routes, "SESSIONS", cache, raising=False)
|
||||
monkeypatch.setattr(routes, "_sync_session_title_to_insights", lambda session: None)
|
||||
monkeypatch.setattr(routes, "_publish_session_list_changed", lambda *args, **kwargs: None)
|
||||
|
||||
stale = models.Session(
|
||||
session_id="cli_import_race",
|
||||
title="CLI Session",
|
||||
workspace=".",
|
||||
model="test-model",
|
||||
messages=[{"role": "user", "content": "first"}],
|
||||
source_tag="cli",
|
||||
raw_source="cli",
|
||||
session_source="external_agent",
|
||||
source_label="CLI",
|
||||
)
|
||||
stale.save(skip_index=True)
|
||||
stale_snapshot = models.Session.load(stale.session_id)
|
||||
|
||||
latest = models.Session(
|
||||
session_id=stale.session_id,
|
||||
title="CLI Session",
|
||||
workspace=".",
|
||||
model="test-model",
|
||||
messages=[
|
||||
{"role": "user", "content": "first"},
|
||||
{"role": "assistant", "content": "reply"},
|
||||
{"role": "user", "content": "second"},
|
||||
],
|
||||
source_tag="cli",
|
||||
raw_source="cli",
|
||||
session_source="external_agent",
|
||||
source_label="CLI",
|
||||
)
|
||||
latest.save(skip_index=True)
|
||||
cache[latest.session_id] = latest
|
||||
|
||||
saved_title = routes._persist_generated_session_title(
|
||||
stale_snapshot,
|
||||
"Better imported title",
|
||||
event_reason="session_title_regenerate",
|
||||
require_default_title=True,
|
||||
)
|
||||
|
||||
reloaded = models.Session.load(stale.session_id)
|
||||
assert saved_title == "Better imported title"
|
||||
assert reloaded.title == "Better imported title"
|
||||
assert len(reloaded.messages) == 3
|
||||
assert reloaded.llm_title_generated is True
|
||||
assert reloaded.manual_title is False
|
||||
assert len(cache[stale.session_id].messages) == 3
|
||||
assert stale_snapshot.title == "Better imported title"
|
||||
|
||||
|
||||
def test_regenerate_endpoint_only_blocks_read_only_imported_sessions():
|
||||
endpoint_idx = ROUTES_PY.index('"/api/session/title/regenerate"')
|
||||
next_endpoint_idx = ROUTES_PY.index('"/api/personality/set"', endpoint_idx)
|
||||
|
||||
Reference in new issue
Block a user