fix(agent): a placeholder title gets re-titled by the first substantive turn

maybe_auto_title used to skip any turn past the opener once the session had a
title of ANY provenance, so a greeting opener locked the name for good: the
instant "hi how are you" (derived) blocked the demoted greeting title, and
nothing ever asked again. Gate the skip on an llm/user title instead, so a
derived placeholder is replaced by the first real request; cap the retry at
turn 3 so a failing title model does not cost one call per turn. Untitled
sessions keep their unlimited retry.

Detect the greeting placeholder by prefix ("Friendly greeting in chat" too)
and keep it as a prompt example on purpose: a predictable placeholder is
detectable, an improvised one would land as llm and lock the title again.

Fixes #113864

Co-authored-by: eminogrande <eminogrande@users.noreply.github.com>
This commit is contained in:
teknium1
2026-09-18 00:47:12 -07:00
committed by Teknium
parent cc19fcf949
commit f85d9910fa
2 changed files with 52 additions and 44 deletions

View File

@@ -81,10 +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"})
# "Friendly greeting" is what the prompt asks for when the opener has no topic yet, so
# it is the one model title that must NOT settle the session: it is persisted at
# ``derived`` authority (a placeholder, like the instant title) and the next substantive
# turn upgrades it. Kept as a prompt example on purpose — a predictable placeholder is
# detectable, an improvised one ("Casual check-in chat") would lock the title as ``llm``.
_PROVISIONAL_GREETING_TITLE = "friendly greeting"
_TITLE_PROMPT_TEMPLATE = (
@@ -320,6 +322,12 @@ def _notify_title(title_callback: Optional[TitleCallback], title: str, source: s
_safe_callback(title_callback, (title, source), "%s callback failed", label)
def _is_provisional_greeting_title(title: str) -> bool:
"""The prompt's greeting placeholder (also "Friendly greeting in chat" and quoted/bracketed variants)."""
normalized = re.sub(r"^[\W_]+|[\W_]+$", "", title.strip(), flags=re.UNICODE).lower()
return normalized.startswith(_PROVISIONAL_GREETING_TITLE)
def _is_prompt_example_echo(title: str) -> bool:
"""Return True when *title* is one of the prompt's own example titles.
@@ -393,7 +401,7 @@ def generate_title(
# ignored the task and answered the user's message instead ("I don't have context on X — that's not
# something I recognize..."). Truncating would store half an assistant blob as the session title,
# which is still an assistant blob — reject instead so the caller retries on the next exchange
# (maybe_auto_title fires for the first two exchanges). Port of can1357/oh-my-pi#7306.
# (maybe_auto_title retries a placeholder title through the third exchange). Port of can1357/oh-my-pi#7306.
if title is not None and len(title.split()) > _MAX_TITLE_WORDS:
# Answer-shaped output: reject (not truncate) so the caller retries next exchange.
logger.debug("Rejecting answer-shaped title output (%d words > %d)", len(title.split()), _MAX_TITLE_WORDS)
@@ -429,18 +437,6 @@ 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.
@@ -521,7 +517,7 @@ 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:
if title and _is_provisional_greeting_title(title):
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"
@@ -596,11 +592,16 @@ def maybe_auto_title(
"""Instant inline title, then a daemon-thread upgrade. Call at the START of a turn, before the model."""
if not session_db or not session_id or not user_message:
return
# 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.
# History may be pre- or post-message. Past the opening turn, skip once the session holds an
# ``llm``/``user`` name: count alone left a machinery-opened session nameless, and a ``derived``
# name is still a placeholder (instant slice, or the model's greeting title for a bare "hi") that
# the first substantive turn should replace. Untitled sessions always get another shot; a
# placeholder gets turns 2-3, so a failing title model costs at most three calls, not one per turn.
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)
and not _has_provisional_greeting_title(session_db, session_id)):
if user_msg_count > 1 and (
_has_upgraded_title(session_db, session_id)
or (user_msg_count > 3 and not _session_is_untitled(session_db, session_id))
):
return
kanban_title = _kanban_task_title()
if kanban_title:

View File

@@ -8,6 +8,7 @@ from agent.title_generator import (
generate_title,
auto_title_session,
maybe_auto_title,
wait_for_title_upgrades,
_title_language,
)
from hermes_state import SessionDB
@@ -609,37 +610,43 @@ class TestMaybeAutoTitle:
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."""
"""A bare "hi" opener leaves only placeholders (instant slice / the model's greeting title);
the next real request must still be allowed to name 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")
answers = iter(["Friendly greeting", "Debug scheduler failures"])
assert db.get_session_title("sess-1") == "Friendly greeting"
assert db.get_session_title_source("sess-1") == "derived"
def stub_call_llm(**kwargs):
resp = MagicMock()
resp.choices[0].message.content = next(answers)
resp.choices[0].message.reasoning = None
return resp
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()
history = [{"role": "user", "content": "hi how are you"}]
with patch("agent.title_generator.call_llm", side_effect=stub_call_llm), \
patch("agent.title_generator._auto_title_enabled", return_value=True), \
patch("agent.title_generator._model_title_upgrade_enabled", return_value=True):
maybe_auto_title(db, "sess-1", "hi how are you", history)
wait_for_title_upgrades(10)
assert db.get_session_title_source("sess-1") == "derived"
history += [{"role": "assistant", "content": "Well, thanks."},
{"role": "user", "content": "help me debug the scheduler"}]
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")
wait_for_title_upgrades(10)
assert db.get_session_title("sess-1") == "Debug scheduler failures"
assert db.get_session_title_source("sess-1") == "llm"
def test_a_placeholder_title_stops_retrying_after_the_third_turn(self, tmp_path):
"""A derived name gets turns 2-3 to upgrade, not a model call on every later turn."""
db = SessionDB(tmp_path / "state.db")
db.create_session(session_id="sess-1", source="cli")
db.set_auto_title("sess-1", "hi", source="derived")
history = [{"role": "user", "content": f"turn {n}"} for n in range(4)]
with patch("agent.title_generator.auto_title_session") as mock_auto:
maybe_auto_title(db, "sess-1", "and now the real question", history)
mock_auto.assert_not_called()
def test_instant_title_declines_a_name_collision(self, tmp_path):
"""A colliding derived title is skipped, not scanned into 'hi #2'.