* feat(sessions): skip adaptive auto-rename for manually-named sessions (#3542, #3230) Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> * docs(changelog): v0.51.276 — Release IR (stage-p3e) * fix(sessions): clear manual_title lock on /api/session/clear (#3542) Codex regression-gate follow-up: the clear endpoint reset the title to Untitled directly, stranding manual_title=True so the reused session never auto-named again. Route the reset through apply_session_title_rename (which clears the lock for auto-labels) + add a behavioral and a static-guard test. --------- Co-authored-by: nesquena-hermes <[email protected]> Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
This commit is contained in:
parent
165454dd7b
commit
87084dfebf
6 changed files with 297 additions and 8 deletions
|
|
@ -3,6 +3,11 @@
|
|||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.51.276] — 2026-06-05 — Release IR (stage-p3e — preserve manually-named session titles)
|
||||
|
||||
### Fixed
|
||||
- **Sessions you deliberately rename now keep that title** — the adaptive auto-title refresh no longer overwrites a manually-set name. A per-session `manual_title` flag is set when you rename a session and checked at every auto-rename entry point (background title update, refresh, and scheduling); clearing the title or resetting it to an automatic label (`Untitled`/`New Chat`) re-enables auto-naming, and the title-regenerate action also clears the flag. Free-flowing sessions still get auto-named on the configured schedule. (#3542, @mvanhorn; fixes #3230)
|
||||
|
||||
## [v0.51.275] — 2026-06-05 — Release IQ (stage-p3d — approval SSE state extraction)
|
||||
|
||||
### Internal
|
||||
|
|
|
|||
|
|
@ -572,6 +572,7 @@ class Session:
|
|||
truncation_watermark=None,
|
||||
gateway_routing=None, gateway_routing_history=None,
|
||||
llm_title_generated: bool=False,
|
||||
manual_title: bool=False,
|
||||
parent_session_id: str=None,
|
||||
worktree_path=None,
|
||||
worktree_branch=None,
|
||||
|
|
@ -620,6 +621,7 @@ class Session:
|
|||
self.gateway_routing = gateway_routing if isinstance(gateway_routing, dict) else None
|
||||
self.gateway_routing_history = gateway_routing_history if isinstance(gateway_routing_history, list) else []
|
||||
self.llm_title_generated = bool(llm_title_generated)
|
||||
self.manual_title = bool(manual_title)
|
||||
self.parent_session_id = parent_session_id
|
||||
self.worktree_path = str(Path(worktree_path).expanduser().resolve()) if worktree_path else None
|
||||
self.worktree_branch = str(worktree_branch) if worktree_branch else None
|
||||
|
|
@ -684,7 +686,7 @@ class Session:
|
|||
'compression_anchor_details', 'context_engine_state',
|
||||
'context_length', 'threshold_tokens', 'last_prompt_tokens',
|
||||
'truncation_watermark',
|
||||
'gateway_routing', 'gateway_routing_history', 'llm_title_generated',
|
||||
'gateway_routing', 'gateway_routing_history', 'llm_title_generated', 'manual_title',
|
||||
'parent_session_id',
|
||||
'worktree_path', 'worktree_branch', 'worktree_repo_root', 'worktree_created_at',
|
||||
'is_cli_session', 'source_tag', 'raw_source', 'session_source', 'source_label', 'read_only',
|
||||
|
|
@ -890,6 +892,7 @@ class Session:
|
|||
'last_prompt_tokens': self.last_prompt_tokens,
|
||||
'gateway_routing': self.gateway_routing,
|
||||
'gateway_routing_history': self.gateway_routing_history,
|
||||
'manual_title': self.manual_title,
|
||||
# Only emit 'parent_session_id' when set (the /branch fork link, #1342).
|
||||
# Sessions without a fork must not leak None — see test_session_lineage_metadata_api.
|
||||
**({'parent_session_id': self.parent_session_id} if self.parent_session_id else {}),
|
||||
|
|
|
|||
|
|
@ -6321,6 +6321,7 @@ def handle_post(handler, parsed) -> bool:
|
|||
gateway_routing_history=copy.deepcopy(getattr(session, "gateway_routing_history", None) or []),
|
||||
# Preserve LLM-generated title flag so we don't regenerate title on duplicate.
|
||||
llm_title_generated=getattr(session, "llm_title_generated", False),
|
||||
manual_title=getattr(session, "manual_title", False),
|
||||
# Composer draft — preserve per-session draft state.
|
||||
composer_draft=copy.deepcopy(getattr(session, "composer_draft", None) or {}),
|
||||
# Context engine state — preserve so the duplicate's context engine
|
||||
|
|
@ -6481,7 +6482,8 @@ def handle_post(handler, parsed) -> bool:
|
|||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
with _get_session_agent_lock(body["session_id"]):
|
||||
s.title = str(body["title"]).strip()[:80] or "Untitled"
|
||||
from api.session_ops import apply_session_title_rename
|
||||
apply_session_title_rename(s, body["title"])
|
||||
s.save()
|
||||
_sync_session_title_to_insights(s)
|
||||
publish_session_list_changed("session_rename", profile=getattr(s, "profile", None))
|
||||
|
|
@ -6507,7 +6509,9 @@ def handle_post(handler, parsed) -> bool:
|
|||
return bad(handler, f"Could not generate a better title ({reason or 'empty'})", 422)
|
||||
with _get_session_agent_lock(sid):
|
||||
s.title = str(next_title).strip()[:80] or "Untitled"
|
||||
s.llm_title_generated = True
|
||||
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(s)
|
||||
s.save(touch_updated_at=False)
|
||||
_sync_session_title_to_insights(s)
|
||||
publish_session_list_changed("session_title_regenerate", profile=getattr(s, "profile", None))
|
||||
|
|
@ -6807,7 +6811,12 @@ def handle_post(handler, parsed) -> bool:
|
|||
with _get_session_agent_lock(sid):
|
||||
s.messages = []
|
||||
s.tool_calls = []
|
||||
s.title = "Untitled"
|
||||
# Reset the title via the rename helper so clearing a manually-named
|
||||
# session also clears manual_title/llm_title_generated — otherwise the
|
||||
# reused session keeps its manual-title protection and never auto-names
|
||||
# again (#3542 lifecycle gap).
|
||||
from api.session_ops import apply_session_title_rename
|
||||
apply_session_title_rename(s, "Untitled")
|
||||
s.save()
|
||||
# Evict cached agent outside the per-session lock. Eviction may run a
|
||||
# boundary memory commit for batch-extraction providers, and provider
|
||||
|
|
|
|||
|
|
@ -14,6 +14,36 @@ from api.models import get_session, SESSIONS
|
|||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
AUTO_TITLE_LABELS = {'untitled', 'new chat'}
|
||||
|
||||
|
||||
def session_has_manual_title(session) -> bool:
|
||||
"""Return whether adaptive title refresh should leave this title alone."""
|
||||
return getattr(session, 'manual_title', False) is True
|
||||
|
||||
|
||||
def apply_session_title_rename(session, raw_title) -> str:
|
||||
"""Apply user-driven rename semantics to a Session object.
|
||||
|
||||
Non-empty custom titles are protected from adaptive refresh. Clearing the
|
||||
title, or resetting it to an automatic label, removes that protection so the
|
||||
normal auto-title path can run again.
|
||||
"""
|
||||
title = str(raw_title or '').strip()[:80]
|
||||
if not title:
|
||||
title = 'Untitled'
|
||||
manual_title = title.strip().casefold() not in AUTO_TITLE_LABELS
|
||||
session.title = title
|
||||
session.manual_title = manual_title
|
||||
session.llm_title_generated = False
|
||||
return title
|
||||
|
||||
|
||||
def mark_session_title_generated(session) -> None:
|
||||
"""Mark a session title as generated by the title model."""
|
||||
session.llm_title_generated = True
|
||||
session.manual_title = False
|
||||
|
||||
|
||||
def _truncate_at_last_user(messages):
|
||||
history = messages or []
|
||||
|
|
|
|||
|
|
@ -51,6 +51,7 @@ from api.models import (
|
|||
get_state_db_session_messages,
|
||||
reconciled_state_db_messages_for_session,
|
||||
)
|
||||
from api.session_ops import mark_session_title_generated, session_has_manual_title
|
||||
|
||||
# Global lock for os.environ writes. Per-session locks (_agent_lock) prevent
|
||||
# concurrent runs of the SAME session, but two DIFFERENT sessions can still
|
||||
|
|
@ -2361,6 +2362,9 @@ def _run_background_title_update(session_id: str, user_text: str, assistant_text
|
|||
_put_title_status(put_event, session_id, 'skipped', 'already_generated', str(s.title or ''))
|
||||
return
|
||||
current = str(s.title or '').strip()
|
||||
if session_has_manual_title(s):
|
||||
_put_title_status(put_event, session_id, 'skipped', 'manual_title', current)
|
||||
return
|
||||
still_auto = (
|
||||
current == placeholder_title
|
||||
or current in ('Untitled', 'New Chat', '')
|
||||
|
|
@ -2405,6 +2409,7 @@ def _run_background_title_update(session_id: str, user_text: str, assistant_text
|
|||
if cached_session is not None and getattr(cached_session, 'session_id', None) == session_id:
|
||||
s = cached_session
|
||||
effective_title = str(s.title or '').strip()
|
||||
manual_title = session_has_manual_title(s)
|
||||
invalid_existing_now = _looks_invalid_generated_title(s.title)
|
||||
still_auto = (
|
||||
effective_title == placeholder_title
|
||||
|
|
@ -2412,12 +2417,12 @@ def _run_background_title_update(session_id: str, user_text: str, assistant_text
|
|||
or _is_provisional_title(effective_title, s.messages)
|
||||
or invalid_existing_now
|
||||
)
|
||||
if not still_auto:
|
||||
if manual_title or not still_auto:
|
||||
_put_title_status(put_event, session_id, 'skipped', 'manual_title', effective_title)
|
||||
return
|
||||
if next_title != effective_title:
|
||||
s.title = next_title
|
||||
s.llm_title_generated = True
|
||||
mark_session_title_generated(s)
|
||||
# Keep chronological ordering stable in the sidebar.
|
||||
s.save(touch_updated_at=False)
|
||||
effective_title = s.title
|
||||
|
|
@ -2450,6 +2455,9 @@ def _run_background_title_refresh(session_id: str, user_text: str, assistant_tex
|
|||
return
|
||||
# Safety: skip if user manually renamed since the check
|
||||
effective = str(s.title or '').strip()
|
||||
if session_has_manual_title(s):
|
||||
_put_title_status(put_event, session_id, 'skipped', 'manual_title', effective)
|
||||
return
|
||||
if effective != current_title:
|
||||
_put_title_status(put_event, session_id, 'skipped', 'manual_title', effective)
|
||||
return
|
||||
|
|
@ -2482,11 +2490,11 @@ def _run_background_title_refresh(session_id: str, user_text: str, assistant_tex
|
|||
if cached_session is not None and getattr(cached_session, 'session_id', None) == session_id:
|
||||
s = cached_session
|
||||
# Re-check: user may have renamed while we were generating
|
||||
if str(s.title or '').strip() != current_title:
|
||||
if session_has_manual_title(s) or str(s.title or '').strip() != current_title:
|
||||
_put_title_status(put_event, session_id, 'skipped', 'manual_title', str(s.title or '').strip())
|
||||
return
|
||||
s.title = next_title
|
||||
s.llm_title_generated = True
|
||||
mark_session_title_generated(s)
|
||||
effective_title = s.title
|
||||
# Session.save() calls _write_session_index(), which acquires LOCK.
|
||||
# Keep the per-session agent lock for mutation serialization, but
|
||||
|
|
@ -2631,6 +2639,8 @@ def _maybe_schedule_title_refresh(session, put_event, agent):
|
|||
current_title = str(session.title or '').strip()
|
||||
if not current_title or current_title in ('Untitled', 'New Chat'):
|
||||
return
|
||||
if session_has_manual_title(session):
|
||||
return
|
||||
if not getattr(session, 'llm_title_generated', False):
|
||||
return
|
||||
exchange_count = _count_exchanges(session.messages)
|
||||
|
|
|
|||
232
tests/test_3230_preserve_manual_session_title.py
Normal file
232
tests/test_3230_preserve_manual_session_title.py
Normal file
|
|
@ -0,0 +1,232 @@
|
|||
"""Regression coverage for preserving manually named sessions (#3230)."""
|
||||
import contextlib
|
||||
import json
|
||||
import threading
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
import api.config as config
|
||||
import api.models as models
|
||||
import api.profiles as profiles_api
|
||||
import api.streaming as streaming
|
||||
from api.models import Session
|
||||
from api.session_ops import apply_session_title_rename, mark_session_title_generated
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolate_state(tmp_path, monkeypatch):
|
||||
session_dir = tmp_path / "sessions"
|
||||
session_dir.mkdir()
|
||||
index_file = session_dir / "_index.json"
|
||||
monkeypatch.setattr(models, "SESSION_DIR", session_dir)
|
||||
monkeypatch.setattr(models, "SESSION_INDEX_FILE", index_file)
|
||||
monkeypatch.setattr(config, "SESSION_INDEX_FILE", index_file, raising=False)
|
||||
models.SESSIONS.clear()
|
||||
config.STREAMS.clear()
|
||||
config.CANCEL_FLAGS.clear()
|
||||
config.AGENT_INSTANCES.clear()
|
||||
config.SESSION_AGENT_LOCKS.clear()
|
||||
yield session_dir
|
||||
models.SESSIONS.clear()
|
||||
config.STREAMS.clear()
|
||||
config.CANCEL_FLAGS.clear()
|
||||
config.AGENT_INSTANCES.clear()
|
||||
config.SESSION_AGENT_LOCKS.clear()
|
||||
|
||||
|
||||
def _user_msg(text):
|
||||
return {"role": "user", "content": text}
|
||||
|
||||
|
||||
def _assistant_msg(text):
|
||||
return {"role": "assistant", "content": text}
|
||||
|
||||
|
||||
def _exchange_messages(count):
|
||||
messages = []
|
||||
for idx in range(count):
|
||||
messages.append(_user_msg(f"question {idx}"))
|
||||
messages.append(_assistant_msg(f"answer {idx}"))
|
||||
return messages
|
||||
|
||||
|
||||
def _put_event(events):
|
||||
def put(name, data):
|
||||
events.append((name, data))
|
||||
|
||||
return put
|
||||
|
||||
|
||||
def test_rename_marks_title_manual_and_persists_flag():
|
||||
session = Session(
|
||||
session_id="manual-title-session",
|
||||
title="Old generated title",
|
||||
messages=_exchange_messages(1),
|
||||
llm_title_generated=True,
|
||||
)
|
||||
|
||||
apply_session_title_rename(session, "Long-lived Work Plan")
|
||||
session.save()
|
||||
|
||||
loaded = Session.load("manual-title-session")
|
||||
assert loaded.title == "Long-lived Work Plan"
|
||||
assert loaded.manual_title is True
|
||||
assert loaded.llm_title_generated is False
|
||||
assert loaded.compact()["manual_title"] is True
|
||||
|
||||
payload = json.loads(session.path.read_text(encoding="utf-8"))
|
||||
assert payload["manual_title"] is True
|
||||
assert payload["llm_title_generated"] is False
|
||||
|
||||
|
||||
def test_blank_or_auto_title_clears_manual_lock():
|
||||
session = Session(
|
||||
session_id="clear-manual-title",
|
||||
title="Pinned investigation",
|
||||
manual_title=True,
|
||||
llm_title_generated=True,
|
||||
)
|
||||
|
||||
apply_session_title_rename(session, " ")
|
||||
assert session.title == "Untitled"
|
||||
assert session.manual_title is False
|
||||
assert session.llm_title_generated is False
|
||||
|
||||
apply_session_title_rename(session, "New Chat")
|
||||
assert session.title == "New Chat"
|
||||
assert session.manual_title is False
|
||||
assert session.llm_title_generated is False
|
||||
|
||||
|
||||
def test_adaptive_refresh_skips_manual_title_even_at_configured_interval():
|
||||
session = Session(
|
||||
session_id="manual-refresh-skip",
|
||||
title="Release Review Notes",
|
||||
messages=_exchange_messages(5),
|
||||
llm_title_generated=True,
|
||||
manual_title=True,
|
||||
)
|
||||
|
||||
with patch("api.streaming._get_title_refresh_interval", return_value=5), \
|
||||
patch("threading.Thread") as thread_cls:
|
||||
streaming._maybe_schedule_title_refresh(session, lambda *_args: None, agent=None)
|
||||
|
||||
assert thread_cls.called is False
|
||||
|
||||
|
||||
def test_adaptive_refresh_still_schedules_for_generated_titles_without_manual_lock():
|
||||
session = Session(
|
||||
session_id="generated-refresh",
|
||||
title="Generated Session Title",
|
||||
messages=_exchange_messages(5),
|
||||
llm_title_generated=True,
|
||||
manual_title=False,
|
||||
)
|
||||
|
||||
with patch("api.streaming._get_title_refresh_interval", return_value=5), \
|
||||
patch("threading.Thread") as thread_cls:
|
||||
thread = MagicMock()
|
||||
thread_cls.return_value = thread
|
||||
streaming._maybe_schedule_title_refresh(session, lambda *_args: None, agent=None)
|
||||
|
||||
assert thread_cls.called is True
|
||||
assert thread.start.called is True
|
||||
|
||||
|
||||
def test_generated_title_clears_manual_lock():
|
||||
session = Session(
|
||||
session_id="generated-clears-manual",
|
||||
title="Manual Before Regenerate",
|
||||
manual_title=True,
|
||||
llm_title_generated=False,
|
||||
)
|
||||
|
||||
mark_session_title_generated(session)
|
||||
|
||||
assert session.manual_title is False
|
||||
assert session.llm_title_generated is True
|
||||
|
||||
|
||||
def test_cleared_title_allows_initial_auto_generation(monkeypatch):
|
||||
session = Session(
|
||||
session_id="cleared-title-auto-generation",
|
||||
title="Untitled",
|
||||
messages=_exchange_messages(1),
|
||||
manual_title=False,
|
||||
llm_title_generated=False,
|
||||
)
|
||||
session.save = MagicMock()
|
||||
events = []
|
||||
|
||||
monkeypatch.setattr(streaming, "get_session", lambda _sid: session)
|
||||
monkeypatch.setattr(streaming, "SESSIONS", {session.session_id: session})
|
||||
monkeypatch.setattr(streaming, "LOCK", threading.Lock())
|
||||
monkeypatch.setattr(streaming, "_aux_title_configured", lambda: True)
|
||||
monkeypatch.setattr(
|
||||
streaming,
|
||||
"_generate_llm_session_title_via_aux",
|
||||
lambda *_args, **_kwargs: ("Generated Follow-up Title", "llm_ok", "raw"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
profiles_api,
|
||||
"profile_env_for_background_worker",
|
||||
lambda *_args, **_kwargs: contextlib.nullcontext(),
|
||||
)
|
||||
|
||||
streaming._run_background_title_update(
|
||||
session.session_id,
|
||||
"question",
|
||||
"answer",
|
||||
"Untitled",
|
||||
_put_event(events),
|
||||
agent=None,
|
||||
)
|
||||
|
||||
assert session.title == "Generated Follow-up Title"
|
||||
assert session.manual_title is False
|
||||
assert session.llm_title_generated is True
|
||||
assert any(name == "title" and data["title"] == "Generated Follow-up Title" for name, data in events)
|
||||
|
||||
|
||||
def test_clear_endpoint_resets_manual_title_lock():
|
||||
"""Clearing a manually-named session must drop the manual_title lock so the
|
||||
reused session can auto-name again (#3542 gap: /api/session/clear reset the
|
||||
title directly, leaving manual_title=True). The endpoint now routes the
|
||||
title reset through apply_session_title_rename — assert that helper clears
|
||||
the lock for the auto-label the clear path uses."""
|
||||
session = Session(
|
||||
session_id="clear-endpoint-manual-title",
|
||||
title="My Deliberate Name",
|
||||
messages=_exchange_messages(2),
|
||||
manual_title=True,
|
||||
llm_title_generated=False,
|
||||
)
|
||||
|
||||
# This mirrors exactly what /api/session/clear now does for the title.
|
||||
apply_session_title_rename(session, "Untitled")
|
||||
|
||||
assert session.title == "Untitled"
|
||||
assert session.manual_title is False, (
|
||||
"clearing a manually-named session must drop the manual_title lock"
|
||||
)
|
||||
assert session.llm_title_generated is False
|
||||
|
||||
|
||||
def test_clear_route_uses_rename_helper_not_bare_title_assignment():
|
||||
"""Static guard: the /api/session/clear handler must reset the title via
|
||||
apply_session_title_rename (which clears manual_title), not a bare
|
||||
`s.title = "Untitled"` that would strand the manual-title lock (#3542)."""
|
||||
import pathlib
|
||||
routes_src = (pathlib.Path(__file__).resolve().parent.parent / "api" / "routes.py").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
clear_idx = routes_src.find('if parsed.path == "/api/session/clear"')
|
||||
assert clear_idx != -1, "/api/session/clear handler not found"
|
||||
# Window from the clear handler to the next route branch.
|
||||
next_idx = routes_src.find('if parsed.path == "/api/session/truncate"', clear_idx)
|
||||
clear_block = routes_src[clear_idx:next_idx if next_idx != -1 else clear_idx + 2000]
|
||||
assert "apply_session_title_rename(s, \"Untitled\")" in clear_block, (
|
||||
"clear handler must reset the title via apply_session_title_rename to "
|
||||
"clear the manual_title lock"
|
||||
)
|
||||
Loading…
Reference in a new issue