fix(agent): upgrade provisional greeting titles
The model title for a bare greeting ("Friendly greeting") is persisted at
derived authority instead of llm, so it is a placeholder like the instant
title rather than the session's final name.
Part of #113864
Co-authored-by: eminogrande <eminogrande@users.noreply.github.com>
This commit is contained in:
@@ -81,6 +81,12 @@ _EXAMPLE_ECHO_REJECT = frozenset(
|
||||
t.lower() for t in _PROMPT_GOOD_EXAMPLES if t != "Friendly greeting"
|
||||
) | {_PROMPT_VAGUE_EXAMPLE.lower()}
|
||||
|
||||
# A generic greeting is intentionally offered as a title for a conversation
|
||||
# with no topic yet. Keep it at derived authority so the first substantive
|
||||
# follow-up can replace it with an LLM title.
|
||||
_PROVISIONAL_GREETING_TITLES = frozenset({"friendly greeting", "friendly greeting in chat"})
|
||||
|
||||
|
||||
_TITLE_PROMPT_TEMPLATE = (
|
||||
"You name chat sessions. Given the user's opening message, write a title "
|
||||
"that lets them find this conversation again in a list.\n\n"
|
||||
@@ -423,6 +429,18 @@ def _has_upgraded_title(session_db, session_id: str) -> bool:
|
||||
return True
|
||||
|
||||
|
||||
def _has_provisional_greeting_title(session_db, session_id: str) -> bool:
|
||||
"""Whether the stored title is the generic greeting placeholder."""
|
||||
try:
|
||||
return (
|
||||
session_db.get_session_title_source(session_id) == "derived"
|
||||
and str(session_db.get_session_title(session_id) or "").strip().lower()
|
||||
in _PROVISIONAL_GREETING_TITLES
|
||||
)
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
def _persist_session_title(session_db, session_id, title, *, source, dedupe=True):
|
||||
"""Persist at *source* authority via ``set_auto_title`` (precedence check + write in one
|
||||
transaction, so a manual ``/title`` is never overwritten); None when a higher authority held the row.
|
||||
@@ -503,6 +521,8 @@ def auto_title_session(
|
||||
title, source = generate_title(
|
||||
user_message, failure_callback=failure_callback, main_runtime=main_runtime, runtime_validator=runtime_validator,
|
||||
), "llm"
|
||||
if title and title.strip().lower() in _PROVISIONAL_GREETING_TITLES:
|
||||
source = "derived"
|
||||
if not title: # the inline attempt declined collisions; off the critical path the lineage scan is affordable
|
||||
title, source = derive_title(user_message), "derived"
|
||||
if not title:
|
||||
@@ -579,7 +599,8 @@ def maybe_auto_title(
|
||||
# History may be pre- or post-message. Skip only when BOTH past the opening turn AND named: count alone
|
||||
# left a machinery-opened session nameless; title alone never titles on an old store.
|
||||
user_msg_count = sum(1 for m in (conversation_history or []) if _is_real_user_turn(m))
|
||||
if user_msg_count > 1 and not _session_is_untitled(session_db, session_id):
|
||||
if (user_msg_count > 1 and not _session_is_untitled(session_db, session_id)
|
||||
and not _has_provisional_greeting_title(session_db, session_id)):
|
||||
return
|
||||
kanban_title = _kanban_task_title()
|
||||
if kanban_title:
|
||||
|
||||
@@ -608,6 +608,38 @@ class TestMaybeAutoTitle:
|
||||
assert db.get_session_title("sess-1") == "Existing name"
|
||||
mock_auto.assert_not_called()
|
||||
|
||||
def test_upgrades_a_provisional_greeting_on_a_substantive_second_turn(self, tmp_path):
|
||||
"""A canned greeting title must not prevent the next real request from naming the session."""
|
||||
db = SessionDB(tmp_path / "state.db")
|
||||
db.create_session(session_id="sess-1", source="cli")
|
||||
greeting = MagicMock()
|
||||
greeting.choices[0].message.content = "Friendly greeting"
|
||||
with patch("agent.title_generator.call_llm", return_value=greeting):
|
||||
auto_title_session(db, "sess-1", "hi how are you")
|
||||
|
||||
assert db.get_session_title("sess-1") == "Friendly greeting"
|
||||
assert db.get_session_title_source("sess-1") == "derived"
|
||||
|
||||
history = [
|
||||
{"role": "user", "content": "hi how are you"},
|
||||
{"role": "assistant", "content": "I'm well, thanks."},
|
||||
]
|
||||
with patch("agent.title_generator.auto_title_session") as mock_auto:
|
||||
import threading
|
||||
called = threading.Event()
|
||||
mock_auto.side_effect = lambda *a, **k: called.set()
|
||||
maybe_auto_title(db, "sess-1", "help me debug the scheduler", history)
|
||||
assert called.wait(timeout=10), "auto-title upgrade thread never ran"
|
||||
mock_auto.assert_called_once()
|
||||
|
||||
substantive = MagicMock()
|
||||
substantive.choices[0].message.content = "Debug scheduler failures"
|
||||
with patch("agent.title_generator.call_llm", return_value=substantive):
|
||||
auto_title_session(db, "sess-1", "help me debug the scheduler")
|
||||
|
||||
assert db.get_session_title("sess-1") == "Debug scheduler failures"
|
||||
assert db.get_session_title_source("sess-1") == "llm"
|
||||
|
||||
def test_instant_title_declines_a_name_collision(self, tmp_path):
|
||||
"""A colliding derived title is skipped, not scanned into 'hi #2'.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user